Fix Hebrew visual-order extraction for per-glyph OCR text layers (fixes #826)

[palmoni5]

Fixes #826.

Problem

Scanned-Hebrew PDFs whose OCR text layer emits one show op per glyph (tesseract-style layers — an enormous corpus of scanned Hebrew religious texts) come out character-reversed from extract_text. Root cause chain:

1. Per-glyph spans carry no storage-order signal of their own, so the flush-path detectors (detect_visual_order_run, the accumulated_width heuristic) never fire — each span is a single character.
2. merge_adjacent_spans walks the reading-order-sorted (x-ascending) spans and concatenates them — producing the visual left-to-right sequence for RTL text.
3. Nothing downstream reverses it: Pass 0 of reverse_rtl_visual_order_runs is gated on Arabic Presentation Forms (Hebrew has none), and the short-span runs that Pass 1 targets no longer exist — the merge already collapsed them.
4. For letter-spaced runs that escaped the merge (running heads), Pass 0.5 reversed the span order and Pass 1 then merged in reverse span order — two reversals that cancel out, leaking visual-order text.

Fix

1. extractors/text.rs — merge_adjacent_spans tracks whether the accumulated span was built entirely from ≥2 single-glyph RTL pieces. Such a concatenation is the visual sequence by construction (regardless of how the producer stored the text), so finalize_visual_rtl_glyph_span reverses it to logical order — keeping digit runs forward (UAX fix: width calculation #9 rule L2) and combining marks attached to their base. Build-time-corrected multi-char show strings and Latin/CJK spans are untouched.

2. document.rs — the short-span run merge moved before Pass 0.5 (as "Pass 0.75") and emits logical characters directly, using atomic groups (base+marks, 2-char build-time pieces kept in order, folded digit runs). This removes the Pass 0.5 ↔ Pass 1 cancellation; the old Pass 1 is deleted as subsumed.

Verification

On a 78-page scanned Talmud volume (Tractate Beitza, one-glyph-per-Tj OCR layer, ~9,000 spans/page):

metric	before	after
per-page unique-token containment vs PDFium	4.8%	91%
reversed-word occurrences (e.g. logical אמר vs reversed רמא)	24 / 2,313	2,420 / 24
running head	הציב ןושאר קרפ	ביצה פרק ראשון ✓

The full test suite (5,714 tests) passes unchanged. The behavior was consistent across a 17-volume sample (containment 90–92% on every volume).

Happy to add a regression fixture if you can point me at the preferred format for golden corpus additions — the repro PDF from #826 is freely redistributable.

🤖 Generated with Claude Code

[yfedoseev]

Thank you — and a regression found: real Hebrew/Arabic PDFs now come out reversed via extract_text()

First off, thank you for reporting #826 and for this fix — the per-glyph diagnosis
and the Beitza-volume validation (4.8%→91% containment) are excellent, careful work
and I can reproduce the improvement locally.

Before merging, we ran this branch through our standard 419-PDF corpus regression
sweep (byte-diff vs main, every format) — our usual gate for any change touching
extraction. It flags a regression on two documents in the corpus:
wiki-cat-he/source.pdf and wiki-cat-ar/source.pdf (Hebrew and Arabic Wikipedia
article exports — ordinary, digitally-authored PDFs, not scanned/OCR).

Repro

import pdf_oxide
d = pdf_oxide.PdfDocument("wiki-cat-he/source.pdf")
print(d.extract_text(0)[:80])

• main: חתול הבית הוא מין מבוית של יונק מסדרת הטורפים... (correct — "The domestic cat is a domesticated species of...")
• this branch: לותח תיבה אוה ןימ תיובמ לש קנוי תרדסמ םיפרוטה... (every word's letters individually mirrored — garbage)

Same pattern on wiki-cat-ar (Arabic), though the corruption there is a partial
jumble rather than a clean mirror — likely because the atomic-grouping heuristic in
finalize_visual_rtl_glyph_span doesn't cleanly reconstruct Arabic's more complex
diacritic/ligature clustering.

Root cause

merge_adjacent_spans's new is_rtl_glyph_piece/finalize_visual_rtl_glyph_span
path fires whenever a maximal run of ≥2 short (≤2-char) RTL-only spans shows up
adjacent in x-order — it doesn't check why the spans are short. Scanned-OCR
text layers are one reason (your target case), but ordinary Hebrew/Arabic PDF
producers routinely emit short spans too, for unrelated reasons (kerning pair
breaks, font-run boundaries, subsetting) — and their short spans already carry
correctly-ordered logical text. The merge-and-reverse fires on both shapes
identically, so it silently corrupts the second, much more common, category.

This isn't a narrow edge case — of the ~120 broad-coverage PDFs in our sweep,
2/2 real-world Hebrew/Arabic documents in the corpus hit it.

Suggested fix direction

Gate the merge-and-reverse on some signal that's actually specific to OCR-sandwich
text, not just "short RTL spans in a row." The most direct one available at this
point in the pipeline is text render mode (Tr 3/7 — invisible, neither fill nor
stroke — is what OCR-sandwich layers use almost universally, since the recognized
text has no rendering-correctness pressure). If TextSpan doesn't currently carry
render mode through to merge_adjacent_spans, that'd need threading through, similar
to how TjBuffer::render_mode is captured for the buffer-flush sites elsewhere in
this file.

Happy to pair on this / test a revised version against the same corpus + the Beitza
volume once you have a candidate — want to make sure the per-glyph fix (which is
clearly needed and well-validated) doesn't ship with this regression attached.

In the meantime, we opened #832, which fixes the narrower per-word OCR-sandwich
shape of #826 (one TJ array per recognized word, rather than per glyph) in
extract_text() — validated against the same 419-PDF corpus with zero regressions.
It's a different code path from this PR (extractors/text.rs's buffer-flush sites,
not merge_adjacent_spans) and doesn't cover the per-glyph shape this PR targets, so
they're complementary rather than overlapping — #826 will need both, once this one's
regression is resolved.

Thanks again for digging into this — real scanned-Hebrew corpora are exactly the kind
of hard case that's easy to get subtly wrong, and the Beitza-volume before/after
numbers made this much easier to review.

[palmoni5]

Thank you — this is a fair catch, and the 419-doc sweep gating is exactly the safety net this kind of change needs. I dug into the fix direction you suggested and hit two empirical dead-ends I want to share, because they narrow the problem in a useful way and I think I now need one thing from your corpus to finish it.

Render mode (Tr 3/7) doesn't separate the cases

I threaded render mode through to TextSpan and checked the target corpus first. Every span in the Beitza volume is Tr 0 (fill / visible), not 3/7. These scanned-Hebrew seforim aren't the classic invisible-text-over-image OCR sandwich — the recognized text is drawn in fill mode as the visible content (image-free "text PDFs" from the OCR/typesetting tool). So gating the reversal on invisible render mode would disable the fix for exactly the corpus #826 is about. Render mode is out.

Draw direction doesn't separate them either

My next hypothesis was the storage-order signal the codebase already computes — detect_rtl_draw_direction / rtl_draw_logical — reverse only visual-order (LTR-drawn) runs, leave logical-order (RTL-drawn) runs alone. But measuring it:

• The Beitza per-glyph runs are flagged rtl_draw_logical = true — i.e. they are drawn right-to-left (decreasing x), logical storage.
• Gating the reversal on !rtl_draw_logical dropped Beitza from 91% containment back to ~10% (the reversed forms return).

So the target and your regression case are both logical-order-drawn. Draw direction can't tell them apart.

What I think is actually going on

The reading-order sort sits between detect_rtl_draw_direction (pre-sort) and merge_adjacent_spans (post-sort). For the per-glyph Beitza case, the sort reorders the individually-drawn glyphs into x-ascending (visual) order, and my merge-then-reverse puts them back to logical — which is why it works there. Your wiki-cat-he/-ar case is also logical-drawn but, I suspect, not one-glyph-per-op — the short spans come from kerning/subsetting and are multi-glyph, each already internally logical. If so, the corruption is most likely coming from the document.rs "Pass 0.75" run-merge (which triggers on any ≤2-char RTL run) over-firing on those, rather than from the merge_adjacent_spans glyph-piece path (which requires genuinely single-glyph pieces). That would make the real discriminator span granularity (true one-glyph-per-op) plus a per-run visual-order check done pre-sort where the x-monotonicity is still intact — mirroring your existing detect_visual_order_run, but across a per-glyph run instead of within one multi-glyph cluster.

I can't confirm that without reproducing the regression, and it's not in anything I have locally.

The ask

Could you share wiki-cat-he/source.pdf and wiki-cat-ar/source.pdf (or, if they're not redistributable, a extract_spans(0) dump for page 0 — per-span text, bbox.x/y, char count, and rtl_draw_logical)? With the actual span shapes next to Beitza's I can pin down the discriminator and re-gate precisely, then you can re-run both against the full corpus.

In the meantime #832 landing the per-word shape safely is great — that's the more common case, and it means #826 isn't blocked on getting the per-glyph shape perfect. No objection to this PR sitting until I have a version that passes your sweep; I'd rather it wait than ship the regression.

🤖 Generated with Claude Code

[yfedoseev]

Thanks for the careful root-causing and the honest write-up above — the render-mode and draw-direction dead-ends you shared saved me from repeating them. Pushed a commit onto this branch: one real fix, plus a fixture test file documenting a second, deeper problem I found but haven't solved.

Fixed: document.rs's Pass 0.75 gate was too loose

Pass 0.75 (the merge-and-reverse for per-glyph scanned-OCR text) fires on any run of 4+ short (≤2-char) same-line RTL spans — it doesn't check why a span is short. A genuine one-glyph-per-show-op OCR run and four ordinary already-correct 2-letter spans (a routine font kerning-pair or subsetting-run boundary — common in any digitally-authored Hebrew/Arabic PDF) both satisfy "≤2 chars," so the second case got merged and reversed too.

Tightened the gate to require exactly one base RTL letter per span — mirrors extractors/text.rs's is_rtl_glyph_piece, which already gets this right (bases > 1 → false), so Pass 0.75 is just catching up to a discriminator this codebase already has correct elsewhere. ordinary_two_char_spans_are_not_reversed in the new test file pins this.

Verified: full lib suite (5716 tests) + every other RTL integration test file in the repo (rtl_tj_array_word_buffer, rtl_ocr_multiscript_regression, rtl_struct_order, rtl_markdown_heading, rtl_paragraph_reflow_md, test_rtl_script_support) all pass unchanged.

Still broken: this fix isn't sufficient on its own

Rebased onto current main (which now has the per-word OCR-sandwich fix) and re-tested against the real documents from my first review — wiki-cat-he/source.pdf and wiki-cat-ar/source.pdf still come out with every word individually letter-mirrored, even with the Pass 0.75 fix applied.

Traced it further: those four short spans are tightly kerned (touching, zero gap), so they're also getting swept into an ordinary adjacent-span merge — unrelated to any OCR/glyph-piece detection — and the resulting combined span is then reversed by whatever this codebase's general geometric visual/logical detector is doing at that point. I confirmed this precisely (not by eyeballing Hebrew text in a terminal, which is unreliable — bidi rendering will fool you): the failing test's output is an exact character-for-character reversal of the full 8-char run, span boundaries preserved as line breaks. That's a different code path than Pass 0.75 and I haven't found or fixed it — ordinary_two_char_spans_are_not_reversed's doc comment has the full trace.

This is a genuinely deeper problem than a gate tweak: it means "one base RTL char per span" and "tightly-kerned short spans" are both unreliable OCR signals, since ordinary digitally-authored PDFs (browser/print-to-PDF renderers doing precise per-glyph kerning, common for justified RTL text) produce the same shapes. Worth trying: checking for an underlying scanned-image XObject on the page — real OCR text sits over a scan, ordinary digital PDFs don't have one. Not available at merge_adjacent_spans's point in the pipeline today, so it'd need threading through, but it's a structural signal rather than another span-shape heuristic, which is what's kept failing here.

Also worth flagging: there's an existing dependency-freshness tracking issue (#793) if any of this touches crate versions — probably not relevant here, just noting it exists.

No PDFs shared (per this repo's fixture policy) — all reproductions are synthetic, built in-code, in the new test file.

[yfedoseev]

🔴 This can't merge yet — the branch leaves a failing test

@palmoni5 thanks again for the deep root-causing. The document.rs Pass-0.75 gate tightening is a strict improvement. But the branch currently has a committed, non-#[ignore]d failing test — reproducible with:

cargo test --test rtl_short_span_glyph_granularity
# ordinary_two_char_spans_are_not_reversed FAILS:
#   got  "חז\nוה\nדג\nבא"   expected  "אבגדהוזח"

So cargo test is red on this branch (the reversal comes from a pre-existing flush_tj_buffer + Pass-0.5 path, not Pass 0.75).

Separately, the net-new finalize_visual_rtl_glyph_span (src/extractors/text.rs) reverses any span merged from ≥2 single-glyph RTL pieces without a text-render-mode (Tr) gate — unlike main's sibling per-word fix — so it can reverse already-correct non-OCR Hebrew/Arabic (the 419-doc sweep note in rtl_tj_array_word_buffer.rs documents this).

Suggested path to green: split the sound document.rs gate from the risky text.rs finalize; #[ignore] the known-failing test with a tracking issue, or Tr-gate finalize_visual_rtl_glyph_span.

[yfedoseev]

Status check on this one, re-verified today against current main.

• ordinary_two_char_spans_are_not_reversed still fails on the branch as it stands (1 passed; 1 failed). To be clear about whose ball this is: that test came in with @yfedoseev's own commit 9595e92c, added deliberately to document the deeper problem found during review, so the red is a known marker rather than anything you left behind.
• The branch is now 74 commits behind main and no longer merges cleanly — src/extractors/text.rs conflicts. That file has moved a lot in the meantime, including RTL-adjacent work (word-fusion fixes in [Bug]: Inter-word spaces still dropped on TJ-positioned runs on 0.3.73 (partial regression of #803) #847, /PlacedPDF handling in fix(text): keep unique /PlacedPDF bodies instead of suppressing them #896, per-glyph advance accuracy), so it's worth rebasing before any further work: some of what you were fighting may have shifted underneath.

Nothing needed from you beyond the rebase right now — the outstanding design question is on our side, and I've asked @yfedoseev whether that documented test should be resolved or marked #[ignore] with a pointer so the branch can go green. The Pass-0.75 gate tightening itself still looks like a strict improvement, and the root-causing you did here remains some of the most careful work on this repo.