From e8b1091d8ac969a4c24fbdb0897624a44fae6d47 Mon Sep 17 00:00:00 2001 From: Cal Corum Date: Wed, 8 Apr 2026 00:37:26 -0500 Subject: [PATCH] refactor: extract evaluate-game business logic from router to service layer (#202) Closes #202 Adds app/services/refractor_service.py with two service functions: - ensure_variant_cards: idempotent function that creates missing variant cards for all tiers up to a target, with partial failure handling and REFRACTOR_BOOST_ENABLED kill switch support. - evaluate_and_boost: combines evaluate_card(dry_run=True) with ensure_variant_cards so both tier computation and variant-card creation happen in one testable call without HTTP round-trips. Updates both router endpoints to use evaluate_and_boost: - POST /cards/{card_id}/evaluate now calls evaluate_and_boost instead of evaluate_card directly, which also fixes the broken variant creation on the manual evaluate path (variant cards are now created when a tier-up is detected, not just recorded). - POST /evaluate-game/{game_id} replaces ~55 lines of inline boost orchestration (boost_enabled flag, tier loop, partial failure tracking, card_type/track null checks) with a single evaluate_and_boost call. Response shape is unchanged. Co-Authored-By: Claude Sonnet 4.6 --- app/routers_v2/refractor.py | 74 ++--------- app/services/refractor_service.py | 200 ++++++++++++++++++++++++++++++ 2 files changed, 210 insertions(+), 64 deletions(-) create mode 100644 app/services/refractor_service.py diff --git a/app/routers_v2/refractor.py b/app/routers_v2/refractor.py index d28aba2..fb127c3 100644 --- a/app/routers_v2/refractor.py +++ b/app/routers_v2/refractor.py @@ -1,5 +1,3 @@ -import os - from fastapi import APIRouter, Depends, HTTPException, Query import logging from typing import Optional @@ -338,7 +336,7 @@ async def evaluate_card(card_id: int, token: str = Depends(oauth2_scheme)): raise HTTPException(status_code=401, detail="Unauthorized") from ..db_engine import Card - from ..services.refractor_evaluator import evaluate_card as _evaluate + from ..services.refractor_service import evaluate_and_boost try: card = Card.get_by_id(card_id) @@ -346,7 +344,7 @@ async def evaluate_card(card_id: int, token: str = Depends(oauth2_scheme)): raise HTTPException(status_code=404, detail=f"Card {card_id} not found") try: - result = _evaluate(card.player_id, card.team_id) + result = evaluate_and_boost(card.player_id, card.team_id) except ValueError as exc: raise HTTPException(status_code=404, detail=str(exc)) @@ -368,8 +366,7 @@ async def evaluate_game(game_id: int, token: str = Depends(oauth2_scheme)): raise HTTPException(status_code=401, detail="Unauthorized") from ..db_engine import RefractorCardState, Player, StratPlay - from ..services.refractor_boost import apply_tier_boost - from ..services.refractor_evaluator import evaluate_card + from ..services.refractor_service import evaluate_and_boost plays = list(StratPlay.select().where(StratPlay.game == game_id)) @@ -383,8 +380,6 @@ async def evaluate_game(game_id: int, token: str = Depends(oauth2_scheme)): evaluated = 0 tier_ups = [] - boost_enabled = os.environ.get("REFRACTOR_BOOST_ENABLED", "true").lower() != "false" - for player_id, team_id in pairs: try: state = RefractorCardState.get_or_none( @@ -405,16 +400,11 @@ async def evaluate_game(game_id: int, token: str = Depends(oauth2_scheme)): continue old_tier = state.current_tier - # Use dry_run=True so that current_tier is NOT written here. - # apply_tier_boost() writes current_tier + variant atomically on - # tier-up. If no tier-up occurs, apply_tier_boost is not called - # and the tier stays at old_tier (correct behaviour). - result = evaluate_card(player_id, team_id, dry_run=True) + result = evaluate_and_boost(player_id, team_id) evaluated += 1 - # Use computed_tier (what the formula says) to detect tier-ups. - computed_tier = result.get("computed_tier", old_tier) - if computed_tier > old_tier: + new_tier = result.get("current_tier", old_tier) + if new_tier > old_tier: player_name = "Unknown" try: p = Player.get_by_id(player_id) @@ -422,63 +412,19 @@ async def evaluate_game(game_id: int, token: str = Depends(oauth2_scheme)): except Exception: pass - # Phase 2: Apply rating boosts for each tier gained. - # apply_tier_boost() writes current_tier + variant atomically. - # If it fails, current_tier stays at old_tier — automatic retry next game. - boost_result = None - if not boost_enabled: - # Boost disabled via REFRACTOR_BOOST_ENABLED=false. - # Skip notification — current_tier was not written (dry_run), - # so reporting a tier-up would be a false notification. - continue - - card_type = state.track.card_type if state.track else None - if card_type: - last_successful_tier = old_tier - failing_tier = old_tier + 1 - try: - for tier in range(old_tier + 1, computed_tier + 1): - failing_tier = tier - boost_result = apply_tier_boost( - player_id, team_id, tier, card_type - ) - last_successful_tier = tier - except Exception as boost_exc: - logger.warning( - f"Refractor boost failed for player={player_id} " - f"team={team_id} tier={failing_tier}: {boost_exc}" - ) - # Report only the tiers that actually succeeded. - # If none succeeded, skip the tier_up notification entirely. - if last_successful_tier == old_tier: - continue - # At least one intermediate tier was committed; report that. - computed_tier = last_successful_tier - else: - # No card_type means no track — skip boost and skip notification. - # A false tier-up notification must not be sent when the boost - # was never applied (current_tier was never written to DB). - logger.warning( - f"Refractor boost skipped for player={player_id} " - f"team={team_id}: no card_type on track" - ) - continue - tier_up_entry = { "player_id": player_id, "team_id": team_id, "player_name": player_name, "old_tier": old_tier, - "new_tier": computed_tier, + "new_tier": new_tier, "current_value": result.get("current_value", 0), "track_name": state.track.name if state.track else "Unknown", } - # Non-breaking addition: include boost info when available. - if boost_result: - tier_up_entry["variant_created"] = boost_result.get( - "variant_created" - ) + variants_created = result.get("variants_created") or [] + if variants_created: + tier_up_entry["variant_created"] = variants_created[-1] tier_ups.append(tier_up_entry) diff --git a/app/services/refractor_service.py b/app/services/refractor_service.py new file mode 100644 index 0000000..85fdf2d --- /dev/null +++ b/app/services/refractor_service.py @@ -0,0 +1,200 @@ +"""Refractor service layer — shared orchestration across router endpoints. + +Provides ``ensure_variant_cards`` and ``evaluate_and_boost`` so that both +the evaluate-game endpoint and the manual card-evaluate endpoint share the +same boost orchestration logic without requiring HTTP round-trips in tests. +""" + +import logging +import os + +logger = logging.getLogger(__name__) + + +def ensure_variant_cards( + player_id: int, + team_id: int, + target_tier: int | None = None, + card_type: str | None = None, + *, + _state_model=None, +) -> dict: + """Ensure variant cards exist for all tiers up to target_tier. + + Idempotent — safe to call multiple times. If a variant card already + exists for a given tier it is skipped. Partial failures are tolerated: + lower tiers that were committed are reported even if a higher tier fails. + + Args: + player_id: Player primary key. + team_id: Team primary key. + target_tier: Highest tier to ensure variants for. If None, uses + ``state.current_tier`` (backfill mode — creates missing variants + for tiers already recorded in the DB). + card_type: One of 'batter', 'sp', 'rp'. If None, derived from + ``state.track.card_type``. + _state_model: Dependency-injection override for RefractorCardState + (used in tests). + + Returns: + Dict with keys: + - ``current_tier`` (int): highest tier confirmed in the DB after + this call (equal to state.current_tier if no new tiers created). + - ``variants_created`` (list[int]): variant hashes newly created. + - ``boost_results`` (list[dict]): raw return values from + apply_tier_boost for each newly created variant. + """ + if _state_model is None: + from app.db_engine import RefractorCardState as _state_model # noqa: PLC0415 + + state = _state_model.get_or_none( + (_state_model.player_id == player_id) & (_state_model.team_id == team_id) + ) + if state is None: + return {"current_tier": 0, "variants_created": [], "boost_results": []} + + if target_tier is None: + target_tier = state.current_tier + + if target_tier == 0: + return { + "current_tier": state.current_tier, + "variants_created": [], + "boost_results": [], + } + + # Resolve card_type from track if not provided. + resolved_card_type = card_type + if resolved_card_type is None: + resolved_card_type = state.track.card_type if state.track else None + if resolved_card_type is None: + logger.warning( + "ensure_variant_cards: no card_type for player=%s team=%s — skipping", + player_id, + team_id, + ) + return { + "current_tier": state.current_tier, + "variants_created": [], + "boost_results": [], + } + + # Respect kill switch. + boost_enabled = os.environ.get("REFRACTOR_BOOST_ENABLED", "true").lower() != "false" + if not boost_enabled: + return { + "current_tier": state.current_tier, + "variants_created": [], + "boost_results": [], + } + + from app.services.refractor_boost import ( # noqa: PLC0415 + apply_tier_boost, + compute_variant_hash, + ) + + is_batter = resolved_card_type == "batter" + if is_batter: + from app.db_engine import BattingCard as CardModel # noqa: PLC0415 + else: + from app.db_engine import PitchingCard as CardModel # noqa: PLC0415 + + variants_created = [] + boost_results = [] + last_successful_tier = state.current_tier + + for tier in range(1, target_tier + 1): + variant_hash = compute_variant_hash(player_id, tier) + existing = CardModel.get_or_none( + (CardModel.player == player_id) & (CardModel.variant == variant_hash) + ) + if existing is not None: + # Already exists — idempotent skip. + continue + try: + boost_result = apply_tier_boost( + player_id, team_id, tier, resolved_card_type + ) + variants_created.append(variant_hash) + boost_results.append(boost_result) + last_successful_tier = tier + except Exception as exc: + logger.warning( + "ensure_variant_cards: boost failed for player=%s team=%s tier=%s: %s", + player_id, + team_id, + tier, + exc, + ) + # Don't attempt higher tiers if a lower one failed; the missing + # lower-tier variant would cause apply_tier_boost to fail for T+1 + # anyway (it reads from the previous tier's variant as its source). + break + + return { + "current_tier": last_successful_tier, + "variants_created": variants_created, + "boost_results": boost_results, + } + + +def evaluate_and_boost( + player_id: int, + team_id: int, + *, + _state_model=None, + _stats_model=None, +) -> dict: + """Full evaluation: recompute tier from career stats, then ensure variant cards exist. + + Combines evaluate_card (dry_run=True) with ensure_variant_cards so that + both tier computation and variant-card creation happen in a single call. + Handles both tier-up cases (computed_tier > stored) and backfill cases + (tier already in DB but variant card was never created). + + Args: + player_id: Player primary key. + team_id: Team primary key. + _state_model: DI override for RefractorCardState (used in tests). + _stats_model: DI override for stats model passed to evaluate_card. + + Returns: + Dict containing all fields from evaluate_card plus: + - ``current_tier`` (int): highest tier confirmed in the DB after + this call (may be higher than eval_result["current_tier"] if a + tier-up was committed here). + - ``variants_created`` (list[int]): variant hashes newly created. + - ``boost_results`` (list[dict]): raw boost result dicts. + + Raises: + ValueError: If no RefractorCardState exists for (player_id, team_id). + """ + from app.services.refractor_evaluator import evaluate_card # noqa: PLC0415 + + eval_kwargs: dict = {"dry_run": True} + if _state_model is not None: + eval_kwargs["_state_model"] = _state_model + if _stats_model is not None: + eval_kwargs["_stats_model"] = _stats_model + + eval_result = evaluate_card(player_id, team_id, **eval_kwargs) + + computed_tier = eval_result.get("computed_tier", 0) + stored_tier = eval_result.get("current_tier", 0) + # target_tier is the higher of formula result and stored value: + # - tier-up case: computed > stored, creates new variant(s) + # - backfill case: stored > computed (stale), creates missing variants + target_tier = max(computed_tier, stored_tier) + + ensure_kwargs: dict = {"target_tier": target_tier} + if _state_model is not None: + ensure_kwargs["_state_model"] = _state_model + + ensure_result = ensure_variant_cards(player_id, team_id, **ensure_kwargs) + + return { + **eval_result, + "current_tier": ensure_result["current_tier"], + "variants_created": ensure_result["variants_created"], + "boost_results": ensure_result["boost_results"], + }