Repository navigation
refactor(photos): bake histograms in zenc instead of Pillow - #373
Merged
Merged
Conversation
`zenc histogram --root www` replaces photo-histograms.py, sharing the JPEG decoder the encoder crate already links. The bake stops being conditional on an optional dependency: add-photos.sh had an `if python3 -c "import PIL"` branch whose else-arm skipped the bake entirely, and $ZENC is built by the same script that needs it. Three things had to match exactly and two are not what you would write fresh. Luminance is Pillow's 16-bit fixed point, `(R*19595 + G*38470 + B*7471 + 0x8000) >> 16`, not the `/1000` form its own "ITU-R 601-2" wording implies: measured over 360,000 pixels, the fixed-point form mismatches 0 and the /1000 form mismatches 169,740. Normalisation rounds half to EVEN, because python's round() is banker's. And the JSON keeps its original key order, which serde_json's preserve_order buys and which is what makes a re-bake a no-op diff. The decoders are not bit-identical and that is the one visible cost. zune-jpeg (image 0.25) and libjpeg-turbo (Pillow) disagree on 0.074% of pixels by one level, which moves 6.76% of bins: 100 of 158 photos shift by at most 1 on a 0..100 scale, 6 are unchanged, and the worst single bin moves 7. On a 40px bar that is 0.4px typical and 2.8px worst, on one bar of 64. jpeg-decoder was tried as a closer libjpeg port and is worse (0.19-0.33%), so exact parity would mean linking libjpeg-turbo from Rust. Isolated by construction: python's own algorithm run on zune's pixels reproduces zenc's output exactly, so the decoder is the sole difference. 152 meta files are re-baked here. Two claims from the dependency teardown were wrong and are corrected rather than repeated. This does NOT retire python3, pip, or the pip dependabot ecosystem: gen-pixel-peeper.py still needs Pillow, and add-photos.sh still calls gen-alt-text.py and build-recipes.py. And speed is not the argument, at 0.33s against python's 0.51s. What it does buy is one fewer prerequisite on the core photo path, an unconditional bake, and a `--check` mode python never had. photo-pipeline.yml no longer pip-installs requirements.txt, verified by import rather than assumed: neither python script that workflow runs imports PIL, and it never runs gen-pixel-peeper.py. No content-hashed asset moved; the tooltip.js comment edit is free because oxc-minify strips it. Built-byte diff is the 152 meta files plus the pipeline files edited here.
Contributor
Wire-size diff vs merge base
Brotli q11, the quality the edge serves. Every number here is deterministic: Client assetsNo change. 20 files, 74.02 KiB Brotli total. PagesNo change. 48 files, 496.53 KiB Brotli total. Worker bundleNo change. 284.56 KiB gzip. Dictionary deltas (
|
This was referenced Aug 14, 2026
oddharsh
added a commit
that referenced
this pull request
Aug 14, 2026
…rules (#389) #388 documented lens-reader, zenc's Cargo pins, cal and cf-garage, and deliberately stated no versions for them: the checker read the root package.json and requirements.txt only, so a number written there would have been an unchecked copy. This is the follow-up that note named, so those numbers can now be written. The root's four loops are extracted into auditManifest() and called once per manifest rather than reimplemented, which is the trap #386 had just finished cleaning up after: two checks over one file, the weaker one reading as coverage that was not there. The root's message strings are preserved byte-for-byte, because the negative tests assert on them and a refactor that quietly reworded them would assert less while still passing. All three pre-existing tests pass unchanged. Four of the nine sub-manifest dependencies are exempted rather than stated, and the split is the useful half: cal and cf-garage caret-range everything, and Cargo reads a bare "0.25" as a caret range. A range is a version the prose cannot honestly state. THREE REAL DEFECTS FOUND WHILE BUILDING IT, none by me: - serde_json arrived in zenc with the histogram bake in #373 and nothing documented it. The reverse direction caught it on its first run over that manifest, the second time that direction has found a genuine gap. - The claim pattern required a bare `name 1.2.3`, but this doc writes package names as `code`. Every sub-manifest entry read as undocumented while the sentences stating them sat in the file. The pattern takes an optional backtick now, and removing it again fails both the new test and the live one. - oxlint caught `\Z` in the Cargo section regex. JavaScript has no \Z, so that arm matched a literal "Z" and the table could only be found when another section followed it. It passed against the real file purely because [profile.release] sits after [dependencies], and would have returned an empty set, silently, for a manifest whose dependencies came last. Fixed, and pinned by a test for exactly that shape. Also fixes an ordering bug of my own: the floor was judged before sub-manifest claims were gathered, so 8 root claims were measured against a floor of 12. Claims are collected across all five manifests first, since the floor is a scanner-broke tripwire and has to see the whole scan. A manifest that cannot be READ is reported rather than audited as {}, because an empty pin set scans clean and asserts nothing, which is how a moved file would read as a healthy project forever. Co-authored-by: Aadharsh Pannirselvam <19518661+oddharsh@users.noreply.github.com>
oddharsh
added a commit
that referenced
this pull request
Aug 24, 2026
…was never wrong (#566) * fix(photos): the two scripts that kept going now stop Gotcha 40 records `zenc histogram ... | tail -1` swallowing a status inside add-photos.sh, and #560 added that of 11 committed shell scripts, 9 carry `set -euo pipefail` and 2 carry a bare `set -e`. Those 2 are the photo pipeline. This closes the gap and adds the check that keeps a 12th from reopening it. The zenc call is no longer piped at all. zenc writes one summary line to stderr on the happy path, so `tail -1` was buying no quiet while costing both the status and the per-stem warnings that name WHICH stem was skipped. Measured on the real binary: exit 2 run bare, 0 through the pipe, 2 through the pipe under pipefail. Adopting -u cost one real fix, and it is the reason not to turn -u on blind. `"${META_MODE[@]}"` on an EMPTY array is an unbound variable under bash 3.2, which is the bash macOS ships and therefore the one `env bash` finds here, and that expansion sits on the DEFAULT path (META_MODE is only non-empty under REMOTE_RENDER_ONLY). Worse than a crash: measured in the real script with -u and pipefail both on, the expansion died inside a `| tail -1` pipeline and the pipeline still reported 0, so `set -e` never fired and the run continued with the metadata regeneration silently skipped. A normal non-zero child exit propagates through that same pipe correctly, which is why the rest of this has teeth. The expansion used is `${META_MODE+"${META_MODE[@]}"}`; `${META_MODE[@]:-}` is not a substitute, since it sends one empty-string argument the callee reads as a path. Exercised rather than reasoned about, on real photos from the source folder: both the default and REMOTE_RENDER_ONLY paths, grayscale and colour sources, and a control run of the pre-change script that produced byte-identical output and the same exit code, so pipefail surfaced no pre-existing failure in these pipes. The two remaining `| tail -1` calls are kept, because those tools print many lines and tail reads all of its input, so there is no SIGPIPE to lose. The contract test discovers scripts rather than listing them, carries a floor so a broken scanner cannot report a pass, and was proven both ways: it names the offending script and its missing options when one reverts to `set -e`, and it fails on its own floor when the scanner is broken. * fix(photos): a HEIC input and every caption run were quietly broken Both found by RUNNING the pipeline while adding pipefail, rather than by reading it, and neither is a pipefail regression: the pre-change scripts fail identically. add-car-photo.sh accepted "any format sips reads (JPG/PNG/HEIC/WEBP/AVIF)" in its own usage line and could only ever process a JPEG. `sips -Z` RESIZES and does not convert, so a .HIF came out still HEIC under a .jpg name and zenc refused it with "Illegal start bytes". Every step there is >/dev/null, so the script just exited 1 with nothing said. add-photos.sh has always passed `-s format jpeg`; this one never did. Verified byte-identical output for a JPEG source, so no committed car photo moves, and exit 0 instead of 1 for a .HIF, checked across grayscale, colour and HEIF inputs. gen-alt-text.py is a NINTH instance of gotcha 40's path class, and the first one in Python. ROOT was `dirname(dirname(__file__))`, which was the served tree at www/scripts/ and became `tools/` after #452/#453, so it read tools/images/metadata.json. Its caller is `python3 ... || echo "captions incomplete"`, which excuses the traceback by design, so every photo add since the move captioned nothing and carried on. It is anchored on the tree now (`../../..` plus `public`), which is what gotcha 40 itself recommends over counting directories. Confirmed by running it: 158 photos, 158 already done, exit 0, alt.json unchanged and no inference calls spent. * docs(gotcha-40): the exit code it blames was always correct Three corrections and one addition to gotcha 40, all measured 2026-08-24. The entry says `zenc histogram` prints its error AND exits 0, so two things made the bake silent. Only one did. It returns 2 there and has done since the day it was written (#373), so the `| tail -1` was the whole mechanism and the repair was entirely in the shell. Kept as a visible correction rather than a quiet edit, because blaming the tool sends the next reader to patch an exit code that is already right, which is the more expensive mistake and the harder one to notice finishing. The 9-versus-2 split #560 recorded is gone: all 11 scripts carry `set -euo pipefail`, and a contract test fails on a 12th that does not. Adopting -u alongside it is written up as the trap it turned out to be. On bash 3.2 an empty `"${arr[@]}"` is an unbound variable, and inside a `| tail -1` pipeline that death was measured NOT propagating: pipefail on, pipeline reports 0, `set -e` never fires, step silently skipped. So -u is not a free consistency win on a script that pipes, and the safe expansion is named along with the `:-` spelling that looks like it and is not. The site table gains a ninth row. `gen-alt-text.py` had the same broken path arithmetic in Python, which is why the shell sweep that caught the other eight could not see it. --------- Co-authored-by: Aadharsh Pannirselvam <19518661+oddharsh@users.noreply.github.com>
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.
zenc histogram --root wwwreplacesphoto-histograms.py, sharing the JPEGdecoder the encoder crate already links. Follows the dependency teardown, with
two of that teardown's claims corrected below.
What it actually buys, and what it does not
Two things I claimed in the teardown were wrong, checked here rather than
repeated:
gen-pixel-peeper.pyimports Pillow, andadd-photos.shstill callsgen-alt-text.pyandbuild-recipes.py.requirements.txtstays, repurposedand re-commented.
What it does buy:
add-photos.shhad anif python3 -c "import PIL"branch whose else-arm printed a warning and skipped the bake;$ZENCisbuilt by the same script that needs it.
--checkmode Python never had, which compares committed values against whatthe shipped bytes reproduce and writes nothing.
photo-pipeline.ymlstops pip-installingrequirements.txt. Verified byimport, not assumed: neither Python script that workflow runs imports PIL, and
it never runs
gen-pixel-peeper.py.Getting it byte-exact
Three things had to match, and two are not what you would write fresh.
Luminance is Pillow's 16-bit fixed point, not the
/1000form its own"ITU-R 601-2" docstring implies. Measured over one 360,000-pixel photo:
(R*19595 + G*38470 + B*7471 + 0x8000) >> 16(R*299 + G*587 + B*114 + 500) / 1000(R*299 + G*587 + B*114) / 1000I had guessed the third one. It passed my first test only because that photo was
monochrome, where two independent errors cancel. Pick a chromatic test case.
Normalisation rounds half to EVEN, because the Python was
int(round(x))andPython 3's
round()is banker's.round_ties_even, neverround.Key order is preserved via serde_json's
preserve_order, which is what makesa re-bake a no-op diff rather than a 158-file reshuffle.
ensure_asciiishandled too, though no meta file needs it today.
The one visible cost: 152 re-baked files
The decoders are not bit-identical. zune-jpeg (image 0.25) and libjpeg-turbo
(Pillow) disagree on 0.074% of pixels by one level, which moves 6.76% of bins:
On a 40px bar that is 0.4px typical and 2.8px worst, on one bar of 64.
Isolated by construction rather than assumed: Python's own algorithm run over
zune's decoded pixels reproduces zenc's output exactly, so every layer I control
is identical and the decoder is the sole difference.
jpeg-decoderwas tried as acloser libjpeg port and is worse (0.19–0.33%), so exact parity would mean linking
libjpeg-turbo from Rust. Owner chose the re-bake over that C dependency.
Worth noting the values were always decoder-dependent. A Pillow upgrade would
have produced the same surprise diff silently, since
check-photo-pipeline.mjsvalidates structure (4 channels × 64 bins) and never values.
Verification
zenc histogram --checkpasses on all 158 after the re-bake, and a second bakeis a no-op, which is the idempotence property
add-photos.shrelies on./a/content-hashed asset moved. Thetooltip.jscomment edit is freebecause oxc-minify strips it (gotcha 35).
crate files, and the readable
tooltip.src.jstwin.www/scriptsis.assetsignored, so none of the Rust source is publicly served.routes:check.
CI has no Rust toolchain in
validate, so--checkstays a local tool; per theowner's call the pipeline check remains structural.