Repository navigation
Consolidating the open rendering, text and font PRs: scope and evidence requirements #428
Description
Summary
First of all, thank you @wittjeff for your continued contributions across the board: rendering, fonts, text segmentation, structure, forms. The amount of work and the depth of the analysis behind these PRs are much appreciated, and we want to get them merged.
There are currently 21 open PRs from @wittjeff against docling-parse, plus several draft PRs against the regression dataset. Many of them overlap: some are stacked copies of each other, several change the same functions, and three pin HF_DATASET_REVISION to different, mutually exclusive dataset revisions. They can't be reviewed in this form.
This issue sets out how they should be consolidated and what evidence each change needs. All open PRs listed below are on hold until they have been resubmitted on this basis.
1. Consolidation by topic
Resubmit as one PR per topic, rebased on current main, without merge commits. One commit per logical fix within a PR is fine. Close the superseded PRs.
| Topic | Current PRs | Notes |
|---|---|---|
| Blend2D library patches | #387, #396, #400, #401 | Already stacked: each one contains the previous one. Keep one. |
| Renderer: stroke clipping, tiling patterns, soft masks | #397, #386, #390, #413 | Change the same files: blend2d_renderer.h, render_instructions.h, stream.h, grph.h, shape.h. #390 contains #386. #413 conflicts with main. |
| Word and cell segmentation | #356, #358, #409 | All change the contraction and word-gap logic in cells.h. |
| Font and ToUnicode decoding | #322, #327, #388, #389, #417, #419 | All change glyph-to-Unicode resolution (page_font.h, cmap.h, sanitizer constants) and share test files. |
| Standalone | #337, #410, #411, #427 | Unrelated to the topics above; can stay separate. |
Please open a new PR in a topic only once the previous one in that topic has been merged or closed.
2. Evidence from real PDFs
Every change must be shown on a real document. Synthetic test PDFs (tests/pdf_builder) are welcome as unit tests, but they don't demonstrate that a change matters in practice.
- Use the regression dataset first. If a document in the regression dataset already shows the problem, name it in the PR description, with the document name and page numbers. Don't open a dataset PR in that case.
- Dataset PRs only for documents that are actually new. Open a PR against the regression dataset only when no existing document reproduces the problem. It should add the new document, with a license that allows redistribution, and say which problem it reproduces.
- Text, font and segmentation fixes: for each cited page, show a before/after diff of the affected text cells or words.
3. Rendering changes: image regression evidence
Rendering changes must show that the output improves, which in practice means it moves closer to pypdfium2.
- For each cited page, include a three-panel comparison: pypdfium2, before, after.
- Include a before/after table of
mean_abs_errorandchanged_pixels_ratioagainst pypdfium2 over the whole regression corpus. The cited pages must improve, and no page may get worse by more than the tolerance intests/test_regression_pypdfium_render.py. - Add an image regression test that fails on
mainand passes with the change, comparing against pypdfium2 on the cited pages.test_regression_pypdfium_render.pyonly reports differences and never fails, so on its own it doesn't count as this test. - Blend2D patches also need a link to the issue or PR filed upstream with Blend2D, and a real document that crashes, hangs or renders incorrectly without the patch.
4. Ground truth
Don't pin HF_DATASET_REVISION to revisions from open dataset PRs. If a change alters the ground truth of existing regression pages, list those pages in the PR description and say why each one changes; the maintainers update the ground truth when merging. The existing ground-truth dataset PRs (3, 4, 7, 8, 9) should be closed. PRs that only add a new document under section 2 are fine.
Checklist for each resubmitted PR
- One topic per PR, rebased on
main, no merge commits - Superseded PRs closed and linked
- Regression dataset documents and pages named, or a dataset PR that adds a new document
- Before/after evidence for each cited page
- Rendering only: pypdfium2 comparison images, corpus-wide metrics table, and an enforced image regression test
- Blend2D only: link to the upstream report
- Changes to existing ground truth listed with the reason for each page
Activity
cau-git commentedon Oct 28, 2026
@wittjeff FYI.
wittjeff commentedon Oct 28, 2026
There's a lot of good efficiency-enhancing structure here. I'll need to study it more. I'll have a couple of questions I'll add later. I can submit a CONTRIBUTING.md PR if you'd like.
I had my senior engineer make a new tool, docmutant, which I think you may find useful for ground truth ingestion and derivation. I'd appreciate any feedback or input y'all might have.
Some feedback on how to increase throughput efficiency:
I am frankly baffled by people who submit an issue, where the drafting of that issue has clearly been aided by top-tier AI (no objection from me), yet they don't include a proposed fix or PR along with it. I know some people feel like they lack technical depth to double-check the AI's assumptions, but I contend that this is still not the more efficient path. AI is that good now. I'm not saying it's perfect. I'm saying it's good enough to make the current exercise increasingly silly.
Suggestion: encourage PR submission with each new issue, with a default prompt provided to explicitly enforce the submission guidelines.
Similarly, I think a deep queue of PRs naturally leads to staleness and minor merge conflicts. But the fix for those conflicts is rarely substantial. I am not offended if you want to kick every one back to the submitter to resolve, but again it doesn't seem like the most efficient path.
I have a ChonkerKeys keyboard, which I originally bought for use in Zoom meetings, and as a demo of an easy hardware accommodation for some dexterity impairments. I'm thinking of getting some stickers made for the keys: "Please investigate" [the issue reported on the current GitHub page], "Submit diagnosis summary comment and linked PR", and "Rebase and Resubmit".
For all of my career, I have straddled the front end and the back end, working with users, designers, product managers, developers, testers, customer support. I have sometimes been paid to write code, but not in most cases, so Wearing many different hats has often meant that I was the slowest developer in the room. Still, I have tried to keep up with technical topics enough to be able to make prudent recommendations to senior people doing the work.
Some people are IMO overly invested in titles, degrees, and years doing the mostly tedious but sometimes interesting implementation work. A lot of those people are having a hard time adjusting to the presence of AI, and IMO are falling into deeply human patterns of biased and counter-productive reactions [I got that degree in psychology because I find this kind of thing fascinating, BTW.]
I have had many ideas that sat in the back of my mind for years, stuck due to the time investment needed to get up to fast-enough speed. With AI, the Get Shit Done activation threshold is now close to zero, so I'm living in a totally different dream world. I have produced more in the last year than in the previous twenty. I am making up for lost time.
Still, I do not have any illusion that the AI magic wand makes me an expert. I can understand how frequent use of AI (and particular my easy default to letting it speak for me) might hide a lack of understanding, and merit more skepticism. I haven't sensed that kind of skepticism from any of your team, and it seems like you just take my submissions and the relevance of my expertise at (unavoidably veiled) face value. I just want to say that I really appreciate that.
wittjeff commentedon Oct 28, 2026
Following up on my offer above: I opened #430. It copies the rules from this issue into a new "Submitting pull requests" section in
CONTRIBUTING.mdand a new pull request template with the checklist. It also corrects three places intests/README.mdthat pointed contributors the other way (prefer synthetic unit tests, the pypdfium2 report as the render check, and bumpingHF_DATASET_REVISIONafter publishing). It adds no rules of its own.If you'd rather keep the rules here, or word them differently, feel free to close it or tell me what to change.
wittjeff commentedon Oct 28, 2026
Thanks again for laying this out. Before I start the topic PRs, a few questions so that I build them the way you want:
One PR per topic, or a sequence? I read section 1 as one PR per topic that carries all of that topic's fixes, one commit each. If one fix in a topic does not have a real document yet, can I leave it out and submit it later as the next PR in that topic? This applies to the last two Blend2D patches (fix(render): patch Blend2D so tiny strokes with a cusp do not streak across the page #400, fix(render): patch Blend2D so a flat cubic that folds back is still drawn #401). I found those defects with synthetic inputs and have no real document for them yet.
Documents I cannot redistribute. Some fixes come from user files that are not in the regression dataset and are not mine to relicense. Examples: the production pages from Segfault in get_image(scale=3.0) on a scanned CCITT page (regression in 7.10.0) #365, and the Nagra document from Page rendering never returns on a sub-pixel stroked curve (Blend2D stroke_path loop) #393. Can I cite the issue attachment or a public URL with page numbers, or do you want a reduced file that I can license for the dataset?
Crash and hang fixes. For the Blend2D patches, is the enforced test "the cited page renders, within the pypdfium2 tolerance, and does not time out" enough? Or do you also want the corpus-wide metrics table when the patch changes no pixels on the corpus?
"No page may get worse by more than the tolerance". Do you mean that a page's
mean_abs_erroragainst pypdfium2 may not increase by more thanPYPDFIUM_IMAGE_TOLERANCE(9.0 / 0.09)? Or that no page may move from below that tolerance to above it?Large ground-truth changes. fix(sanitizer): keep en/em dashes and the minus sign instead of folding to
-#388 changes 228 pages, and the only change is that en dashes, em dashes and minus signs are kept. fix: judge word gaps of runs without a space glyph in em, not in glyph advances #409 changes 104 pages. Is one reason per group of pages enough, with a script that shows that nothing else changes? Or do you want a reason for each page?