feat(go/plugins/compat_oai): support reasoning_content for moonshot#5154
Open
naqerl wants to merge 6 commits intogenkit-ai:mainfrom
Open
feat(go/plugins/compat_oai): support reasoning_content for moonshot#5154naqerl wants to merge 6 commits intogenkit-ai:mainfrom
naqerl wants to merge 6 commits intogenkit-ai:mainfrom
Conversation
…imi compatibility This change adds support for preserving reasoning_content in OpenAI-compatible APIs that require it (specifically Moonshot/Kimi). Without this fix, follow-up tool-result requests are rejected with "reasoning_content is missing" error. The implementation: - Extracts reasoning content from message parts when building assistant messages - Preserves reasoning metadata (reasoningKey) for round-trip compatibility - Parses reasoning fields from API responses using common field names (reasoning_content, reasoning, reasoning_text) - Adds live test for Moonshot-compatible backends ISSUE: N/A CHANGELOG: - [x] Add reasoning_content preservation in WithMessages for tool calls - [x] Add reasoning field extraction from API responses - [x] Add Moonshot live test for end-to-end verification
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for reasoning content in OpenAI-compatible providers such as Moonshot/Kimi. It implements logic to extract reasoning fields from response metadata, prepend them to message content, and preserve them in the conversation history. Feedback highlights a potential issue where reasoning content might be duplicated in assistant messages and suggests expanding the scope of reasoning preservation beyond messages with tool calls. Additionally, it was recommended to refactor helper functions into methods of the ModelGenerator struct for better consistency with the existing codebase.
Comment on lines
83
to
92
| if len(toolCalls) > 0 { | ||
| am.ToolCalls = (toolCalls) | ||
| reasoningKey, ok := firstReasoningMetadataKey(msg.Content) | ||
| reasoning := concatenateReasoningContent(msg.Content) | ||
| if ok && reasoning != "" { | ||
| am.SetExtraFields(map[string]any{ | ||
| reasoningKey: reasoning, | ||
| }) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
The current implementation has two issues:
- Duplication:
am.Contentis already set to include all parts (including reasoning) viaconcatenateContentat line 77. By adding the reasoning again viaSetExtraFields, it is sent twice to the provider, which can degrade model performance or cause errors in strict APIs like Moonshot. - Limited Scope: Reasoning preservation is currently restricted to messages with tool calls. However, reasoning should be preserved for all assistant messages in the conversation history to maintain context and satisfy provider requirements for consistency.
I suggest moving the reasoning extraction outside the tool call check and overriding am.Content with only text/data parts when reasoning is present.
if len(toolCalls) > 0 {
am.ToolCalls = (toolCalls)
}
reasoningKey, ok := g.firstReasoningMetadataKey(msg.Content)
reasoning := g.concatenateReasoningContent(msg.Content)
if ok && reasoning != "" {
am.SetExtraFields(map[string]any{
reasoningKey: reasoning,
})
// Override Content to only include non-reasoning parts to avoid duplication.
am.Content.OfString = param.NewOpt(g.concatenateTextContent(msg.Content))
}
Comment on lines
+287
to
+310
| // concatenateReasoningContent works the same as concatenateContent | ||
| // but does it only for the reasoning parts | ||
| func concatenateReasoningContent(parts []*ai.Part) string { | ||
| var sb strings.Builder | ||
| for _, part := range parts { | ||
| if part.IsReasoning() { | ||
| sb.WriteString(part.Text) | ||
| } | ||
| } | ||
| return sb.String() | ||
| } | ||
|
|
||
| // firstReasoningMetadataKey returns the key which is used for the reasoning values | ||
| func firstReasoningMetadataKey(parts []*ai.Part) (string, bool) { | ||
| for _, part := range parts { | ||
| if !part.IsReasoning() || part.Metadata == nil { | ||
| continue | ||
| } | ||
| if key, ok := part.Metadata["reasoningKey"].(string); ok && key != "" { | ||
| return key, true | ||
| } | ||
| } | ||
| return "", false | ||
| } |
Contributor
There was a problem hiding this comment.
For consistency with the existing concatenateContent method, these helper functions should be defined as methods of ModelGenerator. Additionally, I've added concatenateTextContent to support filtering out reasoning parts when building assistant messages.
// concatenateReasoningContent works the same as concatenateContent
// but does it only for the reasoning parts
func (g *ModelGenerator) concatenateReasoningContent(parts []*ai.Part) string {
var sb strings.Builder
for _, part := range parts {
if part.IsReasoning() {
sb.WriteString(part.Text)
}
}
return sb.String()
}
// concatenateTextContent works the same as concatenateContent
// but excludes reasoning parts
func (g *ModelGenerator) concatenateTextContent(parts []*ai.Part) string {
var sb strings.Builder
for _, part := range parts {
if part.IsText() || part.IsData() {
sb.WriteString(part.Text)
}
}
return sb.String()
}
// firstReasoningMetadataKey returns the key which is used for the reasoning values
func (g *ModelGenerator) firstReasoningMetadataKey(parts []*ai.Part) (string, bool) {
for _, part := range parts {
if !part.IsReasoning() || part.Metadata == nil {
continue
}
if key, ok := part.Metadata["reasoningKey"].(string); ok && key != "" {
return key, true
}
}
return "", false
}
naqerl
force-pushed
the
naqerl/go-oai-compat-reasoning
branch
from
1729ab8 to
219c5d1
Compare
apascal07
requested changes
Apr 21, 2026
Author
|
Yes, also mentioned broken compatibility of sending tool call's parts,
fixing / actualizing tests. Thank you for your time
…On Tue, 21 Apr 2026, 20:00 Alex Pascal, ***@***.***> wrote:
*apascal07* left a comment (genkit-ai/genkit#5154)
<#5154?email_source=notifications&email_token=A4BHPHXGDEJZV7SIOUAGLAT4W6ZNVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMRZGA3TEMBVG4ZKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2LK4DSL5RW63LNMVXHIX3POBSW4X3DNRUWG2Y#issuecomment-4290720572>
Please take a look at the tests, they're not passing.
—
Reply to this email directly, view it on GitHub
<#5154?email_source=notifications&email_token=A4BHPHXGDEJZV7SIOUAGLAT4W6ZNVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIMRZGA3TEMBVG4ZKM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2LK4DSL5RW63LNMVXHIX3POBSW4X3DNRUWG2Y#issuecomment-4290720572>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A4BHPHTFZDVAQ7FZAGWVNQD4W6ZNVAVCNFSM6AAAAACX64LS2GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DEOJQG4ZDANJXGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Contributor
|
Hey @naqerl - thanks for looking at this. Are you planning to sync back w/ the request changes? |
Author
|
Hi! Yes, I'll finish & send update in two days, thanks for your patience.
…On Wed, 6 May 2026, 17:34 Michael Doyle, ***@***.***> wrote:
*MichaelDoyle* left a comment (genkit-ai/genkit#5154)
<#5154 (comment)>
Hey @naqerl <https://github.com/naqerl> - thanks for looking at this. Are
you planning to sync back w/ the request changes?
—
Reply to this email directly, view it on GitHub
<#5154 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A4BHPHW6BMKMEBAUDOL3SNL4ZNLQXAVCNFSM6AAAAACX64LS2GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DGOBZGY2TMOJQGQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
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
3 participants
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.
This change adds support for preserving reasoning_content in OpenAI-compatible APIs that require it (specifically Moonshot/Kimi). Without this fix, follow-up tool-result requests are rejected with "reasoning_content is missing" error.
The implementation:
ISSUE: N/A
CHANGELOG:
Description here... Help the reviewer by:
Checklist (if applicable):