feat(bindings): apply pdf annotations from every binding - #850
Merged
Conversation
andiwand
force-pushed
the
feat/pdf-annotation-browser
branch
from
September 6, 2026 16:03
91216ac to
98f28b0
Compare
andiwand
force-pushed
the
feat/pdf-annotate-bindings
branch
from
September 6, 2026 16:09
826bb02 to
86c6686
Compare
`PdfFile.annotate` in python, java and swift, `annotate()` on the wasm handle, and the `annotate` capability alongside them. Each returns the annotated bytes rather than writing a file: none of these callers has a filesystem the caller would want written to, and the wasm one least of all. Each binding gets a minimal pdf fixture built the way its neighbours are — computed cross-reference offsets, no file fetched — and tests that the source comes back as a prefix of the result, that the annotation is in what follows, and that a payload this build does not understand is refused. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018e3PEzyU2oAFSzsEoWsSmz
Every other binding call in `python/src` that does real work drops the GIL for it and takes it back to build the `py::bytes`; `annotate` held it for the whole parse and write, so a threaded caller stalled on it. Apple: hands the annotated bytes straight to `NSData` through a new `to_nsdata(const std::string &)` rather than wrapping them in an `istringstream` for the stream overload to drain into another `ostringstream` — three copies of the whole file for nothing. The stream overload now ends in the same place. Java: one `assertArrayEquals` for the copied prefix instead of an assertion per byte, and the xref entry's terminator written as the byte the format states rather than `%n` and a platform-separator replace. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0147n68S7LNAv9KynLaGgXN4
andiwand
force-pushed
the
feat/pdf-annotate-bindings
branch
from
September 6, 2026 16:19
86c6686 to
cbe1c5b
Compare
Eight lines had grown around three separate things. The api and its bindings are one entry, `odr.annotation` in the rendered view another, each the length of its neighbours. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0147n68S7LNAv9KynLaGgXN4
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.
🤖 Generated with Claude Code
Phase 6, the last of
docs/design/pdf-annotation.md. Stacked on #849.PdfFile.annotate(annotations) -> bytesPdfFile.annotate(String) -> byte[]PdfFile.annotate(_:) throws -> Datahandle.annotate(annotations) -> Uint8ArrayPlus the
annotatecapability onFileTypeCapabilitiesin all four.Each returns the annotated bytes rather than writing a file — none of these callers has a filesystem the caller would want written to, and the wasm one least of all. That matches how
save()already works in wasm and python.Fixtures
Each binding needed a pdf to test against, and each builds one the way its neighbours build theirs — inline, with the cross-reference offsets computed rather than hardcoded, so no test data is fetched. Four small generators: python
conftest.py, javaTestFiles, swiftFixture, wasmhelper.mjs.Each suite then checks the same three things: the source comes back as a prefix of the result (the incremental update is visible from the outside), the annotation is in what follows, and a payload this build does not understand is refused with that binding's error type.
All four built and run
jni/CMakeLists.txt; without that it compiles into nothing and the suite silently stays at 53, which is what happened on the first run here.Changelog entry extended to name the bindings and
odr.annotation.That completes the plan in
docs/design/pdf-annotation.md, whose status is now landed.