From 68b2e070e6d7e3ce96049ab4ee695dbc310bfc33 Mon Sep 17 00:00:00 2001 From: Chris Parsons <438676+chrisparsons83@users.noreply.github.com> Date: Thu, 9 Apr 2026 09:51:16 -0400 Subject: [PATCH] fix: infer LLWS bracket side from name when externalId is null (#285) Participants are created with externalId=null by default, causing the LLWS simulator to crash at runtime. Fix in two parts: 1. Name-prefix inference: if externalId is null, participants whose name starts with "US " or equals "US" are assigned to the US side; all others are assigned to Intl. Pools are always randomized when inferred (no pool suffix). 2. Admin UI: add an inline-editable "Group" column to the Manage Participants page so admins can explicitly set externalId for any simulator that uses it (LLWS, and future cases like NHL conferences). Co-authored-by: Claude Sonnet 4.6 --- .../admin.sports-seasons.$id.participants.tsx | 68 +++++++-- .../sports-seasons-participants.test.ts | 138 ++++++++++++++++++ .../__tests__/llws-simulator.test.ts | 71 +++++---- app/services/simulations/llws-simulator.ts | 22 ++- 4 files changed, 257 insertions(+), 42 deletions(-) create mode 100644 app/routes/admin/__tests__/sports-seasons-participants.test.ts diff --git a/app/routes/admin.sports-seasons.$id.participants.tsx b/app/routes/admin.sports-seasons.$id.participants.tsx index 2143e5f..b2d4cb1 100644 --- a/app/routes/admin.sports-seasons.$id.participants.tsx +++ b/app/routes/admin.sports-seasons.$id.participants.tsx @@ -64,6 +64,39 @@ export async function action({ request, params }: Route.ActionArgs) { return { success: true }; } + if (intent === "update-participant") { + const participantId = formData.get("participantId"); + const newName = formData.get("newName"); + const newExternalId = formData.get("newExternalId"); + + if (typeof participantId !== "string") { + return { error: "Invalid participant", intent: "update-participant" }; + } + + if (typeof newName !== "string" || !newName.trim()) { + return { error: "Name is required", intent: "update-participant" }; + } + + const trimmedName = newName.trim(); + const existing = await findParticipantByName(params.id, trimmedName); + if (existing && existing.id !== participantId) { + return { error: `"${trimmedName}" already exists in this season.`, intent: "update-participant" }; + } + + const externalIdValue = + typeof newExternalId === "string" && newExternalId.trim() + ? newExternalId.trim() + : null; + + try { + await updateParticipant(participantId, { name: trimmedName, externalId: externalIdValue }); + return { success: true, intent: "update-participant" }; + } catch (error) { + logger.error("Error updating participant:", error); + return { error: "Failed to update participant. Please try again.", intent: "update-participant" }; + } + } + if (intent === "update-name") { const participantId = formData.get("participantId"); const newName = formData.get("newName"); @@ -171,7 +204,7 @@ export async function action({ request, params }: Route.ActionArgs) { export default function ManageParticipants({ loaderData, actionData }: Route.ComponentProps) { const { sportsSeason, participants } = loaderData; - const [editing, setEditing] = useState<{ id: string; name: string } | null>(null); + const [editing, setEditing] = useState<{ id: string; name: string; externalId: string } | null>(null); const cancelEdit = () => setEditing(null); @@ -179,7 +212,7 @@ export default function ManageParticipants({ loaderData, actionData }: Route.Com const formKey = actionData?.success ? Date.now() : 'static'; useEffect(() => { - if (actionData?.success && actionData?.intent === "update-name") { + if (actionData?.success && (actionData?.intent === "update-name" || actionData?.intent === "update-participant")) { cancelEdit(); } }, [actionData]); @@ -221,13 +254,13 @@ export default function ManageParticipants({ loaderData, actionData }: Route.Com /> - {actionData?.error && !actionData?.count && actionData?.intent !== "update-name" && ( + {actionData?.error && !actionData?.count && actionData?.intent !== "update-name" && actionData?.intent !== "update-participant" && (
{actionData.error}
)} - {actionData?.success && !actionData?.count && ( + {actionData?.success && !actionData?.count && actionData?.intent !== "update-participant" && (
Participant added successfully!
@@ -267,7 +300,7 @@ export default function ManageParticipants({ loaderData, actionData }: Route.Com

- {actionData?.error && actionData?.count === undefined && actionData?.intent !== "update-name" && ( + {actionData?.error && actionData?.count === undefined && actionData?.intent !== "update-name" && actionData?.intent !== "update-participant" && (
{actionData.error}
@@ -311,6 +344,7 @@ export default function ManageParticipants({ loaderData, actionData }: Route.Com Name + Group @@ -322,12 +356,12 @@ export default function ManageParticipants({ loaderData, actionData }: Route.Com
setEditing({ id: participant.id, name: e.target.value })} + onChange={(e) => setEditing({ ...editing, name: e.target.value })} className="h-8" autoFocus onKeyDown={(e) => { if (e.key === "Escape") cancelEdit(); }} /> - {actionData?.error && actionData?.intent === "update-name" && ( + {actionData?.error && (actionData?.intent === "update-name" || actionData?.intent === "update-participant") && (

{actionData.error}

)}
@@ -335,13 +369,29 @@ export default function ManageParticipants({ loaderData, actionData }: Route.Com participant.name )} + + {editing?.id === participant.id ? ( + setEditing({ ...editing, externalId: e.target.value })} + className="h-8 font-mono text-xs" + placeholder="US, Intl, US:A…" + onKeyDown={(e) => { if (e.key === "Escape") cancelEdit(); }} + /> + ) : ( + + {participant.externalId ?? "—"} + + )} + {editing?.id === participant.id ? (
- + + diff --git a/app/routes/admin/__tests__/sports-seasons-participants.test.ts b/app/routes/admin/__tests__/sports-seasons-participants.test.ts new file mode 100644 index 0000000..ebcb843 --- /dev/null +++ b/app/routes/admin/__tests__/sports-seasons-participants.test.ts @@ -0,0 +1,138 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("~/models/participant", () => ({ + findParticipantsBySportsSeasonId: vi.fn(), + findParticipantByName: vi.fn(), + createParticipant: vi.fn(), + deleteParticipant: vi.fn(), + updateParticipant: vi.fn(), +})); + +vi.mock("~/models/sports-season", () => ({ + findSportsSeasonById: vi.fn(), +})); + +vi.mock("~/lib/logger", () => ({ + logger: { error: vi.fn() }, +})); + +import { + findParticipantByName, + updateParticipant, +} from "~/models/participant"; +import { action } from "~/routes/admin.sports-seasons.$id.participants"; + +const mockParams = { id: "season-1" }; + +function makeRequest(data: Record) { + const formData = new FormData(); + for (const [key, value] of Object.entries(data)) { + if (value !== null) formData.set(key, value); + } + return new Request("http://localhost/admin/sports-seasons/season-1/participants", { + method: "POST", + body: formData, + }); +} + +describe("admin participants action — update-participant intent", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("updates both name and externalId when valid", async () => { + (findParticipantByName as ReturnType).mockResolvedValue(null); + (updateParticipant as ReturnType).mockResolvedValue({}); + + const result = await action({ + request: makeRequest({ + intent: "update-participant", + participantId: "p-1", + newName: "US Southeast", + newExternalId: "US", + }), + params: mockParams, + context: {}, + } as Parameters[0]); + + expect(updateParticipant).toHaveBeenCalledWith("p-1", { + name: "US Southeast", + externalId: "US", + }); + expect(result).toMatchObject({ success: true, intent: "update-participant" }); + }); + + it("saves null when newExternalId is blank", async () => { + (findParticipantByName as ReturnType).mockResolvedValue(null); + (updateParticipant as ReturnType).mockResolvedValue({}); + + await action({ + request: makeRequest({ + intent: "update-participant", + participantId: "p-1", + newName: "Some Team", + newExternalId: " ", + }), + params: mockParams, + context: {}, + } as Parameters[0]); + + expect(updateParticipant).toHaveBeenCalledWith("p-1", { + name: "Some Team", + externalId: null, + }); + }); + + it("returns error when name is empty", async () => { + const result = await action({ + request: makeRequest({ + intent: "update-participant", + participantId: "p-1", + newName: " ", + newExternalId: "US", + }), + params: mockParams, + context: {}, + } as Parameters[0]); + + expect(updateParticipant).not.toHaveBeenCalled(); + expect(result).toMatchObject({ error: "Name is required", intent: "update-participant" }); + }); + + it("returns error when name conflicts with another participant", async () => { + (findParticipantByName as ReturnType).mockResolvedValue({ id: "p-2", name: "US Midwest" }); + + const result = await action({ + request: makeRequest({ + intent: "update-participant", + participantId: "p-1", + newName: "US Midwest", + newExternalId: "US", + }), + params: mockParams, + context: {}, + } as Parameters[0]); + + expect(updateParticipant).not.toHaveBeenCalled(); + expect(result).toMatchObject({ error: expect.stringContaining("already exists"), intent: "update-participant" }); + }); + + it("allows saving same name back to the same participant (no false conflict)", async () => { + (findParticipantByName as ReturnType).mockResolvedValue({ id: "p-1", name: "US Midwest" }); + (updateParticipant as ReturnType).mockResolvedValue({}); + + const result = await action({ + request: makeRequest({ + intent: "update-participant", + participantId: "p-1", + newName: "US Midwest", + newExternalId: "US", + }), + params: mockParams, + context: {}, + } as Parameters[0]); + + expect(updateParticipant).toHaveBeenCalled(); + expect(result).toMatchObject({ success: true }); + }); +}); diff --git a/app/services/simulations/__tests__/llws-simulator.test.ts b/app/services/simulations/__tests__/llws-simulator.test.ts index b22fb25..e6ccb94 100644 --- a/app/services/simulations/__tests__/llws-simulator.test.ts +++ b/app/services/simulations/__tests__/llws-simulator.test.ts @@ -41,7 +41,7 @@ describe("LLWSSimulator", () => { }); function setupMockDb( - participants: { id: string; externalId: string }[], + participants: { id: string; name?: string; externalId: string | null }[], evRows: { participantId: string; sourceOdds: number | null }[] ) { mockDb.select.mockImplementation(() => { @@ -53,15 +53,15 @@ describe("LLWSSimulator", () => { function defaultParticipants(mode: "randomized" | "fixed" = "randomized") { if (mode === "fixed") { - const usA = US_IDS.slice(0, 5).map((id) => ({ id, externalId: "US:A" })); - const usB = US_IDS.slice(5).map((id) => ({ id, externalId: "US:B" })); - const intlA = INTL_IDS.slice(0, 5).map((id) => ({ id, externalId: "Intl:A" })); - const intlB = INTL_IDS.slice(5).map((id) => ({ id, externalId: "Intl:B" })); + const usA = US_IDS.slice(0, 5).map((id) => ({ id, name: `US Team ${id}`, externalId: "US:A" })); + const usB = US_IDS.slice(5).map((id) => ({ id, name: `US Team ${id}`, externalId: "US:B" })); + const intlA = INTL_IDS.slice(0, 5).map((id) => ({ id, name: `Team ${id}`, externalId: "Intl:A" })); + const intlB = INTL_IDS.slice(5).map((id) => ({ id, name: `Team ${id}`, externalId: "Intl:B" })); return [...usA, ...usB, ...intlA, ...intlB]; } return [ - ...US_IDS.map((id) => ({ id, externalId: "US" })), - ...INTL_IDS.map((id) => ({ id, externalId: "Intl" })), + ...US_IDS.map((id) => ({ id, name: `US Team ${id}`, externalId: "US" })), + ...INTL_IDS.map((id) => ({ id, name: `Team ${id}`, externalId: "Intl" })), ]; } @@ -192,9 +192,9 @@ describe("LLWSSimulator", () => { it("mixed mode: US fixed pools, Intl randomized", async () => { const participants = [ - ...US_IDS.slice(0, 5).map((id) => ({ id, externalId: "US:A" })), - ...US_IDS.slice(5).map((id) => ({ id, externalId: "US:B" })), - ...INTL_IDS.map((id) => ({ id, externalId: "Intl" })), + ...US_IDS.slice(0, 5).map((id) => ({ id, name: `US Team ${id}`, externalId: "US:A" })), + ...US_IDS.slice(5).map((id) => ({ id, name: `US Team ${id}`, externalId: "US:B" })), + ...INTL_IDS.map((id) => ({ id, name: `Team ${id}`, externalId: "Intl" })), ]; setupMockDb(participants, makeEvRows(ALL_IDS)); const results = await new LLWSSimulator().simulate("season-1"); @@ -211,37 +211,52 @@ describe("LLWSSimulator", () => { const nineteen = [...US_IDS, ...INTL_IDS.slice(0, 9)]; const participants = nineteen.map((id, i) => ({ id, + name: i < 10 ? `US Team ${id}` : `Team ${id}`, externalId: i < 10 ? "US" : "Intl", })); setupMockDb(participants, makeEvRows(nineteen)); await expect(new LLWSSimulator().simulate("season-1")).rejects.toThrow(/exactly 20/); }); - it("throws when a participant has a missing externalId", async () => { + it("infers US side from name prefix when externalId is null", async () => { const participants = [ - ...US_IDS.map((id) => ({ id, externalId: "US" })), - ...INTL_IDS.slice(0, 9).map((id) => ({ id, externalId: "Intl" })), - { id: "intl-10", externalId: null as unknown as string }, // missing + ...US_IDS.map((id) => ({ id, name: `US Team ${id}`, externalId: null })), + ...INTL_IDS.map((id) => ({ id, name: `Japan ${id}`, externalId: null })), ]; setupMockDb(participants, makeEvRows(ALL_IDS)); - await expect(new LLWSSimulator().simulate("season-1")).rejects.toThrow(/invalid or missing externalId/); + const results = await new LLWSSimulator().simulate("season-1"); + expect(results).toHaveLength(20); + const total = results.reduce((s, r) => s + r.probabilities.probFirst, 0); + expect(total).toBeCloseTo(1.0, 1); }); - it("throws when a participant has an unrecognized externalId", async () => { + it("infers US side from exact name 'US' when externalId is null", async () => { const participants = [ - ...US_IDS.map((id) => ({ id, externalId: "US" })), - ...INTL_IDS.slice(0, 9).map((id) => ({ id, externalId: "Intl" })), - { id: "intl-10", externalId: "CANADA" }, // unrecognized + ...US_IDS.map((id) => ({ id, name: "US", externalId: null })), + ...INTL_IDS.map((id) => ({ id, name: `Team ${id}`, externalId: null })), ]; setupMockDb(participants, makeEvRows(ALL_IDS)); - await expect(new LLWSSimulator().simulate("season-1")).rejects.toThrow(/invalid or missing externalId/); + const results = await new LLWSSimulator().simulate("season-1"); + expect(results).toHaveLength(20); + const total = results.reduce((s, r) => s + r.probabilities.probFirst, 0); + expect(total).toBeCloseTo(1.0, 1); + }); + + it("throws when a participant has an unrecognized non-null externalId", async () => { + const participants = [ + ...US_IDS.map((id) => ({ id, name: `US Team ${id}`, externalId: "US" })), + ...INTL_IDS.slice(0, 9).map((id) => ({ id, name: `Team ${id}`, externalId: "Intl" })), + { id: "intl-10", name: "Team intl-10", externalId: "CANADA" }, // unrecognized + ]; + setupMockDb(participants, makeEvRows(ALL_IDS)); + await expect(new LLWSSimulator().simulate("season-1")).rejects.toThrow(/invalid externalId/); }); it("throws when US team count is not 10", async () => { // 11 US teams, 9 International const participants = [ - ...Array.from({ length: 11 }, (_, i) => ({ id: `us-${i + 1}`, externalId: "US" })), - ...Array.from({ length: 9 }, (_, i) => ({ id: `intl-${i + 1}`, externalId: "Intl" })), + ...Array.from({ length: 11 }, (_, i) => ({ id: `us-${i + 1}`, name: `US Team ${i + 1}`, externalId: "US" })), + ...Array.from({ length: 9 }, (_, i) => ({ id: `intl-${i + 1}`, name: `Team ${i + 1}`, externalId: "Intl" })), ]; setupMockDb(participants, makeEvRows(ALL_IDS)); await expect(new LLWSSimulator().simulate("season-1")).rejects.toThrow(/10 US teams/); @@ -250,9 +265,9 @@ describe("LLWSSimulator", () => { it("throws when fixed pools have unequal A/B split", async () => { const participants = [ // 6 in Pool A, 4 in Pool B - ...US_IDS.slice(0, 6).map((id) => ({ id, externalId: "US:A" })), - ...US_IDS.slice(6).map((id) => ({ id, externalId: "US:B" })), - ...INTL_IDS.map((id) => ({ id, externalId: "Intl" })), + ...US_IDS.slice(0, 6).map((id) => ({ id, name: `US Team ${id}`, externalId: "US:A" })), + ...US_IDS.slice(6).map((id) => ({ id, name: `US Team ${id}`, externalId: "US:B" })), + ...INTL_IDS.map((id) => ({ id, name: `Team ${id}`, externalId: "Intl" })), ]; setupMockDb(participants, makeEvRows(ALL_IDS)); await expect(new LLWSSimulator().simulate("season-1")).rejects.toThrow(/exactly 5 teams each/); @@ -261,9 +276,9 @@ describe("LLWSSimulator", () => { it("throws when US externalIds mix pool suffixes and bare side", async () => { const participants = [ // Some US:A, some "US" (no pool suffix) → mixed - ...US_IDS.slice(0, 5).map((id) => ({ id, externalId: "US:A" })), - ...US_IDS.slice(5).map((id) => ({ id, externalId: "US" })), // no pool - ...INTL_IDS.map((id) => ({ id, externalId: "Intl" })), + ...US_IDS.slice(0, 5).map((id) => ({ id, name: `US Team ${id}`, externalId: "US:A" })), + ...US_IDS.slice(5).map((id) => ({ id, name: `US Team ${id}`, externalId: "US" })), // no pool + ...INTL_IDS.map((id) => ({ id, name: `Team ${id}`, externalId: "Intl" })), ]; setupMockDb(participants, makeEvRows(ALL_IDS)); await expect(new LLWSSimulator().simulate("season-1")).rejects.toThrow(/mixed externalId formats/); diff --git a/app/services/simulations/llws-simulator.ts b/app/services/simulations/llws-simulator.ts index 212f811..1ac4b01 100644 --- a/app/services/simulations/llws-simulator.ts +++ b/app/services/simulations/llws-simulator.ts @@ -49,8 +49,8 @@ * Admin setup: * 1. Create a Sport with simulatorType = "llws_bracket" * 2. Create a Sports Season and add exactly 20 participants (10 US, 10 International) - * 3. Set externalId on each participant: - * Pre-draw: "US" or "Intl" + * 3. Set externalId on each participant via Admin → Manage Participants (optional if names follow the convention): + * Pre-draw: "US" or "Intl" (or leave null — names starting with "US " infer US, all others infer Intl) * Post-draw: "US:A", "US:B", "Intl:A", or "Intl:B" * 4. Enter championship futures odds via Admin → Futures Odds (sourceOdds) * 5. Run simulation via Admin → Simulate @@ -215,6 +215,17 @@ function parseExternalId(raw: string | null): { side: Side; pool: PoolSuffix } | return null; } +/** + * Infer an externalId from a participant name when none is stored. + * Teams whose name is exactly "US" or starts with "US " (case-insensitive) + * are assigned to the US side; all others are assigned to Intl. + * The inferred value never has a pool suffix, so pools will be randomized. + */ +function inferExternalIdFromName(name: string): string { + const upper = name.trim().toUpperCase(); + return upper === "US" || upper.startsWith("US ") ? "US" : "Intl"; +} + /** * Determine whether pool assignments should be randomized for one side. * - If ALL teams on the side have a pool suffix → fixed pools (returns false). @@ -253,7 +264,7 @@ export class LLWSSimulator implements Simulator { // 1. Load all participants. const participants = await db - .select({ id: schema.participants.id, externalId: schema.participants.externalId }) + .select({ id: schema.participants.id, name: schema.participants.name, externalId: schema.participants.externalId }) .from(schema.participants) .where(eq(schema.participants.sportsSeasonId, sportsSeasonId)); @@ -292,10 +303,11 @@ export class LLWSSimulator implements Simulator { // 4. Parse externalId for each participant to determine side and fixed pool. const teams: Team[] = []; for (const p of participants) { - const parsed = parseExternalId(p.externalId); + const raw = p.externalId ?? inferExternalIdFromName(p.name); + const parsed = parseExternalId(raw); if (!parsed) { throw new Error( - `Participant ${p.id} has invalid or missing externalId "${p.externalId}". ` + + `Participant ${p.id} has invalid externalId "${p.externalId}". ` + `Expected: "US", "Intl", "US:A", "US:B", "Intl:A", or "Intl:B".` ); }