fix(dev): address review feedback on rollback endpoint (#215)

- Move evaluate_card import to top of file (was lazy import in function body)
- Collect audit rows for test-created states in step 5 so step 7 deletes them
  before step 6b deletes BattingCard/PitchingCard rows; previously those card
  rows had RefractorBoostAudit FK RESTRICT references that caused PostgreSQL
  violations (SQLite tests passed because FK enforcement is off by default)
- Update stale comments in steps 6/7/9 to reflect new deletion ordering

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Cal Corum 2026-04-11 10:32:34 -05:00
parent b44c4a6949
commit 42d6f8b195

View File

@ -30,6 +30,7 @@ from app.db_engine import (
StratGame,
StratPlay,
)
from app.services.refractor_evaluator import evaluate_card
from app.services.season_stats import (
_recalc_batting,
_recalc_pitching,
@ -214,6 +215,29 @@ def rollback_test_game(game_id: int) -> dict:
"game may not be a synthetic test game"
)
# Also collect audit rows for test-created states.
# All audit rows for test-created states were written by this game
# (that is how we identified them as test-created in step 4).
# We must delete them explicitly here rather than via cascade in step 9
# because step 6b deletes BattingCard/PitchingCard rows before step 9
# runs, and RefractorBoostAudit has a FK RESTRICT to those card rows.
if state_ids_to_delete:
if processed_at is not None:
test_created_audits = list(
RefractorBoostAudit.select().where(
(RefractorBoostAudit.card_state.in_(state_ids_to_delete))
& (RefractorBoostAudit.applied_at >= window_start)
& (RefractorBoostAudit.applied_at <= window_end)
)
)
else:
test_created_audits = list(
RefractorBoostAudit.select().where(
RefractorBoostAudit.card_state.in_(state_ids_to_delete)
)
)
audit_rows_to_delete.extend(test_created_audits)
# ------------------------------------------------------------------
# Step 6: Collect variant card IDs to delete (do NOT delete yet).
# RefractorBoostAudit has a FK to BattingCard/PitchingCard, so the
@ -223,8 +247,7 @@ def rollback_test_game(game_id: int) -> dict:
pitching_card_ids_to_delete: list[int] = []
# Collect from both reset states and test-created states.
# For test-created states, their audit rows will be cascade-deleted
# by step 9, but we still need the card IDs now.
# We need the card IDs to delete the card rows in step 6b.
all_state_ids_for_variants = reset_state_ids + state_ids_to_delete
if all_state_ids_for_variants and processed_at is not None:
variant_audits = list(
@ -254,10 +277,9 @@ def rollback_test_game(game_id: int) -> dict:
)
# ------------------------------------------------------------------
# Step 7: Delete audit rows for reset states FIRST.
# The audit table has a FK to battingcard/pitchingcard — the audit
# Step 7: Delete audit rows (reset states + test-created states).
# The audit table has a FK to battingcard/pitchingcard — all audit
# rows must be gone before we can delete the card rows in step 6b.
# Test-created states' audit rows are cascade-deleted in step 9.
# ------------------------------------------------------------------
audit_ids_to_delete = [a.id for a in audit_rows_to_delete]
if audit_ids_to_delete:
@ -311,7 +333,7 @@ def rollback_test_game(game_id: int) -> dict:
# ------------------------------------------------------------------
# Step 9: Delete test-created RefractorCardState rows.
# Cascade deletes their audit rows automatically.
# Their audit rows were already deleted in step 7; cascade is a no-op.
# ------------------------------------------------------------------
if state_ids_to_delete:
RefractorCardState.delete().where(
@ -471,8 +493,6 @@ def rollback_test_game(game_id: int) -> dict:
# Because step 8 reset current_tier to the pre-test value, the
# no-regression max() in evaluate_card produces the correct tier.
# ------------------------------------------------------------------
from app.services.refractor_evaluator import evaluate_card # noqa: PLC0415
for state in states_to_reset:
try:
evaluate_card(state.player_id, state.team_id, dry_run=False)