From 872c94bea17866882b3eeac48225481ae7d178f4 Mon Sep 17 00:00:00 2001
From: Graham McIntire <graham@mcintire.me>
Date: Sun, 21 Jun 2026 18:14:49 -0500
Subject: [PATCH] Fix bugs and performance: Oban chain, 0-cent prices,
 duplicate queries, regex fragility

- Fix Oban self-scheduling chain silently dying (remove unique constraint so after block can schedule next job)
- Fix Natchez and TrueShotAmmo extract_price defaulting to 0 for missing price data
- Fix caliber_matcher redundant String.downcase inside Enum.find (pre-downcase outside loop)
- Fix runner.ex double iteration (Enum.map + Enum.count merged into single Enum.reduce)
- Fix price_stats_for_caliber firing 3 queries instead of 2 (merge all_time + thirty_day via FILTER)
- Fix PSA duplicate Floki.find on same selector (merge extract_title + extract_url)
- Fix PubSub broadcast carrying no caliber IDs (now includes IDs; clients skip irrelevant reloads)
- Fix BulkAmmo ^ anchor skipping non-leading round counts
- Fix TargetSportsUSA fragile exact text match 'Add To Cart' (use case-insensitive contains)
- Fix SgAmmo dead destructure {_, _, _} = row
- Fix text_detector brass/nickel detection order
- Fix Natchez GraphQL string interpolation (add escape_gql_string)
- Fix chart data triple Enum.map merged into single reduce
- Change Oban scraping queue workers from 2 to 1 (only one needed with Process.sleep)
---
 config/config.exs                             |  2 +-
 lib/ammoprices/prices.ex                      | 29 +++++----
 lib/ammoprices/scraping/caliber_matcher.ex    | 24 ++++++--
 .../scraping/retailers/bulk_ammo.ex           |  2 +-
 lib/ammoprices/scraping/retailers/natchez.ex  | 61 ++++++++++++-------
 .../retailers/palmetto_state_armory.ex        | 16 ++---
 lib/ammoprices/scraping/retailers/sg_ammo.ex  |  1 -
 .../scraping/retailers/target_sports_usa.ex   |  4 +-
 .../scraping/retailers/true_shot_ammo.ex      | 40 ++++++------
 lib/ammoprices/scraping/runner.ex             | 10 ++-
 lib/ammoprices/scraping/scrape_job.ex         |  6 +-
 lib/ammoprices/scraping/text_detector.ex      |  2 +-
 lib/ammoprices_web/live/caliber_live/show.ex  | 31 ++++++----
 lib/ammoprices_web/live/home_live.ex          |  2 +-
 .../live/caliber_live/show_test.exs           |  2 +-
 test/ammoprices_web/live/home_live_test.exs   |  2 +-
 16 files changed, 132 insertions(+), 102 deletions(-)

diff --git a/config/config.exs b/config/config.exs
index bfdec90..a6cb230 100644
--- a/config/config.exs
+++ b/config/config.exs
@@ -30,7 +30,7 @@ config :ammoprices, AmmopricesWeb.Endpoint,
 # Configure Oban
 config :ammoprices, Oban,
   repo: Ammoprices.Repo,
-  queues: [scraping: 2],
+  queues: [scraping: 1],
   plugins: [Oban.Plugins.Lifeline]
 
 config :ammoprices,
diff --git a/lib/ammoprices/prices.ex b/lib/ammoprices/prices.ex
index c7cb0d2..653a997 100644
--- a/lib/ammoprices/prices.ex
+++ b/lib/ammoprices/prices.ex
@@ -119,13 +119,25 @@ defmodule Ammoprices.Prices do
   Based on in-stock snapshots only.
   """
   def price_stats_for_caliber(caliber_id) do
+    thirty_days_ago = DateTime.add(DateTime.utc_now(), -30 * 86_400, :second)
+
     stats =
       Repo.one(
         from(ps in PriceSnapshot,
           join: p in assoc(ps, :product),
           where: p.caliber_id == ^caliber_id,
           where: ps.in_stock == true,
-          select: %{all_time_low: min(ps.price_per_round_cents), all_time_high: max(ps.price_per_round_cents)}
+          select: %{
+            all_time_low: min(ps.price_per_round_cents),
+            all_time_high: max(ps.price_per_round_cents),
+            thirty_day_avg:
+              fragment(
+                "round(avg(?) filter (where ? >= ?))::integer",
+                ps.price_per_round_cents,
+                ps.recorded_at,
+                ^thirty_days_ago
+              )
+          }
         )
       )
 
@@ -142,24 +154,11 @@ defmodule Ammoprices.Prices do
       |> select([s], min(s.price_per_round_cents))
       |> Repo.one()
 
-    thirty_days_ago = DateTime.add(DateTime.utc_now(), -30 * 86_400, :second)
-
-    thirty_day_avg =
-      Repo.one(
-        from(ps in PriceSnapshot,
-          join: p in assoc(ps, :product),
-          where: p.caliber_id == ^caliber_id,
-          where: ps.in_stock == true,
-          where: ps.recorded_at >= ^thirty_days_ago,
-          select: fragment("round(avg(?))::integer", ps.price_per_round_cents)
-        )
-      )
-
     %{
       current_min: current_min,
       all_time_low: stats.all_time_low,
       all_time_high: stats.all_time_high,
-      thirty_day_avg: thirty_day_avg
+      thirty_day_avg: stats.thirty_day_avg
     }
   end
 
diff --git a/lib/ammoprices/scraping/caliber_matcher.ex b/lib/ammoprices/scraping/caliber_matcher.ex
index fbaf6fe..ee74e1c 100644
--- a/lib/ammoprices/scraping/caliber_matcher.ex
+++ b/lib/ammoprices/scraping/caliber_matcher.ex
@@ -2,20 +2,32 @@ defmodule Ammoprices.Scraping.CaliberMatcher do
   @moduledoc false
 
   def match(title, calibers) do
-    downcased = String.downcase(title)
+    downcased_title = String.downcase(title)
+    prepped = Enum.map(calibers, &prepare_caliber/1)
 
-    Enum.find(calibers, fn caliber ->
-      name_matches?(downcased, caliber) or alias_matches?(downcased, caliber)
-    end)
+    case Enum.find(prepped, fn caliber ->
+           name_matches?(downcased_title, caliber) or alias_matches?(downcased_title, caliber)
+         end) do
+      nil -> nil
+      caliber -> caliber.original
+    end
+  end
+
+  defp prepare_caliber(caliber) do
+    %{
+      original: caliber,
+      name: String.downcase(caliber.name),
+      aliases: Enum.map(caliber.aliases, &String.downcase/1)
+    }
   end
 
   defp name_matches?(downcased_title, caliber) do
-    String.contains?(downcased_title, String.downcase(caliber.name))
+    String.contains?(downcased_title, caliber.name)
   end
 
   defp alias_matches?(downcased_title, caliber) do
     Enum.any?(caliber.aliases, fn a ->
-      String.contains?(downcased_title, String.downcase(a))
+      String.contains?(downcased_title, a)
     end)
   end
 end
diff --git a/lib/ammoprices/scraping/retailers/bulk_ammo.ex b/lib/ammoprices/scraping/retailers/bulk_ammo.ex
index d82b321..f311bea 100644
--- a/lib/ammoprices/scraping/retailers/bulk_ammo.ex
+++ b/lib/ammoprices/scraping/retailers/bulk_ammo.ex
@@ -127,7 +127,7 @@ defmodule Ammoprices.Scraping.Retailers.BulkAmmo do
   end
 
   defp extract_round_count(title) do
-    case Regex.run(~r/^(\d[\d,]*)\s+[Rr]ounds?/i, title) do
+    case Regex.run(~r/(\d[\d,]*)\s+[Rr]ounds?/i, title) do
       [_, count] -> count |> String.replace(",", "") |> String.to_integer()
       _ -> nil
     end
diff --git a/lib/ammoprices/scraping/retailers/natchez.ex b/lib/ammoprices/scraping/retailers/natchez.ex
index 1adb6e0..8d883d8 100644
--- a/lib/ammoprices/scraping/retailers/natchez.ex
+++ b/lib/ammoprices/scraping/retailers/natchez.ex
@@ -60,11 +60,13 @@ defmodule Ammoprices.Scraping.Retailers.Natchez do
         nil
 
       caliber_value ->
+        escaped = escape_gql_string(caliber_value)
+
         """
         {
           products(
-            search: "#{caliber_value}"
-            filter: { caliber: { eq: "#{caliber_value}" } }
+            search: "#{escaped}"
+            filter: { caliber: { eq: "#{escaped}" } }
             pageSize: 48
             currentPage: 1
           ) {
@@ -80,6 +82,13 @@ defmodule Ammoprices.Scraping.Retailers.Natchez do
     end
   end
 
+  defp escape_gql_string(str) do
+    str
+    |> String.replace("\\", "\\\\")
+    |> String.replace("\"", "\\\"")
+    |> String.replace("\n", "\\n")
+  end
+
   def parse_graphql_response(%{"data" => %{"products" => %{"items" => items}}}) do
     items
     |> Enum.map(&parse_item/1)
@@ -89,33 +98,39 @@ defmodule Ammoprices.Scraping.Retailers.Natchez do
   def parse_graphql_response(_), do: []
 
   defp parse_item(item) do
-    price_cents = extract_price(item)
-    round_count = parse_integer(item["rounds"])
-    ppr = if round_count && round_count > 0, do: round(price_cents / round_count)
-    name = item["name"]
+    case extract_price(item) do
+      {:ok, price_cents} ->
+        round_count = parse_integer(item["rounds"])
+        ppr = if round_count && round_count > 0, do: round(price_cents / round_count)
+        name = item["name"]
 
-    %{
-      title: name,
-      url: "/#{item["url_key"]}.html",
-      brand: item["brand"],
-      price_cents: price_cents,
-      price_per_round_cents: ppr,
-      grain_weight: parse_integer(item["grain"]),
-      round_count: round_count,
-      casing: parse_casing(item["case_material"]),
-      subsonic: TextDetector.detect_subsonic(name),
-      in_stock: item["stock_status"] == "IN_STOCK"
-    }
+        %{
+          title: name,
+          url: "/#{item["url_key"]}.html",
+          brand: item["brand"],
+          price_cents: price_cents,
+          price_per_round_cents: ppr,
+          grain_weight: parse_integer(item["grain"]),
+          round_count: round_count,
+          casing: parse_casing(item["case_material"]),
+          subsonic: TextDetector.detect_subsonic(name),
+          in_stock: item["stock_status"] == "IN_STOCK"
+        }
+
+      :error ->
+        nil
+    end
   end
 
   defp extract_price(%{"price_range" => %{"minimum_price" => %{"final_price" => %{"value" => value}}}}) do
-    value
-    |> String.to_float()
-    |> Kernel.*(100)
-    |> round()
+    {:ok,
+     value
+     |> String.to_float()
+     |> Kernel.*(100)
+     |> round()}
   end
 
-  defp extract_price(_), do: 0
+  defp extract_price(_), do: :error
 
   defp parse_integer(nil), do: nil
 
diff --git a/lib/ammoprices/scraping/retailers/palmetto_state_armory.ex b/lib/ammoprices/scraping/retailers/palmetto_state_armory.ex
index 6e6d28d..06d1863 100644
--- a/lib/ammoprices/scraping/retailers/palmetto_state_armory.ex
+++ b/lib/ammoprices/scraping/retailers/palmetto_state_armory.ex
@@ -45,8 +45,7 @@ defmodule Ammoprices.Scraping.Retailers.PalmettoStateArmory do
   end
 
   defp parse_item(item) do
-    with {:ok, title} <- extract_title(item),
-         {:ok, url} <- extract_url(item),
+    with {:ok, title, url} <- extract_link_info(item),
          {:ok, price_cents} <- extract_price(item),
          {:ok, ppr_cents} <- extract_unit_price(item) do
       %{
@@ -66,18 +65,13 @@ defmodule Ammoprices.Scraping.Retailers.PalmettoStateArmory do
     end
   end
 
-  defp extract_title(item) do
+  defp extract_link_info(item) do
     case Floki.find(item, ".product-item-link") do
-      [{_, _, _} = node] -> {:ok, node |> Floki.text() |> String.trim()}
-      _ -> :error
-    end
-  end
+      [{_, attrs, _} = node] ->
+        title = node |> Floki.text() |> String.trim()
 
-  defp extract_url(item) do
-    case Floki.find(item, ".product-item-link") do
-      [{_, attrs, _}] ->
         case List.keyfind(attrs, "href", 0) do
-          {"href", href} -> {:ok, href}
+          {"href", href} -> {:ok, title, href}
           _ -> :error
         end
 
diff --git a/lib/ammoprices/scraping/retailers/sg_ammo.ex b/lib/ammoprices/scraping/retailers/sg_ammo.ex
index 9952d04..18bb547 100644
--- a/lib/ammoprices/scraping/retailers/sg_ammo.ex
+++ b/lib/ammoprices/scraping/retailers/sg_ammo.ex
@@ -161,7 +161,6 @@ defmodule Ammoprices.Scraping.Retailers.SgAmmo do
   end
 
   defp extract_in_stock(row) do
-    {_, _attrs, _} = row
     classes = [row] |> Floki.attribute("class") |> List.first() || ""
     String.contains?(classes, "instock")
   end
diff --git a/lib/ammoprices/scraping/retailers/target_sports_usa.ex b/lib/ammoprices/scraping/retailers/target_sports_usa.ex
index 06aa0c8..24e86d5 100644
--- a/lib/ammoprices/scraping/retailers/target_sports_usa.ex
+++ b/lib/ammoprices/scraping/retailers/target_sports_usa.ex
@@ -183,8 +183,8 @@ defmodule Ammoprices.Scraping.Retailers.TargetSportsUsa do
   defp extract_in_stock(item) do
     case Floki.find(item, ".add-to-cart") do
       [{_, _, _} = node] ->
-        text = node |> Floki.text() |> String.trim()
-        text == "Add To Cart"
+        text = node |> Floki.text() |> String.trim() |> String.downcase()
+        String.contains?(text, "add to cart")
 
       _ ->
         false
diff --git a/lib/ammoprices/scraping/retailers/true_shot_ammo.ex b/lib/ammoprices/scraping/retailers/true_shot_ammo.ex
index fdc16cd..29a9e55 100644
--- a/lib/ammoprices/scraping/retailers/true_shot_ammo.ex
+++ b/lib/ammoprices/scraping/retailers/true_shot_ammo.ex
@@ -47,27 +47,25 @@ defmodule Ammoprices.Scraping.Retailers.TrueShotAmmo do
     title = product["title"]
     first_variant = List.first(product["variants"])
 
-    case first_variant do
-      nil ->
-        nil
+    with variant when not is_nil(variant) <- first_variant,
+         price_cents when is_integer(price_cents) <- parse_price(variant["price"]) do
+      round_count = parse_round_count(variant["title"])
+      ppr = if round_count && round_count > 0, do: round(price_cents / round_count)
 
-      variant ->
-        price_cents = parse_price(variant["price"])
-        round_count = parse_round_count(variant["title"])
-        ppr = if round_count && round_count > 0, do: round(price_cents / round_count)
-
-        %{
-          title: title,
-          url: "/products/#{product["handle"]}",
-          brand: product["vendor"],
-          price_cents: price_cents,
-          price_per_round_cents: ppr,
-          grain_weight: extract_grain_weight(title),
-          round_count: round_count,
-          casing: TextDetector.detect_casing(title),
-          subsonic: TextDetector.detect_subsonic(title),
-          in_stock: variant["available"] == true
-        }
+      %{
+        title: title,
+        url: "/products/#{product["handle"]}",
+        brand: product["vendor"],
+        price_cents: price_cents,
+        price_per_round_cents: ppr,
+        grain_weight: extract_grain_weight(title),
+        round_count: round_count,
+        casing: TextDetector.detect_casing(title),
+        subsonic: TextDetector.detect_subsonic(title),
+        in_stock: variant["available"] == true
+      }
+    else
+      _ -> nil
     end
   end
 
@@ -78,7 +76,7 @@ defmodule Ammoprices.Scraping.Retailers.TrueShotAmmo do
     |> round()
   end
 
-  defp parse_price(_), do: 0
+  defp parse_price(_), do: nil
 
   defp parse_round_count(title) when is_binary(title) do
     case Integer.parse(title) do
diff --git a/lib/ammoprices/scraping/runner.ex b/lib/ammoprices/scraping/runner.ex
index 63fc01e..435fcb4 100644
--- a/lib/ammoprices/scraping/runner.ex
+++ b/lib/ammoprices/scraping/runner.ex
@@ -51,9 +51,13 @@ defmodule Ammoprices.Scraping.Runner do
   defp process_results(parsed, retailer, caliber) do
     now = DateTime.truncate(DateTime.utc_now(), :second)
 
-    results = Enum.map(parsed, &process_product(&1, retailer, caliber, now))
-
-    successful = Enum.count(results, &(&1 == :ok))
+    {_results, successful} =
+      Enum.reduce(parsed, {[], 0}, fn product_data, {acc, count} ->
+        case process_product(product_data, retailer, caliber, now) do
+          :ok -> {[:ok | acc], count + 1}
+          :error -> {[:error | acc], count}
+        end
+      end)
 
     if successful > 0 do
       Catalog.mark_unseen_products_out_of_stock(retailer.id, caliber.id, now)
diff --git a/lib/ammoprices/scraping/scrape_job.ex b/lib/ammoprices/scraping/scrape_job.ex
index cbf0fd3..7cfb74a 100644
--- a/lib/ammoprices/scraping/scrape_job.ex
+++ b/lib/ammoprices/scraping/scrape_job.ex
@@ -2,8 +2,7 @@ defmodule Ammoprices.Scraping.ScrapeJob do
   @moduledoc false
   use Oban.Worker,
     queue: :scraping,
-    max_attempts: 3,
-    unique: [period: :infinity, states: [:available, :scheduled, :executing, :retryable, :suspended]]
+    max_attempts: 3
 
   alias Ammoprices.Catalog
   alias Ammoprices.Scraping.Runner
@@ -26,6 +25,7 @@ defmodule Ammoprices.Scraping.ScrapeJob do
   @impl Oban.Worker
   def perform(_job) do
     calibers = Catalog.list_calibers()
+    caliber_ids = Enum.map(calibers, & &1.id)
 
     for scraper <- @scrapers, caliber <- calibers do
       scrape_delay()
@@ -34,7 +34,7 @@ defmodule Ammoprices.Scraping.ScrapeJob do
 
     prune_stale_products()
 
-    Phoenix.PubSub.broadcast(Ammoprices.PubSub, "prices:updated", {:prices_updated, %{}})
+    Phoenix.PubSub.broadcast(Ammoprices.PubSub, "prices:updated", {:prices_updated, caliber_ids})
 
     :ok
   after
diff --git a/lib/ammoprices/scraping/text_detector.ex b/lib/ammoprices/scraping/text_detector.ex
index 95e4103..0b2f2a0 100644
--- a/lib/ammoprices/scraping/text_detector.ex
+++ b/lib/ammoprices/scraping/text_detector.ex
@@ -12,8 +12,8 @@ defmodule Ammoprices.Scraping.TextDetector do
     cond do
       String.contains?(downcased, "steel") -> "steel"
       String.contains?(downcased, "aluminum") -> "aluminum"
-      String.contains?(downcased, "nickel") -> "brass"
       String.contains?(downcased, "brass") -> "brass"
+      String.contains?(downcased, "nickel") -> "brass"
       true -> nil
     end
   end
diff --git a/lib/ammoprices_web/live/caliber_live/show.ex b/lib/ammoprices_web/live/caliber_live/show.ex
index b7b75ca..43a9380 100644
--- a/lib/ammoprices_web/live/caliber_live/show.ex
+++ b/lib/ammoprices_web/live/caliber_live/show.ex
@@ -103,15 +103,19 @@ defmodule AmmopricesWeb.CaliberLive.Show do
   end
 
   @impl true
-  def handle_info({:prices_updated, _}, socket) do
-    socket =
-      socket
-      |> load_filter_options()
-      |> load_products()
-      |> load_chart_data()
-      |> load_price_stats()
+  def handle_info({:prices_updated, caliber_ids}, socket) do
+    if socket.assigns.caliber.id in caliber_ids do
+      socket =
+        socket
+        |> load_filter_options()
+        |> load_products()
+        |> load_chart_data()
+        |> load_price_stats()
 
-    {:noreply, push_event(socket, "update-chart", %{data: socket.assigns.chart_data})}
+      {:noreply, push_event(socket, "update-chart", %{data: socket.assigns.chart_data})}
+    else
+      {:noreply, socket}
+    end
   end
 
   defp load_filter_options(socket) do
@@ -178,10 +182,15 @@ defmodule AmmopricesWeb.CaliberLive.Show do
     days = Map.get(@range_days, range, 30)
     daily_data = Prices.daily_averages_for_caliber(caliber.id, days: days)
 
+    {labels, avgs, mins} =
+      Enum.reduce(daily_data, {[], [], []}, fn d, {ls, as, ms} ->
+        {[Date.to_iso8601(d.date) | ls], [d.avg_ppr | as], [d.min_ppr | ms]}
+      end)
+
     chart_data = %{
-      labels: Enum.map(daily_data, &Date.to_iso8601(&1.date)),
-      avg: Enum.map(daily_data, & &1.avg_ppr),
-      min: Enum.map(daily_data, & &1.min_ppr)
+      labels: Enum.reverse(labels),
+      avg: Enum.reverse(avgs),
+      min: Enum.reverse(mins)
     }
 
     assign(socket, chart_data: chart_data, chart_empty?: daily_data == [])
diff --git a/lib/ammoprices_web/live/home_live.ex b/lib/ammoprices_web/live/home_live.ex
index 4ff1533..405d224 100644
--- a/lib/ammoprices_web/live/home_live.ex
+++ b/lib/ammoprices_web/live/home_live.ex
@@ -40,7 +40,7 @@ defmodule AmmopricesWeb.HomeLive do
   end
 
   @impl true
-  def handle_info({:prices_updated, _}, socket) do
+  def handle_info({:prices_updated, _caliber_ids}, socket) do
     {:noreply, assign_cheapest_map(socket)}
   end
 
diff --git a/test/ammoprices_web/live/caliber_live/show_test.exs b/test/ammoprices_web/live/caliber_live/show_test.exs
index c26e00c..62d66d2 100644
--- a/test/ammoprices_web/live/caliber_live/show_test.exs
+++ b/test/ammoprices_web/live/caliber_live/show_test.exs
@@ -228,7 +228,7 @@ defmodule AmmopricesWeb.CaliberLive.ShowTest do
       snapshot_fixture(%{product: new_product, price_per_round_cents: 18, in_stock: true, recorded_at: now})
 
       # Broadcast price update
-      Phoenix.PubSub.broadcast(Ammoprices.PubSub, "prices:updated", {:prices_updated, %{}})
+      Phoenix.PubSub.broadcast(Ammoprices.PubSub, "prices:updated", {:prices_updated, [caliber.id]})
 
       # The view should re-render with updated data
       html = render(view)
diff --git a/test/ammoprices_web/live/home_live_test.exs b/test/ammoprices_web/live/home_live_test.exs
index 3c4001d..dbed5a1 100644
--- a/test/ammoprices_web/live/home_live_test.exs
+++ b/test/ammoprices_web/live/home_live_test.exs
@@ -43,7 +43,7 @@ defmodule AmmopricesWeb.HomeLiveTest do
       {:ok, view, _html} = live(conn, "/")
 
       snapshot_fixture(%{product: product, price_per_round_cents: 17, in_stock: true, recorded_at: now})
-      Phoenix.PubSub.broadcast(Ammoprices.PubSub, "prices:updated", {:prices_updated, %{}})
+      Phoenix.PubSub.broadcast(Ammoprices.PubSub, "prices:updated", {:prices_updated, [caliber.id]})
 
       assert render(view) =~ "0.17"
     end