Stop dropping the reviewer's intent verdict when its formatting drifts

[AdamT]

The bug

rcParseReviewOutput accepted the reviewer's closing verdict in exactly one shape — an unadorned key, and a value that is literally verified, partial, or diverges:

switch strings.ToLower(strings.TrimSpace(t[len("INTENT_MATCH:"):])) {
case "verified":  match = finding.MatchVerified
case "partial":   match = finding.MatchPartial
case "diverges":  match = finding.MatchDiverges
}   // no default, no log

Everything else fell through to MatchUnknown, which the console renders as "no contract resolved for this change" — asserting the reviewer never judged the change, when it had.

Evidence

kaicontext/kai-server#70, identical commit content, two runs:

Run	intent_match	note
1st	unknown	(none)
2nd	verified	"Clean security fix — constant-time compare, no open fallback, thorough test coverage…"

The job log for the failing run shows a complete review: 25 graph-grounded tool calls (kai_callers on verifyIngestSecret, kai_grep for IngestFinding), exit 0, Step completed: success. The review worked. The parse didn't.

It also isn't a size problem — in the same batch, a 951-line PR verified while this 237-line one did not.

Shapes that silently became unknown

Reviewer emits	Before	After
INTENT_MATCH: verified	✅	✅
**INTENT_MATCH:** verified	❌	✅
`INTENT_MATCH:` verified	❌	✅
INTENT_MATCH: verified.	❌	✅
INTENT_MATCH: verified — no material gaps	❌	✅
INTENT_MATCH: matches	❌	✅
INTENT MATCH: verified (key drift)	❌	❌ — deliberately still unknown

That sixth row matters: the prompt itself glosses verified as "(matches)", so the model echoing that word back is expected behaviour. kai-engine's intent parser (reviewanalyze.go:334) already accepts matches/match. Two parsers with two vocabularies is the root cause.

What changed

1. Decoration-tolerant — strips * and backticks before matching. Deliberately not _: that would break the INTENT_MATCH key itself and mangle snake_case identifiers in the note (there's a test).
2. Synonyms resolve — aligned with what kai-engine already accepts.
3. First-word matching — a trailing justification no longer hides the verdict.
4. Conflicting verdicts don't resolve — two INTENT_MATCH: lines that disagree record unknown rather than last-one-wins, so a model quoting the block back as an example can't decide the verdict by emission order.
5. Unreadable verdicts are logged — review-commit prints the reviewer's raw closing text to stderr whenever it records unknown, so the failure stops being silent.

Also changed: which findings get recorded

Three rules with a wider blast radius than the verdict fix, found while fixing it:

• A second FINDINGS: header restarts the list. A reviewer that quotes an example block while explaining itself would otherwise have those bullets stored as defects it never reported. Observed in a real run: 3 findings recorded, 2 of them quoted from this command's own test fixtures.
• The list ends at the first non-bullet line. A closing block the model spelled differently used to leave the parser inside FINDINGS, storing its trailing prose as a defect.
• Bullets are matched after decoration is stripped, so **- [security] …** is a finding. It was dropped silently before.

On not inventing verdicts

An earlier draft had a salvage pass that recovered a verdict from a drifted key (INTENT MATCH: with a space). It carried a real false-positive bug, caught by the tests here: the key INTENT_MATCH contains the word "match", so scanning the whole line resolved any unreadable INTENT_MATCH: line to verified.

Salvage was dropped rather than narrowed. A drifted key now stays unknown and gets logged, which is the honest outcome — the project's own rule is that "a confidently wrong verified poisons the whole proposition". Nothing outside a well-formed INTENT_MATCH: line can produce a verdict, and TestRCParseReviewOutput_ProseNeverBecomesAVerdict pins that, drifted key included.

Why the logging is the important half

The parser fix reduces how often this happens. The logging tells you when it still does — previously the job exited 0, the log looked clean, and an amber badge was the only symptom. There was no way to measure the failure rate, which means no way to know whether the cheap-model thesis is holding.

Testing

40 cases across 13 tests: every shape above, unreadable/absent/conflicting verdicts staying unknown, prose never inventing a verdict, the findings-block rules, and inline code and underscores preserved in notes.

go build ./... clean; go test ./cmd/kai/ passes in full (no regressions).

Deploying

The production review pods run this code via the kai-ci image (kai-cli/deploy/kai-ci/Dockerfile → go build -o /out/kai ./cmd/kai), which kai-server's review workflow pins as kai-ci:latest. This needs a kai-ci image rebuild + push to take effect — it does not ride kai-server's deploy. pullPolicyFor already forces PullAlways on :latest, so pods pick it up once pushed.

Follow-up worth considering

This whole class of bug exists because the verdict travels as free text. Having the agent emit it as a structured tool call would remove the parser entirely — larger change, filed as a thought rather than done here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FjRNnVT1rpLU8ewxjTzxXQ

[kaicontext]

Kai review

Warning

Couldn't finish reviewing this change. Push a new commit or re-run the review to try again.