fix: update evolution/cards endpoint to refractor/cards (#113) #114

Merged

cal merged 2 commits from issue/113-update-evolution-cards-endpoint-to-refractor-cards into main 2026-03-24 05:20:12 +00:00

Conversation 1 Commits 2 Files Changed 2 +3 -3

Claude commented 2026-03-23 20:31:51 +00:00

Collaborator

Copy Link

Closes #113

Summary

Updates the API endpoint in get_card_embeds() from evolution/cards/{card_id} to refractor/cards/{card_id} to match the database API rename.

Changes

• helpers/main.py line 118: evolution/cards/{card['id']} → refractor/cards/{card['id']}

Notes

The existing try/except fallback means card display won't hard-break if the endpoint is temporarily unavailable — badges will simply not appear. This PR should be merged after confirming the /refractor/cards/ endpoint is live on prod (pddev.manticorum.com / pd.manticorum.com).

Tests

No test suite changes required — single string constant change. Ruff lint passed.

Closes #113 ## Summary Updates the API endpoint in `get_card_embeds()` from `evolution/cards/{card_id}` to `refractor/cards/{card_id}` to match the database API rename. ## Changes - `helpers/main.py` line 118: `evolution/cards/{card['id']}` → `refractor/cards/{card['id']}` ## Notes The existing `try/except` fallback means card display won't hard-break if the endpoint is temporarily unavailable — badges will simply not appear. This PR should be merged after confirming the `/refractor/cards/` endpoint is live on prod (`pddev.manticorum.com` / `pd.manticorum.com`). ## Tests No test suite changes required — single string constant change. Ruff lint passed.

Claude added 1 commit 2026-03-23 20:31:52 +00:00

fix: update evolution/cards endpoint to refractor/cards (#113) ... 17bd1c085c

Closes #113

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Claude referenced this pull request 2026-03-23 20:31:58 +00:00

Update evolution/cards endpoint to refractor/cards in card embed helper #113

Claude added the

ai-reviewing

label 2026-03-23 20:45:46 +00:00

Claude reviewed 2026-03-23 20:47:02 +00:00

Claude left a comment

Author

Collaborator

Copy Link

AI Code Review

Files Reviewed

• helpers/main.py (modified)
• tests/test_card_embed_refractor.py (not in diff — reviewed for breakage)

Findings

Correctness

Blocking: stale endpoint string in test mocks

tests/test_card_embed_refractor.py was not included in this PR, but the production change breaks it. The mock routing in _patch_db_get still checks for the old endpoint:

# Line 74 — still routes "evolution/cards/"
if str(endpoint).startswith("evolution/cards/"):
    return evo_response

# Lines 207–208 — same stale string in _failing_db_get
if str(endpoint).startswith("evolution/cards/"):
    raise ConnectionError("API unreachable")

After this PR merges, get_card_embeds() calls db_get(f"refractor/cards/{card['id']}"). The mock never matches that string, falls through to return None, and evo_state is always None — meaning all badge-presence assertions fail silently:

• test_tier_one_badge — expects [BC] Mike Trout, gets Mike Trout
• test_tier_two_badge — expects [R] Mike Trout, gets Mike Trout
• test_tier_three_badge — expects [GR] Mike Trout, gets Mike Trout
• test_tier_four_superfractor_badge — expects [SF] Mike Trout, gets Mike Trout
• test_badge_prepended_to_player_name — expects [R] prefix, gets none
• test_fully_evolved_badge — expects [SF] prefix, gets none

The PR body states "No test suite changes required — single string constant change." This is incorrect; the mock strings are also constants tied to the endpoint.

Fix: Update both occurrences in tests/test_card_embed_refractor.py:

# Line 74
if str(endpoint).startswith("refractor/cards/"):

# Lines 207–208
if str(endpoint).startswith("refractor/cards/"):

Security

No issues found. The endpoint path contains only card['id'] which is an API-sourced integer, not user input.

Style & Conventions

No issues found. The change follows the existing db_get(f"<path>/<id>") pattern.

Suggestions

• The PR notes that merge should be contingent on confirming /refractor/cards/ is live on prod. Worth confirming this before merging, as the try/except silently suppresses the badge rather than surfacing a deployment mismatch.

Verdict: REQUEST_CHANGES

The production change is correct, but the test mock strings must be updated in the same PR. Without the fix, the test suite gives false confidence — all badge tests pass (because evo_state=None triggers the no-badge fallback) while the badge rendering is broken.

---

Automated review by Claude PR Reviewer (posted as COMMENT — Gitea blocks self-review)

## AI Code Review ### Files Reviewed - `helpers/main.py` (modified) - `tests/test_card_embed_refractor.py` (not in diff — reviewed for breakage) ### Findings #### Correctness **Blocking: stale endpoint string in test mocks** `tests/test_card_embed_refractor.py` was not included in this PR, but the production change breaks it. The mock routing in `_patch_db_get` still checks for the old endpoint: ```python # Line 74 — still routes "evolution/cards/" if str(endpoint).startswith("evolution/cards/"): return evo_response ``` ```python # Lines 207–208 — same stale string in _failing_db_get if str(endpoint).startswith("evolution/cards/"): raise ConnectionError("API unreachable") ``` After this PR merges, `get_card_embeds()` calls `db_get(f"refractor/cards/{card['id']}")`. The mock never matches that string, falls through to `return None`, and `evo_state` is always `None` — meaning all badge-presence assertions **fail silently**: - `test_tier_one_badge` — expects `[BC] Mike Trout`, gets `Mike Trout` - `test_tier_two_badge` — expects `[R] Mike Trout`, gets `Mike Trout` - `test_tier_three_badge` — expects `[GR] Mike Trout`, gets `Mike Trout` - `test_tier_four_superfractor_badge` — expects `[SF] Mike Trout`, gets `Mike Trout` - `test_badge_prepended_to_player_name` — expects `[R] ` prefix, gets none - `test_fully_evolved_badge` — expects `[SF] ` prefix, gets none The PR body states "No test suite changes required — single string constant change." This is incorrect; the mock strings are also constants tied to the endpoint. **Fix**: Update both occurrences in `tests/test_card_embed_refractor.py`: ```python # Line 74 if str(endpoint).startswith("refractor/cards/"): # Lines 207–208 if str(endpoint).startswith("refractor/cards/"): ``` #### Security No issues found. The endpoint path contains only `card['id']` which is an API-sourced integer, not user input. #### Style & Conventions No issues found. The change follows the existing `db_get(f"<path>/<id>")` pattern. #### Suggestions - The PR notes that merge should be contingent on confirming `/refractor/cards/` is live on prod. Worth confirming this before merging, as the `try/except` silently suppresses the badge rather than surfacing a deployment mismatch. ### Verdict: REQUEST_CHANGES The production change is correct, but the test mock strings must be updated in the same PR. Without the fix, the test suite gives false confidence — all badge tests pass (because `evo_state=None` triggers the no-badge fallback) while the badge rendering is broken. --- *Automated review by Claude PR Reviewer (posted as COMMENT — Gitea blocks self-review)*

Claude added

ai-changes-requested

and removed

ai-reviewing

labels 2026-03-23 20:47:27 +00:00

Claude added the

ai-working

label 2026-03-23 21:00:44 +00:00

cal referenced this issue from a commit 2026-03-23 21:01:25 +00:00

fix: update test mock endpoint strings to refractor/cards/ (#114)

cal added 1 commit 2026-03-23 21:01:25 +00:00

fix: update test mock endpoint strings to refractor/cards/ (#114) ... a4a3fbdac2

Mock routing in _patch_db_get and _failing_db_get still checked for
"evolution/cards/" after the production endpoint was renamed, causing
all badge-presence assertions to pass vacuously (evo_state=None).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Claude commented 2026-03-23 21:01:32 +00:00

Author

Collaborator

Copy Link

Addressed review feedback (commit a4a3fbd):

Fixed: Updated both stale "evolution/cards/" mock routing strings in tests/test_card_embed_refractor.py to "refractor/cards/":

• Line 74 (_patch_db_get side effect) — was falling through to return None, making evo_state always None and all badge-presence tests pass vacuously
• Line 207 (_failing_db_get) — same issue, ConnectionError was never raised for the new endpoint

The production change in helpers/main.py and both test mock strings are now consistent on refractor/cards/.

Addressed review feedback (commit a4a3fbd): **Fixed**: Updated both stale `"evolution/cards/"` mock routing strings in `tests/test_card_embed_refractor.py` to `"refractor/cards/"`: - Line 74 (`_patch_db_get` side effect) — was falling through to `return None`, making `evo_state` always `None` and all badge-presence tests pass vacuously - Line 207 (`_failing_db_get`) — same issue, ConnectionError was never raised for the new endpoint The production change in `helpers/main.py` and both test mock strings are now consistent on `refractor/cards/`.

Claude removed the

ai-working

ai-changes-requested

labels 2026-03-23 21:01:37 +00:00

cal added 1 commit 2026-03-23 21:05:22 +00:00

ci: add dev tag trigger to Docker build workflow ... 4dd204471f

Allows deploying to dev environment by pushing a "dev" tag.
Dev tags build with :dev Docker tag instead of :production.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Claude added the

ai-reviewing

label 2026-03-23 21:15:52 +00:00

Claude reviewed 2026-03-23 21:17:07 +00:00

Claude left a comment

Author

Collaborator

Copy Link

Review: APPROVED

Previous blocking issue resolved. Ready to merge.

What Changed Since Last Review

The blocking issue from the prior review has been fixed:

• tests/test_card_embed_refractor.py line 74: mock routing updated from "evolution/cards/" → "refractor/cards/" ✓
• tests/test_card_embed_refractor.py line 207: same fix ✓

Both mock routing occurrences are now consistent with the production endpoint. Badge-presence assertions will no longer pass vacuously.

Additional Notes

.gitea/workflows/docker-build.yml in diff — These changes (dev tag support) are already in main via merged PR #115. They'll be no-ops on merge. Not a blocker.

PR body stale — Body says "No test suite changes required" but the diff includes two test file changes. Minor documentation gap, not a functional issue.

Checklist

• Endpoint update correct (refractor/cards/{id})
• Test mocks route to correct endpoint
• Fallback behavior unchanged (try/except degrades gracefully)
• No new logic introduced
• Ruff lint passed (per PR body)

(Gitea blocks self-approval — posting as COMMENT. Approve manually to merge.)

## Review: APPROVED Previous blocking issue resolved. Ready to merge. ### What Changed Since Last Review The blocking issue from the prior review has been fixed: - `tests/test_card_embed_refractor.py` line 74: mock routing updated from `"evolution/cards/"` → `"refractor/cards/"` ✓ - `tests/test_card_embed_refractor.py` line 207: same fix ✓ Both mock routing occurrences are now consistent with the production endpoint. Badge-presence assertions will no longer pass vacuously. ### Additional Notes **`.gitea/workflows/docker-build.yml` in diff** — These changes (dev tag support) are already in `main` via merged PR #115. They'll be no-ops on merge. Not a blocker. **PR body stale** — Body says "No test suite changes required" but the diff includes two test file changes. Minor documentation gap, not a functional issue. ### Checklist - [x] Endpoint update correct (`refractor/cards/{id}`) - [x] Test mocks route to correct endpoint - [x] Fallback behavior unchanged (`try/except` degrades gracefully) - [x] No new logic introduced - [x] Ruff lint passed (per PR body) *(Gitea blocks self-approval — posting as COMMENT. Approve manually to merge.)*

Claude removed the

ai-reviewing

label 2026-03-23 21:17:34 +00:00

Claude added the

ai-reviewing

label 2026-03-23 21:45:51 +00:00

Claude reviewed 2026-03-23 21:46:55 +00:00

Claude left a comment

Author

Collaborator

Copy Link

AI Code Review

Files Reviewed

• helpers/main.py (modified)
• tests/test_card_embed_refractor.py (modified)
• .gitea/workflows/docker-build.yml (modified — no-op, already in main via PR #115)

Findings

Correctness

• helpers/main.py:118: evolution/cards/{card['id']} → refractor/cards/{card['id']} is correct and matches the database API rename.
• tests/test_card_embed_refractor.py:74 and :207: both mock side_effect checks updated to refractor/cards/ — tests correctly track the production change.
• docker-build.yml: dev tag support was already merged to main via PR #115; this hunk is a no-op on merge.
• The surrounding try/except Exception: pass at lines 123–124 ensures graceful degradation — if the endpoint is temporarily unavailable, badges are silently suppressed and the embed still renders correctly.
• Minor inaccuracy in the PR body: "No test suite changes required" — tests ARE updated (lines 74 and 207). The changes are correct and necessary; this is just a stale description.

Security

• No issues found. Single internal API endpoint string constant — no user input, no injection surface.

Style & Conventions

• No issues found. Follows existing patterns throughout.

Suggestions

• None.

Verdict: APPROVED

Clean, minimal, and correct. All three touch points (production code + two test mocks) are updated consistently. The docker-build.yml hunk is inert on merge. The try/except fallback guarantees no card embed regressions if prod cutover is delayed.

---

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `helpers/main.py` (modified) - `tests/test_card_embed_refractor.py` (modified) - `.gitea/workflows/docker-build.yml` (modified — no-op, already in main via PR #115) ### Findings #### Correctness - `helpers/main.py:118`: `evolution/cards/{card['id']}` → `refractor/cards/{card['id']}` is correct and matches the database API rename. - `tests/test_card_embed_refractor.py:74` and `:207`: both mock side_effect checks updated to `refractor/cards/` — tests correctly track the production change. - `docker-build.yml`: dev tag support was already merged to `main` via PR #115; this hunk is a no-op on merge. - The surrounding `try/except Exception: pass` at lines 123–124 ensures graceful degradation — if the endpoint is temporarily unavailable, badges are silently suppressed and the embed still renders correctly. - Minor inaccuracy in the PR body: "No test suite changes required" — tests ARE updated (lines 74 and 207). The changes are correct and necessary; this is just a stale description. #### Security - No issues found. Single internal API endpoint string constant — no user input, no injection surface. #### Style & Conventions - No issues found. Follows existing patterns throughout. #### Suggestions - None. ### Verdict: APPROVED Clean, minimal, and correct. All three touch points (production code + two test mocks) are updated consistently. The docker-build.yml hunk is inert on merge. The try/except fallback guarantees no card embed regressions if prod cutover is delayed. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-23 21:47:08 +00:00

cal force-pushed issue/113-update-evolution-cards-endpoint-to-refractor-cards from 4dd204471f to fddac59f7e 2026-03-24 05:19:36 +00:00 Compare

cal referenced this issue from a commit 2026-03-24 05:19:36 +00:00

fix: update test mock endpoint strings to refractor/cards/ (#114)

cal approved these changes 2026-03-24 05:19:56 +00:00

cal left a comment

Owner

Copy Link

Reviewed. The functional change is a one-line endpoint rename in helpers/main.py (evolution/cards/ → refractor/cards/) and the matching test mock update in test_card_embed_refractor.py. Both are correct and consistent with the database rename that landed in DB PR #131. The workflow file changes are already on main from PRs #115/#116 — the diff shows them because the merge base predates those, but git confirms no actual diff on that file vs main. Approved.

Reviewed. The functional change is a one-line endpoint rename in `helpers/main.py` (evolution/cards/ → refractor/cards/) and the matching test mock update in `test_card_embed_refractor.py`. Both are correct and consistent with the database rename that landed in DB PR #131. The workflow file changes are already on main from PRs #115/#116 — the diff shows them because the merge base predates those, but git confirms no actual diff on that file vs main. Approved.

cal merged commit 55efdb39de into main 2026-03-24 05:20:12 +00:00

cal deleted branch issue/113-update-evolution-cards-endpoint-to-refractor-cards 2026-03-24 05:20:13 +00:00

cal referenced this issue from a commit 2026-03-24 05:20:13 +00:00

Merge pull request 'fix: update evolution/cards endpoint to refractor/cards (#113)' (#114) from issue/113-update-evolution-cards-endpoint-to-refractor-cards into main

Sign in to join this conversation.

Reviewers

No reviewers

cal

Labels

Clear labels   

ai-changes-requested

  

ai-failed

  

ai-pr-opened

  

ai-reviewed

  

ai-reviewing

  

ai-working

  

ai-working

  

bug

  

enhancement

  

feature

New feature proposal

  

in-queue

Approved for development

  

performance

  

security

  

tech-debt

  

todo

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

Milestone

Clear milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

No Assignees

2 Participants

Notifications

Subscribe

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#114

No description provided.