diff --git a/.claude/skills/ui-review/SKILL.md b/.claude/skills/ui-review/SKILL.md index dc398a25..3901c76b 100644 --- a/.claude/skills/ui-review/SKILL.md +++ b/.claude/skills/ui-review/SKILL.md @@ -58,24 +58,22 @@ then a **review page** — never a gallery, never a chat summary: 1. Capture the branch: `bash scripts/ui-review/run-review.sh scratch/` (it starts its own server on Vite 5473 and refuses if that port serves another worktree). For a second variant, a second worktree + run dir. -2. Write `docs/active/design//-cards.json` (copy `phase-c-cards.json`), then - `python3 scripts/ui-review/review-cards.py crop ` and `… build `. Crop regions - come from `scripts/ui-review/crops.json`; add new ones there. The page is a **deck — one - point at a time**: one screenshot with ONE ring on the target, one line of problem, one - line of fix, `measured`/`judgment` tag, Yes / No / Tell me more (keys Y/N/M), progress dots, summary + - copyable feedback at the end. Rationale and ledger corrections go under the collapsed - "Why / details", never in the headline. Three formats were rejected before this one - (gallery → prose page → board of cards): "not clear where I'm supposed to glance/select". - `review-page.py` is the old prose format, kept only for the Phase A/B pages. -3. Every item on the page carries, in this order: the problem **with the measured number - or the broken behaviour**, exactly what was edited, 1:1 crops of the element per theme - (before / after, a column per variant), what he'll notice + the risks *against* the - change, alternatives considered, and a decision control. **Tag each item `measured`, - `judgment` or `mixed`** and say which parts are which — on 2026-08-25 a taste argument - (P-12) went in as if it were a defect and was rightly rejected on sight. -4. Hand Destin the page path; he pastes the generated feedback block. Act on it exactly; - record the decisions in the findings ledger (the row, not a new section), the guide, - the ROADMAP entry, and a `banner` on the page. Merge, archive, clean up. +2. Write `docs/active/design//-review.json` (copy `phase-c-review-v2.json`): one + step per point with `surface`, `path`, `crop`, `highlight` (`"auto"` for before/after, else + `{"selector": …}` measured by the rig), and the four texts — **headline** (≤ 25 words, what a + user sees), **changed** (what was edited, plain words, with `measured` when there is a number), + **notice** (what changes for users — intended and side effects), **risk** (what could look + wrong, or is not shown faithfully). The builder refuses jargon (token, primitive, selector, + IPC, prop, reducer, handler, component…), a missing picture, or an unresolved box. +3. `python3 scripts/ui-review/review-cards.py serve ` **in the background** (it builds + first; fix every `missing:` line it prints — a measurement that is missing means the plan + needed a `measure` line before the Before run). The browser opens itself; Destin answers + Yes / No / Other per step with an optional note and presses Submit; the background command + exits with the summary (exit 0 = submitted, summary on stdout; 2 = nobody submitted before the + timeout; 3 = another process already serves this spec — neither 2 nor 3 carries answers, do not + invent a result) (`wait ` if you lost the process). Never ask him to paste anything. +4. Act on the summary exactly (`Other` + note = change it as described); record decisions in the + findings ledger row, the guide, the ROADMAP entry. Merge, archive, clean up. ## Red flags diff --git a/.gitignore b/.gitignore index 391a3dd5..d9aa0848 100644 --- a/.gitignore +++ b/.gitignore @@ -82,3 +82,10 @@ docs/active/investigations/harness-eval-runs/** # UI-audit screenshot sheets (evidence for docs/active/design/2026-08-25-*); regenerate per the folder README docs/active/design/*-ui-audit/images/ +*.answers.json +*.answers.*.json +*.serve.json + +# Python bytecode from the deck tooling + its tests +__pycache__/ +*.pyc diff --git a/CLAUDE.md b/CLAUDE.md index 7df6e6c0..9f06a144 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -116,7 +116,7 @@ bash scripts/run-dev.sh --label "Feature Name" ### New Features & UI/UX Changes -When designing new features or making changes to user-facing app interfaces, the first step should always be to visualize and design the UI/UX of the final feature. Planning sessions should prioritize iterative UI design using the workbench and other tooling to help Destin shape the final user experience of the feature before building backend. When Destin provides final sign-off on the UI/UX design for the feature, the UI/UX should be treated as largely final and backend should be designed around the UI/UX accordingly. The standard every new surface is measured against is `docs/active/design/2026-08-25-ui-design-guide.md` (five laws, primitives, per-surface anatomies, checklist); show him the change as a **review deck** (`scripts/ui-review/review-cards.py` — one point per step: one screenshot, one ring, one problem line, one fix line, Yes / No / Tell me more, Before/After flip), built from the UI review rig below; never a gallery, a prose page or a chat description (all three were rejected). +When designing new features or making changes to user-facing app interfaces, the first step should always be to visualize and design the UI/UX of the final feature. Planning sessions should prioritize iterative UI design using the workbench and other tooling to help Destin shape the final user experience of the feature before building backend. When Destin provides final sign-off on the UI/UX design for the feature, the UI/UX should be treated as largely final and backend should be designed around the UI/UX accordingly. The standard every new surface is measured against is `docs/active/design/2026-08-25-ui-design-guide.md` (five laws, primitives, per-surface anatomies, checklist); show him the change as a **review deck** (scripts/ui-review/review-cards.py — one point per step: Before | After with the changed region boxed by the rig, a headline and three cards — What changed / You'll notice / Risk — Yes / No / Other, answers saved to a file and handed to Claude on Submit; `serve ` in the background does it all), built from the UI review rig below; never a gallery, a prose page or a chat description (all three were rejected). ### UI Workbench diff --git a/ROADMAP.md b/ROADMAP.md index fcfbfddd..d7bf19df 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1141,6 +1141,9 @@ surface, not a history. Destin's ask: the session switcher (`SessionStrip.tsx` — NOT `SessionDrawer.tsx`, which is the artifacts/Files drawer) and panel-opening transitions feel abrupt/undertuned and deserve real motion design. Not scoped — no design pass, no target easing/duration, no list of which panels. Likely pairs with the UI-consistency work under Features once that reaches tranches touching session-switcher UI. - [ ] Session switcher should use the theme's rounding rules `idea` `#renderer` `#ui` `#themes` (added 2026-07-20) Destin's ask: the session switcher's (`SessionStrip.tsx`) corners should follow whatever rounding rule the active theme sets. **Checked 2026-07-20 and the premise may already hold, in which case this is a verify-not-build item:** `SessionStrip.tsx` already uses plain Tailwind `rounded-full`/`rounded-lg`/`rounded-sm` utilities throughout (no hardcoded `rounded-[Npx]` or inline `border-radius` found), and `globals.css`'s `@theme` block maps those straight to `--radius-*` custom properties, which `theme-engine.ts`'s `applyThemeToDom` overwrites per-theme from an optional `shape.radius*` block in `ThemeShape` (`theme-types.ts`) — `theme-builder`'s `manifest-template.jsonc` even lists corner rounding as a *required* Kit field with named presets ("Heavily rounded" vs "Minimal rounding — brutalist", `kit-presets.json`), and the community `golden-sunbreak` theme already overrides it. **The likely real gap:** all four BUILT-IN themes (light/dark/midnight/creme) currently ship identical radius values (`globals.css`), so there's nothing to visually confirm this against without installing/building a theme with a different rounding preset — if Destin observed the switcher NOT rounding correctly under some theme, that's a repro to chase (possibly a stale/cached class, a z-order surface that isn't `.session-strip` itself, or a spot missed by the grep), not a missing token. Verify against an actual differently-rounded theme before assuming code needs to change. +- [ ] Workbench serves community theme folders (`theme-asset://`) so decks show real previews `idea` `#tooling` (added 2026-08-27) +- [ ] Attach your own screenshot to a review-deck step (the serve endpoint can accept uploads) `idea` `#tooling` (added 2026-08-27) +- [ ] Review-deck test hygiene: bare `open()` in `scripts/ui-review/tests/{fixture,test_spec,test_serve}.py` prints ResourceWarnings that bury real failures; `shot-measure.test.mjs` sleeps 800 ms for python's http.server instead of polling the port and never tests the `run: null` branch; no test drives `review-cards.py serve`'s build-failure short-circuit (exit 1, no server) `chore` `#tooling` `#tests` (added 2026-08-27, from the deck-v2 final review) ## Shipped diff --git a/docs/active/design/2026-08-25-ui-audit/phase-c-review-v2.html b/docs/active/design/2026-08-25-ui-audit/phase-c-review-v2.html new file mode 100644 index 00000000..4b6bbbbd --- /dev/null +++ b/docs/active/design/2026-08-25-ui-audit/phase-c-review-v2.html @@ -0,0 +1,283 @@ +Phase C review + + +
+
Review deck
+
+
·
+
+ +
+
+
+
+
100%
+

+
+ + + + + +
+
+
+
+
+

Submit your feedback?

+
Skipped steps are sent as "no answer"; Claude leaves those unchanged.
+ +
+
+ + + diff --git a/docs/active/design/2026-08-25-ui-audit/phase-c-review-v2.json b/docs/active/design/2026-08-25-ui-audit/phase-c-review-v2.json new file mode 100644 index 00000000..b309b7f8 --- /dev/null +++ b/docs/active/design/2026-08-25-ui-audit/phase-c-review-v2.json @@ -0,0 +1,78 @@ +{ + "title": "Phase C review", + "key": "phase-c-review", + "out": "phase-c-review-v2.html", + "images": "images/phase-c-review-v2", + "runs": { "before": "/home/destin/youcoded-dev/scratch/ui-phase-c-baseline", "after": "/home/destin/youcoded-dev/scratch/ui-phase-c-after" }, + "themes": ["midnight", "light", "creme", "dark", "halftone-dimension", "meadow-mist"], + "crops": { + "themes-dialog": ["main", "settings-appearance", "440x600+500+150"], + "market-hero": ["marketplace", "marketplace", "900x200+0+50"], + "market-card-counts": ["main", "marketplace", "600x150+10+375"], + "market-bar": ["marketplace", "marketplace", "760x70+0+248"], + "market-search": ["marketplace", "marketplace", "300x70+1120+248"], + "market-narrow-title": ["narrow", "marketplace", "390x56+0+0"], + "market-explore-empty": ["marketplace", "marketplace-empty", "720x160+0+50"], + "market-themes-empty": ["empty-marketplace", "marketplace-themes", "1440x240+0+50"], + "library-empty": ["empty-marketplace", "library", "1440x430+0+0"], + "library-tabs": ["main", "library", "520x110+0+0"] + }, + "steps": [ + { "id": "P-3.1", "surface": "Themes dialog", "path": "Settings → Appearance", "crop": "themes-dialog", + "headline": "Every theme card is now the same height, so the active card no longer grows and stretches its neighbour.", + "changed": "Picture on top, one text row at the bottom, and every card the same height. Built-ins got preview pictures from the marketplace generator; other themes show their own preview.", + "measured": "The active card 59 px vs 34 px for every other card, before", + "notice": "The grid stops jumping when you pick a theme, and any theme that has a preview picture now shows it instead of a colour strip.", + "risk": "In these screenshots Halftone and Meadow still show the colour strip because the screenshot tool cannot load theme folders; in the app they show their own preview." }, + { "id": "P-3.3", "surface": "Themes dialog", "path": "Settings → Appearance", "crop": "themes-dialog", + "headline": "“Your Themes” reads “Favorited Themes”, “Browse all themes →” is gone, and Browse Theme Marketplace sits above Build New Theme.", + "changed": "Renamed the heading, removed the text-link row, reordered the two buttons.", + "notice": "One less button under the theme grid, and the Marketplace button now comes first. The separate list of your installed themes that the old link opened is no longer reachable from here.", + "risk": "" }, + { "id": "P-21.1", "surface": "Marketplace", "path": "Marketplace → featured card", "crop": "market-hero", + "headline": "The featured card uses the theme’s normal card edge; the gold border is gone.", + "changed": "One border style for every card. The small FEATURED label above the name is now the only thing marking it.", + "measured": "3 different featured borders before, none now", + "notice": "The Marketplace opens calmer — no single card shouts — and the featured one still reads first because it sits first.", + "risk": "" }, + { "id": "P-21.3", "surface": "Marketplace", "path": "Marketplace → plugin card", "crop": "market-card-counts", + "headline": "A plugin with a single install now reads “1 install”, not “1 installs”.", + "changed": "One shared counting rule for both numbers on the card, pinned by a test.", + "measured": "2 spots on the card fixed", + "notice": "Install and like counts read correctly everywhere a card shows them; nothing else on the card moves.", + "risk": "The likes count runs off the edge of these cards, so only the install count is visible in the picture." }, + { "id": "P-2.1", "surface": "Library", "path": "Library → nothing installed", "crop": "library-empty", + "headline": "A brand-new Library shows “Nothing installed yet” and a Browse the Marketplace button, instead of two headings over blank space.", + "changed": "Fixed the counting mistake that made every section believe it already had contents, then used the app’s standard empty message. A test pins it.", + "measured": "2 headings, 0 words of guidance before", + "notice": "A new user lands on a Library that tells them what to do next. If you already have things installed, they still show exactly as before — only the switcher above them changes (next step).", + "risk": "" }, + { "id": "P-2.2", "surface": "Library", "path": "Library → Skills / Themes switcher", "crop": "library-tabs", + "headline": "The Skills and Themes switcher now matches Projects: an icon, a label, and how many you have installed.", + "changed": "Swapped the two plain text pills for the same bordered tabs Projects already uses. Projects itself is untouched.", + "notice": "You can see how many skills and themes you have without opening either tab, and the switcher is bigger and easier to hit.", + "risk": "The tabs stand taller than the old plain pills, so everything below them sits a little lower." }, + { "id": "P-1.1", "surface": "Marketplace", "path": "Marketplace → top bar", "crop": "market-bar", + "headline": "Plugins / Themes is now a pick-one switch with an All option, in the same spot; the filters after it stay pills.", + "changed": "The pick-one choice is drawn as a switch; the pick-any filters keep their pill shape. The phone filter sheet got the same switch.", + "notice": "It reads at a glance that you pick one of Plugins or Themes, but any number of the filters — they no longer look like the same kind of button.", + "risk": "" }, + { "id": "P-1.2", "surface": "Marketplace", "path": "Marketplace → search box", "crop": "market-search", + "headline": "The search box is the app’s shared search field with a magnifier, the same height as the pills beside it.", + "changed": "Replaced the bare box with the shared search field Projects already uses. On phones that same field carries the Filters button.", + "notice": "Search looks the same here as everywhere else in the app, and no longer sits shorter than its neighbours.", + "risk": "" }, + { "id": "P-1.3", "surface": "Marketplace", "path": "Marketplace → nothing matches", "crop": "market-themes-empty", + "headline": "When nothing matches your filters you get one message with a Clear filters button, instead of two messages and no way back.", + "changed": "Replaced the “0 results” line and the second sentence with the app’s standard empty message.", + "measured": "2 messages before, 1 now", + "notice": "One sentence to read, and a button that puts the full list back in a single click.", + "risk": "" }, + { "id": "P-1.5", "surface": "Marketplace", "path": "Marketplace at phone width", "crop": "market-narrow-title", + "headline": "At phone width the title reads “Marketplace” again instead of being cut to “Ma…”.", + "changed": "The “Esc · Back to chat” hint is no longer drawn at phone width. It was meant to hide there and never did, so it took the title’s room.", + "measured": "390 px wide; the title was cut to 2 letters", + "notice": "The screen name is readable on a phone. The Esc hint is gone at that width and unchanged on a desktop-sized window.", + "risk": "The reason the hint refused to hide is a bug in the app’s shared button; it is filed for a proper fix and could affect other screens." } + ] +} diff --git a/docs/active/handoffs/2026-08-27-review-deck-tooling-handoff.md b/docs/active/handoffs/2026-08-27-review-deck-tooling-handoff.md index 8f10aed0..871a897d 100644 --- a/docs/active/handoffs/2026-08-27-review-deck-tooling-handoff.md +++ b/docs/active/handoffs/2026-08-27-review-deck-tooling-handoff.md @@ -1,8 +1,10 @@ --- -status: active +status: shipped created: 2026-08-27 --- +Superseded by `docs/archive/specs/2026-08-27-review-deck-v2-design.md` (built on `feat/review-deck-v2`); gaps 1, 3, 5, 6, 7 closed there; 2 and 4 are ROADMAP ideas. + # Hand-off: the UI review tooling (screenshot rig + review deck) **Purpose of this file:** a prompt Destin can paste into another Claude session to diff --git a/docs/active/prototypes/2026-08-27-deck-mockup-g.html b/docs/active/prototypes/2026-08-27-deck-mockup-g.html new file mode 100644 index 00000000..da0990d1 --- /dev/null +++ b/docs/active/prototypes/2026-08-27-deck-mockup-g.html @@ -0,0 +1,157 @@ +Deck mockup G +
+
Review deckPhase C review
+
·
+
+
+
+
+
+
100%
Before
After
Before
After
Before
After
+

Every theme card is now the same height, so the active card no longer grows and stretches its neighbour.

+

What changed

Picture on top, one text row at the bottom, every card 92 px tall. Built-ins got preview pictures from the marketplace generator; other themes show their own preview.

Measured: Dark 65 px vs Crème 34 px before

+

You'll notice

The grid stops jumping when you pick a theme, and every card shows a real preview picture instead of a colour strip.

Risk

In these screenshots Halftone and Meadow still show the colour strip because the rig cannot serve theme folders; in the app they show their own preview.

+
+
+
+
100%
Before
After
Before
After
Before
After
+

The featured card uses the theme’s normal card edge; the gold border is gone.

+

What changed

One border style for every card. The “Featured” eyebrow alone marks the featured plugin.

Measured: 1 of 6 border colours remains

+

You'll notice

The Marketplace opens calmer — no single card shouts — and the featured one still reads first because it sits first.

+
+
+ + + +
+
+
+

Submit your feedback?

+

Your answers have been saving to a file next to this deck as you went. Submitting tells Claude you're finished — it picks them up in the session and replies there. Nothing to copy or paste: close this tab and go back to the conversation.

+
Skipped steps are sent as "no answer"; Claude will leave those unchanged.
+
+ \ No newline at end of file diff --git a/docs/active/prototypes/2026-08-27-deck-mockup-g.py b/docs/active/prototypes/2026-08-27-deck-mockup-g.py new file mode 100644 index 00000000..454da1a7 --- /dev/null +++ b/docs/active/prototypes/2026-08-27-deck-mockup-g.py @@ -0,0 +1,216 @@ +#!/usr/bin/env python3 +"""Deck mockup G — F plus an adaptive layout (2026-08-27): +The page measures the space it has and the crop's shape, then scores four arrangements — + A Before | After side by side, cards in a row below + B Before over After (stacked) on the left, cards in a column on the right + C Before | After side by side on the left, cards in a column on the right + D Before over After, cards in a row below +— and uses whichever shows the pictures LARGEST (ties go to A). Re-picks on every resize, so +the same deck adapts to a browser tab, a narrow file panel, or a phone. Steps alternate between +a tall crop (Themes dialog 440×600) and a wide one (Marketplace hero 900×200) to show it moving. +Also: the bottom bar only reserves room for the file panel's Edit floater when the page is +embedded (window.top !== window) — in a browser tab there is no floater.""" +import base64, os, sys + +IMG = '/home/destin/youcoded-dev/docs/active/design/2026-08-25-ui-audit/images/phase-c-review' +OUT = sys.argv[1] +THEMES = [('midnight', 'Midnight'), ('light', 'Light'), ('halftone-dimension', 'Halftone')] +COLS = [('before', 'Before'), ('after', 'After')] +N = 13 +CROPS = { + 'themes-dialog': {'box': {'after': (5, 16, 90, 12), 'before': (5, 14, 90, 10)}, 'where': ('Themes dialog', 'Settings → Appearance'), + 'what': 'Every theme card is now the same height, so the active card no longer grows and stretches its neighbour.', + 'changed': 'Picture on top, one text row at the bottom, every card 92 px tall. Built-ins got preview pictures from the marketplace generator; other themes show their own preview.', + 'num': 'Measured: Dark 65 px vs Crème 34 px before', + 'notice': 'The grid stops jumping when you pick a theme, and every card shows a real preview picture instead of a colour strip.', + 'risk': 'In these screenshots Halftone and Meadow still show the colour strip because the rig cannot serve theme folders; in the app they show their own preview.'}, + 'market-hero': {'box': {'after': (1, 22, 26, 62), 'before': (1, 22, 26, 62)}, 'where': ('Marketplace — featured card', 'Marketplace'), + 'what': 'The featured card uses the theme’s normal card edge; the gold border is gone.', + 'changed': 'One border style for every card. The “Featured” eyebrow alone marks the featured plugin.', + 'num': 'Measured: 1 of 6 border colours remains', + 'notice': 'The Marketplace opens calmer — no single card shouts — and the featured one still reads first because it sits first.', + 'risk': ''}, +} + +def data(name): + with open(os.path.join(IMG, name), 'rb') as f: + return 'data:image/png;base64,' + base64.b64encode(f.read()).decode() + +ICON_CHANGE = '' +ICON_EYE = '' +ICON_WARN = '' + +def step_html(crop): + c = CROPS[crop] + frames = ''.join( + f'
{l}
' + f'
' + for t, _ in THEMES for col, l in COLS) + thumbs = ''.join(f'' for t, l in THEMES) + risk = f'

{ICON_WARN}Risk

{c["risk"]}

' if c['risk'] else '' + return f'''
+
{thumbs}
+
+
100%
{frames}
+

{c["what"]}

+

{ICON_CHANGE}What changed

{c["changed"]}

{c["num"]}

+

{ICON_EYE}You'll notice

{c["notice"]}

{risk}
+
''' + +steps_html = step_html('themes-dialog') + step_html('market-hero') + +TOKENS = ''' +[data-theme="midnight"]{--canvas:#0D1117;--panel:#161B22;--inset:#21262D;--well:#0D1117;--accent:#B1BAC4;--on-accent:#0D1117;--fg:#C9D1D9;--fg-2:#A0AAB4;--fg-dim:#8B949E;--fg-muted:#6E7681;--fg-faint:#4E555E;--edge:#343A41;--link:#58A6FF;color-scheme:dark} +[data-theme="light"]{--canvas:#F2F2F2;--panel:#EAEAEA;--inset:#D7D7D7;--well:#F9F9F9;--accent:#1A1A1A;--on-accent:#F2F2F2;--fg:#1A1A1A;--fg-2:#444;--fg-dim:#656565;--fg-muted:#797979;--fg-faint:#989898;--edge:#C0C0C0;--link:#2055CA;color-scheme:light} +[data-theme="halftone-dimension"]{--canvas:#08060e;--panel:#100e1c;--inset:#181430;--well:#0C0A14;--accent:#E51F48;--on-accent:#fff;--fg:#F0E8F8;--fg-2:#D4CAE4;--fg-dim:#A498C0;--fg-muted:#7468A0;--fg-faint:#4D417A;--edge:#372D56;--link:#ff6b8f;color-scheme:dark;--radius-md:16px;--radius-lg:24px} +:root{--radius-sm:4px;--radius-md:8px;--radius-lg:12px;--radius-full:9999px;--yes:#2E9B57;--no:#E5484D;--other:#C99700;--mark:#FFB020;--font:'Cascadia Mono','Cascadia Code','Fira Code',monospace} +''' + +CSS = TOKENS + ''' +*{box-sizing:border-box} html,body{height:100%;margin:0} +body{font:13px/1.45 var(--font);color:var(--fg);background:var(--well);display:flex;flex-direction:column;padding:18px 10px 10px} +.deck{position:relative;flex:1;min-height:0;display:flex;flex-direction:column;background:var(--canvas);border:2px solid rgba(255,176,32,.6);border-radius:14px;overflow:visible;box-shadow:0 0 0 1px rgba(0,0,0,.35),0 8px 30px rgba(0,0,0,.25)} +.eyebrow{font-size:11px;text-transform:uppercase;letter-spacing:.08em;color:var(--fg-muted);font-weight:500} +/* the chip sits ON the frame: tool name | deck title */ +.chip{position:absolute;top:-13px;left:18px;z-index:5;display:inline-flex;align-items:center;height:26px;background:var(--mark);color:#1a1100;border-radius:7px;padding:0 10px 0 9px;box-shadow:0 2px 8px rgba(0,0,0,.35)} +.chip .k{font-size:10px;font-weight:700;letter-spacing:.08em;text-transform:uppercase} .chip .div{width:1px;height:14px;background:rgba(0,0,0,.35);margin:0 9px} .chip .t{font-size:12px;font-weight:500;white-space:nowrap} +:root{--content:clamp(900px,80vw,1640px)} +.wrap{width:min(var(--content),100%);margin:0 auto;display:flex;align-items:center;gap:14px;min-width:0} +.top{height:clamp(56px,6vh,64px);flex:none;display:flex;padding:8px 20px 0;background:var(--panel);border-bottom:1px solid var(--edge);border-radius:12px 12px 0 0} +.top .where{min-width:0} .top .where .id{white-space:nowrap} @media (max-width:1400px){.top .where .eyebrow,.top .where .sep{display:none}} @media (max-width:950px){.top .count{display:none}} +.nav{display:flex;align-items:center;gap:8px;flex:1;justify-content:center;min-width:0} .top .where{flex:none} +.steps{display:flex;gap:3px;align-items:center;flex:1;max-width:360px;min-width:90px} .steps span{flex:1;height:7px;border-radius:3px;background:var(--inset);cursor:pointer;transition:transform .15s} +.steps span:hover{transform:scaleY(1.4)} .steps span.on{box-shadow:0 0 0 2px var(--panel),0 0 0 3px var(--fg)} +.steps span.yes{background:var(--yes)} .steps span.no{background:var(--no)} .steps span.other{background:var(--other)} .steps span.skip{background:var(--fg-faint)} +.count{font-size:11px;color:var(--fg-muted);font-variant-numeric:tabular-nums;white-space:nowrap} +.btn{font:inherit;font-size:12px;font-weight:500;height:32px;padding:0 14px;border-radius:var(--radius-md);border:1px solid var(--edge);background:transparent;color:var(--fg);cursor:pointer;display:inline-flex;align-items:center;gap:8px;white-space:nowrap;transition:background .15s,transform .15s,filter .15s} +.btn.primary{background:var(--accent);color:var(--on-accent);border-color:var(--accent)} .btn.ghost{border-color:transparent;color:var(--fg-2)} .btn.sm{height:28px;padding:0 10px} +@media (hover:hover){ .btn:hover{background:var(--inset)} .btn.primary:hover{background:var(--accent);filter:brightness(1.12);transform:translateY(-1px)} .btn.ghost:hover{background:var(--inset);color:var(--fg)} } +.btn:disabled{opacity:.45;cursor:default;transform:none;filter:none} +.dot{width:8px;height:8px;border-radius:50%;flex:none} .dot.yes{background:var(--yes)} .dot.no{background:var(--no)} .dot.other{background:var(--other)} +.ans{border-color:var(--edge)} .ans.on{background:var(--inset);border-color:var(--fg);box-shadow:inset 0 0 0 1px var(--fg)} +main{flex:1;min-height:0;display:flex;justify-content:center;padding:14px 20px 16px;overflow:auto} +.step{display:none;width:min(var(--content),100%);flex-direction:column;gap:10px;min-height:0} .step.on{display:flex} +.where{display:flex;align-items:center;gap:10px} .where .id{font-weight:500;font-size:14px} .where .sep{color:var(--fg-faint)} +.step{position:relative} +.thumbs{position:absolute;left:calc(100% + 16px);top:0;display:flex;flex-direction:column;gap:10px} +body.thumbs-inline .thumbs{position:static;flex-direction:row;justify-content:flex-end;margin-bottom:8px} +.thumb{border:2px solid transparent;border-radius:var(--radius-md);padding:3px;background:transparent;cursor:pointer;display:flex;flex-direction:column;align-items:center;gap:3px;font:11px var(--font);color:var(--fg-dim)} +.thumb img{height:44px;width:auto;max-width:110px;object-fit:cover;border-radius:3px;display:block} .thumb.on{border-color:var(--accent);color:var(--fg)} +/* ── adaptive content: layout class chosen by JS ── */ +.content{flex:1;min-height:0;display:grid;gap:12px} +.content.row-below{grid-template-columns:1fr;grid-template-rows:1fr auto auto;grid-template-areas:"stage" "info" "ctl"} /* A, D */ +.content.col-right{grid-template-columns:1fr minmax(320px,30%);grid-template-rows:1fr auto;grid-template-areas:"stage info" "ctl ctl"} /* B, C */ +.content.compact{display:flex;flex-direction:column;flex:none} /* too small for any arrangement: one scrolling column */ +.compact .stage{flex:none;overflow:visible} .compact .stage .inner{flex-direction:column;align-items:center} .compact .info{overflow:visible} +.step.compact-step{flex:none;min-height:0} +.stage{grid-area:stage} .info{grid-area:info} .controls{grid-area:ctl} +/* the three peer containers share one frame style */ +.info,.controls{background:var(--panel);border:1px solid var(--edge);border-radius:var(--radius-lg)} +.info{padding:14px 16px} .controls{padding:clamp(10px,1.2vh,14px) 16px;display:flex;flex-wrap:wrap;align-items:center;gap:clamp(8px,0.8vw,14px)} +.compact .controls{display:grid;grid-template-columns:1fr 1fr 1fr;position:sticky;bottom:0;z-index:3;box-shadow:0 -8px 20px rgba(0,0,0,.35)} .compact .controls .ans{max-width:none} .compact .controls .note{grid-column:1/3} .compact .controls #save{grid-column:3} +body.embedded .deck{margin-bottom:62px} /* the file panel's floating Edit button lives in this strip */ +.stage{position:relative;background:var(--panel);border:1px solid var(--edge);border-radius:var(--radius-lg);overflow:auto;min-height:0;line-height:0;font-size:0} +.stage .inner{display:flex;justify-content:center;align-items:center;gap:18px;padding:12px 14px;min-width:100%;min-height:100%} +.content.stacked .stage .inner{flex-direction:column;align-items:center} +.frame{display:none;margin:0} .frame.on{display:block} +figcaption{font:500 11px/1 var(--font);text-transform:uppercase;letter-spacing:.08em;color:var(--fg-muted);margin-bottom:6px;text-align:left} +.pic{position:relative;display:inline-block} .pic img{display:block;height:auto;border-radius:var(--radius-sm);cursor:none} +.box{position:absolute;border:2px solid var(--mark);border-radius:5px;box-shadow:0 0 0 3px rgba(255,176,32,.28),0 0 14px rgba(0,0,0,.45);pointer-events:none} +.zoom{position:sticky;top:10px;float:right;margin:10px 10px -42px 0;z-index:2;display:inline-flex;align-items:center;background:var(--panel);border:1px solid var(--edge);border-radius:var(--radius-full);padding:2px;gap:2px;line-height:1;box-shadow:0 2px 8px rgba(0,0,0,.25)} +.zoom button{font:inherit;font-size:12px;font-weight:500;width:28px;height:26px;border:0;border-radius:var(--radius-full);background:transparent;color:var(--fg-2);cursor:pointer} .zoom .lvl{font-size:11px;color:var(--fg-dim);min-width:38px;text-align:center;font-variant-numeric:tabular-nums} +.loupe{position:fixed;width:180px;height:180px;border-radius:50%;border:2px solid var(--fg);box-shadow:0 0 0 1px rgba(0,0,0,.5),0 8px 24px rgba(0,0,0,.45);background-repeat:no-repeat;pointer-events:none;display:none;z-index:9;background-color:var(--panel)} +.loupe::before,.loupe::after{content:"";position:absolute;left:50%;top:50%;background:var(--mark);box-shadow:0 0 0 1px rgba(0,0,0,.6)} .loupe::before{width:14px;height:2px;margin:-1px 0 0 -7px} .loupe::after{width:2px;height:14px;margin:-7px 0 0 -1px} +.info{display:flex;flex-direction:column;gap:12px;min-width:0;overflow:auto} +.what{font-size:clamp(15px,1.15vw,19px);font-weight:500;margin:0;line-height:1.35} +.cards{display:grid;gap:10px} .row-below .cards{grid-template-columns:repeat(auto-fit,minmax(260px,1fr))} .col-right .cards{grid-template-columns:1fr} +.card{background:var(--inset);border:1px solid var(--edge);border-radius:var(--radius-md);padding:10px 12px} .card h3{margin:0 0 6px;font:500 11px/1 var(--font);text-transform:uppercase;letter-spacing:.08em;color:var(--fg-muted);display:flex;align-items:center;gap:7px} +.card h3 svg{width:14px;height:14px;stroke:currentColor;fill:none;stroke-width:1.8;stroke-linecap:round;stroke-linejoin:round} .card p{margin:0;font-size:13px;color:var(--fg);line-height:1.45} .card .num{margin-top:6px;font-size:11px;color:var(--fg-dim)} +.card.risk{border-color:color-mix(in srgb, var(--mark) 45%, var(--edge))} .card.risk h3{color:var(--mark)} +.ans{flex:1 1 140px;max-width:280px;justify-content:center;height:clamp(34px,4.4vh,52px);font-size:clamp(12px,0.95vw,15px);border-radius:var(--radius-md)} .ans .dot{width:clamp(8px,0.7vw,11px);height:clamp(8px,0.7vw,11px)} +#save{height:clamp(34px,4.4vh,52px);font-size:clamp(12px,0.95vw,15px);padding:0 clamp(14px,1.4vw,26px)} +.note{flex:3 1 220px;font:inherit;font-size:clamp(12px,0.85vw,14px);height:clamp(34px,4.4vh,52px);padding:0 12px;border:1px solid var(--edge);border-radius:var(--radius-md);background:var(--well);color:var(--fg)} .note::placeholder{color:var(--fg-muted)} +@media (max-width:760px){ .top .long{display:none} .top .nav .btn{padding:0 6px} } +.veil{position:fixed;inset:0;background:rgba(0,0,0,.55);display:none;align-items:center;justify-content:center;z-index:30} .veil.on{display:flex} +.dlg{width:min(520px,92vw);background:var(--panel);border:1px solid var(--edge);border-radius:var(--radius-lg);padding:20px;box-shadow:0 20px 60px rgba(0,0,0,.5)} .dlg h2{margin:0 0 10px;font-size:16px;font-weight:500} .dlg p{margin:0 0 10px;color:var(--fg-2);font-size:13px;line-height:1.5} .dlg .warn{display:flex;gap:8px;align-items:flex-start;background:var(--inset);border:1px solid color-mix(in srgb, var(--mark) 45%, var(--edge));border-radius:var(--radius-md);padding:10px 12px;color:var(--fg);margin:12px 0} +.dlg .warn svg{width:16px;height:16px;flex:none;stroke:var(--mark);fill:none;stroke-width:1.8;margin-top:1px} .dlg .row{display:flex;gap:10px;justify-content:flex-end;margin-top:14px} +#laybadge{position:fixed;left:50%;transform:translateX(-50%);bottom:2px;font:10px var(--font);color:#aaa;background:#000;border:1px solid #555;border-radius:999px;padding:3px 8px;z-index:20} +''' + +JS = ''' +const N=%d; const st={}; let cur=0, theme='midnight', zoom=1, loupeOn=true; +const $=s=>document.querySelector(s), $$=s=>[...document.querySelectorAll(s)]; +const stepsEl=$$('.step'); const loupe=$('.loupe'); +const stepFor=i=>stepsEl[i%%2]; // demo: odd steps tall crop, even steps wide crop +if(window.top!==window) document.body.classList.add('embedded'); +const controls=$('.controls'); +/* ── layout scoring: pick the arrangement that shows the pictures largest ── */ +const CARD_COL=360, CARD_ROW=200, CTL=74, GAP=12, PAD=28, CAP=24; +function chooseLayout(step){ + const c=step.querySelector('.content'); const img=step.querySelector('.frame.on img'); if(!img||!img.naturalWidth) return; + const w=img.naturalWidth, h=img.naturalHeight+CAP; const st=step.querySelector('.stage'); + const opts={A:'row-below',B:'col-right stacked',C:'col-right',D:'row-below stacked'}; const score={}; + step.classList.remove('compact-step'); + for(const k in opts){ if(opts[k].includes('col-right')&&c.clientWidth<820){score[k]=0;continue;} + c.className='content '+opts[k]; const SW=st.clientWidth-PAD, SH=st.clientHeight-PAD; const stacked=opts[k].includes('stacked'); + score[k]=Math.min(stacked?SW/w:(SW-18)/2/w, stacked?(SH-18)/2/h:SH/h); } + let best='A'; for(const k of ['B','C','D']) if(score[k]>score[best]*1.05) best=k; // A wins ties (5%%) + if(score[best]<0.5){ c.className='content compact'; step.classList.add('compact-step'); const SW=c.clientWidth-PAD; const s=Math.min(SW/w,1); + step.querySelectorAll('.frame img').forEach(i=>i.style.width=(i.naturalWidth*s*zoom)+'px'); $('#laybadge').textContent='compact · '+Math.round(s*100)+'%%'; return; } + c.className='content '+opts[best]; const s=Math.min(score[best],1.5); + step.querySelectorAll('.frame img').forEach(i=>i.style.width=(i.naturalWidth*s*zoom)+'px'); + $('#laybadge').textContent='layout '+best+' · '+opts[best]+' · '+Math.round(s*100)+'%%'; } +function layout(){ const step=stepFor(cur); const margin=(document.querySelector('main').clientWidth-step.clientWidth)/2; document.body.classList.toggle('thumbs-inline', margin<150); step.querySelectorAll('.frame').forEach(f=>f.classList.toggle('on',f.dataset.theme===theme)); chooseLayout(step); + step.querySelector('.lvl').textContent=Math.round(zoom*100)+'%%'; document.documentElement.dataset.theme=theme; $$('.thumb').forEach(t=>t.classList.toggle('on',t.dataset.v===theme)); + const b=step.querySelector('.frame.on .box'); if(b&&zoom>1) b.scrollIntoView({block:'center',inline:'center'}); } +function paint(){ stepsEl.forEach((s,i)=>s.classList.toggle('on',s===stepFor(cur))); stepFor(cur).querySelector('.content').appendChild(controls); $('#wtitle').textContent=stepFor(cur).dataset.title; $('#wsub').textContent=stepFor(cur).dataset.sub; const a=st[cur]||{}; $$('.ans').forEach(b=>b.classList.toggle('on',b.dataset.v===a.v)); + const note=$('.note'); note.value=a.note||''; note.placeholder=a.v==='other'?'Explain what you’d like instead…':'Add a note (optional)'; + $$('.steps span').forEach((s,i)=>{ s.className=(st[i]?.v||(st[i]?.seen?'skip':''))+(i===cur?' on':''); }); + const done=Object.values(st).filter(x=>x.v).length; $('#count').textContent='step '+(cur+1)+' of '+N+' · '+done+' answered'; + $('#save').disabled=!a.v; $('#prev').disabled=cur===0; $('#next').textContent=cur===N-1?'Last step':'Next ›'; $('#next').disabled=cur===N-1; layout(); } +function go(i){ st[cur]={...(st[cur]||{}),seen:true}; cur=Math.max(0,Math.min(N-1,i)); zoom=1; paint(); } +$$('.ans').forEach(b=>b.onclick=()=>{ st[cur]={...(st[cur]||{}),v:b.dataset.v,seen:true}; paint(); $('.note').focus(); }); +$('.note').addEventListener('input',e=>{ st[cur]={...(st[cur]||{}),note:e.target.value}; }); +$('#save').onclick=()=>go(cur+1); $('#next').onclick=()=>go(cur+1); $('#prev').onclick=()=>go(cur-1); $$('.steps span').forEach((s,i)=>s.onclick=()=>go(i)); +$$('.zin').forEach(b=>b.onclick=()=>{zoom=Math.min(4,Math.round((zoom+0.1)*10)/10);layout();}); $$('.zout').forEach(b=>b.onclick=()=>{zoom=Math.max(1,Math.round((zoom-0.1)*10)/10);layout();}); +$$('.thumb').forEach(t=>t.onclick=()=>{theme=t.dataset.v;layout();}); +$('#done').onclick=()=>{ st[cur]={...(st[cur]||{}),seen:true}; const missing=[]; for(let i=0;i1?'s ':' ')+missing.join(', ')+').'; + $('#first').style.display=missing.length?'inline-flex':'none'; $('#first').onclick=()=>{ $('.veil').classList.remove('on'); go(missing[0]-1); }; $('.veil').classList.add('on'); }; +$('#cancel').onclick=()=>$('.veil').classList.remove('on'); $('#submit').onclick=()=>{ $('.veil').classList.remove('on'); $('#done').textContent='Submitted ✓'; $('#done').disabled=true; }; +const K=2.5,R=90; +$$('.stage').forEach(stage=>{ stage.addEventListener('mousemove',e=>{ if(!loupeOn){loupe.style.display='none';return;} const img=$$('.step.on .frame.on img').find(i=>{const r=i.getBoundingClientRect();return e.clientX>=r.left&&e.clientX<=r.right&&e.clientY>=r.top&&e.clientY<=r.bottom;}); if(!img){loupe.style.display='none';return;} + const r=img.getBoundingClientRect(); const x=e.clientX-r.left,y=e.clientY-r.top; loupe.style.display='block'; loupe.style.left=(e.clientX-R)+'px'; loupe.style.top=(e.clientY-R)+'px'; loupe.style.backgroundImage='url('+img.src+')'; loupe.style.backgroundSize=(r.width*K)+'px '+(r.height*K)+'px'; loupe.style.backgroundPosition=(-x*K+R)+'px '+(-y*K+R)+'px'; }); + stage.addEventListener('mouseleave',()=>loupe.style.display='none'); }); +document.addEventListener('keydown',e=>{ if(e.target.tagName==='INPUT')return; if(e.key==='ArrowRight')go(cur+1); if(e.key==='ArrowLeft')go(cur-1); if(e.key==='+'||e.key==='=')$('.step.on .zin').click(); if(e.key==='-')$('.step.on .zout').click(); if(e.key==='l'){loupeOn=!loupeOn; if(!loupeOn)loupe.style.display='none'; $$('.pic img').forEach(i=>i.style.cursor=loupeOn?'none':'default');} }); +const Q=new URLSearchParams(location.search); if(Q.get('theme'))theme=Q.get('theme'); if(Q.get('step'))cur=+Q.get('step')-1; +if(Q.get('demo')){ st[0]={v:'yes',seen:true}; st[1]={v:'no',seen:true}; st[2]={seen:true}; st[3]={v:'other',note:'make the pencil bigger',seen:true}; cur=Q.get('step')?cur:4; } +window.addEventListener('resize',layout); window.addEventListener('load',paint); $$('.frame img').forEach(i=>i.addEventListener('load',layout)); paint(); +if(Q.get('dialog')){ window.addEventListener('load',()=>$('#done').click()); } +''' + +steps = '
' + ''.join('' for _ in range(N)) + '
' + +page = f'''Deck mockup G +
+
Review deckPhase C review
+
·
+
{steps}
+
+
{steps_html}
+
+ + + +
+
+
+

Submit your feedback?

+

Your answers have been saving to a file next to this deck as you went. Submitting tells Claude you're finished — it picks them up in the session and replies there. Nothing to copy or paste: close this tab and go back to the conversation.

+
{ICON_WARN} Skipped steps are sent as "no answer"; Claude will leave those unchanged.
+
+''' + +os.makedirs(OUT, exist_ok=True) +open(os.path.join(OUT, '2026-08-27-deck-mockup-g.html'), 'w').write(page) +print('ok') diff --git a/docs/archive/plans/2026-08-27-review-deck-v2.md b/docs/archive/plans/2026-08-27-review-deck-v2.md new file mode 100644 index 00000000..5cc250eb --- /dev/null +++ b/docs/archive/plans/2026-08-27-review-deck-v2.md @@ -0,0 +1,2237 @@ +--- +status: shipped +created: 2026-08-27 +spec: docs/archive/specs/2026-08-27-review-deck-v2-design.md +--- + +# Review Deck v2 Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace the v1 review deck with the approved page (mockup G), a local server that saves answers to a file and exits when Destin submits, rig-measured highlight boxes, builder-enforced writing rules, and four hand-off rig fixes. + +**Architecture:** `scripts/ui-review/review-cards.py` becomes a thin CLI over a new `scripts/ui-review/deck/` Python package (`spec` → `boxes` → `crops` → `build` → `serve`). `build` always crops first — there is no separate crop command and no intermediate boxes file, because a stale one would draw wrong boxes with no error; `serve` builds, then serves; `wait` blocks on the answers file alone, so "Destin is done" never depends on the serving process staying alive. The page itself is three static assets (`page.html.tmpl`, `page.css`, `page.js`) that `build` inlines with the deck data as one JSON object, so the browser renders steps from data. `shot.mjs` gains a `measure` list per shot and a run id; `coverage.mjs` merges by run id per plan; `run-review.sh` probes CDP ports and scopes sheets to the run. + +**Tech Stack:** Python 3.14 stdlib only (`http.server`, `json`, `subprocess`, `unittest`); ImageMagick 7 (`magick`); Node 26 (`node --test`, raw CDP over `WebSocket`, `google-chrome-stable`) for the rig and the render check. No new dependencies. + +## Global Constraints + +- Everything lives in the **workspace repo** (`youcoded-dev`), worktree `worktrees/_deck-tooling`, branch `feat/review-deck-v2`. No sub-repo code changes. Never touch the live app (workspace rule). +- Amber `#FFB020` is the deck's only identity colour (spec §2). Built-in token values must equal `youcoded/desktop/src/renderer/styles/globals.css` (Task 3 test). +- Writing rules (spec §5), exact: headline ≤ 25 words; `changed` and `notice` required; banned words (case-insensitive, whole-word): `token, primitive, selector, IPC, prop, props, reducer, handler, component, Tailwind, CSS class, React, DOM, z-index`; warnings for `box` highlights, auto-highlight > 60% of the crop, risk > 40 words, `measured` without a digit. +- Layout picker (spec §3.4): B/C need content ≥ 820px; A wins ties within 5%; best < 50% → compact; upscale cap 150%. +- Answers file is `.answers.json` next to the spec; `serve` exits 0 on submit, 2 on timeout (default 240 min), 3 when the same spec is already served. `wait` exits 0/2 the same way and reads only the answers file. `.gitignore` gains `*.answers.json` and `*.serve.json`. +- **The worktree does NOT contain the sub-repo checkouts.** `youcoded/` and `wecoded-themes/` live only at the workspace root (one level above `worktrees/`). Everything that needs them resolves the root with `spec.workspace_root()` — walk up from the package until a directory containing `wecoded-themes/themes` is found; `YOUCODED_WORKSPACE` overrides. Tests that read those files **fail** when the root is not found — never skip (a skipped pin is no pin). +- ImageMagick dilate is `-morphology Dilate Square:1` (a 3×3 kernel). `Square:3` is a 7×7 kernel and grows a box 3 px per side — measured on 2026-08-27, it broke the auto-box tolerances. +- Every non-trivial edit carries a WHY comment (Destin reads the code through comments). +- Python tests: `python3 -m unittest discover -s scripts/ui-review/tests -p 'test_*.py'`. Node tests: `node --test scripts/ui-review/tests/*.test.mjs`. +- Commit after every task with the `Co-Authored-By` / `Claude-Session` trailers from the session's Bash instructions. + +--- + +## File map + +| Path | Responsibility | +|---|---| +| `scripts/ui-review/review-cards.py` | CLI: `build`, `serve`, `wait`. Rewritten (v1 removed). | +| `scripts/ui-review/deck/__init__.py` | package marker | +| `scripts/ui-review/deck/spec.py` | load spec, merge `crops.json`, `workspace_root()`, writing rules → `(errors, warnings)` | +| `scripts/ui-review/deck/boxes.py` | geometry parsing, window-px → crop-% mapping, pixel-diff bounding box | +| `scripts/ui-review/deck/crops.py` | cut crops with `magick` (one cut per crop × theme × run), resolve each step's highlight per theme × run | +| `scripts/ui-review/deck/build.py` | crop, then inline assets + tokens + deck JSON → one HTML file; refuses on missing pictures/boxes | +| `scripts/ui-review/deck/serve.py` | HTTP server, atomic answers file, submit → exit, feedback summary, browser open, `wait_for_submit` | +| `.gitignore` | `*.answers.json`, `*.serve.json` | +| `scripts/ui-review/deck/tokens.json` | the four built-in token sets the page inlines | +| `scripts/ui-review/deck/page.html.tmpl`, `page.css`, `page.js` | the page (from mockup G) | +| `scripts/ui-review/tests/fixture.py` | builds a synthetic run dir + spec in a temp dir for the Python tests | +| `scripts/ui-review/tests/test_spec.py`, `test_boxes.py`, `test_crops.py`, `test_build.py`, `test_serve.py`, `test_tokens.py`, `test_cli.py` | unit tests | +| `scripts/ui-review/tests/shot-measure.test.mjs`, `coverage.test.mjs`, `deck-render.test.mjs` | node tests | +| `scripts/ui-review/probe-ports.sh` | exits 1 naming any listening port among its args (hand-off gap 1) | +| `scripts/ui-review/shot.mjs` | `measure` per shot; `run` id on every manifest entry | +| `scripts/ui-review/coverage.mjs` | per-plan newest-run merge (gap 6) | +| `scripts/ui-review/run-review.sh` | run id, port probe, sheets scoped to the run (gaps 1, 7) | +| `scripts/ui-review/README.md`, `.claude/skills/ui-review/SKILL.md`, `CLAUDE.md`, `ROADMAP.md`, memory | docs | + +--- + +### Task 1: Spec loading and the writing rules + +**Files:** +- Create: `scripts/ui-review/deck/__init__.py` (empty) +- Create: `scripts/ui-review/deck/spec.py` +- Test: `scripts/ui-review/tests/test_spec.py` + +**Interfaces:** +- Produces: `load_spec(path) -> dict` (adds `_base`, `_stem`, `_crops`, default `themes`); `validate(spec) -> (errors: list[str], warnings: list[str])`; `run_names(spec) -> list[str]`; `word_count(s) -> int`; `banned_in(text) -> list[str]`; `workspace_root() -> str` (raises `SpecError` when no ancestor holds `wecoded-themes/themes` and `YOUCODED_WORKSPACE` is unset); `SpecError(Exception)`; constants `DEFAULT_THEMES`, `BANNED`, `AUTO_WARN_FRACTION = 0.6`. + +- [ ] **Step 1: Write the failing tests** + +```python +# scripts/ui-review/tests/test_spec.py +import json, os, sys, tempfile, unittest +HERE = os.path.dirname(os.path.abspath(__file__)) +sys.path.insert(0, os.path.dirname(HERE)) +from deck.spec import load_spec, validate, run_names, word_count, banned_in, workspace_root, SpecError + +def write_spec(d, **over): + spec = {"title": "T", "key": "t", "out": "t.html", "images": "images", "runs": {"before": "/a", "after": "/b"}, + "crops": {"c": ["main", "home", "100x50+10+20"]}, + "steps": [{"id": "S-1", "surface": "Home", "path": "Chat", "crop": "c", + "headline": "Short headline.", "changed": "What changed.", "notice": "You will notice."}]} + spec.update(over) + p = os.path.join(d, 'deck.json'); json.dump(spec, open(p, 'w')); return p + +class SpecTests(unittest.TestCase): + def setUp(self): self.d = tempfile.mkdtemp() + def test_load_merges_shared_crops_and_defaults(self): + s = load_spec(write_spec(self.d)) + self.assertEqual(s['_stem'], 'deck'); self.assertIn('bubble', s['_crops']); self.assertIn('c', s['_crops']) + self.assertEqual(s['themes'][0], 'midnight'); self.assertEqual(run_names(s), ['before', 'after']) + def test_missing_top_level_key_raises(self): + with self.assertRaises(SpecError): load_spec(self._without('title')) + def test_workspace_root_holds_the_sub_repos(self): + # The worktree has no youcoded/ or wecoded-themes/ of its own; the root above worktrees/ does. + root = workspace_root() + self.assertTrue(os.path.isdir(os.path.join(root, 'wecoded-themes', 'themes')), root) + self.assertTrue(os.path.isfile(os.path.join(root, 'youcoded', 'desktop', 'src', 'renderer', 'styles', 'globals.css')), root) + def _without(self, key): + p = write_spec(self.d); s = json.load(open(p)); del s[key]; json.dump(s, open(p, 'w')); return p + def test_three_runs_rejected(self): + with self.assertRaises(SpecError): load_spec(write_spec(self.d, runs={"a": "/a", "b": "/b", "c": "/c"})) + def test_valid_spec_has_no_errors(self): + self.assertEqual(validate(load_spec(write_spec(self.d))), ([], [])) + def test_headline_word_limit(self): + s = load_spec(write_spec(self.d)); s['steps'][0]['headline'] = ' '.join(['word'] * 26) + errors, _ = validate(s); self.assertTrue(any('26 words' in e for e in errors)) + def test_banned_words_whole_word_case_insensitive(self): + self.assertEqual(banned_in('The Token is a primitive'), ['token', 'primitive']) + self.assertEqual(banned_in('property tokens'), []) # not whole words + self.assertEqual(banned_in('ipc call via the DOM'), ['ipc', 'dom']) + s = load_spec(write_spec(self.d)); s['steps'][0]['changed'] = 'Uses a new CSS class' + errors, _ = validate(s); self.assertTrue(any('banned word "css class"' in e for e in errors)) + def test_required_fields(self): + s = load_spec(write_spec(self.d)); del s['steps'][0]['notice']; s['steps'][0]['surface'] = '' + errors, _ = validate(s); self.assertTrue(any('missing notice' in e for e in errors)); self.assertTrue(any('missing surface' in e for e in errors)) + def test_unknown_crop_is_an_error(self): + s = load_spec(write_spec(self.d)); s['steps'][0]['crop'] = 'nope' + self.assertTrue(any('unknown crop' in e for e in validate(s)[0])) + def test_highlight_rules(self): + s = load_spec(write_spec(self.d, runs={"today": "/a"})) + self.assertTrue(any('needs a highlight' in e for e in validate(s)[0])) + s['steps'][0]['highlight'] = 'auto'; self.assertTrue(any('"auto" highlight needs' in e for e in validate(s)[0])) + s['steps'][0]['highlight'] = {'box': [1, 2, 3, 4]}; errors, warnings = validate(s) + self.assertEqual(errors, []); self.assertTrue(any('hand-placed box' in w for w in warnings)) + s['steps'][0]['highlight'] = {'nothing': 1}; self.assertTrue(any('selector, text or box' in e for e in validate(s)[0])) + def test_warnings_for_long_risk_and_numberless_measured(self): + s = load_spec(write_spec(self.d)); s['steps'][0]['risk'] = ' '.join(['r'] * 41); s['steps'][0]['measured'] = 'a bit taller' + _, warnings = validate(s); self.assertEqual(len(warnings), 2) + def test_duplicate_ids(self): + s = load_spec(write_spec(self.d)); s['steps'].append(dict(s['steps'][0])) + self.assertTrue(any('duplicate id' in e for e in validate(s)[0])) + def test_word_count(self): + self.assertEqual(word_count("it's a two-line, five-word headline"), 5) + +if __name__ == '__main__': unittest.main() +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cd /home/destin/youcoded-dev/worktrees/_deck-tooling && python3 -m unittest scripts/ui-review/tests/test_spec.py -v 2>&1 | tail -3` +Expected: `ModuleNotFoundError: No module named 'deck'` + +- [ ] **Step 3: Write `spec.py`** + +```python +# scripts/ui-review/deck/spec.py +"""Deck spec: loading, crop-registry merge, and the writing rules the builder enforces. + +WHY rules in code: on 2026-08-25 a taste argument went into a review as if it were a defect, +and prose reviews were rejected three times for being unreadable — so the deck's vocabulary +(headline · What changed · You'll notice · Risk) and its word limits are checked here, not +remembered. Spec: docs/active/specs/2026-08-27-review-deck-v2-design.md §4–5.""" +import json +import os +import re + +HERE = os.path.dirname(os.path.abspath(__file__)) +UI_REVIEW = os.path.dirname(HERE) +DEFAULT_THEMES = ['midnight', 'light', 'creme', 'dark', 'halftone-dimension', 'meadow-mist'] +# Whole-word, case-insensitive. "px" and numbers are fine — measurements are wanted. +BANNED = ['token', 'primitive', 'selector', 'ipc', 'prop', 'props', 'reducer', 'handler', + 'component', 'tailwind', 'css class', 'react', 'dom', 'z-index'] +TEXT_FIELDS = ['headline', 'changed', 'measured', 'notice', 'risk', 'surface', 'path'] +HEADLINE_MAX = 25 +RISK_WARN = 40 +AUTO_WARN_FRACTION = 0.6 # an auto-highlight covering more than this much of the crop is "whole surface" + + +class SpecError(Exception): + pass + + +def workspace_root(): + """The directory that holds the sub-repo checkouts (youcoded/, wecoded-themes/). + WHY walk up: this package usually runs from a worktree (worktrees//scripts/…), and a + worktree holds only the workspace repo — the sub-repos are cloned once, at the root above + worktrees/. Resolving relative to the package silently found nothing on 2026-08-27.""" + if os.environ.get('YOUCODED_WORKSPACE'): + return os.environ['YOUCODED_WORKSPACE'] + d = HERE + while True: + if os.path.isdir(os.path.join(d, 'wecoded-themes', 'themes')): + return d + parent = os.path.dirname(d) + if parent == d: + raise SpecError('no workspace root above ' + HERE + ' holds wecoded-themes/themes (set YOUCODED_WORKSPACE)') + d = parent + + +def load_spec(path): + with open(path) as f: + spec = json.load(f) + for k in ('title', 'key', 'out', 'images', 'runs', 'steps'): + if k not in spec or spec[k] is None: + raise SpecError(f'spec is missing "{k}"') + if not 1 <= len(spec['runs']) <= 2: + raise SpecError('runs must have one entry (today) or two (before, after)') + spec['_base'] = os.path.dirname(os.path.abspath(path)) + spec['_stem'] = os.path.splitext(os.path.basename(path))[0] + with open(os.path.join(UI_REVIEW, 'crops.json')) as f: + shared = json.load(f) + shared.pop('_comment', None) + spec['_crops'] = {**shared, **spec.get('crops', {})} + spec.setdefault('themes', list(DEFAULT_THEMES)) + return spec + + +def run_names(spec): + """Display order of the runs: before then after when both exist, else as written.""" + r = list(spec['runs'].keys()) + return ['before', 'after'] if set(r) == {'before', 'after'} else r + + +def word_count(s): + return len(re.findall(r"[\w'’-]+", s or '')) + + +def banned_in(text): + low = (text or '').lower() + return [w for w in BANNED if re.search(r'(? HEADLINE_MAX: + errors.append(f'{sid}: headline is {word_count(st["headline"])} words (max {HEADLINE_MAX})') + for k in TEXT_FIELDS: + for w in banned_in(st.get(k)): + errors.append(f'{sid}: {k} uses banned word "{w}"') + hl = st.get('highlight', 'auto' if two_runs else None) + if hl is None: + errors.append(f'{sid}: a one-run deck needs a highlight (selector or text)') + elif hl == 'auto': + if not two_runs: + errors.append(f'{sid}: "auto" highlight needs a before and an after run') + elif isinstance(hl, dict): + if not any(k in hl for k in ('selector', 'text', 'box')): + errors.append(f'{sid}: highlight must be "auto" or have selector, text or box') + elif 'box' in hl: + warnings.append(f'{sid}: hand-placed box — prefer a selector so the rig measures it') + else: + errors.append(f'{sid}: highlight must be "auto" or an object') + if word_count(st.get('risk')) > RISK_WARN: + warnings.append(f'{sid}: risk is {word_count(st["risk"])} words — keep it to one sentence') + if st.get('measured') and not re.search(r'\d', st['measured']): + warnings.append(f'{sid}: measured has no number in it') + return errors, warnings +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `python3 -m unittest scripts/ui-review/tests/test_spec.py -v 2>&1 | tail -3` +Expected: `OK` (13 tests) + +- [ ] **Step 5: Commit** + +```bash +git add scripts/ui-review/deck/__init__.py scripts/ui-review/deck/spec.py scripts/ui-review/tests/test_spec.py +git commit -m "feat(ui-review): deck v2 spec loader and the writing rules the builder enforces" +``` + +--- + +### Task 2: Box maths — geometry, window-px → crop-%, pixel-diff bounding box + +**Files:** +- Create: `scripts/ui-review/deck/boxes.py` +- Test: `scripts/ui-review/tests/test_boxes.py` + +**Interfaces:** +- Produces: `parse_geometry("WxH+X+Y") -> (w, h, x, y)`; `rect_to_pct({x,y,w,h}, geo) -> [x%, y%, w%, h%] | None`; `diff_bbox(a_png, b_png, threshold='6%', pad=6) -> {x,y,w,h} | None` (pixels of the crop); `image_size(png) -> (w, h)`; `px_to_pct(box, (W, H)) -> [x%, y%, w%, h%]`. + +- [ ] **Step 1: Write the failing tests** + +```python +# scripts/ui-review/tests/test_boxes.py +import os, subprocess, sys, tempfile, unittest +HERE = os.path.dirname(os.path.abspath(__file__)) +sys.path.insert(0, os.path.dirname(HERE)) +from deck.boxes import parse_geometry, rect_to_pct, diff_bbox, image_size, px_to_pct + +class GeometryTests(unittest.TestCase): + def test_parse(self): + self.assertEqual(parse_geometry('440x600+500+150'), (440, 600, 500, 150)) + with self.assertRaises(ValueError): parse_geometry('440x600') + def test_rect_inside_crop(self): + # crop is 400x200 at (100, 50); element at window (200, 100) 100x50 → 25%, 25%, 25%, 25% + self.assertEqual(rect_to_pct({'x': 200, 'y': 100, 'w': 100, 'h': 50}, '400x200+100+50'), [25.0, 25.0, 25.0, 25.0]) + def test_rect_partly_outside_is_clipped(self): + self.assertEqual(rect_to_pct({'x': 0, 'y': 0, 'w': 200, 'h': 100}, '400x200+100+50'), [0.0, 0.0, 25.0, 25.0]) + def test_rect_fully_outside_is_none(self): + self.assertIsNone(rect_to_pct({'x': 900, 'y': 900, 'w': 10, 'h': 10}, '400x200+100+50')) + def test_px_to_pct(self): + self.assertEqual(px_to_pct({'x': 50, 'y': 20, 'w': 100, 'h': 40}, (200, 80)), [25.0, 25.0, 50.0, 50.0]) + +class DiffTests(unittest.TestCase): + def setUp(self): + self.d = tempfile.mkdtemp(); self.a = os.path.join(self.d, 'a.png'); self.b = os.path.join(self.d, 'b.png') + subprocess.run(['magick', '-size', '200x100', 'xc:#333333', self.a], check=True) + subprocess.run(['magick', self.a, '-fill', 'red', '-draw', 'rectangle 50,20 89,49', self.b], check=True) + def test_size(self): self.assertEqual(image_size(self.a), (200, 100)) + def test_identical_images_have_no_box(self): self.assertIsNone(diff_bbox(self.a, self.a)) + def test_changed_rectangle_is_found_with_padding(self): + box = diff_bbox(self.a, self.b) + self.assertIsNotNone(box) + # contains the 40x30 rectangle at (50,20) and is padded, but not by much + self.assertLessEqual(box['x'], 50); self.assertLessEqual(box['y'], 20) + self.assertGreaterEqual(box['x'] + box['w'], 90); self.assertGreaterEqual(box['y'] + box['h'], 50) + self.assertGreaterEqual(box['x'], 40); self.assertGreaterEqual(box['y'], 10) + self.assertLessEqual(box['w'], 62); self.assertLessEqual(box['h'], 52) + def test_box_never_leaves_the_image(self): + c = os.path.join(self.d, 'c.png'); subprocess.run(['magick', self.a, '-fill', 'red', '-draw', 'rectangle 0,0 9,9', c], check=True) + box = diff_bbox(self.a, c); self.assertEqual((box['x'], box['y']), (0, 0)) + def test_whole_image_change_is_the_whole_image(self): + # Trim of a uniform image is empty whether nothing or everything changed; the border in diff_bbox tells them apart. + w = os.path.join(self.d, 'w.png'); subprocess.run(['magick', '-size', '200x100', 'xc:white', w], check=True) + self.assertEqual(diff_bbox(self.a, w), {'x': 0, 'y': 0, 'w': 200, 'h': 100}) + +if __name__ == '__main__': unittest.main() +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `python3 -m unittest scripts/ui-review/tests/test_boxes.py 2>&1 | tail -2` +Expected: `ModuleNotFoundError: No module named 'deck.boxes'` + +- [ ] **Step 3: Write `boxes.py`** + +```python +# scripts/ui-review/deck/boxes.py +"""Highlight-box maths. Two sources of truth for a box, neither hand-typed: + - the rig measured an element (window pixels) → rect_to_pct maps it into the crop; + - nothing was named → diff_bbox finds what changed between the before and after crops. +WHY: v1 decks carried hand-estimated percentages (hand-off gap 3) and the rings drifted.""" +import re +import subprocess + +GEO = re.compile(r'(\d+)x(\d+)\+(\d+)\+(\d+)') + + +def parse_geometry(geo): + m = GEO.fullmatch(geo) + if not m: + raise ValueError(f'bad geometry {geo!r}, want WxH+X+Y') + w, h, x, y = map(int, m.groups()) + return w, h, x, y + + +def _r(v): + return round(v, 2) + + +def rect_to_pct(rect, geo): + """Window-pixel rect {x,y,w,h} → [x%, y%, w%, h%] of the crop, clipped; None when outside.""" + cw, ch, cx, cy = parse_geometry(geo) + x0, y0 = max(rect['x'], cx), max(rect['y'], cy) + x1, y1 = min(rect['x'] + rect['w'], cx + cw), min(rect['y'] + rect['h'], cy + ch) + if x1 <= x0 or y1 <= y0: + return None + return [_r((x0 - cx) / cw * 100), _r((y0 - cy) / ch * 100), _r((x1 - x0) / cw * 100), _r((y1 - y0) / ch * 100)] + + +def image_size(png): + out = subprocess.run(['magick', 'identify', '-format', '%w %h', png], capture_output=True, text=True, check=True).stdout.split() + return int(out[0]), int(out[1]) + + +def px_to_pct(box, size): + W, H = size + return [_r(box['x'] / W * 100), _r(box['y'] / H * 100), _r(box['w'] / W * 100), _r(box['h'] / H * 100)] + + +def diff_bbox(a, b, threshold='6%', pad=6): + """Bounding box (crop pixels) of what differs between two same-size PNGs; None if nothing does. + `%@` is ImageMagick's trim box of the thresholded difference; the 3×3 dilate (`Square:1` — + `Square:3` would be 7×7 and grow the box 3 px a side) joins hairline changes into one region. + WHY the 1 px black border: trim of a UNIFORM image is `0x0+W+H` whether it is all black + (nothing changed) or all white (everything changed) — a whole-surface change read as "nothing + differs" until 2026-08-27. The border gives trim something to shrink from: identical → 0x0, + everything → WxH+1+1, so x and y come back shifted by 1.""" + out = subprocess.run(['magick', a, b, '-compose', 'difference', '-composite', '-threshold', threshold, + '-morphology', 'Dilate', 'Square:1', '-bordercolor', 'black', '-border', '1', + '-format', '%@', 'info:'], + capture_output=True, text=True, check=True).stdout.strip() + m = GEO.fullmatch(out) + if not m: + return None + w, h, x, y = map(int, m.groups()) + if w * h < 4: + return None + x, y = x - 1, y - 1 # undo the border's shift + W, H = image_size(a) + x0, y0 = max(0, x - pad), max(0, y - pad) + x1, y1 = min(W, x + w + pad), min(H, y + h + pad) + return {'x': x0, 'y': y0, 'w': x1 - x0, 'h': y1 - y0} +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `python3 -m unittest scripts/ui-review/tests/test_boxes.py -v 2>&1 | tail -3` +Expected: `OK` (10 tests). Measured on 2026-08-27 with ImageMagick 7.1.2, with the border: identical images print `0x0+202+102` (plus a harmless warning on stderr, exit 0) — the `w * h < 4` guard turns that into `None`; a fully changed image prints `200x100+1+1`; the 40×30 rectangle comes back `42x32+50+20` after the 3×3 dilate (→ 49,19 once the border shift is undone). + +- [ ] **Step 5: Commit** + +```bash +git add scripts/ui-review/deck/boxes.py scripts/ui-review/tests/test_boxes.py +git commit -m "feat(ui-review): highlight-box maths — measured rect to crop %, pixel-diff bounding box" +``` + +--- + +### Task 3: Built-in theme tokens, checked against `globals.css` + +**Files:** +- Create: `scripts/ui-review/deck/tokens.json` +- Test: `scripts/ui-review/tests/test_tokens.py` + +**Interfaces:** +- Produces: `tokens.json` = `{ "": { "canvas": "#…", …13 keys…, "_dark": bool } }` for `light`, `dark`, `midnight`, `creme`. Keys (exact): `canvas panel inset well accent on-accent fg fg-2 fg-dim fg-muted fg-faint edge link`. + +- [ ] **Step 1: Write the failing test** + +```python +# scripts/ui-review/tests/test_tokens.py +"""The deck inlines the built-in token values; this pins them to globals.css so a theme +tweak in the app cannot leave the deck wearing last month's Midnight.""" +import json, os, re, sys, unittest +HERE = os.path.dirname(os.path.abspath(__file__)) +sys.path.insert(0, os.path.dirname(HERE)) +from deck.spec import workspace_root +TOKENS = os.path.join(os.path.dirname(HERE), 'deck', 'tokens.json') +# Resolved through workspace_root(), never relative to this file: the worktree has no youcoded/. +GLOBALS = os.path.join(workspace_root(), 'youcoded', 'desktop', 'src', 'renderer', 'styles', 'globals.css') +KEYS = ['canvas', 'panel', 'inset', 'well', 'accent', 'on-accent', 'fg', 'fg-2', 'fg-dim', 'fg-muted', 'fg-faint', 'edge', 'link'] + +def css_block(css, theme): + sel = f'[data-theme="{theme}"]' + i = css.index(sel); j = css.index('}', i) + return css[i:j] + +class TokenTests(unittest.TestCase): + def test_four_themes_with_all_keys(self): + t = json.load(open(TOKENS)) + self.assertEqual(sorted(t), ['creme', 'dark', 'light', 'midnight']) + for theme, tok in t.items(): + for k in KEYS: self.assertRegex(tok[k], r'^#[0-9A-Fa-f]{6}$', f'{theme}.{k}') + self.assertIsInstance(tok['_dark'], bool) + def test_values_match_globals_css(self): + self.assertTrue(os.path.exists(GLOBALS), GLOBALS + ' missing — the pin must fail, not skip') + css = open(GLOBALS).read(); t = json.load(open(TOKENS)) + for theme, tok in t.items(): + block = css_block(css, theme) + for k in KEYS: + m = re.search(r'--' + re.escape(k) + r':\s*(#[0-9A-Fa-f]{6})', block) + self.assertIsNotNone(m, f'{theme}: --{k} not in globals.css block'); self.assertEqual(tok[k].upper(), m.group(1).upper(), f'{theme}.{k}') + self.assertEqual(tok['_dark'], 'color-scheme: dark' in block, theme) + +if __name__ == '__main__': unittest.main() +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `python3 -m unittest scripts/ui-review/tests/test_tokens.py 2>&1 | tail -2` +Expected: `FileNotFoundError: … tokens.json` + +- [ ] **Step 3: Write `tokens.json`** (values from `globals.css` on 2026-08-27; the test is the authority — if it disagrees, copy the value it reports from `globals.css`) + +```json +{ + "light": {"canvas": "#F2F2F2", "panel": "#EAEAEA", "inset": "#D7D7D7", "well": "#F9F9F9", "accent": "#1A1A1A", "on-accent": "#F2F2F2", "fg": "#1A1A1A", "fg-2": "#444444", "fg-dim": "#656565", "fg-muted": "#797979", "fg-faint": "#989898", "edge": "#C0C0C0", "link": "#2055CA", "_dark": false}, + "dark": {"canvas": "#111111", "panel": "#191919", "inset": "#222222", "well": "#1C1C1C", "accent": "#D4D4D4", "on-accent": "#111111", "fg": "#E0E0E0", "fg-2": "#B0B0B0", "fg-dim": "#999999", "fg-muted": "#6C6C6C", "fg-faint": "#515151", "edge": "#393939", "link": "#66AAFF", "_dark": true}, + "midnight": {"canvas": "#0D1117", "panel": "#161B22", "inset": "#21262D", "well": "#0D1117", "accent": "#B1BAC4", "on-accent": "#0D1117", "fg": "#C9D1D9", "fg-2": "#A0AAB4", "fg-dim": "#8B949E", "fg-muted": "#6E7681", "fg-faint": "#4E555E", "edge": "#343A41", "link": "#58A6FF", "_dark": true}, + "creme": {"canvas": "#F6EEE1", "panel": "#EBE1D1", "inset": "#D8CCB9", "well": "#F9F0E2", "accent": "#3D3229", "on-accent": "#F6EEE1", "fg": "#2C2418", "fg-2": "#564938", "fg-dim": "#695E4D", "fg-muted": "#7D7161", "fg-faint": "#9A8F7F", "edge": "#C4B8A6", "link": "#5B4A1E", "_dark": false} +} +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `python3 -m unittest scripts/ui-review/tests/test_tokens.py -v 2>&1 | tail -3` +Expected: `OK` (2 tests). The `light` block in `globals.css` is `[data-theme="light"], :root {` — `css_block` finds it by the `[data-theme="light"]` prefix, which is fine. + +- [ ] **Step 5: Commit** + +```bash +git add scripts/ui-review/deck/tokens.json scripts/ui-review/tests/test_tokens.py +git commit -m "feat(ui-review): built-in theme tokens for the deck, pinned to globals.css" +``` + +--- + +### Task 4: Test fixture — a synthetic run directory and spec + +**Files:** +- Create: `scripts/ui-review/tests/fixture.py` + +**Interfaces:** +- Produces: `make_fixture(tmpdir, themes=('midnight','light')) -> spec_path`. Layout it creates: + - `/runs/before/shots-main//home.png` and `…/runs/after/…` — 1440×900 flat images; `after` adds a red rectangle at window (560, 260) size 120×40 (inside crop `c` below) and a blue one at (20, 20) 10×10 (outside it). + - `/runs/{before,after}/shots-main/manifest-main-x.json` with one entry per theme: `{name:'home', theme, verified:true, run:'1', measures: {'#send': {x:600,y:300,w:80,h:30}, 'text:Send': {x:600,y:300,w:80,h:30}}}`. + - `/deck/deck.json` with crops `{"c": ["main","home","400x200+500+250"]}` and two steps: `S-1` (auto) and `S-2` (`{"selector": "#send"}`), both crop `c`, and a third `S-3` with `{"text": "Send"}`. + +- [ ] **Step 1: Write the fixture** + +```python +# scripts/ui-review/tests/fixture.py +"""A synthetic screenshot run for the deck tests: flat 1440x900 'shots' with a known +rectangle that changes, and a manifest with known measurements. Lets every deck test run +without Chrome or the workbench.""" +import json, os, subprocess + +GEO = '400x200+500+250' + +def make_fixture(tmp, themes=('midnight', 'light')): + for run in ('before', 'after'): + for theme in themes: + d = os.path.join(tmp, 'runs', run, 'shots-main', theme); os.makedirs(d, exist_ok=True) + cmd = ['magick', '-size', '1440x900', 'xc:#202020' if theme == 'midnight' else 'xc:#EEEEEE'] + if run == 'after': + cmd += ['-fill', 'red', '-draw', 'rectangle 560,260 679,299', '-fill', 'blue', '-draw', 'rectangle 20,20 29,29'] + subprocess.run(cmd + [os.path.join(d, 'home.png')], check=True) + mf = [{'name': 'home', 'theme': t, 'verified': True, 'run': '1', 'file': f'{t}/home.png', + 'measures': {'#send': {'x': 600, 'y': 300, 'w': 80, 'h': 30}, 'text:Send': {'x': 600, 'y': 300, 'w': 80, 'h': 30}}} for t in themes] + json.dump(mf, open(os.path.join(tmp, 'runs', run, 'shots-main', 'manifest-main-x.json'), 'w')) + deck = os.path.join(tmp, 'deck'); os.makedirs(deck, exist_ok=True) + spec = {'title': 'Fixture review', 'key': 'fixture', 'out': 'fixture.html', 'images': 'images', + 'runs': {'before': os.path.join(tmp, 'runs', 'before'), 'after': os.path.join(tmp, 'runs', 'after')}, + 'themes': list(themes), 'crops': {'c': ['main', 'home', GEO]}, + 'steps': [ + {'id': 'S-1', 'surface': 'Home', 'path': 'Chat', 'crop': 'c', 'headline': 'A red block appeared.', + 'changed': 'A red block was painted.', 'measured': '120 px wide', 'notice': 'You see red.', 'risk': 'None really.'}, + {'id': 'S-2', 'surface': 'Home', 'path': 'Chat', 'crop': 'c', 'highlight': {'selector': '#send'}, + 'headline': 'The send button moved.', 'changed': 'Moved 4 px.', 'notice': 'Nothing much.'}, + {'id': 'S-3', 'surface': 'Home', 'path': 'Chat', 'crop': 'c', 'highlight': {'text': 'Send'}, + 'headline': 'Same, by text.', 'changed': 'Moved 4 px.', 'notice': 'Nothing much.'}]} + p = os.path.join(deck, 'deck.json'); json.dump(spec, open(p, 'w'), indent=1); return p +``` + +- [ ] **Step 2: Smoke it** + +Run: `python3 -c "import sys,tempfile; sys.path.insert(0,'scripts/ui-review/tests'); from fixture import make_fixture; import os; p=make_fixture(tempfile.mkdtemp()); print(p, os.path.exists(os.path.join(os.path.dirname(os.path.dirname(p)),'runs','after','shots-main','light','home.png')))"` +Expected: a path and `True` + +- [ ] **Step 3: Commit** + +```bash +git add scripts/ui-review/tests/fixture.py +git commit -m "test(ui-review): synthetic run fixture for the deck tests" +``` + +--- + +### Task 5: `crops.py` — cut the crops and resolve every highlight box + +**Files:** +- Create: `scripts/ui-review/deck/crops.py` +- Test: `scripts/ui-review/tests/test_crops.py` + +**Interfaces:** +- Consumes: Task 1 `run_names`, `AUTO_WARN_FRACTION`; Task 2 `rect_to_pct`, `diff_bbox`, `image_size`, `px_to_pct`. +- Produces: `image_name(crop, theme, run) -> str` (`"----.png"`); `newest_manifest_entry(run_dir, plan, shot, theme) -> dict | None` (manifests ordered by run id, then file time — the same rule as Task 10's `coverage.mjs`); `measure_key(hl) -> str`; `crop_images(spec) -> {'boxes': {id: {theme: {run: [x,y,w,h]}}}, 'missing': [str], 'warnings': [str], 'count': int}` where `count` is the number of crop FILES cut (one per crop × theme × run — steps sharing a crop share the file). Nothing is written besides the crop PNGs; `build` consumes the returned boxes directly. + +- [ ] **Step 1: Write the failing tests** + +```python +# scripts/ui-review/tests/test_crops.py +import json, os, sys, tempfile, unittest +HERE = os.path.dirname(os.path.abspath(__file__)) +sys.path.insert(0, os.path.dirname(HERE)); sys.path.insert(0, HERE) +from fixture import make_fixture +from deck.spec import load_spec +from deck.crops import crop_images, image_name, measure_key, newest_manifest_entry + +class CropTests(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.mkdtemp(); self.spec = load_spec(make_fixture(self.tmp)); self.r = crop_images(self.spec, log=lambda *a: None) + self.images = os.path.join(self.spec['_base'], 'images') + def test_every_theme_and_run_is_cut_once(self): + self.assertEqual(self.r['count'], 1 * 2 * 2) # crops × themes × runs — S-1..3 share crop "c", so 4 files, not 12 + self.assertTrue(os.path.exists(os.path.join(self.images, image_name('c', 'light', 'after')))) + self.assertEqual(sorted(os.listdir(self.images)), sorted(image_name('c', t, r) for t in ('midnight', 'light') for r in ('before', 'after'))) + def test_measured_selector_maps_into_the_crop(self): + # crop is 400x200 at (500,250); #send at (600,300) 80x30 → 25%, 25%, 20%, 15% + self.assertEqual(self.r['boxes']['S-2']['midnight']['before'], [25.0, 25.0, 20.0, 15.0]) + self.assertEqual(self.r['boxes']['S-3']['light']['after'], [25.0, 25.0, 20.0, 15.0]) + def test_auto_box_is_the_changed_region_inside_the_crop_only(self): + b = self.r['boxes']['S-1']['midnight']['after'] + # red block at window (560,260) 120x40 → crop (60,10) 120x40 → 15%,5%,30%,20%; the 3x3 dilate adds 1px + # a side and the pad 6px, so the measured box is (53,3) 134x54 → 13.25%, 1.5%, 33.5%, 27% + self.assertAlmostEqual(b[0], 15.0, delta=2.5); self.assertAlmostEqual(b[1], 5.0, delta=4) + self.assertAlmostEqual(b[2], 30.0, delta=5); self.assertAlmostEqual(b[3], 20.0, delta=8) + self.assertEqual(self.r['boxes']['S-1']['midnight']['before'], b) # same box on both pictures + self.assertEqual(self.r['missing'], []); self.assertEqual(self.r['warnings'], []) + def test_missing_measurement_names_the_fix(self): + self.spec['steps'][1]['highlight'] = {'selector': '#nope'} + r = crop_images(self.spec, log=lambda *a: None) + self.assertTrue(any('"measure": ["#nope"]' in m and 'plans/main.json' in m for m in r['missing'])) + self.assertEqual(r['boxes']['S-2']['light'], {}) + def test_missing_capture_is_reported_not_faked(self): + os.remove(os.path.join(self.spec['runs']['after'], 'shots-main', 'light', 'home.png')) + r = crop_images(self.spec, log=lambda *a: None) + self.assertTrue(any('light/after' in m and 'not captured' in m for m in r['missing'])) + def test_whole_surface_change_warns(self): + import subprocess + p = os.path.join(self.spec['runs']['after'], 'shots-main', 'midnight', 'home.png') + subprocess.run(['magick', p, '-fill', 'red', '-draw', 'rectangle 500,250 899,449', p], check=True) + r = crop_images(self.spec, log=lambda *a: None) + self.assertTrue(any('whole-surface change' in w for w in r['warnings'])) + def test_newest_manifest_entry(self): + e = newest_manifest_entry(self.spec['runs']['before'], 'main', 'home', 'light') + self.assertEqual(e['measures']['#send']['x'], 600); self.assertIsNone(newest_manifest_entry(self.spec['runs']['before'], 'main', 'nope', 'light')) + def test_newest_run_id_beats_a_later_file_time(self): + # An earlier sweep's shard can finish (and write its manifest) AFTER a newer sweep's — the run id decides, not mtime. + import time + d = os.path.join(self.spec['runs']['before'], 'shots-main') + json.dump([{'name': 'home', 'theme': 'light', 'verified': True, 'run': '2', 'measures': {'#send': {'x': 1, 'y': 1, 'w': 1, 'h': 1}}}], open(os.path.join(d, 'manifest-main-newer-run.json'), 'w')) + old = os.path.join(d, 'manifest-main-x.json'); os.utime(old, (time.time() + 60, time.time() + 60)) # the run-'1' file is now the newest on disk + self.assertEqual(newest_manifest_entry(self.spec['runs']['before'], 'main', 'home', 'light')['measures']['#send']['x'], 1) + def test_measure_key(self): + self.assertEqual(measure_key({'selector': '#a'}), '#a'); self.assertEqual(measure_key({'text': 'Send'}), 'text:Send') + +if __name__ == '__main__': unittest.main() +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `python3 -m unittest scripts/ui-review/tests/test_crops.py 2>&1 | tail -2` +Expected: `ModuleNotFoundError: No module named 'deck.crops'` + +- [ ] **Step 3: Write `crops.py`** + +```python +# scripts/ui-review/deck/crops.py +"""Cut the 1:1 crops for every step × theme × run and resolve each step's highlight box. +The spec never carries coordinates: a box comes from the rig's measurement of a named element +(manifest `measures`), or from the pixel difference between the before and after crops.""" +import glob +import json +import os +import subprocess + +from .boxes import diff_bbox, image_size, px_to_pct, rect_to_pct +from .spec import AUTO_WARN_FRACTION, run_names + + +def image_name(crop, theme, run): + return f'{crop}--{theme}--{run}.png' + + +def measure_key(hl): + return hl['selector'] if 'selector' in hl else f'text:{hl["text"]}' + + +def newest_manifest_entry(run_dir, plan, shot, theme): + """The latest manifest entry for (plan, shot, theme) in a run dir. Entries are ordered by + run id first (the sweep's UI_REVIEW_RUN stamp, Task 9), then file time — the same rule as + coverage.mjs — so an earlier sweep's late-finishing shard cannot outrank a newer sweep.""" + found, best = None, (-1, -1.0) + for f in glob.glob(os.path.join(run_dir, f'shots-{plan}', 'manifest-*.json')): + mtime = os.path.getmtime(f) + with open(f) as fh: + for e in json.load(fh): + if e.get('name') == shot and e.get('theme') == theme: + key = (int(e['run']) if str(e.get('run') or '').isdigit() else -1, mtime) + if key >= best: + found, best = e, key + return found + + +def crop_images(spec, log=print): + out_dir = os.path.join(spec['_base'], spec['images']) + os.makedirs(out_dir, exist_ok=True) + runs = run_names(spec) + two = len(runs) == 2 + boxes, missing, warnings, cut = {}, [], [], set() + for st in spec['steps']: + plan, shot, geo = spec['_crops'][st['crop']] + hl = st.get('highlight', 'auto' if two else None) + boxes[st['id']] = {} + for theme in spec['themes']: + per_run = {} + for run in runs: + src = os.path.join(spec['runs'][run], f'shots-{plan}', theme, f'{shot}.png') + dst = os.path.join(out_dir, image_name(st['crop'], theme, run)) + if not os.path.exists(src): + # A missing picture is a capture bug (see coverage.md), never a blank in the deck. + missing.append(f'{st["id"]}: {theme}/{run} — {src} not captured') + continue + if dst not in cut: # steps sharing a crop share the file — cut it once + subprocess.run(['magick', src, '-crop', geo, '+repage', dst], check=True) + cut.add(dst) + if isinstance(hl, dict) and 'box' in hl: + per_run[run] = hl['box'] + elif isinstance(hl, dict): + entry = newest_manifest_entry(spec['runs'][run], plan, shot, theme) + rect = ((entry or {}).get('measures') or {}).get(measure_key(hl)) + if not rect: + want = json.dumps([measure_key(hl) if 'selector' in hl else {'text': hl['text']}]) + missing.append(f'{st["id"]}: no measurement for {measure_key(hl)!r} in {theme}/{run} — add to the ' + f'"{shot}" shot of plans/{plan}.json: "measure": {want} and re-run that plan') + continue + pct = rect_to_pct(rect, geo) + if pct is None: + missing.append(f'{st["id"]}: {measure_key(hl)!r} lies outside crop "{st["crop"]}" in {theme}/{run}') + continue + per_run[run] = pct + paths = [os.path.join(out_dir, image_name(st['crop'], theme, r)) for r in runs] + if hl == 'auto' and all(os.path.exists(p) for p in paths): + box = diff_bbox(paths[0], paths[1]) + if box is None: + missing.append(f'{st["id"]}: nothing differs between before and after in {theme} — name an element instead of "auto"') + else: + size = image_size(paths[0]) + share = box['w'] * box['h'] / (size[0] * size[1]) + if share > AUTO_WARN_FRACTION: + warnings.append(f'{st["id"]}: the change covers {round(share * 100)}% of the crop in {theme} — whole-surface change, name an element instead') + pct = px_to_pct(box, size) + per_run = {r: pct for r in runs} + boxes[st['id']][theme] = per_run + for m in missing: + log('missing: ' + m) + return {'boxes': boxes, 'missing': missing, 'warnings': warnings, 'count': len(cut)} +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `python3 -m unittest scripts/ui-review/tests/test_crops.py -v 2>&1 | tail -3` +Expected: `OK` (9 tests). + +- [ ] **Step 5: Commit** + +```bash +git add scripts/ui-review/deck/crops.py scripts/ui-review/tests/test_crops.py +git commit -m "feat(ui-review): crop step — cut each crop once, resolve highlight boxes from measurements or pixel diff" +``` + +--- + +### Task 6: The page assets and `build.py` + +**Files:** +- Create: `scripts/ui-review/deck/page.html.tmpl`, `scripts/ui-review/deck/page.css`, `scripts/ui-review/deck/page.js`, `scripts/ui-review/deck/build.py` +- Test: `scripts/ui-review/tests/test_build.py` + +**Interfaces:** +- Consumes: Task 1 `validate`, `run_names`, `SpecError`; Task 5 `image_name`; Task 3 `tokens.json`. +- Produces: `build_page(spec, boxes) -> (html: str, warnings: list[str])`; `theme_tokens(themes) -> {theme: {...}}`; `tokens_css(tokens) -> str`; `deck_data(spec, boxes) -> dict` (the `DECK` object the page reads — shape below). The page's HTTP contract used by Task 7: `GET /answers`, `POST /answers`, `POST /submit`. + +`DECK` shape: +```json +{"title": "…", "key": "…", "runs": ["before","after"], "runLabels": {"before":"Before","after":"After","today":"Today"}, + "themes": ["midnight", …], "themeNames": {"midnight":"Midnight", …}, + "steps": [{"id":"S-1","surface":"…","path":"…","headline":"…","changed":"…","measured":"…","notice":"…","risk":"…", + "images": {"midnight": {"before": "images/c--midnight--before.png", "after": "…"}}, + "boxes": {"midnight": {"before": [x,y,w,h], "after": [x,y,w,h]}}}]} +``` + +- [ ] **Step 1: Write the failing tests** + +```python +# scripts/ui-review/tests/test_build.py +import json, os, sys, tempfile, unittest +HERE = os.path.dirname(os.path.abspath(__file__)) +sys.path.insert(0, os.path.dirname(HERE)); sys.path.insert(0, HERE) +from fixture import make_fixture +from deck.spec import load_spec, SpecError +from deck.crops import crop_images +from deck.build import build_page, theme_tokens, tokens_css, deck_data + +class BuildTests(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.mkdtemp(); self.spec = load_spec(make_fixture(self.tmp)); self.boxes = crop_images(self.spec, log=lambda *a: None)['boxes'] + def test_builds_one_self_describing_page(self): + html, warnings = build_page(self.spec, self.boxes) + self.assertIn('Fixture review', html); self.assertIn('const DECK=', html) + self.assertIn('', html) # first paint already wears the first theme + self.assertIn(':root[data-theme="midnight"]{--canvas:#0D1117', html) # tokens inlined, and outranking page.css's :root defaults + self.assertIn('.chip{', html); self.assertIn("fetch('/answers'", html) # css + js inlined + self.assertEqual(warnings, []) + def test_deck_data_shape(self): + d = deck_data(self.spec, self.boxes) + self.assertEqual(d['runs'], ['before', 'after']); self.assertEqual(d['themeNames']['midnight'], 'Midnight') + s = d['steps'][1]; self.assertEqual(s['images']['light']['after'], 'images/c--light--after.png'); self.assertEqual(s['boxes']['light']['after'], [25.0, 25.0, 20.0, 15.0]) + self.assertEqual(s['measured'], ''); self.assertEqual(s['risk'], '') + def test_refuses_when_a_picture_is_missing(self): + os.remove(os.path.join(self.spec['_base'], 'images', 'c--light--after.png')) + with self.assertRaises(SpecError) as cm: build_page(self.spec, self.boxes) + self.assertIn('no picture for light/after', str(cm.exception)) + def test_refuses_when_a_box_is_missing(self): + self.boxes['S-2']['light'] = {} + with self.assertRaises(SpecError) as cm: build_page(self.spec, self.boxes) + self.assertIn('S-2: no highlight box for light', str(cm.exception)) + def test_refuses_on_writing_rule_errors(self): + self.spec['steps'][0]['headline'] = 'We changed the token' + with self.assertRaises(SpecError) as cm: build_page(self.spec, self.boxes) + self.assertIn('banned word "token"', str(cm.exception)) + def test_tokens_for_community_theme_come_from_its_manifest(self): + # No skip: the worktree has no wecoded-themes/ of its own, build.py must find the workspace root's copy. + t = theme_tokens(['midnight', 'halftone-dimension', 'meadow-mist']) + self.assertEqual(t['halftone-dimension']['accent'].lower(), '#e51f48'); self.assertTrue(t['halftone-dimension']['_dark']); self.assertFalse(t['meadow-mist']['_dark']) + self.assertIn('[data-theme="halftone-dimension"]{', tokens_css(t)); self.assertIn('--radius-md:16px', tokens_css(t)) + def test_unknown_theme_is_an_error(self): + with self.assertRaises(SpecError): theme_tokens(['no-such-theme']) + def test_theme_tokens_outrank_the_page_defaults(self): + # page.css sets the deck's default radii under a bare :root in a LATER + +
+
Review deck
+
+
·
+
+ +
+
+
+
+
100%
+

+
+ + + + + +
+
+
+
+
+

Submit your feedback?

+
Skipped steps are sent as "no answer"; Claude leaves those unchanged.
+ +
+
+ + + +``` + +- [ ] **Step 4: Write `page.css`** (mockup G's styles, minus the mockup-only badge; `:root` holds only what the tokens don't) + +```css +/* Review deck v2 — the approved page (docs/active/prototypes/2026-08-27-deck-mockup-g.html). + Theme tokens are inlined by build.py into + +
+
Review deck
+
+
·
+
+ +
+
+
+
+
100%
+

+
+ + + + + +
+
+
+
+
+

Submit your feedback?

+
Skipped steps are sent as "no answer"; Claude leaves those unchanged.
+ +
+
+ + + diff --git a/scripts/ui-review/deck/page.js b/scripts/ui-review/deck/page.js new file mode 100644 index 00000000..fc1cafd0 --- /dev/null +++ b/scripts/ui-review/deck/page.js @@ -0,0 +1,174 @@ +/* Review deck v2 — renders DECK (one JSON object the builder inlines) one step at a time. + Persistence: the serve.py endpoints when the page is served (GET/POST /answers, POST /submit), + localStorage + a copy box when it is opened as a plain file. */ +(function () { + const $ = s => document.querySelector(s), $$ = s => [...document.querySelectorAll(s)]; + const N = DECK.steps.length, runs = DECK.runs; + let cur = 0, theme = DECK.themes[0], zoom = 1, loupeOn = true, server = false, stepStart = Date.now(); + const state = { deck: DECK.key, started: new Date().toISOString(), submitted: null, cur: 0, answers: {} }; + const ICON = { + change: '', + eye: '', + warn: '' }; + const esc = s => String(s ?? '').replace(/[&<>"]/g, c => ({ '&': '&', '<': '<', '>': '>', '"': '"' }[c])); + if (window.top !== window) document.body.classList.add('embedded'); + $('#deck-title').textContent = DECK.title; document.title = DECK.title; + $('#steps').innerHTML = DECK.steps.map(() => '').join(''); + const stage = $('#stage'), inner = $('#inner'), loupe = $('#loupe'); + + // ── persistence ── + const LS = 'deck:' + DECK.key; + async function load() { + try { const r = await fetch('/answers', { cache: 'no-store' }); if (r.ok) { server = true; const j = await r.json(); if (j && j.answers) Object.assign(state, j); return; } } catch (e) { /* not served */ } + try { const j = JSON.parse(localStorage.getItem(LS) || 'null'); if (j && j.answers) Object.assign(state, j); } catch (e) { /* no storage */ } + } + async function save() { + try { localStorage.setItem(LS, JSON.stringify(state)); } catch (e) { /* no storage */ } + if (server) { try { await fetch('/answers', { method: 'POST', headers: { 'content-type': 'application/json' }, body: JSON.stringify(state) }); } catch (e) { /* server gone — the file has everything up to the last successful POST */ } } + } + + // ── render the current step ── + function render() { + const st = DECK.steps[cur]; + document.documentElement.dataset.theme = theme; // before the pictures load, so a theme switch never flashes the old colours + $('#wtitle').textContent = st.surface; $('#wsub').textContent = st.path; + inner.innerHTML = runs.map(r => `
${esc(DECK.runLabels[r] || r)}
`).join(''); + $$('#inner .frame').forEach(f => { const b = (st.boxes[theme] || {})[f.dataset.run]; const box = f.querySelector('.box'); if (b) box.style.cssText = `left:${b[0]}%;top:${b[1]}%;width:${b[2]}%;height:${b[3]}%`; else box.style.display = 'none'; }); + $('#headline').textContent = st.headline; + $('#cards').innerHTML = `

${ICON.change}What changed

${esc(st.changed)}

${st.measured ? `

Measured: ${esc(st.measured)}

` : ''}
` + + `

${ICON.eye}You'll notice

${esc(st.notice)}

` + + (st.risk ? `

${ICON.warn}Risk

${esc(st.risk)}

` : ''); + const last = runs[runs.length - 1]; + $('#thumbs').innerHTML = DECK.themes.map(t => ``).join(''); + $$('.thumb').forEach(b => b.onclick = () => { theme = b.dataset.v; render(); }); + $$('#inner img').forEach(i => i.addEventListener('load', layout)); + layout(); paintState(); + } + function paintState() { + const a = state.answers[DECK.steps[cur].id] || {}; + $$('.ans').forEach(b => b.classList.toggle('on', b.dataset.v === a.v)); + const note = $('#note'); note.value = a.note || ''; note.placeholder = a.v === 'other' ? 'Explain what you’d like instead…' : 'Add a note (optional)'; + $$('#steps span').forEach((s, i) => { const x = state.answers[DECK.steps[i].id]; s.className = (x && x.v ? x.v : '') + (i === cur ? ' on' : ''); }); + const done = Object.values(state.answers).filter(x => x.v && x.v !== 'skip').length; + $('#count').textContent = 'step ' + (cur + 1) + ' of ' + N + ' · ' + done + ' answered' + (state.submitted ? ' · submitted, read-only' : ''); // survives every repaint (theme clicks included) + $('#save').disabled = !(a.v && a.v !== 'skip'); $('#prev').disabled = cur === 0; $('#next').disabled = cur === N - 1; $('#next').textContent = cur === N - 1 ? 'Last step' : 'Next ›'; + } + + // ── layout: try each arrangement for real, keep the one that shows the pictures largest (spec §3.4) ── + const PAD = 28, CAP = 24, GAP = 18; + function layout() { + const c = $('#content'), step = $('#step'); const img = $('#inner img'); if (!img || !img.naturalWidth) return; + const margin = (document.querySelector('main').clientWidth - step.clientWidth) / 2; document.body.classList.toggle('thumbs-inline', margin < 150); + const n = runs.length, w = img.naturalWidth, h = img.naturalHeight + CAP; + const opts = { A: 'row-below', B: 'col-right stacked', C: 'col-right', D: 'row-below stacked' }; const score = {}; + step.classList.remove('compact-step'); + for (const k in opts) { + if (opts[k].includes('col-right') && c.clientWidth < 820) { score[k] = 0; continue; } // a side column needs real width + if (n === 1 && opts[k].includes('stacked')) { score[k] = 0; continue; } // one picture: stacking means nothing + c.className = 'content ' + opts[k]; const SW = stage.clientWidth - PAD, SH = stage.clientHeight - PAD; const stacked = opts[k].includes('stacked'); + score[k] = Math.min(stacked ? SW / w : (SW - GAP * (n - 1)) / n / w, stacked ? (SH - GAP * (n - 1)) / n / h : SH / h); + } + let best = 'A'; for (const k of ['B', 'C', 'D']) if (score[k] > score[best] * 1.05) best = k; // A wins ties + let s; + if (score[best] < 0.5) { c.className = 'content compact'; step.classList.add('compact-step'); s = Math.min((c.clientWidth - PAD) / w, 1); } + else { c.className = 'content ' + opts[best]; s = Math.min(score[best], 1.5); } + $$('#inner img').forEach(i => i.style.width = (i.naturalWidth * s * zoom) + 'px'); + $('#lvl').textContent = Math.round(zoom * 100) + '%'; + // Read by the render test: the choice, and the scores it was made from (so the test checks the RULE, not a table). + document.body.dataset.layout = score[best] < 0.5 ? 'compact' : best; + document.body.dataset.scores = JSON.stringify(score); + const b = $('#inner .frame .box'); if (b && zoom > 1) b.scrollIntoView({ block: 'center', inline: 'center' }); + window.__deckReady = true; // the render test waits for this — set only once a real layout has been chosen + } + + // ── navigation & answers ── + function record() { + const id = DECK.steps[cur].id; const a = state.answers[id] || {}; + if (!a.v) a.v = 'skip'; + a.seconds = (a.seconds || 0) + Math.round((Date.now() - stepStart) / 1000); a.theme = theme; a.zoom = zoom; + state.answers[id] = a; + } + function go(i) { + if (state.submitted) return; // Fix: after Submit, arrow keys / progress segments must not keep POSTing answers + record(); cur = Math.max(0, Math.min(N - 1, i)); state.cur = cur; save(); zoom = 1; stepStart = Date.now(); render(); + } + // Fix: save on EVERY answer, not only when the step changes — a tab closed on the last + // answered step used to lose that answer entirely. The note debounces so a sentence typed + // at speed is one POST, not one per keystroke. + let noteTimer = null; + $$('.ans').forEach(b => b.onclick = () => { const id = DECK.steps[cur].id; state.answers[id] = { ...(state.answers[id] || {}), v: b.dataset.v }; paintState(); save(); $('#note').focus(); }); + $('#note').addEventListener('input', e => { const id = DECK.steps[cur].id; state.answers[id] = { ...(state.answers[id] || {}), note: e.target.value }; clearTimeout(noteTimer); noteTimer = setTimeout(save, 300); }); + $('#save').onclick = () => { if (cur === N - 1) openDialog(); else go(cur + 1); }; + $('#next').onclick = () => go(cur + 1); $('#prev').onclick = () => go(cur - 1); + $$('#steps span').forEach((s, i) => s.onclick = () => go(i)); + + // ── submit ── + function summary() { + const counts = { yes: 0, no: 0, other: 0, skip: 0 }; const lines = []; + for (const st of DECK.steps) { const a = state.answers[st.id] || { v: 'skip' }; const v = a.v || 'skip'; counts[v] = (counts[v] || 0) + 1; lines.push(st.id + ' ' + v + (a.note && a.note.trim() ? ' — "' + a.note.trim() + '"' : '')); } + return DECK.key + ' · ' + (state.submitted ? 'submitted ' + state.submitted.slice(0, 16).replace('T', ' ') : 'not submitted') + ' · ' + counts.yes + ' yes · ' + counts.no + ' no · ' + counts.other + ' other · ' + counts.skip + ' skipped\n' + lines.join('\n'); + } + function openDialog() { + record(); save(); + // Fix: record() already banked elapsed seconds into the current step's answer; without resetting + // stepStart, a Done -> Keep reviewing -> Done round trip would add those seconds a second time. + // paintState() repaints so a step that record() just marked "skip" turns grey immediately. + stepStart = Date.now(); paintState(); + const missing = DECK.steps.map((st, i) => [(state.answers[st.id] || {}).v, i]).filter(([v]) => !v || v === 'skip').map(([, i]) => i + 1); + $('#skipped').style.display = missing.length ? 'flex' : 'none'; + $('#skipn').textContent = missing.length + (missing.length === 1 ? ' step has' : ' steps have') + ' no answer (step' + (missing.length > 1 ? 's ' : ' ') + missing.join(', ') + ').'; + $('#first').style.display = missing.length ? 'inline-flex' : 'none'; $('#first').onclick = () => { $('#veil').classList.remove('on'); go(missing[0] - 1); }; + $('#dlg-text').innerHTML = server + ? 'Your answers have been saving to a file next to this deck as you went. Submitting tells Claude you\'re finished — it picks them up in the session and replies there. Nothing to copy or paste: close this tab and go back to the conversation.' + : 'This deck is not being served, so Claude is not watching it. Copy the feedback below and paste it into the chat.'; + $('#feedback').style.display = server ? 'none' : 'block'; $('#copy').style.display = server ? 'none' : 'inline-flex'; $('#submit').style.display = server ? 'inline-flex' : 'none'; + $('#feedback').value = summary(); $('#veil').classList.add('on'); + } + $('#done').onclick = openDialog; $('#cancel').onclick = () => $('#veil').classList.remove('on'); + $('#submit').onclick = async () => { + // Fix: only a POST the server actually accepted may say "Submitted ✓". Claiming success + // when the server is gone sends Destin back to the conversation with nothing waiting there. + let ok = false, why = ''; + try { const r = await fetch('/submit', { method: 'POST', headers: { 'content-type': 'application/json' }, body: JSON.stringify(state) }); ok = r.ok; if (!ok) why = 'HTTP ' + r.status; } + catch (e) { why = (e && e.message) || String(e); server = false; } // the server is gone: a re-opened dialog must take the copy/paste branch, not repeat "saving as you went" + if (!ok) { + state.submitted = null; // summary() then labels it "not submitted", which is the truth + $('#dlg-text').textContent = 'The deck could not reach its server (' + why + '). Your answers up to the last save are in the answers file; copy the feedback below and paste it into the chat.'; + $('#feedback').value = summary(); $('#feedback').style.display = 'block'; $('#copy').style.display = 'inline-flex'; $('#submit').style.display = 'none'; + return; // veil stays open — there is still something for him to do here + } + state.submitted = new Date().toISOString(); + $('#veil').classList.remove('on'); lockSubmitted(); + }; + // A submitted deck is read-only, and says so — silently ignoring clicks read as "I can't click through the pages". + function lockSubmitted() { $('#done').textContent = 'Submitted ✓'; $('#done').disabled = true; $$('.ans,#save,#note').forEach(e => e.disabled = true); paintState(); } + $('#copy').onclick = () => { const t = $('#feedback'); t.select(); (navigator.clipboard ? navigator.clipboard.writeText(t.value) : Promise.reject()).catch(() => document.execCommand('copy')); $('#copy').textContent = 'Copied'; }; + + // ── loupe, zoom, keys ── + const K = 2.5, R = 90; + stage.addEventListener('mousemove', e => { + if (!loupeOn) { loupe.style.display = 'none'; return; } + const img = $$('#inner img').find(i => { const r = i.getBoundingClientRect(); return e.clientX >= r.left && e.clientX <= r.right && e.clientY >= r.top && e.clientY <= r.bottom; }); + if (!img) { loupe.style.display = 'none'; return; } + const r = img.getBoundingClientRect(); const x = e.clientX - r.left, y = e.clientY - r.top; + loupe.style.display = 'block'; loupe.style.left = (e.clientX - R) + 'px'; loupe.style.top = (e.clientY - R) + 'px'; + loupe.style.backgroundImage = 'url("' + img.src + '")'; loupe.style.backgroundSize = (r.width * K) + 'px ' + (r.height * K) + 'px'; loupe.style.backgroundPosition = (-x * K + R) + 'px ' + (-y * K + R) + 'px'; + }); + stage.addEventListener('mouseleave', () => loupe.style.display = 'none'); + function setZoom(z) { zoom = Math.max(1, Math.min(4, Math.round(z * 10) / 10)); layout(); } + $('#zin').onclick = () => setZoom(zoom + 0.1); $('#zout').onclick = () => setZoom(zoom - 0.1); + document.addEventListener('keydown', e => { + if (e.target.tagName === 'INPUT' || e.target.tagName === 'TEXTAREA') return; + if (!state.submitted) { if (e.key === 'ArrowRight') go(cur + 1); if (e.key === 'ArrowLeft') go(cur - 1); } // Fix: zoom/loupe stay live after Submit, navigation doesn't + if (e.key === '+' || e.key === '=') setZoom(zoom + 0.1); if (e.key === '-') setZoom(zoom - 0.1); if (e.key === '0') setZoom(1); + if (e.key === 'l') { loupeOn = !loupeOn; if (!loupeOn) loupe.style.display = 'none'; document.body.classList.toggle('no-loupe', !loupeOn); } + }); + window.addEventListener('resize', layout); + load().then(() => { + const q = new URLSearchParams(location.search); + cur = q.get('step') ? Math.max(0, Math.min(N - 1, +q.get('step') - 1)) : Math.max(0, Math.min(N - 1, state.cur || 0)); + if (q.get('theme') && DECK.themes.includes(q.get('theme'))) theme = q.get('theme'); + stepStart = Date.now(); render(); + if (state.submitted) lockSubmitted(); // an archived (file://) deck opened after its submit shows its locked state instead of dead buttons + }); +})(); diff --git a/scripts/ui-review/deck/serve.py b/scripts/ui-review/deck/serve.py new file mode 100644 index 00000000..655b8d29 --- /dev/null +++ b/scripts/ui-review/deck/serve.py @@ -0,0 +1,245 @@ +"""Serve a built deck on 127.0.0.1, open it in the browser, save every answer to +.answers.json as it arrives, and exit when Destin submits. + +WHY exit-on-submit: Claude runs `serve` as a background command and is re-invoked when it +exits — that exit IS the notification that the review is done, with the summary on stdout. +No copy, no paste, no "I'm done" message (spec §4.3).""" +import http.server +import json +import os +import signal +import socketserver +import subprocess +import sys +import threading +import time +import webbrowser + + +def answers_path(spec): + return os.path.join(spec['_base'], spec['_stem'] + '.answers.json') + + +def lock_path(spec): + return os.path.join(spec['_base'], spec['_stem'] + '.serve.json') + + +def write_atomic(path, obj): + tmp = path + '.tmp' + with open(tmp, 'w') as f: + json.dump(obj, f, indent=1) + os.replace(tmp, path) + + +def summary(spec, state): + """One line per step, ledger id first, in spec order (spec §4.5).""" + counts = {'yes': 0, 'no': 0, 'other': 0, 'skip': 0} + lines = [] + for st in spec['steps']: + a = (state.get('answers') or {}).get(st['id']) or {} + v = a.get('v') or 'skip' + counts[v] = counts.get(v, 0) + 1 + note = (a.get('note') or '').strip() + lines.append(f'{st["id"]} {v}' + (f' — "{note}"' if note else '')) + when = (state.get('submitted') or '')[:16].replace('T', ' ') + head = (f'{spec["key"]} · {"submitted " + when if when else "not submitted"} · ' + f'{counts["yes"]} yes · {counts["no"]} no · {counts["other"]} other · {counts["skip"]} skipped') + return head + '\n' + '\n'.join(lines) + + +class _Server(socketserver.ThreadingMixIn, http.server.HTTPServer): + daemon_threads = True + allow_reuse_address = True + + +def make_server(spec, port, on_submit): + apath = answers_path(spec) + + class Handler(http.server.SimpleHTTPRequestHandler): + def __init__(self, *a, **k): + super().__init__(*a, directory=spec['_base'], **k) + + def log_message(self, *a): # quiet; the CLI prints what matters + pass + + def _json(self, code, obj): + body = json.dumps(obj).encode() + self.send_response(code) + self.send_header('content-type', 'application/json') + self.send_header('content-length', str(len(body))) + self.end_headers() + self.wfile.write(body) + + def _wrong_origin(self): + """WHY: this server answers on the loopback interface with no authentication. A page + on any other origin could otherwise forge a Submit with a form POST (which needs no + preflight), and a DNS-rebinding name pointed at 127.0.0.1 could read the deck folder. + Pinning both Host and Origin to our own address closes both.""" + port = self.server.server_address[1] + # localhost is not rebindable in any current browser, so a hand-typed localhost URL may work too. + mine = {f'127.0.0.1:{port}', f'localhost:{port}', f'[::1]:{port}'} + origin = self.headers.get('origin') + return (self.headers.get('host') or '') not in mine or (origin is not None and origin not in {f'http://{m}' for m in mine}) + + def do_GET(self): + if self._wrong_origin(): + return self._json(403, {'error': 'wrong host or origin'}) + if self.path.split('?')[0] == '/answers': + if os.path.exists(apath): + with open(apath) as f: + return self._json(200, json.load(f)) + return self._json(200, {}) + return super().do_GET() + + def do_POST(self): + if self._wrong_origin(): + return self._json(403, {'error': 'wrong host or origin'}) + n = int(self.headers.get('content-length') or 0) + try: + state = json.loads(self.rfile.read(n) or b'{}') + except (ValueError, TypeError): + # WHY: a non-JSON body must get a reply, not a dropped connection — + # json.loads raising unhandled here leaves the client hanging. + return self._json(400, {'error': 'body is not JSON'}) + if self.path == '/answers': + write_atomic(apath, state) + return self._json(200, {'ok': True}) + if self.path == '/submit': + state['submitted'] = state.get('submitted') or time.strftime('%Y-%m-%dT%H:%M:%SZ', time.gmtime()) + write_atomic(apath, state) + self._json(200, {'ok': True}) + on_submit(state) + return + return self._json(404, {'error': 'unknown path'}) + + srv = _Server(('127.0.0.1', port), Handler) + return srv, f'http://127.0.0.1:{srv.server_address[1]}/{spec["out"]}' + + +def open_url(url): + for cmd in (['xdg-open', url], ['open', url]): + try: + subprocess.Popen(cmd, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) + return True + except FileNotFoundError: + continue + return webbrowser.open(url) + + +def already_served(spec): + """{'pid', 'url'} of the live process holding this spec's lock, or None (no lock, stale + lock, or an unreadable one). Checked BEFORE anything is built: a second `serve` used to + rebuild the HTML and the crops out from under the first server and only then exit 3.""" + lock = lock_path(spec) + if not os.path.exists(lock): + return None + try: + with open(lock) as f: + other = json.load(f) + pid, other_url = other['pid'], other['url'] + except (OSError, ValueError, KeyError): + return None # unreadable/malformed lock file — treat as stale, proceed + try: + # WHY: kill(pid, 0) sends nothing; ProcessLookupError means dead, + # PermissionError means alive but not ours — both are OSError, so + # they must be told apart. + os.kill(pid, 0) + except ProcessLookupError: + return None # stale lock — the pid is dead + except PermissionError: + return {'pid': pid, 'url': other_url} # alive, owned by someone else + return {'pid': pid, 'url': other_url} + + +def rotate_submitted(spec, log=print): + """If the answers file already carries `submitted`, move it aside and return its new name. + WHY: on 2026-08-27 a deck was re-served after Destin had submitted an earlier version of it; + the page loaded the old file, saw `submitted`, and locked every control — "I can't click + through the pages". A new `serve` is a new review: the old answers stay as history next to + the spec (.answers..json), the new review starts empty.""" + apath = answers_path(spec) + try: + with open(apath) as f: + state = json.load(f) + except (OSError, ValueError): + return None + when = state.get('submitted') + if not when: + return None + stamp = ''.join(c for c in when[:16] if c.isdigit()) or 'submitted' + dest = os.path.join(spec['_base'], f'{spec["_stem"]}.answers.{stamp}.json') + os.replace(apath, dest) + log(f'[deck] the previous review of this deck was submitted {when[:16].replace("T", " ")} — kept as {os.path.basename(dest)}; starting a fresh one') + return dest + + +def serve(spec, port=0, open_browser=True, timeout_min=240, log=print): + """Blocks. Returns 0 after a submit (summary logged), 2 on timeout, 3 if this spec is already served.""" + lock = lock_path(spec) + other = already_served(spec) + if other is not None: + log(f'REFUSING: {spec["_stem"]} is already served by pid {other["pid"]} at {other["url"]}') + return 3 + rotate_submitted(spec, log) + result = {} + holder = {} + + def on_submit(state): + result['state'] = state + # WHY: shutdown() blocks until serve_forever() returns, so calling it on the + # thread that runs serve_forever (the handler thread is one of its children in + # ThreadingMixIn) would deadlock — it must run on a throwaway thread. + threading.Thread(target=holder['srv'].shutdown, daemon=True).start() + srv, url = make_server(spec, port, on_submit) + holder['srv'] = srv + with open(lock, 'w') as f: + json.dump({'pid': os.getpid(), 'url': url}, f) + log(f'[deck] {url}') + if open_browser: + open_url(url) + # WHY: shutdown() blocks until serve_forever() returns, so calling it on the thread + # that runs serve_forever (the handler thread is one of its children in + # ThreadingMixIn) would deadlock — it must run on a throwaway thread. + timer = threading.Timer(timeout_min * 60, lambda: threading.Thread(target=srv.shutdown, daemon=True).start()) + timer.daemon = True + timer.start() + # A plain `kill` (SIGTERM) would end the process without running the finally below and leave + # the lock file behind; turning it into SystemExit lets the cleanup run. + if threading.current_thread() is threading.main_thread(): # signal handlers can only be set there (tests run serve() in a thread) + signal.signal(signal.SIGTERM, lambda *a: sys.exit(143)) + try: + srv.serve_forever() + finally: + timer.cancel() + srv.server_close() + try: + os.remove(lock) + except OSError: + pass + if 'state' in result: + log(summary(spec, result['state'])) + return 0 + log(f'[deck] no submit after {timeout_min} min — answers so far are in {answers_path(spec)}') + return 2 + + +def wait_for_submit(spec, timeout_min=240, poll_s=2, log=print): + """Block until the answers file carries `submitted`; 0 with the summary logged, 2 on timeout. + WHY a second way to wait: `serve` runs as a background command and its exit is the signal — + but a session that was compacted, restarted or lost that process still needs to know when + Destin is done. This reads only the file, so it works whether or not `serve` is alive.""" + deadline = time.monotonic() + timeout_min * 60 + apath = answers_path(spec) + while True: + try: + with open(apath) as f: + state = json.load(f) + if state.get('submitted'): + log(summary(spec, state)) + return 0 + except (OSError, ValueError): + pass # not written yet, or mid-write (write_atomic renames, so this is rare) + if time.monotonic() >= deadline: + log(f'[deck] no submit after {timeout_min} min — answers so far are in {apath}') + return 2 + time.sleep(poll_s) diff --git a/scripts/ui-review/deck/spec.py b/scripts/ui-review/deck/spec.py new file mode 100644 index 00000000..a7aa0fba --- /dev/null +++ b/scripts/ui-review/deck/spec.py @@ -0,0 +1,122 @@ +"""Deck spec: loading, crop-registry merge, and the writing rules the builder enforces. + +WHY rules in code: on 2026-08-25 a taste argument went into a review as if it were a defect, +and prose reviews were rejected three times for being unreadable — so the deck's vocabulary +(headline · What changed · You'll notice · Risk) and its word limits are checked here, not +remembered. Spec: docs/archive/specs/2026-08-27-review-deck-v2-design.md §4–5.""" +import json +import os +import re + +HERE = os.path.dirname(os.path.abspath(__file__)) +UI_REVIEW = os.path.dirname(HERE) +DEFAULT_THEMES = ['midnight', 'light', 'creme', 'dark', 'halftone-dimension', 'meadow-mist'] +# Whole-word, case-insensitive. "px" and numbers are fine — measurements are wanted. +BANNED = ['token', 'primitive', 'selector', 'ipc', 'prop', 'props', 'reducer', 'handler', + 'component', 'tailwind', 'css class', 'react', 'dom', 'z-index'] +TEXT_FIELDS = ['headline', 'changed', 'measured', 'notice', 'risk', 'surface', 'path'] +HEADLINE_MAX = 25 +RISK_WARN = 40 +AUTO_WARN_FRACTION = 0.6 # an auto-highlight covering more than this much of the crop is "whole surface" + + +class SpecError(Exception): + pass + + +def workspace_root(): + """The directory that holds the sub-repo checkouts (youcoded/, wecoded-themes/). + WHY walk up: this package usually runs from a worktree (worktrees//scripts/…), and a + worktree holds only the workspace repo — the sub-repos are cloned once, at the root above + worktrees/. Resolving relative to the package silently found nothing on 2026-08-27.""" + if os.environ.get('YOUCODED_WORKSPACE'): + return os.environ['YOUCODED_WORKSPACE'] + d = HERE + while True: + if os.path.isdir(os.path.join(d, 'wecoded-themes', 'themes')): + return d + parent = os.path.dirname(d) + if parent == d: + raise SpecError('no workspace root above ' + HERE + ' holds wecoded-themes/themes (set YOUCODED_WORKSPACE)') + d = parent + + +def load_spec(path): + with open(path) as f: + spec = json.load(f) + for k in ('title', 'key', 'out', 'images', 'runs', 'steps'): + if k not in spec or spec[k] is None: + raise SpecError(f'spec is missing "{k}"') + if not 1 <= len(spec['runs']) <= 2: + raise SpecError('runs must have one entry (today) or two (before, after)') + spec['_base'] = os.path.dirname(os.path.abspath(path)) + spec['_stem'] = os.path.splitext(os.path.basename(path))[0] + with open(os.path.join(UI_REVIEW, 'crops.json')) as f: + shared = json.load(f) + shared.pop('_comment', None) + spec['_crops'] = {**shared, **spec.get('crops', {})} + spec.setdefault('themes', list(DEFAULT_THEMES)) + return spec + + +def run_names(spec): + """Display order of the runs: before then after when both exist, else as written.""" + r = list(spec['runs'].keys()) + return ['before', 'after'] if set(r) == {'before', 'after'} else r + + +def word_count(s): + # A curly apostrophe (’, pasted from a word processor) must stay inside its word, or "it’s" counts as two. + return len(re.findall(r"[\w'’-]+", s or '')) + + +def banned_in(text): + low = (text or '').lower() + return [w for w in BANNED if re.search(r'(? HEADLINE_MAX: + errors.append(f'{sid}: headline is {word_count(st["headline"])} words (max {HEADLINE_MAX})') + for k in TEXT_FIELDS: + for w in banned_in(st.get(k)): + errors.append(f'{sid}: {k} uses banned word "{w}"') + hl = st.get('highlight', 'auto' if two_runs else None) + if hl is None: + errors.append(f'{sid}: a one-run deck needs a highlight (selector or text)') + elif hl == 'auto': + if not two_runs: + errors.append(f'{sid}: "auto" highlight needs a before and an after run') + elif isinstance(hl, dict): + if not any(k in hl for k in ('selector', 'text', 'box')): + errors.append(f'{sid}: highlight must be "auto" or have selector, text or box') + elif 'box' in hl: + warnings.append(f'{sid}: hand-placed box — prefer a selector so the rig measures it') + else: + errors.append(f'{sid}: highlight must be "auto" or an object') + if word_count(st.get('risk')) > RISK_WARN: + warnings.append(f'{sid}: risk is {word_count(st["risk"])} words — keep it to one sentence') + if st.get('measured') and not re.search(r'\d', st['measured']): + warnings.append(f'{sid}: measured has no number in it') + # WHY: the crops are named after the step, not the deck, so two specs pointed at one images + # folder silently overwrite each other's pictures — the second build leaves the first deck + # showing the second deck's screenshots, with no error anywhere. + if spec['_stem'] not in spec['images']: + warnings.append(f'images "{spec["images"]}" does not contain the spec name "{spec["_stem"]}" ' + '— two decks sharing one images folder overwrite each other\'s pictures') + return errors, warnings diff --git a/scripts/ui-review/deck/tokens.json b/scripts/ui-review/deck/tokens.json new file mode 100644 index 00000000..1b236fa0 --- /dev/null +++ b/scripts/ui-review/deck/tokens.json @@ -0,0 +1,6 @@ +{ + "light": {"canvas": "#F2F2F2", "panel": "#EAEAEA", "inset": "#D7D7D7", "well": "#F9F9F9", "accent": "#1A1A1A", "on-accent": "#F2F2F2", "fg": "#1A1A1A", "fg-2": "#444444", "fg-dim": "#656565", "fg-muted": "#797979", "fg-faint": "#989898", "edge": "#C0C0C0", "link": "#2055CA", "_dark": false}, + "dark": {"canvas": "#111111", "panel": "#191919", "inset": "#222222", "well": "#1C1C1C", "accent": "#D4D4D4", "on-accent": "#111111", "fg": "#E0E0E0", "fg-2": "#B0B0B0", "fg-dim": "#999999", "fg-muted": "#6C6C6C", "fg-faint": "#515151", "edge": "#393939", "link": "#66AAFF", "_dark": true}, + "midnight": {"canvas": "#0D1117", "panel": "#161B22", "inset": "#21262D", "well": "#0D1117", "accent": "#B1BAC4", "on-accent": "#0D1117", "fg": "#C9D1D9", "fg-2": "#A0AAB4", "fg-dim": "#8B949E", "fg-muted": "#6E7681", "fg-faint": "#4E555E", "edge": "#343A41", "link": "#58A6FF", "_dark": true}, + "creme": {"canvas": "#F6EEE1", "panel": "#EBE1D1", "inset": "#D8CCB9", "well": "#F9F0E2", "accent": "#3D3229", "on-accent": "#F6EEE1", "fg": "#2C2418", "fg-2": "#564938", "fg-dim": "#695E4D", "fg-muted": "#7D7161", "fg-faint": "#9A8F7F", "edge": "#C4B8A6", "link": "#5B4A1E", "_dark": false} +} diff --git a/scripts/ui-review/probe-ports.sh b/scripts/ui-review/probe-ports.sh new file mode 100644 index 00000000..f70b8705 --- /dev/null +++ b/scripts/ui-review/probe-ports.sh @@ -0,0 +1,18 @@ +#!/bin/bash +# probe-ports.sh [ ...] — exit 1 naming every port that already has a listener. +# WHY: two review sweeps at offsets 300 and 310 overlapped their CDP port ranges and +# deadlocked for 20 minutes with no error (2026-08-27, hand-off gap 1). Refusing loudly is +# the fix; `ss` is the reliable local truth, bash's /dev/tcp is the fallback where ss is absent. +busy=() +for p in "$@"; do + if command -v ss >/dev/null 2>&1; then + [[ -n "$(ss -ltnH "sport = :$p" 2>/dev/null)" ]] && busy+=("$p") + elif (exec 3<>"/dev/tcp/127.0.0.1/$p") 2>/dev/null; then + busy+=("$p") + fi +done +if [[ ${#busy[@]} -gt 0 ]]; then + echo "REFUSING: ports already in use: ${busy[*]} — another sweep is running; use YOUCODED_PORT_OFFSET at least 100 away" >&2 + exit 1 +fi +exit 0 diff --git a/scripts/ui-review/review-cards.py b/scripts/ui-review/review-cards.py index e8071bbe..9da254af 100644 --- a/scripts/ui-review/review-cards.py +++ b/scripts/ui-review/review-cards.py @@ -1,242 +1,79 @@ #!/usr/bin/env python3 -"""Build a one-point-at-a-time UI review page (a "deck"). Each step shows ONE screenshot -with ONE marker, one line of problem, one line of fix, and three buttons: Yes / No / -Tell me more. Nothing else competes. Keyboard: Y / N / M decide (and advance), ← → move, T cycles themes, space -flips Before/After when the step has both. Progress dots at the bottom jump; the last -step is the summary with the copyable feedback block. - -History: 2026-08-25 a gallery of full-window sheets was rejected ("no quick way to give -feedback, nothing explained"); 2026-08-26 a prose-first page ("WAY too much text in -different areas") and then a board of cards ("still WAY too much going on visually… not -clear where I'm supposed to glance/select") were rejected. The deck is the answer to -"make the hierarchy immediately intuitive": look → read one line → click. - - python3 scripts/ui-review/review-cards.py crop cut the crops (needs magick) - python3 scripts/ui-review/review-cards.py build write the HTML next to the spec - -Spec (JSON, paths relative to the spec's directory unless absolute): - out, images, key, runs, labels, crops, title, lead — see phase-c-cards.json - items[] {id, title, surface?, - image: {crop, themes, cols, zoom?}, # the item's default picture - points: [{n, kind: measured|judgment, what, fix, - at: [x%,y%] | {theme:[x%,y%]}, # ONE marker, percent of the image - why?: html, risk?: text, - image?: {crop, themes, cols, zoom?}}] # per-point picture override - more: [{crop, themes, cols, zoom?, points:[n]}]} # picture for those points - closing optional HTML on the summary step -""" -import html -import json +"""Review deck v2 — the page Destin approves UI changes on, one point per step. + + python3 scripts/ui-review/review-cards.py build cut the crops, resolve every highlight box, write the HTML next to the spec + python3 scripts/ui-review/review-cards.py serve [--no-open] [--no-build] [--port N] [--timeout MIN] + build it, serve it, open the browser, save answers to .answers.json, exit when Destin submits + python3 scripts/ui-review/review-cards.py wait [--timeout MIN] + block until the answers file says submitted (for a session that no longer holds the `serve` process) + +Run `serve` in the background: its exit is the "review finished" signal and it prints the +feedback summary. There is deliberately no separate crop step — a stale intermediate file drew +wrong rings with no error in v1. Spec format + writing rules: +docs/archive/specs/2026-08-27-review-deck-v2-design.md (§4–5). History of the +three rejected formats before this one: docs/active/handoffs/2026-08-27-review-deck-tooling-handoff.md.""" +import argparse import os -import subprocess import sys -HERE = os.path.dirname(os.path.abspath(__file__)) -NICE = {'midnight': 'Midnight', 'dark': 'Dark', 'light': 'Light', 'creme': 'Crème', - 'halftone-dimension': 'Halftone', 'meadow-mist': 'Meadow'} -DEFAULT_LABELS = {'today': 'Today', 'before': 'Before', 'after': 'After', 'A': 'After'} - - -def load(path): - spec = json.load(open(path)) - spec['_base'] = os.path.dirname(os.path.abspath(path)) - shared = json.load(open(os.path.join(HERE, 'crops.json'))) - shared.pop('_comment', None) - spec['_crops'] = {**shared, **spec.get('crops', {})} - spec['_labels'] = {**DEFAULT_LABELS, **spec.get('labels', {})} - return spec - - -def point_image(it, pt): - """The picture a point is shown on: its own, else the `more` figure that claims it, else the item's.""" - if pt.get('image'): - return pt['image'] - for m in it.get('more', []): - if pt['n'] in m.get('points', []): - return m - return it['image'] - - -def all_images(spec): - seen = set() - for it in spec['items']: - for img in [it['image'], *it.get('more', []), *[p['image'] for p in it['points'] if p.get('image')]]: - k = (img['crop'], tuple(img['themes']), tuple(img['cols'])) - if k not in seen: - seen.add(k) - yield img - - -def crop(spec): - out = os.path.join(spec['_base'], spec['images']) - os.makedirs(out, exist_ok=True) - n = 0 - for img in all_images(spec): - plan, shot, geo = spec['_crops'][img['crop']] - for t in img['themes']: - for c in img['cols']: - run = spec['runs'].get(c) - src = os.path.join(run, f'shots-{plan}', t, f'{shot}.png') if run else None - if src and os.path.exists(src): - subprocess.run(['magick', src, '-crop', geo, '+repage', - os.path.join(out, f'{img["crop"]}--{t}--{c}.png')], check=True) - n += 1 - else: - print(f' not captured: {img["crop"]} {t} {c} ({src})', file=sys.stderr) - print(f'{n} crops → {out}') - - -def marker(pt, theme): - at = pt.get('at') - if isinstance(at, dict): - at = at.get(theme) - if not at: - return '' - return f'' - - -def step_html(spec, it, pt, idx, total): - img = point_image(it, pt) - themes, cols = img['themes'], img['cols'] - labels = spec['_labels'] - frames = [] - for t in themes: - for c in cols: - f = f'{spec["images"]}/{img["crop"]}--{t}--{c}.png' - exists = os.path.exists(os.path.join(spec['_base'], f)) - on = t == themes[0] and c == cols[-1] - zoom = f' style="zoom:{img["zoom"]}"' if img.get('zoom') else '' - body = f'' if exists else '
not captured
' - frames.append(f'
{body}{marker(pt, t)}
') - tabs = ''.join(f'' - for i, t in enumerate(themes)) if len(themes) > 1 else '' - coltabs = ''.join(f'' - for i, c in enumerate(cols)) if len(cols) > 1 else '' - why = f'
Why / details{pt["why"]}
' if pt.get('why') else '' - risk = f'

Risk: {html.escape(pt["risk"])}

' if pt.get('risk') else '' - key = f'{it["id"]}:{pt["n"]}' - surface = f' · {html.escape(it["surface"])}' if it.get('surface') else '' - btns = {'yes': 'Yes, build it', 'no': 'No, leave it', 'more': 'Tell me more', **spec.get('buttons', {})} - btn_yes, btn_no, btn_more = (html.escape(btns[k]) for k in ('yes', 'no', 'more')) - return f'''
-
{it["id"]} {it["title"]}{surface}{idx + 1} / {total}
-
{"".join(frames)}
-
{tabs}{coltabs}
-

{pt["what"]}

-

→ {pt["fix"]}

-
{pt["kind"]}{risk}{why}
-
-
-
''' - - -CSS = ''' - :root { --ink:#1a1a1a; --ink2:#666; --line:#dedede; --bg:#f4f4f2; --acc:#2055ca; - --meas:#0a6a2c; --measbg:#e3f3e8; --judg:#8a5a00; --judgbg:#fff0d6; --yes:#137a3a; --no:#b3261e; --more:#8a5a00 } - * { box-sizing:border-box } - html, body { height:100% } body { margin:0; font:15px/1.4 system-ui,-apple-system,Segoe UI,Roboto,sans-serif; color:var(--ink); background:var(--bg); display:flex; flex-direction:column } - main { flex:1; display:flex; flex-direction:column; align-items:center; padding:18px 24px 8px; min-height:0 } - .step { display:none; width:min(1060px, 100%); flex-direction:column; min-height:0 } .step.on { display:flex } - .kicker { color:var(--ink2); font-size:13px; margin-bottom:8px; display:flex; gap:6px; align-items:baseline } .pid { font-weight:700; color:var(--acc) } .count { margin-left:auto; font-variant-numeric:tabular-nums } - .stagewrap { background:#e6e6e3; border-radius:10px; padding:10px } - .stage { position:relative; text-align:center; line-height:0; font-size:0; max-height:56vh; overflow:auto } - .frame { display:none; position:relative; line-height:0 } .frame.on { display:inline-block; max-width:100% } .frame img { display:block; max-width:100%; height:auto } - .missing { font:14px system-ui; color:#666; padding:40px } - .mk { position:absolute; transform:translate(-50%,-50%); width:46px; height:46px; border-radius:50%; border:3px solid #fff; box-shadow:0 0 0 3px var(--acc), 0 4px 14px rgba(0,0,0,.6); background:transparent; animation:pulse 1.6s ease-in-out infinite; pointer-events:none } - @keyframes pulse { 0%,100% { box-shadow:0 0 0 3px var(--acc), 0 4px 14px rgba(0,0,0,.6) } 50% { box-shadow:0 0 0 7px rgba(32,85,202,.55), 0 4px 14px rgba(0,0,0,.6) } } - .figbar { display:flex; justify-content:space-between; margin-top:8px; min-height:22px } .tab, .ctab { font:12px system-ui; padding:2px 9px; border:1px solid #bbb; background:#fff; border-radius:999px; margin-right:4px; cursor:pointer; color:#444 } - .tab.on { background:#333; color:#fff; border-color:#333 } .ctab.on { background:var(--acc); color:#fff; border-color:var(--acc) } - .what { font-size:21px; font-weight:650; line-height:1.3; margin:18px 0 6px } .fix { font-size:17px; color:#1f3a78; margin:0 0 8px } - .meta { display:flex; gap:14px; align-items:baseline; flex-wrap:wrap; font-size:13px; color:var(--ink2); min-height:22px } - .kind { font-size:11px; font-weight:700; text-transform:uppercase; letter-spacing:.06em; padding:2px 8px; border-radius:8px } .kind-measured { background:var(--measbg); color:var(--meas) } .kind-judgment { background:var(--judgbg); color:var(--judg) } - .risk { margin:0; color:#7a3b00 } .why summary { cursor:pointer; color:var(--acc) } .why { max-width:900px } .why p, .why ul { margin:6px 0 0 } - .decide { display:flex; gap:12px; align-items:center; margin-top:14px; flex-wrap:wrap } - .decide button { font:600 17px system-ui; padding:12px 26px; border-radius:10px; border:2px solid transparent; cursor:pointer; color:#fff } .decide kbd { font:11px ui-monospace,monospace; background:rgba(255,255,255,.25); padding:1px 5px; border-radius:4px; margin-left:8px } - .yes { background:var(--yes) } .no { background:var(--no) } .more { background:var(--more) } .decide button.on { border-color:#111; box-shadow:0 0 0 3px #fff, 0 0 0 5px #111 } .step.decided .decide button:not(.on) { opacity:.35 } - .note { flex:1; min-width:200px; font:14px system-ui; padding:10px 12px; border:1px solid var(--line); border-radius:8px } - nav { display:flex; align-items:center; gap:10px; padding:10px 24px 16px; justify-content:center; flex-wrap:wrap } - nav .arrow { font:14px system-ui; padding:6px 12px; border:1px solid #bbb; background:#fff; border-radius:8px; cursor:pointer } nav .arrow kbd { font:11px ui-monospace,monospace; color:#888; margin:0 3px } - .dots { display:flex; gap:5px; flex-wrap:wrap; justify-content:center } .dot { width:14px; height:14px; border-radius:50%; background:#cfcfcf; border:2px solid transparent; cursor:pointer } .dot.yes { background:var(--yes) } .dot.no { background:var(--no) } .dot.more { background:var(--more) } .dot.on { border-color:#111 } .dot.sum { border-radius:3px } - .summary { display:none; width:min(1060px,100%) } .summary.on { display:block } .summary h1 { font-size:22px; margin:0 0 8px } .summary table { border-collapse:collapse; width:100%; margin:10px 0 } .summary td, .summary th { border-bottom:1px solid var(--line); padding:6px 8px; text-align:left; vertical-align:top; font-size:14px } .summary td.v-yes { color:var(--yes); font-weight:700 } .summary td.v-no { color:var(--no); font-weight:700 } .summary td.v-more { color:var(--more); font-weight:700 } - #feedback { width:100%; min-height:150px; font:12.5px ui-monospace,monospace; padding:10px; border:1px solid var(--line); border-radius:8px } #copy { font:600 15px system-ui; padding:10px 18px; border-radius:8px; border:0; background:var(--acc); color:#fff; cursor:pointer; margin:8px 0 } .closing { color:var(--ink2); font-size:13px } - #st { color:var(--ink2); font-size:13px } -''' - -JS = ''' -(function(){ - const KEY=%s, STEPS=%s; // STEPS: [{key,id,n,kind,what}] - let st={}; try{ st=JSON.parse(localStorage.getItem(KEY)||'{}'); }catch(e){} - const q=new URLSearchParams(location.search).get('step'); // ?step=N for screenshots/deep links - let cur=Math.min(q!=null?+q:(st.__cur||0), STEPS.length); - const steps=[...document.querySelectorAll('.step')], sum=document.querySelector('.summary'), dots=[...document.querySelectorAll('.dot')]; - const save=()=>{ try{ localStorage.setItem(KEY, JSON.stringify(st)); }catch(e){} paint(); }; - function go(i){ cur=Math.max(0,Math.min(i,STEPS.length)); st.__cur=cur; save(); window.scrollTo(0,0); } - function decide(v){ if(cur>=STEPS.length) return; st[STEPS[cur].key]=v; save(); setTimeout(()=>go(cur+1), 260); } - function paint(){ - steps.forEach((s,i)=>{ s.classList.toggle('on', i===cur); const v=st[s.dataset.key]; s.classList.toggle('decided', !!v); - s.querySelectorAll('.decide button').forEach(b=>b.classList.toggle('on', b.dataset.v===v)); }); - sum.classList.toggle('on', cur===STEPS.length); - dots.forEach((d,i)=>{ d.classList.toggle('on', i===cur); if(i'+(s.id!==lastId?s.id:'')+''+s.n+''+s.what+''+(v||'—')+''+(note?note.replace(/'); lastId=s.id; } - document.getElementById('feedback').value=lines.join('\\n'); document.getElementById('rows').innerHTML=rows.join(''); - document.getElementById('st').textContent=done+' of '+STEPS.length+' decided'; - } - steps.forEach(s=>{ s.querySelectorAll('.decide button').forEach(b=>b.addEventListener('click',()=>decide(b.dataset.v))); - const n=s.querySelector('.note'); n.value=st['note:'+s.dataset.key]||''; n.addEventListener('input',()=>{ st['note:'+s.dataset.key]=n.value; save(); }); - const show=()=>{ const t=s.querySelector('.tab.on')?.dataset.theme, c=s.querySelector('.ctab.on')?.dataset.col; - s.querySelectorAll('.frame').forEach(f=>f.classList.toggle('on', (!t||f.dataset.theme===t)&&(!c||f.dataset.col===c))); }; - s.querySelectorAll('.tab').forEach(b=>b.addEventListener('click',()=>{ s.querySelectorAll('.tab').forEach(x=>x.classList.remove('on')); b.classList.add('on'); show(); })); - s.querySelectorAll('.ctab').forEach(b=>b.addEventListener('click',()=>{ s.querySelectorAll('.ctab').forEach(x=>x.classList.remove('on')); b.classList.add('on'); show(); })); - }); - dots.forEach((d,i)=>d.addEventListener('click',()=>go(i))); - document.getElementById('prev').addEventListener('click',()=>go(cur-1)); document.getElementById('next').addEventListener('click',()=>go(cur+1)); - document.addEventListener('keydown',e=>{ if(e.target.tagName==='INPUT'||e.target.tagName==='TEXTAREA') return; const k=e.key.toLowerCase(); - if(k==='y') decide('yes'); else if(k==='n') decide('no'); else if(k==='m') decide('more'); else if(e.key==='ArrowRight') go(cur+1); else if(e.key==='ArrowLeft') go(cur-1); - else if(k==='t'){ const s=steps[cur]; if(!s) return; const t=[...s.querySelectorAll('.tab')]; if(t.length<2) return; const i=t.findIndex(x=>x.classList.contains('on')); t[(i+1)%%t.length].click(); } - else if(e.code==='Space'){ const s=steps[cur]; if(!s) return; const t=[...s.querySelectorAll('.ctab')]; if(t.length<2) return; e.preventDefault(); const i=t.findIndex(x=>x.classList.contains('on')); t[(i+1)%%t.length].click(); } }); - document.getElementById('copy').addEventListener('click',()=>{ const t=document.getElementById('feedback'); const ok=()=>{ document.getElementById('copied').textContent='Copied — paste it into the chat.'; }; - (navigator.clipboard?.writeText(t.value)||Promise.reject()).then(ok,()=>{ t.select(); document.execCommand('copy'); ok(); }); }); - paint(); -})(); -''' +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +from deck.build import build_page # noqa: E402 +from deck.crops import crop_images # noqa: E402 +from deck.serve import already_served, serve, wait_for_submit # noqa: E402 +from deck.spec import SpecError, load_spec, validate # noqa: E402 def build(spec): - steps, parts = [], [] - total = sum(len(it['points']) for it in spec['items']) - for it in spec['items']: - for pt in it['points']: - steps.append({'key': f'{it["id"]}:{pt["n"]}', 'id': it['id'], 'n': pt['n'], 'kind': pt['kind'], 'what': pt['what']}) - parts.append(step_html(spec, it, pt, len(steps) - 1, total)) - dots = ''.join('' for _ in steps) + '' - closing = f'
{spec["closing"]}
' if spec.get('closing') else '' - page = f''' -{html.escape(spec["title"])} - -
-{chr(10).join(parts)} -

{html.escape(spec["title"])} — your answers

-

{spec.get("lead", "")}

-
Item#PointAnswerNote
- - -{closing} -
-
- - - -''' + """Crop + resolve boxes + write the page. Returns 0, or 1 with the reasons on stderr and NO page written.""" + errors, warnings = validate(spec) + if errors: + print('\n'.join(errors), file=sys.stderr) + return 1 + r = crop_images(spec, log=lambda m: print(m, file=sys.stderr)) + for w in warnings + r['warnings']: + print('warning: ' + w, file=sys.stderr) + print(f'{r["count"]} crops → {os.path.join(spec["_base"], spec["images"])}') + if r['missing']: + return 1 + page, _ = build_page(spec, r['boxes']) out = os.path.join(spec['_base'], spec['out']) with open(out, 'w') as f: f.write(page) print('wrote', out) + return 0 + + +def main(argv): + ap = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + sub = ap.add_subparsers(dest='cmd', required=True) + for c in ('build', 'serve', 'wait'): + sub.add_parser(c).add_argument('spec') + for c in ('serve', 'wait'): + sub.choices[c].add_argument('--timeout', type=float, default=240, help='minutes to wait for a submit (exit 2 after)') + sv = sub.choices['serve'] + sv.add_argument('--no-open', action='store_true') + sv.add_argument('--no-build', action='store_true', help='serve the page as it is on disk') + sv.add_argument('--port', type=int, default=0) + a = ap.parse_args(argv) + try: + spec = load_spec(a.spec) + if a.cmd == 'build': + return build(spec) + if a.cmd == 'wait': + return wait_for_submit(spec, timeout_min=a.timeout) + # WHY the lock is checked before build(): a second `serve` of the same spec used to + # rebuild the page and re-cut the crops out from under the running server, THEN exit 3. + other = already_served(spec) + if other is not None: + print(f'REFUSING: {spec["_stem"]} is already served by pid {other["pid"]} at {other["url"]}', file=sys.stderr) + return 3 + if not a.no_build and build(spec) != 0: + return 1 + return serve(spec, port=a.port, open_browser=not a.no_open, timeout_min=a.timeout) + except SpecError as e: + print(str(e), file=sys.stderr) + return 1 if __name__ == '__main__': - if len(sys.argv) != 3 or sys.argv[1] not in ('crop', 'build'): - print(__doc__); sys.exit(2) - spec = load(sys.argv[2]) - (crop if sys.argv[1] == 'crop' else build)(spec) + sys.exit(main(sys.argv[1:])) diff --git a/scripts/ui-review/run-review.sh b/scripts/ui-review/run-review.sh index 82c51f3f..e08540f1 100644 --- a/scripts/ui-review/run-review.sh +++ b/scripts/ui-review/run-review.sh @@ -39,7 +39,16 @@ PORT_OFFSET="${YOUCODED_PORT_OFFSET:-300}" VITE_PORT=$((5173 + PORT_OFFSET)) export WB_PORT=$VITE_PORT # shot.mjs rewrites the plans' hardcoded 5233 to this mkdir -p "$OUT/sheets" +# One id per sweep, stamped on every manifest entry: coverage merges by it, and the sheets +# below are rebuilt only for the plans this sweep actually ran (hand-off gaps 6 and 7). +RUN_ID="$(date +%s%3N)"; export UI_REVIEW_RUN=$RUN_ID STARTED_WB=0 +# WHY a trap and not just a line at the end: the port probe and the boot check both `exit 1`, +# which skipped that last line — a refused probe left Vite running on 5473, and the NEXT sweep +# then hit the wrong-worktree refusal because a foreign server was already answering. +# Only ever kills a workbench THIS sweep started (STARTED_WB=1); a reused one is left alone. +cleanup_wb() { [ "${STARTED_WB:-0}" = 1 ] && pkill -f "[v]ite --port $VITE_PORT" || true; } +trap cleanup_wb EXIT if [[ "$REPORTS_ONLY" == 0 ]]; then # 1. Workbench (reuse one that is already up on this port). @@ -74,7 +83,7 @@ IFS=',' read -r -a T <<< "$THEMES" JOBS="${UI_REVIEW_JOBS:-24}" PER_SHARD="${UI_REVIEW_SHARD_SIZE:-8}" # shots per process before the plan is split further jobfile="$OUT/jobs.txt"; : > "$jobfile" -idx=0 +idx=0; ports=() for plan in "$HERE"/plans/*.json; do name="$(basename "$plan" .json)" case "$name" in electron-*) continue;; esac @@ -84,20 +93,24 @@ for plan in "$HERE"/plans/*.json; do for t in "${T[@]}"; do for ((k=0; k> "$jobfile" # CDP ports keyed by offset so two reviews never share one + port=$((30000 + PORT_OFFSET + idx)); ports+=("$port") + echo "$plan $name $t $k/$n $port" >> "$jobfile" # CDP ports keyed by offset so two reviews never share one done done done +bash "$HERE/probe-ports.sh" "${ports[@]}" || exit 1 echo "[ui-review] $idx capture jobs, $JOBS at a time…" run_job() { CDP_PORT=$5 SHARD=$4 node "$HERE/shot.mjs" "$1" "$OUT/shots-$2" "$3" > "$OUT/run-$2-$3-${4%/*}.log" 2>&1 || true; } export -f run_job; export HERE OUT WB_PORT xargs -P "$JOBS" -L 1 bash -c 'run_job "$@"' _ < "$jobfile" fi -rm -rf "$OUT/sheets"/*.jpg - # 3. Sheets (verified shots only — misses live in _unverified/), reports, gallery. +# A sweep rebuilds sheets only for the plans it ran (their manifests carry RUN_ID); +# --reports-only rebuilds everything. for d in "$OUT"/shots-*; do name="${d##*/shots-}" + if [[ "$REPORTS_ONLY" == 0 ]] && ! grep -lq "\"run\": \"$RUN_ID\"" "$d"/manifest-*.json 2>/dev/null; then continue; fi + rm -f "$OUT/sheets/$name-"*.jpg bash "$HERE/montage.sh" "$d" "$OUT/sheets-$name" "$THEMES" >/dev/null for f in "$OUT/sheets-$name"/*.png; do [ -f "$f" ] && magick "$f" -resize 1800x -quality 82 "$OUT/sheets/$name-$(basename "$f" .png).jpg"; done done @@ -106,5 +119,5 @@ node "$HERE/contrast-report.mjs" "$OUT"/shots-* > "$OUT/contrast.md" python3 "$HERE/make-gallery.py" "$OUT/sheets" "$OUT/gallery.html" >/dev/null echo "[ui-review] done → $OUT/gallery.html" head -3 "$OUT/coverage.md" -grep -c MISSED "$OUT/coverage.md" | xargs -I{} echo "[ui-review] {} surfaces MISSED — read $OUT/coverage.md before writing any finding" -[ "$STARTED_WB" = 1 ] && pkill -f "[v]ite --port $VITE_PORT" || true +(grep -c MISSED "$OUT/coverage.md" || true) | xargs -I{} echo "[ui-review] {} surfaces MISSED — read $OUT/coverage.md before writing any finding" +# (the workbench this sweep started is stopped by the EXIT trap above, on every exit path) diff --git a/scripts/ui-review/shot.mjs b/scripts/ui-review/shot.mjs index 4ca72acc..ea759b06 100644 --- a/scripts/ui-review/shot.mjs +++ b/scripts/ui-review/shot.mjs @@ -39,6 +39,10 @@ // {"dispatch": {"name": "buddy:attach-file", "detail": {...}}} window CustomEvent // {"scrollDialog": "bottom"|"top"|} scroll the open dialog's body // {"wait": 500} pause (ms); every action accepts "settle" +// shot-level: "measure": ["", {"text": "Label", "tag": "button"}] +// → entry.measures[key] = {x,y,w,h} in window pixels (key = the css string or "text: