Skip to content

fix: encode thinking content parts as reasoning_content for OpenAI-compatible providers#647

Merged
mikehostetler merged 2 commits intoagentjido:mainfrom
spfoos:fix/encode-reasoning-content
Apr 26, 2026
Merged

fix: encode thinking content parts as reasoning_content for OpenAI-compatible providers#647
mikehostetler merged 2 commits intoagentjido:mainfrom
spfoos:fix/encode-reasoning-content

Conversation

@spfoos
Copy link
Copy Markdown
Contributor

@spfoos spfoos commented Apr 25, 2026

When assistant messages contain ContentPart.thinking(...) parts (e.g. from Moonshot reasoning models), the OpenAI encoder was stripping them via encode_openai_content_part returning nil for :thinking parts.

This caused multi-turn conversations with Moonshot to fail with:

thinking is enabled but reasoning_content is missing in assistant tool call message

Changes:

  • Add extract_reasoning_content/1 to pull thinking parts out of message content
  • In encode_openai_message, extract reasoning_content before encoding content, then add it as a top-level reasoning_content field on the encoded message
  • Remove thinking parts from the content list so they don't get encoded as nil

This matches the OpenAI/Moonshot API contract where reasoning_content lives at the message level, not inside the content array.

…mpatible providers

When assistant messages contain ContentPart.thinking(...) parts (e.g. from
Moonshot reasoning models), the OpenAI encoder was stripping them via
encode_openai_content_part returning nil for :thinking parts.

This caused multi-turn conversations with Moonshot to fail with:
> thinking is enabled but reasoning_content is missing in assistant tool call message

Changes:
- Add extract_reasoning_content/1 to pull thinking parts out of message content
- In encode_openai_message, extract reasoning_content before encoding content,
  then add it as a top-level reasoning_content field on the encoded message
- Remove thinking parts from the content list so they don't get encoded as nil

This matches the OpenAI/Moonshot API contract where reasoning_content lives
at the message level, not inside the content array.
@mikehostetler mikehostetler added the needs_work Changes requested before merge label Apr 25, 2026
@mikehostetler
Copy link
Copy Markdown
Contributor

Thanks for the focused fix here. I marked this needs_work because the encoder change needs a small regression test proving assistant thinking parts are emitted as top-level reasoning_content and removed from the content array. It would also help to either scope that field to the OpenAI-compatible providers/models that accept it, or add a test/note showing the shared encoder behavior is intentional across those providers. Appreciate the contribution; this looks close once that coverage and scope are tightened up.

spfoos pushed a commit to spfoos/req_llm that referenced this pull request Apr 25, 2026
- Only emit reasoning_content for :assistant messages to avoid
  sending it on user/system/tool roles where it is not expected.
- Update stale comment about thinking parts in Defaults encoder.
- Add regression tests:
  - thinking parts become top-level reasoning_content
  - multiple thinking parts are joined
  - non-assistant roles do not get reasoning_content
  - tool_calls preserved alongside reasoning_content

Addresses review feedback on PR agentjido#647.
- Only emit reasoning_content for :assistant messages to avoid
  sending it on user/system/tool roles where it is not expected.
- Update stale comment about thinking parts in Defaults encoder.
- Add regression tests:
  - thinking parts become top-level reasoning_content
  - multiple thinking parts are joined
  - non-assistant roles do not get reasoning_content
  - tool_calls preserved alongside reasoning_content

Addresses review feedback on PR agentjido#647.
@spfoos spfoos force-pushed the fix/encode-reasoning-content branch from 35e8f6f to c66403e Compare April 25, 2026 19:32
@mikehostetler mikehostetler added ready_to_merge and removed needs_work Changes requested before merge labels Apr 26, 2026
@mikehostetler mikehostetler merged commit 735bc83 into agentjido:main Apr 26, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants