Address code review feedback on MLS standings + simulator
1. Update mls-simulator.ts module comment to document the new step-3 externalId fallback in the conference resolution order 2. Tighten normalizeConference to exact-match "Eastern"/"Western Conference" instead of broad substring, preventing false matches on names like "Northeast" 3. Pre-parse statsMap for each entry before the sort so it isn't rebuilt O(n log n) times during comparison 4. Document the externalId name-matching tradeoff on parseConferenceFromExternalId 5. Try ESPN's gamesPlayed stat before falling back to wins+losses+ties sum; export parseConferenceFromExternalId for direct testing 6. Add tests: normalizeConference passthrough, winPct=0 at preseason, and parseConferenceFromExternalId (case-insensitivity, null/undefined, numeric ESPN IDs, unrecognized strings) https://claude.ai/code/session_01WhzXHpv6taXdHzhgvnv83u
This commit is contained in:
Claude
parent
3474288fc3
commit
9ae5bd4cf6
4 changed files with 127 additions and 14 deletions
|
|
@ -5,6 +5,7 @@ import {
|
|||
simulateMlsBestOfThree,
|
||||
simulateMlsRegularSeason,
|
||||
simulateMlsPlayoffs,
|
||||
parseConferenceFromExternalId,
|
||||
type MlsConferenceSeed,
|
||||
type MlsTeamEntry,
|
||||
} from "../mls-simulator";
|
||||
|
|
@ -361,3 +362,33 @@ describe("mls_bracket simulator sync", () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ─── parseConferenceFromExternalId ────────────────────────────────────────────
|
||||
|
||||
describe("parseConferenceFromExternalId", () => {
|
||||
it('returns "Eastern" for exact match', () => {
|
||||
expect(parseConferenceFromExternalId("Eastern")).toBe("Eastern");
|
||||
});
|
||||
|
||||
it('returns "Western" for exact match', () => {
|
||||
expect(parseConferenceFromExternalId("Western")).toBe("Western");
|
||||
});
|
||||
|
||||
it("is case-insensitive", () => {
|
||||
expect(parseConferenceFromExternalId("eastern")).toBe("Eastern");
|
||||
expect(parseConferenceFromExternalId("WESTERN")).toBe("Western");
|
||||
});
|
||||
|
||||
it("returns null for a numeric ESPN team id", () => {
|
||||
expect(parseConferenceFromExternalId("339")).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null for null/undefined", () => {
|
||||
expect(parseConferenceFromExternalId(null)).toBeNull();
|
||||
expect(parseConferenceFromExternalId(undefined)).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null for an unrecognized string", () => {
|
||||
expect(parseConferenceFromExternalId("Northeast")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -18,7 +18,10 @@
|
|||
* Conference assignment resolution order:
|
||||
* 1. regularSeasonStandings.conference (populated by standings sync)
|
||||
* 2. seasonParticipantSimulatorInputs.region ("Eastern" or "Western")
|
||||
* 3. Error — conference must be known for all teams
|
||||
* 3. seasonParticipant.externalId set to "Eastern" or "Western" (pre-sync bootstrap;
|
||||
* note: this prevents externalId-based matching in future syncs for that participant,
|
||||
* which falls back to slower name-matching — acceptable tradeoff for bootstrapping)
|
||||
* 4. Error — conference must be known for all teams
|
||||
*
|
||||
* MLS Cup Playoffs format (18 teams: 9 East + 9 West):
|
||||
* Wild Card : E8 vs E9, W8 vs W9 — single game (no draws; PKs if tied)
|
||||
|
|
@ -70,8 +73,12 @@ function configNumber(config: Record<string, unknown> | undefined, key: string,
|
|||
* Infer MLS conference from seasonParticipant.externalId.
|
||||
* Admins can set externalId to "Eastern" or "Western" before the first standings
|
||||
* sync, mirroring how LLWS uses externalId to encode pool assignment.
|
||||
*
|
||||
* Side-effect: once externalId holds a conference name instead of an ESPN team ID,
|
||||
* the standings-sync write-back won't overwrite it (write-back only fires when
|
||||
* externalId is null). That participant will always match by name, never by ID.
|
||||
*/
|
||||
function parseConferenceFromExternalId(externalId: string | null | undefined): MlsConference | null {
|
||||
export function parseConferenceFromExternalId(externalId: string | null | undefined): MlsConference | null {
|
||||
if (!externalId) return null;
|
||||
const upper = externalId.toUpperCase();
|
||||
if (upper === "EASTERN") return "Eastern";
|
||||
|
|
|
|||
|
|
@ -214,4 +214,73 @@ describe("MlsStandingsAdapter", () => {
|
|||
const adapter = new MlsStandingsAdapter();
|
||||
await expect(adapter.fetchStandings()).rejects.toThrow("no entries");
|
||||
});
|
||||
|
||||
it("passes through unrecognized conference names unchanged", async () => {
|
||||
vi.mocked(fetch).mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: async () => ({
|
||||
children: [
|
||||
{
|
||||
name: "Southern Division",
|
||||
standings: {
|
||||
entries: [
|
||||
{
|
||||
team: { id: "1", displayName: "Test FC" },
|
||||
stats: [
|
||||
makeStat("wins", 5),
|
||||
makeStat("losses", 3),
|
||||
makeStat("ties", 2),
|
||||
makeStat("points", 17),
|
||||
makeStat("pointsFor", 12),
|
||||
makeStat("pointsAgainst", 9),
|
||||
makeStat("pointDifferential", 3),
|
||||
makeStat("playoffSeed", 1),
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
],
|
||||
}),
|
||||
} as Response);
|
||||
|
||||
const adapter = new MlsStandingsAdapter();
|
||||
const records = await adapter.fetchStandings();
|
||||
expect(records[0].conference).toBe("Southern Division");
|
||||
});
|
||||
|
||||
it("returns winPct of 0 when gamesPlayed is 0", async () => {
|
||||
vi.mocked(fetch).mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: async () => ({
|
||||
children: [
|
||||
{
|
||||
name: "Eastern Conference",
|
||||
standings: {
|
||||
entries: [
|
||||
{
|
||||
team: { id: "1", displayName: "Preseason FC" },
|
||||
stats: [
|
||||
makeStat("wins", 0),
|
||||
makeStat("losses", 0),
|
||||
makeStat("ties", 0),
|
||||
makeStat("points", 0),
|
||||
makeStat("pointsFor", 0),
|
||||
makeStat("pointsAgainst", 0),
|
||||
makeStat("pointDifferential", 0),
|
||||
makeStat("playoffSeed", 1),
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
],
|
||||
}),
|
||||
} as Response);
|
||||
|
||||
const adapter = new MlsStandingsAdapter();
|
||||
const records = await adapter.fetchStandings();
|
||||
expect(records[0].winPct).toBe(0);
|
||||
expect(records[0].gamesPlayed).toBe(0);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -71,8 +71,8 @@ function flattenEspnStandings(
|
|||
*/
|
||||
function normalizeConference(raw: string): string {
|
||||
const upper = raw.toUpperCase();
|
||||
if (upper.includes("EASTERN") || upper.includes("EAST")) return "Eastern";
|
||||
if (upper.includes("WESTERN") || upper.includes("WEST")) return "Western";
|
||||
if (upper === "EASTERN" || upper === "EASTERN CONFERENCE") return "Eastern";
|
||||
if (upper === "WESTERN" || upper === "WESTERN CONFERENCE") return "Western";
|
||||
return raw;
|
||||
}
|
||||
|
||||
|
|
@ -100,23 +100,29 @@ export class MlsStandingsAdapter implements StandingsSyncAdapter {
|
|||
);
|
||||
}
|
||||
|
||||
// Sort by points desc for overall league rank
|
||||
const sorted = [...flattened].toSorted((a, b) => {
|
||||
const ptsA = statsMap(a.entry.stats).get("points")?.value ?? 0;
|
||||
const ptsB = statsMap(b.entry.stats).get("points")?.value ?? 0;
|
||||
// Pre-parse each entry's stats once, then sort by points desc for overall league rank
|
||||
const parsed = flattened.map(({ entry, conference }) => ({
|
||||
entry,
|
||||
conference,
|
||||
sm: statsMap(entry.stats),
|
||||
}));
|
||||
|
||||
const sorted = [...parsed].toSorted((a, b) => {
|
||||
const ptsA = a.sm.get("points")?.value ?? 0;
|
||||
const ptsB = b.sm.get("points")?.value ?? 0;
|
||||
if (ptsB !== ptsA) return ptsB - ptsA;
|
||||
const gdA = statsMap(a.entry.stats).get("pointDifferential")?.value ?? 0;
|
||||
const gdB = statsMap(b.entry.stats).get("pointDifferential")?.value ?? 0;
|
||||
const gdA = a.sm.get("pointDifferential")?.value ?? 0;
|
||||
const gdB = b.sm.get("pointDifferential")?.value ?? 0;
|
||||
return gdB - gdA;
|
||||
});
|
||||
|
||||
return sorted.map(({ entry, conference }, leagueIdx): FetchedStandingsRecord => {
|
||||
const sm = statsMap(entry.stats);
|
||||
|
||||
return sorted.map(({ entry, conference, sm }, leagueIdx): FetchedStandingsRecord => {
|
||||
const wins = sm.get("wins")?.value ?? 0;
|
||||
const losses = sm.get("losses")?.value ?? 0;
|
||||
const ties = sm.get("ties")?.value ?? sm.get("draws")?.value ?? 0;
|
||||
const gamesPlayed = Math.round(wins + losses + ties);
|
||||
const gamesPlayed = Math.round(
|
||||
sm.get("gamesPlayed")?.value ?? wins + losses + ties
|
||||
);
|
||||
|
||||
const tablePoints = sm.get("points")?.value;
|
||||
const goalsFor = sm.get("pointsFor")?.value ?? sm.get("gf")?.value;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue