Return reasoning content from model output#112
Open
zoeshawwang wants to merge 1 commit into
Open
Conversation
Protocol conversion should use the returned reasoning payload as the source of truth. MODEL_PARAMETER_RULES can still control model-side thinking, but OpenAI and AG-UI responses should not hide non-empty reasoning_content when the env flag says thinking is false. Constraint: User requested removal of protocol-level thinking_enabled = is_thinking_enabled_from_env() gating. Rejected: Keep env-based response suppression | runtime parameters can drift from the model output and hide returned reasoning. Confidence: high Scope-risk: narrow Directive: Do not reintroduce protocol-level reasoning env gates; only emit reasoning when returned reasoning_content is non-empty. Tested: uv run --python 3.11 --dev --extra server pytest -q tests/unittests/server/test_openai_protocol.py tests/unittests/server/test_agui_protocol.py tests/unittests/server/test_reasoning.py Tested: uv run --python 3.11 --dev --extra server pytest -q tests/unittests/server Tested: uv run --python 3.11 --dev --extra server ruff check agentrun/server/openai_protocol.py agentrun/server/agui_protocol.py tests/unittests/server/test_openai_protocol.py tests/unittests/server/test_agui_protocol.py Tested: git diff --check Change-Id: I638efa7ca19bf8ed9417fb1922d43205d4d52b65 Not-tested: Remote GitHub CI result pending after push.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts the OpenAI Chat Completions and AG-UI protocol handlers to surface reasoning_content based on actual model output (non-empty) rather than gating at the protocol layer via MODEL_PARAMETER_RULES. It also strips transport-only additional_kwargs.reasoning_content from emitted protocol payloads while still promoting it into the standardized reasoning field/event when present.
Changes:
- Removed protocol-layer
MODEL_PARAMETER_RULESgating for reasoning emission in OpenAI and AG-UI protocol conversions. - Promoted
reasoning_contentfromadditional_kwargs(and stripped the nested transport field), emitting reasoning only when non-empty. - Updated unit tests to assert reasoning is included even when
MODEL_PARAMETER_RULESdisables thinking.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
agentrun/server/openai_protocol.py |
Removes protocol gating; promotes and cleans reasoning fields in OpenAI streaming/non-stream responses. |
agentrun/server/agui_protocol.py |
Removes protocol gating; emits AG-UI reasoning events when non-empty and strips transport-only reasoning fields from additions. |
tests/unittests/server/test_openai_protocol.py |
Updates/extends unit coverage for reasoning emission and promotion when thinking is disabled. |
tests/unittests/server/test_agui_protocol.py |
Updates unit coverage for reasoning events and ordering when thinking is disabled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
344
to
+348
| if event.event == EventType.REASONING: | ||
| if thinking_enabled: | ||
| reasoning_content = event.data.get("delta", "") | ||
| if reasoning_content: | ||
| has_text = True | ||
| yield self._build_chunk( | ||
| context, | ||
| {"reasoning_content": reasoning_content}, | ||
| ) | ||
| reasoning_content = event.data.get("delta", "") | ||
| if reasoning_content: | ||
| has_text = True | ||
| yield self._build_chunk( |
Comment on lines
480
to
484
| if event.event == EventType.TEXT: | ||
| content_parts.append(event.data.get("delta", "")) | ||
| reasoning_content = get_reasoning_content(event.addition or {}) | ||
| if thinking_enabled and reasoning_content: | ||
| if reasoning_content: | ||
| reasoning_parts.append(reasoning_content) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MODEL_PARAMETER_RULESgating from OpenAI Chat Completions and AG-UI reasoning output conversion.reasoning_contentpayload is non-empty.additional_kwargs.reasoning_content, while stripping the nested transport-only field from protocol payloads.Validation
uv run --python 3.11 --dev --extra server pytest -q tests/unittests/server/test_openai_protocol.py tests/unittests/server/test_agui_protocol.py tests/unittests/server/test_reasoning.pyuv run --python 3.11 --dev --extra server pytest -q tests/unittests/serveruv run --python 3.11 --dev --extra server ruff check agentrun/server/openai_protocol.py agentrun/server/agui_protocol.py tests/unittests/server/test_openai_protocol.py tests/unittests/server/test_agui_protocol.pygit diff --checkNotes
MODEL_PARAMETER_RULEScan still be used by the runtime/model-call layer to control whether the model thinks. The SDK response protocol layer now trusts the actual model output: if reasoning is returned, it is surfaced; if reasoning is absent or empty, no reasoning field/event is emitted.