Validate chat completions logprobs parameters

[lloydmak99]

Problem

POST /v1/chat/completions accepted invalid OpenAI sampling parameters and sent them to inference:

• top_logprobs without logprobs: true
• top_logprobs outside the OpenAI range 0..=20
• stop arrays with more than 4 sequences

That masks client bugs and can spend inference on requests that should be rejected at the API boundary.

Fixes #613

Change

• Validate top_logprobs from the flattened request extra map.
• Require logprobs: true when top_logprobs is present.
• Reject non-integer top_logprobs values and values outside 0..=20.
• Treat explicit top_logprobs: null as unset, matching OpenAI-style optional field behavior.
• Reject stop arrays with more than 4 sequences while preserving single-string stop and arrays of up to 4 entries.
• Add focused ChatCompletionRequest::validate regression tests for the invalid and valid cases.

What Else Is Needed

• No migration or deployment choreography is needed. This is an API-boundary validation change.
• The broader validation gaps mentioned later in the issue thread, such as Responses metadata limits and Files list cursor validation, are not covered by this PR and should stay as separate follow-up work.

Tests

• cargo test -p api --lib chat_completion_
• cargo fmt --all -- --check
• cargo clippy --all-targets --all-features -- -D warnings
• cargo test --lib --bins

Not Run

• cargo test -p api chat_completion_ also selected e2e tests and failed locally because Postgres was not running (Connection refused from db_setup.rs). The scoped API library test above covers this change without requiring the e2e database.

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[claude]

Review: Validate chat completions logprobs parameters

Clean, well-scoped change. Verified that logprobs / top_logprobs correctly land in the flattened extra map (not typed fields on ChatCompletionRequest), validate() is invoked on every completions route, and the new tests cover the boundary cases (0, 20, minus-1, 21, non-integer, single-string stop, 4 vs 5 sequences). No critical issues.

Minor (non-blocking) notes:

1. Explicit null is treated as an error, not absent. With serde flatten into a HashMap of Value, a body with top_logprobs set to null stores Value::Null, so as_i64() returns None and is rejected with "top_logprobs must be an integer". OpenAI treats explicit null as unset. Likewise logprobs=null + top_logprobs=5 fails the logprobs==true check. Some SDKs serialize unset optionals as null, so this is a small backward-compat sharp edge. To match OpenAI exactly, guard with .filter(|v| !v.is_null()) before validating.

2. Integer-valued floats are rejected: as_i64() returns None for 2.0, so top_logprobs=2.0 errors. Strictly correct per OpenAI integer typing — flagging only so it is a conscious choice.

Neither blocks merge; the stricter behavior is defensible at the API boundary.

Approved (no critical issues found).

[PierreLeGuen]

crates/api/src/models.rs:1097: the new validation only runs when top_logprobs is present, so malformed logprobs by itself still passes ChatCompletionRequest::validate() (for example {"logprobs":"true"} or {"logprobs":1}). Because logprobs is captured by the flattened extra map and forwarded through convert_chat_request_to_service, those invalid requests still reach providers and get provider-dependent failures or behavior instead of the local invalid_request_error this PR is adding. Please validate extra["logprobs"] independently as a boolean (allowing null/unset if desired) and add a regression test for invalid logprobs without top_logprobs.

Local checks run:

• cargo test -p api models::tests::test_chat_completion_top_logprobs --lib
• cargo test -p api models::tests::test_chat_completion_stop_array_may_contain_at_most_four_sequences --lib
• cargo fmt --check

[lloydmak99]

Addressed in 2a8f764: logprobs is now validated independently as a boolean when present/non-null, and top_logprobs reuses that parsed value for the logprobs == true requirement. Added regression coverage for malformed standalone logprobs values and explicit logprobs: null as unset. Local checks: cargo test -p api models::tests::test_chat_completion_logprobs --lib; cargo test -p api models::tests::test_chat_completion_top_logprobs --lib; cargo test -p api models::tests::test_chat_completion_stop_array_may_contain_at_most_four_sequences --lib; cargo fmt --all -- --check.

[PierreLeGuen]

Reviewed the validation changes in crates/api/src/models.rs, the chat completions handler/conversion path, and the provider forwarding path for flattened extras. The previous standalone logprobs gap is covered now, and the stop/top_logprobs validation is exercised by focused tests.

Local checks run:

• cargo test -p api --lib chat_completion_
• cargo fmt --all -- --check

GitHub checks were green for lint, test suite, and security audit on head 2a8f764.

[Evrard-Nil]

🤖 Reviewed independently at 2a8f764 (built, cargo test -p api --lib → 142 pass incl. the 7 new tests; fmt/clippy clean). Logprobs validation is correct — approving, with one thing to disclose.

The logprobs logic matches the OpenAI chat-completions contract exactly (validated in ChatCompletionRequest::validate(), reading from the #[serde(flatten)] extra map, which is the right source):

• top_logprobs present without logprobs:true → 400 (covers both absent and false) ✓
• top_logprobs range 0–20 exact, no off-by-one (!(0..=20).contains(...)) ✓; logprobs:true alone allowed; top_logprobs:0+true allowed; negatives/21 rejected ✓
• non-boolean logprobs / non-integer top_logprobs → 400; null treated as unset ✓
• Returns 400 invalid_request_error before forwarding (completions.rs:1102), correctly scoped to chat completions only (legacy /v1/completions int-logprobs path untouched), no-op when unset, streaming + non-streaming both covered. Well tested.

⚠️ One CONCERN — please disclose/confirm: this PR also bundles an undocumented stop-array validation (models.rs:1091-1095: "stop array may contain at most 4 sequences") not mentioned in the title or description. It's OpenAI-spec-correct (stop ≤ 4), but it's a real behavior change riding in a "logprobs" PR: chat requests with 5+ stop strings will now get a 400 where they previously passed through to the backend — and self-hosted vLLM/SGLang do not enforce a 4-stop cap, so this newly rejects requests those engines would have served. Please (a) call it out in the PR description, and (b) confirm enforcing the OpenAI 4-cap is intended for our self-hosted models (vs. letting the backend decide). Clear 400 rather than silent breakage, so not blocking — but reviewers approving "logprobs" shouldn't miss it.

NITs: float top_logprobs like 2.0 is rejected as non-integer (spec-compatible); no unit round-trip test for the valid happy path (covered by the existing e2e). Approving on the logprobs correctness; please address the stop-array disclosure.