fix(review): report a withheld narrative honestly instead of as a provider failure - #9806
Merged
Merged
Conversation
…vider failure A review that PARSED cleanly but had every public field withheld by the sanitizer made composeAdvisoryNotes return null, which the orchestration misreports as "AI review is unavailable for this PR head" -- a PROVIDER failure -- and holds the PR for manual review. Because the head is unchanged, every re-run reproduces it: observed live on #9794, which re-ran with diagnostics `claude-code#0:parsed` and was re-held every time, including after explicit maintainer re-ticks. The shape is routine for this project's own scoring/gate code: an honest narrative about it says "score"/"ranking"/"reward" in every sentence, the per-sentence sanitizer drops them all, and a clean review (no blockers, no nits) has nothing left to publish. Publish a fixed, public-safe sentence instead, worded by the review's REAL verdict: "found no blocking issues" only when the model returned no blockers; "raised blocking findings ... read the private review record" when it did -- withheld blockers must never read as a clean result. A truly empty parse (no assessment at all) still returns null, so a genuine provider failure keeps its accurate report. One legacy test pinned the old null; updated with the incident reference.
Contributor
|
Important 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏳ LoopOver is waiting…LoopOver has seen this pull request and is waiting on CI checks to finish before reviewing it. This comment will update once the review runs. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed · 🟨 Waiting |
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9806 +/- ##
==========================================
- Coverage 90.33% 89.51% -0.82%
==========================================
Files 918 918
Lines 113935 113938 +3
Branches 26974 26976 +2
==========================================
- Hits 102925 101997 -928
- Misses 9681 10850 +1169
+ Partials 1329 1091 -238
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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 bug (the last of today's review-reliability incidents)
JSONbored/loopover#9794produced "AI review is unavailable for this PR head" on every run — including forced re-ticks, including after the beta.4 deploy — while its diagnostics saidclaude-code#0:parsed. The provider was fine every time.The real path: the model reviewed scoring/proof code, found no blockers and no nits, so its entire output was the narrative — and an honest narrative about this project's own gate/scoring code says "score"/"ranking"/"reward" in every sentence. The per-sentence sanitizer withheld all of them,
composeAdvisoryNotesreturnednull, and the orchestration's null-branch misattributed that to the provider, held the PR, and — the head being unchanged — reproduced identically on every re-run. A permanently wedged PR whose review succeeded every single time.The fix
When the parse produced a narrative but nothing survived sanitization, publish a fixed, public-safe sentence instead of
null— no model text, so the never-echo guarantee is untouched — worded by the review's real verdict:A truly empty parse (no assessment at all) still returns
null, so a genuine provider failure keeps its accurate "unavailable" report.One legacy test explicitly pinned the old
nullbehavior — the exact behavior this incident traces to — and is updated with the incident reference.Tests
286 pass in
test/unit/ai-review.test.ts, including the #9794 regression (all-withheld clean review → honest note,hasPublicReviewAssessmenttrue), the withheld-blockers invariant, and the true-provider-failure null case.tscclean.With this, every failure mode from today's investigation has a landed fix: cadence downgrade + false "skipped" bullet (#9800), restart-orphaned reviews (#9802), and this misattribution. Next image picks up all three.