Skip to content

fix(client): include sideboard in the bracket-estimate cache key#5188

Merged
matthewevans merged 1 commit into
phase-rs:mainfrom
jeffrey701:fix/bracket-estimate-sideboard-key
Jul 9, 2026
Merged

fix(client): include sideboard in the bracket-estimate cache key#5188
matthewevans merged 1 commit into
phase-rs:mainfrom
jeffrey701:fix/bracket-estimate-sideboard-key

Conversation

@jeffrey701

Copy link
Copy Markdown
Contributor

Problem

useBracketEstimate (client/src/hooks/useBracketEstimate.ts) derives a deckKey used both as the effect's short-circuit guard (if (deckKey === storedKeyRef.current) return;) and as the module cache key. But it was built from commanders + deck.main only:

const parts = [...commanders.map(...)];
for (const e of deck.main) parts.push(`m:${e.count}x${e.name...}`);

The estimate request itself, however, sends the sideboard too:

sideboard: deck.sideboard.flatMap((e) => Array(e.count).fill(e.name)),

So two decks identical in commanders + main but differing only in sideboard produced the same deckKey. The effect short-circuits and the cache returns the previous deck's estimate — a sideboard edit silently shows a stale bracket. The effect's own dependency comment even claimed sideboard "content is fully captured by deckKey", which was false.

Fix

Extract the key into a pure, unit-tested buildBracketDeckKey (client/src/hooks/bracketDeckKey.ts) that folds in the sideboard as well (s: parts). This makes the dependency comment true and keeps the sorted, order-independent behavior. Extraction is what lets the key be tested without standing up the debounced async hook.

Tests

Adds bracketDeckKey.test.ts: a sideboard-only difference now changes the key (fails before the fix — keys are equal); identical decks stay equal; ordering within main/sideboard doesn't matter; a main-deck change and a same-name main-vs-sideboard placement both change the key.

Self-contained: only useBracketEstimate.ts, a new pure helper, and its test; no engine/Rust/protocol changes.

Model: claude-opus-4-8

useBracketEstimate built deckKey from commanders + deck.main only, but the estimate request (and the module cache/effect short-circuit keyed on deckKey) also depends on deck.sideboard. Two decks identical in commanders+main but differing in sideboard produced the same key, so a sideboard change returned the stale estimate for the previous deck. Extract the key into a pure, unit-tested buildBracketDeckKey that also folds in the sideboard (making the effect's dependency comment actually true).
@jeffrey701 jeffrey701 requested a review from matthewevans as a code owner July 6, 2026 05:32
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@matthewevans matthewevans added the defer-fe Frontend/client/UI PR deferred to Matt's direct review label Jul 6, 2026
@matthewevans matthewevans self-assigned this Jul 9, 2026
@matthewevans matthewevans removed the defer-fe Frontend/client/UI PR deferred to Matt's direct review label Jul 9, 2026
@matthewevans matthewevans added this pull request to the merge queue Jul 9, 2026
@matthewevans matthewevans removed their assignment Jul 9, 2026
Merged via the queue into phase-rs:main with commit 428db52 Jul 9, 2026
11 checks passed
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.

2 participants