Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
224 changes: 224 additions & 0 deletions packages/core/src/core/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,138 @@ describe('Gemini Client (client.ts)', () => {
expect(newHistory.length).toBe(initialHistory.length);
expect(JSON.stringify(newHistory)).not.toContain('some old message');
});

it('clears the FileReadCache so post-reset Reads re-emit content', async () => {
const cacheClear = mockFileReadCacheClear();

await client.resetChat();

expect(cacheClear).toHaveBeenCalled();
});
});

describe('history mutation invalidates FileReadCache', () => {
it('setHistory clears the cache', () => {
const cacheClear = mockFileReadCacheClear();
client['chat'] = {
setHistory: vi.fn(),
} as unknown as GeminiChat;

client.setHistory([{ role: 'user', parts: [{ text: 'replaced' }] }]);

expect(cacheClear).toHaveBeenCalled();
});

/**
* Test helper: mock a GeminiChat whose history length goes from
* `before` to `after` across truncateHistory(). The first
* getHistoryLength() call (pre-truncate) returns `before`; the
* second (post-truncate) returns `after`.
*/
function mockChatWithLengths(before: number, after: number): GeminiChat {
return {
getHistoryLength: vi
.fn()
.mockReturnValueOnce(before)
.mockReturnValueOnce(after),
truncateHistory: vi.fn(),
} as unknown as GeminiChat;
}

it('truncateHistory clears the cache when entries are actually removed', () => {
const cacheClear = mockFileReadCacheClear();
client['chat'] = mockChatWithLengths(3, 2);

client.truncateHistory(2);

expect(cacheClear).toHaveBeenCalled();
});

it('truncateHistory does NOT clear the cache when nothing was removed (keepCount >= history length)', () => {
const cacheClear = mockFileReadCacheClear();

// keepCount equals history length — nothing dropped.
client['chat'] = mockChatWithLengths(2, 2);
client.truncateHistory(2);
expect(cacheClear).not.toHaveBeenCalled();

// keepCount exceeds history length — also a no-op.
client['chat'] = mockChatWithLengths(2, 2);
client.truncateHistory(99);
expect(cacheClear).not.toHaveBeenCalled();
});

it('truncateHistory clears the cache when a non-finite keepCount empties history (NaN regression)', () => {
// slice(0, NaN) returns [], but `NaN < prevLen` evaluates to
// false. Comparing the actual post-truncate length closes that
// hole — without this guard the cache would survive a history
// wipe and the file_unchanged placeholder bug returns.
const cacheClear = mockFileReadCacheClear();
client['chat'] = mockChatWithLengths(3, 0);

client.truncateHistory(NaN);

expect(cacheClear).toHaveBeenCalled();
});

it('truncateHistory uses O(1) getHistoryLength, not getHistory (avoids structuredClone)', () => {
mockFileReadCacheClear();
const getHistoryLength = vi.fn().mockReturnValue(5);
const getHistory = vi.fn();
client['chat'] = {
getHistoryLength,
getHistory,
truncateHistory: vi.fn(),
} as unknown as GeminiChat;

client.truncateHistory(3);

expect(getHistoryLength).toHaveBeenCalled();
expect(getHistory).not.toHaveBeenCalled();
});

it('retry strips orphaned trailing user entries and clears the cache', async () => {
const cacheClear = mockFileReadCacheClear();
const stripOrphanedUserEntriesFromHistory = vi.fn();
client['chat'] = {
addHistory: vi.fn(),
getHistory: vi.fn().mockReturnValue([]),
stripOrphanedUserEntriesFromHistory,
} as unknown as GeminiChat;
mockTurnRunFn.mockReturnValue(
(async function* () {
yield { type: GeminiEventType.Content, value: 'response' };
})(),
);

const stream = client.sendMessageStream(
[{ text: 'retry' }],
new AbortController().signal,
'prompt-retry-1',
{ type: SendMessageType.Retry },
);
for await (const _ of stream) {
/* drain */
}

expect(stripOrphanedUserEntriesFromHistory).toHaveBeenCalled();
expect(cacheClear).toHaveBeenCalled();
});
});

/**
* Test helper: replace mockConfig.getFileReadCache to return a stub
* whose clear() is a fresh spy. Returned spy lets tests assert on
* whether a code path invalidated the cache.
*/
function mockFileReadCacheClear(): ReturnType<typeof vi.fn> {
const clearMock = vi.fn();
vi.mocked(mockConfig.getFileReadCache).mockReturnValue({
clear: clearMock,
} as unknown as ReturnType<Config['getFileReadCache']>);
return clearMock;
}

describe('thinking block idle cleanup and latch', () => {
let mockChat: Partial<GeminiChat>;

Expand Down Expand Up @@ -506,6 +636,100 @@ describe('Gemini Client (client.ts)', () => {
});
});

describe('microcompaction FileReadCache invalidation', () => {
function makeReadFileResponses(count: number): Content[] {
const out: Content[] = [];
for (let i = 0; i < count; i++) {
out.push({
role: 'model',
parts: [
{
functionCall: {
name: 'read_file',
args: { file_path: `/x/${i}.ts` },
},
},
],
});
out.push({
role: 'user',
parts: [
{
functionResponse: {
name: 'read_file',
response: { output: `content of ${i}` },
},
},
],
});
}
return out;
}

beforeEach(() => {
mockTurnRunFn.mockReturnValue(
(async function* () {
yield { type: GeminiEventType.Content, value: 'response' };
})(),
);
});

it('clears the cache after microcompaction strips old read_file results', async () => {
// Default test fixture: toolResultsThresholdMinutes = 60,
// toolResultsNumToKeep = 5. Six read_file results + a 90-minute
// idle gap means the oldest one gets cleared, so the if-meta
// branch in sendMessageStream fires and must invalidate the cache.
const cacheClear = mockFileReadCacheClear();

const history = makeReadFileResponses(6);
const setHistory = vi.fn();
client['chat'] = {
addHistory: vi.fn(),
getHistory: vi.fn().mockReturnValue(history),
setHistory,
} as unknown as GeminiChat;
client['lastApiCompletionTimestamp'] = Date.now() - 90 * 60_000;

const stream = client.sendMessageStream(
[{ text: 'hi' }],
new AbortController().signal,
'prompt-mc-clear-1',
{ type: SendMessageType.UserQuery },
);
for await (const _ of stream) {
/* drain */
}

expect(setHistory).toHaveBeenCalled();
expect(cacheClear).toHaveBeenCalled();
});

it('does not clear the cache when the idle gap is below the threshold', async () => {
const cacheClear = mockFileReadCacheClear();

const history = makeReadFileResponses(6);
client['chat'] = {
addHistory: vi.fn(),
getHistory: vi.fn().mockReturnValue(history),
setHistory: vi.fn(),
} as unknown as GeminiChat;
// Recent activity — microcompaction must not fire.
client['lastApiCompletionTimestamp'] = Date.now() - 30 * 1000;

const stream = client.sendMessageStream(
[{ text: 'hi' }],
new AbortController().signal,
'prompt-mc-clear-2',
{ type: SendMessageType.UserQuery },
);
for await (const _ of stream) {
/* drain */
}

expect(cacheClear).not.toHaveBeenCalled();
});
});

describe('tryCompressChat', () => {
const mockGetHistory = vi.fn();

Expand Down
49 changes: 49 additions & 0 deletions packages/core/src/core/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,15 +206,47 @@ export class GeminiClient {

private stripOrphanedUserEntriesFromHistory() {
this.getChat().stripOrphanedUserEntriesFromHistory();
// Stripped trailing user entries can include read_file
// functionResponses from a failed-then-retried request. The
// FileReadCache would still record those reads, so the retry's
// re-issued Read could hit the file_unchanged placeholder while
// the model has nothing to fall back on. Clear to be safe.
debugLogger.debug(
'[FILE_READ_CACHE] clear after stripOrphanedUserEntriesFromHistory',
);
this.config.getFileReadCache().clear();
}

setHistory(history: Content[]) {
this.getChat().setHistory(history);
// Replacing history wholesale drops any prior read_file tool
// results the FileReadCache still believes the model has seen.
// Without clearing, a follow-up Read of an unchanged file would
// return the file_unchanged placeholder for bytes that no longer
// exist in the new history.
debugLogger.debug('[FILE_READ_CACHE] clear after setHistory');
this.config.getFileReadCache().clear();
this.forceFullIdeContext = true;
}

truncateHistory(keepCount: number) {
// Use the O(1) length getter rather than getHistory() — the latter
// structuredClone's the entire history just to read .length, which
// gets expensive in long-running sessions.
const prevLen = this.getChat().getHistoryLength();
this.getChat().truncateHistory(keepCount);
// Decide whether to invalidate based on the *actual* post-truncate
// length, not on the keepCount argument. Comparing keepCount alone
// misses pathological inputs (e.g. NaN: slice(0, NaN) returns [],
// emptying history, but `NaN < prevLen` is false and would skip
// the clear, reintroducing the file_unchanged placeholder bug).
const newLen = this.getChat().getHistoryLength();
if (newLen < prevLen) {
debugLogger.debug(
`[FILE_READ_CACHE] clear after truncateHistory(keep=${keepCount}, prev=${prevLen}, new=${newLen})`,
);
this.config.getFileReadCache().clear();
}
this.forceFullIdeContext = true;
}

Expand All @@ -233,6 +265,12 @@ export class GeminiClient {
async resetChat(): Promise<void> {
this.surfacedRelevantAutoMemoryPaths.clear();
this.lastApiCompletionTimestamp = null;
// startChat() rewrites the chat to its initial state. Any prior
// read_file tool results the FileReadCache still tracks are no
// longer in history, so a follow-up Read would serve a placeholder
// pointing at content the model can no longer retrieve.
Comment thread
wenshao marked this conversation as resolved.
debugLogger.debug('[FILE_READ_CACHE] clear after resetChat');
this.config.getFileReadCache().clear();
await this.startChat();
}

Expand Down Expand Up @@ -694,6 +732,16 @@ export class GeminiClient {
);
if (mcResult.meta) {
this.getChat().setHistory(mcResult.history);
// Microcompaction replaces old compactable tool outputs
// (including read_file) with a placeholder, but the
// FileReadCache still records the prior full Reads as "seen in
// this conversation". A follow-up Read of an unchanged file
// would then return the file_unchanged placeholder pointing at
// bytes the model can no longer retrieve from history. Drop the
// cache so post-microcompaction Reads re-emit the bytes,
// mirroring the post-compaction clear in tryCompressChat.
debugLogger.debug('[FILE_READ_CACHE] clear after microcompaction');
this.config.getFileReadCache().clear();
Comment thread
wenshao marked this conversation as resolved.
const m = mcResult.meta;
debugLogger.debug(
`[TIME-BASED MC] gap ${m.gapMinutes}min > ${m.thresholdMinutes}min, ` +
Expand Down Expand Up @@ -1166,6 +1214,7 @@ export class GeminiClient {
// placeholder pointing at content the model can no longer
// retrieve from its own context. Clear the cache so post-
// compaction Reads re-emit the bytes.
debugLogger.debug('[FILE_READ_CACHE] clear after tryCompressChat');
this.config.getFileReadCache().clear();
uiTelemetryService.setLastPromptTokenCount(info.newTokenCount);
this.forceFullIdeContext = true;
Expand Down
19 changes: 19 additions & 0 deletions packages/core/src/core/geminiChat.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,25 @@ describe('GeminiChat', async () => {
});
});

describe('getHistoryLength', () => {
it('returns 0 for an empty history', () => {
expect(chat.getHistoryLength()).toBe(0);
});

it('reflects entries added via addHistory', () => {
chat.addHistory({ role: 'user', parts: [{ text: 'a' }] });
chat.addHistory({ role: 'model', parts: [{ text: 'b' }] });
expect(chat.getHistoryLength()).toBe(2);
});

it('matches getHistory().length without paying the structuredClone cost', () => {
chat.addHistory({ role: 'user', parts: [{ text: 'a' }] });
chat.addHistory({ role: 'model', parts: [{ text: 'b' }] });
chat.addHistory({ role: 'user', parts: [{ text: 'c' }] });
expect(chat.getHistoryLength()).toBe(chat.getHistory().length);
});
});

describe('sendMessageStream with retries', () => {
it('should retry on invalid content, succeed, and report metrics', async () => {
vi.useFakeTimers();
Expand Down
9 changes: 9 additions & 0 deletions packages/core/src/core/geminiChat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,15 @@ export class GeminiChat {
return structuredClone(history);
}

/**
* Returns the number of entries in the raw chat history. O(1) and
* does not clone — use this when you only need the count and would
* otherwise pay the {@link getHistory} `structuredClone` cost.
*/
getHistoryLength(): number {
return this.history.length;
}

/**
* Clears the chat history.
*/
Expand Down
Loading
Loading