You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
There is no Arabic or RTL document in the test matrix, and pdf-evals snapshots can't easily cover it — real Arabic documents are mostly copyrighted or private, so there is nothing convenient to host.
That is a problem right now: #212 and PR #207 are both in flight against text_utils.rs, and whichever lands, nothing will catch it silently regressing later. RTL bugs are especially prone to that, because the output looks like text to a reviewer who doesn't read the script — the failure is invisible in a diff.
Proposal: generate the fixtures instead of hosting them
Print HTML to PDF with headless Chrome at test setup. No sample files in the repo, no licensing question, byte-stable across runs, and works in Linux CI:
We use exactly this in our own test suite and the same trick is the reproduction in #212, so it is already proven to produce the failing behaviour.
Three assertions worth copying
These are the ones that earned their keep in our suite:
1. Digit groups must survive reordering intact. Assert on a string like 126,248.34 embedded in an Arabic line — it catches digit-run reversal, thousands separators being split, and the decimal point migrating, all in one assertion. This is a live defect: see #212 (25 → 52) and #200.
2. Ligature round-trip. A line containing لا sequences (خلال, السلام, الكلام) must come back character-identical. Multi-character ToUnicode expansions are where naive reversal breaks, and it breaks quietly — the text still reads as Arabic, just wrong.
3. A negative control. This is the one people skip. Run the same input with the RTL path disabled and assert the output is wrong. If a test passes both with and without the fix, it isn't testing the fix — it's testing that the pipeline runs. We caught one of our own tests doing exactly that.
What I can contribute
I can send a PR with:
a fixture generator (HTML → PDF via headless Chrome, skipping gracefully when Chrome is absent),
the three assertions above against extractText and extractTextWithPositions,
Before I do — two questions so it lands in the right shape:
Do you want this in this repo's cargo test, or in the pdf-evals sibling repo alongside the snapshot corpus?
Is a headless-Chrome dependency acceptable in CI, or would you rather I emit the PDFs from a checked-in byte template so the tests have no external dependency at all? The second is more work but keeps CI hermetic — happy either way.
Also relevant: AGENTS.md notes that character-level diff alone misclassifies structural improvements as regressions and that bench.py score is the tie-breaker. For RTL that distinction is sharper than usual — a fix that corrects word order will look like a large diff and score as a regression under a naive comparison. Worth deciding up front which harness these fixtures answer to.
Problem
There is no Arabic or RTL document in the test matrix, and
pdf-evalssnapshots can't easily cover it — real Arabic documents are mostly copyrighted or private, so there is nothing convenient to host.That is a problem right now: #212 and PR #207 are both in flight against
text_utils.rs, and whichever lands, nothing will catch it silently regressing later. RTL bugs are especially prone to that, because the output looks like text to a reviewer who doesn't read the script — the failure is invisible in a diff.Proposal: generate the fixtures instead of hosting them
Print HTML to PDF with headless Chrome at test setup. No sample files in the repo, no licensing question, byte-stable across runs, and works in Linux CI:
We use exactly this in our own test suite and the same trick is the reproduction in #212, so it is already proven to produce the failing behaviour.
Three assertions worth copying
These are the ones that earned their keep in our suite:
1. Digit groups must survive reordering intact. Assert on a string like
126,248.34embedded in an Arabic line — it catches digit-run reversal, thousands separators being split, and the decimal point migrating, all in one assertion. This is a live defect: see #212 (25→52) and #200.2. Ligature round-trip. A line containing
لاsequences (خلال,السلام,الكلام) must come back character-identical. Multi-characterToUnicodeexpansions are where naive reversal breaks, and it breaks quietly — the text still reads as Arabic, just wrong.3. A negative control. This is the one people skip. Run the same input with the RTL path disabled and assert the output is wrong. If a test passes both with and without the fix, it isn't testing the fix — it's testing that the pipeline runs. We caught one of our own tests doing exactly that.
What I can contribute
I can send a PR with:
extractTextandextractTextWithPositions,Before I do — two questions so it lands in the right shape:
cargo test, or in thepdf-evalssibling repo alongside the snapshot corpus?Also relevant:
AGENTS.mdnotes that character-level diff alone misclassifies structural improvements as regressions and thatbench.py scoreis the tie-breaker. For RTL that distinction is sharper than usual — a fix that corrects word order will look like a large diff and score as a regression under a naive comparison. Worth deciding up front which harness these fixtures answer to.