feat/energia-det

`updated` counter incremented before null-check on updateEstimate result

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

In the update branch, `updated += 1` runs unconditionally before the `if (finding)` guard that gates the outbox event. If `updateEstimate` returns null (concurrent delete between batch-fetch and write), the shell returns updated:1 even though zero DB rows were modified. Fix: move `updated += 1` inside the `if (finding)` block.

createBillFactRow test factory missing 5 required BillFactRow fields

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

The factory omits `measuredDemandKw`, `totalNet`, `totalGross`, `tax`, `dapMxn` — all required non-optional fields on BillFactRow. TypeScript misses this because test files are excluded from tsconfig. Any future test using these rows for demand/inactive/VAT/DAP detectors will silently get undefined instead of null. The correct factory is `row()` in finding.detectors.test.ts (line 30–53).

detectInactiveServiceCharges skips dead meters on demand-only tariffs

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

The pre-filter `r.kwhTotal !== null` excludes demand-only tariff meters (GDMTH/GDMTO/DIST/DIT) where kwhTotal is always null. A de-energised substation on GDMTH with zero measured demand + fixed charge is exactly the inactive-service pattern but is never flagged. Relax the filter or handle null kwhTotal explicitly.

updateEstimate / updateStatus missing version = currentVersion in WHERE clause

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

Both functions write version: currentVersion+1 in SET but don't add AND version = currentVersion to WHERE. A concurrent actor that already bumped the version will be silently overwritten. The FindingVersionConflictError type and constructor exist; anchor the WHERE clause: .where(and(eq(findings.id, id), eq(findings.version, currentVersion))) to actually use them.

sql.raw with unparameterized UUID array interpolation in runner

scripts/energia/detect-findings.ts:131

The summary query builds `ARRAY['${id}',...']::uuid[]` via string concatenation inside `sql.raw(...)`, bypassing Drizzle's parameterisation. UUIDs come from a prior DB SELECT (low injection risk today), but the pattern is structurally unsafe and should not exist as a model. Use `inArray` or `sql` tagged template with bind params, mirroring `finding.queries.ts` `listByOrg`.

findByContract / findByDedupeKey are org-unscoped — cross-org leakage risk when used from handlers

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

Both functions return findings without any org constraint. No handler ships in this PR, but the queries are exported in FindingFCIS. A future handler calling findByContract with a user-supplied utilityContractId without prior org-membership verification would expose findings from any org. Either add an orgId parameter or mark these service-role-only via doc comment.

detectFindingsForOrgShell has no auth guard — any caller with a DB handle can scan any org

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

The shell accepts an arbitrary orgId without verifying the actor is authorised to scan it. It uses raw service-role db (not createRLSDb), so RLS never fires. The actor context is used for logging/outbox attribution only. If a handler wires this shell without checking actor.orgId === orgId, any authenticated user could trigger detection for any org. Add org-membership enforcement or document clearly that callers MUST enforce it before this shell gets a handler in PR3.

DATABASE_URL partially exposed in runner stdout

scripts/energia/detect-findings.ts:68

The script logs the DB URL with password redacted but host/username/dbname visible. On a preview stack the host encodes the Supabase project ref. Log only an environment label or just the host portion.

Detector functions named detect* instead of decide* in finding.decisions.ts

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

Canonical naming: pure decision functions are decide{Operation}. The 10 detector functions use detect* (not decide*) and stretch the file to 1097 lines. The file comment acknowledges this as a deliberate port pattern. Not a hard FCIS violation (functions are pure), but deviates from the naming convention. A dedicated finding.detectors.ts would follow canonical file-per-role structure.

transitionFindingStatusShell missing — decision has no shell wrapper

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

decideTransitionFindingStatus is implemented but no shell function wraps the fetch→decide→write choreography. A future PR3 handler may bypass the shell pattern. Track as PR3 prerequisite.

createHash import: acknowledged synchronous-computation exception to no-I/O rule

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

File-level comment explicitly acknowledges the createHash import and justifies it as a deterministic synchronous computation matching the precedent in lib/bill-unique-index.ts. No action needed.

Barrel exports finding types directly alongside FindingFCIS namespace — consistent with domain pattern

domains/utility/src/index.ts:125

Consistent with how bill, utility-contract, and other entities are exported. No deviation from actual convention. No action needed.

Outbox event emission not verified in integration tests

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

The idempotency test verifies inserted/updated/unchanged counts but never queries the outbox_events table. Per FCIS rules, outbox events must land in the same transaction; if the outboxQueries.insert call ever silently fails, the finding persists but the downstream event never fires. Add: assert outbox row with event_type='utility.finding.detected' exists after first run; second run emits zero new outbox rows.

No integration test for the update path (re-detection changes estimate/severity)

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

The double-run test only covers insert (first run) and noop (second identical run). No test modifies bill data between runs to trigger an `update` decision and verify the row is patched in the DB plus a second outbox event emitted. The update branch in finding.shells.ts lines 211–239 is untested at the DB level.

fetchBillFactRowsForOrg SQL only exercised for estimated_reading concept in integration tests

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

The integration test seeds only `currentReadingType: E` and `kwh`. The CTE extracts 8+ concept names (powerFactor, powerFactorCharge, kw, totalBill, dap, hiredDemand…). A typo in any concept_name string would produce silent nulls that break 7 of the 10 detectors at runtime but pass all current tests.

No empty-input test for any detector

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

None of the 10 detectors are tested with an empty [] input. The shell has an early-exit (shells.ts:127), but the detectors themselves iterate the array — empty-input behavior is untested across all 10.

Threshold boundary tests missing for multiple detectors

domains/utility/src/finding/__tests__/finding.detectors.test.ts:153

consumptionYoySpike: no test for exactly 15.01% (should fire warning) or exactly 40% (confirm stays warning). detectDemandOversized: no test at exactly 85% utilisation. detectInactiveServiceCharges: no test at exactly 6 bills + $1,000 avg. These probe the strict vs. >= boundary in each condition.

listByOrg status filter and pagination not integration-tested

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

findingQueries.listByOrg supports status filter + limit/offset pagination but no integration test exercises these parameters. A broken WHERE clause on status or off-by-one in offset would go undetected.

Concurrent deduplication race not handled or tested

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

The schema has a UNIQUE constraint on dedupeKey but the shell has no 23505 unique-constraint catch. Two concurrent detectFindingsForOrgShell calls for the same org could throw an uncaught DB error instead of returning ok(). Not tested.

Integration tests gated on POSTGRES_URL with no describe.skipIf guard

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

Follows monitoring-subscription.integration.test.ts precedent. CI without a DB URL throws rather than skips — less actionable error messages. Consistent with domain convention; low priority.

groupByRpuDesc called independently 10 times on the same input

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

Each of the 8+ detectors calls groupByRpuDesc(input) independently, performing the full O(n) grouping + per-RPU sort 10 times on identical data. The shell has the full rows array before dispatching — pass a pre-built Map<string,BillFactRow[]> into each detector instead. Matters for nightly Lambda runs over large orgs.

Outbox event assembly duplicated between insert and update branches

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

The outboxQueries.insert call with identical eventType/aggregateType/eventData shape appears twice (insert path lines 190–208, update path lines 220–238). Extract an emitFindingDetected(tx, finding, orgId, candidate, actor) helper. Reduces divergence risk when eventData fields evolve.

Shell opens a DB transaction even when every decision is noop

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

After decideUpsertFinding runs for all candidates, the shell unconditionally opens a transaction. If every decision.action === 'noop' (common on re-runs), the transaction opens, iterates, and commits with zero writes. Add a hasWrites guard before the transaction.

Inconsistent window-slicing approach across detectors (4 patterns for the same operation)

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

Four distinct patterns for 'get each RPU's non-legado last-N rows': takeRecentPerRpu, groupByRpuDesc+filter+slice, inline groupByRpuDesc+filter, and groupByRpuDesc without legado filter. Extract a sliceNonLegadoWindow helper to make intentional legado-inclusive detectors visibly distinct.