fix(openai): handle providers emitting both reasoning and reasoning_content fields#2911
Merged
tusharmath merged 4 commits intomainfrom Apr 9, 2026
Merged
fix(openai): handle providers emitting both reasoning and reasoning_content fields#2911tusharmath merged 4 commits intomainfrom
tusharmath merged 4 commits intomainfrom
Conversation
Comment on lines
+171
to
+186
| pub fn reasoning(&self) -> Option<&str> { | ||
| match (self.reasoning.as_deref(), self.reasoning_content.as_deref()) { | ||
| (Some(a), Some(b)) => { | ||
| let a = a.trim(); | ||
| let b = b.trim(); | ||
| match (a.is_empty(), b.is_empty()) { | ||
| (true, _) => Some(b).filter(|s| !s.is_empty()), | ||
| (_, true) => Some(a).filter(|s| !s.is_empty()), | ||
| _ => Some(if b.len() > a.len() { b } else { a }), | ||
| } | ||
| } | ||
| (Some(a), None) => Some(a).filter(|s| !s.trim().is_empty()), | ||
| (None, Some(b)) => Some(b).filter(|s| !s.trim().is_empty()), | ||
| (None, None) => None, | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
The reasoning() method returns inconsistent data: trimmed strings when both fields are present, but untrimmed strings when only one field is present.
When both reasoning and reasoning_content exist (line 173-180), the returned value is the trimmed version (a.trim() or b.trim()). However, when only one field exists (lines 182-183), the original untrimmed string is returned.
For example:
reasoning(Some(" hello "), None)returns" hello "(untrimmed)reasoning(Some(" hello "), Some(""))returns"hello"(trimmed)
This inconsistency could cause production issues if downstream code expects consistent whitespace handling.
Fix: Either always return trimmed strings or always return untrimmed strings. For consistency with the filtering logic that checks !s.trim().is_empty(), the method should return trimmed strings in all cases:
pub fn reasoning(&self) -> Option<&str> {
match (self.reasoning.as_deref(), self.reasoning_content.as_deref()) {
(Some(a), Some(b)) => {
let a = a.trim();
let b = b.trim();
match (a.is_empty(), b.is_empty()) {
(true, _) => Some(b).filter(|s| !s.is_empty()),
(_, true) => Some(a).filter(|s| !s.is_empty()),
_ => Some(if b.len() > a.len() { b } else { a }),
}
}
(Some(a), None) => {
let trimmed = a.trim();
Some(trimmed).filter(|s| !s.is_empty())
}
(None, Some(b)) => {
let trimmed = b.trim();
Some(trimmed).filter(|s| !s.is_empty())
}
(None, None) => None,
}
}
Suggested change
| pub fn reasoning(&self) -> Option<&str> { | |
| match (self.reasoning.as_deref(), self.reasoning_content.as_deref()) { | |
| (Some(a), Some(b)) => { | |
| let a = a.trim(); | |
| let b = b.trim(); | |
| match (a.is_empty(), b.is_empty()) { | |
| (true, _) => Some(b).filter(|s| !s.is_empty()), | |
| (_, true) => Some(a).filter(|s| !s.is_empty()), | |
| _ => Some(if b.len() > a.len() { b } else { a }), | |
| } | |
| } | |
| (Some(a), None) => Some(a).filter(|s| !s.trim().is_empty()), | |
| (None, Some(b)) => Some(b).filter(|s| !s.trim().is_empty()), | |
| (None, None) => None, | |
| } | |
| } | |
| pub fn reasoning(&self) -> Option<&str> { | |
| match (self.reasoning.as_deref(), self.reasoning_content.as_deref()) { | |
| (Some(a), Some(b)) => { | |
| let a = a.trim(); | |
| let b = b.trim(); | |
| match (a.is_empty(), b.is_empty()) { | |
| (true, _) => Some(b).filter(|s| !s.is_empty()), | |
| (_, true) => Some(a).filter(|s| !s.is_empty()), | |
| _ => Some(if b.len() > a.len() { b } else { a }), | |
| } | |
| } | |
| (Some(a), None) => { | |
| let trimmed = a.trim(); | |
| Some(trimmed).filter(|s| !s.is_empty()) | |
| } | |
| (None, Some(b)) => { | |
| let trimmed = b.trim(); | |
| Some(trimmed).filter(|s| !s.is_empty()) | |
| } | |
| (None, None) => None, | |
| } | |
| } | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
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
1 participant
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
Fix a deserialization panic when providers (e.g.
moonshotai/Kimi-K2.5-TEEvia Chutes) emit bothreasoningandreasoning_contentfields in the same delta object.Context
Some OpenAI-compatible providers — notably
moonshotai/Kimi-K2.5-TEEserved through Chutes — include bothreasoningandreasoning_contentkeys inside a single streaming delta. The previous implementation used#[serde(alias = "reasoning_content")]on thereasoningfield, which causes serde to emit aduplicate_fielderror when both keys are present, crashing the stream parser.Changes
#[serde(alias)]approach with two separate private fields (reasoningandreasoning_content) onResponseMessageResponseMessage::reasoning()method that merges the two fields by returning the longer non-empty value (favours content over an empty/whitespace-only entry)message.reasoningdirectly to use the newmessage.reasoning()accessorchutes_completion_response.json) capturing a real Chutes/Kimi-K2.5-TEE delta with both keys presenttest_kimi_k2_both_reasoning_keys_event) that asserts the response parses successfully end-to-endKey Implementation Details
The merge strategy in
reasoning(): when both fields are non-empty, the longer string wins. This handles the observed behaviour where both values are identical (Kimi-K2.5-TEE) while staying correct if a future provider sends subtly different content in each key.Testing
Links