Repository navigation
[Router] Fleet-wide sampling contract 1/3: the config surface - #39000
Merged
Merged
Conversation
Stack 1/3 of `--override-sampling-params`: the type an operator's contract is
expressed in, the flag that parses it, and the startup validation. Nothing
reads it on the request path yet — enforcement and injection are 2/3, and with
the flag unset (the default) there is no behavior change at all.
An operator running a hosted endpoint often needs the sampling contract to be
theirs, not the engine's: one fixed configuration for every client of a router,
independent of what the deployed image's defaults happen to be. The concrete
case is serving Kimi models the way Moonshot's own API serves them, which the
Kimi-Vendor-Verifier `tests/params` suite checks: for a governed parameter a
client may omit it, send exactly the configured value, or get a 400.
Two flags, opt-in; unset changes nothing.
--override-sampling-params '{"temperature": 1, "top_p": 0.95}'
--sampling-param-conflict reject # the default
The object is keyed by request-body field names: temperature, top_p, top_k,
min_p, repetition_penalty, frequency_penalty, presence_penalty, n. The first
five are the ones the engine resolves from the model's own `generation_config`
and so the ones that drift when a deployed image changes; the rest have fixed
API defaults. Each value is a number, or an inclusive band
`{"min": LO, "max": HI}` for a parameter that stays tunable inside a range.
Everything an operator can get wrong fails the launch, naming the key and its
domain: unknown or repeated key, wrong type, out-of-domain value, non-integral
top_k/n, an integral literal past the i64 range, inverted or partial band, a
band under `allow`. The flag object and each band are decoded as ordered
entries rather than a `serde_json::Map`, because a map keeps only the last of a
repeated key and would silently enforce a value nobody wrote. The same checks
run from `Config::validate`, so a `SamplingOverrides` built by any other path
(a test fixture, a future config file, an admin API) cannot hold a spec the
parser would have refused.
A band under `allow` is a startup error rather than a no-op, because `allow`
never rejects and a band names no value to inject, so the pair would govern
nothing. `ParamSpec::Exact` holds the parsed `serde_json::Number` rather than
an `f64` so injection re-emits the operator's literal: `"n": 1` stays `1` and
does not reach the engine as `1.0`.
Tests: 12 unit tests on the parser (every parameter, bands, integral
normalization, the top_k sentinel, the i64 boundary, `ALL`/`index` agreement,
`validate` refusing hand-built specs, and a 31-case malformed-input table), and
3 on the flags reaching `ModelConfig` — including that `--sampling-param-conflict`
alone is refused by clap, since the mode governs nothing on its own.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GZEJhYGNbjKAS89iMMkXiY
This was referenced Sep 11, 2026
sherlockwu
approved these changes
Sep 15, 2026
Kangyan-Zhou
pushed a commit
that referenced
this pull request
Sep 16, 2026
`cargo check --all-targets` fails on main as of faaff1e, so every open router PR is red on lint through no fault of its own: error[E0063]: missing field `sampling_overrides` in initializer of `sgl_router::config::ModelConfig` --> tests/proxy/pd_protocol_binding.rs:132:16 A semantic conflict between two individually-green PRs. #39000 added the field to `ModelConfig`; #39006 added `pd_protocol_binding.rs` with an exhaustive `ModelConfig` literal, written before that field existed. Each passed its own CI and the textual merge is clean, so nothing caught it. The fixture spells its other defaulted fields `Default::default()`, so this matches rather than naming the type and adding an import. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FH8igzPYMnqMWrWy8o2xKM
4 of 5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
An operator running a hosted endpoint often needs the sampling contract to be
theirs, not the engine's: one fixed configuration for every client of a
router, independent of what the deployed image's defaults happen to be. The
concrete case is serving Kimi models the way Moonshot's own API serves them,
which the Kimi-Vendor-Verifier
tests/paramssuite checks — for a governedparameter a client may omit it, send exactly the configured value, or get a
400.
Stack 1 of 3. This PR adds only the config surface: the type the contract
is expressed in, the flag that parses it, and the startup validation. Nothing
reads it on the request path yet.
SamplingOverridestype, parser,Config::validatehook, the two flagsWith the flag unset — the default — there is no behavior change at all.
Modifications
Two flags, opt-in:
The object is keyed by request-body field names:
temperature,top_p,top_k,min_p,repetition_penalty,frequency_penalty,presence_penalty,n. The first five are the ones the engine resolves fromthe model's own
generation_config, and so the ones that drift when a deployedimage changes; the rest have fixed API defaults. Each value is a number, or an
inclusive band
{"min": LO, "max": HI}for a parameter that stays tunableinside a range.
experimental/sgl-router/src/config/sampling.rs(new) —SamplingOverrides,ConflictPolicy,ParamSpec,SamplingField, the hand-written parser andthe shared
validate_spec.experimental/sgl-router/src/config/cli.rs— the two flags;--sampling-param-conflictrequires--override-sampling-params, sincethe mode governs nothing on its own.
experimental/sgl-router/src/config/types.rs—ModelConfig.sampling_overrides,defaulting to empty.
experimental/sgl-router/src/config/mod.rs—Config::validatecallsSamplingOverrides::validate.ModelConfigliterals across src and tests gainsampling_overrides: Default::default().Everything an operator can get wrong fails the launch, naming the key and its
domain: unknown or repeated key, wrong type, out-of-domain value, non-integral
top_k/n, an integral literal past the i64 range, inverted or partial band,a band under
allow. The flag object and each band are decoded as orderedentries rather than a
serde_json::Map, because a map keeps only the last ofa repeated key and would silently enforce a value nobody wrote. The same checks
run from
Config::validate, so aSamplingOverridesbuilt by any other path(a test fixture, a future config file, an admin API) cannot hold a spec the
parser would have refused.
A band under
allowis a startup error rather than a no-op:allowneverrejects and a band names no value to inject, so the pair would govern nothing.
ParamSpec::Exactholds the parsedserde_json::Numberrather than anf64so injection re-emits the operator's literal —
"n": 1stays1and does notreach the engine as
1.0.The accepted domains are the OpenAI ones, deliberately narrower than what
SamplingParams.verifywould take: these values get injected into requestbodies, and a fleet contract outside the range every OpenAI client library
validates against is far more likely a typo than an intent.
The clap help for
--override-sampling-paramspoints at a router READMEsection that lands in stack 3 with the rest of the operator docs.
Accuracy Tests
Not applicable — router-side config parsing, no model code touched.
Speed Tests and Profiling
Not applicable — parsed once at startup.
Checklist
Tests:
cargo test708 passed across five suites,cargo clippy --all-targets -- -D warningsandcargo fmt --checkgreen.12 unit tests on the parser (every parameter, bands, integral normalization,
the
top_ksentinel, the i64 boundary,ALL/indexagreement,validaterefusing hand-built specs, and a 31-case malformed-input table); 3 on the flags
reaching
ModelConfig, including that--sampling-param-conflictalone isrefused by clap.
🤖 Generated with Claude Code
https://claude.ai/code/session_01GZEJhYGNbjKAS89iMMkXiY
CI States
Latest PR Test (Base): ✅ Run #34829788126
Latest PR Test (Extra): ❌ Run #34829787845
Latest PR Test (AMD ROCm 10): ➖ No AMD PR run found for this commit.