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.
This commit is contained in:
Dave 2026-11-05 22:00:29 +02:00
parent 23725f10c3
commit 4362eb4201
2 changed files with 33 additions and 2 deletions

View file

@ -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[];
};

View file

@ -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`: