mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
fix: improve OpenRouter model cache persistence to prevent model disappearance
- Add validation to check if API response contains enough models - Implement intelligent model merging to preserve user-selected models - Preserve models specified in preserveModelIds even if not in API response - Add fallback to disk cache when API returns incomplete data - Update tests to verify cache validation and merging logic This fixes the issue where newer OpenRouter models like gpt-5.1, gemini-3-pro-preview, and grok-4.1-fast would disappear after the 5-minute cache expiration when the API returned incomplete or temporarily missing model data. Fixes #9597
This commit is contained in:
parent
4442397507
commit
1f900cdcb1
3 changed files with 272 additions and 21 deletions
|
|
@ -28,6 +28,16 @@ vi.mock("fs", () => ({
|
|||
readFileSync: vi.fn().mockReturnValue("{}"),
|
||||
}))
|
||||
|
||||
// Mock safeWriteJson
|
||||
vi.mock("../../../../utils/safeWriteJson", () => ({
|
||||
safeWriteJson: vi.fn().mockResolvedValue(undefined),
|
||||
}))
|
||||
|
||||
// Mock fileExistsAtPath
|
||||
vi.mock("../../../../utils/fs", () => ({
|
||||
fileExistsAtPath: vi.fn().mockResolvedValue(false),
|
||||
}))
|
||||
|
||||
// Mock all the model fetchers
|
||||
vi.mock("../litellm")
|
||||
vi.mock("../openrouter")
|
||||
|
|
@ -35,9 +45,16 @@ vi.mock("../requesty")
|
|||
vi.mock("../glama")
|
||||
vi.mock("../unbound")
|
||||
vi.mock("../io-intelligence")
|
||||
vi.mock("../ollama")
|
||||
vi.mock("../lmstudio")
|
||||
vi.mock("../vercel-ai-gateway")
|
||||
vi.mock("../deepinfra")
|
||||
vi.mock("../huggingface")
|
||||
vi.mock("../roo")
|
||||
vi.mock("../chutes")
|
||||
|
||||
// Mock ContextProxy with a simple static instance
|
||||
vi.mock("../../../core/config/ContextProxy", () => ({
|
||||
vi.mock("../../../../core/config/ContextProxy", () => ({
|
||||
ContextProxy: {
|
||||
instance: {
|
||||
globalStorageUri: {
|
||||
|
|
@ -47,17 +64,24 @@ vi.mock("../../../core/config/ContextProxy", () => ({
|
|||
},
|
||||
}))
|
||||
|
||||
// Mock getCacheDirectoryPath
|
||||
vi.mock("../../../../utils/storage", () => ({
|
||||
getCacheDirectoryPath: vi.fn().mockResolvedValue("/mock/storage/path/cache"),
|
||||
}))
|
||||
|
||||
// Then imports
|
||||
import type { Mock } from "vitest"
|
||||
import * as fsSync from "fs"
|
||||
import * as fs from "fs/promises"
|
||||
import NodeCache from "node-cache"
|
||||
import { getModels, getModelsFromCache } from "../modelCache"
|
||||
import { getModels, getModelsFromCache, refreshModels } from "../modelCache"
|
||||
import { getLiteLLMModels } from "../litellm"
|
||||
import { getOpenRouterModels } from "../openrouter"
|
||||
import { getRequestyModels } from "../requesty"
|
||||
import { getGlamaModels } from "../glama"
|
||||
import { getUnboundModels } from "../unbound"
|
||||
import { getIOIntelligenceModels } from "../io-intelligence"
|
||||
import { safeWriteJson } from "../../../../utils/safeWriteJson"
|
||||
|
||||
const mockGetLiteLLMModels = getLiteLLMModels as Mock<typeof getLiteLLMModels>
|
||||
const mockGetOpenRouterModels = getOpenRouterModels as Mock<typeof getOpenRouterModels>
|
||||
|
|
@ -266,9 +290,8 @@ describe("getModelsFromCache disk fallback", () => {
|
|||
|
||||
const result = getModelsFromCache("openrouter")
|
||||
|
||||
// In the test environment, ContextProxy.instance may not be fully initialized,
|
||||
// so getCacheDirectoryPathSync returns undefined and disk cache is not attempted
|
||||
expect(result).toBeUndefined()
|
||||
// Now that getCacheDirectoryPathSync returns a path, disk cache should work
|
||||
expect(result).toEqual(diskModels)
|
||||
})
|
||||
|
||||
it("handles disk read errors gracefully", () => {
|
||||
|
|
@ -301,3 +324,132 @@ describe("getModelsFromCache disk fallback", () => {
|
|||
consoleErrorSpy.mockRestore()
|
||||
})
|
||||
})
|
||||
|
||||
describe("OpenRouter model cache validation and merging", () => {
|
||||
let mockCache: any
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
// Get the mock cache instance
|
||||
const MockedNodeCache = vi.mocked(NodeCache)
|
||||
mockCache = new MockedNodeCache()
|
||||
// Reset memory cache to always miss
|
||||
mockCache.get.mockReturnValue(undefined)
|
||||
// Mock safeWriteJson to avoid file system operations
|
||||
vi.mocked(safeWriteJson).mockResolvedValue(undefined)
|
||||
})
|
||||
|
||||
it("uses full API response when it contains enough models", async () => {
|
||||
// API returns complete response with many models
|
||||
const completeApiResponse = {
|
||||
...Array.from({ length: 120 }, (_, i) => ({
|
||||
[`model-${i}`]: {
|
||||
maxTokens: 4096,
|
||||
contextWindow: 8192,
|
||||
supportsPromptCache: false,
|
||||
description: `Model ${i}`,
|
||||
},
|
||||
})).reduce((acc, curr) => ({ ...acc, ...curr }), {}),
|
||||
}
|
||||
|
||||
mockGetOpenRouterModels.mockResolvedValue(completeApiResponse)
|
||||
|
||||
const result = await getModels({
|
||||
provider: "openrouter",
|
||||
})
|
||||
|
||||
// Should use the full API response
|
||||
expect(Object.keys(result).length).toBe(120)
|
||||
expect(result["model-0"]).toBeDefined()
|
||||
expect(result["model-119"]).toBeDefined()
|
||||
})
|
||||
|
||||
it("refreshModels preserves models even during refresh", async () => {
|
||||
// Set up existing cache in memory
|
||||
const existingModels = {
|
||||
"openai/gpt-5.1": {
|
||||
maxTokens: 8192,
|
||||
contextWindow: 128000,
|
||||
supportsPromptCache: true,
|
||||
description: "GPT-5.1 model",
|
||||
},
|
||||
"model-1": {
|
||||
maxTokens: 4096,
|
||||
contextWindow: 8192,
|
||||
supportsPromptCache: false,
|
||||
description: "Model 1",
|
||||
},
|
||||
}
|
||||
|
||||
// Configure memory cache to return existing models
|
||||
mockCache.get.mockReturnValue(existingModels)
|
||||
|
||||
// API returns incomplete response
|
||||
const incompleteApiResponse = {
|
||||
"model-1": {
|
||||
maxTokens: 4096,
|
||||
contextWindow: 8192,
|
||||
supportsPromptCache: false,
|
||||
description: "Model 1 updated",
|
||||
},
|
||||
// Missing gpt-5.1
|
||||
}
|
||||
|
||||
mockGetOpenRouterModels.mockResolvedValue(incompleteApiResponse)
|
||||
|
||||
const result = await refreshModels({
|
||||
provider: "openrouter",
|
||||
preserveModelIds: new Set(["openai/gpt-5.1"]),
|
||||
})
|
||||
|
||||
// Should preserve gpt-5.1 even though it's not in API response
|
||||
expect(result["openai/gpt-5.1"]).toEqual(existingModels["openai/gpt-5.1"])
|
||||
// Should update model-1 with new data
|
||||
expect(result["model-1"].description).toBe("Model 1 updated")
|
||||
})
|
||||
|
||||
it("validates model count threshold for OpenRouter", async () => {
|
||||
const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {})
|
||||
const consoleDebugSpy = vi.spyOn(console, "debug").mockImplementation(() => {})
|
||||
|
||||
// Existing cache with many models
|
||||
const existingModels = {
|
||||
...Array.from({ length: 100 }, (_, i) => ({
|
||||
[`existing-model-${i}`]: {
|
||||
maxTokens: 4096,
|
||||
contextWindow: 8192,
|
||||
supportsPromptCache: false,
|
||||
description: `Existing Model ${i}`,
|
||||
},
|
||||
})).reduce((acc, curr) => ({ ...acc, ...curr }), {}),
|
||||
}
|
||||
|
||||
vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(existingModels))
|
||||
|
||||
// API returns too few models (below threshold)
|
||||
const tooFewModels = {
|
||||
"model-1": {
|
||||
maxTokens: 4096,
|
||||
contextWindow: 8192,
|
||||
supportsPromptCache: false,
|
||||
description: "Model 1",
|
||||
},
|
||||
"model-2": {
|
||||
maxTokens: 4096,
|
||||
contextWindow: 8192,
|
||||
supportsPromptCache: false,
|
||||
description: "Model 2",
|
||||
},
|
||||
}
|
||||
|
||||
mockGetOpenRouterModels.mockResolvedValue(tooFewModels)
|
||||
|
||||
await getModels({ provider: "openrouter" })
|
||||
|
||||
// Should log warning about incomplete response
|
||||
expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("OpenRouter returned only 2 models"))
|
||||
|
||||
consoleWarnSpy.mockRestore()
|
||||
consoleDebugSpy.mockRestore()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -117,6 +117,49 @@ async function fetchModelsFromProvider(options: GetModelsOptions): Promise<Model
|
|||
return models
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate that fetched models contain expected models.
|
||||
* Returns true if the fetched models appear to be complete.
|
||||
*/
|
||||
function validateFetchedModels(provider: RouterName, fetchedModels: ModelRecord): boolean {
|
||||
// For OpenRouter, ensure we have a reasonable minimum number of models
|
||||
// to avoid replacing a full cache with an incomplete response
|
||||
if (provider === "openrouter") {
|
||||
const modelCount = Object.keys(fetchedModels).length
|
||||
// OpenRouter typically has 100+ models, so if we get less than 50,
|
||||
// something might be wrong with the API response
|
||||
if (modelCount < 50) {
|
||||
console.warn(
|
||||
`[MODEL_CACHE] OpenRouter returned only ${modelCount} models, which seems incomplete. Keeping existing cache.`,
|
||||
)
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
/**
|
||||
* Merge new models with existing cache, preserving user-selected models.
|
||||
* This prevents losing models that are in use but temporarily missing from API.
|
||||
*/
|
||||
function mergeModels(existingModels: ModelRecord, newModels: ModelRecord, preserveKeys?: Set<string>): ModelRecord {
|
||||
const merged = { ...newModels }
|
||||
|
||||
// Preserve specific models if they're in the preserve list
|
||||
if (preserveKeys && preserveKeys.size > 0) {
|
||||
for (const key of preserveKeys) {
|
||||
if (existingModels[key] && !merged[key]) {
|
||||
// Keep the existing model if it's not in the new set
|
||||
merged[key] = existingModels[key]
|
||||
console.debug(`[MODEL_CACHE] Preserved model ${key} from existing cache`)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return merged
|
||||
}
|
||||
|
||||
/**
|
||||
* Get models from the cache or fetch them from the provider and cache them.
|
||||
* There are two caches:
|
||||
|
|
@ -126,10 +169,13 @@ async function fetchModelsFromProvider(options: GetModelsOptions): Promise<Model
|
|||
* @param router - The router to fetch models from.
|
||||
* @param apiKey - Optional API key for the provider.
|
||||
* @param baseUrl - Optional base URL for the provider (currently used only for LiteLLM).
|
||||
* @param preserveModelIds - Optional set of model IDs to preserve even if not in API response
|
||||
* @returns The models from the cache or the fetched models.
|
||||
*/
|
||||
export const getModels = async (options: GetModelsOptions): Promise<ModelRecord> => {
|
||||
const { provider } = options
|
||||
export const getModels = async (
|
||||
options: GetModelsOptions & { preserveModelIds?: Set<string> },
|
||||
): Promise<ModelRecord> => {
|
||||
const { provider, preserveModelIds } = options
|
||||
|
||||
let models = getModelsFromCache(provider)
|
||||
|
||||
|
|
@ -138,26 +184,45 @@ export const getModels = async (options: GetModelsOptions): Promise<ModelRecord>
|
|||
}
|
||||
|
||||
try {
|
||||
models = await fetchModelsFromProvider(options)
|
||||
const fetchedModels = await fetchModelsFromProvider(options)
|
||||
|
||||
// Cache the fetched models (even if empty, to signify a successful fetch with no models).
|
||||
// Get existing cache for merging
|
||||
const existingModels = await readModels(provider).catch(() => undefined)
|
||||
|
||||
// Validate the fetched models
|
||||
const isValid = validateFetchedModels(provider, fetchedModels)
|
||||
|
||||
if (!isValid && existingModels) {
|
||||
// If validation fails and we have existing models, merge carefully
|
||||
models = mergeModels(existingModels, fetchedModels, preserveModelIds)
|
||||
} else if (existingModels && preserveModelIds && preserveModelIds.size > 0) {
|
||||
// Even if valid, preserve specific models if requested
|
||||
models = mergeModels(existingModels, fetchedModels, preserveModelIds)
|
||||
} else {
|
||||
models = fetchedModels
|
||||
}
|
||||
|
||||
// Cache the merged models
|
||||
memoryCache.set(provider, models)
|
||||
|
||||
await writeModels(provider, models).catch((err) =>
|
||||
console.error(`[MODEL_CACHE] Error writing ${provider} models to file cache:`, err),
|
||||
)
|
||||
|
||||
try {
|
||||
models = await readModels(provider)
|
||||
} catch (error) {
|
||||
console.error(`[getModels] error reading ${provider} models from file cache`, error)
|
||||
}
|
||||
return models || {}
|
||||
} catch (error) {
|
||||
// Log the error and re-throw it so the caller can handle it (e.g., show a UI message).
|
||||
// On error, try to use existing cache as fallback
|
||||
console.error(`[getModels] Failed to fetch models in modelCache for ${provider}:`, error)
|
||||
|
||||
throw error // Re-throw the original error to be handled by the caller.
|
||||
// Try to load from disk cache as fallback
|
||||
const diskCache = await readModels(provider).catch(() => undefined)
|
||||
if (diskCache) {
|
||||
console.debug(`[MODEL_CACHE] Using disk cache as fallback for ${provider}`)
|
||||
memoryCache.set(provider, diskCache)
|
||||
return diskCache
|
||||
}
|
||||
|
||||
throw error // Re-throw the original error if no fallback available
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -168,12 +233,32 @@ export const getModels = async (options: GetModelsOptions): Promise<ModelRecord>
|
|||
* @param options - Provider options for fetching models
|
||||
* @returns Fresh models from API
|
||||
*/
|
||||
export const refreshModels = async (options: GetModelsOptions): Promise<ModelRecord> => {
|
||||
const { provider } = options
|
||||
export const refreshModels = async (
|
||||
options: GetModelsOptions & { preserveModelIds?: Set<string> },
|
||||
): Promise<ModelRecord> => {
|
||||
const { provider, preserveModelIds } = options
|
||||
|
||||
try {
|
||||
// Force fresh API fetch - skip getModelsFromCache() check
|
||||
const models = await fetchModelsFromProvider(options)
|
||||
const fetchedModels = await fetchModelsFromProvider(options)
|
||||
|
||||
// Get existing models for intelligent merging
|
||||
const existingModels = getModelsFromCache(provider)
|
||||
|
||||
// Validate the fetched models
|
||||
const isValid = validateFetchedModels(provider, fetchedModels)
|
||||
|
||||
let models: ModelRecord
|
||||
if (!isValid && existingModels) {
|
||||
// If validation fails, merge with existing to preserve user models
|
||||
models = mergeModels(existingModels, fetchedModels, preserveModelIds)
|
||||
console.debug(`[refreshModels] Merged ${provider} models due to incomplete API response`)
|
||||
} else if (existingModels && preserveModelIds && preserveModelIds.size > 0) {
|
||||
// Preserve specific models even in valid responses
|
||||
models = mergeModels(existingModels, fetchedModels, preserveModelIds)
|
||||
} else {
|
||||
models = fetchedModels
|
||||
}
|
||||
|
||||
// Update memory cache first
|
||||
memoryCache.set(provider, models)
|
||||
|
|
|
|||
|
|
@ -82,8 +82,16 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
|
|||
|
||||
private async loadDynamicModels(): Promise<void> {
|
||||
try {
|
||||
// Preserve the current model ID even if it's not in the API response
|
||||
const preserveModelIds = this.options.openRouterModelId
|
||||
? new Set([this.options.openRouterModelId])
|
||||
: undefined
|
||||
|
||||
const [models, endpoints] = await Promise.all([
|
||||
getModels({ provider: "openrouter" }),
|
||||
getModels({
|
||||
provider: "openrouter",
|
||||
preserveModelIds,
|
||||
}),
|
||||
getModelEndpoints({
|
||||
router: "openrouter",
|
||||
modelId: this.options.openRouterModelId,
|
||||
|
|
@ -359,8 +367,14 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH
|
|||
}
|
||||
|
||||
public async fetchModel() {
|
||||
// Preserve the current model ID even if it's not in the API response
|
||||
const preserveModelIds = this.options.openRouterModelId ? new Set([this.options.openRouterModelId]) : undefined
|
||||
|
||||
const [models, endpoints] = await Promise.all([
|
||||
getModels({ provider: "openrouter" }),
|
||||
getModels({
|
||||
provider: "openrouter",
|
||||
preserveModelIds,
|
||||
}),
|
||||
getModelEndpoints({
|
||||
router: "openrouter",
|
||||
modelId: this.options.openRouterModelId,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue