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 <noreply@anthropic.com>
This commit is contained in:
Chris Parsons 2026-04-09 09:51:16 -04:00 committed by GitHub
parent a6bb1330e6
commit 68b2e070e6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 257 additions and 42 deletions

View file

@ -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
/>
</div>
{actionData?.error && !actionData?.count && actionData?.intent !== "update-name" && (
{actionData?.error && !actionData?.count && actionData?.intent !== "update-name" && actionData?.intent !== "update-participant" && (
<div className="bg-destructive/15 text-destructive px-4 py-3 rounded-md text-sm">
{actionData.error}
</div>
)}
{actionData?.success && !actionData?.count && (
{actionData?.success && !actionData?.count && actionData?.intent !== "update-participant" && (
<div className="bg-emerald-500/15 text-emerald-400 px-4 py-3 rounded-md text-sm">
Participant added successfully!
</div>
@ -267,7 +300,7 @@ export default function ManageParticipants({ loaderData, actionData }: Route.Com
</p>
</div>
{actionData?.error && actionData?.count === undefined && actionData?.intent !== "update-name" && (
{actionData?.error && actionData?.count === undefined && actionData?.intent !== "update-name" && actionData?.intent !== "update-participant" && (
<div className="bg-destructive/15 text-destructive px-4 py-3 rounded-md text-sm">
{actionData.error}
</div>
@ -311,6 +344,7 @@ export default function ManageParticipants({ loaderData, actionData }: Route.Com
<TableHeader>
<TableRow>
<TableHead>Name</TableHead>
<TableHead className="w-[120px]">Group</TableHead>
<TableHead className="w-[100px]" />
</TableRow>
</TableHeader>
@ -322,12 +356,12 @@ export default function ManageParticipants({ loaderData, actionData }: Route.Com
<div className="space-y-1">
<Input
value={editing.name}
onChange={(e) => 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") && (
<p className="text-xs text-destructive">{actionData.error}</p>
)}
</div>
@ -335,13 +369,29 @@ export default function ManageParticipants({ loaderData, actionData }: Route.Com
participant.name
)}
</TableCell>
<TableCell>
{editing?.id === participant.id ? (
<Input
value={editing.externalId}
onChange={(e) => 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(); }}
/>
) : (
<span className="text-xs font-mono text-muted-foreground">
{participant.externalId ?? "—"}
</span>
)}
</TableCell>
<TableCell>
{editing?.id === participant.id ? (
<div className="flex gap-1">
<Form method="post">
<input type="hidden" name="intent" value="update-name" />
<input type="hidden" name="intent" value="update-participant" />
<input type="hidden" name="participantId" value={participant.id} />
<input type="hidden" name="newName" value={editing.name} />
<input type="hidden" name="newExternalId" value={editing.externalId} />
<Button
type="submit"
variant="ghost"
@ -368,7 +418,7 @@ export default function ManageParticipants({ loaderData, actionData }: Route.Com
variant="ghost"
size="sm"
className="h-8 w-8 p-0"
onClick={() => setEditing({ id: participant.id, name: participant.name })}
onClick={() => setEditing({ id: participant.id, name: participant.name, externalId: participant.externalId ?? "" })}
>
<Pencil className="h-4 w-4" />
</Button>

View file

@ -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<string, string | null>) {
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<typeof vi.fn>).mockResolvedValue(null);
(updateParticipant as ReturnType<typeof vi.fn>).mockResolvedValue({});
const result = await action({
request: makeRequest({
intent: "update-participant",
participantId: "p-1",
newName: "US Southeast",
newExternalId: "US",
}),
params: mockParams,
context: {},
} as Parameters<typeof action>[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<typeof vi.fn>).mockResolvedValue(null);
(updateParticipant as ReturnType<typeof vi.fn>).mockResolvedValue({});
await action({
request: makeRequest({
intent: "update-participant",
participantId: "p-1",
newName: "Some Team",
newExternalId: " ",
}),
params: mockParams,
context: {},
} as Parameters<typeof action>[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<typeof action>[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<typeof vi.fn>).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<typeof action>[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<typeof vi.fn>).mockResolvedValue({ id: "p-1", name: "US Midwest" });
(updateParticipant as ReturnType<typeof vi.fn>).mockResolvedValue({});
const result = await action({
request: makeRequest({
intent: "update-participant",
participantId: "p-1",
newName: "US Midwest",
newExternalId: "US",
}),
params: mockParams,
context: {},
} as Parameters<typeof action>[0]);
expect(updateParticipant).toHaveBeenCalled();
expect(result).toMatchObject({ success: true });
});
});

View file

@ -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/);

View file

@ -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".`
);
}