fix(fantasy): stop marketValue from returning null on cold cache - #291
Merged
Merged
Conversation
getFantasyCalcValuesForOverview waited only 150ms before falling back to an empty values map, but the underlying FantasyCalc request can take up to FANTASYCALC_TIMEOUT_MS (2000ms). On a cold in-memory cache (e.g. right after a deploy), the overview almost always timed out before the fetch completed, so every player's marketValue and positionRank came back null until a second request warmed the cache. Align the overview wait with the request timeout so the first request has a real chance to populate market values. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Add a delayed-response cold-cache test to prevent regression to the shorter wait.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Aligns the FantasyCalc overview wait with the request timeout so cold-cache requests can populate player values.
Changes:
- Increases the overview wait from 150 ms to 2 seconds.
- Documents the cold-cache rationale.
File summaries
| File | Summary |
|---|---|
packages/backend/src/fantasy/fantasy.service.ts |
Aligns FantasyCalc overview wait timing with the request timeout. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Must be long enough for the FantasyCalc request (bounded by FANTASYCALC_TIMEOUT_MS) to actually | ||
| // finish on a cold cache; a short wait here mostly guarantees the fallback (all-null marketValues) | ||
| // on every first request after a deploy, since the in-memory cache resets on restart. | ||
| const FANTASYCALC_OVERVIEW_WAIT_MS = FANTASYCALC_TIMEOUT_MS; |
Addressed in 62a9879 by adding a delayed cold-cache regression test for |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
marketValueandpositionRankwere coming backnullfor all players on the fantasy overview, especially right after a deploy.Root cause
getFantasyCalcValuesForOverviewonly waitedFANTASYCALC_OVERVIEW_WAIT_MS(150ms) before falling back to an empty values map, but the underlying FantasyCalc request is bounded byFANTASYCALC_TIMEOUT_MS(2000ms). On a cold in-memory cache (e.g. right after a restart/deploy), the overview almost always timed out at 150ms before the fetch could complete, so every player'smarketValue/positionRankcame backnulluntil a subsequent request warmed the cache.Verified live: a cold-cache call to
getFantasyCalcValuesForOverviewreturned 0 populated values before the fix, and 423 after.Fix
Align
FANTASYCALC_OVERVIEW_WAIT_MSwithFANTASYCALC_TIMEOUT_MSso the first request has a real chance to populate market values before falling back.Testing
npx vitest run src/fantasy/fantasy.service.spec.ts— 25/25 passing