From c0a3ceb4b9cdb1f2d549615e59ccf739ce52a202 Mon Sep 17 00:00:00 2001 From: Tim Thomas <0800tim@gmail.com> Date: Fri, 5 Jun 2026 13:17:39 +1200 Subject: [PATCH] fix(bracket): only count a knockout as picked when both slots are resolved Tim 2026-06-05: > The number of matches picked shows 104 out of 104 for me in this > screenshot, however I've only selected 6 of the 8 for the Top Eight > Thirds stage. Then, on the round of 32 stage, any matches without > two teams should not be considered picked, so my actual pick should > be a lot less. Root cause: the picked counters across the surface read off `Object.keys(bracket.knockoutPredictions).length`. The bracket's `knockoutPredictions` map stores every winner the user has tapped, including taps on R32 matches whose other side is still a TBD Best-3rd placeholder. The cascade engine deliberately keeps those stored picks (it only invalidates a pick when both sides resolve to teams that exclude the stored winner) so the count stayed full at 104/104 even when 2 of the 8 Best-3rd slots were empty, and consequently several R32 matches had no opponent for the user's pick to play. Fix: a knockout match counts as picked iff the cascaded view says `home.team !== null && away.team !== null && effective_winner !== null`. The third clause folds in the engine's existing `winner_not_in_match` handling at no extra cost. Applied in four places, all of which were reading the same wrong counter: * `BracketBuilder.tsx` -- the big "104 / 104 MATCHES PICKED" hero stat + per-stage tab badges. `knockoutCountFor` no longer takes a `picks` arg; it derives the count purely from `cascaded.knockouts`. * `LockSummary.tsx` -- the "X of N picks saved" deadline panel + the share-modal `isComplete` gate. * `ShareSavePage.tsx` -- the share landing's pick counter + isComplete (used to decide whether the user is ready to share). * `MoleculeScene.tsx` -- the "finish your bracket" modal that decides whether to nudge the user back to /world-cup-2026. It was hiding the nudge when `totalPicks >= totalRequired` under the old semantics, which meant a 6-of-8-thirds user got no nudge at all. Two telemetry-only call sites (`analytics.track` payloads logging the raw map size at save / auto-pick time) are left unchanged: those correctly want "how many entries are in the predictions map", not "how many user-visible picks are valid". Refs: docs/internal/home-polish-spec.md Signed-off-by: Tim Thomas <0800tim@gmail.com> --- .../web/components/bracket/BracketBuilder.tsx | 49 ++++++++++++++++--- apps/web/components/bracket/LockSummary.tsx | 23 +++++++-- .../web/components/molecule/MoleculeScene.tsx | 14 +++++- apps/web/components/share/ShareSavePage.tsx | 17 ++++++- 4 files changed, 88 insertions(+), 15 deletions(-) diff --git a/apps/web/components/bracket/BracketBuilder.tsx b/apps/web/components/bracket/BracketBuilder.tsx index 23a54868..fd868d80 100644 --- a/apps/web/components/bracket/BracketBuilder.tsx +++ b/apps/web/components/bracket/BracketBuilder.tsx @@ -229,11 +229,23 @@ function migrateBracket(b: Bracket | null): { bracket: Bracket | null; wiped: bo /** * Count picks for a given knockout stage so the per-tab progress * indicator reads "x of N picked". + * + * Tim 2026-06-05: previously this counted any cascaded knockout that + * had a stored pick, including matches where one slot was still TBD + * (e.g. a Best-3rd opponent that hadn't been resolved because the + * user only filled in 6 of 8 thirds). The engine keeps the user's + * stored pick for those half-resolved matches and only nulls it out + * when both slots resolve to teams that exclude it -- which inflated + * the counter. We now require both slots to be resolved AND the + * engine-computed `effective_winner` to be non-null. That handles + * three categories of stored-but-not-actually-picked state in one + * check: (a) only one slot resolved, (b) zero slots resolved, and + * (c) both resolved but the pick references a team no longer in the + * matchup (engine sets effective_winner=null + emits a warning). */ function knockoutCountFor( stage: TabId, cascaded: CascadedBracket, - picks: Record, ): { picked: number; total: number } { const stageIds = stage === "sf" @@ -246,7 +258,15 @@ function knockoutCountFor( ); const total = matches.length; let picked = 0; - for (const m of matches) if (picks[m.id]) picked += 1; + for (const m of matches) { + if ( + m.home.team !== null && + m.away.team !== null && + m.effective_winner !== null + ) { + picked += 1; + } + } return { picked, total }; } @@ -1354,7 +1374,20 @@ export function BracketBuilder(props: BracketBuilderProps) { const totalGroupMatches = tournament.group_fixtures.length; const completedGroupMatches = Object.keys(bracket.matchPredictions).length; - const completedKnockouts = Object.keys(bracket.knockoutPredictions).length; + // See `knockoutCountFor` above: a knockout match counts as "picked" + // only when both slots have resolved teams AND the engine has a + // valid effective_winner. Tim 2026-06-05 caught the header reading + // 104/104 with only 6 of 8 thirds picked, because the previous + // count was `Object.keys(bracket.knockoutPredictions).length` which + // includes picks for matches whose other side is still TBD. + const completedKnockouts = cascaded.knockouts.reduce( + (n, k) => + n + + (k.home.team !== null && k.away.team !== null && k.effective_winner !== null + ? 1 + : 0), + 0, + ); const totalKnockouts = tournament.knockouts.length; const totalPicks = totalGroupMatches + totalKnockouts; const totalCompleted = completedGroupMatches + completedKnockouts; @@ -1382,11 +1415,11 @@ export function BracketBuilder(props: BracketBuilderProps) { picked: (bracket.bestThirds ?? []).length, total: 8, }; - const r32Progress = knockoutCountFor("r32", cascaded, bracket.knockoutPredictions); - const r16Progress = knockoutCountFor("r16", cascaded, bracket.knockoutPredictions); - const qfProgress = knockoutCountFor("qf", cascaded, bracket.knockoutPredictions); - const sfProgress = knockoutCountFor("sf", cascaded, bracket.knockoutPredictions); - const finalProgress = knockoutCountFor("final", cascaded, bracket.knockoutPredictions); + const r32Progress = knockoutCountFor("r32", cascaded); + const r16Progress = knockoutCountFor("r16", cascaded); + const qfProgress = knockoutCountFor("qf", cascaded); + const sfProgress = knockoutCountFor("sf", cascaded); + const finalProgress = knockoutCountFor("final", cascaded); const progressByTab: Record = { groups: groupProgress, diff --git a/apps/web/components/bracket/LockSummary.tsx b/apps/web/components/bracket/LockSummary.tsx index b98e8f39..31adc380 100644 --- a/apps/web/components/bracket/LockSummary.tsx +++ b/apps/web/components/bracket/LockSummary.tsx @@ -74,11 +74,24 @@ export function LockSummary(props: LockSummaryProps) { const deadline = Date.parse(deadline_utc); // Per-match counts (group + knockout = up to 104 for World Cup 2026). + // Tim 2026-06-05: knockout picks are counted off the cascaded view + // (both slots resolved AND engine has a valid effective_winner), not + // off the raw bracket.knockoutPredictions map. The raw map keeps + // stored picks for matches whose other side is still TBD (e.g. a + // Best-3rd opponent the user hasn't picked), and those should not + // count toward "X of 104 picks saved". const totalGroup = tournament.group_fixtures.length; const totalKnockout = tournament.knockouts.length; const totalPicks = totalGroup + totalKnockout; const groupPicks = Object.keys(bracket.matchPredictions).length; - const knockoutPicks = Object.keys(bracket.knockoutPredictions).length; + const knockoutPicks = cascaded.knockouts.reduce( + (n, k) => + n + + (k.home.team !== null && k.away.team !== null && k.effective_winner !== null + ? 1 + : 0), + 0, + ); const committed = groupPicks + knockoutPicks; // Predicted champion: cascade's effective_winner of the Final. @@ -95,10 +108,10 @@ export function LockSummary(props: LockSummaryProps) { // synthetic stub that PR #140 generated before the backend lookup // existed. const shareWinner = champion === "—" ? "TBD" : champion; - const isComplete = - Object.keys(bracket.matchPredictions).length + - Object.keys(bracket.knockoutPredictions).length >= - totalPicks; + // Same cascade-aware semantics as `committed` above: the bracket is + // complete only when every group AND every knockout match has a + // genuine pick that the engine accepts. + const isComplete = committed >= totalPicks; const [storedShareGuid, setStoredShareGuid] = useState(null); useEffect(() => { setStoredShareGuid(loadStoredShareGuid(tournament.id, localUserId())); diff --git a/apps/web/components/molecule/MoleculeScene.tsx b/apps/web/components/molecule/MoleculeScene.tsx index 83c73d55..6f23944d 100644 --- a/apps/web/components/molecule/MoleculeScene.tsx +++ b/apps/web/components/molecule/MoleculeScene.tsx @@ -838,7 +838,19 @@ export function MoleculeScene({ // bracket and the viewer can't go fill it in). if (readOnly) return null; const groupPicks = Object.keys(bracket.matchPredictions).length; - const knockoutPicks = Object.keys(bracket.knockoutPredictions).length; + // Tim 2026-06-05: cascade-aware knockout count. The raw + // predictions map keeps picks for matches with one TBD side, + // so it overstated "remaining" -- e.g. with 6 of 8 thirds + // picked the modal would have read "0 remaining" but the + // bracket isn't actually finishable until the thirds are in. + const knockoutPicks = cascaded.knockouts.reduce( + (n, k) => + n + + (k.home.team !== null && k.away.team !== null && k.effective_winner !== null + ? 1 + : 0), + 0, + ); const totalPicks = groupPicks + knockoutPicks; const totalRequired = tournament.group_fixtures.length + tournament.knockouts.length; diff --git a/apps/web/components/share/ShareSavePage.tsx b/apps/web/components/share/ShareSavePage.tsx index 7afd1b75..f457bd04 100644 --- a/apps/web/components/share/ShareSavePage.tsx +++ b/apps/web/components/share/ShareSavePage.tsx @@ -322,7 +322,22 @@ export function ShareSavePage({ const totalKnockout = tournament.knockouts.length; const totalPicks = totalGroup + totalKnockout; const groupPicks = bracket ? Object.keys(bracket.matchPredictions).length : 0; - const knockoutPicks = bracket ? Object.keys(bracket.knockoutPredictions).length : 0; + // Tim 2026-06-05: same cascade-aware semantics as BracketBuilder / + // LockSummary. The raw `bracket.knockoutPredictions` map keeps picks + // for matches whose other side is still TBD, so counting its keys + // inflated the "X of 104" readout (104/104 with only 6 of 8 thirds + // picked). A knockout match counts as picked only when both slots + // are resolved AND the engine accepted a winner. + const knockoutPicks = cascaded + ? cascaded.knockouts.reduce( + (n, k) => + n + + (k.home.team !== null && k.away.team !== null && k.effective_winner !== null + ? 1 + : 0), + 0, + ) + : 0; const committed = groupPicks + knockoutPicks; const isComplete = committed === totalPicks;