fix(phase-7): close review notes — hoist anomaly dedup query, drop dead expr
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 (a6ff368backend +12208a4frontend) and review verdict. Build 0/0, 276 tests still passing.
This commit is contained in:
@@ -69,7 +69,7 @@ parameter configurable.
|
|||||||
| Phase 4: Application + Workers | backend | ✅ Done | ⚠️ Pass with notes (Sonnet) | ✅ Build OK + 202/202 tests | ✅ 2acbaa5 |
|
| 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 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 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 8: Results loader | fullstack | ⬜ Not Started | ⬜ | ⏭️ Big Bang | ⬜ |
|
||||||
| Phase 9: Packaging + polish | fullstack | ⬜ Not Started | ⬜ | ⬜ | ⬜ |
|
| Phase 9: Packaging + polish | fullstack | ⬜ Not Started | ⬜ | ⬜ | ⬜ |
|
||||||
|
|
||||||
|
|||||||
@@ -70,13 +70,18 @@ public sealed class DetectAnomaliesUseCase
|
|||||||
var now = DateTimeOffset.UtcNow.ToOffset(MoscowOffset);
|
var now = DateTimeOffset.UtcNow.ToOffset(MoscowOffset);
|
||||||
var from = now - SnapshotLookback;
|
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)
|
foreach (var ev in events)
|
||||||
{
|
{
|
||||||
ct.ThrowIfCancellationRequested();
|
ct.ThrowIfCancellationRequested();
|
||||||
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
newAnomalyCount += await ProcessEventAsync(detector, ev, from, now, ct);
|
newAnomalyCount += await ProcessEventAsync(detector, ev, from, now, existingAnomalies, ct);
|
||||||
}
|
}
|
||||||
catch (OperationCanceledException)
|
catch (OperationCanceledException)
|
||||||
{
|
{
|
||||||
@@ -104,6 +109,7 @@ public sealed class DetectAnomaliesUseCase
|
|||||||
Event ev,
|
Event ev,
|
||||||
DateTimeOffset from,
|
DateTimeOffset from,
|
||||||
DateTimeOffset to,
|
DateTimeOffset to,
|
||||||
|
IReadOnlyList<Anomaly> existingAnomalies,
|
||||||
CancellationToken ct)
|
CancellationToken ct)
|
||||||
{
|
{
|
||||||
var snapshots = await _snapshotRepo.ListByEventAsync(ev.Id, from, to, ct);
|
var snapshots = await _snapshotRepo.ListByEventAsync(ev.Id, from, to, ct);
|
||||||
@@ -112,9 +118,8 @@ public sealed class DetectAnomaliesUseCase
|
|||||||
if (detected.Count == 0)
|
if (detected.Count == 0)
|
||||||
return 0;
|
return 0;
|
||||||
|
|
||||||
// Load existing anomalies for this event so we can deduplicate.
|
// Slice the cycle-wide existing-anomaly list to just this event for dedup.
|
||||||
var existing = await _anomalyRepo.ListAsync(ct);
|
var existingForEvent = existingAnomalies
|
||||||
var existingForEvent = existing
|
|
||||||
.Where(a => a.EventId == ev.Id)
|
.Where(a => a.EventId == ev.Id)
|
||||||
.ToList();
|
.ToList();
|
||||||
|
|
||||||
|
|||||||
@@ -188,7 +188,7 @@ public sealed class AnomalyDetector
|
|||||||
// Normalise so they sum to 1.
|
// Normalise so they sum to 1.
|
||||||
decimal p1 = rawP1 / total;
|
decimal p1 = rawP1 / total;
|
||||||
decimal p2 = rawP2 / 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(
|
return new MatchWinProbabilities(
|
||||||
P1: p1,
|
P1: p1,
|
||||||
|
|||||||
Reference in New Issue
Block a user