From a4c71ea2c31a77dae422553581f5e9ecc93ec2ee Mon Sep 17 00:00:00 2001 From: Fini Date: Thu, 2 Jul 2026 21:21:33 +0800 Subject: [PATCH] fix(ai): disable thinking for reasoning models on design turns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reasoning models (glm-5.x / minimax) burn their whole token budget on hidden and emit an empty design when thinking is left on (glm-5.2: thinking≈30k, text=0). design_turn_thinking_mode forces ThinkingMode::Disabled for any model whose profile is thinking_disabled, applied across all design-capable paths — the design-agent loop, the builtin tool-executing chat loop, and sub-agent spawns. Claude (thinking productive) keeps the chat default. --- crates/op-host-desktop/src/chat_session.rs | 7 +- .../src/chat_session_launch.rs | 9 +- .../src/chat_session_launch_design.rs | 90 ++++++++++++++++++- .../op-host-desktop/src/sub_agent_session.rs | 9 +- 4 files changed, 106 insertions(+), 9 deletions(-) diff --git a/crates/op-host-desktop/src/chat_session.rs b/crates/op-host-desktop/src/chat_session.rs index 6172690ed..251701fe8 100644 --- a/crates/op-host-desktop/src/chat_session.rs +++ b/crates/op-host-desktop/src/chat_session.rs @@ -22,8 +22,11 @@ mod launch; pub(crate) use launch::builtin_provider_with_tools; pub use launch::{drain_new_chat_request, drain_stop_request, launch_if_pending}; pub(crate) use launch::{provider_for_selected_model, selected_cli_model_id}; -// Sub-agent launcher (Task 3.1) reuses the design-toolset provider builder. -pub(crate) use launch::launch_design::builtin_provider_with_design_tools; +// Sub-agent launcher (Task 3.1) reuses the design-toolset provider builder +// and the design-turn thinking policy. +pub(crate) use launch::launch_design::{ + builtin_provider_with_design_tools, design_turn_thinking_mode, +}; /// Pump the in-flight turn's deltas into the trailing assistant message, then /// execute any pending canvas tool calls against the live editor state. diff --git a/crates/op-host-desktop/src/chat_session_launch.rs b/crates/op-host-desktop/src/chat_session_launch.rs index 3e0cb51c8..9d1b47d26 100644 --- a/crates/op-host-desktop/src/chat_session_launch.rs +++ b/crates/op-host-desktop/src/chat_session_launch.rs @@ -147,8 +147,15 @@ pub fn launch_if_pending( // executes each call via the session's tool channel. if let Some((provider, tool_rx)) = builtin_provider_with_tools(host) { let system_prompt = build_agent_system_prompt(host.editor_state()); + // This builtin agent loop carries the full canvas toolset (`batch_design` + // included), so a design request runs *here* for an API-key model like + // glm-5.2 (experimental flag off → no design-agent loop, builtin → no CLI + // provider). Reasoning models burn their whole budget on hidden `` + // and draw nothing with thinking left on (glm-5.2 measured thinking≈30k / + // text=0 → empty Frame). Force it off for `thinking_disabled` models, same + // as the design-agent loop. Resolved before the `&mut` borrow below. + let thinking = launch_design::design_turn_thinking_mode(host); let chat = &mut host.editor_state_mut().chat; - let thinking = chat.thinking_mode; let effort = chat.effort_level; let attachments = std::mem::take(&mut chat.pending_attachments); let req = ChatRequest { diff --git a/crates/op-host-desktop/src/chat_session_launch_design.rs b/crates/op-host-desktop/src/chat_session_launch_design.rs index 19a7e6eeb..e0d0dbb2d 100644 --- a/crates/op-host-desktop/src/chat_session_launch_design.rs +++ b/crates/op-host-desktop/src/chat_session_launch_design.rs @@ -9,7 +9,7 @@ use std::sync::mpsc::Receiver; use std::sync::Arc; use op_ai::chat_history::{trim_chat_history, DEFAULT_MAX_CHARS, DEFAULT_MAX_MESSAGES}; -use op_ai::chat_provider::{ChatProvider, ChatRequest}; +use op_ai::chat_provider::{ChatProvider, ChatRequest, ThinkingMode}; use op_editor_host_core::chat::ChatSession; use op_editor_host_core::design::DesignSession; use op_host_native::WidgetHostNative; @@ -77,6 +77,50 @@ pub(crate) fn builtin_provider_with_design_tools( Some((Box::new(provider), tool_rx)) } +/// Effective thinking mode for a design turn (design-agent loop and +/// orchestrator sub-agent spawns alike). +/// +/// The design pipeline is structured generation, not free chat: reasoning +/// models whose profile marks `thinking_disabled` (glm-5.x / minimax / …) +/// burn their whole token budget on hidden `` and emit an *empty* +/// design when thinking is left on — glm-5.2 measured at thinking≈30k / +/// text=0 → nothing drawn. Force those to `Disabled`; the wire layer +/// (`chat_builtin_http`) then sends `thinking:{type:"disabled"}`. Claude and +/// other non-`thinking_disabled` models keep the chat's default — they use +/// thinking productively without starving content. +pub(crate) fn design_turn_thinking_mode(host: &WidgetHostNative) -> ThinkingMode { + let state = host.editor_state(); + let model = state + .chat + .selected_model_entry() + .and_then(|e| e.builtin_provider_id.as_deref()) + .and_then(|id| { + state + .editor_ui + .agent_settings + .builtin_agents + .iter() + .find(|a| a.id == id) + .map(|a| a.model.as_str()) + }); + resolve_design_thinking(model, state.chat.thinking_mode) +} + +/// Pure decision behind [`design_turn_thinking_mode`]: a model whose profile +/// is `thinking_disabled` is forced to `Disabled` for the design turn; +/// everything else (unknown model included → keep the user's choice) keeps the +/// chat default. Split out so the policy is unit-testable without a host. +fn resolve_design_thinking(model: Option<&str>, chat_default: ThinkingMode) -> ThinkingMode { + let thinking_disabled = model + .map(|m| op_orchestrator::resolve_model_profile(m).thinking_disabled) + .unwrap_or(false); + if thinking_disabled { + ThinkingMode::Disabled + } else { + chat_default + } +} + /// Launch the design-agent tool-loop turn when the flag is ON and a /// built-in design provider is available. Returns true when the turn was /// launched; false when the flag is OFF or no builtin is ready (caller @@ -106,8 +150,11 @@ pub(super) fn launch_design_loop_turn( DEFAULT_MAX_MESSAGES, DEFAULT_MAX_CHARS, ); + // Force thinking off for reasoning models that would otherwise emit an + // empty design (see `design_turn_thinking_mode`). Resolved before the + // `&mut` borrow below. + let thinking = design_turn_thinking_mode(host); let chat = &mut host.editor_state_mut().chat; - let thinking = chat.thinking_mode; let effort = chat.effort_level; let attachments = std::mem::take(&mut chat.pending_attachments); let req = ChatRequest { @@ -131,6 +178,45 @@ pub(super) fn launch_design_loop_turn( mod tests { use super::*; + // ── design-turn thinking policy ─────────────────────────────────────── + #[test] + fn reasoning_model_forces_thinking_off_for_design() { + // glm-5.2 is `thinking_disabled` in the profile: with thinking left on + // it burns its budget on `` and draws nothing. The design turn + // must override the chat default to `Disabled` regardless of choice. + assert_eq!( + resolve_design_thinking(Some("glm-5.2"), ThinkingMode::Adaptive), + ThinkingMode::Disabled + ); + assert_eq!( + resolve_design_thinking(Some("MiniMax-M3"), ThinkingMode::Enabled), + ThinkingMode::Disabled + ); + } + + #[test] + fn claude_keeps_chat_default_for_design() { + // Claude is NOT `thinking_disabled` — it uses thinking productively + // without starving content, so the design turn keeps the user's choice. + assert_eq!( + resolve_design_thinking(Some("claude-opus-4"), ThinkingMode::Adaptive), + ThinkingMode::Adaptive + ); + assert_eq!( + resolve_design_thinking(Some("claude-sonnet-4-6"), ThinkingMode::Enabled), + ThinkingMode::Enabled + ); + } + + #[test] + fn unknown_or_absent_model_keeps_chat_default() { + // No selected builtin → keep the user's choice (don't silently disable). + assert_eq!( + resolve_design_thinking(None, ThinkingMode::Enabled), + ThinkingMode::Enabled + ); + } + // ── loop_enabled OR-gate: 4 combinations ────────────────────────────── #[test] fn loop_enabled_both_off_is_false() { diff --git a/crates/op-host-desktop/src/sub_agent_session.rs b/crates/op-host-desktop/src/sub_agent_session.rs index e7a43d46b..13b0e3952 100644 --- a/crates/op-host-desktop/src/sub_agent_session.rs +++ b/crates/op-host-desktop/src/sub_agent_session.rs @@ -197,10 +197,11 @@ pub(crate) fn launch_sub_agents( }; let system_prompt = build_sub_agent_prompt(&spec); - let (thinking, effort) = { - let chat = &host.editor_state().chat; - (chat.thinking_mode, chat.effort_level) - }; + // Same design-turn thinking policy as the single design-agent loop: + // reasoning models that would burn their budget on `` and draw + // nothing are forced thinking-off (see `design_turn_thinking_mode`). + let thinking = chat_session::design_turn_thinking_mode(host); + let effort = host.editor_state().chat.effort_level; let req = ChatRequest { system_prompt, user_message: spec.prompt.clone(),