fix(dev_tools): use unified cards/{id} endpoint for /dev refractor-test
All checks were successful
Ruff Lint / lint (pull_request) Successful in 27s

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
This commit is contained in:
Cal Corum 2026-04-11 12:07:35 -05:00
parent 2ce6bbe57a
commit 1c47928356
2 changed files with 265 additions and 143 deletions

View File

@ -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}")

View File

@ -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,