-
Notifications
You must be signed in to change notification settings - Fork 731
feat(amazonq):streaming diff animation #7662
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
feat(amazonq):streaming diff animation #7662
Conversation
- Add messages.ts with streaming diff animation support - Add complete diffAnimation module with: - Animation queue management - Chat processing capabilities - Diff analysis and animation control - File system management - VSCode integration - Webview management - Streaming diff controller
|
|
- Eliminated duplicate temp file creation and diff view opening logic in StreamingDiffController - Consolidated session creation logic in DiffAnimationHandler - Created reusable cleanup helper function in Messages - Removed duplicate getCursorState function - Reduced codebase by 53+ lines of duplicate code
- Created comprehensive test suite for StreamingDiffController, DiffAnimationHandler, and AnimationQueueManager - Tests cover initialization, session management, content streaming, and queue operations - Uses Mocha/Sinon testing framework consistent with existing codebase - Includes sanity tests to improve code coverage for Codecov requirements
- Update diffAnimationHandler for better streaming control - Enhance streamingDiffController with improved state management - Refactor chat messages handling for diff animations
467e84b to
d906edf
Compare
| * Clean up all temporary files for a chat session | ||
| */ | ||
| async cleanupChatSession(): Promise<void> { | ||
| const tempFilesToCleanup: string[] = [] |
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 tempFilePaths: string[] = [];
// Collect from active streaming sessions
this.activeStreamingSessions.forEach(session => {
if (session.tempFilePath) {
tempFilePaths.push(session.tempFilePath);
}
});
// Collect from fs replace sessions
this.fsReplaceSessionsByFile.forEach(session => {
if (session.tempFilePath) {
tempFilePaths.push(session.tempFilePath);
}
});
|
|
||
| languageClient.onNotification(chatUpdateNotificationType.method, (params: ChatUpdateParams) => { | ||
| languageClient.onNotification(chatUpdateNotificationType.method, async (params: ChatUpdateParams) => { | ||
| if ((params.data as any)?.streamingChunk) { |
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.
Move this to separate function or file to make this more readable?
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 have moved the streaming chunk processing logic to a seperate function!
| streamingChunk.isComplete || false | ||
| ) | ||
|
|
||
| if (streamingChunk.isComplete) { |
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 avoid nested if statements?
Something like this?
if (!streamingChunk.isComplete || !streamingChunk.filePath) {
return;
}
const toolUseIds = initializingStreamsByFile.get(streamingChunk.filePath);
if (!toolUseIds) {
return;
}
toolUseIds.delete(streamingChunk.toolUseId);
if (toolUseIds.size === 0) {
initializingStreamsByFile.delete(streamingChunk.filePath);
}
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.
Applies to above code too
96ced1d to
120aba1
Compare
| * Scroll editor to line | ||
| */ | ||
| private scrollEditorToLine(editor: vscode.TextEditor, line: number): void { | ||
| const scrollLine = line |
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.
We dont need this?
| session.activeLineController.clear() | ||
|
|
||
| // Save the temp file one final time | ||
| const diffEditor = session.activeDiffEditor |
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 is not required var?
| private async cleanupSessions(toolUseIds: Set<string>): Promise<void> { | ||
| for (const toolUseId of toolUseIds) { | ||
| const sessionToCleanup = this.activeStreamingSessions.get(toolUseId) | ||
| if (sessionToCleanup) { |
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.
but:
Can add continue here!
…t-vscode into feature/StreamingDiffAnimation
…arcoWang3/aws-toolkit-vscode into feature/StreamingDiffAnimation
|
|
||
| const processedChunks = new Map<string, Set<string>>() // toolUseId -> Set of content hashes | ||
|
|
||
| // Initialize DiffAnimationHandler |
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.
Why do we need this ?
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.
Sometimes there are duplicated chunks are received and this is used for avoiding process them for multiple times
| /** | ||
| * Handle streaming chunk processing for diff animations | ||
| */ | ||
| async function handleStreamingChunk( |
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 move this function to another class?
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.
done!
Problem
The previous diff animation system is totally based on the VSC side to detect the file change events and render the diffAnimation. So all the diff animations can be only rendered after the codes are generated. Previous version uses customized webview for rendering the diff animation.
Solution
Update the mechanism of the current diff animation system: receive stream chunks from the language servers side and render partial diff animation based on them for fsWrite events. Receive a complete chunk for fsReplace and render diff animation line by line. Use VSC's built in diffview for rendering the diff animations.
Demo
create.a.file.mov
edit.a.file.mov