towerops/results.md

Codebase Optimization & Refactoring Review

1. Module Decomposition

Several context modules have grown beyond manageable sizes, making them difficult to navigate and maintain.

• Towerops.Snmp (~3000 lines): This module is acting as a "God Context" for all things SNMP.
  • Recommendation: Split into sub-contexts like Towerops.Snmp.Discovery, Towerops.Snmp.Queries, and Towerops.Snmp.Monitoring.
• Towerops.Accounts (~1900 lines): Handles everything from user registration to TOTP device management.
  • Recommendation: Move TOTP logic to Towerops.Accounts.TOTP and session management to Towerops.Accounts.Sessions.
• Towerops.Topology (~1600 lines): Contains complex graph inference logic alongside standard CRUD.
  • Recommendation: Extract the inference engine logic into Towerops.Topology.InferenceEngine.

2. DRY Refactoring Opportunities

SNMP Check Creation

In Towerops.Snmp.create_checks_from_discovery, there is a repetitive pattern for creating checks for sensors, interfaces, processors, and storage.

# Current Pattern
results = Enum.reduce(snmp_device.sensors, results, fn sensor, acc ->
  case create_sensor_check(device, sensor) do
    {:ok, _check} -> Map.update!(acc, :sensors, &(&1 + 1))
    {:error, reason} -> Map.update!(acc, :errors, &[{:sensor, sensor.id, reason} | &1])
  end
end)

Recommendation: Create a generic create_discovered_checks(device, entities, type, check_fun) helper to reduce boilerplate.

Topology Evidence Collection

collect_lldp_evidence, collect_cdp_evidence, collect_mac_evidence, and collect_arp_evidence all perform similar transformations.

Recommendation: Standardize the evidence collection pipeline. Use a common internal function that takes a query and a transformation mapper.

Ecto Query Repetition

Many modules repeat basic query logic like where(organization_id: ^org_id).

Recommendation: Implement "Query" modules for each schema (e.g., Towerops.Devices.DeviceQuery) containing reusable query fragments that can be composed.

3. Functional Pattern Matching vs. Conditionals

Replace cond with Pattern Matching

In Towerops.Topology.infer_role_from_capabilities, a cond block is used to match strings in a list.

Recommendation: Use multi-clause functions or a more declarative approach.

defp role_from_cap(["router" | _]), do: :router
defp role_from_cap(["bridge", "wlan-ap" | _]), do: :wireless
# ...

Avoid if for Optional Query Filters

In Towerops.Snmp.get_sensor_readings, an if statement is used to conditionally add a where clause.

Recommendation: Use a pipe-friendly helper:

defp maybe_filter_since(query, nil), do: query
defp maybe_filter_since(query, since), do: where(query, [r], r.checked_at >= ^since)

# Usage
query
|> maybe_filter_since(since)
|> Repo.all()

4. Performance Optimizations

Batch Database Operations

In Towerops.Topology.upsert_link, evidence records are inserted one-by-one in an Enum.each loop.

Recommendation: Use Repo.insert_all to insert all evidence records in a single round-trip to the database.

Preloading Strategy

Some functions perform manual preloads inside loops or deep in the call stack.

Recommendation: Ensure associations are preloaded at the edge of the context (the public API) to avoid N+1 queries and redundant database calls.

5. Human-Centric Refactoring

Break Down Large Functions

Functions like Towerops.Topology.build_device_lookup are doing too much in one block.

Recommendation: Break these into smaller, named stages:

1. fetch_lookup_devices(org_id)
2. map_ips_to_ids(devices)
3. map_names_to_ids(devices)
4. map_macs_to_ids(devices)

Consistent Result Handling

While Towerops.Result exists, it isn't used everywhere.

Recommendation: Standardize on {:ok, value} | {:error, reason} across all context boundaries and use Towerops.Result helpers to compose operations.