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

feat: Refractor Phase 2 integration — wire boost into evaluate-game #177

Merged
cal merged 2 commits from feature/refractor-phase2-integration into main 2026-03-30 18:17:30 +00:00
Conversation 0 Commits 2 Files Changed 7 +2814 -58
cal commented 2026-03-30 18:06:32 +00:00
Owner
Copy Link

Summary

  • app/services/refractor_boost.py — Added apply_tier_boost() orchestration function (creates boosted variant card + ratings on tier-up, writes audit record, updates tier+variant atomically via db.atomic()), compute_batter_display_stats() and compute_pitcher_display_stats() helpers, card_type validation guard
  • app/services/refractor_evaluator.py — Added dry_run parameter to evaluate_card() so tier computation is separated from tier write; added computed_tier and computed_fully_evolved to return dict
  • app/routers_v2/refractor.py — Wired boost into evaluate_game(): calls evaluate_card(dry_run=True), loops through intermediate tiers on tier-up, handles partial multi-tier failures, REFRACTOR_BOOST_ENABLED env var kill switch, fixed pre-existing isoformat() crash
  • tests/test_refractor_boost_integration.py (new, 27 tests) — Full integration coverage: batter/pitcher tier-up flows, cumulative T1-T4, idempotency, Card propagation, atomicity rollback, dry_run, cross-player isolation, card_type validation
  • tests/test_refractor_boost.py (12 new tests) — Display stat helper unit tests, clarified 79-sum references
  • tests/test_postgame_refractor.py (7 new tests) — HTTP endpoint tests: kill switch, multi-tier jump, pitcher path, variant_created field, empty card_type
  • tests/test_refractor_evaluator.py (6 new tests) — dry_run unit tests

Test plan

  • python -m pytest tests/test_refractor_boost.py -v — 38/38 passed
  • python -m pytest tests/test_refractor_boost_integration.py -v — 27/27 passed
  • python -m pytest tests/test_postgame_refractor.py -v — 18/18 passed
  • python -m pytest tests/test_refractor_evaluator.py -v — 27/27 passed
  • Full suite: 223 passed, 15 skipped (PG integration), 0 failed

🤖 Generated with Claude Code

## Summary - **`app/services/refractor_boost.py`** — Added `apply_tier_boost()` orchestration function (creates boosted variant card + ratings on tier-up, writes audit record, updates tier+variant atomically via `db.atomic()`), `compute_batter_display_stats()` and `compute_pitcher_display_stats()` helpers, `card_type` validation guard - **`app/services/refractor_evaluator.py`** — Added `dry_run` parameter to `evaluate_card()` so tier computation is separated from tier write; added `computed_tier` and `computed_fully_evolved` to return dict - **`app/routers_v2/refractor.py`** — Wired boost into `evaluate_game()`: calls `evaluate_card(dry_run=True)`, loops through intermediate tiers on tier-up, handles partial multi-tier failures, `REFRACTOR_BOOST_ENABLED` env var kill switch, fixed pre-existing `isoformat()` crash - **`tests/test_refractor_boost_integration.py`** (new, 27 tests) — Full integration coverage: batter/pitcher tier-up flows, cumulative T1-T4, idempotency, Card propagation, atomicity rollback, dry_run, cross-player isolation, card_type validation - **`tests/test_refractor_boost.py`** (12 new tests) — Display stat helper unit tests, clarified 79-sum references - **`tests/test_postgame_refractor.py`** (7 new tests) — HTTP endpoint tests: kill switch, multi-tier jump, pitcher path, variant_created field, empty card_type - **`tests/test_refractor_evaluator.py`** (6 new tests) — dry_run unit tests ## Test plan - [x] `python -m pytest tests/test_refractor_boost.py -v` — 38/38 passed - [x] `python -m pytest tests/test_refractor_boost_integration.py -v` — 27/27 passed - [x] `python -m pytest tests/test_postgame_refractor.py -v` — 18/18 passed - [x] `python -m pytest tests/test_refractor_evaluator.py -v` — 27/27 passed - [x] Full suite: 223 passed, 15 skipped (PG integration), 0 failed 🤖 Generated with [Claude Code](https://claude.com/claude-code)
cal added 1 commit 2026-03-30 18:06:33 +00:00 
When a card reaches a new Refractor tier during game evaluation, the
system now creates a boosted variant card with modified ratings. This
connects the Phase 2 Foundation pure functions (PR #176) to the live
evaluate-game endpoint.

Key changes:
- evaluate_card() gains dry_run parameter so apply_tier_boost() is the
  sole writer of current_tier, ensuring atomicity with variant creation
- apply_tier_boost() orchestrates the full boost flow: source card
  lookup, boost application, variant card + ratings creation, audit
  record, and atomic state mutations inside db.atomic()
- evaluate_game() calls evaluate_card(dry_run=True) then loops through
  intermediate tiers on tier-up, with error isolation per player
- Display stat helpers compute fresh avg/obp/slg for variant cards
- REFRACTOR_BOOST_ENABLED env var provides a kill switch
- 51 new tests: unit tests for display stats, integration tests for
  orchestration, HTTP endpoint tests for multi-tier jumps, pitcher
  path, kill switch, atomicity, idempotency, and cross-player isolation
- Clarified all "79-sum" references to note the 108-total card invariant

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cal reviewed 2026-03-30 18:13:47 +00:00
cal left a comment
Author
Owner
Copy Link

Review — feat: Refractor Phase 2 integration

Verdict: COMMENT (self-review restriction; no blocking defects but two issues below warrant a fix before merge)

Overall assessment

The architecture is well-conceived. Using dry_run=True in the evaluator to separate tier-detection from tier-write, then delegating the write to apply_tier_boost() which commits tier + variant + audit atomically in a single db.atomic() block, is the correct design. The kill switch, partial multi-tier failure handling (report last-successful-tier, retry remainder next game), and card propagation all follow from the design cleanly. Test coverage is exemplary — 52 new tests across four files with detailed docstrings and verified math. The isoformat() crash fix is also correct: hasattr(..., "isoformat") safely handles the case where SQLite returns a string instead of a datetime object.

Two issues need attention before this ships.

Issue 1 — import os inside function body (violates CLAUDE.md)

app/routers_v2/refractor.py, inside evaluate_game():

import os

from ..db_engine import RefractorCardState, Player, StratPlay
from ..services.refractor_boost import apply_tier_boost

os is a stdlib module with zero circular-import risk. The lazy-import pattern used here (and throughout the router) exists specifically to avoid circular dependencies with db_engine. Placing import os inside a function is inconsistent with that rationale and violates the CLAUDE.md rule: "Never add lazy imports to middle of file."

Fix: move import os to the top-level imports in refractor.py.

Issue 2 — Audit idempotency gap: tests pass under SQLite but second call raises IntegrityError in PostgreSQL

apply_tier_boost() guards card creation (step 6) and ratings creation (step 7) with get_or_none checks — both are idempotent. The audit write (step 8a, inside db.atomic()) is not guarded:

with _db.atomic():
    _audit_model.create(**audit_data)   # No existence check
    card_state.current_tier = new_tier
    card_state.save()

The PostgreSQL RefractorBoostAudit table has UNIQUE(card_state_id, tier) (established in the Phase 2 foundation migration). Calling apply_tier_boost a second time for the same (player_id, team_id, tier) will:

  1. Find the existing card and ratings — skip creation ✅
  2. Re-fetch card_state from DB (already at new_tier) ✅
  3. Enter db.atomic(), attempt create(**audit_data) — IntegrityError 💥
  4. Atomic block rolls back (card_state save is never committed)
  5. IntegrityError propagates to caller

The TestIdempotency tests call apply_tier_boost twice without expecting an exception, and pass — because the SQLite in-memory database does not enforce the UNIQUE constraint (it isn't in the Peewee model's Meta.indexes, only in the SQL migration). In PostgreSQL production, those same tests would fail.

Practical impact: The evaluate_game endpoint is safe in the normal flow — the first successful call to apply_tier_boost writes current_tier, so the next evaluate_game invocation reads the updated old_tier and never detects a tier-up for the same tier again. The failure scenario is limited to direct/retry calls and to multi-tier loops where the same tier is re-entered.

Fix: Add an idempotency guard before the audit create in step 8a:

with _db.atomic():
    existing_audit = _audit_model.get_or_none(
        (_audit_model.card_state == card_state.id) & (_audit_model.tier == new_tier)
    )
    if existing_audit is None:
        _audit_model.create(**audit_data)
    card_state.current_tier = new_tier
    card_state.fully_evolved = new_tier >= 4
    card_state.variant = new_variant
    card_state.save()

This makes the entire function idempotent end-to-end, consistent with the existing card/ratings guards.

Minor observations (non-blocking)

  • REFRACTOR_BOOST_ENABLED default — defaults to enabled (!= "false"). Correct for production; the kill switch requires an explicit opt-out.
  • Partial failure reporting — the computed_tier = last_successful_tier reassignment before tier_ups.append correctly reports only the successfully committed tiers. On next game evaluation old_tier will equal the last successful tier, so the remaining tiers are retried automatically. Clean.
  • compute_batter_display_stats / compute_pitcher_display_stats — formulas match the Pydantic validators (denominator 108, double_pull for batters, double_cf for pitchers, bp_homerun/bp_single at 0.5 weight). Unit tests verify each coefficient.
  • source_variant for T2+ — correctly chains from compute_variant_hash(player_id, new_tier - 1) so each tier's boost builds on the previous tier's card, producing accurate cumulative deltas. Verified by TestCumulativeT1ToT4.test_t4_has_cumulative_deltas.
## Review — feat: Refractor Phase 2 integration **Verdict: COMMENT** (self-review restriction; no blocking defects but two issues below warrant a fix before merge) --- ### Overall assessment The architecture is well-conceived. Using `dry_run=True` in the evaluator to separate tier-detection from tier-write, then delegating the write to `apply_tier_boost()` which commits tier + variant + audit atomically in a single `db.atomic()` block, is the correct design. The kill switch, partial multi-tier failure handling (report last-successful-tier, retry remainder next game), and card propagation all follow from the design cleanly. Test coverage is exemplary — 52 new tests across four files with detailed docstrings and verified math. The `isoformat()` crash fix is also correct: `hasattr(..., "isoformat")` safely handles the case where SQLite returns a string instead of a datetime object. Two issues need attention before this ships. --- ### Issue 1 — `import os` inside function body (violates CLAUDE.md) **`app/routers_v2/refractor.py`, inside `evaluate_game()`:** ```python import os from ..db_engine import RefractorCardState, Player, StratPlay from ..services.refractor_boost import apply_tier_boost ``` `os` is a stdlib module with zero circular-import risk. The lazy-import pattern used here (and throughout the router) exists specifically to avoid circular dependencies with `db_engine`. Placing `import os` inside a function is inconsistent with that rationale and violates the CLAUDE.md rule: **"Never add lazy imports to middle of file."** Fix: move `import os` to the top-level imports in `refractor.py`. --- ### Issue 2 — Audit idempotency gap: tests pass under SQLite but second call raises IntegrityError in PostgreSQL `apply_tier_boost()` guards card creation (step 6) and ratings creation (step 7) with `get_or_none` checks — both are idempotent. The audit write (step 8a, inside `db.atomic()`) is **not** guarded: ```python with _db.atomic(): _audit_model.create(**audit_data) # No existence check card_state.current_tier = new_tier card_state.save() ``` The PostgreSQL `RefractorBoostAudit` table has `UNIQUE(card_state_id, tier)` (established in the Phase 2 foundation migration). Calling `apply_tier_boost` a second time for the same `(player_id, team_id, tier)` will: 1. Find the existing card and ratings — skip creation ✅ 2. Re-fetch `card_state` from DB (already at `new_tier`) ✅ 3. Enter `db.atomic()`, attempt `create(**audit_data)` — **IntegrityError** 💥 4. Atomic block rolls back (card_state save is never committed) 5. `IntegrityError` propagates to caller The `TestIdempotency` tests call `apply_tier_boost` twice without expecting an exception, and pass — because the SQLite in-memory database does not enforce the UNIQUE constraint (it isn't in the Peewee model's `Meta.indexes`, only in the SQL migration). In PostgreSQL production, those same tests would fail. **Practical impact:** The `evaluate_game` endpoint is safe in the normal flow — the first successful call to `apply_tier_boost` writes `current_tier`, so the next `evaluate_game` invocation reads the updated `old_tier` and never detects a tier-up for the same tier again. The failure scenario is limited to direct/retry calls and to multi-tier loops where the same tier is re-entered. **Fix:** Add an idempotency guard before the audit `create` in step 8a: ```python with _db.atomic(): existing_audit = _audit_model.get_or_none( (_audit_model.card_state == card_state.id) & (_audit_model.tier == new_tier) ) if existing_audit is None: _audit_model.create(**audit_data) card_state.current_tier = new_tier card_state.fully_evolved = new_tier >= 4 card_state.variant = new_variant card_state.save() ``` This makes the entire function idempotent end-to-end, consistent with the existing card/ratings guards. --- ### Minor observations (non-blocking) - **`REFRACTOR_BOOST_ENABLED` default** — defaults to enabled (`!= "false"`). Correct for production; the kill switch requires an explicit opt-out. - **Partial failure reporting** — the `computed_tier = last_successful_tier` reassignment before `tier_ups.append` correctly reports only the successfully committed tiers. On next game evaluation `old_tier` will equal the last successful tier, so the remaining tiers are retried automatically. Clean. - **`compute_batter_display_stats` / `compute_pitcher_display_stats`** — formulas match the Pydantic validators (denominator 108, double_pull for batters, double_cf for pitchers, bp_homerun/bp_single at 0.5 weight). Unit tests verify each coefficient. - **`source_variant` for T2+** — correctly chains from `compute_variant_hash(player_id, new_tier - 1)` so each tier's boost builds on the previous tier's card, producing accurate cumulative deltas. Verified by `TestCumulativeT1ToT4.test_t4_has_cumulative_deltas`.
app/routers_v2/refractor.py Outdated
@ -305,7 +307,10 @@ async def evaluate_game(game_id: int, token: str = Depends(oauth2_scheme)):
logging.warning("Bad Token: [REDACTED]")
raise HTTPException(status_code=401, detail="Unauthorized")
import os
cal commented 2026-03-30 18:13:47 +00:00
Author
Owner
Copy Link

import os here is unnecessary inside the function body — os is a stdlib module with no circular-import risk. Move to top-level imports. CLAUDE.md: "Never add lazy imports to middle of file."

`import os` here is unnecessary inside the function body — `os` is a stdlib module with no circular-import risk. Move to top-level imports. CLAUDE.md: "Never add lazy imports to middle of file."
app/services/refractor_boost.py
@ -316,0 +563,4 @@
boosted_splits[row.vs_hand] = boosted
# 5. Compute new variant hash.
cal commented 2026-03-30 18:13:47 +00:00
Author
Owner
Copy Link

No idempotency guard on the audit write. If apply_tier_boost is called twice for the same (card_state_id, tier), _audit_model.create() will raise an IntegrityError in PostgreSQL (UNIQUE constraint from migration). Cards and ratings have guards above but audit does not. Add existing_audit = _audit_model.get_or_none(...) check before create. Tests pass today only because SQLite in-memory doesn't enforce the migration-only UNIQUE constraint.

No idempotency guard on the audit write. If `apply_tier_boost` is called twice for the same `(card_state_id, tier)`, `_audit_model.create()` will raise an `IntegrityError` in PostgreSQL (UNIQUE constraint from migration). Cards and ratings have guards above but audit does not. Add `existing_audit = _audit_model.get_or_none(...)` check before `create`. Tests pass today only because SQLite in-memory doesn't enforce the migration-only UNIQUE constraint.
Claude added the
ai-reviewing
 label 2026-03-30 18:15:47 +00:00
cal added 1 commit 2026-03-30 18:16:34 +00:00 
- Move `import os` from inside evaluate_game() to module top-level imports
  (lazy imports are only for circular dependency avoidance)
- Add get_or_none idempotency guard before RefractorBoostAudit.create()
  inside db.atomic() to prevent IntegrityError on UNIQUE(card_state, tier)
  constraint in PostgreSQL when apply_tier_boost is called twice for the
  same tier
- Update atomicity test stub to provide card_state/tier attributes for
  the new Peewee expression in the idempotency guard

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude approved these changes 2026-03-30 18:17:24 +00:00
Dismissed
Claude left a comment
Collaborator
Copy Link

Approved via pd-pr

Approved via pd-pr
Claude approved these changes 2026-03-30 18:17:29 +00:00
Claude left a comment
Collaborator
Copy Link

Approved via pd-pr

Approved via pd-pr
cal merged commit 31c86525de into main 2026-03-30 18:17:30 +00:00
cal deleted branch feature/refractor-phase2-integration 2026-03-30 18:17:30 +00:00
cal added the
ai-merged
 label 2026-03-30 18:17:35 +00:00
Claude reviewed 2026-03-30 18:19:46 +00:00
Claude left a comment
Collaborator
Copy Link

AI Code Review

Files Reviewed

  • app/routers_v2/refractor.py (modified)
  • app/services/refractor_boost.py (modified)
  • app/services/refractor_evaluator.py (modified)
  • tests/test_refractor_boost_integration.py (added, 1317 lines)
  • tests/test_refractor_evaluator.py (modified, 804 lines)
  • tests/test_refractor_boost.py (modified)
  • tests/test_postgame_refractor.py (modified)

Prior Review Issues — Status

Both issues flagged in the previous review are now resolved:

  • ✅ import os placement — moved to top-level imports (line 1 of refractor.py), no longer inside evaluate_game() body. Complies with CLAUDE.md convention.
  • ✅ Audit idempotency gap — apply_tier_boost() now guards the audit write with get_or_none() before _audit_model.create() (lines 670–674), consistent with the card/ratings idempotency guards. A second call for the same (card_state_id, tier) will not raise IntegrityError on PostgreSQL.

Findings

Correctness

  • evaluate_card(dry_run=True) correctly skips writing current_tier/fully_evolved, leaving those writes to apply_tier_boost() which commits them atomically with the variant and audit record.
  • Multi-tier jump loop (for tier in range(old_tier + 1, computed_tier + 1)) correctly handles skipped tiers. Partial failure reports the last successfully committed tier, preventing false-positive tier-up notifications.
  • Kill switch (REFRACTOR_BOOST_ENABLED=false) suppresses both the boost and the tier-up notification, ensuring no false positive is emitted when current_tier was never written.
  • isoformat() crash fix (hasattr(..., "isoformat")) correctly handles both SQLite string and datetime objects.
  • 108-sum invariant assertion fires before the atomic block, so an invariant violation raises ValueError without writing anything to the DB.

Edge Cases

  • state.track FK access in evaluate_game() (line 367) will raise DoesNotExist if the track is missing, not return None. This is caught by the outer except Exception loop handler, logged, and skipped — functionally safe but worth noting.
  • CardModel.get_or_none() in step 2 of apply_tier_boost() filters by player_id and variant only, not team_id. For base cards (variant=0) and computed variants (hash is player+tier), a player may have the same variant across teams; the ratings are identical so getting any matching row is equivalent. Non-blocking.
  • Step 8c (_card_model.update(variant=new_variant).where(player == player_id, team == team_id)) propagates variant to all Card rows for the player+team, not just the card type being boosted. This appears intentional (the docstring describes it as "all Card rows") but means both batting and pitching Card rows receive the same variant key on a batter tier-up. If team+player can have both card types, confirm this is the intended propagation scope.

Security

  • No issues found. No raw SQL, no user input used in queries beyond FK integer IDs, no hardcoded credentials.

Style & Conventions

  • No issues. Lazy model imports via injectable parameters follow the established pattern in refractor_evaluator.py. Module-level db = None + _get_db() lazy-load pattern is documented and consistent with test patching strategy.
  • Test files use SQLite in-memory pattern consistent with prior refractor test files.

Suggestions

  • compute_batter_display_stats() and compute_pitcher_display_stats() could note that the denominator is 108 even though PITCHER_OUTCOME_COLUMNS sums to 79 (the x-check columns bring it to 108). The existing docstring says "All denominators are 108" which is correct but a cross-reference to the column structure would help future readers. Very minor.

Verdict: COMMENT (self-review restriction — no blocking issues)

Both prior blocking issues are resolved. Logic is correct, atomicity guarantees hold, idempotency is enforced at every write point, and test coverage is thorough (52 new tests across 4 files). This is ready to merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/refractor.py` (modified) - `app/services/refractor_boost.py` (modified) - `app/services/refractor_evaluator.py` (modified) - `tests/test_refractor_boost_integration.py` (added, 1317 lines) - `tests/test_refractor_evaluator.py` (modified, 804 lines) - `tests/test_refractor_boost.py` (modified) - `tests/test_postgame_refractor.py` (modified) ### Prior Review Issues — Status Both issues flagged in the previous review are now resolved: - ✅ **`import os` placement** — moved to top-level imports (line 1 of `refractor.py`), no longer inside `evaluate_game()` body. Complies with CLAUDE.md convention. - ✅ **Audit idempotency gap** — `apply_tier_boost()` now guards the audit write with `get_or_none()` before `_audit_model.create()` (lines 670–674), consistent with the card/ratings idempotency guards. A second call for the same (card_state_id, tier) will not raise IntegrityError on PostgreSQL. ### Findings #### Correctness - `evaluate_card(dry_run=True)` correctly skips writing `current_tier`/`fully_evolved`, leaving those writes to `apply_tier_boost()` which commits them atomically with the variant and audit record. - Multi-tier jump loop (`for tier in range(old_tier + 1, computed_tier + 1)`) correctly handles skipped tiers. Partial failure reports the last successfully committed tier, preventing false-positive tier-up notifications. - Kill switch (`REFRACTOR_BOOST_ENABLED=false`) suppresses both the boost and the tier-up notification, ensuring no false positive is emitted when `current_tier` was never written. - `isoformat()` crash fix (`hasattr(..., "isoformat")`) correctly handles both SQLite string and datetime objects. - 108-sum invariant assertion fires before the atomic block, so an invariant violation raises `ValueError` without writing anything to the DB. #### Edge Cases - `state.track` FK access in `evaluate_game()` (line 367) will raise `DoesNotExist` if the track is missing, not return `None`. This is caught by the outer `except Exception` loop handler, logged, and skipped — functionally safe but worth noting. - `CardModel.get_or_none()` in step 2 of `apply_tier_boost()` filters by `player_id` and `variant` only, not `team_id`. For base cards (variant=0) and computed variants (hash is player+tier), a player may have the same variant across teams; the ratings are identical so getting any matching row is equivalent. Non-blocking. - Step 8c (`_card_model.update(variant=new_variant).where(player == player_id, team == team_id)`) propagates variant to all Card rows for the player+team, not just the card type being boosted. This appears intentional (the docstring describes it as "all Card rows") but means both batting and pitching Card rows receive the same variant key on a batter tier-up. If team+player can have both card types, confirm this is the intended propagation scope. #### Security - No issues found. No raw SQL, no user input used in queries beyond FK integer IDs, no hardcoded credentials. #### Style & Conventions - No issues. Lazy model imports via injectable parameters follow the established pattern in `refractor_evaluator.py`. Module-level `db = None` + `_get_db()` lazy-load pattern is documented and consistent with test patching strategy. - Test files use SQLite in-memory pattern consistent with prior refractor test files. #### Suggestions - `compute_batter_display_stats()` and `compute_pitcher_display_stats()` could note that the denominator is 108 even though PITCHER_OUTCOME_COLUMNS sums to 79 (the x-check columns bring it to 108). The existing docstring says "All denominators are 108" which is correct but a cross-reference to the column structure would help future readers. Very minor. ### Verdict: COMMENT (self-review restriction — no blocking issues) Both prior blocking issues are resolved. Logic is correct, atomicity guarantees hold, idempotency is enforced at every write point, and test coverage is thorough (52 new tests across 4 files). This is ready to merge. --- *Automated review by Claude PR Reviewer*
Claude added the
ai-reviewed
 label 2026-03-30 18:20:09 +00:00
cal reviewed 2026-03-30 18:20:24 +00:00
cal left a comment
Author
Owner
Copy Link

Re-review — commit 7f17c9b — APPROVED

Both issues flagged in the previous review are resolved.

Issue 1 — import os at top of file ✅

import os now appears at line 1 of app/routers_v2/refractor.py, before all other imports. The lazy import inside evaluate_game() has been removed. Correct placement.

Issue 2 — Audit idempotency guard ✅

refractor_boost.py lines 670–674 now guard the audit write inside _db.atomic():

existing_audit = _audit_model.get_or_none(
    (_audit_model.card_state == card_state.id) & (_audit_model.tier == new_tier)
)
if existing_audit is None:
    _audit_model.create(**audit_data)

The get_or_none check is correctly placed inside the with _db.atomic(): block (step 8a), before _audit_model.create(). A second call for the same (card_state_id, tier) pair will find the existing row and skip creation — preventing the IntegrityError that would have been raised by the UNIQUE constraint on PostgreSQL.

No other concerns. APPROVED — Gitea blocks self-review so posting as COMMENT.

## Re-review — commit 7f17c9b — **APPROVED** Both issues flagged in the previous review are resolved. **Issue 1 — `import os` at top of file** ✅ `import os` now appears at line 1 of `app/routers_v2/refractor.py`, before all other imports. The lazy import inside `evaluate_game()` has been removed. Correct placement. **Issue 2 — Audit idempotency guard** ✅ `refractor_boost.py` lines 670–674 now guard the audit write inside `_db.atomic()`: ```python existing_audit = _audit_model.get_or_none( (_audit_model.card_state == card_state.id) & (_audit_model.tier == new_tier) ) if existing_audit is None: _audit_model.create(**audit_data) ``` The `get_or_none` check is correctly placed inside the `with _db.atomic():` block (step 8a), before `_audit_model.create()`. A second call for the same `(card_state_id, tier)` pair will find the existing row and skip creation — preventing the `IntegrityError` that would have been raised by the UNIQUE constraint on PostgreSQL. No other concerns. **APPROVED** — Gitea blocks self-review so posting as COMMENT.
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
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#177
No description provided.
Delete Branch "feature/refractor-phase2-integration"

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?