chore(core): remove redundant isModelAvailabilityServiceEnabled toggle and clean up dead code (#15207)

This commit is contained in:
Adam Weidman
2025-12-17 14:23:56 -05:00
committed by GitHub
parent 948401a450
commit b465e12747
17 changed files with 42 additions and 195 deletions
-5
View File
@@ -798,11 +798,6 @@ their corresponding top-level category object in your `settings.json` file.
- **Default:** `false` - **Default:** `false`
- **Requires restart:** Yes - **Requires restart:** Yes
- **`experimental.isModelAvailabilityServiceEnabled`** (boolean):
- **Description:** Enable model routing using new availability service.
- **Default:** `false`
- **Requires restart:** Yes
- **`experimental.jitContext`** (boolean): - **`experimental.jitContext`** (boolean):
- **Description:** Enable Just-In-Time (JIT) context loading. - **Description:** Enable Just-In-Time (JIT) context loading.
- **Default:** `false` - **Default:** `false`
-2
View File
@@ -643,8 +643,6 @@ export async function loadCliConfig(
extensionLoader: extensionManager, extensionLoader: extensionManager,
enableExtensionReloading: settings.experimental?.extensionReloading, enableExtensionReloading: settings.experimental?.extensionReloading,
enableAgents: settings.experimental?.enableAgents, enableAgents: settings.experimental?.enableAgents,
enableModelAvailabilityService:
settings.experimental?.isModelAvailabilityServiceEnabled,
experimentalJitContext: settings.experimental?.jitContext, experimentalJitContext: settings.experimental?.jitContext,
noBrowser: !!process.env['NO_BROWSER'], noBrowser: !!process.env['NO_BROWSER'],
summarizeToolOutput: settings.model?.summarizeToolOutput, summarizeToolOutput: settings.model?.summarizeToolOutput,
@@ -354,21 +354,6 @@ describe('SettingsSchema', () => {
expect(setting.showInDialog).toBe(false); expect(setting.showInDialog).toBe(false);
expect(setting.description).toBe('Enable local and remote subagents.'); 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', () => { it('has JSON schema definitions for every referenced ref', () => {
@@ -1323,15 +1323,6 @@ const SETTINGS_SCHEMA = {
'Enables extension loading/unloading within the CLI session.', 'Enables extension loading/unloading within the CLI session.',
showInDialog: false, 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: { jitContext: {
type: 'boolean', type: 'boolean',
label: 'JIT Context Loading', label: 'JIT Context Loading',
@@ -119,7 +119,6 @@ describe('policyHelpers', () => {
overrides: Partial<Config> = {}, overrides: Partial<Config> = {},
): Config => { ): Config => {
const defaults = { const defaults = {
isModelAvailabilityServiceEnabled: () => true,
getModelAvailabilityService: () => mockAvailabilityService, getModelAvailabilityService: () => mockAvailabilityService,
setActiveModel: vi.fn(), setActiveModel: vi.fn(),
modelConfigService: mockModelConfigService, modelConfigService: mockModelConfigService,
@@ -131,15 +130,6 @@ describe('policyHelpers', () => {
vi.clearAllMocks(); 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', () => { it('returns requested model if it is available', () => {
const config = createExtendedMockConfig(); const config = createExtendedMockConfig();
mockAvailabilityService.selectFirstAvailable.mockReturnValue({ mockAvailabilityService.selectFirstAvailable.mockReturnValue({
@@ -117,9 +117,6 @@ export function createAvailabilityContextProvider(
modelGetter: () => string, modelGetter: () => string,
): () => RetryAvailabilityContext | undefined { ): () => RetryAvailabilityContext | undefined {
return () => { return () => {
if (!config.isModelAvailabilityServiceEnabled()) {
return undefined;
}
const service = config.getModelAvailabilityService(); const service = config.getModelAvailabilityService();
const currentModel = modelGetter(); const currentModel = modelGetter();
@@ -138,11 +135,7 @@ export function createAvailabilityContextProvider(
export function selectModelForAvailability( export function selectModelForAvailability(
config: Config, config: Config,
requestedModel: string, requestedModel: string,
): ModelSelectionResult | undefined { ): ModelSelectionResult {
if (!config.isModelAvailabilityServiceEnabled()) {
return undefined;
}
const chain = resolvePolicyChain(config, requestedModel); const chain = resolvePolicyChain(config, requestedModel);
const selection = config const selection = config
.getModelAvailabilityService() .getModelAvailabilityService()
-7
View File
@@ -326,7 +326,6 @@ export interface ConfigParameters {
} & { disabled?: string[] }); } & { disabled?: string[] });
previewFeatures?: boolean; previewFeatures?: boolean;
enableAgents?: boolean; enableAgents?: boolean;
enableModelAvailabilityService?: boolean;
experimentalJitContext?: boolean; experimentalJitContext?: boolean;
} }
@@ -449,7 +448,6 @@ export class Config {
private previewModelFallbackMode = false; private previewModelFallbackMode = false;
private previewModelBypassMode = false; private previewModelBypassMode = false;
private readonly enableModelAvailabilityService: boolean;
private readonly enableAgents: boolean; private readonly enableAgents: boolean;
private readonly experimentalJitContext: boolean; private readonly experimentalJitContext: boolean;
@@ -513,7 +511,6 @@ export class Config {
this.bugCommand = params.bugCommand; this.bugCommand = params.bugCommand;
this.model = params.model; this.model = params.model;
this._activeModel = params.model; this._activeModel = params.model;
this.enableModelAvailabilityService = true;
this.enableAgents = params.enableAgents ?? false; this.enableAgents = params.enableAgents ?? false;
this.experimentalJitContext = params.experimentalJitContext ?? false; this.experimentalJitContext = params.experimentalJitContext ?? false;
this.modelAvailabilityService = new ModelAvailabilityService(); this.modelAvailabilityService = new ModelAvailabilityService();
@@ -1309,10 +1306,6 @@ export class Config {
return this.enableExtensionReloading; return this.enableExtensionReloading;
} }
isModelAvailabilityServiceEnabled(): boolean {
return this.enableModelAvailabilityService;
}
isAgentsEnabled(): boolean { isAgentsEnabled(): boolean {
return this.enableAgents; return this.enableAgents;
} }
+3 -23
View File
@@ -111,8 +111,9 @@ describe('BaseLlmClient', () => {
.fn() .fn()
.mockImplementation(({ model }) => makeResolvedModelConfig(model)), .mockImplementation(({ model }) => makeResolvedModelConfig(model)),
} as unknown as ModelConfigService, } as unknown as ModelConfigService,
isModelAvailabilityServiceEnabled: vi.fn().mockReturnValue(false), getModelAvailabilityService: vi
getModelAvailabilityService: vi.fn(), .fn()
.mockReturnValue(createAvailabilityServiceMock()),
setActiveModel: vi.fn(), setActiveModel: vi.fn(),
getPreviewFeatures: vi.fn().mockReturnValue(false), getPreviewFeatures: vi.fn().mockReturnValue(false),
getUserTier: vi.fn().mockReturnValue(undefined), getUserTier: vi.fn().mockReturnValue(undefined),
@@ -614,10 +615,6 @@ describe('BaseLlmClient', () => {
let jsonOptions: GenerateJsonOptions; let jsonOptions: GenerateJsonOptions;
beforeEach(() => { beforeEach(() => {
mockConfig.isModelAvailabilityServiceEnabled = vi
.fn()
.mockReturnValue(true);
mockAvailabilityService = createAvailabilityServiceMock({ mockAvailabilityService = createAvailabilityServiceMock({
selectedModel: 'test-model', selectedModel: 'test-model',
skipped: [], 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 () => { it('should mark model as healthy on success', async () => {
const successfulModel = 'gemini-pro'; const successfulModel = 'gemini-pro';
vi.mocked(mockAvailabilityService.selectFirstAvailable).mockReturnValue({ vi.mocked(mockAvailabilityService.selectFirstAvailable).mockReturnValue({
+1 -3
View File
@@ -264,9 +264,8 @@ export class BaseLlmClient {
try { try {
const apiCall = () => { 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. // in case a fallback occurred in a previous attempt.
if (this.config.isModelAvailabilityServiceEnabled()) {
const activeModel = this.config.getActiveModel(); const activeModel = this.config.getActiveModel();
if (activeModel !== requestParams.model) { if (activeModel !== requestParams.model) {
requestParams.model = activeModel; requestParams.model = activeModel;
@@ -280,7 +279,6 @@ export class BaseLlmClient {
...generateContentConfig, ...generateContentConfig,
}; };
} }
}
return this.contentGenerator.generateContent(requestParams, promptId); return this.contentGenerator.generateContent(requestParams, promptId);
}; };
+8 -42
View File
@@ -265,11 +265,12 @@ describe('Gemini Client (client.ts)', () => {
}, },
isInteractive: vi.fn().mockReturnValue(false), isInteractive: vi.fn().mockReturnValue(false),
getExperiments: () => {}, getExperiments: () => {},
isModelAvailabilityServiceEnabled: vi.fn().mockReturnValue(false),
getActiveModel: vi.fn().mockReturnValue('test-model'), getActiveModel: vi.fn().mockReturnValue('test-model'),
setActiveModel: vi.fn(), setActiveModel: vi.fn(),
resetTurn: vi.fn(), resetTurn: vi.fn(),
getModelAvailabilityService: vi.fn(), getModelAvailabilityService: vi
.fn()
.mockReturnValue(createAvailabilityServiceMock()),
} as unknown as Config; } as unknown as Config;
mockConfig.getHookSystem = vi mockConfig.getHookSystem = vi
.fn() .fn()
@@ -2006,9 +2007,6 @@ ${JSON.stringify(
vi.mocked(mockConfig.getModelAvailabilityService).mockReturnValue( vi.mocked(mockConfig.getModelAvailabilityService).mockReturnValue(
mockAvailabilityService, mockAvailabilityService,
); );
vi.mocked(mockConfig.isModelAvailabilityServiceEnabled).mockReturnValue(
true,
);
vi.mocked(mockConfig.setActiveModel).mockClear(); vi.mocked(mockConfig.setActiveModel).mockClear();
mockRouterService.route.mockResolvedValue({ mockRouterService.route.mockResolvedValue({
model: 'model-a', model: 'model-a',
@@ -2611,14 +2609,14 @@ ${JSON.stringify(
const abortSignal = new AbortController().signal; const abortSignal = new AbortController().signal;
await client.generateContent( await client.generateContent(
{ model: DEFAULT_GEMINI_FLASH_MODEL }, { model: 'test-model' },
contents, contents,
abortSignal, abortSignal,
); );
expect(mockContentGenerator.generateContent).toHaveBeenCalledWith( expect(mockContentGenerator.generateContent).toHaveBeenCalledWith(
{ {
model: DEFAULT_GEMINI_FLASH_MODEL, model: 'test-model',
config: { config: {
abortSignal, abortSignal,
systemInstruction: getCoreSystemPrompt({} as unknown as Config, ''), systemInstruction: getCoreSystemPrompt({} as unknown as Config, ''),
@@ -2632,50 +2630,18 @@ ${JSON.stringify(
}); });
it('should use current model from config for content generation', async () => { 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 contents = [{ role: 'user', parts: [{ text: 'test' }] }];
const currentModel = initialModel + '-changed';
vi.spyOn(client['config'], 'getModel').mockReturnValueOnce(currentModel);
await client.generateContent( await client.generateContent(
{ model: DEFAULT_GEMINI_FLASH_MODEL }, { model: initialModel },
contents, contents,
new AbortController().signal, new AbortController().signal,
); );
expect(mockContentGenerator.generateContent).not.toHaveBeenCalledWith({
model: initialModel,
config: expect.any(Object),
contents,
});
expect(mockContentGenerator.generateContent).toHaveBeenCalledWith( 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({ expect.objectContaining({
model: DEFAULT_GEMINI_FLASH_MODEL, model: initialModel,
}), }),
'test-session-id', 'test-session-id',
); );
+1 -19
View File
@@ -31,7 +31,6 @@ import type {
ResumedSessionData, ResumedSessionData,
} from '../services/chatRecordingService.js'; } from '../services/chatRecordingService.js';
import type { ContentGenerator } from './contentGenerator.js'; import type { ContentGenerator } from './contentGenerator.js';
import { DEFAULT_GEMINI_FLASH_MODEL } from '../config/models.js';
import { LoopDetectionService } from '../services/loopDetectionService.js'; import { LoopDetectionService } from '../services/loopDetectionService.js';
import { ChatCompressionService } from '../services/chatCompressionService.js'; import { ChatCompressionService } from '../services/chatCompressionService.js';
import { ideContextStore } from '../ide/ideContext.js'; import { ideContextStore } from '../ide/ideContext.js';
@@ -669,11 +668,6 @@ export class GeminiClient {
model: currentAttemptModel, model: currentAttemptModel,
generateContentConfig: currentAttemptGenerateContentConfig, generateContentConfig: currentAttemptGenerateContentConfig,
} = desiredModelConfig; } = desiredModelConfig;
const fallbackModelConfig =
this.config.modelConfigService.getResolvedConfig({
...modelConfigKey,
model: DEFAULT_GEMINI_FLASH_MODEL,
});
try { try {
const userMemory = this.config.getUserMemory(); const userMemory = this.config.getUserMemory();
@@ -701,16 +695,6 @@ export class GeminiClient {
); );
const apiCall = () => { const apiCall = () => {
let modelConfigToUse = desiredModelConfig;
if (!this.config.isModelAvailabilityServiceEnabled()) {
modelConfigToUse = this.config.isInFallbackMode()
? fallbackModelConfig
: desiredModelConfig;
currentAttemptModel = modelConfigToUse.model;
currentAttemptGenerateContentConfig =
modelConfigToUse.generateContentConfig;
} else {
// AvailabilityService // AvailabilityService
const active = this.config.getActiveModel(); const active = this.config.getActiveModel();
if (active !== currentAttemptModel) { if (active !== currentAttemptModel) {
@@ -720,9 +704,7 @@ export class GeminiClient {
...modelConfigKey, ...modelConfigKey,
model: currentAttemptModel, model: currentAttemptModel,
}); });
currentAttemptGenerateContentConfig = currentAttemptGenerateContentConfig = newConfig.generateContentConfig;
newConfig.generateContentConfig;
}
} }
const requestConfig: GenerateContentConfig = { const requestConfig: GenerateContentConfig = {
+3 -5
View File
@@ -185,12 +185,13 @@ describe('GeminiChat', () => {
setPreviewModelFallbackMode: vi.fn(), setPreviewModelFallbackMode: vi.fn(),
isInteractive: vi.fn().mockReturnValue(false), isInteractive: vi.fn().mockReturnValue(false),
getEnableHooks: vi.fn().mockReturnValue(false), getEnableHooks: vi.fn().mockReturnValue(false),
isModelAvailabilityServiceEnabled: vi.fn().mockReturnValue(false),
getActiveModel: vi.fn().mockImplementation(() => currentActiveModel), getActiveModel: vi.fn().mockImplementation(() => currentActiveModel),
setActiveModel: vi setActiveModel: vi
.fn() .fn()
.mockImplementation((m: string) => (currentActiveModel = m)), .mockImplementation((m: string) => (currentActiveModel = m)),
getModelAvailabilityService: vi.fn(), getModelAvailabilityService: vi
.fn()
.mockReturnValue(createAvailabilityServiceMock()),
} as unknown as Config; } as unknown as Config;
// Use proper MessageBus mocking for Phase 3 preparation // Use proper MessageBus mocking for Phase 3 preparation
@@ -2160,9 +2161,6 @@ describe('GeminiChat', () => {
vi.mocked(mockConfig.getModelAvailabilityService).mockReturnValue( vi.mocked(mockConfig.getModelAvailabilityService).mockReturnValue(
mockAvailabilityService, mockAvailabilityService,
); );
vi.mocked(mockConfig.isModelAvailabilityServiceEnabled).mockReturnValue(
true,
);
// Stateful mock for activeModel // Stateful mock for activeModel
let activeModel = 'model-a'; let activeModel = 'model-a';
@@ -13,6 +13,7 @@ import type { Config } from '../config/config.js';
import { setSimulate429 } from '../utils/testUtils.js'; import { setSimulate429 } from '../utils/testUtils.js';
import { HookSystem } from '../hooks/hookSystem.js'; import { HookSystem } from '../hooks/hookSystem.js';
import { createMockMessageBus } from '../test-utils/mock-message-bus.js'; import { createMockMessageBus } from '../test-utils/mock-message-bus.js';
import { createAvailabilityServiceMock } from '../availability/testUtils.js';
// Mock fs module // Mock fs module
vi.mock('node:fs', () => ({ vi.mock('node:fs', () => ({
@@ -95,12 +96,14 @@ describe('GeminiChat Network Retries', () => {
generateContentConfig: { temperature: 0 }, generateContentConfig: { temperature: 0 },
})), })),
}, },
isModelAvailabilityServiceEnabled: vi.fn().mockReturnValue(false),
isPreviewModelBypassMode: vi.fn().mockReturnValue(false), isPreviewModelBypassMode: vi.fn().mockReturnValue(false),
setPreviewModelBypassMode: vi.fn(), setPreviewModelBypassMode: vi.fn(),
isPreviewModelFallbackMode: vi.fn().mockReturnValue(false), isPreviewModelFallbackMode: vi.fn().mockReturnValue(false),
getEnableHooks: vi.fn().mockReturnValue(false), getEnableHooks: vi.fn().mockReturnValue(false),
setPreviewModelFallbackMode: vi.fn(), setPreviewModelFallbackMode: vi.fn(),
getModelAvailabilityService: vi
.fn()
.mockReturnValue(createAvailabilityServiceMock()),
} as unknown as Config; } as unknown as Config;
const mockMessageBus = createMockMessageBus(); const mockMessageBus = createMockMessageBus();
@@ -65,7 +65,6 @@ const createMockConfig = (overrides: Partial<Config> = {}): Config =>
({ ({
isInFallbackMode: vi.fn(() => false), isInFallbackMode: vi.fn(() => false),
setFallbackMode: vi.fn(), setFallbackMode: vi.fn(),
isModelAvailabilityServiceEnabled: vi.fn(() => true),
isPreviewModelFallbackMode: vi.fn(() => false), isPreviewModelFallbackMode: vi.fn(() => false),
setPreviewModelFallbackMode: vi.fn(), setPreviewModelFallbackMode: vi.fn(),
isPreviewModelBypassMode: vi.fn(() => false), isPreviewModelBypassMode: vi.fn(() => false),
@@ -130,9 +129,6 @@ describe('handleFallback', () => {
policyConfig = createMockConfig(); policyConfig = createMockConfig();
// Ensure we test the availability path // Ensure we test the availability path
vi.mocked(policyConfig.isModelAvailabilityServiceEnabled).mockReturnValue(
true,
);
vi.mocked(policyConfig.getModelAvailabilityService).mockReturnValue( vi.mocked(policyConfig.getModelAvailabilityService).mockReturnValue(
availability, availability,
); );
@@ -23,7 +23,6 @@ vi.mock('../../availability/policyHelpers.js', () => ({
const createMockConfig = (overrides: Partial<Config> = {}): Config => const createMockConfig = (overrides: Partial<Config> = {}): Config =>
({ ({
isModelAvailabilityServiceEnabled: vi.fn().mockReturnValue(true),
getModelAvailabilityService: vi.fn(), getModelAvailabilityService: vi.fn(),
getModel: vi.fn().mockReturnValue(DEFAULT_GEMINI_MODEL), getModel: vi.fn().mockReturnValue(DEFAULT_GEMINI_MODEL),
getPreviewFeatures: vi.fn().mockReturnValue(false), 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 () => { it('should return null if the requested model is available', async () => {
// Mock snapshot to return available // Mock snapshot to return available
vi.mocked(mockService.snapshot).mockReturnValue({ available: true }); vi.mocked(mockService.snapshot).mockReturnValue({ available: true });
@@ -22,10 +22,6 @@ export class FallbackStrategy implements RoutingStrategy {
config: Config, config: Config,
_baseLlmClient: BaseLlmClient, _baseLlmClient: BaseLlmClient,
): Promise<RoutingDecision | null> { ): Promise<RoutingDecision | null> {
if (!config.isModelAvailabilityServiceEnabled()) {
return null;
}
const requestedModel = config.getModel(); const requestedModel = config.getModel();
const resolvedModel = resolveModel( const resolvedModel = resolveModel(
requestedModel, requestedModel,
-7
View File
@@ -1322,13 +1322,6 @@
"default": false, "default": false,
"type": "boolean" "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": { "jitContext": {
"title": "JIT Context Loading", "title": "JIT Context Loading",
"description": "Enable Just-In-Time (JIT) context loading.", "description": "Enable Just-In-Time (JIT) context loading.",