Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/tables/detect_lines.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::collections::{HashMap, HashSet};
use crate::tables::Table;
use crate::types::{PdfLine, PdfRect, TextItem};

use super::detect_rects::{assign_items_to_grid, snap_edges};
use super::detect_rects::{assign_items_to_ruled_grid, snap_edges};

const RULE_Y_TOLERANCE: f32 = 2.0;
const RULE_JOIN_GAP: f32 = 6.0;
Expand Down Expand Up @@ -956,7 +956,8 @@ fn build_open_edge_grid_table_for_rules(
return None;
}

let (body_cells, mut item_indices) = assign_items_to_grid(items, &col_edges, &row_edges, page);
let (body_cells, mut item_indices) =
assign_items_to_ruled_grid(items, &col_edges, &row_edges, page);
let column_count = col_edges.len() - 1;
let occupied_body_rows = body_cells
.iter()
Expand Down Expand Up @@ -1772,7 +1773,8 @@ fn detect_tables_from_lines_inner(
);

// Assign items to grid
let (cells, item_indices) = assign_items_to_grid(items, &col_edges, &row_edges_desc, page);
let (cells, item_indices) =
assign_items_to_ruled_grid(items, &col_edges, &row_edges_desc, page);

// Require at least 2 non-empty rows
let non_empty_rows = cells
Expand Down
146 changes: 134 additions & 12 deletions src/tables/detect_rects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use log::debug;

use crate::types::{PdfRect, TextItem};

use super::financial::try_split_financial_item_across_rule;
use super::Table;

const DOMINANT_PAGE_BACKGROUND_MIN_REPETITIONS: usize = 8;
Expand Down Expand Up @@ -1530,7 +1531,7 @@ fn try_build_grid(
}

// Build table: assign text items to cells
let (mut cells, item_indices) = assign_items_to_grid(items, &col_edges, &row_edges, page);
let (mut cells, item_indices) = assign_items_to_ruled_grid(items, &col_edges, &row_edges, page);

@cubic-dev-ai cubic-dev-ai Bot Aug 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When columns_from_text is true, this call treats inferred text-cluster midpoints as ruled boundaries and splits qualifying two-value items across them. Use assign_items_to_grid for that branch and reserve assign_items_to_ruled_grid for rect-derived columns.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tables/detect_rects.rs, line 1534:

<comment>When `columns_from_text` is true, this call treats inferred text-cluster midpoints as ruled boundaries and splits qualifying two-value items across them. Use `assign_items_to_grid` for that branch and reserve `assign_items_to_ruled_grid` for rect-derived columns.</comment>

<file context>
@@ -1531,7 +1531,7 @@ fn try_build_grid(
 
     // Build table: assign text items to cells
-    let (mut cells, item_indices) = assign_items_to_grid(items, &col_edges, &row_edges, page);
+    let (mut cells, item_indices) = assign_items_to_ruled_grid(items, &col_edges, &row_edges, page);
 
     // Consolidate vertically-merged cells: rects spanning multiple grid rows
</file context>
Suggested change
let (mut cells, item_indices) = assign_items_to_ruled_grid(items, &col_edges, &row_edges, page);
let (mut cells, item_indices) = if columns_from_text {
assign_items_to_grid(items, &col_edges, &row_edges, page)
} else {
assign_items_to_ruled_grid(items, &col_edges, &row_edges, page)
};
Fix with cubic


// Consolidate vertically-merged cells: rects spanning multiple grid rows
// should have their text collected into the first sub-row.
Expand Down Expand Up @@ -1682,6 +1683,25 @@ pub(crate) fn assign_items_to_grid(
col_edges: &[f32],
row_edges: &[f32],
page: u32,
) -> (Vec<Vec<String>>, Vec<usize>) {
assign_items_to_grid_with_options(items, col_edges, row_edges, page, false)
}

pub(crate) fn assign_items_to_ruled_grid(
items: &[TextItem],
col_edges: &[f32],
row_edges: &[f32],
page: u32,
) -> (Vec<Vec<String>>, Vec<usize>) {
assign_items_to_grid_with_options(items, col_edges, row_edges, page, true)
}

fn assign_items_to_grid_with_options(
items: &[TextItem],
col_edges: &[f32],
row_edges: &[f32],
page: u32,
allow_financial_splits: bool,
) -> (Vec<Vec<String>>, Vec<usize>) {
let num_cols = col_edges.len() - 1;
let num_rows = row_edges.len() - 1;
Expand All @@ -1691,25 +1711,94 @@ pub(crate) fn assign_items_to_grid(
vec![vec![Vec::new(); num_cols]; num_rows];
let mut indices = Vec::new();

// A TJ array can join two table values into one item when their gap is
// word-sized but smaller than the extractor's column-gap threshold. Split
// pure numeric items only when their geometry crosses a ruled boundary;
// values contained by one cell will be joined back together below.
let mut split_items = Vec::new();
let mut split_ranges = vec![None; items.len()];
for (idx, item) in items.iter().enumerate() {
if item.page != page {
if item.page != page || item.width <= 0.0 {
continue;
}
// Use item center for assignment
let cx = item.x + item.width / 2.0;
let cy = item.y;
let right = item.x + item.width;
let crosses_rule = allow_financial_splits
&& col_edges
.get(1..col_edges.len().saturating_sub(1))
.is_some_and(|inner_edges| {
inner_edges
.iter()
.any(|&edge| item.x < edge && right > edge)
});
if !crosses_rule {
continue;
}
if let Some(sub_items) = try_split_financial_item_across_rule(item) {
let start = split_items.len();
split_items.extend(sub_items);
debug!(
"split financial item across ruled column boundary: x={:.2} width={:.2} edge_count={} sub_count={}",
item.x,
item.width,
col_edges.len(),
split_items.len() - start
);
split_ranges[idx] = Some(start..split_items.len());
}
}

// Find column: cx must be between col_edges[c] and col_edges[c+1]
let col = (0..num_cols).find(|&c| cx >= col_edges[c] - 2.0 && cx <= col_edges[c + 1] + 2.0);
for (idx, item) in items.iter().enumerate() {
if item.page != page {
continue;
}
// Find row: cy must be between row_edges[r+1] (bottom) and row_edges[r] (top)
let row = (0..num_rows).find(|&r| cy >= row_edges[r + 1] - 2.0 && cy <= row_edges[r] + 2.0);
let row = (0..num_rows)
.find(|&r| item.y >= row_edges[r + 1] - 2.0 && item.y <= row_edges[r] + 2.0);

if let (Some(c), Some(r)) = (col, row) {
cell_items[r][c].push((idx, item));
indices.push(idx);
if let Some(r) = row {
if let Some(range) = split_ranges[idx].clone() {
let candidates: Vec<&TextItem> = split_items[range].iter().collect();
let placements: Vec<Option<usize>> = candidates
.iter()
.map(|candidate| {
let cx = candidate.x + candidate.width / 2.0;
(0..num_cols)
.find(|&c| cx >= col_edges[c] - 2.0 && cx <= col_edges[c + 1] + 2.0)
})
.collect();

if placements.iter().all(Option::is_some) {
for (candidate, col) in candidates.into_iter().zip(placements) {
let Some(c) = col else { continue };
cell_items[r][c].push((idx, candidate));
}
indices.push(idx);
} else {
// A split value outside every column is a bad split. Keep
// the original item together so no value is silently removed.
let cx = item.x + item.width / 2.0;
if let Some(c) = (0..num_cols)
.find(|&c| cx >= col_edges[c] - 2.0 && cx <= col_edges[c + 1] + 2.0)
{
cell_items[r][c].push((idx, item));
indices.push(idx);
}
}
} else {
let cx = item.x + item.width / 2.0;
if let Some(c) = (0..num_cols)
.find(|&c| cx >= col_edges[c] - 2.0 && cx <= col_edges[c + 1] + 2.0)
{
cell_items[r][c].push((idx, item));
indices.push(idx);
}
}
}
}

indices.sort_unstable();
indices.dedup();

// Build cell strings: sort items within each cell by Y descending then X ascending
let mut cells: Vec<Vec<String>> = Vec::with_capacity(num_rows);
for row_items in &mut cell_items {
Expand Down Expand Up @@ -2828,7 +2917,7 @@ fn detect_row_stripe_table_from_cell_rects(
page_items.len()
);

let (mut cells, item_indices) = assign_items_to_grid(items, &col_edges, &row_edges, page);
let (mut cells, item_indices) = assign_items_to_ruled_grid(items, &col_edges, &row_edges, page);

if item_indices.is_empty() {
return None;
Expand Down Expand Up @@ -3457,6 +3546,39 @@ mod tests {
}
}

#[test]
fn consolidated_numeric_item_crossing_rule_boundary_splits_cells() {
let mut item = make_item("236,480,212 10,024,724", 367.05, 524.45, 11.0);
item.width = 115.35;

let (cells, indices) = assign_items_to_ruled_grid(
&[item],
&[300.0, 356.4, 428.35, 485.85, 600.0],
&[600.0, 500.0, 400.0],
1,
);

assert_eq!(cells[0][1], "236,480,212", "cells: {cells:?}");
assert_eq!(cells[0][2], "10,024,724", "cells: {cells:?}");
assert_eq!(indices, vec![0]);
}

#[test]
fn consolidated_numeric_item_stays_together_in_inferred_grids() {
let mut item = make_item("236,480,212 10,024,724", 367.05, 524.45, 11.0);
item.width = 115.35;

let (cells, indices) = assign_items_to_grid(
&[item],
&[300.0, 356.4, 428.35, 485.85, 600.0],
&[600.0, 500.0, 400.0],
1,
);

assert_eq!(cells[0][1], "236,480,212 10,024,724", "cells: {cells:?}");
assert_eq!(indices, vec![0]);
}

// --- is_chart_bar_cluster / detect_chart_regions ---

/// Stacked bar chart: frame + 3 columns of equal-width segments with
Expand Down
101 changes: 94 additions & 7 deletions src/tables/financial.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Financial token splitting for consolidated value items.

use crate::types::ItemType;
use crate::types::TextItem;

/// Check if a whitespace-separated token looks like a financial number.
Expand Down Expand Up @@ -77,29 +78,35 @@ pub(crate) fn tokenize_financial_values(text: &str) -> Option<Vec<String>> {
}
}

/// Try to split a consolidated financial item into individual sub-items.
/// Criteria: width > font_size × 20, no alphabetic words, tokenization yields 3+ values.
/// Creates sub-items with evenly-distributed X positions across the original item's span.
pub(crate) fn try_split_financial_item(item: &TextItem) -> Option<Vec<TextItem>> {
if item.width <= item.font_size * 20.0 {
fn try_split_financial_item_with_limits(
item: &TextItem,
width_font_multiple: f32,
min_values: usize,
center_in_slot: bool,
) -> Option<Vec<TextItem>> {
if item.font_size <= 0.0 || !matches!(item.item_type, ItemType::Text) {
return None;
}
if item.width <= item.font_size * width_font_multiple {
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
return None;
}
let text = &item.text;
if has_alphabetic_words(text) {
return None;
}
let values = tokenize_financial_values(text)?;
if values.len() < 3 {
if values.len() < min_values {
return None;
}
let n = values.len() as f32;
let spacing = item.width / n;
let sub_width = spacing * 0.9;
let mut sub_items = Vec::with_capacity(values.len());
for (i, val) in values.iter().enumerate() {
let slot_offset = if center_in_slot { spacing * 0.5 } else { 0.0 };
sub_items.push(TextItem {
text: val.clone(),
x: item.x + spacing * i as f32 + spacing * 0.5,
x: item.x + spacing * i as f32 + slot_offset,
y: item.y,
width: sub_width,
height: item.height,
Expand All @@ -116,3 +123,83 @@ pub(crate) fn try_split_financial_item(item: &TextItem) -> Option<Vec<TextItem>>
}
Some(sub_items)
}

/// Try to split a consolidated financial item into individual sub-items.
/// Criteria: width > font_size × 20, no alphabetic words, tokenization yields 3+ values.
/// Creates sub-items with evenly-distributed X positions across the original item's span.
pub(crate) fn try_split_financial_item(item: &TextItem) -> Option<Vec<TextItem>> {
try_split_financial_item_with_limits(item, 20.0, 3, true)
}

/// Split a two-value item that physically crosses a ruled column boundary.
///
/// Ruled geometry supplies stronger evidence than the heuristic-table width
/// heuristic, so a word-space join across two cells is allowed to split even
/// when it contains only two values.
pub(crate) fn try_split_financial_item_across_rule(item: &TextItem) -> Option<Vec<TextItem>> {
try_split_financial_item_with_limits(item, 8.0, 2, false)
}

#[cfg(test)]
mod tests {
use super::*;
use crate::types::ItemType;

fn numeric_pair_item() -> TextItem {
TextItem {
text: "236,480,212 10,024,724".to_string(),
x: 367.0,
y: 524.0,
width: 115.0,
height: 11.0,
font: "TestFont".to_string(),
font_size: 11.0,
page: 1,
is_bold: false,
is_italic: false,
is_underline: false,
is_strikeout: false,
item_type: ItemType::Text,
mcid: None,
}
}

#[test]
fn heuristic_splitter_keeps_two_value_items_intact() {
assert!(try_split_financial_item(&numeric_pair_item()).is_none());
}

#[test]
fn ruled_boundary_splitter_splits_two_value_items() {
let split = try_split_financial_item_across_rule(&numeric_pair_item())
.expect("a wide two-value item crossing a rule should split");
assert_eq!(split.len(), 2);
assert_eq!(split[0].text, "236,480,212");
assert_eq!(split[1].text, "10,024,724");
}

#[test]
fn ruled_boundary_splitter_rejects_form_fields() {
let mut item = numeric_pair_item();
item.font_size = 0.0;
assert!(try_split_financial_item_across_rule(&item).is_none());

item.font_size = 11.0;
item.item_type = ItemType::FormField;
assert!(try_split_financial_item_across_rule(&item).is_none());
}

#[test]
fn heuristic_and_ruled_splitters_use_their_original_slot_alignment() {
let mut heuristic_item = numeric_pair_item();
heuristic_item.text = "1 2 3".to_string();
heuristic_item.width = 230.0;
let heuristic_split = try_split_financial_item(&heuristic_item)
.expect("three wide values should use the heuristic splitter");
assert_eq!(heuristic_split[0].x, heuristic_item.x + 230.0 / 6.0);

let ruled_split = try_split_financial_item_across_rule(&numeric_pair_item())
.expect("a wide two-value item crossing a rule should split");
assert_eq!(ruled_split[0].x, numeric_pair_item().x);
}
}
Loading