@@ 1,6 1,6 @@
# lethe-web-ui-aggregates
-**Status:** Design
+**Status:** Reviewed
**Module:** `sourcecraft.dev/bigbes/lethe`
**Branch:** master
**Worktree:** none
@@ 237,3 237,82 @@ All risks restated from Design:
- `?cwd=` on `/sessions` is additive; old clients ignore the param and get the existing behaviour.
- No schema migrations; `internal/shared/wire/` untouched (verify with `git diff` at end of execute).
- No removed/renamed JSON fields anywhere.
+
+## Conclusion
+
+Outcome: All five planned phases shipped + two review-fix commits. HEAD = `1818fac`. Backend adds `/api/v1/projects` and `/api/v1/stats`; SPA replaces three foundation stubs; `?cwd=` extension on `/api/v1/sessions` is additive.
+
+Invariants:
+- IV (auth middleware reused, no bypass): `project.Handler.Mount` and `stats.Handler.Mount` register under the existing `/api/v1` chi group in `server.go`, exercised by the same `auth.MustIdentity` machinery as `sessions`. Confirmed by handler-level tests covering 403 for non-admin `?owner=`.
+- IV (`?owner=` admin override identical across `/sessions`, `/projects`, `/stats`): each handler resolves scope via `session.OwnerScope` switch on `AllOwners | SpecificOwner | default`. Tested per-handler.
+- IV (no schema migrations / no `internal/shared/wire/` changes / no removed JSON fields): `git diff 62dbaf9..HEAD -- internal/shared/wire/ schema/` empty.
+- IV (problem+json on errors): both new handlers route through `apierror.Render`; `TestProjectHandler_List_BadSinceReturns400` and `TestStatsHandler_BadRange_Returns400` assert the content-type.
+- IV (no cache for stats): repository computes fresh per request; no shared state.
+- IV (project identity = exact `working_dir`): server never normalizes; `cwd` flows through SQL `working_dir = ?` with parameter binding.
+- IV (TanStack Query + `apiFetch` only; no raw `fetch` outside `api/client.ts`): grep confirmed empty.
+- IV (no chart library dependency): grep of `web/package*.json` for `recharts|chart.js|d3|victory|nivo` empty.
+- IV (chart primitives inline in TSX): all four new primitives are inline TSX with no library; three of four (`StackedBars`, `Heatmap`, `HourBars`) use inline `<svg>`; `HorizontalBars` uses inline `<div>` bars — see deviation below.
+- IV (no silent fallbacks; per-card `EmptyState`): `routes/stats.tsx` renders an `EmptyState` per empty card; `useStats` does not synthesize cells.
+
+### Unknowns outcome
+- UK1 (TanStack splat-route compatibility with slash-bearing cwds): resolved in Phase 4. `params._splat` captures slash-bearing values as a single segment; `encodeURIComponent`/`decodeURIComponent` round-trips correctly. Real-browser smoke deferred — see Verified by below.
+- UK2 (aggregate query plan at scale): not investigated. At seeded fixture sizes the GROUP BYs are unindexed scans; the design called this out as not blocking. If `EXPLAIN QUERY PLAN` later shows the `top_cwd` scan dominating at production volume, add covering index `(owner, timestamp)` on `turns`.
+
+### Deviations from plan
+
+- **Phase 2 — `Daily` window capped at 60 days when `?range=all`**: Plan was silent on response size for the all-time case. 60 matches the prototype's daily-card width and the sparkline cap; bounding is the conservative read of the implicit "few KB" expectation in the design.
+- **Phase 2 — missing `?range` defaults to 30d**: Plan listed legal values but not absence behavior. Sessions list's analogous `?since` defaults to all-time, so a strictly consistent reading would default to `all` (or 400). Frontend always supplies `?range=` explicitly, so the edge never lands in real use. Reviewer to weigh server-default ergonomics if revisited.
+- **Phase 3 — synthesized 2-point sparkline in `ProjectsTable`**: `/api/v1/projects` returns no per-day series, but the design reuses `Spark` for an activity column. Renders `[0, (sessions/max)*12]` — relative-height placeholder, not real time-series. Tension with no-silent-fallbacks; the column is visual ranking, not a time-window claim. Remediation candidates: (a) drop the column, (b) extend `/api/v1/projects` to emit `daily_sparkline []int64` like `stats.PerTool` does. See Future work.
+- **Phase 4 — `ProjectHeader` derives `hosts` from sessions when `project` is loading**: Plan signature was caller-computes-hosts. Implementer kept the prop but added an internal fallback (use sessions's unique hosts while `project` is undefined). Avoids header flicker during projects-list fetch; widens component responsibility slightly.
+- **Phase 5 — CSS imported per-route rather than in `__root.tsx`**: Plan bullet 5.11 said to modify `__root.tsx`. The established codebase pattern (set by `routes/index.tsx`'s `home.css` import in the foundation, continued by Phases 3/4) is per-route imports. Sibling-consistency rule wins: `stats.css` is imported at the top of `routes/stats.tsx`.
+- **Phase 5 — `toolColor` palette inlined in `routes/stats.tsx`**: `ToolDot.tsx` exports only the component, no palette map. Duplicated a 5-entry palette + hash fallback locally rather than refactoring `ToolDot`. See Future work.
+- **Phase 5 — `HorizontalBars` uses div-based bars, not inline `<svg>`**: Strict-letter reading of the "all chart primitives inline `<svg>`" invariant flags this. Spirit-of-the-rule reading (no library, all inline TSX, server returns numbers) is preserved. Visually equivalent and simpler than an SVG `<rect>` implementation. Reviewer to weigh whether to tighten the invariant or convert.
+- **Cross-phase — Phase 5 commit missed `internal/server/web/dist/index.html`**: Vite stamps content-hash asset filenames; bundle changes flip them. Phases 3 and 4 included the regen; Phase 5 didn't. Resolved with follow-up commit `f6611d7` (`web: regenerate embedded SPA bundle for stats route`).
+
+### Review findings
+
+- Critical: none.
+- Important (4 — all resolved):
+ 1. Double navigation in `ProjectsTable` row click — fixed in `3fbbfc8`. The inner `navigate({ to: '/project/$', ... })` was redundant with the parent's `onOpen` handler; one click was pushing two history entries (back-button regression). Now matches the SessionsTable single-call pattern.
+ 2. `.card` / `.card-head` / `.card-body` rules duplicated in `stats.css` and `shell.css` — fixed in `3fbbfc8`. Removed the duplicates from `stats.css`; the rules live in `shell.css` (loaded globally) only.
+ 3. Risk of double URL encoding when passing pre-encoded path strings to TanStack `<Link to={...}>` — fixed in `3fbbfc8`. `HorizontalBars` now exposes an `onActivate(row)` callback instead of `href: string`; `routes/stats.tsx` provides the typed `navigate({ to: '/project/$', params: { _splat: ... } })`. Decouples the primitive from any specific route shape.
+ 4. Dead unexported `rawHosts` / `rawTools` fields on `Project` struct — fixed in `1818fac`. The fields had `db:"raw_hosts"` / `db:"raw_tools"` tags but the actual scan target is the local `rawRow` struct; sqlx silently ignored the unexported fields, so they never held data. Earlier deferral ("avoid amending a signed commit") was a weak rationalization; new commit is the right path per repo policy.
+
+### Future work
+
+- **Drop the synthesized sparkline column from `ProjectsTable`, or extend `/api/v1/projects` with a real `daily_sparkline`**. Justification: the no-silent-fallbacks invariant prefers no column over a fake one. New backend field is the bigger fix and warrants its own task. Cross-references the Phase 3 deviation above.
+- **Tighten the chart-primitive invariant or convert `HorizontalBars` to inline `<svg>`**. Justification: the literal-vs-spirit ambiguity in the current invariant text invites future drift. The decision belongs to a design revision, not an in-task fix.
+- **Extract `web/src/lib/toolColors.ts`** if a third call site for tool colors appears. Today `ToolDot` and `routes/stats.tsx` are the only two; one duplication is below the rule-of-three threshold.
+
+## Verify
+
+**Result:** passed
+
+Positive:
+- CK1 — `go test ./internal/domain/project/...` → ok
+- CK2 — `go test ./internal/domain/stats/... ./internal/domain/session/...` → ok
+- CK3 — `cd web && npm test` → 44/44 vitest cases pass (5 new for `adaptStats`, 5 for `adaptProject`)
+- CK4 — `cd web && npm run build` → tsc clean + vite 372 modules ok
+- CK5 — `go test ./cmd/lethe/...` (multi-user isolation e2e) → ok; exercises steward registration of `project.Handler` + `stats.Handler`
+
+Negative:
+- CK6 — `TestProjectHandler_List_BadSinceReturns400` covers `?since=garbage` → 400
+- CK7 — `TestStatsHandler_BadRange_Returns400` covers `?range=foo` → 400
+- CK8 — `TestProjectHandler_List_NonAdminOwnerParamReturns403` and `TestStatsHandler_NonAdminOwnerParam_Returns403` cover `?owner=*|alice|bob` → 403
+
+Invariants / assumptions:
+- CK9 (IV "no `internal/shared/wire/` changes") — `git diff 62dbaf9..HEAD -- internal/shared/wire/` empty
+- CK10 (IV "no chart library dependency added") — `grep '"(recharts|chart\.js|d3|victory|nivo)"' web/package*.json` empty
+- CK11 (IV "no raw `fetch` outside `api/client.ts`") — `grep -rn 'fetch(' web/src` finds nothing outside `client.ts`
+- CK12 (IV "all chart primitives are inline `<svg>`") — Spark, Heatmap, StackedBars, HourBars are inline SVG; **HorizontalBars uses div-based bars** — recorded as a deviation above; spirit-of-the-rule (no library, inline) preserved
+- CK13 (IV "auth middleware reused, no bypass") — `project.Handler` and `stats.Handler` mount under the existing `/api/v1` chi group via `server.go`; same auth middleware exercises them as `sessions`. Confirmed by handler tests using the shared `auth.MustIdentity` machinery.
+
+Interfaces:
+- CK14 (IF `useSessions(filters: HomeFilters)` accepts `cwd?: string`) — type-check at `routes/project.$.tsx` calling `useSessions({ cwd, since: 'all' })` passes tsc; `?cwd=` round-trips through `repository.go` query
+- CK15 (IF `useProjects(filters)` and `useStats(range)` shapes match plan) — `web/src/features/projects/useProjects.ts` and `web/src/features/stats/useStats.ts` exist with the declared signatures; vitest imports + tsc resolve them
+- CK16 (IF `/project/$` splat route registered) — `grep ProjectSplat web/src/routeTree.gen.ts` shows `path: '/project/$'` and `'/project/$': typeof ProjectSplatRoute`
+- CK17 (IF `GET /api/v1/projects` returns `{ projects, limit, offset }` JSON) — confirmed via `TestProjectHandler_List_OneProject` decoding and asserting fields
+
+### Deferred (needs user input)
+
+- **Browser smoke through the new SPA routes** — `/projects`, `/project/<encoded cwd>`, and `/stats?range=…` were not exercised in a real browser. The frontend is verified by tsc + vitest + bundle build, and the backend is verified by handler-level Go tests. Standing up the running server requires an auth bearer token + a populated DB; out of reach in autonomous mode without user input. Recommend the user (a) run `go run ./cmd/lethe`, (b) authenticate, (c) walk through the three new routes, before sending to review. Risk if skipped: SPA-side runtime regressions not surfaced by tsc (e.g. an unhandled async rejection, a missing CSS class) escape to review.