feat: Card Evolution Phase 1 — full backend implementation #130
No reviewers
Claude
Labels
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
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: cal/paper-dynasty-database#130
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "card-evolution"
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?
Summary
Merges the complete Card Evolution backend from the
card-evolutionintegration branch intomain. This is Phase 1 of the progression system (to be renamed to "Refractor" in a follow-up PR).What's included
EvolutionCardState,EvolutionTierBoost,EvolutionCosmetic)BattingSeasonStats/PitchingSeasonStatsmodelsGET /evolution/cards/{card_id})EvolutionCardStateon acquisitionGET /teams/{id}/evolutionsStats
Test plan
python -m pytestpasses on the merged codeevolutionreferences torefractor🤖 Generated with Claude Code
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>Implements two new API endpoints the bot calls after a game completes: POST /api/v2/season-stats/update-game/{game_id} Delegates to update_season_stats() service (WP-05). Returns {"updated": N, "skipped": bool} with idempotency via ProcessedGame ledger. POST /api/v2/evolution/evaluate-game/{game_id} Finds all (player_id, team_id) pairs from the game's StratPlay rows, calls evaluate_card() for each pair that has an EvolutionCardState, and returns {"evaluated": N, "tier_ups": [...]} with full tier-up detail. New files: app/services/evolution_evaluator.py — evaluate_card() service (WP-08) tests/test_postgame_evolution.py — 10 integration tests (all pass) Modified files: app/routers_v2/season_stats.py — rewritten to delegate to the service app/routers_v2/evolution.py — evaluate-game endpoint added app/main.py — season_stats router registered Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>AI Code Review (pr-reviewer agent)
Files Reviewed
app/db_engine.py— new models: BattingSeasonStats, PitchingSeasonStats, ProcessedGame, EvolutionTrack, EvolutionCardState, EvolutionTierBoost, EvolutionCosmeticapp/main.py— router registrationapp/routers_v2/evolution.py— tracks catalog + card state + evaluate endpointsapp/routers_v2/season_stats.py— POST /season-stats/update-game/{game_id}app/routers_v2/teams.py— GET /{team_id}/evolutionsapp/routers_v2/cards.py— pack opening hookapp/services/formula_engine.py— batter/SP/RP formula + tier resolutionapp/services/evolution_evaluator.py— evaluate_card entry pointapp/services/evolution_init.py— WP-10 pack opening hookapp/services/season_stats.py— full-recalculation engineapp/seed/evolution_tracks.{py,json}— t1_threshold keys + upsert-on-updatemigrations/2026-03-17_add_evolution_tables.sqlmigrations/2026-03-18_add_processed_game.sqltests/conftest.py+ 8 test filesFindings
Correctness
BUG (BLOCKER):
season_statsrouter is not registered inapp/main.py.app/routers_v2/season_stats.pydefinesPOST /api/v2/season-stats/update-game/{game_id}(WP-13), but neither the import norapp.include_router(season_stats.router)appears inapp/main.py. The endpoint is unreachable — every post-game stats call from the bot will hit 404. This must be added before merge.BUG (MEDIUM):
_get_player_pairsinseason_stats.py— missing null guard forpitcher_id.There is a null guard for
batter_idbut not forpitcher_id:If any StratPlay row has
pitcher_id = NULL, this adds(None, team_id)topitching_pairs, which will either cause a FK lookup error or a silent corrupt write intopitching_season_stats. Addif pitcher_id is not None:to mirror the batter guard.CONCERN:
EvolutionTrack.card_typeis not unique at the model or migration level.evolution_init.pyusesEvolutionTrack.get(EvolutionTrack.card_type == card_type)which raisesMultipleObjectsReturnedif two tracks share the samecard_type. The Peewee model lacksunique=Trueon that field, and the SQL migration adds no unique index oncard_type(only onname). Addingunique=Trueto the model field and aCREATE UNIQUE INDEXin the migration would prevent a subtle future failure.Security
No SQL injection risks. All paths use parameterized queries or Peewee ORM. Path parameters are typed as
int. Auth is correctly applied on every mutating endpoint and on all new read endpoints. No hardcoded credentials.Style & Conventions
evolution.pymixeslogging.warning()(module-level) with the module-levelloggerinstance for auth-failure lines. Low severity inconsistency.season_stats.pywraps the entire game update indb.atomic(). Correct — no partial game writes.evolution_init.pyis correctly fire-and-forget with exceptions caught at all levels. Pack opening is never blocked.Suggestions (non-blocking)
_season_stats_module.db = _test_dbpatch intest_season_stats_update.pyworks but is fragile if import order changes. Amonkeypatchfixture or DI would be more robust.pitcher_id = Noneedge case in_get_player_pairsonce the null guard is in place.unique=TruetoEvolutionTrack.card_typein the Peewee model.Verdict: REQUEST CHANGES
The overall architecture is solid. Full-recalculation with the ProcessedGame ledger is a clean, correct approach. Formula engine is pure and well-tested. Pack opening hook is safely fire-and-forget. Test coverage across 8 files is thorough.
Two items must be fixed before merge:
season_stats.routernot registered inmain.py— WP-13 post-game callback is completely unreachable until this is added.pitcher_idnull guard in_get_player_pairs— can silently corrupt pitching stats for edge-case play types.Automated review by Claude PR Reviewer (claude-sonnet-4-6)
AI Code Review
Files Reviewed
app/db_engine.py(modified — Evolution models added, ProcessedGame added)app/main.py(modified — season_stats router registered)app/routers_v2/cards.py(modified — WP-10 evolution hook)app/routers_v2/evolution.py(modified — card state + evaluate endpoints)app/routers_v2/season_stats.py(modified — delegated to service layer)app/routers_v2/teams.py(modified — evolutions endpoint + f-string cleanup)app/seed/evolution_tracks.json(modified — renamed threshold fields)app/seed/evolution_tracks.py(modified — rewritten seed script)app/services/evolution_evaluator.py(added — WP-08)app/services/evolution_init.py(added — WP-10)app/services/formula_engine.py(modified — t1..t4 → t1_threshold..t4_threshold)app/services/season_stats.py(added — full-recalc season stats service)migrations/2026-03-17_add_evolution_tables.sql(added — WP-04)migrations/2026-03-18_add_processed_game.sql(added — ProcessedGame)ruff.toml(added)tests/conftest.py(modified — SQLite in-memory fixtures + pg_conn)tests/test_evolution_*.py(8 test files added/modified)Findings
Correctness
[BLOCKER]
datetime.utcnow()reintroduced inevolution_evaluator.pyPR #118 (
chore: replace deprecated datetime.utcnow()) was merged to systematically replace alldatetime.utcnow()calls withdatetime.now(). This PR reintroduces it inapp/services/evolution_evaluator.pyat the point wherelast_evaluated_atis written back to the card state:This is a direct regression from the fix in #118. Replace with
datetime.now()to match the established codebase convention.[INFO] Season stats response schema change is intentional but documented inconsistently
The old
/update-game/{game_id}returned{"updated": N}. The new shape is{"updated": N, "skipped": bool}. The router correctly mapsbatters_updated + pitchers_updated→updated. This is a breaking change for bot callers that check the response shape, but the docstring in the router is clear about the new shape. No code fix required — just flagging for the bot-side consumer to verify.[INFO] Two critical gaps from PR #100 are now resolved
EvolutionTrackand evolution models are now properly defined indb_engine.pyand registered withdb.create_tables()— closes the runtimeImportErrorthat would have hit/api/v2/evolution/tracks.season_stats.routeris now registered inmain.py— closes the unreachable endpoint gap.Security
No issues found. Token validation is consistently applied across all new endpoints.
logging.warning("Bad Token: [REDACTED]")(not interpolated) is used correctly throughout — no token leakage. No raw SQL with user input in the new service layer. Theforcequery param is auth-gated (behind the same bearer token check) so it cannot be abused without a valid token.Style & Conventions
[SUGGESTION]
EvolutionTrack.get(card_type == ...)could raiseMultipleObjectsReturnedapp/services/evolution_init.py:card_typehas noUNIQUEconstraint in either the ORM model or the migration SQL. The seed script (get_or_createkeyed onname) prevents duplicates from the seeder, but a manually inserted row or a future extended seed could causeMultipleObjectsReturned— which is not caught by theexcept DoesNotExistbelow it, so it would propagate as an unhandled 500.Consider adding
UNIQUEconstraint oncard_typetoevolution_trackin the migration, or using.get_or_none()with a fallback, or catchingMultipleObjectsReturnedexplicitly. At minimum, catch it in the outerexcept Exception(which is present and will swallow it safely) — but the current explicitexcept DoesNotExistbefore theexcept Exceptionwill not catch it.Wait — looking again at the structure: the outer try catches
Exception, soMultipleObjectsReturnedIS caught at line ~95 and returnsNone. False alarm on crash risk, but the track lookup would silently fail for that player rather than returning the right track. Worth noting as a latent bug.[SUGGESTION] N+1 on
state.track.nameinevaluate_gameapp/routers_v2/evolution.py—evaluate_game:state.tracktriggers a lazy-load FK query per tier-up. Since tier-ups are infrequent this is low-impact, but thestatewas already loaded at the top of the loop. A simple fix would be to usestate.track_idto look up the track name in a pre-fetched dict, or join the track in theget_or_nonequery.[INFO]
noqa: E402added tomain.pyimports — correct, as these imports follow the logging setup block.[INFO]
ruff.tomladded —F403/F405suppression fordb_engine.py's star import is appropriate.Test Coverage
Excellent.
pg_connfixture finally added (resolves the pre-existing gap noted in PRs #84 and #108). 8 test files covering unit and integration scenarios:test_evolution_models.py— ORM-level CRUD and constraint teststest_evolution_evaluator.py— tier progression, no-regression rule, fully_evolved flagtest_evolution_init.py— fire-and-forget guarantee, idempotency, missing track safetytest_evolution_seed.py— upsert/update behavior of seed scripttest_formula_engine.py— threshold boundary conditionstest_evolution_state_api.py— API endpoint contract teststest_postgame_evolution.py— WP-13 post-game callback integrationtest_season_stats_update.py— full-recalc service + idempotency + force flagThe use of a named SQLite shared-memory URI in
test_postgame_evolution.pyto handle cross-thread TestClient access is a thoughtful solution.Verdict: REQUEST_CHANGES
Single blocker:
datetime.utcnow()inevolution_evaluator.pyis a regression from PR #118. Replace withdatetime.now(). Everything else is in good shape — the core architecture is sound, migrations are complete with rollback statements, tests are comprehensive, and the two critical gaps from PR #100 are finally resolved.Automated review by Claude PR Reviewer
Both blockers resolved: pitcher_id null guard added (
d15fc97), datetime.utcnow() replaced with datetime.now() (5ea4c7c). 117 tests pass.All blockers resolved. Tests pass (117 passed). Approving for merge.
AI Code Review
Files Reviewed
app/db_engine.py(modified) — EvolutionTrack, EvolutionCardState, EvolutionTierBoost, EvolutionCosmetic, ProcessedGame models addedapp/main.py(modified) — season_stats router registeredapp/routers_v2/cards.py(modified) — WP-10 evolution hook on card POSTapp/routers_v2/evolution.py(modified) — GET /cards/{id}, POST /cards/{id}/evaluate, POST /evaluate-game/{id}app/routers_v2/season_stats.py(modified) — POST /update-game/{id} rewritten to delegate to serviceapp/routers_v2/teams.py(modified) — GET /{team_id}/evolutions addedapp/services/evolution_evaluator.py(added)app/services/evolution_init.py(added)app/services/formula_engine.py(modified) — t1/t2/t3/t4 → t1_threshold…t4_thresholdapp/services/season_stats.py(added) — full-recalc engine with ProcessedGame idempotencymigrations/2026-03-17_add_evolution_tables.sql(added)migrations/2026-03-18_add_processed_game.sql(added)ruff.toml(added)tests/conftest.py(rewritten)tests/test_evolution_evaluator.py,test_evolution_init.py,test_evolution_models.py,test_evolution_seed.py,test_evolution_state_api.py,test_formula_engine.py,test_postgame_evolution.py,test_season_stats_update.py(added)Findings
Correctness
datetime.utcnow()is gone —evolution_evaluator.pynow correctly usesdatetime.now()throughout. PR #118's fix is preserved.create_tables();season_stats.routerregistered inmain.py;pg_connfixture added toconftest.py.get_or_create(game_id=game_id)correctly uses the raw FK column accessor for the PK FK field.db.atomic()wrapping ensures ledger + stats writes roll back together on failure.force=Truebypass is auth-gated.max(card_state.current_tier, new_tier)correctly prevents tier regression.tier_from_valuedict key rename (t1→t1_threshold) is consistent across model, migration SQL, seed JSON, and tests._build_card_state_response:_NEXT_THRESHOLD_ATTR.get(state.current_tier)+getattr(track, next_attr) if next_attr else Nonecorrectly handles tier 4 (fully evolved) withNone.BEGIN/COMMIT, rollback DROP statements present (convention satisfied). FK references correct.bulk_createatomic block — pack insertion abort before hook runs → no phantom evolution states. Fire-and-forget with broadexcept Exceptionis correct for this path._build_card_state_responseaccessesstate.trackwithout additional queries.count()on the base (pre-paginated) query is correct for total.Security
valid_token(token)checked on all protected endpoints; "Bad Token: [REDACTED]" used throughout (no raw token logging).force=Trueidempotency bypass is gated behind the existing bearer token — acceptable for stat-correction use case.Style & Conventions
noqa: E402on main.py imports correct (logging configured before app imports).ruff.tomlF403/F405 suppression is the right fix forfrom peewee import *in db_engine.logger.warning(f"Evolution eval failed for player={player_id}…")inevolution.py:413— ruff prefers%-style lazy formatting in log calls.f"Season stats update failed for game {game_id}: {exc}"inseason_stats.py:519(HTTPException detail) — f-string here is fine since it's not a log call.Suggestions
EvolutionTrack.get(EvolutionTrack.card_type == card_type)inevolution_init.py: If multiple tracks are ever seeded for the samecard_type(onlynamehas a UNIQUE constraint, notcard_type),MultipleObjectsReturnedis silently swallowed by the outerexcept Exception → return None. Consider.order_by(EvolutionTrack.id).first()or adding a UNIQUE index oncard_typeif one-track-per-type is the intended invariant.evaluate_gametier-ups:state.track.nametriggers a lazy FK load andPlayer.get_by_id(player_id)runs per tier-up. Since tier-ups are rare in practice this is acceptable; flag for cleanup if the endpoint is called in high-frequency paths._season_stats_module.db = _test_dbpatch: The module-leveldbrebind is the correct approach for SQLite shared-memory tests. Well-documented rationale in the test file.Verdict: APPROVED
The previous
REQUEST_CHANGESblocker (datetime.utcnow()regression) is fixed. All three criticals from PR #100 are resolved. The implementation is well-structured — full-recalc idempotency via ProcessedGame, no-tier-regression guarantee, comprehensive 8-file test suite (~2,500 lines). Remaining items are suggestions only and do not block merge.Automated review by Claude PR Reviewer