From 4d95b7af8cfad63e837f9c5453cf7a5d43a47168 Mon Sep 17 00:00:00 2001 From: Dave Date: Sun, 30 Aug 2026 18:28:44 +0200 Subject: [PATCH] docs: non-invoiceable extras design + plan, quantity-handling audit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three documents behind the two features on this branch: - specs/2026-08-30-non-invoiceable-order-extras-design.md — the design the implementation followed, including why the flag must be named for the exception (a positively-named field defaults to false and would mark every existing row non-billable the moment it is created). - plans/2026-08-30-non-invoiceable-order-extras.md — the task-by-task plan, now fully implemented. - specs/2026-08-30-quantity-handling-audit.md — the order_qty / quantity audit that produced the preceding fix, plus the partial collection findings that are still open. Co-Authored-By: Claude Opus 5 --- ...2026-08-30-non-invoiceable-order-extras.md | 260 ++++++++++++++++++ ...-30-non-invoiceable-order-extras-design.md | 202 ++++++++++++++ .../2026-08-30-quantity-handling-audit.md | 203 ++++++++++++++ 3 files changed, 665 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-30-non-invoiceable-order-extras.md create mode 100644 docs/superpowers/specs/2026-08-30-non-invoiceable-order-extras-design.md create mode 100644 docs/superpowers/specs/2026-08-30-quantity-handling-audit.md diff --git a/docs/superpowers/plans/2026-08-30-non-invoiceable-order-extras.md b/docs/superpowers/plans/2026-08-30-non-invoiceable-order-extras.md new file mode 100644 index 0000000..fd0244f --- /dev/null +++ b/docs/superpowers/plans/2026-08-30-non-invoiceable-order-extras.md @@ -0,0 +1,260 @@ +# Non-Invoiceable Order Extras — Implementation Plan + +> **For agentic workers:** Use `superpowers:subagent-driven-development` or +> `superpowers:executing-plans` to implement this plan task-by-task. Steps use +> checkbox (`- [ ]`) syntax for tracking. + +**Goal:** The operator can add extra line items to an order at any stage and +mark them "don't invoice". Such lines get POs, drivers and collection tracking +like any other line, and count toward order completion — but never appear on a +customer invoice or delivery note. + +**Spec:** `docs/superpowers/specs/2026-08-30-non-invoiceable-order-extras-design.md` + +**Tech Stack:** Next.js 16 (App Router), TypeScript, PocketBase (via +`getAdminPocketBase()`), Vitest, Tailwind + shadcn/ui, lucide-react. + +--- + +## Pre-flight: verified before this plan was written + +Do not re-investigate these. + +- **The n8n QuickBooks workflow needs no changes.** Its "Build PocketBase + Invoice Update" node maps over `order_item_ids` and patches each row with + `item_status: 'invoiced'` + `order_item_details.invoice`. The QuickBooks + lines come from the separate `items` array. Verified in + `n8n_workfows/1 - WFDASH QUICKBOOKS INVOICE1 - WFDASH QUICKBOOKS INVOICE.json`. +- **Nothing in the Next app flips `item_status` to `"invoiced"`.** That write + is n8n's alone. Do not add an app-side flip. +- **Both picking-slip routes need no changes.** They query PB by document + identity (`print-bundle/route.ts:88`, `delivery-note-print/route.ts:92`), not + by the submitted row list, so excluded rows are picked up automatically. + Task 8 adds a regression test to lock this in. +- **`selling_price` has `min: 0`** in the PB schema, so `0` is a legal value. +- **Tests:** `npm run test:run`. Dev server is on **port 4001**, not 3000. +- **`vitest.config.ts` has no `clearMocks`** — mock history leaks between + tests. Reset explicitly or you get green-but-vacuous tests. + +## File structure + +| File | Responsibility | +|---|---| +| PocketBase admin *(manual)* | New `order_items.exclude_from_invoice` bool. **Task 0 — blocks everything.** | +| `src/lib/types.ts` *(modify)* | `OrderItem.exclude_from_invoice?: boolean`. | +| `src/lib/server/orders-service.ts` *(modify)* | Drop the pending gate; accept the flag in the create draft. | +| `src/app/(app)/orders/[id]/process/page.tsx` *(modify)* | Add-item form; margin-gate bypass; excluded-row styling. | +| `src/app/(app)/invoicing/view-model.ts` *(modify)* | Split `order_item_ids` from `items` in `buildQbInvoicePayload`. | +| `src/app/(app)/invoicing/page.tsx` *(modify)* | Muted row + locked checkbox + all-excluded guard, in **both** modals. | +| `src/app/api/invoicing/delivery-note/route.ts` *(modify)* | Excluded rows flip status but drop out of `lines:`. | +| `src/app/api/mcp/route.ts` *(modify)* | `order_items.create` accepts the flag. | + +## Phase boundaries + +- **Phase 1 — Tasks 0–4.** Creating and pricing an excluded line. Ends with the + operator able to add extras that survive the whole operational flow. +- **Phase 2 — Tasks 5–9.** Keeping them off customer paperwork. Depends on + Phase 1. + +--- + +## Task 0 — PocketBase field (BLOCKS EVERYTHING) + +**This is a manual step the user performs in the PB admin UI.** Per `AGENTS.md`, +schema changes to PocketBase are not self-service; the API returns an opaque +400/500 if code writes a field that doesn't exist yet. + +- [ ] Ask the user to add to the `order_items` collection: + - Name: `exclude_from_invoice` + - Type: **Bool** + - Required: **no** +- [ ] Confirm it exists before starting Task 1. Do not proceed on assumption. + +**Do not invert the name.** A positively-named flag (`bill_customer`, +`invoiceable`) defaults to `false` and would mark every existing row in the +database non-billable the moment the field is created. + +Optionally refresh `pb-exports/pb_schema-2.json`, but remember it is a +reference snapshot — the running instance is the source of truth. + +## Task 1 — Type the field + +- [ ] Add to the `OrderItem` interface in `src/lib/types.ts` (interface starts + at line 782), near `surplus_qty` / `ready_for_collection`: + +```ts +/** + * When true this line never reaches a customer-facing document. It + * still gets a PO, a driver, a collection and a `delivered` status — + * it is simply dropped from the QuickBooks `items` array and the + * delivery-note PDF lines. Used for supplier-side component splits: + * the customer approved "bolt, nut and washer" as one priced line, + * but we must raise three PO lines to actually buy it. + * + * Named for the exception, not the norm: the PB default of `false` + * must mean "bill this normally". + */ +exclude_from_invoice?: boolean; +``` + +- [ ] `npx tsc --noEmit` passes. + +## Task 2 — Allow adding lines mid-flight + +`src/lib/server/orders-service.ts`. + +- [ ] Add `exclude_from_invoice?: boolean` to `CreateOrderItemForOrderDraft` + (line 3867). +- [ ] **Delete the pending gate** in `createOrderItemForOrder` — the + `nonPending` query and the `if (nonPending.length > 0)` throw at line + 3939. Replace the "Strict `pending` gate" doc comment above the function + with an explanation of why adding a row alongside an in-flight PO is + safe: the new row doesn't mutate the existing PO, it gets its own. +- [ ] There is a second, unrelated `nonPending` gate at line 3744 in + `setOrderQuote`. **Leave it alone.** +- [ ] In the secondary-field patch block, forward the flag. When + `exclude_from_invoice` is true, force `selling_price` to `0` regardless + of what the caller passed — the flag is authoritative. +- [ ] Keep the `orderLocked` field in the return type for API compatibility; + it is now always `false`. + +**Tests** in `orders-service.test.ts`: + +- [ ] Adding a line to an order whose other items are `po_generated` succeeds + (previously threw 409). +- [ ] `exclude_from_invoice: true` persists and forces `selling_price` to 0 + even when the caller passes a non-zero price. +- [ ] Omitting the flag leaves it falsy and preserves existing behaviour. + +## Task 3 — Add-item UI on the process page + +`src/app/(app)/orders/[id]/process/page.tsx`. The empty-state copy at line +1684 ("No line items on this order. Add items before processing.") currently +points at an action that doesn't exist — this task makes it true. + +- [ ] Add an "Add item" button below the item card stack, opening a small form + (reuse the existing `Sheet` pattern used by `PriceEditSheet`). +- [ ] Fields: description (required), quantity, cost price, selling price, + supplier, notes. +- [ ] A `Checkbox` labelled **"Don't invoice this line"**, with helper text: + *"Counts for the order and gets collected, but never appears on the + customer's invoice or delivery note."* +- [ ] When ticked: force the selling-price input to `0`, disable it, and show + "Not invoiced" in place of the margin chip. +- [ ] POST to the existing per-id create surface; refresh `items` on success. +- [ ] Existing rows with `exclude_from_invoice` render with a muted "Not + invoiced" badge in the card stack. + +**Verification:** dev server on **port 4001**. Add an extra to an order that +already has a `po_generated` line and confirm it saves without a 409. + +## Task 4 — Bypass the margin gate (HIGHEST RISK) + +`calculateOrderItemMarkup(cost, 0)` returns `-100`, well under +`MARKUP_APPROVAL_THRESHOLD` (22, declared at `process/page.tsx:246`). Without +this task every extra lands in `approval_needed` and wedges the order. + +- [ ] At `process/page.tsx:643`, where the save path tests + `computedMarkup < MARKUP_APPROVAL_THRESHOLD`, short-circuit when the item + has `exclude_from_invoice` — skip the approval request entirely and save + straight through. +- [ ] Suppress the low-margin colouring for excluded rows (`LOW_MARGIN_THRESHOLD` + at line 247) — a red "-100%" chip on every extra is noise. +- [ ] Also guard the server side: `requestOrderItemMarginApproval` + (`src/lib/server/order-item-margin-approval-service.ts:128`) should + refuse with a clear error if asked to gate an excluded item, so the MCP + path can't wedge an order either. + +**Tests:** + +- [ ] An excluded item with cost 50 / selling 0 saves without entering + `approval_needed`. +- [ ] A **non**-excluded item with the same numbers still triggers approval — + proves the bypass is keyed on the flag, not on the price being zero. + +## Task 5 — Split the QuickBooks payload + +`src/app/(app)/invoicing/view-model.ts`. + +- [ ] Add `excludeFromInvoice?: boolean` to `InvoicingRow` (line 4). +- [ ] In `buildQbInvoicePayload` (line 393), keep `order_item_ids` built from + **all** rows, and build `items` from **only** rows where + `excludeFromInvoice` is not true. +- [ ] Comment the split at the call site — the asymmetry looks like a bug + otherwise. State that `order_item_ids` drives n8n's status write and + `items` drives the QuickBooks lines. +- [ ] Export a small helper (e.g. `billableRows(rows)`) so the modals and the + guard in Task 6 share one definition of "billable". +- [ ] In `src/app/(app)/invoicing/page.tsx`, map the new PB field into + `InvoicingRow`: add `exclude_from_invoice` to both the `RawInvoicingRow` + type (line 42) and the `INVOICING_FIELDS` projection (line 77). **The + projection is load-bearing** — omit it and PB returns the field as + `undefined`, silently invoicing every extra. + +**Tests** in `view-model.test.ts`: + +- [ ] Mixed set: excluded ids present in `order_item_ids`, absent from `items`. +- [ ] All-billable set produces identical output to today (regression). +- [ ] All-excluded set produces an empty `items` array, so Task 6's guard has + something to detect. + +## Task 6 — Invoicing UI + +`src/app/(app)/invoicing/page.tsx`. **There are two modals** — single-order +(~line 576) and bulk (~line 965) — each with its own `includedRowIds` state. +Apply every change to both. + +- [ ] Excluded rows render muted, with a "Not invoiced" chip. +- [ ] Their include-checkbox is checked and **disabled** — the row must ride + along for the status flip, and the operator must not be able to remove + it or add it to the bill. +- [ ] Exclude them from any invoice-total display in the modal. +- [ ] **Guard:** if the billable subset is empty, disable submit and toast + "This order has no billable lines." Apply to `sendToQuickBooks` + (line ~1892) and `submitDeliveryNote` (line ~1980). The existing + `noneIncluded` checks are about the operator's manual unticking and are + not sufficient. + +## Task 7 — Delivery note + +`src/app/api/invoicing/delivery-note/route.ts`. + +- [ ] Leave the status-flip loop at line 230 **untouched** — excluded rows must + receive `item_status: "delivery_note"` and the DN number, or they strand. +- [ ] At line 362, build `lines:` from billable items only. +- [ ] Guard: if no billable items remain, 400 with "No billable lines for a + delivery note" rather than rendering an empty PDF. +- [ ] Fetch `exclude_from_invoice` in the `getFullList` at line 212. + +**Test:** excluded rows get the status and DN number, and are absent from the +`lines` passed to the PDF renderer. + +## Task 8 — Picking-slip regression test + +No production code changes. This task exists to lock in behaviour the design +depends on, so a later refactor doesn't quietly break it. + +- [ ] Test `print-bundle/route.ts`: an invoice whose rows include an excluded + item produces a picking slip containing **all** rows, including the + excluded one. The picker must pull the washers. +- [ ] Same for `delivery-note-print/route.ts`. + +## Task 9 — MCP surface + +`src/app/api/mcp/route.ts`. + +- [ ] `order_items.create` accepts `exclude_from_invoice`. +- [ ] Update the tool description to explain what the flag does and that it + forces `selling_price` to 0. +- [ ] Update the description of the removed `pending` gate — the current text + at line 794 documents a 409 that no longer fires. + +## Definition of done + +- [ ] `npm run test:run` passes. +- [ ] `npx tsc --noEmit` passes. +- [ ] Manually on port 4001: add an extra to an order with a `po_generated` + sibling; process it onto a PO; confirm collection; send the order to + QuickBooks; confirm the QuickBooks invoice shows only the billable line + while all rows flip to `invoiced`; print the bundle and confirm the + picking slip lists the extra. diff --git a/docs/superpowers/specs/2026-08-30-non-invoiceable-order-extras-design.md b/docs/superpowers/specs/2026-08-30-non-invoiceable-order-extras-design.md new file mode 100644 index 0000000..cdbbef6 --- /dev/null +++ b/docs/superpowers/specs/2026-08-30-non-invoiceable-order-extras-design.md @@ -0,0 +1,202 @@ +# Non-Invoiceable Order Extras — Design + +**Date:** 2026-08-30 +**Status:** Approved, ready for implementation planning + +## The problem + +We quote a line as a single sellable unit — "M20 bolt, nut and washer" — and +the customer approves that one line at that one price. But the supply side +doesn't work that way: the bolt comes from one supplier, the nut and washer +from another. To raise the purchase orders we need three line items, not one. + +Today the operator has no way to add those two extra lines. `order_items` rows +are the only unit the PO flow understands, so a component with no row of its +own can't be ordered, can't be assigned to a driver, and never shows as +collected. The operator works around it off-system. + +Adding the rows naively creates the opposite problem: all three would flow to +the invoicing screen and QuickBooks would bill the customer for three lines +against a quote they approved as one. + +## What we're building + +A line item can be marked **excluded from invoice**. Such a line behaves like +any other order item everywhere in the operational flow — it gets a supplier, a +cost price, a purchase order, a driver assignment, and it shows as collected — +but it never appears on a customer-facing invoice or delivery note. + +The customer's paperwork comes out byte-identical to what it looks like today. + +## Decisions + +These were settled during brainstorming and are not open for reinterpretation +during implementation. + +| Question | Decision | +|---|---| +| Delivery note | **Excluded**, same as the invoice. Customer paperwork must agree. | +| Terminal status | Auto-flips to `invoiced` alongside its siblings. The order closes normally. | +| Where lines are added | Process page (`/orders/[id]/process`), for **any** line, with no `pending` gate. | +| Cost and margin | Cost counts as normal. `selling_price` is forced to `0` and locked. | +| Picking slip | **Included.** The picker must physically pull the washers. | + +## Data model + +One new field on `order_items`: + +``` +exclude_from_invoice : bool +``` + +**Polarity is load-bearing.** The flag must name the exception, not the norm. +A positively-named field (`bill_customer`, `invoiceable`) defaults to `false` +in PocketBase, which would silently mark every existing row in the database as +non-billable the instant the field is created. `exclude_from_invoice` defaults +to `false` = "bill this normally", so existing rows are unaffected. + +Per `AGENTS.md`, the user adds this field via the PocketBase admin UI before +any code references it. `pb-exports/pb_schema-2.json` is a snapshot for +reference only. + +No new `item_status` value is introduced. The existing select enum is +untouched, which avoids a second manual PB admin change and avoids updating +every status filter, pill, tab count and lifecycle comment in the app. + +## Architecture + +The design rests on a property of the existing invoice pipeline that makes the +whole feature small. + +### The invoice split + +`buildQbInvoicePayload` (`src/app/(app)/invoicing/view-model.ts:393`) builds two arrays from +one row list: + +```ts +order_item_ids: input.rows.map((row) => row.id), // drives the PB status write +items: input.rows.map((row) => ({ ... })), // drives the QuickBooks lines +``` + +The n8n workflow consumes them independently. Its **"Build PocketBase Invoice +Update"** node maps over `order_item_ids` and patches each row with +`item_status: 'invoiced'` plus `order_item_details.invoice`. The QuickBooks +invoice itself is built from `items`. + +So the change is: **excluded rows stay in `order_item_ids` and drop out of +`items`.** n8n flips them to `invoiced` with the batch and stamps them with the +same invoice number for audit; QuickBooks never learns they exist. + +**No n8n changes are required.** This is the crux of the design — verified +against `n8n_workfows/1 - WFDASH QUICKBOOKS INVOICE1 - WFDASH QUICKBOOKS INVOICE.json`. + +### The delivery note split + +`/api/invoicing/delivery-note/route.ts` has the same shape. Excluded rows stay +in the status-flip loop (`src/app/api/invoicing/delivery-note/route.ts:230`), so they receive +`item_status: "delivery_note"` and the DN number; they drop out of the `lines:` +array that renders the PDF (`src/app/api/invoicing/delivery-note/route.ts:362`). + +### Picking slips need no change at all + +Both picking-slip routes query PocketBase by document identity, not by the row +list the operator submitted: + +- `print-bundle/route.ts:88` — `order_item_details.invoice.number = "…" && item_status = "invoiced"` +- `delivery-note-print/route.ts:92` — `delivery_note_number = "…" && item_status = "delivery_note"` + +Because excluded rows *do* receive the invoice number / DN number and *do* +reach the matching status, they are picked up by these queries automatically. +The picker sees the washers. This falls out of the design for free. + +### The margin gate + +`calculateOrderItemMarkup(cost, 0)` returns `-100`, far below the +`ORDER_MARKUP_APPROVAL_THRESHOLD` of `22` +(`src/lib/server/order-item-margin-approval-service.ts:4`). Without an +explicit bypass, every extra the operator adds would immediately land in +`approval_needed` and block the order pending manager review. + +**This is the highest-risk part of the implementation.** The bypass must be +keyed on `exclude_from_invoice`, applied in the process page's save path +before the threshold comparison. + +### Adding lines mid-flight + +`createOrderItemForOrder` (`src/lib/server/orders-service.ts:3939`) currently throws a +409 `order_locked` if any line on the order is past `pending`. By the time the +operator discovers they need a washer line, the bolt line is already +`po_generated`, so this gate fires every time. It is removed. + +The gate's original purpose — preventing a PO's pricing/supplier reference from +being orphaned — does not apply here. Adding a *new* row alongside an existing +PO doesn't mutate that PO; the new row gets its own PO when the operator +processes it. + +## Flow + +``` +Operator opens /orders/[id]/process + │ + ├── Adds "M20 nut" with [x] Don't invoice + │ → selling_price forced 0, margin gate bypassed + │ → row created with exclude_from_invoice = true, item_status = pending + │ + ├── Normal flow: PO generated → driver assigned → collected → delivered + │ (identical to every other line; order "Complete" needs no change) + │ + └── Invoicing screen + ├── row renders muted with a "Not invoiced" chip, checkbox locked + ├── Send to QuickBooks + │ → order_item_ids: [bolt, nut, washer] ← all three + │ → items: [bolt] ← one line billed + │ → n8n flips all three to `invoiced` + └── Print bundle + → picking slip query finds all three by invoice number + → picker pulls all three +``` + +## Order completion + +Unchanged. `isOrderComplete` (`src/lib/server/order-completion.ts:20`) requires every +item to be `delivered`; excluded items reach `delivered` on the normal path. +`markOrderInvoiced` (`src/lib/server/orders-service.ts:3397`) requires every item to be +`invoiced`; excluded items get there via the n8n batch flip. Neither function +needs to learn about the flag. + +## Error handling + +- **All rows in a group excluded.** Block the send with a toast rather than + firing a zero-line invoice at QuickBooks. Applies to both the invoice and + delivery-note submit paths. +- **Operator un-ticks the only billable row** in the include-picker modal, + leaving only excluded rows. Same guard, same message. +- **Excluded row with a non-zero selling price** (legacy or hand-edited in PB + admin). The payload builder forces the exclusion regardless of price — the + flag is authoritative, price is not consulted. + +## Testing + +Unit tests, Vitest (`npm run test:run`): + +- `view-model.test.ts` — `buildQbInvoicePayload` keeps excluded ids in + `order_item_ids` and omits them from `items`; an all-excluded row set is + detectable by the caller. +- `orders-service.test.ts` — `createOrderItemForOrder` succeeds against an + order with `po_generated` siblings; `exclude_from_invoice` and the zeroed + selling price persist. +- Delivery-note route test — excluded rows receive the status flip and DN + number but are absent from the rendered `lines`. +- Margin-gate test — an excluded line with cost > 0 and selling 0 does not + enter `approval_needed`. + +## Out of scope + +- Reporting changes. Extras eat margin because their cost is real and their + selling price is zero; that is the correct number and no report needs + adjusting to produce it. +- Retrofitting existing orders that were worked around off-system. +- Any change to the n8n QuickBooks workflow. +- Bulk-adding extras, or templating common component sets (bolt/nut/washer as + a saved group). Worth considering later if the operator finds themselves + typing the same three lines repeatedly. diff --git a/docs/superpowers/specs/2026-08-30-quantity-handling-audit.md b/docs/superpowers/specs/2026-08-30-quantity-handling-audit.md new file mode 100644 index 0000000..0872104 --- /dev/null +++ b/docs/superpowers/specs/2026-08-30-quantity-handling-audit.md @@ -0,0 +1,203 @@ +# Quantity Handling Audit — Surplus, Shortfall and Partial Collection + +**Date:** 2026-08-30 +**Status:** Findings only. No design decisions taken yet. + +Answers two operator questions: + +1. We order 2 because of a supplier MOQ but the client only wanted 1. Does the + extra get booked into stock, and does the picking slip show the client only + the quantity that must actually go out? +2. The driver collects 2 of 5. What happens to the other 3? Does it go back to + the driver as outstanding, or is it lost? + +--- + +## Question 1: MOQ surplus — mostly already works + +**The good news: the picking slip already does exactly what you described.** + +`computePickingSlipLine` (`src/app/(app)/invoicing/picking-slip.tsx:250`) +computes five columns per line: + +| Column | Formula | Meaning | +|---|---|---| +| `ordered` | `order_qty` | what the client asked for | +| `collected` | `qty_collected` | what actually came in from the supplier | +| `pickQty` | `min(collected, ordered)` | **what goes out to the client** | +| `outstanding` | `max(ordered - pickQty, 0)` | still owed to the client | +| `toStock` | `max(collected - pickQty, 0)` | the MOQ surplus | + +`pickQty` is capped at the client's quantity, so the picker is never told to +send out the surplus. Ordered 1, received 2 → pick 1, to stock 1. Exactly the +behaviour you asked for, already shipped. + +The invoice agrees. `resolveInvoiceLineDefaults` +(`src/app/(app)/invoicing/view-model.ts:362`) bills `order_qty` and explicitly +refuses to bill `qty_collected`, with a comment naming the MOQ case as the +reason. + +Surplus booking exists too: `src/lib/server/stock-surplus.ts` and +`POST /api/v1/stock/surplus-book`, invoked from `/collection-confirm`. It +increments `stock_items.current_qty`, writes a `purchase_surplus` stock +movement, and stamps `order_items.surplus_qty`. The booking is capped at +`qty_collected - customerQty` so it can't over-book. + +### Gap 1.1 — surplus booking is manual and unprompted + +Nothing makes the operator book the surplus. The picking slip prints +`toStock: 1`, and if the operator never clicks the button on +`/collection-confirm`, that unit is physically in the building and invisible to +inventory forever. There is no "unbooked surplus" indicator, no count, no +queue. + +**Severity: medium.** Silent inventory loss, proportional to how often +operators forget. + +### Gap 1.2 — no check that the full quantity actually arrived + +You said "if the full quantity came in you must check that." There is no such +check. Nothing compares `qty_collected` against `order_qty` at any gate. See +Question 2 — this is the same root cause. + +--- + +## Question 2: Partial collection — the missing units disappear + +**Direct answer: the 3 outstanding units do not go back to the driver. They +are not tracked anywhere. The line is marked fully collected and moves on.** + +Both paths that mark a line collected set `item_status = "collected"` +unconditionally, without comparing quantities: + +**Driver app** — `src/lib/server/driver-stops.ts:620`: + +```ts +const qtyCollected = + input.qty_collected !== undefined + ? Math.floor(input.qty_collected) + : Number(lineTyped.order_qty ?? lineTyped.quantity ?? 0); + +const patch: Record = { qty_collected: qtyCollected }; +if (input.action === "collected") { + patch.item_status = "collected"; // ← no quantity comparison +``` + +Driver reports 2 of 5 → `qty_collected: 2`, `item_status: "collected"`. The +stop leaves their list. Nothing re-queues it. + +**Agent / MCP path** — `confirmOrderItemsCollected` +(`src/lib/server/order-items-service.ts:224`) is more careful, and then throws +the care away. It computes `remaining`, validates the partial doesn't overrun, +and writes a genuinely useful audit note — *"2 units added to qty_collected +(now 2/5)"* — before patching `item_status: "collected"` at line 318 regardless. +`remaining` is used only for validation and note text. It never holds the line +open. + +### What happens next + +The line is `collected`, so it flows to `ready_invoice` and lands on the +invoicing screen. There: + +- The **picking slip is honest** — it shows `outstanding: 3`, because + `computePickingSlipLine` does the subtraction. Good. +- The **invoice is not** — `resolveInvoiceLineDefaults` bills `order_qty`, + which is 5. **You invoice the client for 5 units when 2 arrived.** + +The operator *can* correct this: the invoicing modal supports per-line `qty` +overrides (`InvoiceLineOverride`). But the default is wrong and nothing flags +the discrepancy, so it depends entirely on someone noticing. + +### Gap 2.1 — `splitOrderItemRemainder` is locked out exactly when needed + +There is already a purpose-built tool for a shorted line: +`splitOrderItemRemainder` (`src/lib/server/order-items-service.ts:376`). It +creates a remainder line for the shortfall, supplier-less so it sorts to the +top of the process page as "needs supplier", and re-enters the normal PO flow. + +It refuses unless the line is `pending` or `processed`: + +```ts +if (!["pending", "processed"].includes(status)) { + throw new OrderItemSplitError( + `Cannot split an item in status "${status}" — only pending/processed lines are still editable.`, 409); +} +``` + +A short-collected line is `assigned` or `collection_pool`, so the split is +refused. **The mechanism that would solve this problem is unavailable in the +only situation where the problem occurs.** It was built for a different case: +the operator knowing up-front that supplier A can only supply 40 of 100. + +### Gap 2.2 — `outstanding_since` is about age, not quantity + +`driver-stops.ts:95` exposes `outstanding_since`, which looks like it tracks +this. It doesn't — it's the parent order's `created` date, used for +"outstanding N days" reporting. There is no quantity-outstanding concept +anywhere in the model. + +--- + +## Cross-cutting: `order_qty` means two different things + +This is the root cause underneath both questions, and the most dangerous thing +in this audit. + +**`order_items.order_qty` has no doc comment** (`src/lib/types.ts:807`), while +the neighbouring `quantity` has eleven lines of it. Two subsystems consequently +disagree about what it means. + +**Reading A — `order_qty` is what the CUSTOMER asked for:** + +- `stock-surplus.ts:66` — *"`order_qty` is the canonical 'what the customer + asked for' field"* +- `view-model.ts:362` — invoice quantity +- `driver-stops.ts:313` — `quantity_ordered: order_qty ?? quantity` +- `picking-slip.tsx` — the `ordered` column + +**Reading B — `order_qty` is what we're buying from THIS SUPPLIER:** + +- `splitOrderItemRemainder` doc, `order-items-service.ts`: *"the process page + lets the operator enter a supplier-facing `order_qty` that is LOWER than the + customer-facing `quantity`"* — customer 100, `order_qty` 40. + +The two readings converge only *after* a completed split, because the split +reduces `quantity` down to match `order_qty`. In the window between the +operator typing `order_qty = 40` and clicking Split, every consumer in Reading +A believes the customer ordered 40. + +**Failure scenario:** customer orders 100. Operator sets `order_qty = 40` for +supplier A, intending to split the other 60 to supplier B. They get +interrupted and never click Split. The invoice bills 40. The picking slip says +ordered 40, outstanding 0. The other 60 units are gone with no trace and no +warning. + +There is also a smaller inconsistency: `confirmOrderItemsCollected` computes +`remaining` from `row.quantity`, while `driver-stops.ts` defaults from +`row.order_qty`. Two different denominators for the same concept. + +--- + +## Summary + +| # | Finding | Severity | +|---|---|---| +| X-1 | `order_qty` means "customer qty" in four places and "supplier qty" in one, undocumented. Un-split lines silently under-bill. | **High** | +| 2.1 | Partial collection marks the line fully `collected`; the shortfall is untracked and the invoice defaults to the full quantity. | **High** | +| 2.2 | `splitOrderItemRemainder` is blocked by status exactly when a short collection needs it. | Medium | +| 1.1 | Surplus booking is manual with no prompt or unbooked-surplus queue. | Medium | +| 1.2 | No gate anywhere compares `qty_collected` to `order_qty`. | Medium | + +The picking slip is the one component that already gets all of this right. The +arithmetic you asked for exists — it just isn't enforced anywhere upstream of +the print, and the invoice ignores it. + +## Not yet decided + +No design decisions have been taken. The open questions are: + +- Should a short collection hold the line open (new status / partial state), or + auto-split the remainder into a new line, or just warn loudly at invoicing? +- Should `order_qty` be documented as customer-qty and the split flow moved to a + separate `supplier_qty` field, or the reverse? +- Should surplus booking be forced before a line can be invoiced?