From 2a663232c878b3e10fb4404e5601097f71f1dd98 Mon Sep 17 00:00:00 2001 From: Chris Henesy Date: Tue, 6 Oct 2026 17:40:54 -0400 Subject: [PATCH] =?UTF-8?q?design:=20TDDs=200070=E2=80=930072=20+=20ADR=20?= =?UTF-8?q?0017=20(/ux-author=20phase)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Designs PRD FR-89–FR-101 (PRD-rev 3be6232): the optional /ux-author phase between /prd-author and /tdd-author. - 0070 ux-record-lib: scripts/lib/ux.sh plus python3-stdlib helpers. Covers the [UI] grammar (loud on a malformed marker), per-requirement hash delta, the docs/ux/index.json schema v1, validate (offline-only mocks, required states, no committed captures), scoped headless-Chrome render, capture, the generated index.html flow page, and merged-index coverage. - 0071 ux-author-skill: the /ux-author skill and the agents/ux-reviewer critique. The skill shows a draft screen plan first, interviews, co-creates a rubric, then has mocks made by delegates by role (or the session, with fidelity declared), followed by in-session review, the fresh-worker critique, and a docs/ux/ PR it never merges. - 0072 ux-in-prd-and-tdd-author: /prd-author writes [UI] markers; /tdd-author refuses an un-mocked [UI] requirement unless it is waived; a tdd-lint citation check; a design-reviewer bullet. - ADR 0017: the UX record lives in-repo as self-contained HTML, design is delegated by role, and mocks are design input, never a build gate. Cascade correction to #191: FR-89–FR-101 are new requirements, so no implemented TDD is superseded. ## Design critique (design-reviewer, fresh worker each round) DESIGN_REVIEW: PASS on round 5. Rounds 1–4 BLOCKed: - R1: render-degrade contract; orphaned mappings. - R2: TL_SCREENS "empty means all"; FR-90 plan-first. - R3: screenshot completeness vs scoped render. - R4: screenshot fields owned by render but needing edits from the skill. All were fixed. R4 was fixed at the root: screenshots are derived from disk and never stored in index.json. R5 PASSed with 6 minor findings and 4 nits. All of them were folded in after the PASS as clarifications, not re-reviewed: - step-8 assets limited to data:/SVG/fonts; - the PR screenshot line comes from the final validate count; - design-system cases ordered, and the established ADR written accepted; - merged-screen drop removes requirements left with no screen; - re-render on viewport change, with a stated point; - mktemp paths recorded in the draft; obs wording. Every round graded against the co-created rubric (present in each TDD). No criterion was graded failing in round 5. ## Open assumptions & waivers - requirement id grammar — resolved: strict `** [UI] **`, ID [A-Z][A-Z0-9]*-[0-9]+; any unparseable [UI] makes ux.sh exit 2 naming the line. - delegate output not self-contained — resolved: validate rejects http(s)/`//` URLs in mocks; raster images inlined as data: URIs or SVG; only fonts/SVG vendored. - delegate detection is unverifiable — resolved: the index lists only delegates actually invoked; the reviewer is told provenance is self-reported and grades the declared fidelity (accepted residual). - renderer — resolved: headless Chrome/Chromium CLI found on PATH (or THROUGHLINE_UX_BROWSER); no Playwright; missing → "screenshots not rendered: no headless Chrome on PATH". - changed UI requirement detection — resolved: a per-requirement block hash in index.json; no git-history dependency. - skill file size — resolved: mechanics in ux.sh tl: blocks; interrogator/rubric rules read from /prd-author, not copied. - tdd-author refusal needs merged index — resolved: coverage reads the integration-branch blob (best-effort fetch first); an unmerged UX set does not count. - tokens artifact format — resolved: docs/ux/tokens.css (CSS custom properties), linked by every mock. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NpGG21Y2VGGhTQaFr4oshB --- ...x-record-in-repo-delegated-design-input.md | 70 +++ docs/adr/INDEX.md | 1 + docs/tdd/0070-ux-record-lib.md | 505 +++++++++++++++++ docs/tdd/0071-ux-author-skill.md | 506 ++++++++++++++++++ docs/tdd/0072-ux-in-prd-and-tdd-author.md | 288 ++++++++++ 5 files changed, 1370 insertions(+) create mode 100644 docs/adr/0017-ux-record-in-repo-delegated-design-input.md create mode 100644 docs/tdd/0070-ux-record-lib.md create mode 100644 docs/tdd/0071-ux-author-skill.md create mode 100644 docs/tdd/0072-ux-in-prd-and-tdd-author.md diff --git a/docs/adr/0017-ux-record-in-repo-delegated-design-input.md b/docs/adr/0017-ux-record-in-repo-delegated-design-input.md new file mode 100644 index 0000000..714158b --- /dev/null +++ b/docs/adr/0017-ux-record-in-repo-delegated-design-input.md @@ -0,0 +1,70 @@ +# 0017. UX record: in-repo, self-contained HTML, delegated by role, design input not a build gate +Status: accepted +Date: 2026-10-06 +Scope: workflow / ux / plugin-architecture + +## Context +PRD FR-89–FR-101 add an optional `/ux-author` phase between `/prd-author` +and `/tdd-author`. It runs for requirements marked `[UI]`, and the human +gates its output in a PR. TDDs 0070–0072 design it. Several choices in +that design are durable stances that later work would otherwise reopen: +- where the UX record lives; +- who produces the visual design; +- what authority mocks have over the build. + +Two existing ADRs relate to them. ADR 0004 delegates the verification +*mechanism* to the project, and ADR 0010 makes engineering delegates +optional. The superseded ADR 0001 lineage had throughline own the +authoring phases while delegating ideation. This ADR extends that +ownership to a third authoring phase, UX, and delegates its design +work. + +## Decision +1. **The record is in git, self-contained, and machine-checkable.** The + UX record of record lives under `docs/ux/`: + - offline HTML mocks, one per screen and state, with no `http(s)` or + `//` references; + - `docs/ux/tokens.css`, CSS custom properties, as the design-system + artifact; + - `docs/ux/index.json` (schema v1), which maps `[UI]` requirement ids, + with a text hash of each, to screens and records provenance and + fidelity; + - a generated `index.html` flow page; + - PNG screenshots rendered by a headless Chrome/Chromium CLI when one + is available. + + External design tools (Figma, hosted design canvases) may be used as + delegates, but they are never the record. +2. **Visual design is delegated by role.** `/ux-author` invokes whatever + the harness offers for the `design`, `design-system`, `critique` and + `accessibility` roles. throughline code holds only the mechanics: + parse, hash, validate, render and report. With no delegate, the + session makes the mocks itself and declares the fidelity it reached + (never `high`). The `delegates` field is self-reported, and the + independent critique grades against the declared fidelity. +3. **Mocks are design input, not a build gate.** `/tdd-author` must cite + merged mocks for each `[UI]` requirement, or record a waiver with a + rationale. `/build-tdds` never compares built UI to mocks, and no + visual-diff gate exists. + +## Consequences +- Repos with `[UI]` requirements need python3 for UX tooling. Repos + whose PRD contains no literal `[UI]` text are unaffected: every entry point those repos can reach + (`/prd-author`'s marker check, `/tdd-author`'s coverage step, the + `tdd-lint` citation check) short-circuits when no `[UI]` appears. +- Screenshot PNGs live in git history. A superseding UX run removes the + superseded files, and Git LFS is not required. +- The record is reviewable in a plain PR and readable offline. It cannot + drift with an external tool's later edits. +- The quality of the mocks depends on the delegates present. The degrade + path keeps the phase completable on any harness (FR-79, FR-83). +- Adding a visual-diff or screenshot-comparison gate to `/build-tdds` + later requires a new ADR that supersedes this one. +- Rejected alternatives: + - **An external tool as the record:** it lives outside git, stays + mutable after merge, and may be private. + - **A visual-diff runtime-verify gate:** it couples verification to a + browser and to pixel stability, which contradicts ADR 0004's + delegated mechanism. + - **Bundling a specific design skill:** it creates a hard dependency, + which contradicts ADR 0010's optional delegates. diff --git a/docs/adr/INDEX.md b/docs/adr/INDEX.md index bb92432..77aa54d 100644 --- a/docs/adr/INDEX.md +++ b/docs/adr/INDEX.md @@ -19,3 +19,4 @@ | 0014 | Model capability by job (fresh worker, not a different named model); supersedes 0009 | superseded by 0015 | workflow / gate-architecture / model-selection | | 0015 | Model cost/performance by job (inherit the parent, light tier, escalation on demand); supersedes 0014 | accepted (fall-back detection consequence revised by 0016) | workflow / gate-architecture / model-selection | | 0016 | Escalation outcomes are judged by the model that actually ran; revises 0015's fall-back detection | accepted | workflow / gate-architecture / model-selection / verification-integrity | +| 0017 | UX record: in-repo, self-contained HTML, delegated by role, design input not a build gate | accepted | workflow / ux / plugin-architecture | diff --git a/docs/tdd/0070-ux-record-lib.md b/docs/tdd/0070-ux-record-lib.md new file mode 100644 index 0000000..44561c6 --- /dev/null +++ b/docs/tdd/0070-ux-record-lib.md @@ -0,0 +1,505 @@ +# TDD 0070: UX record library — `[UI]` requirements, the UX index, validation, rendering + +Status: draft +PRD refs: FR-89, FR-90, FR-91, FR-94, FR-99, FR-101, NFR-4 +PRD-rev: 3be6232 +ADR constraints: 0004, 0005, 0006, 0010, 0011, 0017 + +## Approach +`/ux-author` (TDD 0071) and the `/prd-author` / `/tdd-author` +integration (TDD 0072) need the same mechanical facts: +- which requirements are `[UI]`, and which of those changed since the + last UX set; +- whether a UX set on disk is well-formed and self-contained; +- what the merged UX set covers. + +This TDD puts them in one library that the skills call from `tl:` +blocks. The library holds **mechanics only**: it parses, hashes, +validates, renders and reports. Visual design belongs to the delegates +(FR-92, TDD 0071). + +The library has two layers: +- `scripts/lib/ux.sh`: thin `tl_ux_*` bash wrappers; +- `scripts/lib/ux_record.py` and `ux_render.py`: python3 stdlib only, + run as `python3 -I <file> <subcommand>`. + +A wrapper whose input has no `[UI]` returns before looking for python3, +so non-UI repos never need it. Every parse failure fails loudly with a +named `ux:` message, and none reads as "no UI requirements" (rubric: +fail-loud parsing; L-005). + +## Components & interfaces + +### The `[UI]` grammar (FR-89) +A `[UI]` requirement is a line matching (Python `re`, after stripping one +leading list marker `- ` or `* `): + +``` +^\*\*(?P<id>[A-Z][A-Z0-9]*-[0-9]+) \[UI\] (?P<title>[^*]+?)\.?\*\* +``` + +Example: `- **FR-120 [UI] Saved searches.** …`. + +The scan skips two kinds of text: +- **Fenced code blocks.** Lines between ```` ``` ```` / `~~~` fences are + ignored. +- **Inline code spans.** Text between backticks on a line is removed + for the `[UI]` scan only. Titles and block hashes use the **raw** + line, so a backticked span in a title is kept. This lets a PRD talk *about* the marker, as + throughline's own PRD does in FR-89 (`` `[UI]` ``), without marking + anything. + +Every remaining `[UI]` occurrence must sit on a line that matches the +grammar. Otherwise the scan fails with +`ux: malformed [UI] marker at <path>:<line>: <line text, first 120 chars>` +(rc 2). + +Title and boundary matching runs after stripping one leading list +marker. A `^#` line inside a fenced block does not end a block. A +requirement's **block** starts at its title line and runs to the line +before the next requirement title (any line matching the grammar with or +without `[UI]`, i.e. `^\*\*[A-Z][A-Z0-9]*-[0-9]+ `), the next markdown +heading (`^#`), or EOF. Its **hash** is SHA-256 over the block's lines, +with trailing whitespace stripped and joined by `\n`, as lowercase hex. + +### `docs/ux/index.json` schema v1 (FR-91, FR-92, FR-93, FR-94, FR-99) +All paths are relative to `docs/ux/`. They must not be absolute and must +not contain a `..` segment. + +| Field | Type | Rule | +|---|---|---| +| `schema` | int | `1` | +| `prd_rev` | string | `git log -1 --format=%h <integration> -- docs/PRD.md`: the short SHA of the last commit touching the merged PRD the set was made for. It uses the same convention as a TDD's `PRD-rev`, and every UX run sets it | +| `platforms` | array of `web`/`ios`/`android` | non-empty | +| `viewports` | array of `{name, width, height}` | `name` is `[a-z0-9-]+`, unique; width and height are ints in 200–4000 | +| `fidelity` | `high`/`mid`/`low` | `high` is not allowed when `delegates` is empty (FR-92) | +| `delegates` | array of strings | skills or tools **invoked this run**; `[]` means no delegate | +| `design_system` | `{tokens, adr, source}` | `tokens`: `"tokens.css"` or null; `adr`: repo-relative ADR path or null; `source`: `existing-code`/`established`/`none` | +| `requirements` | array of `{id, hash, screens}` | `id` matches the id grammar and is unique; `hash` is 64 lowercase hex; `screens` is non-empty and names existing screen ids. **PRD-consistency (validate only):** `id` is a current `[UI]` id and `hash` equals its current block hash | +| `screens` | array of `{id, title, baseline, states}` | `id` is `[a-z0-9][a-z0-9-]*` and unique; `baseline` is `code-derived` / `live capture (not committed)` / `none (new screen)` | +| `states[]` | `{name, file}` or `{name, na}` | `name` is one of `default`/`empty`/`loading`/`error`, each exactly once; `default` must have a `file`; `file` is `screens/<screen-id>/<name>.html`; `na` is a non-empty reason | +| `flow` | array of screen ids | every screen exactly once, in user-flow order | + +**Screenshots are not in the index.** They are derived from the files on +disk. The expected set is every `screens/<sid>/<state>@<viewport>.png` +for each state with a file and each declared viewport. Whether that set +is complete is *reported*, never stored or enforced. Nothing in the index +can disagree with the PNGs, so neither the skill nor `render` has a +screenshot field to keep in sync. + +Rules not marked "validate only" are **structural**. Every subcommand +that reads an index (`delta`, `validate`, `coverage-check`, `render`, +`index-html`) enforces them, and a violation is rc 2 (invalid index). +The PRD-consistency rules are enforced only by `validate`. `delta` and `coverage-check` exist to *report* PRD drift as +`changed` / `orphaned` / `stale`, never to reject it. + +The skill (TDD 0071) owns `index.json` entirely. The library never +writes it. + +A first run starts from this initial object: +`{"schema":1,"prd_rev":"","platforms":[],"viewports":[],"fidelity":"low","delegates":[],"design_system":{"tokens":null,"adr":null,"source":"none"},"requirements":[],"screens":[],"flow":[]}`. +The skill fills it before the first validate. + +### `scripts/lib/ux.sh` functions +`ux.sh` sources `plugin-root.sh` and `verdicts.sh` (for +`_tl_integration_ref`), failing closed with `ux: cannot source <file>` +(rc 2). It resolves its python files through `tl_plugin_root`. When `python3` is absent and the +input contains `[UI]`, it prints `ux: python3 required` and returns rc 3. + +- **`tl_ux_ui_reqs <prd-path>`.** One line per `[UI]` requirement, + `<id>\t<hash>\t<title>`, in file order. + - rc 0, including empty output when there are no `[UI]` lines; + - rc 1 for an unreadable file (`ux: cannot read <path>`); + - rc 2 for a malformed marker, or a duplicate id + (`ux: duplicate [UI] id <id> at <path>:<l1>,<l2>`); + - rc 3 for missing python3. + + Short-circuit: when `grep -qF '[UI]' <prd-path>` finds nothing, it + prints nothing and returns rc 0 without python3. +- **`tl_ux_delta <prd-path> <index-path>`.** `<index-path>` may be a file + that does not exist (meaning no UX set yet), or `-` (read from stdin). + It prints one line per affected id, in PRD order: + - `new\t<id>\t<title>`: a `[UI]` id that is not in `requirements`; + - `changed\t<id>\t<title>`: in the index, with a different hash; + - `orphaned\t<id>`: in the index, but no longer `[UI]`. `validate` + rejects any orphaned mapping, so the skill always drops it and + reports it in the PR (TDD 0071 step 4). The user is never asked to + keep it. + + Return codes: + - rc 0, including empty output when nothing changed; + - rc 2 for an invalid index (`ux: invalid index <path>: <reason>`) or + a `tl_ux_ui_reqs` failure; + - rc 3 for missing python3. +- **`tl_ux_validate <repo-root>`.** Validates `docs/ux/index.json` + against schema v1 and the PRD at `<repo-root>/docs/PRD.md`, then checks + the files on disk: + - every `states[].file` exists; + - each mock HTML file references no `http:`, `https:` or + protocol-relative `//` URL in `src=`, `href=`, `@import` or `url(` + (checked by regex over the raw text, case-insensitive); + - when `tokens.css` exists, each mock links it through a relative + `<link rel="stylesheet" href="…tokens.css">`; + - every image (`*.png` / `*.jpg` / `*.jpeg` / `*.webp`) under + `docs/ux/` is an **expected** screenshot path: a declared screen, a + state that has a file, and a declared viewport. Anything else is + `unreferenced image`. That covers captures (FR-94), stale PNGs of a + removed viewport or state, and stray files; + - every `*.html` under `screens/` is a listed `states[].file`. A + leftover mock of a state that is now `n/a`, or of a removed screen, + is `unlisted mock (superseded)`. + + Missing expected PNGs are **not** an error, because they are only + reported. It prints + `ok <n> screens, <m> requirements, screenshots <k>/<total>` with rc 0, + where `<k>/<total>` counts the expected PNGs present. Otherwise it + prints one line per problem, `ux-invalid: <path>: <reason>`, with rc 1. + It returns rc 2 when the index is missing or unparseable, and rc 3 for + missing python3. +- **`tl_ux_render <repo-root> <screen-id>…`.** It needs at least one + screen id. Called with none, it returns rc 2 with + `ux: render needs explicit screen ids` and touches nothing. Every id + must exist in the index (rc 2 otherwise). There is no "all" default, + so a run can never re-render or clear screens it did not plan (FR-99). + It renders every state-with-file × viewport for the named screens to + `docs/ux/screens/<sid>/<state>@<viewport>.png`. It works in this fixed + order: + 1. **Clear.** Delete every existing image under the named screens' + directories. This runs before the browser is looked for, so + superseded screenshots never survive (FR-99). + 2. **Find a browser.** + 3. **Render** each shot. + 4. **On failure,** delete only the PNGs this call wrote. + 5. **Report.** The last stdout line is the status of the **whole** + set, from disk: `screenshots: complete` when every expected PNG + exists, otherwise `screenshots not rendered: <reason>`. The reason + is this call's failure or no-browser reason when there was one, + otherwise `partial — <n> missing`. + + `render` never writes `index.json`. Screens it was not asked to render + keep their PNG bytes in every outcome. + - **Browser choice.** It uses the first executable found among + `$THROUGHLINE_UX_BROWSER`, `chromium`, `chromium-browser`, + `google-chrome`, `chrome`. It never passes `--no-sandbox`. Each shot + runs as: + `<browser> --headless=new --disable-gpu --hide-scrollbars --host-resolver-rules="MAP * ~NOTFOUND" --window-size=<w>,<h> --screenshot=<abs png> file://<abs html>`, + under `timeout 60` when `timeout` exists. The resolver rule blocks + network access at render time, so an un-self-contained mock renders + broken. + - **On success:** one `rendered <path>` line per PNG, then the status + line, then rc 0. + - **No browser:** status line + `screenshots not rendered: no headless Chrome on PATH`, rc 4. + - **Failed shot:** a non-zero exit, or a missing or empty PNG, triggers + step 4. Status line + `screenshots not rendered: <browser> failed on <html path> (rc <n>)`, + rc 4. + + rc 4 is a declared degrade, not an error (FR-91). It returns rc 2 for an + invalid index and rc 3 for missing python3. +- **`tl_ux_capture <url> <outdir> <w>x<h>`** (FR-94). It makes one + capture per call. The skill calls it once per existing screen and + viewport, with `<outdir>` = `<capture-dir>/<screen-id>`, which it + creates when it is absent. + - It refuses (rc 2) when `<outdir>` resolves inside a git work tree, + or when `<url>` is not `http://` or `https://`. + - It runs the same browser command as `render`, without + `--host-resolver-rules`, to `<outdir>/capture-<w>x<h>.png`, and + prints the path. A capture is reference only, and this function + writes no index. + - rc 0 means captured. rc 4 means no browser was found or the capture + failed, with `capture failed: <reason>` printed. rc 2 means the + arguments were refused. +- **`tl_ux_index_html <repo-root>`.** Regenerates `docs/ux/index.html` + from the index and the PNG files present. It is deterministic: the + same inputs give the same bytes. The page contains: + - a provenance header: PRD rev, platforms, viewports, fidelity, + delegates (or `none: degraded`), design-system source, and the + screenshot count `<k>/<total>` from disk; + - the flow, one ordered `<a href="screens/<sid>/default.html">` per + screen, followed by links to the screen's other state files; + - a requirement → screens table; + - a **missing screenshots** list: every expected PNG absent on disk, + or `none`. A partial set never looks complete. + + It escapes every value from the index with `html.escape`. It returns + rc 0, rc 2 for an invalid index, and rc 3 for missing python3. This + page is the flow index of FR-91. +- **`tl_ux_merged_index <repo-root>`.** Prints + `git show <integration>:docs/ux/index.json` to stdout. `<integration>` + comes from `_tl_integration_ref`: `THROUGHLINE_INTEGRATION_BRANCH`, + then `origin/HEAD`, then `main`, then `master`. + - rc 0: the blob was printed; + - rc 1: no index on the integration branch yet, so nothing is printed; + - rc 2: no integration ref resolves (`ux: no integration branch`). + + It first runs a best-effort `timeout 20 git fetch -q origin`, at most + once per process (a marker variable). `THROUGHLINE_UX_NOFETCH=1` skips + it; `tdd-lint` sets this so the pre-pass never reaches the network. + A fetch failure is ignored, and the fetch does nothing without an `origin` remote. This + keeps a merge on the host from being misread as "no merged UX set". It + writes `ux: merged index from <ref> @ <short-sha>` to stderr, so a + stale read is visible. It needs no python3. +- **`tl_ux_coverage <repo-root> <id>…`.** Reads the **merged** index + through `tl_ux_merged_index`. For each id it prints one of: + - `covered\t<id>\tdocs/ux/screens/<sid>/[,…]`; + - `stale\t<id>\thash differs from merged index` when the merged hash differs from the current + working-tree PRD block (the UX set predates the change); + - `uncovered\t<id>` when the id is absent from the merged index, or no + merged index exists. + + It returns rc 0 when every id is covered and rc 1 when any id is stale + or uncovered. It returns rc 2 when the blob exists but is invalid, or + when no integration ref resolves (`ux: no integration branch`), and + rc 3 for missing python3. A UX set that exists only on a branch never + counts (FR-101). + +### Python entry points +`ux_record.py` holds the subcommands `ui-reqs`, `delta`, `validate` and +`coverage-check`. `coverage-check` takes the merged blob on stdin and the +PRD path; the git read stays in bash. `ux_render.py` holds `render`, +`capture` and `index-html`. Each one: +- writes its results to stdout; +- writes `ux: …` diagnostics to stderr; +- exits with the codes above; +- runs any unexpected exception through a top-level handler that prints + `ux: internal error: <type>: <msg>` and exits 2, never 0. + +## Data & state +The only persistent state is the committed `docs/ux/` tree: `index.json`, +`index.html`, `tokens.css`, `screens/<sid>/*.html` and +`screens/<sid>/*.png`. The library never writes `index.json`. It writes +PNGs only from `render`, and `index.html` only from `index-html` (through +a temp file and `os.replace`). Everything else under `docs/ux/` is +written by the skill (TDD 0071). + +## Sequencing / implementation plan +1. Write `ux_record.py` with `ui-reqs` (grammar, fence/inline-code + skipping, block hashing) and `delta`, plus their `ux.sh` wrappers with + the `[UI]` short-circuit and the python3 check. +2. Add `validate` (schema, file checks, offline-URL scan, tokens link, + unreferenced images) and `coverage-check`, plus the `tl_ux_validate` / + `tl_ux_coverage` wrappers (integration-ref resolution, `git show`). +3. Write `ux_render.py` with `render` (browser discovery, per-shot + command, scoped cleanup, whole-set status line) and + `index-html`, plus their wrappers. + +## Failure modes & edge cases +**Real risks** +- *`[UI]` in PRD prose.* Inline code and fences are skipped. Any other + stray `[UI]` fails loudly (rc 2 with the line), so a requirement is + never silently lost. +- *Other id schemes.* Any `PREFIX-N` id works (`FR-12`, `R-3`, `UX-4`). + An id of another shape that carries `[UI]` fails loudly. +- *CDN links from a delegate.* `validate` names the file and the URL, + and the render-time resolver rule shows the broken result. +- *A browser hangs.* `timeout 60` turns it into a failed shot, which + gives the scoped cleanup and rc 4. +- *No integration ref* (fresh clone): coverage gives rc 2. It never + treats the working tree as merged. + +**Overblown risks** +- *Hash churn.* Trailing whitespace is stripped, and any other edit is + meant to count as `changed`. +- *PNG size.* Accepted in the PRD (FR-99, no LFS). Re-renders replace + files rather than adding them. + +**Unspoken risks** +- *Hash agreement.* A hash copied wrongly into the index is rejected by + `validate`. It is never committed and later misread as `stale`. +- *A partial screenshot set looks complete.* Prevented three ways: + - the cleanup lists only PNGs that exist; + - completeness is computed from disk, never stored; + - `index.html` lists the missing PNGs. +- *`index.html` injection.* Delegate-supplied titles are escaped. + +## Verification plan +- **Surface:** stdout, stderr and rc of each `tl_ux_*` function; the + bytes of `docs/ux/index.json` and `index.html`; the PNG files on disk. +- **Harness:** temp git repos with a fixture `docs/PRD.md`, run through + the sourced `ux.sh` with `env -i` plus `PATH`. A stub browser + (`THROUGHLINE_UX_BROWSER` pointing at a script that writes a 1×1 PNG + and logs its argv) is used. When a real Chrome/Chromium is on PATH, one + real render is also run. Every negated assertion first asserts that its + file is readable (L-001/L-011), and every temp directory is + trap-cleaned (L-004). +- **Observation points → expected (PASS):** + 1. **Grammar.** A PRD with `**FR-1 [UI] Login.**`, `**R-7 [UI] Cart**`, + a backticked `` `[UI]` `` in prose, and a fenced `[UI]` → exactly + two lines, `FR-1` then `R-7`, rc 0. + 2. **Loud miss.** A bare `see [UI] below` line → rc 2, and stderr names + `<path>:<line>`. A duplicate `FR-1` → rc 2 `duplicate`. + 3. **Short-circuit.** A PRD with no `[UI]` and `PATH` without python3 + → rc 0, empty output. The same PRD with one `[UI]` → rc 3 + `ux: python3 required`. + 4. **Throughline's own PRD.** `tl_ux_ui_reqs docs/PRD.md` on this repo + → rc 0, empty output: every mention there is backticked. + 5. **Delta.** + - No index → all `new`. + - Index holding FR-1's current hash → FR-1 absent from the output. + - Editing FR-1's body text → `changed FR-1`. + - Removing FR-1's marker → `orphaned FR-1`. + - A malformed index → rc 2. + 6. **Validate.** One valid fixture set → + `ok 1 screens, 1 requirements, screenshots 0/1`, + rc 0. Each of these mutations gives rc 1 with a `ux-invalid:` line + naming it: + - a mock with `<script src="https://cdn…">`; + - `url(//fonts…)`; + - a missing `tokens.css` link when `tokens.css` exists; + - a `states` entry missing `error`; + - `fidelity: "high"` with `delegates: []`; + - a wrong hash; + - a `requirements` entry whose id is no longer `[UI]` (orphaned); + - a PNG for an undeclared viewport (`unreferenced image`); + - a leftover `screens/a/error.html` whose state is now `n/a`; + - a stray `docs/ux/capture.png` (`unreferenced image`); + - a `..` path. + 7. **Render (stub).** + - Two viewports × two state files → four `rendered` lines and four + PNGs. + - The last line is `screenshots: complete`, the index bytes are + unchanged, and `validate` reports `screenshots 4/4`. + - The stub's logged argv contains `--host-resolver-rules` and no + `--no-sandbox`. + 8. **Render scoping (FR-99).** After rendering both screens, re-render + only `a` with a changed mock → `a`'s PNGs are rewritten, and `b`'s + PNG bytes and mtimes are unchanged. Delete `a`'s `error.html` from + the index → its old PNG is gone after the re-render. + 9. **Render degrade.** + - No browser on PATH → rc 4, and the last line is exactly + `screenshots not rendered: no headless Chrome on PATH`. + - A stub that exits 1 on the second shot → rc 4, and no PNG written + by this call remains. + - **Two screens.** Render `a` and `b`, then re-render only `a` with + a failing stub: + - `a`'s old PNGs are gone; + - `b`'s bytes are unchanged; + - `validate` gives rc 0 with `screenshots 2/4` (one state per + screen); + - `index.html` lists `a`'s two missing PNGs. + - **Explicit ids.** `tl_ux_render <repo>` with no ids → rc 2, the + message above, and no file changes. An unknown id → rc 2. + - **Pre-render validate.** A second run adds screen `c` (HTML only, + no PNGs) to a set whose other PNGs are complete → `validate` rc 0 + with `screenshots 4/6`. Rendering `c` → `screenshots: complete`. + - **Viewport removed.** Drop a viewport from the index → + `validate` rc 1, `unreferenced image` for each old PNG. Rendering + every screen clears them → rc 0. + - **No browser, scoped.** In the two-screen setup, re-render `a` + with no browser → `a`'s PNGs are gone, `b`'s remain, and + `validate` gives rc 0. + 9b. **Capture.** + - `tl_ux_capture http://127.0.0.1:<port>/ <tmp-outside-repo> 390x844` + against a local `python3 -m http.server` → rc 0, and a PNG + exists in the out dir. + - The same call with `<outdir>` inside the temp repo → rc 2. + - A `file://` URL → rc 2. + - The stub's argv has no `--host-resolver-rules`. + 10. **Real render** (when Chrome is present): one mock at 390×844 → a + PNG whose IHDR width/height read 390×844. + 11. **index.html.** + - Deterministic: two runs give identical bytes. + - A screen titled `<img src=x onerror=alert(1)>` appears escaped. + - Links appear in `flow` order. + - The provenance block names the delegates, or + `none: degraded`. + 12. **Coverage.** Commit index A on `master`, and index B (which adds + FR-2) on a branch, with the branch checked out. + - `tl_ux_coverage . FR-1 FR-2` → `covered FR-1 docs/ux/screens/…/` + and `uncovered FR-2`, rc 1. + - Edit FR-1's text in the working-tree PRD → `stale FR-1`. + - `THROUGHLINE_INTEGRATION_BRANCH=nope` with no main/master/ + origin → rc 2 `ux: no integration branch`. + 13. **Internal error.** Feed `delta` a binary index → rc 2, + `ux: invalid index …` or `ux: internal error …`, never rc 0. + +## Evaluation rubric +| Criterion | High-quality | Acceptable | Failing | +|---|---|---|---| +| requirement traceability | Every FR-89..101 (+79/81/86/87, NFR-1/3) maps to a named ux.sh function, skill block, agent rule, or lint check | One mapping indirect but named | An in-scope FR untraced | +| interface concreteness | Every ux.sh function has args, stdout, rc pinned; index.json schema field-by-field | One rc implicit | Reader cannot tell what a function prints | +| alternatives-analysis substance | Chrome CLI vs Playwright, JSON vs MD index, hash vs git-diff delta, tokens.css vs JSON each with reason | One rejection thin | None named | +| verification-plan actionability | Functions driven in temp repos; skill blocks extracted and run; negated checks assert readability first (L-001/L-011) | One check text-only with stated reason | Behaviour asserted only by grepping prose | +| scope-bound adherence | Each TDD <=8 files, <=500 body lines, padded estimates; exceptions declared | One justified exception | Over a bound unexplained | +| naming consistency | ux.sh names, index.json fields, screen-path form identical across 0070-0072 | One alias reconciled | Same concept named two ways | +| delegation, not reinvention | Skill invokes present delegates by role; ux.sh holds only mechanics, no design logic | Delegates named only as examples | throughline code generates visual design | +| fail-loud parsing | Malformed [UI] marker, unreadable index, awk failure each exit non-zero with a named message | One path returns empty silently but is tested | A parse failure reads as no UI requirements | + +## Requirement traceability +| Requirement | Design element | +|---|---| +| FR-89 `[UI]` marker is the only signal | `[UI]` grammar + `tl_ux_ui_reqs` (inline-code/fence skipping, loud miss); obs 1–4 | +| FR-90 delta-driven scope | per-requirement block hash + `tl_ux_delta` (`new`/`changed`/`orphaned`); obs 5 | +| FR-91 artifact set | index schema v1 (states incl. `n/a`, flow, viewports, provenance, baseline); `tl_ux_validate` offline-URL scan; `tl_ux_index_html` flow index; `tl_ux_render` with declared rc-4 degrade; obs 6, 7, 9–11 | +| FR-94 captures never committed | `validate`'s `unreferenced image` rule; `tl_ux_capture` refuses an in-repo out dir; `baseline` enum; obs 6, 9b | +| FR-99 point-in-time, superseded | `tl_ux_render` scoped deletion and re-render; untouched screens byte-identical; obs 8 | +| FR-101 merged-mock check (query side) | `tl_ux_merged_index` + `tl_ux_coverage` read the integration-branch blob; `stale`/`uncovered`; obs 12 | +| NFR-4 verdict honesty | rc 4 degrade distinct from rc 1/2 failures; internal errors exit 2; obs 9, 13 | + +Skill-side FR-90/91/94/99 is TDD 0071; the FR-101 refusal is TDD 0072. + +## Dependencies considered +- **python3 stdlib (chosen)** for nested JSON, SHA-256, regex HTML + scanning and atomic replace, in one implementation. + - *Rejected: jq + awk + sha256sum.* Awk block and URL parsing repeats + the silent-empty-parse class (L-005), and the logic splits across + three tools. + - *Rejected: a jq → python3 cascade (`drafts.sh` style).* Two + implementations to keep identical. Python is needed only when + `[UI]` appears. +- **Headless Chrome/Chromium CLI (chosen):** an existing binary with + stable `--screenshot` / `--window-size` flags and no install step. + - *Rejected: Playwright via `npx`.* It downloads browsers, needs Node + in non-JS repos, and makes throughline an installer (FR-83, ADR + 0010). + - *Rejected: a delegate's screenshot tool.* It varies by harness. +- **JSON index (chosen):** a strict schema a validator can reject. + *Rejected: `docs/ux/INDEX.md`.* Hand-edited tables parse fragilely, + and the generated `index.html` already covers readability. +- **`tokens.css` custom properties (chosen):** offline mocks link it + directly, with no build step. *Rejected: `tokens.json` (W3C / Style + Dictionary).* Every mock would need a CSS generation step. It is more + portable to design tools, but that is not needed for an HTML record. +- **Text-hash change detection (chosen).** *Rejected: git-diffing the + PRD at the recorded rev.* It breaks on rewritten or squashed history, + and mapping hunks to blocks needs a second parser. + +## PRD conflicts surfaced (and resolution) +- FR-92's acceptance says the UX index "says `delegates: none`". This + design writes `"delegates": []` in `index.json` and renders it as + `none: degraded` in `index.html` and the PR body. Runtime-verify + should read FR-92 as "the index records no delegate": an empty array + in the JSON, and `none: degraded` on the human surfaces. +- FR-89's acceptance says `grep -n '\[UI\]' docs/PRD.md` "lists exactly + the marked requirements". That is false for any PRD that mentions the + marker in backticked prose, including throughline's own. The + authoritative list is `tl_ux_ui_reqs`, which skips inline code and + fences. +- FR-91's acceptance says "networking disabled". The PRD leaves how to + enforce that open. This design checks it twice: statically in + `validate`, and dynamically through the resolver rule at render time. +- The PRD's cascade note (#191) expected supersession of the + `/prd-author` and `/tdd-author` TDDs. FR-89–FR-101 are new + requirements, and the existing FRs those TDDs cover are unchanged, so + this pass adds new TDDs and supersedes none. + +## Decisions to promote (ADR candidates) +- Promoted: ADR 0017 (UX record: in-repo, self-contained HTML, delegated by + role, design input not a build gate), added in this design PR. + +## Touched files +- `scripts/lib/ux.sh` — `tl_ux_*` wrappers, `[UI]` short-circuit, python3 check, integration ref + `git show` +- `scripts/lib/ux_record.py` — `ui-reqs`, `delta`, `validate`, `coverage-check` +- `scripts/lib/ux_render.py` — `render`, `capture`, `index-html` +- `tests/ux-record.test.sh` — obs 1–6, 12, 13 +- `tests/ux-render.test.sh` — obs 7–11, 9b + +## Expected diff size +- `scripts/lib/ux.sh` — 140 lines +- `scripts/lib/ux_record.py` — 290 lines +- `scripts/lib/ux_render.py` — 250 lines +- `tests/ux-record.test.sh` — 290 lines +- `tests/ux-render.test.sh` — 290 lines + +Total expected diff: 1260 lines across 5 files. diff --git a/docs/tdd/0071-ux-author-skill.md b/docs/tdd/0071-ux-author-skill.md new file mode 100644 index 0000000..051491a --- /dev/null +++ b/docs/tdd/0071-ux-author-skill.md @@ -0,0 +1,506 @@ +# TDD 0071: `/ux-author` — the UX phase skill and its independent critique + +Status: draft +PRD refs: FR-90, FR-91, FR-92, FR-93, FR-94, FR-95, FR-96, FR-97, FR-98, FR-99, FR-100, FR-22, FR-79, FR-81, FR-86, FR-87, NFR-1, NFR-3 +PRD-rev: 3be6232 +ADR constraints: 0004, 0006, 0010, 0011, 0015, 0017 + +## Approach +`/ux-author` is a new interactive authoring skill between a merged PRD +update and `/tdd-author`. It has its siblings' shape, so a user who knows +`/tdd-author` can predict it: a crash-safe draft, the FR-86 check, an +interrogated plan, a co-created rubric, a fresh-worker critique, and a +phase-gate PR it never merges. + +throughline owns the **contract**: what gets mocked, the `docs/ux/` +record (TDD 0070's schema), traceability, the critique and the gate. +**Making** the mocks is delegated by **role** to whatever skills and +tools the session has. It reimplements none of them, and it degrades +honestly when none exists (FR-92). The roles are: +- `design`: UI mocks; +- `design-system`: tokens; +- `critique`; +- `accessibility`. + +Every mechanical step is a TDD 0070 function called from a `tl:` block, +so runtime-verify can execute the steps. The draft, interrogator and +rubric rules are read from `skills/prd-author/SKILL.md` and applied with +the draft name `ux-author`, not copied (~150 lines). With one source, the +skills cannot drift apart. + +## Components & interfaces + +### `skills/ux-author/SKILL.md` (new) +Frontmatter: `name: ux-author`, and a description naming the trigger +("after a merged PRD update that adds or changes `[UI]` requirements"). +The steps: + +0. **Resume + FR-86.** The resume check is identical to `/prd-author`'s + step 0, with skill name `ux-author` and the same degraded-mode and + untrusted-draft rules. The `<!-- tl:fr86-check -->` block is + **byte-identical** to the other skills' copies. +1. **Preflight — `<!-- tl:ux-preflight -->`** (inputs `TL_REPO`). It + sources `plugin-root.sh` and `ux.sh`, then: + - fails (rc 1) when `docs/PRD.md` has uncommitted changes: + `throughline: commit docs/PRD.md first`; + - **then** writes `tl_ux_merged_index "$TL_REPO"` to a temp file + (this call does the fetch). rc 1 (no merged index yet) leaves the + file absent, so every `[UI]` id is `new`; rc 2 is shown and stops + the skill; + - **then** fails (rc 1) when `git rev-parse HEAD:docs/PRD.md` differs + from `<integration>:docs/PRD.md`: + `throughline: docs/PRD.md differs from the merged PRD on <ref>; merge the PRD PR (or pull the integration branch) and run /ux-author from it`. + This makes the delta always describe the merged PRD (FR-90); + - runs `tl_ux_delta "$TL_REPO/docs/PRD.md" <temp>`. + + Empty output means it prints + `no UI-bearing requirements in this PRD delta` and exits 0. The skill + then **stops**: no branch, no draft, no PR. Otherwise the block prints + the delta lines. Any rc 2 or 3 is shown and stops the skill. +1b. **Draft screen plan, shown first (FR-90).** Before any question, the + session shows a draft plan built mechanically from the delta: + - one provisional screen per `new` / `changed` id, with id + `<id-lowercased>` (e.g. `fr-120`) or the merged screen it already + maps to; + - all four states as `?`; + - the platform-default viewports (web until step 2 says otherwise); + - the requirement id. + + This satisfies FR-90's "the first thing shown is a screen plan naming + that requirement id". Steps 2–4 refine this draft. Nothing is + approved yet. +2. **Interview (FR-75 posture, FR-100).** The session reads the + "Interrogator discipline (FR-75)" section of + `$(tl_plugin_root)/skills/prd-author/SKILL.md` and applies it with + `ux-author` as the draft skill. That gives the running assumptions + list, `tl_draft_append_elicit ux-author …` per answer, and + resolution-or-waiver before completion. It asks four things: + - the target platforms (`web` / `ios` / `android`); + - the viewports, which are global to the UX set. Defaults: web is + `desktop` 1440×900 plus `mobile` 390×844; iOS is `phone` 390×844; + Android is `phone-android` 412×915. When a merged set exists, its + viewports are kept unless the user changes them. A change puts every + screen into `TL_SCREENS`, so the whole set is **re-rendered** + (clearing PNGs of removed viewports) before the first validate. The + HTML of screens outside the delta is not re-mocked and stays as + merged; + - the baseline (FR-94): for each existing screen, it is derived from + code. The user may give a running-app URL; + - the design-system situation (FR-93). +3. **Delegate discovery (FR-92).** The session lists, from the skills and + tools it can see in this session, a candidate for each role. + Examples on Claude Code: + - `design`: `frontend-design`; + - `design-system`: `design:design-system`; + - `critique`: `design:design-critique`; + - `accessibility`: `design:accessibility-review`. + + On Grok Build, any installed SKILL.md or MCP tool whose description + fits the role. The skill names roles; the examples are illustrations, + never requirements. The discovery result is shown to the user. When + more than one candidate fits a role, the user picks one through a + structured question; throughline sets no precedence among vendors. + When none fits, the role is `none` and its work falls back to the + session (the FR-92 degrade). Only delegates that are actually + **invoked** later are recorded in `delegates`. Which delegates exist + on Grok Build was not inventoried (PRD open question). The degrade + path makes the skill complete there either way. +4. **Screen plan (FR-90, FR-96).** A table is presented as a structured + question (approve / change). Its columns are screen id, title, states + (each `default`, `empty`, `loading`, `error`, or `n/a: <reason>`) and + requirement ids. The set's viewports are listed once above the table. Every delta `new` / `changed` id must + appear. Each `orphaned` id's mapping is always dropped (TDD 0070's + `validate` rejects orphans) and listed in the PR under + "Requirement gaps" as `no longer [UI]: <id>`. The user is not asked to + keep it. + The approved plan is appended to the draft as a `decision`. +5. **Rubric (FR-77, FR-100).** The session reads the "Rubric + co-creation (FR-77)" section of `/prd-author` and applies it, with + header `rubric: UX-set`. It is seeded with five criteria: + requirement → screen coverage, state completeness, accessibility + basics, design-system consistency, and fidelity versus declaration. + The approved table is written to `docs/ux/RUBRIC.md` (replaced each + run). The critique and the PR cite it. +6. **Branch + design system (FR-93).** The skill branches + `docs/ux/<slug>` off the integration branch, then handles the design + system, checking the cases in this order: + 1. **`tokens.css` already merged**: it is reused unchanged. A change + goes through a new ADR and is never made silently. + 2. **Existing** design system in code: the `design-system` delegate, + or the session, derives `tokens.css` from the code's theme. + `source: existing-code`. + 3. **None**: it establishes `tokens.css` and runs `/adr-new` for the + ADR "Design system: <name>", written `accepted`. As with ADR 0017, + merging the UX PR is the acceptance. `source: established`, and + `adr` is set. +7. **Live capture (FR-94, optional).** For each existing screen, the + session asks for that screen's URL in the running app (blank skips + it). It calls + `tl_ux_capture <url> "<capture-dir>/<sid>" <w>x<h>`. `<capture-dir>` + comes from one `mktemp -d` and is recorded in the draft (as is the + review report path), so it survives separate blocks and a resume: + (TDD 0070) once per viewport. That function refuses an out dir inside + the repo. On rc 2 or rc 4 the screen's baseline stays `code-derived`, + and the reason is shown. The index claims + `live capture (not committed)` only for screens with at least one + successful capture. It views the captures as reference only. They are never copied + under `docs/ux/` (`tl_ux_validate` rejects any unlisted image), and + the directory is removed at step 11. +8. **Mocks (FR-91, FR-92, FR-95).** For each screen, the session invokes + the `design` delegate, or authors the mocks itself when there is + none. Fixed instructions go with each call: + - one self-contained HTML file per state at + `docs/ux/screens/<sid>/<state>.html`; + - link `../../tokens.css`; + - no `http(s)`/`//` URLs. Raster images are inlined as `data:` URIs + or drawn as SVG; only fonts and SVG may be vendored under `docs/ux/` + (validate rejects any other raster there); + - real copy; + - platform conventions for the target (iOS HIG / Material); + - every requirement gap rendered as a visible placeholder carrying + `data-ux-gap="<one line>"`. + + The session then updates `docs/ux/index.json` by **read, merge, + write**. It starts from the branch's working `index.json` when one + exists (a loop back from step 9). Otherwise it starts from the merged + index, or from TDD 0070's initial object when there is none. It + replaces only the entries for the planned screens and delta ids, and + keeps every other screen and requirement entry byte-for-byte, so + earlier `[UI]` ids stay covered. Screenshots are not in the index + (TDD 0070). The write sets: + - `prd_rev` to `git log -1 --format=%h <integration> -- docs/PRD.md`; + - hashes copied from the `tl_ux_ui_reqs` output; + - `delegates`: the invoked set; + - `fidelity`: `high` only with a `design` delegate, otherwise the + level actually reached. `validate` checks only that `delegates` is + non-empty, because it cannot know roles. Accepted gap: the reviewer + grades `high` that has no `design`-role delegate; + - `baseline` per screen. + + It runs `<!-- tl:ux-validate -->`, which calls + `tl_ux_validate "$TL_REPO"`, and fixes and reruns until the result is + rc 0. When step 2 changed the viewports, the session runs + `tl:ux-render` with every screen id **before** this validate. Missing + PNGs never fail validate; it reports + `screenshots <k>/<total>`. The session lists each `data-ux-gap` it + placed. It never edits + `docs/PRD.md`. +9. **In-session review (FR-96).** `<!-- tl:ux-render -->` takes inputs + `TL_REPO` and `TL_SCREENS`. `TL_SCREENS` is **exactly** the approved + plan's screen ids (or the single screen being redone, or every screen + id when step 2 changed the viewports), + separated by spaces and read as `${TL_SCREENS:?…}`. An empty value + fails the block, and `tl_ux_render` refuses an empty id list (TDD + 0070), so merged screens outside the plan are never re-rendered or + cleared (FR-99). The block calls `tl_ux_render "$TL_REPO" $TL_SCREENS`. + rc 4 is a declared degrade. + Then, for each screen set, the session: + - shows the PNG paths, or the HTML paths when nothing was rendered; + - asks a structured question: approve / request changes / drop. + + Changes loop back to step 8 for that screen and re-render only it. + Drop depends on the screen: + - a **new** screen is removed, with its files and index entries; + - a screen that **exists in the merged set** is reverted with + `git checkout <integration> -- docs/ux/screens/<sid>`, which + restores its HTML and PNGs together, and with its merged index + entry. Only the delta ids' mappings to it are removed, and a + requirement left with no screen is removed entirely. It then + reaches step 10 as unmapped. Under changed viewports, the restored + screen is re-rendered. + + Because screenshots are derived from disk, both paths leave the index + and the PNGs consistent. `tl_ux_validate` runs again after any drop. + Each approval is appended to the draft as a `decision`. A `[UI]` id + left with no approved screen is a blocking gap, reported in step 10. +10. **Critique (FR-97).** The skill dispatches **one** fresh worker using + `agents/ux-reviewer.md`: + - no model parameter, so it inherits (FR-87); + - it must not spawn children; + - an empty report file from `mktemp` (its path recorded in the draft), + whose path is passed in the dispatch prompt. + + The last line of the **report file** (not the transcript, per ADR + 0011) is authoritative, and it must be exactly `UX_REVIEW: PASS` or + `UX_REVIEW: BLOCK <reason>`. A missing or other last line counts as + BLOCK. On BLOCK, the skill + fixes the set (back to 8 or 9) and dispatches a **new** worker. The + user may instead record a waiver with a rationale. The verdict is + never written to the draft (as FR-50). +11. **Close-out + PR (FR-98, NFR-1).** The skill: + - runs `tl_ux_index_html` and `tl_ux_validate` one last time; + - removes the capture dir and the review report file; + - commits `docs/ux/` and any FR-93 ADR; + - pushes and runs `gh pr create` against the integration branch. + Under "skip git" it commits nothing, and it writes the would-be PR + body to `${TMPDIR:-/tmp}/tl-ux-pr-body-<pid>.md` and prints that + path. + + PR-body sections: **Open assumptions & waivers** (from the draft); + **UX critique** (verdict, findings, waivers); **Delegates & fidelity** + (`delegates` or `none: degraded`, and `fidelity`); **Requirement + gaps** (each `data-ux-gap`, or "none found"); **Approved screen + sets**; and **Screenshots**, with the images embedded via + `https://github.com/<owner>/<repo>/blob/<commit-sha>/docs/ux/<png>?raw=true` + (the pushed commit, so links survive branch auto-deletion), + plus a status line built from the final `tl_ux_validate` count: + `screenshots: complete` when k equals the total, otherwise + `screenshots not rendered: <last render reason from the draft, or partial — <n> missing>`. + Under "skip git", images use local paths; + - a note that `docs/ux/RUBRIC.md` is co-created. + + It never merges. It then runs `tl_draft_discard ux-author` and tells + the user to merge the PR and then run `/tdd-author`. + +### `agents/ux-reviewer.md` (new) +Frontmatter: `name: ux-reviewer`, `tools: Read, Grep, Glob, Bash`, +`model: inherit`. + +It is told that it did not author the set, and that `delegates` is +self-reported. It reads: +- `docs/PRD.md` (the `[UI]` requirements from `tl_ux_ui_reqs`); +- `docs/ux/index.json` and `RUBRIC.md`; +- `tokens.css`; +- every mock, and every PNG (image input where the harness supports it, + otherwise the HTML). + +It runs `tl_ux_validate` first. A non-zero result is an immediate +`UX_REVIEW: BLOCK validate — <first line>`. + +Then it checks the following. **Blocking:** +1. Every in-scope `[UI]` id maps to a screen. This is computed + mechanically: `tl_ux_delta docs/PRD.md docs/ux/index.json` on the + working index. Any `new` line is an unmapped id, which gives + `UX_REVIEW: BLOCK unmapped — <id>`. +2. Every screen has `default` plus each of `empty`/`loading`/`error` as + a file or an `n/a` with a real reason. A boilerplate reason ("not + needed") is a finding. + +**Findings with severity, blocking only when they make a screen +unusable:** +- Accessibility basics: text contrast against the `tokens.css` colours, + labelled inputs, a logical focus order in the DOM, and touch targets + of at least 44×44 CSS px on phone viewports. +- Design-system consistency: values are `var(--…)` from `tokens.css`, + not new literals. +- Visual quality judged against the **declared** fidelity, not against + the claimed delegate. + +It uses the `critique` / `accessibility` delegates when present. It +cites `RUBRIC.md` criteria in its findings. It **writes its full +report to the report-file path given in the dispatch prompt**, ending in +exactly `UX_REVIEW: PASS` or `UX_REVIEW: BLOCK <reason>`. It also +returns the same text. + +### Other surfaces +- `README.md`: a `/ux-author` subsection under "Workflow" (trigger, + delegate roles, degrade path, python3, headless Chrome). +- `.claude-plugin/plugin.json`: the description names the optional + `/ux-author` phase. The version is bumped per the repo's own + convention. +- `tests/plugin-root.test.sh` [K] and `tests/parent-session-check.test.sh` + `NAMES`: both gain `ux-author`, so FR-81 and FR-86 are enforced on the + new skill. + +## Data & state +- **Draft:** `$(tl_draft_path ux-author)` (`drafts.sh`): answers, + `assumption:` entries, the plan and approvals (`decision`), and the + rubric (`rubric: UX-set`). +- **Work state on the branch:** the files under `docs/ux/` on + `docs/ux/<slug>`. A resumed session checks out that branch, when it + exists, before step 8. +- **Transient:** the capture dir and review report under + `${TMPDIR:-/tmp}`, removed at close-out, never committed. + +## Sequencing / implementation plan +1. Write `skills/ux-author/SKILL.md` with steps 0–11 and the `tl:` + blocks `fr86-check`, `ux-preflight`, `ux-validate` and `ux-render`. + Add `ux-author` to the FR-81 [K] list and the FR-86 `NAMES` list. +2. Write `agents/ux-reviewer.md`. +3. Update the README and the `plugin.json` description, and write + `tests/ux-author-skill.test.sh`. + +## Failure modes & edge cases +**Real risks** +- *A delegate ignores "self-contained".* Step 8 loops on `validate`, + and the reviewer re-runs it and BLOCKs, so a CDN-linked set never + reaches the PR. +- *Over-claimed fidelity.* `validate` refuses `high` with no delegates, + and the reviewer grades the declaration. Accepted residual: provenance + is self-report. +- *No renderer:* HTML only (rc 4), reason in the PR. *Invented requirements.* Gaps appear as mock markers and in a PR + section. `docs/PRD.md` is never in the UX diff (obs 9). +- *An unmerged UX PR.* Coverage reads only the integration blob. + +**Overblown risks** +- *Harness-specific delegates* (roles are abstract); *SKILL.md bloat*. + +**Unspoken risks** +- *Two concurrent UX PRs that each establish a design system.* The second + hits a git conflict on `tokens.css` and an ADR number collision. The + README says "one UX PR at a time". +- *Reading `/prd-author`'s sections couples the skills.* That is + intended (a single source), and obs 3 fails CI if a referenced heading + is renamed. + +## Verification plan +- **Surface:** stdout, stderr and rc of the extracted `tl:` blocks; git + state; agent-file text where behaviour is model-driven. +- **Harness:** temp repos (`master` + fixture PRD). Blocks are extracted + by marker and run under `env -i`. A missing marker is a FAIL (L-001), + and every negated grep asserts that its file is readable first. +- **Observation points → expected (PASS):** + 1. **Preflight, no UI.** A PRD without `[UI]` → exactly + `no UI-bearing requirements in this PRD delta`, rc 0, no `docs/ux/*` branch. + 2. **Preflight, delta.** `**FR-1 [UI] Login.**`: + - no merged index → `new\tFR-1\tLogin`; + - a merged index covering FR-1's hash → the no-UI line; + - FR-1 edited → `changed\tFR-1\tLogin`. + - An uncommitted PRD edit → rc 1 with `commit docs/PRD.md first`. + - A committed PRD edit on a branch that is not yet on `master` → + rc 1 with `differs from the merged PRD`. + 3. **Referenced rules exist.** `skills/prd-author/SKILL.md` contains + the headings `### Interrogator discipline (FR-75)` and + `### Rubric co-creation (FR-77)`, and `ux-author/SKILL.md` names + both (a text check by necessity: prose cross-reference). + 3b. **Resume carries the plan (FR-100).** With + `CLAUDE_PLUGIN_DATA=<tmp>`, append the plan as a `ux-author` + `decision`. `tl_draft_exists ux-author` → rc 0, and + `tl_draft_read` contains the plan, which is what step 0 offers. + 4. **FR-86 bytes.** The `tl:fr86-check` block in `ux-author` is + byte-identical to `/prd-author`'s. The extended + `parent-session-check.test.sh` runs it under a fixture + light-tier transcript and gets the light-tier warning line. + 5. **FR-81.** The extended [K] scan: no vendor tool names; uses `tl_plugin_root`. + 6. **Validate block.** A CDN `<script>` → rc 1 and `ux-invalid:`; clean → rc 0. + 7. **Render block, degrade + scoping.** + - With `PATH` lacking any browser and `TL_SCREENS=a`, the extracted + `tl:ux-render` block → rc 4 and + `screenshots not rendered: no headless Chrome on PATH`. + - With merged screens `a` and `b` (PNGs present) and + `TL_SCREENS=a`, `b`'s PNG bytes and index entries are unchanged + after the block. + - `TL_SCREENS` empty or unset → non-zero rc, and nothing changes. + 7b. **Draft plan first.** Step 1b precedes any structured question (a + text check: prose ordering). Preflight's output names `FR-1`, + which is the draft plan's input. + 8. **Reviewer contract** (text check: model prose). `ux-reviewer.md` + has `model: inherit`, both `UX_REVIEW:` tokens, the write-report- + to-dispatched-path rule, `tl_ux_validate` first, and self-reported + `delegates`. + 9. **Gaps never edit the PRD.** Every `PRD.md` use in the extracted + blocks is read-only (`tl_ux_*` args, `git status` / `rev-parse`). + 10. **Regression.** All existing evals stay green. + 11. **Live run** (runtime-verify, nontrivial). In a scratch repo with + one `[UI]` requirement and no design system, run `/ux-author` to + the PR step with `skip git`, then observe: + - the printed PR-body file has the sections Open assumptions & + waivers, UX critique, Delegates & fidelity, Requirement gaps and + Approved screen sets (FR-98), and repeats `delegates` and + `fidelity` (FR-92); + - `docs/ux/tokens.css` and a new `docs/adr/NNNN-*` exist (FR-93); + - `tl_ux_validate` gives rc 0, and `index.html` opens offline and + links every screen; + - `delegates` lists only invoked skills; + - the critique report's last line is `UX_REVIEW: PASS` or BLOCK. + 12. **Critique blocks an unmapped requirement** (runtime-verify, + nontrivial; FR-97 acceptance). Dispatch the `ux-reviewer` agent + against a fixture repo with two `[UI]` ids where `FR-2` has no + screen. The report file's last line is `UX_REVIEW: BLOCK …`, and + its text names `FR-2`. If the harness cannot dispatch the agent, + the result is BLOCKED with that reason. + + If the harness cannot run an interactive skill headlessly, the + result is BLOCKED with that reason, never PASS. + +## Evaluation rubric +| Criterion | High-quality | Acceptable | Failing | +|---|---|---|---| +| requirement traceability | Every FR-89..101 (+79/81/86/87, NFR-1/3) maps to a named ux.sh function, skill block, agent rule, or lint check | One mapping indirect but named | An in-scope FR untraced | +| interface concreteness | Every ux.sh function has args, stdout, rc pinned; index.json schema field-by-field | One rc implicit | Reader cannot tell what a function prints | +| alternatives-analysis substance | Chrome CLI vs Playwright, JSON vs MD index, hash vs git-diff delta, tokens.css vs JSON each with reason | One rejection thin | None named | +| verification-plan actionability | Functions driven in temp repos; skill blocks extracted and run; negated checks assert readability first (L-001/L-011) | One check text-only with stated reason | Behaviour asserted only by grepping prose | +| scope-bound adherence | Each TDD <=8 files, <=500 body lines, padded estimates; exceptions declared | One justified exception | Over a bound unexplained | +| naming consistency | ux.sh names, index.json fields, screen-path form identical across 0070-0072 | One alias reconciled | Same concept named two ways | +| delegation, not reinvention | Skill invokes present delegates by role; ux.sh holds only mechanics, no design logic | Delegates named only as examples | throughline code generates visual design | +| fail-loud parsing | Malformed [UI] marker, unreadable index, awk failure each exit non-zero with a named message | One path returns empty silently but is tested | A parse failure reads as no UI requirements | + +## Requirement traceability +| Requirement | Design element | +|---|---| +| FR-90 delta-driven, plan first | step 1 `tl:ux-preflight` (no-UI line, no branch); step 4 screen plan; obs 1, 2 | +| FR-91 artifact set | step 8 mock instructions + index; step 11 `tl_ux_index_html`; `tl:ux-render`; obs 6, 7, 11 | +| FR-92 delegated design, honest degrade | step 3 role-based discovery; `delegates` = invoked only; fidelity rule; reviewer grades the declaration; obs 8, 11 | +| FR-93 design system | step 6 (existing → derived `tokens.css`; none → `tokens.css` + proposed ADR via `/adr-new`; merged → reused) | +| FR-94 baseline, captures never committed | step 2 baseline; step 7 capture outside the repo; `validate`'s unreferenced-image rule; obs 6 | +| FR-95 gaps recorded, never invented | `data-ux-gap` markers; PR "Requirement gaps"; obs 9 | +| FR-96 in-session review | step 9 per-set approve/change/drop, approvals in the draft | +| FR-97 independent critique | step 10 fresh `ux-reviewer` worker; blocking rules; verdict not cached; obs 8 | +| FR-98 phase gate | step 11 branch `docs/ux/<slug>`, PR sections, never merges | +| FR-99 superseded | step 9 re-renders only changed screens (TDD 0070 scoping) | +| FR-100 authoring discipline | step 0 drafts + FR-86; step 2 interrogator; step 5 rubric; obs 3, 4 | +| FR-22 loads without delegates | the skill needs no delegate (degrade path); README states it | +| FR-79 both harnesses | role-based discovery; harness-agnostic actions; obs 5 | +| FR-81 harness-agnostic language | `ux-author` added to the [K] scan; obs 5 | +| FR-86 light-tier check | byte-identical `tl:fr86-check`; obs 4 | +| FR-87 critique worker inherits | `ux-reviewer` `model: inherit`, dispatched with no model parameter; obs 8 | +| NFR-1 human merge | step 11 never merges | +| NFR-3 judgment on the parent model | authoring + critique inherit; no light-tier worker in this skill | + +## Dependencies considered +- **Delegated design skills, by role (chosen).** No new code dependency. + The skill invokes whatever the harness offers. + - *Rejected: bundling a specific design skill, or requiring the Figma + MCP.* That would make a hard delegate dependency (contradicting + FR-83) and lock the skill to a vendor. Figma also stores the record + outside git. + - *Rejected: generating visual design in throughline's own scripts.* + That reinvents what the delegates do (the PRD's non-goal). +- **`agents/ux-reviewer.md` (chosen), a new agent.** *Rejected: reusing + `design-reviewer` with a UX mode.* Its checklist is TDD-specific, and + merging would dilute both prompts. +- **Reading rules from `/prd-author` by reference (chosen).** + - *Rejected: copying the interrogator and rubric sections.* That is + ~150 duplicated lines, and the copies drift. + - *Rejected: a shared rules file.* It would touch both existing skills + for no behavioural gain. + +## PRD conflicts surfaced (and resolution) +- FR-99's "untouched screens byte-identical" holds for HTML. A viewport + change deliberately re-renders every screen's PNGs. +- FR-90's acceptance says "the first thing shown is a screen plan naming + that requirement id". The step-0 resume prompt and the FR-86 + continue/stop prompt can come first; both are session gates, not + content. FR-100 also applies the FR-75 interview to the + plan. This design satisfies both. Step 1b shows a mechanically built + draft plan before any question, and steps 2–4 interrogate and refine + it into the approved plan. +- FR-97 lists "touch-target size" without a number. This design uses the + common 44×44 CSS px floor on phone viewports, as a finding rather than + a block. +- FR-91's "flow index page" is generated (`tl_ux_index_html`), not + delegate-authored, so it cannot drift from `index.json`. + +## Decisions to promote (ADR candidates) +- Promoted: ADR 0017 (UX record: in-repo, self-contained HTML, delegated by + role, design input not a build gate), added in this design PR. + +## Touched files +- `skills/ux-author/SKILL.md` — new skill (steps 0–11, `tl:` blocks) +- `agents/ux-reviewer.md` — FR-97 critique worker +- `tests/ux-author-skill.test.sh` — obs 1–3, 6–9 +- `tests/plugin-root.test.sh` — add `ux-author` to the FR-81 [K] list +- `tests/parent-session-check.test.sh` — add `ux-author` to `NAMES` +- `README.md` — `/ux-author` subsection +- `.claude-plugin/plugin.json` — description names `/ux-author` + +## Expected diff size +- `skills/ux-author/SKILL.md` — 290 lines +- `agents/ux-reviewer.md` — 80 lines +- `tests/ux-author-skill.test.sh` — 260 lines +- `tests/plugin-root.test.sh` — 4 lines +- `tests/parent-session-check.test.sh` — 6 lines +- `README.md` — 45 lines +- `.claude-plugin/plugin.json` — 2 lines + +Total expected diff: 687 lines across 7 files. diff --git a/docs/tdd/0072-ux-in-prd-and-tdd-author.md b/docs/tdd/0072-ux-in-prd-and-tdd-author.md new file mode 100644 index 0000000..4466ce7 --- /dev/null +++ b/docs/tdd/0072-ux-in-prd-and-tdd-author.md @@ -0,0 +1,288 @@ +# TDD 0072: `[UI]` in `/prd-author`, merged mocks in `/tdd-author` + +Status: draft +PRD refs: FR-89, FR-101, FR-10 +PRD-rev: 3be6232 +ADR constraints: 0005, 0006, 0010, 0017 + +## Approach +This TDD wires the two existing authoring skills into the UX phase. + +- **`/prd-author` writes the signal (FR-89).** For each new or changed + requirement, the interview asks whether it changes what a user sees or + does in a graphical UI. If it does, the title gets `[UI]`. Before the + PR, a `tl:` block runs `tl_ux_ui_reqs` (TDD 0070), so a malformed + marker fails before it is committed rather than silently skipping + `/ux-author`. The PR body names the `[UI]` requirements and says to + run `/ux-author` next. +- **`/tdd-author` consumes it (FR-101).** After the PRD delta is known + (its step 1), a new block asks `tl_ux_coverage` (TDD 0070) about every + in-scope `[UI]` id against the **merged** UX set: + - **covered**: the design proceeds, and the TDD's traceability row + must cite the screen path; + - **stale** or **uncovered**: refused, unless the user waives the mock + or drops that id from this pass. + + The waiver is recorded twice, so both the human and the lint can see + it. A new `tdd-lint` check makes the citation-or-waiver rule + mechanical. The design-reviewer also grades whether the design honors + the cited mocks. + +Mocks remain design input. `/build-tdds` is unchanged (PRD non-goal). + +## Components & interfaces + +### `skills/prd-author/SKILL.md` +- **Interview (Process item 3, under "Interrogator discipline + (FR-75)").** It adds one rule: for each new or changed + requirement, ask (structured question, yes/no) "Does this change what + a user sees or does in a graphical web or mobile UI?". A yes adds + ` [UI]` after the id in the title (`**FR-120 [UI] Saved searches.**`). + CLI, API and log surfaces are never `[UI]`. Each answer is appended + with header `ui: <ID>`. +- **Self-review.** It adds the bullet "`[UI]` markers: every requirement + answered yes carries `[UI]`; none answered no does; the marker grammar + is `**<ID> [UI] <title>**`." +- **New block `<!-- tl:ui-markers -->`** (inputs `TL_REPO`), run after + the PRD is written and before commit. It sources `plugin-root.sh` and + `ux.sh`, then runs `tl_ux_ui_reqs "$TL_REPO/docs/PRD.md"`. On rc 0 it + prints `ui-requirements: <n>`, then the id/title lines. On non-zero it + shows stderr, and the skill fixes the PRD and re-runs the block. It + never commits a PRD that fails. +- **Git.** When the delta adds or changes any `[UI]` requirement, the + commit body gets a `## UI requirements` section listing them and the + line `Next: merge this PR, then run /ux-author before /tdd-author.` + Otherwise it gets `UI requirements: none`. + +### `skills/tdd-author/SKILL.md` +- **New step 1a "UX coverage (FR-101)",** after step 1. The block + `<!-- tl:ux-coverage -->` takes `TL_REPO` and `TL_IDS`: the in-scope + `[UI]` ids, one per line. `tl_ux_coverage` reads through + `tl_ux_merged_index`, which fetches on a best-effort basis first and + names the ref and SHA it read, so a UX PR merged on the host but not + fetched locally is not misreported. The session computes `TL_IDS` as the + `tl_ux_ui_reqs` ids that fall in this pass's PRD delta. When + `TL_IDS` is empty, the block prints `ux-coverage: no [UI] requirements in scope` + and exits 0. Otherwise it runs `tl_ux_coverage "$TL_REPO" $TL_IDS` and + prints its lines. + - **rc 0**: proceed. + - **rc 1**: for each `stale` / `uncovered` id, the skill prints + `throughline: <ID> is a [UI] requirement with no merged UX set (<uncovered|stale: reason>); run /ux-author first, or waive the mock.` + It then asks a structured question per id: + - **Stop and run `/ux-author`**: ends the skill. The draft persists. + - **Waive the mock**: asks for a rationale of 20–400 characters, + appends `assumption: mock waived <ID>` with the answer + `waived: <rationale>`, and proceeds. + - **Drop `<ID>` from this pass**: recorded as a decision, and the id + leaves the TDD scope. + - **rc 2 or 3**: show stderr and stop. It never proceeds on an + unreadable merged index. +- **Template / step 5.** A `[UI]` id in a TDD's `PRD refs` must have a + traceability row that cites every merged screen the coverage line + named, as `docs/ux/screens/<sid>/`. A waived id has the row text + `mock waived: <rationale>` instead. The TDD's design must use the + merged `docs/ux/tokens.css` values for any visual styling it + specifies. +- **Step 9 (PR body).** The "Open assumptions & waivers" section already + renders `assumption:` entries. A mock waiver therefore appears as + `- mock waived <ID> — waived: <rationale>`. The PR body also lists the + covered `[UI]` ids with their screen paths, under `UX coverage`. + +### `scripts/lib/tdd-lint.sh` +- **New `tl_lint_ux_citations <tdd-path>`**, added to `tl_lint_all`'s + function list. + - It resolves the repo root with + `git -C "$(dirname <tdd>)" rev-parse --show-toplevel`. With no git + root, or no `docs/PRD.md`, it returns 0: there is nothing to check. + - When `grep -qF '[UI]' docs/PRD.md` fails, it returns 0 without + sourcing `ux.sh`. + - Otherwise it sources `ux.sh` (fail closed: + `tdd-lint: ux: cannot source ux.sh`, rc 2) and takes the + intersection of `tl_ux_ui_reqs` ids with the ids on the TDD's + `PRD refs:` line. + - For each such id, it reads the `## Requirement traceability` body + through `md_section_body` and checks that body's rc. + - If a line containing the id contains `mock waived: ` followed by at + least one non-space character, the id passes. + - Otherwise it runs `tl_ux_coverage <root> <id>`. A `covered` result + needs every screen path on that line to appear on a traceability + line containing the id. A `stale` or `uncovered` result without a + waiver is a finding. + - Findings are emitted through `_tl_emit` with severity `major` and + code `ux.citation`: + `<id> is [UI]: cite docs/ux/screens/<sid>/ from the merged UX set, or record 'mock waived: <rationale>'`. + A finding makes the function return rc 1. `tl_ux_coverage` rc 1 + (stale/uncovered) is an expected result that feeds the rule above; + it is not a failure. The lint exports `THROUGHLINE_UX_NOFETCH=1`, so + it never fetches. A helper failure (`tl_ux_*` rc 2/3, or a non-zero + `md_section_body`) returns rc 2 with + `tdd-lint: ux: <what> failed (rc <n>) on <tdd>`. A failure never + reads as clean (L-005). + +### `agents/design-reviewer.md` +A new check bullet: "**UX coverage (FR-101).** For each `[UI]` +requirement in scope, the traceability row cites merged +`docs/ux/screens/<sid>/` paths or records `mock waived: <rationale>`. +Open the cited mocks. A design that contradicts a cited mock (a state +the mock shows that the design omits, or styling values that diverge +from `docs/ux/tokens.css`) is a finding. A boilerplate waiver rationale +is a `BLOCK ux-waiver`." + +## Data & state +No new state. This TDD reads `docs/PRD.md`, the merged +`docs/ux/index.json` (through TDD 0070) and the TDD files. Waivers live +in the existing `tdd-author` draft (`assumption:` entries) and in the +TDD's traceability row. + +## Sequencing / implementation plan +1. Add `tl_lint_ux_citations` to `tdd-lint.sh` and wire it into + `tl_lint_all`. +2. Edit `skills/prd-author/SKILL.md`: the interview rule, the + self-review bullet, `tl:ui-markers`, and the commit-body section. +3. Edit `skills/tdd-author/SKILL.md`: step 1a `tl:ux-coverage`, the + refusal and three-way question, the citation rule, and the PR-body + `UX coverage`. Add the design-reviewer bullet. +4. Write `tests/ux-pipeline.test.sh`. + +## Failure modes & edge cases +**Real risks** +- *The author answers "no" for a UI change.* The PRD PR's human review is + the gate. The commit body's `UI requirements: none` line makes the + answer visible in review, not buried. +- *The merged UX set is stale for a requirement that changed again.* + Coverage reports `stale`. The refusal says so, and the user reruns + `/ux-author`. +- *A lint false clean.* Any helper failure is rc 2, never 0. A TDD whose + `PRD refs` omits a `[UI]` id it actually designs is caught by the + existing traceability lint only if the id is referenced. The + design-reviewer bullet covers the rest. +- *A consumer repo without python3 that has `[UI]` markers.* + `tl:ux-coverage` and the lint both return rc 3 / rc 2 with + `ux: python3 required`. That is loud, matching the UX runtime decision. + +**Overblown risks** +- *Old TDDs fail the new lint.* The lint runs only on the set being + authored (step 7a), and old TDDs reference no `[UI]` ids because none + existed before FR-89. + +**Unspoken risks** +- *A waiver as a habit.* A waiver is cheap to type. Three things keep it + visible: the rationale bound, its appearance in the PR body, and the + design-reviewer's `BLOCK ux-waiver` for boilerplate. +- *`/prd-author` and `/ux-author` disagree on the grammar.* Both call the + same `tl_ux_ui_reqs`, so they cannot disagree. + +## Verification plan +- **Surface:** the stdout and rc of the extracted `tl:ui-markers` and + `tl:ux-coverage` blocks; the stdout and rc of `tdd-lint.sh` on fixture + TDDs; the agent and skill text where behaviour is model prose. +- **Harness:** temp git repos with `master` holding `docs/PRD.md` and, + where needed, a merged `docs/ux/index.json` built by TDD 0070's helpers. + Blocks are extracted by marker and run under `env -i`. A missing marker + or file is a FAIL, never a skip. Every negated grep asserts that its + file is readable first (L-001/L-002/L-011). Temp dirs are + trap-cleaned (L-004). +- **Observation points → expected (PASS):** + 1. **`tl:ui-markers`, clean.** A PRD with `**FR-1 [UI] Login.**` and + `**FR-2 CLI flag.**` → `ui-requirements: 1` then `FR-1\t…\tLogin`, + rc 0. + 2. **`tl:ui-markers`, malformed.** A PRD containing `FR-3 [UI]` outside + the bold title grammar → non-zero rc, and stderr has + `malformed [UI] marker` and the line number. + 3. **`tl:ux-coverage`.** + - `TL_IDS` empty → `ux-coverage: no [UI] requirements in scope`, + rc 0. + - FR-1 merged on `master` → a `covered` line, rc 0. + - FR-1 present only on a checked-out branch → `uncovered FR-1`, + rc 1. + - A corrupt merged index → rc 2, and stderr has `ux: invalid index`. + 4. **Lint, cited.** A TDD with `PRD refs: FR-1` and a traceability row + `| FR-1 | … docs/ux/screens/login/ |` where the merged index maps + FR-1 to `login` → `tdd-lint.sh` exits 0 and prints no `ux.citation`. + 5. **Lint, missing citation.** The same TDD with a row lacking the + path → rc 1, and stdout has `major ux.citation: FR-1 is [UI]`. + 6. **Lint, waived.** A row `| FR-1 | mock waived: the button reuses the existing toolbar style |` + → rc 0. The row `| FR-1 | mock waived: |` → rc 1. + 7. **Lint, non-UI repo.** A PRD without `[UI]` on a PATH without + python3 → `tl_lint_ux_citations` rc 0, and `ux.sh` is never sourced + (checked by a sentinel function absent afterwards). + 8. **Lint, helper failure.** A corrupt merged index → rc 2 and + `tdd-lint: ux:` on stderr, never rc 0. + 9. **Skill text** (text checks by necessity: the interview and refusal + are model prose; each asserts that the file is readable first): + - `prd-author/SKILL.md` contains the `tl:ui-markers` marker and the + exact question sentence; + - `tdd-author/SKILL.md` contains the `tl:ux-coverage` marker, the + exact refusal sentence prefix + `is a [UI] requirement with no merged UX set`, and the three + options; + - `design-reviewer.md` contains `UX coverage (FR-101)` and + `BLOCK ux-waiver`. + 10. **Regression.** All existing evals stay green. `tdd-lint.sh` on + every existing `docs/tdd/0*.md` gives the same exit code as before. + This repo's PRD mentions `[UI]` only in backticks. The `grep -qF` + short-circuit therefore does not fire: the lint sources `ux.sh` + and needs python3 here. `tl_ux_ui_reqs` returns no ids, so the + check emits nothing. + +## Evaluation rubric +| Criterion | High-quality | Acceptable | Failing | +|---|---|---|---| +| requirement traceability | Every FR-89..101 (+79/81/86/87, NFR-1/3) maps to a named ux.sh function, skill block, agent rule, or lint check | One mapping indirect but named | An in-scope FR untraced | +| interface concreteness | Every ux.sh function has args, stdout, rc pinned; index.json schema field-by-field | One rc implicit | Reader cannot tell what a function prints | +| alternatives-analysis substance | Chrome CLI vs Playwright, JSON vs MD index, hash vs git-diff delta, tokens.css vs JSON each with reason | One rejection thin | None named | +| verification-plan actionability | Functions driven in temp repos; skill blocks extracted and run; negated checks assert readability first (L-001/L-011) | One check text-only with stated reason | Behaviour asserted only by grepping prose | +| scope-bound adherence | Each TDD <=8 files, <=500 body lines, padded estimates; exceptions declared | One justified exception | Over a bound unexplained | +| naming consistency | ux.sh names, index.json fields, screen-path form identical across 0070-0072 | One alias reconciled | Same concept named two ways | +| delegation, not reinvention | Skill invokes present delegates by role; ux.sh holds only mechanics, no design logic | Delegates named only as examples | throughline code generates visual design | +| fail-loud parsing | Malformed [UI] marker, unreadable index, awk failure each exit non-zero with a named message | One path returns empty silently but is tested | A parse failure reads as no UI requirements | + +## Requirement traceability +| Requirement | Design element | +|---|---| +| FR-89 `[UI]` marker written by `/prd-author` | step-3 question + `ui: <ID>` draft entries; self-review bullet; `tl:ui-markers` block (`tl_ux_ui_reqs`); commit-body `UI requirements`; obs 1, 2 | +| FR-101 design builds on merged mocks | `/tdd-author` step 1a `tl:ux-coverage` (merged blob); refusal sentence + three-way question; waiver as an `assumption:` entry and `mock waived:` row; `tl_lint_ux_citations`; obs 3–8 | +| FR-10 design-reviewer gate (extended) | `design-reviewer.md` UX-coverage bullet + `BLOCK ux-waiver`; obs 9 | + +## Dependencies considered +- **The TDD 0070 helpers (chosen)** for grammar and coverage. + - *Rejected: a separate grep in `/prd-author` and `tdd-lint`.* A + second grammar implementation could disagree with `/ux-author`'s and + silently pass a marker the other rejects. +- **Waiver in the traceability row plus a draft `assumption:` entry + (chosen).** + - *Rejected: a separate `## UX waivers` section in the TDD.* That adds + a required-section variant to `tdd-lint`'s structural check for a + rare case. The row is where a reader already looks for the + requirement's design. +- **Lint inside the existing `tl_lint_all` (chosen).** + - *Rejected: a check only in the design-reviewer prompt.* That is + model-only and costs tokens on a fact a script can decide (FR-51's + rationale). + +## PRD conflicts surfaced (and resolution) +- FR-101's acceptance requires the refusal to name `/ux-author`. The + refusal sentence above includes `run /ux-author first`. +- FR-89 says the marker is "the only signal". This TDD adds the + commit-body `UI requirements` line for review visibility only. No code + reads it. + +## Decisions to promote (ADR candidates) +- Promoted: ADR 0017 (UX record: in-repo, self-contained HTML, delegated by + role, design input not a build gate), added in this design PR. + +## Touched files +- `skills/prd-author/SKILL.md` — `[UI]` question, self-review bullet, `tl:ui-markers`, commit-body section +- `skills/tdd-author/SKILL.md` — step 1a `tl:ux-coverage`, refusal/waiver, citation rule, PR `UX coverage` +- `agents/design-reviewer.md` — UX-coverage check bullet +- `scripts/lib/tdd-lint.sh` — `tl_lint_ux_citations` + `tl_lint_all` wiring +- `tests/ux-pipeline.test.sh` — obs 1–9 + +## Expected diff size +- `skills/prd-author/SKILL.md` — 45 lines +- `skills/tdd-author/SKILL.md` — 75 lines +- `agents/design-reviewer.md` — 14 lines +- `scripts/lib/tdd-lint.sh` — 95 lines +- `tests/ux-pipeline.test.sh` — 280 lines + +Total expected diff: 509 lines across 5 files.