From 396e5039b5a2b1bbba48ff025857ebfb7fca936e Mon Sep 17 00:00:00 2001 From: Fluory Date: Mon, 28 Sep 2026 13:57:38 +0200 Subject: [PATCH 1/3] fix(samples): settle samples an aborted seed run left behind A seed run aborted midway left a sample in NEW/PROCESSING/ERROR that stayed in the list forever: the UI cannot reject it (only from REVIEW) and reprocessing a sample is refused on purpose. Samples are never queued, so these states only mean the run broke off. The seed now first rejects such leftovers through a dedicated status event (sample.retired), with a fixed reason and an audit event - never deleting a row. Fixes #84 Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 4 ++++ docs/technical/deployment-vercel.md | 2 ++ src/features/requests/index.ts | 1 + src/features/requests/repository.ts | 10 ++++++++ src/features/requests/status.test.ts | 8 +++++++ src/features/requests/status.ts | 7 +++++- src/features/samples/index.ts | 2 +- src/features/samples/seed.ts | 36 +++++++++++++++++++++++++--- src/seed-samples.ts | 3 ++- tests/integration/samples.test.ts | 20 ++++++++++++---- 10 files changed, 83 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f87913..1209866 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,10 @@ This file records what changes **in the product** – process and session state ## [Unreleased] +### Changed +- `pnpm seed:samples` first settles samples an aborted run left in progress or in error: rejected with a + fixed reason and an audit event, never deleted – the list no longer keeps such leftovers (#84). + ## [0.1.0] – 2026-09-27 First release: the pilot scope – intake of e-mail and PDF/Excel/Word, AI extraction with sources and diff --git a/docs/technical/deployment-vercel.md b/docs/technical/deployment-vercel.md index e7483ce..8ea0cc5 100644 --- a/docs/technical/deployment-vercel.md +++ b/docs/technical/deployment-vercel.md @@ -124,6 +124,8 @@ route; if it is not reachable, the export is queued and the next drain retries i once with `pnpm samples:record` (`src/features/samples/data/`) – no model call, so the samples work even while the model provider is overloaded. List and detail label them „Vorbereitetes Beispiel – aufgezeichnete KI-Antwort“. The step is idempotent: repeat it whenever visitors have decided the samples, and it adds only what is missing. +Samples an aborted run left behind (in progress or in error) are first set to rejected with a fixed reason and +an audit event – never deleted (#84). ## 7. Jobs without a worker diff --git a/src/features/requests/index.ts b/src/features/requests/index.ts index e89da78..a50e872 100644 --- a/src/features/requests/index.ts +++ b/src/features/requests/index.ts @@ -3,6 +3,7 @@ export { countRequestsByStatus, countRequestsCreatedBy, countSamples, + listSampleIds, createRequest, findDuplicate, getRequest, diff --git a/src/features/requests/repository.ts b/src/features/requests/repository.ts index 425439b..6436f1e 100644 --- a/src/features/requests/repository.ts +++ b/src/features/requests/repository.ts @@ -166,6 +166,16 @@ export async function countSamples(tx: TenantTx, statuses: readonly RequestStatu return row?.count ?? 0; } +/** Ids of the tenant's samples in one of `statuses` (#84: leftovers of an aborted seed run). */ +export async function listSampleIds(tx: TenantTx, statuses: readonly RequestStatus[]): Promise { + tenantOf(tx); + const rows = await tx + .select({ id: requests.id }) + .from(requests) + .where(and(eq(requests.source, "sample"), inArray(requests.status, [...statuses]))); + return rows.map((row) => row.id); +} + /** Requests a user created since `since` (upload rate limit, #59 review) – within the tenant. */ export async function countRequestsCreatedBy(tx: TenantTx, userId: string, since: Date): Promise { tenantOf(tx); diff --git a/src/features/requests/status.test.ts b/src/features/requests/status.test.ts index d4c79bb..e8e65a1 100644 --- a/src/features/requests/status.test.ts +++ b/src/features/requests/status.test.ts @@ -18,11 +18,19 @@ describe("request status machine (ADR-0001: NEW → PROCESSING → REVIEW → AP ["NEW", "reject.duplicate", "REJECTED"], ["REVIEW", "reject.duplicate", "REJECTED"], ["ERROR", "reject.duplicate", "REJECTED"], + // A prepared sample left behind by an aborted seed run is settled (#84) – never a decided one. + ["NEW", "sample.retired", "REJECTED"], + ["PROCESSING", "sample.retired", "REJECTED"], + ["ERROR", "sample.retired", "REJECTED"], ] as const)("%s --%s--> %s", (from, event, to) => { expect(nextStatus(from, event as RequestEvent)).toBe(to); }); it.each([ + ["REVIEW", "sample.retired"], + ["APPROVED", "sample.retired"], + ["EXPORTED", "sample.retired"], + ["REJECTED", "sample.retired"], ["REVIEW", "processing.succeeded"], ["EXPORTED", "processing.started"], ["NEW", "approve"], diff --git a/src/features/requests/status.ts b/src/features/requests/status.ts index 41b4f40..59894ba 100644 --- a/src/features/requests/status.ts +++ b/src/features/requests/status.ts @@ -12,7 +12,8 @@ export type RequestEvent = | "reject.duplicate" | "export.succeeded" | "export.failed" - | "reprocess.export"; + | "reprocess.export" + | "sample.retired"; export type ErrorStage = "processing" | "export"; @@ -31,6 +32,10 @@ const TRANSITIONS: Record s * a sample to the live model; and the export job queued by the approval runs right away, so visitors see * the ERP reference without doing anything. */ -export async function seedSamples(deps: SampleDeps, actor: Actor): Promise { +export async function seedSamples(deps: SampleDeps, actor: Actor): Promise { + const retired = await retireLeftovers(deps, actor); const seeded: SeededSample[] = []; for (const sample of SAMPLES) { const serving = await deps.tenancy.withTenant(actor.companyId, (tx) => countSamples(tx, STILL_SERVING[sample.purpose])); @@ -63,7 +75,25 @@ export async function seedSamples(deps: SampleDeps, actor: Actor): Promise { + return deps.tenancy.withTenant(actor.companyId, async (tx) => { + let retired = 0; + for (const id of await listSampleIds(tx, LEFTOVER)) { + const request = await lockRequest(tx, id); + if (!request || request.source !== "sample" || !LEFTOVER.includes(request.status)) continue; + await transitionRequest(tx, request, "sample.retired", { rejectionReason: LEFTOVER_REASON, errorStage: null, errorMessage: null, nextRetryAt: null }); + await recordAudit(tx, { actorUserId: actor.userId, action: "request.sample_retired", entityType: "request", entityId: id, data: { from: request.status } }); + retired++; + } + return retired; + }); } async function createSample(deps: SampleDeps, actor: Actor, sample: Sample): Promise { diff --git a/src/seed-samples.ts b/src/seed-samples.ts index 1ee93ff..2803040 100644 --- a/src/seed-samples.ts +++ b/src/seed-samples.ts @@ -38,7 +38,8 @@ async function main(): Promise { const cookie = login.headers.getSetCookie().map((line) => line.split(";")[0]).join("; "); const actor = await getActor(auth, database.db, new Headers({ cookie })); if (!actor) throw new Error(`${email} has no company – run pnpm seed:demo first`); - const seeded = await seedSamples(deps, actor); + const { seeded, retired } = await seedSamples(deps, actor); + if (retired > 0) console.log(`${email}: ${retired} leftover sample(s) of an aborted run rejected`); console.log(seeded.length === 0 ? `${email}: samples still in place` : `${email}: ${seeded.map((sample) => `${sample.key} → ${sample.status}`).join(", ")}`); } } finally { diff --git a/tests/integration/samples.test.ts b/tests/integration/samples.test.ts index 0ed4b3f..b0319a6 100644 --- a/tests/integration/samples.test.ts +++ b/tests/integration/samples.test.ts @@ -62,8 +62,9 @@ describe("prepared samples", () => { it("seeds one sample in review and one approved and exported sample; a later delivery of the export job is a no-op", async () => { const [admin] = actors as [Actor]; - const seeded = await seedSamples(deps(), admin); + const { seeded, retired } = await seedSamples(deps(), admin); + expect(retired).toBe(0); expect(seeded.map(({ key, status }) => [key, status])).toEqual([ ["werk-ost", "REVIEW"], ["pumpe-p204", "EXPORTED"], @@ -106,7 +107,7 @@ describe("prepared samples", () => { const [admin] = actors as [Actor]; const before = await samplesOf(admin); - expect(await seedSamples(deps(), admin)).toEqual([]); + expect(await seedSamples(deps(), admin)).toEqual({ seeded: [], retired: 0 }); expect(await samplesOf(admin)).toEqual(before); }); @@ -114,7 +115,7 @@ describe("prepared samples", () => { it("leaves the export to the queued job when the ERP is unreachable – nothing half-written", async () => { const admin = actors[1]!; - const seeded = await seedSamples(deps({ erpDown: true }), admin); + const { seeded } = await seedSamples(deps({ erpDown: true }), admin); const exported = seeded.find((sample) => sample.key === "pumpe-p204")!; expect(exported.status).toBe("APPROVED"); @@ -153,11 +154,22 @@ describe("prepared samples", () => { // Reprocessing a sample would call the live model – refused. await expect(reprocessRequest({ tenancy, boss }, admin, failed!.id)).rejects.toBeInstanceOf(ReprocessRefused); - const seeded = await seedSamples(deps(), admin); + const { seeded, retired } = await seedSamples(deps(), admin); expect(seeded.map(({ key, status }) => [key, status])).toEqual([ ["werk-ost", "REVIEW"], ["pumpe-p204", "EXPORTED"], ]); + + // #84: the leftover is settled – rejected with a reason and an audit event, never deleted. + expect(retired).toBe(1); + expect(await tenancy.withTenant(admin.companyId, (tx) => getRequest(tx, failed!.id))).toMatchObject({ + status: "REJECTED", + rejectionReason: "Beispiel durch einen neuen Lauf ersetzt – das Anlegen war abgebrochen.", + errorMessage: null, + }); + const events = await tenancy.withTenant(admin.companyId, (tx) => listAuditEvents(tx, "request", failed!.id)); + expect(events.filter((event) => event.action === "request.sample_retired")).toHaveLength(1); + expect((await samplesOf(admin)).map((sample) => sample.status).sort()).toEqual(["EXPORTED", "REJECTED", "REVIEW"]); }); it("the database refuses an unknown request source (migration 0019)", async () => { From eaf94e1f7ca7c0d4831da3dc24290d64d264c9df Mon Sep 17 00:00:00 2001 From: Fluory Date: Mon, 28 Sep 2026 14:06:39 +0200 Subject: [PATCH 2/3] fix(samples): never settle an approved sample whose export failed The fresh review of #92 found that the leftover step rejected every sample in ERROR - including an approved one whose export failed, which may already be in the ERP and can be exported again. Only NEW, PROCESSING and ERROR from processing mean an aborted run. The rule now lives in the requests module (isSampleLeftover, retireSampleLeftover), re-checked under the row lock, so no other caller can apply it to an ordinary request. Concurrent seed runs are a follow-up (#93). Refs #84 Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 5 ++-- docs/technical/deployment-vercel.md | 5 ++-- src/features/requests/index.ts | 4 ++- src/features/requests/repository.ts | 21 +++++++++++-- src/features/requests/sample-leftover.test.ts | 30 +++++++++++++++++++ src/features/requests/sample-leftover.ts | 13 ++++++++ src/features/requests/status.ts | 6 ++-- src/features/samples/seed.ts | 14 ++++----- tests/integration/samples.test.ts | 14 +++++++++ 9 files changed, 94 insertions(+), 18 deletions(-) create mode 100644 src/features/requests/sample-leftover.test.ts create mode 100644 src/features/requests/sample-leftover.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c65f184..790ae67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,9 @@ This file records what changes **in the product** – process and session state ## [Unreleased] ### Changed -- `pnpm seed:samples` first settles samples an aborted run left in progress or in error: rejected with a - fixed reason and an audit event, never deleted – the list no longer keeps such leftovers (#84). +- `pnpm seed:samples` first settles samples an aborted run left behind (new, in progress, or failed in + processing): rejected with a fixed reason and an audit event, never deleted – the list no longer keeps + such leftovers. An approved sample whose export failed is kept (#84). ### Fixed - `/api/health` reports the AI service as `starting` instead of `failed` when it does not answer in time – diff --git a/docs/technical/deployment-vercel.md b/docs/technical/deployment-vercel.md index 33f44a2..6f47b0f 100644 --- a/docs/technical/deployment-vercel.md +++ b/docs/technical/deployment-vercel.md @@ -124,8 +124,9 @@ route; if it is not reachable, the export is queued and the next drain retries i once with `pnpm samples:record` (`src/features/samples/data/`) – no model call, so the samples work even while the model provider is overloaded. List and detail label them „Vorbereitetes Beispiel – aufgezeichnete KI-Antwort“. The step is idempotent: repeat it whenever visitors have decided the samples, and it adds only what is missing. -Samples an aborted run left behind (in progress or in error) are first set to rejected with a fixed reason and -an audit event – never deleted (#84). +Samples an aborted run left behind (new, in progress, or in error from processing) are first set to rejected +with a fixed reason and an audit event – never deleted (#84). An approved sample whose export failed is kept: +it can be exported again with „Erneut verarbeiten“. ## 7. Jobs without a worker diff --git a/src/features/requests/index.ts b/src/features/requests/index.ts index a50e872..7135117 100644 --- a/src/features/requests/index.ts +++ b/src/features/requests/index.ts @@ -3,7 +3,7 @@ export { countRequestsByStatus, countRequestsCreatedBy, countSamples, - listSampleIds, + listSampleLeftoverIds, createRequest, findDuplicate, getRequest, @@ -15,9 +15,11 @@ export { recordDuplicateDecision, recordExportRetry, recordProcessingFailure, + retireSampleLeftover, transitionRequest, type NewRequest, type RequestRow, } from "./repository"; export { parseCursor, REQUEST_PAGE_SIZE } from "./cursor"; export { canTransition, InvalidTransition, nextStatus, type ErrorStage, type RequestEvent } from "./status"; +export { isSampleLeftover } from "./sample-leftover"; diff --git a/src/features/requests/repository.ts b/src/features/requests/repository.ts index 6436f1e..3ddf5d5 100644 --- a/src/features/requests/repository.ts +++ b/src/features/requests/repository.ts @@ -2,6 +2,7 @@ import { and, asc, desc, eq, gte, inArray, or, sql, type SQL } from "drizzle-orm import { requests, type RequestStatus } from "@/db/schema"; import { tenantOf, type TenantTx } from "@/features/tenancy"; import { parseCursor, REQUEST_PAGE_SIZE } from "./cursor"; +import { isSampleLeftover } from "./sample-leftover"; import { nextStatus, type RequestEvent } from "./status"; export type RequestRow = typeof requests.$inferSelect; @@ -166,16 +167,30 @@ export async function countSamples(tx: TenantTx, statuses: readonly RequestStatu return row?.count ?? 0; } -/** Ids of the tenant's samples in one of `statuses` (#84: leftovers of an aborted seed run). */ -export async function listSampleIds(tx: TenantTx, statuses: readonly RequestStatus[]): Promise { +/** Ids of the tenant's samples an aborted seed run left behind (#84) – see `isSampleLeftover`. */ +export async function listSampleLeftoverIds(tx: TenantTx): Promise { tenantOf(tx); const rows = await tx .select({ id: requests.id }) .from(requests) - .where(and(eq(requests.source, "sample"), inArray(requests.status, [...statuses]))); + .where( + and( + eq(requests.source, "sample"), + or(inArray(requests.status, ["NEW", "PROCESSING"]), and(eq(requests.status, "ERROR"), eq(requests.errorStage, "processing"))), + ), + ); return rows.map((row) => row.id); } +/** + * Settles a sample leftover on a locked row (#84): rejected with `reason`. The rule lives here, not with + * the caller (#92 review) – any other request, or a sample that is no leftover, is left alone (null). + */ +export async function retireSampleLeftover(tx: TenantTx, row: RequestRow, reason: string): Promise { + if (!isSampleLeftover(row)) return null; + return transitionRequest(tx, row, "sample.retired", { rejectionReason: reason, errorStage: null, errorMessage: null, nextRetryAt: null }); +} + /** Requests a user created since `since` (upload rate limit, #59 review) – within the tenant. */ export async function countRequestsCreatedBy(tx: TenantTx, userId: string, since: Date): Promise { tenantOf(tx); diff --git a/src/features/requests/sample-leftover.test.ts b/src/features/requests/sample-leftover.test.ts new file mode 100644 index 0000000..c7a7217 --- /dev/null +++ b/src/features/requests/sample-leftover.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from "vitest"; +import { isSampleLeftover } from "./sample-leftover"; + +// #84 + #92 review: only what an aborted seed run leaves behind – never a decided sample, never an +// approved one whose export failed (it may already be in the ERP and can be exported again). +const sample = { source: "sample" as const, errorStage: null }; + +describe("isSampleLeftover", () => { + it.each(["NEW", "PROCESSING"] as const)("counts a sample in %s", (status) => { + expect(isSampleLeftover({ ...sample, status })).toBe(true); + }); + + it("counts a sample whose processing failed", () => { + expect(isSampleLeftover({ ...sample, status: "ERROR", errorStage: "processing" })).toBe(true); + }); + + it("never counts an approved sample whose export failed", () => { + expect(isSampleLeftover({ ...sample, status: "ERROR", errorStage: "export" })).toBe(false); + }); + + it.each(["REVIEW", "APPROVED", "EXPORTED", "REJECTED"] as const)("never counts a decided or settled sample (%s)", (status) => { + expect(isSampleLeftover({ ...sample, status })).toBe(false); + }); + + it("never counts an upload, whatever its state", () => { + for (const status of ["NEW", "PROCESSING", "ERROR"] as const) { + expect(isSampleLeftover({ source: "upload", status, errorStage: "processing" })).toBe(false); + } + }); +}); diff --git a/src/features/requests/sample-leftover.ts b/src/features/requests/sample-leftover.ts new file mode 100644 index 0000000..6fa521f --- /dev/null +++ b/src/features/requests/sample-leftover.ts @@ -0,0 +1,13 @@ +import type { RequestRow } from "./repository"; + +/** + * A prepared sample (#71) that an aborted seed run left behind (#84). Processing of a sample is never + * queued, so NEW, PROCESSING and ERROR from processing only mean the run broke off. Never a decided sample, + * and never an approved one whose export failed (ERROR from export): it may already be in the ERP and can be + * exported again (#92 review). + */ +export function isSampleLeftover(request: Pick): boolean { + if (request.source !== "sample") return false; + if (request.status === "NEW" || request.status === "PROCESSING") return true; + return request.status === "ERROR" && request.errorStage === "processing"; +} diff --git a/src/features/requests/status.ts b/src/features/requests/status.ts index 59894ba..f8c59ae 100644 --- a/src/features/requests/status.ts +++ b/src/features/requests/status.ts @@ -32,9 +32,9 @@ const TRANSITIONS: Record { return deps.tenancy.withTenant(actor.companyId, async (tx) => { let retired = 0; - for (const id of await listSampleIds(tx, LEFTOVER)) { + for (const id of await listSampleLeftoverIds(tx)) { const request = await lockRequest(tx, id); - if (!request || request.source !== "sample" || !LEFTOVER.includes(request.status)) continue; - await transitionRequest(tx, request, "sample.retired", { rejectionReason: LEFTOVER_REASON, errorStage: null, errorMessage: null, nextRetryAt: null }); + if (!request) continue; + const settled = await retireSampleLeftover(tx, request, LEFTOVER_REASON); + if (!settled) continue; await recordAudit(tx, { actorUserId: actor.userId, action: "request.sample_retired", entityType: "request", entityId: id, data: { from: request.status } }); retired++; } diff --git a/tests/integration/samples.test.ts b/tests/integration/samples.test.ts index b0319a6..83b526d 100644 --- a/tests/integration/samples.test.ts +++ b/tests/integration/samples.test.ts @@ -139,6 +139,20 @@ describe("prepared samples", () => { expect(await jobsFor(QUEUES.exportRequest, exported.id)).toBe(1); }); + it("keeps an approved sample whose export failed – it is no leftover (#92 review)", async () => { + const admin = actors[1]!; + const exported = (await samplesOf(admin)).find((sample) => sample.status === "APPROVED")!; + await stack.database.pool.query("delete from pgboss.job where name = $1 and singleton_key = $2", [QUEUES.exportRequest, exported.id]); + await tenancy.withTenant(admin.companyId, async (tx) => + transitionRequest(tx, (await lockRequest(tx, exported.id))!, "export.failed", { errorStage: "export", errorMessage: "ERP nicht erreichbar." }), + ); + + const { retired } = await seedSamples(deps(), admin); + + expect(retired).toBe(0); + expect(await tenancy.withTenant(admin.companyId, (tx) => getRequest(tx, exported.id))).toMatchObject({ status: "ERROR", errorStage: "export" }); + }); + it("an aborted run leaves the sample in ERROR, not in progress; the next run replaces it; no live reprocess", async () => { const admin = actors[2]!; const brokenStorage = Object.assign(Object.create(storage) as S3BlobStore, { From 37710eaf13c0d4c46ee4c25e1febc252fa35a693 Mon Sep 17 00:00:00 2001 From: Fluory Date: Mon, 28 Sep 2026 14:12:20 +0200 Subject: [PATCH 3/3] fix(requests): break the import cycle of the leftover rule sample-leftover.ts took RequestRow from repository.ts, which imports the rule - dependency-cruiser's no-circular failed in CI. The rule now types its input from the schema. Refs #84 Co-Authored-By: Claude Opus 5.5 (1M context) --- src/features/requests/sample-leftover.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/features/requests/sample-leftover.ts b/src/features/requests/sample-leftover.ts index 6fa521f..eb0d3d0 100644 --- a/src/features/requests/sample-leftover.ts +++ b/src/features/requests/sample-leftover.ts @@ -1,4 +1,11 @@ -import type { RequestRow } from "./repository"; +import type { RequestSource, RequestStatus } from "@/db/schema"; + +// Its own input type from the schema – importing RequestRow from the repository would close a cycle. +export interface SampleLeftoverCandidate { + source: RequestSource; + status: RequestStatus; + errorStage: "processing" | "export" | null; +} /** * A prepared sample (#71) that an aborted seed run left behind (#84). Processing of a sample is never @@ -6,7 +13,7 @@ import type { RequestRow } from "./repository"; * and never an approved one whose export failed (ERROR from export): it may already be in the ERP and can be * exported again (#92 review). */ -export function isSampleLeftover(request: Pick): boolean { +export function isSampleLeftover(request: SampleLeftoverCandidate): boolean { if (request.source !== "sample") return false; if (request.status === "NEW" || request.status === "PROCESSING") return true; return request.status === "ERROR" && request.errorStage === "processing";