From 2d8cb44b035d05eea8590eb2baa53a962a5e1ef1 Mon Sep 17 00:00:00 2001 From: Destin Date: Thu, 27 Aug 2026 02:10:26 -0700 Subject: [PATCH 01/31] =?UTF-8?q?docs(spec):=20review=20deck=20v2=20?= =?UTF-8?q?=E2=80=94=20the=20approved=20page=20(mockup=20G)=20plus=20the?= =?UTF-8?q?=20answers=20server,=20rig-measured=20highlights=20and=20writin?= =?UTF-8?q?g=20rules?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01P9Gi6kLrnjb3qPEcZMHnkf --- .../prototypes/2026-08-27-deck-mockup-g.html | 157 ++++++++ .../prototypes/2026-08-27-deck-mockup-g.py | 216 +++++++++++ .../specs/2026-08-27-review-deck-v2-design.md | 338 ++++++++++++++++++ 3 files changed, 711 insertions(+) create mode 100644 docs/active/prototypes/2026-08-27-deck-mockup-g.html create mode 100644 docs/active/prototypes/2026-08-27-deck-mockup-g.py create mode 100644 docs/active/specs/2026-08-27-review-deck-v2-design.md 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/active/specs/2026-08-27-review-deck-v2-design.md b/docs/active/specs/2026-08-27-review-deck-v2-design.md new file mode 100644 index 00000000..2b34dca7 --- /dev/null +++ b/docs/active/specs/2026-08-27-review-deck-v2-design.md @@ -0,0 +1,338 @@ +--- +status: draft +created: 2026-08-27 +owner: Destin (decisions) / Claude (draft) +related: + - docs/active/handoffs/2026-08-27-review-deck-tooling-handoff.md (the brief and the known gaps) + - docs/active/prototypes/2026-08-27-deck-mockup-g.html (the approved page — "this is perfect", 2026-08-27) + - docs/active/prototypes/2026-08-27-deck-mockup-g.py (regenerates the mockup) + - scripts/ui-review/README.md, .claude/skills/ui-review/SKILL.md (the tooling this replaces parts of) +--- + +# Review deck v2 — design + +## 1. What this is + +The **review deck** is the page Destin looks at to approve or reject UI changes, one +point per step. This spec replaces the v1 deck (`scripts/ui-review/review-cards.py`, the +generic-looking page with one pulsing ring, Y/N/M hotkeys and a copy-paste feedback +block) with a page that wears the app's own look, shows Before and After side by side +with the changed region measured by the screenshot rig, saves every answer to a file +as it is given, and tells Claude when Destin is done — no copying, no pasting, no +clicking a file to open it. + +Three parts change: + +| Part | Today | After this spec | +|---|---|---| +| The page | `review-cards.py build` → static HTML, generic styling, ring at a guessed % | Same command, new page (§3): app tokens, adaptive layout, loupe + zoom, three cards, coloured progress, submit dialog | +| Getting it to Destin and answers back | He opens the file; Copy feedback → paste into chat | `review-cards.py serve` starts a tiny local server, opens the browser tab itself, writes `.answers.json` on every click, exits when he submits — Claude is waiting on that exit | +| Highlight boxes | Hand-estimated `[x%, y%]` in the spec | The rig measures a named element per shot, or the deck computes the changed region from the Before/After pixels; the spec never carries coordinates | + +Out of scope, deliberately: the workbench toolbar (Destin: deck controls first), the +in-app artifact viewer bridge (rejected in favour of the answers file), theme-folder +serving in the workbench (gap 2 — ROADMAP). + +## 2. Decisions taken in the design session (2026-08-27) + +All of these were reached by looking at rendered mockups, not descriptions; the +approved mockup is the visual reference for §3 and wins over any prose here if they +disagree. + +- **Where reviews happen:** a browser tab the tool opens itself. Answers go to a file + next to the spec; Claude watches for the submit. (Rejected: in-app viewer bridge — + needs app code and lets any HTML fill the composer; copy/paste — the ritual we are + removing.) +- **Theme:** the page follows the theme of the crop being viewed (Midnight page around a + Midnight crop, Light around Light). Built-in tokens are inlined; a community theme's + tokens come from its `manifest.json`. +- **Boundary:** the deck is a distinct surface — inset on `well`, framed in the deck's + amber, with a `REVIEW DECK | ` chip sitting on the frame — because inside the + app's file panel a same-theme deck was mistaken for the app. +- **Amber (`#FFB020`) is the tool's one identity colour**: frame, chip, highlight box, + loupe crosshair, Risk card edge, current progress segment. No built-in theme uses it + for chrome, so it reads as "the review tool" in every theme. +- **Both pictures always.** Before | After side by side (or stacked), no flip toggle, no + separate "zoomed on the change" strip — the loupe and zoom do that. +- **Highlight = a box, not a circle**, with a soft halo; nothing outside it is dimmed + (a spotlight dim greyed the Light theme out). +- **Answers:** Yes · No · Other. No hotkeys, no auto-advance; **Save & Next** commits. + Note placeholder is "Add a note (optional)" after Yes/No and "Explain what you'd like + instead…" after Other. Header **Next ›** moves on without an answer = skipped. +- **Progress bar** segments: green yes · red no · amber other · grey skipped · faint + untouched · outlined current. Clickable to revisit. No ledger ids (P-3, P-21…) anywhere + a reviewer reads — those live in the spec and the answers file only. +- **Done — Submit Feedback** opens a dialog that says what happens (saved as you went; + submitting notifies Claude; nothing to paste; close the tab) and warns about skipped + steps with *Go to first skipped*. +- **Three peer containers on one grid** — pictures, explanation (headline + What changed + / You'll notice / Risk cards), answer controls — with outer edges always meeting, on a + content column that scales with the window (`clamp(900px, 80vw, 1640px)`). +- **Layout is chosen by measurement, not by aspect thresholds** (§3.4). +- **Theme thumbnails** (every theme's After crop) stacked in the right margin; they drop + into a row above the grid when the margin is under 150px. +- **Text vocabulary is fixed** (§5): headline, What changed (+ measured line), You'll + notice, Risk. "Why / details" and "Tell me more" are gone — *Other* with a note is how + Destin asks for more. + +Ideas offered and **not** taken (recorded so they are not re-proposed as new): +cover/intro step, "hold Space to peek", multiple labelled boxes per step, attach-your-own- +screenshot (gap 4), "open this screen live in the workbench". The last two go to ROADMAP +as `idea`. + +## 3. The page + +### 3.1 Anatomy (top to bottom, wide window) + +1. **Chip** on the frame, top-left: `REVIEW DECK | <deck title>`. Amber, dark text, + 1px divider. The title is the spec's `title`. +2. **Header** (panel surface, rounded top): left — the step's **surface name** and its + small uppercase path (`Themes dialog · SETTINGS → APPEARANCE`); centre — `‹ Prev`, + the progress bar, `Next ›`; right — `step n of N · k answered` and **Done — Submit + Feedback**. Below 1400px the path drops; below 950px the count drops; below 760px + Done shortens to "Done". +3. **Content column** (shared width with the header's inner row), a grid of three + framed containers (panel surface, `edge` border, `radius-lg`): + - **Pictures**: `BEFORE` and `AFTER` captions, the crops, the amber highlight box on + each, a `− 100% +` pill sticky at the top-right. Hovering a picture shows a 180px + round **loupe** at 2.5× with an amber crosshair at the cursor point; the cursor is + hidden only over the picture. `L` toggles the loupe; `+`/`−`/`0` zoom in 10% steps + (100–400%); zooming centres on the highlight. + - **Explanation**: the **headline** (largest text on the page), then the cards + ✎ **What changed** (with a small "Measured: …" footnote when there is a number), + 👁 **You'll notice**, ⚠ **Risk** (amber-edged; omitted when the step has no risk — + the others widen to fill). + - **Answer**: Yes, keep it (green dot) · No, revert it (red) · Other (amber) · note + field · **Save & Next ›** (primary; disabled until an answer is chosen; hover lifts). + Buttons scale with the window (`clamp(34px, 4.4vh, 52px)` tall) and share the row in + fixed proportion (three equal answers, the note takes the rest). +4. **Theme thumbnails** in the right margin, vertical, the current one outlined. +5. **Submit dialog** (veil + panel): the explanation, the skipped-steps warning listing + step numbers, `Keep reviewing` · `Go to first skipped` · `Submit`. + +Keyboard: `←`/`→` prev/next (skip semantics), `+`/`−`/`0` zoom, `L` loupe. Nothing else. + +### 3.2 Embedded vs. tab + +`window.top !== window` means the page is inside the app's file panel: the deck adds a +62px bottom margin so the panel's floating **Edit** button never covers Save & Next. +In a browser tab there is no margin. The page also works from `file://` with no server +(§4.4) so archived decks in `docs/` stay readable. + +### 3.3 Theme following + +The `<html data-theme>` attribute follows the selected thumbnail. The page inlines the +four built-in token sets from `globals.css` and, for each community theme in the spec, +the `tokens` block of its `manifest.json` (`wecoded-themes/themes/<slug>/`). Halftone's +larger radii are honoured (`--radius-md/lg`). Only the tokens the page uses are +inlined: `canvas panel inset well accent on-accent fg fg-2 fg-dim fg-muted fg-faint edge +link` plus radii. + +### 3.4 Layout algorithm + +For the current step the page tries each arrangement **for real** (applies the grid +class, reads the picture container's box) and scores it by the scale the two crops +would get: + +| Key | Arrangement | +|---|---| +| A | Before \| After side by side, explanation and answer below | +| B | Before over After on the left, explanation on the right, answer below both | +| C | Before \| After on the left, explanation on the right, answer below both | +| D | Before over After, explanation and answer below | + +Rules: B/C are only allowed when the content column is ≥ 820px wide; A wins ties +within 5%; if the best scale is under **50%** the page switches to **compact** — one +scrolling column, pictures at full width, the answer container pinned to the bottom of +the view. Pictures never upscale past 150%. The choice re-runs on every resize and on +every step change (zoom resets to 100% on step change). + +The mockup prints the chosen layout in a small badge at the bottom; the real page does +not. + +## 4. The machinery + +### 4.1 Spec (JSON, v2) + +```json +{ + "title": "Phase C review", + "key": "phase-c-review", + "out": "phase-c-review.html", + "images": "images/phase-c-review", + "runs": { "before": "/abs/scratch/ui-phase-c-baseline", "after": "/abs/scratch/ui-phase-c-after" }, + "themes": ["midnight", "light", "creme", "dark", "halftone-dimension", "meadow-mist"], + "crops": { "themes-dialog": ["main", "settings-appearance", "440x600+500+150"] }, + "steps": [ + { "id": "P-3.1", + "surface": "Themes dialog", "path": "Settings → Appearance", + "crop": "themes-dialog", + "highlight": { "selector": "[data-testid=theme-card]:first-child" }, + "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, every card 92 px tall. …", + "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.", + "risk": "In these screenshots Halftone and Meadow show the colour-strip fallback because the rig cannot serve theme folders." } + ] +} +``` + +- `runs` has one entry (a "today" deck: the same picture shown once, captioned `TODAY`) + or two (`before`/`after`). More than two is not supported. +- `crops` merges over `scripts/ui-review/crops.json` as today. Geometry is on the + full-window shot; the same crop is cut for every theme × run. +- `highlight` is one of: `{"selector": "<css>"}` (measured by the rig, §4.2), + `{"text": "<visible text>"}` (same, matched by `textContent`), `"auto"` (pixel diff, + §4.2 — only with two runs), or `{"box": [x%, y%, w%, h%]}` (escape hatch; the build + warns). Absent = `"auto"` when there are two runs, else an error. +- `id` is Claude's ledger key (`P-3.1`), never rendered on the page. +- Answers, notes and what was looked at are keyed by `id` in the answers file. + +### 4.2 Highlight boxes + +**Measured (`selector` / `text`).** `shot.mjs` gains a per-shot `measure` list (the +plan's shot, not the deck spec, is what runs in the browser). `review-cards.py crop` +reads the deck spec and, for every step with a `selector`/`text` highlight, checks the +run's manifest for a matching measurement; the rig writes them as +`entry.measures = { "<selector>": {x, y, w, h} }` in **window pixels** — the deck's crop +step converts to percentages of the crop rectangle. If a measurement is missing the +build fails with the exact `measure` line to add to the plan, so the fix is one paste +and one re-run of that plan. (Plans keep their `expect`; `measure` is additive.) + +**Auto (pixel diff).** For two-run decks with no selector, `crop` computes the changed +region per theme: `magick before.png after.png -compose difference -composite +-threshold 6% -morphology dilate square:3 -format %@ info:` gives the bounding box of +what changed; it is intersected with the crop rectangle, padded 6px, and used for both +pictures. If the changed region covers more than 60% of the crop the build warns +("whole-surface change — name an element instead") but still builds. + +Both paths write the resolved boxes into the built HTML per theme × run; the spec stays +coordinate-free. + +### 4.3 Serve, open, answers, submit + +``` +python3 scripts/ui-review/review-cards.py serve <spec.json> [--no-open] [--port N] [--timeout MIN] +``` + +- Starts `http.server` on `127.0.0.1` (free port unless `--port`), serving the spec's + directory. Opens `http://127.0.0.1:<port>/<out>` with `xdg-open` / `open` / `start` + unless `--no-open`. Prints the URL either way (the chat fallback). +- `GET /answers` → the current answers file (or `{}`); the page loads it on open, so a + closed tab resumes where it was. `POST /answers` with the full state → written + atomically (`.tmp` + rename) to `<spec-stem>.answers.json`. `POST /submit` → sets + `submitted` and the server exits 0 after replying. +- **How Claude finds out:** it runs `serve` in the background (`run_in_background`) and + is re-invoked when the process exits — i.e. when Destin submits. `serve` prints the + feedback summary (§4.5) on exit so the notification carries the answers. `--timeout` + (default 240 min) exits 2 with "no submit" so a forgotten deck does not hold a session + forever; the answers file is still complete. +- Only one deck is served per process; a second `serve` for the same spec refuses if + the port file `<spec-stem>.serve.json` names a live pid. + +Answers file: + +```json +{ "deck": "phase-c-review", "started": "2026-08-27T18:02:11Z", "submitted": null, + "answers": { "P-3.1": { "v": "yes", "note": "", "theme": "light", "zoom": 1.2, "seconds": 41 }, + "P-3.2": { "v": "skip" } } } +``` + +`theme`/`zoom`/`seconds` record what Destin was looking at when he answered — invisible +to him, useful to Claude ("No" given on Halftone means Halftone). + +### 4.4 No server (file://) + +If `GET /answers` fails at load, the page keeps state in `localStorage` and the submit +dialog shows the old textarea + **Copy feedback** instead of the notify text. This is +the archive path; it is never the intended review path. + +### 4.5 Feedback summary (what Claude receives) + +Plain text, one line per step, ledger id first, in spec order: + +``` +phase-c-review · submitted 2026-08-27 18:40 · 11 yes · 1 no · 1 other · 0 skipped +P-3.1 yes +P-3.2 no — "keep the pencil" +P-21.1 other — "make the featured card a touch taller instead" +``` + +## 5. Writing rules (enforced by `build`) + +The builder refuses to build, naming the step and the rule, when: + +- `headline` is missing or over 25 words; `changed` or `notice` is missing. +- any text field contains a banned word (case-insensitive): token, primitive, selector, + IPC, prop, props, reducer, handler, component, Tailwind, CSS class, React, DOM, + z-index. (Measurements like "92 px" are fine.) +- a listed theme has no captured crop for a step (a missing picture is a bug in the + capture, never a blank in the deck), or a highlight cannot be resolved (§4.2). +- `surface`/`path` are missing (the header would be empty). + +It warns (builds anyway) on: `box` highlights, auto-highlights over 60% of the crop, +a `risk` over 40 words, a `measured` value with no digit. + +House style, checked by eye not code: the headline says what a user sees, not what +was edited; *What changed* says what was edited in plain words; *You'll notice* is the +sentence Destin's CLAUDE.md asks for — what changes for users, intended and side +effects; *Risk* is what could look wrong or is not shown faithfully in the pictures. + +## 6. Rig changes shipped alongside + +From the hand-off's known gaps, the ones this work touches: + +1. **Two sweeps deadlock (gap 1):** `run-review.sh` probes each CDP port with `ss` + before writing the job file and refuses loudly, naming the conflicting offset. +3. **Hand-estimated markers (gap 3):** closed by §4.2. +5. **Brittle `expect`s (gap 5):** README gets a "prefer `aria-label`/role/`data-testid` + over visible text" rule and the `measure` docs. +6. **Stale coverage rows (gap 6):** manifests carry a `run` id (`Date.now()` at + `run-review.sh` start, passed through `UI_REVIEW_RUN`); `coverage.mjs` only merges + manifests from the newest run id present. +7. **Sheets rebuilt for every plan (gap 7):** `run-review.sh` rebuilds sheets only for + the `shots-<plan>` dirs whose manifests carry the current run id. + +Gaps 2 and 4 go to ROADMAP. + +## 7. Files + +| File | Change | +|---|---| +| `scripts/ui-review/review-cards.py` | Rewritten: `crop`, `build` (v2 page + rules), `serve`. v1 spec format is not read; the three built v1 pages stay as static HTML. | +| `scripts/ui-review/deck/` (new) | `page.html.tmpl`, `page.css`, `page.js` — the page split out of the Python so it can be edited as HTML; `build` inlines them. `tokens.json` — the four built-in token sets, checked against `globals.css` by a test. | +| `scripts/ui-review/shot.mjs` | `measure` per shot → `entry.measures`; `UI_REVIEW_RUN` in the manifest. | +| `scripts/ui-review/run-review.sh` | port probe; run id; sheets scoped to the run. | +| `scripts/ui-review/coverage.mjs` | merge by run id. | +| `scripts/ui-review/tests/` (new) | `test_deck.py` (spec validation, rules, box mapping, auto-diff on fixture PNGs, serve round-trip), run with `python3 -m unittest discover scripts/ui-review/tests`. | +| `scripts/ui-review/README.md`, `.claude/skills/ui-review/SKILL.md` §4, `CLAUDE.md` (the one sentence describing the deck), `.claude/skills/ui-mockup/SKILL.md` | Updated to the v2 flow: `crop → build → serve`, answers file, writing rules. | +| `docs/active/design/2026-08-25-ui-audit/` | Untouched; the next phase writes a v2 spec. | +| Memory `feedback-review-page-format` | Updated: the v2 deck is the format; note what was rejected on the way (rail, alternatives, links, toggles). | + +## 8. Testing + +- **Python unit tests** (`scripts/ui-review/tests/test_deck.py`): spec loading and + merge with `crops.json`; every §5 refusal and warning; window-px → crop-% mapping + including a crop that partially contains the element; auto-diff bounding box on two + synthetic PNGs (a known rectangle differs); `serve` GET/POST/submit round-trip on a + free port with the file written atomically; token sets equal `globals.css` values. +- **Headless render check** (`scripts/ui-review/tests/deck-render.mjs`, node + Chrome as + `shot.mjs` uses): builds the fixture deck, loads it at 1920×1080, 1100×900 and + 520×760, asserts no console errors, the chosen layout per size (C / C / compact), the + answer container visible in all three, and that a click on Yes + Save & Next POSTs + one answers record. +- **Rig**: `shot.mjs` measure covered by running `main.json`'s `settings-appearance` + shot with a `measure` entry in the workbench boot check path; `coverage.mjs` run-id + merge by a unit test over two fixture manifests. +- **Destin's pass** (per the workspace rule, not scripted): open a real deck built + from the Phase C runs, in a browser tab and inside the file panel; hover, zoom, + answer, skip, submit; confirm the chat session wakes with the summary. + +## 9. Rollout + +1. Build the tooling in the `_deck-tooling` worktree of `youcoded-dev` (no sub-repo + code changes); rig changes are in `scripts/`, so they land with it. +2. Rebuild the Phase C review deck from the existing `scratch/ui-phase-c-*` runs as the + first real v2 deck (the mockup's content came from it), and review it end to end. +3. Merge, archive this spec to `docs/archive/specs/`, flip the ROADMAP tooling entry, + update memory. From b9f1e96125b75f0baf2829da44efd759ac65d935 Mon Sep 17 00:00:00 2001 From: Destin <destinj101@gmail.com> Date: Thu, 27 Aug 2026 02:25:18 -0700 Subject: [PATCH 02/31] =?UTF-8?q?docs(plan):=20review=20deck=20v2=20?= =?UTF-8?q?=E2=80=94=2015=20tasks=20from=20spec=20loader=20to=20the=20rebu?= =?UTF-8?q?ilt=20Phase=20C=20deck?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P9Gi6kLrnjb3qPEcZMHnkf --- .../active/plans/2026-08-27-review-deck-v2.md | 2082 +++++++++++++++++ 1 file changed, 2082 insertions(+) create mode 100644 docs/active/plans/2026-08-27-review-deck-v2.md diff --git a/docs/active/plans/2026-08-27-review-deck-v2.md b/docs/active/plans/2026-08-27-review-deck-v2.md new file mode 100644 index 00000000..7dd429c8 --- /dev/null +++ b/docs/active/plans/2026-08-27-review-deck-v2.md @@ -0,0 +1,2082 @@ +--- +status: active +created: 2026-08-27 +spec: docs/active/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`); 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 `<spec-stem>.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. +- 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/`. +- 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: `crop`, `build`, `serve`. Rewritten (v1 removed). | +| `scripts/ui-review/deck/__init__.py` | package marker | +| `scripts/ui-review/deck/spec.py` | load spec, merge `crops.json`, 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`, resolve each step's highlight per theme × run, write `boxes.json` | +| `scripts/ui-review/deck/build.py` | 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 | +| `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` | 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]`; `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, 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(write_spec(self.d, steps=None) and write_spec(self.d, **{}).replace('deck.json', 'x')) if False else load_spec(self._without('title')) + 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() +``` + +(The `test_missing_top_level_key_raises` body is deliberately the simple form — replace it with:) + +```python + def test_missing_top_level_key_raises(self): + with self.assertRaises(SpecError): load_spec(self._without('title')) +``` + +- [ ] **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 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'(?<![\w-])' + re.escape(w) + r'(?![\w-])', low)] + + +def validate(spec): + """Returns (errors, warnings) as 'step-id: message' lines. Errors block crop/build.""" + errors, warnings, ids = [], [], set() + two_runs = len(spec['runs']) == 2 + for i, st in enumerate(spec['steps']): + sid = st.get('id') or f'step {i + 1}' + if not st.get('id'): + errors.append(f'{sid}: missing id') + elif st['id'] in ids: + errors.append(f'{sid}: duplicate id') + ids.add(st.get('id')) + for k in ('surface', 'path', 'crop', 'headline', 'changed', 'notice'): + if not st.get(k): + errors.append(f'{sid}: missing {k}') + if st.get('crop') and st['crop'] not in spec['_crops']: + errors.append(f'{sid}: unknown crop "{st["crop"]}" (add it to crops.json or the spec\'s "crops")') + if word_count(st.get('headline')) > 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` (12 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)) + +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 dilate joins hairline changes.""" + out = subprocess.run(['magick', a, b, '-compose', 'difference', '-composite', '-threshold', threshold, + '-morphology', 'Dilate', 'Square:3', '-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 + 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` (9 tests). If `test_identical_images_have_no_box` fails because `%@` prints a non-empty box for a uniform image, print `out` and adjust the `w * h < 4` guard to the value ImageMagick returns for "nothing" (it must still pass the changed-rectangle test). + +- [ ] **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` = `{ "<theme>": { "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, unittest +HERE = os.path.dirname(os.path.abspath(__file__)) +TOKENS = os.path.join(os.path.dirname(HERE), 'deck', 'tokens.json') +GLOBALS = os.path.join(os.path.dirname(HERE), '..', '..', '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) + @unittest.skipUnless(os.path.exists(GLOBALS), 'youcoded checkout not present') + def test_values_match_globals_css(self): + 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: + - `<tmp>/runs/before/shots-main/<theme>/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). + - `<tmp>/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}}}`. + - `<tmp>/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` (`"<crop>--<theme>--<run>.png"`); `newest_manifest_entry(run_dir, plan, shot, theme) -> dict | None`; `measure_key(hl) -> str`; `crop_images(spec) -> {'boxes': {id: {theme: {run: [x,y,w,h]}}}, 'missing': [str], 'warnings': [str], 'count': int}` and writes `<images>/boxes.json`. + +- [ ] **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(self): + self.assertEqual(self.r['count'], 3 * 2 * 2) # steps × themes × runs (S-1..3 share one crop but are cut per step) + self.assertTrue(os.path.exists(os.path.join(self.images, image_name('c', 'light', 'after')))) + self.assertTrue(os.path.exists(os.path.join(self.images, 'boxes.json'))) + 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%; padded by 6px (=1.5% / 3%) + 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.assertNotIn('light', r['boxes']['S-2']) if 'S-2' not in r['boxes'] else 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_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 — later files win, like coverage.mjs.""" + files = sorted(glob.glob(os.path.join(run_dir, f'shots-{plan}', 'manifest-*.json')), key=os.path.getmtime) + found = None + for f in files: + with open(f) as fh: + for e in json.load(fh): + if e.get('name') == shot and e.get('theme') == theme: + found = e + 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, count = {}, [], [], 0 + 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 + subprocess.run(['magick', src, '-crop', geo, '+repage', dst], check=True) + count += 1 + 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 + with open(os.path.join(out_dir, 'boxes.json'), 'w') as f: + json.dump(boxes, f, indent=1) + for m in missing: + log('missing: ' + m) + return {'boxes': boxes, 'missing': missing, 'warnings': warnings, 'count': count} +``` + +- [ ] **Step 4: Run to verify pass** + +Run: `python3 -m unittest scripts/ui-review/tests/test_crops.py -v 2>&1 | tail -3` +Expected: `OK` (8 tests). In `test_missing_measurement_names_the_fix` the second assertion reduces to `self.assertEqual(r['boxes']['S-2']['light'], {})` — simplify it to that line. + +- [ ] **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 crops, 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('<title>Fixture review', html); self.assertIn('const DECK=', html) + self.assertIn('[data-theme="midnight"]{--canvas:#0D1117', html) # tokens inlined + 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): + t = theme_tokens(['midnight', 'halftone-dimension']) if os.path.exists(os.path.join(os.path.dirname(HERE), '..', '..', 'wecoded-themes', 'themes', 'halftone-dimension', 'manifest.json')) else None + if t is None: self.skipTest('wecoded-themes checkout not present') + self.assertEqual(t['halftone-dimension']['accent'].lower(), '#e51f48'); self.assertTrue(t['halftone-dimension']['_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_script_safe_json(self): + self.spec['steps'][0]['notice'] = 'a tag' + html, _ = build_page(self.spec, self.boxes); self.assertNotIn(' tag', html) + +if __name__ == '__main__': unittest.main() +``` + +- [ ] **Step 2: Run to verify failure** + +Run: `python3 -m unittest scripts/ui-review/tests/test_build.py 2>&1 | tail -2` +Expected: `ModuleNotFoundError: No module named 'deck.build'` + +- [ ] **Step 3: Write `page.html.tmpl`** + +```html +__TITLE__ + + +
+
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..70e58931 --- /dev/null +++ b/scripts/ui-review/deck/page.js @@ -0,0 +1,149 @@ +/* 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; localStorage still has it */ } } + } + + // ── 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'; + $('#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' }); + } + + // ── 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) { record(); cur = Math.max(0, Math.min(N - 1, i)); state.cur = cur; save(); zoom = 1; stepStart = Date.now(); render(); } + $$('.ans').forEach(b => b.onclick = () => { const id = DECK.steps[cur].id; state.answers[id] = { ...(state.answers[id] || {}), v: b.dataset.v }; paintState(); $('#note').focus(); }); + $('#note').addEventListener('input', e => { const id = DECK.steps[cur].id; state.answers[id] = { ...(state.answers[id] || {}), note: e.target.value }; }); + $('#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(); + 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 () => { + state.submitted = new Date().toISOString(); + try { await fetch('/submit', { method: 'POST', headers: { 'content-type': 'application/json' }, body: JSON.stringify(state) }); } catch (e) { /* server already gone */ } + $('#veil').classList.remove('on'); $('#done').textContent = 'Submitted ✓'; $('#done').disabled = true; $$('.ans,#save,#note').forEach(e => e.disabled = true); + }; + $('#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 (e.key === 'ArrowRight') go(cur + 1); if (e.key === 'ArrowLeft') go(cur - 1); + 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(); window.__deckReady = true; // the render test waits for this + }); +})(); diff --git a/scripts/ui-review/tests/test_build.py b/scripts/ui-review/tests/test_build.py new file mode 100644 index 00000000..45207c04 --- /dev/null +++ b/scripts/ui-review/tests/test_build.py @@ -0,0 +1,46 @@ +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('[data-theme="midnight"]{--canvas:#0D1117', html) # tokens inlined + 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_script_safe_json(self): + self.spec['steps'][0]['notice'] = 'a tag' + html, _ = build_page(self.spec, self.boxes); self.assertNotIn(' tag', html) + +if __name__ == '__main__': unittest.main() From b6036dc7654d64f31c166f0ee35bb514cd1bdef9 Mon Sep 17 00:00:00 2001 From: Destin Date: Thu, 27 Aug 2026 02:56:40 -0700 Subject: [PATCH 18/31] =?UTF-8?q?feat(ui-review):=20review-cards.py=20v2?= =?UTF-8?q?=20CLI=20=E2=80=94=20build,=20serve,=20wait?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01W4Kj4r7Vk87Kcb6UrNNp3x --- scripts/ui-review/review-cards.py | 287 ++++++---------------------- scripts/ui-review/tests/test_cli.py | 29 +++ 2 files changed, 88 insertions(+), 228 deletions(-) create mode 100644 scripts/ui-review/tests/test_cli.py diff --git a/scripts/ui-review/review-cards.py b/scripts/ui-review/review-cards.py index e8071bbe..50ae0e3b 100644 --- a/scripts/ui-review/review-cards.py +++ b/scripts/ui-review/review-cards.py @@ -1,242 +1,73 @@ #!/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/active/specs/2026-08-27-review-deck-v2-design.md (§4–5; archived after merge). 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 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) + 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/tests/test_cli.py b/scripts/ui-review/tests/test_cli.py new file mode 100644 index 00000000..dc4b1d15 --- /dev/null +++ b/scripts/ui-review/tests/test_cli.py @@ -0,0 +1,29 @@ +import io, json, os, sys, tempfile, unittest +from contextlib import redirect_stderr, redirect_stdout +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 +import importlib.util +spec_ = importlib.util.spec_from_file_location('review_cards', os.path.join(os.path.dirname(HERE), 'review-cards.py')); rc = importlib.util.module_from_spec(spec_); spec_.loader.exec_module(rc) + +class CliTests(unittest.TestCase): + def setUp(self): self.p = make_fixture(tempfile.mkdtemp()); self.d = os.path.dirname(self.p) + def run_cli(self, *args): + out, err = io.StringIO(), io.StringIO() + with redirect_stdout(out), redirect_stderr(err): code = rc.main(list(args)) + return code, out.getvalue(), err.getvalue() + def test_build_crops_and_writes_the_page(self): + code, out, err = self.run_cli('build', self.p); self.assertEqual(code, 0, err) + self.assertIn('4 crops', out); self.assertIn('wrote', out); self.assertTrue(os.path.exists(os.path.join(self.d, 'fixture.html'))) + def test_build_reports_missing_as_failure_and_writes_no_page(self): + s = json.load(open(self.p)); s['steps'][1]['highlight'] = {'selector': '#nope'}; json.dump(s, open(self.p, 'w')) + code, _, err = self.run_cli('build', self.p); self.assertEqual(code, 1); self.assertIn('missing: S-2', err) + self.assertFalse(os.path.exists(os.path.join(self.d, 'fixture.html'))) + def test_writing_rule_error_is_reported(self): + s = json.load(open(self.p)); s['steps'][0]['headline'] = 'Changed the token'; json.dump(s, open(self.p, 'w')) + code, _, err = self.run_cli('build', self.p); self.assertEqual(code, 1); self.assertIn('banned word', err) + def test_wait_reads_the_answers_file(self): + json.dump({'submitted': '2026-08-27T18:40:00Z', 'answers': {}}, open(os.path.join(self.d, 'deck.answers.json'), 'w')) + code, out, _ = self.run_cli('wait', self.p, '--timeout', '1'); self.assertEqual(code, 0); self.assertIn('3 skipped', out) + +if __name__ == '__main__': unittest.main() From 7f56ca39357b29ee89edc2d0d109287c9474edbe Mon Sep 17 00:00:00 2001 From: Destin Date: Thu, 27 Aug 2026 03:01:20 -0700 Subject: [PATCH 19/31] =?UTF-8?q?fix(ui-review):=20deck=20page=20=E2=80=94?= =?UTF-8?q?=20theme=20tokens=20outrank=20the=20page=20defaults=20(Halftone?= =?UTF-8?q?=20radii=20now=20apply),=20ready=20only=20after=20a=20real=20la?= =?UTF-8?q?yout,=20first=20paint=20in=20the=20first=20theme,=20no=20double?= =?UTF-8?q?-counted=20seconds,=20nothing=20moves=20after=20Submit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01W4Kj4r7Vk87Kcb6UrNNp3x --- scripts/ui-review/deck/build.py | 12 ++++++++++-- scripts/ui-review/deck/page.html.tmpl | 2 +- scripts/ui-review/deck/page.js | 14 +++++++++++--- scripts/ui-review/tests/test_build.py | 14 +++++++++++++- 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/scripts/ui-review/deck/build.py b/scripts/ui-review/deck/build.py index 5c67af43..13a91734 100644 --- a/scripts/ui-review/deck/build.py +++ b/scripts/ui-review/deck/build.py @@ -48,7 +48,11 @@ def tokens_css(tokens): lines = [] for t, tok in tokens.items(): decl = ';'.join(f'--{k}:{v}' for k, v in tok.items() if not k.startswith('_')) - lines.append(f'[data-theme="{t}"]{{{decl};color-scheme:{"dark" if tok.get("_dark", True) else "light"}}}') + # Fix: page.css's defaults live on a bare `:root{...}` (specificity 0,1,0) and match too, + # so a plain `[data-theme]` selector (also 0,1,0) tied on specificity — later-in-source page.css + # won — and a community theme's radius (e.g. Halftone's 16px) never applied. `:root[data-theme]` + # is 0,2,0, so the theme tokens always outrank the page defaults regardless of
diff --git a/scripts/ui-review/deck/page.js b/scripts/ui-review/deck/page.js index 70e58931..3600b297 100644 --- a/scripts/ui-review/deck/page.js +++ b/scripts/ui-review/deck/page.js @@ -78,6 +78,7 @@ 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 ── @@ -87,7 +88,10 @@ a.seconds = (a.seconds || 0) + Math.round((Date.now() - stepStart) / 1000); a.theme = theme; a.zoom = zoom; state.answers[id] = a; } - function go(i) { record(); cur = Math.max(0, Math.min(N - 1, i)); state.cur = cur; save(); zoom = 1; stepStart = Date.now(); render(); } + 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(); + } $$('.ans').forEach(b => b.onclick = () => { const id = DECK.steps[cur].id; state.answers[id] = { ...(state.answers[id] || {}), v: b.dataset.v }; paintState(); $('#note').focus(); }); $('#note').addEventListener('input', e => { const id = DECK.steps[cur].id; state.answers[id] = { ...(state.answers[id] || {}), note: e.target.value }; }); $('#save').onclick = () => { if (cur === N - 1) openDialog(); else go(cur + 1); }; @@ -102,6 +106,10 @@ } 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(', ') + ').'; @@ -135,7 +143,7 @@ $('#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 (e.key === 'ArrowRight') go(cur + 1); if (e.key === 'ArrowLeft') go(cur - 1); + 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); } }); @@ -144,6 +152,6 @@ 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(); window.__deckReady = true; // the render test waits for this + stepStart = Date.now(); render(); }); })(); diff --git a/scripts/ui-review/tests/test_build.py b/scripts/ui-review/tests/test_build.py index 45207c04..eb64c1a8 100644 --- a/scripts/ui-review/tests/test_build.py +++ b/scripts/ui-review/tests/test_build.py @@ -1,4 +1,4 @@ -import json, os, sys, tempfile, unittest +import json, os, re, 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 @@ -14,6 +14,7 @@ def test_builds_one_self_describing_page(self): self.assertIn('Fixture review', html); self.assertIn('const DECK=', html) self.assertIn('[data-theme="midnight"]{--canvas:#0D1117', html) # tokens inlined self.assertIn('.chip{', html); self.assertIn("fetch('/answers'", html) # css + js inlined + self.assertIn('', html) # first paint matches the deck's first theme self.assertEqual(warnings, []) def test_deck_data_shape(self): d = deck_data(self.spec, self.boxes) @@ -37,6 +38,17 @@ def test_tokens_for_community_theme_come_from_its_manifest(self): 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_theme_tokens_outrank_the_page_defaults(self): + # page.css's defaults sit on a bare `:root{...}` (specificity 0,1,0); the theme tokens must + # beat that regardless of
@@ -1011,6 +1025,7 @@ figcaption{font:500 11px/1 var(--font);text-transform:uppercase;letter-spacing:. // 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); + window.__deckReady = true; // the render test waits for this — set only once a real layout has been chosen const b = $('#inner .frame .box'); if (b && zoom > 1) b.scrollIntoView({ block: 'center', inline: 'center' }); } @@ -1021,7 +1036,7 @@ figcaption{font:500 11px/1 var(--font);text-transform:uppercase;letter-spacing:. a.seconds = (a.seconds || 0) + Math.round((Date.now() - stepStart) / 1000); a.theme = theme; a.zoom = zoom; state.answers[id] = a; } - function go(i) { record(); cur = Math.max(0, Math.min(N - 1, i)); state.cur = cur; save(); zoom = 1; stepStart = Date.now(); render(); } + function go(i) { if (state.submitted) return; record(); cur = Math.max(0, Math.min(N - 1, i)); state.cur = cur; save(); zoom = 1; stepStart = Date.now(); render(); } // after Submit nothing moves or saves $$('.ans').forEach(b => b.onclick = () => { const id = DECK.steps[cur].id; state.answers[id] = { ...(state.answers[id] || {}), v: b.dataset.v }; paintState(); $('#note').focus(); }); $('#note').addEventListener('input', e => { const id = DECK.steps[cur].id; state.answers[id] = { ...(state.answers[id] || {}), note: e.target.value }; }); $('#save').onclick = () => { if (cur === N - 1) openDialog(); else go(cur + 1); }; @@ -1035,7 +1050,7 @@ figcaption{font:500 11px/1 var(--font);text-transform:uppercase;letter-spacing:. 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(); + record(); stepStart = Date.now(); paintState(); save(); // reset the clock (Keep reviewing → Done must not count twice) and grey the step just marked skip 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(', ') + ').'; @@ -1069,7 +1084,7 @@ figcaption{font:500 11px/1 var(--font);text-transform:uppercase;letter-spacing:. $('#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 (e.key === 'ArrowRight') go(cur + 1); if (e.key === 'ArrowLeft') go(cur - 1); + if (!state.submitted) { if (e.key === 'ArrowRight') go(cur + 1); if (e.key === 'ArrowLeft') go(cur - 1); } 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); } }); @@ -1078,7 +1093,7 @@ figcaption{font:500 11px/1 var(--font);text-transform:uppercase;letter-spacing:. 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(); window.__deckReady = true; // the render test waits for this + stepStart = Date.now(); render(); // __deckReady is set by layout() once the first picture has decoded }); })(); ``` @@ -1133,10 +1148,13 @@ def theme_tokens(themes): def tokens_css(tokens): + # `:root[data-theme=…]` (specificity 0,2,0), not `[data-theme=…]` (0,1,0): page.css declares 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.
+ +
+
+ + + 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." } + ] +} From f821656955803ab0c7c47b963c87d54bb9d384bb Mon Sep 17 00:00:00 2001 From: Destin Date: Thu, 27 Aug 2026 03:22:15 -0700 Subject: [PATCH 26/31] docs(plan): Phase C deck is 10 steps; example copy carries the measured numbers Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01W4Kj4r7Vk87Kcb6UrNNp3x --- .../active/plans/2026-08-27-review-deck-v2.md | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/active/plans/2026-08-27-review-deck-v2.md b/docs/active/plans/2026-08-27-review-deck-v2.md index 8ad9d008..118122f0 100644 --- a/docs/active/plans/2026-08-27-review-deck-v2.md +++ b/docs/active/plans/2026-08-27-review-deck-v2.md @@ -2070,10 +2070,10 @@ The deck Destin reviewed as a v1 page (`phase-c-review.json`, 13 points), rebuil **Constraints this rebuild has, that a fresh review will not:** - **Auto-diff only.** The 2026-08-25 runs predate `measure` (Task 9), and the Before code was master before Phase C merged — it cannot be re-captured, so no step may use `selector`/`text`. Adding `measure` lines to plans is therefore NOT part of this task (it goes into the README rule in Task 15: *measure is planned before the Before run*). -- **Two of the 13 v1 points are Before-only** (`P-21` #2 — the themes rows, and `Q` #1 — the built-in theme editor question) and cannot live in a two-run deck; both were decided on 2026-08-27 already. Leave them out and say so in the commit message. The deck has **11 steps**. +- **Two of the 13 v1 points are Before-only** (`P-21` #2 — the themes rows, and `Q` #1 — the built-in theme editor question) and cannot live in a two-run deck; both were decided on 2026-08-27 already. Leave them out and say so in the commit message. The deck has **10 steps**: `P-1.4` (the explore-empty state) also drops out, because the after run never captured `marketplace-empty` in Meadow Mist and `themes` is deck-wide — the builder refuses rather than fakes it. - If `auto` reports "nothing differs" for a step in some theme (a change that only shows in certain themes), that is a real limit of the pixel diff for an old run: drop that theme from the deck's `themes` only if it affects every step; otherwise leave the step out and note it in the commit — never hand-place a `box` here. -- [ ] **Step 1: Write the v2 spec** — one step per point of `phase-c-review.json`, in its order, with the v2 fields; `"auto"` everywhere. Use exactly this shape for the first three; continue for the remaining eight following the v1 `what`/`fix`/`risk` text rewritten into the four fields under the §5 rules (no banned words; headline ≤ 25 words): +- [ ] **Step 1: Write the v2 spec** — one step per point of `phase-c-review.json`, in its order, with the v2 fields; `"auto"` everywhere. Use exactly this shape for the first three; continue for the remaining seven following the v1 `what`/`fix`/`risk` text rewritten into the four fields under the §5 rules (no banned words; headline ≤ 25 words): ```json { @@ -2092,25 +2092,25 @@ The deck Destin reviewed as a v1 page (`phase-c-review.json`, 13 points), rebuil "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", "1440x330+0+0"], + "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, 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", - "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." }, + "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": "Two fewer things to read under the theme grid; the Marketplace button comes first.", + "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 “Featured” eyebrow alone marks the featured plugin.", - "measured": "1 of 6 border colours remains", + "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": "" } ] @@ -2125,13 +2125,13 @@ Expected: `… crops → …`, `wrote …`, exit 0. A "nothing differs" line → - [ ] **Step 3: Serve** Run: `python3 scripts/ui-review/review-cards.py serve docs/active/design/2026-08-25-ui-audit/phase-c-review-v2.json --no-build` in the background. -Expected: the browser opens on the 11-step deck; Destin's pass (spec §8) — he already saw the tool in Task 13, this is the full-length run: progress bar with 11 segments, skip, submit; the background command exits with the summary. +Expected: the browser opens on the 10-step deck; Destin's pass (spec §8) — he already saw the tool in Task 13, this is the full-length run: progress bar with 10 segments, skip, submit; the background command exits with the summary. - [ ] **Step 4: Commit the spec and the built page** ```bash git add docs/active/design/2026-08-25-ui-audit/phase-c-review-v2.json docs/active/design/2026-08-25-ui-audit/phase-c-review-v2.html -git commit -m "docs(ui-audit): Phase C review rebuilt as the first v2 deck (11 of 13 points — the two Before-only points cannot live in a two-run deck)" +git commit -m "docs(ui-audit): Phase C review rebuilt as the first v2 deck (10 of 13 points — two are Before-only, one lacks a Meadow Mist capture)" ``` --- @@ -2231,6 +2231,6 @@ git commit -m "docs(ui-review): deck v2 flow — build, serve, wait; measure is ## Self-review (2026-08-27, after the review round) - **Spec coverage:** §3 page → Tasks 6, 12; §3.2 embedded/file:// → Task 6 (`embedded`, copy fallback); §3.3 tokens → Tasks 3, 6; §3.4 layout → Task 6 `layout()` + Task 12 rule check; §4.1 spec → Task 1; §4.2 boxes → Tasks 2, 5, 9; §4.3 serve/notify → Tasks 7, 8 (+ `wait`, a fallback the spec did not have — the background process is a single point of failure otherwise); §4.4 file:// → Task 6; §4.5 summary → Task 7 (`summary`) and page (`summary()`), same format; §5 rules → Task 1; §6 gaps 1, 6, 7 → Tasks 10, 11; gap 3 → Tasks 5, 9; gap 5 → Task 15 README; gaps 2, 4 → Task 15 ROADMAP; §7 files → file map; §8 tests → Tasks 1–12; Destin's pass → Tasks 13, 14; §9 rollout → Tasks 14, 16. -- **Deviations from the spec, all deliberate:** no `crop` command (spec §4.3 listed `crop`; a stale intermediate is the v1 bug class); `wait` added; `Square:1` not `Square:3`; coverage orders by run id rather than discarding older runs; the Phase C rebuild is auto-only and 11 steps. +- **Deviations from the spec, all deliberate:** no `crop` command (spec §4.3 listed `crop`; a stale intermediate is the v1 bug class); `wait` added; `Square:1` not `Square:3`; coverage orders by run id rather than discarding older runs; the Phase C rebuild is auto-only and 10 steps. - **Names used across tasks:** `image_name` (5→6), `run_names` (1→5,6,7), `SpecError` (1→6,8), `validate` (1→6,8), `workspace_root` (1→3,6), `crop_images` (5→8,12), `build_page` (6→8), `serve`/`wait_for_submit`/`make_server`/`summary`/`answers_path`/`write_atomic` (7→8,12 test), `measure_key`/`newest_manifest_entry` (5), `document.body.dataset.layout`/`dataset.scores` + `window.__deckReady` (6→12), `entry.measures` / `entry.run` (9→5,10). - **Placeholders:** none; the Phase C spec in Task 14 gives the shape and the first three steps in full and states the rule for the rest (the remaining eight are the v1 points rewritten under §5 — content, not structure). From f3b2027f503dd8dc54e0cf022de9840660c00dbc Mon Sep 17 00:00:00 2001 From: Destin Date: Thu, 27 Aug 2026 03:25:02 -0700 Subject: [PATCH 27/31] =?UTF-8?q?fix(ui-review):=20final-review=20fixes=20?= =?UTF-8?q?=E2=80=94=20answers=20saved=20on=20every=20click,=20a=20failed?= =?UTF-8?q?=20Submit=20says=20so,=20run-review=20never=20orphans=20its=20w?= =?UTF-8?q?orkbench,=20lock=20checked=20before=20build,=20Host/Origin=20gu?= =?UTF-8?q?ard,=20same-size=20guard=20in=20diff=5Fbbox,=20images-folder=20?= =?UTF-8?q?warning?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also: a sweep with zero MISSED rows no longer exits 1 (grep -c under pipefail); __pycache__ ignored; SKILL.md carries the exit-code contract; README documents `labels`; follow-ups filed in ROADMAP. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01W4Kj4r7Vk87Kcb6UrNNp3x --- .claude/skills/ui-review/SKILL.md | 4 +- .gitignore | 4 ++ ROADMAP.md | 1 + scripts/ui-review/README.md | 2 +- scripts/ui-review/deck/boxes.py | 10 ++++- scripts/ui-review/deck/page.js | 22 ++++++++-- scripts/ui-review/deck/serve.py | 63 ++++++++++++++++++--------- scripts/ui-review/deck/spec.py | 6 +++ scripts/ui-review/review-cards.py | 8 +++- scripts/ui-review/run-review.sh | 10 ++++- scripts/ui-review/tests/fixture.py | 2 +- scripts/ui-review/tests/test_boxes.py | 6 +++ scripts/ui-review/tests/test_build.py | 4 +- scripts/ui-review/tests/test_crops.py | 2 +- scripts/ui-review/tests/test_serve.py | 21 +++++++++ scripts/ui-review/tests/test_spec.py | 10 ++++- 16 files changed, 138 insertions(+), 37 deletions(-) diff --git a/.claude/skills/ui-review/SKILL.md b/.claude/skills/ui-review/SKILL.md index 40396275..3901c76b 100644 --- a/.claude/skills/ui-review/SKILL.md +++ b/.claude/skills/ui-review/SKILL.md @@ -69,7 +69,9 @@ then a **review page** — never a gallery, never a chat summary: 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 (`wait ` if you lost the process). Never ask him to paste anything. + 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. diff --git a/.gitignore b/.gitignore index b74aa688..16ba0120 100644 --- a/.gitignore +++ b/.gitignore @@ -84,3 +84,7 @@ docs/active/investigations/harness-eval-runs/** docs/active/design/*-ui-audit/images/ *.answers.json *.serve.json + +# Python bytecode from the deck tooling + its tests +__pycache__/ +*.pyc diff --git a/ROADMAP.md b/ROADMAP.md index 06402294..d7bf19df 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1143,6 +1143,7 @@ surface, not a history. 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/scripts/ui-review/README.md b/scripts/ui-review/README.md index 5c30d3f3..79ecb0cf 100644 --- a/scripts/ui-review/README.md +++ b/scripts/ui-review/README.md @@ -59,7 +59,7 @@ the reason. **A review must quote `coverage.md` and call unverified surfaces "un | `contrast-report.mjs` | aggregates the painted-pixel probe (fg vs *actual* bg) — catches hardcoded colours and translucent surfaces the token audit can't. Over-reports on glass themes; read it, don't paste it. | | `coverage.mjs` | covered / partial / MISSED per surface × theme, with reasons. | | `make-gallery.py` | the HTML gallery. | -| `review-cards.py` + `deck/` + `crops.json` | **the review surface** (v2, 2026-08-27). `build ` cuts 1:1 crops from the run dirs, resolves every highlight box — from the rig's `measure` of a named element, or from the pixel difference between Before and After (the spec never carries coordinates) — and writes the page; it refuses (no page) on a missing picture, an unresolved box, or a broken writing rule. `serve ` builds, serves on 127.0.0.1, opens the browser, saves `.answers.json` on every click and **exits when Destin submits** — run it in the background and its exit is the notification, with the feedback summary on stdout; `wait ` blocks on the answers file alone for a session that no longer holds that process. Spec template: `docs/active/design/2026-08-25-ui-audit/phase-c-review-v2.json`. | +| `review-cards.py` + `deck/` + `crops.json` | **the review surface** (v2, 2026-08-27). `build ` cuts 1:1 crops from the run dirs, resolves every highlight box — from the rig's `measure` of a named element, or from the pixel difference between Before and After (the spec never carries coordinates; an optional `labels` map renames the run captions (`{"before": "Round 1", "after": "Round 2"}`)) — and writes the page; it refuses (no page) on a missing picture, an unresolved box, or a broken writing rule. `serve ` builds, serves on 127.0.0.1, opens the browser, saves `.answers.json` on every click and **exits when Destin submits** — run it in the background and its exit is the notification, with the feedback summary on stdout; `wait ` blocks on the answers file alone for a session that no longer holds that process. Spec template: `docs/active/design/2026-08-25-ui-audit/phase-c-review-v2.json`. | | `review-page.py` | the earlier prose-first review page (Phase A/B pages). Rejected as a review surface on 2026-08-26 — do not use for new phases. | ## Writing a shot diff --git a/scripts/ui-review/deck/boxes.py b/scripts/ui-review/deck/boxes.py index 85c04185..b00b400c 100644 --- a/scripts/ui-review/deck/boxes.py +++ b/scripts/ui-review/deck/boxes.py @@ -49,7 +49,14 @@ def diff_bbox(a, b, threshold='6%', pad=6): edge to shrink from in both cases, so "nothing differs" and "everything differs" stop being the same answer; the box is then shifted +1,+1 by the border, so x and y are corrected back. 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.""" + hairline changes into one region. + Different sizes → None (the step then lands in `missing:` and the deck refuses to build).""" + # WHY the size guard: comparing a Before and an After captured at different window widths + # makes ImageMagick pad the smaller one, so EVERY pixel past the narrower edge reads as + # "changed" — a confident ring drawn in the wrong place. A refusal is the only honest answer. + W, H = image_size(a) + if image_size(b) != (W, H): + return None out = subprocess.run(['magick', a, b, '-compose', 'difference', '-composite', '-threshold', threshold, '-morphology', 'Dilate', 'Square:1', '-bordercolor', 'black', '-border', '1', '-format', '%@', 'info:'], @@ -61,7 +68,6 @@ def diff_bbox(a, b, threshold='6%', pad=6): x, y = x - 1, y - 1 if w * h < 4: return None - 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} diff --git a/scripts/ui-review/deck/page.js b/scripts/ui-review/deck/page.js index 3600b297..26ef3ad4 100644 --- a/scripts/ui-review/deck/page.js +++ b/scripts/ui-review/deck/page.js @@ -24,7 +24,7 @@ } 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; localStorage still has it */ } } + 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 ── @@ -92,8 +92,12 @@ 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(); } - $$('.ans').forEach(b => b.onclick = () => { const id = DECK.steps[cur].id; state.answers[id] = { ...(state.answers[id] || {}), v: b.dataset.v }; paintState(); $('#note').focus(); }); - $('#note').addEventListener('input', e => { const id = DECK.steps[cur].id; state.answers[id] = { ...(state.answers[id] || {}), note: e.target.value }; }); + // 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)); @@ -122,8 +126,18 @@ } $('#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); } + 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(); - try { await fetch('/submit', { method: 'POST', headers: { 'content-type': 'application/json' }, body: JSON.stringify(state) }); } catch (e) { /* server already gone */ } $('#veil').classList.remove('on'); $('#done').textContent = 'Submitted ✓'; $('#done').disabled = true; $$('.ans,#save,#note').forEach(e => e.disabled = true); }; $('#copy').onclick = () => { const t = $('#feedback'); t.select(); (navigator.clipboard ? navigator.clipboard.writeText(t.value) : Promise.reject()).catch(() => document.execCommand('copy')); $('#copy').textContent = 'Copied'; }; diff --git a/scripts/ui-review/deck/serve.py b/scripts/ui-review/deck/serve.py index 446fe1a9..e286d8b0 100644 --- a/scripts/ui-review/deck/serve.py +++ b/scripts/ui-review/deck/serve.py @@ -68,7 +68,18 @@ def _json(self, code, obj): 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.""" + me = f'127.0.0.1:{self.server.server_address[1]}' + origin = self.headers.get('origin') + return (self.headers.get('host') or '') != me or (origin is not None and origin != f'http://{me}') + 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: @@ -77,6 +88,8 @@ def do_GET(self): 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'{}') @@ -109,30 +122,38 @@ def open_url(url): 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 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) - if os.path.exists(lock): - try: - with open(lock) as f: - other = json.load(f) - pid, other_url = other['pid'], other['url'] - except (OSError, ValueError, KeyError): - other = None # unreadable/malformed lock file — treat as stale, proceed - if other is not None: - 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: - pass # stale lock — the pid is dead - except PermissionError: - log(f'REFUSING: {spec["_stem"]} is already served by pid {pid} at {other_url}') - return 3 - else: - log(f'REFUSING: {spec["_stem"]} is already served by pid {pid} at {other_url}') - return 3 + 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 result = {} holder = {} diff --git a/scripts/ui-review/deck/spec.py b/scripts/ui-review/deck/spec.py index c2b853d7..3716629b 100644 --- a/scripts/ui-review/deck/spec.py +++ b/scripts/ui-review/deck/spec.py @@ -113,4 +113,10 @@ def validate(spec): 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/review-cards.py b/scripts/ui-review/review-cards.py index 50ae0e3b..f7ab870b 100644 --- a/scripts/ui-review/review-cards.py +++ b/scripts/ui-review/review-cards.py @@ -19,7 +19,7 @@ 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 serve, wait_for_submit # noqa: E402 +from deck.serve import already_served, serve, wait_for_submit # noqa: E402 from deck.spec import SpecError, load_spec, validate # noqa: E402 @@ -61,6 +61,12 @@ def main(argv): 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) diff --git a/scripts/ui-review/run-review.sh b/scripts/ui-review/run-review.sh index 7b418bd2..e08540f1 100644 --- a/scripts/ui-review/run-review.sh +++ b/scripts/ui-review/run-review.sh @@ -43,6 +43,12 @@ mkdir -p "$OUT/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). @@ -113,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/tests/fixture.py b/scripts/ui-review/tests/fixture.py index fddcf2c2..5c1eb2d2 100644 --- a/scripts/ui-review/tests/fixture.py +++ b/scripts/ui-review/tests/fixture.py @@ -17,7 +17,7 @@ def make_fixture(tmp, themes=('midnight', 'light')): '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', + spec = {'title': 'Fixture review', 'key': 'fixture', 'out': 'fixture.html', 'images': 'images/deck', # images/, the convention validate() warns about breaking 'runs': {'before': os.path.join(tmp, 'runs', 'before'), 'after': os.path.join(tmp, 'runs', 'after')}, 'themes': list(themes), 'crops': {'c': ['main', 'home', GEO]}, 'steps': [ diff --git a/scripts/ui-review/tests/test_boxes.py b/scripts/ui-review/tests/test_boxes.py index 7040ebf4..3f8ccf59 100644 --- a/scripts/ui-review/tests/test_boxes.py +++ b/scripts/ui-review/tests/test_boxes.py @@ -35,6 +35,12 @@ def test_changed_rectangle_is_found_with_padding(self): 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_different_sizes_get_no_box(self): + # A Before and an After captured at different window widths: ImageMagick would pad the + # narrower one and call the whole overhang "changed", drawing a ring in the wrong place. + wide = os.path.join(self.d, 'wide.png'); subprocess.run(['magick', '-size', '210x100', 'xc:#333333', wide], check=True) + self.assertIsNone(diff_bbox(self.a, wide)) + def test_whole_image_change_is_the_whole_image(self): # every pixel differs (no untouched border) — trim can't shrink from an edge without the # 1px border diff_bbox adds; without it this degenerates to the same box as "identical". diff --git a/scripts/ui-review/tests/test_build.py b/scripts/ui-review/tests/test_build.py index 70adc076..6edff67d 100644 --- a/scripts/ui-review/tests/test_build.py +++ b/scripts/ui-review/tests/test_build.py @@ -22,10 +22,10 @@ def test_builds_one_self_describing_page(self): 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]) + s = d['steps'][1]; self.assertEqual(s['images']['light']['after'], 'images/deck/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')) + os.remove(os.path.join(self.spec['_base'], 'images', 'deck', '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): diff --git a/scripts/ui-review/tests/test_crops.py b/scripts/ui-review/tests/test_crops.py index e87c5cf3..69956a6c 100644 --- a/scripts/ui-review/tests/test_crops.py +++ b/scripts/ui-review/tests/test_crops.py @@ -8,7 +8,7 @@ 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') + self.images = os.path.join(self.spec['_base'], 'images', 'deck') 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')))) diff --git a/scripts/ui-review/tests/test_serve.py b/scripts/ui-review/tests/test_serve.py index 2a7f0a38..323f2dc9 100644 --- a/scripts/ui-review/tests/test_serve.py +++ b/scripts/ui-review/tests/test_serve.py @@ -26,6 +26,7 @@ def test_round_trip_and_submit_stops_the_server(self): post(base + '/submit', {'deck': 'fixture', 'answers': {'S-1': {'v': 'yes'}, 'S-2': {'v': 'other', 'note': 'bigger'}}}) t.join(5); self.assertFalse(t.is_alive()) self.assertTrue(got['submitted']); self.assertTrue(json.load(open(answers_path(self.spec)))['submitted']) + srv.server_close() # shutdown() stops serve_forever but leaves the listening socket open — unclosed, it is what the suite warns about at exit def test_summary_format(self): state = {'submitted': '2026-08-27T18:40:00Z', 'answers': {'S-1': {'v': 'yes'}, 'S-2': {'v': 'other', 'note': ' bigger '}}} s = summary(self.spec, state).split('\n') @@ -64,6 +65,26 @@ def test_bad_json_post_gets_a_400(self): self.assertFalse(os.path.exists(answers_path(self.spec))) finally: srv.shutdown(); srv.server_close() + def test_a_foreign_origin_or_host_is_refused(self): + # WHY: the server has no authentication, so the only thing separating Destin's browser + # from a page on evil.example is that the browser tells us where the request came from. + srv, url = make_server(self.spec, 0, lambda state: None) + t = threading.Thread(target=srv.serve_forever, daemon=True); t.start() + try: + base = url.rsplit('/', 1)[0] + req = urllib.request.Request(base + '/submit', data=json.dumps({'answers': {}}).encode(), + headers={'content-type': 'application/json', 'origin': 'http://evil.example'}, method='POST') + with self.assertRaises(urllib.error.HTTPError) as cm: + urllib.request.urlopen(req, timeout=5) + self.assertEqual(cm.exception.code, 403) + self.assertFalse(os.path.exists(answers_path(self.spec))) # a forged submit writes nothing + g = urllib.request.Request(base + '/answers', headers={'host': 'evil.example'}) + with self.assertRaises(urllib.error.HTTPError) as cm2: + urllib.request.urlopen(g, timeout=5) + self.assertEqual(cm2.exception.code, 403) + finally: + srv.shutdown(); srv.server_close() + def test_write_atomic(self): p = os.path.join(self.tmp, 'a.json'); write_atomic(p, {'x': 1}); self.assertEqual(json.load(open(p)), {'x': 1}) def test_wait_returns_0_when_the_file_says_submitted_and_2_on_timeout(self): diff --git a/scripts/ui-review/tests/test_spec.py b/scripts/ui-review/tests/test_spec.py index 477975e0..1f7838bf 100644 --- a/scripts/ui-review/tests/test_spec.py +++ b/scripts/ui-review/tests/test_spec.py @@ -4,7 +4,9 @@ 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"}, + # images/deck names the spec stem ('deck.json'), which is what validate() wants — see + # test_images_folder_must_name_the_deck for the warning when it does not. + spec = {"title": "T", "key": "t", "out": "t.html", "images": "images/deck", "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."}]} @@ -30,6 +32,12 @@ 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_images_folder_must_name_the_deck(self): + s = load_spec(write_spec(self.d, images='images')) + errors, warnings = validate(s) + self.assertEqual(errors, []) + self.assertEqual(len(warnings), 1); self.assertIn('does not contain the spec name "deck"', warnings[0]) + 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)) From 82991e1696e77f61828ffee41753ccbdfd0079a2 Mon Sep 17 00:00:00 2001 From: Destin Date: Thu, 27 Aug 2026 03:25:42 -0700 Subject: [PATCH 28/31] docs(plan): what shipped after the final review, pointed at the code Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01W4Kj4r7Vk87Kcb6UrNNp3x --- docs/active/plans/2026-08-27-review-deck-v2.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/active/plans/2026-08-27-review-deck-v2.md b/docs/active/plans/2026-08-27-review-deck-v2.md index 118122f0..801abfde 100644 --- a/docs/active/plans/2026-08-27-review-deck-v2.md +++ b/docs/active/plans/2026-08-27-review-deck-v2.md @@ -2232,5 +2232,6 @@ git commit -m "docs(ui-review): deck v2 flow — build, serve, wait; measure is - **Spec coverage:** §3 page → Tasks 6, 12; §3.2 embedded/file:// → Task 6 (`embedded`, copy fallback); §3.3 tokens → Tasks 3, 6; §3.4 layout → Task 6 `layout()` + Task 12 rule check; §4.1 spec → Task 1; §4.2 boxes → Tasks 2, 5, 9; §4.3 serve/notify → Tasks 7, 8 (+ `wait`, a fallback the spec did not have — the background process is a single point of failure otherwise); §4.4 file:// → Task 6; §4.5 summary → Task 7 (`summary`) and page (`summary()`), same format; §5 rules → Task 1; §6 gaps 1, 6, 7 → Tasks 10, 11; gap 3 → Tasks 5, 9; gap 5 → Task 15 README; gaps 2, 4 → Task 15 ROADMAP; §7 files → file map; §8 tests → Tasks 1–12; Destin's pass → Tasks 13, 14; §9 rollout → Tasks 14, 16. - **Deviations from the spec, all deliberate:** no `crop` command (spec §4.3 listed `crop`; a stale intermediate is the v1 bug class); `wait` added; `Square:1` not `Square:3`; coverage orders by run id rather than discarding older runs; the Phase C rebuild is auto-only and 10 steps. +- **Shipped after the final whole-branch review (commit 31653b5) — the code is the reference where it and the task bodies above differ:** `page.js` saves on every answer click and (debounced) on note input, not only on Save & Next; a failed Submit keeps the dialog open with the copy box and a specific message instead of "Submitted ✓"; `serve.py` exposes `already_served(spec)` and `review-cards.py serve` checks it BEFORE building (a second serve used to rebuild under the first); the server refuses any request whose `Host` is not its own `127.0.0.1:` or whose `Origin` is foreign (403 — a forged Submit from another local page); `diff_bbox` returns `None` for images of different sizes; `validate` warns when `images` does not contain the spec stem (two decks would overwrite each other's pictures); `run-review.sh` has an EXIT trap so a refused probe or a failed boot check never orphans the workbench, and a zero-MISSED sweep exits 0. Test hygiene follow-ups are a ROADMAP `chore` row dated 2026-08-27. - **Names used across tasks:** `image_name` (5→6), `run_names` (1→5,6,7), `SpecError` (1→6,8), `validate` (1→6,8), `workspace_root` (1→3,6), `crop_images` (5→8,12), `build_page` (6→8), `serve`/`wait_for_submit`/`make_server`/`summary`/`answers_path`/`write_atomic` (7→8,12 test), `measure_key`/`newest_manifest_entry` (5), `document.body.dataset.layout`/`dataset.scores` + `window.__deckReady` (6→12), `entry.measures` / `entry.run` (9→5,10). - **Placeholders:** none; the Phase C spec in Task 14 gives the shape and the first three steps in full and states the rule for the rest (the remaining eight are the v1 points rewritten under §5 — content, not structure). From 1398954086301610a19aeb474db20b5b4d039465 Mon Sep 17 00:00:00 2001 From: Destin Date: Thu, 27 Aug 2026 03:30:43 -0700 Subject: [PATCH 29/31] =?UTF-8?q?fix(ui-review):=20re-serving=20a=20deck?= =?UTF-8?q?=20after=20its=20submit=20starts=20a=20fresh=20review=20?= =?UTF-8?q?=E2=80=94=20the=20old=20answers=20are=20kept=20aside,=20not=20l?= =?UTF-8?q?oaded=20as=20"already=20submitted"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Destin's second run-through ("I can't click through the pages"): the 10-step deck reused the answers file from the 3-step one, the page saw `submitted` and locked navigation. `serve` now moves a submitted answers file to .answers..json before starting, and a page that does load a submitted state shows it as read-only instead of silently ignoring clicks. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01W4Kj4r7Vk87Kcb6UrNNp3x --- .gitignore | 1 + .../phase-c-review-v2.html | 5 +++- scripts/ui-review/deck/page.js | 5 +++- scripts/ui-review/deck/serve.py | 23 +++++++++++++++++++ scripts/ui-review/tests/test_serve.py | 23 ++++++++++++++++++- 5 files changed, 54 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 16ba0120..d9aa0848 100644 --- a/.gitignore +++ b/.gitignore @@ -83,6 +83,7 @@ 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 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 index 9fe3b6fa..16b58c6f 100644 --- 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 @@ -245,8 +245,10 @@ return; // veil stays open — there is still something for him to do here } state.submitted = new Date().toISOString(); - $('#veil').classList.remove('on'); $('#done').textContent = 'Submitted ✓'; $('#done').disabled = true; $$('.ans,#save,#note').forEach(e => e.disabled = true); + $('#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); $('#count').textContent += ' · submitted, read-only'; } $('#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 ── @@ -274,6 +276,7 @@ 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/page.js b/scripts/ui-review/deck/page.js index 26ef3ad4..d056a54c 100644 --- a/scripts/ui-review/deck/page.js +++ b/scripts/ui-review/deck/page.js @@ -138,8 +138,10 @@ return; // veil stays open — there is still something for him to do here } state.submitted = new Date().toISOString(); - $('#veil').classList.remove('on'); $('#done').textContent = 'Submitted ✓'; $('#done').disabled = true; $$('.ans,#save,#note').forEach(e => e.disabled = true); + $('#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); $('#count').textContent += ' · submitted, read-only'; } $('#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 ── @@ -167,5 +169,6 @@ 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 index e286d8b0..fe8900f8 100644 --- a/scripts/ui-review/deck/serve.py +++ b/scripts/ui-review/deck/serve.py @@ -147,6 +147,28 @@ def already_served(spec): 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) @@ -154,6 +176,7 @@ def serve(spec, port=0, open_browser=True, timeout_min=240, log=print): 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 = {} diff --git a/scripts/ui-review/tests/test_serve.py b/scripts/ui-review/tests/test_serve.py index 323f2dc9..342be6a9 100644 --- a/scripts/ui-review/tests/test_serve.py +++ b/scripts/ui-review/tests/test_serve.py @@ -3,7 +3,7 @@ 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.serve import answers_path, make_server, serve, summary, wait_for_submit, write_atomic +from deck.serve import answers_path, make_server, rotate_submitted, serve, summary, wait_for_submit, write_atomic def post(url, obj): req = urllib.request.Request(url, data=json.dumps(obj).encode(), headers={'content-type': 'application/json'}, method='POST') @@ -87,6 +87,27 @@ def test_a_foreign_origin_or_host_is_refused(self): def test_write_atomic(self): p = os.path.join(self.tmp, 'a.json'); write_atomic(p, {'x': 1}); self.assertEqual(json.load(open(p)), {'x': 1}) + def test_a_submitted_answers_file_is_kept_aside_and_the_review_starts_fresh(self): + # Re-serving a deck after its submit must not load the old file: the page would see `submitted` and lock every control. + self.assertIsNone(rotate_submitted(self.spec, log=lambda *a: None)) # no file: nothing to do + write_atomic(answers_path(self.spec), {'answers': {'S-1': {'v': 'yes'}}}) + self.assertIsNone(rotate_submitted(self.spec, log=lambda *a: None)) # saved but not submitted: keep going + self.assertTrue(os.path.exists(answers_path(self.spec))) + write_atomic(answers_path(self.spec), {'submitted': '2026-08-27T10:10:30.568Z', 'answers': {'S-1': {'v': 'yes'}}}) + out = []; dest = rotate_submitted(self.spec, log=out.append) + self.assertEqual(os.path.basename(dest), 'deck.answers.202608271010.json'); self.assertTrue(os.path.exists(dest)) + self.assertFalse(os.path.exists(answers_path(self.spec))); self.assertTrue(any('starting a fresh one' in l for l in out)) + # and serve() does it before it starts (the lock check in test_second_serve_of_same_spec_refuses runs first) + write_atomic(answers_path(self.spec), {'submitted': '2026-08-27T11:00:00Z', 'answers': {}}) + result = {} + def run(): result['code'] = serve(self.spec, port=0, open_browser=False, timeout_min=1, log=out.append) + t = threading.Thread(target=run, daemon=True); t.start() + for _ in range(50): + if any(l.startswith('[deck] http') for l in out): break + time.sleep(0.1) + self.assertFalse(os.path.exists(answers_path(self.spec))); self.assertTrue(os.path.exists(os.path.join(self.spec['_base'], 'deck.answers.202608271100.json'))) + url = next(l for l in out if l.startswith('[deck] http')).split(' ', 1)[1] + post(url.rsplit('/', 1)[0] + '/submit', {'deck': 'fixture', 'answers': {}}); t.join(5); self.assertEqual(result['code'], 0) def test_wait_returns_0_when_the_file_says_submitted_and_2_on_timeout(self): out = [] self.assertEqual(wait_for_submit(self.spec, timeout_min=0.002, poll_s=0.05, log=out.append), 2) # ~0.12 s, no file From 94e3b7521a0b50f1bc38dc62be60365eb27bf45f Mon Sep 17 00:00:00 2001 From: Destin Date: Thu, 27 Aug 2026 03:35:40 -0700 Subject: [PATCH 30/31] fix(ui-review): read-only label survives repaints, a dead server flips the dialog to copy/paste, localhost accepted, SIGTERM cleans the lock Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01W4Kj4r7Vk87Kcb6UrNNp3x --- .../2026-08-25-ui-audit/phase-c-review-v2.html | 6 +++--- scripts/ui-review/deck/page.js | 6 +++--- scripts/ui-review/deck/serve.py | 12 ++++++++++-- 3 files changed, 16 insertions(+), 8 deletions(-) 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 index 16b58c6f..4b6bbbbd 100644 --- 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 @@ -157,7 +157,7 @@ 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'; + $('#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 ›'; } @@ -237,7 +237,7 @@ // 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); } + 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.'; @@ -248,7 +248,7 @@ $('#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); $('#count').textContent += ' · submitted, read-only'; } + 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 ── diff --git a/scripts/ui-review/deck/page.js b/scripts/ui-review/deck/page.js index d056a54c..fc1cafd0 100644 --- a/scripts/ui-review/deck/page.js +++ b/scripts/ui-review/deck/page.js @@ -50,7 +50,7 @@ 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'; + $('#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 ›'; } @@ -130,7 +130,7 @@ // 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); } + 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.'; @@ -141,7 +141,7 @@ $('#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); $('#count').textContent += ' · submitted, read-only'; } + 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 ── diff --git a/scripts/ui-review/deck/serve.py b/scripts/ui-review/deck/serve.py index fe8900f8..655b8d29 100644 --- a/scripts/ui-review/deck/serve.py +++ b/scripts/ui-review/deck/serve.py @@ -7,8 +7,10 @@ import http.server import json import os +import signal import socketserver import subprocess +import sys import threading import time import webbrowser @@ -73,9 +75,11 @@ def _wrong_origin(self): 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.""" - me = f'127.0.0.1:{self.server.server_address[1]}' + 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 '') != me or (origin is not None and origin != f'http://{me}') + 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(): @@ -199,6 +203,10 @@ def on_submit(state): 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: From bfbf2c2d2f456c09f9a264ed22a90a320d798060 Mon Sep 17 00:00:00 2001 From: Destin Date: Thu, 27 Aug 2026 03:36:00 -0700 Subject: [PATCH 31/31] docs: archive the review-deck v2 spec and plan as shipped; pointers follow them Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01W4Kj4r7Vk87Kcb6UrNNp3x --- .../active/handoffs/2026-08-27-review-deck-tooling-handoff.md | 2 +- docs/{active => archive}/plans/2026-08-27-review-deck-v2.md | 4 ++-- .../specs/2026-08-27-review-deck-v2-design.md | 2 +- scripts/ui-review/deck/spec.py | 2 +- scripts/ui-review/review-cards.py | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) rename docs/{active => archive}/plans/2026-08-27-review-deck-v2.md (99%) rename docs/{active => archive}/specs/2026-08-27-review-deck-v2-design.md (99%) 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 38586ce4..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 @@ -3,7 +3,7 @@ status: shipped created: 2026-08-27 --- -Superseded by `docs/active/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. +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) diff --git a/docs/active/plans/2026-08-27-review-deck-v2.md b/docs/archive/plans/2026-08-27-review-deck-v2.md similarity index 99% rename from docs/active/plans/2026-08-27-review-deck-v2.md rename to docs/archive/plans/2026-08-27-review-deck-v2.md index 801abfde..5cc250eb 100644 --- a/docs/active/plans/2026-08-27-review-deck-v2.md +++ b/docs/archive/plans/2026-08-27-review-deck-v2.md @@ -1,7 +1,7 @@ --- -status: active +status: shipped created: 2026-08-27 -spec: docs/active/specs/2026-08-27-review-deck-v2-design.md +spec: docs/archive/specs/2026-08-27-review-deck-v2-design.md --- # Review Deck v2 Implementation Plan diff --git a/docs/active/specs/2026-08-27-review-deck-v2-design.md b/docs/archive/specs/2026-08-27-review-deck-v2-design.md similarity index 99% rename from docs/active/specs/2026-08-27-review-deck-v2-design.md rename to docs/archive/specs/2026-08-27-review-deck-v2-design.md index 2b34dca7..9413cb56 100644 --- a/docs/active/specs/2026-08-27-review-deck-v2-design.md +++ b/docs/archive/specs/2026-08-27-review-deck-v2-design.md @@ -1,5 +1,5 @@ --- -status: draft +status: shipped created: 2026-08-27 owner: Destin (decisions) / Claude (draft) related: diff --git a/scripts/ui-review/deck/spec.py b/scripts/ui-review/deck/spec.py index 3716629b..a7aa0fba 100644 --- a/scripts/ui-review/deck/spec.py +++ b/scripts/ui-review/deck/spec.py @@ -3,7 +3,7 @@ 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.""" +remembered. Spec: docs/archive/specs/2026-08-27-review-deck-v2-design.md §4–5.""" import json import os import re diff --git a/scripts/ui-review/review-cards.py b/scripts/ui-review/review-cards.py index f7ab870b..9da254af 100644 --- a/scripts/ui-review/review-cards.py +++ b/scripts/ui-review/review-cards.py @@ -10,7 +10,7 @@ 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/active/specs/2026-08-27-review-deck-v2-design.md (§4–5; archived after merge). History of the +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