From 9549590253007d51d85f4a31a788d15a794d32ca Mon Sep 17 00:00:00 2001 From: toBerlinWay Date: Sun, 2 Aug 2026 14:26:47 +0800 Subject: [PATCH] fix(chat): avoid false file output cards --- .../src/components/chat/MessageList.test.tsx | 12 +- desktop/src/components/chat/MessageList.tsx | 11 +- .../src/lib/assistantOutputTargets.test.ts | 19 ++- desktop/src/lib/assistantOutputTargets.ts | 5 +- src/server/__tests__/sessions.test.ts | 113 ++++++++++++++++++ src/server/services/sessionRewindService.ts | 22 ++-- 6 files changed, 159 insertions(+), 23 deletions(-) diff --git a/desktop/src/components/chat/MessageList.test.tsx b/desktop/src/components/chat/MessageList.test.tsx index 2a3af084a..bd1a3a8fc 100644 --- a/desktop/src/components/chat/MessageList.test.tsx +++ b/desktop/src/components/chat/MessageList.test.tsx @@ -5254,7 +5254,7 @@ describe('MessageList nested tool calls', () => { expect(await screen.findByText('blank-response.ts')).toBeTruthy() }) - it('keeps historical turn change cards visible while the next turn is running', async () => { + it('keeps checkpoint evidence while hiding change cards for a running background task', async () => { vi.spyOn(sessionsApi, 'getTurnCheckpoints').mockResolvedValue({ checkpoints: [ { @@ -5283,7 +5283,7 @@ describe('MessageList nested tool calls', () => { { id: 'assistant-1', type: 'assistant_text', - content: 'done', + content: '我正准备查看 test123.md', timestamp: 2, }, ] @@ -5297,6 +5297,7 @@ describe('MessageList nested tool calls', () => { render() expect(await screen.findByText('first.ts')).toBeTruthy() + expect(screen.queryByText('Markdown')).toBeNull() act(() => { useChatStore.setState({ @@ -5322,6 +5323,7 @@ describe('MessageList nested tool calls', () => { await waitFor(() => { expect(screen.queryByText('first.ts')).toBeNull() }) + expect(screen.queryByText('Markdown')).toBeNull() act(() => { useChatStore.setState({ @@ -5347,6 +5349,7 @@ describe('MessageList nested tool calls', () => { await waitFor(() => { expect(screen.getByText('first.ts')).toBeTruthy() }) + expect(screen.queryByText('Markdown')).toBeNull() }) it('does not load turn change cards while background tasks are still running', async () => { @@ -5604,7 +5607,7 @@ describe('MessageList nested tool calls', () => { { id: 'assistant-2', type: 'assistant_text', - content: 'second done', + content: '我正准备查看 test123.md', timestamp: 4, }, ], @@ -5618,6 +5621,9 @@ describe('MessageList nested tool calls', () => { expect(cards).toHaveLength(1) expect(screen.getByText('first.ts')).toBeTruthy() expect(screen.queryByText('second.ts')).toBeNull() + await waitFor(() => { + expect(screen.queryByText('Markdown')).toBeNull() + }) }) it('shows raw startup details under translated CLI startup errors', () => { diff --git a/desktop/src/components/chat/MessageList.tsx b/desktop/src/components/chat/MessageList.tsx index 234e74e0a..94ce871d2 100644 --- a/desktop/src/components/chat/MessageList.tsx +++ b/desktop/src/components/chat/MessageList.tsx @@ -793,6 +793,7 @@ function buildTurnCardInsertionMap( const cardsByRenderIndex = new Map() turnChangeCards.forEach((card) => { + if (card.checkpoint.code.filesChanged.length === 0) return const renderIndex = lastResponseIndexByTurnId.get(card.target.messageId) ?? userIndexByTurnId.get(card.target.messageId) @@ -820,9 +821,7 @@ function buildChangedFilesByRenderIndex( ): Map { const filesByTurnId = new Map() for (const card of turnChangeCards) { - if (card.checkpoint.code.filesChanged.length > 0) { - filesByTurnId.set(card.target.messageId, card.checkpoint.code.filesChanged) - } + filesByTurnId.set(card.target.messageId, card.checkpoint.code.filesChanged) } if (filesByTurnId.size === 0) return new Map() @@ -2022,8 +2021,8 @@ export function MessageList({ sessionId, compact = false, mobileLayout = false } [renderItems, visibleTurnChangeCards], ) const changedFilesByRenderIndex = useMemo( - () => buildChangedFilesByRenderIndex(renderItems, visibleTurnChangeCards), - [renderItems, visibleTurnChangeCards], + () => buildChangedFilesByRenderIndex(renderItems, turnChangeCards), + [renderItems, turnChangeCards], ) const renderItemKeys = useMemo( () => renderItems.map(getRenderItemKey), @@ -2170,7 +2169,7 @@ export function MessageList({ sessionId, compact = false, mobileLayout = false } const target = targetByMessageId.get(checkpoint.target.targetUserMessageId) ?? targetByUserMessageIndex.get(checkpoint.target.userMessageIndex) - if (!target || !checkpoint.code.available || checkpoint.code.filesChanged.length === 0) { + if (!target || !checkpoint.code.available) { return [] } return [{ diff --git a/desktop/src/lib/assistantOutputTargets.test.ts b/desktop/src/lib/assistantOutputTargets.test.ts index 3d65e3a78..9bf552cc6 100644 --- a/desktop/src/lib/assistantOutputTargets.test.ts +++ b/desktop/src/lib/assistantOutputTargets.test.ts @@ -333,13 +333,26 @@ describe('extractAssistantOutputTargets with changedFiles reconciliation', () => }) }) - it('falls back to text-only behavior when changedFiles is empty', () => { + it('drops file mentions when changedFiles explicitly confirms no files changed', () => { + const targets = extractAssistantOutputTargets( + '我正准备查看 test123.md，服务地址是 http://localhost:5173/', + { + workDir: '/private/tmp', + changedFiles: [], + }, + ) + + expect(targets).toHaveLength(1) + expect(targets).toMatchObject([ + { kind: 'localhost-url', href: 'http://localhost:5173/' }, + ]) + }) + + it('falls back to text-only behavior when changedFiles is unavailable', () => { const targets = extractAssistantOutputTargets('已创建 `index.html`', { workDir: '/private/tmp', - changedFiles: [], }) - // No reconciliation → original bare-path behavior (mention kept as-is). expect(targets).toMatchObject([{ kind: 'local-html', normalizedPath: 'index.html' }]) }) diff --git a/desktop/src/lib/assistantOutputTargets.ts b/desktop/src/lib/assistantOutputTargets.ts index 8c4430ca2..a605a6986 100644 --- a/desktop/src/lib/assistantOutputTargets.ts +++ b/desktop/src/lib/assistantOutputTargets.ts @@ -32,7 +32,8 @@ export type ExtractAssistantOutputTargetOptions = { * file is corrected to the actual changed path (so `index.html` resolves to the * `todo-app/index.html` that was really written), and a mentioned file that the * turn never changed is dropped instead of pointing at a non-existent path. - * Localhost URLs are unaffected. Omitted/empty → fall back to text-only behavior. + * Localhost URLs are unaffected. Omitted → fall back to text-only behavior; + * an empty array confirms the turn changed no files, so file targets are dropped. */ changedFiles?: string[] } @@ -239,7 +240,7 @@ export function extractAssistantOutputTargets( results.push(candidate.target) } - if (options.changedFiles && options.changedFiles.length > 0) { + if (options.changedFiles !== undefined) { return reconcileTargetsWithChangedFiles(results, options.changedFiles, workDir) } diff --git a/src/server/__tests__/sessions.test.ts b/src/server/__tests__/sessions.test.ts index 2dd7fbafe..0b78c774c 100644 --- a/src/server/__tests__/sessions.test.ts +++ b/src/server/__tests__/sessions.test.ts @@ -5572,6 +5572,119 @@ describe('Sessions API', () => { ]) }) + it('GET /api/sessions/:id/turn-checkpoints should keep an available empty preview for an unchanged snapshot-backed turn', async () => { + const sessionId = '99999999-bbbb-cccc-dddd-000000000005' + const workDir = path.join(tmpDir, 'unchanged-snapshot-session') + const targetFile = path.join(workDir, 'src', 'unchanged.ts') + const userId = crypto.randomUUID() + const backupName = 'unchanged-snapshot@v1' + const content = 'export const unchanged = true\n' + + await fs.mkdir(path.dirname(targetFile), { recursive: true }) + await fs.writeFile(targetFile, content, 'utf-8') + await writeFileHistoryBackup(sessionId, backupName, content) + await writeSessionFile('-tmp-unchanged-snapshot-session', sessionId, [ + makeSessionMetaEntry(workDir), + makeFileHistorySnapshotEntry(userId, { + 'src/unchanged.ts': { + backupFileName: backupName, + version: 1, + backupTime: '2026-01-01T00:00:00.000Z', + }, + }), + { + ...makeUserEntry('inspect the project', userId), + cwd: workDir, + sessionId, + }, + makeAssistantEntry('No files needed changes.', userId), + ]) + + const res = await fetch(`${baseUrl}/api/sessions/${sessionId}/turn-checkpoints`) + expect(res.status).toBe(200) + const body = await res.json() as { + checkpoints: Array<{ + target: { targetUserMessageId: string } + code: { + available: boolean + filesChanged: string[] + insertions: number + deletions: number + } + }> + } + + expect(body.checkpoints).toHaveLength(1) + expect(body.checkpoints[0]).toMatchObject({ + target: { targetUserMessageId: userId }, + code: { + available: true, + filesChanged: [], + insertions: 0, + deletions: 0, + }, + }) + }) + + it('GET /api/sessions/:id/turn-checkpoints should retain transcript changes when the snapshot diff is empty', async () => { + const sessionId = '99999999-bbbb-cccc-dddd-000000000006' + const workDir = path.join(tmpDir, 'empty-snapshot-transcript-session') + const unchangedFile = path.join(workDir, 'src', 'unchanged.ts') + const transcriptFile = path.join(workDir, 'test123.md') + const userId = crypto.randomUUID() + const backupName = 'empty-snapshot-transcript@v1' + const content = 'export const unchanged = true\n' + + await fs.mkdir(path.dirname(unchangedFile), { recursive: true }) + await fs.writeFile(unchangedFile, content, 'utf-8') + await writeFileHistoryBackup(sessionId, backupName, content) + await writeSessionFile('-tmp-empty-snapshot-transcript-session', sessionId, [ + makeSessionMetaEntry(workDir), + makeFileHistorySnapshotEntry(userId, { + 'src/unchanged.ts': { + backupFileName: backupName, + version: 1, + backupTime: '2026-01-01T00:00:00.000Z', + }, + }), + { + ...makeUserEntry('write a short note', userId), + cwd: workDir, + sessionId, + }, + makeAssistantToolUseEntry([{ + id: 'Write:empty-snapshot-fallback', + name: 'Write', + input: { + file_path: transcriptFile, + content: '# Notes\n', + }, + }], userId), + makeAssistantEntry('Note written.', userId), + ]) + + const res = await fetch(`${baseUrl}/api/sessions/${sessionId}/turn-checkpoints`) + expect(res.status).toBe(200) + const body = await res.json() as { + checkpoints: Array<{ + code: { + available: boolean + filesChanged: string[] + insertions: number + deletions: number + } + }> + } + + expect(body.checkpoints).toHaveLength(1) + expect(body.checkpoints[0]!.code).toEqual({ + available: true, + filesChanged: [transcriptFile], + insertions: 1, + deletions: 0, + }) + }) + it('GET /api/sessions/:id/turn-checkpoints/diff should return target-bound checkpoint diffs', async () => { const fixture = await createThreeTurnCheckpointFixture( '99999999-bbbb-cccc-dddd-ffffffffffff', diff --git a/src/server/services/sessionRewindService.ts b/src/server/services/sessionRewindService.ts index 8f523ee67..4f7cbf085 100644 --- a/src/server/services/sessionRewindService.ts +++ b/src/server/services/sessionRewindService.ts @@ -955,15 +955,19 @@ export async function listSessionTurnCheckpoints( const checkpointPreview = targetSnapshot ? await buildTurnCodePreview(sessionId, checkpointBaseDir, targetSnapshot, nextSnapshot) : null - const preview = checkpointPreview?.available && checkpointPreview.filesChanged.length > 0 - ? checkpointPreview - : buildTranscriptTurnCodePreview( - activeMessages, - target.targetUserMessageId, - checkpointBaseDir, - ) - - if (!preview.available || preview.filesChanged.length === 0) continue + let preview = checkpointPreview + if (!preview?.available || preview.filesChanged.length === 0) { + const transcriptPreview = buildTranscriptTurnCodePreview( + activeMessages, + target.targetUserMessageId, + checkpointBaseDir, + ) + if (transcriptPreview.available) { + preview = transcriptPreview + } + } + + if (!preview?.available) continue checkpoints.push(buildTurnPreview(target, preview, checkpointBaseDir)) }