ReviewGate verdict guard rejects an APPROVE whose DISPOSITIONS explicitly mark the flagged findings [ADDRESSED] - #2022
Merged
ryanrborn merged 2 commits intoSep 24, 2026
Conversation
…itioned every open finding citing a dotfile bd-bm6bfs (emr-8fqbng, MR !294): the guard parked a green, correct D0 fix whose round-2 DISPOSITIONS block plainly addressed F1.2/F1.4, both of which cite `.gitlab-ci.yml`. The disposition PARSER was fine — the bug was in `ReviewFindings`'s `@path` regex, which dropped the leading dot off a dotfile (`.gitlab-ci.yml` -> `gitlab-ci.yml`). That mismatched against `git diff --name-only`'s own `.gitlab-ci.yml`, so the mechanical unproven-file backstop rejected every ADDRESSED disposition citing a dotfile, no matter how many retries were spent. Also make the park message name what it actually is: `disposition_banner_text/2` now quotes the disposition line the parser DID parse for an unaddressed/ unproven id (and says plainly when there was none), so a coordinator can tell a parser miss from a real omission without reading the raw transcript. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…gs text, and harden @path against a leading prose dot The bd-bm6bfs regression test paired round-2's dispositions with the wrong round-1 (a later, unrelated request-changes round about audit:hex/tflint) and had shortened the round-2 text too, so it didn't pin the real F1.1-F1.4 shape the incident hinged on. Replace both heredocs with the verbatim review_gate_rounds.findings text for the actual runs (0ddfed09 round 1, 0d06cc85 round 2). Also tighten @path's optional leading dot with a negative lookbehind so a preceding prose dot ("...config.exs") can no longer be pulled into the match as a false leading dot, which would otherwise diverge the captured path from touched?/2's basename comparison. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ryanrborn
deleted the
bugfix/2020-reviewgate-verdict-guard-rejects-approve-whose
branch
September 24, 2026 05:09
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ReviewGate's
:unaddressed_findingsverdict guard falsely parked a green,correct D0 fix (bd-bm6bfs / emr-8fqbng, MR !294): the round-2 reviewer's
DISPOSITIONS:block plainly addressed every open finding, but the guardstill rejected the
APPROVE, burning a re-prompt and forcing a coordinatorhand-ruling.
The disposition parser itself was never the problem —
ReviewFindings.dispositions/1correctly saw
F1.1–F1.4as dispositioned. The real bug was in the@pathregex used to extract cited file paths: it dropped the leading dot off a
dotfile, so a finding/disposition citing
.gitlab-ci.ymlwas recorded asciting
gitlab-ci.yml. That never matches a realgit diff --name-onlyentry (which keeps the dot), so the mechanical "unproven ADDRESSED"
backstop — meant to catch a reviewer that claims a fix landed in a file
nothing actually touched — fired incorrectly on every dotfile-citing
[ADDRESSED]disposition, no matter how many retries were spent.Fix: preserve a dotfile's leading dot when extracting cited paths
(
apps/arbiter/lib/arbiter/worker/review_findings.ex).Also hardened the guard's own park message:
disposition_banner_text/2nowtakes the round's findings text and quotes the actual disposition line the
parser matched for each unaddressed/unproven finding id (and says plainly
when there was none at all), so a coordinator reading a park can tell a
parser miss from a real omission without pulling the raw transcript.
Test plan
review_findings_test.exsreproducing the exactemr-8fqbng round-1/round-2 findings text:
dispositions/1sees allfour ids, and
approval_gap/3returns no gap once the fix lands(fails without it).
review_gate_test.exs(
round-2 APPROVE dispositioning a finding that cites a dotfile merges without a re-prompt) drives a realWorker/ReviewGateround-tripthrough a new
DOTFILEreviewer fixture mode and a pairedrevise_commit_dotfile.shimplementer fixture that really commits to.gitlab-ci.yml; asserts the round converges on the first attempt andthat the persisted
review_gate_rounds.findingscolumn is exactly thetext the guard evaluated.
[ADDRESSED]/[NOT ADDRESSED]line for unaddressed/unproven findings,and says "no disposition at all" only when the parser truly found
nothing.
pre-fix code, then pass after the fix (git-stash bisected).
(read from
review_gate_roundsvia the live DB):approval_gap/3now returns no gap for the exact text/finding shape that was
falsely rejected.
mix precommit(compile --warnings-as-errors, deps.unlock --unused,format, full test suite — 1 doctest, 6 properties, 7384 + 1711 + 887
tests) passes with 0 failures.
References
bd-bm6bfs
Closes #2020
🤖 Generated with Claude Code