GSAP carries the counter roll-ups, the bandeja card physics, the dialog transitions and the three success moments FLOWS.md allows. Every one of them checks prefers-reduced-motion first and does nothing when it is set. boneyard and canvas-ui are not what SPEC.md's stack table says they are: on npm the names belong to two abandoned projects that do neither job. The skeletons were already ours; the two canvas spots are now sixty lines each with no dependency. DECISIONS.md records the substitution. The app installs, keeps a scan taken with no network in IndexedDB and sends it when there is one, falls back to a page that explains itself, and can push a deadline notice. Reading the log of what is queued is the source of truth, so the notice clears when the capture actually lands. The CSP now allows scripts by per-request nonce rather than by 'unsafe-inline'. That forced /offline to render per request: a prerendered page carries a build-time nonce no live policy matches, so its scripts were blocked and it never hydrated. Two crashes fixed on the way. web-push throws on a VAPID subject that is not https: or mailto:, and the code handed it APP_PUBLIC_URL, so any machine with push keys died at boot; a misconfigured optional channel now switches itself off and says why. And a subscription the push service answers 410 for is deleted rather than retried forever. Lighthouse on the production build: accessibility 100, best practices 96, SEO 100, performance 73. The performance number is not trustworthy on this machine and DECISIONS.md says why; total blocking time did fall from 17.6s to 1.7s once the hero canvas stopped drawing at full resolution every frame and the landing page stopped importing GSAP. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
731 lines
42 KiB
Markdown
731 lines
42 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.
|
|
|
|
### 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: `/offline` 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.
|