chat: Use ref-counted model collection for tool confirmation inputs (#298829)

* chat: Use ref-counted model collection for tool confirmation inputs

Fixes duplicate model error when ToolConfirmationSubPart re-renders with
the same toolCallId. Models keyed by URI are now managed via a ref-counted
ReferenceCollection (InlineTextModelCollection) on IChatContentPartRenderContext,
so multiple SubPart instances can safely share the same model. The model is only
disposed when the last reference is released, preventing the 'Cannot add model
because it already exists!' error during list re-rendering or parallel tool calls.

- Add InlineTextModelCollection class wrapping ReferenceCollection<ITextModel>
  to handle ref-counted model lifecycle keyed by URI
- Add inlineTextModels field to IChatContentPartRenderContext
- Wire InlineTextModelCollection into chatListRenderer, passing to all context
  sites
- Update chatToolConfirmationSubPart to use context.inlineTextModels.acquire()
  instead of modelService.createModel()
- Remove unused IModelService injection from chatToolConfirmationSubPart

Fixes https://github.com/microsoft/vscode/issues/294345

(Commit message generated by Copilot)

* ci failure
This commit is contained in:
Connor Peet
2026-03-02 15:53:01 -08:00
committed by GitHub
parent 2a1295cbda
commit 122e527a81
7 changed files with 56 additions and 12 deletions

View File

@@ -3,13 +3,17 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { IDisposable } from '../../../../../../base/common/lifecycle.js';
import { Disposable, IDisposable, ReferenceCollection } from '../../../../../../base/common/lifecycle.js';
import { ChatTreeItem, IChatCodeBlockInfo } from '../../chat.js';
import { IChatRendererContent, IChatRequestViewModel, IChatResponseViewModel } from '../../../common/model/chatViewModel.js';
import { CodeBlockModelCollection } from '../../../common/widget/codeBlockModelCollection.js';
import { DiffEditorPool, EditorPool } from './chatContentCodePools.js';
import { IObservable } from '../../../../../../base/common/observable.js';
import { Event } from '../../../../../../base/common/event.js';
import { ITextModel } from '../../../../../../editor/common/model.js';
import { ILanguageSelection } from '../../../../../../editor/common/languages/language.js';
import { IModelService } from '../../../../../../editor/common/services/model.js';
import { URI } from '../../../../../../base/common/uri.js';
export interface IChatContentPart extends IDisposable {
domNode: HTMLElement | undefined;
@@ -53,4 +57,37 @@ export interface IChatContentPartRenderContext {
readonly codeBlockModelCollection: CodeBlockModelCollection;
readonly currentWidth: IObservable<number>;
readonly onDidChangeVisibility: Event<boolean>;
readonly inlineTextModels: InlineTextModelCollection;
}
/**
* Ref-counted collection of inline text models keyed by URI. Models are
* created on first acquire and disposed only when the last reference is
* released, preventing duplicate-model errors during re-renders.
*/
export class InlineTextModelCollection extends Disposable {
private readonly _collection: InlineTextModelReferenceCollection;
constructor(@IModelService modelService: IModelService) {
super();
this._collection = new InlineTextModelReferenceCollection(modelService);
}
acquire(uri: URI, value: string, languageSelection: ILanguageSelection | null, isForSimpleWidget: boolean) {
return this._collection.acquire(uri.toString(), uri, value, languageSelection, isForSimpleWidget);
}
}
class InlineTextModelReferenceCollection extends ReferenceCollection<ITextModel> {
constructor(private readonly modelService: IModelService) {
super();
}
protected override createReferencedObject(key: string, uri: URI, value: string, languageSelection: ILanguageSelection | null, isForSimpleWidget: boolean): ITextModel {
return this.modelService.createModel(value, languageSelection, uri, isForSimpleWidget);
}
protected override destroyReferencedObject(_key: string, model: ITextModel): void {
model.dispose();
}
}

View File

@@ -13,7 +13,6 @@ import { isEmptyObject } from '../../../../../../../base/common/types.js';
import { generateUuid } from '../../../../../../../base/common/uuid.js';
import { ElementSizeObserver } from '../../../../../../../editor/browser/config/elementSizeObserver.js';
import { ILanguageService } from '../../../../../../../editor/common/languages/language.js';
import { IModelService } from '../../../../../../../editor/common/services/model.js';
import { localize } from '../../../../../../../nls.js';
import { ICommandService } from '../../../../../../../platform/commands/common/commands.js';
import { IContextKeyService } from '../../../../../../../platform/contextkey/common/contextkey.js';
@@ -53,7 +52,6 @@ export class ToolConfirmationSubPart extends AbstractToolConfirmationSubPart {
private readonly codeBlockStartIndex: number,
@IInstantiationService instantiationService: IInstantiationService,
@IKeybindingService keybindingService: IKeybindingService,
@IModelService private readonly modelService: IModelService,
@ILanguageService private readonly languageService: ILanguageService,
@IContextKeyService contextKeyService: IContextKeyService,
@IChatWidgetService chatWidgetService: IChatWidgetService,
@@ -185,13 +183,14 @@ export class ToolConfirmationSubPart extends AbstractToolConfirmationSubPart {
const langId = this.languageService.getLanguageIdByLanguageName('json');
const rawJsonInput = JSON.stringify(inputData.rawInput ?? {}, null, 1);
const canSeeMore = count(rawJsonInput, '\n') > 2; // if more than one key:value
const model = this._register(this.modelService.createModel(
const modelRef = this._register(this.context.inlineTextModels.acquire(
createToolInputUri(toolInvocation.toolCallId),
// View a single JSON line by default until they 'see more'
rawJsonInput.replace(/\n */g, ' '),
this.languageService.createById(langId),
createToolInputUri(toolInvocation.toolCallId),
true
));
const model = modelRef.object;
const markerOwner = generateUuid();
const schemaUri = createToolSchemaUri(toolInvocation.toolId);

View File

@@ -78,7 +78,7 @@ import { ChatCodeCitationContentPart } from './chatContentParts/chatCodeCitation
import { ChatCommandButtonContentPart } from './chatContentParts/chatCommandContentPart.js';
import { ChatConfirmationContentPart } from './chatContentParts/chatConfirmationContentPart.js';
import { DiffEditorPool, EditorPool } from './chatContentParts/chatContentCodePools.js';
import { IChatContentPart, IChatContentPartRenderContext } from './chatContentParts/chatContentParts.js';
import { IChatContentPart, IChatContentPartRenderContext, InlineTextModelCollection } from './chatContentParts/chatContentParts.js';
import { ChatElicitationContentPart } from './chatContentParts/chatElicitationContentPart.js';
import { ChatErrorConfirmationContentPart } from './chatContentParts/chatErrorConfirmationPart.js';
import { ChatErrorContentPart } from './chatContentParts/chatErrorContentPart.js';
@@ -243,6 +243,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
* TODO@roblourens shouldn't use the CodeBlockModelCollection at all
*/
private readonly _toolInvocationCodeBlockCollection: CodeBlockModelCollection;
private readonly _inlineTextModels: InlineTextModelCollection;
/**
* Prevents re-announcement of already rendered chat progress
@@ -282,6 +283,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
this._contentReferencesListPool = this._register(this.instantiationService.createInstance(CollapsibleListPool, this._onDidChangeVisibility.event, undefined, undefined));
this._register(this.instantiationService.createInstance(ChatCodeBlockContentProvider));
this._inlineTextModels = this._register(this.instantiationService.createInstance(InlineTextModelCollection));
this._toolInvocationCodeBlockCollection = this._register(this.instantiationService.createInstance(CodeBlockModelCollection, 'tools'));
this._autoReply = this._register(this.instantiationService.createInstance(ChatQuestionCarouselAutoReply));
@@ -1139,6 +1141,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
codeBlockModelCollection: this.codeBlockModelCollection,
currentWidth: this._currentLayoutWidth,
onDidChangeVisibility: this._onDidChangeVisibility.event,
inlineTextModels: this._inlineTextModels,
get codeBlockStartIndex() {
return parts.reduce((acc, part) => acc + (part.codeblocks?.length ?? 0), 0);
},
@@ -1285,6 +1288,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer<Ch
codeBlockModelCollection: this.codeBlockModelCollection,
currentWidth: this._currentLayoutWidth,
onDidChangeVisibility: this._onDidChangeVisibility.event,
inlineTextModels: this._inlineTextModels,
get codeBlockStartIndex() {
return preceedingContentParts.reduce((acc, part) => acc + (part.codeblocks?.length ?? 0), 0);
},

View File

@@ -13,7 +13,7 @@ import { mainWindow } from '../../../../../../../base/browser/window.js';
import { workbenchInstantiationService } from '../../../../../../test/browser/workbenchTestServices.js';
import { ChatSubagentContentPart } from '../../../../browser/widget/chatContentParts/chatSubagentContentPart.js';
import { IChatMarkdownContent, IChatSubagentToolInvocationData, IChatToolInvocation, IChatToolInvocationSerialized, ToolConfirmKind } from '../../../../common/chatService/chatService.js';
import { IChatContentPartRenderContext } from '../../../../browser/widget/chatContentParts/chatContentParts.js';
import { IChatContentPartRenderContext, InlineTextModelCollection } from '../../../../browser/widget/chatContentParts/chatContentParts.js';
import { IChatResponseViewModel } from '../../../../common/model/chatViewModel.js';
import { IChatMarkdownAnchorService } from '../../../../browser/widget/chatContentParts/chatMarkdownAnchorService.js';
import { IMarkdownRenderer } from '../../../../../../../platform/markdown/browser/markdownRenderer.js';
@@ -55,6 +55,7 @@ suite('ChatSubagentContentPart', () => {
return {
element: mockElement as IChatResponseViewModel,
inlineTextModels: {} as InlineTextModelCollection,
elementIndex: 0,
container: mainWindow.document.createElement('div'),
content: [],

View File

@@ -15,7 +15,7 @@ import { IConfigurationService } from '../../../../../../../platform/configurati
import { TestConfigurationService } from '../../../../../../../platform/configuration/test/common/testConfigurationService.js';
import { ChatThinkingContentPart } from '../../../../browser/widget/chatContentParts/chatThinkingContentPart.js';
import { IChatMarkdownContent, IChatThinkingPart } from '../../../../common/chatService/chatService.js';
import { IChatContentPartRenderContext } from '../../../../browser/widget/chatContentParts/chatContentParts.js';
import { IChatContentPartRenderContext, InlineTextModelCollection } from '../../../../browser/widget/chatContentParts/chatContentParts.js';
import { IChatRendererContent, IChatResponseViewModel } from '../../../../common/model/chatViewModel.js';
import { IChatMarkdownAnchorService } from '../../../../browser/widget/chatContentParts/chatMarkdownAnchorService.js';
import { IMarkdownRenderer } from '../../../../../../../platform/markdown/browser/markdownRenderer.js';
@@ -49,6 +49,7 @@ suite('ChatThinkingContentPart', () => {
return {
element: mockElement as IChatResponseViewModel,
inlineTextModels: {} as InlineTextModelCollection,
elementIndex: 0,
container: mainWindow.document.createElement('div'),
content: [],

View File

@@ -9,11 +9,11 @@ import { Event } from '../../../../base/common/event.js';
import { observableValue } from '../../../../base/common/observable.js';
import { Codicon } from '../../../../base/common/codicons.js';
import { ThemeIcon } from '../../../../base/common/themables.js';
import { mock } from '../../../../base/test/common/mock.js';
import { mock, upcastPartial } from '../../../../base/test/common/mock.js';
import { IMarkdownRendererService, MarkdownRendererService } from '../../../../platform/markdown/browser/markdownRenderer.js';
import { ChatProgressContentPart } from '../../../contrib/chat/browser/widget/chatContentParts/chatProgressContentPart.js';
import { ChatContentMarkdownRenderer } from '../../../contrib/chat/browser/widget/chatContentMarkdownRenderer.js';
import { IChatContentPartRenderContext } from '../../../contrib/chat/browser/widget/chatContentParts/chatContentParts.js';
import { IChatContentPartRenderContext, InlineTextModelCollection } from '../../../contrib/chat/browser/widget/chatContentParts/chatContentParts.js';
import { IChatMarkdownAnchorService } from '../../../contrib/chat/browser/widget/chatContentParts/chatMarkdownAnchorService.js';
import { IChatProgressMessage } from '../../../contrib/chat/common/chatService/chatService.js';
import { IChatResponseViewModel } from '../../../contrib/chat/common/model/chatViewModel.js';
@@ -27,6 +27,7 @@ function createMockContext(opts?: { isComplete?: boolean; hasFollowingContent?:
}();
return {
element,
inlineTextModels: upcastPartial<InlineTextModelCollection>({}),
elementIndex: 0,
container: document.createElement('div'),
content: opts?.hasFollowingContent ? [{ kind: 'progressMessage', content: new MarkdownString('test') }] : [],

View File

@@ -7,9 +7,9 @@ import * as dom from '../../../../base/browser/dom.js';
import { IMarkdownRendererService, MarkdownRendererService } from '../../../../platform/markdown/browser/markdownRenderer.js';
import { IChatQuestion, IChatQuestionCarousel } from '../../../contrib/chat/common/chatService/chatService.js';
import { ChatQuestionCarouselPart, IChatQuestionCarouselOptions } from '../../../contrib/chat/browser/widget/chatContentParts/chatQuestionCarouselPart.js';
import { IChatContentPartRenderContext } from '../../../contrib/chat/browser/widget/chatContentParts/chatContentParts.js';
import { IChatContentPartRenderContext, InlineTextModelCollection } from '../../../contrib/chat/browser/widget/chatContentParts/chatContentParts.js';
import { ComponentFixtureContext, createEditorServices, defineComponentFixture, defineThemedFixtureGroup } from './fixtureUtils.js';
import { mock } from '../../../../base/test/common/mock.js';
import { mock, upcastPartial } from '../../../../base/test/common/mock.js';
import { Event } from '../../../../base/common/event.js';
import { observableValue } from '../../../../base/common/observable.js';
import { IChatRequestViewModel } from '../../../contrib/chat/common/model/chatViewModel.js';
@@ -26,6 +26,7 @@ function createCarousel(questions: IChatQuestion[], allowSkip: boolean = true):
function createMockContext(): IChatContentPartRenderContext {
return {
element: new class extends mock<IChatRequestViewModel>() { }(),
inlineTextModels: upcastPartial<InlineTextModelCollection>({}),
elementIndex: 0,
container: document.createElement('div'),
content: [],