Skip to content

fix(litellm): preserve reasoning content in history using convertToR1Format#892

Closed
daewoongoh wants to merge 3 commits into
Zoo-Code-Org:mainfrom
daewoongoh:fix/litellm-preserve-reasoning
Closed

fix(litellm): preserve reasoning content in history using convertToR1Format#892
daewoongoh wants to merge 3 commits into
Zoo-Code-Org:mainfrom
daewoongoh:fix/litellm-preserve-reasoning

Conversation

@daewoongoh

@daewoongoh daewoongoh commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Related GitHub Issue

Closes: #891

Description

This PR fixes an issue where reasoning blocks were lost in multi-turn conversations when using reasoning-capable models through the LiteLLM provider.

Changes made:

  • Parsed and mapped preserveReasoning and reasoningEffort from the LiteLLM proxy response (ModelInfo).
  • Conditionally routed message formatting to use convertToR1Format with mergeToolResultText: true when preserveReasoning is enabled. This accurately merges tool results with subsequent user texts to prevent strict APIs (like DeepSeek) from dropping the reasoning_content.
  • Refactored LiteLLMHandler to ensure a clean fallback to convertToOpenAiMessages when the flag is absent or disabled.
  • Added a robust test case in lite-llm.spec.ts verifying that convertToR1Format correctly merges tool results and preserves the reasoning blocks for LiteLLM models.

Test Procedure

  1. Connect to LiteLLM with deepseek-reasoner (or any model supporting preserveReasoning).
  2. Make a multi-turn request involving a tool call.
  3. Observe the API payload logs to ensure reasoning_content remains intact in the assistant's message history.
  4. Run unit tests (pnpm --filter=zoo-code test -- -t "preserve reasoning when preserveReasoning is true").

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and/or updated tests have been added to cover my changes (if applicable).
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Documentation Updates

  • No documentation updates are required.
  • Yes, documentation updates are required. (Please describe what needs to be updated or link to a PR in the docs repository).

Get in Touch

hehegwk_23849

Summary by CodeRabbit

  • New Features
    • Added support for preserving reasoning content with compatible LiteLLM reasoning models.
    • Added per-model reasoning-effort handling for supported LiteLLM deployments.
  • Bug Fixes
    • Improved message conversion for R1-style reasoning, including correct merging of tool results and following text.
    • Ensured reasoning metadata is passed through when available from the model.
  • Tests
    • Expanded coverage for DeepSeek/R1-style reasoning preservation and updated LiteLLM model-formatting expectations.

…Format

- Map 'preserveReasoning' and 'reasoningEffort' from LiteLLM proxy response
- Use 'convertToR1Format' with 'mergeToolResultText: true' when 'preserveReasoning' is enabled to prevent reasoning drop
- Append 'reasoning_effort' parameter for supported models

Signed-off-by: daewoongoh <dw.oh@samsung.com>
@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e0980cd3-bef2-459a-b193-212a8b412ee0

📥 Commits

Reviewing files that changed from the base of the PR and between 7fa8ac9 and 3d87abf.

📒 Files selected for processing (3)
  • src/api/providers/__tests__/lite-llm.spec.ts
  • src/api/providers/fetchers/__tests__/litellm.spec.ts
  • src/api/providers/lite-llm.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/api/providers/tests/lite-llm.spec.ts
  • src/api/providers/lite-llm.ts

📝 Walkthrough

Walkthrough

LiteLLM model metadata now carries reasoning settings; createMessage uses R1 conversion when preservation is enabled and forwards reasoning effort. Tests verify reasoning content and tool/text message merging.

Changes

LiteLLM reasoning support

Layer / File(s) Summary
Expose reasoning model metadata
src/api/providers/fetchers/litellm.ts, src/api/providers/fetchers/__tests__/litellm.spec.ts
LiteLLM model entries conditionally include preserveReasoning and reasoningEffort, with tests covering the formatted response and reasoning properties.
Preserve reasoning in message requests
src/api/providers/lite-llm.ts, src/api/providers/__tests__/lite-llm.spec.ts
createMessage selects R1 formatting for reasoning models, forwards supported reasoning effort values, and verifies reasoning content and tool-message merging.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested labels: awaiting-review

Suggested reviewers: hannesrudolph, jamesrobert20, taltas, navedmerchant, edelauna

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main fix: preserving reasoning content in LiteLLM history.
Description check ✅ Passed The description matches the template with issue link, implementation summary, testing steps, checklist, and docs notes.
Linked Issues check ✅ Passed The changes implement #891 by preserving reasoning_content and merging tool results for reasoning-capable LiteLLM models.
Out of Scope Changes check ✅ Passed The diff stays focused on the reported LiteLLM reasoning-history bug and supporting tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

- Add none, minimal, xhigh, max options to reasoning_effort parameter typecast
- Add test coverage for preserveReasoning and reasoningEffort in fetcher and handler

Signed-off-by: daewoongoh <dw.oh@samsung.com>
@daewoongoh
daewoongoh force-pushed the fix/litellm-preserve-reasoning branch from 7fa8ac9 to 3d87abf Compare July 14, 2026 13:54
@github-actions github-actions Bot added the awaiting-review PR changes are ready and waiting for maintainer re-review label Jul 14, 2026
@zoomote

zoomote Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

P1: Respect the user’s reasoning-effort settings for LiteLLM

LiteLLMHandler now always forwards info.reasoningEffort from the proxy metadata. This bypasses the existing enableReasoningEffort opt-out and ignores an explicit reasoningEffort selected by the user, so a model with a default such as high cannot have reasoning disabled or adjusted.

Please resolve the effective effort through the shared model-parameter / shouldUseReasoningEffort convention before adding reasoning_effort. The fetched model metadata should also expose supportsReasoningEffort so the normal capability-driven settings UI can enable the control.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting-review PR changes are ready and waiting for maintainer re-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] reasoning_content is dropped in conversation history when using LiteLLM with reasoning models

1 participant