diff --git a/.planning/phases/04-tasks-kanban/04-REVIEW.md b/.planning/phases/04-tasks-kanban/04-REVIEW.md
new file mode 100644
index 0000000..63e6d23
--- /dev/null
+++ b/.planning/phases/04-tasks-kanban/04-REVIEW.md
@@ -0,0 +1,248 @@
+---
+phase: 04-tasks-kanban
+reviewed: 2026-05-15T00:00:00Z
+depth: standard
+files_reviewed: 11
+files_reviewed_list:
+  - backend/cmd/web/main.go
+  - backend/internal/db/queries/tasks.sql
+  - backend/internal/web/handlers_tasks.go
+  - backend/internal/web/handlers_tasks_test.go
+  - backend/internal/web/router.go
+  - backend/internal/web/ui/button.css
+  - backend/migrations/0004_tasks.sql
+  - backend/templates/layout.templ
+  - backend/templates/tablos.templ
+  - backend/templates/tasks.templ
+  - backend/templates/tasks_forms.go
+findings:
+  critical: 2
+  warning: 4
+  info: 2
+  total: 8
+status: issues_found
+---
+
+# Phase 04: Code Review Report
+
+**Reviewed:** 2026-05-15T00:00:00Z
+**Depth:** standard
+**Files Reviewed:** 11
+**Status:** issues_found
+
+## Summary
+
+Phase 04 adds the Tasks (Kanban) surface on top of the existing Tablos workflow. The core CRUD and ownership model is sound: `loadOwnedTabloForTask` correctly gates all task mutations behind tablo ownership and the `tablo_id` WHERE clause in `UpdateTask` / `DeleteTask` queries prevents cross-tablo task access. CSRF is applied uniformly, input lengths are validated, and all status values are validated against a whitelist before touching the DB.
+
+Two blocking issues were found: a missing `ParseForm` call in `TaskCreateHandler` that silently swallows the request body on most Go HTTP clients, and an integer overflow path in `TaskCreateHandler` that can corrupt positions when a column holds 21 million+ tasks. Four additional warnings cover a silent-skip `UpdateTask` error in the reorder loop, an inconsistency between the `TaskCreateFormFragment` swap target and the `HX-Retarget` header, a security-relevant XSS vector in the raw `fmt.Fprintf` delete response, and a `description` field that is left trimmed to empty string but still passed as `Valid: true` in `TaskUpdateHandler`.
+
+---
+
+## Critical Issues
+
+### CR-01: `TaskCreateHandler` never calls `r.ParseForm()` — form body not parsed
+
+**File:** `backend/internal/web/handlers_tasks.go:132`
+
+**Issue:** `TaskCreateHandler` calls `r.PostFormValue("title")` and `r.PostFormValue("status")` without first calling `r.ParseForm()`. Go's `http.Request.PostFormValue` only parses the form once implicitly; however the gorilla/csrf middleware reads the request body to validate the `_csrf` token and may consume the body before this handler runs, depending on whether the CSRF middleware buffers the body. In practice, if the middleware does not restore the body after reading it, all subsequent `r.PostFormValue` calls return empty strings. The HTMX path will always render a "Title is required" 422 error instead of creating the task. `TaskUpdateHandler` has the same missing call at line 321.
+
+By contrast, `TaskReorderHandler` (line 401) correctly calls `r.ParseForm()` first. The two create/update handlers omit this entirely.
+
+**Fix:**
+```go
+func TaskCreateHandler(deps TasksDeps) http.HandlerFunc {
+    return func(w http.ResponseWriter, r *http.Request) {
+        tablo, _, ok := loadOwnedTablo(w, r, TablosDeps{Queries: deps.Queries})
+        if !ok {
+            return
+        }
+        ctx := r.Context()
+
+        if err := r.ParseForm(); err != nil {
+            http.Error(w, "bad request", http.StatusBadRequest)
+            return
+        }
+
+        title := strings.TrimSpace(r.PostFormValue("title"))
+        statusStr := r.PostFormValue("status")
+        // ... rest unchanged
+    }
+}
+```
+
+Apply the same fix to `TaskUpdateHandler` at line 321.
+
+---
+
+### CR-02: `fmt.Fprintf` in `TaskDeleteHandler` emits unescaped HTML — potential XSS
+
+**File:** `backend/internal/web/handlers_tasks.go:274`
+
+**Issue:** After a successful delete, the HTMX response is built with a bare `fmt.Fprintf`:
+
+```go
+fmt.Fprintf(w, `<div id="task-%s" class="task-card-zone"></div>`, task.ID.String())
+```
+
+`task.ID` is a `uuid.UUID` whose `String()` output is always a lowercase hexadecimal UUID (safe). However the pattern is dangerous: it bypasses the `templ` auto-escaping pipeline used everywhere else in this codebase. If this pattern is copied for a field that is not a UUID (e.g., a task title in a future notification snippet), it becomes an XSS vector immediately. Additionally, the `id` attribute constructed here must exactly match `id="task-{task.ID}"` used by `TaskCard`; the manual string construction is fragile and inconsistent with every other handler.
+
+**Fix:** Use a proper templ component instead of raw `fmt.Fprintf`:
+
+```go
+// In tasks.templ — add:
+templ TaskCardGone(taskID uuid.UUID) {
+    <div id={ "task-" + taskID.String() } class="task-card-zone"></div>
+}
+
+// In handlers_tasks.go replace fmt.Fprintf with:
+_ = templates.TaskCardGone(task.ID).Render(r.Context(), w)
+```
+
+---
+
+## Warnings
+
+### WR-01: `UpdateTask` errors silently discarded in `TaskReorderHandler` loop
+
+**File:** `backend/internal/web/handlers_tasks.go:479`
+
+**Issue:** Inside the main reorder loop and also in the single-task path (line 441), `UpdateTask` errors are explicitly discarded with `_, _ =`. If the DB is temporarily unavailable or a constraint fails mid-loop, the handler returns 200 with a stale board render. The user sees an apparent success while some (or all) position updates were never committed. The same silent discard appears at line 441 in the single-task branch.
+
+**Fix:** Log errors and either abort with a 500 or collect failures and surface them:
+
+```go
+if _, err := deps.Queries.UpdateTask(ctx, sqlc.UpdateTaskParams{...}); err != nil {
+    slog.Default().Error("tasks reorder: UpdateTask failed",
+        "task_id", existing.ID, "err", err)
+    // option A: abort entire reorder
+    http.Error(w, "internal server error", http.StatusInternalServerError)
+    return
+}
+```
+
+---
+
+### WR-02: `TaskCreateHandler` swap target mismatch — `TaskCardOOB` body lands in `#column-{status}` but card and OOB fragment need different targets
+
+**File:** `backend/internal/web/handlers_tasks.go:213-215`
+
+**Issue:** On success, the handler sets:
+
+```go
+w.Header().Set("HX-Reswap", "beforeend")
+w.Header().Set("HX-Retarget", "#column-"+string(status))
+```
+
+Then renders `TaskCardOOB`, which emits two top-level elements: a `TaskCard` div AND an OOB swap div. The `HX-Retarget` header redirects the primary swap to `#column-{status}` with `beforeend`. The OOB div (`hx-swap-oob="innerHTML:#add-task-slot-{status}"`) is processed separately by HTMX. This appears correct at first glance.
+
+However, `TaskCreateFormFragment` (in `tasks.templ:240`) declares `hx-target="#column-{status}"` and `hx-swap="beforeend"` on the form itself. When HTMX processes the response, it uses the server-sent `HX-Retarget` / `HX-Reswap` headers to override the form's own targets. The form is currently inside `#add-task-slot-{status}` (added there by `AddTaskTrigger`), not inside `#column-{status}`. If `HX-Retarget` successfully redirects the primary swap target to `#column-{status}`, the TaskCard will appear in the column — but the original form element inside `#add-task-slot-{status}` will remain because no swap targets it directly. The OOB resets `#add-task-slot-{status}`, which should clean it up.
+
+The real issue: on a **validation error** path (422), the handler does NOT set `HX-Retarget`. The form's own `hx-target="#column-{status}"` and `hx-swap="beforeend"` then replace the entire column's content with the error form, not the add-task slot, destroying all task cards in that column for the duration of the error display. This is a functional bug that corrupts the user's view.
+
+**Fix:** On the 422 validation-error path, either set `HX-Retarget`/`HX-Reswap` headers to point back at `#add-task-slot-{status}` with `innerHTML`, or change `TaskCreateFormFragment`'s `hx-target` and `hx-swap` to target the slot (and rely on server headers only for the success path).
+
+---
+
+### WR-03: Description whitespace-only string sets `Valid: true` in `TaskUpdateHandler`
+
+**File:** `backend/internal/web/handlers_tasks.go:352`
+
+**Issue:** The description field is read without trimming:
+
+```go
+description := r.PostFormValue("description")
+```
+
+Then stored as:
+
+```go
+Description: pgtype.Text{String: description, Valid: description != ""},
+```
+
+A user who submits a description of `"   "` (spaces only) will store a whitespace-only string as a valid, non-null description in the DB. This is inconsistent with how other nullable text fields are handled (e.g., tablo description and color fields are typically empty-string checked). Querying for tasks with non-empty descriptions, or displaying them, will produce unexpected results.
+
+**Fix:**
+```go
+description := strings.TrimSpace(r.PostFormValue("description"))
+// Then the existing Valid check `description != ""` correctly excludes whitespace-only.
+```
+
+---
+
+### WR-04: Integer overflow in `TaskCreateHandler` position arithmetic
+
+**File:** `backend/internal/web/handlers_tasks.go:189`
+
+**Issue:** `maxPos` is `int32` (returned by `MaxPositionByTabloAndStatus`). `maxPos + 100` is also `int32`. If `maxPos` is `math.MaxInt32 - 99` (2,147,483,548) or higher, the addition overflows silently to a negative value, and the `InsertTask` call will store a negative position. Postgres stores it without complaint because the column is `integer NOT NULL` with no positive constraint.
+
+Subsequent `MaxPositionByTabloAndStatus` calls will return the negative position, causing the next task to be inserted with an even-more-negative position. The board will silently render cards in an incorrect order.
+
+While reaching 2 billion tasks per column requires pathological abuse, there is no server-side guard preventing a user from creating that many tasks, and the condition is undetectable from the user side.
+
+**Fix:** Add an explicit overflow check:
+
+```go
+const maxAllowedPosition = int32(2_000_000_000)
+if maxPos > maxAllowedPosition-100 {
+    errs.General = "Column has too many tasks to add more."
+    // render error fragment
+    return
+}
+```
+
+---
+
+## Info
+
+### IN-01: `UpdateTask` query lacks `tablo_id` in WHERE clause — task can be updated across tablos via race condition
+
+**File:** `backend/internal/db/queries/tasks.sql:17-21`
+
+**Issue:** The `UpdateTask` query filters only on `id`:
+
+```sql
+UPDATE tasks SET ... WHERE id = $1
+```
+
+There is no `AND tablo_id = $2` guard. The handler correctly fetches the task via `GetTaskByID` (which includes `AND tablo_id = $2`) before calling `UpdateTask`, providing application-layer isolation. However this means ownership enforcement is entirely in Go rather than in the SQL layer. A future handler that calls `UpdateTask` directly with a task ID obtained from an untrusted source would silently bypass the tablo-ownership check. The `DeleteTask` query correctly uses `AND tablo_id = $2` — `UpdateTask` should too.
+
+**Fix:**
+```sql
+-- name: UpdateTask :one
+UPDATE tasks
+SET title = $3, description = $4, status = $5, position = $6, updated_at = now()
+WHERE id = $1 AND tablo_id = $2
+RETURNING id, tablo_id, title, description, status, position, created_at, updated_at;
+```
+
+Update `UpdateTaskParams` and all callers accordingly.
+
+---
+
+### IN-02: Dead code — redundant `taskIDs` re-assignment in `TaskReorderHandler`
+
+**File:** `backend/internal/web/handlers_tasks.go:415`
+
+**Issue:** Inside the `len(taskCols) == 0` branch, lines 413-415 contain:
+
+```go
+if len(taskIDs) == 0 {
+    taskIDs = r.Form["task_id"]  // identical to line 406 — already assigned
+}
+```
+
+`taskIDs` was assigned `r.Form["task_id"]` at line 406 unconditionally. Re-reading the same form field inside the branch produces the same value. The inner `if` block is unreachable dead code.
+
+**Fix:** Remove lines 413-416:
+```go
+// Delete this block entirely:
+if len(taskIDs) == 0 {
+    taskIDs = r.Form["task_id"]
+}
+```
+
+---
+
+_Reviewed: 2026-05-15T00:00:00Z_
+_Reviewer: Claude (gsd-code-reviewer)_
+_Depth: standard_