Files
ai-for-dummies/plans/astro-refactor/task-19-verify-repoint.md
T

167 lines
8.7 KiB
Markdown

# Task 19 — Re-point the verification contract
**Agent**: `verification-engineer` · **Model**: Codex **Depends on**: 15, 16 ·
**Parallel with**: 18 **Worktree**:
`.agents/scripts/worktree.sh start 19 verify-repoint`
## Goal
All 42 assertions pin the same user-visible facts against the new architecture.
Coverage does not fall.
You are the only role permitted to remove an assertion, and every removal needs
a written reason.
## The three kinds
| Kind | Example | What to do |
| --------------------- | --------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------- |
| Content presence | `data-phase="plan"` | re-point at `dist/full-guide/index.html`; the token should survive rendering. If it does not, a component dropped content — **stop and report** |
| Implementation detail | `const phases`, `renderTree`, `from './catalog.js'` | obsolete as written, but each pins a **feature**. Replace with an output-level assertion of that feature. Never drop |
| Asset version | `app.js?v=20260904-vote-widget` | Astro hashes assets — assert the built HTML references a hashed asset |
## Steps
1. `pnpm run build`, then re-point `read()` calls at `dist/`.
2. Work through all 42 in order. For each: does the fact it pins still exist?
Yes → re-point. No → content was lost; escalate.
3. Add rendered-text snapshot assertions for all ten routes so this class of
regression is caught structurally, not by string luck.
4. Confirm `check-tokens.mjs` and the extended `audit-ui.mjs` are in
`pnpm run verify`.
## Done when
- [ ] `grep -c 'throw new Error' scripts/verify.mjs` ≥ the `origin/main`
baseline
- [ ] Every removal has a one-line reason in this file
- [ ] Snapshot assertions cover all ten routes
- [ ] `pnpm run gate` green, and it **fails** when you deliberately delete a
paragraph from a component (prove the net works, then revert)
## Do not
Do not weaken an assertion to make it pass. If it cannot pass, something is
broken — that is the assertion doing its job.
## Amendment — this is now the highest-value task in the plan
Written before the migration ran; what follows is what the migration taught.
Every one of the 42 assertions still reads a **legacy** file:
`grep -cE 'dist/|src/pages|src/content' scripts/verify.mjs` returns 0. Nothing
in the gate looks at what the Astro pages render. Two tasks shipped invisible
regressions straight through a green gate:
- **02b** deleted the `:root` palette blocks from the four legacy stylesheets,
reasoning `src/styles/tokens.css` is the single source of truth. It is — for
Astro pages. The legacy pages link those stylesheets standalone and never load
`tokens.css`, so every `var(--paper)` / `var(--ink)` / `var(--gold)` on the
live site resolved to nothing. Eight pages, colourless. Gate green.
- **15d attempt 1** built `/full-guide/` by importing
`full-guide/index.html?raw` and `set:html`-ing the `<main>` out of it. Zero
`data-language-content` attributes, zero `.pt` reads: Portuguese gone from the
largest page on a bilingual site. Gate green. Tagged `rejected/15d-attempt-1`.
The gate is doing its job — it is a legacy-content contract. Your job is to make
it an output contract too. Until you land, "gate green" means nothing about the
new site.
### Three checks to build in, each of which caught a real regression
1. **Bilingual coverage of the built HTML.** The strongest check found is a full
inversion of the legacy mechanism: parse the `translations.pt` object out of
`app.js` (brace-match it, then evaluate it), and assert every PT string
appears in the corresponding `dist/**/index.html`. Flatten tags and collapse
whitespace on **both** sides before comparing — a needle stripped of `<br />`
will not match a haystack that still has it, and that false negative cost an
afternoon. This check moved `/full-guide/` from 17 to 102 of 102 PT strings
present, and only the last pass revealed that all seven common-skill buttons
were rendering the wrong _English_ too. `translations.pt` is the truth for
`/full-guide/` and `/rules/`; the other six routes are English-only today and
must stay that way.
2. **Every `var()` resolves.** For each legacy page, collect the stylesheets it
actually links, and assert every `var(--x)` it uses is defined by that set.
`--score` is the only legitimate miss — `app.js:289` sets it inline. This is
the check that would have caught 02b in seconds.
3. **Built-CSS diff against `main`.** Build, then enumerate every colour and
size declaration in `dist/_astro/*.css` that exists on `main` and not on the
branch. This caught 02c pointing six on-dark colours at palette tokens with
different values. Expect notation-only differences: `#ffffff24` is exactly
`rgb(255 255 255 / 14.1176%)`, and `32px`/`48px` now resolve through
`--step-32`/`--step-48`.
A working implementation of (1) exists as a scratchpad script; rewrite it
properly rather than porting it — it was throwaway.
### On the assertion count
Baseline is 42 and the gate enforces it. Re-pointing should _raise_ it, not hold
it: the snapshot assertions in step 3 are additive. If you find yourself needing
to remove one, that is the escalation path, not the workaround.
### Screenshots
`playwright` is now a devDependency (`c2a046d`) and
`.agents/scripts/visual-regression.mjs` runs. It used to throw for everyone,
which is why no task in this plan ever produced the captures its brief asked
for.
## Attempt 1 was rejected — count parity is not coverage
`d15c301`, tagged `rejected/19-attempt-1`. It did good work: `verify.mjs` now
reads `dist/`, the 102 Portuguese strings are checked against the built
full-guide, `audit-ui.mjs` gained per-page CSS-variable resolution and a
built-CSS baseline (`.agents/snapshots/built-css-values.json`). Keep all of
that.
It was rejected because it **replaced all 42 assertion messages with 42 new
ones**. Not one of the originals survives. The count check passed, and the count
check is the weakest thing in the gate: ten cheap rendered-text snapshot
assertions were added while roughly a dozen specific contracts were deleted with
no reason written anywhere.
Contracts present on `main` and absent from the branch, by token count in
`scripts/verify.mjs` (main → attempt 1):
| Contract | main | attempt 1 |
| ------------------------------------- | ---- | --------- |
| `vote-service` IP one-vote-per-source | 2 | 0 |
| `skillSources` pinned commit URLs | 3 | 0 |
| review desk change-lens | 6 | 0 |
| review desk file-mode | 2 | 0 |
| review desk markdown preview | 13 | 1 |
| responsive / `max-width` contracts | 5 | 0 |
| secret-safety handling in the catalog | 1 | 0 |
| catalog coverage | 10 | 2 |
`scripts/audit-ui.mjs` also went from 6 assertions to 5.
A rendered-text snapshot does not replace these. "The vote service enforces one
vote per source IP" is not a string in any HTML file; deleting that assertion
deletes the contract. Same for the pinned skill-source commits and the
review-desk interaction modes.
### What attempt 2 must produce
1. **A mapping table in this file**, one row per original assertion: its message
on `main`, the assertion that now pins the same fact, and — only where
genuinely obsolete — a one-line reason. Build it from
`git show rejected/19-attempt-1^:scripts/verify.mjs | grep -oP 'throw new Error\(\s*\K.*'`
so no row is missed. 42 rows, no gaps.
2. **Every fact still pinned.** Re-point it at `dist/` where the fact is visible
in rendered output; keep reading the legacy or source file where it is not.
`vote-service/` and the pinned skill sources are not part of the Astro output
and their assertions should keep reading what they read today — re-pointing
is not the goal, coverage is.
3. **Raise the baseline.** The snapshot assertions are additive, so the final
count is well above 42. Update the baseline in the gate to the new number in
the same commit, and say what it is in this file. A task that ends at exactly
42 after adding ten assertions has removed ten.
4. Keep `audit-ui.mjs` at 6 or more.
Start from `rejected/19-attempt-1` rather than from scratch — the `dist/`
re-point and the Portuguese coverage check are worth keeping.