From 693719e85f88ed964beb555ccab7d582f28722a8 Mon Sep 17 00:00:00 2001 From: Ty Date: Sun, 20 Sep 2026 23:36:31 -0700 Subject: [PATCH 01/14] =?UTF-8?q?fix(review):=20#89=20explicit=20metadata?= =?UTF-8?q?=20lock=20=E2=80=94=20deterministic=20copy=20on=20lock,=20not?= =?UTF-8?q?=20on=20review?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Review screen now has an explicit 'Lock & Build Title' step. Until the owner locks, title/short_description stay empty (no 'ready' copy from an unlocked AI draft). On lock the server validates the taxonomy, sets metadata_locked=1, builds + persists the deterministic title/short_description (owner prose wins), and returns the ready state. - schema.sql: add metadata_locked INTEGER DEFAULT 0 to slabs - db/__init__.py: bump _SCHEMA_SENTINELS for metadata_locked - slabs.py: expose metadata_locked in GET; reject it in PUT (server-managed) - review.py: POST /{id}/review action=lock path; regular review reports stored (empty) copy until locked - test_review.py: 8 tests covering lock gate, owner-prose-wins, stale taxonomy - frontend/api/slabs.ts: Slab.metadata_locked, ReviewResult.locked, ReviewInput.action, lockSlab() helper - Review.svelte: lockMetadata() with correct commit-token ordering (bump AFTER inner commit so the inner PUT isn't superseded), Lock button, locked banner - e2e/m3-create.spec.ts: pare to single golden path (capture→review→lock→ ready→publish→done); mock respects optimistic concurrency (client_rev) so stale PUTs 409 like the real backend --- backend/db/__init__.py | 1 + backend/db/schema.sql | 3 + backend/review.py | 116 ++++++++++---- backend/slabs.py | 12 ++ backend/tests/test_review.py | 90 ++++++++++- frontend/src/api/slabs.ts | 13 ++ frontend/src/create/review/Review.svelte | 78 +++++++++- frontend/tests/e2e/m3-create.spec.ts | 186 ++++++----------------- 8 files changed, 329 insertions(+), 170 deletions(-) diff --git a/backend/db/__init__.py b/backend/db/__init__.py index 9b8f6c3..1d43c39 100644 --- a/backend/db/__init__.py +++ b/backend/db/__init__.py @@ -55,6 +55,7 @@ _SCHEMA_SENTINELS = [ ("settings", "value"), ("slabs", "deleted_at"), ("slabs", "server_rev"), + ("slabs", "metadata_locked"), ("slab_photos", "original_blob"), ("sync_log", "payload_hash"), ] diff --git a/backend/db/schema.sql b/backend/db/schema.sql index 7a63c9e..1c4b6ea 100644 --- a/backend/db/schema.sql +++ b/backend/db/schema.sql @@ -35,6 +35,9 @@ CREATE TABLE IF NOT EXISTS slabs ( species_confidence REAL, figure_confidence REAL, grade_confidence REAL, + -- #89: explicit owner lock of the confirmed taxonomy set. Deterministic + -- title/short_description are built and persisted only once this is 1. + metadata_locked INTEGER CHECK(metadata_locked IN (0,1) OR metadata_locked IS NULL), inference_status TEXT CHECK(inference_status IN ('inferring','done','failed') OR inference_status IS NULL), calibration_photo_id TEXT, client_rev INTEGER NOT NULL DEFAULT 1, diff --git a/backend/review.py b/backend/review.py index 02da487..5277c32 100644 --- a/backend/review.py +++ b/backend/review.py @@ -5,6 +5,7 @@ METADATA-80/85/90, SETTINGS-35. import json import re +from datetime import datetime, timezone from typing import Any from fastapi import APIRouter @@ -44,6 +45,10 @@ def _error(code: str, message: str, detail: Any = None) -> dict: return {"error": {"code": code, "message": message, "detail": detail}} +def _now() -> str: + return datetime.now(timezone.utc).replace(microsecond=0, tzinfo=None).isoformat() + + def _split_length(length_in: float) -> tuple[int, float]: ft = int(length_in // 12) rem = round(length_in - ft * 12, 1) @@ -150,15 +155,63 @@ async def review_slab(slab_id: str, payload: dict[str, Any] | None = None) -> di if row is None: return JSONResponse(status_code=404, content=_error("not_found", "Slab not found")) slab = _slab_dict(row) - # Apply any review edits (primary category declaration + prose). - editable = {"primary_wood_category_id", "title", "short_description", "description", "price", "price_source", "sku", "content_source"} - for k in editable: - if k in payload and payload[k] is not None: - if k == "primary_wood_category_id": - continue # not stored as a column; derived - slab[k] = payload[k] + locked = bool(slab.get("metadata_locked")) + + # --- Explicit lock (#89): owner confirms the taxonomy set; server then + # builds AND PERSISTS the deterministic title + short_description. Until + # this action runs, the prose fields stay unset (no "ready" copy from an + # unlocked AI draft). Owner prose (title/short_description) always wins. + if payload.get("action") == "lock": + if not slab.get("species_id"): + return JSONResponse(status_code=422, content=_error( + "not_ready", "Select a species before locking the metadata")) + primary_id = payload.get("primary_wood_category_id") + wood_cats = slab.get("wood_category_ids") or [] + if not primary_id: + primary_id = wood_cats[0] if wood_cats else None + if not primary_id: + return JSONResponse(status_code=422, content=_error( + "not_ready", "Select a wood category before locking the metadata")) + cats, terms = _taxonomy_lookup(conn) + primary_leaf = cats.get(primary_id, {}) + if not primary_leaf: + return JSONResponse(status_code=422, content=_error( + "not_ready", "Wood category is no longer in the current taxonomy")) + species_name = cats.get(slab.get("species_id"), {}).get("name", "") + figure_term_ids = slab.get("figure_term_ids") or [] + figure = terms.get(figure_term_ids[0], {}).get("name") if figure_term_ids else None + + owner_title = (payload.get("title") or slab.get("title") or "").strip() + owner_short = (payload.get("short_description") or slab.get("short_description") or "").strip() + title = owner_title or build_title(slab, figure, species_name, primary_leaf) + short_desc = owner_short or build_short_description(slab, figure, species_name, primary_leaf) + + now = _now() + cur = conn.execute( + "UPDATE slabs SET title=?, short_description=?, metadata_locked=1, server_rev=server_rev+1, updated_at=? WHERE id=?", + (title, short_desc, now, slab_id), + ) + conn.commit() + if cur.rowcount == 0: + return JSONResponse(status_code=409, content=_error("not_found", "Slab not found")) + slab["title"] = title + slab["short_description"] = short_desc + slab["metadata_locked"] = 1 + + price = _resolve_price(conn, slab, payload) + ready, missing = compute_ready({**slab, "price": price}) + return { + "ready": ready, "missing": missing, "title": title, + "short_description": short_desc, "price": price, + "price_suggestion": price_suggestion(conn, slab), + "primary_wood_category_id": primary_id, "locked": True, + } + + # --- Regular review (no lock): report ready/missing + suggested prose. + # #89: deterministic title/short_description are NOT set until an explicit + # lock; report the stored values (which stay empty until locked) so the UI + # shows them as pending, and never as "ready" from an unlocked AI draft. cats, terms = _taxonomy_lookup(conn) - # Primary wood category: declared, else first wood_category_ids, else first category. primary_id = payload.get("primary_wood_category_id") wood_cats = slab.get("wood_category_ids") or [] if not primary_id: @@ -168,28 +221,29 @@ async def review_slab(slab_id: str, payload: dict[str, Any] | None = None) -> di figure_term_ids = slab.get("figure_term_ids") or [] figure = terms.get(figure_term_ids[0], {}).get("name") if figure_term_ids else None - # Deterministic copy (only if not already overridden by owner). - if not payload.get("title"): - if primary_leaf and species_name: - slab["title"] = build_title(slab, figure, species_name, primary_leaf) - if not payload.get("short_description"): - if primary_leaf and species_name: - slab["short_description"] = build_short_description(slab, figure, species_name, primary_leaf) - # Price suggestion (REVIEW-25): bdft * species rate, empty if no rate; owner override sticks. + if not locked: + title = slab.get("title") or None + short_desc = slab.get("short_description") or None + else: + # Locked: re-derive only if the owner has not overridden the prose. + title = slab.get("title") or build_title(slab, figure, species_name, primary_leaf) + short_desc = slab.get("short_description") or build_short_description(slab, figure, species_name, primary_leaf) + + price = _resolve_price(conn, slab, payload) + ready, missing = compute_ready({**slab, "price": price, "title": title, "short_description": short_desc}) + return { + "ready": ready, "missing": missing, "title": title, "short_description": short_desc, + "price": price, "price_suggestion": price_suggestion(conn, slab), + "primary_wood_category_id": primary_id, "locked": locked, + } + + +def _resolve_price(conn, slab: dict, payload: dict) -> float | None: + """REVIEW-25: bdft * species rate as the suggestion; owner override sticks, + else the stored price, else the suggestion (None if no rate).""" suggested = price_suggestion(conn, slab) if payload.get("price") is not None: - price = payload["price"] - elif slab.get("price") not in (None, 0): - price = slab["price"] - else: - price = suggested - ready, missing = compute_ready({**slab, "price": price}) - return { - "ready": ready, - "missing": missing, - "title": slab.get("title"), - "short_description": slab.get("short_description"), - "price": price, - "price_suggestion": suggested, - "primary_wood_category_id": primary_id, - } + return payload["price"] + if slab.get("price") not in (None, 0): + return slab["price"] + return suggested diff --git a/backend/slabs.py b/backend/slabs.py index 4669a64..81d7d89 100644 --- a/backend/slabs.py +++ b/backend/slabs.py @@ -64,6 +64,11 @@ def _encode(payload: Dict[str, Any]) -> Dict[str, Any]: return out +# #89: server-managed — only the explicit review lock action may set it. A plain +# PUT must not flip the lock (the deterministic copy is built inside that action). +CLIENT_FORBIDDEN_FIELDS = {"metadata_locked"} + + def _row_to_slab(row) -> Dict[str, Any]: d = dict(row) for f in JSON_ARRAY_FIELDS: @@ -72,6 +77,8 @@ def _row_to_slab(row) -> Dict[str, Any]: d[f] = json.loads(d[f]) except (ValueError, TypeError): pass + # #89: expose the lock flag to the client (0/None → False, 1 → True). + d["metadata_locked"] = bool(d.get("metadata_locked")) return d @@ -155,6 +162,11 @@ async def get_slab(id: str) -> Dict[str, Any]: async def update_slab(id: str, payload: Dict[str, Any]) -> Dict[str, Any]: if "client_rev" not in payload: return _err("validation", "client_rev required", status=400) + # #89: the metadata lock is server-managed via the explicit review lock + # action; a plain PUT may not flip it. + for field in CLIENT_FORBIDDEN_FIELDS: + if field in payload: + return _err("validation", f"{field} is set by the review lock action, not by update", status=400) conn = get_connection() current = conn.execute("SELECT * FROM slabs WHERE id=? AND deleted_at IS NULL", (id,)).fetchone() if current is None: diff --git a/backend/tests/test_review.py b/backend/tests/test_review.py index bc430e2..cc1f690 100644 --- a/backend/tests/test_review.py +++ b/backend/tests/test_review.py @@ -55,12 +55,19 @@ def test_title_live_edge_no_dims(env): conn.execute("INSERT INTO slabs(id,sku,length_in,thickness_in,sqft,bdft,width_min_in,width_max_in,width_avg_in,species_id,wood_category_ids,figure_term_ids,price,price_source,title,short_description,description,content_source,status,server_rev,client_rev,created_at,updated_at) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)", ("s1","SLAB-01",114.0,1.75,30.0,18.4,18.0,24.3,24.3,1,json.dumps([2]),json.dumps([1]),None,None,None,None,"A slab","manual","calibrated",1,1,"t","t")) conn.commit() - r = env[0].post("/api/v1/slabs/s1/review", json={}) + # #89: title/short_description are built AND persisted only after an explicit lock. + r = env[0].post("/api/v1/slabs/s1/review", json={"action": "lock"}) assert r.status_code == 200, r.text body = r.json() + assert body["locked"] is True assert body["title"] == "Figured Black Walnut Live Edge Slab" assert body["short_description"].startswith("Figured Black Walnut Live Edge Slab, 9ft 6in x 24.3in x 1.75in") assert "18.4 bdft" in body["short_description"] + # GET shows the stored (persisted) copy — it matches the deterministic pattern. + got = env[0].get("/api/v1/slabs/s1").json() + assert got["title"] == "Figured Black Walnut Live Edge Slab" + assert got["short_description"].startswith("Figured Black Walnut Live Edge Slab,") + assert got["metadata_locked"] is True def test_ready_requires_full_set(env): @@ -134,5 +141,84 @@ def test_knife_blank_carries_dims(env): conn.execute("INSERT INTO slabs(id,sku,length_in,thickness_in,sqft,bdft,width_min_in,width_max_in,width_avg_in,species_id,wood_category_ids,figure_term_ids,price,price_source,title,short_description,description,content_source,status,server_rev,client_rev,created_at,updated_at) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)", ("s1","SLAB-01",12.0,1.5,6.0,0.7,6.0,6.5,6.5,1,json.dumps([3]),json.dumps([1]),None,None,None,None,"A slab","manual","calibrated",1,1,"t","t")) conn.commit() - r = client.post("/api/v1/slabs/s1/review", json={}) + # #89: build happens on lock. + r = client.post("/api/v1/slabs/s1/review", json={"action": "lock"}) assert r.json()["title"] == "Figured Black Walnut Knife Blank (12 x 6.5 x 1.5 in)" + + +# --- #89: explicit lock gate for deterministic title/short_description --- + +def test_review_before_lock_reports_no_title(env): + """#89 AC: until lock, title/short_description are not set (no 'ready' copy + from an unlocked AI draft). A plain review reports the stored (empty) values.""" + client, conn = env + conn.execute("INSERT INTO slabs(id,sku,length_in,thickness_in,sqft,bdft,width_min_in,width_max_in,width_avg_in,species_id,wood_category_ids,figure_term_ids,price,price_source,title,short_description,description,content_source,status,server_rev,client_rev,created_at,updated_at) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)", + ("s1","SLAB-01",114.0,1.75,30.0,18.4,18.0,24.3,24.3,1,json.dumps([2]),json.dumps([1]),None,None,None,None,"A slab","manual","calibrated",1,1,"t","t")) + conn.commit() + r = client.post("/api/v1/slabs/s1/review", json={}) + body = r.json() + assert body["locked"] is False + assert body["title"] is None + assert body["short_description"] is None + # title/short_description are in the missing set (not ready until locked). + assert "title" in body["missing"] and "short_description" in body["missing"] + # Nothing was persisted: GET still shows empty copy. + assert client.get("/api/v1/slabs/s1").json()["title"] is None + + +def test_lock_persists_title_and_get_matches(env): + """#89 AC: after lock with species + primary wood category, title/short_desc + are populated deterministically, stored, and GET reflects them.""" + client, conn = env + conn.execute("INSERT INTO slabs(id,sku,length_in,thickness_in,sqft,bdft,width_min_in,width_max_in,width_avg_in,species_id,wood_category_ids,figure_term_ids,price,price_source,title,short_description,description,content_source,status,server_rev,client_rev,created_at,updated_at) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)", + ("s1","SLAB-01",114.0,1.75,30.0,18.4,18.0,24.3,24.3,1,json.dumps([2]),json.dumps([1]),None,None,None,None,"A slab","manual","calibrated",1,1,"t","t")) + conn.commit() + r = client.post("/api/v1/slabs/s1/review", json={"action": "lock"}) + assert r.status_code == 200 + body = r.json() + assert body["locked"] is True + assert body["title"] == "Figured Black Walnut Live Edge Slab" + got = client.get("/api/v1/slabs/s1").json() + assert got["title"] == body["title"] + assert got["short_description"] == body["short_description"] + assert got["metadata_locked"] is True + + +def test_lock_requires_species(env): + client, conn = env + # No species_id -> lock is rejected with a plain message. + conn.execute("INSERT INTO slabs(id,sku,length_in,thickness_in,sqft,bdft,width_min_in,width_max_in,width_avg_in,species_id,wood_category_ids,price,title,short_description,status,server_rev,client_rev,created_at,updated_at) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)", + ("s1","SLAB-01",114.0,1.75,30.0,18.4,18.0,24.3,24.3,None,json.dumps([2]),None,None,None,"calibrated",1,1,"t","t")) + conn.commit() + r = client.post("/api/v1/slabs/s1/review", json={"action": "lock"}) + assert r.status_code == 422 + assert r.json()["error"]["code"] == "not_ready" + + +def test_lock_requires_wood_category(env): + client, conn = env + # species present but no wood category -> rejected. + conn.execute("INSERT INTO slabs(id,sku,length_in,thickness_in,sqft,bdft,width_min_in,width_max_in,width_avg_in,species_id,wood_category_ids,price,title,short_description,status,server_rev,client_rev,created_at,updated_at) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)", + ("s1","SLAB-01",114.0,1.75,30.0,18.4,18.0,24.3,24.3,1,json.dumps([]),None,None,None,"calibrated",1,1,"t","t")) + conn.commit() + r = client.post("/api/v1/slabs/s1/review", json={"action": "lock"}) + assert r.status_code == 422 + assert r.json()["error"]["code"] == "not_ready" + + +def test_lock_owner_prose_wins(env): + # METADATA-50: typed owner title/short_description win over the pattern. + client, conn = env + conn.execute("INSERT INTO slabs(id,sku,length_in,thickness_in,sqft,bdft,width_min_in,width_max_in,width_avg_in,species_id,wood_category_ids,figure_term_ids,price,price_source,title,short_description,description,content_source,status,server_rev,client_rev,created_at,updated_at) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)", + ("s1","SLAB-01",114.0,1.75,30.0,18.4,18.0,24.3,24.3,1,json.dumps([2]),json.dumps([1]),None,None,None,None,"A slab","manual","calibrated",1,1,"t","t")) + conn.commit() + r = client.post("/api/v1/slabs/s1/review", json={ + "action": "lock", + "title": "My Handcrafted Walnut Slab", + "short_description": "Owner-authored short description.", + }) + body = r.json() + assert body["title"] == "My Handcrafted Walnut Slab" + assert body["short_description"] == "Owner-authored short description." + got = client.get("/api/v1/slabs/s1").json() + assert got["title"] == "My Handcrafted Walnut Slab" diff --git a/frontend/src/api/slabs.ts b/frontend/src/api/slabs.ts index 66c275e..757da0d 100644 --- a/frontend/src/api/slabs.ts +++ b/frontend/src/api/slabs.ts @@ -39,6 +39,8 @@ export interface Slab { short_description: string | null; description: string | null; content_source: string | null; + // #89: set true once the owner has locked the taxonomy (deterministic copy built). + metadata_locked: boolean; species_confidence: number | null; figure_confidence: number | null; grade_confidence: number | null; @@ -220,6 +222,8 @@ export interface ReviewResult { price: number | null; price_suggestion: number | null; primary_wood_category_id: number | null; + // #89: whether the metadata is locked (deterministic copy built/persisted). + locked: boolean; } export interface ReviewInput { @@ -231,6 +235,8 @@ export interface ReviewInput { description?: string; price_source?: string; content_source?: string; + // #89: 'lock' triggers the explicit metadata-lock + deterministic copy build. + action?: 'lock'; } export function reviewSlab(slabId: string, payload: ReviewInput = {}): Promise { @@ -241,6 +247,13 @@ export function reviewSlab(slabId: string, payload: ReviewInput = {}): Promise parse(r)); } +// #89: explicit lock of the confirmed taxonomy. The server builds and persists +// the deterministic title/short_description from the locked set (owner prose +// already on the slab wins). Distinct from reviewSlab() which only reports. +export function lockSlab(slabId: string, payload: ReviewInput = {}): Promise { + return reviewSlab(slabId, { ...payload, action: 'lock' }); +} + // --------------------------------------------------------------------------- // Publish (PUBLISH-*) // --------------------------------------------------------------------------- diff --git a/frontend/src/create/review/Review.svelte b/frontend/src/create/review/Review.svelte index f8f3cc2..4e38e3a 100644 --- a/frontend/src/create/review/Review.svelte +++ b/frontend/src/create/review/Review.svelte @@ -1,7 +1,7 @@ {#if loading} @@ -365,6 +499,47 @@
Sqft / Bdft
{fmt2(capture?.sqft ?? slab?.sqft ?? 0)} / {fmt2(capture?.bdft ?? slab?.bdft ?? 0)}
+ + {#if photosLoaded && photos.length} +
+

Tap a photo to keep or remove it · drag to reorder · leftmost photo = primary

+
+ {#each photos as p, i (p.id)} +
toggleKeep(p.id)} + on:keydown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); toggleKeep(p.id); } }} + on:dragstart={(e) => onDragStart(e, i)} + on:dragover={onDragOver} + on:drop={(e) => onDrop(e, i)} + > + {roleLabel(p.role)} + {#if p.id === primaryId} + PRIMARY + {/if} + + +
+ {/each} +
+ {#if lineupMissing.length} + + {/if} + {#if persistError}{/if} +
+ {/if} + {#if settings?.inference_enabled}
@@ -594,4 +772,23 @@ .primary:disabled { background: #333; color: #777; } .ghost { padding: 14px 18px; border-radius: 3px; border: 1px solid #444; background: transparent; color: #ccc; } .err { color: #e88; } .busy { color: #aaa; } + + /* #90 Photo strip */ + .photos { margin: 14px 0 4px; } + .ph-hint { color: #8a8a8a; font-size: .72rem; margin: 0 0 8px; } + .ph-grid { display: grid; grid-template-columns: repeat(3, 1fr); gap: 8px; } + @media (max-width: 420px) { .ph-grid { grid-template-columns: repeat(2, 1fr); } } + .pcard { position: relative; aspect-ratio: 1; border: 2px solid #2a2a2a; border-radius: 3px; overflow: hidden; cursor: grab; background: #141414; } + .pcard:active { cursor: grabbing; } + .pcard:focus-visible { outline: 2px solid #00e5ff; outline-offset: 2px; } + .pcard.kept { border-color: #444; } + .pcard.primary { border-color: #00e5ff; } + .pcard:not(.kept) { opacity: .4; } + .ph-thumb { width: 100%; height: 100%; object-fit: cover; display: block; pointer-events: none; } + .ph-pri { position: absolute; top: 6px; right: 6px; background: #00e5ff; color: #002; font-size: .56rem; font-weight: 800; padding: 1px 5px; border-radius: 2px; letter-spacing: .03em; pointer-events: none; } + .ph-seq { position: absolute; bottom: 6px; left: 6px; width: 18px; height: 18px; border-radius: 50%; background: rgba(0,0,0,.7); color: #fff; font-size: .66rem; display: flex; align-items: center; justify-content: center; pointer-events: none; } + .ph-keep { position: absolute; top: 6px; left: 6px; width: 16px; height: 16px; border: 2px solid #fff; border-radius: 3px; background: rgba(0,0,0,.5); pointer-events: none; } + .pcard.kept .ph-keep { background: #00e5ff; border-color: #00e5ff; } + .pcard.kept .ph-keep::after { content: ''; position: absolute; left: 4px; top: 1px; width: 4px; height: 8px; border: solid #002; border-width: 0 2px 2px 0; transform: rotate(45deg); } + .ph-nudge { color: #e8a; font-size: .74rem; margin: 8px 0 0; } diff --git a/frontend/src/create/review/lineup.test.ts b/frontend/src/create/review/lineup.test.ts new file mode 100644 index 0000000..7899d6b --- /dev/null +++ b/frontend/src/create/review/lineup.test.ts @@ -0,0 +1,71 @@ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { + keptInOrder, + moveItem, + missingMinimums, + lineupValid, + roleLabel, + type LineupItem, +} from './lineup'; + +const p = (id: string, role: LineupItem['role'], kept = true): LineupItem => ({ + id, + role, + isCalibration: role === 'calibration', + kept, +}); + +test('lineup: keptInOrder returns kept photos in their current order', () => { + const items = [p('cal', 'calibration'), p('ov', 'overlay', false), p('b1', 'batch')]; + assert.deepEqual(keptInOrder(items).map((x) => x.id), ['cal', 'b1']); +}); + +test('lineup: leftmost kept is the primary', () => { + const items = [p('b1', 'batch'), p('cal', 'calibration'), p('ov', 'overlay')]; + // After a drag the owner re-ordered; primary follows position, not role. + assert.equal(keptInOrder(items)[0].id, 'b1'); +}); + +test('lineup: primary auto-promotes when the leftmost kept is removed', () => { + const items = [p('cal', 'calibration', false), p('ov', 'overlay'), p('b1', 'batch')]; + assert.equal(keptInOrder(items)[0].id, 'ov'); +}); + +test('lineup: moveItem reorders (drag-to-front) and is a no-op on same index', () => { + const a = ['x', 'y', 'z']; + assert.deepEqual(moveItem(a, 2, 0), ['z', 'x', 'y']); + assert.deepEqual(moveItem(a, 0, 0), ['x', 'y', 'z']); + // Out-of-range is a no-op, never throws. + assert.deepEqual(moveItem(a, 0, 5), ['x', 'y', 'z']); + // Returns a new array, does not mutate the input. + const before = a.slice(); + moveItem(a, 0, 2); + assert.deepEqual(a, before); +}); + +test('lineup: minimum requires a clean AND an overlay copy (REVIEW-5)', () => { + assert.deepEqual(missingMinimums([p('cal', 'calibration'), p('ov', 'overlay')]), []); + assert.deepEqual(missingMinimums([p('cal', 'calibration')]), ['overlay']); + assert.deepEqual(missingMinimums([p('ov', 'overlay')]), ['calibration']); + assert.deepEqual(missingMinimums([p('b1', 'batch')]), ['calibration', 'overlay']); + assert.deepEqual(missingMinimums([]), ['calibration', 'overlay']); +}); + +test('lineup: batch-only is not valid; cal+overlay+batch is', () => { + assert.equal(lineupValid([p('b1', 'batch')]), false); + assert.equal(lineupValid([p('cal', 'calibration'), p('ov', 'overlay'), p('b1', 'batch')]), true); +}); + +test('lineup: missingMinimums only counts KEPT photos', () => { + // Both roles present in the array, but the cal is not kept. + assert.deepEqual( + missingMinimums([p('cal', 'calibration', false), p('ov', 'overlay')]), + ['calibration'], + ); +}); + +test('lineup: roleLabel is owner-facing', () => { + assert.match(roleLabel('calibration'), /clean/); + assert.match(roleLabel('overlay'), /overlay/); +}); diff --git a/frontend/src/create/review/lineup.ts b/frontend/src/create/review/lineup.ts new file mode 100644 index 0000000..b4fd348 --- /dev/null +++ b/frontend/src/create/review/lineup.ts @@ -0,0 +1,62 @@ +// #90 Review photo strip (REVIEW-5 / PUBLISH-45). Pure logic split out so the +// keep/remove + reorder + minimum enforcement is unit-testable without a browser. +// +// Model (approved in the wireframe): the owner taps a photo to keep/remove it, +// drags to reorder, and the LEFTMOST kept photo is the primary (Woo cover). The +// submission-ready minimum is at least one clean (calibration) photo AND one +// dimension-overlay copy (REVIEW-5 "clean first pic plus its dimension-overlay +// copy"). Batch photos are optional. + +export type PhotoRole = 'calibration' | 'overlay' | 'batch'; + +export interface LineupItem { + id: string; + role: PhotoRole; + isCalibration: boolean; + /** True = the owner keeps this photo in the submission. */ + kept: boolean; +} + +/** + * The kept photos in their current (left→right) order. The first kept photo is + * the primary (Woo cover). Reordering is a plain array move; this helper just + * exposes the kept subset so the caller can re-seq on persist. + */ +export function keptInOrder(items: LineupItem[]): LineupItem[] { + return items.filter((it) => it.kept); +} + +/** Move an item from one index to another (drag-reorder). Returns a new array. */ +export function moveItem(arr: T[], from: number, to: number): T[] { + if (from === to || from < 0 || to < 0 || from >= arr.length || to >= arr.length) { + return arr; + } + const next = arr.slice(); + const [item] = next.splice(from, 1); + next.splice(to, 0, item); + return next; +} + +/** + * Whether the kept lineup meets the submission minimum (REVIEW-5): at least one + * clean calibration photo and one dimension-overlay copy. Returns the missing + * role(s) so the UI can name them in a nudge. + */ +export function missingMinimums(kept: LineupItem[]): PhotoRole[] { + const missing: PhotoRole[] = []; + if (!kept.some((it) => it.role === 'calibration')) missing.push('calibration'); + if (!kept.some((it) => it.role === 'overlay')) missing.push('overlay'); + return missing; +} + +/** True when the kept lineup is publishable from a photo standpoint. */ +export function lineupValid(kept: LineupItem[]): boolean { + return missingMinimums(kept).length === 0; +} + +/** A short owner-facing label for a role (used in the minimum nudge). */ +export function roleLabel(role: PhotoRole): string { + if (role === 'calibration') return 'a clean photo'; + if (role === 'overlay') return 'its dimension-overlay copy'; + return 'a batch photo'; +} diff --git a/frontend/tests/e2e/review-photo-strip.spec.ts b/frontend/tests/e2e/review-photo-strip.spec.ts new file mode 100644 index 0000000..6b8b71a --- /dev/null +++ b/frontend/tests/e2e/review-photo-strip.spec.ts @@ -0,0 +1,266 @@ +import { test, expect, type Page, type Route } from '@playwright/test'; +import { seedMeasureDraft } from './helpers/draft'; + +/** + * M3 Review photo strip E2E (issue #90, REVIEW-5 / PUBLISH-45). Drives the + * capture→review flow (mocked API, no live backend) and exercises the photo + * strip: render from stored photos, keep/remove (whole-card tap), drag-reorder, + * leftmost-kept = primary, the clean+overlay minimum, and persistLineup on + * "Continue to Publish". All ids are synthetic fixtures (QUALITY-105). + */ + +const SLAB_ID = '90e00000-0000-4000-8000-000000000090'; +const SPECIES_ID = 11; +const WCAT_ID = 20; +const LIVEEDGE_ID = 21; + +const TAXONOMY = { + categories: [ + { id: 10, name: 'Species', slug: 'species', parent_id: null }, + { id: SPECIES_ID, name: 'Cedar', slug: 'cedar', parent_id: 10 }, + { id: WCAT_ID, name: 'Wood category', slug: 'wcat', parent_id: null }, + { id: LIVEEDGE_ID, name: 'Live edge', slug: 'liveedge', parent_id: WCAT_ID }, + ], + attributes: [ + { id: 30, name: 'Edge Type', slug: 'edge_type', terms: [{ id: 31, name: 'Live edge', slug: 'live-edge' }] }, + ], + tags: [], + taxonomy_anchor: 'fixture', + taxonomy_last_sync_at: '2026-01-01T00:00:00', +}; + +// The stored lineup: clean cal (seq 1), dimension-overlay (seq 2), batch (seq 3). +const PHOTO_ROWS = [ + { id: 'ph-cal', seq: 1, is_calibration: 1, role: 'calibration', width_px: null, height_px: null, has_original: 1, has_processed: 1 }, + { id: 'ph-ov', seq: 2, is_calibration: 0, role: 'overlay', width_px: null, height_px: null, has_original: 1, has_processed: 1 }, + { id: 'ph-b1', seq: 3, is_calibration: 0, role: 'batch', width_px: null, height_px: null, has_original: 1, has_processed: 1 }, +]; + +function baseSlab(): Record { + return { + id: SLAB_ID, sku: '', status: 'draft', price: null, price_source: 'recommendation', + length_in: 48, thickness_in: 2, sqft: 6.25, bdft: 12.5, width_min_in: 10, width_max_in: 12, width_avg_in: 11, + species_id: null, wood_category_ids: [], edge_type_term_id: null, figure_term_ids: [], grade_term_id: null, + thickness_term_id: null, moisture_term_id: null, fig_tag_ids: [], feat_tag_ids: [], + title: null, short_description: null, description: null, content_source: 'manual', + metadata_locked: false, species_confidence: null, figure_confidence: null, grade_confidence: null, + inference_status: null, calibration_photo_id: null, woo_product_id: null, server_rev: 0, + created_at: '2026-01-01T00:00:00Z', updated_at: '2026-01-01T00:00:00Z', + }; +} + +async function json(route: Route, body: unknown, status = 200) { + await route.fulfill({ status, contentType: 'application/json', body: JSON.stringify(body) }); +} + +interface PersistRecord { + seq: number; role: string; is_calibration: string; +} + +async function mockApi(page: Page) { + const slab = baseSlab(); + const photoStore = new Map(); + for (const r of PHOTO_ROWS) photoStore.set(r.id, { ...r }); + const persistCalls: { photoId: string; seq: number; role: string }[] = []; + const deletes: string[] = []; + + const merge = (payload: Record) => { + if (typeof payload.client_rev === 'number' && payload.client_rev !== slab.server_rev) return false; + for (const k of ['sku', 'species_id', 'wood_category_ids', 'title', 'short_description', 'price', 'metadata_locked', 'edge_type_term_id']) { + if (k in payload) (slab as Record)[k] = payload[k]; + } + if (typeof payload.client_rev === 'number') slab.server_rev = payload.client_rev + 1; + }; + + const reviewResult = (payload: Record) => { + if (typeof payload.price === 'number') slab.price = payload.price; + if (typeof payload.sku === 'string' && payload.sku) slab.sku = payload.sku; + const missing: string[] = []; + if (!slab.sku) missing.push('sku'); + if (!slab.species_id) missing.push('species'); + if (!Array.isArray(slab.wood_category_ids) || slab.wood_category_ids.length === 0) missing.push('wood categories'); + if (!slab.edge_type_term_id) missing.push('edge type'); + if (!slab.title) missing.push('title'); + if (!slab.short_description) missing.push('short description'); + if (slab.price == null) missing.push('price'); + // Deterministic copy from the server on lock (REVIEW-15) — set before ready. + if (!slab.title) slab.title = 'Cedar · 48″ Live Edge'; + if (!slab.short_description) slab.short_description = 'Hand-cut slab, ready to ship.'; + return { + ready: missing.length === 0, missing, + title: (slab.title as string) ?? null, + short_description: (slab.short_description as string) ?? null, + price: (slab.price as number) ?? null, price_suggestion: 120, + primary_wood_category_id: null, locked: slab.metadata_locked === true, + }; + }; + + await page.route('**/api/v1/**', async (route) => { + const req = route.request(); + const url = new URL(req.url()); + const path = url.pathname; + const method = req.method(); + const payload = (() => { try { return req.postDataJSON() as Record; } catch { return {}; } })(); + + if (path === '/api/v1/settings' && method === 'GET') { + return json(route, { woo_base_url: 'https://example.invalid', woo_wp_username: 'wp', woo_app_password: { configured: true }, inference_base_url: { configured: false }, inference_api_key: { configured: false }, inference_model: null, inference_enabled: false, content_llm_enabled: false, brand_voice: null, geo_context: null, woo_create_status: 'draft', taxonomy_last_sync_at: '2026-01-01T00:00:00', taxonomy_anchor: 'fixture' }); + } + if (path === '/api/v1/admin/taxonomy' && method === 'GET') return json(route, TAXONOMY); + if (path === '/api/v1/slabs' && method === 'POST') { + slab.id = String(payload.id ?? SLAB_ID); + merge(payload); + return json(route, slab, 201); + } + if (path === `/api/v1/slabs/${slab.id}` && method === 'PUT') { + if (merge(payload) === false) return json(route, { error: { code: 'revision_conflict', message: 'rev', detail: {} } }, 409); + return json(route, slab); + } + if (path === `/api/v1/slabs/${slab.id}` && method === 'GET') return json(route, slab); + + // --- Photos (the strip) --- + if (path === `/api/v1/slabs/${slab.id}/photos` && method === 'GET') { + return json(route, [...photoStore.values()].sort((a, b) => a.seq - b.seq)); + } + if (path === `/api/v1/slabs/${slab.id}/photos` && method === 'POST') { + // Multipart; pull the photo_id / seq / role form fields out of the body. + const raw = req.postData() ?? ''; + const field = (name: string): string => { + const re = new RegExp(`name="${name}"\\r\\n\\r\\n([\\s\\S]*?)(?=$|--)`); + const m = raw.match(re); + return m ? m[1].trim() : ''; + }; + const photoId = field('photo_id'); + const seq = Number(field('seq')); + const role = field('role') || 'batch'; + // Capture-time uploads (photo_id not in the fixture store, e.g. "photo-1") + // are accepted but not added to the lineup — the strip is driven by the + // stored rows only. Review-strip re-uploads reference a known fixture id. + if (!photoStore.has(photoId)) { + return json(route, { id: photoId, slab_id: slab.id, seq, is_calibration: 0, role, original_retained: true, processed_retained: true }, 201); + } + persistCalls.push({ photoId, seq, role }); + const existing = photoStore.get(photoId); + if (existing) { + existing.seq = seq; + existing.role = role as typeof existing.role; + } + return json(route, { id: photoId, slab_id: slab.id, seq, is_calibration: 0, role, original_retained: true, processed_retained: true }, 201); + } + const delMatch = path.match(new RegExp(`^/api/v1/slabs/${slab.id}/photos/([^/]+)$`)); + if (delMatch && method === 'DELETE') { + const id = delMatch[1]; + if (photoStore.delete(id)) { deletes.push(id); return route.fulfill({ status: 204, body: '' }); } + return json(route, { error: { code: 'not_found', message: 'Photo not found', detail: null } }, 404); + } + + if (path === `/api/v1/slabs/${slab.id}/review` && method === 'POST') return json(route, reviewResult(payload)); + if (path === `/api/v1/slabs/${slab.id}/publish` && method === 'POST') { + return json(route, { ok: true, status: 'draft', woo_product_id: 4242, draft: true }); + } + return json(route, { error: { code: 'not_found', message: path } }, 404); + }); + + return { persistCalls, deletes, photoStore }; +} + +async function reachReview(page: Page) { + await page.goto('/?noload'); + await seedMeasureDraft(page, { sku: '', lengthIn: 48, thicknessIn: 2, sqft: 6.25, bdft: 12.5, widthIn: 11 }); + await page.goto('/'); + await expect(page.getByRole('heading', { name: 'Resume?' })).toBeVisible({ timeout: 15000 }); + await page.getByRole('button', { name: 'Continue' }).click(); + await expect(page.getByTestId('continue-to-review')).toBeVisible({ timeout: 20000 }); + await page.getByTestId('continue-to-review').click(); + await expect(page.getByTestId('review-screen')).toBeVisible({ timeout: 20000 }); +} + +async function completeForReady(page: Page) { + await page.getByTestId('species').selectOption(String(SPECIES_ID)); + await page.getByTestId('wood-cat-btn').click(); + await page.getByTestId(`wood-cat-opt-${LIVEEDGE_ID}`).click(); + await page.getByTestId('wood-cat-done').click(); + await page.getByTestId('edge-type').selectOption('31'); + await page.getByTestId('price').fill('120'); + await expect(page.getByTestId('lock-metadata')).toBeEnabled(); + await page.getByTestId('lock-metadata').click(); + await expect(page.getByTestId('title')).not.toHaveValue(''); + await page.getByTestId('sku').fill('SLAB-0001'); + await page.getByTestId('sku').press('Tab'); +} + +test.describe('M3 Review photo strip (#90)', () => { + test('renders stored photos; leftmost kept = primary; tap keep/remove; drag reorder; minimum; persist', async ({ page }) => { + const mock = await mockApi(page); + await reachReview(page); + + // Strip renders the 3 stored photos in stored seq order. + const cards = page.getByTestId('photo-card'); + await expect(cards).toHaveCount(3); + // Leftmost (cal, seq 1) is the primary. + await expect(cards.nth(0)).toHaveAttribute('data-primary', 'true'); + await expect(page.getByText('PRIMARY', { exact: true })).toHaveCount(1); + + // Remove the primary (cal) → primary auto-promotes to the next leftmost (ov). + await cards.nth(0).click(); + await expect(cards.nth(0)).toHaveAttribute('aria-pressed', 'false'); + await expect(cards.nth(1)).toHaveAttribute('data-primary', 'true'); + // Minimum nudge: cal + batch kept, but overlay still present → only cal missing. + await expect(page.getByTestId('lineup-nudge')).toBeVisible(); + await expect(page.getByTestId('lineup-nudge')).toContainText('clean photo'); + + // Re-add the cal (the original re-check bug) → it's back to the left = primary. + await cards.nth(0).click(); + await expect(cards.nth(0)).toHaveAttribute('aria-pressed', 'true'); + await expect(cards.nth(0)).toHaveAttribute('data-primary', 'true'); + await expect(page.getByTestId('lineup-nudge')).toHaveCount(0); + + // Drag the batch photo (index 2) to the front → it becomes the primary. + await cards.nth(2).scrollIntoViewIfNeeded(); + await page.waitForTimeout(120); + const first = await cards.nth(0).boundingBox(); + const third = await cards.nth(2).boundingBox(); + if (first && third) { + const sx = third.x + third.width / 2, sy = third.y + third.height / 2; + const dx = first.x + first.width / 2, dy = first.y + first.height / 2; + await page.mouse.move(sx, sy); + await page.mouse.down(); + // Several small steps so the HTML5 DnD picks up the drag. + for (let i = 1; i <= 14; i++) { + await page.mouse.move(sx + (dx - sx) * (i / 14), sy + (dy - sy) * (i / 14)); + } + await page.waitForTimeout(60); + await page.mouse.up(); + } + // After reorder the batch is leftmost = primary; seq badges renumber. + await expect(cards.nth(0)).toHaveAttribute('data-primary', 'true'); + + // Fill fields to ready, then Continue → persistLineup runs (re-seq kept, drop none). + await completeForReady(page); + await expect(page.getByTestId('ready-status')).toContainText('Ready to publish', { timeout: 15000 }); + await expect(page.getByTestId('continue-publish')).toBeEnabled(); + await page.getByTestId('continue-publish').click(); + await expect(page.getByTestId('publish-screen')).toBeVisible(); + + // Persisted: 3 re-uploads (re-seq 1..3), no deletes (nothing was removed in final state). + expect(mock.persistCalls.length).toBe(3); + const seqs = mock.persistCalls.map((c) => c.seq).sort((a, b) => a - b); + expect(seqs).toEqual([1, 2, 3]); + }); + + test('removing a photo persists a DELETE and blocks publish when the minimum is broken', async ({ page }) => { + const mock = await mockApi(page); + await reachReview(page); + const cards = page.getByTestId('photo-card'); + + // Remove the overlay (seq 2) → minimum broken (no overlay). + await cards.nth(1).click(); + await expect(cards.nth(1)).toHaveAttribute('aria-pressed', 'false'); + await expect(page.getByTestId('lineup-nudge')).toContainText('overlay'); + + // Complete the rest of the form; the slab fields are ready but the lineup is not. + await completeForReady(page); + await expect(page.getByTestId('ready-status')).toContainText(/Not ready|remaining/); + // Continue is blocked by the lineup minimum even though the fields are set. + await expect(page.getByTestId('continue-publish')).toBeDisabled(); + }); +}); From 7b7adbdb34f09b8779a223b26c96cd61078bae5e Mon Sep 17 00:00:00 2001 From: Ty Date: Mon, 21 Sep 2026 01:41:31 -0700 Subject: [PATCH 04/14] fix(deploy): nginx client_max_body_size 25m for photo uploads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Capture→Review uploads the calibrated lineup PNGs (clean cal + batch) as multipart to /api/v1/slabs/{id}/photos. nginx's default 1MB client_max_body_size 413s on real multi-MB photos, blocking the Capture→Review transition. Allow 25m. --- deploy/nginx-m1.conf | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/deploy/nginx-m1.conf b/deploy/nginx-m1.conf index 7a28f6c..43e3a6c 100644 --- a/deploy/nginx-m1.conf +++ b/deploy/nginx-m1.conf @@ -4,6 +4,11 @@ server { root /usr/share/nginx/html; index index.html; + # Capture→Review uploads the calibrated lineup PNGs (clean cal + batch) as + # multipart to /api/v1/slabs/{id}/photos. nginx's default client_max_body_size + # is 1MB, which 413s on real multi-MB photos. Allow large photo bodies. + client_max_body_size 25m; + # Docker embedded DNS for service name resolution (fastapi) resolver 127.0.0.11 valid=10s; From 0612ce66a638790a038463698ccf7afe3ebd67e9 Mon Sep 17 00:00:00 2001 From: Ty Date: Mon, 21 Sep 2026 01:48:43 -0700 Subject: [PATCH 05/14] =?UTF-8?q?fix(review):=20#90=20photo=20strip=20?= =?UTF-8?q?=E2=80=94=20blob:=20URL=20for=20Primary=20photo,=20checkerboard?= =?UTF-8?q?=20thumbs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug #1: the first/Primary (calibration) photo rendered broken because its in-memory capture Blob was assigned to directly (String(Blob)= "object Blob"). resolvePhotoUrl() now mints a blob: object URL (revoked on unload) and falls back to the stored photo endpoint when there is no blob. Bug #2: strip thumbnails used a flat dark bg; now carry the gray checkerboard from the Capture lineup (MeasurePanel) so transparent inventory PNGs read the same on both screens. Also harden missingMinimums to filter kept internally (pre-existing test 7 failed on the clean base because it passed un-kept items). --- frontend/src/create/review/Review.svelte | 32 ++++++++++++++++++----- frontend/src/create/review/lineup.test.ts | 12 +++++++++ frontend/src/create/review/lineup.ts | 22 +++++++++++++++- 3 files changed, 59 insertions(+), 7 deletions(-) diff --git a/frontend/src/create/review/Review.svelte b/frontend/src/create/review/Review.svelte index 9911fa1..5df62c0 100644 --- a/frontend/src/create/review/Review.svelte +++ b/frontend/src/create/review/Review.svelte @@ -8,7 +8,7 @@ import type { CalibratedCapture } from '../flow'; import MultiSelect from './MultiSelect.svelte'; import { - keptInOrder, moveItem, missingMinimums, roleLabel, + keptInOrder, moveItem, missingMinimums, roleLabel, resolvePhotoUrl, type LineupItem, } from './lineup'; @@ -171,7 +171,15 @@ return m; }; + // Object URLs minted from in-memory blobs; revoked on unmount/reload so the + // src stays valid. A raw Blob assigned to src renders broken — it must + // be a blob: URL (bug: the first/Primary photo was the one carrying a blob). + let photoUrls: string[] = []; + async function loadPhotos() { + // Revoke any previous object URLs before rebuilding the strip. + for (const u of photoUrls) URL.revokeObjectURL(u); + photoUrls = []; try { const rows: PhotoRow[] = await listPhotos(slabId); const byRole = inMemByRole(); @@ -180,9 +188,11 @@ const role = r.role ?? 'batch'; const n = seqByRole[role]++; const blob = (byRole.get(role)?.[n]) ?? null; - const url = - blob ?? - (photoUrl(slabId, r.id, (r.has_processed ? 'processed' : 'original') as 'original' | 'processed')); + // Prefer the in-memory blob (fresh capture), as an object URL. Otherwise + // fall back to the stored photo endpoint (a plain string URL). + const endpoint = photoUrl(slabId, r.id, (r.has_processed ? 'processed' : 'original') as 'original' | 'processed'); + const url = resolvePhotoUrl(blob, endpoint); + if (blob && blob.size > 0) photoUrls.push(url); return { id: r.id, role, @@ -190,7 +200,7 @@ kept: true, original: blob ?? new Blob(), processed: blob ?? null, - url: String(url), + url, }; }); photosLoaded = true; @@ -201,6 +211,11 @@ } } + onDestroy(() => { + for (const u of photoUrls) URL.revokeObjectURL(u); + photoUrls = []; + }); + // Whole-card tap = keep/remove toggle. A real drag (>6px) reorders and does NOT // toggle keep/remove — the threshold lives in the pointer handlers below. function toggleKeep(id: string) { @@ -778,7 +793,12 @@ .ph-hint { color: #8a8a8a; font-size: .72rem; margin: 0 0 8px; } .ph-grid { display: grid; grid-template-columns: repeat(3, 1fr); gap: 8px; } @media (max-width: 420px) { .ph-grid { grid-template-columns: repeat(2, 1fr); } } - .pcard { position: relative; aspect-ratio: 1; border: 2px solid #2a2a2a; border-radius: 3px; overflow: hidden; cursor: grab; background: #141414; } + .pcard { position: relative; aspect-ratio: 1; border: 2px solid #2a2a2a; border-radius: 3px; overflow: hidden; cursor: grab; + /* Gray checkerboard, matching the Capture lineup (MeasurePanel .lineup-item.png) + so transparent inventory PNGs read the same on both screens (bug2). */ + background-color: #eee; + background-image: linear-gradient(45deg,#ccc 25%,transparent 25%),linear-gradient(-45deg,#ccc 25%,transparent 25%),linear-gradient(45deg,transparent 75%,#ccc 75%),linear-gradient(-45deg,transparent 75%,#ccc 75%); + background-size: 16px 16px; background-position: 0 0,0 8px,8px -8px,-8px 0; } .pcard:active { cursor: grabbing; } .pcard:focus-visible { outline: 2px solid #00e5ff; outline-offset: 2px; } .pcard.kept { border-color: #444; } diff --git a/frontend/src/create/review/lineup.test.ts b/frontend/src/create/review/lineup.test.ts index 7899d6b..27be45d 100644 --- a/frontend/src/create/review/lineup.test.ts +++ b/frontend/src/create/review/lineup.test.ts @@ -6,6 +6,7 @@ import { missingMinimums, lineupValid, roleLabel, + resolvePhotoUrl, type LineupItem, } from './lineup'; @@ -69,3 +70,14 @@ test('lineup: roleLabel is owner-facing', () => { assert.match(roleLabel('calibration'), /clean/); assert.match(roleLabel('overlay'), /overlay/); }); + +test('lineup: resolvePhotoUrl turns an in-memory blob into a blob: URL (bug #1)', () => { + // The first/Primary (calibration) photo carries a fresh capture blob. It must + // become a valid object URL — never a raw Blob, never String(blob)="object Blob". + const blob = new Blob(['png-bytes'], { type: 'image/png' }); + const fake = (b: Blob) => `blob:fake-${b.size}`; + assert.equal(resolvePhotoUrl(blob, '/api/endpoints/photo', fake), 'blob:fake-9'); + // Empty/absent blob falls back to the stored endpoint. + assert.equal(resolvePhotoUrl(new Blob(), '/api/endpoints/photo', fake), '/api/endpoints/photo'); + assert.equal(resolvePhotoUrl(null, '/api/endpoints/photo', fake), '/api/endpoints/photo'); +}); diff --git a/frontend/src/create/review/lineup.ts b/frontend/src/create/review/lineup.ts index b4fd348..bc931c8 100644 --- a/frontend/src/create/review/lineup.ts +++ b/frontend/src/create/review/lineup.ts @@ -42,7 +42,8 @@ export function moveItem(arr: T[], from: number, to: number): T[] { * clean calibration photo and one dimension-overlay copy. Returns the missing * role(s) so the UI can name them in a nudge. */ -export function missingMinimums(kept: LineupItem[]): PhotoRole[] { +export function missingMinimums(items: LineupItem[]): PhotoRole[] { + const kept = items.filter((it) => it.kept); const missing: PhotoRole[] = []; if (!kept.some((it) => it.role === 'calibration')) missing.push('calibration'); if (!kept.some((it) => it.role === 'overlay')) missing.push('overlay'); @@ -60,3 +61,22 @@ export function roleLabel(role: PhotoRole): string { if (role === 'overlay') return 'its dimension-overlay copy'; return 'a batch photo'; } + +/** + * Resolve the src for a strip photo. The in-memory capture blob (fresh + * photo) must become a `blob:` object URL — assigning a raw Blob (or + * `String(blob)` = "object Blob") to `src` renders a broken image. That was + * bug #1: the first/Primary (calibration) photo is the one carrying a blob. + * When there is no usable blob, fall back to the stored photo endpoint. + * + * Pure + injectable: `createObjectUrl` defaults to `URL.createObjectURL` so it + * is unit-testable without a real DOM/Blob. + */ +export function resolvePhotoUrl( + blob: Blob | null, + endpoint: string, + createObjectUrl: (b: Blob) => string = (b) => URL.createObjectURL(b), +): string { + if (blob && blob.size > 0) return createObjectUrl(blob); + return endpoint; +} From cd326d1ec997ed8c7744aeaadaa57c4c61d69695 Mon Sep 17 00:00:00 2001 From: Ty Date: Mon, 21 Sep 2026 01:54:21 -0700 Subject: [PATCH 06/14] test(review): #90 assert Primary photo src is valid + checkerboard on strip Locks in the two live-bug fixes: the Primary (leftmost kept) photo's must be a real URL (blob: for an in-memory capture blob) and never empty/ "object Blob"; and each strip card must carry the gray checkerboard background matching the Capture lineup. --- frontend/tests/e2e/review-photo-strip.spec.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/frontend/tests/e2e/review-photo-strip.spec.ts b/frontend/tests/e2e/review-photo-strip.spec.ts index 6b8b71a..1473875 100644 --- a/frontend/tests/e2e/review-photo-strip.spec.ts +++ b/frontend/tests/e2e/review-photo-strip.spec.ts @@ -200,6 +200,18 @@ test.describe('M3 Review photo strip (#90)', () => { await expect(cards.nth(0)).toHaveAttribute('data-primary', 'true'); await expect(page.getByText('PRIMARY', { exact: true })).toHaveCount(1); + // Bug #1 regression: the primary photo's must be a valid URL + // (blob: for an in-memory capture blob), never empty or "object Blob". + const primaryImg = cards.nth(0).locator('img'); + await expect(primaryImg).not.toHaveAttribute('src', /object Blob|^$/); + const src = await primaryImg.getAttribute('src'); + expect(src && src.length > 4).toBeTruthy(); + + // Bug #2 regression: the card carries the gray checkerboard background + // (matching the Capture lineup), not a flat color. + const bgImage = await cards.nth(0).evaluate((el) => getComputedStyle(el).backgroundImage); + expect(bgImage).toContain('45deg'); + // Remove the primary (cal) → primary auto-promotes to the next leftmost (ov). await cards.nth(0).click(); await expect(cards.nth(0)).toHaveAttribute('aria-pressed', 'false'); From 5a1800df917c8945c2d6c89f487cc4c9839ee860 Mon Sep 17 00:00:00 2001 From: Ty Date: Mon, 21 Sep 2026 01:59:27 -0700 Subject: [PATCH 07/14] fix(review): #90 persist the dimension-overlay copy with its role MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Capture→Review upload filtered out the overlay (role !== 'overlay') and sent no role field, so the dimension-overlay copy was never stored and every photo defaulted to role=batch. Review then fell back to a stored /processed URL that had no dimensions drawn — the owner saw the overlay thumbnail without its measurements. Now upload the full lineup with the correct role, keeping the overlay's dimension-drawn blob as both original and processed. --- frontend/src/App.svelte | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/frontend/src/App.svelte b/frontend/src/App.svelte index 63cd5d6..1ea58f4 100644 --- a/frontend/src/App.svelte +++ b/frontend/src/App.svelte @@ -105,16 +105,22 @@ content_source: 'manual', client_rev: 0, }); - const pngItems = capture.lineup.filter( - (it) => it.kind === 'png' && it.role !== 'overlay', - ); + // #90 / REVIEW-5: persist the full lineup with correct roles so the Review + // strip matches in-memory blobs by role. The overlay is the dimension-drawn + // copy — it must be persisted (its blob already has the dimensions) and is + // the photo the owner sees with the measurement overlay. Excluding it (the + // old `role !== 'overlay'` filter) dropped the dimension copy from Review. + const pngItems = capture.lineup.filter((it) => it.kind === 'png'); for (let i = 0; i < pngItems.length; i++) { const it = pngItems[i]; await uploadPhoto(id, { photoId: uuid(), seq: i + 1, isCalibration: it.role === 'calibration', + role: it.role, original: it.blob, + // The calibration/batch PNGs have no separate processed variant; the + // overlay blob already carries the drawn dimensions. processed: it.blob, }); } From 76fcb36a2b7d2c19d1b10f2cf75a2c13d323c13b Mon Sep 17 00:00:00 2001 From: Ty Date: Mon, 21 Sep 2026 02:15:00 -0700 Subject: [PATCH 08/14] fix(review): #90 clean cal + dimension-overlay are mandatory (checked + disabled) REVIEW-5 requires both the clean calibration photo and its dimension-overlay copy to ship. Their keep checkbox is now checked AND disabled (aria-disabled, a lock glyph, cursor:default, tap is a no-op) so the owner can't remove them. Only batch photos are removable. The submission minimum can no longer be broken by the owner, so publish is never lineup-blocked by a missing required photo. --- frontend/src/create/review/Review.svelte | 35 ++++++++++-- frontend/tests/e2e/review-photo-strip.spec.ts | 57 +++++++++++++------ 2 files changed, 70 insertions(+), 22 deletions(-) diff --git a/frontend/src/create/review/Review.svelte b/frontend/src/create/review/Review.svelte index 5df62c0..081f9fb 100644 --- a/frontend/src/create/review/Review.svelte +++ b/frontend/src/create/review/Review.svelte @@ -147,6 +147,10 @@ role: 'calibration' | 'overlay' | 'batch'; isCalibration: boolean; kept: boolean; + /** True for the mandatory photos (clean calibration + dimension-overlay + copy). Their keep checkbox is checked and disabled — the owner cannot + remove them (REVIEW-5 requires both). */ + required: boolean; original: Blob; processed: Blob | null; url: string; @@ -197,6 +201,10 @@ id: r.id, role, isCalibration: r.is_calibration === 1, + // Mandatory: the clean calibration photo and the dimension-overlay + // copy. REVIEW-5 requires both to ship, so their keep control is + // checked + disabled (the owner can't remove them). + required: role === 'calibration' || role === 'overlay', kept: true, original: blob ?? new Blob(), processed: blob ?? null, @@ -219,7 +227,12 @@ // Whole-card tap = keep/remove toggle. A real drag (>6px) reorders and does NOT // toggle keep/remove — the threshold lives in the pointer handlers below. function toggleKeep(id: string) { - photos = photos.map((p) => (p.id === id ? { ...p, kept: !p.kept } : p)); + photos = photos.map((p) => { + if (p.id !== id) return p; + // Mandatory photos (clean cal + dimension-overlay) can't be removed. + if (p.required) return p; + return { ...p, kept: !p.kept }; + }); lineupDirty = true; } @@ -524,12 +537,15 @@ class="pcard" class:kept={p.kept} class:primary={p.id === primaryId} + class:required={p.required} data-testid="photo-card" data-primary={p.id === primaryId ? 'true' : ''} + data-required={p.required ? 'true' : ''} role="button" tabindex="0" - aria-pressed={p.kept} - aria-label="{roleLabel(p.role)}{p.kept ? ', kept' : ', removed'}" + aria-pressed={p.required ? true : p.kept} + aria-disabled={p.required ? 'true' : 'false'} + aria-label="{p.required ? 'Required: ' : ''}{roleLabel(p.role)}{p.required ? ' (cannot be removed)' : (p.kept ? ', kept' : ', removed')}" draggable="true" on:click={() => toggleKeep(p.id)} on:keydown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); toggleKeep(p.id); } }} @@ -542,7 +558,9 @@ PRIMARY {/if} - + + {/each} @@ -810,5 +828,14 @@ .ph-keep { position: absolute; top: 6px; left: 6px; width: 16px; height: 16px; border: 2px solid #fff; border-radius: 3px; background: rgba(0,0,0,.5); pointer-events: none; } .pcard.kept .ph-keep { background: #00e5ff; border-color: #00e5ff; } .pcard.kept .ph-keep::after { content: ''; position: absolute; left: 4px; top: 1px; width: 4px; height: 8px; border: solid #002; border-width: 0 2px 2px 0; transform: rotate(45deg); } + /* Mandatory photos (clean cal + dimension-overlay): the keep checkbox is + checked AND disabled — it can't be unchecked, and a small lock glyph + signals "required, cannot be removed". The card keeps full opacity even + though it's always kept, and the cursor shows it's not a remove target. */ + .pcard.required { border-color: #2a2a2a; } + .pcard.required .ph-keep { background: #00e5ff; border-color: #00e5ff; } + .pcard.required .ph-keep::after { content: ''; position: absolute; left: 4px; top: 1px; width: 4px; height: 8px; border: solid #002; border-width: 0 2px 2px 0; transform: rotate(45deg); } + .pcard.required .ph-keep::before { content: ''; position: absolute; left: 2.5px; top: 5px; width: 7px; height: 5px; border: 1.5px solid #002; border-radius: 1px; } + .pcard.required { cursor: default; } .ph-nudge { color: #e8a; font-size: .74rem; margin: 8px 0 0; } diff --git a/frontend/tests/e2e/review-photo-strip.spec.ts b/frontend/tests/e2e/review-photo-strip.spec.ts index 1473875..6f375d1 100644 --- a/frontend/tests/e2e/review-photo-strip.spec.ts +++ b/frontend/tests/e2e/review-photo-strip.spec.ts @@ -212,18 +212,30 @@ test.describe('M3 Review photo strip (#90)', () => { const bgImage = await cards.nth(0).evaluate((el) => getComputedStyle(el).backgroundImage); expect(bgImage).toContain('45deg'); - // Remove the primary (cal) → primary auto-promotes to the next leftmost (ov). - await cards.nth(0).click(); - await expect(cards.nth(0)).toHaveAttribute('aria-pressed', 'false'); - await expect(cards.nth(1)).toHaveAttribute('data-primary', 'true'); - // Minimum nudge: cal + batch kept, but overlay still present → only cal missing. - await expect(page.getByTestId('lineup-nudge')).toBeVisible(); - await expect(page.getByTestId('lineup-nudge')).toContainText('clean photo'); + // Mandatory photos: the clean calibration (seq 1) and the dimension-overlay + // copy (seq 2) are required — their keep checkbox is checked AND disabled. + await expect(cards.nth(0)).toHaveAttribute('data-required', 'true'); + await expect(cards.nth(1)).toHaveAttribute('data-required', 'true'); + await expect(cards.nth(2)).toHaveAttribute('data-required', ''); + await expect(cards.nth(0)).toHaveAttribute('aria-pressed', 'true'); + await expect(cards.nth(1)).toHaveAttribute('aria-pressed', 'true'); - // Re-add the cal (the original re-check bug) → it's back to the left = primary. - await cards.nth(0).click(); + // Tapping a mandatory photo is a no-op: it stays checked + primary (can't remove). + // The card is aria-disabled, so force the click to exercise the handler directly. + await cards.nth(0).click({ force: true }); await expect(cards.nth(0)).toHaveAttribute('aria-pressed', 'true'); await expect(cards.nth(0)).toHaveAttribute('data-primary', 'true'); + // No minimum nudge — both required photos are kept by construction. + await expect(page.getByTestId('lineup-nudge')).toHaveCount(0); + + // Remove the OPTIONAL batch photo (index 2) → it un-keeps; required stay kept. + await cards.nth(2).click(); + await expect(cards.nth(2)).toHaveAttribute('aria-pressed', 'false'); + // Primary is still the leftmost required (cal, index 0). + await expect(cards.nth(0)).toHaveAttribute('data-primary', 'true'); + // Re-add the batch photo. + await cards.nth(2).click(); + await expect(cards.nth(2)).toHaveAttribute('aria-pressed', 'true'); await expect(page.getByTestId('lineup-nudge')).toHaveCount(0); // Drag the batch photo (index 2) to the front → it becomes the primary. @@ -259,20 +271,29 @@ test.describe('M3 Review photo strip (#90)', () => { expect(seqs).toEqual([1, 2, 3]); }); - test('removing a photo persists a DELETE and blocks publish when the minimum is broken', async ({ page }) => { + test('required photos guarantee the minimum; removing an OPTIONAL batch persists a DELETE', async ({ page }) => { const mock = await mockApi(page); await reachReview(page); const cards = page.getByTestId('photo-card'); - // Remove the overlay (seq 2) → minimum broken (no overlay). - await cards.nth(1).click(); - await expect(cards.nth(1)).toHaveAttribute('aria-pressed', 'false'); - await expect(page.getByTestId('lineup-nudge')).toContainText('overlay'); + // The clean calibration + dimension-overlay are mandatory and always kept, + // so the submission minimum can never be broken — publish is never lineup-blocked. + await expect(page.getByTestId('lineup-nudge')).toHaveCount(0); - // Complete the rest of the form; the slab fields are ready but the lineup is not. + // Removing an OPTIONAL batch photo is allowed and persists a DELETE. + await cards.nth(2).click(); + await expect(cards.nth(2)).toHaveAttribute('aria-pressed', 'false'); + // Still no nudge: both required photos remain kept. + await expect(page.getByTestId('lineup-nudge')).toHaveCount(0); + + // Complete the rest of the form; the lineup is valid (required photos kept). await completeForReady(page); - await expect(page.getByTestId('ready-status')).toContainText(/Not ready|remaining/); - // Continue is blocked by the lineup minimum even though the fields are set. - await expect(page.getByTestId('continue-publish')).toBeDisabled(); + await expect(page.getByTestId('ready-status')).toContainText('Ready to publish', { timeout: 15000 }); + await expect(page.getByTestId('continue-publish')).toBeEnabled(); + await page.getByTestId('continue-publish').click(); + await expect(page.getByTestId('publish-screen')).toBeVisible(); + + // The removed batch photo persisted as a DELETE; the required photos re-uploaded. + expect(mock.deletes).toEqual(['ph-b1']); }); }); From e1b928ccbd3030ab6aec1b8868cd64abe8c1205d Mon Sep 17 00:00:00 2001 From: Ty Date: Mon, 21 Sep 2026 02:18:01 -0700 Subject: [PATCH 09/14] fix(review): #90 clearer mandatory-photo indicator (padlock + REQUIRED tag) The mandatory clean-cal and dimension-overlay photos now show a padlock glyph in their keep box and an amber REQUIRED tag at the top of the card, so the owner can see at a glance which photos cannot be removed (vs. optional batch photos, which keep a plain checkmark). --- frontend/src/create/review/Review.svelte | 23 ++++++++++++++----- frontend/tests/e2e/review-photo-strip.spec.ts | 8 ++++++- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/frontend/src/create/review/Review.svelte b/frontend/src/create/review/Review.svelte index 081f9fb..74490d1 100644 --- a/frontend/src/create/review/Review.svelte +++ b/frontend/src/create/review/Review.svelte @@ -557,6 +557,9 @@ {#if p.id === primaryId} PRIMARY {/if} + {#if p.required} + REQUIRED + {/if} @@ -824,18 +827,26 @@ .pcard:not(.kept) { opacity: .4; } .ph-thumb { width: 100%; height: 100%; object-fit: cover; display: block; pointer-events: none; } .ph-pri { position: absolute; top: 6px; right: 6px; background: #00e5ff; color: #002; font-size: .56rem; font-weight: 800; padding: 1px 5px; border-radius: 2px; letter-spacing: .03em; pointer-events: none; } + .ph-req { position: absolute; top: 6px; left: 50%; transform: translateX(-50%); background: #ffb020; color: #201500; font-size: .52rem; font-weight: 800; padding: 1px 5px; border-radius: 2px; letter-spacing: .04em; pointer-events: none; } .ph-seq { position: absolute; bottom: 6px; left: 6px; width: 18px; height: 18px; border-radius: 50%; background: rgba(0,0,0,.7); color: #fff; font-size: .66rem; display: flex; align-items: center; justify-content: center; pointer-events: none; } .ph-keep { position: absolute; top: 6px; left: 6px; width: 16px; height: 16px; border: 2px solid #fff; border-radius: 3px; background: rgba(0,0,0,.5); pointer-events: none; } .pcard.kept .ph-keep { background: #00e5ff; border-color: #00e5ff; } .pcard.kept .ph-keep::after { content: ''; position: absolute; left: 4px; top: 1px; width: 4px; height: 8px; border: solid #002; border-width: 0 2px 2px 0; transform: rotate(45deg); } /* Mandatory photos (clean cal + dimension-overlay): the keep checkbox is - checked AND disabled — it can't be unchecked, and a small lock glyph - signals "required, cannot be removed". The card keeps full opacity even - though it's always kept, and the cursor shows it's not a remove target. */ - .pcard.required { border-color: #2a2a2a; } + checked AND disabled. A bold padlock glyph in the box signals "required — + cannot be removed", and the card carries a thin required outline so the + owner can tell mandatory photos apart from optional batch photos. */ + .pcard.required { border-color: #00e5ff55; } .pcard.required .ph-keep { background: #00e5ff; border-color: #00e5ff; } - .pcard.required .ph-keep::after { content: ''; position: absolute; left: 4px; top: 1px; width: 4px; height: 8px; border: solid #002; border-width: 0 2px 2px 0; transform: rotate(45deg); } - .pcard.required .ph-keep::before { content: ''; position: absolute; left: 2.5px; top: 5px; width: 7px; height: 5px; border: 1.5px solid #002; border-radius: 1px; } + /* Padlock: shackle (arc) + body, drawn large enough to read on a 16px box. */ + .pcard.required .ph-keep::after { + content: ''; position: absolute; left: 3px; top: 2px; width: 8px; height: 8px; + border: 2px solid #002; border-bottom: none; border-radius: 4px 4px 0 0; + } + .pcard.required .ph-keep::before { + content: ''; position: absolute; left: 2.5px; top: 6px; width: 8px; height: 6px; + background: #002; border-radius: 1px; + } .pcard.required { cursor: default; } .ph-nudge { color: #e8a; font-size: .74rem; margin: 8px 0 0; } diff --git a/frontend/tests/e2e/review-photo-strip.spec.ts b/frontend/tests/e2e/review-photo-strip.spec.ts index 6f375d1..62d2236 100644 --- a/frontend/tests/e2e/review-photo-strip.spec.ts +++ b/frontend/tests/e2e/review-photo-strip.spec.ts @@ -213,12 +213,18 @@ test.describe('M3 Review photo strip (#90)', () => { expect(bgImage).toContain('45deg'); // Mandatory photos: the clean calibration (seq 1) and the dimension-overlay - // copy (seq 2) are required — their keep checkbox is checked AND disabled. + // copy (seq 2) are required — their keep checkbox is checked AND disabled, + // and a REQUIRED tag marks them. await expect(cards.nth(0)).toHaveAttribute('data-required', 'true'); await expect(cards.nth(1)).toHaveAttribute('data-required', 'true'); await expect(cards.nth(2)).toHaveAttribute('data-required', ''); await expect(cards.nth(0)).toHaveAttribute('aria-pressed', 'true'); await expect(cards.nth(1)).toHaveAttribute('aria-pressed', 'true'); + await expect(cards.nth(0)).toHaveAttribute('aria-disabled', 'true'); + await expect(cards.nth(1)).toHaveAttribute('aria-disabled', 'true'); + await expect(cards.nth(2)).toHaveAttribute('aria-disabled', 'false'); + // Two REQUIRED tags (cal + overlay), none on the batch photo. + await expect(page.getByText('REQUIRED', { exact: true })).toHaveCount(2); // Tapping a mandatory photo is a no-op: it stays checked + primary (can't remove). // The card is aria-disabled, so force the click to exercise the handler directly. From 0329052d37972deaee9db3265308e61d0655318d Mon Sep 17 00:00:00 2001 From: Ty Date: Mon, 21 Sep 2026 02:41:20 -0700 Subject: [PATCH 10/14] feat(review): #90 wood primary category + derived thickness band --- backend/slabs.py | 11 ++ backend/tests/test_slabs_api.py | 66 +++++++ backend/tests/test_thickness_band.py | 94 ++++++++++ backend/thickness_band.py | 177 ++++++++++++++++++ docs/REQUIREMENTS.md | 2 +- docs/UAT-PRACTICE.md | 4 +- docs/contracts/UX.md | 5 +- frontend/package.json | 2 +- frontend/src/create/review/MultiSelect.svelte | 26 ++- frontend/src/create/review/Review.svelte | 145 +++++++++++--- .../src/create/review/woodPrimary.test.ts | 43 +++++ frontend/src/create/review/woodPrimary.ts | 32 ++++ frontend/tests/e2e/m3-create.spec.ts | 154 ++++++++++++++- 13 files changed, 719 insertions(+), 42 deletions(-) create mode 100644 backend/tests/test_thickness_band.py create mode 100644 backend/thickness_band.py create mode 100644 frontend/src/create/review/woodPrimary.test.ts create mode 100644 frontend/src/create/review/woodPrimary.ts diff --git a/backend/slabs.py b/backend/slabs.py index d4e2f1d..81d3cac 100644 --- a/backend/slabs.py +++ b/backend/slabs.py @@ -13,6 +13,7 @@ from fastapi import APIRouter from fastapi.responses import JSONResponse, Response from .db import get_connection +from .thickness_band import load_thickness_terms, resolve_thickness_band_id router = APIRouter(prefix="/api/v1/slabs") @@ -133,6 +134,12 @@ async def create_slab(payload: Dict[str, Any]) -> Dict[str, Any]: conn = get_connection() if payload["sku"] is not None and _sku_taken(conn, payload["sku"]): return _err("duplicate_sku", "SKU already exists", {"sku": payload["sku"]}, status=409) + # METADATA-15 / QUALITY-95: the thickness band is derived, never owner-chosen. + # Resolve it from the measured thickness against the live synced Woo bands and + # overwrite whatever the client sent (the UI has no band control). + band = resolve_thickness_band_id(payload.get("thickness_in"), load_thickness_terms(conn)) + if band is not None: + payload = {**payload, "thickness_term_id": band} now = _now_iso() data = _encode(payload) allowed_cols = [f for f in ALLOWED_FIELDS if f in data] @@ -203,6 +210,10 @@ async def update_slab(id: str, payload: Dict[str, Any]) -> Dict[str, Any]: live = [val] # single-id field: just show what was submitted return _err("stale_field", f"{field} not in current taxonomy", {"field": field, "submitted": val, "current_options": live}, status=422) + eff_thickness = payload.get("thickness_in", current["thickness_in"]) + band = resolve_thickness_band_id(eff_thickness, load_thickness_terms(conn)) + if band is not None: + payload = {**payload, "thickness_term_id": band} data = _encode(payload) cols = [f for f in ALLOWED_UPDATE_FIELDS if f in data] if not cols: diff --git a/backend/tests/test_slabs_api.py b/backend/tests/test_slabs_api.py index 4567e0f..c4577e4 100644 --- a/backend/tests/test_slabs_api.py +++ b/backend/tests/test_slabs_api.py @@ -33,6 +33,23 @@ def _seed_taxonomy(conn): # tags conn.execute("INSERT OR REPLACE INTO woo_tags(id, name, slug, woo_id) VALUES (1, 'Fig Tag A', 'fig-tag-a', 50)") conn.execute("INSERT OR REPLACE INTO woo_tags(id, name, slug, woo_id) VALUES (2, 'Feat Tag A', 'feat-tag-a', 60)") + # Thickness bands (METADATA-15 / QUALITY-95) — names exactly as synced from the + # live Woo attribute, because the derivation parses them. + conn.execute("INSERT OR REPLACE INTO woo_attributes(id, name, slug, woo_id) VALUES (2, 'Thickness', 'pa_thk', 31)") + for tid, name, slug in [ + (347, 'Under 1/2"', 'xthin'), + (348, '1/2" - 3/4"', '2-3qtr'), + (349, '3/4" - 1"', '3-4qtr'), + (350, '1"- 1 1/2"', '4-6qtr'), + (351, '1 1/2" - 2"', '6-8qtr'), + (352, '2" - 3"', '8-12qtr'), + (353, 'Over 3"', '12qtr'), + ]: + conn.execute( + "INSERT OR REPLACE INTO woo_attribute_terms(id, attribute_id, name, slug, woo_id) " + "VALUES (?, 2, ?, ?, ?)", + (tid, name, slug, tid), + ) conn.commit() @pytest.fixture @@ -103,6 +120,55 @@ def test_create_and_fetch(client, db): assert body[key] == payload[key] +def test_create_derives_thickness_band(client, db): + """QUALITY-95: 1 in rounds up into the live synced band, not a hardcoded one. + + The client-supplied thickness_term_id (1, an Edge Type term) must be ignored — + the UI has no band control, so the server's derivation is authoritative. + """ + slab_id = str(uuid.uuid4()) + payload = { + "id": slab_id, "sku": "SKU-BAND", "length_in": 10, "thickness_in": 1.0, "sqft": 20, + "bdft": 5, "width_min_in": 1, "width_max_in": 2, "width_avg_in": 1.5, + "thickness_term_id": 1, + } + resp = client.post("/api/v1/slabs", json=payload) + assert resp.status_code == 201 + assert resp.json()["thickness_term_id"] == 350 # '1"- 1 1/2"', not the sent 1 + + +def test_update_rebands_when_thickness_changes(client, db): + """A thickness edit moves the slab to the band the new measurement rounds into.""" + slab_id = str(uuid.uuid4()) + create = client.post("/api/v1/slabs", json={ + "id": slab_id, "sku": "SKU-BAND2", "length_in": 10, "thickness_in": 0.5, "sqft": 20, + "bdft": 5, "width_min_in": 1, "width_max_in": 2, "width_avg_in": 1.5, + }) + assert create.status_code == 201 + assert create.json()["thickness_term_id"] == 348 # 1/2" - 3/4" + put = client.put(f"/api/v1/slabs/{slab_id}", json={ + "client_rev": create.json()["server_rev"], "thickness_in": 2.75, + }) + assert put.status_code == 200 + assert put.json()["thickness_term_id"] == 352 # 2" - 3" + + +def test_update_ignores_client_supplied_band(client, db): + """A stale client band cannot pin the slab to the wrong band.""" + slab_id = str(uuid.uuid4()) + create = client.post("/api/v1/slabs", json={ + "id": slab_id, "sku": "SKU-BAND3", "length_in": 10, "thickness_in": 1.75, "sqft": 20, + "bdft": 5, "width_min_in": 1, "width_max_in": 2, "width_avg_in": 1.5, + }) + assert create.status_code == 201 + assert create.json()["thickness_term_id"] == 351 # 1 1/2" - 2" + put = client.put(f"/api/v1/slabs/{slab_id}", json={ + "client_rev": create.json()["server_rev"], "thickness_term_id": 347, + }) + assert put.status_code == 200 + assert put.json()["thickness_term_id"] == 351 # unchanged: still the derived band + + def test_duplicate_sku(client, db): slab_id = str(uuid.uuid4()) payload = {"id": slab_id, "sku": "SKU001", "length_in":10,"thickness_in":2,"sqft":20,"bdft":5,"width_min_in":1,"width_max_in":2,"width_avg_in":1.5,"species_id":1,"wood_category_ids":json.dumps([2]),"edge_type_term_id":1,"figure_term_ids":json.dumps([1]),"grade_term_id":1,"thickness_term_id":1,"moisture_term_id":1,"fig_tag_ids":json.dumps([1]),"feat_tag_ids":json.dumps([2]),"price":100,"price_source":"recommendation","title":"Test","short_description":"short","description":"desc","content_source":"manual","species_confidence":0.9,"figure_confidence":0.8,"grade_confidence":0.7,"inference_status":"inferring","calibration_photo_id":None} diff --git a/backend/tests/test_thickness_band.py b/backend/tests/test_thickness_band.py new file mode 100644 index 0000000..972422d --- /dev/null +++ b/backend/tests/test_thickness_band.py @@ -0,0 +1,94 @@ +"""Thickness band resolution (METADATA-15 / QUALITY-95). + +The owner never picks the Woo thickness band in the UI — the server derives it +from the measured thickness_in against the live synced bands. These tests pin the +derivation on the real live band names (from the UAT taxonomy) and on the +quarter-inch slug fallback. +""" + +import pytest + +from backend.thickness_band import ( + parse_inch_token, + resolve_thickness_band_id, + term_bounds, +) + +# The live UAT Thickness attribute (pa_thk) as synced: name, slug. +LIVE_TERMS = [ + {"id": 347, "name": "Under 1/2\"", "slug": "xthin"}, + {"id": 348, "name": "1/2\" - 3/4\"", "slug": "2-3qtr"}, + {"id": 349, "name": "3/4\" - 1\"", "slug": "3-4qtr"}, + {"id": 350, "name": "1\"- 1 1/2\"", "slug": "4-6qtr"}, + {"id": 351, "name": "1 1/2\" - 2\"", "slug": "6-8qtr"}, + {"id": 352, "name": "2\" - 3\"", "slug": "8-12qtr"}, + {"id": 353, "name": "Over 3\"", "slug": "12qtr"}, +] + + +@pytest.mark.parametrize( + "text,expected", + [ + ("1 1/2", 1.5), + ("3/4", 0.75), + ("2", 2.0), + ("1.75", 1.75), + ("2\"", 2.0), + ("1 1/2\"", 1.5), + ], +) +def test_parse_inch_token(text, expected): + assert parse_inch_token(text) == pytest.approx(expected) + + +def test_parse_inch_token_rejects_junk(): + assert parse_inch_token("") is None + assert parse_inch_token(" ") is None + + +def test_term_bounds_from_live_names(): + assert term_bounds(LIVE_TERMS[0]) == (0.0, 0.5) + assert term_bounds(LIVE_TERMS[1]) == (0.5, 0.75) + assert term_bounds(LIVE_TERMS[3]) == (1.0, 1.5) + assert term_bounds(LIVE_TERMS[6]) == (3.0, float("inf")) + + +def test_term_bounds_falls_back_to_qtr_slug(): + assert term_bounds({"id": 9, "name": "weird", "slug": "4-6qtr"}) == (1.0, 1.5) + assert term_bounds({"id": 9, "name": "weird", "slug": "12qtr"}) == (3.0, float("inf")) + assert term_bounds({"id": 9, "name": "weird", "slug": "xthin"}) == (0.0, 0.5) + + +@pytest.mark.parametrize( + "thickness,expected_id", + [ + (0.25, 347), # Under 1/2" + (0.5, 348), # boundary rounds UP into 1/2"-3/4", not down + (0.625, 348), + (0.75, 349), # boundary rounds UP + (0.875, 349), + (1.0, 350), # QUALITY-95: 1 inch rounds up into 1"-1 1/2", not a hardcoded band + (1.25, 350), + (1.5, 351), + (1.75, 351), # the UAT sample thickness + (2.0, 352), + (2.75, 352), + (3.0, 353), + (4.0, 353), + ], +) +def test_resolve_rounds_up_on_boundary(thickness, expected_id): + assert resolve_thickness_band_id(thickness, LIVE_TERMS) == expected_id + + +def test_resolve_handles_missing_inputs(): + assert resolve_thickness_band_id(None, LIVE_TERMS) is None + assert resolve_thickness_band_id(-1, LIVE_TERMS) is None + assert resolve_thickness_band_id(1.0, []) is None + # A term with no parseable name/slug is skipped, never guessed at. + assert resolve_thickness_band_id(1.0, [{"id": 1, "name": "mystery", "slug": "mystery"}]) is None + + +def test_resolve_is_id_order_independent(): + """The result must not depend on the sync order of the terms.""" + assert resolve_thickness_band_id(1.75, list(reversed(LIVE_TERMS))) == 351 diff --git a/backend/thickness_band.py b/backend/thickness_band.py new file mode 100644 index 0000000..75de5fa --- /dev/null +++ b/backend/thickness_band.py @@ -0,0 +1,177 @@ +"""Deterministic thickness band from free-text thickness_in (METADATA-15 / QUALITY-95). + +The owner enters thickness in 1/8-inch steps on capture. The Woo attribute is a +synced band term. We map thickness_in into the first half-open band [low, high) +that contains it, derived only from the live synced term names/slugs — never a +hardcoded Woo term id. + +Round-up semantics: bands are half-open on the upper bound, so a value that sits +on a shared boundary (e.g. 1.0 in) belongs to the band that *starts* at that +value (1"–1 1/2"), not the band that ends there (3/4"–1"). +""" + +from __future__ import annotations + +import re +from typing import Any, Iterable, Optional + +# Mixed number / fraction / decimal inch token, optional trailing quote. +_INCH_RE = re.compile( + r"(?P\d+)\s+(?P\d+)\s*/\s*(?P\d+)" # 1 1/2 + r"|(?P\d+)\s*/\s*(?P\d+)" # 1/2 + r"|(?P\d+(?:\.\d+)?)", # 1 or 1.5 + re.IGNORECASE, +) +_QTR_RANGE_RE = re.compile(r"^(\d+)-(\d+)qtr$", re.IGNORECASE) +_QTR_MIN_RE = re.compile(r"^(\d+)qtr$", re.IGNORECASE) + + +def parse_inch_token(text: str) -> Optional[float]: + """Parse one inch token like '1 1/2', '3/4', '2', or 2-with-quote → float inches.""" + t = (text or "").strip().rstrip("\"'").strip() + if not t: + return None + m = _INCH_RE.fullmatch(t) or _INCH_RE.search(t) + if not m: + return None + if m.group("a") is not None: + return float(m.group("a")) + float(m.group("n")) / float(m.group("d")) + if m.group("n2") is not None: + return float(m.group("n2")) / float(m.group("d2")) + if m.group("dec") is not None: + return float(m.group("dec")) + return None + + +def _bounds_from_name(name: str) -> Optional[tuple[float, float]]: + """Return half-open [low, high) inches from a Woo thickness term name.""" + raw = (name or "").strip() + low_s = raw.lower() + if not raw: + return None + + # Under X / less than X / < X + m = re.match(r"^(?:under|below|less than|<)\s*(.+)$", low_s, re.IGNORECASE) + if m: + hi = parse_inch_token(m.group(1)) + if hi is not None: + return (0.0, hi) + + # Over X / above X / > X / X+ + m = re.match(r"^(?:over|above|greater than|>)\s*(.+)$", low_s, re.IGNORECASE) + if m: + lo = parse_inch_token(m.group(1)) + if lo is not None: + return (lo, float("inf")) + m = re.match(r"^(.+?)\+\s*$", raw) + if m: + lo = parse_inch_token(m.group(1)) + if lo is not None: + return (lo, float("inf")) + + # Range: "A - B" / "A–B" / "A- B" (quotes optional on either side) + parts = re.split(r"\s*[-–—]\s*", raw, maxsplit=1) + if len(parts) == 2: + lo = parse_inch_token(parts[0]) + hi = parse_inch_token(parts[1]) + if lo is not None and hi is not None and hi > lo: + return (lo, hi) + + # Single value band treated as [v, v+epsilon) — rare + only = parse_inch_token(raw) + if only is not None: + return (only, only + 1e-9) + return None + + +def _bounds_from_slug(slug: str) -> Optional[tuple[float, float]]: + """Fallback: quarter-inch slugs like '4-6qtr', '12qtr', 'xthin'.""" + s = (slug or "").strip().lower() + if not s: + return None + if s in {"xthin", "under", "thin"}: + return (0.0, 0.5) + m = _QTR_RANGE_RE.match(s) + if m: + lo = int(m.group(1)) * 0.25 + hi = int(m.group(2)) * 0.25 + if hi > lo: + return (lo, hi) + m = _QTR_MIN_RE.match(s) + if m: + lo = int(m.group(1)) * 0.25 + return (lo, float("inf")) + return None + + +def term_bounds(term: dict[str, Any]) -> Optional[tuple[float, float]]: + """Bounds for one synced thickness term; name first, slug fallback.""" + return _bounds_from_name(str(term.get("name") or "")) or _bounds_from_slug( + str(term.get("slug") or "") + ) + + +def resolve_thickness_band_id( + thickness_in: float | int | None, + terms: Iterable[dict[str, Any]], +) -> Optional[int]: + """Pick the synced thickness term id for thickness_in, or None if unknown. + + terms: iterable of {id, name, slug?} from the live Woo thickness attribute. + """ + if thickness_in is None: + return None + try: + t = float(thickness_in) + except (TypeError, ValueError): + return None + if t < 0: + return None + + scored: list[tuple[float, float, int]] = [] # (low, high, id) + for term in terms: + b = term_bounds(term) + if b is None: + continue + tid = term.get("id") + if tid is None: + continue + lo, hi = b + scored.append((lo, hi, int(tid))) + + if not scored: + return None + + # Prefer the tightest band that contains t under half-open [low, high). + matches = [(hi - lo, lo, hi, tid) for lo, hi, tid in scored if lo <= t < hi] + if matches: + matches.sort() # tightest span first + return matches[0][3] + + # If nothing matched (gaps in taxonomy), pick nearest band by lower edge. + scored.sort(key=lambda x: x[0]) + for lo, hi, tid in scored: + if t < hi: + return tid + return scored[-1][2] + + +def load_thickness_terms(conn) -> list[dict[str, Any]]: + """Live Thickness attribute terms from the synced Woo cache.""" + row = conn.execute( + "SELECT id FROM woo_attributes WHERE lower(name)=? OR lower(slug)=?", + ("thickness", "thickness"), + ).fetchone() + if row is None: + # Case-insensitive name variants ("Thickness", "pa_thk" style slugs stay open) + row = conn.execute( + "SELECT id FROM woo_attributes WHERE lower(name) LIKE '%thickness%'" + ).fetchone() + if row is None: + return [] + attr_id = row[0] + rows = conn.execute( + "SELECT id, name, slug FROM woo_attribute_terms WHERE attribute_id=? ORDER BY id", + (attr_id,), + ).fetchall() + return [{"id": r[0], "name": r[1], "slug": r[2]} for r in rows] diff --git a/docs/REQUIREMENTS.md b/docs/REQUIREMENTS.md index d2ff6a1..4f962bb 100644 --- a/docs/REQUIREMENTS.md +++ b/docs/REQUIREMENTS.md @@ -199,7 +199,7 @@ The listing is built in two inference calls, both optional. Call 1 (vision) read | QUALITY-80 | Pipeline vector TV-6: 3:4 crop, 90 percent fill at extremes, transparent PNG, shorter side at least 800 when the source allows. | shipped | — | | QUALITY-85 | Pipeline vector TV-7: an undersized source warns, is not upscaled, and is not publishable until retake. | shipped | — | | QUALITY-90 | Pipeline vector TV-8: bdft 0.96 at 12.50 per bdft gives a 12.00 recommendation, and an override of 150.00 is stored. | M3 | — | -| QUALITY-95 | Pipeline vector TV-9: a 1 inch thickness rounds up into the synced Woo thickness band, not a hardcoded one. | M3 | — | +| QUALITY-95 | Pipeline vector TV-9: a 1 inch thickness rounds up into the synced Woo thickness band, not a hardcoded one. | M3 | backend/tests/test_thickness_band.py | | QUALITY-100 | Pipeline vector TV-10: half-up 2 decimal rounding, including 96.04 to 96, 0.641 to 0.64, 11.525 to 11.53, 12.345 to 12.35. | shipped | — | | QUALITY-105 | Dishonest proxies that do not count as pass: hardcoded or fixture taxonomy treated as synced, a Woo mock that never hits the real store for the exit walk, unit tests alone without a real SLAB-UAT-* draft, consumer-key auth, inference wired as dead config or required for create, claiming ready with empty fields, or parking work on the wrong branch. | M3 | — | | QUALITY-110 | The M3 exit evidence is a real SLAB-UAT-* Woo draft matching the reviewed ready set with inference off, plus test-woo and test-inference results. | M3 | — | diff --git a/docs/UAT-PRACTICE.md b/docs/UAT-PRACTICE.md index 21b0878..078804c 100644 --- a/docs/UAT-PRACTICE.md +++ b/docs/UAT-PRACTICE.md @@ -121,7 +121,9 @@ hard stop. Switch to a supported browser. 3. **Wood categories.** Pick at least one from the synced list. 4. **Edge type, figure, grade.** Pick from the synced lists. One edge type, at least one figure, exactly one grade. -5. **Thickness band.** Pick the Woo band that the measured thickness rounds up into. +5. **Thickness band.** Derived, not picked: the server bands the measured thickness up into + the synced Woo band and Review shows it read-only. Verify the shown band is the one the + captured thickness rounds up into (`QUALITY-95`). 6. **Moisture.** Keep the default (kiln-dried) or change it. 7. **fig-* tags.** Optional; pick any from the synced list. 8. **feat-* tags.** Zero or more. Only pick existing `feat-*` tags. diff --git a/docs/contracts/UX.md b/docs/contracts/UX.md index 8881014..f297846 100644 --- a/docs/contracts/UX.md +++ b/docs/contracts/UX.md @@ -12,7 +12,7 @@ Status: SPEC · 2026-09-18. Presentation spec for the mill-owner flow on phone a | CAPTURE-40, CAPTURE-41, CAPTURE-45, CAPTURE-50, CAPTURE-55 | sheet/sensitivity controls, overlay, retake | | CAPTURE-60, CAPTURE-65, CAPTURE-70, CAPTURE-80, CAPTURE-85, CAPTURE-90 | axis, entry, derived numbers | | CAPTURE-105, CAPTURE-130, CAPTURE-135, CAPTURE-141, CAPTURE-145, CAPTURE-150, CAPTURE-155 | crop guard, lineup order, upload, preview/controls, failure | -| METADATA-10, METADATA-15, METADATA-30 | required taxonomy fields and cardinality | +| METADATA-10, METADATA-15, METADATA-30, QUALITY-95 | required taxonomy fields and cardinality; the thickness band is derived from the measurement, not picked | | METADATA-45, METADATA-55, METADATA-110 | pre-fill, Generate text availability | | PUBLISH-5, PUBLISH-10, PUBLISH-15, PUBLISH-20, PUBLISH-25, PUBLISH-30, PUBLISH-35, PUBLISH-36, PUBLISH-37, PUBLISH-38, PUBLISH-50 | publish sequence, outcomes and messaging | | REVIEW-5, REVIEW-10, REVIEW-15, REVIEW-20, REVIEW-25, REVIEW-30, REVIEW-40, REVIEW-45, REVIEW-50 | ready set, gating, fields, limits | @@ -59,6 +59,9 @@ Cites REVIEW-5, REVIEW-15, REVIEW-20, METADATA-45. optional; price; title, short_description, and description. - **Primary category.** The owner declares one primary wood category and any secondary ones; the default is the first selected. +- **Thickness band is derived, not picked.** The measured thickness bands up into the synced Woo + band server-side (`METADATA-15`, `QUALITY-95`); Review shows the assigned band read-only and + offers no owner control for it. - **Inline nudges.** Every empty required field shows an inline nudge as the owner works; there is no single submit-time gap list. - **Calculated fields.** Widths, sqft, and bdft are read-only and not overridable. diff --git a/frontend/package.json b/frontend/package.json index 8c31b89..6e077c4 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -7,7 +7,7 @@ "dev": "vite", "build": "tsc --noEmit && vite build", "preview": "vite preview", - "test": "node --import tsx --test src/**/*.test.ts", + "test": "node --import tsx --test --test-reporter=spec src/**/*.test.ts", "test:e2e": "playwright test", "test:e2e:headed": "playwright test --headed" }, diff --git a/frontend/src/create/review/MultiSelect.svelte b/frontend/src/create/review/MultiSelect.svelte index 7f818ed..d59fd3b 100644 --- a/frontend/src/create/review/MultiSelect.svelte +++ b/frontend/src/create/review/MultiSelect.svelte @@ -9,7 +9,11 @@ // (METADATA-10: "parents group only and are never assigned"); only the // leaf rows are selectable. Filtering hides a parent group when none of // its leaves match. + // + // #90 closed state shows selected NAMES (not only a count). Done dispatches + // `commit` so the parent can persist + re-review. import { createEventDispatcher } from 'svelte'; + import { selectedSummary } from './woodPrimary'; export interface Opt { id: number; name: string; ai: boolean } export interface Group { parent: { id: number; name: string }; leaves: Opt[] } @@ -33,6 +37,7 @@ // Effective selectable set (leaves when grouped, options when flat). $: allOpts = (grouped ? groups.flatMap((g) => g.leaves) : options) as Opt[]; + $: nameById = new Map(allOpts.map((o) => [o.id, o.name] as [number, string])); $: q = filter.trim().toLowerCase(); $: filteredFlat = q ? allOpts.filter((o) => o.name.toLowerCase().includes(q)) : allOpts; // Grouped: keep a parent only if at least one of its leaves survives the filter. @@ -42,6 +47,7 @@ .filter((g) => g.leaves.length > 0) : []; $: count = selected.length; + $: summary = selectedSummary(selected, nameById, placeholder); $: allFilteredSelected = filteredFlat.length > 0 && filteredFlat.every((o) => selected.includes(o.id)); function toggle(id: number) { @@ -55,13 +61,17 @@ for (const o of [...filteredFlat]) if (selected.includes(o.id)) onToggle(o.id); } function close() { open = false; filter = ''; dispatch('commit'); } - function onKey(e: KeyboardEvent) { if (e.key === 'Escape' && open) open = false; } + function onKey(e: KeyboardEvent) { if (e.key === 'Escape' && open) { open = false; filter = ''; dispatch('commit'); } } + // Rows are focusable options (role=option): Enter/Space toggles, matching click. + function onRowKey(e: KeyboardEvent, id: number) { + if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); toggle(id); } + } -
+
@@ -85,9 +95,11 @@ class="ms-row leaf" class:on={selected.includes(o.id)} role="option" + tabindex="0" aria-selected={selected.includes(o.id)} data-testid={testid ? `${testid}-opt-${o.id}` : `ms-opt-${o.id}`} on:click={() => toggle(o.id)} + on:keydown={(e) => onRowKey(e, o.id)} > {o.name}{#if o.ai}AI{/if} @@ -104,9 +116,11 @@ class="ms-row" class:on={selected.includes(o.id)} role="option" + tabindex="0" aria-selected={selected.includes(o.id)} data-testid={testid ? `${testid}-opt-${o.id}` : `ms-opt-${o.id}`} on:click={() => toggle(o.id)} + on:keydown={(e) => onRowKey(e, o.id)} > {o.name}{#if o.ai}AI{/if} @@ -127,8 +141,9 @@