MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-04-27 13:34:15 -07:00
Code Issues Packages Projects Releases Wiki Activity

refactor: make baseTimestamp optional in addItem and remove redundant calls (#16471)

Browse Source
This commit is contained in:
Sehoon Shon
2026-01-13 14:15:04 -05:00
committed by GitHub
parent aa52462550
commit 91fcca3b1c
30 changed files with 528 additions and 888 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/cli/src/ui/hooks/useGeminiStream.test.tsx
+24 -45
View File
@@ -780,7 +780,6 @@ describe('useGeminiStream', () => {
'Agent execution stopped: Stop reason from hook',
),
}),
expect.any(Number),
);
// Ensure we do NOT call back to the API
expect(mockSendMessageStream).not.toHaveBeenCalled();
@@ -1085,13 +1084,10 @@ describe('useGeminiStream', () => {
// Verify cancellation message is added
await waitFor(() => {
expect(mockAddItem).toHaveBeenCalledWith(
{
type: MessageType.INFO,
text: 'Request cancelled.',
},
expect.any(Number),
);
expect(mockAddItem).toHaveBeenCalledWith({
type: MessageType.INFO,
text: 'Request cancelled.',
});
});
// Verify state is reset
@@ -1194,7 +1190,6 @@ describe('useGeminiStream', () => {
expect.objectContaining({
text: 'Request cancelled.',
}),
expect.any(Number),
);
});
@@ -1330,7 +1325,6 @@ describe('useGeminiStream', () => {
expect.objectContaining({
text: 'Request cancelled.',
}),
expect.any(Number),
);
});
@@ -1995,13 +1989,10 @@ describe('useGeminiStream', () => {
});
await waitFor(() => {
expect(mockAddItem).toHaveBeenCalledWith(
{
type: 'info',
text: expectedMessage,
},
expect.any(Number),
);
expect(mockAddItem).toHaveBeenCalledWith({
type: 'info',
text: expectedMessage,
});
});
},
);
@@ -2644,13 +2635,10 @@ describe('useGeminiStream', () => {
expect(result.current.loopDetectionConfirmationRequest).toBeNull();
// Verify appropriate message was added
expect(mockAddItem).toHaveBeenCalledWith(
{
type: 'info',
text: 'Loop detection has been disabled for this session. Retrying request...',
},
expect.any(Number),
);
expect(mockAddItem).toHaveBeenCalledWith({
type: 'info',
text: 'Loop detection has been disabled for this session. Retrying request...',
});
// Verify that the request was retried
await waitFor(() => {
@@ -2707,13 +2695,10 @@ describe('useGeminiStream', () => {
expect(result.current.loopDetectionConfirmationRequest).toBeNull();
// Verify appropriate message was added
expect(mockAddItem).toHaveBeenCalledWith(
{
type: 'info',
text: 'A potential loop was detected. This can happen due to repetitive tool calls or other model behavior. The request has been halted.',
},
expect.any(Number),
);
expect(mockAddItem).toHaveBeenCalledWith({
type: 'info',
text: 'A potential loop was detected. This can happen due to repetitive tool calls or other model behavior. The request has been halted.',
});
// Verify that the request was NOT retried
expect(mockSendMessageStream).toHaveBeenCalledTimes(1);
@@ -2750,13 +2735,10 @@ describe('useGeminiStream', () => {
expect(result.current.loopDetectionConfirmationRequest).toBeNull();
// Verify first message was added
expect(mockAddItem).toHaveBeenCalledWith(
{
type: 'info',
text: 'A potential loop was detected. This can happen due to repetitive tool calls or other model behavior. The request has been halted.',
},
expect.any(Number),
);
expect(mockAddItem).toHaveBeenCalledWith({
type: 'info',
text: 'A potential loop was detected. This can happen due to repetitive tool calls or other model behavior. The request has been halted.',
});
// Second loop detection - set up fresh mock for second call
mockSendMessageStream.mockReturnValueOnce(
@@ -2800,13 +2782,10 @@ describe('useGeminiStream', () => {
expect(result.current.loopDetectionConfirmationRequest).toBeNull();
// Verify second message was added
expect(mockAddItem).toHaveBeenCalledWith(
{
type: 'info',
text: 'Loop detection has been disabled for this session. Retrying request...',
},
expect.any(Number),
);
expect(mockAddItem).toHaveBeenCalledWith({
type: 'info',
text: 'Loop detection has been disabled for this session. Retrying request...',
});
// Verify that the request was retried
await waitFor(() => {
packages/cli/src/ui/hooks/useGeminiStream.ts
+41 -72
View File
@@ -149,7 +149,6 @@ export const useGeminiStream = (
mapTrackedToolCallsToDisplay(
completedToolCallsFromScheduler as TrackedToolCall[],
),
Date.now(),
);
// Clear the live-updating display now that the final state is in history.
@@ -248,10 +247,7 @@ export const useGeminiStream = (
prevActiveShellPtyIdRef.current !== null &&
activeShellPtyId === null
) {
addItem(
{ type: MessageType.INFO, text: 'Request cancelled.' },
Date.now(),
);
addItem({ type: MessageType.INFO, text: 'Request cancelled.' });
setIsResponding(false);
}
prevActiveShellPtyIdRef.current = activeShellPtyId;
@@ -351,12 +347,9 @@ export const useGeminiStream = (
}
return tool;
});
addItem(
{ ...toolGroup, tools: updatedTools } as HistoryItemWithoutId,
Date.now(),
);
addItem({ ...toolGroup, tools: updatedTools } as HistoryItemWithoutId);
} else {
addItem(pendingHistoryItemRef.current, Date.now());
addItem(pendingHistoryItemRef.current);
}
}
setPendingHistoryItem(null);
@@ -368,13 +361,10 @@ export const useGeminiStream = (
// If shell is active, we delay this message to ensure correct ordering
// (Shell item first, then Info message).
if (!activeShellPtyId) {
addItem(
{
type: MessageType.INFO,
text: 'Request cancelled.',
},
Date.now(),
);
addItem({
type: MessageType.INFO,
text: 'Request cancelled.',
});
setIsResponding(false);
}
}
@@ -719,32 +709,26 @@ export const useGeminiStream = (
addItem(pendingHistoryItemRef.current, userMessageTimestamp);
setPendingHistoryItem(null);
}
return addItem(
{
type: 'info',
text:
`IMPORTANT: This conversation exceeded the compress threshold. ` +
`A compressed context will be sent for future messages (compressed from: ` +
`${eventValue?.originalTokenCount ?? 'unknown'} to ` +
`${eventValue?.newTokenCount ?? 'unknown'} tokens).`,
},
Date.now(),
);
return addItem({
type: 'info',
text:
`IMPORTANT: This conversation exceeded the compress threshold. ` +
`A compressed context will be sent for future messages (compressed from: ` +
`${eventValue?.originalTokenCount ?? 'unknown'} to ` +
`${eventValue?.newTokenCount ?? 'unknown'} tokens).`,
});
},
[addItem, pendingHistoryItemRef, setPendingHistoryItem],
);
const handleMaxSessionTurnsEvent = useCallback(
() =>
addItem(
{
type: 'info',
text:
`The session has reached the maximum number of turns: ${config.getMaxSessionTurns()}. ` +
`Please update this limit in your setting.json file.`,
},
Date.now(),
),
addItem({
type: 'info',
text:
`The session has reached the maximum number of turns: ${config.getMaxSessionTurns()}. ` +
`Please update this limit in your setting.json file.`,
}),
[addItem, config],
);
@@ -764,13 +748,10 @@ export const useGeminiStream = (
' Please try reducing the size of your message or use the `/compress` command to compress the chat history.';
}
addItem(
{
type: 'info',
text,
},
Date.now(),
);
addItem({
type: 'info',
text,
});
},
[addItem, onCancelSubmit, config],
);
@@ -1041,13 +1022,10 @@ export const useGeminiStream = (
.getGeminiClient()
.getLoopDetectionService()
.disableForSession();
addItem(
{
type: 'info',
text: `Loop detection has been disabled for this session. Retrying request...`,
},
Date.now(),
);
addItem({
type: 'info',
text: `Loop detection has been disabled for this session. Retrying request...`,
});
if (lastQueryRef.current && lastPromptIdRef.current) {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
@@ -1058,13 +1036,10 @@ export const useGeminiStream = (
);
}
} else {
addItem(
{
type: 'info',
text: `A potential loop was detected. This can happen due to repetitive tool calls or other model behavior. The request has been halted.`,
},
Date.now(),
);
addItem({
type: 'info',
text: `A potential loop was detected. This can happen due to repetitive tool calls or other model behavior. The request has been halted.`,
});
}
},
});
@@ -1215,13 +1190,10 @@ export const useGeminiStream = (
);
if (stopExecutionTool && stopExecutionTool.response.error) {
addItem(
{
type: MessageType.INFO,
text: `Agent execution stopped: ${stopExecutionTool.response.error.message}`,
},
Date.now(),
);
addItem({
type: MessageType.INFO,
text: `Agent execution stopped: ${stopExecutionTool.response.error.message}`,
});
setIsResponding(false);
const callIdsToMarkAsSubmitted = geminiTools.map(
@@ -1240,13 +1212,10 @@ export const useGeminiStream = (
// If the turn was cancelled via the imperative escape key flow,
// the cancellation message is added there. We check the ref to avoid duplication.
if (!turnCancelledRef.current) {
addItem(
{
type: MessageType.INFO,
text: 'Request cancelled.',
},
Date.now(),
);
addItem({
type: MessageType.INFO,
text: 'Request cancelled.',
});
}
setIsResponding(false);
packages/cli/src/ui/hooks/useHistoryManager.test.ts
+19
View File
@@ -200,4 +200,23 @@ describe('useHistoryManager', () => {
expect(result.current.history[1].text).toBe('Gemini response');
expect(result.current.history[2].text).toBe('Message 1');
});
it('should use Date.now() as default baseTimestamp if not provided', () => {
const { result } = renderHook(() => useHistory());
const before = Date.now();
const itemData: Omit<HistoryItem, 'id'> = {
type: 'user',
text: 'Default timestamp test',
};
act(() => {
result.current.addItem(itemData);
});
const after = Date.now();
expect(result.current.history).toHaveLength(1);
// ID should be >= before + 1 (since counter starts at 0 and increments to 1)
expect(result.current.history[0].id).toBeGreaterThanOrEqual(before + 1);
expect(result.current.history[0].id).toBeLessThanOrEqual(after + 1);
});
});
packages/cli/src/ui/hooks/useHistoryManager.ts
+2 -2
View File
@@ -17,7 +17,7 @@ export interface UseHistoryManagerReturn {
history: HistoryItem[];
addItem: (
itemData: Omit<HistoryItem, 'id'>,
baseTimestamp: number,
baseTimestamp?: number,
isResuming?: boolean,
) => number; // Returns the generated ID
updateItem: (
@@ -56,7 +56,7 @@ export function useHistory({
const addItem = useCallback(
(
itemData: Omit<HistoryItem, 'id'>,
baseTimestamp: number,
baseTimestamp: number = Date.now(),
isResuming: boolean = false,
): number => {
const id = getNextMessageId(baseTimestamp);
packages/cli/src/ui/hooks/useIncludeDirsTrust.test.tsx
+1 -2
View File
@@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { vi, describe, it, expect, beforeEach } from 'vitest';
import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
import type { Mock } from 'vitest';
import { renderHook } from '../../test-utils/render.js';
import { waitFor } from '../../test-utils/async.js';
@@ -132,7 +132,6 @@ describe('useIncludeDirsTrust', () => {
expect.objectContaining({
text: expect.stringContaining("Error adding '/dir2': Test error"),
}),
expect.any(Number),
);
expect(
mockConfig.clearPendingIncludeDirectories,
packages/cli/src/ui/hooks/useIncludeDirsTrust.tsx
+9 -9
View File
@@ -18,18 +18,18 @@ import { MessageType, type HistoryItem } from '../types.js';
async function finishAddingDirectories(
config: Config,
addItem: (itemData: Omit<HistoryItem, 'id'>, baseTimestamp: number) => number,
addItem: (
itemData: Omit<HistoryItem, 'id'>,
baseTimestamp?: number,
) => number,
added: string[],
errors: string[],
) {
if (!config) {
addItem(
{
type: MessageType.ERROR,
text: 'Configuration is not available.',
},
Date.now(),
);
addItem({
type: MessageType.ERROR,
text: 'Configuration is not available.',
});
return;
}
@@ -49,7 +49,7 @@ async function finishAddingDirectories(
}
if (errors.length > 0) {
addItem({ type: MessageType.ERROR, text: errors.join('\n') }, Date.now());
addItem({ type: MessageType.ERROR, text: errors.join('\n') });
}
}