Skip to content

Allow to set reasoning_effort and capture reasoning content#1447

Open
nforro wants to merge 3 commits into
i-am-bee:mainfrom
nforro:reasoning
Open

Allow to set reasoning_effort and capture reasoning content#1447
nforro wants to merge 3 commits into
i-am-bee:mainfrom
nforro:reasoning

Conversation

@nforro
Copy link
Copy Markdown
Contributor

@nforro nforro commented May 11, 2026

Description

For models with thinking enabled, LiteLLM exposes the thoughts as reasoning_content. This PR captures it within AssistantMessage. To be able to enable/control model thinking, ChatModelOptions is extended with reasoning_effort.

This can be useful especially when not using/forcing the Think tool.

Which issue(s) does this pull-request address?

Closes: #1448

Checklist

General

Code quality checks

  • Code quality checks pass: mise check (mise fix to auto-fix)

Testing

  • Unit tests pass: mise test:unit
  • E2E tests pass: mise test:e2e
  • Tests are included (for bug fixes or new features)

Documentation

  • Documentation is updated
  • Embedme embeds code examples in docs. To update after edits, run: Python mise docs:fix

@nforro nforro requested a review from a team as a code owner May 11, 2026 12:20
@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label May 11, 2026
@github-actions github-actions Bot added the python Python related functionality label May 11, 2026
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for model reasoning content by adding a MessageReasoningContent type and updating AssistantMessage to include reasoning blocks. It also adds a reasoning_effort parameter to chat model options and enhances the LiteLLM adapter to handle reasoning and Anthropic-specific thinking_blocks. Review feedback identifies a logic error in the LiteLLM adapter where metadata could be lost if content parts are empty, suggests making text and tool calls non-exclusive, and recommends adding missing imports for improved type safety.

Comment thread python/beeai_framework/adapters/litellm/chat.py Outdated
Comment thread python/beeai_framework/adapters/litellm/chat.py
@nforro
Copy link
Copy Markdown
Contributor Author

nforro commented May 11, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for model reasoning by adding MessageReasoningContent to the message framework and a reasoning_effort option to the chat model. It also updates the LiteLLM adapter to handle reasoning content and thinking_blocks for conversation history. A potential issue was identified in the LiteLLM adapter where the use of model_dump() might filter out necessary metadata like thinking_blocks if they are not explicitly defined as model fields; simplifying the condition to check for the existence of the update object is recommended.

Comment thread python/beeai_framework/adapters/litellm/chat.py Outdated
@jpodivin
Copy link
Copy Markdown

This would partly address the #1448, but we would still lose the content if a tool_call was present in the delta. Could you integrate relevant parts of main...jpodivin:beeai-framework:fix/reasoning-content-drop into this?

@nforro
Copy link
Copy Markdown
Contributor Author

nforro commented May 11, 2026

This would partly address the #1448, but we would still lose the content if a tool_call was present in the delta.

I assume that issue has been resolved when addressing this feedback from gemini-code-assist:
#1447 (comment)

Could you integrate relevant parts of main...jpodivin:beeai-framework:fix/reasoning-content-drop into this?

Let me at least include the tests.

@nforro nforro force-pushed the reasoning branch 2 times, most recently from 787ab80 to b5ef17d Compare May 11, 2026 13:25
@nforro
Copy link
Copy Markdown
Contributor Author

nforro commented May 11, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for model reasoning and 'thinking blocks' by adding MessageReasoningContent, updating AssistantMessage to handle reasoning parts, and including reasoning_effort parameters. The LiteLLM adapter is updated to extract reasoning content and manage signed thinking blocks for Anthropic models. Feedback suggests adding a type check for thinking_blocks in the LiteLLM adapter to prevent potential runtime errors during iteration.

Comment thread python/beeai_framework/adapters/litellm/chat.py Outdated
Comment thread python/beeai_framework/backend/message.py Outdated
Comment thread python/beeai_framework/adapters/litellm/chat.py Outdated
Copy link
Copy Markdown
Contributor

@Tomas2D Tomas2D left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great improvement. I am just worried whether LiteLLM handles the parameter for providers where thinkink_blocks attribute is not supported. Did you test it? If not I can do that.

@nforro
Copy link
Copy Markdown
Contributor Author

nforro commented May 13, 2026

I am just worried whether LiteLLM handles the parameter for providers where thinkink_blocks attribute is not supported. Did you test it? If not I can do that.

Good point, I didn't think of that. I didn't test it.

nforro and others added 3 commits May 13, 2026 14:31
Signed-off-by: Nikola Forró <nforro@redhat.com>
Assisted-by: Claude Opus 4.6 via Claude Code
…tions

Signed-off-by: Nikola Forró <nforro@redhat.com>
Assisted-by: Claude Opus 4.6 via Claude Code
Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
Co-authored-by: Nikola Forró <nforro@redhat.com>
@jpodivin
Copy link
Copy Markdown

I am just worried whether LiteLLM handles the parameter for providers where thinkink_blocks attribute is not supported. Did you test it? If not I can do that.

Good point, I didn't think of that. I didn't test it.

Perhaps we could implement something in littellm.

@nforro
Copy link
Copy Markdown
Contributor Author

nforro commented May 13, 2026

Sorry, I'm not following, implement what in LiteLLM? I think I misunderstood the comment - are we talking about setting thinking_blocks attribute in assistant messages in _transform_input()? That should be stripped by exclude_none() if there are no thinking blocks in meta, and thinking blocks can only be there if we got them from an Anthropic chat model in the first place.

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

Labels

python Python related functionality size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The _transform_output method of ChatModel may discard text content or override reasoning content with possible side effect of AttributeError

3 participants