From 4759d1313e4accf421946c41ae114b476320c392 Mon Sep 17 00:00:00 2001
From: Graham McIntire <graham@mcintire.me>
Date: Fri, 1 May 2026 12:40:17 -0500
Subject: [PATCH] fix(aprs): tighten PathParser self-loop check and Q-construct
 regex
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three spec-compliance fixes to PathParser:

1. Q-construct regex now accepts lowercase letters (qAo, qAr, etc.).
   The moduledoc lists qAo as a Q-construct to skip but the previous
   ~r/^qA[A-Z]$/ rejected it, leaving it to be misclassified.

2. self_loop?/2 now compares callsigns only; the spurious
   src_pos == dst_pos clause would falsely suppress two distinct
   stations that happen to share coarse coordinates (e.g. same building).

3. Strengthen the "lookup returned nil → drop hop, do NOT advance source"
   test: extend the path to three tokens so the assertion actually proves
   the third hop chains from the previous good source rather than from
   the dropped one. The two-token version passed regardless of behavior.

Self-loop fixture updated to trigger via callsign equality (the only
remaining mechanism) instead of position equality.
---
 lib/microwaveprop/aprs/path_parser.ex        |  8 ++--
 test/microwaveprop/aprs/path_parser_test.exs | 42 +++++++++++++++-----
 2 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/lib/microwaveprop/aprs/path_parser.ex b/lib/microwaveprop/aprs/path_parser.ex
index a94c9641..6fe08458 100644
--- a/lib/microwaveprop/aprs/path_parser.ex
+++ b/lib/microwaveprop/aprs/path_parser.ex
@@ -38,7 +38,7 @@ defmodule Microwaveprop.Aprs.PathParser do
 
   @alias_regex ~r/^(WIDE[12](-[0-9])?|TRACE[12](-[0-9])?|RELAY|ECHO|GATE|NOGATE|RFONLY)$/
   @callsign_regex ~r/^[A-Z0-9]{1,2}[0-9][A-Z]{1,3}(-[0-9]{1,2})?$/
-  @q_construct_regex ~r/^qA[A-Z]$/
+  @q_construct_regex ~r/^qA[A-Za-z]$/
 
   @spec parse_hops(
           sender_callsign :: String.t(),
@@ -89,7 +89,7 @@ defmodule Microwaveprop.Aprs.PathParser do
       dst_pos ->
         {src_callsign, src_pos} = source
 
-        if self_loop?(src_callsign, src_pos, callsign, dst_pos) do
+        if self_loop?(src_callsign, callsign) do
           {source, hops}
         else
           hop = build_hop(src_callsign, src_pos, callsign, dst_pos, heard_at)
@@ -108,8 +108,8 @@ defmodule Microwaveprop.Aprs.PathParser do
     {source, hops}
   end
 
-  defp self_loop?(src_callsign, src_pos, dst_callsign, dst_pos) do
-    src_callsign == dst_callsign or src_pos == dst_pos
+  defp self_loop?(src_callsign, dst_callsign) do
+    src_callsign == dst_callsign
   end
 
   defp build_hop(src_callsign, src_pos, dst_callsign, dst_pos, heard_at) do
diff --git a/test/microwaveprop/aprs/path_parser_test.exs b/test/microwaveprop/aprs/path_parser_test.exs
index 5fce75cf..2549d5d8 100644
--- a/test/microwaveprop/aprs/path_parser_test.exs
+++ b/test/microwaveprop/aprs/path_parser_test.exs
@@ -128,25 +128,32 @@ defmodule Microwaveprop.Aprs.PathParserTest do
       assert hops == []
     end
 
-    test "used callsign with nil lookup is dropped and source does NOT advance" do
+    test "lookup returns nil for a used digi → drop that hop and do NOT advance source" do
       lookup =
         lookup_fn(%{
-          "K5XYZ-3" => {33.0, -96.5}
+          "K5XYZ-3" => {33.0, -96.5},
           # W5ABC-7 deliberately missing
+          "K5DEF-1" => {33.5, -95.5}
         })
 
       hops =
         PathParser.parse_hops(
           @sender,
           @sender_pos,
-          "K5XYZ-3*,W5ABC-7*",
+          "K5XYZ-3*,W5ABC-7*,K5DEF-1*",
           @heard_at,
           lookup
         )
 
-      assert [hop] = hops
-      assert hop.src_callsign == @sender
-      assert hop.dst_callsign == "K5XYZ-3"
+      assert length(hops) == 2
+      # First hop is sender → K5XYZ-3 as before.
+      assert Enum.at(hops, 0).src_callsign == @sender
+      assert Enum.at(hops, 0).dst_callsign == "K5XYZ-3"
+      # Critical: second emitted hop chains from K5XYZ-3 (NOT from W5ABC-7
+      # which was dropped). Proves source did not advance through the
+      # missing-position digi.
+      assert Enum.at(hops, 1).src_callsign == "K5XYZ-3"
+      assert Enum.at(hops, 1).dst_callsign == "K5DEF-1"
     end
 
     test "empty path string yields no hops" do
@@ -172,6 +179,21 @@ defmodule Microwaveprop.Aprs.PathParserTest do
       assert hops == []
     end
 
+    test "lowercase Q-construct qAo is recognized and yields no hops" do
+      lookup = lookup_fn(%{})
+
+      hops =
+        PathParser.parse_hops(
+          @sender,
+          @sender_pos,
+          "qAo",
+          @heard_at,
+          lookup
+        )
+
+      assert hops == []
+    end
+
     test "Q-construct followed by IGate name yields no hops" do
       lookup = lookup_fn(%{"IGATE-CALL" => {33.0, -96.0}})
 
@@ -187,14 +209,16 @@ defmodule Microwaveprop.Aprs.PathParserTest do
       assert hops == []
     end
 
-    test "self-loop (callsign resolves to sender's own position) yields no hops" do
-      lookup = lookup_fn(%{"K5GVL-10" => @sender_pos})
+    test "self-loop suppressed when used digi callsign matches sender callsign" do
+      # Sender sees its own callsign reflected in the digi path (corrupted
+      # frame). Should be skipped.
+      lookup = lookup_fn(%{@sender => {99.0, 99.0}})
 
       hops =
         PathParser.parse_hops(
           @sender,
           @sender_pos,
-          "K5GVL-10*",
+          "#{@sender}*",
           @heard_at,
           lookup
         )