Files
impuestospy/DECISIONS.md
T
MichilisandClaude Opus 5 0d7651b17c phase-2: identity, from the landing hook to the profile screen
Flows A1 to A6 and E3 end to end. A visitor types a RUC on the landing page,
sees their real filing dates, registers, verifies a six digit code, grants
consent, completes a three step setup and lands on the first run screen, with
the profile, consent and audit rows to show for it.

API: public RUC lookup behind a token bucket (10/min/IP), the full /me surface
(profile, dependents, consents, notification prefs, data export, account
deletion), an append-only audit module that exports an insert and nothing else,
and a PII module that is the only thing allowed near those tables.

Deletion and consent revocation both freeze the account and drop every session,
reusing better-auth's ban flag rather than adding a second notion of disabled.
Nothing is destroyed yet: the purge is a job for phase 4. deadlineDigit is
always derived server side, never accepted from the client.

Web: landing with the RUC hook, registration, OTP verification, consent, the
setup wizard, the profile screen with "Tus datos", and legal pages that ship as
marked placeholders per COPY.md section 13. Money, Skeleton, Switch and
EmptyState components added.

The seed is now complete for identity: Maria at 4123456-1, filing digit 6 and
day 19, with a dependant, consents and prefs; Carlos as an IVA-only company.

Two real defects found by building the screens and fixed with tests:
the OTP boxes dropped a digit because the handler fired effects inside a
setState updater that React 19 invokes twice, and the switch knob rendered
outside its track because translate-x-5.5 does not resolve.

232 vitest tests, 26 Playwright tests across mobile and desktop, coverage still
100% on the rules, typecheck and lint clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-03 23:33:10 +00:00

276 lines
16 KiB
Markdown

# DECISIONS
One entry per decision that is not already obvious from the specs. Newest phase last.
Markers used in the code:
- `// SPEC-GAP:` the specs did not settle this and a choice was made here.
- `// TODO-TAX-VERIFY:` a tax rule that RULES.md does not state. Never invented, always flagged.
---
## Blocking gaps in the source material
### RULES.md was missing during phase 0, supplied for phase 1
`packages/rules` shipped phase 0 with only `RULES_VERSION` and the phase 0 seed stopped
short of profiles, because the RUC check digit and the deadline digit are RULES.md
algorithms. Both are implemented in phase 1 and the seed can now be completed.
**Resolved.**
### `boneyard` and `canvas-ui` are not the packages the prompt means
Both names resolve on npm to unrelated projects: `boneyard@0.1.4` is a 2015 Backbone
"architectural toolkit", `canvas-ui@0.2.3` is a Mesosphere Bootstrap theme. Neither does
skeleton loading or canvas effects. Neither is needed before phase 7.
Plan unless corrected: keep the *behaviour* the prompt specifies (skeletons on every
content load and never a spinner; canvas effects in exactly two places, degrading
gracefully) behind a single `<Skeleton name>` component and a single effect component, so
swapping in the real library later is a one file change.
**Please confirm the intended packages before phase 7.**
### Neither named skill is installed
`ponytail` and `ui-ux-pro-max-skill` are not available in this environment. Their stated
intent was applied by hand: nothing speculative, no unused configuration, and FLOWS.md
section 1 as the design constraint.
---
## Phase 0
### Migrations: better-auth generates its own four tables, we own the rest
`src/db/migrator.ts` runs two ordered steps: better-auth's `getMigrations()` creates and
updates `user`, `session`, `account` and `verification`, then the Kysely migrator applies
`src/db/migrations`. Delegating the auth tables keeps them in step with the installed
better-auth version and emits correct DDL for both dialects, so no hand written dialect
SQL was needed for them. Upgrading better-auth in a way that adds a column means adding a
migration that calls the same generator again.
### The whole schema ships in migration `001_core`, not phase by phase
Every table in SPEC.md section 5 is created now. The schema is fully specified and stable;
splitting it across phases would produce a pile of migration files and no benefit before
release. Later phases add modules on top, not tables.
### Timestamps and money are portable by construction
Timestamps are ISO-8601 text and dates are `YYYY-MM-DD` text in both dialects: they sort
chronologically as strings, so no dialect specific date type or comparison is needed
anywhere. Money is `bigint`, because guaranies pass int4 at about Gs. 2.100.000.000, and
`apps/api/src/db/postgres.ts` registers an int8 parser that returns a number and throws
outside the safe integer range.
### `/api` is proxied by a route handler, not a Next rewrite
SPEC-GAP against SPEC.md section 2, which specifies Next rewrites. Next bakes rewrite
destinations into the build manifest, so `API_INTERNAL_URL` would become a build time
value and one image could not serve both compose and k8s. `apps/web/app/api/[...path]/route.ts`
forwards at request time instead. The single origin model is unchanged: the browser only
ever sees the web origin, cookies stay first party, and there is still no CORS anywhere.
### Workspace packages ship TypeScript source with no build step
`packages/*` have no `dist`. The web app lists them in `transpilePackages` and tsup bundles
them into the API. Their relative imports are extensionless, because Turbopack does not
rewrite a `.js` specifier onto a `.ts` source file.
### The es catalog is split from the strings COPY.md does not define
`catalogs/es.ts` is generated from COPY.md and is verbatim; `copy-parity.test.ts` re-derives
it from `docs/COPY.md` on every run and fails on any drift, in either direction. Strings the
product needs that COPY.md does not list (the seven error envelope messages, three sign in
labels, the language switcher) live in `catalogs/es.extra.ts` and follow the tone rules in
COPY.md section 0. `es` is the merge of the two.
### `decl.approve` is both a message and a namespace
COPY.md defines `decl.approve` (the button) alongside `decl.approve.confirmTitle`. A nested
message tree cannot hold both, and next-intl walks a nested tree. `unflatten` moves such a
message to a reserved `_` child and `resolveKey` maps the key for callers, so components
still address messages by their COPY.md key. `apps/web/src/i18n/t.ts` is the wrapper; it is
computed from the catalog, so a future collision is handled without another change.
### Locale negotiation on `/`
next-intl's default detection is left on: a browser asking for English lands on `/en`,
anything else falls back to `es`. The en catalog exists for expats and international users
(COPY.md section 0-EN), which is exactly the population whose browser is in English. An
explicit choice through the switcher always wins and is in the URL.
### SQLite refuses `JOBS_INLINE=false`
Implemented literally as SPEC.md section 15 instructs, which is narrower than the mode
matrix in the same section: that table allows "SQLite, 1 dedicated worker" under Split
small. Two pollers cannot be made safe against a single writer, so the boot check wins and
SQLite stays single process. Worth reconciling in SPEC.md.
### Deferred to the phase that needs them, deliberately
- `RateLimiter`: SPEC.md section 6 specifies a token bucket, but the first endpoint with a
stated limit is `GET /lookup/ruc/:number` in phase 2. better-auth's own rate limiting
covers the auth routes until then.
- DTO schemas in `packages/contracts`: enums, the error envelope, the client and
`ProfileDto` exist because phase 0 uses them. The rest arrive with their endpoints.
- Storage in `/readyz`: the check covers the database and pending migrations. The storage
driver probe is added in phase 3 with the driver.
### Smaller choices
- TypeScript 5.9, not 7.x: `typescript-eslint@8` declares `typescript <6.1.0`.
- `better-sqlite3` is kept out of `onlyBuiltDependencies`: it ships prebuilt binaries, so
letting pnpm run the implicit `node-gyp rebuild` would compile it for nothing and force a
toolchain into the image.
- The language switcher is a native `<select>`: one dependency fewer than a popover, and
the better mobile and keyboard experience for a two item choice.
- `apps/web/proxy.ts`, not `middleware.ts`: Next 16 deprecates the middleware convention.
- The dark mode palette and the `dark` variant are wired now; the toggle itself is phase 7
polish. System preference works today.
- `audit_log` is append only by construction: the module exposes no update or delete. A
database trigger was written and then removed, because it would have been the only piece
of dialect specific SQL outside the two files SPEC.md section 5 allows.
### Legal pages
`/legal/privacidad` and `/legal/terminos` do not exist yet (they belong to phase 2's
marketing routes). Per COPY.md section 13 they will ship with clearly marked placeholder
content and no generated legal text. **TODO: human written before launch.**
---
## Phase 1
### TODO-TAX-VERIFY register
Every item RULES.md flags, where it is implemented, and the test that pins today's
behaviour so verification is a red/green diff rather than archaeology.
| # | What needs verifying | Implemented in | Pinned by |
|---|---|---|---|
| 1 | IRP tranche boundaries (50M, 150M) and the Gs. 80.000.000 registration threshold, against the live DNIT tables for the current fiscal year | `constants.ts` | `constants.test.ts` "pins the IRP tranche boundaries and threshold pending verification" |
| 2 | The RUC check digit algorithm (modulo 11, basis 2) against at least five real published RUCs | `ruc.ts` | `ruc.test.ts` "pins the check digit it computes today" and "agrees with an independent transcription" |
| 3 | Government decreed one off holidays and "dias no laborables trasladables", and movable holidays past 2028 | `holidays.ts` | `calendario.test.ts` "pins that movable holidays are only known for 2026 to 2028" |
| 4 | F120 casilla numbers, currently placeholder keys, against the live Marangatu F120 v4 | `forms/f120.v1.ts` | `forms.test.ts` "pins that they are still placeholders" |
| 5 | F515 casilla numbers, same | `forms/f515.v1.ts` | same test |
| 6 | The progressive-by-tranche reading of the IRP rates | `f515.ts` `taxForNetIncome` | `f515.test.ts` bracket edge suite, including the worked example |
Nothing outside this list was invented. Where RULES.md was silent the gap is marked
`SPEC-GAP` in the code and listed below.
### Both worked examples reproduce exactly
RULES.md section 6 gives F120 a Gs. 277.273 monto a pagar and a Gs. 350.000 flip case;
section 7 gives F515 a Gs. 11.290.000 tax at a 5,65% effective rate. Both are transcribed
verbatim as tests and both pass on the first implementation, which is the main evidence
that the integer money math and the rounding rule are right.
### Coverage is enforced, not observed
`vitest.config.ts` fails the run below 100% statements, branches, functions and lines for
`packages/rules`. Three defensive throws carry an explicit `v8 ignore` and a comment
saying why they are unreachable: the bounded loops in `rollForward` and `nextDeadline`
guard against an edit to `holidays.ts` that would otherwise spin forever, and no input can
reach them with any sane holiday table.
### Classification emits reason codes, not sentences
CONTRACTS.md types `reasons` as `string[]` and RULES.md gives one of them as Spanish text
("Sin categoria sugerida"). A pure, locale free package cannot emit user facing copy in
one language without breaking the en locale, which is a hard constraint. `classify`
therefore returns stable codes and `packages/i18n` carries a `classification.reason.*`
string per code, with RULES.md's wording used verbatim for the Spanish of that one.
### SPEC-GAPs in the tax logic
**`taxpayer.hasIrp` gates the deduction amount.** RULES.md states the IVA eligibility test
in full but never says whether being registered for IRP is required for a deduction. It is
the only field of the documented `ClassificationInput` that would otherwise go unused, so
it gates `irpDeductibleAmount`. The category is still suggested either way, so the data is
already correct if the user registers for IRP later. Pinned in `classification.test.ts`.
**A sale reads as "no keyword hit", so its confidence is 0,4.** RULES.md defines confidence
only for the keyword map, and a sale can never hit it. Taking that literally rather than
inventing a number means sales never reach the auto confirm threshold. Worth a decision
before the auto confirm sweep ships in phase 4. Pinned in `classification.test.ts`.
**"Annualized YTD sales" is straight line.** RULES.md section 8 names the fallback without
defining it: sales so far, scaled by the share of the year elapsed. It is only ever shown
as a "proyeccion" and never filed.
**Ñ folds to N when normalising an emitter name.** The tilde is a diacritic to Unicode, so
"diacritic-insensitive" folds it. No keyword contains Ñ, so matching is unaffected, and
folding is the forgiving choice when a printed name spells it either way.
**CDC type `07` maps to DocKind `otro`.** `nota de remision` is a known CDC document type
but has no DocKind of its own in CONTRACTS.md section 2. The code and its Spanish label
survive on the parsed fields, so nothing is lost.
**`nextDeadline` returns a `Date`, as RULES.md types it.** Every calculation inside the
package works on civil year/month/day values, and the returned `Date` is midnight UTC of
that civil date, so it cannot drift a day with the host timezone. `America/Asuncion` is
consulted in exactly one place, `todayInAsuncion`.
### The IRP category vocabulary lives in packages/rules
`packages/contracts` now imports `IRP_CATEGORIES` from `packages/rules` and builds its Zod
enum from it, rather than declaring the eight strings a second time. The vocabulary sits
next to the tax logic that uses it and the two cannot drift.
### Local ports: web 3005, and the API must agree
`apps/web/.env.example` ships `PORT=3005` and `apps/api/.env.example` names the same port
in `APP_PUBLIC_URL` and `BETTER_AUTH_URL`. They have to agree: better-auth checks the
request Origin, so a mismatch makes sign in fail with a 403 that looks nothing like a port
problem. `http://localhost` and `http://127.0.0.1` are different origins too, which is why
the Playwright default base URL uses localhost.
This deviates from the literal example values in SPEC.md section 15, which use port 3000.
The compose stack still publishes 3000 and is unaffected.
---
## Phase 2
### The seed is now complete for identity
Maria is an individual whose RUC is her CI plus a check digit: base `4123456`, DV computed
by `computeRucDv`, so her filing digit is 6 and her day is the 19th, which is the example
FLOWS.md uses throughout. Carlos is a company registered for IVA only, so the IVA-only view
has real data behind it. Both have consents, notification preferences and the audit rows
CONTRACTS.md section 4 asks for. Documents arrive in phase 3.
### `GET /lookup/ruc/:number` reads a bare number as a CI
A number with a hyphen and a check digit is a RUC and the digit is verified. A bare number
is a CI, which is what individuals type when their RUC is their CI plus a digit; the filing
day is identical either way, which is the only thing the landing card shows. Neither form
touches the database: the endpoint reveals only what the number itself already encodes.
An unparseable number is a 200 with `valid: false`, per CONTRACTS.md section 3, so the
landing corrects the user inline rather than dead ending.
### Freezing an account reuses better-auth's ban flag
Both a deletion and a data processing consent revocation set `banned` with a distinct
`banReason` and drop every session. That reuses the check better-auth's sign in path
already performs, rather than introducing a second, parallel notion of a disabled account
that some code path would eventually forget to check. Nothing is destroyed yet: the purge
is a job, which lands with the jobs module in phase 4 (`softDeleteAccount` carries the TODO).
### `deadlineDigit` is derived, never accepted from the client
`PUT /me/profile` takes `ProfileInput`, which is `ProfileDto` minus `deadlineDigit`. The
digit decides real filing dates, so it is computed from the identity document server side.
The document itself is read only on the profile screen for the same reason: changing it is
a support conversation, not a text field.
### Deactivating a dependant rather than deleting it
A confirmed document may already be classified against a dependant, and that classification
has to keep making sense. `DELETE /me/dependents/:id` sets `active = 0`; the list filters on
it. Scoped by `user_id`, so one user cannot touch another's row (404, tested).
### Two real defects found by building the screens
**The OTP boxes dropped a digit.** The handler spread the pasted code inside a `setState`
updater and fired the focus move and the verify request from in there. React 19 invokes
updaters twice in development to check they are pure, so the effects ran twice and a digit
was lost. The spread is now computed outside the updater. Caught by golden path 1.
**The switch knob rendered outside its track.** `translate-x-5.5` does not resolve, so the
knob sat 44px along a 44px track, entirely outside it. Now `left-0.5` plus `translate-x-5`,
both on the spacing scale. `e2e/profile.spec.ts` asserts the knob stays within the track
bounds in both states, because this is invisible to every other kind of test.
### Deferred, deliberately
- **Motion.** FLOWS.md A2 and A6 call for GSAP (the card flip, the deadline stagger, the
first savings counter) and A1 for a canvas hero effect. The phase plan puts the motion
pass and the canvas touches in phase 7, so the screens here are built with the right
structure and states and no animation.
- **Push and Telegram.** Hidden until configured (FLOWS.md section 9). Setup step 3 offers
only email today; `POST /push/subscribe` needs the service worker and arrives in phase 7.
- **`/comprobantes`.** The first run buttons point at it. Ingestion is phase 3.
### The legal pages ship as marked placeholders
`/legal/privacidad` and `/legal/terminos` render "Documento en preparacion" and say a
document is being drafted, per COPY.md section 13. No legal text was generated.
**TODO: human written before launch.**