[varshavasthavi]

KAI_REVIEW_LATENT_DEFECTS — recover latent wrong-as-written defects without losing precision (default off)

On the Martian offline benchmark (50 PRs, judges opus-4.5 / sonnet-4.5 / gpt-5.2; canonical metric opus · core · F2) this flag takes Kai's GLM reviewer from "safe but misses bugs" to "catches more bugs at the same precision." Everything below is measured from run artifacts, and every claim has an example.

---

1. What we tried, and what the artifacts showed

The machinery this sits inside — the multi-sweep ensemble (-ens). Every run here uses the ensemble pipeline, not a single pass:

• Two finders — the main pass (z-ai/glm-5.2) plus finder2, a second pass on a different model (run42/44: google/gemini-3.5-flash low) — so two independent models surface candidates.
• Two sweeps — sweep (deepseek/deepseek-v4-pro) and sweep2 (glm-5.2) — re-reading for anything missed.
• A rank stage (deepseek/deepseek-v4-pro) that reads all the gate-confirmed defects together and keeps the strongest (rcRankSupported).

The design intent: diversity for recall, rank for precision. What the artifacts showed is the rank stage is doing real work but imperfectly — in run42 110 gate-supported findings were dropped before publishing, and only ~10% of that dropped tail were real bugs, i.e. the ranker can't cleanly tell a gate-approved true finding from a gate-approved false one. That's a recurring theme below.

Prior-review context (rcPriorBlock). We also carry a re-review path: a re-review starts from what Kai said last time and checks it against the new commit (with a force-push/shallow-clone fallback to the base...ref file set). It reduces re-review churn — but it is benchmark-neutral: Martian reviews each PR as one squashed diff with no prior review, so this path is never exercised in these numbers. Listed for completeness, not as a driver of the results.

Baseline: safe but low-recall (run34, GLM main, no flag). Precise but timid — it skipped anything whose trigger wasn't staring it in the face.

• Measured (opus, 2 repeats): P ≈ 52.9 / R ≈ 43.0 / F1 ≈ 47.4 (F1 46.9 and 47.9). High precision, low recall — ~98 real bugs left on the table.

Attempt A — stronger/verbose main model (run38). Hoped a beefier main pass would catch more. Instead it fragmented into 1.41× more candidates and precision fell to ~43 — more output, not better output. Lesson: throwing a bigger model at the main pass without better ranking just multiplies noise. Rejected.

Attempt B — the latent flag, first cut (run42). Tell the finder to raise latent defects and let the gate support them. It worked on recall but broke precision:

• Measured (opus): recall 43 → 53, but precision 52.9 → 46.2 and false positives 65 → 107.
• Reading run42's 107 FP, they split in two:
  • ~70% confident, line-cited-but-wrong finder claims that look exactly like real bugs — the gate can't tell them apart. Examples: "removeStoredCredentialById only removes local DB credentials and fails to remove federated storage"; "in santizeAnchors, when an anchor tag is found but the English matcher has no more…" — specific, cited, and wrong about the actual code.
  • ~30% a soft tail the gate can target: a stale docstring typo "humam readible"; a stale # TODO: … when we build that abstraction on a class that already does; a comment claiming traceItemType "only supports LOGS" when the enum now adds SPANS.

Conclusion: the latent idea recovers the right bugs, but needs a gate rule that trims the soft tail without touching the confident real findings.

---

2. What a "latent defect" is, and the ones we were missing

A latent defect is code that is wrong as written but whose triggering input/caller/state doesn't exist in the tree yet — it hasn't fired, but will the moment a legitimate path reaches it. Different from a footgun (code correct today that only breaks if someone later rewrites it).

Production refused both as "hypothetical." The default gate clause refutes "dormant today" / "if X is ever added" — wrongly lumping a real dormant bug with a harmless footgun. By example:

• Latent (support it): "only the SMS handler writes retryCount today, but the delete removes the e-mail and WhatsApp rows the moment any other handler increments it." Already wrong; nothing needs rewriting.
• Footgun (keep refuting): "if these guards are ever reordered…" — every input is handled correctly today.

Real latent bugs the benchmark rewards that the baseline was missing (from run44's gained set vs run34):

• Optional.get() called directly on the Optional from RecoveryAuthnCodesUtils.getCredential — dormant NPE when empty.
• credential can be undefined when passed to updateEvent if reference.credentialId is set but the credential isn't found — dormant null deref.
• Salesforce OAuth refresh checks response.statusText instead of response.ok — wrong as written.
• A loop on tempfile.size may never terminate because the size is cached stale — latent infinite loop.

---

3. What we did to fix it

A single flag, KAI_REVIEW_LATENT_DEFECTS (default off — production prompt byte-identical when unset):

• Finder raises latent wrong-as-written defects instead of self-censoring them.
• Gate supports them at a higher bar, via an explicit three-rule drop-list — refute when: (1) cosmetic/style, (2) comment/doc/test-only, or (3) unverified consequence at low/medium severity.
  • Recall guards so the drop-list doesn't eat real bugs: a high/critical guardrail (a severe latent defect survives an unverified consequence), and a user-facing-text carve-out on rule 2 — a "Lithuanian translation replaced with Italian" defect is production behaviour and is not dropped.
• Still refutes pure footguns at the gate and in a narrowed speculative pre-filter.
• Rank cap +2 under the flag so recovered defects can publish.
• Each prompt change is a strings.Replace of a verbatim substring of the shipped prompt; TestReachableClauseIsVerbatim fails if an edit breaks the substring.

Drop-list in action: refutes the "humam readible" typo and the stale TODO (rule 2), while the carve-out keeps the wrong-translation defect.

---

4. Results (full 50-PR benchmark, all three judges)

Canonical opus · core · F2 (run42 → run44):

metric	run42	run44	Δ
Precision	43.7	51.6	+7.9
Recall	52.5	52.5	0.0
F1	47.7	52.0	+4.3
F2	50.5	52.3	+1.8

The drop-list removed 29 false positives and zero core true positives.

All three judges agree (core): F1 +3.0 / +4.3 / +7.2 (sonnet/opus/gpt), F2 +0.7 / +1.8 / +5.1, FP down 22–28%.

Versus the safe baseline (run34): run44 keeps run34's precision (~53) and adds +9 recall → F1 +5.4. At the golden level it catches +12 real bugs (+10 core), gains bug/security-weighted (the Optional.get NPE, the Salesforce OAuth check, a billing-slug logic inversion). run34's two repeats varied only ~1 F1 point, so +5.4 is ~5× the noise floor.

---

5. Shortcomings (straight from the results)

• The real F1 ceiling is finder false-detection, not gate filtering. ~70% of FP are confident, line-cited-but-wrong claims the gate/drop-list cannot separate from real bugs — e.g. "Recovery codes in user storage are never consumed after login…" reads like a genuine data bug but is wrong about the code. No prompt rule filters these; the finder has to stop raising them.
• Uneven by repo — sentry regressed (−2.4 core F1). Its golden set is test-heavy and the drop-list trims test findings; it dropped a legit "monkeypatched time.sleep never actually waits" test_gap finding. discourse, by contrast, gained +11.5 F1 (all-cat).
• Golden churn, not a clean superset. Vs run34 it lost 13 core bugs while gaining 25 (net +10) — e.g. a couple of real concurrency/api findings run34 caught dropped out.
• The rank stage is a blunt instrument. 110 gate-supported findings were dropped pre-publish in run42 with only ~10% real — it can't promote the true ones out of the tail.
• Single run + engine confound. repeat=1 (cross-judge consistency is the only significance signal so far), and run44 is on kai-engine v0.6.85 vs run42's v0.6.84, so a sliver of the FP drop may not be the gate change.

---

6. Future improvements (with benchmark examples)

• Source-grounded verification pass — before publishing, fetch the cited code and verify the claim against it. This is the only lever that touches the 70% confident-wrong bucket: e.g. a grounding step that read the actual removeStoredCredentialById would see the "fails to remove federated storage" claim is false and drop it, without a prompt heuristic. An offline replay harness for this is already built.
• Grounded rank signal — feed that verification confidence into the rank stage so it promotes the ~10% real findings currently buried in the 110-item dropped tail, instead of ranking blind.
• Test-defect carve-out for the drop-list — keep test findings that assert wrong behaviour / wrong test data (e.g. the "monkeypatched sleep" and "wrong value 6 in test data" losses) while still dropping pure naming nitpicks — recovers most of the sentry regression without reopening the FP floodgates.
• Multi-sweep tuning, done right — a stronger or third finder could lift recall further, but run38 showed naive "bigger main" just fragments; diversity has to be paired with the grounded ranker above, not added raw.
• A repeat run — to convert the cross-judge-consistent single run into a significance-cleared result.

---

Testing

All behavior is flag-gated and locked by tests in review_commit_latent_defects_test.go:

• TestReachableClauseIsVerbatim — each default clause is a verbatim substring of its shipped prompt, and rcFutureRewritePhrases ⊂ rcSpeculativePhrases, so an edit can't make a swap silently no-op.
• TestLatentDefectsFlagOffUnchanged / TestDeepFinderSwapsUnderFlag / TestGateClauseSwapsUnderFlag — flag off is byte-identical to production; flag on swaps finder and gate clauses.
• TestSpeculativeDropNarrowsUnderFlag — dormant (wrong-as-written) flows to the gate; pure footgun stays dropped pre-gate.
• TestRankCapRaisedUnderFlag — cap is +2 only under the flag.
• TestLatentClauseRaisesTheBar — asserts the drop-list, the high/critical guardrail ("even when you could not fully verify", "unverified consequence is a refutation"), and the user-facing-text carve-out ("text shown to a user", "IS production behaviour").

🤖 Generated with Claude Code

---

Kai context

_{Written by Kai from the change itself. Kai only edits this section; everything above it is the author's.}

Adds the KAI_REVIEW_LATENT_DEFECTS flag (default off) so the reviewer raises wrong-as-written defects whose trigger doesn't exist in the tree yet, relaxing the "trigger must exist today" contract across the deep finder, pre-gate speculative filter, and challenge gate while keeping production prompts byte-identical when unset.

Changes

• review_commit.go: adds rcReviewTriggerClauseDefault/rcReviewTriggerClauseLatent and rcReviewScoreClauseDefault/rcReviewScoreClauseLatent clause pairs plus rcReviewSystemText(), which verbatim-substring-swaps both clauses into the deep finder prompt only when the flag is on; rcRunReviewAgent and rcConcludeWithModel now call rcReviewSystemText() instead of rcReviewSystem.
• review_commit_challenge.go: adds rcReachableClauseDefault/rcReachableClauseLatent and rcChallengeSystemHeadText(); the latent clause supports wrong-as-written defects and applies a three-rule drop-list (cosmetic/style, comment/doc/test-only with a user-facing-text carve-out, unverified-consequence at low/medium severity) with a high/critical guardrail; rcChallengeSystemPromptWith uses the accessor; rcValidateChallenge logs supported/refuted/unresolved counts under the flag.
• review_commit_speculation.go: adds rcFutureRewritePhrases (strict subset of rcSpeculativePhrases), rcActiveSpeculativePhrases() returning the full list by default and the future-rewrite subset under the flag, and rcLatentDefectsEnabled() reading KAI_REVIEW_LATENT_DEFECTS (1/true/on/yes); rcSpeculativePhrase iterates the active list.
• review_commit_ensemble.go: rcRankCap raises the publish cap by 2 under the flag so latent defects (ordered last) can publish; default cap stays 3–6.
• review_commit_fast.go: documents that the fast pass is deliberately not relaxed by the flag.
• review_commit_latent_defects_test.go (new): TestReachableClauseIsVerbatim, TestDeepFinderSwapsUnderFlag, TestLatentDefectsFlagOffUnchanged, TestGateClauseSwapsUnderFlag, TestSpeculativeDropNarrowsUnderFlag, TestRankCapRaisedUnderFlag, and TestLatentClauseRaisesTheBar.
• review_commit_ensemble_test.go and review_commit_speculation_test.go: pin KAI_REVIEW_LATENT_DEFECTS off in the flag-off-behaviour tests (TestRankCapGrowsWithTheChange, TestSpeculativePhraseCatchesTheBenchmarkCases, and the speculative-draft tests) so they stay deterministic when the flag is exported.

Testing

• Verbatim-substring invariants for all default clauses against their shipped prompts, plus rcFutureRewritePhrases ⊆ rcSpeculativePhrases, are asserted in TestReachableClauseIsVerbatim.
• Flag-off byte-identity and flag-on clause swaps covered by TestLatentDefectsFlagOffUnchanged, TestGateClauseSwapsUnderFlag, and TestDeepFinderSwapsUnderFlag.
• Pre-gate filter narrowing (dormant case flows through, footgun still dropped) and end-to-end rcWithoutSpeculativeIssues behaviour asserted in TestSpeculativeDropNarrowsUnderFlag.
• Rank cap +2 under the flag asserted in TestRankCapRaisedUnderFlag; latent gate drop-list, severity guardrail, and user-facing-text carve-out strings pinned in TestLatentClauseRaisesTheBar.

[kaicontext]

Kai review · Ready to merge (10/10)

Read the whole thing — it does what it says, and nothing jumped out at me.

Next: merge when your checks are green — I found nothing to fix.

Since the last review: 1 fixed

• an earlier point — fixed at cmd/kai/review_commit_ensemble_test.go:119: t.Setenv("KAI_REVIEW_LATENT_DEFECTS", "") now pins the flag off for the base-cap test, matching the guard TestRankCapRaisedUnderFlag uses.

Important files changed

File	Change
cmd/kai/review_commit.go	modified · +23 −2
cmd/kai/review_commit_challenge.go	modified · +29 −1
cmd/kai/review_commit_ensemble.go	modified · +19 −5
cmd/kai/review_commit_ensemble_test.go	modified · +5 −0
cmd/kai/review_commit_fast.go	modified · +5 −0
cmd/kai/review_commit_latent_defects_test.go	modified · +163 −0
cmd/kai/review_commit_speculation.go	modified · +50 −1
cmd/kai/review_commit_speculation_test.go	modified · +9 −0

How I reviewed it — 8 files, 18 turns, 1m52s

The determinism defect from the prior review is fixed; all six default-asserting tests now pin KAI_REVIEW_LATENT_DEFECTS="", the verbatim swap invariants hold, all production callers were switched to the flag-aware accessors, and the fast pass is correctly left unrelaxed.

What I opened

• cmd/kai/review_commit.go
• cmd/kai/review_commit_challenge.go
• cmd/kai/review_commit_ensemble.go
• cmd/kai/review_commit_ensemble_test.go
• cmd/kai/review_commit_fast.go
• cmd/kai/review_commit_latent_defects_test.go
• cmd/kai/review_commit_speculation.go
• cmd/kai/review_commit_speculation_test.go

Full read-through

Review: kaicontext/kai-cli @ 2b18ff4 — re-review of KAI_REVIEW_LATENT_DEFECTS flag-pinning follow-up

I re-read all files in this change, including the three I opened in the prior pass (cmd/kai/review_commit_challenge.go, cmd/kai/review_commit_ensemble.go, cmd/kai/review_commit_fast.go). I could not read any external repo or deployed config.

What the change does: Pins KAI_REVIEW_LATENT_DEFECTS="" in the six review_commit_speculation_test.go tests and TestRankCapGrowsWithTheChange in review_commit_ensemble_test.go that assert default (flag-off) behavior, so the suite is deterministic under a flag-on A/B benchmark recipe that exports the flag. The flag-on behavior (narrowing + rank-cap bump) is asserted in dedicated tests that don't rely on the ambient env. The default path returns the shipped prompts byte-for-byte unchanged.

Earlier finding: - d9920a0a548b (TestRankCapGrowsWithTheChange missing the flag pin) is fixed at cmd/kai/review_commit_ensemble_test.go:119 — the t.Setenv("KAI_REVIEW_LATENT_DEFECTS", "") guard is now present, matching the pattern TestRankCapRaisedUnderFlag uses. The other five tests in review_commit_speculation_test.go received the same pin.

Sweep of the change:

• cmd/kai/review_commit_fast.go:55-67 — the comment block correctly states the fast pass is "deliberately NOT relaxed by KAI_REVIEW_LATENT_DEFECTS," and rcFastReviewSystem is a separate const with no flag-conditional accessor. Consistent with the intent: latent defects belong to the grounded finder and the gate, not the fast pass.

• cmd/kai/review_commit_challenge.go:82-101 — rcReachableClauseDefault is a verbatim substring of rcChallengeSystemHead (line 34 contains the sentence), and rcChallengeSystemHeadText() (line 96-101) swaps it for rcReachableClauseLatent only when the flag is on. The production caller rcChallengeSystemPromptWith (line 119) uses rcChallengeSystemHeadText(). No other production caller of the bare head was missed.

• cmd/kai/review_commit_challenge.go:1248-1253 — the rcValidateChallenge stderr diagnostic under the flag surfaces the supported/refuted/unresolved counts so an A/B run can catch budget starvation (edge case R3). Gated on rcLatentDefectsEnabled(), correct.

• cmd/kai/review_commit_ensemble.go:211-239 — rcRankCap raises the cap by 2 under the flag (line 235-237), consumed at line 295 by rcRankSupported. TestRankCapRaisedUnderFlag (latent_defects_test.go:128-138) asserts on == off+2 across {1,3,8,20,40}; TestRankCapGrowsWithTheChange (ensemble_test.go:114-128) pins the flag off and asserts the base cap 3..6 is unchanged. Both consistent.

Verbatim invariants confirmed: rcReviewTriggerClauseDefault and rcReviewScoreClauseDefault are exact substrings of rcReviewSystem (review_commit.go:62, :116). rcReachableClauseDefault is an exact substring of rcChallengeSystemHead (challenge.go:34). Both production callers of rcReviewSystem (review_commit.go:986, :1812) use rcReviewSystemText(). The one production caller of rcChallengeSystemHead (challenge.go:119) uses rcChallengeSystemHeadText(). rcFutureRewritePhrases is a strict subset of rcSpeculativePhrases (latent_defects_test.go:29-39 guards it).

No new defects found. The change is a clean test-determinism fix. Every test asserting default behavior now pins the flag off; flag-on behavior is covered by dedicated tests. The score moves from 3 to 5 — the earlier defect is fixed and the three files introduce no new concerns.

_{Size M · Risk M · +303 −9 · 8 files · reaches 57 · the full analysis · Reply to any of my comments and I'll answer, or say @kaicontext anywhere on this PR — a question, or "take another look at the retry logic".
Kept up to date: this is update 9, for 2b18ff4. Earlier points that still apply stay on their lines.}

[kaicontext]

Kai · 1 new point on ab4cf3c. The review is updated.

• TestRankCapGrowsWithTheChange asserts rcRankCap(n) > 6 is an error and does not set the flag, so running the suite with KAI_REVIEW_LATENT_DEFECTS=1 (the PR's own A/B recipe) fails it at n=200 (returns 8); the test needs the same flag-off guard TestRankCapRaisedUnderFlag already uses