graham/towerops
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

docs: merge upstream Phoenix AGENTS.md with project-specific patterns

Browse source 
- Add module design/complexity guidelines, quality checks (credo, sobelow,
  coverage), security guidelines (deps.audit), updated auth section with
  live_session details, OTP guidelines, and expanded Elixir core rules
  from cheezy/kanban upstream AGENTS.md
- Move project-specific patterns into AGENTS.md: browser navigation URL
  state, JS hook memory leak prevention, LiveView form data access,
  SNMP test mocking patterns
- Trim CLAUDE.md to project architecture docs only, removing sections
  now covered in AGENTS.md
This commit is contained in:
Graham McIntire 2026-02-18 09:08:33 -06:00
parent b765d6d6ce
commit 076b8454de
No known key found for this signature in database
2 changed files with 250 additions and 251 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

291
AGENTS.md
View file

@ -4,6 +4,50 @@ This is a web application written using the Phoenix web framework.
- Use `mix precommit` alias when you are done with all changes and fix any pending issues
- Use the already included and available `:req` (`Req`) library for HTTP requests, **avoid** `:httpoison`, `:tesla`, and `:httpc`. Req is included by default and is the preferred HTTP client for Phoenix apps
- Always provide translations of all text that is visible in the UI
- Never put Ecto queries directly in LiveViews. Instead always put them in the appropriate context module
### Module design and complexity
**IMPORTANT**: When working with modules that are becoming too large or complex:
- **Monitor module size and complexity**: If a module exceeds ~500-600 lines or contains deeply nested logic with high cyclomatic complexity, consider refactoring
- **Break out logical concerns**: Extract related functionality into separate, focused modules that handle a single responsibility
- **Use helper modules**: For complex domains, consider creating dedicated modules for specific sub-concerns:
- Positioning logic (e.g., `Tasks.Positioning`)
- Dependency management (e.g., `Tasks.Dependencies`)
- Validation logic (e.g., `Tasks.Validation`)
- Query builders (e.g., `Tasks.Queries`)
- **Extract helper functions**: When a function becomes complex (cyclomatic complexity > 9), extract complex conditional logic into smaller, well-named helper functions
- **Maintain clear module boundaries**: Each module should have a clear, single purpose with a well-defined public API
- **Document module organization**: When splitting modules, update documentation to explain the new structure and how modules relate to each other
This approach improves:
- Code maintainability and readability
- Test isolation and coverage
- Collaboration between developers
- Ability to reason about individual components
- Credo compliance and code quality metrics
### Quality guidelines
**ALWAYS** follow these quality guidelines:
- **IMPORTANT**: When you complete a task that has new functions, write unit tests for the new function
- **IMPORTANT**: When you complete a task that updates code, make sure all existing unit tests pass and write new tests if needed
- Each time you write or update a unit test run them with `mix test` and ensure they pass
- **IMPORTANT**: When you complete a task run `mix test --cover` and ensure coverage is above the threshold
- **IMPORTANT**: When you complete a task run `mix credo --strict` to check for code quality issues and fix them
- **IMPORTANT**: Run `mix dialyzer` for static type analysis — never suppress warnings, fix root causes
- **IMPORTANT**: Run `mix precommit` before committing: compiles with warnings as errors, formats, runs tests
### Security guidelines
**ALWAYS** follow these security guidelines:
- **IMPORTANT**: When you add or update a dependency run `mix deps.audit` and `mix hex.audit` to check for security issues
- **IMPORTANT**: When you add or update a dependency run `mix hex.outdated` to check for outdated dependencies
- **IMPORTANT**: When you complete a task run `mix sobelow --config` to check for security issues and fix any issue
### Phoenix v1.8 guidelines
@ -26,7 +70,7 @@ custom classes must fully style the input
@import "tailwindcss" source(none);
@source "../css";
@source "../js";
@source "../../lib/my_app_web";
@source "../../lib/towerops_web";
- **Always use and maintain this import syntax** in the app.css file for projects generated with `phx.new`
- **Never** use `@apply` when writing raw css
@ -34,36 +78,58 @@ custom classes must fully style the input
- Out of the box **only the app.js and app.css bundles are supported**
- You cannot reference an external vendor'd script `src` or link `href` in the layouts
- You must import the vendor deps into app.js and app.css to use them
- **Never write inline <script>custom js</script> tags within templates**
- **Never write inline `<script>` tags within templates** — use LiveView hooks or colocated hook scripts
### UI/UX & design guidelines
- **Produce world-class UI designs** with a focus on usability, aesthetics, and modern design principles
- **Always follow existing application styles and patterns** when adding new UI elements
- Before creating custom styles, check `core_components.ex` for existing components like `<.input>`, `<.button>`, `<.form>`, etc.
- When adding form fields, use the standard component structure to match existing forms
- New buttons should use the `<.button>` component without custom classes unless specifically requested
- Implement **subtle micro-interactions** (e.g., button hover effects, and smooth transitions)
- Ensure **clean typography, spacing, and layout balance** for a refined, premium look
- Focus on **delightful details** like hover effects, loading states, and smooth page transitions
- Maintain consistency with existing color schemes, spacing, and typography throughout the application
<!-- phoenix-gen-auth-start -->
## Authentication
- **Always** handle authentication flow at the router level with proper redirects
- **Always** be mindful of where to place routes. `phx.gen.auth` creates multiple router plugs:
- **Always** be mindful of where to place routes. `phx.gen.auth` creates multiple router plugs and `live_session` scopes:
- A plug `:fetch_current_scope_for_user` that is included in the default browser pipeline
- A plug `:require_authenticated_user` that redirects to the log in page when the user is not authenticated
- In both cases, a `@current_scope` is assigned to the Plug connection
- A plug `redirect_if_user_is_authenticated` that redirects to a default path in case the user is authenticated - useful for a registration page that should only be shown to unauthenticated users
- **Always let the user know in which router scopes and pipeline you are placing the route, AND SAY WHY**
- A `live_session :current_user` scope - for routes that need the current user but don't require authentication
- A `live_session :require_authenticated_user` scope - for routes that require authentication
- In both cases, a `@current_scope` is assigned to the Plug connection and LiveView socket
- A plug `redirect_if_user_is_authenticated` that redirects to a default path in case the user is authenticated
- **Always let the user know in which router scopes, `live_session`, and pipeline you are placing the route, AND SAY WHY**
- `phx.gen.auth` assigns the `current_scope` assign - it **does not assign a `current_user` assign**
- Always pass the assign `current_scope` to context modules as first argument. When performing queries, use `current_scope.user` to filter the query results
- To derive/access `current_user` in templates, **always use the `@current_scope.user`**, never use **`@current_user`** in templates
- Anytime you hit `current_scope` errors or the logged in session isn't displaying the right content, **always double check the router and ensure you are using the correct plug as described below**
- To derive/access `current_user` in templates, **always use the `@current_scope.user`**, never use **`@current_user`** in templates or LiveViews
- **Never** duplicate `live_session` names. A `live_session :current_user` can only be defined __once__ in the router, so all routes for the `live_session :current_user` must be grouped in a single block
- Anytime you hit `current_scope` errors or the logged in session isn't displaying the right content, **always double check the router and ensure you are using the correct plug and `live_session` as described below**
### Routes that require authentication
LiveViews that require login should **always be placed inside the __existing__ `live_session :require_authenticated_user` block**:
scope "/", ToweropsWeb do
pipe_through [:browser, :require_authenticated_user]
live_session :require_authenticated_user,
on_mount: [{ToweropsWeb.UserAuth, :require_authenticated}] do
# phx.gen.auth generated routes
live "/users/settings", UserLive.Settings, :edit
# our own routes that require logged in user
live "/", MyLiveThatRequiresAuth, :index
end
end
Controller routes must be placed in a scope that sets the `:require_authenticated_user` plug:
scope "/", AppWeb do
scope "/", ToweropsWeb do
pipe_through [:browser, :require_authenticated_user]
get "/", MyControllerThatRequiresAuth, :index
@ -115,6 +181,34 @@ Controllers automatically have the `current_scope` available if they use the `:b
- Predicate function names should not start with `is_` and should end in a question mark. Names like `is_thing` should be reserved for guards
- Elixir's builtin OTP primitives like `DynamicSupervisor` and `Registry`, require names in the child spec, such as `{DynamicSupervisor, name: MyApp.MyDynamicSup}`, then you can use `DynamicSupervisor.start_child(MyApp.MyDynamicSup, child_spec)`
- Use `Task.async_stream(collection, callback, options)` for concurrent enumeration with back-pressure. The majority of times you will want to pass `timeout: :infinity` as option
- Elixir has no `return` statement, nor early returns. The last expression in a block is always returned
- Don't use `Enum` functions on large collections when `Stream` is more appropriate
- Avoid nested `case` statements — refactor to a single `case`, `with`, or separate functions
- Prefer `Enum` functions like `Enum.reduce` over manual recursion
- When recursion is necessary, prefer to use pattern matching in function heads for base case detection
- Use guard clauses: `when is_binary(name) and byte_size(name) > 0`
- Prefer multiple function clauses over complex conditional logic
- Use structs over maps when the shape is known: `defstruct [:name, :age]`
- Prefer to prepend to lists `[new | list]` rather than `list ++ [new]`
- Use `with` for chaining operations that return `{:ok, _}` or `{:error, _}`
- Use `{:ok, result}` and `{:error, reason}` tuples for operations that can fail
- Using the process dictionary is typically a sign of unidiomatic code
- Only use macros if explicitly requested
- Use `dbg/1` to print values while debugging — displays formatted value and context in console
## OTP guidelines
- Keep GenServer state simple and serializable
- Handle all expected messages explicitly
- Use `handle_continue/2` for post-init work
- Implement proper cleanup in `terminate/2` when necessary
- Use `GenServer.call/3` for synchronous requests expecting replies
- Use `GenServer.cast/2` for fire-and-forget messages — when in doubt, prefer `call` for back-pressure
- Set appropriate timeouts for `call/3` operations
- Set up processes such that they can handle crashing and being restarted by supervisors
- Use `:max_restarts` and `:max_seconds` to prevent restart loops
- Use `Task.Supervisor` for better fault tolerance in async tasks
- Handle task failures with `Task.yield/2` or `Task.shutdown/2`
## Mix guidelines
@ -132,6 +226,23 @@ Controllers automatically have the `current_scope` available if they use the `:b
assert_receive {:DOWN, ^ref, :process, ^pid, :normal}
- Instead of sleeping to synchronize before the next call, **always** use `_ = :sys.get_state/1` to ensure the process has handled prior messages
- Use `@tag` to tag specific tests, and `mix test --only tag` to run only those tests
- Use `assert_raise` for testing expected exceptions: `assert_raise ArgumentError, fn -> invalid_function() end`
- Limit the number of failed tests with `mix test --max-failures n`
- Use `DataCase` for database tests, `ConnCase` for controllers/LiveView
- Use `async: true` when possible
- Never disable tests instead of fixing them
## SNMP test mocking (project-specific)
- `snmp_adapter().get/3` returns `{:ok, value}` or `{:error, reason}`
- `snmp_adapter().walk/3` returns `{:ok, [%{oid: "...", value: ...}]}` (list, not map)
- Values already extracted (no type wrapper needed)
**Common pitfalls**:
- ❌ `walk` returning `{:ok, %{}}` instead of `{:ok, []}`
- ❌ Returning type wrappers like `{:integer, 123}`
- ❌ Not matching OIDs in get expectations
<!-- phoenix:elixir-end -->
<!-- phoenix:phoenix-start -->
@ -141,13 +252,13 @@ Controllers automatically have the `current_scope` available if they use the `:b
- You **never** need to create your own `alias` for route definitions! The `scope` provides the alias, ie:
scope "/admin", AppWeb.Admin do
scope "/admin", ToweropsWeb.Admin do
pipe_through :browser
live "/users", UserLive, :index
end
the UserLive route would point to the `AppWeb.Admin.UserLive` module
the UserLive route would point to the `ToweropsWeb.Admin.UserLive` module
- `Phoenix.View` no longer is needed or included with Phoenix, don't use it
<!-- phoenix:phoenix-end -->
@ -158,10 +269,18 @@ Controllers automatically have the `current_scope` available if they use the `:b
- **Always** preload Ecto associations in queries when they'll be accessed in templates, ie a message that needs to reference the `message.user.email`
- Remember `import Ecto.Query` and other supporting modules when you write `seeds.exs`
- `Ecto.Schema` fields always use the `:string` type, even for `:text`, columns, ie: `field :name, :string`
- `Ecto.Changeset.validate_number/2` **DOES NOT SUPPORT the `:allow_nil` option**. By default, Ecto validations only run if a change for the given field exists and the change value is not nil, so such as option is never needed
- `Ecto.Changeset.validate_number/2` **DOES NOT SUPPORT the `:allow_nil` option**. By default, Ecto validations only run if a change for the given field exists and the change value is not nil, so such an option is never needed
- You **must** use `Ecto.Changeset.get_field(changeset, :field)` to access changeset fields
- Fields which are set programatically, such as `user_id`, must not be listed in `cast` calls or similar for security purposes. Instead they must be explicitly set when creating the struct
- **Always** invoke `mix ecto.gen.migration migration_name_using_underscores` when generating migration files, so the correct timestamp and conventions are applied
- **Binary UUID Primary Keys**: All tables MUST use `:binary_id`, not `bigint`:
create table(:table_name, primary_key: false) do
add :id, :binary_id, primary_key: true
end
- **SNMP Binary Data**: Convert to printable strings before saving. Non-printable bytes → colon-separated hex.
See `lib/towerops/snmp/neighbor_discovery.ex:sanitize_string_field/1`.
<!-- phoenix:ecto-end -->
<!-- phoenix:html-start -->
@ -171,7 +290,7 @@ Controllers automatically have the `current_scope` available if they use the `:b
- **Always** use the imported `Phoenix.Component.form/1` and `Phoenix.Component.inputs_for/1` function to build forms. **Never** use `Phoenix.HTML.form_for` or `Phoenix.HTML.inputs_for` as they are outdated
- When building forms **always** use the already imported `Phoenix.Component.to_form/2` (`assign(socket, form: to_form(...))` and `<.form for={@form} id="msg-form">`), then access those forms in the template via `@form[:field]`
- **Always** add unique DOM IDs to key elements (like forms, buttons, etc) when writing templates, these IDs can later be used in tests (`<.form for={@form} id="product-form">`)
- For "app wide" template imports, you can import/alias into the `my_app_web.ex`'s `html_helpers` block, so they will be available to all LiveViews, LiveComponent's, and all modules that do `use MyAppWeb, :html` (replace "my_app" by the actual app name)
- For "app wide" template imports, you can import/alias into the `towerops_web.ex`'s `html_helpers` block, so they will be available to all LiveViews, LiveComponents, and all modules that do `use ToweropsWeb, :html`
- Elixir supports `if/else` but **does NOT support `if/else if` or `if/elsif`**. **Never use `else if` or `elseif` in Elixir**, **always** use `cond` or `case` for multiple conditionals.
@ -246,13 +365,63 @@ Controllers automatically have the `current_scope` available if they use the `:b
<!-- phoenix:liveview-start -->
## Phoenix LiveView guidelines
- **Never** use the deprecated `live_redirect` and `live_patch` functions, instead **always** use the `<.link navigate={href}>` and `<.link patch={href}>` in templates, and `push_navigate` and `push_patch` functions LiveViews
- **Avoid LiveComponent's** unless you have a strong, specific need for them
- LiveViews should be named like `AppWeb.WeatherLive`, with a `Live` suffix. When you go to add LiveView routes to the router, the default `:browser` scope is **already aliased** with the `AppWeb` module, so you can just do `live "/weather", WeatherLive`
- **Never** use the deprecated `live_redirect` and `live_patch` functions, instead **always** use the `<.link navigate={href}>` and `<.link patch={href}>` in templates, and `push_navigate` and `push_patch` functions in LiveViews
- **Avoid LiveComponents** unless you have a strong, specific need for them
- LiveViews should be named like `ToweropsWeb.WeatherLive`, with a `Live` suffix. When you go to add LiveView routes to the router, the default `:browser` scope is **already aliased** with the `ToweropsWeb` module, so you can just do `live "/weather", WeatherLive`
### Browser Navigation and URL State
**Critical**: All LiveViews MUST sync UI state to URL for browser back/forward buttons.
**Core Principle**: The URL is the single source of truth for visible state (tabs, modals, filters, pagination).
**Pattern**: Use `push_patch/2` and `handle_params/3`:
```elixir
# Tab navigation
def handle_params(params, _url, socket) do
tab = case params["tab"] do
tab when tab in ~w(overview sensors) -> tab
_ -> "overview" # Safe default
end
{:noreply, assign(socket, :active_tab, tab) |> load_tab_data(tab)}
end
def handle_event("select_tab", %{"tab" => tab}, socket) do
{:noreply, push_patch(socket, to: ~p"/devices/#{socket.assigns.device.id}?tab=#{tab}")}
end
```
Template:
```heex
<.link patch={~p"/devices/#{@device.id}?tab=overview"}
class={if @active_tab == "overview", do: "active"}>Overview</.link>
```
**Modal state**:
```elixir
def handle_params(params, _url, socket) do
modal = params["modal"]
{:noreply, assign(socket, show_modal: modal != nil, modal_name: modal)}
end
def handle_event("show_modal", _, socket) do
{:noreply, push_patch(socket, to: ~p"/settings?modal=add_token")}
end
# Close modal by removing ?modal param
{:noreply, push_patch(socket, to: ~p"/settings")}
```
**Rules**:
1. Every LiveView implements `handle_params/3` — ONLY place to read URL params
2. Validate all params, provide safe defaults
3. Never use `assign` for URL-backed state — use `push_patch`
4. Test browser back/forward buttons
### LiveView streams
- **Always** use LiveView streams for collections for assigning regular lists to avoid memory ballooning and runtime termination with the following operations:
- **Always** use LiveView streams for collections to avoid memory ballooning with the following operations:
- basic append of N items - `stream(socket, :messages, [new_msg])`
- resetting stream with new items - `stream(socket, :messages, [new_msg], reset: true)` (e.g. for filtering items)
- prepend to stream - `stream(socket, :messages, [new_msg], at: -1)`
@ -269,13 +438,11 @@ Controllers automatically have the `current_scope` available if they use the `:b
- LiveView streams are *not* enumerable, so you cannot use `Enum.filter/2` or `Enum.reject/2` on them. Instead, if you want to filter, prune, or refresh a list of items on the UI, you **must refetch the data and re-stream the entire stream collection, passing reset: true**:
def handle_event("filter", %{"filter" => filter}, socket) do
# re-fetch the messages based on the filter
messages = list_messages(filter)
{:noreply,
socket
|> assign(:messages_empty?, messages == [])
# reset the stream with the new messages
|> stream(:messages, messages, reset: true)}
end
@ -288,43 +455,15 @@ Controllers automatically have the `current_scope` available if they use the `:b
</div>
</div>
The above only works if the empty state is the only HTML block alongside the stream for-comprehension.
- When updating an assign that should change content inside any streamed item(s), you MUST re-stream the items
along with the updated assign:
def handle_event("edit_message", %{"message_id" => message_id}, socket) do
message = Chat.get_message!(message_id)
edit_form = to_form(Chat.change_message(message, %{content: message.content}))
# re-insert message so @editing_message_id toggle logic takes effect for that stream item
{:noreply,
socket
|> stream_insert(:messages, message)
|> assign(:editing_message_id, String.to_integer(message_id))
|> assign(:edit_form, edit_form)}
end
And in the template:
<div id="messages" phx-update="stream">
<div :for={{id, message} <- @streams.messages} id={id} class="flex group">
{message.username}
<%= if @editing_message_id == message.id do %>
<%!-- Edit mode --%>
<.form for={@edit_form} id="edit-form-#{message.id}" phx-submit="save_edit">
...
</.form>
<% end %>
</div>
</div>
along with the updated assign.
- **Never** use the deprecated `phx-update="append"` or `phx-update="prepend"` for collections
### LiveView JavaScript interop
- Remember anytime you use `phx-hook="MyHook"` and that JS hook manages its own DOM, you **must** also set the `phx-update="ignore"` attribute
- **Always** provide an unique DOM id alongside `phx-hook` otherwise a compiler error will be raised
- **Always** provide a unique DOM id alongside `phx-hook` otherwise a compiler error will be raised
LiveView hooks come in two flavors, 1) colocated js hooks for "inline" scripts defined inside HEEx,
and 2) external `phx-hook` annotations where JavaScript object literals are defined and passed to the `LiveSocket` constructor.
@ -364,6 +503,33 @@ LiveSocket constructor:
hooks: { MyHook }
});
#### Memory leak prevention in JS hooks
Always remove event listeners in `destroyed()`:
```typescript
const MyHook = {
handleClick: null as ((e: Event) => void) | null,
mounted(this: any) {
this.handleClick = (e: Event) => { /* logic */ }
this.el.addEventListener("click", this.handleClick)
},
destroyed(this: any) {
if (this.handleClick) {
this.el.removeEventListener("click", this.handleClick)
this.handleClick = null
}
}
}
```
- Clean up third-party instances (Chart.js, Cytoscape, etc.) in `destroyed()`
- Store event handler references — never pass anonymous functions to `addEventListener`
- Set hook properties to `null` in `destroyed()`
- Phoenix.PubSub auto-cleans when LiveView terminates — no manual cleanup needed
#### Pushing events between client and server
Use LiveView's `push_event/3` when you need to push events/data to the client for a phx-hook to handle.
@ -403,7 +569,7 @@ Where the server handled it via:
- Form tests are driven by `Phoenix.LiveViewTest`'s `render_submit/2` and `render_change/2` functions
- Come up with a step-by-step test plan that splits major test cases into small, isolated files. You may start with simpler tests that verify content exists, gradually add interaction tests
- **Always reference the key element IDs you added in the LiveView templates in your tests** for `Phoenix.LiveViewTest` functions like `element/2`, `has_element/2`, selectors, etc
- **Never** tests again raw HTML, **always** use `element/2`, `has_element/2`, and similar: `assert has_element?(view, "#my-form")`
- **Never** test against raw HTML, **always** use `element/2`, `has_element/2`, and similar: `assert has_element?(view, "#my-form")`
- Instead of relying on testing text content, which can change, favor testing for the presence of key elements
- Focus on testing outcomes rather than implementation details
- Be aware that `Phoenix.Component` functions like `<.form>` might produce different HTML than expected. Test against the output HTML structure, not your mental model of what you expect it to be
@ -436,20 +602,20 @@ You can also specify a name to nest the params:
When using changesets, the underlying data, form params, and errors are retrieved from it. The `:as` option is automatically computed too. E.g. if you have a user schema:
defmodule MyApp.Users.User do
defmodule Towerops.Users.User do
use Ecto.Schema
...
end
And then you create a changeset that you pass to `to_form`:
%MyApp.Users.User{}
%Towerops.Users.User{}
|> Ecto.Changeset.change()
|> to_form()
Once the form is submitted, the params will be available under `%{"user" => user_params}`.
In the template, the form form assign can be passed to the `<.form>` function component:
In the template, the form assign can be passed to the `<.form>` function component:
<.form for={@form} id="todo-form" phx-change="validate" phx-submit="save">
<.input field={@form[:field]} type="text" />
@ -457,6 +623,23 @@ In the template, the form form assign can be passed to the `<.form>` function co
Always give the form an explicit, unique DOM ID, like `id="todo-form"`.
#### Accessing form data from LiveView events
**Problem**: `form.params` is empty until `phx-change="validate"` fires.
**Solution**: Access current values from changeset:
```elixir
def handle_event("test_connection", _params, socket) do
changeset = socket.assigns.form.source
form_data = Ecto.Changeset.apply_changes(changeset) # Returns struct with all values
# Convert to map if needed
device_map = %{"ip_address" => form_data.ip_address, ...}
end
```
This combines: DB values + default values + user changes.
#### Avoiding form errors
**Always** use a form assigned via `to_form/2` in the LiveView, and the `<.input>` component in the template. In the template **always access forms this**:
@ -477,4 +660,4 @@ And **never** do this:
- **Never** use `<.form let={f} ...>` in the template, instead **always use `<.form for={@form} ...>`**, then drive all form references from the form assign as in `@form[:field]`. The UI should **always** be driven by a `to_form/2` assigned in the LiveView module that is derived from a changeset
<!-- phoenix:liveview-end -->
<!-- usage-rules-end -->
<!-- usage-rules-end -->

210
CLAUDE.md
View file

@ -6,10 +6,10 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
**Before starting any work in this repository**, you MUST read `AGENTS.md` in the project root.
- `AGENTS.md` contains comprehensive Phoenix/LiveView/Elixir guidelines that are mandatory for this project
- `AGENTS.md` contains mandatory Phoenix/LiveView/Elixir coding guidelines AND project-specific patterns for this codebase
- Covers: Elixir/OTP conventions, Phoenix/LiveView best practices, browser navigation URL state, JS hook memory management, form handling patterns, test guidelines, quality/security checks
- These guidelines take precedence over general development practices when there's a conflict
- Always read `AGENTS.md` at the start of a new conversation or when resuming work
- Follow the project-specific patterns, conventions, and constraints documented there
## Project Overview
@ -194,30 +194,13 @@ Note: SnmpKit still used for SNMP protocol operations (get, walk, etc.).
## Project-Specific Constraints
From AGENTS.md (see that file for details):
- Use `mix precommit` before committing
- Use `:req` (Req) for HTTP - never `:httpoison`, `:tesla`, `:httpc`
- Never use `daisyUI` - custom Tailwind components
- LiveView templates start with `<Layouts.app flash={@flash}>`
- Use `<.icon name="hero-x-mark">` for icons
- Use LiveView streams for large collections
- Forms: `to_form/2` in LiveView, `<.form for={@form}>` in templates
- Never access changesets directly in templates - use form assign
- Tailwind v4: new import syntax, never `@apply`
See `AGENTS.md` for the full set of coding constraints. Key reminders:
- Use `mix precommit` before committing (formats, compiles with warnings-as-errors, runs tests)
- Use `:req` (Req) for HTTP — never `:httpoison`, `:tesla`, `:httpc`
- Never use `daisyUI` — custom Tailwind components only
- Never put Ecto queries directly in LiveViews — use context modules
- Always run `mix format` after Elixir changes
### Database Schema Critical Notes
**Binary UUID Primary Keys**: All tables MUST use `:binary_id`, not `bigint`:
```elixir
create table(:table_name, primary_key: false) do
add :id, :binary_id, primary_key: true
end
```
**SNMP Binary Data**: Convert to printable strings before saving. Non-printable → colon-separated hex.
See `lib/towerops/snmp/neighbor_discovery.ex:sanitize_string_field/1`.
### API Documentation
- Available at `/docs/api` (Tailwind UI Protocol template)
@ -292,186 +275,19 @@ Template:
For large datasets (>10K), use database-level `limit/offset` instead of in-memory slicing.
### Browser Navigation and URL State
## Testing Notes
**Critical**: All LiveViews MUST sync UI state to URL for browser back/forward buttons.
**Core Principle**: URL is single source of truth for visible state (tabs, modals, filters, pagination).
**Pattern**: Use `push_patch/2` and `handle_params/3`:
```elixir
# Tab navigation
def handle_params(params, _url, socket) do
tab = case params["tab"] do
tab when tab in ~w(overview sensors) -> tab
_ -> "overview" # Safe default
end
{:noreply, assign(socket, :active_tab, tab) |> load_tab_data(tab)}
end
def handle_event("select_tab", %{"tab" => tab}, socket) do
{:noreply, push_patch(socket, to: ~p"/devices/#{socket.assigns.device.id}?tab=#{tab}")}
end
```
Template:
```heex
<.link patch={~p"/devices/#{@device.id}?tab=overview"}
class={if @active_tab == "overview", do: "active"}>Overview</.link>
```
**Modal state**:
```elixir
def handle_params(params, _url, socket) do
modal = params["modal"]
{:noreply, assign(socket, show_modal: modal != nil, modal_name: modal)}
end
def handle_event("show_modal", _, socket) do
{:noreply, push_patch(socket, to: ~p"/settings?modal=add_token")}
end
# Close modal by removing ?modal param
{:noreply, push_patch(socket, to: ~p"/settings")}
```
**Rules**:
1. Every LiveView implements `handle_params/3` - ONLY place to read URL params
2. Validate all params, provide defaults
3. Never use `assign` for URL-backed state - use `push_patch`
4. Test browser back/forward buttons
**Achieves**: Browser navigation, bookmarkable URLs, deep linking, refresh preservation.
Reference: `docs/plans/2026-02-12-browser-navigation-fix-design.md`
## Testing Patterns
### SNMP Mocking with Mox
- `snmp_adapter().get/3` returns `{:ok, value}` or `{:error, reason}`
- `snmp_adapter().walk/3` returns `{:ok, [%{oid: "...", value: ...}]}` (list, not map)
- Values already extracted (no type wrapper needed)
**Common Pitfalls**:
- ❌ `walk` returning `{:ok, %{}}` instead of `{:ok, []}`
- ❌ Returning type wrappers like `{:integer, 123}`
- ❌ Not matching OIDs in get expectations
### Test Organization
- `DataCase` for database tests, `ConnCase` for controllers/LiveView
- `async: true` when possible
- Never open test coverage HTML files
- Never use npm - esbuild is built into Phoenix
- Never use npm — esbuild is built into Phoenix
- Run `cargo fmt` before committing Rust changes
- Never open test coverage HTML files — read results in terminal
- See `AGENTS.md` for full test guidelines, SNMP mocking patterns, and LiveView test helpers
## Dialyzer
- `mix dialyzer` - Run analysis (builds PLT first time)
- `mix dialyzer --format dialyzer` - Detailed error locations
- Start with broad types, narrow gradually
- Never suppress valid warnings - fix root cause
- PLT files in `priv/plts/` - don't commit
## Memory Leak Prevention
### JavaScript Hook Cleanup
Always remove event listeners in `destroyed()`:
```typescript
const MyHook = {
handleClick: null as ((e: Event) => void) | null,
mounted(this: any) {
this.handleClick = (e: Event) => { /* logic */ }
this.el.addEventListener("click", this.handleClick)
},
destroyed(this: any) {
if (this.handleClick) {
this.el.removeEventListener("click", this.handleClick)
this.handleClick = null
}
}
}
```
Clean up third-party instances (Chart.js, Cytoscape) in `destroyed()`.
### LiveView Patterns
**Use streams for**:
- Lists with hundreds/thousands of items
- Lists with frequent PubSub updates
- When only adding/updating/removing individual items
**Example**:
```elixir
# Mount
{:ok, stream(socket, :alerts, load_all_alerts())}
# Add item
stream_insert(socket, :alerts, alert, at: 0)
# Remove item
stream_delete(socket, :alerts, alert)
```
Template:
```heex
<div id="alerts" phx-update="stream">
<%= for {id, alert} <- @streams.alerts do %>
<div id={id}>{alert.message}</div>
<% end %>
</div>
```
**Use pagination for**: Audit logs, historical alerts, any list that could grow to hundreds of items.
### LiveView Form Data Access
**Problem**: `form.params` is empty until `phx-change="validate"` fires.
**Solution**: Access current values from changeset:
```elixir
def handle_event("test_connection", _params, socket) do
changeset = socket.assigns.form.source
form_data = Ecto.Changeset.apply_changes(changeset) # Returns struct with all values
# Convert to map if needed
device_map = %{"ip_address" => form_data.ip_address, ...}
end
```
Combines: DB values + default values + user changes.
### Phoenix.PubSub
Phoenix automatically unsubscribes when LiveView terminates - no manual cleanup needed.
### Global Event Listeners
Safe patterns (already correct):
- `document.addEventListener('DOMContentLoaded', ...)` - Once per page load
- `window.addEventListener("phx:page-loading-start", ...)` - Global app events
LiveView patch navigation doesn't reload page, so no duplicates.
### Checklist
**LiveView**:
- [ ] Use `stream/3` for large/frequently-updated lists
- [ ] Use pagination for lists that could grow large
- [ ] PubSub auto-cleans (no action needed)
**JavaScript Hooks**:
- [ ] Every `addEventListener` has matching `removeEventListener` in `destroyed()`
- [ ] Store event handler references (no anonymous functions)
- [ ] Clean up third-party instances in `destroyed()`
- [ ] Set hook properties to `null` in `destroyed()`
- Never suppress valid warnings — fix root cause
- PLT files in `priv/plts/` — don't commit
## Changelog