fix(markdown): reject non-text strikeout rules - #253
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Shadow auto-approve: would not auto-approve because issues were found.
Fix all with cubic | Re-trigger cubic
|
Addressed all three findings in @cubic-dev-ai please re-review. |
@abimaelmartell I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Shadow auto-approve: would not auto-approve because issues were found.
Fix all with cubic | Re-trigger cubic
|
Addressed the filled-rectangle connectivity finding in @cubic-dev-ai please re-review. |
@abimaelmartell I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Shadow auto-approve: would not auto-approve because issues were found.
Fix all with cubic | Re-trigger cubic
|
Addressed the baseline-drift finding in @cubic-dev-ai please re-review. |
@abimaelmartell I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 1 file
Shadow auto-approve: would require human review. The diff is truncated, so the full ownership heuristic and its tests are not visible; I cannot verify that tightened strikeout detection preserves genuine redlines while rejecting non-text rules. This tradeoff needs human review.
Re-trigger cubic
CI checks out a merge of the PR head with main, and main advanced 8 commits since this branch was cut — including #201 (contextual digit runs), #240 and #253 (markdown fixes). Those change extraction output, so a snapshot generated on the unmerged branch could not match; the Test job failed on the merge commit while passing on the branch itself. The merged behaviour is better: the footnote marker '2' before 'Hartley, R. V. L.' is now recovered instead of dropped. 950 tests pass on the merged tree, clippy clean.
#241) * fix(extractor): supply built-in metrics for non-embedded base-14 fonts PDFs may legally omit /Widths for non-embedded standard fonts (Times, Helvetica, Courier, Symbol, ZapfDingbats) — the spec requires the reader to supply the metrics. We returned None, so every glyph advanced 0 and each text item got width 0, silently breaking every gap-based heuristic downstream: space synthesis, sub/superscript detection, table column detection, heading merging. - src/extractor/base14.rs: Adobe Core-14 AFM width tables keyed by Unicode char, plus the standard Symbol/ZapfDingbats encoding vectors (their glyphs sit at byte positions unrelated to Latin text, so widths must resolve through the built-in encoding, not cp1252) - Width resolution order: Differences -> built-in encoding -> the same cp1252-style fallback the text decoder uses, so a code's advance always matches the character we emit for it - Type3 visual sizing: PK bitmap fonts (dvips) use FontMatrix [1 0 0 -1 0 0] with nominal sizes like 0.12pt; scale by FontBBox height x |matrix_y|. Applied in the page-stream and Form XObject paths. Indirect numeric array elements are resolved before use. Effect on Shannon's 'A Mathematical Theory of Communication' (1998 dvips/Distiller, the reported case): glued sentences 95 -> 5. Corpus impact: 12 of 184 eval documents, e.g. Data-Processing-Agreement recovers a paragraph that a phantom table had shredded into cells. Layout heuristics tuned on the same document (indent-based paragraph breaks, heading reclassification, table script filtering) are held back for a separate PR — they change ~98 further documents and need to be justified against the corpus, not against one PDF. * review: narrow Type3 rescaling to self-inconsistent fonts; dedup + test all width tables Addresses cubic review on #241, plus a follow-up from a local cubic run. - Type3 visual scaling was applied to every Type3 font whose FontBBox height x |matrix_y| deviated >5% from 1.0. FontBBox is the glyph box, not the em box, so a conventional 1/1000-matrix font with a descender..ascender bbox (~700 units) computed 0.7 and had every reported size shrunk by 30% — corrupting the drop-cap, heading-tier, sub/superscript and table heuristics this is meant to fix. First attempt gated on the matrix being unit-scale, but a local cubic run pointed out that wrongly excludes valid non-standard matrices (a 0.005 matrix with a full-em bbox legitimately needs a 5x scale). The product is the right discriminator, not the matrix: a self-consistent font lands near 1.0 because the matrix is the reciprocal of the glyph-space em, so only a wildly inconsistent one (dvips/PK bitmap fonts sit at ~159) is renormalized. Band widened to [0.25, 4.0]. Corpus effect: 12 -> 7 documents change. The 5 that drop out were being wrongly rescaled — including Data-Processing-Agreement, whose phantom-table fix turned out to come from this bug rather than from the width fallback, so it is correctly given up. - base14: all 14 width tables now covered by the sort-invariant test via an ALL_TABLES registry, not a hand-picked subset. - base14: identical tables share one static (all four Courier variants are monospace 600; the oblique Helvetica variants match their upright forms), removing 5 duplicate copies. * test: refresh Shannon snapshot after merging main CI checks out a merge of the PR head with main, and main advanced 8 commits since this branch was cut — including #201 (contextual digit runs), #240 and #253 (markdown fixes). Those change extraction output, so a snapshot generated on the unmerged branch could not match; the Test job failed on the merge commit while passing on the branch itself. The merged behaviour is better: the footnote marker '2' before 'Hartley, R. V. L.' is now recovered instead of dropped. 950 tests pass on the merged tree, clippy clean.
1 participant
Summary
Tightens geometric strikeout detection so a horizontal rule must be snugly owned by a contiguous text row and isolated from connected diagram geometry. This prevents bullets, formula bars, signature lines, chart rules, and schematic connectors from being emitted as deleted text while preserving genuine redline strikeouts.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Tightened strikeout detection so only real text strikeouts are marked. Handles split runs, inline scripts, and small baseline drift, and rejects non-text lines like connectors, bullets, chart rules, and thin filled connectors.
Written for commit f729f92. Summary will update on new commits.