Stop the simulator re-run from trampling its caller's side effects

A review of this branch found four problems, all downstream of one decision:
calling runSportsSeasonSimulation from inside the result path. That function
does three jobs — recompute probabilities, recalculate standings, write the
daily EV snapshot — and the result path wants only the first.

1. The Discord standings post was silently suppressed on every scored match,
   for all 13 bracket-aware sports.

   recalculateAffectedLeagues detects change by snapshotting teamStandings,
   recalculating, then diffing; changedTeamIds gates the notification. But
   processMatchResult runs updateProbabilitiesAfterResult first, which now
   reached the runner's own recalculateStandings. The new totals were therefore
   already written when the "before" snapshot was taken, the diff came back
   empty, and the post never fired. previousRank went the same way:
   recalculateStandings rolls it forward on every call, so the extra one erased
   rank movement.

   runSportsSeasonSimulation now takes skipStandingsRecalc / skipSnapshots and
   the probability updater passes both. The snapshot is skipped because it is a
   per-day series keyed by snapshotDate — writing it per match result just
   overwrites the day's row with intra-day values.

2. finalizeQualifyingPoints marks the season completed immediately before
   calling the updater, and the runner rejects a completed season outright. With
   the ICM fallback gone that failed every time, stranding anyone still in the
   unfinished set on permanently stale probabilities — reachable for
   cs2_major_qualifying_points, the one bracket-aware qualifying-points sport.

   A completed season is not this branch's case rather than a failure: every
   placement is final and the floor it protects can no longer be contradicted.
   shouldRerunSimulator now excludes it and it falls through to ICM as before.
   The genuine failure modes still leave probabilities alone rather than falling
   back to the path being replaced.

3. match-sync calls processMatchResult in a per-match loop with no
   skipSideEffects, so each synced match ran a full Monte Carlo plus EV rewrite,
   snapshot and standings recalc. processMatchResult gains skipProbabilities —
   mirroring the option processPlayoffEvent already takes, and narrower than
   skipSideEffects — which match-sync passes in the loop before refreshing once
   at the end. Per-match standings and Discord posts are unchanged.

4. A partially-seeded afl_10 bracket now throws from inside the result path.
   The throw is correct and stays; the concern was that it was silent, which the
   error surfacing in 2 covers.

Two claims from the review did not hold up and were left alone: the batch
bracket route already passes skipSideEffects per match and refreshes once after
the loop, and autoCompleteRoundIfDone already passes skipProbabilities, so there
is no double run per round completion.

Tests: the runner honors both skip flags and still writes EVs; the updater asks
for probabilities only; a completed season takes the ICM path without erroring;
processMatchResult skips the refresh but still announces. The two behavioral
ones were confirmed to fail against the previous behavior.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VPxDnSEKVoKx9HFcQ6gmcS

app/models/__tests__/process-match-result.test.ts

@@ -319,6 +319,22 @@ describe("processMatchResult", () => {
       expect(updateProbabilitiesAfterResult).toHaveBeenCalledWith("ss-1", true);
     });
 
+    it("skips only the probability refresh when asked, still announcing", async () => {
+      // For a caller scoring several matches in a loop: the refresh is season-wide and, for a
+      // bracket-aware sport, a full Monte Carlo run, so it belongs once after the loop rather
+      // than once per match. Standings and the announcement still happen per match.
+      const { db } = makeDb();
+
+      await processMatchResult(
+        { ...BASE, round: "Quarterfinals", isScoring: true, skipProbabilities: true },
+        db
+      );
+
+      expect(updateProbabilitiesAfterResult).not.toHaveBeenCalled();
+      // recalculateAffectedLeagues still ran: it is the only thing that reads seasonSports.
+      expect(db.query.seasonSports.findMany).toHaveBeenCalled();
+    });
+
     it("does not throw even if probability update fails", async () => {
       (updateProbabilitiesAfterResult as ReturnType<typeof vi.fn>).mockRejectedValueOnce(
         new Error("network error")

app/models/scoring-calculator.ts

@@ -547,6 +547,19 @@ export async function processMatchResult(
     /** When set, Discord notification only shows this match (not all completed matches for the event). */
     matchId?: string;
     skipSideEffects?: boolean;
+    /**
+     * Skip only the probability refresh, still recalculating standings and announcing.
+     *
+     * For a caller scoring several matches in a loop: the refresh is season-wide and
+     * idempotent, so running it per match repeats the whole thing needlessly — and for a
+     * bracket-aware sport that now means a full Monte Carlo run each time. Set this in the
+     * loop and call updateProbabilitiesAfterResult once when it finishes. Per-match
+     * announcements then project from the previous probabilities until that final call.
+     *
+     * Distinct from skipSideEffects, which also suppresses the standings recalculation and
+     * the announcement.
+     */
+    skipProbabilities?: boolean;
     /**
      * When true, the loser of this non-scoring round advances to another match
      * (e.g. NBA Play-In Round 1 7v8 loser → Play-In Round 2) and must NOT be
@@ -557,7 +570,7 @@ export async function processMatchResult(
   providedDb?: ReturnType<typeof database>
 ): Promise<void> {
   const db = providedDb || database();
-  const { round, winnerId, loserId, isScoring, sportsSeasonId, bracketTemplateId, eventId, eventName, matchId, skipSideEffects, loserAdvances } = params;
+  const { round, winnerId, loserId, isScoring, sportsSeasonId, bracketTemplateId, eventId, eventName, matchId, skipSideEffects, skipProbabilities, loserAdvances } = params;
 
   if (!isScoring) {
     // Non-scoring (pre-bracket) round: loser permanently eliminated (0 pts),
@@ -637,13 +650,15 @@ export async function processMatchResult(
       : undefined;
     // Update probabilities first so the standings recalc reads fresh EVs and
     // projected points reflect the new result.
-    try {
-      await updateProbabilitiesAfterResult(sportsSeasonId, true);
-    } catch (error) {
-      logger.error(
-        `[ScoringCalculator] Failed to auto-update probabilities for sports season ${sportsSeasonId}:`,
-        error
-      );
+    if (!skipProbabilities) {
+      try {
+        await updateProbabilitiesAfterResult(sportsSeasonId, true);
+      } catch (error) {
+        logger.error(
+          `[ScoringCalculator] Failed to auto-update probabilities for sports season ${sportsSeasonId}:`,
+          error
+        );
+      }
     }
     await recalculateAffectedLeagues(sportsSeasonId, db, sideEffectOptions);
   }

app/services/__tests__/probability-updater.test.ts

@@ -9,6 +9,7 @@ import * as participantEVModel from "~/models/participant-expected-value";
 vi.mock("~/models/participant-result");
 vi.mock("~/models/participant-expected-value");
 vi.mock("~/models/simulator");
+vi.mock("~/models/sports-season");
 vi.mock("~/services/simulations/runner");
 vi.mock("~/database/context", () => ({
   database: () => ({
@@ -373,10 +374,17 @@ describe("updateProbabilitiesAfterResult — simulator-backed seasons", () => {
     evSource: string;
     simulatorType: string | null;
     results?: ReturnType<typeof finishedResult>[];
+    seasonStatus?: string;
   }) {
     const simulatorModel = await import("~/models/simulator");
+    const sportsSeasonModel = await import("~/models/sports-season");
     const runner = await import("~/services/simulations/runner");
 
+    vi.mocked(sportsSeasonModel.findSportsSeasonById).mockResolvedValue({
+      id: "season-1",
+      status: opts.seasonStatus ?? "active",
+    } as never);
+
     vi.mocked(participantResultModel.findParticipantResultsBySportsSeasonId).mockResolvedValue(
       opts.results ?? []
     );
@@ -407,11 +415,43 @@ describe("updateProbabilitiesAfterResult — simulator-backed seasons", () => {
 
     const result = await updateProbabilitiesAfterResult("season-1", true);
 
-    expect(runner.runSportsSeasonSimulation).toHaveBeenCalledWith("season-1");
+    expect(runner.runSportsSeasonSimulation).toHaveBeenCalledWith("season-1", expect.anything());
     expect(icmWrites()).toHaveLength(0);
     expect(result.errors).toEqual([]);
   });
 
+  it("asks the run for probabilities only, leaving standings and snapshots to the caller", async () => {
+    // recalculateAffectedLeagues detects change by diffing teamStandings across its own
+    // recalculation, and that diff gates the Discord standings post. A recalculation in here
+    // runs before it takes its "before" snapshot, so the diff comes back empty and the post is
+    // silently dropped — and previousRank gets rolled forward twice, erasing rank movement.
+    const { runSim } = await setup({ evSource: "elo_simulation", simulatorType: "afl_bracket" });
+
+    await updateProbabilitiesAfterResult("season-1", true);
+
+    expect(runSim).toHaveBeenCalledWith("season-1", {
+      skipStandingsRecalc: true,
+      skipSnapshots: true,
+    });
+  });
+
+  it("falls through to ICM on a completed season rather than failing every time", async () => {
+    // finalizeQualifyingPoints marks the season completed immediately before calling here, and
+    // runSportsSeasonSimulation rejects a completed season outright. Treating that as a failure
+    // would strand anyone still unfinished on stale probabilities forever.
+    const { runner } = await setup({
+      evSource: "elo_simulation",
+      simulatorType: "cs2_major_qualifying_points",
+      seasonStatus: "completed",
+    });
+
+    const result = await updateProbabilitiesAfterResult("season-1", true);
+
+    expect(runner.runSportsSeasonSimulation).not.toHaveBeenCalled();
+    expect(icmWrites().length).toBeGreaterThan(0);
+    expect(result.errors).toEqual([]);
+  });
+
   it("still pins finished participants before re-running the simulator", async () => {
     const { runner } = await setup({
       evSource: "elo_simulation",
@@ -464,7 +504,7 @@ describe("updateProbabilitiesAfterResult — simulator-backed seasons", () => {
 
     await updateProbabilitiesAfterResult("season-1", true);
 
-    expect(runner.runSportsSeasonSimulation).toHaveBeenCalledWith("season-1");
+    expect(runner.runSportsSeasonSimulation).toHaveBeenCalledWith("season-1", expect.anything());
     expect(icmWrites()).toHaveLength(0);
   });
 

app/services/match-sync/index.ts

@@ -22,6 +22,7 @@ import {
   processQualifyingBracketEvent,
   recalculateAffectedLeagues,
 } from "~/models/scoring-calculator";
+import { updateProbabilitiesAfterResult } from "~/services/probability-updater";
 import { fanOutMajorIfPrimary } from "~/services/sync-tournament-results";
 import { notifyQualifyingPointsUpdate } from "~/services/qualifying-points-discord.server";
 import {
@@ -283,6 +284,11 @@ export async function syncMatches(sportsSeasonId: string): Promise<MatchSyncResu
             eventId: event.id,
             eventName: event.name ?? undefined,
             matchId: playoffMatch.id,
+            // The probability refresh is season-wide and idempotent, and for a bracket-aware
+            // sport it is a full Monte Carlo run — doing it per match would repeat that for
+            // every match in the sync. It runs once after the loop instead. Standings and the
+            // per-match Discord post still happen here as before.
+            skipProbabilities: true,
             loserAdvances: event.bracketTemplateId
               ? doesLoserAdvance(playoffMatch.round, playoffMatch.matchNumber, event.bracketTemplateId)
               : false,
@@ -300,6 +306,15 @@ export async function syncMatches(sportsSeasonId: string): Promise<MatchSyncResu
         }
       }
     }
+
+    // The refresh skipped inside the loop, run once for the whole sync.
+    if (playoffUpdated > 0) {
+      try {
+        await updateProbabilitiesAfterResult(sportsSeasonId, true);
+      } catch (err) {
+        logger.error(`[match-sync] Error updating probabilities after bracket sync:`, err);
+      }
+    }
   }
 
   return { swissCreated, swissUpdated, playoffUpdated, unmatchedTeams, errors };

app/services/probability-updater.ts

@@ -22,6 +22,7 @@ import * as schema from "~/database/schema";
 import { eq } from "drizzle-orm";
 import { DEFAULT_SCORING_RULES } from "~/lib/scoring-types";
 import { getSportsSeasonSimulatorConfig } from "~/models/simulator";
+import { findSportsSeasonById } from "~/models/sports-season";
 import { getManifestSimulatorProfile } from "~/services/simulations/manifest";
 import { logger } from "~/lib/logger";
 
@@ -119,6 +120,15 @@ function createFinishedProbabilities(finalPosition: number): number[] {
  * simulator would re-draw the field and hand equity back to teams already knocked out.
  */
 async function shouldRerunSimulator(sportsSeasonId: string): Promise<boolean> {
+  // A completed season cannot be simulated — runSportsSeasonSimulation rejects it outright —
+  // and finalizeQualifyingPoints marks the season completed immediately before calling here,
+  // so taking this branch there would fail every single time and leave anyone still in the
+  // unfinished set on permanently stale probabilities. It is not a failure, it is not this
+  // branch's case: the season is over, every placement is final, and the floor this branch
+  // exists to protect can no longer be contradicted. Fall through to ICM as before.
+  const sportsSeason = await findSportsSeasonById(sportsSeasonId);
+  if (sportsSeason?.status === "completed") return false;
+
   const simulatorConfig = await getSportsSeasonSimulatorConfig(sportsSeasonId);
   if (!simulatorConfig) return false;
 
@@ -185,13 +195,23 @@ export async function updateProbabilitiesAfterResult(
         // undefined at module-init time.
         try {
           const { runSportsSeasonSimulation } = await import("~/services/simulations/runner");
-          await runSportsSeasonSimulation(sportsSeasonId);
+          // Probabilities only. Our callers recalculate standings themselves right after this,
+          // and recalculateAffectedLeagues detects change by diffing teamStandings across its
+          // own recalculation — a recalculation slipped in here empties that diff and silently
+          // suppresses the Discord standings post, and rolls previousRank forward a second time
+          // so rank movement disappears. The daily EV snapshot is not ours to write either: it
+          // is keyed by date, so writing it per result overwrites the day with intra-day values.
+          await runSportsSeasonSimulation(sportsSeasonId, {
+            skipStandingsRecalc: true,
+            skipSnapshots: true,
+          });
           updated += unfinishedEVs.length;
         } catch (error) {
-          // runSportsSeasonSimulation throws on a completed season, on a run already in
-          // flight, and on failed readiness. Leave the existing probabilities alone rather
-          // than falling back to ICM: for these seasons ICM is the thing being replaced, and
-          // a completed season has nothing unfinished left to recalculate anyway.
+          // A run already in flight, failed readiness, or a bracket the simulator refuses to
+          // read (afl_10 seeded into only some of its slots). Leave the existing probabilities
+          // alone rather than falling back to ICM — for these seasons ICM is precisely the
+          // thing being replaced, and reintroducing it here would reintroduce sub-floor EVs.
+          // Completed seasons never reach this: shouldRerunSimulator excludes them.
           logger.error(
             `[ProbabilityUpdater] Failed to re-run simulator for sports season ${sportsSeasonId}; ` +
               `leaving existing probabilities in place:`,

app/services/simulations/__tests__/runner.test.ts

@@ -50,6 +50,8 @@ import {
 import { findParticipantsBySportsSeasonId } from "~/models/season-participant";
 import { batchUpsertParticipantEVs } from "~/models/participant-expected-value";
 import { batchUpsertParticipantEvSnapshots } from "~/models/ev-snapshot";
+import { recalculateStandings } from "~/models/scoring-calculator";
+import { database } from "~/database/context";
 import { getSimulator } from "~/services/simulations/registry";
 import { normalizeSimulationResultColumns } from "~/services/simulations/simulation-probabilities";
 
@@ -126,6 +128,42 @@ describe("runSportsSeasonSimulation", () => {
     expect(vi.mocked(updateSportsSeason).mock.calls[1]).toEqual(["season-1", { simulationStatus: "idle" }]);
   });
 
+  /** The default mock has no linked leagues, so nothing to recalculate. Give it one. */
+  function withLinkedLeague() {
+    vi.mocked(database).mockReturnValue({
+      query: {
+        seasonSports: { findMany: vi.fn().mockResolvedValue([{ seasonId: "fantasy-1" }]) },
+        seasons: { findFirst: vi.fn() },
+      },
+    } as never);
+  }
+
+  it("recalculates standings and writes the daily snapshot by default", async () => {
+    withLinkedLeague();
+
+    await runSportsSeasonSimulation("season-1");
+
+    expect(recalculateStandings).toHaveBeenCalledWith("fantasy-1");
+    expect(batchUpsertParticipantEvSnapshots).toHaveBeenCalled();
+  });
+
+  it("skips standings and snapshots when the caller owns them", async () => {
+    withLinkedLeague();
+
+    // updateProbabilitiesAfterResult runs inside the result path, where the caller
+    // recalculates standings straight afterwards. A recalculation here lands before
+    // recalculateAffectedLeagues takes its "before" snapshot, emptying the diff that gates the
+    // Discord standings post and rolling previousRank forward twice. EVs are still written.
+    await runSportsSeasonSimulation("season-1", {
+      skipStandingsRecalc: true,
+      skipSnapshots: true,
+    });
+
+    expect(recalculateStandings).not.toHaveBeenCalled();
+    expect(batchUpsertParticipantEvSnapshots).not.toHaveBeenCalled();
+    expect(batchUpsertParticipantEVs).toHaveBeenCalled();
+  });
+
   it("throws when the sports season is not found", async () => {
     vi.mocked(findSportsSeasonById).mockResolvedValue(undefined);
 

app/services/simulations/runner.ts

@@ -62,6 +62,29 @@ async function getPersistenceContext(
   };
 }
 
+/**
+ * Side effects a caller can opt out of.
+ *
+ * A simulation run does three jobs — recompute probabilities, recalculate standings, and record
+ * the day's EV snapshot. `updateProbabilitiesAfterResult` wants only the first: it runs inside
+ * the result path, where the caller recalculates standings itself immediately afterwards.
+ *
+ * Letting the run recalculate there is not merely redundant, it is wrong.
+ * recalculateAffectedLeagues detects change by snapshotting teamStandings, recalculating, then
+ * diffing, and that diff gates the Discord standings post; a recalculation slipped in
+ * beforehand makes the diff empty and silently suppresses the notification. recalculateStandings
+ * also rolls previousRank forward on every call, so an extra one erases rank movement.
+ */
+export interface RunSportsSeasonSimulationOptions {
+  /** Leave standings to the caller. */
+  skipStandingsRecalc?: boolean;
+  /**
+   * Skip the daily EV snapshot. The snapshot is a per-day series keyed by snapshotDate, so
+   * writing it on every match result just overwrites the day's row with intra-day values.
+   */
+  skipSnapshots?: boolean;
+}
+
 export interface RunSportsSeasonSimulationResult {
   sportsSeasonId: string;
   simulatorType: SimulatorType;
@@ -71,7 +94,8 @@ export interface RunSportsSeasonSimulationResult {
 }
 
 export async function runSportsSeasonSimulation(
-  sportsSeasonId: string
+  sportsSeasonId: string,
+  options: RunSportsSeasonSimulationOptions = {}
 ): Promise<RunSportsSeasonSimulationResult> {
   const sportsSeason = await findSportsSeasonById(sportsSeasonId);
   if (!sportsSeason) {
@@ -135,29 +159,33 @@ export async function runSportsSeasonSimulation(
       })),
     ]);
 
-    const seasonSports = await database().query.seasonSports.findMany({
-      where: eq(schema.seasonSports.sportsSeasonId, sportsSeasonId),
-    });
-    await Promise.all(seasonSports.map(({ seasonId }) => recalculateStandings(seasonId)));
+    if (!options.skipStandingsRecalc) {
+      const seasonSports = await database().query.seasonSports.findMany({
+        where: eq(schema.seasonSports.sportsSeasonId, sportsSeasonId),
+      });
+      await Promise.all(seasonSports.map(({ seasonId }) => recalculateStandings(seasonId)));
+    }
 
     const snapshotDate = new Date().toISOString().slice(0, 10);
-    await batchUpsertParticipantEvSnapshots(
-      results.map((r) => ({
-        participantId: r.participantId,
-        sportsSeasonId,
-        snapshotDate,
-        probFirst: r.probabilities.probFirst,
-        probSecond: r.probabilities.probSecond,
-        probThird: r.probabilities.probThird,
-        probFourth: r.probabilities.probFourth,
-        probFifth: r.probabilities.probFifth,
-        probSixth: r.probabilities.probSixth,
-        probSeventh: r.probabilities.probSeventh,
-        probEighth: r.probabilities.probEighth,
-        calculatedEV: calculateEV(r.probabilities, persistence.scoringRules),
-        source: r.source,
-      }))
-    );
+    if (!options.skipSnapshots) {
+      await batchUpsertParticipantEvSnapshots(
+        results.map((r) => ({
+          participantId: r.participantId,
+          sportsSeasonId,
+          snapshotDate,
+          probFirst: r.probabilities.probFirst,
+          probSecond: r.probabilities.probSecond,
+          probThird: r.probabilities.probThird,
+          probFourth: r.probabilities.probFourth,
+          probFifth: r.probabilities.probFifth,
+          probSixth: r.probabilities.probSixth,
+          probSeventh: r.probabilities.probSeventh,
+          probEighth: r.probabilities.probEighth,
+          calculatedEV: calculateEV(r.probabilities, persistence.scoringRules),
+          source: r.source,
+        }))
+      );
+    }
 
     await updateSportsSeason(sportsSeasonId, { simulationStatus: "idle" });