feat/energia-led

Hardcoded '2026-04' straddle guard permanently suppresses April 2026 FP findings

domains/utility/src/finding/finding.decisions.ts

detectFpRecompute contains `if (row.yearMonth === '2026-04') continue;` written for the CFE §5.5 umbral transition month when either 95% or 97% might apply. Today is 2026-07-14 — April bills are 3 months old and the correct umbral is now deterministic. Any customer overcharged in April 2026 will never receive an fp_recompute_mismatch finding. Remove the guard now, or replace it with `if (yearMonth < '2026-05') continue;` and document the expiry. The test that locks in the suppression should also be removed.

fetchBillFactRowsForOrg DISTINCT ON axis (contract_number, year_month) wrong for contract-anchored ledger

domains/utility/src/finding/finding.queries.ts

DISTINCT ON (uc.contract_number, b.year_month) collapses to one bill when multiple utility_contracts share the same contract_number and overlap on year_month (compound unique is contract_number+service_id+tariff_id). In a terminate-and-replace scenario where both old and new contracts have a bill for the same month, the finding could be anchored to whichever contract's bill wins the DISTINCT, silently dropping the other. The correct dedup axis for a contract-anchored ledger is (uc.id, b.year_month) not (uc.contract_number, b.year_month).

jsonb_object_agg in lateral join throws on duplicate concept_name within one bill

domains/utility/src/finding/finding.queries.ts

jsonb_object_agg(bcc.concept_name, ...) inside the LATERAL join has no duplicate-key guard. If a bill has two line_items whose conceptId values resolve to the same concept_name, PostgreSQL raises a runtime error rather than silently dropping one. The concept catalog enforces unique concept_name for normally-ingested bills, but a malformed or manually-inserted bill would silently break org-wide finding detection.

FindingDetectedEvent field renames safe — zero consumers today

domains/utility/src/events/finding.events.ts

findingType→type, yearMonth→period, estimatedMxnAnnual→estimatedImpact on the event payload. No handler, subscriber, or Lambda consumer references this event type yet. The rename is safe; when an EventBridge consumer is wired, the in-flight event shape must match.

findByContract unscoped and exported — callers must resolve org access externally

domains/utility/src/finding/finding.queries.ts

findByContract returns all findings for a contract with no org guard. No handler exists in PR2 (domain-only), so there is no live exposure. But the function is exported in FindingFCIS and is available for any PR3+ handler to call without noticing the missing guard. Consider removing it from the public barrel, adding a findByContractForOrg overload, or renaming to make the unscoped nature explicit at the call site.

RLS SELECT policy structurally equivalent to bills — no regression

packages/database/src/schema/utility-contract-findings.ts

ucf_select_member mirrors bills_select_member exactly: same join path, same get_user_org_ids() SECURITY DEFINER helper. No cross-org leakage path exists.

No SQL injection risk in fetchBillFactRowsForOrg raw SQL

domains/utility/src/finding/finding.queries.ts

orgId is interpolated via Drizzle's sql template literal as a parameterized bind value with a ::uuid cast. No injection surface.

updateEstimate also lacks version WHERE predicate and deviates from updateWithVersion naming

domains/utility/src/finding/finding.queries.ts

Same root issue as updateStatus — the currentVersion argument is passed but not used in the WHERE clause. Canonical convention for versioned updates across the codebase is updateWithVersion; descriptive names are fine but only if the optimistic-lock contract is actually enforced. Also: DetectFindingsResult.updated is incremented before the null-check on the returned row, so on a hard-delete race the counter inflates by 1 with no corresponding outbox event.

FINDING_CATEGORIES / FINDING_STATUSES / FINDING_SEVERITIES duplicated across schema and domain type

domains/utility/src/finding/finding.type.ts

These constants are independently defined in both packages/database/src/schema/utility-contract-findings.ts and domains/utility/src/finding/finding.type.ts. Currently they agree. The type-check AssertEqual will catch a drift if the Drizzle { enum } hint diverges from FindingCategory, but a future edit to one file won't cause a compiler error in the other. Canonical pattern: define once in the domain type and import in the schema (or re-export from the schema). FINDING_STATUSES and FINDING_SEVERITIES have the same duplication.

UpsertFindingPatch correctly omits immutable anchor fields

domains/utility/src/finding/finding.decisions.ts

utilityContractId, type, and period are part of the dedupe key, so a key match guarantees they already match the existing row. They are correctly excluded from the patch.

decideUpsertFinding correctly checks billId (not utilityContractId) for change detection

domains/utility/src/finding/finding.decisions.ts

billId can legitimately differ on re-detection when CFE's delete-and-replace supersedes the original bill (onDelete: set null). Checking utilityContractId would be dead code — same dedupeKey guarantees same contract. The change is correct.

findByContract is untested — new query function has zero coverage

domains/utility/src/finding/__tests__/finding.integration.test.ts

findByContract was added as a first-class exported query in this refactor but has no test at all. Given that it is the contract-anchor read path (the named invariant of this PR), a missing test is a critical gap. Suggested: insert a finding on contractA and one on contractB, call findByContract(db, contractA.contractId), assert only contractA's finding is returned.

Outbox events never asserted in integration tests — delivery guarantee is structurally untested

domains/utility/src/finding/__tests__/finding.integration.test.ts

detectFindingsForOrgShell emits utility.finding.detected for every insert/update in the same transaction (the guaranteed-delivery invariant). The integration test only checks DetectFindingsResult counters (inserted, updated, unchanged) but never queries the outbox table to assert an event was written. The entity + outbox in same tx invariant is completely untested here.

12-bill rolling window per-RPU cutoff is completely untested at the DB layer

domains/utility/src/finding/__tests__/finding.integration.test.ts

fetchBillFactRowsForOrg enforces DENSE_RANK() <= 12 per RPU. No test inserts 13+ bills for an RPU and asserts the oldest is excluded. The decisions tests operate on caller-provided slices so windowing is purely a query concern — exercised only indirectly through the idempotent-double-run test which inserts only one bill.

Cross-org isolation assertion is fragile and potentially vacuous

domains/utility/src/finding/__tests__/finding.integration.test.ts

items.items.every((f) => f.utilityContractId === orgA.contractId) works only because orgA has exactly one contract in the test setup. With multiple contracts per org (a valid real-world case), findings on orgA's second contract would be incorrectly rejected. The correct assertion is the negative: .every((f) => f.utilityContractId !== orgB.contractId). Also: Array.every() returns true for an empty array — missing a prerequisite expect(items.items.length).toBeGreaterThan(0) assertion.

decideUpsertFinding noop: numeric-parity case (string DB value vs number command) untested

domains/utility/src/finding/__tests__/finding.decisions.test.ts

The noop test covers the null-null case only. The case where existing.estimatedImpact is a numeric string ('100.00') and command.estimatedImpact is the same number (100) is not tested: existingEstimated = Number('100.00') = 100, command.estimatedImpact = 100 → equal → should noop. A test for this string-to-number equality noop is missing.

detectFpRecompute critical severity threshold boundary ($5000, 20%) untested

domains/utility/src/finding/__tests__/finding.decisions.test.ts

Critical requires BOTH |delta| > 5000 AND relativeDeviation > 0.2. Tests cover clearly-above and clearly-below but not the exact boundary (which uses strict > so |delta| = 5000 should be warning). Missing boundary tests mirror the detectEstimatedReadings suite's pattern (line 545: 'deviation exactly at 20% does NOT escalate to critical').

Integration test missing: status transition at DB layer and optimistic-lock race

domains/utility/src/finding/__tests__/finding.integration.test.ts

The spec lists 'status transitions' as an integration invariant. Only a single happy-path verified transition exists (inside the never-regress-status test). No test covers: (a) updateStatus returning null when version is stale, or (b) the full detected→in_action→verified chain at the DB layer.

rpu in FindingDetectedEvent is sourced from in-memory candidate — not a stable property of the finding

domains/utility/src/finding/finding.shells.ts

Both INSERT and UPDATE outbox events use `rpu: candidate.rpu` (the contract_number from BillFactRow). The finding entity no longer stores rpu, so this field is a denormalized convenience sourced at detection time. Any downstream consumer indexing by the event's rpu may see inconsistencies on re-detection events if the contract-to-RPU mapping is not 1:1. The event type doc should clarify that rpu is detection-time-only and not a stable property of the persisted finding.

Migration 0058 CREATE POLICY statements lack idempotency guard

packages/database/drizzle/0058_steady_human_cannonball.sql

The two CREATE POLICY statements have no IF NOT EXISTS guard. Per packages/database/CLAUDE.md Supabase Branching Caveat, migrations arriving post-fork are marked applied but never run, so manually re-applying via psql will fail on these lines. The FK blocks in the same migration already use the DO $$ BEGIN...EXCEPTION WHEN duplicate_object THEN null; END $$; pattern — wrap the policy statements the same way.

currency: 'MXN' hardcoded in both detectors — breaks provider-neutral claim

domains/utility/src/finding/finding.decisions.ts

detectEstimatedReadings and detectFpRecompute both hardcode currency: 'MXN'. The Finding type header describes currency as ISO-4217; provider-neutral. FindingCandidate exposes currency as a field specifically to support this, but both current detectors bypass it. A named constant CFE_CURRENCY = 'MXN' at module level (or a detector-context object) would make the assumption explicit.

resolveDetectorVersion fallback silently mints version strings for misspelled types

domains/utility/src/finding/finding.shells.ts

resolveDetectorVersion(type) falls back to `${type}@1` for any type not in DETECTOR_VERSIONS. A misspelled type silently produces a wrong version string instead of failing fast. A dev-time assertion or structured warning log on fallthrough would surface the omission at development time.

inArray with Drizzle subquery builder confirmed safe — established codebase pattern

domains/utility/src/finding/finding.queries.ts

Drizzle-orm 0.37.0 supports passing a subquery builder directly to inArray, generating SQL IN (SELECT ...). The same pattern is used in bill.queries.ts and utility-contract.queries.ts. No issue.

updateStatus WHERE clause missing version predicate — optimistic lock is decision-layer-only

domains/utility/src/finding/finding.queries.ts

updateStatus (and updateEstimate) filter by eq(findings.id, id) with no version gate. The doc comment claims 'Returns null if no row was updated (e.g. concurrent modification)' but that is impossible: a valid id will always match. A concurrent status transition (detected→verified races with detected→dismissed) silently wins — the last writer wins without detection. Add and(eq(findings.id, id), eq(findings.version, currentVersion)) to the WHERE clause so a racing write causes 0 rows affected → null return. The decision layer's version check in decideTransitionFindingStatus is necessary but not sufficient — it only catches conflicts visible before the write.