feat/tax-regime

resolveTaxRate does not guard against NaN from Number() coercion in the query layer

domains/utility/src/tax-rate/tax-rate.decisions.ts:22

The query layer converts the postgres `numeric` column via `Number(row.rate)`. For a malformed row, `Number()` returns `NaN`, which satisfies `!== null`. `resolveTaxRate` would return `{ rate: NaN, source: 'table' }`, and `NaN × subtotal = NaN` on the bill — the exact failure mode the whole chain aims to prevent. A `Number.isFinite(tableRate)` guard instead of `!== null` would close the gap. No test covers `resolveTaxRate('iva', NaN)`.

'constant' source not logged — no observability when table is unseeded

domains/cross-domain/src/bill-compute.shells.ts:194

The warn fires only on `source === 'default'`. When tax_rates is empty (unseeded preview/local), billing silently uses the TAX_REGIME_RATES constants. The constants are correct, so no billing bug, but there is no observability signal distinguishing 'running off the table' from 'running off a fallback constant'. Intentional per the design; noted for ops visibility.

Customer site identifiers logged to console.warn without structured-log routing

domains/cross-domain/src/bill-compute.shells.ts:184

`sitePublicId` and `yearMonth` appear in the warn JSON. Values are non-secret (opaque ULIDs, non-PII period strings) but routed through `console.warn` which lands in CloudWatch as plaintext without field-level filtering. No injection risk; minor hygiene concern for log governance if field-level access controls are ever needed.

resolveTaxRate parameter typed as `string` rather than `TaxRegime` enum

domains/utility/src/tax-rate/tax-rate.decisions.ts:22

Any string can be passed; unrecognized codes silently apply 16% IVA. The API boundary is closed (TaxRegimeSchema z.enum validates incoming strings), so external risk is nil. Internal misuse (direct domain calls bypassing schema) could produce silent financial defaults. Narrowing to `TaxRegime` type at compile time would make this a type-system invariant.

Decision function not prefixed `decide{Operation}` per canonical naming

domains/utility/src/tax-rate/tax-rate.decisions.ts:14

Canonical-form.md requires decision functions to be named `decide{Operation}` (e.g. `decideResolveTaxRate`). The exported function is `resolveTaxRate`, matching query/utility naming instead. The function is infallible so not returning `Result<T,E>` is acceptable, but the naming convention is independent of return type.

index.ts barrel missing TaxRateFCIS namespace re-export

domains/utility/src/tax-rate/index.ts:1

Per canonical-form.md, `index.ts` should export a `{Entity}FCIS` namespace grouping the entity's public surface. The shell already calls `TaxRateFCIS.resolveTaxRate` and `TaxRateFCIS.findRateByCode`, so the namespace object is expected but not exported from the barrel (only named exports exist).

resolveTaxRate not returning Result<T,E> — intentional and acceptable

domains/utility/src/tax-rate/tax-rate.decisions.ts:14

ADR-016 requires Result<T,E> for fallible operations. resolveTaxRate is total (every path returns ResolvedTaxRate), so omitting Result is correct. Flagged for reviewer awareness only.

console.warn in shell — consider structured logger if one exists

domains/cross-domain/src/bill-compute.shells.ts:195

Project conventions favor a structured logger over raw console.* in shells. Consistent with the file's existing style (other warn calls also use console.warn + JSON.stringify). Standardise when a logger utility lands.

tableRate = 0 (zero-rate) is untested — falsy coercion regression risk

domains/utility/src/tax-rate/__tests__/tax-rate.decisions.test.ts:4

The guard is `tableRate !== null`, which correctly handles 0, but no test passes tableRate = 0. A future refactor tightening the guard to `!tableRate` or `tableRate ?? ...` would silently break zero-rate and this suite would not catch it. Add: `expect(resolveTaxRate('iva', 0)).toEqual({ rate: 0, source: 'table' })`.

tableRate = undefined not exercised — null vs undefined assumption undocumented

domains/utility/src/tax-rate/__tests__/tax-rate.decisions.test.ts:10

The function accepts `number | null` but Drizzle nullable columns sometimes surface as `undefined` after a left-join. `undefined !== null` is true, so `resolveTaxRate('iva', undefined as any)` would fall through the null-check and reach TAX_REGIME_RATES lookup — coincidentally correct but for the wrong reason. A test with undefined documents the assumption and guards against looser callers.

NaN as tableRate untested — original bug vector resurfaces through table branch

domains/utility/src/tax-rate/__tests__/tax-rate.decisions.test.ts:14

The test verifies NaN protection for unknown *regime codes* but not for `NaN` as the *tableRate* itself. `NaN !== null` is true so `resolveTaxRate('iva', NaN)` returns `{ rate: NaN, source: 'table' }`. Add a case: `expect(resolveTaxRate('iva', NaN)).toEqual({ rate: 0.16, source: 'constant' })` (once the guard is fixed).

mockContract.taxRegime fixture assumption is implicit in decideUpdateContract tests

domains/utility/src/utility-contract/__tests__/utility-contract.decisions.test.ts:290

The idempotency test (`taxRegime: 'iva'` unchanged) relies on `mockContract.taxRegime` being `'iva'`. The diff shows `taxRegime: 'iva'` added to the mock, but if it were missing, the test would pass vacuously (`undefined === undefined`). Confirmed present in the diff, so this is just a fragility note.

findRateByCode fires per-bill; multi-bill callers should prefer findAllRates

domains/cross-domain/src/bill-compute.shells.ts:191

computeBillShell issues one SELECT per call. savings-report paths call persistBillForContract 2-3× per site-period (baseline + PPA + CFE_GRID), each hitting tax_rates independently for the same regime code. findAllRates (already exported) loads the full table in one round-trip. A memoised per-request cache would collapse N queries to 1 for multi-bill callers. Low urgency — table is tiny — but worth noting as a hot-loop pattern.

0.16 default literal duplicates TAX_REGIME_RATES.iva

domains/utility/src/tax-rate/tax-rate.decisions.ts:26

The default `{ rate: 0.16, source: 'default' }` hard-codes a value already defined as `TAX_REGIME_RATES.iva`. If the standard IVA rate ever changed, both sites need updating. `{ rate: TAX_REGIME_RATES.iva, source: 'default' }` makes the intent explicit and keeps constants DRY.

console.warn could adopt a structured-log abstraction when one lands

domains/cross-domain/src/bill-compute.shells.ts:195

The unknown_tax_regime warning is the most compliance-sensitive log line in the billing path. It is a natural early adopter for any structured-logger utility. No action before merge.

TaxRegimeSchema not re-exported from api package schemas barrel

packages/api/src/schemas/utility-contract.schemas.ts:63

TaxRegimeSchema is defined and used internally. A future handler or MCP tool validating taxRegime standalone would need to re-derive it or import from an internal path. Consider whether exporting it from the package's exports entry is warranted.

'constant' fallback unreachable in seeded environments — worth documenting

domains/utility/src/tax-rate/tax-rate.decisions.ts:24

The 'constant' fallback can only be reached in tests or a freshly-provisioned environment before the seed runs. The code is correct and the safety is valuable, but a comment noting 'seeding-gap fallback, not a normal operating mode' would help on-call readability.