feat(agent): detect cross-subtree text collisions in geometry diagnostics

crates/op-orchestrator/src/geometry_echo.rs

@@ -13,6 +13,7 @@
 
 use op_editor_core::{EditorState, NodeId};
 
+use super::text_collision::push_text_collision_diagnostics;
 use super::{collect_diagnostics, resolved_rects, MAX_DIAGNOSTICS};
 
 /// Geometry diagnostics for exactly the subtrees rooted at `root_ids` —
@@ -42,6 +43,7 @@ pub(crate) fn geometry_diagnostics_for_roots(
         let Ok(v) = serde_json::to_value(root) else {
             continue;
         };
+        push_text_collision_diagnostics(&v, &rects, &mut out);
         collect_diagnostics(&v, &rects, &mut out);
     }
     out.truncate(MAX_DIAGNOSTICS);

crates/op-orchestrator/src/geometry_validation.rs

@@ -34,7 +34,11 @@ use geometry_compact_status::{
     is_compact_status_badge_structure, stack_compact_status_header,
 };
 
-#[derive(Clone, Copy)]
+#[path = "text_collision.rs"]
+mod text_collision;
+use text_collision::push_text_collision_diagnostics;
+
+#[derive(Clone, Copy, Debug)]
 struct Rect {
     x: f64,
     /// Not read yet — vertical stacking-overlap detection will need it; kept
@@ -1045,6 +1049,7 @@ pub fn geometry_diagnostics(state: &EditorState) -> Vec<String> {
         if let Ok(v) = serde_json::to_value(root) {
             bottom_nav_root_containment_diagnostic(&v, &rects, &mut out);
             bottom_nav_order_diagnostic(&v, &mut out);
+            push_text_collision_diagnostics(&v, &rects, &mut out);
             collect_diagnostics(&v, &rects, &mut out);
         }
     }

crates/op-orchestrator/src/text_collision.rs

@@ -0,0 +1,187 @@
+//! Cross-subtree TEXT-vs-TEXT collision detector.
+//!
+//! `collect_sibling_jam_diagnostics` (the existing overlap/jam detector)
+//! only compares DIRECT siblings inside one flex row/column, and it
+//! deliberately steps over `layout:none` parents — a `layout:none` card
+//! stack is usually an intentional deck where the CARDS overlap on
+//! purpose (front card hides the back one). That gate is correct for
+//! boxes, but it also makes the sibling-jam detector structurally blind
+//! to the failure this module catches: a weak model authoring a
+//! `layout:none` deck routinely lands a LEAF text node from one card
+//! directly on top of a leaf text node from another card. The two text
+//! nodes are cousins, not siblings, and neither one's ancestor chain sets
+//! `layout:none` at the SAME level the jam detector inspects, so the jam
+//! pass never sees the pair at all.
+//!
+//! The contract here is deliberately flatter than the box-overlap one:
+//! unlike containers (where overlap is often the deliberate point — deck
+//! stacking, ring badges, notification dots), two rendered TEXT blocks
+//! landing on the same pixels is never design intent. So this detector
+//! applies a single fact-level rule with no name/role exceptions: real
+//! resolved overlap between two non-empty visible text leaves is always a
+//! defect. Detect-only — which text should move is an intent judgment for
+//! the model in the loop, not a fixer here.
+//!
+//! Measured root cause (0724-1-gm-2.op, "今日词卡" stacked word-card
+//! preview): the front card's Chinese meaning resolves to y 708–728 and
+//! the back card's example sentence resolves to y 705–722 (68% of the
+//! smaller block's area) because both cards anchor their text near the
+//! bottom of a shared `layout:none` container without either one
+//! accounting for the other card's text block.
+
+use std::collections::HashMap;
+
+use serde_json::Value;
+
+use super::{children, Rect};
+
+/// Both axes must show more than this many px of overlap before a pair
+/// counts as colliding — guards against sub-pixel rounding touches
+/// between adjacent (non-overlapping) flow text.
+const MIN_AXIS_OVERLAP_PX: f64 = 4.0;
+
+/// The overlapping area must cover more than this fraction of the
+/// SMALLER text block's own area — guards against a long line merely
+/// clipping the corner of a short label it doesn't meaningfully collide
+/// with.
+const MIN_OVERLAP_AREA_RATIO: f64 = 0.25;
+
+/// One detected TEXT-vs-TEXT collision. Deliberately a structured payload
+/// (id + name + resolved rect for each side, plus the measured overlap)
+/// rather than a pre-formatted string — this is `geometry_validation`'s
+/// first detector with a real category/shape, laying the ground for
+/// `loop_blocker_ledger` to eventually classify it directly instead of
+/// pattern-matching message text.
+#[derive(Debug)]
+pub(crate) struct TextCollision {
+    pub(crate) a_id: String,
+    pub(crate) a_name: String,
+    pub(crate) a_rect: Rect,
+    pub(crate) b_id: String,
+    pub(crate) b_name: String,
+    pub(crate) b_rect: Rect,
+    pub(crate) overlap_x: f64,
+    pub(crate) overlap_y: f64,
+    /// Overlap area as a fraction of the SMALLER of the two text blocks'
+    /// own areas (0.0–1.0+; values are already known > MIN_OVERLAP_AREA_RATIO).
+    pub(crate) overlap_area_ratio: f64,
+}
+
+fn is_visible_nonempty_text(v: &Value) -> bool {
+    v.get("type").and_then(Value::as_str) == Some("text")
+        && v.get("visible").and_then(Value::as_bool) != Some(false)
+        && v.get("content")
+            .and_then(Value::as_str)
+            .is_some_and(|c| !c.trim().is_empty())
+}
+
+/// Depth-first collection of every visible, non-empty text LEAF under `v`
+/// that has a positive-area resolved rect. Document order, so pairwise
+/// comparison downstream is deterministic.
+fn collect_text_leaves<'a>(
+    v: &'a Value,
+    rects: &HashMap<String, Rect>,
+    out: &mut Vec<(&'a str, &'a str, Rect)>,
+) {
+    if is_visible_nonempty_text(v) {
+        if let Some(id) = v.get("id").and_then(Value::as_str) {
+            if let Some(rect) = rects.get(id) {
+                if rect.w > 0.0 && rect.h > 0.0 {
+                    let name = v.get("name").and_then(Value::as_str).unwrap_or("text");
+                    out.push((id, name, *rect));
+                }
+            }
+        }
+    }
+    // Text is a leaf in this schema; recursing defensively costs nothing
+    // and matches the `bears_text` convention elsewhere in this module.
+    for c in children(v) {
+        collect_text_leaves(c, rects, out);
+    }
+}
+
+/// All TEXT-vs-TEXT collisions inside `root`'s subtree (root included),
+/// scoped to that one root the same way the caller scopes every other
+/// per-root geometry diagnostic.
+pub(crate) fn collect_text_collisions(
+    root: &Value,
+    rects: &HashMap<String, Rect>,
+) -> Vec<TextCollision> {
+    let mut leaves = Vec::new();
+    collect_text_leaves(root, rects, &mut leaves);
+    let mut out = Vec::new();
+    for i in 0..leaves.len() {
+        let (a_id, a_name, a) = leaves[i];
+        for &(b_id, b_name, b) in &leaves[i + 1..] {
+            let overlap_x = (a.x + a.w).min(b.x + b.w) - a.x.max(b.x);
+            let overlap_y = (a.y + a.h).min(b.y + b.h) - a.y.max(b.y);
+            if overlap_x <= MIN_AXIS_OVERLAP_PX || overlap_y <= MIN_AXIS_OVERLAP_PX {
+                continue;
+            }
+            let smaller_area = (a.w * a.h).min(b.w * b.h);
+            if smaller_area <= 0.0 {
+                continue;
+            }
+            let overlap_area_ratio = (overlap_x * overlap_y) / smaller_area;
+            if overlap_area_ratio <= MIN_OVERLAP_AREA_RATIO {
+                continue;
+            }
+            out.push(TextCollision {
+                a_id: a_id.to_string(),
+                a_name: a_name.to_string(),
+                a_rect: a,
+                b_id: b_id.to_string(),
+                b_name: b_name.to_string(),
+                b_rect: b,
+                overlap_x,
+                overlap_y,
+                overlap_area_ratio,
+            });
+        }
+    }
+    out
+}
+
+fn format_collision(c: &TextCollision) -> String {
+    format!(
+        "{} ({}, resolved at {}x{} @{},{}) and {} ({}, resolved at {}x{} @{},{}): TEXT leaves \
+         OVERLAP by {}x{}px (~{}% of the smaller block's area) — two separate text blocks are \
+         rendering on the same pixels; give each its own non-overlapping position (or \
+         hide/remove one) so neither is unreadable",
+        c.a_name,
+        c.a_id,
+        c.a_rect.w.round(),
+        c.a_rect.h.round(),
+        c.a_rect.x.round(),
+        c.a_rect.y.round(),
+        c.b_name,
+        c.b_id,
+        c.b_rect.w.round(),
+        c.b_rect.h.round(),
+        c.b_rect.x.round(),
+        c.b_rect.y.round(),
+        c.overlap_x.round(),
+        c.overlap_y.round(),
+        (c.overlap_area_ratio * 100.0).round()
+    )
+}
+
+/// Detect + format text collisions under `root`, appending to `out` and
+/// respecting the shared `MAX_DIAGNOSTICS` budget the same way every
+/// other collector in this module does.
+pub(crate) fn push_text_collision_diagnostics(
+    root: &Value,
+    rects: &HashMap<String, Rect>,
+    out: &mut Vec<String>,
+) {
+    for collision in collect_text_collisions(root, rects) {
+        if out.len() >= super::MAX_DIAGNOSTICS {
+            return;
+        }
+        out.push(format_collision(&collision));
+    }
+}
+
+#[cfg(test)]
+#[path = "text_collision_tests.rs"]
+mod tests;

crates/op-orchestrator/src/text_collision_tests.rs

@@ -0,0 +1,240 @@
+use super::*;
+use serde_json::json;
+use std::collections::HashMap;
+
+fn text_node(id: &str, name: &str, content: &str) -> serde_json::Value {
+    json!({ "type": "text", "id": id, "name": name, "content": content })
+}
+
+fn rect(x: f64, y: f64, w: f64, h: f64) -> Rect {
+    Rect { x, y, w, h }
+}
+
+// ── positive: reproduces the measured 0724-1-gm-2.op "今日词卡" shape ──
+//
+// A `layout:none` deck stacks a front card over a back card. Both anchor
+// their own text near the bottom of their own card, and because the two
+// cards are offset by only 12px (front on top, back peeking out below —
+// the deliberate "stacked deck" look), the front card's meaning line and
+// the back card's example line land on the same pixels. The two text
+// nodes are cousins (different parents, both `layout:none` descendants),
+// so `collect_sibling_jam_diagnostics` never sees this pair at all.
+
+#[test]
+fn stacked_deck_cousin_text_collision_is_detected() {
+    let deck = json!({
+        "type": "frame", "id": "deck", "name": "Stacked Card Container", "layout": "none",
+        "children": [
+            { "type": "frame", "id": "front", "name": "Front Vocabulary Card", "children": [
+                text_node("front-meaning", "Chinese Meaning", "有适应力的；能迅速恢复的")
+            ] },
+            { "type": "frame", "id": "back", "name": "Back Example Card", "children": [
+                text_node("back-example", "Example English", "She is resilient in facing hard times")
+            ] }
+        ]
+    });
+    let mut rects = HashMap::new();
+    // Mirrors the measured resolved geometry: both text blocks land in the
+    // same 300-ish-wide column, ~14-20px tall, offset by only a few px.
+    rects.insert("front-meaning".to_string(), rect(118.0, 708.0, 244.0, 20.0));
+    rects.insert("back-example".to_string(), rect(126.0, 705.0, 283.0, 17.0));
+
+    let collisions = collect_text_collisions(&deck, &rects);
+    assert_eq!(
+        collisions.len(),
+        1,
+        "exactly one colliding pair: {collisions:?}"
+    );
+    let c = &collisions[0];
+    assert_eq!(c.a_id, "front-meaning");
+    assert_eq!(c.b_id, "back-example");
+    assert!(c.overlap_x > MIN_AXIS_OVERLAP_PX && c.overlap_y > MIN_AXIS_OVERLAP_PX);
+    assert!(c.overlap_area_ratio > MIN_OVERLAP_AREA_RATIO);
+
+    let mut out = Vec::new();
+    push_text_collision_diagnostics(&deck, &rects, &mut out);
+    assert!(
+        out.iter().any(|line| line.contains("Chinese Meaning")
+            && line.contains("Example English")
+            && line.contains("OVERLAP")),
+        "diagnostic line must name both text nodes: {out:?}"
+    );
+}
+
+/// Real jian layout end-to-end (not a manually-set rects map): a
+/// `layout:none` deck of two cards, each bottom-anchoring its own text via
+/// `justifyContent: end`, with the cards offset by only 4px — the
+/// canonical "stacked deck" authoring shape. Proves the detector is
+/// actually wired into `geometry_diagnostics`, not just unit-testable in
+/// isolation.
+#[test]
+fn wired_into_geometry_diagnostics_under_real_layout() {
+    let doc: jian_ops_schema::PenDocument = serde_json::from_value(json!({
+        "version": "1.0",
+        "children": [{
+            "type": "frame", "id": "root", "name": "Screen", "width": 375, "height": 200,
+            "children": [{
+                "type": "frame", "id": "deck", "name": "Stacked Card Container", "layout": "none",
+                "width": "fill_container", "height": "fill_container",
+                "children": [
+                    { "type": "frame", "id": "front", "name": "Front Card",
+                      "x": 0, "y": 0, "width": 300, "height": 130, "layout": "vertical",
+                      "justifyContent": "end",
+                      "children": [ text_node("front-meaning", "Chinese Meaning", "有适应力的；能迅速恢复的") ] },
+                    { "type": "frame", "id": "back", "name": "Back Card",
+                      "x": 8, "y": 4, "width": 284, "height": 130, "layout": "vertical",
+                      "justifyContent": "end",
+                      "children": [ text_node("back-example", "Example English", "She is resilient in facing hard times") ] }
+                ]
+            }]
+        }]
+    }))
+    .expect("doc");
+    let state = op_editor_core::EditorState::from_document(doc);
+    let issues = crate::geometry_validation::geometry_diagnostics(&state);
+    assert!(
+        issues.iter().any(|i| i.contains("Chinese Meaning")
+            && i.contains("Example English")
+            && i.contains("OVERLAP")),
+        "real-layout deck must surface the text collision: {issues:?}"
+    );
+}
+
+// ── false-positive defenses ──
+
+/// Normal vertical stack (title above subtitle) that merely touches by a
+/// rounding-scale ~1px gap must not be reported — no real overlap.
+#[test]
+fn adjacent_flow_text_with_a_hairline_gap_is_not_reported() {
+    let col = json!({
+        "type": "frame", "id": "col", "name": "Text Column", "layout": "vertical",
+        "children": [
+            text_node("title", "Task Title", "听力训练"),
+            text_node("subtitle", "Task Subtitle", "听短文回答 5 个问题 · +20 XP")
+        ]
+    });
+    let mut rects = HashMap::new();
+    rects.insert("title".to_string(), rect(0.0, 0.0, 120.0, 20.0));
+    rects.insert("subtitle".to_string(), rect(0.0, 21.0, 200.0, 16.0));
+    let collisions = collect_text_collisions(&col, &rects);
+    assert!(
+        collisions.is_empty(),
+        "no overlap, must not report: {collisions:?}"
+    );
+}
+
+/// A ring/badge number centered on a donut (ellipse) is not a text-vs-text
+/// pair at all — the ellipse never enters the text-leaf list, so there is
+/// nothing to compare it against.
+#[test]
+fn ring_center_text_over_a_non_text_ring_is_not_reported() {
+    let stack = json!({
+        "type": "frame", "id": "ring-stack", "name": "Progress Ring Stack", "layout": "none",
+        "children": [
+            { "type": "frame", "id": "ring-center", "name": "Ring Center", "children": [
+                text_node("percent", "Percent Text", "2/3")
+            ] },
+            { "type": "ellipse", "id": "arc", "name": "Progress Arc" },
+            { "type": "ellipse", "id": "track", "name": "Track Ring" }
+        ]
+    });
+    let mut rects = HashMap::new();
+    rects.insert("percent".to_string(), rect(20.0, 20.0, 20.0, 14.0));
+    rects.insert("arc".to_string(), rect(0.0, 0.0, 60.0, 60.0));
+    rects.insert("track".to_string(), rect(0.0, 0.0, 60.0, 60.0));
+    let collisions = collect_text_collisions(&stack, &rects);
+    assert!(
+        collisions.is_empty(),
+        "ellipse is never a text leaf: {collisions:?}"
+    );
+}
+
+/// A correctly-authored opaque deck where only the FRONT card carries text
+/// (the back card is pure chrome, no text child) — nothing to collide with.
+#[test]
+fn opaque_deck_with_text_only_on_the_front_card_is_not_reported() {
+    let deck = json!({
+        "type": "frame", "id": "deck", "name": "Stacked Card Container", "layout": "none",
+        "children": [
+            { "type": "frame", "id": "front", "name": "Front Card", "children": [
+                text_node("only-text", "Word", "resilient")
+            ] },
+            { "type": "frame", "id": "back", "name": "Back Card", "children": [] }
+        ]
+    });
+    let mut rects = HashMap::new();
+    rects.insert("only-text".to_string(), rect(0.0, 0.0, 80.0, 20.0));
+    let collisions = collect_text_collisions(&deck, &rects);
+    assert!(
+        collisions.is_empty(),
+        "no second text leaf to collide with: {collisions:?}"
+    );
+}
+
+/// Ordinary horizontal flex row of short text/value pairs sitting flush
+/// side-by-side (not overlapping) must not be reported — that shape is
+/// `collect_sibling_jam_diagnostics`'s territory (and even there, flush
+/// but non-overlapping is a JAM warning, not an OVERLAP).
+#[test]
+fn normal_flex_row_text_siblings_are_not_reported() {
+    let row = json!({
+        "type": "frame", "id": "row", "name": "Price Row", "layout": "horizontal",
+        "children": [
+            text_node("price", "Price", "$29"),
+            text_node("unit", "Unit", "/mo")
+        ]
+    });
+    let mut rects = HashMap::new();
+    rects.insert("price".to_string(), rect(0.0, 0.0, 30.0, 20.0));
+    rects.insert("unit".to_string(), rect(30.0, 4.0, 20.0, 14.0));
+    let collisions = collect_text_collisions(&row, &rects);
+    assert!(
+        collisions.is_empty(),
+        "flush but non-overlapping: {collisions:?}"
+    );
+}
+
+/// Empty / whitespace-only text and explicitly hidden text never enter
+/// the leaf list, regardless of their resolved rect.
+#[test]
+fn empty_and_hidden_text_are_excluded() {
+    let group = json!({
+        "type": "frame", "id": "group", "name": "Group", "layout": "none",
+        "children": [
+            text_node("empty", "Empty", "   "),
+            { "type": "text", "id": "hidden", "name": "Hidden", "content": "resilient", "visible": false },
+            text_node("real", "Real", "resilient")
+        ]
+    });
+    let mut rects = HashMap::new();
+    // Deliberately overlapping rects — if either exclusion were missing
+    // this would false-positive against "real".
+    for id in ["empty", "hidden", "real"] {
+        rects.insert(id.to_string(), rect(0.0, 0.0, 80.0, 20.0));
+    }
+    let collisions = collect_text_collisions(&group, &rects);
+    assert!(
+        collisions.is_empty(),
+        "empty/hidden text must never participate: {collisions:?}"
+    );
+}
+
+// ── real-sample harness (manual, not part of the default suite) ──
+//
+// `OP_TEXT_COLLISION_SAMPLE=/path/to/0724-1-gm-2.op cargo test -p
+// op-orchestrator text_collision -- --ignored --nocapture`
+#[test]
+#[ignore]
+fn detects_the_measured_stacked_word_card_sample() {
+    let path = std::env::var("OP_TEXT_COLLISION_SAMPLE").expect("OP_TEXT_COLLISION_SAMPLE");
+    let text = std::fs::read_to_string(&path).expect("read sample .op");
+    let doc: jian_ops_schema::PenDocument = serde_json::from_str(&text).expect("parse .op");
+    let state = op_editor_core::EditorState::from_document(doc);
+    let issues = crate::geometry_validation::geometry_diagnostics(&state);
+    assert!(
+        issues.iter().any(|i| i.contains("Chinese Meaning")
+            && i.contains("Example English")
+            && i.contains("OVERLAP")),
+        "front card meaning vs back card example must be reported: {issues:?}"
+    );
+}