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 <noreply@anthropic.com>
This commit is contained in:
parent
c8ec976626
commit
e8b1091d8a
@ -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)
|
||||
|
||||
|
||||
200
app/services/refractor_service.py
Normal file
200
app/services/refractor_service.py
Normal file
@ -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"],
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user