diff --git a/findings.md b/findings.md
new file mode 100644
index 00000000..f5bbd2b0
--- /dev/null
+++ b/findings.md
@@ -0,0 +1,303 @@
+# Codebase Findings
+
+## HIGH
+
+### H1. Hardcoded email bypasses security validation
+
+**File**: `lib/towerops_web/live/device_live/form.ex:779`
+
+```elixir
+assigns.current_scope.user.email == "graham@mcintire.me"
+```
+
+A personal email address is hardcoded to bypass the non-routable IP check for cloud pollers. If anyone registers with this email, they bypass IP validation. Should use a proper configuration flag or user attribute (e.g., `developer?` boolean).
+
+---
+
+### H2. `String.to_atom/1` on user-supplied parameter keys
+
+**File**: `lib/towerops_web/controllers/api/v1/integrations_controller.ex:127`
+
+```elixir
+{String.to_atom(k), v}
+```
+
+Converts user-supplied parameter keys directly to atoms. This is an atom table DoS vector — an attacker can send requests with unique parameter names and exhaust the VM atom table. Use `String.to_existing_atom/1` or a whitelist-based approach.
+
+---
+
+### H3. Missing `@impl true` on callback implementations
+
+Multiple `handle_event` and `handle_info` callbacks are missing `@impl true`. The project compiles with `--warnings-as-errors`, so this will cause compilation failures.
+
+**`lib/towerops_web/live/alert_live/index.ex`** — missing `@impl true` on:
+- Line 65: `handle_event("resolve", ...)`
+- Line 89: `handle_event("toggle_alert", ...)`
+- Line 94: `handle_event("toggle_select", ...)`
+- Line 105: `handle_event("select_all", ...)`
+- Line 110: `handle_event("select_none", ...)`
+- Line 114: `handle_event("bulk_acknowledge", ...)`
+- Line 135: `handle_event("bulk_resolve", ...)`
+- Line 155: `handle_event("toggle_site", ...)`
+- Line 194: `handle_info(_msg, socket)` (catch-all)
+
+**`lib/towerops_web/live/dashboard_live.ex:87`** — missing `@impl true` on:
+- Line 87: `handle_info({:alert_changed, _org_id}, socket)`
+
+---
+
+## MEDIUM
+
+### M1. Direct `changeset.changes` access instead of `Ecto.Changeset.get_field/2`
+
+**File**: `lib/towerops_web/live/device_live/form.ex:321,352,383`
+
+The project guidelines explicitly require using `Ecto.Changeset.get_field/2` to access changeset fields, but `changeset.changes` is accessed directly:
+- Line 321: `build_device_snmpv3_creds(form_data, changeset.changes)`
+- Line 352: `Map.get(changes, :snmpv3_auth_password)`
+- Line 383: `changes = changeset.changes`
+
+Also in `lib/towerops/admin.ex:241`: `changes: changeset.changes`
+
+---
+
+### M2. N+1 query: 4 separate count queries for alert counts
+
+**File**: `lib/towerops_web/live/alert_live/index.ex:207-211`
+
+```elixir
+counts = %{
+  all: Alerts.count_organization_alerts(organization_id),
+  critical: Alerts.count_organization_alerts(organization_id, "critical"),
+  unresolved: Alerts.count_organization_alerts(organization_id, "unresolved"),
+  resolved: Alerts.count_organization_alerts(organization_id, "resolved")
+}
+```
+
+Each call executes a separate `SELECT COUNT(*)`. A single query with `COUNT(*) FILTER(WHERE ...)` or `CASE WHEN` aggregation could return all counts in one roundtrip.
+
+---
+
+### M3. N+1 in bulk alert operations
+
+**File**: `lib/towerops_web/live/alert_live/index.ex:118-126` (bulk_acknowledge), `139-146` (bulk_resolve)
+
+```elixir
+Enum.reduce(socket.assigns.selected_alerts, 0, fn id, acc ->
+  with {:ok, alert} <- AccessControl.verify_alert_access(id, organization.id),
+       {:ok, _} <- Alerts.acknowledge_alert(alert, user_id) do
+    acc + 1
+  end
+end)
+```
+
+If a user selects 100 alerts, this makes 100 `verify_alert_access` queries + 100 `acknowledge_alert` calls. A single `WHERE id IN (^ids)` query would be far more efficient.
+
+---
+
+### M4. No URL parameter validation in `DashboardLive.handle_params`
+
+**File**: `lib/towerops_web/live/dashboard_live.ex:37`
+
+```elixir
+insight_source = params["insight_source"]
+```
+
+The `insight_source` param is assigned directly without validation against allowed values (`preseem`, `snmp`, `gaiia`, `system`). While downstream code likely handles invalid values gracefully, this is inconsistent with the project's URL-sync guidelines.
+
+---
+
+### M5. `Ecto.UUID.dump/1` pattern match can crash
+
+**File**: `lib/towerops_web/live/device_live/show.ex:938-939`
+
+```elixir
+{:ok, iid} = Ecto.UUID.dump(interface_id)
+{:ok, oid} = Ecto.UUID.dump(org_id)
+```
+
+If `interface_id` or `org_id` is not a valid UUID (e.g., race condition where device was deleted between access check and this call), `dump/1` returns `:error`, causing a `MatchError` crash.
+
+---
+
+### M6. `Repo.query!` in LiveView context
+
+**File**: `lib/towerops_web/live/device_live/show.ex:942`
+
+```elixir
+result = Towerops.Repo.query!(...)
+```
+
+The `!` variant raises on any database error, crashing the LiveView process. A non-bang variant with proper error handling would be more resilient.
+
+---
+
+### M7. Smoke tests use wrong query parameter names
+
+**File**: `test/towerops_web/live/smoke_test.exs:174,187`
+
+```elixir
+test "GET /alerts with query filter", %{conn: conn} do
+  assert {:ok, _view, _html} = live(conn, ~p"/alerts?severity=critical")
+end
+```
+
+The test uses `severity=critical` but `AlertLive.Index.handle_params/3` reads `filter`, not `severity`. Similarly, line 187 uses `?status=resolved` but the handler reads `filter=resolved`. Tests pass because `live/2` just mounts, but the params are silently ignored.
+
+---
+
+### M8. Double query in token deletion
+
+**File**: `lib/towerops/accounts.ex:389-390`
+
+```elixir
+tokens_to_expire = Repo.all(from t in UserToken, where: t.user_id == ^user.id)
+Repo.delete_all(from(t in UserToken, where: t.id in ^Enum.map(tokens_to_expire, & &1.id)))
+```
+
+This SELECTs all tokens just to extract IDs, then DELETEs by those IDs. A single `Repo.delete_all` with the user_id condition suffices.
+
+---
+
+### M9. N+1 risk: `membership.user` without preload
+
+**File**: `lib/towerops/billing/billing_notifier.ex:52`
+
+```elixir
+memberships |> Enum.map(fn m -> m.user.email end)
+```
+
+If `membership` records are not preloaded with `:user`, each access triggers a separate query.
+
+Also in `lib/towerops_web/controllers/api/v1/members_controller.ex:96` and `lib/towerops_web/graphql/types/member.ex:15`.
+
+---
+
+### M10. Process dictionary for cross-component state
+
+**Files**: 
+- `lib/towerops_web/user_auth.ex:611,1085` — `Process.put(:unresolved_alert_count, ...)`
+- `lib/towerops_web/components/layouts.ex:124` — `Process.get(:unresolved_alert_count, 0)`
+
+The `:unresolved_alert_count` is stored in the process dictionary and read from a layout component. The project guidelines state "Using the process dictionary is typically a sign of unidiomatic code." A socket assign would be cleaner and less fragile.
+
+---
+
+### M11. Many `list_*` functions lack limits
+
+At least 18 `list_*` functions across context modules return `Repo.all` results without any limit, risking unbounded result sets:
+
+- `lib/towerops/reports.ex:19` — `list_reports/1`
+- `lib/towerops/sites.ex` — `list_sites_with_coordinates/0`, `list_organization_sites/1`, `list_root_sites/1`
+- `lib/towerops/devices.ex` — `list_site_devices/1`, `list_monitored_devices/0`, `list_snmp_enabled_devices/0`, `list_mikrotik_devices_with_api/0`
+- `lib/towerops/agents.ex` — `list_all_agent_tokens/0`, `list_cloud_pollers/0`, `list_updatable_agents/0`, `list_online_cloud_pollers/0`, `list_cloud_polled_devices/0`
+- `lib/towerops/organizations.ex` — `list_organization_ids/0`, `list_user_organizations/1`, `list_organizations_with_active_subscriptions/0`
+- `lib/towerops/monitoring.ex` — `list_checks/2`, `list_checks_for_agent/2`
+
+---
+
+### M12. `DashboardLive` never unsubscribes from PubSub
+
+**File**: `lib/towerops_web/live/dashboard_live.ex:18-22`
+
+`DashboardLive` subscribes to PubSub topics in `mount` but has no cleanup. Phoenix auto-cleans subscriptions on process termination, but if the LiveView is reused across navigations within the same process (LiveView's default behavior), subscriptions accumulate.
+
+---
+
+### M13. Missing `@doc false` or unnecessary `@doc false` on public functions
+
+**File**: `lib/towerops_web/live/alert_live/index.ex`
+
+Functions like `filter_alerts/2`, `sort_alerts/3`, `severity_weight/1`, `severity_color/1`, `age_severity_color/1`, `format_age_minutes/1`, `format_number/1`, `duration_text/1`, `format_duration_minutes/1`, `get_subscriber_count/2`, `get_site_subscriber_count/1` are marked `@doc false` but are used only within the module. These should be `defp` instead of `def`.
+
+---
+
+## LOW
+
+### L1. `then/2` used for socket rebind instead of explicit assignment
+
+**File**: `lib/towerops_web/live/schedule_live/index.ex:188`
+
+```elixir
+|> then(&{:noreply, &1})
+```
+
+This is a style issue — explicit assignment would be clearer.
+
+---
+
+### L2. `Process.sleep(2000)` in application startup
+
+**File**: `lib/towerops/application.ex:165`
+
+A 2-second `Process.sleep` in `post_startup` before running `JobCleanupTask` delays application startup for every environment, including production restarts.
+
+---
+
+### L3. `Process.sleep` in device deletion
+
+**File**: `lib/towerops/devices.ex:878`
+
+```elixir
+Process.sleep(deletion_delay_ms)
+```
+
+Sleeping during a device deletion blocks the calling process. If this is called from a LiveView or web request, it blocks the process for the duration of the sleep.
+
+---
+
+### L4. `Scope.for_user(nil)` returns `nil`
+
+**File**: `lib/towerops/accounts/scope.ex:39`
+
+```elixir
+def for_user(nil), do: nil
+```
+
+This means `socket.assigns.current_scope` can be `nil` for unauthenticated users. Most callers handle this with `&&` guards, but it's a fragile pattern — one nil access on `.user` and the LiveView crashes.
+
+---
+
+
+
+### L6. `favicon_status/2` in DashboardLive unused argument
+
+**File**: `lib/towerops_web/live/dashboard_live.ex:267`
+
+```elixir
+defp favicon_status(devices_down, _alert_count) when devices_down > 0, do: "red"
+```
+
+The second argument `_alert_count` is unused in all but the middle clause, suggesting the function could be simplified.
+
+---
+
+## INFO
+
+### I1. 55 Oban worker modules — consider organizing
+
+**Pattern**: The workers directory has 55 modules. Some have grown large (`discovery_worker.ex` at 522 lines). Consider splitting into subdirectories by domain (monitoring, sync, maintenance, etc.).
+
+### I2. Heavy dashboard data load on every navigation
+
+**File**: `lib/towerops_web/live/dashboard_live.ex:109-187`
+
+`load_dashboard_data/2` fetches alerts, devices, sites, subscribers, impact data, incidents, activity feed, insights, config changes, and a setup checklist on every `handle_params`. For heavily used routes, consider caching or lazy loading some sections.
+
+### I3. SNMP discovery module is very large
+
+**File**: `lib/towerops/snmp/discovery.ex` — 1745+ lines
+
+This single module is the largest in the codebase. Per the project guidelines on module complexity, it should be broken into focused sub-modules.
+
+### I4. `device_live/show.ex` is very large
+
+The device show LiveView is likely the largest LiveView in the codebase. Consider extracting helper modules for chart builders, data loaders, and formatters (some already exist in `live/device_live/helpers/`).
+
+### I5. `.dialyzer_ignore.exs` is clean
+
+Only dependency and vendored-code suppressions — no app-level warnings are suppressed. Good practice.
+
+### I6. No bare `rescue _ ->` catch-all blocks
+
+The codebase properly avoids silent error swallowing. Good practice.