Skip to content

v1.91.153.0 fix(outside-review): no-findings counts only as the review's conclusion; add regressions (#3294) - #3304

Merged
capy-ai[bot] merged 8 commits into
mainfrom
fix/outside-review-no-findings-conclusion
Oct 19, 2026
Merged

capy-ai[bot] merged 8 commits into
mainfrom
fix/outside-review-no-findings-conclusion

Conversation

@garrytan

@garrytan garrytan commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Fixes #3294. Thanks to @SchizasNik for the report and repro, and to @xg-gh-25 for pointing out the false-pass half.

What was wrong

classifyOutsideReview accepted a reply with no severity tags as clean whenever its no-findings regex matched anywhere in the text. Two consequences:

  1. False pass (the serious half). Untagged prose findings read clean as soon as any "no X" phrase appeared, e.g. "The parser change has no new issues. However, db/002_drop_idx.sql drops users_email_idx…" passed /ship's structured gate.
  2. False unverified. The vocabulary lacked regressions, so codex review --uncommitted's clean reply "I found no actionable regressions in the staged, unstaged, or untracked changes." was recorded as missing coverage.

What changed

  • A no-findings statement counts only as the review's conclusion (concludesNoFindings in lib/outside-review-result.ts). It must open the reply's last sentence or last ", and" clause, or the because clause of a review's Recommendation: line (wrapped continuation lines included), and carry no contrast or exception ("but", "however", "except", "other than", "elsewhere"…; "read-only" is not "only"). When other prose precedes it, anything after the statement except a predicate ("were found", "is evident") or a colon explanation must scope it to the change as a whole, so "No issues in the parser." or "No regressions were found by the existing tests." do not conclude the review.
  • Vocabulary: regressions / defects (singular too), and intro forms like "After reviewing the patch, I found no issues." NO_FINDINGS in inline code still counts.
  • Everything else stays unverified / untagged_review; the reason code and its docs/troubleshooting.md#outside-review-unverified anchor are unchanged (owned by lib/gate-outcomes.ts). The "No findings" bullet in docs/troubleshooting.md#outside-review-verdict describes the new rule.

Gate integrity

Gate edit: the structured and review gates' no-findings check in classifyOutsideReview (stricter about position, wider in vocabulary). No threshold, trial count or case kind changed; no existing test was edited.

New free regressions in test/outside-review-result.test.ts:

  • the reporter's exact reply through their exact CLI invocation → VERDICT: clean, exit 0;
  • two clean codex review --uncommitted replies captured live from codex-cli 0.160.0 / gpt-6-sol on a scratch repo (both unverified on main);
  • negative controls, 15 mixed replies that must read unverified under both the structured and review gates: the two strings from the issue thread, the phrase before / after / inside untagged findings, exception conclusions, conclusions scoped to one place or to the tests, a quoted NO_FINDINGS, a no-issues line inside a code fence;
  • Recommendation-clause cases, including a wrapped clause carrying "but".

All four new tests fail against main's classifier. Every existing negative control passes unchanged (outside-review-severity, outside-review-unparsed-severity, outside-voice-preflight), including the stored Codex clean review that still reads unverified ("no actionable correctness issues" — vocabulary deliberately not widened beyond the report).

An outside review by GPT-6.1 Sol probed for false passes; its realistic ones are now among the negative controls. Known residual, same as main: untagged findings followed by a conclusion that names the change ("No issues found in the changed parser.") still read clean, since existing negative controls pin that phrasing as clean and the parser cannot tell a finding from a summary without tags. A clean review ending in a non-conclusion footer ("Tests: not run") now reads unverified (fail-closed).

Validation

  • Full free suite on Ubicloud (bun run test:ubicloud) at 59945ee: pass. One unrelated file, test/gstack-upgrade-fresh-shell.test.ts, failed once under parallel load and passed on the runner's serial retry (logged as flaky).
  • The first CI run caught test/run-in-background-guidance.test.ts: its stub review (<id>: current findings / No issues found. / Recommendation: …) read unverified because the wrapped-line join glued the label line onto the conclusion. Now a line joins the one before only when it continues in lowercase, which fixes that test with no test edit. Two positive controls pin the rule. The remote pregate's diff selection had not picked that file.
  • Preflight (secrets, strays, literals, timing) pass; bun run typecheck and typecheck:test report no new diagnostics.
  • bun run eval:select: no PR-profile paid cases are selected. codex-review, codex-review-findings and the outside-voice behaviors are deferred to broad coverage.

garrytan and others added 3 commits October 19, 2026 18:36
…on; add regressions/defects (#3294)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
…e/otherwise are exceptions; real codex captures

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
…ed Recommendation clauses; outside-review findings as controls

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

trunk-io Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

😎 This pull request was merged.

garrytan and others added 3 commits October 19, 2026 19:30
…-findings-conclusion

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

# Conflicts:
#	CHANGELOG.md
…wercase

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

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

# Conflicts:
#	CHANGELOG.md
@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

E2E Evals: ✅ PASS (reused: main moved, R2)

No paid case ran on 7c8747719c9345fcce2e6e13c76fc771b71f02c3. It carries the GREEN verdict of run 39314907236 at 59945ee1ef722677b238a630955a3804bcc23a9f, because main moved (afac8df -> 9cce38b) and the PR's own diff from its merge-base, excluding stamp files, is byte-identical to 59945ee's (sha256 428296fdad06).

OWN DIFF: identical sha256=428296fdad0648a0a1e6601bbff86639b8f7e82e539fdf4ffb006fe7e1862415 (merge-base afac8dfd873a7c9c8d8b14dac8ebc30dfcc50e2a -> 9cce38bd46f9037029d7eda76f4c84777903e977)
base: afac8dfd873a7c9c8d8b14dac8ebc30dfcc50e2a -> 9cce38bd46f9037029d7eda76f4c84777903e977
selection: 9a5ee32ffff7529a9e72c08fae835a6ddc82e70653c83a1cc0dfe466a6e67e1d (identical to the reused gate)

Verdict reuse (R2a restamp-only, R2 main moved): a changed own diff, a non-stamp byte, a changed selection, a newer red or unbound gate, or a missing binding or receipt runs the full gate; the evals-fresh label opts out. Free Tests and every other check still ran on this head.

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
@github-actions github-actions Bot changed the title fix(outside-review): no-findings counts only as the review's conclusion; add regressions (#3294) v1.91.153.0 fix(outside-review): no-findings counts only as the review's conclusion; add regressions (#3294) Oct 19, 2026
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.

outside-review-result: "no actionable regressions" is read as an untagged (unverified) review

1 participant