Skip to content

fix(protocols): omit absent optional fields in chat completion responses - #202

Merged
luckyq merged 2 commits into
ai-dynamo:mainfrom
luckyq:fix/issue-157-omit-absent-optional-fields
Sep 2, 2026
Merged

luckyq merged 2 commits into
ai-dynamo:mainfrom
luckyq:fix/issue-157-omit-absent-optional-fields

Conversation

@luckyq

@luckyq luckyq commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Aligns chat completion response serialization with the official OpenAI spec (openai/openai-openapi v2.3.0): optional fields are omitted when absent instead of serialized as explicit null, while required-and-nullable fields stay always-present.

The two axes are orthogonal — required governs whether the key must be present; nullable governs whether the value may be null:

nullable non-nullable
required key must be present, value may be null (e.g. refusal, non-stream logprobs) key must be present with a real value (e.g. non-stream finish_reason)
optional omitting or emitting null are both spec-legal must omit when absent — emitting null is invalid (e.g. tool_calls)

Field by field:

Field Spec Before After
top-level usage / service_tier / system_fingerprint (non-stream + stream) optional emitted null omitted
stream tool-call chunk id / type / function, function.name optional emitted null on continuation chunks omitted
message.reasoning_content (our extension, not in spec) — emitted null omitted
ChatCompletionStreamResponseDeltaFunctionCall.name / arguments optional emitted null omitted
message.refusal required, nullable omitted (bug in the opposite direction) always serialized — OpenAI emits "refusal": null on every response
choices[].finish_reason, non-stream choices[].logprobs required always present unchanged (omitting would deviate from spec/OpenAI)
stream delta fields, message.tool_calls / function_call optional already omitted unchanged

One nuance on streaming usage: with stream_options.include_usage, OpenAI emits "usage": null on chunks before the final one. Omitting the key is still spec-legal, so we omit when None; callers needing exact-null mirroring can inject it at the HTTP boundary (documented on the struct).

Also fixed: usage-details nulls (same bug class)

Upstream async-openai 0.41.1 serializes absent usage-detail fields as null ("audio_tokens": null) although the spec marks them optional and non-nullable — OpenAI emits "audio_tokens": 0, never null.

Fixed at the serialization boundary rather than by owning types: the two chat response usage fields carry a serialize_with that routes through a private shadow struct which skips absent fields. CompletionUsage, PromptTokensDetails, and CompletionTokensDetails remain upstream re-exports, so public type identity is unchanged and callers holding upstream values still compile; deserialization stays derived. The legacy completions surface (CreateCompletionResponse) is not touched by this PR.

(An earlier revision of this PR replaced those types with same-named local structs — a source-breaking change, reverted in 7035dfb after review. The upstream serialization bug is worth reporting to async-openai separately.)

Validation

  • 5 new serialization-shape unit tests asserting key presence/absence in both directions (omitted keys absent, required-and-nullable keys present as null), including a deserialization round-trip through the serialize_with path
  • cargo test --workspace, cargo clippy --all-targets --all-features -- -D warnings, cargo fmt --check all clean
  • Verified end-to-end on a live deployment (full dynamo stack + vLLM + Qwen3-0.6B on an aarch64 H100, this branch patched in via [patch.crates-io]): 47 shape assertions across non-stream, stream, tool-calling, include_usage, logprobs/top_logprobs, n>1, and reasoning-parser paths — including a recursive sweep asserting no unexpected null anywhere in any response. Live usage at current head: {"prompt_tokens":10,"completion_tokens":25,"total_tokens":35,"prompt_tokens_details":{"cached_tokens":0},"completion_tokens_details":{"reasoning_tokens":24}} — both shadow branches exercised, no null fields.

Closes #157

🤖 Generated with Claude Code

Align chat response serialization with the OpenAI spec (openai/openai-openapi
v2.3.0): optional fields are omitted when absent instead of serialized as
explicit null, while required-and-nullable fields stay always-present.

Omit when None (previously serialized as null):
- CreateChatCompletionResponse / CreateChatCompletionStreamResponse:
  service_tier, system_fingerprint, usage
- ChatCompletionMessageToolCallChunk: id, type, function (continuation
  chunks now carry only index + function.arguments, matching OpenAI)
- FunctionCallStream: name, arguments
- ChatCompletionStreamResponseDeltaFunctionCall: name, arguments
- ChatCompletionResponseMessage.reasoning_content (our extension, not in
  the spec at all)

Always serialize again (was omitted; spec marks it required + nullable):
- ChatCompletionResponseMessage.refusal — OpenAI emits "refusal": null on
  every non-refusal response

Deliberately unchanged (required per spec; omitting would deviate):
- choices[].finish_reason, non-stream choices[].logprobs

Own CompletionUsage / PromptTokensDetails / CompletionTokensDetails and the
legacy CreateCompletionResponse instead of re-exporting them: upstream
async-openai 0.41.1 serializes absent usage-detail fields as null (e.g.
"audio_tokens": null) although the spec marks them optional and
non-nullable, and serializes the legacy response's optional
system_fingerprint/usage as null. Field sets mirror upstream, so
constructors are unaffected.

Verified end-to-end on a live vLLM deployment (Qwen3-0.6B): 47 shape
assertions across non-stream, stream, tool-calling, include_usage,
logprobs, n>1, and reasoning-parser paths.

Closes ai-dynamo#157

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: doxu <doxu@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Aug 30, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added external-contribution PR originates from a fork fix labels Aug 30, 2026
Comment thread protocols/src/types/chat.rs Outdated
Comment thread protocols/src/types/completion.rs Outdated
Comment thread protocols/src/types/chat.rs Outdated
Comment thread protocols/src/types/chat.rs Outdated
Comment thread protocols/src/types/chat.rs Outdated
Comment thread protocols/src/types/chat.rs Outdated
…alization

Address review: replacing the re-exported async-openai CompletionUsage /
PromptTokensDetails / CompletionTokensDetails (and the legacy
CreateCompletionResponse) with same-named local structs was a
source-breaking API change — callers holding upstream values could no
longer assign them to the public usage fields. Restore the upstream
re-exports and fix the wire shape instead with a serialize_with shadow on
the chat response usage fields, so absent usage-details fields are omitted
without changing any public type identity. The legacy completions surface
is no longer touched by this PR.

Also per review: drop pre-existing-behavior delta assertions from the
stream shape test and remove narrating test comments.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: doxu <doxu@nvidia.com>
@luckyq

luckyq commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

@indrajit96

@indrajit96
indrajit96 self-requested a review August 31, 2026 18:32
@indrajit96

Copy link
Copy Markdown
Contributor

[P3] Update the usage-details description after restoring upstream types

The “Also fixed: usage-details nulls” section still says CompletionUsage, PromptTokensDetails, and CompletionTokensDetails are crate-owned and that this PR owns the legacy CreateCompletionResponse.

At current head 7035dfb, the upstream type re-exports were restored, omission is implemented through a custom serializer on the two chat response usage fields, and the legacy completions surface is no longer changed. The current description therefore misstates the public API and scope.

Suggested fix

Update this section to describe the serialize_with shadow used by chat responses and remove the legacy-completions ownership claim.

@indrajit96 indrajit96 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.

LGTM!
Thanks for adding this! @luckyq great work!

@rmccorm4

rmccorm4 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

/ok to test 7035dfb

@luckyq
luckyq merged commit fd6bdc1 into ai-dynamo:main Sep 2, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

external-contribution PR originates from a fork fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Omit absent optional fields in chat completion responses

3 participants