-
Notifications
You must be signed in to change notification settings - Fork 689
feat(amazonq): add diff animation support for chat responses #7492
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
Closed
Closed
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
0b168f9
Add language server to workspace
MarcoWang3 562dc53
Write diffController to open files on changing and render diff
MarcoWang3 10b64f1
Update message.ts to start animation while receiving ChatResult
MarcoWang3 ad520c1
Diff animation render
MarcoWang3 5fa6623
Animation controller
MarcoWang3 3b9ed72
Render the diff animation totally based on frontend
MarcoWang3 43ea2aa
Create a temporary file for showing diff animation
MarcoWang3 45920dc
Fix the remove lines not shown problems
MarcoWang3 b279a7b
Change the streamline type to the exact Cline one
MarcoWang3 5f17170
fix a Type error
MarcoWang3 a7ded23
optimize the animation scope detection
MarcoWang3 03d94f8
Apply vscode's diffview and display static diff when clicking on file…
MarcoWang3 ddb89f0
Display diff animation progressively
MarcoWang3 7de7ffc
refactor the diffAnimation codes and write unit tests
MarcoWang3 2b721ef
Merge Stream_Line_Optimization into feature/code-diff
MarcoWang3 d4332f0
FIx duplication error
MarcoWang3 f1738de
Fix final duplicate in controller test - use helper function
MarcoWang3 d33e110
Eliminate final duplicate with setupAnimationAndStop helper
MarcoWang3 3aebfd1
fix duplicate issues
MarcoWang3 0652aa6
Final duplicate elimination - refactor isAnimating test with helper
MarcoWang3 ee1e101
Eliminate final duplicate - use setupAnimationAndCheckStaticDiff helper
MarcoWang3 3e5c026
fix an mock error in test
MarcoWang3 82a8e1a
fix a test duplication error
MarcoWang3 25e8d35
fix a duplication error
MarcoWang3 8e76fef
remove duplicate tests
MarcoWang3 5bdef4b
remove a duplicate test
MarcoWang3 8d4d9fe
remove a duplicate test
MarcoWang3 878dfe8
remove a duplicate test
MarcoWang3 a8c02b5
fix a test logic error
MarcoWang3 a52e0d2
Remove unnecessary workspace file from decorations folder
MarcoWang3 e4c2449
Revert workspace file to original state without language-servers path
MarcoWang3 ffdf088
Add language-servers back to workspace
MarcoWang3 86dbf85
realize chunk by chunk animaton
MarcoWang3 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,157 @@ | ||
# DiffAnimation Module Refactoring | ||
|
||
## Overview | ||
|
||
The `diffAnimation` directory has been refactored from 2 large files (~1700 lines total) into 9 smaller, focused modules following the Single Responsibility Principle. This improves maintainability, testability, and code organization while preserving all existing functionality. | ||
|
||
## File Structure | ||
|
||
### Core Files | ||
|
||
- **`diffAnimationHandler.ts`** - Main orchestrator and public API (reduced from ~800 to ~300 lines) | ||
- **`diffAnimationController.ts`** - Animation control and coordination (reduced from ~900 to ~400 lines) | ||
|
||
### Supporting Components | ||
|
||
- **`types.ts`** - Shared TypeScript interfaces and types | ||
- **`fileSystemManager.ts`** - File system operations, path resolution, and file watching | ||
- **`chatProcessor.ts`** - Chat message processing and tool use handling | ||
- **`animationQueueManager.ts`** - Animation queuing and coordination logic | ||
- **`webviewManager.ts`** - Webview creation, HTML generation, and messaging | ||
- **`diffAnalyzer.ts`** - Diff calculation, line parsing, and scan planning | ||
- **`vscodeIntegration.ts`** - VS Code API integration and utilities | ||
|
||
## Architecture | ||
|
||
``` | ||
DiffAnimationHandler (Main Entry Point) | ||
├── FileSystemManager (File Operations) | ||
├── ChatProcessor (Message Processing) | ||
├── AnimationQueueManager (Queue Management) | ||
└── DiffAnimationController (Animation Control) | ||
├── WebviewManager (UI Management) | ||
├── DiffAnalyzer (Diff Logic) | ||
└── VSCodeIntegration (VS Code APIs) | ||
``` | ||
|
||
## Key Benefits | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like the section but does not add values for future users, testers. |
||
|
||
### 1. **Improved Maintainability** | ||
|
||
- Each component has a single, clear responsibility | ||
- Easier to locate and modify specific functionality | ||
- Reduced cognitive load when working on individual features | ||
|
||
### 2. **Better Testability** | ||
|
||
- Components can be unit tested in isolation | ||
- Dependencies are injected, making mocking easier | ||
- Clear interfaces between components | ||
|
||
### 3. **Enhanced Reusability** | ||
|
||
- Components can be reused in different contexts | ||
- Easier to extract functionality for other features | ||
- Clear separation of concerns | ||
|
||
### 4. **Preserved Functionality** | ||
|
||
- All existing public APIs remain unchanged | ||
- No breaking changes to external consumers | ||
- Backward compatibility maintained | ||
|
||
## Component Responsibilities | ||
|
||
### FileSystemManager | ||
|
||
- File system watching and event handling | ||
- Path resolution and normalization | ||
- File content capture and preparation | ||
- Directory creation and file operations | ||
|
||
### ChatProcessor | ||
|
||
- Chat message parsing and processing | ||
- Tool use detection and handling | ||
- Message deduplication | ||
- File write preparation coordination | ||
|
||
### AnimationQueueManager | ||
|
||
- Animation queuing for concurrent file changes | ||
- Animation state management | ||
- Queue processing and coordination | ||
- Statistics and monitoring | ||
|
||
### WebviewManager | ||
|
||
- Webview panel creation and management | ||
- HTML content generation | ||
- Message passing between extension and webview | ||
- Auto-scroll control and user interaction handling | ||
|
||
### DiffAnalyzer | ||
|
||
- Diff calculation and analysis | ||
- Changed region detection | ||
- Scan plan creation for animations | ||
- Animation timing calculations | ||
- Complexity analysis for optimization | ||
|
||
### VSCodeIntegration | ||
|
||
- VS Code API abstractions | ||
- Built-in diff view integration | ||
- Editor operations and file management | ||
- Status messages and user notifications | ||
- Configuration and theme management | ||
|
||
## Migration Notes | ||
|
||
### For Developers | ||
|
||
- Import paths remain the same for main classes | ||
- All public methods and interfaces are preserved | ||
- Internal implementation is now modular but transparent to consumers | ||
|
||
### For Testing | ||
|
||
- Individual components can now be tested in isolation | ||
- Mock dependencies can be easily injected | ||
- Test coverage can be more granular and focused | ||
|
||
### For Future Development | ||
|
||
- New features can be added to specific components | ||
- Components can be enhanced without affecting others | ||
- Clear boundaries make refactoring safer and easier | ||
|
||
## ESLint Compliance | ||
|
||
All files follow the project's ESLint configuration: | ||
|
||
- Proper TypeScript typing | ||
- Consistent code formatting | ||
- No unused imports or variables | ||
- Proper error handling patterns | ||
|
||
## Performance Considerations | ||
|
||
- No performance impact from refactoring | ||
- Same memory usage patterns | ||
- Identical animation behavior | ||
- Preserved optimization strategies | ||
|
||
## Future Enhancements | ||
|
||
The modular structure enables several future improvements: | ||
|
||
1. **Enhanced Testing**: Unit tests for individual components | ||
2. **Performance Monitoring**: Better metrics collection per component | ||
3. **Feature Extensions**: Easier addition of new animation types | ||
4. **Configuration**: Component-level configuration options | ||
5. **Debugging**: Better error isolation and debugging capabilities | ||
|
||
## Conclusion | ||
|
||
This refactoring successfully breaks down the large `diffAnimation` codebase into manageable, focused components while maintaining full backward compatibility and functionality. The new structure provides a solid foundation for future development and maintenance. |
178 changes: 178 additions & 0 deletions
178
packages/amazonq/src/lsp/chat/diffAnimation/animationQueueManager.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,178 @@ | ||
/*! | ||
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
import { getLogger } from 'aws-core-vscode/shared' | ||
import { QueuedAnimation, PendingFileWrite } from './types' | ||
import { FileSystemManager } from './fileSystemManager' | ||
|
||
export class AnimationQueueManager { | ||
// Track which files are being animated | ||
private animatingFiles = new Set<string>() | ||
// Animation queue for handling multiple changes | ||
private animationQueue = new Map<string, QueuedAnimation[]>() | ||
|
||
constructor( | ||
private fileSystemManager: FileSystemManager, | ||
private startFullAnimation: ( | ||
filePath: string, | ||
originalContent: string, | ||
newContent: string, | ||
toolUseId: string | ||
) => Promise<void>, | ||
private startPartialAnimation: ( | ||
filePath: string, | ||
originalContent: string, | ||
newContent: string, | ||
changeLocation: { startLine: number; endLine: number }, | ||
toolUseId: string | ||
) => Promise<void> | ||
) {} | ||
|
||
/** | ||
* Check if a file is currently being animated | ||
*/ | ||
public isAnimating(filePath: string): boolean { | ||
return this.animatingFiles.has(filePath) | ||
} | ||
|
||
/** | ||
* Mark file as animating | ||
*/ | ||
public markAsAnimating(filePath: string): void { | ||
this.animatingFiles.add(filePath) | ||
} | ||
|
||
/** | ||
* Mark file as no longer animating | ||
*/ | ||
public markAsNotAnimating(filePath: string): void { | ||
this.animatingFiles.delete(filePath) | ||
} | ||
|
||
/** | ||
* Queue an animation for later processing | ||
*/ | ||
public queueAnimation(filePath: string, animation: QueuedAnimation): void { | ||
const queue = this.animationQueue.get(filePath) || [] | ||
queue.push(animation) | ||
this.animationQueue.set(filePath, queue) | ||
getLogger().info(`[AnimationQueueManager] 📋 Queued animation for ${filePath} (queue size: ${queue.length})`) | ||
} | ||
|
||
/** | ||
* Start animation and handle queuing logic | ||
*/ | ||
public async startAnimation(filePath: string, pendingWrite: PendingFileWrite, newContent: string): Promise<void> { | ||
// If already animating, queue the change | ||
if (this.isAnimating(filePath)) { | ||
this.queueAnimation(filePath, { | ||
originalContent: pendingWrite.originalContent, | ||
newContent, | ||
toolUseId: pendingWrite.toolUseId, | ||
changeLocation: pendingWrite.changeLocation, | ||
}) | ||
return | ||
} | ||
|
||
// Mark as animating | ||
this.markAsAnimating(filePath) | ||
|
||
try { | ||
// Check if we have change location for partial update | ||
if (pendingWrite.changeLocation) { | ||
// Use partial animation for targeted changes | ||
await this.startPartialAnimation( | ||
filePath, | ||
pendingWrite.originalContent, | ||
newContent, | ||
pendingWrite.changeLocation, | ||
pendingWrite.toolUseId | ||
) | ||
} else { | ||
// Use full file animation | ||
await this.startFullAnimation( | ||
filePath, | ||
pendingWrite.originalContent, | ||
newContent, | ||
pendingWrite.toolUseId | ||
) | ||
} | ||
|
||
// Process queued animations | ||
await this.processQueuedAnimations(filePath) | ||
} finally { | ||
// Always mark as not animating when done | ||
this.markAsNotAnimating(filePath) | ||
} | ||
} | ||
|
||
/** | ||
* Process queued animations for a file | ||
*/ | ||
private async processQueuedAnimations(filePath: string): Promise<void> { | ||
const queue = this.animationQueue.get(filePath) | ||
if (!queue || queue.length === 0) { | ||
return | ||
} | ||
|
||
const next = queue.shift() | ||
if (!next) { | ||
return | ||
} | ||
|
||
getLogger().info( | ||
`[AnimationQueueManager] 🎯 Processing queued animation for ${filePath} (${queue.length} remaining)` | ||
) | ||
|
||
// Use the current file content as the "original" for the next animation | ||
const currentContent = await this.fileSystemManager.getCurrentFileContent(filePath) | ||
|
||
// Create a new pending write for the queued animation | ||
const queuedPendingWrite: PendingFileWrite = { | ||
filePath, | ||
originalContent: currentContent, | ||
toolUseId: next.toolUseId, | ||
timestamp: Date.now(), | ||
changeLocation: next.changeLocation, | ||
} | ||
|
||
// Recursively start the next animation | ||
await this.startAnimation(filePath, queuedPendingWrite, next.newContent) | ||
} | ||
|
||
/** | ||
* Get animation statistics | ||
*/ | ||
public getAnimationStats(): { animatingCount: number; queuedCount: number; filePaths: string[] } { | ||
let queuedCount = 0 | ||
for (const queue of this.animationQueue.values()) { | ||
queuedCount += queue.length | ||
} | ||
|
||
return { | ||
animatingCount: this.animatingFiles.size, | ||
queuedCount, | ||
filePaths: Array.from(this.animatingFiles), | ||
} | ||
} | ||
|
||
/** | ||
* Clear all queues and reset state | ||
*/ | ||
public clearAll(): void { | ||
this.animatingFiles.clear() | ||
this.animationQueue.clear() | ||
getLogger().info('[AnimationQueueManager] 🧹 Cleared all animation queues and state') | ||
} | ||
|
||
/** | ||
* Clear queue for a specific file | ||
*/ | ||
public clearFileQueue(filePath: string): void { | ||
this.animationQueue.delete(filePath) | ||
this.markAsNotAnimating(filePath) | ||
getLogger().info(`[AnimationQueueManager] 🧹 Cleared queue for ${filePath}`) | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This does not add value!