Skip to content

Commit 3edbb38

Browse files
Improve applyDiffs to handle failures robustly (onlook-dev#1663)
* Improve applyDiffs to handle failures robustly * Handle failing to apply code change --------- Co-authored-by: Kiet Ho <[email protected]>
1 parent 3651e3a commit 3edbb38

File tree

6 files changed

+106
-27
lines changed

6 files changed

+106
-27
lines changed

apps/studio/src/lib/editor/engine/chat/code.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { sendAnalytics } from '@/lib/utils';
33
import { CodeBlockProcessor } from '@onlook/ai';
44
import { ChatMessageRole, type AssistantChatMessage, type CodeBlock } from '@onlook/models/chat';
55
import type { CodeDiff } from '@onlook/models/code';
6+
import { toast } from '@onlook/ui/use-toast';
67
import { makeAutoObservable } from 'mobx';
78
import type { ChatManager } from '.';
89
import type { EditorEngine } from '..';
@@ -42,7 +43,16 @@ export class ChatCodeManager {
4243
}
4344
let content = originalContent;
4445
for (const block of codeBlocks) {
45-
content = await this.processor.applyDiff(content, block.content);
46+
const result = await this.processor.applyDiff(content, block.content);
47+
if (!result.success) {
48+
console.error('Failed to apply code block', block);
49+
toast({
50+
title: 'Failed to apply code block',
51+
variant: 'destructive',
52+
description: 'Please try again or prompt the AI to fix it.',
53+
});
54+
}
55+
content = result.text;
4656
}
4757

4858
const success = await this.writeFileContent(file, content, originalContent);

apps/studio/src/routes/editor/EditPanel/ChatTab/CodeChangeDisplay/CodeModal.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { useEditorEngine } from '@/components/Context';
22
import { Button } from '@onlook/ui/button';
3-
import { Dialog, DialogContent, DialogTrigger } from '@onlook/ui/dialog';
3+
import { Dialog, DialogContent, DialogTitle, DialogTrigger } from '@onlook/ui/dialog';
44
import { Icons } from '@onlook/ui/icons';
55
import { Tabs, TabsContent, TabsList, TabsTrigger } from '@onlook/ui/tabs';
66
import { useState } from 'react';
@@ -31,6 +31,7 @@ export default function CodeModal({
3131
<Dialog open={isOpen} onOpenChange={setIsOpen}>
3232
<DialogTrigger asChild>{children}</DialogTrigger>
3333
<DialogContent className="min-w-[90vw] h-[80vh]">
34+
<DialogTitle className="sr-only">{fileName}</DialogTitle>
3435
<Tabs value={selectedTab} onValueChange={(val) => setSelectedTab(val as TabValue)}>
3536
<TabsList className="bg-transparent w-full gap-2 justify-start">
3637
<TabsTrigger

packages/ai/src/coder/block.ts

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,6 @@ import { FENCE } from '../prompt/format';
33
import { flexibleSearchAndReplace } from './search-replace';
44

55
export class CodeBlockProcessor {
6-
/**
7-
* Sequentially applies a list of diffs to the original text
8-
*/
9-
async applyDiffs(originalText: string, diffs: string[]): Promise<string> {
10-
let text = originalText;
11-
for (const diff of diffs) {
12-
text = await this.applyDiff(text, diff);
13-
}
14-
return text;
15-
}
16-
176
/**
187
* Extracts search and replace content from a diff string using the defined fence markers
198
*/
@@ -58,21 +47,46 @@ export class CodeBlockProcessor {
5847
* Applies a search/replace diff to the original text with advanced formatting handling
5948
* Uses multiple strategies and preprocessing options to handle complex replacements
6049
*/
61-
async applyDiff(originalText: string, diffText: string): Promise<string> {
50+
async applyDiff(
51+
originalText: string,
52+
diffText: string,
53+
): Promise<{
54+
success: boolean;
55+
text: string;
56+
failures?: Array<{ search: string; error?: string }>;
57+
}> {
6258
const searchReplaces = CodeBlockProcessor.parseDiff(diffText);
6359
let text = originalText;
60+
const failures: Array<{ search: string; error?: string }> = [];
6461

6562
for (const { search, replace } of searchReplaces) {
6663
const result = await flexibleSearchAndReplace(search, replace, text);
67-
if (!result.success) {
68-
// Fallback to simple replacement if flexible strategies fail
69-
text = text.replace(search, replace);
70-
} else if (result.text) {
64+
if (result.success && result.text) {
7165
text = result.text;
66+
} else {
67+
// Fallback to simple replacement if flexible strategies fail
68+
try {
69+
const newText = text.replace(search, replace);
70+
if (newText !== text) {
71+
text = newText;
72+
} else {
73+
failures.push({ search, error: 'No changes made' });
74+
}
75+
} catch (error) {
76+
failures.push({
77+
search,
78+
error: error instanceof Error ? error.message : 'Unknown error',
79+
});
80+
console.warn('Simple replacement failed:', error);
81+
}
7282
}
7383
}
7484

75-
return text;
85+
return {
86+
success: failures.length === 0,
87+
text,
88+
...(failures.length > 0 && { failures }),
89+
};
7690
}
7791

7892
/**

packages/ai/test/coder/diffs/diffs.test.ts

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,40 @@ describe('Parse and Apply Code Block Diffs', () => {
2828
const afterText = await Bun.file(path.resolve(__dirname, './data/single/after.txt')).text();
2929

3030
const result = await coder.applyDiff(beforeText, diffText);
31-
expect(result.trim()).toBe(afterText.trim());
31+
expect(result.success).toBe(true);
32+
expect(result.text.trim()).toBe(afterText.trim());
33+
expect(result.failures).toBeUndefined();
34+
});
35+
36+
test('should handle failed replacements', async () => {
37+
const diffText = coder.createDiff('non-existent-text', 'replacement');
38+
const originalText = 'some sample text';
39+
40+
const result = await coder.applyDiff(originalText, diffText);
41+
expect(result.success).toBe(false);
42+
expect(result.text).toBe(originalText);
43+
expect(result.failures).toHaveLength(1);
44+
expect(result.failures![0]).toEqual({
45+
search: 'non-existent-text',
46+
error: 'No changes made',
47+
});
48+
});
49+
50+
test('should fail when any replacement fails in multiple diffs', async () => {
51+
const diffText =
52+
coder.createDiff('sample', 'example') +
53+
'\n' +
54+
coder.createDiff('non-existent', 'replacement');
55+
const originalText = 'some sample text';
56+
57+
const result = await coder.applyDiff(originalText, diffText);
58+
expect(result.success).toBe(false);
59+
expect(result.text).toBe('some example text');
60+
expect(result.failures).toHaveLength(1);
61+
expect(result.failures![0]).toEqual({
62+
search: 'non-existent',
63+
error: 'No changes made',
64+
});
3265
});
3366

3467
test('should create diff correctly', () => {

packages/ai/test/coder/search-replace.test.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,33 @@ describe('CodeBlockProcessor Integration', () => {
9999
const originalText = ' if (x) {\n oldFunc();\n }';
100100
const diffText = processor.createDiff(' oldFunc();', ' newFunc();');
101101
const result = await processor.applyDiff(originalText, diffText);
102-
expect(result).toBe(' if (x) {\n newFunc();\n }');
102+
expect(result.success).toBe(true);
103+
expect(result.text).toBe(' if (x) {\n newFunc();\n }');
104+
expect(result.failures).toBeUndefined();
103105
});
104106

105107
test('should fall back to simple replace if needed', async () => {
106108
const originalText = 'simple old text';
107109
const diffText = processor.createDiff('old', 'new');
108110
const result = await processor.applyDiff(originalText, diffText);
109-
expect(result).toBe('simple new text');
111+
expect(result.success).toBe(true);
112+
expect(result.text).toBe('simple new text');
113+
expect(result.failures).toBeUndefined();
114+
});
115+
116+
test('should mark as failed if any replacement fails', async () => {
117+
const originalText = 'simple old text';
118+
const diffText =
119+
processor.createDiff('old', 'new') +
120+
'\n' +
121+
processor.createDiff('missing', 'replacement');
122+
const result = await processor.applyDiff(originalText, diffText);
123+
expect(result.success).toBe(false); // Should be false even though one replacement succeeded
124+
expect(result.text).toBe('simple new text');
125+
expect(result.failures).toHaveLength(1);
126+
expect(result.failures![0]).toEqual({
127+
search: 'missing',
128+
error: 'No changes made',
129+
});
110130
});
111131
});

packages/ui/src/components/command.tsx

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
import * as React from 'react';
21
import type { DialogProps } from '@radix-ui/react-dialog';
32
import { MagnifyingGlassIcon } from '@radix-ui/react-icons';
43
import { Command as CommandPrimitive } from 'cmdk';
4+
import * as React from 'react';
55

66
import { cn } from '../utils';
7-
import { Dialog, DialogContent } from './dialog';
7+
import { Dialog, DialogContent, DialogTitle } from './dialog';
88

99
const Command = React.forwardRef<
1010
React.ElementRef<typeof CommandPrimitive>,
@@ -27,6 +27,7 @@ const CommandDialog = ({ children, ...props }: CommandDialogProps) => {
2727
return (
2828
<Dialog {...props}>
2929
<DialogContent className="overflow-hidden p-0">
30+
<DialogTitle className="sr-only">Command</DialogTitle>
3031
<Command className="[&_[cmdk-group-heading]]:px-2 [&_[cmdk-group-heading]]:font-medium [&_[cmdk-group-heading]]:text-muted-foreground [&_[cmdk-group]:not([hidden])_~[cmdk-group]]:pt-0 [&_[cmdk-group]]:px-2 [&_[cmdk-input-wrapper]_svg]:h-5 [&_[cmdk-input-wrapper]_svg]:w-5 [&_[cmdk-input]]:h-12 [&_[cmdk-item]]:px-2 [&_[cmdk-item]]:py-3 [&_[cmdk-item]_svg]:h-5 [&_[cmdk-item]_svg]:w-5">
3132
{children}
3233
</Command>
@@ -133,11 +134,11 @@ CommandShortcut.displayName = 'CommandShortcut';
133134
export {
134135
Command,
135136
CommandDialog,
136-
CommandInput,
137-
CommandList,
138137
CommandEmpty,
139138
CommandGroup,
139+
CommandInput,
140140
CommandItem,
141-
CommandShortcut,
141+
CommandList,
142142
CommandSeparator,
143+
CommandShortcut,
143144
};

0 commit comments

Comments
 (0)