Skip to content

fix(protocols): always serialize chat message content (null when absent)#8372

Merged
rmccorm4 merged 2 commits into
mainfrom
rmccormick/dgh-651-missing-content-field
Apr 20, 2026
Merged

fix(protocols): always serialize chat message content (null when absent)#8372
rmccorm4 merged 2 commits into
mainfrom
rmccormick/dgh-651-missing-content-field

Conversation

@rmccorm4
Copy link
Copy Markdown
Contributor

@rmccorm4 rmccorm4 commented Apr 20, 2026

Summary

Non-streaming /v1/chat/completions responses were dropping the content key entirely from the JSON message any time content was None (reasoning-only responses, tool-call-only responses). This broke clients like the one in DGH-651 that read response.json()['choices'][0]['message']['content'].

Root cause: ChatCompletionResponseMessage.content carried #[serde(skip_serializing_if = "Option::is_none")], so None was omitted instead of serialized as null. Upstream OpenAI's API always emits the content key (as null when there is no content) alongside reasoning_content or tool_calls.

Fix: remove skip_serializing_if from that one field. Now content: null is always emitted when there's no text/content-parts, matching OpenAI's wire format.

Approach history:

  • First commit attempted a narrower workaround in the aggregator (set content to Some(Text("")) when reasoning_content was present). That was a semantic muddle ("no content" vs "empty content") and only fixed the reasoning case, not the tool-call-only case.
  • Second commit replaces it with the serialization-level fix, which is simpler (1 attribute removed) and covers both shapes.

Fixes: DGH-651
Closes #7154

Test plan

  • New unit test test_reasoning_only_response_serializes_content_key_as_null asserts content serializes as serde_json::Value::Null when reasoning-only.
  • Empirically verified the test fails on main without the fix, passes with it.
  • Full workspace test suite: 2000+ tests, 0 failures.

```
cargo test --workspace --lib

all passing

```

🤖 Generated with Claude Code

When the reasoning parser (e.g. gpt_oss) produces only reasoning_content
with no final-channel text or content parts, the DeltaChoice->ChatChoice
conversion was setting content to None. Combined with the
skip_serializing_if on ChatCompletionResponseMessage.content, this dropped
the content key entirely from the non-streaming /v1/chat/completions JSON
response, breaking clients that expect both content and reasoning_content.

Fall through to Some(Text("")) when reasoning_content is present so the
key always survives serialization, matching the OpenAI convention of
returning content: "" rather than omitting the field.

Fixes: DGH-651
Closes: #7154

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@rmccorm4 rmccorm4 requested a review from a team as a code owner April 20, 2026 17:02
@github-actions github-actions Bot added fix frontend `python -m dynamo.frontend` and `dynamo-run in=http|text|grpc` labels Apr 20, 2026
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 0 potential issues.

View 1 additional finding in Devin Review.

Open in Devin Review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 20, 2026

Walkthrough

Modified the From<DeltaChoice> implementation in the OpenAI chat completions aggregator to ensure the content field is retained in the message response when only reasoning_content is present, preventing the field from being omitted during serialization.

Changes

Cohort / File(s) Summary
Protocol Aggregator
lib/llm/src/protocols/openai/chat_completions/aggregator.rs
Updated delta choice conversion to retain content as an empty string when reasoning_content exists but text and content\_parts are absent; added unit test validating both fields serialize correctly.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The code changes directly address the issue #7154 by ensuring both content and reasoning_content fields are present in the JSON response, matching OpenAI convention.
Out of Scope Changes check ✅ Passed All changes are focused on fixing the aggregator conversion logic and adding relevant unit tests; no unrelated modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Title check ✅ Passed The title accurately summarizes the main change: ensuring chat message content is always serialized, including null values when absent, which directly matches the fix implementation.
Description check ✅ Passed The PR description is comprehensive and well-structured, covering overview, detailed explanation of root cause, fix approach, and test plan with command verification.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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 and usage tips.

…ting

Replace the reasoning-only workaround in aggregator.rs with a direct fix
at the serialization config: remove skip_serializing_if from
ChatCompletionResponseMessage.content so the key is always present in
the JSON (as null when None), matching the upstream OpenAI API shape.

This also fixes the tool-call-only response path, which previously
dropped the content key the same way. Verified by running the full
workspace test suite (2000+ tests, 0 failures).

Update the DGH-651 regression test to assert content serializes as
serde_json::Value::Null rather than an empty string.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@rmccorm4 rmccorm4 changed the title fix(aggregator): include content field when reasoning_content is present fix(protocols): always serialize chat message content (null when absent) Apr 20, 2026
@rmccorm4 rmccorm4 enabled auto-merge (squash) April 20, 2026 18:46
@rmccorm4 rmccorm4 merged commit 846ce71 into main Apr 20, 2026
91 checks passed
@rmccorm4 rmccorm4 deleted the rmccormick/dgh-651-missing-content-field branch April 20, 2026 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix frontend `python -m dynamo.frontend` and `dynamo-run in=http|text|grpc` size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: Missing 'content' in response while 'reasoning_content' is present for gpt-oss-120B

2 participants