-
Notifications
You must be signed in to change notification settings - Fork 36.7k
Implement resetSession method in ChatListItemRenderer and invoke it in ChatWidget for session management. Fix #277574 #281505
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
…n ChatWidget for session management
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @roblourensMatched files:
|
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.
Pull request overview
This PR implements a resetSession() method in ChatListItemRenderer to fix rendering glitches when switching between chat sessions or reloading saved sessions (issue #277574). The root cause was that cached renderer state (template data, code blocks, file trees, etc.) from previous sessions was being reused when loading a new session, causing stale DOM elements and events to persist.
Key Changes
- Added
resetSession()method toChatListItemRendererthat clears all cached session-specific state - Invokes
resetSession()inChatWidgetat three strategic points: when clearing the chat, when setting no model, and when switching to a different session
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/vs/workbench/contrib/chat/browser/chatListRenderer.ts |
Implements resetSession() method that clears templateDataByRequestId map, code block caches, file tree caches, and resets element disposables for each cached template |
src/vs/workbench/contrib/chat/browser/chatWidget.ts |
Invokes renderer?.resetSession() in three locations: in clear() method, when setModel() is called with undefined, and when setModel() switches to a different session |
| this.codeBlocksByResponseId.clear(); | ||
| this.codeBlocksByEditorUri.clear(); | ||
| this.fileTreesByResponseId.clear(); | ||
| this.focusedFileTreesByResponseId.clear(); |
Copilot
AI
Dec 5, 2025
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.
The resetSession() method should clear _announcedToolProgressKeys for consistency with updateViewModel(). This set tracks which tool progress has been announced to screen readers. When a session is reset, this state should also be cleared to ensure proper accessibility announcements in the new session.
Suggested fix:
resetSession(): void {
for (const templateData of this.templateDataByRequestId.values()) {
this.clearRenderedParts(templateData);
templateData.elementDisposables.clear();
templateData.currentElement = undefined;
}
this.templateDataByRequestId.clear();
this.codeBlocksByResponseId.clear();
this.codeBlocksByEditorUri.clear();
this.fileTreesByResponseId.clear();
this.focusedFileTreesByResponseId.clear();
this._announcedToolProgressKeys.clear(); // Add this line
}| this.focusedFileTreesByResponseId.clear(); | |
| this.focusedFileTreesByResponseId.clear(); | |
| this._announcedToolProgressKeys.clear(); |
|
I can't repro the issue, and I don't see why this is necessary. Why isn't it enough to clear these parts the next time the template is reused? |
|
I asked gpt-5.1-codex-mini to make the issue reproducible, and it proposed the following change. It adds an artificial delay inside reproduce steps:
prompt gpt-5.1-codex-mini's explanation: The bug where code blocks disappear when reloading a saved chat session happens because When diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts
index e68d5b9fe87..23835258e33 100644
--- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts
+++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts
@@ -169,6 +169,7 @@ const supportsAllAttachments: Required<IChatAgentAttachmentCapabilities> = {
};
const DISCLAIMER = localize('chatDisclaimer', "AI responses may be inaccurate.");
+const SAVED_SESSION_BUG_REPRO_DELAY_MS = 64;
export class ChatWidget extends Disposable implements IChatWidget {
@@ -283,6 +284,9 @@ export class ChatWidget extends Disposable implements IChatWidget {
private _isLoadingPromptDescriptions = false;
private _mostRecentlyFocusedItemIndex: number = -1;
+ private _savedSessionDelayToken = 0;
+ private _scheduledSavedSessionDelayToken = 0;
+ private _scrollToEndAfterDelay = false;
private readonly viewModelDisposables = this._register(new DisposableStore());
private _viewModel: ChatViewModel | undefined;
@@ -777,6 +781,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
async clear(): Promise<void> {
this.logService.debug('ChatWidget#clear');
+ this._resetSavedSessionDelayState();
if (this._dynamicMessageLayoutData) {
this._dynamicMessageLayoutData.enabled = true;
}
@@ -800,6 +805,44 @@ export class ChatWidget extends Disposable implements IChatWidget {
}
private onDidChangeItems(skipDynamicLayout?: boolean) {
+ if (this._savedSessionDelayToken > 0) {
+ this._scheduleSavedSessionDelayedUpdate(skipDynamicLayout);
+ return;
+ }
+ this._performOnDidChangeItems(skipDynamicLayout);
+ }
+
+ private _scheduleSavedSessionDelayedUpdate(skipDynamicLayout?: boolean): void {
+ if (this._savedSessionDelayToken === 0 || this._scheduledSavedSessionDelayToken === this._savedSessionDelayToken) {
+ return;
+ }
+ const token = this._savedSessionDelayToken;
+ this._scheduledSavedSessionDelayToken = token;
+ void this._runSavedSessionDelayedUpdate(skipDynamicLayout, token);
+ }
+
+ private async _runSavedSessionDelayedUpdate(skipDynamicLayout: boolean | undefined, token: number): Promise<void> {
+ await this._sleepForSavedSessionRace();
+ if (token !== this._savedSessionDelayToken) {
+ this._scheduledSavedSessionDelayToken = 0;
+ return;
+ }
+ this._savedSessionDelayToken = 0;
+ this._scheduledSavedSessionDelayToken = 0;
+ this._performOnDidChangeItems(skipDynamicLayout);
+ if (this._scrollToEndAfterDelay) {
+ this._scrollToEndAfterDelay = false;
+ if (this.visible) {
+ this.scrollToEnd();
+ }
+ }
+ }
+
+ private async _sleepForSavedSessionRace(): Promise<void> {
+ await new Promise<void>(resolve => setTimeout(resolve, SAVED_SESSION_BUG_REPRO_DELAY_MS));
+ }
+
+ private _performOnDidChangeItems(skipDynamicLayout?: boolean): void {
if (this._visible || !this.viewModel) {
const treeItems = (this.viewModel?.getItems() ?? [])
.map((item): ITreeElement<ChatTreeItem> => {
@@ -857,6 +900,21 @@ export class ChatWidget extends Disposable implements IChatWidget {
}
}
+ private _resetSavedSessionDelayState(): void {
+ this._savedSessionDelayToken = 0;
+ this._scheduledSavedSessionDelayToken = 0;
+ this._scrollToEndAfterDelay = false;
+ }
+
+ private _prepareSavedSessionDelayState(): void {
+ this._savedSessionDelayToken = this._savedSessionDelayToken + 1;
+ if (this._savedSessionDelayToken === 0) {
+ this._savedSessionDelayToken = 1;
+ }
+ this._scheduledSavedSessionDelayToken = 0;
+ this._scrollToEndAfterDelay = true;
+ }
+
/**
* Updates the DOM visibility of welcome view and chat list immediately
*/
@@ -1967,6 +2025,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
}
if (!model) {
+ this._resetSavedSessionDelayState();
this.viewModel = undefined;
this.onDidChangeItems();
return;
@@ -1982,6 +2041,12 @@ export class ChatWidget extends Disposable implements IChatWidget {
this.container.setAttribute('data-session-id', model.sessionId);
this.viewModel = this.instantiationService.createInstance(ChatViewModel, model, this._codeBlockModelCollection);
+ const shouldDelaySavedSession = model.sessionResource.scheme === Schemas.vscodeLocalChatSession && model.getRequests().length > 0;
+ if (shouldDelaySavedSession) {
+ this._prepareSavedSessionDelayState();
+ } else {
+ this._resetSavedSessionDelayState();
+ }
// Pass input model reference to input part for state syncing
this.inputPart.setInputModel(model.inputModel);
@@ -2066,7 +2131,9 @@ export class ChatWidget extends Disposable implements IChatWidget {
if (this.tree && this.visible) {
this.onDidChangeItems();
- this.scrollToEnd();
+ if (!this._scrollToEndAfterDelay) {
+ this.scrollToEnd();
+ }
}
this.renderer.updateViewModel(this.viewModel); |
Implement resetSession method in ChatListItemRenderer and invoke it in ChatWidget for session management. Fix #277574.
gpt-5.1-codex-mini and I found another fix for #277574. I confirmed it is fixed.
A message from gpt-5.1-codex-mini:
ChatListItemRendererreused stale DOM/events (focusing, progressive rendering, etc.) tied to the previous session becausetemplateDataByRequestId, code block maps, hover artifacts, and in-progress rendered parts were never reset.resetSession()clears everytemplateData(rendered parts, disposables, current element) and wipes the cached maps before a new model is bound or the widget is cleared, ensuring ListView always renders against a clean slate instead of recycling the old nodes, which eliminates the leftover rendering glitches.cc: @connor4312