Repository navigation
Conversation
cmap_parser::get_source() decoded each bfchar/bfrange source code as UTF-16 and set_mapping/set_range read the key back with utf8::next. Codes 0xD800-0xDFFF are lone surrogates, so the decode threw and the fallback "GLYPH<cmap:...>" string was keyed as 'G' (0x47): the code lost its text and code 0x47 took the text of the bad entry. Large CJK fonts use this range; before 7.16.0 the lost mapping also crashed the parse with "Invalid code point" (docling#1111). Read source codes as big-endian integers with to_uint32(), as the codespacerange bounds already are, and pass integer codes to set_mapping and set_range. Fixes docling-project#145 Fixes docling-project#133 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Jeff Witt <1848307+wittjeff@users.noreply.github.com>
Contributor
|
✅ DCO Check Passed Thanks @wittjeff, all your commits are properly signed off. 🎉 |
Contributor
Merge Protections🟢 Merge protection satisfied — ready to merge. Show 1 satisfied protection🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
PeterStaar-IBM
self-requested a review
October 22, 2026 15:06
7 tasks
Member
|
Thanks for this contribution. This PR is on hold until it has been consolidated as described in #428. Please resubmit it as part of the topic PR listed there, with the evidence the issue asks for. |
5 tasks done
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/ToUnicodesource codes from0xD800to0xDFFFwere lost. A second mapping was also overwritten.cmap_parser::get_source()decoded each source code as UTF-16 text (to_utf8→utf8::utf16to8). It then read the key back withutf8::next. A source code is a byte sequence, not text, and the values0xD800–0xDFFFare lone surrogates in UTF-16. For those codes:utf16to8throws, and the catch returns the string"GLYPH<cmap:dfa8>".set_mapping/set_rangeread the first character of that string as the key:'G'=0x47.The result is that the code itself has no mapping, and code
0x47gets the text of the bad entry. Large CJK fonts use this range a lot. Before 7.16.0, the missing mapping also crashed the parse withRuntimeError: Invalid code point(docling#1111). Theis_code_point_validguard from #325 stopped the crash, but the mapping was still lost.The fix reads source codes as big-endian integers with the existing
to_uint32().codespacerangebounds are already read this way.set_mappingand bothset_rangeoverloads now take integer codes. For codes outside the surrogate range, the keys are the same as before.Fixes #145. Fixes #133.
Evidence
The issue files from docling#1111, parsed with 7.22.1 and with this branch (characters that come out as U+FFFD):
pranger-3.pdfp1 (NotoSansCJKjp,<DC77>)NASA_23_p1.pdfp1 (<DFA7>–<DFAC>= curly quotes)default.pdfp265 (Haansoft Batang,<DFCF>,<DFA2>)On
NASA_23_p1.pdf, the<0047> <0020>mapping was overwritten four times, so every word space in that font came out as”. With this branch, the text matchespdftotext:대략 ‘23m 전후’, 직경은 대략 ‘2m 전후’.Not fixed by this PR:
25.8._._0807.1.pdfpages 1–2 use codes such as<DF2B>that are not in their/ToUnicodeat all (the CMap has 37 entries, the highest is<526C>).pdftotextalso gets no text from these runs.Behavior change
A 4-byte source code now keeps all 32 bits. Before,
utf16to8masked it to 16 bits. A 1- or 2-byte code that is not a surrogate gives the same key as before.Tests
tests/test_unit_cmap_encoding.py: two new tests, for abfcharsource code and forbfrangebounds in the surrogate range. Both fail on 7.22.1 and pass with this branch.test_regression_parse.py::test_reference_documents_from_filenames,test_regression_threaded_parse.py::test_threaded_reference_documents_from_filenamesandtest_regression_threaded_render.py::test_render_reference_documents_from_filenames.test_reference_documents_from_filenames, 9 page/mode checks differ, onskipped_1page.pdfp2 andskipped_2pages.pdfp2–p3. In every case, the groundtruth at the pinned revision6d3458e2holds the bug output (GLYPH<c=55938,font=/AAAAAI+NotoSansCJKkr-Bold>, code0xDA82), and this branch gives the correct text (블록체인,헬스케어).pdftotextgives the same text. No other page in the corpus changed. The groundtruth for these 3 pages needs to be regenerated. I can prepare the dataset PR if you want me to.🤖 Generated with Claude Code