From 31918ff62fb8f253b69c555159124406d60dcb36 Mon Sep 17 00:00:00 2001 From: Abimael Martell <1450169+abimaelmartell@users.noreply.github.com> Date: Wed, 15 Jul 2026 11:58:14 -0700 Subject: [PATCH] fix(layout): unfuse independent column runs sharing a baseline (#163) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(layout): unfuse independent column runs sharing a baseline Two-column report pages with charts fused headings into the adjacent column's body text: the columns' ~6pt gutter is below what histogram valley detection can safely use, so the page grouped single-column and same-baseline items from both columns joined into one line ('6.2. Expectations for Re-Hiring Employees' + mid-sentence text, killing MHS and NID on the whole survey-report doc family). Three changes: - Line grouping splits same-baseline runs separated by a wide void (>3x font size, >=30pt) when the incoming run starts lowercase (mid-sentence continuation from another column) and both sides are multi-word prose. TOC page numbers, dot leaders, and table cells (numbered/capitalized) stay joined. - Column detection is blind to chart-region text (tight 2pt bounds — wider padding ate rows adjacent to charts), via a chart-aware line grouping variant wired from the markdown pipeline. - validate_and_build_columns computes its vertical span from histogram-eligible items only, so full-width captions no longer sink the overlap ratio for partial-page column regions. opendataloader-bench: overall 0.8532 -> 0.8554, MHS 0.761 -> 0.769; doc 038 +0.434, no regressions. pdf-evals: 34 snapshots change, semantic composite wash (0.5749 -> 0.5748), no per-doc mover >0.015. Co-Authored-By: Claude Fable 5 * fix(layout): review follow-ups — chart-aware band-split grouping, single chart scan per page Band-split pages now route through the chart-aware grouping too, and the band loop reuses the precomputed page_chart_map instead of re-scanning the rect list per page. Co-Authored-By: Claude Fable 5 --------- Co-authored-by: Claude Fable 5 --- src/extractor/layout.rs | 137 ++++++++++++++++++++++++++++++++++++++-- src/extractor/mod.rs | 1 + src/markdown/mod.rs | 48 ++++++++++---- 3 files changed, 169 insertions(+), 17 deletions(-) diff --git a/src/extractor/layout.rs b/src/extractor/layout.rs index a8390ab..35e098a 100644 --- a/src/extractor/layout.rs +++ b/src/extractor/layout.rs @@ -674,9 +674,24 @@ fn validate_and_build_columns( page: u32, center_assign: bool, ) -> Vec { - // Compute Y range of the page - let y_min = page_items.iter().map(|i| i.y).fold(f32::INFINITY, f32::min); - let y_max = page_items + // Compute the Y range from column-eligible items only — the same items + // the histogram counted. Spanning items (full-width captions, titles) + // are excluded from the projection, so letting them stretch the page's + // vertical extent here would sink the overlap ratio for column regions + // that legitimately occupy only part of the page (e.g. two-column text + // below a figure). + let x_span = page_items + .iter() + .map(|i| i.x + effective_width(i)) + .fold(f32::NEG_INFINITY, f32::max) + - page_items.iter().map(|i| i.x).fold(f32::INFINITY, f32::min); + let narrow: Vec<&&TextItem> = page_items + .iter() + .filter(|i| effective_width(i) <= x_span * 0.6) + .collect(); + let span_items: &[&&TextItem] = if narrow.is_empty() { &[] } else { &narrow }; + let y_min = span_items.iter().map(|i| i.y).fold(f32::INFINITY, f32::min); + let y_max = span_items .iter() .map(|i| i.y) .fold(f32::NEG_INFINITY, f32::max); @@ -722,6 +737,10 @@ fn validate_and_build_columns( (right_items.len(), left_items.len()) }; if larger < min_items || smaller < 3 { + debug!( + " valley rejected: counts smaller={} larger={}", + smaller, larger + ); continue; } @@ -735,6 +754,7 @@ fn validate_and_build_columns( &right_items }; if is_list_marker_column(smaller_items) { + debug!(" valley rejected: list-marker column"); continue; } @@ -759,6 +779,13 @@ fn validate_and_build_columns( let overlap = (overlap_max - overlap_min).max(0.0); if overlap / y_range < min_vertical_span { + debug!( + " valley rejected: overlap {:.0}/{:.0} = {:.2} < {:.2}", + overlap, + y_range, + overlap / y_range, + min_vertical_span + ); continue; } } @@ -1133,6 +1160,25 @@ pub(crate) fn group_into_lines_with_thresholds( items: Vec, page_thresholds: &HashMap, table_pages: &HashSet, +) -> Vec { + group_into_lines_with_thresholds_and_charts( + items, + page_thresholds, + table_pages, + &HashMap::new(), + ) +} + +/// Like `group_into_lines_with_thresholds`, but items inside chart regions +/// are excluded from column detection: chart text scattered across the page +/// fills the gutter in the projection histogram, so two-column pages read as +/// one column and same-baseline items from both columns fuse into one line +/// (headings absorbed into the neighboring column's body text). +pub(crate) fn group_into_lines_with_thresholds_and_charts( + items: Vec, + page_thresholds: &HashMap, + table_pages: &HashSet, + chart_regions: &HashMap>, ) -> Vec { if items.is_empty() { return Vec::new(); @@ -1159,8 +1205,35 @@ pub(crate) fn group_into_lines_with_thresholds( // Non-Canva pages use the default 0.10 threshold. let adaptive_threshold = page_thresholds.get(&page).copied().unwrap_or(0.10); - // Detect columns for this page - let columns = detect_columns(&page_items, page, table_pages.contains(&page)); + // Detect columns for this page, blind to chart text. + debug!( + "page {}: grouping chart-aware={} regions={:?}", + page, + chart_regions.contains_key(&page), + chart_regions.get(&page).map(|v| v + .iter() + .map(|&(a, b, c, d)| (a as i32, b as i32, c as i32, d as i32)) + .collect::>()) + ); + let columns = match chart_regions.get(&page).filter(|r| !r.is_empty()) { + Some(regions) => { + let col_input: Vec = page_items + .iter() + .filter(|it| { + let cx = it.x + it.width / 2.0; + // Tight bounds: this only blinds the histogram to + // chart-internal text; rows adjacent to the chart + // belong to the column layout. + !regions.iter().any(|&(x0, y0, x1, y1)| { + cx >= x0 - 2.0 && cx <= x1 + 2.0 && it.y >= y0 - 2.0 && it.y <= y1 + 2.0 + }) + }) + .cloned() + .collect(); + detect_columns(&col_input, page, table_pages.contains(&page)) + } + None => detect_columns(&page_items, page, table_pages.contains(&page)), + }; if columns.len() <= 1 { // Single column - use simple sorting @@ -1446,6 +1519,37 @@ fn group_single_column(items: Vec, adaptive_threshold: f32) -> Vec (item.font_size.max(last_item.font_size) * 3.0).max(30.0) + && item + .text + .trim() + .chars() + .next() + .is_some_and(|c| c.is_lowercase() && c.is_alphabetic()) + { + let wordy = |t: &str| { + t.split_whitespace().count() >= 3 + && t.chars().filter(|c| c.is_alphabetic()).count() >= 10 + }; + let line_text = last_line + .items + .iter() + .map(|i| i.text.trim()) + .collect::>() + .join(" "); + if wordy(&line_text) && wordy(item.text.trim()) { + return false; + } + } + } true }); @@ -1519,6 +1623,29 @@ mod tests { items } + #[test] + fn same_baseline_wide_gap_lowercase_continuation_splits() { + // Heading in the left column, mid-sentence body text from the right + // column at the same y, separated by a wide void: two lines. + let items = vec![ + make_item(1, 94.0, 242.0, "6.2. Expectations for Re-Hiring Staff"), + make_item(1, 380.0, 242.0, "they had no plans to re-hire and more"), + ]; + let lines = group_single_column(items, 0.10); + assert_eq!(lines.len(), 2, "independent column runs must not fuse"); + } + + #[test] + fn same_baseline_wide_gap_table_label_stays_joined() { + // Outline-numbered cell content to the right: table-ish, keep joined. + let items = vec![ + make_item(1, 94.0, 242.0, "2. Embracing complexity in"), + make_item(1, 380.0, 242.0, "2.1 Systems thinking and practice"), + ]; + let lines = group_single_column(items, 0.10); + assert_eq!(lines.len(), 1, "numbered table cells stay on one line"); + } + #[test] fn three_zone_layout_detected() { // Left months (x=15..330), right months (x=345..660), sidebar (x=675..800) diff --git a/src/extractor/mod.rs b/src/extractor/mod.rs index ca820df..dcf88a0 100644 --- a/src/extractor/mod.rs +++ b/src/extractor/mod.rs @@ -28,6 +28,7 @@ pub(crate) use fonts::FontStyleCache; pub(crate) use layout::detect_columns; pub use layout::group_into_lines; pub(crate) use layout::group_into_lines_with_thresholds; +pub(crate) use layout::group_into_lines_with_thresholds_and_charts; pub(crate) use layout::is_newspaper_layout; pub(crate) use layout::ColumnRegion; diff --git a/src/markdown/mod.rs b/src/markdown/mod.rs index 63b4747..33e7600 100644 --- a/src/markdown/mod.rs +++ b/src/markdown/mod.rs @@ -16,7 +16,6 @@ pub use convert::to_markdown_from_lines; use std::collections::{HashMap, HashSet}; -use crate::extractor::group_into_lines_with_thresholds; use crate::types::{PdfLine, PdfRect, TextItem}; use analysis::calculate_font_stats_from_items; @@ -724,6 +723,20 @@ pub(crate) fn to_markdown_from_items_with_rects_and_lines( .push((global_idx, item)); } + // Chart regions per page: their text must not steer column detection + // during line grouping (it fills the gutter and fuses two-column lines). + let mut page_chart_map: HashMap> = HashMap::new(); + for &page in page_groups.keys() { + let page_items_ref: Vec = page_groups[&page] + .iter() + .map(|(_, item)| (*item).clone()) + .collect(); + let regions = crate::tables::detect_chart_regions(&page_items_ref, rects, page); + if !regions.is_empty() { + page_chart_map.insert(page, regions); + } + } + let mut pages: Vec = page_groups.keys().copied().collect(); pages.sort(); let page_count = pages.last().copied().unwrap_or(0) + 1; @@ -748,9 +761,8 @@ pub(crate) fn to_markdown_from_items_with_rects_and_lines( // rects or aligned text and get gridded into phantom tables. Their // items are excluded from every table detector below and flow through // as plain text instead. - let page_rect_vec: Vec = - rects.iter().filter(|r| r.page == page).cloned().collect(); - let chart_regions = crate::tables::detect_chart_regions(&page_items, &page_rect_vec, page); + let chart_regions: Vec<(f32, f32, f32, f32)> = + page_chart_map.get(&page).cloned().unwrap_or_default(); // Pad the claim region: axis/category labels sit just outside the // bar rects (below the axis, left of the scale) and belong to the // chart as much as the bars do. @@ -1259,7 +1271,12 @@ pub(crate) fn to_markdown_from_items_with_rects_and_lines( // items from different side-by-side zones (e.g. left/right month columns // in a calendar) don't merge into the same line. let lines = if page_band_splits.is_empty() { - group_into_lines_with_thresholds(non_table_items, page_thresholds, &table_page_set) + crate::extractor::group_into_lines_with_thresholds_and_charts( + non_table_items, + page_thresholds, + &table_page_set, + &page_chart_map, + ) } else { // Separate items into band-split pages and non-split pages let mut split_page_items: HashMap> = HashMap::new(); @@ -1272,8 +1289,12 @@ pub(crate) fn to_markdown_from_items_with_rects_and_lines( } } // Process unsplit pages normally - let mut all_lines = - group_into_lines_with_thresholds(unsplit_items, page_thresholds, &table_page_set); + let mut all_lines = crate::extractor::group_into_lines_with_thresholds_and_charts( + unsplit_items, + page_thresholds, + &table_page_set, + &page_chart_map, + ); // Process each split page's bands independently, then interleave // by Y position so paired zones (e.g. left/right months) appear together. let mut split_pages: Vec = split_page_items.keys().copied().collect(); @@ -1290,11 +1311,14 @@ pub(crate) fn to_markdown_from_items_with_rects_and_lines( .cloned() .collect(); if !band_items.is_empty() { - page_lines.extend(group_into_lines_with_thresholds( - band_items, - page_thresholds, - &table_page_set, - )); + page_lines.extend( + crate::extractor::group_into_lines_with_thresholds_and_charts( + band_items, + page_thresholds, + &table_page_set, + &page_chart_map, + ), + ); } } // Sort by Y descending (top to bottom) so left and right