feat: refractor card art pipeline — render trigger + /player view #139
No reviewers
Labels
No Label
ai-changes-requested
ai-failed
ai-pr-opened
ai-reviewed
ai-reviewing
ai-working
ai-working
bug
enhancement
feature
in-queue
performance
security
tech-debt
todo
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: cal/paper-dynasty-discord#139
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "feat/refractor-card-art-pipeline"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
/player <name> refractor_tier:<int>— view evolved card images with fallback UXDetails
Post-game trigger: After
evaluate-gamereturns tier-ups, the bot fires GET requests to the card render endpoint for each new variant card. Fire-and-forget — failures are logged but never raised./playerrefractor view: New optional parameter. When provided:Test plan
pytest tests/test_post_game_render.py— 4 passedpytest tests/test_player_refractor_view.py— 5 passedpytest tests/test_refractor_commands.py— 61 passed (no regressions)pytest tests/test_refractor_notifs.py— 25 passed (no regressions)Deploy note
Depends on database API PR being deployed first (needs image_url in refractor response + S3 upload on render).
🤖 Generated with Claude Code
AI Code Review
Files Reviewed
cogs/players.py(modified — refractor view helper +/playercommand update)command_logic/logic_gameplay.py(modified — post-game render trigger)tests/test_post_game_render.py(added)tests/test_player_refractor_view.py(added)Findings
Correctness
Bug:
variant_created: 0is silently skipped in_trigger_variant_rendersIn
command_logic/logic_gameplay.py, the render trigger guard is:if not variantevaluates toTruewhenvariant_createdis0(falsy integer), so a tier-up that produces variant 0 would never trigger a render. The correct guard is an explicitNonecheck:Whether variant 0 is a meaningful value in practice depends on the API contract, but
if not variantis fragile regardless. The test suite only exercises variants 7 and 3, so this edge case is not currently caught.Correctness note:
refractor/cardspagination is hard-capped at 100In
player_slash_command, the refractor lookup uses("limit", 100). A team with more than 100 refractor cards will silently return incomplete data, meaning a valid match could be missed and the user shown a "not found" response. This is a product call (whether a hard cap is acceptable), but worth an intentional decision.Security
DB_URLimport fromapi_callsis appropriate.Style & Conventions
players.pyis consistent with the rest of the file and the project's Ruff config.REFRACTOR_TIER_NAMESand_build_refractor_responseare cleanly separated at module level, consistent with existing patterns.teamparameter is accepted by_build_refractor_responsebut never used inside the function. This is a dead parameter — either remove it from the signature and call sites, or note it in the docstring as reserved.Suggestions
T{refractor_tier} refractorbutresult['tier_name']contains the proper human name (e.g., "Gold Refractor"). Using the tier name in the user-facing message would be more consistent with the embed title format.test_player_refractor_view.pyusessys.path.insertfor imports. Worth verifying that the existingpytest.iniconfiguration already handles path resolution so the manual hack is not needed.Verdict: REQUEST_CHANGES (advisory — Gitea blocked formal review on own PR)
One concrete bug should be fixed before merge: the
if not variantguard in_trigger_variant_renderswill silently skip variant 0 tier-ups. This is a one-line fix. The 100-card pagination cap and the deadteamparameter are worth addressing, but are not blocking on their own.Automated review by Claude PR Reviewer