# Code Health Audit ## Purpose This document gives us a practical read on whether the repo is accumulating unhealthy debt after rapid feature delivery, and where to focus cleanup without turning it into a broad rewrite. ## Current Read The fear is reasonable, but this does not look like a codebase in crisis. The repo has grown quickly because it now contains real product surface area: RBAC, tenant config, ticket workflows, AI-assisted expert responses, Spanish/English localization, translation backfill, email flows, audit-friendly data handling, and a meaningful test suite. The main risk is not total line count. The risk is that a few large modules and repeated patterns become hard to change safely as more customer-specific behavior arrives. ## Size Snapshot Recent tracked-file counts: - Tracked files: about 330 - Raw TypeScript/JavaScript/CSS/Markdown/YAML/JSON lines: about 82k - Approximate hand-written code and docs, excluding lockfile and Drizzle migration metadata: about 21k - Tests: about 11k lines - Docs: about 6k lines Largest implementation areas: - `src/app`: about 10.7k lines - `src/lib`: about 5.1k lines - `src/inngest`: about 1.5k lines - `src/db`: about 1k lines, excluding generated migration metadata - `scripts`: about 1.4k lines Large files worth watching: - `src/app/(app)/submit/SubmitForm.tsx` - `src/app/(app)/tickets/[ticketId]/page.tsx` - `src/app/(app)/knowledge/KnowledgeForm.tsx` - `src/app/(app)/admin/users/UsersClient.tsx` - `src/app/(app)/tickets/[ticketId]/ExpertResponseForm.tsx` - `src/lib/ai-gateway.ts` - `src/inngest/completeness-agent.ts` - Larger integration and e2e test files under `tests/` ## Healthy Complexity Some growth is expected and useful: - Spanish/English dictionaries intentionally duplicate keys across languages. That is normal i18n structure, not bloat, as long as parity tests remain in place. - RBAC and ticket state behavior are domain complexity. They should stay explicit and tested rather than hidden behind overly clever abstractions. - Translation storage, source-language metadata, and backfill behavior add lines because they protect auditability and prevent English/Spanish content drift. - The test suite is large relative to app size, but that is a strength for a product moving quickly in regulated-adjacent workflows. - Drizzle migration snapshots and lockfiles inflate raw line counts and should not be interpreted as maintainability debt. ## Suspected Debt The highest-risk debt areas are: 1. Email template duplication Status: addressed in the current local change set by adding `src/lib/email-template.ts` and migrating the existing email builders to shared brand, layout, CTA, fallback-link, ticket-header, and HTML escaping helpers. 2. Large client/page components `SubmitForm`, the ticket detail page, `KnowledgeForm`, `UsersClient`, and `ExpertResponseForm` are doing enough work that future changes could become slower and riskier. These are not necessarily broken, but they are past the size where focused extraction will help. 3. Translation behavior spread across the app Translation behavior touches seed data, KB entries, tickets, user preferences, invite language, response generation, and backfill. That is expected, but the rules need to remain centralized enough that future features do not accidentally bypass them. 4. AI gateway concentration `src/lib/ai-gateway.ts` is a natural place for growth, but it can become a mixed provider adapter, prompt runner, policy layer, and workflow helper if we keep adding to it. 5. Test harness pressure Status: addressed in the current local change set by bounding the shared Postgres client pool in `src/db/index.ts` and adding `DB_MAX_CONNECTIONS` / `DB_IDLE_TIMEOUT_SECONDS` overrides. This should be enough for the connection buildup we observed; further work would only be needed if production telemetry still shows connection pressure after deploy. 6. Large integration/e2e test files Large tests are valuable, but they become hard to maintain when every case hand-rolls tenants, users, tickets, and localization setup. ## Leave Alone For Now These areas should not be refactored just to reduce lines: - Database schema and migrations, unless a concrete schema change is needed. - I18n dictionaries, as long as key parity and selected-language behavior are tested. - Ticket state machine logic, unless changing behavior or improving test coverage. - Inngest agents, unless new agent workflows make the current files materially harder to reason about. - Tenant config loading, unless adding new config dimensions. ## Recommended Punch List ### P0: Finish DB Connection Hygiene Status: implemented locally in `src/db/index.ts`; awaiting commit. Goal: keep test and serverless database connection counts bounded. Done criteria: - `DB_MAX_CONNECTIONS` and `DB_IDLE_TIMEOUT_SECONDS` overrides are supported. - Test and development defaults are intentionally small. - `pnpm type-check` and `pnpm test` pass. - Change is committed separately from documentation or refactor work. ### P1: Extract Shared Email Layout Status: implemented locally in `src/lib/email-template.ts`; awaiting commit. Goal: reduce duplicated email HTML and localization branching. Completed scope: - Create a shared email rendering helper for shell, branding, CTA button, footer, and plain-text fallback patterns. - Migrate the existing email builders to the helper. - Keep localized copy explicit inside each builder. - Reuse shared HTML escaping for rendered dynamic email content. Done criteria: - Visible email behavior is unchanged. - Localized copy remains explicit. - No template loses plain-text coverage. ### P1: Split `SubmitForm` Status: implemented locally by extracting submit form types/helpers, field rendering, and post-submit polling into focused files under `src/app/(app)/submit/`. Goal: make the submit flow easier to change without risking unrelated behavior. Candidate extraction: - Required field rendering - Narrative/problem details section - Attachment or supporting metadata section, if present - Submit/pending/success state handling Done criteria: - No user-facing behavior changes. - Existing tests remain green. - New components have clear prop boundaries and no hidden global state. ### P2: Split Ticket Detail Page Status: implemented locally by extracting ticket detail display sections into `src/app/(app)/tickets/[ticketId]/TicketDetailSections.tsx`. Goal: separate route-level data loading from display and expert actions. Candidate extraction: - Message thread component - Requester-visible response area - Expert-only AI draft/review panel - Ticket metadata/sidebar - Server-side data shaping helper Done criteria: - Requester never sees AI-only draft metadata or labels. - Expert confirmation behavior remains intact. - Existing route tests continue to cover visibility rules. ### P2: Create A Translation Rule Registry Status: implemented locally in `src/lib/translation-rules.ts` with registry tests. Goal: make it obvious which entities are translated, when, and from which source language. Candidate scope: - Document or encode the translatable entities: KB entries, ticket titles/descriptions, expert responses, email content, UI labels. - Centralize source-language and generated-variant rules. - Preserve manual override behavior for native Spanish or English edits. Done criteria: - Future features have one place to check translation expectations. - The existing `docs/spanish_translation_coverage.md` remains accurate. - No new runtime translation-on-every-page-load behavior is introduced. ### P2: Add Test Data Builders Status: started locally in `tests/helpers/db-factories.ts` and adopted by two integration tests. Goal: reduce repeated setup in integration and e2e tests. Candidate scope: - Tenant builder - User builder - Ticket builder - KB entry builder - Language preference helpers Done criteria: - At least two large test files become shorter or easier to read. - Builders do not hide important test-specific data. - Tests remain deterministic. ### P3: Split AI Gateway By Responsibility Goal: prevent `ai-gateway.ts` from becoming a mixed provider/workflow/policy module. Candidate split: - Provider adapter - Prompt invocation helper - Workflow-specific services - Error/retry/telemetry handling Done criteria: - Public call sites become clearer. - Prompt text remains in `prompts/`. - Existing mock-LLM tests remain meaningful. ### P3: Add Periodic Code Health Metrics Goal: make bloat visible before it becomes painful. Candidate command: - Count large files. - Count TODO/FIXME/HACK markers. - Count test files and implementation files. - Exclude lockfiles and generated migration metadata. Done criteria: - One command can produce a lightweight report. - The report is advisory and does not block normal feature work by default. ## Operating Principles - Prefer small, targeted refactors after a feature lands, not broad cleanup branches. - Refactor only around a concrete change or a known hotspot. - Keep behavior-preserving refactors separate from behavior changes. - Every server-side refactor should keep or improve test coverage. - Do not chase line count as the primary metric. Chase unclear ownership, duplicated rules, and difficult tests. ## Bottom Line The repo size looks reasonable for the current functionality, especially with a serious test suite and documented compliance/localization work. The next best move is controlled cleanup: commit the DB connection hygiene fix and shared email layout extraction, then take the next maintainability win in a small piece, likely splitting the largest user-facing form/page components.