# Frontend Improvement Report

## 1. Remove Duplicate `.js` / `.tsx` Files — Code Slop

**Severity: High**

The codebase contains leftover `.js` files from an incomplete TypeScript migration. Every significant source file has a `.tsx`/`.ts` counterpart, making the `.js` versions dead code that adds confusion and maintenance burden.

Files to delete:
```
src/api/client.js
src/App.js
src/main.js
src/types/index.js
src/utils/chartHelpers.js
src/utils/time.js
src/components/tracker/SleepButton.js
src/components/tracker/TrackerPage.js
src/hooks/useSettings.js
src/hooks/useSleepSession.js
src/hooks/useStats.js
```

---

## 2. Dead Code — `chartHelpers.ts` exports never called — Code Slop / Duplication

**Severity: High**

All 7 exported functions in `src/utils/chartHelpers.ts` are tested but **never imported or called** from any component. Components duplicate the same logic inline instead of delegating to these utilities.

Dead exports:
- `buildTimetableSlots()` — reimplemented inline in `SleepTimetable.tsx`
- `computeWakeupMinutes()` — reimplemented inline in `WakeupPlot.tsx:31-44`
- `computeBedtimeMinutes()` — reimplemented inline in `BedtimePlot.tsx:31-44`
- `computeDailySummaryData()` — reimplemented inline in `DailySummary.tsx:41-48`
- `computeAverageStats()` — unused
- `reverseDateOrder()` — reimplemented inline in multiple chart components
- `filterDaysWithData()` — unused

**Fix options (pick one):**
- **Preferred:** Refactor chart components to call the helpers instead of reimplementing — removes ~60 lines of duplicated transformation code.
- **Alternative:** Delete the helpers and the tests for them — the logic lives only in the components.

Either way, having tested utilities that the application never actually uses is misleading and inflates the apparent test coverage.

---

## 3. Duplicated Datetime Formatting Logic — Duplication

**Severity: Medium**

`toLocalDatetimeInputValue()` is implemented in `src/utils/time.ts:40` _and_ again locally in `src/components/tracker/TimeAdjuster.tsx:8-19`. Additionally, `src/components/tracker/SessionList.tsx:6-24` defines its own `formatDateTime()` and `toDatetimeLocal()` helpers that overlap with what `time.ts` already provides.

**Fix:** Consolidate all datetime formatting into `src/utils/time.ts`. Have `TimeAdjuster` and `SessionList` import from there instead of reimplementing.

---

## 4. Duplicated Chart Data Transformation — Duplication / Simplification

**Severity: Medium**

`WakeupPlot.tsx:31-44` and `BedtimePlot.tsx:31-44` contain near-identical map → reverse → daysAgo pipelines:

```typescript
// Both files do essentially this:
const chartData = (data ?? [])
  .map((d) => ({
    ...d,
    daysAgo: Math.round((todayMs - new Date(d.date).getTime()) / MS_PER_DAY),
    minutes: adjustForTimezone(d.someTime),
  }))
  .reverse();
```

The only difference is which field (`wakeup_time` vs `bedtime`) is mapped to `minutes`.

**Fix:** Extract a single `buildTimeSeriesChartData(data, timeField)` helper (in `chartHelpers.ts`) and call it from both components.

---

## 5. Loop Inefficiency — Performance

**Severity: Low**

`WakeupPlot.tsx:31-34` and `BedtimePlot.tsx:31-34` call `new Date(getToday()).getTime()` _inside_ the `.map()` callback, re-creating the same `Date` object on every iteration.

```typescript
// Current (creates Date on every iteration)
.map((d) => ({
  daysAgo: Math.round((new Date(getToday()).getTime() - ...) / MS_PER_DAY),
}))

// Fix: hoist above the map
const todayMs = new Date(getToday()).getTime();
const chartData = (data ?? []).map((d) => ({
  daysAgo: Math.round((todayMs - ...) / MS_PER_DAY),
}));
```

---

## 6. `any` Type Usage — Code Quality / Type Safety

**Severity: Medium**

Five locations use `any`, undermining the value of TypeScript:

| File | Line | Issue |
|------|------|-------|
| `src/components/tracker/SleepButton.tsx` | 19 | `catch (error: any)` |
| `src/components/stats/WakeupPlot.tsx` | 74, 77 | Recharts `formatter=(value: any)` |
| `src/components/stats/BedtimePlot.tsx` | 74, 77 | Recharts `formatter=(value: any)` |
| `src/components/stats/DailySummary.tsx` | 98, 101 | Recharts `formatter=(value: any, name: any)` |
| `src/hooks/useSleepSession.ts` | 58 | `payload: any` in mutation function |

**Fixes:**
- Catch blocks: use `catch (error: unknown)` and narrow with `error instanceof Error`.
- Recharts formatters: the Recharts library exports `ValueType` and `NameType` — use those, or declare a local `type RechartsFormatter = (value: number, name: string) => ReactNode`.
- `useSleepSession` payload: create a `SleepSessionUpdatePayload` type in `src/types/index.ts` and use it.

---

## 7. Unsafe User Input Parsing — Security

**Severity: Medium**

`src/components/tracker/SessionList.tsx:40-41` parses datetime-local input values directly without validation:

```typescript
start_time: new Date(editStart).toISOString(),
end_time: editEnd ? new Date(editEnd).toISOString() : null,
```

If `editStart` is an empty string or a malformed value, `new Date(editStart)` produces an `Invalid Date` object and `.toISOString()` throws a `RangeError`, which bubbles up as an unhandled rejection.

**Fix:** Validate before submitting:
```typescript
const parsedStart = new Date(editStart);
if (isNaN(parsedStart.getTime())) {
  // show error to user
  return;
}
```

---

## 8. `alert()` for Error Messages — Code Slop

**Severity: Low**

`src/components/tracker/SessionList.tsx:47` uses the browser native `alert()` on save failure:
```typescript
alert('Failed to update session');
```

This blocks the UI thread, looks out of place in a modern app, and cannot be styled or dismissed programmatically.

**Fix:** Replace with a toast notification or an inline error state rendered in the component.

---

## 9. Missing Memoisation — Performance

**Severity: Low**

Several components create new function references on every render, which defeats React's reconciliation optimisations for any child that uses `React.memo` or `useCallback` itself:

- `src/components/tracker/SettingsPopover.tsx:72,82,92,102` — four `onChange` handlers as inline arrow functions
- `src/components/tracker/SessionList.tsx:76,85,91,98,109` — five inline handlers
- `src/components/tracker/TimeAdjuster.tsx:46` — inline `onChange`
- `src/pages/StatsPage.tsx:44` — inline `onChange`

**Fix:** Wrap each with `useCallback`. For purely presentational components with stable props (`IntervalPicker`, `Navbar`, chart components), wrap with `React.memo` to prevent unnecessary re-renders.

---

## 10. No Route-Level Code Splitting — Performance

**Severity: Medium**

`src/App.tsx:14-16` eagerly imports all page components:
```typescript
import TrackerPage from './components/tracker/TrackerPage';
import StatsPage from './components/stats/StatsPage';
```

`StatsPage` pulls in Recharts (a large library) on the initial load even when the user only visits the tracker tab.

**Fix:**
```typescript
const StatsPage = React.lazy(() => import('./components/stats/StatsPage'));
// Wrap route in <Suspense fallback={<LoadingSpinner />}>
```

---

## 11. Missing Tests — Testing

**Severity: High**

Most components have zero unit tests. The ones most at risk:

| Component | Gap |
|-----------|-----|
| `SessionList` | Edit flow, validation, delete confirmation |
| `SleepButton` | Optimistic state, error handling |
| `StatusBanner` | Conditional rendering, active session display |
| `SettingsPopover` | Click-outside dismiss, input changes persisted |
| `DailySummary` | Data transformation rendering |
| `useSleepSession` | Mutation callbacks, query invalidation |

**Recommended additions:**
- Unit tests for `SessionList` edit/save/discard/delete using Testing Library + MSW for API mocks
- Hook tests for `useSleepSession` verifying query invalidation after mutations
- Snapshot or rendering smoke tests for all chart components to catch regressions when data shape changes
- Test that invalid date inputs in `SessionList` are rejected before submission

---

## 12. Coarse `isLoading` in `useStats` — Code Quality

**Severity: Low**

`src/hooks/useStats.ts:32` ORs together three query loading flags:
```typescript
const isLoading = dailyQuery.isLoading || timetableQuery.isLoading || wakeupQuery.isLoading;
```

This means consumers cannot show partial data while other queries are still loading — the whole stats page shows a spinner until _all three_ queries complete.

**Fix:** Either export per-query loading states so consumers can render progressively, or use `isFetching` instead of `isLoading` if you want to show stale data while refreshing.

---

## 13. Missing Error Boundaries on Chart Components — Reliability

**Severity: Medium**

There is only one global `ErrorBoundary` wrapping the whole app. If any chart's data transformation throws (e.g. a malformed date from the API), it takes down the entire stats page.

**Fix:** Wrap each chart component in its own `ErrorBoundary` that renders a "Failed to load chart" fallback. This is a two-line change per chart using a reusable boundary component.

---

## Summary Table

| # | Category | File(s) | Severity |
|---|----------|---------|----------|
| 1 | Code slop — dead .js files | Multiple | High |
| 2 | Dead code / duplication | `chartHelpers.ts` | High |
| 3 | Duplication | `TimeAdjuster.tsx`, `SessionList.tsx`, `time.ts` | Medium |
| 4 | Duplication | `WakeupPlot.tsx`, `BedtimePlot.tsx` | Medium |
| 5 | Performance — loop | `WakeupPlot.tsx`, `BedtimePlot.tsx` | Low |
| 6 | Type safety — `any` | Multiple | Medium |
| 7 | Security — unvalidated input | `SessionList.tsx:40-41` | Medium |
| 8 | Code slop — `alert()` | `SessionList.tsx:47` | Low |
| 9 | Performance — memoisation | `SettingsPopover.tsx`, `SessionList.tsx` | Low |
| 10 | Performance — code splitting | `App.tsx` | Medium |
| 11 | Testing | Most components | High |
| 12 | Code quality — loading state | `useStats.ts:32` | Low |
| 13 | Reliability — error boundaries | Stats components | Medium |