Repository navigation
fix(litellm): preserve reasoning_content for known reasoning model families #899
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
902d63c
ce38822
9f9c53b
d57a97a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| import { LITELLM_PRESERVE_REASONING_PATTERN } from "../providers/lite-llm.js" | ||
|
|
||
| describe("LITELLM_PRESERVE_REASONING_PATTERN", () => { | ||
| it("matches known DeepSeek reasoning aliases", () => { | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("deepseek-v4-flash")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("deepseek-v4-pro")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("deepseek/deepseek-reasoner")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("deepseek-v4-mini")).toBe(false) | ||
| }) | ||
|
|
||
| it("matches known MiMo reasoning aliases", () => { | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("mimo-v2.5")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("mimo-v2.5-pro")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("mimo-v2.6")).toBe(false) | ||
| }) | ||
|
|
||
| it("matches known Kimi K2 reasoning aliases across routed providers", () => { | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("bedrock/moonshot.kimi-k2-thinking")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("fireworks_ai/accounts/fireworks/models/kimi-k2p7-code")).toBe( | ||
| true, | ||
| ) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("kimi-k2.7-code")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("kimi-k2.6")).toBe(false) | ||
| }) | ||
|
|
||
| it("matches MiniMax M2/M3 aliases but not other MiniMax generations", () => { | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("minimax-m2")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("minimax.m3")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("minimax-m2.5")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("minimax-m2-highspeed")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("minimax-m2-stable")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("minimax-m4")).toBe(false) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("minimax-m1")).toBe(false) | ||
| }) | ||
|
|
||
| it("matches GLM-4.7 but excludes the flash variants", () => { | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-4.7")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-4.7-flash")).toBe(false) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-4.7-flashx")).toBe(false) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-4.8")).toBe(false) | ||
| }) | ||
|
|
||
| it("matches GLM-5 variants but excludes the flash variant", () => { | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-5")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-5.1")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-5.2")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-5-turbo")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-5.1-turbo")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("glm-5-flash")).toBe(false) | ||
| }) | ||
|
|
||
| it("matches curated Qwen3 plus/max aliases used by opencode-go", () => { | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("qwen3.7-plus")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("qwen3.6-max")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("qwen3.5-plus")).toBe(false) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("qwen3.7-mini")).toBe(false) | ||
| }) | ||
|
|
||
| it("does not match unrelated model names", () => { | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("gpt-4")).toBe(false) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("claude-3-opus")).toBe(false) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("")).toBe(false) | ||
| }) | ||
|
|
||
| it("matches when the fragment appears anywhere in a combined alias/routed-model string", () => { | ||
| // Mirrors how litellm.ts calls it: `${modelName} ${litellmModelName}` | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("my-deepseek-alias deepseek/deepseek-reasoner")).toBe(true) | ||
| expect(LITELLM_PRESERVE_REASONING_PATTERN.test("my-gpt4-alias openai/gpt-4")).toBe(false) | ||
| }) | ||
| }) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -697,4 +697,92 @@ describe("getLiteLLMModels", () => { | |
| description: "model-with-only-max-output-tokens via LiteLLM proxy", | ||
| }) | ||
| }) | ||
|
|
||
| describe("preserveReasoning inference", () => { | ||
| it("sets preserveReasoning: true when the routed model matches a known reasoning family", async () => { | ||
| const mockResponse = { | ||
| data: { | ||
| data: [ | ||
| { | ||
| model_name: "deepseek-reasoner-alias", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this alias be something neutral that does not match the preserve-reasoning pattern? As written, this still passes if the implementation only checks
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in d57a97a — the alias (model_name) is now a neutral string that doesn't match the allowlist on its own (my-deepseek-alias, my-kimi-alias), so this test only passes if litellm_params.model is actually checked. Added a companion test for the reverse case (alias matches, routed model doesn't) to confirm both fields are checked independently. |
||
| model_info: { | ||
| max_tokens: 8192, | ||
| max_input_tokens: 128000, | ||
| }, | ||
| litellm_params: { | ||
| model: "deepseek/deepseek-reasoner", | ||
| }, | ||
| }, | ||
| { | ||
| model_name: "kimi-k2-thinking", | ||
| model_info: { | ||
| max_tokens: 8192, | ||
| max_input_tokens: 128000, | ||
| }, | ||
| litellm_params: { | ||
| model: "bedrock/moonshot.kimi-k2-thinking", | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| } | ||
|
|
||
| mockedAxios.get.mockResolvedValue(mockResponse) | ||
|
|
||
| const result = await getLiteLLMModels("test-api-key", "http://localhost:4000") | ||
|
|
||
| expect(result["deepseek-reasoner-alias"]).toMatchObject({ preserveReasoning: true }) | ||
| expect(result["kimi-k2-thinking"]).toMatchObject({ preserveReasoning: true }) | ||
| }) | ||
|
|
||
| it("omits preserveReasoning when the routed model does not match a known reasoning family", async () => { | ||
| const mockResponse = { | ||
| data: { | ||
| data: [ | ||
| { | ||
| model_name: "gpt-4-turbo", | ||
| model_info: { | ||
| max_tokens: 8192, | ||
| max_input_tokens: 128000, | ||
| }, | ||
| litellm_params: { | ||
| model: "openai/gpt-4-turbo", | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| } | ||
|
|
||
| mockedAxios.get.mockResolvedValue(mockResponse) | ||
|
|
||
| const result = await getLiteLLMModels("test-api-key", "http://localhost:4000") | ||
|
|
||
| expect(result["gpt-4-turbo"]).not.toHaveProperty("preserveReasoning") | ||
| }) | ||
|
|
||
| it("matches against the model alias even when the routed model name does not match", async () => { | ||
| const mockResponse = { | ||
| data: { | ||
| data: [ | ||
| { | ||
| model_name: "glm-5.1-turbo", | ||
| model_info: { | ||
| max_tokens: 8192, | ||
| max_input_tokens: 128000, | ||
| }, | ||
| litellm_params: { | ||
| model: "zai/some-custom-deployment", | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| } | ||
|
|
||
| mockedAxios.get.mockResolvedValue(mockResponse) | ||
|
|
||
| const result = await getLiteLLMModels("test-api-key", "http://localhost:4000") | ||
|
|
||
| expect(result["glm-5.1-turbo"]).toMatchObject({ preserveReasoning: true }) | ||
| }) | ||
| }) | ||
| }) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this still match
glm-5as a prefix for names likeglm-5.1-flashorglm-5.3? If those should stay excluded, it may be worth making this fragment consume the accepted suffix fully and adding negative cases for those variants.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in d57a97a by dropping the regex/prefix approach entirely. isLiteLLMPreserveReasoningModel now does an exact match against LITELLM_PRESERVE_REASONING_MODEL_IDS via Set.has() on the full (lowercased) model id, so glm-5 no longer matches glm-5.1-flash, glm-5.3, etc. Added negative test cases for exactly these variants (glm-5-flash, glm-4.7-flash, glm-4.7-flashx, glm-4.8, etc.) in lite-llm.test.ts.