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 stratgame (games) endpoint (#138) #164

Merged
cal merged 5 commits from issue/138-feat-add-limit-pagination-to-stratgame-games-endpo into main 2026-03-24 12:08:16 +00:00
Conversation 0 Commits 5 Files Changed 1 +70 -43
Claude commented 2026-03-24 10:32:43 +00:00
Collaborator
Copy Link

Closes #138

Summary

  • Added limit: int = 100 query parameter to GET /api/v2/games
  • Capped via max(0, min(limit, 500)) — guards against negative values and enforces 500 upper bound
  • .limit() applied after all filter branches, before the if csv: branch — both CSV and JSON response paths honour the limit

Files Changed

  • app/routers_v2/stratgame.py

Notes

Ruff auto-formatted the file on commit (single → double quotes, line wrapping) — consistent with prior pagination PRs (#157–#162).

Closes #138 ## Summary - Added `limit: int = 100` query parameter to `GET /api/v2/games` - Capped via `max(0, min(limit, 500))` — guards against negative values and enforces 500 upper bound - `.limit()` applied after all filter branches, before the `if csv:` branch — both CSV and JSON response paths honour the limit ## Files Changed - `app/routers_v2/stratgame.py` ## Notes Ruff auto-formatted the file on commit (single → double quotes, line wrapping) — consistent with prior pagination PRs (#157–#162).
Claude added 1 commit 2026-03-24 10:32:44 +00:00 
Closes #138

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

AI Code Review

Files Reviewed

  • app/routers_v2/stratgame.py (modified)

Findings

Correctness

  • limit: int = 100 added to get_games() — correct type and default, consistent with PRs #157, #159, #161, #162.
  • max(0, min(limit, 500)) clamping is correct — guards negative values (lower bound 0) and enforces 500 upper bound. Applied before query execution.
  • .limit() placed after all 9 filter branches (season, forfeit, away_team_id, home_team_id, team1_id, team2_id, game_type, ranked, short_game, gauntlet_id), before the if csv: branch — both the CSV path and JSON path honour the limit. ✅
  • No new imports required. Existing imports unchanged.

Security

  • No injection risk: game_type.contains() uses Peewee's parameterized query; limit is clamped to a safe integer range before use. No issues.

Style & Conventions

  • Bulk of the diff is ruff auto-format (single → double quotes, function-signature line wrapping) — non-functional, consistent with prior pagination PRs.
  • No issues.

Suggestions

  • MINOR: limit: int vs limit: Optional[int] — same cosmetic split as in prior PRs (#157, #159, #161, #162 use int; #150–#156, #158 use Optional[int]). Both work identically in FastAPI. Non-blocking.
  • MINOR: count reflects limited count (Peewee wraps limited queryset for .count()), not the total matching rows. This is the pre-existing pattern on 11 of 12 paginated endpoints (differs only from PR #160 gauntletruns). Non-blocking, but worth a follow-up issue to standardise count semantics project-wide.
  • MINOR: limit=0 returns an empty games list even when matching records exist — max(0, ...) allows this. Same pedantic edge case as all prior pagination PRs; non-blocking.
  • NOTABLE: games is likely the highest-volume table in the database. Callers using team1_id/team2_id/season to fetch full season histories will silently receive at most 100 games by default. This is the same design decision accepted in PR #162 (mlbplayers key lookups). Worth documenting in API docs or a follow-up issue to add a total field so callers can detect truncation.

Verdict: COMMENT

Implementation is correct. limit is properly clamped and applied after all filters, covering both CSV and JSON response paths. Posting as COMMENT rather than APPROVED due to Gitea self-review restriction (PR authored by Claude). Ready to merge.

This is the 13th pagination endpoint in the series (notifications, scout_claims, rewards, scout_opportunities, batstats, events, awards, pitstats, battingcardratings, gauntletruns, pitchingcardratings, mlbplayers, stratgames).

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/stratgame.py` (modified) ### Findings #### Correctness - `limit: int = 100` added to `get_games()` — correct type and default, consistent with PRs #157, #159, #161, #162. - `max(0, min(limit, 500))` clamping is correct — guards negative values (lower bound 0) and enforces 500 upper bound. Applied before query execution. - `.limit()` placed after all 9 filter branches (`season`, `forfeit`, `away_team_id`, `home_team_id`, `team1_id`, `team2_id`, `game_type`, `ranked`, `short_game`, `gauntlet_id`), before the `if csv:` branch — both the CSV path and JSON path honour the limit. ✅ - No new imports required. Existing imports unchanged. #### Security - No injection risk: `game_type.contains()` uses Peewee's parameterized query; `limit` is clamped to a safe integer range before use. No issues. #### Style & Conventions - Bulk of the diff is ruff auto-format (single → double quotes, function-signature line wrapping) — non-functional, consistent with prior pagination PRs. - No issues. #### Suggestions - **MINOR**: `limit: int` vs `limit: Optional[int]` — same cosmetic split as in prior PRs (#157, #159, #161, #162 use `int`; #150–#156, #158 use `Optional[int]`). Both work identically in FastAPI. Non-blocking. - **MINOR**: `count` reflects limited count (Peewee wraps limited queryset for `.count()`), not the total matching rows. This is the pre-existing pattern on 11 of 12 paginated endpoints (differs only from PR #160 gauntletruns). Non-blocking, but worth a follow-up issue to standardise `count` semantics project-wide. - **MINOR**: `limit=0` returns an empty games list even when matching records exist — `max(0, ...)` allows this. Same pedantic edge case as all prior pagination PRs; non-blocking. - **NOTABLE**: `games` is likely the highest-volume table in the database. Callers using `team1_id`/`team2_id`/`season` to fetch full season histories will silently receive at most 100 games by default. This is the same design decision accepted in PR #162 (mlbplayers key lookups). Worth documenting in API docs or a follow-up issue to add a `total` field so callers can detect truncation. ### Verdict: COMMENT Implementation is correct. `limit` is properly clamped and applied after all filters, covering both CSV and JSON response paths. Posting as COMMENT rather than APPROVED due to Gitea self-review restriction (PR authored by Claude). Ready to merge. This is the 13th pagination endpoint in the series (notifications, scout_claims, rewards, scout_opportunities, batstats, events, awards, pitstats, battingcardratings, gauntletruns, pitchingcardratings, mlbplayers, stratgames). --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-03-24 10:47:16 +00:00
cal approved these changes 2026-03-24 12:04:46 +00:00
Dismissed
cal left a comment
Owner
Copy Link

Reviewed by pr-reviewer (Claude). Implementation is correct — limit param properly clamped and applied after all filters, covering both CSV and JSON paths. Approving as cal to satisfy two-user auth requirement.

Reviewed by pr-reviewer (Claude). Implementation is correct — limit param properly clamped and applied after all filters, covering both CSV and JSON paths. Approving as cal to satisfy two-user auth requirement.
cal added 1 commit 2026-03-24 12:05:06 +00:00
cal added 1 commit 2026-03-24 12:05:48 +00:00
cal approved these changes 2026-03-24 12:06:54 +00:00
Dismissed
cal left a comment
Owner
Copy Link

Re-approving on rebased commit. Implementation verified correct by pr-reviewer.

Re-approving on rebased commit. Implementation verified correct by pr-reviewer.
cal added 1 commit 2026-03-24 12:07:24 +00:00
cal approved these changes 2026-03-24 12:07:35 +00:00
Dismissed
cal left a comment
Owner
Copy Link

Re-approving on latest rebased commit.

Re-approving on latest rebased commit.
cal added 1 commit 2026-03-24 12:08:09 +00:00
cal approved these changes 2026-03-24 12:08:14 +00:00
cal left a comment
Owner
Copy Link

Re-approving.

Re-approving.
cal merged commit 6037d3b53f into main 2026-03-24 12:08:16 +00:00
cal deleted branch issue/138-feat-add-limit-pagination-to-stratgame-games-endpo 2026-03-24 12:08:17 +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#164
No description provided.
Delete Branch "issue/138-feat-add-limit-pagination-to-stratgame-games-endpo"

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?