From 828dcf5a08aba355eda10e54b890a8ceec563674 Mon Sep 17 00:00:00 2001
From: "alexei.dolgolyov" <dolgolyov.alexei@gmail.com>
Date: Tue, 5 May 2026 13:46:34 +0300
Subject: [PATCH] =?UTF-8?q?fix(phase-7):=20close=20review=20notes=20?=
 =?UTF-8?q?=E2=80=94=20hoist=20anomaly=20dedup=20query,=20drop=20dead=20ex?=
 =?UTF-8?q?pr?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Phase 7 reviewer (Sonnet, combined backend + frontend) flagged 3 🟡 warnings;
two real fixes here, one tracking:

W1 — DetectAnomaliesUseCase had an undocumented N+1: _anomalyRepo.ListAsync
  was called inside ProcessEventAsync, once per event. Hoisted to ExecuteAsync
  before the loop and threaded into ProcessEventAsync as a parameter. The
  per-event slice happens in-memory now. O(N_events) DB round-trips → 1.

W2 — AnomalyDetector.ExtractMatchWinProbabilities had a dead expression
  '(decimal?)null ?? 0m' that always evaluated to 0m. Simplified to
  'drawBet is not null ? rawDraw / total : 0m'. The 0m is never surfaced
  anyway (PDraw in the return uses the same null guard), so behaviour is
  identical.

W3 — PLAN.md row updated with both Phase 7 commit hashes (a6ff368 backend
  + 12208a4 frontend) and review verdict.

Build 0/0, 276 tests still passing.
---
 plans/initial-implementation/PLAN.md                |  2 +-
 .../UseCases/DetectAnomaliesUseCase.cs              | 13 +++++++++----
 .../AnomalyDetection/AnomalyDetector.cs             |  2 +-
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/plans/initial-implementation/PLAN.md b/plans/initial-implementation/PLAN.md
index eb8cf9f..64b7fc3 100644
--- a/plans/initial-implementation/PLAN.md
+++ b/plans/initial-implementation/PLAN.md
@@ -69,7 +69,7 @@ parameter configurable.
 | Phase 4: Application + Workers | backend | ✅ Done | ⚠️ Pass with notes (Sonnet) | ✅ Build OK + 202/202 tests | ✅ 2acbaa5 |
 | Phase 5: Host + Theme + i18n | frontend | ✅ Done | ⚠️ Pass with notes (Sonnet, combined batch) | ✅ Build OK + 11/11 UI tests | ✅ batch (e4d8476…686550d…+) |
 | Phase 6: Event browsing UI | frontend | ✅ Done | ⚠️ Pass with notes (Sonnet) | ✅ Build OK + 228/228 tests | ✅ 553db2b |
-| Phase 7: Anomaly detection | fullstack | ✅ Done | ⬜ | ✅ Build OK + 276/276 tests | ⬜ |
+| Phase 7: Anomaly detection | fullstack | ✅ Done | ⚠️ Pass with notes (Sonnet) | ✅ Build OK + 276/276 tests | ✅ a6ff368 + 12208a4 |
 | Phase 8: Results loader | fullstack | ⬜ Not Started | ⬜ | ⏭️ Big Bang | ⬜ |
 | Phase 9: Packaging + polish | fullstack | ⬜ Not Started | ⬜ | ⬜ | ⬜ |
 
diff --git a/src/Marathon.Application/UseCases/DetectAnomaliesUseCase.cs b/src/Marathon.Application/UseCases/DetectAnomaliesUseCase.cs
index da184d0..0fa93e3 100644
--- a/src/Marathon.Application/UseCases/DetectAnomaliesUseCase.cs
+++ b/src/Marathon.Application/UseCases/DetectAnomaliesUseCase.cs
@@ -70,13 +70,18 @@ public sealed class DetectAnomaliesUseCase
         var now  = DateTimeOffset.UtcNow.ToOffset(MoscowOffset);
         var from = now - SnapshotLookback;
 
+        // Hoisted outside the per-event loop: load existing anomalies ONCE per cycle
+        // and slice per-event in the loop. Previously this was reloaded per event
+        // (O(N_events) round-trips). Reviewer W1, Phase 7.
+        var existingAnomalies = await _anomalyRepo.ListAsync(ct);
+
         foreach (var ev in events)
         {
             ct.ThrowIfCancellationRequested();
 
             try
             {
-                newAnomalyCount += await ProcessEventAsync(detector, ev, from, now, ct);
+                newAnomalyCount += await ProcessEventAsync(detector, ev, from, now, existingAnomalies, ct);
             }
             catch (OperationCanceledException)
             {
@@ -104,6 +109,7 @@ public sealed class DetectAnomaliesUseCase
         Event ev,
         DateTimeOffset from,
         DateTimeOffset to,
+        IReadOnlyList<Anomaly> existingAnomalies,
         CancellationToken ct)
     {
         var snapshots = await _snapshotRepo.ListByEventAsync(ev.Id, from, to, ct);
@@ -112,9 +118,8 @@ public sealed class DetectAnomaliesUseCase
         if (detected.Count == 0)
             return 0;
 
-        // Load existing anomalies for this event so we can deduplicate.
-        var existing = await _anomalyRepo.ListAsync(ct);
-        var existingForEvent = existing
+        // Slice the cycle-wide existing-anomaly list to just this event for dedup.
+        var existingForEvent = existingAnomalies
             .Where(a => a.EventId == ev.Id)
             .ToList();
 
diff --git a/src/Marathon.Domain/AnomalyDetection/AnomalyDetector.cs b/src/Marathon.Domain/AnomalyDetection/AnomalyDetector.cs
index 74dc7b2..810f541 100644
--- a/src/Marathon.Domain/AnomalyDetection/AnomalyDetector.cs
+++ b/src/Marathon.Domain/AnomalyDetection/AnomalyDetector.cs
@@ -188,7 +188,7 @@ public sealed class AnomalyDetector
         // Normalise so they sum to 1.
         decimal p1   = rawP1   / total;
         decimal p2   = rawP2   / total;
-        decimal pDraw = drawBet is not null ? rawDraw / total : (decimal?)null ?? 0m;
+        decimal pDraw = drawBet is not null ? rawDraw / total : 0m;
 
         return new MatchWinProbabilities(
             P1:     p1,