fix(odds): stop a stalled ESPN taking the whole plugin update with it

[ChuckBuilds]

Odds are fetched per live game from inside SportsLive.update(), with show_odds defaulting on, and the plugin executor kills an operation at 30s. Two numbers were the same and shouldn't have been:

odds request_timeout           = 30s
PluginExecutor default_timeout = 30s

So a single stalled ESPN request consumed the entire budget and the update carrying every game's score was killed. Observed on a live device:

00:43:43  ERROR  plugin football-scoreboard operation timed out after 30.0s
01:43:43  ERROR  plugin football-scoreboard operation timed out after 30.0s

Out of season this is invisible — preseason week 1 returns one game. A Sunday slate is ~16, so the chance of at least one slow request rises sharply just as the cost of losing the update does.

Fix

A 5s request timeout plus a 60s cooldown after any network failure. The timeout alone isn't sufficient: 16 consecutive 5s timeouts still blow through. And when ESPN is unreachable it's unreachable for the whole slate, so the first failure already answers the question for the rest of the pass.

before: one stalled request = 30s = the entire budget
after : 5s, the rest of the slate skipped, retry after 60s

The stale-cache fallback is unchanged — the cache is consulted before any of this, and the failing request still falls back to it.

What I removed, and why

An earlier version of this branch jittered the cache TTL to stagger expiry across a slate. I've dropped it: it was inert. CacheManager.set() stores ttl for compatibility, but the read path expires entries by a per-data-type max_age — 1800s for odds — and never consults the stored value. Its own docstring says so, and I confirmed it in get_cached_data(), which passes max_age to both cache layers.

Credit to the review for catching that; I'd asserted a mechanism without checking the read side.

Making the read path honour a per-entry ttl is a genuine fix, but 48 plugin call sites already pass ttl= believing it works, so changing that contract would alter their expiry behaviour all at once. That's not a change to make two weeks before the season, and it deserves its own PR.

I've also stopped claiming the stampede caused the hourly timeouts. Neither cache is hourly (schedule 86400s, odds 1800s), and normal updates already take 6–10s ("consider optimizing"), so the cadence isn't explained yet. This PR fixes a real hazard on its own terms; the hourly timeout still wants a proper investigation.

Verification

8 tests in test_odds_request_budget.py: the timeout leaves room in the budget and is the one actually used; a 16-game slate makes exactly one network call when ESPN stalls; the worst case stays inside the budget; the breaker reopens on schedule; a healthy fetch clears it; and the stale-cache fallback still returns cached odds for the failing request.

Two tests on main pinned the 30s timeout and now assert the new value plus the property that matters. Full suite: 2703 passed, 1 pre-existing unrelated failure (test_install_lowmem).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

[coderabbitai]

📝 Walkthrough

Walkthrough

BaseOddsManager now applies deterministic ±15% jitter to positive odds-cache TTLs and uses the jittered value for odds and no-odds writes. It also uses a 5-second default request timeout and suppresses ESPN requests for 60 seconds after network failures.

Changes

Odds cache resilience

Layer / File(s)	Summary
TTL jitter and cache writes src/base_odds_manager.py, test/test_odds_cache_stampede.py	Positive cache intervals use deterministic BLAKE2b-derived ±15% jitter. Nonpositive intervals remain unchanged. Odds and no-odds writes use the jittered TTL. Tests cover bounds, spread, edge cases, expiry distribution, and cache propagation.
Network timeout and failure cooldown src/base_odds_manager.py, test/test_odds_cache_stampede.py	The default request timeout is 5 seconds. Network failures start a 60-second cooldown. Requests are skipped during the cooldown, and successful responses clear it.
Existing test alignment test/test_base_odds_manager.py	Existing assertions now allow TTL jitter and expect the 5-second timeout across cache and configuration cases.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant BaseOddsManager
  participant ESPN
  participant Cache
  BaseOddsManager->>ESPN: Request odds with 5-second timeout
  ESPN-->>BaseOddsManager: Successful response
  BaseOddsManager->>BaseOddsManager: Clear failure cooldown
  BaseOddsManager->>Cache: Write result with jittered TTL
  ESPN-->>BaseOddsManager: Network failure
  BaseOddsManager->>BaseOddsManager: Start 60-second cooldown
  BaseOddsManager->>ESPN: Skip request during cooldown

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the changes that prevent stalled ESPN requests from blocking the plugin update.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/odds-cache-stampede

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/base_odds_manager.py`:
- Around line 156-163: Update the cache read path used by the odds manager so
each cached entry expires using its stored jittered ttl, rather than the default
max_age. Align the read-side logic in CacheManager with the ttl written by
BaseOddsManager’s _jittered_ttl flow, while preserving the existing serial ESPN
refetch behavior.

In `@test/test_odds_cache_stampede.py`:
- Around line 61-75: Update
test_a_slate_fetched_together_does_not_expire_together to remove the
probabilistic worst <= 6 assertion; either mock
src.base_odds_manager.random.uniform with evenly spaced deterministic values
before calling _jittered_ttl, or assert only a deterministic property that does
not impose a random maximum-concurrency guarantee.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 71fe319f-706f-4339-bb01-9f6ed63f763c

📥 Commits

Reviewing files that changed from the base of the PR and between ca26c1b and 16e1cef.

📒 Files selected for processing (2)

• src/base_odds_manager.py
• test/test_odds_cache_stampede.py

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric	Results
Complexity	0
Duplication	0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/base_odds_manager.py`:
- Around line 194-201: Update the interval selection before _jittered_ttl in the
odds caching flow to use self.update_interval only when update_interval_seconds
is None, preserving an explicit zero override so _jittered_ttl returns the
no-expiry TTL.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4f56b4ec-0d74-4961-b32a-c94d19d00d98

📥 Commits

Reviewing files that changed from the base of the PR and between 16e1cef and 354f05c.

📒 Files selected for processing (3)

• src/base_odds_manager.py
• test/test_base_odds_manager.py
• test/test_odds_cache_stampede.py

[ChuckBuilds]

Both remaining findings predate the rewrite of this branch and no longer apply, so I'm closing them out rather than acting on them.

"Make the cache read path apply the per-entry TTL" — this was the important one, and it was right: CacheManager.set() stored ttl while the read path expired by a per-type max_age, so the jitter I'd added was inert. I dropped the jitter from this branch entirely; what remains here is only the request timeout and the failure cooldown, neither of which touches the cache.

The underlying defect is now fixed properly in #450, which makes both cache layers honour a stored ttl and falls back to max_age when there isn't one. Measured against a real device's cache of 8,875 entries with a ttl, the inferred and intended values disagreed nearly everywhere — stocks 600 vs 1800, news 3600 vs 600, odds 1800 vs 3600 — and no sports_live entry carries a ttl at all, so live freshness is untouched.

"Remove the random maximum-concurrency guarantee" — the file it refers to, test/test_odds_cache_stampede.py, was deleted in the rewrite. Its replacement, test_odds_request_budget.py, makes no probabilistic claim: the assertions are that exactly one network call happens for a 16-game slate when ESPN stalls, that the timeout is below the operation budget, and that the breaker reopens on schedule.

"Preserve an explicit zero cache interval" — no longer reachable from this branch. _jittered_ttl() and its zero passthrough are gone, so nothing here interacts with update_interval_seconds or self.update_interval. That quirk is pre-existing and deliberately characterised on main by test_zero_interval_falls_back_to_default ("Quirk pin: treats an explicit 0 as falsy, so the 3600 default wins"), so changing it belongs in its own PR — and it becomes more meaningful now that #450 makes ttl real, since a caller asking for 0 would presumably mean it.