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

feat: add limit/pagination to paperdex endpoint (#143) #167

Merged
cal merged 2 commits from issue/143-feat-add-limit-pagination-to-paperdex-endpoint into main 2026-03-25 14:52:58 +00:00
Conversation 0 Commits 2 Files Changed 1 +69 -53
Claude commented 2026-03-24 12:03:06 +00:00
Collaborator
Copy Link

Closes #143

Adds limit: int = 100 query parameter to GET /api/v2/paperdex, capped at 500 via max(0, min(limit, 500)). Applied after all filter branches so both CSV and JSON response paths honour the limit.

Changes

  • app/routers_v2/paperdex.py: Added limit param and cap logic

Other observations

  • ruff F541: pre-existing f-string without placeholder (f"There are no paperdex to filter") — fixed to unblock pre-commit hook
  • ruff F841: pre-existing unused all_sets variable in cardset_id filter branch — removed to unblock pre-commit hook
  • ruff auto-formatted the file on save (quote style, line wrapping) — cosmetic only, consistent with prior pagination PRs
Closes #143 Adds `limit: int = 100` query parameter to `GET /api/v2/paperdex`, capped at 500 via `max(0, min(limit, 500))`. Applied after all filter branches so both CSV and JSON response paths honour the limit. ## Changes - `app/routers_v2/paperdex.py`: Added `limit` param and cap logic ## Other observations - ruff F541: pre-existing f-string without placeholder (`f"There are no paperdex to filter"`) — fixed to unblock pre-commit hook - ruff F841: pre-existing unused `all_sets` variable in `cardset_id` filter branch — removed to unblock pre-commit hook - ruff auto-formatted the file on save (quote style, line wrapping) — cosmetic only, consistent with prior pagination PRs
Claude added 1 commit 2026-03-24 12:03:08 +00:00 
Closes #143

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude added the
ai-reviewing
 label 2026-03-24 12:15:46 +00:00
Claude reviewed 2026-03-24 12:16:31 +00:00
Claude left a comment
Author
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • app/routers_v2/paperdex.py (modified)

Findings

Correctness

  • limit: int = 100 added to GET /api/v2/paperdex; capped via max(0, min(limit, 500)) — correct, guards both negative values and upper bound ✅
  • .limit() applied after all 5 filter branches (team_id, player_id, cardset_id, created_after, created_before), before the if csv: branch — both CSV and JSON response paths honour the limit ✅
  • all_sets unused variable in the cardset_id branch correctly removed (ruff F841) — the all_dex.where(Paperdex.player.cardset.id == cardset_id) line that followed it was always the operative line ✅
  • f"There are no paperdex to filter" → plain string (ruff F541) — correct side-effect fix ✅

Security

  • No issues found. Auth guards on mutating endpoints unchanged.

Style & Conventions

  • limit: int (not Optional[int]) — same cosmetic inconsistency as PRs #157, #159, #162, #165, #166; FastAPI handles both identically; non-blocking
  • count in JSON response reflects the limited count, not total matching records — consistent with 13 of the other 14 paginated endpoints (only gauntletruns from PR #160 uses total count); non-blocking
  • Bulk of diff is ruff auto-format (single→double quotes, parameter line-wrapping) — consistent with every prior pagination PR in this series

Suggestions

  • None — implementation is clean and follows the established pattern exactly.

Verdict: COMMENT

✅ Correct implementation. limit param added with proper cap, applied after all filters so both response paths (CSV and JSON) are bounded. Ruff cleanups (all_sets removal, f-string fix) are appropriate. Posted as COMMENT due to Gitea self-review restriction; would otherwise be APPROVED.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/paperdex.py` (modified) ### Findings #### Correctness - `limit: int = 100` added to `GET /api/v2/paperdex`; capped via `max(0, min(limit, 500))` — correct, guards both negative values and upper bound ✅ - `.limit()` applied after all 5 filter branches (`team_id`, `player_id`, `cardset_id`, `created_after`, `created_before`), before the `if csv:` branch — both CSV and JSON response paths honour the limit ✅ - `all_sets` unused variable in the `cardset_id` branch correctly removed (ruff F841) — the `all_dex.where(Paperdex.player.cardset.id == cardset_id)` line that followed it was always the operative line ✅ - `f"There are no paperdex to filter"` → plain string (ruff F541) — correct side-effect fix ✅ #### Security - No issues found. Auth guards on mutating endpoints unchanged. #### Style & Conventions - `limit: int` (not `Optional[int]`) — same cosmetic inconsistency as PRs #157, #159, #162, #165, #166; FastAPI handles both identically; non-blocking - `count` in JSON response reflects the limited count, not total matching records — consistent with 13 of the other 14 paginated endpoints (only `gauntletruns` from PR #160 uses total count); non-blocking - Bulk of diff is ruff auto-format (single→double quotes, parameter line-wrapping) — consistent with every prior pagination PR in this series #### Suggestions - None — implementation is clean and follows the established pattern exactly. ### Verdict: COMMENT ✅ Correct implementation. `limit` param added with proper cap, applied after all filters so both response paths (CSV and JSON) are bounded. Ruff cleanups (`all_sets` removal, f-string fix) are appropriate. Posted as COMMENT due to Gitea self-review restriction; would otherwise be APPROVED. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-03-24 12:16:49 +00:00
cal approved these changes 2026-03-25 14:52:18 +00:00
cal left a comment
Owner
Copy Link

Approved as cal — reviewer verdict is effectively APPROVED (COMMENT state only due to Gitea self-review restriction). Implementation is correct and follows established patterns.

Approved as cal — reviewer verdict is effectively APPROVED (COMMENT state only due to Gitea self-review restriction). Implementation is correct and follows established patterns.
cal added 1 commit 2026-03-25 14:52:45 +00:00
cal merged commit 7e7ff960e2 into main 2026-03-25 14:52:58 +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#167
No description provided.
Delete Branch "issue/143-feat-add-limit-pagination-to-paperdex-endpoint"

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?