liveview-code-review

# LiveView Code Review

## Quick Reference

| Issue Type | Reference |
|------------|-----------|
| mount, handle_params, handle_event, handle_async | [references/lifecycle.md](references/lifecycle.md) |
| When to use assigns vs streams, AsyncResult | [references/assigns-streams.md](references/assigns-streams.md) |
| Function vs LiveComponent, slots, attrs | [references/components.md](references/components.md) |
| Authorization per event, phx-value trust | [references/security.md](references/security.md) |

## Review Checklist

### Critical Issues
- [ ] No socket copying into async functions (extract values first)
- [ ] Every handle_event validates authorization
- [ ] No sensitive data in assigns (visible in DOM)
- [ ] phx-value data is validated (user-modifiable)

### Lifecycle
- [ ] Subscriptions wrapped in `connected?(socket)`
- [ ] handle_params used for URL-based state
- [ ] handle_async handles :loading and :error states

### Data Management
- [ ] Streams used for large collections (100+ items)
- [ ] temporary_assigns for data not needed after render
- [ ] AsyncResult patterns for loading states

### Components
- [ ] Function components preferred over LiveComponents
- [ ] LiveComponents preserve :inner_block in update/2
- [ ] Slots use proper attr declarations
- [ ] phx-debounce on text inputs

## Valid Patterns (Do NOT Flag)

- **Empty mount returning {:ok, socket}** - Valid for simple LiveViews
- **Using assigns for small lists** - Streams only needed for 100+ items
- **LiveComponent without update/2** - Default update/2 assigns all
- **phx-click without phx-value** - Event may not need data
- **Inline function in heex** - Valid for simple transforms

## Context-Sensitive Rules

| Issue | Flag ONLY IF |
|-------|--------------|
| Missing debounce | Input is text/textarea AND triggers server event |
| Use streams | Collection has 100+ items OR is paginated |
| Missing auth check | Event modifies data AND no auth in mount |

## Critical Anti-Patterns

### Socket Copying (MOST IMPORTANT)

```elixir
# BAD - socket copied into async function
def handle_event("load", _, socket) do
  Task.async(fn ->
    user = socket.assigns.user  # Socket copied!
    fetch_data(user.id)
  end)
  {:noreply, socket}
end

# GOOD - extract values first
def handle_event("load", _, socket) do
  user_id = socket.assigns.user.id
  Task.async(fn ->
    fetch_data(user_id)  # Only primitive copied
  end)
  {:noreply, socket}
end
```

### Missing Authorization

```elixir
# BAD - trusts phx-value without auth
def handle_event("delete", %{"id" => id}, socket) do
  Posts.delete_post!(id)  # Anyone can delete any post!
  {:noreply, socket}
end

# GOOD - verify authorization
def handle_event("delete", %{"id" => id}, socket) do
  post = Posts.get_post!(id)

  if post.user_id == socket.assigns.current_user.id do
    Posts.delete_post!(post)
    {:noreply, stream_delete(socket, :posts, post)}
  else
    {:noreply, put_flash(socket, :error, "Unauthorized")}
  end
end
```

## Hard gates (sequence)

Advance only when each **pass condition** is objectively true (prevents reporting without evidence):

| Gate | Pass condition |
|------|----------------|
| **G1 — Files in evidence** | You have an explicit list of paths under review (e.g. `*.ex`, `*.heex`, or the paths the user named). **Every** finding names a file from that list. |
| **G2 — Verification protocol** | You loaded [review-verification-protocol](../review-verification-protocol/SKILL.md) and applied its Pre-Report Verification (and issue-type sections where relevant) **before** treating something as a finding. |
| **G3 — Line anchors** | Each finding uses `[FILE:LINE]` where that line exists in the current file (confirmed by read/grep output, not inferred). |
| **G4 — Valid-pattern screen** | You checked the finding against **Valid Patterns (Do NOT Flag)** and **Context-Sensitive Rules**; if it matches a “do not flag” case or fails a “Flag ONLY IF,” you **do not** report it. |

## Issue format

Use `[FILE:LINE] ISSUE_TITLE` for each finding.