docs: Stage 3b plan (request_deletion, reassign_receipt, merge_suppliers, split_or_crop_page, background fragments)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
DEls
parent
82c6f95878
commit
d94d29f0a2
1 changed files with 112 additions and 0 deletions
112
docs/superpowers/plans/2026-10-09-dn-agent-tools-stage-3b.md
Normal file
112
docs/superpowers/plans/2026-10-09-dn-agent-tools-stage-3b.md
Normal file
|
|
@ -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 <changed files>`. `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 `"<old name> (merged into <kept name>)"`. 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 <kept name>; 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-<batchId>-p<page>-<n>.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: <new batch ids>. <reason>')`. 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.
|
||||
Loading…
Reference in a new issue