diff --git a/crates/op-ai-skills/skills/phases/generation/schema.md b/crates/op-ai-skills/skills/phases/generation/schema.md index 3d1159f5d..299b8ed43 100644 --- a/crates/op-ai-skills/skills/phases/generation/schema.md +++ b/crates/op-ai-skills/skills/phases/generation/schema.md @@ -17,6 +17,7 @@ PenNode types (the ONLY format you output for designs): - 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) +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 Interactivity (multi-screen apps only): frame accepts `screen` (a top-level frame's route path — `"/"` for the entry screen, `"/slug"` for every other screen, unique across the document) and `events` (`{ onTap: [ {"replace": "\"/path\""} | {"push": "\"/path\""} | {"pop": null} ] }` — the action body is the literal JSON STRING `"\"/path\""`, quote characters included, since it compiles as an expression; a bare `/path` fails). Never write `route` — it is schema-only metadata the tap dispatcher ignores. Fill = [{ type: "solid", color: "#hex" }] or [{ type: "linear_gradient", angle, stops: [{ offset, color }] }] or [{ type: "radial_gradient", stops: [{ offset, color }] }] or [{ type: "mesh_gradient", rows, cols, stops: [{ row, col, color }] }] diff --git a/crates/op-ai-skills/skills/phases/planning/decomposition.md b/crates/op-ai-skills/skills/phases/planning/decomposition.md index ac4764cd1..265246822 100644 --- a/crates/op-ai-skills/skills/phases/planning/decomposition.md +++ b/crates/op-ai-skills/skills/phases/planning/decomposition.md @@ -44,13 +44,14 @@ OTHERWISE classify by purpose: - Every subtask MUST carry a `screen` field naming that slide, and no two may share a value. This field is what makes each slide its own frame; without it the whole deck collapses onto one board. - Each subtask's region is the full slide: {"width":1920,"height":1080} - Structure per slide: one idea — a takeaway title plus its supporting content. NO status bar, NO navigation bar, NO footer. - - Honour an explicitly requested slide count ("6 页"/"5-slide"); otherwise plan 5-8 slides. - - OUTLINE MODE — pick the running order from the deck's PURPOSE, then name each subtask after the slide it produces (that name is the board's title, so it must read as a slide title, not as "Section 3"): - - Pitch / 路演 / 融资: cover - the problem (3 pains) - the solution - proof data (3 KPIs) - milestones/roadmap - the ask + contact. - - Lecture / 课件 / 培训: cover - learning objectives - the concept - a worked example (numbered steps) - a comparison of the two things students confuse - summary + homework. - - Report / 汇报 / 季度: cover - agenda - what we did (3 points) - results (3 KPIs) - a trend chart with its takeaway - next steps. - - Product launch / 发布: cover - the change in the market - the product - three capabilities - evidence/benchmark - availability + CTA. - - Anything else: cover - agenda - 2-4 argument slides - one evidence slide - closing. + - SLIDE COUNT — from the material, never from a template. An explicit count ("6 页", "12 slides", "不超过 8 页") is a HARD constraint: emit exactly that many slide subtasks. A range ("8-10 页") — pick a value inside it. No count given — size it to the subject: one concept / 单个知识点 5-8, 汇报 / report 8-12, 路演 / pitch 8-12, a multi-chapter 课件 / training course 12-20, a one-idea keynote 3-6. A multi-part subject goes LONGER — 6 slides for a 5-chapter syllabus is a failure, not a summary. Never pad to a number, never drop material to fit one. + - OUTLINE MODE — pick the running order from the deck's PURPOSE, then name each subtask after the slide it produces (that name is the board's title, so it must read as a slide title, not as "Section 3"). These are running ORDERS, not lengths — expand any of them: split one step across slides, add a section break, give an item its own slide. + - Pitch / 路演 / 融资: cover - the problem (3 pains) - why now - the solution - how it works - proof data (3 KPIs) - traction/milestones - roadmap - the ask + contact. + - Lecture / 课件 / 培训: cover - learning objectives - the concept - a worked example (one slide PER substantive step) - a comparison of the two things students confuse - common mistakes - summary + homework. + - Report / 汇报 / 季度: cover - agenda - what we did (one slide per workstream) - results (3 KPIs) - a trend chart with its takeaway - what missed and why - next steps. + - Product launch / 发布: cover - the change in the market - the product - capabilities (one slide each when one needs its own visual) - evidence/benchmark - availability + CTA. + - Anything else: cover - agenda - one slide per argument the subject has - one evidence slide - closing. + - Worked counts: "讲解快速排序的课件" (one algorithm) plans 6; "2026 Q3 增长复盘汇报" (four workstreams with data) plans 11. Copy the METHOD, not either number. - COPY LIMITS (a slide is a visual aid, not a document — plan the amount, do not leave it to the generator): slide title <= 14 CJK chars / ~10 English words. Bullet or card item <= 20 CJK chars / ~14 English words. Total body copy per slide <= 80 CJK chars / ~55 English words. At most 3 cards, 3 KPIs, 5 timeline nodes, or 5 bullets on one slide — if the content needs more, plan another slide instead. - The "elements" field for a slide names the slide's ONE takeaway plus its supporting parts (e.g. "takeaway title, 3 KPI cards each with value + unit + label + note"), never a list of paragraphs. @@ -82,7 +83,6 @@ RULES: - If a section is about "App截图"/"XX截图"/"screenshot"/"mockup", plan it as a phone mockup placeholder block, not a detailed mini-app reconstruction. - For landing pages: navigation sections should preserve good horizontal balance, links evenly distributed in the center group. - Regions tile to fill rootFrame. vertical = top-to-bottom. -- Mobile default: 375x812 (both width AND height are fixed). Desktop default: 1200x0 (width fixed, height auto-expands). Deck: 1920x1080 (both fixed). - WIDTH SELECTION: Presentation decks (type 4) are ALWAYS width=1920, height=1080 — do not derive a 16:9 box from the 1200 desktop default. Type 0 components default to width=400, height=0. Type 2 single-task SCREENS (login screen, profile page, settings page) default to width=375, height=812 (mobile). Multi-section pages and data-rich workspaces (types 1 & 3) default to width=1200, height=0 (desktop). A "profile card" is Type 0 (width=400), NOT Type 2. An explicit user-requested root width or width×height pair overrides every default and must be copied exactly into rootFrame. - MULTI-SCREEN APPS: When the request involves multiple distinct screens/pages (e.g. "登录页+个人中心", "login and profile", "continue generating the remaining 3 pages"), add "screen":"" to EVERY subtask. Each DISTINCT "screen" value becomes its OWN top-level root frame, placed as a separate sibling screen on the canvas; subtasks sharing the same "screen" value land together in that one root. Tagging is mandatory to get separate screens — if you omit "screen" (or give every subtask the SAME value), all subtasks collapse into a single shared root frame, even when the request clearly asked for multiple distinct pages. This applies starting at just 2-3 distinct screens — do NOT wait for a larger count before tagging; a separate skill's guidance about when to hand a screen off to a sub-agent (parallel delegation) is a DIFFERENT decision with its own higher threshold and has no bearing on whether you tag "screen" here. Use a concise page name per screen (e.g. "登录", "Profile") — it becomes that root frame's name. Single-screen requests don't need "screen" at all. Example (2 screens, "Login" then "Profile"): [{"id":"brand","label":"Brand Area","screen":"Login","region":{...}},{"id":"form","label":"Login Form","screen":"Login","region":{...}},{"id":"card","label":"User Card","screen":"Profile","region":{...}}] - SHARED CHROME ACROSS SCREENS: only plan a FULL bottom-nav/sidebar subtask for the FIRST screen. For every screen after that, either omit the nav subtask entirely or plan it as a minimal placeholder (no need to invent its own icon/label set) — a deterministic pass copies the first screen's nav onto every screen whose name matches one of that nav's tabs (even a screen with NO nav content at all, e.g. its own nav subtask failed) and fixes up which tab is active, so re-planning a full nav per screen wastes subtasks on content that gets replaced anyway. A screen whose name matches none of the nav's tabs (a standalone detail view, say) is correctly left alone — give it its own nav subtask only if it genuinely needs different chrome. diff --git a/crates/op-ai-skills/src/deck_corpus_tests.rs b/crates/op-ai-skills/src/deck_corpus_tests.rs index a549ab860..f05fefb19 100644 --- a/crates/op-ai-skills/src/deck_corpus_tests.rs +++ b/crates/op-ai-skills/src/deck_corpus_tests.rs @@ -169,6 +169,61 @@ fn decomposition_carries_the_deck_outline_templates_and_copy_caps() { assert!(body.contains("slide title <= 14 CJK chars")); } +/// A deck used to come back as six slides no matter what was asked, and the +/// cause was here rather than in any cap: every OUTLINE MODE recipe listed +/// exactly six steps, so a planner copying the shape copied the length too, +/// and the only count guidance ("otherwise plan 5-8 slides") sat below them. +/// The corpus must state a count RULE and must not re-converge the outlines +/// onto one length. +#[test] +fn decomposition_derives_slide_count_from_the_material_not_the_outline_length() { + let body = &get_skill_by_name("decomposition") + .expect("decomposition registered") + .content; + + assert!(body.contains("SLIDE COUNT"), "count rules must be stated"); + assert!( + body.contains("HARD constraint"), + "an explicitly requested count must be taught as binding, not advisory" + ); + for range in ["5-8", "8-12", "12-20", "3-6"] { + assert!( + body.contains(range), + "count must be sized per deck kind; missing the {range} band" + ); + } + + // Structural anti-anchor: the running orders must not all be the same + // length again. Each outline line is `- : a - b - c.`, so the step + // count is the number of ` - ` separators plus one. + let step_counts: Vec = body + .lines() + .map(str::trim) + .filter(|line| { + line.starts_with("- ") + && line.ends_with('.') + && line.contains("cover -") + && line.contains(':') + }) + .map(|line| line.matches(" - ").count() + 1) + .collect(); + assert!( + step_counts.len() >= 4, + "expected the per-purpose outlines to be parseable, found {step_counts:?}" + ); + assert!( + step_counts.iter().any(|&n| n != step_counts[0]), + "every outline is {} steps long again — that uniform length IS the \ + page-count anchor this guard exists to prevent: {step_counts:?}", + step_counts[0] + ); + assert!( + step_counts.iter().any(|&n| n > 6), + "no outline runs past six steps, so copying one still caps the deck at \ + the old default: {step_counts:?}" + ); +} + #[test] fn every_deck_prompt_resolves_both_deck_skills_untruncated() { // The generation-phase total is what actually decides this; these prompts diff --git a/crates/op-mcp/src/script_runner.rs b/crates/op-mcp/src/script_runner.rs index 83b4459d7..8952179a9 100644 --- a/crates/op-mcp/src/script_runner.rs +++ b/crates/op-mcp/src/script_runner.rs @@ -16,6 +16,9 @@ use rquickjs::{Context, Function, Runtime}; #[path = "script_runner_error.rs"] mod error; +#[path = "script_runner_dotted_keys.rs"] +mod dotted_keys; + pub use error::ScriptError; pub const MAX_SCRIPT_BYTES: usize = 262_144; @@ -96,6 +99,28 @@ fn truncate_duplicate_script(script: &str) -> Option { } fn eval_after_initial_failure(script: &str, first_err: ScriptError) -> Result { + // gemini-3.6-flash writes schema property names with the separator it + // reads in the docs — `justify.content:` instead of `justifyContent:`. + // A bare dotted key is a SyntaxError at the first `.`, so QuickJS + // rejected an otherwise-correct slide before recording one `I(...)`. + // Normalize FIRST, then let the rest of the ladder work on the repaired + // source: a script can be both mis-keyed and truncated. + let script = match dotted_keys::repair_dotted_object_keys(script) { + Some(repaired) => match eval_to_program(&repaired) { + Ok(p) => { + tracing::warn!( + original_len = script.len(), + repaired_len = repaired.len(), + "script failed as-is; dotted-property-key repair recovered a runnable source" + ); + return Ok(p); + } + Err(_) => repaired, + }, + None => script.to_string(), + }; + let script = script.as_str(); + // GLM-5.2 commonly drops the outer `}` when `stroke:{...}` is the final // property of an I() object, so QuickJS reaches `)` with `{` still open. let balanced = balance_brackets(script); diff --git a/crates/op-mcp/src/script_runner_dotted_keys.rs b/crates/op-mcp/src/script_runner_dotted_keys.rs new file mode 100644 index 000000000..ba137bcbc --- /dev/null +++ b/crates/op-mcp/src/script_runner_dotted_keys.rs @@ -0,0 +1,244 @@ +//! Repair for dotted property keys in an object literal — `justify.content:` +//! where the schema means `justifyContent:`. +//! +//! Measured 2026-08-05 (gemini-3.6-flash, script-gen subagent): the model +//! wrote `justify.content: "space_between"` in three places of an otherwise +//! correct slide script. A bare dotted key is a `SyntaxError` at the first +//! `.`, so QuickJS rejected the file before recording a single `I(...)` call +//! and the whole page was lost to a retry — a typo in three lines cost every +//! node on the board. +//! +//! Two properties keep this narrow enough to be safe: +//! +//! 1. **It only runs after the script has already failed to eval.** A script +//! that runs is never rewritten, so this pass cannot regress a working +//! generation. +//! 2. **The pattern it matches is never valid JavaScript.** A key position is +//! "the token right after `{` or `,`", and JS has no bare dotted key there +//! — `{ a.b: 1 }` does not parse under any interpretation. So a rewrite +//! cannot change the meaning of a program that had one. +//! +//! Everything else is left alone by construction: the scan tracks string and +//! comment state, so `"a.b: c"` inside a literal, a URL in a `src`, and a +//! template literal's body are all invisible to it. Member access on the +//! right-hand side (`item.color + "40"`, `obj.prop = x`) never sits in a key +//! position, and `cond ? a.b : c` / `case a.b:` fail the preceding-token test. + +/// Rewrite dotted object-literal keys to camelCase (`justify.content:` → +/// `justifyContent:`). Returns `None` when the source has none. +pub(crate) fn repair_dotted_object_keys(script: &str) -> Option { + let chars: Vec = script.chars().collect(); + let mut out = String::with_capacity(script.len()); + let mut i = 0usize; + // The last non-whitespace character seen in code (not string / comment) + // context. `{` or `,` means the next token is an object-literal key. + let mut prev_code: Option = None; + let mut repaired = false; + + while i < chars.len() { + let c = chars[i]; + + // ── string + comment skipping (verbatim copy, no rewriting inside) ── + if c == '/' && i + 1 < chars.len() && chars[i + 1] == '/' { + while i < chars.len() && chars[i] != '\n' { + out.push(chars[i]); + i += 1; + } + continue; + } + if c == '/' && i + 1 < chars.len() && chars[i + 1] == '*' { + out.push(chars[i]); + out.push(chars[i + 1]); + i += 2; + while i < chars.len() + && !(chars[i] == '*' && i + 1 < chars.len() && chars[i + 1] == '/') + { + out.push(chars[i]); + i += 1; + } + continue; + } + if c == '"' || c == '\'' || c == '`' { + out.push(c); + i += 1; + while i < chars.len() { + let s = chars[i]; + out.push(s); + i += 1; + if s == '\\' { + if i < chars.len() { + out.push(chars[i]); + i += 1; + } + continue; + } + if s == c { + break; + } + } + prev_code = Some(c); + continue; + } + + // ── key position: the token right after `{` or `,` ── + if matches!(prev_code, Some('{') | Some(',')) && is_ident_start(c) { + if let Some((end, camel)) = dotted_key_at(&chars, i) { + out.push_str(&camel); + i = end; + prev_code = camel.chars().next_back(); + repaired = true; + continue; + } + } + + out.push(c); + if !c.is_whitespace() { + prev_code = Some(c); + } + i += 1; + } + + repaired.then_some(out) +} + +fn is_ident_start(c: char) -> bool { + c.is_ascii_alphabetic() || c == '_' || c == '$' +} + +fn is_ident_char(c: char) -> bool { + c.is_ascii_alphanumeric() || c == '_' || c == '$' +} + +/// Match `(.)+` followed by optional whitespace and `:` starting +/// at `start`. Returns the index just past the identifier chain (the colon and +/// any whitespace before it are left for the main loop) plus the camelCase +/// join. `None` when the chain has no dot or is not followed by a colon — +/// which is what keeps ordinary member access untouched. +fn dotted_key_at(chars: &[char], start: usize) -> Option<(usize, String)> { + let mut segments: Vec = Vec::new(); + let mut i = start; + loop { + let seg_start = i; + while i < chars.len() && is_ident_char(chars[i]) { + i += 1; + } + if i == seg_start { + return None; + } + segments.push(chars[seg_start..i].iter().collect()); + if i < chars.len() && chars[i] == '.' { + i += 1; + continue; + } + break; + } + if segments.len() < 2 { + return None; + } + + let mut probe = i; + while probe < chars.len() && chars[probe].is_whitespace() { + probe += 1; + } + // A colon is required, and `::` is not a key (nothing in JS produces it, + // but refusing it costs nothing and keeps the match unambiguous). + if chars.get(probe) != Some(&':') || chars.get(probe + 1) == Some(&':') { + return None; + } + + let mut camel = segments[0].clone(); + for segment in &segments[1..] { + let mut cs = segment.chars(); + if let Some(first) = cs.next() { + camel.push(first.to_ascii_uppercase()); + camel.extend(cs); + } + } + Some((i, camel)) +} + +#[cfg(test)] +mod tests { + use super::repair_dotted_object_keys; + + #[test] + fn rewrites_the_measured_gemini_shape() { + let script = + r#"I(null, {type:"frame", justify.content: "space_between", align.items:"center"});"#; + let out = repair_dotted_object_keys(script).expect("dotted keys found"); + assert!(out.contains(r#"justifyContent: "space_between""#), "{out}"); + assert!(out.contains(r#"alignItems:"center""#), "{out}"); + assert!(!out.contains('.'), "no dotted key left: {out}"); + } + + #[test] + fn rewrites_multi_line_and_multi_segment_keys() { + let script = "const a = I(null, {\n corner.radius: 24,\n font.weight.value: 700\n});"; + let out = repair_dotted_object_keys(script).expect("dotted keys found"); + assert!(out.contains("cornerRadius: 24"), "{out}"); + assert!(out.contains("fontWeightValue: 700"), "{out}"); + } + + #[test] + fn leaves_a_clean_script_alone() { + let script = r#"const s = I(null, {type:"frame", justifyContent:"center"});"#; + assert_eq!(repair_dotted_object_keys(script), None); + } + + // ── negative cases: everything below must come back untouched ── + + #[test] + fn ignores_dotted_text_inside_string_literals() { + for script in [ + r#"I(null, {type:"text", text:"justify.content: space_between"});"#, + r#"I(null, {type:"image", src:"https://cdn.test/a.b:8080/x.png"});"#, + "I(null, {type:\"text\", text:`line\n justify.content: center\n`});", + r#"I(null, {type:"text", text:'{ align.items: "x" }'});"#, + ] { + assert_eq!( + repair_dotted_object_keys(script), + None, + "rewrote inside a string: {script}" + ); + } + } + + #[test] + fn ignores_member_access_on_the_right_hand_side() { + for script in [ + r#"I(null, {fill:[{type:"solid", color: item.color + "40"}]});"#, + "obj.prop = 1;\nI(null, {type:\"frame\"});", + r#"const w = theme.sizes.width; I(null, {width: w});"#, + "I(null, {height: cfg.rows.length});", + ] { + assert_eq!( + repair_dotted_object_keys(script), + None, + "rewrote a member access: {script}" + ); + } + } + + #[test] + fn ignores_colons_that_are_not_object_keys() { + for script in [ + "const x = flag ? a.b : c; I(null, {width: x});", + "switch (k) { case a.b: break; }", + "I(null, {name: cond ? item.a : item.b});", + ] { + assert_eq!( + repair_dotted_object_keys(script), + None, + "rewrote a non-key colon: {script}" + ); + } + } + + #[test] + fn ignores_dotted_text_inside_comments() { + let script = "// justify.content: space_between is wrong\n\ + /* align.items: center */\n\ + I(null, {type:\"frame\"});"; + assert_eq!(repair_dotted_object_keys(script), None); + } +} diff --git a/crates/op-mcp/src/script_runner_tests.rs b/crates/op-mcp/src/script_runner_tests.rs index 1206af48a..09dcaee60 100644 --- a/crates/op-mcp/src/script_runner_tests.rs +++ b/crates/op-mcp/src/script_runner_tests.rs @@ -286,3 +286,36 @@ fn glm_missing_outer_brace_repairs_on_eval_failure() { assert_eq!(lines.len(), 1); assert!(lines[0].contains(r#""name":"Profile Button""#)); } + +/// gemini-3.6-flash measured shape (2026-08-05): three `justify.content:` +/// keys in an otherwise valid slide script. A bare dotted key is a +/// SyntaxError, so the whole board used to be lost to a retry. +#[test] +fn dotted_property_keys_repair_on_eval_failure() { + let broken = r##"const slide = I(null, {type:"frame", name:"01 Cover", width:1920, height:1080, layout:"vertical", justify.content:"space_between", align.items:"start"}); +I(slide, {type:"text", content:"Q3 Review", font.size:104});"##; + + let program = run_script_to_program(broken).expect("runner retries with dotted-key repair"); + + let lines: Vec<&str> = program.lines().collect(); + assert_eq!(lines.len(), 2, "both inserts survive: {program}"); + assert!( + lines[0].contains(r#""justifyContent":"space_between""#), + "{program}" + ); + assert!(lines[0].contains(r#""alignItems":"start""#), "{program}"); + assert!(lines[1].contains(r#""fontSize":104"#), "{program}"); +} + +/// The repair must compose with the ladder below it — a script can be both +/// mis-keyed AND missing its closing brace, which is exactly what a model +/// that got the property names wrong tends to also get wrong. +#[test] +fn dotted_keys_and_a_missing_brace_repair_together() { + let broken = r##"const card = I(null, {type:"frame", name:"KPI", corner.radius:24, stroke:{thickness:1, fill:[{type:"solid", color:"#EAD8C8"}]});"##; + + let program = run_script_to_program(broken).expect("both repairs apply"); + + assert_eq!(program.lines().count(), 1, "{program}"); + assert!(program.contains(r#""cornerRadius":24"#), "{program}"); +} diff --git a/crates/op-orchestrator/src/plan.rs b/crates/op-orchestrator/src/plan.rs index d9ba8e28c..15e0e0184 100644 --- a/crates/op-orchestrator/src/plan.rs +++ b/crates/op-orchestrator/src/plan.rs @@ -4,7 +4,7 @@ //! 失败时 `build_fallback_plan` 给一个启发式的可跑 plan(对齐 TS //! `buildFallbackPlanFromPrompt`)。 -use crate::design_type::{detect_design_type, DesignType}; +use crate::design_type::{detect_design_type, DesignType, DesignTypePreset}; use crate::types::DesignRequest; use serde::{Deserialize, Serialize}; @@ -187,6 +187,9 @@ pub fn build_fallback_plan(req: &DesignRequest) -> OrchestratorPlan { const SECTION_HEIGHT: f64 = 360.0; let preset = detect_design_type(&req.prompt); + if preset.type_ == DesignType::Slides { + return build_fallback_deck_plan(req, preset); + } if preset.type_ == DesignType::MobileScreen { let (width, height) = explicit_mobile_size(&req.prompt) .unwrap_or((preset.width, preset.root_height.max(preset.height))); @@ -299,6 +302,154 @@ pub fn build_fallback_plan(req: &DesignRequest) -> OrchestratorPlan { } } +/// Heuristic deck fallback — the shape a deck plan MUST have, built without +/// the planning LLM. +/// +/// Before this branch existed, a deck request whose planning call failed fell +/// through to the generic 1200-wide "1-3 stacked sections" skeleton below: the +/// user asked for a presentation and got one scrolling page. The three things +/// that make a deck a deck are all decided here: +/// +/// 1. every board is the projector-shaped 1920x1080 preset, never 1200x0; +/// 2. ONE subtask per slide, each carrying its OWN `screen` label — that label +/// is what makes `plan_normalize` fan the subtasks into separate root +/// frames instead of collapsing them onto one board; +/// 3. the slide count comes from the request when it states one, and from the +/// prompt's size otherwise — a fallback must not re-introduce the fixed +/// page count the corpus was just de-anchored from. +fn build_fallback_deck_plan(req: &DesignRequest, preset: DesignTypePreset) -> OrchestratorPlan { + let count = explicit_slide_count(&req.prompt).unwrap_or( + // No count in the request: scale with how much the prompt describes, + // the same signal the generic branch uses for its section count. + match req.prompt.chars().count() { + 0..=80 => 5, + 81..=200 => 6, + _ => 8, + }, + ); + + let subtasks = fallback_slide_titles(count) + .into_iter() + .enumerate() + .map(|(i, title)| { + let id = format!("slide-{}", i + 1); + Subtask { + id: id.clone(), + label: title.clone(), + region: Region { + width: preset.width, + height: preset.root_height, + }, + id_prefix: id, + // Left to `plan_normalize`, which rewrites it per screen group. + parent_frame_id: None, + elements: Some(fallback_slide_elements(&title).to_string()), + screen: Some(title), + generated_root_id: None, + existing_section_labels: None, + retry_feedback: None, + } + }) + .collect(); + + OrchestratorPlan { + root_frame: RootFrameSpec { + id: "deck".into(), + name: "Deck".into(), + width: preset.width, + height: preset.root_height, + layout: Some("vertical".into()), + gap: Some(0.0), + padding: Some(0.0), + fill: Some(vec![PlanFill { + kind: "solid".into(), + color: "#FFFFFF".into(), + }]), + }, + subtasks, + style_guide_name: None, + } +} + +/// A generic running order that grows with the count instead of repeating a +/// fixed six-step outline: cover, optional agenda, N key points, optional +/// evidence slide, closing. Every title is distinct, which the `screen` +/// tagging depends on — two slides sharing a label would land on one board. +fn fallback_slide_titles(count: usize) -> Vec { + let mut titles: Vec = vec!["Cover".into()]; + let mut tail: Vec = vec!["Closing".into()]; + if count >= 5 { + tail.insert(0, "Evidence".into()); + } + if count >= 4 { + titles.push("Agenda".into()); + } + let middle = count.saturating_sub(titles.len() + tail.len()); + titles.extend((1..=middle).map(|i| format!("Key Point {i}"))); + titles.extend(tail); + titles +} + +/// Per-slide `elements` in the deck corpus's own terms — one takeaway plus its +/// supporting parts, never a list of paragraphs. +fn fallback_slide_elements(title: &str) -> &'static str { + match title { + "Cover" => { + "the deck's title on one line, a one-line subtitle, and a meta row \ + with the presenter and the date" + } + "Agenda" => "a takeaway title plus 3-5 numbered agenda entries, one short line each", + "Evidence" => { + "a takeaway title plus the figures that support it — 3 KPI cards \ + (value + unit + label) or one chart placeholder with its insight \ + written out" + } + "Closing" => "a closing headline, one supporting line, and a contact / next-step block", + _ => { + "this slide's ONE takeaway as the title, plus the supporting \ + content that makes the point — short phrases, never paragraphs" + } + } +} + +/// An explicitly requested slide count ("12 页", "10-slide", "8 slides"). +/// +/// Bounded to a plausible deck size so an unrelated number in the prompt (a +/// year, a pixel size, a price) cannot turn into a 2026-slide plan — the unit +/// word right after the digits is what qualifies it as a slide count at all. +fn explicit_slide_count(prompt: &str) -> Option { + const UNITS: [&str; 5] = ["slides", "slide", "pages", "page", "页"]; + let lower = prompt.to_lowercase(); + let bytes = lower.as_bytes(); + let mut i = 0usize; + while i < bytes.len() { + if !bytes[i].is_ascii_digit() { + i += 1; + continue; + } + let start = i; + while i < bytes.len() && bytes[i].is_ascii_digit() { + i += 1; + } + // "10-slide" / "12 页" / "8slides" — only separators may intervene. + let mut unit_start = i; + while unit_start < bytes.len() && matches!(bytes[unit_start], b' ' | b'-' | b'_' | b'\t') { + unit_start += 1; + } + if UNITS + .iter() + .any(|unit| lower[unit_start..].starts_with(unit)) + { + if let Ok(count) = lower[start..i].parse::() { + if (2..=30).contains(&count) { + return Some(count); + } + } + } + } + None +} + fn explicit_mobile_size(prompt: &str) -> Option<(f64, f64)> { let normalized = prompt.replace('×', "x").to_lowercase(); let bytes = normalized.as_bytes(); @@ -341,6 +492,10 @@ fn explicit_mobile_size(prompt: &str) -> Option<(f64, f64)> { None } +#[cfg(test)] +#[path = "plan_fallback_deck_tests.rs"] +mod fallback_deck_tests; + #[cfg(test)] mod tests { use super::*; diff --git a/crates/op-orchestrator/src/plan_fallback_deck_tests.rs b/crates/op-orchestrator/src/plan_fallback_deck_tests.rs new file mode 100644 index 000000000..038c28c07 --- /dev/null +++ b/crates/op-orchestrator/src/plan_fallback_deck_tests.rs @@ -0,0 +1,152 @@ +//! Guards for the Slides arm of [`build_fallback_plan`]. +//! +//! Measured 2026-08-05 on a user's desktop run: two `planning stream error` +//! attempts on a deck request dropped straight through to the heuristic +//! fallback, which had no Slides branch — the user asked for a PPT and the +//! orchestrator built the generic 1200-wide three-section page. These tests +//! lock the three properties that make the fallback an actual deck. + +use super::*; +use crate::plan_normalize::normalize; + +fn deck_req(prompt: &str) -> DesignRequest { + DesignRequest { + prompt: prompt.into(), + ..Default::default() + } +} + +#[test] +fn deck_fallback_uses_the_projector_artboard_not_the_desktop_page() { + let plan = build_fallback_plan(&deck_req("帮我做一个季度汇报 PPT")); + assert_eq!(plan.root_frame.width, 1920.0); + assert_eq!(plan.root_frame.height, 1080.0); + for st in &plan.subtasks { + assert_eq!( + (st.region.width, st.region.height), + (1920.0, 1080.0), + "every slide region is the full board: {}", + st.id + ); + } +} + +#[test] +fn deck_fallback_tags_every_slide_with_its_own_screen() { + let plan = build_fallback_plan(&deck_req("a pitch deck for our seed round")); + let screens: Vec<&str> = plan + .subtasks + .iter() + .map(|st| st.screen.as_deref().expect("every slide carries a screen")) + .collect(); + let mut unique = screens.clone(); + unique.sort_unstable(); + unique.dedup(); + assert_eq!( + unique.len(), + screens.len(), + "duplicate screen labels collapse slides onto one board: {screens:?}" + ); +} + +#[test] +fn deck_fallback_screens_survive_normalize_as_separate_roots() { + // The `screen` tags are only worth anything if `plan_normalize` fans them + // out — that is the pass which decides one root vs N. + let mut plan = build_fallback_plan(&deck_req("做一份 7 页的产品发布演示文稿")); + let req = deck_req("做一份 7 页的产品发布演示文稿"); + let info = normalize(&mut plan, &req); + + assert!( + info.is_deck, + "normalize must classify the fallback as a deck" + ); + let roots: std::collections::BTreeSet<&str> = plan + .subtasks + .iter() + .map(|st| st.parent_frame_id.as_deref().expect("parent assigned")) + .collect(); + assert_eq!( + roots.len(), + 7, + "each slide needs its own root frame, got {roots:?}" + ); +} + +#[test] +fn deck_fallback_honours_an_explicitly_requested_slide_count() { + for (prompt, expected) in [ + ("帮我做一个 12 页的产品培训课件 PPT", 12), + ("做一个3页的极简 keynote", 3), + ("a 10-slide pitch deck", 10), + ("build a deck, 9 slides, dark theme", 9), + ] { + let plan = build_fallback_plan(&deck_req(prompt)); + assert_eq!( + plan.subtasks.len(), + expected, + "{prompt:?} asked for {expected} slides" + ); + } +} + +#[test] +fn deck_fallback_without_a_count_scales_with_the_prompt_instead_of_fixing_six() { + let short = build_fallback_plan(&deck_req("做个 PPT")); + let long = build_fallback_plan(&deck_req(&format!( + "帮我做一套关于分布式系统的培训幻灯片,覆盖一致性、复制、分区容错、\ + 共识算法、故障恢复和可观测性六个主题,每个主题都要有示意图和要点。{}", + "补充说明。".repeat(20) + ))); + assert!( + long.subtasks.len() > short.subtasks.len(), + "a richly described deck must plan more slides than a bare one: \ + {} vs {}", + long.subtasks.len(), + short.subtasks.len() + ); +} + +#[test] +fn slide_count_parser_ignores_numbers_that_are_not_slide_counts() { + for prompt in [ + "做一个 1920x1080 的 PPT", + "2026 年度汇报 PPT", + "a deck about our $500 pricing tier", + "16:9 keynote", + ] { + assert_eq!( + explicit_slide_count(prompt), + None, + "{prompt:?} carries no slide count" + ); + } + // …and still reads the real ones out of a noisy prompt. + assert_eq!( + explicit_slide_count("2026 年度汇报,1920x1080,一共 8 页"), + Some(8) + ); +} + +#[test] +fn slide_titles_stay_distinct_and_bracketed_at_every_count() { + for count in 2..=30usize { + let titles = fallback_slide_titles(count); + assert_eq!(titles.len(), count, "count {count}"); + assert_eq!(titles.first().map(String::as_str), Some("Cover")); + assert_eq!(titles.last().map(String::as_str), Some("Closing")); + let mut unique = titles.clone(); + unique.sort(); + unique.dedup(); + assert_eq!(unique.len(), count, "duplicate titles at count {count}"); + } +} + +#[test] +fn non_deck_fallback_is_unchanged() { + // Regression lock: the Slides branch must not intercept anything else. + let plan = build_fallback_plan(&deck_req("a marketing landing page for a fintech product")); + assert_eq!(plan.root_frame.id, "root"); + assert_eq!(plan.root_frame.width, 1200.0); + assert!(plan.subtasks.iter().all(|st| st.screen.is_none())); +} diff --git a/crates/op-orchestrator/src/prompt_planning_tests.rs b/crates/op-orchestrator/src/prompt_planning_tests.rs index 4ae95f58a..4820cd172 100644 --- a/crates/op-orchestrator/src/prompt_planning_tests.rs +++ b/crates/op-orchestrator/src/prompt_planning_tests.rs @@ -499,3 +499,33 @@ fn subagent_prompt_basic_mobile_food_keeps_mobile_app_skill() { "mobile-app carries bottom-nav and top-rhythm rules and must not be truncated; report={report:?}" ); } + +/// gemini-3.6-flash wrote `justify.content:` three times in one slide script +/// and QuickJS threw the whole board away at the first `.`. The runner now +/// repairs that shape, but the cheaper fix is the model not writing it — so +/// the property-naming rule must actually reach the sub-agent prompt. +#[test] +fn subagent_prompt_teaches_camelcase_property_names() { + let st = Subtask { + id: "cover".into(), + label: "Cover".into(), + region: Region { + width: 1920.0, + height: 1080.0, + }, + id_prefix: "cover".into(), + parent_frame_id: Some("root".into()), + elements: None, + screen: Some("Cover".into()), + generated_root_id: None, + existing_section_labels: None, + retry_feedback: None, + }; + let (cr, _) = bsp(&st, &plan(), &req(), AbortFlag::new(), false, false); + for rule in ["PROPERTY NAMES are camelCase", "justify.content"] { + assert!( + cr.system_prompt.contains(rule), + "sub-agent prompt lost the property-naming rule {rule:?}" + ); + } +} diff --git a/crates/op-orchestrator/src/prompt_skill_budget_tests.rs b/crates/op-orchestrator/src/prompt_skill_budget_tests.rs index ec4ea491c..b7464aa62 100644 --- a/crates/op-orchestrator/src/prompt_skill_budget_tests.rs +++ b/crates/op-orchestrator/src/prompt_skill_budget_tests.rs @@ -137,6 +137,49 @@ fn every_augmented_placeholder_has_a_worst_case_in_this_guard() { } } +/// `decomposition` is the skill that decides how many slides a deck gets, and +/// the budget trimmer cuts from the END — so a skill that overruns loses its +/// tail silently while still looking present in the prompt. Asserting the +/// LAST line of the corpus file reaches the assembled system prompt is what +/// proves the whole skill arrived, and the deck rules in its middle with it. +#[test] +fn decomposition_reaches_the_planning_prompt_with_its_tail_intact() { + let request = DesignRequest { + prompt: "帮我做一个 12 页的产品培训课件 PPT".into(), + ..req() + }; + let pp = build_orchestrator_prompt(&request, PlanningMode::Rich, AbortFlag::new()); + let system_prompt = &pp.call_request.system_prompt; + + let body = &op_ai_skills::get_skill_by_name("decomposition") + .expect("decomposition registered") + .content; + let last_line = body + .trim_end() + .lines() + .next_back() + .expect("decomposition is not empty"); + assert!( + system_prompt.contains(last_line), + "decomposition's last line {last_line:?} never reached the planning prompt — \ + the skill is over its {} budget (measured {}) and its tail was trimmed", + op_ai_skills::get_skill_by_name("decomposition") + .expect("registered") + .meta + .budget, + estimate_tokens(body) + ); + + // The de-anchored slide-count teaching sits mid-file; assert it directly + // so a future edit can't drop it while keeping the tail. + for rule in ["SLIDE COUNT", "HARD constraint"] { + assert!( + system_prompt.contains(rule), + "planning prompt lost the slide-count rule {rule:?}" + ); + } +} + #[test] fn no_planning_skill_is_dropped_or_truncated_by_the_phase_budget() { // End-to-end through the real resolver: the per-skill cap AND the phase diff --git a/crates/op-orchestrator/src/run.rs b/crates/op-orchestrator/src/run.rs index 0331cf4b0..252553768 100644 --- a/crates/op-orchestrator/src/run.rs +++ b/crates/op-orchestrator/src/run.rs @@ -190,12 +190,14 @@ async fn planning_loop( "planning parse failure" ); } - Err(true) => { - // abort 在流中发生 → 立即返回 - return Err(OrchestratorError::Aborted); - } - Err(false) => { - tracing::warn!(attempt, "planning stream error"); + // abort 在流中发生 → 立即返回 + Err(error) if error.aborted => return Err(OrchestratorError::Aborted), + Err(error) => { + // 带上原因:此前只记 attempt,用户贴来的日志里 + // "planning stream error" 无从区分 429 限流 / 网络中断 / + // provider 报错,只能靠猜。 + let reason = error.message.trim().chars().take(200).collect::(); + tracing::warn!(attempt, error = %reason, "planning stream error"); } } if abort.is_set() { @@ -211,16 +213,17 @@ async fn planning_loop( } /// 消费一次 LLM 调用的流 —— 拼接所有 `Text` chunk,丢弃 `Thinking`。 -/// `Err(true)` 表示中止,`Err(false)` 表示真实错误。 +/// 错误原样透出(`aborted` 区分中止 / 真实错误),调用方据此决定是 +/// 立即返回还是重试,并把 `message` 写进日志。 async fn collect_text( mut stream: futures::stream::BoxStream<'static, Result>, -) -> Result { +) -> Result { let mut text = String::new(); while let Some(item) = stream.next().await { match item { Ok(LlmChunk::Text(t)) => text.push_str(&t), Ok(LlmChunk::Thinking(_)) => {} - Err(e) => return Err(e.aborted), + Err(e) => return Err(e), } } Ok(text)