MEGAINFRA-WEBAPP/.superpowers/sdd/2026-09-18-hybrid-delivery-note-ingestion/task-7-report.md
2026-09-19 12:20:48 +02:00

13 KiB

Task 7 report: atomic leases and staged orchestration

Status: complete

Code commit

d6e9c471afabc6c1a0bbf5f467c012eea7cc40ef — feat(dn): add staged ingestion orchestration

Files

  • src/lib/dn-pipeline/lock.ts and lock.test.ts
    • Atomic create-based batch lease using a deterministic, valid 15-character PocketBase record id.
    • One expired row may be reclaimed; a live or unobservable competing row is not claimed.
    • A per-acquisition owner token is checked before release, so an old worker does not delete a reclaimed replacement lock.
  • src/lib/dn-pipeline/run-staged.ts and run-staged.test.ts
    • Reads immutable dn_batch_sources, normalises fresh canonical PNG pages, persists/reuses dn_batch_pages by source/page/region, and invokes readOrReusePage for the vision cache.
    • Assembles persisted reads, resolves entities only within the confirmed batch site, and independently creates/updates staged candidates. Clear candidates advance to ready_to_post; ambiguous candidates remain needs_review; no delivery-note or financial rows are written.
  • src/app/api/dn-batches/tick/route.ts and test
    • Uses runStagedBatch only when DN_STAGED_INGESTION_ENABLED === "true". All other values retain the prior runBatch/claim path. Staged lease losers are reported as not_claimed, and shared retry accounting remains intact.
  • src/app/api/dn-batches/[id]/assembly/route.ts and test
    • Replaced the manual route's incompatible lease with withBatchLease.

RED evidence

Before production modules existed:

npx vitest run src/lib/dn-pipeline/lock.test.ts
FAIL: Failed to load url ./lock ... Does the file exist?
Test Files 1 failed; Tests 0

npx vitest run src/lib/dn-pipeline/run-staged.test.ts
FAIL: Failed to load url ./run-staged ... Does the file exist?
Test Files 1 failed; Tests 0

Before the manual route refactor:

npx vitest run 'src/app/api/dn-batches/[id]/assembly/route.test.ts'
FAIL: expected lock create payload to contain deterministic id and an
owner-qualified manual-assembly worker; existing payload lacked both.

Before the tick switch:

npx vitest run src/app/api/dn-batches/tick/route.test.ts
FAIL: staged runner was not called when DN_STAGED_INGESTION_ENABLED=true;
lease-loser result was not reported as not_claimed.

GREEN evidence

npx vitest run src/lib/dn-pipeline/lock.test.ts
3 passed

npx vitest run src/lib/dn-pipeline/run-staged.test.ts
2 passed

npx vitest run 'src/app/api/dn-batches/[id]/assembly/route.test.ts'
14 passed

npx vitest run src/app/api/dn-batches/tick/route.test.ts
29 passed

npx vitest run src/lib/dn-pipeline/run.test.ts
38 passed

npx vitest run src/lib/dn-pipeline src/app/api/dn-batches
21 files passed; 285 tests passed

npm run typecheck
passed

python3 scripts/pb-add-dn-staged-ingestion.py --dry-run
passed; offline preview only, no schema changes written

git diff --check
passed

The expected test logs contain deliberate mocked failure/reclaim paths and the existing legacy promotion-failure fixture; no test failed.

Decisions and self-review

  • The lease claim is an initial create with a deterministic record id; it does not use the old read/update/re-read claim sequence. Reads occur only after a failed create, to inspect a possible expiry. The lease tests cover a create race, one expiry reclaim, final release, and a replaced-owner row that must survive the first worker's finally.
  • Source files are fetched read-only. Page rows always store image/png PNG bytes from normaliseSource; the original source record/file is never updated. Cached successful reads bypass a second extractor invocation on a rerun.
  • The confirmed batch site is passed both to the site-filtered PO query and to resolveEntities. No coordinate, Clock, image-quality, or nearest-site code was changed.
  • Failed reads remain visible in page_count and open_pages; their sibling groups still assemble and stage. Candidate upserts use the existing batch note index and leave dismissed/posted candidates unchanged, so reruns do not create duplicate page/candidate records.
  • Staged orchestration deliberately ends at independent candidates. It never calls promoteNote, delivery_notes, or delivery_note_items; Task 8 owns promotion and ledger idempotency.
  • Read before route edits: Next 16 Route Handlers guide and route.ts file convention reference. The route retains force-dynamic, Node runtime, secret validation, stale-attempt handling, and the default legacy path.

Concerns

  • No PocketBase migration was applied; dry-run was intentionally used only.
  • Superseded by Fix Round 1: each acquisition now has a distinct record id, so a later replacement cannot be deleted by an old acquisition's id.

Fix Round 1

Code commit: e388e460d050c6dc53aac6ae2cd9063a5ccd948b — fix(dn): harden staged lease and candidate reuse.

Review fixes

  • Leases now use an acquisition-specific valid 15-character record id, hashed from the batch and a fresh owner token. UNIQUE(batch) remains the atomic exclusion gate. A post-failed-create lookup finds an expired holder by batch, deletes only that acquisition id, and retries exactly once. Replacement acquisitions therefore have a different id and survive a late old-worker delete without needing PocketBase conditional-delete support.
  • The staged winner writes a fresh claimed_at together with analysing and preparing before listing or reading sources. Terminal staged updates clear it; the existing tick retry/reclaim path also clears it on errors.
  • Added additive dn_batch_notes.candidate_key schema/type and unique (batch,candidate_key) index. Staged upserts now select by this key and move note_index rather than identifying candidates by their transient position. The key hashes batch plus the first persisted page's source/page/region and normalised document number. Blank/unknown numbers remain page-specific, so they cannot collide with another unknown document.
  • Reused page rows now load their stored dn_batch_pages.image bytes through pb.files.getURL before invoking readOrReusePage; source files use the same PocketBase URL helper. A changed normaliser cannot change cached or refreshed extraction input.

RED evidence

npx vitest run src/lib/dn-pipeline/lock.test.ts
3 failed: contenders used the same id; expired old id could not be reclaimed
through the new batch lookup; replacement-interleaving delete never occurred.

npx vitest run src/lib/dn-pipeline/run-staged.test.ts
3 failed: staged winner had no claimed_at; re-extraction received changed
normalised bytes instead of persisted PNG bytes; recovered DN-1 overwrote
candidate-1 that represented DN-2.

npx vitest run src/lib/dn-pipeline/run-staged.test.ts
1 failed: candidateKey was not exported, so unknown-number collision behavior
was not independently testable.

GREEN evidence

npx vitest run src/lib/dn-pipeline/lock.test.ts
4 passed

npx vitest run src/lib/dn-pipeline/run-staged.test.ts
6 passed

npx vitest run 'src/app/api/dn-batches/[id]/assembly/route.test.ts'
14 passed

npx vitest run src/lib/dn-pipeline src/app/api/dn-batches
21 files passed; 290 tests passed

npm run typecheck
passed

python3 scripts/pb-add-dn-staged-ingestion.py --self-test
passed

python3 scripts/pb-add-dn-staged-ingestion.py --dry-run
passed; no schema changes written

git diff --check
passed

Self-review

  • Lock tests model both vulnerable interleavings: a replacement arrives after expired-lock lookup but before old-id deletion, and another arrives after the final ownership read but before old-id release. Both replacements survive.
  • The staged overlap test proves the first worker's fresh claimed_at is present before blocking normalization begins, a second worker reports not_claimed, and only the eventual terminal update clears the marker.
  • Recovery-order coverage starts with page 1 failing and DN-2 as candidate 0; after page 1 becomes DN-1, DN-2 remains the same row and moves to index 1.
  • No financial promotion, Clock/photo, image-quality, nearest-site, or production PocketBase write was added. The exact feature flag behavior is unchanged.

Remaining concern

The design relies on a cryptographically negligible hash/id collision risk for two fresh owner tokens. A collision fails safely at create (no second worker runs); it cannot delete a different acquisition unless its id is identical.

Fix Round 2

Code commit: c960729 — fix: reconcile staged delivery-note candidates.

Review fixes

  • Added pocketBaseFileUrl, which supplies collectionName when narrow PocketBase record projections omit it. Source URLs are generated as dn_batch_sources; canonical persisted-page URLs are generated as dn_batch_pages. The test uses the installed PocketBase SDK rather than the test double, and verifies both escaped URLs.
  • Candidate reconciliation now uses each existing row at most once, in this order: exact durable key; a unique normalised delivery/document number; unique strongest persisted-page overlap; then a compatible, unambiguous legacy note_index. A matched durable key is retained forever; only a genuinely new row receives the newly proposed key. Matched keyless legacy rows are migrated in-place.
  • Terminal dismissed/posted rows keep commercial fields immutable, while their candidate_key and note_index can move on a recovered earlier sibling. Surplus nonterminal rows become review-visible (blocked / needs_review) instead of being silently duplicated or overwritten.

RED evidence

pnpm exec vitest run src/lib/dn-pipeline/run-staged.test.ts
5 failed (before implementation):
- pocketBaseFileUrl is not a function;
- recovered continuation [page 2] -> [page 1,page 2] created a second row;
- terminal DN-2 retained note_index 0 after DN-1 recovered;
- a legacy keyless DN row created a second candidate;
- ambiguous duplicate numbers were not surfaced for review.

GREEN evidence

pnpm exec vitest run src/lib/dn-pipeline/lock.test.ts \
  src/lib/dn-pipeline/run-staged.test.ts \
  src/app/api/dn-batches/tick/route.test.ts \
  'src/app/api/dn-batches/[id]/assembly/route.test.ts'
4 files passed; 58 tests passed

pnpm exec vitest run src/lib/dn-pipeline src/app/api/dn-batches
21 files passed; 295 tests passed

pnpm typecheck
passed

python3 scripts/pb-add-dn-staged-ingestion.py --self-test
passed

python3 scripts/pb-add-dn-staged-ingestion.py --dry-run
passed; offline preview only and no schema changes written

git diff --check
passed

Exact files and self-review

  • src/lib/dn-pipeline/run-staged.ts: collection-aware URL construction and one-to-one candidate reconciliation; no financial promotion writes added.
  • src/lib/dn-pipeline/run-staged.test.ts: real-SDK URL coverage, changed-normaliser persisted-image coverage, recovered reorder and continuation coverage, terminal reindexing, legacy migration, and duplicate number ambiguity coverage.
  • No lock, tick, assembly-route, Clock, photo/image-quality, nearest-site, or migration implementation was changed in this round. The lease fix and the exact default-off feature flag remain intact. No production PocketBase write or migration apply was performed.

Remaining concern

Persisted-page overlap uses the batch's stored page-number membership as the deterministic fallback after durable keys and unique printed numbers. Equal overlap scores deliberately remain unmatched and review-visible, prioritising non-overwrite safety over automatic reconciliation.

Fix Round 3

Code commit: 05eaa1f — fix: preserve terminal staged candidates.

Review fix

  • Existing-candidate reads now include the delivery_note relation. Terminal detection recognises legacy status: promoted and any non-empty delivery_note, in addition to the existing dismissed/posted markers. Consequently, surplus review handling cannot rewrite commercial rows that have already promoted, even when older rows lack a modern status/stage or posted_at marker. Matched terminal rows retain the existing metadata-only reindex behavior.

RED evidence

pnpm exec vitest run src/lib/dn-pipeline/run-staged.test.ts
2 failed (before implementation):
- unmatched status=promoted row was rewritten to status=blocked,
  stage=needs_review;
- unmatched row with delivery_note but no status/stage/posted_at was likewise
  rewritten to status=blocked, stage=needs_review.

GREEN evidence

pnpm exec vitest run src/lib/dn-pipeline/run-staged.test.ts
13 passed

pnpm exec vitest run src/lib/dn-pipeline src/app/api/dn-batches
21 files passed; 297 tests passed

pnpm typecheck
passed

python3 scripts/pb-add-dn-staged-ingestion.py --self-test
passed

python3 scripts/pb-add-dn-staged-ingestion.py --dry-run
passed; offline preview only and no schema changes written

git diff --check
passed

Self-review

  • The regression tests run the staged runner, not the helper in isolation: unmatched legacy-promoted and delivery-note-related rows retain their exact status/stage absence and note indices. The existing matched-terminal test still proves metadata-only reindexing for a real recovered sibling.
  • No promotion writes, migration apply, lease/tick/route changes, or unrelated files were changed.