From 022d1935ad50490f065a059ec9562dd3dec5b17e Mon Sep 17 00:00:00 2001 From: Ricardo Mancinas Date: Sat, 15 Aug 2026 01:28:26 -0700 Subject: [PATCH] feat(policy-ocr): set policyTypeId and insuranceProviderId on confirm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The BACKLOG claimed this was blocked on incomplete `policy_types` rows. Querying the dev database says otherwise: AUTO (1316 policies) and LICENCIAS (306) are both live and healthy, so ANA's two faces were never blocked at all. Three separate things had been conflated. What the parser now emits is a NAME, not an id -- it is a pure function over text and must not reach for the database: ANA AUTOMOBILE -> AUTO ANA DRIVER'S POLICY -> LICENCIAS GMX (both documents) -> MULT `resolveLookups()` turns that into a foreign key at confirm, and does the same for the carrier off the parser's provider code. It resolves, never creates: a missing `policy_types` row means a human deleted it, and silently recreating it would undo that with no record. An explicit `policyTypeId` / `insuranceProviderId` on the confirm payload always wins. GMX is MULT rather than INCENDIO because the caratula's own header reads "Multiple Policy / Home" and the especificación is "PVL Hogar" -- one product, two artifacts. MULT is the live row carrying 769 of them; INCENDIO is fire-only and no policy in the book has ever used it. The parser's provider code is not the carrier's row name, so PROVIDER_ROW_NAME maps ANA onto "ANA SEGUROS", which is where the office's 738 ANA policies already are. --- the actual defect underneath ----------------------------------------- `policies.policyTypeId`, `policies.insuranceProviderId` and `claims.adjusterId` are all ON DELETE SET NULL, and the lookups screen deleted unconditionally. So deleting a lookup row returned 200 and silently blanked the field on every row referencing it -- no error, nothing in the UI. That is how M_EMPR disappeared and left 5 policies with no ramo, found months later only by querying. All three deletes now refuse while the row is in use, naming it and the count ("El tipo de póliza «M_EMPR» está en uso por 5 póliza(s)"). The schema-level `onDelete: Restrict` the spec once recommended is deliberately not used: a raw FK error is not something the operator can act on. `20260815160000_policy_type_repair` cleans up what already happened: - restores M_EMPR and re-points its 5 policies, scoped to `policyTypeId IS NULL AND legacySourceTable = 'm_empr'` so it can never claim a policy blanked for some other reason - merges the duplicate "ANA" carrier (1 policy) into "ANA SEGUROS" (738). OCR is about to start assigning the carrier automatically and two rows would keep splitting the book. Written as joins, not subqueries, so both statements are no-ops when either row is absent -- a subquery form would resolve to NULL and blank the carrier off every ANA policy. - does NOT restore INCENDIO. It is the other row the migration would have produced, but the legacy INCENDIO table has 1 row that never loaded, so the type has zero policies and restoring it would only put a dead option in the type picker. Verified by running the repair against the real broken dev data inside a transaction and rolling back: 5 orphans -> 0, ANA/ANA SEGUROS -> one row with 739, and a second run in the same transaction changes nothing. The DDL half matches `prisma migrate diff` exactly. 186 tests pass. Co-Authored-By: Claude Opus 5 --- .../src/policies/lookup-delete-guard.spec.ts | 94 +++++++++++++++++++ apps/api/src/policies/policies.service.ts | 52 +++++++++- .../policy-ocr/parsers/policy-parser.spec.ts | 30 ++++++ .../src/policy-ocr/parsers/policy-parser.ts | 31 +++++- apps/api/src/policy-ocr/policy-ocr.dto.ts | 5 + apps/api/src/policy-ocr/policy-ocr.service.ts | 94 +++++++++++++++++-- apps/web/src/components/PolicyOcrReview.tsx | 7 ++ apps/web/src/lib/types.ts | 5 + docs/BACKLOG.md | 41 +++++--- docs/POLICY_OCR.md | 52 +++++++++- .../migration.sql | 55 +++++++++++ packages/database/prisma/schema.prisma | 5 + 12 files changed, 444 insertions(+), 27 deletions(-) create mode 100644 apps/api/src/policies/lookup-delete-guard.spec.ts create mode 100644 packages/database/prisma/migrations/20260815160000_policy_type_repair/migration.sql diff --git a/apps/api/src/policies/lookup-delete-guard.spec.ts b/apps/api/src/policies/lookup-delete-guard.spec.ts new file mode 100644 index 0000000..a372743 --- /dev/null +++ b/apps/api/src/policies/lookup-delete-guard.spec.ts @@ -0,0 +1,94 @@ +import { BadRequestException } from "@nestjs/common"; +import { PoliciesService } from "./policies.service"; + +/** + * Deleting a lookup row that policies still reference used to succeed and + * silently blank the field on every one of them, because both FKs are + * `ON DELETE SET NULL` (`0000_init`). That is not a hypothetical: it is how + * the `M_EMPR` policy type disappeared from the dev database and left 5 + * policies with a null `policyTypeId`, found only by querying months later. + * + * These tests pin the refusal. They drive the service with a stub client + * rather than a database because what is being asserted is the guard, not + * Prisma — and a test that needed a live MySQL would not run in CI. + */ +function serviceWith(counts: { + policies?: number; + claims?: number; +}): { service: PoliciesService; deleted: string[] } { + const deleted: string[] = []; + const prisma = { + policy: { count: async () => counts.policies ?? 0 }, + claim: { count: async () => counts.claims ?? 0 }, + insuranceProvider: { + findUnique: async () => ({ id: "p1", name: "ANA SEGUROS" }), + delete: async () => { + deleted.push("provider"); + return { id: "p1" }; + }, + }, + policyType: { + findUnique: async () => ({ id: "t1", name: "M_EMPR" }), + delete: async () => { + deleted.push("policyType"); + return { id: "t1" }; + }, + }, + adjuster: { + findUnique: async () => ({ id: "a1", name: "JUAN PEREZ" }), + delete: async () => { + deleted.push("adjuster"); + return { id: "a1" }; + }, + }, + }; + const storage = {} as never; + return { + service: new PoliciesService(prisma as never, storage), + deleted, + }; +} + +describe("lookup deletes refuse while the row is in use", () => { + it("refuses a policy type that policies still carry, and names the count", () => { + const { service, deleted } = serviceWith({ policies: 5 }); + return service.removePolicyType("t1").then( + () => { + throw new Error("expected the delete to be refused"); + }, + (err: unknown) => { + expect(err).toBeInstanceOf(BadRequestException); + // The operator has to be told WHICH row and HOW MANY, or the message + // is not actionable. + expect((err as Error).message).toContain("M_EMPR"); + expect((err as Error).message).toContain("5"); + expect(deleted).toEqual([]); + }, + ); + }); + + it("refuses a carrier that policies still carry", async () => { + const { service, deleted } = serviceWith({ policies: 738 }); + await expect(service.removeProvider("p1")).rejects.toBeInstanceOf( + BadRequestException, + ); + expect(deleted).toEqual([]); + }); + + it("refuses an adjuster still assigned to claims", async () => { + // Same `ON DELETE SET NULL` trap, on `claims.adjusterId`. + const { service, deleted } = serviceWith({ claims: 2 }); + await expect(service.removeAdjuster("a1")).rejects.toBeInstanceOf( + BadRequestException, + ); + expect(deleted).toEqual([]); + }); + + it("allows the delete once nothing references the row", async () => { + const { service, deleted } = serviceWith({ policies: 0, claims: 0 }); + await service.removePolicyType("t1"); + await service.removeProvider("p1"); + await service.removeAdjuster("a1"); + expect(deleted).toEqual(["policyType", "provider", "adjuster"]); + }); +}); diff --git a/apps/api/src/policies/policies.service.ts b/apps/api/src/policies/policies.service.ts index 6c404f2..2103a88 100644 --- a/apps/api/src/policies/policies.service.ts +++ b/apps/api/src/policies/policies.service.ts @@ -1,4 +1,4 @@ -import { Injectable, NotFoundException } from "@nestjs/common"; +import { BadRequestException, Injectable, NotFoundException } from "@nestjs/common"; import { randomUUID } from "node:crypto"; import { Prisma } from "@jorgecuadros/database"; import { PrismaService } from "../prisma/prisma.service"; @@ -554,7 +554,40 @@ export class PoliciesService { updateProvider(id: string, dto: UpdateProviderDto) { return this.prisma.insuranceProvider.update({ where: { id }, data: dto }); } - removeProvider(id: string) { + /** + * Deleting a lookup row that policies still point at is silent data loss. + * + * Both FKs are `ON DELETE SET NULL` (see `0000_init`), so the delete + * succeeds, returns 200, and blanks the field on every policy that used it + * — with no error and nothing in the UI to suggest anything happened. That + * is how the `M_EMPR` policy type disappeared and left 5 policies with a + * null `policyTypeId`, only found later by querying. + * + * Refusing is the whole fix. There is no "are you sure": the operator + * reassigns those policies first, which is work the app cannot do for them + * because only they know which type is correct. + */ + private async assertLookupUnused( + kind: "provider" | "policyType", + id: string, + ): Promise { + const where = kind === "provider" ? { insuranceProviderId: id } : { policyTypeId: id }; + const count = await this.prisma.policy.count({ where }); + if (count === 0) return; + + const label = + kind === "provider" + ? (await this.prisma.insuranceProvider.findUnique({ where: { id } }))?.name + : (await this.prisma.policyType.findUnique({ where: { id } }))?.name; + const noun = kind === "provider" ? "La aseguradora" : "El tipo de póliza"; + throw new BadRequestException( + `${noun} «${label ?? id}» está en uso por ${count} póliza(s). ` + + "Reasígnelas antes de eliminarlo.", + ); + } + + async removeProvider(id: string) { + await this.assertLookupUnused("provider", id); return this.prisma.insuranceProvider.delete({ where: { id } }); } @@ -564,7 +597,8 @@ export class PoliciesService { updatePolicyType(id: string, dto: UpdatePolicyTypeDto) { return this.prisma.policyType.update({ where: { id }, data: dto }); } - removePolicyType(id: string) { + async removePolicyType(id: string) { + await this.assertLookupUnused("policyType", id); return this.prisma.policyType.delete({ where: { id } }); } @@ -574,7 +608,17 @@ export class PoliciesService { updateAdjuster(id: string, dto: UpdateAdjusterDto) { return this.prisma.adjuster.update({ where: { id }, data: dto }); } - removeAdjuster(id: string) { + /** Same `ON DELETE SET NULL` trap as the two above, on `claims.adjusterId`: + * deleting a busy adjuster would quietly strip them off their claims. */ + async removeAdjuster(id: string) { + const count = await this.prisma.claim.count({ where: { adjusterId: id } }); + if (count > 0) { + const row = await this.prisma.adjuster.findUnique({ where: { id } }); + throw new BadRequestException( + `El ajustador «${row?.name ?? id}» está asignado a ${count} siniestro(s). ` + + "Reasígnelos antes de eliminarlo.", + ); + } return this.prisma.adjuster.delete({ where: { id } }); } } diff --git a/apps/api/src/policy-ocr/parsers/policy-parser.spec.ts b/apps/api/src/policy-ocr/parsers/policy-parser.spec.ts index 68c1c1a..d7f2480 100644 --- a/apps/api/src/policy-ocr/parsers/policy-parser.spec.ts +++ b/apps/api/src/policy-ocr/parsers/policy-parser.spec.ts @@ -116,6 +116,13 @@ describe("parsePolicy / GMX", () => { expect(p.coverages.length).toBeGreaterThan(10); }); + it("names the product MULT for confirm to resolve", () => { + // The caratula's own header reads "Multiple Policy / Home". MULT is the + // legacy discriminator for that multi-line home policy; INCENDIO is + // fire-only and no policy in the book has ever used it. + expect(parsePolicy(GMX_FULL).policyTypeName).toBe("MULT"); + }); + it("leaves premium fields null on the certificate page and notes it", () => { const p = parsePolicy(GMX_FULL); expect(p.netPremium).toBeNull(); @@ -405,6 +412,10 @@ describe("parsePolicy / GMX especificación (PVL Hogar)", () => { expect(c["Fenómenos hidrometeorológicos — Sección Contenidos"]?.insuredAmount).toBe(20000); }); + it("names the same product as the caratula — one policy, two artifacts", () => { + expect(parsePolicy(GMX_ESPEC).policyTypeName).toBe("MULT"); + }); + it("carries the underwriting context the fields have no home for", () => { const notes = parsePolicy(GMX_ESPEC).notes.join(" | "); expect(notes).toMatch(/tipo de persona asegurada: Propietario/); @@ -783,6 +794,25 @@ describe("parsePolicy / ANA automobile", () => { }); }); +describe("policy type, as a name for confirm to resolve", () => { + it("names ANA's two faces after the legacy tables they belong to", () => { + expect(parsePolicy(ANA_AUTO_AMPLIA).policyTypeName).toBe("AUTO"); + expect(parsePolicy(ANA_AUTO_RC_DIAS).policyTypeName).toBe("AUTO"); + expect(parsePolicy(ANA_LICENCIA).policyTypeName).toBe("LICENCIAS"); + }); + + it("emits a NAME, never an id — the parser must not need a database", () => { + // Anything id-shaped here would mean the parser had reached for the DB. + for (const p of [ANA_AUTO_AMPLIA, ANA_AUTO_RC_DIAS, ANA_LICENCIA]) { + expect(parsePolicy(p).policyTypeName).toMatch(/^[A-Z_]+$/); + } + }); + + it("leaves the type unnamed when no parser claimed the page", () => { + expect(parsePolicy(page("a laundry receipt")).policyTypeName).toBeNull(); + }); +}); + describe("parsePolicy / ANA responsabilidad civil por días", () => { const p = parsePolicy(ANA_AUTO_RC_DIAS); diff --git a/apps/api/src/policy-ocr/parsers/policy-parser.ts b/apps/api/src/policy-ocr/parsers/policy-parser.ts index 7e12ff7..72f1416 100644 --- a/apps/api/src/policy-ocr/parsers/policy-parser.ts +++ b/apps/api/src/policy-ocr/parsers/policy-parser.ts @@ -8,8 +8,18 @@ import type { OcrPage } from "../../statements/ocr/ocr.provider"; * "field was read" vs "field was not" rather than guessing. */ export interface ParsedPolicy { - /** "GMX" today; the dispatcher lives on `detectProvider`. */ + /** "GMX" | "ANA"; the dispatcher lives on `detectPolicyProvider`. */ provider: string; + /** + * The `PolicyType.name` this document's product corresponds to — "AUTO", + * "LICENCIAS", "MULT". A **name**, not an id: the parser is a pure function + * over text and must not reach for the database, so the confirm step + * resolves it (and leaves the field null if no such row exists). + * + * Null when the layout cannot tell. Guessing is worse than null here — the + * type drives which renewal report a policy appears on. + */ + policyTypeName: string | null; policyNumber: string | null; insuredName: string | null; additionalInsured: string | null; @@ -295,6 +305,7 @@ const PARSERS: Record ParsedPolicy> = { function emptyParsedPolicy(provider: string): ParsedPolicy { return { provider, + policyTypeName: null, policyNumber: null, insuredName: null, additionalInsured: null, @@ -346,6 +357,17 @@ export function parsePolicy(page: OcrPage): ParsedPolicy { * matcher keys on the policy number alone and must not care which artifact * the office happened to upload. */ +/** + * Both GMX documents describe the same product: the caratula's own header + * reads "Multiple Policy / Home" and the especificación is "PVL Hogar". MULT + * is the legacy discriminator for that multi-line home policy, and the live + * row carrying 769 of them. + * + * Not INCENDIO: that row is fire-only and no policy in the book has ever + * used it. + */ +const GMX_POLICY_TYPE = "MULT"; + function parseGmx(page: OcrPage): ParsedPolicy { return isEspecificacion(page.text) ? parseGmxEspecificacion(page) : parseGmxCaratula(page); } @@ -457,6 +479,7 @@ function parseGmxCaratula(page: OcrPage): ParsedPolicy { return { ...emptyParsedPolicy("GMX"), + policyTypeName: GMX_POLICY_TYPE, policyNumber: normalizePolicyNumber(policyNumber), insuredName, additionalInsured, @@ -685,6 +708,7 @@ function parseGmxEspecificacion(page: OcrPage): ParsedPolicy { return { ...emptyParsedPolicy("GMX"), + policyTypeName: GMX_POLICY_TYPE, policyNumber, insuredName, additionalInsured, @@ -1409,6 +1433,8 @@ function parseAnaAutomobile(lines: string[]): ParsedPolicy { return { ...emptyParsedPolicy("ANA"), + // The face that insures a car. + policyTypeName: "AUTO", policyNumber: header.policyNumber, insuredName, agentName: header.agentName, @@ -1805,6 +1831,9 @@ function parseAnaDriverPolicy(lines: string[]): ParsedPolicy { return { ...emptyParsedPolicy("ANA"), + // The office's own name for this product is *licencia*, and LICENCIAS is + // the legacy Access table it was migrated from -- 306 policies. + policyTypeName: "LICENCIAS", policyNumber: header.policyNumber, insuredName: holder?.fullName ?? null, agentName: header.agentName, diff --git a/apps/api/src/policy-ocr/policy-ocr.dto.ts b/apps/api/src/policy-ocr/policy-ocr.dto.ts index 86259e0..2c5f355 100644 --- a/apps/api/src/policy-ocr/policy-ocr.dto.ts +++ b/apps/api/src/policy-ocr/policy-ocr.dto.ts @@ -22,6 +22,11 @@ export class ConfirmPolicyDocumentDto { /** Required when creating a new Policy; ignored if `policyId` is set. */ @IsOptional() @IsString() customerId?: string; + /** Reviewer's explicit lookup picks. Both beat the parsed name; omitted, + * the service resolves `policy_types` / `insurance_providers` by name and + * leaves the FK null when there is no such row. */ + @IsOptional() @IsString() policyTypeId?: string; + @IsOptional() @IsString() insuranceProviderId?: string; /** Set when the document matched an existing Policy. */ @IsOptional() @IsString() policyId?: string; diff --git a/apps/api/src/policy-ocr/policy-ocr.service.ts b/apps/api/src/policy-ocr/policy-ocr.service.ts index 513e28a..1928f9a 100644 --- a/apps/api/src/policy-ocr/policy-ocr.service.ts +++ b/apps/api/src/policy-ocr/policy-ocr.service.ts @@ -205,6 +205,7 @@ export class PolicyOcrService { extractedDriversJson: parsed.drivers.length ? (parsed.drivers as unknown as Prisma.InputJsonValue) : Prisma.DbNull, + extractedPolicyTypeName: parsed.policyTypeName, matchedPolicyId: match.policyId, matchedCustomerId: match.customerId, matchCandidates: match.candidates.length @@ -466,14 +467,18 @@ export class PolicyOcrService { ); } - // 1. Resolve target Policy (create or update). Field selection: every + // 1. Resolve the lookup rows the parser can only name. The reviewer's + // explicit pick always wins; the parsed name is the fallback. + const lookups = await this.resolveLookups(item, doc); + + // 2. Resolve target Policy (create or update). Field selection: every // non-null `extracted*` on the doc (post-review) is written. Null is // preserved — never overwrite an existing Policy's `netPremium` with // null because the certificate page didn't carry one. let policyId = item.policyId ?? null; if (policyId) { - const updateData = buildPolicyUpdateFromDoc(item, doc); + const updateData = buildPolicyUpdateFromDoc(item, doc, lookups); await this.prisma.policy.update({ where: { id: policyId }, data: updateData, @@ -486,25 +491,25 @@ export class PolicyOcrService { `Documento página ${doc.pageNumber}: falta número de póliza.`, ); } - const createData = buildPolicyCreateFromDoc(item, doc, item.customerId!); + const createData = buildPolicyCreateFromDoc(item, doc, item.customerId!, lookups); const created = await this.prisma.policy.create({ data: createData, }); policyId = created.id; } - // 2. Vehicles and named drivers, for the providers whose face carries + // 3. Vehicles and named drivers, for the providers whose face carries // them (ANA's automobile and driver's policies; never GMX Hogar). await this.applyVehiclesAndDrivers(doc, policyId); - // 3. Attach the source PDF as a PolicyDocument. `doc.storageKey` + // 4. Attach the source PDF as a PolicyDocument. `doc.storageKey` // already points at the exact upload (`policy-ocr/{batchId}/source-N.pdf`) // so the attach is just a stream copy into the policy's namespace — // the previous per-page "which file did this page come from" walk is // gone because one PDF = one doc now. await this.attachSourcePdf(doc.storageKey, policyId, doc.provider); - // 4. Optionally post the premium to the ledger. Only when staff + // 5. Optionally post the premium to the ledger. Only when staff // explicitly asked (`postPremium` true) and netPremium parses — without // that gate a missing premium would silently book $0. let postedTransactionId: string | null = null; @@ -562,6 +567,51 @@ export class PolicyOcrService { }; } + /** + * Turn the two things the parser can only NAME into foreign keys. + * + * The parser is a pure function over text and never touches the database, + * so it emits `policyTypeName` ("AUTO") and `provider` ("ANA"). Resolving + * them here keeps that boundary and means a renamed lookup row is a data + * change rather than a parser change. + * + * **Resolve, never create.** A missing `policy_types` row is a signal that + * a human deleted it (that is exactly how M_EMPR disappeared), and silently + * recreating it would undo that decision with no record. The field stays + * null and the reviewer can add the row through the lookups screen. + * + * An explicit pick from the reviewer always beats the parsed name. + */ + private async resolveLookups( + item: ConfirmPolicyDocumentDto, + doc: { extractedPolicyTypeName: string | null; provider: string | null }, + ): Promise<{ policyTypeId?: string; insuranceProviderId?: string }> { + const out: { policyTypeId?: string; insuranceProviderId?: string } = {}; + + if (item.policyTypeId) { + out.policyTypeId = item.policyTypeId; + } else if (doc.extractedPolicyTypeName) { + const row = await this.prisma.policyType.findUnique({ + where: { name: doc.extractedPolicyTypeName }, + select: { id: true }, + }); + if (row) out.policyTypeId = row.id; + } + + if (item.insuranceProviderId) { + out.insuranceProviderId = item.insuranceProviderId; + } else if (doc.provider) { + const name = PROVIDER_ROW_NAME[doc.provider] ?? doc.provider; + const row = await this.prisma.insuranceProvider.findFirst({ + where: { name }, + select: { id: true }, + }); + if (row) out.insuranceProviderId = row.id; + } + + return out; + } + /** * Write the parsed `Vehicle` and `InsuredDriver` rows onto the policy. * @@ -705,6 +755,27 @@ export class PolicyOcrService { } } +/** + * The parser's provider code is not the carrier's row name in + * `insurance_providers`, and the two namespaces are allowed to differ. + * + * ANA is the case that forces this: the office's book is filed under + * "ANA SEGUROS" (738 policies). A bare "ANA" row also existed with 1 policy + * and is merged away by `20260815160000_policy_type_repair`, so an exact-name + * lookup on the parser's "ANA" would find nothing at all after that migration. + * + * Anything not listed resolves by its own name. + */ +const PROVIDER_ROW_NAME: Record = { + ANA: "ANA SEGUROS", +}; + +/** The lookup FKs resolved for one document, absent when unresolvable. */ +interface ResolvedLookups { + policyTypeId?: string; + insuranceProviderId?: string; +} + /** Map a (post-review) doc + final confirmed fields onto a `Policy.update` * payload. Every field that is null in both inputs is omitted so we never * write null over a value the Policy already carries (the GMX certificate @@ -730,6 +801,7 @@ function buildPolicyUpdateFromDoc( extractedPremiumPayment: string | null; extractedCoveragePeriodDays: number | null; }, + lookups: ResolvedLookups, ): Prisma.PolicyUpdateInput { const numOrUndef = (a: number | undefined, b: Prisma.Decimal | null): Prisma.Decimal | undefined => { if (a != null) return new Prisma.Decimal(a); @@ -749,6 +821,13 @@ function buildPolicyUpdateFromDoc( return { policyNumber: strOrUndef(item.policyNumber, doc.extractedPolicyNumber), + // `connect` rather than a raw id: this is the CHECKED update input. Left + // undefined when unresolved, so an existing Policy never loses a type or + // carrier it already had because this document could not name one. + policyType: lookups.policyTypeId ? { connect: { id: lookups.policyTypeId } } : undefined, + insuranceProvider: lookups.insuranceProviderId + ? { connect: { id: lookups.insuranceProviderId } } + : undefined, agentName: strOrUndef(item.agentName, doc.extractedAgentName), policyFrom: dateOrUndef(item.policyFrom, doc.extractedPolicyFrom), policyTo: dateOrUndef(item.policyTo, doc.extractedPolicyTo), @@ -810,6 +889,7 @@ function buildPolicyCreateFromDoc( extractedCoveragePeriodDays: number | null; }, customerId: string, + lookups: ResolvedLookups, ): Prisma.PolicyUncheckedCreateInput { const numOrUndef = (a: number | undefined, b: Prisma.Decimal | null): Prisma.Decimal | undefined => { if (a != null) return new Prisma.Decimal(a); @@ -837,6 +917,8 @@ function buildPolicyCreateFromDoc( return { policyNumber, customerId, + policyTypeId: lookups.policyTypeId, + insuranceProviderId: lookups.insuranceProviderId, agentName: strOrUndef(item.agentName, doc.extractedAgentName), policyFrom: dateOrUndef(item.policyFrom, doc.extractedPolicyFrom), policyTo: dateOrUndef(item.policyTo, doc.extractedPolicyTo), diff --git a/apps/web/src/components/PolicyOcrReview.tsx b/apps/web/src/components/PolicyOcrReview.tsx index eec55b4..7bb2abf 100644 --- a/apps/web/src/components/PolicyOcrReview.tsx +++ b/apps/web/src/components/PolicyOcrReview.tsx @@ -363,6 +363,13 @@ function DocumentRow({ doc, customerIndex, canReview, onSave, onReject }: Docume {doc.extractedInsuredName && ( · {doc.extractedInsuredName} )} + {/* Read-only: the parser names the type, the confirm step resolves it + to a policy_types row. Reassigning it is the policy screen's job, + where the full picker already lives. */} + {doc.extractedPolicyTypeName && ( + {doc.extractedPolicyTypeName} + )} + {doc.provider && {doc.provider}}
diff --git a/apps/web/src/lib/types.ts b/apps/web/src/lib/types.ts index c29d079..e2de3f1 100644 --- a/apps/web/src/lib/types.ts +++ b/apps/web/src/lib/types.ts @@ -1502,6 +1502,8 @@ export interface PolicyOcrDocument { extractedCoveragePeriodDays: number | null; extractedVehiclesJson: PolicyOcrVehicle[] | null; extractedDriversJson: PolicyOcrDriver[] | null; + /** `PolicyType.name` the parser read, resolved to an id only at confirm. */ + extractedPolicyTypeName: string | null; matchedPolicy: { id: string; policyNumber: string | null; @@ -1557,6 +1559,9 @@ export interface PolicyOcrConfirmDocument { premiumPayment?: string; coveragePeriodDays?: number; coveragesJson?: PolicyOcrCoverage[]; + /** Explicit lookup picks; both beat the name the parser read. */ + policyTypeId?: string; + insuranceProviderId?: string; postPremium?: boolean; } diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index b74d3d8..0856c30 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -12,8 +12,8 @@ carries the reasoning. Close an item *there* as well as here, or the two drift. **Verified against dev at compile time** (re-run before trusting the numbers): ``` -policy_types: AUTO, LICENCIAS, MULT -policies NULL policyTypeId: 5 +policy_types: AUTO, LICENCIAS, MULT (+ M_EMPR after 20260815160000) +policies NULL policyTypeId: 5 (0 after 20260815160000) policies pending liquidación: 226 customers: 1536 last tag: v1.0.6 (2026-08-02 02:06 UTC) — 14 commits, 5 migrations behind HEAD @@ -98,16 +98,31 @@ in the same phone call — (55) 5480-4000. ## 2. Live data defects — open, and confirmed open today -### 2.1 `policy_types` is missing `INCENDIO` and `M_EMPR`, and 5 policies are orphaned +### 2.1 ~~`policy_types` missing rows + 5 orphaned policies~~ — FIXED 2026-08-15 `policyTypeId` is `String?` with a plain relation, so Prisma's default is -`SetNull`. The spec's recommended `onDelete: Restrict` was **never applied**. -Five `m_empr` policies lost their ramo; four of them are pending liquidación -and are invisible to every ramo-filtered query — including the pending report -§2 is supposed to produce. +`SetNull`, and `removePolicyType()` had no in-use guard — deleting a lookup row +returned 200 and silently blanked the ramo on every policy using it. That is +what happened to `M_EMPR` and its 5 `m_empr` policies. -Fix alongside the liquidación work (3.1), since it distorts that feature's own -report. Source: INSURANCE "Two defects found while verifying this spec". +Closed by `20260815160000_policy_type_repair` plus the guard in +`policies.service.ts`: + +- `M_EMPR` restored and the 5 policies re-pointed at it, scoped to + `policyTypeId IS NULL AND legacySourceTable = 'm_empr'` so it cannot claim a + policy blanked for some other reason. Idempotent; verified against dev inside + a rolled-back transaction. +- **`INCENDIO` deliberately not recreated.** The legacy `INCENDIO` table has + 1 row and it never loaded, so the type has zero policies — restoring it would + only add a dead option to the type picker. +- Deleting an in-use policy type, carrier or adjuster now **refuses** with the + name and the count. `claims.adjusterId` had the identical `SET NULL` trap and + is guarded too. `onDelete: Restrict` at the schema level was not applied — + the application guard gives a Spanish message the operator can act on, where + a raw FK error would not. +- The duplicate `ANA` carrier row (1 policy) was merged into `ANA SEGUROS` + (738), since OCR now assigns the carrier automatically and two rows would + keep splitting the book. ### 2.2 ≤41 MULT second settlements were dropped in migration @@ -172,10 +187,10 @@ Each of these is a known, deliberate stopping point rather than a bug. all; reading the separate receipt and pairing it to its certificate is what would let `postPremium` stop being a manual tick. A.N.A. prints its premium on the face, so this is a GMX-only gap. -- **No `policyTypeId` from OCR.** A.N.A.'s two faces are distinguishable in the - parser (automobile vs driver's policy) and the platform has a `PolicyType` - discriminator, but confirm never sets one — partly because the `policy_types` - rows are themselves incomplete (INSURANCE §live defects). +- **No `insuranceProviderId` beyond the two OCR carriers.** Confirm resolves + the parser's provider to an `insurance_providers` row by name, so GMX and + A.N.A. land correctly; a policy typed in by hand still gets whatever the + operator picks. - **No versioning.** A re-issued policy arrives as a new certificate with the same number and confirm updates the existing row. Nothing records that this is the 2027 issue of that policy. diff --git a/docs/POLICY_OCR.md b/docs/POLICY_OCR.md index 81b0c4c..2681293 100644 --- a/docs/POLICY_OCR.md +++ b/docs/POLICY_OCR.md @@ -34,7 +34,7 @@ was **reused, not copied**. |---|---| | API module | `apps/api/src/policy-ocr/` (service, controller, DTOs, matcher, parser) | | Shared OCR seam | `apps/api/src/ocr/ocr.module.ts` | -| Tables | `policy_ocr_batches`, `policy_ocr_documents` (`20260801000000_policy_ocr_intake`, extended by `20260815120000_policy_ocr_ana`) | +| Tables | `policy_ocr_batches`, `policy_ocr_documents` (`20260801000000_policy_ocr_intake`, extended by `20260815120000_policy_ocr_ana` and `20260815160000_policy_type_repair`) | | Web | `components/PolicyCaptura.tsx` (tab shell), `PolicyOcrIntake.tsx` (upload), `PolicyOcrReview.tsx` (review queue) | | Abilities | `policy:ingest`, `policy:ocr-review` — both **STAFF** | @@ -320,13 +320,55 @@ Deductible and loss participation are stored as **strings** (`"5%"`, `"20%"`, `"USD 1,000"`) — they are printed as a mix of percentages, currency amounts and free text, and normalising them would lose the distinction. +## Policy type and carrier + +Confirm sets `Policy.policyTypeId` and `Policy.insuranceProviderId` from what +the parser read. + +| document | `policyTypeName` | +|---|---| +| ANA `AUTOMOBILE` | `AUTO` | +| ANA `DRIVER´S POLICY` | `LICENCIAS` | +| GMX caratula **and** especificación | `MULT` | + +The parser emits a **name**, never an id — it is a pure function over text and +must not reach for the database, so `resolveLookups()` in the service turns the +name into a foreign key. A renamed lookup row is then a data change rather than +a parser change. + +**Resolve, never create.** A missing `policy_types` row means a human deleted +it, and silently recreating it would undo that with no record. The field stays +null and the reviewer adds the row through the lookups screen. An explicit +`policyTypeId` / `insuranceProviderId` on the confirm payload always wins. + +Two judgement calls worth recording: + +- **GMX is `MULT`, not `INCENDIO`.** The caratula's own header reads "Multiple + Policy / Home" and the especificación is "PVL Hogar" — one product, two + artifacts. `MULT` is the live row carrying 769 of them; `INCENDIO` is + fire-only and no policy in the book has ever used it. +- **The parser's provider code is not the carrier's row name.** The office's + book is filed under `ANA SEGUROS`, so `PROVIDER_ROW_NAME` maps `ANA` onto it. + A bare `ANA` row with 1 policy also existed and is merged away by + `20260815160000_policy_type_repair`. + +> **Deleting a lookup row used to be silent data loss.** `policies.policyTypeId`, +> `policies.insuranceProviderId` and `claims.adjusterId` are all +> `ON DELETE SET NULL`, and the lookups screen deleted unconditionally — so the +> delete returned 200 and blanked the field on every row that used it. That is +> how `M_EMPR` vanished and left 5 policies with no ramo, found months later by +> querying. All three deletes now refuse while the row is in use, naming it and +> the count. See `assertLookupUnused` and BACKLOG §2.1. + ## Confirm: what actually gets written Per confirmed document, in order: 1. **The `Policy` row** — updated if a policy was matched, created under the picked customer if not. Only non-null `extracted*` fields are written; null - never overwrites existing data. + never overwrites existing data. `policyTypeId` and `insuranceProviderId` are + resolved first (above) and left untouched when unresolvable, so an existing + policy never loses a type or carrier it already had. 2. **`Vehicle` and `InsuredDriver` rows** — for the providers whose face carries them (A.N.A.; never GMX Hogar), skipping any that already exist on the policy. See *Vehicles and drivers* above. @@ -374,7 +416,7 @@ feature is disabled. ## Tests -`apps/api/src/policy-ocr/parsers/policy-parser.spec.ts` — 53 cases against +`apps/api/src/policy-ocr/parsers/policy-parser.spec.ts` — 58 cases against verbatim text extracted from five real documents, indentation and blank lines included (the column positions are what the parser reads, so a cleaned-up fixture would test nothing — and on A.N.A.'s driver's policy the offsets are @@ -398,6 +440,10 @@ sublimit block whose amount sits after both a blank line and a page break, the excluded earthquake coverage, and the hydrometeorological deductible and coinsurance pulled from their own per-zone block. +Plus four cases in `apps/api/src/policies/lookup-delete-guard.spec.ts` pinning +the refusal that stops a lookup delete from silently blanking the rows that use +it, and four in the parser suite on the policy-type NAME each document yields. + From the three A.N.A. PDFs: brand detection (and that GMX's layout rules cannot claim an ANA page), the header band, DD MM YYYY read out of three separate column cells, the six money cells with `DISCOUNT` printed as a bare diff --git a/packages/database/prisma/migrations/20260815160000_policy_type_repair/migration.sql b/packages/database/prisma/migrations/20260815160000_policy_type_repair/migration.sql new file mode 100644 index 0000000..95eac1e --- /dev/null +++ b/packages/database/prisma/migrations/20260815160000_policy_type_repair/migration.sql @@ -0,0 +1,55 @@ +-- The policy type the OCR parser read the product as, resolved to a +-- `policy_types` row at confirm time. +ALTER TABLE `policy_ocr_documents` + ADD COLUMN `extractedPolicyTypeName` VARCHAR(191) NULL; + +-- --------------------------------------------------------------------------- +-- Repair: M_EMPR was deleted from the lookups screen and took its policies' +-- type with it. +-- +-- `policies.policyTypeId` is ON DELETE SET NULL, and removePolicyType() had no +-- in-use guard, so deleting the row silently blanked the field on every policy +-- referencing it — 5 of them, all from the legacy `m_empr` table. The guard +-- against a repeat ships in the same change as this migration. What follows +-- repairs what already happened. +-- +-- Idempotent on purpose: `policy_types.name` is UNIQUE so the INSERT IGNORE is +-- a no-op once the row exists, and the UPDATE is scoped to rows that are still +-- null AND came from that one legacy table, so it can never claim a policy +-- whose type was blanked for some other reason. +INSERT IGNORE INTO `policy_types` (`id`, `name`) VALUES (UUID(), 'M_EMPR'); + +UPDATE `policies` p + JOIN `policy_types` pt ON pt.`name` = 'M_EMPR' + SET p.`policyTypeId` = pt.`id` + WHERE p.`policyTypeId` IS NULL + AND p.`legacySourceTable` = 'm_empr'; + +-- INCENDIO is deliberately NOT recreated. It is the other row the migration +-- would have produced, but no policy in the book has ever carried it, so +-- adding it back would only put a dead option in the type picker. + +-- --------------------------------------------------------------------------- +-- Merge the duplicate ANA carrier. +-- +-- `insurance_providers` holds both "ANA" (1 policy) and "ANA SEGUROS" (738). +-- They are one carrier, and OCR is about to start assigning it automatically — +-- picking either row while both exist would keep splitting the book. +-- +-- "ANA SEGUROS" is the survivor because it is where the 738 already are. +-- +-- Written as joins rather than subqueries so that BOTH statements are no-ops +-- when either row is absent (a fresh database, or one where this was already +-- tidied by hand). A subquery form would resolve to NULL and blank the +-- carrier off every ANA policy. +UPDATE `policies` p + JOIN `insurance_providers` dup ON dup.`id` = p.`insuranceProviderId` AND dup.`name` = 'ANA' + JOIN `insurance_providers` keep ON keep.`name` = 'ANA SEGUROS' + SET p.`insuranceProviderId` = keep.`id`; + +DELETE dup FROM `insurance_providers` dup + JOIN `insurance_providers` keep ON keep.`name` = 'ANA SEGUROS' + WHERE dup.`name` = 'ANA' + -- Belt and braces: never drop a row that still has policies hanging off + -- it, whatever the UPDATE above did or did not manage to move. + AND NOT EXISTS (SELECT 1 FROM `policies` p WHERE p.`insuranceProviderId` = dup.`id`); diff --git a/packages/database/prisma/schema.prisma b/packages/database/prisma/schema.prisma index 2cdeda4..aa06be4 100644 --- a/packages/database/prisma/schema.prisma +++ b/packages/database/prisma/schema.prisma @@ -461,6 +461,11 @@ model PolicyOcrDocument { /// POLICY HOLDER list on its driver's policy. Written to `InsuredDriver` /// rows on confirm. extractedDriversJson Json? + /// The `PolicyType.name` the parser read the product as ("AUTO", + /// "LICENCIAS", "MULT"). A NAME, not an id — the parser never touches the + /// database, so confirm resolves it against `policy_types` and leaves + /// `Policy.policyTypeId` null if there is no such row. + extractedPolicyTypeName String? // Match by `Policy.policyNumber` → existing Policy / Customer. matchedPolicyId String?