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

Reviewed-on: #100

tests/test_utils_autocomplete.py

@@ -3,10 +3,16 @@ Tests for shared autocomplete utility functions.
 
 Validates the shared autocomplete functions used across multiple command modules.
 """
+
 import pytest
 from unittest.mock import AsyncMock, MagicMock, patch
 
-from utils.autocomplete import player_autocomplete, team_autocomplete, major_league_team_autocomplete
+import utils.autocomplete
+from utils.autocomplete import (
+    player_autocomplete,
+    team_autocomplete,
+    major_league_team_autocomplete,
+)
 from tests.factories import PlayerFactory, TeamFactory
 from models.team import RosterType
 
@@ -14,6 +20,13 @@ from models.team import RosterType
 class TestPlayerAutocomplete:
     """Test player autocomplete functionality."""
 
+    @pytest.fixture(autouse=True)
+    def clear_user_team_cache(self):
+        """Clear the module-level user team cache before each test to prevent interference."""
+        utils.autocomplete._user_team_cache.clear()
+        yield
+        utils.autocomplete._user_team_cache.clear()
+
     @pytest.fixture
     def mock_interaction(self):
         """Create a mock Discord interaction."""
@@ -26,41 +39,43 @@ class TestPlayerAutocomplete:
         """Test successful player autocomplete."""
         mock_players = [
             PlayerFactory.mike_trout(id=1),
-            PlayerFactory.ronald_acuna(id=2)
+            PlayerFactory.ronald_acuna(id=2),
         ]
 
-        with patch('utils.autocomplete.player_service') as mock_service:
+        with patch("utils.autocomplete.player_service") as mock_service:
             mock_service.search_players = AsyncMock(return_value=mock_players)
 
-            choices = await player_autocomplete(mock_interaction, 'Trout')
+            choices = await player_autocomplete(mock_interaction, "Trout")
 
             assert len(choices) == 2
-            assert choices[0].name == 'Mike Trout (CF)'
-            assert choices[0].value == 'Mike Trout'
-            assert choices[1].name == 'Ronald Acuna Jr. (OF)'
-            assert choices[1].value == 'Ronald Acuna Jr.'
+            assert choices[0].name == "Mike Trout (CF)"
+            assert choices[0].value == "Mike Trout"
+            assert choices[1].name == "Ronald Acuna Jr. (OF)"
+            assert choices[1].value == "Ronald Acuna Jr."
 
     @pytest.mark.asyncio
     async def test_player_autocomplete_with_team_info(self, mock_interaction):
         """Test player autocomplete with team information."""
-        mock_team = TeamFactory.create(id=499, abbrev='LAA', sname='Angels', lname='Los Angeles Angels')
+        mock_team = TeamFactory.create(
+            id=499, abbrev="LAA", sname="Angels", lname="Los Angeles Angels"
+        )
         mock_player = PlayerFactory.mike_trout(id=1)
         mock_player.team = mock_team
 
-        with patch('utils.autocomplete.player_service') as mock_service:
+        with patch("utils.autocomplete.player_service") as mock_service:
             mock_service.search_players = AsyncMock(return_value=[mock_player])
 
-            choices = await player_autocomplete(mock_interaction, 'Trout')
+            choices = await player_autocomplete(mock_interaction, "Trout")
 
             assert len(choices) == 1
-            assert choices[0].name == 'Mike Trout (CF - LAA)'
-            assert choices[0].value == 'Mike Trout'
+            assert choices[0].name == "Mike Trout (CF - LAA)"
+            assert choices[0].value == "Mike Trout"
 
     @pytest.mark.asyncio
     async def test_player_autocomplete_prioritizes_user_team(self, mock_interaction):
         """Test that user's team players are prioritized in autocomplete."""
-        user_team = TeamFactory.create(id=1, abbrev='POR', sname='Loggers')
-        other_team = TeamFactory.create(id=2, abbrev='LAA', sname='Angels')
+        user_team = TeamFactory.create(id=1, abbrev="POR", sname="Loggers")
+        other_team = TeamFactory.create(id=2, abbrev="LAA", sname="Angels")
 
         # Create players - one from user's team, one from other team
         user_player = PlayerFactory.mike_trout(id=1)
@@ -71,32 +86,35 @@ class TestPlayerAutocomplete:
         other_player.team = other_team
         other_player.team_id = other_team.id
 
-        with patch('utils.autocomplete.player_service') as mock_service, \
-             patch('utils.autocomplete.get_user_major_league_team') as mock_get_team:
-
-            mock_service.search_players = AsyncMock(return_value=[other_player, user_player])
+        with (
+            patch("utils.autocomplete.player_service") as mock_service,
+            patch("utils.autocomplete.get_user_major_league_team") as mock_get_team,
+        ):
+            mock_service.search_players = AsyncMock(
+                return_value=[other_player, user_player]
+            )
             mock_get_team.return_value = user_team
 
-            choices = await player_autocomplete(mock_interaction, 'player')
+            choices = await player_autocomplete(mock_interaction, "player")
 
             assert len(choices) == 2
             # User's team player should be first
-            assert choices[0].name == 'Mike Trout (CF - POR)'
-            assert choices[1].name == 'Ronald Acuna Jr. (OF - LAA)'
+            assert choices[0].name == "Mike Trout (CF - POR)"
+            assert choices[1].name == "Ronald Acuna Jr. (OF - LAA)"
 
     @pytest.mark.asyncio
     async def test_player_autocomplete_short_input(self, mock_interaction):
         """Test player autocomplete with short input returns empty."""
-        choices = await player_autocomplete(mock_interaction, 'T')
+        choices = await player_autocomplete(mock_interaction, "T")
         assert len(choices) == 0
 
     @pytest.mark.asyncio
     async def test_player_autocomplete_error_handling(self, mock_interaction):
         """Test player autocomplete error handling."""
-        with patch('utils.autocomplete.player_service') as mock_service:
+        with patch("utils.autocomplete.player_service") as mock_service:
             mock_service.search_players.side_effect = Exception("API Error")
 
-            choices = await player_autocomplete(mock_interaction, 'Trout')
+            choices = await player_autocomplete(mock_interaction, "Trout")
             assert len(choices) == 0
 
 
@@ -114,35 +132,35 @@ class TestTeamAutocomplete:
     async def test_team_autocomplete_success(self, mock_interaction):
         """Test successful team autocomplete."""
         mock_teams = [
-            TeamFactory.create(id=1, abbrev='LAA', sname='Angels'),
-            TeamFactory.create(id=2, abbrev='LAAMIL', sname='Salt Lake Bees'),
-            TeamFactory.create(id=3, abbrev='LAAAIL', sname='Angels IL'),
-            TeamFactory.create(id=4, abbrev='POR', sname='Loggers')
+            TeamFactory.create(id=1, abbrev="LAA", sname="Angels"),
+            TeamFactory.create(id=2, abbrev="LAAMIL", sname="Salt Lake Bees"),
+            TeamFactory.create(id=3, abbrev="LAAAIL", sname="Angels IL"),
+            TeamFactory.create(id=4, abbrev="POR", sname="Loggers"),
         ]
 
-        with patch('utils.autocomplete.team_service') as mock_service:
+        with patch("utils.autocomplete.team_service") as mock_service:
             mock_service.get_teams_by_season = AsyncMock(return_value=mock_teams)
 
-            choices = await team_autocomplete(mock_interaction, 'la')
+            choices = await team_autocomplete(mock_interaction, "la")
 
             assert len(choices) == 3  # All teams with 'la' in abbrev or sname
-            assert any('LAA' in choice.name for choice in choices)
-            assert any('LAAMIL' in choice.name for choice in choices)
-            assert any('LAAAIL' in choice.name for choice in choices)
+            assert any("LAA" in choice.name for choice in choices)
+            assert any("LAAMIL" in choice.name for choice in choices)
+            assert any("LAAAIL" in choice.name for choice in choices)
 
     @pytest.mark.asyncio
     async def test_team_autocomplete_short_input(self, mock_interaction):
         """Test team autocomplete with very short input."""
-        choices = await team_autocomplete(mock_interaction, '')
+        choices = await team_autocomplete(mock_interaction, "")
         assert len(choices) == 0
 
     @pytest.mark.asyncio
     async def test_team_autocomplete_error_handling(self, mock_interaction):
         """Test team autocomplete error handling."""
-        with patch('utils.autocomplete.team_service') as mock_service:
+        with patch("utils.autocomplete.team_service") as mock_service:
             mock_service.get_teams_by_season.side_effect = Exception("API Error")
 
-            choices = await team_autocomplete(mock_interaction, 'LAA')
+            choices = await team_autocomplete(mock_interaction, "LAA")
             assert len(choices) == 0
 
 
@@ -157,101 +175,197 @@ class TestMajorLeagueTeamAutocomplete:
         return interaction
 
     @pytest.mark.asyncio
-    async def test_major_league_team_autocomplete_filters_correctly(self, mock_interaction):
+    async def test_major_league_team_autocomplete_filters_correctly(
+        self, mock_interaction
+    ):
         """Test that only major league teams are returned."""
         # Create teams with different roster types
         mock_teams = [
-            TeamFactory.create(id=1, abbrev='LAA', sname='Angels'),  # ML
-            TeamFactory.create(id=2, abbrev='LAAMIL', sname='Salt Lake Bees'),  # MiL
-            TeamFactory.create(id=3, abbrev='LAAAIL', sname='Angels IL'),  # IL
-            TeamFactory.create(id=4, abbrev='FA', sname='Free Agents'),  # FA
-            TeamFactory.create(id=5, abbrev='POR', sname='Loggers'),  # ML
-            TeamFactory.create(id=6, abbrev='PORMIL', sname='Portland MiL'),  # MiL
+            TeamFactory.create(id=1, abbrev="LAA", sname="Angels"),  # ML
+            TeamFactory.create(id=2, abbrev="LAAMIL", sname="Salt Lake Bees"),  # MiL
+            TeamFactory.create(id=3, abbrev="LAAAIL", sname="Angels IL"),  # IL
+            TeamFactory.create(id=4, abbrev="FA", sname="Free Agents"),  # FA
+            TeamFactory.create(id=5, abbrev="POR", sname="Loggers"),  # ML
+            TeamFactory.create(id=6, abbrev="PORMIL", sname="Portland MiL"),  # MiL
         ]
 
-        with patch('utils.autocomplete.team_service') as mock_service:
+        with patch("utils.autocomplete.team_service") as mock_service:
             mock_service.get_teams_by_season = AsyncMock(return_value=mock_teams)
 
-            choices = await major_league_team_autocomplete(mock_interaction, 'l')
+            choices = await major_league_team_autocomplete(mock_interaction, "l")
 
             # Should only return major league teams that match 'l' (LAA, POR)
             choice_values = [choice.value for choice in choices]
-            assert 'LAA' in choice_values
-            assert 'POR' in choice_values
+            assert "LAA" in choice_values
+            assert "POR" in choice_values
             assert len(choice_values) == 2
             # Should NOT include MiL, IL, or FA teams
-            assert 'LAAMIL' not in choice_values
-            assert 'LAAAIL' not in choice_values
-            assert 'FA' not in choice_values
-            assert 'PORMIL' not in choice_values
+            assert "LAAMIL" not in choice_values
+            assert "LAAAIL" not in choice_values
+            assert "FA" not in choice_values
+            assert "PORMIL" not in choice_values
 
     @pytest.mark.asyncio
     async def test_major_league_team_autocomplete_matching(self, mock_interaction):
         """Test search matching on abbreviation and short name."""
         mock_teams = [
-            TeamFactory.create(id=1, abbrev='LAA', sname='Angels'),
-            TeamFactory.create(id=2, abbrev='LAD', sname='Dodgers'),
-            TeamFactory.create(id=3, abbrev='POR', sname='Loggers'),
-            TeamFactory.create(id=4, abbrev='BOS', sname='Red Sox'),
+            TeamFactory.create(id=1, abbrev="LAA", sname="Angels"),
+            TeamFactory.create(id=2, abbrev="LAD", sname="Dodgers"),
+            TeamFactory.create(id=3, abbrev="POR", sname="Loggers"),
+            TeamFactory.create(id=4, abbrev="BOS", sname="Red Sox"),
         ]
 
-        with patch('utils.autocomplete.team_service') as mock_service:
+        with patch("utils.autocomplete.team_service") as mock_service:
             mock_service.get_teams_by_season = AsyncMock(return_value=mock_teams)
 
             # Test abbreviation matching
-            choices = await major_league_team_autocomplete(mock_interaction, 'la')
+            choices = await major_league_team_autocomplete(mock_interaction, "la")
             assert len(choices) == 2  # LAA and LAD
             choice_values = [choice.value for choice in choices]
-            assert 'LAA' in choice_values
-            assert 'LAD' in choice_values
+            assert "LAA" in choice_values
+            assert "LAD" in choice_values
 
             # Test short name matching
-            choices = await major_league_team_autocomplete(mock_interaction, 'red')
+            choices = await major_league_team_autocomplete(mock_interaction, "red")
             assert len(choices) == 1
-            assert choices[0].value == 'BOS'
+            assert choices[0].value == "BOS"
 
     @pytest.mark.asyncio
     async def test_major_league_team_autocomplete_short_input(self, mock_interaction):
         """Test major league team autocomplete with very short input."""
-        choices = await major_league_team_autocomplete(mock_interaction, '')
+        choices = await major_league_team_autocomplete(mock_interaction, "")
         assert len(choices) == 0
 
     @pytest.mark.asyncio
-    async def test_major_league_team_autocomplete_error_handling(self, mock_interaction):
+    async def test_major_league_team_autocomplete_error_handling(
+        self, mock_interaction
+    ):
         """Test major league team autocomplete error handling."""
-        with patch('utils.autocomplete.team_service') as mock_service:
+        with patch("utils.autocomplete.team_service") as mock_service:
             mock_service.get_teams_by_season.side_effect = Exception("API Error")
 
-            choices = await major_league_team_autocomplete(mock_interaction, 'LAA')
+            choices = await major_league_team_autocomplete(mock_interaction, "LAA")
             assert len(choices) == 0
 
     @pytest.mark.asyncio
-    async def test_major_league_team_autocomplete_roster_type_detection(self, mock_interaction):
+    async def test_major_league_team_autocomplete_roster_type_detection(
+        self, mock_interaction
+    ):
         """Test that roster type detection works correctly for edge cases."""
         # Test edge cases like teams whose abbreviation ends in 'M' + 'IL'
         mock_teams = [
-            TeamFactory.create(id=1, abbrev='BHM', sname='Iron'),  # ML team ending in 'M'
-            TeamFactory.create(id=2, abbrev='BHMIL', sname='Iron IL'),  # IL team (BHM + IL)
-            TeamFactory.create(id=3, abbrev='NYYMIL', sname='Staten Island RailRiders'),  # MiL team (NYY + MIL)
-            TeamFactory.create(id=4, abbrev='NYY', sname='Yankees'),  # ML team
+            TeamFactory.create(
+                id=1, abbrev="BHM", sname="Iron"
+            ),  # ML team ending in 'M'
+            TeamFactory.create(
+                id=2, abbrev="BHMIL", sname="Iron IL"
+            ),  # IL team (BHM + IL)
+            TeamFactory.create(
+                id=3, abbrev="NYYMIL", sname="Staten Island RailRiders"
+            ),  # MiL team (NYY + MIL)
+            TeamFactory.create(id=4, abbrev="NYY", sname="Yankees"),  # ML team
         ]
 
-        with patch('utils.autocomplete.team_service') as mock_service:
+        with patch("utils.autocomplete.team_service") as mock_service:
             mock_service.get_teams_by_season = AsyncMock(return_value=mock_teams)
 
-            choices = await major_league_team_autocomplete(mock_interaction, 'b')
+            choices = await major_league_team_autocomplete(mock_interaction, "b")
 
             # Should only return major league teams
             choice_values = [choice.value for choice in choices]
-            assert 'BHM' in choice_values  # Major league team
-            assert 'BHMIL' not in choice_values  # Should be detected as IL, not MiL
-            assert 'NYYMIL' not in choice_values  # Minor league team
+            assert "BHM" in choice_values  # Major league team
+            assert "BHMIL" not in choice_values  # Should be detected as IL, not MiL
+            assert "NYYMIL" not in choice_values  # Minor league team
 
             # Verify the roster type detection is working
-            bhm_team = next(t for t in mock_teams if t.abbrev == 'BHM')
-            bhmil_team = next(t for t in mock_teams if t.abbrev == 'BHMIL')
-            nyymil_team = next(t for t in mock_teams if t.abbrev == 'NYYMIL')
+            bhm_team = next(t for t in mock_teams if t.abbrev == "BHM")
+            bhmil_team = next(t for t in mock_teams if t.abbrev == "BHMIL")
+            nyymil_team = next(t for t in mock_teams if t.abbrev == "NYYMIL")
 
             assert bhm_team.roster_type() == RosterType.MAJOR_LEAGUE
             assert bhmil_team.roster_type() == RosterType.INJURED_LIST
-            assert nyymil_team.roster_type() == RosterType.MINOR_LEAGUE
+            assert nyymil_team.roster_type() == RosterType.MINOR_LEAGUE
+
+
+class TestGetCachedUserTeam:
+    """Test the _get_cached_user_team caching helper.
+
+    Verifies that the cache avoids redundant get_user_major_league_team calls
+    on repeated invocations within the TTL window, and that expired entries are
+    re-fetched.
+    """
+
+    @pytest.fixture(autouse=True)
+    def clear_cache(self):
+        """Isolate each test from cache state left by other tests."""
+        utils.autocomplete._user_team_cache.clear()
+        yield
+        utils.autocomplete._user_team_cache.clear()
+
+    @pytest.fixture
+    def mock_interaction(self):
+        interaction = MagicMock()
+        interaction.user.id = 99999
+        return interaction
+
+    @pytest.mark.asyncio
+    async def test_caches_result_on_first_call(self, mock_interaction):
+        """First call populates the cache; API function called exactly once."""
+        user_team = TeamFactory.create(id=1, abbrev="POR", sname="Loggers")
+
+        with patch(
+            "utils.autocomplete.get_user_major_league_team", new_callable=AsyncMock
+        ) as mock_get_team:
+            mock_get_team.return_value = user_team
+
+            from utils.autocomplete import _get_cached_user_team
+
+            result1 = await _get_cached_user_team(mock_interaction)
+            result2 = await _get_cached_user_team(mock_interaction)
+
+            assert result1 is user_team
+            assert result2 is user_team
+            # API called only once despite two invocations
+            mock_get_team.assert_called_once_with(99999)
+
+    @pytest.mark.asyncio
+    async def test_re_fetches_after_ttl_expires(self, mock_interaction):
+        """Expired cache entries cause a fresh API call."""
+        import time
+
+        user_team = TeamFactory.create(id=1, abbrev="POR", sname="Loggers")
+
+        with patch(
+            "utils.autocomplete.get_user_major_league_team", new_callable=AsyncMock
+        ) as mock_get_team:
+            mock_get_team.return_value = user_team
+
+            from utils.autocomplete import _get_cached_user_team, _USER_TEAM_CACHE_TTL
+
+            # Seed the cache with a timestamp that is already expired
+            utils.autocomplete._user_team_cache[99999] = (
+                user_team,
+                time.time() - _USER_TEAM_CACHE_TTL - 1,
+            )
+
+            await _get_cached_user_team(mock_interaction)
+
+            # Should have called the API to refresh the stale entry
+            mock_get_team.assert_called_once_with(99999)
+
+    @pytest.mark.asyncio
+    async def test_caches_none_result(self, mock_interaction):
+        """None (user has no team) is cached to avoid repeated API calls."""
+        with patch(
+            "utils.autocomplete.get_user_major_league_team", new_callable=AsyncMock
+        ) as mock_get_team:
+            mock_get_team.return_value = None
+
+            from utils.autocomplete import _get_cached_user_team
+
+            result1 = await _get_cached_user_team(mock_interaction)
+            result2 = await _get_cached_user_team(mock_interaction)
+
+            assert result1 is None
+            assert result2 is None
+            mock_get_team.assert_called_once()

utils/autocomplete.py

@@ -4,16 +4,33 @@ Autocomplete Utilities
 Shared autocomplete functions for Discord slash commands.
 """
 
-from typing import List
+import time
+from typing import Dict, List, Optional, Tuple
 import discord
 from discord import app_commands
 
 from config import get_config
-from models.team import RosterType
+from models.team import RosterType, Team
 from services.player_service import player_service
 from services.team_service import team_service
 from utils.team_utils import get_user_major_league_team
 
+# Cache for user team lookups: user_id -> (team, cached_at)
+_user_team_cache: Dict[int, Tuple[Optional[Team], float]] = {}
+_USER_TEAM_CACHE_TTL = 60  # seconds
+
+
+async def _get_cached_user_team(interaction: discord.Interaction) -> Optional[Team]:
+    """Return the user's major league team, cached for 60 seconds per user."""
+    user_id = interaction.user.id
+    if user_id in _user_team_cache:
+        team, cached_at = _user_team_cache[user_id]
+        if time.time() - cached_at < _USER_TEAM_CACHE_TTL:
+            return team
+    team = await get_user_major_league_team(user_id)
+    _user_team_cache[user_id] = (team, time.time())
+    return team
+
 
 async def player_autocomplete(
     interaction: discord.Interaction, current: str
@@ -34,12 +51,12 @@ async def player_autocomplete(
         return []
 
     try:
-        # Get user's team for prioritization
-        user_team = await get_user_major_league_team(interaction.user.id)
+        # Get user's team for prioritization (cached per user, 60s TTL)
+        user_team = await _get_cached_user_team(interaction)
 
         # Search for players using the search endpoint
         players = await player_service.search_players(
-            current, limit=50, season=get_config().sba_season
+            current, limit=25, season=get_config().sba_season
         )
 
         # Separate players by team (user's team vs others)