fix(extractor): clip sparse off-page text - #437
Conversation
There was a problem hiding this comment.
3 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/extractor/mod.rs">
<violation number="1" location="src/extractor/mod.rs:343">
P2: When a text run continues into the visible page from the left edge, `straddles` does not recognize it because it only checks right-side adjacency. Check adjacency in both directions before clipping, or this drops left-edge continuations despite the retention rule.</violation>
<violation number="2" location="src/extractor/mod.rs:346">
P2: Removing the `off.len() >= 10` floor and the `!coords_rotated` guard makes clipping much more aggressive for rotated pages while the surrounding comment still claims rotated display text "must stay." The `wordy_chars * 2 >= total_chars.max(1)` ratio treats a single off-box item of 4+ characters as coherent, so a lone rotated or curved display label/word lying outside the CropBox (e.g. a vertical side label) is now silently dropped, where previously rotated pages were never clipped and needed at least 10 off-page items even for non-rotated pages. The `straddles` guard only protects items that continue a visible on-page line, so it does not cover a standalone wordy fragment. If a sparse coherent phrase is the goal, a minimal item-length floor (2+) would still clip the regression cases while preserving single legitimate fragments.</violation>
</file>
<file name="tests/integration_tests.rs">
<violation number="1" location="tests/integration_tests.rs:600">
P3: The `a` and `b` assertions don't actually verify the short fragments survive, because the retained words already contain those letters: "Continuation" has an `a` and "Visible" has a `b`. The joined string would still match these substrings if the `a`/`b` fragments were dropped, silently masking a partial regression. Only the `c` assertion is meaningful. Assert against the item list (e.g. check each fragment exists as its own item) or a more specific substring so each fragment is genuinely validated.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Fix all with cubic | Re-trigger cubic
| // artifacts of transforms we mis-model — don't clip those. | ||
| let straddles = off.iter().any(|o| { | ||
| items.iter().any(|i| { | ||
| !outside(i) && (i.y - o.y).abs() <= 2.0 && (o.x - (i.x + i.width)).abs() <= 10.0 |
There was a problem hiding this comment.
P2: When a text run continues into the visible page from the left edge, straddles does not recognize it because it only checks right-side adjacency. Check adjacency in both directions before clipping, or this drops left-edge continuations despite the retention rule.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/extractor/mod.rs, line 343:
<comment>When a text run continues into the visible page from the left edge, `straddles` does not recognize it because it only checks right-side adjacency. Check adjacency in both directions before clipping, or this drops left-edge continuations despite the retention rule.</comment>
<file context>
@@ -303,74 +303,80 @@ fn extract_positioned_text_impl(
+ // artifacts of transforms we mis-model — don't clip those.
+ let straddles = off.iter().any(|o| {
+ items.iter().any(|i| {
+ !outside(i) && (i.y - o.y).abs() <= 2.0 && (o.x - (i.x + i.width)).abs() <= 10.0
+ })
+ });
</file context>
| !outside(i) && (i.y - o.y).abs() <= 2.0 && (o.x - (i.x + i.width)).abs() <= 10.0 | |
| !outside(i) | |
| && (i.y - o.y).abs() <= 2.0 | |
| && ((o.x - (i.x + i.width)).abs() <= 10.0 | |
| || (i.x - (o.x + o.width)).abs() <= 10.0) |
| !outside(i) && (i.y - o.y).abs() <= 2.0 && (o.x - (i.x + i.width)).abs() <= 10.0 | ||
| }) | ||
| }); | ||
| let coherent = wordy_chars * 2 >= total_chars.max(1) && !straddles; |
There was a problem hiding this comment.
P2: Removing the off.len() >= 10 floor and the !coords_rotated guard makes clipping much more aggressive for rotated pages while the surrounding comment still claims rotated display text "must stay." The wordy_chars * 2 >= total_chars.max(1) ratio treats a single off-box item of 4+ characters as coherent, so a lone rotated or curved display label/word lying outside the CropBox (e.g. a vertical side label) is now silently dropped, where previously rotated pages were never clipped and needed at least 10 off-page items even for non-rotated pages. The straddles guard only protects items that continue a visible on-page line, so it does not cover a standalone wordy fragment. If a sparse coherent phrase is the goal, a minimal item-length floor (2+) would still clip the regression cases while preserving single legitimate fragments.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/extractor/mod.rs, line 346:
<comment>Removing the `off.len() >= 10` floor and the `!coords_rotated` guard makes clipping much more aggressive for rotated pages while the surrounding comment still claims rotated display text "must stay." The `wordy_chars * 2 >= total_chars.max(1)` ratio treats a single off-box item of 4+ characters as coherent, so a lone rotated or curved display label/word lying outside the CropBox (e.g. a vertical side label) is now silently dropped, where previously rotated pages were never clipped and needed at least 10 off-page items even for non-rotated pages. The `straddles` guard only protects items that continue a visible on-page line, so it does not cover a standalone wordy fragment. If a sparse coherent phrase is the goal, a minimal item-length floor (2+) would still clip the regression cases while preserving single legitimate fragments.</comment>
<file context>
@@ -303,74 +303,80 @@ fn extract_positioned_text_impl(
+ !outside(i) && (i.y - o.y).abs() <= 2.0 && (o.x - (i.x + i.width)).abs() <= 10.0
+ })
+ });
+ let coherent = wordy_chars * 2 >= total_chars.max(1) && !straddles;
+ if bx1 - bx0 >= 72.0 && by1 - by0 >= 72.0 && coherent {
+ let before = items.len();
</file context>
|
|
||
| assert!(text.contains("Visible")); | ||
| assert!(text.contains("Continuation")); | ||
| assert!(text.contains("a")); |
There was a problem hiding this comment.
P3: The a and b assertions don't actually verify the short fragments survive, because the retained words already contain those letters: "Continuation" has an a and "Visible" has a b. The joined string would still match these substrings if the a/b fragments were dropped, silently masking a partial regression. Only the c assertion is meaningful. Assert against the item list (e.g. check each fragment exists as its own item) or a more specific substring so each fragment is genuinely validated.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/integration_tests.rs, line 600:
<comment>The `a` and `b` assertions don't actually verify the short fragments survive, because the retained words already contain those letters: "Continuation" has an `a` and "Visible" has a `b`. The joined string would still match these substrings if the `a`/`b` fragments were dropped, silently masking a partial regression. Only the `c` assertion is meaningful. Assert against the item list (e.g. check each fragment exists as its own item) or a more specific substring so each fragment is genuinely validated.</comment>
<file context>
@@ -527,6 +529,79 @@ fn test_digit_only_text_runs_are_preserved_in_markdown() {
+
+ assert!(text.contains("Visible"));
+ assert!(text.contains("Continuation"));
+ assert!(text.contains("a"));
+ assert!(text.contains("b"));
+ assert!(text.contains("c"));
</file context>
Summary
/Rotate0/90/180/270.Testing
main(HIDDEN-OFF-PAGEsurvived) and passes after the change./Rotate0/90/180/270, normalized embedded rotated text, line continuations, and short fragments.cargo test— 987 lib, 3 CLI, 166 integration, and 2 doc tests passed.cargo clippy -- -D warningspassed.cargo fmt --all -- --checkpassed.git diff --checkpassed.