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 scout_claims endpoint (#149) #151

Merged
cal merged 4 commits from issue/149-feat-add-limit-pagination-to-scout-claims-endpoint into main 2026-03-24 12:04:39 +00:00
Conversation 0 Commits 4 Files Changed 1 +8 -2
Claude commented 2026-03-24 04:32:13 +00:00
Collaborator
Copy Link

Closes #149

Adds an optional limit query parameter to GET /api/v2/scout_claims:

  • Default: 100
  • Max: 500 (clamped via max(0, min(limit, 500)))
  • Pass limit=None is handled — no .limit() call applied

The clamp uses max(0, ...) to guard against negative values (PostgreSQL LIMIT -1 would return all rows).

Files changed:

  • app/routers_v2/scout_claims.py

Other observations:

  • ScoutOpportunity was imported but unused (pre-existing) — removed by the ruff pre-commit hook enforcement to allow the commit to proceed.
Closes #149 Adds an optional `limit` query parameter to `GET /api/v2/scout_claims`: - Default: `100` - Max: `500` (clamped via `max(0, min(limit, 500))`) - Pass `limit=None` is handled — no `.limit()` call applied The clamp uses `max(0, ...)` to guard against negative values (PostgreSQL `LIMIT -1` would return all rows). **Files changed:** - `app/routers_v2/scout_claims.py` **Other observations:** - `ScoutOpportunity` was imported but unused (pre-existing) — removed by the ruff pre-commit hook enforcement to allow the commit to proceed.
Claude added 3 commits 2026-03-24 04:32:14 +00:00
ci: add dev tag trigger to Docker build workflow
Some checks are pending
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
All checks were successful
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>
Closes #149

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

AI Code Review

Files Reviewed

  • app/routers_v2/scout_claims.py (modified)
  • .gitea/workflows/build.yml (in diff — pre-existing changes already in main, no-op on merge)

Findings

Correctness

  • Implementation matches the PR description. limit defaults to 100, is clamped via max(0, min(limit, 500)), and is applied only when not None. Logic is correct.
  • MINOR: The count field in the response reflects the number of rows returned (after limiting), not the total number of matching rows. For example, if 300 claims match the filters but limit=100, count returns 100. A caller implementing pagination cannot determine if there are more pages without an additional request. This is the same pattern as the notifications endpoint (PR #150) — consistent with the existing codebase, but worth noting if this endpoint ever needs to support cursor/offset pagination.

Security

  • No issues. The GET endpoint is unauthenticated (consistent with the existing endpoint). limit is typed Optional[int] so FastAPI rejects non-integer inputs automatically.

Style & Conventions

  • ScoutOpportunity unused import correctly removed. Matches ruff enforcement in the project.
  • Parameter formatting (multi-line signature) matches the updated style in similar endpoints.

Suggestions

  • If count semantics are important to callers, consider renaming it returned_count or adding a separate total_count field (requires an additional SELECT COUNT(*) before applying .limit()). Non-blocking — pre-existing pattern.

Note on build.yml

The build.yml changes appear in the diff because this branch was cut before commits e12dac3 and d4d93cd landed on main. The diff is already present in main — the merge will be a no-op for that file.

Verdict: COMMENT

Implementation is correct and the negative-value guard (max(0, ...)) is properly applied — this is an improvement over the parallel notifications PR (#150). The one observation about count semantics is pre-existing and non-blocking.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/scout_claims.py` (modified) - `.gitea/workflows/build.yml` (in diff — pre-existing changes already in `main`, no-op on merge) ### Findings #### Correctness - Implementation matches the PR description. `limit` defaults to `100`, is clamped via `max(0, min(limit, 500))`, and is applied only when not `None`. Logic is correct. - **MINOR**: The `count` field in the response reflects the number of rows returned (after limiting), not the total number of matching rows. For example, if 300 claims match the filters but `limit=100`, `count` returns `100`. A caller implementing pagination cannot determine if there are more pages without an additional request. This is the same pattern as the notifications endpoint (PR #150) — consistent with the existing codebase, but worth noting if this endpoint ever needs to support cursor/offset pagination. #### Security - No issues. The `GET` endpoint is unauthenticated (consistent with the existing endpoint). `limit` is typed `Optional[int]` so FastAPI rejects non-integer inputs automatically. #### Style & Conventions - `ScoutOpportunity` unused import correctly removed. Matches ruff enforcement in the project. - Parameter formatting (multi-line signature) matches the updated style in similar endpoints. #### Suggestions - If `count` semantics are important to callers, consider renaming it `returned_count` or adding a separate `total_count` field (requires an additional `SELECT COUNT(*)` before applying `.limit()`). Non-blocking — pre-existing pattern. ### Note on `build.yml` The `build.yml` changes appear in the diff because this branch was cut before commits `e12dac3` and `d4d93cd` landed on `main`. The diff is already present in `main` — the merge will be a no-op for that file. ### Verdict: COMMENT Implementation is correct and the negative-value guard (`max(0, ...)`) is properly applied — this is an improvement over the parallel notifications PR (#150). The one observation about `count` semantics is pre-existing and non-blocking. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-03-24 04:47:15 +00:00
cal approved these changes 2026-03-24 12:04:20 +00:00
cal left a comment
Owner
Copy Link

Approving after positive reviewer verdict. Implementation is correct, style matches conventions, no blocking issues.

Approving after positive reviewer verdict. Implementation is correct, style matches conventions, no blocking issues.
cal added 1 commit 2026-03-24 12:04:28 +00:00
cal merged commit 58f408020a into main 2026-03-24 12:04:39 +00:00
cal deleted branch issue/149-feat-add-limit-pagination-to-scout-claims-endpoint 2026-03-24 12:04:40 +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#151
No description provided.
Delete Branch "issue/149-feat-add-limit-pagination-to-scout-claims-endpoint"

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?