mu-ref/pdf-inspector
8
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: replace partial_cmp with total_cmp to prevent NaN sort panics (#20)

Browse Source 
* fix sort panics on NaN values from bogus PDF font metrics

Replace all `partial_cmp(...).unwrap_or(Ordering::Equal)` and bare
`partial_cmp(...).unwrap()` with `total_cmp()` across the codebase.

`partial_cmp` returns `None` for NaN, and mapping that to `Equal`
violates total ordering: `a == NaN` and `NaN == b` but `a != b`.
Rust 1.81+ detects this and panics in sort_by. `total_cmp` handles
NaN deterministically (sorts to end) and guarantees total ordering.

The critical crash was in `extract_text_in_regions` (lib.rs:478)
where PDFs with bogus font ascent/descent values produced NaN in
text item coordinates, causing process abort via NAPI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix missed partial_cmp in layout.rs and restore napi exports

- Convert two remaining b.y.partial_cmp(&a.y) calls to total_cmp
  in group_single_column and column layout sorting
- Restore missing napi exports: detectPdf, extractText,
  extractTextWithPositions, processPdf

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Abimael Martell
2026-04-02 14:39:42 -07:00
committed by GitHub
co-authored by Claude Opus 4.6
parent 732b1b1359
commit fba0a644ef
13 changed files with 137 additions and 137 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/extractor/layout.rs
+14 -22
View File
@@ -218,7 +218,7 @@ fn columns_have_prose(columns: &[ColumnRegion], items: &[&TextItem]) -> bool {
// Sort by Y descending (top of page = higher Y in PDF coords)
let mut sorted: Vec<&TextItem> = col_items;
sorted.sort_by(|a, b| b.y.partial_cmp(&a.y).unwrap_or(std::cmp::Ordering::Equal));
sorted.sort_by(|a, b| b.y.total_cmp(&a.y));
// Group into lines by Y-proximity and measure fill + item count
let mut full_lines = 0usize;
@@ -616,7 +616,7 @@ fn identify_spanning_lines(items: &[TextItem], columns: &[ColumnRegion]) -> Vec<
// Build (original_index, y) pairs sorted by Y descending for grouping
let mut indexed: Vec<(usize, f32)> =
items.iter().enumerate().map(|(i, it)| (i, it.y)).collect();
indexed.sort_by(|a, b| b.1.partial_cmp(&a.1).unwrap_or(std::cmp::Ordering::Equal));
indexed.sort_by(|a, b| b.1.total_cmp(&a.1));
// Group by Y-proximity into rough lines (as index sets)
let mut groups: Vec<Vec<usize>> = Vec::new();
@@ -778,7 +778,7 @@ pub(crate) fn is_newspaper_layout(
return 0.0;
}
let mut ys: Vec<f32> = lines.iter().map(|l| l.y).collect();
ys.sort_by(|a, b| a.partial_cmp(b).unwrap());
ys.sort_by(|a, b| a.total_cmp(b));
let span = ys.last().unwrap() - ys.first().unwrap();
span / (lines.len() as f32 - 1.0)
};
@@ -845,7 +845,7 @@ fn split_column_stragglers(lines: Vec<TextLine>) -> (Vec<TextLine>, Vec<TextLine
// Median gap = typical line spacing
let mut sorted_gaps = gaps.clone();
sorted_gaps.sort_by(|a, b| a.partial_cmp(b).unwrap_or(std::cmp::Ordering::Equal));
sorted_gaps.sort_by(|a, b| a.total_cmp(b));
let median_gap = sorted_gaps[sorted_gaps.len() / 2];
// A gap > 3× median (min 30pt) indicates a break between content clusters
@@ -1078,9 +1078,8 @@ pub(crate) fn group_into_lines_with_thresholds(
}
}
above.sort_by(|a, b| b.y.partial_cmp(&a.y).unwrap_or(std::cmp::Ordering::Equal));
below_spanning
.sort_by(|a, b| b.y.partial_cmp(&a.y).unwrap_or(std::cmp::Ordering::Equal));
above.sort_by(|a, b| b.y.total_cmp(&a.y));
below_spanning.sort_by(|a, b| b.y.total_cmp(&a.y));
all_lines.extend(above);
for col in core_columns {
@@ -1101,16 +1100,13 @@ pub(crate) fn group_into_lines_with_thresholds(
// Sort by Y descending (top-first), then by X for same-Y lines
all_page_lines.sort_by(|a, b| {
b.y.partial_cmp(&a.y)
.unwrap_or(std::cmp::Ordering::Equal)
.then(
a.items
.first()
.map(|i| i.x)
.unwrap_or(0.0)
.partial_cmp(&b.items.first().map(|i| i.x).unwrap_or(0.0))
.unwrap_or(std::cmp::Ordering::Equal),
)
b.y.total_cmp(&a.y).then(
a.items
.first()
.map(|i| i.x)
.unwrap_or(0.0)
.total_cmp(&b.items.first().map(|i| i.x).unwrap_or(0.0)),
)
});
// Merge lines at the same Y (within tolerance) into single lines
@@ -1185,11 +1181,7 @@ fn group_single_column(items: Vec<TextItem>, adaptive_threshold: f32) -> Vec<Tex
let items = if use_y_sorting {
// Sort by Y descending (top to bottom in PDF coords)
let mut sorted = items;
sorted.sort_by(|a, b| {
b.y.partial_cmp(&a.y)
.unwrap_or(std::cmp::Ordering::Equal)
.then(a.x.partial_cmp(&b.x).unwrap_or(std::cmp::Ordering::Equal))
});
sorted.sort_by(|a, b| b.y.total_cmp(&a.y).then(a.x.total_cmp(&b.x)));
sorted
} else {
items
src/extractor/mod.rs
+4 -7
View File
@@ -334,17 +334,14 @@ pub(crate) fn merge_text_items(items: Vec<TextItem>) -> Vec<TextItem> {
for (_, _, group) in &mut line_groups {
let rtl = is_rtl_text(group.iter().map(|i| &i.text));
if rtl {
group.sort_by(|a, b| b.x.partial_cmp(&a.x).unwrap_or(std::cmp::Ordering::Equal));
group.sort_by(|a, b| b.x.total_cmp(&a.x));
} else {
group.sort_by(|a, b| a.x.partial_cmp(&b.x).unwrap_or(std::cmp::Ordering::Equal));
group.sort_by(|a, b| a.x.total_cmp(&b.x));
}
}
// Sort groups by page then Y descending (top of page first)
line_groups.sort_by(|a, b| {
a.0.cmp(&b.0)
.then_with(|| b.1.partial_cmp(&a.1).unwrap_or(std::cmp::Ordering::Equal))
});
line_groups.sort_by(|a, b| a.0.cmp(&b.0).then_with(|| b.1.total_cmp(&a.1)));
let mut merged = Vec::new();
@@ -452,7 +449,7 @@ pub(crate) fn merge_subscript_items(items: Vec<TextItem>) -> Vec<TextItem> {
for (_, _, mut group) in line_groups {
// Sort by X position
group.sort_by(|a, b| a.x.partial_cmp(&b.x).unwrap_or(std::cmp::Ordering::Equal));
group.sort_by(|a, b| a.x.total_cmp(&b.x));
// Find the dominant (most common) font size in this group
let max_fs = group.iter().map(|i| i.font_size).fold(0.0_f32, f32::max);