This site uses one functional cookie to keep feature rollouts consistent for you. Nothing is set until you choose. See the privacy notice.
Dev notes
The whole-project review told me where the code was weak. This is the plan for fixing it — what I’m deduping against abstractions that already exist, what I’m deliberately leaving alone so I don’t overfit, and the order I’m shipping it in. The goal is a codebase that’s easier for the next engineer and cheaper for an AI to work in, without a rewrite.
The bones are good. The lib/ domain layer, the hooks/ data layer, the shared ui/ primitives, the BFF/auth boundary, the pure command-palette and world and flags cores, and the context docs are all things I’m happy to point at. So this isn’t a rewrite — the problems are localised duplication and inconsistency. Good abstractions already exist (site.ts, threads.tsx, backendFetch.ts, usePersistentState); they just aren’t applied everywhere, and a few files grew into content-as-code blobs.
src/lib or src/hooks so the coverage gate enforces it and the failing test is cheap. Every change is a small PR into develop, test first, suite green.All 50 thoughts/* and 14 learn/* pages hand-wrote the same ~25-line Metadata block — openGraph and twitter copy-pasted per page. There’s now a buildArticleMetadata({ title, description, path, ogType }) helper next to the existing SITE_URL/OG_IMAGE in lib/site.ts, and the pages call it.
Gain: Net −737 lines across 65 files, and an OG-convention change is now one edit instead of 65.
Guardrail: It deliberately doesn’t pull the description from the hub’s THOUGHTS registry — that stores the shorter card preview, not the SEO description, and coupling them would have silently changed the meta text. The world write-up keeps its hand-written block because its social copy intentionally differs; routing, which had no og tags at all, gained them. I used satisfies Metadata instead of a return annotation so the concrete shape stays readable in the test — no type assertion.
The fallback process.env.NEXT_PUBLIC_API_URL ?? "http://localhost:3001" is re-declared in 19 files even though backendFetch.ts already exports API_URL. The fix isn’t just to import it everywhere: backendFetch.ts also imports auth0 and next/server, so pulling API_URL into client modules (flags-client, referrals, the vitals and calendar pages) would drag server-only code into client bundles.
Gain: Extract the constant to a dependency-free lib/apiUrl.ts; backendFetch re-exports it so nothing breaks. Fixes a latent bundling smell, not just the copy-paste.
The iMessage “phone” shell — the same three-div flex justify-center → .phone → .chat wrapper — is duplicated in 36 write-ups. Worse, its stylesheet lives inside one feature’s folder (thoughts/styling/styling.module.css) but is imported by 40 unrelated pages.
Gain: Add a ChatThread component to lib/threads.tsx (which already owns Sent/Received/Timestamp), migrate the 36 pages, then move the CSS to a neutral home — a one-line import change once nothing else references it, instead of a 40-file rename.
Guardrail: The ordering was the whole trick: extract the component first so pages stop referencing the module directly, then move the file. The styling write-up — whose folder used to hold that stylesheet — has the full story.
Five hooks (useOperatorSales/Stores/Inventory/Activity/Planogram) repeat the same fetch→check→schema.parse→poll shape. A generic useOperatorResource collapses them; each existing hook becomes an ~8-line adapter that keeps its current return shape, so no call site changes.
Guardrail: The polling tiers are intentional, so they stay as explicit per-hook config, and the response field is a select function rather than a magic string — an odd-shaped endpoint shouldn’t break the abstraction. The tradeoff: a slightly longer call site per hook, bought in exchange for the tiers staying visible. The operator dashboard write-up carries the update in full — that’s also where the 5,120-line split landed.
Three styles coexisted: the clean withOperatorErrors wrapper, the calendar routes’ withBackend + upstreamErrorResponse, and ~35 bare hand-rolled try/catch blocks across NBA, TCG, google and vitals.
Gain: The two families that genuinely shared a shape collapsed onto one helper each: the NBA routes onto a proxyUpstream (public GET → JSON, beside fetchUpstream), and the TCG list routes onto a serveTcg (in a server-only module so the client bundle stays clean — the TCG write-up has that update). Each swap is behind characterisation tests written against the old routes that still pass.
Guardrail: No uber-wrapper, and I stopped there on purpose. The vitals and google routes are authenticated with bespoke shaping (a beacon POST, parallel fetches, field defaulting) — the “genuinely unique, keep the try/catch” case. And the localStorage-vs-usePersistentState item turned out to be a documented no-op: the hook is already used everywhere it fits, and the rest deliberately use custom serialisation or read-after-mount for hydration, so forcing them through it would reset saved preferences or break hydration.
ThoughtLayout (summary/chat toggle, “Dev notes” eyebrow) and the learn pages’ PageHeader + section-nav pattern are only conceptually similar — different navigation model, voice, and theming. Fusing them into one component with a dozen mode flags is the textbook overfit this whole review exists to avoid. Step 1 already unified the part that’s genuinely identical: the metadata.
OperatorDashboardContent.tsx was 5,120 lines — the biggest file in the repo. I split the worst offender into a 32-line orchestrator plus five section components (the chat, the timeline/overview, the build write-up, and the dated updates in two halves), cut only at <section> boundaries so the prose is byte-identical. Its exhaustive test suite — 72 assertions on exact text, section order, and every anchor — passes unchanged, which is the proof nothing moved.
Guardrail: The remaining 1,000–2,000-line write-ups are the same job and can follow one file per PR, but they’re deliberately not batched into this sweep — it’s review-churn with no behaviour change, best scheduled on its own.
The four landing versions look like duplication but are the point — a redesign history behind a ?version= registry. That’s a feature, not debt.
Every step is test-first and small enough to review in one sitting. The logic-bearing steps (the metadata builder, the apiUrl module, the ChatThread shell, the operator factory) land in lib/hooks, which the coverage gate already watches; the mechanical migrations and the route work lean on behaviour tests, since those files sit outside the gate. ts-prune and depcheck both block CI, so a stray export or dependency from an extraction can’t sneak through. The measure of success is boring: fewer lines, the same rendered output, and a shorter path to the code the next change needs.
satisfies, not a cast. The metadata builder returns a value typed with satisfies Metadata instead of an annotation, so the concrete shape stays readable in the test without ever reaching for a type assertion — the house rule is no assertions without a reason, and there wasn’t one.@paul-portfolio/* version drift — not my changes, just a local node_modules that never got reinstalled. A pnpm install cleared it; worth writing down because “it was already red” is easy to say and easy to be wrong about.This page ended by saying the pass was a one-off and duplication would accrue again, and that the trigger I trust is copying something a third time. toCents had been copied eight times. So I ran the sweep again, but started it differently: four read-only scanners over lib, api, hooks and components in parallel, each returning ranked, concrete findings with a risk level and whether a test already covered the code. The repo’s own deadcode and deadexports gates were already green, so there was no dead code to delete — every finding was duplication or complexity in live code.
Gain: What shipped, each as its own verified commit with the suite staying green: eight toCents copies, a fill-ratio ternary in five files, a seeded-hash loop and a couple of others collapsed to shared pure helpers; three google/auth routes adopted the withBackend wrapper and two NBA routes adopted proxyUpstream; a query-error block copied across six hooks and a query key recomputed ten times in one hook were hoisted; useOperatorAlerts joined its five siblings behind useOperatorResource; a swatch picker, a segmented control and a hover-popover state machine were pulled out of the components that had each grown their own copy; and the ten uniform operator-BFF read fallbacks collapsed to one readWithFallback.
Guardrail: What I left alone matters as much. discountedPrice is arithmetically identical to promoPrice, but it’s a deliberate cross-repo parity mirror with a guard test — collapsing it would delete the check, not the duplication. A “forward the JSON or a labelled error” helper would have touched fifteen routes, but a few of them forward the upstream error body instead of a fixed label, so one helper would have quietly changed their responses; not worth it. And a wrapper that looked like single-caller indirection turned out to be imported directly by a test, so deleting it would have gone red. The scanners flag; the judgement about whether a thing is duplication or a mirror stays mine.
Problem: The scan also turned up two real defects, which are behaviour changes rather than simplifications and so ride their own PR with their own tests: GET /api/vitals was calling bare fetch() with no timeout, bypassing the eight-second deadline fetchUpstream exists to enforce — the exact hang that helper was written for — and a scroll listener was re-attaching on every render for want of a dependency array. A “no behaviour change” PR is the wrong place to hide a behaviour change, however small.
Where a single feature was touched, its own dev-notes page carries the update, and links back here:
serveTcg, the detail routes deliberately left alone.