diff --git a/Cargo.lock b/Cargo.lock index dd712aaa0..c91c6486c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3253,6 +3253,8 @@ dependencies = [ "serde_json", "skia-safe", "tokio", + "tracing", + "tracing-subscriber", "zip", ] diff --git a/Cargo.toml b/Cargo.toml index dc81962ed..e83ddfce3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ serde_stacker = "0.1" thiserror = "1" anyhow = "1" tracing = "0.1" +tracing-subscriber = { version = "0.3", features = ["env-filter"] } # WASM-boundary related (per kickoff §1.2 blacklist = must not appear here). # Intentionally NOT exposing tokio / reqwest / agent / pen-agent-cli / diff --git a/crates/op-orchestrator/src/cleanup.rs b/crates/op-orchestrator/src/cleanup.rs index d8c98a50e..635b721f4 100644 --- a/crates/op-orchestrator/src/cleanup.rs +++ b/crates/op-orchestrator/src/cleanup.rs @@ -456,9 +456,15 @@ fn nav_surface_target(child: &PenNode) -> Option<&PenNode> { fn is_nav_surface(node: &PenNode) -> bool { let role = node.base().role.as_deref().unwrap_or("").to_lowercase(); + // Matches tree_heuristics::NAV_ROLES exactly. The TOP header roles + // (`navbar` / `top-nav-bar` / `top-app-bar`) are deliberately EXCLUDED: on a + // light mobile page the header is transparent (TS references), and re-filling + // it with the root surface hex + a drop-shadow is exactly what re-boxed the + // mobile header the user flagged. Only bottom navs / floating tab bars — which + // float over scrolling content — need a surface to read against the page. if matches!( role.as_str(), - "navbar" | "nav" | "tab-bar" | "bottom-tab-bar" | "top-nav-bar" | "top-app-bar" | "tab-row" + "nav" | "tab-bar" | "bottom-tab-bar" | "tab-row" ) { return true; } @@ -474,9 +480,6 @@ fn is_nav_surface(node: &PenNode) -> bool { || hay.contains("tab-bar") || hay.contains("bottom tab") || hay.contains("bottom-tab") - || hay.contains("navbar") - || hay.contains("nav bar") - || hay.contains("navigation bar") } fn nav_surface_repair(nav: &PenNode, root_surface_hex: Option<&str>) -> Option { diff --git a/crates/op-orchestrator/src/cleanup_tests.rs b/crates/op-orchestrator/src/cleanup_tests.rs index d849f3cec..3328f154b 100644 --- a/crates/op-orchestrator/src/cleanup_tests.rs +++ b/crates/op-orchestrator/src/cleanup_tests.rs @@ -300,6 +300,67 @@ fn cleanup_injects_missing_bottom_nav_surface_on_light_mobile_root() { ); } +#[test] +fn cleanup_leaves_top_navbar_transparent_on_light_mobile_root() { + // The top header is transparent on mobile (TS references). A previous + // version of `is_nav_surface` matched `role:"navbar"`, so this pass re-filled + // the header with the root surface hex + a downward shadow — the "mysterious + // background + rounded border" the user flagged. The bottom nav still gets a + // surface; the top header must be left untouched. + let mut sink = VecDocSink::new(); + let tree: PenNode = serde_json::from_value(json!({ + "type": "frame", + "id": "root", + "name": "Brooklyn Food Delivery", + "x": 80, "y": 40, "width": 390, "height": 844, + "layout": "vertical", + "fill": [{ "type": "solid", "color": "#FFF8F0" }], + "children": [ + { + "type": "frame", "id": "header", "name": "Header", "role": "navbar", + "width": "fill_container", "height": 56, "children": [] + }, + { + "type": "frame", "id": "content", "name": "Content", + "width": "fill_container", "height": 704, "children": [] + }, + { + "type": "frame", "id": "bottom-nav", "name": "Bottom Navigation", + "role": "bottom-tab-bar", "width": "fill_container", "height": 84, "children": [] + } + ] + })) + .expect("mobile root json"); + sink.state.apply(EditorCommand::InsertSubtree { + nodes: vec![tree], + parent_id: NodeId::NONE, + page_id: None, + }); + let root_id = sink.state.active_children()[0].id_str().to_string(); + sink.applied.clear(); + + run_cleanup_passes(&mut sink, &plan(), &[&root_id]); + + // The bottom nav is still repaired → a downward (offsetY = -4) shadow exists. + assert!( + sink.applied.iter().any(|c| matches!( + c, + EditorCommand::SetEffectParam { field: EffectField::OffsetY, value, .. } + if (*value - -4.0).abs() < f32::EPSILON + )), + "bottom nav should still receive its upward-pointing shadow" + ); + // The top header is NOT repaired → no downward (offsetY = +4) header shadow. + assert!( + !sink.applied.iter().any(|c| matches!( + c, + EditorCommand::SetEffectParam { field: EffectField::OffsetY, value, .. } + if (*value - 4.0).abs() < f32::EPSILON + )), + "top navbar must stay transparent — no surface shadow re-boxing it" + ); +} + #[test] fn cleanup_recolors_white_bottom_nav_to_tinted_mobile_root_surface() { let mut sink = VecDocSink::new(); diff --git a/crates/op-orchestrator/src/lib.rs b/crates/op-orchestrator/src/lib.rs index 30abe619b..5f91717c7 100644 --- a/crates/op-orchestrator/src/lib.rs +++ b/crates/op-orchestrator/src/lib.rs @@ -18,6 +18,7 @@ pub mod intent; pub mod manifest; pub mod manifest_hints; pub mod model_profile; +pub mod palette_harmonize; pub mod parse; pub mod plan; pub mod plan_normalize; @@ -51,6 +52,7 @@ pub mod run_dashboard; pub mod scaffold; pub mod scaffold_dashboard; pub mod subagent; +pub mod tree_heuristics; #[cfg(test)] mod test_support; diff --git a/crates/op-orchestrator/src/manifest.rs b/crates/op-orchestrator/src/manifest.rs index c8f2ccf0b..2a892b07d 100644 --- a/crates/op-orchestrator/src/manifest.rs +++ b/crates/op-orchestrator/src/manifest.rs @@ -44,7 +44,14 @@ static NEXT_SECTION_ID: AtomicU64 = AtomicU64::new(1); /// —— minimax-m3 92% / ark-code 92% / glm-5.1 98% / deepseek 83%。 /// 强模型(Claude / GPT / Gemini 等)无基准数据不翻:目录是弱模型的 /// 地板,不做强模型的天花板。kimi 同样无数据,待跑分后再进表。 -const MANIFEST_DEFAULT_FAMILIES: &[&str] = &["minimax", "glm", "deepseek", "ark-code"]; +/// +/// glm 家族精确到 `glm-5.1`(不是泛 `glm`):glm-5.2(2026-06-18 定为 Full tier +/// 强模型)的 manifest 输出在**完整页面**上反复崩 —— 深 section 嵌套超 depth-2 +/// 被 clamp、拿 element 当容器被扁平成兄弟 → 全堆顶层重叠(ab-v9 组件级 M3 96% +/// 不代表整页布局)。强模型本就该走裸 PenNode JSONL(parse_nodes),它擅长嵌套 +/// 树(op-smoke 实测 restaurant 卡片嵌套正确)。`glm-5.2` 子串不命中 → 裸 JSONL; +/// `glm-5.1` 仍走 manifest 脚手架。 +const MANIFEST_DEFAULT_FAMILIES: &[&str] = &["minimax", "glm-5.1", "deepseek", "ark-code"]; /// M2 起的清单协议开关,M4 升级为按模型路由:`OPENPENCIL_MANIFEST` /// 保留为双向 override —— `1/true/on` 强制开(op-smoke 基准的 @@ -234,6 +241,37 @@ fn build_element_entry( raw: false, } } + // Unknown kind = the model invented a semantic name not in the + // catalog. Most often it's a *container* (`card` / `container` / + // `box` / `wrapper` / `list` …) the model groups children into via + // `in:`. Dropping it loses the whole subtree — children then dangle + // and flatten to siblings (the横排 overlap we saw with glm-5.2's + // `card`). Coerce to a raw frame instead: layout/style attrs + // (cornerRadius / fill / layout / gap / padding) carry over, it + // counts as a container (raw Frame, see resolve_in_ref), and + // children nest correctly. A *known* kind that fails on args + // (BuildFailed) still drops — that's a real builder rejection. + Err(op_mcp::element_manifest::ElementBuildError::UnknownKind(_)) => { + warnings.push(format!( + "W-COERCED-FRAME line {line_no}: unknown element kind {kind:?} → raw frame container" + )); + object.remove("el"); + object.insert("type".to_string(), Value::String("frame".to_string())); + // Frame needs concrete sizing/layout; the model usually gives a + // card cornerRadius/fill but not width/height — fill flex-section + // defaults so `from_value::` succeeds and the frame lays + // out as a normal vertical container. + object + .entry("width") + .or_insert_with(|| Value::String("fill_container".to_string())); + object + .entry("height") + .or_insert_with(|| Value::String("fit_content".to_string())); + object + .entry("layout") + .or_insert_with(|| Value::String("vertical".to_string())); + build_raw_entry(line_no, object, entries, warnings) + } Err(err) => { warnings.push(format!("W-DROPPED line {line_no} ({kind}): {err}")); Entry::Dropped @@ -599,6 +637,51 @@ Here is the design: .any(|w| w.contains("W-IN-NOT-CONTAINER"))); } + #[test] + fn unknown_kind_used_as_container_coerces_to_frame() { + // glm-5.2 invents `card` (not in the catalog) as a container and + // nests children into it via `in:`. Dropping it would dangle the + // children → they flatten to siblings → horizontal overflow/overlap + // (the崩 layout reported 2026-06-18). It must coerce to a raw frame + // that holds the children, keeping the card's cornerRadius/fill. + let text = r##" +{"el":"section","direction":"vertical","role":"popular"} +{"el":"card","in":1,"cornerRadius":16,"fill":[{"type":"solid","color":"#FFFFFF"}]} +{"el":"stat_card","in":2,"label":"MRR","value":"$48k"} +"##; + let outcome = parse_manifest(text).expect("manifest"); + assert_eq!(outcome.nodes.len(), 1, "one section root"); + assert!( + outcome + .warnings + .iter() + .any(|w| w.contains("W-COERCED-FRAME")), + "expected a coerce warning, got {:?}", + outcome.warnings + ); + assert!( + !outcome.warnings.iter().any(|w| w.contains("W-DROPPED")), + "card must NOT be dropped: {:?}", + outcome.warnings + ); + // section > card(frame) > stat_card — nested, NOT flattened to siblings. + let section_kids = frame_children(&outcome.nodes[0]); + assert_eq!( + section_kids.len(), + 1, + "section holds the coerced card frame" + ); + assert!( + matches!(§ion_kids[0], PenNode::Frame(_)), + "card coerced to a frame container" + ); + assert_eq!( + frame_children(§ion_kids[0]).len(), + 1, + "stat_card nested inside the card, not flattened to a sibling" + ); + } + #[test] fn forbidden_id_fields_are_stripped_with_warning() { let text = r#"{"el":"badge","label":"New","id":"my-id","parent_id":"root"}"#; @@ -718,29 +801,52 @@ Here is the design: } #[test] - fn bad_element_line_is_dropped_without_killing_the_batch() { + fn unknown_element_line_coerces_to_frame_without_killing_the_batch() { + // An invented kind no longer drops — dropping loses container + // subtrees (the glm-5.2 `card` regression). It coerces to a frame; + // the rest of the batch is unaffected. let text = r#" {"el":"nonexistent_widget","label":"x"} {"el":"badge","label":"Live"} "#; let outcome = parse_manifest(text).expect("manifest"); - assert_eq!(outcome.dropped_lines, 1); - assert_eq!(outcome.element_lines, 1); - assert_eq!(outcome.nodes.len(), 1); - assert!(outcome.warnings.iter().any(|w| w.contains("W-DROPPED"))); + assert_eq!( + outcome.dropped_lines, 0, + "unknown coerces now, no longer drops" + ); + assert_eq!( + outcome.element_lines, 1, + "badge is the only catalog element" + ); + assert_eq!(outcome.nodes.len(), 2, "coerced frame + badge"); + assert!(outcome + .warnings + .iter() + .any(|w| w.contains("W-COERCED-FRAME"))); } #[test] - fn line_numbers_count_dropped_lines() { - // 行 1 失败被丢,但行号仍占住:行 3 的 `in:2` 指向行 2 的 section。 + fn line_numbers_survive_a_coerced_line() { + // 行 1 是未知 kind(coerce 成 frame、占住行号);行 3 的 `in:2` 仍 + // 正确指向行 2 的 section,badge 进 section 而非错位到顶层。 let text = r#" {"el":"nonexistent_widget"} {"el":"section","role":"body"} {"el":"badge","in":2,"label":"Live"} "#; let outcome = parse_manifest(text).expect("manifest"); - assert_eq!(outcome.nodes.len(), 1); - assert_eq!(frame_children(&outcome.nodes[0]).len(), 1); + assert_eq!( + outcome.nodes.len(), + 2, + "coerced frame (line 1) + section (line 2)" + ); + // The section is the 2nd root and still holds the badge — proving + // the coerced line 1 占住了行号 so `in:2` resolved to the section. + assert_eq!( + frame_children(&outcome.nodes[1]).len(), + 1, + "badge nested in the section" + ); } // ── M4 按模型路由 ────────────────────────────────────────────────── @@ -777,10 +883,17 @@ Here is the design: "gpt-4o", "gemini-2.5-pro", "kimi-k2.6", // 无基准数据,刻意不进表 + "glm-5.2", // Full-tier 强模型:manifest 脚手架限制其嵌套→整页崩,走裸 JSONL "", // chat 路径没有模型信息 → 裸 JSONL ] { assert!(!manifest_enabled_for_model(id), "{id:?} should route OFF"); } + // glm-5.1 (the benchmarked weak model) still routes ON — the family + // is precise (`glm-5.1`), so 5.2 falls through but 5.1 doesn't. + assert!( + manifest_enabled_for_model("glm-5.1"), + "glm-5.1 keeps the scaffold" + ); } #[test] diff --git a/crates/op-orchestrator/src/model_profile.rs b/crates/op-orchestrator/src/model_profile.rs index 4f55c671e..c4d3ba672 100644 --- a/crates/op-orchestrator/src/model_profile.rs +++ b/crates/op-orchestrator/src/model_profile.rs @@ -125,6 +125,19 @@ const MODEL_PROFILES: &[Entry] = &[ timeout_multiplier: 2.0, label: "DeepSeek V4 Pro", }, + // GLM-5.2(及以后更高版本)—— ab-v9 manifest arm 实测很强(M3 96%、composite + // 5/5),给 Full tier。**只 5.2 起算强**(用户判定:glm-5 / glm-5.1 不够,不命中 + // 这条 → 落 default Standard)。关思考与 tier 无关:builtin HTTP 路由按 + // is_glm_model 对**所有** glm(含 5 / 5.1)强制下发 thinking:{type:disabled}, + // 因为它们都是推理模型、不关都会烧光 content。GLM 体量大、即便关思考也偏慢, + // timeout ×2。未来 glm-5.3 / glm-6 等更高版本同样算强,届时各加一条 matcher。 + Entry { + matcher: Match::Sub("glm-5.2"), + tier: ModelTier::Full, + thinking_disabled: true, + timeout_multiplier: 2.0, + label: "GLM-5.2 (Coding Plan)", + }, e( Match::Sub("deepseek-v4-flash"), ModelTier::Standard, diff --git a/crates/op-orchestrator/src/palette_harmonize.rs b/crates/op-orchestrator/src/palette_harmonize.rs new file mode 100644 index 000000000..758d94770 --- /dev/null +++ b/crates/op-orchestrator/src/palette_harmonize.rs @@ -0,0 +1,173 @@ +//! Palette hue harmonization. +//! +//! The semantic palette ([`crate::semantic_palette`]) ships a FIXED cool-slate +//! neutral ramp (`color-surface-2` = `#F1F5F9`, `color-bg-deep` = `#F8FAFC`, +//! borders in `#E2E8F0`/`#CBD5E1` …). Those blue-tinted grays clash badly on a +//! WARM design — a cream/orange food-delivery app gets a cool-gray search bar, +//! cool-gray category chips and a cool-gray avatar circle that read as "not part +//! of the palette" (the user flagged exactly this). +//! +//! The TS design-system generator tints its neutrals toward the brand hue; the +//! Rust port used the raw slate table. This module closes that gap: given the +//! page background color, it re-tints the neutral tokens to share the page's hue +//! while preserving each token's own lightness and chroma magnitude (so the ramp +//! stays as subtle as before — only its temperature changes). A neutral page +//! (white / true gray) has no hue to harmonize toward, so the slate ramp is left +//! untouched; a cool design therefore keeps its cool grays. + +use jian_ops_schema::variable::{VariableDefinition, VariableScalar, VariableValue}; +use std::collections::BTreeMap; + +/// Neutral surface + border tokens that should follow the design temperature. +/// `color-surface` (`#FFFFFF`) is intentionally omitted — pure white reads clean +/// on any palette, and it has zero chroma to rotate anyway. +const NEUTRAL_TINT_TOKENS: &[&str] = &[ + "color-surface-2", + "color-surface-3", + "color-bg-deep", + "color-border", + "color-border-strong", +]; + +fn parse_rgb(hex: &str) -> Option<(f64, f64, f64)> { + let h = hex.strip_prefix('#').unwrap_or(hex); + if !h.is_ascii() || h.len() < 6 { + return None; + } + let r = u8::from_str_radix(&h[0..2], 16).ok()? as f64; + let g = u8::from_str_radix(&h[2..4], 16).ok()? as f64; + let b = u8::from_str_radix(&h[4..6], 16).ok()? as f64; + Some((r, g, b)) +} + +/// The page background's warm/cool direction — each channel's deviation from the +/// color's own gray (mean), normalized so the strongest channel is ±1. `None` +/// when the bg is effectively neutral (nothing to harmonize toward). +fn page_hue_direction(page_bg_hex: &str) -> Option<[f64; 3]> { + let (r, g, b) = parse_rgb(page_bg_hex)?; + let mean = (r + g + b) / 3.0; + let dev = [r - mean, g - mean, b - mean]; + let max_abs = dev.iter().fold(0.0_f64, |m, &d| m.max(d.abs())); + // A delta below ~3/255 is a true gray/white — keep the slate ramp. + if max_abs < 3.0 { + return None; + } + Some([dev[0] / max_abs, dev[1] / max_abs, dev[2] / max_abs]) +} + +/// Re-tint a neutral hex to the page hue `dir`, preserving its lightness (mean) +/// and chroma magnitude (max−min). A pure gray (chroma 0) returns unchanged. +/// Non-6-digit values (e.g. an `#RRGGBBAA` scrim) are returned untouched. +fn retint_neutral(hex: &str, dir: [f64; 3]) -> String { + let Some((r, g, b)) = parse_rgb(hex) else { + return hex.to_string(); + }; + let mean = (r + g + b) / 3.0; + let chroma = r.max(g).max(b) - r.min(g).min(b); + let half = chroma / 2.0; + let ch = |i: usize| (mean + dir[i] * half).round().clamp(0.0, 255.0) as u8; + format!("#{:02X}{:02X}{:02X}", ch(0), ch(1), ch(2)) +} + +fn retint_definition(def: &mut VariableDefinition, dir: [f64; 3]) { + match &mut def.value { + VariableValue::Themed(vals) => { + for tv in vals.iter_mut() { + if let VariableScalar::Str(s) = &mut tv.value { + *s = retint_neutral(s, dir); + } + } + } + VariableValue::Scalar(VariableScalar::Str(s)) => { + *s = retint_neutral(s, dir); + } + _ => {} + } +} + +/// Re-tint the neutral surface + border tokens of `palette` to share +/// `page_bg_hex`'s hue. No-op when the page background is neutral. +pub fn harmonize_palette_neutrals( + palette: &mut BTreeMap, + page_bg_hex: &str, +) { + let Some(dir) = page_hue_direction(page_bg_hex) else { + return; + }; + for name in NEUTRAL_TINT_TOKENS { + if let Some(def) = palette.get_mut(*name) { + retint_definition(def, dir); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn light_hex(def: &VariableDefinition) -> String { + match &def.value { + VariableValue::Themed(vals) => match &vals[0].value { + VariableScalar::Str(s) => s.clone(), + _ => unreachable!(), + }, + _ => unreachable!(), + } + } + + fn warm(r: u8, _g: u8, b: u8) -> bool { + // Warm = red channel above blue channel. + r > b + } + + #[test] + fn warm_page_warms_the_cool_neutrals() { + let mut palette = crate::semantic_palette::palette_variables(); + // Cream warm page bg. + harmonize_palette_neutrals(&mut palette, "#FFF8F0"); + // surface-2 was #F1F5F9 (cool: B>R) → now warm (R>B). + let s2 = light_hex(palette.get("color-surface-2").unwrap()); + let (r, g, b) = parse_rgb(&s2).unwrap(); + assert!( + warm(r as u8, g as u8, b as u8), + "surface-2 should be warm after harmonizing to a cream page, got {s2}" + ); + // Lightness (mean) preserved within a couple points of the original 245. + let mean = (r + g + b) / 3.0; + assert!( + (mean - 245.0).abs() <= 2.0, + "lightness preserved (~245), got mean {mean}" + ); + } + + #[test] + fn neutral_page_leaves_slate_untouched() { + let mut palette = crate::semantic_palette::palette_variables(); + let before = light_hex(palette.get("color-surface-2").unwrap()); + // A true-gray page → nothing to harmonize toward. + harmonize_palette_neutrals(&mut palette, "#FAFAFA"); + let after = light_hex(palette.get("color-surface-2").unwrap()); + assert_eq!(before, after, "neutral page keeps the cool slate ramp"); + } + + #[test] + fn cool_page_keeps_cool_neutrals() { + let mut palette = crate::semantic_palette::palette_variables(); + // A cool blue-gray page → neutrals stay cool (B≥R). + harmonize_palette_neutrals(&mut palette, "#EEF2FF"); + let s2 = light_hex(palette.get("color-surface-2").unwrap()); + let (r, _g, b) = parse_rgb(&s2).unwrap(); + assert!( + b as u8 >= r as u8, + "cool page keeps cool neutrals, got {s2}" + ); + } + + #[test] + fn white_surface_token_is_not_in_the_tint_set() { + // color-surface (#FFFFFF) must stay pure white. + let mut palette = crate::semantic_palette::palette_variables(); + harmonize_palette_neutrals(&mut palette, "#FFF8F0"); + assert_eq!(light_hex(palette.get("color-surface").unwrap()), "#FFFFFF"); + } +} diff --git a/crates/op-orchestrator/src/role_defaults.rs b/crates/op-orchestrator/src/role_defaults.rs index 29e3ff1d9..c2ed2b42b 100644 --- a/crates/op-orchestrator/src/role_defaults.rs +++ b/crates/op-orchestrator/src/role_defaults.rs @@ -224,13 +224,23 @@ fn role_defaults(role: &str, node_json: &Value, ctx: &RoleCtx) -> Option } // ── Navigation ── - "navbar" => json!({ - "layout": "horizontal", "width": "fill_container", - "height": if mobile { 56 } else { 72 }, - "padding": if mobile { pad2(0.0, 16.0) } else { pad2(0.0, 80.0) }, - "alignItems": "center", "justifyContent": "space_between", - "fill": navbar_fill(theme), "stroke": navbar_bottom_border(theme) - }), + "navbar" => { + let mut m = json!({ + "layout": "horizontal", "width": "fill_container", + "height": if mobile { 56 } else { 72 }, + "padding": if mobile { pad2(0.0, 16.0) } else { pad2(0.0, 80.0) }, + "alignItems": "center", "justifyContent": "space_between" + }); + // Mobile app top headers sit transparent on the page background (the + // TS references — "Hey, Amanda" / "Good Morning, Daniel" — have no + // surface fill or hairline). Only a DESKTOP top-nav gets the white + // bar + bottom border; on mobile a fill+border boxes the header. + if !mobile { + m["fill"] = navbar_fill(theme); + m["stroke"] = navbar_bottom_border(theme); + } + m + } "bottom-tab-bar" => json!({ "layout": "horizontal", "width": "fill_container", "height": if mobile { 68 } else { 72 }, diff --git a/crates/op-orchestrator/src/role_infer.rs b/crates/op-orchestrator/src/role_infer.rs index 227879c50..f9c09d1a9 100644 --- a/crates/op-orchestrator/src/role_infer.rs +++ b/crates/op-orchestrator/src/role_infer.rs @@ -102,6 +102,15 @@ static NAME_PATTERN_MAP: LazyLock> = LazyLock:: (Regex::new(r"\bform\b").unwrap(), "form-group", false), (Regex::new(r"\bsearch").unwrap(), "search-bar", false), (Regex::new(r"\bnav\s*link").unwrap(), "nav-link", false), + // Bottom tab bars: "Bottom Navigation Bar", "Bottom Tab Bar", "Tab + // Bar". Must precede the generic `nav` exact-map so it wins the + // bottom-specific role (drives the upward lift shadow + nav-surface + // inject). skip_containers=false: the trailing "Bar" IS the role here. + ( + Regex::new(r"bottom\s*(tab|nav)|\btab\s*bar\b").unwrap(), + "bottom-tab-bar", + false, + ), (Regex::new(r"\bstat").unwrap(), "stat-card", true), (Regex::new(r"\bpricing").unwrap(), "pricing-card", true), ( @@ -242,6 +251,34 @@ fn resolve_node_role(node: &mut PenNode, parent_role: Option<&str>) { return; } + // Section-wrapper guard (BOTH explicit + inferred roles, like the tiny-card + // guard above). A card-like / search-bar / input role on a TOP-LEVEL section + // (`parent_role` None) that is actually a multi-block layout WRAPPER is a + // false positive from loose name matching: "Categories & Featured Banner" + // hits `\bfeature` → feature-card, "Location & Search" hits `\bsearch` → + // search-bar — but both are section wrappers, not a single card / search + // field. Left in place, `apply_role_defaults` injects a card fill + border + + // radius that boxes the whole section (the "mysterious background" the user + // flagged). Heuristic: a real card groups LEAF content (icon/title/text); a + // real search bar holds an icon + the input, not sub-frames. So ≥2 sub-frames + // (card) or ≥1 sub-frame (search/input) means it's a wrapper → drop the role. + if parent_role.is_none() { + let sub_frames = node + .children() + .map(|c| { + c.iter() + .filter(|n| matches!(n, PenNode::Frame(_) | PenNode::Group(_))) + .count() + }) + .unwrap_or(0); + let card_like = CARD_LIKE_ROLES.contains(&role_str.as_str()); + let input_like = role_str == "search-bar" || role_str == "input"; + if (card_like && sub_frames >= 2) || (input_like && sub_frames >= 1) { + node.base_mut().role = None; + return; + } + } + node.base_mut().role = Some(role_str); } diff --git a/crates/op-orchestrator/src/role_infer_tests.rs b/crates/op-orchestrator/src/role_infer_tests.rs index 565f7c8aa..c4a9a56e2 100644 --- a/crates/op-orchestrator/src/role_infer_tests.rs +++ b/crates/op-orchestrator/src/role_infer_tests.rs @@ -163,6 +163,74 @@ fn card_role_stripped_on_tiny_node() { assert_eq!(role_of(&big).as_deref(), Some("stat-card")); } +#[test] +fn section_wrapper_feature_card_role_stripped() { + // tt5: "Categories & Featured Banner" hits `\bfeature` → feature-card, but + // it's a top-level section WRAPPER holding 2 sub-frames (categories + + // banner). Must drop the role so no card fill/border/radius boxes the + // section (the "mysterious background" the user flagged). + let mut sec = node(serde_json::json!({ + "type":"frame","id":"sec","name":"Categories & Featured Banner","children":[ + {"type":"frame","id":"chips","name":"Category Chips","children":[]}, + {"type":"frame","id":"banner","name":"Promo Banner","children":[]} + ] + })); + resolve_tree_roles(&mut sec, &ctx()); + assert_eq!( + role_of(&sec), + None, + "multi-block section wrapper must not be a feature-card" + ); +} + +#[test] +fn section_wrapper_search_bar_role_stripped() { + // tt5: "Location & Search" hits `\bsearch` → search-bar, but it's a header + // wrapper (a Location sub-frame + the real search input). Drop the role so + // the bg-deep fill + border + radius don't box the whole header. + let mut sec = node(serde_json::json!({ + "type":"frame","id":"sec","name":"Location & Search","children":[ + {"type":"frame","id":"loc","name":"Location & Actions","children":[]}, + {"type":"text_input","id":"si","placeholder":"Search for food"} + ] + })); + resolve_tree_roles(&mut sec, &ctx()); + assert_eq!( + role_of(&sec), + None, + "header wrapper must not be a search-bar" + ); +} + +#[test] +fn real_feature_card_with_leaf_content_keeps_role() { + // Negative control: a genuine feature-card groups LEAF content (icon/title/ + // text), not sub-frames — the guard must leave its role + card fill intact. + let mut card = node(serde_json::json!({ + "type":"frame","id":"fc","name":"Feature Highlight","children":[ + {"type":"icon_font","id":"ic","iconFontName":"zap"}, + {"type":"text","id":"t","name":"Title","content":"Fast"}, + {"type":"text","id":"d","name":"Desc","content":"Delivered hot"} + ] + })); + resolve_tree_roles(&mut card, &ctx()); + assert_eq!(role_of(&card).as_deref(), Some("feature-card")); +} + +#[test] +fn real_search_bar_with_icon_and_input_keeps_role() { + // Negative control: a real search bar holds an icon + input (no sub-frame) — + // keep search-bar so it still gets its pill fill. + let mut bar = node(serde_json::json!({ + "type":"frame","id":"sb","name":"Search","children":[ + {"type":"icon_font","id":"m","iconFontName":"search"}, + {"type":"text_input","id":"si","placeholder":"Search"} + ] + })); + resolve_tree_roles(&mut bar, &ctx()); + assert_eq!(role_of(&bar).as_deref(), Some("search-bar")); +} + #[test] fn explicit_tiny_card_role_is_stripped() { // An LLM-emitted role:"stat-card" on a 6×6 frame must be STRIPPED (not just diff --git a/crates/op-orchestrator/src/role_layout_post_pass.rs b/crates/op-orchestrator/src/role_layout_post_pass.rs index f23dc938e..410a4f796 100644 --- a/crates/op-orchestrator/src/role_layout_post_pass.rs +++ b/crates/op-orchestrator/src/role_layout_post_pass.rs @@ -84,7 +84,16 @@ pub(crate) fn fix_horizontal_overflow(node: &mut Value, canvas_width: f64) { if needed_w > parent_w && needed_w <= canvas_width { node["width"] = json!(needed_w); } else if needed_w > canvas_width * 0.8 { + // Content exceeds the viewport — widening can't make the children fit + // (their sum already overflows the canvas). Span the viewport and clip + // the overflow at the row edge so it reads as a scroll row cut at the + // screen, instead of chips spilling off-canvas into the void. + // overflow.md mandates a `clipContent` wrapper for scroll rows; weak + // models (e.g. glm-5.2) routinely emit a bare horizontal frame without + // it, so this is the deterministic floor that keeps off-screen children + // from rendering outside the device frame. node["width"] = json!("fill_container"); + node["clipContent"] = json!(true); } } } diff --git a/crates/op-orchestrator/src/role_post_pass.rs b/crates/op-orchestrator/src/role_post_pass.rs index 0667a2649..ddb5ad3ff 100644 --- a/crates/op-orchestrator/src/role_post_pass.rs +++ b/crates/op-orchestrator/src/role_post_pass.rs @@ -376,6 +376,145 @@ fn is_ring_like_decorative(node: &Value) -> bool { corner_radius(node) >= w.min(h) * 0.35 } +/// layout.md:24 — interior section wrappers (Header / Search Section / +/// Categories Section …) MUST be transparent; an explicit near-white fill +/// makes an unwanted "white card" on the page bg. The model (esp. glm-5.2) +/// ignores that rule, and `role_defaults`' AI-explicit-wins won't strip it — +/// so force it here: a structural wrapper carrying a near-white / page-tone +/// solid fill gets cleared to transparent. CARD_LIKE roles (card, promo +/// banner, …) are intentional surfaces and keep their fill; an intentional +/// dark / saturated section background is also left alone (only near-white +/// fills — luminance ≥ 0.85 — read as the "white card on cream" smell). +/// Neutral page/surface design-variable tokens. A structural wrapper carrying +/// any of these is repainting the page background and must go transparent. +/// Variable refs are NOT resolved at post-pass time (binding runs afterwards), +/// so `hex_luminance` can't read them — match the token names directly. Colored +/// tokens (`$color-accent`, dark bands, etc.) are deliberate → never listed here. +const LIGHT_SURFACE_REFS: &[&str] = &[ + "$color-bg-deep", + "$color-surface", + "$color-surface-2", + "$color-surface-3", +]; + +/// True when a fill color reads as a light page/surface tone — either a parsed +/// hex with luminance ≥ 0.85, or one of the neutral surface variable refs that +/// glm emits before variable binding resolves them. +fn is_light_surface_fill(color: &str) -> bool { + LIGHT_SURFACE_REFS.contains(&color) || hex_luminance(color).map(|l| l >= 0.85).unwrap_or(false) +} + +/// A child paints a colored (non-page-tone) surface: a gradient, or a solid +/// that is NOT a light page/surface tone. The signal that a child is a real +/// banner/card surface rather than a neutral panel. +fn child_has_colored_fill(node: &Value) -> bool { + let Some(first) = fill_array(node).and_then(|a| a.first()) else { + return false; + }; + match first.get("type").and_then(Value::as_str) { + Some("linear_gradient") | Some("radial_gradient") => true, + Some("solid") => first + .get("color") + .and_then(Value::as_str) + .map(|c| !is_light_surface_fill(c)) + .unwrap_or(false), + _ => false, + } +} + +/// True when the node has EXACTLY ONE container child and that child spans the +/// width (`fill_container`) painting a colored surface. The node is then a +/// redundant frame wrapped around a full-bleed banner/card — its own light +/// surface fill is a box showing around the colored child. (Tiny leaf siblings +/// like an orphan badge are ignored for the count.) +fn is_redundant_colored_wrapper(node: &Value) -> bool { + let Some(kids) = node.get("children").and_then(Value::as_array) else { + return false; + }; + let containers: Vec<&Value> = kids + .iter() + .filter(|c| { + matches!( + c.get("type").and_then(Value::as_str), + Some("frame") | Some("group") + ) + }) + .collect(); + if containers.len() != 1 { + return false; + } + let child = containers[0]; + child.get("width").and_then(Value::as_str) == Some("fill_container") + && child_has_colored_fill(child) +} + +fn fix_structural_wrapper_transparency(node: &mut Value) { + let Some(role) = role_of(node).map(str::to_string) else { + return; + }; + if CARD_LIKE_ALLOWLIST.contains(&role.as_str()) { + // A card-like role normally keeps its fill — EXCEPT when it is a + // redundant wrapper around a single full-bleed colored child card (glm + // wraps an orange promo banner in a `feature-card` whose own + // `$color-surface` fill then shows as a light box around / under the + // colored child). The colored child IS the banner surface, so strip the + // wrapper's light fill + chrome. + if is_redundant_colored_wrapper(node) { + if let Some(color) = get_first_solid_color(node) { + if is_light_surface_fill(&color) { + if let Some(obj) = node.as_object_mut() { + obj.insert("fill".to_string(), Value::Array(Vec::new())); + obj.remove("stroke"); + obj.remove("effects"); + } + } + } + } + return; // intentional surface — keep its fill + } + let structural = STRUCTURAL_DENYLIST.contains(&role.as_str()) + || SECTION_ROLES.contains(&role.as_str()) + || role == "header" + || role == "app-bar" + || role.contains("section"); + if !structural { + return; + } + let Some(color) = get_first_solid_color(node) else { + return; + }; + if is_light_surface_fill(&color) { + if let Some(obj) = node.as_object_mut() { + obj.insert("fill".to_string(), Value::Array(Vec::new())); + // Drop the accompanying hairline border too: a structural wrapper + // made transparent keeps no "card/bar chrome" (the user flagged the + // navbar's bottom border as unreasonable alongside its background). + obj.remove("stroke"); + // …and no elevation shadow — a now-transparent wrapper would cast it + // as a gray ghost box around its content. + obj.remove("effects"); + } + } +} + +/// True when an immediate child container (frame/group) already paints a +/// visible surface. Such a node is a redundant wrapper around the real card — +/// it must NOT be whitewashed into its own surface, or the injected drop-shadow +/// ghosts out around the colored child as a gray rounded box. +fn has_filled_container_child(node: &Value) -> bool { + node.get("children") + .and_then(Value::as_array) + .map(|kids| { + kids.iter().any(|c| { + matches!( + c.get("type").and_then(Value::as_str), + Some("frame") | Some("group") + ) && has_visible_fill(c) + }) + }) + .unwrap_or(false) +} + fn fix_orphan_container_contrast(node: &mut Value, parent_fill: Option<&Value>) { if parent_fill.is_none() { return; // root has no parent context @@ -397,6 +536,18 @@ fn fix_orphan_container_contrast(node: &mut Value, parent_fill: Option<&Value>) { return; } + // A child container already paints the visible surface (e.g. glm wraps an + // orange promo banner in a bare `feature-card` frame). Whitewashing this + // redundant wrapper into a second card leaks the injected drop-shadow out + // around the colored child as a gray "ghost box" — the exact mysterious + // background + rounded border the user flagged. Let the child be the card. + if has_filled_container_child(node) { + return; + } + // Only rescue containers we POSITIVELY identify as card-like; structural + // wrappers (header / section / banner / search bar) and roleless rounded + // frames stay transparent — white-washing them is what produced the spurious + // white panels behind headers / banners / search rows. let Some(role) = role_of(node) else { return; }; @@ -791,6 +942,10 @@ fn post_pass_value(node: &mut Value, parent_fill: Option, canvas_width: f fix_text_heights(node); } apply_clip_content_for_image(node); + // Strip unwanted white-card fills from structural wrappers (layout.md:24) + // before the contrast fixes, so they evaluate the corrected transparent + // section against the page bg instead of a bogus white surface. + fix_structural_wrapper_transparency(node); // I3 contrast fixes (TS order 9-12). fix_button_foreground_contrast(node); fix_section_alternation(node); @@ -812,6 +967,93 @@ fn post_pass_value(node: &mut Value, parent_fill: Option, canvas_width: f /// Run the contrast post-pass over a forest of sub-agent section roots. Each /// root is round-tripped through JSON; on any (de)serialize failure that root /// is left untouched (a fix can never drop a node). +/// Semantic state-feedback tokens. Legit only on status/alert elements; glm +/// grabs them as "a light color" for decorative surfaces (a `$color-danger-bg` +/// search input renders pink and clashes with the theme). +const STATE_BG_REFS: &[&str] = &[ + "$color-danger-bg", + "$color-info-bg", + "$color-success-bg", + "$color-warning-bg", +]; + +/// The page-background token. Only the page root paints it; an inner node using +/// it just repaints a redundant — or theme-clashing (cool `#F8FAFC` over a warm +/// page) — panel. +const PAGE_BG_REF: &str = "$color-bg-deep"; + +/// A status / feedback element — the ONLY legitimate user of a state-bg token. +fn is_status_element(node: &Value) -> bool { + if let Some(role) = role_of(node) { + if matches!(role, "badge" | "alert" | "toast" | "status") { + return true; + } + } + if let Some(name) = node.get("name").and_then(Value::as_str) { + let l = name.to_lowercase(); + return [ + "error", + "success", + "warning", + "alert", + "danger", + "status", + "toast", + "notification", + ] + .iter() + .any(|k| l.contains(k)); + } + false +} + +/// Surface-color discipline — a deterministic floor walking EVERY node type +/// (incl. `text_input`, which the frame-only `post_pass_value` skips). The TS +/// pipeline relies on the prompt for this; weak models (glm-5.2) ignore it, so +/// Rust enforces it after the fact: +/// 1. A state-bg token misused as a decorative surface → neutral +/// `$color-surface-2`. (the pink search input / chips) +/// 2. `$color-bg-deep` on any inner node → transparent. (the cool grey panel +/// behind the search row / a nav tab repainting the page bg) +/// +/// Refs are still UNRESOLVED here (binding runs later), so match token names. +fn node_has_effects(node: &Value) -> bool { + node.get("effects") + .and_then(Value::as_array) + .map(|a| !a.is_empty()) + .unwrap_or(false) +} + +fn has_any_stroke(node: &Value) -> bool { + node.get("stroke").map(|s| !s.is_null()).unwrap_or(false) +} + +fn fix_surface_color_discipline(node: &mut Value, is_root: bool) { + if let Some(color) = get_first_solid_color(node) { + if STATE_BG_REFS.contains(&color.as_str()) && !is_status_element(node) { + node["fill"] = solid_fill("$color-surface-2"); + } else if !is_root && color == PAGE_BG_REF { + node["fill"] = json!([]); + } + } + // An elevation shadow needs a surface to sit on. A frame with no visible + // fill and no stroke that still carries a drop-shadow renders the shadow as + // a gray "ghost box" floating around its children — strip it. This runs last + // (after binding + every fill-stripping pass), so it sees the FINAL fill + // state and catches both our own injected card shadows on wrappers that got + // emptied and model-authored shadows on bare wrapper frames. + if node_has_effects(node) && !has_visible_fill(node) && !has_any_stroke(node) { + if let Some(obj) = node.as_object_mut() { + obj.remove("effects"); + } + } + if let Some(children) = node.get_mut("children").and_then(Value::as_array_mut) { + for child in children.iter_mut() { + fix_surface_color_discipline(child, false); + } + } +} + pub fn post_pass_forest(nodes: &mut [PenNode], canvas_width: f64) { for node in nodes.iter_mut() { let Ok(mut v) = serde_json::to_value(&*node) else { @@ -824,6 +1066,24 @@ pub fn post_pass_forest(nodes: &mut [PenNode], canvas_width: f64) { } } +/// Surface-color discipline over the whole forest. MUST run AFTER +/// `bind_generated_color_variables` — glm emits raw hex (`#FEE2E2`, +/// `#F8FAFC`), and binding is what normalizes those to the `$color-danger-bg` +/// / `$color-bg-deep` refs this pass matches. Calling it pre-binding (the old +/// site inside `post_pass_forest`) only ever saw hex, so it never matched and +/// the pink search input / cool-grey panels survived. +pub fn enforce_surface_color_discipline(nodes: &mut [PenNode]) { + for node in nodes.iter_mut() { + let Ok(mut v) = serde_json::to_value(&*node) else { + continue; + }; + fix_surface_color_discipline(&mut v, true); + if let Ok(new_node) = serde_json::from_value::(v) { + *node = new_node; + } + } +} + #[cfg(test)] #[path = "role_post_pass_tests.rs"] mod tests; diff --git a/crates/op-orchestrator/src/role_post_pass_tests.rs b/crates/op-orchestrator/src/role_post_pass_tests.rs index fe9613ab1..758cd90d9 100644 --- a/crates/op-orchestrator/src/role_post_pass_tests.rs +++ b/crates/op-orchestrator/src/role_post_pass_tests.rs @@ -121,21 +121,307 @@ fn orphan_skipped_without_corner_radius() { } #[test] -fn orphan_skips_unroled_layout_carousel_shell() { - let mut shell = json!({ - "type":"frame","name":"Promo Carousel Viewport","cornerRadius":20, - "layout":"horizontal","children":[ - {"type":"frame","role":"banner","children":[{"type":"text","content":"50%"}]} - ] +fn orphan_skipped_for_roleless_container() { + // A roleless rounded container is NOT assumed to be a card. Rust role + // inference is thinner than TS, so roleless wrappers (header / section / + // banner) are common; white-washing them is what produced the spurious + // panels behind the header / banner / search row. + let mut wrap = json!({ + "type":"frame","cornerRadius":16, + "children":[{"type":"frame","role":"section","children":[{"type":"text"}]}] }); - fix_orphan_container_contrast(&mut shell, Some(&Value::Null)); + fix_orphan_container_contrast(&mut wrap, Some(&Value::Null)); assert!( - shell.get("fill").is_none(), - "a rounded unroled layout/carousel frame must not become a white card shell" + wrap.get("fill").is_none(), + "roleless container is not white-washed into a card" + ); +} + +#[test] +fn orphan_skipped_when_child_container_paints_surface() { + // glm wraps an orange promo banner in a bare `feature-card` frame. The child + // (Promo Card) carries the full-bleed gradient, so the wrapper must NOT be + // whitewashed — doing so leaks the injected drop-shadow out as a gray ghost + // box around the orange child (the user's "mysterious bg + rounded border"). + let mut wrap = json!({ + "type":"frame","role":"feature-card","cornerRadius":12, + "children":[{ + "type":"frame","role":"card", + "fill":[{"type":"linear_gradient","angle":135,"stops":[ + {"offset":0.0,"color":"#FB923C"},{"offset":1.0,"color":"#F97316"}]}], + "children":[{"type":"text","content":"50% Off"}] + }] + }); + fix_orphan_container_contrast(&mut wrap, Some(&Value::Null)); + assert!( + wrap.get("fill").is_none(), + "wrapper around a filled card stays transparent" ); assert!( - shell.get("effects").is_none(), - "a structural shell must not get card shadow" + wrap.get("effects").is_none(), + "no ghost shadow injected onto the bare wrapper" + ); +} + +// ── orphaned-shadow strip (ghost-box cleanup) ──────────────────────────── + +#[test] +fn orphaned_shadow_stripped_from_fill_less_frame() { + // A drop-shadow with no surface (no fill, no stroke) is a gray ghost box. + let mut node = json!({ + "type":"frame","cornerRadius":12, + "effects":[{"type":"shadow","offsetX":0,"offsetY":1,"blur":3,"spread":0,"color":"#0000001A"}], + "children":[{"type":"frame","role":"card","fill":[{"type":"solid","color":"#FB923C"}]}] + }); + fix_surface_color_discipline(&mut node, false); + assert!( + node.get("effects").is_none(), + "shadow on a fill-less, stroke-less frame is stripped" + ); +} + +#[test] +fn shadow_kept_when_node_has_visible_fill() { + // A real card (visible fill) keeps its elevation shadow. + let mut node = json!({ + "type":"frame","role":"card","cornerRadius":12, + "fill":[{"type":"solid","color":"#FFFFFF"}], + "effects":[{"type":"shadow","offsetX":0,"offsetY":1,"blur":3,"spread":0,"color":"#0000001A"}], + }); + fix_surface_color_discipline(&mut node, false); + assert!(node["effects"].is_array(), "a filled card keeps its shadow"); +} + +// ── fixStructuralWrapperTransparency ───────────────────────────────────── + +#[test] +fn structural_wrapper_white_fill_stripped() { + // A section wrapper that glm gave an explicit white fill → forced transparent. + let mut sect = json!({ + "type":"frame","role":"section","fill":[{"type":"solid","color":"#FFFFFF"}], + "children":[{"type":"text","content":"x"}] + }); + fix_structural_wrapper_transparency(&mut sect); + assert_eq!( + sect["fill"], + json!([]), + "white structural wrapper → transparent" + ); +} + +#[test] +fn structural_wrapper_keeps_card_fill() { + // card is in CARD_LIKE_ALLOWLIST — an intentional surface, fill stays. + let mut card = json!({ + "type":"frame","role":"card","fill":[{"type":"solid","color":"#FFFFFF"}], + "children":[{"type":"text","content":"x"}] + }); + fix_structural_wrapper_transparency(&mut card); + assert_eq!( + card["fill"], + json!([{"type":"solid","color":"#FFFFFF"}]), + "card keeps its white surface" + ); +} + +#[test] +fn redundant_colored_wrapper_strips_light_surface() { + // feature-card with a $color-surface fill wrapping a single full-bleed + // gradient banner child → its surface is a redundant box, strip it. + let mut wrap = json!({ + "type":"frame","role":"feature-card","cornerRadius":12, + "fill":[{"type":"solid","color":"$color-surface"}], + "children":[ + { + "type":"frame","role":"card","width":"fill_container", + "fill":[{"type":"linear_gradient","angle":135,"stops":[ + {"offset":0.0,"color":"$color-chart-6"},{"offset":1.0,"color":"#FB923C"}]}], + "children":[{"type":"text","content":"50% Off"}] + }, + {"type":"text","content":"-30%"} + ] + }); + fix_structural_wrapper_transparency(&mut wrap); + assert_eq!( + wrap["fill"], + json!([]), + "redundant wrapper around a full-bleed colored card → transparent" + ); +} + +#[test] +fn card_with_small_colored_child_keeps_fill() { + // A real card whose colored child is NOT full-bleed (a small icon tile) is + // a genuine surface — must keep its fill. + let mut card = json!({ + "type":"frame","role":"card","fill":[{"type":"solid","color":"#FFFFFF"}], + "children":[ + { + "type":"frame","role":"icon-tile","width":48, + "fill":[{"type":"solid","color":"#FB923C"}], + "children":[] + }, + {"type":"text","content":"Title"} + ] + }); + fix_structural_wrapper_transparency(&mut card); + assert_eq!( + card["fill"], + json!([{"type":"solid","color":"#FFFFFF"}]), + "card with a small colored tile keeps its surface" + ); +} + +#[test] +fn structural_wrapper_keeps_dark_fill() { + // A dark section fill (luminance well below 0.85) is a deliberate band → kept. + let mut sect = json!({ + "type":"frame","role":"section","fill":[{"type":"solid","color":"#1A1A1A"}], + "children":[{"type":"text","content":"x"}] + }); + fix_structural_wrapper_transparency(&mut sect); + assert_eq!( + sect["fill"], + json!([{"type":"solid","color":"#1A1A1A"}]), + "dark structural band is intentional, not stripped" + ); +} + +#[test] +fn structural_wrapper_section_substring_and_header_stripped() { + // role *containing* "section" (e.g. a custom "feature-section") → stripped. + let mut feat = json!({ + "type":"frame","role":"feature-section","fill":[{"type":"solid","color":"#FAFAFA"}], + "children":[{"type":"text"}] + }); + fix_structural_wrapper_transparency(&mut feat); + assert_eq!(feat["fill"], json!([]), "*-section role → transparent"); + // header is structural too. + let mut header = json!({ + "type":"frame","role":"header","fill":[{"type":"solid","color":"#FFFFFF"}], + "children":[{"type":"text"}] + }); + fix_structural_wrapper_transparency(&mut header); + assert_eq!(header["fill"], json!([]), "header → transparent"); +} + +#[test] +fn structural_wrapper_surface_variable_ref_stripped() { + // glm emits UNRESOLVED $color-* refs (variable binding runs after post-pass), + // so hex_luminance can't read them — the strip must match neutral surface + // tokens by name. This is the actual header-background bug: role navbar + + // fill $color-surface-2 was surviving because luminance("$color-surface-2") + // returned None. + let mut header = json!({ + "type":"frame","role":"navbar","fill":[{"type":"solid","color":"$color-surface-2"}], + "children":[{"type":"text"}] + }); + fix_structural_wrapper_transparency(&mut header); + assert_eq!( + header["fill"], + json!([]), + "navbar with $color-surface-2 → transparent" + ); + // A colored token (deliberate accent band) is NOT a neutral surface → kept. + let mut band = json!({ + "type":"frame","role":"section","fill":[{"type":"solid","color":"$color-accent"}], + "children":[{"type":"text"}] + }); + fix_structural_wrapper_transparency(&mut band); + assert_eq!( + band["fill"], + json!([{"type":"solid","color":"$color-accent"}]), + "deliberate colored band kept" + ); +} + +#[test] +fn structural_wrapper_strips_border_with_fill() { + // The mobile header (role navbar) carried a $color-surface fill AND a bottom + // hairline stroke — the user flagged BOTH the background and the border. + // Stripping the surface fill must drop the accompanying border too; a + // transparent structural wrapper keeps no card/bar chrome. + let mut header = json!({ + "type":"frame","role":"navbar","fill":[{"type":"solid","color":"$color-surface"}], + "stroke":{"thickness":[0,0,1,0],"fill":[{"type":"solid","color":"$color-border"}]}, + "children":[{"type":"text"}] + }); + fix_structural_wrapper_transparency(&mut header); + assert_eq!(header["fill"], json!([]), "surface fill stripped"); + assert!( + header.get("stroke").is_none(), + "bottom border stripped alongside the fill" + ); +} + +#[test] +fn structural_wrapper_non_structural_role_left_alone() { + // A plain content role that isn't structural and isn't card-like → untouched. + let mut text = json!({ + "type":"text","role":"body","fill":[{"type":"solid","color":"#FFFFFF"}] + }); + fix_structural_wrapper_transparency(&mut text); + assert_eq!( + text["fill"], + json!([{"type":"solid","color":"#FFFFFF"}]), + "non-structural role is out of scope" + ); +} + +// ── fixSurfaceColorDiscipline ──────────────────────────────────────────── + +#[test] +fn state_bg_token_on_input_recolored_to_neutral() { + // The pink-search bug: glm used $color-danger-bg as the input surface. + let mut input = json!({ + "type":"text_input","name":"Search Input", + "fill":[{"type":"solid","color":"$color-danger-bg"}] + }); + fix_surface_color_discipline(&mut input, false); + assert_eq!( + input["fill"], + json!([{"type":"solid","color":"$color-surface-2"}]), + "danger-bg misused as input surface → neutral surface-2" + ); +} + +#[test] +fn state_bg_token_kept_on_status_element() { + // A real status element (name says "Error") legitimately uses danger-bg. + let mut badge = json!({ + "type":"frame","role":"badge","name":"Error Badge", + "fill":[{"type":"solid","color":"$color-danger-bg"}], + "children":[{"type":"text","content":"Failed"}] + }); + fix_surface_color_discipline(&mut badge, false); + assert_eq!( + badge["fill"], + json!([{"type":"solid","color":"$color-danger-bg"}]), + "status element keeps its semantic state color" + ); +} + +#[test] +fn page_bg_token_stripped_from_inner_node_kept_on_root() { + // Inner wrapper repainting the page bg (the cool grey panel behind search). + let mut root = json!({ + "type":"frame","name":"Page","fill":[{"type":"solid","color":"$color-bg-deep"}], + "children":[ + {"type":"frame","name":"Search & Categories", + "fill":[{"type":"solid","color":"$color-bg-deep"}], + "children":[{"type":"text_input","name":"Search"}]} + ] + }); + fix_surface_color_discipline(&mut root, true); + assert_eq!( + root["fill"], + json!([{"type":"solid","color":"$color-bg-deep"}]), + "page root keeps the page-bg token" + ); + assert_eq!( + root["children"][0]["fill"], + json!([]), + "inner wrapper using the page-bg token → transparent" ); } @@ -200,101 +486,6 @@ fn input_siblings_unified_to_first() { ); } -#[test] -fn nested_search_shell_is_flattened_to_neutral_row() { - let mut row = json!({ - "type":"frame","name":"Search Shell","layout":"horizontal","cornerRadius":28, - "padding":[12,16],"gap":14, - "fill":[{"type":"solid","color":"#DBEAFE"}], - "stroke":{"thickness":1,"fill":[{"type":"solid","color":"#BFDBFE"}]}, - "effects":[{"type":"shadow","offsetY":4,"blur":16,"color":"#00000022"}], - "children":[ - { - "type":"frame","role":"form-input","height":48,"cornerRadius":12, - "fill":[{"type":"solid","color":"#FADADA"}], - "stroke":{"thickness":1,"fill":[{"type":"solid","color":"#E5E7EB"}]}, - "children":[{"type":"path","name":"SearchIcon","width":20,"height":20}] - }, - { - "type":"frame","role":"icon-button","width":52,"height":52, - "fill":[{"type":"solid","color":"#FF6B00"}], - "children":[{"type":"path","name":"SlidersHorizontalIcon","width":20,"height":20}] - } - ] - }); - normalize_nested_search_shell(&mut row); - assert!( - row.get("fill").is_none(), - "outer search shell fill is visual noise" - ); - assert!( - row.get("stroke").is_none(), - "outer search shell stroke is visual noise" - ); - assert!( - row.get("effects").is_none(), - "outer search shell shadow is visual noise" - ); - assert!( - row.get("cornerRadius").is_none(), - "outer shell should not read as a second input" - ); - assert_eq!( - row["children"][0]["fill"], - json!([{"type":"solid","color":"#FFFFFF"}]), - "nested search input is neutralized instead of keeping a pink fill" - ); -} - -#[test] -fn standalone_search_bar_keeps_its_surface() { - let mut search = json!({ - "type":"frame","role":"search-bar","layout":"horizontal","cornerRadius":22, - "fill":[{"type":"solid","color":"#F8FAFC"}], - "children":[ - {"type":"path","name":"SearchIcon","width":20,"height":20}, - {"type":"text","content":"Search"} - ] - }); - normalize_nested_search_shell(&mut search); - assert_eq!(search["fill"], json!([{"type":"solid","color":"#F8FAFC"}])); - assert_eq!(search["cornerRadius"], json!(22)); -} - -#[test] -fn favorite_icon_button_loses_bubble_chrome() { - let mut card = json!({ - "type":"frame","role":"image-card","width":160,"height":240,"children":[ - { - "type":"frame","role":"icon-button","name":"Favorite Button", - "width":36,"height":36,"cornerRadius":999, - "fill":[{"type":"solid","color":"#FFFFFF"}], - "stroke":{"thickness":1,"fill":[{"type":"solid","color":"#E2E8F0"}]}, - "effects":[{"type":"shadow","offsetY":2,"blur":6,"color":"#0000001A"}], - "children":[{"type":"path","name":"HeartIcon","width":18,"height":18}] - } - ] - }); - normalize_favorite_icon_buttons(&mut card); - let button = &card["children"][0]; - assert!( - button.get("fill").is_none(), - "heart should not sit in a white bubble" - ); - assert!( - button.get("stroke").is_none(), - "heart bubble border is visually noisy" - ); - assert!( - button.get("effects").is_none(), - "heart bubble shadow is visually noisy" - ); - assert!( - button.get("cornerRadius").is_none(), - "heart should not force a circular badge" - ); -} - #[test] fn non_ascii_color_does_not_panic() { // A malformed multi-byte color string must not panic the byte-slicer. @@ -409,6 +600,31 @@ fn horizontal_overflow_uses_fill_when_needed_width_nears_canvas() { assert_eq!(row["width"], json!("fill_container")); } +#[test] +fn horizontal_overflow_beyond_viewport_clips_instead_of_spilling() { + // The food-app category bug: 6 chips that physically can't fit a 375px phone. + // Widening is futile (children sum > canvas), so the row spans the viewport and + // clips at the edge instead of letting chips spill off-canvas into the void. + let mut row = json!({ + "type":"frame","layout":"horizontal","width":327,"gap":12, + "children":[ + {"type":"frame","width":46,"height":34}, + {"type":"frame","width":84,"height":34}, + {"type":"frame","width":85,"height":34}, + {"type":"frame","width":91,"height":34}, + {"type":"frame","width":89,"height":34}, + {"type":"frame","width":96,"height":34} + ] + }); + fix_horizontal_overflow(&mut row, 375.0); + assert_eq!(row["width"], json!("fill_container")); + assert_eq!( + row["clipContent"], + json!(true), + "an over-viewport horizontal row clips at the edge (scroll-row floor)" + ); +} + #[test] fn text_heights_removed_unless_fixed_width_height() { let mut root = json!({ diff --git a/crates/op-orchestrator/src/run.rs b/crates/op-orchestrator/src/run.rs index 77355f891..88245b21a 100644 --- a/crates/op-orchestrator/src/run.rs +++ b/crates/op-orchestrator/src/run.rs @@ -31,7 +31,7 @@ use crate::plan_repair::parse_orchestrator_response; use crate::prompt::build_orchestrator_prompt; use crate::retry::{attempt_modes, is_non_retryable}; use crate::run_dashboard::run_dashboard_path; -use crate::scaffold::{build_scaffold, build_scaffold_concurrent_mobile}; +use crate::scaffold::{build_scaffold, build_scaffold_concurrent_mobile, build_scaffold_reusing}; use crate::subagent::{apply_command_with_reveal, reveal_now_millis, run_subtask_with_reveal_at}; use crate::types::{ AbortFlag, DesignRequest, DocSink, LlmChunk, LlmClient, OrchestratorError, Progress, @@ -40,7 +40,44 @@ use crate::types::{ use crate::validation::run_post_generation_validation; use crate::variables::{rollback, seed_commands, snapshot_plan_vars}; use futures::StreamExt; -use op_editor_core::{EditorCommand, NodeId, PenNodeExt}; +use op_editor_core::{EditorCommand, EditorState, NodeId, PenNodeExt}; + +/// TS `replaceEmptyFrame` parity: detect a single EMPTY top-level frame (the +/// fresh-canvas starter) that can be REUSED as the design root instead of +/// inserting a brand-new root. Returns its id when the active page holds +/// exactly one empty container; `None` otherwise (multi-node canvas, filled +/// frame, or non-container) so the normal insert path runs. +fn detect_reusable_empty_frame(state: &EditorState) -> Option { + let kids = state.active_children(); + if kids.len() != 1 { + return None; + } + let node = &kids[0]; + if node.is_container() && node.children().map(|c| c.is_empty()).unwrap_or(true) { + Some(node.id_str().to_string()) + } else { + None + } +} + +/// Remove the fresh-canvas starter frame (a single empty top-level +/// container) so a path that builds its OWN root(s) doesn't leave it +/// orphaned beside the design. The sequential path REUSES it instead +/// (see [`detect_reusable_empty_frame`] + `build_scaffold_reusing`); the +/// concurrent (N roots) + dashboard (bespoke sidebar+main root) paths +/// can't reuse a single starter, so they clear it here. This mirrors the +/// host's former `clear_fresh_starter_frame_for_design` — now owned +/// orchestrator-side so the host can leave the starter in place for the +/// reuse path. No-op on a headless / empty canvas (op-smoke), where +/// there is no starter to detect. +fn clear_reusable_empty_frame(sink: &mut dyn DocSink) { + if let Some(id) = detect_reusable_empty_frame(sink.state()) { + sink.apply(EditorCommand::DeleteNode { + node_id: NodeId::new(id), + page_id: None, + }); + } +} /// 设计编排器。 #[derive(Debug, Default, Clone, Copy)] @@ -94,6 +131,17 @@ impl Orchestrator { let append_result = apply_append_context_to_plan(&mut plan, request.append_context.as_ref()); + // Surface the FULL planned task list upfront (TS parity) so the UI can + // render the complete checklist immediately, rather than revealing + // subtasks one-by-one as each starts. + on_progress(Progress::Planned { + subtasks: plan + .subtasks + .iter() + .map(|s| (s.id.clone(), s.label.clone())) + .collect(), + }); + // -- S3b-2 Task C2: concurrency branch decision -- // Port of `orchestrator.ts:780-810`. let screen_groups = group_subtasks_by_screen(&plan.subtasks); @@ -108,6 +156,9 @@ impl Orchestrator { }; if effective > 1 { + // Concurrent builds N screen roots — can't reuse the single + // fresh-canvas starter, so clear it (host no longer does). + clear_reusable_empty_frame(sink); return run_concurrent_path( request, plan, @@ -136,6 +187,14 @@ impl Orchestrator { for cmd in seed_commands(&plan, &var_snapshot) { sink.apply(cmd); } + // The dashboard path builds a bespoke sidebar+main root that can't + // reuse the fresh-canvas starter; clear it BEFORE indexing so the + // dashboard root lands at index 0 (not orphaned beside the starter). + // The sequential path below keeps the starter and REUSES it via + // ReplaceSubtree, so it must NOT be cleared here. + if !append_result.skip_root_insertion && use_dashboard { + clear_reusable_empty_frame(sink); + } let scaffold_root_index = sink.state().active_children().len(); let scaffold_root_ids_before: Vec = sink .state() @@ -187,7 +246,17 @@ impl Orchestrator { (target_id, baseline) } else { let effective_is_mobile = norm.is_mobile && !append_result.skip_status_bar; - match build_scaffold(&plan, effective_is_mobile) { + // TS `replaceEmptyFrame` parity: when the canvas is a single empty + // top-level frame (the fresh-canvas starter), REUSE it as the design + // root (ReplaceSubtree in place) instead of inserting a brand-new + // root — which the host would otherwise clear + re-add, the visible + // "delete then re-draw" flash the user flagged. + let reuse_id = detect_reusable_empty_frame(sink.state()); + let scaffold_cmds = match reuse_id.as_deref() { + Some(id) => build_scaffold_reusing(&plan, effective_is_mobile, id), + None => build_scaffold(&plan, effective_is_mobile), + }; + match scaffold_cmds { Ok(cmds) => { for cmd in cmds { if !apply_command_with_reveal( @@ -380,13 +449,18 @@ impl Orchestrator { break; } if zero { - // 零节点失败(非 abort,全部 3 次皆失败)—— 停止后续 subtask。 + // 零节点失败(非 abort,全部 3 次皆失败)。**不 break** —— 一个 + // section 失败不该放弃后续所有 subtask。各 subtask 独立 + // InsertSubtree 到 root、互不依赖;break 会把失败点之后的必要 + // 内容(bottom nav 等)全丢掉(用户报的"管线丢内容")。跳过这个、 + // 继续后面的;`zero_node_failure` 仍标记"至少一个失败",最终若 + // **全部**零内容(zero_content)才删 scaffold root。 on_progress(Progress::SubtaskFailed { id: subtask.id.clone(), error: err_msg.unwrap_or_default(), }); zero_node_failure = true; - break; + continue; } on_progress(Progress::SubtaskDone { id: subtask.id.clone(), diff --git a/crates/op-orchestrator/src/scaffold.rs b/crates/op-orchestrator/src/scaffold.rs index 4720c8a39..eebb31c14 100644 --- a/crates/op-orchestrator/src/scaffold.rs +++ b/crates/op-orchestrator/src/scaffold.rs @@ -218,6 +218,23 @@ fn mobile_status_bar_json(root_id: &str, fill_hex: &str, width: f64) -> serde_js /// /// 内部辅助函数,被 `build_scaffold`(单屏)和 /// `build_scaffold_concurrent`(多屏)共用。 +#[allow(clippy::too_many_arguments)] +/// Canonical inter-section spacing for the page-root vertical stack (Pencil's +/// reverse-engineered demo uses `gap: 20` between sections). The LLM frequently +/// omits the page gap, leaving it `0` so every section touches the next — the +/// cramped, no-breathing-room look that reads nothing like the TS references. +const SECTION_STACK_GAP: f64 = 20.0; + +/// Resolve the page-root section gap: honor an explicit positive plan gap, +/// otherwise fall back to [`SECTION_STACK_GAP`]. Mirrors the dashboard +/// main-column gap fallback (`root.gap > 0 ? root.gap : 20`). +fn resolve_section_gap(plan_gap: Option) -> f64 { + match plan_gap { + Some(g) if g > 0.0 => g, + _ => SECTION_STACK_GAP, + } +} + #[allow(clippy::too_many_arguments)] fn build_root_frame_node( id: &str, @@ -279,30 +296,60 @@ pub fn build_scaffold( plan: &OrchestratorPlan, is_mobile: bool, ) -> Result, String> { + let node = build_scaffold_root_node(plan, is_mobile, &plan.root_frame.id)?; + Ok(vec![EditorCommand::InsertSubtree { + nodes: vec![node], + parent_id: NodeId::NONE, + page_id: None, + }]) +} + +/// Reuse an existing EMPTY top-level frame (the fresh-canvas starter) as the +/// design root, mirroring TS `replaceEmptyFrame`: instead of clearing the +/// starter + inserting a brand-new root (the visible "delete + re-add" the +/// user flagged), REPLACE the starter frame's subtree in place. The scaffold +/// root takes the reused id, so its slot/identity is preserved and the canvas +/// fills smoothly rather than flashing empty. +pub fn build_scaffold_reusing( + plan: &OrchestratorPlan, + is_mobile: bool, + reuse_id: &str, +) -> Result, String> { + let node = build_scaffold_root_node(plan, is_mobile, reuse_id)?; + Ok(vec![EditorCommand::ReplaceSubtree { + node_id: NodeId::new(reuse_id.to_string()), + node: Box::new(node), + drop_children: true, + page_id: None, + }]) +} + +/// Build the root-frame node (status-bar child injected when `is_mobile`), +/// stamping `root_id` as its id so the caller can either insert it fresh or +/// replace an existing frame's slot with it. +fn build_scaffold_root_node( + plan: &OrchestratorPlan, + is_mobile: bool, + root_id: &str, +) -> 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 node = build_root_frame_node( - &rf.id, + build_root_frame_node( + root_id, &rf.name, SAFE_CANVAS_X, SAFE_CANVAS_Y, rf.width, rf.height, layout, - rf.gap.unwrap_or(0.0), + resolve_section_gap(rf.gap), &fill_hex, is_mobile, - )?; - - Ok(vec![EditorCommand::InsertSubtree { - nodes: vec![node], - parent_id: NodeId::NONE, - page_id: None, - }]) + ) } /// 构建 N-root-frame 并发画布搭建命令(多屏并发路径,S3b-2 Task A2)。 @@ -401,7 +448,7 @@ fn build_scaffold_concurrent_inner( rf.width, frame_height, layout, - rf.gap.unwrap_or(0.0), + resolve_section_gap(rf.gap), &fill_hex, is_mobile, )?; diff --git a/crates/op-orchestrator/src/scaffold_tests.rs b/crates/op-orchestrator/src/scaffold_tests.rs index a42317b03..0a167a8dc 100644 --- a/crates/op-orchestrator/src/scaffold_tests.rs +++ b/crates/op-orchestrator/src/scaffold_tests.rs @@ -87,6 +87,62 @@ fn build_scaffold_mobile_injects_status_bar() { } } +// ── Section-stack gap default (breathing room between sections) ───────────── + +#[test] +fn resolve_section_gap_defaults_to_20_when_unset_or_zero() { + // The LLM frequently omits the page gap (None) or emits 0 → sections touch. + // Both fall back to the canonical 20 px so the page breathes like the TS refs. + assert_eq!(resolve_section_gap(None), 20.0); + assert_eq!(resolve_section_gap(Some(0.0)), 20.0); + // An explicit positive gap is the design's intent — honor it. + assert_eq!(resolve_section_gap(Some(12.0)), 12.0); + assert_eq!(resolve_section_gap(Some(32.0)), 32.0); +} + +#[test] +fn build_scaffold_root_gap_breathes_when_plan_gap_is_zero() { + // plan() authors gap: Some(0.0); the scaffold root must still carry 20. + let cmds = build_scaffold(&plan(), true).expect("scaffold"); + match &cmds[0] { + EditorCommand::InsertSubtree { nodes, .. } => { + let json = serde_json::to_value(&nodes[0]).expect("root json"); + assert_eq!(json["gap"], 20.0, "zero plan gap must breathe at 20px"); + } + other => panic!("expected InsertSubtree, got {other:?}"), + } +} + +// ── TS `replaceEmptyFrame` parity: reuse the starter in place ─────────────── + +#[test] +fn build_scaffold_reusing_emits_replace_subtree_with_reused_id() { + // The fresh-canvas starter has a different id ("n10") than the planned + // root ("root"); reuse must stamp the built root with the STARTER's id + // (so the slot is preserved in place) and replace its subtree — never + // insert a brand-new root beside it. + let cmds = build_scaffold_reusing(&plan(), false, "n10").expect("reuse scaffold"); + assert_eq!(cmds.len(), 1); + match &cmds[0] { + EditorCommand::ReplaceSubtree { + node_id, + node, + drop_children, + .. + } => { + assert_eq!(node_id.as_str(), "n10"); + assert!( + *drop_children, + "must drop the empty starter's (absent) children" + ); + // The built root carries the reused id, not the plan's "root". + assert_eq!(node.id_str(), "n10"); + assert!(node.children().map(|c| c.is_empty()).unwrap_or(true)); + } + other => panic!("expected ReplaceSubtree, got {other:?}"), + } +} + #[test] fn build_scaffold_mobile_status_bar_uses_fixed_icon_positions() { let cmds = build_scaffold(&plan(), true).expect("scaffold"); diff --git a/crates/op-orchestrator/src/subagent.rs b/crates/op-orchestrator/src/subagent.rs index 3ccce3d16..88b01633b 100644 --- a/crates/op-orchestrator/src/subagent.rs +++ b/crates/op-orchestrator/src/subagent.rs @@ -82,12 +82,18 @@ pub(crate) async fn run_subtask_with_reveal_at( reduced_complexity, minimal_skills, ); + tracing::debug!(subtask = %subtask.id, model = req.model.as_deref().unwrap_or(""), "subagent LLM call"); let mut stream = llm.call(call_req); let mut text = String::new(); + let mut thinking_len = 0usize; while let Some(item) = stream.next().await { match item { Ok(LlmChunk::Text(t)) => text.push_str(&t), - Ok(LlmChunk::Thinking(_)) => {} + // Thinking models (glm-5.2 etc.) stream reasoning here. We don't + // parse it, but track its size: a large reasoning blob with an + // empty `text` is the classic "no JSON found" failure mode — the + // model spent its budget thinking and never emitted the answer. + Ok(LlmChunk::Thinking(t)) => thinking_len += t.len(), Err(e) => { return fail(if e.aborted { "aborted".into() @@ -97,6 +103,12 @@ pub(crate) async fn run_subtask_with_reveal_at( } } } + tracing::debug!( + subtask = %subtask.id, + text_len = text.len(), + thinking_len, + "subagent text collected" + ); // 解析成 PenNode 树。manifest 模式(按模型路由 + `OPENPENCIL_MANIFEST` // override)先按元素清单解析;文本里没有清单行(如重试梯度回落到裸 @@ -116,13 +128,31 @@ pub(crate) async fn run_subtask_with_reveal_at( } None => match parse_nodes(&text) { Ok(n) => n, - Err(e) => return fail(e.to_string()), + Err(e) => { + tracing::warn!( + subtask = %subtask.id, + text_len = text.len(), + thinking_len, + raw = %text, + "subagent parse failed (manifest + JSONL both missed)" + ); + return fail(e.to_string()); + } }, } } else { match parse_nodes(&text) { Ok(n) => n, - Err(e) => return fail(e.to_string()), + Err(e) => { + tracing::warn!( + subtask = %subtask.id, + text_len = text.len(), + thinking_len, + raw = %text, + "subagent parse failed" + ); + return fail(e.to_string()); + } } }; if is_blank_container_forest(&nodes) { @@ -147,11 +177,48 @@ pub(crate) async fn run_subtask_with_reveal_at( .filter(|c| !c.is_empty()); crate::role_defaults::detect_theme_from_fill(first_solid) }; + // Weak-model deterministic floor (runs BEFORE role resolution so roles land + // on the cleaned forest): a subtask is ONE section, but weak models split it + // into sibling pieces — an empty wrapper + the content/leaves that belong + // inside it. Reparent those back so they don't normalize into separate + // full-width page bands (empty banner + floating invisible text, etc.). + coalesce_subtask_section(&mut nodes); crate::role_infer::resolve_forest_roles(&mut nodes, canvas_width, theme); // Cross-node contrast post-pass (I3) runs AFTER role resolution (it keys off // the roles I1/I2 set) and before the fallback sizing normalize. crate::role_post_pass::post_pass_forest(&mut nodes, canvas_width); + // Post-streaming tree heuristics (TS applyPostStreamingTreeHeuristics parity): + // nav-surface anchor, redundant section-fill strip, nested-card decoration + // strip. Runs BEFORE binding — the section-fill strip matches literal hedge + // HEX that binding would convert to refs. Each forest root is a page-root + // child (a section). + let page_bg = plan.root_frame.first_solid_hex(); + // Dominant brand accent from the already-assembled page (prior subtasks live + // in the sink) — drives the invisible-band fill so it matches the screen's + // real accent (often a chart token) instead of a clashing palette default. + let prior_accent = { + let st = sink.state(); + let roots = st + .doc + .pages + .as_ref() + .and_then(|p| p.get(st.ui.active_page_index)) + .map(|pg| pg.children.as_slice()) + .unwrap_or(st.doc.children.as_slice()); + crate::tree_heuristics::dominant_design_accent(roots) + }; + crate::tree_heuristics::apply_tree_heuristics( + &mut nodes, + page_bg.as_deref(), + theme == crate::role_defaults::Theme::Light, + prior_accent.as_deref(), + ); crate::variable_binding::bind_generated_color_variables(&mut nodes, sink.state()); + // Surface-color discipline runs AFTER binding: glm emits raw hex, and binding + // is what turns it into the `$color-danger-bg` / `$color-bg-deep` refs this + // pass matches (recolor misused state-bg surfaces → neutral, strip the + // page-bg token off inner wrappers). Pre-binding it only saw hex and missed. + crate::role_post_pass::enforce_surface_color_discipline(&mut nodes); normalize_section_roots_for_parent_layout(&mut nodes); let node_count = nodes.len(); @@ -369,6 +436,113 @@ fn has_content_node(node: &PenNode) -> bool { } } +/// The content slot a split-out section piece should be reparented into: +/// `primary` itself if it is an empty wrapper, else its FIRST empty direct-child +/// container (the "Dish Row" / "Card List" the model emitted but left empty). +/// Depth is capped at one on purpose — a deeply-nested incidental empty frame +/// (an await-input box, a spacer) must not swallow whole sections. +fn section_content_slot(primary: &mut PenNode) -> Option<&mut Vec> { + let primary_empty = primary.children().map(|c| c.is_empty()).unwrap_or(true); + if primary_empty { + return primary.children_mut(); + } + primary + .children_mut()? + .iter_mut() + .find(|c| { + matches!(c, PenNode::Frame(_) | PenNode::Group(_)) + && c.children().map(|cc| cc.is_empty()).unwrap_or(true) + }) + .and_then(|c| c.children_mut()) +} + +/// Weak-model deterministic floor (TS `applyPostStreamingTreeHeuristics` +/// spirit): a subtask maps to ONE section, but glm-class models routinely split +/// a section into sibling pieces — an (often empty) wrapper plus the content +/// roots that belong inside it (e.g. an empty `Promo Banner` next to a +/// `Promo Content` text block and a `Promo Food Image`; an empty `Dish Row` next +/// to the dish cards). Each forest root is later normalized into a full-width +/// page band ([`normalize_section_roots_for_parent_layout`]), so the wrapper +/// renders as a blank gap while its content floats with no background — invisible +/// white-on-cream text, the broken Featured/Promo screens the user flagged. +/// +/// Resolution, keyed off the first section container (Frame/Group) as the +/// "primary" section: +/// - **Wrapper slot present** — the primary is empty, or has an empty direct +/// child container waiting for content: reparent every orphan root into that +/// slot (forest order preserved). Fixes the empty-banner / empty-row splits. +/// - **No slot, orphans all leaves or EMPTY containers** — stray decorations a +/// header / section spilled outside itself (a bell icon, a heading, an empty +/// notification-badge frame): leaf/heading orphans fold into the primary +/// (leading prepended, trailing appended) and childless containers are dropped +/// (they would only normalize into a blank full-width band). +/// - **No slot, an orphan is a POPULATED container (≥1 child)** — a legitimate +/// multi-section forest with nowhere to nest: left untouched so real sections +/// are never collapsed into one another. +fn coalesce_subtask_section(nodes: &mut Vec) { + if nodes.len() < 2 { + return; + } + let Some(primary_idx) = nodes + .iter() + .position(|n| matches!(n, PenNode::Frame(_) | PenNode::Group(_))) + else { + return; // no section container to reparent into. + }; + // An orphan is a stray DECORATION to fold (not a real sibling section) when + // it's a non-container leaf (icon / badge text), OR an EMPTY container — a + // childless `Notification Badge` frame the model hung OUTSIDE the header that + // would otherwise normalize into a blank full-width band. A container that + // actually holds content (≥1 child) is a genuine section → bail (never + // collapse real sibling sections into one another). + let kid_count = |n: &PenNode| n.children().map(|c| c.len()).unwrap_or(0); + let orphans_all_foldable = nodes + .iter() + .enumerate() + .filter(|(i, _)| *i != primary_idx) + .all(|(_, n)| !n.is_container() || kid_count(n) == 0); + + // Split into before / primary / after, preserving forest order. Empty + // containers (0 children — junk like a stray empty badge that would + // normalize into a blank full-width band) are dropped rather than folded. + let taken = std::mem::take(nodes); + let mut before: Vec = Vec::new(); + let mut after: Vec = Vec::new(); + let mut primary: Option = None; + let keep = |n: &PenNode| !n.is_container() || kid_count(n) > 0; + for (i, node) in taken.into_iter().enumerate() { + match i.cmp(&primary_idx) { + std::cmp::Ordering::Less if keep(&node) => before.push(node), + std::cmp::Ordering::Equal => primary = Some(node), + std::cmp::Ordering::Greater if keep(&node) => after.push(node), + _ => {} // dropped empty-container orphan + } + } + let mut primary = primary.expect("primary index was computed from nodes"); + + if section_content_slot(&mut primary).is_some() { + // Wrapper slot: drop the split-out pieces into it, forest order kept. + let slot = section_content_slot(&mut primary).expect("slot present"); + slot.extend(before); + slot.extend(after); + } else if orphans_all_foldable { + // Stray decorations: heading-before prepends, badge/icon-after appends. + if let Some(kids) = primary.children_mut() { + let existing = std::mem::take(kids); + kids.extend(before); + kids.extend(existing); + kids.extend(after); + } + } else { + // Legitimate multi-section forest — restore original order, change nothing. + nodes.extend(before); + nodes.push(primary); + nodes.extend(after); + return; + } + nodes.push(primary); +} + fn normalize_section_roots_for_parent_layout(nodes: &mut [PenNode]) { for node in nodes { node.base_mut().x = None; @@ -450,6 +624,155 @@ mod tests { const NODE_JSON: &str = r#"[{"type":"frame","id":"hero-1","name":"Card","x":0,"y":0,"width":1200,"height":200,"children":[{"type":"text","id":"hero-title","content":"Hero","fontSize":18}]}]"#; + #[test] + fn coalesce_folds_trailing_badge_leaf_into_lone_section() { + // glm shape: a populated Top Bar section frame + a stray cart-count + // badge "3" emitted as a SIBLING. No empty slot exists, the orphan is a + // leaf → it appends into the section, not survive as a floating "3" band. + let json = r#"[ + {"type":"frame","id":"s1","name":"Top Bar","width":"fill_container","height":"fit_content","layout":"horizontal","children":[ + {"type":"text","id":"loc","content":"Home"} + ]}, + {"type":"text","id":"badge","content":"3"} + ]"#; + let mut nodes: Vec = serde_json::from_str(json).expect("parse forest"); + coalesce_subtask_section(&mut nodes); + assert_eq!(nodes.len(), 1, "badge must fold into the lone section"); + let kids = nodes[0].children().expect("section children"); + assert_eq!(kids.len(), 2); + assert_eq!(kids[0].id_str(), "loc"); + assert_eq!( + kids[1].id_str(), + "badge", + "trailing badge nests as last child" + ); + } + + #[test] + fn coalesce_prepends_leading_leaf_as_section_heading() { + // A heading text emitted BEFORE a populated content frame folds in as the + // FIRST child (preserving the intended heading-above-content order). + let json = r#"[ + {"type":"text","id":"head","content":"Featured"}, + {"type":"frame","id":"sec","name":"List","children":[{"type":"text","id":"item","content":"x"}]} + ]"#; + let mut nodes: Vec = serde_json::from_str(json).expect("parse forest"); + coalesce_subtask_section(&mut nodes); + assert_eq!(nodes.len(), 1); + let kids = nodes[0].children().expect("section children"); + assert_eq!( + kids[0].id_str(), + "head", + "leading leaf prepended as heading" + ); + assert_eq!(kids[1].id_str(), "item"); + } + + #[test] + fn coalesce_fills_empty_wrapper_with_split_pieces() { + // tt5 Promo shape: an EMPTY `Promo Banner` wrapper emitted first, with + // its `Promo Content` (text) + `Promo Food Image` hung as sibling roots. + // They must reparent INTO the empty banner (forest order kept) so the + // banner stops rendering as a blank gap with floating invisible text. + let json = r#"[ + {"type":"frame","id":"banner","name":"Promo Banner","layout":"vertical","children":[]}, + {"type":"frame","id":"content","name":"Promo Content","children":[{"type":"text","id":"title","content":"Get 30% off"}]}, + {"type":"image","id":"img","name":"Promo Food Image","src":""} + ]"#; + let mut nodes: Vec = serde_json::from_str(json).expect("parse forest"); + coalesce_subtask_section(&mut nodes); + assert_eq!(nodes.len(), 1, "split pieces fold into the empty wrapper"); + assert_eq!(nodes[0].id_str(), "banner"); + let kids = nodes[0].children().expect("banner children"); + assert_eq!(kids.len(), 2); + assert_eq!(kids[0].id_str(), "content"); + assert_eq!(kids[1].id_str(), "img"); + } + + #[test] + fn coalesce_fills_empty_direct_child_row_with_orphan_cards() { + // tt5 Popular Dishes shape: section with a Header + an EMPTY `Dish Row` + // direct child, and the dish cards hung as sibling roots. The cards must + // land inside the empty row, not flatten into separate page bands. + let json = r#"[ + {"type":"frame","id":"sec","name":"Popular Dishes","children":[ + {"type":"frame","id":"header","name":"Header","children":[{"type":"text","id":"t","content":"Popular Dishes"}]}, + {"type":"frame","id":"row","name":"Dish Row","children":[]} + ]}, + {"type":"image","id":"pizza","name":"Margherita Pizza","src":""}, + {"type":"frame","id":"pinfo","name":"Info","children":[{"type":"text","id":"pn","content":"Margherita"}]} + ]"#; + let mut nodes: Vec = serde_json::from_str(json).expect("parse forest"); + coalesce_subtask_section(&mut nodes); + assert_eq!( + nodes.len(), + 1, + "cards fold into the section, not the page root" + ); + let sec_kids = nodes[0].children().expect("section children"); + assert_eq!(sec_kids.len(), 2, "Header + Dish Row preserved"); + let row = &sec_kids[1]; + assert_eq!(row.id_str(), "row"); + let row_kids = row.children().expect("dish row children"); + assert_eq!( + row_kids.len(), + 2, + "both orphan cards landed in the empty row" + ); + assert_eq!(row_kids[0].id_str(), "pizza"); + assert_eq!(row_kids[1].id_str(), "pinfo"); + } + + #[test] + fn coalesce_leaves_populated_multi_section_forest_untouched() { + // Two POPULATED section containers with no empty slot → a legitimate + // multi-section forest; never collapse real sections into one another. + let json = r#"[ + {"type":"frame","id":"a","name":"A","children":[{"type":"text","id":"ta","content":"A"}]}, + {"type":"frame","id":"b","name":"B","children":[{"type":"text","id":"tb","content":"B"}]} + ]"#; + let mut nodes: Vec = serde_json::from_str(json).expect("parse forest"); + coalesce_subtask_section(&mut nodes); + assert_eq!(nodes.len(), 2, "two populated sections must be left as-is"); + } + + #[test] + fn coalesce_folds_stray_icon_and_drops_empty_badge() { + // tt5 header: the Bell icon (leaf) + an EMPTY Notification Badge frame + // were emitted as top-level SIBLINGS of the header (bell floated below + // the search; the empty badge would normalize into a blank full-width + // band). The icon folds into the header; the empty badge is dropped — + // neither survives as a floating page section. + let json = r#"[ + {"type":"frame","id":"hdr","name":"Header & Search","children":[ + {"type":"frame","id":"loc","name":"Location & Actions","children":[{"type":"text","id":"l","content":"NYC"}]}, + {"type":"frame","id":"sb","name":"Search Bar","children":[{"type":"text_input","id":"si","placeholder":"Search"}]} + ]}, + {"type":"icon_font","id":"bell","iconFontName":"bell"}, + {"type":"frame","id":"badge","name":"Notification Badge","children":[]} + ]"#; + let mut nodes: Vec = serde_json::from_str(json).expect("parse forest"); + coalesce_subtask_section(&mut nodes); + assert_eq!( + nodes.len(), + 1, + "only the header section survives at top level" + ); + let kids = nodes[0].children().expect("header children"); + assert!( + kids.iter().any(|k| matches!(k, PenNode::IconFont(_))), + "stray bell icon folded into the header" + ); + let names: Vec<&str> = kids + .iter() + .filter_map(|k| k.base().name.as_deref()) + .collect(); + assert!( + !names.contains(&"Notification Badge"), + "empty badge dropped, not folded as a blank band" + ); + } + #[test] fn run_subtask_ok_applies_insert_subtree() { let llm = ScriptedLlm::new(vec![ScriptResponse::Text(NODE_JSON.into())]); diff --git a/crates/op-orchestrator/src/tree_heuristics.rs b/crates/op-orchestrator/src/tree_heuristics.rs new file mode 100644 index 000000000..aafc4aed8 --- /dev/null +++ b/crates/op-orchestrator/src/tree_heuristics.rs @@ -0,0 +1,1020 @@ +//! Post-streaming tree heuristics — a faithful port of the cleanup passes +//! TS runs in `applyPostStreamingTreeHeuristics` (design-canvas-ops.ts) via +//! `packages/pen-core/src/layout/*.ts`. These run on the ASSEMBLED page in TS; +//! the Rust orchestrator runs the role pipeline PER-SUBTASK, and each subtask +//! forest root is a direct child of the page root — i.e. a section. So the +//! per-section passes here are applied to each forest root, which reproduces +//! the TS effect (iterating the page root's children) without a page-assembly +//! hook. +//! +//! Ported passes (HIGH/MED visual impact per the 2026-06-19 TS↔Rust parity +//! audit — the Rust port had NONE of these, degrading every model incl. Opus): +//! - `inject_missing_nav_surface_fill` — anchor a transparent nav bar with a +//! surface fill + lift shadow (port of inject-nav-surface-fill.ts). +//! - `strip_redundant_section_fill` — drop a section wrapper's hedge fill +//! (safe-dark / safe-light hex) + the wrapper chrome that travels with it +//! (port of strip-redundant-section-fills.ts). Matches HEX, so it must run +//! BEFORE variable binding. +//! - `strip_nested_card_decoration` — strip a nested card's redundant +//! stroke / cornerRadius / shadow when an ancestor already carries it +//! (port of strip-nested-card-decoration.ts) — kills box-in-box borders. + +use jian_ops_schema::node::PenNode; +use serde_json::{json, Value}; + +// ── shared tiny helpers (self-contained; mirror role_post_pass) ────────────── + +fn role_of(node: &Value) -> Option<&str> { + node.get("role").and_then(Value::as_str) +} + +fn first_solid_color(node: &Value) -> Option { + let fill = node.get("fill")?; + if let Some(s) = fill.as_str() { + return Some(s.to_string()); + } + let arr = fill.as_array()?; + let first = arr.first()?; + if first.get("type").and_then(Value::as_str) == Some("solid") { + return first + .get("color") + .and_then(Value::as_str) + .map(str::to_string); + } + None +} + +/// Will the first fill paint a visible color? (port of `hasAnyFill`). A truthy +/// `type` alone isn't enough — `{type:solid}` with no/empty color, or an +/// unknown variant, render as transparent and count as "no fill". +fn has_renderable_fill(node: &Value) -> bool { + let Some(fill) = node.get("fill") else { + return false; + }; + if let Some(s) = fill.as_str() { + return !s.is_empty(); + } + let Some(arr) = fill.as_array() else { + return false; + }; + let Some(first) = arr.first() else { + return false; + }; + match first.get("type").and_then(Value::as_str) { + Some("solid") => first + .get("color") + .and_then(Value::as_str) + .map(|c| !c.is_empty()) + .unwrap_or(false), + Some("linear_gradient") | Some("radial_gradient") => first + .get("stops") + .and_then(Value::as_array) + .map(|s| !s.is_empty()) + .unwrap_or(false), + Some("image") => first + .get("src") + .and_then(Value::as_str) + .map(|s| !s.is_empty()) + .unwrap_or(false), + _ => false, + } +} + +fn children_of(node: &Value) -> &[Value] { + node.get("children") + .and_then(Value::as_array) + .map(Vec::as_slice) + .unwrap_or(&[]) +} + +fn child_count(node: &Value) -> usize { + children_of(node).len() +} + +// ── inject_missing_nav_surface_fill (port of inject-nav-surface-fill.ts) ────── + +// Surface-fill injection is for a BOTTOM nav / tab bar that floats over content +// and needs a surface to read against the page. A TOP header (`navbar` / +// `top-nav-bar` / `top-app-bar`) is intentionally transparent on mobile (the TS +// references), and `fix_structural_wrapper_transparency` already strips its +// "white-card-on-cream" fill — so they must NOT be re-filled here (that fight is +// exactly what re-boxed the mobile header). Desktop top-nav fills still come +// from `role_defaults::navbar`. +const NAV_ROLES: &[&str] = &["nav", "tab-bar", "bottom-tab-bar", "tab-row"]; +const BOTTOM_NAV_ROLES: &[&str] = &["bottom-tab-bar"]; + +fn apply_nav_surface_fill(nav: &mut Value, role: &str) -> bool { + if has_renderable_fill(nav) { + return false; + } + let Some(obj) = nav.as_object_mut() else { + return false; + }; + obj.insert( + "fill".to_string(), + json!([{ "type": "solid", "color": "$color-surface" }]), + ); + // Lift shadow only when no effects were authored. Bottom nav → shadow + // points up (offsetY < 0); top/other nav → down. + let has_effects = obj + .get("effects") + .and_then(Value::as_array) + .map(|e| !e.is_empty()) + .unwrap_or(false); + if !has_effects { + let is_bottom = BOTTOM_NAV_ROLES.contains(&role); + obj.insert( + "effects".to_string(), + json!([{ + "type": "shadow", "offsetX": 0, "offsetY": if is_bottom { -4 } else { 4 }, + "blur": 12, "spread": 0, "color": "#0000000F" + }]), + ); + } + true +} + +/// Apply to one page-root child (a section / nav). Handles the nav-as-root +/// case AND the section-wrapping-a-single-nav case (one hop). +fn inject_nav_surface_for_section(node: &mut Value) { + let role = role_of(node).map(str::to_string); + if let Some(r) = role.as_deref() { + if NAV_ROLES.contains(&r) { + apply_nav_surface_fill(node, r); + return; + } + if r == "section" && child_count(node) == 1 { + // wrapper section around a single nav child — one hop in. + let inner_role = children_of(node) + .first() + .and_then(role_of) + .map(str::to_string); + if let Some(ir) = inner_role { + if NAV_ROLES.contains(&ir.as_str()) { + if let Some(inner) = node + .get_mut("children") + .and_then(Value::as_array_mut) + .and_then(|a| a.first_mut()) + { + apply_nav_surface_fill(inner, &ir); + } + } + } + } + } +} + +// ── strip_redundant_section_fill (port of strip-redundant-section-fills.ts) ─── + +const PROTECTED_ROLES: &[&str] = &[ + "card", + "stat-card", + "pricing-card", + "feature-card", + "image-card", + "testimonial", + "button", + "icon-button", + "badge", + "chip", + "tag", + "pill", + "input", + "form-input", + "search-bar", + "phone-mockup", + "banner", + "metric-card", + "gallery-item", + "status-bar", + "navbar", + "nav", + "tab-bar", + "bottom-tab-bar", + "top-nav-bar", +]; +const ATOMIC_PROTECTED_ROLES: &[&str] = &[ + "button", + "icon-button", + "badge", + "chip", + "tag", + "pill", + "input", + "form-input", + "search-bar", +]; +const PRIMARY_ATOMIC_ROLES: &[&str] = &["input", "form-input", "search-bar"]; +const STRUCTURAL_ROLES: &[&str] = &[ + "section", + "row", + "column", + "stack", + "container", + "content-area", + "section-header", + "wrapper", + "group", + "hero", + "footer", + "cta-section", + "stats-section", +]; +const CONTAINER_PROTECTED_ROLES: &[&str] = &[ + "card", + "stat-card", + "pricing-card", + "feature-card", + "image-card", + "testimonial", + "banner", + "metric-card", + "gallery-item", + "phone-mockup", +]; +const SAFE_DARK_HEXES: &[&str] = &[ + "#000000", "#000", "#0a0a0a", "#0f0f0f", "#111", "#111111", "#121212", "#141414", "#1a1a1a", + "#181818", "#1c1c1c", "#1e1e1e", "#202020", +]; +const SAFE_LIGHT_HEXES: &[&str] = &[ + "#ffffff", "#fff", "#fefefe", "#fdfdfd", "#fcfcfc", "#fafafa", "#f9fafb", "#f8f8f8", "#f8fafc", + "#f5f5f5", "#f4f4f5", "#f3f4f6", +]; + +fn normalize_hex(color: &str) -> String { + let mut c = color.trim().to_lowercase(); + if c.len() == 9 && c.starts_with('#') { + c.truncate(7); + } + c +} + +fn has_multiple_same_role_children(node: &Value, parent_role: &str) -> bool { + if !CONTAINER_PROTECTED_ROLES.contains(&parent_role) { + return false; + } + let mut n = 0; + for c in children_of(node) { + if role_of(c) == Some(parent_role) { + n += 1; + if n >= 2 { + return true; + } + } + } + false +} + +fn has_nested_filled_component(node: &Value, parent_role: &str) -> bool { + if !ATOMIC_PROTECTED_ROLES.contains(&parent_role) { + return false; + } + for c in children_of(node) { + let Some(cr) = role_of(c) else { continue }; + if cr == parent_role { + return true; + } + if PRIMARY_ATOMIC_ROLES.contains(&cr) && first_solid_color(c).is_some() { + return true; + } + } + false +} + +fn is_section_level_frame(node: &Value) -> bool { + let Some(role) = role_of(node) else { + return true; // unrolled section root + }; + if PROTECTED_ROLES.contains(&role) { + if has_nested_filled_component(node, role) { + return true; + } + if has_multiple_same_role_children(node, role) { + return true; + } + return false; + } + if STRUCTURAL_ROLES.contains(&role) { + return true; + } + false +} + +fn should_strip_fill(child_fill: &str, root_fill: Option<&str>) -> bool { + let child_key = normalize_hex(child_fill); + if let Some(rf) = root_fill { + if child_key == normalize_hex(rf) { + return true; + } + } + SAFE_DARK_HEXES.contains(&child_key.as_str()) || SAFE_LIGHT_HEXES.contains(&child_key.as_str()) +} + +/// Per-section decision (the body of the TS loop, applied to one page-root +/// child). `page_bg` is the page root's fill hex when known. +fn strip_redundant_section_fill(node: &mut Value, page_bg: Option<&str>) { + if node.get("type").and_then(Value::as_str) != Some("frame") { + return; + } + if !is_section_level_frame(node) { + return; + } + let Some(child_fill) = first_solid_color(node) else { + return; + }; + if should_strip_fill(&child_fill, page_bg) { + if let Some(obj) = node.as_object_mut() { + obj.remove("fill"); + obj.remove("stroke"); + obj.remove("cornerRadius"); + } + } +} + +// ── strip_nested_card_decoration (port of strip-nested-card-decoration.ts) ──── + +const KEEP_DECORATION_ROLES: &[&str] = &[ + "button", + "icon-button", + "fab", + "tag", + "chip", + "badge", + "status-badge", + "pill", + "input", + "search-bar", + "form-field", + "textarea", + "select", + "combobox", + "avatar", + "avatar-stack", + "switch", + "checkbox", + "radio", + "toolbar", + "segmented-control", +]; +const MEDIA_CLIP_ROLES: &[&str] = &[ + "image", + "image-card", + "image-placeholder", + "video", + "video-placeholder", + "media", + "media-thumbnail", + "thumbnail", + "cover", + "cover-image", + "gallery-item", +]; + +#[derive(Clone, Copy, Default)] +struct DecoFlags { + stroke: bool, + corner: bool, + shadow: bool, +} + +fn read_decoration(node: &Value) -> DecoFlags { + let stroke = node + .get("stroke") + .and_then(|s| s.get("thickness")) + .and_then(Value::as_f64) + .map(|t| t > 0.0) + .unwrap_or(false); + let corner = match node.get("cornerRadius") { + Some(Value::Number(n)) => n.as_f64().unwrap_or(0.0) > 0.0, + Some(Value::Array(a)) => a.first().and_then(Value::as_f64).unwrap_or(0.0) > 0.0, + _ => false, + }; + let shadow = node + .get("effects") + .and_then(Value::as_array) + .map(|e| { + e.iter() + .any(|x| x.get("type").and_then(Value::as_str) == Some("shadow")) + }) + .unwrap_or(false); + DecoFlags { + stroke, + corner, + shadow, + } +} + +fn role_lower(node: &Value) -> String { + role_of(node).unwrap_or("").to_lowercase() +} + +fn is_role_protected(node: &Value) -> bool { + KEEP_DECORATION_ROLES.contains(&role_lower(node).as_str()) +} + +fn is_media_clipper(node: &Value) -> bool { + let role = role_lower(node); + if MEDIA_CLIP_ROLES.contains(&role.as_str()) { + return true; + } + if node.get("clipContent").and_then(Value::as_bool) != Some(true) { + return false; + } + children_of(node).iter().any(|c| { + c.get("type").and_then(Value::as_str) == Some("image") + || MEDIA_CLIP_ROLES.contains(&role_lower(c).as_str()) + }) +} + +fn strip_nested_card_decoration(node: &mut Value, ancestor: DecoFlags) { + let is_frame = node.get("type").and_then(Value::as_str) == Some("frame"); + let mut next_ancestor = ancestor; + if is_frame { + let own = read_decoration(node); + if !is_role_protected(node) { + let media = is_media_clipper(node); + if let Some(obj) = node.as_object_mut() { + if own.stroke && ancestor.stroke { + obj.remove("stroke"); + } + if own.corner && ancestor.corner && !media { + obj.remove("cornerRadius"); + } + if own.shadow && ancestor.shadow { + obj.remove("effects"); + } + } + } + // Accumulate THIS node's (original) decoration for descendants. + next_ancestor = DecoFlags { + stroke: ancestor.stroke || own.stroke, + corner: ancestor.corner || own.corner, + shadow: ancestor.shadow || own.shadow, + }; + } + if let Some(children) = node.get_mut("children").and_then(Value::as_array_mut) { + for child in children.iter_mut() { + strip_nested_card_decoration(child, next_ancestor); + } + } +} + +// ── fix_invisible_text_band (glm banner-fill floor, not a TS pass) ─────────── +// +// glm intermittently designs a promo banner with WHITE text but omits the +// colored/gradient fill it intended (gen2/3/5: "Get 30% Off" white-on-cream = +// invisible). Deterministic floor: on a LIGHT page, a fill-less container whose +// text descendants are ALL white/light → it was meant to sit on a colored +// surface → stamp `$color-accent` so the copy becomes readable. Conservative: +// fires only when every text is light (no dark text to contradict) and the +// container truly has no renderable fill. + +// Light text tokens/hexes that vanish on a light page — white + the neutral +// surface tints (a banner headline written in any of these needs a colored +// surface beneath it). +const LIGHT_TEXT_REFS: &[&str] = &[ + "$color-surface", + "$color-surface-2", + "$color-surface-3", + "$color-bg-deep", +]; +const LIGHT_TEXT_HEXES: &[&str] = &["#ffffff", "#fff", "#fefefe", "#fdfdfd", "white"]; + +fn is_light_text(color: &str) -> bool { + if LIGHT_TEXT_REFS.contains(&color) { + return true; + } + LIGHT_TEXT_HEXES.contains(&normalize_hex(color).as_str()) +} + +/// Tally text colors that sit DIRECTLY on this container's (unfilled) surface. +/// Do NOT descend into a child that carries its own renderable fill — a button +/// / avatar / chip has its own surface, so its text colour says nothing about +/// whether THIS container needs a fill (e.g. a promo banner's white headline + +/// an orange-text "Order Now" button on a white pill: only the headline counts). +fn tally_surface_text_colors(node: &Value, light: &mut usize, dark: &mut usize) { + for child in children_of(node) { + if child.get("type").and_then(Value::as_str) == Some("text") { + if let Some(c) = first_solid_color(child) { + if is_light_text(&c) { + *light += 1; + } else { + *dark += 1; + } + } + } else if !has_renderable_fill(child) { + tally_surface_text_colors(child, light, dark); + } + } +} + +/// The emphasis color the design ACTUALLY uses (glm often repurposes a chart +/// token as the brand accent because the palette's `$color-accent` defaults to +/// blue — wrong for a warm app). Returns the first chart/accent/primary token +/// found in the subtree, so the band matches the rest of the screen instead of +/// stamping a clashing default blue. +fn find_design_accent(node: &Value) -> Option { + if let Some(c) = first_solid_color(node) { + if c == "$color-accent" + || c == "$color-primary" + || c == "$color-brand" + || c.starts_with("$color-chart-") + { + return Some(c); + } + } + for child in children_of(node) { + if let Some(c) = find_design_accent(child) { + return Some(c); + } + } + None +} + +/// True when a solid fill color reads as a light page/surface tone — light/white +/// text on it is invisible. Covers the neutral surface variable refs (binding +/// hasn't resolved them at this pass) plus white-ish hexes. +fn is_light_surface_color(color: &str) -> bool { + matches!( + color, + "$color-surface" | "$color-surface-2" | "$color-surface-3" | "$color-bg-deep" + ) || SAFE_LIGHT_HEXES.contains(&normalize_hex(color).as_str()) +} + +fn fix_invisible_text_band(node: &mut Value, light_theme: bool, design_accent: &str) { + if !light_theme { + return; // white text on a dark page is fine + } + if node.get("type").and_then(Value::as_str) != Some("frame") { + return; + } + // Skip only when the node ALREADY paints a non-light surface (a colored or + // dark solid, or a gradient / image) — light text reads fine there. A node + // with NO fill, OR a LIGHT-SURFACE solid fill (`$color-surface`, white), is a + // band where light text is invisible. The latter is the broken-promo-banner + // case: glm gives the card white text + a dark CTA + a translucent-white + // badge (all implying a colored background) yet fills the card with + // `$color-surface`, so the headline vanishes. Repaint with the design accent. + if has_renderable_fill(node) { + match first_solid_color(node) { + Some(c) if is_light_surface_color(&c) => {} // light surface → still invisible + _ => return, // colored/dark solid or gradient → real surface, text fine + } + } + let (mut light, mut dark) = (0usize, 0usize); + tally_surface_text_colors(node, &mut light, &mut dark); + if light >= 1 && dark == 0 { + if let Some(obj) = node.as_object_mut() { + obj.insert( + "fill".to_string(), + json!([{ "type": "solid", "color": design_accent }]), + ); + } + } +} + +/// The design's DOMINANT accent token across already-generated siblings — glm +/// uses a chart token as the de-facto brand accent (the palette's +/// `$color-accent` often defaults to a clashing blue). Counting across the +/// assembled-so-far page (passed by the caller from the doc sink) picks e.g. +/// `$color-chart-6` when it's used 9× vs `$color-accent` 1×, so an injected +/// banner band matches the rest of the screen. +pub fn dominant_design_accent(nodes: &[PenNode]) -> Option { + let mut counts: Vec<(String, usize)> = Vec::new(); + for n in nodes { + if let Ok(v) = serde_json::to_value(n) { + tally_accent(&v, &mut counts); + } + } + counts.into_iter().max_by_key(|(_, n)| *n).map(|(c, _)| c) +} + +fn tally_accent(node: &Value, counts: &mut Vec<(String, usize)>) { + if let Some(c) = first_solid_color(node) { + if c == "$color-accent" + || c == "$color-primary" + || c == "$color-brand" + || c.starts_with("$color-chart-") + { + if let Some(e) = counts.iter_mut().find(|(k, _)| *k == c) { + e.1 += 1; + } else { + counts.push((c, 1)); + } + } + } + for child in children_of(node) { + tally_accent(child, counts); + } +} + +// ── clip_card_image_corners (port of the TS pass) ──────────────────────────── +// +// A card-shape frame (scalar cornerRadius > 0) whose FIRST child is a +// full-width image carrying its OWN scalar cornerRadius renders the image's +// bottom corners rounded inside the card while the title below sits flush — +// the ragged look. Enforce `clipContent:true` on the card (its outer radius +// clips the image) + drop the image's own radius. Port of +// clip-card-image-corners.ts. +fn is_image_node(node: &Value) -> bool { + match node.get("type").and_then(Value::as_str) { + Some("image") => true, + Some("frame") => role_of(node) == Some("image-placeholder"), + _ => false, + } +} + +/// A card's leading header image authored at a FIXED width narrower than the +/// card leaves a white gap beside it — a 160 px image inside a 252 px dish +/// card fills only ~63 % of the width. A vertical card's header image should +/// span the full card width, so set the leading image to +/// `width: fill_container`. The renderer's object-fit cover (parity with TS) +/// keeps the photo from distorting once it widens. +/// +/// Scoped tightly to avoid touching avatars / logos / inline thumbnails: only +/// a LARGE (>= 80 px) leading image, in a `vertical` container that is NOT +/// center-aligned (centered leading images are avatars/logos, not full-bleed +/// headers), with at least one sibling below it. +fn fill_card_leading_image_width(node: &mut Value) { + let is_container = matches!( + node.get("type").and_then(Value::as_str), + Some("frame") | Some("group") + ); + let is_vertical = node.get("layout").and_then(Value::as_str) == Some("vertical"); + let is_centered = node.get("alignItems").and_then(Value::as_str) == Some("center"); + if is_container && is_vertical && !is_centered && child_count(node) >= 2 { + let lead_is_wide_fixed_image = children_of(node) + .first() + .map(|c| { + is_image_node(c) + && c.get("width") + .and_then(Value::as_f64) + .map(|w| w >= 80.0) + .unwrap_or(false) + }) + .unwrap_or(false); + if lead_is_wide_fixed_image { + if let Some(first) = node + .get_mut("children") + .and_then(Value::as_array_mut) + .and_then(|a| a.first_mut()) + .and_then(Value::as_object_mut) + { + first.insert("width".to_string(), json!("fill_container")); + } + } + } + if let Some(children) = node.get_mut("children").and_then(Value::as_array_mut) { + for child in children.iter_mut() { + fill_card_leading_image_width(child); + } + } +} + +// ── fix_notification_badge_overlay ─────────────────────────────────────── + +/// A small, roughly-square icon button whose two children are exactly one icon +/// glyph and one tiny filled dot — the notification-badge pattern. +fn is_small_icon_button(node: &Value) -> bool { + if node.get("type").and_then(Value::as_str) != Some("frame") { + return false; + } + let (Some(w), Some(h)) = ( + node.get("width").and_then(Value::as_f64), + node.get("height").and_then(Value::as_f64), + ) else { + return false; + }; + w <= 56.0 && h <= 56.0 && (w - h).abs() <= 12.0 && child_count(node) == 2 +} + +fn is_icon_glyph(node: &Value) -> bool { + node.get("type").and_then(Value::as_str) == Some("icon_font") || is_image_node(node) +} + +/// A tiny (<= 14 px), roughly-square, filled, childless shape — a badge dot. +fn is_badge_dot(node: &Value) -> bool { + if !matches!( + node.get("type").and_then(Value::as_str), + Some("frame") | Some("rectangle") | Some("ellipse") + ) { + return false; + } + let (Some(w), Some(h)) = ( + node.get("width").and_then(Value::as_f64), + node.get("height").and_then(Value::as_f64), + ) else { + return false; + }; + let has_fill = node + .get("fill") + .and_then(Value::as_array) + .map(|a| !a.is_empty()) + .unwrap_or(false); + w <= 14.0 && h <= 14.0 && (w - h).abs() <= 4.0 && has_fill && children_of(node).is_empty() +} + +/// A notification badge authored as a flex SIBLING of the icon inside a small +/// icon button renders BESIDE the icon (a square dot to its left, per the live +/// `Notification Button` = horizontal frame > [8×8 dot, bell]) instead of +/// overlaying the icon's top-right corner. Detect the pattern and convert it to +/// a corner-overlay badge: the button becomes `layout: none`, the icon is +/// centered, and the dot is rounded to a circle (square dots only — ellipses +/// are already round) and pinned to the icon's top-right corner. +fn fix_notification_badge_overlay(node: &mut Value) { + if is_small_icon_button(node) { + let kids = children_of(node); + let mut icon_i: Option = None; + let mut dot_i: Option = None; + for (i, c) in kids.iter().enumerate() { + if is_icon_glyph(c) { + icon_i = Some(i); + } else if is_badge_dot(c) { + dot_i = Some(i); + } + } + if let (Some(ii), Some(di)) = (icon_i, dot_i) { + let bw = node.get("width").and_then(Value::as_f64).unwrap_or(0.0); + let bh = node.get("height").and_then(Value::as_f64).unwrap_or(0.0); + let iw = kids[ii].get("width").and_then(Value::as_f64).unwrap_or(0.0); + let ih = kids[ii] + .get("height") + .and_then(Value::as_f64) + .unwrap_or(0.0); + let dw = kids[di].get("width").and_then(Value::as_f64).unwrap_or(0.0); + let icon_x = ((bw - iw) * 0.5).max(0.0); + let icon_y = ((bh - ih) * 0.5).max(0.0); + let dot_x = (icon_x + iw - dw * 0.5).min((bw - dw).max(0.0)); + let dot_y = (icon_y - dw * 0.5).max(0.0); + if let Some(obj) = node.as_object_mut() { + obj.insert("layout".to_string(), json!("none")); + obj.remove("gap"); + } + if let Some(arr) = node.get_mut("children").and_then(Value::as_array_mut) { + if let Some(icon) = arr.get_mut(ii).and_then(Value::as_object_mut) { + icon.insert("x".to_string(), json!(icon_x)); + icon.insert("y".to_string(), json!(icon_y)); + } + if let Some(dot) = arr.get_mut(di).and_then(Value::as_object_mut) { + dot.insert("x".to_string(), json!(dot_x)); + dot.insert("y".to_string(), json!(dot_y)); + if dot.get("type").and_then(Value::as_str) != Some("ellipse") { + dot.insert("cornerRadius".to_string(), json!(dw * 0.5)); + } + } + } + } + } + if let Some(children) = node.get_mut("children").and_then(Value::as_array_mut) { + for child in children.iter_mut() { + fix_notification_badge_overlay(child); + } + } +} + +fn clip_card_image_corners(node: &mut Value) { + let is_container = matches!( + node.get("type").and_then(Value::as_str), + Some("frame") | Some("group") + ); + if is_container { + let radius_ok = node + .get("cornerRadius") + .and_then(Value::as_f64) + .map(|r| r > 0.0) + .unwrap_or(false); + let first_image_with_radius = children_of(node) + .first() + .map(|c| { + is_image_node(c) + && c.get("cornerRadius") + .and_then(Value::as_f64) + .map(|r| r > 0.0) + .unwrap_or(false) + }) + .unwrap_or(false); + if radius_ok && child_count(node) >= 2 && first_image_with_radius { + if let Some(obj) = node.as_object_mut() { + obj.insert("clipContent".to_string(), json!(true)); + } + if let Some(first) = node + .get_mut("children") + .and_then(Value::as_array_mut) + .and_then(|a| a.first_mut()) + .and_then(Value::as_object_mut) + { + first.remove("cornerRadius"); + } + } + } + if let Some(children) = node.get_mut("children").and_then(Value::as_array_mut) { + for child in children.iter_mut() { + clip_card_image_corners(child); + } + } +} + +// ── convert_stacked_overlay_to_absolute (port of the TS pass) ──────────────── +// +// Hero/banner overlay: a `layout:vertical` frame with a NUMERIC height whose +// children include ≥2 full-height bg-like nodes (a full-bleed image + a +// gradient-overlay rect/frame) that the model intends to LAYER, not stack. +// Vertical layout sequences them instead → the image + overlay collapse to +// h=0 and the content overflows the section (the broken "Featured Restaurants" +// promo card: Restaurant Image h=0, Card Details spilling below the section). +// Switch to `layout:none` so children stack at their own x/y (0,0 default), +// which is the layered intent. Conservative: requires EXPLICIT vertical + +// numeric height + ≥2 same-height/fill bg children (false positives are worse +// than misses). Port of convert-stacked-overlay-to-absolute.ts. +fn convert_stacked_overlay_to_absolute(node: &mut Value) { + if node.get("type").and_then(Value::as_str) == Some("frame") + && node.get("layout").and_then(Value::as_str) == Some("vertical") + { + if let Some(h) = node.get("height").and_then(Value::as_f64) { + let mut bg_like = 0; + for child in children_of(node) { + let t = child.get("type").and_then(Value::as_str); + if !matches!(t, Some("image") | Some("rectangle") | Some("frame")) { + continue; + } + let ch = child.get("height"); + let matches_h = ch.and_then(Value::as_f64).map(|x| x == h).unwrap_or(false) + || ch.and_then(Value::as_str) == Some("fill_container"); + if matches_h { + bg_like += 1; + } + } + if bg_like >= 2 { + if let Some(obj) = node.as_object_mut() { + obj.insert("layout".to_string(), json!("none")); + } + } + } + } + if let Some(children) = node.get_mut("children").and_then(Value::as_array_mut) { + for child in children.iter_mut() { + convert_stacked_overlay_to_absolute(child); + } + } +} + +// ── forest entry ───────────────────────────────────────────────────────────── + +/// Run the post-streaming heuristics over a subtask forest. Each root is a +/// page-root child (a section). MUST run BEFORE variable binding — +/// `strip_redundant_section_fill` matches literal hedge HEX that binding would +/// otherwise convert to `$color-*` refs. `page_bg` is the plan root frame's +/// fill hex when known (enables the "section fill == page bg" strip case). +/// A bottom-tab-bar's ACTIVE tab is frequently authored as a solid-filled frame +/// with NO `cornerRadius` — a sharp rectangle that pokes out of the rounded nav +/// pill (the "active block is an unreasonable square overflowing the navbar" the +/// user flagged). The TS references render the active item as a rounded pill. +/// Round any filled tab-item frame inside a bottom-tab-bar so it reads as a +/// contained highlight; `cornerRadius: 999` lets the renderer clamp to height/2 +/// → a pill. The bar / pill container itself (already carrying a radius) and the +/// inactive tabs (no fill) are untouched. +/// Roles / structures that identify a bottom navigation bar. The role set is +/// broad (raw-JSON models tag the bar `bottom-tab-bar`; others use `tab-bar` / +/// `nav` / `tab-row`), and as a fallback ANY horizontal container whose children +/// are nav tab items (`nav-item` / `nav-item-active`, the manifest +/// element-builder roles) counts — so the active-tab rounding fires regardless +/// of which generation path produced the bar. +const NAV_BAR_ROLES: &[&str] = &[ + "bottom-tab-bar", + "tab-bar", + "nav", + "tab-row", + "navbar", + "bottom-nav", +]; + +fn is_nav_bar_container(node: &Value) -> bool { + if NAV_BAR_ROLES.contains(&role_of(node).unwrap_or_default()) { + return true; + } + let horizontal = node.get("layout").and_then(Value::as_str) == Some("horizontal"); + horizontal + && node + .get("children") + .and_then(Value::as_array) + .map(|kids| { + kids.iter() + .any(|c| matches!(role_of(c), Some("nav-item") | Some("nav-item-active"))) + }) + .unwrap_or(false) +} + +fn round_active_nav_tab(node: &mut Value, accent: &str) { + if is_nav_bar_container(node) { + // A rounded-pill bar MUST clip its children — a full-height active block + // that pokes out past the pill's rounded corners gets confined. + let bar_radius = node + .get("cornerRadius") + .and_then(Value::as_f64) + .unwrap_or(0.0); + if bar_radius >= 8.0 { + if let Some(obj) = node.as_object_mut() { + obj.insert("clipContent".to_string(), Value::Bool(true)); + } + } + round_nav_tab_items(node, true, accent); + } else if let Some(children) = node.get_mut("children").and_then(Value::as_array_mut) { + for child in children { + round_active_nav_tab(child, accent); + } + } +} + +fn round_nav_tab_items(node: &mut Value, is_bar_root: bool, accent: &str) { + // Cover both `frame` tab items AND a bare `rectangle` highlight behind the + // icon/label — either is a sharp overflowing square if left un-rounded. + let is_block = matches!( + node.get("type").and_then(Value::as_str), + Some("frame") | Some("rectangle") + ); + let role = role_of(node).map(str::to_string).unwrap_or_default(); + let is_active_tab = role == "nav-item-active"; + let has_fill = node + .get("fill") + .and_then(Value::as_array) + .map(|a| !a.is_empty()) + .unwrap_or(false); + let is_full_row = node.get("width").and_then(Value::as_str) == Some("fill_container") + && node.get("layout").and_then(Value::as_str) == Some("horizontal"); + let radius = node + .get("cornerRadius") + .and_then(Value::as_f64) + .unwrap_or(0.0); + if !is_bar_root && is_block && !is_full_row { + if is_active_tab { + // The active tab MUST read as a CONTAINED rounded pill. Round it, and + // if it carries no visible highlight fill, inject the design accent so + // the active state is a proper orange pill (the manifest element + // builder leaves `nav-item-active` unfilled; glm sometimes gives it a + // sharp filled square). Either path → rounded accent pill. + if radius < 8.0 { + node["cornerRadius"] = Value::from(999.0); + } + if !has_renderable_fill(node) { + node["fill"] = json!([{ "type": "solid", "color": accent }]); + } + } else if has_fill && radius < 8.0 { + node["cornerRadius"] = Value::from(999.0); + } + } + if let Some(children) = node.get_mut("children").and_then(Value::as_array_mut) { + for child in children { + round_nav_tab_items(child, false, accent); + } + } +} + +pub fn apply_tree_heuristics( + nodes: &mut [PenNode], + page_bg: Option<&str>, + light_theme: bool, + prior_accent: Option<&str>, +) { + // The emphasis color to use when filling an invisible band. Prefer the + // DESIGN-WIDE dominant accent (counted across already-generated siblings, + // passed by the caller) — this subtask's own forest may lack any accent + // token. Fall back to a per-subtask scan, then the palette default. + let design_accent = prior_accent + .map(str::to_string) + .or_else(|| { + nodes + .iter() + .filter_map(|n| serde_json::to_value(n).ok()) + .find_map(|v| find_design_accent(&v)) + }) + .unwrap_or_else(|| "$color-accent".to_string()); + for node in nodes.iter_mut() { + let Ok(mut v) = serde_json::to_value(&*node) else { + continue; + }; + // Section-level (operate on the forest root = a page-root child). + strip_redundant_section_fill(&mut v, page_bg); + inject_nav_surface_for_section(&mut v); + fix_invisible_text_band(&mut v, light_theme, &design_accent); + // Subtree-recursive. + convert_stacked_overlay_to_absolute(&mut v); + clip_card_image_corners(&mut v); + fill_card_leading_image_width(&mut v); + fix_notification_badge_overlay(&mut v); + strip_nested_card_decoration(&mut v, DecoFlags::default()); + // Round the active nav tab into a pill LAST — it must run AFTER + // `strip_nested_card_decoration`, which would otherwise strip the pill's + // `cornerRadius` back off (the active tab is a rounded frame nested + // inside an already-rounded nav pill, so the dedup pass reads it as + // redundant nested-card decoration). Running last makes the pill the + // final word, fixing the sharp active square overflowing the nav pill. + round_active_nav_tab(&mut v, &design_accent); + if let Ok(new_node) = serde_json::from_value::(v) { + *node = new_node; + } + } +} + +#[cfg(test)] +#[path = "tree_heuristics_tests.rs"] +mod tests; diff --git a/crates/op-orchestrator/src/tree_heuristics_tests.rs b/crates/op-orchestrator/src/tree_heuristics_tests.rs new file mode 100644 index 000000000..eec49c2da --- /dev/null +++ b/crates/op-orchestrator/src/tree_heuristics_tests.rs @@ -0,0 +1,728 @@ +use super::*; +use serde_json::json; + +// ── inject_missing_nav_surface_fill ────────────────────────────────────── + +#[test] +fn nav_without_fill_gets_surface_and_upward_shadow() { + let mut nav = json!({ + "type":"frame","role":"bottom-tab-bar", + "children":[{"type":"frame","role":"button","name":"Home"}] + }); + inject_nav_surface_for_section(&mut nav); + assert_eq!( + nav["fill"], + json!([{"type":"solid","color":"$color-surface"}]), + "transparent bottom nav anchored with surface fill" + ); + // bottom nav → shadow points up (offsetY < 0) + assert_eq!(nav["effects"][0]["offsetY"], json!(-4)); +} + +#[test] +fn nav_with_existing_fill_left_alone() { + let mut nav = json!({ + "type":"frame","role":"bottom-tab-bar","fill":[{"type":"solid","color":"#222222"}], + "children":[{"type":"frame","role":"button"}] + }); + inject_nav_surface_for_section(&mut nav); + assert_eq!( + nav["fill"], + json!([{"type":"solid","color":"#222222"}]), + "explicit nav fill preserved (intent signal)" + ); + assert!( + nav.get("effects").is_none(), + "no shadow stamped over intent" + ); +} + +#[test] +fn nav_wrapped_in_single_child_section_reached_one_hop() { + let mut wrap = json!({ + "type":"frame","role":"section", + "children":[{"type":"frame","role":"bottom-tab-bar","children":[{"type":"frame"}]}] + }); + inject_nav_surface_for_section(&mut wrap); + assert_eq!( + wrap["children"][0]["fill"], + json!([{"type":"solid","color":"$color-surface"}]), + "nav nested one hop under a wrapper section still anchored" + ); +} + +// ── round_active_nav_tab ───────────────────────────────────────────────── + +#[test] +fn active_nav_tab_square_rounded_to_pill() { + // tt5: the active tab is a solid-filled frame with NO cornerRadius → a sharp + // square poking out of the rounded nav pill. Round it to a pill; leave the + // inactive (fill-less) tab and the already-rounded bar container alone. + let mut nav = json!({ + "type":"frame","role":"bottom-tab-bar","cornerRadius":100, + "fill":[{"type":"solid","color":"$color-surface"}], + "children":[ + {"type":"frame","name":"Home Tab","fill":[{"type":"solid","color":"$color-accent"}], + "children":[{"type":"icon_font","iconFontName":"home"},{"type":"text","content":"Home"}]}, + {"type":"frame","name":"Search Tab", + "children":[{"type":"icon_font","iconFontName":"search"},{"type":"text","content":"Search"}]} + ] + }); + round_active_nav_tab(&mut nav, "$color-chart-6"); + assert_eq!( + nav["children"][0]["cornerRadius"], + json!(999.0), + "active filled tab rounded to a pill" + ); + assert!( + nav["children"][1].get("cornerRadius").is_none(), + "inactive fill-less tab untouched" + ); + assert_eq!( + nav["cornerRadius"], + json!(100), + "the bar container's own radius is preserved" + ); +} + +#[test] +fn rounded_nav_bar_gets_clip_content() { + // A full-height sharp active block can be TALLER than the bar — rounding it + // alone can't stop the vertical overflow. A rounded-pill bar must clip its + // children to the pill silhouette (image 52: the orange HOME square pokes + // above + below the white pill). + let mut nav = json!({ + "type":"frame","role":"bottom-tab-bar","cornerRadius":32, + "fill":[{"type":"solid","color":"$color-surface"}], + "children":[ + {"type":"frame","name":"Home Tab","height":"fill_container", + "fill":[{"type":"solid","color":"$color-accent"}], + "children":[{"type":"text","content":"Home"}]} + ] + }); + round_active_nav_tab(&mut nav, "$color-chart-6"); + assert_eq!( + nav["clipContent"], + json!(true), + "rounded bar clips children so a tall active block can't overflow" + ); + assert_eq!( + nav["children"][0]["cornerRadius"], + json!(999.0), + "the filled active tab is still rounded to a pill" + ); +} + +#[test] +fn full_pipeline_rounds_nested_active_tab_in_pill() { + // tt8 (image 53) EXACT structure: bottom-tab-bar > Nav Pill(cr=100) > + // Home Tab(fill orange, no radius, fill_container/vertical). Run the FULL + // apply_tree_heuristics (incl. the PenNode JSON round-trip) and confirm the + // active Home Tab actually comes out rounded to a pill — a square here is + // the sharp block overflowing the rounded pill the user keeps seeing. + let mut forest: Vec = serde_json::from_value(json!([{ + "type":"frame","id":"bar","role":"bottom-tab-bar","name":"Bottom Navigation", + "width":"fill_container","fill":[{"type":"solid","color":"$color-surface"}], + "children":[{ + "type":"frame","id":"pill","name":"Nav Pill","width":"fill_container","layout":"horizontal", + "cornerRadius":100,"fill":[{"type":"solid","color":"$color-surface"}], + "children":[ + {"type":"frame","id":"home","name":"Home Tab","width":"fill_container","height":"fit_content", + "layout":"vertical","fill":[{"type":"solid","color":"$color-chart-6"}], + "children":[ + {"type":"icon_font","id":"hi","name":"Home Icon","iconFontName":"home","width":20,"height":20}, + {"type":"text","id":"hl","name":"Home Label","content":"HOME"} + ]}, + {"type":"frame","id":"search","name":"Search Tab","width":"fill_container","height":"fit_content", + "layout":"vertical","children":[ + {"type":"icon_font","id":"si","name":"Search Icon","iconFontName":"search","width":20,"height":20} + ]} + ] + }] + }])) + .expect("nav forest json"); + apply_tree_heuristics(&mut forest, Some("#FFF8F0"), true, Some("$color-chart-6")); + let out = serde_json::to_value(&forest[0]).expect("serialize"); + let home = &out["children"][0]["children"][0]; + assert_eq!(home["name"], json!("Home Tab")); + assert_eq!( + home["cornerRadius"], + json!(999.0), + "filled active Home Tab must round to a pill end-to-end (got {:?})", + home.get("cornerRadius") + ); +} + +#[test] +fn full_pipeline_rounds_manifest_nav_item_active() { + // glm-5.1 / manifest (element-builder) nav: bottom-tab-bar > nav-item-active + // tab (role="nav-item-active", width=fit_content) carrying an accent fill + // (the orange active block). This is the structure the USER actually gets + // (their model is glm-5.1, the manifest path) — distinct from glm-5.2's + // `bottom-tab-bar > Nav Pill > Home Tab`. The active tab must round to a pill. + let mut forest: Vec = serde_json::from_value(json!([{ + "type":"frame","id":"bar","role":"bottom-tab-bar","name":"Bottom Tab Bar", + "width":"fill_container","cornerRadius":100, + "fill":[{"type":"solid","color":"$color-surface"}], + "children":[ + {"type":"frame","id":"home","role":"nav-item-active","name":"Tab (Home)", + "width":"fit_content","height":"fit_content","layout":"vertical","padding":[4,12], + "fill":[{"type":"solid","color":"$color-chart-6"}], + "children":[ + {"type":"icon_font","id":"hi","name":"Icon","iconFontName":"house","width":24,"height":24}, + {"type":"text","id":"hl","name":"Label","content":"Home"} + ]}, + {"type":"frame","id":"search","role":"nav-item","name":"Tab (Search)", + "width":"fit_content","height":"fit_content","layout":"vertical","padding":[4,12], + "children":[ + {"type":"icon_font","id":"si","name":"Icon","iconFontName":"search","width":24,"height":24} + ]} + ] + }])) + .expect("manifest nav json"); + apply_tree_heuristics(&mut forest, Some("#FFF8F0"), true, Some("$color-chart-6")); + let out = serde_json::to_value(&forest[0]).expect("serialize"); + let active = &out["children"][0]; + assert_eq!(active["role"], json!("nav-item-active")); + assert_eq!( + active["cornerRadius"], + json!(999.0), + "manifest nav-item-active tab must round to a pill (got {:?})", + active.get("cornerRadius") + ); +} + +#[test] +fn full_pipeline_rounds_user_tt5_exact_nav() { + // The USER's actual tt5.op (glm-5.2): bottom-tab-bar(vertical) > Tab + // Pill(role=None, cr=100, horizontal) > Home Tab(role=None, fill_container, + // vertical, cornerRadius=0.0, $color-chart-6 fill, white icon/label). This is + // the EXACT square the user keeps seeing. Run the full pipeline; the active + // Home Tab must come out rounded (cornerRadius=999). + let src = r##"[{ + "type":"frame","id":"bar","role":"bottom-tab-bar","name":"Bottom Nav", + "width":"fill_container","height":"fit_content","layout":"vertical", + "fill":[{"type":"solid","color":"$color-surface"}], + "children":[{ + "type":"frame","id":"pill","name":"Tab Pill", + "width":"fill_container","height":"fit_content","layout":"horizontal","cornerRadius":100, + "fill":[{"type":"solid","color":"$color-surface"}], + "children":[ + {"type":"frame","id":"home","name":"Home Tab", + "width":"fill_container","height":"fit_content","layout":"vertical","cornerRadius":0.0, + "fill":[{"type":"solid","color":"$color-chart-6"}], + "children":[ + {"type":"icon_font","id":"hi","name":"Home Icon","iconFontName":"home","width":18,"height":18, + "fill":[{"type":"solid","color":"$color-surface"}]}, + {"type":"text","id":"hl","name":"Home Label","content":"HOME", + "fill":[{"type":"solid","color":"$color-surface"}]} + ]}, + {"type":"frame","id":"sr","name":"Search Tab","role":"search-bar", + "width":"fill_container","height":"fit_content","layout":"vertical","cornerRadius":26, + "children":[ + {"type":"icon_font","id":"si","name":"Search Icon","iconFontName":"search","width":18,"height":18} + ]} + ] + }] + }]"##; + let mut forest: Vec = serde_json::from_str(src).expect("user nav json"); + apply_tree_heuristics(&mut forest, Some("#FFF8F0"), true, Some("$color-chart-6")); + let out = serde_json::to_value(&forest[0]).expect("serialize"); + let home = &out["children"][0]["children"][0]; + assert_eq!(home["name"], json!("Home Tab")); + assert_eq!( + home["cornerRadius"], + json!(999.0), + "user's exact square Home Tab must round to a pill (got {:?})", + home.get("cornerRadius") + ); +} + +#[test] +fn bare_highlight_rect_in_nav_rounded() { + // The active highlight is sometimes a bare filled rect (no icon/label + // children) sitting behind the tab content. It must still round to a pill. + let mut nav = json!({ + "type":"frame","role":"bottom-tab-bar","cornerRadius":28, + "children":[ + {"type":"frame","name":"Tab Row","width":"fill_container","layout":"horizontal", + "children":[ + {"type":"frame","name":"Active Highlight", + "fill":[{"type":"solid","color":"$color-accent"}]} + ]} + ] + }); + round_active_nav_tab(&mut nav, "$color-chart-6"); + assert_eq!( + nav["children"][0]["children"][0]["cornerRadius"], + json!(999.0), + "bare filled highlight rect rounded to a pill" + ); + assert!( + nav["children"][0].get("cornerRadius").is_none(), + "the full-width horizontal tab ROW is not rounded" + ); +} + +// ── strip_redundant_section_fill ───────────────────────────────────────── + +#[test] +fn section_safe_light_hedge_fill_stripped_with_chrome() { + // Weak-model "search section" wrapper: white hedge fill + pill chrome. + let mut sect = json!({ + "type":"frame","role":"section","fill":[{"type":"solid","color":"#FFFFFF"}], + "stroke":{"thickness":1,"fill":[{"type":"solid","color":"#E2E8F0"}]}, + "cornerRadius":16, + "children":[{"type":"text","content":"x"}] + }); + strip_redundant_section_fill(&mut sect, None); + assert!(sect.get("fill").is_none(), "hedge fill stripped"); + assert!(sect.get("stroke").is_none(), "wrapper stroke stripped"); + assert!( + sect.get("cornerRadius").is_none(), + "wrapper radius stripped" + ); +} + +#[test] +fn section_dark_hedge_fill_stripped() { + let mut sect = json!({ + "type":"frame","role":"section","fill":[{"type":"solid","color":"#0A0A0A"}], + "children":[{"type":"text"}] + }); + strip_redundant_section_fill(&mut sect, None); + assert!(sect.get("fill").is_none(), "safe-dark hedge stripped"); +} + +#[test] +fn card_fill_protected_from_strip() { + let mut card = json!({ + "type":"frame","role":"card","fill":[{"type":"solid","color":"#FFFFFF"}], + "children":[{"type":"text","content":"x"}] + }); + strip_redundant_section_fill(&mut card, None); + assert_eq!( + card["fill"], + json!([{"type":"solid","color":"#FFFFFF"}]), + "card is a protected surface, not a section" + ); +} + +#[test] +fn misrolled_search_bar_wrapper_stripped() { + // sub-agent emits Search Bar(role=search-bar) > Search Input(role=input,fill) + // — the outer "search-bar" is a section wrapper, strip its hedge fill. + let mut wrap = json!({ + "type":"frame","role":"search-bar","fill":[{"type":"solid","color":"#F8FAFC"}], + "children":[{"type":"text_input","role":"input","fill":[{"type":"solid","color":"#FFFFFF"}]}] + }); + strip_redundant_section_fill(&mut wrap, None); + assert!( + wrap.get("fill").is_none(), + "misrolled search-bar wrapper around a real input → fill stripped" + ); +} + +#[test] +fn section_fill_matching_page_bg_stripped() { + let mut sect = json!({ + "type":"frame","role":"section","fill":[{"type":"solid","color":"#FFF8F0"}], + "children":[{"type":"text"}] + }); + strip_redundant_section_fill(&mut sect, Some("#FFF8F0")); + assert!( + sect.get("fill").is_none(), + "section repainting the page bg is redundant → stripped" + ); +} + +// ── clip_card_image_corners ────────────────────────────────────────────── + +#[test] +fn card_image_corners_clipped() { + let mut card = json!({ + "type":"frame","role":"card","cornerRadius":12,"children":[ + {"type":"image","name":"Photo","cornerRadius":12}, + {"type":"text","content":"Title"} + ] + }); + clip_card_image_corners(&mut card); + assert_eq!( + card["clipContent"], + json!(true), + "card clips to its outer radius" + ); + assert!( + card["children"][0].get("cornerRadius").is_none(), + "image's own radius removed (card clips instead)" + ); +} + +#[test] +fn standalone_image_not_clipped() { + // single image (no title sibling) → not a card-image pattern, left alone. + let mut frame = json!({ + "type":"frame","cornerRadius":12,"children":[ + {"type":"image","cornerRadius":12} + ] + }); + clip_card_image_corners(&mut frame); + assert!( + frame.get("clipContent").is_none(), + "1-child image frame untouched" + ); +} + +// ── fill_card_leading_image_width ──────────────────────────────────────── + +#[test] +fn dish_card_leading_image_fills_width() { + // 160 px image inside a 252 px vertical dish card → white gap on the + // right. The header image should span the card width. + let mut card = json!({ + "type":"frame","layout":"vertical","width":252.0,"children":[ + {"type":"image","name":"Photo","width":160.0,"height":120.0}, + {"type":"frame","name":"Info","children":[{"type":"text","content":"Truffle Burger"}]} + ] + }); + fill_card_leading_image_width(&mut card); + assert_eq!( + card["children"][0]["width"], + json!("fill_container"), + "leading header image spans the card width" + ); +} + +#[test] +fn centered_leading_avatar_not_widened() { + // a centered leading image is an avatar / logo, not a full-bleed header. + let mut card = json!({ + "type":"frame","layout":"vertical","alignItems":"center","children":[ + {"type":"image","name":"Avatar","width":96.0,"height":96.0,"cornerRadius":48.0}, + {"type":"text","content":"Jane Doe"} + ] + }); + fill_card_leading_image_width(&mut card); + assert_eq!( + card["children"][0]["width"], + json!(96.0), + "centered avatar keeps its fixed width" + ); +} + +#[test] +fn horizontal_card_side_image_not_widened() { + // image-left / text-right row → the leading image is a side thumbnail. + let mut row = json!({ + "type":"frame","layout":"horizontal","children":[ + {"type":"image","name":"Thumb","width":120.0,"height":120.0}, + {"type":"text","content":"Order #1"} + ] + }); + fill_card_leading_image_width(&mut row); + assert_eq!( + row["children"][0]["width"], + json!(120.0), + "horizontal-row side image keeps its fixed width" + ); +} + +#[test] +fn small_leading_icon_not_widened() { + // a small (< 80 px) leading image is an inline icon/badge, left alone. + let mut card = json!({ + "type":"frame","layout":"vertical","children":[ + {"type":"image","name":"Icon","width":48.0,"height":48.0}, + {"type":"text","content":"Label"} + ] + }); + fill_card_leading_image_width(&mut card); + assert_eq!( + card["children"][0]["width"], + json!(48.0), + "small leading icon keeps its fixed width" + ); +} + +// ── fix_notification_badge_overlay ─────────────────────────────────────── + +#[test] +fn notification_badge_becomes_corner_circle() { + // live structure: 44×44 horizontal button > [8×8 filled dot, 22×22 bell]. + let mut button = json!({ + "type":"frame","name":"Notification Button","width":44.0,"height":44.0, + "layout":"horizontal","gap":8.0,"cornerRadius":12.0, + "children":[ + {"type":"frame","name":"Badge","width":8.0,"height":8.0, + "fill":[{"type":"solid","color":"$color-chart-6"}],"children":[]}, + {"type":"icon_font","name":"Bell","iconFontName":"bell","width":22.0,"height":22.0} + ] + }); + fix_notification_badge_overlay(&mut button); + assert_eq!( + button["layout"], + json!("none"), + "button switches to absolute" + ); + assert!(button.get("gap").is_none(), "flex gap dropped"); + // badge: rounded to a circle, pinned to the bell's top-right corner. + let badge = &button["children"][0]; + assert_eq!( + badge["cornerRadius"], + json!(4.0), + "square dot rounded to a circle" + ); + assert_eq!(badge["x"], json!(29.0), "badge x at bell top-right"); + assert_eq!(badge["y"], json!(7.0), "badge y at bell top-right"); + // bell: centered in the 44×44 button. + let bell = &button["children"][1]; + assert_eq!(bell["x"], json!(11.0), "bell centered x"); + assert_eq!(bell["y"], json!(11.0), "bell centered y"); +} + +#[test] +fn switch_like_frame_not_treated_as_badge() { + // a non-square pill (44×24) is a switch/toggle, not an icon button. + let mut sw = json!({ + "type":"frame","width":44.0,"height":24.0,"layout":"horizontal","cornerRadius":12.0, + "children":[ + {"type":"ellipse","width":20.0,"height":20.0,"fill":[{"type":"solid","color":"#fff"}]}, + {"type":"icon_font","iconFontName":"check","width":12.0,"height":12.0} + ] + }); + let before = sw.clone(); + fix_notification_badge_overlay(&mut sw); + assert_eq!(sw, before, "non-square pill left untouched"); +} + +#[test] +fn icon_button_without_dot_unchanged() { + // a plain icon button (icon + text label) is not a badge pattern. + let mut btn = json!({ + "type":"frame","width":44.0,"height":44.0,"layout":"horizontal", + "children":[ + {"type":"icon_font","iconFontName":"heart","width":22.0,"height":22.0}, + {"type":"text","content":"12"} + ] + }); + let before = btn.clone(); + fix_notification_badge_overlay(&mut btn); + assert_eq!(btn, before, "icon+text button left untouched"); +} + +// ── convert_stacked_overlay_to_absolute ────────────────────────────────── + +#[test] +fn stacked_overlay_switches_to_absolute() { + // full-bleed image + gradient overlay (both full-height) in a vertical + // fixed-height frame → layered hero → layout:none (fixes the collapsed + // "Featured Restaurants" promo card: image h=0 + content overflow). + let mut hero = json!({ + "type":"frame","layout":"vertical","height":180.0,"children":[ + {"type":"image","height":180.0}, + {"type":"frame","name":"Gradient","height":"fill_container"}, + {"type":"frame","name":"Content","children":[{"type":"text","content":"30% Off"}]} + ] + }); + convert_stacked_overlay_to_absolute(&mut hero); + assert_eq!( + hero["layout"], + json!("none"), + "layered hero (2+ full-height bg children) → absolute stacking" + ); +} + +#[test] +fn normal_vertical_stack_not_converted() { + // a normal content stack (1 image, rest text) → stays vertical. + let mut col = json!({ + "type":"frame","layout":"vertical","height":200.0,"children":[ + {"type":"image","height":120.0}, + {"type":"text","content":"Title"}, + {"type":"text","content":"Body"} + ] + }); + convert_stacked_overlay_to_absolute(&mut col); + assert_eq!( + col["layout"], + json!("vertical"), + "only 1 bg-like child → not a hero overlay, left alone" + ); +} + +// ── dominant_design_accent ─────────────────────────────────────────────── + +#[test] +fn dominant_accent_prefers_most_used_chart_token() { + // glm uses $color-chart-6 as the de-facto accent (3×) over $color-accent (1×). + let nodes: Vec = vec![serde_json::from_value(json!({ + "type":"frame","id":"r","name":"Page","children":[ + {"type":"frame","id":"b","name":"Btn","role":"button","fill":[{"type":"solid","color":"$color-chart-6"}]}, + {"type":"frame","id":"c","name":"Chip","role":"chip","fill":[{"type":"solid","color":"$color-chart-6"}]}, + {"type":"text","id":"t","name":"T","content":"x","fill":[{"type":"solid","color":"$color-chart-6"}]}, + {"type":"frame","id":"a","name":"A","fill":[{"type":"solid","color":"$color-accent"}]} + ] + })) + .unwrap()]; + assert_eq!( + dominant_design_accent(&nodes).as_deref(), + Some("$color-chart-6"), + "the most-used accent token wins over the palette default" + ); +} + +// ── strip_nested_card_decoration ───────────────────────────────────────── + +#[test] +fn nested_card_redundant_shadow_and_radius_stripped() { + let mut outer = json!({ + "type":"frame","role":"card","cornerRadius":12, + "effects":[{"type":"shadow","offsetX":0,"offsetY":1,"blur":3,"color":"#0000001A"}], + "children":[{ + "type":"frame","role":"card","cornerRadius":10, + "effects":[{"type":"shadow","offsetX":0,"offsetY":1,"blur":2,"color":"#0000000F"}], + "children":[{"type":"text","content":"hi"}] + }] + }); + strip_nested_card_decoration(&mut outer, DecoFlags::default()); + let inner = &outer["children"][0]; + assert!( + inner.get("effects").is_none(), + "inner card's redundant shadow stripped (kills box-in-box)" + ); + assert!( + inner.get("cornerRadius").is_none(), + "inner card's redundant radius stripped" + ); + // outer keeps its decoration + assert_eq!(outer["cornerRadius"], json!(12)); +} + +// ── fix_invisible_text_band (glm banner-fill floor) ────────────────────── + +#[test] +fn invisible_white_text_band_gets_accent_on_light_page() { + // glm's transparent promo banner: white copy, no fill → invisible on cream. + let mut band = json!({"type":"frame","role":"section","children":[ + {"type":"text","content":"50% OFF","fill":[{"type":"solid","color":"$color-surface"}]}, + {"type":"text","content":"first order","fill":[{"type":"solid","color":"#FFFFFF"}]} + ]}); + fix_invisible_text_band(&mut band, true, "$color-accent"); + assert_eq!( + band["fill"], + json!([{"type":"solid","color":"$color-accent"}]), + "all-white-text band on a light page → accent fill so copy is readable" + ); +} + +#[test] +fn banner_with_filled_button_still_filled() { + // white headline + an "Order Now" button (own white fill, accent text). The + // button's accent text sits on its OWN surface → must not count → the + // all-light headline alone triggers the banner fill. (gen9 Promo Content.) + let mut band = json!({"type":"frame","role":"section","children":[ + {"type":"text","content":"50% OFF","fill":[{"type":"solid","color":"$color-surface"}]}, + {"type":"text","content":"Code: X","fill":[{"type":"solid","color":"$color-surface-2"}]}, + {"type":"frame","role":"button","fill":[{"type":"solid","color":"$color-surface"}],"children":[ + {"type":"text","content":"Order Now","fill":[{"type":"solid","color":"$color-accent"}]} + ]} + ]}); + fix_invisible_text_band(&mut band, true, "$color-accent"); + assert_eq!( + band["fill"], + json!([{"type":"solid","color":"$color-accent"}]), + "button's accent text excluded (own surface) → light headline → filled" + ); +} + +#[test] +fn light_surface_filled_banner_repainted_accent() { + // tt5: a feature-card carrying a `$color-surface` (light) fill with white + // headline + subtext + a translucent-white badge + a dark CTA button. The + // light surface makes the white headline INVISIBLE. The light-surface fill + // must be treated like no fill → repaint with the design accent. + let mut band = json!({ + "type":"frame","role":"feature-card","cornerRadius":12, + "fill":[{"type":"solid","color":"$color-surface"}], + "children":[ + {"type":"frame","name":"Promo Content","children":[ + {"type":"frame","role":"badge","fill":[{"type":"solid","color":"#FFFFFF30"}],"children":[ + {"type":"text","content":"FLASH","fill":[{"type":"solid","color":"$color-surface"}]} + ]}, + {"type":"text","content":"Get 30% off","fill":[{"type":"solid","color":"$color-surface"}]}, + {"type":"text","content":"first order","fill":[{"type":"solid","color":"#FFFFFFCC"}]}, + {"type":"frame","role":"button","fill":[{"type":"solid","color":"#1C1410"}],"children":[ + {"type":"text","content":"Order Now","fill":[{"type":"solid","color":"$color-surface"}]} + ]} + ]} + ] + }); + fix_invisible_text_band(&mut band, true, "$color-chart-6"); + assert_eq!( + band["fill"], + json!([{"type":"solid","color":"$color-chart-6"}]), + "light-surface banner with all-white copy → repainted with the accent" + ); +} + +#[test] +fn gradient_filled_banner_left_alone() { + // A banner that already paints a real colored surface (a gradient) reads its + // white text fine — must NOT be repainted. + let mut band = json!({ + "type":"frame","role":"feature-card", + "fill":[{"type":"linear_gradient","angle":135,"stops":[ + {"offset":0.0,"color":"$color-chart-6"},{"offset":1.0,"color":"#FB923C"}]}], + "children":[ + {"type":"text","content":"Get 30% off","fill":[{"type":"solid","color":"$color-surface"}]} + ] + }); + let before = band["fill"].clone(); + fix_invisible_text_band(&mut band, true, "$color-accent"); + assert_eq!( + band["fill"], before, + "gradient banner keeps its real surface" + ); +} + +#[test] +fn white_text_band_untouched_on_dark_page() { + let mut band = json!({"type":"frame","children":[ + {"type":"text","fill":[{"type":"solid","color":"#FFFFFF"}]} + ]}); + fix_invisible_text_band(&mut band, false, "$color-accent"); + assert!( + band.get("fill").is_none(), + "white text on a dark page is fine — no fill stamped" + ); +} + +#[test] +fn mixed_text_band_not_filled() { + // a dark text present → not an all-white invisible band → leave alone. + let mut band = json!({"type":"frame","children":[ + {"type":"text","fill":[{"type":"solid","color":"#FFFFFF"}]}, + {"type":"text","fill":[{"type":"solid","color":"#1C1410"}]} + ]}); + fix_invisible_text_band(&mut band, true, "$color-accent"); + assert!(band.get("fill").is_none(), "mixed text → not a hidden band"); +} + +#[test] +fn media_clipper_radius_preserved_under_decorated_card() { + // A card with cornerRadius wrapping an image-placeholder: the placeholder's + // own cornerRadius rounds the photo, not card decoration → keep it. + let mut outer = json!({ + "type":"frame","role":"card","cornerRadius":12, + "children":[{ + "type":"frame","role":"image-card","cornerRadius":12,"clipContent":true, + "children":[{"type":"image","name":"photo"}] + }] + }); + strip_nested_card_decoration(&mut outer, DecoFlags::default()); + assert_eq!( + outer["children"][0]["cornerRadius"], + json!(12), + "media-clipper radius preserved (rounds the photo)" + ); +} diff --git a/crates/op-orchestrator/src/types.rs b/crates/op-orchestrator/src/types.rs index d4ed967bb..f67e1c139 100644 --- a/crates/op-orchestrator/src/types.rs +++ b/crates/op-orchestrator/src/types.rs @@ -227,6 +227,13 @@ pub enum Intent { #[derive(Debug, Clone)] pub enum Progress { Planning, + /// Planning produced the FULL subtask list — emitted ONCE right after + /// planning so the UI can show the complete task checklist upfront (TS + /// parity), instead of revealing tasks one-by-one as each starts. Pairs + /// `id` with `label` so the UI can mark each row done on `SubtaskDone`. + Planned { + subtasks: Vec<(String, String)>, + }, ScaffoldDone, SubtaskStarted { id: String, diff --git a/crates/op-orchestrator/src/variables.rs b/crates/op-orchestrator/src/variables.rs index 8991ac77a..e37b850ea 100644 --- a/crates/op-orchestrator/src/variables.rs +++ b/crates/op-orchestrator/src/variables.rs @@ -52,11 +52,17 @@ pub fn snapshot_plan_vars(sink: &dyn DocSink, _plan: &OrchestratorPlan) -> VarSn /// 缺失的调色板 token / 轴 → 一条 `MergeThemePreset`。全部已存在时 /// 返回空(完全播种过的文档零命令)。 -pub fn seed_commands(_plan: &OrchestratorPlan, snap: &VarSnapshot) -> Vec { +pub fn seed_commands(plan: &OrchestratorPlan, snap: &VarSnapshot) -> Vec { if snap.created.is_empty() && snap.created_axes.is_empty() { return Vec::new(); } - let palette = semantic_palette::palette_variables(); + let mut palette = semantic_palette::palette_variables(); + // Harmonize the cool-slate neutral ramp to the design's temperature: a warm + // page (cream/orange) gets warm grays so the search bar / chips / avatar + // circle no longer read as off-palette cool gray. Neutral pages keep slate. + if let Some(page_bg) = plan.root_frame.first_solid_hex() { + crate::palette_harmonize::harmonize_palette_neutrals(&mut palette, &page_bg); + } let variables = snap .created .iter()