Skip to content

fix(responses): preserve polymorphic reasoning and Codex metadata - #2178

Merged
looplj merged 3 commits into
looplj:unstablefrom
MedAIerHHL:codex/fix-reasoning-content-polymorphic
Aug 10, 2026
Merged

fix(responses): preserve polymorphic reasoning and Codex metadata#2178
looplj merged 3 commits into
looplj:unstablefrom
MedAIerHHL:codex/fix-reasoning-content-polymorphic

Conversation

@MedAIerHHL

@MedAIerHHL MedAIerHHL commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • accept both string and array forms of Responses reasoning_content
  • preserve the original representation during JSON round trips
  • forward an explicit allowlist of Codex Responses metadata headers when body pass-through is active
  • keep credentials and transport headers excluded from forwarding

Why

Some Chat-compatible Responses upstreams attach a plain string reasoning_content to function-call items, while native Responses reasoning items use an array. Rejecting the string form breaks otherwise valid responses. In addition, dropping Codex protocol metadata while replaying the original body can change upstream behavior.

Verification

  • added transformer coverage for string, array, null, and invalid reasoning content
  • added orchestrator coverage for allowed metadata and excluded sensitive headers
  • no additional build or lint command was run while preparing this PR, following the repository workflow rules

Summary by CodeRabbit

  • New Features
    • Preserved approved Codex Responses metadata during pass-through requests.
    • Added support for reasoning content represented as either text or structured content.
  • Bug Fixes
    • Prevented unapproved, sensitive, or transport-specific headers from being forwarded.
    • Improved compatibility with valid Responses request formats.
  • Tests
    • Added coverage for metadata forwarding, header filtering, disabled pass-through, and reasoning-content formats.

aiusage added 3 commits August 8, 2026 20:41
Chat-compatible upstreams (e.g. CC-Switch bridge) return
reasoning_content as a plain string on function_call items. The
native Responses API uses an array of {type, text} objects.

Add PolymorphicReasoningContent (same pattern as Input) so both
shapes unmarshal without errors.
Copilot AI lite review requested due to automatic review settings August 9, 2026 07:56
@coderabbitai

coderabbitai Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change adds allowlisted Codex Responses metadata header forwarding for eligible pass-through requests. It also changes reasoning_content handling to support string, array, and null JSON representations with validation.

Changes

Pass-through request headers

Layer / File(s) Summary
Allowlisted header forwarding and validation
internal/server/orchestrator/orchestrator.go, internal/server/orchestrator/pass_through.go, internal/server/orchestrator/pass_through_test.go
The orchestrator applies header pass-through after request-body handling. The middleware copies approved inbound metadata headers and excludes provider authorization, transport headers, and unapproved headers. Tests cover active, inactive, and mismatched request formats.

Polymorphic reasoning content

Layer / File(s) Summary
Polymorphic reasoning content JSON handling
llm/transformer/openai/responses/model.go, llm/transformer/openai/responses/model_test.go
Item.ReasoningContent accepts string or array representations through PolymorphicReasoningContent. JSON handling supports string, array, and null values and rejects malformed objects.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Suggested reviewers: looplj, ttttmr

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes both primary changes: polymorphic reasoning support and Codex metadata preservation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the OpenAI Responses transformer and orchestrator pass-through behavior to better preserve upstream semantics: it supports polymorphic reasoning_content shapes in Responses items and forwards a curated set of Codex protocol metadata headers when the original request body is replayed.

Changes:

  • Update Responses Item.reasoning_content to accept either a string (Chat-compatible function-call items) or an array (native reasoning items), and preserve the original representation during JSON round trips.
  • Add transformer tests covering string/array/null/invalid reasoning_content cases.
  • Forward an explicit allowlist of Codex Responses metadata headers when request body pass-through is active, while keeping sensitive headers excluded (tested).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
llm/transformer/openai/responses/model.go Introduces a polymorphic JSON type for reasoning_content to accept string-or-array and round-trip the same representation.
llm/transformer/openai/responses/model_test.go Adds coverage validating polymorphic reasoning_content unmarshalling, round-tripping, and invalid-type rejection.
internal/server/orchestrator/pass_through.go Adds an allowlist and middleware to forward Codex Responses metadata headers when pass-through body replay is applied.
internal/server/orchestrator/pass_through_test.go Adds tests ensuring allowlisted metadata is forwarded and sensitive/untrusted headers are not.
internal/server/orchestrator/orchestrator.go Wires the new header pass-through middleware into the outbound middleware chain.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +137 to +140
if !outbound.state.PassThroughApplied || outbound.state.LlmRequest == nil ||
outbound.state.LlmRequest.APIFormat != llm.APIFormatOpenAIResponse ||
outbound.state.LlmRequest.RawRequest == nil {
return request, nil

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 `@llm/transformer/openai/responses/model.go`:
- Line 580: Preserve explicit null for Item.ReasoningContent by tracking whether
the field was present and null during deserialization, then emitting
reasoning_content:null during serialization instead of omitting it; retain
normal behavior for non-null and absent values. Update
llm/transformer/openai/responses/model.go#L580-L580 accordingly, and change
llm/transformer/openai/responses/model_test.go#L118-L123 to expect
reasoning_content:null after the JSON round trip.
🪄 Autofix

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8cf9feef-18c3-4887-9021-b9ce22323aa3

📥 Commits

Reviewing files that changed from the base of the PR and between dba642a and bd58363.

📒 Files selected for processing (5)
  • internal/server/orchestrator/orchestrator.go
  • internal/server/orchestrator/pass_through.go
  • internal/server/orchestrator/pass_through_test.go
  • llm/transformer/openai/responses/model.go
  • llm/transformer/openai/responses/model_test.go

Summary []ReasoningSummary `json:"summary,omitempty"`
// Reasoning text content - array of reasoning text items.
ReasoningContent []ReasoningContent `json:"reasoning_content,omitempty"`
ReasoningContent *PolymorphicReasoningContent `json:"reasoning_content,omitempty"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Preserve explicit null during the JSON round trip.

On Line 580, JSON null sets Item.ReasoningContent to nil without calling PolymorphicReasoningContent.UnmarshalJSON. omitempty then removes the field during marshal. This converts a supported reasoning_content:null representation into an absent property.

Track field presence and explicit null separately in Item serialization. Update the test to require reasoning_content:null after the round trip.

  • llm/transformer/openai/responses/model.go#L580-L580: retain explicit-null state when decoding and emit null when that state is set.
  • llm/transformer/openai/responses/model_test.go#L118-L123: expect explicit null instead of an omitted property.
📍 Affects 2 files
  • llm/transformer/openai/responses/model.go#L580-L580 (this comment)
  • llm/transformer/openai/responses/model_test.go#L118-L123
🤖 Prompt for 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.

In `@llm/transformer/openai/responses/model.go` at line 580, Preserve explicit
null for Item.ReasoningContent by tracking whether the field was present and
null during deserialization, then emitting reasoning_content:null during
serialization instead of omitting it; retain normal behavior for non-null and
absent values. Update llm/transformer/openai/responses/model.go#L580-L580
accordingly, and change llm/transformer/openai/responses/model_test.go#L118-L123
to expect reasoning_content:null after the JSON round trip.

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.

3 participants