Skip to content

feat(review): port reviewbot blockers[]/nits[] shape for extensive reviews - #1089

Merged
JSONbored merged 1 commit into
mainfrom
feat/review-depth-blockers-nits
Jun 23, 2026
Merged

feat(review): port reviewbot blockers[]/nits[] shape for extensive reviews#1089
JSONbored merged 1 commit into
mainfrom
feat/review-depth-blockers-nits

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Why

The free Workers-AI pair was producing shallow, hedging reviews ("appears to…", "seems to…") even with the diff fully present. Confirmed via #1049: the stored file payloads contain real patches (has_patch=1, 5–6 KB each) and the AI re-ran on the re-run checkbox — so the diff was never the problem. The gap is the prompt structure.

reviewbot's gold-standard depth comes from forcing the model to enumerate findings into explicit blockers[] (concrete must-fix) + nits[] (non-blocking) arrays under a severity rubric — not an open-ended "be thorough".

What

Port reviewbot's recipe into src/services/ai-review.ts:

  • REVIEW_SYSTEM_PROMPTblockers[]/nits[]/suggestions arrays + severity discipline (a blocker points to a real diff defect; nits/hypotheticals never block; CI status is never a code finding), "do not rubber-stamp", no hedging.
  • ModelReview / parseModelReview → parse blockers + nits (finding cap 6 → 12).
  • composeAdvisoryNotes → render Blockers + Nits (nits + suggestions merged).
  • consensusDefectOf → a consensus defect = a concrete blocker in both reviews (severity-disciplined; a lone blocker is a split, not a hard block) — replaces the numeric confidence floor.

Internal to ai-review.ts: the external advisoryNotes string + consensusDefect types are unchanged, so the gate and unified comment are unaffected.

Tests

Full suite green (3483 passed). Updated ai-review, ai-review-advisory, queue fixtures/assertions to the new shape; added a nits-don't-block consensus test.

@dosubot dosubot Bot added the size:M label Jun 23, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.76%. Comparing base (01c315b) to head (9947d13).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1089      +/-   ##
==========================================
- Coverage   94.76%   94.76%   -0.01%     
==========================================
  Files         153      153              
  Lines       18542    18540       -2     
  Branches     6711     6705       -6     
==========================================
- Hits        17572    17570       -2     
  Misses        418      418              
  Partials      552      552              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored force-pushed the feat/review-depth-blockers-nits branch from ff3a875 to 5f9e587 Compare June 23, 2026 17:48
…views

The free Workers-AI pair produced shallow, hedging reviews under the open-ended
assessment/suggestions/risks/criticalDefect shape. reviewbot's gold-standard depth
comes from forcing the model to ENUMERATE findings via explicit blockers[] (concrete
must-fix defects) + nits[] (non-blocking) arrays with a severity rubric. Port it:

- REVIEW_SYSTEM_PROMPT: blockers[]/nits[]/suggestions arrays + severity discipline
  (a blocker points to a real diff defect; nits/hypotheticals never block; CI status
  is never a code finding), 'do not rubber-stamp', no hedging language.
- ModelReview / parseModelReview: parse blockers + nits (finding cap 6 -> 12).
- composeAdvisoryNotes: render **Blockers** + **Nits** (nits + suggestions merged).
- consensusDefectOf: a consensus defect = a concrete blocker in BOTH reviews
  (severity-disciplined; a lone blocker is a split) -> replaces the numeric floor.

Internal to services/ai-review.ts; the external advisoryNotes string + consensusDefect
types are unchanged, so the gate + unified comment are unaffected. Diff patches were
verified present in storage -- the gap was purely the prompt structure.
@JSONbored
JSONbored force-pushed the feat/review-depth-blockers-nits branch from 5f9e587 to 9947d13 Compare June 23, 2026 17:51
@JSONbored
JSONbored merged commit 2c35207 into main Jun 23, 2026
17 checks passed
@JSONbored
JSONbored deleted the feat/review-depth-blockers-nits branch June 23, 2026 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant