Skip to content

Commit 5fa1716

Browse files
committed
fix(model-cache): drop redundant telemetry gate, restore ordering comment, cover in-flight cleanup
1 parent 7a4cd75 commit 5fa1716

2 files changed

Lines changed: 35 additions & 4 deletions

File tree

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,37 @@ describe("empty cache protection", () => {
430430
expect(result2).toEqual(mockModels)
431431
})
432432

433+
it("removes the in-flight entry after settlement so a later call starts a fresh fetch", async () => {
434+
// Proves the dedupedFetch() finally() cleanup actually runs: if the in-flight map
435+
// entry were never removed, this second, later call would resolve to the first
436+
// call's stale result instead of invoking the fetcher again.
437+
const firstModels = {
438+
"openrouter/first": {
439+
maxTokens: 8192,
440+
contextWindow: 128000,
441+
supportsPromptCache: false,
442+
description: "First response",
443+
},
444+
}
445+
const secondModels = {
446+
"openrouter/second": {
447+
maxTokens: 4096,
448+
contextWindow: 64000,
449+
supportsPromptCache: false,
450+
description: "Second response",
451+
},
452+
}
453+
mockGetOpenRouterModels.mockResolvedValueOnce(firstModels).mockResolvedValueOnce(secondModels)
454+
mockGet.mockReturnValue(undefined)
455+
456+
const result1 = await getModels({ provider: providerIdentifiers.openrouter })
457+
const result2 = await getModels({ provider: providerIdentifiers.openrouter })
458+
459+
expect(mockGetOpenRouterModels).toHaveBeenCalledTimes(2)
460+
expect(result1).toEqual(firstModels)
461+
expect(result2).toEqual(secondModels)
462+
})
463+
433464
it("shares a single in-flight fetch between getModels() and refreshModels() for the same key", async () => {
434465
// Both entry points converge on the same coordinator so a getModels() cache miss
435466
// racing a concurrent refreshModels() call can't produce two unordered cache writes.

src/api/providers/fetchers/modelCache.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,6 @@ function captureModelCacheEmptyResponseOnce(
5959
return
6060
}
6161

62-
if (!TelemetryService.instance.isTelemetryEnabled()) {
63-
return
64-
}
65-
6662
reportedEmptyModelResponse.add(cacheKey)
6763
TelemetryService.instance.captureEvent(TelemetryEventName.MODEL_CACHE_EMPTY_RESPONSE, { provider, ...properties })
6864
}
@@ -369,6 +365,10 @@ function dedupedFetch(cacheKey: string, options: GetModelsOptions): Promise<Mode
369365
inFlightRefresh.delete(cacheKey)
370366
})
371367

368+
// The finally cleanup above can only run after this function's current synchronous run --
369+
// including the set() below -- completes, since that's the earliest a promise reaction can
370+
// fire. So the entry is always registered before finally can delete it, even if
371+
// fetchModelsFromProvider() resolves immediately.
372372
inFlightRefresh.set(cacheKey, fetchPromise)
373373

374374
return fetchPromise

0 commit comments

Comments
 (0)