diff --git a/app/models/__tests__/process-match-result.test.ts b/app/models/__tests__/process-match-result.test.ts index 8967180..86b42d8 100644 --- a/app/models/__tests__/process-match-result.test.ts +++ b/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).mockRejectedValueOnce( new Error("network error") diff --git a/app/models/scoring-calculator.ts b/app/models/scoring-calculator.ts index 431b467..f0a3302 100644 --- a/app/models/scoring-calculator.ts +++ b/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 ): Promise { 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); } diff --git a/app/services/__tests__/probability-updater.test.ts b/app/services/__tests__/probability-updater.test.ts index e9639a5..c890ccf 100644 --- a/app/services/__tests__/probability-updater.test.ts +++ b/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[]; + 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); }); diff --git a/app/services/match-sync/index.ts b/app/services/match-sync/index.ts index 891739d..a958f60 100644 --- a/app/services/match-sync/index.ts +++ b/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 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 }; diff --git a/app/services/probability-updater.ts b/app/services/probability-updater.ts index a239e4d..b2caabb 100644 --- a/app/services/probability-updater.ts +++ b/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 { + // 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:`, diff --git a/app/services/simulations/__tests__/runner.test.ts b/app/services/simulations/__tests__/runner.test.ts index 0c4f9b4..9d50a7d 100644 --- a/app/services/simulations/__tests__/runner.test.ts +++ b/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); diff --git a/app/services/simulations/runner.ts b/app/services/simulations/runner.ts index 3c4fa45..cc41968 100644 --- a/app/services/simulations/runner.ts +++ b/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 { 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" });