Skip to content

refactor(protocols): use serde_with::skip_serializing_none to reduce boilerplate - #342

Merged
slin1237 merged 1 commit into
mainfrom
chang/serde
Feb 6, 2026
Merged

slin1237 merged 1 commit into
mainfrom
chang/serde

Conversation

@CatherineSue

@CatherineSue CatherineSue commented Feb 6, 2026 •

Copy link
Copy Markdown
Member

Description

Problem

The protocols crate has ~400 repetitive #[serde(skip_serializing_if = "Option::is_none")] field annotations scattered across 12 files, adding visual noise and maintenance burden.

Solution

Add the serde_with crate and use #[serde_with::skip_serializing_none] at the struct/enum level, which automatically applies skip_serializing_if = "Option::is_none" to all Option<T> fields. This removes ~290 boilerplate annotations with zero serialization behavior changes.

The macro was only applied to types where every Option<T> field already had the annotation, ensuring no behavioral differences.

Changes

  • Add serde_with = { version = "3", features = ["macros"] } to workspace and protocols dependencies
  • Apply #[serde_with::skip_serializing_none] to qualifying types across 11 files
  • Remove redundant per-field #[serde(skip_serializing_if = "Option::is_none")] from those types
  • Preserve all non-Option skip patterns (Vec::is_empty, HashMap::is_empty) and #[serde(flatten)]

Types skipped (not all Option fields were annotated — applying the macro would change serialization behavior)

Type Unannotated Option Field(s) Reason
ChatCompletionMessage reasoning_content Intentionally serializes None as null
ChatChoice finish_reason OpenAI API: null in response until determined
ChatMessageDelta reasoning_content Intentionally serializes None as null
ChatStreamChoice logprobs, finish_reason OpenAI streaming: null until final chunk
CompletionChoice finish_reason Same as ChatChoice
CompletionStreamChoice finish_reason Same as above
CompletionStreamResponse system_fingerprint Only 1 annotation (threshold: 2+)
ResponseInputOutputItem id (in FunctionCallOutput) Intentionally nullable
ResponseContentPart logprobs Only 1 annotation
ResponsesRequest stream Has #[serde(default)] but no skip — serializes None as null
GenerateRequest model Intentionally nullable
RerankRequest user Not annotated

Test Plan

  • cargo check -p openai-protocol passes
  • Verified all Vec::is_empty / HashMap::is_empty / flatten annotations preserved
  • Verified 0 remaining Option::is_none annotations in transformed types
  • Verified 105 Option::is_none annotations remain in non-transformed types (unchanged)
Checklist
  • cargo +nightly fmt passes
  • cargo clippy --all-targets --all-features -- -D warnings passes
  • (Optional) Documentation updated

Summary by CodeRabbit

  • Chores
    • Standardized JSON serialization across all API endpoints: optional empty fields are now consistently omitted from both request and response payloads instead of being included as null values, resulting in cleaner response formatting, reduced payload sizes, and improved clarity in API integrations.

…boilerplate

Replace repetitive per-field `#[serde(skip_serializing_if = "Option::is_none")]`
annotations with the struct-level `#[serde_with::skip_serializing_none]` macro
across the protocols crate. This removes ~290 boilerplate annotations.

Only applied to types where ALL Option<T> fields already had the annotation,
ensuring zero serialization behavior changes.
@github-actions github-actions Bot added dependencies Dependency updates protocols Protocols crate changes labels Feb 6, 2026
@coderabbitai

coderabbitai Bot commented Feb 6, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR adds the serde_with crate as a workspace dependency and systematically refactors serialization behavior across protocol structures by consolidating per-field skip_serializing_if attributes into struct-level #[serde_with::skip_serializing_none] attributes.

Changes

Cohort / File(s) Summary
Workspace Dependencies
Cargo.toml, protocols/Cargo.toml
Added serde_with = { version = "3", features = ["macros"] } to workspace dependencies and enabled it in protocols crate.
Chat Protocol Serialization
protocols/src/chat.rs
Applied #[serde_with::skip_serializing_none] to ChatMessage, ChatCompletionRequest, ChatCompletionResponse, and ChatCompletionStreamResponse; removed 44 per-field skip attributes from optional fields.
Request/Response Serialization
protocols/src/classify.rs, protocols/src/embedding.rs, protocols/src/completion.rs
Added struct-level skip attribute to ClassifyRequest, EmbeddingRequest, and CompletionRequest/CompletionResponse; consolidated per-field serialization guards into struct-level behavior.
Common Types & Metadata
protocols/src/common.rs, protocols/src/generate.rs, protocols/src/rerank.rs
Applied #[serde_with::skip_serializing_none] to ToolCallDelta, FunctionCallDelta, Function, ErrorDetail, UsageInfo, GenerateMetaInfo, and RerankResult; removed redundant per-field skip directives.
Message & Tool Types
protocols/src/messages.rs
Consolidated serialization across 27+ public structs and enums including CreateMessageRequest, TextBlock, ToolChoice, various beta content blocks, and MCP tool configs; removed 66 per-field skip attributes.
Response & Configuration Types
protocols/src/responses.rs, protocols/src/sampling_params.rs, protocols/src/worker_spec.rs
Added struct-level skip attribute to ResponseTool, ResponseOutputItem, SamplingParams, and worker-related structs (WorkerConfigRequest, WorkerInfo, WorkerUpdateRequest, ServerInfo); systematically removed per-field serialization guards.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

protocols, dependencies

Suggested reviewers

  • key4ng
  • slin1237

Poem

🐰 A refactor neat, so tidy and clean,
Per-field skips consolidated to the scene,
Struct-level wisdom now guides the way,
Serialization flows with grace each day,
None fields vanish—what a delight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main refactoring change: consolidating per-field serde boilerplate annotations into struct-level skip_serializing_none attributes across the protocols crate.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chang/serde

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @CatherineSue, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refactors the protocols crate by integrating the serde_with library. The primary goal is to reduce code verbosity and improve maintainability by replacing numerous explicit skip_serializing_if annotations with a single, declarative macro. This change streamlines the serialization logic for optional fields without altering the external behavior of the API, making the codebase cleaner and easier to read.

Highlights

  • Dependency Addition: The serde_with crate, version 3, with macros feature, has been added to the workspace and protocols crate dependencies.
  • Serialization Boilerplate Reduction: Approximately 290 repetitive #[serde(skip_serializing_if = "Option::is_none")] field annotations have been removed across 11 Rust source files within the protocols crate. This was achieved by applying the #[serde_with::skip_serializing_none] attribute at the struct/enum level.
  • Serialization Behavior Preservation: The #[serde_with::skip_serializing_none] macro was only applied to types where every Option<T> field already had the skip_serializing_if = "Option::is_none" annotation, ensuring no changes in serialization behavior. Existing non-Option skip patterns (e.g., Vec::is_empty, HashMap::is_empty) and #[serde(flatten)] attributes were preserved.
Changelog
  • Cargo.toml
    • Added serde_with = { version = "3", features = ["macros"] } as a new dependency.
  • protocols/Cargo.toml
    • Added serde_with.workspace = true to leverage the workspace dependency.
  • protocols/src/chat.rs
    • Applied #[serde_with::skip_serializing_none] to ChatMessage, ChatCompletionRequest, ChatCompletionResponse, and ChatCompletionStreamResponse structs/enums.
    • Removed numerous #[serde(skip_serializing_if = "Option::is_none")] attributes from fields within these types.
  • protocols/src/classify.rs
    • Applied #[serde_with::skip_serializing_none] to ClassifyRequest.
    • Removed #[serde(skip_serializing_if = "Option::is_none")] attributes from optional fields.
  • protocols/src/common.rs
    • Applied #[serde_with::skip_serializing_none] to ToolCallDelta, FunctionCallDelta, Function, UsageInfo, and ErrorDetail.
    • Removed #[serde(skip_serializing_if = "Option::is_none")] attributes from optional fields.
  • protocols/src/completion.rs
    • Applied #[serde_with::skip_serializing_none] to CompletionRequest and CompletionResponse.
    • Removed #[serde(skip_serializing_if = "Option::is_none")] attributes from optional fields.
  • protocols/src/embedding.rs
    • Applied #[serde_with::skip_serializing_none] to EmbeddingRequest.
    • Removed #[serde(skip_serializing_if = "Option::is_none")] attributes from optional fields.
  • protocols/src/generate.rs
    • Applied #[serde_with::skip_serializing_none] to GenerateMetaInfo.
    • Removed #[serde(skip_serializing_if = "Option::is_none")] attributes from optional fields.
  • protocols/src/messages.rs
    • Applied #[serde_with::skip_serializing_none] to CreateMessageRequest, TextBlock, DocumentBlock, ToolResultBlock, SearchResultBlock, CustomTool, InputSchema, WebSearchTool, UserLocation, ToolChoice, Usage, MessageDelta, MessageDeltaUsage, CountMessageTokensRequest, ContainerConfig, McpServerConfig, McpToolConfiguration, McpToolResultBlock, McpToolset, McpToolDefaultConfig, McpToolConfig, CodeExecutionTool, WebFetchTool, ToolSearchTool, MemoryTool, ComputerUseTool, and BetaContentBlock.
    • Removed numerous #[serde(skip_serializing_if = "Option::is_none")] attributes from optional fields within these types.
  • protocols/src/rerank.rs
    • Applied #[serde_with::skip_serializing_none] to RerankResult.
    • Removed #[serde(skip_serializing_if = "Option::is_none")] attributes from optional fields.
  • protocols/src/responses.rs
    • Applied #[serde_with::skip_serializing_none] to ResponseTool, ResponseReasoningParam, McpToolInfo, ResponseUsage, ResponsesResponse, and TextFormat.
    • Removed #[serde(skip_serializing_if = "Option::is_none")] attributes from optional fields.
  • protocols/src/sampling_params.rs
    • Applied #[serde_with::skip_serializing_none] to SamplingParams.
    • Removed #[serde(skip_serializing_if = "Option::is_none")] attributes from optional fields.
  • protocols/src/worker_spec.rs
    • Applied #[serde_with::skip_serializing_none] to WorkerConfigRequest, WorkerInfo, WorkerUpdateRequest, and ServerInfo.
    • Removed #[serde(skip_serializing_if = "Option::is_none")] attributes from optional fields.
Activity
  • The author has implemented the solution by adding the serde_with crate and applying its skip_serializing_none macro.
  • The author has verified that cargo check -p openai-protocol passes.
  • The author has confirmed that all Vec::is_empty / HashMap::is_empty / flatten annotations were preserved.
  • The author has verified that no Option::is_none annotations remain in the transformed types.
  • The author has verified that 105 Option::is_none annotations remain in non-transformed types, as expected.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request is an excellent refactoring that significantly reduces boilerplate in the protocols crate. Using #[serde_with::skip_serializing_none] at the struct/enum level instead of per-field #[serde(skip_serializing_if = "Option::is_none")] annotations makes the code much cleaner and more maintainable.

I'm impressed by the thoroughness of this change. The detailed PR description, which includes a list of types that were intentionally skipped to avoid changing serialization behavior, shows great attention to detail. I've spot-checked several of these cases and can confirm the reasoning is sound.

Overall, this is a high-quality contribution that improves the codebase. Great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Dependency updates protocols Protocols crate changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants