feat(WP-07): Card State API endpoints (#72) #108

Merged

cal merged 2 commits from feature/wp-07-card-state-api into card-evolution 2026-03-18 20:33:19 +00:00

Conversation 1 Commits 2 Files Changed 4 +224 -41

cal commented 2026-03-18 18:48:54 +00:00

Owner

Copy Link

Closes #72

Summary

• GET /api/v2/teams/{team_id}/evolutions — paginated list of a team's evolution card states with filters (card_type, tier, page, per_page)
• GET /api/v2/evolution/cards/{card_id} — single card state with card_id resolved to (player_id, team_id) via Card table join
• Response includes nested track dict with thresholds + computed next_threshold (null if fully evolved)
• _build_card_state_response() shared helper avoids duplication between endpoints
• 10 integration tests in test_evolution_state_api.py

Test plan

• 10 integration tests (PG-backed, auto-skip without POSTGRES_HOST)
• CI with real PostgreSQL
• Manual: verify response shape matches spec (nested track, next_threshold)

🤖 Generated with Claude Code

Closes #72 ## Summary - `GET /api/v2/teams/{team_id}/evolutions` — paginated list of a team's evolution card states with filters (card_type, tier, page, per_page) - `GET /api/v2/evolution/cards/{card_id}` — single card state with card_id resolved to (player_id, team_id) via Card table join - Response includes nested track dict with thresholds + computed `next_threshold` (null if fully evolved) - `_build_card_state_response()` shared helper avoids duplication between endpoints - 10 integration tests in `test_evolution_state_api.py` ## Test plan - [x] 10 integration tests (PG-backed, auto-skip without POSTGRES_HOST) - [ ] CI with real PostgreSQL - [ ] Manual: verify response shape matches spec (nested track, next_threshold) 🤖 Generated with [Claude Code](https://claude.com/claude-code)

cal added 1 commit 2026-03-18 18:48:55 +00:00

feat(WP-07): card state API endpoints — closes #72 ... 4a03f98803

Add two endpoints for reading EvolutionCardState:

  GET /api/v2/teams/{team_id}/evolutions
    - Optional filters: card_type, tier
    - Pagination: page / per_page (default 10, max 100)
    - Joins EvolutionTrack so card_type filter is a single query
    - Returns {count, items} with full card state + threshold context

  GET /api/v2/evolution/cards/{card_id}
    - Resolves card_id -> (player_id, team_id) via Card table
    - Duplicate cards for same player+team share one state row
    - Returns 404 when card missing or has no evolution state

Both endpoints:
  - Require bearer token auth (valid_token dependency)
  - Embed the EvolutionTrack in each item (not just the FK id)
  - Compute next_threshold: threshold for tier above current (null at T4)
  - Share _build_card_state_response() helper in evolution.py

Also cleans up 30 pre-existing ruff violations in teams.py that were
blocking the pre-commit hook: F541 bare f-strings, E712 boolean
comparisons (now noqa where Peewee ORM requires == False/True),
and F841 unused variable assignments.

Tests: tests/test_evolution_state_api.py — 10 integration tests that
skip automatically without POSTGRES_HOST, following the same pattern as
test_evolution_track_api.py.

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

Claude added the

ai-reviewing

label 2026-03-18 19:15:47 +00:00

Claude approved these changes 2026-03-18 19:18:52 +00:00

Claude left a comment

Collaborator

Copy Link

AI Code Review

Files Reviewed

• app/routers_v2/evolution.py (modified — new _build_card_state_response helper and GET /evolution/cards/{card_id} endpoint)
• app/routers_v2/teams.py (modified — new GET /{team_id}/evolutions endpoint + style cleanup)
• tests/test_evolution_state_api.py (added — 10 integration tests)

Findings

Correctness

• get_card_state: Card lookup via Card.get_by_id(card_id) then EvolutionCardState JOIN is correct. The card.player_id / card.team_id raw FK accessor pattern is valid (Peewee auto-generates _id suffix for FK fields) — same pattern verified in PR #106.
• list_team_evolutions: Peewee's query.count() wraps the full query (including the JOIN and WHERE clauses) in a subquery, so the total count correctly reflects all applied filters before slicing with .offset().limit().
• _NEXT_THRESHOLD_ATTR lookup: Handles all valid tiers (0-4) explicitly. For out-of-range tier values, dict.get() returns None (Python default), which is falsy, so getattr(track, next_attr) is never called — silently returns None. Acceptable behaviour given the DB constraint on tier values.
• _build_card_state_response shared helper: Correctly called from both endpoints. The cross-router lazy import in teams.py (from ..routers_v2.evolution import _build_card_state_response) follows the lazy import pattern used throughout the codebase and avoids circular imports at module load time.
• lc_id / r_query removals: lc_id was only referenced in a commented-out sheets.post_new_cards call — safe to remove. r_query = Team.update(...) was an unused assignment; .execute() is called directly now — correct.

Security

• Auth guard (valid_token → 401) is present on both new endpoints, consistent with all other endpoints in the router.
• Token logging uses [REDACTED] — no credential exposure. Pattern matches the project standard.
• User-supplied card_id and team_id are integer path/query params — FastAPI validates types before the handler runs. No injection vector.

Style & Conventions

• noqa: E712 annotations on Peewee boolean comparisons (== True / == False) match the PostgreSQL-compatible pattern documented in the existing code and avoid ruff false positives.
• f-string → plain string cleanup throughout teams.py (no interpolation in the string) is correct and reduces noise.

Suggestions

• pg_conn fixture is referenced in both test_evolution_state_api.py and the pre-existing test_evolution_track_api.py but is not defined in tests/conftest.py. This is a pre-existing gap (noted in PR #84 review). When POSTGRES_HOST is set, all integration tests in both files will fail with fixture 'pg_conn' not found. The pg_conn fixture (a session-scoped psycopg2 connection) should be added to conftest.py. This is a follow-up item, not a new regression introduced by this PR.
• list_team_evolutions applies card_type and tier WHERE clauses after the base query object is built but before count() is called. The ordering is correct — filters are applied before count. Consider adding a comment to make the filter-then-count ordering explicit, since Peewee queries are lazy and the sequence matters.

Verdict: APPROVED

Implementation is correct and follows all established patterns in this codebase. The shared _build_card_state_response helper is a clean design that avoids duplication between the two endpoints. Auth, error handling, and response shapes are consistent with the rest of the evolution API. The pg_conn fixture gap is pre-existing and not introduced by this PR.

---

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/evolution.py` (modified — new `_build_card_state_response` helper and `GET /evolution/cards/{card_id}` endpoint) - `app/routers_v2/teams.py` (modified — new `GET /{team_id}/evolutions` endpoint + style cleanup) - `tests/test_evolution_state_api.py` (added — 10 integration tests) ### Findings #### Correctness - `get_card_state`: Card lookup via `Card.get_by_id(card_id)` then EvolutionCardState JOIN is correct. The `card.player_id` / `card.team_id` raw FK accessor pattern is valid (Peewee auto-generates `_id` suffix for FK fields) — same pattern verified in PR #106. - `list_team_evolutions`: Peewee's `query.count()` wraps the full query (including the JOIN and WHERE clauses) in a subquery, so the `total` count correctly reflects all applied filters before slicing with `.offset().limit()`. - `_NEXT_THRESHOLD_ATTR` lookup: Handles all valid tiers (0-4) explicitly. For out-of-range tier values, `dict.get()` returns `None` (Python default), which is falsy, so `getattr(track, next_attr)` is never called — silently returns `None`. Acceptable behaviour given the DB constraint on tier values. - `_build_card_state_response` shared helper: Correctly called from both endpoints. The cross-router lazy import in `teams.py` (`from ..routers_v2.evolution import _build_card_state_response`) follows the lazy import pattern used throughout the codebase and avoids circular imports at module load time. - `lc_id` / `r_query` removals: `lc_id` was only referenced in a commented-out `sheets.post_new_cards` call — safe to remove. `r_query = Team.update(...)` was an unused assignment; `.execute()` is called directly now — correct. #### Security - Auth guard (`valid_token` → 401) is present on both new endpoints, consistent with all other endpoints in the router. - Token logging uses `[REDACTED]` — no credential exposure. Pattern matches the project standard. - User-supplied `card_id` and `team_id` are integer path/query params — FastAPI validates types before the handler runs. No injection vector. #### Style & Conventions - `noqa: E712` annotations on Peewee boolean comparisons (`== True` / `== False`) match the PostgreSQL-compatible pattern documented in the existing code and avoid ruff false positives. - f-string → plain string cleanup throughout `teams.py` (no interpolation in the string) is correct and reduces noise. #### Suggestions - `pg_conn` fixture is referenced in both `test_evolution_state_api.py` and the pre-existing `test_evolution_track_api.py` but is not defined in `tests/conftest.py`. This is a pre-existing gap (noted in PR #84 review). When `POSTGRES_HOST` is set, all integration tests in both files will fail with `fixture 'pg_conn' not found`. The `pg_conn` fixture (a `session`-scoped psycopg2 connection) should be added to `conftest.py`. This is a follow-up item, not a new regression introduced by this PR. - `list_team_evolutions` applies `card_type` and `tier` WHERE clauses *after* the base query object is built but *before* `count()` is called. The ordering is correct — filters are applied before count. Consider adding a comment to make the filter-then-count ordering explicit, since Peewee queries are lazy and the sequence matters. ### Verdict: APPROVED Implementation is correct and follows all established patterns in this codebase. The shared `_build_card_state_response` helper is a clean design that avoids duplication between the two endpoints. Auth, error handling, and response shapes are consistent with the rest of the evolution API. The `pg_conn` fixture gap is pre-existing and not introduced by this PR. --- *Automated review by Claude PR Reviewer*

cal commented 2026-03-18 20:14:56 +00:00

Author

Owner

Copy Link

AI Code Review

Files Reviewed

• app/routers_v2/evolution.py (modified — new endpoints and helper)
• app/routers_v2/teams.py (modified — new list endpoint + incidental linting fixes)
• tests/test_evolution_state_api.py (added)

---

Findings

Correctness

[BLOCKING] pg_conn fixture is undefined — tests will error, not skip

test_evolution_state_api.py uses pg_conn as a fixture argument throughout (e.g. seeded_data(pg_conn), test_list_filter_by_card_type(client, seeded_data, pg_conn)), but pg_conn is not defined in tests/conftest.py and is not defined in this file. The same gap exists in test_evolution_track_api.py (a pre-existing issue from WP-06), so this PR inherits and propagates a broken pattern.

When POSTGRES_HOST is not set, tests are skipped via @_skip_no_pg, but when it is set, pytest will error with fixture 'pg_conn' not found before any test body runs. This means the 10 integration tests cannot pass in CI. A pg_conn fixture returning a psycopg2 connection needs to be added to tests/conftest.py (scoped to session or module, guarded by POSTGRES_HOST).

Suggested addition to tests/conftest.py:

@pytest.fixture(scope="module")
def pg_conn():
    import psycopg2
    host = os.environ.get("POSTGRES_HOST")
    if not host:
        pytest.skip("POSTGRES_HOST not set")
    conn = psycopg2.connect(
        host=host,
        dbname=os.environ.get("POSTGRES_DB", "pd_master"),
        user=os.environ.get("POSTGRES_USER", "pd_admin"),
        password=os.environ.get("POSTGRES_PASSWORD", ""),
    )
    yield conn
    conn.close()

This fix also retroactively resolves the same gap in test_evolution_track_api.py from WP-06.

[MINOR] Out-of-range current_tier silently returns next_threshold: null

_NEXT_THRESHOLD_ATTR.get(state.current_tier) returns None for any value outside 0–4 (e.g. 5, -1). This makes a bad-data row indistinguishable from a legitimately fully-evolved card. The DB schema constrains tier to 0–4 in practice, but a defensive log warning would help diagnose corruption early.

[CONFIRMED CORRECT] card.player_id and card.team_id attribute access

Peewee exposes the raw FK integer via card.player_id (not requiring a join). The WHERE clause in get_card_state is correct as written.

[CONFIRMED CORRECT] total = query.count() reflects filtered total

Peewee wraps the filtered query in a subquery for .count(), so count in the paginated response reflects the total number of matching rows, not the page size. This is correct.

[CONFIRMED CORRECT] Removed unused variables in teams.py

The removed last_card/lc_id block and the r_query = assignment were both unused after their assignment. Correct cleanup.

Security

• Auth enforced on both new endpoints via if not valid_token(token) — consistent with existing pattern.
• No raw SQL interpolation. Peewee parameterises all queries. No injection risk.
• No hardcoded secrets or credentials.

Style & Conventions

• Lazy imports inside function bodies follow the established pattern in this router.
• Cross-router import in teams.py (from ..routers_v2.evolution import _build_card_state_response) is functional but creates coupling between two routers. Moving the helper to a shared module (e.g. app/routers_v2/evolution_helpers.py) would be cleaner at larger scale — non-blocking suggestion at current project size.
• _NEXT_THRESHOLD_ATTR module-level dict is clean and readable.
• model_to_dict(track, recurse=False) consistent with rest of codebase.
• The track dict in the response correctly exposes id, name, card_type, formula, and the four threshold fields — matches spec.
• Ordering by EvolutionCardState.player_id provides stable pagination since player_id is a primary key. Correct.
• All noqa: E712 suppressions in teams.py are appropriate for Peewee's ORM boolean filter pattern.

Test Coverage

All 10 acceptance criteria from the PR description are covered:

• Baseline list response shape
• card_type filter
• tier filter
• Pagination (page/per_page)
• Full response shape (all required fields)
• next_threshold at multiple tier levels including tier=4 null case
• card_id resolves via Card table join
• 404 when no evolution state exists
• Duplicate cards share one state row
• Auth required on both endpoints

The test docstrings and module-level docstring clearly explain the object graph, test matrix, and rationale for each test. Good structure.

One gap (non-blocking): no test for GET /teams/{team_id}/evolutions when the team has zero evolution states. Should return {"count": 0, "items": []}. Worth adding to prevent a future empty-list guard regression.

One gap (non-blocking): next_threshold at tier=3 is not explicitly tested (should equal t4_threshold=896). Tier=2 and tier=4 are tested, but tier=3 is skipped.

---

Verdict: REQUEST_CHANGES

The endpoint logic, response shape, auth enforcement, duplicate-card sharing, and test coverage are all correct and well-structured. The single blocking issue is the missing pg_conn fixture: without it, pytest will raise fixture 'pg_conn' not found when POSTGRES_HOST is set, causing all 10 integration tests to error before running. The fix is a small, one-time addition to tests/conftest.py that will also retroactively fix the same gap from WP-06.

---

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/evolution.py` (modified — new endpoints and helper) - `app/routers_v2/teams.py` (modified — new list endpoint + incidental linting fixes) - `tests/test_evolution_state_api.py` (added) --- ### Findings #### Correctness **[BLOCKING] `pg_conn` fixture is undefined — tests will error, not skip** `test_evolution_state_api.py` uses `pg_conn` as a fixture argument throughout (e.g. `seeded_data(pg_conn)`, `test_list_filter_by_card_type(client, seeded_data, pg_conn)`), but `pg_conn` is not defined in `tests/conftest.py` and is not defined in this file. The same gap exists in `test_evolution_track_api.py` (a pre-existing issue from WP-06), so this PR inherits and propagates a broken pattern. When `POSTGRES_HOST` is not set, tests are skipped via `@_skip_no_pg`, but when it *is* set, pytest will error with `fixture 'pg_conn' not found` before any test body runs. This means the 10 integration tests cannot pass in CI. A `pg_conn` fixture returning a psycopg2 connection needs to be added to `tests/conftest.py` (scoped to `session` or `module`, guarded by `POSTGRES_HOST`). Suggested addition to `tests/conftest.py`: ```python @pytest.fixture(scope="module") def pg_conn(): import psycopg2 host = os.environ.get("POSTGRES_HOST") if not host: pytest.skip("POSTGRES_HOST not set") conn = psycopg2.connect( host=host, dbname=os.environ.get("POSTGRES_DB", "pd_master"), user=os.environ.get("POSTGRES_USER", "pd_admin"), password=os.environ.get("POSTGRES_PASSWORD", ""), ) yield conn conn.close() ``` This fix also retroactively resolves the same gap in `test_evolution_track_api.py` from WP-06. **[MINOR] Out-of-range `current_tier` silently returns `next_threshold: null`** `_NEXT_THRESHOLD_ATTR.get(state.current_tier)` returns `None` for any value outside 0–4 (e.g. 5, -1). This makes a bad-data row indistinguishable from a legitimately fully-evolved card. The DB schema constrains tier to 0–4 in practice, but a defensive log warning would help diagnose corruption early. **[CONFIRMED CORRECT] `card.player_id` and `card.team_id` attribute access** Peewee exposes the raw FK integer via `card.player_id` (not requiring a join). The WHERE clause in `get_card_state` is correct as written. **[CONFIRMED CORRECT] `total = query.count()` reflects filtered total** Peewee wraps the filtered query in a subquery for `.count()`, so `count` in the paginated response reflects the total number of matching rows, not the page size. This is correct. **[CONFIRMED CORRECT] Removed unused variables in `teams.py`** The removed `last_card`/`lc_id` block and the `r_query =` assignment were both unused after their assignment. Correct cleanup. #### Security - Auth enforced on both new endpoints via `if not valid_token(token)` — consistent with existing pattern. - No raw SQL interpolation. Peewee parameterises all queries. No injection risk. - No hardcoded secrets or credentials. #### Style & Conventions - Lazy imports inside function bodies follow the established pattern in this router. - Cross-router import in `teams.py` (`from ..routers_v2.evolution import _build_card_state_response`) is functional but creates coupling between two routers. Moving the helper to a shared module (e.g. `app/routers_v2/evolution_helpers.py`) would be cleaner at larger scale — non-blocking suggestion at current project size. - `_NEXT_THRESHOLD_ATTR` module-level dict is clean and readable. - `model_to_dict(track, recurse=False)` consistent with rest of codebase. - The `track` dict in the response correctly exposes `id`, `name`, `card_type`, `formula`, and the four threshold fields — matches spec. - Ordering by `EvolutionCardState.player_id` provides stable pagination since `player_id` is a primary key. Correct. - All `noqa: E712` suppressions in `teams.py` are appropriate for Peewee's ORM boolean filter pattern. #### Test Coverage All 10 acceptance criteria from the PR description are covered: - Baseline list response shape - `card_type` filter - `tier` filter - Pagination (page/per_page) - Full response shape (all required fields) - `next_threshold` at multiple tier levels including tier=4 null case - `card_id` resolves via Card table join - 404 when no evolution state exists - Duplicate cards share one state row - Auth required on both endpoints The test docstrings and module-level docstring clearly explain the object graph, test matrix, and rationale for each test. Good structure. **One gap (non-blocking)**: no test for `GET /teams/{team_id}/evolutions` when the team has zero evolution states. Should return `{"count": 0, "items": []}`. Worth adding to prevent a future empty-list guard regression. **One gap (non-blocking)**: `next_threshold` at tier=3 is not explicitly tested (should equal `t4_threshold=896`). Tier=2 and tier=4 are tested, but tier=3 is skipped. --- ### Verdict: REQUEST_CHANGES The endpoint logic, response shape, auth enforcement, duplicate-card sharing, and test coverage are all correct and well-structured. The single blocking issue is the missing `pg_conn` fixture: without it, pytest will raise `fixture 'pg_conn' not found` when `POSTGRES_HOST` is set, causing all 10 integration tests to error before running. The fix is a small, one-time addition to `tests/conftest.py` that will also retroactively fix the same gap from WP-06. --- *Automated review by Claude PR Reviewer*

cal added 1 commit 2026-03-18 20:19:19 +00:00

fix: add missing pg_conn fixture to conftest.py ... 374b82f362

Session-scoped psycopg2 fixture that skips gracefully when
POSTGRES_HOST is absent (local dev) and connects in CI. Required
by seeded_data/seeded_tracks fixtures in evolution API tests.

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

cal force-pushed feature/wp-07-card-state-api from 374b82f362 to 503e570da5 2026-03-18 20:33:08 +00:00 Compare

cal merged commit e66a1ea9c4 into card-evolution 2026-03-18 20:33:19 +00:00

cal referenced this issue from a commit 2026-03-18 20:33:20 +00:00

Merge pull request 'feat(WP-07): Card State API endpoints (#72)' (#108) from feature/wp-07-card-state-api into card-evolution

cal deleted branch feature/wp-07-card-state-api 2026-03-18 20:33:20 +00:00

cal removed the

ai-reviewing

label 2026-03-23 15:33:00 +00:00

Sign in to join this conversation.

Reviewers

No reviewers

Claude

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

Subscribe

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

No description provided.