mirror of
https://github.com/multica-ai/multica.git
synced 2026-08-03 19:20:07 +02:00
fix(transcript): stop treating header-like content lines as file headers
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 <github@multica.ai>
This commit is contained in:
@@ -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<TraceSummaryLabels>(
|
||||
() => ({
|
||||
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],
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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<string, unknown> | undefined): string {
|
||||
export function traceToolArgSummary(
|
||||
input: Record<string, unknown> | 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<string, unknown>): TracePatchFile[] | nu
|
||||
}
|
||||
|
||||
/** First path plus a count, for the collapsed one-line summary. */
|
||||
function readPatchSummary(input: Record<string, unknown> | undefined): string {
|
||||
function readPatchSummary(
|
||||
input: Record<string, unknown> | 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<string, unknown>): FileMutation | null {
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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": "キャンセル",
|
||||
|
||||
@@ -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": "취소됨",
|
||||
|
||||
@@ -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": "已取消",
|
||||
|
||||
Reference in New Issue
Block a user