perf: cache user team lookup in player_autocomplete, reduce limit to 25 #100

Claude · 2026-03-20T13:34:50Z

Claude commented

2026-03-20 13:34:50 +00:00

Summary

Closes #99

Add _get_cached_user_team() with a 60-second per-user TTL so get_user_major_league_team is called at most once per minute instead of on every keystroke
Reduce search_players(limit=50) → limit=25 to match Discord's 25-choice display cap and avoid fetching unused results
Add TestGetCachedUserTeam test class covering cache hit, TTL expiry, and None caching; add clear_user_team_cache autouse fixture to prevent test interference via module-level state

Test plan

python -m pytest tests/test_utils_autocomplete.py -v — all 16 tests pass (3 new cache tests)
python -m pytest --tb=short -q — 956 passed, 2 skipped

🤖 Generated with Claude Code

## Summary Closes #99 - Add `_get_cached_user_team()` with a 60-second per-user TTL so `get_user_major_league_team` is called at most once per minute instead of on every keystroke - Reduce `search_players(limit=50)` → `limit=25` to match Discord's 25-choice display cap and avoid fetching unused results - Add `TestGetCachedUserTeam` test class covering cache hit, TTL expiry, and `None` caching; add `clear_user_team_cache` autouse fixture to prevent test interference via module-level state ## Test plan - [ ] `python -m pytest tests/test_utils_autocomplete.py -v` — all 16 tests pass (3 new cache tests) - [ ] `python -m pytest --tb=short -q` — 956 passed, 2 skipped 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Claude added 1 commit 2026-03-20 13:34:51 +00:00

perf: cache user team lookup in player_autocomplete, reduce limit to 25

Build Docker Image / build (pull_request) Successful in 1m16s

Details

c8ed4dee38

Closes #99

- Add module-level `_user_team_cache` dict with 60-second TTL so
  `get_user_major_league_team` is called at most once per minute per
  user instead of on every keystroke.
- Reduce `search_players(limit=50)` to `limit=25` to match Discord's
  25-choice display cap and avoid fetching unused results.
- Add `TestGetCachedUserTeam` class covering cache hit, TTL expiry, and
  None caching; add `clear_user_team_cache` autouse fixture to prevent
  test interference via module-level state.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Claude added the

ai-reviewing

label 2026-03-20 13:45:50 +00:00

Claude reviewed 2026-03-20 13:47:25 +00:00

Claude left a comment

AI Code Review

Files Reviewed

utils/autocomplete.py (modified)
tests/test_utils_autocomplete.py (modified)

Findings

Correctness

The _get_cached_user_team function is logically sound. Cache lookup checks TTL correctly using time.time() - cached_at < _USER_TEAM_CACHE_TTL, and the fallthrough path calls get_user_major_league_team(user_id) and writes the result back to the dict unconditionally — including None — which matches the PR's stated goal of caching "no team" users too.
Reducing limit=50 to limit=25 is correct. The existing display logic already capped output at 25 (remaining_slots = 25 - len(choices) and user_team_players[:15]), so the previous 50-item fetch was discarding up to half the results every time.
The Team type added to the models.team import is needed for the _user_team_cache dict annotation — no phantom import.
_USER_TEAM_CACHE_TTL is exported as a module-level name so the TTL-expiry test can import it to seed the stale cache entry. Pairing the constant with the test that depends on it is the right approach.
In test_player_autocomplete_prioritizes_user_team, the test patches utils.autocomplete.get_user_major_league_team via plain patch(...). Since get_user_major_league_team is async def, Python 3.8+ patch auto-creates an AsyncMock, so mock_get_team.return_value = user_team is valid and the await inside _get_cached_user_team resolves correctly.

Security

No user input flows into queries or shell commands through the new code path. The cache key is interaction.user.id (a Discord snowflake int), which is safe to use as a dict key.
No secrets or credentials introduced.

Style & Conventions

Private helper _get_cached_user_team follows the existing underscore-prefix convention for module-private names.
All imports added at the top of the file — no lazy imports mid-file, consistent with CLAUDE.md.
autouse fixture in both TestPlayerAutocomplete and TestGetCachedUserTeam clears _user_team_cache before and after each test, correctly handling the shared module-level state. The fixture in TestPlayerAutocomplete is defensive (tests there patch the underlying function anyway) but it is the right habit given the cache now exists.
Cosmetic reformatting (single → double quotes, trailing commas, with (...) context manager syntax) throughout the test file is consistent and expected from the project's linter.

Suggestions

The module-level _user_team_cache dict is unbounded. In normal bot operation with a fixed set of Discord users this is fine — a league has a small number of owners — but it is worth noting that entries are never evicted except by TTL expiry on next access. A simple if len(_user_team_cache) > 500: _user_team_cache.clear() safety valve could prevent any pathological growth, though this is not a realistic concern for this codebase size.
The existing test_player_autocomplete_success and test_player_autocomplete_with_team_info tests do not patch get_user_major_league_team. Because _get_cached_user_team will now attempt a real call (or raise, since team_service is not mocked in those tests), these tests rely on the exception being swallowed by the outer except Exception: return [] block in player_autocomplete. The tests still pass, but the user_team being silently None due to an error rather than legitimately None is a subtle difference in what the tests actually cover. Not a blocker — the behavior is correct.

Verdict: APPROVED

Clean, targeted optimization. The caching implementation is correct, the limit reduction eliminates wasted API fetches, and the three new tests (cache hit, TTL expiry, None caching) plus the autouse isolation fixture give solid coverage of the new code path. No convention violations, no security issues.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `utils/autocomplete.py` (modified) - `tests/test_utils_autocomplete.py` (modified) ### Findings #### Correctness - The `_get_cached_user_team` function is logically sound. Cache lookup checks TTL correctly using `time.time() - cached_at < _USER_TEAM_CACHE_TTL`, and the fallthrough path calls `get_user_major_league_team(user_id)` and writes the result back to the dict unconditionally — including `None` — which matches the PR's stated goal of caching "no team" users too. - Reducing `limit=50` to `limit=25` is correct. The existing display logic already capped output at 25 (`remaining_slots = 25 - len(choices)` and `user_team_players[:15]`), so the previous 50-item fetch was discarding up to half the results every time. - The `Team` type added to the `models.team` import is needed for the `_user_team_cache` dict annotation — no phantom import. - `_USER_TEAM_CACHE_TTL` is exported as a module-level name so the TTL-expiry test can import it to seed the stale cache entry. Pairing the constant with the test that depends on it is the right approach. - In `test_player_autocomplete_prioritizes_user_team`, the test patches `utils.autocomplete.get_user_major_league_team` via plain `patch(...)`. Since `get_user_major_league_team` is `async def`, Python 3.8+ `patch` auto-creates an `AsyncMock`, so `mock_get_team.return_value = user_team` is valid and the `await` inside `_get_cached_user_team` resolves correctly. #### Security - No user input flows into queries or shell commands through the new code path. The cache key is `interaction.user.id` (a Discord snowflake int), which is safe to use as a dict key. - No secrets or credentials introduced. #### Style & Conventions - Private helper `_get_cached_user_team` follows the existing underscore-prefix convention for module-private names. - All imports added at the top of the file — no lazy imports mid-file, consistent with CLAUDE.md. - `autouse` fixture in both `TestPlayerAutocomplete` and `TestGetCachedUserTeam` clears `_user_team_cache` before and after each test, correctly handling the shared module-level state. The fixture in `TestPlayerAutocomplete` is defensive (tests there patch the underlying function anyway) but it is the right habit given the cache now exists. - Cosmetic reformatting (single → double quotes, trailing commas, `with (...)` context manager syntax) throughout the test file is consistent and expected from the project's linter. #### Suggestions - The module-level `_user_team_cache` dict is unbounded. In normal bot operation with a fixed set of Discord users this is fine — a league has a small number of owners — but it is worth noting that entries are never evicted except by TTL expiry on next access. A simple `if len(_user_team_cache) > 500: _user_team_cache.clear()` safety valve could prevent any pathological growth, though this is not a realistic concern for this codebase size. - The existing `test_player_autocomplete_success` and `test_player_autocomplete_with_team_info` tests do not patch `get_user_major_league_team`. Because `_get_cached_user_team` will now attempt a real call (or raise, since `team_service` is not mocked in those tests), these tests rely on the exception being swallowed by the outer `except Exception: return []` block in `player_autocomplete`. The tests still pass, but the `user_team` being silently `None` due to an error rather than legitimately `None` is a subtle difference in what the tests actually cover. Not a blocker — the behavior is correct. ### Verdict: APPROVED Clean, targeted optimization. The caching implementation is correct, the limit reduction eliminates wasted API fetches, and the three new tests (cache hit, TTL expiry, None caching) plus the `autouse` isolation fixture give solid coverage of the new code path. No convention violations, no security issues. --- *Automated review by Claude PR Reviewer*

cal changed target branch from main to next-release

2026-03-20 15:27:09 +00:00

cal merged commit 8862850c59 into next-release

2026-03-20 15:27:35 +00:00

cal deleted branch ai/major-domo-v2#99

2026-03-20 15:27:35 +00:00

cal referenced this issue from a commit

2026-03-20 15:27:36 +00:00

Merge pull request 'perf: cache user team lookup in player_autocomplete, reduce limit to 25' (#100) from ai/major-domo-v2#99 into next-release

cal referenced this issue from a commit

2026-03-21 12:35:49 +00:00

perf: add caching for frequently-accessed stable data (#91)