fix(sage): carry a run's failure cause as a token, not only as prose

[chenmingwei23]

Diff shape, up front: 10 files -- 8 source/test, and 2 regenerated screenshots
under temp-screenshots/sage-cause-translator/ (the two frames this PR's own
harness produces; 0-before-* is left untouched, being the historical pre-fix
frame from #7686). No other generated assets.

Problem / Motivation

A Sage run's failure cause exists as a stable token in the backend --
skipped_reason is one of no_review_recorded, review_record_incomplete,
runtime_unavailable, review_failed -- but nothing in the payload the
dashboard receives named it.

• routes.py::_first_change_error maps the token to an English sentence before
  the run is serialized, so run["error"] is prose only.
• Each per-change progress entry carries prose and no token, so the entry the
  dashboard reads per change cannot name its own cause.

Correction, from First Principles review on this PR: an earlier version of
this description said the token "sat on the driver-internal per_change record
... which the dashboard never sees". That was false and is worth stating plainly
rather than quietly editing. run["summary"] = summary (routes.py:491) includes
per_change, and _handle_runs (routes.py:914) returns run dicts whole, so
summary.per_change[].skipped_reason IS already in the /runs payload for
completed runs. The frontend even declares the field (per_change?: unknown[],
types.ts:72) with zero readers. So a frontend-only fix consuming that existing
field was available for completed runs, and the reviewer was right that it was
never weighed.

It is still not the shape this PR takes, for three reasons that hold
independently of the correction:

1. Interrupted runs have no summary at all. run["summary"] is assigned
   only after run_review returns; a run interrupted by a gateway restart is
   promoted from running at routes.py:215 having never got there. failureReason
   handles status === 'interrupted', so for those runs progress[cid].reason is
   the ONLY carrier a token can arrive on.
2. progress is keyed by change id; per_change is an unordered list. The
   translator resolves the per-change cause first precisely because a multi-PR
   run's run-level error may belong to a different change. Reading per_change
   means re-deriving that mapping in the frontend.
3. The sentence and the token must come from ONE record. The sentence is often
   built from a record's deep_error while the token is that record's
   skipped_reason; _first_change_failure guarantees the pairing in one pass and
   has a test for the mis-pairing. A frontend consumer would have to re-implement
   that, against a field currently typed unknown[].

So the backend field is one string with a stated contract, rather than the
frontend taking a dependency on the driver's internal record shape.

So failureReason had to recognize causes by their wording, and now carries
eight prose-keyed regexes for that.

Why it matters

Rewording a backend message silently reverted its card to verbatim
pass-through: the regex stops matching, the reader gets untranslated English
again, and no test goes red -- the frontend's fixtures are its own copies of the
backend's strings. That is the defect #7242 fixed from the outside, and the next
reword re-creates it.

And the drift has already happened -- it is on main right now.
_first_change_error maps no_review_recorded to "the reviewer finished but
wrote no findings record", and NONE of the eight prose branches matches it: the
no-record branch keys on "no result record", which is the driver's per-change
wording for the same cause. I checked all eight against that sentence on main's
own format.ts; zero match, so that card renders raw backend English today.
The sibling branch for review_record_incomplete deliberately covers both of
its wordings (per-change and run-level, see the comment on it), so the gap is an
asymmetry rather than a decision. This PR closes it for new runs via the token.

What changed (motivation -> approach -> change)

The token is carried BESIDE the sentence, never instead of it, so nothing that
reads the payload today changes.

• _first_change_failure(summary) -> (sentence, token) does it in ONE pass over
  per_change. One pass rather than two functions because the sentence is often
  built from a record's deep_error while the token is that record's
  skipped_reason; an independent scan would return a LATER record's token and
  label this failure with an unrelated cause. There is a test for exactly that.
• _first_change_error keeps its -> str signature, as a thin wrapper. It
  is pinned by eight assertions across two files (tests/test_backend_routes.py
  and test/test_sage_backend_routes_coverage.py, including a table test over
  the reason-to-sentence mapping). tests/test_backend_routes.py is untouched by
  this diff, and the change to the coverage file is a pure insertion, so those
  pins are byte-identical and still passing -- the change is additive, not a
  signature migration.
• The run record gains reason on both error branches. Absent, never blank,
  when no record carried a token, so a reader cannot mistake "no token" for one.
• Each of the driver's five failure paths puts its token on the progress entry
  it already wrote prose to, agreeing with the skipped_reason on that path.
• failureReason resolves the token through the same per-change-then-run
  precedence it already uses for the prose, and consults it exactly where the
  verbatim pass-through used to be -- which is where a reword lands.

The prose path is permanent, not transitional. progress lives inside a
persisted run record (runs.json), so every run already on disk has no token
and never will. Prose matching stays as the compatibility path for those runs
for good; it is not scaffolding to remove in a follow-up. The acceptance test
for this change is a token-less record still rendering the correct sentence.

Two deliberate narrowings of "let the translator key on the token", each with a
test pinning it:

1. The token is consulted AFTER the prose branches, not before. All three
   runtime-preflight messages carry the single token runtime_unavailable, so a
   token-first lookup would replace "no kiro-cli executable was found" and "the
   ACP runtime is not importable" -- one of which carries the exact repair
   command -- with the generic sentence. The translator's own comment on those
   branches already makes this argument about a bare /kiro-cli/ pattern;
   keying on the token first reintroduces the loss it warns about by another
   route. Prose-first changes behaviour ONLY on the pass-through path.
2. review_failed is emitted but is NOT in the translator's token table.
   The other three label a closed set of backend-authored sentences the token
   faithfully stands in for. review_failed labels whatever the failed dispatch
   returned, so its prose is arbitrary and usually far more specific -- the
   missing-agent-spec message arrives on that path carrying its own
   kirocrew setup --agent-only --clean command. It is still emitted, because
   the field's contract is "the cause token" and omitting one value would make a
   review_failed entry read as having no token at all.

Tests

Backend, test/test_sage_backend_routes_coverage.py (119 passed):

• TestFirstChangeFailureToken -- the token accompanies the humanized sentence,
  comes from the SAME record as the sentence, and is empty when that record kept
  none.
• TestRunPayloadCarriesReasonToken -- drives the real _run_review_bg with the
  driver, pool and persistence stubbed, asserting the token on the record the job
  leaves behind, on both error branches, absent on success.

Driver, tests/test_review_driver.py (59 passed) -- one test per failure path
asserting the progress entry's token AND that it agrees with the record's
skipped_reason, including every change of a preflight-failed run.

tests/test_backend_routes.py (222 passed) -- untouched, and passing, which is
the evidence the string function's contract is intact.

Frontend, src/test/CodeReviewSageRunList.test.tsx (48 passed) -- the
token-less acceptance table over nine real backend strings (asserting the
fixture carries no token, then that each still translates); the already-drifted
run-level no-record sentence, asserted both before and after; a reworded message
translating by its token; the token read from the named change; a matching prose
branch winning over the token; the agent-spec message staying verbatim despite
carrying a token; an unknown token falling back to prose.

Mutation-verified, 14 mutations, each reddening exactly its guarding test on an
AssertionError (not an exception), all Python runs with PYTHONPATH pinned to
the worktree so the shared venv's editable install could not silently test
main:

• 3 backend: token neutered; token scan decoupled from the sentence's record;
  run-level assignment deleted.
• 5 driver: one per emission site, each stripped individually.
• 6 frontend: token lookup removed; review_failed added to the table;
  per-change token read dropped; unknown token no longer falling back;
  no_review_recorded dropped from the table; and -- for the persisted-record
  property -- token-less records bypassing the prose chain, which reddens the
  acceptance table plus nine pre-existing prose tests, so old runs cannot
  degrade without a wide red.

Two of those enforce the narrowings above rather than leaving them as comments:
adding review_failed to the table reddens the verbatim guard, which is how
that regression was caught before it shipped.

Screenshots / video

The list, one card per cause. The 6th card (59m ago) is the reword case: its
backend sentence matches no prose branch, and it renders the translated
explanation via the token. The 5th is the missing-agent-spec message, still
verbatim with its repair command intact.

The detail notice, showing the explanation with the driver's raw wording kept
beneath it.

Manual verification

Both frames come from website/scripts/capture-sage-cause-translator.mjs run
against the real built SPA, and are the files it writes, so a later re-run lands
on the reviewed images.

That harness grew the reword fixture -- and an assertion that made it
observable. Its per-card check finds a card BY its expected sentence, so it could
not see a regression on a cause whose sentence another fixture also renders: the
find returned the healthy sibling's card and the broken one was never
examined. The first run passed with the token lookup deleted, which is how the
hole surfaced. It now also asserts that no cause's raw backend wording appears
anywhere in the list, and with that it fails under the same mutation, naming the
untranslated text.

Operator-facing path, for the record: failureReason feeds FailureNotice.tsx
and RunCard.tsx, both of which render its text. This change surfaces the
token through that existing channel and adds no new one.

Related Issues

Refs #7688

A bare Refs rather than a closing keyword, and not for the usual reason. The
issue's title is the payload dropping the token, and narrowing 2 above leaves
review_failed keyed on prose deliberately -- so "the token keys the
translator" is by design only partially true, and Closes would misreport that
decision as a complete fix.

Both remaining prose-keyed cases are now filed as #8198, which is also where the
reasoning for keeping review_failed prose-keyed is recorded -- the table reads
as though an entry were forgotten, and the obvious symmetry "fix" reddens the
verbatim guard and drops the missing-agent-spec repair command:

• The measured residue behind this Refs: a failed dispatch reporting ok=False
  with an EMPTY error string renders the backend's own "the review turn failed",
  which is reword-fragile but arrives on the path that must stay prose-keyed. It
  is untranslated on main today as well, so this change neither closes nor
  worsens it.
• Gap A from the token-less acceptance test: the run-level no_review_recorded
  sentence matches no prose branch on main today. This PR fixes it for new runs
  via the token; records already on disk still need the branch widened, the way
  the incomplete-record branch already covers both of its wordings.

#7688 should stay open for those.

Housekeeping for a maintainer: #7688 carries a blocked label that has been
stale since 2026-09-02T04:35:55Z. It was applied for a stated prerequisite,
#7686, which merged at that timestamp (df653d1e7, an ancestor of main), so
the label can be dropped.

Pattern harvest

Rule candidate: review-prompt

Pattern: a discriminating token collapsed into human prose at a serialization
boundary, leaving a downstream consumer to recover it by pattern-matching the
prose. The prose is the presentation and the token is the contract; when only
the prose crosses, every reword is a silent breaking change with no failing
test, because the consumer's fixtures are its own copies of the producer's
strings. This PR carries its own proof that the failure mode is real and not
theoretical -- one backend sentence already has no matching branch on main.
Worth flagging wherever a producer maps an enum to a sentence and the consumer
regexes it back.

Second, narrower candidate for the same prompt: an evidence harness that locates
its subject BY the subject's expected output cannot observe a regression in a
subject whose expected output a sibling fixture also produces -- the lookup
silently returns the healthy one. Assert the complement (the bad output appears
nowhere) rather than the per-subject form.

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of ca42552add902353d4169cc496ccce0e5162f4bd — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: PASS

Failure cards now translate by a stable cause token instead of prose-matching, so reworded backend messages can no longer silently regress to raw English.

The screenshots confirm every cause renders as an assertive what-happened + what-to-do sentence (e.g. "Reviewer never started — no kiro-cli executable was found on this host. Install it, or add it to PATH."), the token fallback resolves to existing translated catalog keys in all locales, actionable prose (the kirocrew setup --agent-only --clean repair message) still wins over the generic token, and pre-token runs on disk render exactly as before. No new surface, no new strings, no state or control changes for the user — only fewer paths to an untranslated card.

[UX-REVIEWED] ca42552

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of ca42552add902353d4169cc496ccce0e5162f4bd — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Additive machine-readable cause token beside prose, with prose-first precedence and permanent legacy-run compatibility — the right shape, alternatives explicitly weighed and pinned by tests.

[DESIGN-REVIEWED] ca42552

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed ca42552add902353d4169cc496ccce0e5162f4bd — this comment is updated in place on each push.

Review details

The candidate list contains no candidates, and my independent falsification of the diff confirms the change is sound: _first_change_failure derives sentence and token from the same per_change record in one pass (no cross-record mispairing), both run-level error branches feed the same summary so the token aligns with whichever error is set, the token is set only when non-empty (absent vs empty-string distinction preserved), and the frontend resolves the token through the identical per-change-then-run precedence as the prose while keeping prose-first ordering so the more-specific runtime-preflight messages and review_failed dispatch output are never clobbered by a generic token. No reachable crash, data loss, removed guard, or security-boundary weakening on the changed lines, and no AUTOSDE rule violation.

No findings.

[OPUS-REVIEWED] ca42552

Verdict parsed from the review's SHA-scoped output markers for commit ca42552add902353d4169cc496ccce0e5162f4bd.

False positive or not applicable? A repository writer can comment:
/ai-review override fable ca42552add902353d4169cc496ccce0e5162f4bd: <one-sentence reason>

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of ca42552add902353d4169cc496ccce0e5162f4bd and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- src/kiro_crew/apps/builtins/code_review_sage/backend/routes.py:527 -- "payload names the cause nowhere" contradicts execution because summary.per_change[].skipped_reason is serialized; the same contradiction appears in both added test comments -> Fix: reword all three comments to describe the new direct carrier without claiming the existing token is absent.
[GPT-REVIEWED] ca42552

False positive or not applicable? A repository writer can comment:
/ai-review override gpt ca42552add902353d4169cc496ccce0e5162f4bd: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of ca42552add902353d4169cc496ccce0e5162f4bd — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All verification is done. The fix is genuine (I confirmed the on-main drift: none of the eight prose branches matches the run-level no_review_recorded sentence), the i18n keys exist in all locales, all five failed progress emission sites are covered (the sixth skipped_reason value, cancelled, goes through a non-failed phase), and consumers are real. The one premise issue: _handle_runs at routes.py:914 returns run dicts verbatim, and run["summary"] = summary (routes.py:491) includes per_change with skipped_reason — so the dashboard payload already carries the token for completed runs, contradicting the description's "which the dashboard never sees" (the frontend even types it at types.ts:72, with zero readers).

First-Principles-Verdict: CONCERNS

Cause-level fix that earns its surface — but its premise "the dashboard never sees the token" is false: /runs already ships summary.per_change[].skipped_reason verbatim.

What this change ships

Intent: stop failed-run cards from silently reverting to untranslated English whenever a backend failure message is reworded — a FIX (drift verified on main).

1. Failed-run cards translate by a stable cause token when no prose branch matches — justified
2. Run payload gains a reason field — declared; 1 consumer (format.ts:240)
3. Each failed change's progress entry gains a reason token, 5 driver paths — declared; sole carrier for interrupted runs
4. The already-drifted run-level "wrote no findings record" card translates again — justified
5. review_failed emitted but deliberately never translated — declared narrowing, tested
6. Matching prose still outranks the token, preserving remedy-specific messages — declared narrowing, tested
7. Screenshot harness gains a reword fixture and a no-raw-wording-anywhere check — rides along, harness-internal
8. Regenerated after-screenshots — declared
9. _first_change_error becomes a wrapper over _first_change_failure — internal, declared

Watch

• The motivation's "the token sat on the driver-internal per_change record… which the dashboard never sees" is contradicted by the code: _handle_runs (routes.py:914) returns runs whole, run["summary"] = summary (routes.py:491) includes per_change, and the frontend already declares it (per_change?: unknown[], types.ts:72 — grepped per_change under website/src: 1 hit, zero readers). A frontend-only fix reading that existing field was therefore available for every completed run and is never weighed. The shipped shape still has independent legs — interrupted runs persist progress but no summary, and the same-record sentence/token pairing lives once, backend-side — so this is a premise correction, not a duplicate verdict. A human should confirm the extra backend surface is preferred over consuming the field the payload already carries.

[FIRST-PRINCIPLES-REVIEWED] ca42552

[chenmingwei23]

First Principles is right and I have corrected the description rather than
argued with it. Recording the verification and the answer to the question it
raises for a human.

The premise was false. I checked the three claims independently:

• run["summary"] = summary at routes.py:491, and summary carries
  per_change.
• _handle_runs at routes.py:914 returns {"runs": runs, ...} with the run
  dicts whole -- no field filtering.
• grep -rn per_change website/src/ returns exactly one hit,
  types.ts:72 per_change?: unknown[], and zero readers.

So summary.per_change[].skipped_reason is already in the /runs payload for
completed runs, and "which the dashboard never sees" was wrong. The description
now says so explicitly instead of quietly dropping the sentence, because the
mistaken claim is what made a frontend-only alternative invisible, and a reader
comparing the description against the payload deserves to see the correction.

On whether the backend field is preferred over consuming the existing one --
the reviewer asks a human to confirm this, so here is the case, with the parts I
verified:

1. Interrupted runs have no summary at all. run["summary"] is assigned
   only after run_review returns. A run interrupted by a gateway restart is
   promoted from running at routes.py:215 having never reached that line, so
   it has progress and no summary. failureReason handles
   status === 'interrupted', so for those runs progress[cid].reason is the only
   carrier a token can arrive on. per_change cannot serve them.
2. progress is keyed by change id; per_change is an unordered list. The
   translator prefers the per-change cause specifically because a multi-PR run's
   run-level error may belong to a different change. Consuming per_change means
   re-deriving that change-to-record mapping in the frontend.
3. The sentence and the token have to come from the same record. The sentence
   is often built from a record's deep_error while the token is that record's
   skipped_reason. _first_change_failure does both in one pass for exactly
   that reason, and
   TestFirstChangeFailureToken::test_token_comes_from_the_SAME_record_as_the_sentence
   pins the mis-pairing a second independent scan would produce. A frontend
   consumer would have to re-implement that pairing against a field typed
   unknown[].

Net: the added surface is one string with a stated contract, versus the frontend
taking a dependency on the driver's internal per-change record shape. I think that
is the right trade, but it is a judgement and the reviewer was correct that it was
never written down. It is in the description now.

One thing the review got exactly right that is worth echoing for the record: the
sixth skipped_reason value, cancelled, is deliberately not covered here --
it reaches a cancelled phase, not failed, and failureReason returns null for
a cancelled run. Nothing to translate on that path.

[bolichen97]

Approving on the strength of a full readiness audit of every open PR against main, not a
line-by-line reading of this diff — recording that plainly so the next reader knows what this
stamp does and does not cover.

Verified against this exact head SHA:

• readiness: passed present, and PR Readiness — the one required status context on main
  (ruleset protected-branches) — is success on this head.
• No check run on this head is failure, cancelled, timed_out or still in flight. Skipped
  jobs are path-filtered conditionals, none of them required.
• mergeable: true, and the head is not far enough behind main for its green CI to describe a
  base that no longer exists.
• No surviving reviewer CHANGES_REQUESTED: any such review is on an older commit and therefore
  already dismissed by dismiss_stale_reviews_on_push.
• Every issue comment, inline review comment and review thread was read and classified. Nothing
  left is an unresolved human change request — the remainder is bot review-lane output, resolved
  or outdated threads, explicitly non-blocking suggestions, and author status notes.

Auto-merge (squash) is armed, so this lands once every other ruleset requirement is met.

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #8143 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8143: REBASE. Unrelated goal (failure-cause token vs parallelization) but a direct textual and semantic collision in the rewritten block; the rebase must re-add PR #8186's run["reason"] handling in the new, lock-free control flow. Files: src/kiro_crew/apps/builtins/code_review_sage/backend/routes.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.