Address code review: route cloned windows correctly, dedupe canonical query
Follow-up to the QP scoring fix, from self-review: - Cloned qualifying windows carry a bracketTemplateId (copied by cloneSportsSeason) but no playoff matches, so they hit the bracket branch, where processQualifyingBracketEvent derives zero states and writes no QP — the canonical-tie-count fix never ran for them. processQualifyingEvent now routes to the bracket writer only when the window actually has playoff matches; otherwise it falls through to the placement/canonical path like any no-template sibling. - The canonical tournament_results tie span was queried once per window inside the fan-out loop. syncTournamentResults already loads the canonical results, so it now builds the tie-count map once (buildTieCountByPlacement) and passes it to each processQualifyingEvent call; direct callers still query it lazily. - syncTournamentResults no longer re-stamps completedAt on windows that are already complete, so a backfill re-run doesn't churn timestamps. Adds regression tests: a cloned window (template id, no matches) scores R16 losers at 1.5 via the canonical path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017AUcHy9m7aF6axsY6Grpe4
This commit is contained in:
parent
efec504c08
commit
ad238e6bfb
4 changed files with 164 additions and 32 deletions
|
|
@ -205,9 +205,16 @@ describe("Qualifying Points - Fantasy Scoring Integration", () => {
|
|||
options: {
|
||||
tournamentId?: string | null;
|
||||
canonicalPlacements?: number[];
|
||||
bracketTemplateId?: string | null;
|
||||
playoffMatchIds?: string[];
|
||||
} = {}
|
||||
) {
|
||||
const { tournamentId = null, canonicalPlacements = [] } = options;
|
||||
const {
|
||||
tournamentId = null,
|
||||
canonicalPlacements = [],
|
||||
bracketTemplateId = null,
|
||||
playoffMatchIds = [],
|
||||
} = options;
|
||||
const setCalls: unknown[] = [];
|
||||
const updateChain = {
|
||||
set: (values: unknown) => {
|
||||
|
|
@ -225,6 +232,7 @@ describe("Qualifying Points - Fantasy Scoring Integration", () => {
|
|||
sportsSeasonId: "sports-season-1",
|
||||
isQualifyingEvent: true,
|
||||
tournamentId,
|
||||
bracketTemplateId,
|
||||
sportsSeason: { majorsCompleted: 1 },
|
||||
}),
|
||||
},
|
||||
|
|
@ -246,13 +254,23 @@ describe("Qualifying Points - Fantasy Scoring Integration", () => {
|
|||
}),
|
||||
},
|
||||
},
|
||||
// Canonical tournament_results lookup for the structural tie span:
|
||||
// db.select({placement}).from(tournamentResults).where(...) → rows.
|
||||
select: () => ({
|
||||
// Two select shapes flow through the non-bracket path:
|
||||
// • playoff-match existence check (has an `id` projection, uses .limit(1))
|
||||
// • canonical tournament_results tie span (has a `placement` projection).
|
||||
// Discriminate by the projection keys and return a thenable that also
|
||||
// exposes .limit so both call shapes resolve.
|
||||
select: (fields: Record<string, unknown>) => {
|
||||
const isPlayoff = fields && "id" in fields;
|
||||
const rows = isPlayoff
|
||||
? playoffMatchIds.map((id) => ({ id }))
|
||||
: canonicalPlacements.map((placement) => ({ placement }));
|
||||
return {
|
||||
from: () => ({
|
||||
where: async () => canonicalPlacements.map((placement) => ({ placement })),
|
||||
}),
|
||||
where: () =>
|
||||
Object.assign(Promise.resolve(rows), { limit: async () => rows }),
|
||||
}),
|
||||
};
|
||||
},
|
||||
update: () => updateChain,
|
||||
} as any;
|
||||
|
||||
|
|
@ -330,6 +348,48 @@ describe("Qualifying Points - Fantasy Scoring Integration", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it("scores a cloned window (bracketTemplateId but no playoff matches) via the canonical path", async () => {
|
||||
// cloneSportsSeason copies bracketTemplateId to league windows but not the
|
||||
// playoff matches. Such a window must NOT take the bracket branch (which would
|
||||
// derive zero states and write no QP) — it has to fall through to the
|
||||
// placement/canonical path and still split R16 losers to 1.5.
|
||||
const windowRows = [
|
||||
{
|
||||
id: "result-1",
|
||||
seasonParticipantId: "participant-1",
|
||||
placement: 9,
|
||||
qualifyingPointsAwarded: null,
|
||||
scoringEvent: { id: "event-1", sportsSeasonId: "sports-season-1" },
|
||||
},
|
||||
{
|
||||
id: "result-2",
|
||||
seasonParticipantId: "participant-2",
|
||||
placement: 9,
|
||||
qualifyingPointsAwarded: null,
|
||||
scoringEvent: { id: "event-1", sportsSeasonId: "sports-season-1" },
|
||||
},
|
||||
];
|
||||
const { db, setCalls } = makeProcessQPMockDb(windowRows, {
|
||||
tournamentId: "tournament-1",
|
||||
bracketTemplateId: "tennis_128", // copied by clone…
|
||||
playoffMatchIds: [], // …but no matches exist on this window
|
||||
canonicalPlacements: Array.from({ length: 8 }, () => 9),
|
||||
});
|
||||
|
||||
await processQualifyingEvent("event-1", db, { skipNotifications: true });
|
||||
|
||||
expect(setCalls).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({ qualifyingPointsAwarded: "1.50" }),
|
||||
])
|
||||
);
|
||||
expect(setCalls).not.toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({ qualifyingPointsAwarded: "2.00" }),
|
||||
])
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to the live roster count for standalone events with no tournament", async () => {
|
||||
// No canonical field exists for a manual/standalone qualifying event, so the
|
||||
// tie span is the players actually present: 2 players tied at 9th →
|
||||
|
|
|
|||
|
|
@ -847,6 +847,23 @@ export async function processQualifyingBracketEvent(
|
|||
await writeEventResultsQP(eventId, event.sportsSeasonId, entries, db);
|
||||
}
|
||||
|
||||
/**
|
||||
* Count how many results share each placement — the structural tie span used to
|
||||
* split QP across a tied group. Callers pass the FULL canonical field
|
||||
* (tournament_results) so the span reflects the whole tournament, not one window's
|
||||
* roster subset. Null placements (filler / not-participating) are ignored.
|
||||
*/
|
||||
export function buildTieCountByPlacement(
|
||||
results: Array<{ placement: number | null }>
|
||||
): Map<number, number> {
|
||||
const map = new Map<number, number>();
|
||||
for (const r of results) {
|
||||
if (r.placement === null) continue;
|
||||
map.set(r.placement, (map.get(r.placement) ?? 0) + 1);
|
||||
}
|
||||
return map;
|
||||
}
|
||||
|
||||
/**
|
||||
* Process a qualifying event completion and update QP totals.
|
||||
* Ties in QP are handled by sharing placements (averaged points).
|
||||
|
|
@ -854,7 +871,16 @@ export async function processQualifyingBracketEvent(
|
|||
export async function processQualifyingEvent(
|
||||
eventId: string,
|
||||
providedDb?: ReturnType<typeof database>,
|
||||
options: { skipNotifications?: boolean } = {}
|
||||
options: {
|
||||
skipNotifications?: boolean;
|
||||
/**
|
||||
* Pre-computed full-field tie span (placement → count) from the canonical
|
||||
* tournament_results. When the fan-out already loaded the canonical results it
|
||||
* passes this in so we don't re-query per window. Omitted for direct callers,
|
||||
* which fall back to querying it here.
|
||||
*/
|
||||
canonicalTieCountByPlacement?: Map<number, number>;
|
||||
} = {}
|
||||
): Promise<void> {
|
||||
const db = providedDb || database();
|
||||
|
||||
|
|
@ -886,7 +912,23 @@ export async function processQualifyingEvent(
|
|||
qp: r.qualifyingPointsAwarded,
|
||||
}));
|
||||
|
||||
// Route to the bracket writer only when this window actually OWNS a bracket (has
|
||||
// playoff matches). A window can carry a bracketTemplateId with no matches — e.g. a
|
||||
// league window cloned from a bracket season copies the template id but not the
|
||||
// matches (cloneSportsSeason) — and processQualifyingBracketEvent would derive zero
|
||||
// states and write NO QP. Those windows must be scored via the placement/canonical
|
||||
// path below, exactly like a no-template sibling.
|
||||
let hasBracketMatches = false;
|
||||
if (event.bracketTemplateId) {
|
||||
const existing = await db
|
||||
.select({ id: schema.playoffMatches.id })
|
||||
.from(schema.playoffMatches)
|
||||
.where(eq(schema.playoffMatches.scoringEventId, eventId))
|
||||
.limit(1);
|
||||
hasBracketMatches = existing.length > 0;
|
||||
}
|
||||
|
||||
if (hasBracketMatches) {
|
||||
// Bracket-based qualifying event (e.g. CS2 Champions Stage): QP is owned by the
|
||||
// bracket/stage writers, which assign each placement its STRUCTURAL tie span.
|
||||
// Re-derive via processQualifyingBracketEvent and leave the Swiss-exit rows
|
||||
|
|
@ -918,23 +960,20 @@ export async function processQualifyingEvent(
|
|||
// (group.length) under-counts a tied group and over-awards it: tennis R16 losers
|
||||
// all sit at placement 9 with a structural span of 8 → (2+2+2+2+1+1+1+1)/8 = 1.5
|
||||
// QP; a window holding only 4 of them would wrongly split 4 ways → 2 QP. Deriving
|
||||
// the span from canonical results keeps every window/league identical. Standalone
|
||||
// events (no tournamentId) have no canonical field, so fall back to the live count.
|
||||
let canonicalTieCountByPlacement: Map<number, number> | null = null;
|
||||
if (event.tournamentId) {
|
||||
const canonicalResults = await db
|
||||
// the span from canonical results keeps every window/league identical. The fan-out
|
||||
// passes this map in (already loaded once per tournament); direct callers with a
|
||||
// tournament link query it here. Standalone events (no tournamentId, no map) have
|
||||
// no canonical field, so fall back to the live count.
|
||||
const canonicalTieCountByPlacement: Map<number, number> | null =
|
||||
options.canonicalTieCountByPlacement ??
|
||||
(event.tournamentId
|
||||
? buildTieCountByPlacement(
|
||||
await db
|
||||
.select({ placement: schema.tournamentResults.placement })
|
||||
.from(schema.tournamentResults)
|
||||
.where(eq(schema.tournamentResults.tournamentId, event.tournamentId));
|
||||
canonicalTieCountByPlacement = new Map();
|
||||
for (const cr of canonicalResults) {
|
||||
if (cr.placement === null) continue;
|
||||
canonicalTieCountByPlacement.set(
|
||||
cr.placement,
|
||||
(canonicalTieCountByPlacement.get(cr.placement) ?? 0) + 1
|
||||
);
|
||||
}
|
||||
}
|
||||
.where(eq(schema.tournamentResults.tournamentId, event.tournamentId))
|
||||
)
|
||||
: null);
|
||||
|
||||
// Group results by placement to handle ties
|
||||
const placementGroups = new Map<number, typeof results>();
|
||||
|
|
|
|||
|
|
@ -8,6 +8,16 @@ vi.mock("~/database/context", () => ({
|
|||
vi.mock("~/models/scoring-calculator", () => ({
|
||||
processQualifyingEvent: vi.fn(),
|
||||
recalculateAffectedLeagues: vi.fn(),
|
||||
// Pure helper — keep the real implementation so the tie-count map handed to
|
||||
// processQualifyingEvent reflects the mock canonical results.
|
||||
buildTieCountByPlacement: (results: Array<{ placement: number | null }>) => {
|
||||
const map = new Map<number, number>();
|
||||
for (const r of results) {
|
||||
if (r.placement === null) continue;
|
||||
map.set(r.placement, (map.get(r.placement) ?? 0) + 1);
|
||||
}
|
||||
return map;
|
||||
},
|
||||
}));
|
||||
|
||||
vi.mock("~/models/scoring-event", () => ({
|
||||
|
|
@ -360,7 +370,10 @@ describe("syncTournamentResults", () => {
|
|||
expect(c?.qualifyingPointsAwarded).toBe("0");
|
||||
|
||||
expect(processQualifyingEvent).toHaveBeenCalledTimes(1);
|
||||
expect(processQualifyingEvent).toHaveBeenCalledWith("ev-1", db, { skipNotifications: false });
|
||||
expect(processQualifyingEvent).toHaveBeenCalledWith("ev-1", db, {
|
||||
skipNotifications: false,
|
||||
canonicalTieCountByPlacement: expect.any(Map),
|
||||
});
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
|
|
@ -412,8 +425,14 @@ describe("syncTournamentResults", () => {
|
|||
expect(bRows.find((r) => r.seasonParticipantId === "sp-BZ")?.placement).toBe(3);
|
||||
|
||||
expect(processQualifyingEvent).toHaveBeenCalledTimes(2);
|
||||
expect(processQualifyingEvent).toHaveBeenNthCalledWith(1, "ev-A", db, { skipNotifications: false });
|
||||
expect(processQualifyingEvent).toHaveBeenNthCalledWith(2, "ev-B", db, { skipNotifications: false });
|
||||
expect(processQualifyingEvent).toHaveBeenNthCalledWith(1, "ev-A", db, {
|
||||
skipNotifications: false,
|
||||
canonicalTieCountByPlacement: expect.any(Map),
|
||||
});
|
||||
expect(processQualifyingEvent).toHaveBeenNthCalledWith(2, "ev-B", db, {
|
||||
skipNotifications: false,
|
||||
canonicalTieCountByPlacement: expect.any(Map),
|
||||
});
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
|
|
@ -723,7 +742,10 @@ describe("syncTournamentResults", () => {
|
|||
|
||||
expect(report.windowsSynced).toBe(1);
|
||||
expect(processQualifyingEvent).toHaveBeenCalledTimes(1);
|
||||
expect(processQualifyingEvent).toHaveBeenCalledWith("ev-SIBLING", db, { skipNotifications: false });
|
||||
expect(processQualifyingEvent).toHaveBeenCalledWith("ev-SIBLING", db, {
|
||||
skipNotifications: false,
|
||||
canonicalTieCountByPlacement: expect.any(Map),
|
||||
});
|
||||
// No event_results written for the skipped primary window.
|
||||
expect(
|
||||
state.eventResults.some((r) => r.scoringEventId === "ev-PRIMARY")
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import * as schema from "~/database/schema";
|
|||
import {
|
||||
processQualifyingEvent,
|
||||
recalculateAffectedLeagues,
|
||||
buildTieCountByPlacement,
|
||||
} from "~/models/scoring-calculator";
|
||||
import { completeScoringEvent, getScoringEventById } from "~/models/scoring-event";
|
||||
import { getEventResults } from "~/models/event-result";
|
||||
|
|
@ -79,6 +80,11 @@ export async function syncTournamentResults(
|
|||
.from(schema.tournamentResults)
|
||||
.where(eq(schema.tournamentResults.tournamentId, tournamentId));
|
||||
|
||||
// The full-field tie span (placement → count) is a property of the whole
|
||||
// tournament, so compute it once here and hand it to every window's
|
||||
// processQualifyingEvent instead of re-querying canonical results per window.
|
||||
const canonicalTieCountByPlacement = buildTieCountByPlacement(canonicalResults);
|
||||
|
||||
// 2. Load every scoring_event that points at this tournament.
|
||||
const allLinkedEvents = await db
|
||||
.select()
|
||||
|
|
@ -183,12 +189,17 @@ export async function syncTournamentResults(
|
|||
}
|
||||
|
||||
// 3d. Delegate to scoring engine inside the same transaction.
|
||||
await processQualifyingEvent(ev.id, tx, { skipNotifications });
|
||||
await processQualifyingEvent(ev.id, tx, {
|
||||
skipNotifications,
|
||||
canonicalTieCountByPlacement,
|
||||
});
|
||||
|
||||
// 3e. Mark the window event complete (final-results sync only). This is
|
||||
// what makes "score once" actually complete every window — without it,
|
||||
// each sibling stayed "In Progress" and had to be completed by hand.
|
||||
if (markComplete) {
|
||||
// Skip windows already complete so a re-run (e.g. a backfill) doesn't
|
||||
// needlessly re-stamp completedAt.
|
||||
if (markComplete && !ev.isComplete) {
|
||||
await completeScoringEvent(ev.id, tx);
|
||||
}
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue