From 4b47e8a1034ad3e72ba6739cbfd4d4cac2bb7b46 Mon Sep 17 00:00:00 2001 From: Fini Date: Sun, 9 Aug 2026 01:18:01 +0800 Subject: [PATCH] fix(agent): gate root-seed inheritance on real continuation intent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preserving original-case screen names broke the lowercase existing- screen guard, quantifier stripping killed whole target lists over one short latin name, and root-seed inheritance fired on any canvas that happened to hold a same-class screen — silently overwriting authored widths and fills. Continuation is now an explicit signal derived from the same promised-screen parser the orchestrator uses, inheritance never overrides an authored fill, and the guards are case-robust. Claude-Session: https://claude.ai/code/session_01FqKQqNj8exYwopGDpYUU7x --- crates/op-editor-core/src/agent_indicators.rs | 30 +++-- .../src/chat_session_launch_design.rs | 78 ++++++++++++- .../src/keyboard_input_tests.rs | 2 + .../op-host-desktop/src/sub_agent_session.rs | 13 ++- .../src/sub_agent_session_tests.rs | 5 + .../src/chat_intent_screen_sets.rs | 75 ++++++++++++- .../src/design_agent_tools/root_seed.rs | 90 ++++++++++----- .../design_agent_tools_continuation_tests.rs | 105 +++++++++++++++++- .../src/design_agent_tools_tests.rs | 17 ++- 9 files changed, 365 insertions(+), 50 deletions(-) diff --git a/crates/op-editor-core/src/agent_indicators.rs b/crates/op-editor-core/src/agent_indicators.rs index ed6c7fc8c..53cdc5f4f 100644 --- a/crates/op-editor-core/src/agent_indicators.rs +++ b/crates/op-editor-core/src/agent_indicators.rs @@ -103,8 +103,7 @@ pub struct AgentIndicators { needs_final_frame: bool, last_reveal_snapshot_ms: Option, /// Optional per-turn root seed profile for the design-agent loop. - /// `Some(true)` means mobile artboard, `Some(false)` means desktop. - root_seed_mobile: Option, + root_seed: Option, /// True after the first successful `batch_design` of this epoch has /// consumed the root seed guard. root_seed_consumed: bool, @@ -123,7 +122,7 @@ impl AgentIndicators { self.finishing = false; self.needs_final_frame = false; self.last_reveal_snapshot_ms = None; - self.root_seed_mobile = None; + self.root_seed = None; self.root_seed_consumed = false; } } @@ -282,16 +281,31 @@ pub fn begin() -> u64 { r.epoch } +/// The per-turn root seed profile a design run attaches to its epoch. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct RootSeedHint { + /// Mobile artboard when true, desktop when false. + pub mobile: bool, + /// True only when the REQUEST asked to continue an existing screen set — + /// the one case in which a new root may inherit a live screen's artboard. + pub continuation: bool, +} + /// Begin a new run with a design-loop root seed profile attached. /// /// The profile is consumed by the first successful `batch_design` executor -/// call for this epoch. Non-design runs should keep using [`begin`]. -pub fn begin_with_root_seed_hint(mobile: bool) -> u64 { +/// call for this epoch — except on a continuation turn, which keeps it +/// pending so later sibling screens inherit the same contract. Non-design +/// runs should keep using [`begin`]. +pub fn begin_with_root_seed_hint(mobile: bool, continuation: bool) -> u64 { let mut r = REGISTRY.lock().unwrap(); r.epoch += 1; r.clear_maps(); r.run_active = true; - r.root_seed_mobile = Some(mobile); + r.root_seed = Some(RootSeedHint { + mobile, + continuation, + }); r.epoch } @@ -309,10 +323,10 @@ pub fn is_frame_generating(frame_id: &str) -> bool { /// Return the root seed profile for `epoch` if its first batch has not /// consumed the guard yet. -pub fn root_seed_hint_if_pending(epoch: u64) -> Option { +pub fn root_seed_hint_if_pending(epoch: u64) -> Option { let r = REGISTRY.lock().unwrap(); if r.epoch == epoch && r.run_active && !r.root_seed_consumed { - r.root_seed_mobile + r.root_seed } else { None } 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 91475e882..be358c12e 100644 --- a/crates/op-host-desktop/src/chat_session_launch_design.rs +++ b/crates/op-host-desktop/src/chat_session_launch_design.rs @@ -18,7 +18,9 @@ use op_host_services::chat_builtin_http::{ }; use op_host_services::chat_canvas_tools::{chat_tool_channel, ChatToolRequest}; use op_host_services::chat_system_prompt::chat_history_from_transcript; -use op_host_services::design_agent_tools::{design_tool_defs, root_seed_prompt_is_mobile}; +use op_host_services::design_agent_tools::{ + design_tool_defs, root_seed_prompt_is_continuation, root_seed_prompt_is_mobile, +}; use op_editor_core::scene_template_catalog::TemplateScene; @@ -299,6 +301,9 @@ pub(super) fn launch_design_loop_turn( // `&mut` borrow below. let thinking = design_turn_thinking_mode(host); let root_seed_mobile = root_seed_mobile_for_turn(host.editor_state(), &user_text); + // Only a request that promises sibling screens may make later roots + // inherit the live screen's artboard; see `root_seed_prompt_is_continuation`. + let root_seed_continuation = root_seed_prompt_is_continuation(&user_text); let agent_team_size = host.editor_state().chat.agent_team_size; let chat = &mut host.editor_state_mut().chat; let effort = chat.effort_level; @@ -340,7 +345,10 @@ pub(super) fn launch_design_loop_turn( }; // The host starts the indicator epoch before the worker can apply its first // design batch; the indicator pump adopts this epoch for badges/teardown. - op_editor_core::agent_indicators::begin_with_root_seed_hint(root_seed_mobile); + op_editor_core::agent_indicators::begin_with_root_seed_hint( + root_seed_mobile, + root_seed_continuation, + ); *current_chat = Some(ChatSession::start_with_tools(provider, req, Some(tool_rx)).into_design_loop()); // Signal one agent running so the chat header shows "1/1 designing…" @@ -383,6 +391,72 @@ mod tests { ); } + #[test] + fn clicking_the_footer_toggle_changes_what_a_design_turn_asks_for() { + // The whole chain in one test, because every link of it was present + // and the feature still did not exist: the chip that set + // `thinking_mode` was painted by nothing, so no click could reach the + // policy below. Black-box on purpose — the test finds the control the + // way a user does (a press somewhere in the panel), not by importing + // its rect. + use op_editor_core::EditorState; + use op_editor_ui::widgets::chat_click_flow::apply_chat_hit; + use op_editor_ui::widgets::{AIChatHit, AIChatPlaceholder, AI_CHAT_HEIGHT, AI_CHAT_WIDTH}; + use op_editor_ui::{Point2D, Rect}; + + let mut state = EditorState::new(); + let rect = Rect::xywh(0.0, 0.0, AI_CHAT_WIDTH, AI_CHAT_HEIGHT); + let toggle_point = { + let panel = AIChatPlaceholder::from_editor(&state); + let input = panel.input_rect(rect); + let mut found = None; + let mut y = input.origin.y; + while y < input.origin.y + input.size.y && found.is_none() { + let mut x = input.origin.x; + while x < input.origin.x + input.size.x { + let point = Point2D::new(x, y); + if panel.hit_test(rect, point) == Some(AIChatHit::CycleThinking) { + found = Some(point); + break; + } + x += 1.0; + } + y += 1.0; + } + found.expect("the chat footer must expose a thinking-mode control") + }; + + // A model the profile does not force off follows the user's choice. + const FREE: Option<&str> = Some("claude-opus-4"); + assert_eq!( + resolve_design_thinking(FREE, state.chat.thinking_mode), + ThinkingMode::Adaptive + ); + for want in [ + ThinkingMode::Disabled, + ThinkingMode::Enabled, + ThinkingMode::Adaptive, + ] { + let hit = AIChatPlaceholder::from_editor(&state) + .hit_test(rect, toggle_point) + .expect("the toggle stays where it was found"); + apply_chat_hit(&mut state, hit, 0); + assert_eq!(state.chat.thinking_mode, want); + assert_eq!( + resolve_design_thinking(FREE, state.chat.thinking_mode), + want, + "a design turn must ask for what the footer now shows" + ); + // …and a `thinking_disabled` model stays forced off through every + // one of those states. That override is deliberate, not a bug the + // toggle is expected to defeat. + assert_eq!( + resolve_design_thinking(Some("glm-5.2"), state.chat.thinking_mode), + ThinkingMode::Disabled + ); + } + } + #[test] fn unknown_or_absent_model_keeps_chat_default() { // No selected builtin → keep the user's choice (don't silently disable). diff --git a/crates/op-host-desktop/src/keyboard_input_tests.rs b/crates/op-host-desktop/src/keyboard_input_tests.rs index a39f72a5d..80a3cdabd 100644 --- a/crates/op-host-desktop/src/keyboard_input_tests.rs +++ b/crates/op-host-desktop/src/keyboard_input_tests.rs @@ -393,6 +393,7 @@ fn stop_chat_aborts_sub_agents_and_clears_the_running_counter() { }, indicator: None, root_seed_mobile: true, + root_seed_continuation: false, }); app.active_sub_agent = 0; app.host.editor_state_mut().chat.agents_running = (1, 1); @@ -438,6 +439,7 @@ fn new_chat_aborts_the_old_running_tab_without_dirtying_the_fresh_tab() { }, indicator: None, root_seed_mobile: true, + root_seed_continuation: false, }); app.active_sub_agent = 0; app.chat_running_tab = Some(0); diff --git a/crates/op-host-desktop/src/sub_agent_session.rs b/crates/op-host-desktop/src/sub_agent_session.rs index 0a7e33c6d..3cdec5731 100644 --- a/crates/op-host-desktop/src/sub_agent_session.rs +++ b/crates/op-host-desktop/src/sub_agent_session.rs @@ -44,7 +44,9 @@ use op_mcp::spawn_agents_tool::SpawnSpec; use op_orchestrator::agent_identity::assign_agent_identities_seeded; use op_editor_core::{agent_indicators, ChatMessage, PenNodeExt}; -use op_host_services::design_agent_tools::root_seed_prompt_is_mobile; +use op_host_services::design_agent_tools::{ + root_seed_prompt_is_continuation, root_seed_prompt_is_mobile, +}; use crate::chat_session::{self, builtin_provider_with_design_tools, ChatSession}; use crate::design_loop_indicator::{ @@ -76,6 +78,9 @@ pub(crate) struct SubAgentSession { pub indicator: Option, /// Precomputed from the sub prompt and attached when the lazy epoch begins. pub root_seed_mobile: bool, + /// Whether this sub's own prompt asks to continue an existing screen set — + /// the only case in which its roots may inherit a live screen's artboard. + pub root_seed_continuation: bool, } // --------------------------------------------------------------------------- @@ -243,6 +248,7 @@ pub(crate) fn launch_sub_agents( identity, indicator: None, root_seed_mobile: existing_mobile_screen || root_seed_prompt_is_mobile(&spec.prompt), + root_seed_continuation: root_seed_prompt_is_continuation(&spec.prompt), }); } @@ -326,7 +332,10 @@ pub(crate) fn pump_sub_agents( // The initial-frame snapshot is taken NOW (after prior subs // finished), so only THIS sub's new frames get badged. if subs[*active].indicator.is_none() { - let epoch = agent_indicators::begin_with_root_seed_hint(subs[*active].root_seed_mobile); + let epoch = agent_indicators::begin_with_root_seed_hint( + subs[*active].root_seed_mobile, + subs[*active].root_seed_continuation, + ); let initial_frame_ids = collect_top_level_frame_ids(host.editor_state()); let identity = subs[*active].identity.clone(); agent_indicators::confirm_cursor_agent(epoch, &identity.color, &identity.name); diff --git a/crates/op-host-desktop/src/sub_agent_session_tests.rs b/crates/op-host-desktop/src/sub_agent_session_tests.rs index 4c95f4745..83fab2a6e 100644 --- a/crates/op-host-desktop/src/sub_agent_session_tests.rs +++ b/crates/op-host-desktop/src/sub_agent_session_tests.rs @@ -397,6 +397,7 @@ fn pump_with_finished_subs_decrements_then_clears_agents_running() { }, indicator: None, root_seed_mobile: false, + root_seed_continuation: false, }, SubAgentSession { session: None, @@ -406,6 +407,7 @@ fn pump_with_finished_subs_decrements_then_clears_agents_running() { }, indicator: None, root_seed_mobile: false, + root_seed_continuation: false, }, ]; let mut active = 0usize; @@ -462,6 +464,7 @@ fn pumped_sub_agent_message_carries_its_identity() { }, indicator: None, root_seed_mobile: false, + root_seed_continuation: false, }]; let mut active = 0; @@ -515,6 +518,7 @@ fn sequential_sub_agents_keep_distinct_identity_messages() { }, indicator: None, root_seed_mobile: false, + root_seed_continuation: false, }, SubAgentSession { session: Some(session("second")), @@ -524,6 +528,7 @@ fn sequential_sub_agents_keep_distinct_identity_messages() { }, indicator: None, root_seed_mobile: false, + root_seed_continuation: false, }, ]; let mut active = 0; diff --git a/crates/op-host-services/src/chat_intent_screen_sets.rs b/crates/op-host-services/src/chat_intent_screen_sets.rs index de4552d03..d22b30b07 100644 --- a/crates/op-host-services/src/chat_intent_screen_sets.rs +++ b/crates/op-host-services/src/chat_intent_screen_sets.rs @@ -44,6 +44,14 @@ fn has_name_chars(part: &str) -> bool { letters.iter().any(|c| !c.is_ascii()) || letters.len() >= 3 } +/// A segment that shed a declared count (`My 2个`) proved it belongs to a +/// counted list, so the generic 3-letter floor is not needed to tell it apart +/// from `16/9` or `UI/UX`. Two letters keep short real names (`My`, `Me`) while +/// still rejecting a bare count (`2个`) or a stray initial. +fn has_counted_name_chars(part: &str) -> bool { + part.chars().filter(|c| c.is_alphabetic()).count() >= 2 +} + fn normalize_target_name(raw: &str) -> Option { let mut name = raw .trim() @@ -53,6 +61,7 @@ fn normalize_target_name(raw: &str) -> Option { // A shared CJK screen noun commonly carries the declared count after the // final name: `星图、观测计划、我的3个界面`. The count describes the list; it is // not part of the last screen's name. + let mut shed_declared_count = false; if name.ends_with('个') { name.pop(); name = name.trim_end().to_string(); @@ -68,6 +77,7 @@ fn normalize_target_name(raw: &str) -> Option { .trim_end_matches('这') .trim_end() .to_string(); + shed_declared_count = true; } if let Some(without_article) = name @@ -77,7 +87,14 @@ fn normalize_target_name(raw: &str) -> Option { name = without_article.trim().to_string(); } - has_name_chars(&name).then_some(name) + // The list is all-or-nothing on purpose (`16/9` must not half-parse), so a + // short trailing name would otherwise kill an otherwise well-formed + // request: `Home、My 2个页面` collapsed to no names at all because `My` is + // two letters. Falling back to the RAW segment was rejected — it would + // ship `My 2个` as a screen name into the continuation contract, which is + // worse than not matching. + (has_name_chars(&name) || (shed_declared_count && has_counted_name_chars(&name))) + .then_some(name) } fn named_list_with(targets: &str, separator: char) -> Option> { @@ -192,9 +209,16 @@ fn listed_whole_screen_names_en(prompt: &str) -> Option> { continue; } let targets = after_verb_original[..noun_pos].trim(); + // The "current / these / …" guard compares against a + // lowercase marker table, so it must read the LOWERCASED + // slice (`the Current explore/profile interfaces` is the + // same request as the all-lowercase one). Name extraction + // keeps the original casing. `to_ascii_lowercase` is + // byte-length preserving, so both slices share indices. + let targets_lower = after_verb[..noun_pos].trim(); let tail = &after_verb_original[noun_pos + noun.len()..]; if !crosses_clause_boundary(targets) - && !targets_reference_existing_screen_en(targets) + && !targets_reference_existing_screen_en(targets_lower) && is_clause_end(tail) { if let Some(names) = named_list_names(targets) { @@ -262,6 +286,53 @@ mod tests { ); } + /// A declared count must not disqualify the name it follows. The list is + /// all-or-nothing, so a short trailing latin name used to take the whole + /// request down with it. + #[test] + fn keeps_short_latin_names_that_carried_the_declared_count() { + assert_eq!( + listed_whole_screen_names("继续生成 Home、My 2个页面"), + ["Home", "My"] + ); + assert_eq!( + listed_whole_screen_names("继续生成 Home、Me 两个页面"), + ["Home", "Me"] + ); + assert_eq!( + listed_whole_screen_names("继续生成 探索、Me 两个界面"), + ["探索", "Me"] + ); + // The relaxation is scoped to the counted segment: an uncounted + // two-letter pair stays out, so `UI/UX` is still not a screen list. + assert!(listed_whole_screen_names("继续完成 UI/UX界面").is_empty()); + // A bare count is not a name. + assert!(listed_whole_screen_names("继续生成 Home、2个页面").is_empty()); + } + + /// The English existing-screen guard reads a lowercase marker table, so it + /// has to be fed the lowercased slice. Reading the original-cased one let + /// `Current` through AND leaked `Current explore` as a screen name. + #[test] + fn english_existing_screen_guard_is_case_insensitive() { + for prompt in [ + "Continue generating the Current explore/profile interfaces", + "Continue generating THE CURRENT explore/profile interfaces", + "Continue generating These explore/profile interfaces", + "Continue generating Those explore/profile interfaces", + "Continue generating the Same explore/profile interfaces", + ] { + assert!( + !requests_listed_whole_screens(prompt), + "expected an in-place request: {prompt}" + ); + assert!( + listed_whole_screen_names(prompt).is_empty(), + "an in-place request must promise no screen names: {prompt}" + ); + } + } + #[test] fn recognizes_listed_follow_on_screens_with_shared_cjk_noun() { for prompt in [ diff --git a/crates/op-host-services/src/design_agent_tools/root_seed.rs b/crates/op-host-services/src/design_agent_tools/root_seed.rs index da6bfb972..dcbb3df9b 100644 --- a/crates/op-host-services/src/design_agent_tools/root_seed.rs +++ b/crates/op-host-services/src/design_agent_tools/root_seed.rs @@ -40,6 +40,7 @@ impl RootSeedTarget { #[derive(Debug, Clone)] pub struct RootSeedGuard { target: RootSeedTarget, + continuation: bool, consumed: bool, } @@ -47,6 +48,7 @@ impl RootSeedGuard { pub fn from_prompt(prompt: &str) -> Self { Self { target: root_seed_target_for_prompt(prompt), + continuation: root_seed_prompt_is_continuation(prompt), consumed: false, } } @@ -54,12 +56,13 @@ impl RootSeedGuard { pub fn disabled() -> Self { Self { target: RootSeedTarget::Desktop, + continuation: false, consumed: true, } } - fn pending_target(&self) -> Option { - (!self.consumed).then_some(self.target) + fn pending_profile(&self) -> Option<(RootSeedTarget, bool)> { + (!self.consumed).then_some((self.target, self.continuation)) } fn mark_consumed(&mut self) { @@ -71,6 +74,22 @@ pub fn root_seed_target_for_prompt(prompt: &str) -> RootSeedTarget { RootSeedTarget::from_mobile(root_seed_prompt_is_mobile(prompt)) } +/// Whether the REQUEST asks to continue an existing screen set. +/// +/// This is the same signal the orchestrator's `ContinuationContext` is built +/// from (`chat_design_request::sibling_continuation_context`), so both paths +/// agree on what counts as a continuation: a prompt that promises named +/// sibling screens. +/// +/// Without this gate, "the canvas happens to hold a frame of the same class" +/// was enough to inherit — and because a continuation deliberately keeps the +/// guard pending, EVERY later top-level frame of that turn was then stamped +/// with the first screen's size and background, including the values the model +/// wrote itself. +pub fn root_seed_prompt_is_continuation(prompt: &str) -> bool { + !crate::chat_intent::listed_whole_screen_names(prompt).is_empty() +} + pub fn root_seed_prompt_is_mobile(prompt: &str) -> bool { if prompt.contains("手机") { return true; @@ -100,7 +119,7 @@ pub(super) fn should_track_root_seed_candidate( return false; } root_seed_guard - .and_then(RootSeedGuard::pending_target) + .and_then(RootSeedGuard::pending_profile) .is_some() || indicator_epoch .and_then(op_editor_core::agent_indicators::root_seed_hint_if_pending) @@ -121,14 +140,14 @@ pub(super) fn maybe_apply_root_seed_guard( indicator_epoch: Option, root_seed_guard: Option<&mut RootSeedGuard>, ) -> Option { - let explicit_target = root_seed_guard + let explicit_profile = root_seed_guard .as_ref() - .and_then(|guard| guard.pending_target()); - let epoch_target = indicator_epoch + .and_then(|guard| guard.pending_profile()); + let epoch_profile = indicator_epoch .and_then(op_editor_core::agent_indicators::root_seed_hint_if_pending) - .map(RootSeedTarget::from_mobile); - let target = explicit_target.or(epoch_target)?; - let profile = resolve_root_seed_profile(state, ids_before, target); + .map(|hint| (RootSeedTarget::from_mobile(hint.mobile), hint.continuation)); + let (target, continuation) = explicit_profile.or(epoch_profile)?; + let profile = resolve_root_seed_profile(state, ids_before, target, continuation); let allow_existing_single_root = !profile.inherited; let seed_hint = seed_root_frame_if_needed(state, ids_before, &profile, allow_existing_single_root); @@ -328,8 +347,13 @@ fn seed_root_frame_if_needed( root.set_height_px(profile.height); changed += 1; } + // The artboard is a contract — sibling screens of one product that do + // not share a frame size are a continuation bug, so a wrong numeric + // width/height above is corrected. The background is not: it is + // authored design intent, so an inherited colour only SEEDS a root the + // model left unfilled and never overwrites one it chose. if let Some(color) = profile.background_color.as_deref() { - if op_editor_core::first_solid_fill_hex(root) != Some(color) + if op_editor_core::first_solid_fill_hex(root).is_none() && op_editor_core::fills::set_primary_fill_hex(root, color) { changed += 1; @@ -352,28 +376,34 @@ fn resolve_root_seed_profile( state: &EditorState, ids_before: &HashSet, target: RootSeedTarget, + continuation: bool, ) -> RootSeedProfile { - let matching_existing_screen = state.active_children().iter().rev().find(|node| { - if !ids_before.contains(node.id_str()) - || !matches!(node, PenNode::Frame(_)) - || node.children().is_none_or(|children| children.is_empty()) - { - return false; - } - let Some(width) = node.width_px() else { - return false; - }; - let Some(height) = node.height_px() else { - return false; - }; - let mobile_width = (320.0..=480.0).contains(&width); - width.is_finite() - && height.is_finite() - && width > 0.0 - && height > 0.0 - && mobile_width == (target == RootSeedTarget::Mobile) + // Inheriting is a CONTINUATION contract, not a canvas fact. Without the + // gate, any turn on a non-empty canvas kept the guard pending forever and + // rewrote every root it produced to the first screen's dimensions. + let matching_existing_screen = continuation.then(|| { + state.active_children().iter().rev().find(|node| { + if !ids_before.contains(node.id_str()) + || !matches!(node, PenNode::Frame(_)) + || node.children().is_none_or(|children| children.is_empty()) + { + return false; + } + let Some(width) = node.width_px() else { + return false; + }; + let Some(height) = node.height_px() else { + return false; + }; + let mobile_width = (320.0..=480.0).contains(&width); + width.is_finite() + && height.is_finite() + && width > 0.0 + && height > 0.0 + && mobile_width == (target == RootSeedTarget::Mobile) + }) }); - if let Some(screen) = matching_existing_screen { + if let Some(screen) = matching_existing_screen.flatten() { return RootSeedProfile { target, width: screen.width_px().expect("matched numeric width"), diff --git a/crates/op-host-services/src/design_agent_tools_continuation_tests.rs b/crates/op-host-services/src/design_agent_tools_continuation_tests.rs index a839e869b..b9484409d 100644 --- a/crates/op-host-services/src/design_agent_tools_continuation_tests.rs +++ b/crates/op-host-services/src/design_agent_tools_continuation_tests.rs @@ -1,4 +1,5 @@ -//! Sequential continuation regressions for the turn-scoped root contract. +//! Regressions for the turn-scoped root contract: which turns may inherit a +//! live screen's artboard, and what an inherited profile is allowed to rewrite. use super::*; @@ -15,7 +16,10 @@ fn continuation_guard_survives_existing_only_batch_and_normalizes_later_roots() })) .expect("existing mobile screen"), ); - let mut guard = RootSeedGuard::from_prompt("mobile continuation"); + // The prompt is what makes this a continuation: it names the sibling + // screens it promises. A turn that merely runs on a non-empty canvas must + // not inherit anything. + let mut guard = RootSeedGuard::from_prompt("手机上继续生成 星图、观测计划 两个界面"); // DeepSeek may spend its first batch editing the old screen. That must // neither mutate its chrome nor consume the contract needed by siblings. @@ -66,10 +70,13 @@ fn continuation_guard_survives_existing_only_batch_and_normalizes_later_roots() (Some(390.0), Some(844.0)), "{name} must inherit the live artboard" ); + // 星图 chose its own background and KEEPS it — an inherited artboard + // fixes the frame size, not the palette. 观测计划 authored none, so it + // inherits the newest live screen's, which by then is 星图's. assert_eq!( op_editor_core::first_solid_fill_hex(root), - Some("#050508"), - "{name} must inherit the live background" + Some("#16002E"), + "{name} background" ); assert_eq!( root.children() @@ -80,3 +87,93 @@ fn continuation_guard_survives_existing_only_batch_and_normalizes_later_roots() ); } } + +/// The empty-canvas twin of this test cannot fail: with nothing to match +/// against, the seed profile is the default one, which already leaves authored +/// numbers alone. The regression only exists once a matching screen is on the +/// canvas — an ORDINARY turn there used to inherit its artboard and stamp the +/// model's explicit width, height AND fill. +#[test] +fn ordinary_turn_on_a_populated_canvas_keeps_every_authored_root_value() { + let mut state = EditorState::new(); + state.active_children_mut().clear(); + state.active_children_mut().push( + serde_json::from_value(serde_json::json!({ + "type": "frame", "id": "home", "name": "Home", + "width": 390, "height": 844, + "fill": [{ "type": "solid", "color": "#050508" }], + "children": [{ "type": "text", "id": "home-title", "content": "Home" }] + })) + .expect("existing mobile screen"), + ); + // No promised sibling screens -> not a continuation. + let mut guard = RootSeedGuard::from_prompt("mobile app"); + let (result, mutated) = execute_design_tool_with_root_seed_guard( + &mut state, + "batch_design", + r##"{"operations":"wide=I(null,{type:'frame',name:'Wide',width:1200,height:1600,fill:[{type:'solid',color:'#FFFFFF'}]})"}"##, + None, + Some(&mut guard), + ); + + assert!(!result.is_error, "batch failed: {}", result.content); + assert!(mutated); + let wide = state + .active_children() + .iter() + .find(|node| node.base().name.as_deref() == Some("Wide")) + .expect("authored root"); + assert_eq!( + (wide.width_px(), wide.height_px()), + (Some(1200.0), Some(1600.0)), + "authored dimensions must survive an ordinary turn" + ); + assert_eq!( + op_editor_core::first_solid_fill_hex(wide), + Some("#FFFFFF"), + "authored background must survive an ordinary turn" + ); +} + +/// The guard stays pending across a continuation turn, so without this the +/// stamping applied to every root for the rest of the turn. A turn with no +/// continuation signal must consume it after the first batch, exactly as a +/// fresh-design turn always did. +#[test] +fn ordinary_turn_on_a_populated_canvas_consumes_the_guard_after_one_batch() { + let mut state = EditorState::new(); + state.active_children_mut().clear(); + state.active_children_mut().push( + serde_json::from_value(serde_json::json!({ + "type": "frame", "id": "home", "name": "Home", + "width": 390, "height": 844, + "children": [{ "type": "text", "id": "home-title", "content": "Home" }] + })) + .expect("existing mobile screen"), + ); + let mut guard = RootSeedGuard::from_prompt("mobile app"); + for operations in [ + r#"{"operations":"first=I(null,{type:'frame',name:'First',width:390,height:844})"}"#, + r#"{"operations":"second=I(null,{type:'frame',name:'Second',width:'fit_content',height:'fit_content'})"}"#, + ] { + let (result, mutated) = execute_design_tool_with_root_seed_guard( + &mut state, + "batch_design", + operations, + None, + Some(&mut guard), + ); + assert!(!result.is_error, "batch failed: {}", result.content); + assert!(mutated); + } + let second = state + .active_children() + .iter() + .find(|node| node.base().name.as_deref() == Some("Second")) + .expect("second top-level frame"); + assert_eq!( + (second.width_px(), second.height_px()), + (None, None), + "the guard must not survive an ordinary turn's first batch" + ); +} diff --git a/crates/op-host-services/src/design_agent_tools_tests.rs b/crates/op-host-services/src/design_agent_tools_tests.rs index 736afe7f4..87d477f1f 100644 --- a/crates/op-host-services/src/design_agent_tools_tests.rs +++ b/crates/op-host-services/src/design_agent_tools_tests.rs @@ -469,7 +469,9 @@ fn continuation_seed_inherits_every_mobile_screen_and_repairs_wrong_numeric_size })) .expect("existing mobile screen"), ); - let mut guard = RootSeedGuard::from_prompt("mobile continuation"); + // The artboard is inherited because the REQUEST promises sibling screens, + // not because the canvas happens to hold a frame. + let mut guard = RootSeedGuard::from_prompt("手机上继续生成 星图、观测计划、我的3个界面"); let (result, mutated) = execute_design_tool_with_root_seed_guard( &mut state, "batch_design", @@ -497,13 +499,24 @@ fn continuation_seed_inherits_every_mobile_screen_and_repairs_wrong_numeric_size (root.width_px(), root.height_px()), (Some(390.0), Some(844.0)) ); - assert_eq!(op_editor_core::first_solid_fill_hex(root), Some("#050508")); assert_eq!( root.children() .and_then(|children| children.first()) .and_then(|child| child.base().role.as_deref()), Some("status-bar") ); + // The artboard is a contract; the background is authored intent. Only + // the roots the model left unfilled inherit the live screen's colour. + let expected_fill = match root.base().name.as_deref() { + Some("星图") => "#16002E", + _ => "#050508", + }; + assert_eq!( + op_editor_core::first_solid_fill_hex(root), + Some(expected_fill), + "{:?}", + root.base().name + ); } let value: serde_json::Value = serde_json::from_str(&result.content).unwrap(); assert!(value["layoutHint"]