fix(openai): preserve reasoning for local OpenAI-compatible models via R1 toggle

[ch405canova-sudo]

Summary

Local OpenAI-compatible reasoning models (llama.cpp llama-server, LM Studio, Ollama's OpenAI endpoint, vLLM, etc.) stream a reasoning_content field, but Zoo Code strips it from the follow-up context for these providers — so the model can never see its own reasoning chain on the next turn.

Root cause

openAiR1FormatEnabled (exposed as a UI checkbox via R1FormatSetting and declared in provider-settings.ts) only forces the R1 request format in openai.ts. It never propagates to info.preserveReasoning, which is what gates reasoning retention in Task.ts:

// src/core/task/Task.ts
const shouldPreserveForApi = this.api.getModel().info.preserveReasoning === true

OpenAiHandler.getModel() builds its ModelInfo from openAiCustomModelInfo ?? openAiModelInfoSaneDefaults — neither sets preserveReasoning. For the built-in OpenAI-compatible provider the value is therefore always undefined, and reasoning is stripped from messages sent back to the API.

Fix

When the user enables the R1 format toggle, getModel() now sets preserveReasoning: true on the returned model info:

return {
    id,
    info: this.options.openAiR1FormatEnabled ? { ...info, preserveReasoning: true } : info,
    ...params,
}

This preserves reasoning_content in the assistant history sent to the model on follow-up turns. Default behaviour (toggle off) is unchanged.

Tests

Added two getModel cases to src/api/providers/__tests__/openai.spec.ts:

• preserveReasoning is true when openAiR1FormatEnabled is on
• preserveReasoning stays undefined by default

Related

• Issue: [BUG] Reasoning from local OpenAI-compatible models (llama.cpp/Ollama/LM Studio) is discarded — no official toggle #1118 (bug report for the same root cause)

Fixes #1118

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: Zoo-Code-Org/Zoo-Code/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 59469b66-5687-4993-9dd6-7815ae7ee057

📥 Commits

Reviewing files that changed from the base of the PR and between 60b1b45 and 1a47887.

📒 Files selected for processing (3)

• src/api/providers/__tests__/openai.spec.ts
• src/api/providers/openai.ts
• src/core/task/__tests__/reasoning-preservation.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📜 Recent review details

🧰 Additional context used

📓 Path-based instructions (6)

Check persistence and lifecycle invariants: awaited atomic writes, rollback or explicit partial-failure behavior, cross-window state consistency, stale listeners/watchers, cancellation, idempotency, and safe restart/resume without lost or d...

⚙️ CodeRabbit configuration file

Files:

• src/core/task/__tests__/reasoning-preservation.test.ts

Treat model, provider, MCP, path, command, and tool data as untrusted.

⚙️ CodeRabbit configuration file

Files:

• src/api/providers/__tests__/openai.spec.ts
• src/api/providers/openai.ts

Require regression coverage at the lowest valid harness with behavior-focused assertions, including relevant negative, error, false/unset, and boundary cases.

⚙️ CodeRabbit configuration file

Files:

• src/api/providers/__tests__/openai.spec.ts
• src/core/task/__tests__/reasoning-preservation.test.ts

Check strict typing and exhaustive behavior across normal, boundary, error, cancellation, retry, and compatibility paths.

⚙️ CodeRabbit configuration file

Files:

• src/api/providers/__tests__/openai.spec.ts
• src/api/providers/openai.ts
• src/core/task/__tests__/reasoning-preservation.test.ts

Verify extension/webview contracts, cancellation and error propagation, VS Code lifecycle correctness, and behavior under retries and partial failure.

⚙️ CodeRabbit configuration file

Files:

• src/api/providers/__tests__/openai.spec.ts
• src/api/providers/openai.ts
• src/core/task/__tests__/reasoning-preservation.test.ts

Act as an adversarial second-opinion reviewer.

⚙️ CodeRabbit configuration file

Files:

• src/api/providers/__tests__/openai.spec.ts
• src/api/providers/openai.ts
• src/core/task/__tests__/reasoning-preservation.test.ts

🔇 Additional comments (3)

src/api/providers/openai.ts (1)

95-95: LGTM!

Also applies to: 296-302

src/api/providers/__tests__/openai.spec.ts (1)

1121-1160: LGTM!

src/core/task/__tests__/reasoning-preservation.test.ts (1)

515-580: LGTM!

---

📝 Summary

Summary by CodeRabbit

• New Features

  • OpenAI models using the R1 format—including deepseek-reasoner models—can preserve reasoning information in conversation history alongside assistant responses.
  • Custom model settings are retained when reasoning preservation is enabled.

• Bug Fixes

  • Reasoning information remains excluded from conversation history by default, preserving existing behavior when reasoning preservation is not enabled.

Walkthrough

The OpenAI provider now uses a shared predicate for R1 format detection. When the predicate is true, getModel adds preserveReasoning: true to copied model information. Tests cover model configuration and conversation-history preservation.

Changes

OpenAI reasoning configuration

Layer / File(s)	Summary
Conditional model configuration and history preservation src/api/providers/openai.ts, src/api/providers/__tests__/openai.spec.ts, src/core/task/__tests__/reasoning-preservation.test.ts	usesR1Format returns true when the model ID contains deepseek-reasoner or the setting is enabled. createMessage and getModel use this predicate. getModel sets preserveReasoning: true on copied model information when the predicate is true. Tests cover the model ID, setting, default behavior, custom model fields, and conversation-history handling.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 1a478

The R1 setting and DeepSeek Reasoner model IDs now enable reasoning preservation, with the default behavior unchanged. No merge-blocking risk is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 1a478

The change is narrowly scoped, but switching configurations while a request is being prepared can leave reasoning included when the new configuration would normally omit it. Exposure depends on timing and is limited to the active conversation and selected destination.

Retained concerns

• Medium · security · inferred: The preservation decision is not bound to the handler that sends the request. After an R1-enabled snapshot is captured, a profile change during preparation can replace the handler with a non-R1, non-preserving configuration. The prepared history still contains reasoning, and the generic serializer forwards it as reasoning_content to the newly selected endpoint. Retries also retain the earlier metadata snapshot. This is newly reachable with default R1 settings, although the underlying handler-replacement behavior predates the PR. The interleaving is inferred from source, not runtime-reproduced.

Security review details

Security Blast Radius

• inferred — The identified exposure is the reasoning in effective history for the current task, sent through the selected handler. It requires a configuration change during preparation or retry; provider response text alone has no inspected path to choose a new endpoint. No broader tenant, service, or credential authority is established by this evidence.

Security Findings and Attack Paths

• inferred — A pending request can retain reasoning under an earlier R1 snapshot, then use a newly selected non-preserving handler. The generic converter still forwards the retained block. The concern is unintended disclosure during a user-initiated transition, not demonstrated remote configuration takeover.

Trust Boundaries and Controls

• observed — Endpoint and credentials remain determined by provider configuration. Profile mutations are queued with each other, but the inspected queue does not bind active request construction to that configuration. Task snapshots preservation metadata while handler replacement remains a separate transition.

Resilience and Maintainability Implications

• observed — Request retries reuse preservation metadata, preventing ordinary metadata-refresh drift but retaining a stale decision after handler replacement. Existing history preparation stores reasoning independently of the flag, and history cleaning constructs a separate request projection rather than deleting persisted records.

Hardening Proposals

• proposed — Bind preservation metadata, history projection, destination, and retries to one request configuration identity. When that identity changes, cancel or reconstruct pending requests rather than dispatching previously prepared history through the replacement handler. Validate transitions from R1 to non-preserving configurations across preparation and retry waits.

---

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Linked Issues check	❓ Inconclusive	#1118 requires reasoning preservation for local OpenAI-compatible models or an official R1 setting. openai.ts now sets preserveReasoning when usesR1Format is true, and tests cover that behavior …	Reviewable evidence is needed to confirm that the existing R1 setting is surfaced and passed into OpenAiHandler options for local OpenAI-compatible models.

✅ Passed checks (7 passed)

Check name	Status	Explanation
Out of Scope Changes check	✅ Passed	The provider change and tests cover #1118's reasoning-preservation behavior. The Task tests verify the existing preserve and strip paths. No unrelated changes appear in the whole-PR diff.
Regression Evidence	✅ Passed	Focused coverage exists for the changed preservation behavior. OpenAI provider tests cover the R1 toggle, deepseek-reasoner IDs, custom model-info merging, and the default unset case. Task tests exe…
Security Boundaries	✅ Passed	The changed code only enables R1-format conversion and retains reasoning in the assistant history when the model ID or R1 toggle matches (src/api/providers/openai.ts:300-324). That reasoning is sent…
Persistence Integrity	✅ Passed	No changed persistence path exists. The openai.ts diff derives preserveReasoning in the getModel() return value and shares the R1 predicate with createMessage(); it adds no storage write, roll…
Lifecycle Resource Cleanup	✅ Passed	No changed lifecycle path introduces a resource leak or duplicate work. In src/api/providers/openai.ts:296-325, the new predicate reads configuration and getModel() returns model data; neither acq…
Title check	✅ Passed	The title clearly identifies the OpenAI-compatible provider change and its goal of preserving reasoning through the R1 toggle.
Description check	✅ Passed	The description explains the root cause and fix, links issue #1118, and lists added tests. It omits the template’s explicit Test Procedure section and completed Pre-Submission Checklist, but is otherw…

Full details: Linked Issues check

Explanation

#1118 requires reasoning preservation for local OpenAI-compatible models or an official R1 setting. openai.ts now sets preserveReasoning when usesR1Format is true, and tests cover that behavior plus the deepseek-reasoner path. The provider tests pass the option directly. The PR description says a UI setting already supplies it, but the issue says the option is not wired; the changed files do not resolve this conflict.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[ch405canova-sudo]

This work is a joint effort by chaos (@ch405canova-sudo) and opencode — the bug was found and the fix developed together on a local llama.cpp stack (August 2026).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[ch405canova-sudo]

This PR addresses the same root cause as #1096 (local OpenAI-compatible reasoning is discarded because preserveReasoning is never enabled for the built-in OpenAI-compatible provider).

Note on approach: #1096 proposes a separate preserveReasoning checkbox, while this PR couples it to the existing R1-format toggle. Both solve the underlying issue — happy to align with whichever approach the maintainers prefer.

[edelauna]

Thank you for your contribution, but could we align with #1096 and include this a a checkbox in the ui, I don't feel great about reusing an unrelated flag.

[edelauna]

createMessage() treats a model as R1-format via modelId.includes("deepseek-reasoner") || enabledR1Format (line 90), but this gate only checks the toggle. If someone points openAiModelId at a custom endpoint whose id contains deepseek-reasoner without flipping the toggle, wouldn't createMessage still use R1 conversion while preserveReasoning never gets set here — leaving the original bug open for that case?

[edelauna]

This forces preserveReasoning: true whenever the toggle is on, even if openAiCustomModelInfo.preserveReasoning was explicitly set to false. Should an explicit value win here, e.g. info.preserveReasoning ?? true?

[github-actions]

This PR has been awaiting author changes for 14 days and will be automatically closed in 7 days. Please address the review comments or leave a comment if you need more time.

[github-actions]

Review status

Thanks for contributing. This comment tracks the review sequence and the next action.

Current step: Awaiting fresh human maintainer or CODEOWNER approval.

Automated review is complete for the latest commit but does not replace human approval.

Review-state labels are managed by this workflow; do not edit them manually. community-approved is managed the same way — do not add or remove it manually. It signals a fresh community code approval for the current head as an advisory priority only; maintainer review is still required.

[edelauna]

Thanks for the update - had one comment on how to handle deepseek

[edelauna]

Line 95 derives the R1 path from modelId.includes("deepseek-reasoner") || enabledR1Format, so a model id containing deepseek-reasoner goes through convertToR1Format while Task.ts still strips its reasoning (preserveReasoning stays unset here). Should both gates share one predicate?

Suggested change

	info: this.options.openAiR1FormatEnabled ? { ...info, preserveReasoning: true } : info,
	info: (id.includes("deepseek-reasoner") || this.options.openAiR1FormatEnabled) ? { ...info, preserveReasoning: true } : info,

(id is already in scope in getModel; createMessage could reuse the same condition.)

[ch405canova-sudo]

Done in 1a47887 — both gates now share a single predicate, OpenAiHandler.usesR1Format(modelId) (modelId.includes("deepseek-reasoner") || (this.options.openAiR1FormatEnabled ?? false)), so createMessage() and getModel() can no longer drift apart. Added a test covering a deepseek-reasoner model id with the toggle off.

[edelauna]

Both new tests leave openAiCustomModelInfo unset, so info is value-identical to the defaults here — the { ...info } merge only gets exercised on the defaults path. A case combining custom model info (e.g. a distinct contextWindow) with the toggle would pin the merge for the llama.cpp / LM Studio / Ollama setups this PR targets. Worth adding?

[ch405canova-sudo]

Added in 1a47887: should merge preserveReasoning into custom model info when openAiR1FormatEnabled is on builds openAiCustomModelInfo with a distinct contextWindow (32_768) and supportsImages: false, and asserts the full merged object { ...customInfo, preserveReasoning: true }, so the { ...info } merge path is actually exercised.

[edelauna]

This pins the flag, but the consumer it feeds — buildCleanConversationHistory in Task.ts (preserveReasoning === true at Task.ts:5020) — is only covered by tests that re-implement the branch (reasoning-preservation.test.ts:223, 290, 347, 398), so deleting the production gate would still pass everything. Would a regression test calling the real method be worth adding here?

[ch405canova-sudo]

Added in 1a47887: two tests in reasoning-preservation.test.ts store a plain-text reasoning block through the real addToApiConversationHistory and then call the real Task.buildCleanConversationHistory for both the preserve and the strip path. I mutation-checked them: removing the production gate fails the preserve test, inverting it fails the strip test. Note that on current main the method takes requestModelInfo as a second argument (the gate now reads requestModelInfo.preserveReasoning === true, Task.ts:5800), so the tests pass the ModelInfo through the real signature instead of stubbing api.getModel().

[LouisClt]

Hello, I'm very interested in this fix but it seems nothing has really moved on this PR for several weeks. Could we try to finish it? I can help if needed.

[ch405canova-sudo]

Update: the three review comments from Sep 11 are addressed and the branch is rebased onto the current main — all of it in 1a47887.

What changed

• OpenAiHandler.usesR1Format(modelId) is now the single predicate used by both createMessage() and getModel(), exactly as suggested: modelId.includes("deepseek-reasoner") || (openAiR1FormatEnabled ?? false). A deepseek-reasoner model id without the toggle now keeps its reasoning as well, instead of going through convertToR1Format while Task.ts stripped the chain.
• New test pinning the openAiCustomModelInfo + toggle merge: distinct contextWindow (32_768) / supportsImages: false are preserved alongside preserveReasoning: true.
• New regression tests calling the real Task.buildCleanConversationHistory (preserve and strip path) instead of re-implementing the branch. Mutation-verified: deleting the production gate fails the preserve test, inverting it fails the strip test. On current main the method takes requestModelInfo as a second argument, so the tests exercise the real signature and the gate at Task.ts:5800.

Verification

• vitest: openai.spec.ts + reasoning-preservation.test.ts green; full src suite green except the pre-existing dist_assets.spec.ts failures (they need a built dist/, unrelated to this PR)
• tsc --noEmit clean, pnpm lint clean across all packages

@edelauna — re-requesting your review.

@LouisClt — thanks for the ping, and for filing #1665. That one (Bedrock "[Unknown Block Type]" corruption + MiniMax sanitization) has a different root cause in bedrock-converse-format.ts / minimax.ts and is not touched by this PR — it's also not blocked by it, the two fixes are independent. Happy to coordinate if you want to take the Bedrock/MiniMax one; resolving path #1 there (teaching the Converse converter reasoning/thinking blocks) is a self-contained change.