feat: refractor card art pipeline — render trigger + /player view #139

Merged
cal merged 3 commits from feat/refractor-card-art-pipeline into main 2026-04-06 22:34:02 +00:00
Owner

Summary

  • Post-game render trigger: after tier-ups, bot hits card render URLs to warm S3 cache
  • /player <name> refractor_tier:<int> — view evolved card images with fallback UX

Details

Post-game trigger: After evaluate-game returns 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.

/player refractor view: New optional parameter. When provided:

  • Happy path: shows the refractor card image from S3
  • Image not yet rendered: defers response, uses API render URL directly as embed image
  • Not found: shows "You don't have that refractor" + top 5 refractor cards as clickable S3 links

Test plan

  • pytest tests/test_post_game_render.py — 4 passed
  • pytest tests/test_player_refractor_view.py — 5 passed
  • pytest 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

## Summary - Post-game render trigger: after tier-ups, bot hits card render URLs to warm S3 cache - `/player <name> refractor_tier:<int>` — view evolved card images with fallback UX ## Details **Post-game trigger:** After `evaluate-game` returns 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. **`/player` refractor view:** New optional parameter. When provided: - Happy path: shows the refractor card image from S3 - Image not yet rendered: defers response, uses API render URL directly as embed image - Not found: shows "You don't have that refractor" + top 5 refractor cards as clickable S3 links ## Test plan - [x] `pytest tests/test_post_game_render.py` — 4 passed - [x] `pytest tests/test_player_refractor_view.py` — 5 passed - [x] `pytest tests/test_refractor_commands.py` — 61 passed (no regressions) - [x] `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](https://claude.com/claude-code)
cal added 2 commits 2026-04-06 22:25:36 +00:00
After refractor tier-ups, the bot hits the card render URL for each
variant card to trigger Playwright render + S3 upload as a side
effect. Fire-and-forget — failures are logged but never raised.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
feat: add /player refractor_tier parameter for viewing evolved cards
All checks were successful
Ruff Lint / lint (pull_request) Successful in 13s
46744d139c
Adds optional refractor_tier parameter to the /player slash command.
When provided: looks up the user's team refractor data, shows the
evolved card image if available, triggers on-demand render if image
not yet generated, or shows top 5 refractor cards as fallback.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cal reviewed 2026-04-06 22:28:57 +00:00
cal left a comment
Author
Owner

AI Code Review

Files Reviewed

  • cogs/players.py (modified — refractor view helper + /player command 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: 0 is silently skipped in _trigger_variant_renders

In command_logic/logic_gameplay.py, the render trigger guard is:

variant = tier_up.get("variant_created")
if not variant:
    continue

if not variant evaluates to True when variant_created is 0 (falsy integer), so a tier-up that produces variant 0 would never trigger a render. The correct guard is an explicit None check:

variant = tier_up.get("variant_created")
if variant is None:
    continue

Whether variant 0 is a meaningful value in practice depends on the API contract, but if not variant is fragile regardless. The test suite only exercises variants 7 and 3, so this edge case is not currently caught.

Correctness note: refractor/cards pagination is hard-capped at 100

In 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

  • No issues found. The render URL is constructed from integer player IDs and variant numbers sourced from API response data — no user-controlled string interpolation.
  • DB_URL import from api_calls is appropriate.

Style & Conventions

  • f-string quote style cleanup throughout players.py is consistent with the rest of the file and the project's Ruff config.
  • REFRACTOR_TIER_NAMES and _build_refractor_response are cleanly separated at module level, consistent with existing patterns.
  • The team parameter is accepted by _build_refractor_response but 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

  • The "not found" message says T{refractor_tier} refractor but result['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.py uses sys.path.insert for imports. Worth verifying that the existing pytest.ini configuration 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 variant guard in _trigger_variant_renders will silently skip variant 0 tier-ups. This is a one-line fix. The 100-card pagination cap and the dead team parameter are worth addressing, but are not blocking on their own.


Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `cogs/players.py` (modified — refractor view helper + `/player` command 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: 0` is silently skipped in `_trigger_variant_renders`** In `command_logic/logic_gameplay.py`, the render trigger guard is: ```python variant = tier_up.get("variant_created") if not variant: continue ``` `if not variant` evaluates to `True` when `variant_created` is `0` (falsy integer), so a tier-up that produces variant 0 would never trigger a render. The correct guard is an explicit `None` check: ```python variant = tier_up.get("variant_created") if variant is None: continue ``` Whether variant 0 is a meaningful value in practice depends on the API contract, but `if not variant` is fragile regardless. The test suite only exercises variants 7 and 3, so this edge case is not currently caught. **Correctness note: `refractor/cards` pagination is hard-capped at 100** In `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 - No issues found. The render URL is constructed from integer player IDs and variant numbers sourced from API response data — no user-controlled string interpolation. - `DB_URL` import from `api_calls` is appropriate. #### Style & Conventions - f-string quote style cleanup throughout `players.py` is consistent with the rest of the file and the project's Ruff config. - `REFRACTOR_TIER_NAMES` and `_build_refractor_response` are cleanly separated at module level, consistent with existing patterns. - The `team` parameter is accepted by `_build_refractor_response` but 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 - The "not found" message says `T{refractor_tier} refractor` but `result['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.py` uses `sys.path.insert` for imports. Worth verifying that the existing `pytest.ini` configuration 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 variant` guard in `_trigger_variant_renders` will silently skip variant 0 tier-ups. This is a one-line fix. The 100-card pagination cap and the dead `team` parameter are worth addressing, but are not blocking on their own. --- *Automated review by Claude PR Reviewer*
cal added 1 commit 2026-04-06 22:33:44 +00:00
fix: review feedback — variant 0 guard, remove dead team param
All checks were successful
Ruff Lint / lint (pull_request) Successful in 11s
78f313663e
- Use `variant is None` instead of `not variant` to avoid skipping
  variant 0 tier-ups (0 is falsy in Python)
- Remove unused `team` parameter from _build_refractor_response
- Update tests to match

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cal merged commit 5cfddaa89a into main 2026-04-06 22:34:02 +00:00
cal deleted branch feat/refractor-card-art-pipeline 2026-04-06 22:34:02 +00:00
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cal/paper-dynasty-discord#139
No description provided.