diff --git a/app/lib/__tests__/bracket-layout.test.ts b/app/lib/__tests__/bracket-layout.test.ts index f1bd782..70968b0 100644 --- a/app/lib/__tests__/bracket-layout.test.ts +++ b/app/lib/__tests__/bracket-layout.test.ts @@ -12,6 +12,7 @@ import { LLWS_20, SIMPLE_16, NFL_14, + BRACKET_TEMPLATES, getBracketTemplate, type BracketTemplate, type ConferenceGroup, @@ -28,6 +29,10 @@ import { GAME_TO_MATCH, MATCH_TO_GAME } from "~/test/fixtures/llws-bracket"; interface TestMatch { round: string; matchNumber: number; + /** Only the fallback reads these, to trace edges through an unrecognised shape. */ + winnerId?: string | null; + participant1Id?: string | null; + participant2Id?: string | null; } /** Every match a template defines, as the renderer would receive them. */ @@ -311,6 +316,157 @@ describe("computeGroupLayout — standard brackets are unchanged", () => { }); }); +describe("buildFeederMap — templates with routing of their own", () => { + // The halving rule describes advanceWinnerTemplate, not every bracket. Inventing it + // where it doesn't hold draws confident, wrong connectors and mislabels slots, which + // is worse than drawing nothing. + + it("follows feedsInto rather than the order rounds are listed in", () => { + // AFL's Wildcard Round feeds the Elimination Finals, skipping the round printed + // next to it, so array order would fabricate the whole chain. + const afl = BRACKET_TEMPLATES.afl_10; + const feeders = buildFeederMap(afl); + const fed = [...feeders.entries()].filter(([, pair]) => + pair.some((s) => s.kind === "match") + ); + // Only Preliminary Finals → Grand Final actually halves. + expect(fed.map(([key]) => key)).toEqual(["Grand Final#1"]); + }); + + it("leaves a bye round's slots seeded rather than inventing feeds", () => { + // CFP's First Round (4) feeds the Quarterfinals (4) — the top seeds have byes. + const feeders = buildFeederMap(BRACKET_TEMPLATES.cfp_12); + expect(feeders.get(matchKey("Quarterfinals", 1))).toEqual([ + { kind: "seed" }, + { kind: "seed" }, + ]); + }); + + it("leaves the First Four out of the Round of 64", () => { + // advanceFirstFourWinner puts each winner in a specific seed slot, not games 1-2. + const feeders = buildFeederMap(BRACKET_TEMPLATES.ncaa_68); + expect(feeders.get(matchKey("Round of 64", 1))).toEqual([ + { kind: "seed" }, + { kind: "seed" }, + ]); + }); + + it("leaves the NBA play-in alone, where a loser feeds forward", () => { + // Play-In Round 2 pairs the 7v8 loser with the 9v10 winner, so the round sizes + // halve but the winners-only rule still doesn't describe it. + const feeders = buildFeederMap(BRACKET_TEMPLATES.nba_20); + expect(feeders.get(matchKey("Play-In Round 2", 1))).toEqual([ + { kind: "seed" }, + { kind: "seed" }, + ]); + }); + + it("does not route the FIFA final through the third place game", () => { + // Third Place Game sits between Semifinals and Finals in round order, so array + // order made it the Finals' feeder and left the Finals' second slot empty. + const feeders = buildFeederMap(BRACKET_TEMPLATES.fifa_48); + expect(feeders.get(matchKey("Finals", 1))).toEqual([ + { kind: "match", ref: { round: "Semifinals", matchNumber: 1 }, result: "winner" }, + { kind: "match", ref: { round: "Semifinals", matchNumber: 2 }, result: "winner" }, + ]); + }); +}); + +describe("computeGroupLayout — every template still draws connectors", () => { + /** Lay a whole template out the way BracketTreeView would. */ + function layOut(template: BracketTemplate) { + const byRound = allMatches(template); + const order = template.rounds.map((r) => r.name); + // BracketTreeView renders a third place game outside the tree. + const rounds = order.filter((r) => r !== "Third Place Game"); + return computeGroupLayout(rounds, byRound, buildFeederMap(template), order); + } + + // A gutter joining a column to one exactly half its size is a plain bracket join and + // must always be drawn. Where the sizes don't halve — a bye round, a play-in, the + // First Four — the routing is bespoke and nothing is drawn until the games decide it, + // which is what these brackets did before. + it.each(Object.keys(BRACKET_TEMPLATES).filter((id) => id !== "llws_20"))( + "%s draws every gutter that halves", + (id) => { + const layout = layOut(BRACKET_TEMPLATES[id]); + const gutters = new Set(layout.edges.map((e) => e.fromColumn)); + let halvingGutters = 0; + for (let ci = 0; ci < layout.columns.length - 1; ci++) { + const from = layout.columns[ci].matches.length; + const to = layout.columns[ci + 1].matches.length; + if (from !== to * 2) continue; + halvingGutters += 1; + expect(gutters).toContain(ci); + } + // Every template has at least one, so a template that lost all its lines fails. + expect(halvingGutters).toBeGreaterThan(0); + } + ); + + it("keeps the FIFA bracket a single tree once the third place game is set aside", () => { + const layout = layOut(BRACKET_TEMPLATES.fifa_48); + expect(layout.columns.map((c) => c.label)).toEqual([ + "Round of 32", + "Round of 16", + "Quarterfinals", + "Semifinals", + "Finals", + ]); + expect(layout.edges).toHaveLength(30); + }); + + // llws_20 is excluded above because both sides in one group is genuinely not a tree; + // it renders per side, which the tests further up cover. +}); + +describe("computeGroupLayout — fallback keeps the old connectors", () => { + const rounds = ["Quarterfinals", "Semifinals", "Finals"]; + const byRound = new Map([ + ["Quarterfinals", [1, 2, 3, 4].map((n) => ({ round: "Quarterfinals", matchNumber: n }))], + ["Semifinals", [1, 2].map((n) => ({ round: "Semifinals", matchNumber: n }))], + ["Finals", [{ round: "Finals", matchNumber: 1 }]], + ]); + + it("infers halving edges when there is no feeder map at all", () => { + // A bracket with no template id, which SportSeasonDisplay renders. + const layout = computeGroupLayout(rounds, byRound, new Map(), rounds); + expect(layout.edges).toHaveLength(6); + // Quarterfinals 1 and 2 both join Semifinal 1. + const intoFirstSemi = layout.edges.filter((e) => e.toCenter === 1); + expect(intoFirstSemi.map((e) => e.fromCenter)).toEqual([0.5, 1.5]); + }); + + it("traces played winners when the shape is not a halving", () => { + const irregular = new Map([ + [ + "Wildcard", + [ + { round: "Wildcard", matchNumber: 1, winnerId: "a" }, + { round: "Wildcard", matchNumber: 2, winnerId: "b" }, + ], + ], + [ + "Semifinals", + [ + { round: "Semifinals", matchNumber: 1, participant1Id: "seeded", participant2Id: "b" }, + { round: "Semifinals", matchNumber: 2, participant1Id: "seeded2", participant2Id: "a" }, + ], + ], + ]); + const layout = computeGroupLayout( + ["Wildcard", "Semifinals"], + irregular, + new Map(), + ["Wildcard", "Semifinals"] + ); + // b won Wildcard 2 (centre 1.5) and plays Semifinal 1 (centre 0.5) — a crossing + // edge that only the actual result can reveal. + expect(layout.edges).toContainEqual({ fromColumn: 0, fromCenter: 1.5, toCenter: 0.5 }); + expect(layout.edges).toContainEqual({ fromColumn: 0, fromCenter: 0.5, toCenter: 1.5 }); + }); +}); + describe("describeSlotSource", () => { const feeders = buildFeederMap(LLWS_20); const sourcesFor = (game: number): [SlotSource, SlotSource] => { diff --git a/app/lib/bracket-layout.ts b/app/lib/bracket-layout.ts index 49f5a09..e63027e 100644 --- a/app/lib/bracket-layout.ts +++ b/app/lib/bracket-layout.ts @@ -47,6 +47,16 @@ export type FeederMap = Map; const SEED: SlotSource = { kind: "seed" }; +/** + * `template.id:roundName` for transitions routed by a dedicated advancement function + * rather than advanceWinnerTemplate's ceil(n/2) rule, and whose round sizes happen to + * halve so the check in buildFeederMap can't rule them out on shape alone. + * + * The NBA play-in is the case: Play-In Round 2 pairs the 7v8 *loser* with the 9v10 + * winner (advanceNBAPlayInWinner), which no winners-only halving describes. + */ +const BESPOKE_TRANSITIONS = new Set(["nba_20:Play-In Round 1"]); + export function matchKey(round: string, matchNumber: number): string { return `${round}#${matchNumber}`; } @@ -94,22 +104,34 @@ export function buildFeederMap(template: BracketTemplate | undefined): FeederMap return feeders; } - for (let ri = 1; ri < template.rounds.length; ri++) { - const round = template.rounds[ri]; - const prev = template.rounds[ri - 1]; + // Follow each round's declared `feedsInto` rather than array order — AFL's Wildcard + // Round feeds the Elimination Finals, skipping the round printed next to it. + for (const prev of template.rounds) { + if (!prev.feedsInto) continue; + const round = template.rounds.find((r) => r.name === prev.feedsInto); + if (!round) continue; + + // advanceWinnerTemplate sends match n to ceil(n/2) in the next round, slot by + // parity. That describes the bracket only where the round halves exactly; a + // play-in, a bye round, or a First Four routes by rules of its own, and inventing + // a halving there would draw connectors and slot labels that are simply wrong. + // Leaving those edges out drops the group to computeGroupLayout's fallback, which + // is the geometry these brackets already had. + if (prev.matchCount !== round.matchCount * 2) continue; + if (BESPOKE_TRANSITIONS.has(`${template.id}:${prev.name}`)) continue; + for (let n = 1; n <= round.matchCount; n++) { const pair = slots(matchKey(round.name, n)); - for (const [slotIdx, source] of [ - [0, 2 * n - 1], - [1, 2 * n], - ] as const) { - if (source > prev.matchCount) continue; - pair[slotIdx] = { - kind: "match", - ref: { round: prev.name, matchNumber: source }, - result: "winner", - }; - } + pair[0] = { + kind: "match", + ref: { round: prev.name, matchNumber: 2 * n - 1 }, + result: "winner", + }; + pair[1] = { + kind: "match", + ref: { round: prev.name, matchNumber: 2 * n }, + result: "winner", + }; } } @@ -195,6 +217,10 @@ export interface BracketLayout { interface PositionedMatch { round: string; matchNumber: number; + /** Only read by the fallback, to trace edges through an unrecognised shape. */ + winnerId?: string | null; + participant1Id?: string | null; + participant2Id?: string | null; } /** @@ -328,8 +354,10 @@ export function computeGroupLayout( } /** - * The previous behaviour: one column per round, matches spread evenly, no edges. - * Used when a group's shape can't be resolved into a single tree. + * The previous behaviour, kept for groups whose shape can't be resolved into a single + * tree: one column per round, matches spread evenly over it, and edges inferred from the + * round sizes. Brackets with bespoke routing (AFL, CFP byes, a bracket with no template) + * land here, so it has to keep drawing what they drew before rather than nothing. */ function fallbackLayout( visibleRounds: string[], @@ -339,13 +367,54 @@ function fallbackLayout( ...visibleRounds.map((r) => matchesByRound.get(r)?.length ?? 0), 1 ); + const centersFor = (matches: M[]) => { + const span = leafCount / Math.max(matches.length, 1); + return matches.map((_, i) => (i + 0.5) * span); + }; + const columns = visibleRounds.map((round) => { const matches = matchesByRound.get(round) ?? []; - const span = leafCount / Math.max(matches.length, 1); + const centers = centersFor(matches); return { label: round, - matches: matches.map((match, i) => ({ match, center: (i + 0.5) * span })), + matches: matches.map((match, i) => ({ match, center: centers[i] })), }; }); - return { columns, leafCount, edges: [] }; + + const edges: BracketLayout["edges"] = []; + for (let ci = 0; ci < columns.length - 1; ci++) { + const from = columns[ci].matches; + const to = columns[ci + 1].matches; + + if (to.length === Math.ceil(from.length / 2) && from.length > 1) { + // A halving: matches 2k and 2k+1 feed match k. + for (let k = 0; k < to.length; k++) { + for (const idx of [2 * k, 2 * k + 1]) { + if (idx >= from.length) continue; + edges.push({ + fromColumn: ci, + fromCenter: from[idx].center, + toCenter: to[k].center, + }); + } + } + continue; + } + + // Otherwise the only thing that can be known is where a winner actually went, so + // nothing is drawn until the games are played. + const winnerToCenter = new Map(); + for (const { match, center } of from) { + if (match.winnerId) winnerToCenter.set(match.winnerId, center); + } + for (const { match, center } of to) { + for (const id of [match.participant1Id, match.participant2Id]) { + const fromCenter = id ? winnerToCenter.get(id) : undefined; + if (fromCenter === undefined) continue; + edges.push({ fromColumn: ci, fromCenter, toCenter: center }); + } + } + } + + return { columns, leafCount, edges }; } diff --git a/app/routes/__tests__/admin.sports-seasons.bracket.clear.test.ts b/app/routes/__tests__/admin.sports-seasons.bracket.clear.test.ts index 4fb255a..c4e8e12 100644 --- a/app/routes/__tests__/admin.sports-seasons.bracket.clear.test.ts +++ b/app/routes/__tests__/admin.sports-seasons.bracket.clear.test.ts @@ -68,7 +68,7 @@ describe("clear-bracket", () => { ); }); - it("deletes the matches and the placements derived from them", async () => { + it("deletes the matches", async () => { vi.mocked(findPlayoffMatchesByEventId).mockResolvedValue([ match(false), match(false), @@ -78,7 +78,20 @@ describe("clear-bracket", () => { expect(result.success).toContain("2 match(es) removed"); expect(deletePlayoffMatchesByEventId).toHaveBeenCalledWith("event-1"); - expect(deleteParticipantResultsBySportsSeasonId).toHaveBeenCalledWith("season-1"); + }); + + it("leaves placements alone — they belong to the whole season, not this event", async () => { + // seasonParticipantResults is keyed by sports season, so deleting here would wipe + // every other event's placements with nothing to rebuild them. Reprocess Bracket is + // the tool that rebuilds them correctly. + vi.mocked(findPlayoffMatchesByEventId).mockResolvedValue([ + match(true), + ] as unknown as Awaited>); + + const result = await run(clearRequest("true")); + + expect(deleteParticipantResultsBySportsSeasonId).not.toHaveBeenCalled(); + expect(result.success).toContain("Reprocess Bracket"); }); it("refuses to discard completed matches without confirmation", async () => { @@ -91,7 +104,6 @@ describe("clear-bracket", () => { expect(result.error).toContain("1 completed match(es)"); expect(deletePlayoffMatchesByEventId).not.toHaveBeenCalled(); - expect(deleteParticipantResultsBySportsSeasonId).not.toHaveBeenCalled(); }); it("discards completed matches once confirmed", async () => { 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 a91a3db..0ffcc73 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 @@ -310,13 +310,20 @@ export async function action({ request, params }: Route.ActionArgs) { } await deletePlayoffMatchesByEventId(params.eventId); - // Placements were derived from the matches just deleted; leaving them behind would - // keep stale points on the standings. - await deleteParticipantResultsBySportsSeasonId(event.sportsSeasonId); - await recalculateAffectedLeagues(event.sportsSeasonId, database(), { skipDiscord: true }); + + // Placements are deliberately left alone. seasonParticipantResults is keyed by + // sports season, not by event, so a season-wide delete here would wipe the + // placements of every other event in the season with nothing to rebuild them — + // and on a finalized qualifying season that means permanently zeroed standings. + // Reprocess Bracket already rebuilds placements correctly, qualifying path + // included, so point the admin at it once the new bracket is in place. + const note = + completed > 0 + ? " Run Reprocess Bracket after rebuilding to clear the placements those results produced." + : ""; return { - success: `Bracket cleared (${existing.length} match(es) removed). Set it up again below.`, + success: `Bracket cleared (${existing.length} match(es) removed). Set it up again below.${note}`, }; } catch (error) { logger.error("Error clearing bracket:", error); diff --git a/app/routes/admin.sports-seasons.$id.events.$eventId.bracket.tsx b/app/routes/admin.sports-seasons.$id.events.$eventId.bracket.tsx index 7be75fa..6442f20 100644 --- a/app/routes/admin.sports-seasons.$id.events.$eventId.bracket.tsx +++ b/app/routes/admin.sports-seasons.$id.events.$eventId.bracket.tsx @@ -622,17 +622,19 @@ export default function EventBracket({ Clear Bracket Delete every match in this bracket so it can be set up again from - scratch. Use this when the wrong participants were seeded. This also - clears the season's placements and the points derived from them. + scratch. Use this when the wrong participants were seeded. Placements + are left alone — run Reprocess Bracket after rebuilding to clear any + that the discarded results produced.
{ if ( !confirm( - `Delete all ${matches.length} match(es) in this bracket? Recorded results and placements will be lost.` + `Delete all ${matches.length} match(es) in this bracket? Recorded results will be lost.` ) ) { e.preventDefault(); @@ -640,7 +642,23 @@ export default function EventBracket({ }} > - + {/* The server refuses to discard completed matches unless this is + checked. Sending it unconditionally from a hidden field would make + that guard unreachable, including for a submit without JS. */} + {matches.some((m: { isComplete: boolean }) => m.isComplete) && ( +
+ + +
+ )}