hotfix: remove output caps from GET /players #103

cal · 2026-04-02T01:14:52Z

cal commented

2026-04-02 01:14:52 +00:00

Summary

Removes MAX_LIMIT (500) and DEFAULT_LIMIT (200) caps from GET /api/v3/players — bot and website consumers need full player lists without pagination
Reverts limit param to Optional[int] = Query(default=None, ge=1) (no ceiling)
All other endpoints retain their caps from #99

Context

The caps added in #99 (16f3f8d) fixed Gunicorn worker timeouts from unbounded queries across all list endpoints. However, the /players endpoint is a known bounded dataset (~1500 rows/season) and its consumers depend on fetching complete lists. This hotfix exempts it while keeping protections everywhere else.

Test plan

test_player_service.py — all get_players tests pass (10/10)
test_query_limits.py — all 5 tests pass (they test /decisions, not /players)
Verify on dev server that GET /players accepts uncapped requests without 422

🤖 Generated with Claude Code

## Summary - Removes `MAX_LIMIT` (500) and `DEFAULT_LIMIT` (200) caps from `GET /api/v3/players` — bot and website consumers need full player lists without pagination - Reverts `limit` param to `Optional[int] = Query(default=None, ge=1)` (no ceiling) - All other endpoints retain their caps from #99 ## Context The caps added in #99 (`16f3f8d`) fixed Gunicorn worker timeouts from unbounded queries across all list endpoints. However, the `/players` endpoint is a known bounded dataset (~1500 rows/season) and its consumers depend on fetching complete lists. This hotfix exempts it while keeping protections everywhere else. ## Test plan - [x] `test_player_service.py` — all `get_players` tests pass (10/10) - [x] `test_query_limits.py` — all 5 tests pass (they test `/decisions`, not `/players`) - [ ] Verify on dev server that `GET /players` accepts uncapped requests without 422 🤖 Generated with [Claude Code](https://claude.com/claude-code)

cal added 1 commit 2026-04-02 01:14:54 +00:00

hotfix: remove output caps from GET /players endpoint

Build Docker Image / build (pull_request) Successful in 2m29s

Details

d92f571960

The MAX_LIMIT/DEFAULT_LIMIT caps added in 16f3f8d are too restrictive
for the /players endpoint — bot and website consumers need full player
lists without pagination. Reverts limit param to Optional[int] with no
ceiling while keeping caps on all other endpoints.

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

Claude added the

ai-reviewing

label 2026-04-02 01:15:08 +00:00

Claude reviewed 2026-04-02 01:16:10 +00:00

Claude left a comment

AI Code Review

Files Reviewed

app/routers_v3/players.py (modified)

Findings

Correctness

Change is minimal and correct: removes MAX_LIMIT/DEFAULT_LIMIT imports (no longer referenced) and reverts limit to Optional[int] = Query(default=None, ge=1).
PlayerService.get_players() already accepts limit: Optional[int] = None and branches correctly at lines 144/150: if limit is not None: query = query.limit(limit). So limit=None (the new default) returns the full unbounded result set as intended.
total_count is computed pre-pagination at the service layer (query.count() before .limit() is applied), so the response count/total fields remain semantically correct.
The ge=1 constraint is preserved — callers passing limit=0 still get a 422.
search_players endpoint retains its own explicit le=50 cap — unrelated, unchanged, correct.

Security

No issues. This is a read-only endpoint on an internal API. The ge=1 guard prevents degenerate inputs. The only theoretical concern is an uncapped large limit=N query, but the PR body's rationale is sound: the players table is bounded (~1500 rows/season), so even a limit=999999 request returns at most ~1500 rows — no meaningful DoS vector.

Style & Conventions

No issues. Optional[int] = Query(default=None, ge=1) is the correct FastAPI idiom for an optional positive integer param with no ceiling.

Suggestions

None. The change is surgical and well-scoped.

Verdict: COMMENT

(Gitea blocks self-approval.) Clean, minimal hotfix. The service layer handles limit=None correctly, the players dataset is bounded, and the import cleanup leaves no dead imports. Ready to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v3/players.py` (modified) ### Findings #### Correctness - Change is minimal and correct: removes `MAX_LIMIT`/`DEFAULT_LIMIT` imports (no longer referenced) and reverts `limit` to `Optional[int] = Query(default=None, ge=1)`. - `PlayerService.get_players()` already accepts `limit: Optional[int] = None` and branches correctly at lines 144/150: `if limit is not None: query = query.limit(limit)`. So `limit=None` (the new default) returns the full unbounded result set as intended. - `total_count` is computed pre-pagination at the service layer (`query.count()` before `.limit()` is applied), so the response `count`/`total` fields remain semantically correct. - The `ge=1` constraint is preserved — callers passing `limit=0` still get a 422. - `search_players` endpoint retains its own explicit `le=50` cap — unrelated, unchanged, correct. #### Security - No issues. This is a read-only endpoint on an internal API. The `ge=1` guard prevents degenerate inputs. The only theoretical concern is an uncapped large `limit=N` query, but the PR body's rationale is sound: the players table is bounded (~1500 rows/season), so even a `limit=999999` request returns at most ~1500 rows — no meaningful DoS vector. #### Style & Conventions - No issues. `Optional[int] = Query(default=None, ge=1)` is the correct FastAPI idiom for an optional positive integer param with no ceiling. #### Suggestions - None. The change is surgical and well-scoped. ### Verdict: COMMENT (Gitea blocks self-approval.) Clean, minimal hotfix. The service layer handles `limit=None` correctly, the players dataset is bounded, and the import cleanup leaves no dead imports. Ready to merge. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-04-02 01:16:26 +00:00

cal merged commit c063f5c4ef into main

2026-04-02 01:19:53 +00:00

cal deleted branch hotfix/remove-players-output-caps

2026-04-02 01:19:54 +00:00

cal referenced this issue from a commit

2026-04-02 01:19:55 +00:00

Merge pull request 'hotfix: remove output caps from GET /players' (#103) from hotfix/remove-players-output-caps into main