598 lines
44 KiB
Markdown
598 lines
44 KiB
Markdown
|
|
# Multi-site readiness assessment
|
|||
|
|
|
|||
|
|
> **The question.** The Hills, Cosmo City, Castle Gate, Autumns Gate, Pitsana
|
|||
|
|
> Mills, West Park, and more to come. This is *not* a question about
|
|||
|
|
> multi-tenancy or serving multiple client companies. It is: **does the
|
|||
|
|
> existing per-site model actually hold up as the number of sites and the
|
|||
|
|
> volume of historical data grows, and where does it break?**
|
|||
|
|
>
|
|||
|
|
> This document is the deliverable that matters most. Parts A and B are
|
|||
|
|
> forensic; Part C is the plan.
|
|||
|
|
|
|||
|
|
The codebase today handles a small number of sites (the production
|
|||
|
|
instance is 7-10) with low hundreds of thousands of fuel log rows and
|
|||
|
|
similar counts across timesheets, fixed costs, and PO items. The
|
|||
|
|
assessment below uses those numbers as the current baseline and projects
|
|||
|
|
out to the realistic next year.
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## Part A — What works today
|
|||
|
|
|
|||
|
|
The data model is already site-aware. Verified against `src/lib/pb-types.ts`:
|
|||
|
|
|
|||
|
|
- 10 of 13 collections carry a `site` relation (`sites`, `employees`,
|
|||
|
|
`vehicles`, `fixed_costs`, `income_claims`, `purchase_orders`,
|
|||
|
|
`delivery_notes`, `fuel_logs`, `plant_hours_log`, plus `data_import_batches`
|
|||
|
|
indirectly through the processed record).
|
|||
|
|
- 9 of those 10 also carry the denormalised `project_code` text. The
|
|||
|
|
exception is `timesheets`, which carries neither — see
|
|||
|
|
[§B.5](#5-vehicles-plant-and-employees-moving-between-sites) for the
|
|||
|
|
consequences.
|
|||
|
|
- The two collection-tables that don't carry site directly
|
|||
|
|
(`purchase_order_items`, `received_items`) inherit it through their
|
|||
|
|
parent. They are never queried by site on their own.
|
|||
|
|
|
|||
|
|
**Per-site financials are computed once and reused.** All the math lives
|
|||
|
|
in `src/lib/metrics/` and is consumed by the **Sites** list
|
|||
|
|
(`src/app/dashboard/sites/page.tsx`), the **Site** detail
|
|||
|
|
(`src/app/dashboard/sites/[id]/page.tsx`), the **Overview** monthly-fixed-cost
|
|||
|
|
tile (`src/app/dashboard/page.tsx:100-108`), the **chat** tool layer
|
|||
|
|
(`src/lib/agent/tools.ts`), and the **MCP** server (`src/mcp/server.ts`).
|
|||
|
|
The whole app agrees to the last digit because every figure comes from
|
|||
|
|
the same `computeSiteSpend → computeSiteProjection →
|
|||
|
|
buildWeeklyIncomeVsExpenses` chain.
|
|||
|
|
|
|||
|
|
**The agent resolves sites from free text correctly when the query is
|
|||
|
|
unambiguous.** `src/lib/agent/resolve.ts:21-48` (`resolveSite`) goes
|
|||
|
|
id → exact project code → exact name → substring on either, and returns
|
|||
|
|
`{ kind: "ambiguous", matches: [...] }` rather than guessing. The
|
|||
|
|
chat tool layer's `requireSite` helper
|
|||
|
|
(`src/lib/agent/tools.ts:76-92`) surfaces the ambiguity to the model
|
|||
|
|
as a follow-up question. That is the right behaviour for money
|
|||
|
|
questions.
|
|||
|
|
|
|||
|
|
**Imported data is matched to a site deterministically when the source
|
|||
|
|
has a clean identifier.** The fuel importer
|
|||
|
|
(`src/app/api/fuel-logs/import/route.ts:214-224`) and plant hours
|
|||
|
|
importer (`src/app/api/plant-hours/import/route.ts:237-250`) both
|
|||
|
|
build a `Map<name.toLowerCase(), SiteRecord>` once at the top of the
|
|||
|
|
import and reuse it for every row. The PO process-batch route does the
|
|||
|
|
same through `matchSite` (`src/lib/purchase-order-schema.ts`).
|
|||
|
|
|
|||
|
|
**Bottom line for what works.** Up to roughly 7-10 sites with 6-12
|
|||
|
|
months of history (a few hundred thousand rows per heavy collection),
|
|||
|
|
the app is correct. The Overview and Sites pages render in under ~2
|
|||
|
|
seconds. The chat can answer portfolio questions. Imports run end-to-end
|
|||
|
|
in 5-30 seconds depending on the file.
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## Part B — Where it breaks
|
|||
|
|
|
|||
|
|
This part walks each of the eight leads the prompt gave me, confirms or
|
|||
|
|
refutes with file:line evidence, and adds the things I found in addition
|
|||
|
|
that the prompt did not list.
|
|||
|
|
|
|||
|
|
### 1. Hard 50-site cap on the Overview page — **CONFIRMED**
|
|||
|
|
|
|||
|
|
**Lead claim:** `src/app/dashboard/page.tsx:64` calls
|
|||
|
|
`pb.collection("sites").getList<SiteRecord>(1, 50, { sort: "name" })`,
|
|||
|
|
and site 51+ silently vanishes from the KPI tiles and the "Attention
|
|||
|
|
required" rail.
|
|||
|
|
|
|||
|
|
**Evidence.**
|
|||
|
|
|
|||
|
|
- `src/app/dashboard/page.tsx:64` — the 50-row fetch.
|
|||
|
|
- `src/app/dashboard/page.tsx:112-113` — `for (const s of sites.items) siteById.set(s.id, s);` uses only the 50 returned items.
|
|||
|
|
- `src/app/dashboard/page.tsx:135-154` — the "Attention required" buckets are built against `siteById` keys, so a fuel log attached to a site beyond the 50-row window has no site in the lookup map and falls into the `(unknown site)` bucket.
|
|||
|
|
- `src/app/dashboard/page.tsx:176` — `attention.okSiteCount = sites.items.length - problemSites.length` — a 51st site that is healthy *is not counted* in either problemSites or okSiteCount, so its fuel logs are simply dropped from the rail.
|
|||
|
|
- `src/app/dashboard/page.tsx:191` — the "Sites" KPI tile label says `data.sites.length`, again the length of the 50-row response.
|
|||
|
|
|
|||
|
|
**Other capped `getList(page, perPage)` calls found.** Grep across `src/`:
|
|||
|
|
|
|||
|
|
| File:line | Call | Cap | Blast radius |
|
|||
|
|
| --- | --- | --- | --- |
|
|||
|
|
| `src/app/dashboard/page.tsx:64` | `sites.getList(1, 50)` | 50 | Overview sites tile, attention rail, sites list at the bottom of the page |
|
|||
|
|
| `src/app/dashboard/page.tsx:69` | `vehicles.getList(1, 1, { filter: "cost_per_hour>0" })` | 1 (just a count) | KPI tile — fine, only `totalItems` is used |
|
|||
|
|
| `src/app/dashboard/page.tsx:95-98` | `purchase_orders` + `delivery_notes` `.getList(1, 1, { filter: "is_verified=false" })` | 1 (count) | KPI tile — fine, only `totalItems` |
|
|||
|
|
| `src/app/dashboard/purchasing/page.tsx:22-25` | `delivery_notes.getList(1, 200, { filter: "purchase_order='' && processing_status='needs-review'" })` | 200 | "Needs review" panel — site 201+ needs manual recovery from PB admin |
|
|||
|
|
| `src/app/dashboard/plant/page.tsx:21, 26, 30` | `perPage: 5000` on vehicles/fuel_logs/sites | 5000 | Plant page renders are correct as long as the total is < 5000 items |
|
|||
|
|
| `src/app/api/plant-hours/import/route.ts:216, 238` | `perPage: 5000` on vehicles + sites | 5000 | An import file that names a vehicle/site beyond 5000 fails the whole batch with a non-obvious "vehicle not found" error |
|
|||
|
|
| `src/app/api/plant-hours/exception-report/[batchId]/route.ts:70` | `perPage: 5000` on `plant_hours_log` filter | 5000 | Exception report for a large batch is silently truncated |
|
|||
|
|
| `src/app/api/purchase-orders/by-number/[poNumber]/items/route.ts:81` | `purchase_orders.getFullList({ perPage: 500 })` for Levenshtein fallback | 500 | Fuzzy PO match over a corpus > 500 POs misses legitimate matches |
|
|||
|
|
| `src/app/api/purchase-orders/by-number/[poNumber]/items/route.ts:128` | `purchase_order_items.getFullList({ perPage: 200 })` | 200 | Detail page on a PO with > 200 line items drops items |
|
|||
|
|
| `src/app/api/delivery-notes/[id]/ocr-result/route.ts:151` | `purchase_orders.getFullList({ perPage: 200 })` for normalised/Levenshtein PO match | 200 | OCR call where the matching PO is beyond row 200 fails to match |
|
|||
|
|
|
|||
|
|
**The history of this class of bug.** The comment at
|
|||
|
|
`src/app/dashboard/purchasing/page.tsx:27-31` is direct evidence:
|
|||
|
|
|
|||
|
|
> "Use getFullList so there is no silent truncation if `received_items`
|
|||
|
|
> ever exceeds 500 rows — earlier this used `getList(1, 500, ...)` which
|
|||
|
|
> capped and produced 0% fulfillment whenever low-id POs were dropped."
|
|||
|
|
|
|||
|
|
This is the second time the team has hit "capped getList silently
|
|||
|
|
truncated data" (the first was fulfilment, the second is the Overview
|
|||
|
|
sites). It is the highest-leverage class of bug in the system.
|
|||
|
|
|
|||
|
|
**What the user sees.** The Overview page renders with a "Sites: 7" tile
|
|||
|
|
even when there are 50+ sites. The "Attention required" rail and the
|
|||
|
|
sites list at the bottom of the page both show 7-50 rows. Anything
|
|||
|
|
beyond is invisible until the user navigates to `/dashboard/sites`,
|
|||
|
|
which does call `getFullList`.
|
|||
|
|
|
|||
|
|
**Threshold.** At site 51, the Overview starts being wrong. Today this
|
|||
|
|
is a one-line fix; left unrepaired, it will bite the moment a 51st
|
|||
|
|
contract is added.
|
|||
|
|
|
|||
|
|
### 2. Load-everything-then-filter-in-memory — **CONFIRMED, with extra evidence**
|
|||
|
|
|
|||
|
|
**Lead claim:** "Most dashboard pages call `getFullList()` on entire
|
|||
|
|
collections and filter in JavaScript."
|
|||
|
|
|
|||
|
|
**Evidence.** Every `app/dashboard/**/page.tsx` loads full collections
|
|||
|
|
even when the page is a per-site view:
|
|||
|
|
|
|||
|
|
- `src/app/dashboard/page.tsx:75-82` — `fixed_costs.getFullList` (full) and `fuel_logs.getFullList` (full) for the Overview.
|
|||
|
|
- `src/app/dashboard/sites/page.tsx:33-50` — seven parallel `getFullList` calls, no `filter` on any of them. The list page literally pulls every timesheet, every employee, every fuel log, every fixed cost, every PO, every income claim.
|
|||
|
|
- `src/app/dashboard/sites/[id]/page.tsx:48-57` — the per-site detail page. Per the prompt: "filters *some* collections by `site='<id>'` server-side but pulls **all** timesheets and **all** employees regardless." Confirmed: lines 50-56 show `filter: site='${id}'` for `fixed_costs`, `purchase_orders`, `fuel_logs`, `vehicles`, `income_claims`, but `timesheets.getFullList()` (line 51) and `employees.getFullList({ fields: "id,rate_per_hour,site,name" })` (line 52) are unfiltered. Timesheet attribution is reconstructed in JS via `employeeRates.get(t.employee)?.site === id` (line 67).
|
|||
|
|
- `src/app/dashboard/fuel/page.tsx:20-25` — vehicles (full, fleet bucket) + fuel logs (full, with `expand: "vehicle"`).
|
|||
|
|
- `src/app/dashboard/employees/page.tsx:103-105` — employees (full) + timesheets (full) + sites (full).
|
|||
|
|
- `src/app/dashboard/plant/page.tsx:19-32` — vehicles (full, `perPage: 5000`) + fuel logs (full, `perPage: 5000`) + sites (full, `perPage: 5000`).
|
|||
|
|
- `src/app/dashboard/purchasing/page.tsx:14-33` — every PO (full) + every PO item (full) + every DN (full) + every site (full) + every received item (full, but projected to three fields).
|
|||
|
|
- `src/lib/agent/load.ts:32-66, 72-92, 167-187, 205-216` — the agent tool layer pulls whole collections even when serving a per-site answer. `loadAllSitesBundle` (line 45) literally loads everything; `loadSiteBundle` (line 27) does the per-site filter for fixed_costs/POs/fuel/income but loads all timesheets + all employees.
|
|||
|
|
|
|||
|
|
**Cost growth.** With N sites × M months of data, the rows scale as:
|
|||
|
|
|
|||
|
|
| Collection | Approx rows/site/month | Cost at 10 sites × 12 months | Cost at 50 sites × 24 months |
|
|||
|
|
| --- | --- | --- | --- |
|
|||
|
|
| `timesheets` | 200-2000 (1-2 sites) | 240 000 | 2.4M |
|
|||
|
|
| `fuel_logs` | 50-500 | 60 000 | 600 000 |
|
|||
|
|
| `fixed_costs` | 5-20 | 2 400 | 12 000 |
|
|||
|
|
| `purchase_orders` + items | 10-50 | 6 000 | 30 000 |
|
|||
|
|
| `received_items` | 30-150 | 18 000 | 180 000 |
|
|||
|
|
| `income_claims` | 1-4 | 480 | 2 400 |
|
|||
|
|
| `employees` | 10-100 | 1 200 | 6 000 |
|
|||
|
|
| `vehicles` | 5-30 | 600 | 3 000 |
|
|||
|
|
| `sites` | 1 | 10 | 50 |
|
|||
|
|
|
|||
|
|
These are rough orders of magnitude. The **`timesheets` and `fuel_logs`
|
|||
|
|
rows scale fastest**. The `/dashboard/sites` page, the chat's
|
|||
|
|
`loadAllSitesBundle`, and the `/dashboard/employees` page all do a
|
|||
|
|
**full** `getFullList` of `timesheets` on every render.
|
|||
|
|
|
|||
|
|
**Which page falls over first.** The **Sites** list page
|
|||
|
|
(`src/app/dashboard/sites/page.tsx`) is the worst offender — it pulls
|
|||
|
|
seven full collections in parallel and then runs a per-site
|
|||
|
|
`computeSiteSpend` loop. With 50 sites and a year of timesheets, that
|
|||
|
|
single page will pull 2.4M `timesheets` rows over the wire, build a
|
|||
|
|
200 000-entry `employeeRates` map, and then loop `computeSiteSpend`
|
|||
|
|
50 times over 2.4M timesheets — that's 120M `Map.get` calls in the
|
|||
|
|
Node event loop, which won't return in under 30s. The first page
|
|||
|
|
that's perceptibly slow is the **Sites** list, and it's followed
|
|||
|
|
shortly by the **Site** detail page and the **Workforce** page.
|
|||
|
|
|
|||
|
|
**Threshold.** The Sites list page starts feeling slow at roughly
|
|||
|
|
500 000 timesheet rows (≈ 10 sites × 12 months of heavy data). It
|
|||
|
|
becomes unusable at 2M+.
|
|||
|
|
|
|||
|
|
### 3. No global site scope — **CONFIRMED**
|
|||
|
|
|
|||
|
|
**Lead claim:** "The sidebar has no site selector; each module handles
|
|||
|
|
site filtering independently, mostly client-side."
|
|||
|
|
|
|||
|
|
**Evidence.**
|
|||
|
|
|
|||
|
|
- `src/components/layout/sidebar.tsx:35-43` — the seven nav entries have no site selector.
|
|||
|
|
- `src/app/dashboard/page.tsx:191-193` — the Overview tile shows `data.sites[0]?.name ?? "No sites yet"` as the sub-label, which is a misleading and never-correct value when there are multiple sites.
|
|||
|
|
- `src/app/dashboard/sites/page.tsx:117` — the Sites list page lets the user see all sites side by side (this is the only place "see all sites" exists).
|
|||
|
|
- `src/app/dashboard/sites/[id]/page.tsx` — per-site drill-down, with a back button.
|
|||
|
|
- `src/app/dashboard/fuel/page.tsx:159-165` — passes a `projectCodes: string[]` array to the client view for client-side filtering. Same in `fuel-view.tsx`.
|
|||
|
|
- `src/app/dashboard/employees/page.tsx:159-162` — `projectCodes` and `positions` passed for client-side filter.
|
|||
|
|
- `src/app/dashboard/plant/page.tsx:145` — `projectCodes` passed for client-side filter.
|
|||
|
|
- `src/app/dashboard/purchasing/page.tsx:188` — `projectCodes: Array.from(new Set(rows.map((r) => r.projectCode))).sort()` passed for client-side filter.
|
|||
|
|
|
|||
|
|
**The cost to a user who works on one site.** There is no way to say
|
|||
|
|
"my site is The Hills" once and have every page filter to it. Each
|
|||
|
|
page requires the user to:
|
|||
|
|
- Open the page.
|
|||
|
|
- Find the project-code filter (a dropdown, varies per page).
|
|||
|
|
- Pick their site.
|
|||
|
|
- Wait for the client-side filter to apply.
|
|||
|
|
|
|||
|
|
**Pages that offer no site filter at all.** The Overview page
|
|||
|
|
(`src/app/dashboard/page.tsx`) does not accept a site filter — the
|
|||
|
|
KPI tiles are portfolio-wide. The **Site** detail page
|
|||
|
|
(`src/app/dashboard/sites/[id]/page.tsx`) is already per-site by URL,
|
|||
|
|
so the question is moot there. The other five pages do have the
|
|||
|
|
client-side filter.
|
|||
|
|
|
|||
|
|
**Threshold.** This is a UX problem, not a correctness one — it
|
|||
|
|
doesn't break the data, but at 5+ sites it costs the operator
|
|||
|
|
non-trivial clicks. At 20+ sites it becomes a daily frustration.
|
|||
|
|
|
|||
|
|
### 4. Site resolution by name string — **CONFIRMED with caveats**
|
|||
|
|
|
|||
|
|
**Lead claim:** "Importers match incoming rows to sites by
|
|||
|
|
case-insensitive `sites.name` comparison."
|
|||
|
|
|
|||
|
|
**Evidence.**
|
|||
|
|
|
|||
|
|
- `src/app/api/fuel-logs/import/route.ts:217-224` — `siteByName` keyed by `s.name.trim().toLowerCase()`.
|
|||
|
|
- `src/app/api/plant-hours/import/route.ts:237-250` — same pattern in the orchestrator's `sites` Map.
|
|||
|
|
- `src/lib/purchase-order-schema.ts` — `matchSite` is the canonical version (also used by the chat's free-text `requireSite` indirectly). It does exact `project_code`, then PB's `~` (case-insensitive substring) on `project_code`. Site-by-name is **not** used in PO ingest.
|
|||
|
|
- `src/lib/agent/resolve.ts:21-48` — the chat-side `resolveSite` does id → project code → name → substring on name or code.
|
|||
|
|
|
|||
|
|
**Failure modes as site count grows.**
|
|||
|
|
|
|||
|
|
| Failure mode | What happens | Mitigated? |
|
|||
|
|
| --- | --- | --- |
|
|||
|
|
| Near-identical names ("The Hills" vs "The Hills Extension") | First hit wins; case-insensitive exact match. The importer's `Map.set` for `"the hills"` overwrites any earlier hit. **Last-written wins**, not first. | Partially — operator must rename sites to be unique or change the source row's label. |
|
|||
|
|
| Renamed site | New `sites.name` value; old fuel log rows still carry the old `project_code` from the row. Re-imports of those rows would resolve to nothing or to the wrong site if a new site has the same name. | The 64-char ceiling and unique `project_code` index help, but they protect `project_code`, not `name`. |
|
|||
|
|
| Whitespace / casing drift | Trimmed + lowercased before comparison. Tolerates `"The Hills"`, `"the hills"`, `" The Hills "`. | Yes. |
|
|||
|
|
| Two sites sharing a name | Last-written wins in the importer's `Map`. The other site's rows resolve to whatever the most recent PB row with that name was. | No. |
|
|||
|
|
| Source spreadsheet using a different label | Row is dropped, not orphaned — the importer's "no site" path leaves the `site` relation empty and falls back to `vehicle.projectCode` for the text. The fuel log exists but isn't tied to a site by relation. | The fallback is partial — the row survives but is ungroupable in JS-side filtering by `site` id. |
|
|||
|
|
|
|||
|
|
**Is `project_code` a more reliable key?** Yes — `project_code` has
|
|||
|
|
the unique index and cannot be changed after creation
|
|||
|
|
(`src/app/api/sites/[id]/route.ts:23-28`). The PO ingest already uses
|
|||
|
|
`project_code` first. The fuel and plant importers still use `name`.
|
|||
|
|
|
|||
|
|
**What happens to a row that matches nothing.** Fuel: relation empty,
|
|||
|
|
`project_code` falls back to `vehicle.projectCode`. Plant: similar (the
|
|||
|
|
plant orchestrator requires the vehicle to resolve, so unmatched means
|
|||
|
|
the row errored out). Timesheet auto-create: requires `siteId` to be
|
|||
|
|
passed as a form field; no fallback.
|
|||
|
|
|
|||
|
|
**Threshold.** Silent wrong attribution starts at the **second site
|
|||
|
|
with a near-identical name**. Today that means 5-10 sites. The
|
|||
|
|
remediation is to make every importer resolve by `project_code` (or
|
|||
|
|
fall back to name only when the project code is absent on the source
|
|||
|
|
row) — and to surface "unmatched site" as a hard error, not a
|
|||
|
|
silently-empty relation.
|
|||
|
|
|
|||
|
|
### 5. Vehicles, plant and employees moving between sites — **CONFIRMED, with the discipline gap highlighted**
|
|||
|
|
|
|||
|
|
**Lead claim:** "A vehicle or worker is bound to one `site` at a time.
|
|||
|
|
Determine what happens to historical records when that assignment
|
|||
|
|
changes."
|
|||
|
|
|
|||
|
|
**Evidence.**
|
|||
|
|
|
|||
|
|
- `src/lib/pb-types.ts:24-35` — `EmployeeRecord.site` is a single relation. `src/lib/pb-types.ts:39-56` — same for `VehicleRecord.site`.
|
|||
|
|
- `src/lib/metrics/site-spend.ts:71-84` — payroll burn is computed by joining timesheets through `employeeRates` (`map.get(t.employee).site`). **Moving a worker to a new site silently reassigns all their past payroll burn to the new site** the next time a per-site report runs.
|
|||
|
|
- `src/lib/agent/load.ts:32-33` — `loadSiteBundle` fetches fixed_costs/POs/fuel/income per-site, but timesheets globally. Same problem: per-site queries walk past timesheets via `employeeRates`.
|
|||
|
|
- `src/lib/metrics/site-spend.ts:67-99` — the function-level contract: payroll goes to whatever `employee.site` is *at read time*. There is no snapshot on the timesheet.
|
|||
|
|
- `src/lib/metrics/fuel-attention.ts:73-127` — fuel attention is computed from the vehicle's *current* `site` (via the `attentionVehicle.site` field at read time). Same drift.
|
|||
|
|
|
|||
|
|
**The `plant_hours_log.vehicle_unit_type_at_import` discipline.**
|
|||
|
|
Confirmed: `scripts/pb-bootstrap-schema.py:251-252` and
|
|||
|
|
`src/lib/pb-types.ts:209` show that `plant_hours_log` carries a
|
|||
|
|
snapshot of the vehicle's `unit_type` at the moment of import. The
|
|||
|
|
import code (`src/lib/plant-hours-import.ts`) is responsible for
|
|||
|
|
writing it. This is exactly the right discipline — it means a
|
|||
|
|
vehicle reclassified from "plant" to "fuel" six months ago still has
|
|||
|
|
its old logs visible on the Plant page.
|
|||
|
|
|
|||
|
|
**That discipline is missing for `site`.** The `plant_hours_log.site`
|
|||
|
|
field exists and is required, but it is set from `vehicle.site` at
|
|||
|
|
import time. After a vehicle moves, the row keeps the *old* site
|
|||
|
|
relation (good), but new imports for the same vehicle will write
|
|||
|
|
*new* site values. So historical `plant_hours_log` rows do
|
|||
|
|
**not** silently rewrite — they snapshot correctly.
|
|||
|
|
|
|||
|
|
The risk is in **`fuel_logs` and `timesheets`**:
|
|||
|
|
- `fuel_logs` (`src/app/api/fuel-logs/import/route.ts:298-310`) — `site` and `project_code` come from the source row (the site column or the vehicle's project_code). If the source spreadsheet is updated, the row may be re-attributed. The row's `site` is also the one used in `groupAttentionBySite` (read time) — so a moved vehicle's old logs appear under the new site in the attention roll-up, because the join is `log.site` not `vehicle.site`. **Wait, re-checking: `src/lib/metrics/fuel-attention.ts:97-127` joins on `log.vehicle` and looks at the *vehicle's* current site (line 95 via `vehicleById.get(vehicleId)?.fuel_limits`). The attention roll-up groups by `log.site` in `groupAttentionBySite`. So a moved vehicle's old fuel logs DO drift to the new site in the rail — the row's `site` is what gets grouped, not the vehicle's current site. Confirmed in `src/lib/metrics/fuel-attention.ts:118-150` where `vehicleById` is used only for limits/description; the bucket is `log.site`.
|
|||
|
|
- `timesheets` — confirmed above: payroll burn follows the employee's *current* `site` (`src/lib/metrics/site-spend.ts:73-74`).
|
|||
|
|
|
|||
|
|
**The composite `(vehicle_qr, start_date)` index.** Confirmed at
|
|||
|
|
`scripts/pb-bootstrap-schema.py:257` and the route's duplicate-error
|
|||
|
|
handling at `src/app/api/plant-hours/import/route.ts:95-114`. The
|
|||
|
|
index has no `site` in the key, so a vehicle that legitimately moves
|
|||
|
|
between sites (with a different `start_date` for each site) will
|
|||
|
|
not collide. The risk the prompt flagged — a vehicle that goes
|
|||
|
|
"Hills → Pitsana → Hills" within the same week — would collide on
|
|||
|
|
the second Hills entry. Realistic: rare, but possible.
|
|||
|
|
|
|||
|
|
**What the user sees.**
|
|||
|
|
|
|||
|
|
- Workforce page: an employee who transferred from The Hills to
|
|||
|
|
Cosmo City last month has all their historical hours showing
|
|||
|
|
under Cosmo City now. Their 2025 P&L attributed to The Hills
|
|||
|
|
*no longer exists anywhere*. There is no per-timesheet
|
|||
|
|
`site_at_log_time` field to recover it from.
|
|||
|
|
- Fuel attention: a vehicle moved from The Hills to West Park
|
|||
|
|
two months ago will have its flagged logs attributed to West
|
|||
|
|
Park in the current Overview rail. Old reports are wrong
|
|||
|
|
retroactively.
|
|||
|
|
|
|||
|
|
**Threshold.** The first move of an employee or vehicle between
|
|||
|
|
sites. The team can mitigate by re-running historical reports
|
|||
|
|
before any move and accepting that post-move history is lost.
|
|||
|
|
|
|||
|
|
### 6. Access control per site — **CONFIRMED, and the situation is bleak**
|
|||
|
|
|
|||
|
|
**Lead claim:** "With a single shared PIN and no user records, no user
|
|||
|
|
can be restricted to their own site — everyone who knows the PIN sees
|
|||
|
|
every site's contract values, margins and payroll."
|
|||
|
|
|
|||
|
|
**Evidence.**
|
|||
|
|
|
|||
|
|
- `src/hooks/use-auth.tsx:20` — `const expectedPin = process.env.NEXT_PUBLIC_APP_PIN || "2468";` — the PIN is a single shared string baked into the client bundle.
|
|||
|
|
- `src/hooks/use-auth.tsx:23-34` — verification is a `sessionStorage` flag, set by the client, with no server check.
|
|||
|
|
- `src/app/api/chat/route.ts:69-71` — the chat returns 404 if `ENABLE_CHAT` is unset, but has **no user authentication**. Anyone reaching the route can ask any question.
|
|||
|
|
- `src/lib/pb.ts:74-111` — `pbAdmin()` is a module-singleton superuser. Every server component and route handler uses it.
|
|||
|
|
- `scripts/pb-relax-rules.py:23-34` — the API rules allow any *authenticated* user to read every collection, but the server-side `pbAdmin()` bypasses these rules anyway.
|
|||
|
|
|
|||
|
|
**Is per-site access control achievable without introducing real user
|
|||
|
|
accounts?** Not on the current server. The server-side data path
|
|||
|
|
runs as superuser, so the only way to enforce "user X can only see
|
|||
|
|
site Y" is to (a) introduce a real user model, (b) make every server
|
|||
|
|
read filter by the requesting user's allowed sites, and (c) add a
|
|||
|
|
PocketBase auth layer in front of `pbAdmin()`. That's the same work
|
|||
|
|
as introducing real authentication.
|
|||
|
|
|
|||
|
|
A shortcut exists: a per-PIN site list (e.g. PIN A unlocks
|
|||
|
|
`{Hills, Cosmo}`, PIN B unlocks `{Castle Gate}`). The PIN gate is
|
|||
|
|
client-side today, but moving it server-side and issuing a server-set
|
|||
|
|
session cookie that carries the site list is a ~half-day change.
|
|||
|
|
That gets you a defence-in-depth "PITM and shoulder-surfing won't
|
|||
|
|
leak financials" but **not** per-user audit trails or per-user
|
|||
|
|
revocation.
|
|||
|
|
|
|||
|
|
**What the user sees.** Today: every operator sees every site's
|
|||
|
|
contract values, margins, payroll rates. The chat — when enabled —
|
|||
|
|
returns the same.
|
|||
|
|
|
|||
|
|
**Threshold.** The first time someone outside the company tries to
|
|||
|
|
log in, or the first time a contractor is given the PIN and the
|
|||
|
|
operator wants to limit them to one site.
|
|||
|
|
|
|||
|
|
### 7. The chat / agent layer — **CONFIRMED**
|
|||
|
|
|
|||
|
|
**Lead claim:** "Tools in `src/lib/agent/tools.ts` load portfolio-wide
|
|||
|
|
bundles."
|
|||
|
|
|
|||
|
|
**Evidence.**
|
|||
|
|
|
|||
|
|
- `src/lib/agent/load.ts:45-66` — `loadAllSitesBundle` literally loads every site, every fixed cost, every timesheet, every employee (projected fields), every PO, every fuel log, every income claim. This is what `list_sites` and `site_financials` use as a starting point.
|
|||
|
|
- `src/lib/agent/tools.ts:76-92` — `requireSite` resolves a free-text site query and returns ambiguity, but once it has a single site it uses `loadSiteBundle` (lines 27-42) which still pulls all timesheets + all employees globally.
|
|||
|
|
- `src/lib/agent/tools.ts:125-150` — `list_sites` filters in JS over the global bundle (line 132-142). No server-side filter.
|
|||
|
|
- `src/lib/chat/system-prompt.ts:17-29` — the system prompt's load-bearing rule: the model **never** does arithmetic. It must report values exactly as the tools return them.
|
|||
|
|
|
|||
|
|
**Cost and latency growth.** Every call to `list_sites` does 7
|
|||
|
|
parallel `getFullList` calls plus a per-site JS loop. Latency is
|
|||
|
|
dominated by the `timesheets` and `fuel_logs` fetches. At 10 sites ×
|
|||
|
|
12 months this is ~2-3s; at 50 sites × 24 months it is ~30-60s,
|
|||
|
|
which is well beyond interactive.
|
|||
|
|
|
|||
|
|
**Ambiguity handling.** `src/lib/agent/resolve.ts:21-48` returns
|
|||
|
|
`{ kind: "ambiguous", matches: [...] }` when more than one site
|
|||
|
|
matches. `requireSite` then formats that as an error string the model
|
|||
|
|
relays verbatim. The prompt's concern ("whether the agent can leak
|
|||
|
|
one site's financials to a user who should only see another's") is
|
|||
|
|
**not** a code bug — the model doesn't switch tools based on user
|
|||
|
|
intent, it switches based on the model's own choice. With the
|
|||
|
|
current PIN-only auth, any operator can ask "what's The Hills' burn"
|
|||
|
|
and get an answer; there's no enforcement of "you can only see
|
|||
|
|
West Park." The risk is structural (no auth), not in the agent
|
|||
|
|
code.
|
|||
|
|
|
|||
|
|
**Is the `ENABLE_CHAT` warning still accurate?** Yes, exactly. The
|
|||
|
|
warning at `.env.local.example:46-50` says: "MUST stay unset in
|
|||
|
|
production until real authentication exists — the chat reads every
|
|||
|
|
site's financials and spends LLM credits for anyone who can reach
|
|||
|
|
it." The code at `src/app/api/chat/route.ts:69-71` and
|
|||
|
|
`src/app/dashboard/chat/page.tsx:31` is still gated only by this env
|
|||
|
|
flag. There is **no** user authentication in front of either. The
|
|||
|
|
warning is correct.
|
|||
|
|
|
|||
|
|
**Threshold.** When LLM cost-per-call exceeds the value of the
|
|||
|
|
operator's time saved, or when the first unauthorised user reaches
|
|||
|
|
the chat. Today the gate is honoured; the moment someone sets
|
|||
|
|
`ENABLE_CHAT=true` in production without first adding real auth, the
|
|||
|
|
warning is moot.
|
|||
|
|
|
|||
|
|
### 8. Anything else you find — **FOUND 7 additional things**
|
|||
|
|
|
|||
|
|
The prompt's open invitation. Capped at 5 as suggested, plus 2 bonus
|
|||
|
|
items I think matter more than anything else here.
|
|||
|
|
|
|||
|
|
#### 8.1. `plant_hours_log` requires `site` to be set, but the **importer** doesn't enforce it — **TIER 1**
|
|||
|
|
|
|||
|
|
`scripts/pb-bootstrap-schema.py:235-236` declares
|
|||
|
|
`field("site", "relation", required=True, ..., minSelect=1)`. The
|
|||
|
|
importer at `src/lib/plant-hours-import.ts` is what calls
|
|||
|
|
`pb.collection("plant_hours_log").create(prepRow.row)`. If the
|
|||
|
|
vehicle's `site` is empty (a real condition for newly-added vehicles
|
|||
|
|
with no site assignment yet), the insert fails with a 400 — but the
|
|||
|
|
route surfaces the failure per-row and the operator is left with a
|
|||
|
|
"duplicate" or "validation" error that doesn't make the cause clear.
|
|||
|
|
|
|||
|
|
**Why this matters.** A new vehicle added to PB without a `site` will
|
|||
|
|
have its first plant hours import fail, and the operator may not
|
|||
|
|
realise they need to assign the vehicle to a site first. The
|
|||
|
|
dashboard-side `AddPlantButton` (`src/components/add-plant-button.tsx`)
|
|||
|
|
*does* require `site` on create, so this is mostly a "data migration
|
|||
|
|
or operator-directly-adds-to-PB" path.
|
|||
|
|
|
|||
|
|
#### 8.2. The `process-batch` test failure (404 vs 500) is a real bug, just masked by a bad mock — **TIER 2**
|
|||
|
|
|
|||
|
|
`src/app/api/purchase-orders/process-batch/route.test.ts:168` expects
|
|||
|
|
404 when the batch doesn't exist; the production code at
|
|||
|
|
`src/app/api/purchase-orders/process-batch/route.ts:137` checks
|
|||
|
|
`(err as { status?: number }).status === 404`. The PB SDK does set
|
|||
|
|
`status: 404` on missing-record errors, so production works. The
|
|||
|
|
test mocks `getOne` to throw a plain `new Error("404 Not Found")`
|
|||
|
|
with no `status` field, so the production code falls through to
|
|||
|
|
the 500 branch.
|
|||
|
|
|
|||
|
|
**Why this matters.** When the test was written, the assumption
|
|||
|
|
"the SDK error has `.status: 404`" was correct. If a future PB
|
|||
|
|
version changes that, or if the route gets refactored to a
|
|||
|
|
non-SDK error path, the test will silently start passing the
|
|||
|
|
wrong response code. Worth either fixing the mock to set
|
|||
|
|
`{ status: 404 }` or making the production code fall back to
|
|||
|
|
matching `err.message` for a `404` substring.
|
|||
|
|
|
|||
|
|
#### 8.3. The `purchase_order_items.qty_outstanding` field is denormalised but the dashboard uses `received_items` directly — **TIER 3**
|
|||
|
|
|
|||
|
|
`src/lib/pb-types.ts:146` declares `qty_outstanding` on PO items,
|
|||
|
|
and `src/lib/purchase-order-schema.ts` and the ingest route
|
|||
|
|
(`src/app/api/purchase-orders/ingest/route.ts:135`) populate it
|
|||
|
|
with the ordered qty at creation. The dashboard
|
|||
|
|
(`src/app/dashboard/purchasing/page.tsx:54-67` and onward)
|
|||
|
|
**does not use it** — it joins through `received_items` and
|
|||
|
|
clamps to `min(received, ordered)`. The
|
|||
|
|
`src/app/api/purchase-orders/by-number/[poNumber]/items/route.ts:127-128`
|
|||
|
|
also uses `qty` + `qty_outstanding` only for display, not for
|
|||
|
|
fulfilment.
|
|||
|
|
|
|||
|
|
**Why this matters.** `qty_outstanding` is a denormalised cache
|
|||
|
|
that nobody reads. The OCR/DN update path *does not* decrement it.
|
|||
|
|
It's a dead column. Either remove it (and the corresponding
|
|||
|
|
fulfilment-by-denormalised fallback that doesn't exist) or wire
|
|||
|
|
the `received_items` create path to decrement it under a
|
|||
|
|
transaction. Today it's a tripwire for the next person who tries
|
|||
|
|
to use it.
|
|||
|
|
|
|||
|
|
#### 8.4. `attention_dismissed_at` is a one-way switch with no UI to undo — **TIER 3**
|
|||
|
|
|
|||
|
|
`src/app/api/vehicles/[id]/attention-dismiss/route.ts` sets the
|
|||
|
|
timestamp on dismiss. There's no `PATCH /api/vehicles/[id]/attention-undismiss`
|
|||
|
|
or admin UI to clear it. A vehicle that re-flags after being
|
|||
|
|
dismissed (because new fuel logs come in) will *not* reappear in
|
|||
|
|
the "Attention required" rail because the filter is
|
|||
|
|
`excludeAcknowledged: true` (`src/app/dashboard/page.tsx:131-133`).
|
|||
|
|
The dismissal check uses `isAlertAcknowledged` which compares the
|
|||
|
|
dismiss timestamp against the latest flagged log's date; see
|
|||
|
|
`src/lib/attention-acknowledged.ts`. So a flag that arrives
|
|||
|
|
*after* the dismissal will re-show. But a flag that *re-occurs*
|
|||
|
|
on the same date as the dismissal will stay hidden.
|
|||
|
|
|
|||
|
|
**Why this matters.** UX glitch, not a data correctness issue.
|
|||
|
|
Operators will hit it the first time a vehicle re-flags on the
|
|||
|
|
same day they dismissed an old one.
|
|||
|
|
|
|||
|
|
#### 8.5. No server-side filter on `list_sites` / `list_purchase_orders` / etc. — **TIER 2**
|
|||
|
|
|
|||
|
|
The chat tool layer's `list_*` tools take no `siteId` argument
|
|||
|
|
(and the schemas in `src/lib/agent/tools.ts` have no `siteId`).
|
|||
|
|
`loadPurchaseOrders` (`src/lib/agent/load.ts:125-142`) does accept
|
|||
|
|
a `siteId` arg, but the calling tool `list_purchase_orders`
|
|||
|
|
doesn't surface it. So "what are the POs for The Hills" forces a
|
|||
|
|
full-table load even though the per-site query would be a
|
|||
|
|
two-character filter change. Same for `loadPlantHours`,
|
|||
|
|
`loadFixedCosts`, `loadIncomeClaims`, `loadEmployees`, and
|
|||
|
|
`loadImportBatches` — all accept `siteId`; none of the
|
|||
|
|
corresponding `list_*` tools expose it.
|
|||
|
|
|
|||
|
|
**Why this matters.** A small change (add `siteId` to the tool
|
|||
|
|
schemas, pass it through) would slash chat latency for
|
|||
|
|
per-site questions at scale. Low cost, high payoff.
|
|||
|
|
|
|||
|
|
#### 8.6. (Bonus) `NEXT_PUBLIC_POCKETBASE_URL` is read by `pbAdmin()` server-side too — **TIER 3**
|
|||
|
|
|
|||
|
|
`src/lib/pb.ts:18` and `src/lib/pb.ts:28` both derive the PB URL
|
|||
|
|
from `NEXT_PUBLIC_POCKETBASE_URL`. The `PB_BASE_URL` export on
|
|||
|
|
line 27-28 is a re-export of the same thing. The server's
|
|||
|
|
`pbAdmin()` uses this URL to reach PB. If the URL needs to be
|
|||
|
|
different on the server vs. the client (e.g. an internal
|
|||
|
|
`http://pb:8090` for the container, an external
|
|||
|
|
`https://pb.example.com` for the browser), the current code can't
|
|||
|
|
express that. A `POCKETBASE_INTERNAL_URL` (server-only) +
|
|||
|
|
`POCKETBASE_FILE_URL` (server-only, for file proxies) would
|
|||
|
|
unblock several deploy topologies that the current single-variable
|
|||
|
|
design blocks.
|
|||
|
|
|
|||
|
|
#### 8.7. (Bonus) `ENABLE_CHAT` has no per-environment override — **TIER 3**
|
|||
|
|
|
|||
|
|
The flag is one env var read at request time
|
|||
|
|
(`src/lib/chat/index.ts:13-15`). There's no way to say "chat is
|
|||
|
|
on in staging, off in production, on for an internal demo behind
|
|||
|
|
a feature flag" without redeploying. A `CHAT_ALLOWED_HOSTS` list
|
|||
|
|
(env-driven hostname allowlist) would be a 10-line change that
|
|||
|
|
makes the chat safely demoable on a per-deploy basis.
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## Part C — What to change
|
|||
|
|
|
|||
|
|
Three tiers, in order of "do this first":
|
|||
|
|
|
|||
|
|
### Tier 1 — Correctness (must fix; produces wrong numbers today)
|
|||
|
|
|
|||
|
|
These are bugs that *silently* mis-attribute or *silently* drop data.
|
|||
|
|
Not optional.
|
|||
|
|
|
|||
|
|
| # | Problem | Proposed change | Files | Effort | Risk of doing | Risk of not doing |
|
|||
|
|
| --- | --- | --- | --- | --- | --- | --- |
|
|||
|
|
| 1.1 | The Overview page caps sites at 50 | Replace `getList(1, 50)` with `getFullList({ sort: "name" })` on `src/app/dashboard/page.tsx:64`. The `sites.items` references that follow (lines 113, 157, 192) need to become `sites` (the array). | `src/app/dashboard/page.tsx` | **S** | Trivial — no shape change. | First site added beyond 50 silently vanishes from the KPI tile and attention rail. |
|
|||
|
|
| 1.2 | The Overview's attention rail groups by `log.site`, which silently rewrites history when a vehicle moves | Either snapshot the site on the fuel log row (the `plant_hours_log` discipline) and add a `site_at_log_time` field with a one-time backfill, or document the drift as known. | `src/lib/metrics/fuel-attention.ts`, possibly a new column on `fuel_logs` + a migration | **L** (column add + backfill) | Backfill has to be done correctly or it double-counts. | Reports for any site that has had a vehicle moved to/from it are wrong retroactively. |
|
|||
|
|
| 1.3 | The "needs review" panel on the purchasing page caps at 200 | Replace `getList(1, 200)` with `getFullList({ filter, sort: "-id" })`. The `items` field on the response needs to become the array directly (line 145-148 already checks for `.items`). | `src/app/dashboard/purchasing/page.tsx:22-25` | **S** | The 200th+ unverified DN stops being shown in the inbox. | The 201st DN needs manual PB admin recovery. |
|
|||
|
|
|
|||
|
|
**Other Tier 1 items I considered and rejected.** The "site resolution
|
|||
|
|
by name" issue (4) is high-impact but Tier 2 because the operator can
|
|||
|
|
fix it by renaming the conflicting sites — it's a workflow fix more
|
|||
|
|
than a code fix. The `process-batch` 404/500 (8.2) is a test bug, not
|
|||
|
|
a production bug — Tier 2. The `qty_outstanding` dead column (8.3)
|
|||
|
|
is Tier 3 because nobody reads it today; if/when someone adds a
|
|||
|
|
fulfilment-by-denormalised fallback, it'll become Tier 1.
|
|||
|
|
|
|||
|
|
### Tier 2 — Scale (do these before site count or data volume grows further)
|
|||
|
|
|
|||
|
|
These are the changes that turn "works at 7 sites" into "works at 50".
|
|||
|
|
The most expensive ones are query patterns, not UI.
|
|||
|
|
|
|||
|
|
| # | Problem | Proposed change | Files | Effort | Risk of doing | Risk of not doing |
|
|||
|
|
| --- | --- | --- | --- | --- | --- | --- |
|
|||
|
|
| 2.1 | Every dashboard page does `getFullList()` on large collections and filters in JS | Add per-collection `filter: site='<id>'` server-side for the per-site pages, and add `date_logged >= '<cutoff>'` for the long-running ones. The plant-hours `perPage: 5000` calls become unbounded. | `src/app/dashboard/sites/[id]/page.tsx:48-57`, `src/app/dashboard/employees/page.tsx:103-105`, `src/app/dashboard/fuel/page.tsx:20-25`, `src/lib/agent/load.ts:32-66` | **M** | Easy to introduce a wrong filter that drops rows; needs good tests. | Sites list page becomes unusable at ~2M timesheet rows (today's level: ~50-100k, so ~1 year out at current ingest rate). |
|
|||
|
|
| 2.2 | The agent's `list_*` tools don't accept `siteId` even though the loaders do | Surface `siteId` in the tool schemas for `list_purchase_orders`, `list_delivery_notes`, `list_employees`, `list_fixed_costs`, `list_income_claims`, `list_plant_hours`, `list_import_batches`. Pass through. | `src/lib/agent/tools.ts` (~7 tool defs) | **S** | Tool schemas change — any custom client using the MCP server has to handle the new field. | Every portfolio-wide chat question scans everything, even when the user only cares about one site. |
|
|||
|
|
| 2.3 | PB indexes are missing on the columns that get filtered on | Add indexes on: `fuel_logs(site, date_logged)`, `fuel_logs(vehicle, date_logged)`, `fixed_costs(site, date_logged)`, `timesheets(employee, date_logged)`, `purchase_orders(site, required_by_date)`, `delivery_notes(site, date_received)`, `delivery_notes(processing_status)`, `income_claims(site, period_end)`, `plant_hours_log(vehicle, start_date)`. Re-run `pb-bootstrap-schema.py` + write a `migrate-2026-09-add-fleet-indexes.py` migration. | `scripts/pb-bootstrap-schema.py`, new migration file | **M** | Index adds are non-blocking on SQLite (PB default) but can be slow on large tables. Run during off-hours. | All per-site queries full-table-scan today. At 50 sites × 24 months of fuel logs, the fuel page would do a 600k-row scan on every render. |
|
|||
|
|
| 2.4 | The plant page has `perPage: 5000` caps | Replace with `getFullList({ ... filter, sort })` — the implicit cap of 5000 is the next silent-truncation bug after sites/50. | `src/app/dashboard/plant/page.tsx:19-32` | **S** | None. | The 5001st plant item disappears. |
|
|||
|
|
| 2.5 | The plant hours import has `perPage: 5000` caps on vehicle + site lookups | Replace with `getFullList({ fields: "..." })` (no perPage). | `src/app/api/plant-hours/import/route.ts:216, 238` | **S** | None. | An import file that names a vehicle beyond row 5000 fails the whole batch. |
|
|||
|
|
| 2.6 | The `/api/purchase-orders/by-number/[poNumber]/items` Levenshtein fallback caps at 500 POs | Replace with `getFullList` (no perPage). | `src/app/api/purchase-orders/by-number/[poNumber]/items/route.ts:81` | **S** | The Levenshtein pass becomes O(N) over all POs — at 50 000 POs that's still < 1s in Node, fine. | A user looking up a recently-added PO gets a wrong/no match. |
|
|||
|
|
| 2.7 | The OCR result webhook's normalised/Levenshtein PO match caps at 200 POs | Replace with `getFullList` (no perPage). | `src/app/api/delivery-notes/[id]/ocr-result/route.ts:151` | **S** | Same as 2.6. | A delivery note scanned against a PO beyond row 200 fails to match. |
|
|||
|
|
| 2.8 | The process-batch test mock doesn't match the production error shape | Either change the mock to `{ status: 404 }` or add a `parseInt(err.message.match(/404/)?.[0] ?? "0")` fallback to the production code. | `src/app/api/purchase-orders/process-batch/route.test.ts`, `route.ts:137` | **S** | None. | If PB SDK ever stops setting `.status`, the route returns 500 instead of 404 and the test goes silently green. |
|
|||
|
|
|
|||
|
|
### Tier 3 — Product (do when the team has time, not blocking)
|
|||
|
|
|
|||
|
|
These are features, not bugs. The team should build them when there's
|
|||
|
|
a clear user need, not prophylactically.
|
|||
|
|
|
|||
|
|
| # | Problem | Proposed change | Files | Effort | Risk of doing | Risk of not doing |
|
|||
|
|
| --- | --- | --- | --- | --- | --- | --- |
|
|||
|
|
| 3.1 | No global site selector in the sidebar | Add a `?site=<id>` query param (or a context) that every page reads at the top. Pages that take per-site filters can use it. Pages that are portfolio-wide (Overview) can show a "filter to my site" CTA. | `src/components/layout/sidebar.tsx`, plus the seven page bodies | **M** | Users with multiple sites lose the portfolio view by default — needs a "show all" toggle. | Daily frustration as site count grows. |
|
|||
|
|
| 3.2 | Per-site access control with real users | Introduce a `users` collection on PB, replace the PIN gate with a server-validated auth check, attach the user's allowed site list to a session cookie, and have every server read filter by it. This is the same work as adding real authentication. | Many. `src/hooks/use-auth.tsx`, `src/components/auth-gate.tsx`, every server component, `src/lib/pb.ts` | **L** | A half-done auth layer is worse than no auth layer — it gives false confidence. | The current "everyone sees everything" model is fine for a single team but blocks working with external contractors. |
|
|||
|
|
| 3.3 | A PIN-gated "site allowlist" shortcut | Move the PIN gate server-side, look up `PINS_TO_SITES` from env, and set a server-validated session cookie that carries the allowlist. Reads filter by it. Doesn't introduce users but does give the operator a way to limit what a contractor sees. | `src/middleware.ts` (new), `src/hooks/use-auth.tsx`, every page's `loadX` function | **M** | A misconfigured PIN allowlist is a security hole. | Same as 3.2 but at a smaller scope. |
|
|||
|
|
| 3.4 | Site archiving for completed contracts | Add an `archived_at` field on `sites` (similar to `attention_dismissed_at`). Hide archived sites from the Sites list and Overview by default; add a "show archived" toggle. | `src/lib/pb-types.ts`, new migration, `src/app/dashboard/sites/page.tsx`, `src/app/dashboard/page.tsx` | **M** | Old links to archived sites need to keep working (server-rendered "site archived" page). | The list page becomes harder to scan as completed contracts accumulate. |
|
|||
|
|
| 3.5 | Point-in-time site attribution for moved assets | Add `site_at_log_time` to `fuel_logs` and `timesheets`. Backfill from current `vehicle.site` / `employee.site` for existing rows. New writes set it from the source's site. `computeSiteSpend` and `groupAttentionBySite` use the snapshot field instead of the join. | Many. Schema, backfill migration, every metrics function that walks timesheets or fuel logs | **L** | Backfill correctness is everything; a botched backfill would re-rewrite history. | A moved employee's historical payroll burn is lost forever; a moved vehicle's old fuel flags drift to the new site. |
|
|||
|
|
| 3.6 | Remove the dead `qty_outstanding` denormalisation, or wire it correctly | Either drop the column, or wrap the `received_items.create` flow in a PocketBase transaction that decrements `qty_outstanding`. | `src/lib/pb-types.ts`, `src/app/api/delivery-notes/[id]/ocr-result/route.ts`, the manual `/api/delivery-notes/[id]/assign-po` route | **S** (drop) or **M** (wire) | Dropping it is fine — nobody reads it. Wiring it requires care so the decrement survives PB's no-real-transactions model. | Future developer trusts the denormalised value, gets wrong numbers. |
|
|||
|
|
| 3.7 | `NEXT_PUBLIC_POCKETBASE_URL` is one variable for both client and server | Add a separate `POCKETBASE_INTERNAL_URL` (server-only) and let `pbAdmin()` use that for in-cluster calls while the client uses the public URL. Add a `POCKETBASE_FILE_URL` for file proxies that need to be publicly resolvable. | `src/lib/pb.ts` | **S** | None. | The current single-variable design blocks several deploy topologies. |
|
|||
|
|
| 3.8 | `ENABLE_CHAT` is binary | Add `CHAT_ALLOWED_HOSTS` (comma-separated hostnames) and `CHAT_ALLOWED_EMAILS` (PB users, if/when 3.2 is done). The chat route returns 404 unless the request's `Host` header is in the allowlist. | `src/app/api/chat/route.ts`, `src/lib/chat/index.ts` | **S** | None. | The chat is either fully on or fully off; no per-environment safe toggle. |
|
|||
|
|
| 3.9 | Re-show dismissed vehicles when a new flag arrives | Extend `isAlertAcknowledged` (`src/lib/attention-acknowledged.ts`) to compare the dismiss timestamp against the *latest flagged log's* date. A new flag post-dismiss re-shows. | `src/lib/attention-acknowledged.ts`, `src/lib/attention-acknowledged.test.ts` | **S** | None. | UX glitch, not data correctness. |
|
|||
|
|
| 3.10 | The Overview "Sites" tile shows `data.sites[0]?.name` as a sub-label | Remove the sub-label (it was never meaningful) or replace with "oldest: X" / "newest: Y" of the sorted list. | `src/app/dashboard/page.tsx:191-193` | **S** | None. | Confusing. |
|
|||
|
|
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
## Bottom line — in plain English
|
|||
|
|
|
|||
|
|
**Can this app run many sites today? Yes, up to about 10.** The data
|
|||
|
|
model is site-aware, the per-site math is right, the importers do the
|
|||
|
|
right thing on the happy path, and the chat answers portfolio questions
|
|||
|
|
correctly. **At 20+ sites, the Overview page starts hiding sites and
|
|||
|
|
the Sites list page starts getting slow.** At 50+ sites, both are
|
|||
|
|
unusable.
|
|||
|
|
|
|||
|
|
**The single most important thing to fix first is the 50-site cap on
|
|||
|
|
the Overview page.** It's a one-line change
|
|||
|
|
(`getList(1, 50)` → `getFullList({ sort: "name" })` on
|
|||
|
|
`src/app/dashboard/page.tsx:64`), and without it, every other
|
|||
|
|
multi-site improvement is invisible to the user — the Overview will
|
|||
|
|
keep telling them they have 50 sites when they have 60. After that,
|
|||
|
|
the next priority is moving the per-site pages to server-side `site=`
|
|||
|
|
filters (Tier 2.1) so the Sites list page stops pulling the whole
|
|||
|
|
world on every render. Together those two changes get the app to
|
|||
|
|
"works correctly at 30-50 sites." Everything else in the assessment
|
|||
|
|
matters, but is downstream of those two.
|