mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
Stop claiming a summary is pending when auto-read is off
The upload path enqueued a summary job and stamped PENDING without consulting the owner's auto-read opt-in; the job's opt-out branch then deliberately left the state alone, so a document uploaded with the opt-in off said "summary is being generated" forever. Two writers, contradictory assumptions. Both ends of the contract now agree. enqueueDocumentSummary checks the opt-in first and refuses to mint a job (or a PENDING) it knows the job cannot resolve; the job keeps its own check for a pref flipped between enqueue and run and heals a stranded PENDING back to NONE, scoped so READY / WITHHELD / UNAVAILABLE stay untouched. Migration 0333 drains the rows already orphaned. The view needs nothing: NONE already renders the manual summary action. Refs #788
This commit is contained in:
1 parent
c8b3f49915
commit
2a85da67f7
3 files changed
+123
-12
No files matched your search
@@ -0,0 +1,12 @@
|
||||
-- Refs #788 — heal phantom summary PENDING rows.
|
||||
-- `enqueueDocumentSummary` used to claim PENDING even when the owner's
|
||||
-- auto-read opt-in was OFF; the job's opt-out branch then left the state
|
||||
-- alone, so the detail view said "being generated" forever. Both writers now
|
||||
-- agree (the enqueue end no longer claims what the job cannot resolve, the
|
||||
-- job end heals PENDING→NONE on opt-out); this one-time data fix drains the
|
||||
-- rows already stranded. Safe by construction: READY / WITHHELD / UNAVAILABLE
|
||||
-- are never touched, and a job that is genuinely mid-flight rewrites its
|
||||
-- terminal state after this UPDATE anyway.
|
||||
UPDATE "inbound_documents"
|
||||
SET "summary_state" = 'NONE'
|
||||
WHERE "summary_state" = 'PENDING';
|
||||
@@ -55,8 +55,12 @@ vi.mock("@/lib/ai/ai-budgets", () => ({
|
||||
AI_BUDGETS: { documentSummary: { temperature: 0.3, maxTokens: 600 } },
|
||||
}));
|
||||
|
||||
import { runDocumentSummaryJob } from "../document-summary";
|
||||
import {
|
||||
enqueueDocumentSummary,
|
||||
runDocumentSummaryJob,
|
||||
} from "../document-summary";
|
||||
import { prisma } from "@/lib/db";
|
||||
import { getGlobalBoss } from "@/lib/jobs/boss-instance";
|
||||
import { documentAutoReadEnabled } from "@/lib/documents/document-settings";
|
||||
import { resolveDocumentVisionProvider } from "@/lib/documents/provider-order";
|
||||
import {
|
||||
@@ -124,14 +128,19 @@ beforeEach(() => {
|
||||
});
|
||||
|
||||
describe("runDocumentSummaryJob — gating", () => {
|
||||
it("does NOTHING when the opt-in is OFF (no provider call, no write)", async () => {
|
||||
it("does no provider work when the opt-in is OFF (only the PENDING heal may write)", async () => {
|
||||
vi.mocked(documentAutoReadEnabled).mockResolvedValue(false);
|
||||
|
||||
await runDocumentSummaryJob({ userId: "user-1", documentId: "doc-1" });
|
||||
|
||||
expect(resolveDocumentVisionProvider).not.toHaveBeenCalled();
|
||||
expect(runDocumentSummary).not.toHaveBeenCalled();
|
||||
expect(prisma.inboundDocument.updateMany).not.toHaveBeenCalled();
|
||||
// The single permitted write is the PENDING→NONE heal; nothing may claim
|
||||
// an attempt (UNAVAILABLE/WITHHELD) or a summary (READY) here.
|
||||
const calls = vi.mocked(prisma.inboundDocument.updateMany).mock.calls;
|
||||
for (const [arg] of calls) {
|
||||
expect(arg.data).toEqual({ summaryState: "NONE" });
|
||||
}
|
||||
});
|
||||
|
||||
it("generates and PERSISTS the summary encrypted when opt-in is ON + a provider is configured", async () => {
|
||||
@@ -248,4 +257,64 @@ describe("runDocumentSummaryJob — gating", () => {
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("heals a phantom PENDING back to NONE when the opt-in is OFF", async () => {
|
||||
vi.mocked(documentAutoReadEnabled).mockResolvedValue(false);
|
||||
|
||||
await runDocumentSummaryJob({ userId: "user-1", documentId: "doc-1" });
|
||||
|
||||
// The opt-out branch may not leave a claimed PENDING standing — the view
|
||||
// reads PENDING as "being generated" and this run is the only writer that
|
||||
// can resolve it. The heal is scoped to PENDING: UNAVAILABLE / WITHHELD
|
||||
// record real attempts and stay, READY is never touched.
|
||||
expect(prisma.inboundDocument.updateMany).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
where: expect.objectContaining({ summaryState: "PENDING" }),
|
||||
data: { summaryState: "NONE" },
|
||||
}),
|
||||
);
|
||||
// Still strictly no provider work.
|
||||
expect(resolveDocumentVisionProvider).not.toHaveBeenCalled();
|
||||
expect(runDocumentSummary).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("enqueueDocumentSummary — the pending contract", () => {
|
||||
it("does not enqueue and does not claim PENDING when the opt-in is OFF", async () => {
|
||||
vi.mocked(documentAutoReadEnabled).mockResolvedValue(false);
|
||||
const send = vi.fn().mockResolvedValue("job-1");
|
||||
vi.mocked(getGlobalBoss).mockReturnValue({ send } as never);
|
||||
|
||||
const result = await enqueueDocumentSummary("user-1", "doc-1");
|
||||
|
||||
// With auto-read OFF the job's only move is the opt-out no-op, so a
|
||||
// PENDING claimed here could never be resolved to a real outcome. No job,
|
||||
// no state write — the view keeps offering the manual action.
|
||||
expect(result).toEqual({ enqueued: false });
|
||||
expect(send).not.toHaveBeenCalled();
|
||||
expect(prisma.inboundDocument.updateMany).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("enqueues and claims PENDING when the opt-in is ON and a job was created", async () => {
|
||||
const send = vi.fn().mockResolvedValue("job-1");
|
||||
vi.mocked(getGlobalBoss).mockReturnValue({ send } as never);
|
||||
|
||||
const result = await enqueueDocumentSummary("user-1", "doc-1");
|
||||
|
||||
expect(result).toEqual({ enqueued: true });
|
||||
expect(send).toHaveBeenCalledTimes(1);
|
||||
expect(prisma.inboundDocument.updateMany).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ data: { summaryState: "PENDING" } }),
|
||||
);
|
||||
});
|
||||
|
||||
it("claims nothing when the boss dropped the send (no job id)", async () => {
|
||||
const send = vi.fn().mockResolvedValue(null);
|
||||
vi.mocked(getGlobalBoss).mockReturnValue({ send } as never);
|
||||
|
||||
const result = await enqueueDocumentSummary("user-1", "doc-1");
|
||||
|
||||
expect(result).toEqual({ enqueued: false });
|
||||
expect(prisma.inboundDocument.updateMany).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -109,8 +109,23 @@ export async function runDocumentSummaryJob(
|
||||
// on-demand route stays the only way to summarise). Fail-closed on a missing
|
||||
// row.
|
||||
if (!(await documentAutoReadEnabled(userId))) {
|
||||
// Opting out is not a failed attempt — leave the state alone so the view
|
||||
// keeps offering the manual action rather than claiming we tried.
|
||||
// Opting out is not a failed attempt — but a claimed PENDING may not
|
||||
// outlive this run either. PENDING promises the reader "a job will resolve
|
||||
// this", and this job IS that resolution: with the opt-in OFF the honest
|
||||
// terminal state is NONE (never attempted), which is what the enqueue end
|
||||
// of the contract (`enqueueDocumentSummary`) also concludes when the pref
|
||||
// is off. The heal is scoped to PENDING only — NONE needs no write,
|
||||
// UNAVAILABLE / WITHHELD record real attempts, READY is a stored summary.
|
||||
// This also drains any phantom PENDING left by a pref flip mid-flight.
|
||||
await prisma.inboundDocument.updateMany({
|
||||
where: {
|
||||
id: documentId,
|
||||
userId,
|
||||
deletedAt: null,
|
||||
summaryState: "PENDING",
|
||||
},
|
||||
data: { summaryState: "NONE" },
|
||||
});
|
||||
annotate({
|
||||
action: { name: "documents.summary.autoSkipped" },
|
||||
meta: { documentId, reason: "opt-out" },
|
||||
@@ -282,14 +297,29 @@ export async function enqueueDocumentSummary(
|
||||
userId: string,
|
||||
documentId: string,
|
||||
): Promise<{ enqueued: boolean }> {
|
||||
const boss = getGlobalBoss();
|
||||
if (!boss) return { enqueued: false };
|
||||
const payload: DocumentSummaryPayload = {
|
||||
userId,
|
||||
documentId,
|
||||
enqueuedAt: new Date().toISOString(),
|
||||
};
|
||||
try {
|
||||
// Contract, enqueue end: PENDING may only be claimed for work the job can
|
||||
// terminally resolve. With the auto-read opt-in OFF the job's only branch
|
||||
// is the opt-out no-op, so enqueueing would mint a job whose sole purpose
|
||||
// is to undo this function's own PENDING — don't. No job, no state write;
|
||||
// the document stays NONE and the view offers the manual action. The job
|
||||
// keeps its own pref check (belt-and-braces for a pref flipped between
|
||||
// enqueue and run) and heals PENDING→NONE there — the other end of this
|
||||
// contract lives in `runDocumentSummaryJob`'s opt-out branch.
|
||||
if (!(await documentAutoReadEnabled(userId))) {
|
||||
annotate({
|
||||
action: { name: "documents.summary.autoSkipped" },
|
||||
meta: { documentId, reason: "opt-out" },
|
||||
});
|
||||
return { enqueued: false };
|
||||
}
|
||||
const boss = getGlobalBoss();
|
||||
if (!boss) return { enqueued: false };
|
||||
const payload: DocumentSummaryPayload = {
|
||||
userId,
|
||||
documentId,
|
||||
enqueuedAt: new Date().toISOString(),
|
||||
};
|
||||
const jobId = await boss.send(DOCUMENT_SUMMARY_QUEUE, payload, {
|
||||
retryLimit: 2,
|
||||
retryDelay: 30,
|
||||
|
||||
Reference in new issue
Block a user