Codex/optimization roadmap - #52
Merged
Merged
Conversation
R13. The audit named the Writing Insights exclusion at eslint.config.mjs:12. The
gap was wider: `eslint .` reached 131 files and applied exactly 8 rules to each,
all eslint-plugin-astro deprecated-API checks. Flat config claims no .ts file
unless a block names it, so every TypeScript file in src/ was skipped silently
("File ignored because no matching configuration was supplied"), and .js/.mjs
files got the Astro rules and no JavaScript rules at all. A green lint run
established almost nothing.
The config now covers 214 files with a correctness rule set on .js, .mjs, .cjs,
.ts, and .astro, and no source directory is excluded. Dropping the Writing
Insights exclusion surfaced one real defect: an unescaped `>` in
insights.astro:621 that the Astro compiler tolerates but astro-eslint-parser
cannot parse. Rendered output is unchanged.
Unused symbols are split by language. Core no-unused-vars cannot read TypeScript
type positions and reported all 45 parameter names in function types as unused
arguments, with no true positives; it now runs on plain JavaScript only, where
TypeScript does not look. noUnusedLocals and noUnusedParameters are enabled in
tsconfig.json so astro check covers TypeScript. Both were already clean.
Two rules are deliberately omitted and documented in the config: no-undef, which
needs a globals list the repo does not depend on and which TypeScript already
covers, and no-unmodified-loop-condition, which cannot see a Date mutated through
setMonth and reads brainScience/data.ts as an infinite loop.
Verified by planting a deliberate violation in each file class — duplicate key in
a .ts, unused variable in an .mjs, invalid typeof comparison in public/sw.js,
duplicate case label in a Writing Insights .astro, and an unused local plus
parameter for the TypeScript half — confirming each was reported, then reverting.
Type-aware rules remain uncovered; they need typescript-eslint as a new
devDependency and are out of scope here.
Local gate: 261 tests, astro check (0/0/0), ESLint, Prettier, and build all pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
R17. Fourteen journeys drive headless Chromium against the real dist/client output: the lightbox keyboard path, search announcements and the rapid-clear regression, carousel pause including reduced motion, mobile navigation focus return, theme persistence, and EN/ES routing, at 390, 768, and 1440 pixels. The harness uses the playwright library through Vitest rather than adding @playwright/test, so the repo keeps one test runner and no new devDependency. It serves dist/client via scripts/serve-dist.mjs on an OS-assigned port. `pnpm run test:browser` is a separate command and a separate CI step after the build; it does not build, because building inside the test run would make a failure ambiguous between the build and the behavior. Acceptance was not taken on a green run. Three regressions were planted in the built output — announce() made a no-op, mouseleave clearing the reader's pause, and the lightbox's activeImage.focus() removed — and the suite run against them. The search and carousel mutations were caught. The lightbox mutation was not, which exposed a defective test: <dialog> natively restores focus to whatever was focused before showModal(), so a journey that focuses the image before opening it passes either way. A second journey now opens from a different element, and fails against that mutation. All three were reverted and the suite re-run green. This closes the R11 verification gap from the previous batch: Playwright emulates prefers-reduced-motion, so that path is covered by a journey rather than by inspection. Also fixes a real defect found while building the harness: listen() in serve-dist.mjs returned the requested port instead of the bound one, so port 0 produced http://127.0.0.1:0. Scope limits, stated rather than implied: service workers are blocked in the test context, so these say nothing about caching or offline behavior (that is R18); and a local static server does not exercise Vercel redirects, rewrites, headers, or /api/quotes, so a deployment smoke check is still owed. Local gate: 261 unit tests, 14 browser journeys (~34s), astro check (0/0/0), ESLint, Prettier, and build all pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
R16. validate-structured-data.mjs greps the source module for export names, and validate-feeds.mjs looks for substrings, so neither can say whether a layout wired those utilities correctly or whether the emitted XML is valid. The new validate-generated-content.mjs parses the emitted bytes: - JSON-LD on every representative surface parses and carries @context / @type - rss.xml is well-formed XML by a real parser. A substring check passes on an unescaped & and on an unbalanced tag, which are the faults that break readers - feed ids are unique, and RSS and JSON Feed carry the same posts - canonical URL, feed entry URL, and JSON-LD identity agree - site-local og:image targets and internal links resolve to real files - drafts, published: false, and future-dated posts appear nowhere; public English posts appear everywhere they should; Spanish twins stay out of the feeds Surfaces: homepage, /everything, /writing-insights, an English essay, a Spanish twin, and a recipe. External URLs are never fetched, so a release cannot fail because someone else's site is down. fast-xml-parser@5.3.8 becomes a direct devDependency. It was already resolved in the store at that exact version — pnpm-workspace.yaml already pins it as an override — so this links an existing package rather than pulling a new one. The rules are pure functions in scripts/lib/generated-content-checks.mjs with 25 unit tests, because the repo has no draft or future-dated post to exercise them against. vitest.config.ts now also collects scripts/**/*.test.mjs. Acceptance was checked by mutation at both levels. Against the real build: malformed JSON-LD, a duplicated feed id, an unescaped ampersand, and a deleted og:image target each exited non-zero, then were reverted. The draft rule was run end to end — a fixture built with draft: false so a page was emitted, then flipped to draft: true in source; the gate reported the page and the feed entry and exited 1. Fixture removed, site rebuilt clean. Also: a check that inspects nothing passes like one that inspects everything, so the script reports what it actually examined (24 ld+json blocks, 568 internal links, 6 og:images) and fails if any count is zero. Known limit: it does not detect a stale dist/. In CI the build step fails first, but running it by hand after a failed build can mislead. Local gate: 286 unit tests, 14 browser journeys, astro check (0/0/0), ESLint, Prettier, build, and every validator pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…listings R21. The eligibility rules existed in three places — publishFilters.ts over astro:content entries, sitemapTranslations.ts over raw frontmatter at config-load time, and the plain-JS copy R16 added for the post-build validator — and they had drifted. src/utils/publishEligibility.mjs is now the single definition. Plain JavaScript with JSDoc types on purpose: TypeScript reads the typedefs across the .mjs boundary and plain Node imports the same file, so all three consumers share one implementation with no build step. Node 24 could run a .ts module directly, but CI is pinned to 22.12.0 where type stripping is still flagged. Environment- dependent rules stay put: isCollectionListed keeps its import.meta.env.PROD dev-preview branch, and nothing in the shared module imports astro:content or Vite, so astro.config.mjs still loads. Two real defects in sitemap lastmod attribution: 1. The one the audit named. Every public post bumped every listing path, including /, /everything, /guided-path, /rss.xml and /feed.json — all English-only surfaces. This was live in the shipped sitemap: /tag/children moves from 2025-05-06T16:30 back to 2025-05-01 and /tag/digital-safety from 2025-05-01T10:30 back to 2025-05-01, because both dates came from Spanish posts those English-only tag pages never list. 258 URLs before and after; exactly those two lastmods change. 2. One the audit did not name. Category pages were bumped by recipes, although isCategoryListedPost excludes recipes from category and tag pages. The tag side already had the guard; the category side did not. The attribution logic is extracted as buildSitemapIndex(entries, options) so it can be tested against fixtures — which mattered, because the real content set demonstrates neither fix: the newest post today is English and no recipe currently leads its category. A green run over real content proves nothing here. Adds 11 tests for the shared module and 15 for cross-surface agreement, the latter running 11 fixtures (drafts, Spanish drafts, unpublished, future-dated, ['en','es'] vs ['es','en'], a date exactly equal to now, a YAML Date) through all three surfaces and asserting they agree. Search stays the documented exception: public in any language. Both fixes mutation-checked: restoring the language guard to isPublicMeta fails the Spanish-listing test; restoring the category loop for recipes fails the recipe test. Left alone deliberately: scripts/audit-frontmatter.mjs still parses frontmatter its own way. It checks required fields and applies no eligibility rules. Local gate: 312 unit tests, 14 browser journeys, astro check (0/0/0), ESLint, Prettier, build, and every validator pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
R18. The promise, set by the author and now stated at the top of public/sw.js: everything you have already opened stays readable offline — article pages, their images, the browse pages you navigated through, and the site's CSS, JS, and fonts. Anything never opened is not, and /offline.html says so. Precaching the whole site was considered and rejected on measurement. This build emits 153.5 MB across 906 files, ~118 MB of it reader-facing: 25.9 MB of article HTML over 129 posts (~205 KB each), 67.3 MB of images, ~24 MB of listing pages, ~1 MB of CSS/JS/fonts. The other 30.3 MB is /social/ Open Graph artwork no reader displays. Precaching would cost a first visit more data than most phone plans allow, take minutes, and be re-fetched on every deploy. Caching as you read gets the same practical result for a few hundred KB per article. Defects fixed, each confirmed in source and then by mutation: - Activation deleted every cache on the origin except its own current two. It now deletes only notes-prefixed caches from older versions. - The asset matcher had no avif or webp, so no article image was cached at all on a site whose images are 67.3 MB of AVIF. - Only / and /p/ were treated as pages. /everything, /category/*, /tag/*, /recipes and the static pages had no offline behaviour whatsoever. - Background revalidation was a floating promise whose cache.put was never awaited; writes are now awaited and attached to event.waitUntil. - No same-origin boundary — the extension matcher would store another origin's files. Cross-origin requests are now skipped entirely. - Unbounded growth; page/asset/image caches now have ceilings (120/120/250), trimmed FIFO, which is what the Cache API's insertion order supports. - Placeholder sync and push handlers removed. The sync handler walked the page cache deleting /api/ entries that cannot exist, since API traffic is never stored. - A dead fallback returned CSS for /styles/ and /fonts/; neither path exists. Adds 8 browser journeys for activation, upgrade, the offline promise, and what must never be stored. Four regressions were planted in the built worker — the old cache sweep, AVIF dropped from the matcher, page handling narrowed to / and /p/, and the /social/ guard removed — and each was caught, then reverted. Two testing findings baked into the harness. context.setOffline(true) emulates the page's network but not the worker's own fetch(), so three journeys were silently vacuous; the harness now has stopServing()/resumeServing() which take the origin down for real. And asserting the image cache was merely non-empty passed even with AVIF removed, because favicons are .ico and .png — the assertion had to name the format the bug was about. Local gate: 312 unit tests, 22 browser journeys, astro check (0/0/0), ESLint, Prettier, build, and every validator pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…version
The `>=22.12.0` range added in R12 had a consequence I did not anticipate and
did not flag when offering the choice. Vercel reads `engines.node` and it takes
precedence over the Node version in Project Settings, so the open range moved
production from the configured 22.x to 24.x, and Vercel warned it would keep
promoting on every future major:
Due to "engines": { "node": ">=22.12.0" } ... the Node.js Version defined in
your Project Settings ("22.x") will not apply, Node.js Version "24.x" will be
used instead.
That is the exact CI/production drift R12 set out to remove, so `22.x` now pins
the major, matching .nvmrc and CI. Both Vercel warnings go away and production
returns to the version the test suite actually runs against.
pnpm does not enforce engines without engine-strict, so a newer local Node still
installs and runs; README now says to run `nvm use` before trusting a local
result, because 24.x is no longer the declared target.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This branch was successfully deployed
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.
No description provided.