feat: capture total_count before limit across all paginated endpoints #169

cal · 2026-03-24T12:43:36Z

cal commented

2026-03-24 12:43:36 +00:00

Summary

Adds total_count = queryset.count() before .limit() across 14 paginated endpoints so the JSON response count field reflects total matching records, not page size
Guards total_count with if not csv ternary to skip the COUNT query on CSV export paths (no regression)
Consolidates rewards.py from 3 redundant COUNT queries down to 1
Cleans up scout_claims.py double if limit block
Normalizes scout_opportunities.py from max(1,...) to max(0,...) for consistency

Files changed

All in app/routers_v2/:
awards, batstats, battingcardratings, events, gamerewards, gauntletrewards, mlbplayers, pitchingcardratings, pitstats, results, rewards, scout_claims, scout_opportunities, stratgame

Notes

gauntletruns.py was already accidentally correct (inline .limit()) — left unchanged
notifications.py already had the correct pattern from PR #150 — not modified
Ruff auto-formatting included in batstats.py, events.py, scout_opportunities.py

## Summary - Adds `total_count = queryset.count()` **before** `.limit()` across 14 paginated endpoints so the JSON response `count` field reflects total matching records, not page size - Guards `total_count` with `if not csv` ternary to skip the COUNT query on CSV export paths (no regression) - Consolidates `rewards.py` from 3 redundant COUNT queries down to 1 - Cleans up `scout_claims.py` double `if limit` block - Normalizes `scout_opportunities.py` from `max(1,...)` to `max(0,...)` for consistency ## Files changed All in `app/routers_v2/`: awards, batstats, battingcardratings, events, gamerewards, gauntletrewards, mlbplayers, pitchingcardratings, pitstats, results, rewards, scout_claims, scout_opportunities, stratgame ## Notes - `gauntletruns.py` was already accidentally correct (inline `.limit()`) — left unchanged - `notifications.py` already had the correct pattern from PR #150 — not modified - Ruff auto-formatting included in batstats.py, events.py, scout_opportunities.py

cal added 2 commits 2026-03-24 12:43:37 +00:00

feat: capture total_count before limit across all paginated endpoints 54dccd1981

Ensures the `count` field in JSON responses reflects total matching
records rather than the page size, consistent with the notifications
endpoint pattern from PR #150.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

fix: skip total_count query for CSV requests and consolidate rewards.py counts 1e21894898

- Guard total_count with `if not csv` ternary to avoid unnecessary
  COUNT query on CSV export paths (10 files)
- Consolidate rewards.py from 3 COUNT queries to 1 (used for both
  empty-check and response)
- Clean up scout_claims.py double `if limit is not None` block
- Normalize scout_opportunities.py from max(1,...) to max(0,...)

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

cal reviewed 2026-03-24 12:45:13 +00:00

cal left a comment

AI Code Review

Files Reviewed

app/routers_v2/awards.py (modified)
app/routers_v2/batstats.py (modified — ruff reformatted)
app/routers_v2/battingcardratings.py (modified)
app/routers_v2/events.py (modified — ruff reformatted)
app/routers_v2/gamerewards.py (modified)
app/routers_v2/gauntletrewards.py (modified)
app/routers_v2/mlbplayers.py (modified)
app/routers_v2/pitchingcardratings.py (modified)
app/routers_v2/pitstats.py (modified)
app/routers_v2/results.py (modified)
app/routers_v2/rewards.py (modified — COUNT consolidation)
app/routers_v2/scout_claims.py (modified — double-limit cleanup)
app/routers_v2/scout_opportunities.py (modified — max(1,…) → max(0,…) + decoded from base64)
app/routers_v2/stratgame.py (modified)

Findings

Correctness

Core fix — verified correct across all 14 files. The total_count = queryset.count() call is placed after all filter predicates are applied but before .limit() in every changed endpoint. The Peewee queryset is lazy, so count() at this point issues SELECT COUNT(*) FROM ... WHERE <all filters> — exactly the total-matches semantics that was broken before. The JSON count field now accurately reflects total matching records regardless of page size.

gauntletrewards.py — no CSV path, unconditional count is correct. This endpoint has no CSV branch, so the simpler total_count = all_rewards.count() (no ternary guard) is the right call. Consistent with the endpoint's design.

rewards.py consolidation — correct and an improvement. The removed pre-filter guard (if all_rewards.count() == 0 before any filters are applied) was logically wrong — it would raise 404 if the entire rewards table was empty rather than if the filtered result set was empty. The single total_count = all_rewards.count() after filters, reused for both the 404 guard and the response count field, is the correct approach and saves two extra COUNT queries.

scout_claims.py — count is correctly placed before the optional limit branch. total_count = query.count() fires before the if limit is not None block, so it always reflects the full untruncated result set. Correct.

scout_opportunities.py — max(1, ...) → max(0, ...) normalization. Aligns limit clamping with every other endpoint in the codebase. Callers passing limit=0 previously got 1 row; they will now correctly get 0. Also note: the file was previously stored as base64 content in git (the old diff shows a single binary-encoded line), and is now a proper readable text file — this is a correctness fix in itself.

battingcardratings.py and pitchingcardratings.py apply the inline max(0, min(limit, 500)) expression directly in .limit() without an intermediate variable. total_count is correctly assigned on the line immediately before in both cases. Correct.

No logic errors, off-by-one issues, or missed endpoints found.

Edge Cases

CSV paths — if not csv ternary is safe. When csv=True, total_count is set to 0 and the count query is skipped entirely. The CSV response path never reads total_count, so the 0 placeholder is never exposed to callers. No regression.

csv=None (default) treated as falsy — correct. The parameter defaults to None in most endpoints. if not csv evaluates not None as True, so the count query fires on the default path as expected. The CSV skip only triggers when csv=True is explicitly passed.

rewards.py 404 on empty filtered results — the count-then-404 pattern works correctly. This is the only endpoint in the PR that raises 404 on an empty result set; the others return {"count": 0, "results": []}. This pre-existing asymmetry is preserved unchanged and is out of scope for this PR.

Security

No issues found. No new inputs, no new query construction paths, no credentials introduced. Changes are purely structural.

Style & Conventions

Ruff reformatting in batstats.py, events.py, and scout_opportunities.py is large in line count but mechanically correct — single/double quote normalization, multi-line argument lists, trailing commas. No behavioral changes. Pattern consistency across all 14 files is confirmed: every non-CSV endpoint now uses a pre-limit total_count returned as the count field, with the ternary guard applied uniformly on all CSV-capable endpoints.

Suggestions

rewards.py 404-on-empty is an outlier compared to the rest of the API surface. Not a blocker, but worth standardizing in a follow-up to avoid surprising API consumers who expect a consistent {"count": 0, ...} shape everywhere.
scout_opportunities.py computes limit = max(0, min(limit, 500)) at the top of the handler before filters are applied (pre-existing). Other endpoints clamp just before .limit(). Harmless, but could be normalized in a future cleanup pass.

Verdict: APPROVED (self-review — Gitea blocks self-approval)

The implementation is correct across all 14 files. total_count is consistently captured after filter predicates and before .limit() in every endpoint. The if not csv ternary correctly skips the COUNT query on CSV paths without exposing the placeholder value to callers. The rewards.py consolidation eliminates a logically flawed pre-filter 404 guard and reduces from 3 to 1 COUNT query. No security issues, no regressions found.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/awards.py` (modified) - `app/routers_v2/batstats.py` (modified — ruff reformatted) - `app/routers_v2/battingcardratings.py` (modified) - `app/routers_v2/events.py` (modified — ruff reformatted) - `app/routers_v2/gamerewards.py` (modified) - `app/routers_v2/gauntletrewards.py` (modified) - `app/routers_v2/mlbplayers.py` (modified) - `app/routers_v2/pitchingcardratings.py` (modified) - `app/routers_v2/pitstats.py` (modified) - `app/routers_v2/results.py` (modified) - `app/routers_v2/rewards.py` (modified — COUNT consolidation) - `app/routers_v2/scout_claims.py` (modified — double-limit cleanup) - `app/routers_v2/scout_opportunities.py` (modified — max(1,…) → max(0,…) + decoded from base64) - `app/routers_v2/stratgame.py` (modified) --- ### Findings #### Correctness **Core fix — verified correct across all 14 files.** The `total_count = queryset.count()` call is placed after all filter predicates are applied but before `.limit()` in every changed endpoint. The Peewee queryset is lazy, so `count()` at this point issues `SELECT COUNT(*) FROM ... WHERE <all filters>` — exactly the total-matches semantics that was broken before. The JSON `count` field now accurately reflects total matching records regardless of page size. **`gauntletrewards.py` — no CSV path, unconditional count is correct.** This endpoint has no CSV branch, so the simpler `total_count = all_rewards.count()` (no ternary guard) is the right call. Consistent with the endpoint's design. **`rewards.py` consolidation — correct and an improvement.** The removed pre-filter guard (`if all_rewards.count() == 0` before any filters are applied) was logically wrong — it would raise 404 if the entire rewards table was empty rather than if the filtered result set was empty. The single `total_count = all_rewards.count()` after filters, reused for both the 404 guard and the response `count` field, is the correct approach and saves two extra COUNT queries. **`scout_claims.py` — count is correctly placed before the optional limit branch.** `total_count = query.count()` fires before the `if limit is not None` block, so it always reflects the full untruncated result set. Correct. **`scout_opportunities.py` — `max(1, ...)` → `max(0, ...)` normalization.** Aligns limit clamping with every other endpoint in the codebase. Callers passing `limit=0` previously got 1 row; they will now correctly get 0. Also note: the file was previously stored as base64 content in git (the old diff shows a single binary-encoded line), and is now a proper readable text file — this is a correctness fix in itself. **`battingcardratings.py` and `pitchingcardratings.py`** apply the inline `max(0, min(limit, 500))` expression directly in `.limit()` without an intermediate variable. `total_count` is correctly assigned on the line immediately before in both cases. Correct. No logic errors, off-by-one issues, or missed endpoints found. #### Edge Cases **CSV paths — `if not csv` ternary is safe.** When `csv=True`, `total_count` is set to `0` and the count query is skipped entirely. The CSV response path never reads `total_count`, so the `0` placeholder is never exposed to callers. No regression. **`csv=None` (default) treated as falsy — correct.** The parameter defaults to `None` in most endpoints. `if not csv` evaluates `not None` as `True`, so the count query fires on the default path as expected. The CSV skip only triggers when `csv=True` is explicitly passed. **`rewards.py` 404 on empty filtered results** — the count-then-404 pattern works correctly. This is the only endpoint in the PR that raises 404 on an empty result set; the others return `{"count": 0, "results": []}`. This pre-existing asymmetry is preserved unchanged and is out of scope for this PR. #### Security No issues found. No new inputs, no new query construction paths, no credentials introduced. Changes are purely structural. #### Style & Conventions Ruff reformatting in `batstats.py`, `events.py`, and `scout_opportunities.py` is large in line count but mechanically correct — single/double quote normalization, multi-line argument lists, trailing commas. No behavioral changes. Pattern consistency across all 14 files is confirmed: every non-CSV endpoint now uses a pre-limit `total_count` returned as the `count` field, with the ternary guard applied uniformly on all CSV-capable endpoints. #### Suggestions - `rewards.py` 404-on-empty is an outlier compared to the rest of the API surface. Not a blocker, but worth standardizing in a follow-up to avoid surprising API consumers who expect a consistent `{"count": 0, ...}` shape everywhere. - `scout_opportunities.py` computes `limit = max(0, min(limit, 500))` at the top of the handler before filters are applied (pre-existing). Other endpoints clamp just before `.limit()`. Harmless, but could be normalized in a future cleanup pass. --- ### Verdict: APPROVED (self-review — Gitea blocks self-approval) The implementation is correct across all 14 files. `total_count` is consistently captured after filter predicates and before `.limit()` in every endpoint. The `if not csv` ternary correctly skips the COUNT query on CSV paths without exposing the placeholder value to callers. The `rewards.py` consolidation eliminates a logically flawed pre-filter 404 guard and reduces from 3 to 1 COUNT query. No security issues, no regressions found. --- *Automated review by Claude PR Reviewer*

Claude added the

ai-reviewing

label 2026-03-24 12:45:46 +00:00

Claude approved these changes 2026-03-24 12:45:50 +00:00

Claude left a comment

Approved — clean implementation across all 14 endpoints.

cal merged commit bc6c23ef2e into main

2026-03-24 12:45:54 +00:00

cal deleted branch enhancement/total-count-pagination

2026-03-24 12:45:54 +00:00

cal referenced this issue from a commit

2026-03-24 12:45:56 +00:00

Merge pull request 'feat: capture total_count before limit across all paginated endpoints' (#169) from enhancement/total-count-pagination into main

Claude reviewed 2026-03-24 12:48:06 +00:00

Claude left a comment

AI Code Review

Files Reviewed

app/routers_v2/awards.py (modified)
app/routers_v2/batstats.py (modified)
app/routers_v2/battingcardratings.py (modified)
app/routers_v2/events.py (modified)
app/routers_v2/gamerewards.py (modified)
app/routers_v2/gauntletrewards.py (modified)
app/routers_v2/mlbplayers.py (modified)
app/routers_v2/pitchingcardratings.py (modified)
app/routers_v2/pitstats.py (modified)
app/routers_v2/results.py (modified)
app/routers_v2/rewards.py (modified)
app/routers_v2/scout_claims.py (modified)
app/routers_v2/scout_opportunities.py (modified)
app/routers_v2/stratgame.py (modified)

Findings

Correctness

Core pattern is correct across all 14 files. total_count = queryset.count() is computed before .limit() is applied, so count in the JSON response now reflects total matching records, not page size. This is the intended fix.
CSV guard (if not csv) is correctly applied in all files that have a CSV path (awards, batstats, battingcardratings, events, gamerewards, mlbplayers, pitchingcardratings, pitstats, results, stratgame). gauntletrewards.py correctly omits the guard since it has no CSV path.
rewards.py consolidation is correct. The removed pre-filter check (if all_rewards.count() == 0: raise 404 "There are no rewards to filter") was redundant — if the table is empty, the post-filter check still catches it. Going from 3 COUNT queries to 1 is a clean improvement, and total_count is computed unconditionally because it's needed for the zero-check regardless of the CSV path.
scout_claims.py cleanup is correct. total_count captured before the if limit is not None guard, returned in place of len(results).

Notable: scout_opportunities.py was base64-corrupted

The diff reveals the old scout_opportunities.py contained a single line of base64-encoded text (aW1wb3J0IGpzb24K...) rather than Python source. This decodes to the same Python code shown in the new file. The corruption likely originated from a git API commit (Gitea's update_file tool encodes content as base64 for the API payload; if that encoding was accidentally stored literally, the file would be unrunnable). The endpoint was broken — FastAPI would have raised a SyntaxError on import. This PR silently fixes that corruption alongside the count fix. Worth confirming the endpoint was healthy in prod before merging, but the fix itself is correct.

Missing endpoints (MINOR)

The PR title says "across all paginated endpoints" but paperdex.py (PR #167) and cardpositions.py (PR #168) are also paginated endpoints and were not updated. Both still return count = limited count, not total. Follow-up PR recommended to close the gap, or update the title to clarify scope.

Security

No security concerns. No new inputs, no query injection risks introduced.

Style & Conventions

Ruff auto-formatting in batstats.py, events.py, scout_opportunities.py (single→double quotes, parameter reformatting) is non-functional and consistent with prior PRs in this series.
gauntletrewards.py omits if not csv guard (correct — no CSV path); the inconsistent-looking pattern is intentional and correct.

Verdict: COMMENT

Solid correctness fix. The pattern is applied uniformly and accurately. The scout_opportunities.py base64 corruption was a latent breakage that this PR inadvertently resolved — worth confirming no prod incidents occurred in the window between that corruption and this merge. The only gaps are paperdex and cardpositions (missed in scope), which should be a one-line fix each.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/awards.py` (modified) - `app/routers_v2/batstats.py` (modified) - `app/routers_v2/battingcardratings.py` (modified) - `app/routers_v2/events.py` (modified) - `app/routers_v2/gamerewards.py` (modified) - `app/routers_v2/gauntletrewards.py` (modified) - `app/routers_v2/mlbplayers.py` (modified) - `app/routers_v2/pitchingcardratings.py` (modified) - `app/routers_v2/pitstats.py` (modified) - `app/routers_v2/results.py` (modified) - `app/routers_v2/rewards.py` (modified) - `app/routers_v2/scout_claims.py` (modified) - `app/routers_v2/scout_opportunities.py` (modified) - `app/routers_v2/stratgame.py` (modified) ### Findings #### Correctness - **Core pattern is correct across all 14 files.** `total_count = queryset.count()` is computed before `.limit()` is applied, so `count` in the JSON response now reflects total matching records, not page size. This is the intended fix. - **CSV guard (`if not csv`) is correctly applied** in all files that have a CSV path (awards, batstats, battingcardratings, events, gamerewards, mlbplayers, pitchingcardratings, pitstats, results, stratgame). `gauntletrewards.py` correctly omits the guard since it has no CSV path. - **`rewards.py` consolidation is correct.** The removed pre-filter check (`if all_rewards.count() == 0: raise 404 "There are no rewards to filter"`) was redundant — if the table is empty, the post-filter check still catches it. Going from 3 COUNT queries to 1 is a clean improvement, and `total_count` is computed unconditionally because it's needed for the zero-check regardless of the CSV path. - **`scout_claims.py` cleanup is correct.** `total_count` captured before the `if limit is not None` guard, returned in place of `len(results)`. #### Notable: scout_opportunities.py was base64-corrupted The diff reveals the old `scout_opportunities.py` contained a single line of base64-encoded text (`aW1wb3J0IGpzb24K...`) rather than Python source. This decodes to the same Python code shown in the new file. The corruption likely originated from a git API commit (Gitea's `update_file` tool encodes content as base64 for the API payload; if that encoding was accidentally stored literally, the file would be unrunnable). The endpoint was broken — FastAPI would have raised a `SyntaxError` on import. This PR silently fixes that corruption alongside the count fix. Worth confirming the endpoint was healthy in prod before merging, but the fix itself is correct. #### Missing endpoints (MINOR) The PR title says "across all paginated endpoints" but `paperdex.py` (PR #167) and `cardpositions.py` (PR #168) are also paginated endpoints and were not updated. Both still return `count` = limited count, not total. Follow-up PR recommended to close the gap, or update the title to clarify scope. #### Security - No security concerns. No new inputs, no query injection risks introduced. #### Style & Conventions - Ruff auto-formatting in `batstats.py`, `events.py`, `scout_opportunities.py` (single→double quotes, parameter reformatting) is non-functional and consistent with prior PRs in this series. - `gauntletrewards.py` omits `if not csv` guard (correct — no CSV path); the inconsistent-looking pattern is intentional and correct. ### Verdict: COMMENT Solid correctness fix. The pattern is applied uniformly and accurately. The scout_opportunities.py base64 corruption was a latent breakage that this PR inadvertently resolved — worth confirming no prod incidents occurred in the window between that corruption and this merge. The only gaps are paperdex and cardpositions (missed in scope), which should be a one-line fix each. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-24 12:49:16 +00:00

Sign in to join this conversation.

No reviewers

No Milestone

No project

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#169