fix(outside-review): the exact NO_FINDINGS verdict is clean; document consult verdicts as unverified (#3925)

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

docs/troubleshooting.md

@@ -401,7 +401,13 @@ bun lib/outside-review-result.ts --verdict execution reply.md
 ...)`, or it approves but also says something was excluded, blocked, skipped,
 not run or could not be verified. The
 reviewer did not say the review finished and found the change ready, so gstack
-does not count it as `clean`. The quoted verdict is in parentheses.
+does not count it as `clean`. The quoted verdict is in parentheses. Besides
+the approvals, only the exact `NO_FINDINGS` token counts as a clean verdict.
+
+A `/codex` consult or challenge answer whose verdict picks an option
+(`Verdict: go with B`, `Verdict: Proceed with the plan`) is `unverified` on
+purpose: a consult is advice, not a pass/fail review, so there is no pass to
+report. Read the answer itself; an answer with no verdict line stays `clean`.
 
 **What is kept.** The full reply is shown above.
 

lib/outside-review-result.ts

@@ -268,7 +268,7 @@ function lastVerdict(plain: string): string | undefined {
   }
   return undefined;
 }
-/** A verdict that approves the change. */
+/** A verdict that approves the change; the exact NO_FINDINGS token (its underscore is gone after plainReview) is clean too. */
 const READY_VERDICT = /^(?:ready|lgtm|approved?|ship(?:[\t ]+it)?|pass(?:ed)?|clean|good to (?:go|merge|ship))\b(?![-?/])/i;
 /** A verdict that says the reviewer could not check, run or see something is never a ready one (#3925). */
 const UNVERIFIED_CLAIM = /\b(?:can['\u2019]?t|cannot|can not|couldn['\u2019]t|could not|unable to|not able to|did not|didn['\u2019]t)\s+(?:be\s+)?(?:verif|run|test|check|open|read|review|execut)|\bunverifi|\bnot (?:yet|until)\b|\bnot (?:been )?(?:verified|run|tested|checked|reviewed|executed)\b|\b(?:excluded|blocked|skipped)\b/i;
@@ -309,7 +309,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 && (!READY_VERDICT.test(v) || UNVERIFIED_CLAIM.test(v)));
+  const unready = verdicts.find(v => v !== undefined && !/^NO_?FINDINGS$/.test(v) && (!READY_VERDICT.test(v) || 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

@@ -228,6 +228,7 @@ describe('#3925 an unrecognized verdict is not clean; numbered findings never pr
         "Verdict \u2014 can't verify", 'Verdict: ready, though one changed file was excluded', 'Verdict: ready (tests not run, sandbox blocked)',
         '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.',
       ])
         expect({ gate, verdictLine, ...see(`No issues found.\n${verdictLine}`, gate) }).toEqual({ gate, verdictLine, verdict: 'unverified', reason: 'unverified_verdict', unlabeled: 0 });
   });
@@ -242,7 +243,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`'])
+      for (const verdictLine of ['Verdict: Ready to merge.', 'Verdict: LGTM', 'Verdict: Approve', 'Verdict: Ship it', '**Verdict:** `ready`', 'Verdict: NO_FINDINGS', 'Verdict: `NO_FINDINGS`'])
         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 });
     }