Skip to content

Commit 8a1aec0

Browse files
phrrngtnclaude
andcommitted
html: one document-global row counter instead of one per table
bb_html gave each <table> its own row counter starting at 0, separate from the flow-block counter. That made y ambiguous across structural boundaries: an <h2> after a table got the same y as a row inside it, so ordering a document by (y, x) interleaved the two. Measured against Chromium's rendered geometry (Playwright + TreeWalker + Range.getClientRects), that single collision caused 23 of 24 reading-order inversions. Sharing one counter: Spearman rho 0.9552 -> 0.9996 inversions 24/465 (5.2%) -> 1/465 (0.2%) row grouping 30/50 (60%) -> 30/30 (100%) Table geometry was already exact and stays so: 30/30 row grouping and 40/40 column alignment among table cells, before and after. The residual single inversion is the flex two-column block, which renders side by side and which the grid places on separate rows. Layout resolves that, not the DOM, so no static walk can capture it - recorded as an inherent limit rather than a defect. A nested table is seeded from its enclosing cell's row and does not write back to the document counter, so it cannot disturb the outer numbering. An empty table emits no <tr> and costs no rows. experiments/html_grid_diagnose.py is now a pass/fail regression gate on the counter staying global. Verified no regression elsewhere: pdf 107, xlsx 54, xls 68, docx 35, and the 470-file xls corpus gate unchanged at 0 UNEXPECTED-FAIL with 2339 formulas matched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent eed1148 commit 8a1aec0

2 files changed

Lines changed: 40 additions & 10 deletions

File tree

experiments/html_grid_diagnose.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,12 @@ def main() -> int:
7777
resets = [i for i in range(1, len(ys)) if ys[i] < ys[i - 1]]
7878
print(f" grid y sequence : {ys}")
7979
print(f" backward jumps : {len(resets)} at emission index {resets}")
80-
print(" -> the table keeps its own row counter, so a heading after the")
81-
print(" table can share a y with a row inside it. Sorting the whole")
82-
print(" document by (y, x) therefore interleaves blocks.")
80+
if resets:
81+
print(" FAIL -> a block restarts the row counter, so a heading after a")
82+
print(" table can share a y with a row inside it, and ordering")
83+
print(" the document by (y, x) interleaves the two.")
84+
else:
85+
print(" PASS -> monotonic; tables continue the document numbering.")
8386

8487
# ── what does that cost, and what would fixing it buy? ───────────
8588
# "Fixed" grid = emission order made monotonic by numbering rows globally,
@@ -96,12 +99,17 @@ def main() -> int:
9699
lambda r: (round(r["y"], 1), r["x"]))
97100
inv1, _, rho1 = inversions(pairs, lambda g, i: (fixed[id(g)], g["x"]),
98101
lambda r: (round(r["y"], 1), r["x"]))
99-
print("\n2. COST OF THE COLLIDING COUNTERS")
102+
print("\n2. COST OF ANY COLLIDING COUNTERS")
100103
print(f" as emitted : rho={rho0:.4f} inversions={inv0}/{tot} "
101104
f"({100*inv0/tot:.1f}%)")
102105
print(f" with a global y : rho={rho1:.4f} inversions={inv1}/{tot} "
103106
f"({100*inv1/tot:.1f}%)")
104-
print(f" -> the single defect accounts for {inv0-inv1} of {inv0} inversions")
107+
if inv0 == inv1:
108+
print(f" -> no gain available: numbering is already global")
109+
else:
110+
print(f" -> restarting counters accounts for {inv0-inv1} of {inv0} inversions")
111+
print(f" (the residual {inv1} is the flex side-by-side case in section 3,")
112+
print(f" which no DOM-order model can capture)")
105113

106114
# ── the inherent limit: side-by-side layout ──────────────────────
107115
print("\n3. WHAT A GRID CANNOT CAPTURE (inherent, not a bug)")
@@ -117,7 +125,8 @@ def main() -> int:
117125
print(f" grid puts them on separate rows : {gl['y'] != gr_['y']}")
118126
print(" -> flex/float layout is resolved by the layout engine, not by")
119127
print(" the DOM. No static walk can know it without doing layout.")
120-
return 0
128+
# Regression gate: the row counter must stay document-global.
129+
return 1 if resets else 0
121130

122131

123132
if __name__ == "__main__":

src/bboxes_html.cpp

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,20 +67,33 @@ static int span_attr(lxb_dom_node_t* n, const char* attr, int def) {
6767

6868
/* ── DOM walk ────────────────────────────────────────────────────── */
6969

70-
/* Per-table cursor: row starts at -1 (bumped by the first <tr>), col resets per row. */
70+
/* Per-table cursor. `row` is seeded from the enclosing document row rather than
71+
from -1, so table rows continue the document's row numbering instead of
72+
restarting; `col` resets per row. See the note on flow_line below. */
7173
struct TableCtx { int row; int col; };
7274

7375
struct WalkCtx {
7476
Page* page;
7577
uint32_t style_id;
76-
int flow_line = 0; /* reading-order line counter for flow content */
78+
/* The document row counter, shared by flow blocks AND tables.
79+
*
80+
* It used to be flow-only, with each <table> starting its own counter at 0.
81+
* That made y ambiguous: a heading after a table got the same y as a row
82+
* inside it, so ordering the document by (y, x) interleaved the two. Against
83+
* Chromium's rendered geometry that single collision produced 23 of 24
84+
* reading-order inversions; sharing one counter took Spearman rho from
85+
* 0.9552 to 0.9996. See experiments/html_grid_diagnose.py, which is the
86+
* regression test for this. */
87+
int flow_line = 0;
7788
double max_x = 0.0; /* widest extent (x + w) */
7889
double max_y = 0.0; /* tallest extent (y + h) */
7990
};
8091

8192
/* Walk children in document order. `tc != nullptr` means we are inside a <table>.
8293
Imputed integer geometry (0-based), mirroring the proven reducer:
83-
- <table>: open a fresh grid cursor and descend.
94+
- <table>: open a grid cursor seeded from the current document row, and
95+
descend; on the way out, resume flow numbering after the last
96+
row the table used.
8497
- <tr>: advance row, reset col.
8598
- <td>/<th>: emit a cell box at (x=col, y=row, w=colspan, h=rowspan).
8699
- flow block (p/li/hN/…): emit a box at (x=depth, y=line++, w=len(text), h=1).
@@ -94,8 +107,16 @@ static void walk(lxb_dom_node_t* node, int depth, WalkCtx* ctx, TableCtx* tc) {
94107
name_is(n, "noscript") || name_is(n, "template")) continue;
95108

96109
if (name_is(n, "table")) {
97-
TableCtx t{-1, 0};
110+
/* Seed from the enclosing row so the table continues the document's
111+
numbering. The first <tr> bumps this, hence the -1. A nested
112+
table sits within its cell's row and must not disturb the
113+
document counter, so only a top-level table writes back. */
114+
const int base = tc ? tc->row : ctx->flow_line - 1;
115+
TableCtx t{base, 0};
98116
walk(n, depth + 1, ctx, &t);
117+
/* Resume after the last row the table actually used. An empty table
118+
emits no <tr>, leaves t.row == base, and so costs no rows. */
119+
if (!tc) ctx->flow_line = t.row + 1;
99120

100121
} else if (tc && name_is(n, "tr")) {
101122
tc->row++;

0 commit comments

Comments
 (0)