refactor: classify Table as Data vs Toc once at construction (#52)

The TOC/data-table distinction was being recomputed at every consumer:
- format.rs::table_to_markdown ran is_table_of_contents to decide between
  flat-list and markdown-table rendering.
- compute_layout_complexity ran it again to filter TOCs out of
  pages_with_tables.
- detect_heuristic validations used it to decide whether to relax val 1/9.

Each caller had to remember tables can be either kind, which leaks the
TOC concept across the codebase.

Add `TableKind { Data, Toc }` and a `Table::new` constructor that classifies
once from the cells. All five detectors (heuristic, rect, line, struct,
columns) now go through `Table::new`. Consumers match on `kind` instead of
re-running classification.

Pure refactor — no behavior change. Verified: pdf-evals output is byte-for-
byte identical (0 changed snapshots).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Abimael Martell
2026-04-20 14:04:50 -07:00
committed by GitHub
co-authored by Claude Opus 4.7
parent 462a7fdfde
commit b4c34ba4b7
8 changed files with 96 additions and 80 deletions
+5 -8
View File
@@ -1786,25 +1786,22 @@ fn compute_layout_complexity(
// lists. They aren't tables in any user-facing sense, so don't // lists. They aren't tables in any user-facing sense, so don't
// count them toward LayoutComplexity (would also trip the // count them toward LayoutComplexity (would also trip the
// table-page guard in column detection below). // table-page guard in column detection below).
let has_real_table = |tables: &[tables::Table]| { let has_data_table =
tables |tables: &[tables::Table]| tables.iter().any(|t| t.kind == tables::TableKind::Data);
.iter()
.any(|t| !tables::is_table_of_contents(&t.cells))
};
let (rect_tables, _) = tables::detect_tables_from_rects(&band_items, &band_rects, page); let (rect_tables, _) = tables::detect_tables_from_rects(&band_items, &band_rects, page);
if has_real_table(&rect_tables) { if has_data_table(&rect_tables) {
found_table = true; found_table = true;
break; break;
} }
let line_tables = tables::detect_tables_from_lines(&band_items, &band_lines, page); let line_tables = tables::detect_tables_from_lines(&band_items, &band_lines, page);
if has_real_table(&line_tables) { if has_data_table(&line_tables) {
found_table = true; found_table = true;
break; break;
} }
// Heuristic fallback for borderless tables // Heuristic fallback for borderless tables
let heuristic_tables = tables::detect_tables(&band_items, base_size, false); let heuristic_tables = tables::detect_tables(&band_items, base_size, false);
if has_real_table(&heuristic_tables) { if has_data_table(&heuristic_tables) {
found_table = true; found_table = true;
break; break;
} }
+1 -6
View File
@@ -687,12 +687,7 @@ fn detect_table_in_region(items: &[(usize, &TextItem)], mode: TableDetectionMode
item_indices.len() item_indices.len()
); );
Some(Table { Some(Table::new(columns, rows, cells, item_indices))
columns,
rows,
cells,
item_indices,
})
} }
/// Check if this looks like a key-value pair layout rather than a table /// Check if this looks like a key-value pair layout rather than a table
+4 -4
View File
@@ -265,12 +265,12 @@ pub fn detect_tables_from_lines(items: &[TextItem], lines: &[PdfLine], page: u32
page, num_rows, num_cols, item_indices.len(), page_item_count, non_empty_rows, cols_with_content page, num_rows, num_cols, item_indices.len(), page_item_count, non_empty_rows, cols_with_content
); );
vec![Table { vec![Table::new(
columns: col_edges, col_edges,
rows: row_edges_desc[..num_rows].to_vec(), row_edges_desc[..num_rows].to_vec(),
cells, cells,
item_indices, item_indices,
}] )]
} }
#[cfg(test)] #[cfg(test)]
+4 -24
View File
@@ -1037,12 +1037,7 @@ fn try_build_grid(
(columns, cells) (columns, cells)
}; };
GridResult::Ok(Table { GridResult::Ok(Table::new(columns, rows, cells, item_indices))
columns,
rows,
cells,
item_indices,
})
} }
/// Deduplicate nearby edge values within a tolerance, returning sorted unique edges. /// Deduplicate nearby edge values within a tolerance, returning sorted unique edges.
@@ -1442,12 +1437,7 @@ fn detect_row_stripe_table(
content_ratio * 100.0 content_ratio * 100.0
); );
Some(Table { Some(Table::new(column_centers, row_centers, cells, item_indices))
columns: column_centers,
rows: row_centers,
cells,
item_indices,
})
} }
/// Detect a table from cell-background rects that failed grid detection. /// Detect a table from cell-background rects that failed grid detection.
@@ -1691,12 +1681,7 @@ fn detect_row_stripe_table_from_cell_rects(
non_empty_cells as f32 / total_cells * 100.0 non_empty_cells as f32 / total_cells * 100.0
); );
Some(Table { Some(Table::new(column_centers, row_centers, cells, item_indices))
columns: column_centers,
rows: row_centers,
cells,
item_indices,
})
} }
/// Detect a table by merging all cluster rects into one group. /// Detect a table by merging all cluster rects into one group.
@@ -1875,12 +1860,7 @@ fn detect_merged_cluster_table(
content_ratio * 100.0 content_ratio * 100.0
); );
Some(Table { Some(Table::new(column_centers, row_centers, cells, item_indices))
columns: column_centers,
rows: row_centers,
cells,
item_indices,
})
} }
/// Cluster text item X positions into column centers with a given minimum threshold. /// Cluster text item X positions into column centers with a given minimum threshold.
+5 -5
View File
@@ -188,12 +188,12 @@ pub fn detect_tables_from_struct_tree(
all_item_indices.sort_unstable(); all_item_indices.sort_unstable();
all_item_indices.dedup(); all_item_indices.dedup();
tables.push(Table { tables.push(Table::new(
columns: col_positions, col_positions,
rows: row_positions, row_positions,
cells, cells,
item_indices: all_item_indices, all_item_indices,
}); ));
} }
tables tables
+21 -21
View File
@@ -1,26 +1,19 @@
//! Table-to-markdown formatting and cell cleanup. //! Table-to-markdown formatting and cell cleanup.
use super::detect_heuristic::is_table_of_contents; use super::{Table, TableKind};
use super::Table;
pub fn table_to_markdown(table: &Table) -> String { pub fn table_to_markdown(table: &Table) -> String {
if table.cells.is_empty() || table.cells[0].is_empty() { if table.cells.is_empty() || table.cells[0].is_empty() {
return String::new(); return String::new();
} }
// Detect TOC on the raw cells: clean_table_cells merges rows in ways // TOCs render poorly as markdown tables — emit a flat per-row text list
// that can make genuine data tables superficially resemble a TOC // instead so the page numbers stay aligned with their section titles
// (short numeric cells, few columns) — but the raw detection here // rather than drifting to a separate column. Format from raw cells
// preserves the original multi-column structure and only matches the // because continuation-row merging in clean_table_cells collapses
// true TOC pattern. // separate TOC entries (e.g. "6.2 Contamination" + "6.2.1 SWE-bench")
// // into one line where sub-entries leave column 0 empty.
// Tables of contents render poorly as markdown tables — emit a flat if table.kind == TableKind::Toc {
// per-row text list instead so the page numbers stay aligned with
// their section titles rather than drifting to a separate column.
// Format from raw cells: continuation-row merging collapses separate
// TOC entries (e.g. "6.2 Contamination" + "6.2.1 SWE-bench") into a
// single line because sub-entries leave column 0 empty.
if is_table_of_contents(&table.cells) {
return format_toc_as_list(&table.cells, &[]); return format_toc_as_list(&table.cells, &[]);
} }
@@ -470,6 +463,7 @@ mod tests {
vec!["Bob".into(), "25".into()], vec!["Bob".into(), "25".into()],
], ],
item_indices: vec![], item_indices: vec![],
kind: TableKind::Data,
}; };
let md = table_to_markdown(&table); let md = table_to_markdown(&table);
assert!(md.contains("|Name|")); assert!(md.contains("|Name|"));
@@ -485,6 +479,7 @@ mod tests {
rows: vec![500.0], rows: vec![500.0],
cells: vec![vec!["Only".into(), "Row".into()]], cells: vec![vec!["Only".into(), "Row".into()]],
item_indices: vec![], item_indices: vec![],
kind: TableKind::Data,
}; };
let md = table_to_markdown(&table); let md = table_to_markdown(&table);
assert!(md.contains("|Only|")); assert!(md.contains("|Only|"));
@@ -498,6 +493,7 @@ mod tests {
rows: vec![], rows: vec![],
cells: vec![], cells: vec![],
item_indices: vec![], item_indices: vec![],
kind: TableKind::Data,
}; };
assert_eq!(table_to_markdown(&table), ""); assert_eq!(table_to_markdown(&table), "");
} }
@@ -513,6 +509,7 @@ mod tests {
vec!["(1)".into(), "Footnote text".into()], vec!["(1)".into(), "Footnote text".into()],
], ],
item_indices: vec![], item_indices: vec![],
kind: TableKind::Data,
}; };
let md = table_to_markdown(&table); let md = table_to_markdown(&table);
assert!(md.contains("(1) Footnote text")); assert!(md.contains("(1) Footnote text"));
@@ -528,6 +525,7 @@ mod tests {
vec!["太郎".into(), "25".into()], vec!["太郎".into(), "25".into()],
], ],
item_indices: vec![], item_indices: vec![],
kind: TableKind::Data,
}; };
let md = table_to_markdown(&table); let md = table_to_markdown(&table);
assert!(md.contains("名前")); assert!(md.contains("名前"));
@@ -541,6 +539,7 @@ mod tests {
rows: vec![500.0], rows: vec![500.0],
cells: vec![vec![]], cells: vec![vec![]],
item_indices: vec![], item_indices: vec![],
kind: TableKind::Data,
}; };
assert_eq!(table_to_markdown(&table), ""); assert_eq!(table_to_markdown(&table), "");
} }
@@ -550,10 +549,10 @@ mod tests {
// A TOC-shaped table with section numbers in col 0 and page numbers // A TOC-shaped table with section numbers in col 0 and page numbers
// in the last column should render as a flat list, not a markdown // in the last column should render as a flat list, not a markdown
// table, so the page numbers stay on the same line as their titles. // table, so the page numbers stay on the same line as their titles.
let table = Table { let table = Table::new(
columns: vec![50.0, 80.0, 300.0], vec![50.0, 80.0, 300.0],
rows: vec![500.0; 5], vec![500.0; 5],
cells: vec![ vec![
vec![ vec![
"4.3".into(), "4.3".into(),
"Case studies and targeted evaluations".into(), "Case studies and targeted evaluations".into(),
@@ -572,8 +571,9 @@ mod tests {
vec!["4.4".into(), "Capability evaluations".into(), "101".into()], vec!["4.4".into(), "Capability evaluations".into(), "101".into()],
vec!["4.5".into(), "White-box analyses".into(), "113".into()], vec!["4.5".into(), "White-box analyses".into(), "113".into()],
], ],
item_indices: vec![], vec![],
}; );
assert_eq!(table.kind, TableKind::Toc);
let md = table_to_markdown(&table); let md = table_to_markdown(&table);
assert!( assert!(
!md.contains("|---|"), !md.contains("|---|"),
+6
View File
@@ -499,6 +499,7 @@ pub(crate) fn recover_header_row(
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use crate::tables::TableKind;
use crate::types::ItemType; use crate::types::ItemType;
fn make_item(text: &str, x: f32, y: f32, font_size: f32) -> TextItem { fn make_item(text: &str, x: f32, y: f32, font_size: f32) -> TextItem {
@@ -756,6 +757,7 @@ mod tests {
rows: vec![500.0, 480.0], rows: vec![500.0, 480.0],
cells: vec![vec!["A".into(), "B".into()], vec!["C".into(), "D".into()]], cells: vec![vec!["A".into(), "B".into()], vec!["C".into(), "D".into()]],
item_indices: vec![2, 3], item_indices: vec![2, 3],
kind: TableKind::Data,
}; };
recover_header_row(&mut table, &all_items, 9.0); recover_header_row(&mut table, &all_items, 9.0);
@@ -774,6 +776,7 @@ mod tests {
rows: vec![500.0], rows: vec![500.0],
cells: vec![vec!["A".into(), "B".into()]], cells: vec![vec!["A".into(), "B".into()]],
item_indices: vec![0, 1], item_indices: vec![0, 1],
kind: TableKind::Data,
}; };
let rows_before = table.rows.len(); let rows_before = table.rows.len();
@@ -794,6 +797,7 @@ mod tests {
rows: vec![500.0, 480.0], rows: vec![500.0, 480.0],
cells: vec![vec!["A".into(), "B".into()], vec!["C".into(), "D".into()]], cells: vec![vec!["A".into(), "B".into()], vec!["C".into(), "D".into()]],
item_indices: vec![2, 3], item_indices: vec![2, 3],
kind: TableKind::Data,
}; };
let rows_before = table.rows.len(); let rows_before = table.rows.len();
@@ -814,6 +818,7 @@ mod tests {
rows: vec![500.0], rows: vec![500.0],
cells: vec![vec!["A".into(), "B".into()]], cells: vec![vec!["A".into(), "B".into()]],
item_indices: vec![1, 2], item_indices: vec![1, 2],
kind: TableKind::Data,
}; };
let rows_before = table.rows.len(); let rows_before = table.rows.len();
@@ -829,6 +834,7 @@ mod tests {
rows: vec![], rows: vec![],
cells: vec![], cells: vec![],
item_indices: vec![], item_indices: vec![],
kind: TableKind::Data,
}; };
recover_header_row(&mut table, &all_items, 9.0); recover_header_row(&mut table, &all_items, 9.0);
+50 -12
View File
@@ -10,7 +10,8 @@ mod financial;
mod format; mod format;
mod grid; mod grid;
pub use detect_heuristic::{detect_tables, is_table_of_contents}; pub use detect_heuristic::detect_tables;
pub(crate) use detect_heuristic::is_table_of_contents;
pub use detect_lines::detect_tables_from_lines; pub use detect_lines::detect_tables_from_lines;
pub(crate) use detect_rects::cluster_rects; pub(crate) use detect_rects::cluster_rects;
pub use detect_rects::{detect_tables_from_rects, RectHintRegion}; pub use detect_rects::{detect_tables_from_rects, RectHintRegion};
@@ -166,12 +167,12 @@ pub(crate) fn try_build_rect_guided_table(
used_indices.sort_unstable(); used_indices.sort_unstable();
used_indices.dedup(); used_indices.dedup();
Some(Table { Some(Table::new(
columns: col_boundaries, col_boundaries,
rows: row_boundaries, row_boundaries,
cells, cells,
item_indices: used_indices, used_indices,
}) ))
} }
/// Split a TextItem whose text contains multiple whitespace-separated tokens /// Split a TextItem whose text contains multiple whitespace-separated tokens
@@ -541,12 +542,22 @@ pub(crate) fn try_build_table_from_columns(items: &[TextItem], page: u32) -> Opt
multi_col_rows multi_col_rows
); );
Some(Table { Some(Table::new(col_xs, row_ys, cells, item_indices))
columns: col_xs, }
rows: row_ys,
cells, /// What kind of structure a detected `Table` represents. Classification is
item_indices, /// computed once at construction so consumers don't have to re-analyze the
}) /// cells (and stay consistent across detection backends).
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
pub enum TableKind {
/// A real data table — renders as markdown table syntax.
#[default]
Data,
/// A table of contents — renders as a flat list with tab-aligned page
/// numbers via `format_toc_as_list`. Detected through the table pipeline
/// because TOCs share row/column structure with tables, but they are not
/// data tables and shouldn't appear in `pages_with_tables` etc.
Toc,
} }
/// A detected table. /// A detected table.
@@ -560,6 +571,31 @@ pub struct Table {
pub cells: Vec<Vec<String>>, pub cells: Vec<Vec<String>>,
/// Items that belong to this table /// Items that belong to this table
pub item_indices: Vec<usize>, pub item_indices: Vec<usize>,
/// Data table vs TOC. Set by `Table::new` from `cells`.
pub kind: TableKind,
}
impl Table {
/// Build a table and classify it (data vs TOC) from its cells.
pub fn new(
columns: Vec<f32>,
rows: Vec<f32>,
cells: Vec<Vec<String>>,
item_indices: Vec<usize>,
) -> Self {
let kind = if is_table_of_contents(&cells) {
TableKind::Toc
} else {
TableKind::Data
};
Self {
columns,
rows,
cells,
item_indices,
kind,
}
}
} }
#[cfg(test)] #[cfg(test)]
@@ -642,6 +678,7 @@ mod tests {
vec!["Cell 1".into(), "Cell 2".into()], vec!["Cell 1".into(), "Cell 2".into()],
], ],
item_indices: vec![], item_indices: vec![],
kind: TableKind::Data,
}; };
let md = table_to_markdown(&table); let md = table_to_markdown(&table);
@@ -1002,6 +1039,7 @@ mod tests {
vec!["3".into(), "5/2".into(), "Item C".into(), "300".into()], vec!["3".into(), "5/2".into(), "Item C".into(), "300".into()],
], ],
item_indices: vec![], item_indices: vec![],
kind: TableKind::Data,
}; };
let md = table_to_markdown(&table); let md = table_to_markdown(&table);