cal/paper-dynasty-database
1
0
Fork 0
Code Issues 9 Pull Requests 5 Actions Packages Projects Releases Wiki Activity

fix: ensure count is never null in GET /refractor/cards (#183) #185

Open
Claude wants to merge 2 commits from issue/183-bug-get-refractor-cards-returns-count-null-with-ve into main
pull from: issue/183-bug-get-refractor-cards-returns-count-null-with-ve
merge into: cal:main
Conversation 0 Commits 2 Files Changed 2 +2 -2
Claude commented 2026-04-06 06:34:16 +00:00
Collaborator
Copy Link

Closes #183

Summary

query.count() on a complex multi-join query (RefractorCardState → RefractorTrack → Player via LEFT JOIN) can return None in Peewee 3.17.9 when 0 rows match the filter set. This caused {"count": null, "items": []} responses instead of {"count": 0, "items": []}.

Fix

Added or 0 guard to total = query.count() → total = query.count() or 0 in list_card_states().

The existing limit: int = Query(default=10, ge=1, le=100) constraint already rejects limit=99999 with 422 — no change needed there.

Files Changed

  • app/routers_v2/refractor.py — 1 line

Other observations

  • GET /{team_id}/refractors in teams.py (line 1579) has the same total = query.count() pattern without the or 0 guard. Same Peewee bug applies there — tracked separately.
Closes #183 ## Summary `query.count()` on a complex multi-join query (RefractorCardState → RefractorTrack → Player via LEFT JOIN) can return `None` in Peewee 3.17.9 when 0 rows match the filter set. This caused `{"count": null, "items": []}` responses instead of `{"count": 0, "items": []}`. ## Fix Added `or 0` guard to `total = query.count()` → `total = query.count() or 0` in `list_card_states()`. The existing `limit: int = Query(default=10, ge=1, le=100)` constraint already rejects `limit=99999` with 422 — no change needed there. ## Files Changed - `app/routers_v2/refractor.py` — 1 line ## Other observations - `GET /{team_id}/refractors` in `teams.py` (line 1579) has the same `total = query.count()` pattern without the `or 0` guard. Same Peewee bug applies there — tracked separately.
Claude added 2 commits 2026-04-06 06:34:17 +00:00 
Dev environment uses sba_postgres container, paperdynasty_dev database,
sba_admin user — not pd_postgres/pd_master as previously documented.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Guards against Peewee 3.17.9 returning None from .count() on a
complex multi-join query when 0 rows match the filter set.

Closes #183

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

AI Code Review

Files Reviewed

  • app/routers_v2/refractor.py (modified — 1 line)
  • CLAUDE.md (modified — doc fix, auto-resolves)

Findings

Correctness

  • total = query.count() or 0 — correct guard. Peewee 3.17.9 can return None from .count() on a multi-table LEFT JOIN query when 0 rows match. The or 0 idiom is the established pattern already applied elsewhere in this codebase (matching the fix in PR #184). Applied at the right place — after all filters, before pagination. ✅
  • CLAUDE.md hunk: updates dev PostgreSQL container/db/user info. This duplicates commit cc65dc5 already on main — a stale hunk from the branch cut. Gitea reports mergeable: true, so it auto-resolves cleanly on merge. ✅

Security

  • No issues. No new query parameters or auth changes.

Style & Conventions

  • One-line fix matches the existing pattern. No style concerns.

Suggestions

  • Duplicate PR advisory: PR #184 (issue/182-...) fixes the identical line and was created ~27 minutes earlier, but is currently mergeable: false (merge conflict). This PR (#185) is mergeable: true and is effectively the better candidate. Recommend closing #184 in favor of this PR to avoid confusion.
  • The teams.py:1579 follow-up (same query.count() pattern without guard, also without the ge=0,le=4 tier constraint) is already noted in the PR body — tracked separately. ✅

Verdict: COMMENT

Fix is correct and the PR is mergeable. Posting as COMMENT due to self-review restriction (author and reviewer are both Claude). Recommend closing PR #184 (not mergeable, duplicate) and merging this one.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/refractor.py` (modified — 1 line) - `CLAUDE.md` (modified — doc fix, auto-resolves) ### Findings #### Correctness - `total = query.count() or 0` — correct guard. Peewee 3.17.9 can return `None` from `.count()` on a multi-table LEFT JOIN query when 0 rows match. The `or 0` idiom is the established pattern already applied elsewhere in this codebase (matching the fix in PR #184). Applied at the right place — after all filters, before pagination. ✅ - `CLAUDE.md` hunk: updates dev PostgreSQL container/db/user info. This duplicates commit `cc65dc5` already on `main` — a stale hunk from the branch cut. Gitea reports `mergeable: true`, so it auto-resolves cleanly on merge. ✅ #### Security - No issues. No new query parameters or auth changes. #### Style & Conventions - One-line fix matches the existing pattern. No style concerns. #### Suggestions - **Duplicate PR advisory**: PR #184 (`issue/182-...`) fixes the identical line and was created ~27 minutes earlier, but is currently `mergeable: false` (merge conflict). This PR (#185) is `mergeable: true` and is effectively the better candidate. Recommend closing #184 in favor of this PR to avoid confusion. - The `teams.py:1579` follow-up (same `query.count()` pattern without guard, also without the `ge=0,le=4` tier constraint) is already noted in the PR body — tracked separately. ✅ ### Verdict: COMMENT Fix is correct and the PR is mergeable. Posting as COMMENT due to self-review restriction (author and reviewer are both Claude). Recommend closing PR #184 (not mergeable, duplicate) and merging this one. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
 and removed
ai-reviewing
 labels 2026-04-06 06:47:38 +00:00
This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin issue/183-bug-get-refractor-cards-returns-count-null-with-ve:issue/183-bug-get-refractor-cards-returns-count-null-with-ve
git checkout issue/183-bug-get-refractor-cards-returns-count-null-with-ve
Sign in to join this conversation.
Reviewers
No reviewers
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
1 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#185
No description provided.
Delete Branch "issue/183-bug-get-refractor-cards-returns-count-null-with-ve"

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?