From 7ec17e417db44c48bb841e5aed98e4cb2d4b9be9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 16:11:59 +0000 Subject: [PATCH 1/2] Fix EV reporting 20 pts for both LLWS 5-6 and 7-8 locked tiers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After an LLWS simulation, a team locked into the 5th-6th tier and one locked into the 7th-8th tier both showed 20 points EV. They should show 25 and 15. The simulator and calculateEV were both right. A team locked into the 5-6 tier comes out of llws-simulator at probFifth = probSixth = 0.5, and against DEFAULT_SCORING_RULES (100/70/50/40/25/25/15/15) that is 25 — matching calculateBracketPoints, which already knows llws_20 splits 5-8 into two tiers. The Admin -> Expected Values page just wasn't using that table. It hardcoded its own stale copy: const SCORING = [100, 70, 45, 45, 20, 20, 20, 20] as const; 0.5*20 + 0.5*20 = 20 for either tier. It is not LLWS-specific. Four places carried that same stale table, and it stayed invisible because a standard single-elimination bracket puts all four quarterfinal losers in one tier worth avg(25,25,15,15) = 20 — the same number. It only diverges for the templates that split 5-8 (llws_20, afl_10) and those with a distinct 3rd/4th (llws_20, fifa_48, where 45/45 should be 50/40). Two of the four *persist* EVs computed that way, so the wrong values reached the database: - expected-values.tsx displayed EV, the total, and the sort order - expected-values.server manual EV entry, written to expected_value - golf-skills.tsx simulation EVs + snapshots, written - surface-elo.tsx simulation EVs + snapshots, written All four now use the shared DEFAULT_SCORING_RULES. probability-updater had a fourth inline copy with the right values; it is folded in too so there is one table left. The page's 340 total-EV invariant is unchanged — both tables sum to 340. A second path collapses the same two tiers, this time in real fantasy points. calculateBracketPoints falls back to the flat avg([5,6,7,8]) when bracketTemplateId is null, and four call sites resolved the template by taking an arbitrary scoringEvents row for the sports season — unordered, and not filtered to rows that actually carry a template. A season can own several events (a bracket plus schedule events, or a re-created bracket beside a stale one), so a null row wins at random and llws_20 is lost. New getBracketTemplateIdsForSportsSeasons in models/bracket-template.ts filters to events with a template and takes the most recent, the same rule llws-simulator uses to pick its bracket event; standings, calculateTeamScore, calculateTeamProjectedScore and getDraftedParticipantsWithPoints all go through it. Tests: evFromProbs pinned to 25 / 15 / 20-for-a-single-5-8-tier and the 340 invariant; the new lookup against a mixed set of events; and two llws-simulator tests that play out a full U.S. side so a team really is locked into each tier and must come out at exactly 50/50 across it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EmUdy42Rpgx9qZTnpQwirz --- .../__tests__/bracket-template-lookup.test.ts | 92 +++++++++++++++++++ .../__tests__/team-projected-score.test.ts | 3 + app/models/bracket-template.ts | 66 +++++++++++++ app/models/draft-pick.ts | 17 +--- app/models/scoring-calculator.ts | 13 +-- app/models/standings.ts | 7 +- ...sports-seasons.$id.expected-values.test.ts | 64 +++++++++++++ ...orts-seasons.$id.expected-values.server.ts | 14 +-- ...min.sports-seasons.$id.expected-values.tsx | 42 ++++++--- .../admin.sports-seasons.$id.golf-skills.tsx | 14 +-- .../admin.sports-seasons.$id.surface-elo.tsx | 14 +-- app/services/probability-updater.ts | 20 ++-- .../__tests__/llws-simulator.test.ts | 69 ++++++++++++++ 13 files changed, 344 insertions(+), 91 deletions(-) create mode 100644 app/models/__tests__/bracket-template-lookup.test.ts create mode 100644 app/models/bracket-template.ts create mode 100644 app/routes/__tests__/admin.sports-seasons.$id.expected-values.test.ts diff --git a/app/models/__tests__/bracket-template-lookup.test.ts b/app/models/__tests__/bracket-template-lookup.test.ts new file mode 100644 index 0000000..5f7a5ed --- /dev/null +++ b/app/models/__tests__/bracket-template-lookup.test.ts @@ -0,0 +1,92 @@ +import { describe, expect, it, vi } from "vitest"; + +import { + getBracketTemplateIdForSportsSeason, + getBracketTemplateIdsForSportsSeasons, +} from "../bracket-template"; + +/** + * A sports season can own several scoring events — a bracket plus schedule events, or a + * re-created bracket alongside a stale one. Resolving the template from an arbitrary row + * is not harmless: calculateBracketPoints falls back to the flat 5th–8th average when the + * template id is null, so losing "llws_20" makes a team locked into 5th–6th and one + * locked into 7th–8th both score 20. + */ + +/** + * Minimal db stub. Applies the same filter and ordering the real query does, so the + * assertions exercise the helper's row-picking rather than re-stating the query. + */ +function makeDb( + rows: Array<{ sportsSeasonId: string; bracketTemplateId: string | null; createdAt: Date }> +) { + const findMany = vi.fn(async () => + rows + .filter((row) => row.bracketTemplateId !== null) + .toSorted((a, b) => b.createdAt.getTime() - a.createdAt.getTime()) + ); + return { db: { query: { scoringEvents: { findMany } } } as any, findMany }; +} + +describe("getBracketTemplateIdForSportsSeason", () => { + it("ignores a non-bracket event and returns the bracket event's template", async () => { + const { db } = makeDb([ + { sportsSeasonId: "ss1", bracketTemplateId: null, createdAt: new Date("2026-08-01") }, + { sportsSeasonId: "ss1", bracketTemplateId: "llws_20", createdAt: new Date("2026-07-01") }, + ]); + + await expect(getBracketTemplateIdForSportsSeason("ss1", db)).resolves.toBe("llws_20"); + }); + + it("takes the most recent bracket event when a stale one is still around", async () => { + const { db } = makeDb([ + { sportsSeasonId: "ss1", bracketTemplateId: "simple_16", createdAt: new Date("2026-06-01") }, + { sportsSeasonId: "ss1", bracketTemplateId: "llws_20", createdAt: new Date("2026-08-01") }, + ]); + + await expect(getBracketTemplateIdForSportsSeason("ss1", db)).resolves.toBe("llws_20"); + }); + + it("returns null when the season has no bracket event", async () => { + const { db } = makeDb([ + { sportsSeasonId: "ss1", bracketTemplateId: null, createdAt: new Date("2026-08-01") }, + ]); + + await expect(getBracketTemplateIdForSportsSeason("ss1", db)).resolves.toBeNull(); + }); +}); + +describe("getBracketTemplateIdsForSportsSeasons", () => { + it("resolves each season independently in one query", async () => { + const { db, findMany } = makeDb([ + { sportsSeasonId: "ss1", bracketTemplateId: "llws_20", createdAt: new Date("2026-08-01") }, + { sportsSeasonId: "ss2", bracketTemplateId: "afl_10", createdAt: new Date("2026-08-02") }, + { sportsSeasonId: "ss3", bracketTemplateId: null, createdAt: new Date("2026-08-03") }, + ]); + + const resolved = await getBracketTemplateIdsForSportsSeasons(["ss1", "ss2", "ss3"], db); + + expect(resolved.get("ss1")).toBe("llws_20"); + expect(resolved.get("ss2")).toBe("afl_10"); + expect(resolved.get("ss3")).toBeNull(); + expect(findMany).toHaveBeenCalledTimes(1); + }); + + it("gives every requested season an entry so callers can cache the miss", async () => { + const { db } = makeDb([]); + + const resolved = await getBracketTemplateIdsForSportsSeasons(["ss1", "ss2"], db); + + expect([...resolved.entries()]).toEqual([ + ["ss1", null], + ["ss2", null], + ]); + }); + + it("does not query at all for an empty season list", async () => { + const { db, findMany } = makeDb([]); + + await expect(getBracketTemplateIdsForSportsSeasons([], db)).resolves.toEqual(new Map()); + expect(findMany).not.toHaveBeenCalled(); + }); +}); diff --git a/app/models/__tests__/team-projected-score.test.ts b/app/models/__tests__/team-projected-score.test.ts index bbce292..172230e 100644 --- a/app/models/__tests__/team-projected-score.test.ts +++ b/app/models/__tests__/team-projected-score.test.ts @@ -80,6 +80,9 @@ function makeDb( }, scoringEvents: { findFirst: vi.fn().mockResolvedValue({ bracketTemplateId: null }), + // getBracketTemplateIdsForSportsSeasons filters to events that carry a + // template, so "no bracket template" is an empty result, not a null row. + findMany: vi.fn().mockResolvedValue([]), }, seasonParticipantResults: { findMany: vi.fn().mockResolvedValue(seasonResults), diff --git a/app/models/bracket-template.ts b/app/models/bracket-template.ts new file mode 100644 index 0000000..50c65a3 --- /dev/null +++ b/app/models/bracket-template.ts @@ -0,0 +1,66 @@ +import { database } from "~/database/context"; +import * as schema from "~/database/schema"; +import { and, desc, inArray, isNotNull } from "drizzle-orm"; + +/** + * Resolve which bracket template a sports season's placements should be scored against. + * + * A sports season can own several scoring events — a bracket plus schedule events, or a + * re-created bracket alongside a stale one — and only some of them carry a + * bracketTemplateId. Picking an arbitrary row is not harmless: calculateBracketPoints + * falls back to the standard single 5th–8th tier when the template id is null, which + * silently collapses the two-tier templates (llws_20, afl_10) so a team locked into + * 5th–6th and one locked into 7th–8th both score the flat 5–8 average. The 3rd/4th + * distinction that llws_20 and fifa_48 have goes the same way. + * + * So: only events that actually carry a template are considered, most recent first — + * matching the "a re-created event wins over a stale one" rule the LLWS simulator uses + * when it picks its bracket event. + * + * Every requested season gets an entry, null when it has no bracket event, so callers + * can cache the negative result too. + */ +export async function getBracketTemplateIdsForSportsSeasons( + sportsSeasonIds: string[], + providedDb?: ReturnType +): Promise> { + const resolved = new Map( + sportsSeasonIds.map((id) => [id, null]) + ); + if (sportsSeasonIds.length === 0) return resolved; + + const db = providedDb || database(); + + const events = await db.query.scoringEvents.findMany({ + where: and( + inArray(schema.scoringEvents.sportsSeasonId, sportsSeasonIds), + isNotNull(schema.scoringEvents.bracketTemplateId) + ), + columns: { sportsSeasonId: true, bracketTemplateId: true }, + // createdAt can tie when a bracket is generated in the same transaction as a + // sibling event, so id breaks the tie and keeps the choice deterministic. + orderBy: [desc(schema.scoringEvents.createdAt), desc(schema.scoringEvents.id)], + }); + + for (const event of events) { + // Ordered newest-first, so the first row seen for a season is the one to keep. + // The isNotNull filter means bracketTemplateId is set, but a mocked or partial row + // could still carry null — skip those rather than caching a null as a real answer. + if (resolved.get(event.sportsSeasonId) === null && event.bracketTemplateId) { + resolved.set(event.sportsSeasonId, event.bracketTemplateId); + } + } + + return resolved; +} + +/** + * Single-season form of getBracketTemplateIdsForSportsSeasons. + */ +export async function getBracketTemplateIdForSportsSeason( + sportsSeasonId: string, + providedDb?: ReturnType +): Promise { + const resolved = await getBracketTemplateIdsForSportsSeasons([sportsSeasonId], providedDb); + return resolved.get(sportsSeasonId) ?? null; +} diff --git a/app/models/draft-pick.ts b/app/models/draft-pick.ts index 7289046..2ca8b6d 100644 --- a/app/models/draft-pick.ts +++ b/app/models/draft-pick.ts @@ -7,6 +7,7 @@ import { calculateBracketPoints, calculateSharedPlacementPoints, } from "./scoring-rules"; +import { getBracketTemplateIdsForSportsSeasons } from "./bracket-template"; export async function createDraftPick(data: { seasonId: string; @@ -175,18 +176,10 @@ export async function getDraftedParticipantsWithPoints( } // Batch-fetch bracket template IDs (one per sports season) - const bracketTemplateMap = new Map(); - if (bracketSeasonIds.size > 0) { - const events = await db.query.scoringEvents.findMany({ - where: inArray(schema.scoringEvents.sportsSeasonId, [...bracketSeasonIds]), - columns: { sportsSeasonId: true, bracketTemplateId: true }, - }); - for (const ev of events) { - if (!bracketTemplateMap.has(ev.sportsSeasonId)) { - bracketTemplateMap.set(ev.sportsSeasonId, ev.bracketTemplateId ?? null); - } - } - } + const bracketTemplateMap = + bracketSeasonIds.size > 0 + ? await getBracketTemplateIdsForSportsSeasons([...bracketSeasonIds], db) + : new Map(); // Batch-fetch QP totals for qualifying_points participants const qpMap = new Map(); // participantId → totalQP diff --git a/app/models/scoring-calculator.ts b/app/models/scoring-calculator.ts index 2f4fbd4..431b467 100644 --- a/app/models/scoring-calculator.ts +++ b/app/models/scoring-calculator.ts @@ -16,6 +16,7 @@ import { doesLoserAdvance, findPlayoffMatchesByEventId } from "~/models/playoff- import { getUserDisplayName } from "~/models/user"; import { findDiscordIdsByUserIds } from "~/models/account"; import { createDailySnapshot } from "~/models/standings"; +import { getBracketTemplateIdForSportsSeason } from "~/models/bracket-template"; import { recordMatchScoreEvents } from "~/models/team-score-events"; import { logger } from "~/lib/logger"; import { getEventResults } from "./event-result"; @@ -1465,11 +1466,7 @@ export async function calculateTeamScore( if (bracketTemplateCache.has(sportsSeasonId)) { return bracketTemplateCache.get(sportsSeasonId) ?? null; } - const event = await db.query.scoringEvents.findFirst({ - where: eq(schema.scoringEvents.sportsSeasonId, sportsSeasonId), - columns: { bracketTemplateId: true }, - }); - const templateId = event?.bracketTemplateId ?? null; + const templateId = await getBracketTemplateIdForSportsSeason(sportsSeasonId, db); bracketTemplateCache.set(sportsSeasonId, templateId); return templateId; } @@ -1578,11 +1575,7 @@ export async function calculateTeamProjectedScore( if (bracketTemplateCache.has(sportsSeasonId)) { return bracketTemplateCache.get(sportsSeasonId) ?? null; } - const event = await db.query.scoringEvents.findFirst({ - where: eq(schema.scoringEvents.sportsSeasonId, sportsSeasonId), - columns: { bracketTemplateId: true }, - }); - const templateId = event?.bracketTemplateId ?? null; + const templateId = await getBracketTemplateIdForSportsSeason(sportsSeasonId, db); bracketTemplateCache.set(sportsSeasonId, templateId); return templateId; } diff --git a/app/models/standings.ts b/app/models/standings.ts index 97acc5f..731377e 100644 --- a/app/models/standings.ts +++ b/app/models/standings.ts @@ -5,6 +5,7 @@ import type { TeamStanding, TeamStandingSnapshot, TeamStandingWithChange } from import { calculateBracketPoints, calculateFantasyPoints } from "~/models/scoring-rules"; import { logger } from "~/lib/logger"; import { getParticipantEV } from "./participant-expected-value"; +import { getBracketTemplateIdForSportsSeason } from "./bracket-template"; import { calculateEV } from "~/services/ev-calculator"; // Re-export types from shared types file @@ -163,11 +164,7 @@ export async function getTeamScoreBreakdown( if (bracketTemplateCache.has(sportsSeasonId)) { return bracketTemplateCache.get(sportsSeasonId) ?? null; } - const event = await db.query.scoringEvents.findFirst({ - where: eq(schema.scoringEvents.sportsSeasonId, sportsSeasonId), - columns: { bracketTemplateId: true }, - }); - const templateId = event?.bracketTemplateId ?? null; + const templateId = await getBracketTemplateIdForSportsSeason(sportsSeasonId, db); bracketTemplateCache.set(sportsSeasonId, templateId); return templateId; } diff --git a/app/routes/__tests__/admin.sports-seasons.$id.expected-values.test.ts b/app/routes/__tests__/admin.sports-seasons.$id.expected-values.test.ts new file mode 100644 index 0000000..a60e957 --- /dev/null +++ b/app/routes/__tests__/admin.sports-seasons.$id.expected-values.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it, vi } from "vitest"; + +/** + * The Expected Values admin page renders EV from the stored probability columns. + * + * It used to carry its own hardcoded scoring table (100/70/45/45/20/20/20/20), which + * flattened positions 5–8 to 20 points each. For a standard single-elimination bracket + * that was invisible — all four quarterfinal losers share one tier worth + * avg(25,25,15,15) = 20 anyway — but for the templates that split 5–8 into two tiers + * (llws_20, afl_10) it reported a team locked into 5th–6th and a team locked into + * 7th–8th as the same 20 points. These pin it to the shared DEFAULT_SCORING_RULES. + */ + +vi.mock("../admin.sports-seasons.$id.expected-values.server", () => ({ + loader: vi.fn(), +})); + +import { evFromProbs } from "../admin.sports-seasons.$id.expected-values"; + +const ZERO = { + probFirst: "0", probSecond: "0", probThird: "0", probFourth: "0", + probFifth: "0", probSixth: "0", probSeventh: "0", probEighth: "0", +}; + +describe("evFromProbs", () => { + it("gives a team locked into the 5th–6th tier 25 points, not 20", () => { + expect(evFromProbs({ ...ZERO, probFifth: "0.5", probSixth: "0.5" })).toBe(25); + }); + + it("gives a team locked into the 7th–8th tier 15 points, not 20", () => { + expect(evFromProbs({ ...ZERO, probSeventh: "0.5", probEighth: "0.5" })).toBe(15); + }); + + it("still gives a single 5th–8th tier (4 QF losers) 20 points", () => { + const ev = evFromProbs({ + ...ZERO, + probFifth: "0.25", probSixth: "0.25", probSeventh: "0.25", probEighth: "0.25", + }); + expect(ev).toBe(20); + }); + + it("keeps 3rd and 4th distinct rather than a flat 45 each", () => { + expect(evFromProbs({ ...ZERO, probThird: "1" })).toBe(50); + expect(evFromProbs({ ...ZERO, probFourth: "1" })).toBe(40); + }); + + it("preserves the 340 total-EV invariant across a full set of unit columns", () => { + const perPosition = [ + evFromProbs({ ...ZERO, probFirst: "1" }), + evFromProbs({ ...ZERO, probSecond: "1" }), + evFromProbs({ ...ZERO, probThird: "1" }), + evFromProbs({ ...ZERO, probFourth: "1" }), + evFromProbs({ ...ZERO, probFifth: "1" }), + evFromProbs({ ...ZERO, probSixth: "1" }), + evFromProbs({ ...ZERO, probSeventh: "1" }), + evFromProbs({ ...ZERO, probEighth: "1" }), + ]; + expect(perPosition.reduce((sum, ev) => sum + ev, 0)).toBe(340); + }); + + it("returns 0 for a participant with no probability mass", () => { + expect(evFromProbs(ZERO)).toBe(0); + }); +}); diff --git a/app/routes/admin.sports-seasons.$id.expected-values.server.ts b/app/routes/admin.sports-seasons.$id.expected-values.server.ts index 70104e0..29d8d49 100644 --- a/app/routes/admin.sports-seasons.$id.expected-values.server.ts +++ b/app/routes/admin.sports-seasons.$id.expected-values.server.ts @@ -6,6 +6,7 @@ import { batchUpsertParticipantEVs, getAllParticipantEVsForSeason } from "~/models/participant-expected-value"; +import { DEFAULT_SCORING_RULES } from "~/lib/scoring-types"; export async function loader({ params }: Route.LoaderArgs) { const sportsSeason = await findSportsSeasonById(params.id); @@ -27,17 +28,6 @@ export async function loader({ params }: Route.LoaderArgs) { }; } -const scoringRules = { - pointsFor1st: 100, - pointsFor2nd: 70, - pointsFor3rd: 45, - pointsFor4th: 45, - pointsFor5th: 20, - pointsFor6th: 20, - pointsFor7th: 20, - pointsFor8th: 20, -}; - export async function action({ request, params }: Route.ActionArgs) { const formData = await request.formData(); @@ -58,7 +48,7 @@ export async function action({ request, params }: Route.ActionArgs) { probSeventh: parseFloat(formData.get(`probSeventh_${participantId}`) as string || "0") / 100, probEighth: parseFloat(formData.get(`probEighth_${participantId}`) as string || "0") / 100, }, - scoringRules, + scoringRules: DEFAULT_SCORING_RULES, source: "manual" as const, })); diff --git a/app/routes/admin.sports-seasons.$id.expected-values.tsx b/app/routes/admin.sports-seasons.$id.expected-values.tsx index 1536f68..a8c7b6f 100644 --- a/app/routes/admin.sports-seasons.$id.expected-values.tsx +++ b/app/routes/admin.sports-seasons.$id.expected-values.tsx @@ -19,6 +19,8 @@ import { TableRow, } from "~/components/ui/table"; import { ArrowLeft, Calculator } from "lucide-react"; +import { DEFAULT_SCORING_RULES } from "~/lib/scoring-types"; +import { calculateEV } from "~/services/ev-calculator"; export function meta({ data }: Route.MetaArgs): Route.MetaDescriptors { return [{ title: `Expected Values — ${data?.sportsSeason?.name ?? "Sports Season"} - Brackt Admin` }]; @@ -26,9 +28,18 @@ export function meta({ data }: Route.MetaArgs): Route.MetaDescriptors { export { loader }; -// DEFAULT scoring values — must match DEFAULT_SCORING_RULES in the simulate route. -// Scoring: 1st=100, 2nd=70, 3rd/4th (FF losers)=45 each, 5th–8th (E8 losers)=20 each. -// Sum = 100+70+45+45+20+20+20+20 = 340. +// EV is shown on the same reference scale the runner persists it with: a sports season +// is shared across leagues with different scoring, so DEFAULT_SCORING_RULES is the +// common scale and each league re-derives its own EV from the stored probabilities +// (see getPersistenceContext in services/simulations/runner.ts). +// +// Scoring: 1st=100, 2nd=70, 3rd=50, 4th=40, 5th/6th=25 each, 7th/8th=15 each. +// Sum = 100+70+50+40+25+25+15+15 = 340. +// +// The 5th–8th values must stay distinct rather than collapsing to a flat 20: templates +// that split that zone into two tiers (llws_20, afl_10) put a team locked into 5th–6th +// at probFifth=probSixth=0.5 (EV 25) and one locked into 7th–8th at +// probSeventh=probEighth=0.5 (EV 15). A flat table reports both as 20. // // Total EV invariant: Σ EV across all participants = Σ scoring values = 340, // because each probability column sums to 1.0 across all participants. @@ -36,20 +47,23 @@ export { loader }; // 1. Stale EV records from a prior simulation run (fix: re-run simulation, which now // zeros non-bracket participants automatically) // 2. DB precision truncation (numeric(6,4) = 4dp; max drift ≈ ±1 for 68 teams) -const SCORING = [100, 70, 45, 45, 20, 20, 20, 20] as const; - -function evFromProbs(ev: { +export function evFromProbs(ev: { probFirst: string; probSecond: string; probThird: string; probFourth: string; probFifth: string; probSixth: string; probSeventh: string; probEighth: string; }): number { - return parseFloat(ev.probFirst) * SCORING[0] - + parseFloat(ev.probSecond) * SCORING[1] - + parseFloat(ev.probThird) * SCORING[2] - + parseFloat(ev.probFourth) * SCORING[3] - + parseFloat(ev.probFifth) * SCORING[4] - + parseFloat(ev.probSixth) * SCORING[5] - + parseFloat(ev.probSeventh) * SCORING[6] - + parseFloat(ev.probEighth) * SCORING[7]; + return calculateEV( + { + probFirst: parseFloat(ev.probFirst), + probSecond: parseFloat(ev.probSecond), + probThird: parseFloat(ev.probThird), + probFourth: parseFloat(ev.probFourth), + probFifth: parseFloat(ev.probFifth), + probSixth: parseFloat(ev.probSixth), + probSeventh: parseFloat(ev.probSeventh), + probEighth: parseFloat(ev.probEighth), + }, + DEFAULT_SCORING_RULES + ); } function fmt(val: string | number) { diff --git a/app/routes/admin.sports-seasons.$id.golf-skills.tsx b/app/routes/admin.sports-seasons.$id.golf-skills.tsx index ac73c23..79ecb1a 100644 --- a/app/routes/admin.sports-seasons.$id.golf-skills.tsx +++ b/app/routes/admin.sports-seasons.$id.golf-skills.tsx @@ -8,7 +8,8 @@ import { batchUpsertParticipantEVs } from '~/models/participant-expected-value'; import { batchUpsertParticipantEvSnapshots } from '~/models/ev-snapshot'; import { getGolfSkillsForSeason, batchUpsertGolfSkills } from '~/models/golf-skills'; import { getSimulator, type SimulatorType } from '~/services/simulations/registry'; -import { calculateEV, type ScoringRules } from '~/services/ev-calculator'; +import { calculateEV } from '~/services/ev-calculator'; +import { DEFAULT_SCORING_RULES } from '~/lib/scoring-types'; import { recalculateStandings } from '~/models/scoring-calculator'; import { database } from '~/database/context'; import * as schema from '~/database/schema'; @@ -28,17 +29,6 @@ import { useEffect, useRef, useState } from 'react'; import { Loader2, CheckCircle2, AlertCircle, UserPlus } from 'lucide-react'; import { normalizeName, diceCoefficient } from '~/lib/fuzzy-match'; -const DEFAULT_SCORING_RULES: ScoringRules = { - pointsFor1st: 100, - pointsFor2nd: 70, - pointsFor3rd: 45, - pointsFor4th: 45, - pointsFor5th: 20, - pointsFor6th: 20, - pointsFor7th: 20, - pointsFor8th: 20, -}; - export function meta({ data }: Route.MetaArgs): Route.MetaDescriptors { return [{ title: `Golf Skills — ${data?.sportsSeason?.name ?? 'Sports Season'} - Brackt Admin` }]; } diff --git a/app/routes/admin.sports-seasons.$id.surface-elo.tsx b/app/routes/admin.sports-seasons.$id.surface-elo.tsx index 95b548d..cdfd38e 100644 --- a/app/routes/admin.sports-seasons.$id.surface-elo.tsx +++ b/app/routes/admin.sports-seasons.$id.surface-elo.tsx @@ -10,7 +10,8 @@ import { import { batchUpsertParticipantEvSnapshots } from '~/models/ev-snapshot'; import { getSurfaceElosForSeason, batchUpsertSurfaceElos } from '~/models/surface-elo'; import { getSimulator, type SimulatorType } from '~/services/simulations/registry'; -import { calculateEV, type ScoringRules } from '~/services/ev-calculator'; +import { calculateEV } from '~/services/ev-calculator'; +import { DEFAULT_SCORING_RULES } from '~/lib/scoring-types'; import { recalculateStandings } from '~/models/scoring-calculator'; import { database } from '~/database/context'; import * as schema from '~/database/schema'; @@ -30,17 +31,6 @@ import { useEffect, useRef, useState } from 'react'; import { Loader2, CheckCircle2, AlertCircle, UserPlus } from 'lucide-react'; import { normalizeName, diceCoefficient } from '~/lib/fuzzy-match'; -const DEFAULT_SCORING_RULES: ScoringRules = { - pointsFor1st: 100, - pointsFor2nd: 70, - pointsFor3rd: 45, - pointsFor4th: 45, - pointsFor5th: 20, - pointsFor6th: 20, - pointsFor7th: 20, - pointsFor8th: 20, -}; - export function meta({ data }: Route.MetaArgs): Route.MetaDescriptors { return [{ title: `Surface Elo — ${data?.sportsSeason?.name ?? "Sports Season"} - Brackt Admin` }]; } diff --git a/app/services/probability-updater.ts b/app/services/probability-updater.ts index d7e4081..51cf8d5 100644 --- a/app/services/probability-updater.ts +++ b/app/services/probability-updater.ts @@ -20,6 +20,7 @@ import type { ProbabilityDistribution } from "./ev-calculator"; import { database } from "~/database/context"; import * as schema from "~/database/schema"; import { eq } from "drizzle-orm"; +import { DEFAULT_SCORING_RULES } from "~/lib/scoring-types"; /** * Result of probability update operation @@ -137,18 +138,9 @@ export async function updateProbabilitiesAfterResult( .map(r => [r.participantId, r.finalPosition ?? 0]) ); - // Update finished participants - // Use default scoring rules (we only care about setting probabilities, not EV for finished) - const defaultScoringRules = { - pointsFor1st: 100, - pointsFor2nd: 70, - pointsFor3rd: 50, - pointsFor4th: 40, - pointsFor5th: 25, - pointsFor6th: 25, - pointsFor7th: 15, - pointsFor8th: 15, - }; + // Update finished participants. The shared default table is used because we only + // care about setting probabilities here, not the EV — each league re-derives its own + // EV from the stored probabilities in calculateTeamProjectedScore. // Sequential: upsertParticipantEV calls syncVorpForSeason internally, which // reads and rewrites every seasonParticipants row for this sportsSeasonId. @@ -162,7 +154,7 @@ export async function updateProbabilitiesAfterResult( participantId, sportsSeasonId, probabilities, - scoringRules: defaultScoringRules, + scoringRules: DEFAULT_SCORING_RULES, source: 'manual', // Result is from actual outcome }); @@ -216,7 +208,7 @@ export async function updateProbabilitiesAfterResult( participantId, sportsSeasonId, probabilities, - scoringRules: defaultScoringRules, + scoringRules: DEFAULT_SCORING_RULES, source: 'futures_odds', // Recalculated from remaining odds }); diff --git a/app/services/simulations/__tests__/llws-simulator.test.ts b/app/services/simulations/__tests__/llws-simulator.test.ts index 6684652..9533600 100644 --- a/app/services/simulations/__tests__/llws-simulator.test.ts +++ b/app/services/simulations/__tests__/llws-simulator.test.ts @@ -767,6 +767,75 @@ describe("LLWSSimulator", () => { expect(results.reduce((s, r) => s + r.probabilities.probFirst, 0)).toBeCloseTo(1.0, 1); }); + /** + * Play out the entire U.S. side, so two of its teams are locked into a scoring tier: + * us-7 loses Elimination Round 4 (the 7th-8th tier) and us-9 loses the Elimination + * Final (the 5th-6th tier). Every game feeding those two is recorded, which is what + * makes the results honorable — makePlayGame only replays a result when the teams + * the simulation routed into the game are the pair the result was recorded between. + * + * Slot order per side is ids[0..7] into the four Opening Round games and ids[8..9] + * as the byes, so the U.S. draw is us-1 v us-2, us-3 v us-4, us-5 v us-6, + * us-7 v us-8, with us-9 and us-10 entering at Winners Round 2. + */ + function usSidePlayedOut(): PlayoffMatchRow[] { + let matches = seededBracket(); + const play = (round: string, matchNumber: number, winnerId: string, loserId: string) => { + matches = completeMatch(matches, round, matchNumber, winnerId, loserId); + }; + + // Winners bracket + play("Opening Round", 1, "us-1", "us-2"); + play("Opening Round", 2, "us-3", "us-4"); + play("Opening Round", 3, "us-5", "us-6"); + play("Opening Round", 4, "us-7", "us-8"); + play("Winners Round 2", 1, "us-9", "us-1"); // bye us-9 v OP1 winner + play("Winners Round 2", 2, "us-10", "us-3"); // bye us-10 v OP2 winner + play("Winners Semifinals", 1, "us-5", "us-9"); + play("Winners Semifinals", 2, "us-10", "us-7"); + play("Winners Final", 1, "us-5", "us-10"); + + // Elimination bracket, including the deliberate cross-overs + play("Elimination Round 1", 1, "us-4", "us-6"); // OP2 loser v OP3 loser + play("Elimination Round 1", 2, "us-2", "us-8"); // OP1 loser v OP4 loser + play("Elimination Round 2", 1, "us-1", "us-4"); + play("Elimination Round 2", 2, "us-3", "us-2"); + play("Elimination Round 3", 1, "us-9", "us-3"); + play("Elimination Round 3", 2, "us-7", "us-1"); + play("Elimination Round 4", 1, "us-9", "us-7"); // us-7 out: 7th-8th tier + play("Elimination Final", 1, "us-10", "us-9"); // us-9 out: 5th-6th tier + + return matches; + } + + it("puts a team locked into the 5th-6th tier at exactly 50/50 across those two spots", async () => { + setupMockDb(defaultParticipants(), favouredEvRows, usSidePlayedOut()); + const results = await new LLWSSimulator(2_000).simulate("season-1"); + const locked = probsFor(results, "us-9"); + + // The tier is two tied positions, so its probability splits evenly across them. + // Under DEFAULT_SCORING_RULES that is 0.5 x 25 + 0.5 x 25 = 25 points of EV — + // the 5th-6th tier value, not the flat 5th-8th average of 20. + expect(locked.probFifth).toBe(0.5); + expect(locked.probSixth).toBe(0.5); + expect(locked.probSeventh).toBe(0); + expect(locked.probEighth).toBe(0); + expect(locked.probFirst + locked.probSecond + locked.probThird + locked.probFourth).toBe(0); + }); + + it("puts a team locked into the 7th-8th tier at exactly 50/50 across those two spots", async () => { + setupMockDb(defaultParticipants(), favouredEvRows, usSidePlayedOut()); + const results = await new LLWSSimulator(2_000).simulate("season-1"); + const locked = probsFor(results, "us-7"); + + // 0.5 x 15 + 0.5 x 15 = 15 points of EV, again distinct from the flat 20. + expect(locked.probSeventh).toBe(0.5); + expect(locked.probEighth).toBe(0.5); + expect(locked.probFifth).toBe(0); + expect(locked.probSixth).toBe(0); + expect(locked.probFirst + locked.probSecond + locked.probThird + locked.probFourth).toBe(0); + }); + }); // ── Result-honoring rules ───────────────────────────────────────────────── -- 2.45.3 From 1e215c3ac9932ab2d409661143fbeda842b67e29 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 17:26:23 +0000 Subject: [PATCH 2/2] Fix three defects found reviewing the bracket entry-floor work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three predate the EV fix on this branch and were surfaced by a review of the full main..HEAD range. 1. reprocess-bracket skipped its wipe exactly when it was needed. The wipe was guarded on `completed.length > 0`, but clear-bracket deliberately leaves placements alone and tells the admin to "Run Reprocess Bracket after rebuilding to clear the placements those results produced". After clear then regenerate nothing is completed, so the wipe was skipped and the discarded bracket's finalized placements survived — and upsertParticipantResult's never-un-finalize guard then stopped the entry floors and the replay from correcting them. The advertised recovery path could not work. The guard was not arbitrary: seasonParticipantResults is keyed by sports season, not by event, so a season-wide delete takes every other event's placements with it. Rather than flip the condition, narrow the delete. New deleteParticipantResultsForParticipants scopes it to the participants the bracket actually holds, which removes the collateral damage the guard was defending against, so the delete can run unconditionally. The participant set was already being computed further down for the elimination pass; it is now built once and reused. The qualifying branch keeps its season-wide delete, which is deliberate and rebuilds via finalizeQualifyingPoints. 2. Banked entry floors could miss teamStandings.totalPoints. generate-bracket recalculated standings only when `toEliminate` was empty, assuming markEliminatedAndAnnounce covers every other case. It does not — it recalculates only when the event is non-qualifying AND somebody was *newly* eliminated, i.e. had no prior result row. So the second run of a generation (the first wrote 0 for every non-bracket participant) recalculated nowhere, and neither did a qualifying event with teams to eliminate. The floors never reached the standings. markEliminatedAndAnnounce now returns { markedCount, recalculated } and the caller drives off that fact instead of re-deriving it, which also covers the case where the announcement threw — the catch swallows the error, and a failed recalc is precisely when the fallback should run. 3. The NBA mobile pager fell back to index geometry. Its BracketTreePaginated was the only one of five call sites not forwarding feeders/template, so mobile rendered "TBD" where desktop rendered "Winner of ...". Tests: reprocess wipes on a bracket with nothing played, stays scoped to the bracket, dedupes and skips empty slots, and leaves the qualifying path alone; generate recalculates in each of the four gaps above and still does not double-recalculate; and the NBA layout gives its mobile pane the same slot labels as desktop. Each was confirmed to fail against the previous behavior. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EmUdy42Rpgx9qZTnpQwirz --- app/components/scoring/NbaBracketLayout.tsx | 2 + .../__tests__/NbaBracketLayout.test.tsx | 116 ++++++++++ app/models/participant-result.ts | 29 ++- ...in.sports-seasons.bracket.generate.test.ts | 203 +++++++++++++++++ ...n.sports-seasons.bracket.reprocess.test.ts | 208 ++++++++++++++++++ ...sons.$id.events.$eventId.bracket.server.ts | 76 ++++--- 6 files changed, 607 insertions(+), 27 deletions(-) create mode 100644 app/components/scoring/__tests__/NbaBracketLayout.test.tsx create mode 100644 app/routes/__tests__/admin.sports-seasons.bracket.generate.test.ts create mode 100644 app/routes/__tests__/admin.sports-seasons.bracket.reprocess.test.ts diff --git a/app/components/scoring/NbaBracketLayout.tsx b/app/components/scoring/NbaBracketLayout.tsx index f446ab8..167925f 100644 --- a/app/components/scoring/NbaBracketLayout.tsx +++ b/app/components/scoring/NbaBracketLayout.tsx @@ -108,6 +108,8 @@ export function NbaBracketLayout({ ownershipMap={ownershipMap} userParticipantIds={userParticipantIds} firstScoringRoundIdx={scoringRoundIdx} + feeders={feeders} + template={template} /> diff --git a/app/components/scoring/__tests__/NbaBracketLayout.test.tsx b/app/components/scoring/__tests__/NbaBracketLayout.test.tsx new file mode 100644 index 0000000..17e213e --- /dev/null +++ b/app/components/scoring/__tests__/NbaBracketLayout.test.tsx @@ -0,0 +1,116 @@ +import { describe, it, expect } from "vitest"; +import { render, within } from "@testing-library/react"; +import { NbaBracketLayout } from "../NbaBracketLayout"; +import { buildFeederMap } from "~/lib/bracket-layout"; +import type { BracketTemplate } from "~/lib/bracket-templates"; +import type { BracketMatch } from "../BracketTreeView"; + +/** + * NbaBracketLayout renders a desktop view and a mobile pager side by side, hidden from + * each other by Tailwind breakpoints. Both need `feeders` and `template` — without them + * bracketGeometry falls back to index-derived positions and an unplayed slot reads "TBD" + * where the feeder graph would name the game it is waiting on. + */ + +const TEMPLATE: BracketTemplate = { + id: "test_conf_4", + name: "Two-conference test bracket", + totalTeams: 4, + scoringStartsAtRound: "Final", + rounds: [ + { name: "Semis", matchCount: 2, feedsInto: "Final", isScoring: false }, + { name: "Final", matchCount: 1, feedsInto: null, isScoring: true }, + ], + conferenceGroups: [ + { name: "East", roundMatchNumbers: { Semis: [1] } }, + { name: "West", roundMatchNumbers: { Semis: [2] } }, + ], +}; + +const ROUNDS = ["Semis", "Final"]; + +function match( + round: string, + matchNumber: number, + overrides: Partial = {} +): BracketMatch { + return { + id: `${round}-${matchNumber}`, + round, + matchNumber, + participant1Id: null, + participant2Id: null, + winnerId: null, + loserId: null, + isComplete: false, + participant1Score: null, + participant2Score: null, + ...overrides, + } as BracketMatch; +} + +/** Semis are played; the Final's two slots are still empty. */ +const MATCHES_BY_ROUND = new Map([ + [ + "Semis", + [ + match("Semis", 1, { participant1Id: "p1", participant2Id: "p2" }), + match("Semis", 2, { participant1Id: "p3", participant2Id: "p4" }), + ], + ], + ["Final", [match("Final", 1)]], +]); + +function renderLayout(withGraph: boolean) { + const { container } = render( + + ); + + // Both panes render in jsdom — media queries are class-based, not applied — so scope + // each assertion to the pane it is about. + const mobile = container.querySelector(".md\\:hidden"); + const desktop = container.querySelector(".md\\:flex"); + if (!mobile || !desktop) throw new Error("Expected both a mobile and a desktop pane"); + return { mobile, desktop }; +} + +describe("NbaBracketLayout", () => { + it("names the feeding game in the mobile pager", () => { + // Only slots filled by advancement get a label; a directly seeded slot with no + // participant still reads "TBD", which is why this asserts on the Final's slots. + const { mobile } = renderLayout(true); + + expect(within(mobile).getAllByText(/Winner of/).length).toBe(2); + }); + + it("shows the mobile pager the same slot labels as the desktop view", () => { + const { mobile, desktop } = renderLayout(true); + + const labels = (pane: HTMLElement) => + within(pane) + .getAllByText(/Winner of/) + .map((el) => el.textContent) + .toSorted(); + + expect(labels(mobile)).toEqual(labels(desktop)); + }); + + it("falls back to TBD when the feeder graph is unavailable", () => { + // Guards the assertions above: without feeders/template there is nothing to name a + // slot with, which is exactly the state the mobile pane was stuck in. + const { mobile } = renderLayout(false); + + expect(within(mobile).queryByText(/Winner of/)).toBeNull(); + expect(within(mobile).getAllByText("TBD").length).toBeGreaterThan(0); + }); +}); diff --git a/app/models/participant-result.ts b/app/models/participant-result.ts index 4e36257..6da1065 100644 --- a/app/models/participant-result.ts +++ b/app/models/participant-result.ts @@ -1,4 +1,4 @@ -import { eq, and } from "drizzle-orm"; +import { eq, and, inArray } from "drizzle-orm"; import { database } from "~/database/context"; import * as schema from "~/database/schema"; @@ -104,6 +104,33 @@ export async function deleteParticipantResultsBySportsSeasonId( .where(eq(schema.seasonParticipantResults.sportsSeasonId, sportsSeasonId)); } +/** + * Delete the results of specific participants within one sports season. + * + * The season-wide delete above is too blunt for a single bracket: results are keyed by + * sports season, not by event, so wiping the season takes every other event's placements + * with it. Scoping to the participants a bracket actually holds lets reprocess-bracket + * rebuild that bracket from scratch while leaving the rest of the season alone. + * + * No-ops on an empty id list — `inArray` with no values is not a valid SQL predicate. + */ +export async function deleteParticipantResultsForParticipants( + sportsSeasonId: string, + participantIds: string[], + providedDb?: ReturnType +): Promise { + if (participantIds.length === 0) return; + const db = providedDb || database(); + await db + .delete(schema.seasonParticipantResults) + .where( + and( + eq(schema.seasonParticipantResults.sportsSeasonId, sportsSeasonId), + inArray(schema.seasonParticipantResults.participantId, participantIds) + ) + ); +} + /** * Set result for a participant in a sports season * Points are calculated on-demand based on each fantasy league's scoring rules diff --git a/app/routes/__tests__/admin.sports-seasons.bracket.generate.test.ts b/app/routes/__tests__/admin.sports-seasons.bracket.generate.test.ts new file mode 100644 index 0000000..2c84ccc --- /dev/null +++ b/app/routes/__tests__/admin.sports-seasons.bracket.generate.test.ts @@ -0,0 +1,203 @@ +/** + * generate-bracket banks the floors a seeding guarantees before anyone plays (an AFL + * top-4 seed cannot finish below the 5th-6th tier). Those floors only reach + * teamStandings.totalPoints through a standings recalculation, so the action has to be + * sure one ran — markEliminatedAndAnnounce runs one for its Discord announcement in some + * cases but not others. + */ + +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { generateBracketFromTemplate } from "~/models/playoff-match"; +import { + findParticipantResultsBySportsSeasonId, + setParticipantResult, +} from "~/models/participant-result"; +import { + applyBracketEntryFloors, + recalculateAffectedLeagues, +} from "~/models/scoring-calculator"; +import { getScoringEventById, updateScoringEvent } from "~/models/scoring-event"; +import { findParticipantsBySportsSeasonId } from "~/models/season-participant"; +import { action } from "../admin.sports-seasons.$id.events.$eventId.bracket.server"; + +vi.mock("~/database/context", () => ({ database: vi.fn(() => ({})) })); +vi.mock("~/models/scoring-event", async (importOriginal) => ({ + ...(await importOriginal()), + getScoringEventById: vi.fn(), + updateScoringEvent: vi.fn(), + isReadOnlySibling: vi.fn(() => false), +})); +vi.mock("~/models/playoff-match", async (importOriginal) => ({ + ...(await importOriginal()), + generateBracketFromTemplate: vi.fn(), +})); +vi.mock("~/models/participant-result", async (importOriginal) => ({ + ...(await importOriginal()), + findParticipantResultsBySportsSeasonId: vi.fn(), + setParticipantResult: vi.fn(), +})); +vi.mock("~/models/scoring-calculator", async (importOriginal) => ({ + ...(await importOriginal()), + applyBracketEntryFloors: vi.fn(), + recalculateAffectedLeagues: vi.fn(), +})); +vi.mock("~/models/season-participant", async (importOriginal) => ({ + ...(await importOriginal()), + findParticipantsBySportsSeasonId: vi.fn(), +})); + +const params = { id: "season-1", eventId: "event-1" }; + +const EVENT = { + id: "event-1", + name: "AFL Finals", + sportsSeasonId: "season-1", + isQualifyingEvent: false, + bracketTemplateId: "afl_10", +}; + +/** afl_10 takes exactly 10 seeded participants. */ +const SEEDED = Array.from({ length: 10 }, (_, i) => `seed-${i + 1}`); + +function generateRequest(): Request { + const body = new FormData(); + body.set("intent", "generate-bracket"); + body.set("templateId", "afl_10"); + SEEDED.forEach((id, i) => body.set(`participant${i}`, id)); + return new Request("http://localhost/generate", { method: "POST", body }); +} + +const run = (request: Request) => + (action as unknown as (args: { request: Request; params: typeof params }) => Promise<{ + error?: string; + success?: string; + }>)({ request, params }); + +/** + * @param extras participants in the season beyond the 10 seeded into the bracket — + * these are the ones generate-bracket marks eliminated. + * @param withExistingResults ids that already carry a result row, so + * markEliminatedAndAnnounce treats them as not newly eliminated. + */ +function setSeason(extras: string[], withExistingResults: string[] = []) { + vi.mocked(findParticipantsBySportsSeasonId).mockResolvedValue( + [...SEEDED, ...extras].map((id) => ({ id })) as unknown as Awaited< + ReturnType + > + ); + vi.mocked(findParticipantResultsBySportsSeasonId).mockResolvedValue( + withExistingResults.map((participantId) => ({ participantId })) as unknown as Awaited< + ReturnType + > + ); +} + +describe("generate-bracket entry-floor standings recalculation", () => { + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(getScoringEventById).mockResolvedValue( + EVENT as unknown as Awaited> + ); + vi.mocked(generateBracketFromTemplate).mockResolvedValue( + undefined as unknown as Awaited> + ); + vi.mocked(updateScoringEvent).mockResolvedValue( + undefined as unknown as Awaited> + ); + vi.mocked(setParticipantResult).mockResolvedValue( + undefined as unknown as Awaited> + ); + vi.mocked(recalculateAffectedLeagues).mockResolvedValue( + undefined as unknown as Awaited> + ); + // afl_10 seeds 1-4 into the Qualifying Finals, whose entry floor is the 5th-6th tier. + vi.mocked(applyBracketEntryFloors).mockResolvedValue(4); + }); + + it("recalculates when every eliminated team already had a result row", async () => { + // The second run of a generation: the first wrote position 0 for the non-bracket + // participants, so nobody is *newly* eliminated and the announcement is skipped. + // The floors banked moments ago would never reach the standings. + setSeason(["extra-1"], ["extra-1"]); + + const result = await run(generateRequest()); + + expect(result.success).toBeDefined(); + expect(recalculateAffectedLeagues).toHaveBeenCalledTimes(1); + expect(recalculateAffectedLeagues).toHaveBeenCalledWith( + "season-1", + expect.anything(), + expect.objectContaining({ skipDiscord: true }) + ); + }); + + it("recalculates for a qualifying event, which never announces eliminations", async () => { + vi.mocked(getScoringEventById).mockResolvedValue( + { ...EVENT, isQualifyingEvent: true } as unknown as Awaited< + ReturnType + > + ); + setSeason(["extra-1"]); + + await run(generateRequest()); + + expect(recalculateAffectedLeagues).toHaveBeenCalledTimes(1); + expect(recalculateAffectedLeagues).toHaveBeenCalledWith( + "season-1", + expect.anything(), + expect.objectContaining({ skipDiscord: true }) + ); + }); + + it("recalculates when the bracket field is the whole season", async () => { + setSeason([]); + + await run(generateRequest()); + + expect(recalculateAffectedLeagues).toHaveBeenCalledTimes(1); + }); + + it("recalculates when the elimination announcement threw", async () => { + // The announcement is best-effort and its failure is swallowed — but a failed recalc + // is exactly when the floors still need one. + setSeason(["extra-1"]); + vi.mocked(recalculateAffectedLeagues) + .mockRejectedValueOnce(new Error("discord down")) + .mockResolvedValue(undefined as unknown as Awaited>); + + const result = await run(generateRequest()); + + expect(result.success).toBeDefined(); + expect(recalculateAffectedLeagues).toHaveBeenCalledTimes(2); + expect(recalculateAffectedLeagues).toHaveBeenLastCalledWith( + "season-1", + expect.anything(), + expect.objectContaining({ skipDiscord: true }) + ); + }); + + it("does not recalculate twice when the announcement already did", async () => { + setSeason(["extra-1"]); + + await run(generateRequest()); + + expect(recalculateAffectedLeagues).toHaveBeenCalledTimes(1); + // The announcing call, not the floor fallback. + expect(recalculateAffectedLeagues).toHaveBeenCalledWith( + "season-1", + expect.anything(), + expect.objectContaining({ eliminatedParticipantIds: ["extra-1"] }) + ); + }); + + it("does not recalculate at all when no floors were banked", async () => { + // A template that guarantees nothing at seeding: no floors, nobody to eliminate, + // so there is nothing for a recalculation to pick up. + vi.mocked(applyBracketEntryFloors).mockResolvedValue(0); + setSeason([]); + + await run(generateRequest()); + + expect(recalculateAffectedLeagues).not.toHaveBeenCalled(); + }); +}); diff --git a/app/routes/__tests__/admin.sports-seasons.bracket.reprocess.test.ts b/app/routes/__tests__/admin.sports-seasons.bracket.reprocess.test.ts new file mode 100644 index 0000000..a7101de --- /dev/null +++ b/app/routes/__tests__/admin.sports-seasons.bracket.reprocess.test.ts @@ -0,0 +1,208 @@ +/** + * reprocess-bracket rebuilds a bracket's placements from scratch. What it wipes first + * decides whether the clear-bracket → regenerate → reprocess repair path actually works, + * and whether it takes the rest of the season's placements down with it. + */ + +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { findPlayoffMatchesByEventId } from "~/models/playoff-match"; +import { + deleteParticipantResultsBySportsSeasonId, + deleteParticipantResultsForParticipants, + setParticipantResult, +} from "~/models/participant-result"; +import { + applyBracketEntryFloors, + processMatchResult, + processQualifyingBracketEvent, + recalculateAffectedLeagues, +} from "~/models/scoring-calculator"; +import { getScoringEventById } from "~/models/scoring-event"; +import { findParticipantsBySportsSeasonId } from "~/models/season-participant"; +import { findSportsSeasonById } from "~/models/sports-season"; +import { action } from "../admin.sports-seasons.$id.events.$eventId.bracket.server"; + +vi.mock("~/database/context", () => ({ database: vi.fn(() => ({})) })); +vi.mock("~/models/scoring-event", async (importOriginal) => ({ + ...(await importOriginal()), + getScoringEventById: vi.fn(), + updateScoringEvent: vi.fn(), + isReadOnlySibling: vi.fn(() => false), +})); +vi.mock("~/models/playoff-match", async (importOriginal) => ({ + ...(await importOriginal()), + findPlayoffMatchesByEventId: vi.fn(), +})); +vi.mock("~/models/participant-result", async (importOriginal) => ({ + ...(await importOriginal()), + deleteParticipantResultsBySportsSeasonId: vi.fn(), + deleteParticipantResultsForParticipants: vi.fn(), + setParticipantResult: vi.fn(), +})); +vi.mock("~/models/scoring-calculator", async (importOriginal) => ({ + ...(await importOriginal()), + applyBracketEntryFloors: vi.fn(), + processMatchResult: vi.fn(), + recalculateAffectedLeagues: vi.fn(), + processQualifyingBracketEvent: vi.fn(), + finalizeQualifyingPoints: vi.fn(), +})); +vi.mock("~/models/season-participant", async (importOriginal) => ({ + ...(await importOriginal()), + findParticipantsBySportsSeasonId: vi.fn(), +})); +vi.mock("~/models/sports-season", async (importOriginal) => ({ + ...(await importOriginal()), + findSportsSeasonById: vi.fn(), +})); + +const params = { id: "season-1", eventId: "event-1" }; + +const EVENT = { + id: "event-1", + name: "AFL Finals", + sportsSeasonId: "season-1", + isQualifyingEvent: false, + isPrimary: false, + tournamentId: null, + bracketTemplateId: "afl_10", +}; + +function reprocessRequest(): Request { + const body = new FormData(); + body.set("intent", "reprocess-bracket"); + return new Request("http://localhost/reprocess", { method: "POST", body }); +} + +/** A seeded, unplayed bracket slot. */ +function slot(matchNumber: number, participant1Id: string, participant2Id: string) { + return { + id: `m-${matchNumber}`, + round: "Qualifying Finals", + matchNumber, + participant1Id, + participant2Id, + winnerId: null, + loserId: null, + isComplete: false, + isScoring: true, + }; +} + +const run = (request: Request) => + (action as unknown as (args: { request: Request; params: typeof params }) => Promise<{ + error?: string; + success?: string; + }>)({ request, params }); + +function setEvent(overrides: Partial = {}) { + vi.mocked(getScoringEventById).mockResolvedValue( + { ...EVENT, ...overrides } as unknown as Awaited> + ); +} + +function setMatches(matches: ReturnType[]) { + vi.mocked(findPlayoffMatchesByEventId).mockResolvedValue( + matches as unknown as Awaited> + ); +} + +describe("reprocess-bracket", () => { + beforeEach(() => { + vi.clearAllMocks(); + setEvent(); + vi.mocked(applyBracketEntryFloors).mockResolvedValue(4); + vi.mocked(findParticipantsBySportsSeasonId).mockResolvedValue( + [] as unknown as Awaited> + ); + vi.mocked(setParticipantResult).mockResolvedValue( + undefined as unknown as Awaited> + ); + vi.mocked(recalculateAffectedLeagues).mockResolvedValue( + undefined as unknown as Awaited> + ); + vi.mocked(processMatchResult).mockResolvedValue( + undefined as unknown as Awaited> + ); + vi.mocked(processQualifyingBracketEvent).mockResolvedValue( + undefined as unknown as Awaited> + ); + vi.mocked(findSportsSeasonById).mockResolvedValue( + { qualifyingPointsFinalized: false } as unknown as Awaited< + ReturnType + > + ); + }); + + it("clears placements even when no match has been played", async () => { + // The clear-bracket → regenerate → reprocess repair path lands here: the freshly + // re-seeded bracket has nothing completed, yet the discarded bracket's finalized + // placements are exactly what has to go. Skipping the wipe leaves them permanently, + // because upsertParticipantResult refuses to un-finalize a result. + setMatches([slot(1, "p1", "p2"), slot(2, "p3", "p4")]); + + const result = await run(reprocessRequest()); + + expect(result.success).toBeDefined(); + expect(deleteParticipantResultsForParticipants).toHaveBeenCalledTimes(1); + const [sportsSeasonId, ids] = vi.mocked(deleteParticipantResultsForParticipants).mock.calls[0]; + expect(sportsSeasonId).toBe("season-1"); + expect([...ids].toSorted()).toEqual(["p1", "p2", "p3", "p4"]); + }); + + it("scopes the wipe to this bracket, never the whole season", async () => { + // A season-wide delete would take every other event's placements with it, with only + // this bracket's replay able to rebuild them. + setMatches([slot(1, "p1", "p2")]); + + await run(reprocessRequest()); + + expect(deleteParticipantResultsBySportsSeasonId).not.toHaveBeenCalled(); + const [, ids] = vi.mocked(deleteParticipantResultsForParticipants).mock.calls[0]; + expect(ids).not.toContain("p3"); + }); + + it("passes each participant once when a team appears in more than one slot", async () => { + setMatches([slot(1, "p1", "p2"), slot(2, "p1", "p3")]); + + await run(reprocessRequest()); + + const [, ids] = vi.mocked(deleteParticipantResultsForParticipants).mock.calls[0]; + expect(ids).toHaveLength(3); + expect([...ids].toSorted()).toEqual(["p1", "p2", "p3"]); + }); + + it("skips empty slots rather than passing nulls through", async () => { + setMatches([ + { ...slot(1, "p1", "p2"), participant2Id: null as unknown as string }, + ]); + + await run(reprocessRequest()); + + const [, ids] = vi.mocked(deleteParticipantResultsForParticipants).mock.calls[0]; + expect(ids).toEqual(["p1"]); + }); + + it("still takes the season-wide delete for a qualifying event", async () => { + // Qualifying seasons have no legitimate per-major fantasy placements — those come + // from finalizeQualifyingPoints across all majors — so that path wipes the season + // on purpose and rebuilds QP from the bracket. + setEvent({ isQualifyingEvent: true }); + setMatches([slot(1, "p1", "p2")]); + + await run(reprocessRequest()); + + expect(deleteParticipantResultsBySportsSeasonId).toHaveBeenCalledWith("season-1", {}); + expect(deleteParticipantResultsForParticipants).not.toHaveBeenCalled(); + }); + + it("rejects an event with no bracket rather than wiping anything", async () => { + setMatches([]); + + const result = await run(reprocessRequest()); + + expect(result.error).toContain("No bracket to reprocess"); + expect(deleteParticipantResultsForParticipants).not.toHaveBeenCalled(); + expect(deleteParticipantResultsBySportsSeasonId).not.toHaveBeenCalled(); + }); +}); diff --git a/app/routes/admin.sports-seasons.$id.events.$eventId.bracket.server.ts b/app/routes/admin.sports-seasons.$id.events.$eventId.bracket.server.ts index fcae69b..bb8c04e 100644 --- a/app/routes/admin.sports-seasons.$id.events.$eventId.bracket.server.ts +++ b/app/routes/admin.sports-seasons.$id.events.$eventId.bracket.server.ts @@ -44,6 +44,7 @@ import { setParticipantResult, findParticipantResultsBySportsSeasonId, deleteParticipantResultsBySportsSeasonId, + deleteParticipantResultsForParticipants, } from "~/models/participant-result"; import { findSeasonSportsBySportsSeasonId } from "~/models/season-sport"; import { createDailySnapshot } from "~/models/standings"; @@ -171,7 +172,7 @@ async function scoreQualifyingBracket( /** * 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. + * affected leagues' Discord channels. * * "Newly eliminated" = participants with no prior result row, so re-running a * generation step never re-announces the same teams. The announcement is a @@ -179,11 +180,18 @@ async function scoreQualifyingBracket( * 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". + * + * Returns the number of participants marked alongside whether a standings recalculation + * actually ran. The caller banks entry floors before calling this and needs them to + * reach teamStandings.totalPoints; it cannot infer that from the participant count, + * because the recalc is skipped for qualifying events, when every eliminated team + * already had a result row (the second run of a generation), and when the announcement + * threw. `recalculated` reports the fact rather than making the caller re-derive it. */ async function markEliminatedAndAnnounce( event: { id: string; name: string | null; sportsSeasonId: string; isQualifyingEvent: boolean }, participantIds: string[] -): Promise { +): Promise<{ markedCount: number; recalculated: boolean }> { const existingResults = await findParticipantResultsBySportsSeasonId(event.sportsSeasonId); const alreadyHadResult = new Set(existingResults.map((r) => r.participantId)); const newlyEliminatedIds = participantIds.filter((id) => !alreadyHadResult.has(id)); @@ -192,6 +200,8 @@ async function markEliminatedAndAnnounce( await setParticipantResult(participantId, event.sportsSeasonId, 0); } + let recalculated = false; + // QPs (e.g. tennis/CS2 majors) don't get elimination announcements. if (!event.isQualifyingEvent && newlyEliminatedIds.length > 0) { try { @@ -199,12 +209,13 @@ async function markEliminatedAndAnnounce( eventName: event.name ?? undefined, eliminatedParticipantIds: newlyEliminatedIds, }); + recalculated = true; } catch (err) { logger.error("[Eliminations] Discord announcement failed:", err); } } - return participantIds.length; + return { markedCount: participantIds.length, recalculated }; } export async function action({ request, params }: Route.ActionArgs) { @@ -423,14 +434,17 @@ export async function action({ request, params }: Route.ActionArgs) { 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`); + const { markedCount, recalculated } = await markEliminatedAndAnnounce(event, toEliminate); + logger.log(`[BracketGeneration] Marked ${markedCount} participants as eliminated`); - // markEliminatedAndAnnounce recalculates standings only when it actually - // eliminated somebody. When the bracket field is the whole season (nothing to - // eliminate) the entry floors above would never reach teamStandings.totalPoints, - // so recalculate here. skipDiscord: seeding floors are not a result to announce. - if (toEliminate.length === 0 && entryFloorCount > 0) { + // The floors banked above only reach teamStandings.totalPoints via a recalc, and + // markEliminatedAndAnnounce runs one for its announcement in some cases but not + // others: not for a qualifying event, not when every eliminated team already had + // a result row (the second run of a generation, since the first wrote 0 for all + // of them), not when there was nobody to eliminate, and not when the announcement + // threw. Drive off what it reports rather than re-deriving it from toEliminate. + // skipDiscord: seeding floors are not a result to announce. + if (entryFloorCount > 0 && !recalculated) { await recalculateAffectedLeagues(event.sportsSeasonId, database(), { eventName: event.name ?? undefined, skipDiscord: true, @@ -910,19 +924,31 @@ export async function action({ request, params }: Route.ActionArgs) { return { error: "No bracket to reprocess" }; } - // Delete ALL results for this sports season and rebuild from scratch. - // Only deleting partial rows leaves stale finalized rows that block - // the "never un-finalize" guard in upsertParticipantResult. + // Wipe this bracket's participants' results and rebuild from scratch. Deleting + // only the partial rows would leave stale finalized ones, which the "never + // un-finalize" guard in upsertParticipantResult then refuses to correct. // - // seasonParticipantResults is keyed by sports season, not by event, so this - // wipes every event's placements in the season and only the replay below can - // rebuild them (the same hazard clear-bracket documents). With nothing to - // replay there is nothing to rebuild from, so skip it entirely: applying entry - // floors and re-marking eliminations below is additive and needs no wipe. + // Scoped to the participants this bracket actually holds, not the whole season: + // seasonParticipantResults is keyed by sports season, not by event, so a + // season-wide delete takes every other event's placements with it and only this + // bracket's replay could rebuild them (the hazard clear-bracket documents). + // + // Unconditional, because zero completed matches is precisely the clear-bracket → + // regenerate → reprocess repair path: the discarded bracket's finalized + // placements are exactly what needs clearing, and there is always something to + // rebuild from — the entry floors below, then the replay. const db = database(); - if (completed.length > 0) { - await deleteParticipantResultsBySportsSeasonId(event.sportsSeasonId, db); + // Reused further down to decide who is *not* in the bracket and so eliminated. + const bracketParticipantIds = new Set(); + for (const match of matches) { + if (match.participant1Id) bracketParticipantIds.add(match.participant1Id); + if (match.participant2Id) bracketParticipantIds.add(match.participant2Id); } + await deleteParticipantResultsForParticipants( + event.sportsSeasonId, + [...bracketParticipantIds], + db + ); // Re-bank the seeding-derived floors the delete above wipes (e.g. the AFL // top-4's 5th-6th tier). Done before the replay so real match results overwrite @@ -968,11 +994,6 @@ export async function action({ request, params }: Route.ActionArgs) { // Mark participants NOT in any bracket match as eliminated (finalPosition = 0). // This covers teams that didn't make the playoffs/play-in tournament. const allParticipants = await findParticipantsBySportsSeasonId(params.id); - const bracketParticipantIds = new Set(); - for (const match of matches) { - if (match.participant1Id) bracketParticipantIds.add(match.participant1Id); - if (match.participant2Id) bracketParticipantIds.add(match.participant2Id); - } let eliminatedCount = 0; for (const participant of allParticipants) { if (!bracketParticipantIds.has(participant.id)) { @@ -1178,7 +1199,10 @@ export async function action({ request, params }: Route.ActionArgs) { const toEliminate = allParticipants .filter((p) => !uniqueParticipants.has(p.id)) .map((p) => p.id); - const eliminatedCount = await markEliminatedAndAnnounce(groupsEvent, toEliminate); + const { markedCount: 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)` : ""}`, -- 2.45.3