feat: promote tool-derived diffs to first-class chat cards

[CrysisDeu]

Summary

Adopts the structured-diff display model (RFC: docs/request-for-change/rfc-tool-derived-diff-cards.md, included in this PR): kiro-cli's edit tools already deliver a structured diff on the ACP tool_call event, and the backend already renders it to a unified-diff string on tool_input — but the dashboard hid it inside the collapsed tool-call details panel while the prompt ordered the model to hand-write the same diff as a ```diff block, on every file change, no exceptions.

This PR makes the tool-derived diff the primary display and relaxes the prompt mandate to a runtime-conditional fallback.

Changes

• Frontend: ToolCallLine renders an always-visible DiffBlock card below the pill when the tool kind is edit and its input is a unified diff. New pages/chat/toolDiff.ts owns detection: kind gate first (a shell command with diff-shaped input never promotes), 400-line cap (whole-file creates keep the collapsed rendering). TurnBlock excludes diff-card rows from the "Worked through N steps" collapse — same predicate family as workflow_run / spawn_run / MCP-app rows.
• Backend: make_unified_diff's transport cap is raised from 6 000 chars to 64 KiB (sized so the full-card range is never cut; ~11x larger persisted meta.input / WS payloads for large edits, within the existing 1 MB tool-field ceiling), with line-boundary truncation and a machine-detectable annotation. derive_edit_diff turns bare-JSON edit args (create / insert / strReplace) into a unified diff server-side, so every tool_input consumer sees the same diff.
• Backend: _tool_meta persists the ACP tool kind so historical rows gate identically to live ones. Rows persisted before the field never promote (fail-safe).
• Prompt: the diff-block rule in prompt.md, prompt-orchestrator.md, and _CRITICAL_RULES becomes conditional — ```diff blocks are required only when (a) the change was made outside file-editing tools (sed, scripted bulk edits, git apply), or (b) the runtime is not the dashboard. Slack/Discord/Telegram renderers never see tool_input (verified: `messaging/driver.py` copies four fields onto OutputEvent), so message text remains their only file-change display and the mandate stays for them.

Evidence

Isolated dev-gateway session showing an edit turn (fs_write, kind=edit): the card renders and stays visible with the turn collapsed. The shell negative case (kind=execute never promotes, even with diff-shaped input) is pinned by automated tests in toolDiff.test.tsx and TurnBlock.test.tsx.

Light	Dark

Tested

• New: toolDiff.test.tsx (13 cases: kind gating, shell false-positive rejection, line cap, historical-meta path, card render), TurnBlock.test.tsx diff-card visibility x3, backend test_tool_meta_persists_kind.
• Full gates: tsc, vitest 22747 passed (one failing file, CliPanelCoverage, passes standalone and pre-dates this change — full-run cross-file pollution), pytest 60119 passed (21 failures reproduce identically on pristine origin/main — host-environment class), flake8, mypy, docs-lint, brand gate all green. isort failure on validation.py also reproduces on pristine main.

Not in scope

Extending messaging OutputEvent with tool_input so channel renderers could show diffs (would let the conditional prompt rule retire entirely) — recorded in the RFC as future work.

Behaviors a reader should expect (beyond the headline)

• Over-cap or transport-truncated diffs degrade to a summary chip (filename, −N/+M, truncation note) that expands the details panel — a new affordance, not the old collapsed rendering.
• Each card folds to its chip and back via the header chevron; fold state persists across virtualized scrolling within the session.
• A dashboard-started session continued from a channel (Slack/Discord/...) gets a per-turn re-assertion of the hard ```diff mandate on non-dashboard turns; it is never retracted in the dashboard direction.

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

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

UX-Verdict: CONCERNS

Solid promotion with fail-safe degradation, but the new density controls are ephemeral and override the user's stated collapse preference with no lasting relief.

Watch

• Folds don't survive a reload: foldedDiffCards is a module-scope Set ("survives virtualizer unmounts for the page lifetime"), so a reader who folded five 400-line cards reopens the session to all of them expanded again. Frequency: every revisit of an edit-heavy session × friction × every time — persist folds (storage or message meta), or default-fold cards past a smaller line threshold.
• Cards deliberately ignore the global collapse-all-steps preference ("renders REGARDLESS of the collapse-all-steps preference") and are exempt from the "Worked through N steps" fold. Combined with the 400-line card cap and non-persistent folds, a density-minded user who opted into collapse-all now gets an immovable wall they must re-fold per card per visit. Smallest fix: when collapse-all is on, render the summary chip (still a visible trace) instead of the open card.
• In both screenshots (diff-card-dark.png, diff-card-light.png) no resting chevron is discernible at the card header's left edge, where the code places the sole per-card fold handle ("visible at rest (muted)") in the same 32px zone Pierre paints its file icon. If it's invisible or colliding at rest, the only density control is undiscoverable — verify the rendered rest state.

Suggestions

• The folded-card chip and the summary chip are pixel-identical but do different things (unfold inline card vs. expand details panel); a caret or distinct icon on the details-opening variant would set expectations.

[UX-REVIEWED] 79fe604

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

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

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 79fe604

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

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Server-side runtime selection with every ambiguity failing toward the hard mandate makes the risky half of this — relaxing a display-critical prompt rule — safely directional.

The one residual model-judgment surface (tool-vs-shell on the dashboard) is named in the RFC, its failure is backstopped by the untouched file_changes chips and Activity Files tab, and every other failure mode (unknown runtime, missing meta.kind, mislabeled tool kind, transport truncation, denied edits, channel-continued sessions) degrades to either the old rendering or the unconditional mandate. New persisted surface (meta.kind, 64 KiB tool_input) is additive and reversible; the deferred OutputEvent extension is correctly scoped out rather than half-shipped.

[DESIGN-REVIEWED] 79fe604

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 79fe604e8c1756dd6ab26fb200bd40e49bcf4d5f — 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.

I've verified the key claims against the repo: the temp-screenshots/ convention is established (2,036 committed files, a cleanup workflow), countDiffStats/isDiffText are reused rather than duplicated, and I checked the two refinement-event builders for the derive_edit_diff fallback. Final review below.

First-Principles-Verdict: CONCERNS

The header-path regex rewrite in DiffBlock is an undeclared rider the RFC explicitly denies ("DiffBlock is reused unchanged"), and the derive fallback covers 2 of 4 input builders.

What this change ships

Intent: show file changes as first-class diff cards built from tool data instead of model-restated ```diff prose — an ADDITION.

1. Edit-tool diffs render as always-visible cards below the pill — justified
2. Over-cap or truncated diffs degrade to a summary chip with lower-bound counts — justified
3. Cards fold to a chip; folds survive virtualized scrolling — justified (declared; sole density relief)
4. Diff rows stay out of both turn folds — justified
5. Diff-block prompt mandate becomes server-selected: relaxed on dashboard, hard elsewhere — justified
6. Channel turns in dashboard-started sessions re-assert the mandate per turn — justified, declared
7. Backend derives diffs from bare create/insert/strReplace args — justified; siblings unfixed
8. Diff transport cap 6 000 → 64 KiB, line-boundary cut, marked — justified
9. Tool kind persisted in message meta; pre-existing rows never promote — justified
10. Diff header paths with spaces now resolve fully (TAB terminator) for ALL diff blocks — undeclared, rides along

Watch

• Item 10 changes path extraction for every existing model-authored diff block, not just new cards, yet the description never mentions it and the shipped RFC §4 says "DiffBlock … reused unchanged" while the diff adds onFold, exports extractFilePath, and rewrites four regexes. The fix is real (a spaced path resolved to a sibling file and the open-in-panel affordance targeted the wrong file) — keep it, but declare it; the RFC sentence is contradicted by the diff.
• Point coverage: derive_edit_diff is called from 2 sites (grep derive_edit_diff(: _dispatch.py:866, client.py:5235 — both initial tool_call builders). The 2 refinement builders (_build_tool_refinement_event, _dispatch.py:1120-1142; _extract_tool_call_refinement, client.py:5404-5424) still json.dumps bare edit args — and client.py:5260 documents that some backends stream empty initial rawInput, so exactly those backends never derive a diff and never promote. Fail-safe direction, but name the gap or close it.

Subtractions

• Fold the folded-chip stats branch in ToolCallLine.tsx (extractDiffHeaderPath + countDiffStats, lines ~541 and ~584) into presentToolDiff by returning path/±counts for card mode too — 2 sites currently compute the same summary from the same diff (toolDiff.ts:81 is the other); one spelling suffices.

[FIRST-PRINCIPLES-REVIEWED] 79fe604

[CrysisDeu]

Disposition — UX Review (lane: ux-review)

Finding 1 (over-cap edits lose their only visible diff): ADOPTED, fixed in 78eba5d. presentToolDiff now degrades an over-cap edit to a summary chip — filename, −N/+M counts, click expands the details panel holding the full diff — instead of returning null. Every edit-tool diff has a visible transcript trace regardless of size. Covered by toolDiff.test.tsx ("degrades an over-cap diff to a summary — never to nothing") and a ToolCallLine render test asserting the chip appears and the full card does not.

Finding 2 (cards exempt from collapseAll with no override): ADOPTED, fixed in 78eba5d. isDiffCardItem was removed from isVisibleInline: collapseAll (the user's explicit fold preference) now folds diff rows with the other working steps; only DEFAULT mode keeps them out of the tool-call fold. TurnBlock.test.tsx re-pinned to the new contract (step counter includes the edit row under collapseAll).

[CrysisDeu]

Disposition — Design Review (lane: design-review)

Finding 1 (channel regression rides on per-turn model judgment): ADOPTED, fixed in 8038325. Implemented your suggestion: the runtime clause is now decided SERVER-SIDE. _critical_rules_for (context.py) reuses the trusted _runtime_display_name resolution to select between two fixed module constants — dashboard sessions get the don't-repeat rule, every other surface (Slack/Discord/Telegram/cron/subagent/CLI) gets the original unconditional mandate verbatim. Only the tool-vs-shell clause (a) stays with the model, and its failure mode is a duplicate diff, never a missing one. Covered by test_diff_rule_is_runtime_selected (explicit runtime_source wins over the key-derived guess).

Finding 2 (over-cap edits lose their primary display): ADOPTED, fixed in 78eba5d (also raised by UX Review). Over-cap edits degrade to a summary chip (filename, −N/+M, expands the details panel) instead of nothing.

Finding 3 (temp-screenshots committed at repo top level): NOT ADOPTED. Committed QA media under temp-screenshots/<feature>/ is this repo's documented convention (kirocrew-worktree-dev Rule 9 names it as the one sanctioned committed deliverable; ~2,000 files already live there, as First Principles' review of this same PR counted). The PR-body links are commit-SHA-pinned and require the files on the merged history. Deviating from the convention in one PR would break the screenshot-evidence pattern the other lanes verify.

[CrysisDeu]

Disposition — First Principles Review (lane: first-principles-review)

Watch (RFC ships self-contradicting): ADOPTED, fixed in 8038325. Front matter now status: in-progress, implementation-prs: ["#5012"]; the index row states what ships with this PR and names the unstarted §3.3 remainder.

Watch (RFC §3.1/§5 contradict the shipped code): ADOPTED, fixed in 8038325. §3.1 rewritten to the actual architecture (presentation renders inside ToolCallLine — one site covers ChatPage, ChatPane, and app-sdk surfaces — plus the summary-chip degradation); the §5 complete={false} shimmer row replaced with the true statement (live toolLog input and persisted meta.input carry the same redacted value, so no truncation asymmetry exists).

Subtraction (un-export DIFF_CARD_MAX_LINES): ADOPTED, fixed in 8038325. Module-private; tests inline the boundary values.

[CrysisDeu]

Disposition — Design Review round 2 (lane: design-review)

Finding 1 (edits outside the built-in tools lose their trace under the relaxed rule): ADOPTED, fixed in 76ac00f. The dashboard rule now scopes the don't-repeat clause to "the BUILT-IN file-editing tools" and explicitly lists "an MCP tool that writes files" in the must-emit set (_DIFF_RULE_DASHBOARD, prompt.md, prompt-orchestrator.md). Which tool it invoked is something the model directly observes, so this keeps the judgment on an observable — an MCP or adapted-harness edit gets the model-authored diff block exactly as before this PR.

Finding 2 (6 KB make_unified_diff cap vs the chip's full-diff promise): ADOPTED, fixed in 76ac00f. The cap defaults to 64 KiB — sized so the full-card range (≤400 lines) is never cut — and truncation now lands on a LINE boundary with a \ diff truncated annotation (the unified-diff escape convention; renderers skip it), so a cut is machine-detectable instead of a silent mid-line slice. client.py's duplicate copy now delegates to the single implementation. Covered by test_truncation_cuts_at_line_boundary_and_is_marked / test_under_cap_diff_is_not_marked; the residual (counts understate past 64 KiB, marked) is documented in the RFC risk table.

Suggestion (compare raw source, not localized display): ADOPTED, fixed in 76ac00f. Resolution extracted to _resolve_runtime_source (raw key); _runtime_display_name is now a display mapping over it and _critical_rules_for compares the raw source — no dependency on _RUNTIME_DISPLAY string stability.

[CrysisDeu]

Disposition — UX Review round 2 (lane: ux-review)

Finding 1 (denied edits look applied): ADOPTED, fixed in e5c5f31. A rejected or auto-denied call never promotes: diffView is gated on !isRejected && !isAutoDenied. The full diff stays readable in the expanded details panel; the pill's red/amber status icon is no longer visually contradicted by a first-class card. Covered by "suppresses the card for a rejected edit".

Finding 2 (collapseAll loses the visible change record): ADOPTED, fixed in e5c5f31, exactly as you suggested. Under "collapse all working steps" every diff row demotes to the one-line −N/+M summary chip (new useCollapseAllSteps hook; presentToolDiff takes a card-line ceiling and 0 forces summary), and TurnBlock keeps diff rows visible-inline again — one visible line per change, no wall of cards, chip click expands the full diff. Covered by "demotes the card to the summary chip under collapse-all-steps" and the TurnBlock visibility test.

[CrysisDeu]

Disposition — First Principles Review round 2 (lane: first-principles-review)

Subtraction (inline +/− loop duplicates countDiffStats): ADOPTED, fixed in e5c5f31. presentToolDiff imports countDiffStats from utils/diffLineCounts — thanks for the pointer, the module was unknown to this change.

Subtraction (headerPath duplicates extractFilePath): ADOPTED, fixed in e5c5f31. extractFilePath is exported from DiffBlock.tsx and reused; the local scanner is deleted.

Subtraction (one-member _DIFF_CARD_RUNTIMES frozenset): ADOPTED, fixed in e5c5f31. Direct source == "dashboard" comparison.

Watch (prompt.md still ships the model-judged runtime clause, three spellings to sync): ADOPTED, fixed in e5c5f31. Clause (b) is deleted from both bundled prompts; the paragraph now defers to the runtime-selected critical rules as authoritative and keeps only the tool-vs-shell/MCP distinction (the one thing the model can observe). One authoritative spelling per surface, selected server-side.

Watch (AGENTS.md: new markdown file needs explicit instruction): CONFIRMED-AS-INSTRUCTED. The RFC was explicitly requested by the repository owner in the originating session ("写一个 RFC 把方案定下来" — "write an RFC to settle the design"), not convention-inherited.

[CrysisDeu]

Disposition — UX Review round 3 (lane: ux-review)

Finding 1 (truncated diffs look complete): ADOPTED, fixed in 820bf32. presentToolDiff detects the transport's \ diff truncated annotation: a truncated payload never renders as a card (it can sit under the line cap while missing most of the change) — always the summary chip, now carrying a visible localized "diff truncated" note (new pages.chat.toolCallLine.diff_truncated key, all 14 catalogs) so the −N/+M counts read as lower bounds. Covered by "a transport-truncated diff is always a summary, flagged truncated".

Finding 2 (edit-kind row without a unified diff leaves no trace): ADOPTED, fixed in 820bf32. A kind === 'edit' row whose input is not diff-shaped falls back to a pathname chip (basename from the existing arg-path extraction, expands the details panel); counts are omitted because nothing exists to count them from. Covered by "falls back to a pathname chip for an edit row without a transported diff".

[CrysisDeu]

Disposition — Design Review round 3 (lane: design-review)

Finding (cross-surface sessions outrun the session-start rule selection): ADOPTED, fixed in 820bf32, using your asymmetric suggestion. The per-turn runtime refresh in build_message (the same path that re-injects [RUNTIME] from trusted dispatcher metadata) now appends the hard channel mandate whenever the resolved source for THIS turn is not dashboard — and never retracts the mandate in the other direction, exactly because the fail direction is asymmetric (dashboard duplicate = cosmetic; missing channel diff = no record). prompt.md / prompt-orchestrator.md no longer call the session-start rules "authoritative"; they now defer to "the critical rules injected for your session and any per-turn surface note next to the [RUNTIME] line — the most recent instruction wins", so the model is never told to disbelieve the refreshed runtime. Covered by test_channel_turn_reasserts_diff_mandate_mid_session (discord turn injects the mandate; dashboard turn injects nothing).

[CrysisDeu]

Response — GPT 5.6 blocking finding (fixed in 6950ee5)

Malformed edit args abort the turn: FIXED. derive_edit_diff type-narrows every argument it forwards: a non-string path returns "" outright, and non-string oldStr/newStr coerce to "" before difflib — a TypeError can no longer escape the dispatch path. Pinned by test_non_string_arguments_never_reach_difflib (numeric path, dict oldStr, list path, numeric fileText). The create/insert branches already isinstance-gated their content values.

[CrysisDeu]

Disposition — First Principles CONCERNS (on 6950ee5):

1. Subtraction: delete aria_show_diff (0 consumers) — ADOPTED. Removed
   from en.manual.json and all 12 hand-maintained locales (pure line-delete),
   en-XA.json regenerated. The dead-keys ratchet independently caught the
   same key and is back at baseline.
2. Items 2/3/6 undeclared in the PR body — ADOPTED. The body now carries a
   "Behaviors a reader should expect" section declaring the summary-chip
   affordance (correcting the stale "keep the collapsed rendering" phrasing),
   per-card fold with scroll-persistent state, and the per-turn channel
   re-assertion of the hard diff mandate.

[CrysisDeu]

Fix note for the blocking finding (head 7284819):

• BLOCKING — header paths truncated at spaces — fixed at the root instead
  of removing the affordance: extractFilePath's terminator is now TAB-only
  (the unified-diff timestamp separator; both difflib and git emit the path as
  the whole remainder of the line, so a space is a legal path character).
  /work/report final.md now resolves exactly; a space-separated timestamp
  from a foreign tool stays attached, fails the existence probe, and the
  open affordance is simply not offered — fail-safe in the harmless direction.
  Regression tests cover the spaced-sibling case and TAB-timestamp stripping.
  (The regexes predate this PR, but the promotion raised their blast radius —
  the finding stands and the shared fix benefits every consumer.)
• FINDING — local imports in client.py — fixed: make_unified_diff and
  derive_edit_diff moved into the existing top-level _dispatch import
  block; no cycle (_dispatch does not import client).

[CrysisDeu]

Fix note for the blocking finding (head 79fe604):

• BLOCKING — minimal-context crons lose the file-diff mandate — fixed
  exactly as suggested: both bundled prompts now default to the unconditional

  surface note) acting as the RELAXATION rather than the source of the
  requirement. A session that receives no critical-rules block at all — the
  `minimal_context=True` cron early-return — falls back to the hard mandate
  instead of silence. The pointer structure survives (the prompts still do not
  restate the per-surface rule); only the fail-safe direction flipped. RFC
  §3.2 records the default direction and the minimal-context rationale.

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

Based on my analysis of the diff and the candidate list:

CANDIDATE 1 (make_unified_diff exceeds max_len for tiny caps): The overflow only occurs when max_len < len(DIFF_TRUNCATION_MARK)+1 (17). Both call sites default to 65536; derive_edit_diff's internal calls use the default; no caller anywhere passes a small max_len, and tests use ≥100. (a) has no concrete reachable input. Killed.

CANDIDATE 2 (insert hunk off-by-one): The candidate itself concedes the effect is cosmetic gutter numbering and that it could not confirm kiro-cli's insertLine semantics. No observable wrong outcome grounded in code I opened — (c) fails, and it is not a defect class. Killed.

CANDIDATE 3 (new RFC markdown file): docs/request-for-change/ is a sanctioned category (README index carries three prior RFCs, and this PR adds its index row per docs-lint). Not a runtime defect; the "no new markdown" convention is not an AUTOSDE blocking rule and this pipeline does not own it as a finding. Killed.

CANDIDATE 4 (temp-screenshots/ PNGs): Repo hygiene, not a behavioral defect; not packaged into the wheel (only src/kiro_crew ships). Not a finding class here. Killed.

Step 2 review of the introduced code: the make_unified_diff truncation stays within max_len at the default; the frontend TRUNCATION_RE correctly matches the backend \n\ diff truncated tail; derive_edit_diff guards non-string path/content before reaching difflib; the _resolve_runtime_source split preserves display behavior (the wrapper re-applies _RUNTIME_DISPLAY) and the rule selection compares the raw source key; the neutralization prefix check covers both fixed critical-rules constants. Nothing grounds (a)(b)(c) at 80+.

No findings.

[OPUS-REVIEWED] 79fe604

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

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