diff --git a/eslint.config.cjs b/eslint.config.cjs index 6ed968b4..27ba6a75 100644 --- a/eslint.config.cjs +++ b/eslint.config.cjs @@ -47,6 +47,7 @@ module.exports = require('eslint-config-sukka').sukka( // Workspace tests belong to their source tsconfig or a tests/tsconfig.json referenced by // the root solution; listing them here too would breach typescript-eslint's 8-file cap. 'vitest.config.ts', + 'vitest.setup.ts', ], }, }, diff --git a/packages/client/workbench/src/mock/data/showcase.ts b/packages/client/workbench/src/mock/data/showcase.ts index a598b048..2594af43 100644 --- a/packages/client/workbench/src/mock/data/showcase.ts +++ b/packages/client/workbench/src/mock/data/showcase.ts @@ -435,6 +435,51 @@ export function createShowcaseToolBursts(terminalId = SHOWCASE_TERMINAL_ID): Sho }, ], }, + { + // The gutter/context case: a real multi-hunk patch, so the card shows absolute line + // numbers, surrounding rows, and a `@@` separator between hunks. Every other diff fixture + // here is a 1-3 line oldText/newText pair, which exercises none of that. + toolCallId: 'mock-tool-edit-multi-hunk', + title: 'Rewire the conversation pipeline', + kind: 'edit', + status: 'completed', + content: [ + { + type: 'diff', + change: 'modify', + path: 'packages/client/core/src/conversation.ts', + patch: { + format: 'git_patch', + text: [ + '@@ -12,7 +12,8 @@', + ' ', + ' const MAX_ITEMS = 500;', + ' ', + '-export function foldTurn(items: ConversationItem[]): Turn {', + '+/** Folds one turn, dropping items past the cap. */', + '+export function foldTurn(items: readonly ConversationItem[]): Turn {', + ' const turn = createTurn();', + ' for (const item of items) {', + ' turn.push(item);', + '@@ -84,9 +85,11 @@', + ' return turn;', + ' }', + ' ', + '-function createTurn(): Turn {', + '- return { items: [], tokens: 0 };', + '+function createTurn(seed?: Partial): Turn {', + '+ return { items: [], tokens: 0, ...seed };', + ' }', + ' ', + ' export function isBoundary(item: ConversationItem): boolean {', + '- return item.kind === "user";', + '+ return item.kind === "user" || item.kind === "compaction";', + ' }', + ].join('\n'), + }, + }, + ], + }, { toolCallId: 'mock-tool-write-plan', title: 'Write PLAN.md', diff --git a/packages/foundation/schema/src/model/__tests__/tool-call.test.ts b/packages/foundation/schema/src/model/__tests__/tool-call.test.ts index 103bb8dc..cbd395c8 100644 --- a/packages/foundation/schema/src/model/__tests__/tool-call.test.ts +++ b/packages/foundation/schema/src/model/__tests__/tool-call.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { ToolCallContentSchema } from '../tool-call'; +import { ToolCallContentSchema, unifiedPatchText } from '../tool-call'; describe('ToolCallContentSchema diff', () => { it('keeps legacy oldText/newText diffs readable', () => { @@ -25,3 +25,18 @@ describe('ToolCallContentSchema diff', () => { expect(ToolCallContentSchema.safeParse({ type: 'diff', ...diff }).success).toBe(true); }); }); + +describe('unifiedPatchText', () => { + it('emits a header per hunk followed by that hunk body', () => { + expect( + unifiedPatchText([ + { oldStart: 1, oldLines: 2, newStart: 1, newLines: 2, lines: [' same', '-before'] }, + { oldStart: 40, oldLines: 1, newStart: 40, newLines: 2, lines: ['+added', ' tail'] }, + ]), + ).toBe('@@ -1,2 +1,2 @@\n same\n-before\n@@ -40,1 +40,2 @@\n+added\n tail'); + }); + + it('renders no hunks as empty text', () => { + expect(unifiedPatchText([])).toBe(''); + }); +}); diff --git a/packages/foundation/schema/src/model/tool-call.ts b/packages/foundation/schema/src/model/tool-call.ts index 8e0d99d1..fb6c6a9e 100644 --- a/packages/foundation/schema/src/model/tool-call.ts +++ b/packages/foundation/schema/src/model/tool-call.ts @@ -38,6 +38,28 @@ export const ToolDiffPatchSchema = z.object({ }); export type ToolDiffPatch = z.infer; +/** One `@@` group of a unified diff. Structurally what both jsdiff's `structuredPatch` and the + * claude SDK's `structuredPatch` hand back, so one formatter serves both. */ +export interface UnifiedDiffHunk { + oldStart: number; + oldLines: number; + newStart: number; + newLines: number; + /** Body lines, each already carrying its ` ` / `-` / `+` prefix. */ + lines: readonly string[]; +} + +/** Hunks → `ToolDiffPatch.text`. Only the text crosses the wire, so the hunk shape stays a plain + * interface rather than a schema. */ +export function unifiedPatchText(hunks: readonly UnifiedDiffHunk[]): string { + return hunks + .flatMap((hunk) => [ + `@@ -${hunk.oldStart},${hunk.oldLines} +${hunk.newStart},${hunk.newLines} @@`, + ...hunk.lines, + ]) + .join('\n'); +} + /** Tool-call output content: a wrapped ContentBlock, a file diff, or a live terminal reference. */ export const ToolCallContentSchema = z.discriminatedUnion('type', [ z.object({ type: z.literal('content'), content: ContentBlockSchema }), diff --git a/packages/host/agent-adapter/src/__tests__/claude-code-compaction.test.ts b/packages/host/agent-adapter/src/__tests__/claude-code-compaction.test.ts index d4be2f43..716cb0bd 100644 --- a/packages/host/agent-adapter/src/__tests__/claude-code-compaction.test.ts +++ b/packages/host/agent-adapter/src/__tests__/claude-code-compaction.test.ts @@ -235,8 +235,56 @@ describe('buildClaudeTranscriptSupplement', () => { // Error results carry a plain string — nothing to project. resultRow('r3', ['toolu_4'], 'String to replace not found'), ]); - expect([...supplement.toolUseResults.entries()]).toEqual([ - ['toolu_1', { code: 200, codeText: 'OK' }], + expect([...supplement.toolUseResults]).toEqual([['toolu_1', { code: 200, codeText: 'OK' }]]); + + // An Edit result keys both projections off the one raw field: the envelope keeps the small + // scalars (dropping the bulk `originalFile`), and the patch survives the same filter, which + // would otherwise drop `structuredPatch` for being an array. + const edits = buildClaudeTranscriptSupplement([ + resultRow('r1', ['toolu_1'], { + filePath: 'src/a.ts', + oldString: 'a', + newString: 'b', + originalFile: 'x'.repeat(400), + userModified: false, + replaceAll: true, + structuredPatch: [ + { oldStart: 12, oldLines: 3, newStart: 12, newLines: 3, lines: [' ctx', '-a', '+b'] }, + ], + }), + resultRow('r2', ['toolu_2', 'toolu_3'], { + filePath: 'src/b.ts', + oldString: 'a', + newString: 'b', + structuredPatch: [{ oldStart: 1, oldLines: 1, newStart: 1, newLines: 1, lines: ['-a'] }], + }), + ]); + expect([...edits.toolUseResults]).toEqual([ + [ + 'toolu_1', + { + filePath: 'src/a.ts', + oldString: 'a', + newString: 'b', + userModified: false, + replaceAll: true, + }, + ], + ]); + expect([...edits.toolUsePatches]).toEqual([ + [ + 'toolu_1', + [ + { + type: 'diff', + change: 'modify', + path: 'src/a.ts', + oldText: 'a', + newText: 'b', + patch: { format: 'git_patch', text: '@@ -12,3 +12,3 @@\n ctx\n-a\n+b' }, + }, + ], + ], ]); }); }); @@ -256,7 +304,13 @@ describe('ClaudeCodeAdapter readHistory transcript supplement', () => { } function supplementOf(partial: Partial): ClaudeTranscriptSupplement { - return { records: new Map(), droppedRows: [], toolUseResults: new Map(), ...partial }; + return { + records: new Map(), + droppedRows: [], + toolUseResults: new Map(), + toolUsePatches: new Map(), + ...partial, + }; } class HistoryClaude extends ClaudeCodeAdapter { @@ -376,6 +430,41 @@ describe('ClaudeCodeAdapter readHistory transcript supplement', () => { } }); + it('replays an Edit settle with the recovered patch instead of the announce fragment', async () => { + const patch = { + type: 'diff' as const, + change: 'modify' as const, + path: 'src/a.ts', + oldText: 'a', + newText: 'b', + patch: { format: 'git_patch' as const, text: '@@ -12,3 +12,3 @@\n ctx\n-a\n+b' }, + }; + const adapter = new HistoryClaude( + [ + row('assistant', 'a0', [ + { + type: 'tool_use', + id: 'toolu_1', + name: 'Edit', + input: { file_path: 'src/a.ts', old_string: 'a', new_string: 'b' }, + }, + ]), + row('user', 'u0', [{ type: 'tool_result', tool_use_id: 'toolu_1', content: 'updated' }]), + ], + supplementOf({ toolUsePatches: new Map([['toolu_1', [patch]]]) }), + ); + const result = await adapter.readHistory({ historyId: asHistoryId(SESSION) }); + const settle = result.events.at(-1)?.event; + expect(settle?.type).toBe('tool-call'); + if (settle?.type === 'tool-call') { + // Superseded, not stacked — one diff, matching what the live settle emits. + expect(settle.toolCall.content).toEqual([ + patch, + { type: 'content', content: { type: 'text', text: 'updated' } }, + ]); + } + }); + it('reads the supplement on every page but splices dropped rows only into the first', async () => { const adapter = new HistoryClaude( [row('user', 'u0', [{ type: 'text', text: 'hello' }])], diff --git a/packages/host/agent-adapter/src/__tests__/claude-code-subagent.test.ts b/packages/host/agent-adapter/src/__tests__/claude-code-subagent.test.ts index 21d96d03..2914aaa6 100644 --- a/packages/host/agent-adapter/src/__tests__/claude-code-subagent.test.ts +++ b/packages/host/agent-adapter/src/__tests__/claude-code-subagent.test.ts @@ -1,8 +1,9 @@ import type { SDKMessage, SessionMessage } from '@anthropic-ai/claude-agent-sdk'; import type { AgentEvent, ToolCall } from '@linkcode/schema'; +import { nullthrow } from 'foxts/guard'; import { describe, expect, it } from 'vitest'; import { asHistoryId } from '../history-util'; -import { ClaudeCodeAdapter } from '../native/claude-code'; +import { ClaudeCodeAdapter, editResultDiffContent } from '../native/claude-code'; /** * Subagent (Task tool) routing: every subagent frame carries `parent_tool_use_id`. The adapter @@ -388,4 +389,71 @@ describe('ClaudeCodeAdapter readHistory subagent splice', () => { 'agent-message', ]); }); + + it('replays a subagent Edit with the recovered patch, matching the live settle', async () => { + const EDIT_ID = 'toolu_sub_edit'; + const input = { file_path: 'src/a.ts', old_string: 'a', new_string: 'b' }; + const toolUseResult = { + filePath: 'src/a.ts', + oldString: 'a', + newString: 'b', + structuredPatch: [ + { oldStart: 12, oldLines: 3, newStart: 12, newLines: 3, lines: [' ctx', '-a', '+b'] }, + ], + }; + + // Live: the subagent's settle frame carries the structured result, so `handleUser` upgrades the + // announce fragment in place. + const live = harness(); + live.feed({ + type: 'assistant', + parent_tool_use_id: TASK_ID, + message: { content: [{ type: 'tool_use', id: EDIT_ID, name: 'Edit', input }] }, + }); + live.feed({ + type: 'user', + parent_tool_use_id: TASK_ID, + message: { content: [{ type: 'tool_result', tool_use_id: EDIT_ID, content: 'updated' }] }, + tool_use_result: toolUseResult, + }); + const liveSettle = live + .tools() + .filter((t) => t.toolCallId === EDIT_ID) + .at(-1); + + // History: `getSubagentMessages` strips `tool_use_result`, so the patch has to come from the + // subagent's own raw transcript instead. + class SubagentPatchClaude extends HistoryClaude { + protected override readSubagentPatches(): Promise> { + return Promise.resolve( + new Map([[EDIT_ID, nullthrow(editResultDiffContent(toolUseResult))]]), + ); + } + } + const adapter = new SubagentPatchClaude( + fakeSdk({ + messages: mainMessages, + subagents: { + agent1: [ + subRow('assistant', 's0', [{ type: 'tool_use', id: EDIT_ID, name: 'Edit', input }]), + subRow('user', 's1', [ + { type: 'tool_result', tool_use_id: EDIT_ID, content: 'updated' }, + ]), + ], + }, + }), + ); + const result = await adapter.readHistory({ historyId: asHistoryId(SESSION) }); + const replayedSettle = result.events.filter((e) => e.itemId === EDIT_ID).at(-1)?.event; + + expect(replayedSettle?.type).toBe('tool-call'); + if (replayedSettle?.type !== 'tool-call') return; + // The patch supersedes the announce fragment rather than stacking beside it, and the replayed + // content matches what the live path emitted. + expect(replayedSettle.toolCall.content).toEqual(liveSettle?.content); + expect(replayedSettle.toolCall.content[0]).toMatchObject({ + type: 'diff', + patch: { format: 'git_patch', text: '@@ -12,3 +12,3 @@\n ctx\n-a\n+b' }, + }); + }); }); diff --git a/packages/host/agent-adapter/src/__tests__/normalize.test.ts b/packages/host/agent-adapter/src/__tests__/normalize.test.ts index 7c3b9cac..474d8469 100644 --- a/packages/host/agent-adapter/src/__tests__/normalize.test.ts +++ b/packages/host/agent-adapter/src/__tests__/normalize.test.ts @@ -6,6 +6,7 @@ import { asHistoryId } from '../history-util'; import { ClaudeCodeAdapter, createClaudeHistoryEventMapper, + editResultDiffContent, mapClaudeStop, toolUseResultEnvelope, } from '../native/claude-code'; @@ -393,7 +394,121 @@ describe('toolUseResultEnvelope', () => { }); }); +describe('editResultDiffContent', () => { + const structuredPatch = [ + { oldStart: 26, oldLines: 7, newStart: 26, newLines: 7, lines: [' ctx', '-a', '+b', ' tail'] }, + ]; + + it('lifts structuredPatch into a git patch beside the legacy text', () => { + expect( + editResultDiffContent({ + filePath: 'src/a.ts', + oldString: 'a', + newString: 'b', + originalFile: null, + structuredPatch, + userModified: false, + replaceAll: false, + }), + ).toEqual([ + { + type: 'diff', + change: 'modify', + path: 'src/a.ts', + oldText: 'a', + newText: 'b', + patch: { format: 'git_patch', text: '@@ -26,7 +26,7 @@\n ctx\n-a\n+b\n tail' }, + }, + ]); + }); + + it('ignores a Write result, whose create case carries no hunks', () => { + expect( + editResultDiffContent({ + type: 'create', + filePath: 'src/new.ts', + content: 'export const a = 1;\n', + structuredPatch: [], + }), + ).toBeUndefined(); + }); + + it('returns undefined for an error string, a hunkless edit, and malformed hunks', () => { + expect(editResultDiffContent('String to replace not found')).toBeUndefined(); + expect( + editResultDiffContent({ + filePath: 'a.ts', + oldString: 'a', + newString: 'b', + structuredPatch: [], + }), + ).toBeUndefined(); + expect( + editResultDiffContent({ + filePath: 'a.ts', + oldString: 'a', + newString: 'b', + structuredPatch: [{ oldStart: 'nope', oldLines: 1, newStart: 1, newLines: 1, lines: [] }], + }), + ).toBeUndefined(); + }); +}); + describe('ClaudeCodeAdapter Edit diff normalization', () => { + it('upgrades the announce fragment to the settle patch without stacking a second diff', () => { + const adapter = new TestClaude(); + const seen: AgentEvent[] = []; + adapter.onEvent((e) => seen.push(e)); + + adapter.feed({ + type: 'assistant', + message: { + content: [ + { + type: 'tool_use', + id: 't1', + name: 'Edit', + input: { file_path: 'src/a.ts', old_string: 'a', new_string: 'b' }, + }, + ], + }, + }); + adapter.feed({ + type: 'user', + message: { content: [{ type: 'tool_result', tool_use_id: 't1', content: 'updated' }] }, + tool_use_result: { + filePath: 'src/a.ts', + oldString: 'a', + newString: 'b', + structuredPatch: [ + { oldStart: 26, oldLines: 3, newStart: 26, newLines: 3, lines: [' ctx', '-a', '+b'] }, + ], + }, + }); + + const patchedDiff = { + type: 'diff', + change: 'modify', + path: 'src/a.ts', + oldText: 'a', + newText: 'b', + patch: { format: 'git_patch', text: '@@ -26,3 +26,3 @@\n ctx\n-a\n+b' }, + }; + const tools = toolSnapshots(seen); + // announce (fragment) → content replace (patch) → settle (completed). + expect(tools).toHaveLength(3); + expect(tools[0].toolCall.content).toEqual([ + { type: 'diff', change: 'modify', path: 'src/a.ts', oldText: 'a', newText: 'b' }, + ]); + expect(tools[1].toolCall.status).toBe('in_progress'); + expect(tools[1].toolCall.content).toEqual([patchedDiff]); + expect(tools[2].toolCall.status).toBe('completed'); + expect(tools[2].toolCall.content).toEqual([ + patchedDiff, + { type: 'content', content: { type: 'text', text: 'updated' } }, + ]); + }); + it('announces the Edit diff and keeps it through the settle', () => { const adapter = new TestClaude(); const seen: AgentEvent[] = []; diff --git a/packages/host/agent-adapter/src/native/claude-code.ts b/packages/host/agent-adapter/src/native/claude-code.ts index ae686527..ca3a553b 100644 --- a/packages/host/agent-adapter/src/native/claude-code.ts +++ b/packages/host/agent-adapter/src/native/claude-code.ts @@ -44,6 +44,7 @@ import type { ToolCall, ToolCallContent, ToolCallLocation, + UnifiedDiffHunk, UsageRateLimitWindow, UsageReport, } from '@linkcode/schema'; @@ -53,6 +54,7 @@ import { isSupportedAttachmentImageMimeType, textBlock, UsageReportSchema, + unifiedPatchText, } from '@linkcode/schema'; import { extractErrorMessage } from 'foxts/extract-error-message'; import { nullthrow } from 'foxts/guard'; @@ -396,6 +398,7 @@ const EMPTY_SUPPLEMENT: ClaudeTranscriptSupplement = { records: new Map(), droppedRows: [], toolUseResults: new Map(), + toolUsePatches: new Map(), }; /** @@ -540,7 +543,9 @@ export class ClaudeCodeAdapter extends BaseAgentAdapter { limit: limit + 1, offset, }), - readSubagentTranscripts(mod, opts.historyId), + readSubagentTranscripts(mod, opts.historyId, (agentId) => + this.readSubagentPatches(opts.historyId, agentId), + ), // Every page needs the raw transcript: getSessionMessages strips each result row's // structured toolUseResult, so the mapper re-attaches envelopes from here. The compaction // splice below stays first-page-only (the swapped-in summary is the SDK chain's head row). @@ -551,6 +556,7 @@ export class ClaudeCodeAdapter extends BaseAgentAdapter { historyId, supplement.records, supplement.toolUseResults, + supplement.toolUsePatches, ); const events: AgentHistoryEvent[] = []; // Splice each subagent's transcript right after its spawn announce so children land inside the @@ -596,6 +602,14 @@ export class ClaudeCodeAdapter extends BaseAgentAdapter { return readClaudeTranscriptSupplement(sessionId); } + /** Test seam over the per-subagent transcript probe (see `readSubagentPatches`). */ + protected readSubagentPatches( + sessionId: string, + agentId: string, + ): Promise> { + return readSubagentPatches(sessionId, agentId); + } + protected async onPrompt(content: ContentBlock[]): Promise { this.freshSegment(); interface ClaudeImageBlock { @@ -1244,8 +1258,13 @@ export class ClaudeCodeAdapter extends BaseAgentAdapter { // tool_use_result is message-level; only an unambiguous single-result frame can claim it. const results = content.filter((block) => block.type === 'tool_result'); const envelope = results.length === 1 ? toolUseResultEnvelope(msg.tool_use_result) : undefined; + const patched = results.length === 1 ? editResultDiffContent(msg.tool_use_result) : undefined; for (const block of content) { if (block.type !== 'tool_result') continue; + // Replace (not append) so the patch-bearing diff supersedes the announce-time fragment + // instead of stacking a second card, and do it before the settle below: a completed tool is + // terminal, so any content emitted after it is silently dropped. + if (patched) this.emitTool({ toolCallId: block.tool_use_id, content: patched }); for (const result of toolResultContent(block.content)) { this.appendToolContent(block.tool_use_id, result); } @@ -1334,6 +1353,53 @@ function editDiffContent(toolName: string, input: unknown): ToolCallContent[] | return undefined; } +function isUnifiedDiffHunk(value: unknown): value is UnifiedDiffHunk { + return ( + isRecord(value) && + typeof value.oldStart === 'number' && + typeof value.oldLines === 'number' && + typeof value.newStart === 'number' && + typeof value.newLines === 'number' && + Array.isArray(value.lines) && + value.lines.every((line) => typeof line === 'string') + ); +} + +/** + * Upgrade an Edit's announce-time diff with the patch the settle frame carries. `editDiffContent` + * only sees the tool INPUT — `old_string`/`new_string`, the replaced region with no line numbers and + * no surrounding rows — while `tool_use_result.structuredPatch` carries real hunk offsets and three + * context lines a side, which is what the UI needs to place an edit in its file (CODE-399). + * + * `structuredPatch` is the only usable source: over real transcripts `gitDiff` is never populated + * and `originalFile` is frequently null. Write is excluded deliberately — its dominant `create` case + * ships an EMPTY `structuredPatch`, and whole-file `newText` already beats an all-`+` patch. + * + * Duck-typed throughout: `tool_use_result` is `unknown` on the SDK's user frame, so `FileEditOutput` + * is documentation rather than a runtime guarantee. The `oldString`/`newString` pair is also what + * distinguishes an Edit result from a Write one, since the settle frame carries no tool name. + */ +export function editResultDiffContent(value: unknown): ToolCallContent[] | undefined { + if (!isRecord(value)) return undefined; + const { filePath, oldString: oldText, newString: newText, structuredPatch } = value; + if (typeof filePath !== 'string' || typeof oldText !== 'string' || typeof newText !== 'string') { + return undefined; + } + if (!Array.isArray(structuredPatch)) return undefined; + const hunks = structuredPatch.filter(isUnifiedDiffHunk); + if (hunks.length === 0) return undefined; + return [ + { + type: 'diff', + change: 'modify', + path: toHostPath(filePath), + oldText, + newText, + patch: { format: 'git_patch', text: unifiedPatchText(hunks) }, + }, + ]; +} + const TOOL_USE_RESULT_SCALAR_MAX = 256; /** @@ -1425,6 +1491,10 @@ export interface ClaudeTranscriptSupplement { /** tool_use_id → projected `toolUseResult` envelope (`toolUseResultEnvelope`), another field * `getSessionMessages` strips per row. Keyed only for unambiguous single-result rows. */ toolUseResults: Map>; + /** tool_use_id → the Edit diff recovered from the same raw `toolUseResult`. Separate from + * `toolUseResults` because the envelope keeps scalars only, so its filter drops `structuredPatch` + * (an array) by construction. Keyed on the same single-result rows. */ + toolUsePatches: Map; } /** @@ -1442,6 +1512,7 @@ export function buildClaudeTranscriptSupplement( ): ClaudeTranscriptSupplement { const records = new Map(); const toolUseResults = new Map>(); + const toolUsePatches = new Map(); /** Conversation rows in file order, with the index of the last boundary seen before each. */ const rows: Array<{ row: TimestampedSessionMessage; boundariesBefore: number }> = []; let boundaries = 0; @@ -1476,7 +1547,7 @@ export function buildClaudeTranscriptSupplement( } else if (row.type !== 'user' && row.type !== 'assistant') continue; // Harvested before the exclusions: tool_use ids are globally unique, so keying a row the // timeline itself skips is harmless. - if (row.type === 'user') harvestToolUseResult(toolUseResults, row); + if (row.type === 'user') harvestToolUseResult(toolUseResults, toolUsePatches, row); // Same exclusions as the SDK's own reader: meta rows, sidechains, and teammate rows. if (row.isMeta === true || row.isSidechain === true || row.teamName) continue; rows.push({ @@ -1501,17 +1572,21 @@ export function buildClaudeTranscriptSupplement( return dropped; }, []), toolUseResults, + toolUsePatches, }; } -/** Key a raw result row's `toolUseResult` envelope by its tool_use id. The field is row-level, so - * only a row with exactly one tool_result block pairs unambiguously. */ +/** Key a raw result row's `toolUseResult` projections by its tool_use id. The field is row-level, so + * only a row with exactly one tool_result block pairs unambiguously. The envelope and the Edit patch + * are independent projections of that one field — either can be absent. */ function harvestToolUseResult( - map: Map>, + envelopes: Map>, + patches: Map, row: Record, ): void { const envelope = toolUseResultEnvelope(row.toolUseResult); - if (!envelope) return; + const patch = editResultDiffContent(row.toolUseResult); + if (!envelope && !patch) return; const message = isRecord(row.message) ? row.message : undefined; const content = message?.content; if (!Array.isArray(content)) return; @@ -1521,7 +1596,9 @@ function harvestToolUseResult( } return ids; }, []); - if (ids.length === 1) map.set(ids[0], envelope); + if (ids.length !== 1) return; + if (envelope) envelopes.set(ids[0], envelope); + if (patch) patches.set(ids[0], patch); } /** @@ -1530,27 +1607,50 @@ function harvestToolUseResult( * `.jsonl` (the id is unique, so at most one probe succeeds). Any failure degrades to * an empty supplement: history still reads, just without compaction markers or result envelopes. */ -async function readClaudeTranscriptSupplement( - sessionId: string, -): Promise { - // The id becomes a filename — refuse anything that could traverse out of the projects dir. - if (!SAFE_SESSION_ID.test(sessionId)) return EMPTY_SUPPLEMENT; +async function readClaudeProjectText(segments: readonly string[]): Promise { const projectsDir = path.join(homedir(), '.claude', 'projects'); let dirs: string[]; try { dirs = await readdir(projectsDir); } catch { - return EMPTY_SUPPLEMENT; + return null; } const texts = await Promise.all( - dirs.map((dir) => - readFile(path.join(projectsDir, dir, `${sessionId}.jsonl`), 'utf8').catch(() => null), - ), + dirs.map((dir) => readFile(path.join(projectsDir, dir, ...segments), 'utf8').catch(() => null)), ); - const text = texts.find((t) => t !== null); + return texts.find((t) => t !== null) ?? null; +} + +async function readClaudeTranscriptSupplement( + sessionId: string, +): Promise { + // The id becomes a filename — refuse anything that could traverse out of the projects dir. + if (!SAFE_SESSION_ID.test(sessionId)) return EMPTY_SUPPLEMENT; + const text = await readClaudeProjectText([`${sessionId}.jsonl`]); return text ? buildClaudeTranscriptSupplement(text.split('\n')) : EMPTY_SUPPLEMENT; } +/** + * A subagent's own `subagents/agent-{id}.jsonl`, read for the same reason the parent transcript is: + * the SDK's `getSubagentMessages` projection strips `toolUseResult`, so a replayed subagent Edit + * would fall back to the announce-time fragment while the live one carries real hunks. + * + * Caveat worth knowing before trusting this: across the local corpus (117 subagent transcripts) the + * CLI writes `toolUseResult` as a bare error string only — never the structured `FileEditOutput` — + * and the one real subagent Edit had no `toolUseResult` on its settle row in either transcript. So + * this recovers nothing on today's CLI; it removes the asymmetry with the parent path and starts + * working the moment the CLI persists the field. + */ +async function readSubagentPatches( + sessionId: string, + agentId: string, +): Promise> { + // Both ids become path segments. + if (!SAFE_SESSION_ID.test(sessionId) || !SAFE_SESSION_ID.test(agentId)) return new Map(); + const text = await readClaudeProjectText([sessionId, 'subagents', `agent-${agentId}.jsonl`]); + return text ? buildClaudeTranscriptSupplement(text.split('\n')).toolUsePatches : new Map(); +} + function mapClaudeHistorySession(session: SDKSessionInfo): AgentHistorySession { return { historyId: asHistoryId(session.sessionId), @@ -1576,15 +1676,26 @@ function mapClaudeHistorySession(session: SDKSessionInfo): AgentHistorySession { async function readSubagentTranscripts( mod: typeof import('@anthropic-ai/claude-agent-sdk'), sessionId: string, + patchesFor: (agentId: string) => Promise>, ): Promise> { const agentIds = await mod.listSubagents(sessionId); const byParent = new Map(); await Promise.all( agentIds.map(async (agentId) => { - const rows = await mod.getSubagentMessages(sessionId, agentId, { limit: 1000 }); + const [rows, patches] = await Promise.all([ + mod.getSubagentMessages(sessionId, agentId, { limit: 1000 }), + patchesFor(agentId), + ]); const parent = rows.find((row) => row.parent_tool_use_id !== null)?.parent_tool_use_id; if (!parent) return; - byParent.set(parent, rows.flatMap(createClaudeHistoryEventMapper(asHistoryId(sessionId)))); + byParent.set( + parent, + rows.flatMap( + // No compaction records or result envelopes: a subagent transcript has no compaction + // boundary, and `rawOutput` recovery there is a separate concern. + createClaudeHistoryEventMapper(asHistoryId(sessionId), undefined, undefined, patches), + ), + ); }), ); return byParent; @@ -1605,6 +1716,9 @@ export function createClaudeHistoryEventMapper( /** Result envelopes recovered from the raw transcript (`ClaudeTranscriptSupplement`) — * getSessionMessages strips them, so replayed settles read theirs from here. */ toolUseResults?: ReadonlyMap>, + /** Edit diffs recovered from the same raw results, so a replayed settle carries the patch the + * live path emits rather than the announce-time fragment. */ + toolUsePatches?: ReadonlyMap, ): (message: SessionMessage) => AgentHistoryEvent[] { const announced = new Map(); /** Last model announced to the timeline; assistant rows re-announce only on change. */ @@ -1696,8 +1810,13 @@ export function createClaudeHistoryEventMapper( title: existing?.title ?? block.tool_use_id, kind: existing?.kind ?? 'other', status: block.is_error === true ? 'failed' : 'completed', - // Announce-time content is the Edit diff (or empty); keep it ahead of the result text. - content: [...(existing?.content ?? []), ...toolResultContent(block.content)], + // A recovered patch supersedes the announce-time input fragment (the live path replaces + // it too); otherwise the announce content — the Edit diff, or empty — leads the result + // text. + content: [ + ...(toolUsePatches?.get(block.tool_use_id) ?? existing?.content ?? []), + ...toolResultContent(block.content), + ], rawInput: existing?.rawInput, rawOutput: toolUseResults?.get(block.tool_use_id) ?? block.content, }), diff --git a/packages/integrations/im-render/src/render.ts b/packages/integrations/im-render/src/render.ts index a9af33c9..f45e2d20 100644 --- a/packages/integrations/im-render/src/render.ts +++ b/packages/integrations/im-render/src/render.ts @@ -10,6 +10,7 @@ import type { ToolCallStatus, ToolCallUpdate, } from '@linkcode/schema'; +import { unifiedPatchText } from '@linkcode/schema'; import { structuredPatch } from 'diff'; import { appendArrayInPlace } from 'foxts/append-array-in-place'; import { blockquote, capLines, contentToMarkdown, fence } from './blocks'; @@ -74,14 +75,7 @@ function toolContentMarkdown(content: ToolCallContent, opts: RenderOptions): str content.newText ?? '', ) : undefined; - const patchText = - suppliedPatch ?? - generatedPatch?.hunks - .flatMap((hunk) => [ - `@@ -${hunk.oldStart},${hunk.oldLines} +${hunk.newStart},${hunk.newLines} @@`, - ...hunk.lines, - ]) - .join('\n'); + const patchText = suppliedPatch ?? (generatedPatch && unifiedPatchText(generatedPatch.hunks)); return patchText ? `✏\u{FE0F} \`${label}\`\n${fence(capLines(patchText, opts.maxCodeBlockLines), 'diff')}` : `✏\u{FE0F} \`${label}\``; diff --git a/packages/presentation/i18n/src/locales/en.ts b/packages/presentation/i18n/src/locales/en.ts index a17ec6a8..3a479d95 100644 --- a/packages/presentation/i18n/src/locales/en.ts +++ b/packages/presentation/i18n/src/locales/en.ts @@ -146,6 +146,8 @@ export const en = { duration: 'Duration', size: 'Size', failed: 'Failed', + expand: 'Expand', + collapse: 'Collapse', }, subagent: { label: 'Subagent', diff --git a/packages/presentation/i18n/src/locales/zh-cn.ts b/packages/presentation/i18n/src/locales/zh-cn.ts index 0f84dd46..868bd537 100644 --- a/packages/presentation/i18n/src/locales/zh-cn.ts +++ b/packages/presentation/i18n/src/locales/zh-cn.ts @@ -142,6 +142,8 @@ export const zhCN = { duration: '耗时', size: '大小', failed: '失败', + expand: '展开', + collapse: '收起', }, subagent: { label: '子代理', diff --git a/packages/presentation/ui/src/__tests__/diff-utils.test.ts b/packages/presentation/ui/src/__tests__/diff-utils.test.ts index fdc1c9d3..a6dce6da 100644 --- a/packages/presentation/ui/src/__tests__/diff-utils.test.ts +++ b/packages/presentation/ui/src/__tests__/diff-utils.test.ts @@ -34,6 +34,15 @@ describe('diffStats', () => { ]); expect(diffStats(undefined, undefined, patch)).toEqual({ additions: 1, deletions: 1 }); }); + + it('falls back to the text when a patch parses to no rows', () => { + // `chatFileDiff` renders the text in this case, so counting the empty patch instead would show + // a visible diff with no +/- badge. Both must pick the same source. + expect(diffStats('a\n', 'b\n', '')).toEqual({ additions: 1, deletions: 1 }); + expect(diffStats('a\n', 'b\n', 'not a patch at all')).toEqual({ additions: 1, deletions: 1 }); + // Nothing to fall back to stays zero rather than inventing rows. + expect(diffStats(undefined, undefined, '')).toEqual({ additions: 0, deletions: 0 }); + }); }); describe('toolCallDiffStats', () => { diff --git a/packages/presentation/ui/src/chat/__tests__/diff-block.test.ts b/packages/presentation/ui/src/chat/__tests__/diff-block.test.ts new file mode 100644 index 00000000..8700c8ad --- /dev/null +++ b/packages/presentation/ui/src/chat/__tests__/diff-block.test.ts @@ -0,0 +1,146 @@ +import { nullthrow } from 'foxact/nullthrow'; +import { describe, expect, it } from 'vitest'; +import { chatFileDiff, hasAuthoritativeLineNumbers } from '../diff-block'; +import type { DiffToolCallContent } from '../diff-utils'; + +function diff(content: Omit): DiffToolCallContent { + return { type: 'diff', ...content }; +} + +describe('chatFileDiff', () => { + it('recovers hunks from a headerless patch, the shape both adapters emit', () => { + // codex forwards its app-server `unified_diff` verbatim and the claude adapter formats + // `structuredPatch`; neither writes a `---`/`+++` preamble, and pierre parses zero hunks + // without one — the card would render blank. + const parsed = chatFileDiff( + diff({ + change: 'modify', + path: 'src/a.ts', + patch: { format: 'git_patch', text: '@@ -26,3 +26,3 @@\n ctx\n-a\n+b' }, + }), + ); + expect(parsed?.name).toBe('src/a.ts'); + expect(parsed?.type).toBe('change'); + expect(parsed?.hunks).toHaveLength(1); + // The gutter number the issue is about: the edit sits at line 26, not line 1. + expect(parsed?.hunks[0].additionStart).toBe(26); + }); + + it('keeps every hunk and its real line numbers from a git patch', () => { + const parsed = chatFileDiff( + diff({ + change: 'modify', + path: 'src/a.ts', + patch: { + format: 'git_patch', + text: [ + 'diff --git a/src/a.ts b/src/a.ts', + '--- a/src/a.ts', + '+++ b/src/a.ts', + '@@ -12,3 +12,3 @@', + ' ctx', + '-a', + '+b', + '@@ -84,2 +84,3 @@', + ' tail', + '+added', + ].join('\n'), + }, + }), + ); + expect(parsed?.name).toBe('src/a.ts'); + expect(parsed?.hunks.map((hunk) => hunk.additionStart)).toEqual([12, 84]); + }); + + it('renders every supplied line when only the replaced region is available', () => { + // oldText/newText are the region, not the file, so a finite context window would drop rows the + // payload already carries. One hunk, everything in it. + const parsed = chatFileDiff( + diff({ change: 'modify', path: 'src/a.ts', oldText: 'a\nb\nc\n', newText: 'a\nx\nc\n' }), + ); + expect(parsed?.type).toBe('change'); + expect(parsed?.hunks).toHaveLength(1); + expect(parsed?.hunks[0].additionLines).toBe(1); + expect(parsed?.hunks[0].deletionLines).toBe(1); + }); + + it('reads a whole-file write as an addition and a delete as a removal', () => { + expect(chatFileDiff(diff({ change: 'add', path: 'new.ts', newText: 'x\ny\n' }))?.type).toBe( + 'new', + ); + expect(chatFileDiff(diff({ change: 'delete', path: 'gone.ts', oldText: 'x\n' }))?.type).toBe( + 'deleted', + ); + }); + + it('detects a rename from the differing paths', () => { + const parsed = chatFileDiff( + diff({ + change: 'move', + oldPath: 'old.ts', + path: 'new.ts', + oldText: 'a\n', + newText: 'b\n', + }), + ); + expect(parsed?.prevName).toBe('old.ts'); + expect(parsed?.name).toBe('new.ts'); + }); + + it('falls back to the text when a patch parses to nothing', () => { + const parsed = chatFileDiff( + diff({ + change: 'modify', + path: 'src/a.ts', + oldText: 'a\n', + newText: 'b\n', + patch: { format: 'git_patch', text: '' }, + }), + ); + expect(parsed?.hunks).toHaveLength(1); + }); + + it.each([ + ['binary content', diff({ change: 'delete', path: 'logo.bin', isBinary: true })], + ['a delete carrying no text', diff({ change: 'delete', path: 'gone.ts' })], + ['an unchanged pair', diff({ change: 'modify', path: 'a.ts', oldText: 'x\n', newText: 'x\n' })], + ])('draws nothing for %s', (_label, content) => { + expect(chatFileDiff(content)).toBeNull(); + }); +}); + +describe('hasAuthoritativeLineNumbers', () => { + function verdictFor(content: DiffToolCallContent): boolean { + return hasAuthoritativeLineNumbers(content, nullthrow(chatFileDiff(content))); + } + + it('trusts patch hunk offsets — pierre marks the patch branch isPartial', () => { + const content = diff({ + change: 'modify', + path: 'src/a.ts', + patch: { format: 'git_patch', text: '@@ -26,3 +26,3 @@\n ctx\n-a\n+b' }, + }); + // The rule keys off isPartial as "parsed from a patch"; pin that reading of pierre. + expect(nullthrow(chatFileDiff(content)).isPartial).toBe(true); + expect(verdictFor(content)).toBe(true); + }); + + it('trusts a whole-file write, where 1..N is exact by adapter contract', () => { + expect(verdictFor(diff({ change: 'add', path: 'new.ts', newText: 'x\ny\n' }))).toBe(true); + }); + + it.each([ + [ + 'a modify fragment', + diff({ change: 'modify', path: 'a.ts', oldText: 'a\nb\n', newText: 'a\nc\n' }), + ], + [ + 'a move with region text', + diff({ change: 'move', oldPath: 'old.ts', path: 'new.ts', oldText: 'a\n', newText: 'b\n' }), + ], + ['a delete with region text', diff({ change: 'delete', path: 'gone.ts', oldText: 'x\n' })], + ])('hides the gutter for %s — its hunk starts at 1 regardless of true position', (_label, content) => { + expect(nullthrow(chatFileDiff(content)).isPartial).toBe(false); + expect(verdictFor(content)).toBe(false); + }); +}); diff --git a/packages/presentation/ui/src/chat/__tests__/file-preview-card.test.tsx b/packages/presentation/ui/src/chat/__tests__/file-preview-card.test.tsx new file mode 100644 index 00000000..2683ea39 --- /dev/null +++ b/packages/presentation/ui/src/chat/__tests__/file-preview-card.test.tsx @@ -0,0 +1,133 @@ +// @vitest-environment jsdom + +import { cleanup, render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { nullthrow } from 'foxact/nullthrow'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { FilePreviewCard } from '../file-preview-card'; + +function translateKey(key: string): string { + return key; +} + +vi.mock('use-intl', () => ({ useTranslations: () => translateKey })); + +/** jsdom lays nothing out, so both metrics read 0 and the card can never look overflowing. + * Stub the pair the overflow check reads to drive each branch deterministically. */ +function stubPanelMetrics(scrollHeight: number, clientHeight: number): void { + for (const [name, value] of [ + ['scrollHeight', scrollHeight], + ['clientHeight', clientHeight], + ] as const) { + vi.spyOn(HTMLElement.prototype, name, 'get').mockReturnValue(value); + } +} + +const PEEK = 'chat-card-peek'; +const FADE = 'chat-card-peek-fade'; + +function panelOf(container: HTMLElement): HTMLElement { + return nullthrow( + container.querySelector('[data-slot="frame-panel"]'), + 'no panel rendered', + ); +} + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +describe('FilePreviewCard peek', () => { + it('offers a toggle only when the body outgrows the peek', () => { + stubPanelMetrics(600, 200); + const { container } = render(body); + + expect(panelOf(container).className).toContain(PEEK); + expect(panelOf(container).className).toContain(FADE); + expect(screen.getByRole('button', { name: 'expand' })).toBeDefined(); + }); + + it('leaves a body that already fits unclamped and uncontrolled', () => { + stubPanelMetrics(120, 120); + const { container } = render(body); + + // The clamp class stays — it is what caps the peek — but nothing is hidden behind it, so the + // card offers no affordance and must NOT fade: the gradient is relative to the element's own + // height, so an unconditional mask greys out the last line of a fully-visible body. + expect(panelOf(container).className).toContain(PEEK); + expect(panelOf(container).className).not.toContain(FADE); + expect(container.querySelector('footer')).toBeNull(); + expect(screen.queryByRole('button', { name: 'expand' })).toBeNull(); + }); + + it('drops the clamp when expanded and keeps the control to collapse again', async () => { + stubPanelMetrics(600, 200); + const { container } = render(body); + + await userEvent.click(screen.getByRole('button', { name: 'expand' })); + + expect(panelOf(container).className).not.toContain(PEEK); + expect(panelOf(container).className).not.toContain(FADE); + // Expanding makes the panel fit its content, so the toggle must survive its own success. + expect(screen.getByRole('button', { name: 'collapse' })).toBeDefined(); + }); + + it('renders no body chrome at all when the card has no children', () => { + stubPanelMetrics(600, 200); + const { container } = render(); + + expect(container.querySelector('[data-slot="frame-panel"]')).toBeNull(); + expect(container.querySelector('footer')).toBeNull(); + }); + + it('measures a body that mounts after the header-only first render', () => { + // The live shape for a Read or an Edit: the announce renders a header-only card and the body + // only arrives with the settle. The panel element therefore does not exist on the first + // render, so the size subscription has to attach when it appears rather than once at mount. + stubPanelMetrics(600, 200); + const { container, rerender } = render(); + expect(container.querySelector('[data-slot="frame-panel"]')).toBeNull(); + + rerender(body); + + expect(panelOf(container).className).toContain(PEEK); + expect(panelOf(container).className).toContain(FADE); + expect(screen.getByRole('button', { name: 'expand' })).toBeDefined(); + }); + + it('reveals the body when focus reaches a control clipped behind the peek', async () => { + // `overflow: clip` hides the overflow visually but keeps its links and checkboxes tabbable, + // and cannot be scrolled to bring them into view. Expanding on focus is what keeps a + // keyboard user from landing on something invisible. + stubPanelMetrics(600, 200); + const { container } = render( + + clipped link + , + ); + expect(panelOf(container).className).toContain(PEEK); + + await userEvent.tab(); + await userEvent.tab(); + + expect(document.activeElement).toBe(screen.getByRole('link', { name: 'clipped link' })); + expect(panelOf(container).className).not.toContain(PEEK); + }); + + it('does not arm focus-expansion on a body that fits', async () => { + stubPanelMetrics(120, 120); + const { container } = render( + + visible link + , + ); + + await userEvent.tab(); + await userEvent.tab(); + + // Nothing was hidden, so focusing inside must not sprout a collapse control. + expect(panelOf(container).className).toContain(PEEK); + expect(container.querySelector('footer')).toBeNull(); + }); +}); diff --git a/packages/presentation/ui/src/chat/__tests__/tool-call-item.test.tsx b/packages/presentation/ui/src/chat/__tests__/tool-call-item.test.tsx index 299a0667..26f6e673 100644 --- a/packages/presentation/ui/src/chat/__tests__/tool-call-item.test.tsx +++ b/packages/presentation/ui/src/chat/__tests__/tool-call-item.test.tsx @@ -362,13 +362,16 @@ describe('ToolCallBody', () => { content: [{ type: 'diff', change: 'delete', path: '/repo/logo.bin', isBinary: true }], }; - const { rerender } = render(); + const { container, rerender } = render(); expect(screen.getByText('/repo/old.ts → /repo/new.ts')).toBeDefined(); - expect(screen.getByText('old')).toBeDefined(); - expect(screen.getByText('new')).toBeDefined(); + // The rows themselves live in @pierre/diffs' shadow root, which queries do not descend into; + // their content is covered by the chatFileDiff tests. Here, only that a diff mounted at all. + expect(container.querySelector('diffs-container')).not.toBeNull(); rerender(); expect(screen.getByText('logo.bin')).toBeDefined(); + // Binary deletes have nothing to render — the card stays header-only. + expect(container.querySelector('diffs-container')).toBeNull(); }); it('keeps a failed mutation explanation visible without presenting it as file content', () => { diff --git a/packages/presentation/ui/src/chat/chat-card.tsx b/packages/presentation/ui/src/chat/chat-card.tsx index c039f0cf..b420fbf5 100644 --- a/packages/presentation/ui/src/chat/chat-card.tsx +++ b/packages/presentation/ui/src/chat/chat-card.tsx @@ -1,4 +1,4 @@ -import { FrameHeader, FramePanel } from 'coss-ui/components/frame'; +import { FrameFooter, FrameHeader, FramePanel } from 'coss-ui/components/frame'; import { cn } from '../lib/cn'; export type ChatCardHeaderProps = React.ComponentProps; @@ -37,3 +37,15 @@ export type ChatCardPanelProps = React.ComponentProps; export function ChatCardPanel({ className, ...props }: ChatCardPanelProps): React.ReactNode { return ; } + +export type ChatCardFooterProps = React.ComponentProps; + +/** coss-ui `FrameFooter` as a single compact row, at chat-flow density. */ +export function ChatCardFooter({ className, ...props }: ChatCardFooterProps): React.ReactNode { + return ( + + ); +} diff --git a/packages/presentation/ui/src/chat/diff-block.tsx b/packages/presentation/ui/src/chat/diff-block.tsx index 65f96e59..56f6bf0d 100644 --- a/packages/presentation/ui/src/chat/diff-block.tsx +++ b/packages/presentation/ui/src/chat/diff-block.tsx @@ -1,7 +1,10 @@ +import type { FileDiffMetadata, FileDiffOptions } from '@pierre/diffs'; +import { parseDiffFromFile, processFile } from '@pierre/diffs'; +import { FileDiff } from '@pierre/diffs/react'; import { cn } from '../lib/cn'; import type { ArtifactNavigation } from './artifacts/host-actions'; -import type { DiffStats } from './diff-utils'; -import { diffLines, patchLines } from './diff-utils'; +import type { DiffStats, DiffToolCallContent } from './diff-utils'; +import { diffContentStats } from './diff-utils'; import { FilePreviewCard } from './file-preview-card'; export function DiffCounter({ @@ -21,54 +24,115 @@ export function DiffCounter({ ); } +const CHAT_DIFF_OPTIONS: FileDiffOptions = { + // The card is one narrow column inside the chat flow, so `split` (the library default) is + // unreadable here. `wrap` keeps the card to a single scroll axis — a nested horizontal scroller + // would fight the disclosure's ScrollArea and the virtualized timeline outside it. + diffStyle: 'unified', + overflow: 'wrap', + // FilePreviewCard already renders the file identity, path tooltip, and open action. + disableFileHeader: true, + // Renders the raw `@@ -120,8 +120,9 @@` range. The default `line-info` separator hardcodes + // English ("N unmodified lines", "Expand all") with an en-US plural rule, which cannot go through + // use-intl; its expand affordance is dead here anyway, since patch-derived diffs carry no file + // body to expand into. + hunkSeparators: 'metadata', +}; + +const CHAT_DIFF_UNNUMBERED_OPTIONS: FileDiffOptions = { + ...CHAT_DIFF_OPTIONS, + disableLineNumbers: true, +}; + +/** The gutter renders only when its numbers are true file positions. A parsed patch carries real + * hunk offsets (`isPartial` is pierre's parsed-from-patch marker), and a Write is whole-file by + * adapter contract, so 1..N is exact. Everything else on the text branch is a region — + * `old_string`/`new_string`, codex replay hunk bodies — whose synthesized hunk starts at 1, and + * numbering those rows would assert the fragment sits at the top of the file. */ +export function hasAuthoritativeLineNumbers( + content: DiffToolCallContent, + fileDiff: FileDiffMetadata, +): boolean { + return fileDiff.isPartial || content.change === 'add'; +} + +/** A hunk stream with no `---`/`+++` (or `diff --git`) preamble. Both producers emit one: codex + * forwards its app-server `unified_diff` verbatim, and the claude adapter formats + * `structuredPatch` hunks. Pierre parses zero hunks out of that, so the card would render blank. */ +const HEADERLESS_PATCH = /^\s*@@/; + +/** Pierre only strips the `a/`…`b/` prefixes when a `diff --git` line proves it is a git patch, so + * the synthesized header uses the bare path on both sides — otherwise the two names differ and the + * diff is misread as a rename. */ +function patchWithHeader(text: string, path: string): string { + return HEADERLESS_PATCH.test(text) ? `--- ${path}\n+++ ${path}\n${text}` : text; +} + +// Parsing runs jsdiff (or pierre's patch parser) and is far too heavy for a render pass. The +// conversation builder replaces content objects rather than mutating them, so object identity is a +// sound cache key — the same trick `diffContentStats` uses. +const fileDiffCache = new WeakMap(); + +/** Build the renderable diff for one `type: 'diff'` content item, or null when there is nothing to + * draw (binary, or a rename/delete carrying neither a patch nor text). */ +export function chatFileDiff(content: DiffToolCallContent): FileDiffMetadata | null { + const cached = fileDiffCache.get(content); + if (cached !== undefined) return cached; + const built = buildFileDiff(content); + fileDiffCache.set(content, built); + return built; +} + +function buildFileDiff(content: DiffToolCallContent): FileDiffMetadata | null { + const { path, oldPath, oldText, newText, patch, isBinary } = content; + if (isBinary) return null; + if (patch?.text) { + const parsed = processFile(patchWithHeader(patch.text, path)); + // A patch that yields no hunks is not authoritative; codex ships hunk text alongside it. + if (parsed && parsed.hunks.length > 0) return parsed; + } + if (oldText === undefined && newText === undefined) return null; + // Infinite context: `oldText`/`newText` are the replaced region, not the file, so any finite + // window would drop rows the payload already carries and fence a snippet with `@@` ranges whose + // line numbers mean nothing. Pierre infers new/deleted/renamed from the contents and names. + const parsed = parseDiffFromFile( + { name: oldPath ?? path, contents: oldText ?? '' }, + { name: path, contents: newText ?? '' }, + { context: Number.MAX_SAFE_INTEGER }, + ); + return parsed.hunks.length > 0 ? parsed : null; +} + export function DiffBlock({ - path, - oldPath, - oldText, - newText, - patch, + content, navigation, }: { - path: string; - oldPath?: string; - oldText?: string; - newText?: string; - patch?: string; + content: DiffToolCallContent; navigation?: ArtifactNavigation | null; }): React.ReactNode { - const rows = patch === undefined ? diffLines(oldText ?? '', newText ?? '') : patchLines(patch); - const stats = { - additions: rows.filter((row) => row.type === 'add').length, - deletions: rows.filter((row) => row.type === 'del').length, - }; + const fileDiff = chatFileDiff(content); + const { path, oldPath } = content; + // `overflow-clip`: the panel is rounded, but pierre's rows are square and full-bleed, so they + // need a clip to stay inside its corners. `clip` over `hidden` so the box cannot be scrolled + // programmatically out from under the collapsed peek. return ( } + headerEnd={} label={oldPath ? `${oldPath} → ${path}` : undefined} navigation={navigation} - panelClassName={ - rows.length > 0 ? 'overflow-x-auto p-0 font-mono text-xs leading-relaxed' : undefined - } + panelClassName={fileDiff ? 'chat-diff-surface overflow-clip p-0' : undefined} path={path} > - {rows.length > 0 - ? rows.map((row) => ( -
- - {row.type === 'add' ? '+' : row.type === 'del' ? '-' : ' '} - - {row.text} -
- )) - : undefined} + {fileDiff ? ( + + ) : undefined}
); } diff --git a/packages/presentation/ui/src/chat/diff-utils.ts b/packages/presentation/ui/src/chat/diff-utils.ts index c7f42247..f160d7d4 100644 --- a/packages/presentation/ui/src/chat/diff-utils.ts +++ b/packages/presentation/ui/src/chat/diff-utils.ts @@ -104,7 +104,14 @@ export function diffStats( newText: string | undefined, patch?: string, ): DiffStats { - const rows = patch === undefined ? diffLines(oldText ?? '', newText ?? '') : patchLines(patch); + // A patch that parses to nothing is not authoritative — codex ships hunk text alongside one, and + // an empty or malformed patch would otherwise report 0/0 next to a card that renders real rows. + // This precedence must track `chatFileDiff`, which falls back to the text on the same condition. + const patchRows = patch === undefined ? undefined : patchLines(patch); + const rows = + patchRows !== undefined && patchRows.length > 0 + ? patchRows + : diffLines(oldText ?? '', newText ?? ''); return { additions: rows.filter((row) => row.type === 'add').length, deletions: rows.filter((row) => row.type === 'del').length, diff --git a/packages/presentation/ui/src/chat/file-preview-card.tsx b/packages/presentation/ui/src/chat/file-preview-card.tsx index 357ec6f2..87530e9f 100644 --- a/packages/presentation/ui/src/chat/file-preview-card.tsx +++ b/packages/presentation/ui/src/chat/file-preview-card.tsx @@ -1,11 +1,24 @@ import { Badge } from 'coss-ui/components/badge'; import { Frame } from 'coss-ui/components/frame'; -import { useRef } from 'react'; +import { falseFn, noop } from 'foxts/noop'; +import { useCallback, useRef, useState, useSyncExternalStore } from 'react'; +import { useTranslations } from 'use-intl'; import { cn } from '../lib/cn'; import { fileBasename } from './artifacts/file-kind'; import type { ArtifactNavigation } from './artifacts/host-actions'; import { artifactNavigationAction, useArtifactHostActions } from './artifacts/host-actions'; -import { ChatCardActions, ChatCardHeader, ChatCardPanel, ChatCardTitle } from './chat-card'; +import { + ChatCardActions, + ChatCardFooter, + ChatCardHeader, + ChatCardPanel, + ChatCardTitle, +} from './chat-card'; +import { + CHAT_DISCLOSURE_TITLE_CLASS_NAME, + CHAT_DISCLOSURE_TRIGGER_CLASS_NAME, + ChatDisclosureChevron, +} from './disclosure-header'; import { FileIdentityIcon } from './file-identity-icon'; import { FilePathTooltip } from './with-tooltip'; @@ -33,6 +46,35 @@ export function FilePreviewCard({ tooltip?: string; }): React.ReactNode { const tooltipAnchorRef = useRef(null); + const t = useTranslations('workbench.tool'); + // The body starts clamped to a peek rather than hidden: a long diff shouldn't dominate the + // timeline, but a card showing nothing at all is worse than one showing too much. Height-clamped + // (not unmounted) so the peek is real content and expanding never re-renders the body. + const [expanded, setExpanded] = useState(false); + // State, not a ref: a tool card announces header-only and grows its body on the settle, so the + // panel is absent on the first render. A ref read inside the subscribe would be null then, and + // `useSyncExternalStore` only resubscribes when the subscribe identity changes — the card would + // stay unmeasured for the rest of its life. + const [panel, setPanel] = useState(null); + // Observed rather than measured once: the diff renderer highlights asynchronously, so a body + // that fits on first paint can outgrow the clamp a frame later. Children are observed too — a + // body sitting exactly at the clamp grows without moving the panel's own box. + const subscribeToPanelSize = useCallback( + (onChange: () => void): (() => void) => { + if (!panel || typeof ResizeObserver === 'undefined') return noop; + const observer = new ResizeObserver(onChange); + observer.observe(panel); + for (const child of panel.children) observer.observe(child); + return () => observer.disconnect(); + }, + [panel], + ); + // `overflow: clip` still reports the full scroll height, so this holds while clamped. + const readPanelOverflow = useCallback( + (): boolean => (panel ? panel.scrollHeight > panel.clientHeight : false), + [panel], + ); + const panelOverflowing = useSyncExternalStore(subscribeToPanelSize, readPanelOverflow, falseFn); const actions = useArtifactHostActions(); const target = navigation === undefined ? { kind: 'file' as const, path } : navigation; const onOpen = artifactNavigationAction(actions, target); @@ -78,7 +120,46 @@ export function FilePreviewCard({ {header} {children === undefined ? null : ( - {children} + <> + + {/* A body that fits the peek has nothing to reveal, so it gets no toggle. Stays mounted + once expanded: collapsing shrinks the panel back under the clamp, and dropping the + control then would strand the card open. */} + {panelOverflowing || expanded ? ( + + + + ) : null} + )} ); diff --git a/packages/presentation/ui/src/chat/tool-result-preview.tsx b/packages/presentation/ui/src/chat/tool-result-preview.tsx index 66813bc5..928602ac 100644 --- a/packages/presentation/ui/src/chat/tool-result-preview.tsx +++ b/packages/presentation/ui/src/chat/tool-result-preview.tsx @@ -46,16 +46,7 @@ function RenderedContent({ }): React.ReactNode { if (content.type === 'content') return ; if (content.type === 'diff') { - return ( - - ); + return ; } const command = toolCallCommand(toolCall); if (TerminalBlockComponent) { diff --git a/packages/presentation/ui/src/styles.css b/packages/presentation/ui/src/styles.css index 6eb1a7b7..613473b7 100644 --- a/packages/presentation/ui/src/styles.css +++ b/packages/presentation/ui/src/styles.css @@ -342,4 +342,44 @@ .chat-terminal-output .ansi-hidden { visibility: hidden; } + + /* Collapsed file-card body: about ten rows at the chat mono scale (20px each, matching the diff + renderer's line height), so a long diff or file read previews instead of taking over the + timeline. The footer toggle lifts the clamp. */ + /* `clip`, not `hidden`: a hidden box is still programmatically scrollable, so a focus move or + `scrollIntoView` inside the body could strand the peek mid-content. */ + .chat-card-peek { + max-height: 12.5rem; + overflow: clip; + } + + /* Separate from the clamp because the gradient is relative to the element's OWN height: applied + unconditionally it fades the last line of a short body that is not clipped at all, costing + legibility with nothing to reveal. Paint-only, so adding it cannot disturb the overflow + measurement that gates it. */ + .chat-card-peek-fade { + mask-image: linear-gradient(to bottom, black calc(100% - 2rem), transparent); + } + + /* `@pierre/diffs` renders the inline chat diff into a shadow root, so Tailwind utilities cannot + reach its rows — only inherited custom properties can. Match the card's mono/xs type scale, + and drop the library's 8px block padding: it frames a standalone viewer, but here the panel + already provides the card's inset and the rows should meet its edges. */ + .chat-diff-surface { + --diffs-font-family: var(--font-mono); + --diffs-font-size: var(--text-xs); + --diffs-gap-block: 0; + } + + /* The palette is all `light-dark()`, which reads `color-scheme` — and pierre hard-declares + `color-scheme: light dark` on its own `:host`, so an inherited value loses and the diff would + follow the OS instead of the app's `.dark` class. Outer-document rules matching the host + element outrank `:host`, so target the custom element itself. */ + .chat-diff-surface diffs-container { + color-scheme: light; + } + + .dark .chat-diff-surface diffs-container { + color-scheme: dark; + } } diff --git a/vitest.config.ts b/vitest.config.ts index ed84e63a..6a51468a 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -22,6 +22,7 @@ export default defineConfig({ '.github/scripts/**/*.test.mjs', ], environment: 'node', + setupFiles: ['./vitest.setup.ts'], }, resolve: { // Mirror apps/desktop's `@renderer` path alias (apps/desktop/tsconfig.json + electron.vite.config.ts) diff --git a/vitest.setup.ts b/vitest.setup.ts new file mode 100644 index 00000000..1ba6081b --- /dev/null +++ b/vitest.setup.ts @@ -0,0 +1,30 @@ +/** + * jsdom gaps that `@pierre/diffs` walks into the moment the chat diff card mounts a `FileDiff`. + * Applied globally because `setupFiles` runs for every environment; both writes are skipped when + * the real API exists, so a node-environment test is unaffected. + * + * Written through `Reflect`: the DOM lib types both of these as always present, so a plain + * assignment needs a force cast (banned) and `??=` reads as a provably-unnecessary condition. + */ + +// `undefined`-returning rather than empty-bodied: an empty function body is a lint error. +const noop = (): void => undefined; + +/** Constructed eagerly by the library's ResizeManager, and absent from jsdom. Left un-stubbed the + * render throws, and the library's own error boundary swaps the whole diff for an error box. */ +class NoopResizeObserver implements ResizeObserver { + readonly observe = noop; + readonly unobserve = noop; + readonly disconnect = noop; +} + +if (!Reflect.has(globalThis, 'ResizeObserver')) { + Reflect.set(globalThis, 'ResizeObserver', NoopResizeObserver); +} + +/** base-ui's ScrollAreaViewport calls `getAnimations()` from a timeout. That timer never outlived a + * test before; the diff card's asynchronous highlight keeps the viewport alive long enough for it + * to fire, and an uncaught throw there fails the run. */ +if (typeof Element !== 'undefined' && !Reflect.has(Element.prototype, 'getAnimations')) { + Reflect.set(Element.prototype, 'getAnimations', () => []); +}