SemianiakaVY/openpencil
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

fix(agent): harden generated mobile geometry

Browse source
This commit is contained in:
Fini 2026-07-31 00:29:22 +08:00
parent fd627e1654
commit f7004e34bc
15 changed files with 688 additions and 118 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
crates/op-orchestrator/src/cleanup.rs
View file

@ -525,6 +525,11 @@ pub fn run_cleanup_passes_with_summary(
// the corrected tree, not the pre-repair one.
crate::section_shell_fill_repair::repair_section_shell_fill_ownership(sink, rid);
counter.checkpoint(summary, CheckCategory::Structure);
// No-nav mobile screens share one deterministic closing contract:
// 24-32px of bottom room. The repair reads the same resolved geometry
// as the diagnostic and grows only root padding, never business nodes.
crate::geometry_validation::repair_mobile_bottom_breathing(sink, rid);
counter.checkpoint(summary, CheckCategory::Layout);
// Geometry-driven validation LOOP: run the REAL jian layout, detect +
// fix what the resolved rects prove wrong (table columns overflowing
// their row, fill containers collapsed to 0 height by a hugging ancestor

6
crates/op-orchestrator/src/cleanup_mobile_bottom_nav_dedup_tests.rs
View file

@ -101,7 +101,11 @@ fn bottom_nav_detected_by_cjk_name() {
// Authored absolute inset is CLEARED so the nav rejoins flex flow —
// a written x (even 0) is absolute placement and gets buried at (0,0).
assert_eq!(nav.base().x, None);
assert_eq!(nav.width_px(), Some(390.0));
assert_eq!(nav.base().role.as_deref(), Some("bottom-tab-bar"));
assert_eq!(
serde_json::to_value(nav).expect("nav serializes")["width"],
serde_json::json!("fill_container")
);
assert_eq!(nav.height_px(), Some(72.0));
}

20
crates/op-orchestrator/src/cleanup_mobile_chrome.rs
View file

@ -89,7 +89,6 @@ pub(crate) fn repair_mobile_structural_chrome(sink: &mut dyn DocSink, root_id: &
if !super::is_mobile_root(root) {
return;
}
let root_width = root.width_px().unwrap_or(0.0);
let Some(children) = root.children() else {
return;
};
@ -107,7 +106,7 @@ pub(crate) fn repair_mobile_structural_chrome(sink: &mut dyn DocSink, root_id: &
.structural_shells
.push(NodeId::new(child.id_str().to_string()));
}
collect_bottom_nav_chrome_repairs(child, root_width, allow_structural, &mut repairs);
collect_bottom_nav_chrome_repairs(child, allow_structural, &mut repairs);
}
repairs
};
@ -120,20 +119,18 @@ pub(crate) fn repair_mobile_structural_chrome(sink: &mut dyn DocSink, root_id: &
page_id: None,
});
}
for (node_id, root_width) in repairs.bottom_nav_surfaces {
for node_id in repairs.bottom_nav_surfaces {
// `x`/`y` are CLEARED, never set: any authored position reads as
// ABSOLUTE placement in jian and yanks the nav out of flex flow —
// the old `"x":0` patch (without y) pinned a healthy 4-tab
// BottomTabBar to the root's top-left corner where later-painted
// siblings buried it ("the navbar vanished at finalize",
// test0711-22 23:34 run). The nav is the root's last FLEX child;
// the numeric width alone gives the full-bleed span.
// test0711-22 23:34 run). `fill_container` also makes horizontal
// padding part of the root-width slot; a numeric root width plus
// padding resolves wider than the artboard in jian.
sink.apply(EditorCommand::PatchNodeData {
node_id,
patch_json: format!(
r#"{{"x":null,"y":null,"width":{},"height":72,"layout":"horizontal","gap":0,"padding":[8,16,8,16],"justifyContent":"space_between","alignItems":"center","stroke":null,"effects":null,"cornerRadius":0}}"#,
root_width.round()
),
patch_json: r#"{"role":"bottom-tab-bar","x":null,"y":null,"width":"fill_container","height":72,"layout":"horizontal","gap":0,"padding":[8,16,8,16],"justifyContent":"space_between","alignItems":"center","stroke":null,"effects":null,"cornerRadius":0}"#.to_string(),
page_id: None,
});
}
@ -337,7 +334,7 @@ fn contains_meaningful_business_content(node: &PenNode) -> bool {
#[derive(Default)]
struct MobileChromeRepairs {
structural_shells: Vec<NodeId>,
bottom_nav_surfaces: Vec<(NodeId, f64)>,
bottom_nav_surfaces: Vec<NodeId>,
bottom_nav_items: Vec<NodeId>,
}
@ -459,7 +456,6 @@ fn is_input_like_haystack(hay: &str) -> bool {
fn collect_bottom_nav_chrome_repairs(
root_child: &PenNode,
root_width: f64,
allow_structural: bool,
repairs: &mut MobileChromeRepairs,
) {
@ -468,7 +464,7 @@ fn collect_bottom_nav_chrome_repairs(
};
repairs
.bottom_nav_surfaces
.push((NodeId::new(nav.id_str().to_string()), root_width));
.push(NodeId::new(nav.id_str().to_string()));
let Some(children) = nav.children() else {
return;

6
crates/op-orchestrator/src/cleanup_mobile_chrome_nav_wrapper_tests.rs
View file

@ -284,7 +284,7 @@ fn mixed_business_wrapper_promotes_real_tabbar_to_mobile_root() {
assert_eq!(nav.height_px(), Some(72.0));
assert_eq!(
serde_json::to_value(nav).expect("nav serializes")["width"],
json!(375.0),
json!("fill_container"),
"normal nav normalization runs after promotion"
);
}
@ -510,7 +510,7 @@ fn explicit_horizontal_text_only_bottom_nav_is_not_demoted() {
assert_eq!(nav.base().role.as_deref(), Some("bottom-tab-bar"));
assert_eq!(nav.base().name.as_deref(), Some("Tab Bar"));
let nav_json = serde_json::to_value(nav).expect("nav serializes");
assert_eq!(nav_json["width"], json!(375.0));
assert_eq!(nav_json["width"], json!("fill_container"));
assert_eq!(nav_json["height"], json!(72.0));
assert_eq!(nav_json["layout"], json!("horizontal"));
}
@ -553,7 +553,7 @@ fn explicit_bottom_nav_with_missing_layout_is_repaired_not_demoted() {
let nav = find_node(root, "nav").expect("nav survives");
assert_eq!(nav.base().role.as_deref(), Some("bottom-tab-bar"));
let nav_json = serde_json::to_value(nav).expect("nav serializes");
assert_eq!(nav_json["width"], json!(375.0));
assert_eq!(nav_json["width"], json!("fill_container"));
assert_eq!(nav_json["height"], json!(72.0));
assert_eq!(nav_json["layout"], json!("horizontal"));
}

105
crates/op-orchestrator/src/cleanup_mobile_nav_chrome_tests.rs
View file

@ -121,7 +121,10 @@ fn cleanup_strips_mobile_bottom_nav_pill_chrome() {
let nav = find_node(root, "bottom-nav").expect("nav survives");
let home = find_node(root, "home-tab").expect("home tab survives");
let search = find_node(root, "search-tab").expect("search tab survives");
assert_eq!(nav.width_px(), Some(390.0));
assert_eq!(
serde_json::to_value(nav).expect("nav serializes")["width"],
json!("fill_container")
);
match nav {
PenNode::Frame(frame) => {
assert!(frame.container.stroke.is_none(), "nav stroke is removed");
@ -249,7 +252,7 @@ fn cleanup_normalizes_mobile_bottom_nav_spacing_and_tab_slots() {
.expect("root survives");
let nav = find_node(root, "bottom-nav").expect("nav survives");
let nav_json = serde_json::to_value(nav).expect("nav serializes");
assert_eq!(nav_json["width"], json!(390.0));
assert_eq!(nav_json["width"], json!("fill_container"));
assert_eq!(nav_json["height"], json!(72.0));
assert_eq!(nav_json["gap"], json!(0.0));
assert_eq!(nav_json["padding"], json!([8.0, 16.0, 8.0, 16.0]));
@ -324,7 +327,11 @@ fn cleanup_normalizes_structurally_detected_bottom_nav_without_role_or_name() {
.expect("root survives");
let nav = find_node(root, "footer").expect("nav survives");
let nav_json = serde_json::to_value(nav).expect("nav serializes");
assert_eq!(nav_json["width"], json!(390.0), "nav spread to full width");
assert_eq!(
nav_json["width"],
json!("fill_container"),
"nav spread to full width"
);
assert_eq!(nav_json["justifyContent"], json!("space_between"));
for id in ["home", "search", "orders", "profile"] {
let tab = find_node(root, id).expect("tab survives");
@ -333,6 +340,88 @@ fn cleanup_normalizes_structurally_detected_bottom_nav_without_role_or_name() {
}
}
#[test]
fn cjk_named_nav_stays_inside_real_jian_width_and_stamps_semantics() {
let mut sink = VecDocSink::new();
let tree: PenNode = serde_json::from_value(json!({
"type": "frame", "id": "root", "name": "Mobile", "width": 375, "height": 812,
"layout": "vertical",
"children": [
{
"type": "frame", "id": "header", "name": "Header Actions",
"width": 180, "height": 56, "layout": "horizontal",
"children": [
{"type": "frame", "id": "header-home", "role": "tab"},
{"type": "frame", "id": "header-search", "role": "tab"},
{"type": "frame", "id": "header-profile", "role": "tab"}
]
},
{
"type": "frame", "id": "content", "name": "Content",
"width": "fill_container", "height": 684
},
{
"type": "frame", "id": "nav", "name": "底部导航栏",
"width": 375, "height": 72, "layout": "horizontal",
"padding": [8, 16, 8, 16],
"children": [
{"type": "frame", "id": "home", "name": "Home Tab", "role": "tab"},
{"type": "frame", "id": "search", "name": "Search Tab", "role": "tab"},
{"type": "frame", "id": "profile", "name": "Profile Tab", "role": "tab"}
]
}
]
}))
.expect("CJK nav fixture");
sink.state.apply(EditorCommand::InsertAuthoredSubtree {
nodes: vec![tree],
parent_id: NodeId::NONE,
page_id: None,
});
sink.applied.clear();
crate::cleanup::repair_mobile_structural_chrome_for_all_roots(&mut sink);
let root = sink.state.active_children().first().expect("root");
let header = find_node(root, "header").expect("header");
let nav = find_node(root, "nav").expect("nav");
assert_eq!(
serde_json::to_value(header).expect("header serializes")["width"],
json!(180.0),
"the top header row must not be normalized as bottom navigation"
);
assert_eq!(nav.base().role.as_deref(), Some("bottom-tab-bar"));
assert_eq!(
serde_json::to_value(nav).expect("nav serializes")["width"],
json!("fill_container")
);
fn resolved_width(nodes: &[jian_scene::layout_scene::SceneNode], id: &str) -> Option<f64> {
nodes.iter().find_map(|node| {
(node.id == id)
.then(|| f64::from(node.aggregate_bounds().size.x))
.or_else(|| resolved_width(&node.children, id))
})
}
let scene = op_pen_loader::editor_state_to_active_page_layout_scene(&sink.state);
let page = scene.active_page().expect("active page");
let root_width = resolved_width(&page.children, "root").expect("resolved root");
let nav_width = resolved_width(&page.children, "nav").expect("resolved nav");
assert!(
(nav_width - root_width).abs() <= 1.0,
"nav padding must stay inside the root-width slot, got nav={nav_width} root={root_width}"
);
let issues = crate::geometry_validation::geometry_diagnostics(&sink.state);
assert!(
issues.iter().all(|issue| {
!issue.contains("mobile-bottom-flush")
&& !issue.contains("mobile-root-bottom-nav-overflow")
}),
"normalized bottom navigation must not echo as missing or overflowing: {issues:?}"
);
}
#[test]
fn cleanup_does_not_treat_header_action_row_as_bottom_nav() {
// Guard against over-broad structural detection: a HEADER row at the TOP —
@ -528,13 +617,9 @@ fn cleanup_structural_nav_only_matches_bottom_row_inside_single_wrapper() {
// The bottom nav row IS spread to full width.
let nav = find_node(root, "nav-row").expect("nav-row survives");
let nav_json = serde_json::to_value(nav).expect("nav serializes");
// Full-width in FLEX FLOW: numeric root-width or fill_container both
// qualify. The nav must NOT carry an authored x/y — that reads as
// absolute placement and buries it at the root's top-left corner.
assert!(
nav_json["width"] == json!(390.0) || nav_json["width"] == json!("fill_container"),
"the bottom nav row should still be spread full-width: {nav_json}"
);
// Full-width in FLEX FLOW: the nav must use fill sizing and carry no
// authored x/y, otherwise padding widens it or absolute-positions it.
assert_eq!(nav_json["width"], json!("fill_container"));
assert!(
nav_json.get("x").is_none_or(serde_json::Value::is_null),
"nav must stay in flex flow (no authored x): {nav_json}"

153
crates/op-orchestrator/src/geometry_bottom_gap.rs
View file

@ -6,11 +6,9 @@
//! deliberate exception: the nav IS the closing element and is supposed to sit
//! flush.
//!
//! This is detect-only. Whether a screen wants 24px, 32px, or a trailing
//! spacer is INTENT, and the "contract → auto-fix, intent → echo" split says
//! we report the fact and let the in-loop model decide. The corpus rule the
//! model is being held to lives in `skills/phases/generation/mobile-ui.md`
//! (MOBILE BOTTOM BREATHING ROOM).
//! The shared cleanup contract repairs a flush no-nav screen to 28px while
//! preserving every authored business node. The diagnostic uses the same
//! resolved-geometry predicate, so cleanup and echo cannot disagree.
//!
//! No name heuristics: the bottom-nav exception is decided from the authored
//! `role` semantic plus resolved geometry (full-width trailing band, nav-band
@ -18,6 +16,7 @@
//! the model happened to call the frame.
use super::*;
use op_editor_core::PenNodeExt;
/// Widest root that still reads as a phone artboard — same threshold the
/// bottom-nav containment echo uses.
@ -29,6 +28,10 @@ const MOBILE_ROOT_MIN_HEIGHT: f64 = 500.0;
/// under the corpus minimum (24px) so the echo states a fact rather than
/// nagging about a slightly tight but clearly intentional inset.
const FLUSH_BOTTOM_GAP: f64 = 12.0;
/// Corpus-compliant minimum before cleanup leaves an authored gap alone.
const MIN_BOTTOM_GAP: f64 = 24.0;
/// Stable midpoint of the documented 24-32px breathing-room range.
const TARGET_BOTTOM_GAP: f64 = 28.0;
/// Height band a bottom navigation bar occupies. The corpus asks for 62-72px;
/// the band is widened at both ends so a nav that came out slightly short or
/// tall still earns its exception instead of drawing a false echo.
@ -50,39 +53,10 @@ pub(super) fn push_mobile_bottom_gap_diagnostic(
if out.len() >= MAX_DIAGNOSTICS {
return;
}
let Some(root_rect) = resolved(root, rects) else {
let Some(gap) = resolved_mobile_bottom_gap(root, rects) else {
return;
};
if root_rect.w > MOBILE_ROOT_MAX_WIDTH || root_rect.h < MOBILE_ROOT_MIN_HEIGHT {
return;
}
// Only a flow-laid screen has a meaningful "last content edge"; an
// absolutely-positioned root stacks its children wherever it likes.
if layout_str(root) != Some("vertical") {
return;
}
let kids = children(root);
let Some(last) = kids.last() else {
return;
};
if is_bottom_nav_shape(last, &root_rect, rects) {
return;
}
// The lowest resolved edge across the root's direct children — not just
// the last one in document order, since an overlay or a taller sibling can
// be what actually reaches the bottom.
let content_bottom = kids
.iter()
.filter_map(|child| resolved(child, rects))
.map(|rect| rect.y + rect.h)
.fold(f64::NEG_INFINITY, f64::max);
if !content_bottom.is_finite() {
return;
}
let gap = root_rect.y + root_rect.h - content_bottom;
// A negative gap is content OVERFLOWING the root — a different fact, and
// the spill diagnostics already report it.
if !(0.0..FLUSH_BOTTOM_GAP).contains(&gap) {
if gap >= FLUSH_BOTTOM_GAP {
return;
}
out.push(format!(
@ -95,16 +69,119 @@ pub(super) fn push_mobile_bottom_gap_diagnostic(
));
}
/// Is this trailing child a bottom navigation bar?
/// Repair a no-nav mobile screen to the shared 28px bottom-room contract.
///
/// OpenPencil's post-layout reconciliation can grow an unclipped numeric root
/// to include its content plus padding. Increasing only the root's bottom
/// padding therefore grows the resolved artboard without relocating any
/// business child. Existing compliant gaps, navigation chrome, desktop roots,
/// and expression-authored padding are left unchanged.
pub(crate) fn repair_mobile_bottom_breathing(sink: &mut dyn DocSink, root_id: &str) -> bool {
let rects = resolved_rects(sink.state());
let Some(root) = op_editor_core::walkers::find_node(
sink.state().active_children(),
&NodeId::new(root_id.to_string()),
) else {
return false;
};
let Ok(value) = serde_json::to_value(root) else {
return false;
};
let Some(gap) = resolved_mobile_bottom_gap(&value, &rects) else {
return false;
};
if gap >= MIN_BOTTOM_GAP {
return false;
}
let Some(mut padding) = numeric_padding_sides(&value) else {
return false;
};
if padding[2] >= TARGET_BOTTOM_GAP {
return false;
}
padding[2] = TARGET_BOTTOM_GAP;
sink.apply(EditorCommand::PatchNodeData {
node_id: NodeId::new(root_id.to_string()),
patch_json: serde_json::json!({ "padding": padding }).to_string(),
page_id: None,
})
}
pub(crate) fn repair_mobile_bottom_breathing_for_all_roots(sink: &mut dyn DocSink) -> bool {
let root_ids: Vec<String> = sink
.state()
.active_children()
.iter()
.map(|root| root.id_str().to_string())
.collect();
let mut changed = false;
for root_id in root_ids {
changed |= repair_mobile_bottom_breathing(sink, &root_id);
}
changed
}
fn resolved_mobile_bottom_gap(root: &Value, rects: &HashMap<String, Rect>) -> Option<f64> {
let root_rect = resolved(root, rects)?;
if root_rect.w > MOBILE_ROOT_MAX_WIDTH || root_rect.h < MOBILE_ROOT_MIN_HEIGHT {
return None;
}
// Only a flow-laid screen has a meaningful "last content edge"; an
// absolutely-positioned root stacks its children wherever it likes.
if layout_str(root) != Some("vertical") {
return None;
}
let kids = children(root);
let last = kids.last()?;
if is_bottom_nav_shape(last, &root_rect, rects) {
return None;
}
// The lowest resolved edge across the root's direct children — not just
// the last one in document order, since an overlay or a taller sibling can
// be what actually reaches the bottom.
let content_bottom = kids
.iter()
.filter_map(|child| resolved(child, rects))
.map(|rect| rect.y + rect.h)
.fold(f64::NEG_INFINITY, f64::max);
if !content_bottom.is_finite() {
return None;
}
let gap = root_rect.y + root_rect.h - content_bottom;
// A negative gap is content OVERFLOWING the root — a different fact, and
// the spill diagnostics already report it.
if gap < 0.0 {
return None;
}
Some(gap)
}
/// Does this trailing root child close the screen with bottom navigation?
///
/// Two independent sufficient signals, neither of which reads `name`:
/// the authored `role` semantic the corpus mandates, or the resolved shape a
/// tab bar always has — a full-width band in the nav height range laying out
/// at least three evenly-sized tap targets in a row.
/// at least three evenly-sized tap targets in a row. A generated screen may
/// keep that bar as the last child of one final content wrapper; treat that
/// one-level shape as the same closing fact so diagnostics, cleanup, and
/// interaction backfill cannot disagree.
pub(super) fn is_bottom_nav_shape(
child: &Value,
root_rect: &Rect,
rects: &HashMap<String, Rect>,
) -> bool {
if is_bottom_nav_surface_shape(child, root_rect, rects) {
return true;
}
children(child)
.last()
.is_some_and(|nested| is_bottom_nav_surface_shape(nested, root_rect, rects))
}
fn is_bottom_nav_surface_shape(
child: &Value,
root_rect: &Rect,
rects: &HashMap<String, Rect>,
) -> bool {
if child.get("role").and_then(Value::as_str) == Some("bottom-tab-bar") {
return true;

206
crates/op-orchestrator/src/geometry_bottom_gap_tests.rs
View file

@ -7,7 +7,11 @@
//! include a trailing section that a name heuristic would have mistaken for
//! chrome.
use crate::geometry_validation::geometry_diagnostics;
use crate::geometry_validation::{
geometry_diagnostics, repair_mobile_bottom_breathing,
repair_mobile_bottom_breathing_for_all_roots,
};
use op_editor_core::PenNodeExt;
use serde_json::json;
const ECHO: &str = "mobile-bottom-flush";
@ -26,6 +30,51 @@ fn echoed(root: serde_json::Value) -> bool {
diagnostics_for(root).iter().any(|d| d.contains(ECHO))
}
fn state_for(root: serde_json::Value) -> op_editor_core::EditorState {
let doc: jian_ops_schema::PenDocument = serde_json::from_value(json!({
"version": "1.0",
"children": [root]
}))
.expect("valid document");
op_editor_core::EditorState::from_document(doc)
}
fn repair(root: serde_json::Value) -> (op_editor_core::EditorState, bool) {
let mut state = state_for(root);
let changed = {
let mut sink = crate::loop_finalize::StateDocSink { state: &mut state };
repair_mobile_bottom_breathing(&mut sink, "root")
};
(state, changed)
}
fn resolved_direct_child_gap(state: &op_editor_core::EditorState) -> f64 {
let scene = op_pen_loader::editor_state_to_active_page_layout_scene(state);
let page = scene.active_page().expect("active page");
let root = page.children.first().expect("root scene");
let root_bounds = root.aggregate_bounds();
let content_bottom = root
.children
.iter()
.map(|child| {
let bounds = child.aggregate_bounds();
f64::from(bounds.origin.y + bounds.size.y)
})
.fold(f64::NEG_INFINITY, f64::max);
f64::from(root_bounds.origin.y + root_bounds.size.y) - content_bottom
}
fn fixed_gap_screen(gap: f64) -> serde_json::Value {
json!({
"type": "frame", "id": "root", "name": "Mobile Screen",
"width": 390, "height": 844, "layout": "vertical", "gap": 0,
"children": [
{ "type": "frame", "id": "body", "name": "Body",
"width": "fill_container", "height": 844.0 - gap }
]
})
}
/// A movie-detail screen: no bottom nav, last section ends exactly at the
/// screen bottom. This is the reported failure (2026-07-28 screenshot).
fn detail_screen(trailing_bottom_padding: f64) -> serde_json::Value {
@ -194,3 +243,158 @@ fn content_overflowing_the_root_is_left_to_the_spill_diagnostics() {
});
assert!(!echoed(root), "overflow is not a flush-bottom report");
}
#[test]
fn cleanup_repairs_zero_and_eleven_pixel_gaps_to_twenty_eight() {
for initial_gap in [0.0, 11.0] {
let (state, changed) = repair(fixed_gap_screen(initial_gap));
assert!(changed, "{initial_gap}px must be repaired");
let gap = resolved_direct_child_gap(&state);
assert!(
(gap - 28.0).abs() <= 1.0,
"expected 28px after repairing {initial_gap}px, got {gap}"
);
assert!(
!geometry_diagnostics(&state)
.iter()
.any(|issue| issue.contains(ECHO)),
"repair and echo must share one fact predicate"
);
}
}
#[test]
fn cleanup_preserves_compliant_gap_navigation_desktop_and_business_nodes() {
let (compliant, changed) = repair(fixed_gap_screen(24.0));
assert!(!changed, "an existing 24px gap is already compliant");
assert!((resolved_direct_child_gap(&compliant) - 24.0).abs() <= 1.0);
let nav = json!({
"type": "frame", "id": "root", "name": "Home",
"width": 390, "height": 844, "layout": "vertical",
"children": [
{ "type": "frame", "id": "body", "width": "fill_container", "height": 772 },
{ "type": "frame", "id": "nav", "role": "bottom-tab-bar",
"width": "fill_container", "height": 72 }
]
});
let (with_nav, changed) = repair(nav);
assert!(!changed, "bottom navigation deliberately closes the screen");
assert_eq!(with_nav.active_children()[0].children().unwrap().len(), 2);
let desktop = json!({
"type": "frame", "id": "root", "name": "Dashboard",
"width": 1440, "height": 900, "layout": "vertical",
"children": [
{ "type": "frame", "id": "body", "width": "fill_container", "height": 900 }
]
});
let (desktop, changed) = repair(desktop);
assert!(!changed, "desktop roots are outside the mobile contract");
assert_eq!(desktop.active_children()[0].children().unwrap().len(), 1);
}
#[test]
fn cleanup_preserves_a_last_wrapper_closed_by_nested_bottom_navigation() {
let root = json!({
"type": "frame", "id": "root", "name": "Explore",
"width": 390, "height": 844, "layout": "vertical",
"children": [
{ "type": "frame", "id": "status", "role": "status-bar",
"width": "fill_container", "height": 62 },
{
"type": "frame", "id": "wrapper", "name": "Content Wrapper",
"width": "fill_container", "height": 782, "layout": "vertical",
"children": [
{ "type": "frame", "id": "content",
"width": "fill_container", "height": 710 },
{ "type": "frame", "id": "nav", "role": "bottom-tab-bar",
"width": "fill_container", "height": 72,
"layout": "horizontal" }
]
}
]
});
let (state, changed) = repair(root);
assert!(
!changed,
"a nested trailing bottom navigation already closes the screen"
);
let value = serde_json::to_value(&state.active_children()[0]).expect("root serializes");
assert!(
value.get("padding").is_none(),
"cleanup must not add blank space below nested navigation: {value}"
);
assert!(
!geometry_diagnostics(&state)
.iter()
.any(|issue| issue.contains(ECHO)),
"the shared diagnostic must recognize the same closing nav fact"
);
}
#[test]
fn cleanup_repairs_a_last_wrapper_without_nested_bottom_navigation() {
let root = json!({
"type": "frame", "id": "root", "name": "Article Detail",
"width": 390, "height": 844, "layout": "vertical",
"children": [{
"type": "frame", "id": "wrapper", "name": "Content Wrapper",
"width": "fill_container", "height": 844, "layout": "vertical",
"children": [{
"type": "frame", "id": "content",
"width": "fill_container", "height": 844
}]
}]
});
let (state, changed) = repair(root);
assert!(
changed,
"an ordinary trailing wrapper still needs breathing room"
);
assert!((resolved_direct_child_gap(&state) - 28.0).abs() <= 1.0);
let value = serde_json::to_value(&state.active_children()[0]).expect("root serializes");
assert_eq!(value["padding"], json!([0.0, 0.0, 28.0, 0.0]));
}
#[test]
fn all_roots_driver_repairs_every_mobile_screen() {
let doc: jian_ops_schema::PenDocument = serde_json::from_value(json!({
"version": "1.0",
"children": [
{
"type": "frame", "id": "root-a", "width": 390, "height": 844,
"layout": "vertical",
"children": [
{ "type": "frame", "id": "body-a",
"width": "fill_container", "height": 844 }
]
},
{
"type": "frame", "id": "root-b", "x": 440, "width": 390, "height": 844,
"layout": "vertical",
"children": [
{ "type": "frame", "id": "body-b",
"width": "fill_container", "height": 844 }
]
}
]
}))
.expect("valid two-screen document");
let mut state = op_editor_core::EditorState::from_document(doc);
let changed = {
let mut sink = crate::loop_finalize::StateDocSink { state: &mut state };
repair_mobile_bottom_breathing_for_all_roots(&mut sink)
};
assert!(changed);
for root in state.active_children() {
let value = serde_json::to_value(root).expect("root serializes");
assert_eq!(
value["padding"],
json!([0.0, 0.0, 28.0, 0.0]),
"every root must be visited"
);
}
}

36
crates/op-orchestrator/src/geometry_echo_spill_tests.rs
View file

@ -110,6 +110,7 @@ fn image_much_taller_than_its_parent_is_echoed_vertically() {
"children": [
{ "type": "frame", "id": "avatar", "name": "Avatar",
"width": "fill_container", "height": 42, "layout": "horizontal",
"padding": [8, 0],
"children": [
{ "type": "image", "id": "img", "name": "woman face headshot", "src": "",
"width": "fill_container", "height": 300 }
@ -130,6 +131,41 @@ fn image_much_taller_than_its_parent_is_echoed_vertically() {
);
}
/// The bottom-breathing cleanup adds numeric root padding without changing
/// business children. OpenPencil's post-layout reconciliation includes that
/// padding in the resolved root extent; it is not evidence of a tall child.
#[test]
fn numeric_root_padding_alone_is_not_echoed_as_vertical_spill() {
let doc: jian_ops_schema::PenDocument = serde_json::from_value(serde_json::json!({
"version": "1.0",
"children": [{
"type": "frame", "id": "root", "name": "Screen",
"width": 390, "height": 844, "layout": "vertical",
"padding": [0, 0, 28, 0],
"children": [
{ "type": "frame", "id": "body", "name": "Body",
"width": "fill_container", "height": 844 }
]
}]
}))
.expect("doc");
let state = op_editor_core::EditorState::from_document(doc);
let rects = resolved_rects(&state);
let resolved_root = rects.get("root").expect("root rect").h;
assert!(
resolved_root > 844.0 + VERTICAL_SPILL_SLACK,
"fixture must exercise post-layout padding growth, got {resolved_root}"
);
let issues = super::geometry_diagnostics(&state);
assert!(
!issues
.iter()
.any(|issue| issue.contains("Screen (root): declared")),
"numeric padding alone is not a vertical spill: {issues:?}"
);
}
/// `clipContent` parents are intentional croppers — no vertical-spill noise.
#[test]
fn clipping_parent_suppresses_vertical_spill_echo() {

94
crates/op-orchestrator/src/geometry_row_fixes.rs
View file

@ -203,16 +203,92 @@ pub(super) const ROW_OVERFULL_EPS: f64 = 8.0;
/// can't meaningfully absorb a deficit.
pub(super) const MIN_FLEXIFY_W: f64 = 120.0;
/// Is `v` table-shaped (≥2 horizontal rows of ≥3 cells)? Overfull TABLE rows
/// belong to the column scaler, which keeps columns aligned across rows —
/// flexifying one row's widest column would break the vertical alignment.
pub(super) fn is_table_shape(v: &Value) -> bool {
layout_str(v) != Some("horizontal")
&& children(v)
#[derive(Clone, Copy, PartialEq, Eq)]
enum TableCellWidthMode {
Fixed,
Fill,
Hug,
Other,
}
fn table_row_signature(row: &Value) -> Option<Vec<TableCellWidthMode>> {
if layout_str(row) != Some("horizontal") || children(row).len() < 3 {
return None;
}
Some(
children(row)
.iter()
.filter(|r| layout_str(r) == Some("horizontal") && children(r).len() >= 3)
.count()
>= 2
.map(|cell| {
if fixed_width(cell).is_some() {
TableCellWidthMode::Fixed
} else {
match cell.get("width").and_then(Value::as_str) {
Some("fill_container") => TableCellWidthMode::Fill,
Some("fit_content") | None => TableCellWidthMode::Hug,
Some(_) => TableCellWidthMode::Other,
}
}
})
.collect(),
)
}
/// Return the repeated, contiguous row run that gives `v` a table shape.
///
/// A data table has a header and data rows next to each other under one
/// container, with the same per-column sizing contract. Merely finding two
/// unrelated horizontal bands is not enough: a mobile content column often
/// contains a three-item top bar and a three-item bottom tab bar separated by
/// business sections. Treating those bands as table rows suppresses the normal
/// row fixers and can emit an impossible "too many columns" diagnostic.
///
/// The predicate uses only layout and width-mode facts. Names and roles are
/// deliberately irrelevant, so unnamed generated tables remain covered.
pub(super) fn table_rows(v: &Value) -> Vec<&Value> {
if layout_str(v) == Some("horizontal") {
return Vec::new();
}
let mut best = Vec::new();
let mut run = Vec::new();
let mut run_signature: Option<Vec<TableCellWidthMode>> = None;
for child in children(v) {
let Some(signature) = table_row_signature(child) else {
if run.len() > best.len() {
best = std::mem::take(&mut run);
} else {
run.clear();
}
run_signature = None;
continue;
};
if run_signature.as_ref() == Some(&signature) {
run.push(child);
} else {
if run.len() > best.len() {
best = std::mem::take(&mut run);
} else {
run.clear();
}
run.push(child);
run_signature = Some(signature);
}
}
if run.len() > best.len() {
best = run;
}
if best.len() >= 2 {
best
} else {
Vec::new()
}
}
/// Is `v` table-shaped? Overfull TABLE rows belong to the column scaler, which
/// keeps columns aligned across rows — flexifying one row's widest column would
/// break the vertical alignment.
pub(super) fn is_table_shape(v: &Value) -> bool {
!table_rows(v).is_empty()
}
/// A horizontal row whose children's RESOLVED widths + gaps sum wider than

38
crates/op-orchestrator/src/geometry_scale_ops.rs
View file

@ -11,14 +11,8 @@ pub(super) fn collect_scale_ops(
if let Some(scale) = table_overflow_scale(v, rects) {
// Apply the same scale to EVERY row's fixed cells (columns stay aligned)
// and to each row's gap.
for row in children(v) {
if layout_str(row) != Some("horizontal") {
continue;
}
for row in table_rows(v) {
let cells = children(row);
if cells.len() < 3 {
continue;
}
for cell in cells {
if let (Some(w), Some(id)) =
(fixed_width(cell), cell.get("id").and_then(Value::as_str))
@ -52,25 +46,19 @@ pub(super) fn collect_scale_ops(
}
}
/// If `v` is a table-shaped container (≥2 horizontal rows of ≥3 cells — the
/// STRUCTURE is the gate, not the name; "VIP Client List" shipped a starved
/// 6px email column because a name gate only trusted `table`-named frames)
/// whose fixed columns crowd out the rows' RESOLVED inner width, return the
/// scale factor (< 1.0) to apply to its fixed columns + gap. Each row is
/// measured against its own inner width (rect minus padding) and each
/// text-bearing flex column reserves a readable floor; the WORST row decides,
/// so uneven header/data column sets can't hide the deficit. `None` when the
/// shape isn't a table or everything fits.
/// If `v` is a table-shaped container (a repeated contiguous run of ≥2
/// horizontal rows with ≥3 cells and matching width modes — the STRUCTURE is
/// the gate, not the name; "VIP Client List" shipped a starved 6px email column
/// because a name gate only trusted `table`-named frames) whose fixed columns
/// crowd out the rows' RESOLVED inner width, return the scale factor (< 1.0) to
/// apply to its fixed columns + gap. Each row is measured against its own inner
/// width (rect minus padding) and each text-bearing flex column reserves a
/// readable floor; the WORST row decides, so uneven header/data column sets
/// can't hide the deficit. `None` when the shape isn't a table or everything
/// fits.
pub(super) fn table_overflow_scale(v: &Value, rects: &HashMap<String, Rect>) -> Option<f64> {
if layout_str(v) == Some("horizontal") {
return None;
}
let rows: Vec<&Value> = children(v)
.iter()
.filter(|r| layout_str(r) == Some("horizontal") && children(r).len() >= 3)
.collect();
// Need at least a header + one data row to be a real table.
if rows.len() < 2 {
let rows = table_rows(v);
if rows.is_empty() {
return None;
}
let mut worst: Option<f64> = None;

9
crates/op-orchestrator/src/geometry_spill_diagnostics.rs
View file

@ -33,7 +33,14 @@ pub(super) fn collect_vertical_spill_diagnostics(
else {
return;
};
if resolved <= declared + VERTICAL_SPILL_SLACK {
// Taffy treats a numeric size as the border box, but OpenPencil's
// post-layout repair reconciles an open container to its children's
// measured extent plus authored padding. A generated container whose
// children already consume the declared height can therefore resolve
// taller by exactly its numeric vertical padding. That is breathing room,
// not an oversized child. Expression padding is deliberately not guessed.
let padding_allowance = numeric_vertical_padding(v).unwrap_or(0.0);
if resolved <= declared + padding_allowance + VERTICAL_SPILL_SLACK {
return;
}
let culprit = children(v)

61
crates/op-orchestrator/src/geometry_table_scale_tests.rs
View file

@ -100,6 +100,67 @@ fn single_overflowing_row_is_not_a_table() {
assert!(table_overflow_scale(&strip, &rects).is_none());
}
#[test]
fn separated_mobile_chrome_rows_are_not_a_table() {
// gallery-wander's destination screen: the content wrapper contained a
// three-item top bar and a three-item bottom tab bar with business sections
// between them. Counting horizontal children alone called this a table and
// emitted "3 columns cannot fit a 295px row".
let fixed = |id: &str| cell(id, json!(36));
let top_bar = json!({
"type": "frame", "id": "top", "layout": "horizontal", "children": [
fixed("back"),
{ "type": "text", "id": "title", "width": "fit_content", "content": "Destination Details" },
fixed("bookmark")
]
});
let nav_item = |id: &str| {
json!({ "type": "frame", "id": id, "layout": "vertical",
"width": "fill_container", "children": [] })
};
let content = json!({
"type": "frame", "id": "content", "layout": "vertical", "children": [
top_bar,
{ "type": "frame", "id": "hero", "layout": "vertical", "children": [] },
{ "type": "frame", "id": "body", "layout": "vertical", "children": [] },
{ "type": "frame", "id": "nav", "layout": "horizontal", "children": [
nav_item("trips"), nav_item("destination"), nav_item("saved")
]}
]
});
let mut rects = std::collections::HashMap::new();
for id in ["top", "nav"] {
rects.insert(
id.to_string(),
Rect {
x: 0.0,
y: 0.0,
w: 295.0,
h: 72.0,
},
);
}
assert!(!is_table_shape(&content));
assert!(table_overflow_scale(&content, &rects).is_none());
assert!(table_columns_exceed_width(&content, &rects).is_none());
}
#[test]
fn contiguous_unnamed_rows_with_matching_width_modes_remain_a_table() {
let widths = [json!(180), json!("fill_container"), json!(120), json!(96)];
let table = json!({
"type": "frame", "id": "records", "layout": "vertical", "children": [
row("header", &widths),
row("record-a", &widths),
row("record-b", &widths)
]
});
assert!(is_table_shape(&table));
assert_eq!(table_rows(&table).len(), 3);
}
#[test]
fn padded_row_with_text_fill_column_triggers_on_inner_width() {
// test0703.op's exact failure shape: 860px rows padded [12,16] (inner

13
crates/op-orchestrator/src/geometry_validation.rs
View file

@ -46,6 +46,9 @@ use text_collision::push_text_collision_diagnostics;
#[path = "geometry_bottom_gap.rs"]
mod geometry_bottom_gap;
use geometry_bottom_gap::push_mobile_bottom_gap_diagnostic;
pub(crate) use geometry_bottom_gap::{
repair_mobile_bottom_breathing, repair_mobile_bottom_breathing_for_all_roots,
};
#[path = "geometry_interaction_backfill.rs"]
mod geometry_interaction_backfill;
use geometry_interaction_backfill::push_interaction_backfill_diagnostics;
@ -286,14 +289,8 @@ pub fn fix_rail_width_collapse(sink: &mut dyn DocSink, root_id: &str) -> bool {
/// rescale; the design needs fewer columns. Returns `(columns, inner_px)`
/// of the worst row for the diagnostic.
fn table_columns_exceed_width(v: &Value, rects: &HashMap<String, Rect>) -> Option<(usize, i64)> {
if layout_str(v) == Some("horizontal") {
return None;
}
let rows: Vec<&Value> = children(v)
.iter()
.filter(|r| layout_str(r) == Some("horizontal") && children(r).len() >= 3)
.collect();
if rows.len() < 2 {
let rows = table_rows(v);
if rows.is_empty() {
return None;
}
for row in rows {

53
crates/op-orchestrator/src/geometry_value_readers.rs
View file

@ -86,6 +86,46 @@ pub(super) fn num(v: &Value, key: &str) -> f64 {
}
}
/// Parse authored numeric padding into CSS-order `[top, right, bottom, left]`.
///
/// `None` means the padding is expression-backed or malformed. Callers that
/// mutate padding must preserve that distinction instead of replacing intent
/// they cannot evaluate; diagnostic callers may conservatively treat it as no
/// known allowance.
pub(super) fn numeric_padding_sides(v: &Value) -> Option<[f64; 4]> {
match v.get("padding") {
None | Some(Value::Null) => Some([0.0; 4]),
Some(Value::Number(value)) => {
let padding = value.as_f64()?;
Some([padding; 4])
}
Some(Value::Array(values)) if values.len() == 1 => {
let padding = values[0].as_f64()?;
Some([padding; 4])
}
Some(Value::Array(values)) if values.len() == 2 => {
let vertical = values[0].as_f64()?;
let horizontal = values[1].as_f64()?;
Some([vertical, horizontal, vertical, horizontal])
}
Some(Value::Array(values)) if values.len() == 4 => Some([
values[0].as_f64()?,
values[1].as_f64()?,
values[2].as_f64()?,
values[3].as_f64()?,
]),
_ => None,
}
}
/// Known vertical padding that can legitimately participate in post-layout
/// content reconciliation. Negative values are not useful breathing room and
/// therefore cannot widen a spill tolerance.
pub(super) fn numeric_vertical_padding(v: &Value) -> Option<f64> {
let [top, _, bottom, _] = numeric_padding_sides(v)?;
Some(top.max(0.0) + bottom.max(0.0))
}
/// Sum of a frame's LEFT + RIGHT padding — the schema authors `padding` as a
/// number (all sides), `[vertical, horizontal]`, or `[top, right, bottom,
/// left]`. The overflow math must compare column widths against the row's
@ -93,14 +133,7 @@ pub(super) fn num(v: &Value, key: &str) -> f64 {
/// and ignoring that put a real table 2px on the "fits" side of the gate
/// while its flex column starved to 6px (measured: test0703.op).
pub(super) fn horizontal_padding(v: &Value) -> f64 {
match v.get("padding") {
Some(Value::Number(n)) => n.as_f64().unwrap_or(0.0) * 2.0,
Some(Value::Array(a)) => match a.len() {
1 => a[0].as_f64().unwrap_or(0.0) * 2.0,
2 => a[1].as_f64().unwrap_or(0.0) * 2.0,
4 => a[1].as_f64().unwrap_or(0.0) + a[3].as_f64().unwrap_or(0.0),
_ => 0.0,
},
_ => 0.0,
}
numeric_padding_sides(v)
.map(|[_, right, _, left]| right + left)
.unwrap_or(0.0)
}

1
crates/op-orchestrator/src/loop_finalize.rs
View file

@ -476,6 +476,7 @@ pub fn apply_loop_finalize_counted(state: &mut EditorState) -> RepairSummary {
crate::cleanup::pad_clipping_horizontal_row_for_stroke_for_all_roots(sink);
crate::cleanup::equalize_horizontal_card_heights_for_all_roots(sink);
crate::cleanup::collapse_fill_container_content_sections_for_all_roots(sink);
crate::geometry_validation::repair_mobile_bottom_breathing_for_all_roots(sink);
counter.checkpoint(&mut summary, CheckCategory::Layout);
}
if state.active_children().is_empty() {