From 469c0c8e73965e2c98d277e846e56a12b1e47e65 Mon Sep 17 00:00:00 2001 From: Dallin Romney Date: Sat, 23 Aug 2025 16:37:18 -0700 Subject: [PATCH 1/4] feat: UI for diff tools --- gui/package-lock.json | 20 +- gui/package.json | 2 + .../StepContainerPreToolbar/ApplyActions.tsx | 2 +- .../StepContainerPreToolbar/FileInfo.tsx | 3 +- .../StepContainerPreToolbar/index.tsx | 2 +- .../gui/ToolCallDiv/FindAndReplace.test.tsx | 209 ++++++++++++++ .../pages/gui/ToolCallDiv/FindAndReplace.tsx | 269 ++++++++++++++++++ .../FunctionSpecificToolCallDiv.tsx | 42 ++- gui/src/redux/slices/sessionSlice.ts | 16 ++ gui/src/redux/thunks/enhanceParsedArgs.ts | 53 ++++ gui/src/redux/thunks/streamNormalInput.ts | 15 + gui/src/util/clientTools/multiEditImpl.ts | 6 +- gui/src/util/clientTools/searchReplaceImpl.ts | 5 +- .../clientTools/singleFindAndReplaceImpl.ts | 11 +- 14 files changed, 620 insertions(+), 35 deletions(-) create mode 100644 gui/src/pages/gui/ToolCallDiv/FindAndReplace.test.tsx create mode 100644 gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx create mode 100644 gui/src/redux/thunks/enhanceParsedArgs.ts diff --git a/gui/package-lock.json b/gui/package-lock.json index a3dffda4cd5..8e4bfb7d9d6 100644 --- a/gui/package-lock.json +++ b/gui/package-lock.json @@ -27,10 +27,12 @@ "@tiptap/react": "^2.1.13", "@tiptap/starter-kit": "^2.1.13", "@tiptap/suggestion": "^2.1.13", + "@types/diff": "^7.0.2", "@types/uuid": "^10.0.0", "anser": "^2.3.2", "clsx": "^2.1.1", "core": "file:../core", + "diff": "^8.0.2", "dompurify": "^3.0.6", "downshift": "^7.6.0", "escape-carriage": "^1.3.1", @@ -129,7 +131,6 @@ "@sentry/esbuild-plugin": "^4.0.2", "@sentry/node": "^9.43.0", "@sentry/vite-plugin": "^4.0.2", - "@typescript-eslint/eslint-plugin": "^7.8.0", "@xenova/transformers": "2.14.0", "adf-to-md": "^1.1.0", "async-mutex": "^0.5.0", @@ -209,6 +210,8 @@ "@types/tar": "^6.1.13", "@types/uuid": "^9.0.7", "@types/win-ca": "^3.5.4", + "@typescript-eslint/eslint-plugin": "^8.40.0", + "@typescript-eslint/parser": "^8.40.0", "cross-env": "^7.0.3", "esbuild": "0.17.19", "eslint": "^8", @@ -3865,6 +3868,12 @@ "@types/ms": "*" } }, + "node_modules/@types/diff": { + "version": "7.0.2", + "resolved": "https://registry.npmjs.org/@types/diff/-/diff-7.0.2.tgz", + "integrity": "sha512-JSWRMozjFKsGlEjiiKajUjIJVKuKdE3oVy2DNtK+fUo8q82nhFZ2CPQwicAIkXrofahDXrWJ7mjelvZphMS98Q==", + "license": "MIT" + }, "node_modules/@types/dompurify": { "version": "3.2.0", "resolved": "https://registry.npmjs.org/@types/dompurify/-/dompurify-3.2.0.tgz", @@ -6194,6 +6203,15 @@ "integrity": "sha512-gxtyfqMg7GKyhQmb056K7M3xszy/myH8w+B4RT+QXBQsvAOdc3XymqDDPHx1BgPgsdAA5SIifona89YtRATDzw==", "dev": true }, + "node_modules/diff": { + "version": "8.0.2", + "resolved": "https://registry.npmjs.org/diff/-/diff-8.0.2.tgz", + "integrity": "sha512-sSuxWU5j5SR9QQji/o2qMvqRNYRDOcBTgsJ/DeCf4iSN4gW+gNMXM7wFIP+fdXZxoNiAnHUTGjCr+TSWXdRDKg==", + "license": "BSD-3-Clause", + "engines": { + "node": ">=0.3.1" + } + }, "node_modules/direction": { "version": "1.0.4", "resolved": "https://registry.npmjs.org/direction/-/direction-1.0.4.tgz", diff --git a/gui/package.json b/gui/package.json index ab83efb22fd..6cb45030c9c 100644 --- a/gui/package.json +++ b/gui/package.json @@ -36,10 +36,12 @@ "@tiptap/react": "^2.1.13", "@tiptap/starter-kit": "^2.1.13", "@tiptap/suggestion": "^2.1.13", + "@types/diff": "^7.0.2", "@types/uuid": "^10.0.0", "anser": "^2.3.2", "clsx": "^2.1.1", "core": "file:../core", + "diff": "^8.0.2", "dompurify": "^3.0.6", "downshift": "^7.6.0", "escape-carriage": "^1.3.1", diff --git a/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/ApplyActions.tsx b/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/ApplyActions.tsx index 1214b9e2425..9b7ba953d2a 100644 --- a/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/ApplyActions.tsx +++ b/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/ApplyActions.tsx @@ -11,7 +11,7 @@ interface ApplyActionsProps { applyState?: ApplyState; onClickAccept: () => void; onClickReject: () => void; - onClickApply: () => void; + onClickApply?: () => void; } export function ApplyActions(props: ApplyActionsProps) { diff --git a/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/FileInfo.tsx b/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/FileInfo.tsx index 550d1fd284e..c288c7cd095 100644 --- a/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/FileInfo.tsx +++ b/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/FileInfo.tsx @@ -1,10 +1,11 @@ import { getLastNPathParts } from "core/util/uri"; +import { MouseEventHandler } from "react"; import FileIcon from "../../FileIcon"; import { ToolTip } from "../../gui/Tooltip"; export interface FileInfoProps { filepath: string; - onClick?: () => void; + onClick?: MouseEventHandler; range?: string; } diff --git a/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/index.tsx b/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/index.tsx index e97c13061f6..f9d268ccc3f 100644 --- a/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/index.tsx +++ b/gui/src/components/StyledMarkdownPreview/StepContainerPreToolbar/index.tsx @@ -96,7 +96,7 @@ export function StepContainerPreToolbar({ toolCallState?.status === "errored" || toolCallState?.status === "done") ? (
{ if (toolCallState.output) { ideMessenger.post("showVirtualFile", { diff --git a/gui/src/pages/gui/ToolCallDiv/FindAndReplace.test.tsx b/gui/src/pages/gui/ToolCallDiv/FindAndReplace.test.tsx new file mode 100644 index 00000000000..3e912bd66a8 --- /dev/null +++ b/gui/src/pages/gui/ToolCallDiv/FindAndReplace.test.tsx @@ -0,0 +1,209 @@ +import { configureStore } from "@reduxjs/toolkit"; +import { render, screen } from "@testing-library/react"; +import { ApplyState } from "core"; +import { Provider } from "react-redux"; +import { describe, expect, it, Mock, vi } from "vitest"; +import { useFileContent } from "../../../hooks/useFileContent"; +import { SingleFindAndReplace } from "./FindAndReplace"; + +// Mock the useFileContent hook +vi.mock("../../../hooks/useFileContent"); + +// Mock the selectApplyStateByToolCallId selector +vi.mock("../../../redux/selectors/selectToolCalls", () => ({ + selectApplyStateByToolCallId: vi.fn(), +})); + +const mockUseFileContent = useFileContent as Mock; + +// Create a minimal store for testing +const createTestStore = (applyState?: ApplyState) => { + return configureStore({ + reducer: { + session: () => ({ + codeBlockApplyStates: { + states: applyState ? [applyState] : [], + }, + }), + }, + }); +}; + +describe("SingleFindAndReplace", () => { + const defaultProps = { + relativeFilePath: "test.ts", + oldString: "old code", + newString: "new code", + replaceAll: false, + toolCallId: "test-tool-call-id", + historyIndex: 0, + }; + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("shows loading state when file content is loading", () => { + mockUseFileContent.mockReturnValue({ + fileContent: null, + isLoading: true, + error: null, + }); + + const store = createTestStore(); + render( + + + , + ); + + expect(screen.getByText("Loading file content...")).toBeInTheDocument(); + }); + + it("shows error when file content fails to load", () => { + mockUseFileContent.mockReturnValue({ + fileContent: null, + isLoading: false, + error: "File not found", + }); + + const store = createTestStore(); + render( + + + , + ); + + expect( + screen.getByText(`Failed to load file: ${defaultProps.relativeFilePath}`), + ).toBeInTheDocument(); + }); + + it("shows error when find and replace operation fails", () => { + mockUseFileContent.mockReturnValue({ + fileContent: "some content without the old string", + isLoading: false, + error: null, + }); + + const store = createTestStore(); + render( + + + , + ); + + expect(screen.getByText(/Error:/)).toBeInTheDocument(); + expect(screen.getByText(/String not found in file/)).toBeInTheDocument(); + }); + + it("shows no changes message when old and new strings result in same content", () => { + mockUseFileContent.mockReturnValue({ + fileContent: "some content", + isLoading: false, + error: null, + }); + + const store = createTestStore(); + render( + + + , + ); + + // This should show an error since the string doesn't exist + expect(screen.getByText(/Error:/)).toBeInTheDocument(); + }); + + it("shows diff when find and replace operation succeeds", () => { + const fileContent = "Hello old code world"; + mockUseFileContent.mockReturnValue({ + fileContent, + isLoading: false, + error: null, + }); + + const store = createTestStore(); + render( + + + , + ); + + expect(screen.getByText(defaultProps.relativeFilePath)).toBeInTheDocument(); + // The diff should show the old line being removed and new line being added + expect(screen.getByText("Hello old code world")).toBeInTheDocument(); + expect(screen.getByText("Hello new code world")).toBeInTheDocument(); + }); + + it("shows apply state status when available", () => { + mockUseFileContent.mockReturnValue({ + fileContent: "Hello old code world", + isLoading: false, + error: null, + }); + + const applyState: ApplyState = { + streamId: "test-stream", + status: "streaming", + toolCallId: "test-tool-call-id", + }; + + const store = createTestStore(applyState); + render( + + + , + ); + + expect(screen.getByText("Applying...")).toBeInTheDocument(); + }); + + it("shows tool call status icon when enabled", () => { + mockUseFileContent.mockReturnValue({ + fileContent: "Hello old code world", + isLoading: false, + error: null, + }); + + const store = createTestStore(); + render( + + + , + ); + + // Look for the status indicator (green dot for done status) + const statusIndicator = document.querySelector(".bg-green-500"); + expect(statusIndicator).toBeInTheDocument(); + }); + + it("handles replace all option correctly", () => { + const fileContent = "old code and more old code"; + mockUseFileContent.mockReturnValue({ + fileContent, + isLoading: false, + error: null, + }); + + const store = createTestStore(); + render( + + + , + ); + + // Should show both replacements in the diff + expect(screen.getByText(defaultProps.relativeFilePath)).toBeInTheDocument(); + expect(screen.getByText("old code and more old code")).toBeInTheDocument(); + expect(screen.getByText("new code and more new code")).toBeInTheDocument(); + }); +}); diff --git a/gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx b/gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx new file mode 100644 index 00000000000..55cf27ae31e --- /dev/null +++ b/gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx @@ -0,0 +1,269 @@ +import { ChevronDownIcon } from "@heroicons/react/24/outline"; +import { ApplyState } from "core"; +import { EditOperation } from "core/tools/definitions/multiEdit"; +import { renderContextItems } from "core/util/messageContent"; +import { getLastNPathParts, getUriPathBasename } from "core/util/uri"; +import { diffLines } from "diff"; +import { useContext, useMemo, useState } from "react"; +import { ApplyActions } from "../../../components/StyledMarkdownPreview/StepContainerPreToolbar/ApplyActions"; +import { FileInfo } from "../../../components/StyledMarkdownPreview/StepContainerPreToolbar/FileInfo"; +import { useFontSize } from "../../../components/ui"; +import { IdeMessengerContext } from "../../../context/IdeMessenger"; +import { useAppSelector } from "../../../redux/hooks"; +import { + selectApplyStateByToolCallId, + selectToolCallById, +} from "../../../redux/selectors/selectToolCalls"; +import { performFindAndReplace } from "../../../util/clientTools/findAndReplaceUtils"; +import { getStatusIcon } from "./utils"; + +interface FindAndReplaceDisplayProps { + fileUri?: string; + editingFileContents?: string; + relativeFilePath?: string; + edits: EditOperation[]; + toolCallId: string; + historyIndex: number; +} + +export function FindAndReplaceDisplay({ + fileUri, + relativeFilePath, + editingFileContents, + edits, + toolCallId, + historyIndex, +}: FindAndReplaceDisplayProps) { + const [isExpanded, setIsExpanded] = useState(false); + const ideMessenger = useContext(IdeMessengerContext); + const applyState: ApplyState | undefined = useAppSelector((state) => + selectApplyStateByToolCallId(state, toolCallId), + ); + const fontSize = useFontSize(); + const toolCallState = useAppSelector((state) => + selectToolCallById(state, toolCallId), + ); + const config = useAppSelector((state) => state.config.config); + + const displayName = useMemo(() => { + if (fileUri) { + return getUriPathBasename(fileUri); + } + if (relativeFilePath) { + return getLastNPathParts(relativeFilePath, 1); + } + return ""; + }, [fileUri, relativeFilePath]); + + // Get file content from tool call state instead of reading file + const currentFileContent = useMemo(() => { + if (editingFileContents) { + return editingFileContents; + } + return edits.map((edit) => edit.old_string).join("\n"); + }, [editingFileContents, edits]); + + const diffResult = useMemo(() => { + if (!currentFileContent) { + return null; + } + + try { + // Apply all edits sequentially + let newContent = currentFileContent; + for (let i = 0; i < edits.length; i++) { + const { + old_string: oldString, + new_string: newString, + replace_all: replaceAll, + } = edits[i]; + newContent = performFindAndReplace( + newContent, + oldString, + newString, + replaceAll, + i, + ); + } + + // Generate diff between original and final content + const diff = diffLines(currentFileContent, newContent); + return { diff, newContent, error: null }; + } catch (error) { + return { + diff: null, + newContent: null, + error: error instanceof Error ? error.message : "Unknown error", + }; + } + }, [currentFileContent, edits]); + + const statusIcon = useMemo(() => { + const status = toolCallState?.status; + if (status) { + if (status === "canceled" || status === "errored" || status === "done") { + return ( +
{ + if (toolCallState.output) { + e.stopPropagation(); + ideMessenger.post("showVirtualFile", { + name: "Edit output", + content: renderContextItems(toolCallState.output), + }); + } + }} + > + {getStatusIcon(toolCallState.status)} +
+ ); + } + } + }, [toolCallState?.status]); + + // Unified container component that always renders the same structure + const renderContainer = (content: React.ReactNode) => ( +
+
{ + setIsExpanded((prev) => !prev); + }} + > +
+ {statusIcon} + + { + if (!fileUri) { + return; + } + e.stopPropagation(); + ideMessenger.post("openFile", { path: fileUri }); + }} + /> +
+ + {applyState && ( + { + ideMessenger.post(`acceptDiff`, { + filepath: fileUri, + streamId: applyState.streamId, + }); + }} + onClickReject={() => { + ideMessenger.post(`rejectDiff`, { + filepath: fileUri, + streamId: applyState.streamId, + }); + }} + disableManualApply={true} + applyState={applyState} + /> + )} +
+ {toolCallState?.status === "generated" || isExpanded ? content : null} +
+ ); + + if (diffResult?.error) { + return renderContainer( +
+ Error generating diff {diffResult.error} +
, + ); + } + + if (!diffResult?.diff) { + return renderContainer( +
No changes to display
, + ); + } + + return renderContainer( +
+ 
+        {diffResult.diff.map((part, index) => {
+          if (part.removed) {
+            return (
+              

+                {part.value.split("\n").map((line, lineIndex) => {
+                  if (
+                    line === "" &&
+                    lineIndex === part.value.split("\n").length - 1
+                  )
+                    return null;
+                  return (
+                    

+                      -
+                      {line}
+                    

+                  );
+                })}
+              

+            );
+          } else if (part.added) {
+            return (
+              

+                {part.value.split("\n").map((line, lineIndex) => {
+                  if (
+                    line === "" &&
+                    lineIndex === part.value.split("\n").length - 1
+                  )
+                    return null;
+                  return (
+                    

+                      +
+                      {line}
+                    

+                  );
+                })}
+              

+            );
+          } else {
+            return (
+              

+                {part.value.split("\n").map((line, lineIndex) => {
+                  if (
+                    line === "" &&
+                    lineIndex === part.value.split("\n").length - 1
+                  )
+                    return null;
+                  return (
+                    

+                      
+                        {" "}
+                      
+                      {line}
+                    

+                  );
+                })}
+              

+            );
+          }
+        })}
+
+
, + ); +} diff --git a/gui/src/pages/gui/ToolCallDiv/FunctionSpecificToolCallDiv.tsx b/gui/src/pages/gui/ToolCallDiv/FunctionSpecificToolCallDiv.tsx index ac74802a25d..0fcdc0fc16e 100644 --- a/gui/src/pages/gui/ToolCallDiv/FunctionSpecificToolCallDiv.tsx +++ b/gui/src/pages/gui/ToolCallDiv/FunctionSpecificToolCallDiv.tsx @@ -1,7 +1,9 @@ import { ToolCallState } from "core"; import { BuiltInToolNames } from "core/tools/builtIn"; +import { EditOperation } from "core/tools/definitions/multiEdit"; import { CreateFile } from "./CreateFile"; import { EditFile } from "./EditFile"; +import { FindAndReplaceDisplay } from "./FindAndReplace"; import { RunTerminalCommand } from "./RunTerminalCommand"; function FunctionSpecificToolCallDiv({ @@ -50,40 +52,30 @@ function FunctionSpecificToolCallDiv({ /> ); case BuiltInToolNames.SingleFindAndReplace: - const fakeSearchReplaceBlock = `===== SEARCH -${args?.old_string ?? ""} -===== -${args?.new_string ?? ""} -===== REPLACE`; - + const edits: EditOperation[] = [ + { + old_string: args?.old_string ?? "", + new_string: args?.new_string ?? "", + replace_all: args?.replace_all, + }, + ]; return ( - ); case BuiltInToolNames.MultiEdit: - const fakeSearchReplaceBlocks = - (args?.edits as { old_string: string; new_string: string }[]) - ?.map( - (edit) => `===== SEARCH -${edit?.old_string ?? ""} -===== -${edit?.new_string ?? ""} -===== REPLACE`, - ) - ?.join("\n\n---\n\n") ?? ""; - return ( - diff --git a/gui/src/redux/slices/sessionSlice.ts b/gui/src/redux/slices/sessionSlice.ts index 3ec2fe27b87..7251b04ac58 100644 --- a/gui/src/redux/slices/sessionSlice.ts +++ b/gui/src/redux/slices/sessionSlice.ts @@ -849,6 +849,21 @@ export const sessionSlice = createSlice({ ); } }, + setToolCallArgs: ( + state, + action: PayloadAction<{ + toolCallId: string; + newArgs: Record; + }>, + ) => { + const toolCallState = findToolCallById( + state.history, + action.payload.toolCallId, + ); + if (toolCallState) { + toolCallState.parsedArgs = action.payload.newArgs; + } + }, cancelToolCall: ( state, action: PayloadAction<{ @@ -1024,6 +1039,7 @@ export const { acceptToolCall, setToolGenerated, updateToolCallOutput, + setToolCallArgs, setMode, setIsSessionMetadataLoading, setAllSessionMetadata, diff --git a/gui/src/redux/thunks/enhanceParsedArgs.ts b/gui/src/redux/thunks/enhanceParsedArgs.ts new file mode 100644 index 00000000000..75141abb8bc --- /dev/null +++ b/gui/src/redux/thunks/enhanceParsedArgs.ts @@ -0,0 +1,53 @@ +import { BuiltInToolNames } from "core/tools/builtIn"; +import { resolveRelativePathInDir } from "core/util/ideUtils"; +import { getUriPathBasename } from "core/util/uri"; +import { IIdeMessenger } from "../../context/IdeMessenger"; +import { setToolCallArgs } from "../slices/sessionSlice"; +import { AppThunkDispatch } from "../store"; + +export async function enhanceParsedArgs( + ideMessenger: IIdeMessenger, + dispatch: AppThunkDispatch, + toolName: string | undefined, + toolCallId: string, + currentArgs: undefined | Record, +) { + // Add file content to parsedArgs for find/replace tools + let enhancedArgs = { ...currentArgs }; + if ( + (toolName === BuiltInToolNames.SingleFindAndReplace || + toolName === BuiltInToolNames.MultiEdit || + toolName === BuiltInToolNames.SearchAndReplaceInFile) && + currentArgs?.filepath && + !currentArgs?.editingFileContents + ) { + try { + const fileUri = await resolveRelativePathInDir( + currentArgs.filepath, + ideMessenger.ide, + ); + if (!fileUri) { + throw new Error(`File ${currentArgs.filepath} not found`); + } + const baseName = getUriPathBasename(fileUri); + const fileContent = await ideMessenger.ide.readFile(fileUri); + enhancedArgs = { + ...currentArgs, + fileUri, + baseName, + editingFileContents: fileContent, + }; + dispatch( + setToolCallArgs({ + toolCallId, + newArgs: enhancedArgs, + }), + ); + } catch (error) { + // If we can't read the file, let the tool handle the error + console.warn( + `Failed to enhance args: failed to read file ${currentArgs?.filepath}`, + ); + } + } +} diff --git a/gui/src/redux/thunks/streamNormalInput.ts b/gui/src/redux/thunks/streamNormalInput.ts index b17cf3d2992..6adfaf19910 100644 --- a/gui/src/redux/thunks/streamNormalInput.ts +++ b/gui/src/redux/thunks/streamNormalInput.ts @@ -28,6 +28,7 @@ import { selectCurrentToolCalls } from "../selectors/selectToolCalls"; import { DEFAULT_TOOL_SETTING } from "../slices/uiSlice"; import { getBaseSystemMessage } from "../util/getBaseSystemMessage"; import { callToolById } from "./callToolById"; +import { enhanceParsedArgs } from "./enhanceParsedArgs"; /** * Handles the execution of tool calls that may be automatically accepted. * Sets all tools as generated first, then executes auto-approved tool calls. @@ -253,6 +254,20 @@ export const streamNormalInput = createAsyncThunk< const newState = getState(); const toolSettings = newState.ui.toolSettings; const allToolCallStates = selectCurrentToolCalls(newState); + + console.log("ADDING EDITNG CURRENT FILE"); + await Promise.all( + allToolCallStates.map((tcState) => + enhanceParsedArgs( + extra.ideMessenger, + dispatch, + tcState?.toolCall.function.name, + tcState.toolCallId, + tcState.parsedArgs, + ), + ), + ); + const generatingToolCalls = allToolCallStates.filter( (toolCallState) => toolCallState.status === "generating", ); diff --git a/gui/src/util/clientTools/multiEditImpl.ts b/gui/src/util/clientTools/multiEditImpl.ts index 66d1167231d..8b12109e5ad 100644 --- a/gui/src/util/clientTools/multiEditImpl.ts +++ b/gui/src/util/clientTools/multiEditImpl.ts @@ -16,7 +16,7 @@ export const multiEditImpl: ClientToolImpl = async ( toolCallId, extras, ) => { - const { filepath, edits } = args; + const { filepath, edits, editingFileContents } = args; const streamId = uuid(); @@ -64,7 +64,9 @@ export const multiEditImpl: ClientToolImpl = async ( `file ${filepath} does not exist. If you are trying to edit it, correct the filepath. If you are trying to create it, you must pass old_string=""`, ); } - newContent = await extras.ideMessenger.ide.readFile(resolvedUri); + newContent = + editingFileContents ?? + (await extras.ideMessenger.ide.readFile(resolvedUri)); fileUri = resolvedUri; for (let i = 0; i < edits.length; i++) { const { old_string, new_string, replace_all } = edits[i]; diff --git a/gui/src/util/clientTools/searchReplaceImpl.ts b/gui/src/util/clientTools/searchReplaceImpl.ts index ab931d40a28..5b750b26f57 100644 --- a/gui/src/util/clientTools/searchReplaceImpl.ts +++ b/gui/src/util/clientTools/searchReplaceImpl.ts @@ -11,7 +11,7 @@ export const searchReplaceToolImpl: ClientToolImpl = async ( toolCallId, extras, ) => { - const { filepath, diffs } = args; + const { filepath, diffs, editingFileContents } = args; const state = extras.getState(); const allowAnonymousTelemetry = state.config.config.allowAnonymousTelemetry; @@ -46,7 +46,8 @@ export const searchReplaceToolImpl: ClientToolImpl = async ( try { // Read the current file content const originalContent = - await extras.ideMessenger.ide.readFile(resolvedFilepath); + editingFileContents ?? + (await extras.ideMessenger.ide.readFile(resolvedFilepath)); let currentContent = originalContent; // Apply all replacements sequentially to build the final content diff --git a/gui/src/util/clientTools/singleFindAndReplaceImpl.ts b/gui/src/util/clientTools/singleFindAndReplaceImpl.ts index aec0ebf816a..f3bfe96d869 100644 --- a/gui/src/util/clientTools/singleFindAndReplaceImpl.ts +++ b/gui/src/util/clientTools/singleFindAndReplaceImpl.ts @@ -12,7 +12,13 @@ export const singleFindAndReplaceImpl: ClientToolImpl = async ( toolCallId, extras, ) => { - const { filepath, old_string, new_string, replace_all = false } = args; + const { + filepath, + old_string, + new_string, + replace_all = false, + editingFileContents, + } = args; const streamId = uuid(); @@ -33,7 +39,8 @@ export const singleFindAndReplaceImpl: ClientToolImpl = async ( // Read the current file content const originalContent = - await extras.ideMessenger.ide.readFile(resolvedFilepath); + (await editingFileContents) ?? + extras.ideMessenger.ide.readFile(resolvedFilepath); // Perform the find and replace operation const newContent = performFindAndReplace( From 07c39309c18588f61b082ae3672539b1954043f3 Mon Sep 17 00:00:00 2001 From: Dallin Romney Date: Sat, 23 Aug 2025 16:58:25 -0700 Subject: [PATCH 2/4] test: find and replace ui tests --- .../gui/ToolCallDiv/FindAndReplace.test.tsx | 446 ++++++++++++------ .../pages/gui/ToolCallDiv/FindAndReplace.tsx | 9 +- .../clientTools/singleFindAndReplaceImpl.ts | 4 +- 3 files changed, 301 insertions(+), 158 deletions(-) diff --git a/gui/src/pages/gui/ToolCallDiv/FindAndReplace.test.tsx b/gui/src/pages/gui/ToolCallDiv/FindAndReplace.test.tsx index 3e912bd66a8..8cf92ba4c7d 100644 --- a/gui/src/pages/gui/ToolCallDiv/FindAndReplace.test.tsx +++ b/gui/src/pages/gui/ToolCallDiv/FindAndReplace.test.tsx @@ -1,209 +1,347 @@ -import { configureStore } from "@reduxjs/toolkit"; -import { render, screen } from "@testing-library/react"; +import { fireEvent, render, screen } from "@testing-library/react"; import { ApplyState } from "core"; -import { Provider } from "react-redux"; -import { describe, expect, it, Mock, vi } from "vitest"; -import { useFileContent } from "../../../hooks/useFileContent"; -import { SingleFindAndReplace } from "./FindAndReplace"; +import { EditOperation } from "core/tools/definitions/multiEdit"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { FindAndReplaceDisplay } from "./FindAndReplace"; -// Mock the useFileContent hook -vi.mock("../../../hooks/useFileContent"); +// Mock the dependencies +vi.mock("../../../context/IdeMessenger", () => ({ + IdeMessengerContext: { + _currentValue: { post: vi.fn() }, + }, +})); -// Mock the selectApplyStateByToolCallId selector -vi.mock("../../../redux/selectors/selectToolCalls", () => ({ - selectApplyStateByToolCallId: vi.fn(), +vi.mock("../../../redux/hooks", () => ({ + useAppSelector: vi.fn(), })); -const mockUseFileContent = useFileContent as Mock; +vi.mock("../../../components/ui", () => ({ + useFontSize: () => 14, +})); -// Create a minimal store for testing -const createTestStore = (applyState?: ApplyState) => { - return configureStore({ - reducer: { - session: () => ({ - codeBlockApplyStates: { - states: applyState ? [applyState] : [], - }, - }), - }, - }); -}; +vi.mock("../../../util/clientTools/findAndReplaceUtils", () => ({ + performFindAndReplace: vi.fn(), +})); + +vi.mock("./utils", () => ({ + getStatusIcon: vi.fn(() =>
✓
), +})); -describe("SingleFindAndReplace", () => { +vi.mock("react", async () => { + const actual = await vi.importActual("react"); + return { + ...actual, + useContext: () => ({ post: vi.fn() }), + }; +}); + +// Import mocked modules +import { useAppSelector } from "../../../redux/hooks"; +import { performFindAndReplace } from "../../../util/clientTools/findAndReplaceUtils"; + +const mockPost = vi.fn(); +const mockUseAppSelector = useAppSelector as any; +const mockPerformFindAndReplace = performFindAndReplace as any; + +describe("FindAndReplaceDisplay", () => { const defaultProps = { - relativeFilePath: "test.ts", - oldString: "old code", - newString: "new code", - replaceAll: false, + fileUri: "file:///test/file.ts", + relativeFilePath: "test/file.ts", + editingFileContents: "const old = 'value';\nconst other = 'test';", + edits: [ + { + old_string: "const old = 'value';", + new_string: "const new = 'value';", + replace_all: false, + }, + ] as EditOperation[], toolCallId: "test-tool-call-id", historyIndex: 0, }; + const mockToolCallState = { + status: "done" as const, + output: null, + }; + + const mockConfig = { + ui: { + showChatScrollbar: true, + codeWrap: false, + }, + }; + beforeEach(() => { vi.clearAllMocks(); - }); - it("shows loading state when file content is loading", () => { - mockUseFileContent.mockReturnValue({ - fileContent: null, - isLoading: true, - error: null, + mockUseAppSelector.mockImplementation((selector: any) => { + const mockState = { + config: { config: mockConfig }, + session: { + history: [], + codeBlockApplyStates: { states: [] }, + }, + }; + + if (selector.toString().includes("selectApplyStateByToolCallId")) { + return undefined; // No apply state by default + } + if (selector.toString().includes("selectToolCallById")) { + return mockToolCallState; + } + + return selector(mockState); }); - const store = createTestStore(); - render( - - - , + mockPerformFindAndReplace.mockImplementation( + (content: string, oldStr: string, newStr: string) => { + return content.replace(oldStr, newStr); + }, ); - - expect(screen.getByText("Loading file content...")).toBeInTheDocument(); }); - it("shows error when file content fails to load", () => { - mockUseFileContent.mockReturnValue({ - fileContent: null, - isLoading: false, - error: "File not found", + describe("basic rendering", () => { + it("should render with collapsed state by default", () => { + render(); + + expect(screen.getByText("file.ts")).toBeInTheDocument(); + expect( + screen.getByTestId("toggle-find-and-replace-diff"), + ).toBeInTheDocument(); + + // Should not show diff content when collapsed + const diffContent = screen.queryByText("-"); + expect(diffContent).not.toBeInTheDocument(); }); - const store = createTestStore(); - render( - - - , - ); + it("should display file name from fileUri", () => { + render(); + expect(screen.getByText("file.ts")).toBeInTheDocument(); + }); + + it("should display file name from relativeFilePath when no fileUri", () => { + render( + , + ); + expect(screen.getByText("test.tsx")).toBeInTheDocument(); + }); - expect( - screen.getByText(`Failed to load file: ${defaultProps.relativeFilePath}`), - ).toBeInTheDocument(); + it("should handle missing file paths gracefully", () => { + render( + , + ); + + // Should still render the component structure + expect( + screen.getByTestId("toggle-find-and-replace-diff"), + ).toBeInTheDocument(); + }); }); - it("shows error when find and replace operation fails", () => { - mockUseFileContent.mockReturnValue({ - fileContent: "some content without the old string", - isLoading: false, - error: null, + describe("expand/collapse functionality", () => { + it("should expand when clicked", () => { + render(); + + const toggleButton = screen.getByTestId("toggle-find-and-replace-diff"); + fireEvent.click(toggleButton); + + // Should show diff content when expanded + expect(screen.getByText("-")).toBeInTheDocument(); + expect(screen.getByText("+")).toBeInTheDocument(); }); - const store = createTestStore(); - render( - - - , - ); + it("should show content when tool call status is 'generated'", () => { + mockUseAppSelector.mockImplementation((selector: any) => { + const mockState = { + config: { config: mockConfig }, + session: { + history: [], + codeBlockApplyStates: { states: [] }, + }, + }; + + if (selector.toString().includes("selectToolCallById")) { + return { ...mockToolCallState, status: "generated" }; + } + if (selector.toString().includes("selectApplyStateByToolCallId")) { + return undefined; + } - expect(screen.getByText(/Error:/)).toBeInTheDocument(); - expect(screen.getByText(/String not found in file/)).toBeInTheDocument(); + return selector(mockState); + }); + + render(); + + // Should show diff content without expanding + expect(screen.getByText("-")).toBeInTheDocument(); + expect(screen.getByText("+")).toBeInTheDocument(); + }); }); - it("shows no changes message when old and new strings result in same content", () => { - mockUseFileContent.mockReturnValue({ - fileContent: "some content", - isLoading: false, - error: null, + describe("diff generation", () => { + it("should generate and display diff correctly", () => { + render(); + + const toggleButton = screen.getByTestId("toggle-find-and-replace-diff"); + fireEvent.click(toggleButton); + + // Should show removed line + expect(screen.getByText("const old = 'value';")).toBeInTheDocument(); + + // Should show added line + expect(screen.getByText("const new = 'value';")).toBeInTheDocument(); + + // Should show unchanged line + expect(screen.getByText("const other = 'test';")).toBeInTheDocument(); }); - const store = createTestStore(); - render( - - - , - ); + it("should handle multiple edits", () => { + const multipleEdits = [ + { + old_string: "const old = 'value';", + new_string: "const new = 'value';", + replace_all: false, + }, + { + old_string: "const other = 'test';", + new_string: "const other = 'updated';", + replace_all: false, + }, + ] as EditOperation[]; - // This should show an error since the string doesn't exist - expect(screen.getByText(/Error:/)).toBeInTheDocument(); - }); + mockPerformFindAndReplace + .mockReturnValueOnce("const new = 'value';\nconst other = 'test';") + .mockReturnValueOnce("const new = 'value';\nconst other = 'updated';"); + + render(); - it("shows diff when find and replace operation succeeds", () => { - const fileContent = "Hello old code world"; - mockUseFileContent.mockReturnValue({ - fileContent, - isLoading: false, - error: null, + const toggleButton = screen.getByTestId("toggle-find-and-replace-diff"); + fireEvent.click(toggleButton); + + expect(mockPerformFindAndReplace).toHaveBeenCalledTimes(2); }); - const store = createTestStore(); - render( - - - , - ); + it("should handle diff generation errors", () => { + mockPerformFindAndReplace.mockImplementation(() => { + throw new Error("Test error"); + }); - expect(screen.getByText(defaultProps.relativeFilePath)).toBeInTheDocument(); - // The diff should show the old line being removed and new line being added - expect(screen.getByText("Hello old code world")).toBeInTheDocument(); - expect(screen.getByText("Hello new code world")).toBeInTheDocument(); - }); + render(); + + const toggleButton = screen.getByTestId("toggle-find-and-replace-diff"); + fireEvent.click(toggleButton); - it("shows apply state status when available", () => { - mockUseFileContent.mockReturnValue({ - fileContent: "Hello old code world", - isLoading: false, - error: null, + expect(screen.getByText("Error generating diff")).toBeInTheDocument(); + expect(screen.getByText("Test error")).toBeInTheDocument(); }); - const applyState: ApplyState = { - streamId: "test-stream", + it("should show 'No changes to display' when diff is empty", () => { + // Mock the function to return the exact same content (no changes) + mockPerformFindAndReplace.mockReturnValue( + defaultProps.editingFileContents, + ); + + render(); + + const toggleButton = screen.getByTestId("toggle-find-and-replace-diff"); + fireEvent.click(toggleButton); + + expect(screen.getByText("No changes to display")).toBeInTheDocument(); + }); + }); + + describe("apply actions", () => { + const mockApplyState: ApplyState = { + streamId: "test-stream-id", status: "streaming", - toolCallId: "test-tool-call-id", + numDiffs: 1, + fileContent: "test content", }; - const store = createTestStore(applyState); - render( - - - , - ); + beforeEach(() => { + mockUseAppSelector.mockImplementation((selector: any) => { + const mockState = { + config: { config: mockConfig }, + session: { + history: [], + codeBlockApplyStates: { states: [mockApplyState] }, + }, + }; - expect(screen.getByText("Applying...")).toBeInTheDocument(); - }); + if (selector.toString().includes("selectApplyStateByToolCallId")) { + return mockApplyState; + } + if (selector.toString().includes("selectToolCallById")) { + return mockToolCallState; + } - it("shows tool call status icon when enabled", () => { - mockUseFileContent.mockReturnValue({ - fileContent: "Hello old code world", - isLoading: false, - error: null, + return selector(mockState); + }); }); - const store = createTestStore(); - render( - - - , - ); + it("should show apply actions when applyState exists", () => { + render(); + + // ApplyActions component should be rendered (we can test by looking for its structure) + // Since we don't have the exact structure, we test that the container is there + expect( + screen.getByTestId("toggle-find-and-replace-diff"), + ).toBeInTheDocument(); + }); + + it("should handle accept action", () => { + render(); - // Look for the status indicator (green dot for done status) - const statusIndicator = document.querySelector(".bg-green-500"); - expect(statusIndicator).toBeInTheDocument(); + // This would need more detailed testing if ApplyActions was properly mocked + // For now, we verify the component renders without errors + expect( + screen.getByTestId("toggle-find-and-replace-diff"), + ).toBeInTheDocument(); + }); }); - it("handles replace all option correctly", () => { - const fileContent = "old code and more old code"; - mockUseFileContent.mockReturnValue({ - fileContent, - isLoading: false, - error: null, + describe("content handling", () => { + it("should use editingFileContents when provided", () => { + render(); + + const toggleButton = screen.getByTestId("toggle-find-and-replace-diff"); + fireEvent.click(toggleButton); + + expect(mockPerformFindAndReplace).toHaveBeenCalledWith( + "const old = 'value';\nconst other = 'test';", + "const old = 'value';", + "const new = 'value';", + false, + 0, + ); }); - const store = createTestStore(); - render( - - - , - ); + it("should fallback to edit old_strings when no editingFileContents", () => { + render( + , + ); - // Should show both replacements in the diff - expect(screen.getByText(defaultProps.relativeFilePath)).toBeInTheDocument(); - expect(screen.getByText("old code and more old code")).toBeInTheDocument(); - expect(screen.getByText("new code and more new code")).toBeInTheDocument(); + const toggleButton = screen.getByTestId("toggle-find-and-replace-diff"); + fireEvent.click(toggleButton); + + expect(mockPerformFindAndReplace).toHaveBeenCalledWith( + "const old = 'value';", + "const old = 'value';", + "const new = 'value';", + false, + 0, + ); + }); }); }); diff --git a/gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx b/gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx index 55cf27ae31e..f13ad6855e3 100644 --- a/gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx +++ b/gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx @@ -60,7 +60,7 @@ export function FindAndReplaceDisplay({ if (editingFileContents) { return editingFileContents; } - return edits.map((edit) => edit.old_string).join("\n"); + return edits?.map((edit) => edit.old_string ?? "").join("\n"); }, [editingFileContents, edits]); const diffResult = useMemo(() => { @@ -182,7 +182,12 @@ export function FindAndReplaceDisplay({ ); } - if (!diffResult?.diff) { + if ( + !diffResult?.diff || + (diffResult.diff.length === 1 && + !diffResult.diff[0].added && + !diffResult.diff[0].removed) + ) { return renderContainer(
No changes to display
, ); diff --git a/gui/src/util/clientTools/singleFindAndReplaceImpl.ts b/gui/src/util/clientTools/singleFindAndReplaceImpl.ts index f3bfe96d869..92c3da17838 100644 --- a/gui/src/util/clientTools/singleFindAndReplaceImpl.ts +++ b/gui/src/util/clientTools/singleFindAndReplaceImpl.ts @@ -39,8 +39,8 @@ export const singleFindAndReplaceImpl: ClientToolImpl = async ( // Read the current file content const originalContent = - (await editingFileContents) ?? - extras.ideMessenger.ide.readFile(resolvedFilepath); + editingFileContents ?? + (await extras.ideMessenger.ide.readFile(resolvedFilepath)); // Perform the find and replace operation const newContent = performFindAndReplace( From 0208969b856579173d381707795cdd84c7b5e13c Mon Sep 17 00:00:00 2001 From: Dallin Romney Date: Sat, 23 Aug 2025 18:31:08 -0700 Subject: [PATCH 3/4] chore: cleanup console logs --- gui/src/redux/thunks/streamNormalInput.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/gui/src/redux/thunks/streamNormalInput.ts b/gui/src/redux/thunks/streamNormalInput.ts index 6adfaf19910..dbd322357d8 100644 --- a/gui/src/redux/thunks/streamNormalInput.ts +++ b/gui/src/redux/thunks/streamNormalInput.ts @@ -255,7 +255,6 @@ export const streamNormalInput = createAsyncThunk< const toolSettings = newState.ui.toolSettings; const allToolCallStates = selectCurrentToolCalls(newState); - console.log("ADDING EDITNG CURRENT FILE"); await Promise.all( allToolCallStates.map((tcState) => enhanceParsedArgs( From 56b7195c712403331c2a8a9b43d43e06915023f1 Mon Sep 17 00:00:00 2001 From: Dallin Romney Date: Mon, 25 Aug 2025 12:34:54 -0700 Subject: [PATCH 4/4] fix: cubic feedback --- gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx b/gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx index f13ad6855e3..18d995601cd 100644 --- a/gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx +++ b/gui/src/pages/gui/ToolCallDiv/FindAndReplace.tsx @@ -120,7 +120,7 @@ export function FindAndReplaceDisplay({ ); } } - }, [toolCallState?.status]); + }, [toolCallState?.status, toolCallState?.output]); // Unified container component that always renders the same structure const renderContainer = (content: React.ReactNode) => ( @@ -205,7 +205,7 @@ export function FindAndReplaceDisplay({ return (
{part.value.split("\n").map((line, lineIndex) => { if ( @@ -226,7 +226,7 @@ export function FindAndReplaceDisplay({ return (
{part.value.split("\n").map((line, lineIndex) => { if (