From e8735d7b7a7e6476f7fb1186a1300086a6db63de Mon Sep 17 00:00:00 2001 From: plotarmordev <299844489+plotarmordev@users.noreply.github.com> Date: Thu, 10 Sep 2026 14:11:35 +0800 Subject: [PATCH] feat(perf): add explicit enable-thinking control --- crates/grill-perf/src/model.rs | 14 +++ crates/grill-perf/src/wire.rs | 21 ++++- crates/grill-perf/tests/cli.rs | 162 +++++++++++++++++++++++++++++++++ docs/performance/CONTRACT.md | 17 +++- 4 files changed, 209 insertions(+), 5 deletions(-) diff --git a/crates/grill-perf/src/model.rs b/crates/grill-perf/src/model.rs index 36cee0b..35e7dd9 100644 --- a/crates/grill-perf/src/model.rs +++ b/crates/grill-perf/src/model.rs @@ -57,6 +57,14 @@ pub struct RequestSettings { // Omitted when absent so plans recorded before this field keep their digest. #[serde(skip_serializing_if = "Option::is_none")] pub thinking: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub thinking_control: Option, +} +#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, Eq)] +#[serde(tag = "kind", deny_unknown_fields)] +pub enum ThinkingControl { + #[serde(rename = "vllm-enable-thinking-v1")] + VllmEnableThinkingV1 { enabled: bool }, } #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)] #[serde(deny_unknown_fields)] @@ -172,9 +180,15 @@ impl Workload { { return Err("invalid output budget or sampling controls".into()); } + if r.thinking.is_some() && r.thinking_control.is_some() { + return Err( + "request.thinking and request.thinking_control are mutually exclusive".into(), + ); + } if r.profile == Profile::PortableChatV1 && (r.output.mode == OutputMode::Exact || r.cache != Cache::Observe + || r.thinking_control.is_some() || r.thinking.is_some()) { return Err("exact output, required prefix evidence, and thinking controls need the explicit vllm-fixed-v1 request profile".into()); diff --git a/crates/grill-perf/src/wire.rs b/crates/grill-perf/src/wire.rs index 815c65f..c09e29b 100644 --- a/crates/grill-perf/src/wire.rs +++ b/crates/grill-perf/src/wire.rs @@ -12,8 +12,10 @@ struct StreamOptions { include_usage: bool, } #[derive(Serialize)] -struct ChatTemplateKwargs { - thinking: bool, +#[serde(untagged)] +enum ChatTemplateKwargs { + Legacy { thinking: bool }, + EnableThinking { enable_thinking: bool }, } #[derive(Serialize)] struct Body<'a> { @@ -112,7 +114,20 @@ pub fn request_body(plan: &Plan, wave: &WaveSpec, lane: u32) -> Result { seed: r .seed .map(|seed| seed + i64::from(wave.trial) * 64 + i64::from(lane)), - chat_template_kwargs: r.thinking.map(|thinking| ChatTemplateKwargs { thinking }), + chat_template_kwargs: match (r.thinking, r.thinking_control) { + (Some(thinking), None) => Some(ChatTemplateKwargs::Legacy { thinking }), + (None, Some(ThinkingControl::VllmEnableThinkingV1 { enabled })) => { + Some(ChatTemplateKwargs::EnableThinking { + enable_thinking: enabled, + }) + } + (None, None) => None, + (Some(_), Some(_)) => { + return Err( + "request.thinking and request.thinking_control are mutually exclusive".into(), + ); + } + }, stream_options: r.stream.then_some(StreamOptions { include_usage: true, }), diff --git a/crates/grill-perf/tests/cli.rs b/crates/grill-perf/tests/cli.rs index d73334f..abea496 100644 --- a/crates/grill-perf/tests/cli.rs +++ b/crates/grill-perf/tests/cli.rs @@ -808,6 +808,168 @@ fn portable_thinking_is_rejected_before_dispatch() { assert_eq!(server.count.load(Ordering::SeqCst), 1); } +#[test] +fn enable_thinking_is_explicit_identity_and_does_not_require_answer_first() { + let temp = Temp::new(); + let server = Server::new(normal); + for (name, enabled) in [("off", false), ("on", true)] { + let mut w = workload(2, 1, 1); + w["request"]["profile"] = json!("vllm-fixed-v1"); + w["request"]["thinking_control"] = + json!({"kind":"vllm-enable-thinking-v1","enabled":enabled}); + successful(&run(&temp, &server, name, &w)); + let seen: Vec<_> = server.seen.try_iter().collect(); + assert_eq!(seen.len(), 4); + for body in seen { + assert_eq!( + body["chat_template_kwargs"], + json!({"enable_thinking":enabled}) + ); + } + assert_eq!( + wave(&temp, name, 0)["attempts"][0]["timing"]["first_generated_channel"], + "reasoning" + ); + successful( + &cli() + .arg("compare") + .arg(temp.path(name)) + .arg(temp.path(name)) + .arg("--json") + .output() + .unwrap(), + ); + } + let compared = cli() + .arg("compare") + .arg(temp.path("off")) + .arg(temp.path("on")) + .arg("--json") + .output() + .unwrap(); + assert_eq!(compared.status.code(), Some(1)); +} + +#[test] +fn thinking_control_rejects_conflicts_profiles_and_open_extensions_before_dispatch() { + let temp = Temp::new(); + let server = Server::new(normal); + for (name, profile, legacy, control) in [ + ( + "agree", + "vllm-fixed-v1", + json!(false), + json!({"kind":"vllm-enable-thinking-v1","enabled":false}), + ), + ( + "disagree", + "vllm-fixed-v1", + json!(true), + json!({"kind":"vllm-enable-thinking-v1","enabled":false}), + ), + ( + "portable", + "portable-chat-v1", + Value::Null, + json!({"kind":"vllm-enable-thinking-v1","enabled":false}), + ), + ( + "kind", + "vllm-fixed-v1", + Value::Null, + json!({"kind":"other","enabled":false}), + ), + ( + "field", + "vllm-fixed-v1", + Value::Null, + json!({"kind":"vllm-enable-thinking-v1","enabled":false,"extra":true}), + ), + ( + "missing", + "vllm-fixed-v1", + Value::Null, + json!({"kind":"vllm-enable-thinking-v1"}), + ), + ( + "type", + "vllm-fixed-v1", + Value::Null, + json!({"kind":"vllm-enable-thinking-v1","enabled":"false"}), + ), + ] { + let mut w = workload(1, 0, 1); + w["request"]["profile"] = json!(profile); + w["request"]["thinking"] = legacy; + w["request"]["thinking_control"] = control; + assert_eq!(run(&temp, &server, name, &w).status.code(), Some(1)); + assert!(!temp.path(name).exists()); + } + assert_eq!(server.count.load(Ordering::SeqCst), 0); +} + +#[test] +fn absent_new_control_preserves_legacy_workload_hashes_request_bytes_and_loading() { + use sha2::{Digest, Sha256}; + + let temp = Temp::new(); + let server = Server::new(normal); + // These serialized contracts predate thinking_control; receipt loading hashes + // the normalized workload and compares regenerated request strings exactly. + let normalized = r#"{"version":1,"name":"fixture-v1","request":{"profile":"vllm-fixed-v1","stream":true,"output":{"tokens":8,"mode":"cap"},"cache":"observe","temperature_milli":0,"top_p_milli":1000,"seed":42},"limits":{"total_ms":3000,"idle_ms":1000,"response_bytes":65536,"wave_buffer_bytes":33554432},"cases":[{"id":"one","messages":[{"role":"user","content":"Say cafe."}]}],"cells":[{"id":"cell","case":"one","concurrency":1,"warmup_trials":0,"trials":1}]}"#; + let request = r#"{"model":"fixture-model","messages":[{"role":"user","content":"Say cafe."}],"stream":true,"max_tokens":8,"temperature":0.0,"top_p":1.0,"seed":42,"stream_options":{"include_usage":true}}"#; + for (name, legacy) in [ + ("omitted", None), + ("null", Some(Value::Null)), + ("false", Some(json!(false))), + ("true", Some(json!(true))), + ] { + let mut w = workload(1, 0, 1); + w["request"]["profile"] = json!("vllm-fixed-v1"); + if let Some(legacy) = &legacy { + w["request"]["thinking"] = legacy.clone(); + } + for null_control in [false, true] { + if null_control { + w["request"]["thinking_control"] = Value::Null; + } + let name = format!("{name}-{null_control}"); + successful(&run(&temp, &server, &name, &w)); + let mut normalized = normalized.to_owned(); + let mut request = request.to_owned(); + if let Some(enabled) = legacy.as_ref().and_then(Value::as_bool) { + normalized = normalized.replace( + r#""seed":42"#, + &format!(r#""seed":42,"thinking":{enabled}"#), + ); + request = request.replace( + r#""seed":42"#, + &format!(r#""seed":42,"chat_template_kwargs":{{"thinking":{enabled}}}"#), + ); + } + let plan = value(temp.path(&name).join("plan.json")); + assert_eq!( + plan["workload_sha256"], + Sha256::digest(normalized.as_bytes()) + .iter() + .map(|b| format!("{b:02x}")) + .collect::() + ); + let reservation = value(temp.path(&name).join("wave-000000/reservation.json")); + assert_eq!(reservation["requests"][0], request); + successful( + &cli() + .arg("compare") + .arg(temp.path(&name)) + .arg(temp.path(&name)) + .arg("--json") + .output() + .unwrap(), + ); + } + } +} + #[test] fn fill_renders_distinct_lane_and_trial_salts_and_observe_plans_load() { let temp = Temp::new(); diff --git a/docs/performance/CONTRACT.md b/docs/performance/CONTRACT.md index 9432625..59c0f6e 100644 --- a/docs/performance/CONTRACT.md +++ b/docs/performance/CONTRACT.md @@ -14,8 +14,21 @@ Unknown workload fields and invalid controls are rejected before dispatch. nullable `temperature_milli`, `top_p_milli`, `seed`. Thousandths are encoded as decimal sampling values. Nullable `thinking` requires `vllm-fixed-v1` when declared and is sent as `chat_template_kwargs.thinking`; null leaves the provider default. -No other generation fields are sent or inferred. -Model-side thinking defaults are not overridden unless declared. +Alternatively, nullable `thinking_control: {"kind":"vllm-enable-thinking-v1", +"enabled":false}` declares `chat_template_kwargs.enable_thinking` under the same +explicit profile; `enabled` accepts either boolean. Unknown kinds and nested +fields are rejected. Both controls cannot be non-null, even when their booleans +agree. Neither key is inferred from a model name or sent alongside the other. +Absent or null controls are omitted from normalized workloads and leave provider +defaults unchanged; legacy `thinking` request bytes remain unchanged. +`portable-chat-v1` rejects either non-null control. No generic `extra_body` or +other generation fields are sent or inferred. + +These controls record requested behavior, not evidence that a template honored +it. Inspect reported reasoning tokens and observed generated/answer channels; +an answer-first event does not prove reasoning was absent elsewhere. A successful +response does not qualify a provider's template support. Thinking declarations +do not change eligibility or impose an answer-only requirement. Streaming requests explicitly request usage with `stream_options.include_usage`. Cases may declare `fill: {unit, repeat}`. The unit is nonempty and at most 64