Skip to content

Unify Tax-Rate Models — one TaxRate for operational + fiscal + GL

Date: 2026-06-16 Status: 🟢 Phase 1a shipped (additive EXPAND foundation): TaxRate.display_name/dgi_code/rate_percent, nullable Item.tax_rate_id FK + relationship, migration backfilling the 4 ITBMS rows + existing items (joined by DGI code string, no float math), and GraphQL exposure of the new fields. Nothing reads the new FK yet → fully backward-compatible. Decisions: Q1 expand/contract · Q2 seed accounts→rates with GL · Q3 pin + delete-guard · Q4 tenant-editable label + relaxed taxRates gate.

EXPAND also includes dual-write (folded into the same PR): item create/update/bulk resolve Item.tax_rate_id from the ITBMS rate by DGI code, so every item (existing via backfill, new via dual-write, new-tenant via the migration's row seed) has it populated. Per-tenant TaxRate rows are created by the expand_unified_tax_rate migration itself (runs for every tenant before seeding), so no run_itbms_taxes rewrite is needed for rows.

⚠️ Update 2026-06-19 — rate is now a whole percent, not a fraction. A follow-up flipped the canonical convention of TaxRate.rate from the decimal fraction (0.0700) to the whole percent (7.0000) — same number ITBMSTaxRate.rate and the per-line snapshot already use. This supersedes every "rate stays the canonical fraction" statement below (§2 table, §3 trap #1, §5 step A). Consequences: rate_percent is now an identity alias of rate (kept for back-compat) and a new rate_fraction (rate / 100) serves fraction consumers; trap #1's "100× under-tax" risk is gone because there is no longer a fraction/percent scale gap. See Store TaxRate.rate as a whole percent (migration tax_rate_store_percent + scripts/backfill_tax_rate_percent.py).

Remaining (next increments): - Phase 2 (MIGRATE reads) — populate GL accounts on the seeded ITBMS rate rows via the async service (Q2: get_or_create_account), which is natural here where the DI context exists. The §3 correctness traps land here (they only matter once code reads the merged rate for percent): rate_percent bridge at purchase_order_factory/tabular/value-match, fail-loud value-match, ITBMS-sale-code + effective filter (no MultipleResultsFound), TaxRateDeletionStrategy guard, relaxed/ungated item rate query. Frontend moves item pickers + invoicing to the unified field; old surface @deprecated. - Phase 3 (CONTRACT) — Item.tax_rate_id NOT NULL; drop itbms_tax_rate_id + shared.itbms_tax_rates + ITBMSTaxRate model/type/query/loader/repo; remove load_models registration. Also drop tax_rates.dgi_code: it is now derived from TaxRateCode.dgi_code (law-fixed, see enums.py) — TaxRateResponse and GrupoITBMS already read the enum, so the column survives only as the EXPAND join key (item_repository._resolve_unified_tax_rate_id). When that resolver goes, rewrite it to translate via the enum (from_dgi_code) instead of joining on dgi_code, then drop the column and its input/service field. (Supersedes §2/§3's "non-derivable" note.) Related: Tax-Rate GL-Account Wiring; Accounting Placeholder Audit

1. Problem / motivation

Tax is modelled twice, glued by matching the percent value:

  • items.ITBMSTaxRate (operational — shared schema, code 00/07/10/15 as String, rate as a percent 7.00, unique per code, Item.itbms_tax_rate_id NOT NULL) — drives tax_amount = sub_total × rate / 100.
  • tax_filings.TaxRate (fiscal/posting — accounting schema, code TaxRateCode IntEnum, rate as a fraction 0.0700, validity windows, the four GL-account FKs from the last feature, InvoiceDetail.tax_rate_id nullable) — drives the DGI worksheet + ledger posting.

A non-accountant should pick one named rate ("ITBMS 7%") and the system should derive the operational percent, the DGI fiscal code, and the GL accounts from that single selection — no value-match glue, no duplicate catalogs.

2. Target model (canonical = tax_filings.TaxRate, delete ITBMSTaxRate)

Extend tax_filings.TaxRate (it already holds everything ITBMSTaxRate has plus validity + the 4 GL FKs, and the invoice/supplier-invoice detail FKs already point at tax_rates). Add two columns to serve the items/PAC world:

Column Type Purpose
display_name String human label ("ITBMS 7%") for non-accountant pickers
dgi_code String(2) the 2-char DGI code (00/01/02/03) ITBMSTaxRate.code carried; preserved as the only non-derivable ITBMS field
rate_percent hybrid/property (rate * 100) read-only bridge so percent consumers never see the fraction

rate stays the canonical fraction Numeric(7,4). Item.itbms_tax_rate_id → Item.tax_rate_id (retargeted to tax_rates.id, in-schema, NOT NULL). The per-line InvoiceDetail.tax_rate percent snapshot stays unchanged (historical, not derived).

3. ⚠️ Correctness traps (mandatory — flagged by both adversarial reviewers)

These are not optional; the merge is unsafe without all of them:

  1. 100× under-tax (live risk). purchase_order_factory.py copies itbms_tax_rate.rate (today 7.00) into a percent field feeding tax_amount = sub_total × rate / 100. After the merge rate is the fraction 0.07 → 0.07%. Every percent consumer must read rate_percent (rate × 100), not rate. Sites: purchase_order_factory, map_itbms_tax_rates, item/order tabular, supplier-quote→PO.
  2. Silent zero-rating. map_itbms_tax_rates falls back to tax_rates[0] (the 0%/EXEMPT row) on any non-match. With the scale change, 7.00 != 0.0700 → every import silently becomes exempt. Fix: compare against rate × 100 and make the fallback fail-loud (raise / route to ITBMS_UNCLASSIFIED, never the 0% row). Call sites: item_tabular:72, item_lifecycle:92, order_tabular:100.
  3. MultipleResultsFound. get_itbms_tax_rate_by_rate_or_default does .one_or_none() with no code/effective filter. Post-merge (multiple validity rows per code) it crashes. Fix: filter to ITBMS sale codes (EXEMPT/7/10/15) and the effective row (valid_to IS NULL).
  4. Float-join orphan (migration). Backfill must remap items by integer percent (0/7/10/15 → TaxRateCode) and build the id-map by code, never itbms.rate/100 = tax_rates.rate (a DOUBLE vs Numeric(7,4) equality that silently misses → NOT NULL tighten aborts).
  5. NULL GL accounts drop the withholding-asset leg. Item-seeded rows with NULL GL accounts make _resolve_withholding_asset_account return None, skipping the retention DR leg (no fallback). See §10 Q2.
  6. deleteTaxRate is unguarded. No DeletionCheckStrategy exists for TaxRate; once Item.tax_rate_id is NOT NULL, delete raises a raw Postgres FK error. Add a TaxRateDeletionStrategy counting items + invoice_details + supplier_invoice_details references.

4. Consumer changes (per subsystem — incl. consumers the reviewers caught)

  • items: delete itbms_tax_rate.py; item.py FK itbms_tax_rate_id → tax_rate_id → tax_rates; retarget relationship + loader. item_input.py (ItemInput.itbms_tax_rate_id, a required field — write path) and item_event_dto.py / item_qbo_event_handler (QBO to_event) must change in lockstep. Rewrite items/repositories/tax_rate_repository.py per §3.2/3.3.
  • invoices/orders: detail FKs already point at tax_rates → zero migration, zero change for tax_rate_id/tax_rate_obj. order_tabular resolves an id then discards it (dead wiring) — wire it through or drop the param.
  • worksheet (build_itbms_worksheet): no change (groups by TaxRate.code, merge-neutral).
  • ledger (invoice_ledger_service / supplier_invoice_ledger_service): no resolver change — but depends on seeded rows having GL accounts (§10 Q2).
  • recurring invoices (recurring_invoice_service._line_from_detail) and PAC credit notes (pac_credit_note_factory → GrupoITBMS.from_detail): tax-rate/percent consumers — verify and cover.
  • PAC: GrupoITBMS.from_detail reads the per-line detail.tax_rate percent, not the model → table-merge-neutral. (dgi_code is kept for future row-based use, not to fix current PAC drift.)
  • seeding: replace multi_tenant_migration.run_itbms_taxes + shared_data_importer.insert_itbms_tax_rates with TaxRate seeding (IntEnum codes, fraction rates, valid_from floor, valid_to NULL, display_name, dgi_code, GL accounts).
  • graphql: deprecate ITBMSTaxRate type + itbmsTaxRates; keep a separate ungated lite query (ITBMS sale codes, effective) for non-accountant pickers (don't relax the TAX_FILINGS-gated taxRates). load_models.py registration of ITBMSTaxRate must be removed when the model is deleted.
  • tests: rewrite inventory + tax fixtures to build TaxRate; keep tests/scripts/test_backfill_tax_rate_id.py green.

5. Migration sequence (expand/contract, float-safe)

Pre-check: assert the 4 GL columns exist (the rename_tax_rate_account_columns revision is applied) or the backfill INSERT fails.

  • A — backfill 4 ITBMS rows into tax_rates, idempotent, remap by integer percent → TaxRateCode; rate = percent/100 as Numeric(7,4), valid_from='1900-01-01', valid_to=NULL, display_name, dgi_code. Insert an open-ended row for any code that only has a closed row.
  • A2 — id-map itbms_tax_rates.id → tax_rates.id joined by code (deterministic: valid_to IS NULL, then MIN(valid_from)).
  • B — expand: items ADD COLUMN tax_rate_id UUID NULL REFERENCES tax_rates(id); keep itbms_tax_rate_id.
  • C — data move: UPDATE items SET tax_rate_id = map[itbms_tax_rate_id].
  • D — verify + tighten: assert no NULLs, then SET NOT NULL.
  • E — contract (later revision, after deploy reads tax_rate_id): drop order FK → column → table shared.itbms_tax_rates, remove load_models registration. Irreversible point — gate behind a no-callers check.
  • F — replace seeders (after chart-of-accounts exists, see §10 Q2).

Downgrade is lossy (original ITBMS ids unreconstructable) — document it.

6. Non-accountant UX

Item setup + invoicing show one dropdown of display_name ("ITBMS Exento / 7% / 10% / 15%") from the ungated effective-sale-rate query. Selecting it sets tax_rate_id; the system derives the percent (rate_percent), the DGI code (dgi_code), the fiscal code (TaxRateCode), and the GL accounts. Retention/ISR/UNCLASSIFIED codes are filtered out of the item picker. Accountants (gated) manage validity windows, GL wiring, and retention codes — same row, no value-match glue.

7. GraphQL surface (breaking)

ITBMSTaxRate type (code: String!→ gone), itbmsTaxRates query, Item.itbmsTaxRate/itbmsTaxRateId (required input), Item.itbmsTaxRate: ITBMSTaxRate! (non-null) all change. During EXPAND they keep working via a TaxRate-backed compatibility projection (String code from dgi_code, percent from rate_percent); CONTRACT removes them.

8. RBAC / tasks

No new Path/Resource. No background tasks. Item-facing rate read stays ungated via the lite query.

9. Frontend contract

Per phase. EXPAND: nothing breaks (old + new both work). MIGRATE: move item pickers + invoicing to the unified rate field (enum code + ratePercent/fraction + displayName); old surface @deprecated. CONTRACT: old type/query/input removed — frontend must have migrated.

10. Open questions — blocking (resolve before coding)

  • Q1 — Phasing. Expand/contract over a deprecation window (safe, reversible until the table drop; 3 PRs) (recommended) vs big-bang one breaking PR (faster, frontend must ship in lockstep — like the 2.0.0 rename).
  • Q2 — GL accounts on seeded item rates. There is no chart-of-accounts seed in tenant provisioning today, and a migration can't call get_or_create_account. Options: (a) seed rates with NULL GL accounts and accept the withholding-asset leg is dropped until an accountant configures them (interim, documented); (b) add chart-of-accounts seeding first and seed rates with GL accounts via a service. (a) is faster; (b) is correct-from-day-one.
  • Q3 — Retire/delete policy. When an accountant retires a rate, do existing items pin to the retired row (historical integrity) (recommended) or auto-rebind to the new effective row? And deleteTaxRate: hard FK guard vs soft-delete via valid_to.
  • Q4 — display_name / item-query gating. System-fixed Spanish labels + separate ungated lite query (recommended) vs tenant-editable labels / relax the gated taxRates.