-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat: Add Files Changed Overview (FCO) - Real-time file diff tracking and UI (refactored from #5626) #8193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
… and UI Files Changed Overview is a new experimental feature that provides real-time tracking of file modifications during AI coding sessions. FCO displays a live diff view in the sidebar, helping users understand what files are being changed by the AI assistant and review changes before accepting them. ## Feature Overview The FCO system tracks file modifications through git-like checkpoints and provides: - Real-time file change tracking with baseline diffs - Interactive UI to view, accept, or reject changes - Subtask file collection and parent task aggregation - Performance optimization with virtualization for large file lists - Full internationalization support (15+ languages) ## New Files Created ### Core Services (`src/services/files-changed/`) **FilesChangedManager.ts** - Core file tracking engine - Minimal in-memory store for file changes with Map-based storage - Tracks files against git checkpoint baselines (default: HEAD) - Provides upsert/remove operations for individual file changes - Integrates with FileContextTracker for roo_edited events **FilesChangedMessageHandler.ts** - Central orchestrator and event handler - Manages FCO lifecycle (enable/disable experiment toggle) - Handles checkpoint events and file diff processing - Orchestrates subtask completion and file collection - Implements defensive fallback for missed roo_edited events - Provides message passing bridge to webview UI **TaskFilesChangedState.ts** - Per-task state management - Encapsulates FilesChangedManager instance per task - Manages queued child URIs from subtask completion - Handles state cloning for task inheritance - Provides waiting/checkpoint synchronization flags ### Tests (`src/services/files-changed/__tests__/`) **FilesChangedManager.test.ts** - Unit tests for core manager - Tests file upsert/remove operations and state management - Validates baseline checkpoint handling - Ensures proper Map-based storage behavior **FilesChangedMessageHandler.test.ts** - Integration tests for orchestrator - Tests experiment toggle lifecycle and state transitions - Validates checkpoint event processing and diff generation - Tests subtask completion flow and child file collection - Includes comprehensive fallback mechanism testing for missed events ### UI Components (`webview-ui/src/components/file-changes/`) **FilesChangedOverview.tsx** - React component for sidebar UI - Self-managing component with checkpoint event listeners - Performance-optimized with virtualization for 50+ files - Collapsible file list with expand/collapse state - File action buttons (view, accept, reject) with debounced handlers - Responsive design with mobile-friendly styling ### Internationalization (`webview-ui/src/i18n/locales/*/`) **file-changes.json** - Translation files for 15+ languages - Supports: en, es, fr, de, it, pt-BR, ru, zh-CN, zh-TW, ja, ko, hi, vi, tr, nl, pl, ca, id - Translates UI labels: "Files Changed", "Accept", "Reject", "View Changes", etc. - Maintains consistent terminology across all supported languages ## Modified Files - Core Integration ### Task Management (`src/core/task/Task.ts`) - Added FilesChangedState integration to Task class - Implemented getFilesChangedState() and disposeFilesChangedState() methods - Integrated FCO state management into task lifecycle ### Main Provider (`src/core/webview/ClineProvider.ts`) - Integrated FilesChangedMessageHandler as core service - Added FCO initialization in constructor with proper dependency injection - Modified finishSubTask() to await async handleChildTaskCompletion() - Updated dispose() to clean up FCO resources ### File Context Tracking (`src/core/context-tracking/FileContextTracker.ts`) - Enhanced roo_edited event emission to include FCO integration - Added file change metadata for better diff tracking - Improved event payload structure for FCO consumption ### Message Types (`src/shared/ExtensionMessage.ts` & `WebviewMessage.ts`) - Added FilesChangedToggle message type for experiment control - Added FilesChangedUpdate message type for real-time UI updates - Extended message union types to support FCO communication ### Experiments (`src/shared/experiments.ts`) - Added FILES_CHANGED_OVERVIEW experiment ID and default configuration - Integrated with existing experiment framework (disabled by default) - Added experiment metadata for UI display ### Types (`packages/types/src/`) - Added FileChange and FileChangeset interfaces - Defined file change operations (edit, create, delete) - Added experiment type definitions for FCO ### Checkpoint Integration (`src/services/checkpoints/ShadowCheckpointService.ts`) - Enhanced checkpoint events to trigger FCO updates - Added baseline checkpoint support for diff calculations - Improved git diff integration for file change detection ## Architecture Highlights ### Event-Driven Design - FCO listens to checkpoint events from ShadowCheckpointService - FileContextTracker emits roo_edited events consumed by FCO - WebView receives real-time updates via message passing ### Performance Optimizations - Virtualization for file lists with 50+ items (10 visible items max) - Debounced UI actions to prevent excessive API calls - Map-based file storage for O(1) lookup/update operations ### Separation of Concerns - FilesChangedManager: Pure file state management - FilesChangedMessageHandler: Event orchestration and business logic - TaskFilesChangedState: Per-task state isolation - FilesChangedOverview: UI presentation layer ### Defensive Programming - Fallback mechanisms for missed file tracking events - Graceful error handling for git operations - Null-safe state access throughout the codebase - Comprehensive edge case testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Summary
The Files Changed Overview (FCO) feature implementation is comprehensive and well-structured. The architecture follows good separation of concerns with clear responsibilities for each component. However, there are several critical issues that need to be addressed before merging.
Critical Issues Found:
- Memory leak in FilesChangedMessageHandler due to uncleaned debounce timeout
- Race condition in task cancellation that could cause FCO state loss
Important Improvements Needed:
- Inefficient diff calculation algorithm
- Missing React error boundaries
- Incomplete test coverage for error scenarios
Minor Suggestions:
- Type safety improvements
- Performance optimization for virtualization threshold
- Accessibility enhancements
Overall, this is a solid implementation that adds valuable functionality for tracking file changes during AI coding sessions. Once the critical issues are resolved, this will be a great addition to the codebase.
webview-ui/src/components/file-changes/FilesChangedOverview.tsx
Outdated
Show resolved
Hide resolved
- Added focused unit tests highlighting review concerns (diff accuracy, debounce cleanup, checkpoint failure, rehydrate state, UI virtualization, error- boundary containment). - Replaced calculateLineDifferences with diffLines and updated coverage so insert/delete counts are correct. - Hardened FilesChangedMessageHandler (debounce cleanup, safe enable/disable, state replay after rehydrate, checkpoint-failure guard). - Simplified checkpoint mock usage in tests (vi.mocked(getCheckpointService)), ensuring the failure case is properly asserted. - Verified targeted specs: - pnpm -C Roo-Code/src test services/files-changed/__tests__/FilesChangedMessageHandler.test.ts - pnpm -C Roo-Code/webview-ui test src/components/file-changes/__tests__/FilesChangedOverview.spec.tsx - pnpm -C Roo-Code/webview-ui test src/components/chat/__tests__/TaskHeader.fco-boundary.spec.tsx - FilesChangedOverview component - Virtualization threshold is still 50; we only wrote a failing test earlier but haven’t lowered the constant or adjusted behavior. - No dedicated error boundary: the TaskHeader test we added expects one, but the UI code still renders the component directly. - Accessibility/hardening suggestions (e.g., button focus states, aria refinements) haven’t been implemented. - Any “type-safety” or “performance” tweaks flagged in the review beyond the diff/handler items are pending.
@roomote-agent use code reviewer to review this pr. |
Hi @playcations! I've seen your request to review this PR. I'll perform a comprehensive code review of the Files Changed Overview feature. Let me analyze the changes and provide detailed feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the comprehensive implementation of the Files Changed Overview feature! The architecture is well-structured with good separation of concerns. I've reviewed the changes and found that the critical issues from the previous review have been addressed. However, there are still some areas that need attention before merging.
Resolved Issues from Previous Review
- ✅ Memory leak in FilesChangedMessageHandler - properly cleaned with clearTrackerDebounce()
- ✅ Race condition in task cancellation - FCO state now preserved through cloneFrom()
New Findings
I've identified one critical issue and several improvements that would enhance the robustness and maintainability of this feature.
* and displays file changes. It manages its own state and communicates with the backend | ||
* through VS Code message passing. | ||
*/ | ||
const FilesChangedOverview: React.FC = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical Issue: This component lacks error boundary protection. If an error occurs during rendering or event handling, it could crash the entire UI. Consider wrapping this component with an error boundary or adding try-catch blocks in critical event handlers. Would implementing a dedicated ErrorBoundary wrapper component work for your use case?
const FilesChangedOverview: React.FC = () => { | ||
const { t } = useTranslation() | ||
const { experiments } = useExtensionState() | ||
const filesChangedEnabled = !!experiments?.[EXPERIMENT_IDS.FILES_CHANGED_OVERVIEW] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing null check - the experiments object might be undefined. Consider using optional chaining:
const filesChangedEnabled = experiments?.[EXPERIMENT_IDS.FILES_CHANGED_OVERVIEW] ?? false;
Hi @playcations! I've completed my comprehensive review of the Files Changed Overview feature. The implementation looks solid overall - the architecture is well-structured with good separation of concerns, and I can confirm that the critical issues from the previous review (memory leak and race condition) have been properly addressed. I've left inline comments on the PR with one critical issue that needs attention (missing React error boundary) and several suggestions for improvements. The most important items to address before merging are:
The other suggestions are nice-to-have improvements for performance and maintainability. Great work on this feature - it will be a valuable addition once these issues are resolved! 🎉 |
1. Button Focus States - Added focus-visible:outline styles to all buttons for proper keyboard navigation accessibility 2. Event Handler Error Handling - Added try-catch blocks around all vscode.postMessage calls to prevent UI crashes from communication errors
@roomote-agent use code reviewer to review this pr. |
Hi @playcations! I see your request to review PR #8193. I will perform a comprehensive code review of the Files Changed Overview feature. Let me check out the branch and analyze the changes. |
@roomote-agent use code reviewer to review this pr. |
Hi @playcations! I see your request to review PR #8193. I will perform a comprehensive code review of the Files Changed Overview feature. Let me check out the branch and analyze the changes. |
🔍 Code Review Complete@playcations I've completed a comprehensive review of the Files Changed Overview (FCO) feature in PR #8193. ✅ Overall Assessment: APPROVEDRating: 8.5/10 - This is a well-implemented feature with solid architecture and good test coverage. 🎯 Key Strengths
🔧 Minor Improvements Suggested
✅ Test Results
🛡️ Security & Performance
📊 Impact Analysis
🚀 RecommendationThis PR is ready to merge. The implementation successfully addresses the previous memory leak and race condition issues. The feature provides valuable real-time file tracking capabilities that will enhance the user experience during AI coding sessions. The minor suggestions above can be addressed in follow-up PRs and don't block the current implementation. Great work on this feature! 👏 |
A not e Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
splitting on both separators: uri.split(/[/\\]/) for getFileName/getFilePath. Prefer git diff stats for line counts and dropped full-content fallback for large files. Use getDiffStats() and simplify logic in mapDiffToFileChange(). Review issues Added fcoTextDocumentContentProvider.ts Added proper diff opening and closing of files to fcoTextDocumentContentProvider.ts better debounce and added batching timeoutRef should be cross-env safe. Prefer ReturnType<typeof setTimeout> over NodeJS.Timeout to avoid DOM vs Node typing issues. fixed language parenthesis issue
Okay, that should cover the changes mentioned here. I know previously you mentioned logs, and I am sure there could be excessive logs but I can take care of that tomorrow. |
|
@playcations merge conflicts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Auto-generated review. Please have Dan double-check this before taking action.
You should undo that one and let @daniel-lxs handle the merge. That's messy. |
7912bad
to
e2dc694
Compare
Merged fileChanges/baseline properties from feature branch with upsellId/list/organizationId properties from main branch. Removed duplicate upsellId property. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
I missed a duplicate line in the merge conflict. I can revert again if need be but I tested this and it seems fine. |
That was my bad, My at work env sucks I am going to back out of that last push. |
011e5cf
to
a9ab507
Compare
Hardened the listener for FilesChangedOverview Added more validation for each type. @danial-lxs LMK if I am wrong but from what I looked up, "event.source === window" is not needed since vsCode already handles this. Also, that version (and many other versions) of the check always seems to fail. If you know something I don't pls let me know.
- Diff cleanup added. - Listener for users closing diff window added to clean up files. - added cleanup in extension.ts - test modified for new listener added
…oo-Code into fco-clean-refactor
removed the leading slash when it reopens our fco-diff scheme, so the provider was looking up /file instead of file, and returning empty content for earlier rows.
FilesChangedMessageHandler was posting { filesChanged: undefined } when the list emptied. - clearFilesChangedDisplay now sends filesChanged: null. - postChanges uses null for the empty state. - Updated the unit expectations in FilesChangedMessageHandler.test.ts to match.
Instead of files opening when file is accepted or rejected, only open the file if the diff is already opened.
Switching between tasks would prevent fco to restart properly. - Added a guard when attaching to a task so a brand-new FilesChangedManager immediately sets the task into “waiting for checkpoint” (src/services/files-changed/FilesChangedMessageHandler.ts:268-294) - Updated handleRejectAllFileChanges to honor the optional uris list by reverting only the specified files, removing them from the manager with rejectChange, and leaving the rest untouched. The old logic always ran rejectAll, clearing the whole queue and forcing a checkpoint wait even for partial rejections. - Extended the unit suite (src/services/files-changed/__tests__/FilesChangedMessageHandler.test.ts) with scenarios for both changes: one ensures a newly attached task stays idle until a checkpoint, the other verifies partial rejections touch only the targeted files.
Related GitHub Issue
Closes: #4454 and #5626
Roo Code Task Context (Optional)
Description
Files Changed Overview is a new feature that provides real-time tracking of file modifications during AI coding sessions. FCO displays a live diff view in the sidebar, helping users understand what files are being changed by the AI assistant and review changes before accepting them.
The FCO system tracks file modifications through git-like checkpoints and provides:
New Files Created
Core Services (
src/services/files-changed/
)FilesChangedManager.ts - Core file tracking engine
FilesChangedMessageHandler.ts - Central orchestrator and event handler
TaskFilesChangedState.ts - Per-task state management
Tests (
src/services/files-changed/__tests__/
)FilesChangedManager.test.ts - Unit tests for core manager
FilesChangedMessageHandler.test.ts - Integration tests for orchestrator
UI Components (
webview-ui/src/components/file-changes/
)FilesChangedOverview.tsx - React component for sidebar UI
Internationalization (
webview-ui/src/i18n/locales/*/
)file-changes.json - Translation files for 15+ languages
Modified Files - Core Integration
Task Management (
src/core/task/Task.ts
)Main Provider (
src/core/webview/ClineProvider.ts
)File Context Tracking (
src/core/context-tracking/FileContextTracker.ts
)Message Types (
src/shared/ExtensionMessage.ts
&WebviewMessage.ts
)Experiments (
src/shared/experiments.ts
)Types (
packages/types/src/
)Checkpoint Integration (
src/services/checkpoints/ShadowCheckpointService.ts
)Architecture Highlights
Event-Driven Design
Performance Optimizations
Separation of Concerns
Defensive Programming
Test Procedure
run the following tests:
src/core/webview/tests/ClineProvider.spec.ts
src/services/checkpoints/tests/ShadowCheckpointService.spec.ts
src/services/file-changes/tests/FileChangeManager.test.ts
webview-ui/src/components/file-changes/tests/FilesChangedOverview.spec.tsx
webview-ui/src/components/settings/tests/UISettings.spec.tsx
webview-ui/src/context/tests/ExtensionStateContext.spec.tsx
check all languages exist.
open chat and ask it to modify some files, make new files, different types of tool calls, etc. accept and reject the changes
open settings menu and under interfaces, turn off the feature, go through the steps again
turn on the feature on last time and go through the steps again.
make sure checkpoints still work.
when restoring a checkpoint, fco clears and disappears.
Pre-Submission Checklist
Screenshots / Videos
Before:
After Checkpoint is hit:
Settings option:
As this is still experimental, I did not add any documentation
Documentation Updates
As this is still experimental, I did not add any documentation
Additional Notes
Get in Touch
Playcations on Discord
Important
Introduces Files Changed Overview (FCO) for real-time file modification tracking with UI, core services, and internationalization support, integrated into task management lifecycle.
FilesChangedManager.ts
: Core file tracking engine with Map-based storage.FilesChangedMessageHandler.ts
: Manages FCO lifecycle and event handling.TaskFilesChangedState.ts
: Manages per-task state.FilesChangedOverview.tsx
: React component for sidebar UI with virtualization.Task.ts
for task lifecycle management.ClineProvider.ts
: Initializes FCO and manages task state transitions.FileContextTracker.ts
: Emits events for FCO integration.webview-ui/src/i18n/locales/*/file-changes.json
.This description was created by
for d7cb1e3. You can customize this summary. It will automatically update as commits are pushed.