Push model-switching logging into loadCliConfig (#815)

This commit is contained in:
Tommaso Sciortino 2025-06-07 11:12:30 -07:00 committed by GitHub
parent 680f4cdd61
commit 6ea4479064
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 45 additions and 112 deletions

View File

@ -83,29 +83,29 @@ describe('loadCliConfig', () => {
it('should set showMemoryUsage to true when --memory flag is present', async () => {
process.argv = ['node', 'script.js', '--show_memory_usage'];
const settings: Settings = {};
const result = await loadCliConfig(settings);
expect(result.config.getShowMemoryUsage()).toBe(true);
const config = await loadCliConfig(settings);
expect(config.getShowMemoryUsage()).toBe(true);
});
it('should set showMemoryUsage to false when --memory flag is not present', async () => {
process.argv = ['node', 'script.js'];
const settings: Settings = {};
const result = await loadCliConfig(settings);
expect(result.config.getShowMemoryUsage()).toBe(false);
const config = await loadCliConfig(settings);
expect(config.getShowMemoryUsage()).toBe(false);
});
it('should set showMemoryUsage to false by default from settings if CLI flag is not present', async () => {
process.argv = ['node', 'script.js'];
const settings: Settings = { showMemoryUsage: false };
const result = await loadCliConfig(settings);
expect(result.config.getShowMemoryUsage()).toBe(false);
const config = await loadCliConfig(settings);
expect(config.getShowMemoryUsage()).toBe(false);
});
it('should prioritize CLI flag over settings for showMemoryUsage (CLI true, settings false)', async () => {
process.argv = ['node', 'script.js', '--show_memory_usage'];
const settings: Settings = { showMemoryUsage: false };
const result = await loadCliConfig(settings);
expect(result.config.getShowMemoryUsage()).toBe(true);
const config = await loadCliConfig(settings);
expect(config.getShowMemoryUsage()).toBe(true);
});
});
@ -128,50 +128,50 @@ describe('loadCliConfig telemetry', () => {
it('should set telemetry to false by default when no flag or setting is present', async () => {
process.argv = ['node', 'script.js'];
const settings: Settings = {};
const result = await loadCliConfig(settings);
expect(result.config.getTelemetry()).toBe(false);
const config = await loadCliConfig(settings);
expect(config.getTelemetry()).toBe(false);
});
it('should set telemetry to true when --telemetry flag is present', async () => {
process.argv = ['node', 'script.js', '--telemetry'];
const settings: Settings = {};
const result = await loadCliConfig(settings);
expect(result.config.getTelemetry()).toBe(true);
const config = await loadCliConfig(settings);
expect(config.getTelemetry()).toBe(true);
});
it('should set telemetry to false when --no-telemetry flag is present', async () => {
process.argv = ['node', 'script.js', '--no-telemetry'];
const settings: Settings = {};
const result = await loadCliConfig(settings);
expect(result.config.getTelemetry()).toBe(false);
const config = await loadCliConfig(settings);
expect(config.getTelemetry()).toBe(false);
});
it('should use telemetry value from settings if CLI flag is not present (settings true)', async () => {
process.argv = ['node', 'script.js'];
const settings: Settings = { telemetry: true };
const result = await loadCliConfig(settings);
expect(result.config.getTelemetry()).toBe(true);
const config = await loadCliConfig(settings);
expect(config.getTelemetry()).toBe(true);
});
it('should use telemetry value from settings if CLI flag is not present (settings false)', async () => {
process.argv = ['node', 'script.js'];
const settings: Settings = { telemetry: false };
const result = await loadCliConfig(settings);
expect(result.config.getTelemetry()).toBe(false);
const config = await loadCliConfig(settings);
expect(config.getTelemetry()).toBe(false);
});
it('should prioritize --telemetry CLI flag (true) over settings (false)', async () => {
process.argv = ['node', 'script.js', '--telemetry'];
const settings: Settings = { telemetry: false };
const result = await loadCliConfig(settings);
expect(result.config.getTelemetry()).toBe(true);
const config = await loadCliConfig(settings);
expect(config.getTelemetry()).toBe(true);
});
it('should prioritize --no-telemetry CLI flag (false) over settings (true)', async () => {
process.argv = ['node', 'script.js', '--no-telemetry'];
const settings: Settings = { telemetry: true };
const result = await loadCliConfig(settings);
expect(result.config.getTelemetry()).toBe(false);
const config = await loadCliConfig(settings);
expect(config.getTelemetry()).toBe(false);
});
});

View File

@ -18,10 +18,7 @@ import {
ApprovalMode,
} from '@gemini-code/core';
import { Settings } from './settings.js';
import {
getEffectiveModel,
type EffectiveModelCheckResult,
} from '../utils/modelCheck.js';
import { getEffectiveModel } from '../utils/modelCheck.js';
import { getCliVersion } from '../utils/version.js';
// Simple console logger for now - replace with actual logger if available
@ -119,17 +116,10 @@ export async function loadHierarchicalGeminiMemory(
return loadServerHierarchicalMemory(currentWorkingDirectory, debugMode);
}
export interface LoadCliConfigResult {
config: Config;
modelWasSwitched: boolean;
originalModelBeforeSwitch?: string;
finalModel: string;
}
export async function loadCliConfig(
settings: Settings,
geminiIgnorePatterns: string[],
): Promise<LoadCliConfigResult> {
): Promise<Config> {
loadEnvironment();
const geminiApiKey = process.env.GEMINI_API_KEY;
@ -180,19 +170,8 @@ export async function loadCliConfig(
const useVertexAI = hasGeminiApiKey ? false : undefined;
let modelToUse = argv.model || DEFAULT_GEMINI_MODEL;
let modelSwitched = false;
let originalModel: string | undefined = undefined;
if (apiKeyForServer) {
const checkResult: EffectiveModelCheckResult = await getEffectiveModel(
apiKeyForServer,
modelToUse,
);
if (checkResult.switched) {
modelSwitched = true;
originalModel = checkResult.originalModelIfSwitched;
modelToUse = checkResult.effectiveModel;
}
modelToUse = await getEffectiveModel(apiKeyForServer, modelToUse);
}
const configParams: ConfigParameters = {
@ -227,11 +206,5 @@ export async function loadCliConfig(
settings.fileFiltering?.allowBuildArtifacts,
};
const config = createServerConfig(configParams);
return {
config,
modelWasSwitched: modelSwitched,
originalModelBeforeSwitch: originalModel,
finalModel: modelToUse,
};
return createServerConfig(configParams);
}

View File

@ -66,18 +66,11 @@ export async function main() {
process.exit(1);
}
const { config, modelWasSwitched, originalModelBeforeSwitch, finalModel } =
await loadCliConfig(settings.merged, geminiIgnorePatterns);
const config = await loadCliConfig(settings.merged, geminiIgnorePatterns);
// Initialize centralized FileDiscoveryService
await config.getFileService();
if (modelWasSwitched && originalModelBeforeSwitch) {
console.log(
`[INFO] Your configured model (${originalModelBeforeSwitch}) was temporarily unavailable. Switched to ${finalModel} for this session.`,
);
}
if (settings.merged.theme) {
if (!themeManager.setActiveTheme(settings.merged.theme)) {
// If the theme is not found during initial load, log a warning and continue.
@ -176,9 +169,8 @@ async function loadNonInteractiveConfig(
...settings.merged,
coreTools: nonInteractiveTools,
};
const nonInteractiveConfigResult = await loadCliConfig(
return await loadCliConfig(
nonInteractiveSettings,
config.getGeminiIgnorePatterns(),
);
return nonInteractiveConfigResult.config;
}

View File

@ -5,10 +5,7 @@
*/
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import {
getEffectiveModel,
type EffectiveModelCheckResult,
} from './modelCheck.js';
import { getEffectiveModel } from './modelCheck.js';
import {
DEFAULT_GEMINI_MODEL,
DEFAULT_GEMINI_FLASH_MODEL,
@ -45,13 +42,10 @@ describe('getEffectiveModel', () => {
});
describe('when currentConfiguredModel is not DEFAULT_GEMINI_MODEL', () => {
it('should return the currentConfiguredModel and switched: false without fetching', async () => {
it('should return the currentConfiguredModel without fetching', async () => {
const customModel = 'custom-model-name';
const result = await getEffectiveModel(apiKey, customModel);
expect(result).toEqual({
effectiveModel: customModel,
switched: false,
});
expect(result).toEqual(customModel);
expect(fetch).not.toHaveBeenCalled();
});
});
@ -62,15 +56,8 @@ describe('getEffectiveModel', () => {
ok: false,
status: 429,
});
const result: EffectiveModelCheckResult = await getEffectiveModel(
apiKey,
DEFAULT_GEMINI_MODEL,
);
expect(result).toEqual({
effectiveModel: DEFAULT_GEMINI_FLASH_MODEL,
switched: true,
originalModelIfSwitched: DEFAULT_GEMINI_MODEL,
});
const result = await getEffectiveModel(apiKey, DEFAULT_GEMINI_MODEL);
expect(result).toEqual(DEFAULT_GEMINI_FLASH_MODEL);
expect(fetch).toHaveBeenCalledTimes(1);
expect(fetch).toHaveBeenCalledWith(
`https://generativelanguage.googleapis.com/v1beta/models/${DEFAULT_GEMINI_MODEL}:generateContent?key=${apiKey}`,
@ -84,10 +71,7 @@ describe('getEffectiveModel', () => {
status: 200,
});
const result = await getEffectiveModel(apiKey, DEFAULT_GEMINI_MODEL);
expect(result).toEqual({
effectiveModel: DEFAULT_GEMINI_MODEL,
switched: false,
});
expect(result).toEqual(DEFAULT_GEMINI_MODEL);
expect(fetch).toHaveBeenCalledTimes(1);
});
@ -97,20 +81,14 @@ describe('getEffectiveModel', () => {
status: 500,
});
const result = await getEffectiveModel(apiKey, DEFAULT_GEMINI_MODEL);
expect(result).toEqual({
effectiveModel: DEFAULT_GEMINI_MODEL,
switched: false,
});
expect(result).toEqual(DEFAULT_GEMINI_MODEL);
expect(fetch).toHaveBeenCalledTimes(1);
});
it('should return DEFAULT_GEMINI_MODEL if fetch throws a network error', async () => {
(fetch as vi.Mock).mockRejectedValueOnce(new Error('Network error'));
const result = await getEffectiveModel(apiKey, DEFAULT_GEMINI_MODEL);
expect(result).toEqual({
effectiveModel: DEFAULT_GEMINI_MODEL,
switched: false,
});
expect(result).toEqual(DEFAULT_GEMINI_MODEL);
expect(fetch).toHaveBeenCalledTimes(1);
});
@ -146,10 +124,7 @@ describe('getEffectiveModel', () => {
expect(mockAbort).toHaveBeenCalledTimes(0); // setTimeout calls controller.abort(), not our direct mockAbort
expect(abortControllerInstance.abort).toHaveBeenCalledTimes(1);
expect(result).toEqual({
effectiveModel: DEFAULT_GEMINI_MODEL,
switched: false,
});
expect(result).toEqual(DEFAULT_GEMINI_MODEL);
expect(fetch).toHaveBeenCalledTimes(1);
});

View File

@ -9,12 +9,6 @@ import {
DEFAULT_GEMINI_FLASH_MODEL,
} from '../config/config.js';
export interface EffectiveModelCheckResult {
effectiveModel: string;
switched: boolean;
originalModelIfSwitched?: string;
}
/**
* Checks if the default "pro" model is rate-limited and returns a fallback "flash"
* model if necessary. This function is designed to be silent.
@ -26,10 +20,10 @@ export interface EffectiveModelCheckResult {
export async function getEffectiveModel(
apiKey: string,
currentConfiguredModel: string,
): Promise<EffectiveModelCheckResult> {
): Promise<string> {
if (currentConfiguredModel !== DEFAULT_GEMINI_MODEL) {
// Only check if the user is trying to use the specific pro model we want to fallback from.
return { effectiveModel: currentConfiguredModel, switched: false };
return currentConfiguredModel;
}
const modelToTest = DEFAULT_GEMINI_MODEL;
@ -59,17 +53,16 @@ export async function getEffectiveModel(
clearTimeout(timeoutId);
if (response.status === 429) {
return {
effectiveModel: fallbackModel,
switched: true,
originalModelIfSwitched: modelToTest,
};
console.log(
`[INFO] Your configured model (${modelToTest}) was temporarily unavailable. Switched to ${fallbackModel} for this session.`,
);
return fallbackModel;
}
// For any other case (success, other error codes), we stick to the original model.
return { effectiveModel: currentConfiguredModel, switched: false };
return currentConfiguredModel;
} catch (_error) {
clearTimeout(timeoutId);
// On timeout or any other fetch error, stick to the original model.
return { effectiveModel: currentConfiguredModel, switched: false };
return currentConfiguredModel;
}
}