# Code Review: Candle Annotator

**Date**: 2026-02-18
**Reviewer**: Claude Code (Automated Audit)
**Scope**: Full codebase - frontend (Next.js), backend (FastAPI), configuration, deployment
**Verdict**: **Request Changes** - Critical and major issues must be fixed before production deployment.

---

## Executive Summary

93 issues identified across the entire codebase: **13 Critical**, **35 Major**, **45 Minor**.

The application has **zero SQL injection vulnerabilities** (Drizzle ORM and SQLAlchemy parameterize all queries), good error handling structure (every route has try/catch), and well-implemented timeouts on proxy routes.

However, the codebase has **no authentication anywhere**, hardcoded credentials in committed files, path traversal vectors, unsafe deserialization, and numerous frontend race conditions/memory leaks.

**Zero test files** exist anywhere in the codebase.

---

## Table of Contents

1. [Critical Issues](#1-critical-issues)
2. [Major Issues](#2-major-issues)
3. [Minor Issues](#3-minor-issues)
4. [Positive Feedback](#4-positive-feedback)
5. [Prioritized Fix Plan](#5-prioritized-fix-plan)

---

## 1. Critical Issues

### C-1. No Authentication on Any Endpoint (Frontend + Backend)

- **Files**: All 27 API route files in `src/app/api/`, all FastAPI endpoints in `services/ml/app/main.py`
- **Impact**: Anyone with network access can read all data, delete all charts/annotations, upload files, trigger training, load arbitrary models.
- **Fix**: Add Next.js middleware (`src/middleware.ts`) with API key or session auth. Add FastAPI `Depends()` for API key auth on the ML service. At minimum, protect write/delete/training operations.

### C-2. SSRF / Path Traversal via `run_id` in Training Runs DELETE

- **File**: `src/app/api/training/runs/[run_id]/route.ts:15`
- **Code**: `` fetch(`${INFERENCE_API_URL}/training/runs/${run_id}`) ``
- **Impact**: A crafted `run_id` like `../../admin/delete-all` traverses to arbitrary internal endpoints.
- **Fix**: Validate `run_id` matches `/^[a-zA-Z0-9_-]+$/` before interpolation.

### C-3. `.env` File NOT Gitignored - Credentials in Repo

- **File**: `.gitignore` only ignores `.env*.local`, not `.env`
- **Impact**: Database credentials (`ml_user:ml_password`) in `DATABASE_URL` are committed to git.
- **Fix**: Add `.env` to `.gitignore`, remove `.env` from git history with `git rm --cached .env`.

### C-4. Hardcoded Database Credentials Everywhere

- **Files**: `docker-compose.yml:10,37,73-74`, `services/ml/app/db.py:18-31`, `.env.example:3`
- **Impact**: `ml_user` / `ml_password` in plaintext in committed files.
- **Fix**: Use Docker secrets or `${POSTGRES_PASSWORD}` interpolation from `.env`. Remove SQL comment with credentials from `db.py`. Replace real creds in `.env.example` with placeholders.

### C-5. PostgreSQL Port Exposed to Host

- **File**: `docker-compose.yml:69` - `ports: "5432:5432"`
- **Impact**: Combined with weak hardcoded password, database is directly accessible externally.
- **Fix**: Remove port mapping or bind to `127.0.0.1:5432:5432`.

### C-6. No File Upload Size Limit

- **File**: `src/app/api/upload/route.ts:27-39`
- **Impact**: Denial of service via memory exhaustion or database flooding with unlimited CSV rows.
- **Fix**: Add explicit `file.size` check (e.g., reject > 10MB), limit row count inserted.

### C-7. Unsafe Pickle Deserialization via `joblib.load`

- **File**: `services/ml/app/main.py:266`
- **Impact**: `joblib.load(model_path)` uses pickle internally - can execute arbitrary code if a malicious `.pkl` is placed in `models/`.
- **Fix**: Add SHA256 integrity checks for model files. Consider ONNX/safetensors format.

### C-8. CORS Wildcard with Credentials on ML Service

- **File**: `services/ml/app/main.py:51-57`
- **Code**: `allow_origins=["*"]` + `allow_credentials=True`
- **Impact**: API accessible from any origin with credentials.
- **Fix**: Replace `*` with explicit origins like `["http://localhost:3000"]`.

### C-9. Stale Closure in `fetchPredictions`

- **File**: `src/app/page.tsx:552`
- **Impact**: `predictionState.modelInfo` captured in `generateCacheKey` (line 489) can be stale, producing incorrect cache keys and showing wrong predictions.
- **Fix**: Extract `modelInfo` into its own `useState` or use a ref for model version in `generateCacheKey`.

### C-10. Race Condition in `fetchPredictions` - No AbortController

- **File**: `src/app/page.tsx:494-552`
- **Impact**: Rapid clicks on "Run on Visible" causes earlier responses to resolve after later ones, overwriting correct state. Same for `handleFetchBatchPredictions` (lines 600-663).
- **Fix**: Use `AbortController` to cancel previous in-flight requests, or track a request ID and discard stale results.

### C-11. Stale Closure in CandleChart Click Handler

- **File**: `src/components/CandleChart.tsx:572-971`
- **Impact**: Async click handler captures `drawingState`, `selectedLineId`, `dragState`, `annotations` from closure. 12+ item dependency array causes excessive re-subscriptions and stale values between re-subscriptions.
- **Fix**: Use refs for frequently-changing values that the click handler reads, reducing re-subscription frequency.

### C-12. Stale Closure in SpanAnnotationManager Keyboard Handler

- **File**: `src/components/SpanAnnotationManager.tsx:461-535`
- **Impact**: Keyboard handler references `handleDeleteSpan` (depends on `selectedSpanId`) but it's not in the dependency array and not wrapped in `useCallback`. Pressing Delete uses stale `selectedSpanId`, potentially deleting wrong span.
- **Fix**: Wrap `handleDeleteSpan` in `useCallback` with proper dependencies, or use a ref for `selectedSpanId`.

### C-13. Pervasive `any` Types Disable Type Safety

- **Files**: `page.tsx:134,147,196-198`, `CandleChart.tsx:62,389`, `SpanAnnotationManager.tsx:25`, `Toolbox.tsx:23`, `SpanAnnotationList.tsx:15`, `TalibPatternPanel.tsx:19`, `FileUpload.tsx:45`, `rectangle-drawing.ts:60,63`
- **Impact**: Type errors at runtime go undetected at compile time. Especially dangerous for `geometry`, `sub_spans`, and prediction cache values.
- **Fix**: Define proper interfaces for `geometry`, `sub_spans`. Type candle arrays properly.

---

## 2. Major Issues

### M-1. Bulk Delete All Annotations Without Guard

- **File**: `src/app/api/annotations/route.ts:102-106`
- **Code**: `if (all === 'true')` deletes ALL annotations across ALL charts in one unauthenticated request.
- **Fix**: Require `chartId` for bulk deletes. Never allow unscoped deletion.

### M-2. Proxy Routes Forward Arbitrary JSON Without Validation

- **Files**: `src/app/api/predict/route.ts`, `predict/batch/route.ts`, `model/load/route.ts`, `training/start/route.ts`, `patterns/detect/route.ts`
- **Impact**: Arbitrary payloads forwarded to internal service.
- **Fix**: Validate request body with Zod schemas before forwarding.

### M-3. Error Messages Leak Internal Details (Frontend)

- **Files**: `health/route.ts:25`, `candles/route.ts:40`, `annotations/route.ts:35,86`, `annotations/[id]/route.ts:47,83`, `upload/route.ts:135,153`, `export/route.ts:67`, `span-annotations/export/route.ts:123`
- **Impact**: Raw `error.message` returned to clients can reveal table names, paths, connection strings.
- **Fix**: Return generic error messages to clients. Log full errors server-side only.

### M-4. Error Messages Leak Internal Details (Backend)

- **File**: `services/ml/app/main.py:640,778,1091,1134,1199,1296`
- **Impact**: Exception details including tracebacks returned to clients.
- **Fix**: Return generic messages. Log details server-side.

### M-5. Race Condition in Chart Cascade Delete

- **File**: `src/app/api/charts/[id]/route.ts:43-45`
- **Impact**: Three separate DELETE queries without a transaction.
- **Fix**: Wrap in `db.transaction(async (tx) => { ... })`.

### M-6. Missing Span Annotations Deletion in Chart Delete

- **File**: `src/app/api/charts/[id]/route.ts:42-45`
- **Impact**: Deletes annotations and candles but NOT `spanAnnotations` - FK violation or orphans.
- **Fix**: Add `db.delete(spanAnnotations).where(eq(spanAnnotations.chart_id, chartId))`.

### M-7. Race Condition in Unique Chart Name Generation

- **File**: `src/app/api/upload/route.ts:7-25`
- **Impact**: Concurrent uploads with same filename can cause unique constraint violation.
- **Fix**: Handle constraint error with retry, or use database-level upsert.

### M-8. Path Traversal via `run_id` in Model Load/Delete (Python)

- **File**: `services/ml/app/main.py:1203,1312`
- **Code**: `Path("models") / f"{run_id}.pkl"` - not sanitized.
- **Fix**: Validate UUID format, use `Path.resolve()` and verify within expected directory.

### M-9. Dynamic Module Import from Config

- **File**: `services/ml/features/custom_loader.py:54`
- **Code**: `importlib.import_module(feature_path)` from YAML config.
- **Fix**: Restrict to a whitelist of allowed module paths.

### M-10. No Resource Limits on Training

- **File**: `services/ml/app/main.py:907-1030`
- **Impact**: Loads entire dataset into memory, no timeout, daemon thread can corrupt models.
- **Fix**: Add file size check, training timeout, consider Celery/RQ task queue.

### M-11. Thread Safety Issue with Model State

- **File**: `services/ml/app/main.py:59-74,579-588,1330`
- **Impact**: Model read during prediction has no lock, but writes use `_model_swap_lock`.
- **Fix**: Use same lock for reads, or atomic reference swap pattern.

### M-12. ML Service Container Runs as Root

- **File**: `services/ml/Dockerfile` - no `USER` directive.
- **Fix**: Add `RUN useradd -m -r appuser && USER appuser`.

### M-13. MLflow + ML Service Ports Exposed Without Auth

- **File**: `docker-compose.yml:31,54-55`
- **Fix**: Remove port mappings or bind to `127.0.0.1`.

### M-14. TA-Lib Downloaded Over HTTP Without Checksum

- **File**: `services/ml/Dockerfile:10`
- **Fix**: Use HTTPS, add `sha256sum -c` verification.

### M-15. No Security Response Headers (CSP, X-Frame-Options)

- **File**: `next.config.js`
- **Fix**: Add `headers()` function with security headers.

### M-16. Unvalidated JSONB Fields Accept Arbitrary JSON

- **Files**: `annotations/route.ts`, `span-annotations/route.ts` (POST/PATCH)
- **Impact**: Storage abuse, deeply nested JSON DoS.
- **Fix**: Validate JSON structure and impose size limits.

### M-17. Full `node_modules` Copied to Production Image

- **File**: `Dockerfile:29`
- **Fix**: Remove the COPY line - standalone output bundles what it needs.

### M-18. `@types/*` in Production Dependencies

- **File**: `package.json:23-27`
- **Fix**: Move `@types/*`, `typescript`, `eslint`, `autoprefixer`, `postcss` to `devDependencies`.

### M-19. Missing `response.ok` Checks Before `.json()` (page.tsx)

- **File**: `src/app/page.tsx:214,230,245,257`
- **Impact**: If API returns 500 with non-JSON body, uncaught exception and silent failure.
- **Fix**: Add `if (!response.ok) throw new Error(...)` before parsing JSON.

### M-20. Missing `response.ok` Checks Before `.json()` (CandleChart)

- **File**: `src/components/CandleChart.tsx:163-164,178-179,192-193`
- **Fix**: Check response status before parsing.

### M-21. Memory Leak - SpanAnnotationManager Preview Primitive

- **File**: `src/components/SpanAnnotationManager.tsx:305-316`
- **Impact**: New `SpanRectanglePrimitive` on every mouse move, state/effect feedback loop.
- **Fix**: Use a ref for the preview primitive instead of state.

### M-22. Duplicate Interface Definitions Across 6 Files

- **Files**: `page.tsx:123-161`, `CandleChart.tsx:21-76`, `SpanAnnotationManager.tsx:8-39`, `Toolbox.tsx:9-50`, `SpanAnnotationList.tsx:6-29`, `SpanPopover.tsx:25-34`
- **Impact**: Maintenance burden and drift bugs.
- **Fix**: Centralize shared interfaces in `/src/types/` and import everywhere.

### M-23. Chart Re-creation on Theme Change Destroys State

- **File**: `src/components/CandleChart.tsx:333`
- **Impact**: Theme toggle destroys entire chart and re-creates it - loses all primitives, causes flicker.
- **Fix**: Use `chart.applyOptions()` to update theme colors without re-creating.

### M-24. Unbounded Prediction Cache

- **File**: `src/app/page.tsx:195-199`
- **Impact**: `predictionCacheRef` Map grows without limit.
- **Fix**: Implement LRU cache or limit to fixed number of entries.

### M-25. `fitContent()` Called on Every Span Change

- **File**: `src/components/SpanAnnotationManager.tsx:160`
- **Impact**: Resets user zoom/scroll position on every span annotation change.
- **Fix**: Remove from reconciliation effect. Only call on initial load.

### M-26. No Database SSL/TLS

- **Files**: `.env:3`, `docker-compose.yml:10,37`
- **Fix**: For production, add `?sslmode=require` to connection strings.

### M-27. `eslint-disable` Suppressing Missing Dependencies

- **File**: `src/components/TrainingPanel.tsx:128-129`
- **Fix**: Include stable callbacks in dependency array and remove the suppress.

### M-28. Hardcoded DB Credentials in Python Source

- **File**: `services/ml/app/db.py:18-31`
- **Fix**: Remove SQL comment with credentials. Require env vars, fail fast if missing.

### M-29. Health Check Fakes MLflow/DB Status

- **File**: `services/ml/app/main.py:396-409`
- **Fix**: Implement actual connectivity checks (`SELECT 1` for DB).

### M-30. Delete All Annotations Has No Confirmation Dialog

- **File**: `src/app/page.tsx:412-425`
- **Fix**: Add confirmation dialog before destructive action.

### M-31. No React Error Boundaries

- **File**: `src/app/layout.tsx`
- **Impact**: No Error Boundary anywhere in the component tree. If CandleChart (canvas APIs) or any child throws during render, the entire app crashes to a white screen.
- **Fix**: Add Error Boundary wrapping `{children}` in `layout.tsx` or wrapping CandleChart and sidebar sections individually.

### M-32. Hardcoded 1-Minute Candle Interval Assumption

- **File**: `src/components/CandleChart.tsx:452`
- **Code**: `for (let t = span.start_time; t <= span.end_time; t += 60)`
- **Impact**: For 5m/15m/1h/daily timeframes, this creates orders of magnitude more iterations than necessary, potentially freezing the browser.
- **Fix**: Determine actual candle interval from data or iterate over actual candle times.

### M-33. Excessive Prop Drilling (~25+ State Items)

- **File**: `src/app/page.tsx`
- **Impact**: Home component manages ~25+ pieces of state and passes them through layers. CandleChart receives 20+ props, Toolbox receives 15+.
- **Fix**: Consider React Context or Zustand for shared state (prediction, span annotations, chart selection).

### M-34. SpanAnnotationManager Recreates ALL Primitives on Every Change

- **File**: `src/components/SpanAnnotationManager.tsx:104-161`
- **Impact**: O(n) detach + O(n) attach on every selection change or annotation modification.
- **Fix**: Only update selection state of existing primitives instead of full recreation.

### M-35. Preview Primitive Cleanup Missing on Unmount

- **File**: `src/components/SpanAnnotationManager.tsx:265-324`
- **Impact**: Mouse move handler creates `SpanRectanglePrimitive` on every move. If component unmounts mid-draw, last preview is never detached.
- **Fix**: Add cleanup in useEffect return that detaches `previewPrimitive`.

---

## 3. Minor Issues

### Configuration & Deployment

| # | File | Issue | Fix |
|---|------|-------|-----|
| m-1 | All routes | No rate limiting on any endpoint | Add rate limiting middleware |
| m-2 | Dockerfiles | Base images not pinned to digest | Use `@sha256:<hash>` |
| m-3 | (missing) | No `.dockerignore` file | Create with `.git`, `.env`, `node_modules` |
| m-4 | `.gitignore` | `models/` and `.pkl` files not ignored | Add `models/`, `*.pkl` |
| m-5 | `Dockerfile` vs `docker-compose.yml` | Healthcheck tool mismatch (wget vs curl) | Use same tool |
| m-6 | `package.json` | Wide dependency ranges with `^` | Pin critical deps |
| m-7 | `Dockerfile:38` | `EURUSD.csv` data file baked into image | Mount at runtime |

### API Routes

| # | File | Issue | Fix |
|---|------|-------|-----|
| m-8 | Multiple routes | `parseInt()` missing radix or NaN check | Always `parseInt(v, 10)` + `isNaN()` guard |
| m-9 | `upload/route.ts:30-42` | No file type validation on upload | Check MIME type/extension |
| m-10 | Export routes | CSV injection (values starting with `=+@-`) | Prefix dangerous cells with `'` |
| m-11 | Multiple routes | `console.error` logs full error objects | Use structured logger |
| m-12 | `src/lib/db/migrate.ts` | Dead SQLite migration code | Remove if unused |

### Python ML Service

| # | File | Issue | Fix |
|---|------|-------|-----|
| m-13 | `app/main.py:307` | Deprecated `@app.on_event("startup")` | Migrate to `lifespan` pattern |
| m-14 | `app/db.py:45` | Deprecated `declarative_base()` | Use `class Base(DeclarativeBase)` |
| m-15 | `app/main.py:325,1000,1019,1080` | Deprecated `datetime.utcnow()` | Use `datetime.now(datetime.UTC)` |
| m-16 | `app/patterns.py` + `generate_talib_annotations.py` | Duplicate `TALIB_PATTERNS` dict | Import from one location |
| m-17 | `app/db.py:99-111` | `get_db_session()` leaks sessions (dead code) | Remove |
| m-18 | `app/main.py:129-134` | No input size limits on batch prediction | Add max date range validation |
| m-19 | `app/main.py:93` | No validation candles are time-sorted | Add sort validation |
| m-20 | `features/talib_features.py:169-190` | Missing fallback return for volume indicators | Add default return/raise |
| m-21 | `pyproject.toml:32` | Dead `inference*` package reference | Remove |
| m-22 | `app/main.py:9` | Inconsistent `uuid as uuid_lib` import | Use standard `import uuid` |

### React Components

| # | File | Issue | Fix |
|---|------|-------|-----|
| m-23 | `Toolbox.tsx:4` | Unused imports: `TrendingUp`, `ChevronUp` | Remove |
| m-24 | `CandleChart.tsx:452,569,837` | Magic numbers (8px, 60s, hardcoded 1-min interval) | Extract to named constants |
| m-25 | Multiple files | Missing ARIA labels on buttons, dropdowns, modals | Add `aria-label`, `aria-pressed` |
| m-26 | `ChartSelector.tsx:38-109` | Custom dropdown not keyboard accessible | Use Radix DropdownMenu |
| m-27 | `KeyboardShortcutsModal.tsx:38-77` | Modal not focus-trapped, missing `role="dialog"` | Add focus trapping |
| m-28 | `SpanAnnotationList.tsx:110-115` | Falsy check on `confidence` fails for value `0` | Use `!= null` check |
| m-29 | `page.tsx:56-85` | O(n*m) in `detectDisagreements` | Use sweep-line algorithm |
| m-30 | `page.tsx:49-53` | Dead filter code (TODO comment, no-op) | Implement or remove |
| m-31 | `CandleChart.tsx:125` | `new Set<string>()` default prop recreated each render | Use module-level constant |
| m-32 | `page.tsx` | ~20 props drilled to CandleChart | Consider React Context |
| m-33 | `ui/tooltip.tsx` | Tooltip component is a no-op placeholder | Use Radix Tooltip or remove |
| m-34 | `SpanAnnotationList.tsx:105` | White text on light label colors - poor contrast | Dynamic text color from luminance |
| m-35 | `page.tsx:892-906` | Settings menu uses `<a>` tags instead of Next.js `Link` | Use `Link` component |
| m-36 | `Toolbox.tsx:246-247` | `@ts-ignore` for CSS custom property | Use type assertion |
| m-37 | `SpanPopover.tsx:114-117` | Manual Escape handler conflicts with Radix Dialog | Remove manual handler |
| m-38 | `TalibPatternPanel.tsx:39` | Re-fetches when API returns empty array | Use `hasLoaded` flag |
| m-39 | `CandleChart.tsx:202-221` | `useImperativeHandle` missing dependency array | Add `[candles]` dep |
| m-40 | `CandleChart.tsx:1059-1110` | Missing `activeChartId` in primitive cleanup effects | Add to dependency arrays |
| m-41 | `ChartSelector.tsx:48` | Dropdown does not close on outside click | Add click-outside handler |
| m-42 | `annotation-types/page.tsx`, `span-label-types/page.tsx` | Dark theme not applied - hardcoded light colors | Use theme-aware CSS variables |
| m-43 | `PredictionPanel.tsx:148-155` | No debounce on confidence slider - re-renders chart every pixel | Debounce or use `onPointerUp` |
| m-44 | `globals.css:1` | External Google Font `@import` blocks rendering | Use `next/font` for optimization |
| m-45 | `CandleChart.tsx` | ~1100 line component - too large | Extract into custom hooks (`useChartInit`, `useChartAnnotations`) |

---

## 4. Positive Feedback

- **Zero SQL injection** - Drizzle ORM parameterizes all frontend queries; SQLAlchemy does the same on the backend.
- **Consistent error handling** - Every API route has try/catch with appropriate HTTP status codes.
- **Good timeout management** - All proxy routes use `AbortController` with configurable timeouts.
- **Solid Docker practices for Next.js** - Multi-stage build, non-root user (`nextjs`), healthcheck, standalone output.
- **Clean project structure** - Clear separation between frontend, API routes, ML service, and database layers.
- **Pydantic validation on ML endpoints** - Request/response models are well-defined with proper types.
- **Thread-safe training state** - Uses `threading.Lock` for `active_training_run_id` management.
- **Parameterized queries throughout** - Both Drizzle and SQLAlchemy prevent injection by design.
- **Proper OHLC data model** - Schema is well-designed with unique constraints and proper FK relationships.

---

## 5. Prioritized Fix Plan

### Phase 1: Security Critical (Immediate)

| Task | Files to Change | Effort |
|------|----------------|--------|
| Add `.env` to `.gitignore`, remove from git history | `.gitignore` | Low |
| Replace hardcoded creds with env var interpolation | `docker-compose.yml` | Low |
| Remove real creds from `.env.example` and `db.py` comments | `.env.example`, `services/ml/app/db.py` | Low |
| Bind DB port to `127.0.0.1` only | `docker-compose.yml` | Low |
| Validate `run_id` format in training routes | `src/app/api/training/runs/[run_id]/route.ts`, `services/ml/app/main.py` | Low |
| Add file upload size limit | `src/app/api/upload/route.ts` | Low |
| Fix CORS on ML service | `services/ml/app/main.py` | Low |

### Phase 2: Before Production

| Task | Files to Change | Effort |
|------|----------------|--------|
| Add API key authentication middleware | `src/middleware.ts` (new), ML service | Medium |
| Add Zod input validation on proxy routes | 5 proxy route files | Medium |
| Wrap chart cascade delete in transaction + add span delete | `src/app/api/charts/[id]/route.ts` | Low |
| Stop leaking error details to clients | 7+ route files, ML `main.py` | Medium |
| Add security response headers | `next.config.js` | Low |
| Fix ML Dockerfile to run as non-root | `services/ml/Dockerfile` | Low |
| Create `.dockerignore` | `.dockerignore` (new) | Low |
| Remove bulk delete-all without chartId guard | `src/app/api/annotations/route.ts` | Low |
| Add model file integrity checks | `services/ml/app/main.py` | Medium |

### Phase 3: Code Quality & Frontend Fixes

| Task | Files to Change | Effort |
|------|----------------|--------|
| Add AbortController to prediction fetching | `src/app/page.tsx` | Medium |
| Fix SpanAnnotationManager preview primitive memory leak | `src/components/SpanAnnotationManager.tsx` | Medium |
| Use `chart.applyOptions()` for theme changes | `src/components/CandleChart.tsx` | Medium |
| Add `response.ok` checks on all fetch calls | `page.tsx`, `CandleChart.tsx` | Low |
| Centralize duplicate type definitions | `src/types/` (new files), 6 component files | Medium |
| Remove `fitContent()` from reconciliation effect | `src/components/SpanAnnotationManager.tsx` | Low |
| Use refs for click handler closure values | `src/components/CandleChart.tsx` | Medium |
| Add React Error Boundary | `src/app/layout.tsx` or new component | Low |
| Fix hardcoded 1-min candle interval | `src/components/CandleChart.tsx` | Low |
| Fix SpanAnnotationManager full primitive recreation | `src/components/SpanAnnotationManager.tsx` | Medium |
| Move dev deps to `devDependencies` | `package.json` | Low |
| Remove dead code (`migrate.ts`, `get_db_session`, dead filter) | Multiple files | Low |

### Phase 4: Hardening & Polish

| Task | Files to Change | Effort |
|------|----------------|--------|
| Add rate limiting | Middleware | Medium |
| Add confirmation dialog for delete-all | `src/app/page.tsx` | Low |
| Fix accessibility (ARIA labels, focus trapping, keyboard nav) | Multiple components | Medium |
| Add CSV injection protection to exports | 3 export route files | Low |
| Pin Docker images to digests | Dockerfiles | Low |
| Implement proper health checks (ML service) | `services/ml/app/main.py` | Low |
| Replace deprecated Python APIs | `services/ml/app/main.py`, `db.py` | Low |
| Extract magic numbers to constants | `CandleChart.tsx`, `page.tsx` | Low |
| Fix dark theme on settings pages | `annotation-types/page.tsx`, `span-label-types/page.tsx` | Low |
| Add debounce to confidence slider | `PredictionPanel.tsx` | Low |
| Use `next/font` instead of CSS `@import` | `globals.css`, `layout.tsx` | Low |
| Break up CandleChart into custom hooks | `CandleChart.tsx` | High |
| Add click-outside handler to chart dropdown | `ChartSelector.tsx` | Low |
| Consider React Context for shared state | `page.tsx`, multiple components | Medium |
| Write tests | New test files | High |

---

## Test Coverage Assessment

- [ ] No unit tests for API routes
- [ ] No integration tests
- [ ] No ML model/pipeline tests
- [ ] No frontend component tests
- [ ] No end-to-end tests

---

## Summary by Area

| Category | Critical | Major | Minor | Total |
|----------|----------|-------|-------|-------|
| API Routes (TS) | 3 | 7 | 5 | 15 |
| React Components | 5 | 14 | 23 | 42 |
| Python ML Service | 2 | 7 | 10 | 19 |
| Config & Deployment | 3 | 7 | 7 | 17 |
| **Total** | **13** | **35** | **45** | **93** |

> Note: Some issues overlap across categories (e.g., auth affects both frontend and backend).

---

## No XSS or Client-Side Secret Issues Found

- No usage of `dangerouslySetInnerHTML` anywhere in the codebase.
- All user content rendered through React JSX (auto-escaped).
- No API keys, tokens, or sensitive data stored in client-side state.
- All API calls use relative URLs to same origin.