From 0b799025706177d628d35dc9cb6904b67a8e19c4 Mon Sep 17 00:00:00 2001 From: Fini Date: Fri, 7 Aug 2026 01:57:39 +0800 Subject: [PATCH] fix(agent): preserve Kimi K3 design script output --- .../op-host-services/src/chat_agent_loop.rs | 4 + .../src/chat_agent_loop/openai.rs | 17 ++-- .../src/chat_agent_loop_wire_tests.rs | 80 ++++++++++++++++++ .../op-host-services/src/chat_builtin_http.rs | 20 ++--- .../src/chat_builtin_http_tests.rs | 81 ++++++++++++++++++ .../src/chat_builtin_http_wire.rs | 42 ++++++++-- crates/op-orchestrator/src/lib.rs | 4 +- crates/op-orchestrator/src/model_profile.rs | 82 ++++++++++++++----- 8 files changed, 281 insertions(+), 49 deletions(-) create mode 100644 crates/op-host-services/src/chat_agent_loop_wire_tests.rs diff --git a/crates/op-host-services/src/chat_agent_loop.rs b/crates/op-host-services/src/chat_agent_loop.rs index 75b7dcaac..bd3876ff5 100644 --- a/crates/op-host-services/src/chat_agent_loop.rs +++ b/crates/op-host-services/src/chat_agent_loop.rs @@ -451,3 +451,7 @@ mod quality_tests; #[cfg(test)] #[path = "chat_agent_loop_retry_tests.rs"] mod retry_tests; + +#[cfg(test)] +#[path = "chat_agent_loop_wire_tests.rs"] +mod wire_tests; diff --git a/crates/op-host-services/src/chat_agent_loop/openai.rs b/crates/op-host-services/src/chat_agent_loop/openai.rs index 18901c996..b83ff7140 100644 --- a/crates/op-host-services/src/chat_agent_loop/openai.rs +++ b/crates/op-host-services/src/chat_agent_loop/openai.rs @@ -228,14 +228,15 @@ pub(super) async fn run_openai_agent_loop_inner( // tool calls and then simply stops — measured with deepseek-v4-pro, // whose thinking defaults to `effort=high` (2026-07-31). // - // Shares `accepts_thinking_body_field` with the single-shot body in - // `chat_builtin_http`; this loop previously carried its own copy of - // the family list and that is exactly how DeepSeek fell through. - if cfg.disable_thinking && op_orchestrator::accepts_thinking_body_field(&cfg.model) { - if let Some(obj) = body.as_object_mut() { - obj.insert("thinking".into(), json!({ "type": "disabled" })); - } - } + // The same helper builds the single-shot body. It maps Kimi K3 to + // top-level `reasoning_effort:"low"`, while K2.5/K2.6, GLM, + // DeepSeek, and MiniMax keep `thinking:{type:"disabled"}`. The two + // mutually-exclusive fields can therefore never drift by path. + crate::chat_builtin_http::apply_reasoning_wire_control( + &mut body, + &cfg.model, + cfg.disable_thinking, + ); // Through the shared throttle/backoff: this tool-loop path used // to post raw, so a provider rate limit killed the design run with // no retries and a raw JSON error (measured: glm-5.2, 429 diff --git a/crates/op-host-services/src/chat_agent_loop_wire_tests.rs b/crates/op-host-services/src/chat_agent_loop_wire_tests.rs new file mode 100644 index 000000000..20421fca3 --- /dev/null +++ b/crates/op-host-services/src/chat_agent_loop_wire_tests.rs @@ -0,0 +1,80 @@ +//! OpenAI-compatible reasoning-control and mixed-delta wire regressions. + +use super::tests::{run_loop_collect, serve_sse_script, update_node_tool_def, ScriptedExecutor}; +use super::*; +use serde_json::Value; + +fn text_turn() -> String { + [ + r#"data: {"choices":[{"delta":{"content":"done"}}]}"#, + "", + r#"data: {"choices":[{"delta":{},"finish_reason":"stop"}]}"#, + "", + "data: [DONE]", + "", + "", + ] + .join("\n") +} + +fn request_body(request: &str) -> Value { + let body_start = request + .find("\r\n\r\n") + .map(|index| index + 4) + .expect("request body separator"); + serde_json::from_str(&request[body_start..]).expect("request body JSON") +} + +fn capture_agent_body(model: &str) -> Value { + let (base, requests) = serve_sse_script(vec![text_turn()]); + let executor = ScriptedExecutor::ok(r#"{"success":true,"data":{}}"#); + let cfg = AgentLoopConfig { + url: format!("{base}/chat/completions"), + api_key: "sk-test".into(), + model: model.into(), + system_prompt: "You are a design editor.".into(), + history: Vec::new(), + user_prompt: "continue the design".into(), + max_output_tokens: 512, + tools: vec![update_node_tool_def()], + executor, + max_turns: 2, + finalize_on_exit: false, + disable_thinking: true, + dial_policy: crate::provider_dial::EndpointDialPolicy::Trusted, + }; + let (outcome, deltas) = run_loop_collect(cfg, false); + assert_eq!(outcome, Ok(true), "model={model}, deltas={deltas:?}"); + request_body(&requests.recv().expect("captured agent-loop request")) +} + +#[test] +fn agent_loop_uses_the_same_mutually_exclusive_reasoning_controls_as_classic() { + let k3 = capture_agent_body("kimi-k3"); + assert_eq!(k3["reasoning_effort"], "low"); + assert!(k3.get("thinking").is_none(), "K3 request: {k3}"); + + for model in ["kimi-k2.5", "kimi-k2.6", "glm-5.2", "deepseek-v4-pro"] { + let body = capture_agent_body(model); + assert_eq!(body["thinking"]["type"], "disabled", "model={model}"); + assert!( + body.get("reasoning_effort").is_none(), + "model={model}: {body}" + ); + } +} + +#[test] +fn agent_loop_mixed_reasoning_and_content_delta_emits_both_in_order() { + let mut collector = OpenAiCollector::default(); + let deltas = collector.handle( + r#"{"choices":[{"delta":{"reasoning_content":"plan","content":"batch_design(...)"}}]}"#, + ); + assert_eq!( + deltas, + vec![ + ChatDelta::Thinking("plan".into()), + ChatDelta::TextDelta("batch_design(...)".into()), + ] + ); +} diff --git a/crates/op-host-services/src/chat_builtin_http.rs b/crates/op-host-services/src/chat_builtin_http.rs index 21cdb15fc..16f748c74 100644 --- a/crates/op-host-services/src/chat_builtin_http.rs +++ b/crates/op-host-services/src/chat_builtin_http.rs @@ -25,11 +25,11 @@ use crate::chat_runtime::{resolved_skill_preamble, shared_runtime, BlockingRecvI mod error; pub use error::BuiltinHttpError; -pub use crate::chat_builtin_http_wire::{map_anthropic_stop_reason, map_openai_stop_reason}; pub(crate) use crate::chat_builtin_http_wire::{ - normalize_provider_base_url, parse_anthropic_sse_data, parse_openai_sse_data, - provider_endpoint, pump_sse_response, + apply_reasoning_wire_control, normalize_provider_base_url, parse_anthropic_sse_data, + parse_openai_sse_data, provider_endpoint, pump_sse_response, }; +pub use crate::chat_builtin_http_wire::{map_anthropic_stop_reason, map_openai_stop_reason}; /// Design turns build one <=25-op section batch per model turn, plus repair /// turns after layoutIssues feedback. 28 sits in the requested 24-32 window: @@ -540,15 +540,13 @@ async fn run_openai_chat( // 留空(glm-5.2 实测一个设计子任务 thinking≈3 万字符、content 0,整段 parse // 失败、重试也撞同一堵墙)。当调用方明确要求关思考(`disable_thinking`,如编排器 // 的设计子任务),且该模型家族能在线级表达这条意图时,下发 - // `thinking:{type:"disabled"}`。普通对话走 `Adaptive` 不进这里,保留推理。 + // provider-specific wire control. K2.5/K2.6, GLM, DeepSeek, and MiniMax + // use `thinking:{type:"disabled"}`; Kimi K3 rejects that field and uses + // top-level `reasoning_effort:"low"`. Ordinary chat stays Adaptive. // - // 名单是 `op_orchestrator::accepts_thinking_body_field` 的单一来源 —— 曾经 - // 散在三处、drift 过、也漏过 DeepSeek,详见那里的注释。 - if disable_thinking && op_orchestrator::accepts_thinking_body_field(&provider.model) { - if let Some(obj) = body.as_object_mut() { - obj.insert("thinking".into(), json!({ "type": "disabled" })); - } - } + // The policy lives in `op_orchestrator::reasoning_wire_control`; the JSON + // mutation is shared with the agent loop so the two paths stay identical. + apply_reasoning_wire_control(&mut body, &provider.model, disable_thinking); let client = provider.dial_client(&url).await?; let resp = send_with_backoff( "openai-compatible", diff --git a/crates/op-host-services/src/chat_builtin_http_tests.rs b/crates/op-host-services/src/chat_builtin_http_tests.rs index d7be125cf..c5a040548 100644 --- a/crates/op-host-services/src/chat_builtin_http_tests.rs +++ b/crates/op-host-services/src/chat_builtin_http_tests.rs @@ -34,6 +34,58 @@ fn read_http_request(stream: &mut TcpStream) -> String { String::from_utf8_lossy(&buf).to_string() } +fn http_request_body(request: &str) -> Value { + let body_start = request + .find("\r\n\r\n") + .map(|index| index + 4) + .expect("request body separator"); + serde_json::from_str(&request[body_start..]).expect("request body JSON") +} + +fn capture_classic_openai_body(model: &str, thinking: ThinkingMode) -> Value { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind request capture server"); + let addr = listener.local_addr().expect("request capture address"); + let (request_tx, request_rx) = std_mpsc::channel(); + let server = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept request"); + let request = read_http_request(&mut stream); + request_tx.send(request).expect("capture request"); + let body = concat!( + "data: {\"choices\":[{\"delta\":{\"content\":\"ok\"}}]}\n\n", + "data: [DONE]\n\n" + ); + let response = format!( + "HTTP/1.1 200 OK\r\ncontent-type: text/event-stream\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{body}", + body.len() + ); + stream + .write_all(response.as_bytes()) + .expect("write SSE response"); + }); + + let mut config = builtin_config(BuiltinAgentKind::OpenAiCompat, format!("http://{addr}/v1")); + config.model = model.to_string(); + let mut provider = + ConfiguredBuiltinProvider::from_builtin_agent(&config).expect("ready capture provider"); + provider.max_retries = 0; + provider.min_gap = Duration::ZERO; + let deltas: Vec<_> = provider + .send(ChatRequest { + user_message: "continue the design".into(), + thinking, + ..Default::default() + }) + .collect(); + assert!( + deltas + .iter() + .any(|delta| matches!(delta, ChatDelta::TextDelta(text) if text == "ok")), + "capture response should complete: {deltas:?}" + ); + server.join().expect("request capture server exits"); + http_request_body(&request_rx.recv().expect("captured request")) +} + fn builtin_config(kind: BuiltinAgentKind, base_url: impl Into) -> BuiltinAgentConfig { BuiltinAgentConfig { id: "builtin-test".into(), @@ -453,6 +505,35 @@ fn parse_openai_sse_data_extracts_text_delta() { ); } +#[test] +fn classic_openai_mixed_reasoning_and_content_delta_preserves_content() { + let data = + r#"{"choices":[{"delta":{"reasoning_content":"plan","content":"batch_design(...)"}}]}"#; + assert_eq!( + parse_openai_sse_data(data), + Some(ChatDelta::TextDelta("batch_design(...)".into())) + ); +} + +#[test] +fn classic_openai_reasoning_only_delta_stays_visible_as_thinking() { + let data = r#"{"choices":[{"delta":{"reasoning_content":"plan"}}]}"#; + assert_eq!( + parse_openai_sse_data(data), + Some(ChatDelta::Thinking("plan".into())) + ); +} + +#[test] +fn classic_kimi_k3_request_uses_only_low_reasoning_effort() { + let body = capture_classic_openai_body("kimi-k3", ThinkingMode::Disabled); + assert_eq!(body["reasoning_effort"], "low"); + assert!( + body.get("thinking").is_none(), + "K3 rejects `thinking` and cannot receive both controls: {body}" + ); +} + #[test] fn parse_anthropic_sse_data_extracts_text_delta() { let data = r#"{"type":"content_block_delta","delta":{"type":"text_delta","text":"hello"}}"#; diff --git a/crates/op-host-services/src/chat_builtin_http_wire.rs b/crates/op-host-services/src/chat_builtin_http_wire.rs index 65f9740d2..47f12108f 100644 --- a/crates/op-host-services/src/chat_builtin_http_wire.rs +++ b/crates/op-host-services/src/chat_builtin_http_wire.rs @@ -13,6 +13,30 @@ use tokio::sync::mpsc; use crate::chat_builtin_http::BuiltinHttpError; +/// Apply the provider-specific low-reasoning control for structured design +/// turns. The two supported wire shapes are deliberately centralized here so +/// classic streaming and the tool-executing agent loop cannot drift or send +/// mutually-exclusive fields together. +pub(crate) fn apply_reasoning_wire_control(body: &mut Value, model: &str, reduce_reasoning: bool) { + if !reduce_reasoning { + return; + } + let Some(obj) = body.as_object_mut() else { + return; + }; + match op_orchestrator::reasoning_wire_control(model) { + Some(op_orchestrator::ReasoningWireControl::ThinkingDisabled) => { + obj.remove("reasoning_effort"); + obj.insert("thinking".into(), serde_json::json!({ "type": "disabled" })); + } + Some(op_orchestrator::ReasoningWireControl::ReasoningEffortLow) => { + obj.remove("thinking"); + obj.insert("reasoning_effort".into(), Value::String("low".into())); + } + None => {} + } +} + pub(crate) async fn pump_sse_response( resp: reqwest::Response, tx: &mpsc::Sender, @@ -151,6 +175,17 @@ pub(crate) fn parse_openai_sse_data(data: &str) -> Option { } let choice = value.get("choices")?.as_array()?.first()?; if let Some(delta) = choice.get("delta") { + // Some OpenAI-compatible providers put reasoning and the first + // content token in the SAME delta. This classic parser returns one + // event, so content must win; preferring reasoning here used to drop + // the only script-bearing token and could leave orchestration empty. + if let Some(content) = delta + .get("content") + .and_then(Value::as_str) + .filter(|s| !s.is_empty()) + { + return Some(ChatDelta::TextDelta(content.to_string())); + } if let Some(reasoning) = delta .get("reasoning_content") .or_else(|| delta.get("reasoning")) @@ -159,13 +194,6 @@ pub(crate) fn parse_openai_sse_data(data: &str) -> Option { { return Some(ChatDelta::Thinking(reasoning.to_string())); } - if let Some(content) = delta - .get("content") - .and_then(Value::as_str) - .filter(|s| !s.is_empty()) - { - return Some(ChatDelta::TextDelta(content.to_string())); - } } choice .get("finish_reason") diff --git a/crates/op-orchestrator/src/lib.rs b/crates/op-orchestrator/src/lib.rs index 2284bc5ad..a843de54c 100644 --- a/crates/op-orchestrator/src/lib.rs +++ b/crates/op-orchestrator/src/lib.rs @@ -131,8 +131,8 @@ pub use intent::classify_intent; pub use loop_finalize::{apply_loop_finalize, apply_loop_finalize_counted}; pub use mobile_reflow::repair_mobile_trailing_nav_reflow; pub use model_profile::{ - accepts_thinking_body_field, is_acp_capability_marker, resolve_model_profile, ModelProfile, - ModelTier, + accepts_thinking_body_field, is_acp_capability_marker, reasoning_wire_control, + resolve_model_profile, ModelProfile, ModelTier, ReasoningWireControl, }; pub use prompt::build_orchestrator_prompt; pub use repair_summary::{CheckCategory, RepairSummary}; diff --git a/crates/op-orchestrator/src/model_profile.rs b/crates/op-orchestrator/src/model_profile.rs index 55ee35bd4..87bb12f39 100644 --- a/crates/op-orchestrator/src/model_profile.rs +++ b/crates/op-orchestrator/src/model_profile.rs @@ -11,6 +11,17 @@ pub enum ModelTier { Basic, } +/// Provider-specific control used to minimize hidden reasoning on structured +/// design turns. The transport layer maps this semantic policy to exactly one +/// OpenAI-compatible request field. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ReasoningWireControl { + /// `thinking: { "type": "disabled" }`. + ThinkingDisabled, + /// `reasoning_effort: "low"` (Kimi K3; `thinking` is unsupported). + ReasoningEffortLow, +} + /// 一个模型的能力画像。 #[derive(Debug, Clone)] pub struct ModelProfile { @@ -276,11 +287,12 @@ pub fn resolve_model_profile(model_id: &str) -> ModelProfile { DEFAULT_PROFILE } -/// 模型家族是否接受 `thinking:{"type":"disabled"}` 这个 body 字段。 +/// Resolve the provider-specific wire control for reducing hidden reasoning. /// /// 这是**协议能力**,不是 [`ModelProfile::thinking_disabled`](ModelProfile) -/// 那条"该不该关思考"的策略:后者说意图,前者说这条意图能否在线级表达出来。 -/// 两者都为真时,传输层才真正下发该字段。 +/// 那条"该不该关思考"的策略:后者说意图,这里说这条意图应如何在线级表达。 +/// Kimi K3 使用 `reasoning_effort:"low"`;其余已验证家族使用 +/// `thinking:{"type":"disabled"}`。调用方明确要求降低推理时才真正下发。 /// /// 单一来源。此前这份知识以 `is_minimax_model` / `is_glm_model` 两个谓词的形式 /// 散在传输层(`chat_builtin_http`)、agent tool-loop(`chat_agent_loop::openai`) @@ -305,13 +317,16 @@ pub fn resolve_model_profile(model_id: &str) -> ModelProfile { /// 不做无条件下发:内置 provider 允许用户把 base_url 指向任意 openai-compat 端点 /// (含 OpenAI 官方),那里的未知 body 字段会 400。名单是这条风险的边界,新增一家 /// 只改这里一处。 -pub fn accepts_thinking_body_field(model_id: &str) -> bool { +pub fn reasoning_wire_control(model_id: &str) -> Option { let normalized = match model_id.find('/') { Some(i) => &model_id[i + 1..], None => model_id, }; let lower = normalized.to_ascii_lowercase(); - lower.starts_with("minimax") + if lower.contains("kimi-k3") { + return Some(ReasoningWireControl::ReasoningEffortLow); + } + (lower.starts_with("minimax") || lower.starts_with("abab") || lower.contains("glm") || lower.starts_with("deepseek") @@ -332,7 +347,22 @@ pub fn accepts_thinking_body_field(model_id: &str) -> bool { // 出处:platform.kimi.ai/docs/api/chat 的逐模型 thinking / // reasoning_effort 对照表 + docs/models 的在售模型列表。 || lower.contains("kimi-k2.5") - || lower.contains("kimi-k2.6") + || lower.contains("kimi-k2.6")) + .then_some(ReasoningWireControl::ThinkingDisabled) +} + +/// Whether a model accepts `thinking: { "type": "disabled" }`. +/// +/// Kept as the narrow compatibility predicate for callers that specifically +/// need that field. New request builders should match on +/// [`reasoning_wire_control`] so Kimi K3 receives its mutually-exclusive +/// `reasoning_effort` control instead of being mistaken for an unsupported +/// model. +pub fn accepts_thinking_body_field(model_id: &str) -> bool { + matches!( + reasoning_wire_control(model_id), + Some(ReasoningWireControl::ThinkingDisabled) + ) } #[cfg(test)] @@ -358,19 +388,34 @@ mod tests { assert!(!accepts_thinking_body_field("")); } + #[test] + fn kimi_k3_uses_low_reasoning_effort_instead_of_thinking() { + for model in ["kimi-k3", "moonshot/kimi-k3", "kimi-k3.1-preview"] { + let profile = resolve_model_profile(model); + assert_eq!(profile.tier, ModelTier::Full, "model={model}"); + assert!(profile.thinking_disabled, "model={model}"); + assert_eq!( + reasoning_wire_control(model), + Some(ReasoningWireControl::ReasoningEffortLow), + "model={model}" + ); + assert!(!accepts_thinking_body_field(model), "model={model}"); + } + } + /// The capability table must cover every model whose profile asks for /// thinking off — otherwise the profile's intent is silently dropped at /// the wire, which is exactly how deepseek-v4-pro regressed. #[test] - fn every_reasoning_model_we_disable_thinking_for_can_express_it() { - for model in ["deepseek-v4-pro", "deepseek-v4-flash", "glm-5.2"] { + fn every_reasoning_model_we_reduce_can_express_it() { + for model in ["deepseek-v4-pro", "deepseek-v4-flash", "glm-5.2", "kimi-k3"] { assert!( resolve_model_profile(model).thinking_disabled, "{model} profile should ask for thinking off" ); assert!( - accepts_thinking_body_field(model), - "{model} asks for thinking off but cannot express it on the wire" + reasoning_wire_control(model).is_some(), + "{model} asks for reduced reasoning but cannot express it on the wire" ); } } @@ -379,7 +424,7 @@ mod tests { /// off but which deliberately do NOT get the body field, each with the /// reason. Being on this list is a decision, not an oversight — that /// distinction is the whole point of the sweep below. - const THINKING_FIELD_WITHHELD: &[(&str, &str)] = &[ + const REASONING_CONTROL_WITHHELD: &[(&str, &str)] = &[ ( "gpt-5.4", "OpenAI's official endpoint rejects unknown body fields", @@ -388,11 +433,6 @@ mod tests { "gemini-3-flash-preview", "Google's OpenAI-compat shim is not documented to accept it", ), - ( - "kimi-k3", - "K3 always reasons; control is top-level reasoning_effort, and \ - sending both fields is a documented 400", - ), ("qwen-plus", "no verified thinking-disable field"), ("qwen3-coder-plus", "no verified thinking-disable field"), ("doubao-seed-2.0-pro", "no verified thinking-disable field"), @@ -421,7 +461,7 @@ mod tests { /// listed above with a reason. Adding a preset with a new default model /// fails here until someone decides which. #[test] - fn every_shipped_preset_model_is_classified_for_the_thinking_field() { + fn every_shipped_preset_model_is_classified_for_reasoning_control() { for preset in op_editor_core::BUILTIN_AGENT_PRESETS { // The Custom preset's `model` is a form placeholder, not an id. if preset.key == op_editor_core::BuiltinAgentPresetKey::Custom { @@ -431,16 +471,16 @@ mod tests { if !resolve_model_profile(model).thinking_disabled { continue; } - let on_the_wire = accepts_thinking_body_field(model); - let withheld = THINKING_FIELD_WITHHELD + let on_the_wire = reasoning_wire_control(model).is_some(); + let withheld = REASONING_CONTROL_WITHHELD .iter() .any(|(listed, _)| *listed == model); assert!( on_the_wire != withheld, "preset `{}` ships model `{model}`, whose profile asks for \ thinking off, but it is {} — add it to \ - `accepts_thinking_body_field` (with a source) or to \ - THINKING_FIELD_WITHHELD (with a reason)", + `reasoning_wire_control` (with a source) or to \ + REASONING_CONTROL_WITHHELD (with a reason)", preset.display_name, if on_the_wire { "both sent on the wire AND listed as withheld"