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 batstats endpoint (#133) #155

Merged
cal merged 1 commits from issue/133-feat-add-limit-pagination-to-batstats-endpoint into main 2026-03-24 12:04:44 +00:00
Conversation 0 Commits 1 Files Changed 1 +5 -1
Claude commented 2026-03-24 06:03:25 +00:00
Collaborator
Copy Link

Closes #133

Adds limit query param (default 100, max 500) to GET /api/v2/batstats.

Changes:

  • Added limit: Optional[int] = 100 to get_batstats signature
  • Clamped via max(0, min(limit, 500)) to guard negative values and cap at 500
  • Applied .limit() to query before both CSV and JSON response branches

Files changed: app/routers_v2/batstats.py

Other observations: The existing params like card_id: int = None use a pre-existing pattern of int = None (without Optional) — Pyright flags these as type errors but they are pre-existing and not introduced by this PR.

Closes #133 Adds `limit` query param (default 100, max 500) to `GET /api/v2/batstats`. **Changes:** - Added `limit: Optional[int] = 100` to `get_batstats` signature - Clamped via `max(0, min(limit, 500))` to guard negative values and cap at 500 - Applied `.limit()` to query before both CSV and JSON response branches **Files changed:** `app/routers_v2/batstats.py` **Other observations:** The existing params like `card_id: int = None` use a pre-existing pattern of `int = None` (without `Optional`) — Pyright flags these as type errors but they are pre-existing and not introduced by this PR.
Claude added 1 commit 2026-03-24 06:03:26 +00:00 
Closes #133

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

AI Code Review

Files Reviewed

  • app/routers_v2/batstats.py (modified)

Findings

Correctness

  • Optional is already imported at line 4 (from typing import Optional, List) — no missing import.
  • limit: Optional[int] = 100 added correctly to get_batstats signature.
  • max(0, min(limit, 500)) correctly guards both negative values and the upper cap — this is the improved form (vs. the min-only version in PR #150 that allowed LIMIT -1).
  • .limit() applied to all_stats before the if csv: branch — both CSV and JSON paths correctly honour the limit. ✅

Security

  • No new injection vectors. Limit clamping prevents runaway queries. No issues found.

Style & Conventions

  • Implementation is consistent with the established pagination pattern across notifications, scout_claims, rewards, and scout_opportunities endpoints.
  • Pre-existing int = None (without Optional) on other params is noted in the PR body and is out of scope for this review.

Suggestions

  • MINOR (non-blocking): count in the JSON response (line 120) reflects the limited count, not the total number of matching rows. This is a pre-existing pattern across all four previously paginated endpoints. A follow-up issue to add a total field (or rename count to clarify) would benefit all five endpoints consistently — no action needed here.
  • MINOR (non-blocking): limit=0 returns an empty response even when records exist. max(0, ...) intentionally allows this; a lower bound of 1 would prevent the edge case. Same pedantic note as PRs #149, #151, #152, #154 — no action needed.
  • No tests added — consistent with the other four pagination PRs in this series.

Verdict: COMMENT

Clean implementation that correctly follows the established pagination pattern. The max(0, min(limit, 500)) clamp is the right form (improvement over the original PR #150 pattern). Both minor observations are pre-existing patterns across the codebase and do not block merging.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/batstats.py` (modified) ### Findings #### Correctness - `Optional` is already imported at line 4 (`from typing import Optional, List`) — no missing import. - `limit: Optional[int] = 100` added correctly to `get_batstats` signature. - `max(0, min(limit, 500))` correctly guards both negative values and the upper cap — this is the improved form (vs. the `min`-only version in PR #150 that allowed `LIMIT -1`). - `.limit()` applied to `all_stats` before the `if csv:` branch — both CSV and JSON paths correctly honour the limit. ✅ #### Security - No new injection vectors. Limit clamping prevents runaway queries. No issues found. #### Style & Conventions - Implementation is consistent with the established pagination pattern across `notifications`, `scout_claims`, `rewards`, and `scout_opportunities` endpoints. - Pre-existing `int = None` (without `Optional`) on other params is noted in the PR body and is out of scope for this review. #### Suggestions - **MINOR (non-blocking)**: `count` in the JSON response (line 120) reflects the limited count, not the total number of matching rows. This is a pre-existing pattern across all four previously paginated endpoints. A follow-up issue to add a `total` field (or rename `count` to clarify) would benefit all five endpoints consistently — no action needed here. - **MINOR (non-blocking)**: `limit=0` returns an empty response even when records exist. `max(0, ...)` intentionally allows this; a lower bound of 1 would prevent the edge case. Same pedantic note as PRs #149, #151, #152, #154 — no action needed. - No tests added — consistent with the other four pagination PRs in this series. ### Verdict: COMMENT Clean implementation that correctly follows the established pagination pattern. The `max(0, min(limit, 500))` clamp is the right form (improvement over the original PR #150 pattern). Both minor observations are pre-existing patterns across the codebase and do not block merging. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-03-24 06:16:29 +00:00
cal approved these changes 2026-03-24 12:04:24 +00:00
cal left a comment
Owner
Copy Link

Reviewer verdict is COMMENT (no blocking issues). Approving to merge.

Reviewer verdict is COMMENT (no blocking issues). Approving to merge.
cal merged commit 630b334528 into main 2026-03-24 12:04:42 +00:00
cal deleted branch issue/133-feat-add-limit-pagination-to-batstats-endpoint 2026-03-24 12:04:44 +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#155
No description provided.
Delete Branch "issue/133-feat-add-limit-pagination-to-batstats-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?