feat(orchestrator): app-shell restructure for flat sidebar dashboards
Weak models emit a desktop dashboard's sidebar either as a full-width band on a vertical root or crammed into a horizontal row with every section — both render broken (the removed bespoke scaffold used to pre-build the two-column root). app_shell::reshape_sidebar_to_app_shell detects a sidebar dashboard and rewrites it to a horizontal [sidebar(260) | content-column(fill)] shell, run in the shared cleanup::run_cleanup_passes finalize point (covers orchestrator + agentic loop). A layout==vertical|none guard on fix_horizontal_overflow stops it re-widening the narrowed sidebar. Detection is hardened (strong sidebar token + structural dashboard-content gate) against restaurant/landing/top-nav/mobile/multi-screen false-positives; 14 unit + 1 integration test, verified end-to-end via op-smoke.
This commit is contained in:
Fini
parent
e830759cab
commit
46fe45a5b0
318
crates/op-orchestrator/src/app_shell.rs
Normal file
318
crates/op-orchestrator/src/app_shell.rs
Normal file
|
|
@ -0,0 +1,318 @@
|
|||
//! App-shell restructure — a single deterministic structural post-pass that
|
||||
//! turns a flat-vertical desktop dashboard whose FIRST child is a full-width
|
||||
//! sidebar into a horizontal `[sidebar(fixed) | content-column(fill)]` shell.
|
||||
//!
|
||||
//! Weak models (glm-5.x etc.) routinely emit a "Sidebar Navigation" as the
|
||||
//! first child of a vertical root at the *full* page width, so it renders as a
|
||||
//! full-width top band instead of a narrow left column. The orchestrator's old
|
||||
//! plan-level "dashboard bespoke scaffold" (that pre-built the two-column root)
|
||||
//! was removed in 346bcaa8; this is its lightweight node-tree replacement — one
|
||||
//! pass, no row/slot bin-packing, no placeholder-height estimation.
|
||||
//!
|
||||
//! Runs once over each assembled page-root in `cleanup::run_cleanup_passes` —
|
||||
//! the whole-doc finalize point SHARED by the orchestrator (per-subtask role
|
||||
//! passes already ran) and the agentic loop (`apply_loop_finalize` ran the
|
||||
//! whole-doc role passes) — so the moved sections keep their resolved roles.
|
||||
//!
|
||||
//! Both broken shapes the orchestrator emits are handled: a VERTICAL root with
|
||||
//! the sidebar stacked as a full-width band, and a HORIZONTAL root with the
|
||||
//! sidebar AND every content section crammed into one row. Detection is
|
||||
//! intentionally strict (see [`detect`]); the adversarial design review flagged
|
||||
//! restaurant "Menu" pages, "Navy" hero sections, top-nav bars, mobile/tablet
|
||||
//! roots, and multi-screen files as the false-positives to avoid, so the gate
|
||||
//! leans on a STRONG sidebar-name token + a real dashboard-content gate (a short
|
||||
//! numeric header height is rejected, but `fit_content` sidebars are allowed).
|
||||
|
||||
use jian_ops_schema::node::PenNode;
|
||||
use serde_json::{json, Value};
|
||||
|
||||
/// Fixed left-sidebar column width. Mirrors the surviving sizing constant in
|
||||
/// `dashboard_columns.rs` for parity with the removed scaffold.
|
||||
const SIDEBAR_WIDTH: f64 = 260.0;
|
||||
/// Desktop floor — excludes phones/tablets. Mirrors
|
||||
/// `cleanup_desktop_dashboard::DESKTOP_DASHBOARD_MIN_WIDTH`.
|
||||
const DESKTOP_MIN_WIDTH: f64 = 900.0;
|
||||
/// A real sidebar is full-height; a 64–96px full-width band is a header.
|
||||
const MIN_SIDEBAR_HEIGHT: f64 = 200.0;
|
||||
|
||||
/// Restructure a flat-vertical desktop dashboard whose first child is a
|
||||
/// full-width sidebar into a horizontal `[sidebar(fixed) | content(fill)]`
|
||||
/// app-shell. Mutates the page-root wrapper in place via the same serialize →
|
||||
/// mutate `Value` → deserialize round-trip the section passes use. Returns
|
||||
/// `true` iff it restructured; no-op + `false` when the strict detection
|
||||
/// criteria do not hold or the round-trip fails (the node is never dropped).
|
||||
pub(crate) fn reshape_sidebar_to_app_shell(wrapper: &mut PenNode) -> bool {
|
||||
let Ok(mut v) = serde_json::to_value(&*wrapper) else {
|
||||
return false;
|
||||
};
|
||||
if !detect(&v) {
|
||||
return false;
|
||||
}
|
||||
if !restructure(&mut v) {
|
||||
return false;
|
||||
}
|
||||
match serde_json::from_value::<PenNode>(v) {
|
||||
Ok(new_node) => {
|
||||
*wrapper = new_node;
|
||||
true
|
||||
}
|
||||
// Bad round-trip: leave the wrapper exactly as it was.
|
||||
Err(_) => false,
|
||||
}
|
||||
}
|
||||
|
||||
// ── Value readers (mirror the tolerant accessors the sibling passes use) ──
|
||||
|
||||
/// `width`/`height`/`gap` as f64, tolerant of numeric strings like `"605"`.
|
||||
/// Returns `None` for keyword sizings (`fill_container` / `fit_content`).
|
||||
fn num(v: &Value, key: &str) -> Option<f64> {
|
||||
let field = v.get(key)?;
|
||||
field
|
||||
.as_f64()
|
||||
.or_else(|| field.as_str().and_then(|s| s.parse::<f64>().ok()))
|
||||
}
|
||||
|
||||
fn layout_str(v: &Value) -> Option<&str> {
|
||||
v.get("layout").and_then(Value::as_str)
|
||||
}
|
||||
|
||||
/// Lowercased `name + id` identity text used for keyword matching.
|
||||
fn ident_text(v: &Value) -> String {
|
||||
let name = v.get("name").and_then(Value::as_str).unwrap_or("");
|
||||
let id = v.get("id").and_then(Value::as_str).unwrap_or("");
|
||||
format!("{name} {id}").to_lowercase()
|
||||
}
|
||||
|
||||
/// A STRONG left-rail signal. Deliberately excludes the bare `"nav"` / `"menu"`
|
||||
/// substrings (they match "Navy Hero", restaurant "Menu", "Navigation Guide"),
|
||||
/// and excludes anything that reads as a top bar.
|
||||
fn is_sidebar_named(t: &str) -> bool {
|
||||
let strong = t.contains("sidebar")
|
||||
|| t.contains("side bar")
|
||||
|| t.contains("side nav")
|
||||
|| t.contains("side-nav")
|
||||
|| t.contains("left rail")
|
||||
|| t.contains("left nav")
|
||||
|| t.contains("nav rail")
|
||||
|| t.contains("navigation rail")
|
||||
|| t.contains("nav-rail");
|
||||
strong && !is_topbar_named(t)
|
||||
}
|
||||
|
||||
fn is_topbar_named(t: &str) -> bool {
|
||||
t.contains("top bar")
|
||||
|| t.contains("topbar")
|
||||
|| t.contains("top nav")
|
||||
|| t.contains("top-nav")
|
||||
|| t.contains("header")
|
||||
|| t.contains("app bar")
|
||||
|| t.contains("appbar")
|
||||
|| t.contains("breadcrumb")
|
||||
}
|
||||
|
||||
/// True when a section subtree carries a dashboard-grade signal (a table, a
|
||||
/// metric/KPI/stat block, or a chart). Used as the STRUCTURAL intent gate — far
|
||||
/// more robust than a root-name keyword (the bug root "Barbershop Client
|
||||
/// Management" carries no dashboard word, but its sections are "Key Metrics" +
|
||||
/// "Client Table"). Recurses the whole section.
|
||||
fn section_has_dashboard_signal(v: &Value) -> bool {
|
||||
let t = ident_text(v);
|
||||
if t.contains("table")
|
||||
|| t.contains("metric")
|
||||
|| t.contains("stat")
|
||||
|| t.contains("kpi")
|
||||
|| t.contains("chart")
|
||||
|| t.contains("graph")
|
||||
|| t.contains("analytics")
|
||||
|| t.contains("data grid")
|
||||
|| t.contains("datagrid")
|
||||
{
|
||||
return true;
|
||||
}
|
||||
v.get("children")
|
||||
.and_then(Value::as_array)
|
||||
.is_some_and(|kids| kids.iter().any(section_has_dashboard_signal))
|
||||
}
|
||||
|
||||
/// A child that is itself a whole screen/page (multi-screen file guard): named
|
||||
/// screen/page, or both wide and tall enough to be a standalone artboard.
|
||||
fn is_screen_like(v: &Value) -> bool {
|
||||
let t = ident_text(v);
|
||||
if t.contains("screen") || t.contains("artboard") || t.contains(" page") || t.ends_with("page")
|
||||
{
|
||||
return true;
|
||||
}
|
||||
matches!(num(v, "width"), Some(w) if w >= DESKTOP_MIN_WIDTH)
|
||||
&& matches!(num(v, "height"), Some(h) if h >= 700.0)
|
||||
}
|
||||
|
||||
/// Vertical / none / absent layout (a column or absolute container — NOT a row).
|
||||
fn is_column_layout(v: &Value) -> bool {
|
||||
!matches!(layout_str(v), Some("horizontal"))
|
||||
}
|
||||
|
||||
// ── Detection ──
|
||||
|
||||
/// All criteria must hold (see module doc for the false-positives each guards).
|
||||
fn detect(v: &Value) -> bool {
|
||||
// 1. Desktop width (numeric ≥ 900 — excludes mobile/tablet; a
|
||||
// keyword-width root is treated as out-of-scope, not reshaped).
|
||||
let Some(root_w) = num(v, "width") else {
|
||||
return false;
|
||||
};
|
||||
if root_w < DESKTOP_MIN_WIDTH {
|
||||
return false;
|
||||
}
|
||||
// 2. Sidebar + at least two content sections. A *vertical* root has the
|
||||
// sidebar stacked as a full-width band; a *horizontal* root has the
|
||||
// sidebar AND every content section crammed into one row (the orchestrator
|
||||
// assembles both shapes). Both are handled — only the already-correct
|
||||
// `[sidebar | content]` 2-child app-shell is left alone, which the ≥3
|
||||
// floor + the `Main Content` idempotency guard below cover.
|
||||
let Some(kids) = v.get("children").and_then(Value::as_array) else {
|
||||
return false;
|
||||
};
|
||||
if kids.len() < 3 {
|
||||
return false;
|
||||
}
|
||||
// Idempotency: never re-wrap a root that already carries our content column.
|
||||
if kids.iter().any(|c| ident_text(c).contains("main content")) {
|
||||
return false;
|
||||
}
|
||||
// 9. Multi-screen file: never app-shell a wrapper of standalone screens.
|
||||
if kids.iter().filter(|c| is_screen_like(c)).count() >= 2 {
|
||||
return false;
|
||||
}
|
||||
let first = &kids[0];
|
||||
// 4. First child reads as a sidebar (strong tokens, not a top bar).
|
||||
if !is_sidebar_named(&ident_text(first)) {
|
||||
return false;
|
||||
}
|
||||
// 5. First child is a column, not a horizontal nav row.
|
||||
if !is_column_layout(first) {
|
||||
return false;
|
||||
}
|
||||
// 6. First child is NOT a short top strip. A header/top-bar band is a small
|
||||
// EXPLICIT numeric height (64–96px); skip those. A `fit_content` /
|
||||
// `fill_container` (non-numeric) height is allowed — real sidebars are
|
||||
// frequently authored that way — because the STRONG sidebar-name token
|
||||
// (criterion 4) + the dashboard-content gate (criterion 8) already carry
|
||||
// the specificity; height is only used to reject short numeric headers.
|
||||
if matches!(num(first, "height"), Some(h) if h < MIN_SIDEBAR_HEIGHT) {
|
||||
return false;
|
||||
}
|
||||
// 7. First child spans ~full width (the actual bug signature). A child
|
||||
// already narrower than half the root is an existing left column → skip.
|
||||
let first_is_full_width = match first.get("width") {
|
||||
Some(Value::String(s)) if s == "fill_container" => true,
|
||||
None => true,
|
||||
_ => matches!(num(first, "width"), Some(w) if w >= 0.9 * root_w),
|
||||
};
|
||||
if !first_is_full_width {
|
||||
return false;
|
||||
}
|
||||
if matches!(num(first, "width"), Some(w) if w < 0.5 * root_w) {
|
||||
return false;
|
||||
}
|
||||
// 8. STRUCTURAL dashboard gate — ≥2 content sections carry a table / metric
|
||||
// / chart signal. Kills restaurant-menu / landing / portfolio / settings
|
||||
// false-positives without depending on the (too-narrow) root name.
|
||||
let dashboard_sections = kids[1..]
|
||||
.iter()
|
||||
.filter(|s| section_has_dashboard_signal(s))
|
||||
.count();
|
||||
dashboard_sections >= 2
|
||||
}
|
||||
|
||||
// ── Restructure ──
|
||||
|
||||
/// Recursively retarget any descendant whose width was sized to (about) the OLD
|
||||
/// full root width down to `fill_container`, so full-width sections/dividers
|
||||
/// fill their new narrower column instead of overflowing it (taffy never
|
||||
/// shrinks the cross axis). Targeted: only ~full-width Number widths change.
|
||||
fn fill_full_width_descendants(v: &mut Value, old_root_w: f64) {
|
||||
if let Some(w) = num(v, "width") {
|
||||
if w >= 0.9 * old_root_w {
|
||||
if let Some(obj) = v.as_object_mut() {
|
||||
obj.insert("width".into(), json!("fill_container"));
|
||||
}
|
||||
}
|
||||
}
|
||||
if let Some(kids) = v.get_mut("children").and_then(Value::as_array_mut) {
|
||||
for c in kids.iter_mut() {
|
||||
fill_full_width_descendants(c, old_root_w);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Detection already passed: split the wrapper's children into `[sidebar |
|
||||
/// content-column]` and flip the wrapper to a horizontal app-shell. Returns
|
||||
/// `false` only if the tree shape changed under us (defensive).
|
||||
fn restructure(v: &mut Value) -> bool {
|
||||
let root_w = match num(v, "width") {
|
||||
Some(w) => w,
|
||||
None => return false,
|
||||
};
|
||||
let content_gap = num(v, "gap").filter(|g| *g > 0.0).unwrap_or(24.0);
|
||||
let content_id = format!(
|
||||
"{}-content",
|
||||
v.get("id").and_then(Value::as_str).unwrap_or("root")
|
||||
);
|
||||
|
||||
let Some(obj) = v.as_object_mut() else {
|
||||
return false;
|
||||
};
|
||||
let Some(kids) = obj.get_mut("children").and_then(Value::as_array_mut) else {
|
||||
return false;
|
||||
};
|
||||
if kids.len() < 3 {
|
||||
return false;
|
||||
}
|
||||
let mut sidebar = kids.remove(0);
|
||||
let mut content_sections: Vec<Value> = std::mem::take(kids);
|
||||
|
||||
// Sidebar → fixed narrow vertical column that stretches to the row height.
|
||||
// `height: fill_container` is the LOAD-BEARING cross-axis stretch (jian maps
|
||||
// `alignItems: stretch` to FlexStart, so the wrapper's alignItems alone
|
||||
// would NOT stretch it). Clip + width-retarget keep its old full-width
|
||||
// descendants from bleeding over the content column.
|
||||
if let Some(s) = sidebar.as_object_mut() {
|
||||
s.insert("width".into(), json!(SIDEBAR_WIDTH));
|
||||
s.insert("height".into(), json!("fill_container"));
|
||||
if s.get("layout").and_then(Value::as_str) != Some("vertical") {
|
||||
s.insert("layout".into(), json!("vertical"));
|
||||
}
|
||||
s.insert("clipContent".into(), json!(true));
|
||||
}
|
||||
fill_full_width_descendants(&mut sidebar, root_w);
|
||||
|
||||
// Content sections → fill the new column instead of the old full root width.
|
||||
for section in content_sections.iter_mut() {
|
||||
fill_full_width_descendants(section, root_w);
|
||||
}
|
||||
|
||||
let content = json!({
|
||||
"type": "frame",
|
||||
"id": content_id,
|
||||
"name": "Main Content",
|
||||
"width": "fill_container",
|
||||
"height": "fit_content",
|
||||
"layout": "vertical",
|
||||
"gap": content_gap,
|
||||
"children": content_sections,
|
||||
});
|
||||
|
||||
obj.insert("children".into(), json!([sidebar, content]));
|
||||
obj.insert("layout".into(), json!("horizontal"));
|
||||
obj.insert("gap".into(), json!(0));
|
||||
obj.insert("alignItems".into(), json!("stretch"));
|
||||
// The old height was the SUM of the vertical stack (incl. the sidebar); the
|
||||
// horizontal shell only needs the taller column. Track content instead of
|
||||
// leaving ~600px of dead canvas below.
|
||||
obj.insert("height".into(), json!("fit_content"));
|
||||
true
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "app_shell_tests.rs"]
|
||||
mod tests;
|
||||
337
crates/op-orchestrator/src/app_shell_tests.rs
Normal file
337
crates/op-orchestrator/src/app_shell_tests.rs
Normal file
|
|
@ -0,0 +1,337 @@
|
|||
//! Tests for the app-shell restructure pass. The positive case is the reported
|
||||
//! glm barbershop dashboard; the negatives are the false-positives the
|
||||
//! adversarial design review flagged (top-nav, short header, mobile, already
|
||||
//! horizontal, already-narrow sidebar, too-few sections, restaurant "Menu",
|
||||
//! "Navy" hero, `fit_content` nav, no dashboard content, multi-screen file).
|
||||
|
||||
use super::*;
|
||||
|
||||
fn node(v: Value) -> PenNode {
|
||||
serde_json::from_value::<PenNode>(v).expect("valid PenNode fixture")
|
||||
}
|
||||
|
||||
fn val(n: &PenNode) -> Value {
|
||||
serde_json::to_value(n).expect("serialize PenNode")
|
||||
}
|
||||
|
||||
/// A leaf section frame with a name + sizing (children optional).
|
||||
fn section(name: &str, width: Value, height: Value) -> Value {
|
||||
json!({
|
||||
"type": "frame", "id": name.replace(' ', "-"), "name": name,
|
||||
"width": width, "height": height, "layout": "vertical", "children": []
|
||||
})
|
||||
}
|
||||
|
||||
/// The reported bug shape: vertical 1200-wide root, full-width sidebar first.
|
||||
fn bug_wrapper() -> PenNode {
|
||||
node(json!({
|
||||
"type": "frame", "id": "root", "name": "Barbershop Client Management",
|
||||
"width": 1200, "height": 1775, "layout": "vertical", "gap": 32,
|
||||
"children": [
|
||||
{ "type": "frame", "id": "n2", "name": "Sidebar Navigation",
|
||||
"width": 1200, "height": 605, "layout": "vertical",
|
||||
"children": [ section("Logo", json!(1152), json!(27)) ] },
|
||||
section("Top Header", json!(1200), json!(94)),
|
||||
section("Key Metrics", json!(1200), json!(117)),
|
||||
section("Client Table Section", json!(1200), json!(488)),
|
||||
section("Upcoming Appointments", json!(1200), json!(391)),
|
||||
]
|
||||
}))
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn positive_full_width_sidebar_dashboard_restructured() {
|
||||
let mut w = bug_wrapper();
|
||||
assert!(
|
||||
reshape_sidebar_to_app_shell(&mut w),
|
||||
"bug shape must restructure"
|
||||
);
|
||||
let v = val(&w);
|
||||
assert_eq!(
|
||||
layout_str(&v),
|
||||
Some("horizontal"),
|
||||
"root flips to horizontal"
|
||||
);
|
||||
let kids = v["children"].as_array().unwrap();
|
||||
assert_eq!(kids.len(), 2, "[sidebar | content]");
|
||||
|
||||
let sidebar = &kids[0];
|
||||
assert!(ident_text(sidebar).contains("sidebar"));
|
||||
assert_eq!(
|
||||
num(sidebar, "width"),
|
||||
Some(SIDEBAR_WIDTH),
|
||||
"sidebar pinned to 260"
|
||||
);
|
||||
assert_eq!(
|
||||
sidebar["height"],
|
||||
json!("fill_container"),
|
||||
"sidebar stretches"
|
||||
);
|
||||
assert_eq!(sidebar["clipContent"], json!(true));
|
||||
|
||||
let content = &kids[1];
|
||||
assert_eq!(content["name"], json!("Main Content"));
|
||||
assert_eq!(content["width"], json!("fill_container"));
|
||||
assert_eq!(layout_str(content), Some("vertical"));
|
||||
let sections: Vec<&str> = content["children"]
|
||||
.as_array()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.map(|s| s["name"].as_str().unwrap())
|
||||
.collect();
|
||||
assert_eq!(
|
||||
sections,
|
||||
[
|
||||
"Top Header",
|
||||
"Key Metrics",
|
||||
"Client Table Section",
|
||||
"Upcoming Appointments"
|
||||
],
|
||||
"sections moved into the content column IN ORDER"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn positive_full_width_sections_retargeted_to_fill() {
|
||||
let mut w = bug_wrapper();
|
||||
reshape_sidebar_to_app_shell(&mut w);
|
||||
let v = val(&w);
|
||||
let content = &v["children"][1];
|
||||
// The 1200-wide sections must become fill_container so they don't overflow
|
||||
// the ~940 content column.
|
||||
for s in content["children"].as_array().unwrap() {
|
||||
assert_eq!(
|
||||
s["width"],
|
||||
json!("fill_container"),
|
||||
"section {} retargeted",
|
||||
s["name"]
|
||||
);
|
||||
}
|
||||
// The sidebar's 1152-wide logo child likewise fills the 260 column.
|
||||
let logo = &v["children"][0]["children"][0];
|
||||
assert_eq!(logo["width"], json!("fill_container"));
|
||||
}
|
||||
|
||||
fn assert_untouched(mut w: PenNode, why: &str) {
|
||||
let before = val(&w);
|
||||
assert!(
|
||||
!reshape_sidebar_to_app_shell(&mut w),
|
||||
"must NOT restructure: {why}"
|
||||
);
|
||||
assert_eq!(val(&w), before, "node unchanged: {why}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn negative_top_nav_untouched() {
|
||||
assert_untouched(
|
||||
node(json!({
|
||||
"type": "frame", "id": "r", "name": "Dashboard", "width": 1200, "layout": "vertical",
|
||||
"children": [
|
||||
{ "type": "frame", "id": "tn", "name": "Top Navigation", "width": 1200,
|
||||
"height": 72, "layout": "horizontal", "children": [] },
|
||||
section("Key Metrics", json!(1200), json!(117)),
|
||||
section("Client Table", json!(1200), json!(400)),
|
||||
]
|
||||
})),
|
||||
"top navigation (topbar keyword + horizontal + short)",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn negative_header_short_strip_untouched() {
|
||||
assert_untouched(
|
||||
node(json!({
|
||||
"type": "frame", "id": "r", "name": "Dashboard", "width": 1200, "layout": "vertical",
|
||||
"children": [
|
||||
section("Header", json!(1200), json!(80)),
|
||||
section("Metrics Grid", json!(1200), json!(117)),
|
||||
section("Data Table", json!(1200), json!(400)),
|
||||
]
|
||||
})),
|
||||
"short full-width header, not a sidebar",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn negative_mobile_untouched() {
|
||||
assert_untouched(
|
||||
node(json!({
|
||||
"type": "frame", "id": "r", "name": "App", "width": 390, "layout": "vertical",
|
||||
"children": [
|
||||
section("Sidebar Navigation", json!(390), json!(605)),
|
||||
section("Key Metrics", json!(390), json!(117)),
|
||||
section("Table", json!(390), json!(400)),
|
||||
]
|
||||
})),
|
||||
"mobile width < 900",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn negative_already_horizontal_untouched() {
|
||||
assert_untouched(
|
||||
node(json!({
|
||||
"type": "frame", "id": "r", "name": "Dashboard", "width": 1200, "layout": "horizontal",
|
||||
"children": [
|
||||
section("Sidebar Navigation", json!(260), json!(605)),
|
||||
section("Main Content", json!("fill_container"), json!("fit_content")),
|
||||
]
|
||||
})),
|
||||
"already app-shelled (horizontal root)",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn negative_already_narrow_sidebar_untouched() {
|
||||
assert_untouched(
|
||||
node(json!({
|
||||
"type": "frame", "id": "r", "name": "Dashboard", "width": 1200, "layout": "vertical",
|
||||
"children": [
|
||||
section("Sidebar Navigation", json!(240), json!(605)),
|
||||
section("Key Metrics", json!(1200), json!(117)),
|
||||
section("Client Table", json!(1200), json!(400)),
|
||||
]
|
||||
})),
|
||||
"sidebar already a narrow left column (240 < 0.5*1200)",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn negative_fewer_than_two_content_sections() {
|
||||
assert_untouched(
|
||||
node(json!({
|
||||
"type": "frame", "id": "r", "name": "Dashboard", "width": 1200, "layout": "vertical",
|
||||
"children": [
|
||||
section("Sidebar Navigation", json!(1200), json!(605)),
|
||||
section("Client Table", json!(1200), json!(400)),
|
||||
]
|
||||
})),
|
||||
"only one content section (len < 3)",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn negative_restaurant_menu_untouched() {
|
||||
assert_untouched(
|
||||
node(json!({
|
||||
"type": "frame", "id": "r", "name": "Restaurant", "width": 1280, "layout": "vertical",
|
||||
"children": [
|
||||
section("Menu", json!("fill_container"), json!(900)),
|
||||
section("About", json!(1280), json!(300)),
|
||||
section("Hours", json!(1280), json!(200)),
|
||||
section("Footer", json!(1280), json!(120)),
|
||||
]
|
||||
})),
|
||||
"'Menu' is not a strong sidebar token (no sidebar/rail)",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn negative_navy_hero_untouched() {
|
||||
assert_untouched(
|
||||
node(json!({
|
||||
"type": "frame", "id": "r", "name": "Landing", "width": 1440, "layout": "vertical",
|
||||
"children": [
|
||||
section("Navy Hero", json!("fill_container"), json!(640)),
|
||||
section("Features", json!(1440), json!(400)),
|
||||
section("Pricing", json!(1440), json!(500)),
|
||||
]
|
||||
})),
|
||||
"'Navy Hero' contains 'nav' substring but is not a sidebar",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn negative_weak_nav_name_untouched() {
|
||||
// A WEAK nav name ("Navigation", no "sidebar"/rail token) is the real
|
||||
// false-positive risk now that fit_content heights are allowed — it must be
|
||||
// excluded by the strong-token name gate (criterion 4), not the height.
|
||||
assert_untouched(
|
||||
node(json!({
|
||||
"type": "frame", "id": "r", "name": "Dashboard", "width": 1200, "layout": "vertical",
|
||||
"children": [
|
||||
{ "type": "frame", "id": "nav", "name": "Navigation",
|
||||
"width": "fill_container", "height": "fit_content", "layout": "vertical",
|
||||
"children": [] },
|
||||
section("Key Metrics", json!(1200), json!(117)),
|
||||
section("Client Table", json!(1200), json!(400)),
|
||||
]
|
||||
})),
|
||||
"'Navigation' is not a strong sidebar token (criterion 4)",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn positive_horizontal_root_fit_content_sidebar_restructured() {
|
||||
// The orchestrator also emits the bug as a HORIZONTAL root with the sidebar
|
||||
// AND every section crammed into one row, the sidebar sized fill_container /
|
||||
// fit_content (the op-smoke barbershop output). This must restructure too.
|
||||
let mut w = node(json!({
|
||||
"type": "frame", "id": "r", "name": "Barbershop Dashboard",
|
||||
"width": 1200, "layout": "horizontal", "gap": 24,
|
||||
"children": [
|
||||
{ "type": "frame", "id": "sb", "name": "Left Sidebar Navigation",
|
||||
"width": "fill_container", "height": "fit_content", "layout": "vertical",
|
||||
"children": [] },
|
||||
section("Top Header Bar", json!("fill_container"), json!("fit_content")),
|
||||
section("Key Metrics Row", json!("fill_container"), json!("fit_content")),
|
||||
section("Recent Clients Table", json!("fill_container"), json!("fit_content")),
|
||||
]
|
||||
}));
|
||||
assert!(
|
||||
reshape_sidebar_to_app_shell(&mut w),
|
||||
"horizontal-root fit_content sidebar must restructure"
|
||||
);
|
||||
let v = val(&w);
|
||||
assert_eq!(layout_str(&v), Some("horizontal"));
|
||||
let kids = v["children"].as_array().unwrap();
|
||||
assert_eq!(kids.len(), 2, "[sidebar | Main Content]");
|
||||
assert_eq!(
|
||||
num(&kids[0], "width"),
|
||||
Some(SIDEBAR_WIDTH),
|
||||
"sidebar narrowed to 260"
|
||||
);
|
||||
assert_eq!(kids[1]["name"], json!("Main Content"));
|
||||
let inner: Vec<&str> = kids[1]["children"]
|
||||
.as_array()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.map(|s| s["name"].as_str().unwrap())
|
||||
.collect();
|
||||
assert_eq!(
|
||||
inner,
|
||||
["Top Header Bar", "Key Metrics Row", "Recent Clients Table"],
|
||||
"the row sections moved into the content column"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn negative_no_dashboard_content_untouched() {
|
||||
assert_untouched(
|
||||
node(json!({
|
||||
"type": "frame", "id": "r", "name": "Marketing Site", "width": 1280, "layout": "vertical",
|
||||
"children": [
|
||||
section("Sidebar Navigation", json!(1280), json!(605)),
|
||||
section("About Us", json!(1280), json!(300)),
|
||||
section("Our Team", json!(1280), json!(300)),
|
||||
section("Contact", json!(1280), json!(200)),
|
||||
]
|
||||
})),
|
||||
"no table/metric/chart sections (structural dashboard gate)",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn negative_multiscreen_wrapper_untouched() {
|
||||
assert_untouched(
|
||||
node(json!({
|
||||
"type": "frame", "id": "r", "name": "Flows", "width": 1440, "layout": "vertical",
|
||||
"children": [
|
||||
section("Navigation Rail Screen", json!(1440), json!(900)),
|
||||
section("Detail Screen", json!(1440), json!(900)),
|
||||
section("Settings Screen", json!(1440), json!(900)),
|
||||
]
|
||||
})),
|
||||
"multiple standalone screens, not dashboard sections",
|
||||
);
|
||||
}
|
||||
|
|
@ -673,6 +673,14 @@ pub fn finalize_design(sink: &mut dyn DocSink, plan: &OrchestratorPlan, root_ids
|
|||
|
||||
pub fn run_cleanup_passes(sink: &mut dyn DocSink, plan: &OrchestratorPlan, root_ids: &[&str]) {
|
||||
for root_id in root_ids {
|
||||
// FIRST: turn a flat-vertical desktop dashboard with a full-width
|
||||
// sidebar into a horizontal [sidebar | content] app-shell, so the
|
||||
// remaining cleanup passes (esp. `adjust_root_height_to_content`) see
|
||||
// the corrected shape. This is the shared whole-doc finalize point for
|
||||
// BOTH the orchestrator (per-subtask role passes already ran) and the
|
||||
// agentic loop (whole-doc role passes ran in `apply_loop_finalize`), so
|
||||
// the moved sections keep their resolved roles.
|
||||
restructure_app_shell_root(sink, root_id);
|
||||
remove_duplicate_status_bars(sink, root_id);
|
||||
repair_light_mobile_nav_surfaces(sink, root_id);
|
||||
repair_mobile_content_sections(sink, root_id);
|
||||
|
|
@ -685,6 +693,26 @@ pub fn run_cleanup_passes(sink: &mut dyn DocSink, plan: &OrchestratorPlan, root_
|
|||
}
|
||||
}
|
||||
|
||||
/// Restructure the page-root in place when it is a flat-vertical desktop
|
||||
/// dashboard whose first child is a full-width sidebar (see
|
||||
/// [`crate::app_shell::reshape_sidebar_to_app_shell`] for the strict gate). The
|
||||
/// whole subtree is swapped via `ReplaceSubtree` (the new root carries the
|
||||
/// reorganized children, so `drop_children: true` loses nothing).
|
||||
fn restructure_app_shell_root(sink: &mut dyn DocSink, root_id: &str) {
|
||||
let Some(root) = find_root(sink.state(), root_id) else {
|
||||
return;
|
||||
};
|
||||
let mut new_root = root.clone();
|
||||
if crate::app_shell::reshape_sidebar_to_app_shell(&mut new_root) {
|
||||
sink.apply(EditorCommand::ReplaceSubtree {
|
||||
node_id: NodeId::new(root_id.to_string()),
|
||||
node: Box::new(new_root),
|
||||
drop_children: true,
|
||||
page_id: None,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/// Strip the REDUNDANT border off a filled, shadowed container. When a
|
||||
/// frame / group / rectangle has a fill AND a drop shadow AND a stroke,
|
||||
/// the stroke is a "莫名其妙" hairline — the shadow already separates the
|
||||
|
|
|
|||
|
|
@ -41,6 +41,7 @@ pub(crate) mod validation_fixes_b3;
|
|||
mod variable_binding;
|
||||
pub mod variables;
|
||||
|
||||
pub mod app_shell;
|
||||
pub mod append;
|
||||
pub mod cleanup;
|
||||
pub(crate) mod cleanup_layout;
|
||||
|
|
|
|||
|
|
@ -192,6 +192,12 @@ pub fn apply_loop_finalize(state: &mut EditorState) {
|
|||
crate::role_post_pass::enforce_surface_color_discipline(forest);
|
||||
}
|
||||
|
||||
// The app-shell restructure (flat-vertical sidebar dashboard → horizontal
|
||||
// [sidebar | content]) runs inside `cleanup::finalize_design` below, the
|
||||
// whole-doc finalize point SHARED with the orchestrator path — so both the
|
||||
// agentic loop and the orchestrator get it exactly once, after roles are
|
||||
// resolved. See `cleanup::run_cleanup_passes`.
|
||||
|
||||
// -- Per-root cleanup (`cleanup::finalize_design`) over every top-level
|
||||
// root, via a borrowed-state DocSink. The cleanup passes are whole-root
|
||||
// (they take the page-root id and recurse), so they always run over the
|
||||
|
|
|
|||
|
|
@ -177,3 +177,79 @@ fn loop_finalize_on_empty_doc_is_noop() {
|
|||
apply_loop_finalize(&mut state);
|
||||
assert!(state.active_children().is_empty());
|
||||
}
|
||||
|
||||
/// The reported glm bug shape (a single page-root wrapper whose first child is a
|
||||
/// full-width sidebar) is restructured into a horizontal `[sidebar | Main
|
||||
/// Content]` app-shell by `apply_loop_finalize`, AND the restructure survives
|
||||
/// the subsequent Class-A passes — in particular the sidebar is NOT re-widened
|
||||
/// by `fix_horizontal_overflow` (the guard added for this).
|
||||
#[test]
|
||||
fn loop_finalize_restructures_sidebar_dashboard() {
|
||||
let mut state = state_with_forest(json!([
|
||||
{
|
||||
"type": "frame", "id": "root", "name": "Barbershop Client Management",
|
||||
"width": 1200, "height": 1775, "layout": "vertical", "gap": 32,
|
||||
"children": [
|
||||
{ "type": "frame", "id": "sb", "name": "Sidebar Navigation",
|
||||
"width": 1200, "height": 605, "layout": "vertical",
|
||||
"children": [
|
||||
{ "type": "frame", "id": "logo", "name": "Logo",
|
||||
"width": 1152, "height": 27, "layout": "vertical", "children": [] }
|
||||
] },
|
||||
{ "type": "frame", "id": "hd", "name": "Top Header",
|
||||
"width": 1200, "height": 94, "layout": "vertical", "children": [] },
|
||||
{ "type": "frame", "id": "km", "name": "Key Metrics",
|
||||
"width": 1200, "height": 117, "layout": "horizontal", "children": [] },
|
||||
{ "type": "frame", "id": "ct", "name": "Client Table Section",
|
||||
"width": 1200, "height": 488, "layout": "vertical", "children": [] },
|
||||
{ "type": "frame", "id": "ua", "name": "Upcoming Appointments",
|
||||
"width": 1200, "height": 391, "layout": "vertical", "children": [] }
|
||||
]
|
||||
}
|
||||
]));
|
||||
apply_loop_finalize(&mut state);
|
||||
|
||||
let root = &state.active_children()[0];
|
||||
let rv = serde_json::to_value(root).unwrap();
|
||||
assert_eq!(
|
||||
rv.get("layout").and_then(|l| l.as_str()),
|
||||
Some("horizontal"),
|
||||
"root restructured to a horizontal app-shell"
|
||||
);
|
||||
assert_eq!(
|
||||
rv["children"].as_array().unwrap().len(),
|
||||
2,
|
||||
"[sidebar | Main Content]"
|
||||
);
|
||||
|
||||
// Sidebar stayed a narrow left column through the whole sequence (the
|
||||
// fix_horizontal_overflow guard must NOT re-widen this vertical frame).
|
||||
let sidebar = find_by_name(state.active_children(), "Sidebar Navigation").unwrap();
|
||||
let sv = serde_json::to_value(sidebar).unwrap();
|
||||
assert!(
|
||||
sv["width"].as_f64().is_some_and(|w| w <= 300.0),
|
||||
"sidebar must stay narrow, got {:?}",
|
||||
sv["width"]
|
||||
);
|
||||
|
||||
// The data sections moved into the new content column, in order.
|
||||
let content =
|
||||
find_by_name(state.active_children(), "Main Content").expect("content column created");
|
||||
let cv = serde_json::to_value(content).unwrap();
|
||||
let names: Vec<String> = cv["children"]
|
||||
.as_array()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.map(|s| s["name"].as_str().unwrap_or("").to_string())
|
||||
.collect();
|
||||
assert_eq!(
|
||||
names,
|
||||
[
|
||||
"Top Header",
|
||||
"Key Metrics",
|
||||
"Client Table Section",
|
||||
"Upcoming Appointments"
|
||||
],
|
||||
"data sections nested under Main Content in order"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -40,6 +40,20 @@ fn padding_lr(node: &Value) -> (f64, f64) {
|
|||
}
|
||||
|
||||
pub(crate) fn fix_horizontal_overflow(node: &mut Value, canvas_width: f64) {
|
||||
// Summing child widths as a ROW is only valid for a row layout. A
|
||||
// `vertical` column stacks its children (widths don't sum — the max
|
||||
// applies) and a `none` container positions them absolutely; running the
|
||||
// row-sum on those wrongly widens them — e.g. a narrow vertical sidebar of N
|
||||
// `fill_container` items (each counted as 80px) sums to ~80*N and gets
|
||||
// re-widened to a fraction of the canvas, undoing the app-shell pass. A
|
||||
// frame with no explicit `layout` defaults to a row, so only the explicit
|
||||
// column / absolute cases are skipped.
|
||||
if matches!(
|
||||
node.get("layout").and_then(Value::as_str),
|
||||
Some("vertical") | Some("none")
|
||||
) {
|
||||
return;
|
||||
}
|
||||
let parent_w = size_number(node, "width");
|
||||
if parent_w <= 0.0 {
|
||||
return;
|
||||
|
|
|
|||
Loading…
Reference in a new issue