Serve the Pocket Planetarium from the vendored runtime (RFC 0003) - #6
Merged
Conversation
added 2 commits
August 18, 2026 07:11
The beta's copy of the runtime loaded React, ReactDOM and Babel from unpkg while every other page loaded the same three bundles from vendor/. That made pocket-planetarium.html the only page needing the network to boot, kept it outside SITE-001 and SITE-008, and kept it out of the regression suite. The vendored files are byte-for-byte the artefacts those URLs serve: React 18.3.1, ReactDOM 18.3.1 and @babel/standalone 7.29.0, each with a SHA-384 digest already equal to the SRI constant the beta declared. So only the three URL constants change; the digests stay as they were. Rewriting them would have been the risk, since a wrong digest renders the page blank with no error. Both runtime copies are now byte-identical. Also closes the gap that let the deviation hide. SITE-001 reads <script src> out of the HTML, and the beta's one script tag is a local reference, so the remote URLs sitting in variables inside that file passed the static gate. New SITE-019 reads the runtime sources themselves, and SITE-016 now covers every copy of the runtime rather than support.js alone. - docs/rfcs/0003-vendored-runtime-for-the-beta.md: the proposal - docs/specs/site.spec.md: add SITE-019, amend SITE-016, delete deviation D-1 - tests/validation/documents.test.js: cover both requirements - tests/support/pages.js: page moves into PAGES, CDN_DEPENDENT_PAGES retired - tests/regression/invariants.spec.js: cite D-2, not the deleted D-1 - README, CHANGELOG, SDLC, landing.spec: drop the stale CDN exception
The test held the unawaited page.evaluate() promise that installs the slidechange listener, then pressed ArrowRight. Installing the listener needs a round trip into the page, so the keypress raced it. Under parallel workers the press sometimes won: the event fired with nobody listening, the promise never settled, and the test failed on the 30s timeout. It looked like a deck bug and was not one. Awaiting the install, then reading the result back afterwards, removes the race. Stressed at --repeat-each=20 --workers=6: before, 4 of 60 runs failed; after, 60 of 60 pass.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…ild hazard Three points raised reviewing this branch. Hash each vendored bundle once rather than once per runtime. The digests do not depend on which runtime declares them, so computing them inside the runtime loop re-hashed 3MB of babel.min.js for every entry in RUNTIMES. Derive VENDORED from BUNDLE_FILES. They listed the same three paths independently, so a future rename could update one and leave SITE-009 checking a different set of files from SITE-016 and SITE-019, silently. Record the rebuild hazard in the RFC. Both runtimes are marked "GENERATED from dc-runtime/src/*.ts - do not edit" and dc-runtime is not in this repository, so SITE-019 now enforces a hand-edit that a legitimate rebuild would undo, with no way to fix it here. The root support.js already carried vendor/ paths under the same header, so this is existing practice rather than something new, but it was not written down and the honest fix is upstream.
This was referenced Aug 18, 2026
The RFC described the problem and named the upstream fix but left it with nowhere to go. Issue #9 tracks it.
Protonmatter
added a commit
that referenced
this pull request
Aug 18, 2026
Independent of #6 and #7. Targets `main` directly. ## What `SITE-013` asks every page to declare a meta description. **Two of eight did.** It was the last requirement in the specification with no test, and a test written against the old state would have failed on six pages, which is presumably why nobody wrote one. | Page | Before | After | | --- | --- | --- | | `index.html`, `beta.html` | had one | unchanged | | `solar-system.dc.html` | none | 157 chars | | `junocam-explorer.dc.html` | none | 137 chars | | `junocam-deck.dc.html` | none | 132 chars | | `pocket-planetarium.html` | none | 132 chars | | `the-moons.dc.html` | none | 127 chars | | `junocam-poster.dc.html` | none | 118 chars | The copy is drawn from what the README already says each page contains, so it describes the real thing rather than inventing a pitch. ## The test Checks presence, and rejects anything under 40 characters. A description short enough to be a placeholder passes a presence check while telling a search result or a shared link nothing. The shortest genuine one here is 118 characters, so the floor has plenty of room and is not tuned to the current content. Confirmed it fails when a description is removed, naming the offending page. ## Checked the descriptions actually reach the browser The `.dc.html` pages hold their head content in a `<helmet>` block that the runtime rewrites into the document on boot, so a static `<meta>` in the head could plausibly have been stripped or duplicated. It is not: exactly one survives on each page, with the right content. ``` /solar-system.dc.html {"count":1,"content":"Thirteen slides on the Sun, the eight planets and "} /junocam-poster.dc.html {"count":1,"content":"A printable 18 by 24 inch sheet on JunoCam at Jupi"} /pocket-planetarium.html {"count":1,"content":"The Pocket Planetarium prototype: a scroll-driven "} ``` ## Result - `npm test`: 22/22 validation, traceability OK, 104/104 browser, 0 failures - `SITE-013` no longer appears in the traceability report as untested With #7 merged, the report will have **no uncovered requirements at any level**. ## The level stays SHOULD Every page now satisfies it and it is covered, so it is no longer an unkept promise. Raising it to MUST would change what the gate enforces, which [RFC 0004](docs/rfcs/0004-enforcing-the-phone-viewport-requirement.md) establishes needs its own proposal. Say the word and I will write it.
Protonmatter
added a commit
that referenced
this pull request
Aug 18, 2026
Follow-up to #6. **Stacked on `topic/beta-vendored-runtime`**, since it edits the same `BETA-005` paragraph #6 touches. GitHub will retarget this to `main` when #6 merges. ## Why now `BETA-005` requires the Pocket Planetarium to render its own content at an iPhone-sized viewport. It was advisory only because the prototype loaded its runtime from unpkg, and a mandatory requirement would have made the build gate depend on that service being reachable. [RFC 0003](docs/rfcs/0003-vendored-runtime-for-the-beta.md) removed the dependency, so the absence of a test was the only thing still holding the level down. ## The test Opens the prototype at 390x844 and asserts it: 1. renders its own content, not just a booted shell 2. draws nothing wider than the screen it is on 3. does not scroll sideways Assertions 2 and 3 are what catch a broken layout. On a desktop the prototype draws a 402x874 device frame on a backdrop; below 560px it drops the frame and becomes the device. A frame left at its fixed width on a 390px screen spills past the edge, and **no existing test ran at a phone viewport**, so nothing would have caught that. ## Both assertions verified against a broken build A passing test proves little on its own. Both were run against a build with `window.matchMedia` stubbed to report no match, so the frame keeps its desktop width: | | adapts correctly | mode defeated | | --- | --- | --- | | elements wider than viewport | 0 | **40** | | document scrolls sideways | no | **yes** | Both assertions fail in the broken state, which is the only evidence either is worth keeping. An earlier revision of this PR asserted on the host element's width and claimed that was the discriminating check. It was not: the host is a full-width backdrop, so its width tracks the viewport in both modes and stayed at 390px while the layout was broken. The justification given for it, that the assertion failed at a 600px viewport, was also bogus, since it failed only because the viewport was wider than the hardcoded 390. Corrected in `7717226`. ## Result - `npm test`: 22/22 validation, traceability OK, **110/110** browser, 0 failures - `npm run test:trace` no longer lists `BETA-005` as untested. Only `SITE-013` (SHOULD) remains. ## On process Treated as routine under [RFC 0001](docs/rfcs/0001-rfc-process.md), which lists "a test added for behaviour that is already specified" as routine. The requirement text is unchanged; only its level moves, and the reason it was advisory was recorded as void when RFC 0003 landed. Raising a level does tighten what CI enforces, so if you read that as substantive I will write it up as RFC 0004 instead. ## Also `tests/functional/landing.spec.js` opened with a comment saying its pages need no readiness handling because they use no runtime. That is no longer true of this one test, so the comment now says so.
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
1 participant
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.
Implements RFC 0003 and resolves deviation D-1.
What
pocket-planetarium.htmlwas the only page on the site that needed the network to boot. Its runtime, a separate copy atbeta/pocket-planetarium/support.js, loaded React, ReactDOM and Babel from unpkg while every other page loaded the same three bundles fromvendor/. This points it atvendor/.The bundles are the same bytes
vendor/react.production.min.jsreact@18.3.1vendor/react-dom.production.min.js18.3.1-next-f1338f8080-20240426, the real 18.3.1 UMD buildreact-dom@18.3.1vendor/babel.min.js@babel/standalone@7.29.0The decisive check is not the version strings: the SHA-384 of each vendored file already equalled the SRI constant the beta declared. A SHA-384 match is byte equality, so the vendored copies are the unpkg artefacts.
The SRI constants are therefore unchanged. Only the three URL strings move. Rewriting six digests by hand to arrive at the same values would only create a chance to typo one, and a wrong digest renders the page blank with no console error and an HTTP 200. The two runtime copies are now byte-identical.
Closing the gap that let D-1 hide
SITE-001is enforced by reading<script src>out of the HTML.pocket-planetarium.htmldeclares exactly one script, a local reference, so it passedSITE-001statically. The remote URLs were string constants inside that.jsfile, where nothing looked.Deleting D-1 without closing that hole would leave it open for the next copy of the runtime. So:
SITE-019(new): every copy of the runtime resolves its bundle URLs tovendor/rather than a remote origin.SITE-016(amended): now covers every copy of the runtime, notsupport.jsalone.Both were confirmed to fail red on the old code before implementing.
Verification
npm test: 22/22 validation, traceability OK, 109/109 browser, 0 failures.PAGES, so it is now covered bySITE-006,SITE-007,SITE-008,SITE-017andSITE-018. All five pass.Also included
A fix for the flaky
DECK-012test. It held the unawaitedpage.evaluate()promise that installs theslidechangelistener, then pressed ArrowRight, so the keypress raced the round trip that registers the listener. Under parallel workers the press sometimes won and the promise never settled, failing on the 30s timeout. It looked like a deck bug and was not one. Stressed at--repeat-each=20 --workers=6: before, 4 of 60 runs failed; after, 60 of 60 pass. It was the only instance of that pattern in the suite.A stale reference.
tests/regression/invariants.spec.jscited "deviation (D-1)" for the template-placeholder issue, which is D-2. Already wrong, and deleting D-1 made it point at nothing.BETA-005's rationale inlanding.spec.mdjustified its SHOULD level by the unpkg dependency. I corrected the reasoning but left the level alone, since raising it needs a viewport test that does not exist.Notes for review
CDN_DEPENDENT_PAGESis removed rather than emptied, as nothing imported it.