From e5c9eac93383e70f99ee923804e338f64e066ba2 Mon Sep 17 00:00:00 2001 From: Philip Zeyliger Date: Mon, 12 Jan 2026 06:30:36 -0800 Subject: [PATCH] shelley/ui: default to simple diff view, feature flag for Monaco Prompt: In a sort of recent commit, we switched the patch tool output to use Monaco diffs. These seem to not be working well yet. Let's reintroduce the old simple diff view. Keep the Monaco code behind a feature flag that can be set with local storage so I can keep trying the new thing. Use localStorage.setItem('shelley-use-monaco-diff', 'true') to enable the Monaco diff view. The simple text-based diff view is now the default as the Monaco diff had some issues. Both views are still available: - Simple diff: default, shows unified diff with syntax highlighting - Monaco diff: feature-flagged, side-by-side view with commenting --- ui/src/components/PatchTool.tsx | 297 ++++++++++++++++++++------------ ui/src/styles.css | 49 ++++++ 2 files changed, 239 insertions(+), 107 deletions(-) diff --git a/ui/src/components/PatchTool.tsx b/ui/src/components/PatchTool.tsx index 3db864435103b30cd50680c90a6b1f511a0af241..f1c1ad59e5d8290ed0708d94e63777d5fe5e5ca9 100644 --- a/ui/src/components/PatchTool.tsx +++ b/ui/src/components/PatchTool.tsx @@ -3,6 +3,15 @@ import type * as Monaco from "monaco-editor"; import { LLMContent } from "../types"; import { isDarkModeActive } from "../services/theme"; +// Feature flag for Monaco diff view (set localStorage.setItem('shelley-use-monaco-diff', 'true') to enable) +function useMonacoDiff(): boolean { + try { + return localStorage.getItem("shelley-use-monaco-diff") === "true"; + } catch { + return false; + } +} + // Display data structure from the patch tool interface PatchDisplayData { path: string; @@ -62,19 +71,66 @@ function loadMonaco(): Promise { return monacoLoadPromise; } -function PatchTool({ - toolInput, - isRunning, - toolResult, - hasError, +// Simple diff view component (default) +function SimpleDiffView({ + displayData, executionTime, - display, +}: { + displayData: PatchDisplayData | null; + executionTime?: string; +}) { + // Get diff text from displayData or fall back to empty + const diff = displayData?.diff || ""; + + // Parse unified diff to extract lines + const lines = diff ? diff.split("\n") : []; + + return ( +
+ {executionTime && ( +
+ Diff: + {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 view component (feature-flagged) +function MonacoDiffView({ + displayData, + isMobile, onCommentTextChange, -}: PatchToolProps) { - // Default to collapsed for errors (since agents typically recover), expanded otherwise - const [isExpanded, setIsExpanded] = useState(!hasError); + filename, +}: { + displayData: PatchDisplayData; + isMobile: boolean; + onCommentTextChange?: (text: string) => void; + filename: string; +}) { const [monacoLoaded, setMonacoLoaded] = useState(false); - const [isMobile, setIsMobile] = useState(window.innerWidth < 768); const [showCommentDialog, setShowCommentDialog] = useState<{ line: number; selectedText?: string; @@ -94,48 +150,9 @@ function PatchTool({ modified: null, }); - // Track viewport size + // Load Monaco useEffect(() => { - const handleResize = () => { - setIsMobile(window.innerWidth < 768); - }; - window.addEventListener("resize", handleResize); - return () => window.removeEventListener("resize", handleResize); - }, []); - - // Extract path from toolInput - const path = - typeof toolInput === "object" && - toolInput !== null && - "path" in toolInput && - typeof toolInput.path === "string" - ? toolInput.path - : typeof toolInput === "string" - ? toolInput - : ""; - - // 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; - - // 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) { + if (!monacoLoaded) { loadMonaco() .then((monaco) => { monacoRef.current = monaco; @@ -145,17 +162,11 @@ function PatchTool({ console.error("Failed to load Monaco:", err); }); } - }, [isExpanded, displayData, monacoLoaded]); + }, [monacoLoaded]); // Create Monaco editor when data is ready useEffect(() => { - if ( - !monacoLoaded || - !displayData || - !editorContainerRef.current || - !monacoRef.current || - !isExpanded - ) { + if (!monacoLoaded || !editorContainerRef.current || !monacoRef.current) { return; } @@ -325,7 +336,7 @@ function PatchTool({ modelsRef.current.modified = null; } }; - }, [monacoLoaded, displayData, isMobile, isExpanded, onCommentTextChange]); + }, [monacoLoaded, displayData, isMobile, onCommentTextChange]); // Update Monaco theme when dark mode changes useEffect(() => { @@ -376,7 +387,6 @@ function PatchTool({ // 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, @@ -386,6 +396,112 @@ function PatchTool({ return `${height}px`; }; + return ( + <> +
+ + {/* Comment dialog */} + {showCommentDialog && onCommentTextChange && ( +
+

Add Comment (Line {showCommentDialog.line})

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