Skip to content

fix(inference): tolerate both reasoning and reasoning_content keys (#3547)#3552

Merged
senamakel merged 2 commits into
tinyhumansai:mainfrom
oxoxDev:fix/3547-reasoning-content-duplicate-field
Jun 9, 2026
Merged

fix(inference): tolerate both reasoning and reasoning_content keys (#3547)#3552
senamakel merged 2 commits into
tinyhumansai:mainfrom
oxoxDev:fix/3547-reasoning-content-duplicate-field

Conversation

@oxoxDev

@oxoxDev oxoxDev commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Custom OpenAI-compatible providers that emit both reasoning and reasoning_content in the same message no longer fail to parse.
  • Replaces the serde alias = "reasoning" on ResponseMessage and StreamDelta with a manual Deserialize that folds the two wire keys into the single canonical field.
  • Restores response delivery for affected providers (the model's entire reply was being dropped).

Problem

ResponseMessage and StreamDelta in src/openhuman/inference/provider/compatible_types.rs declared:

#[serde(default, alias = "reasoning")]
pub(crate) reasoning_content: Option<String>,

serde's alias maps both reasoning and reasoning_content onto the same field slot. When a provider (some OpenRouter / vLLM-SGLang proxies) sends both keys in one object, the derived visitor sets the slot on the first key and returns duplicate field \reasoning_content`on the second — failing the whole response parse and surfacing ascustom response parse error: duplicate field `reasoning_content``. The user's turn errors out with no reply.

Regression from the alias added in #3124; before that an extra reasoning key was an unknown field and silently ignored.

Solution

  • Drop the alias; add a manual impl Deserialize on both structs backed by a private shadow struct that reads reasoning and reasoning_content as distinct optional fields.
  • Fold to the single public field via reasoning_content.or(reasoning) — canonical wins when both are present, alias-only still folds in, neither stays None.
  • All existing readers and the tool-call echo-back path are unchanged (still one reasoning_content field); Serialize on ResponseMessage is untouched.

Submission Checklist

If a section does not apply to this change, mark the item as N/A with a one-line reason. Do not delete items.

  • Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
  • Diff coverage ≥ 80% — verified by CI (Coverage Gate green); changed lines are the manual Deserialize impls, both exercised by the added tests.
  • N/A: behaviour-only change — no feature rows added/removed/renamed in the coverage matrix
  • N/A: bug fix, no matrix feature IDs affected
  • No new external network dependencies introduced (mock backend used per Testing Strategy)
  • N/A: parsing-only change, does not touch release-cut smoke surfaces
  • Linked issue closed via Closes #NNN in the ## Related section

Impact

  • Desktop/CLI inference path for OpenAI-compatible custom providers. Restores responses from thinking-model proxies that echo both reasoning keys.
  • No performance, migration, or security implications. No API or wire-format change (deserialize is strictly more tolerant; serialize unchanged).

Related


AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: fix/3547-reasoning-content-duplicate-field
  • Commit SHA: 2bc0f6d

Validation Run

  • N/A: pnpm --filter openhuman-app format:check — no frontend changes
  • N/A: pnpm typecheck — no frontend changes
  • Focused tests: cargo test --lib openhuman::inference::provider::compatible (159 passed); cargo test --lib both_present (new regression tests pass)
  • Rust fmt/check (if changed): cargo fmt --check clean; cargo clippy --lib clean on changed files
  • N/A: Tauri fmt/check — app/src-tauri not changed

Validation Blocked

  • command: N/A
  • error: N/A
  • impact: N/A

Behavior Changes

  • Intended behavior change: responses carrying both reasoning keys parse instead of erroring.
  • User-visible effect: affected custom providers return replies again instead of a parse error.

Parity Contract

Duplicate / Superseded PR Handling

  • Duplicate PR(s): none
  • Canonical PR: this PR
  • Resolution: N/A

Summary by CodeRabbit

  • Bug Fixes

    • Improved compatibility with provider responses that include reasoning fields in alternate formats. The system now gracefully handles both field variants without parsing errors.
  • Tests

    • Added regression tests to verify robust handling of reasoning content in both streaming and non-streaming response scenarios.

oxoxDev added 2 commits June 9, 2026 18:17
…inyhumansai#3547)

Custom OpenAI-compatible providers (some OpenRouter / vLLM-SGLang proxies)
emit both `reasoning` and `reasoning_content` in the same message object.
The serde `alias = "reasoning"` mapped both wire keys onto one field slot,
so the derived visitor failed the whole response with
`duplicate field `reasoning_content`` and dropped the model's entire reply.

Replace the alias with a manual Deserialize on ResponseMessage and StreamDelta
that reads the two names as distinct fields and folds them into the single
canonical `reasoning_content` (canonical wins when both are present). Accepts
any combination; reasoning-only providers keep working.
…nyhumansai#3547)

Cover the TAURI-RUST-A5N failure on both the chat and stream-delta paths:
a message carrying both `reasoning` and `reasoning_content` must parse
without a duplicate-field error, and the canonical field wins.
@oxoxDev oxoxDev requested a review from a team June 9, 2026 12:48
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR fixes a deserialization failure where OpenAI-compatible providers emit both reasoning and reasoning_content keys in the same JSON object. Manual Deserialize implementations for ResponseMessage and StreamDelta now accept both keys and fold them to the canonical reasoning_content field. Regression tests validate both paths.

Changes

OpenAI-compatible dual reasoning field deserialization

Layer / File(s) Summary
ResponseMessage manual deserialization with dual-field folding
src/openhuman/inference/provider/compatible_types.rs
Added Deserializer to Serde imports. Replaced derived Deserialize for ResponseMessage with manual impl<'de> Deserialize<'de> that deserializes reasoning_content and reasoning as separate shadow fields and selects reasoning_content as canonical, preventing duplicate-field errors.
StreamDelta manual deserialization with dual-field folding
src/openhuman/inference/provider/compatible_types.rs
Replaced derived Deserialize for StreamDelta with manual impl<'de> Deserialize<'de> using the same dual-field folding pattern as ResponseMessage, ensuring consistent handling of reasoning_content and reasoning in streaming deltas.
Regression tests for dual reasoning field variants
src/openhuman/inference/provider/compatible_tests.rs
Added two regression tests: one verifying ApiChatResponse parsing tolerates both reasoning and reasoning_content keys without error, and one verifying the same for StreamChunkResponse, confirming canonical reasoning_content selection.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 When two fields waltz in from the wire,
reasoning dances with reasoning_content's fire!
Now deserialization bows without duplicate despair—
One canonical value floats gently through the air. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: fixing deserialization to tolerate both reasoning and reasoning_content keys in provider responses.
Linked Issues check ✅ Passed The PR implementation fully addresses issue #3547 by replacing serde alias with manual Deserialize that accepts both reasoning and reasoning_content keys without duplicate-field errors.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the duplicate-field parsing issue: manual Deserialize implementations in compatible_types.rs and regression tests in compatible_tests.rs.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

@senamakel

Copy link
Copy Markdown
Member

thanks for this...

@senamakel senamakel merged commit 4107c4e into tinyhumansai:main Jun 9, 2026
36 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Custom provider response parse fails: duplicate field reasoning_content when provider emits both reasoning and reasoning_content

2 participants