From 91346e094ea9249208ebfbbb6a09f0b921acf939 Mon Sep 17 00:00:00 2001 From: Eve Date: Thu, 30 Jul 2026 13:30:31 +0800 Subject: [PATCH] fix(transcript): stop treating header-like content lines as file headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review on #6158. parseUnifiedDiff matched "---" / "+++" / "diff --git" / "index " at any position, so a changed line whose *content* starts with a dash or plus was silently discarded: parseUnifiedDiff("@@ -1 +1 @@\n--- old markdown\n+++ new markdown\n") // before: [{ kind: "gap", ... }] — both changed lines gone A removal of "-- old markdown" is spelled "--- old markdown" on the wire, so this hit Markdown rules, embedded patches, and comment banners. File headers only exist ahead of the first hunk, so they are only recognised there; once inside a hunk every line is parsed strictly by its first character. Also localizes the multi-file summary count, which was hardcoded English and so leaked into the zh-Hans / ja / ko transcript rows. The presenter owns no React and no i18n by design, so the phrasing is injected by the caller rather than imported here, keeping the module unit-testable in isolation; the English form remains the fallback. The three Chinese/Japanese/Korean truncation strings now use "..." to match the English source they translate. Verified: both new parser assertions fail against the previous strip-anywhere behaviour and pass now; 47 presenter tests green, repo typecheck and lint clean. Co-authored-by: multica-agent --- .../agent-transcript-dialog.tsx | 11 +++- .../trace-event-presenter.test.ts | 50 +++++++++++++- .../task-transcript/trace-event-presenter.ts | 66 +++++++++++++------ packages/views/locales/en/agents.json | 1 + packages/views/locales/ja/agents.json | 5 +- packages/views/locales/ko/agents.json | 5 +- packages/views/locales/zh-Hans/agents.json | 5 +- 7 files changed, 115 insertions(+), 28 deletions(-) diff --git a/packages/views/common/task-transcript/agent-transcript-dialog.tsx b/packages/views/common/task-transcript/agent-transcript-dialog.tsx index a06859e718..6e0e0cd648 100644 --- a/packages/views/common/task-transcript/agent-transcript-dialog.tsx +++ b/packages/views/common/task-transcript/agent-transcript-dialog.tsx @@ -65,7 +65,7 @@ import { traceEventSummary, traceEventSummaryIsMono, } from "./trace-event-presenter"; -import type { TraceDiffLine, TracePatchFile } from "./trace-event-presenter"; +import type { TraceDiffLine, TracePatchFile, TraceSummaryLabels } from "./trace-event-presenter"; import { highlightBlock, highlightToLines, languageForPath } from "./diff-highlight"; import { useT } from "../../i18n"; import "../../editor/styles/code.css"; @@ -1102,7 +1102,14 @@ const TranscriptEventRow = ({ const kind = traceEventKind(item); const color = getEventColor(item); const label = traceEventLabel(item); - const summary = traceEventSummary(item); + // The presenter stays i18n-free, so the localizable phrasing is injected. + const summaryLabels = useMemo( + () => ({ + morePaths: (path, count) => t(($) => $.transcript.patch_summary_more, { path, count }), + }), + [t], + ); + const summary = traceEventSummary(item, summaryLabels); const date = useMemo( () => (item.created_at ? new Date(item.created_at) : null), [item.created_at], diff --git a/packages/views/common/task-transcript/trace-event-presenter.test.ts b/packages/views/common/task-transcript/trace-event-presenter.test.ts index 7047fa35e6..0435fea238 100644 --- a/packages/views/common/task-transcript/trace-event-presenter.test.ts +++ b/packages/views/common/task-transcript/trace-event-presenter.test.ts @@ -308,7 +308,7 @@ describe("parseUnifiedDiff", () => { ]); }); - it("drops file headers and no-newline metadata", () => { + it("drops file headers ahead of the first hunk", () => { const lines = parseUnifiedDiff( "diff --git a/x b/x\nindex 111..222 100644\n--- a/x\n+++ b/x\n@@ -1 +1 @@\n-a\n+b\n\\ No newline at end of file\n", ); @@ -319,6 +319,25 @@ describe("parseUnifiedDiff", () => { ]); }); + // Inside a hunk, "---"/"+++" are ordinary changed lines whose content starts + // with a dash or plus — a Markdown rule, a nested patch, a comment banner. + // Treating them as file headers anywhere silently deleted real content. + it("keeps header-like content lines once inside a hunk", () => { + expect(parseUnifiedDiff("@@ -1 +1 @@\n--- old markdown\n+++ new markdown\n")).toEqual([ + { kind: "gap", text: "@@ -1 +1 @@" }, + { kind: "remove", text: "-- old markdown" }, + { kind: "add", text: "++ new markdown" }, + ]); + }); + + it("keeps a removed line that is exactly a Markdown rule", () => { + expect(parseUnifiedDiff("@@ -1,2 +1,1 @@\n---\n ok\n")).toEqual([ + { kind: "gap", text: "@@ -1,2 +1,1 @@" }, + { kind: "remove", text: "--" }, + { kind: "context", text: "ok" }, + ]); + }); + it("keeps an empty context line and does not invent a trailing one", () => { // " " is a blank unchanged line; the phantom element left by the trailing // newline is not. @@ -475,6 +494,35 @@ describe("traceToolArgSummary / traceEventHasDetail — Codex changes[]", () => ).toBe(".../d/e.go"); }); + // The presenter stays i18n-free, so the caller injects the phrasing; the + // English form is only the fallback. + it("uses injected phrasing for the multi-file count", () => { + expect( + traceToolArgSummary( + { + changes: [ + { path: "src/a.go", kind: "update", diff: "@@\n+x" }, + { path: "src/b.go", kind: "add", content: "y" }, + ], + }, + { morePaths: (path, count) => `${path} 等 ${count} 个文件` }, + ), + ).toBe("src/a.go 等 1 个文件"); + }); + + it("does not consult the injected phrasing for a single file", () => { + expect( + traceToolArgSummary( + { changes: [{ path: "only.go", kind: "add", content: "x" }] }, + { + morePaths: () => { + throw new Error("must not be called for a single-file patch"); + }, + }, + ), + ).toBe("only.go"); + }); + it("makes a patch row expandable — the bug was two blank unexpandable rows", () => { expect( traceEventHasDetail({ diff --git a/packages/views/common/task-transcript/trace-event-presenter.ts b/packages/views/common/task-transcript/trace-event-presenter.ts index ed1ce555fb..bfa0b93ef4 100644 --- a/packages/views/common/task-transcript/trace-event-presenter.ts +++ b/packages/views/common/task-transcript/trace-event-presenter.ts @@ -95,18 +95,32 @@ function clip(value: string, max: number): string { return value.length > max ? value.slice(0, max) + "..." : value; } +/** + * Localizable phrasing the presenter cannot produce on its own. This module + * stays free of React and i18n so it remains unit-testable in isolation, so the + * caller injects the wording instead. Omitting it falls back to English, which + * keeps the fallback safe rather than blank. + */ +export interface TraceSummaryLabels { + /** Phrase a multi-file patch, e.g. `src/a.go +2 more`. */ + morePaths?: (path: string, count: number) => string; +} + /** * The single most informative argument of a tool call, as one line. Preference * order matches what a reviewer scans for first, falling back to the first * short string value. */ -export function traceToolArgSummary(input: Record | undefined): string { +export function traceToolArgSummary( + input: Record | undefined, + labels?: TraceSummaryLabels, +): string { if (!input) return ""; const str = (v: unknown): string => (typeof v === "string" ? v : ""); if (str(input.query)) return str(input.query); // A multi-file patch has no single path field; without this the row's // summary would fall through to the generic scan and come back empty. - const patch = readPatchSummary(input); + const patch = readPatchSummary(input, labels); if (patch) return patch; if (str(input.file_path)) return shortenTracePath(str(input.file_path)); if (str(input.path)) return shortenTracePath(str(input.path)); @@ -135,12 +149,12 @@ function collapseWhitespace(value: string | undefined): string { } /** One-line summary for the collapsed row — never contains a newline. */ -export function traceEventSummary(event: TraceEvent): string { +export function traceEventSummary(event: TraceEvent, labels?: TraceSummaryLabels): string { switch (traceEventKind(event)) { case "thinking": return clip(firstLine(event.content), 200); case "tool_use": - return traceToolArgSummary(event.input); + return traceToolArgSummary(event.input, labels); case "tool_result": // Unwrap first: the collapsed row is the one people read without // clicking, so it must not show transport escaping. @@ -355,24 +369,33 @@ export function parseUnifiedDiff(diff: string): TraceDiffLine[] { if (raw.length > 0 && raw[raw.length - 1] === "") raw.pop(); const out: TraceDiffLine[] = []; + // File headers only exist ahead of the first hunk. Past that point every + // line belongs to the file, and "---"/"+++" are ordinary changed lines whose + // content happens to start with a dash or plus — a Markdown rule, a nested + // patch, a comment banner. Treating them as headers anywhere deleted real + // content from the diff. + let inHunk = false; for (const line of raw) { - // File headers carry no content and the path is reported alongside. - if ( - line.startsWith("diff --git") || - line.startsWith("index ") || - line.startsWith("--- ") || - line.startsWith("+++ ") || - line === "---" || - line === "+++" - ) { - continue; - } - // "\ No newline at end of file" is metadata, not a line of the file. - if (line.startsWith("\\")) continue; if (line.startsWith("@@")) { + inHunk = true; out.push({ kind: "gap", text: line }); continue; } + if (!inHunk) { + if ( + line.startsWith("diff --git") || + line.startsWith("index ") || + line.startsWith("--- ") || + line.startsWith("+++ ") || + line === "---" || + line === "+++" + ) { + continue; + } + } + // "\ No newline at end of file" is metadata, not a line of the file: a + // real line starting with a backslash carries a +/-/space prefix first. + if (line.startsWith("\\")) continue; if (line.startsWith("+")) { out.push({ kind: "add", text: line.slice(1) }); continue; @@ -443,14 +466,19 @@ function readPatchChanges(input: Record): TracePatchFile[] | nu } /** First path plus a count, for the collapsed one-line summary. */ -function readPatchSummary(input: Record | undefined): string { +function readPatchSummary( + input: Record | undefined, + labels?: TraceSummaryLabels, +): string { if (!input) return ""; const files = readPatchChanges(input); if (files === null) return ""; const first = files[0]; if (first === undefined) return ""; const head = shortenTracePath(first.path); - return files.length > 1 ? `${head} +${files.length - 1} more` : head; + if (files.length === 1) return head; + const extra = files.length - 1; + return labels?.morePaths?.(head, extra) ?? `${head} +${extra} more`; } function readFileMutation(input: Record): FileMutation | null { diff --git a/packages/views/locales/en/agents.json b/packages/views/locales/en/agents.json index 45866d1aff..564b3b9b31 100644 --- a/packages/views/locales/en/agents.json +++ b/packages/views/locales/en/agents.json @@ -792,6 +792,7 @@ "patch_truncated": "... (truncated)", "patch_body_truncated": "... (body truncated)", "patch_no_content": "(no content reported)", + "patch_summary_more": "{{path}} +{{count}} more", "status_queued": "Queued", "status_dispatched": "Dispatched", "status_cancelled": "Cancelled", diff --git a/packages/views/locales/ja/agents.json b/packages/views/locales/ja/agents.json index 335037d239..90a4dd9887 100644 --- a/packages/views/locales/ja/agents.json +++ b/packages/views/locales/ja/agents.json @@ -667,9 +667,10 @@ "density_collapsed_desc": "1 行 1 イベントの最もコンパクトな表示", "no_output": "出力なし", "show_all": "すべて表示", - "patch_truncated": "…（切り詰め済み）", - "patch_body_truncated": "…（本文を切り詰めました）", + "patch_truncated": "...（切り詰め済み）", + "patch_body_truncated": "...（本文を切り詰めました）", "patch_no_content": "（内容の記録なし）", + "patch_summary_more": "{{path}} 他 {{count}} 件", "status_queued": "待機列", "status_dispatched": "ディスパッチ済み", "status_cancelled": "キャンセル", diff --git a/packages/views/locales/ko/agents.json b/packages/views/locales/ko/agents.json index 34938af686..e8847bae77 100644 --- a/packages/views/locales/ko/agents.json +++ b/packages/views/locales/ko/agents.json @@ -675,9 +675,10 @@ "density_collapsed_desc": "이벤트당 한 줄, 가장 간결한 보기", "no_output": "출력 없음", "show_all": "모두 보기", - "patch_truncated": "…(잘림)", - "patch_body_truncated": "…(본문 잘림)", + "patch_truncated": "...(잘림)", + "patch_body_truncated": "...(본문 잘림)", "patch_no_content": "(기록된 내용 없음)", + "patch_summary_more": "{{path}} 외 {{count}}개", "status_queued": "대기열", "status_dispatched": "디스패치됨", "status_cancelled": "취소됨", diff --git a/packages/views/locales/zh-Hans/agents.json b/packages/views/locales/zh-Hans/agents.json index e05d65afba..9d898ac225 100644 --- a/packages/views/locales/zh-Hans/agents.json +++ b/packages/views/locales/zh-Hans/agents.json @@ -774,9 +774,10 @@ "density_collapsed_desc": "一行一事件，最紧凑的扫读视图", "no_output": "无输出", "show_all": "显示全部", - "patch_truncated": "……（已截断）", - "patch_body_truncated": "……（内容已截断）", + "patch_truncated": "...（已截断）", + "patch_body_truncated": "...（内容已截断）", "patch_no_content": "（未记录内容）", + "patch_summary_more": "{{path}} 等 {{count}} 个文件", "status_queued": "排队中", "status_dispatched": "已派发", "status_cancelled": "已取消",