Skip to content

AGENTS.md: three conventions earned while shipping #651 + #652 #662

Closed

Description

@mforce

Three rules the SPA look-and-feel pass paid for. Filed rather than folded into #661, whose simplicity ceiling is web/src/styles.css plus two guard files.

Full retro: docs/designs/651-652-spa-elevation-and-caps.md and PR #661's review history.

1. A selector an issue names is dead until its call sites are counted

Three selectors named across #651 and #652 had moved on before the work started:

Selector Reality at 396ba233
.stat / .stat-label did not exist — #654 replaced them with .capture-tile*
.eyebrow zero call sites — deleted as dead in #661
.toolbar zero call sites — restyled anyway, so that half of #651 changes nothing on screen

The first two were caught by reading the shipped code before designing. The third was not, and only surfaced when the owner compared before/after screenshots and said they looked the same.

Proposed rule, near the guard-writing section: before styling a selector an issue names, run grep -rn "<class>" web/src --include='*.tsx' and record the count in the design. Zero is a legitimate answer — .toolbar is deliberate groundwork for #653 — but it has to be a stated decision, not a discovery after merge.

2. Removing a text transform makes every string it transformed a caller

.badge applied text-transform: uppercase, so two i18n strings had been authored lower case at source because the stylesheet capitalised them. With the transform gone they rendered no entry and yes — typos, in all three locales. Now pinned by web/src/i18n/badgeCase.test.ts.

Proposed rule, in the i18n conventions: a string rendered inside a .badge starts with a capital; and more generally, when a presentational transform is removed, every string it was transforming is a caller in the #394 sense and must be read.

3. A screenshot comparison is a verification step, not a PR decoration

It is the only check in this repo that reads the rendered result. On #661 it found the one product defect that four review seats, three CI Playwright runs and the driver's own verification all passed. Two mechanics matter:

  • Capture at 1:1. The first comparison was downscaled to 900px wide, which destroys exactly the 1px hairlines and 4–5% alpha shadows a styling change is about — it made a real change look like no change.
  • Rebuild first. A long-running sim stack serves the bytes it was built from, not the branch you are on.

Proposed rule, beside the SPA E2E section: any PR whose purpose is visual attaches a 1:1 before/after comparison, captured from a stack rebuilt at the head under review.

4. Runbook note — reset.sh from a git worktree

tools/simulation/reset.sh fails immediately in a worktree with tools/simulation/.env.sim not found: that file and .sim-cast.json are gitignored and exist only in the main checkout. Copy both into the worktree first. tools/simulation/ui/ also has no node_modules there — but the screenshot specs drive the stack over HTTP, so they can be run from the main checkout against a worktree-built stack.

Belongs in docs/runbooks/simulation-fixture-on-a-dev-database.md.

Activity

  1. mforce commented on Sep 2, 2026

    @mforce
    OwnerAuthor

    Shipped in #665, merged as 930ea308. All three conventions are in AGENTS.md, in the file's own form — one paragraph each, bolded lead, reasoning inline, issue number in the text.

    1. Count a selector's call sites before styling it. Three selectors named across SPA: elevation hierarchy — stop using one card treatment for stat cards, panels, toolbars and entry panes #651/SPA: retire tracked all-caps labels outside the nav group dividers #652 had moved on; .toolbar was restyled with zero consumers, so half of SPA: elevation hierarchy — stop using one card treatment for stat cards, panels, toolbars and entry panes #651 changed nothing on screen.
    2. Removing a presentational transform makes every string it transformed a caller. .badge's uppercase had let two i18n strings ship lower-case at source; removing it rendered no entry and yes.
    3. A PR whose purpose is visual attaches a 1:1 before/after comparison from a stack rebuilt at the head under review. Both mechanics are stated, because both were learned the hard way — downscaling destroys the hairlines and low-alpha shadows the change is about, and a long-running stack serves the bytes it was built from.

    The runbook note also landed in docs/runbooks/simulation-fixture-on-a-dev-database.md: reset.sh from a git worktree fails on the gitignored .env.sim, and tools/simulation/ui/ has no node_modules there, but the capture specs drive the stack over HTTP so they can run from the main checkout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentation

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions