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

fix: include player_name in /teams/{id}/evolutions response (#115) #119

Merged
cal merged 1 commits from ai/paper-dynasty-database#115 into next-release 2026-03-19 18:20:22 +00:00
Conversation 0 Commits 1 Files Changed 1 +9 -4
Claude commented 2026-03-19 18:03:47 +00:00
Collaborator
Copy Link

Closes #115

Summary

GET /api/v2/teams/{team_id}/evolutions was missing player_name in each item, causing the Discord bot's /evo status to fall back to "Player #795" instead of the actual name.

Changes

app/routers_v2/teams.py — list_team_evolutions:

  • Added Player to the .select() and joined it via .switch(EvolutionCardState).join(Player) — eager-loads player data in a single query (no N+1).
  • Added item["player_name"] = state.player.p_name to each serialized item.

Response shape (after)

{
  "player_id": 795,
  "player_name": "Tyler Wells",
  "team_id": 31,
  "current_tier": 1,
  "current_value": 5.0,
  ...
}

Tests

No test suite in this repo. Fix verified by reading back the modified function.

Closes #115 ## Summary `GET /api/v2/teams/{team_id}/evolutions` was missing `player_name` in each item, causing the Discord bot's `/evo status` to fall back to "Player #795" instead of the actual name. ## Changes **`app/routers_v2/teams.py`** — `list_team_evolutions`: - Added `Player` to the `.select()` and joined it via `.switch(EvolutionCardState).join(Player)` — eager-loads player data in a single query (no N+1). - Added `item["player_name"] = state.player.p_name` to each serialized item. ## Response shape (after) ```json { "player_id": 795, "player_name": "Tyler Wells", "team_id": 31, "current_tier": 1, "current_value": 5.0, ... } ``` ## Tests No test suite in this repo. Fix verified by reading back the modified function.
Claude added 1 commit 2026-03-19 18:03:48 +00:00 
JOIN the Player table in the evolutions query so p_name can be
included in each serialized item without N+1 queries.

Closes #115

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cal approved these changes 2026-03-19 18:17:34 +00:00
cal left a comment
Owner
Copy Link

AI Code Review

Files Reviewed

  • app/routers_v2/teams.py (modified)

Findings

Correctness

  • The JOIN is correct. EvolutionCardState has player = ForeignKeyField(Player) in db_engine.py, so .switch(EvolutionCardState).join(Player) is the right Peewee idiom after the existing .join(EvolutionTrack). Both FK targets are eager-loaded in a single SQL query — no N+1.
  • state.player.p_name is the correct attribute name per the Player model definition (p_name = CharField()). The CLAUDE.md critical note ("API parameter for player name is name, NOT p_name") refers to the HTTP request parameter name for player lookup, not the ORM column — this internal use of p_name is appropriate.
  • state.player_id (the pre-existing FK integer accessor) is unaffected; Peewee keeps both the raw integer accessor and the relation accessor separate.
  • The blank line removal at line 1049 is an unrelated whitespace cleanup with no behavioral impact.

Edge Cases

  • No null risk on p_name: the Player model defines it as a non-nullable CharField(). If somehow a player row is missing (referential integrity broken), Peewee would raise an AttributeError at state.player.p_name, which would surface as a 500. This is acceptable for what would be a corrupt DB state, and mirrors how the rest of the codebase handles such cases.
  • Pagination, filtering, and count logic are untouched and remain correct.

Security

  • No new user-controlled data is introduced into queries. The player name is read from the DB and returned as-is, not interpolated into any query or template.
  • No issues found.

Style & Conventions

  • The loop-based serialization replacing the list comprehension is idiomatic for adding per-item side-effects and matches patterns used elsewhere in the file.
  • Imports remain lazy (inside the function body), consistent with how the function already imported EvolutionCardState and EvolutionTrack.
  • Base branch is next-release, matching the release workflow described in CLAUDE.md.

Suggestions

  • The docstring for list_team_evolutions (line 1555-1558) still says "Each item in 'items' has the same shape as GET /evolution/cards/{card_id}." This is now slightly inaccurate since the /teams/{id}/evolutions response adds player_name which the single-card endpoint does not return. Worth updating in a follow-up, but not a blocker.
  • The single-card endpoint at GET /evolution/cards/{card_id} (in evolution.py) calls the same _build_card_state_response without joining Player. If that endpoint ever needs player_name too, it will need the same treatment. Not a problem introduced by this PR.

Verdict: APPROVED

The change is minimal, correct, and solves the stated problem cleanly. The JOIN is a proper eager load, player_name is correctly sourced from p_name, and no existing fields or behavior are disrupted.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/teams.py` (modified) ### Findings #### Correctness - The JOIN is correct. `EvolutionCardState` has `player = ForeignKeyField(Player)` in `db_engine.py`, so `.switch(EvolutionCardState).join(Player)` is the right Peewee idiom after the existing `.join(EvolutionTrack)`. Both FK targets are eager-loaded in a single SQL query — no N+1. - `state.player.p_name` is the correct attribute name per the `Player` model definition (`p_name = CharField()`). The CLAUDE.md critical note ("API parameter for player name is `name`, NOT `p_name`") refers to the HTTP request parameter name for player lookup, not the ORM column — this internal use of `p_name` is appropriate. - `state.player_id` (the pre-existing FK integer accessor) is unaffected; Peewee keeps both the raw integer accessor and the relation accessor separate. - The blank line removal at line 1049 is an unrelated whitespace cleanup with no behavioral impact. #### Edge Cases - No null risk on `p_name`: the `Player` model defines it as a non-nullable `CharField()`. If somehow a player row is missing (referential integrity broken), Peewee would raise an `AttributeError` at `state.player.p_name`, which would surface as a 500. This is acceptable for what would be a corrupt DB state, and mirrors how the rest of the codebase handles such cases. - Pagination, filtering, and count logic are untouched and remain correct. #### Security - No new user-controlled data is introduced into queries. The player name is read from the DB and returned as-is, not interpolated into any query or template. - No issues found. #### Style & Conventions - The loop-based serialization replacing the list comprehension is idiomatic for adding per-item side-effects and matches patterns used elsewhere in the file. - Imports remain lazy (inside the function body), consistent with how the function already imported `EvolutionCardState` and `EvolutionTrack`. - Base branch is `next-release`, matching the release workflow described in CLAUDE.md. #### Suggestions - The docstring for `list_team_evolutions` (line 1555-1558) still says "Each item in 'items' has the same shape as GET /evolution/cards/{card_id}." This is now slightly inaccurate since the `/teams/{id}/evolutions` response adds `player_name` which the single-card endpoint does not return. Worth updating in a follow-up, but not a blocker. - The single-card endpoint at `GET /evolution/cards/{card_id}` (in `evolution.py`) calls the same `_build_card_state_response` without joining `Player`. If that endpoint ever needs `player_name` too, it will need the same treatment. Not a problem introduced by this PR. ### Verdict: APPROVED The change is minimal, correct, and solves the stated problem cleanly. The JOIN is a proper eager load, `player_name` is correctly sourced from `p_name`, and no existing fields or behavior are disrupted. --- *Automated review by Claude PR Reviewer*
Claude added the
ai-reviewing
 label 2026-03-19 18:18:17 +00:00
Claude reviewed 2026-03-19 18:19:35 +00:00
Claude left a comment
Author
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • app/routers_v2/teams.py (modified)

Findings

Correctness

  • Fix is correct. Player is added to .select() and eagerly loaded via .switch(EvolutionCardState).join(Player). This is the correct multi-table join pattern in Peewee — after joining EvolutionTrack from EvolutionCardState, .switch(EvolutionCardState) resets the join pivot back to the base model before chaining .join(Player). The result is a single SQL query with two JOINs rather than N+1 player lookups.
  • state.player.p_name is valid. EvolutionCardState.player is declared as ForeignKeyField(Player) (db_engine.py line 1228). Player.p_name is a CharField (db_engine.py line 281). With Player in .select(), Peewee populates the relation without an additional query.
  • item["player_name"] = ... is safe. _build_card_state_response returns a plain dict, so mutation after the call is fine. The key player_name does not conflict with any existing key in that dict (player_id, team_id, current_tier, current_value, fully_evolved, last_evaluated_at, track, next_threshold).
  • The unrelated blank-line removal at line 1049 (team_buy_players) is cosmetic and harmless.

Security

  • No issues. The [REDACTED] token log pattern is preserved. player_name is sourced from a DB-backed Player row — no user-supplied data flows into the response field. No credential exposure.

Style & Conventions

  • Redundant lazy import of Player. Player is already imported at module scope on line 18 (from ..db_engine import (..., Player, ...)). Adding it again inside the lazy import block (from ..db_engine import EvolutionCardState, EvolutionTrack, Player) is harmless — Python deduplicates module-level names — but it is slightly inconsistent. The lazy import block exists specifically for EvolutionCardState and EvolutionTrack (which are not in the module-level import) to avoid circular-import issues. Player does not need to be there.

Suggestions

  • The docstring on list_team_evolutions still says items have "the same shape as GET /evolution/cards/{card_id}" but the response now includes an additional player_name field that the single-card endpoint does not return. Updating the docstring to document player_name would prevent consumer confusion. Not a blocker.

Verdict: APPROVED

Correct, focused fix. The multi-table Peewee join is sound, p_name is a valid field, and the response mutation is safe. The redundant lazy import of Player and the stale docstring are minor nits that do not affect correctness or security.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/teams.py` (modified) ### Findings #### Correctness - **Fix is correct.** `Player` is added to `.select()` and eagerly loaded via `.switch(EvolutionCardState).join(Player)`. This is the correct multi-table join pattern in Peewee — after joining `EvolutionTrack` from `EvolutionCardState`, `.switch(EvolutionCardState)` resets the join pivot back to the base model before chaining `.join(Player)`. The result is a single SQL query with two JOINs rather than N+1 player lookups. - **`state.player.p_name` is valid.** `EvolutionCardState.player` is declared as `ForeignKeyField(Player)` (db_engine.py line 1228). `Player.p_name` is a `CharField` (db_engine.py line 281). With `Player` in `.select()`, Peewee populates the relation without an additional query. - **`item["player_name"] = ...` is safe.** `_build_card_state_response` returns a plain `dict`, so mutation after the call is fine. The key `player_name` does not conflict with any existing key in that dict (`player_id`, `team_id`, `current_tier`, `current_value`, `fully_evolved`, `last_evaluated_at`, `track`, `next_threshold`). - **The unrelated blank-line removal** at line 1049 (`team_buy_players`) is cosmetic and harmless. #### Security - No issues. The `[REDACTED]` token log pattern is preserved. `player_name` is sourced from a DB-backed `Player` row — no user-supplied data flows into the response field. No credential exposure. #### Style & Conventions - **Redundant lazy import of `Player`.** `Player` is already imported at module scope on line 18 (`from ..db_engine import (..., Player, ...)`). Adding it again inside the lazy import block (`from ..db_engine import EvolutionCardState, EvolutionTrack, Player`) is harmless — Python deduplicates module-level names — but it is slightly inconsistent. The lazy import block exists specifically for `EvolutionCardState` and `EvolutionTrack` (which are not in the module-level import) to avoid circular-import issues. `Player` does not need to be there. #### Suggestions - The docstring on `list_team_evolutions` still says items have "the same shape as GET /evolution/cards/{card_id}" but the response now includes an additional `player_name` field that the single-card endpoint does not return. Updating the docstring to document `player_name` would prevent consumer confusion. Not a blocker. ### Verdict: APPROVED Correct, focused fix. The multi-table Peewee join is sound, `p_name` is a valid field, and the response mutation is safe. The redundant lazy import of `Player` and the stale docstring are minor nits that do not affect correctness or security. --- *Automated review by Claude PR Reviewer*
cal merged commit 57d8a929fd into next-release 2026-03-19 18:20:22 +00:00
cal deleted branch ai/paper-dynasty-database#115 2026-03-19 18:20:23 +00:00
Claude removed the
ai-reviewing
 label 2026-03-19 18:20:28 +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#119
No description provided.
Delete Branch "ai/paper-dynasty-database#115"

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?