fix: restore apply_patch diff stats/viewer and align tool card rendering#214
fix: restore apply_patch diff stats/viewer and align tool card rendering#214hightman wants to merge 2 commits intoOpen-ACP:developfrom
Conversation
0xmrpeter
left a comment
There was a problem hiding this comment.
Hey @hightman, thanks for putting this together! The overall approach is solid — the defensive parsing is consistent, the backward-compatibility guard works well, and the test coverage is genuinely good quality. A couple of things worth addressing before merge:
[Important] Dead code in buildTitle
The new isApplyPatchOther early-return block at the top of buildTitle correctly handles all apply_patch kind=other calls. However, there are two older nameLower === "apply_patch" blocks further down in the same function that are now unreachable for any apply_patch call going through buildToolSpec — since effectiveKind is already remapped to "edit" before buildTitle is called, the old blocks will never fire.
Leaving them in creates confusion about which path is authoritative. Would be great to clean those up as part of this PR.
[Important] Multi-file patch: viewer link only shows first file
parseApplyPatchRawOutput correctly sums additions/deletions across all files in metadata.files for diffStats — nice. But the function returns on the first valid file, so when apply_patch touches multiple files in one call, only a single viewer link is generated (for whichever file comes first in the array), which may not be the most relevant one.
A simple improvement: sort files by additions + deletions descending before the loop, so the viewer link points to the file with the most changes — consistent with what diffStats reflects. If a full fix isn't in scope right now, at minimum a comment acknowledging this limitation would help future maintainers.
[Minor] Detection logic is duplicated across 3 files
The apply_patch detection heuristic (kind === "other" + name match or patchText presence) is re-implemented inline in message-transformer.ts, extract-file-info.ts, and display-spec-builder.ts. Not a blocker, but if the heuristic ever needs updating it'll need to change in 3 places. Worth extracting to a shared utility in a follow-up.
[Minor] PR checklist
The - [ ] Ran pnpm test — all tests pass item is still unchecked. Totally fine if you only ran the affected test files, just worth confirming the full suite doesn't surface anything unexpected.
Overall this is close — the core logic is correct and the tests are well-structured. Addressing the dead code cleanup and the multi-file viewer gap (even just with a comment) would get this to approve.
Unify apply_patch detection, harden diff/viewer extraction for tool_update payloads, prioritize highest-churn files for multi-file viewer links, and map apply_patch cards to edit-style rendering for consistent UI behavior. Co-authored-by: Codex <noreply@openai.com>
Keep a single TodoWrite title formatter in buildTitle to reduce ambiguity while preserving existing compatibility behavior. Co-authored-by: Codex <noreply@openai.com>
47ac7e0 to
edfa5f3
Compare
|
All review feedback has now been addressed and the branch was force-pushed.
Validation:
Latest commit: edfa5f3 |
Summary
Improve apply_patch compatibility for OpenCode-style ACP payloads so Telegram tool cards can consistently show line-change stats and viewer links, including tool_update events that omit the tool name. Also render apply_patch with existing edit-style tool card semantics for simpler, backward-compatible UI behavior.
Related Issues
Type of Change
Changes Made
MessageTransformerand added a safe apply_patch compatibility branch forkind=otherpayloads.rawOutput.metadata(files[].additions/deletions, optional top-level totals) without affecting non-apply_patch agents.extractFileInfowith an apply_patch-only early branch (includingpatchText/patch_textfallback) so viewer links are generated fromrawOutput.metadata.files[].before/after.nameis missing by recognizing cached patch text input.kind=otherreuses edit-style card rendering while preserving patch target title extraction from patch text.Testing
pnpm test— all tests passTest commands executed:
pnpm vitest run src/core/adapter-primitives/__tests__/display-spec-builder.test.ts src/core/__tests__/message-transformer-extended.test.ts src/__tests__/extract-file-info.test.tspnpm buildPlatform Impact
Screenshots / Recordings
Screenshots(original):

Screenshots(after patching):

Checklist
.jsextensions)pnpm buildsuccessfullysrc/cli/plugin-template/