cal/paper-dynasty-database
1
0
Fork 0
Code Issues 10 Pull Requests 4 Actions Packages Projects Releases Wiki Activity

fix: return image/apng media type from animated card endpoint (#196) #204

Merged
cal merged 3 commits from issue/196-bug-apng-endpoint-returns-wrong-media-type-image-p into main 2026-04-08 02:26:05 +00:00
Conversation 1 Commits 3 Files Changed 1 +2 -2
Claude commented 2026-04-08 01:01:58 +00:00
Collaborator
Copy Link

Closes #196

Summary

The animated card endpoint (GET /{player_id}/{card_type}card/{d}/{variant}/animated) was returning FileResponse with media_type="image/png". APNG files are technically valid PNG, but Discord and browser clients use the MIME type to decide whether to animate the content — image/png may cause the file to be treated as a static image.

Change

app/routers_v2/players.py line 863: "image/png" → "image/apng"

Files Changed

  • app/routers_v2/players.py

Tests

No test suite applies to this change. Verified by reading back the modified line.

Closes #196 ## Summary The animated card endpoint (`GET /{player_id}/{card_type}card/{d}/{variant}/animated`) was returning `FileResponse` with `media_type="image/png"`. APNG files are technically valid PNG, but Discord and browser clients use the MIME type to decide whether to animate the content — `image/png` may cause the file to be treated as a static image. ## Change `app/routers_v2/players.py` line 863: `"image/png"` → `"image/apng"` ## Files Changed - `app/routers_v2/players.py` ## Tests No test suite applies to this change. Verified by reading back the modified line.
Claude added 1 commit 2026-04-08 01:01:59 +00:00 
Closes #196

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude added the
ai-reviewing
 label 2026-04-08 01:15:52 +00:00
Claude reviewed 2026-04-08 01:16:57 +00:00
Claude left a comment
Author
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • app/routers_v2/players.py (modified)

Findings

Correctness

  • BLOCKING: The fix is incomplete. get_animated_card has two FileResponse return paths, and only one was updated.

    • Line 773 (cache hit — NOT changed): 
      if os.path.isfile(cache_path) and tier is None:
    return FileResponse(path=cache_path, media_type="image/png", headers=headers)
    • Line 863 (cache miss — changed by this PR): 
      return FileResponse(path=cache_path, media_type="image/apng", headers=headers)

    The cache hit path at line 773 still returns media_type="image/png". Since cards are cached after first render, the vast majority of production requests go through the cache hit path — meaning the MIME type fix will almost never be seen by clients. Both lines must return "image/apng".

Security

  • No issues found.

Style & Conventions

  • No issues found.

Verdict: REQUEST_CHANGES (posted as COMMENT — self-review restriction)

The change correctly identifies image/apng as the right MIME type, but misses the cache hit return at line 773. Fix that line to match and the PR is ready to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/players.py` (modified) ### Findings #### Correctness - **BLOCKING**: The fix is incomplete. `get_animated_card` has **two** `FileResponse` return paths, and only one was updated. - **Line 773 (cache hit — NOT changed):** ```python if os.path.isfile(cache_path) and tier is None: return FileResponse(path=cache_path, media_type="image/png", headers=headers) ``` - **Line 863 (cache miss — changed by this PR):** ```python return FileResponse(path=cache_path, media_type="image/apng", headers=headers) ``` The cache hit path at line 773 still returns `media_type="image/png"`. Since cards are cached after first render, the vast majority of production requests go through the cache hit path — meaning the MIME type fix will almost never be seen by clients. Both lines must return `"image/apng"`. #### Security - No issues found. #### Style & Conventions - No issues found. ### Verdict: REQUEST_CHANGES (posted as COMMENT — self-review restriction) The change correctly identifies `image/apng` as the right MIME type, but misses the cache hit return at line 773. Fix that line to match and the PR is ready to merge. --- *Automated review by Claude PR Reviewer*
app/routers_v2/players.py Outdated
Claude commented 2026-04-08 01:16:57 +00:00
Author
Collaborator
Copy Link

BLOCKING: This cache-hit return path also needs media_type="image/apng". It serves the same APNG file but still returns the wrong MIME type. Since most production requests hit this path after first render, the fix at line 863 will almost never take effect.

**BLOCKING**: This cache-hit return path also needs `media_type="image/apng"`. It serves the same APNG file but still returns the wrong MIME type. Since most production requests hit this path after first render, the fix at line 863 will almost never take effect.
Claude added
ai-changes-requested
 and removed
ai-reviewing
 labels 2026-04-08 01:17:15 +00:00
Claude added the
ai-working
 label 2026-04-08 01:30:49 +00:00
cal added 1 commit 2026-04-08 01:31:17 +00:00 
The cache hit branch at line 773 still returned image/png, meaning
the MIME type fix was never seen in production since cached responses
dominate. Update it to match the cache miss path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude commented 2026-04-08 01:31:20 +00:00
Author
Collaborator
Copy Link

Addressed review feedback: updated the cache hit path at line 773 to return media_type="image/apng" (matching the cache miss path at line 863). Both FileResponse return paths in get_animated_card now correctly return image/apng.

Addressed review feedback: updated the cache hit path at line 773 to return `media_type="image/apng"` (matching the cache miss path at line 863). Both `FileResponse` return paths in `get_animated_card` now correctly return `image/apng`.
Claude removed the
ai-working
ai-changes-requested
 labels 2026-04-08 01:31:25 +00:00
Claude added the
ai-reviewing
 label 2026-04-08 01:45:54 +00:00
Claude reviewed 2026-04-08 01:47:04 +00:00
Claude left a comment
Author
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • app/routers_v2/players.py (modified)

Findings

Correctness

  • Both FileResponse return paths in get_animated_card are now fixed:
    • Line 773 (cache hit path): "image/png" → "image/apng" ✅
    • Line 863 (cache miss path): "image/png" → "image/apng" ✅
  • This addresses the blocking issue from the prior review — the cache hit path (which handles most production requests) was previously missed.
  • Fix is complete and correct.

Note

: The PR body description only mentions "line 863", but the diff correctly updates both lines. The description is slightly incomplete but the code is right.

Security

  • No issues found.

Style & Conventions

  • Minimal, targeted change. No style issues.

Suggestions

  • None.

Verdict: APPROVED (posted as COMMENT — self-review restriction)

Both FileResponse return paths in get_animated_card now correctly use image/apng. The prior blocking issue is resolved. Ready to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/players.py` (modified) ### Findings #### Correctness - Both `FileResponse` return paths in `get_animated_card` are now fixed: - Line 773 (cache hit path): `"image/png"` → `"image/apng"` ✅ - Line 863 (cache miss path): `"image/png"` → `"image/apng"` ✅ - This addresses the blocking issue from the prior review — the cache hit path (which handles most production requests) was previously missed. - Fix is complete and correct. > **Note**: The PR body description only mentions "line 863", but the diff correctly updates both lines. The description is slightly incomplete but the code is right. #### Security - No issues found. #### Style & Conventions - Minimal, targeted change. No style issues. #### Suggestions - None. ### Verdict: APPROVED (posted as COMMENT — self-review restriction) Both `FileResponse` return paths in `get_animated_card` now correctly use `image/apng`. The prior blocking issue is resolved. Ready to merge. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-04-08 01:47:22 +00:00
cal approved these changes 2026-04-08 02:25:57 +00:00
cal left a comment
Owner
Copy Link

Approved via pd-pr

Approved via pd-pr
cal added 1 commit 2026-04-08 02:26:02 +00:00
cal merged commit c8ec976626 into main 2026-04-08 02:26:05 +00:00
cal deleted branch issue/196-bug-apng-endpoint-returns-wrong-media-type-image-p 2026-04-08 02:26:05 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
cal
Labels
Clear labels   
ai-changes-requested

   
ai-failed

   
ai-merged

PR merged by AI ops agent

  
ai-pr-opened

   
ai-reviewed

   
ai-reviewing

   
ai-reviewing

   
ai-working

   
bug

   
enhancement

   
evolution

Card Evolution system

  
performance

   
phase-0

Render Pipeline Optimization

  
phase-1a

Schema & Data Foundation

  
phase-1b

API & Formula Engine

  
phase-1c

Bot Integration

  
phase-1d

Deployment

  
security

   
tech-debt

   
todo
No Label
ai-changes-requested
ai-failed
ai-merged
ai-pr-opened
ai-reviewed
ai-reviewing
ai-reviewing
ai-working
bug
enhancement
evolution
performance
phase-0
phase-1a
phase-1b
phase-1c
phase-1d
security
tech-debt
todo
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 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-database#204
No description provided.
Delete Branch "issue/196-bug-apng-endpoint-returns-wrong-media-type-image-p"

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?