Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions packages/runtime/src/__tests__/builtin-tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2186,6 +2186,24 @@ describe('builtin write tools path containment', () => {
expect(await readFile(join(root, 'inside.txt'), 'utf8')).toBe('hello Maka');
});

test('Edit returns a file diff for a localized change in a large file', async () => {
const root = await mkdtemp(join(tmpdir(), 'maka-edit-large-diff-'));
const content = Array.from({ length: 900 }, (_, index) => `const v${index} = ${index};`).join(
'\n',
);
await writeFile(join(root, 'large.ts'), `${content}\n`, 'utf8');

const result = await runTool(
tool('Edit'),
{ path: 'large.ts', old_string: 'const v500 = 500;', new_string: 'const v500 = -1;' },
root,
);

expect(result).toMatchObject({ kind: 'file_diff' });
assert.match((result as { diff: string }).diff, /-const v500 = 500;/);
assert.match((result as { diff: string }).diff, /\+const v500 = -1;/);
});

test('file tools stay usable when the session cwd is reached through a symlink', async () => {
// The session cwd itself sits under a symlink (macOS hands out `/var/...`
// tmpdirs whose realpath is `/private/var/...`, and workspace roots are often
Expand Down
29 changes: 28 additions & 1 deletion packages/runtime/src/__tests__/filesystem-worker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ describe('filesystem worker operations', () => {
assert.equal(await readFile(target, 'utf8'), 'external');
});

test('omits the diff when the content is too large to diff cheaply', async () => {
test('returns a localized diff for a small Edit in a large file', async () => {
const root = await temporaryDirectory('maka-worker-huge-');
const target = join(root, 'huge.ts');
const before = Array.from({ length: 900 }, (_, i) => `const v${i} = ${i};`).join('\n') + '\n';
Expand All @@ -496,10 +496,37 @@ describe('filesystem worker operations', () => {
),
);

assert.ok(response.ok);
assert.equal(response.result.kind, 'edit');
if (response.result.kind !== 'edit') return;
assert.match(response.result.diff ?? '', /-const v0 = 0;/);
assert.match(response.result.diff ?? '', /\+const v0 = -1;/);
});

test('omits an oversized Edit diff while still applying the replacement', async () => {
const root = await temporaryDirectory('maka-worker-large-replacement-');
const target = join(root, 'large.ts');
await writeFile(target, 'const value = 1;\n', 'utf8');
const replacement = Array.from({ length: 900 }, (_, i) => `const value${i} = ${i};`).join('\n');

const response = await executeFilesystemWorkerRequest(
await requestFor(
{
kind: 'edit',
cwd: root,
path: target,
oldString: 'const value = 1;',
newString: replacement,
},
{ enforcementPath: target, access: 'write', scope: 'exact', targetType: 'file' },
),
);

assert.ok(response.ok);
assert.equal(response.result.kind, 'edit');
if (response.result.kind !== 'edit') return;
assert.equal(response.result.diff, undefined);
assert.equal(await readFile(target, 'utf8'), `${replacement}\n`);
});

test('omits the diff when FormatJson leaves the file unchanged', async () => {
Expand Down
16 changes: 7 additions & 9 deletions packages/runtime/src/filesystem-executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import type { PermissionMode } from '@maka/core/permission';
import type { PermissionProfile } from '@maka/core/permission-profile';
import { ToolOutcomeUnknownError } from '@maka/core/events';
import { computeEditedSource } from './edit-replace.js';
import { createUnifiedDiff } from './unified-diff.js';
import { createEditUnifiedDiff, createUnifiedDiff } from './unified-diff.js';
import {
classifyFailedMutationOutcome,
type FilesystemTargetIdentity,
Expand Down Expand Up @@ -501,27 +501,25 @@ function createWorkspaceFilesystemExecutor(
if (isSupportedImagePath(path)) throw new Error('Edit does not support image files.');
if (workspace.readModifyWrite) {
let edited!: ReturnType<typeof computeEditedSource>;
const result = await workspace.readModifyWrite({
let originalContent = '';
await workspace.readModifyWrite({
cwd,
path,
label: 'Edit',
scope,
approvedIdentity: expectedIdentity,
transform: (ctx) => {
originalContent = ctx.content ?? '';
edited = computeEditedSource(
ctx.content ?? '',
originalContent,
operation.oldString,
operation.newString,
operation.path,
);
return edited.content;
},
});
const diff = createUnifiedDiff(
path,
result.previous === 'unknown' || result.previous === 'new' ? '' : result.previous,
result.finalContent ?? '',
);
const diff = createEditUnifiedDiff(path, originalContent, edited.content, edited);
return {
kind: 'edit',
ok: true,
Expand All @@ -542,7 +540,7 @@ function createWorkspaceFilesystemExecutor(
operation.path,
);
await workspace.writeFile({ cwd, path, content: edited.content });
const diff = createUnifiedDiff(path, read.content, edited.content);
const diff = createEditUnifiedDiff(path, read.content, edited.content, edited);
return {
kind: 'edit',
ok: true,
Expand Down
4 changes: 2 additions & 2 deletions packages/runtime/src/filesystem-worker/operations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
} from '../apply-patch-file.js';

import { computeEditedSource } from '../edit-replace.js';
import { createUnifiedDiff } from '../unified-diff.js';
import { createEditUnifiedDiff, createUnifiedDiff } from '../unified-diff.js';
import {
compareAndDeleteEntry,
hostVisibilityAfterWrite,
Expand Down Expand Up @@ -293,7 +293,7 @@ export async function executeFilesystemOperation(
await writeThroughHandle(handle, source.content);
const visibility = await hostVisibilityAfterWrite(path, handle);
if (visibility) throw visibility;
const diff = createUnifiedDiff(path, content, source.content);
const diff = createEditUnifiedDiff(path, content, source.content, source);
return {
kind: 'edit',
ok: true,
Expand Down
53 changes: 50 additions & 3 deletions packages/runtime/src/unified-diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

const MAX_DIFF_SOURCE_LINES = 800;
const MAX_DIFF_SOURCE_BYTES = 32 * 1024;
const MAX_DIFF_OUTPUT_BYTES = 32 * 1024;
const CONTEXT_LINES = 3;

/**
Expand Down Expand Up @@ -57,10 +58,56 @@ export function createUnifiedDiff(
return [...header, ...formatHunks(ops)].join('\n');
}

/**
* Localized unified diff for Edit's one known replacement span. Unlike the
* generic full-file diff, its cost depends on the changed hunk rather than the
* file size; the final output remains bounded for huge lines or replacements.
*/
export function createEditUnifiedDiff(
path: string,
oldContent: string,
newContent: string,
match: { startLine: number; endLine: number },
): string | undefined {
if (oldContent.includes('\0') || newContent.includes('\0')) return undefined;
const oldLines = splitLines(oldContent);
const newLines = splitLines(newContent);
const startIndex = match.startLine - 1;
const oldEndIndex = match.endLine - 1;
if (startIndex < 0 || oldEndIndex < startIndex || oldEndIndex >= oldLines.length) {
return undefined;
}

const windowStart = Math.max(0, startIndex - CONTEXT_LINES);
const oldAfterStart = oldEndIndex + 1;
const newAfterStart = oldAfterStart + newLines.length - oldLines.length;
const oldWindow = oldLines.slice(
windowStart,
Math.min(oldLines.length, oldAfterStart + CONTEXT_LINES),
);
const newWindow = newLines.slice(
windowStart,
Math.min(newLines.length, Math.max(windowStart, newAfterStart) + CONTEXT_LINES),
);
if (isDiffWindowTooLarge(oldWindow) || isDiffWindowTooLarge(newWindow)) return undefined;
const ops = diffLines(oldWindow, newWindow);
if (ops.every((op) => op.kind === 'keep')) return undefined;
const diff = [
`--- a/${path}`,
`+++ b/${path}`,
...formatHunks(ops, windowStart, windowStart),
].join('\n');
return Buffer.byteLength(diff, 'utf8') <= MAX_DIFF_OUTPUT_BYTES ? diff : undefined;
}

function isUndiffable(content: string): boolean {
return content.includes('\0') || Buffer.byteLength(content, 'utf8') > MAX_DIFF_SOURCE_BYTES;
}

function isDiffWindowTooLarge(lines: string[]): boolean {
return lines.length > MAX_DIFF_SOURCE_LINES || isUndiffable(lines.join('\n'));
}

function splitLines(content: string): string[] {
const lines = content.split('\n');
// A trailing newline terminates the last line rather than starting an empty
Expand Down Expand Up @@ -113,7 +160,7 @@ function diffLines(oldLines: string[], newLines: string[]): DiffOp[] {
}

/** Group the edit script into hunks with CONTEXT_LINES of surrounding context. */
function formatHunks(ops: DiffOp[]): string[] {
function formatHunks(ops: DiffOp[], oldLineOffset = 0, newLineOffset = 0): string[] {
const changed = ops.flatMap((op, index) => (op.kind === 'keep' ? [] : [index]));
if (changed.length === 0) return [];

Expand All @@ -133,8 +180,8 @@ function formatHunks(ops: DiffOp[]): string[] {
const out: string[] = [];
for (const group of groups) {
const slice = ops.slice(group.start, group.end + 1);
const oldStart = slice[0].oldIndex + 1;
const newStart = slice[0].newIndex + 1;
const oldStart = slice[0].oldIndex + oldLineOffset + 1;
const newStart = slice[0].newIndex + newLineOffset + 1;
const oldCount = slice.filter((op) => op.kind !== 'add').length;
const newCount = slice.filter((op) => op.kind !== 'del').length;
out.push(
Expand Down
5 changes: 0 additions & 5 deletions packages/ui/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,6 @@ export { Card, type CardProps, type CardVariant } from '@astryxdesign/core';
// cross-package consumer — `apps/desktop`'s `artifact-preview.tsx` — which is the
// promotion condition the off-barrel convention named, so the export is the rule.
export { previewVariants } from './primitives/chat.js';
// `diffLineKind` rides the same seam for the same reason: it decides the
// `data-line` values those parts are selected by, so a second copy of it is a
// second answer to "what colour is this line". `apps/desktop` had one, and the
// two had already diverged on `diff --git` / `index` headers.
export { diffLineKind } from './tool-activity/tool-result-preview.js';
export { DiffCodePreview } from './tool-activity/diff-code-preview.js';
export { syntaxLanguageForPath } from './tool-activity/diff-syntax.js';
export { MarkdownBody } from './markdown-body.js';
Expand Down
22 changes: 0 additions & 22 deletions packages/ui/src/tool-activity/tool-result-preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -304,28 +304,6 @@ function PtyControlPreview(props: {
);
}

/**
* Which tint a unified-diff line takes. Deliberately shallow: it reads the
* line's first character, not the hunk semantics, which is all the colouring
* needs and all a preview should promise.
*
* `+++`/`---` are file markers, not an addition and a deletion — they have to
* be tested before the single-character cases or every diff opens with one
* green and one red line that mean nothing.
*/
export function diffLineKind(line: string): 'add' | 'del' | 'hunk' | 'meta' | 'ctx' {
// The trailing space is what separates a file marker from content: unified
// diff writes `--- a/path`, never a bare `---`. Without it, deleting a YAML
// document separator or an SQL `--` comment paints the removal as a header —
// the one line the reader most needs to see as red.
if (line.startsWith('--- ') || line.startsWith('+++ ')) return 'meta';
if (line.startsWith('@@')) return 'hunk';
if (line.startsWith('+')) return 'add';
if (line.startsWith('-')) return 'del';
if (line.startsWith('diff ') || line.startsWith('index ')) return 'meta';
return 'ctx';
}

/**
* Line-level diff colouring — green additions, red deletions, a tinted hunk
* header — in the same surface a command uses, with the changed paths as its
Expand Down