fix(dn): keep the site's open question through a grouping change
Co-Authored-By: MiniMax Code <noreply@minimax.io>
This commit is contained in:
parent
f1e3c43de5
commit
4c437cd320
7 changed files with 101 additions and 9 deletions
|
|
@ -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})});
|
||||
|
|
|
|||
|
|
@ -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)],
|
||||
} });
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)] });
|
||||
}));
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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", () => {
|
||||
|
|
|
|||
|
|
@ -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<Record<string, unknown>> {
|
||||
return checksOf(checks).filter(isSiteDecision);
|
||||
}
|
||||
|
|
@ -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<Record<string,unknown>>;
|
||||
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<Record<string,unknown>>).map(c=>c.id)).toEqual(['sources_current']);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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.'}]});
|
||||
|
|
|
|||
Loading…
Reference in a new issue