diff --git a/backend/db/__init__.py b/backend/db/__init__.py index ff39271..471905e 100644 --- a/backend/db/__init__.py +++ b/backend/db/__init__.py @@ -55,7 +55,6 @@ _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 c0fefd3..0aa229d 100644 --- a/backend/db/schema.sql +++ b/backend/db/schema.sql @@ -35,9 +35,6 @@ 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 286bce8..c3cff56 100644 --- a/backend/review.py +++ b/backend/review.py @@ -155,65 +155,15 @@ 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) - 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. - # #80: persist owner overrides (price/sku/title/short_description) from the - # payload so the review endpoint is self-contained — the client doesn't - # need a separate PUT for each override. #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. + # --- Deterministic review: report ready/missing + the deterministic + # title/short_description built from the current locked metadata. The client's + # "Create title" / "Create short desc" buttons call this endpoint and write the + # returned strings into the fields (item 1: no lock action, no metadata_locked). + # #80: persist owner overrides (price/sku) from the payload so the review + # endpoint is self-contained. title/short_description are NOT overridden here — + # they are produced by the deterministic build below, not persisted from the + # payload (so the review POST can't clobber the owner's Create workflow). cats, terms = _taxonomy_lookup(conn) primary_id = payload.get("primary_wood_category_id") wood_cats = slab.get("wood_category_ids") or [] @@ -224,9 +174,9 @@ 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 - # #80: persist owner overrides from the payload. + # #80: persist owner overrides from the payload (price/sku only). overrides: dict[str, Any] = {} - for k in ("price", "sku", "title", "short_description"): + for k in ("price", "sku"): if k in payload and payload[k] is not None: overrides[k] = payload[k] if overrides: @@ -238,20 +188,17 @@ async def review_slab(slab_id: str, payload: dict[str, Any] | None = None) -> di conn.commit() slab.update(overrides) - 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) + # Deterministic title / short_description, re-derived from the current + # metadata on every review (the Create button's source of truth). + title = build_title(slab, figure, species_name, primary_leaf) if (primary_leaf and species_name) else slab.get("title") or None + short_desc = build_short_description(slab, figure, species_name, primary_leaf) if (primary_leaf and species_name) else slab.get("short_description") or None 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, + "primary_wood_category_id": primary_id, } diff --git a/backend/slabs.py b/backend/slabs.py index 81d3cac..7ea73ed 100644 --- a/backend/slabs.py +++ b/backend/slabs.py @@ -65,9 +65,10 @@ 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"} +# #89: server-managed fields that a plain client PUT must not set directly. +# (metadata_locked was dropped — the deterministic title/short-desc are built by +# the review endpoint and written back by the client's Create buttons.) +CLIENT_FORBIDDEN_FIELDS = set() def _row_to_slab(row) -> Dict[str, Any]: @@ -78,8 +79,6 @@ 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 diff --git a/backend/tests/test_review.py b/backend/tests/test_review.py index 74fb9f3..ba38aeb 100644 --- a/backend/tests/test_review.py +++ b/backend/tests/test_review.py @@ -55,19 +55,14 @@ 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() - # #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"}) + # #89: the review always returns the deterministic title/short_description + # built from the current metadata (the Create button's source of truth). + r = env[0].post("/api/v1/slabs/s1/review", json={}) 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): @@ -141,103 +136,54 @@ 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() - # #89: build happens on lock. - r = client.post("/api/v1/slabs/s1/review", json={"action": "lock"}) + # #89: the deterministic build runs on every review. + r = client.post("/api/v1/slabs/s1/review", json={}) 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.""" +def test_review_always_reports_deterministic_title(env): + """#89: the review always returns the deterministic title/short_description + built from the current metadata. The client's Create button writes these into + the fields; the server does not persist them from the review payload.""" 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" + assert body["short_description"].startswith("Figured Black Walnut Live Edge Slab,") + # The review does NOT persist title/short_description from the payload. 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 + assert got["title"] is None + assert got["short_description"] is None -def test_lock_requires_species(env): +def test_review_no_species_reports_stored_title(env): + """With no species, there is no deterministic pattern; the review reports the + stored title (None here) and lists title as missing.""" 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.", - }) + r = client.post("/api/v1/slabs/s1/review", json={}) 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" + assert body["title"] is None + assert "title" in body["missing"] def test_review_persists_owner_overrides(env): - """#80: regular review (no lock) persists price/sku/title/short_description - overrides from the payload to the DB, so the client doesn't need a separate - PUT for each override.""" + """#80: regular review persists price/sku overrides from the payload to the DB, + so the client doesn't need a separate PUT for each. (title/short_description + are NOT persisted here — they come from the deterministic build, not the payload.)""" 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() - # Review with owner overrides for price + sku + title + short_description. + # Review with owner overrides for price + sku. r = client.post("/api/v1/slabs/s1/review", json={ "price": 250.0, "sku": "SLAB-99", - "title": "Owner Title", - "short_description": "Owner Short Desc", }) assert r.status_code == 200 body = r.json() @@ -246,7 +192,5 @@ def test_review_persists_owner_overrides(env): got = client.get("/api/v1/slabs/s1").json() assert got["price"] == 250.0 assert got["sku"] == "SLAB-99" - assert got["title"] == "Owner Title" - assert got["short_description"] == "Owner Short Desc" # server_rev was incremented (the UPDATE fired). assert got["server_rev"] == 2 diff --git a/deploy/INVENTORY.md b/deploy/INVENTORY.md index 722a341..47d8974 100644 --- a/deploy/INVENTORY.md +++ b/deploy/INVENTORY.md @@ -51,3 +51,28 @@ Do not put a host name in the product spec. - Live verification: served HTML and bundle contain zero occurrences of "Thickness band" or "set from the measurement". - Sample data unchanged (still 25 slabs post-recreate). - All gates (unit, build, requirements, coverage, m3-create e2e) pass. + +### 2026-09-21 11:52 PDT — merge `issue-89-metadata-lock` → main (Create-string model) + +- Source: branch `work/merge-89-90` (base `origin/issue-89-metadata-lock` + Create-string re-applied). + `matches HEAD`: **no** — staged from the uncommitted working tree (local sync; Gitea push not + attempted this round). 9 files changed: Review.svelte, slabs.ts, review.py, slabs.py, schema.sql, + db/__init__.py, test_review.py, m3-create.spec.ts, review-photo-strip.spec.ts. +- Model change: `metadata_locked` DB column + "Save and lock" UI **dropped end-to-end**. The + deterministic title/short_description are now built by the review endpoint on every call; the + owner's **Create title / Create short desc** buttons fire the build and write the result + (overwrite contract). `commitReview` auto-fills only empty fields (no owner-clobber); `createTitle` + is the explicit force-overwrite. Photo strip, wood primary/secondary, and server-side thickness + band (item: not shown in UI, passed to Woo on publish) all taken from the branch. +- LAN binds: `deploy/override-m3.yml` was absent from disk; the live stack's 10881/10882 LAN binds + were re-created by editing `deploy/docker-compose.yml` ports in-place (loopback 18082/18083 from + `.env` + `0.0.0.0:10881`/`0.0.0.0:10882`), then `down --remove-orphans` + `up -d` (no `-v`). +- Bundle: `assets/index-YPAqx609.js` (new; contains `create-title`). API `/api/health` + `woo_reachable:true`. +- Sample data preserved: **yes** — 34 slabs before/after. +- Live verification (LAN URL, real DB): `slabs` table has **no** `metadata_locked` column; + `slab_photos.role` present. `POST /review` on slab `ec4088da…` → `title:'Curl Black Walnut Slab'`, + `ready:True`. `PUT` title persisted (rev 1→2) then restored (original sample value intact, rev 3). +- Not exercised live: the full browser Create-button click (verified via in-repo Playwright 6/6 + m3-create + review-photo-strip; the endpoint + PUT path are proven live above). Woo publish path + not exercised this round. diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index 7e710e9..e4f4867 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -16,7 +16,9 @@ services: build: context: .. dockerfile: frontend/Dockerfile - ports: ["127.0.0.1:${FRONTEND_PORT:-18080}:80"] + ports: + - "127.0.0.1:${FRONTEND_PORT:-18080}:80" + - "0.0.0.0:10881:80" depends_on: [fastapi] restart: unless-stopped @@ -34,7 +36,9 @@ services: volumes: - slab_db:/data - slab_images:/images - ports: ["127.0.0.1:${FASTAPI_PORT:-18081}:8000"] + ports: + - "127.0.0.1:${FASTAPI_PORT:-18081}:8000" + - "0.0.0.0:10882:8000" mem_limit: 2g restart: unless-stopped diff --git a/frontend/src/api/slabs.ts b/frontend/src/api/slabs.ts index 0e5a7ed..4b5bc4d 100644 --- a/frontend/src/api/slabs.ts +++ b/frontend/src/api/slabs.ts @@ -39,8 +39,6 @@ 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; @@ -271,8 +269,6 @@ 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 { @@ -284,8 +280,6 @@ 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 { @@ -296,13 +290,6 @@ 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 a6274cc..2fa1d4d 100644 --- a/frontend/src/create/review/Review.svelte +++ b/frontend/src/create/review/Review.svelte @@ -1,7 +1,7 @@