mega-plant/AGENTS.md
David 80ca4788a2 Give external invoices a supplier name; keep them off the sites page
The name on an invoice means two different things. On an internal invoice it is a
SITE, transcribed from the invoice's own description. On an external invoice it is
the SUPPLIER - and the PDF does not print the supplier's company name at all, so
there is nothing to transcribe and a human has to type it.

`invoices.site` stays the single field; the column header follows the page (Site on
Internal, Supplier on External) and only the external one is editable, via
components/SupplierCell.tsx and PATCH /api/invoices/:id/site. It saves on blur or
Enter, never per keystroke, and an empty value clears the name so a stray
description fragment can be removed rather than displayed as a supplier.

Sites stop taking their names from external invoices, in both places:

- syncSitesFromInvoices only creates sites from internal invoices, so a job
  description can never be registered as a yard.
- buildSiteSummaries only counts internal invoice lines towards a site, which is
  what /sites and the dashboard's per-site chart read.

This is the "Move containers" case. PHI 192's description is "Crane Truck Hire 17
September 2026 Move containers", which had registered a site literally called
"Move containers" - a job, not a yard - on /sites, on the dashboard's per-site
chart and in the plant register's site filter. The record stays in the collection
(sites are never deleted) but is no longer surfaced; the count went 13 -> 12.
External income is not lost by this - it is booked to a machine and still appears on
/income and /charts. It is simply not a site.

The intake route stops resolving an external invoice's text against the site list,
which would have invented a site out of a description, and reports needsSupplier so
the import toast says the name is outstanding.

npm run seed now captures existing invoice names before it wipes the collection and
re-applies them where data/source does not specify one - the same data-loss trap the
uploaded PDFs already had. A JSON name always wins, so internal sites are unchanged.

Verified: typecheck clean, 45 tests pass (4 new, covering the "Move containers" case
directly), production build green, the new PATCH route 401s when anonymous, and
/sites plus both invoice pages pass the mobile checklist at 375x812 and 768x1024. The
supplier edit was exercised end to end and the test value reverted.

Still open: the three external invoices (PHI 178/186/192) have no supplier name yet
and none can be derived from the data. They show "Set supplier" until the client
supplies them.

Note: lib/rows.ts, lib/server-data.ts and the running/insurance feature are
someone else's in-flight work and were deliberately left uncommitted here.
2026-10-08 12:04:42 +02:00

335 lines
20 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# AGENTS.md — Mega Plant
## Mobile checklist (Round 4 standing rule)
Every page must pass the mobile checklist before it is considered complete:
1. **No horizontal page scroll at 375px.** Individual tables may scroll inside their own container; the page itself may not.
2. **No text under 12px on mobile.** Bump `text-[10px]` labels up.
3. **No tap target under 44×44px.** Keep the visual size if you like — add padding to the hit area.
4. **Wide tables scroll inside their own container** with a sticky first column and `-webkit-overflow-scrolling: touch`. Add a visible scroll affordance (e.g. right-edge fade).
5. **Filter controls collapse** below `lg` — pills become a `<select>` dropdown. Don't stack 13 pills into 13 rows.
6. **Verify at 375×812 and 768×1024.** Run `document.documentElement.scrollWidth > document.documentElement.clientWidth` and confirm it returns `false`. Count tap targets and text sizes. Screenshot every route.
If a route fails any of the above, it is not done.
## Build / dev
- `npm run dev` → port 6007 (nohup-detached: `nohup npm run dev > /tmp/plant-dev.log 2>&1 & disown`)
- `npm run build` → 15 routes, webpack mode (PWA is webpack-only)
- `npm run typecheck` → `npx tsc --noEmit`
- `npm run seed` → wipe + re-import `data/source/*.json` into PocketBase. Fails loudly on any unmatched row or broken VAT.
- `node scripts/setup-invoice-collections.mjs` → create the `invoices` / `invoice_lines` collections (idempotent).
- `node scripts/book-invoice.mjs` → post invoice lines into `monthly_income` as additional rows.
## Invoices — adding another one
`scripts/seed.mjs` imports **every** `data/source/invoice-*.json` automatically, so a new
invoice is one file in that folder — no script edit. Each needs `invoice_no`, the period,
`vat_pct`, the header totals and a `lines` array. The seed re-checks the arithmetic on every
invoice and refuses any one that does not balance, while still seeding the others.
⚠️ `npm run seed` **deletes and recreates the `invoices` rows**, so any PDF uploaded through
`/income/invoices` is lost on the next seed and must be re-attached. Copy the PDFs somewhere
durable first — they are not in the repo.
`invoices.site` is a plain text label transcribed from the invoice's own description
(`PHI 166` → "West Park", `PHI 167` → "Halfway Gardens"). There is **no `sites` collection
yet** — build one when the client has sites to list, and back-fill `site` from this field.
## Internal vs external invoices — one collection, two pages
The register is split in two by the **debtor printed on the invoice**:
- **`/income/invoices` — Internal Invoices.** The debtor is a Mega group company.
- **`/income/external-invoices` — External Invoices.** The debtor is a third party.
**This is one `invoices` collection filtered two ways, never two collections.** Do not
"fix" it by giving each page its own collection — that would let a row exist on neither
page and make the totals unable to reconcile against `/income`. The split lives in
`lib/invoice-party-policy.json` (the entity list) and `lib/invoice-party.ts` (the matching),
and both pages render through the single `components/InvoiceRegisterPage.tsx`, so they
cannot drift apart the way two hand-written pages would.
As of 2026-10-08 this splits the 19 invoices in the register **16 internal / 3 external**:
- Internal — debtor `Mega Infrastructure`, no `09MIS001`. All the seeded `data/source/invoice-*.json` files.
- External — `PHI 178` (`09CIG001`), `PHI 186` (`09DTE001`), `PHI 192` (`09DAS001`).
⚠️ **A third-party invoice may carry an empty `debtor` name and only a `debtor_no`.** All three
external invoices do exactly that. So the policy matches on **name OR debtor number**, never
name alone — matching by name only would misfile every one of them. An invoice naming neither is
never internal: it lands on External and the page reports it as unclassified.
Three consequences that are easy to get wrong:
- **Every write must revalidate BOTH pages.** Booking a line, importing a PDF and retrying an
intake can land on either side depending on the debtor, so a bare
`revalidatePath("/income/invoices")` leaves the other page showing stale rows. Use
`revalidateInvoicePages(...)` from `lib/invoice-revalidate.ts` — never a bare
`revalidatePath` for anything touching `invoices` or `invoice_lines`.
- **Route paths are client-safe; the revalidate helper is not.** `lib/invoice-routes.ts` holds
the path constants and imports no `next/*`, so a client component may import it.
`lib/invoice-revalidate.ts` imports `next/cache` and is server-only.
- **The two routes are siblings, never nested.** `isGroupActive` in `Sidebar.tsx` matches a
child by path prefix, so `/income/invoices/external` would satisfy the prefix test for both
leaves and highlight "Internal" and "External" together.
`invoices.category` is `"External Plant Hire"` on **every** invoice, internal ones included —
it is the supplier's own wording transcribed from the PDF, **not** this internal/external
classification. Never derive the split from it.
### The name on an invoice means two different things
`invoices.site` is a **SITE** on an internal invoice and the **SUPPLIER** on an external
one. Same field, different meaning — so the column header follows the page (`Site` on
Internal, `Supplier` on External) and only the external one is editable.
- **Internal** — transcribed from the invoice's description and folded against the existing
sites by `resolveSiteName`. Read-only in the UI; changing it would move real site data.
- **External** — **the PDF does not print the supplier's company name**, so there is nothing
to transcribe. A human types it on `/income/external-invoices` (`components/SupplierCell.tsx`,
`PATCH /api/invoices/:id/site`). Saves on blur or Enter, never per keystroke; an empty
value **clears** the name.
The three external invoices are seeded that way and their names are **not yet known** — ask
the client rather than inventing them. Until then the page shows `Set supplier`.
⚠️ **`syncSitesFromInvoices` and `buildSiteSummaries` only ever consider INTERNAL invoices.**
This is what keeps the sites page honest. PHI 192's description is "Crane Truck Hire 17
September 2026 Move containers", which had registered a site literally called **"Move
containers"** — a job description, not a yard — on `/sites`, on the dashboard's per-site
chart and in the plant register's site filter. The site record is left in the collection
(never delete — see above); it is simply no longer surfaced, and the count went 13 → 12.
External income is **not** lost by this: it is booked to a machine and still appears on
`/income` and `/charts`. It is simply not a site.
The plant register's site dropdown still reads the `sites` collection directly, so "Move
containers" remains selectable there. That is deliberate — a container genuinely can be
parked for a move job — but change it if the client disagrees.
## Invoice booking — run order matters
`npm run seed` **wipes `monthly_income`** and re-imports only the workbook, so invoice-derived
income is destroyed on every seed. Always:
```bash
npm run seed && node scripts/book-invoice.mjs
```
Booking is idempotent: each posted row is stamped with `monthly_income.invoice_line`, and
re-running removes the prior invoice-derived rows before re-inserting. Rows with an empty
`invoice_line` are workbook data and are never touched.
An invoice line with no `booked_asset` is **left unbooked and reported loudly** — never guessed
at. Assign it from `/income/invoices` (the **Booked to** column), or set `asset_code` in the
relevant `data/source/invoice-*.json` and re-run the pair above.
## One machine, many sites — not a duplicate
A machine hired to more than one site in the same month is **normal use, not a duplicate
billing**. Client ruling, 2026-10-01. Such a line is booked to the **same machine** as income,
and every hire accumulates on that one asset.
- `MPLANT05` (SC01) → PHI 166 West Park **and** PHI 171 Cosmo City, same period.
- `SS-01-CASE` (SS01) → PHI 167 Halfway Gardens **and** PHI 171 Cosmo City, same period.
`findDuplicateMachineCodes` still finds these overlaps, but the result is **informational, not
a problem**: it must never tint a row red, badge it as unbooked, or block booking. The income
figure's hover/tap breakdown shows each machine's full hire history across sites, which is
where a multi-site machine should be read. Only an **unbooked** line is a real problem.
The register has no `MPLANT04` / `MPLANT05` codes, so those map to `OC01` / `SC01` as the
consistent target — chosen, not derived. Confirm with the client if either should differ.
## Site monthly chart — which month an invoice lands in
`/sites` plots one sparkline per site (billed per month) and a site's row expands to the
invoices behind each month. `buildSiteSummaries` (`lib/sites.ts`) produces both.
**The month is always the invoice's `booked_month`, never `invoice_date` and never
`period_from`.** `seed.mjs` sets `booked_month`/`booked_year` from the month of
`period_to`, which is the same rule that posts the income into `monthly_income`. So a
site's September total on `/sites` is the same rand as the September column on `/income`
— the two screens cannot disagree. PHI 166 (period 21 Aug → 18 Sep) therefore books to
**September**. Re-deriving the month any other way is the one way to break this.
Other rules worth not re-learning:
- A month total is the **sum of that site's lines**, not the invoice header total. One
invoice carries many lines to a site (PHI 166 has five), so an invoice is listed once
per month with its lines accumulated — never one card per line.
- `months[]` is 12 slots of `null` for "no invoice", never `0`. A site whose billing
starts in September is not eight months of zero.
- The series covers ONE year — `latestInvoiceYear(invoices)`, the max `booked_year`.
Months from other years are dropped, not folded in, and the panel says so.
- `SiteSummary.pinned` exists because the table is a client component and cannot look
the site up in the `sites` collection itself.
- **`/sites` defaults to the Cards view, deliberately mirroring `IncomeView`** — same
toolbar, same card shape, same 12-month strip on expand, same three-up footer. The
client likes the vehicles overview and asked for the sites to read the same way, so
keep the two in step when either changes. Grid is the old table, kept as a toggle.
The site detail link lives **inside the expanded panel only** — a card is a
`<button>`, so a nested `<a>` would be invalid HTML.
## Nested wide tables — the sticky-column trap
`/income/invoices` expands an invoice row into a line-item table **inside a `<td>` of the
invoice list's own scrollable table**. Two things break if this is not pinned down, and
both look like a rendering glitch rather than a layout bug:
- The inner table's `min-width` propagates up and **stretches the outer table** (820px
became 1234px). The whole list then scrolled sideways whenever a row was open. The
detail is now measured with a `ResizeObserver` and given that pixel width, so it
cannot influence the outer table.
- Both tables' first columns are `sticky left-0`, and **two nested stickies at the same
offset stack on top of each other** — the invoice number, the site and the period all
printed over one another. The detail wrapper is `sticky left-0 z-20` (above the outer
cell's `z-10`) and holds a fixed width, so the inner table scrolls inside its own box
and is the only sticky in it.
**A sticky cell must have an OPAQUE background.** The unbooked rows were tinted
`bg-[#D1495B]/[0.04]` — 96% transparent — so every column they covered bled through them
on scroll. The row tint is now the solid `#FDF3F4` (dark: `#292934`) and the sticky
cells repeat it exactly.
Two more traps on this screen: a `flex-wrap` KPI row sizes tiles by CONTENT, so the wide
"booked to income" tile pushed the third one onto a second row below ~644px — it is a
`grid-cols-3` now. And the **Booked to** column needs `min-w-[240px]` with the two
pickers stacked inside it: at 768px the column is only ~166px, so two 200px controls
could not sit side by side, and when they were not allowed to shrink they spilled past
the edge of the table.
## The machine dropdown — a portal is NOT inside its trigger
`MachineCombobox` renders its menu with `createPortal(..., document.body)`, because the
line table lives in an `overflow-x-auto` container that would clip an in-place dropdown.
**A portal is not a DOM descendant of the component that made it.** The click-away
handler tested `boxRef.contains(target)`, so every option in the menu read as an outside
click: `mousedown` closed the menu, React unmounted the button, and the `click` that
would have booked it never fired. Selecting a machine silently did nothing — the menu
just closed. The menu now has its own ref and is treated as part of the control.
Symptom to recognise: a dropdown opens, options highlight, and choosing one does nothing
at all — no request, no error, no change. Suspect the outside-click test before the API.
## Money on invoices — VAT is NOT revenue
On `PHI 166` the totals block reads `SUB TOTAL 161,367.50 / Escalation 0.00 / PLUS VAT
24,205.13 / INVOICE TOTAL 185,572.63`. The R24,205.13 sits under the column headed **PLUS
VAT** — it is 15% VAT, *not* an escalation, and escalation is R0.00 on every line.
- The **invoice total (`total_incl`) is VAT-inclusive** — that is the headline figure.
- `total_excl` (hire) + `vat` = `total_incl`, checked per line and on the header by the seed.
- **Booked amount follows `lib/invoice-booking-policy.json` → `bookInclusiveVat`.** It is
currently `true`: amounts are booked VAT-**inclusive**, per the client's ruling of
2026-10-01. That means SARS output VAT of R29,935.13 (Sept 2026 set) sits **inside** the
R229,502.63 shown as income on `/income`, so those figures are a cash/billing view, not a
VAT-excl. P&L. Flip the flag to `false` and re-run `npm run seed && node scripts/book-invoice.mjs`
to book ex-VAT instead. Both the app and the CLI script read that one file, so they
cannot disagree.
- The VAT stays visible rather than hidden: `/income` shows "includes R 29 935 VAT" and each
machine card shows "incl R X VAT" (`IncomeRow.invoiceVat`). Do not collapse the two.
Extract these by coordinate (`pdfplumber`, x-positions) or read the rendered page visually.
Plain text extraction reorders these columns and silently turns VAT into escalation.
## Manual invoice actions
- `POST /api/invoice-lines/:id/book` body `{ assetId }` (`""` clears) — sets `booked_asset`
and **moves** the `monthly_income` row to the new machine rather than adding a second one.
- `POST /api/invoices/upload` multipart `file` + `invoiceNo` — PDF only, 20 MB cap, stored on
`invoices.source_pdf`.
- `GET /api/invoices/:id/document` — proxies the PDF. Do **not** link to
`NEXT_PUBLIC_PB_URL/api/files/<filename>`: the `invoices` collection is rule-locked and
that path 404s in a browser. PocketBase also resolves protected files by the full
`/api/files/<collectionId>/<recordId>/<filename>` path — build it with `pb.files.getURL()`.
## Color palette — never change
Coolors:
- `#EDAE49` Sunny — orange/positive accent
- `#D1495B` Brick Red — deficit/destructive
- `#00798C` Notartis Seaweed — teal/stable
- `#30638E` Lights Casual — main blue/brand
- `#003D5B` Indigo Dye — deep navy
Chart marks (deeper steps of the same hues — see HANDOFF-PROMPT-3 §A.4):
- `--color-chart-income` (light `#0090A6`, dark `#00A6BE`)
- `--color-chart-fixed` (`#BE851F` both)
- `--color-chart-running` (`#8B5CF6` both) — the project's existing
`--color-brand-purple`, so Running introduces no new hue. **Client ruling, 2026-10-02:**
Running was moved off red because red is now reserved for a month that made a loss, and a
red Running line sitting next to a red loss segment was unreadable. Yellow was considered
and rejected — `--color-chart-fixed` is already a gold.
- `--color-chart-loss` (light `#C13B4D`, dark `#D94F60`) — a month that did not make
profit. **It has its own token on purpose:** it used to read
`--color-chart-running`, so recolouring Running silently dragged the loss red with it.
- `--color-chart-profit` (`#7A9E06` both) — the profit line's green. **Client ruling,
2026-10-02: this is a second deliberate exception to the "never change the palette" rule.**
It is a bright yellow-green, chosen so the profit line reads as its own series instead of
blending into the teal income line.
`globals.css` hard-codes `.recharts-cartesian-axis-tick text { font-size: 11px }`, which
**beats Recharts' `tick.fontSize` prop**. Any "bump chart text to 12px on mobile" work has to
happen in that CSS rule or it silently does nothing.
## The profit line on `/charts` — Recharts constraints, do not re-derive
`MachineLineChart` draws profit as teal where the month made money and red where it did not,
fading across each sign crossing. Three things make this fragile:
- **A `Line` is one `<Curve>` path.** `<Cell>` does **not** recolour a segment — Cells only
work for `Bar`. Per-segment colour therefore requires **one `<Line>` per same-sign run**,
plus one extra `<Line>` per crossing for the gradient.
- **Never give a `Line` its own `data` prop inside a shared chart.** Recharts builds a
categorical axis by concatenating the data of *every* series, so a short sliced `data`
re-adds the month labels: the axis renders "Jan … Aug, Jan … Aug" while that series plots
at the wrong months. Put each variant on the shared dataset as its own key
(`profitRun0`, `profitCross0`) and null it outside its run; `connectNulls={false}` then
draws just that run in the right place.
- **Profit `Line`s must be direct children of the chart.** Recharts parses only known child
types, so wrapping them in a `<g>` makes the entire profit line disappear without an error.
- **The tooltip needs a custom `content` component** (`ChartTooltip` in `MachineLineChart`).
Recharts feeds the tooltip one entry per series, so the per-run keys would surface as
`profitRun0` / `profitCross0` and "profit" would appear three or four times. The component
drops every profit-backed series and adds one computed row, coloured by the month's sign.
The base series carry an explicit `name`, because Recharts otherwise labels them by key
(`running` reads as "Running cost", which is what the key is called).
Two deliberate honesty rules: profit stays **null** in a month with no income figure (never
treated as 0, which would draw a green dot for a month that was never billed), and a month
with no `monthly_running_costs` row gets a **hollow dot** because its profit is optimistic.
`/charts` alone reads profit including running cost — `lib/profit.ts` keeps its workbook
default for every other surface, and the card badge, the profit line and the Profit bars
view all share the one flag in `ChartsGrid`.
The toolbar's **Series** button toggles which lines a card draws (`mp:chartsSeries` in
localStorage, so the choice survives a reload). Two rules keep it honest: a hidden series
leaves the legend **and its Y axis** with it, so there is never an axis for a line that is
not drawn; and the last visible series cannot be switched off, because an empty chart reads
as broken. The Profit toggle also owns the loss colour, since red is a sign of the profit
line rather than a series of its own.
## Cost model
- `lib/profit.ts` is the single source of truth for profit. Default `includeRunningCost: false` (matches workbook). To flip: one-line change in `lib/profit.ts`.
- Term 1 = instalment ✅ confirmed.
- Term 2 = insurance ✅ confirmed.
- Term 3 = operator wages — varies per month per machine — **STILL NEEDS CLIENT INPUT**.
## Open questions
- Q1: Does profit include running cost? (default: no, matches workbook).
- Q2: What is term 3 of the fixed-expense formula? (See Round 1/2/3 HANDOFF-PROMPT files.)
## PocketBase
- URL: `process.env.NEXT_PUBLIC_PB_URL` (env or `.env.local`)
- Auth: `pb.collection("_superusers").authWithPassword(env)` (Round 1 §5.3)
- Collections: `assets`, `monthly_income`, `monthly_running_costs`, `overheads`, `cost_lines`, `invoices`, `invoice_lines`
- `monthly_income` no longer carries `running_cost` — derive from `monthly_running_costs` at read time
- `monthly_income.invoice_line` → `invoice_lines`, nullable. Empty = workbook row, set = invoice-derived row
- `invoice_lines.booked_asset` → `assets`, nullable. Empty = line not yet booked to a machine