From cd2131ce28028f723fa55baa2294ddc899fbd658 Mon Sep 17 00:00:00 2001 From: Fini Date: Thu, 2 Jul 2026 21:21:28 +0800 Subject: [PATCH] feat(smoke): minimal scaffold seed for loop (OPENPENCIL_SMOKE_LOOP_SEED) reusing build_fallback_plan; data-gate harness for orchestrator-vs-loop migration --- Cargo.lock | 1 + crates/op-smoke/Cargo.toml | 5 + crates/op-smoke/src/loop_mode.rs | 27 ++++ crates/op-smoke/src/loop_mode_tests.rs | 58 ++++++++ crates/op-smoke/src/loop_seed.rs | 178 +++++++++++++++++++++++++ crates/op-smoke/src/loop_seed_tests.rs | 127 ++++++++++++++++++ crates/op-smoke/src/main.rs | 11 +- 7 files changed, 406 insertions(+), 1 deletion(-) create mode 100644 crates/op-smoke/src/loop_seed.rs create mode 100644 crates/op-smoke/src/loop_seed_tests.rs diff --git a/Cargo.lock b/Cargo.lock index 182aa233d..4111a06b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3428,6 +3428,7 @@ version = "0.8.0" dependencies = [ "agent", "futures", + "jian-ops-schema", "op-ai", "op-ai-skills", "op-editor-core", diff --git a/crates/op-smoke/Cargo.toml b/crates/op-smoke/Cargo.toml index 166ca557f..184ed462f 100644 --- a/crates/op-smoke/Cargo.toml +++ b/crates/op-smoke/Cargo.toml @@ -24,6 +24,11 @@ op-orchestrator = { path = "../op-orchestrator" } # `EditorState` + `EditorCommand` — the smoke's own `InlineDocSink` owns # one and applies commands on it. op-editor-core = { path = "../op-editor-core" } +# `PenNode` — the minimal-seed loop mode (OPENPENCIL_SMOKE_LOOP_SEED=1) builds a +# page-root + named section-stub subtree as canonical `PenNode`s (same JSON-build +# → deserialize idiom op-orchestrator's `build_scaffold` uses) and applies it via +# `EditorCommand::InsertSubtree`. op-orchestrator already pulls this same path dep. +jian-ops-schema = { path = "../../vendor/jian/crates/jian-ops-schema" } # `merge_library_into_state` — honors OPENPENCIL_SMOKE_LIBRARY by loading a # harvested component library (.lib.op) into the doc before generation so the # generator can instantiate its reusable masters. diff --git a/crates/op-smoke/src/loop_mode.rs b/crates/op-smoke/src/loop_mode.rs index 7befe98a3..1f8acddc3 100644 --- a/crates/op-smoke/src/loop_mode.rs +++ b/crates/op-smoke/src/loop_mode.rs @@ -181,6 +181,13 @@ fn build_design_provider( /// /// `OPENPENCIL_SMOKE_DUMP` streams per-delta text / tool-call / thinking /// lines to stderr so ab-v9 logs capture the run. +/// +/// `seed = true` (the `OPENPENCIL_SMOKE_LOOP_SEED=1` path) applies a MINIMAL +/// scaffold seed to the `EditorState` BEFORE the loop runs — a page-root frame +/// plus a few empty named section stubs, derived from the orchestrator's +/// heuristic fallback planner (see [`crate::loop_seed`]). The system prompt is +/// augmented to tell the model the scaffold exists and to fill the sections. +/// `seed = false` is byte-for-byte the original pure-loop path. #[allow(clippy::too_many_arguments)] pub fn run_loop( base_url: String, @@ -193,6 +200,7 @@ pub fn run_loop( max_output_tokens: u32, dump: bool, library_path: Option, + seed: bool, ) -> Result>, String> { // `OPENPENCIL_SMOKE_STARTER=1` seeds the fresh-canvas starter frame so the // loop exercises the same `replaceEmptyFrame` reuse path the desktop GUI @@ -220,6 +228,25 @@ pub fn run_loop( report.themes_added ); } + // Minimal scaffold seed (`OPENPENCIL_SMOKE_LOOP_SEED=1`): apply a page-root + // + named empty section stubs to the live state BEFORE the loop runs, so a + // weak model has structure to fill instead of an empty canvas it collapses + // on. `seed = false` ⇒ this whole block is skipped (pure-loop, unchanged). + let mut system_prompt = system_prompt; + if seed { + let cmd = crate::loop_seed::build_seed_command(&user_prompt)?; + let applied = initial.apply(cmd); + if !applied { + return Err("minimal scaffold seed failed to apply to EditorState".into()); + } + system_prompt.push_str(&crate::loop_seed::seed_system_prompt_suffix(&user_prompt)); + eprintln!( + "[LOOP][seed] minimal scaffold applied: {} top-level node(s), \ + system prompt augmented to fill seeded sections", + initial.active_children().len() + ); + } + let state = Arc::new(Mutex::new(initial)); let executor = Arc::new(HeadlessExecutor::new(state.clone(), dump)); diff --git a/crates/op-smoke/src/loop_mode_tests.rs b/crates/op-smoke/src/loop_mode_tests.rs index d112d8a5c..d443a0555 100644 --- a/crates/op-smoke/src/loop_mode_tests.rs +++ b/crates/op-smoke/src/loop_mode_tests.rs @@ -48,6 +48,63 @@ fn headless_executor_batch_design_mutates_shared_state() { ); } +#[test] +fn headless_executor_emit_elements_produces_role_tagged_nodes() { + // The element-builder loop tool: a couple of high-level `el` lines must + // expand — via the SAME `op_orchestrator::manifest` assembler the + // orchestrator MANIFEST path uses — into role-tagged subtrees (stat-card), + // and land in the shared EditorState. This is the load-bearing capability + // the bare loop lacked: high-level element kinds + semantic roles, not raw + // primitives. + use jian_ops_schema::node::PenNode; + use op_editor_core::PenNodeExt; + + fn collect_roles(node: &PenNode, out: &mut Vec) { + if let Some(role) = node.base().role.as_deref() { + if !role.is_empty() { + out.push(role.to_string()); + } + } + if let Some(children) = node.children() { + for child in children { + collect_roles(child, out); + } + } + } + + let state = Arc::new(Mutex::new(EditorState::new())); + let exec = HeadlessExecutor::new(state.clone(), false); + + let args = r#"{"elements":"[{\"el\":\"stat_card\",\"label\":\"MRR\",\"value\":\"$48.2k\",\"trend\":\"up\"},{\"el\":\"stat_card\",\"label\":\"Users\",\"value\":\"12.4k\"}]"}"#; + let result = exec.execute("emit_elements", args); + assert!( + !result.is_error, + "emit_elements must succeed, got: {}", + result.content + ); + let v: serde_json::Value = serde_json::from_str(&result.content).expect("valid JSON envelope"); + assert_eq!(v["success"], serde_json::Value::Bool(true)); + assert_eq!( + v["data"]["elementLines"], "2", + "two element lines built, got: {}", + result.content + ); + + // The shared state really mutated, and the inserted subtree carries the + // semantic `stat-card` role the element builder stamps. + let guard = state.lock().unwrap(); + let children = guard.active_children(); + assert_eq!(children.len(), 2, "two top-level stat cards inserted"); + let mut roles = Vec::new(); + for node in children { + collect_roles(node, &mut roles); + } + assert!( + roles.iter().any(|r| r == "stat-card"), + "emit_elements must produce role-tagged nodes (stat-card), got roles {roles:?}" + ); +} + #[test] fn headless_executor_spawn_agents_is_honestly_deferred() { // spawn_agents can't run headlessly (desktop-only sub-loop launch). The @@ -176,6 +233,7 @@ fn loop_mode_against_mock_provider_produces_real_op() { 2048, false, None, + false, ) .expect("loop runs to completion"); diff --git a/crates/op-smoke/src/loop_seed.rs b/crates/op-smoke/src/loop_seed.rs new file mode 100644 index 000000000..c0571aaaa --- /dev/null +++ b/crates/op-smoke/src/loop_seed.rs @@ -0,0 +1,178 @@ +//! Minimal scaffold seed for the headless agentic loop +//! (`OPENPENCIL_SMOKE_LOOP=1` + `OPENPENCIL_SMOKE_LOOP_SEED=1`). +//! +//! This is the SIMPLIFIED generation path that copies Pencil's single +//! agentic loop while keeping a tiny weak-model safety net. Instead of the +//! COMPLEX `Orchestrator` (planning LLM with mode-rotation, 3 scaffold +//! strategies, per-subtask sub-agent fan-out, 3-attempt retry ladder), the +//! seed path is exactly: +//! +//! ```text +//! MINIMAL SCAFFOLD SEED → ONE agentic tool-loop fills it → finalize backstop +//! ``` +//! +//! ## What is reused vs. new +//! +//! - **The lightweight planner is reused** — [`build_seed_subtree`] calls +//! `op_orchestrator::plan::build_fallback_plan`, the orchestrator's +//! heuristic, NO-LLM fallback planner. It already derives the root-frame +//! spec (width / height / layout / gap / fill, mobile-aware including +//! explicit `390x844`-style sizes) AND a small set of named sections +//! (1–3 for landing, top-summary + main for mobile) straight from the +//! prompt. We call it with `concurrency: 1` so none of the concurrent / +//! dashboard branches can engage — this is the planner's simplest mode. +//! - **The scaffold *construction idiom* is reused** — like the +//! orchestrator's `scaffold::build_scaffold` we build the root frame as a +//! `serde_json::Value` and `serde_json::from_value` it into a canonical +//! `PenNode`, then apply ONE `EditorCommand::InsertSubtree`. The single +//! genuinely new piece is injecting the EMPTY named SECTION child frames: +//! the orchestrator deliberately leaves the root childless because its +//! per-subtask fan-out fills the sections later — the seed path drops the +//! fan-out, so it seeds the section stubs itself. They are the structure +//! weak models collapse without. +//! - **No new post-processing** — `finalize_on_exit` stays true on the +//! loop, so `op_orchestrator::apply_loop_finalize` (the Class-A +//! structural backstop) runs at the end exactly as in pure-loop mode. + +use jian_ops_schema::node::PenNode; +use op_editor_core::{EditorCommand, NodeId}; +use op_orchestrator::plan::{build_fallback_plan, OrchestratorPlan}; +use op_orchestrator::types::DesignRequest; + +/// Build the minimal seed as ONE `EditorCommand::InsertSubtree`: a +/// page-root frame carrying a small number of EMPTY named section-stub +/// child frames, derived from the orchestrator's heuristic fallback plan. +/// +/// Returns `Err` only when the root-frame JSON template fails to +/// deserialize (an implementation bug, never a prompt problem) — the +/// caller treats that as "skip the seed, run the pure loop". +pub fn build_seed_command(prompt: &str) -> Result { + let req = DesignRequest { + prompt: prompt.to_string(), + model: None, + provider: None, + design_md: None, + // concurrency 1 ⇒ the planner's simplest single-screen shape; the + // seed path never engages the concurrent / dashboard branches. + concurrency: 1, + append_context: None, + validation_enabled: false, + visual_ref_enabled: false, + }; + // Reuse the orchestrator's NO-LLM heuristic planner for the root-frame + // spec + the section names/count. + let plan = build_fallback_plan(&req); + let node = build_seed_subtree(&plan)?; + Ok(EditorCommand::InsertSubtree { + nodes: vec![node], + parent_id: NodeId::NONE, + page_id: None, + }) +} + +/// Section stubs hug their content vertically as the loop fills them — no +/// explicit height — and span the page-root width. +fn section_stub_json(plan: &OrchestratorPlan, index: usize) -> serde_json::Value { + let st = &plan.subtasks[index]; + let id = if st.id.is_empty() { + format!("section-{}", index + 1) + } else { + st.id.clone() + }; + let name = if st.label.is_empty() { + format!("Section {}", index + 1) + } else { + st.label.clone() + }; + serde_json::json!({ + "type": "frame", + "id": format!("seed-{id}"), + "name": name, + "width": "fill_container", + // No "height" key ⇒ fit_content: the section grows as the loop + // populates it instead of being frozen to a guessed pixel box. + "layout": "vertical", + "gap": 16, + "fill": [], + "children": [], + }) +} + +/// Build the page-root frame `PenNode` with one empty named section child +/// per plan subtask. Mirrors `scaffold::build_root_frame_node`'s +/// build-JSON-then-deserialize idiom so the canonical parse path (not a +/// hand-written struct literal) validates the shape. +fn build_seed_subtree(plan: &OrchestratorPlan) -> Result { + let rf = &plan.root_frame; + let layout = rf.layout.as_deref().unwrap_or("vertical"); + let fill_hex = rf + .first_solid_hex() + .unwrap_or_else(|| "#FFFFFF".to_string()); + let gap = rf.gap.filter(|g| *g > 0.0).unwrap_or(20.0); + + let sections: Vec = (0..plan.subtasks.len()) + .map(|i| section_stub_json(plan, i)) + .collect(); + + let root = serde_json::json!({ + "type": "frame", + "id": format!("seed-{}", rf.id), + "name": rf.name, + "x": 80, + "y": 40, + "width": rf.width, + "height": rf.height, + "layout": layout, + "gap": gap, + "fill": [{ "type": "solid", "color": fill_hex }], + "children": sections, + }); + + serde_json::from_value(root).map_err(|e| format!("seed root frame: {e}")) +} + +/// Augment the design-agent system prompt so the model knows a scaffold +/// already exists and its job is to FILL the seeded sections (not to start +/// from an empty canvas). Appended to the base prompt for seed mode only. +pub fn seed_system_prompt_suffix(prompt: &str) -> String { + let req = DesignRequest { + prompt: prompt.to_string(), + model: None, + provider: None, + design_md: None, + concurrency: 1, + append_context: None, + validation_enabled: false, + visual_ref_enabled: false, + }; + let plan = build_fallback_plan(&req); + let section_list = plan + .subtasks + .iter() + .enumerate() + .map(|(i, st)| { + let label = if st.label.is_empty() { + format!("Section {}", i + 1) + } else { + st.label.clone() + }; + format!(" - \"{label}\"") + }) + .collect::>() + .join("\n"); + + format!( + "\n\n## Scaffold already created\n\ + A page-root frame with these EMPTY named sections has ALREADY been \ + inserted on the canvas:\n{section_list}\n\ + Do NOT recreate the page-root or duplicate the design. Read the canvas \ + (get_editor_state / get_screenshot), then FILL each named section in \ + place with its content using batch_design — adjust or add sections as \ + the prompt needs, but build on the existing scaffold rather than \ + starting over.\n" + ) +} + +#[cfg(test)] +#[path = "loop_seed_tests.rs"] +mod tests; diff --git a/crates/op-smoke/src/loop_seed_tests.rs b/crates/op-smoke/src/loop_seed_tests.rs new file mode 100644 index 000000000..c3ca6eb2c --- /dev/null +++ b/crates/op-smoke/src/loop_seed_tests.rs @@ -0,0 +1,127 @@ +//! Tests for the minimal scaffold seed (`OPENPENCIL_SMOKE_LOOP_SEED=1`). +//! +//! The seed is the structure-providing step that runs BEFORE the agentic +//! loop. These tests prove that — independent of any network / LLM — a +//! fresh `EditorState` becomes a non-empty seeded tree (a page-root frame +//! carrying named empty section stubs) once the seed command is applied. + +use jian_ops_schema::node::PenNode; +use op_editor_core::EditorState; + +use crate::loop_seed::{build_seed_command, seed_system_prompt_suffix}; + +/// Pull the single seeded page-root frame out of a state's active children, +/// asserting exactly one top-level node exists. +fn seeded_root(state: &EditorState) -> &jian_ops_schema::node::frame::FrameNode { + let children = state.active_children(); + assert_eq!( + children.len(), + 1, + "seed must produce exactly one top-level page-root frame, got {}", + children.len() + ); + match &children[0] { + PenNode::Frame(f) => f, + other => panic!("seeded root must be a frame, got: {other:?}"), + } +} + +#[test] +fn seed_produces_page_root_with_named_section_stubs() { + // A landing-page prompt long enough to trip the planner's 3-section split. + let prompt = "Design a marketing landing page for a developer productivity \ + tool with a hero, feature highlights, and a call to action footer"; + let mut state = EditorState::new(); + assert!( + state.active_children().is_empty(), + "fresh state must start empty" + ); + + let cmd = build_seed_command(prompt).expect("seed command builds"); + assert!(state.apply(cmd), "seed command must apply to EditorState"); + + // The state is now NON-EMPTY: a page-root frame exists. + let root = seeded_root(&state); + assert!( + root.base.name.is_some(), + "page-root frame must carry a name" + ); + + // The page-root carries ≥ 1 empty named section stub. + let sections = root + .children + .as_ref() + .expect("page-root must carry section children"); + assert!( + !sections.is_empty(), + "page-root must seed at least one section stub" + ); + for section in sections { + let PenNode::Frame(sf) = section else { + panic!("each section stub must be a frame, got: {section:?}"); + }; + assert!( + sf.base + .name + .as_deref() + .map(|n| !n.is_empty()) + .unwrap_or(false), + "each section stub must be NAMED" + ); + // Stubs are EMPTY — the loop fills them, the seed only provides slots. + let empty = sf.children.as_ref().map(|c| c.is_empty()).unwrap_or(true); + assert!(empty, "section stub `{:?}` must start empty", sf.base.name); + } +} + +#[test] +fn seed_mobile_prompt_produces_top_summary_and_main() { + // A mobile prompt routes the heuristic planner to the mobile preset + // (Top Summary + Main Content), proving the seed reuses the planner's + // design-type detection rather than a single fixed shape. + let prompt = "a 390x844 mobile food delivery home screen"; + let mut state = EditorState::new(); + let cmd = build_seed_command(prompt).expect("mobile seed builds"); + assert!(state.apply(cmd), "mobile seed applies"); + + let root = seeded_root(&state); + let names: Vec = root + .children + .as_ref() + .expect("mobile page-root has sections") + .iter() + .filter_map(|n| match n { + PenNode::Frame(f) => f.base.name.clone(), + _ => None, + }) + .collect(); + assert!( + names.iter().any(|n| n == "Top Summary"), + "mobile seed must include a Top Summary section, got: {names:?}" + ); + assert!( + names.iter().any(|n| n == "Main Content"), + "mobile seed must include a Main Content section, got: {names:?}" + ); +} + +#[test] +fn seed_system_prompt_suffix_lists_seeded_sections() { + // The augmented system prompt must tell the model the scaffold exists + // and name the very sections the seed inserted, so a weak model fills + // them instead of redrawing the whole design. + let prompt = "a 390x844 mobile food delivery home screen"; + let suffix = seed_system_prompt_suffix(prompt); + assert!( + suffix.contains("Scaffold already created"), + "suffix must announce the scaffold, got: {suffix}" + ); + assert!( + suffix.contains("Top Summary") && suffix.contains("Main Content"), + "suffix must list the seeded section names, got: {suffix}" + ); + assert!( + suffix.to_lowercase().contains("do not recreate"), + "suffix must warn against recreating the page-root, got: {suffix}" + ); +} diff --git a/crates/op-smoke/src/main.rs b/crates/op-smoke/src/main.rs index df2c3cf63..c37798f9a 100644 --- a/crates/op-smoke/src/main.rs +++ b/crates/op-smoke/src/main.rs @@ -39,6 +39,7 @@ use std::sync::Arc; mod loop_mode; +mod loop_seed; use agent::abort::AbortController; use agent::provider::anthropic::AnthropicProvider; @@ -449,8 +450,15 @@ async fn run_loop_mode(prompt: String) -> std::process::ExitCode { .and_then(|s| s.parse().ok()) .unwrap_or(8192); let thinking = loop_thinking_mode(); + // `OPENPENCIL_SMOKE_LOOP_SEED=1` (only meaningful with OPENPENCIL_SMOKE_LOOP=1) + // arms the minimal-seed path: a page-root + named section stubs are applied + // before the SAME agentic loop runs to fill them. Unset ⇒ pure loop (the + // existing behaviour, byte-for-byte unchanged). + let seed = std::env::var("OPENPENCIL_SMOKE_LOOP_SEED") + .map(|v| v == "1" || v.eq_ignore_ascii_case("true") || v.eq_ignore_ascii_case("on")) + .unwrap_or(false); - eprintln!("[SMOKE] mode=loop model={model} base_url={base_url}"); + eprintln!("[SMOKE] mode=loop seed={seed} model={model} base_url={base_url}"); eprintln!("[SMOKE] prompt={prompt:?} thinking={thinking:?} max_tokens={max_tokens}"); let system_prompt = op_ai_skills::design_agent_system_prompt().to_string(); @@ -477,6 +485,7 @@ async fn run_loop_mode(prompt: String) -> std::process::ExitCode { max_tokens, dump, library_path, + seed, ) }) .await;