[davidschachterADFA]

ADFA-6629

Adds scripts/optimize_pdfs.py, a ship-time step that rewrites PDFs losslessly so the Brotli they ship in compresses them better. It handles the database's PDF rows, and (with --file) a Bookshelf plugin book. Download: −373,806 bytes on documentation.db.br, and −7,340,849 on the Bookshelf books (PR 2, addons, will ship the rewritten books).

Why

Every shipped PDF is already inside Brotli: a Content row, or a .pdf.br plugin asset. But inside the PDF most streams are Flate, and Brotli can't shrink Flate output. So the step stores those streams plain and lets the outer Brotli do the compressing.

Classic optimization was measured and rejected:

• qpdf object streams with Flate recompressed at level 9 saved 0.1%.
• --recompress-flate strips PNG predictors from images (Java Notes grew by 60 KB), so it is never used.

What the rewrite does

• Non-image streams with only generalized filters (content, fonts, ICC profiles, forms) are stored plain.
• [/FlateDecode /DCTDecode] JPEGs are stored bare, keeping their DCT decode parameters. This happens only when the Flate data is whole: the checksum is right and nothing follows the end.
• Flate images (8-bit gray, 8-bit RGB, 1-bit) are re-encoded with PNG predictors when that is smaller, and only when Pillow gives back exactly the same sample bytes. Pillow zeroes set pad bits in 1-bit rows, so such an image stays as it is.
• Bare JPEG, JPEG 2000, JBIG2 and CCITT images are left as they are.
• A stream qpdf can't decode without a warning is never rewritten, images included.
• Output is saved with object streams and a deterministic /ID, so a second run changes nothing.

In the database, rows are UPDATEd in place via replace_row. Deleting and re-inserting the base row would fire AddBook/DeleteBook and replace the curated Bookshelf entry. Continuation rows are re-chunked as needed.

Checks

Each rewrite is refused (the PDF stays as it is, and the report names why) unless all of these hold:

• Same object graph. Walked from /Root and /Info, the two graphs match key for key; only a stream's /Filter, /DecodeParms and /Length may change.
• Same stream data. Generalized filters are fully decoded, so a re-encoded image must give back the same samples. JPEG, JPX, JBIG2 and CCITT streams must have the same payload and parameters. A stream among the parameters, such as a JBIG2Globals, is compared by its data too.
• Clean syntax check. pikepdf's check_pdf_syntax() must find nothing. It decodes every stream, JPEGs and JBIG2 included. JBIG2 goes through jbig2dec; where that's missing, a PDF with a JBIG2 image is refused.
• Same structure. The page count, named destinations (/Dests and the /Names /Dests tree) and the outline, with its nested items, actions and levels, are unchanged. A stream inside any of them is compared by its data.

A rewrite that isn't smaller once compressed is left alone.

Left alone by policy, which is not a failure:

• an encrypted PDF;
• a signed PDF (/AcroForm SigFlags bit 1, or a /Perms entry), because a rewrite would break the signature while every check passed;
• a PDF row that isn't Brotli-compressed.

Report and exit code: the report ends with a summary, e.g. 6 rewritten; left as they are: 9 not smaller. Refusals are counted by check, policy skips by reason. Exit 1 means nothing was written: a row could not be read, or a check refused every PDF that policy did not skip. In file mode, a check refusal writes nothing and exits 1. A policy skip or a not-smaller result copies the input. The target keeps its mode; a new file gets 0666 less the umask.

Real data

Database: 2026-10-19 item-B ship-time build (4c2005de). Both copies were packed with rebuild_database.py --pack Content,TooltipTexts --page-size 4096, then compressed with brotli -q 11 -w 24. The baseline is the same database packed the same way.

	Without step	With step	Saved
.br (download)	67,288,387	66,914,581	373,806
File	85,708,800	85,340,160	368,640

• The 6 d/ guides go from 1,187,754 to 821,335 stored bytes.
• The 9 placeholder PDF rows (2.4 KB or less each) stay as they are, because their rewrite is larger.
• A second run reports nothing to do.
• integrity_check and foreign_key_check pass.
• The Bookshelf join (titles and paths) is identical.

Books (addons 6d14aa7, file mode): 20,357,211 → 13,016,362 bytes.

Book	Before	After	Plain PDF
AndroidNotesForProfessionals	8,810,306	6,320,438	9.8 → 33.9 MB
JavaNotesForProfessionals	4,772,016	2,812,106	6.9 → 25.5 MB
JavaJavaJava	4,162,347	2,258,547	4.8 → 14.7 MB
BeejGuideToCProgramming	1,346,539	705,655	1.4 → 6.8 MB
KotlinNotesForProfessionals	787,651	590,175	1.1 → 3.7 MB
ModernCplusplusTutorialOuChangkun	388,053	278,975	0.4 → 1.2 MB
PebbleTemplateGuide	90,299	50,466	0.15 → 0.32 MB

Render check: every page of all 13 PDFs (3,816 pages) renders identically in poppler at 72 dpi, in colour, before and after. The page counts and pdfinfo -dests lists are unchanged.

The review's fixes change no shipped output. Rerun at 6916e49:

• the 7 books are byte-identical to the first run's;
• the 6 database rows match, PDF and stored row, the output of dfc064e's code byte for byte.

qpdf's CLI --check reports "skipping over invalid kid" on the rewritten Beej (3) and JavaJavaJava (44). These are empty dictionaries that their /Dests name trees already hold in the originals, at the same places, and the counts are the same before and after (Beej 3 and 3, JavaJavaJava 44 and 44). The graph check proves the trees are unchanged.

Tests

scripts/test_optimize_pdfs.py has 74 tests. They run under pytest and as a plain script, and now in CI (python-tests.yaml). That step also installs jbig2dec for the JBIG2 test, which skips through pytest where pikepdf can't decode JBIG2. test_svg_images_to_webp.py still passes on the shared helpers. The fixtures are PDFs built with pikepdf in the tests; the one binary is a 116-byte JBIG2 sample made with jbig2enc, embedded as base64. CI's exact step was run locally (Python 3.11, pip pikepdf 10.17.0, jbig2dec): 326 passed.

Mutants: 75, none surviving. Each was applied to a copy of scripts/ and checked against the tests it targets, each failing for the reason it is named for:

• my 34 from the first round;
• the review's 23 single-edit mutants, as reconstructed from its list (all 14 it reported surviving, plus 9 guesses at the rest);
• 18 reverts and mutants of this round's fixes.

Separately, the brotli-CLI skip shows as "skipped" under pytest with the fix, and as "passed" without it.

Notes

• Ship-time position: after svg_images_to_webp.py, before the dictionary re-mint and rebuild_database.py --pack. That order lives on the ADFA-3305 stack branches, not on main, so main's CLAUDE.md only names the step among the live scripts.
• New dependency: pikepdf>=10 (a pip wheel with qpdf inside), added to requirements.txt. CI also installs jbig2dec.
• Phone memory (open, D4): the plain PDFs grow about 3.4x. The app decodes a whole row into memory (largeHeap 512 MB), so the phone test will measure Android Notes' load time and heap. PR 2 waits for it. If a cap is needed, it goes in keep_rewrite().

🤖 Generated with Claude Code

https://claude.ai/code/session_018CMZQuXT3GT7nEct2w4tg3

[davidschachterADFA]

Independent Claude review (head dfc064e)

No data loss, and no Critical or High findings. Every real PDF tested comes out lossless, and the database mode is safe. The weak spots are test strength (most of the graph check's structural guards are unpinned), a few cases that wrongly refuse a whole PDF, and one docstring claim the code doesn't honour.

ID	Severity	Where	Issue	Failure scenario
R55-01	Medium	optimize_pdfs.py:346-390	8 graph-check guards untested	A refactor drops one; tests still pass
R55-02	Low	optimize_pdfs.py:297-298	canonical() compares a stream by its raw dictionary, never by its data	Flate JBIG2Globals, or a /JS outline action: whole PDF refused
R55-03	Low	optimize_pdfs.py:206-209	Pillow zeroes the pad bits of 1-bit rows	Odd width with pad bits set: whole PDF refused
R55-04	Low	optimize_pdfs.py:83-86	The docstring says a stream with a decode warning is never rewritten	An image or wrapped JPEG with a warning is rewritten and accepted
R55-05	Low	optimize_pdfs.py:425-426	Signed PDFs are not refused	The signature breaks; every check passes
R55-06	Low	optimize_pdfs.py:578-582	File mode writes mode 0600	An in-place run turns a 644 book into 600
R55-07	Low	optimize_pdfs.py:508-510	An all-refused run says "Nothing to do" and exits 0	A pikepdf change refuses every PDF; the ship build can't tell
R55-08	Low	python-tests.yaml:68-74, test_optimize_pdfs.py:667-669	Suite not in CI; one test passes when it skips	The DB-writing tool regresses unseen

R55-01: 23 single-edit mutants ran against the 40 tests, and 14 survived.

• Eight of the survivors are guards in compare_graphs: the dict key set, the array length, the /Info walk, both identity-mapping checks, the stream key set, indirect on one side only, and same_scalar's string comparison.
• test_the_graph_check_refuses_a_changed_stream_key changes Length1's value, not a key.
• The other survivors: ICCBased component counting, outline /A and depth, object-stream mode, fix_metadata_version, and file mode's not-smaller comparison.
• The PR's fail-without-fix claim holds for value changes only.
• Fix: add a _with_fault test for each of an added dict key, an appended array element, an added stream key, a changed /Info string, a changed string elsewhere and a direct-to-indirect change, plus a file-mode not-smaller test.

R55-02: a stream inside a compared structure is compared as ("stream", stream_dict), which includes /Filter and /Length.

• A Flate /JBIG2Globals produced "decodes differently", an outline /JS action "the outline changed", and a stream in a named destination "the named destinations changed".
• None of the 13 shipped PDFs hits this, but scanned books often use JBIG2.
• Fix: drop the rewritable keys from a stream's dictionary in canonical() and compare its payload, or leave streams referenced from /DecodeParms alone.

R55-03: a 509×64 1-bit Flate image with its pad bits set is refused; with them zeroed, it is accepted. The fixture noisy_rows is 512 wide, so the case is never exercised.

• Fix: have png_flate return None unless Image.frombytes(...).tobytes() == samples.

R55-04:

• A Flate image with a cut adler32 makes qpdf warn, yet it is re-encoded and accepted. A Flate-wrapped JPEG with a cut adler32 is unwrapped and accepted.
• The data was complete in both cases, so the output is lossless, but the docstring and the PR body say otherwise.
• A truncated wrapped JPEG is caught.
• Fix: either reword the docs, or count warnings in the image branch and unwrap with strict zlib.decompress.

R55-05: a PDF with /AcroForm /SigFlags 3 and a /Sig field with /ByteRange was rewritten and passed every check. None of the shipped PDFs is signed.

• Fix: refuse when SigFlags & 1 is set or a /Perms entry exists.

R55-06: mkstemp then os.replace leaves the file at 0600. The e2e books are already -rw-------. Git records 100644 regardless, so the impact is small.

• Fix: chmod to the target's mode, or to 0666 & ~umask, before the replace.

R55-07: with rewrite_pdf patched to always refuse, the run printed the refusals, then "Nothing to do", and exited 0.

• Fix: count the refusals, report them in the summary, and exit non-zero when every candidate was refused.

R55-08: CI already has what the suite needs: pikepdf is in requirements.txt and the workflow installs the brotli CLI. test_rows_stay_in_the_database_dictionary returns early, and passes, without the brotli CLI.

• Fix: add the suite to the pytest step, and call pytest.skip under pytest.

Checked and sound:

• Books:
  • verify passes for all 7 books, a fresh run reproduces the e2e bytes exactly, and a second run is byte-identical.
  • poppler's pdfimages -all gives byte-identical output for all 1,216 images, covering JPEG, JPX, Flate RGB and gray, Indexed (8-bit and 4-bit), and 16-bit with SMask.
• Database copy (4c2005de):
  • The dry run left it byte-identical and wrote no backup.
  • The per-row figures match.
  • The placeholders are left as they are.
  • integrity_check is ok and foreign_key_check is empty.
  • The Bookshelf join and every Content key are unchanged, and every non-PDF row is unchanged.
  • A second run says "Nothing to do".
• Triggers: only AFTER INSERT and AFTER DELETE exist, so the UPDATE never fires them.
• Interrupts: an interrupt mid-transaction rolls back.
• Syntax check: check_pdf_syntax decodes every stream, JPEGs included.
• qpdf's "invalid kid" warnings: the counts are identical before and after (Beej 3, JavaJavaJava 44).
• Edge cases that behave correctly:
  • TIFF predictors;
  • an unsupported predictor (refused cleanly);
  • trailing bytes after the zlib stream;
  • dangling references;
  • a direct /Info;
  • encrypted PDFs (refused).
• Linearization: WebServer.kt has no Range support, so losing it costs nothing.
• Not verified: whether the output is byte-identical across Pillow or zlib builds.

Open risk, not a finding: the plain PDFs grow up to 3.5× (Android Notes from 9.8 to 33.9 MB). This is the PR's open item D4, and the addons PR should wait for the phone test.

[davidschachterADFA]

Response to the independent review (head dfc064e, now 6916e49)

Thanks, all eight are fixed. R55-01 was the real gap: the first round's proofs pinned value changes only. None of the fixes changes the output for the shipped PDFs (proof below).

ID	What changed	Commit
R55-01	A test for every graph-check guard you listed; the stream-key test now adds a key. Also tests for ICCBased, outline /A and levels, object streams, XMP left alone, and file mode not-smaller. Three more survived my rerun and are now tested: "a stream/dictionary on one side only" and png_flate's length check	924b28b
R55-02	canonical() compares a stream by its dictionary less /Filter, /DecodeParms and /Length, plus its payload. A Flate JBIG2Globals, an outline /JS stream and a stream in a named destination are now accepted, and refused when their data changes	924b28b
R55-03	png_flate returns None unless Image.frombytes(...).tobytes() gives back the samples. A 509-wide 1-bit image with its pad bits set is left as it is	924b28b
R55-04	The code now does what the docstring says. Decode warnings are counted in the image branch too, and a wrapped JPEG is unwrapped only through inflate_whole (checksum right, nothing after the end)	924b28b
R55-05	Skipped by policy: /AcroForm SigFlags bit 1, or a /Perms entry	924b28b
R55-06	File mode chmods to the target's mode, or 0666 less the umask for a new file. The 7 e2e books are now 0644	924b28b
R55-07	A summary line, with refusals counted by check and policy skips by reason. Exit 1 when a check refused every PDF that policy did not skip (the repo's "exit 1 means nothing was written")	924b28b
R55-08	The suite is in the pytest step of python-tests.yaml, and CI also installs jbig2dec. The brotli-CLI test calls pytest.skip under pytest	6916e49, 924b28b

Notes on four of the fixes:

• R55-04, cut adler32:
  • On a Flate image, qpdf warns, so the image is left as it is. Kept encoded, it then fails the syntax check, and the PDF is refused.
  • On a Flate-wrapped JPEG, the JPEG is no longer unwrapped. qpdf's syntax check accepts a cut checksum ahead of a DCT filter, so that PDF is rewritten with this JPEG left wrapped.
  • A JPEG with bytes after its Flate end is left wrapped too.
• R55-07, file mode: a check refusal now writes nothing and exits 1. Before, it copied the input. A policy skip or a not-smaller result still copies the input and exits 0.
• R55-08, JBIG2: pip's pikepdf decodes JBIG2 through a separate jbig2dec binary, which is why CI now installs it. Without jbig2dec, the JBIG2 test skips through pytest, and the step refuses a PDF that has a JBIG2 image ("it cannot be rewritten").
• Your "Checked and sound" item on the invalid-kid counts: it also corrects my first PR body. That said the original JavaJavaJava's --check reported its name trees differently. It doesn't: it reports the same 44 "invalid kid" warnings, which my summary of its 17,175 warnings had hidden.

Proof:

• Tests: 74, run under pytest and as a plain script. CI's exact step ran locally (Python 3.11, pip pikepdf 10.17.0, jbig2dec): 326 passed. Without jbig2dec, the JBIG2 test skips.
• Fail without the fix: every new test fails, for the reason it is named for, when its fix is reverted on a copy of scripts/. For R55-08, with shutil.which("brotli") returning None, the dictionary test reports "skipped" with the fix and "passed" without it.
• Mutants: 75 entries, 0 surviving.
  • My first-round 34 are rerun, with snippets updated where the code moved.
  • Your 23 are reconstructed from the review: V01-V14 are the 14 you listed as surviving; V15-V23 are my guesses at the other 9.
  • 18 are reverts and mutants of this round's fixes, for example: SigFlags & 2, a new file at 0644, policy skips counted as candidates, encrypted counted as a refusal.
  • Three tests no entry targets: the bare JPEG left byte for byte, destinations and outline kept, and the process pool's equality. They are invariant checks.
• Real data, rerun at 6916e49:
  • The 7 books, rewritten again from addons origin/main (6d14aa7), are byte-identical to /home/david/adfa-6629-e2e/books/.
  • On a fresh copy of the item-B database (4c2005de), the run rewrites the same 6 rows, 821,335 stored bytes in total, and verify() passes for each. Re-running dfc064e's rewrite_pdf on the same 6 PDFs gives the same PDF and the same stored row byte for byte.
  • The summary reads 6 rewritten; left as they are: 9 not smaller. (exit 0), and a second run reports 0 rewritten; left as they are: 15 not smaller. (exit 0).
  • The copies are deleted.