feat/energia-api

Batch shell: in-memory counters diverge from DB on transaction rollback

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

detectFindingsForOrgShell runs all writes in a single db.transaction(). The inserted/updated/unchanged counters are incremented inside the loop BEFORE the transaction commits. If any write throws mid-loop the whole transaction rolls back — zero rows committed — but the shell returns ok({ inserted: N, updated: M }) reflecting the in-memory tally. Logs and callers will report successful upserts that never happened. Additionally a single bad candidate aborts all prior valid upserts in the same batch with no per-candidate isolation.

updateEstimate WHERE clause missing version guard — optimistic lock is illusory

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

updateEstimate filters only by eq(findings.id, id) — it never adds AND findings.version = currentVersion. decideUpsertFinding validates the version in-memory before the decision, but the gap between fetch and write is not closed at the DB level. Two concurrent re-detection runs targeting the same finding both succeed; the second silently overwrites the first. Also deviates from the canonical updateWithVersion naming convention in canonical-form.md.

404 message reflects caller-supplied publicId verbatim

apps/platform/src/api/handlers/finding.handler.ts:119

GET /findings/:publicId returns `notFound('NOT_FOUND', \`Finding not found: ${params.publicId}\`)`. The params.publicId comes from the URL path (validated only as z.string()) and is reflected in the response body. Adding an `fnd_` prefix regex constraint on the pathParams schema would eliminate the reflective content concern and is consistent with how other public-id routes constrain their path params.

Cross-org data isolation verified — org A cannot read org B findings

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

buildOrgScopedFindingsWhere uses inArray(findings.utilityContractId, subquery) where the subquery scopes by sites.org_id = orgId. Both enriched queries apply this gate. The RLS SELECT policy is also correctly wired as defense-in-depth. No cross-org read path exists.

Handler signature: doubly-optional context + redundant inline type annotation

apps/platform/src/api/handlers/finding.handler.ts:37

Both handlers are declared as `typeof findingsContract.X.handler` AND carry a redundant explicit inline parameter type. The typeof annotation already fully constrains the signature; the inline type can drift silently if FindingsListQuery is widened independently. More critically, context is typed as `context?: { nextRequest?: Request }` (doubly optional), making the runtime `throw new Error('Request object is required')` reachable in production without a compile-time guarantee. The canonical pattern uses non-optional destructuring: `async ({ query }, { nextRequest })`. Remove the redundant inline type and make context non-optional.

mapFindingReadError: exported dead code — exhaustiveness switch provides no protection

apps/platform/src/api/mappers/finding.mapper.ts:44

mapFindingReadError is exported but never imported by finding.handler.ts or any route file. The handler calls notFound() directly, bypassing the mapper. The exhaustive switch on error._tag provides no compile-time protection in the active code path. Either wire the handler to use the mapper (returning a proper Result-typed error and dispatching through it) or remove the export. As-is it is dead code with zero protection.

Stale comment in finding/index.ts claims no handler or contract exists

domains/utility/src/finding/index.ts:3

The barrel file header says 'Domain-only in PR2 — no handler, no contract, no ts-rest route'. This was accurate before this PR but is now incorrect: finding.handler.ts, finding.contract.ts, and route registration in [...ts-rest]/route.ts all ship in this PR. The stale comment misleads future readers.

Stale scope comment — all 10 detectors implemented, comment says 2

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

The file header and inline comments state only detectEstimatedReadings and detectFpRecompute are in scope for PR2, with 'Detectors 2/3 land in PR3+'. All 10 detector functions are fully implemented and called by the shell. This is a stale comment creating confusion about the stable API surface.

Shell transaction deviation lacks ADR-016 cross-reference

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

The shell comment documents FETCH+DECIDE running outside the transaction (deliberate deviation from the single-transaction ADR-016 shape). The rationale is sound for a batch detector. However no cross-reference exists in ADR-016 or domain-patterns.md for this batch-detector exception pattern. When the ADR is next updated it should document this exception to avoid the pattern being flagged or incorrectly copied.

ADR-018 exception correctly documented in finding.events.ts

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

Co-location of Zod schemas with TypeScript event types cites 'ADR-018 pragmatic exception'. ADR-018-type-first-pattern.md explicitly permits this. Correctly documented; no action needed.

403 declared in contract — ensureModuleEntitled requirement met

apps/platform/src/api/contracts/finding.contract.ts

Both list and get routes declare 403: JSendFailSchema(['FORBIDDEN']) and the handler calls ensureModuleEntitled before data access. api-patterns.md requirement is fully satisfied.

Cross-org 404 masking correctly implemented — no existence oracle

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

findByPublicIdForOrgEnriched applies both the publicId equality filter AND the org-scoped subquery in the same WHERE clause. Cross-org and missing-id paths both return null, mapped to 404 in the handler. No existence oracle leaks.

No handler test — auth/entitlement gates are not exercised end-to-end

apps/platform/src/api/handlers/finding.handler.ts

No finding.handler.test.ts exists. Both handlers contain two security checkpoints — withAuth (401) and ensureModuleEntitled('energia') (403). The 403 is the only server-side gate for a paid feature. The contract test confirms 403 is declared; the soft-launch test confirms client-side nav state. Neither exercises the handler actually returning 401/403 in a live auth context. Patterns elsewhere (e.g. helioscope-intake-validation.test.ts) show handler-level auth tests are expected.

Outbox event emission not verified in integration test

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

The shell writes utility.finding.detected outbox events in the SAME transaction as the finding row — the FCIS non-negotiable (entity + outbox atomically). The integration test asserts first.value.inserted >= 1 but never queries the outbox table to confirm the event row exists with the correct aggregateId, aggregateType, or eventData. A regression in outboxQueries.insert (wrong aggregate type, missing orgId) would be invisible. The double-run noop path also doesn't assert zero outbox events.

Pagination (limit/offset/total) not exercised in integration tests

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

listByOrgEnriched accepts limit and offset and returns a total count. The integration tests always call it without these options. The dual-query pattern (rows + COUNT(*)) could have a bug where total doesn't respect category/status filters — this would go undetected. At minimum: one test should insert N findings and assert total === N, and another should verify offset skips rows.

Filter parameters (category/status/severity/type) not exercised in integration tests

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

The buildOrgScopedFindingsWhere filters (category, status, severity, type) are never passed to listByOrgEnriched in the integration tests. A broken WHERE clause — e.g. a typo in the column name causing the filter to silently be ignored — is not caught. Contract tests validate the Zod schema; they do not verify that filters reach the SQL.

detectFpChronicPenalty: severity boundary (payback < 24 months) not boundary-tested

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

The test confirms severity === 'critical' when payback < 24 months and 'warning' when hiredDemandKw is null. Missing: payback exactly at 24 months (should be warning) and 25 months (should be warning). The boundary between critical and warning is a billing-grade severity distinction; an off-by-one in the threshold would produce wrong severity in production.

detectDemandOversized: 85% utilization boundary not pinned in tests

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

Tests cover 50% utilization (fires) and 90% (does not fire), but not exactly 85%. A <= vs < off-by-one would only be caught at exactly 85%.

detectConsumptionYoySpike: exactly 15% YoY boundary naming is slightly misleading

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

The boundary direction is correct (strict >15% fires) but the test covers only exactly 15%, not a value just below (e.g. 14.9%). Minor — the boundary direction is validated, but the test name 'at or below' is slightly imprecise.

findByContract: dead code, no production callers

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

findByContract is declared and exported via FindingFCIS but has no callers in handlers, shells, scripts, or tests. Remove while the entity is new to prevent type-drift accumulation.

listByOrg (non-enriched): only used in integration tests, not in production

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

listByOrg is called only by finding.integration.test.ts. All production callers use listByOrgEnriched. Consider whether the integration test should use the enriched query (exercises more code paths), or remove the production export and make it test-local.

isJsonEqual: custom deep-equality could be replaced with JSON.stringify comparison

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

The ~20-line recursive isJsonEqual is used to compare JSONB-origin values. For JSON-serializable data, `JSON.stringify(a) === JSON.stringify(b)` is semantically equivalent and eliminates the custom implementation. Key ordering is not a concern since both sides originate from the same DB column.

EnrichedFinding interface defined in queries file, not type file

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

EnrichedFinding (join result with contractPublicId, rpu, billPublicId) is defined in finding.queries.ts per canonical-form.md the SSOT for domain types is {entity}.type.ts. Low-priority since it exports correctly via FindingFCIS, but future type-check assertions would naturally live in the type file.