Skip to content

Commit d57a97a

Browse files
daewoongohdaewoongoh
committed
refactor(litellm): replace preserveReasoning regex with an explicit model id list
Regex-based family matching could over-match unrelated aliases sharing a substring (e.g. glm-5-flash matching a glm-5 prefix), a gap flagged in PR #899 review. LITELLM_PRESERVE_REASONING_MODEL_IDS now lists the exact model ids that set preserveReasoning: true in their native provider config, matched via isLiteLLMPreserveReasoningModel() against the final segment of model_name/litellm_params.model. Also fixes a test in litellm.spec.ts where the model_name alias itself matched a known id, so the assertion didn't actually prove litellm_params.model was being checked. Signed-off-by: daewoongoh <[email protected]>
1 parent 9f9c53b commit d57a97a

5 files changed

Lines changed: 141 additions & 89 deletions

File tree

‎packages/types/src/__tests__/lite-llm.test.ts‎

Lines changed: 33 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,70 +1,47 @@
1-
import { LITELLM_PRESERVE_REASONING_PATTERN } from "../providers/lite-llm.js"
1+
import { isLiteLLMPreserveReasoningModel, LITELLM_PRESERVE_REASONING_MODEL_IDS } from "../providers/lite-llm.js"
22

3-
describe("LITELLM_PRESERVE_REASONING_PATTERN", () => {
4-
it("matches known DeepSeek reasoning aliases", () => {
5-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("deepseek-v4-flash")).toBe(true)
6-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("deepseek-v4-pro")).toBe(true)
7-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("deepseek/deepseek-reasoner")).toBe(true)
8-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("deepseek-v4-mini")).toBe(false)
3+
describe("LiteLLM preserveReasoning model detection", () => {
4+
it("matches every explicitly listed model id", () => {
5+
for (const modelId of LITELLM_PRESERVE_REASONING_MODEL_IDS) {
6+
expect(isLiteLLMPreserveReasoningModel(modelId)).toBe(true)
7+
}
98
})
109

11-
it("matches known MiMo reasoning aliases", () => {
12-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("mimo-v2.5")).toBe(true)
13-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("mimo-v2.5-pro")).toBe(true)
14-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("mimo-v2.6")).toBe(false)
10+
it("does not contain duplicate model ids", () => {
11+
expect(new Set(LITELLM_PRESERVE_REASONING_MODEL_IDS).size).toBe(LITELLM_PRESERVE_REASONING_MODEL_IDS.length)
1512
})
1613

17-
it("matches known Kimi K2 reasoning aliases across routed providers", () => {
18-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("bedrock/moonshot.kimi-k2-thinking")).toBe(true)
19-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("fireworks_ai/accounts/fireworks/models/kimi-k2p7-code")).toBe(
20-
true,
21-
)
22-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("kimi-k2.7-code")).toBe(true)
23-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("kimi-k2.6")).toBe(false)
14+
it("matches provider-prefixed routed model names by their final segment", () => {
15+
expect(isLiteLLMPreserveReasoningModel("deepseek/deepseek-reasoner")).toBe(true)
16+
expect(isLiteLLMPreserveReasoningModel("bedrock/moonshot.kimi-k2-thinking")).toBe(true)
17+
expect(isLiteLLMPreserveReasoningModel("fireworks_ai/accounts/fireworks/models/kimi-k2p7-code")).toBe(true)
2418
})
2519

26-
it("matches MiniMax M2/M3 aliases but not other MiniMax generations", () => {
27-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("minimax-m2")).toBe(true)
28-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("minimax.m3")).toBe(true)
29-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("minimax-m2.5")).toBe(true)
30-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("minimax-m2-highspeed")).toBe(true)
31-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("minimax-m2-stable")).toBe(true)
32-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("minimax-m4")).toBe(false)
33-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("minimax-m1")).toBe(false)
20+
it("matches case-insensitively", () => {
21+
expect(isLiteLLMPreserveReasoningModel("MiniMax-M2.7-Highspeed")).toBe(true)
22+
expect(isLiteLLMPreserveReasoningModel("GLM-5.2")).toBe(true)
3423
})
3524

36-
it("matches GLM-4.7 but excludes the flash variants", () => {
37-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-4.7")).toBe(true)
38-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-4.7-flash")).toBe(false)
39-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-4.7-flashx")).toBe(false)
40-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-4.8")).toBe(false)
41-
})
42-
43-
it("matches GLM-5 variants but excludes the flash variant", () => {
44-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-5")).toBe(true)
45-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-5.1")).toBe(true)
46-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-5.2")).toBe(true)
47-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-5-turbo")).toBe(true)
48-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-5.1-turbo")).toBe(true)
49-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-5-flash")).toBe(false)
50-
})
51-
52-
it("matches curated Qwen3 plus/max aliases used by opencode-go", () => {
53-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("qwen3.7-plus")).toBe(true)
54-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("qwen3.6-max")).toBe(true)
55-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("qwen3.5-plus")).toBe(false)
56-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("qwen3.7-mini")).toBe(false)
25+
it("does not match model ids that merely contain a known family as a substring", () => {
26+
expect(isLiteLLMPreserveReasoningModel("deepseek-v4-mini")).toBe(false)
27+
expect(isLiteLLMPreserveReasoningModel("mimo-v2.6")).toBe(false)
28+
expect(isLiteLLMPreserveReasoningModel("kimi-k2.6")).toBe(false)
29+
expect(isLiteLLMPreserveReasoningModel("kimi-k2.7-code")).toBe(false)
30+
expect(isLiteLLMPreserveReasoningModel("minimax-m4")).toBe(false)
31+
expect(isLiteLLMPreserveReasoningModel("minimax-m1")).toBe(false)
32+
expect(isLiteLLMPreserveReasoningModel("glm-4.7-flash")).toBe(false)
33+
expect(isLiteLLMPreserveReasoningModel("glm-4.7-flashx")).toBe(false)
34+
expect(isLiteLLMPreserveReasoningModel("glm-5-flash")).toBe(false)
35+
expect(isLiteLLMPreserveReasoningModel("glm-4.8")).toBe(false)
36+
expect(isLiteLLMPreserveReasoningModel("qwen3.5-plus")).toBe(false)
37+
expect(isLiteLLMPreserveReasoningModel("qwen3.7-mini")).toBe(false)
38+
expect(isLiteLLMPreserveReasoningModel("qwen3.6-max")).toBe(false)
5739
})
5840

5941
it("does not match unrelated model names", () => {
60-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("gpt-4")).toBe(false)
61-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("claude-3-opus")).toBe(false)
62-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("")).toBe(false)
63-
})
64-
65-
it("matches when the fragment appears anywhere in a combined alias/routed-model string", () => {
66-
// Mirrors how litellm.ts calls it: `${modelName} ${litellmModelName}`
67-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("my-deepseek-alias deepseek/deepseek-reasoner")).toBe(true)
68-
expect(LITELLM_PRESERVE_REASONING_PATTERN.test("my-gpt4-alias openai/gpt-4")).toBe(false)
42+
expect(isLiteLLMPreserveReasoningModel("gpt-4")).toBe(false)
43+
expect(isLiteLLMPreserveReasoningModel("claude-3-opus")).toBe(false)
44+
expect(isLiteLLMPreserveReasoningModel("")).toBe(false)
45+
expect(isLiteLLMPreserveReasoningModel(undefined)).toBe(false)
6946
})
7047
})

‎packages/types/src/providers/lite-llm.ts‎

Lines changed: 70 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,25 +22,74 @@ export const litellmDefaultModelInfo: ModelInfo = {
2222
* (`litellm_params.model`, e.g. `deepseek/deepseek-reasoner`,
2323
* `bedrock/moonshot.kimi-k2-thinking`, `fireworks_ai/.../kimi-k2p7-code`).
2424
*
25-
* Each fragment below matches a model-family substring that requires
26-
* `preserveReasoning: true` in its native provider config (see deepseek.ts,
27-
* mimo.ts, moonshot.ts, bedrock.ts, fireworks.ts, zai.ts, minimax.ts,
28-
* opencode-go.ts), so the same behavior can be inferred for a LiteLLM-routed
29-
* alias of the same underlying model. This is best-effort: unrecognized
30-
* aliases or renamed deployments will not match, and callers should treat
31-
* it as a heuristic, not a source of truth.
25+
* Rather than matching model-family substrings with a regex (which can
26+
* over-match unrelated aliases, e.g. a family fragment appearing inside a
27+
* longer unrelated model id), this is an explicit list of the exact model
28+
* ids that set `preserveReasoning: true` in their native provider config
29+
* (see deepseek.ts, mimo.ts, moonshot.ts, bedrock.ts, fireworks.ts, zai.ts,
30+
* minimax.ts, opencode-go.ts). The same behavior is inferred for a
31+
* LiteLLM-routed alias of the same underlying model. Keep this list in sync
32+
* with those registries. This is still best-effort: unrecognized aliases or
33+
* renamed deployments will not match, and callers should treat it as a
34+
* heuristic, not a source of truth.
3235
*/
33-
const LITELLM_PRESERVE_REASONING_FRAGMENTS = [
34-
String.raw`deepseek-v4-(flash|pro)`, // deepseek.ts
35-
String.raw`deepseek-reasoner`, // deepseek.ts (legacy alias)
36-
String.raw`mimo-v2\.5(-pro)?`, // mimo.ts
37-
String.raw`kimi-k2-thinking`, // moonshot.ts, bedrock.ts, fireworks.ts
38-
String.raw`kimi-k2p7-code`, // fireworks.ts (dash-separated id)
39-
String.raw`kimi-k2\.7-code`, // fireworks.ts (dotted alias variant)
40-
String.raw`minimax[.-]?m[23](\.\d+)?(-highspeed|-stable)?\b`, // bedrock.ts, minimax.ts, opencode-go.ts
41-
String.raw`glm-4\.7(?!-flash)\b`, // zai.ts (excludes glm-4.7-flash(x))
42-
String.raw`glm-5(\.[12])?(-turbo)?(?!-flash)\b`, // zai.ts, opencode-go.ts
43-
String.raw`qwen3\.[67]-(plus|max)`, // opencode-go.ts
44-
]
45-
46-
export const LITELLM_PRESERVE_REASONING_PATTERN = new RegExp(LITELLM_PRESERVE_REASONING_FRAGMENTS.join("|"), "i")
36+
export const LITELLM_PRESERVE_REASONING_MODEL_IDS = [
37+
// deepseek.ts
38+
"deepseek-v4-flash",
39+
"deepseek-v4-pro",
40+
"deepseek-reasoner",
41+
42+
// mimo.ts, opencode-go.ts
43+
"mimo-v2.5",
44+
"mimo-v2.5-pro",
45+
46+
// moonshot.ts, bedrock.ts, fireworks.ts
47+
"kimi-k2-thinking",
48+
"moonshot.kimi-k2-thinking",
49+
"kimi-k2p7-code",
50+
51+
// zai.ts
52+
"glm-4.7",
53+
"glm-5",
54+
"glm-5.1",
55+
"glm-5.2",
56+
"glm-5-turbo",
57+
58+
// bedrock.ts, minimax.ts, opencode-go.ts
59+
"minimax.minimax-m2",
60+
"minimax-m2",
61+
"minimax-m2-stable",
62+
"minimax-m2.1",
63+
"minimax-m2.1-highspeed",
64+
"minimax-m2.5",
65+
"minimax-m2.5-highspeed",
66+
"minimax-m2.7",
67+
"minimax-m2.7-highspeed",
68+
"minimax-m3",
69+
70+
// opencode-go.ts
71+
"qwen3.6-plus",
72+
"qwen3.7-plus",
73+
"qwen3.7-max",
74+
] as const
75+
76+
const LITELLM_PRESERVE_REASONING_MODEL_ID_SET = new Set<string>(LITELLM_PRESERVE_REASONING_MODEL_IDS)
77+
78+
/**
79+
* Checks whether `modelName` (a LiteLLM alias or routed `litellm_params.model`
80+
* value) identifies a model that requires `preserveReasoning: true`.
81+
* Provider-prefixed routed names (e.g. `deepseek/deepseek-reasoner`,
82+
* `fireworks_ai/accounts/fireworks/models/kimi-k2p7-code`) are matched by
83+
* their final slash-delimited segment.
84+
*/
85+
export function isLiteLLMPreserveReasoningModel(modelName: string | undefined): boolean {
86+
const normalized = modelName?.trim().toLowerCase()
87+
88+
if (!normalized) {
89+
return false
90+
}
91+
92+
const modelId = normalized.split("/").pop()
93+
94+
return modelId !== undefined && LITELLM_PRESERVE_REASONING_MODEL_ID_SET.has(modelId)
95+
}

‎src/api/providers/fetchers/__tests__/litellm.spec.ts‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -699,12 +699,12 @@ describe("getLiteLLMModels", () => {
699699
})
700700

701701
describe("preserveReasoning inference", () => {
702-
it("sets preserveReasoning: true when the routed model matches a known reasoning family", async () => {
702+
it("sets preserveReasoning: true when only the routed model (not the alias) matches a known reasoning model id", async () => {
703703
const mockResponse = {
704704
data: {
705705
data: [
706706
{
707-
model_name: "deepseek-reasoner-alias",
707+
model_name: "my-deepseek-alias",
708708
model_info: {
709709
max_tokens: 8192,
710710
max_input_tokens: 128000,
@@ -714,7 +714,7 @@ describe("getLiteLLMModels", () => {
714714
},
715715
},
716716
{
717-
model_name: "kimi-k2-thinking",
717+
model_name: "my-kimi-alias",
718718
model_info: {
719719
max_tokens: 8192,
720720
max_input_tokens: 128000,
@@ -731,11 +731,11 @@ describe("getLiteLLMModels", () => {
731731

732732
const result = await getLiteLLMModels("test-api-key", "http://localhost:4000")
733733

734-
expect(result["deepseek-reasoner-alias"]).toMatchObject({ preserveReasoning: true })
735-
expect(result["kimi-k2-thinking"]).toMatchObject({ preserveReasoning: true })
734+
expect(result["my-deepseek-alias"]).toMatchObject({ preserveReasoning: true })
735+
expect(result["my-kimi-alias"]).toMatchObject({ preserveReasoning: true })
736736
})
737737

738-
it("omits preserveReasoning when the routed model does not match a known reasoning family", async () => {
738+
it("omits preserveReasoning when the routed model does not match a known reasoning model id", async () => {
739739
const mockResponse = {
740740
data: {
741741
data: [
@@ -765,7 +765,7 @@ describe("getLiteLLMModels", () => {
765765
data: {
766766
data: [
767767
{
768-
model_name: "glm-5.1-turbo",
768+
model_name: "glm-5.2",
769769
model_info: {
770770
max_tokens: 8192,
771771
max_input_tokens: 128000,
@@ -782,7 +782,32 @@ describe("getLiteLLMModels", () => {
782782

783783
const result = await getLiteLLMModels("test-api-key", "http://localhost:4000")
784784

785-
expect(result["glm-5.1-turbo"]).toMatchObject({ preserveReasoning: true })
785+
expect(result["glm-5.2"]).toMatchObject({ preserveReasoning: true })
786+
})
787+
788+
it("does not match a model id that merely contains a known family as a substring", async () => {
789+
const mockResponse = {
790+
data: {
791+
data: [
792+
{
793+
model_name: "glm-5-flash",
794+
model_info: {
795+
max_tokens: 8192,
796+
max_input_tokens: 128000,
797+
},
798+
litellm_params: {
799+
model: "zai/glm-5-flash",
800+
},
801+
},
802+
],
803+
},
804+
}
805+
806+
mockedAxios.get.mockResolvedValue(mockResponse)
807+
808+
const result = await getLiteLLMModels("test-api-key", "http://localhost:4000")
809+
810+
expect(result["glm-5-flash"]).not.toHaveProperty("preserveReasoning")
786811
})
787812
})
788813
})

‎src/api/providers/fetchers/litellm.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import axios from "axios"
22

33
import type { ModelRecord } from "@roo-code/types"
4-
import { LITELLM_PRESERVE_REASONING_PATTERN } from "@roo-code/types"
4+
import { isLiteLLMPreserveReasoningModel } from "@roo-code/types"
55

66
import { DEFAULT_HEADERS } from "../constants"
77
/**
@@ -42,8 +42,9 @@ export async function getLiteLLMModels(apiKey: string, baseUrl: string): Promise
4242
if (!modelName || !modelInfo || !litellmModelName) continue
4343

4444
// LiteLLM's /v1/model/info never reports reasoning capability flags, so infer
45-
// preserveReasoning from the alias/routed-model name against known model families.
46-
const preservesReasoning = LITELLM_PRESERVE_REASONING_PATTERN.test(`${modelName} ${litellmModelName}`)
45+
// preserveReasoning from explicit model ids in either the alias or routed model name.
46+
const preservesReasoning =
47+
isLiteLLMPreserveReasoningModel(modelName) || isLiteLLMPreserveReasoningModel(litellmModelName)
4748

4849
models[modelName] = {
4950
maxTokens: modelInfo.max_output_tokens || modelInfo.max_tokens || 8192,

‎src/api/providers/lite-llm.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ export class LiteLLMHandler extends RouterProvider implements SingleCompletionHa
119119
const { id: modelId, info } = await this.fetchModel()
120120

121121
// Models that require reasoning_content to be echoed back during tool-call
122-
// continuations (see LITELLM_PRESERVE_REASONING_PATTERN) need convertToR1Format:
122+
// continuations (see LITELLM_PRESERVE_REASONING_MODEL_IDS) need convertToR1Format:
123123
// it merges consecutive same-role messages and, via mergeToolResultText, folds
124124
// text following tool_results into the last tool message so a user message
125125
// never gets inserted mid-turn and causes the model to drop prior reasoning_content.

0 commit comments

Comments
 (0)