feat(review): regenerate a public-safe summary when the narrative is withheld - #9956
Conversation
…withheld Closes #9809. When every sentence of a review's narrative trips the public-safety sanitizer -- routine for a PR touching this project's own scoring/gate code, where an honest sentence says 'score' or 'ranking' -- the reader got a fixed placeholder rather than a real summary. #9806 made that placeholder honest; this makes it a summary. On the withheld branch only, the narrative is rewritten for a public audience in one small completion and then held to the IDENTICAL sanitizer. The prompt is not the boundary: a rewrite that still names forbidden vocabulary is discarded and the fixed sentence stays the floor, so this can only improve the text a reader sees and can never widen what is publishable. A prompt-injected or careless rewrite fails closed, as does a provider error or the model's own CANNOT_SUMMARIZE opt-out. Scoped to the NO-BLOCKERS branch. The issue asks for a real summary every time and that a withheld blocker never read as clean; for a withheld blocker those conflict, and this resolves toward the safety property -- prose reading clean over a blocker the model actually raised is the one outcome that could green-light a PR. Blockers keep their fixed sentence, structurally, not by trusting the prompt. The provider call lives at the single caller rather than inside composeAdvisoryNotes, which stays pure and synchronous: making the composer async would push await through its call site and cost the testability that makes the sanitizer's behaviour verifiable. narrativeWasWithheld() lets the caller decide whether to spend the call before composing, computed the same way the composer decides, so the two cannot drift. The counter records both outcomes -- an all-fallback result must not look identical to the feature never firing.
The provider-path cases stub global fetch and never restored it, so the stub leaked into whatever file the worker ran next -- it took out selfhost-metrics.test.ts in a full run while passing in isolation.
|
Caution 🛑 LoopOver review result - fixes requiredReview updated: 2026-07-30 21:22:45 UTC
Review summary Nits — 6 non-blocking
CI checks failing
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Decision record
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Full suite re-run after the fetch-unstub commit: 25,772 passing, 0 failures (1,326 files). The Combined with the 36/36 checker sweep, this branch is green end to end. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9956 +/- ##
==========================================
- Coverage 91.88% 91.00% -0.88%
==========================================
Files 928 928
Lines 113675 113697 +22
Branches 27412 27421 +9
==========================================
- Hits 104445 103466 -979
- Misses 7931 9125 +1194
+ Partials 1299 1106 -193
Flags with carried forward coverage won't be shown. Click here to find out more.
|
1 participant
Closes #9809.
When every sentence of a review's narrative trips the public-safety sanitizer — routine for a PR touching this project's own scoring/gate code, where an honest sentence says "score" or "ranking" — the reader got a fixed placeholder rather than a real summary. #9806 made that placeholder honest; this makes it a summary.
On the withheld branch only, the narrative is rewritten for a public audience in one small completion, then held to the identical sanitizer.
The prompt is not the boundary
The rewrite is not trusted because it was asked nicely. It goes through the same
toPublicSafeBySentencegate, and anything that does not survive is discarded for the fixed sentence. So this can only improve the text a reader sees — it cannot widen what is publishable. A prompt-injected or careless rewrite fails closed, as does a provider error and the model's ownCANNOT_SUMMARIZEopt-out.Mutation-verified: replacing
toPublicSafeBySentence(trimmed, options)withtrimmedfails the suite.One conflict in the issue, resolved toward safety
The issue asks for a real summary every time, and that a withheld blocker never read as clean. For a withheld blocker those pull against each other — the rewrite would be describing findings it cannot name.
Blockers keep their fixed sentence, structurally rather than by trusting the prompt. Prose reading clean over a blocker the model actually raised is the one outcome that could green-light a PR. Also mutation-verified: letting the regenerated text through on that branch fails the suite.
If you'd rather have B (regenerate for blockers too, with the prompt required to state that findings exist) or C (regenerate and append the fixed blocker sentence), both are small follow-ups — but only this version makes the never-green-light property independent of prompt compliance.
Where the IO lives
The provider call is at the single caller, not inside
composeAdvisoryNotes. The issue's design put it in the composer, but that function is pure and synchronous; making it async would pushawaitthrough its call site and cost the testability that makes the sanitizer's behaviour verifiable at all.narrativeWasWithheld()lets the caller decide whether to spend the call before composing, and computes it the same way the composer does — reproducing the sanitizer's decision by other means would let the two drift, and a mis-fire would spend a completion on every review rather than the ~2-3/day this targets.Telemetry
loopover_ai_review_summary_regenerated_total{outcome=published|fallback}, registered inDEFAULT_METRIC_METAlike its siblings so it renders with HELP/TYPE. Both outcomes are counted — an all-fallback result must not look identical to the feature never firing.Validation
check-*scripts: none failing.selfhost:validate-observabilitygreen.fetchand never restored it, which leaked intoselfhost-metrics.test.tsin a full run while passing in isolation. Second commit unstubs; re-run in flight and I'll post the result.