From e98cd522eef808f62f67dd21f656c523b654af69 Mon Sep 17 00:00:00 2001 From: kingwl Date: Fri, 31 Jul 2026 11:41:16 +0800 Subject: [PATCH] fix TUI diff context line accounting --- ...tui-diff-context-line-accounting.i18n.yaml | 6 ++ ...-07-31-tui-diff-context-line-accounting.md | 29 ++++++++++ ...-31-tui-diff-context-line-accounting.zh.md | 29 ++++++++++ packages/ui/tui/README.i18n.yaml | 4 +- packages/ui/tui/README.md | 2 +- packages/ui/tui/README.zh.md | 2 +- packages/ui/tui/package.json | 1 + packages/ui/tui/src/components/transcript.ts | 49 +++++++++++++--- .../advanced-cards-collapsed.expected.txt | 4 +- .../advanced-cards-expanded.expected.txt | 56 +++++++++---------- packages/ui/tui/tests/tui.spec.ts | 17 ++++-- pnpm-lock.yaml | 3 + 12 files changed, 153 insertions(+), 49 deletions(-) create mode 100644 .agents/notes/implemented/bug-fix/2026-07-31-tui-diff-context-line-accounting.i18n.yaml create mode 100644 .agents/notes/implemented/bug-fix/2026-07-31-tui-diff-context-line-accounting.md create mode 100644 .agents/notes/implemented/bug-fix/2026-07-31-tui-diff-context-line-accounting.zh.md diff --git a/.agents/notes/implemented/bug-fix/2026-07-31-tui-diff-context-line-accounting.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-07-31-tui-diff-context-line-accounting.i18n.yaml new file mode 100644 index 0000000000..2d43863567 --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-07-31-tui-diff-context-line-accounting.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# side as of the last confirmed-consistent state. Both languages carry equal authority; +# after editing either side, bring the other along and re-record with: +# pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-07-31-tui-diff-context-line-accounting.md +2026-07-31-tui-diff-context-line-accounting.md: 71593022b56d9e675025f3a7a6d1e5e3edfa9b57 +2026-07-31-tui-diff-context-line-accounting.zh.md: df374c23b73fc5667cf733b4916d9d0d2ecb9198 diff --git a/.agents/notes/implemented/bug-fix/2026-07-31-tui-diff-context-line-accounting.md b/.agents/notes/implemented/bug-fix/2026-07-31-tui-diff-context-line-accounting.md new file mode 100644 index 0000000000..71593022b5 --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-07-31-tui-diff-context-line-accounting.md @@ -0,0 +1,29 @@ +# Agent Note: TUI diff context lines stay neutral + +Status: implemented + +English | [中文](2026-07-31-tui-diff-context-line-accounting.zh.md) + +## Problem + +Result-time filesystem diffs carry the applied change with three surrounding context lines in each `FileDiff.oldText` and `FileDiff.newText`. The TUI rendered every old-side row as removed and every new-side row as added, including the identical context present on both sides. A one-line edit therefore appeared as seven removals plus seven additions, and the footer repeated those inflated totals. + +## Decision + +The TUI compares each non-create `FileDiff.oldText` and `FileDiff.newText` at render time. Added and removed rows retain their green `+` and red `-` markers; equal context rows use the recessed body tone with a neutral two-space prefix. The footer sums only the rows classified as added or removed. A create (`oldText: null`) continues to classify every non-empty new-content row as added. + +This remains a consumer-side interpretation of the existing `FileDiff` contract. Filesystem tools continue to persist contextual before/after snippets, so other consumers keep their placement context and existing session logs replay with corrected TUI presentation. The TUI uses the same maintained `diff` package as `dsh-tool-fs` instead of introducing a second line-diff implementation. + +## Alternatives considered + +**Remove context from filesystem result metadata.** Rejected: contextual applied hunks are intentional producer output used by capable editors, and changing them would weaken every consumer while leaving old session logs misleading in the TUI. + +**Extend `FileDiff` with persisted per-line tags.** Rejected: the tags can be derived deterministically from the existing before/after pair; persisting them would widen the cross-package and session-log contract solely for one renderer. + +**Match equal lines by position without a diff algorithm.** Rejected: insertions and deletions shift subsequent context, so positional pairing would misclassify valid hunks. + +## Consequences + +TUI diff cards distinguish evidence-bearing context from the mutation itself, and their `+A -R` footer reports the actual line delta. Replaying an existing contextual diff gains the corrected rendering without a migration. Rendering performs one additional line comparison per non-create hunk; result-time hunks are already context-bounded, while create cards bypass the comparison. + +The focused TUI test covers neutral context and exact totals. The assembled `advanced-cards` terminal snapshots pin the neutral context style, semantic change colors, and `+1 -1` footer through collapsed and expanded card states. diff --git a/.agents/notes/implemented/bug-fix/2026-07-31-tui-diff-context-line-accounting.zh.md b/.agents/notes/implemented/bug-fix/2026-07-31-tui-diff-context-line-accounting.zh.md new file mode 100644 index 0000000000..df374c23b7 --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-07-31-tui-diff-context-line-accounting.zh.md @@ -0,0 +1,29 @@ +# Agent Note: TUI diff 上下文行保持中性 + +Status: implemented + +[English](2026-07-31-tui-diff-context-line-accounting.md) | 中文 + +## 问题 + +文件系统 diff 返回结果时，每个 `FileDiff.oldText` 和 `FileDiff.newText` 都会包含已应用的变更及其前后各 3 行上下文。TUI 将旧侧的每一行都渲染为删除行，将新侧的每一行都渲染为新增行，其中包括两侧相同的上下文。因此，一行编辑会显示为删除 7 行并新增 7 行，页脚还会重复这些虚高的合计值。 + +## 决策 + +对于每个不对应文件创建的 `FileDiff`，TUI 在渲染时比较 `FileDiff.oldText` 和 `FileDiff.newText`。新增行和删除行仍分别使用绿色 `+` 和红色 `-` 标记；相同的上下文行则使用弱化的正文色调，并带有由两个空格构成的中性前缀。页脚只汇总归类为新增或删除的行。创建操作（`oldText: null`）仍将新内容中的每个非空行归类为新增行。 + +该行为仍然只是消费方对现有 `FileDiff` 契约的解释。文件系统工具仍会持久化带上下文的变更前后片段，因此其他消费方仍能获得定位上下文，已有会话日志在回放时也会采用修正后的 TUI 呈现。TUI 与 `dsh-tool-fs` 共用同一个受维护的 `diff` 包（package），无需引入第二套逐行 diff 实现。 + +## 考虑过的替代方案 + +**从文件系统结果元数据中移除上下文。** 不予采纳：带上下文的已应用 hunk 是有意保留的生产方输出，供具备相应能力的编辑器使用；更改这些内容会让所有消费方丢失信息，同时旧会话日志在 TUI 中仍会产生误导。 + +**为 `FileDiff` 扩展持久化的逐行标签。** 不予采纳：这些标签可以根据现有的变更前后文本对确定性派生；仅为一个渲染器持久化标签，会扩大跨包契约和会话日志契约。 + +**不使用 diff 算法，按位置匹配相同行。** 不予采纳：插入和删除会使后续上下文发生位移，因此按位置配对会把有效 hunk 错误分类。 + +## 后果 + +TUI diff 卡片会区分用于佐证的上下文与变更本身，其 `+A -R` 页脚报告实际的行变更量。回放已有的上下文 diff 无需迁移即可获得修正后的渲染。渲染每个不对应文件创建的 hunk 时，会额外执行一次逐行比较；结果时刻的 hunk 本就受上下文范围限制，创建卡片则会跳过比较。 + +聚焦的 TUI 测试覆盖中性上下文和精确合计值。组装后的 `advanced-cards` 终端快照在卡片折叠和展开状态下固定了中性上下文样式、变更行的语义色彩，以及 `+1 -1` 页脚。 diff --git a/packages/ui/tui/README.i18n.yaml b/packages/ui/tui/README.i18n.yaml index e94be1a857..92bfb18e29 100644 --- a/packages/ui/tui/README.i18n.yaml +++ b/packages/ui/tui/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/ui/tui/README.md -README.md: 63c888b1d51c02fa85a8f0cc1617874debd87c4e -README.zh.md: ca5efc9ae26a9833d271991f73a21c607d8fb09d +README.md: b021789d660fd831c3fa0dad20d0bc174538eb57 +README.zh.md: b9cd7210932558a3a2feb0d5c1bfaf7e115703f6 diff --git a/packages/ui/tui/README.md b/packages/ui/tui/README.md index 63c888b1d5..b021789d66 100644 --- a/packages/ui/tui/README.md +++ b/packages/ui/tui/README.md @@ -83,7 +83,7 @@ Every SGR code the TUI emits lives in one table, `paletteSpec` in `components/th There is one role per visual meaning: `dim` is the single recessed tone and `accent` the single emphasis color, while `success` and `error` double as a diff's added and removed lines. Colors and attributes are separately typed, so `bold(accent(x))` compiles and `accent(error(x))` does not — SGR has no color stack, so nesting one color inside another silently drops the outer color at the inner one's close. Attributes occupy independent SGR groups and compose with any color in either order. Run `/palette` to see every role as your terminal renders it, with its SGR pair. -Grouped regions (user prompts, assistant replies, tool cards) are separated by a bold, underlined role header in the role color and blank-line spacing rather than a filled block or a per-line prefix, so a mouse drag-select copies the message text without any leading bar or indent; a tool card's status (pending, error, success) shows in its colored, underlined title glyph and title. Inside a tool card, the whole body — presenter title, a terminal `$` command and cwd, and the tool's own output — renders in one dim tone, so only the status-colored header carries color and the body reads as one recessed block instead of a run of competing shades; an injected-context card's prose is the same tone as its header. A diff card's `+`/`-` lines and a `[signal …]` marker stay colored, because there the color is the meaning rather than emphasis. The question panel emphasizes its active row with bold accent text, while selectors use reverse video. These treatments are foreground-only, so they never collide with the terminal background. Set `color: false` to strip all styling. +Grouped regions (user prompts, assistant replies, tool cards) are separated by a bold, underlined role header in the role color and blank-line spacing rather than a filled block or a per-line prefix, so a mouse drag-select copies the message text without any leading bar or indent; a tool card's status (pending, error, success) shows in its colored, underlined title glyph and title. Inside a tool card, the whole body — presenter title, a terminal `$` command and cwd, and the tool's own output — renders in one dim tone, so only the status-colored header carries color and the body reads as one recessed block instead of a run of competing shades; an injected-context card's prose is the same tone as its header. A diff card colors and counts only added `+` and removed `-` lines; unchanged context stays dim and uncounted. A `[signal …]` marker remains colored because there the color is the meaning rather than emphasis. The question panel emphasizes its active row with bold accent text, while selectors use reverse video. These treatments are foreground-only, so they never collide with the terminal background. Set `color: false` to strip all styling. ## Model Experience diff --git a/packages/ui/tui/README.zh.md b/packages/ui/tui/README.zh.md index ca5efc9ae2..b9cd721093 100644 --- a/packages/ui/tui/README.zh.md +++ b/packages/ui/tui/README.zh.md @@ -83,7 +83,7 @@ TUI 发出的所有 SGR 代码都集中在一个表中，即 `components/theme.t 每种视觉语义只对应一个角色：`dim` 是唯一的弱化色调，`accent` 是唯一的强调色，`success` 和 `error` 还分别充当 diff 的新增行与删除行。颜色和属性分属不同类型，因此 `bold(accent(x))` 可以通过编译，`accent(error(x))` 则不行——SGR 没有颜色栈；在一种颜色内嵌套另一种颜色时，内层颜色闭合时会静默丢弃外层颜色。各属性占用彼此独立的 SGR 组，可以按任一顺序与任何颜色组合。运行 `/palette` 可查看每个角色在你的终端上的实际渲染效果及其 SGR 码对。 -成组区域（用户提示词、assistant 回复、工具卡片）通过以角色色渲染的粗体带下划线角色标题和空行分隔，而非填充背景块或逐行前缀，因此用鼠标框选复制时不会带上任何左侧竖条或缩进；工具卡片的状态（进行中、错误、成功）由其彩色带下划线的标题字形与标题体现。在工具卡片内部，整个正文——presenter 标题、终端 `$` 命令与 cwd，以及工具自身的输出——统一以同一种暗色渲染，因此只有带状态色的表头携带颜色，正文读作一个整体弱化的区块，而不是一串互相竞争的色调；注入上下文卡片的正文与其表头也是同一种色调。diff 卡片的 `+`/`-` 行与 `[signal …]` 标记保留颜色，因为那里的颜色本身就是语义，而非强调。问题面板使用粗体强调色文本突出活跃行，选择器则使用反色。所有效果都只作用于前景色，因此不会与终端背景冲突。设置 `color: false` 可移除所有样式。 +成组区域（用户提示词、assistant 回复、工具卡片）通过以角色色渲染的粗体带下划线角色标题和空行分隔，而非填充背景块或逐行前缀，因此用鼠标框选复制时不会带上任何左侧竖条或缩进；工具卡片的状态（进行中、错误、成功）由其彩色带下划线的标题字形与标题体现。在工具卡片内部，整个正文——presenter 标题、终端 `$` 命令与 cwd，以及工具自身的输出——统一以同一种暗色渲染，因此只有带状态色的表头携带颜色，正文读作一个整体弱化的区块，而不是一串互相竞争的色调；注入上下文卡片的正文与其表头也是同一种色调。diff 卡片只为新增的 `+` 行和删除的 `-` 行着色并计数；未变更的上下文保持暗色且不纳入计数。`[signal …]` 标记仍保留颜色，因为那里的颜色本身就是语义，而非强调。问题面板使用粗体强调色文本突出活跃行，选择器则使用反色。所有效果都只作用于前景色，因此不会与终端背景冲突。设置 `color: false` 可移除所有样式。 ## 模型体验 diff --git a/packages/ui/tui/package.json b/packages/ui/tui/package.json index c3506ea338..a68fe7968a 100644 --- a/packages/ui/tui/package.json +++ b/packages/ui/tui/package.json @@ -66,6 +66,7 @@ }, "dependencies": { "@earendil-works/pi-tui": "0.80.7", + "diff": "^9.0.0", "saxes": "6.0.0", "schemastery": "^3.18.0" }, diff --git a/packages/ui/tui/src/components/transcript.ts b/packages/ui/tui/src/components/transcript.ts index 774e982f81..5c8b9bf749 100644 --- a/packages/ui/tui/src/components/transcript.ts +++ b/packages/ui/tui/src/components/transcript.ts @@ -15,6 +15,7 @@ import { type Component, type MarkdownTheme, } from '@earendil-works/pi-tui' +import { diffLines as compareLines } from 'diff' import type { Agent } from '@deepseek-ai/dsh-agent' import type { ContentBlock, StreamChunk } from '@deepseek-ai/dsh-llm' import type { JsonValue, SessionEvent, TodoItem } from '@deepseek-ai/dsh-session' @@ -52,16 +53,45 @@ function pretty(value: unknown): string { return displayText(serialized ?? String(value)) } -/** A file diff as colored `+`/`-` lines, optionally prefixed with its path. */ -function diffLines(diff: FileDiff, palette: Palette): string[] { +interface RenderedDiff { + lines: string[] + added: number + removed: number +} + +/** Split one diff change into display rows without counting its trailing line terminator. */ +function diffValueLines(value: string): string[] { + if (value === '') return [] + const safe = displayText(value) + return (safe.endsWith('\n') ? safe.slice(0, -1) : safe).split('\n') +} + +/** A file diff whose unchanged context stays neutral and does not affect change totals. */ +function renderDiff(diff: FileDiff, palette: Palette): RenderedDiff { // The card header is a fixed `Tool / ` frame that never names a file, so // each hunk always carries its own path header (no redundancy to suppress). const lines = [palette.bold(displayText(diff.path))] - if (diff.oldText !== null) { - for (const line of displayText(diff.oldText).split('\n')) lines.push(palette.error(`- ${line}`)) + let added = 0 + let removed = 0 + if (diff.oldText === null) { + const newLines = diffValueLines(diff.newText) + added = newLines.length + for (const line of newLines) lines.push(palette.success(`+ ${line}`)) + return { lines, added, removed } } - for (const line of displayText(diff.newText).split('\n')) lines.push(palette.success(`+ ${line}`)) - return lines + for (const change of compareLines(diff.oldText, diff.newText)) { + const changedLines = diffValueLines(change.value) + if (change.added) { + added += changedLines.length + for (const line of changedLines) lines.push(palette.success(`+ ${line}`)) + } else if (change.removed) { + removed += changedLines.length + for (const line of changedLines) lines.push(palette.error(`- ${line}`)) + } else { + for (const line of changedLines) lines.push(palette.dim(` ${line}`)) + } + } + return { lines, added, removed } } /** @@ -505,9 +535,10 @@ export class ToolCardComponent implements Component { let added = 0 let removed = 0 const hunks = view.diffs.flatMap((diff, index) => { - if (diff.oldText !== null) removed += displayText(diff.oldText).split('\n').length - added += displayText(diff.newText).split('\n').length - return [...index > 0 ? [''] : [], ...diffLines(diff, this.palette)] + const rendered = renderDiff(diff, this.palette) + added += rendered.added + removed += rendered.removed + return [...index > 0 ? [''] : [], ...rendered.lines] }) const files = view.diffs.length const footer = this.palette.dim(`└ +${added} -${removed} · ${files} file${files === 1 ? '' : 's'}`) diff --git a/packages/ui/tui/tests/snapshots/advanced-cards-collapsed.expected.txt b/packages/ui/tui/tests/snapshots/advanced-cards-collapsed.expected.txt index 2a005383e1..62f69c641f 100644 --- a/packages/ui/tui/tests/snapshots/advanced-cards-collapsed.expected.txt +++ b/packages/ui/tui/tests/snapshots/advanced-cards-collapsed.expected.txt @@ -31,9 +31,9 @@ buffer style 0-10 bold 14| "- old line " style 0-9 fg=red -15| "… +3 lines (Ctrl+O to expand) " +15| "… +2 lines (Ctrl+O to expand) " style 0-28 dim -16| "└ +2 -2 · 1 file " +16| "└ +1 -1 · 1 file " style 0-15 dim 17| 18| "● Tool / subagent" diff --git a/packages/ui/tui/tests/snapshots/advanced-cards-expanded.expected.txt b/packages/ui/tui/tests/snapshots/advanced-cards-expanded.expected.txt index 6f9aa094f3..55479a6f34 100644 --- a/packages/ui/tui/tests/snapshots/advanced-cards-expanded.expected.txt +++ b/packages/ui/tui/tests/snapshots/advanced-cards-expanded.expected.txt @@ -1,7 +1,7 @@ -terminal 100x40 buffer=normal length=43 base=3 viewport=3 +terminal 100x40 buffer=normal length=42 base=2 viewport=2 lifecycle started=1 stopped=0 progress=inactive title "DSH snapshot" -cursor hidden column=7 viewportRow=39 bufferRow=42 +cursor hidden column=7 viewportRow=39 bufferRow=41 buffer 0| " DEEPSEEK HARNESS" style 1-8 fg=bright-magenta bold @@ -37,54 +37,52 @@ buffer style 0-10 bold 17| "- old line " style 0-9 fg=red -18| "- keep " - style 0-5 fg=red -19| "+ new line " +18| "+ new line " style 0-9 fg=green -20| "+ keep " - style 0-5 fg=green -21| "└ +2 -2 · 1 file " +19| " keep " + style 0-5 dim +20| "└ +1 -1 · 1 file " style 0-15 dim -22| -23| "● Tool / subagent" +21| +22| "● Tool / subagent" style 0-16 fg=green -24| "Delegate renderer audit " +23| "Delegate renderer audit " style 0-99 dim -25| "The renderer has explicit lifecycle ownership. " +24| "The renderer has explicit lifecycle ownership. " style 0-99 dim -26| -27| "● Tool / task_output" +25| +26| "● Tool / task_output" style 0-19 fg=green -28| "Read output from background task subagent-7 " +27| "Read output from background task subagent-7 " style 0-99 dim -29| " " -30| "console " +28| " " +29| "console " style 0-6 dim -31| " started background task bash-5 " +30| " started background task bash-5 " style 0-1 dim style 2-31 fg=cyan dim style 32-99 dim -32| " " -33| -34| "● Tool / skill" +31| " " +32| +33| "● Tool / skill" style 0-13 fg=green -35| "Load skill dsh-code-review " +34| "Load skill dsh-code-review " style 0-99 dim -36| "Loaded review instructions. " +35| "Loaded review instructions. " style 0-99 dim -37| "Model wait 0.0s " +36| "Model wait 0.0s " style 0-14 dim -38| -39| "Tool and context cards expanded. " +37| +38| "Tool and context cards expanded. " style 0-31 dim -40| -41| "/workspace/project (tui-staging) deepseek-v4-flash ↑0 ↓0 0% context" +39| +40| "/workspace/project (tui-staging) deepseek-v4-flash ↑0 ↓0 0% context" style 0-17 fg=bright-magenta bold style 18-31 dim style 34-50 dim style 53-57 dim style 60-69 dim -42| " dsh > " +41| " dsh > " style 1-3 fg=bright-magenta bold style 5-6 dim style 7-7 inverse diff --git a/packages/ui/tui/tests/tui.spec.ts b/packages/ui/tui/tests/tui.spec.ts index 79aa6f4f97..1b28fa65c5 100644 --- a/packages/ui/tui/tests/tui.spec.ts +++ b/packages/ui/tui/tests/tui.spec.ts @@ -4315,7 +4315,11 @@ describe('tool cards and surface replay', () => { presentCall: () => ({ card: 'diff', title: 'Edit src/only.ts', - diffs: [{ path: 'src/only.ts', oldText: 'old', newText: 'new' }], + diffs: [{ + path: 'src/only.ts', + oldText: 'my: my-MM\nne: ne-NP\nnl: nl-NL\nnb: no-NO\npa: pa-Guru-IN\npl: pl-PL\npt_pt: pt-PT', + newText: 'my: my-MM\nne: ne-NP\nnl: nl-NL\nnb: nb-NO\npa: pa-Guru-IN\npl: pl-PL\npt_pt: pt-PT', + }], }), }, generic: { @@ -4622,7 +4626,7 @@ describe('tool cards and surface replay', () => { }) it('names a single-file diff in the body once, under a fixed Tool header', async () => { - const result = await setup({ tools }) + const result = await setup({ tools, config: { maxToolOutputLines: 20 } }) appendUser(result.session, 'edit one file') appendAssistant(result.session, [ { type: 'text', text: 'Editing' }, @@ -4638,9 +4642,12 @@ describe('tool cards and surface replay', () => { expect(output).toContain('Tool / singleDiff') expect(output).not.toContain('Edit src/only.ts') expect(output.split('src/only.ts').length - 1).toBe(1) - expect(output).toContain('- old') - expect(output).toContain('+ new') - expect(output).toContain('· 1 file') + expect(output).toContain(' my: my-MM') + expect(output).not.toContain('- my: my-MM') + expect(output).not.toContain('+ my: my-MM') + expect(output).toContain('- nb: no-NO') + expect(output).toContain('+ nb: nb-NO') + expect(output).toContain('└ +1 -1 · 1 file') await dispose(result) }) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5ca1ec67f6..300b2731a7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -5458,6 +5458,9 @@ importers: '@earendil-works/pi-tui': specifier: 0.80.7 version: 0.80.7(patch_hash=6c30c5386c0159131e1361023cddf31377f5728962524841964373312c1ed946) + diff: + specifier: ^9.0.0 + version: 9.0.0 saxes: specifier: 6.0.0 version: 6.0.0