Skip to content

Commit d2896bc

Browse files
authored
fix(code): fix truncated diff preview (#1235)
## problem diff viewer is a little broken - mostly because sometimes the tool output is too large, causing the "diff preview content" to be something like this: ``` <persisted-output> Output too large (110.7KB). Full output saved to: /Users/adambowker/Library/Application Support/@posthog/posthog-code-dev/claude/projects/-Users-adambowker-posthog/019ce8a1-8438-7365-af52-3e0d8a594eb2/tool-results/toolu_019XVYxMKKxpPvfecMUzFEwj.txt Preview (first 2KB): import './EditSurvey.scss' import { DndContext } from '@dnd-kit/core' import { SortableContext, verticalListSortingStrategy } from '@dnd-kit/sortable' import { BindLogic, useActions, useMountedLogic, useValues } from 'kea' ... </persisted-output> ``` when this happens, 1) it looks bad, 2) the stats are computed incorrectly as `+0 -0` , and 3) just means you can’t actually view the diff also, the diff sometimes doesn't work if it's a delete-only, because we only check `newText` when rendering the diff ## changes 1. updates `EditToolView` to check `oldText || newText` to determine if there is a diff, instead of only `newText` 2. updates `toolInfoFromToolUse` to build the diff by: 1. first, check file content cache exists and is valid (old text exists in the content) 2. if missing or not valid, attempt to read the file contents from disk 3. if both of those fail, fall back to rendering fragment diff only, no context lines
1 parent f16371e commit d2896bc

File tree

2 files changed

+52
-15
lines changed

2 files changed

+52
-15
lines changed

apps/code/src/renderer/features/sessions/components/session-update/EditToolView.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ export function EditToolView({
6969
const oldText = diff?.oldText;
7070
const newText = diff?.newText;
7171
const isNewFile = diff && !oldText;
72+
const hasDiff = diff && (oldText || newText);
7273
const diffStats = diff ? getDiffStats(oldText, newText) : null;
7374

7475
const isPlanFile = filePath.includes("claude/plans/");
@@ -98,7 +99,7 @@ export function EditToolView({
9899
)}
99100
<StatusIndicators isFailed={isFailed} wasCancelled={wasCancelled} />
100101
</Flex>
101-
{newText && (
102+
{hasDiff && (
102103
<IconButton asChild size="1" variant="ghost" color="gray">
103104
<span>
104105
{isExpanded ? (
@@ -111,9 +112,9 @@ export function EditToolView({
111112
)}
112113
</button>
113114

114-
{isExpanded && newText && (
115+
{isExpanded && hasDiff && (
115116
<CodePreview
116-
content={newText}
117+
content={newText ?? ""}
117118
filePath={filePath}
118119
oldContent={isNewFile ? null : oldText}
119120
maxHeight="700px"

packages/agent/src/adapters/claude/conversion/tool-use-to-acp.ts

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import fs from "node:fs";
12
import path from "node:path";
23
import type {
34
PlanEntry,
@@ -179,18 +180,22 @@ export function toolInfoFromToolUse(
179180
: null;
180181
let newText: string = input?.new_string ? String(input.new_string) : "";
181182

182-
// If we have cached file content, show a full-file diff
183-
if (
184-
filePath &&
185-
options?.cachedFileContent &&
186-
filePath in options.cachedFileContent
187-
) {
188-
const oldContent = options.cachedFileContent[filePath];
189-
const newContent = input?.replace_all
190-
? oldContent.replaceAll(oldText ?? "", newText)
191-
: oldContent.replace(oldText ?? "", newText);
192-
oldText = oldContent;
193-
newText = newContent;
183+
// try to display a rich diff by first checking if file content is cached
184+
// and valid (old_text exists in the content), then fall back to reading
185+
// file from disk, then fall back to fragemented snippet diff
186+
if (filePath && oldText !== null) {
187+
const fileContent = resolveFileContent(
188+
filePath,
189+
oldText,
190+
options?.cachedFileContent,
191+
);
192+
if (fileContent) {
193+
const newContent = input?.replace_all
194+
? fileContent.replaceAll(oldText, newText)
195+
: fileContent.replace(oldText, newText);
196+
oldText = fileContent;
197+
newText = newContent;
198+
}
194199
}
195200

196201
return {
@@ -759,6 +764,37 @@ export function planEntries(input: { todos: ClaudePlanEntry[] }): PlanEntry[] {
759764
}));
760765
}
761766

767+
/**
768+
* attempt to resolve full file contents for diff generation
769+
*
770+
* 1) check file content cache exists, and is valid (old_text in content)
771+
* 2) if missing or invalid, read file from disk
772+
* 3) if both fail, return null, we'll fall back to fragmented snippet diff
773+
*/
774+
function resolveFileContent(
775+
filePath: string,
776+
oldText: string,
777+
cachedFileContent?: Record<string, string>,
778+
): string | null {
779+
if (cachedFileContent && filePath in cachedFileContent) {
780+
const cached = cachedFileContent[filePath];
781+
if (cached.includes(oldText)) {
782+
return cached;
783+
}
784+
}
785+
786+
try {
787+
const content = fs.readFileSync(filePath, "utf-8");
788+
if (content.includes(oldText)) {
789+
return content;
790+
}
791+
} catch {
792+
return null;
793+
}
794+
795+
return null;
796+
}
797+
762798
function markdownEscape(text: string): string {
763799
let escapedText = "```";
764800
for (const [m] of text.matchAll(/^```+/gm)) {

0 commit comments

Comments
 (0)