feat: add limit/pagination to rewards endpoint (#139) #152

Claude · 2026-03-24T05:02:20Z

Claude commented

2026-03-24 05:02:20 +00:00

Closes #139

Adds optional limit query parameter to GET /api/v2/rewards.

Default: 100
Max: 500 (capped via max(0, min(limit, 500)) — guards negative values)
Applied after all filters, before serialization

Files changed: app/routers_v2/rewards.py

Other observations: The file was auto-reformatted by ruff (single→double quotes, indentation normalization) during the pre-commit hook — cosmetic diff is wider than the functional change but all ruff checks pass.

Closes #139 Adds optional `limit` query parameter to `GET /api/v2/rewards`. - Default: `100` - Max: `500` (capped via `max(0, min(limit, 500))` — guards negative values) - Applied after all filters, before serialization **Files changed:** `app/routers_v2/rewards.py` **Other observations:** The file was auto-reformatted by ruff (single→double quotes, indentation normalization) during the pre-commit hook — cosmetic diff is wider than the functional change but all ruff checks pass.

Claude added 3 commits 2026-03-24 05:02:21 +00:00

ci: add dev tag trigger to Docker build workflow

Build Docker Image / build (push) Waiting to run

Details

d4d93cd95e

Allows deploying to dev environment by pushing a "dev" tag.
Dev tags build with :dev Docker tag instead of :production.

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

ci: switch buildx cache from registry to local volume

Build Docker Image / build (push) Successful in 10m39s

Details

e12dac347e

Replaces type=registry cache (which causes 400 errors from Docker Hub
due to stale buildx builders) with type=local backed by a named Docker
volume on the runner. Adds cache rotation step to prevent unbounded growth.

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

feat: add limit/pagination to rewards endpoint (#139 ) 890625e770

Closes #139

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

Claude referenced this pull request

2026-03-24 05:02:27 +00:00

feat: add limit/pagination to rewards endpoint #139

Claude added the

ai-reviewing

label 2026-03-24 05:15:46 +00:00

Claude reviewed 2026-03-24 05:16:56 +00:00

Claude left a comment

AI Code Review

Files Reviewed

app/routers_v2/rewards.py (modified)
.gitea/workflows/build.yml (modified — pre-existing in main, see note)

Findings

Correctness

limit: Optional[int] = 100 parameter added correctly with appropriate default.
limit = max(0, min(limit, 500)) correctly guards both negative values and the upper bound — improvement over the PR #150 pattern which only used min(limit, 500).
Limit is applied after all filters (name, team_id, created_after, in_name, season, week) and after the second count() == 0 guard, before the CSV/JSON branching. Placement is correct.
Applies to both CSV and JSON response paths consistently.

Security

No issues. Auth checks (valid_token) unchanged. [REDACTED] token logging preserved correctly.

Style & Conventions

Ruff reformatting (single→double quotes, indentation normalization) is cosmetic and passes all checks. Consistent with prior PRs (#103, #53, #57).

Suggestions

MINOR — count field reflects limited count, not total matching rows. After all_rewards = all_rewards.limit(limit) is applied, all_rewards.count() wraps the limited query in a subquery, so the count key in the JSON response reflects the number of rows returned (≤ limit), not the total number of matching rewards. This is the same pre-existing pattern as the notifications endpoint (PR #150) and scout_claims endpoint (PR #151). Non-blocking given consistency with the rest of the codebase, but pagination callers may find it misleading.
MINOR — limit=0 returns empty results even when rewards exist. max(0, min(0, 500)) = 0 → LIMIT 0 in SQL. If a caller passes ?limit=0, the response is {"count": 0, "rewards": []} despite matching rows existing. Pedantic edge case (who requests 0 rows?), but worth noting. A lower bound of 1 instead of 0 would prevent it.
INFORMATIONAL — build.yml diff is a no-op. The branch was not rebased onto current main. The build.yml changes in this diff (dev tag trigger, local volume cache) correspond to commits c185d72 and d0f45d5, which are already in main. Merging this PR will not alter .gitea/workflows/build.yml. Same pattern as PRs #150 and #151.

Verdict: COMMENT

Implementation is correct and follows the established pattern. The max(0, min(limit, 500)) guard correctly addresses the negative-limit bug from PR #150. No blockers — the count field quirk and limit=0 edge case are both non-blocking observations consistent with patterns already present in the codebase.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/rewards.py` (modified) - `.gitea/workflows/build.yml` (modified — pre-existing in main, see note) ### Findings #### Correctness - `limit: Optional[int] = 100` parameter added correctly with appropriate default. - `limit = max(0, min(limit, 500))` correctly guards both negative values and the upper bound — improvement over the PR #150 pattern which only used `min(limit, 500)`. - Limit is applied after all filters (`name`, `team_id`, `created_after`, `in_name`, `season`, `week`) and after the second `count() == 0` guard, before the CSV/JSON branching. Placement is correct. - Applies to both CSV and JSON response paths consistently. #### Security - No issues. Auth checks (`valid_token`) unchanged. `[REDACTED]` token logging preserved correctly. #### Style & Conventions - Ruff reformatting (single→double quotes, indentation normalization) is cosmetic and passes all checks. Consistent with prior PRs (#103, #53, #57). #### Suggestions 1. **MINOR — `count` field reflects limited count, not total matching rows.** After `all_rewards = all_rewards.limit(limit)` is applied, `all_rewards.count()` wraps the limited query in a subquery, so the `count` key in the JSON response reflects the number of rows returned (≤ limit), not the total number of matching rewards. This is the same pre-existing pattern as the notifications endpoint (PR #150) and scout_claims endpoint (PR #151). Non-blocking given consistency with the rest of the codebase, but pagination callers may find it misleading. 2. **MINOR — `limit=0` returns empty results even when rewards exist.** `max(0, min(0, 500))` = `0` → `LIMIT 0` in SQL. If a caller passes `?limit=0`, the response is `{"count": 0, "rewards": []}` despite matching rows existing. Pedantic edge case (who requests 0 rows?), but worth noting. A lower bound of `1` instead of `0` would prevent it. 3. **INFORMATIONAL — build.yml diff is a no-op.** The branch was not rebased onto current `main`. The build.yml changes in this diff (dev tag trigger, local volume cache) correspond to commits `c185d72` and `d0f45d5`, which are already in `main`. Merging this PR will not alter `.gitea/workflows/build.yml`. Same pattern as PRs #150 and #151. ### Verdict: COMMENT Implementation is correct and follows the established pattern. The `max(0, min(limit, 500))` guard correctly addresses the negative-limit bug from PR #150. No blockers — the `count` field quirk and `limit=0` edge case are both non-blocking observations consistent with patterns already present in the codebase. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-24 05:17:30 +00:00

cal approved these changes 2026-03-24 12:04:26 +00:00

cal left a comment

LGTM. Reviewer flagged two non-blocking observations (count reflects limited rows, limit=0 edge case) — both consistent with established patterns in #150 and #151. Approving.

cal added 1 commit 2026-03-24 12:04:36 +00:00

Merge branch 'main' into issue/139-feat-add-limit-pagination-to-rewards-endpoint ecc37c1df2

cal added 1 commit 2026-03-24 12:05:11 +00:00

Merge branch 'main' into issue/139-feat-add-limit-pagination-to-rewards-endpoint 2a2a20b1d7

cal added 1 commit 2026-03-24 12:06:05 +00:00

Merge branch 'main' into issue/139-feat-add-limit-pagination-to-rewards-endpoint dbc473b1b5

cal referenced this issue from a commit

2026-03-24 12:06:41 +00:00

fix: clamp limit lower bound to 1 to prevent silent empty responses

cal added 1 commit 2026-03-24 12:08:44 +00:00

Merge branch 'main' into issue/139-feat-add-limit-pagination-to-rewards-endpoint 23b95f2d3d

cal added 1 commit 2026-03-24 12:08:56 +00:00