Skip to content

File diff and checkpoints (WIP)#3040

Closed
Kvadratni wants to merge 184 commits intomainfrom
feature/global-checkpoints
Closed

File diff and checkpoints (WIP)#3040
Kvadratni wants to merge 184 commits intomainfrom
feature/global-checkpoints

Conversation

@Kvadratni
Copy link
Collaborator

@Kvadratni Kvadratni commented Jun 23, 2025

Implement checkpoint system with intelligent diff splitting

Adds file versioning and enhanced diff handling for better change management and review.

Checkpoint system:

  • Automatic backup creation before file modifications with timestamp-based storage
  • Persistent checkpoint index tracking file modification history
  • Restore functionality with diff generation for change tracking

Enhanced diff generation:

  • File-type aware context sizing for better hunk boundaries
  • Intelligent splitting of large diffs into manageable chunks
  • Improved review experience for AI-generated changes

New tools:

  • list_checkpoints - View available file versions
  • restore_checkpoint - Revert files to previous states

Integration:

  • Checkpoint payloads embedded in message stream for UI consumption
  • Maintains backward compatibility with existing undo functionality
  • Supports both traditional string replacement and Editor API workflows

Addresses large diff readability issues while providing robust file versioning capabilities.

new-diff.mp4

@Kvadratni Kvadratni force-pushed the feature/global-checkpoints branch from d2c1e11 to 381bede Compare June 24, 2025 15:58
}

// Helper function to create a checkpoint of a file
fn create_checkpoint(&self, file: &Path) -> Result<(PathBuf, String), ToolError> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this creating a full file for each checkpoint? Just thinking about storage efficiency, I wonder if you could just store just the diffs, mapped to the file

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah full file for now.

@Kvadratni Kvadratni force-pushed the feature/global-checkpoints branch from 2e46a2f to def69d7 Compare June 27, 2025 18:58
@Kvadratni Kvadratni force-pushed the feature/global-checkpoints branch 2 times, most recently from febeb99 to 37f258a Compare July 2, 2025 20:03
@Kvadratni Kvadratni marked this pull request as ready for review July 2, 2025 20:03
Copy link
Collaborator

@DOsinga DOsinga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This definitely solves a problem worth solving; I'm not convinced though still that we should do our own version management rather than just relying on goose. commits are pretty cheap and can be squashed when needed.

@@ -0,0 +1,17 @@
import { createContext, useContext } from 'react';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the Revert link on the message. allows to gather all changes made since this messge, so that rollback would gather all the changes made to this point.

.with_priority(0.2),
Content::embedded_text(
"goose://checkpoint",
serde_json::to_string(&payload).unwrap(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we're storing the diff here in the conversation and then skipping sending it to the LLM? that doesn't seem like the way to do this if we actually have the version history of a file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

diffs can be huge. I wanted to save context

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I get that we don't want to send this to the LLMs. But as you point out correctly, diffs can be huge, so storing them in our fragile sessions doesn't seem great either. For one thing we now have a 2Mb size limit on sessions after which they will just dissapear

}),
);

let list_checkpoints_tool = Tool::new(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

absolutely does

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, when does the LLM call this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I ask goose to restore file to a checkpoint

Kvadratni and others added 21 commits July 14, 2025 09:17
- Fixed JSX structure error in App.tsx (missing closing div tag)
- Removed duplicate imports for RecipeEditor and RecipesView
- Removed unused DiffViewer import
- Added missing onRestore prop to UserMessage components in BaseChat and SessionHistoryView
- Implemented proper onRestore functionality for file restoration in BaseChat
- Added no-op onRestore for read-only SessionHistoryView

All TypeScript checks now pass successfully.
- Modified MessageRestoreLink to accept messages as a prop instead of using useChatMessages hook
- Updated UserMessage to accept optional messages prop and pass it to MessageRestoreLink
- Updated BaseChat and SessionHistoryView to pass messages array to UserMessage
- Removed dependency on ChatContextProvider for MessageRestoreLink functionality
- Added backward compatibility with optional messages prop

This resolves the 'useChatMessages must be used within a ChatContextProvider' error
that occurred when UserMessage was used outside of a ChatContextProvider context.
- Enhanced RestoreModal with proper backdrop and solid background
- Fixed transparent modal issue using design system classes
- Updated BaseChat to show RestoreModal before sending chat message
- Integrated proper restore flow: modal confirmation -> chat message -> tool execution
- Fixed text colors and button styling for dark theme compatibility
- Restore proper border and background structure from base branch
- Add border-borderSubtle and shadow-sm to tool call containers
- Use bg-background-default for tool sections with proper border-t
- Maintain all diff functionality and checkpoint features
- Fix visual issues with tool call backgrounds and borders
- Ensure proper rounded corners and spacing
✨ New Features:
- Created SidecarPanel component with floating action buttons
- Built DiffViewer with Figma design styling (#1E1E1E background, proper borders)
- Implemented SidecarLayout provider for managing sidecar state
- Auto-show diff viewer when tool calls have diff content

🎨 Design System:
- Thin 60px sidecar panel between chat and diff viewer
- Floating pill-shaped action buttons (12x8 rounded-2xl)
- 700px diff viewer panel with proper dark theme styling
- Smooth transitions and proper z-index layering

🔧 Technical Implementation:
- Context-based sidecar state management
- Auto-detection of diff content from tool responses
- Proper TypeScript types and error handling
- Integration with existing checkpoint system

🚀 Future Ready:
- Extensible sidecar action system for future features
- Proper positioning at tool call level
- Clean separation of concerns between chat and sidecar
🐛 Bug Fix:
- Made useSidecar hook return null instead of throwing error when no provider
- Updated ToolCallWithResponse to handle optional sidecar context
- Added proper null checks before calling sidecar methods

🔧 Technical Details:
- useSidecar() now returns SidecarContextType | null
- Added conditional check: if (hasDiff && diffContent && sidecar)
- Prevents 'useSidecar must be used within SidecarProvider' error
- Allows components to work with or without sidecar functionality

✅ Result:
- No more context errors when sidecar is not available
- Graceful degradation when SidecarProvider is missing
- Tool calls work normally, diff viewer shows when sidecar is present
🐛 Bug Fixes:
- Sidecar panel now only shows when there's active content (not always visible)
- Fixed positioning: thin panel at right-[700px], content panel at right-0
- Main content properly adjusts width: mr-[760px] when sidecar is active
- Removed 'always show sample content' behavior

🎨 Layout Improvements:
- Proper layering: thin panel (z-30), content panel (z-20)
- Smooth transitions when opening/closing sidecar
- Content doesn't overlap with main chat area
- Collapsible behavior: click to close diff viewer

✅ Result:
- Sidecar only appears when tool calls have diff content
- Proper positioning without floating over main content
- Clean toggle behavior for opening/closing diff viewer
- Main chat area properly resizes when sidecar is active
🎯 **Complete Sidecar System Refactor:**

✅ **New Architecture:**
- Sidecar is now a proper secondary panel (700px wide)
- Can contain different views (diff viewer, future tools)
- Extensible system for adding new sidecar content
- Clean separation between sidecar content and main chat

✅ **Diff Viewer Integration:**
- Moved diff viewer directly into sidecar content
- Figma-accurate styling: #1E1E1E background, #232323 borders
- GitBranch icon instead of history icon
- Manual trigger via diff button (no auto-show)

✅ **User Experience:**
- Diff button positioned next to tool calls
- Only shows when there's actual diff content
- Sidecar hidden by default, only appears when activated
- Main content properly adjusts width (mr-[700px])
- Smooth transitions (300ms duration)

✅ **Code Cleanup:**
- Removed old diff viewer components (DiffViewer.tsx, SidecarPanel.tsx, DiffSidePanel.tsx)
- Removed old window event system
- Removed auto-show diff functionality
- Fixed TypeScript errors and unused variables
- Clean, maintainable codebase

✅ **Technical Implementation:**
- SidecarProvider wraps AppLayout for context
- MonacoDiffViewer component with proper styling
- Extensible SidecarView interface for future tools
- Proper React Context usage with optional fallback

🎨 **Result:**
- Clean secondary panel system matching Figma design
- Manual diff viewing with proper user control
- Extensible architecture for future sidecar tools
- No more floating panels or auto-popup behavior
🎨 **Major Diff Viewer Improvements:**

✅ **Proper Diff Highlighting:**
- Red background/text for removed lines (bg-red-900/30, text-red-300)
- Green background/text for added lines (bg-green-900/30, text-green-300)
- Gray text for context lines (text-gray-300)
- Left border indicators (border-red-500, border-green-500)

✅ **Split View Implementation:**
- True side-by-side diff view (Before | After)
- Proper line number tracking for both sides
- Separate parsing for removed vs added lines
- Color-coded headers (red for Before, green for After)

✅ **Line Numbers & Formatting:**
- Line numbers on the left (w-12, right-aligned)
- Diff prefixes (+, -, space) in center column
- Proper monospace font (font-mono)
- Clean, readable layout with proper spacing

✅ **Code Cleanup:**
- Removed hardcoded 'Changed import statements' section
- Removed non-functional 'Approve/Deny' buttons
- Removed fake line count displays
- Clean, minimal header with just filename and icon

✅ **Proper Diff Parsing:**
- Correctly handles unified diff format (@@ markers)
- Tracks line numbers for both before/after states
- Handles context lines, additions, and deletions
- Proper line numbering continuation

🎯 **Result:**
- Professional diff viewer matching Git/GitHub standards
- Clear visual distinction between changes
- Proper line-by-line comparison
- Clean, distraction-free interface
🔄 **View Mode Toggle Implementation:**

✅ **Toggle Button:**
- Clean toggle buttons in header (Split | Unified)
- Active state styling with proper highlighting
- Smooth transitions between view modes
- Professional GitHub-style toggle design

✅ **Split View (Default):**
- Side-by-side Before/After layout
- Color-coded headers (red/green)
- Single line numbers per side
- Clear visual separation

✅ **Unified View:**
- Traditional diff format in single column
- Dual line numbers (before | after)
- Same red/green highlighting
- Compact view for smaller screens

✅ **Enhanced Parsing:**
- Unified lines array with both line numbers
- Proper tracking of before/after line numbers
- Context lines show both numbers
- Added/removed lines show appropriate number

✅ **UI/UX Features:**
- Toggle state persists during diff session
- Consistent styling across both views
- Same syntax highlighting in both modes
- Proper line number alignment

🎯 **Result:**
- Complete diff viewer with both view modes
- Professional toggle interface
- Flexible viewing options for different use cases
- Maintains all existing highlighting and features
🎨 **Color Consistency Improvements:**

✅ **Replaced Hardcoded Colors with Theme Variables:**
-  →
-  →
-  →
-  →
-  →

✅ **Updated Diff Highlighting:**
- Red:  →  (lighter, more subtle)
- Green:  →  (lighter, more subtle)
- Text colors:  → ,  →
- Line numbers:  →

✅ **Toggle Button Styling:**
- Background:  →
- Active state:  →
- Hover states: Consistent with app theme

✅ **Header Improvements:**
- Split view headers:  instead of colored backgrounds
- Consistent text colors throughout
- Proper border styling with theme variables

✅ **Sidecar Panel:**
- Background:
- Borders:
- Text colors:  and

🎯 **Result:**
- Diff viewer now matches the application's design system
- Consistent with main chat, settings, and other UI components
- Proper theme integration for light/dark mode support
- Professional, cohesive visual experience
…ll-based activation

🎯 **Enhanced Sidecar Panel System:**

✅ **Collapsed Panel Always Visible:**
- 64px (w-16) thin panel on the right side when sidecar is not expanded
- Shows floating action buttons for available sidecar actions
- Buttons highlight when their associated message is closest to viewport center
- Smooth transitions between collapsed and expanded states

✅ **Smart Scroll-Based Activation:**
- Automatically detects which message is closest to the middle of the viewport
- Highlights the corresponding action button in the collapsed panel
- Uses data-message-id attributes for message tracking
- Real-time scroll event handling with viewport center calculation

✅ **Action Registration System:**
- Messages with diff content automatically register actions with the sidecar
- Actions include icon, label, messageId, and onClick handler
- Automatic cleanup when messages unmount or change
- Supports multiple action types (currently diff viewer, extensible for future tools)

✅ **Enhanced Layout Management:**
- Main content adjusts margin: mr-16 (collapsed) or mr-[700px] (expanded)
- Fixed positioning for consistent sidecar placement
- Z-index layering: collapsed panel (z-40), expanded panel (z-50)
- Smooth 300ms transitions for all state changes

✅ **Improved User Experience:**
- Tooltips on collapsed panel buttons with proper positioning
- Visual feedback: hover states, active states, scaling effects
- Buttons scale up (scale-110) when their message is in focus
- Primary color highlighting for active message actions

✅ **Technical Implementation:**
- SidecarAction interface with id, icon, label, onClick, messageId
- useEffect hooks for scroll tracking and action registration
- Proper TypeScript types and error handling
- Cleanup functions to prevent memory leaks

🎨 **Visual Design:**
- Consistent with application color palette
- Proper border styling with borderSubtle
- Background colors: background-default, background-muted
- Text colors: textStandard, textSubtle, primary
- Smooth hover and focus transitions

The sidecar now provides a persistent thin panel that shows contextual actions, with the diff viewer being the first implementation. Actions automatically highlight based on scroll position, and the system is extensible for future sidecar tools.
🎯 **Simplified Sidecar Panel System:**

✅ **Route-Based Sidecar Visibility:**
- Sidecar only shows on chat routes (/ and /pair)
- Hidden on settings, sessions, and other non-chat views
- ConditionalSidecarProvider determines visibility based on current route

✅ **Simplified Floating Action Buttons:**
- Removed complex scroll tracking and action registration system
- Action buttons render inline with messages using relative positioning
- Positioned at top-2 -right-20 to float over collapsed sidecar panel
- Buttons only show when sidecar is collapsed (!sidecar.activeView)

✅ **Clean Architecture:**
- Eliminated complex useEffect hooks for scroll tracking
- Removed action registration/cleanup system
- Messages directly render their own floating action buttons
- Simpler state management with fewer moving parts

✅ **Enhanced User Experience:**
- Action buttons scroll naturally with their associated messages
- No complex viewport center calculations needed
- Buttons appear exactly where users expect them (next to relevant messages)
- Consistent positioning without complex JavaScript calculations

✅ **Technical Improvements:**
- Removed data-message-id attributes (no longer needed)
- Simplified SidecarLayout component (removed action system)
- Cleaner GooseMessage component with direct button rendering
- Better performance without scroll event listeners

🎨 **Visual Design:**
- Floating buttons positioned over collapsed 64px sidecar panel
- Proper z-index layering (z-50 for buttons)
- Hover effects with scale and color transitions
- Tooltips positioned to the left of buttons

The sidecar system is now much simpler and more reliable, with action buttons that naturally scroll with their messages and appear exactly where users expect them to be.
🎯 **Route-Based Sidecar Control:**

✅ **Clarified Route Logic:**
- Sidecar only shows on specific chat routes:  and
- Hidden on welcome/onboarding routes: ,
- Hidden on all other routes: , , , etc.

✅ **Route Structure:**
-  and  are separate full-screen routes
- Main layout route  contains nested chat routes with ConditionalSidecarProvider
- Sidecar provider only wraps the main chat layout, not onboarding flows

✅ **Expected Behavior:**
- New users on  or : No sidecar
- Users on main chat route : Collapsed sidecar visible (64px)
- Users on pair chat route : Collapsed sidecar visible (64px)
- Users on settings/sessions/etc: No sidecar (outside main layout)

The sidecar now correctly appears only where it should - in the main chat interface.
🎯 **Smart Sidecar Visibility:**

✅ **Conditional Panel Display:**
- Collapsed sidecar panel only appears when messages have diff content
- No more empty sidecar panel on home page or empty chat states
- Background panel rendered per-message with action buttons

✅ **Improved Layout Logic:**
- Removed automatic mr-16 margin from main content
- Main content only gets margin when sidecar is actually expanded (mr-[700px])
- Background panel (z-30) renders behind action buttons (z-50) when needed

✅ **Message-Driven Approach:**
- Each message with diff content renders its own background panel
- Action buttons float over their individual background panels
- Multiple messages with diffs will each show their own action buttons
- Clean separation of concerns - messages control their own sidecar presence

✅ **User Experience:**
- Empty chat states show no sidecar (clean interface)
- Sidecar only appears when there's actually something to interact with
- Smooth transitions when action buttons appear/disappear
- No visual clutter when not needed

The sidecar now intelligently appears only when there are actual actions available, providing a much cleaner experience for users starting new conversations or on empty chat states.
@Kvadratni Kvadratni force-pushed the feature/global-checkpoints branch from c513e87 to 7c14908 Compare July 14, 2025 16:19
@DOsinga
Copy link
Collaborator

DOsinga commented Jul 16, 2025

I thought about this some more and I would like to get the diff bit in, but for the versioning I do think we want to use git. Can we split the functionality? i.e. change the file tools to produce a diff and then store that into the conversation as an attachment and show that to the user when they click open the tool result?

@DOsinga DOsinga self-assigned this Jul 16, 2025
@Kvadratni
Copy link
Collaborator Author

I thought about this some more and I would like to get the diff bit in, but for the versioning I do think we want to use git. Can we split the functionality? i.e. change the file tools to produce a diff and then store that into the conversation as an attachment and show that to the user when they click open the tool result?

yes I will see what I can do, once I properly rebase on the latest re-design

@Kvadratni
Copy link
Collaborator Author

replaced by #3493

@angiejones angiejones deleted the feature/global-checkpoints branch October 15, 2025 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Comments