From 5df84f05914e37a7d04ace0f9713291326ecacd8 Mon Sep 17 00:00:00 2001 From: Fini Date: Mon, 13 Jul 2026 21:41:38 +0800 Subject: [PATCH] fix(agent): an omitted container layout stacks instead of turning into a row The engine's flex default is a ROW, so a section frame that never declared layout laid its title beside its card rail and pushed the cards off the screen (measured: every section in a DS run). An omission is not a request for a row - a container with flow children and no layout now stacks, and the program-DSL contract says to declare it. --- crates/op-mcp/src/batch_design.rs | 37 ++++++++++++++ crates/op-mcp/src/batch_design_tests.rs | 65 +++++++++++++++++++++++++ crates/op-orchestrator/src/prompt.rs | 3 ++ 3 files changed, 105 insertions(+) diff --git a/crates/op-mcp/src/batch_design.rs b/crates/op-mcp/src/batch_design.rs index c0facab56..d78479e12 100644 --- a/crates/op-mcp/src/batch_design.rs +++ b/crates/op-mcp/src/batch_design.rs @@ -645,6 +645,7 @@ pub(crate) fn normalize_node_shape(value: &mut serde_json::Value) { // correctly, then every `U(n1,{layout:{type:horizontal…}})` was rejected and // the tree thrashed to empty). normalize_layout_object(obj); + default_container_layout(obj); if let Some(fill) = obj.get_mut("fill") { normalize_fill(fill); } @@ -921,6 +922,42 @@ fn normalize_text_growth(obj: &mut serde_json::Map) { } } +/// A container with flow children and NO `layout` renders as a ROW — the +/// engine's flex default. That is never what an omission means: a section +/// whose children are [title row, card rail] wants them STACKED, and models +/// that want a row always say so explicitly (measured test0711-1-ds: every +/// section frame omitted `layout`, so each title landed to the LEFT of its +/// rail and the cards ran off the screen). An omitted layout stacks. +/// +/// Absolutely-positioned children are out of flow, so a frame whose children +/// all carry `x`/`y` is left alone — its direction is irrelevant, and forcing +/// one could contradict a `layout: none` the caller means to set later. +fn default_container_layout(obj: &mut serde_json::Map) { + const CONTAINERS: [&str; 3] = ["frame", "group", "rectangle"]; + let is_container = obj + .get("type") + .and_then(serde_json::Value::as_str) + .is_some_and(|t| CONTAINERS.contains(&t)); + if !is_container || obj.contains_key("layout") { + return; + } + let flow_children = obj + .get("children") + .and_then(serde_json::Value::as_array) + .map(|kids| { + kids.iter() + .filter(|c| c.get("x").is_none_or(serde_json::Value::is_null)) + .count() + }) + .unwrap_or(0); + if flow_children >= 2 { + obj.insert( + "layout".to_string(), + serde_json::Value::String("vertical".to_string()), + ); + } +} + fn normalize_layout_keyword(obj: &mut serde_json::Map, key: &str) { let Some(serde_json::Value::String(value)) = obj.get_mut(key) else { return; diff --git a/crates/op-mcp/src/batch_design_tests.rs b/crates/op-mcp/src/batch_design_tests.rs index 4aa455c2c..a66073b19 100644 --- a/crates/op-mcp/src/batch_design_tests.rs +++ b/crates/op-mcp/src/batch_design_tests.rs @@ -1465,3 +1465,68 @@ fn a_lone_empty_operations_list_still_reports_its_own_error() { other => panic!("empty program silently accepted: {other:?}"), } } + +/// Measured test0711-1-ds: every section frame omitted `layout`, so the engine +/// laid [title, card rail] out as a ROW — each section title sat to the LEFT of +/// its cards and the rail ran off the screen. An omitted layout stacks. +#[test] +fn a_container_without_a_layout_stacks_its_children() { + let mut section = serde_json::json!({ + "type": "frame", "id": "n6", "name": "Popular Destinations", + "width": "fill_container", "height": 240, + "children": [ + { "type": "frame", "id": "n25", "name": "Section Header", "layout": "horizontal" }, + { "type": "frame", "id": "n28", "name": "Rail", "layout": "horizontal" } + ] + }); + crate::batch_design::normalize_node_shape(&mut section); + assert_eq!( + section.get("layout").and_then(|v| v.as_str()), + Some("vertical"), + "the section stacks its title above its rail" + ); +} + +#[test] +fn an_authored_layout_and_an_absolute_stack_are_left_alone() { + let mut row = serde_json::json!({ + "type": "frame", "id": "row", "layout": "horizontal", + "children": [ + { "type": "frame", "id": "a" }, + { "type": "frame", "id": "b" } + ] + }); + crate::batch_design::normalize_node_shape(&mut row); + assert_eq!( + row.get("layout").and_then(|v| v.as_str()), + Some("horizontal") + ); + + // Absolute children are out of flow — direction is irrelevant, and the + // caller may mean `layout: none`. + let mut overlay = serde_json::json!({ + "type": "frame", "id": "overlay", + "children": [ + { "type": "frame", "id": "badge", "x": 8, "y": 8 }, + { "type": "frame", "id": "heart", "x": 300, "y": 8 } + ] + }); + crate::batch_design::normalize_node_shape(&mut overlay); + assert!( + overlay.get("layout").is_none(), + "an absolutely-positioned stack keeps its authored (absent) layout" + ); +} + +#[test] +fn a_single_child_container_is_not_given_a_direction() { + let mut wrapper = serde_json::json!({ + "type": "frame", "id": "w", + "children": [{ "type": "image", "id": "img", "src": "data:image/png;base64,AA" }] + }); + crate::batch_design::normalize_node_shape(&mut wrapper); + assert!( + wrapper.get("layout").is_none(), + "one child has no direction to get wrong" + ); +} diff --git a/crates/op-orchestrator/src/prompt.rs b/crates/op-orchestrator/src/prompt.rs index 67ec74e4a..b79bcccd4 100644 --- a/crates/op-orchestrator/src/prompt.rs +++ b/crates/op-orchestrator/src/prompt.rs @@ -82,6 +82,9 @@ data array. PREFER a loop over copy-pasting near-identical I(...) calls. Each node object starts with type ("frame"/"text"/"rectangle"/"ellipse"/"path"/"icon_font") and uses camelCase props (cornerRadius, fontSize, fontWeight, justifyContent, alignItems, clipContent). Do NOT set x/y on children inside layout frames. +EVERY frame with children MUST declare layout ("vertical" or "horizontal"; "none" for an +absolute stack). A section that holds a title and a card rail is layout:"vertical" — omitting +it stacks by default, but say it, because a row is only ever a row when you write it. Example: const sec = I(null, {type:"frame", name:"Clients", layout:"vertical", width:"fill_container", gap:0}); const tbl = I(sec, {type:"frame", layout:"vertical", width:"fill_container"});