docs!: retire the three repository-owned skills into docs/, and cut the README back - #228
Conversation
Relocate the three repository-owned skills' implementation half and both deployment runbooks' procedures into docs/conventions/ and docs/operations/, and record the constraints whose rationale this repository has already written down as dated decision records. The CSS document drops ~48 of its 62 guideline bullets: react-component-styling already owns @layer/@scope, logical properties, oklch, container queries, scroll timelines, hit-area expansion, and the module-per-component pairing — including this project's own examples verbatim. What survives is the token names, the two breakpoints, the imagery filter recipe, and the four templates. Decision dates come from the commits that introduced each rationale, which required unshallowing the clone. Previews were branched from production on 2026-07-06 and reseeded from fixtures on 2026-07-18, so that pair is recorded as a supersede chain rather than a single record. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0137GC1hE7fdpadf8vHhegUd
…he README back docs/ becomes the single documentation root, in three parts: specs/ for what the product does, conventions/ for how the code is written, operations/ for how the repository is built and run, with decisions/ beside them. project-structure, visual-identity, and markdown-pipeline-development are deleted; .claude/skills/ now holds installed skills only, and skills-lock.json describes it entirely. Both deployment runbooks are decomposed rather than kept: their procedures move to operations/, and the constraints whose rationale was already written down become dated decision records. README keeps four subjects — what this is, how to start it, the command table, and where everything else lives — dropping from 450 lines to 104. CLAUDE.md routes by surface to a named document instead of to a project skill, and drops its reference to the removed when_to_use frontmatter field. Two comment references to the deleted skills are repointed: playwright.config.ts and two workflow files cited them by path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0137GC1hE7fdpadf8vHhegUd
|
@claude review Generated by Claude Code |
🚀 Preview deploymenthttps://btnopen-pr-228.vercel.app Deployed |
Code review1 important, 1 nit. This is an exceptionally well-scoped and self-verified documentation restructuring. I re-ran all five Mandatory checks:
Findings are inline. Nice work on the self-critique in the PR body — the "are the specs true?" framing is exactly where I focused verification. |
…t dropped Both are review findings against #228, and both were real gaps rather than relocations I had recorded elsewhere. The single-source-of-truth constraint on classifyLinkHref was lost entirely: grep found no mention of it anywhere under docs/, while the helper is live and called from both rehypeAllowedLinkProtocols and the Link component. Losing it is what would let someone widen one call site and leave the layers inconsistent — which converts the deliberate redundancy into a hole. It lands in a new Content Safety section beside the re-check-every-control rule, which was thin for the same reason. The content-source rule carried only the filesystem half of "filesystem or arbitrary HTTP"; the surviving prose implied the rest rather than stating it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0137GC1hE7fdpadf8vHhegUd
|
@claude review Generated by Claude Code |
🚀 Preview deploymenthttps://btnopen-pr-228.vercel.app Deployed |
There was a problem hiding this comment.
Code review
0 important, 0 nits.
Re-reviewed after 4141b6a, which addresses both findings from the prior round (classifyLinkHref's single-source-of-truth rule, and the "filesystem or arbitrary HTTP" content-source rule) — I independently verified both restorations against the live code (classifyLinkHref in app/(app)/_/helpers/link-href.ts, called from rehypeAllowedLinkProtocols and the Link component, matches exactly) and against the original skill text they were dropped from.
Mandatory checks (per REVIEW.md):
- Skill conformance — re-ran all five
living-product-specificationvalidators againstdocs/(index, references — now 147 links, glossary, decision-naming, decision-supersede); all exit 0. Confirmed.claude/skills/andskills-lock.jsoncorrespond exactly (23/23). Confirmed every link inCLAUDE.mdresolves and it no longer referenceswhen_to_use. No skill-conformance violations found. - Acceptance criteria (#218) — checked the diff against every criterion: the
docs/tree shape and document counts (2 + 6 specs + 11 conventions + 6 operations +decisions/),index.md's content and link count,overview.mdspanning both the product and the repository, the glossary's#-level product/development split, both runbooks deleted with their content underoperations/,README.mdat 104 lines,docs/operations/agent-skills.md's deviation register (both prior deviations plus theaxross/skills#315gap), and the decision records. All met.- Independently re-derived the decision-record dates from
git logafter unshallowing the clone (matches the PR's own described method) and spot-checked the2026-07-06and2026-07-08records against the actual introducing commits — both check out. - Spot-checked several spec claims directly against the code rather than trusting the relocation: the five-check comment write path and the identity-fallback chain in
specs/comments.mdagainstapp/(app)/posts/[slug]/comments/route.ts(exact match, including theauthorNamefallback order), the admin lockout and live-preview breakpoints inspecs/content-authoring.mdagainstpayload/collections/user.tsandpayload/config.ts(exact match), and theblog-postsfield table againstpayload/collections/blog-post.ts(matches). - Traced the content-safety guideline-bullet ledger for the retired
markdown-pipeline-developmentskill bullet-by-bullet: every bullet not converted to spec prose or restored in4141b6ais already covered by the installedapplication-securityskill'sinjection-and-output-encoding.md(attribute allowlisting, event-handler stripping, the raw-HTML-sink and dual-context-encoding rules), consistent with the PR's stated "owned by an installed capability" methodology.
- Independently re-derived the decision-record dates from
No new findings survived scrutiny. Nice work tightening the content-safety gap — the fix reads as a faithful restoration rather than a rewrite.
🚀 Preview deploymentTorn down — the isolated Turso database ( |
`main` retired the three repository-owned skills into `docs/` in #228, deleting every file this branch's documentation half edits. All seven conflicts are modify/delete on paths that commit removed, and they are resolved by taking the deletion: keeping them would resurrect exactly the structure #228 retired, against that change's own decision record. The content those edits carried is not lost — it is ported to the documents that replaced them in the follow-up commit. The `app/` half of this branch is untouched by #228 and merged clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01815bmfenhiXEhhjrMKpfHw
The documentation half of this branch edited three repository-owned skills that #228 retired into `docs/` while the branch was open. This ports what those edits carried to the documents that replaced them, so nothing is lost and nothing resurrects the retired structure. `specs/visual-identity.md` gains the role layer as the component-facing surface: the slot table replaces the step table, the scales become the theme's private tier, and `text.onSolid` and the Radix-derived alpha twins are described. `conventions/styling.md` drops the paragraph saying components still reference ramp steps — this change is what made that false — and its focus-ring template and token table name roles. One claim did not survive the port. The retired `visual-identity` skill carried a MUST that new colour values be `oklch()` rather than `rgb()`/`hsl()`/hex, and #228 did not carry it into `docs/`. The register entry therefore rests on the hue-knob rule, which did survive, rather than citing a project rule that no longer exists. The four surfaces whose role name does not match their local job are recorded under Recorded Exceptions, so the next reader does not "correct" a colour this change deliberately preserved. Refs #221 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01815bmfenhiXEhhjrMKpfHw
…cales and the components (#229) * feat(app): add a semantic colour-role layer over the palette scales The accent and neutral scales become the theme's private tier and are mapped once onto roles named `--color-<tier>-<scheme>-<slot>`, so a component can name what a surface is rather than which step it sampled. Each role is a fixed map onto one step, so both colour schemes come free. Three gaps the role set needs are closed at the same time. The accent scale gains step 0, matching `--neutral-0` as the extreme. Every step gains a translucent twin, whose alpha grading is the published Radix UI Colors scale — `purple` for accent, `mauve` for neutral — rather than an invented curve; Radix also solves each alpha step's colour so it composites back to the opaque step, and that half has no solution here because this scale sits on the sRGB gamut boundary from step 4 up, so the twins carry the scale's own colours. And `text.onSolid` gains a role per scheme, taking the value the comment avatar's initial hand-wrote. Nothing rendered changes: the roles are declared here and consumed in a following commit. Refs #221 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01815bmfenhiXEhhjrMKpfHw * refactor(app): repoint every component colour reference at a role All 161 references across 20 CSS Modules, `globals.css`, and the inline SVG fills in `leave-a-review.tsx` now read a role instead of a scale step, so `--accent-*` and `--neutral-*` survive only inside `variables.css`. The substitution ran one pass per token rather than file by file, which is what makes it reviewable: reversing the mapping returns every file but the four below byte-identical to its parent, modulo the line reflow the longer token names cost. Four files change by more than a rename: - the web-embed fallback tile moves onto `solid.rest-alpha` and `solid.hovered-alpha`, replacing two hand-written slash-alpha derivations. This is the one rendered colour that moves, and it moves because no alpha role reproduces the old wash. The tile appears in no snapshot — the post content snapshot masks embed cards — so it was checked by hand in both schemes. - the comment avatar's initial reads `text.accent.onSolid` instead of the literal that role now carries. - two comments named a scale token to describe a surface, and move with the code they describe. Refs #221 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01815bmfenhiXEhhjrMKpfHw * docs(skills): teach the colour-role layer instead of scale steps `visual-identity` was the skill that taught picking a colour by its numbered step, so leaving it alone would reintroduce the pattern on the next component written. The step-role table becomes a role table carrying the token names, the scale is described as the theme's private tier that a component must not reference, and the alpha twins and their Radix provenance are documented. Two call sites whose role name reads oddly — the focus ring on `component.selected` and the baseline contrast pair — are recorded as what they are rather than smoothed over, so the next reader does not "correct" a step choice this change deliberately preserved. Refs #221 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01815bmfenhiXEhhjrMKpfHw * docs(skills): record the oklch gamut-fallback departure in the register The review found that `variables.css` assigns 54 `oklch()` values to custom properties with no `@supports` sRGB fallback, which the React component styling capability makes a MUST. The finding is correct and the departure is real, but it pre-dates this branch — `main` already carried 25 unguarded declarations — so it is a standing property of the codebase rather than something this change introduced. Recording it rather than complying, because two of this repository's own MUSTs stand against the fallback: colour values must be `oklch()` rather than `rgb()`/hex, and the brand hue must stay a single global knob that 108 static literals would replace. Compliance is also all-or-nothing, since an unguarded declaration invalidates at computed-value time and takes the page's colours with it regardless of what the guarded ones do. What settles it is that the same capability mandates `@scope` with no fallback at all. `@scope` reached Baseline in December 2025 and `oklch()` in May 2023, so a browser that cannot parse the colour cannot apply a single component rule here and never reaches a token to fall back on. That inconsistency is the capability's rather than this project's, and the entry notes the upstream issue as pending the maintainer's go-ahead. Refs #221 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01815bmfenhiXEhhjrMKpfHw * docs(skills): cite the upstream issue in the gamut deviation entry The entry recorded the departure as a gap awaiting the maintainer's go-ahead for a public write on a repository this project does not own. That go-ahead was given and axross/skills#316 is filed, so the entry now names it the way the Sentry entry names its own. The issue reports what the register describes locally: the capability requires an sRGB fallback for `oklch()`, Baseline since May 2023, while mandating `@scope` with no guard at all, Baseline only since December 2025 — so a browser that cannot parse the colour cannot apply a component rule either and never reaches a token to fall back on. Refs #221 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01815bmfenhiXEhhjrMKpfHw * docs: teach the colour-role layer in docs/ instead of scale steps The documentation half of this branch edited three repository-owned skills that #228 retired into `docs/` while the branch was open. This ports what those edits carried to the documents that replaced them, so nothing is lost and nothing resurrects the retired structure. `specs/visual-identity.md` gains the role layer as the component-facing surface: the slot table replaces the step table, the scales become the theme's private tier, and `text.onSolid` and the Radix-derived alpha twins are described. `conventions/styling.md` drops the paragraph saying components still reference ramp steps — this change is what made that false — and its focus-ring template and token table name roles. One claim did not survive the port. The retired `visual-identity` skill carried a MUST that new colour values be `oklch()` rather than `rgb()`/`hsl()`/hex, and #228 did not carry it into `docs/`. The register entry therefore rests on the hue-knob rule, which did survive, rather than citing a project rule that no longer exists. The four surfaces whose role name does not match their local job are recorded under Recorded Exceptions, so the next reader does not "correct" a colour this change deliberately preserved. Refs #221 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01815bmfenhiXEhhjrMKpfHw --------- Co-authored-by: Claude <noreply@anthropic.com>
Integrates the seven commits main gained since this branch's merge base (#228, #229, #230, #231, #234, #233, #236). Merged rather than rebased: the branch carries 31 commits and several conflicting files were touched by more than one of them, so a rebase would replay each conflict once per commit. One merge resolves each once and needs no force-push. Conflict resolutions: - jest.config.cjs: deleted. #230 removed it; this branch's edits to it are obsolete under Vitest. - app/(app)/_/repositories/shared-types.ts: stays deleted. #234 deleted it on main and this branch deleted it too, so no conflict arose. - .claude/skills/project-structure/references/component-conventions.md and testing-conventions.md: accepted main's deletion. #228 retired the repository-owned skills into docs/; the rule changes relocate there. - app/(app)/layout.tsx: kept both sides. #236's AnalyticsConsentProvider, Footer, and AnalyticsConsentBanner wiring survives alongside this branch's Suspense-boundary comments, and the third bare <Suspense> #236 introduced around <Footer> gets a comment of its own. - app/(app)/_/translations/catalogs.spec.ts: took main's Vitest import with this branch's relocated relative paths. The four auto-merged files — both locale catalogs, blog-post-list/loaded.module.css, and loading-placeholder.module.css — were each read back rather than trusted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu
#228 retired the repository-owned skills into docs/, so the two rule changes this branch made against project-structure's references land here instead. react-components.md: the Loading and Loaded Split section made the orchestrator/loaded/loading triad a MUST, and the two new skeletons are standalone -loading components that depart from it. The departure rested only on a comment in each file. The section now names the shape, the condition that makes it legitimate — a route-level <Suspense> boundary owns the pending state, so no component has a loaded sibling — and the mirroring obligation it carries in place of the file adjacency a triad gets for free. testing.md: the branch's jest.doMock rule is gone rather than relocated. It described a workaround for SWC's hoisting under next/jest, and #230 removed the runner it applied to; Vite hoists vi.mock whatever the import style, so the rule has no subject. The two traps that replaced it are real and both cost time in this change: - A vi.mock factory runs while the subject is imported, before the spec module's body, so anything it assigns to needs vi.hoisted(). - Biome's noMisplacedAssertion recognizes a vitest import where it did not recognize @jest/globals, so a helper that asserts stops passing lint on migration. All five living-product-specification validators pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu
* test(config): let the unit runner transform node_modules The markdown toolchain (unified, remark-*, rehype-*, mdast-util-*, micromark*, shiki, @shikijs/*) is ESM-only, and `next/jest` ignores `node_modules` for transformation, so importing `app/(app)/_/helpers/markdown.ts` under Jest fails with "Cannot use import statement outside a module". `next/jest` only appends to `transformIgnorePatterns`, so the pattern cannot be narrowed through the config it accepts. Post-process the resolved config instead, dropping the `node_modules` entries and keeping the CSS-module one. Adding the packages to `transpilePackages` in `next.config.ts` was the alternative, but it changes the production build to solve a test-runner problem and would have to track every transitive dependency of the toolchain. Refs: #179 * test(markdown): cover renderMarkdown() through the real pipeline Render the returned element to HTML with `renderToStaticMarkup` and no component map, so the assertions describe the pipeline rather than the components `markdown.tsx` maps onto it. The real Shiki highlighter runs rather than a mock: highlighting is part of what the spec protects. Covers a fenced code block (including the `<code>` unnesting), a GFM table, an `::embed` leaf directive, the paragraph-only-link rewrite, an inline link, and the banner container directive — plus the two content-safety behaviours the pipeline exists to enforce: a blocked link protocol losing its href while keeping its text, and an unhandled `:text` / `:::fence` directive surviving as literal source. Refs: #179 * test(payload): cover the image upload helpers Cover `getStaticDir()` on both sides of the Vercel Blob branch, the webp format options, and both shapes `createPngImageSize()` accepts — the width-derived `${width}w` name and an explicit one — including the `generateImageName` callback it hands Payload. The environment barrel is faked with `jest.doMock()` plus a dynamic import rather than `jest.mock()`: SWC's Jest transform hoists a `jest.mock` call only when it is written against the bare global `jest`, and this project imports its Jest APIs from `@jest/globals`, so a hoisted mock is never registered before the subject is required. Refs: #179 * test(payload): cover the banner block contract and round-trip Cover the block slug, its type and body field contract, and the `jsx` export/import round trip through `banner-directive.ts`, modelled on the existing `embed-block.spec.ts`. `@payloadcms/richtext-lexical` is faked at the boundary: it is ESM-only and drags in Payload's upload chain, which fails to resolve under Jest, and the editor is not what this spec is about. The fake is registered with `jest.doMock()` plus a dynamic import rather than `jest.mock()`, for the hoisting reason recorded in `image.spec.ts`, and the module registry is left alone so the block and the spec share one copy of `banner-directive.ts`. Refs: #179 * test(payload): cover the MCP plugin sanitizer, normalizer, and auth Capture the configuration object `mcp.ts` hands to `mcpPlugin()` by faking the plugin package, then exercise its closures directly. That covers the same functions plus the wiring around them, and adds no export that exists only for a test. Covers the sanitized response for each configured collection and the website global, the sanitizer failure path (an explicit error payload rather than an empty response), an absent document degrading to null, the `onEvent` normalizer on a non-record event, an unknown-shaped record, and an ERROR event, and `overrideAuth` normalizing an absent user to null so the collections' access rules deny it. Refs: #179 * refactor(repositories): drop the unreferenced shared-types module `app/(app)/_/repositories/shared-types.ts` is imported nowhere: a repository-wide search for the module specifier and for each of its three exports (`User`, `Image`, `zDateTime`) returns only the file itself, and the repositories directory has no barrel that could re- export it. Speccing it would have pinned a contract no caller has, so it is removed instead. Refs: #179 * test(translations): colocate the catalog spec beside the catalogs `translations.spec.ts` sat in `_/helpers/` with no `translations.ts` beside it — its subject is the two JSON catalogs in `_/translations/`, so the name read as though the module under test had been deleted. Moving it to `_/translations/catalogs.spec.ts` satisfies the colocation rule rather than documenting an exception to it. Only the two relative JSON import paths change; the assertions are untouched. Refs: #179 * test(e2e): move the post comments tests under the slug route, split by journey `e2e/tests/routes/posts/comments.test.ts` covered `/posts/[slug]`, whose siblings all live under `e2e/tests/routes/posts/slug/`, and at 693 lines it was the suite outlier where every other route directory keeps one file per journey. It moves to `posts/slug/` and splits four ways: the section and author reply, the avatar journeys, the composer journeys (sign-in, submit, and the CSRF rejection), and the journeys where no section or composer is expected. Every helper and constant it carried is used by exactly one of the four, so nothing is duplicated. The test bodies and their `@scenario:` / `@area:` / `@priority:` tags move verbatim: the tag multiset is identical before and after (28 tags, 10 scenario ids, none renamed or dropped), no line of the original was lost, and Playwright collects the same 27 tests from the new files as from the old one. Refs: #179 * refactor(payload): drop the unreferenced createPngImageSize factory Nothing calls it: the three upload collections use getStaticDir and webpFormatOptions from this module, and cover-images writes its own og size inline. It is the same unreferenced condition that removed shared-types.ts in this change, so it goes the same way rather than gaining a spec that pins a contract no caller has. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * docs(skills): record the jest.doMock requirement in the testing conventions jest.mock() cannot work in this repository and fails silently: SWC's Jest transform hoists it only when written against the bare global jest, and these conventions mandate importing Jest APIs from @jest/globals, so the call lands below the subject's own require and the fake never registers. The installed unit-testing capability names no Jest mocking API at all, so this is a repository-specific gotcha rather than a deviation from an installed rule, and belongs here. The resetModules bullet records the opposite hazard the same specs hit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * test(hooks): suffix the loading components' inner test ids `blog-post-list/loading.tsx` and `blog-post-header/loading.tsx` reused their loaded siblings' `data-testid` values on every inner element, so a test could not tell a skeleton from real content. Both orchestrators already suffix the fallback root; this extends the same `-loading` suffix inward. No e2e test references any of these ids, so nothing needed updating alongside. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * feat(app): add the shared empty and error message surface `<StateMessage>` is the one component the empty state and the error state both render through, so the two cannot drift apart. Its composition follows `not-found-content.tsx` — copy as props, a `ComponentProps<T>` base, a spread, a CSS Module, a test hook — while staying deliberately quieter than the 404: a single muted line, no heading, no glyph, and a text-link action. The visual is option A of the #179 exhibit, chosen by @axross over the recommendation. A source comment records that the missing heading is the decision rather than an oversight. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * feat(app): map a render failure to its copy and retryability `resolveRenderFailure()` decides what a failed render says and whether a retry is offered, so no component body branches on a status code or an error class name. Its catalog keys land here with it, in both locales. The terminal branch is deliberately unmapped: the site has no permanent render failure today — a missing document calls `notFound()` and renders the 404 surface instead of an error boundary — so the kind carries the distinction while the mapping stays unconditional. A source comment records that, and the spec asserts the contract rather than a branch nothing can reach. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * feat(app): render a first-party surface for a render failure `app/(app)/error.tsx` catches a render failure anywhere under the route group and renders the shared surface from the helper's result, with the site header and locale provider still in place. The retry control is wired to React's `reset`, so it re-renders the failed region rather than reloading the document, and it appears only while the helper reports the failure retryable. The failure is reported to Sentry from the boundary, matching what `global-error.tsx` does for the root layout. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * feat(index): render the empty state inside the blog post list `BlogPostListLoaded` rendered whatever `getBlogPosts` returned, so zero posts produced a bare `<ul>`. It now renders the shared surface inside the list container instead, keeping the section heading anchored and the page from reflowing. The branch lives in the loaded component because only it knows the collection came back empty rather than pending. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * feat(index): add the main region's loading skeleton The `<Suspense>` around `<IndexPageMain>` had no fallback, so the whole intro and posts section arrived at once with nothing in its place. It now streams a full-parity skeleton: rectangles for the portrait and the social icons, text placeholders for the bio and the posts heading, and the post list's own existing skeleton nested inside. Option B of the #179 exhibit. Its stylesheet mirrors `page.module.css` rather than importing it, since a CSS Module may not read another component's; a comment records that the two move together. The JSON-LD boundary beside it gains the comment that records why it deliberately has no fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * feat(posts): add the comments region's loading skeleton The `<Suspense>` around `<MaybeComments>` had no fallback, so the bottom of a post appeared without warning. It now streams a heading placeholder plus two comment rows, each an avatar beside a meta line and a body line — option B of the #179 exhibit. `MaybeComments` renders nothing on a post with comments disabled, so the skeleton shows and then vanishes there; a comment at the boundary records that as the accepted cost of the chosen option rather than a bug. The body boundary above it, and the JSON-LD and live-preview boundaries below, gain the comments that record why each deliberately has no fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * docs(app): record the last two deliberate render decisions The layout's page-view-tracking boundary now says why it carries no fallback, and `get-tweet.ts` says that returning null on a failed fetch is the decision — the tweet card degrades to a plain link — rather than an unhandled path. The web embed's bare-URL card, the tweet card's own fallback, and the embed's null return on an invalid URL already carried theirs and are left alone. With these, every `<Suspense>` under `app/(app)` either has a fallback or states why it blocks, and every deliberate degradation is recorded in the tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * docs(app): record why the document boundary carries no fallback The root layout's own `<Suspense>` was the last bare boundary under `(app)` whose comment explained why the boundary exists without saying why it has no fallback. It now says both, so every boundary in the route group either has a fallback or states why it blocks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * fix(i18n): localize the retry label in the Japanese catalog The plan named the retry action `Try again`, and visual-identity does say error-state actions take a short English imperative — but its canonical example, `Go back home`, is the en-US string of a key whose ja-JP side reads ホームに戻る. Every action label in the catalogs is localized, so that rule describes the English copy rather than licensing an untranslated Japanese one. Bring the retry label in line with its neighbours. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * fix(i18n): use the settled Japanese retry label @axross chose 再試行 over もう一度試す while reviewing #179; the shorter form reads as the control it labels rather than as a sentence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * fix(app): root the render-failure surface in a main landmark The boundary replaces the whole page content, so a failing route was serving a document with a header and no main landmark. Wrapping in the boundary rather than adding an element-type prop to <StateMessage> keeps that component a <div>, which is what its other caller needs inside the blog post list's <li>, and matches the ComponentProps<T> anatomy every component here follows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * refactor(components): stop advertising children on the skeletons Both new loading components destructured children only to discard it, while their props types still offered it to callers. <StateMessage> and <Comments> already use Omit<..., "children"> for exactly this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * fix(posts): name the comments skeleton's region The loaded section is a region named from the comments heading; its skeleton announced itself as an unnamed one. The page resolves the label and passes it in, because a <Suspense> fallback that suspends hands the wait to the nearest ancestor boundary — here, the document's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * docs(index): correct the skeleton stylesheet's mirroring claim The header comment claimed every rule mirrors a counterpart in page.module.css, but the scope root has none: page.tsx renders <main className={css.main}> and page.module.css defines no .main, so the loaded root ships class-less. The dead css.main reference itself predates this change and is left alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * test(app): mark the render-failure spec's invariant guards resolveRenderFailure() returns a constant, so two of the four cases would pass under any constant return and cannot fail today. They stay, because each pins a contract for the day the mapping stops being constant, but the doc comment now says so rather than letting the suite read as four live checks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * docs(skills): sanction the route-boundary skeleton shape The two new skeletons are standalone -loading components rather than the orchestrator/loaded/loading triad component-conventions.md makes a MUST, and the departure rested only on a comment in each file. The Loading and Loaded Split section now names the shape, the condition that makes it legitimate, and the mirroring obligation it carries in place of the file adjacency a triad gets for free. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * docs(skills): scan-size the Jest mocking guidelines The two bullets this change added carried their whole rationale inside the guideline, where the skill-authoring style asks for rationale in the section intro and bullets short enough to scan. The rules are unchanged; the why now sits in two intro paragraphs and the resetModules bullet is split so each states one action. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * fix(app): repair the loading placeholder's max-lines clamp attr()'s typed form spells its type as type(<syntax>), so the bare `number` attr-type made the whole max-block-size declaration invalid and the browser dropped it. Computed max-block-size was `none` on every text placeholder, which meant no maxLines prop on the site clamped anything: each block was simply as tall as its sample text happened to wrap. Measured in Chromium 141 before this change, the index bio placeholder computed max-block-size: none with data-max-lines="3"; after it computes 3 * 1lh. The sibling `width: attr(data-length ch)` on the word pills uses the valid <attr-unit> form and is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * fix(posts): lengthen the samples a live clamp leaves short With the clamp inert, a placeholder was exactly as tall as its sample wrapped, so two samples that fitted on fewer rows than their maxLines went unnoticed. Now that the clamp applies, a sample has to reach its own maxLines or the block renders short. The post header's title sample fitted on one row from ~800px up, leaving the skeleton 66.6px shorter than the two-line header it stands in for at 1280px (66.61 vs 133.25); it now measures 133.25 against the same 133.25. The second comment row's body sample did the same, rendering that row a line shorter than the first at the same widths; both rows now measure 83.56 against a loaded row's 83.73. Neither maxLines value changes, and no loaded surface is touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * docs(index): record the bio skeleton's live-copy parity target The bio placeholders' maxLines values are 3 and 2 because that is how the bio the live site serves wraps at 412px and 1280px alike, not because of the seeded fixture. With the clamp repaired they measure 157.63 and 174.23 against the live bio's 157.81 and 174.42 at those widths, closing the ~30px overshoot the pre-flight review found. The comment also records the trade-off that choice carries: the seed fixture in payload/helpers/seed/bio.md is several times longer than the live copy, so a preview deployment or an e2e run shows a visibly larger gap. No value in this file changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * test(app): re-express the app specs against vitest #230 migrated the unit suite off Jest, so the two specs this branch adds under app/ import their test API from `vitest` rather than `@jest/globals`. Neither uses a mock, so the import line is the whole change. `markdown.spec.ts` does not yet run: `vitest.config.ts` resolves the whole graph under the `react-server` condition so `markdown.ts` can import `server-only`, and `react-dom/server` answers that same condition with a module that throws on import. Repairing it needs a runner configuration change, which is outside the approved plan. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * test(payload): re-express the payload specs against vitest All three faked a module boundary with `jest.doMock()` plus a dynamic import. That shape existed only to work around SWC's Jest transform, which hoists `jest.mock` only when written against the bare global — it was never the behaviour these specs wanted. Vite hoists `vi.mock` whatever the import style, so each one collapses back to a plain top-level mock and a static import of the subject. - image.spec.ts: `getStaticDir()` reads the token per call rather than at module scope, so a single mutable holder covers both branches and the re-import helper disappears entirely. - banner-block.spec.ts: the editor package fake moves to a top-level `vi.mock`, dropping the `beforeAll` and the module-registry caveat with it. - mcp.spec.ts: the captured plugin configuration needs `vi.hoisted()`. The factory runs while the subject is imported, which is before this module's own body, so a plain module-level binding would still be in its temporal dead zone when the factory assigned to it. The explicit `restoreAllMocks` hook is gone: `vitest.config.ts` already sets `clearMocks` and `restoreMocks`. `sanitizedPayload()`'s two shape checks become throws. Biome's `noMisplacedAssertion` recognizes `vitest` imports where it did not recognize `@jest/globals`, so it now reports the `expect` calls this helper always made outside a scenario; throwing matches how the override lookups beside it already report a missing hook. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * fix(app): read colour roles, not scales, in the state message #229 put a semantic colour-role layer between the palette scales and the components, and docs/conventions/styling.md now forbids reading a private-tier scale variable outside app/(app)/variables.css. This stylesheet was written before that layer existed. Each swap is behaviour-preserving: variables.css defines all four roles as exactly the scale steps that were being read. --neutral-11 -> --color-text-neutral-low --accent-11 -> --color-text-accent-low --accent-2 -> --color-background-accent-subtle --accent-5 -> --color-component-accent-selected The focus ring now matches the Focus Ring template verbatim as a result. The hover style is also gated on a fine pointer, which #233 made the rule after this stylesheet was written — an ungated background-color hover sticks after a tap on a touch device. The retry control needs no coarse-pointer sizing: it measures roughly 74x40, clearing both the 44px long-side and 24px short-side minimums. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * docs(conventions): record the standalone skeleton and two vitest traps #228 retired the repository-owned skills into docs/, so the two rule changes this branch made against project-structure's references land here instead. react-components.md: the Loading and Loaded Split section made the orchestrator/loaded/loading triad a MUST, and the two new skeletons are standalone -loading components that depart from it. The departure rested only on a comment in each file. The section now names the shape, the condition that makes it legitimate — a route-level <Suspense> boundary owns the pending state, so no component has a loaded sibling — and the mirroring obligation it carries in place of the file adjacency a triad gets for free. testing.md: the branch's jest.doMock rule is gone rather than relocated. It described a workaround for SWC's hoisting under next/jest, and #230 removed the runner it applied to; Vite hoists vi.mock whatever the import style, so the rule has no subject. The two traps that replaced it are real and both cost time in this change: - A vi.mock factory runs while the subject is imported, before the spec module's body, so anything it assigns to needs vi.hoisted(). - Biome's noMisplacedAssertion recognizes a vitest import where it did not recognize @jest/globals, so a helper that asserts stops passing lint on migration. All five living-product-specification validators pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * build(test): stub server-only instead of the react-server condition The markdown pipeline's new spec needs both `server-only`, which its subject imports, and `react-dom/server`, which it uses to serialize the pipeline's output and assert the HTML. Those two cannot both resolve under one global condition set: `server-only` answers `react-server` with an empty module and everything else with a module that throws, while `react-dom` answers `react-server` with a module that throws, on every `server*` and `static*` subpath. The condition applies process-wide, so no spec can escape it from the inside — Node's own createRequire resolves to the throwing build too — and Vite has no per-package or per-file condition override. Aliasing `server-only` to its own empty stub and dropping the condition is what next/jest did before #230, and the only arrangement measured green across the whole suite: condition declared 37 files / 376 tests, markdown spec fails condition removed 35 files / 320 tests, three specs fail stub alias, no cond. 38 files / 387 tests, green This reverses a deliberate choice from #230 that carried no decision record, so it gets one rather than flipping silently: docs/decisions/2026-08-10-stub-server-only-in-the-unit-runner.md. The config comment that argued for the condition is replaced rather than left contradicting the code, and both it and the record name the trade being accepted — the stub is a fake where the condition was the package's real mechanism, and `server-only` now enforces nothing under the unit runner. The build still enforces it, which is where that boundary actually lives. Approved by @axross as an amendment to the plan on #179, which had expected no runner-configuration change. All five living-product-specification validators pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * docs(specs): record the error surface and scope the heading voice reader-surfaces.md gains an Error section: what a reader sees when a route fails, that the boundary renders inside the root layout so the header and footer survive, and that the retry re-renders the failed region in place rather than reloading the document. It also notes what does not reach the surface — a missing post renders the not-found surface instead. The open question on #179 is resolved as the surface keeping its approved design and the specification changing around it. @axross chose option A — one muted line, no heading — with the alternatives in front of him, while the conflicting sentence was written for the not-found surface before this one existed and generalized from a sample of one. So the `## Empty` section now states that an in-container empty message carries no heading, and scopes the code-syntax voice to the not-found surface's own heading rather than presenting it as a rule every empty surface inherits. visual-identity.md asserted the same generalization in its Voice section and is corrected to match; leaving the two contradicting each other would have been the real defect. The design selection overrides a written convention and constrains every future empty or failed region, so it gets a record: docs/decisions/2026-08-10-give-the-shared-empty-and-error-surface-no-heading.md docs/index.md's entry for reader-surfaces.md now enumerates the error state alongside the others it covers. All five living-product-specification validators pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * fix(index): size the skeleton's social icons from a literal 24px Re-measuring skeleton parity after the merge showed the index skeleton standing ~5px taller than the row it is replaced by at desktop width. The cause is that the placeholder sized the glyph from `--size-24`, which is fluid: it resolves to 24.48px at 412px and 29.14px at 1280px, while the real social icon is a literal 24px SVG. The stylesheet's own comment had recorded this as an accepted cost; it is not one worth paying, and a glyph's size is a fixed asset dimension rather than a spacing step, so it does not belong on the scale. Measured against the bio the live site serves — two Japanese paragraphs of 75 and 39 characters, fetched rather than assumed: 412px 1280px before after before after intro -0.29 +0.19 -4.95 +0.19 social-links -0.48 0 -5.14 0 Identical under a coarse pointer, where #233's negative margin keeps the link's outer box at 24px while its hit area grows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu * fix(index): mirror the social row's real selector names The skeleton's social rules were named `.socialLinks` / `.socialLink` while the loaded row they stand in for is `<SocialLinkList>`, whose stylesheet calls them `.socialLinkList` / `.listItem`. The values already agreed, but nothing textual pointed from one to the other, so a spacing change on the loaded side had no cue leading back here — the drift the mirror rule exists to prevent. The file's header comment also claimed every rule mirrors `page.module.css`, which has no social selectors at all. It now names the right counterpart, and records that `.socialIcon` has none because the loaded glyph takes its size from the SVG asset rather than a rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FjDKycriTUnQXxYT9mgjgu --------- Co-authored-by: Claude <noreply@anthropic.com>
Summary
Nothing in this repository described the product in the present tense, and what it knew about itself lived in three repository-owned skills only an agent whose discovery happened to surface them could read. This makes
docs/the single documentation root for both, in three parts —specs/for what the product does,conventions/for how the code is written,operations/for how the repository is built and run — withdecisions/beside them.Why one root rather than two.
living-product-specificationscopes conventions and runbooks out of the corpus without naming a home for them, which is a statement about ownership rather than location; its own bootstrapping reference already contemplates adocs/that also holds contributor documentation. A separateCONTRIBUTING.mdtree would have created a second root and a decision, at every write, about which tree a paragraph belongs in. The three bodies are kept apart by directory and named apart byindex.md, which is what the validators permit exactly:check-glossary.mjsinspectsspecs/only, andcheck-index.mjsforces every document to be listed.What retiring the skills costs. Discovery. A skill loads because its frontmatter matched the surface being edited; a document loads because an instruction file said to read it. The reasoning for accepting that is that
CLAUDE.mdis injected into every session unconditionally while skill discovery fires only conditionally — but it holds becauseCLAUDE.mdnow routes by surface to a named document, so that table is load-bearing rather than decorative.Both deployment runbooks are decomposed rather than preserved, which deliberately overturns #218's original non-goal. Their procedures moved to
operations/; the constraints whose rationale was already written down became dated decision records.README.mdkeeps four subjects — what this is, how to start it, the command table, and where everything else lives — dropping from 450 lines to 104.The bullet ledger
533 guideline bullets across the three skills, accounted for:
conventions/andoperations/(363 total there, of which 61 came from the runbooks)visual-identity's product half (129),comments-subsystem(18),content-safety(19),content-source(2)The CSS drop is the largest and worth scrutiny.
react-component-stylingalready owns@layer/@scope/:where(:scope), logical properties, one colour space, container queries over media queries,stretch/dvh,currentColor, all four scroll-timeline rules, hit-area expansion, the module-per-component pairing, and consumer-classNamemerging — including this project's own examples verbatim (theblog-post-header.tsx→blog-post-header.module.csspairing, the1lh/1chguidance, the "promote at the third surface" threshold). The library generalized this project's CSS conventions upstream at some point and the skill has been a duplicate copy since. What survives inconventions/styling.mdis the token names, the two breakpoints and the--variantbinary, the--themebridge, the imagery filter recipe,tab-size, the focus-ring and hit-area templates, the truncation patterns, and the recorded exceptions.Decision records
Seven, each dated from the commit that introduced its rationale (which required unshallowing the clone — the original 50-commit clone floored at 2026-07-19 and would have mis-dated three of them):
2026-07-06branch preview databases from production — superseded2026-07-08target the canonical www host for the MCP endpoint2026-07-15split destructive schema changes expand-then-contract2026-07-16namespace preview media under a blob prefix2026-07-18seed preview databases from fixtures rather than branching production2026-07-21build on the Actions runner and deploy prebuilt2026-08-09keep project conventions in docs rather than repository-local skillsThe 07-06 → 07-18 pair is a genuine supersede chain, not two independent records: previews were originally branched from production, which put real
usersandpayload-mcp-api-keysrows on a publicly reachable URL.The Japanese-primary locale and the disabled GraphQL surface were excluded as reconstruction — both are stated in the code, neither has its rationale written down anywhere, and inventing one would be a guess presented as history.
Related issues
Closes #218
Supersedes #180 and #216, both closed ahead of this change. #216's second half — the discovery contract keying on the removed
when_to_usefield — is fixed here rather than left behind:CLAUDE.mdno longer references it.Filed upstream while doing this: axross/skills#315, arguing that
agent-skill-management's tier question presupposes a skill and never asks whether project-specific knowledge should be one, plus a feature request thatliving-product-specificationsanction theconventions/+operations/shape adopted here. Recorded locally as a gap indocs/operations/agent-skills.md, per that register's own rule that a gap does not wait on an upstream issue landing.Verification
check-index.mjs docscheck-references.mjs docscheck-glossary.mjs docscheck-decision-naming.mjs docscheck-decision-supersede.mjs docsnpm run lintnpm run typechecknpm run test:unitagent-skill-authoring, 1 is an ellipsis in seeded content).claude/skills/vsskills-lock.jsonAcceptance criteria from #218, each met: the tree has
index.md/overview.md/glossary.md/ 6 specs / 11 conventions / 6 operations /decisions/;index.mdstates the boundary in prose and linksdecisions/once as a directory; both runbooks are gone with their procedures underoperations/;README.mdis 104 lines;CLAUDE.mdnames a document per surface and contains nowhen_to_use; the deviations register carries both existing deviations plus the new gap naming axross/skills#315.Spot-checked spec claims against the code rather than against the skills they came from: the auth lockout (5 attempts / 5 minutes), the banner kinds (
note/warning), the/posts→/redirect, the live-preview breakpoints, the locale fallback direction, and every glossary term's presence in the codebase as an identifier, slug, field, or route segment.Not run:
npm run test:e2eandnpm run build. No application code changed — the only non-Markdown edits are three comments repointing paths to the deleted skills — so neither exercises anything this diff touches.Risks and breaking changes
Three skills are deleted. Anything referencing them by path breaks. The three in-tree references were repointed (
playwright.config.ts,check-and-deploy.yaml,claude-review.yaml); a reference outside this repository would not have been found.The discovery regression is real and unmitigated by anything except
CLAUDE.md. If that routing table decays into a general pointer, the conventions stop being read. That is stated in the decision record rather than left implicit, because it is the assumption the whole change rests on.The corpus validators are not wired into CI — excluded from scope at the interview. Nothing currently stops
docs/drifting out of the shape the validators check.conventions/styling.mdnow depends onreact-component-stylingstaying installed. Roughly 44 rules were dropped on the grounds that it owns them; uninstalling it would silently drop them from this project's guidance too.The audit also surfaced a conflict this change deliberately does not resolve:
color-and-gamut.mdforbids referencing a ramp step from a component, while this project references--accent-5and--accent-11directly throughout. That is #221, and the rules were relocated as they stand.Notes for reviewers
Start with
docs/index.md, thendocs/overview.md— between them they state the boundary the whole tree rests on, and if that reads wrong nothing below it is worth reviewing yet.The two things most worth scrutiny:
specs/visual-identity.mdandspecs/reader-surfaces.mdare converted from rule bullets to present-tense prose, which is where a "should" could have quietly become an "is".specs/blog-posts.md,specs/comments.md, andspecs/content-authoring.mdare written from the collections rather than from the skills.cd5da23; each entry names the installed reference that owns it.The
glossary.mdsplit at the#level is deliberate:check-glossary.mjsreads##headings only, so the##level stays reserved for the spec pairing it enforces.🤖 Generated with Claude Code
https://claude.ai/code/session_0137GC1hE7fdpadf8vHhegUd
Generated by Claude Code