feat: add limit/pagination to scout_opportunities endpoint (#148) #154

Claude · 2026-03-24T05:32:42Z

Claude commented

2026-03-24 05:32:42 +00:00

Closes #148

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

Changes

app/routers_v2/scout_opportunities.py: Added limit: Optional[int] = 100 parameter; capped via max(0, min(limit, 500)); applied as .limit(limit) before query execution
Also removed pre-existing unused fn import (required to pass ruff pre-commit hook)

Behaviour

Default: returns up to 100 records (previously unbounded)
Max: 500 records per request
Negative values clamped to 0
Consistent with pagination pattern applied in #149, #139

Other observations

count in the response reflects the limited count, not the total matching rows — same pre-existing pattern as notifications, scout_claims, and rewards endpoints

Closes #148 Adds `limit` query parameter (default 100, max 500) to `GET /api/v2/scout_opportunities`. ## Changes - `app/routers_v2/scout_opportunities.py`: Added `limit: Optional[int] = 100` parameter; capped via `max(0, min(limit, 500))`; applied as `.limit(limit)` before query execution - Also removed pre-existing unused `fn` import (required to pass ruff pre-commit hook) ## Behaviour - Default: returns up to 100 records (previously unbounded) - Max: 500 records per request - Negative values clamped to 0 - Consistent with pagination pattern applied in #149, #139 ## Other observations - `count` in the response reflects the limited count, not the total matching rows — same pre-existing pattern as notifications, scout_claims, and rewards endpoints

Claude added 1 commit 2026-03-24 05:32:43 +00:00

feat: add limit/pagination to scout_opportunities endpoint (#148 ) f9817b3d04

Closes #148

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

Claude referenced this pull request

2026-03-24 05:32:49 +00:00

feat: add limit/pagination to scout_opportunities endpoint #148

Claude added the

ai-reviewing

label 2026-03-24 05:45:49 +00:00

Claude reviewed 2026-03-24 05:46:32 +00:00

Claude left a comment

AI Code Review

Files Reviewed

app/routers_v2/scout_opportunities.py (modified)

Findings

Correctness

Implementation is correct. limit: Optional[int] = 100 added as a query param with max(0, min(limit, 500)) clamping applied before all filters run and .limit(limit) applied after all where() clauses — correct ordering.
fn removal is valid: confirmed unused throughout the file; no calls reference it.
MINOR: limit=0 returns an empty response even when matching records exist (lower bound of max(0, ...) admits zero). A caller who accidentally passes limit=0 gets no results with no error. Using max(1, min(limit, 500)) would prevent this — same pedantic edge case flagged in PRs #149 and #152.
MINOR: count in the response reflects the limited count, not the total number of matching rows. Acknowledged in the PR body; consistent with pre-existing pattern across /notifs, /scout_claims, and /rewards. Non-blocking, but worth a follow-up issue to standardise pagination metadata across all endpoints.

Security

No issues. limit is typed as int; FastAPI validates it at the boundary. The clamp prevents pathological queries.

Style & Conventions

Matches the pagination pattern established in PRs #149 and #139 exactly. Consistent placement, consistent clamping, consistent default/max values.

Suggestions

The four paginated endpoints (/notifs, /scout_claims, /rewards, /scout_opportunities) all share the same count-reflects-limited-count behaviour. A single follow-up issue to add a total field (or rename count to returned_count) would clean this up project-wide rather than one endpoint at a time.

Verdict: COMMENT

Clean, minimal, correct. Consistent with the established pagination pattern. The two minor observations (zero-limit edge case, misleading count) are pre-existing across the series and are acknowledged in the PR body — nothing blocks merging.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/scout_opportunities.py` (modified) ### Findings #### Correctness - Implementation is correct. `limit: Optional[int] = 100` added as a query param with `max(0, min(limit, 500))` clamping applied before all filters run and `.limit(limit)` applied after all `where()` clauses — correct ordering. - `fn` removal is valid: confirmed unused throughout the file; no calls reference it. - **MINOR**: `limit=0` returns an empty response even when matching records exist (lower bound of `max(0, ...)` admits zero). A caller who accidentally passes `limit=0` gets no results with no error. Using `max(1, min(limit, 500))` would prevent this — same pedantic edge case flagged in PRs #149 and #152. - **MINOR**: `count` in the response reflects the limited count, not the total number of matching rows. Acknowledged in the PR body; consistent with pre-existing pattern across `/notifs`, `/scout_claims`, and `/rewards`. Non-blocking, but worth a follow-up issue to standardise pagination metadata across all endpoints. #### Security - No issues. `limit` is typed as `int`; FastAPI validates it at the boundary. The clamp prevents pathological queries. #### Style & Conventions - Matches the pagination pattern established in PRs #149 and #139 exactly. Consistent placement, consistent clamping, consistent default/max values. #### Suggestions - The four paginated endpoints (`/notifs`, `/scout_claims`, `/rewards`, `/scout_opportunities`) all share the same `count`-reflects-limited-count behaviour. A single follow-up issue to add a `total` field (or rename `count` to `returned_count`) would clean this up project-wide rather than one endpoint at a time. ### Verdict: COMMENT Clean, minimal, correct. Consistent with the established pagination pattern. The two minor observations (zero-limit edge case, misleading `count`) are pre-existing across the series and are acknowledged in the PR body — nothing blocks merging. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-24 05:46:48 +00:00

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

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

Addresses reviewer feedback: max(0,...) admitted limit=0 which would
silently return no results even when matching records exist.
Changed to max(1,...) consistent with feedback on PRs #149 and #152.

cal approved these changes 2026-03-24 12:06:50 +00:00

cal left a comment

Approved. Limit lower-bound fix (max(1,...)) looks correct — addresses the reviewer edge-case note cleanly.

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

Merge branch 'main' into issue/148-feat-add-limit-pagination-to-scout-opportunities-e aeb37c20f2

cal merged commit 11e8fba6c5 into main

2026-03-24 12:07:35 +00:00

cal deleted branch issue/148-feat-add-limit-pagination-to-scout-opportunities-e

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

cal referenced this issue from a commit

2026-03-24 12:07:37 +00:00

Merge pull request 'feat: add limit/pagination to scout_opportunities endpoint (#148)' (#154) from issue/148-feat-add-limit-pagination-to-scout-opportunities-e into main

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