diff --git a/loop/predictable.go b/loop/predictable.go index b93c2cf23496f3390cceee73e2d6cd5baf7c362b..8dcf8401859489a1ecdcfb0113b58aa5c9577eae 100644 --- a/loop/predictable.go +++ b/loop/predictable.go @@ -120,6 +120,10 @@ func (s *PredictableService) Do(ctx context.Context, req *llm.Request) (*llm.Res // Trigger a patch that will fail (file doesn't exist) return s.makePatchToolResponse("/nonexistent/file/that/does/not/exist.txt", inputTokens), nil + case "patch success": + // Trigger a patch that will succeed (using overwrite, which creates the file) + return s.makePatchToolResponseOverwrite("/tmp/test-patch-success.txt", inputTokens), nil + case "patch bad json": // Trigger a patch with malformed JSON (simulates Anthropic sending invalid JSON) return s.makeMalformedPatchToolResponse(inputTokens), nil @@ -336,6 +340,47 @@ func (s *PredictableService) makePatchToolResponse(filePath string, inputTokens } } +// makePatchToolResponseOverwrite creates a response that uses overwrite operation (always succeeds) +func (s *PredictableService) makePatchToolResponseOverwrite(filePath string, inputTokens uint64) *llm.Response { + toolInputData := map[string]interface{}{ + "path": filePath, + "patches": []map[string]string{ + { + "operation": "overwrite", + "newText": "This is the new content of the file.\nLine 2\nLine 3\n", + }, + }, + } + toolInputBytes, _ := json.Marshal(toolInputData) + toolInput := json.RawMessage(toolInputBytes) + responseText := fmt.Sprintf("I'll create/overwrite the file: %s", filePath) + outputTokens := uint64(len(responseText)/4 + len(toolInputBytes)/4) + if outputTokens == 0 { + outputTokens = 1 + } + return &llm.Response{ + ID: fmt.Sprintf("pred-patch-overwrite-%d", time.Now().UnixNano()), + Type: "message", + Role: llm.MessageRoleAssistant, + Model: "predictable-v1", + Content: []llm.Content{ + {Type: llm.ContentTypeText, Text: responseText}, + { + ID: fmt.Sprintf("tool_%d", time.Now().UnixNano()%1000), + Type: llm.ContentTypeToolUse, + ToolName: "patch", + ToolInput: toolInput, + }, + }, + StopReason: llm.StopReasonToolUse, + Usage: llm.Usage{ + InputTokens: inputTokens, + OutputTokens: outputTokens, + CostUSD: 0.0, + }, + } +} + // makeMalformedPatchToolResponse creates a response with malformed JSON that will fail to parse // This simulates when Anthropic sends back invalid JSON in the tool input func (s *PredictableService) makeMalformedPatchToolResponse(inputTokens uint64) *llm.Response { diff --git a/ui/e2e/tool-components.spec.ts b/ui/e2e/tool-components.spec.ts index 5a0fd2d50c29eb8eb780557520c856439817bee1..1f67d1786144eee0286a1c8aebc6fa640d046c88 100644 --- a/ui/e2e/tool-components.spec.ts +++ b/ui/e2e/tool-components.spec.ts @@ -145,6 +145,53 @@ test.describe('Tool Component Verification', () => { await expect(navigateTool.locator('.tool-command').filter({ hasText: 'https://example.com' })).toBeVisible(); }); + test('patch tool can be collapsed and expanded without errors', async ({ page }) => { + await page.goto('/'); + await page.waitForLoadState('domcontentloaded'); + + const messageInput = page.getByTestId('message-input'); + const sendButton = page.getByTestId('send-button'); + + // Trigger a successful patch tool (uses overwrite operation which always succeeds) + await messageInput.fill('patch success'); + await sendButton.click(); + + // Wait for successful patch tool with Monaco editor + // Use specific locator to find the successful patch (not the failed ones from other tests) + const patchTool = page.locator('.patch-tool[data-testid="tool-call-completed"]').filter({ hasText: 'test-patch-success.txt' }).first(); + await expect(patchTool).toBeVisible({ timeout: 30000 }); + // Wait for Monaco editor to be fully rendered (only visible for successful patches) + await expect(patchTool.locator('.patch-tool-monaco-editor')).toBeVisible({ timeout: 10000 }); + + // Get console errors before toggling + const errors: string[] = []; + page.on('pageerror', (error) => errors.push(error.message)); + + const header = patchTool.locator('.patch-tool-header'); + + // Collapse + await header.click(); + await expect(patchTool.locator('.patch-tool-details')).toBeHidden(); + + // Expand - Monaco should reinitialize + await header.click(); + await expect(patchTool.locator('.patch-tool-details')).toBeVisible(); + await expect(patchTool.locator('.patch-tool-monaco-editor')).toBeVisible({ timeout: 10000 }); + + // Collapse again + await header.click(); + await expect(patchTool.locator('.patch-tool-details')).toBeHidden(); + + // Expand again - this was triggering "Cannot add model because it already exists!" in Firefox + await header.click(); + await expect(patchTool.locator('.patch-tool-details')).toBeVisible(); + await expect(patchTool.locator('.patch-tool-monaco-editor')).toBeVisible({ timeout: 10000 }); + + // Check no Monaco model errors occurred + const modelErrors = errors.filter(e => e.includes('model') && e.includes('already exists')); + expect(modelErrors).toHaveLength(0); + }); + test('emoji sizes are consistent across all tools', async ({ page }) => { await page.goto('/'); await page.waitForLoadState('domcontentloaded'); diff --git a/ui/src/components/PatchTool.tsx b/ui/src/components/PatchTool.tsx index 52e5682afa3c623ce1dbca657aa7e81d67bca8b1..3db864435103b30cd50680c90a6b1f511a0af241 100644 --- a/ui/src/components/PatchTool.tsx +++ b/ui/src/components/PatchTool.tsx @@ -86,6 +86,13 @@ function PatchTool({ const monacoRef = useRef(null); const commentInputRef = useRef(null); const hoverDecorationsRef = useRef([]); + const modelsRef = useRef<{ + original: Monaco.editor.ITextModel | null; + modified: Monaco.editor.ITextModel | null; + }>({ + original: null, + modified: null, + }); // Track viewport size useEffect(() => { @@ -154,11 +161,19 @@ function PatchTool({ const monaco = monacoRef.current; - // Dispose previous editor + // Dispose previous editor and models if (editorRef.current) { editorRef.current.dispose(); editorRef.current = null; } + if (modelsRef.current.original) { + modelsRef.current.original.dispose(); + modelsRef.current.original = null; + } + if (modelsRef.current.modified) { + modelsRef.current.modified.dispose(); + modelsRef.current.modified = null; + } // Get language from file extension const ext = "." + (displayData.path.split(".").pop()?.toLowerCase() || ""); @@ -176,8 +191,15 @@ function PatchTool({ const originalUri = monaco.Uri.file(`patch-original-${timestamp}-${displayData.path}`); const modifiedUri = monaco.Uri.file(`patch-modified-${timestamp}-${displayData.path}`); + // Check for and dispose any existing models with these URIs (defensive, shouldn't happen) + const existingOriginal = monaco.editor.getModel(originalUri); + if (existingOriginal) existingOriginal.dispose(); + const existingModified = monaco.editor.getModel(modifiedUri); + if (existingModified) existingModified.dispose(); + const originalModel = monaco.editor.createModel(displayData.oldContent, language, originalUri); const modifiedModel = monaco.editor.createModel(displayData.newContent, language, modifiedUri); + modelsRef.current = { original: originalModel, modified: modifiedModel }; // Create diff editor const diffEditor = monaco.editor.createDiffEditor(editorContainerRef.current, { @@ -294,6 +316,14 @@ function PatchTool({ editorRef.current.dispose(); editorRef.current = null; } + if (modelsRef.current.original) { + modelsRef.current.original.dispose(); + modelsRef.current.original = null; + } + if (modelsRef.current.modified) { + modelsRef.current.modified.dispose(); + modelsRef.current.modified = null; + } }; }, [monacoLoaded, displayData, isMobile, isExpanded, onCommentTextChange]);