perf: Fix CKS bug and don't store full instructions contents on the ChatModel (#308920)

* perf: Fix CKS bug and don't store full instructions contents on the ChatModel

* Add test
This commit is contained in:
Rob Lourens
2026-04-10 01:51:54 +00:00
committed by GitHub
parent 7ff79304b1
commit 6df6bbde2a
3 changed files with 37 additions and 5 deletions
@@ -529,9 +529,9 @@ class ScopedContextKeyService extends AbstractContextKeyService {
}
public disposeContext(contextId: number): void {
if (this._isDisposed) {
return;
}
// Always forward to parent even after disposal — a child context may
// be disposed after us and must still reach the root ContextKeyService
// to delete its entry from _contexts.
this._parent.disposeContext(contextId);
}
@@ -298,4 +298,27 @@ suite('ContextKeyService', () => {
assert.strictEqual(eventFired, true, 'Should fire event when setting different number');
});
test('disposeContext forwards through disposed scoped service', () => {
const root = testDisposables.add(new ContextKeyService(new TestConfigurationService()));
const scoped = root.createScoped(document.createElement('div'));
const child = scoped.createScoped(document.createElement('div'));
// Set a value on the child so we can observe it
child.createKey('testKey', 'value');
assert.strictEqual(child.getContextKeyValue('testKey'), 'value');
// Dispose the intermediate scoped service first
scoped.dispose();
// Now dispose the child — this should still forward to root
// and clean up the child's context entry. Before the fix,
// ScopedContextKeyService.disposeContext bailed out when
// _isDisposed was true, leaking Context objects.
child.dispose();
// The child's context should no longer be accessible through root
assert.strictEqual(root.getContextKeyValue('testKey'), undefined,
'Child context should be cleaned up even when parent scoped service was disposed first');
});
});
@@ -48,7 +48,7 @@ import { ChatSessionStore, IChatSessionEntryMetadata } from '../model/chatSessio
import { IChatSlashCommandService } from '../participants/chatSlashCommands.js';
import { IChatTransferService } from '../model/chatTransferService.js';
import { chatSessionResourceToId, getChatSessionType, isUntitledChatSession, LocalChatSessionUri } from '../model/chatUri.js';
import { ChatRequestVariableSet, IChatRequestVariableEntry } from '../attachments/chatVariableEntries.js';
import { ChatRequestVariableSet, IChatRequestVariableEntry, isPromptTextVariableEntry } from '../attachments/chatVariableEntries.js';
import { ChatAgentLocation, ChatModeKind } from '../constants.js';
import { ChatMessageRole, IChatMessage, ILanguageModelsService } from '../languageModels.js';
import { ILanguageModelToolsService } from '../tools/languageModelToolsService.js';
@@ -1171,8 +1171,17 @@ export class ChatService extends Disposable implements IChatService {
if (instructionEntries.length > 0) {
allContext.push(...instructionEntries);
}
// Store only non-instruction variables on the model.
// Automatically-added promptText entries (~33 KB each) are
// ephemeral — re-collected every turn, never rendered in
// the UI, and not needed in serialized session history.
const storedVariables = allContext.filter(v => !(isPromptTextVariableEntry(v) && v.automaticallyAdded));
model.updateRequest(request, { variables: storedVariables });
// The full set (including instructions) is passed to the
// agent request only — not stored on the request model.
let variableData: IChatRequestVariableData = { variables: allContext };
model.updateRequest(request, variableData);
// Merge resolved variables (e.g. images from directories) for the
// agent request only - they are not stored on the request model.