Compare commits

...
Author SHA1 Message Date
Abimael MartellandClaude Opus 4.7 57bf8b89e2 refactor: classify Table as Data vs Toc once at construction
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>
2026-04-20 14:02:47 -07:00
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
// count them toward LayoutComplexity (would also trip the
// table-page guard in column detection below).
let has_real_table = |tables: &[tables::Table]| {
tables
.iter()
.any(|t| !tables::is_table_of_contents(&t.cells))
};
let has_data_table =
|tables: &[tables::Table]| tables.iter().any(|t| t.kind == tables::TableKind::Data);
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;
break;
}
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;
break;
}
// Heuristic fallback for borderless tables
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;
break;
}
+1 -6
View File
@@ -687,12 +687,7 @@ fn detect_table_in_region(items: &[(usize, &TextItem)], mode: TableDetectionMode
item_indices.len()
);
Some(Table {
columns,
rows,
cells,
item_indices,
})
Some(Table::new(columns, rows, cells, item_indices))
}
/// 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
);
vec![Table {
columns: col_edges,
rows: row_edges_desc[..num_rows].to_vec(),
vec![Table::new(
col_edges,
row_edges_desc[..num_rows].to_vec(),
cells,
item_indices,
}]
)]
}
#[cfg(test)]
+4 -24
View File
@@ -1037,12 +1037,7 @@ fn try_build_grid(
(columns, cells)
};
GridResult::Ok(Table {
columns,
rows,
cells,
item_indices,
})
GridResult::Ok(Table::new(columns, rows, cells, item_indices))
}
/// Deduplicate nearby edge values within a tolerance, returning sorted unique edges.
@@ -1442,12 +1437,7 @@ fn detect_row_stripe_table(
content_ratio * 100.0
);
Some(Table {
columns: column_centers,
rows: row_centers,
cells,
item_indices,
})
Some(Table::new(column_centers, row_centers, cells, item_indices))
}
/// 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
);
Some(Table {
columns: column_centers,
rows: row_centers,
cells,
item_indices,
})
Some(Table::new(column_centers, row_centers, cells, item_indices))
}
/// Detect a table by merging all cluster rects into one group.
@@ -1875,12 +1860,7 @@ fn detect_merged_cluster_table(
content_ratio * 100.0
);
Some(Table {
columns: column_centers,
rows: row_centers,
cells,
item_indices,
})
Some(Table::new(column_centers, row_centers, cells, item_indices))
}
/// 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.dedup();
tables.push(Table {
columns: col_positions,
rows: row_positions,
tables.push(Table::new(
col_positions,
row_positions,
cells,
item_indices: all_item_indices,
});
all_item_indices,
));
}
tables
+21 -21
View File
@@ -1,26 +1,19 @@
//! Table-to-markdown formatting and cell cleanup.
use super::detect_heuristic::is_table_of_contents;
use super::Table;
use super::{Table, TableKind};
pub fn table_to_markdown(table: &Table) -> String {
if table.cells.is_empty() || table.cells[0].is_empty() {
return String::new();
}
// Detect TOC on the raw cells: clean_table_cells merges rows in ways
// that can make genuine data tables superficially resemble a TOC
// (short numeric cells, few columns) — but the raw detection here
// preserves the original multi-column structure and only matches the
// true TOC pattern.
//
// Tables of contents render poorly as markdown tables — emit a flat
// 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) {
// TOCs render poorly as markdown tables — emit a flat 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
// because continuation-row merging in clean_table_cells collapses
// separate TOC entries (e.g. "6.2 Contamination" + "6.2.1 SWE-bench")
// into one line where sub-entries leave column 0 empty.
if table.kind == TableKind::Toc {
return format_toc_as_list(&table.cells, &[]);
}
@@ -470,6 +463,7 @@ mod tests {
vec!["Bob".into(), "25".into()],
],
item_indices: vec![],
kind: TableKind::Data,
};
let md = table_to_markdown(&table);
assert!(md.contains("|Name|"));
@@ -485,6 +479,7 @@ mod tests {
rows: vec![500.0],
cells: vec![vec!["Only".into(), "Row".into()]],
item_indices: vec![],
kind: TableKind::Data,
};
let md = table_to_markdown(&table);
assert!(md.contains("|Only|"));
@@ -498,6 +493,7 @@ mod tests {
rows: vec![],
cells: vec![],
item_indices: vec![],
kind: TableKind::Data,
};
assert_eq!(table_to_markdown(&table), "");
}
@@ -513,6 +509,7 @@ mod tests {
vec!["(1)".into(), "Footnote text".into()],
],
item_indices: vec![],
kind: TableKind::Data,
};
let md = table_to_markdown(&table);
assert!(md.contains("(1) Footnote text"));
@@ -528,6 +525,7 @@ mod tests {
vec!["太郎".into(), "25".into()],
],
item_indices: vec![],
kind: TableKind::Data,
};
let md = table_to_markdown(&table);
assert!(md.contains("名前"));
@@ -541,6 +539,7 @@ mod tests {
rows: vec![500.0],
cells: vec![vec![]],
item_indices: vec![],
kind: TableKind::Data,
};
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
// 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.
let table = Table {
columns: vec![50.0, 80.0, 300.0],
rows: vec![500.0; 5],
cells: vec![
let table = Table::new(
vec![50.0, 80.0, 300.0],
vec![500.0; 5],
vec![
vec![
"4.3".into(),
"Case studies and targeted evaluations".into(),
@@ -572,8 +571,9 @@ mod tests {
vec!["4.4".into(), "Capability evaluations".into(), "101".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);
assert!(
!md.contains("|---|"),
+6
View File
@@ -499,6 +499,7 @@ pub(crate) fn recover_header_row(
#[cfg(test)]
mod tests {
use super::*;
use crate::tables::TableKind;
use crate::types::ItemType;
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],
cells: vec![vec!["A".into(), "B".into()], vec!["C".into(), "D".into()]],
item_indices: vec![2, 3],
kind: TableKind::Data,
};
recover_header_row(&mut table, &all_items, 9.0);
@@ -774,6 +776,7 @@ mod tests {
rows: vec![500.0],
cells: vec![vec!["A".into(), "B".into()]],
item_indices: vec![0, 1],
kind: TableKind::Data,
};
let rows_before = table.rows.len();
@@ -794,6 +797,7 @@ mod tests {
rows: vec![500.0, 480.0],
cells: vec![vec!["A".into(), "B".into()], vec!["C".into(), "D".into()]],
item_indices: vec![2, 3],
kind: TableKind::Data,
};
let rows_before = table.rows.len();
@@ -814,6 +818,7 @@ mod tests {
rows: vec![500.0],
cells: vec![vec!["A".into(), "B".into()]],
item_indices: vec![1, 2],
kind: TableKind::Data,
};
let rows_before = table.rows.len();
@@ -829,6 +834,7 @@ mod tests {
rows: vec![],
cells: vec![],
item_indices: vec![],
kind: TableKind::Data,
};
recover_header_row(&mut table, &all_items, 9.0);
+50 -12
View File
@@ -10,7 +10,8 @@ mod financial;
mod format;
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(crate) use detect_rects::cluster_rects;
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.dedup();
Some(Table {
columns: col_boundaries,
rows: row_boundaries,
Some(Table::new(
col_boundaries,
row_boundaries,
cells,
item_indices: used_indices,
})
used_indices,
))
}
/// 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
);
Some(Table {
columns: col_xs,
rows: row_ys,
cells,
item_indices,
})
Some(Table::new(col_xs, row_ys, cells, item_indices))
}
/// What kind of structure a detected `Table` represents. Classification is
/// 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.
@@ -560,6 +571,31 @@ pub struct Table {
pub cells: Vec<Vec<String>>,
/// Items that belong to this table
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)]
@@ -642,6 +678,7 @@ mod tests {
vec!["Cell 1".into(), "Cell 2".into()],
],
item_indices: vec![],
kind: TableKind::Data,
};
let md = table_to_markdown(&table);
@@ -1002,6 +1039,7 @@ mod tests {
vec!["3".into(), "5/2".into(), "Item C".into(), "300".into()],
],
item_indices: vec![],
kind: TableKind::Data,
};
let md = table_to_markdown(&table);