Avoid unnecessary work when putting request into edit mode

We fire onDidChange to trigger a rerender to apply a css class. When onDidChange happens and the response is complete, then we update all the codeblock textmodels (?). Use an observable to avoid all this. #286660

Author: roblourens

src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts

@@ -93,7 +93,7 @@ import { ChatToolInvocationPart } from './chatContentParts/toolInvocationParts/c
 import { ChatMarkdownDecorationsRenderer } from './chatContentParts/chatMarkdownDecorationsRenderer.js';
 import { ChatEditorOptions } from './chatOptions.js';
 import { ChatCodeBlockContentProvider, CodeBlockPart } from './chatContentParts/codeBlockPart.js';
-import { observableValue } from '../../../../../base/common/observable.js';
+import { autorun, observableValue } from '../../../../../base/common/observable.js';
 
 const $ = dom.$;
 
@@ -610,7 +610,10 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
 		const editing = element.id === this.viewModel?.editing?.id;
 		const isInput = this.configService.getValue<string>('chat.editRequests') === 'input';
 
-		templateData.disabledOverlay.classList.toggle('disabled', element.shouldBeBlocked && !editing && this.viewModel?.editing !== undefined);
+		templateData.elementDisposables.add(autorun(r => {
+			const shouldBeBlocked = element.shouldBeBlocked.read(r);
+			templateData.disabledOverlay.classList.toggle('disabled', shouldBeBlocked && !editing && this.viewModel?.editing !== undefined);
+		}));
 		templateData.rowContainer.classList.toggle('editing', editing && !isInput);
 		templateData.rowContainer.classList.toggle('editing-input', editing && isInput);
 		templateData.requestHover.classList.toggle('editing', editing && isInput);

src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts

@@ -838,8 +838,6 @@ export class ChatWidget extends Disposable implements IChatWidget {
 							(isResponseVM(element) ? `_${element.contentReferences.length}` : '') +
 							// Re-render if element becomes hidden due to undo/redo
 							`_${element.shouldBeRemovedOnSend ? `${element.shouldBeRemovedOnSend.afterUndoStop || '1'}` : '0'}` +
-							// Re-render if element becomes enabled/disabled due to checkpointing
-							`_${element.shouldBeBlocked ? '1' : '0'}` +
 							// Re-render if we have an element currently being edited
 							`_${this.viewModel?.editing ? '1' : '0'}` +
 							// Re-render if we have an element currently being checkpointed
@@ -1620,7 +1618,7 @@ export class ChatWidget extends Disposable implements IChatWidget {
 			for (let i = requests.length - 1; i >= 0; i -= 1) {
 				const request = requests[i];
 				if (request.id === currentElement.id) {
-					request.shouldBeBlocked = false; // unblocking just this request.
+					request.setShouldBeBlocked(false); // unblocking just this request.
 					request.attachedContext?.forEach(addToContext);
 					currentElement.variables.forEach(addToContext);
 				}

src/vs/workbench/contrib/chat/common/model/chatModel.ts

@@ -77,7 +77,8 @@ export interface IChatRequestModel {
 	readonly response?: IChatResponseModel;
 	readonly editedFileEvents?: IChatAgentEditedFileEvent[];
 	shouldBeRemovedOnSend: IChatRequestDisablement | undefined;
-	shouldBeBlocked: boolean;
+	readonly shouldBeBlocked: IObservable<boolean>;
+	setShouldBeBlocked(value: boolean): void;
 	readonly modelId?: string;
 	readonly userSelectedTools?: UserSelectedTools;
 }
@@ -215,7 +216,7 @@ export interface IChatResponseModel {
 	readonly isPendingConfirmation: IObservable<{ startedWaitingAt: number; detail?: string } | undefined>;
 	readonly isInProgress: IObservable<boolean>;
 	readonly shouldBeRemovedOnSend: IChatRequestDisablement | undefined;
-	shouldBeBlocked: boolean;
+	readonly shouldBeBlocked: IObservable<boolean>;
 	readonly isCompleteAddedRequest: boolean;
 	/** A stale response is one that has been persisted and rehydrated, so e.g. Commands that have their arguments stored in the EH are gone. */
 	readonly isStale: boolean;
@@ -290,7 +291,14 @@ export class ChatRequestModel implements IChatRequestModel {
 	public readonly modeInfo?: IChatRequestModeInfo;
 	public readonly userSelectedTools?: UserSelectedTools;
 
-	public shouldBeBlocked: boolean = false;
+	private readonly _shouldBeBlocked = observableValue<boolean>(this, false);
+	public get shouldBeBlocked(): IObservable<boolean> {
+		return this._shouldBeBlocked;
+	}
+
+	public setShouldBeBlocked(value: boolean): void {
+		this._shouldBeBlocked.set(value, undefined);
+	}
 
 	private _session: ChatModel;
 	private readonly _attempt: number;
@@ -811,13 +819,13 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel
 	private _result?: IChatAgentResult;
 	private _shouldBeRemovedOnSend: IChatRequestDisablement | undefined;
 	public readonly isCompleteAddedRequest: boolean;
-	private _shouldBeBlocked: boolean = false;
+	private readonly _shouldBeBlocked = observableValue<boolean>(this, false);
 	private readonly _timestamp: number;
 	private _timeSpentWaitingAccumulator: number;
 
 	public confirmationAdjustedTimestamp: IObservable<number>;
 
-	public get shouldBeBlocked() {
+	public get shouldBeBlocked(): IObservable<boolean> {
 		return this._shouldBeBlocked;
 	}
 
@@ -980,7 +988,7 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel
 		this._followups = params.followups ? [...params.followups] : undefined;
 		this.isCompleteAddedRequest = params.isCompleteAddedRequest ?? false;
 		this._shouldBeRemovedOnSend = params.shouldBeRemovedOnSend;
-		this._shouldBeBlocked = params.shouldBeBlocked ?? false;
+		this._shouldBeBlocked.set(params.shouldBeBlocked ?? false, undefined);
 
 		// If we are creating a response with some existing content, consider it stale
 		this._isStale = Array.isArray(params.responseContent) && (params.responseContent.length !== 0 || isMarkdownString(params.responseContent) && params.responseContent.value.length !== 0);
@@ -1028,11 +1036,7 @@ export class ChatResponseModel extends Disposable implements IChatResponseModel
 		this._register(this._session.onDidChange((e) => {
 			if (e.kind === 'setCheckpoint') {
 				const isDisabled = e.disabledResponseIds.has(this.id);
-				const didChange = this._shouldBeBlocked === isDisabled;
-				this._shouldBeBlocked = isDisabled;
-				if (didChange) {
-					this._onDidChange.fire(defaultChatResponseModelChangeReason);
-				}
+				this._shouldBeBlocked.set(isDisabled, undefined);
 			}
 		}));
 
@@ -1948,7 +1952,7 @@ export class ChatModel extends Disposable implements IChatModel {
 						followups: raw.followups,
 						restoredId: raw.responseId,
 						timeSpentWaiting: raw.timeSpentWaiting,
-						shouldBeBlocked: request.shouldBeBlocked,
+						shouldBeBlocked: request.shouldBeBlocked.get(),
 						codeBlockInfos: raw.responseMarkdownInfo?.map<ICodeBlockInfo>(info => ({ suggestionId: info.suggestionId })),
 					});
 					request.response.shouldBeRemovedOnSend = raw.isHidden ? { requestId: raw.requestId } : raw.shouldBeRemovedOnSend;
@@ -1994,7 +1998,7 @@ export class ChatModel extends Disposable implements IChatModel {
 
 	resetCheckpoint(): void {
 		for (const request of this._requests) {
-			request.shouldBeBlocked = false;
+			request.setShouldBeBlocked(false);
 		}
 	}
 
@@ -2006,7 +2010,7 @@ export class ChatModel extends Disposable implements IChatModel {
 				if (request.id === requestId) {
 					checkpointIndex = index;
 					checkpoint = request;
-					request.shouldBeBlocked = true;
+					request.setShouldBeBlocked(true);
 				}
 			});
 
@@ -2020,15 +2024,15 @@ export class ChatModel extends Disposable implements IChatModel {
 		for (let i = this._requests.length - 1; i >= 0; i -= 1) {
 			const request = this._requests[i];
 			if (this._checkpoint && !checkpoint) {
-				request.shouldBeBlocked = false;
+				request.setShouldBeBlocked(false);
 			} else if (checkpoint && i >= checkpointIndex) {
-				request.shouldBeBlocked = true;
+				request.setShouldBeBlocked(true);
 				disabledRequestIds.add(request.id);
 				if (request.response) {
 					disabledResponseIds.add(request.response.id);
 				}
 			} else if (checkpoint && i < checkpointIndex) {
-				request.shouldBeBlocked = false;
+				request.setShouldBeBlocked(false);
 			}
 		}
 

src/vs/workbench/contrib/chat/common/model/chatViewModel.ts

@@ -9,6 +9,7 @@ import { hash } from '../../../../../base/common/hash.js';
 import { IMarkdownString } from '../../../../../base/common/htmlContent.js';
 import { Disposable, dispose } from '../../../../../base/common/lifecycle.js';
 import * as marked from '../../../../../base/common/marked/marked.js';
+import { IObservable } from '../../../../../base/common/observable.js';
 import { ThemeIcon } from '../../../../../base/common/themables.js';
 import { URI } from '../../../../../base/common/uri.js';
 import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js';
@@ -96,7 +97,7 @@ export interface IChatRequestViewModel {
 	readonly isCompleteAddedRequest: boolean;
 	readonly slashCommand: IChatAgentCommand | undefined;
 	readonly agentOrSlashCommandDetected: boolean;
-	readonly shouldBeBlocked?: boolean;
+	readonly shouldBeBlocked: IObservable<boolean>;
 	readonly modelId?: string;
 }
 
@@ -224,7 +225,7 @@ export interface IChatResponseViewModel {
 	usedReferencesExpanded?: boolean;
 	vulnerabilitiesListExpanded: boolean;
 	setEditApplied(edit: IChatTextEditGroup, editCount: number): void;
-	readonly shouldBeBlocked: boolean;
+	readonly shouldBeBlocked: IObservable<boolean>;
 }
 
 export class ChatViewModel extends Disposable implements IChatViewModel {