fix: fixes forwarding of reasoning content while conversion of Responses to Chat - #3584
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughExport now emits assistant reasoning as standalone Responses reasoning messages; import buffers incoming reasoning and attaches it to the next assistant chat turn. Tests validate ordering and attachment. UI rendering gains defensive checks against falsy history entries. ChangesReasoning message handling and UI safety
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/app/workspace/logs/views/columns.tsx`:
- Around line 86-94: When building the preview from log.input_history or
log.responses_input_history (variables: input_history, responses_input_history,
lastInput, lastMessage), don't assume the final array slot is non-null; instead
walk the array backwards to find the last truthy entry (e.g., loop from length-1
down to 0 or use .slice().reverse().find(Boolean)) and then pass that entry's
content into getMessageFromContent; if none found, return ""—apply this for both
input_history and responses_input_history branches so earlier valid content
isn't dropped when the last element is null/undefined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6b13709-d379-444a-871b-ee55bfbf6be5
📒 Files selected for processing (4)
core/schemas/mux.gocore/schemas/mux_test.goui/app/workspace/logs/sheets/logDetailView.tsxui/app/workspace/logs/views/columns.tsx
Confidence Score: 4/5The core conversion logic is sound for all well-formed inputs; the only gap is an end-of-sequence edge case where buffered reasoning is silently dropped if no assistant turn follows. The bidirectional buffering and flush logic works correctly for all inputs covered by the new tests. The only gap is an end-of-sequence edge case in ToChatMessages where buffered reasoning is dropped if no assistant turn follows; this is benign in practice today but could mask future issues. core/schemas/mux.go — post-loop handling of orphaned pending reasoning. Important Files Changed
Reviews (2): Last reviewed commit: "fix: fixes forwarding of reasoning conte..." | Re-trigger Greptile |
8c26def to
0773eb1
Compare
Merge activity
|
The base branch was changed.
…ses to Chat (#3584) ## Summary Reasoning content (e.g. from DeepSeek thinking mode or Anthropic extended thinking) was being silently dropped when converting between the Responses and Chat message formats. This caused multi-turn flows that route through the Responses→Chat fallback path to 400 on providers that require reasoning content to be echoed back. This PR fixes the bidirectional conversion so reasoning is preserved across both directions. ## Changes - **`ToResponsesMessages`**: Reasoning content on an assistant `ChatMessage` is now emitted as a `reasoning`-typed `ResponsesMessage` *before* any tool calls or text content, matching the order providers expect and allowing clients to echo it back correctly. - **`ToChatMessages`**: Reasoning messages are no longer silently skipped. Instead, they are buffered and attached to the next assistant turn (whether that turn carries text content or tool calls), populating both `Reasoning` and `ReasoningDetails` fields including signatures, summaries, and encrypted content. - Null-safety guards were added in the UI log detail view and column helpers to prevent crashes when `input_history` contains `null`/`undefined` entries (which can occur when reasoning messages are injected into history). ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go version go test ./... ``` Specific test cases added: - `TestToChatMessages_AttachesReasoningToNextAssistantMessage` — verifies reasoning is buffered and attached to the following assistant text message, including signature preservation. - `TestToChatMessages_AttachesReasoningToToolCallAssistantMessage` — verifies reasoning is attached to tool-call assistant messages. - `TestToResponsesMessages_EmitsReasoningMessageBeforeToolCalls` — verifies reasoning is emitted first when an assistant message has both reasoning and tool calls. - `TestToResponsesMessages_EmitsReasoningMessageBeforeTextContent` — verifies reasoning is emitted first when an assistant message has both reasoning and text content. ## Breaking changes - [ ] Yes - [x] No ## Related issues Fixes multi-turn DeepSeek thinking mode and Anthropic extended thinking flows that were returning 400 errors due to missing reasoning content in echoed history. ## Security considerations No auth, secrets, PII, or sandboxing implications. Reasoning content is treated the same as other message content. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
- maximhq#4780: tool_search_tool_result was silently dropped on /v1/responses (streaming, non-streaming ingest, and Anthropic egress/replay). Calls are keyed by tool_use ID rather than a single slot, since Claude can emit multiple tool_search calls before any of their results arrive; caller provenance (code-execution-spawned searches) round-trips correctly. - maximhq#3233: tools[].strict is now dropped only for Anthropic-family providers that don't support it (Vertex), and kept for native Anthropic. - maximhq#3802: added a regression test confirming reasoning_content survives assistant tool-call turns with extended thinking (already fixed by maximhq#3584). - web_search_call: same bug class as maximhq#4780 — missing from response.completed and vulnerable to the same multi-call concurrency bug; fixed identically.
- maximhq#4780: tool_search_tool_result was silently dropped on /v1/responses (streaming, non-streaming ingest, and Anthropic egress/replay). Calls are keyed by tool_use ID rather than a single slot, since Claude can emit multiple tool_search calls before any of their results arrive; caller provenance (code-execution-spawned searches) round-trips correctly. - maximhq#3233: tools[].strict is now dropped only for Anthropic-family providers that don't support it (Vertex), and kept for native Anthropic. - maximhq#3802: added a regression test confirming reasoning_content survives assistant tool-call turns with extended thinking (already fixed by maximhq#3584). - web_search_call: same bug class as maximhq#4780 — missing from response.completed and vulnerable to the same multi-call concurrency bug; fixed identically.
- maximhq#4780: tool_search_tool_result was silently dropped on /v1/responses (streaming, non-streaming ingest, and Anthropic egress/replay). Calls are keyed by tool_use ID rather than a single slot, since Claude can emit multiple tool_search calls before any of their results arrive; caller provenance (code-execution-spawned searches) round-trips correctly. - maximhq#3233: tools[].strict is now dropped only for Anthropic-family providers that don't support it (Vertex), and kept for native Anthropic. - maximhq#3802: added a regression test confirming reasoning_content survives assistant tool-call turns with extended thinking (already fixed by maximhq#3584). - web_search_call: same bug class as maximhq#4780 — missing from response.completed and vulnerable to the same multi-call concurrency bug; fixed identically.
- maximhq#4780: tool_search_tool_result was silently dropped on /v1/responses (streaming, non-streaming ingest, and Anthropic egress/replay). Calls are keyed by tool_use ID rather than a single slot, since Claude can emit multiple tool_search calls before any of their results arrive; caller provenance (code-execution-spawned searches) round-trips correctly. - maximhq#3233: tools[].strict is now dropped only for Anthropic-family providers that don't support it (Vertex), and kept for native Anthropic. - maximhq#3802: added a regression test confirming reasoning_content survives assistant tool-call turns with extended thinking (already fixed by maximhq#3584). - web_search_call: same bug class as maximhq#4780 — missing from response.completed and vulnerable to the same multi-call concurrency bug; fixed identically.
…ses to Chat (maximhq#3584) ## Summary Reasoning content (e.g. from DeepSeek thinking mode or Anthropic extended thinking) was being silently dropped when converting between the Responses and Chat message formats. This caused multi-turn flows that route through the Responses→Chat fallback path to 400 on providers that require reasoning content to be echoed back. This PR fixes the bidirectional conversion so reasoning is preserved across both directions. ## Changes - **`ToResponsesMessages`**: Reasoning content on an assistant `ChatMessage` is now emitted as a `reasoning`-typed `ResponsesMessage` *before* any tool calls or text content, matching the order providers expect and allowing clients to echo it back correctly. - **`ToChatMessages`**: Reasoning messages are no longer silently skipped. Instead, they are buffered and attached to the next assistant turn (whether that turn carries text content or tool calls), populating both `Reasoning` and `ReasoningDetails` fields including signatures, summaries, and encrypted content. - Null-safety guards were added in the UI log detail view and column helpers to prevent crashes when `input_history` contains `null`/`undefined` entries (which can occur when reasoning messages are injected into history). ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go version go test ./... ``` Specific test cases added: - `TestToChatMessages_AttachesReasoningToNextAssistantMessage` — verifies reasoning is buffered and attached to the following assistant text message, including signature preservation. - `TestToChatMessages_AttachesReasoningToToolCallAssistantMessage` — verifies reasoning is attached to tool-call assistant messages. - `TestToResponsesMessages_EmitsReasoningMessageBeforeToolCalls` — verifies reasoning is emitted first when an assistant message has both reasoning and tool calls. - `TestToResponsesMessages_EmitsReasoningMessageBeforeTextContent` — verifies reasoning is emitted first when an assistant message has both reasoning and text content. ## Breaking changes - [ ] Yes - [x] No ## Related issues Fixes multi-turn DeepSeek thinking mode and Anthropic extended thinking flows that were returning 400 errors due to missing reasoning content in echoed history. ## Security considerations No auth, secrets, PII, or sandboxing implications. Reasoning content is treated the same as other message content. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
…ses to Chat (maximhq#3584) ## Summary Reasoning content (e.g. from DeepSeek thinking mode or Anthropic extended thinking) was being silently dropped when converting between the Responses and Chat message formats. This caused multi-turn flows that route through the Responses→Chat fallback path to 400 on providers that require reasoning content to be echoed back. This PR fixes the bidirectional conversion so reasoning is preserved across both directions. ## Changes - **`ToResponsesMessages`**: Reasoning content on an assistant `ChatMessage` is now emitted as a `reasoning`-typed `ResponsesMessage` *before* any tool calls or text content, matching the order providers expect and allowing clients to echo it back correctly. - **`ToChatMessages`**: Reasoning messages are no longer silently skipped. Instead, they are buffered and attached to the next assistant turn (whether that turn carries text content or tool calls), populating both `Reasoning` and `ReasoningDetails` fields including signatures, summaries, and encrypted content. - Null-safety guards were added in the UI log detail view and column helpers to prevent crashes when `input_history` contains `null`/`undefined` entries (which can occur when reasoning messages are injected into history). ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [x] UI (React) - [ ] Docs ## How to test ```sh # Core/Transports go version go test ./... ``` Specific test cases added: - `TestToChatMessages_AttachesReasoningToNextAssistantMessage` — verifies reasoning is buffered and attached to the following assistant text message, including signature preservation. - `TestToChatMessages_AttachesReasoningToToolCallAssistantMessage` — verifies reasoning is attached to tool-call assistant messages. - `TestToResponsesMessages_EmitsReasoningMessageBeforeToolCalls` — verifies reasoning is emitted first when an assistant message has both reasoning and tool calls. - `TestToResponsesMessages_EmitsReasoningMessageBeforeTextContent` — verifies reasoning is emitted first when an assistant message has both reasoning and text content. ## Breaking changes - [ ] Yes - [x] No ## Related issues Fixes multi-turn DeepSeek thinking mode and Anthropic extended thinking flows that were returning 400 errors due to missing reasoning content in echoed history. ## Security considerations No auth, secrets, PII, or sandboxing implications. Reasoning content is treated the same as other message content. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable

Summary
Reasoning content (e.g. from DeepSeek thinking mode or Anthropic extended thinking) was being silently dropped when converting between the Responses and Chat message formats. This caused multi-turn flows that route through the Responses→Chat fallback path to 400 on providers that require reasoning content to be echoed back. This PR fixes the bidirectional conversion so reasoning is preserved across both directions.
Changes
ToResponsesMessages: Reasoning content on an assistantChatMessageis now emitted as areasoning-typedResponsesMessagebefore any tool calls or text content, matching the order providers expect and allowing clients to echo it back correctly.ToChatMessages: Reasoning messages are no longer silently skipped. Instead, they are buffered and attached to the next assistant turn (whether that turn carries text content or tool calls), populating bothReasoningandReasoningDetailsfields including signatures, summaries, and encrypted content.input_historycontainsnull/undefinedentries (which can occur when reasoning messages are injected into history).Type of change
Affected areas
How to test
Specific test cases added:
TestToChatMessages_AttachesReasoningToNextAssistantMessage— verifies reasoning is buffered and attached to the following assistant text message, including signature preservation.TestToChatMessages_AttachesReasoningToToolCallAssistantMessage— verifies reasoning is attached to tool-call assistant messages.TestToResponsesMessages_EmitsReasoningMessageBeforeToolCalls— verifies reasoning is emitted first when an assistant message has both reasoning and tool calls.TestToResponsesMessages_EmitsReasoningMessageBeforeTextContent— verifies reasoning is emitted first when an assistant message has both reasoning and text content.Breaking changes
Related issues
Fixes multi-turn DeepSeek thinking mode and Anthropic extended thinking flows that were returning 400 errors due to missing reasoning content in echoed history.
Security considerations
No auth, secrets, PII, or sandboxing implications. Reasoning content is treated the same as other message content.
Checklist
docs/contributing/README.mdand followed the guidelines