Address code review on elimination announcements

- Extract markEliminatedAndAnnounce helper, removing the duplicated load-existing-results / collect-newly-eliminated / gated-recalc pattern from the generate-bracket, generate-groups, and populate-knockout intents. - Wrap the Discord announcement in a log-only try/catch so a failed standings recalc/snapshot never fails the (already-committed) bracket generation action. - Drop eventId from the elimination recalc call so announcements don't pull in unrelated completed matches as Scored Matches. - Hoist the per-season draftPicks lookup in recalculateAffectedLeagues so the scored-match and elimination sections share one query. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E5gLy3K9vW65ogG8GTCjWE
2026-06-28 18:43:28 +00:00 · 2026-06-28 18:43:28 +00:00 · f75f3a228e
commit f75f3a228e
parent c7dcbb0682
2 changed files with 69 additions and 93 deletions
--- a/app/models/scoring-calculator.ts
+++ b/app/models/scoring-calculator.ts
@ -1812,6 +1812,19 @@ export async function recalculateAffectedLeagues(
      afterStandings.map((s) => [s.teamId, s.team.ownerId])
    );

+    // Shared draft-pick lookup for both the scored-match and elimination sections,
+    // loaded once per season only when at least one section needs it.
+    const needDraftPicks = allCompletedMatches.length > 0 || eliminatedIds.length > 0;
+    const draftPicks = needDraftPicks
+      ? await db.query.draftPicks.findMany({
+          where: eq(schema.draftPicks.seasonId, seasonId),
+        })
+      : [];
+    const draftedIds = new Set(draftPicks.map((p) => p.participantId));
+    const teamIdByParticipantId = new Map(
+      draftPicks.map((p) => [p.participantId, p.teamId])
+    );
+
    // Build scored matches for the notification.
    // Winners only appear if their team's score changed (they earned points this round).
    // Losers appear if their team's score changed OR they were definitively eliminated
@ -1821,11 +1834,6 @@ export async function recalculateAffectedLeagues(
    // correctly suppressed.
    let scoredMatches: ScoredMatch[] | undefined;
    if (allCompletedMatches.length > 0) {
-      const draftPicks = await db.query.draftPicks.findMany({
-        where: eq(schema.draftPicks.seasonId, seasonId),
-      });
-      const draftedIds = new Set(draftPicks.map((p) => p.participantId));
-
      const relevant = allCompletedMatches.filter(
        (m) =>
          (m.winnerId && draftedIds.has(m.winnerId)) ||
@ -1833,10 +1841,6 @@ export async function recalculateAffectedLeagues(
      );

      if (relevant.length > 0) {
-        const teamIdByParticipantId = new Map(
-          draftPicks.map((p) => [p.participantId, p.teamId])
-        );
-
        const lookupOwner = (participantId: string | null | undefined) => {
          if (!participantId) return undefined;
          const teamId = teamIdByParticipantId.get(participantId);
@ -1870,12 +1874,6 @@ export async function recalculateAffectedLeagues(
    // 0-pt result and no match, so they only surface here.
    let eliminatedTeams: EliminatedTeam[] | undefined;
    if (eliminatedIds.length > 0) {
-      const draftPicks = await db.query.draftPicks.findMany({
-        where: eq(schema.draftPicks.seasonId, seasonId),
-      });
-      const teamIdByParticipantId = new Map(
-        draftPicks.map((p) => [p.participantId, p.teamId])
-      );
      const teams = eliminatedIds
        .filter((id) => teamIdByParticipantId.has(id))
        .map((id) => {
--- a/app/routes/admin.sports-seasons.$id.events.$eventId.bracket.server.ts
+++ b/app/routes/admin.sports-seasons.$id.events.$eventId.bracket.server.ts
@ -148,6 +148,45 @@ async function scoreQualifyingBracket(
  await fanOutMajorIfPrimary(event, { markComplete: false });
 }

+/**
+ * Mark the given participants as eliminated (finalPosition = 0) and, for fantasy
+ * (non-qualifying) events, announce the teams newly eliminated by this run to the
+ * affected leagues' Discord channels. Returns the number of participants marked.
+ *
+ * "Newly eliminated" = participants with no prior result row, so re-running a
+ * generation step never re-announces the same teams. The announcement is a
+ * best-effort side effect: a failure must not fail the generation action, since
+ * the eliminations themselves are already committed. eventId is deliberately
+ * omitted from the recalc call so the announcement doesn't pull in unrelated
+ * completed matches as "Scored Matches".
+ */
+async function markEliminatedAndAnnounce(
+  event: { id: string; name: string | null; sportsSeasonId: string; isQualifyingEvent: boolean },
+  participantIds: string[]
+): Promise<number> {
+  const existingResults = await findParticipantResultsBySportsSeasonId(event.sportsSeasonId);
+  const alreadyHadResult = new Set(existingResults.map((r) => r.participantId));
+  const newlyEliminatedIds = participantIds.filter((id) => !alreadyHadResult.has(id));
+
+  for (const participantId of participantIds) {
+    await setParticipantResult(participantId, event.sportsSeasonId, 0);
+  }
+
+  // QPs (e.g. tennis/CS2 majors) don't get elimination announcements.
+  if (!event.isQualifyingEvent && newlyEliminatedIds.length > 0) {
+    try {
+      await recalculateAffectedLeagues(event.sportsSeasonId, database(), {
+        eventName: event.name ?? undefined,
+        eliminatedParticipantIds: newlyEliminatedIds,
+      });
+    } catch (err) {
+      logger.error("[Eliminations] Discord announcement failed:", err);
+    }
+  }
+
+  return participantIds.length;
+}
+
 export async function action({ request, params }: Route.ActionArgs) {
  const formData = await request.formData();
  const intent = formData.get("intent");
@ -294,46 +333,16 @@ export async function action({ request, params }: Route.ActionArgs) {
    try {
      await generateBracketFromTemplate(params.eventId, templateId, participantIds, regionOverride);

-      // PHASE 5.3: Mark participants NOT in the bracket as eliminated
+      // PHASE 5.3: Mark participants NOT in the bracket as eliminated (and announce).
      const event = await getScoringEventById(params.eventId);
      if (event) {
-        // Get all participants in the sports season
        const allParticipants = await findParticipantsBySportsSeasonId(params.id);
-
-        // Create a set of participants in the bracket for fast lookup
        const participantsInBracket = new Set(participantIds);
-
-        // Participants that already had a result before this run — used to scope
-        // the Discord announcement to teams newly eliminated by this action.
-        const existingResults = await findParticipantResultsBySportsSeasonId(params.id);
-        const alreadyHadResult = new Set(existingResults.map((r) => r.participantId));
-
-        // Mark participants NOT in the bracket as eliminated
-        const newlyEliminatedIds: string[] = [];
-        for (const participant of allParticipants) {
-          if (!participantsInBracket.has(participant.id)) {
-            if (!alreadyHadResult.has(participant.id)) {
-              newlyEliminatedIds.push(participant.id);
-            }
-            await setParticipantResult(
-              participant.id,
-              event.sportsSeasonId,
-              0 // 0 = didn't make playoffs, eliminated
-            );
-          }
-        }
-
-        logger.log(`[BracketGeneration] Marked ${allParticipants.length - participantIds.length} participants as eliminated`);
-
-        // Announce eliminations to league Discord channels (fantasy events only;
-        // QPs like tennis/CS2 majors don't get elimination announcements).
-        if (!event.isQualifyingEvent && newlyEliminatedIds.length > 0) {
-          await recalculateAffectedLeagues(event.sportsSeasonId, database(), {
-            eventId: event.id,
-            eventName: event.name ?? undefined,
-            eliminatedParticipantIds: newlyEliminatedIds,
-          });
-        }
+        const toEliminate = allParticipants
+          .filter((p) => !participantsInBracket.has(p.id))
+          .map((p) => p.id);
+        const eliminatedCount = await markEliminatedAndAnnounce(event, toEliminate);
+        logger.log(`[BracketGeneration] Marked ${eliminatedCount} participants as eliminated`);
      }

      // Update the event to store the template ID, scoring start round, and region config
@ -1004,30 +1013,16 @@ export async function action({ request, params }: Route.ActionArgs) {
      await generateBracketFromTemplate(params.eventId, templateId);

      // Eliminate participants from this sport season who are not in any group
-      const allParticipants = await findParticipantsBySportsSeasonId(params.id);
-      const existingResults = await findParticipantResultsBySportsSeasonId(params.id);
-      const alreadyHadResult = new Set(existingResults.map((r) => r.participantId));
-      const newlyEliminatedIds: string[] = [];
-      let eliminatedCount = 0;
-      for (const participant of allParticipants) {
-        if (!uniqueParticipants.has(participant.id)) {
-          if (!alreadyHadResult.has(participant.id)) {
-            newlyEliminatedIds.push(participant.id);
-          }
-          await setParticipantResult(participant.id, params.id, 0);
-          eliminatedCount++;
-        }
-      }
-
-      // Announce eliminations to league Discord channels (fantasy events only).
+      // (and announce to leagues for fantasy events).
      const groupsEvent = await getScoringEventById(params.eventId);
-      if (groupsEvent && !groupsEvent.isQualifyingEvent && newlyEliminatedIds.length > 0) {
-        await recalculateAffectedLeagues(groupsEvent.sportsSeasonId, database(), {
-          eventId: groupsEvent.id,
-          eventName: groupsEvent.name ?? undefined,
-          eliminatedParticipantIds: newlyEliminatedIds,
-        });
+      if (!groupsEvent) {
+        return { error: "Event not found" };
      }
+      const allParticipants = await findParticipantsBySportsSeasonId(params.id);
+      const toEliminate = allParticipants
+        .filter((p) => !uniqueParticipants.has(p.id))
+        .map((p) => p.id);
+      const eliminatedCount = await markEliminatedAndAnnounce(groupsEvent, toEliminate);

      return {
        success: `Groups and knockout bracket structure created successfully${eliminatedCount > 0 ? ` (${eliminatedCount} participant(s) not in any group marked as eliminated)` : ""}`,
@ -1152,32 +1147,15 @@ export async function action({ request, params }: Route.ActionArgs) {
      // Assign participants to knockout bracket
      await assignParticipantsToKnockout(params.eventId, assignments);

-      // Mark eliminated group participants with finalPosition = 0
+      // Mark eliminated group participants with finalPosition = 0 (and announce
+      // the group-stage eliminations to leagues for fantasy events).
      const eliminatedIds = await getEliminatedParticipantIds(params.eventId);
-      const existingResults = await findParticipantResultsBySportsSeasonId(params.id);
-      const alreadyHadResult = new Set(existingResults.map((r) => r.participantId));
-      const newlyEliminatedIds: string[] = [];
-      for (const participantId of eliminatedIds) {
-        if (!alreadyHadResult.has(participantId)) {
-          newlyEliminatedIds.push(participantId);
-        }
-        await setParticipantResult(participantId, event.sportsSeasonId, 0);
-      }
+      await markEliminatedAndAnnounce(event, eliminatedIds);

      logger.log(
        `[PopulateKnockout] Assigned 32 participants to knockout, marked ${eliminatedIds.length} as eliminated`
      );

-      // Announce group-stage eliminations to league Discord channels (fantasy
-      // events only; QPs don't get elimination announcements).
-      if (!event.isQualifyingEvent && newlyEliminatedIds.length > 0) {
-        await recalculateAffectedLeagues(event.sportsSeasonId, database(), {
-          eventId: event.id,
-          eventName: event.name ?? undefined,
-          eliminatedParticipantIds: newlyEliminatedIds,
-        });
-      }
-
      return { success: "Knockout bracket populated successfully" };
    } catch (error) {
      logger.error("Error populating knockout:", error);