Skip to content

fix(bracket): only count a knockout as picked when both slots are resolved#292

Merged
0800tim merged 1 commit into
mainfrom
fix/picked-count-excludes-incomplete
Jun 5, 2026
Merged

fix(bracket): only count a knockout as picked when both slots are resolved#292
0800tim merged 1 commit into
mainfrom
fix/picked-count-excludes-incomplete

Conversation

@0800tim

@0800tim 0800tim commented Jun 5, 2026

Copy link
Copy Markdown
Owner

Tim 2026-06-05: the "104 / 104 MATCHES PICKED" hero counter (and the per-tab badges) read 104/104 with only 6 of 8 thirds picked, because the count came off the raw bracket.knockoutPredictions map which keeps picks for R32 matches whose other side is still a Best-3rd TBD placeholder.

A knockout match now counts as picked iff the engine's cascade says home.team !== null && away.team !== null && effective_winner !== null. Applied to:

  • BracketBuilder (hero stat + per-tab counters)
  • LockSummary (deadline panel + share-complete gate)
  • ShareSavePage (share-page pick counter + isComplete)
  • MoleculeScene (the finish-your-bracket nudge modal)

Telemetry call sites that log the raw map size for storage analytics are intentionally left unchanged.

…olved

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>
@0800tim 0800tim merged commit fec783a into main Jun 5, 2026
11 of 12 checks passed
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

DRY-RUN — this verdict is informational; CI is not blocked.

Auto-triage: GREEN — auto-triage clear

Risk score: 0/100

Metric Value
Files changed 4
Lines added 88
Lines removed 15
Apps touched apps/web
New dependencies 0
New 3rd-party hosts 0

No flags raised by the automated scanners. A human reviewer will still take a look.

Labels applied: area:web, auto-triage:green

Posted by @vtorn/pr-triage-bot. How this works: docs/security/01-pr-triage-process.md. Disagree with the verdict? Comment /triage override <reason> and a maintainer will re-review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant