feat/tax-regime

CHECK constraint will fail migration if existing rows have out-of-range rates

packages/database/drizzle/0060_needy_menace.sql:1

ALTER TABLE … ADD CONSTRAINT validates all existing rows at migration time. If any row has a rate stored as a percentage (e.g. 16 instead of 0.16), the migration fails. This surfaces a latent data bug, which is desirable, but is a deploy risk that should be confirmed against staging data before prod.

findAllRates silently drops non-finite rows with no observability

domains/utility/src/tax-rate/tax-rate.queries.ts:30

A seed or admin error inserting a non-numeric rate will silently cause that regime to fall back to 16% on all bills with no log or alert. The CHECK constraint makes this structurally impossible going forward, but a console.warn or structured log for dropped rows would aid debugging during migrations.

Migration file missing newline at EOF

packages/database/drizzle/0060_needy_menace.sql:1

Diff shows '\No newline at end of file'. Cosmetic; does not affect SQL execution or migration runners.

source:'default' fallback — confirm caller surfaces over-taxation to user

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

When a code is absent from both the DB table and TAX_REGIME_RATES, resolveTaxRate returns source:'default' (16%). If the caller does not act on this flag before issuing a calculated bill, a border-rate site could be silently overtaxed. Confirm the calling shell checks source:'default' and rejects or flags the bill.

Stale describe block title still references bare '0.16'

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

The describe string 'resolveTaxRate — table → constant → 0.16' hard-codes the default rate. The JSDoc was updated to say 'TAX_REGIME_RATES.iva (the standard-16% default)'; the test title should follow to avoid divergence if the constant changes.

NaN-filter in query layer is a convention smell — belongs in decision/shell

domains/utility/src/tax-rate/tax-rate.queries.ts:25

FCIS convention: thin query wrappers surface data faithfully; guard logic (treat-as-miss) belongs in the decision layer or shell. The CHECK constraint makes non-finite DB rows impossible going forward, so this filter is purely defensive. Acceptable here given the safety motivation, but worth noting the pattern deviation.

findAllRates NaN-filter has no test coverage

domains/utility/src/tax-rate/tax-rate.queries.ts:27

The isFinite guard added to findAllRates is the primary behavioral change in this commit, but no test — unit or integration — exercises it. A test should inject rows containing non-finite numeric strings ('NaN', '', null-coerced) and assert they are dropped from the returned map while finite rows survive. Without this the guard can be silently removed with no regression signal.

No integration test for findAllRates happy path

domains/utility/src/tax-rate/__tests__/tax-rate.queries.test.ts

findAllRates has no integration test for the happy path. A DB-level test seeding two valid rows and asserting the returned Record matches expected code→rate pairs would document expected behaviour and provide a regression harness for future schema changes.

Comment-only test change adds no executable assertion

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

The added comment claims the behavior holds for 'any code with a row, including a hypothetical exempt (0%) code not in TAX_REGIMES'. A second expect with an arbitrary unknown code + finite DB row would make this a proven invariant instead of prose.

map+filter two-pass could be a single reduce

domains/utility/src/tax-rate/tax-rate.queries.ts:25

rows.map(...).filter(...) iterates twice. A reduce combines them and eliminates the 'as const' cast: rows.reduce<Record<string,number>>((acc,r)=>{ const n=Number(r.rate); if(Number.isFinite(n)) acc[r.code]=n; return acc; },{}). At 2-10 rows this is cosmetic, but the intent reads more directly.

Inline test comment is implementation-level, not intent-level

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

The comment explains internal dispatch logic rather than observable caller behavior. Reframing as: 'a code absent from TAX_REGIMES but present in the DB still resolves via table — forward-compatible with new regime codes' would be more useful to a future reader.