From 1f135e9287f76d7038f1217eb0025739de42587a Mon Sep 17 00:00:00 2001 From: Philip Zeyliger Date: Sat, 10 Jan 2026 14:32:45 -0800 Subject: [PATCH] shelley: use Monaco diff view for patch tool Hey, you can leave comments too, why not. Prompt: In a new worktree (reset to origin/main after fetching), work on the UI of the patch tool. See if you can use the monaco diff view to produce a side-by-side from that patch display. (Note that you can modify the display output of the tool if you need to, within reason.) Do side-by-side if there's space; not side-by-side if there isn't; theme (light/dark) should follow the rest of the interface. There's a separate diff view which can be leaned on, though the interfaces are quite different. It would be nice to have commenting in both - Modify patch tool backend to return structured display data with old/new content - Update PatchTool.tsx to use Monaco diff editor - Side-by-side view on desktop, inline view on mobile (<768px) - Theme follows dark/light mode automatically via MutationObserver - Commenting support: click on lines to add comments (works in both streaming and static views) - Pass onCommentTextChange prop through Message.tsx for commenting after page reload - Removed legacy text-based diff fallback (always use Monaco now) --- claudetool/patch.go | 19 +- ui/src/components/ChatInterface.tsx | 6 + ui/src/components/Message.tsx | 20 +- ui/src/components/PatchTool.tsx | 368 ++++++++++++++++++++++++---- ui/src/styles.css | 130 +++++++--- 5 files changed, 455 insertions(+), 88 deletions(-) diff --git a/claudetool/patch.go b/claudetool/patch.go index 0695320340c126fc9dbb035da21bdeba14bb47e9..40d1887c74d673c38b7c3bf62f3a2f2553c09236 100644 --- a/claudetool/patch.go +++ b/claudetool/patch.go @@ -260,6 +260,14 @@ type PatchInputOneString struct { Patches string `json:"patches"` // contains Patches as a JSON string 🤦 } +// PatchDisplayData is the structured data sent to the UI for display. +type PatchDisplayData struct { + Path string `json:"path"` + OldContent string `json:"oldContent"` + NewContent string `json:"newContent"` + Diff string `json:"diff"` +} + // PatchRequest represents a single patch operation. type PatchRequest struct { Operation string `json:"operation"` @@ -543,10 +551,17 @@ func (p *PatchTool) patchRun(ctx context.Context, input *PatchInput) llm.ToolOut diff := generateUnifiedDiff(input.Path, string(orig), string(patched)) - // TODO: maybe report the patch result to the model, i.e. some/all of the new code after the patches and formatting. + // Display data for the UI includes structured content for Monaco diff editor + displayData := PatchDisplayData{ + Path: input.Path, + OldContent: string(orig), + NewContent: string(patched), + Diff: diff, + } + return llm.ToolOut{ LLMContent: llm.TextContent(response.String()), - Display: diff, + Display: displayData, } } diff --git a/ui/src/components/ChatInterface.tsx b/ui/src/components/ChatInterface.tsx index 4c38656c6ef6ffa0f2165e218f4e95ba88b82698..7cee2335a8b42f92051398a3cf8433e82780dbc7 100644 --- a/ui/src/components/ChatInterface.tsx +++ b/ui/src/components/ChatInterface.tsx @@ -124,6 +124,7 @@ interface CoalescedToolCallProps { toolEndTime?: string | null; hasResult?: boolean; display?: unknown; + onCommentTextChange?: (text: string) => void; } // Map tool names to their specialized components. @@ -155,6 +156,7 @@ function CoalescedToolCall({ toolEndTime, hasResult, display, + onCommentTextChange, }: CoalescedToolCallProps) { // Calculate execution time if available let executionTime = ""; @@ -183,6 +185,8 @@ function CoalescedToolCall({ ...(toolName === "browser_recent_console_logs" || toolName === "browser_clear_console_logs" ? { toolName } : {}), + // Patch tool can add comments + ...(toolName === "patch" && onCommentTextChange ? { onCommentTextChange } : {}), }; return ; } @@ -932,6 +936,7 @@ function ChatInterface({ setDiffViewerInitialCommit(commit); setShowDiffViewer(true); }} + onCommentTextChange={setDiffCommentText} /> ); } else if (item.type === "tool") { @@ -946,6 +951,7 @@ function ChatInterface({ toolEndTime={item.toolEndTime} hasResult={item.hasResult} display={item.display} + onCommentTextChange={setDiffCommentText} /> ); } diff --git a/ui/src/components/Message.tsx b/ui/src/components/Message.tsx index 566d1221e054b1a7dbaaa2f3890e6473ffd4b8be..2fb157743ac1862b8a9dab6ac15ce212b2c02029 100644 --- a/ui/src/components/Message.tsx +++ b/ui/src/components/Message.tsx @@ -26,9 +26,10 @@ interface ToolDisplay { interface MessageProps { message: MessageType; onOpenDiffViewer?: (commit: string) => void; + onCommentTextChange?: (text: string) => void; } -function Message({ message, onOpenDiffViewer }: MessageProps) { +function Message({ message, onOpenDiffViewer, onCommentTextChange }: MessageProps) { // Hide system messages from the UI if (message.type === "system") { return null; @@ -352,7 +353,13 @@ function Message({ message, onOpenDiffViewer }: MessageProps) { } // Use specialized component for patch tool if (content.ToolName === "patch") { - return ; + return ( + + ); } // Use specialized component for screenshot tool if (content.ToolName === "screenshot" || content.ToolName === "browser_take_screenshot") { @@ -475,6 +482,7 @@ function Message({ message, onOpenDiffViewer }: MessageProps) { hasError={hasError} executionTime={executionTime} display={content.Display} + onCommentTextChange={onCommentTextChange} /> ); } @@ -724,7 +732,13 @@ function Message({ message, onOpenDiffViewer }: MessageProps) { ]; return ( - + ); } diff --git a/ui/src/components/PatchTool.tsx b/ui/src/components/PatchTool.tsx index 09428d756624ad5bdb73486a492503430de6a7df..724b30fa346b4162575e439cbb4539930958879d 100644 --- a/ui/src/components/PatchTool.tsx +++ b/ui/src/components/PatchTool.tsx @@ -1,5 +1,15 @@ -import React, { useState } from "react"; +import React, { useState, useEffect, useRef, useCallback } from "react"; +import type * as Monaco from "monaco-editor"; import { LLMContent } from "../types"; +import { isDarkModeActive } from "../services/theme"; + +// Display data structure from the patch tool +interface PatchDisplayData { + path: string; + oldContent: string; + newContent: string; + diff: string; +} interface PatchToolProps { // For tool_use (pending state) @@ -10,7 +20,46 @@ interface PatchToolProps { toolResult?: LLMContent[]; hasError?: boolean; executionTime?: string; - display?: unknown; // Display data from the tool_result Content (contains the diff) + display?: unknown; // Display data from the tool_result Content (contains the diff or structured data) + onCommentTextChange?: (text: string) => void; +} + +// Global Monaco instance - loaded lazily +let monacoInstance: typeof Monaco | null = null; +let monacoLoadPromise: Promise | null = null; + +function loadMonaco(): Promise { + if (monacoInstance) { + return Promise.resolve(monacoInstance); + } + if (monacoLoadPromise) { + return monacoLoadPromise; + } + + monacoLoadPromise = (async () => { + // Configure Monaco environment for web workers before importing + const monacoEnv: Monaco.Environment = { + getWorkerUrl: () => "/editor.worker.js", + }; + (self as Window).MonacoEnvironment = monacoEnv; + + // Load Monaco CSS if not already loaded + if (!document.querySelector('link[href="/monaco-editor.css"]')) { + const link = document.createElement("link"); + link.rel = "stylesheet"; + link.href = "/monaco-editor.css"; + document.head.appendChild(link); + } + + // Load Monaco from our local bundle (runtime URL, cast to proper types) + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore - dynamic runtime URL import + const monaco = (await import("/monaco-editor.js")) as typeof Monaco; + monacoInstance = monaco; + return monacoInstance; + })(); + + return monacoLoadPromise; } function PatchTool({ @@ -20,9 +69,31 @@ function PatchTool({ hasError, executionTime, display, + onCommentTextChange, }: PatchToolProps) { // Default to collapsed for errors (since agents typically recover), expanded otherwise const [isExpanded, setIsExpanded] = useState(!hasError); + const [monacoLoaded, setMonacoLoaded] = useState(false); + const [isMobile, setIsMobile] = useState(window.innerWidth < 768); + const [showCommentDialog, setShowCommentDialog] = useState<{ + line: number; + selectedText?: string; + } | null>(null); + const [commentText, setCommentText] = useState(""); + + const editorContainerRef = useRef(null); + const editorRef = useRef(null); + const monacoRef = useRef(null); + const commentInputRef = useRef(null); + + // Track viewport size + useEffect(() => { + const handleResize = () => { + setIsMobile(window.innerWidth < 768); + }; + window.addEventListener("resize", handleResize); + return () => window.removeEventListener("resize", handleResize); + }, []); // Extract path from toolInput const path = @@ -35,38 +106,213 @@ function PatchTool({ ? toolInput : ""; - // Extract diff from display (preferred) or fall back to toolResult - const diff = - typeof display === "string" - ? display - : toolResult && toolResult.length > 0 && toolResult[0].Text - ? toolResult[0].Text - : ""; + // Parse display data (structured format from backend) + const displayData: PatchDisplayData | null = + display && + typeof display === "object" && + "path" in display && + "oldContent" in display && + "newContent" in display + ? (display as PatchDisplayData) + : null; + + // Extract error message from toolResult if present + const errorMessage = + toolResult && toolResult.length > 0 && toolResult[0].Text ? toolResult[0].Text : ""; const isComplete = !isRunning && toolResult !== undefined; - // Parse unified diff to extract filename and colorize lines - const parseDiff = (diffText: string) => { - if (!diffText) return { filename: path, lines: [] }; + // Extract filename from path or diff headers + const filename = displayData?.path || path || "patch"; + + // Load Monaco when expanded and we have display data + useEffect(() => { + if (isExpanded && displayData && !monacoLoaded) { + loadMonaco() + .then((monaco) => { + monacoRef.current = monaco; + setMonacoLoaded(true); + }) + .catch((err) => { + console.error("Failed to load Monaco:", err); + }); + } + }, [isExpanded, displayData, monacoLoaded]); + + // Create Monaco editor when data is ready + useEffect(() => { + if ( + !monacoLoaded || + !displayData || + !editorContainerRef.current || + !monacoRef.current || + !isExpanded + ) { + return; + } + + const monaco = monacoRef.current; + + // Dispose previous editor + if (editorRef.current) { + editorRef.current.dispose(); + editorRef.current = null; + } + + // Get language from file extension + const ext = "." + (displayData.path.split(".").pop()?.toLowerCase() || ""); + const languages = monaco.languages.getLanguages(); + let language = "plaintext"; + for (const lang of languages) { + if (lang.extensions?.includes(ext)) { + language = lang.id; + break; + } + } + + // Create models with unique URIs (include timestamp to avoid conflicts) + const timestamp = Date.now(); + const originalUri = monaco.Uri.file(`patch-original-${timestamp}-${displayData.path}`); + const modifiedUri = monaco.Uri.file(`patch-modified-${timestamp}-${displayData.path}`); + + const originalModel = monaco.editor.createModel(displayData.oldContent, language, originalUri); + const modifiedModel = monaco.editor.createModel(displayData.newContent, language, modifiedUri); + + // Create diff editor + const diffEditor = monaco.editor.createDiffEditor(editorContainerRef.current, { + theme: isDarkModeActive() ? "vs-dark" : "vs", + readOnly: true, + originalEditable: false, + automaticLayout: true, + renderSideBySide: !isMobile, + enableSplitViewResizing: true, + renderIndicators: true, + renderMarginRevertIcon: false, + lineNumbers: isMobile ? "off" : "on", + minimap: { enabled: false }, + scrollBeyondLastLine: false, + wordWrap: "on", + glyphMargin: false, + lineDecorationsWidth: isMobile ? 0 : 10, + lineNumbersMinChars: isMobile ? 0 : 3, + quickSuggestions: false, + suggestOnTriggerCharacters: false, + lightbulb: { enabled: false }, + codeLens: false, + contextmenu: false, + links: false, + folding: !isMobile, + }); + + diffEditor.setModel({ + original: originalModel, + modified: modifiedModel, + }); - const lines = diffText.split("\n"); - let filename = path; + editorRef.current = diffEditor; - // Extract filename from diff header if present - for (const line of lines) { - if (line.startsWith("---")) { - // Format: --- a/path/to/file.txt - const match = line.match(/^---\s+(.+?)\s*$/); - if (match) { - filename = match[1].replace(/^[ab]\//, ""); // Remove a/ or b/ prefix + // Add click handler for commenting if callback is provided + if (onCommentTextChange) { + const modifiedEditor = diffEditor.getModifiedEditor(); + + const openCommentDialog = (lineNumber: number) => { + const model = modifiedEditor.getModel(); + const selection = modifiedEditor.getSelection(); + let selectedText = ""; + + if (selection && !selection.isEmpty() && model) { + selectedText = model.getValueInRange(selection); + } else if (model) { + selectedText = model.getLineContent(lineNumber) || ""; + } + + setShowCommentDialog({ + line: lineNumber, + selectedText, + }); + }; + + modifiedEditor.onMouseDown((e: Monaco.editor.IEditorMouseEvent) => { + const isLineClick = + e.target.type === monaco.editor.MouseTargetType.CONTENT_TEXT || + e.target.type === monaco.editor.MouseTargetType.CONTENT_EMPTY; + + if (isLineClick) { + const position = e.target.position; + if (position) { + openCommentDialog(position.lineNumber); + } + } + }); + } + + // Cleanup function + return () => { + if (editorRef.current) { + editorRef.current.dispose(); + editorRef.current = null; + } + }; + }, [monacoLoaded, displayData, isMobile, isExpanded, onCommentTextChange]); + + // Update Monaco theme when dark mode changes + useEffect(() => { + if (!monacoRef.current) return; + + const updateMonacoTheme = () => { + const theme = isDarkModeActive() ? "vs-dark" : "vs"; + monacoRef.current?.editor.setTheme(theme); + }; + + const observer = new MutationObserver((mutations) => { + for (const mutation of mutations) { + if (mutation.attributeName === "class") { + updateMonacoTheme(); } } + }); + + observer.observe(document.documentElement, { attributes: true }); + + return () => observer.disconnect(); + }, [monacoLoaded]); + + // Focus comment input when dialog opens + useEffect(() => { + if (showCommentDialog && commentInputRef.current) { + setTimeout(() => { + commentInputRef.current?.focus(); + }, 50); } + }, [showCommentDialog]); - return { filename, lines }; - }; + // Handle adding a comment + const handleAddComment = useCallback(() => { + if (!showCommentDialog || !commentText.trim() || !onCommentTextChange) return; + + const line = showCommentDialog.line; + const codeSnippet = showCommentDialog.selectedText?.split("\n")[0]?.trim() || ""; + const truncatedCode = + codeSnippet.length > 60 ? codeSnippet.substring(0, 57) + "..." : codeSnippet; - const { filename, lines } = parseDiff(diff); + const commentBlock = `> ${filename}:${line}: ${truncatedCode}\n${commentText}\n\n`; + + onCommentTextChange(commentBlock); + setShowCommentDialog(null); + setCommentText(""); + }, [showCommentDialog, commentText, onCommentTextChange, filename]); + + // Calculate editor height based on content + const getEditorHeight = () => { + if (!displayData) return "200px"; + const lineCount = Math.max( + displayData.oldContent.split("\n").length, + displayData.newContent.split("\n").length, + ); + // Clamp between 100px and 400px, with 18px per line + const height = Math.min(400, Math.max(100, lineCount * 18 + 20)); + return `${height}px`; + }; return (
setIsExpanded(!isExpanded)}>
🖋️ - {filename || "patch"} + {filename} {isComplete && hasError && ✗} {isComplete && !hasError && ✓}
@@ -109,7 +355,7 @@ function PatchTool({ {isExpanded && (
- {isComplete && !hasError && diff && ( + {isComplete && !hasError && displayData && (
{executionTime && (
@@ -117,27 +363,13 @@ function PatchTool({ {executionTime}
)} - 
-                {lines.map((line, idx) => {
-                  // Determine line type for styling
-                  let className = "patch-diff-line";
-                  if (line.startsWith("+") && !line.startsWith("+++")) {
-                    className += " patch-diff-addition";
-                  } else if (line.startsWith("-") && !line.startsWith("---")) {
-                    className += " patch-diff-deletion";
-                  } else if (line.startsWith("@@")) {
-                    className += " patch-diff-hunk";
-                  } else if (line.startsWith("---") || line.startsWith("+++")) {
-                    className += " patch-diff-header";
-                  }
-
-                  return (
-                    

-                      {line || " "}
-                    

-                  );
-                })}
-
+ + {/* Monaco diff editor */} +
)} @@ -147,7 +379,7 @@ function PatchTool({ Error: {executionTime && {executionTime}}
- 
{diff || "Patch failed"}
+ 
{errorMessage || "Patch failed"}
)} @@ -158,6 +390,46 @@ function PatchTool({ )}
)} + + {/* Comment dialog */} + {showCommentDialog && onCommentTextChange && ( +
+

Add Comment (Line {showCommentDialog.line})

+ {showCommentDialog.selectedText && ( + 
{showCommentDialog.selectedText}
+ )} +