From 101e102e23aa98681e09267a1c3f973da9f0ff33 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 06:18:41 +0000 Subject: [PATCH] Fix review findings on the MLB projected-wins change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit did not build. `simulatorInputLabel` was called from the rendered component for the Base Elo Source select, which defeated the treeshaking that keeps the simulator manifest — and through it the registry, every simulator, and ~/database/context — out of the browser. Vite fails the client build outright with "Server-only module referenced by client". The label is now resolved in the loader alongside inputColumns, which is the pattern the loader comment already documents. Typecheck, lint and the unit suite all passed on the broken commit; none of them run a production build. A stale projection now yields to Elo instead of clamping to an extreme. seedingWinRateFor clamped the rest-of-season target into [0.01, 0.99], so a 96-40 team projected for 95 was simulated to go 0-26 and a 40-70 team projected for 95 was simulated to win out. A target outside (0, 1) is proof the projection has gone stale, not a reason to bet everything on it, so it falls back to the Elo rate — the same escape hatch nll-simulator uses. The blend weight is also clamped inside the helper now, so a stray config value cannot turn it into an extrapolation. Seeding only applies a projection that actually produced the resolved Elo, gated on metadata.sourceEloMethod via projectionForSeeding. Previously the raw projection drove seeding regardless of the input policy: with the default Elo-first ordering a projection was ignored as the Elo source yet still dictated the standings, and with futures odds blended in at oddsWeight, seeding and playoff matchups ran on two different strength scales. The simulator page's preview resolved its Elo and rating maps only for required inputs, but renders those columns solely from the maps, so stored values displayed as "—" for profiles that treat the input as optional (playoff_bracket, ncaam_bracket, golf_qualifying_points). Both maps now key off the same required-plus-optional set the columns do. The metadata upsert's CASE branches were mutually exclusive, so supplying any metadata skipped the stale-flag clearing: a bulk row carrying both a direct rating and a projection kept a stale ratingMethod and hid the rating it had just set. Stripping now always runs, with the caller's metadata merged over the result. Not changed: projectedWinsWeight still defaults to 1 with no decay toward Elo. That is the final-win-total semantics chosen for this work; the stale-target fallback removes its pathological case. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CQSEmmojmqmGdJttgzqCWK --- app/models/simulator.ts | 33 +++++--- .../admin.sports-seasons.$id.simulator.tsx | 81 +++++++++++-------- .../__tests__/mlb-simulator.test.ts | 80 +++++++++++++++++- app/services/simulations/mlb-simulator.ts | 69 ++++++++++++---- docs/agents/simulators.md | 14 ++++ 5 files changed, 211 insertions(+), 66 deletions(-) diff --git a/app/models/simulator.ts b/app/models/simulator.ts index 381d5ff..380e45a 100644 --- a/app/models/simulator.ts +++ b/app/models/simulator.ts @@ -359,21 +359,28 @@ export async function batchUpsertParticipantSimulatorInputs( region: sql`COALESCE(excluded.region, ${schema.seasonParticipantSimulatorInputs.region})`, // Metadata carries the method flags (sourceEloMethod/ratingMethod) that // tell readers whether the stored Elo/rating is generated vs. a trusted - // direct value. When a caller supplies explicit metadata, merge it over - // what is already stored (prepareSimulatorInputsForRun and the projection - // importers set the correct flags) — a merge rather than a replace so a - // caller that only needs to stamp a method flag does not wipe unrelated - // metadata keys. Otherwise preserve existing metadata, but drop the - // method flag for any column receiving a fresh direct value — otherwise a - // stale "generated" flag would cause that newly-entered Elo/rating to be - // filtered out as derived (see getParticipantSimulatorInputs). - metadata: sql`CASE - WHEN excluded.metadata IS NOT NULL - THEN COALESCE(${schema.seasonParticipantSimulatorInputs.metadata}, '{}'::jsonb) || excluded.metadata - ELSE COALESCE(${schema.seasonParticipantSimulatorInputs.metadata}, '{}'::jsonb) + // direct value. Two rules apply, and both always apply — they are not + // alternatives: + // + // 1. Drop the method flag for any column receiving a fresh direct + // value, otherwise a stale "generated" flag would cause that + // newly-entered Elo/rating to be filtered out as derived (see + // getParticipantSimulatorInputs). + // 2. Merge any metadata the caller supplied over the result + // (prepareSimulatorInputsForRun and the projection importers set the + // correct flags) — a merge rather than a replace so a caller that + // only needs to stamp one method flag does not wipe unrelated keys. + // + // Ordering matters: strip first, then merge, so a caller stamping one flag + // still gets the other column's stale flag cleared. Running these as + // exclusive CASE branches instead would mean a bulk row carrying both a + // direct `rating` and a `projectedWins` (which stamps sourceEloMethod) + // silently kept a stale ratingMethod, hiding the rating it just set. + metadata: sql`( + COALESCE(${schema.seasonParticipantSimulatorInputs.metadata}, '{}'::jsonb) - (CASE WHEN excluded.source_elo IS NOT NULL THEN 'sourceEloMethod' ELSE '' END) - (CASE WHEN excluded.rating IS NOT NULL THEN 'ratingMethod' ELSE '' END) - END`, + ) || COALESCE(excluded.metadata, '{}'::jsonb)`, updatedAt: sql`excluded.updated_at`, }, }); diff --git a/app/routes/admin.sports-seasons.$id.simulator.tsx b/app/routes/admin.sports-seasons.$id.simulator.tsx index e18f138..e886de7 100644 --- a/app/routes/admin.sports-seasons.$id.simulator.tsx +++ b/app/routes/admin.sports-seasons.$id.simulator.tsx @@ -36,7 +36,6 @@ import { resolveSourceElos, type MissingEloStrategy, type MissingRatingStrategy, - type ResolvedSourceElo, } from "~/services/simulations/input-policy"; import { runSportsSeasonSimulation } from "~/services/simulations/runner"; import { @@ -74,31 +73,6 @@ export async function loader({ params }: Route.LoaderArgs) { const inputPolicy = getSimulatorInputPolicy(config.config); - // The Elo each participant will actually run with, and which source produced it. - // Without this the preview is misleading: getParticipantSimulatorInputs blanks a - // generated Elo (so it is re-derived rather than frozen), which reads as "nothing - // saved" — and a raw Elo silently beating a projection is invisible. - const resolvedElos = config.profile.requiredInputs.includes("sourceElo") - ? resolveSourceElos(inputs, config.profile, config.config) - : new Map(); - const resolvedEloRows = Object.fromEntries( - [...resolvedElos.values()].map((resolved) => [ - resolved.participantId, - { sourceElo: resolved.sourceElo, method: resolved.method }, - ]) - ); - // Same for ratings, which are blanked by the same rule when generated. The - // preview's "missing a required input" marker reads both, so it agrees with - // readiness instead of flagging every participant a projection resolved. - const resolvedRatingRows = Object.fromEntries( - config.profile.requiredInputs.includes("rating") - ? [...resolveRatings(inputs, config.profile, config.config).values()].map((resolved) => [ - resolved.participantId, - { rating: resolved.rating, method: resolved.method }, - ]) - : [] - ); - // Sport-aware preview columns: the intersection of the displayable numeric keys // with this simulator's required + optional inputs, so each season shows exactly // the inputs its simulator consumes (F1 = odds, NBA = Elo, NCAA = rating, ...). @@ -108,12 +82,52 @@ export async function loader({ params }: Route.LoaderArgs) { ...config.profile.requiredInputs, ...config.profile.optionalInputs, ]); + + // The Elo each participant will actually run with, and which source produced it. + // Without this the preview is misleading: getParticipantSimulatorInputs blanks a + // generated Elo (so it is re-derived rather than frozen), which reads as "nothing + // saved" — and a raw Elo silently beating a projection is invisible. + // + // Keyed off `relevantInputs`, not requiredInputs: the preview renders these + // columns solely from these maps, so gating on "required" would blank a stored + // value for every simulator that treats the input as optional (playoff_bracket + // and ncaam_bracket for Elo, golf_qualifying_points for rating). + const resolvedEloRows = Object.fromEntries( + relevantInputs.has("sourceElo") + ? [...resolveSourceElos(inputs, config.profile, config.config).values()].map((resolved) => [ + resolved.participantId, + { sourceElo: resolved.sourceElo, method: resolved.method }, + ]) + : [] + ); + // Same for ratings, which are blanked by the same rule when generated. The + // preview's "missing a required input" marker reads both, so it agrees with + // readiness instead of flagging every participant a projection resolved. + const resolvedRatingRows = Object.fromEntries( + relevantInputs.has("rating") + ? [...resolveRatings(inputs, config.profile, config.config).values()].map((resolved) => [ + resolved.participantId, + { rating: resolved.rating, method: resolved.method }, + ]) + : [] + ); const inputColumns = DISPLAY_INPUT_ORDER.filter((key) => relevantInputs.has(key)).map((key) => ({ key, label: simulatorInputLabel(key), required: config.profile.requiredInputs.includes(key), })); + // The projection this simulator can derive Elo from, labelled here for the same + // reason as inputColumns: calling simulatorInputLabel from the rendered component + // would pull the manifest (and through it the registry and every simulator) into + // the client bundle. + const projectionEloKey = (config.profile.derivableInputs?.sourceElo ?? []).find( + (key) => key === "projectedWins" || key === "projectedTablePoints" + ); + const projectionEloOption = projectionEloKey + ? { key: projectionEloKey, label: simulatorInputLabel(projectionEloKey) } + : null; + return { sportsSeason, participants, @@ -124,6 +138,7 @@ export async function loader({ params }: Route.LoaderArgs) { inputColumns, resolvedEloRows, resolvedRatingRows, + projectionEloOption, }; } @@ -440,10 +455,6 @@ export default function AdminSportsSeasonSimulator({ loaderData }: Route.Compone const isSubmitting = navigation.state === "submitting"; const setupSections = config.profile.setupSections; const sourceEloAlternatives = config.profile.derivableInputs?.sourceElo ?? []; - // The projection key this simulator can derive Elo from (wins or table points), - // or null when it has none — the base-priority control only makes sense with one. - const projectionEloKey = - sourceEloAlternatives.find((key) => key === "projectedWins" || key === "projectedTablePoints") ?? null; const projectionsOutrankElo = inputPolicy.baseEloPriority[0] !== "sourceElo"; const ratingAlternatives = config.profile.derivableInputs?.rating ?? []; const showsInputPolicy = @@ -458,7 +469,7 @@ export default function AdminSportsSeasonSimulator({ loaderData }: Route.Compone // Preview columns are resolved server-side in the loader (see note there) and // arrive as plain data, so this client component never imports the manifest. - const { inputColumns, resolvedEloRows, resolvedRatingRows } = loaderData; + const { inputColumns, resolvedEloRows, resolvedRatingRows, projectionEloOption } = loaderData; const requiredInputs = config.profile.requiredInputs; const gridTemplate = `2fr repeat(${Math.max(inputColumns.length, 1)}, 1fr)`; // For this sport the inputs live on a dedicated page, not the shared bulk paste. @@ -652,7 +663,7 @@ export default function AdminSportsSeasonSimulator({ loaderData }: Route.Compone this Elo — they are not blended again per game.

- {projectionEloKey && ( + {projectionEloOption && (

Raw Elo and projections are substitutes — the first one a participant has wins, and the other is ignored (futures odds are separate and blend on top via the weight above). - Pick {simulatorInputLabel(projectionEloKey)} first when projections are + Pick {projectionEloOption.label} first when projections are the source of truth for this season and a previously entered Elo should not override them.

diff --git a/app/services/simulations/__tests__/mlb-simulator.test.ts b/app/services/simulations/__tests__/mlb-simulator.test.ts index f2bf082..f0adbd0 100644 --- a/app/services/simulations/__tests__/mlb-simulator.test.ts +++ b/app/services/simulations/__tests__/mlb-simulator.test.ts @@ -7,6 +7,7 @@ import { rawWinRateFromElo, rdifWinProbability, eloToRDif, + projectionForSeeding, seedingWinRateFor, sampleBinomial, simBo3, @@ -404,12 +405,47 @@ describe("seedingWinRateFor", () => { expect(currentWins + rate * remaining).toBeCloseTo(95, 6); }); - it("clamps a team that has already passed its projection", () => { - expect(seedingWinRateFor(eloRate, 95, 96, 20)).toBe(0.01); + it("falls back to the Elo rate once a team has passed its projection", () => { + // Clamping to a floor instead would simulate a 96-40 team to go 0-26 for the + // rest of the season and drop out of the field. The projection is stale, so it + // is dropped rather than obeyed. + expect(seedingWinRateFor(eloRate, 95, 96, 26)).toBe(eloRate); }); - it("clamps a target that is unreachable", () => { - expect(seedingWinRateFor(eloRate, 95, 60, 10)).toBe(0.99); + it("falls back to the Elo rate when a team has exactly met its projection", () => { + expect(seedingWinRateFor(eloRate, 95, 95, 26)).toBe(eloRate); + }); + + it("falls back to the Elo rate when the projection is unreachable", () => { + // 40-70 projected for 95 needs better than 1.000 — the mirror image of the + // case above, and dropped for the same reason. + expect(seedingWinRateFor(eloRate, 95, 40, 52)).toBe(eloRate); + }); + + it("falls back to the Elo rate when the target is exactly 1.000", () => { + expect(seedingWinRateFor(eloRate, 95, 43, 52)).toBe(eloRate); + }); + + it("keeps a target just inside the reachable range", () => { + expect(seedingWinRateFor(eloRate, 95, 94, 26)).toBeCloseTo(1 / 26, 6); + }); + + it("clamps a weight above 1 rather than extrapolating past the target", () => { + const target = 35 / 52; + expect(seedingWinRateFor(eloRate, 95, 60, 52, 3)).toBeCloseTo(target, 6); + expect(seedingWinRateFor(eloRate, 95, 60, 52, 3)).toBe( + seedingWinRateFor(eloRate, 95, 60, 52, 1) + ); + }); + + it("never returns a rate outside (0, 1) for any weight", () => { + for (const weight of [0.25, 0.5, 0.75, 1, 5]) { + for (const [current, remaining] of [[0, 162], [60, 52], [94, 26], [10, 152]]) { + const rate = seedingWinRateFor(eloRate, 95, current, remaining, weight); + expect(rate).toBeGreaterThan(0); + expect(rate).toBeLessThan(1); + } + } }); it("falls back to the Elo rate with no projection", () => { @@ -432,3 +468,39 @@ describe("seedingWinRateFor", () => { ); }); }); + +// ─── projectionForSeeding ───────────────────────────────────────────────────── + +describe("projectionForSeeding", () => { + it("uses the projection when it alone produced the resolved Elo", () => { + expect(projectionForSeeding(95, { sourceEloMethod: "projectedWins" })).toBe(95); + }); + + it("ignores a projection that lost the baseEloPriority race", () => { + // The season resolved its Elo from a hand-entered value. Seeding off the + // projection anyway would ignore it as the Elo source while still letting it + // dictate the standings. + expect(projectionForSeeding(95, { sourceEloMethod: "direct" })).toBeNull(); + }); + + it("ignores a projection that was blended with futures odds", () => { + // The blend lives in the Elo; seeding off the raw projection would discard it + // and run seeding and playoff matchups on different strength scales. + expect(projectionForSeeding(95, { sourceEloMethod: "blend" })).toBeNull(); + expect(projectionForSeeding(95, { sourceEloMethod: "sourceOdds" })).toBeNull(); + }); + + it("ignores a projection on a participant resolved by a fallback", () => { + expect(projectionForSeeding(95, { sourceEloMethod: "averageKnown" })).toBeNull(); + }); + + it("ignores a projection with no method recorded", () => { + expect(projectionForSeeding(95, null)).toBeNull(); + expect(projectionForSeeding(95, undefined)).toBeNull(); + expect(projectionForSeeding(95, {})).toBeNull(); + }); + + it("passes a null projection through", () => { + expect(projectionForSeeding(null, { sourceEloMethod: "projectedWins" })).toBeNull(); + }); +}); diff --git a/app/services/simulations/mlb-simulator.ts b/app/services/simulations/mlb-simulator.ts index 7d49c32..961606d 100644 --- a/app/services/simulations/mlb-simulator.ts +++ b/app/services/simulations/mlb-simulator.ts @@ -35,11 +35,13 @@ * Regular season simulation (seeding): * Each team's base per-game win rate is derived from sourceElo (if set) or * from the hardcoded RDif using SEEDING_RDIF_SCALE ≈ 10 runs/win × 162 games. - * When the team also has a user-entered projected win total, that base rate is - * replaced by the rest-of-season rate that actually reaches the projection: + * When the resolved Elo came from a projected win total and nothing else, that + * base rate is replaced by the rest-of-season rate that reaches the projection: * target = (projectedWins − currentWins) / remainingGames * (see seedingWinRateFor; config `projectedWinsWeight` blends it back toward the - * base rate, and is a no-op pre-season where the two rates coincide). + * base rate). Pre-season the two rates coincide, so this is a no-op then. A + * projection that lost the baseEloPriority race, or that was blended with futures + * odds, is left to the resolved Elo — see projectionForSeeding. * Remaining games = TOTAL_SEASON_GAMES − gamesPlayed are drawn from a * Binomial distribution. This makes playoff seeding respond to both current * standings and user-entered projected wins. @@ -240,6 +242,30 @@ export function eloToRDif(elo: number): number { return (rawWinRateFromElo(elo) - 0.5) * SEEDING_RDIF_SCALE; } +/** + * The projected win total seeding should use, or null to leave seeding on the Elo. + * + * `prepareSimulatorInputsForRun` records which source won the base-Elo race in + * `metadata.sourceEloMethod`, and only `"projectedWins"` means the resolved Elo is + * the projection and nothing else. Every other method has to be left alone: + * + * - `"direct"` — the season's `baseEloPriority` put a hand-entered Elo ahead of + * the projection. Honouring the projection here anyway would ignore it as the + * Elo source while still letting it dictate seeding. + * - `"blend"` / `"sourceOdds"` — futures odds are folded into the Elo at + * `oddsWeight` (0.3 for MLB). Seeding off the raw projection would discard that + * blend and run seeding and playoff matchups on two different strength scales. + * - a fallback — the participant had no usable input of its own. + * + * Exported for unit testing. + */ +export function projectionForSeeding( + projectedWins: number | null, + metadata: Record | null | undefined +): number | null { + return metadata?.sourceEloMethod === "projectedWins" ? projectedWins : null; +} + /** * Per-game win rate to use for a team's remaining regular-season games. * @@ -254,11 +280,18 @@ export function eloToRDif(elo: number): number { * simulation actually land on the projection: a team at 60-50 projected for 95 * needs .673 over its last 52 games, not the .586 its season-long Elo implies. * - * `weight` (config `projectedWinsWeight`, default 1) blends the target back - * toward the Elo-implied rate. At 1 the projection is treated as authoritative; - * lower values hedge it. Note that at weight 1 a team that has already passed its - * projection is clamped to a .01 rest-of-season rate — lower the weight if that - * proves too rigid for in-season use. + * A target outside (0, 1) is proof the projection has gone stale rather than a + * reason to bet everything on it: a 96-40 team projected for 95 would need a + * negative rate, and a 40-70 team projected for 95 would need better than 1.000. + * Both fall back to the Elo rate — clamping them instead would simulate a team to + * stop winning entirely, or to win out. NLL takes the same escape hatch + * (`nll-simulator.ts` clamps its prior at 0 and then uses the Elo rate outright). + * + * `weight` (config `projectedWinsWeight`, default 1) blends the target back toward + * the Elo-implied rate. At 1 the projection is authoritative wherever it is still + * reachable; lower values hedge it; 0 or less ignores it. Values above 1 are + * clamped — this is a blend weight, like inputPolicy.oddsWeight, and above 1 it + * would extrapolate past the target rather than blending toward it. * * Exported for unit testing. */ @@ -271,8 +304,11 @@ export function seedingWinRateFor( ): number { if (projectedWins === null || remainingGames <= 0 || weight <= 0) return eloRate; const target = (projectedWins - currentWins) / remainingGames; - const rate = weight * target + (1 - weight) * eloRate; - return Math.min(0.99, Math.max(0.01, rate)); + if (target <= 0 || target >= 1) return eloRate; + // Clamped here rather than at the call site so the blend cannot be turned into an + // extrapolation by a stray config value, whichever caller supplies it. + const blend = Math.min(1, weight); + return blend * target + (1 - blend) * eloRate; } /** @@ -507,6 +543,7 @@ export class MLBSimulator implements Simulator { const numSimulations = Math.round(positiveConfigNumber(config, "iterations", DEFAULT_NUM_SIMULATIONS)); // configNumber (not positiveConfigNumber) so an explicit 0 — ignore projections, // use the Elo-implied rate — is honored rather than falling back to the default. + // seedingWinRateFor clamps the upper end; the knob is free-form on the Engine card. const projectedWinsWeight = configNumber(config, "projectedWinsWeight", DEFAULT_PROJECTED_WINS_WEIGHT); const db = database(); @@ -530,12 +567,16 @@ export class MLBSimulator implements Simulator { const standings = await getRegularSeasonStandings(sportsSeasonId); const standingsByParticipantId = new Map(standings.map((s) => [s.participantId, s])); - // 3. Load the raw projected win totals. The resolved Elo already encodes the - // projection as a season-long rate, but the raw total is what lets seeding - // spread the *remaining* wins correctly once games have been played. + // 3. Load the raw projected win totals, keeping only those that actually + // produced the resolved Elo. The Elo encodes the projection as a season-long + // rate; the raw total is what lets seeding spread the *remaining* wins + // correctly once games have been played — see projectionForSeeding. const simInputs = await getParticipantSimulatorInputs(sportsSeasonId); const projectedWinsMap = new Map( - simInputs.map((input) => [input.participantId, input.projectedWins]) + simInputs.map((input) => [ + input.participantId, + projectionForSeeding(input.projectedWins, input.metadata), + ]) ); const teams: TeamEntry[] = participantRows.map((r) => { diff --git a/docs/agents/simulators.md b/docs/agents/simulators.md index 175a2ca..5af2515 100644 --- a/docs/agents/simulators.md +++ b/docs/agents/simulators.md @@ -133,6 +133,20 @@ See `seedingWinRateFor` in `mlb-simulator.ts` (config knob `projectedWinsWeight` `nll-simulator.ts` (which additionally decays a preseason prior as the season completes). +Two guards belong on any such rest-of-season rate: + +- **A target outside `(0, 1)` means the projection is stale** — the team has + already met it, or can no longer reach it. Fall back to the Elo rate. Clamping to + a floor or ceiling instead simulates a team to stop winning entirely, or to win + out, and collapses its seeding variance. +- **Only apply a projection that actually produced the resolved Elo.** Check + `metadata.sourceEloMethod === "projectedWins"` (see `projectionForSeeding` in + `mlb-simulator.ts`). Any other method means the Elo represents something else: a + hand-entered Elo that won the `baseEloPriority` race, or a futures blend. Seeding + off the raw projection in those cases makes the projection simultaneously ignored + as the Elo source and authoritative for the standings, and runs seeding and + playoff matchups on two different strength scales. + Missing tail participants must remain blocked unless the season config explicitly chooses an `inputPolicy.missingEloStrategy`: ```json