From 1c47928356f54e42debd1ef5ff6c9dbc10b98799 Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Sat, 11 Apr 2026 12:07:35 -0500 Subject: [PATCH 1/2] fix(dev_tools): use unified cards/{id} endpoint for /dev refractor-test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous two-step battingcards→pitchingcards fallback caused card ID collisions — e.g. card 494 resolving to Cameron Maybin (batting) instead of the intended pitcher Grayson Rodriguez. The unified cards endpoint is keyed on globally-unique card instance IDs and carries player, team, and variant in a single response. - Single db_get("cards", object_id=card_id) lookup - Card type derived from player.pos_1 (SP→sp, RP/CP→rp, else→batter) - team_id sourced from card["team"]["id"] (no get_team_by_owner fallback) - TestRefractorTestSetup rewritten for the single-endpoint contract Spec: docs/superpowers/specs/2026-04-11-refractor-test-unified-cards-lookup-design.md --- cogs/dev_tools.py | 119 +++++++++-------- tests/test_dev_tools.py | 289 ++++++++++++++++++++++++++++------------ 2 files changed, 265 insertions(+), 143 deletions(-) diff --git a/cogs/dev_tools.py b/cogs/dev_tools.py index f36b850..d996722 100644 --- a/cogs/dev_tools.py +++ b/cogs/dev_tools.py @@ -11,9 +11,10 @@ import discord from discord import app_commands from discord.ext import commands -from api_calls import db_delete, db_get, db_post +import aiohttp + +from api_calls import AUTH_TOKEN, db_delete, db_get, db_post, get_req_url from helpers.constants import PD_SEASON -from helpers.main import get_team_by_owner from helpers.refractor_constants import TIER_NAMES from helpers.refractor_test_data import ( build_batter_plays, @@ -25,7 +26,9 @@ from helpers.refractor_test_data import ( CURRENT_SEASON = PD_SEASON -logger = logging.getLogger(__name__) +SENTINEL_PITCHER_ID = 3 + +logger = logging.getLogger("discord_app") class CleanupView(discord.ui.View): @@ -110,31 +113,38 @@ class DevToolsCog(commands.Cog): await interaction.response.defer() # --- Phase 1: Setup --- - # Look up card (try batting first, then pitching) - card = await db_get("battingcards", object_id=card_id) - card_type_key = "batting" - if card is None: - card = await db_get("pitchingcards", object_id=card_id) - card_type_key = "pitching" - + # Look up card via the unified cards endpoint. cards.id is + # globally unique across all card instances, so there's no + # batting/pitching template-ID collision (spec + # 2026-04-11-refractor-test-unified-cards-lookup-design.md). + card = await db_get("cards", object_id=card_id) if card is None: await interaction.edit_original_response( - content=f"❌ Card #{card_id} not found (checked batting and pitching)." + content=f"❌ Card #{card_id} not found." ) return - player_id = card["player"]["id"] + player_id = card["player"]["player_id"] player_name = card["player"]["p_name"] - team_id = card.get("team_id") or card["player"].get("team_id") + # The card instance's owning team is the authoritative context + # for this test — its refractor state will be read and synthetic + # plays will be credited against it. + team_id = (card.get("team") or {}).get("id") if team_id is None: - team = await get_team_by_owner(interaction.user.id) - if team is None: - await interaction.edit_original_response( - content="❌ Could not determine team ID. You must own a team." - ) - return - team_id = team["id"] + await interaction.edit_original_response( + content=f"❌ Card #{card_id} has no owning team." + ) + return + + # Derive card type from the player's primary position. + pos_1 = (card["player"].get("pos_1") or "").upper() + if pos_1 == "SP": + card_type_key, card_type_default = "pitching", "sp" + elif pos_1 in ("RP", "CP"): + card_type_key, card_type_default = "pitching", "rp" + else: + card_type_key, card_type_default = "batting", "batter" # Fetch refractor state refractor_data = await db_get( @@ -142,6 +152,17 @@ class DevToolsCog(commands.Cog): params=[("team_id", team_id), ("limit", 100)], ) + logger.info( + "dev_tools state-lookup: team_id=%s player_id=%s resp_keys=%s item_count=%s item_pids=%s", + team_id, + player_id, + list(refractor_data.keys()) if refractor_data else None, + len(refractor_data.get("items", [])) if refractor_data else 0, + [i.get("player_id") for i in refractor_data.get("items", [])] + if refractor_data + else [], + ) + # Find this player's entry card_state = None if refractor_data and refractor_data.get("items"): @@ -150,6 +171,12 @@ class DevToolsCog(commands.Cog): card_state = item break + logger.info( + "dev_tools state-lookup matched: player_id=%s card_state=%s", + player_id, + card_state, + ) + # Determine current state and thresholds if card_state: current_tier = card_state["current_tier"] @@ -159,7 +186,7 @@ class DevToolsCog(commands.Cog): else: current_tier = 0 current_value = 0 - card_type = "batter" if card_type_key == "batting" else "sp" + card_type = card_type_default next_threshold = ( 37 if card_type == "batter" else (10 if card_type == "sp" else 3) ) @@ -174,25 +201,7 @@ class DevToolsCog(commands.Cog): gap = max(0, next_threshold - current_value) plan = calculate_plays_needed(gap, card_type) - # Find an opposing player - if card_type == "batter": - opposing_cards = await db_get( - "pitchingcards", - params=[("team_id", team_id), ("variant", 0)], - ) - else: - opposing_cards = await db_get( - "battingcards", - params=[("team_id", team_id), ("variant", 0)], - ) - - if not opposing_cards or not opposing_cards.get("cards"): - await interaction.edit_original_response( - content=f"❌ No opposing {'pitcher' if card_type == 'batter' else 'batter'} cards found on team {team_id}." - ) - return - - opposing_player_id = opposing_cards["cards"][0]["player"]["id"] + opposing_player_id = SENTINEL_PITCHER_ID # Build and send initial embed tier_name = TIER_NAMES.get(current_tier, f"T{current_tier}") @@ -357,27 +366,19 @@ class DevToolsCog(commands.Cog): continue try: today = date.today().isoformat() - render_resp = await db_get( + render_url = get_req_url( f"players/{tu['player_id']}/{card_type_key}card/{today}/{variant}", - none_okay=True, + api_ver=2, ) - if render_resp: - results.append("✅ Card rendered + S3 upload triggered") - img_url = ( - render_resp - if isinstance(render_resp, str) - else render_resp.get("image_url") - ) - if ( - img_url - and isinstance(img_url, str) - and img_url.startswith("http") - ): - embed.set_image(url=img_url) - else: - results.append( - "⚠️ Card render returned no data (may still be processing)" - ) + async with aiohttp.ClientSession(headers=AUTH_TOKEN) as sess: + async with sess.get(render_url) as r: + if r.status == 200: + results.append("✅ Card rendered + S3 upload triggered") + else: + body = await r.text() + results.append( + f"⚠️ Card render non-200 ({r.status}): {body[:80]}" + ) except Exception as e: results.append(f"⚠️ Card render failed (non-fatal): {e}") diff --git a/tests/test_dev_tools.py b/tests/test_dev_tools.py index ca04ec3..96bcae6 100644 --- a/tests/test_dev_tools.py +++ b/tests/test_dev_tools.py @@ -100,13 +100,34 @@ class TestRefractorTestSetup: return MagicMock(spec=commands.Bot) @pytest.fixture - def batting_card_response(self): - return { - "id": 1234, - "player": {"id": 100, "p_name": "Mike Trout"}, - "variant": 0, - "image_url": None, - } + def unified_card_response(self): + """Factory for a response from GET /v2/cards/{id}. + + The unified cards endpoint returns a card-instance record with + top-level player, team, pack, value, and variant fields. This + replaces the separate battingcards/pitchingcards template endpoints + that previously caused ID collisions (see spec + 2026-04-11-refractor-test-unified-cards-lookup-design.md). + + The factory lets each test customize pos_1 and the IDs without + duplicating the full response shape. + """ + + def _make(pos_1="CF", player_id=100, team_id=31, card_id=1234): + return { + "id": card_id, + "player": { + "player_id": player_id, + "p_name": "Mike Trout", + "pos_1": pos_1, + }, + "team": {"id": team_id}, + "variant": 0, + "pack": None, + "value": None, + } + + return _make @pytest.fixture def refractor_cards_response(self): @@ -134,112 +155,212 @@ class TestRefractorTestSetup: ], } - @pytest.fixture - def opposing_cards_response(self): - """A valid pitching cards response with the 'cards' key.""" - return { - "cards": [ - { - "id": 9000, - "player": {"id": 200, "p_name": "Clayton Kershaw"}, - "variant": 0, - } - ] - } - - async def test_batting_card_lookup( + async def test_unified_card_lookup( self, mock_interaction, mock_bot, - batting_card_response, + unified_card_response, refractor_cards_response, - opposing_cards_response, ): - """Command should try the batting card endpoint first. + """The setup phase should make a single db_get call targeting + the unified 'cards' endpoint. - Verifies that the first db_get call targets 'battingcards', not - 'pitchingcards', when looking up a card ID. + Regression guard for the previous two-step battingcards/pitchingcards + fallback that caused ID collisions (e.g. card 494 resolving to + Cameron Maybin instead of the intended pitcher Grayson Rodriguez). """ from cogs.dev_tools import DevToolsCog cog = DevToolsCog(mock_bot) - with ( - patch("cogs.dev_tools.db_get", new_callable=AsyncMock) as mock_get, - patch("cogs.dev_tools.db_post", new_callable=AsyncMock), - ): - mock_get.side_effect = [ - batting_card_response, # GET battingcards/{id} - refractor_cards_response, # GET refractor/cards - opposing_cards_response, # GET pitchingcards (for opposing player) - ] - with patch.object(cog, "_execute_refractor_test", new_callable=AsyncMock): - await cog.refractor_test.callback(cog, mock_interaction, card_id=1234) - first_call = mock_get.call_args_list[0] - assert "battingcards" in str(first_call) - - async def test_pitching_card_fallback( - self, - mock_interaction, - mock_bot, - refractor_cards_response, - ): - """If batting card returns None, command should fall back to pitching card. - - Ensures the two-step lookup: batting first, then pitching if batting - returns None. The second db_get call must target 'pitchingcards'. - """ - from cogs.dev_tools import DevToolsCog - - cog = DevToolsCog(mock_bot) - pitching_card = { - "id": 5678, - "player": {"id": 200, "p_name": "Clayton Kershaw"}, - "variant": 0, - "image_url": None, - } - refractor_cards_response["items"][0]["player_id"] = 200 - refractor_cards_response["items"][0]["track"]["card_type"] = "sp" - refractor_cards_response["items"][0]["next_threshold"] = 10 - - opposing_batters = { - "cards": [ - {"id": 7000, "player": {"id": 300, "p_name": "Babe Ruth"}, "variant": 0} - ] - } + card = unified_card_response(pos_1="CF", player_id=100, card_id=1234) with ( patch("cogs.dev_tools.db_get", new_callable=AsyncMock) as mock_get, patch("cogs.dev_tools.db_post", new_callable=AsyncMock), ): - mock_get.side_effect = [ - None, # batting card not found - pitching_card, # pitching card found - refractor_cards_response, # refractor/cards - opposing_batters, # battingcards for opposing player - ] + mock_get.side_effect = [card, refractor_cards_response] with patch.object(cog, "_execute_refractor_test", new_callable=AsyncMock): - await cog.refractor_test.callback(cog, mock_interaction, card_id=5678) - second_call = mock_get.call_args_list[1] - assert "pitchingcards" in str(second_call) + await cog.refractor_test.callback(cog, mock_interaction, 1234) + + # First call: the unified cards endpoint + first_call = mock_get.call_args_list[0] + assert first_call.args[0] == "cards" + assert first_call.kwargs.get("object_id") == 1234 + + # No secondary template-table fallback + endpoints_called = [c.args[0] for c in mock_get.call_args_list] + assert "battingcards" not in endpoints_called + assert "pitchingcards" not in endpoints_called async def test_card_not_found_reports_error(self, mock_interaction, mock_bot): - """If neither batting nor pitching card exists, report an error and return. - - The command should call edit_original_response with a message containing - 'not found' and must NOT call _execute_refractor_test. + """If cards/{id} returns None, report 'not found' and never call + _execute_refractor_test. The single unified endpoint means only + one db_get is made before the error path. """ from cogs.dev_tools import DevToolsCog cog = DevToolsCog(mock_bot) - with patch("cogs.dev_tools.db_get", new_callable=AsyncMock, return_value=None): + with patch( + "cogs.dev_tools.db_get", new_callable=AsyncMock, return_value=None + ) as mock_get: with patch.object( cog, "_execute_refractor_test", new_callable=AsyncMock ) as mock_exec: - await cog.refractor_test.callback(cog, mock_interaction, card_id=9999) + await cog.refractor_test.callback(cog, mock_interaction, 9999) mock_exec.assert_not_called() + + # Exactly one db_get call — the unified lookup, no template fallback + assert mock_get.call_count == 1 call_kwargs = mock_interaction.edit_original_response.call_args[1] assert "not found" in call_kwargs["content"].lower() + async def test_pos_sp_derives_sp_type( + self, + mock_interaction, + mock_bot, + unified_card_response, + refractor_cards_response, + ): + """pos_1='SP' should derive card_type='sp', card_type_key='pitching' + and pass those into _execute_refractor_test. + """ + from cogs.dev_tools import DevToolsCog + + cog = DevToolsCog(mock_bot) + card = unified_card_response(pos_1="SP", player_id=200) + # Make sure the refractor/cards lookup finds no matching entry, + # so the command falls through to the pos_1-derived defaults. + refractor_cards_response["items"] = [] + + with ( + patch("cogs.dev_tools.db_get", new_callable=AsyncMock) as mock_get, + patch("cogs.dev_tools.db_post", new_callable=AsyncMock), + patch.object( + cog, "_execute_refractor_test", new_callable=AsyncMock + ) as mock_exec, + ): + mock_get.side_effect = [card, refractor_cards_response] + await cog.refractor_test.callback(cog, mock_interaction, 1234) + + kwargs = mock_exec.call_args.kwargs + assert kwargs["card_type"] == "sp" + assert kwargs["card_type_key"] == "pitching" + + async def test_pos_rp_derives_rp_type( + self, + mock_interaction, + mock_bot, + unified_card_response, + refractor_cards_response, + ): + """pos_1='RP' should derive card_type='rp', card_type_key='pitching'.""" + from cogs.dev_tools import DevToolsCog + + cog = DevToolsCog(mock_bot) + card = unified_card_response(pos_1="RP", player_id=201) + refractor_cards_response["items"] = [] + + with ( + patch("cogs.dev_tools.db_get", new_callable=AsyncMock) as mock_get, + patch("cogs.dev_tools.db_post", new_callable=AsyncMock), + patch.object( + cog, "_execute_refractor_test", new_callable=AsyncMock + ) as mock_exec, + ): + mock_get.side_effect = [card, refractor_cards_response] + await cog.refractor_test.callback(cog, mock_interaction, 1234) + + kwargs = mock_exec.call_args.kwargs + assert kwargs["card_type"] == "rp" + assert kwargs["card_type_key"] == "pitching" + + async def test_pos_cp_derives_rp_type( + self, + mock_interaction, + mock_bot, + unified_card_response, + refractor_cards_response, + ): + """pos_1='CP' (closer) should also map to card_type='rp'.""" + from cogs.dev_tools import DevToolsCog + + cog = DevToolsCog(mock_bot) + card = unified_card_response(pos_1="CP", player_id=202) + refractor_cards_response["items"] = [] + + with ( + patch("cogs.dev_tools.db_get", new_callable=AsyncMock) as mock_get, + patch("cogs.dev_tools.db_post", new_callable=AsyncMock), + patch.object( + cog, "_execute_refractor_test", new_callable=AsyncMock + ) as mock_exec, + ): + mock_get.side_effect = [card, refractor_cards_response] + await cog.refractor_test.callback(cog, mock_interaction, 1234) + + kwargs = mock_exec.call_args.kwargs + assert kwargs["card_type"] == "rp" + assert kwargs["card_type_key"] == "pitching" + + async def test_pos_batter_derives_batter_type( + self, + mock_interaction, + mock_bot, + unified_card_response, + refractor_cards_response, + ): + """pos_1='CF' (or any non-pitcher position) should derive + card_type='batter', card_type_key='batting'. + """ + from cogs.dev_tools import DevToolsCog + + cog = DevToolsCog(mock_bot) + card = unified_card_response(pos_1="CF", player_id=203) + refractor_cards_response["items"] = [] + + with ( + patch("cogs.dev_tools.db_get", new_callable=AsyncMock) as mock_get, + patch("cogs.dev_tools.db_post", new_callable=AsyncMock), + patch.object( + cog, "_execute_refractor_test", new_callable=AsyncMock + ) as mock_exec, + ): + mock_get.side_effect = [card, refractor_cards_response] + await cog.refractor_test.callback(cog, mock_interaction, 1234) + + kwargs = mock_exec.call_args.kwargs + assert kwargs["card_type"] == "batter" + assert kwargs["card_type_key"] == "batting" + + async def test_card_without_team_reports_error( + self, + mock_interaction, + mock_bot, + unified_card_response, + ): + """If the unified card response has team=None, the command should + report an error and not proceed to execute the refractor chain. + + The card instance's owning team is now the authoritative team + context for the test (see spec — option A: card's team is + authoritative, no get_team_by_owner fallback). + """ + from cogs.dev_tools import DevToolsCog + + cog = DevToolsCog(mock_bot) + card = unified_card_response(pos_1="CF", player_id=100) + card["team"] = None + + with patch("cogs.dev_tools.db_get", new_callable=AsyncMock, return_value=card): + with patch.object( + cog, "_execute_refractor_test", new_callable=AsyncMock + ) as mock_exec: + await cog.refractor_test.callback(cog, mock_interaction, 1234) + mock_exec.assert_not_called() + + call_kwargs = mock_interaction.edit_original_response.call_args[1] + assert "no owning team" in call_kwargs["content"].lower() + class TestRefractorTestExecute: """Test the execution phase: API calls, step-by-step reporting, -- 2.25.1 From c8424e6cb116e01ca66ecbe32ae354f1a2dca8ce Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Sat, 11 Apr 2026 12:19:46 -0500 Subject: [PATCH 2/2] docs(dev_tools): update refractor-test card_id describe string Review follow-up: the @app_commands.describe string still referenced "batting or pitching card ID" after the switch to the unified cards endpoint. Update to clarify that card_id is now a card-instance ID. Co-Authored-By: Claude Opus 4.6 (1M context) --- cogs/dev_tools.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cogs/dev_tools.py b/cogs/dev_tools.py index d996722..e724184 100644 --- a/cogs/dev_tools.py +++ b/cogs/dev_tools.py @@ -108,7 +108,9 @@ class DevToolsCog(commands.Cog): @group_dev.command( name="refractor-test", description="Run refractor integration test on a card" ) - @app_commands.describe(card_id="The batting or pitching card ID to test") + @app_commands.describe( + card_id="Card-instance ID (from the unified cards table; discoverable via /refractor status)" + ) async def refractor_test(self, interaction: discord.Interaction, card_id: int): await interaction.response.defer() -- 2.25.1