mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-26 16:07:53 +00:00
51bb19fa1b
* fix(github): drop redundant pull_request_review_comment fan-out noise
GitHub fires one pull_request_review_comment webhook per inline comment
attached to a pull_request_review submission, in addition to the single
pull_request_review event for the review itself. A bot reviewer like
CodeRabbit commonly leaves 20-30 inline comments per review, flooding
the webhook with near-duplicate deliveries that carry nothing an agent
doesn't already have -- it fetches every inline comment itself via
`gh api` when it processes the parent pull_request_review event.
Filter these out in fanout_event() before the registry lookup / per-
agent loop, since the redundancy is a property of the event itself, not
of any specific agent binding. A companion comment is identified by
pull_request_review_id being set (it belongs to a review) and
in_reply_to_id being absent (it is not itself a reply within an
existing thread -- that case is a genuine new interaction and must
still fire).
* fix(channels): scope review-comment suppression to bindings that also see the review
The redundant pull_request_review_comment filter suppressed every
companion comment unconditionally, before the registry lookup even ran.
That premise only holds for a binding that also subscribes to
pull_request_review on the same repo -- events are opt-in per binding,
so a binding registered for pull_request_review_comment alone never
receives the parent review event and the companion comments were its
only delivery of the review's inline content. Suppressing those too was
a silent, total loss for that binding, not noise reduction.
Move the check into the per-agent loop so it only suppresses a matched
binding's companion comment when that same binding also has an active
pull_request_review trigger on this repo, reusing the existing registry
lookup rather than re-walking bindings by hand. Bindings subscribed to
pull_request_review_comment alone now always fire, matching the
opt-in-per-binding contract documented in triggers.py.
* fix(channels): close require_mention gap in review-comment redundancy gate
willem-bd's second review round found that the per-binding redundancy gate
(commit b3791b33) treated a binding as "covered" purely by checking whether
it also registers a `pull_request_review` trigger on this repo, ignoring
whether that trigger itself requires a mention. A `pull_request_review_comment`
payload never carries the paired review's own top-level body, so there is no
way to verify from a comment delivery whether that trigger's own
`require_mention` check would actually pass. A human `@mention` living only
in one inline comment (not the review summary) could therefore be lost
twice: the review event filtered out by its own `no_mention` gate, and the
one inline comment that carries the mention dropped here as "redundant" --
the same silent-loss shape as the original bug, through a narrower path.
The gate now also requires the paired `pull_request_review` trigger's
resolved `require_mention` to be false before treating it as coverage,
trading a small amount of residual redundancy (an extra companion delivery
when the review would have fired anyway) for zero silent loss.
While in the same code, also addressed two smaller review notes:
- The redundant-comment skip reason now prefers the companion's own trigger
verdict when that verdict is also a skip (e.g. its own `require_mention`
independently fails), instead of always reporting the generic
`redundant_review_comment` label.
- `_pr_review_prompt` now tells the agent to fetch a review's inline
comments via `gh api .../reviews/{id}/comments` -- the redundancy gate's
suppression is only genuinely redundant if the agent actually recovers
that content from the parent review event, and nothing previously told it
to (zhfeng's review).
Tests: reproduces willem-bd's exact scenario (dual-subscribed binding,
require_mention on the review trigger, mention present only in the inline
comment) and confirms it fails on the prior code and passes with the fix;
adds a multi-agent-independence test and a skip-reason-precedence test;
adds prompt tests locking the new fetch-hint text and its missing-id guard.
Fail-before/pass-after verified via patch-file revert (not stash, to avoid
colliding with sibling worktrees).
254 lines
8.4 KiB
Python
254 lines
8.4 KiB
Python
"""Tests for the GitHub webhook → prompt translator."""
|
|
|
|
from __future__ import annotations
|
|
|
|
from app.gateway.github.prompts import build_prompt
|
|
|
|
|
|
def test_pull_request_prompt_contains_core_fields() -> None:
|
|
payload = {
|
|
"action": "opened",
|
|
"pull_request": {
|
|
"number": 7,
|
|
"title": "Add webhook receiver",
|
|
"user": {"login": "zhfeng"},
|
|
"html_url": "https://github.com/a/b/pull/7",
|
|
"body": "This adds X and fixes Y.",
|
|
},
|
|
"repository": {"full_name": "a/b"},
|
|
}
|
|
prompt = build_prompt("pull_request", payload)
|
|
assert "pull request" in prompt.lower()
|
|
assert "#7" in prompt
|
|
assert "Add webhook receiver" in prompt
|
|
assert "zhfeng" in prompt
|
|
assert "https://github.com/a/b/pull/7" in prompt
|
|
assert "This adds X and fixes Y." in prompt
|
|
|
|
|
|
def test_pull_request_prompt_handles_missing_body() -> None:
|
|
payload = {
|
|
"action": "opened",
|
|
"pull_request": {"number": 1, "title": "x", "user": {}, "body": None},
|
|
"repository": {"full_name": "a/b"},
|
|
}
|
|
prompt = build_prompt("pull_request", payload)
|
|
assert "(no description)" in prompt
|
|
|
|
|
|
def test_pull_request_prompt_truncates_huge_body() -> None:
|
|
huge = "X" * 50000
|
|
payload = {
|
|
"action": "opened",
|
|
"pull_request": {"number": 1, "title": "x", "user": {}, "body": huge},
|
|
"repository": {"full_name": "a/b"},
|
|
}
|
|
prompt = build_prompt("pull_request", payload)
|
|
assert "truncated" in prompt
|
|
assert len(prompt) < 10000
|
|
|
|
|
|
def test_issue_comment_prompt_includes_body_verbatim() -> None:
|
|
payload = {
|
|
"action": "created",
|
|
"issue": {"number": 11, "pull_request": {"url": "..."}},
|
|
"comment": {
|
|
"body": "Hey @coding-llm-gateway please look at this",
|
|
"user": {"login": "zhfeng"},
|
|
"html_url": "https://github.com/a/b/issues/11#issuecomment-1",
|
|
},
|
|
"repository": {"full_name": "a/b"},
|
|
}
|
|
prompt = build_prompt("issue_comment", payload)
|
|
assert "pull request #11" in prompt # is_pr True
|
|
assert "@coding-llm-gateway please look at this" in prompt
|
|
assert "zhfeng" in prompt
|
|
|
|
|
|
def test_issue_comment_plain_issue_says_issue() -> None:
|
|
payload = {
|
|
"action": "created",
|
|
"issue": {"number": 12},
|
|
"comment": {"body": "x", "user": {"login": "u"}},
|
|
"repository": {"full_name": "a/b"},
|
|
}
|
|
assert "issue #12" in build_prompt("issue_comment", payload)
|
|
|
|
|
|
def test_issue_comment_prompt_includes_issue_title_and_body() -> None:
|
|
"""The comment alone isn't enough — the agent needs the issue context too.
|
|
|
|
The webhook payload already includes the parent issue/PR's title and body
|
|
on the ``issue`` object; we just have to render them.
|
|
"""
|
|
payload = {
|
|
"action": "created",
|
|
"issue": {
|
|
"number": 42,
|
|
"title": "Login button is broken",
|
|
"body": "Clicking login throws a 500. Repro: open /login, click submit.",
|
|
"user": {"login": "reporter"},
|
|
},
|
|
"comment": {
|
|
"body": "@bot what do you think?",
|
|
"user": {"login": "asker"},
|
|
},
|
|
"repository": {"full_name": "a/b"},
|
|
}
|
|
prompt = build_prompt("issue_comment", payload)
|
|
assert "Login button is broken" in prompt
|
|
assert "Clicking login throws a 500" in prompt
|
|
assert "reporter" in prompt # issue author, distinct from comment author
|
|
assert "@bot what do you think?" in prompt
|
|
|
|
|
|
def test_pull_request_review_comment_includes_pr_title_and_body() -> None:
|
|
payload = {
|
|
"action": "created",
|
|
"pull_request": {
|
|
"number": 9,
|
|
"title": "Refactor auth flow",
|
|
"body": "Splits AuthService into AuthN/AuthZ.",
|
|
"user": {"login": "author"},
|
|
},
|
|
"comment": {
|
|
"user": {"login": "alice"},
|
|
"path": "src/foo.py",
|
|
"line": 42,
|
|
"diff_hunk": "@@ -1 +1 @@\n-a\n+b",
|
|
"body": "consider renaming",
|
|
},
|
|
"repository": {"full_name": "a/b"},
|
|
}
|
|
prompt = build_prompt("pull_request_review_comment", payload)
|
|
assert "Refactor auth flow" in prompt
|
|
assert "Splits AuthService into AuthN/AuthZ." in prompt
|
|
assert "consider renaming" in prompt
|
|
|
|
|
|
def test_pull_request_review_prompt_includes_pr_title_and_body() -> None:
|
|
payload = {
|
|
"action": "submitted",
|
|
"pull_request": {
|
|
"number": 5,
|
|
"title": "Bump deps",
|
|
"body": "Updates everything to latest.",
|
|
"user": {"login": "author"},
|
|
},
|
|
"review": {
|
|
"state": "changes_requested",
|
|
"user": {"login": "alice"},
|
|
"body": "Looks risky",
|
|
},
|
|
"repository": {"full_name": "a/b"},
|
|
}
|
|
prompt = build_prompt("pull_request_review", payload)
|
|
assert "Bump deps" in prompt
|
|
assert "Updates everything to latest." in prompt
|
|
assert "changes_requested" in prompt
|
|
assert "Looks risky" in prompt
|
|
|
|
|
|
def test_pull_request_review_comment_includes_file_and_diff() -> None:
|
|
payload = {
|
|
"action": "created",
|
|
"pull_request": {"number": 9},
|
|
"comment": {
|
|
"user": {"login": "alice"},
|
|
"path": "src/foo.py",
|
|
"line": 42,
|
|
"diff_hunk": "@@ -1,3 +1,4 @@\n a\n+b\n c",
|
|
"body": "consider renaming",
|
|
},
|
|
"repository": {"full_name": "a/b"},
|
|
}
|
|
prompt = build_prompt("pull_request_review_comment", payload)
|
|
assert "src/foo.py:42" in prompt
|
|
assert "+b" in prompt
|
|
assert "consider renaming" in prompt
|
|
|
|
|
|
def test_pull_request_review_prompt_includes_state() -> None:
|
|
payload = {
|
|
"action": "submitted",
|
|
"pull_request": {"number": 5},
|
|
"review": {
|
|
"state": "changes_requested",
|
|
"user": {"login": "alice"},
|
|
"body": "Looks risky",
|
|
},
|
|
"repository": {"full_name": "a/b"},
|
|
}
|
|
prompt = build_prompt("pull_request_review", payload)
|
|
assert "changes_requested" in prompt
|
|
assert "Looks risky" in prompt
|
|
assert "alice" in prompt
|
|
|
|
|
|
def test_pull_request_review_prompt_instructs_fetching_inline_comments() -> None:
|
|
"""PR #4131 review (Concern 1, zhfeng): the dispatcher's redundant
|
|
review-comment gate assumes the agent recovers inline comment content
|
|
from the parent `pull_request_review` event -- nothing enforced that
|
|
before this. The prompt must tell the agent how, with the exact
|
|
`gh api` path GitHub uses for a review's inline comments.
|
|
"""
|
|
payload = {
|
|
"action": "submitted",
|
|
"pull_request": {"number": 5},
|
|
"review": {
|
|
"id": 999001,
|
|
"state": "changes_requested",
|
|
"user": {"login": "alice"},
|
|
"body": "Looks risky",
|
|
},
|
|
"repository": {"full_name": "a/b"},
|
|
}
|
|
prompt = build_prompt("pull_request_review", payload)
|
|
assert "gh api repos/a/b/pulls/5/reviews/999001/comments" in prompt
|
|
|
|
|
|
def test_pull_request_review_prompt_omits_fetch_hint_without_review_id() -> None:
|
|
"""Guard: never render a broken `.../reviews/None/comments` path when
|
|
the payload is missing `review.id` (or the PR number) -- omit the
|
|
instruction entirely rather than pointing the agent at a bad command.
|
|
"""
|
|
payload = {
|
|
"action": "submitted",
|
|
"pull_request": {"number": 5},
|
|
"review": {"state": "approved", "user": {"login": "alice"}, "body": "LGTM"},
|
|
"repository": {"full_name": "a/b"},
|
|
}
|
|
prompt = build_prompt("pull_request_review", payload)
|
|
assert "gh api" not in prompt
|
|
|
|
|
|
def test_issues_prompt() -> None:
|
|
payload = {
|
|
"action": "opened",
|
|
"issue": {
|
|
"number": 3,
|
|
"title": "bug",
|
|
"user": {"login": "u"},
|
|
"html_url": "https://github.com/a/b/issues/3",
|
|
"body": "things broken",
|
|
},
|
|
"repository": {"full_name": "a/b"},
|
|
}
|
|
prompt = build_prompt("issues", payload)
|
|
assert "#3" in prompt
|
|
assert "bug" in prompt
|
|
assert "things broken" in prompt
|
|
|
|
|
|
def test_ping_prompt() -> None:
|
|
payload = {"zen": "Practicality beats purity.", "hook": {"id": 42}}
|
|
prompt = build_prompt("ping", payload)
|
|
assert "ping" in prompt.lower()
|
|
assert "42" in prompt
|
|
|
|
|
|
def test_unknown_event_returns_generic_stub() -> None:
|
|
prompt = build_prompt("workflow_run", {"action": "completed", "repository": {"full_name": "a/b"}})
|
|
assert "workflow_run" in prompt
|
|
assert "a/b" in prompt
|