# Maintainability Priorities 2026-03

## Purpose
- Capture the current highest-value maintainability improvements across frontend and backend.
- Give tomorrow's work a prioritized sequence instead of reopening the same analysis.
- Allow large refactors when they clearly reduce long-term complexity or operational risk.

## Scope And Caveat
- This note reflects the current Ballbox repo shape as reviewed on 2026-03-30.
- It is intentionally opinionated and includes both incremental fixes and large refactor candidates.
- Items are ordered by combined impact on safety, maintainability, and future change cost.

## Current Status Snapshot
- Done:
  - P0.1 fail-closed payments callback auth
  - P0.2 current `ballbox-store.ts` split phase
  - P0.3 current admin-client split phase for `network`, `players`, and `sports`
  - P0.4 TV player runtime source-of-truth unification
  - P1.6 first route-helper/envelope standardization pass across `network`, `players`, `sports`, and `payments`
- In progress:
  - P1.5 stop reloading full admin datasets after every mutation
  - P0.3 remaining admin clients: `payments`, `tvs`
- Not started:
  - P1.7 route feedback redesign
  - P1.8 transaction boundaries for multi-entity writes
  - P2.9 admin design system pass
  - P2.11 auth adapter isolation
  - P2.12 observability pass
  - P3.x lower-priority cleanup/modeling work

## P0

### 1. Fail closed on payments callback auth
- Status: done
- Problem: `lib/shared-secret.ts` returns `true` from `hasValidBearerAuthHeader()` when no secret is configured.
- Risk: `app/api/integrations/payments/approved/route.ts` can accept writes in a misconfigured environment.
- Refs:
  - `lib/shared-secret.ts`
  - `app/api/integrations/payments/approved/route.ts`
- Why this is first:
  - It is a real trust-boundary failure, not just a code-quality issue.
  - It weakens the whole payments integration if env setup drifts.
- Recommended direction:
  - Fail closed by default.
  - Validate required integration secrets during boot.
  - Return explicit misconfiguration errors instead of silently allowing the request.
  - Standardize server-to-server auth helpers so future integrations do not copy the same bug.

### 2. Split `lib/ballbox-store.ts` by bounded context
- Status: done for current phase; remaining work is migration/cleanup, not monolith-first extraction anymore
- Problem: one file owns DTOs, Prisma access, domain mapping, and cross-domain persistence concerns.
- Risk: every feature change raises cognitive load and regression risk across unrelated modules.
- Refs:
  - `lib/ballbox-store.ts`
  - `docs/domain-model-target.md`
- Why this is first:
  - This is the main structural bottleneck in the backend.
  - The file shape encourages cross-domain leakage and accidental coupling.
- Recommended direction:
  - Split by domain: `network`, `tvs`, `payments`, `players`, `sports`, `classes`.
  - Keep a minimal internal structure per domain:
    - repository: Prisma reads/writes only
    - service: domain rules and orchestration
    - mapper: API/admin DTO shaping
  - Do not replace one giant file with many giant files; keep boundaries explicit.
- Shipped:
  - `lib/ballbox-store.ts` is now primarily a facade/re-export layer.
  - domain/backend modules now exist for `network`, `tvs`, `players`, `sports`, and `classes`.
  - focused contract/module coverage was added around the new seams.
- Remaining follow-up:
  - migrate callers to domain modules directly where it improves clarity
  - add transaction boundaries and deeper integration coverage where multi-entity writes still deserve it

### 3. Break admin client monoliths into domain modules
- Status: in progress
- Problem: admin surfaces bundle data loading, mutation orchestration, local editing state, messaging, and render logic in very large client components.
- Refs:
  - `components/network-admin-client.tsx`
  - `components/tvs-admin-client.tsx`
  - `components/payments-admin-client.tsx`
  - `components/sports-admin-client.tsx`
  - `components/players-admin-client.tsx`
- Why this is first:
  - These files are the frontend equivalent of `ballbox-store.ts`.
  - They are already large enough to slow down safe product iteration.
- Current smell examples:
  - one global `busy` flag locking entire surfaces
  - repeated `readError` and error normalization patterns
  - repeated create/edit/delete flows with similar fetch plumbing
  - broad local state rewrites on every field change
- Recommended direction:
  - Split data and mutation logic from rendering.
  - Introduce small domain hooks such as `useNetworkAdminData()` and `useNetworkAdminMutations()`.
  - Extract repeated admin UI primitives only after the data/mutation split is clear.
- First cuts shipped:
  - `network-admin-client` is now a thin shell over config, hooks, shell, styles, and smaller sections/cards.
  - `players` now has dedicated client config, state hook, shell, and smaller stats/create/list-card seams.
  - `sports` now also has dedicated client config, state hook, shell, and smaller coach/event sections.
- Remaining follow-up:
  - apply the same pattern to `payments-admin-client.tsx`
  - apply the same pattern to `tvs-admin-client.tsx`
  - then reassess whether a small shared admin design-system layer should be extracted

### 4. Remove dual player runtime implementations
- Status: done
- Problem: the TV player runtime exists twice, once in React and once as plain JS.
- Refs:
  - `components/screen-player.tsx`
  - `public/player-runtime.js`
  - `app/player/[screenId]/route.ts`
- Why this is first:
  - The duplicated runtime logic covers manifest refresh, cache storage, blob URLs, heartbeats, and impression retry behavior.
  - This kind of duplication drifts silently and is expensive to verify.
- Recommended direction:
  - Define one runtime core as the source of truth.
  - Either compile/adapt it for both delivery modes or delete one implementation path.
  - Treat parity bugs here as high-severity because they affect constrained devices and offline behavior.
- Shipped:
  - TV runtime cache/media/impression/heartbeat/manifest-refresh logic now lives in one shared core used by both shells.

## P1

### 5. Stop reloading entire admin datasets after every mutation
- Status: in progress
- Problem: many admin flows perform a mutation and then refetch the full dataset.
- Refs:
  - `components/network-admin-client.tsx`
  - `components/tvs-admin-client.tsx`
  - `components/payments-admin-client.tsx`
  - `components/players-admin-client.tsx`
  - `components/sports-admin-client.tsx`
- Why it matters:
  - broad rerenders
  - poor responsiveness
  - extra code duplication
  - harder path to partial loading and pending row states
- Recommended direction:
  - patch local state per entity after successful mutation
  - scope pending state to row/form level instead of whole page
  - introduce a small fetch/cache abstraction before adopting a heavier data library
- Current note:
  - `network`, `players`, and `sports` are structurally ready for this next pass because config/state/shell seams now exist.

### 6. Standardize route helper patterns and API envelopes
- Status: done for first planned rollout
- Problem: some routes are strict and contract-backed, others are more ad hoc.
- Refs:
  - Good baseline: `app/api/network/route.ts`
  - Inconsistent candidates: `app/api/admin/tvs/screens/[screenId]/route.ts`, `app/api/admin/tvs/tv-devices/route.ts`, `app/api/admin/payments/sessions/route.ts`
- Why it matters:
  - frontend code accumulates branchy response handling
  - domain errors leak inconsistently
  - contract testing becomes uneven
- Recommended direction:
  - shared helpers for body parsing, success envelopes, error mapping, and response validation
  - one route style per app boundary
- First cut shipped: `app/api/network/network-route-helpers.ts` now standardizes JSON body parsing plus success/error envelopes for the network route cluster.
- Follow-on slices shipped: players, sports, and payments route families now use the same helper pattern for JSON parsing plus strict success/error envelopes, including focused invalid-payload contract coverage.
- Remaining follow-up:
  - decide whether to keep one helper per domain or add one thinner cross-domain base helper under them

### 7. Replace global imperative route feedback with narrower pending UX
- Status: pending
- Problem: `components/route-feedback.tsx` mutates document-level interactions, styles, and disabled states.
- Ref:
  - `components/route-feedback.tsx`
- Why it matters:
  - behavior is hard to reason about
  - easier to create accidental UX regressions around forms and links
  - weak locality: debugging requires understanding app-wide listeners
- Recommended direction:
  - prefer localized pending boundaries per layout/section/form
  - reduce direct DOM mutation
  - reserve global fallback behavior for genuinely app-wide transitions only

### 8. Make multi-entity writes transactional
- Status: pending
- Problem: some writes span related records without explicit transaction boundaries.
- Ref:
  - `lib/ballbox-store.ts`
- Why it matters:
  - partial failures leave inconsistent links
  - these bugs are difficult to notice until operations data looks wrong
- Recommended direction:
  - require `prisma.$transaction` for mutations that touch multiple related records
  - add focused integration tests for these paths

## P2

### 9. Establish a real admin design system instead of repeated class strings
- Status: pending
- Problem: the repo has a partial UI foundation, but admin surfaces still repeat large raw class strings and native controls.
- Refs:
  - `components/network-admin-client.tsx`
  - `components/ui/button.tsx`
  - `components/ui/input.tsx`
  - `app/styles/admin-tokens.css`
- Why it matters:
  - visual consistency degrades over time
  - simple layout changes become global search-and-edit work
  - it encourages copy-paste component growth
- Recommended direction:
  - add a small set of admin-only primitives such as `AdminCard`, `AdminField`, `AdminSelect`, `AdminStatusBadge`
  - migrate repeated panel and form patterns incrementally

### 10. Rebalance testing away from contracts plus e2e only
- Status: in progress
- Problem: current `vitest` coverage is mostly contract-scoped and misses many frontend and persistence seams.
- Refs:
  - `vitest.config.ts`
  - `tests/contracts/**/*.test.ts(x)`
- Why it matters:
  - too much confidence depends on browser tests
  - structural regressions in store/admin logic can slip through
- Recommended direction:
  - add Prisma-backed integration tests around repository/service modules
  - add focused component tests for admin mutation flows and runtime edge behavior
  - keep Playwright for smoke and cross-surface workflows, not every branch
- Shipped in this cycle:
  - focused backend-module coverage for `network`, `tvs`, `players`, `sports`, and `classes`
  - focused component/hook coverage for `network`, `players`, and `sports` admin seams

### 11. Isolate MVP auth behind an adapter before expanding more surfaces
- Status: pending
- Problem: auth is intentionally simple, but more modules are now leaning on it.
- Refs:
  - `lib/simple-password-session.ts`
  - `lib/admin-auth.ts`
  - `lib/student-auth.ts`
- Why it matters:
  - the current auth strategy is acceptable for now, but the migration path gets harder each time more routes couple to it directly
- Recommended direction:
  - define a small auth interface now
  - keep current password-based implementation behind it
  - avoid spreading direct cookie/session assumptions further into route code

### 12. Add minimal observability for integrations and runtime flows
- Status: pending
- Problem: health endpoints exist, but structured operational visibility is still thin.
- Refs:
  - `app/api/integrations/payments/health/route.ts`
  - `app/api/admin/summary/route.ts`
  - `lib/prisma.ts`
- Why it matters:
  - callback failures, heartbeat issues, and reconciliation anomalies are expensive to debug without traces or request IDs
- Recommended direction:
  - add request IDs
  - log integration auth failures and payload rejects
  - count heartbeat/impression failures and DB mutation errors

## P3

### 13. Reduce unnecessary client hydration on static or mostly-static surfaces
- Problem: some frontend surfaces likely hydrate more than necessary.
- Refs:
  - `components/navbar.tsx`
  - `components/hero.tsx`
  - `components/faq.tsx`
  - `components/cta.tsx`
  - `components/advertising.tsx`
- Recommended direction:
  - move static content back to server components
  - keep only small interactive islands on the client

### 14. Tighten domain lifecycle modeling in sports and payments
- Problem: some model boundaries feel early-stage and vulnerable to accidental overlap.
- Refs:
  - `prisma/schema.prisma`
  - `lib/payments.ts`
  - classes and sports-related store logic under `lib/ballbox-store.ts`
- Recommended direction:
  - document aggregate roots and lifecycle states before expanding workflows further
  - avoid shipping new features on top of ambiguous model ownership

### 15. Clean up repo hygiene signals
- Problem: a few small rough edges suggest normalization has not caught up with the current maturity of the repo.
- Refs:
  - `package.json` still uses `my-v0-project`
  - `components/classes-request-form.tsx` has a trailing import after the component body
- Why it matters:
  - small issues are not the main problem, but they are useful signals that repo-wide cleanup is due

## Recommended Execution Order
1. Fix payments callback auth and add env validation.
2. Carve `ballbox-store.ts` into domain modules without changing behavior.
3. Choose the player runtime source of truth and collapse duplicate logic.
4. Break admin clients into smaller data/mutation/view modules.
5. Normalize admin mutation flows to stop full reloads.
6. Standardize route helpers and error envelopes.
7. Add missing integration and component tests around the new seams.

## Refactors Worth Considering Even If They Feel Excessive

### 1. Generate most CRUD admin surfaces from contracts
- Single source: Prisma model metadata + Zod contract + error map.
- Output: route handlers, field config, validation, labels, and mutation wiring.
- Why it could be worth it:
  - current CRUD repetition is already expensive enough to justify a generator if the admin surface keeps expanding.

### 2. Extract a `runtime-core` package for TV playback
- Source of truth for cache, retry, manifest polling, diagnostics, and playback state.
- React shell and plain JS shell become adapters.
- Why it could be worth it:
  - prevents long-term runtime drift across low-connectivity devices.

### 3. Add a domain flight recorder
- Append-only events for inbound callbacks and admin mutations.
- Read models stay query-friendly, but audit and replay become trivial.
- Why it could be worth it:
  - overkill today, but high leverage if Ballbox keeps accumulating operational surfaces.

### 4. Turn Ballbox into an explicit modular monolith
- Keep one deployable app, but enforce internal module boundaries and public interfaces.
- Why it could be worth it:
  - this is the cleanest path if the repo grows beyond the current founder-stage complexity without jumping to microservices.

## Notes For Tomorrow
- The best near-term ROI is structural, not cosmetic.
- Do not start with style-only cleanup while `ballbox-store.ts`, admin monoliths, and runtime duplication still dominate change cost.
- When in doubt, prefer a refactor that creates a stable seam over a one-off local fix.