Files
impuestospy/DECISIONS.md
T
MichilisandClaude Opus 5 37370dd079
CI / types, lint and rules coverage (push) Canceled after 0s
CI / tests (postgres) (push) Canceled after 0s
CI / tests (sqlite) (push) Canceled after 0s
CI / playwright (push) Canceled after 0s
phase-8: run it on Postgres, and find out what that was hiding
Six phases claimed the product runs on SQLite and on Postgres. Nothing
had ever run it on Postgres. TEST_DATABASE_URL now points the whole
suite at a real server and CI runs both arms.

The first run found a bug that would have shipped. better-auth's banned
flag is integer 0/1 on SQLite and a real boolean on Postgres, and the
code read it as `banned === 1`, so on Postgres an account someone asked
us to freeze went on receiving email. Both of those flags are now typed
for either dialect and read through isFlagSet.

It also found the migration advisory lock being taken on a pool. An
advisory lock belongs to the session that took it, so a lock on one
pooled connection and an unlock on another leaves it held. Two replicas
migrating at once is the ordinary case in k8s and is precisely what it
was there to protect.

New for the scaled mode: k8s manifests with migrations as an
initContainer, one poller in its own worker Deployment rather than one
per API replica, and an Ingress that exposes the web app only. The
storage drivers finally have tests, S3 included, since scaled mode
requires it and it had never been exercised.

Two acceptance tests, both checked against a deliberately broken build
first: two workers claiming a hundred jobs report 188 claims with SKIP
LOCKED removed, and the in-flight request is cut off with the drain wait
removed.

340 tests on SQLite, 341 on Postgres, 70 Playwright, rules coverage 100%.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-06 22:03:13 +00:00

830 lines
48 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
_Still open. The `<Skeleton name>` component is in place and used on every data screen, so
swapping in the real library is one file._
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.**
---
## Phase 3
### The pipeline has three doors, and the trustworthy one wins
`ingestScan` tries the QR first, then queued OCR, then the manual form. A QR that will not
parse is recorded as an ingest error and falls through rather than failing the scan, so a
smudged code never costs the user their photograph.
**A QR carries the emitter's RUC but not its name.** Verifying a CDC against DNIT is out of
scope for v1 (SPEC.md section 10 makes `verify_cdc` a noop), so a QR-only document shows
`RUC 80011223` as its emitter and the keyword classifier cannot match on it: it lands in
"Sin categoria" and the user picks one in the bandeja. Worth revisiting when CDC
verification exists, since it is the one thing standing between a QR scan and a fully
automatic classification.
### `dedupe_hash` is defined here, not in the specs
SPEC-GAP. SPEC.md section 8 requires the column and never says what goes in it. With a CDC
it is the CDC, which identifies a comprobante nationally. Without one it is
`emitter | date | total | kind`, the tuple a human would use. Two photos of the same
factura collapse; two genuinely different facturas from the same shop on the same day for
the same amount would collapse too, which is why a merge is shown to the user ("Ya tenias
esta factura") rather than applied silently.
A merge prefers QR sourced data over OCR or typing, and never overwrites a classification
the user has already decided.
### Job claiming is the only dialect divergence, as specified
`jobs/claim.ts` holds both: Postgres takes `FOR UPDATE SKIP LOCKED` inside a transaction so
N workers take different rows; SQLite relies on its single writer and a conditional
`UPDATE ... WHERE status = 'pending'`, where the changed-row count decides the winner. A
test claims five queued jobs five times and asserts no id comes back twice.
Retries follow SPEC.md section 10 exactly (1m, 5m, 25m, 2h, 12h, then dead) and a job
abandoned by a killed process returns to the queue once its lock is older than
`JOBS_STALE_MINUTES`. That is the phase 3 acceptance case and it has three tests: recovery,
the negative case of a merely slow job, and a restarted poller finishing the recovered work.
### OCR is structured output, not prose parsing
The Anthropic call uses `messages.parse` with a Zod schema, so the model cannot return
anything but the shape in RULES.md section 9 and there is no JSON to repair. Every field is
nullable because "unreadable" is a real answer. `temperature: 0` from RULES.md is not sent:
the current models reject sampling parameters, and constraining the output format achieves
what the setting was there for.
A dead `ocr_extract` job records an ingest error; a retryable failure does not, so a
transient API blip never shows up in the user's error list.
### The fixtures are decoder fixtures, not visual replicas
SPEC-GAP against CONTRACTS.md section 4, which suggests HTML or canvas. `scripts/render-fixtures.ts`
draws them with raw pixels and a real, decodable QR. Faithful KUDE artwork would mean
carrying a browser or an SVG rasteriser to produce three images that nothing but a decoder
ever reads; the emitter, date and totals live in `fixtures.ts`, which is where the tests and
the seed read them from. A test decodes all three and asserts the third has no QR.
### The ZXing wasm is served from our own origin
`zxing-wasm` fetches it from a CDN by default, which the CSP forbids and which would break
the offline queue. A script copies it into `public/zxing` before dev and build. Headless
Chromium has no `BarcodeDetector`, so the e2e runs exercise the ZXing path end to end,
which is the fallback that matters most.
### Two defects the screens surfaced
**Seeded documents were dated in the future.** `seedDate` placed current-month documents on
a fixed day of the month, so a day-8 document seeded on the 4th landed four days ahead. A
comprobante dated in a period that has not happened corrupts every projection computed from
it. `seedDate` now clamps to today and has its own tests.
**Category buttons announced their keyboard shortcut.** The 1-to-8 hints were part of each
button's accessible name, so a screen reader read "Alimentacion 1". They are `aria-hidden`
now with an explicit label on the button.
### Test isolation against a shared demo account
The flows that confirm and reject run against Maria's bandeja, which is shared mutable
state. They are serial and desktop only, and `bandeja.spec.ts` covers the same screen on a
phone viewport without mutating anything. A test that creates a document varies the total
as well as the name, because the name is deliberately not part of the dedupe key.
### Deferred, deliberately
- **The scan FAB.** FLOWS.md B1 puts a persistent `[+ Escanear]` on every `(app)` screen.
There is no tab bar to anchor it above until the dashboard lands, so scanning is reached
from `/inicio`, `/comprobantes` and the empty bandeja. The FAB arrives with the app shell
in phase 4.
- **The offline queue.** FLOWS.md B2. It needs the service worker, which is phase 7.
- **Auto-confirm.** The setting and its copy are live; the sweep that acts on it is a job
for phase 4.
- **Seeded declarations.** CONTRACTS.md section 4 asks for a ready F120 and an approved one.
The declarations module is phase 5 and seeds them then; phase 3 seeds the documents they
will be computed from.
---
## Phase 4
### The dashboard does no arithmetic
`modules/dashboard` chooses inputs and calls `packages/rules`. Every figure on the screen
comes out of `computeF120`, `projectIrp` or `savingsThisMonth`, and the acceptance test
asserts the API's numbers equal what those functions return over the same rows, before and
after the bandeja is confirmed (CONTRACTS.md 5.2). Confirmed documents only: nothing the
user has not agreed to is ever counted.
The IVA carry-in comes from the last approved F120's summary and is zero when there is
none, per RULES.md section 8.
### A deadline before the obligation existed is not overdue
`upcomingDeadlines` skips any period earlier than the `since` date on the obligation.
Without it a brand new account opens on a red "overdue" card for a period that predates it,
which is both wrong and the worst possible first impression.
Maria still shows overdue, correctly: she has been registered since 2024 and has filed
nothing, because declarations arrive in phase 5.
### Sweeps are idempotent by dedupe key, not by bookkeeping
Each sweep is enqueued with a key naming the day (or the hour, for the digest) it belongs
to, so a restarted poller, a second replica and a crash mid-sweep all converge on one run.
The notifications they queue are keyed the same way, down to the milestone:
`deadline:<user>:<obligation>:<period>:T-10` can only be queued once, whatever else
happens. Sweeps queue notifications rather than sending them, so a channel being down
retries on the job schedule instead of losing the message.
The time-travel test derives T-10 from `dueDateFor` rather than hardcoding a date, so it
follows the calendario rather than restating it.
### Auto-confirm only touches what the rules were sure about
The sweep confirms documents older than the user's window whose classification is still
`decided_by = 'auto'` and at least 0.85 confident. A low confidence read is exactly the
thing a person still needs to look at, and a decision the user already made is never
revisited. Zero days means off.
### An unconfigured channel is absent, not broken
`createChannels` returns `null` per channel when its configuration is missing. The fan-out
skips those, the profile screen hides them, and a user with everything off receives nothing
at all, which is the setting working rather than a failure. One dead push subscription does
not stop the others; a frozen account is not a recipient.
### `hasDocuments` extends DashboardDto
SPEC-GAP. FLOWS.md A6 wants the guided empty state until the first comprobante lands, and a
position of all zeros is indistinguishable from a real one without this. It is an addition
to CONTRACTS.md section 2, not a rename.
### `insight_dismissals` is a new table
SPEC-GAP. FLOWS.md C3 requires dismissals to persist and SPEC.md section 5 lists nowhere to
put them. One row per dismissal keeps the dashboard read free of per-user JSON to merge.
### The scan button is now global
FLOWS.md B1 asks for a persistent scan affordance on every `(app)` screen. It arrives with
the tab bar, and the per-screen scan link that stood in for it on `/comprobantes` is gone,
since two of them on one screen is one too many.
### A copy bug the screenshot caught
The overdue card filled a `{period}` slot with the formatted due date, so it read "sin
presentar de 19 de marzo". The string now says what the data actually is: a form that was
due on a date and never filed.
### Test isolation, again
The read-only bandeja tests run against Carlos rather than Maria, because the mutating
ingestion flows pull cards out of Maria's stack and two workers cannot share one queue.
The suite also erodes the seed it runs against: after a run, documents that started in the
bandeja are confirmed and insights are dismissed, so the next run is not the same run.
`pnpm db:reset` rebuilds the local database from the migrations and the seed, and the
README says to use it before an e2e run. It refuses to touch anything but a local SQLite
file, because it is destructive by definition, and it warns that the API has to be stopped
first: deleting a file the running process holds open leaves it writing to an inode nothing
else can see, which looks like half the suite breaking at once.
Where a test can avoid depending on a fresh seed, it does. Golden path 4 generates its own
declaration for a month that has documents and no declaration yet, because approving is a
one way door. The ones that genuinely cannot, like scanning into an empty bandeja, say so
in the assertion message rather than timing out on a locator.
The dashboard's insight test self-skips once every insight for that account has been
dismissed, since dismissals persist by design; the persistence guarantee itself is asserted
against the API from a fresh database.
### Deferred, deliberately
- **Push subscription registration.** `POST /push/subscribe` needs the service worker, which
is phase 7. The channel itself is built and tested.
- **Telegram linking.** The channel sends to a stored `telegram_chat_id`; the flow that
obtains one is not in v1 scope.
- **Declarations.** `declaration_ready` is in the next-action priority and the sweep reads
from it, but nothing sets a declaration to `ready` until phase 5.
---
## Phase 5
### A generated declaration is `ready`, and approval is a promise about what was seen
Generating assembles the numbers and marks them `ready`: settled, waiting for a person to
agree. Approving recomputes them first, and if the documents moved in between it returns a
409, stores the new figures as a `draft`, and asks the user to look again. Approving
figures nobody saw is the one outcome that must be impossible, and it is the reason the
recompute happens on the way in rather than at render time.
An approved declaration is never regenerated or invalidated. Once a person has put their
name to a set of numbers, rewriting them behind their back is worse than any staleness.
### A document that moves invalidates the declaration that counted it
CONTRACTS.md 5.4. Confirming, rejecting, editing or reclassifying a document flips any
`ready` declaration covering its period back to `draft`, which is what the dashboard reads
to stop offering it for review. The month decides for a 120 and the year for a 515.
### The PDF is cached against the declaration, not regenerated per download
Two downloads of the same declaration are byte for byte the same document, which matters
when someone is transcribing from a printout. Every write that changes the numbers clears
`pdf_file_id`, which is what makes the cache safe. Approval enqueues a render so the
download on the success screen is instant; the route renders on demand too, so a failed
job costs a wait rather than a missing document.
### The form preview and the PDF stay Spanish in both locales
FLOWS.md section 1 and the COPY.md policy. The casilla labels mirror the DNIT form, and a
form whose labels have been translated is harder to transcribe from, not easier. The
caption around the preview says so in the reader's language. Two strings are hoisted out
of JSX with the reason written down, because the inline copy lint rule is right to flag
them and the answer is a comment, not an exception.
### The platform never files anything
Flow D3 is a checklist, not a submission: numbered steps, every casilla one tap from the
clipboard, and "Ya la presente" as the user telling us rather than us finding out. Marking
filed is only possible after approval, because the checklist is what follows approval.
### The seeded declarations are computed, not written down
CONTRACTS.md section 4 asks for a ready F120 for the previous month and an approved one two
months back. Both are assembled through the same code path the product uses, so their
figures agree with the documents behind them. A fixture whose numbers disagreed with its
own documents would be worse than no fixture.
### The declarations tab joined the shell
Five tabs now. Vencimientos stays reachable from the dashboard's deadline card and its
overdue action, so nothing lost a route.
### A test that had to grow
The next-action priority test now walks two rungs of the ladder rather than one: with the
seeded `ready` declaration in place, `declaration_ready` correctly outranks `bandeja`, and
the test approves it to check the fall through. The 5.4 test clears the outstanding
periods first, because `overdue` outranks everything and Maria has never filed a 515.
### Three defects the screenshots caught
**The scan button swallowed the tap meant for "Aprobar".** The floating button and the
sticky approve footer overlap on a 390px screen, and the desktop e2e never saw it because
the viewport is wider. The button now appears on the screens you arrive at, not on a detail
screen that has its own primary action, and a test asserts it.
**The summary breakdown was mislabelled.** It called the debito "A pagar" and the saldo
anterior "Casilla", because it reused keys meant for other screens. Three labels of its own
now.
**Five tab labels collided at 390px.** Smaller, truncating labels rather than dropping a
tab, since every one of them is a place the user needs to reach.
### Deferred, deliberately
- **Motion.** The confetti on approval and the canvas moment on the filed screen are
FLOWS.md D3; the motion pass and the canvas touches are phase 7. The screens are built
with the right structure and states.
- **Payment reminders after filing.** FLOWS.md D3 mentions scheduling one on "Ya lo
presente". The deadline sweep already covers T-2 and T-0 for the period; a separate
payment date is not in RULES.md and was not invented.
## Phase 6
### The console is a separate route group with its own shell
FLOWS.md Flow H asks for plain and dense, desktop first. `(admin)` gets a wide layout, a
text nav and no tab bar, no floating scan button and no playfulness. The audit reminder
sits above every screen in the group rather than only the user search: staff reading the
error queue are reading user data too.
### The role is checked in the layout and again in every handler
The layout calls `GET /me/session` and renders "solo para el equipo" for anyone else, which
is a convenience. Every admin handler calls `requireRole` itself, which is the rule. A
plain user who guesses the URL gets a plain page from the layout and a 403 from the API.
### SPEC-GAP: GET /me/session
CONTRACTS.md section 3 has no way to ask who you are signed in as, and a console that must
know a viewer's role before it renders needs one. It returns id, email and role, and
nothing else.
### A search has no subject, and the log now says so
`writeAudit` used to default a missing `subjectUserId` to the actor, which made a user
search read as staff looking themselves up. `undefined` still means "acting on yourself";
`null` now means "no subject", which is what a search, a retry, an error resolution and an
export are. The distinction is a fact about what happened, so the log keeps it.
### Reading the audit log is not audited, exporting it is
A row for every scroll of the audit screen would bury the accesses that matter under the
act of looking for them. Taking a copy out of the building is a different act, so
`GET /admin/audit/export.csv` writes `admin.audit_export` with the filters that produced it.
### The error queue merges two sources into one table
Ingest errors and dead jobs are one queue with a `source` on each row, filtered and paged
together by a `createdAt|id` cursor applied to both sides before the merge. Only a job row
can be retried and only an ingest row can be resolved, and a dead job never appears under
the resolved filter: retrying it takes it out of the queue instead. A manual retry resets
`attempts` to zero, so the retry gets the whole backoff schedule rather than dying on its
first stumble.
### SPEC-GAP: ingest_errors.resolution_note
Flow H resolves an error "with note" and SPEC.md section 5 gives the row nowhere to put
one. Migration 003 adds a column, because the note is what the next person reads, not
another key inside the payload the failure wrote.
### Role changes are superadmin only and never on yourself
The one thing worse than an account with too much power is the last superadmin demoting
themselves out of the console. Setting the role a user already has is a 409 rather than a
silent success. No session shuffling is needed: `getSession` reads the role off the user
row on every request, so a demotion takes effect on the demoted user's next call.
### The audit table shows the action code, not a translated phrase
It is the same token the filter takes and the same one in the CSV. An operator matching a
log wants to see what they can search for, and fourteen action names in two languages would
be copy that has to stay in step with an enum.
### Timestamps in the console are ISO, not prose
`2026-09-04 20:42` rather than "4 de septiembre". A console sorts, compares and copies
timestamps; the year is part of the fact and the format has to be unambiguous. Everywhere
the user sees a date, it is still formatted for the reader.
### CONTRACTS.md 5.5 is pinned against the API, not the browser
"Exactly one `admin.user_lookup` row per overview" is asserted in `admin.test.ts`, where it
is exact. It cannot be asserted through Playwright: React strict mode mounts a client
component twice in development, so the browser asks for the overview twice and writes two
rows. The e2e asserts the flow writes a lookup that then shows up on the audit screen.
### Seed: one dead job
CONTRACTS.md section 4 asks for two open ingest errors, which `seedDocuments` has written
since phase 3. The dead job is an addition: the queue merges two sources and the retry
action has nothing to act on without one. It also cost a test its assumption that the jobs
table starts empty, which was an assumption worth removing anyway.
### Two defects the screenshots caught
**The user summary read as the wrong pairs.** Label and value side by side across a wide
card put each value next to the following pair's label, so "4123456-1" and "Rol" read as
one field. Label above value fixed it.
**A search recorded itself against the searcher.** See above: visible only once real rows
were on screen next to each other.
### Deferred, deliberately
- **Impersonation and account freezing.** Neither is in SPEC.md or FLOWS.md. better-auth's
admin plugin ships both; leaving them off is the smaller surface.
- **Filtering the error queue by account.** The overview lists a user's own open errors and
links to the queue. CONTRACTS.md gives `/admin/errors` a stage and status filter and no
user filter, and one was not invented.
## Phase 7
### boneyard and canvas-ui do not exist as SPEC.md describes them
SPEC.md section 3 names both in the stack table. On npm, `boneyard` is an abandoned 2015
"architectural toolkit" and `canvas-ui` is Mesosphere's abandoned design system. Neither
does what FLOWS.md asks of the name, so both were built here instead, to the behaviour
FLOWS.md specifies rather than to the package name:
- **Skeletons** are `components/ui/skeleton.tsx`, shipped since phase 0, with exactly the
named shapes FLOWS.md section 1 lists. No content load anywhere shows a spinner.
- **The two canvas spots** are `components/canvas/`: a slow liquid wash behind the landing
hero (A1) and a brief bloom on the screen that says a declaration is filed (D3). Sixty
lines each, no dependency, off under reduced motion with the same picture held still as
the fallback.
Installing either package would have added dead weight and done none of the work. This is
recorded here rather than buried in a comment because it contradicts the stack table.
### Motion is one module and one preference
`lib/motion.ts` holds two durations, one ease, and the four helpers everything uses:
`useGsap` (a context that reverts on unmount), `useReveal`, `useCountUp` and the reduced
motion read. Every animation checks the preference, so a reader who has asked for less
motion gets none rather than a fast one.
`usePrefersReducedMotion` lives in `lib/browser.ts`, not next to the GSAP helpers. The
landing page needs the answer and must not pull an animation library in to ask the
question; the motion module re-exports it so callers have one place to look.
### Reading a browser-only value is not state
`lib/browser.ts` reads through `useSyncExternalStore`. "Is this iOS", "which language does
the browser want", "is the app installed", "is reduced motion on": all are true on the
first client render and none needs a second pass. Writing them from an effect renders once
with a placeholder and again with the truth, which for reduced motion means an animation
that starts and is then told not to.
### The stack shows a shoulder, not a card
FLOWS.md B4 wants the next card to scale up as the current one flies out. A whole card
behind sits entirely hidden behind a tall front card and entirely exposed behind a short
one, since the front card's height follows its content. A shoulder above the top edge reads
as a stack at every height.
### The service worker caches the shell and nothing else
No API response is cached. A tax figure that is quietly out of date is worse than one that
is honestly missing, so a data request that fails, fails, and the screen says so. What is
cached is one offline page, which explains itself and offers a retry.
`/offline` sits outside `[locale]`, because the worker caches exactly one URL and a page
that only existed per locale would mean caching one and showing it to everybody. It picks
its language in the browser from the two catalogs we already ship.
### Offline captures wait in IndexedDB, and the queue is the source of truth
A capture taken with no network is stored whole and sent later. Both the scanner and the
shell read the queue rather than remembering that something was queued, so the notice
disappears when the capture actually lands rather than when the screen guesses it has.
Sending retries on an interval as well as on the `online` event: a phone walking back into
coverage does not reliably fire that event, and a stranded photograph is the one failure
this feature exists to prevent.
### The theme is a cookie, not an inline script
The first attempt read localStorage in an inline script before the first paint. Under a
nonce policy that script is one React and the browser disagree about: the browser hides a
nonce attribute from the DOM, so the server rendered `nonce=""` and the client saw
`undefined`, and every page load logged a hydration mismatch.
The cookie is better than a workaround for that. The server reads it and stamps
`data-theme` on the html element, so the first frame is already the right colour and there
is no inline script at all. `system` writes no attribute, so the reader's own setting keeps
being followed as it changes.
### The offline page lives under `[locale]`
It was briefly at `/offline`, outside the locale segment, so that the service worker could
cache exactly one URL. That was wrong twice over: `[locale]/layout.tsx` is the root layout,
so a sibling route got Next's generated fallback instead, with no stylesheet, no font, no
theme and a second `<html>` element; and the page guessed its language from the browser
rather than knowing it.
It is now a normal screen. The worker keeps one copy per language the reader actually
visits, learned from their own successful navigations, so the fallback matches the page
they were trying to reach. No list of locales lives in the worker: a third language stays a
catalog file and nothing else, which is exactly the sort of place a hardcoded list would be
forgotten.
### CSP by nonce, styles still inline
The proxy mints a nonce per request and Next stamps it on the scripts it renders, with
`'strict-dynamic'` for the chunks they load. `style-src` keeps `'unsafe-inline'`: React
writes inline `style` attributes for things like a dragged card and there is no way to
nonce those.
This forced one change: the offline page is rendered per request rather than prerendered. A
prerendered page carries a build-time nonce that no live policy matches, so its scripts
were blocked and the page rendered without ever hydrating. The service worker caches
headers along with the body, so the copy it serves offline stays self consistent.
### A crash at boot, from an optional channel
`webpush.setVapidDetails` throws when the VAPID subject is not an `https:` or a `mailto:`
URL, and the code handed it `APP_PUBLIC_URL`, which is `http://localhost:3005` in
development. Any machine with push keys configured therefore died at boot. There is now a
`PUSH_VAPID_SUBJECT` variable, a fallback to `APP_PUBLIC_URL` when it is https and then to
`mailto:SMTP_FROM`, and with none of the three push switches itself off and says so. A
misconfigured optional channel must never take the API down.
### Subscriptions the push service has given up on are deleted
A 404 or a 410 from the push service means the browser threw the subscription away, and the
row is removed. Anything else is transient and the row stays: a network blip is not a
reason to stop notifying someone forever.
### Lighthouse: measured, not met
The target is a mobile score of 90 or better. Measured here on the production build:
accessibility 100, best practices 96, SEO 100, performance 73. The performance number is
not trustworthy on this machine. Lighthouse reports a `benchmarkIndex` of about 560 (a
healthy development machine is 1000 or more) on a shared, loaded box, and then applies a
4x CPU multiplier on top of that. The same page with the multiplier removed scores 89.
What the phase actually fixed is real and measurable: total blocking time went from
17,590ms to 1,700ms once the hero canvas stopped drawing at full resolution on every frame
and the landing page stopped importing GSAP. First contentful paint 0.9s, largest
contentful paint 1.9s, cumulative layout shift 0. The remaining 4 points of best practices
are Chrome flagging `style-src 'unsafe-inline'`, which is the deliberate choice above.
**A 90+ mobile score cannot be verified in this environment.** It belongs on the list with
Docker.
### The four states, screen by screen
Every data screen ships a skeleton, an error with a retry and content. The two detail
screens (a comprobante, a declaration) have no empty state on purpose: a detail screen
either has its subject or is a 404, and there is no third case. The crossfade from skeleton
to content is applied where the skeleton is a separate early return; the three list screens
render their skeleton inside the same tree as their heading, where fading the whole tree
would also fade a heading that never changed.
## Phase 8
### The dual-dialect claim is now run, not asserted
Six phases said the product runs on SQLite and on Postgres. Nothing had ever run it on
Postgres. `TEST_DATABASE_URL` now points the whole suite at a real server, and CI runs both
arms of the matrix.
The first run found three things, one of them a bug that would have shipped:
**A frozen account went on being notified on Postgres.** `banned` is integer 0/1 on SQLite
and a real boolean on Postgres, and the code read it as `recipient.banned === 1`. On
Postgres the driver hands back `true`, the comparison is false, and an account someone
asked us to freeze keeps getting email. The two better-auth flags are now typed for both
dialects and read through `isFlagSet`, and a test asserts the behaviour on whichever
dialect the suite is pointed at.
**The migration advisory lock was taken on a pool.** `pg_advisory_lock` belongs to the
session that took it. Issued on one pooled connection and released on another, the unlock
is a no-op with a warning and the lock stays held until that connection happens to close.
Two replicas migrating at once, which is the ordinary case in k8s, is exactly what it was
supposed to protect. It now runs on one pinned connection.
**Writes survived by accident.** `set banned = 1` and `where banned = 0` work on Postgres
only because its boolean input syntax accepts the strings '1' and '0'. That is luck rather
than design, and it is why the read path was the one that broke.
### Each Postgres harness gets a database, not a schema
A schema per harness was the obvious choice and it does not work. Kysely's migrator asks
the introspector whether its bookkeeping tables exist, and with no schema configured a
table of that name in any schema counts, so twenty parallel schemas each holding a
`kysely_migration` convince each other the work is already done. A database each has no
such ambiguity. It is slower, which is what the longer timeout under `TEST_DATABASE_URL`
is for: building a real database is honest work, not a hang.
### DATABASE_POOL_MAX
Connections are the resource that runs out first when replicas multiply. It was hardcoded
at ten. It is now a variable, documented next to the arithmetic that matters: pool size
times (api replicas plus worker replicas) has to stay under the server's max_connections.
### Both new tests were checked against a broken implementation
A green test that cannot fail proves nothing, so both were run against a deliberately
broken version first:
- Remove `FOR UPDATE SKIP LOCKED` from the Postgres claim and the two-worker test reports
188 claims for 100 jobs.
- Remove the drain wait from the shutdown path and the in-flight request is cut off.
The shutdown test spawns a real process and holds a request open by sending its body in two
pieces, so the server is genuinely inside the handler when SIGTERM arrives. Asserting on
the app object would have proved nothing about a socket.
### The manifests are checked for the thing that actually rots
They cannot be applied to a cluster from here, so `deploy.test.ts` checks the part that
drifts: every variable set in a ConfigMap, Secret or `env:` block has to be one the env
schema knows, the grace period has to outlast the 25 second drain, and the API has to be
the one that does not poll. A variable renamed in `src/lib/env.ts` and forgotten in a
ConfigMap is a pod that starts and then refuses to boot, and no generic schema validator
would catch it.
### The e2e job stays on SQLite
SPEC.md section 13 puts the golden paths against the compose stack, which is combined mode:
SQLite and local files. It is also the only shape `e2e/db.ts` can read, since those helpers
open the database file to check what a flow actually wrote. S3 is covered where it belongs,
in the unit suite against MinIO, and Postgres by the dialect matrix.
### The storage drivers had no tests at all
S3 is required in scaled mode and had never been exercised. Both drivers are now held to
the same round trip, with the S3 half gated on `TEST_S3_ENDPOINT`. That half has not run on
this machine: there is no object store here and no Docker to start one. CI runs it.
### Still not run here
The images, the compose stack and the manifests have never been built or applied on this
machine. No Docker daemon it can reach, no cluster. The README says so where someone about
to deploy will read it, rather than only here.