Skip to content

Commit b7f78f8

Browse files
WebMadWebMad
authored
refactor(providers): canonicalize model-router calls (#1165)
Use the shared providerIdentifiers registry throughout model-router integrations while preserving existing serialized and wire values. Centralize missing API-key placeholders and extend provider regression coverage.
1 parent 05f8a3e commit b7f78f8

30 files changed

Lines changed: 600 additions & 234 deletions

‎src/api/providers/__tests__/openrouter.spec.ts‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ const MOCK_TIMEOUT_MS = 300_000
1717
import { Anthropic } from "@anthropic-ai/sdk"
1818
import OpenAI from "openai"
1919

20+
import { providerIdentifiers } from "@roo-code/types"
21+
2022
import { OpenRouterHandler } from "../openrouter"
2123
import { Package } from "../../../shared/package"
2224
import { makeApiHandlerOptions } from "../../../test-utils/api"
@@ -349,7 +351,7 @@ describe("OpenRouterHandler", () => {
349351
expect(mockCaptureException).toHaveBeenCalledWith(
350352
expect.objectContaining({
351353
message: "API Error",
352-
provider: "OpenRouter",
354+
provider: providerIdentifiers.openrouter,
353355
modelId: mockOptions.openRouterModelId,
354356
operation: "createMessage",
355357
errorCode: 500,
@@ -371,7 +373,7 @@ describe("OpenRouterHandler", () => {
371373
expect(mockCaptureException).toHaveBeenCalledWith(
372374
expect.objectContaining({
373375
message: "Connection failed",
374-
provider: "OpenRouter",
376+
provider: providerIdentifiers.openrouter,
375377
modelId: mockOptions.openRouterModelId,
376378
operation: "createMessage",
377379
}),
@@ -394,7 +396,7 @@ describe("OpenRouterHandler", () => {
394396
expect(mockCaptureException).toHaveBeenCalledWith(
395397
expect.objectContaining({
396398
message: "Rate limit exceeded: free-models-per-day",
397-
provider: "OpenRouter",
399+
provider: providerIdentifiers.openrouter,
398400
modelId: mockOptions.openRouterModelId,
399401
operation: "createMessage",
400402
}),
@@ -415,7 +417,7 @@ describe("OpenRouterHandler", () => {
415417
expect(mockCaptureException).toHaveBeenCalledWith(
416418
expect.objectContaining({
417419
message: "429 Rate limit exceeded: free-models-per-day",
418-
provider: "OpenRouter",
420+
provider: providerIdentifiers.openrouter,
419421
modelId: mockOptions.openRouterModelId,
420422
operation: "createMessage",
421423
}),
@@ -436,7 +438,7 @@ describe("OpenRouterHandler", () => {
436438
expect(mockCaptureException).toHaveBeenCalledWith(
437439
expect.objectContaining({
438440
message: "Request failed due to rate limit",
439-
provider: "OpenRouter",
441+
provider: providerIdentifiers.openrouter,
440442
modelId: mockOptions.openRouterModelId,
441443
operation: "createMessage",
442444
}),
@@ -458,7 +460,7 @@ describe("OpenRouterHandler", () => {
458460
expect(mockCaptureException).toHaveBeenCalledWith(
459461
expect.objectContaining({
460462
message: "Rate limit exceeded",
461-
provider: "OpenRouter",
463+
provider: providerIdentifiers.openrouter,
462464
modelId: mockOptions.openRouterModelId,
463465
operation: "createMessage",
464466
errorCode: 429,
@@ -585,7 +587,7 @@ describe("OpenRouterHandler", () => {
585587
expect(mockCaptureException).toHaveBeenCalledWith(
586588
expect.objectContaining({
587589
message: "API Error",
588-
provider: "OpenRouter",
590+
provider: providerIdentifiers.openrouter,
589591
modelId: mockOptions.openRouterModelId,
590592
operation: "completePrompt",
591593
errorCode: 500,
@@ -608,7 +610,7 @@ describe("OpenRouterHandler", () => {
608610
expect(mockCaptureException).toHaveBeenCalledWith(
609611
expect.objectContaining({
610612
message: "Unexpected error",
611-
provider: "OpenRouter",
613+
provider: providerIdentifiers.openrouter,
612614
modelId: mockOptions.openRouterModelId,
613615
operation: "completePrompt",
614616
}),
@@ -630,7 +632,7 @@ describe("OpenRouterHandler", () => {
630632
expect(mockCaptureException).toHaveBeenCalledWith(
631633
expect.objectContaining({
632634
message: "Rate limit exceeded: free-models-per-day",
633-
provider: "OpenRouter",
635+
provider: providerIdentifiers.openrouter,
634636
modelId: mockOptions.openRouterModelId,
635637
operation: "completePrompt",
636638
}),
@@ -651,7 +653,7 @@ describe("OpenRouterHandler", () => {
651653
expect(mockCaptureException).toHaveBeenCalledWith(
652654
expect.objectContaining({
653655
message: "429 Rate limit exceeded: free-models-per-day",
654-
provider: "OpenRouter",
656+
provider: providerIdentifiers.openrouter,
655657
modelId: mockOptions.openRouterModelId,
656658
operation: "completePrompt",
657659
}),
@@ -672,7 +674,7 @@ describe("OpenRouterHandler", () => {
672674
expect(mockCaptureException).toHaveBeenCalledWith(
673675
expect.objectContaining({
674676
message: "Request failed due to rate limit",
675-
provider: "OpenRouter",
677+
provider: providerIdentifiers.openrouter,
676678
modelId: mockOptions.openRouterModelId,
677679
operation: "completePrompt",
678680
}),
@@ -701,7 +703,7 @@ describe("OpenRouterHandler", () => {
701703
expect(mockCaptureException).toHaveBeenCalledWith(
702704
expect.objectContaining({
703705
message: "Rate limit exceeded",
704-
provider: "OpenRouter",
706+
provider: providerIdentifiers.openrouter,
705707
modelId: mockOptions.openRouterModelId,
706708
operation: "completePrompt",
707709
errorCode: 429,

‎src/api/providers/__tests__/poe.spec.ts‎

Lines changed: 124 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,55 @@
1-
const mockStreamText = vitest.fn()
2-
const mockGenerateText = vitest.fn()
3-
const mockCreatePoe = vitest.fn()
1+
import { poeDefaultModelId, providerIdentifiers } from "@roo-code/types"
2+
3+
import { PoeHandler } from "../poe"
4+
import { getModelsFromCache } from "../fetchers/modelCache"
5+
6+
import { clearAllMocks } from "../../../test-utils/reset"
7+
8+
const { mockStreamText, mockGenerateText, mockCreatePoe, mockGetModelsFromCache, mockCaptureException } =
9+
vitest.hoisted(() => ({
10+
mockStreamText: vitest.fn(),
11+
mockGenerateText: vitest.fn(),
12+
mockCreatePoe: vitest.fn(),
13+
mockCaptureException: vitest.fn(),
14+
mockGetModelsFromCache: vitest.fn(),
15+
}))
16+
17+
const cachedModels = {
18+
"anthropic/claude-sonnet-4": {
19+
maxTokens: 10_000,
20+
contextWindow: 200_000,
21+
supportsImages: true,
22+
supportsPromptCache: true,
23+
supportsReasoningBudget: true,
24+
inputPrice: 3,
25+
outputPrice: 15,
26+
},
27+
"openai/gpt-4o": {
28+
maxTokens: 16_384,
29+
contextWindow: 128_000,
30+
supportsImages: true,
31+
supportsPromptCache: false,
32+
inputPrice: 2.5,
33+
outputPrice: 10,
34+
},
35+
"openai/o3": {
36+
maxTokens: 100_000,
37+
contextWindow: 200_000,
38+
supportsImages: true,
39+
supportsPromptCache: false,
40+
supportsReasoningEffort: ["low", "medium", "high"],
41+
inputPrice: 10,
42+
outputPrice: 40,
43+
},
44+
}
45+
46+
vitest.mock("@roo-code/telemetry", () => ({
47+
TelemetryService: {
48+
instance: {
49+
captureException: (...args: unknown[]) => mockCaptureException(...args),
50+
},
51+
},
52+
}))
453

554
vitest.mock("ai-sdk-provider-poe", () => ({
655
createPoe: (...args: unknown[]) => mockCreatePoe(...args),
@@ -41,48 +90,17 @@ vitest.mock("ai", async (importOriginal) => {
4190
})
4291

4392
vitest.mock("../fetchers/modelCache", () => ({
44-
getModelsFromCache: vitest.fn().mockReturnValue({
45-
"anthropic/claude-sonnet-4": {
46-
maxTokens: 10_000,
47-
contextWindow: 200_000,
48-
supportsImages: true,
49-
supportsPromptCache: true,
50-
supportsReasoningBudget: true,
51-
inputPrice: 3,
52-
outputPrice: 15,
53-
},
54-
"openai/gpt-4o": {
55-
maxTokens: 16_384,
56-
contextWindow: 128_000,
57-
supportsImages: true,
58-
supportsPromptCache: false,
59-
inputPrice: 2.5,
60-
outputPrice: 10,
61-
},
62-
"openai/o3": {
63-
maxTokens: 100_000,
64-
contextWindow: 200_000,
65-
supportsImages: true,
66-
supportsPromptCache: false,
67-
supportsReasoningEffort: ["low", "medium", "high"],
68-
inputPrice: 10,
69-
outputPrice: 40,
70-
},
71-
}),
93+
getModelsFromCache: mockGetModelsFromCache,
7294
}))
7395

74-
import { poeDefaultModelId } from "@roo-code/types"
75-
import { PoeHandler } from "../poe"
76-
77-
import { clearAllMocks } from "../../../test-utils/reset"
78-
7996
describe("PoeHandler", () => {
8097
const mockLanguageModel = { modelId: "test-model" }
8198
const mockPoeProvider = vitest.fn().mockReturnValue(mockLanguageModel)
8299

83100
beforeEach(() => {
84101
clearAllMocks()
85102
mockCreatePoe.mockReturnValue(mockPoeProvider)
103+
mockGetModelsFromCache.mockReturnValue(cachedModels)
86104
})
87105

88106
describe("constructor", () => {
@@ -116,9 +134,19 @@ describe("PoeHandler", () => {
116134

117135
describe("getModel", () => {
118136
it("returns model info from cache", () => {
119-
const handler = new PoeHandler({ poeApiKey: "key", apiModelId: "anthropic/claude-sonnet-4" })
137+
const options = {
138+
poeApiKey: "key",
139+
poeBaseUrl: "https://custom.poe.com/v1",
140+
apiModelId: "anthropic/claude-sonnet-4",
141+
}
142+
const handler = new PoeHandler(options)
120143
const result = handler.getModel()
121144

145+
expect(getModelsFromCache).toHaveBeenCalledWith({
146+
provider: providerIdentifiers.poe,
147+
apiKey: options.poeApiKey,
148+
baseUrl: options.poeBaseUrl,
149+
})
122150
expect(result.id).toBe("anthropic/claude-sonnet-4")
123151
expect(result.info.contextWindow).toBe(200_000)
124152
expect(result.info.maxTokens).toBe(10_000)
@@ -166,6 +194,49 @@ describe("PoeHandler", () => {
166194
expect(chunks).toContainEqual({ type: "text", text: "world!" })
167195
expect(chunks).toContainEqual(expect.objectContaining({ type: "usage", inputTokens: 10, outputTokens: 5 }))
168196
})
197+
198+
it("reports synchronous completion failures with the canonical provider identifier", async () => {
199+
const handler = new PoeHandler({ poeApiKey: "key", apiModelId: "openai/gpt-4o" })
200+
mockStreamText.mockImplementationOnce(() => {
201+
throw new Error("request failed")
202+
})
203+
204+
await expect(
205+
handler.createMessage("system", [{ role: "user" as const, content: "hello" }]).next(),
206+
).rejects.toThrow("Poe completion error: request failed")
207+
expect(mockCaptureException).toHaveBeenCalledWith(
208+
expect.objectContaining({
209+
provider: providerIdentifiers.poe,
210+
modelId: "openai/gpt-4o",
211+
operation: "createMessage",
212+
}),
213+
)
214+
})
215+
216+
it("reports asynchronous stream failures with the canonical provider identifier", async () => {
217+
const handler = new PoeHandler({ poeApiKey: "key", apiModelId: "openai/gpt-4o" })
218+
const failedStream = {
219+
[Symbol.asyncIterator]() {
220+
return this
221+
},
222+
next: vitest.fn().mockRejectedValueOnce(new Error("stream failed")),
223+
}
224+
mockStreamText.mockReturnValueOnce({
225+
fullStream: failedStream,
226+
usage: Promise.resolve(undefined),
227+
})
228+
229+
await expect(
230+
handler.createMessage("system", [{ role: "user" as const, content: "hello" }]).next(),
231+
).rejects.toThrow("Poe streaming error: stream failed")
232+
expect(mockCaptureException).toHaveBeenCalledWith(
233+
expect.objectContaining({
234+
provider: providerIdentifiers.poe,
235+
modelId: "openai/gpt-4o",
236+
operation: "createMessage",
237+
}),
238+
)
239+
})
169240
})
170241

171242
describe("reasoning", () => {
@@ -311,5 +382,21 @@ describe("PoeHandler", () => {
311382
}),
312383
)
313384
})
385+
386+
it("reports failures with the canonical provider identifier", async () => {
387+
const handler = new PoeHandler({ poeApiKey: "key", apiModelId: "openai/gpt-4o" })
388+
mockGenerateText.mockRejectedValueOnce(new Error("generation failed"))
389+
390+
await expect(handler.completePrompt("complete this")).rejects.toThrow(
391+
"Poe completion error: generation failed",
392+
)
393+
expect(mockCaptureException).toHaveBeenCalledWith(
394+
expect.objectContaining({
395+
provider: providerIdentifiers.poe,
396+
modelId: "openai/gpt-4o",
397+
operation: "completePrompt",
398+
}),
399+
)
400+
})
314401
})
315402
})

‎src/api/providers/anthropic-vertex.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
} from "../../core/prompts/tools/native-tools/converters"
2525

2626
import { BaseProvider } from "./base-provider"
27+
import { NOT_PROVIDED } from "./constants"
2728
import { parseVertexJsonCredentials } from "./utils/vertex-credentials"
2829
import type { SingleCompletionHandler, ApiHandlerCreateMessageMetadata, CompletePromptOptions } from "../index"
2930

@@ -38,7 +39,7 @@ export class AnthropicVertexHandler extends BaseProvider implements SingleComple
3839
this.options = options
3940

4041
// https://cloud.google.com/vertex-ai/generative-ai/docs/partner-models/use-claude#regions
41-
const projectId = this.options.vertexProjectId ?? "not-provided"
42+
const projectId = this.options.vertexProjectId ?? NOT_PROVIDED
4243
const region = this.options.vertexRegion ?? "us-east5"
4344

4445
const parsedVertexCredentials = parseVertexJsonCredentials(this.options.vertexJsonCredentials)

‎src/api/providers/constants.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,5 @@ export const DEFAULT_HEADERS = {
55
"X-Title": "Zoo Code",
66
"User-Agent": `ZooCode/${Package.version}`,
77
}
8+
9+
export const NOT_PROVIDED = "not-provided"

‎src/api/providers/deepseek.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { getModelParams } from "../transform/model-params"
1717
import { convertToR1Format } from "../transform/r1-format"
1818

1919
import { OpenAiHandler } from "./openai"
20+
import { NOT_PROVIDED } from "./constants"
2021
import { extractReasoningFromDelta } from "./utils/extract-reasoning"
2122
import type { ApiHandlerCreateMessageMetadata } from "../index"
2223
import { handleOpenAIError } from "./utils/error-handler"
@@ -84,7 +85,7 @@ export class DeepSeekHandler extends OpenAiHandler {
8485
constructor(options: ApiHandlerOptions) {
8586
super({
8687
...options,
87-
openAiApiKey: options.deepSeekApiKey ?? "not-provided",
88+
openAiApiKey: options.deepSeekApiKey ?? NOT_PROVIDED,
8889
openAiModelId: options.apiModelId ?? deepSeekDefaultModelId,
8990
openAiBaseUrl: options.deepSeekBaseUrl || "https://api.deepseek.com",
9091
openAiStreamingEnabled: true,

0 commit comments

Comments
 (0)