feat(orchestrator): table-repair post-pass + thread root id through cleanup

Weak models emit a table as a header row followed by FLAT row-indexed sibling
cells (R1 Client Cell, R1 Visit, …, R2 Client Cell, …) that render stacked
vertically with full-width status bars. table_repair::regroup_flat_table_rows
groups them into Table→Row→Cell with header-aligned column widths. Detection is
narrow and never guesses (adversarial review caught heuristic-header /
chunk-of-N over-firing on toolbars/feeds): it requires an explicit table header
AND every cell to carry an R{n} row index, aborting on any ragged/ambiguous run.

ReplaceSubtree allocates a fresh root id, so the structural restructures
(app_shell + table_repair) ran via a new apply_root_transform helper that
returns the current root id; run_cleanup_passes threads it into the subsequent
per-root passes instead of the stale id (which they'd otherwise no-op on).
This commit is contained in:
Fini 2026-07-02 21:21:36 +08:00
parent 46fe45a5b0
commit 7956595853
4 changed files with 568 additions and 33 deletions

View file

@ -673,44 +673,68 @@ 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);
cleanup_mobile_chrome::repair_mobile_structural_chrome(sink, root_id);
cleanup_mobile_dense::repair_dense_mobile_rows(sink, root_id);
cleanup_desktop_dashboard::repair_sparse_desktop_dashboard_rows(sink, plan, root_id);
repair_overbold_text_hierarchy(sink, root_id);
strip_decorative_filled_strokes(sink, root_id);
adjust_root_height_to_content(sink, root_id);
// FIRST: whole-root structural restructures, shared by 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. These swap the root via
// `ReplaceSubtree`, which allocates a FRESH root id — `apply_root_transform`
// returns the new id so the per-root cleanup passes below don't look up a
// stale id and silently no-op.
let mut rid = root_id.to_string();
// Flat-vertical / crammed-horizontal sidebar dashboard → [sidebar | content].
rid = apply_root_transform(sink, &rid, crate::app_shell::reshape_sidebar_to_app_shell);
// Flat table cells → Table → Row → Cell.
rid = apply_root_transform(sink, &rid, crate::table_repair::regroup_flat_table_rows);
let rid = rid.as_str();
remove_duplicate_status_bars(sink, rid);
repair_light_mobile_nav_surfaces(sink, rid);
repair_mobile_content_sections(sink, rid);
cleanup_mobile_chrome::repair_mobile_structural_chrome(sink, rid);
cleanup_mobile_dense::repair_dense_mobile_rows(sink, rid);
cleanup_desktop_dashboard::repair_sparse_desktop_dashboard_rows(sink, plan, rid);
repair_overbold_text_hierarchy(sink, rid);
strip_decorative_filled_strokes(sink, rid);
adjust_root_height_to_content(sink, rid);
}
}
/// 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;
/// Apply a whole-root transform (the serialize → mutate → deserialize round-trip
/// the structural passes use) to the page-root and commit it via `ReplaceSubtree`.
///
/// `ReplaceSubtree` allocates a FRESH id for the replaced node (see
/// `command_replace_tests`), so the root's id changes on every successful
/// transform. This returns the root's CURRENT id (re-resolved by its unchanged
/// position) so the caller threads it into the next pass — otherwise every
/// subsequent per-root cleanup pass would look up the stale id and no-op.
fn apply_root_transform(
sink: &mut dyn DocSink,
root_id: &str,
transform: fn(&mut PenNode) -> bool,
) -> String {
let Some(idx) = sink
.state()
.active_children()
.iter()
.position(|n| n.id_str() == root_id)
else {
return root_id.to_string();
};
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,
});
let mut new_root = sink.state().active_children()[idx].clone();
if !transform(&mut new_root) {
return root_id.to_string();
}
sink.apply(EditorCommand::ReplaceSubtree {
node_id: NodeId::new(root_id.to_string()),
node: Box::new(new_root),
drop_children: true,
page_id: None,
});
sink.state()
.active_children()
.get(idx)
.map(|n| n.id_str().to_string())
.unwrap_or_else(|| root_id.to_string())
}
/// Strip the REDUNDANT border off a filled, shadowed container. When a

View file

@ -57,6 +57,7 @@ pub mod run;
pub mod scaffold;
pub mod spawn_concurrent;
pub mod subagent;
pub mod table_repair;
pub mod tree_heuristics;
#[cfg(test)]

View file

@ -0,0 +1,326 @@
//! Table repair — regroup flat table cells into a Table→Row→Cell hierarchy.
//!
//! Weak models (glm-5.2 "Client Roster") emit a table as a header row followed
//! by FLAT sibling cells stacked vertically: `Table Header`, `R1 Client Cell`,
//! `R1 Visit`, `R1 Barber`, `R1 Spend`, `R1 Status Badge`, `R2 Client Cell`, …
//! Each client's cells render stacked (left-aligned, not under their columns)
//! and the full-width cells become full-width bars. This pass groups those flat
//! cells back into `Table(vertical) → Row(horizontal) → Cell` whose body cells
//! share the header's per-column widths.
//!
//! Run alongside `app_shell` in `cleanup::run_cleanup_passes`. Detection is
//! deliberately NARROW and never guesses (the adversarial design review showed
//! a heuristic header + chunk-of-N grouping over-fires on toolbars / tab bars /
//! text feeds and mis-segments when the column count drifts): it requires BOTH
//! an explicit header (role/name) AND every trailing cell to carry an explicit
//! `R{n}` / `row N` row-index name, and aborts on any ragged / conflicting /
//! ambiguous run. It is a safe backstop for the exact glm shape, not a general
//! table inferencer.
use jian_ops_schema::node::PenNode;
use serde_json::{json, Value};
/// Regroup a dashboard's flat table cells into Table→Row→Cell. Mutates the
/// page-root in place via the serialize → mutate `Value` → deserialize
/// round-trip the section passes use. Returns `true` iff it restructured at
/// least one table; no-op + `false` otherwise (the node is never dropped).
pub(crate) fn regroup_flat_table_rows(root: &mut PenNode) -> bool {
let Ok(mut v) = serde_json::to_value(&*root) else {
return false;
};
if !regroup_in_value(&mut v) {
return false;
}
match serde_json::from_value::<PenNode>(v) {
Ok(new_node) => {
*root = new_node;
true
}
Err(_) => false,
}
}
/// Post-order: recurse into children first, then try to regroup THIS node's
/// children (so a freshly-built Table subtree isn't re-descended in one pass).
fn regroup_in_value(v: &mut Value) -> bool {
let mut changed = false;
if let Some(kids) = v.get_mut("children").and_then(Value::as_array_mut) {
for c in kids.iter_mut() {
changed |= regroup_in_value(c);
}
}
changed | try_regroup_children(v)
}
// ── tolerant Value readers (mirror the app_shell idiom) ──
fn num(v: &Value, key: &str) -> Option<f64> {
let f = v.get(key)?;
f.as_f64()
.or_else(|| f.as_str().and_then(|s| s.parse::<f64>().ok()))
}
fn layout_str(v: &Value) -> Option<&str> {
v.get("layout").and_then(Value::as_str)
}
fn role_str(v: &Value) -> Option<&str> {
v.get("role").and_then(Value::as_str)
}
fn ident_text(v: &Value) -> String {
let name = v.get("name").and_then(Value::as_str).unwrap_or("");
let role = v.get("role").and_then(Value::as_str).unwrap_or("");
format!("{name} {role}").to_lowercase()
}
fn is_column_layout(v: &Value) -> bool {
!matches!(layout_str(v), Some("horizontal"))
}
/// An explicit table-header row: a horizontal frame of ≥2 column labels, tagged
/// by role or name. The heuristic "any horizontal frame of short texts" header
/// is intentionally NOT accepted — it false-positives on toolbars / tab bars.
fn is_table_header(v: &Value) -> Option<usize> {
let t = ident_text(v);
let tagged = role_str(v) == Some("table-header")
|| t.contains("table header")
|| t.contains("header row")
|| t.contains("column header")
|| t.contains("thead");
if !tagged {
return None;
}
if layout_str(v) != Some("horizontal") {
return None;
}
let n = v
.get("children")
.and_then(Value::as_array)
.map_or(0, Vec::len);
(2..=12).contains(&n).then_some(n)
}
/// A cell's explicit row index from an `R{n}` / `row N` / `row-N` name. Returns
/// `None` when the name carries no row index — which aborts the whole regroup
/// (we never guess boundaries from position alone).
fn row_index(v: &Value) -> Option<u32> {
let name = v.get("name").and_then(Value::as_str)?.trim().to_lowercase();
let rest = name
.strip_prefix('r')
.or_else(|| name.strip_prefix("row-"))
.or_else(|| name.strip_prefix("row "))
.or_else(|| name.strip_prefix("row"))?;
let digits: String = rest.chars().take_while(char::is_ascii_digit).collect();
digits.parse::<u32>().ok()
}
/// A node that can be a table cell: a text, or a frame/group whose role/name
/// reads as a cell-ish element. Crucially every candidate must ALSO carry a row
/// index (checked by the caller) — this predicate only bounds the run.
fn is_cell_like(v: &Value) -> bool {
matches!(
v.get("type").and_then(Value::as_str),
Some("text" | "frame" | "group")
)
}
/// A structural region that terminates the flat-cell run (and is preserved).
fn is_run_terminator(v: &Value) -> bool {
let t = ident_text(v);
role_str(v) == Some("section")
|| t.contains("section")
|| t.contains("pagination")
|| t.contains("footer")
|| t.contains("navbar")
|| t.contains("table-row")
}
// ── detection + grouping ──
fn try_regroup_children(parent: &mut Value) -> bool {
if !is_column_layout(parent) {
return false;
}
let Some(kids) = parent.get("children").and_then(Value::as_array) else {
return false;
};
if kids.len() < 3 {
return false;
}
// Locate the header + its column count.
let Some((h_idx, n_cols)) = kids
.iter()
.enumerate()
.find_map(|(i, c)| is_table_header(c).map(|n| (i, n)))
else {
return false;
};
// Already-grouped? A table-row sibling means this ran already.
if kids[h_idx + 1..]
.iter()
.any(|c| role_str(c) == Some("table-row"))
{
return false;
}
// Collect the contiguous flat-cell run after the header, requiring EVERY
// cell to carry an explicit row index (abort on the first that doesn't).
let header = &kids[h_idx];
let col_widths = header_column_widths(header, n_cols);
let (header_pad, header_gap) = header_metrics(header);
let mut run: Vec<(u32, &Value)> = Vec::new();
for c in &kids[h_idx + 1..] {
if is_run_terminator(c) {
break;
}
if !is_cell_like(c) {
break;
}
let Some(idx) = row_index(c) else {
// A cell with no row index → ambiguous → do not guess; abort.
return false;
};
run.push((idx, c));
}
let group = match group_rows(&run, n_cols) {
Some(g) => g,
None => return false,
};
let k = group.len();
let run_len = run.len();
// Build the Table subtree: [header, row_0, … row_{k-1}].
let parent_id = parent.get("id").and_then(Value::as_str).unwrap_or("table");
let mut table_children: Vec<Value> = Vec::with_capacity(k + 1);
table_children.push(header.clone());
for (ri, row_cells) in group.into_iter().enumerate() {
table_children.push(build_row(
parent_id,
ri,
row_cells,
&col_widths,
&header_pad,
header_gap,
));
}
let table = json!({
"type": "frame",
"id": format!("{parent_id}-table"),
"name": "Table",
"role": "table",
"width": "fill_container",
"layout": "vertical",
"children": table_children,
});
// Splice: keep children[..h_idx], drop header+run, insert the Table frame,
// keep the rest (the terminator and anything after it).
let Some(arr) = parent.get_mut("children").and_then(Value::as_array_mut) else {
return false;
};
let tail_start = h_idx + 1 + run_len;
let tail: Vec<Value> = arr.split_off(tail_start);
arr.truncate(h_idx); // drop the header + the flat cells
arr.push(table);
arr.extend(tail);
true
}
/// Group the row-indexed run into `k` rows of exactly `n_cols` each. Returns
/// `None` (abort) on any irregularity: <2 rows, a row whose size ≠ n_cols, or a
/// non-contiguous index sequence. Order is preserved as encountered.
fn group_rows<'a>(run: &[(u32, &'a Value)], n_cols: usize) -> Option<Vec<Vec<&'a Value>>> {
if run.is_empty() {
return None;
}
let mut rows: Vec<(u32, Vec<&'a Value>)> = Vec::new();
for (idx, cell) in run {
match rows.last_mut() {
Some((cur, cells)) if cur == idx => cells.push(cell),
_ => rows.push((*idx, vec![*cell])),
}
}
if rows.len() < 2 {
return None;
}
// Every row must be exactly the header width, and the indices must be a
// strictly increasing contiguous sequence (1,2,3,… or 0,1,2,…).
let first = rows[0].0;
for (offset, (idx, cells)) in rows.iter().enumerate() {
if cells.len() != n_cols {
return None;
}
if *idx != first + offset as u32 {
return None;
}
}
Some(rows.into_iter().map(|(_, cells)| cells).collect())
}
/// Per-column width from the header children: numeric width, else the x-gap to
/// the next column, else `fill_container`.
fn header_column_widths(header: &Value, n_cols: usize) -> Vec<Value> {
let kids = header.get("children").and_then(Value::as_array);
let mut out = Vec::with_capacity(n_cols);
for j in 0..n_cols {
let w = kids.and_then(|k| k.get(j)).and_then(|c| num(c, "width"));
out.push(match w {
Some(v) => json!(v),
None => json!("fill_container"),
});
}
out
}
fn header_metrics(header: &Value) -> (Value, f64) {
let pad = header
.get("padding")
.cloned()
.unwrap_or_else(|| json!([12, 16]));
let gap = num(header, "gap").unwrap_or(16.0);
(pad, gap)
}
/// Build one Row frame (role table-row → horizontal/fill/center defaults),
/// mapping each cell positionally to its column width.
fn build_row(
parent_id: &str,
ri: usize,
cells: Vec<&Value>,
col_widths: &[Value],
pad: &Value,
gap: f64,
) -> Value {
let row_cells: Vec<Value> = cells
.into_iter()
.enumerate()
.map(|(j, c)| {
let mut cell = c.clone();
if let Some(obj) = cell.as_object_mut() {
let w = col_widths
.get(j)
.cloned()
.unwrap_or(json!("fill_container"));
obj.insert("width".into(), w);
}
cell
})
.collect();
json!({
"type": "frame",
"id": format!("{parent_id}-row-{ri}"),
"name": format!("Row {}", ri + 1),
"role": "table-row",
"layout": "horizontal",
"width": "fill_container",
"alignItems": "center",
"padding": pad,
"gap": gap,
"children": row_cells,
})
}
#[cfg(test)]
#[path = "table_repair_tests.rs"]
mod tests;

View file

@ -0,0 +1,184 @@
//! Tests for the flat-table regroup pass. Positive = the glm "Client Roster"
//! flat-cell shape; negatives are the false-positives the adversarial review
//! flagged (toolbar/tab-bar header, no row index, ragged run, already grouped,
//! plain text feed).
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")
}
fn txt(id: &str, name: &str, w: i64) -> Value {
json!({"type":"text","id":id,"name":name,"width":w,"content":name})
}
fn header_5col() -> Value {
json!({"type":"frame","id":"hd","name":"Table Header","layout":"horizontal",
"width":"fill_container","padding":[12,16],"gap":16,"children":[
txt("th1","TH Client",452), txt("th2","TH Last Visit",120),
txt("th3","TH Barber",140), txt("th4","TH Spend",100), txt("th5","TH Status",80)]})
}
/// `Main Content` (vertical) holding a prefix section, the header, then 2×5 flat
/// row-indexed cells — the reported shape.
fn flat_table_root() -> PenNode {
node(json!({
"type":"frame","id":"main","name":"Main Content","width":940,"layout":"vertical",
"children":[
{"type":"frame","id":"kpi","name":"Key Metrics","layout":"horizontal","width":"fill_container","children":[]},
header_5col(),
{"type":"frame","id":"r1c","name":"R1 Client Cell","width":940,"layout":"horizontal","children":[]},
txt("r1v","R1 Visit",120), txt("r1b","R1 Barber",140), txt("r1s","R1 Spend",100),
{"type":"frame","id":"r1st","name":"R1 Status Badge","width":940,"layout":"horizontal","children":[]},
{"type":"frame","id":"r2c","name":"R2 Client Cell","width":940,"layout":"horizontal","children":[]},
txt("r2v","R2 Visit",120), txt("r2b","R2 Barber",140), txt("r2s","R2 Spend",100),
{"type":"frame","id":"r2st","name":"R2 Status Badge","width":940,"layout":"horizontal","children":[]}
]
}))
}
#[test]
fn positive_flat_cells_grouped_into_table_rows() {
let mut root = flat_table_root();
assert!(
regroup_flat_table_rows(&mut root),
"flat table must regroup"
);
let v = val(&root);
let kids = v["children"].as_array().unwrap();
// Prefix section preserved; header+10 cells collapsed into one Table frame.
assert_eq!(kids.len(), 2, "[Key Metrics, Table]");
assert_eq!(kids[0]["name"], json!("Key Metrics"));
let table = &kids[1];
assert_eq!(table["role"], json!("table"));
assert_eq!(layout_str(table), Some("vertical"));
let tkids = table["children"].as_array().unwrap();
assert_eq!(tkids.len(), 3, "[header, Row 1, Row 2]");
assert_eq!(tkids[0]["name"], json!("Table Header"));
let row1 = &tkids[1];
assert_eq!(row1["role"], json!("table-row"));
assert_eq!(layout_str(row1), Some("horizontal"));
let r1: Vec<&str> = row1["children"]
.as_array()
.unwrap()
.iter()
.map(|c| c["name"].as_str().unwrap())
.collect();
assert_eq!(
r1,
[
"R1 Client Cell",
"R1 Visit",
"R1 Barber",
"R1 Spend",
"R1 Status Badge"
]
);
// Body cells take the header column widths (fixes the full-width status bar).
let w: Vec<f64> = row1["children"]
.as_array()
.unwrap()
.iter()
.map(|c| c["width"].as_f64().unwrap())
.collect();
assert_eq!(w, [452.0, 120.0, 140.0, 100.0, 80.0]);
let row2: Vec<&str> = tkids[2]["children"]
.as_array()
.unwrap()
.iter()
.map(|c| c["name"].as_str().unwrap())
.collect();
assert_eq!(
row2,
[
"R2 Client Cell",
"R2 Visit",
"R2 Barber",
"R2 Spend",
"R2 Status Badge"
]
);
}
fn assert_untouched(mut root: PenNode, why: &str) {
let before = val(&root);
assert!(
!regroup_flat_table_rows(&mut root),
"must NOT regroup: {why}"
);
assert_eq!(val(&root), before, "unchanged: {why}");
}
#[test]
fn negative_toolbar_not_a_header() {
// A horizontal toolbar of short labels is NOT tagged table-header → ignored.
assert_untouched(
node(
json!({"type":"frame","id":"m","name":"Main","width":940,"layout":"vertical","children":[
{"type":"frame","id":"tb","name":"Toolbar","layout":"horizontal","children":[
txt("a","Filter",60), txt("b","Sort",50), txt("c","Export",60)]},
txt("x1","R1 Item",100), txt("x2","R2 Item",100), txt("x3","R3 Item",100)]}),
),
"toolbar is not a tagged table header",
);
}
#[test]
fn negative_cells_without_row_index() {
// Header present, but the cells carry no R{n} index → never guess → abort.
assert_untouched(
node(
json!({"type":"frame","id":"m","name":"Main","width":940,"layout":"vertical","children":[
header_5col(),
txt("a","Client A",452), txt("b","Oct 12",120), txt("c","Marcus",140),
txt("d","$1,240",100), {"type":"frame","id":"e","name":"Status","width":80,"children":[]}]}),
),
"cells lack explicit row index",
);
}
#[test]
fn negative_ragged_run() {
// R1 has 5 cells, R2 has only 3 → group size != header columns → abort.
assert_untouched(
node(
json!({"type":"frame","id":"m","name":"Main","width":940,"layout":"vertical","children":[
header_5col(),
txt("a","R1 Client",452), txt("b","R1 Visit",120), txt("c","R1 Barber",140),
txt("d","R1 Spend",100), txt("e","R1 Status",80),
txt("f","R2 Client",452), txt("g","R2 Visit",120), txt("h","R2 Barber",140)]}),
),
"row 2 is ragged (3 != 5 columns)",
);
}
#[test]
fn negative_already_grouped() {
assert_untouched(
node(
json!({"type":"frame","id":"m","name":"Main","width":940,"layout":"vertical","children":[
header_5col(),
{"type":"frame","id":"row1","name":"Row 1","role":"table-row","layout":"horizontal","children":[]},
{"type":"frame","id":"row2","name":"Row 2","role":"table-row","layout":"horizontal","children":[]}]}),
),
"rows already grouped (table-row role present)",
);
}
#[test]
fn negative_plain_text_feed() {
// A vertical list of texts with no table header → nothing to regroup.
assert_untouched(
node(
json!({"type":"frame","id":"m","name":"Activity Feed","width":600,"layout":"vertical","children":[
txt("a","Alice commented",400), txt("b","Bob shared a file",400),
txt("c","Carol joined",400), txt("d","Dave updated status",400)]}),
),
"plain text feed, no header",
);
}