feat(review-kit): strip the model's preamble from the review#287
Conversation
defineReviewAgent posted `run.output.trim()` — the harness's raw stdout, which carries whatever the model said on its way to an answer: "Evidence confirmed.", "I have all evidence needed.", a draft findings list, "Writing the review now." Every kit-built reviewer publishes the search alongside the findings. This cannot be fixed in a charter, and #281's trap list did not include it because I only learned it after filing. Measured on a real reviewer against a 150-word cap: 537 words, then 256 once the ban was sharpened, then 191, then 187 after moving the entire output contract to the top of the charter where it is read first. Four rewrites of increasing severity bought 4 words. Stripping in code took the same reviewer to 153 on the next run. The reason is simple in hindsight: you cannot instruct a model not to think, only decline to publish the thinking. So the charter owns the SHAPE — which it gets right reliably, every review nailed the finding format — and the kit owns the BOUNDARY. Same reason agents/review strips its own trailing READY sentinel instead of trusting the model to omit it. reviewBody() cuts at the LAST verdict line: a model that drafts before writing emits two, and the real review is the final one. No verdict line means the model ignored the format — pass it through rather than post nothing, since for an advisory agent silence reads as approval. Exported so a lens with a bespoke handler gets the same boundary. Tests cover the three cases: preamble stripped, last-verdict-wins, and passthrough/empty. Note: `pnpm --filter @agentworkforce/review-kit typecheck` and test `traps 2-4` already fail on pristine origin/main in a local checkout — both assert trigger `paths`, which needs persona-kit >= 4.1.26 (#276). Unrelated to this change; verified by stashing it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a reviewBody function to strip the model's preamble and thinking process from the final review output, keeping only the content starting from the last verdict line. It also adds corresponding unit tests to verify this behavior. The review feedback suggests making the regular expression used to detect the verdict line more robust to handle common LLM formatting variations (such as casing and bolding styles) and adding tests to cover these cases.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| export function reviewBody(output: string): string { | ||
| const text = (output ?? '').trim(); | ||
| let start = -1; | ||
| for (const match of text.matchAll(/^\*\*Verdict:/gmu)) { | ||
| start = match.index ?? start; | ||
| } | ||
| return start >= 0 ? text.slice(start).trim() : text; | ||
| } |
There was a problem hiding this comment.
The current regular expression /^\*\*Verdict:/gmu is quite strict and might fail to match common variations in LLM outputs. For example, LLMs often output **Verdict**: Blocker (with the colon outside the bold asterisks), **Verdict:** Blocker (with the colon inside but closed bold), or use different casing like **verdict**: or **VERDICT**:.
Updating the regex to be case-insensitive and to support optional closing asterisks and spaces before the colon makes the preamble stripping much more robust against non-deterministic model formatting.
export function reviewBody(output: string): string {
const text = (output ?? '').trim();
let start = -1;
for (const match of text.matchAll(/^\*\*Verdict(?:\*\*)?\s*:/gimu)) {
start = match.index ?? start;
}
return start >= 0 ? text.slice(start).trim() : text;
}| test('reviewBody passes through a body with no verdict line', () => { | ||
| // The model ignored the format. A malformed review still beats silence — | ||
| // for an advisory agent, silence reads as approval. | ||
| assert.equal(reviewBody('no verdict here, just prose'), 'no verdict here, just prose'); | ||
| assert.equal(reviewBody(' '), ''); | ||
| assert.equal(reviewBody(''), ''); | ||
| }); |
There was a problem hiding this comment.
To ensure the robustness of the updated reviewBody regex, let's add test cases covering alternative bolding styles, spacing, and case-insensitivity variations.
| test('reviewBody passes through a body with no verdict line', () => { | |
| // The model ignored the format. A malformed review still beats silence — | |
| // for an advisory agent, silence reads as approval. | |
| assert.equal(reviewBody('no verdict here, just prose'), 'no verdict here, just prose'); | |
| assert.equal(reviewBody(' '), ''); | |
| assert.equal(reviewBody(''), ''); | |
| }); | |
| test('reviewBody passes through a body with no verdict line', () => { | |
| // The model ignored the format. A malformed review still beats silence — | |
| // for an advisory agent, silence reads as approval. | |
| assert.equal(reviewBody('no verdict here, just prose'), 'no verdict here, just prose'); | |
| assert.equal(reviewBody(' '), ''); | |
| assert.equal(reviewBody(''), ''); | |
| }); | |
| test('reviewBody handles alternative bolding and casing for the verdict line', () => { | |
| assert.equal(reviewBody('thinking\n**Verdict**: Blocker'), '**Verdict**: Blocker'); | |
| assert.equal(reviewBody('thinking\n**Verdict:** Blocker'), '**Verdict:** Blocker'); | |
| assert.equal(reviewBody('thinking\n**verdict**: Blocker'), '**verdict**: Blocker'); | |
| assert.equal(reviewBody('thinking\n**VERDICT** : Blocker'), '**VERDICT** : Blocker'); | |
| }); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3d7906844
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const match of text.matchAll(/^\*\*Verdict:/gmu)) { | ||
| start = match.index ?? start; |
There was a problem hiding this comment.
When the final review quotes a diff or example containing a later line beginning with **Verdict:—for example, inside a fenced block—the last-match rule treats that quoted text as the review boundary and drops the real verdict and all preceding findings. This is especially reachable when reviewing PR-controlled Markdown or prompt-like content; distinguish the actual top-level verdict from fenced/quoted occurrences rather than unconditionally selecting the last matching line.
Useful? React with 👍 / 👎.
From @gemini-code-assist's review: /^\*\*Verdict:/ assumed one exact format. Verified the concern is real — the old pattern misses three variants models actually produce: **Verdict**: Blocker (colon outside the bold) -> MISS **Verdict** : Blocker (space before colon) -> MISS **verdict:** Blocker (casing) -> MISS **Verdict: Blocker.** (what production emits) -> match A miss is not a graceful degradation here: an unmatched line publishes the entire preamble, which is the one thing this function exists to prevent. Being lax costs nothing; being strict costs the whole feature on a formatting wobble. The charter asks for one form and the production reviewer emits it — but the point of stripping in code was precisely that the model's output is not something you can legislate. The ^ anchor still does the real work: a verdict mentioned mid-sentence in prose is not matched. Covered by a test. Tests for all five variants plus the prose case, per the review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Adopted in I checked the concern rather than taking it on faith, and the old pattern does miss three variants:
The reason strictness is the wrong default here: a miss is not a graceful degradation. An unmatched verdict line publishes the entire preamble — the exact failure this function exists to prevent. Being lax costs nothing; being strict costs the whole feature on a formatting wobble. And the premise of stripping in code (rather than in the charter) was precisely that the model's output is not something you can legislate — so hard-coding one blessed format reintroduced the assumption I was trying to remove. The Tests added for all five variants plus the prose case, as suggested. |
1 participant
Follow-up to #283 (which closed #281). One trap the checklist couldn't have — I only found it after filing the issue.
The gap
defineReviewAgentpostsrun.output.trim(). That's the harness's raw stdout, so it carries the model's thinking:Every kit-built reviewer will publish its search alongside its findings.
Why the charter can't fix it
Measured on a real reviewer against a 150-word cap:
Four rewrites of increasing severity bought four words. You cannot instruct a model not to think — only decline to publish the thinking.
So: the charter owns the shape (it gets that right reliably — every review nailed the finding format), the kit owns the boundary. Same reason
agents/reviewstrips its own trailingREADYsentinel rather than trusting the model to omit it.The change
reviewBody()cuts at the last verdict line — a model that drafts before writing emits two, and the real review is the final one. No verdict line at all → pass through unchanged, because a malformed review still beats silence (for an advisory agent, silence reads as approval).Exported, so a lens with a bespoke handler gets the same boundary.
Three tests: preamble stripped, last-verdict-wins, passthrough/empty. All pass.
Note on the suite
typecheckand testtraps 2-4already fail on pristineorigin/mainin a local checkout — both assert triggerpaths, which needs persona-kit ≥ 4.1.26 (#276). Unrelated; verified by stashing this change and re-running.🤖 Generated with Claude Code