Skip to content

Commit 926163e

Browse files
committed
feat: distinguish authoritative model lists from cache
1 parent 4c2a1b6 commit 926163e

10 files changed

Lines changed: 206 additions & 47 deletions

File tree

packages/types/src/vscode-extension-host.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,8 @@ export interface ExtensionMessage {
172172
ollamaModels?: ModelRecord
173173
lmStudioModels?: ModelRecord
174174
fullResponseData?: ICostrictModelResponseData[]
175+
/** Whether a costrictModels payload came directly from a successful provider request. */
176+
modelListAuthoritative?: boolean
175177
vsCodeLmModels?: { vendor?: string; family?: string; version?: string; id?: string }[]
176178
mcpServers?: McpServer[]
177179
commits?: GitCommit[]

src/api/providers/fetchers/__tests__/modelCache.spec.ts

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ vi.mock("../../../core/config/ContextProxy", () => ({
5858
import type { Mock } from "vitest"
5959
import * as fsSync from "fs"
6060
import NodeCache from "node-cache"
61-
import { getModels, getModelsFromCache } from "../modelCache"
61+
import { getModels, getModelsFromCache, getModelsWithMetadata, refreshModelsWithMetadata } from "../modelCache"
6262
import { getLiteLLMModels } from "../litellm"
6363
import { getOpenRouterModels } from "../openrouter"
6464
import { getRequestyModels } from "../requesty"
@@ -71,6 +71,78 @@ const mockGetCostrictModels = getCostrictModels as Mock<typeof getCostrictModels
7171

7272
const DUMMY_REQUESTY_KEY = "requesty-key-for-testing"
7373

74+
describe("model list authority metadata", () => {
75+
beforeEach(() => {
76+
vi.clearAllMocks()
77+
const mockCache: any = new NodeCache()
78+
mockCache.get.mockReturnValue(undefined)
79+
vi.mocked(fsSync.existsSync).mockReturnValue(false)
80+
})
81+
82+
afterEach(() => {
83+
const mockCache: any = new NodeCache()
84+
mockCache.get.mockReturnValue(undefined)
85+
})
86+
87+
it("marks a direct provider response as authoritative", async () => {
88+
const models = {
89+
"openrouter/fresh-model": {
90+
maxTokens: 8192,
91+
contextWindow: 128000,
92+
supportsPromptCache: false,
93+
},
94+
}
95+
mockGetOpenRouterModels.mockResolvedValue(models)
96+
97+
await expect(getModelsWithMetadata({ provider: "openrouter" })).resolves.toEqual({
98+
models,
99+
authoritative: true,
100+
})
101+
})
102+
103+
it("marks a cache hit as non-authoritative", async () => {
104+
const models = {
105+
"openrouter/cached-model": {
106+
maxTokens: 8192,
107+
contextWindow: 128000,
108+
supportsPromptCache: false,
109+
},
110+
}
111+
const mockCache: any = new NodeCache()
112+
mockCache.get.mockReturnValue(models)
113+
114+
await expect(getModelsWithMetadata({ provider: "openrouter" })).resolves.toEqual({
115+
models,
116+
authoritative: false,
117+
})
118+
})
119+
120+
it("marks refresh fallback data as non-authoritative", async () => {
121+
const models = {
122+
"costrict/cached-model": {
123+
id: "costrict/cached-model",
124+
maxTokens: 8192,
125+
contextWindow: 128000,
126+
supportsPromptCache: false,
127+
},
128+
}
129+
const mockCache: any = new NodeCache()
130+
mockCache.get.mockReturnValue(models)
131+
mockGetCostrictModels.mockRejectedValue(new Error("API error"))
132+
133+
await expect(
134+
refreshModelsWithMetadata({
135+
provider: "costrict",
136+
baseUrl: "https://api.example.com",
137+
apiKey: "test-api-key",
138+
}),
139+
).resolves.toEqual({
140+
models,
141+
authoritative: false,
142+
})
143+
})
144+
})
145+
74146
describe("getModels with new GetModelsOptions", () => {
75147
beforeEach(() => {
76148
vi.clearAllMocks()

src/api/providers/fetchers/costrict.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import axios from "axios"
22
import { v7 as uuidv7 } from "uuid"
33
import { COSTRICT_DEFAULT_HEADERS } from "../../../shared/headers"
44
import type { InviteCodeInfo, ICostrictModelResponseData, QuotaInfo } from "@roo-code/types"
5-
import { readModels } from "./modelCache"
65
import { CostrictAuthService } from "../../../core/costrict/auth"
76

87
export async function getCostrictModels(
@@ -52,9 +51,7 @@ export async function getCostrictModels(
5251
`Error fetching costrictModels from [${requestId}|${baseUrl}/ai-gateway/api/v1/models]:`,
5352
error.message,
5453
)
55-
const modelCache = (await readModels("costrict")) || {}
56-
57-
return Object.keys(modelCache).map((key) => modelCache[key])
54+
throw error
5855
}
5956
}
6057

src/api/providers/fetchers/modelCache.ts

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,18 @@ const memoryCache = new NodeCache({ stdTTL: 5 * 60, checkperiod: 5 * 60 })
3636
// Zod schema for validating ModelRecord structure from disk cache
3737
const modelRecordSchema = z.record(z.string(), modelInfoSchema)
3838

39+
export interface ModelFetchResult {
40+
models: ModelRecord
41+
/**
42+
* True only when `models` came directly from a successful provider request.
43+
* Memory/disk cache hits and graceful-degradation fallbacks are never authoritative.
44+
*/
45+
authoritative: boolean
46+
}
47+
3948
// Track in-flight refresh requests to prevent concurrent API calls for the same provider
4049
// This prevents race conditions where multiple calls might overwrite each other's results
41-
const inFlightRefresh = new Map<RouterName, Promise<ModelRecord>>()
50+
const inFlightRefresh = new Map<RouterName, Promise<ModelFetchResult>>()
4251

4352
async function writeModels(router: RouterName, data: ModelRecord) {
4453
const filename = `${router}_models.json`
@@ -137,7 +146,7 @@ async function fetchModelsFromProvider(options: GetModelsOptions): Promise<Model
137146
* @param baseUrl - Optional base URL for the provider (currently used only for LiteLLM).
138147
* @returns The models from the cache or the fetched models.
139148
*/
140-
export const getModels = async (options: GetModelsOptions): Promise<ModelRecord> => {
149+
export const getModelsWithMetadata = async (options: GetModelsOptions): Promise<ModelFetchResult> => {
141150
const { provider } = options
142151
const refreshOnDiskCacheHit = "refreshOnDiskCacheHit" in options && options.refreshOnDiskCacheHit
143152
const hadMemoryModels = memoryCache.get<ModelRecord>(provider) != null
@@ -156,7 +165,7 @@ export const getModels = async (options: GetModelsOptions): Promise<ModelRecord>
156165
console.error(`[getModels] Background refresh failed for ${provider}:`, error)
157166
})
158167
}
159-
return models
168+
return { models, authoritative: false }
160169
}
161170
}
162171
models = await fetchModelsFromProvider(options)
@@ -178,7 +187,7 @@ export const getModels = async (options: GetModelsOptions): Promise<ModelRecord>
178187
})
179188
}
180189

181-
return models
190+
return { models, authoritative: true }
182191
} catch (error) {
183192
// Log the error and re-throw it so the caller can handle it (e.g., show a UI message).
184193
console.error(`[getModels] Failed to fetch models in modelCache for ${provider}:`, error)
@@ -187,6 +196,9 @@ export const getModels = async (options: GetModelsOptions): Promise<ModelRecord>
187196
}
188197
}
189198

199+
export const getModels = async (options: GetModelsOptions): Promise<ModelRecord> =>
200+
(await getModelsWithMetadata(options)).models
201+
190202
/**
191203
* Force-refresh models from API, bypassing cache.
192204
* Uses atomic writes so cache remains available during refresh.
@@ -196,7 +208,7 @@ export const getModels = async (options: GetModelsOptions): Promise<ModelRecord>
196208
* @param options - Provider options for fetching models
197209
* @returns Fresh models from API, or existing cache if refresh yields worse data
198210
*/
199-
export const refreshModels = async (options: GetModelsOptions): Promise<ModelRecord> => {
211+
export const refreshModelsWithMetadata = async (options: GetModelsOptions): Promise<ModelFetchResult> => {
200212
const { provider } = options
201213

202214
// Check if there's already an in-flight refresh for this provider
@@ -208,7 +220,7 @@ export const refreshModels = async (options: GetModelsOptions): Promise<ModelRec
208220
}
209221

210222
// Create the refresh promise and track it
211-
const refreshPromise = (async (): Promise<ModelRecord> => {
223+
const refreshPromise = (async (): Promise<ModelFetchResult> => {
212224
try {
213225
// Force fresh API fetch - skip getModelsFromCache() check
214226
const models = await fetchModelsFromProvider(options)
@@ -226,9 +238,9 @@ export const refreshModels = async (options: GetModelsOptions): Promise<ModelRec
226238
existingCacheSize: existingCount,
227239
})
228240
if (existingCount > 0) {
229-
return existingCache!
241+
return { models: existingCache!, authoritative: false }
230242
} else {
231-
return {}
243+
return { models: {}, authoritative: false }
232244
}
233245
}
234246

@@ -240,11 +252,11 @@ export const refreshModels = async (options: GetModelsOptions): Promise<ModelRec
240252
console.error(`[refreshModels] Error writing ${provider} models to disk:`, err),
241253
)
242254

243-
return models
255+
return { models, authoritative: true }
244256
} catch (error) {
245257
// Log the error for debugging, then return existing cache if available (graceful degradation)
246258
console.error(`[refreshModels] Failed to refresh ${provider} models:`, error)
247-
return getModelsFromCache(provider) || {}
259+
return { models: getModelsFromCache(provider) || {}, authoritative: false }
248260
} finally {
249261
// Always clean up the in-flight tracking
250262
inFlightRefresh.delete(provider)
@@ -257,6 +269,9 @@ export const refreshModels = async (options: GetModelsOptions): Promise<ModelRec
257269
return refreshPromise
258270
}
259271

272+
export const refreshModels = async (options: GetModelsOptions): Promise<ModelRecord> =>
273+
(await refreshModelsWithMetadata(options)).models
274+
260275
/**
261276
* Initialize background model cache refresh.
262277
* Refreshes public provider caches without blocking or requiring auth.
@@ -292,16 +307,16 @@ export async function initializeModelCacheRefresh(): Promise<void> {
292307
export const flushModels = async (
293308
options: GetModelsOptions,
294309
refresh: boolean = false,
295-
cb?: (v: any) => void,
310+
cb?: (models: ModelRecord, metadata: ModelFetchResult) => void,
296311
): Promise<void> => {
297312
const { provider } = options
298313
if (refresh) {
299314
// Don't delete memory cache - let refreshModels atomically replace it
300315
// This prevents a race condition where getModels() might be called
301316
// before refresh completes, avoiding a gap in cache availability
302317
// Await the refresh to ensure the cache is updated before returning
303-
await refreshModels(options)
304-
.then(cb)
318+
await refreshModelsWithMetadata(options)
319+
.then((result) => cb?.(result.models, result))
305320
.catch((error) => {
306321
console.log(`[flushModels] Refresh failed for ${provider}:`, error.message)
307322
})

src/core/webview/__tests__/ClineProvider.spec.ts

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,7 @@ vi.mock("../../../integrations/misc/extract-text", () => ({
294294

295295
vi.mock("../../../api/providers/fetchers/modelCache", () => ({
296296
getModels: vi.fn().mockResolvedValue({}),
297+
getModelsWithMetadata: vi.fn().mockResolvedValue({ models: {}, authoritative: false }),
297298
flushModels: vi.fn(),
298299
getModelsFromCache: vi.fn().mockReturnValue(undefined),
299300
}))
@@ -368,6 +369,7 @@ vi.mock("../../../integrations/misc/extract-text", () => ({
368369

369370
vi.mock("../../../api/providers/fetchers/modelCache", () => ({
370371
getModels: vi.fn().mockResolvedValue({}),
372+
getModelsWithMetadata: vi.fn().mockResolvedValue({ models: {}, authoritative: false }),
371373
flushModels: vi.fn(),
372374
getModelsFromCache: vi.fn().mockReturnValue(undefined),
373375
}))
@@ -3068,8 +3070,9 @@ describe("ClineProvider - Router Models", () => {
30683070
},
30693071
}
30703072

3071-
const { getModels } = await import("../../../api/providers/fetchers/modelCache")
3073+
const { getModels, getModelsWithMetadata } = await import("../../../api/providers/fetchers/modelCache")
30723074
vi.mocked(getModels).mockResolvedValue(mockModels)
3075+
vi.mocked(getModelsWithMetadata).mockResolvedValue({ models: mockModels, authoritative: false })
30733076

30743077
await messageHandler({ type: "requestRouterModels" })
30753078

@@ -3090,6 +3093,7 @@ describe("ClineProvider - Router Models", () => {
30903093
type: "costrictModels",
30913094
openAiModels: ["model-1", "model-2"],
30923095
fullResponseData: [mockModels["model-1"], mockModels["model-2"]],
3096+
modelListAuthoritative: false,
30933097
})
30943098

30953099
// Verify response was sent
@@ -3127,12 +3131,12 @@ describe("ClineProvider - Router Models", () => {
31273131
const mockModels = {
31283132
"model-1": { maxTokens: 4096, contextWindow: 8192, description: "Test model", supportsPromptCache: false },
31293133
}
3130-
const { getModels } = await import("../../../api/providers/fetchers/modelCache")
3134+
const { getModels, getModelsWithMetadata } = await import("../../../api/providers/fetchers/modelCache")
3135+
vi.mocked(getModelsWithMetadata).mockResolvedValue({ models: mockModels, authoritative: false })
31313136

31323137
// Mock some providers to succeed and others to fail
3133-
// Provider order in source: costrict, openrouter, requesty, vercel-ai-gateway, litellm (conditional)
3138+
// Costrict uses getModelsWithMetadata; remaining providers use getModels.
31343139
vi.mocked(getModels)
3135-
.mockResolvedValueOnce(mockModels) // costrict success (first call)
31363140
.mockResolvedValueOnce(mockModels) // openrouter success
31373141
.mockRejectedValueOnce(new Error("Requesty API error")) // requesty fail
31383142
.mockResolvedValueOnce(mockModels) // unbound success
@@ -3146,6 +3150,7 @@ describe("ClineProvider - Router Models", () => {
31463150
type: "costrictModels",
31473151
openAiModels: ["model-1"],
31483152
fullResponseData: [mockModels["model-1"]],
3153+
modelListAuthoritative: false,
31493154
})
31503155

31513156
// Verify main response includes successful providers and empty objects for failed ones
@@ -3198,8 +3203,9 @@ describe("ClineProvider - Router Models", () => {
31983203
const mockModels = {
31993204
"model-1": { maxTokens: 4096, contextWindow: 8192, description: "Test model", supportsPromptCache: false },
32003205
}
3201-
const { getModels } = await import("../../../api/providers/fetchers/modelCache")
3206+
const { getModels, getModelsWithMetadata } = await import("../../../api/providers/fetchers/modelCache")
32023207
vi.mocked(getModels).mockResolvedValue(mockModels)
3208+
vi.mocked(getModelsWithMetadata).mockResolvedValue({ models: mockModels, authoritative: false })
32033209

32043210
await messageHandler({
32053211
type: "requestRouterModels",
@@ -3232,10 +3238,9 @@ describe("ClineProvider - Router Models", () => {
32323238
const mockModels = {
32333239
"model-1": { maxTokens: 4096, contextWindow: 8192, description: "Test model", supportsPromptCache: false },
32343240
}
3235-
const { getModels } = await import("../../../api/providers/fetchers/modelCache")
3236-
vi.mocked(getModels)
3237-
.mockResolvedValueOnce(mockModels) // costrict success (first call)
3238-
.mockResolvedValue(mockModels) // other providers success
3241+
const { getModels, getModelsWithMetadata } = await import("../../../api/providers/fetchers/modelCache")
3242+
vi.mocked(getModels).mockResolvedValue(mockModels)
3243+
vi.mocked(getModelsWithMetadata).mockResolvedValue({ models: mockModels, authoritative: false })
32393244

32403245
await messageHandler({ type: "requestRouterModels" })
32413246

@@ -3251,6 +3256,7 @@ describe("ClineProvider - Router Models", () => {
32513256
type: "costrictModels",
32523257
openAiModels: ["model-1"],
32533258
fullResponseData: [mockModels["model-1"]],
3259+
modelListAuthoritative: false,
32543260
})
32553261

32563262
// Verify response includes empty object for LiteLLM

0 commit comments

Comments
 (0)