fix(packs): remove unfiltered pre-count in GET /packs (3 round-trips to 2) #211

cal · 2026-04-10T07:53:04Z

cal commented

2026-04-10 07:53:04 +00:00

Autonomous pipeline work item

Finding: `GET /api/v2/packs` calls `Pack.select().count()` on the unfiltered table before applying filters, raising 404 if no packs exist globally. Correctness bug (wrong 404 on empty filter matches) + perf bug (extra DB round-trip per request).

Change: Remove the pre-filter count check at `routers_v2/packs.py:36-39`. Filtered queries now make 2 DB calls instead of 3, and empty filter matches return an empty result set instead of 404 (standard REST pattern). Also removed commented-out dead code block (lines 81-83) and added `# noqa: E711` to Peewee `== None` where-clauses (intentional ORM syntax for SQL IS NULL).

Verification:

Consumer check: grep discord-app for 404 handling on `/packs` — clean
Test suite: `python -m pytest tests/` — 266 passed, 13 pre-existing failures (all unrelated: animated card endpoint, APNG generator, postgame refractor — same failures on main before this change)

Source: analyst-2026-04-10-008 (autonomous nightly run)

## Autonomous pipeline work item **Finding:** \`GET /api/v2/packs\` calls \`Pack.select().count()\` on the unfiltered table before applying filters, raising 404 if no packs exist globally. Correctness bug (wrong 404 on empty filter matches) + perf bug (extra DB round-trip per request). **Change:** Remove the pre-filter count check at \`routers_v2/packs.py:36-39\`. Filtered queries now make 2 DB calls instead of 3, and empty filter matches return an empty result set instead of 404 (standard REST pattern). Also removed commented-out dead code block (lines 81-83) and added \`# noqa: E711\` to Peewee \`== None\` where-clauses (intentional ORM syntax for SQL IS NULL). **Verification:** - Consumer check: grep discord-app for 404 handling on \`/packs\` — clean - Test suite: \`python -m pytest tests/\` — 266 passed, 13 pre-existing failures (all unrelated: animated card endpoint, APNG generator, postgame refractor — same failures on main before this change) **Source:** analyst-2026-04-10-008 (autonomous nightly run)

cal added 1 commit 2026-04-10 07:53:05 +00:00

fix(packs): remove unfiltered pre-count in GET /packs (3 round-trips → 2) fd377014db

Remove Pack.select().count() on the unfiltered table at the top of GET
/api/v2/packs. This check raised 404 if zero packs existed globally —
wrong for filtered queries where no match is the expected empty-list
result. The filtered count at the end of the handler already handles
the empty-result case. Endpoint now returns {count: 0, packs: []} on
empty filter matches (standard REST pattern) and saves one DB round-trip
per request.

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

cal added the

autonomous

size:S

type:stability

labels 2026-04-10 07:54:07 +00:00

Claude added the

ai-reviewing

label 2026-04-10 08:15:50 +00:00

Claude reviewed 2026-04-10 08:16:52 +00:00

Claude left a comment

AI Code Review

Files Reviewed

app/routers_v2/packs.py (modified)

Findings

Correctness

No issues found. The pre-filter count removal is the right fix:

Old behavior: Pack.select().count() == 0 checked the entire unfiltered table; 404 raised when no packs existed globally — wrong response for a valid filtered request on an empty (or newly-seeded) table.
New behavior: filters are applied first; zero results return {"count": 0, "packs": []} — standard REST pattern. ✅
Commented-out dead code (lines 81-83) correctly removed. ✅
# noqa: E711 on Pack.pack_team == None and Pack.pack_cardset == None is appropriate — Peewee ORM intentionally uses == None to generate SQL IS NULL, not an equality check. ✅

Performance claim of 3 → 2 DB round-trips is accurate: old code did (1) unfiltered count + (2) filtered .count() for response + (3) filtered iteration; new code does (1) filtered .count() + (2) filtered iteration.

Security

No issues found. No new inputs, no new query construction paths. Existing auth guards on write endpoints unchanged.

Style & Conventions

The diff includes ruff auto-reformatting throughout (single → double quotes, parameter list wrapping). Non-functional, consistent with project style. No CLAUDE.md violations.

Suggestions

No new tests for the behavior change (404 → empty 200 on empty table). Low risk given the simplicity of the removal, and the PR body confirms pre-existing test count is unchanged — acceptable per codebase pattern. A follow-up test covering GET /packs with no matching results returning {"count": 0, "packs": []} would close the gap.

Verdict: COMMENT

No blocking issues. Self-review restriction applies (autonomous pipeline PR, single-repo author). Ready to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/packs.py` (modified) ### Findings #### Correctness No issues found. The pre-filter count removal is the right fix: - **Old behavior**: `Pack.select().count() == 0` checked the entire unfiltered table; 404 raised when no packs existed globally — wrong response for a valid filtered request on an empty (or newly-seeded) table. - **New behavior**: filters are applied first; zero results return `{"count": 0, "packs": []}` — standard REST pattern. ✅ - Commented-out dead code (`lines 81-83`) correctly removed. ✅ - `# noqa: E711` on `Pack.pack_team == None` and `Pack.pack_cardset == None` is appropriate — Peewee ORM intentionally uses `== None` to generate SQL `IS NULL`, not an equality check. ✅ Performance claim of 3 → 2 DB round-trips is accurate: old code did (1) unfiltered count + (2) filtered `.count()` for response + (3) filtered iteration; new code does (1) filtered `.count()` + (2) filtered iteration. #### Security No issues found. No new inputs, no new query construction paths. Existing auth guards on write endpoints unchanged. #### Style & Conventions The diff includes ruff auto-reformatting throughout (single → double quotes, parameter list wrapping). Non-functional, consistent with project style. No CLAUDE.md violations. #### Suggestions - No new tests for the behavior change (404 → empty 200 on empty table). Low risk given the simplicity of the removal, and the PR body confirms pre-existing test count is unchanged — acceptable per codebase pattern. A follow-up test covering `GET /packs` with no matching results returning `{"count": 0, "packs": []}` would close the gap. ### Verdict: COMMENT No blocking issues. Self-review restriction applies (autonomous pipeline PR, single-repo author). Ready to merge. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-04-10 08:17:25 +00:00

cal force-pushed autonomous/packs-remove-prefilter-count from fd377014db to 7f7d9ffe1f

2026-04-10 11:45:38 +00:00

Compare

cal merged commit 5e85e27cda into main

2026-04-10 11:45:54 +00:00

cal referenced this issue from a commit

2026-04-10 11:45:55 +00:00

Merge pull request 'fix(packs): remove unfiltered pre-count in GET /packs (3 round-trips to 2)' (#211) from autonomous/packs-remove-prefilter-count into main

cal deleted branch autonomous/packs-remove-prefilter-count

2026-04-10 11:45:55 +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#211