feat(orchestrator): geometry-proven row gaps + rigid fit-child shrink
Self-loop run v1 findings, fixed at the root: - geometry row-gap repair: a horizontal row whose >=3 text-bearing frame cells ALL resolve jammed gets a gap injected — name-blind, so rows buried under any depth of unnamed wrappers are covered (the name-gated table pass also now recurses through wrapper chains as a fast path). - rigid fit_content children overflowing a narrow flex parent are retargeted to fill_container (fit never shrinks; an icon+text pair painted over its siblings inside an 80px card); the text inside then wraps via the text-overflow fixer on the next loop round. - prompt: one brand name reused verbatim across logo/footer/sample data. Offline replay of both v1 artifacts: 12 audit issues -> 0.
This commit is contained in:
parent
2dca4d44c0
commit
caf80d41ec
|
|
@ -129,6 +129,10 @@ Prefer components found in `get_editor_state` components / retrieved via `batch_
|
|||
|
||||
Every visible string in one screen MUST use the SAME language as the user's request. Do not mix CJK characters and English labels on the same screen unless the request explicitly calls for bilingual UI.
|
||||
|
||||
### One brand, everywhere
|
||||
|
||||
Invent the product/brand name ONCE and reuse it verbatim in every slot that mentions it — top-bar logo, sidebar brand, footer, copyright line, email domains in sample data. A footer naming a different shop than the logo reads as broken.
|
||||
|
||||
### Mark scaffolding for removal
|
||||
|
||||
Any content you add as temporary scaffolding (placeholder text, dummy images) MUST be marked with `placeholder: true` on the node. Remove all placeholder nodes before the design is considered finished.
|
||||
|
|
|
|||
|
|
@ -173,6 +173,7 @@ pub fn geometry_validate_and_fix(sink: &mut dyn DocSink, root_id: &str) -> usize
|
|||
collect_collapse_fixes(&v, &rects, &mut cmds);
|
||||
collect_text_overflow_fixes(&v, &rects, &mut cmds);
|
||||
collect_frame_overflow_fixes(&v, &rects, &mut cmds);
|
||||
collect_row_gap_fixes(&v, &rects, &mut cmds);
|
||||
cmds
|
||||
};
|
||||
if cmds.is_empty() {
|
||||
|
|
@ -304,13 +305,19 @@ fn collect_text_overflow_fixes(
|
|||
}
|
||||
}
|
||||
|
||||
/// A NON-TEXT child with an authored NUMERIC width that resolved wider than its
|
||||
/// flex parent (a model wrote an 800px avatar bar into a ~550px row — measured
|
||||
/// on a glm loop run) → retarget it to `fill_container` so it shares the row
|
||||
/// instead of spilling across the design. Only numeric widths are touched: a
|
||||
/// keyword-sized child that overflows is the PARENT chain's problem, and text
|
||||
/// is handled by [`collect_text_overflow_fixes`]. `clipContent` parents crop on
|
||||
/// purpose — skipped.
|
||||
/// A NON-TEXT child that resolved wider than its flex parent → retarget it to
|
||||
/// `fill_container` so it shares the row instead of spilling across the design.
|
||||
/// Two authored shapes trip this:
|
||||
/// - a NUMERIC width bigger than the parent (an 800px avatar bar in a ~550px
|
||||
/// row — measured on a loop run);
|
||||
/// - a `fit_content` container whose max-content is rigid — fit never shrinks,
|
||||
/// so an icon+text pair inside an 80px card paints over its siblings
|
||||
/// (measured: a hero's "Brewing now" chip stack). Retargeting to
|
||||
/// `fill_container` gives it `min:0` shrink; the text inside then overflows
|
||||
/// ITS block and the text-overflow fixer wraps it on the NEXT loop round —
|
||||
/// the detectors converge as a chain.
|
||||
/// Text children are handled by [`collect_text_overflow_fixes`]; `clipContent`
|
||||
/// parents crop on purpose — skipped.
|
||||
fn collect_frame_overflow_fixes(
|
||||
v: &Value,
|
||||
rects: &HashMap<String, Rect>,
|
||||
|
|
@ -328,7 +335,9 @@ fn collect_frame_overflow_fixes(
|
|||
if c.get("type").and_then(Value::as_str) == Some("text") {
|
||||
continue;
|
||||
}
|
||||
if fixed_width(c).is_none() {
|
||||
let resizable = fixed_width(c).is_some()
|
||||
|| c.get("width").and_then(Value::as_str) == Some("fit_content");
|
||||
if !resizable {
|
||||
continue;
|
||||
}
|
||||
let Some(cid) = c.get("id").and_then(Value::as_str) else {
|
||||
|
|
@ -352,6 +361,65 @@ fn collect_frame_overflow_fixes(
|
|||
}
|
||||
}
|
||||
|
||||
/// Default gap injected into a geometry-proven jammed data row.
|
||||
const ROW_GAP_FIX: f64 = 16.0;
|
||||
|
||||
/// GEOMETRY-driven column-gap repair — the name-blind big brother of
|
||||
/// `table_repair::ensure_table_column_gap`. A row qualifies when the REAL
|
||||
/// layout proves every adjacent pair of its ≥3 frame cells touches (<3px
|
||||
/// breathing) and the cells carry text — "Oct 24, 2024"+"42" reading as
|
||||
/// "202442" regardless of how many unnamed wrappers bury the table (measured:
|
||||
/// rows nested TWO wrapper levels below the table-named frame slipped past
|
||||
/// the name gate). Flush segmented controls stay safe: those are 2-3 equal
|
||||
/// small children, gated out by the ≥3-cells + row-cell height + text checks.
|
||||
fn collect_row_gap_fixes(v: &Value, rects: &HashMap<String, Rect>, cmds: &mut Vec<EditorCommand>) {
|
||||
if layout_str(v) == Some("horizontal") && num(v, "gap") <= 0.0 {
|
||||
let kids = children(v);
|
||||
let frame_kids: Vec<&Value> = kids
|
||||
.iter()
|
||||
.filter(|c| {
|
||||
matches!(
|
||||
c.get("type").and_then(Value::as_str),
|
||||
Some("frame" | "group")
|
||||
)
|
||||
})
|
||||
.collect();
|
||||
if frame_kids.len() >= 3 {
|
||||
let rects_of: Vec<Option<&Rect>> = frame_kids
|
||||
.iter()
|
||||
.map(|c| {
|
||||
c.get("id")
|
||||
.and_then(Value::as_str)
|
||||
.and_then(|id| rects.get(id))
|
||||
})
|
||||
.collect();
|
||||
let all_resolved = rects_of.iter().all(|r| r.is_some_and(|r| r.w > 0.0));
|
||||
let row_cell_like = rects_of
|
||||
.iter()
|
||||
.flatten()
|
||||
.all(|r| r.h <= ROW_CELL_MAX_H && r.h > 0.0);
|
||||
let all_jammed = all_resolved
|
||||
&& rects_of.windows(2).all(|p| {
|
||||
let (a, b) = (p[0].unwrap(), p[1].unwrap());
|
||||
(b.x - (a.x + a.w)) < SIBLING_JAM_GAP
|
||||
});
|
||||
let texty = frame_kids.iter().filter(|c| bears_text(c)).count() >= 2;
|
||||
if all_resolved && row_cell_like && all_jammed && texty {
|
||||
if let Some(id) = v.get("id").and_then(Value::as_str) {
|
||||
cmds.push(EditorCommand::SetNodeLayoutProp {
|
||||
node_id: NodeId::new(id.to_string()),
|
||||
property: "gap".to_string(),
|
||||
value: LayoutPropValue::Number(ROW_GAP_FIX),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
for c in children(v) {
|
||||
collect_row_gap_fixes(c, rects, cmds);
|
||||
}
|
||||
}
|
||||
|
||||
fn is_collapsed_fill_container(v: &Value, rects: &HashMap<String, Rect>) -> bool {
|
||||
if v.get("height").and_then(Value::as_str) != Some("fill_container") {
|
||||
return false;
|
||||
|
|
|
|||
|
|
@ -868,3 +868,123 @@ fn page_level_columns_touching_are_not_a_jam() {
|
|||
collect_sibling_jam_diagnostics(&row, &rects, &mut out);
|
||||
assert!(out.is_empty(), "page columns are not a jam: {out:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn real_layout_gap_fix_reaches_doubly_wrapped_jammed_rows() {
|
||||
use crate::test_support::VecDocSink;
|
||||
use crate::types::DocSink;
|
||||
use jian_ops_schema::node::PenNode;
|
||||
use op_editor_core::PenNodeExt;
|
||||
|
||||
// p01's verbatim shape: table-named frame > unnamed vertical > unnamed
|
||||
// vertical > gap-less 4-cell text rows. The NAME gate never sees the rows;
|
||||
// the geometry gap fixer must prove the jam from resolved rects and inject
|
||||
// a gap regardless of nesting.
|
||||
let mkrow = |id: &str| {
|
||||
json!({"type":"frame","id":id,"name":null,"layout":"horizontal","width":"fill_container","height":48,"children":[
|
||||
{"type":"frame","id":format!("{id}a"),"width":200,"height":40,"children":[{"type":"text","id":format!("{id}at"),"content":"James Wilson","fontSize":14}]},
|
||||
{"type":"frame","id":format!("{id}b"),"width":130,"height":40,"children":[{"type":"text","id":format!("{id}bt"),"content":"Oct 24, 2024","fontSize":13}]},
|
||||
{"type":"frame","id":format!("{id}c"),"width":110,"height":40,"children":[{"type":"text","id":format!("{id}ct"),"content":"42","fontSize":13}]},
|
||||
{"type":"frame","id":format!("{id}d"),"width":100,"height":40,"children":[{"type":"text","id":format!("{id}dt"),"content":"VIP","fontSize":12}]}
|
||||
]})
|
||||
};
|
||||
let root: PenNode = serde_json::from_value(json!({
|
||||
"type":"frame","id":"root","name":"Client Directory Data Table","width":800,"height":"fit_content","layout":"vertical","children":[
|
||||
{"type":"frame","id":"w1","layout":"vertical","width":"fill_container","height":"fit_content","children":[
|
||||
{"type":"frame","id":"w2","layout":"vertical","width":"fill_container","height":"fit_content","children":[
|
||||
mkrow("r1"), mkrow("r2"), mkrow("r3")
|
||||
]}
|
||||
]}
|
||||
]
|
||||
}))
|
||||
.expect("valid root");
|
||||
let mut sink = VecDocSink::new();
|
||||
sink.apply(EditorCommand::InsertSubtree {
|
||||
nodes: vec![root],
|
||||
parent_id: NodeId::NONE,
|
||||
page_id: None,
|
||||
});
|
||||
let root_id = sink.state().active_children()[0].id_str().to_string();
|
||||
assert!(geometry_validate_and_fix(&mut sink, &root_id) >= 1);
|
||||
let v = serde_json::to_value(sink.state().active_children()[0].clone()).unwrap();
|
||||
fn rows_with_gap(v: &serde_json::Value, n: &mut usize) {
|
||||
if v.get("layout").and_then(|l| l.as_str()) == Some("horizontal")
|
||||
&& v.get("gap").and_then(|g| g.as_f64()).unwrap_or(0.0) > 0.0
|
||||
{
|
||||
*n += 1;
|
||||
}
|
||||
for c in v
|
||||
.get("children")
|
||||
.and_then(|c| c.as_array())
|
||||
.into_iter()
|
||||
.flatten()
|
||||
{
|
||||
rows_with_gap(c, n);
|
||||
}
|
||||
}
|
||||
let mut n = 0;
|
||||
rows_with_gap(&v, &mut n);
|
||||
assert!(n >= 3, "all three buried rows got a gap, found {n}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn real_layout_shrinks_rigid_fit_child_overflowing_a_narrow_card() {
|
||||
use crate::test_support::VecDocSink;
|
||||
use crate::types::DocSink;
|
||||
use jian_ops_schema::node::PenNode;
|
||||
use op_editor_core::PenNodeExt;
|
||||
|
||||
// p02's verbatim shape: an 80px card whose fit_content icon+text pair is
|
||||
// rigid at max-content (~150px) and paints over siblings. The fixer must
|
||||
// retarget it to fill_container (shrinkable); the text inside then wraps
|
||||
// via the text-overflow fixer on the next loop round.
|
||||
let root: PenNode = serde_json::from_value(json!({
|
||||
"type":"frame","id":"root","name":"Hero Card","width":80,"height":"fit_content","layout":"vertical","children":[
|
||||
{"type":"frame","id":"row","layout":"horizontal","gap":8,"width":"fill_container","height":"fit_content","children":[
|
||||
{"type":"frame","id":"pair","layout":"horizontal","gap":6,"width":"fit_content","height":"fit_content","children":[
|
||||
{"type":"icon_font","id":"ic","iconFontName":"coffee","width":14,"height":14},
|
||||
{"type":"text","id":"t","content":"Ethiopian Yirgacheffe pour-over","fontSize":13}
|
||||
]}
|
||||
]}
|
||||
]
|
||||
}))
|
||||
.expect("valid root");
|
||||
let mut sink = VecDocSink::new();
|
||||
sink.apply(EditorCommand::InsertSubtree {
|
||||
nodes: vec![root],
|
||||
parent_id: NodeId::NONE,
|
||||
page_id: None,
|
||||
});
|
||||
let root_id = sink.state().active_children()[0].id_str().to_string();
|
||||
geometry_validate_and_fix(&mut sink, &root_id);
|
||||
let v = serde_json::to_value(sink.state().active_children()[0].clone()).unwrap();
|
||||
fn find<'a>(v: &'a serde_json::Value, name: &str) -> Option<&'a serde_json::Value> {
|
||||
if v.get("name").and_then(|x| x.as_str()) == Some(name) {
|
||||
return Some(v);
|
||||
}
|
||||
v.get("children")
|
||||
.and_then(|c| c.as_array())
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.find_map(|c| find(c, name))
|
||||
}
|
||||
// The pair was renamed by id remap; find the frame that HOLDS the icon.
|
||||
fn find_pair<'a>(v: &'a serde_json::Value) -> Option<&'a serde_json::Value> {
|
||||
let kids = v.get("children").and_then(|c| c.as_array())?;
|
||||
if kids
|
||||
.iter()
|
||||
.any(|c| c.get("type").and_then(|t| t.as_str()) == Some("icon_font"))
|
||||
{
|
||||
return Some(v);
|
||||
}
|
||||
kids.iter().find_map(find_pair)
|
||||
}
|
||||
let _ = find; // silence potential unused in future edits
|
||||
let pair = find_pair(&v).expect("icon+text pair survives");
|
||||
assert_eq!(
|
||||
pair.get("width").and_then(|w| w.as_str()),
|
||||
Some("fill_container"),
|
||||
"rigid fit pair retargeted to fill, got {:?}",
|
||||
pair.get("width")
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -478,15 +478,7 @@ fn ensure_gap_in_value(v: &mut Value) -> bool {
|
|||
// rows all lived in such a wrapper and rendered columns
|
||||
// touching). The table NAME gate stays on the outer node; an
|
||||
// unnamed vertical wrapper inherits it.
|
||||
if is_unnamed_vertical_wrapper(row) {
|
||||
if let Some(inner) = row.get_mut("children").and_then(Value::as_array_mut) {
|
||||
for r in inner.iter_mut() {
|
||||
changed |= give_row_gap(r);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
changed |= give_row_gap(row);
|
||||
}
|
||||
changed |= give_row_gap_through_wrappers(row);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -498,6 +490,22 @@ fn ensure_gap_in_value(v: &mut Value) -> bool {
|
|||
changed
|
||||
}
|
||||
|
||||
/// Apply [`give_row_gap`] to `node`, descending through any CHAIN of unnamed
|
||||
/// structural wrappers first (a model buries its rows arbitrarily deep in
|
||||
/// nameless verticals — measured: two levels below the table frame).
|
||||
fn give_row_gap_through_wrappers(node: &mut Value) -> bool {
|
||||
if is_unnamed_vertical_wrapper(node) {
|
||||
let mut changed = false;
|
||||
if let Some(inner) = node.get_mut("children").and_then(Value::as_array_mut) {
|
||||
for r in inner.iter_mut() {
|
||||
changed |= give_row_gap_through_wrappers(r);
|
||||
}
|
||||
}
|
||||
return changed;
|
||||
}
|
||||
give_row_gap(node)
|
||||
}
|
||||
|
||||
/// Insert the default column gap when `row` is a gap-less ≥3-column row.
|
||||
fn give_row_gap(row: &mut Value) -> bool {
|
||||
if layout_str(row) == Some("horizontal") && row_needs_gap(row) {
|
||||
|
|
@ -537,14 +545,7 @@ fn is_table_container(v: &Value) -> bool {
|
|||
.map(|k| {
|
||||
// Count rows both directly AND through one unnamed
|
||||
// structural wrapper (same tolerance as the gap pass).
|
||||
if is_unnamed_vertical_wrapper(k) {
|
||||
k.get("children")
|
||||
.and_then(Value::as_array)
|
||||
.map(|inner| inner.iter().filter(|r| is_row_like(r)).count())
|
||||
.unwrap_or(0)
|
||||
} else {
|
||||
usize::from(is_row_like(k))
|
||||
}
|
||||
count_rows_through_wrappers(k)
|
||||
})
|
||||
.sum::<usize>()
|
||||
>= 2
|
||||
|
|
@ -552,6 +553,18 @@ fn is_table_container(v: &Value) -> bool {
|
|||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// Count row-like nodes, descending through chains of unnamed wrappers.
|
||||
fn count_rows_through_wrappers(node: &Value) -> usize {
|
||||
if is_unnamed_vertical_wrapper(node) {
|
||||
return node
|
||||
.get("children")
|
||||
.and_then(Value::as_array)
|
||||
.map(|inner| inner.iter().map(count_rows_through_wrappers).sum())
|
||||
.unwrap_or(0);
|
||||
}
|
||||
usize::from(is_row_like(node))
|
||||
}
|
||||
|
||||
/// A horizontal frame with ≥2 children — the row shape the container gate
|
||||
/// counts.
|
||||
fn is_row_like(r: &Value) -> bool {
|
||||
|
|
|
|||
Loading…
Reference in a new issue