From bc70639f94e442576696b5af0e3eb8c8fb5534d9 Mon Sep 17 00:00:00 2001 From: Fini Date: Fri, 7 Aug 2026 01:56:35 +0800 Subject: [PATCH] fix(ai): require first-class styled interactive widgets --- .../skills/phases/agent/design-agent.md | 4 +- .../phases/generation/jian-components.md | 75 ++++++------- .../skills/phases/generation/schema.md | 12 +++ crates/op-ai-skills/src/lib.rs | 100 +++++++++++++++--- crates/op-ai-skills/src/loader.rs | 83 +++++++++++---- crates/op-orchestrator/src/parse.rs | 9 ++ crates/op-orchestrator/src/parse_tests.rs | 31 ++++++ crates/op-orchestrator/src/prompt.rs | 29 +++-- .../src/subagent_run_subtask_tests.rs | 81 ++++++++++++++ 9 files changed, 340 insertions(+), 84 deletions(-) diff --git a/crates/op-ai-skills/skills/phases/agent/design-agent.md b/crates/op-ai-skills/skills/phases/agent/design-agent.md index 85e17d3f3..624f7c07d 100644 --- a/crates/op-ai-skills/skills/phases/agent/design-agent.md +++ b/crates/op-ai-skills/skills/phases/agent/design-agent.md @@ -89,12 +89,14 @@ const id = I(parent, { ...node... }); // inserts a node, RETURNS its new id (a - `parent` is `null` for a top-level root, or an id returned by an earlier `I(...)` call — a node is a child of X only if you call `I(X, {...})`. - Use REAL JavaScript — `const`/`let`, arrays of data, `for...of` / `.forEach` loops — to generate repeated structure (table rows, nav items, cards, list items) by looping over a data array instead of copy-pasting near-identical `I(...)` calls. PREFER a loop over hand-repeated calls. - `C`, `U`, `D`, `M`, `R`, and `G` are unsupported in script mode. Calling one rejects the script with an instruction to use `operations`; it never reports success while silently dropping the edit. `console.log`/`warn`/`error` are swallowed. `I(parent, obj)` and `K(kitId, parent, overrides)` are the only design calls with real effect inside a script. -- Each node object starts with `type` (`"frame"`/`"text"`/`"rectangle"`/`"ellipse"`/`"path"`/`"icon_font"`) and uses camelCase props (`cornerRadius`, `fontSize`, `fontWeight`, `justifyContent`, `alignItems`, `clipContent`). Do NOT set `x`/`y` on children inside layout frames. Inside a `layout: "none"` container the OPPOSITE holds: every child needs explicit NUMERIC `x`/`y`/`width`/`height` — `fill_container` has no meaning without a flex parent and renders skewed. +- Each node object starts with `type`. Primitive types include `"frame"`/`"text"`/`"rectangle"`/`"ellipse"`/`"path"`/`"icon_font"`; native interactive types are first-class nodes governed by the shared contract below. Use camelCase props (`cornerRadius`, `fontSize`, `fontWeight`, `justifyContent`, `alignItems`, `clipContent`). Do NOT set `x`/`y` on children inside layout frames. Inside a `layout: "none"` container the OPPOSITE holds: every child needs explicit NUMERIC `x`/`y`/`width`/`height` — `fill_container` has no meaning without a flex parent and renders skewed. - Every frame/group/rectangle with flow children MUST declare `layout: "vertical"` or `layout: "horizontal"`; use `layout: "none"` only for a deliberate absolute stack. Omission is ambiguous and will be reported as an `intentQuestion` rather than auto-corrected. - **Absolute-stack z-order is front-to-back by child index:** in `layout: "none"`, `children[0]` is TOPMOST because the canvas paints children in reverse. Put badges, labels, controls, scrims, and other overlays BEFORE the full-bleed image/background they must cover; repair a hidden overlay with `M(overlayId, stackId, 0)`. Keep media in a separate EMPTY frame/rectangle image slot and target that exact slot with strict `G(...)` — never target the stack container that also owns the overlay. - **Icons:** `iconFontName` is the GLYPH name (`"home"`, `"compass"`, `"heart"`, `"search"`), NEVER the font family. Correct: `{type:"icon_font", iconFontName:"compass", width:20, height:20, fill:"#78716C"}`. Writing `iconFontName:"lucide"` renders a tiny fallback dot — every icon in the design breaks. - **`$variable` refs only when they exist:** reference `$color-*` variables only after `get_variables` shows them (or you created them via `set_variables`). A `$ref` against an empty variable table renders as a fallback color. +{{jianComponents}} + Example: ``` diff --git a/crates/op-ai-skills/skills/phases/generation/jian-components.md b/crates/op-ai-skills/skills/phases/generation/jian-components.md index bfe87fa21..70b2f72fc 100644 --- a/crates/op-ai-skills/skills/phases/generation/jian-components.md +++ b/crates/op-ai-skills/skills/phases/generation/jian-components.md @@ -1,6 +1,6 @@ --- name: jian-components -description: Interactive widget family — emit role-marked frames the promotion pass collapses into real text_input / switch / select / checkbox / slider / text_area nodes +description: Interactive widget family — emit first-class native controls with explicit state and design-system styling phase: [generation] trigger: null priority: 5 @@ -8,55 +8,42 @@ budget: 1200 category: base --- -INTERACTIVE WIDGETS (jian component family): +INTERACTIVE WIDGETS (jian component family) — FIRST-CLASS OUTPUT: -When a design has a form field, toggle, dropdown, slider, or multi-line -text box, emit a `frame` and set its `role` to one of the markers below. -A promotion pass collapses each marked frame into a real widget node — so -the output `.op` carries a true `text_input` / `switch` / … node, not a -mockup frame. ONLY these exact `role` strings are honoured; any other -value stays a plain frame. +Emit the native node directly through `I(parent, {...})` or canonical JSON. +Do not assemble a visual imitation from frame/rectangle/text children, and do +not author a role-marked frame expecting a later promotion pass. All controls +except `tabs` are leaves; `tabs.children[i]` is the panel for `tabs[i]`. -ROLE → WIDGET (the only honoured markers): +REQUIRED SEMANTIC PROPS (never omit these in generated designs): -- `role: "input"` or `role: "form-input"` → text_input (single-line field) -- `role: "textarea"` or `role: "text-area"` → text_area (multi-line) -- `role: "select"` or `role: "dropdown"` → select (option picker) -- `role: "switch"` or `role: "toggle"` → switch (on/off) -- `role: "checkbox"` → checkbox (label + box) -- `role: "slider"` → slider (range) -- `role: "radio-group"` (alias `"radio"`) → radio_group — options; each - visible text child of a marked legacy frame becomes one option. -- `role: "number-input"` → number_input — placeholder / value / min / - max; muted text child = placeholder, plain text child = value. -- `role: "progress"` (alias `"progress-bar"`) → progress — value / max / - indeterminate; display-only (not focusable). +- `text_input`, `text_area`: `value` plus an intentional `placeholder`. +- `select`, `radio_group`: `options: [{value,label}]` plus selected `value`. +- `switch`, `checkbox`: explicit `checked`; checkbox may also carry `label`. +- `slider`: numeric `min`, `max`, `step`, and `value`. +- `number_input`: numeric `min`, `max`, `step`, and `value`. +- `progress`: numeric `max` and `value` (`indeterminate` only when intended). +- `tabs`: `tabs: [{value,label}]`, active `value`, and one child panel per tab. -Tabs are NOT promoted from role markers — emit first-class `tabs` nodes -directly (they're in the allowed node kinds). +DESIGN-SYSTEM STYLE CONTRACT: -(Alternatively `semantics: { role: "input" }` also promotes to text_input.) +- Every native control MUST explicitly carry `fill`, `stroke`, and + `cornerRadius` values taken from the active design system; keep width/height + intentional too. Never rely on renderer defaults. +- `fill` is the active/accent paint (or the field/control surface where + applicable). `stroke.fill` is the inactive track/border paint. Use palette + tokens consistently so switches, sliders, progress, selections, and fields + belong to the same product instead of falling back to generic white/grey. +- `fill` is an array; `stroke` is `{thickness, fill:[...]}`. -CHILD STRUCTURE the promotion reads (put these INSIDE the marked frame): +Example native controls (the same objects work in JSONL): -- Placeholder text: a `text` child whose fill is a MUTED grey - (e.g. `#9CA3AF`). The first muted text becomes the widget `placeholder`. -- Value text: a `text` child with any non-muted fill becomes the `value` - (for checkbox it becomes the `label`). -- Leading / trailing icons: `icon_font` children. The FIRST `icon_font` is - the leading icon (e.g. `mail`), a SECOND is the trailing icon (e.g. an - `eye` password reveal). They are carried onto the promoted text_input. -- Style (fill / stroke / cornerRadius / effects) and width/height on the - frame are carried verbatim onto the widget. Other children are dropped — - widgets are leaves. +`I(parent,{type:"select",value:"north",options:[{value:"north",label:"North"}],width:240,height:44,fill:[{type:"solid",color:"#211238"}],stroke:{thickness:1,fill:[{type:"solid",color:"#7C5A9E"}]},cornerRadius:12})` -EXAMPLE (an email field that becomes a text_input): +`I(parent,{type:"slider",min:0,max:100,step:5,value:40,width:280,height:44,fill:[{type:"solid",color:"#A855F7"}],stroke:{thickness:1,fill:[{type:"solid",color:"#4B3A5F"}]},cornerRadius:22})` -{"type":"frame","id":"emailField","role":"input","width":320,"height":48, - "cornerRadius":12,"fill":[{"type":"solid","color":"#F3F4F6"}],"children":[ - {"type":"icon_font","id":"mailIcon","iconFontName":"mail","width":20,"height":20}, - {"type":"text","id":"ph","content":"you@example.com","fill":[{"type":"solid","color":"#9CA3AF"}]} - ]} - -A `role: "switch"` frame needs no children; a `role: "checkbox"` frame -takes one non-muted `text` child as its label. +LEGACY COMPATIBILITY ONLY: old documents may still contain frames whose roles +are promoted: `input`/`form-input`, `textarea`/`text-area`, `select`/`dropdown`, +`switch`/`toggle`, `checkbox`, `slider`, `radio-group`/`radio`, `number-input`, +or `progress`/`progress-bar` (and `semantics.role: "input"`). Keep accepting +those inputs, but NEVER choose that representation for new generation. diff --git a/crates/op-ai-skills/skills/phases/generation/schema.md b/crates/op-ai-skills/skills/phases/generation/schema.md index 299b8ed43..8d1c20acf 100644 --- a/crates/op-ai-skills/skills/phases/generation/schema.md +++ b/crates/op-ai-skills/skills/phases/generation/schema.md @@ -16,6 +16,18 @@ PenNode types (the ONLY format you output for designs): - text: Props: content, fontFamily, fontSize, fontWeight, fontStyle ('normal'|'italic'), fill, width, height, textAlign ('left'|'center'|'right'|'justify' — NEVER 'start'/'end'; those are container-axis values), textGrowth ('auto'|'fixed-width'|'fixed-width-height'), lineHeight (multiplier), letterSpacing (px), textAlignVertical ('top'|'middle'|'bottom') - path: SVG icon. Props: d (SVG path), width, height, fill, stroke, effects - image: Props: width, height, cornerRadius, effects, imageSearchQuery (2-3 English keywords UNIQUE per image — derive from the surrounding card/dish/title text; reusing one query across multiple images makes every card render the same photo. For food cards, use prepared-dish queries like "pasta plate", "salmon bowl", "pizza plate", "sushi platter"; avoid ingredient-only, outdoor/grass, raw-object, or novelty queries), imagePrompt (a fuller natural-language description of the SAME subject for AI image generation — e.g. "professional food photography of a pasta plate, warm natural light, shallow depth of field". ALWAYS emit it alongside imageSearchQuery: a configured image-gen model uses imagePrompt for a rich original image, otherwise imageSearchQuery drives the stock-search fallback — so every image element carries both) +- text_input: First-class single-line control. Props: width, height, placeholder, value, leadingIcon, trailingIcon, fill, stroke, cornerRadius, effects +- text_area: First-class multi-line control. Props: width, height, placeholder, value, maxVisibleLines, leadingIcon, trailingIcon, fill, stroke, cornerRadius, effects +- select: First-class dropdown. Props: width, height, options: [{value,label}], value, placeholder, fill, stroke, cornerRadius, effects +- switch: First-class toggle. Props: width, height, checked, fill, stroke, cornerRadius, effects +- checkbox: First-class checkbox. Props: width, height, checked, label, fill, stroke, cornerRadius, effects +- slider: First-class range control. Props: width, height, min, max, step, value, fill, stroke, cornerRadius, effects +- radio_group: First-class single-choice control. Props: width, height, options: [{value,label}], value, fill, stroke, cornerRadius, effects +- number_input: First-class numeric control. Props: width, height, placeholder, min, max, step, value, fill, stroke, cornerRadius, effects +- progress: First-class display control. Props: width, height, max, value, indeterminate, fill, stroke, cornerRadius, effects +- tabs: First-class tab container. Props: width, height, tabs: [{value,label}], value, children[] (one panel per tab), fill, stroke, cornerRadius, effects + +INTERACTIVE CONTROL CONTRACT: Emit the native types above directly; NEVER build a frame/rectangle/text lookalike or a new role-marked frame. Supply every semantic prop listed for the chosen control (`options`/`tabs` and `value`, `checked`, or `min`/`max`/`step`/`value`) rather than depending on defaults. Every control MUST explicitly carry design-system-derived `fill`, `stroke`, and `cornerRadius`: `fill` is the active/accent paint (or the field surface), while `stroke.fill` is the inactive track/border paint. PROPERTY NAMES are camelCase and are ALWAYS one unbroken identifier. Never split one at a word boundary: `justify.content` ✗ / `justify_content` ✗ / `"justify-content"` ✗ — write `justifyContent` ✓. Same for `alignItems`, `cornerRadius`, `fontSize`, `lineHeight`, `clipContent`, `imageSearchQuery`. A dotted key is a syntax error that throws away the entire script, not just that one property. All nodes share: id, type, name, role, x, y, rotation, opacity diff --git a/crates/op-ai-skills/src/lib.rs b/crates/op-ai-skills/src/lib.rs index 2fb9fe166..4f743843e 100644 --- a/crates/op-ai-skills/src/lib.rs +++ b/crates/op-ai-skills/src/lib.rs @@ -148,21 +148,35 @@ pub fn guideline_topics() -> Vec<&'static str> { GUIDELINE_TOPICS.iter().map(|(name, _, _)| *name).collect() } +const JIAN_COMPONENTS_PLACEHOLDER: &str = "{{jianComponents}}"; + /// Return the system prompt for the design agentic tool-loop. /// -/// The prompt is embedded at compile time from -/// `skills/phases/agent/design-agent.md` and returned verbatim — no -/// frontmatter stripping, no template substitution. Callers (e.g. the -/// `BuiltInProvider` `QueryLoop`) pass it directly as the system message -/// for a design turn. +/// The protocol template is embedded from +/// `skills/phases/agent/design-agent.md`. Its native-widget placeholder is +/// expanded from the same `jian-components` generation skill used by the +/// single-shot pipeline, so builtin turns and spawned design sub-agents cannot +/// drift onto a second interactive-control contract. /// /// Panics at startup if the embedded file is missing or not valid UTF-8 /// (a build-time invariant: the file is checked in alongside this crate). pub fn design_agent_system_prompt() -> &'static str { - SKILLS - .get_file("phases/agent/design-agent.md") - .and_then(|f| f.contents_utf8()) - .expect("skills/phases/agent/design-agent.md must be embedded in the op-ai-skills corpus") + static PROMPT: std::sync::OnceLock = std::sync::OnceLock::new(); + PROMPT.get_or_init(|| { + let template = SKILLS + .get_file("phases/agent/design-agent.md") + .and_then(|f| f.contents_utf8()) + .expect( + "skills/phases/agent/design-agent.md must be embedded in the op-ai-skills corpus", + ); + assert!( + template.contains(JIAN_COMPONENTS_PLACEHOLDER), + "design-agent.md must mount the shared jian-components contract" + ); + let widgets = get_skill_by_name("jian-components") + .expect("jian-components must be registered for the design-agent prompt"); + template.replace(JIAN_COMPONENTS_PLACEHOLDER, widgets.content.trim()) + }) } /// Return the design-agent tool-loop system prompt with the prompt-matched @@ -182,9 +196,12 @@ pub fn design_agent_system_prompt() -> &'static str { /// /// Output-protocol skills (schema / layout / text-rules / codegen-* / …) /// are deliberately NOT appended: the loop's protocol is the tool loop -/// itself, and the model can still pull topic guides on demand via -/// `get_guidelines`. Budget trimming is the resolver's (per-skill budgets + -/// phase cap), so a keyword-rich prompt cannot balloon the system prompt. +/// itself. The one shared protocol dependency, `jian-components`, is already +/// mounted inside [`design_agent_system_prompt`] because native widget syntax +/// must be identical across both generation paths. The model can still pull +/// topic guides on demand via `get_guidelines`. Budget trimming is the +/// resolver's (per-skill budgets + phase cap), so a keyword-rich prompt cannot +/// balloon the system prompt. pub fn design_agent_system_prompt_with_skills(user_message: &str) -> String { let base = design_agent_system_prompt(); let ctx = resolve::resolve_skills( @@ -307,6 +324,65 @@ mod tests { ); } + #[test] + fn builtin_design_loop_prompt_mounts_the_shared_first_class_widget_contract() { + let widgets = get_skill_by_name("jian-components") + .expect("jian-components must be registered") + .content + .trim(); + let base = design_agent_system_prompt(); + let builtin = design_agent_system_prompt_with_skills( + "Continue this mobile app with an interactive settings screen", + ); + + assert!( + base.contains(widgets), + "bare tool-loop prompt must mount the exact shared generation contract" + ); + assert!( + builtin.contains(widgets), + "the prompt passed to builtin design turns must retain the shared contract" + ); + assert!(!base.contains(JIAN_COMPONENTS_PLACEHOLDER)); + assert_eq!( + builtin.matches("FIRST-CLASS OUTPUT").count(), + 1, + "the builtin prompt must mount one authoritative widget contract" + ); + for kind in [ + "text_input", + "text_area", + "select", + "switch", + "checkbox", + "slider", + "radio_group", + "number_input", + "progress", + "tabs", + ] { + assert!( + builtin.contains(kind), + "builtin design loop must receive first-class widget `{kind}`" + ); + } + for contract in [ + "options: [{value,label}]", + "`checked`", + "`min`, `max`, `step`, and `value`", + "MUST explicitly carry `fill`, `stroke`, and", + "`cornerRadius`", + "`fill` is the active/accent paint", + "`stroke.fill` is the inactive track/border paint", + "LEGACY COMPATIBILITY ONLY", + ] { + assert!( + builtin.contains(contract), + "builtin design loop lost native-widget contract {contract:?}" + ); + } + } + #[test] fn design_agent_base_prompt_keeps_final_hug_height_invariant() { let prompt = design_agent_system_prompt(); diff --git a/crates/op-ai-skills/src/loader.rs b/crates/op-ai-skills/src/loader.rs index 19dea5bf0..a4240a086 100644 --- a/crates/op-ai-skills/src/loader.rs +++ b/crates/op-ai-skills/src/loader.rs @@ -113,17 +113,31 @@ mod tests { } #[test] - fn generation_format_emits_text_input_widgets() { - // Phase 1: form fields must be generated as real `text_input` - // nodes (interactive in preview), not `role=input` mockup frames. - // The generation schema's node-type list must mention text_input. - let mentions = get_skills_by_phase(Phase::Generation) - .iter() - .any(|s| s.content.contains("text_input")); - assert!( - mentions, - "the generation format must list text_input as an emittable node" - ); + fn generation_schema_lists_every_first_class_widget() { + let skill = get_skill_by_name("schema").expect("schema skill must be registered"); + for kind in [ + "text_input", + "text_area", + "select", + "switch", + "checkbox", + "slider", + "radio_group", + "number_input", + "progress", + "tabs", + ] { + assert!( + skill.content.contains(&format!("- {kind}:")), + "generation schema must list first-class widget `{kind}`" + ); + } + assert!(skill + .content + .contains("Emit the native types above directly")); + assert!(skill.content.contains("design-system-derived `fill`")); + assert!(skill.content.contains("`stroke.fill`")); + assert!(skill.content.contains("`cornerRadius`")); } #[test] @@ -253,7 +267,7 @@ mod tests { } #[test] - fn jian_components_skill_loads_as_always_base_and_teaches_role_vocab() { + fn jian_components_skill_teaches_native_widgets_with_legacy_role_compatibility() { let skill = get_skill_by_name("jian-components").expect("jian-components skill must be registered"); // Frontmatter contract (Component 8a): Base category, always-considered. @@ -261,18 +275,47 @@ mod tests { assert_eq!(skill.meta.priority, 5); assert!(matches!(skill.meta.trigger, SkillTrigger::Always)); assert!(skill.meta.phase.contains(&Phase::Generation)); - // Lockstep with jian's promote table: every role string the promote - // pass honours must be taught by the skill. Exported from jian so - // this test fails the moment the table grows or shrinks without a - // doc update, instead of trusting a hand-copied list that can drift. + for kind in [ + "text_input", + "text_area", + "select", + "switch", + "checkbox", + "slider", + "radio_group", + "number_input", + "progress", + "tabs", + ] { + assert!( + skill.content.contains(kind), + "jian-components must teach native widget `{kind}`" + ); + } + for required in [ + "options: [{value,label}]", + "`checked`", + "`min`, `max`, `step`, and `value`", + "MUST explicitly carry `fill`, `stroke`, and", + "`cornerRadius`", + "`fill` is the active/accent paint", + "`stroke.fill` is the inactive track/border paint", + ] { + assert!( + skill.content.contains(required), + "jian-components lost required native-widget contract {required:?}" + ); + } + assert!(skill.content.contains("LEGACY COMPATIBILITY ONLY")); + assert!(skill.content.contains("NEVER choose that representation")); + + // Promotion remains accepted only as a compatibility dialect. Keep the + // documented aliases in lockstep with jian's legacy promote table. for role in jian_ops_schema::promote::promotable_roles() { assert!( skill.content.contains(role), - "jian-components must teach role marker `{role}`" + "jian-components must document legacy role marker `{role}`" ); } - // Must teach the child-structure promote_frame extracts. - assert!(skill.content.contains("placeholder")); - assert!(skill.content.contains("leading")); } } diff --git a/crates/op-orchestrator/src/parse.rs b/crates/op-orchestrator/src/parse.rs index 0f9d59649..a8d1ed604 100644 --- a/crates/op-orchestrator/src/parse.rs +++ b/crates/op-orchestrator/src/parse.rs @@ -134,6 +134,15 @@ const NODE_KINDS: &[&str] = &[ "path", "text", "text_input", + "text_area", + "select", + "switch", + "checkbox", + "slider", + "radio_group", + "number_input", + "progress", + "tabs", "image", "icon_font", "ref", diff --git a/crates/op-orchestrator/src/parse_tests.rs b/crates/op-orchestrator/src/parse_tests.rs index 6d6b3409f..020c3df06 100644 --- a/crates/op-orchestrator/src/parse_tests.rs +++ b/crates/op-orchestrator/src/parse_tests.rs @@ -94,6 +94,37 @@ fn parse_nodes_reads_jsonl_bare_objects() { assert_eq!(nodes[1].id_str(), "b"); } +#[test] +fn parse_nodes_accepts_every_first_class_widget_in_flat_jsonl() { + let text = r#" +{"type":"frame","id":"root","layout":"vertical","_parent":null} +{"type":"text_input","id":"input","value":"","placeholder":"Search","_parent":"root"} +{"type":"text_area","id":"area","value":"","placeholder":"Notes","_parent":"root"} +{"type":"select","id":"select","options":[{"value":"a","label":"A"}],"value":"a","_parent":"root"} +{"type":"switch","id":"switch","checked":true,"_parent":"root"} +{"type":"checkbox","id":"check","checked":false,"label":"Alerts","_parent":"root"} +{"type":"slider","id":"slider","min":0,"max":10,"step":1,"value":4,"_parent":"root"} +{"type":"radio_group","id":"radio","options":[{"value":"a","label":"A"}],"value":"a","_parent":"root"} +{"type":"number_input","id":"number","min":0,"max":10,"step":1,"value":4,"_parent":"root"} +{"type":"progress","id":"progress","max":100,"value":40,"_parent":"root"} +{"type":"tabs","id":"tabs","tabs":[{"value":"a","label":"A"}],"value":"a","_parent":"root"} +"#; + + let nodes = parse_nodes(text).expect("all native widget kinds must survive JSONL filtering"); + let children = nodes[0].children().expect("widget children"); + assert_eq!(children.len(), 10); + assert!(matches!(&children[0], PenNode::TextInput(_))); + assert!(matches!(&children[1], PenNode::TextArea(_))); + assert!(matches!(&children[2], PenNode::Select(_))); + assert!(matches!(&children[3], PenNode::Switch(_))); + assert!(matches!(&children[4], PenNode::Checkbox(_))); + assert!(matches!(&children[5], PenNode::Slider(_))); + assert!(matches!(&children[6], PenNode::RadioGroup(_))); + assert!(matches!(&children[7], PenNode::NumberInput(_))); + assert!(matches!(&children[8], PenNode::Progress(_))); + assert!(matches!(&children[9], PenNode::Tabs(_))); +} + #[test] fn parse_nodes_bare_object_returns_top_level_not_nested_children() { // 单个裸对象(无数组包裹)带真实嵌套 children。合并深度判定下, diff --git a/crates/op-orchestrator/src/prompt.rs b/crates/op-orchestrator/src/prompt.rs index 694bd1669..801d6f1c2 100644 --- a/crates/op-orchestrator/src/prompt.rs +++ b/crates/op-orchestrator/src/prompt.rs @@ -23,9 +23,7 @@ use crate::timeouts::{ }; use crate::types::{AbortFlag, CallRequest, DesignRequest, PlanningMode, PlanningPrompt}; use op_ai_skills::resolve_style::{resolve_style, ResolveOutcome}; -use op_ai_skills::style_guide::{ - extract_style_guide_values, select_style_guide, style_guide_registry, SelectOptions, -}; +use op_ai_skills::style_guide::extract_style_guide_values; use op_ai_skills::{ budget::trim_by_budget_pinned, get_skills_by_phase, @@ -58,7 +56,8 @@ pub use prompt_subagent::*; const NODE_FORMAT: &str = r#" Respond with THIS section's canonical PenNode objects in the FLAT _parent format: output ONE JSON object per line (NO enclosing [ ] array), each tagged by "type" -(frame/group/rectangle/ellipse/line/polygon/path/text/text_input/image/icon_font) +(frame/group/rectangle/ellipse/line/polygon/path/text/text_input/text_area/select/ +switch/checkbox/slider/radio_group/number_input/progress/tabs/image/icon_font) and carrying "_parent" — null for the section root, else the id of its parent node (which MUST appear on an earlier line). EVERY non-root node MUST set "_parent". Do NOT emit a flat list of siblings with @@ -66,6 +65,13 @@ no _parent links, and do NOT rely on a "children" array — a flat list renders BROKEN: a horizontal row whose items are not _parent-linked to it collapses into a vertical stack. Express the WHOLE tree through _parent (row -> its cards -> each card's texts/icons). +Interactive controls MUST be first-class nodes. Emit text_input/text_area with value; +select/radio_group with options:[{value,label}] and value; switch/checkbox with checked; +slider/number_input with min/max/step/value; progress with max/value; tabs with +tabs:[{value,label}] and value. Never generate a frame/rectangle mockup with a role marker. +Every interactive node MUST explicitly carry design-system fill, stroke, and cornerRadius. +fill is the active/accent paint (or field surface). +stroke.fill is the inactive track/border paint. Do not rely on renderer defaults. Example (a horizontal row of two cards inside a section): {"_parent":null,"id":"-root","type":"frame","name":"Section","width":"fill_container","height":"fit_content","layout":"vertical","gap":16} {"_parent":"-root","id":"-row","type":"frame","name":"Row","width":"fill_container","height":"fit_content","layout":"horizontal","gap":16} @@ -97,9 +103,18 @@ not call console.log or any helper; just call I(...). USE REAL JAVASCRIPT — const/let, arrays of data, and for...of / .forEach loops — to generate repeated structure (table rows, nav items, cards, list items) by looping over a data array. PREFER a loop over copy-pasting near-identical I(...) calls. -Each node object starts with type ("frame"/"text"/"rectangle"/"ellipse"/"path"/"icon_font") -and uses camelCase props (cornerRadius, fontSize, fontWeight, justifyContent, alignItems, -clipContent). Do NOT set x/y on children inside layout frames. +Each node object starts with type ("frame"/"text"/"rectangle"/"ellipse"/"path"/ +"icon_font"/"text_input"/"text_area"/"select"/"switch"/"checkbox"/"slider"/ +"radio_group"/"number_input"/"progress"/"tabs") and uses camelCase props +(cornerRadius, fontSize, fontWeight, justifyContent, alignItems, clipContent). Do NOT set +x/y on children inside layout frames. +INTERACTIVE CONTROLS are native nodes: emit the first-class type directly with I(...), +never a frame/rectangle mockup with a role marker. text_input/text_area require value; +select/radio_group require options:[{value,label}] plus value; switch/checkbox require +checked; slider/number_input require min/max/step/value; progress requires max/value; +tabs requires tabs:[{value,label}] plus value. Every native control MUST explicitly carry +the design system's fill, stroke, and cornerRadius. fill is the active/accent paint (or +field surface); stroke.fill is the inactive track/border paint. Do not rely on defaults. Each script runs in a FRESH sandbox: variables from an EARLIER batch do not exist. To attach to a node an earlier batch created, pass its id STRING — I("n12", {...}) — never a `const` from that batch. Ids come back in the batch result. diff --git a/crates/op-orchestrator/src/subagent_run_subtask_tests.rs b/crates/op-orchestrator/src/subagent_run_subtask_tests.rs index ce58a2876..48c7a6a2a 100644 --- a/crates/op-orchestrator/src/subagent_run_subtask_tests.rs +++ b/crates/op-orchestrator/src/subagent_run_subtask_tests.rs @@ -489,3 +489,84 @@ fn run_subtask_promotes_role_input_frame_to_text_input() { assert_eq!(ti.leading_icon.as_deref(), Some("mail")); assert_eq!(ti.placeholder.as_deref(), Some("Email address")); } + +/// First-class widgets emitted by classic script-gen must survive the +/// orchestrator's role/post-pass/final insert path without losing either their +/// authored surface style or the interaction data needed by Preview. +#[test] +fn run_subtask_preserves_first_class_widget_style_and_interaction_props() { + let llm_script = r##"I(null, { + "type":"frame","name":"Interactive Controls","width":1200,"height":320, + "layout":"vertical","children":[ + {"type":"select","name":"Observatory","width":320,"height":48, + "fill":[{"type":"solid","color":"#2A1645"}], + "stroke":{"thickness":2,"fill":[{"type":"solid","color":"#A855F7"}]}, + "cornerRadius":12,"value":"shanghai","options":[ + {"value":"shanghai","label":"Shanghai Observatory"}, + {"value":"beijing","label":"Beijing Observatory"} + ]}, + {"type":"switch","name":"Night Vision","width":48,"height":28, + "fill":[{"type":"solid","color":"#9333EA"}], + "stroke":{"thickness":1,"fill":[{"type":"solid","color":"#C084FC"}]}, + "cornerRadius":14,"checked":true}, + {"type":"slider","name":"Magnitude","width":320,"height":24, + "fill":[{"type":"solid","color":"#7C3AED"}], + "stroke":{"thickness":1,"fill":[{"type":"solid","color":"#E9D5FF"}]}, + "cornerRadius":8,"min":0,"max":6.5,"step":0.5,"value":5.5} + ] + });"##; + let llm = ScriptedLlm::new(vec![ScriptResponse::Text(llm_script.into())]); + let mut sink = VecDocSink::new(); + let outcome = block_on(run_subtask( + &subtask(), + &plan(), + &req(), + &llm, + &mut sink, + &AbortFlag::new(), + false, + false, + )); + + assert!( + outcome.error.is_none(), + "unexpected error: {:?}", + outcome.error + ); + let Some(EditorCommand::InsertSubtree { nodes, .. }) = sink.applied.last() else { + panic!("expected InsertSubtree, got {:?}", sink.applied.last()); + }; + let PenNode::Frame(section) = &nodes[0] else { + panic!("outer section must remain a frame"); + }; + let children = section.children.as_ref().expect("section has controls"); + assert_eq!(children.len(), 3); + + let select = serde_json::to_value(&children[0]).expect("serialize select"); + assert_eq!(select["type"], "select"); + assert_eq!(select["fill"][0]["color"], "#2A1645"); + assert_eq!(select["stroke"]["thickness"].as_f64(), Some(2.0)); + assert_eq!(select["stroke"]["fill"][0]["color"], "#A855F7"); + assert_eq!(select["cornerRadius"].as_f64(), Some(12.0)); + assert_eq!(select["value"], "shanghai"); + assert_eq!(select["options"].as_array().map(Vec::len), Some(2)); + + let switch = serde_json::to_value(&children[1]).expect("serialize switch"); + assert_eq!(switch["type"], "switch"); + assert_eq!(switch["fill"][0]["color"], "#9333EA"); + assert_eq!(switch["stroke"]["thickness"].as_f64(), Some(1.0)); + assert_eq!(switch["stroke"]["fill"][0]["color"], "#C084FC"); + assert_eq!(switch["cornerRadius"].as_f64(), Some(14.0)); + assert_eq!(switch["checked"], true); + + let slider = serde_json::to_value(&children[2]).expect("serialize slider"); + assert_eq!(slider["type"], "slider"); + assert_eq!(slider["fill"][0]["color"], "#7C3AED"); + assert_eq!(slider["stroke"]["thickness"].as_f64(), Some(1.0)); + assert_eq!(slider["stroke"]["fill"][0]["color"], "#E9D5FF"); + assert_eq!(slider["cornerRadius"].as_f64(), Some(8.0)); + assert_eq!(slider["min"].as_f64(), Some(0.0)); + assert_eq!(slider["max"].as_f64(), Some(6.5)); + assert_eq!(slider["step"].as_f64(), Some(0.5)); + assert_eq!(slider["value"].as_f64(), Some(5.5)); +}