The two review findings from #93. They were pushed to that branch minutes after it merged, so the merge closed above them — main currently has the hero artwork with neither fix.
Verified against main: package.json has no sharp, and Illustration.tsx has no band-JPEG <source>.
The fallback path was not art-directed
<img src> cannot vary by viewport, so it was always the wide desktop crop. A browser taking neither AVIF nor WebP therefore fell through to the desktop frame on a phone and lost the schoolhouse — the exact failure the two-crop <picture> exists to prevent, surviving in the one path nobody looks at.
The tell was already in the build script: it emitted hero-band-700.jpg and nothing referenced it. That file now backs a <source media>, placed after the modern formats so they still win wherever supported, and with no type so it matches anywhere the media query does.
Verified by stripping the AVIF and WebP <source>s at runtime and letting <picture> re-resolve — which is what an old browser actually sees:
viewport
modern formats
without them
phone (390)
hero-band-500.avif
hero-band-700.jpg — band crop, school kept
desktop (1440)
hero-wide-1672.avif
hero-wide-1200.jpg
Before this change the phone row fell to hero-wide-1200.jpg.
sharp was undeclared
It arrives transitively from next@16.1.6:
nextjs-app@0.1.0
└─┬ next@16.1.6
└── sharp@0.34.5
So node scripts/build-hero-images.js works today and breaks on a Next upgrade or a clean install that resolves differently — a documented command that silently stops working. Now in devDependencies, for the same reason next.config.js already declares its traced font files rather than trusting the tracer to keep finding them.
Not changed
The review's third finding — that the hero's licence is marked unconfirmed in CREDITS.md while the artwork ships — is accurate and deliberate. Recording it as unknown is the point of the file; resolving it is the owner's call, not a code change.
Verification
tsc clean, 159/159 unit tests, build green — re-run after the cherry-pick onto current main.
The two review findings from #93. They were pushed to that branch minutes after it merged, so the merge closed above them — main currently has the hero artwork with neither fix.
Verified against main: `package.json` has no `sharp`, and `Illustration.tsx` has no band-JPEG `<source>`.
## The fallback path was not art-directed
`<img src>` cannot vary by viewport, so it was always the wide desktop crop. A browser taking neither AVIF nor WebP therefore fell through to the desktop frame **on a phone** and lost the schoolhouse — the exact failure the two-crop `<picture>` exists to prevent, surviving in the one path nobody looks at.
The tell was already in the build script: it emitted `hero-band-700.jpg` and nothing referenced it. That file now backs a `<source media>`, placed *after* the modern formats so they still win wherever supported, and with no `type` so it matches anywhere the media query does.
Verified by stripping the AVIF and WebP `<source>`s at runtime and letting `<picture>` re-resolve — which is what an old browser actually sees:
| viewport | modern formats | without them |
|---|---|---|
| phone (390) | `hero-band-500.avif` | **`hero-band-700.jpg`** — band crop, school kept |
| desktop (1440) | `hero-wide-1672.avif` | `hero-wide-1200.jpg` |
Before this change the phone row fell to `hero-wide-1200.jpg`.
## sharp was undeclared
It arrives transitively from `next@16.1.6`:
```
nextjs-app@0.1.0
└─┬ next@16.1.6
└── sharp@0.34.5
```
So `node scripts/build-hero-images.js` works today and breaks on a Next upgrade or a clean install that resolves differently — a documented command that silently stops working. Now in `devDependencies`, for the same reason `next.config.js` already declares its traced font files rather than trusting the tracer to keep finding them.
## Not changed
The review's third finding — that the hero's licence is marked unconfirmed in `CREDITS.md` while the artwork ships — is accurate and deliberate. Recording it as unknown is the point of the file; resolving it is the owner's call, not a code change.
## Verification
`tsc` clean, 159/159 unit tests, build green — re-run after the cherry-pick onto current main.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Two findings from review on #93, both verified before fixing.
<img src> cannot vary by viewport, so it was always the wide desktop crop. A
browser taking neither AVIF nor WebP therefore fell through to the desktop
frame on a phone and lost the schoolhouse — the exact failure the two-crop
<picture> exists to prevent, surviving in the one path nobody looks at. The
band JPEG the build script already emitted was never referenced, which was the
tell. It now backs a <source media> placed after the modern formats, so they
still win wherever they are supported.
Verified by stripping the AVIF and WebP <source>s at runtime and letting
<picture> re-resolve, which is what an old browser actually sees:
phone hero-band-500.avif → hero-band-700.jpg (band crop, school kept)
desktop hero-wide-1672.avif → hero-wide-1200.jpg
sharp was not declared: it arrives transitively from next@16.1.6, so the
documented regeneration command works today and breaks on a Next upgrade or a
clean install that resolves differently. Declared in devDependencies for the
same reason next.config.js already declares its traced font files rather than
trusting the tracer to keep finding them.
The third finding — that the hero's licence is marked unconfirmed in
CREDITS.md while the artwork ships — is accurate and deliberate. It is the
owner's to answer; recording it as unknown is the point of the file.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This PR fixes a real bug where old browsers without AVIF/WebP support would fall back to the desktop-crop image on mobile, losing the schoolhouse illustration, by adding a JPEG for the band crop. It also promotes sharp from an implicit transitive dependency to a declared devDependency so the hero image build script doesn't silently break on a Next.js upgrade. The new is correctly ordered (after the typed AVIF/WebP band sources, before the wide sources) so modern browsers still get optimized formats, the referenced hero-band-700.jpg is generated by the existing build script and is committed, and the package-lock.json changes are internally consistent with the devDependency promotion.
✅ No issues found.
## 🤖 AI Code Review (Claude Code)
This PR fixes a real bug where old browsers without AVIF/WebP support would fall back to the desktop-crop image on mobile, losing the schoolhouse illustration, by adding a JPEG <source> for the band crop. It also promotes sharp from an implicit transitive dependency to a declared devDependency so the hero image build script doesn't silently break on a Next.js upgrade. The new <source> is correctly ordered (after the typed AVIF/WebP band sources, before the wide sources) so modern browsers still get optimized formats, the referenced hero-band-700.jpg is generated by the existing build script and is committed, and the package-lock.json changes are internally consistent with the devDependency promotion.
✅ No issues found.
At 0.52 the scene was legible but heavily suppressed; 0.75 lets the hills,
path and schoolhouse read while the white H1 stays comfortably the brightest
thing on the panel.
Re-measured rather than assumed, because in the dark theme the text is light
and the artwork is behind it — brightening the image lowers text contrast
rather than raising it. Off rendered pixels, sampling background up to 120px
past each line's right edge:
brightness title body
0.52 11.01:1 6.44:1
0.75 9.48:1 5.21:1
Both still clear the 4.5:1 floor, body being the binding one. The trade is
recorded next to the value so the next person to reach for it knows it has a
floor and not just a taste range.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tudor
merged commit 7f4dfa2748 into main2026-08-14 21:28:14 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
The two review findings from #93. They were pushed to that branch minutes after it merged, so the merge closed above them — main currently has the hero artwork with neither fix.
Verified against main:
package.jsonhas nosharp, andIllustration.tsxhas no band-JPEG<source>.The fallback path was not art-directed
<img src>cannot vary by viewport, so it was always the wide desktop crop. A browser taking neither AVIF nor WebP therefore fell through to the desktop frame on a phone and lost the schoolhouse — the exact failure the two-crop<picture>exists to prevent, surviving in the one path nobody looks at.The tell was already in the build script: it emitted
hero-band-700.jpgand nothing referenced it. That file now backs a<source media>, placed after the modern formats so they still win wherever supported, and with notypeso it matches anywhere the media query does.Verified by stripping the AVIF and WebP
<source>s at runtime and letting<picture>re-resolve — which is what an old browser actually sees:hero-band-500.avifhero-band-700.jpg— band crop, school kepthero-wide-1672.avifhero-wide-1200.jpgBefore this change the phone row fell to
hero-wide-1200.jpg.sharp was undeclared
It arrives transitively from
next@16.1.6:So
node scripts/build-hero-images.jsworks today and breaks on a Next upgrade or a clean install that resolves differently — a documented command that silently stops working. Now indevDependencies, for the same reasonnext.config.jsalready declares its traced font files rather than trusting the tracer to keep finding them.Not changed
The review's third finding — that the hero's licence is marked unconfirmed in
CREDITS.mdwhile the artwork ships — is accurate and deliberate. Recording it as unknown is the point of the file; resolving it is the owner's call, not a code change.Verification
tscclean, 159/159 unit tests, build green — re-run after the cherry-pick onto current main.🤖 Generated with Claude Code
🤖 AI Code Review (Claude Code)
This PR fixes a real bug where old browsers without AVIF/WebP support would fall back to the desktop-crop image on mobile, losing the schoolhouse illustration, by adding a JPEG for the band crop. It also promotes sharp from an implicit transitive dependency to a declared devDependency so the hero image build script doesn't silently break on a Next.js upgrade. The new is correctly ordered (after the typed AVIF/WebP band sources, before the wide sources) so modern browsers still get optimized formats, the referenced hero-band-700.jpg is generated by the existing build script and is committed, and the package-lock.json changes are internally consistent with the devDependency promotion.
✅ No issues found.