alla-tms/docs/reviews/phase-5-code-quality-performance-audit.md

Phase 5 - Code Quality & Performance Audit

Executive Summary

The system currently passes typecheck, lint, and production build, so the codebase is operational and deployable in its current state. The main risk in this phase is not immediate build failure, but accumulated maintainability and performance debt concentrated in several oversized route handlers, server data modules, and workflow-heavy client forms.

The most important quality issue is structural complexity. A small set of files currently own too many responsibilities at once: authorization-aware data loading, filtering, export generation, workflow transitions, storage handling, notifications, and UI orchestration. This increases regression risk, makes optimization harder, and reduces testability.

The most important performance issue is repeated work inside large reporting and workflow paths rather than obvious catastrophic hotspots. In particular, report generation repeats scope-resolution queries, export logic is tightly coupled to dataset construction, and several large client components keep broad interactive state on the client without further decomposition.

Build/Config Assessment

Observed configuration

• package.json:10-15 defines dev, build, lint, lint:fix, and lint:strict.
• There is no dedicated typecheck script in package.json.
• tsconfig.json:5-19 uses strict: true, isolatedModules: true, and incremental: true.
• tsconfig.json:5-6 also keeps allowJs: true and skipLibCheck: true.
• Linting is done by oxlint, not a stricter combined lint-plus-type gate.

Assessment

Strengths:

• The project already compiles cleanly under production build.
• TypeScript strict mode is enabled.
• The repo has a stricter lint mode available through lint:strict.

Weaknesses:

• Missing typecheck script makes CI and contributor workflows less explicit.
• allowJs: true weakens long-term type-safety expectations in a TypeScript-first codebase.
• skipLibCheck: true is a practical default, but it can hide dependency typing drift.
• The default lint command allows warnings to pass, so code-quality drift can accumulate silently.

TypeScript Assessment

Strengths

• npx tsc --noEmit completed successfully.
• Most feature modules use explicit contracts and typed query/mutation boundaries.
• Shared data access patterns appear to prefer typed service layers over untyped ad hoc fetches.

Findings

• src/instrumentation-client.ts uses (Sentry as any), which weakens observability-related type safety.
• src/components/forms/demo-form.tsx and src/features/products/schemas/product.ts use permissive any-style schema patterns, suggesting leftover demo/legacy looseness.
• src/features/forms/components/sheet-product-form.tsx uses productSchema as any, which bypasses compiler guarantees.
• src/lib/api-client.ts:57 throws generic Error instances instead of a typed API/domain error model.

Assessment

TypeScript quality is acceptable at the baseline level, but not yet fully mature. The compiler settings are strong enough to prevent many failures, while a small set of escape hatches and generic error shapes still reduce confidence in cross-layer correctness.

React Assessment

Strengths

• The app reuses shared primitives and a shared query client.
• Several flows use TanStack Query instead of bespoke client fetching logic.
• Shared table state is centralized in src/hooks/use-data-table.ts, which is a good reuse decision even though the hook is now quite complex.

Findings

• src/features/training-records/components/training-record-form.tsx is 765 lines and combines form orchestration, uploads, mutations, attachment state, and workflow logic in one client component.
• src/features/online-lessons/components/online-lesson-form.tsx is 554 lines and keeps many local file/removal states plus multiple workflow mutations in one component.
• src/features/announcements/components/announcement-form.tsx uses the same multi-mutation workflow pattern and is trending toward the same shape.
• src/features/reports/components/reports-page-content.tsx:278,317 triggers react(no-unstable-nested-components) warnings.
• src/components/ui/calendar.tsx:58 and src/components/kbar/render-result.tsx:10 also define nested components during render.

Assessment

The main React risk is not hook misuse at scale, but oversized client components with broad responsibility and broad rerender surfaces. These components are harder to reason about, harder to profile, and harder to unit test than smaller view-model plus presentational splits.

Next.js Assessment

Strengths

• Production build succeeded under Next.js 16.2.6.
• The app makes meaningful use of server-side data loading.
• src/features/overview/server/overview-data.ts:171,185,249,519 uses cache() well in several high-read paths.

Findings

• src/app/api/announcements/[id]/route.ts is 683 lines and combines multipart parsing, draft/save/submit transitions, storage operations, audit logging, and delete handling in a single route module.
• src/lib/api-client.ts:29-41 resolves server request headers and reconstructs origin dynamically on every server request path, which adds per-call overhead and keeps the route-handler pattern tightly coupled to internal HTTP.
• The build output still includes many legacy/demo screens and routes such as /dashboard/forms, /dashboard/kanban, /dashboard/chat, /dashboard/product, and /dashboard/employees, which increases bundle and maintenance surface.

Assessment

The app is using Next.js capably, but it is not yet taking full advantage of cleaner server boundaries. The biggest framework-level opportunity is reducing workflow complexity inside route handlers and separating internal server logic from HTTP transport concerns.

Validation Assessment

Strengths

• The broader codebase follows a Zod-plus-form-wrapper approach.
• Workflow modules show evidence of server-side validation before persistence.

Findings

• Validation quality varies by feature. Demo and product-related areas still contain permissive schema patterns such as .any().
• File-heavy workflow routes appear to validate through branching procedural logic inside route handlers rather than through smaller reusable validation units.
• Status/date/file validation logic appears repeated across announcement and online-lesson workflow code paths instead of being fully centralized.

Assessment

Validation is good enough for core runtime flows, but consistency is not uniform across the repo. The main weakness is schema fragmentation and procedural validation logic inside large handlers.

Error Handling Assessment

Strengths

• Auth/session helpers throw explicit AuthError instances for unauthorized and forbidden cases.
• Route handlers generally return structured JSON responses rather than raw exceptions.

Findings

• src/lib/api-client.ts:57 throws plain Error, collapsing HTTP status, domain code, and user-display concerns into one generic error shape.
• src/lib/auth/session.ts contains many repeated throw new AuthError(...) branches, which is clear but repetitive and hints at opportunities for more composable authorization guards.
• Large route handlers and data modules mix business logic with logging and side effects, which makes it harder to guarantee consistent error mapping.

Assessment

Error handling is functional but not yet strongly normalized. The biggest gap is the lack of a consistent typed application error model spanning route handlers, services, and client consumers.

Duplicate Code Assessment

Findings

• Date formatting is scattered across many files through direct new Date(...).toISOString() and toLocaleDateString(...) usage, including src/app/api/announcements/route.ts, src/app/api/announcements/[id]/route.ts, src/app/api/online-lessons/[id]/route.ts, src/app/api/audit-logs/export/route.ts, and src/features/reports/components/reports-page-content.tsx:78-83.
• Workflow patterns for draft/save/submit/approve/archive actions are conceptually similar between announcements and online lessons, but remain implemented separately.
• Report rendering repeats table-like configuration and inline mapping logic inside src/features/reports/components/reports-page-content.tsx.
• Authorization guard usage is centralized, but authorization-related branching still appears repeatedly inside high-level server modules.

Assessment

The repo has good reuse in core scaffolding, but duplication is still visible in domain workflows, date formatting, report presentation, and status transition logic. This is manageable now, but it will get more expensive as more content modules are added.

Naming & Convention Assessment

Strengths

• Feature and file naming is generally readable and follows the project structure.
• Route names align well with feature intent.

Findings

• src/components/ui/slider.tsx:16 uses _values, which triggers a lint naming warning.
• Mixed legacy/demo/runtime naming remains across the repo, especially in products, forms, chat, kanban, and old employees areas.
• Some modules use domain-accurate names but still house too many concerns for the file name to communicate actual scope, for example report-data.ts and overview-data.ts.

Assessment

Naming is mostly not a blocking issue. The larger convention problem is scope drift: files whose names imply a focused responsibility but whose implementations have become mini-subsystems.

Database Query Performance Assessment

Strengths

• Several aggregation-heavy paths use SQL composition and Promise.all rather than purely sequential querying.
• Overview data makes a meaningful attempt to cache expensive scope/filter work.

Findings

• src/features/reports/server/report-data.ts:126,247,308,386 repeatedly calls getScopedEmployeeIds(...) in multiple report builders, which risks repeated scope-resolution queries within one report/export request.
• src/features/reports/server/report-data.ts:314,567,621 mixes broad dataset fetching with export preparation and follow-up joins, making it harder to deduplicate query work.
• src/features/permission-management/server/user-permission-data.ts:375 relies on transactional app logic to preserve one-active-template behavior, while the file itself contains a TODO noting the absence of a database-level unique constraint.
• Large export paths in reports include PDF and Excel generation in the same module as query logic, increasing the chance of over-fetching and memory-heavy processing.

Assessment

The main database-performance risk is repeated query work in analytics/reporting flows rather than obvious N+1 loops in everyday CRUD pages. Reporting will likely be the first area to degrade under scale.

Frontend Performance Assessment

Strengths

• Shared data-table state management includes debounce support and URL-state integration.
• Many pages are server-rendered first and hydrate into targeted client interactivity rather than fully client-side shells.

Findings

• src/features/training-records/components/training-record-form.tsx, src/features/online-lessons/components/online-lesson-form.tsx, and src/features/announcements/components/announcement-form.tsx keep large interactive workflows entirely in single client components.
• src/features/reports/components/reports-page-content.tsx contains nested render-time component definitions flagged by lint, which can cause unnecessary remounting and rerender churn.
• The build output still contains many legacy/demo dashboard surfaces, which increases long-term client bundle pressure even if each page is route-split.
• Heavy UI packages and legacy dashboard features remain installed even where they do not appear central to the TMS runtime.

Assessment

Frontend performance risk is moderate. The biggest concerns are rerender breadth and bundle sprawl, not obviously broken runtime performance today.

API Performance Assessment

Strengths

• The production build completed successfully, which suggests current route composition is at least operational.
• Many server modules already use batching through Promise.all.

Findings

• src/app/api/announcements/[id]/route.ts:88,162-623 performs multipart parsing, state transition branching, storage actions, and many audit log writes in one path.
• The same route logs multiple events in sequence at 445,458,475,522,549,573,609,623, and only one section uses Promise.all at 492.
• src/app/api/online-lessons/[id]/route.ts follows a similar content-workflow route shape, increasing the chance of repeated sequential I/O.
• src/lib/api-client.ts:41 routes internal feature services through HTTP fetch calls, which is acceptable for consistency but adds overhead versus direct server-side composition when both sides live in the same app.

Assessment

API performance is acceptable for current scale, but workflow-heavy content endpoints are likely to become latency hotspots because they combine validation, persistence, storage, notification, and audit concerns in one request lifecycle.

Dead Code Assessment

Findings

• npm run lint reports unused imports and variables in active code:
  • src/components/ui/duration-picker.tsx:13
  • src/features/online-lessons/components/online-lesson-form.tsx:121
  • src/features/organizers/components/organizers-page.tsx:31
  • src/features/reports/components/reports-page-content.tsx:120
  • src/app/api/online-lessons/[id]/route.ts:240
  • src/features/reports/server/report-data.ts:8,11,12
• The build output confirms many legacy/demo pages and features are still present in the shipped app tree.
• src/constants/mock-api.ts and src/constants/mock-api-users.ts still exist and receive lint warnings, indicating legacy/demo residue remains in the repository.

Assessment

Dead code is not overwhelming, but there is enough residue to create noise, hide real issues, and expand the maintenance surface beyond the actual TMS runtime.

Testability Assessment

Strengths

• Some data access is already separated into feature-level server modules.
• Shared hooks and shared auth helpers create a starting point for isolated testing.

Findings

• src/features/reports/server/report-data.ts couples filtering, authorization scoping, querying, table shaping, and export generation in one file.
• src/features/overview/server/overview-data.ts couples multiple dashboard datasets and policy/filter logic in one file despite good cache usage.
• src/app/api/announcements/[id]/route.ts mixes multipart parsing, workflow policy, storage, database updates, notifications, and auditing, which is difficult to unit test without broad integration scaffolding.
• src/hooks/use-data-table.ts is valuable shared infrastructure, but its breadth makes it a likely source of subtle regressions and harder isolated testing.

Assessment

Testability is the clearest structural weakness after maintainability. The codebase is not short on logic separation everywhere, but its heaviest business flows are still assembled in units that are larger than ideal for focused tests.

Findings by Severity

High

1. Oversized workflow handlers and data modules concentrate too many responsibilities.

   • Evidence: src/app/api/announcements/[id]/route.ts (683 lines), src/features/reports/server/report-data.ts (759 lines), src/features/overview/server/overview-data.ts (773 lines), src/features/training-records/components/training-record-form.tsx (765 lines).

2. Reporting paths repeat scope-resolution work and combine querying with export generation.

   • Evidence: src/features/reports/server/report-data.ts:126,247,308,386,766-822.

3. Core workflow-heavy client forms are broad client boundaries with large rerender surfaces and low testability.

   • Evidence: training-record-form.tsx, online-lesson-form.tsx, announcement-form.tsx.

Medium

1. Error handling is not yet consistently typed across service and client boundaries.

   • Evidence: src/lib/api-client.ts:57.

2. Database integrity for active permission-template assignment relies on app logic instead of a DB constraint.

   • Evidence: TODO in src/features/permission-management/server/user-permission-data.ts, transaction at 375.

3. Duplicate date formatting and workflow status handling remain spread across many routes and components.

4. Default lint workflow allows warnings to pass, and no dedicated typecheck script exists.

5. Legacy/demo modules continue to expand bundle and maintenance surface.

Low

1. Small unused variables/imports remain in active modules.
2. Naming/convention drift exists in a few UI utility files and legacy/demo areas.

Quick Wins

1. Add a dedicated typecheck script and run it in CI with lint:strict.
2. Remove current unused imports/variables reported by oxlint.
3. Centralize date formatting helpers for API payloads and Thai display formatting.
4. Extract nested render-time component definitions flagged by lint.
5. Split report export formatting from report data retrieval.

Structural Improvements

1. Break report-data.ts into scoped-query, dataset-builder, and export-renderer modules.
2. Break overview-data.ts into dashboard-section loaders plus a smaller composition layer.
3. Move announcement and online-lesson workflow transition logic into dedicated domain services invoked by thin route handlers.
4. Split large client forms into smaller sections with isolated mutation/upload adapters.
5. Introduce a typed application error model shared by route handlers, service callers, and client UI.
6. Audit legacy/demo routes and dependencies for removal or quarantine outside the production dashboard surface.

Recommended Refactor Order

1. Reports server path

   • Highest combined maintainability, query-efficiency, and export-performance payoff.

2. Announcement and online-lesson workflow handlers

   • Highest API complexity and side-effect density.

3. Large client workflow forms

   • Best follow-up once workflow services are thinner and easier to consume.

4. Error model and date-format normalization

   • Broad cross-cutting cleanup with good leverage.

5. Legacy/demo surface reduction

   • Useful after core runtime paths are stabilized.

Commands Executed and Results

• npx tsc --noEmit
  • Passed.
• npm run lint
  • Passed with warnings. Key warnings included unused imports/variables and react(no-unstable-nested-components).
• npm run build
  • Passed on Next.js 16.2.6 with production build and static page generation completed successfully.
• Multiple rg and Select-String read-only scans
  • Used to inspect large files, duplicate patterns, lint targets, and route complexity.

Files Reviewed

• package.json
• tsconfig.json
• src/lib/api-client.ts
• src/lib/auth/session.ts
• src/lib/query-client.ts
• src/hooks/use-data-table.ts
• src/features/reports/server/report-data.ts
• src/features/overview/server/overview-data.ts
• src/features/permission-management/server/user-permission-data.ts
• src/features/reports/components/reports-page-content.tsx
• src/features/training-records/components/training-record-form.tsx
• src/features/online-lessons/components/online-lesson-form.tsx
• src/features/announcements/components/announcement-form.tsx
• src/app/api/announcements/[id]/route.ts
• src/app/api/online-lessons/[id]/route.ts
• src/components/ui/calendar.tsx
• src/components/ui/duration-picker.tsx
• src/components/ui/slider.tsx
• src/constants/mock-api.ts
• src/constants/mock-api-users.ts

Areas Not Verified

• No runtime profiling was performed in a browser or with production tracing, so rerender cost and interaction latency were assessed from structure and lint/tool evidence rather than flamegraphs.
• No database execution plans were captured, so index/selectivity findings are risk-based rather than EXPLAIN-verified.
• No bundle analyzer output was generated, so bundle-size findings are based on route inventory and dependency surface, not measured chunk sizes.
• No automated test suite review was performed because this phase focused on code quality, static analysis, and build/runtime structure.