feat(protocols): add serde(flatten) to ChatCompletionRequest for engine-specific fields

[DavidBellamy]

Summary

• ChatCompletionRequest silently drops request fields not explicitly defined in the struct. This prevents engine-specific parameters (e.g. sglang's return_routed_experts for MoE routing replay) from passing through the gateway to backend workers.
• CompletionRequest already has a #[serde(flatten)] pub other: Map<String, Value> catch-all. This PR makes ChatCompletionRequest consistent by adding the same pattern.
• All existing construction sites use ..Default::default(), so the new field is automatically initialized to an empty map with no code changes needed elsewhere.

Fixes: sgl-project/sglang#22740

Summary by CodeRabbit

• New Features
  • Chat completion requests and responses now accept and include additional engine-specific JSON properties. This improves compatibility with varied chat engines, lets custom parameters pass through unchanged, and increases flexibility without altering existing validation or normalization behavior.

[coderabbitai]

Warning

Rate limit exceeded

@DavidBellamy has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 55 minutes and 32 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 55 minutes and 32 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 80899985-29ca-4704-9e4e-9735c213363c

📥 Commits

Reviewing files that changed from the base of the PR and between 46fa164 and 8ac5695.

📒 Files selected for processing (1)

• crates/protocols/src/chat.rs

📝 Walkthrough

Walkthrough

The ChatCompletionRequest struct in crates/protocols/src/chat.rs gained a flattened catch‑all field other: serde_json::Map<String, serde_json::Value> so unknown JSON properties are preserved during (de)serialization.

Changes

Cohort / File(s)	Summary
Protocol Data Structure crates/protocols/src/chat.rs	Imported serde_json::Map/Value and added #[serde(flatten)] pub other: Map<String, Value> to ChatCompletionRequest to capture and re-emit unknown engine-specific JSON fields.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• feat(protocols): add serde(flatten) to ChatCompletionRequest for engine-specific fields #1119 — Adds #[serde(flatten)] pub other: Map<String, Value> to ChatCompletionRequest upstream (directly related fix).
• fix(protocols): accept null for boolean fields logprobs and stream #1020 — Recent modifications to ChatCompletionRequest serde attributes; overlaps in affected area and serialization concerns.

Suggested reviewers

• key4ng

Poem

🐰
I nibble at bytes beneath moonlight's glow,
Extra fields now safe where wild JSON go.
Flattened and tucked in a cozy map,
No stray key lost on the forwarding lap.
Hoppity hooray — preserved in its wrap!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and specifically describes the main change: adding serde(flatten) to ChatCompletionRequest for engine-specific fields.
Linked Issues check	✅ Passed	The PR successfully implements the required fix by adding #[serde(flatten)] pub other: Map<String, Value> to ChatCompletionRequest, matching the pattern on CompletionRequest and addressing issue #22740's requirement.
Out of Scope Changes check	✅ Passed	All changes are scoped to the stated objective: updating ChatCompletionRequest with a serde(flatten) field to preserve unknown JSON fields, with no unrelated modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[mergify]

Hi @DavidBellamy, the DCO sign-off check has failed. All commits must include a Signed-off-by line.

To fix existing commits:

# Sign off the last N commits (replace N with the number of unsigned commits)
git rebase HEAD~N --signoff
git push --force-with-lease

To sign off future commits automatically:

• Use git commit -s every time, or
• VSCode: enable Git: Always Sign Off in Settings
• PyCharm: enable Sign-off commit in the Commit tool window

[gemini-code-assist]

Code Review

This pull request updates the ChatCompletionRequest struct in crates/protocols/src/chat.rs to include an other field of type Map<String, Value> with the #[serde(flatten)] attribute. This change allows the struct to capture additional, non-explicitly defined fields from incoming JSON requests. I have no feedback to provide as there were no review comments.

[slin1237]

thanks for the fix, lgtm
please address ci failures

[DavidBellamy]

@slin1237 Thanks for the review! I've addressed the CI failures:

• DCO sign-off: Added via rebase
• PR title: Updated to conventional commits format (feat(protocols): ...)

The new workflow runs need maintainer approval to execute (fork PR after force push). Could you approve them?

Re: the openai-responses failures from the previous run, all 5 were flaky MCP tool-calling tests (test_mcp_basic_tool_call_streaming, test_mcp_multi_server_tool_call, test_mcp_multi_server_tool_call_streaming, etc.) that failed after multiple retries. Unrelated to this one-line serde change. Should clear on re-run.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/protocols/src/chat.rs`:
- Around line 320-322: Add a unit test that verifies serde(flatten) preserves
unknown fields: create a JSON string for the chat request that includes an extra
key like "return_routed_experts": true, deserialize it with serde_json::from_str
into the struct that defines the #[serde(flatten)] pub other: Map<String,
Value>, assert that request.other.get("return_routed_experts") exists and equals
Value::Bool(true), then re-serialize the struct with serde_json::to_string and
assert the resulting JSON contains the "return_routed_experts" key (or that
serde_json::from_str of the re-serialized JSON still yields the same other
entry). Use the struct with the other field and serde_json utilities to
implement the test.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 19466c72-c51d-4bb7-8982-e8ac869383e1

📥 Commits

Reviewing files that changed from the base of the PR and between ed64c29 and 46fa164.

📒 Files selected for processing (1)

• crates/protocols/src/chat.rs

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Add a round-trip regression test for unknown-field preservation.

Please add a test that deserializes a request with an extra key (e.g., return_routed_experts), asserts it lands in other, and verifies it is present after re-serialization.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@crates/protocols/src/chat.rs` around lines 320 - 322, Add a unit test that
verifies serde(flatten) preserves unknown fields: create a JSON string for the
chat request that includes an extra key like "return_routed_experts": true,
deserialize it with serde_json::from_str into the struct that defines the
#[serde(flatten)] pub other: Map<String, Value>, assert that
request.other.get("return_routed_experts") exists and equals Value::Bool(true),
then re-serialize the struct with serde_json::to_string and assert the resulting
JSON contains the "return_routed_experts" key (or that serde_json::from_str of
the re-serialized JSON still yields the same other entry). Use the struct with
the other field and serde_json utilities to implement the test.

[DavidBellamy]

Replaced by #1130 (branch renamed to conform to Mergify naming convention).