fix(outside-review): read verdicts from a view that unwraps inline code, reject a verdict with a question mark (#3925 fix-check)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

lib/outside-review-result.ts

@@ -200,11 +200,11 @@ function plainReview(text: string): string {
  * blocking signals use that view, so an unclosed fence cannot hide a later `[P1]`
  * or `Verdict: not ready`, while the evidence for clean keeps it blanked. One pass:
  * an opener closes only when a later marker of its kind is at least as long.
+ * `keepSpans` unwraps every inline span: the verdict is read from that view, so
+ * `Verdict: \`ready\` (tests \`not run\`)` is read whole.
  */
 const CODE_LABEL_BEFORE = /^[\t ]*(?:>[\t ]*)?(?:#{1,6}[\t ]+|[-+*][\t ]+|\(?\d{1,3}[.)][\t ]+)?(?:\*\*|__)?$|\b(?:severity|priority)[\t ]*[:=][\t ]*(?:\*\*|__)?$|\|[\t ]*(?:\*\*)?$/i;
-/** A verdict line keeps every inline span: `Verdict: \`ready\` (tests \`not run\`)` is read whole. */
-const VERDICT_LINE = /^[\t ]*(?:>[\t ]*)*(?:#{1,6}[\t ]+|[-+*][\t ]+)?(?:\*\*|__)?(?:(?:overall|final)[\t ]+)?verdict\b/i;
-function withoutCode(text: string, readUnclosed = false): string {
+function withoutCode(text: string, readUnclosed = false, keepSpans = false): string {
   const lines = text.split(/\r?\n/);
   const markers = lines.map(line => /^[\t ]*(`{3,}|~{3,})/.exec(line)?.[1]);
   const closes = markers.map(() => true);
@@ -226,8 +226,7 @@ function withoutCode(text: string, readUnclosed = false): string {
     }
     if (marker) { if (closes[i]) fence = marker; return ''; }
     return line.replace(/`([^`]+)`/g, (span, inner: string, at: number) =>
-      inner.trim() === 'NO_FINDINGS' || /^\[?(?:P[0-3]|critical|high|medium|low)\]?$/i.test(inner.trim()) && CODE_LABEL_BEFORE.test(line.slice(0, at))
-        || VERDICT_LINE.test(line) ? inner : ' ');
+      keepSpans || inner.trim() === 'NO_FINDINGS' || /^\[?(?:P[0-3]|critical|high|medium|low)\]?$/i.test(inner.trim()) && CODE_LABEL_BEFORE.test(line.slice(0, at)) ? inner : ' ');
   }).join('\n');
 }
 
@@ -283,7 +282,7 @@ export function classifyOutsideReview(input: OutsideReviewInput): OutsideReviewC
   const levelsOf = (c: string, p: string) => [...[...p.matchAll(/\[(P[0-3])\]|^(P[0-3]):/gm)].map(m => (m[1] ?? m[2]) as Severity),
     ...severityTokens(c), ...(reviewed ? severityWords(c) : [])];
   const levels = levelsOf(read, plainRead);
-  const verdicts = [lastVerdict(plainCode), lastVerdict(plainRead)];
+  const verdicts = [lastVerdict(plainReview(withoutCode(input.text, false, true))), lastVerdict(plainReview(withoutCode(input.text, true, true)))];
   const unlabeled = reviewed && verdicts[1] !== undefined && !concludesNoFindings(code)
     ? [...read.matchAll(NUMBERED_ITEM)].filter(m => !levelsOf(m[0], plainReview(m[0])).length).length : 0;
   const findings = { highest: levels.length ? levels.sort()[0]! : null, ...(unlabeled ? { unlabeled } : {}) };
@@ -309,7 +308,7 @@ export function classifyOutsideReview(input: OutsideReviewInput): OutsideReviewC
   const notReady = verdicts.some(v => v !== undefined && /^not[\t ]+ready\b/i.test(v));
   if (blocking || notReady) return { ...result('findings'), ...(unparsed ? { unparsed } : {}) };
   if (unparsed) return { ...result('unverified', 'unparsed_severity', unparsed), unparsed };
-  const unready = verdicts.find(v => v !== undefined && !/^NO_?FINDINGS$/.test(v) && (!READY_VERDICT.test(v) || UNVERIFIED_CLAIM.test(v)));
+  const unready = verdicts.find(v => v !== undefined && !/^NO_?FINDINGS$/.test(v) && (!READY_VERDICT.test(v) || v.includes('?') || UNVERIFIED_CLAIM.test(v)));
   if (unready !== undefined) return result('unverified', 'unverified_verdict', `Verdict: ${unready}`.trim().slice(0, 160));
   if ((input.gate === 'structured' || input.gate === 'review') && !cleanEvidence && !concludesNoFindings(code)) {
     return result('unverified', 'untagged_review', 'missing severity or explicit no-findings conclusion');

test/outside-review-unparsed-severity.test.ts

@@ -229,6 +229,7 @@ describe('#3925 an unrecognized verdict is not clean; numbered findings never pr
         'Verdict: `ready` (tests `not run`)', 'Verdict: Ready? Not until the migration is fixed.', 'Verdict: clean-up required before merge',
         'Verdict: ready, but I did not execute the suites', 'Verdict: ready (tests not executed)',
         'Verdict: no_findings', 'Verdict: NO_FINDINGS except the migration', 'Verdict: go with B.', 'Verdict: Proceed with the plan.',
+        '## Verdict\nready (tests `not run`)', '**Verdict**\nready (suites `skipped`)', 'Verdict: Ship it? No.', 'Verdict: Ready ? No.',
       ])
         expect({ gate, verdictLine, ...see(`No issues found.\n${verdictLine}`, gate) }).toEqual({ gate, verdictLine, verdict: 'unverified', reason: 'unverified_verdict', unlabeled: 0 });
   });
@@ -243,7 +244,7 @@ describe('#3925 an unrecognized verdict is not clean; numbered findings never pr
   test('negative controls: real clean replies, consult answers, tagged findings and quoted examples stay as they were', () => {
     for (const gate of ['execution', 'structured', 'review'] as const) {
       expect({ gate, ...see(read('no-findings.txt'), gate) }).toEqual({ gate, verdict: 'clean', reason: null, unlabeled: 0 });
-      for (const verdictLine of ['Verdict: Ready to merge.', 'Verdict: LGTM', 'Verdict: Approve', 'Verdict: Ship it', '**Verdict:** `ready`', 'Verdict: NO_FINDINGS', 'Verdict: `NO_FINDINGS`'])
+      for (const verdictLine of ['Verdict: Ready to merge.', 'Verdict: LGTM', 'Verdict: Approve', 'Verdict: Ship it', '**Verdict:** `ready`', 'Verdict: NO_FINDINGS', 'Verdict: `NO_FINDINGS`', 'Verdict: ready. The `[P1]` from round 1 is fixed.'])
         expect({ gate, verdictLine, ...see(`No issues found.\n${verdictLine}`, gate) }).toEqual({ gate, verdictLine, verdict: 'clean', reason: null, unlabeled: 0 });
       expect({ gate, ...see('I checked:\n1. the retry path\n2. every caller\nNo issues found.\nVerdict: ready', gate) }).toEqual({ gate, verdict: 'clean', reason: null, unlabeled: 0 });
     }