opt: openai outbound reasoning field config, close #1584 by looplj · Pull Request #1586 · looplj/axonhub

looplj · 2026-05-02T05:47:45Z

No description provided.

greptile-apps · 2026-05-02T05:49:56Z

Greptile Summary

This PR centralizes outbound reasoning field handling by adding an explicit ReasoningField config option to the OpenAI base transformer and propagating it to all provider-specific transformers. RequestFromLLM now takes a reasoningField parameter instead of always stripping reasoning, and MessageFromLLM (used for client-facing serialization) now defaults to ReasoningFieldAll, preserving reasoning in responses. Fireworks is refactored from a custom TransformRequest that stripped reasoning into a simple delegation to the base OpenAI transformer with ReasoningFieldContent, reflecting actual API support.

Confidence Score: 5/5

Safe to merge — all providers correctly wire their reasoning field mode through both config and any direct RequestFromLLM calls; only a stale code comment remains.

The previously flagged P1 bypasses (Gemini, OpenRouter) are fully fixed. All 12 provider transformers show consistent reasoning field configuration. The single remaining finding is a P2 stale comment on ReasoningFieldNone naming Fireworks and Bailian as examples, which no longer applies after this PR.

llm/transformer/openai/outbound.go — minor stale comment on ReasoningFieldNone

Important Files Changed

Filename	Overview
llm/transformer/openai/outbound.go	Introduces `ReasoningField` as a config option with four modes (Content, Reasoning, None, All); default remains `ReasoningFieldNone` when unset. Minor stale comment on line 39 lists Fireworks/Bailian as `ReasoningFieldNone` users, but both now use `ReasoningFieldContent`.
llm/transformer/openai/outbound_convert.go	Refactored `RequestFromLLM` to take an explicit `reasoningField` parameter; `MessageFromLLM` (used by `ChoiceFromLLM` for client responses) now defaults to `ReasoningFieldAll`, preserving reasoning in client-facing serialization.
llm/transformer/fireworks/outbound.go	Removed custom `TransformRequest` and `OutboundTransformer` struct; now delegates directly to the base OpenAI transformer with `ReasoningFieldContent`, reflecting that Fireworks does support `reasoning_content`.
llm/transformer/gemini/openai/outbound.go	Custom `TransformRequest` now calls `RequestFromLLM(&req, openai.ReasoningFieldContent)` directly, consistent with the `ReasoningFieldContent` config value — the previous bypass is fixed.
llm/transformer/openrouter/outbound.go	Custom `TransformRequest` now calls `RequestFromLLM(llmReq, openai.ReasoningFieldReasoning)`, consistent with the `ReasoningFieldReasoning` config — the previous bypass is fixed.
llm/transformer/fireworks/integration_test.go	Deleted — previously tested that reasoning fields were stripped; no longer valid after behavior change to `ReasoningFieldContent`.
llm/transformer/openai/outbound_reasoning_test.go	New tests covering all four `ReasoningField` modes for `MessageFromLLMWithConfig` and `RequestFromLLM`, including sync and fallback logic.
llm/transformer/deepseek/outbound.go	Added `ReasoningFieldContent` to config and updated direct `RequestFromLLM` call signature; both config and explicit call agree.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    LLMReq["llm.Request"] --> TF["TransformRequest\n(per-provider)"]
    TF --> RFL{"ReasoningField\nconfig / explicit"}

    RFL -->|ReasoningFieldContent| RFC["reasoning_content only\n(DeepSeek, Gemini, Fireworks,\nBailian, Moonshot, Zai,\nXAI, Doubao, Longcat, ModelScope)"]
    RFL -->|ReasoningFieldReasoning| RFR["reasoning only\n(NanoGPT, OpenRouter)"]
    RFL -->|ReasoningFieldNone| RFN["strip both\n(default when unset)"]
    RFL -->|ReasoningFieldAll| RFA["preserve both\n(Copilot, client responses\nvia MessageFromLLM)"]

    RFC --> OAIReq["openai.Request → HTTP body"]
    RFR --> OAIReq
    RFN --> OAIReq
    RFA --> OAIReq

    LLMResp["llm.Response"] --> CI["ChoiceFromLLM\n(inbound_convert.go)"]
    CI --> MFLLM["MessageFromLLM\n→ ReasoningFieldAll"]
    MFLLM --> ClientResp["Client-facing OpenAI response\n(both reasoning fields preserved)"]

_{Reviews (7): Last reviewed commit: "opt: openai outbound reasoning field con..." | Re-trigger Greptile}

gemini-code-assist

Code Review

This pull request introduces a ReasoningField configuration to the OpenAI outbound transformer to standardize how reasoning fields are handled across different LLM providers. The review highlights a breaking change where legacy conversion helpers now default to stripping reasoning fields, which negatively impacts providers like Gemini and OpenRouter that haven't been updated to use the new configuration-aware methods. Feedback also notes an inconsistency in the Fireworks provider setup and suggests a correction for a misleading test name in the inbound conversion suite.

gemini-code-assist · 2026-05-02T05:50:00Z


 // RequestFromLLM creates OpenAI Request from unified llm.Request.
 func RequestFromLLM(r *llm.Request) *Request {
+	return RequestFromLLMWithConfig(r, ReasoningFieldNone)


Changing the default behavior of RequestFromLLM to ReasoningFieldNone is a breaking change. Any transformer that overrides TransformRequest and relies on this helper (such as Gemini and OpenRouter in this repository) will now silently strip reasoning fields from outbound requests.

Please ensure all callers of RequestFromLLM and MessageFromLLM (line 104) are updated to use RequestFromLLMWithConfig with the appropriate field configuration, or consider maintaining the previous behavior (preserving both fields) as the default for these legacy helpers.

gemini-code-assist · 2026-05-02T05:50:00Z

 		PlatformType:   openai.PlatformGoogle,
 		BaseURL:        baseURL,
 		APIKeyProvider: config.APIKeyProvider,
+		ReasoningField: openai.ReasoningFieldContent,


The TransformRequest override in this file (line 344 in the full file) calls openai.RequestFromLLM, which now defaults to ReasoningFieldNone. This will cause reasoning fields to be stripped in outbound requests, ignoring the ReasoningFieldContent configuration set here. You should update TransformRequest to use openai.RequestFromLLMWithConfig with the configured field.

gemini-code-assist · 2026-05-02T05:50:00Z

 		PlatformType:   openai.PlatformOpenAI,
 		BaseURL:        config.BaseURL,
 		APIKeyProvider: config.APIKeyProvider,
+		ReasoningField: openai.ReasoningFieldReasoning,


The TransformRequest override in this file (line 104 in the full file) calls openai.RequestFromLLM, which now defaults to ReasoningFieldNone. This will cause reasoning fields to be stripped in outbound requests, ignoring the ReasoningFieldReasoning configuration set here. You should update TransformRequest to use openai.RequestFromLLMWithConfig with the configured field.

gemini-code-assist · 2026-05-02T05:50:00Z

 		PlatformType:   openai.PlatformOpenAI,
 		BaseURL:        baseURL,
 		APIKeyProvider: config.APIKeyProvider,
+		ReasoningField: openai.ReasoningFieldContent,


This configuration is inconsistent with the Fireworks provider's capabilities and the implementation in TransformRequest (line 104), which explicitly uses ReasoningFieldNone. Since Fireworks does not support reasoning fields, this should be set to None to avoid confusion.

Suggested change

ReasoningField: openai.ReasoningFieldContent,

ReasoningField: openai.ReasoningFieldNone,

gemini-code-assist · 2026-05-02T05:50:00Z

 		},
 		{
-			name: "Nil reasoning field - should not populate ReasoningContent",
+			name: "Empty reasoning_content with non-empty reasoning - should sync from Reasoning",


The test name is misleading. The test case actually verifies that the field is not synced when it is an empty string (as opposed to nil), which matches the implementation logic.

Suggested change

name: "Empty reasoning_content with non-empty reasoning - should sync from Reasoning",

name: "Empty reasoning_content with non-empty reasoning - should not sync from Reasoning",

looplj · 2026-05-02T11:38:47Z

@gemini-code-assist review

gemini-code-assist

Code Review

This pull request introduces a ReasoningField configuration to the OpenAI outbound transformer, enabling granular control over the handling of reasoning fields across different LLM providers. The Fireworks transformer has been refactored to leverage the base OpenAI implementation. Review feedback identifies a breaking change in the default reasoning field behavior and recommends defaulting to ReasoningFieldAll to maintain backward compatibility. Inconsistencies were also noted in the Doubao, Moonshot, and Zai transformers where explicit transformation calls do not align with their respective configurations. Furthermore, improvements are suggested for the message conversion logic to ensure proper field syncing and empty string handling.

gemini-code-assist · 2026-05-02T11:40:58Z

+	// Determine which reasoning field to use, default to ReasoningFieldNone
+	reasoningField := t.config.ReasoningField
+	if reasoningField == "" {
+		reasoningField = ReasoningFieldNone
+	}


Defaulting to ReasoningFieldNone when the configuration is empty is a breaking change for the base OpenAI transformer. Previously, reasoning fields were preserved and synced. It should default to ReasoningFieldAll to maintain backward compatibility for users of the generic OpenAI transformer who expect reasoning content to be passed through.

Suggested change

// Determine which reasoning field to use, default to ReasoningFieldNone

reasoningField := t.config.ReasoningField

if reasoningField == "" {

reasoningField = ReasoningFieldNone

}

// Determine which reasoning field to use, default to ReasoningFieldAll

reasoningField := t.config.ReasoningField

if reasoningField == "" {

reasoningField = ReasoningFieldAll

}

gemini-code-assist · 2026-05-02T11:40:58Z

+	switch reasoningField {
+	case ReasoningFieldContent:
+		// Only use reasoning_content field
+		// Prefer ReasoningContent, fallback to Reasoning if ReasoningContent is nil
+		reasoningContent = m.ReasoningContent
+		if reasoningContent == nil && m.Reasoning != nil {
+			reasoningContent = m.Reasoning
+		}
+		reasoning = nil
+	case ReasoningFieldReasoning:
+		// Only use reasoning field
+		// Prefer Reasoning, fallback to ReasoningContent if Reasoning is nil
+		reasoning = m.Reasoning
+		if reasoning == nil && m.ReasoningContent != nil {
+			reasoning = m.ReasoningContent
+		}
+		reasoningContent = nil
+	case ReasoningFieldNone:
+		// Strip all reasoning fields
+		reasoningContent = nil
+		reasoning = nil
+	default: // ReasoningFieldAll
+		// Preserve both reasoning fields as-is
+		reasoningContent = m.ReasoningContent
+		reasoning = m.Reasoning
+	}


The new implementation of reasoning field handling has two issues:

It omits the empty string check (* != "") when performing fallbacks in ReasoningFieldContent and ReasoningFieldReasoning cases. This might lead to syncing empty reasoning strings which were previously ignored.

The ReasoningFieldAll (default) case currently just copies the fields without the syncing/fallback logic that existed in the previous implementation. This is a regression for the default behavior.

Restoring the syncing logic ensures that if only one field is provided, it is copied to the other to maximize compatibility with various providers.

switch reasoningField { case ReasoningFieldContent: // Only use reasoning_content field // Prefer ReasoningContent, fallback to Reasoning if ReasoningContent is nil reasoningContent = m.ReasoningContent if reasoningContent == nil && m.Reasoning != nil && *m.Reasoning != "" { reasoningContent = m.Reasoning } reasoning = nil case ReasoningFieldReasoning: // Only use reasoning field // Prefer Reasoning, fallback to ReasoningContent if Reasoning is nil reasoning = m.Reasoning if reasoning == nil && m.ReasoningContent != nil && *m.ReasoningContent != "" { reasoning = m.ReasoningContent } reasoningContent = nil case ReasoningFieldNone: // Strip all reasoning fields reasoningContent = nil reasoning = nil default: // ReasoningFieldAll // Preserve both reasoning fields and sync them if one is missing reasoningContent = m.ReasoningContent reasoning = m.Reasoning if reasoningContent == nil && reasoning != nil && *reasoning != "" { reasoningContent = reasoning } if reasoning == nil && reasoningContent != nil && *reasoningContent != "" { reasoning = reasoningContent } }

gemini-code-assist · 2026-05-02T11:40:58Z


 	// Convert llm.Request to openai.Request first
-	oaiReq := openai.RequestFromLLM(llmReq)
+	oaiReq := openai.RequestFromLLM(llmReq, openai.ReasoningFieldContent)


There is an inconsistency between the transformer configuration (which sets ReasoningFieldNone at line 72) and this explicit call to RequestFromLLM which uses ReasoningFieldContent. Since Doubao is configured to strip reasoning fields, this call should use openai.ReasoningFieldNone to be consistent and avoid sending unsupported fields to the provider.

Suggested change

oaiReq := openai.RequestFromLLM(llmReq, openai.ReasoningFieldContent)

oaiReq := openai.RequestFromLLM(llmReq, openai.ReasoningFieldNone)

gemini-code-assist · 2026-05-02T11:40:58Z


 	// Convert llm.Request to openai.Request first
-	oaiReq := openai.RequestFromLLM(llmReq)
+	oaiReq := openai.RequestFromLLM(llmReq, openai.ReasoningFieldContent)


There is an inconsistency between the transformer configuration (which sets ReasoningFieldNone at line 47) and this explicit call to RequestFromLLM which uses ReasoningFieldContent. Since Moonshot is configured to strip reasoning fields, this call should use openai.ReasoningFieldNone to align with the intended configuration.

Suggested change

oaiReq := openai.RequestFromLLM(llmReq, openai.ReasoningFieldContent)

oaiReq := openai.RequestFromLLM(llmReq, openai.ReasoningFieldNone)

gemini-code-assist · 2026-05-02T11:40:58Z

+
+	// ReasoningField specifies which reasoning field to use in outbound messages.
+	// Use ReasoningFieldContent for DeepSeek/Gemini, ReasoningFieldReasoning for NanoGPT/OpenRouter,
+	// or ReasoningFieldNone (default) to strip all reasoning fields.


If the default behavior is changed to ReasoningFieldAll to maintain backward compatibility (as suggested in the TransformRequest logic), this comment should be updated accordingly.

Suggested change

// or ReasoningFieldNone (default) to strip all reasoning fields.

// or ReasoningFieldAll (default) to preserve both reasoning fields.

gemini-code-assist · 2026-05-02T11:40:58Z


 	// Convert llm.Request to openai.Request first
-	oaiReq := openai.RequestFromLLM(llmReq)
+	oaiReq := openai.RequestFromLLM(llmReq, openai.ReasoningFieldContent)


There is an inconsistency between the transformer configuration (which sets ReasoningFieldNone at line 51) and this explicit call to RequestFromLLM which uses ReasoningFieldContent. Since Zai is configured to strip reasoning fields, this call should use openai.ReasoningFieldNone.

Suggested change

oaiReq := openai.RequestFromLLM(llmReq, openai.ReasoningFieldContent)

oaiReq := openai.RequestFromLLM(llmReq, openai.ReasoningFieldNone)

gemini-code-assist Bot reviewed May 2, 2026

View reviewed changes

looplj force-pushed the dev-tmp branch from 67f4bf3 to 474a74e Compare May 2, 2026 08:25

greptile-apps Bot reviewed May 2, 2026

View reviewed changes

Comment thread llm/transformer/openai/outbound_convert.go Outdated

looplj force-pushed the dev-tmp branch 2 times, most recently from 42fa818 to c36bd7e Compare May 2, 2026 10:28

gemini-code-assist Bot reviewed May 2, 2026

View reviewed changes

looplj force-pushed the dev-tmp branch 2 times, most recently from c177ccd to 1898d1e Compare May 2, 2026 12:25

opt: openai outbound reasoning field config, close #1584

142a9d4

looplj force-pushed the dev-tmp branch from 1898d1e to 142a9d4 Compare May 2, 2026 15:25

looplj merged commit c3ea750 into unstable May 2, 2026
4 checks passed

	ReasoningField: openai.ReasoningFieldContent,
	ReasoningField: openai.ReasoningFieldNone,

	name: "Empty reasoning_content with non-empty reasoning - should sync from Reasoning",
	name: "Empty reasoning_content with non-empty reasoning - should not sync from Reasoning",

	oaiReq := openai.RequestFromLLM(llmReq, openai.ReasoningFieldContent)
	oaiReq := openai.RequestFromLLM(llmReq, openai.ReasoningFieldNone)

	// or ReasoningFieldNone (default) to strip all reasoning fields.
	// or ReasoningFieldAll (default) to preserve both reasoning fields.

Conversation

looplj commented May 2, 2026

Uh oh!

greptile-apps Bot commented May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

looplj commented May 2, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot May 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot May 2, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps Bot commented May 2, 2026 •

edited

Loading