Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions src/vs/workbench/contrib/chat/common/model/chatSessionStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ export class ChatSessionStore extends Disposable {

// Write succeeded, update index
const newMetadata = await getSessionMetadata(session);
index.entries[session.sessionId] = newMetadata;
index.entries[session.sessionId] = preserveNonEmpty(index.entries[session.sessionId], newMetadata);
} catch (e) {
this.reportError('sessionWrite', 'Error writing chat session', e);
}
Expand All @@ -413,7 +413,7 @@ export class ChatSessionStore extends Disposable {

// TODO get this class on sessionResource
const externalSessionId = session.sessionResource.toString();
index.entries[externalSessionId] = await getSessionMetadata(session);
index.entries[externalSessionId] = preserveNonEmpty(index.entries[externalSessionId], await getSessionMetadata(session));
} catch (e) {
this.reportError('sessionMetadataWrite', 'Error writing chat session metadata', e);
}
Expand Down Expand Up @@ -759,11 +759,11 @@ export class ChatSessionStore extends Disposable {
updateAndFlushIndexSync(localSessions: ChatModel[], externalSessions: ChatModel[]): void {
const index = this.internalGetIndex();
for (const session of localSessions) {
index.entries[session.sessionId] = getSessionMetadataSync(session);
index.entries[session.sessionId] = preserveNonEmpty(index.entries[session.sessionId], getSessionMetadataSync(session));
Comment thread
abmahdy marked this conversation as resolved.
}
for (const session of externalSessions) {
const externalSessionId = session.sessionResource.toString();
index.entries[externalSessionId] = getSessionMetadataSync(session);
index.entries[externalSessionId] = preserveNonEmpty(index.entries[externalSessionId], getSessionMetadataSync(session));
}
try {
this.storageService.store(ChatIndexStorageKey, index, this.getIndexStorageScope(), StorageTarget.MACHINE);
Expand Down Expand Up @@ -893,6 +893,23 @@ function getSessionMetadataSync(session: ChatModel): IChatSessionEntryMetadata {
};
}

/**
* `isEmpty` exists to keep never-used sessions out of the history list, which
* filters on it. Removing every request from a session that *did* have them —
* restoring the checkpoint on the first request, for example — must therefore
* not re-classify it as never-used: doing so drops it from the list permanently
* even though its title and full transcript are still on disk, and makes it
* flicker in and out beforehand, since the live-model path applies no such
* filter.
*/
Comment thread
abmahdy marked this conversation as resolved.
function preserveNonEmpty(previous: IChatSessionEntryMetadata | undefined, next: IChatSessionEntryMetadata): IChatSessionEntryMetadata {
if (next.isEmpty && previous && !previous.isEmpty) {
next.isEmpty = false;
}

return next;
}

async function getSessionMetadata(session: ChatModel | ISerializableChatData): Promise<IChatSessionEntryMetadata> {
if (session instanceof ChatModel) {
const metadata = getSessionMetadataSync(session);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { MockChatModel } from './mockChatModel.js';
import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js';
import { TestConfigurationService } from '../../../../../../platform/configuration/test/common/testConfigurationService.js';

function createMockChatModel(sessionResource: URI, options?: { customTitle?: string }): ChatModel {
function createMockChatModel(sessionResource: URI, options?: { customTitle?: string; requestCount?: number }): ChatModel {
const sessionId = LocalChatSessionUri.parseLocalSessionId(sessionResource);
if (!sessionId) {
throw new Error('createMockChatModel requires a local session URI');
Expand All @@ -39,10 +39,18 @@ function createMockChatModel(sessionResource: URI, options?: { customTitle?: str
if (options?.customTitle) {
model.customTitle = options.customTitle;
}
if (options?.requestCount) {
setRequestCount(model, options.requestCount);
}
// Cast to ChatModel - the mock implements enough of the interface for testing
return model as unknown as ChatModel;
}

/** Only the request count matters for the index metadata under test. */
function setRequestCount(model: MockChatModel, count: number): void {
model.requests = Array.from({ length: count }, () => ({} as MockChatModel['requests'][number]));
}

class MockWorkspaceEditingService extends Disposable implements Partial<IWorkspaceEditingService> {
private readonly _onDidEnterWorkspace = this._register(new Emitter<IDidEnterWorkspaceEvent>());
readonly onDidEnterWorkspace = this._onDidEnterWorkspace.event;
Expand Down Expand Up @@ -183,6 +191,32 @@ suite('ChatSessionStore', () => {
assert.strictEqual(index['session-1'].title, 'My Custom Title');
});

test('storeSessions marks a session with no requests as empty', async () => {
const store = createChatSessionStore();
const model = testDisposables.add(createMockChatModel(LocalChatSessionUri.forSession('session-1')));

await store.storeSessions([model]);

const index = await store.getIndex();
assert.strictEqual(index['session-1'].isEmpty, true);
});

test('storeSessions keeps a session non-empty after all of its requests are removed', async () => {
const store = createChatSessionStore();
const model = testDisposables.add(createMockChatModel(LocalChatSessionUri.forSession('session-1'), { requestCount: 2 }));

await store.storeSessions([model]);
assert.strictEqual((await store.getIndex())['session-1'].isEmpty, false);

// Restoring the checkpoint on the first request removes every request.
// The session still has its title and transcript on disk, so it must not
// be reclassified as never-used and filtered out of the history list.
Comment thread
abmahdy marked this conversation as resolved.
Outdated
setRequestCount(model as unknown as MockChatModel, 0);
await store.storeSessions([model]);

assert.strictEqual((await store.getIndex())['session-1'].isEmpty, false);
});

test('readSession returns stored session data', async () => {
const store = createChatSessionStore();
const model = testDisposables.add(createMockChatModel(LocalChatSessionUri.forSession('session-1')));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export class MockChatModel extends Disposable implements IChatModel {
readonly timestamp = 0;
readonly timing: IChatSessionTiming = { created: Date.now(), lastRequestStarted: undefined, lastRequestEnded: undefined };
readonly initialLocation = ChatAgentLocation.Chat;
readonly sessionTypeSelectionReason = undefined;
readonly title = '';
readonly hasCustomTitle = false;
customTitle: string | undefined;
Expand Down Expand Up @@ -67,7 +68,7 @@ export class MockChatModel extends Disposable implements IChatModel {
}

startEditingSession(isGlobalEditingSession?: boolean, transferFromSession?: IChatEditingSession): void { }
getRequests(): IChatRequestModel[] { return []; }
getRequests(): IChatRequestModel[] { return this.requests; }
setCheckpoint(requestId: string | undefined): void { }
setRepoData(data: IExportableRepoData | undefined): void { this.repoData = data; }
workingDirectory: URI | undefined = undefined;
Expand Down
Loading