claude/zealous-lovelace-cbc97f

NULL count fields silently coerce to 0 or NaN — not pinned by a test

domains/utility/src/utility-contract-overview/__tests__/utility-contract-overview.reconcile.unit.test.ts:29

Number(null) === 0 and Number(undefined) === NaN. If the SQL function returns NULL for a count column (e.g. due to a future schema change), the TS wrapper would silently produce 0 or NaN without any test catching it. The test only exercises the string-to-number coercion path ('3' → 3). Adding a fixture with a null count field and asserting NaN or a thrown error would pin this boundary. Not a current bug since the SQL function is documented to return non-null aggregates.

Query function throws instead of returning Result<T,E> — violates ADR-016

domains/utility/src/utility-contract-overview/__tests__/utility-contract-overview.reconcile.unit.test.ts:24

The test asserts rejects.toThrow(/returned no row/) for the no-row invariant guard in runOverviewReconcile. domain-patterns.md and ADR-016 list 'Throwing exceptions for business errors' as an explicit anti-pattern: all fallible operations must return Result<T,E>. An infrastructure-level invariant break (SQL function returned no row) should be modeled as err({ _tag: 'ReconcileInvariantViolation', statusCode: 500 }). The test correctly covers the boundary but is pinning a violation of the project's error-handling convention. Fixing requires updating the production query to return Result, then updating both this unit test and the integration sibling.

Non-standard double-qualifier test file name (.reconcile.unit.test.ts)

domains/utility/src/utility-contract-overview/__tests__/utility-contract-overview.reconcile.unit.test.ts:1

The canonical naming pattern is {entity}.{layer}.test.ts (e.g. .decisions.test.ts, .queries.test.ts, .integration.test.ts). The double qualifier .reconcile.unit. is unique in the repo — the prior commit's sibling .reconcile.integration.test.ts set this precedent. The .unit. qualifier is redundant for a no-DB stub file; the conventional choice would be utility-contract-overview.reconcile.test.ts or utility-contract-overview.queries.unit.test.ts.

Stub's execute return is a plain array — postgres.js RowList protocol not modeled

domains/utility/src/utility-contract-overview/__tests__/utility-contract-overview.reconcile.unit.test.ts:19

The stub returns a plain JS array from execute. The real implementation does Array.from(result as Iterable<ReconcileRow>) which works because arrays are iterable. However, postgres.js's RowList carries additional properties (e.g. columns, count) that the stub silently omits. A comment noting this deliberate simplification, or narrowing stubDb to DbOrTx so future editors understand the contract, would prevent confusion. Not a current bug — Array.from on a plain array behaves correctly.

stubDb accepts unknown[] — column-name typos silently produce NaN/0

domains/utility/src/utility-contract-overview/__tests__/utility-contract-overview.reconcile.unit.test.ts:19

stubDb accepts unknown[], so a typo in a column name (e.g. drift_ccount) silently produces Number(undefined) = NaN. Typing the rows parameter as Partial<Record<'drift_count'|'missing_count'|'orphaned_count'|'mismatch_count'|'repaired_count'|'run_id', string>>[] would make the compiler catch field-name drift, mirroring the SQL column contract the test is meant to pin.