diff --git a/experimental/sgl-router/README.md b/experimental/sgl-router/README.md index bac246372..d1b8eeefe 100644 --- a/experimental/sgl-router/README.md +++ b/experimental/sgl-router/README.md @@ -69,6 +69,106 @@ The Indexer replaces the Router-local radix tree as the native Cache-Aware signal. Query timeouts and local concurrency are bounded by the two Indexer options, which default to 100 ms and 32 respectively. +### Fleet-wide sampling contract + +`--override-sampling-params` fixes the sampling configuration for every client +of this router, independently of what the engine's own defaults happen to be: + +```bash +sgl-router \ + --model-id qwen3 \ + --tokenizer-path /models/qwen3/tokenizer.json \ + --worker-urls http://10.0.0.1:30000 \ + --override-sampling-params '{"temperature": 1, "top_p": 0.95, "n": 1}' \ + --sampling-param-conflict reject +``` + +It takes one JSON object keyed by the request-body field names +(`temperature`, `top_p`, `top_k`, `min_p`, `repetition_penalty`, +`frequency_penalty`, `presence_penalty`, `n`). A configured value is injected +whenever the request omits that field, so the engine's defaults cannot drift +from what the operator declared. `temperature`, `top_p`, `top_k`, `min_p` and +`repetition_penalty` are the five the engine resolves from the model's own +`generation_config`, which is what makes them drift when a deployed image +changes; the rest have fixed API defaults. + +An explicit `null` counts as omitting the field, not as a client-supplied +value: the OpenAI API types these parameters as nullable with a documented +default, so `null` asks for the default — and on a governed fleet the +configured value is what the default is. + +For a request that does send a value, `--sampling-param-conflict` decides: +`reject` (the default) 400s a differing value before admission, while `allow` +forwards the client's value untouched — the router never silently rewrites what +a client sent. A `reject` response carries +`x-router-error-code: sampling_contract_violation` and is counted in +`sgl_router_sampling_contract_rejections_total{param}`, so a rollout's blast +radius is visible per parameter rather than folded into `bad_request`. + +`reject` also 400s a value it cannot read as a number, on a governed parameter +only. The engine coerces more than JSON numbers — a bool and a numeric string +both become numbers — by rules that are undocumented and need not match across +a fleet, so a value the router cannot read is one it cannot prove conforms, and +waving it through would make the pin bypassable. The common coercions are +matched exactly (`false` is 0, `"0.5"` and `"1_0"` are 0.5 and 10), so this +refuses only genuine garbage. `allow` is unaffected: it promises nothing, so +such a value keeps flowing to the engine, which owns the request schema. + +A value may also be an inclusive band, `{"min": LO, "max": HI}`, for a +parameter that stays tunable inside a range. A band names no value to inject, +so it constrains only the requests that name the parameter; one that omits it +gets the model's own `generation_config` default, which the router cannot see. +Because a band can only ever reject, combining one with `allow` is a startup +error. + +Values are range-checked at startup, so a misconfiguration fails the launch +instead of 400ing every request at the engine. Repeating a key in the flag is +also a startup error, rather than silently enforcing whichever copy came last. + +| parameter | accepted | notes | +| --- | --- | --- | +| `temperature` | `[0, 2]` | | +| `top_p` | `(0, 1]` | | +| `top_k` | `>= 1`, or exactly `-1` | `-1` is the engine's "whole vocabulary" spelling and its default. Being non-contiguous it cannot bound a band. Note `top_k: 1` is greedy decoding, not "disabled". | +| `min_p` | `[0, 1]` | not an OpenAI parameter; the engine's domain | +| `repetition_penalty` | `(0, 2]` | not an OpenAI parameter; the engine's domain | +| `frequency_penalty` | `[-2, 2]` | | +| `presence_penalty` | `[-2, 2]` | | +| `n` | `[1, 128]` | | + +The OpenAI domains are deliberately narrower than what the engine itself +accepts (`SamplingParams.verify` would take `temperature: 5`): these values are +injected into request bodies, and a fleet contract outside the range every +OpenAI client library validates against is far more likely a typo than an +intent. + +Cost: a request that named every configured field forwards its original bytes +untouched. One that omits a field has the scalars spliced directly into the +body bytes — no parse, no re-serialize — which on a 16 MiB body is ~0.24 ms +against ~5.7 ms for a `serde_json` round-trip. Only `input_ids` and PD +bootstrap injection still parse and re-serialize, because those may have to +overwrite a key the client sent. + +### Relationship to the engine's own `--preferred-sampling-params` + +The engine has an inject-when-absent flag of its own, +`--preferred-sampling-params`, merged in +`python/sglang/srt/managers/tokenizer_manager.py` as +`{**preferred, **obj.sampling_params}`. On `/v1/chat/completions` it is +currently a no-op: `ChatCompletionRequest.to_sampling_params` +(`python/sglang/srt/entrypoints/openai/protocol.py`) resolves every sampling +key eagerly through `generation_config` and then its own defaults, so the +right-hand side of that merge is always fully populated and always wins. +(`/v1/responses`, in the same file, already omits `None` entries for exactly +this reason.) If that is fixed engine-side, `--preferred-sampling-params` +covers the inject-when-absent half for a single engine. + +What stays the router's to own either way is the enforcement half: the +`reject` immutability contract with its 400 before admission — costing no +queue slot and no engine round-trip — the `sampling_contract_violation` code +and per-parameter counter, bands, and one contract applied at a shared ingress +across engines whose own flags the router operator may not control. + ## HTTP/2 There is nothing to configure. The router negotiates per connection inbound and diff --git a/experimental/sgl-router/src/server/routes/chat.rs b/experimental/sgl-router/src/server/routes/chat.rs index 1dc19fb1d..b05e04580 100644 --- a/experimental/sgl-router/src/server/routes/chat.rs +++ b/experimental/sgl-router/src/server/routes/chat.rs @@ -1247,6 +1247,55 @@ struct BootstrapFields { room: u64, } +/// Write `members` in as top-level keys of the JSON object in `body`, without +/// parsing it. +/// +/// WHY: going through `serde_json` costs a full parse into a `Value` plus a +/// full re-serialize, over a body that runs to [`MAX_CHAT_BODY_BYTES`]. +/// +/// Members go in before the CLOSING brace so they win the last-wins reading +/// every JSON parser performs — the authority `obj.insert` has on the parse +/// path. Inserting after the opening brace would lose to a client's own later +/// copy of the key, which a request sending an explicit `null` for a governed +/// parameter has: the probe reads `null` as absent, so the value IS injected. +/// The last `}` is the object's closing brace, since `parse_probe` proved the +/// body is an object and only whitespace may follow it. +/// +/// `None` (braces not located) falls back to the parse path rather than +/// panicking on a shape `parse_probe` should already have rejected. +fn splice_top_level( + body: &Bytes, + members: &[(SamplingField, serde_json::Number)], +) -> Option { + use std::io::Write as _; + + let open = body.iter().position(|&b| b == b'{')?; + let close = body.iter().rposition(|&b| b == b'}')?; + if close <= open { + return None; + } + // An empty object takes no separating comma: `{"temperature":1}`, not + // `{,"temperature":1}`. + let has_members = body[open + 1..close] + .iter() + .any(|b| !b.is_ascii_whitespace()); + // 24 bytes per member covers `"repetition_penalty":` plus a short number; + // an over-run just costs one realloc, never correctness. + let mut out = Vec::with_capacity(body.len() + 24 * members.len() + 1); + out.extend_from_slice(&body[..close]); + for (i, (field, value)) in members.iter().enumerate() { + if has_members || i > 0 { + out.push(b','); + } + // Wire names are a fixed set of JSON-safe identifiers and a + // `serde_json::Number` renders as valid JSON, so neither needs + // escaping. Written straight into `out` — no intermediate `String`. + write!(out, "\"{}\":{}", field.wire_name(), value).ok()?; + } + out.extend_from_slice(&body[close..]); + Some(Bytes::from(out)) +} + /// Build the body forwarded to the engine, injecting (when present) the /// precomputed `input_ids`, the PD `bootstrap_*` fields and the fleet-wide /// sampling values into the already-parsed request object and serializing @@ -1258,12 +1307,12 @@ struct BootstrapFields { /// tokens, tool-call constraint and response shape it still derives from them. /// Set only when `input_ids_safe_to_forward` held. /// -/// `value` is the ingress parse when one is on hand (the cache-aware path -/// parses once at ingress); it is consumed so the mutation reuses that parse. -/// It is `None` for a load-only policy — a path that never parses at ingress — -/// so injection re-parses the bytes here. The body shape was validated by -/// `parse_probe`; the non-object arm defends against a TOCTOU regression -/// rather than panicking. +/// `value` is the ingress parse when one is on hand, reused rather than +/// repeated — and dropped unused when splicing makes it unnecessary. Sampling +/// alone never reaches `serde_json`: only `input_ids` and bootstrap injection +/// do, because those may have to OVERWRITE a key the client sent, which +/// [`splice_top_level`] cannot. The non-object arm defends against a TOCTOU +/// regression rather than panicking. fn build_outgoing_body( body: &Bytes, value: Option, @@ -1271,13 +1320,33 @@ fn build_outgoing_body( bootstrap: Option<&BootstrapFields>, sampling: &[(SamplingField, serde_json::Number)], ) -> Result { - if input_ids.is_none() && bootstrap.is_none() && sampling.is_empty() { + // `input_ids` and bootstrap injection may have to OVERWRITE a key the + // client sent, which only the parse path can do; sampling injection never + // does, because the inject-set holds only keys the request omitted. + let only_sampling = input_ids.is_none() && bootstrap.is_none(); + if only_sampling && sampling.is_empty() { // Nothing to inject — forward the original bytes (cheap Arc clone). return Ok(body.clone()); } + // Splice regardless of whether a parse is already on hand: `value` is + // read-only up to this point, so having one does not make splicing wrong — + // it only means the parse was already paid for elsewhere. Gating on + // `value.is_none()` would confine the splice to the load-only path and + // skip every configuration that parses at ingress without forwarding + // `input_ids`: the cache-aware policy, bucket routing, and a chat-encoder + // model whose request `input_ids_safe_to_forward` withholds (tools, + // multimodal, thinking). A chat-encoder model on a plain request is NOT + // one of them — there `input_ids` is `Some`, so the parse path runs + // either way. + if only_sampling { + if let Some(spliced) = splice_top_level(body, sampling) { + return Ok(spliced); + } + } let parsed = match value { Some(v) => v, - // The ingress skipped the parse, so re-parse for the injection. + // The ingress skipped the parse, so re-parse. Reached for bootstrap + // injection, and as the fallback if `splice_top_level` declined. None => serde_json::from_slice(body).map_err(|_| { ApiError::BadRequest("invalid request: body must be a JSON object".to_string()) })?, @@ -2402,6 +2471,59 @@ mod tests { ); } + /// The steady state of a governed fleet: a load-only policy on a model + /// with no chat encoder, so the ingress never parsed, and only sampling + /// scalars to add. This must NOT re-parse and re-serialize the body — + /// proven by the original bytes surviving verbatim, which a + /// `serde_json::Value` round-trip would have normalized away. + #[test] + fn build_outgoing_body_splices_sampling_without_reparsing() { + let body = Bytes::from_static(br#"{ "model" : "x" , "messages" : [ ] }"#); + let inject = apply_sampling_overrides( + &overrides_of(ConflictPolicy::Reject, r#"{"temperature": 1.0, "n": 1}"#), + &probe_of(r#"{"model":"x"}"#), + &metrics(), + ) + .unwrap(); + + let out = build_outgoing_body(&body, None, None, None, &inject).unwrap(); + assert_eq!( + std::str::from_utf8(&out).unwrap(), + r#"{ "model" : "x" , "messages" : [ ] ,"temperature":1.0,"n":1}"# + ); + let parsed: serde_json::Value = serde_json::from_slice(&out).unwrap(); + assert_eq!(parsed.get("temperature"), Some(&serde_json::json!(1.0))); + assert_eq!(parsed.get("n"), Some(&serde_json::json!(1))); + assert_eq!(parsed.get("model"), Some(&serde_json::json!("x"))); + } + + /// Splice edge cases: an empty object must not gain a trailing comma, and + /// leading whitespace before the root brace must not shift the insert. + #[test] + fn splice_top_level_handles_empty_objects_and_leading_whitespace() { + let inject = apply_sampling_overrides( + &overrides_of(ConflictPolicy::Reject, r#"{"temperature": 1.0}"#), + &probe_of(r#"{"model":"x"}"#), + &metrics(), + ) + .unwrap(); + + for (raw, want) in [ + (r#"{}"#, r#"{"temperature":1.0}"#), + (r#"{ }"#, r#"{ "temperature":1.0}"#), + ("\n\t {\"a\":1}", "\n\t {\"a\":1,\"temperature\":1.0}"), + // A `}` inside a string literal is not the closing brace. + (r#"{"a":"}"}"#, r#"{"a":"}","temperature":1.0}"#), + // Trailing whitespace stays outside the object. + ("{\"a\":1} \n", "{\"a\":1,\"temperature\":1.0} \n"), + ] { + let out = splice_top_level(&Bytes::copy_from_slice(raw.as_bytes()), &inject).unwrap(); + assert_eq!(std::str::from_utf8(&out).unwrap(), want, "input {raw:?}"); + serde_json::from_slice::(&out) + .unwrap_or_else(|e| panic!("{raw:?} spliced to invalid JSON: {e}")); + } + } + /// Nothing configured -> the body is forwarded as the same `Bytes`, with /// neither a parse nor a copy. #[test] @@ -2696,4 +2818,52 @@ mod tests { "a value at the cap is still read" ); } + + /// A request sending an explicit `null` for a governed parameter is the + /// one case where the inject-set and a key PRESENT in the body overlap: + /// the probe reads `null` as absent (the OpenAI contract), so the value is + /// injected even though the key is there. The injected value therefore has + /// to win the engine's last-wins parse — which is why members are spliced + /// in before the CLOSING brace. Inserting after the opening brace would + /// leave the client's trailing `null` authoritative and silently defeat + /// the contract. + #[test] + fn spliced_value_outranks_an_explicit_null_the_client_sent() { + let overrides = overrides_of(ConflictPolicy::Reject, r#"{"temperature": 1.0}"#); + let raw = r#"{"model":"x","temperature":null}"#; + let body = Bytes::copy_from_slice(raw.as_bytes()); + let inject = apply_sampling_overrides(&overrides, &probe_of(raw), &metrics()).unwrap(); + assert_eq!(inject.len(), 1, "null must be treated as omitted"); + + let out = build_outgoing_body(&body, None, None, None, &inject).unwrap(); + let parsed: serde_json::Value = serde_json::from_slice(&out).unwrap(); + assert_eq!( + parsed.get("temperature"), + Some(&serde_json::json!(1.0)), + "the engine must read the configured value, not the client's null: {}", + std::str::from_utf8(&out).unwrap() + ); + } + + /// The splice must also fire when the ingress ALREADY parsed the body, as + /// long as nothing needs overwriting — see the WHY on the unconditional + /// splice in `build_outgoing_body` for which configurations those are. + #[test] + fn splice_fires_even_when_a_parse_is_already_on_hand() { + let body = Bytes::from_static(br#"{ "model" : "x" }"#); + let value: serde_json::Value = serde_json::from_slice(&body).unwrap(); + let inject = apply_sampling_overrides( + &overrides_of(ConflictPolicy::Reject, r#"{"temperature": 1.0}"#), + &probe_of(r#"{"model":"x"}"#), + &metrics(), + ) + .unwrap(); + + let out = build_outgoing_body(&body, Some(value), None, None, &inject).unwrap(); + // Byte-identical to the no-parse case: the parse was dropped unused. + assert_eq!( + std::str::from_utf8(&out).unwrap(), + r#"{ "model" : "x" ,"temperature":1.0}"# + ); + } } diff --git a/experimental/sgl-router/tests/proxy/main.rs b/experimental/sgl-router/tests/proxy/main.rs index cd360f173..ef30f1628 100644 --- a/experimental/sgl-router/tests/proxy/main.rs +++ b/experimental/sgl-router/tests/proxy/main.rs @@ -24,6 +24,7 @@ mod pd_pool_isolation; mod pd_protocol_binding; mod radix_tree_routing; mod roundrobin_input_ids; +mod sampling_overrides; mod shared_prefill_admission; mod sticky_input_ids; mod sticky_routing; diff --git a/experimental/sgl-router/tests/proxy/sampling_overrides.rs b/experimental/sgl-router/tests/proxy/sampling_overrides.rs new file mode 100644 index 000000000..3b4826f6b --- /dev/null +++ b/experimental/sgl-router/tests/proxy/sampling_overrides.rs @@ -0,0 +1,314 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 The SGLang Authors +// SPDX-License-Identifier: Apache-2.0 + +//! `--override-sampling-params` end to end: from the CLI flag an operator +//! writes in a manifest to the JSON body the engine actually receives. +//! +//! The unit tests in `config::sampling` and `server::routes::chat` cover +//! parsing and the per-parameter decision; these drive the whole path, because +//! the failure this flag exists to prevent (an engine serving sampling +//! parameters the operator did not declare) is only observable on the wire. + +use axum::body::Body; +use axum::http::{Request, StatusCode}; +use serde_json::{json, Value}; +use sgl_router::config::{Cli, Config}; +use sgl_router::discovery::{ModelId, WorkerId, WorkerMode, WorkerSpec}; +use sgl_router::policies::factory::build_registry_with_defaults; +use sgl_router::proxy::Proxy; +use sgl_router::server::app::build_router; +use sgl_router::server::app_context::AppContext; +use sgl_router::tokenizer::TokenizerRegistry; +use sgl_router::workers::WorkerRegistry; +use std::sync::Arc; +use std::time::Duration; +use tower::ServiceExt; + +use crate::common::mock_worker::MockWorker; + +const MODEL: &str = "tiny"; + +const OVERRIDES: &str = r#"{"temperature": 1, "top_p": 0.95, "top_k": 1000, + "frequency_penalty": 0, "presence_penalty": 0, "n": 1}"#; + +/// Build the config the way a deployment does — through `Cli`, so what these +/// tests pin is the flag spelling in a manifest, not a hand-built struct that +/// could drift from what the parser produces. +fn config(flags: &[&str]) -> Config { + let mut argv = vec![ + "sgl-router", + "--model-id", + MODEL, + "--tokenizer-path", + "tests/fixtures/tiny_tokenizer.json", + "--worker-urls", + "http://placeholder:0", + ]; + argv.extend_from_slice(flags); + ::parse_from(argv) + .into_config() + .expect("flags must parse") +} + +fn build_ctx(url: String, flags: &[&str]) -> Arc { + let cfg = config(flags); + let tokenizers = Arc::new(TokenizerRegistry::load_from_config(&cfg).unwrap()); + let registry = Arc::new(WorkerRegistry::default()); + let _ = registry.add(WorkerSpec { + id: WorkerId(url.clone()), + url, + mode: WorkerMode::Plain, + model_ids: vec![ModelId(MODEL.into())], + bootstrap_port: None, + }); + let policies = Arc::new(build_registry_with_defaults(&cfg).unwrap()); + let proxy = Arc::new(Proxy::new(Duration::from_secs(5)).unwrap()); + Arc::new(AppContext::new(cfg, tokenizers, proxy, registry, policies)) +} + +async fn send(ctx: Arc, body: Value) -> StatusCode { + send_raw(ctx, serde_json::to_vec(&body).unwrap()).await.0 +} + +/// Send a body verbatim, so a test can express what `serde_json::Value` +/// cannot — a repeated key. Returns the status and the router's own error +/// code header. +async fn send_raw(ctx: Arc, body: Vec) -> (StatusCode, Option) { + let req = Request::builder() + .method("POST") + .uri("/v1/chat/completions") + .header("content-type", "application/json") + .body(Body::from(body)) + .unwrap(); + let resp = build_router(ctx).oneshot(req).await.unwrap(); + let code = resp + .headers() + .get("x-router-error-code") + .and_then(|v| v.to_str().ok()) + .map(str::to_owned); + (resp.status(), code) +} + +fn captured(mock: &MockWorker) -> Option { + let b = mock.captured.lock().unwrap().last_body.clone()?; + Some(serde_json::from_slice(&b).expect("captured body is valid JSON")) +} + +/// The values an operator configures replace the engine's own defaults: a +/// request that names no sampling parameter reaches the engine carrying every +/// configured one. +#[tokio::test] +async fn configured_values_reach_the_engine_when_the_request_omits_them() { + let mock = MockWorker::start(vec![]).await; + let ctx = build_ctx(mock.url.clone(), &["--override-sampling-params", OVERRIDES]); + let status = send( + ctx, + json!({"model": MODEL, "messages": [{"role": "user", "content": "hi"}]}), + ) + .await; + assert_eq!(status, StatusCode::OK); + + let body = captured(&mock).expect("worker received a request"); + assert_eq!(body.get("temperature"), Some(&json!(1))); + assert_eq!(body.get("top_p"), Some(&json!(0.95))); + assert_eq!(body.get("top_k"), Some(&json!(1000))); + assert_eq!(body.get("frequency_penalty"), Some(&json!(0))); + assert_eq!(body.get("presence_penalty"), Some(&json!(0))); + assert_eq!(body.get("n"), Some(&json!(1))); +} + +/// Under the default `reject` mode a conflicting request is a 400 that never +/// reaches a worker — the contract costs no engine round-trip and no +/// admission slot. +#[tokio::test] +async fn reject_mode_400s_a_conflicting_request_without_touching_the_engine() { + // Covers the whole rejection contract: status, wire error code, and that + // the engine is never reached. + let mock = MockWorker::start(vec![]).await; + let ctx = build_ctx(mock.url.clone(), &["--override-sampling-params", OVERRIDES]); + let body = json!({ + "model": MODEL, + "messages": [{"role": "user", "content": "hi"}], + "temperature": 0.7, + }); + let (status, code) = send_raw(ctx, serde_json::to_vec(&body).unwrap()).await; + assert_eq!(status, StatusCode::BAD_REQUEST); + // Its own error code, so an operator rolling `reject` across a fleet can + // alert on contract rejections without them being buried among the + // `bad_request`s from clients sending malformed JSON. + assert_eq!(code.as_deref(), Some("sampling_contract_violation")); + assert!( + captured(&mock).is_none(), + "a rejected request must not reach the engine" + ); +} + +/// A repeated sampling key must not become a router-side 400: it is legal JSON +/// that every engine reads last-wins, and the router forwarded it before these +/// fields were probed. The contract judges the value the engine will use. +#[tokio::test] +async fn duplicate_sampling_key_is_judged_on_its_last_value() { + // Last value agrees with the contract -> served. + let mock = MockWorker::start(vec![]).await; + let ctx = build_ctx(mock.url.clone(), &["--override-sampling-params", OVERRIDES]); + let (status, _) = send_raw( + ctx, + br#"{"model":"tiny","messages":[{"role":"user","content":"hi"}], + "temperature":0.7,"temperature":1}"# + .to_vec(), + ) + .await; + assert_eq!(status, StatusCode::OK); + assert_eq!( + captured(&mock).and_then(|b| b.get("temperature").cloned()), + Some(json!(1)), + "the engine must see the last value" + ); + + // Last value disagrees -> rejected, even though the first one matched. + let mock = MockWorker::start(vec![]).await; + let ctx = build_ctx(mock.url.clone(), &["--override-sampling-params", OVERRIDES]); + let (status, _) = send_raw( + ctx, + br#"{"model":"tiny","messages":[{"role":"user","content":"hi"}], + "temperature":1,"temperature":0.7}"# + .to_vec(), + ) + .await; + assert_eq!(status, StatusCode::BAD_REQUEST); +} + +/// `temperature`, `top_p`, `top_k`, `min_p` and `repetition_penalty` are the +/// parameters the engine resolves from the model's own `generation_config`, so +/// they are the ones a fleet-wide pin exists for. Drive them the whole way to +/// the engine. +#[tokio::test] +async fn engine_defaulted_parameters_reach_the_engine() { + let mock = MockWorker::start(vec![]).await; + let ctx = build_ctx( + mock.url.clone(), + &[ + "--override-sampling-params", + r#"{"min_p": 0.05, "repetition_penalty": 1.1}"#, + ], + ); + let status = send( + ctx, + json!({"model": MODEL, "messages": [{"role": "user", "content": "hi"}]}), + ) + .await; + assert_eq!(status, StatusCode::OK); + let body = captured(&mock).expect("engine must receive a body"); + assert_eq!(body.get("min_p"), Some(&json!(0.05))); + assert_eq!(body.get("repetition_penalty"), Some(&json!(1.1))); +} + +/// The same request under `allow` is forwarded with the client's value intact, +/// and the parameters it did not name still get the configured ones. +#[tokio::test] +async fn allow_mode_forwards_the_client_value_and_fills_the_rest() { + let mock = MockWorker::start(vec![]).await; + let ctx = build_ctx( + mock.url.clone(), + &[ + "--override-sampling-params", + OVERRIDES, + "--sampling-param-conflict", + "allow", + ], + ); + let status = send( + ctx, + json!({ + "model": MODEL, + "messages": [{"role": "user", "content": "hi"}], + "temperature": 0.7, + }), + ) + .await; + assert_eq!(status, StatusCode::OK); + + let body = captured(&mock).expect("worker received a request"); + assert_eq!(body.get("temperature"), Some(&json!(0.7))); + assert_eq!(body.get("top_p"), Some(&json!(0.95))); + assert_eq!(body.get("top_k"), Some(&json!(1000))); + assert_eq!(body.get("n"), Some(&json!(1))); +} + +/// A band accepts anything inside it, 400s outside, and injects nothing: +/// temperature tunable in [0, 1], everything else fixed. +#[tokio::test] +async fn a_temperature_band_admits_in_range_values_and_injects_nothing() { + let flags = [ + "--override-sampling-params", + r#"{"temperature": {"min": 0, "max": 1}, "top_p": 0.95}"#, + ]; + + let mock = MockWorker::start(vec![]).await; + let ctx = build_ctx(mock.url.clone(), &flags); + let status = send( + ctx, + json!({ + "model": MODEL, + "messages": [{"role": "user", "content": "hi"}], + "temperature": 0.6, + }), + ) + .await; + assert_eq!(status, StatusCode::OK); + let body = captured(&mock).expect("worker received a request"); + assert_eq!(body.get("temperature"), Some(&json!(0.6))); + assert_eq!(body.get("top_p"), Some(&json!(0.95))); + + // Omitted: the band names no value, so the engine's own default applies. + let mock = MockWorker::start(vec![]).await; + let ctx = build_ctx(mock.url.clone(), &flags); + let status = send( + ctx, + json!({"model": MODEL, "messages": [{"role": "user", "content": "hi"}]}), + ) + .await; + assert_eq!(status, StatusCode::OK); + let body = captured(&mock).expect("worker received a request"); + assert_eq!(body.get("temperature"), None); + + // Outside the band: 400. + let mock = MockWorker::start(vec![]).await; + let ctx = build_ctx(mock.url.clone(), &flags); + let status = send( + ctx, + json!({ + "model": MODEL, + "messages": [{"role": "user", "content": "hi"}], + "temperature": 1.5, + }), + ) + .await; + assert_eq!(status, StatusCode::BAD_REQUEST); + assert!(captured(&mock).is_none()); +} + +/// With the flag unset the router touches nothing: the body the client sent +/// is the body the engine sees, including a sampling parameter the operator +/// could have fixed. +#[tokio::test] +async fn unset_flag_forwards_the_body_untouched() { + let mock = MockWorker::start(vec![]).await; + let ctx = build_ctx(mock.url.clone(), &[]); + let status = send( + ctx, + json!({ + "model": MODEL, + "messages": [{"role": "user", "content": "hi"}], + "temperature": 0.7, + }), + ) + .await; + assert_eq!(status, StatusCode::OK); + + let body = captured(&mock).expect("worker received a request"); + assert_eq!(body.get("temperature"), Some(&json!(0.7))); + assert_eq!(body.get("top_p"), None); + assert_eq!(body.get("top_k"), None); + assert_eq!(body.get("n"), None); +}