feat(review): a verdict follows from the blockers, not from a confidence percentage - #28
Merged
Merged
Conversation
The cockpit's verdict chip read "comment · 72%" on a review whose own
findings were 3 minors and a nit — no blockers, no approval. The
percentage was never what withheld it: nothing derives the
recommendation from confidence. The prompt's verdict rule carried an
escape hatch — "approve (or comment when you genuinely take no
position)" — and the run took it, over a regression it had graded
minor. That is the hedge the prompt forbids one line earlier for
grades, moved one field over.
Close the hatch: blockers → request changes, none → approve, no third
option. If something would stop you approving it is a blocker, so grade
it. "comment" is left for a review that graded nothing and only asks
questions.
Then show the fact behind the verdict instead of the feeling. The chip,
the queue cell and the markdown export all name the live blocker count
("approve · no blockers") — something a reader can check against the
findings list, unlike how sure the AI reported being. Confidence stays
in the artifact as the auto-send threshold's dial and is shown nowhere.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e means
The blocker count in the queue's verdict column didn't fit — 136px of
column turned "approve · no blockers" into "approve · no bloc…", which
tells a reader less than either half alone would. The cell goes back to
the verdict and the percentage; the count moves down to the strip, which
has room to spell it out ("4 comments drafted, no blockers").
The percentage now means something narrower than it used to. With the
verdict following mechanically from the blockers, "should this merge" is
already answered, so confidence is left saying one thing: how sure the
review is of itself — findings real, grades right, nothing worth
blocking missed. The prompt says that in those words, and it now moves
on what the run could not check rather than on how the PR looks. On the
review page it sits beside the findings it is about, labelled "72% sure
of the findings" rather than left as a bare number.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two places lost it in the last change. The queue cell printed a bare "approve 78" — a number with no unit next to a word, which reads as a count of something. And the review page's chip dropped the number entirely when it took on the blocker count, so the one screen with room for both showed one. Both now carry the verdict, what it rests on, and how sure the review is of itself: "approve 78%" in the queue, "approve · no blockers · 78%" on the review. The "why" card keeps the spelled-out version that says which of the two the number is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
At 136px the column clipped its own longest label to "request changes 9…", eating the number the last change had just put there. The widest the cell can ever be is "request changes 100%", which measures 144.5px in the row's monospace face, so the column goes to 152px — verified in the browser rather than from character arithmetic, which is what got it wrong the first time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR tightens cerber’s review contract so the verdict is determined mechanically by live blockers (blockers → request changes; none → approve), while “confidence” is re-scoped to mean confidence in the review’s findings/grades (not confidence that merging is a good idea). It also updates the cockpit UI/export surfaces to show the blocker count where it helps readers validate the verdict, and relocates confidence to sit beside the findings it describes.
Changes:
- Adds
blockerCount/gradedCountto the queue payload and uses them to surface “no blockers / N blockers” in the queue strip. - Introduces
verdictBasis()and updates the review page chip + “why” card to show blocker basis and “% sure of the findings”. - Rewrites prompt/docs/help text so “confidence” is explicitly about the review’s correctness rather than the merge decision.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/types.ts | Extends queue item types with blockerCount / gradedCount and clarifies verdict confidence semantics. |
| web/src/styles.css | Widens the verdict column and adjusts “why” card layout for new basis/confidence display. |
| web/src/review.ts | Adds verdictBasis() to compute the blocker-based rationale string for verdict displays. |
| web/src/review.test.ts | Adds unit tests covering verdictBasis() behavior (including dropped blockers). |
| web/src/inbox.ts | Updates queue verdict cell to show “verdict + %” and moves blocker basis text into the strip meta. |
| web/src/inbox.test.ts | Updates/extends tests for the new verdict cell formatting and strip blocker-basis text. |
| web/src/Detail.tsx | Shows verdictBasis() in the review chip and adds explicit confidence wording in the “why” card. |
| src/server/index.ts | Adds blockerCount / gradedCount to the API payload for queue rows. |
| src/runner/prompt.ts | Reframes confidence as “confidence in the review” and tightens verdict rules in the reviewer prompt. |
| src/core/export.ts | Updates Markdown export to include blocker basis (when graded) and “% sure of the findings”. |
| src/core/artifact.ts | Updates verdict schema JSDoc to match the new confidence meaning. |
| src/cli/index.ts | Updates --auto-send-threshold help text to match the new confidence definition. |
| CLAUDE.md | Updates project principles/docs to reflect the stricter verdict/blocker rule and confidence semantics. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… "comment" Review feedback on #28, both findings correct. The verdict rule claimed there was no third option and then, one line later, reserved "comment" for a review that graded nothing — a contradiction to anyone reading the prompt top to bottom, and the kind that reintroduces the inconsistent recommendations this change exists to remove. Scope the rule to what it actually governs: once anything is graded there is nothing between approve and request changes, and "comment" is named separately as the one case that rule cannot reach. The queue payload's comment also went stale in the same series — it still said the verdict cell names the blocker count, which stopped being true when the count moved to the strip to stop the cell truncating. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🎉 This PR is included in version 0.20.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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
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.
The problem
A review of a PR with three minors and one nit — no blockers at all — came back "comment · 72%", and the cockpit gave no approval. Its own verdict said why: "…is the team's call, which is why I take no position on merging."
Two separate things were wrong.
The verdict had an escape hatch. The prompt's rule read "any blocker → request_changes; none → approve (or "comment" when you genuinely take no position)", and the run took the parenthesis — over a regression it had itself graded
minor. That is exactly the hedge the prompt forbids one line earlier for grades ("Never lower a grade to hedge doubt"); it just moved the hedge fromseveritytorecommendation. The result is a review that found nothing blocking and still withholds approval, which is the one thing a reviewer's verdict is for.The percentage was answering a question nobody asked.
72%was the loudest thing on the chip, and a reader can't check it against anything. Meanwhile the fact the verdict actually rests on — how many blockers are still standing — wasn't shown at all.The fix
Close the hatch. Blockers → request changes, none → approve. No third option. If something would stop you approving, that is a blocker: grade it.
commentis left for a review that graded nothing and only asks questions.Show the blocker count wherever there's room, because that is the fact a reader can check the verdict against:
comment · 72%request changes · 1 blocker · 92%approve 91approve 78%4 comments drafted4 comments drafted, no blockers⚠️ 3 minors · 🎨 1 nit⚠️ 3 minors · 🎨 1 nit · 78% sure of the findings.mdexport**Verdict: comment** (confidence 72%)**Verdict: approve** (no blockers · 78% sure of the findings)Narrow what the percentage means. With the verdict following from the blockers, "should this merge" is already answered — so confidence is left saying one thing, and the prompt now says it in those words:
It now moves on what the run could not check rather than on how the PR looks — which also makes the auto-send gate more coherent than it was:
≥90%now means the review vouches for itself, not the review feels good about merging.Technical details
src/runner/prompt.ts— the verdict rule loses its third option and gains an explicit "taking no position is not a verdict" clause; bothconfidencerules (source and diff-only) are rewritten around the review rather than the merge.web/src/review.ts— newverdictBasis():"no blockers"/"2 blockers", ornullfor a review drafted before severities existed, which makes no claim.src/server/index.ts→web/src/types.ts— the queue payload gainsblockerCount/gradedCount.web/src/inbox.ts— the narrow verdict cell keeps verdict + percentage; the count goes to the strip, which has room to spell it out.web/src/styles.css—.col-verdict136px → 152px. At 136 it clipped its own longest label torequest changes 9…; the widest it can ever hold isrequest changes 100%, measured at 144.5px in the row's monospace face.confidencestays on the artifact as the auto-send threshold's dial, with its jsdoc rewritten to say what it does and does not claim.What is deliberately not here
confidenceis still load-bearing in one place — the auto-send gate (--auto-send-threshold, default 90), the only path that writes to GitHub unattended. Auto-send already has a blocker backstop (autosend.tsrefuses an approve standing next to a live blocker), so the gate could become approve + no blockers and the field could go entirely. That rewrites the safety contract stated at the top ofCLAUDE.md, so it's a separate, deliberate decision rather than a side effect of this one.Also unchanged: no code derives the verdict from severity. The rule lives in the prompt;
verdictMismatch()still only points out a disagreement. That principle was reaffirmed, not dropped.Verification
pnpm typecheck && pnpm test— 369 passing (10 new:verdictBasis, the reworkedverdictCell, and the strip's blocker line).pnpm buildclean..col-verdictcell reportsscrollWidth === clientWidth(no clipping), and a hidden probe ofrequest changes 100%in the cell's own computed font measures 144.5px against the 152px column.request changes · 1 blocker · 92%unclipped; the "why" card renders🚨 1 blocker · ⚠️ 2 minors · 🎨 1 nit · 92% sure of the findings.CLAUDE.md's severity/verdict principle updated to match, since the prompt rules are meant to be changed from evidence and this PR is the evidence.