From 4362eb42019a891aaba2d50f34d38fbcfdedd1b1 Mon Sep 17 00:00:00 2001 From: Dave Date: Thu, 5 Nov 2026 22:00:29 +0200 Subject: [PATCH] docs(spec,plan): a failed write is its own status Reporting a failed write as an unmatched line tells the operator they typed the SKU wrong when the cause was a transient database error. That sends them hunting for a typo instead of retrying - different problems, different responses. Also records the dry-run fix: on a dry run a matched line is reported applied with its before and after numbers, and nothing is written. --- ...026-11-05-giellie-stock-quantity-update.md | 31 ++++++++++++++++++- ...05-giellie-stock-quantity-update-design.md | 4 ++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/plans/2026-11-05-giellie-stock-quantity-update.md b/docs/superpowers/plans/2026-11-05-giellie-stock-quantity-update.md index 9b69855..b071ac9 100644 --- a/docs/superpowers/plans/2026-11-05-giellie-stock-quantity-update.md +++ b/docs/superpowers/plans/2026-11-05-giellie-stock-quantity-update.md @@ -601,6 +601,33 @@ The PocketBase work: fetch candidates, resolve, apply through the audited servic **Note on the two `normaliseSku` helpers.** `@/lib/stock-match` strips whitespace and punctuation and uppercases; `@/lib/server/price-list-match` strips everything that is not A-Z or 0-9 and uppercases. They produce the same string. The resolver uses the first, the catalogue lookup uses the second, because `price_list.sku_normalised` was written with the second. +### Task 3 amendments (operator decision 2026-11-05, plus implementation findings) + +The steps below are the original task text. Four changes were decided after it was first implemented. **They govern.** The original Step 3 code is superseded on these points; the original Step 1 test list is superseded by the list here. + +**Amendment 1 — a dry run reports the plan, not a write failure.** The original snippet fell through to the "write did not complete" branch for every matched line on a dry run, because a dry run produces no write results. That contradicted the brief's own dry-run test, which expects `applied === 1` plus the before/after numbers. + +- On a dry run, a matched line is reported `applied` with its `previous_qty`, `new_qty` and `delta`, and **nothing is written** — no ledger row, no `stock_items` update, no `price_list` update. + +**Amendment 2 — a failed write is its own status.** The operator decided that a write failure must not be reported as an unmatched line: telling the operator "you typed the SKU wrong" when the cause was a transient PocketBase error sends them hunting for a typo instead of retrying. + +- `StockQuantitiesBulkRowStatus` gains `"failed"`, and the summary gains a `failed: number` count. +- When the write result row comes back `ok === false` (or is missing), the result row's `status` is `"failed"`, the write service's `message` is carried through unchanged, and the row is counted under `summary.failed`. +- `summary.unmatched` keeps its meaning: the line never resolved to an active stock item — either nothing matched, or the best match was inactive. The `message` already distinguishes those two, and they have the same operator response. +- The five counts plus `failed` still sum to `summary.total`. + +**Amendment 3 — the result row carries a status from the start.** The `base` row object in the original snippet omitted `status`, which TypeScript requires on `StockQuantitiesBulkResultRow`. The base row carries `status: "unmatched"` and each branch overwrites it. + +**Amendment 4 — the price-list mirror reads one page, by design.** The mirror reads a single 200-row page of `price_list`. The catalogue is ~2,907 rows over ~2,739 SKUs and holds one row per SKU **per supplier**, so a SKU realistically has one to three catalogue rows. The single page is unbounded at this data volume, and the code says so in a comment so the next reader does not mistake it for an oversight. + +**Amendment 5 — test list.** The test file must cover, in addition to the original tests: + +- a dry run that writes nothing at all — no ledger row, no stock item update, no catalogue update — while still reporting the full before/after plan; +- a line whose write fails: reported `status: "failed"` with the write failure's reason in `message`, counted under `summary.failed`, while the rest of the batch still applies; +- a batch covering all five buckets at once — applied, unchanged, unmatched, ambiguous, failed — asserting the whole summary object and that the five counts sum to `total`. + +The original Step 3 code also needed a small mechanical correction: the test stub's `collection()` union is not assignable to `StockQuantitiesBulkPb`, so the stub is cast once at its source, matching the repo's existing `pb as never` pattern. + - [ ] **Step 1: Write the failing test** Create `src/lib/server/stock-quantities-bulk-service.test.ts`: @@ -879,7 +906,8 @@ export type StockQuantitiesBulkRowStatus = | "applied" | "unchanged" | "unmatched" - | "ambiguous"; + | "ambiguous" + | "failed"; export type StockQuantitiesBulkResultRow = { sku: string; @@ -907,6 +935,7 @@ export type StockQuantitiesBulkResult = { unchanged: number; unmatched: number; ambiguous: number; + failed: number; }; results: StockQuantitiesBulkResultRow[]; }; diff --git a/docs/superpowers/specs/2026-11-05-giellie-stock-quantity-update-design.md b/docs/superpowers/specs/2026-11-05-giellie-stock-quantity-update-design.md index b1c5d47..772490a 100644 --- a/docs/superpowers/specs/2026-11-05-giellie-stock-quantity-update-design.md +++ b/docs/superpowers/specs/2026-11-05-giellie-stock-quantity-update-design.md @@ -118,7 +118,7 @@ The write goes through `updateStockItemsQuantities` (`src/lib/server/stock-quant "ok": true, "mode": "set", "dry_run": false, - "summary": { "total": 12, "applied": 9, "unchanged": 1, "unmatched": 1, "ambiguous": 1 }, + "summary": { "total": 12, "applied": 9, "unchanged": 1, "unmatched": 1, "ambiguous": 0, "failed": 1 }, "results": [ { "sku": "ABC-123", "id": "abc…", "name": "Bolt M12 x 60", "match_method": "sku_exact", @@ -135,6 +135,8 @@ The write goes through `updateStockItemsQuantities` (`src/lib/server/stock-quant Per-row independence: one bad line never stops the others. +A **failed write is its own status**, not an unmatched line. Telling the operator "you typed the SKU wrong" when the cause was a transient database error sends them hunting for a typo instead of retrying — different problems, different responses. `unmatched` keeps its meaning: the line never resolved to an active stock item (nothing matched, or the best match was inactive — the message says which). + ### 2. The one service change — negatives `updateStockItemsQuantities` currently rejects a negative `current_qty`: