From b465e12747823d29e8c3522e5c1cb70f0013a266 Mon Sep 17 00:00:00 2001 From: Adam Weidman <65992621+adamfweidman@users.noreply.github.com> Date: Wed, 17 Dec 2025 14:23:56 -0500 Subject: [PATCH] chore(core): remove redundant isModelAvailabilityServiceEnabled toggle and clean up dead code (#15207) --- docs/get-started/configuration.md | 5 -- packages/cli/src/config/config.ts | 2 - .../cli/src/config/settingsSchema.test.ts | 15 ------ packages/cli/src/config/settingsSchema.ts | 9 ---- .../src/availability/policyHelpers.test.ts | 10 ---- .../core/src/availability/policyHelpers.ts | 9 +--- packages/core/src/config/config.ts | 7 --- packages/core/src/core/baseLlmClient.test.ts | 26 ++-------- packages/core/src/core/baseLlmClient.ts | 28 +++++------ packages/core/src/core/client.test.ts | 50 +++---------------- packages/core/src/core/client.ts | 38 ++++---------- packages/core/src/core/geminiChat.test.ts | 8 ++- .../src/core/geminiChat_network_retry.test.ts | 5 +- packages/core/src/fallback/handler.test.ts | 4 -- .../strategies/fallbackStrategy.test.ts | 10 ---- .../routing/strategies/fallbackStrategy.ts | 4 -- schemas/settings.schema.json | 7 --- 17 files changed, 42 insertions(+), 195 deletions(-) diff --git a/docs/get-started/configuration.md b/docs/get-started/configuration.md index e3415eccfa..725e083006 100644 --- a/docs/get-started/configuration.md +++ b/docs/get-started/configuration.md @@ -798,11 +798,6 @@ their corresponding top-level category object in your `settings.json` file. - **Default:** `false` - **Requires restart:** Yes -- **`experimental.isModelAvailabilityServiceEnabled`** (boolean): - - **Description:** Enable model routing using new availability service. - - **Default:** `false` - - **Requires restart:** Yes - - **`experimental.jitContext`** (boolean): - **Description:** Enable Just-In-Time (JIT) context loading. - **Default:** `false` diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 255eb6f1be..8118268503 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -643,8 +643,6 @@ export async function loadCliConfig( extensionLoader: extensionManager, enableExtensionReloading: settings.experimental?.extensionReloading, enableAgents: settings.experimental?.enableAgents, - enableModelAvailabilityService: - settings.experimental?.isModelAvailabilityServiceEnabled, experimentalJitContext: settings.experimental?.jitContext, noBrowser: !!process.env['NO_BROWSER'], summarizeToolOutput: settings.model?.summarizeToolOutput, diff --git a/packages/cli/src/config/settingsSchema.test.ts b/packages/cli/src/config/settingsSchema.test.ts index 343b3526e7..fd4ddf8ff9 100644 --- a/packages/cli/src/config/settingsSchema.test.ts +++ b/packages/cli/src/config/settingsSchema.test.ts @@ -354,21 +354,6 @@ describe('SettingsSchema', () => { expect(setting.showInDialog).toBe(false); expect(setting.description).toBe('Enable local and remote subagents.'); }); - - it('should have isModelAvailabilityServiceEnabled setting in schema', () => { - const setting = - getSettingsSchema().experimental.properties - .isModelAvailabilityServiceEnabled; - expect(setting).toBeDefined(); - expect(setting.type).toBe('boolean'); - expect(setting.category).toBe('Experimental'); - expect(setting.default).toBe(false); - expect(setting.requiresRestart).toBe(true); - expect(setting.showInDialog).toBe(false); - expect(setting.description).toBe( - 'Enable model routing using new availability service.', - ); - }); }); it('has JSON schema definitions for every referenced ref', () => { diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 5b8f75af60..8e8347ab15 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -1323,15 +1323,6 @@ const SETTINGS_SCHEMA = { 'Enables extension loading/unloading within the CLI session.', showInDialog: false, }, - isModelAvailabilityServiceEnabled: { - type: 'boolean', - label: 'Enable Model Availability Service', - category: 'Experimental', - requiresRestart: true, - default: false, - description: 'Enable model routing using new availability service.', - showInDialog: false, - }, jitContext: { type: 'boolean', label: 'JIT Context Loading', diff --git a/packages/core/src/availability/policyHelpers.test.ts b/packages/core/src/availability/policyHelpers.test.ts index 52789a342e..df20d28fad 100644 --- a/packages/core/src/availability/policyHelpers.test.ts +++ b/packages/core/src/availability/policyHelpers.test.ts @@ -119,7 +119,6 @@ describe('policyHelpers', () => { overrides: Partial = {}, ): Config => { const defaults = { - isModelAvailabilityServiceEnabled: () => true, getModelAvailabilityService: () => mockAvailabilityService, setActiveModel: vi.fn(), modelConfigService: mockModelConfigService, @@ -131,15 +130,6 @@ describe('policyHelpers', () => { vi.clearAllMocks(); }); - it('returns requested model if availability service is disabled', () => { - const config = createExtendedMockConfig({ - isModelAvailabilityServiceEnabled: () => false, - }); - const result = applyModelSelection(config, 'gemini-pro'); - expect(result.model).toBe('gemini-pro'); - expect(config.setActiveModel).not.toHaveBeenCalled(); - }); - it('returns requested model if it is available', () => { const config = createExtendedMockConfig(); mockAvailabilityService.selectFirstAvailable.mockReturnValue({ diff --git a/packages/core/src/availability/policyHelpers.ts b/packages/core/src/availability/policyHelpers.ts index 4177461544..484bec3215 100644 --- a/packages/core/src/availability/policyHelpers.ts +++ b/packages/core/src/availability/policyHelpers.ts @@ -117,9 +117,6 @@ export function createAvailabilityContextProvider( modelGetter: () => string, ): () => RetryAvailabilityContext | undefined { return () => { - if (!config.isModelAvailabilityServiceEnabled()) { - return undefined; - } const service = config.getModelAvailabilityService(); const currentModel = modelGetter(); @@ -138,11 +135,7 @@ export function createAvailabilityContextProvider( export function selectModelForAvailability( config: Config, requestedModel: string, -): ModelSelectionResult | undefined { - if (!config.isModelAvailabilityServiceEnabled()) { - return undefined; - } - +): ModelSelectionResult { const chain = resolvePolicyChain(config, requestedModel); const selection = config .getModelAvailabilityService() diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 0289259f47..a0ad61c41f 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -326,7 +326,6 @@ export interface ConfigParameters { } & { disabled?: string[] }); previewFeatures?: boolean; enableAgents?: boolean; - enableModelAvailabilityService?: boolean; experimentalJitContext?: boolean; } @@ -449,7 +448,6 @@ export class Config { private previewModelFallbackMode = false; private previewModelBypassMode = false; - private readonly enableModelAvailabilityService: boolean; private readonly enableAgents: boolean; private readonly experimentalJitContext: boolean; @@ -513,7 +511,6 @@ export class Config { this.bugCommand = params.bugCommand; this.model = params.model; this._activeModel = params.model; - this.enableModelAvailabilityService = true; this.enableAgents = params.enableAgents ?? false; this.experimentalJitContext = params.experimentalJitContext ?? false; this.modelAvailabilityService = new ModelAvailabilityService(); @@ -1309,10 +1306,6 @@ export class Config { return this.enableExtensionReloading; } - isModelAvailabilityServiceEnabled(): boolean { - return this.enableModelAvailabilityService; - } - isAgentsEnabled(): boolean { return this.enableAgents; } diff --git a/packages/core/src/core/baseLlmClient.test.ts b/packages/core/src/core/baseLlmClient.test.ts index 4d768923d1..c3499f6e75 100644 --- a/packages/core/src/core/baseLlmClient.test.ts +++ b/packages/core/src/core/baseLlmClient.test.ts @@ -111,8 +111,9 @@ describe('BaseLlmClient', () => { .fn() .mockImplementation(({ model }) => makeResolvedModelConfig(model)), } as unknown as ModelConfigService, - isModelAvailabilityServiceEnabled: vi.fn().mockReturnValue(false), - getModelAvailabilityService: vi.fn(), + getModelAvailabilityService: vi + .fn() + .mockReturnValue(createAvailabilityServiceMock()), setActiveModel: vi.fn(), getPreviewFeatures: vi.fn().mockReturnValue(false), getUserTier: vi.fn().mockReturnValue(undefined), @@ -614,10 +615,6 @@ describe('BaseLlmClient', () => { let jsonOptions: GenerateJsonOptions; beforeEach(() => { - mockConfig.isModelAvailabilityServiceEnabled = vi - .fn() - .mockReturnValue(true); - mockAvailabilityService = createAvailabilityServiceMock({ selectedModel: 'test-model', skipped: [], @@ -647,23 +644,6 @@ describe('BaseLlmClient', () => { }; }); - it('should preserve legacy behavior when availability is disabled', async () => { - mockConfig.isModelAvailabilityServiceEnabled = vi - .fn() - .mockReturnValue(false); - mockGenerateContent.mockResolvedValue( - createMockResponse('Some text response'), - ); - - await client.generateContent(contentOptions); - - expect( - mockAvailabilityService.selectFirstAvailable, - ).not.toHaveBeenCalled(); - expect(mockConfig.setActiveModel).not.toHaveBeenCalled(); - expect(mockAvailabilityService.markHealthy).not.toHaveBeenCalled(); - }); - it('should mark model as healthy on success', async () => { const successfulModel = 'gemini-pro'; vi.mocked(mockAvailabilityService.selectFirstAvailable).mockReturnValue({ diff --git a/packages/core/src/core/baseLlmClient.ts b/packages/core/src/core/baseLlmClient.ts index 6c0d5b7012..22c406ef0f 100644 --- a/packages/core/src/core/baseLlmClient.ts +++ b/packages/core/src/core/baseLlmClient.ts @@ -264,22 +264,20 @@ export class BaseLlmClient { try { const apiCall = () => { - // If availability is enabled, ensure we use the current active model + // Ensure we use the current active model // in case a fallback occurred in a previous attempt. - if (this.config.isModelAvailabilityServiceEnabled()) { - const activeModel = this.config.getActiveModel(); - if (activeModel !== requestParams.model) { - requestParams.model = activeModel; - // Re-resolve config if model changed during retry - const { generateContentConfig } = - this.config.modelConfigService.getResolvedConfig({ - model: activeModel, - }); - requestParams.config = { - ...requestParams.config, - ...generateContentConfig, - }; - } + const activeModel = this.config.getActiveModel(); + if (activeModel !== requestParams.model) { + requestParams.model = activeModel; + // Re-resolve config if model changed during retry + const { generateContentConfig } = + this.config.modelConfigService.getResolvedConfig({ + model: activeModel, + }); + requestParams.config = { + ...requestParams.config, + ...generateContentConfig, + }; } return this.contentGenerator.generateContent(requestParams, promptId); }; diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index ad874170f6..2f70e03879 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -265,11 +265,12 @@ describe('Gemini Client (client.ts)', () => { }, isInteractive: vi.fn().mockReturnValue(false), getExperiments: () => {}, - isModelAvailabilityServiceEnabled: vi.fn().mockReturnValue(false), getActiveModel: vi.fn().mockReturnValue('test-model'), setActiveModel: vi.fn(), resetTurn: vi.fn(), - getModelAvailabilityService: vi.fn(), + getModelAvailabilityService: vi + .fn() + .mockReturnValue(createAvailabilityServiceMock()), } as unknown as Config; mockConfig.getHookSystem = vi .fn() @@ -2006,9 +2007,6 @@ ${JSON.stringify( vi.mocked(mockConfig.getModelAvailabilityService).mockReturnValue( mockAvailabilityService, ); - vi.mocked(mockConfig.isModelAvailabilityServiceEnabled).mockReturnValue( - true, - ); vi.mocked(mockConfig.setActiveModel).mockClear(); mockRouterService.route.mockResolvedValue({ model: 'model-a', @@ -2611,14 +2609,14 @@ ${JSON.stringify( const abortSignal = new AbortController().signal; await client.generateContent( - { model: DEFAULT_GEMINI_FLASH_MODEL }, + { model: 'test-model' }, contents, abortSignal, ); expect(mockContentGenerator.generateContent).toHaveBeenCalledWith( { - model: DEFAULT_GEMINI_FLASH_MODEL, + model: 'test-model', config: { abortSignal, systemInstruction: getCoreSystemPrompt({} as unknown as Config, ''), @@ -2632,50 +2630,18 @@ ${JSON.stringify( }); it('should use current model from config for content generation', async () => { - const initialModel = client['config'].getModel(); + const initialModel = 'test-model'; const contents = [{ role: 'user', parts: [{ text: 'test' }] }]; - const currentModel = initialModel + '-changed'; - - vi.spyOn(client['config'], 'getModel').mockReturnValueOnce(currentModel); await client.generateContent( - { model: DEFAULT_GEMINI_FLASH_MODEL }, + { model: initialModel }, contents, new AbortController().signal, ); - expect(mockContentGenerator.generateContent).not.toHaveBeenCalledWith({ - model: initialModel, - config: expect.any(Object), - contents, - }); expect(mockContentGenerator.generateContent).toHaveBeenCalledWith( - { - model: DEFAULT_GEMINI_FLASH_MODEL, - config: expect.any(Object), - contents, - }, - 'test-session-id', - ); - }); - - it('should use the Flash model when fallback mode is active', async () => { - const contents = [{ role: 'user', parts: [{ text: 'hello' }] }]; - const abortSignal = new AbortController().signal; - const requestedModel = 'gemini-2.5-pro'; // A non-flash model - - // Mock config to be in fallback mode - vi.spyOn(client['config'], 'isInFallbackMode').mockReturnValue(true); - - await client.generateContent( - { model: requestedModel }, - contents, - abortSignal, - ); - - expect(mockGenerateContentFn).toHaveBeenCalledWith( expect.objectContaining({ - model: DEFAULT_GEMINI_FLASH_MODEL, + model: initialModel, }), 'test-session-id', ); diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index 7e78aef7f0..8d14c8cebd 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -31,7 +31,6 @@ import type { ResumedSessionData, } from '../services/chatRecordingService.js'; import type { ContentGenerator } from './contentGenerator.js'; -import { DEFAULT_GEMINI_FLASH_MODEL } from '../config/models.js'; import { LoopDetectionService } from '../services/loopDetectionService.js'; import { ChatCompressionService } from '../services/chatCompressionService.js'; import { ideContextStore } from '../ide/ideContext.js'; @@ -669,11 +668,6 @@ export class GeminiClient { model: currentAttemptModel, generateContentConfig: currentAttemptGenerateContentConfig, } = desiredModelConfig; - const fallbackModelConfig = - this.config.modelConfigService.getResolvedConfig({ - ...modelConfigKey, - model: DEFAULT_GEMINI_FLASH_MODEL, - }); try { const userMemory = this.config.getUserMemory(); @@ -701,28 +695,16 @@ export class GeminiClient { ); const apiCall = () => { - let modelConfigToUse = desiredModelConfig; - - if (!this.config.isModelAvailabilityServiceEnabled()) { - modelConfigToUse = this.config.isInFallbackMode() - ? fallbackModelConfig - : desiredModelConfig; - currentAttemptModel = modelConfigToUse.model; - currentAttemptGenerateContentConfig = - modelConfigToUse.generateContentConfig; - } else { - // AvailabilityService - const active = this.config.getActiveModel(); - if (active !== currentAttemptModel) { - currentAttemptModel = active; - // Re-resolve config if model changed - const newConfig = this.config.modelConfigService.getResolvedConfig({ - ...modelConfigKey, - model: currentAttemptModel, - }); - currentAttemptGenerateContentConfig = - newConfig.generateContentConfig; - } + // AvailabilityService + const active = this.config.getActiveModel(); + if (active !== currentAttemptModel) { + currentAttemptModel = active; + // Re-resolve config if model changed + const newConfig = this.config.modelConfigService.getResolvedConfig({ + ...modelConfigKey, + model: currentAttemptModel, + }); + currentAttemptGenerateContentConfig = newConfig.generateContentConfig; } const requestConfig: GenerateContentConfig = { diff --git a/packages/core/src/core/geminiChat.test.ts b/packages/core/src/core/geminiChat.test.ts index 98e74631e2..0801c0747b 100644 --- a/packages/core/src/core/geminiChat.test.ts +++ b/packages/core/src/core/geminiChat.test.ts @@ -185,12 +185,13 @@ describe('GeminiChat', () => { setPreviewModelFallbackMode: vi.fn(), isInteractive: vi.fn().mockReturnValue(false), getEnableHooks: vi.fn().mockReturnValue(false), - isModelAvailabilityServiceEnabled: vi.fn().mockReturnValue(false), getActiveModel: vi.fn().mockImplementation(() => currentActiveModel), setActiveModel: vi .fn() .mockImplementation((m: string) => (currentActiveModel = m)), - getModelAvailabilityService: vi.fn(), + getModelAvailabilityService: vi + .fn() + .mockReturnValue(createAvailabilityServiceMock()), } as unknown as Config; // Use proper MessageBus mocking for Phase 3 preparation @@ -2160,9 +2161,6 @@ describe('GeminiChat', () => { vi.mocked(mockConfig.getModelAvailabilityService).mockReturnValue( mockAvailabilityService, ); - vi.mocked(mockConfig.isModelAvailabilityServiceEnabled).mockReturnValue( - true, - ); // Stateful mock for activeModel let activeModel = 'model-a'; diff --git a/packages/core/src/core/geminiChat_network_retry.test.ts b/packages/core/src/core/geminiChat_network_retry.test.ts index 5b96c9d555..712be19b06 100644 --- a/packages/core/src/core/geminiChat_network_retry.test.ts +++ b/packages/core/src/core/geminiChat_network_retry.test.ts @@ -13,6 +13,7 @@ import type { Config } from '../config/config.js'; import { setSimulate429 } from '../utils/testUtils.js'; import { HookSystem } from '../hooks/hookSystem.js'; import { createMockMessageBus } from '../test-utils/mock-message-bus.js'; +import { createAvailabilityServiceMock } from '../availability/testUtils.js'; // Mock fs module vi.mock('node:fs', () => ({ @@ -95,12 +96,14 @@ describe('GeminiChat Network Retries', () => { generateContentConfig: { temperature: 0 }, })), }, - isModelAvailabilityServiceEnabled: vi.fn().mockReturnValue(false), isPreviewModelBypassMode: vi.fn().mockReturnValue(false), setPreviewModelBypassMode: vi.fn(), isPreviewModelFallbackMode: vi.fn().mockReturnValue(false), getEnableHooks: vi.fn().mockReturnValue(false), setPreviewModelFallbackMode: vi.fn(), + getModelAvailabilityService: vi + .fn() + .mockReturnValue(createAvailabilityServiceMock()), } as unknown as Config; const mockMessageBus = createMockMessageBus(); diff --git a/packages/core/src/fallback/handler.test.ts b/packages/core/src/fallback/handler.test.ts index 488006c1da..9e62b8bb11 100644 --- a/packages/core/src/fallback/handler.test.ts +++ b/packages/core/src/fallback/handler.test.ts @@ -65,7 +65,6 @@ const createMockConfig = (overrides: Partial = {}): Config => ({ isInFallbackMode: vi.fn(() => false), setFallbackMode: vi.fn(), - isModelAvailabilityServiceEnabled: vi.fn(() => true), isPreviewModelFallbackMode: vi.fn(() => false), setPreviewModelFallbackMode: vi.fn(), isPreviewModelBypassMode: vi.fn(() => false), @@ -130,9 +129,6 @@ describe('handleFallback', () => { policyConfig = createMockConfig(); // Ensure we test the availability path - vi.mocked(policyConfig.isModelAvailabilityServiceEnabled).mockReturnValue( - true, - ); vi.mocked(policyConfig.getModelAvailabilityService).mockReturnValue( availability, ); diff --git a/packages/core/src/routing/strategies/fallbackStrategy.test.ts b/packages/core/src/routing/strategies/fallbackStrategy.test.ts index c9a0aa2179..6196e59526 100644 --- a/packages/core/src/routing/strategies/fallbackStrategy.test.ts +++ b/packages/core/src/routing/strategies/fallbackStrategy.test.ts @@ -23,7 +23,6 @@ vi.mock('../../availability/policyHelpers.js', () => ({ const createMockConfig = (overrides: Partial = {}): Config => ({ - isModelAvailabilityServiceEnabled: vi.fn().mockReturnValue(true), getModelAvailabilityService: vi.fn(), getModel: vi.fn().mockReturnValue(DEFAULT_GEMINI_MODEL), getPreviewFeatures: vi.fn().mockReturnValue(false), @@ -49,15 +48,6 @@ describe('FallbackStrategy', () => { }); }); - it('should return null if service is disabled', async () => { - vi.mocked(mockConfig.isModelAvailabilityServiceEnabled).mockReturnValue( - false, - ); - - const decision = await strategy.route(mockContext, mockConfig, mockClient); - expect(decision).toBeNull(); - }); - it('should return null if the requested model is available', async () => { // Mock snapshot to return available vi.mocked(mockService.snapshot).mockReturnValue({ available: true }); diff --git a/packages/core/src/routing/strategies/fallbackStrategy.ts b/packages/core/src/routing/strategies/fallbackStrategy.ts index 1f89b2caa3..dbaa484094 100644 --- a/packages/core/src/routing/strategies/fallbackStrategy.ts +++ b/packages/core/src/routing/strategies/fallbackStrategy.ts @@ -22,10 +22,6 @@ export class FallbackStrategy implements RoutingStrategy { config: Config, _baseLlmClient: BaseLlmClient, ): Promise { - if (!config.isModelAvailabilityServiceEnabled()) { - return null; - } - const requestedModel = config.getModel(); const resolvedModel = resolveModel( requestedModel, diff --git a/schemas/settings.schema.json b/schemas/settings.schema.json index 7570abe6b8..58ad1b78f9 100644 --- a/schemas/settings.schema.json +++ b/schemas/settings.schema.json @@ -1322,13 +1322,6 @@ "default": false, "type": "boolean" }, - "isModelAvailabilityServiceEnabled": { - "title": "Enable Model Availability Service", - "description": "Enable model routing using new availability service.", - "markdownDescription": "Enable model routing using new availability service.\n\n- Category: `Experimental`\n- Requires restart: `yes`\n- Default: `false`", - "default": false, - "type": "boolean" - }, "jitContext": { "title": "JIT Context Loading", "description": "Enable Just-In-Time (JIT) context loading.",