diff --git a/docs/superpowers/plans/2026-10-09-dn-agent-tools-stage-3b.md b/docs/superpowers/plans/2026-10-09-dn-agent-tools-stage-3b.md new file mode 100644 index 0000000..07b3a96 --- /dev/null +++ b/docs/superpowers/plans/2026-10-09-dn-agent-tools-stage-3b.md @@ -0,0 +1,112 @@ +# DN agent tools — Stage 3b (fix wrong bookings, flag deletes, crop, merge suppliers) + +Agents need these tools to finish the work the acceptance run on 2026-10-09 could not: +- **Wrong receipts:** IO019312's 7 bends were reversed off PO 9330 but never rebooked on PO 9376; IN018888 sleeves are booked as 60/91 instead of 360/546; Global Precast 61833 is booked as 65 m but was 65 pipes (162.5 m). +- **Duplicates:** there are duplicate supplier records (e.g. two Vula records). +- **Photos:** some photos hold two slips, or a slip with background paper. +- **Deletes:** agents may never delete. They need a way to flag a record for deletion with a reason, so a person can do it. + +**Spec:** `docs/superpowers/specs/2026-10-09-delivery-agent-tools-design.md` ("everything except delete; deletes are flagged with a reason"). + +## Global constraints +- **No deletes anywhere** (`.delete(` must not appear in new code). No PocketBase schema changes. No file-field replacement either: replacing a file deletes the old file. +- Every write goes through `runOperation` (`src/lib/purchasing/operations.ts`). Read it first: it handles the idempotent journal (`mcp_operations`) and buffers writes into one transaction. Use the `resourceKey`/scope parameter as the existing tools do (see `site-decision.ts`, `document-edit.ts`). Where a batch exists, also write a `dn_review_events` row (`batch` is required on that collection, so skip it when there is no batch; the journal holds before/after). +- Register each new MCP tool in `src/mcp/purchasing-write-tools.ts` with `registerPurchasingTool(..., permission:'write')`, following the Stage 2 tools there. Give each a plain-language description. +- Reuse: `PurchasingError`, `RecordId`, `MutationSchema`, `memoryDatabase()` (test-support.ts), `setAsideWithinLease` (`document-disposition.ts`), `uploadPurchasingDocument` (`uploads.ts`), `normaliseSupplierKey` (find it: `grep -rn "export function normaliseSupplierKey" src`). +- Style: dense one-line TS like the rest of `src/lib/purchasing/*`. +- Verify each task: `npx vitest run src/lib/purchasing src/lib/dn-pipeline src/mcp src/app/api/dn-batches src/app/dashboard/financial/delivery-notes`, `./node_modules/.bin/tsc --noEmit --project tsconfig.json`, `npx eslint `. `split.test.ts` is flaky; re-run it alone if it fails. +- Branch `feat/dn-stage-3b`. One commit per task. Do not push or merge. + +--- + +### Task 1: `request_deletion` (flag, never delete) + +New `src/lib/purchasing/deletion-requests.ts`: +- `RequestDeletionSchema = MutationSchema.extend({collection: z.enum(['delivery_notes','delivery_note_items','dn_batch_notes','dn_batches','dn_batch_pages','purchase_orders','purchase_order_items','suppliers','supplier_document_aliases']), recordId: RecordId, reason: z.string().trim().min(8).max(2000)}).strict()`. +- `requestDeletion(ctx,input)`: check the record exists (`getOne`, 404 `record_not_found` otherwise). Through `runOperation(ctx,'request_deletion',args, ...)`, return `{collection, recordId, reason, summary}`. `summary` is a short human label read from the record (e.g. a PO number, doc number, supplier name, or `raw_value`; fall back to the id). The journal row IS the request; nothing else is written. +- `listDeletionRequests(pb)`: read `mcp_operations` with `tool="request_deletion" && status="committed"`. For each, return `{operationId, collection, recordId, reason, summary, requestedBy: credential}`. Keep only requests whose record **still exists**: once a person deletes it, the request disappears by itself. Requests for the same collection+recordId collapse into one (keep every reason). +- MCP tool `request_deletion`: "Flag a record that should be deleted, with the reason. Agents never delete; a person reviews the flag and deletes it." Add `deletionRequests` to `inbox_board`'s result (`src/lib/purchasing/inbox-board.ts`). +- Web: on the inbox page (`src/app/dashboard/financial/delivery-notes/inbox/page.tsx`), if there are any requests, show a small "Flagged for deletion" section under the board. One line each: collection, summary, reason, and who asked. Read only; no delete button. + +**Tests:** request on a missing record → `record_not_found`; a request is journaled and listed; it drops off the list once the record is gone (delete it in the memory DB *inside the test only*); duplicates collapse; `inbox_board` includes `deletionRequests`. + +Commit: `feat(mcp): agents flag records for deletion instead of deleting`. + +--- + +### Task 2: `reassign_receipt` (fix a wrong booking) + +New `src/lib/purchasing/receipt-reassign.ts`. Read `receipt-correction.ts` first; it is the model for checks and `expected*` guards. +- Schema: `MutationSchema.extend({deliveryNoteId: RecordId, purchaseOrderId: RecordId.optional(), reason: z.string().trim().min(8).max(2000), lines: z.array(z.object({id: RecordId, expectedQtyReceived: z.number().finite().min(0), purchaseOrderItemId: RecordId, qtyReceived: z.number().finite().min(0).max(1e9)}).strict()).min(1).max(100)}).strict()`. +- Load `delivery_notes` (404 if missing) and all its `delivery_note_items`. The target PO is `args.purchaseOrderId ?? delivery.purchase_order`. +- Refusals (PurchasingError, 409 unless noted): + - `line_not_on_delivery` (400): a line id is not one of this delivery's items. + - `receipt_changed_reload`: `|row.qty_received - expectedQtyReceived| > 0.0001`. + - `po_line_not_on_order` (400): a target item is not on the target PO. + - `purchase_order_site_mismatch`: the target PO's site ≠ the delivery's site. + - `supplier_mismatch`: the target PO's `supplier_ref` ≠ the current PO's `supplier_ref` (when both are set). + - `partial_move_not_supported`: moving to a different PO but not every item of the delivery is listed. The whole receipt moves or nothing does. + - `immutable_line_receipt`: the delivery came from line-by-line posting. Check the linked `dn_batch_notes` (`delivery.batch_note`) for a non-empty `line_posting`, the same rule `receipt-correction.ts` uses. + - `nothing_changed` (400). +- Writes: each `delivery_note_items` row gets `{purchase_order_item, qty_received}`. If the PO changes, `delivery_notes.purchase_order` changes, and so does the linked `dn_batch_notes.purchase_order` when there is one (`receipt-correction.ts` checks the two agree). Result `{deliveryNoteId, purchaseOrder:{from,to}, lines:[{id, before:{purchase_order_item, qty_received}, after:{...}}]}`. Write a `dn_review_events` row (`action:'reassign_receipt'`) when `delivery.batch` is set. +- MCP tool `reassign_receipt`: "Fix a wrong booking: move a posted receipt's lines to the right PO line, or the whole receipt to the right PO (same site and supplier), and/or correct the received quantity. Give the quantity you expect is booked now so a concurrent change is caught." + +**Tests (memoryDatabase):** quantity-only fix on the same PO (the 61833 case: 65 → 162.5); a whole receipt moves to another PO of the same supplier and site, with item links and both PO references updated (the IO019312 case); each refusal above; a retry with the same operationId returns the stored result. + +Commit: `feat(mcp): reassign_receipt fixes a wrong booking without deleting it`. + +--- + +### Task 3: `merge_suppliers` + +New `src/lib/purchasing/supplier-merge.ts`: +- Schema: `MutationSchema.extend({keepId: RecordId, duplicateId: RecordId, reason: z.string().trim().min(8).max(2000)}).strict()` and refine `keepId !== duplicateId` (`same_supplier`). +- Re-point every reference from duplicate to keep: `purchase_orders.supplier_ref` (also set the text field `supplier` to the kept name on those POs), `dn_batch_notes.supplier`, `supplier_document_aliases.supplier`. These are the only relation fields to `suppliers`; confirm with `grep -rn "supplier_ref\|collection('suppliers')" src/lib`. Add an alias row mapping the duplicate's name to the kept supplier (`source:'supplier_merge'`), unless one with that `normalised_key` exists. +- Rename the duplicate to `" (merged into )"`. Then create a deletion request for it in the same operation: write a second `mcp_operations` row exactly like Task 1's journal row, or call a shared helper from Task 1 (preferred). Reason: "Merged into ; no records point to it any more." +- Result `{keep, duplicate, moved:{purchase_orders, dn_batch_notes, supplier_document_aliases}}`. +- If the writes are too many for one `runOperation` transaction, refuse with `merge_too_large` and the counts. Find the limit in `operations.ts`/`batch-mutations.ts`; don't guess. +- MCP tool `merge_suppliers`: "Merge a duplicate supplier into the one to keep: repoints its orders, documents and aliases, renames it, and flags it for deletion." + +**Tests:** references move, the alias is added, the duplicate is renamed and appears in `listDeletionRequests`; `same_supplier`; a retry is idempotent. + +Commit: `feat(mcp): merge_suppliers repoints a duplicate supplier and flags it for deletion`. + +--- + +### Task 4: `split_or_crop_page` + +Cropping must keep the original photo, so a crop becomes a **new upload**, and the original document is set aside with a link to it. + +New `src/lib/purchasing/page-split.ts`: +- Schema: `MutationSchema.extend({batchId: RecordId, page: z.number().int().min(1).max(10000), regions: z.array(z.object({x:z.number().min(0).max(1), y:z.number().min(0).max(1), width:z.number().gt(0).max(1), height:z.number().gt(0).max(1)}).strict()).min(1).max(6), reason: z.string().trim().min(8).max(2000)}).strict()`. Regions are fractions of the page image; clamp them to the image bounds. +- Load the `dn_batch_pages` row for that batch and `global_page` (fields must include `collectionId,collectionName`: see `move-to-purchase-orders.ts`, which had this exact bug). Fetch the image the same way `move-to-purchase-orders.ts` does. With `sharp`, `extract` each region and write it as a PNG. +- Upload each crop as its own delivery document for the batch's site, using the existing path behind the MCP tool `upload_delivery_documents` (`uploadPurchasingDocument(ctx,'upload_delivery_documents',{...})`). Read `uploads.ts` for the exact input (base64, filename, site, etc.). Use filename `crop--p-.png`. +- Then, inside a batch lease, set aside every open note of the original batch covering that page with `setAsideWithinLease(..., kind:'other', reason:'Split into new uploads: . ')`. If no note covers the page, leave the notes alone and only record a `dn_review_events` row `action:'split_page'`. +- Result `{batchId, page, uploads:[{batchId, filename}], setAside:[noteIds]}`. Idempotent through the journal: a retry must not upload twice. Look at how `move-to-purchase-orders.ts` orders the upload versus the journal, and follow it. +- MCP tool `split_or_crop_page`: "Cut a page photo into one or more regions (fractions of the image) and upload each as its own delivery document; the original is kept and set aside." + +**Tests:** with a small generated PNG (make it with `sharp` in the test), 2 regions → 2 uploads with the right pixel sizes, and the covering note is set aside; a page that doesn't exist → `page_not_found`; a retry doesn't upload again. Mock the upload call if the memory DB can't run it, and assert its arguments. + +Commit: `feat(mcp): split_or_crop_page uploads crops and keeps the original`. + +--- + +### Task 5: grouping lets background fragments through + +On 2026-10-09 an ARS14 invoice photo was held in "Check grouping" because the reader set `multiple_documents: true` for a corner of another paper in the background. The extraction prompt already says to ignore fragments (`src/lib/dn-pipeline/extract.ts` ~line 140); the model still flags them sometimes. + +In `src/lib/dn-pipeline/assemble.ts` (~line 45, `if (read.multiple_documents)`): do not hold the page when **all** of these hold: +- the read has a `doc_number`; +- the read has at least one line item; +- the read's own evidence text for `multiple_documents` describes the other paper as background (case-insensitive match on `background|behind|fragment|cropped|partial|edge|corner|stack`), and it does not mention a second document number. + +Then group the page normally and keep a non-blocking note in the group (follow the existing warning/explanation field in the group type, if there is one; otherwise skip the note). Otherwise keep today's behaviour. Find where the evidence text lives in the read type (`types.ts` / `extract.ts`). + +**Tests (`assemble.test.ts`):** a read with background evidence, a doc number and items is grouped and not opened; one with evidence "two delivery notes side by side" stays open; one without items stays open. + +Commit: `fix(dn): a fragment of background paper no longer holds a page in grouping`. + +--- + +## Done when +Five commits on `feat/dn-stage-3b`, the full verify set is green, and each new test fails without its fix.