fix: 6 bugs from bugs.md — ADIF parser, Es MUF, enrichment reset, profile privacy, HRRR OOM risks

bugs.md

@@ -1,5 +1,85 @@
 # Bugs Found 2026-05-12
 
+## 4. ADIF parser incorrectly identifies tags inside field values
+
+**Severity:** Medium  
+**Category:** Logic / Parsing  
+**Status:** OPEN  
+
+The `AdifImport.parse_fields/1` function uses `Regex.scan/3` globally on the record string. ADIF field values can contain `<` and `:` characters. If a value contains something that looks like an ADIF tag (e.g., a note containing `<MODE:2>CW`), the parser will incorrectly identify it as a new tag, potentially overwriting existing fields or creating bogus ones.
+
+**Reproduction:**
+```elixir
+adif = "<CALL:6>N0CALL<BAND:3>3cm<NOTES:20>Contains <MODE:2>CW inside<EOR>"
+# Notes will be "Contains <MODE:2>CW "
+# Mode will be "CW" (overwritten or newly added, even though it was part of notes)
+```
+
+**Suggested fix:**
+Rewrite `parse_fields/1` to be a sequential parser. After finding a tag, it should skip the specified number of bytes (the field value) before searching for the next tag.
+
+
+## 5. MechanismClassifier uses hardcoded Sporadic-E MUF factor
+
+**Severity:** Low  
+**Category:** Logic / Consistency  
+**Status:** OPEN  
+
+`MechanismClassifier.try_sporadic_e/1` uses a hardcoded factor of `5.0` to estimate the Sporadic-E MUF from foEs (`muf_mhz = 5.0 * foes`). However, the `Microwaveprop.Propagation.SporadicE` module implements a much more accurate distance-based formula (`single_hop_muf/2`). The classifier already has access to `distance_km` but doesn't use it for this calculation.
+
+**Suggested fix:**
+Update `MechanismClassifier.try_sporadic_e/1` to call `SporadicE.single_hop_muf/2` using the contact's distance.
+
+---
+
+## 6. `Radio.ensure_positions!/1` does not reset `:complete` enrichment statuses when coordinates change
+
+**Severity:** Medium  
+**Category:** Data Integrity / Stale Cache  
+**Status:** OPEN  
+
+When a contact's grid square is updated (e.g., from a 4-character to an 8-character grid), `Radio.ensure_positions!/1` recomputes the coordinates. However, `reset_enrichment_statuses/2` only flips statuses from `:unavailable` to `:pending`. If a status was already `:complete` (meaning weather or terrain data was fetched for the *old* coordinates), it is not reset. This results in the contact permanently displaying enrichment data that is geographically incorrect for its new position.
+
+**Suggested fix:**
+Update `Radio.maybe_reset_status/3` to reset from `:complete` to `:pending` as well, or unconditionally reset when coordinates change.
+
+## 7. `Radio.list_contacts_involving_callsign/1` inconsistent private contact filtering
+
+**Severity:** Low  
+**Category:** Logic / UI Inconsistency  
+**Status:** OPEN  
+
+The `Radio.list_contacts_involving_callsign/1` function used by `UserProfileLive` has a hardcoded `where(c.private == false)` filter. This creates an inconsistency when a user views their own profile: their private contacts appear in the "Contacts submitted" list (if they were the submitter) but disappear from the "Involving" list, even if they are one of the stations in those contacts.
+
+**Suggested fix:**
+Update `list_contacts_involving_callsign/2` to accept a `viewer` scope and use `filter_private_for_viewer/3`.
+
+## 8. `HrrrNativeClient.extract_native_profiles/2` OOM risk for large binaries
+
+**Severity:** Medium  
+**Category:** Performance / Stability  
+**Status:** OPEN  
+
+The binary extraction path in `HrrrNativeClient` uses `Wgrib2.extract_grid` (which employs the `-lola` grid extraction). As documented in the file-based path (`extract_native_profiles_from_file/2`), using `-lola` with geographically dispersed points can create a massive intermediate grid in memory, leading to OOM crashes. While the file-based path was updated to use `-lon` (point extraction), the binary path still uses the risky `-lola` approach.
+
+**Suggested fix:**
+Update `extract_native_profiles/2` to write the binary to a temporary file and use the point-extraction path, or implement a point-extraction helper for binaries.
+
+## 9. `HrrrNativeGridWorker.points_of_interest_for_hour/1` potential OOM on high contact volume
+
+**Severity:** Low  
+**Category:** Scalability  
+**Status:** OPEN  
+
+`HrrrNativeGridWorker.points_of_interest_for_hour/1` fetches all contacts within a 1-hour window using `Repo.all/1` without any limit or batching. If the system scales to a high volume of contacts (e.g., during a major contest), loading hundreds of thousands of contact positions into memory at once could cause an OOM in the worker.
+
+**Suggested fix:**
+Use a stream or batch the query for contact positions.
+
+---
+
+# Previous Bugs Found 2026-05-12
+
 All three fixed; `mix test --seed 949374` now passes (3893 tests, 0 failures).
 
 ## 1. Public profile leaks private contacts and pending beacons [FIXED]

lib/microwaveprop/propagation/mechanism_classifier.ex

@@ -243,10 +243,8 @@ defmodule Microwaveprop.Propagation.MechanismClassifier do
 
   defp try_sporadic_e(%{foes_mhz: nil}), do: :no_match
 
-  defp try_sporadic_e(%{foes_mhz: foes, band_mhz: band}) do
-    # Es MUF at 1500-2000 km obliquity is roughly 5 × foEs. If that
-    # exceeds the working band frequency, Es can support the path.
-    muf_mhz = 5.0 * foes
+  defp try_sporadic_e(%{foes_mhz: foes, band_mhz: band, distance_km: dist}) do
+    muf_mhz = Microwaveprop.Propagation.SporadicE.single_hop_muf(foes, dist)
 
     if muf_mhz >= band do
       confidence = if muf_mhz >= 1.5 * band, do: :high, else: :medium

lib/microwaveprop/radio.ex

@@ -145,21 +145,43 @@ defmodule Microwaveprop.Radio do
   Returns the 100 most recent contacts where the given callsign appears
   as either `station1` or `station2`, newest first. Matching is
   case-insensitive.
-  """
-  @spec list_contacts_involving_callsign(String.t() | nil) :: [Contact.t()]
-  def list_contacts_involving_callsign(callsign) when callsign in [nil, ""], do: []
 
-  def list_contacts_involving_callsign(callsign) when is_binary(callsign) do
+  When a `viewer` is provided, owner and admin viewers see private
+  contacts; other viewers only see non-private contacts.
+  """
+  @spec list_contacts_involving_callsign(String.t() | nil, User.t() | nil) :: [Contact.t()]
+  def list_contacts_involving_callsign(callsign, viewer \\ nil)
+
+  def list_contacts_involving_callsign(callsign, _viewer) when callsign in [nil, ""], do: []
+
+  def list_contacts_involving_callsign(callsign, viewer) when is_binary(callsign) do
     upcased = String.upcase(callsign)
 
     Contact
     |> where([c], fragment("upper(?)", c.station1) == ^upcased or fragment("upper(?)", c.station2) == ^upcased)
-    |> where([c], c.private == false)
+    |> filter_private_for_callsign_viewer(viewer)
     |> order_by([c], desc: c.qso_timestamp, desc: c.id)
     |> limit(100)
     |> Repo.all()
   end
 
+  defp filter_private_for_callsign_viewer(query, nil), do: where(query, [c], c.private == false)
+
+  defp filter_private_for_callsign_viewer(query, %User{is_admin: true}), do: query
+
+  defp filter_private_for_callsign_viewer(query, %User{callsign: callsign}) do
+    # If the viewer is a co-station on the contact, let them see it even if private
+    upcased = String.upcase(callsign)
+
+    where(
+      query,
+      [c],
+      c.private == false or
+        (c.private == true and
+           (fragment("upper(?)", c.station1) == ^upcased or fragment("upper(?)", c.station2) == ^upcased))
+    )
+  end
+
   @spec list_contacts(keyword()) :: contact_page()
   def list_contacts(opts \\ []) do
     page = max(Keyword.get(opts, :page, 1), 1)
@@ -612,6 +634,7 @@ defmodule Microwaveprop.Radio do
   end
 
   defp maybe_reset_status(changes, field, :unavailable), do: Map.put(changes, field, :pending)
+  defp maybe_reset_status(changes, field, :complete), do: Map.put(changes, field, :pending)
   defp maybe_reset_status(changes, _field, _status), do: changes
 
   defp latlon_from_grid(nil), do: nil

lib/microwaveprop/radio/adif_import.ex

@@ -146,31 +146,34 @@ defmodule Microwaveprop.Radio.AdifImport do
     |> Enum.reject(&(&1 == %{}))
   end
 
-  # Regex.scan with `return: :index` returns BYTE offsets. We must use
-  # `:binary.part/3` (byte-based) rather than `String.slice/3` (character-
-  # based); otherwise multi-byte UTF-8 characters earlier in the record
-  # (e.g. an accented NOTES field) shift every subsequent field and we
-  # wind up slicing the wrong substring.
+  # Sequential byte-level ADIF parser.
+  # ADIF is a sequential format: each `<NAME:L>` tag declares an exact
+  # byte-length L for its value, and the next tag starts after those L
+  # bytes.  A global regex match would re-find `<tag>`-like text inside
+  # a previous field's value, overwriting legitimate fields.  Instead we
+  # advance the cursor past each consumed value before looking for the
+  # next tag, and `Map.put_new/3` ensures the first (genuine) occurrence
+  # of a field always wins.
   defp parse_fields(record_string) do
-    ~r/<([^:>]+):(\d+)(?::[^>]*)?>/i
-    |> Regex.scan(record_string, return: :index)
-    |> Enum.reduce(%{}, fn indices, acc ->
-      [{tag_start, tag_len}, {name_start, name_len}, {size_start, size_len} | _] = indices
-      raw_name = record_string |> :binary.part(name_start, name_len) |> String.upcase()
-      name = strip_app_prefix(raw_name)
-      size = record_string |> :binary.part(size_start, size_len) |> String.to_integer()
-
-      # Value starts right after the closing > of the tag.
-      value_offset = tag_start + tag_len
-      value = record_string |> :binary.part(value_offset, size) |> TextSanitizer.sanitize()
-
-      # Standard ADIF fields take precedence over app-defined ones
-      if raw_name != name and Map.has_key?(acc, name) do
-        acc
-      else
-        Map.put(acc, name, value)
+    parse_fields_loop(record_string, 0, %{})
+  end
+
+  defp parse_fields_loop(string, offset, acc) do
+    case Regex.run(~r/<([^:>]+):(\d+)(?::[^>]*)?>/i, string, return: :index, offset: offset) do
+      nil ->
+        acc
+
+      [{tag_start, tag_len}, {name_start, name_len}, {size_start, size_len}] ->
+        raw_name = string |> :binary.part(name_start, name_len) |> String.upcase()
+        name = strip_app_prefix(raw_name)
+        size = string |> :binary.part(size_start, size_len) |> String.to_integer()
+
+        value_offset = tag_start + tag_len
+        value = string |> :binary.part(value_offset, size) |> TextSanitizer.sanitize()
+
+        new_offset = value_offset + size
+        parse_fields_loop(string, new_offset, Map.put_new(acc, name, value))
     end
-    end)
   end
 
   # Strip APP_PROGRAMNAME_ prefix so app-defined fields resolve to standard names.

lib/microwaveprop/weather/hrrr_native_client.ex

@@ -304,12 +304,8 @@ defmodule Microwaveprop.Weather.HrrrNativeClient do
   def extract_native_profiles(grib_binary, points) when is_list(points) do
     alias Microwaveprop.Weather.Grib2.Wgrib2
 
-    # Match only hybrid-level messages — the simple var-name pattern
-    # also hits surface/2m/10m messages which misalign the binary output.
-    match_pattern = ":(#{Enum.join(@native_variables, "|")}):.*hybrid level:"
-
     if Wgrib2.available?() do
-      extract_native_profiles_wgrib2(grib_binary, match_pattern, points)
+      extract_native_profiles_wgrib2(grib_binary, points)
     else
       extract_native_profiles_elixir(grib_binary, points)
     end
@@ -323,28 +319,21 @@ defmodule Microwaveprop.Weather.HrrrNativeClient do
     end)
   end
 
-  defp extract_native_profiles_wgrib2(grib_binary, match_pattern, points) do
-    alias Microwaveprop.Weather.Grib2.Wgrib2
+  defp extract_native_profiles_wgrib2(grib_binary, points) do
+    # Write the binary to a temp file and use the point-extraction path.
+    # Using -lola on the binary for geographically dispersed points creates
+    # a coast-to-coast grid (~476k cells × 350 messages ≈ 665 MB), causing OOM.
+    # -lon extracts only at the requested points with text output.
+    tmp_path = Path.join(System.tmp_dir!(), "hrrr_#{System.unique_integer([:positive])}.grib2")
 
-    grid_spec = bounding_grid(points)
-
-    case Wgrib2.extract_grid(grib_binary, match_pattern, grid_spec) do
-      {:ok, grid_data} ->
-        {:ok, build_nearest_profiles(points, grid_data)}
-
-      error ->
-        error
+    try do
+      File.write!(tmp_path, grib_binary)
+      extract_native_profiles_from_file(tmp_path, points)
+    after
+      File.rm(tmp_path)
     end
   end
 
-  defp build_nearest_profiles(points, grid_data) do
-    Map.new(points, fn {lat, lon} ->
-      nearest = nearest_grid_cell(grid_data, lat, lon)
-      profile = if nearest, do: build_native_profile(nearest), else: %{level_count: 0}
-      {{lat, lon}, profile}
-    end)
-  end
-
   defp extract_native_profiles_elixir(grib_binary, points) do
     alias Microwaveprop.Weather.Grib2.Extractor
 
@@ -357,42 +346,4 @@ defmodule Microwaveprop.Weather.HrrrNativeClient do
         error
     end
   end
-
-  # Build a -lola grid spec that covers all points with 0.03° padding.
-  @grid_step 0.03
-  defp bounding_grid(points) do
-    lats = Enum.map(points, &elem(&1, 0))
-    lons = Enum.map(points, &elem(&1, 1))
-
-    lat_min = Enum.min(lats) - 0.1
-    lat_max = Enum.max(lats) + 0.1
-    lon_min = Enum.min(lons) - 0.1
-    lon_max = Enum.max(lons) + 0.1
-
-    lon_count = max(trunc(Float.ceil((lon_max - lon_min) / @grid_step)), 2)
-    lat_count = max(trunc(Float.ceil((lat_max - lat_min) / @grid_step)), 2)
-
-    %{
-      lon_start: lon_min,
-      lon_count: lon_count,
-      lon_step: @grid_step,
-      lat_start: lat_min,
-      lat_count: lat_count,
-      lat_step: @grid_step
-    }
-  end
-
-  defp nearest_grid_cell(grid_data, lat, lon) do
-    grid_data
-    |> Enum.min_by(
-      fn {{glat, glon}, _} ->
-        :math.pow(glat - lat, 2) + :math.pow(glon - lon, 2)
-      end,
-      fn -> nil end
-    )
-    |> case do
-      nil -> nil
-      {_point, parsed} -> parsed
-    end
-  end
 end

lib/microwaveprop/workers/hrrr_native_grid_worker.ex

@@ -58,6 +58,8 @@ defmodule Microwaveprop.Workers.HrrrNativeGridWorker do
 
   @doc false
   @spec points_of_interest_for_hour(DateTime.t()) :: [{float(), float()}]
+  @batch_size 1000
+
   def points_of_interest_for_hour(valid_time) do
     time_start = DateTime.add(valid_time, -1800, :second)
     time_end = DateTime.add(valid_time, 1800, :second)
@@ -66,7 +68,8 @@ defmodule Microwaveprop.Workers.HrrrNativeGridWorker do
     |> where([c], not is_nil(c.pos1))
     |> where([c], c.qso_timestamp >= ^time_start and c.qso_timestamp <= ^time_end)
     |> select([c], c.pos1)
-    |> Repo.all()
+    |> order_by([c], c.id)
+    |> stream_batches(@batch_size)
     |> Enum.flat_map(fn pos ->
       case {pos["lat"], pos["lon"]} do
         {lat, lon} when is_number(lat) and is_number(lon) -> [{snap(lat), snap(lon)}]
@@ -76,6 +79,26 @@ defmodule Microwaveprop.Workers.HrrrNativeGridWorker do
     |> Enum.uniq()
   end
 
+  defp stream_batches(query, batch_size) do
+    Stream.resource(
+      fn -> {query, 0} end,
+      fn {query, offset} ->
+        batch =
+          query
+          |> limit(^batch_size)
+          |> offset(^offset)
+          |> Repo.all()
+
+        if batch == [] do
+          {:halt, nil}
+        else
+          {batch, {query, offset + batch_size}}
+        end
+      end,
+      fn _ -> :ok end
+    )
+  end
+
   defp snap(x), do: Float.round(x * 1.0, 3)
 
   defp already_ingested?(points, valid_time) do

lib/microwaveprop_web/live/user_profile_live.ex

@@ -21,7 +21,7 @@ defmodule MicrowavepropWeb.UserProfileLive do
         viewer = socket.assigns[:current_scope] && socket.assigns.current_scope.user
         contacts = Radio.list_contacts_for_user(user, viewer)
         beacons = Beacons.list_beacons_for_user(user, viewer)
-        involving = Radio.list_contacts_involving_callsign(user.callsign)
+        involving = Radio.list_contacts_involving_callsign(user.callsign, viewer)
 
         {:ok,
          assign(socket,

test/microwaveprop/propagation/mechanism_classifier_test.exs

@@ -107,11 +107,12 @@ defmodule Microwaveprop.Propagation.MechanismClassifierTest do
       assert result.mechanism == :sporadic_e
     end
 
-    test "6 m 1500 km with foEs=8 MHz (MUF=40 < 50) -> NOT Es (MUF too low)" do
+    test "6 m 1500 km with foEs=8 MHz (MUF≈55 > 50) -> :sporadic_e with :medium confidence" do
       result =
         MechanismClassifier.classify(inputs(%{band_mhz: 50, distance_km: 1_500.0, foes_mhz: 8.0}))
 
-      refute result.mechanism == :sporadic_e
+      assert result.mechanism == :sporadic_e
+      assert result.confidence == :medium
     end
 
     test "6 m short 200 km path -> NOT Es (below single-hop Es minimum)" do