Stop the reviewer re-posting inline findings it already delivered

[FeliLucero1]

The bug

base-pr-review.md Step 6 tells the model to "skip any issue that was already raised in an existing PR comment or inline review comment". Nothing ever gave it the inline ones.

fetch-pr-context.py builds existing_findings by scanning the bot's summary comments for lines starting with - that contain a backtick (L576-L582). Inline review threads are never collected. So an open inline finding on unchanged code is invisible to the next run, and gets posted again — byte for byte.

resolve-outdated-threads.py does not cover the gap either: it only auto-resolves threads GitHub marks isOutdated whose commenters are all bots. A current thread stays open and unrecorded, and once anyone replies it is permanently disqualified from auto-resolution too.

Measured

On ConductorOne/baton-admin#928, six pushes over about 25 minutes produced 19 inline comments for 11 distinct findings. I diffed four of the suspected duplicate pairs and all four were byte-identical re-posts. The 8 duplicates buried the genuinely new findings — which were real, and one of them was a production bug.

The fix

resolve-outdated-threads.py already fetches every thread with path, line, isResolved, isOutdated, comment bodies and authors, then discards all of it except the outdated ones it resolves. It now also writes two buckets into .github/resolved-threads.json:

• open_findings — reviewer findings still open. Each carries path, line, a truncated body, and has_human_reply.
• settled_findings — findings now resolved, whether by this run or by a human. (A human resolving a thread was the other invisible case: nothing recorded it.)

Bodies are capped at 600 chars and each bucket at 60 entries, with findings_truncated set so the model knows to be conservative when it is capped.

The prompt then uses them:

• Step 3 is rewritten from "note pre-resolved threads" to an inventory of what the reviewer has already said, and states plainly that re-posting is the most common defect in this review's output. It also gives the alternative: if a finding is still relevant, say so once in the Review Summary and leave the original thread to carry the detail.
• Step 6 now names all three sources a candidate must be checked against before posting.
• A thread with has_human_reply: true is off limits entirely — it has been discussed, so the summary is the only place to revisit it.

No new API calls: this is data the script already had in memory.

Not changed

Auto-resolution behavior. should_resolve is untouched, and there are tests asserting it did not widen — in particular that a thread with a human reply still never auto-resolves.

Testing

New test_resolve_outdated_threads.py, 14 cases over is_bot_finding_thread, finding_digest and the unchanged should_resolve.

While writing it I found that neither existing test file in this directory is run by anything — not make test-scripts, not any workflow. make test-scripts now runs the suite.

$ python3 -m unittest discover -s .github/actions/pr-review/scripts -p 'test_*.py'
Ran 38 tests in 0.006s
OK

(make test-scripts in full needs yq, which I do not have locally; the step that fails is test-release-workflow-tag-pin.sh, unrelated to this change.)

Related

• ConductorOne/baton-admin#928 — the log-level criteria work that surfaced this.
• ConductorOne/claude-marketplace#174 — the local review skills reading ci-review.md so CI and local reviewers stop contradicting each other.

🤖 Generated with Claude Code