Skip to content

Enrich Phabricator webhook comment context - #6516

Open
ayoubdiourin7 wants to merge 3 commits into
mozilla:masterfrom
ayoubdiourin7:feature/phabricator-inline-comment-context
Open

Enrich Phabricator webhook comment context#6516
ayoubdiourin7 wants to merge 3 commits into
mozilla:masterfrom
ayoubdiourin7:feature/phabricator-inline-comment-context

Conversation

@ayoubdiourin7

@ayoubdiourin7 ayoubdiourin7 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Structure webhook-triggered Phabricator comments as XML with comment_id, type, and, for inline comments, diff_id.
  • Add coverage for normal comments, inline comments, mixed comments, and XML escaping.

Resolve #6434

@ayoubdiourin7
ayoubdiourin7 force-pushed the feature/phabricator-inline-comment-context branch from 8efb917 to a5a8f36 Compare August 6, 2026 11:42
@ayoubdiourin7
ayoubdiourin7 marked this pull request as ready for review August 6, 2026 11:48
@ayoubdiourin7
ayoubdiourin7 requested a review from a team as a code owner August 6, 2026 11:48
Comment thread agents/bug-fix/hackbot_agents/bug_fix/prompts/follow-up.md Outdated
Comment thread agents/bug-fix/hackbot_agents/bug_fix/prompts/follow-up.md Outdated
Comment thread services/hackbot-api/app/phabricator_webhook.py Outdated
Comment thread services/hackbot-api/app/phabricator_webhook.py Outdated
Comment thread services/hackbot-api/app/phabricator_webhook.py Outdated
author_phid: str
comment_id: int
comment_type: Literal["comment", "inline"]
diff_id: int | None = None

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have it all the time, it would be nice. But if it will increase complexity (e.g., extra http requests), let us file a follow-up issue and do it later, not part of this PR.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All metadata added here is already returned by the existing transaction.search call, so no additional HTTP request or lookup is needed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean to have it even for regular comments as well.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed it in bea1a18

Comment thread services/hackbot-api/app/phabricator_webhook.py Outdated
@ayoubdiourin7
ayoubdiourin7 force-pushed the feature/phabricator-inline-comment-context branch from 83c7f11 to c8b12fe Compare August 6, 2026 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enrich the comment context when triggering a run through a Phabricator comment

2 participants