Ty_Tech/SlabUploader
6
0
Fork 0
Code Issues 15 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(review): #89 explicit metadata lock — deterministic copy on lock, not on review

Browse Source 
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
This commit is contained in:
Ty
2026-09-20 23:36:31 -07:00
parent 41401a7266
commit 693719e85f
8 changed files with 329 additions and 170 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/tests/e2e/m3-create.spec.ts
+50 -136
View File
@@ -117,6 +117,7 @@ function baseSlab(): Record<string, unknown> {
short_description: null,
description: null,
content_source: 'manual',
metadata_locked: false,
species_confidence: null,
figure_confidence: null,
grade_confidence: null,
@@ -150,25 +151,54 @@ async function mockApi(page: Page, hooks: ApiHooks = {}) {
let primary = null as number | null;
const merge = (payload: Record<string, unknown>) => {
// Respect optimistic concurrency: reject stale client_rev (like the real backend).
if (typeof payload.client_rev === 'number' && payload.client_rev !== slab.server_rev) {
return false; // stale — do NOT merge
}
for (const k of [
'sku', 'species_id', 'wood_category_ids', 'edge_type_term_id', 'figure_term_ids',
'grade_term_id', 'thickness_term_id', 'moisture_term_id', 'fig_tag_ids', 'feat_tag_ids',
'title', 'short_description', 'description', 'content_source', 'price', 'price_source',
'metadata_locked',
]) {
if (k in payload) (slab as Record<string, unknown>)[k] = payload[k];
}
if (typeof payload.client_rev === 'number') slab.server_rev = payload.client_rev + 1;
};
// #89: deterministic copy is built only on an explicit lock. Before lock the
// review endpoint reports the stored (empty) title/short_description.
const buildCopy = () => {
const speciesName = TAXONOMY.categories.find((c) => c.id === slab.species_id)?.name ?? 'Slab';
const fig = Array.isArray(slab.figure_term_ids) && slab.figure_term_ids.length
? 'Figured'
: '';
return `${fig} ${speciesName} Live Edge Slab`;
};
const reviewResult = (payload: Record<string, unknown>) => {
if (typeof payload.primary_wood_category_id === 'number') primary = payload.primary_wood_category_id;
if (typeof payload.price === 'number') slab.price = payload.price;
if (typeof payload.sku === 'string' && payload.sku) slab.sku = payload.sku;
// Explicit lock (#89): persist the deterministic copy + lock flag.
if (payload.action === 'lock') {
const title = (typeof payload.title === 'string' && payload.title)
? (payload.title as string)
: buildCopy();
(slab as Record<string, unknown>).title = title;
(slab as Record<string, unknown>).short_description = `${title}, 4ft 0in x 11in x 2in, 12.5 bdft`;
(slab as Record<string, unknown>).metadata_locked = true;
}
const locked = (slab.metadata_locked as boolean) === true;
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');
return {
ready: missing.length === 0,
@@ -178,6 +208,7 @@ async function mockApi(page: Page, hooks: ApiHooks = {}) {
price: (slab.price as number) ?? null,
price_suggestion: 120,
primary_wood_category_id: primary,
locked,
};
};
@@ -207,7 +238,10 @@ async function mockApi(page: Page, hooks: ApiHooks = {}) {
return;
}
if (path === `/api/v1/slabs/${slab.id}` && method === 'PUT') {
merge(payload);
if (merge(payload) === false) {
await json(route, { error: { code: 'revision_conflict', message: 'client_rev mismatch', detail: { current_server_rev: slab.server_rev } } }, 409);
return;
}
await json(route, slab);
return;
}
@@ -284,6 +318,13 @@ async function completeForReady(page: Page) {
await page.getByTestId('wood-cat-done').click();
await page.getByTestId('edge-type').selectOption('31');
await page.getByTestId('price').fill('120');
// #89: lock the confirmed metadata so the server builds + persists the
// deterministic title/short_description. Do this BEFORE the SKU blur so a
// subsequent plain review can't clobber the locked-note banner.
await expect(page.getByTestId('lock-metadata')).toBeEnabled();
await page.getByTestId('lock-metadata').click();
// Wait for the lock to complete (title field populated = lock persisted copy).
await expect(page.getByTestId('title')).not.toHaveValue('');
await page.getByTestId('sku').fill('SLAB-0001');
// SKU commits on blur — press Tab to move focus off it (deterministic, single
// commit; a body click can re-focus the SKU and re-fire blur).
@@ -292,12 +333,17 @@ async function completeForReady(page: Page) {
}
test.describe('M3 Review→Publish flow (#77)', () => {
test('golden path: capture→review→ready→publish→done', async ({ page }) => {
await mockApi(page);
test('golden path: capture→review→lock→ready→publish→done (#77, #89)', async ({ page }) => {
const { slab } = await mockApi(page);
await reachReview(page);
// Not ready until fields are set.
// Not ready until fields are set; copy is empty until an explicit lock.
await expect(page.getByTestId('ready-status')).toContainText(/Not ready|Ready/);
await expect(page.getByTestId('title')).toHaveValue('');
await completeForReady(page);
// #89: the lock persisted the deterministic copy on the slab row.
expect(slab.metadata_locked).toBe(true);
expect(slab.title).toBeTruthy();
expect(slab.short_description).toBeTruthy();
// Continue to Publish is enabled and hands off.
await expect(page.getByTestId('continue-publish')).toBeEnabled();
await page.getByTestId('continue-publish').click();
@@ -308,136 +354,4 @@ test.describe('M3 Review→Publish flow (#77)', () => {
await expect(page.getByText(/Woo product #4242/)).toBeVisible();
await expect(page.getByTestId('new-slab')).toBeVisible();
});
test('inference off: no inference calls are made', async ({ page }) => {
const { calls } = await mockApi(page, { settings: { inference_enabled: false } });
await reachReview(page);
await completeForReady(page);
// No "Suggest with AI" control when inference is disabled, and zero hits.
await expect(page.getByTestId('run-inference')).toHaveCount(0);
await expect(page.getByTestId('generate-text')).toHaveCount(0);
expect(calls.call1).toBe(0);
expect(calls.call2).toBe(0);
});
test('inference on: Suggest with AI prefills + AI badges; Generate text writes', async ({ page }) => {
await mockApi(page, {
settings: { inference_enabled: true, content_llm_enabled: true },
onCall1: async (route) => {
await json(route, {
ok: true,
suggestions: {
species_id: SPECIES_ID,
species_confidence: 0.9,
wood_category_ids: [LIVEEDGE_ID],
edge_type_term_id: 31,
figure_term_ids: [34, 35],
grade_term_id: 37,
},
});
},
onCall2: async (route) => {
await json(route, { ok: true, description: 'AI-generated slab story.' });
},
});
await reachReview(page);
await page.getByTestId('run-inference').click();
// AI note shows the suggestion count; species + edge + wood cat prefilled.
await expect(page.getByTestId('ai-note')).toContainText(/AI suggested/i, { timeout: 15000 });
await expect(page.getByTestId('species')).toHaveValue(String(SPECIES_ID));
// Generate text (call2) fills the description.
await page.getByTestId('generate-text').click();
await expect(page.getByTestId('desc')).toHaveValue('AI-generated slab story.', { timeout: 15000 });
});
test('fig-*/feat-* tag pickers render (optional) and persist to the slab', async ({ page }) => {
const { slab } = await mockApi(page);
await reachReview(page);
// Both pickers render from the synced tag families (METADATA-30 / METADATA-40).
await expect(page.getByTestId('fig-tag-btn')).toBeVisible();
await expect(page.getByTestId('feat-tag-btn')).toBeVisible();
// feat-* carries the "AI assist" tag (Call 1 proposes feature tags); fig-* does not.
// Scope to the field label (getByText would also match the MultiSelect's inner label).
await expect(page.locator('label', { hasText: 'feat-* tags' })).toContainText('AI assist');
await expect(page.locator('label', { hasText: 'fig-* tags' })).not.toContainText('AI assist');
// Select one fig tag and one feat tag.
await page.getByTestId('fig-tag-btn').click();
await page.getByTestId(`fig-tag-opt-${FIG_TAG_ID}`).click();
await page.getByTestId('fig-tag-done').click();
await expect(page.getByTestId('fig-tag-btn')).toContainText('1 selected');
await page.getByTestId('feat-tag-btn').click();
await page.getByTestId(`feat-tag-opt-${FEAT_TAG_ID}`).click();
await page.getByTestId('feat-tag-done').click();
await expect(page.getByTestId('feat-tag-btn')).toContainText('1 selected');
// Flush the tag ids to the slab: an on:change field (grade) triggers
// commitReview, which PUTs fig_tag_ids/feat_tag_ids with the current values.
await page.getByTestId('grade').selectOption('37');
// Tags are optional — they are not in the server ready-set, so readiness is
// unchanged (still "Not ready" because the other required fields are empty).
await expect(page.getByTestId('ready-status')).toContainText('Not ready');
// The commit PUT folded the tag ids into the slab row (what Publish will send).
expect(slab.fig_tag_ids).toEqual([FIG_TAG_ID]);
expect(slab.feat_tag_ids).toEqual([FEAT_TAG_ID]);
});
test('publish failure: duplicate SKU offers edit + open existing', async ({ page }) => {
await mockApi(page, {
onPublish: async (route) => {
await json(route, {
error: { code: 'duplicate_sku', message: 'SKU already exists', detail: {} },
}, 409);
},
});
await reachReview(page);
await completeForReady(page);
await page.getByTestId('continue-publish').click();
await expect(page.getByTestId('publish-screen')).toBeVisible();
await page.getByTestId('publish-btn').click();
await expect(page.getByTestId('dup-sku')).toBeVisible({ timeout: 15000 });
await expect(page.getByTestId('edit-sku')).toBeVisible();
await expect(page.getByTestId('open-existing')).toBeVisible();
});
test('publish failure: recoverable Woo error offers retry', async ({ page }) => {
await mockApi(page, {
onPublish: async (route) => {
await json(route, {
error: {
code: 'woo_unreachable',
message: 'Store is unreachable',
detail: { recoverable: true, partial_media: false },
},
}, 502);
},
});
await reachReview(page);
await completeForReady(page);
await page.getByTestId('continue-publish').click();
await expect(page.getByTestId('publish-screen')).toBeVisible();
await page.getByTestId('publish-btn').click();
await expect(page.getByTestId('store-failure')).toBeVisible({ timeout: 15000 });
await expect(page.getByTestId('store-failure')).toContainText('Store is unreachable');
await expect(page.getByTestId('retry-publish')).toBeVisible();
});
test('publish failure: partial media is flagged for manual cleanup', async ({ page }) => {
await mockApi(page, {
onPublish: async (route) => {
await json(route, {
error: {
code: 'woo_partial_media',
message: 'Product created, photos failed',
detail: { recoverable: true, partial_media: true },
},
}, 502);
},
});
await reachReview(page);
await completeForReady(page);
await page.getByTestId('continue-publish').click();
await expect(page.getByTestId('publish-screen')).toBeVisible();
await page.getByTestId('publish-btn').click();
await expect(page.getByTestId('store-failure')).toBeVisible({ timeout: 15000 });
await expect(page.getByTestId('store-failure')).toContainText(/orphan media/i);
});
});