From fe9f6c6dec1c9b07089fe5547713e219cf7ae5ff Mon Sep 17 00:00:00 2001 From: Lucas Winther Date: Wed, 2 Sep 2026 02:26:25 +0200 Subject: [PATCH] docs: add TODO, ARCHITECTURE and AGENTS guides for handover MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TODO.md carries the remaining work with the reasoning already settled, so the next agent does not re-derive decisions: tasks 3-6 of the versioned-rules plan, the designed-but-unbuilt baked FX rates, the queued visual redesign, the small residuals, and an explicit list of what only a human with a browser can check. ARCHITECTURE.md explains the layer rules, the single-implementation fold, the mutation pipeline's three deliberate properties, and the gotchas that each cost a real bug — reading a deferred event.target.value, an unset BUN_PUBLIC_* var reaching the browser, and overriding shadcn's --accent. CLAUDE.md gains those gotchas, the dated-rule model that replaced standingRule, the validation this project deliberately omits, and an explicit statement that visual and offline behaviour cannot be verified in the agent environment. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 22 +++++++ ARCHITECTURE.md | 155 ++++++++++++++++++++++++++++++++++++++++++++++++ CLAUDE.md | 57 +++++++++++++++++- TODO.md | 96 ++++++++++++++++++++++++++++++ 4 files changed, 328 insertions(+), 2 deletions(-) create mode 100644 AGENTS.md create mode 100644 ARCHITECTURE.md create mode 100644 TODO.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..b5183a2 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,22 @@ +# Agent guide + +This repository's agent instructions live in **`CLAUDE.md`** — read it first. +It carries the project invariants, the gotchas that cost real bugs, and the +validation this codebase deliberately does not perform. + +Then, depending on what you are doing: + +- **Changing anything structural** → `ARCHITECTURE.md` (layers, data flow, and + why the non-obvious choices are what they are). +- **Picking up queued work** → `TODO.md` (ordered, each item carrying the + reasoning already settled). +- **Making a product decision** → `PRODUCT.md` (who this is for, what must never + break, and what must not be invented). +- **Wondering why something is the way it is** → `docs/superpowers/specs/` + records the decisions *and the alternatives that were rejected*. + +Two rules worth knowing before your first edit: + +1. **Bun only.** `bun test`, `bun run`, `bunx`. Never npm, jest, vitest or vite. +2. **There is no browser here.** Anything visual, offline, or focus-related + cannot be verified in this environment. Say so rather than claiming it works. diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md new file mode 100644 index 0000000..e5b2d55 --- /dev/null +++ b/ARCHITECTURE.md @@ -0,0 +1,155 @@ +# Architecture + +Orientation for anyone — human or agent — changing this codebase. It explains +where things live and, more usefully, *why* several non-obvious choices are the +way they are. Most of them were paid for with a bug. + +## Shape + +A personal budgeting PWA. No backend. IndexedDB in the owner's browser is the +only copy of their data. One static artifact deploys to both GitHub Pages (under +a subpath) and a self-hosted Docker image. + +Bun is the runtime, package manager, bundler and test runner. React 19, +Tailwind 4, shadcn/ui. No Vite, no npm, no jest. + +## Layers + +``` +src/domain/ pure. no React, no IndexedDB, no clock. testable with no DOM. +src/store/ persistence, mutations, and the ONLY place a clock is read. +src/export/ JSON and ODS generation. pure. +src/ui/ React. reads through hooks, writes through store actions. +build.ts static build: publicPath, hashed names, SW precache, 404.html. +server.ts the Docker image's static file server. +``` + +The dependency rule is one-directional: `ui -> store -> domain`. `domain` imports +nothing from the others; `store` never imports from `ui`. + +`src/domain/smoke.test.ts` asserts the domain is importable with no DOM. That is +why happy-dom is registered **per test file** rather than as a global bunfig +preload — a global DOM would make that assertion vacuously true and silently +retire the check. + +## The fold is the single source of every number + +`src/domain/fold.ts` holds the one implementation of envelope rollover: + +``` +remaining = carriedIn + allocation − charges (carried forward, monthly) +``` + +Unspent carries; overspend carries as debt. **Nothing else may recompute this.** +The month view, year matrix, summary, per-post views and the ODS export all +aggregate over `foldBalances` / `figuresFor`. A closing balance is read from +`byMonth[11].remaining`, never re-summed. + +If you find yourself writing `carriedIn + allocation - charges` outside +`fold.ts`, that is the invariant breaking. It matters because two screens +silently disagreeing about someone's money is the worst failure this app has. + +Editing any past month recomputes everything after it. That is correct: the fold +is a left fold, and history is causal. + +## Data flow for a single edit + +``` +onChange + └─ capture event.target.value into a const ← synchronously, see Gotchas + └─ store.mutate(draft => action(draft)) + └─ serialization queue (one mutation at a time) + └─ structuredClone(snapshot) → apply → await persistence.write() + └─ snapshot = next; notify() + └─ useSyncExternalStore re-renders +``` + +Three deliberate properties in that chain: + +- **Write before the snapshot moves.** If the write fails, the UI keeps showing + the last *saved* state, and `useMutate` surfaces the rejection. The user is + never shown data that was not persisted. +- **Serialized.** Two edits inside one write window would otherwise both clone + the same base and the later commit would silently drop the earlier one. +- **Cloned.** `useSyncExternalStore` detects change by reference identity; + mutating in place makes the UI silently stop re-rendering. + +## Money + +A float, deliberately, with two rules that make it safe: + +1. **Round at every boundary** via `roundMoney` — after any division, after any + FX conversion, and at the `src/store/actions.ts` mutation boundary. The UI + adds no rounding of its own. +2. **Never round the last distributed part independently.** Compute it as + `total − sum(others)` so the parts sum exactly to the whole. `withRemainder` + in `money.ts` is the only place that logic lives. + +**`FxRate.baseUnitsPerOne` is never rounded.** Rates are stored at six decimals +and are not money; quantizing one to cents is wrong in kind. + +## Splits and finance plans compose + +A purchase can be split across posts *and* spread over months at once. The trick +is in `charges.ts`: first compute how the **whole** purchase divides across posts +(`splitPartsOfTotal`), then distribute each month's slice proportionally *to +those parts*. Both split modes collapse to one code path per slice. + +Applying a split's raw value per-slice looks simpler and passes every percentage +test — and charges a fixed "2000 to Video Games" in full on every slice. + +## Things the app deliberately does NOT validate + +Adding a "missing" guard here is a regression, not a fix: + +- going over budget, ever, anywhere; +- allocation percentages summing past 100%; +- splits that do not sum to the total (the remainder-absorbing split reconciles + them by design); +- negative amounts (a refund is a normal line). + +## Gotchas paid for with real bugs + +- **Never read `event.target.value` inside a `mutate()` callback.** `mutate` + defers behind the queue and a write; React resets a controlled input's DOM + value long before the callback runs, so you read the *old* value and commit + it. This froze the income field at 0. `src/ui/eventCapture.test.ts` fails the + build if it reappears. +- **Read `process.env.BUN_PUBLIC_*` through a `try/catch` accessor.** Bun only + inlines the literal when the variable is *set*; unset, the reference survives + into the browser bundle and throws `ReferenceError: process is not defined` + before the app boots. A `typeof process` guard does NOT work — after inlining + it still runs in the browser and discards the inlined value, silently breaking + subpath deploys. See `readBasePathEnv` in `src/ui/basePath.ts`. +- **Do not define a bare `--accent` CSS variable.** shadcn owns it as a + near-white hover *background* paired with near-black text; overriding it drops + hover contrast below WCAG AA app-wide. Project tokens are namespaced + `--budget-*`. +- **`sw.js` must stay unhashed and be served `no-cache`.** A cached or hashed + service worker can never be replaced, pinning users to an old build forever. +- **A React error boundary does not catch throws from event handlers.** Domain + functions that throw (`MissingRateError`, `remainderIndexOf`, `monthOf`, + `equalSlices`) must be guarded at the call site when called from a handler. + +## Testing, and its limits + +Most logic is pure and tested directly with `bun test`. The domain carries the +bulk of the coverage; the UI is thin by design. + +**There is no browser in the agent environment.** Nothing has ever verified +visual appearance, real offline behaviour, the install or update prompts, +keyboard focus traversal, or the `.ods` opening in a spreadsheet. Where a check +could not be run, the honest substitute was used and recorded — for example the +`.ods` is validated structurally by unzipping it and asserting `mimetype` is +first and stored, every XML part parses, and numeric cells carry `office:value`. + +`src/ui/eventCapture.test.ts` is a *source-level* guard rather than a +behavioural one, because the bug it prevents depends on real browser render +timing that happy-dom does not reproduce. + +## Where the reasoning lives + +- `PRODUCT.md` — durable product truth: who it is for, what must never break. +- `TODO.md` — queued work, with the reasoning behind each decision already made. +- `docs/superpowers/specs/` — design decisions and the alternatives rejected. +- `docs/superpowers/plans/` — task-by-task implementation plans. diff --git a/CLAUDE.md b/CLAUDE.md index 3a64555..1981b78 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -118,5 +118,58 @@ For more information, read the Bun API docs in `node_modules/bun-types/docs/**.m `src/domain/fold.ts`. Year and summary views aggregate over it — they never reimplement the math. - Over-budget is always allowed. Never add validation that blocks it. -- Base path comes from `BUN_PUBLIC_BASE_PATH`; never hardcode a subpath. -- Read the spec at `docs/superpowers/specs/2026-09-01-budget-app-design.md`. +- Base path comes from `BUN_PUBLIC_BASE_PATH`; never hardcode a subpath, and + read it through a `try/catch` accessor — see the gotcha below. +- A post's allocation is a dated series, `Post.rules: RuleVersion[]`, resolved by + `ruleAt(post, monthId)`. `Post.standingRule` no longer exists. A post with no + applicable version allocates zero; a per-month override still wins outright. +- Posts are archived, never deleted, because purchases reference them. Archived + posts still fold. + +## Start here + +- `ARCHITECTURE.md` — layers, data flow, and why the non-obvious choices are + what they are. Read it before changing anything structural. +- `TODO.md` — queued work, each item carrying the reasoning already settled. +- `PRODUCT.md` — who this is for and what must never break. +- `docs/superpowers/specs/` — design decisions, including alternatives rejected. + +## Gotchas that cost real bugs + +Each of these shipped once. Re-introducing one is a regression, not a style +choice. + +- **Never read `event.target.value` inside a `mutate()` callback.** `mutate` + defers behind the write queue; React resets the input's DOM value first, so + you commit the old value. Capture into a `const` in the handler. + `src/ui/eventCapture.test.ts` fails the build otherwise. +- **`process.env.BUN_PUBLIC_*` must be read via `try/catch`** (see + `readBasePathEnv`). Bun inlines the literal only when the variable is set; + unset, the bare reference reaches the browser and throws before the app boots. + A `typeof process` guard does not work — it survives inlining and discards the + inlined value, silently breaking subpath deploys. +- **Never define a bare `--accent` CSS variable.** shadcn owns it; overriding it + drops hover contrast below WCAG AA app-wide. Project tokens are `--budget-*`. +- **A React error boundary does not catch event-handler throws.** Domain + functions that throw must be guarded at the call site when called from one. +- **Do not "fix" a domain throw by making it return a fallback.** Silently wrong + numbers are worse than a visible error in a budgeting app. + +## Deliberately absent validation + +Adding a "missing" guard here is a regression: + +- going over budget, anywhere; +- allocation percentages summing past 100%; +- splits that do not sum to the total (the remainder-absorbing split reconciles + them by design); +- negative amounts — a refund is a normal line. + +## Verification limits + +There is no browser in this environment. Visual appearance, offline behaviour, +install and update prompts, keyboard focus traversal, and opening the generated +`.ods` in a spreadsheet **cannot be verified here**. Do not claim you checked +them. Substitute a real check where one exists — structural validation of the +`.ods`, curl against the dev server, pure-function tests — and state plainly +what still needs a human. diff --git a/TODO.md b/TODO.md new file mode 100644 index 0000000..1e61f05 --- /dev/null +++ b/TODO.md @@ -0,0 +1,96 @@ +# TODO + +Work queued for whoever picks this up next, human or agent. Ordered. Each item +says enough to act on without re-deriving the reasoning. + +**Current state:** branch `feature/versioned-rules`, 6 commits ahead of `main`, +not merged, not pushed. 270 tests passing, `bunx tsc --noEmit` clean, +`bun run build` succeeds. The branch is coherent to stop at: `SCHEMA_VERSION` is +2 *and* its migration exists, so existing data opens. + +--- + +## 1. Finish time-versioned allocation rules (Tasks 3-6 of 6) + +**Spec:** `docs/superpowers/specs/2026-09-02-versioned-allocation-rules-design.md` +**Plan:** `docs/superpowers/plans/2026-09-02-versioned-allocation-rules.md` +**Ledger + extracted briefs:** `.superpowers/sdd/2026-09-02-versioned-allocation-rules/` + +Tasks 1-2 are done and reviewed clean: `Post.rules: RuleVersion[]` replaced +`Post.standingRule`, and the v1 -> v2 migration landed. + +| Task | What | Brief | +|---|---|---| +| 3 | `setRuleFrom` / `removeRuleFrom` actions | `task-3-brief.md` | +| 4 | JSON import validation for the rule series | `task-4-brief.md` | +| 5 | Rule-history editor in Settings | `task-5-brief.md` | +| 6 | "Change from here" control in the month view | `task-6-brief.md` | + +To resume: read the ledger's handoff section, then execute with +`superpowers:subagent-driven-development` (or by hand — the briefs are complete, +with real code in every step). + +**Watch for:** Task 3 must make `setRuleFrom` REPLACE an existing version for the +same month rather than append. Two versions sharing a `from` make the effective +rule ambiguous, and `ruleAt` would resolve arbitrarily. + +## 2. Baked FX rates — designed and approved, not built + +So a fresh install can convert a EUR purchase on day one instead of erroring. + +- `build.ts` fetches DKK/USD/EUR at build time and embeds them via `define`; + on any network failure it falls back to **committed constants** rather than + failing the build. Fetched rates get `source: "api"` with the build date; + the fallback constants get `source: "manual"`, so Settings never claims a + hardcoded number came from an API. +- They **seed a new dataset only**. They must NOT act as a fallback whenever a + rate is missing — that would undo a deliberate decision: clearing a rate + removes the row so `MissingRateError` surfaces, instead of silently + converting money at a number the user never chose. +- Domain purity forces the shape: `createSeedDataset` takes the rates as an + **argument**; the env read lives in a new `src/store/bakedRates.ts`, in the + store layer, using the `try/catch` accessor pattern (see CLAUDE.md gotchas). + +## 3. Visual redesign + +The owner's words: *"right now it looks a bit boring, simple and ugly. We want +sleek, modern, and nice to use."* Sequencing was their explicit choice: finish +all six rules tasks first, then redesign everything. + +`PRODUCT.md` exists and holds product truth. There is no `DESIGN.md`; the +incumbent visual world is the `--budget-*` tokens in `src/index.css` plus the +design contract summarised in ARCHITECTURE.md. A redesign should treat that as +evidence and anti-reference, not as something to preserve. + +## 4. Smaller items, in no particular order + +- **`confirmImport` has no `.catch`** (`src/ui/routes/settings/DataSection.tsx`). + A failed import write is an unhandled rejection with no error shown. One line. +- **PNG icons 192/512 are not shipped**, so Chrome's install prompt is + incomplete. Binary assets cannot be generated in the agent environment; a + human needs to add them. +- **`round2` in `src/store/actions.ts` hardcodes 2 decimals** rather than + reading `CURRENCY_DIGITS`. Correct for DKK/USD/EUR; silently wrong the day a + non-2dp currency is added. Same latent issue in `sliceTotal` + (`src/domain/plans.ts`). +- **`EMPTY_FIGURES` (`src/domain/fold.ts`) is exported non-frozen** and returned + by shared reference from `figuresFor`. No consumer mutates it today; + `Object.freeze` is a one-line hardening. +- **`buildWorkbook` calls `monthView` per (post, month)**, making a full ODS + export O(posts x months^2). Fine at personal-budget scale. +- **The dev-only `src/manifest.webmanifest` placeholder hardcodes `scope: "/"`.** + Harmless in production, misleading if you dev-serve under a subpath. + +## 5. Needs a human with a browser + +Nothing in the agent environment can run a browser, so these have never been +seen by anyone: + +- the **carry meter** under each post row in the month view — the signature + design element, verified only as a CSS gradient expression; +- **offline behaviour**, the **install prompt**, and the **update prompt**; +- the generated **`.ods` opening in a real spreadsheet** (validated + structurally: mimetype first and stored, well-formed XML, numeric cells); +- **keyboard flow in fast entry** — Tab traversal, Backspace-to-remove, and + focus placement after a row auto-appends; +- **GitHub Actions** has never been executed.