-
Notifications
You must be signed in to change notification settings - Fork 663
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
base: feature/code-diff
Are you sure you want to change the base?
Conversation
- Integrated DiffAnimationHandler functionality - Resolved merge conflicts in messages.ts - Added diff animation components and tests
|
dea9ec9
to
e4c2449
Compare
|
||
## 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. |
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!
} | ||
} | ||
} catch (error) { | ||
getLogger().error(`[ChatProcessor] ❌ Failed to process chat result: ${error}`) |
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.
Do we show any error to the user in this case? Was that handled somewhere?
const lines = newContent.split('\n') | ||
return { | ||
startLine: 0, | ||
endLine: Math.min(lines.length - 1, 99), // Cap at 100 lines |
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.
Do we need this limit? any reason if yes, please add comment here.
} | ||
|
||
// Add context lines (3 before and after) | ||
const contextLines = 3 |
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.
Same question?
return i < arr.length - 1 || l !== '' | ||
}) | ||
|
||
if (change.removed) { |
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.
Can you refactor this function?
scanDelay: number | ||
totalDuration: number | ||
} { | ||
const scanDelay = scanPlanLength > 50 ? 40 : 70 |
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.
Can you add comment how we came up with this arbitrary numbers?
}) | ||
|
||
// Pre-add lines that are before the scan region (context) | ||
for (let i = 0; i < Math.min(changedRegion.startLine, 3); i++) { |
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.
nit: use variable naming as index
instead of i
.
|
||
// Pre-add lines that are before the scan region (context) | ||
for (let i = 0; i < Math.min(changedRegion.startLine, 3); i++) { | ||
if (leftLines[i]) { |
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.
nit:
const contextLines = Math.min(changedRegion.startLine, 3);
for (let lineIndex = 0; lineIndex < contextLines; lineIndex++) {
const sides = [
{ side: 'left', lines: leftLines },
{ side: 'right', lines: rightLines }
];
for (const { side, lines } of sides) {
if (lines[lineIndex]) {
await this.webviewManager.sendMessageToWebview(filePath, {
command: 'addLine',
side,
line: lines[lineIndex],
immediately: true
});
}
}
}
/** | ||
* Cancel ongoing animation | ||
*/ | ||
private cancelAnimation(filePath: string): void { |
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.
Its better to have a early return!
private cancelAnimation(filePath: string): void {
const animation = this.activeAnimations.get(filePath);
// Early return if no animation or if showing static diff
if (!animation || animation.isShowingStaticDiff) {
return;
}
// Mark animation as cancelled
animation.animationCancelled = true;
// Clear all timeouts and cleanup
this.clearAnimationTimeouts(filePath);
}
// Separate method for timeout clearing logic
private clearAnimationTimeouts(filePath: string): void {
const timeouts = this.animationTimeouts.get(filePath);
if (!timeouts) {
return;
}
timeouts.forEach(clearTimeout);
this.animationTimeouts.delete(filePath);
}
*/ | ||
|
||
/** | ||
* DiffAnimationHandler - Cline-style Diff View Approach |
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.
Can we remove this comment mentioning our competitor ?
import { PendingFileWrite } from './types' | ||
|
||
export class DiffAnimationHandler implements vscode.Disposable { | ||
/** |
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.
nit: This comment can be at L31.
private pendingWrites = new Map<string, PendingFileWrite>() | ||
|
||
constructor() { | ||
getLogger().info(`[DiffAnimationHandler] 🚀 Initializing DiffAnimationHandler with Cline-style diff view`) |
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.
Same comment
*/ | ||
public async testAnimation(): Promise<void> { | ||
const originalContent = `function hello() { | ||
console.log("Hello World"); |
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.
Please don't use console.log statements instead use Logger.
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.
+1
} | ||
|
||
/** | ||
* Test method to manually trigger animation (for debugging) |
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.
Looks like this is not required for prod version. If yes please remove this code!
}` | ||
|
||
const newContent = `function hello(name) { | ||
console.log(\`Hello \${name}!\`); |
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.
Same comment
) | ||
getLogger().info(`[DiffAnimationHandler] 🧪 Running test animation for: ${testFilePath}`) | ||
|
||
// Run the animation using Cline-style diff view |
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.
same as above comment
): Promise<void> { | ||
const animationId = `${path.basename(filePath)}_${Date.now()}` | ||
|
||
getLogger().info(`[DiffAnimationHandler] 🎬 Starting Cline-style diff animation ${animationId}`) |
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.
Same here.
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.
Please address comments!
└── VSCodeIntegration (VS Code APIs) | ||
``` | ||
|
||
## Key Benefits |
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.
I like the section but does not add values for future users, testers.
Problem
Amazon Q chat currently only supports static diff views that are shown after code generation is complete. Users cannot see the real-time changes being made to their files, which creates a disconnect between the AI's explanation and the actual code modifications. This lack of visual feedback makes it difficult to:
Solution
Implemented a comprehensive Diff Animation System that provides real-time, Cline-style animated diff views during code generation. This new feature enhances user experience by showing live code changes with visual animations, making the AI coding process more transparent and engaging.
High-Level Diff Animation Workflow
Chat Message Detection → 2. File Change Monitoring → 3. Animation Triggering → 4. Live Diff Rendering → 5. Interactive Review
Detailed Flow:
Architecture & Component Structure
Main Components
DiffAnimationHandler (Main Entry Point - 300 lines)
DiffAnimationController (Animation Engine - 400 lines)
DiffAnimationHandler Supporting Components
FileSystemManager (File Operations - ~200 lines)
ChatProcessor (Message Processing - ~150 lines)
AnimationQueueManager (Queue Management - ~180 lines)
DiffAnimationController Sub-Components
WebviewManager (UI Management - ~250 lines)
DiffAnalyzer (Diff Logic - ~220 lines)
VSCodeIntegration (VS Code APIs - ~180 lines)
Data Flow
New Functionalities
Animation Features
Integration Features
User Experience Enhancements
Unit Test Design & Coverage
Comprehensive Test Strategy
Implemented 95%+ code coverage across all new components with systematic edge case testing:
DiffAnimationHandler Tests (15 test categories, 45+ test cases)
DiffAnimationController Tests (12 test categories, 40+ test cases)
Edge Cases Covered
Input Validation & Robustness
Performance & Scalability
Platform Compatibility
Mock Strategy & Test Infrastructure
Benefits of This New Feature
Enhanced User Experience
Improved Development Workflow
Technical Advantages
Screenshots
-Create a new file
Create_New_Files.mov
-Edit a single file
Edit_Single_FIle.mov
-Edit multiple files
Edit_Multiple_Files.mov
feature/x
branches will not be squash-merged at release time.