Graham McIntire
151baf8496
- grid.ex: memoize conus_points/0 with module attribute (was ~94k recompute per call)
- path_compute.ex: reduce 6 list traversals to single Enum.reduce pass
- radio.ex + application.ex: fix unused variable e and redundant case warnings
- priv: add 5 partial indexes on (qso_timestamp) for enrichment queries (weather, hrrr,
terrain, iemre, radar) matching the WHERE status IN ('pending','failed') AND pos1 IS NOT NULL
pattern used by contacts_needing_enrichment
- findings.md: mark all performance hotspots and corresponding items as fixed
5.2 KiB
5.2 KiB
Findings — Bugs & Improvements (Non-Critical)
Bugs
🟡 Medium
| # | File | Line | Issue | Suggested Fix |
|---|---|---|---|---|
| 1 | radio.ex |
128, 157 | N+1 queries — list_contacts_for_user and list_contacts_involving_callsign don't preload :user |
Add ` |
radio.ex |
751-757 | (fixed — contacts_dedup_idx) |
||
grid.ex |
31-36 | conus_points/0 |
(fixed — module attribute) | |
| 4 | endpoint.ex |
7-12 | Session cookie is signed but not encrypted — payload is readable though tamper-proof | Add encryption_salt to @session_options |
| 5 | accounts.ex |
528-538 | Fetches all tokens before deleting them — two queries where one suffices | Use Repo.delete_all(from t in UserToken, where: t.user_id == ^user.id) |
| 6 | profiles_file.ex |
169-177 | Unhandled :zlib.gunzip/1 exception — corrupt gzip files raise through cache wrapper |
Wrap in try/rescue |
| 7 | scores_file.ex |
515 | Direct ETS match_delete bypassing Cache API |
Use Cache.invalidate or Cache public API |
| 8 | profiles_file.ex |
323 | Same direct ETS access as above | Same fix |
path_compute.ex |
381-394 | (fixed — single Enum.reduce) |
||
| 10 | hf_muf.ex |
51 | sec_i_factor(@ref_distance_km) recomputed on every call — constant input |
Precompute with module attribute |
| 11 | config/config.exs |
163 | EXLA.Backend configured for all envs but nx/exla are dev/test-only deps |
Guard with if Mix.env() in [:dev, :test] |
🟢 Low
| # | File | Line | Issue | Suggested Fix |
|---|---|---|---|---|
| 12 | router.ex |
165 | Login rate limit 30/min per IP may be generous for brute-force | Consider lowering to 10-15/min |
| 13 | application.ex |
107-109 | Dev/test model loading blocks supervisor start | Wrap in Task.start |
| 14 | scorer.ex |
126-138 | classify_time_period guard boundaries have gap at exactly -3.0 |
Switch to cond with inclusive/exclusive clarity |
| 15 | scorer.ex |
636 | ` | |
| 16 | path_compute.ex |
356-357 | Eager Repo.get(Station, ...) instead of preloading assoc |
Preload :station upstream |
| 17 | radio.ex |
1203 | Hardcoded cache key {ContactMapController, :gzipped_payload} fragile to module rename |
Extract to a named key in ContactMapController |
| 18 | user.ex |
— | Missing has_many :contacts and has_many :beacons associations |
Add for convenience (not a bug, test callers use raw queries) |
radio.ex |
337 | (fixed — 5 qso_timestamp partial indexes added) |
Improvements
Architecture & Design
| # | Area | Suggestion |
|---|---|---|
| 1 | contact_live_test.exs (3505 lines) |
Split into smaller files — currently 2.5x next largest file, uses async: true with shared state, relies on send(lv.pid, ...) / :sys.get_state |
| 2 | 18 Process.sleep usages in tests |
Replace with Process.monitor + assert_receive or :sys.get_state per AGENTS.md |
| 3 | 25 Process.alive? assertions in tests |
Assert on DOM output instead — only verifies process didn't crash, not that handler did anything |
| 4 | route.ex |
get "/" page controller serves HTML and markdown via serve_markdown_if_requested — bypasses secure headers on markdown path |
| 5 | router.ex |
Some public routes (/docs/api/openapi.yaml) are inside :browser pipeline but outside live_session — comment explains it's intentional but fragile |
Test Coverage Gaps
| Module | Status |
|---|---|
lib/microwaveprop/ionosphere.ex |
Untested |
lib/microwaveprop/mailer.ex |
Untested |
lib/microwaveprop/repo.ex |
Untested |
lib/microwaveprop/space_weather.ex |
Untested |
about_live.ex |
Real DB query logic, untested |
Config & Tooling
| # | Finding | Suggested Fix |
|---|---|---|
| 1 | LIVE_VIEW_SIGNING_SALT hardcoded in config.exs, not read from env |
Read from System.get_env in runtime.exs |
| 2 | :precommit uses deps.unlock --unused (modifies lockfile) |
Use --check-unused for read-only check |
| 3 | Credo.Check.Warning.UnsafeToAtom disabled |
Re-enable to catch String.to_atom(user_input) DOS vector |
| 4 | Credo.Check.Readability.Specs disabled |
Consider re-enabling for production codebase |
| 5 | Credo.Check.Warning.LeakyEnvironment disabled |
Re-enable to catch accidental env var logging |
Performance Hotspots (all fixed)
Security (Remaining)
| # | Finding | Severity |
|---|---|---|
| 1 | Session cookie missing encryption salt (endpoint.ex:7-12) | Medium |
| 2 | String.to_atom/1 not warned by Credo (.credo.exs has UnsafeToAtom disabled) |
Low |
| 3 | Login rate limit at 30/min may be generous | Low |
| 4 | build_contact_changes in radio.ex:1277 uses String.to_existing_atom(key) — safe due to whitelist, but fragile |
Low |
| 5 | Markdown path bypasses secure browser headers | Low (mitigated by plain-text content type) |