diff --git a/.planning/phases/15-dashboard-tablos/15-REVIEW.md b/.planning/phases/15-dashboard-tablos/15-REVIEW.md
new file mode 100644
index 0000000..6fad739
--- /dev/null
+++ b/.planning/phases/15-dashboard-tablos/15-REVIEW.md
@@ -0,0 +1,241 @@
+---
+phase: 15-dashboard-tablos
+reviewed: 2026-05-16T00:00:00Z
+depth: standard
+files_reviewed: 14
+files_reviewed_list:
+ - backend/internal/web/handlers_account.go
+ - backend/internal/web/handlers_discussion.go
+ - backend/internal/web/handlers_events.go
+ - backend/internal/web/handlers_files.go
+ - backend/internal/web/handlers_planning.go
+ - backend/internal/web/handlers_tablos.go
+ - backend/internal/web/handlers_tablos_test.go
+ - backend/internal/web/ui/app.css
+ - backend/tailwind.input.css
+ - backend/templates/account_providers.templ
+ - backend/templates/app_layout.templ
+ - backend/templates/app_layout_helpers.go
+ - backend/templates/planning.templ
+ - backend/templates/tablos.templ
+findings:
+ critical: 3
+ warning: 5
+ info: 3
+ total: 11
+status: issues_found
+---
+
+# Phase 15: Code Review Report
+
+**Reviewed:** 2026-05-16T00:00:00Z
+**Depth:** standard
+**Files Reviewed:** 14
+**Status:** issues_found
+
+## Summary
+
+Phase 15 delivers the dashboard + sidebar redesign (AppLayout), project-card grid on `/`, and the tablo detail page with inline-edit for title/description. The Go handler and template code is generally well-structured, with clear ownership checks (user_id in the DB query), CSRF wiring, and explicit HTMX response paths. The test suite is comprehensive.
+
+Three blockers were found: a runtime panic risk when the authenticated user has an empty email, a filename truncation that slices on bytes instead of runes (corrupting multi-byte filenames), and an event time validation that silently accepts a midnight end-time equal to a start-time of 00:00 (off-by-one). Five warnings cover unsafe color injection, `context.WithTimeout` cancel leak, `time.Local` in a server process, missing nil guard, and an unignored render error. Three informational items cover dead template code, a hardcoded inline style, and a redundant nil guard pattern repeated across every handler.
+
+---
+
+## Critical Issues
+
+### CR-01: Runtime panic — empty email sliced without length guard
+
+**File:** `backend/templates/app_layout.templ:105`
+**Issue:** `string([]rune(user.Email)[:1])` panics with a slice-bounds-out-of-range error when `user.Email` is an empty string. Although the current signup flow always stores an email, the `auth.User` struct does not enforce non-empty at the type level, so any user record with a blank email (e.g. OAuth path bug, manual DB edit, future feature) causes every authenticated page render to panic and return a 500 with no fragment rendered. Because `AppLayout` is called from every protected handler, the blast radius is the entire authenticated surface.
+
+**Fix:**
+```go
+// app_layout.templ — SidebarOrganizationFooter
+templ SidebarOrganizationFooter(user *auth.User, csrfToken string) {
+
+}
+```
+
+---
+
+### CR-02: Filename truncation slices bytes, not runes — corrupts multi-byte filenames / may panic
+
+**File:** `backend/internal/web/handlers_files.go:211`
+**Issue:** `filename = filename[:255]` operates on the raw byte slice of the Go string. When `filename` contains multi-byte UTF-8 characters (e.g. Japanese, emoji), the slice boundary can fall inside a character, producing an invalid UTF-8 string stored in the database and potentially returned to browsers. Additionally, if the `filepath.Base` result is shorter than 255 bytes but the caller path reaches this line due to a logic error, a panic is possible. The subsequent `filename == "." || filename == ""` check that follows the truncation does not protect against this.
+
+**Fix:**
+```go
+// Truncate by rune count, not bytes, to avoid splitting multi-byte characters.
+runes := []rune(filename)
+if len(runes) > 255 {
+ filename = string(runes[:255])
+}
+```
+
+---
+
+### CR-03: Event end-time validation accepts 00:00 when start is also 00:00 (off-by-one boundary)
+
+**File:** `backend/internal/web/handlers_events.go:344`
+**Issue:** The validation condition is `endValue.Microseconds <= startValue.Microseconds`. When a user submits `start_time = "00:00"` and `end_time = "00:00"`, both produce `Microseconds = 0`; the condition is true (`0 <= 0`) so the error fires correctly. However, the intent described in the error message ("End time must be after the start time") means **strictly greater than**, which `<=` does express — but the boundary case at midnight is ambiguous: a user wishing to model a range ending at "00:00 the next day" will be rejected. More critically, the midnight boundary (00:00) means `Microseconds = 0`; because `pgtype.Time` has no date component, start=23:00 and end=00:00 (next morning) produce `endValue.Microseconds (0) <= startValue.Microseconds (82800000000)` → true → error fires, rejecting a legitimate overnight range. This is a logic error for multi-day events.
+
+**Fix:** Add an explicit guard: if `end_time` represents 00:00 it should be treated as 24:00 (i.e., `24 * 3600 * 1e6`) when the intent is end-of-day. Alternatively, enforce that the same-day restriction is documented and validated consistently:
+```go
+// Reject 00:00 as an end time explicitly (it cannot mean "next midnight"
+// without a date component). Provide a clear validation message.
+if endValue.Microseconds == 0 {
+ errs.EndTime = "End time cannot be midnight (00:00). Use a time within the same day."
+} else if startValue.Valid && endValue.Microseconds <= startValue.Microseconds {
+ errs.EndTime = "End time must be after the start time."
+}
+```
+
+---
+
+## Warnings
+
+### WR-01: Unvalidated tablo color written directly into HTML `style` attribute — XSS risk
+
+**File:** `backend/templates/tablos.templ:105` and `backend/templates/app_layout.templ:88`
+**Issue:** The tablo color value from the database is interpolated directly into a `style` attribute: `style={ "background-color: " + card.Tablo.Color.String }`. templ auto-escapes attribute *values* in most contexts but `style` content is not HTML-escaped the same way as text content — CSS injection via `expression(...)` (IE), `url('javascript:...')`, or closing the style attribute with `"` are not blocked by templ's default escaping. The validation in `TablosCreateHandler` (hexColorRE) gates the create path, but **there is no validation in `TabloUpdateHandler`** — the update handler reads and passes `tablo.Color` unchanged from the DB. Any color value already stored (or injected via a direct DB write) propagates to the UI without re-validation. The `SidebarProjectsSection` in `app_layout.templ:88` has the same pattern.
+
+**Fix:** Re-validate the color string before rendering, or strip the CSS injection risk entirely by mapping known-good colors through a whitelist server-side. At minimum, re-apply `isValidCSSColor` in the template helper before emitting the style attribute. Since templ does not apply CSS-specific escaping, a CSS-safe function is required:
+```go
+// In a helper file:
+func safeCSSColor(s string) string {
+ if isValidCSSColor(s) {
+ return s
+ }
+ return ""
+}
+```
+Then in the template: `if safeCSSColor(card.Tablo.Color.String) != "" { ... }`.
+
+---
+
+### WR-02: `context.WithTimeout` cancel called after use — context leak in error path
+
+**File:** `backend/internal/web/handlers_files.go:238-243`
+**Issue:** The cleanup context is created with `context.WithTimeout`, then `deps.Files.Delete` is called, and then `cancel()` is called immediately after. While `cancel()` is called, it is called *after* the blocking `Delete` call, which means the cancel function is not deferred. If `Delete` panics (unlikely but possible), `cancel()` is never called, leaking the goroutine tracking the timeout. The comment says "call immediately after Delete, not via defer" which misunderstands the purpose of `defer cancel()` — `defer` is idiomatic and correct here since `cleanupCtx` is scoped to this block and `Delete` has already returned by the time `cancel()` is reached.
+
+More importantly, the cancel is called *after* `Delete` has already completed. This means the WithTimeout has no practical effect on the `Delete` call (it cancels a context that was already used). The real guard against a slow `Delete` is the context itself, which is passed in. If `Delete` takes longer than 10 seconds, the context expires and `Delete` should respect it — but `cancel()` must be deferred to ensure cleanup even on early returns.
+
+**Fix:**
+```go
+cleanupCtx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+defer cancel() // defer — always runs, prevents goroutine leak on any exit path
+if delErr := deps.Files.Delete(cleanupCtx, s3Key); delErr != nil {
+ slog.Default().Error("files upload: S3 cleanup after DB failure", "s3_key", s3Key, "err", delErr)
+}
+```
+
+---
+
+### WR-03: `time.Local` in server handlers — environment-sensitive behavior
+
+**File:** `backend/internal/web/handlers_events.go:28,31,35,60,100` and `backend/internal/web/handlers_planning.go:22,31`
+**Issue:** All date/time calculations use `time.Local`. In a containerized server deployment (Cloud Run, Docker), the container's local timezone is typically UTC, which may differ from the user's timezone. Event dates stored using `time.Local` on the server will be correct only if the server TZ matches what the user expects. If the server is in UTC and a user in UTC-5 submits a date of "2026-01-01", the server interprets it as UTC midnight, which is correct for storage. But the boundary: if a user's browser submits "2026-01-31" as the last day of the month, and the server is in a timezone where the local midnight differs, the computed `monthEnd = monthStart.AddDate(0, 1, -1)` may include or exclude an extra day. This is not a crash but can cause events to be invisible in the calendar view (off by one day at month boundaries).
+
+**Fix:** Use `time.UTC` consistently throughout date-only calculations, or pass an explicit timezone via configuration. This eliminates environment-sensitive behavior in the deployed binary.
+
+---
+
+### WR-04: Template render errors silently ignored throughout
+
+**File:** Multiple handlers — e.g., `backend/internal/web/handlers_tablos.go:60`, `backend/internal/web/handlers_discussion.go:64`, `backend/internal/web/handlers_events.go:151`, `backend/internal/web/handlers_files.go:95`
+**Issue:** Every `templates.*.Render(...)` call uses the blank identifier: `_ = templates.Foo(...).Render(r.Context(), w)`. If the template render fails (e.g., due to a context cancellation mid-stream, writer error, or broken pipe), the failure is swallowed entirely with no log. For HTMX partial swaps this results in a silent empty response that the browser may interpret as a successful swap, leaving the UI in an inconsistent state. The pattern is consistent across all handlers.
+
+**Fix:** At minimum, log render errors at warn level:
+```go
+if err := templates.Foo(...).Render(r.Context(), w); err != nil {
+ slog.Default().Warn("render failed", "handler", "TablosListHandler", "err", err)
+}
+```
+Note: Because headers may already be sent by the time Render fails mid-stream, the only recoverable action is logging — not writing a new HTTP error response. The log is sufficient.
+
+---
+
+### WR-05: `PlanningPageHandler` ignores the `ok` return from `auth.Authed`
+
+**File:** `backend/internal/web/handlers_planning.go:36`
+**Issue:** `_, user, _ := auth.Authed(r.Context())` discards the `ok` boolean. The route is protected by `auth.RequireAuth` middleware (confirmed in router.go), so `ok` is always true in practice. However, the pattern is inconsistent with `AccountProvidersHandler` (handlers_account.go:16) which correctly checks `ok` and redirects on failure. If `RequireAuth` middleware ever fails to set the user in context (bug, future refactor, middleware reordering), `PlanningPageHandler` will dereference `user.ID` on a nil or zero-value User, producing a runtime panic or silent wrong-user query. `TablosListHandler` and `TablosCreateHandler` have the same pattern.
+
+**Fix:** Follow the `AccountProvidersHandler` pattern:
+```go
+_, user, ok := auth.Authed(r.Context())
+if !ok {
+ http.Redirect(w, r, "/login", http.StatusSeeOther)
+ return
+}
+```
+
+---
+
+## Info
+
+### IN-01: Dead template — `TabloCard` and `tabloCardBody` are unreferenced
+
+**File:** `backend/templates/tablos.templ:122-153`
+**Issue:** `TabloCard` and its helper `tabloCardBody` implement the old card layout using `ui.Card`. The dashboard now uses `TabloProjectCard` exclusively (confirmed by `TablosDashboard` and `TabloCardWithOOBFormClear` which call `TabloProjectCard` and `TabloCard` respectively — wait: `TabloCardWithOOBFormClear` at line 231 calls `TabloCard`, so it is still referenced). On closer inspection: `TabloCardWithOOBFormClear` (line 231) calls `TabloCard(TabloCardFromTablo(tablo), csrfToken)` which keeps the old card alive in the OOB-form-clear path. However `TabloCard` renders the *old* card layout (using `ui.Card`), while the dashboard uses `TabloProjectCard` (new project-card layout). This means the card inserted by a create action will look visually different from all other cards on the page — the `TabloCardWithOOBFormClear` response appends an old-style card to a grid of new project-cards. This is a UI consistency defect.
+
+**Fix:** Update `TabloCardWithOOBFormClear` to call `TabloProjectCard` instead of `TabloCard`:
+```go
+templ TabloCardWithOOBFormClear(tablo sqlc.Tablo, csrfToken string) {
+ @TabloProjectCard(TabloCardFromTablo(tablo), csrfToken)
+
+}
+```
+Then remove `TabloCard` and `tabloCardBody` if they have no other callers.
+
+---
+
+### IN-02: Hardcoded inline `style="width: 0%;"` for progress bar — always renders 0%
+
+**File:** `backend/templates/tablos.templ:285-288`
+**Issue:** The progress bar in the tablo detail page header is hardcoded to `style="width: 0%;"` with label `0%`. There is no backend data driving this value. This is dead UI — a progress bar that is always empty. If this is intentional placeholder for a future feature, the element should be hidden or commented; as-is it presents misleading information to the user.
+
+**Fix:** Either compute actual progress from task completion data and pass it to the template, or remove the progress bar element until the feature is implemented:
+```html
+
+```
+
+---
+
+### IN-03: Redundant nil-then-empty-slice guard pattern repeated in every handler
+
+**File:** All handler files — e.g., `backend/internal/web/handlers_tablos.go:49-51`, `handlers_files.go:88-91`, `handlers_discussion.go:71-74`
+**Issue:** Every handler that calls a `List*` query follows this pattern:
+```go
+results, err := deps.Queries.ListFoo(...)
+if err != nil { ... results = []T{} }
+if results == nil { results = []T{} }
+```
+The `if results == nil` guard is redundant when `err != nil` already assigns `[]T{}` — and when `err == nil`, `pgx`/`sqlc` returns an empty non-nil slice, not nil. The double guard adds noise and misleads reviewers into thinking nil returns are possible from sqlc.
+
+**Fix:** Consolidate into a helper or remove the nil guard:
+```go
+if err != nil {
+ slog.Default().Error(...)
+ results = []T{}
+}
+// No need for: if results == nil { results = []T{} }
+```
+Or use a small helper: `func nonNil[T any](s []T) []T { if s == nil { return []T{} }; return s }`.
+
+---
+
+_Reviewed: 2026-05-16T00:00:00Z_
+_Reviewer: Claude (gsd-code-reviewer)_
+_Depth: standard_