diff --git a/src/app/api/dn-batches/[id]/assembly/route.test.ts b/src/app/api/dn-batches/[id]/assembly/route.test.ts index ac8eb77..5dacf5d 100644 --- a/src/app/api/dn-batches/[id]/assembly/route.test.ts +++ b/src/app/api/dn-batches/[id]/assembly/route.test.ts @@ -147,6 +147,31 @@ describe("assembly revision route", () => { }); }); +// Changing the grouping resets checks to one sources_current failure. That +// must not silently delete the site's open question from the board. +it("keeps an open site question when the operator saves a new grouping", async () => { + notesList.mockResolvedValue([ + { id: "note-1", status: "candidate", pages: [1], checks: [ + { id: "sources_current", passed: true }, + { id: "site_decision", passed: false, detail: { question: "How long is each pipe?" } }, + ] }, + ]); + const response = await POST(request({ revision: 1, groups: [{ pages: [1, 2] }], dismissedPages: [], reason: "join pages" }), { params: Promise.resolve({ id }) }); + expect(response.status).toBe(200); + expect(notesUpdate).toHaveBeenCalledWith("note-1", expect.objectContaining({ + checks: [ + expect.objectContaining({ id: "sources_current", passed: false }), + expect.objectContaining({ id: "site_decision", passed: false, detail: { question: "How long is each pipe?" } }), + ], + })); +}); + +it("leaves a note with no site question carrying only sources_current", async () => { + const response = await POST(request({ revision: 1, groups: [{ pages: [1, 2] }], dismissedPages: [], reason: "join pages" }), { params: Promise.resolve({ id }) }); + expect(response.status).toBe(200); + expect(notesUpdate.mock.calls[0][1].checks).toEqual([expect.objectContaining({ id: "sources_current" })]); +}); + it("clears old line-index and header overrides when different pages move into a candidate",async()=>{ notesList.mockResolvedValue([{id:"note-1",status:"candidate",pages:[1],manual_overrides:{po_number_raw:"OLD",items:{0:"old-item"},item_edits:{0:{qty:99}}}}]); const response=await POST(request({revision:1,groups:[{pages:[2]},{pages:[1]}],dismissedPages:[],reason:"swap documents"}),{params:Promise.resolve({id})}); diff --git a/src/app/api/dn-batches/[id]/assembly/route.ts b/src/app/api/dn-batches/[id]/assembly/route.ts index a2c29ea..566cd13 100644 --- a/src/app/api/dn-batches/[id]/assembly/route.ts +++ b/src/app/api/dn-batches/[id]/assembly/route.ts @@ -2,6 +2,7 @@ import { authorizeOperation } from "@/lib/auth/operation"; import { NextResponse } from "next/server"; import { pbAdmin } from "@/lib/pb"; import { withBatchLease } from "@/lib/dn-pipeline/lock"; +import { siteDecisionChecks } from "@/lib/dn-pipeline/site-decision-model"; import type { BatchMutation } from "@/lib/dn-pipeline/batch-mutations"; export const dynamic = "force-dynamic"; @@ -57,7 +58,7 @@ export async function POST( return NextResponse.json({ error: "invalid_page_coverage" }, { status: 400 }); } - const notes = await pb.collection("dn_batch_notes").getFullList({ filter: `batch = "${id}"`, sort: "note_index", fields: "id,note_index,status,stage,posted_at,line_posting,pages,page_from,page_to,assembly_revision" }); + const notes = await pb.collection("dn_batch_notes").getFullList({ filter: `batch = "${id}"`, sort: "note_index", fields: "id,note_index,status,stage,posted_at,line_posting,pages,page_from,page_to,assembly_revision,checks" }); notes.sort((a, b) => Number(a.note_index ?? 0) - Number(b.note_index ?? 0)); const posted = notes.find((note) => note.status === "promoted" || note.stage === "posted" || Boolean(note.posted_at) || Boolean(note.line_posting)); if (posted) return NextResponse.json({ error: "posted_candidate", note: posted.id }, { status: 409 }); @@ -110,7 +111,7 @@ export async function POST( status: "blocked", stage: "needs_review", reconciliation_preview: null, - checks: [{ id: "sources_current", passed: false, reason: "Page grouping changed. Re-run batch to rebuild this document from its saved pages." }], + checks: [{ id: "sources_current", passed: false, reason: "Page grouping changed. Re-run batch to rebuild this document from its saved pages." }, ...siteDecisionChecks(note.checks)], } }); } } diff --git a/src/lib/dn-pipeline/run-staged.ts b/src/lib/dn-pipeline/run-staged.ts index aa976f6..840276e 100644 --- a/src/lib/dn-pipeline/run-staged.ts +++ b/src/lib/dn-pipeline/run-staged.ts @@ -6,7 +6,7 @@ import type { GroupingProposal } from "./grouping-review"; import { createHash } from "node:crypto"; import type PocketBase from "pocketbase"; import { normalisePoNumber } from "../po-matching"; -import { openSiteDecision } from "./site-decision-model"; +import { openSiteDecision, siteDecisionChecks } from "./site-decision-model"; import { documentIdentity, identityReferences, normaliseDocumentNumber, resolveNoteLines, runNoteChecks, type PendingDocumentIdentity } from "./checks"; import type { PoItemRef } from "./match-po-item"; import type { AliasRow } from "./aliases"; @@ -609,9 +609,7 @@ async function upsertCandidate( // A question an agent left for the site is a person's pending decision, not // something a page read can answer: carry it across the re-run and keep the // document blocked until the site replies. - const siteDecisions = Array.isArray(existing?.checks) - ? (existing.checks as CheckResult[]).filter((check) => check?.id === "site_decision") - : []; + const siteDecisions = siteDecisionChecks(existing?.checks); const waitingForSite = openSiteDecision(siteDecisions) !== null; const ready = isReady && !waitingForSite; const fields = { @@ -854,7 +852,7 @@ async function markSurplusCandidatesForReview( await Promise.all(existing .filter((candidate) => !used.has(candidate.id) && !isTerminalCandidate(candidate) && !candidate.line_posting) .map(async (candidate) => { - await ownership.update("dn_batch_notes", candidate.id, { status: "blocked", stage: "needs_review", reconciliation_preview: null, checks: [{ id: "sources_current", passed: false, reason: "This document is outside the current page grouping. Review grouping and re-run batch." }] }); + await ownership.update("dn_batch_notes", candidate.id, { status: "blocked", stage: "needs_review", reconciliation_preview: null, checks: [{ id: "sources_current", passed: false, reason: "This document is outside the current page grouping. Review grouping and re-run batch." }, ...siteDecisionChecks(candidate.checks)] }); })); } diff --git a/src/lib/dn-pipeline/site-decision-model.test.ts b/src/lib/dn-pipeline/site-decision-model.test.ts index 529d0c6..94d157f 100644 --- a/src/lib/dn-pipeline/site-decision-model.test.ts +++ b/src/lib/dn-pipeline/site-decision-model.test.ts @@ -1,5 +1,34 @@ import { describe, expect, it } from "vitest"; -import { lastAnsweredSiteDecision, openSiteDecision } from "./site-decision-model"; +import { lastAnsweredSiteDecision, openSiteDecision, siteDecisionChecks } from "./site-decision-model"; + +describe("siteDecisionChecks", () => { + // Changing the page grouping rewrites a note's checks to a single + // sources_current failure. Without this the site's open question is wiped + // from the board and no one is asked any more. + it("keeps every site_decision check and drops everything else", () => { + const open = { id: "site_decision", passed: false, detail: { question: "How long is each pipe?" } }; + const answered = { id: "site_decision", passed: true, detail: { answer: "6 m" } }; + expect(siteDecisionChecks([ + { id: "sources_current", passed: false, reason: "Page grouping changed." }, + open, + { id: "po_resolved", passed: false, reason: "other" }, + answered, + ])).toEqual([open, answered]); + }); + + it("returns an empty array for non-arrays and malformed entries", () => { + expect(siteDecisionChecks(undefined)).toEqual([]); + expect(siteDecisionChecks(null)).toEqual([]); + expect(siteDecisionChecks("nonsense")).toEqual([]); + expect(siteDecisionChecks({ id: "site_decision" })).toEqual([]); + expect(siteDecisionChecks([null, "text", 7, ["site_decision"]])).toEqual([]); + }); + + it("returns the original check objects, not copies, so detail survives", () => { + const check = { id: "site_decision", passed: false, detail: { question: "q", suggestions: ["a"] } }; + expect(siteDecisionChecks([check])[0]).toBe(check); + }); +}); describe("openSiteDecision", () => { it("reads the last unanswered question with its suggestions", () => { diff --git a/src/lib/dn-pipeline/site-decision-model.ts b/src/lib/dn-pipeline/site-decision-model.ts index a3476b3..242cec8 100644 --- a/src/lib/dn-pipeline/site-decision-model.ts +++ b/src/lib/dn-pipeline/site-decision-model.ts @@ -35,4 +35,18 @@ export function lastAnsweredSiteDecision(checks: unknown): AnsweredSiteDecision if (!answered) return null; const detail = detailOf(answered); return { question: text(detail.question) || text(answered.reason).replace(/^Answered:\s*/i, ''), answer: text(detail.answer), answeredAt: text(detail.answeredAt) }; +} + +/** + * Every site_decision check, ready to be spliced back onto a rewritten + * `checks` array. + * + * A question an agent left for the site is a person's pending decision, not + * pipeline output. Every producer that resets checks — a re-run, a page + * regrouping in the MCP tool, the same regrouping in the web UI — resets them + * to a single `sources_current` failure, which used to drop the question off + * the board with nothing left to ask it again. + */ +export function siteDecisionChecks(checks: unknown): Array> { + return checksOf(checks).filter(isSiteDecision); } \ No newline at end of file diff --git a/src/lib/purchasing/delivery-candidates.test.ts b/src/lib/purchasing/delivery-candidates.test.ts index db84151..d8a4b28 100644 --- a/src/lib/purchasing/delivery-candidates.test.ts +++ b/src/lib/purchasing/delivery-candidates.test.ts @@ -22,3 +22,27 @@ it('retains surplus candidates when regrouping pages',async()=>{ expect(result.status).toBe('committed');expect(store('dn_batch_notes').size).toBe(2); expect(store('dn_batch_notes').get('abcdefghijklmn3')?.status).toBe('dismissed'); }); +// Changing the grouping resets the note to one sources_current failure. That +// used to wipe the site's open question, so it silently vanished from the +// board and nobody was asked (this happened live on ARS14 INV205). +it('keeps an open site question when regrouping pages',async()=>{ + const {pb,store}=memoryDatabase();const batchId='abcdefghijklmn1'; + await pb.collection('dn_batches').create({id:batchId,assembly_revision:0,page_count:1}); + await pb.collection('dn_batch_notes').create({id:'abcdefghijklmn2',batch:batchId,note_index:0,pages:[1],status:'candidate', + checks:[{id:'sources_current',passed:true},{id:'site_decision',passed:false,reason:'Pipe length unclear',detail:{question:'How long is each pipe?',suggestions:['6 m','12 m'],askedBy:'agent',askedAt:'2026-10-09T08:00:00.000Z'}}]}); + await pb.collection('dn_batch_pages').create({id:'page00000000000',batch:batchId,global_page:1}); + const ctx={pb,principal:{id:'agent',permissions:new Set<'read'|'write'>(['read','write'])}}; + await mutateDeliveryCandidate(ctx,'set_delivery_grouping',{operationId:'83f33948-63c7-4a02-a45a-9ab18d3bdc02',batchId,revision:0,reason:'fix',groups:[{pages:[1]}],dismissedPages:[]}); + const checks=store('dn_batch_notes').get('abcdefghijklmn2')?.checks as Array>; + expect(checks.map(c=>c.id)).toEqual(['sources_current','site_decision']); + expect(checks[1]).toMatchObject({passed:false,detail:{question:'How long is each pipe?',suggestions:['6 m','12 m']}}); +}); +it('does not invent a site question on a note that never had one',async()=>{ + const {pb,store}=memoryDatabase();const batchId='abcdefghijklmn1'; + await pb.collection('dn_batches').create({id:batchId,assembly_revision:0,page_count:1}); + await pb.collection('dn_batch_notes').create({id:'abcdefghijklmn2',batch:batchId,note_index:0,pages:[1],status:'candidate',checks:[{id:'po_resolved',passed:true}]}); + await pb.collection('dn_batch_pages').create({id:'page00000000000',batch:batchId,global_page:1}); + const ctx={pb,principal:{id:'agent',permissions:new Set<'read'|'write'>(['read','write'])}}; + await mutateDeliveryCandidate(ctx,'set_delivery_grouping',{operationId:'83f33948-63c7-4a02-a45a-9ab18d3bdc02',batchId,revision:0,reason:'fix',groups:[{pages:[1]}],dismissedPages:[]}); + expect((store('dn_batch_notes').get('abcdefghijklmn2')?.checks as Array>).map(c=>c.id)).toEqual(['sources_current']); +}); diff --git a/src/lib/purchasing/delivery-candidates.ts b/src/lib/purchasing/delivery-candidates.ts index 290a079..d4fec8e 100644 --- a/src/lib/purchasing/delivery-candidates.ts +++ b/src/lib/purchasing/delivery-candidates.ts @@ -4,6 +4,7 @@ import type {PurchasingContext,OperationResult} from './types'; import {MutationSchema,runOperation,operationRecordId} from './operations'; import {RecordId} from './documents'; import {PurchasingError} from './errors'; +import {siteDecisionChecks} from '@/lib/dn-pipeline/site-decision-model'; const page=z.number().int().min(1).max(10000); const fieldSchema=z.object({doc_type:z.enum(['delivery_note','invoice','collection_note','unknown']).optional(),delivery_note_number:z.string().max(200).optional(),po_number_raw:z.string().max(200).optional(),supplier_name_raw:z.string().max(500).optional(),supplier_registration_raw:z.string().max(200).optional(),site_name_raw:z.string().max(500).optional(),customer_vat_no:z.string().max(200).optional(),date:z.string().max(100).optional(),received_by_name:z.string().max(200).optional(),skip_customer_identity:z.boolean().optional()}).strict(); const itemEdit=z.object({lineIndex:z.number().int().min(0).max(199),description:z.string().max(2000).optional(),qty:z.number().finite().min(0).nullable().optional(),unit:z.string().max(100).optional(),product_code:z.string().max(200).optional()}).strict(); @@ -52,7 +53,7 @@ export async function mutateDeliveryCandidate(ctx:PurchasingContext,action:'crea const ids:string[]=[]; for(const [index,group] of grouping.groups.entries()){ const note=active[index],id=note?.id??hash('sha256',`${root}:candidate:${index}`).slice(0,15);ids.push(id); - const data={pages:group.pages,page_from:Math.min(...group.pages),page_to:Math.max(...group.pages),assembly_revision:nextRevision,manual_overrides:{},status:'blocked',stage:'needs_review',reconciliation_preview:null,checks:[{id:'sources_current',passed:false,reason:'Page grouping changed. Process this batch again.'}]}; + const data={pages:group.pages,page_from:Math.min(...group.pages),page_to:Math.max(...group.pages),assembly_revision:nextRevision,manual_overrides:{},status:'blocked',stage:'needs_review',reconciliation_preview:null,checks:[{id:'sources_current',passed:false,reason:'Page grouping changed. Process this batch again.'},...siteDecisionChecks(note?.checks)]}; if(note)await lease.update('dn_batch_notes',id,data);else await lease.create('dn_batch_notes',{id,batch:batchId,note_index:index,candidate_key:`mcp:${id}`,doc_type:'unknown',items:[],...data}); } for(const stale of active.slice(grouping.groups.length))await lease.update('dn_batch_notes',stale.id,{status:'dismissed',stage:'dismissed',reconciliation_preview:null,checks:[{id:'grouping_superseded',passed:false,reason:'Retained previous candidate after page grouping changed.'}]});