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

feat(orchestrator): teach the geometry detectors design intent

Browse source 
Corpus-run refinements, each anchored to a measured case:
- a JAM participant must be a text-bearing CONTAINER cell — two bare
  text siblings set tight on purpose (price + unit) are typography;
- the row-gap repair also covers a TWO-column jam (date column against
  a details stack at 0px) when the row top-packs;
- text overflow now triggers on the right EDGE, catching the combined
  overflow a width-only check misses (avatar + name pair);
- one sibling centered inside another is an intentional overlay (a
  number set on a ring), not an overlap accident;
- drop an unreachable duplicate match arm the merge left in the lint
  contrast detector.

Corpus re-audit: 51/52 clean as generated, 52/52 after finalize.
This commit is contained in:
Fini 2026-07-03 03:02:43 +08:00
parent 3bce0ea5d8
commit dd034b030b
3 changed files with 135 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
crates/op-design-lint/src/detectors/typography.rs
View file

@ -210,9 +210,6 @@ fn first_solid_color(fills: Option<&Vec<PenFill>>) -> Option<String> {
}
}
PenFill::Image(_) => continue,
// Mesh gradients / SkSL shaders have no single representative
// color for contrast checks — treated like image fills.
PenFill::MeshGradient(_) | PenFill::Shader(_) => continue,
}
}
None

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

@ -260,11 +260,11 @@ fn collect_text_overflow_fixes(
// absolutely positioned, so "wider than the parent" is not an overflow to
// repair (and `width: fill_container` means nothing there).
let flex_parent = matches!(layout_str(v), Some("vertical" | "horizontal"));
if let Some(parent_w) = v
if let Some((parent_x, parent_w)) = v
.get("id")
.and_then(Value::as_str)
.and_then(|id| rects.get(id))
.map(|r| r.w)
.map(|r| (r.x, r.w))
.filter(|_| flex_parent)
{
for c in children(v) {
@ -286,7 +286,11 @@ fn collect_text_overflow_fixes(
if fill && wrap {
continue;
}
if cr.w > parent_w + TEXT_OVERFLOW_EPS {
// Wider than the block, OR its right edge past the block's right
// edge (a sibling pushed it out — combined overflow the width-only
// check misses: a 36px avatar + a fit name inside a 116px row).
let past_right = cr.x + cr.w > parent_x + parent_w + TEXT_OVERFLOW_EPS;
if cr.w > parent_w + TEXT_OVERFLOW_EPS || past_right {
cmds.push(EditorCommand::SetNodeLayoutProp {
node_id: NodeId::new(cid.to_string()),
property: "width".to_string(),
@ -385,7 +389,16 @@ fn collect_row_gap_fixes(v: &Value, rects: &HashMap<String, Rect>, cmds: &mut Ve
)
})
.collect();
if frame_kids.len() >= 3 {
let distributes = matches!(
v.get("justifyContent").and_then(Value::as_str),
Some("space_between" | "space_around" | "space_evenly")
);
// TWO text-bearing frame columns jammed at 0px (a date column against
// a details stack) are the two-column form of the same defect — but
// only when the row top-packs (a space_between pair separates itself,
// so a jammed pair there is the ancestor chain's problem, not gap's).
let enough_cells = frame_kids.len() >= 3 || (frame_kids.len() == 2 && !distributes);
if enough_cells {
let rects_of: Vec<Option<&Rect>> = frame_kids
.iter()
.map(|c| {
@ -563,6 +576,17 @@ fn bears_text(v: &Value) -> bool {
children(v).iter().any(bears_text)
}
/// A JAM participant must be a text-bearing CONTAINER cell. Two bare `text`
/// siblings set tight on purpose ("$29"+"/mo", value+unit pairs) are
/// typography, not a data-column jam — measured false positive on a pricing
/// card's price row.
fn is_cell_like(v: &Value) -> bool {
matches!(
v.get("type").and_then(Value::as_str),
Some("frame" | "group")
) && bears_text(v)
}
/// Report adjacent siblings of a horizontal FLEX row that resolved jammed
/// (text columns touching) or overlapping. Report-only: flush layouts are
/// sometimes intentional (joined button groups), so the model — not a fixer —
@ -595,7 +619,16 @@ fn collect_sibling_jam_diagnostics(
continue;
}
let breathing = rb.x - (ra.x + ra.w);
if breathing < -SIBLING_OVERLAP_EPS {
// Intentional OVERLAY, not an accident: one sibling's center inside
// the other's box — a number set on a ring (ellipse + short text), a
// corner badge on an avatar. Layout can't express children for an
// ellipse, so models stack a sibling on purpose; don't report it.
let center_inside = |inner: &Rect, outer: &Rect| {
let (cx, cy) = (inner.x + inner.w / 2.0, inner.y + inner.h / 2.0);
cx > outer.x && cx < outer.x + outer.w && cy > outer.y && cy < outer.y + outer.h
};
let overlay = center_inside(ra, rb) || center_inside(rb, ra);
if breathing < -SIBLING_OVERLAP_EPS && !overlay {
out.push(format!(
"{} and {}: siblings OVERLAP by {}px — their combined width exceeds the row; shrink widths or wrap text so they fit side by side",
diag_label(a),
@ -604,8 +637,8 @@ fn collect_sibling_jam_diagnostics(
));
} else if breathing < SIBLING_JAM_GAP
&& ra.h.min(rb.h) <= ROW_CELL_MAX_H
&& bears_text(a)
&& bears_text(b)
&& is_cell_like(a)
&& is_cell_like(b)
{
out.push(format!(
"{} and {}: text columns touch (only {}px apart) — their contents read as one word; add a gap on the row (e.g. gap: 16)",

96
crates/op-orchestrator/src/geometry_validation_tests.rs
View file

@ -796,7 +796,10 @@ fn resolved_rect_probe() {
let v = serde_json::to_value(root).unwrap();
fn walk(v: &serde_json::Value, rects: &HashMap<String, Rect>, pat: &str, depth: usize) {
let name = v.get("name").and_then(|x| x.as_str()).unwrap_or("");
if !pat.is_empty() && name.to_lowercase().contains(pat) {
let nid = v.get("id").and_then(|x| x.as_str()).unwrap_or("");
if !pat.is_empty()
&& (name.to_lowercase().contains(pat) || nid.eq_ignore_ascii_case(pat))
{
if let Some(r) = v
.get("id")
.and_then(|x| x.as_str())
@ -988,3 +991,94 @@ fn real_layout_shrinks_rigid_fit_child_overflowing_a_narrow_card() {
pair.get("width")
);
}
#[test]
fn real_layout_wraps_text_pushed_past_the_row_edge_by_a_sibling() {
use crate::test_support::VecDocSink;
use crate::types::DocSink;
use jian_ops_schema::node::PenNode;
use op_editor_core::PenNodeExt;
// p44's verbatim shape: a 116px centered row holding [36px ellipse, fit
// text] — the text alone fits the row, but the PAIR overflows and the
// text's right edge lands past the row edge. The width-only check missed
// this; the right-edge check must wrap the text.
let root: PenNode = serde_json::from_value(json!({
"type":"frame","id":"root","name":"Step Card","width":400,"height":"fit_content","layout":"vertical","children":[
{"type":"frame","id":"row","name":"Avatar Row","layout":"horizontal","width":116,"height":"fit_content","justifyContent":"center","alignItems":"center","children":[
{"type":"ellipse","id":"av","width":36,"height":36},
{"type":"text","id":"nm","name":"Name","content":"Personalize your workspace","fontSize":14}
]}
]
}))
.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))
}
let nm = find(&v, "Name").expect("text survives");
assert_eq!(
nm.get("width").and_then(|w| w.as_str()),
Some("fill_container"),
"pair-overflowed text wrapped, got {:?}",
nm.get("width")
);
}
#[test]
fn ring_badge_overlay_is_not_reported_as_an_overlap() {
// A step-ring: ellipse + a short number stacked ON it (center inside) —
// an intentional overlay, not an overflow accident.
let row = json!({
"type":"frame","id":"row","name":"Ring","layout":"horizontal","children":[
{"type":"ellipse","id":"e","width":36,"height":36},
{"type":"text","id":"t","content":"2","fontSize":15}
]
});
let mut rects = std::collections::HashMap::new();
rects.insert(
"row".into(),
Rect {
x: 0.0,
y: 0.0,
w: 116.0,
h: 36.0,
},
);
rects.insert(
"e".into(),
Rect {
x: 40.0,
y: 0.0,
w: 36.0,
h: 36.0,
},
);
rects.insert(
"t".into(),
Rect {
x: 53.0,
y: 9.0,
w: 10.0,
h: 18.0,
},
); // centered on the ring
let mut out = Vec::new();
collect_sibling_jam_diagnostics(&row, &rects, &mut out);
assert!(out.is_empty(), "overlay must not be reported: {out:?}");
}