claude/discord-qp-notifications-75wa7k (#122)

Co-authored-by: Claude <noreply@anthropic.com>
Reviewed-on: #122

app/models/__tests__/qualifying-points.test.ts

@@ -2,9 +2,48 @@ import { describe, it, expect } from "vitest";
 import {
   calculateSplitQualifyingPoints,
   DEFAULT_QP_VALUES,
+  diffChangedQualifyingPoints,
   hasProcessedQualifyingPlacement,
 } from "../qualifying-points";
 
+describe("diffChangedQualifyingPoints", () => {
+  it("flags a changed value, a first-time score (null → value), and ignores unchanged", () => {
+    const before = [
+      { id: "A", qp: "10.00" },
+      { id: "B", qp: "5.00" },
+      { id: "C", qp: null },
+    ];
+    const after = [
+      { id: "A", qp: "10.00" }, // unchanged
+      { id: "B", qp: "8.00" }, // changed
+      { id: "C", qp: "3.00" }, // first-time score
+    ];
+
+    expect([...diffChangedQualifyingPoints(before, after)].toSorted()).toEqual(["B", "C"]);
+  });
+
+  it("normalizes decimal formatting so 10 and 10.00 are equal", () => {
+    const changed = diffChangedQualifyingPoints([{ id: "A", qp: "10" }], [{ id: "A", qp: "10.00" }]);
+    expect(changed.size).toBe(0);
+  });
+
+  it("does not report participants absent from the after set", () => {
+    const changed = diffChangedQualifyingPoints(
+      [
+        { id: "A", qp: "10.00" },
+        { id: "D", qp: "2.00" },
+      ],
+      [{ id: "A", qp: "10.00" }],
+    );
+    expect(changed.size).toBe(0);
+  });
+
+  it("treats a value → zero drop as a change", () => {
+    const changed = diffChangedQualifyingPoints([{ id: "A", qp: "10.00" }], [{ id: "A", qp: "0.00" }]);
+    expect([...changed]).toEqual(["A"]);
+  });
+});
+
 describe("Qualifying Points Configuration", () => {
   describe("DEFAULT_QP_VALUES", () => {
     it("should have 16 placement values", () => {

app/models/__tests__/scoring-calculator-qp-notify.test.ts

@@ -0,0 +1,107 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import type * as QualifyingPointsModule from "~/models/qualifying-points";
+
+// Spy on the QP Discord notification, and no-op the per-participant total recalc
+// (its own eventResults query would otherwise interfere with the before/after
+// snapshot counter below). Everything else in the scorer runs for real.
+vi.mock("~/services/qualifying-points-discord.server", () => ({
+  notifyQualifyingPointsUpdate: vi.fn().mockResolvedValue(undefined),
+}));
+
+vi.mock("~/models/qualifying-points", async (importActual) => {
+  const actual = await importActual<typeof QualifyingPointsModule>();
+  return {
+    ...actual,
+    recalculateParticipantQP: vi.fn().mockResolvedValue({ totalQP: 0, eventsScored: 0 }),
+  };
+});
+
+import { processQualifyingEvent } from "../scoring-calculator";
+import { DEFAULT_QP_VALUES } from "../qualifying-points";
+import { notifyQualifyingPointsUpdate } from "~/services/qualifying-points-discord.server";
+
+const EVENT_ID = "event-1";
+const SPORTS_SEASON_ID = "sports-season-1";
+
+type ResultRow = {
+  id: string;
+  seasonParticipantId: string;
+  placement: number | null;
+  qualifyingPointsAwarded: string | null;
+  scoringEvent: { id: string; sportsSeasonId: string };
+};
+
+/**
+ * Mock db for the non-bracket `processQualifyingEvent` path. `eventResults.findMany`
+ * returns `before` on its first call (the initial getEventResults snapshot) and
+ * `after` on its second (the post-reprocess re-query that drives change detection).
+ * Awarding updates go to a no-op update chain — `after` stands in for the DB state
+ * once the scorer has written its QP.
+ */
+function makeDb(before: ResultRow[], after: ResultRow[]) {
+  const results = [before, after];
+  let findManyCall = 0;
+  return {
+    query: {
+      scoringEvents: {
+        findFirst: async () => ({
+          id: EVENT_ID,
+          sportsSeasonId: SPORTS_SEASON_ID,
+          isQualifyingEvent: true,
+          bracketTemplateId: null,
+          sportsSeason: { majorsCompleted: 1 },
+        }),
+      },
+      eventResults: {
+        findMany: async () => results[Math.min(findManyCall++, 1)],
+      },
+      qualifyingPointConfig: {
+        findMany: async () =>
+          DEFAULT_QP_VALUES.map((qp) => ({ placement: qp.placement, points: qp.points.toString() })),
+      },
+    },
+    update: () => ({ set: () => ({ where: async () => [] }) }),
+  } as never;
+}
+
+const scoringEvent = { id: EVENT_ID, sportsSeasonId: SPORTS_SEASON_ID };
+
+beforeEach(() => {
+  vi.clearAllMocks();
+});
+
+describe("processQualifyingEvent — QP change notification", () => {
+  it("notifies only participants whose QP changed (first-time score: null → value)", async () => {
+    const db = makeDb(
+      [
+        { id: "r1", seasonParticipantId: "p1", placement: 1, qualifyingPointsAwarded: null, scoringEvent },
+        { id: "r2", seasonParticipantId: "p2", placement: 20, qualifyingPointsAwarded: "0.00", scoringEvent },
+      ],
+      [
+        { id: "r1", seasonParticipantId: "p1", placement: 1, qualifyingPointsAwarded: "100.00", scoringEvent },
+        { id: "r2", seasonParticipantId: "p2", placement: 20, qualifyingPointsAwarded: "0.00", scoringEvent },
+      ],
+    );
+
+    await processQualifyingEvent(EVENT_ID, db);
+
+    expect(notifyQualifyingPointsUpdate).toHaveBeenCalledTimes(1);
+    const [ssId, evId, , filter] = vi.mocked(notifyQualifyingPointsUpdate).mock.calls[0];
+    expect(ssId).toBe(SPORTS_SEASON_ID);
+    expect(evId).toBe(EVENT_ID);
+    // p1 went null → 100 (changed); p2 stayed at 0 (unchanged).
+    expect([...(filter as Set<string>)]).toEqual(["p1"]);
+  });
+
+  it("does not notify when a re-sync produces identical QP", async () => {
+    const rows: ResultRow[] = [
+      { id: "r1", seasonParticipantId: "p1", placement: 1, qualifyingPointsAwarded: "100.00", scoringEvent },
+      { id: "r2", seasonParticipantId: "p2", placement: 20, qualifyingPointsAwarded: "0.00", scoringEvent },
+    ];
+    const db = makeDb(rows, rows.map((r) => ({ ...r })));
+
+    await processQualifyingEvent(EVENT_ID, db);
+
+    expect(notifyQualifyingPointsUpdate).not.toHaveBeenCalled();
+  });
+});

app/models/qualifying-points.ts

@@ -33,6 +33,33 @@ export function roundQualifyingPoints(points: number): number {
   return Math.round((points + Number.EPSILON) * 100) / 100;
 }
 
+/**
+ * Given a participant's awarded QP before and after a re-score, return the ids
+ * whose QP changed. A row's `qp` is the raw decimal string (or null when no QP
+ * was awarded). A participant counts as changed when their new value differs from
+ * the old, including a first-time score (null → value). Participants absent from
+ * `after` are not reported (their removal is handled by the caller's recalc).
+ *
+ * Shared by the two QP re-score paths — processQualifyingEvent (sibling windows,
+ * CS2, manual admin) and rescoreTennisBracketAndDetectChanges (tennis primary) —
+ * so the "only announce real changes" semantics live in one place.
+ */
+export function diffChangedQualifyingPoints(
+  before: Iterable<{ id: string; qp: string | null }>,
+  after: Iterable<{ id: string; qp: string | null }>
+): Set<string> {
+  const beforeQP = new Map<string, number>();
+  for (const r of before) {
+    if (r.qp !== null) beforeQP.set(r.id, parseFloat(r.qp));
+  }
+  const changed = new Set<string>();
+  for (const r of after) {
+    if (r.qp === null) continue;
+    if (beforeQP.get(r.id) !== parseFloat(r.qp)) changed.add(r.id);
+  }
+  return changed;
+}
+
 export function calculateSplitQualifyingPoints(
   placement: number,
   tieCount: number,

app/models/scoring-calculator.ts

@@ -21,6 +21,7 @@ import { logger } from "~/lib/logger";
 import { getEventResults } from "./event-result";
 import {
   calculateSplitQualifyingPoints,
+  diffChangedQualifyingPoints,
   getQPConfig,
   hasProcessedQualifyingPlacement,
   recalculateParticipantQP,
@@ -879,6 +880,15 @@ export async function processQualifyingEvent(
   // Check if this was already processed (for majorsCompleted counter)
   const wasAlreadyProcessed = hasProcessedQualifyingPlacement(results);
 
+  // Snapshot awarded QP before reprocessing so the Discord notification below can
+  // announce only the participants whose QP actually changed. Without this, a
+  // sibling window re-scored on every fan-out sync (syncTournamentResults) would
+  // re-ping the full QP standings each run even when nothing changed.
+  const beforeQP = results.map((r) => ({
+    id: r.seasonParticipantId,
+    qp: r.qualifyingPointsAwarded,
+  }));
+
   if (event.bracketTemplateId) {
     // Bracket-based qualifying event (e.g. CS2 Champions Stage): QP is owned by the
     // bracket/stage writers, which assign each placement its STRUCTURAL tie span.
@@ -960,10 +970,23 @@ export async function processQualifyingEvent(
     `[ScoringCalculator] Processed qualifying event ${eventId}: awarded QP to ${results.length} participants`
   );
 
-  try {
-    await notifyQualifyingPointsUpdate(event.sportsSeasonId, eventId, db);
-  } catch (error) {
-    logger.error(`[ScoringCalculator] QP Discord notification failed for event ${eventId}:`, error);
+  // Announce only participants whose awarded QP changed vs. the pre-reprocess
+  // snapshot (a differing value, or a first-time score: null → value). This keeps
+  // repeated fan-out syncs of an unchanged window from re-pinging the standings.
+  const afterRows = await db.query.eventResults.findMany({
+    where: eq(schema.eventResults.scoringEventId, eventId),
+  });
+  const changedParticipantIds = diffChangedQualifyingPoints(
+    beforeQP,
+    afterRows.map((r) => ({ id: r.seasonParticipantId, qp: r.qualifyingPointsAwarded }))
+  );
+
+  if (changedParticipantIds.size > 0) {
+    try {
+      await notifyQualifyingPointsUpdate(event.sportsSeasonId, eventId, db, changedParticipantIds);
+    } catch (error) {
+      logger.error(`[ScoringCalculator] QP Discord notification failed for event ${eventId}:`, error);
+    }
   }
 }
 

app/services/match-sync/__tests__/sync-tennis-rescore.test.ts

@@ -0,0 +1,120 @@
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import type * as QualifyingPointsModule from "~/models/qualifying-points";
+
+// The helper under test re-derives QP via processQualifyingBracketEvent and
+// recalculates dropped participants — both stubbed so the test drives the
+// before/after event_results rows directly and asserts the change-detection diff.
+vi.mock("~/models/scoring-calculator", () => ({
+  processMatchResult: vi.fn(),
+  autoCompleteRoundIfDone: vi.fn(),
+  processQualifyingBracketEvent: vi.fn().mockResolvedValue(undefined),
+  recalculateAffectedLeagues: vi.fn(),
+}));
+
+// Keep the real diffChangedQualifyingPoints (the change-detection primitive under
+// test here); only stub the dropped-participant total recalc.
+vi.mock("~/models/qualifying-points", async (importActual) => {
+  const actual = await importActual<typeof QualifyingPointsModule>();
+  return {
+    ...actual,
+    recalculateParticipantQP: vi.fn().mockResolvedValue(undefined),
+  };
+});
+
+import { rescoreTennisBracketAndDetectChanges } from "../index";
+import { processQualifyingBracketEvent } from "~/models/scoring-calculator";
+import { recalculateParticipantQP } from "~/models/qualifying-points";
+
+const EVENT_ID = "ev-1";
+const SPORTS_SEASON_ID = "ss-1";
+
+type Row = { id: string; qp: string | null };
+
+/**
+ * Fake Drizzle db whose two `select().from().where()` calls resolve, in order, to
+ * the provided before-rows then after-rows. `transaction` runs its callback with
+ * the same fake, and `delete().where()` is a no-op.
+ */
+function makeDb(beforeRows: Row[], afterRows: Row[]) {
+  const results = [beforeRows, afterRows];
+  let selectCall = 0;
+  const db = {
+    select: vi.fn(() => ({
+      from: vi.fn(() => ({
+        where: vi.fn(() => Promise.resolve(results[selectCall++] ?? [])),
+      })),
+    })),
+    delete: vi.fn(() => ({ where: vi.fn(() => Promise.resolve(undefined)) })),
+    transaction: vi.fn(async (fn: (tx: unknown) => Promise<void>) => fn(db)),
+  };
+  return db;
+}
+
+beforeEach(() => {
+  vi.clearAllMocks();
+});
+
+describe("rescoreTennisBracketAndDetectChanges", () => {
+  it("flags participants whose QP changed and newly-scored participants, ignoring unchanged ones", async () => {
+    const db = makeDb(
+      [
+        { id: "A", qp: "10" },
+        { id: "B", qp: "5" },
+      ],
+      [
+        { id: "A", qp: "10" }, // unchanged
+        { id: "B", qp: "8" }, // changed 5 -> 8
+        { id: "C", qp: "3" }, // newly scored
+      ],
+    );
+
+    const changed = await rescoreTennisBracketAndDetectChanges(EVENT_ID, SPORTS_SEASON_ID, db as never);
+
+    expect([...changed].toSorted()).toEqual(["B", "C"]);
+    expect(processQualifyingBracketEvent).toHaveBeenCalledWith(EVENT_ID, db);
+    // No participant dropped out, so no manual total recalc.
+    expect(recalculateParticipantQP).not.toHaveBeenCalled();
+  });
+
+  it("treats a null-before → value-after transition as a change", async () => {
+    const db = makeDb([{ id: "A", qp: null }], [{ id: "A", qp: "10" }]);
+
+    const changed = await rescoreTennisBracketAndDetectChanges(EVENT_ID, SPORTS_SEASON_ID, db as never);
+
+    expect([...changed]).toEqual(["A"]);
+  });
+
+  it("announces nothing when a re-sync produces identical QP", async () => {
+    const db = makeDb(
+      [
+        { id: "A", qp: "10" },
+        { id: "B", qp: "5" },
+      ],
+      [
+        { id: "A", qp: "10" },
+        { id: "B", qp: "5" },
+      ],
+    );
+
+    const changed = await rescoreTennisBracketAndDetectChanges(EVENT_ID, SPORTS_SEASON_ID, db as never);
+
+    expect(changed.size).toBe(0);
+    expect(recalculateParticipantQP).not.toHaveBeenCalled();
+  });
+
+  it("recalculates totals for participants dropped from the rewritten results", async () => {
+    const db = makeDb(
+      [
+        { id: "A", qp: "10" },
+        { id: "D", qp: "2" }, // present before, gone after (early-round loser)
+      ],
+      [{ id: "A", qp: "10" }],
+    );
+
+    const changed = await rescoreTennisBracketAndDetectChanges(EVENT_ID, SPORTS_SEASON_ID, db as never);
+
+    expect(changed.size).toBe(0); // A unchanged; D no longer in results
+    expect(recalculateParticipantQP).toHaveBeenCalledTimes(1);
+    expect(recalculateParticipantQP).toHaveBeenCalledWith("D", SPORTS_SEASON_ID, db);
+  });
+});

app/services/match-sync/index.ts

@@ -23,7 +23,11 @@ import {
   recalculateAffectedLeagues,
 } from "~/models/scoring-calculator";
 import { fanOutMajorIfPrimary } from "~/services/sync-tournament-results";
-import { recalculateParticipantQP } from "~/models/qualifying-points";
+import { notifyQualifyingPointsUpdate } from "~/services/qualifying-points-discord.server";
+import {
+  recalculateParticipantQP,
+  diffChangedQualifyingPoints,
+} from "~/models/qualifying-points";
 import { getScoringEventById, updateScoringEvent, isReadOnlySibling } from "~/models/scoring-event";
 import { findMatchingTeamName, normalizeTeamName } from "~/lib/normalize-team-name";
 import { buildUnmatchedTeamResolutionView } from "~/lib/unmatched-team-reconciliation";
@@ -592,35 +596,13 @@ export async function syncTennisDraw(eventId: string): Promise<DrawSyncResult> {
   const { written, completed } = await populateBracketFromDraw(eventId, resolvedMatches);
 
   // ---- Score (qualifying points) + fan out to siblings ----------------------
-  // The tennis bracket is the sole QP source for its event, so reconcile stale
-  // rows: snapshot existing result rows, clear them, re-derive from the bracket,
-  // then recalc QP totals for any participant who no longer earns points (e.g.
-  // early-round losers — only Round of 16+ losers score). writeEventResultsQP
-  // upserts but never deletes, so without this a previously mis-scored player
-  // would keep phantom QP across re-syncs.
-  //
-  // Wrapped in a transaction so a mid-rescore failure can't leave the event with
-  // its results deleted and not rebuilt. NOTE: this clears ALL event_results for
-  // the event, including any manually-entered rows (e.g. a notParticipating
-  // withdrawal) — acceptable because the synced draw is authoritative for tennis.
-  await db.transaction(async (tx) => {
-    const beforeRows = await tx
-      .select({ pid: schema.eventResults.seasonParticipantId })
-      .from(schema.eventResults)
-      .where(eq(schema.eventResults.scoringEventId, eventId));
-    await tx.delete(schema.eventResults).where(eq(schema.eventResults.scoringEventId, eventId));
-
-    await processQualifyingBracketEvent(eventId, tx);
-
-    const afterRows = await tx
-      .select({ pid: schema.eventResults.seasonParticipantId })
-      .from(schema.eventResults)
-      .where(eq(schema.eventResults.scoringEventId, eventId));
-    const afterIds = new Set(afterRows.map((r) => r.pid));
-    for (const { pid } of beforeRows) {
-      if (!afterIds.has(pid)) await recalculateParticipantQP(pid, sportsSeasonId, tx);
-    }
-  });
+  // Re-derive QP from the bracket and learn which participants' QP changed so the
+  // Discord notification below only announces new/changed results on a re-sync.
+  const changedParticipantIds = await rescoreTennisBracketAndDetectChanges(
+    eventId,
+    sportsSeasonId,
+    db,
+  );
 
   await recalculateAffectedLeagues(sportsSeasonId, db, {
     eventId,
@@ -631,5 +613,76 @@ export async function syncTennisDraw(eventId: string): Promise<DrawSyncResult> {
     { markComplete: false },
   );
 
+  // Announce QP changes for the primary window. Sibling windows are announced by
+  // the fan-out (via processQualifyingEvent); the primary is scored directly by
+  // processQualifyingBracketEvent, which does NOT notify — so do it here. Run
+  // outside the rescore transaction so the webhook HTTP call neither holds the
+  // transaction open nor rolls back the score if Discord fails.
+  if (changedParticipantIds.size > 0) {
+    try {
+      await notifyQualifyingPointsUpdate(sportsSeasonId, eventId, db, changedParticipantIds);
+    } catch (error) {
+      logger.error(`[syncTennisDraw] QP Discord notification failed for event ${eventId}:`, error);
+    }
+  }
+
   return { matchesWritten: written, completed, participantsCreated, unmatched };
 }
+
+/**
+ * Re-derive a tennis major's qualifying points from its bracket and report which
+ * season participants' awarded QP changed since the previous scoring.
+ *
+ * The synced draw is authoritative for tennis, so this clears ALL event_results
+ * for the event (including any manually-entered rows, e.g. a notParticipating
+ * withdrawal) and rebuilds them from the bracket. Change detection therefore
+ * snapshots each participant's awarded QP *before* the delete and diffs it against
+ * the freshly written rows: a value that differs, or a participant scored for the
+ * first time (null → value), counts as changed. Wrapped in a transaction so a
+ * mid-rescore failure can't leave the event with results deleted-but-not-rebuilt.
+ *
+ * writeEventResultsQP (inside processQualifyingBracketEvent) upserts but never
+ * deletes, so QP totals for participants who no longer earn points (e.g. an
+ * early-round loser dropped from the rewritten results — only Round of 16+ losers
+ * score) are recalculated by hand.
+ *
+ * Returns the set of season_participant ids whose QP changed, used to scope the
+ * QP Discord notification so a re-sync that changes nothing announces nothing.
+ */
+export async function rescoreTennisBracketAndDetectChanges(
+  eventId: string,
+  sportsSeasonId: string,
+  db: ReturnType<typeof database>,
+): Promise<Set<string>> {
+  let changed = new Set<string>();
+
+  await db.transaction(async (tx) => {
+    const beforeRows = await tx
+      .select({
+        id: schema.eventResults.seasonParticipantId,
+        qp: schema.eventResults.qualifyingPointsAwarded,
+      })
+      .from(schema.eventResults)
+      .where(eq(schema.eventResults.scoringEventId, eventId));
+    await tx.delete(schema.eventResults).where(eq(schema.eventResults.scoringEventId, eventId));
+
+    await processQualifyingBracketEvent(eventId, tx);
+
+    const afterRows = await tx
+      .select({
+        id: schema.eventResults.seasonParticipantId,
+        qp: schema.eventResults.qualifyingPointsAwarded,
+      })
+      .from(schema.eventResults)
+      .where(eq(schema.eventResults.scoringEventId, eventId));
+    const afterIds = new Set(afterRows.map((r) => r.id));
+
+    for (const { id } of beforeRows) {
+      if (!afterIds.has(id)) await recalculateParticipantQP(id, sportsSeasonId, tx);
+    }
+
+    changed = diffChangedQualifyingPoints(beforeRows, afterRows);
+  });
+
+  return changed;
+}