golfgame/docs/v3/refactor-ai.md

Plan 2: ai.py Refactor

Overview

ai.py is 1,978 lines with a single function (choose_swap_or_discard) at 666 lines and cyclomatic complexity 50+. The goal is to decompose it into testable, understandable pieces without changing any AI behavior.

Key constraint: AI behavior must remain identical. This is pure structural refactoring. We can validate with python server/simulate.py 500 before and after - stats should match within normal variance.

---

The Problem Functions

Function	Lines	What It Does
choose_swap_or_discard()	~666	Decides which position (0-5) to swap drawn card into, or None to discard
calculate_swap_score()	~240	Scores a single position for swapping
should_take_discard()	~160	Decides whether to take from discard pile
process_cpu_turn()	~240	Orchestrates a full CPU turn with timing

---

Refactoring Plan

Step 1: Extract Named Constants

Create section at top of ai.py (or a separate ai_constants.py if preferred):

# =============================================================================
# AI Decision Constants
# =============================================================================

# Expected value of an unknown (face-down) card, based on deck distribution
EXPECTED_HIDDEN_VALUE = 4.5

# Pessimistic estimate for hidden cards (used in go-out safety checks)
PESSIMISTIC_HIDDEN_VALUE = 6.0

# Conservative estimate (used by conservative personality)
CONSERVATIVE_HIDDEN_VALUE = 2.5

# Cards at or above this value should never be swapped into unknown positions
HIGH_CARD_THRESHOLD = 8

# Maximum card value for unpredictability swaps
UNPREDICTABLE_MAX_VALUE = 7

# Pair potential discount when adjacent card matches
PAIR_POTENTIAL_DISCOUNT = 0.25

# Blackjack target score
BLACKJACK_TARGET = 21

# Base acceptable score range for go-out decisions
GO_OUT_SCORE_BASE = 12
GO_OUT_SCORE_MAX = 20

Locations to update: ~30 magic number sites across the file. Each becomes a named reference.

Step 2: Extract Column/Pair Utility Functions

The "iterate columns, check pairs" pattern appears 8+ times. Create shared utilities:

def iter_columns(player: Player):
    """Yield (col_index, top_idx, bot_idx, top_card, bot_card) for each column."""
    for col in range(3):
        top_idx = col
        bot_idx = col + 3
        yield col, top_idx, bot_idx, player.cards[top_idx], player.cards[bot_idx]


def project_score(player: Player, swap_pos: int, new_card: Card, options: GameOptions) -> int:
    """Calculate what the player's score would be if new_card were swapped into swap_pos.

    Handles pair cancellation correctly. Used by multiple decision paths.
    """
    total = 0
    for col, top_idx, bot_idx, top_card, bot_card in iter_columns(player):
        # Substitute the new card if it's in this column
        effective_top = new_card if top_idx == swap_pos else top_card
        effective_bot = new_card if bot_idx == swap_pos else bot_card

        if effective_top.rank == effective_bot.rank:
            # Pair cancels (with house rule exceptions)
            continue
        total += get_ai_card_value(effective_top, options)
        total += get_ai_card_value(effective_bot, options)
    return total


def count_hidden(player: Player) -> int:
    """Count face-down cards."""
    return sum(1 for c in player.cards if not c.face_up)


def hidden_positions(player: Player) -> list[int]:
    """Get indices of face-down cards."""
    return [i for i, c in enumerate(player.cards) if not c.face_up]


def known_score(player: Player, options: GameOptions) -> int:
    """Calculate score from face-up cards only, using EXPECTED_HIDDEN_VALUE for unknowns."""
    # Centralized version of the repeated estimation logic
    ...

This replaces duplicated loops at roughly lines: 679, 949, 1002, 1053, 1145, 1213, 1232.

Step 3: Decompose choose_swap_or_discard()

Break into focused sub-functions. The current flow is roughly:

1. Go-out safety check (lines ~1087-1186) - "I'm about to go out, pick the best swap to minimize my score"
2. Score all 6 positions (lines ~1190-1270) - Calculate swap benefit for each position
3. Filter and rank candidates (lines ~1270-1330) - Safety filters, personality tie-breaking
4. Blackjack special case (lines ~1330-1380) - If blackjack rule enabled, check for 21
5. Endgame safety (lines ~1380-1410) - Don't swap 8+ into unknowns in endgame
6. Denial logic (lines ~1410-1480) - Block opponent by taking their useful cards

Proposed decomposition:

def choose_swap_or_discard(player, drawn_card, profile, game, ...) -> Optional[int]:
    """Main orchestrator - delegates to focused sub-functions."""

    # Check if we should force a go-out swap
    go_out_pos = _check_go_out_swap(player, drawn_card, profile, game, ...)
    if go_out_pos is not None:
        return go_out_pos

    # Score all positions
    candidates = _score_all_positions(player, drawn_card, profile, game, ...)

    # Apply filters and select best
    best = _select_best_candidate(candidates, player, drawn_card, profile, game, ...)

    if best is not None:
        return best

    # Try denial as fallback
    return _check_denial_swap(player, drawn_card, profile, game, ...)


def _check_go_out_swap(player, drawn_card, profile, game, ...) -> Optional[int]:
    """If player is close to going out, find the best position to minimize final score.

    Handles:
    - All-but-one face-up: find the best slot for the drawn card
    - Acceptable score threshold based on game state and personality
    - Pair completion opportunities
    """
    # Lines ~1087-1186 of current choose_swap_or_discard
    ...


def _score_all_positions(player, drawn_card, profile, game, ...) -> list[tuple[int, float]]:
    """Calculate swap benefit score for each of the 6 positions.

    Returns list of (position, score) tuples, sorted by score descending.
    Each score represents how much the swap improves the player's hand.
    """
    # Lines ~1190-1270 - calls calculate_swap_score() for each position
    ...


def _select_best_candidate(candidates, player, drawn_card, profile, game, ...) -> Optional[int]:
    """From scored candidates, apply personality modifiers and safety filters.

    Handles:
    - Minimum improvement threshold
    - Personality tie-breaking (pair_hunter prefers pair columns, etc.)
    - Unpredictability (occasional random choice with value threshold)
    - High-card safety filter (never swap 8+ into hidden positions)
    - Blackjack special case (swap to reach exactly 21)
    - Endgame safety (discard 8+ rather than force into unknown)
    """
    # Lines ~1270-1410
    ...


def _check_denial_swap(player, drawn_card, profile, game, ...) -> Optional[int]:
    """Check if we should swap to deny opponents a useful card.

    Only triggers for profiles with denial_aggression > 0.
    Skips hidden positions for high cards (8+).
    """
    # Lines ~1410-1480
    ...

Step 4: Simplify calculate_swap_score()

Currently ~240 lines. Some of its complexity comes from inlined pair calculations and standings pressure. Extract:

def _pair_improvement(player, position, new_card, options) -> float:
    """Calculate pair-related benefit of swapping into this position."""
    # Would the swap create a new pair? Break an existing pair?
    ...

def _standings_pressure(player, game) -> float:
    """Calculate how much standings position should affect decisions."""
    # Shared between calculate_swap_score and should_take_discard
    ...

Step 5: Simplify should_take_discard()

Currently ~160 lines. Much of the complexity is from re-deriving information that calculate_swap_score also computes. After Step 2's utilities exist, this should shrink significantly since project_score() and known_score() handle the repeated estimation logic.

Step 6: Clean up process_cpu_turn()

Currently ~240 lines. This function is the CPU turn orchestrator and is mostly fine structurally, but has some inline logic for:

• Flip-as-action decisions (~30 lines)
• Knock-early decisions (~30 lines)
• Game logging (~20 lines repeated twice)

Extract:

def _should_flip_as_action(player, game, profile) -> Optional[int]:
    """Decide whether to use flip-as-action and which position."""
    ...

def _should_knock_early(player, game, profile) -> bool:
    """Decide whether to knock early."""
    ...

def _log_cpu_action(game_id, player, action, card=None, position=None, reason=""):
    """Log a CPU action if logger is available."""
    ...

---

Execution Order

1. Step 1 (constants) - Safe, mechanical, reduces cognitive load immediately
2. Step 2 (utilities) - Foundation for everything else
3. Step 3 (decompose choose_swap_or_discard) - The big win
4. Step 4 (simplify calculate_swap_score) - Benefits from Step 2 utilities
5. Step 5 (simplify should_take_discard) - Benefits from Step 2 utilities
6. Step 6 (clean up process_cpu_turn) - Lower priority

Run python server/simulate.py 500 before Step 1 and after each step to verify identical behavior.

---

Validation Strategy

# Before any changes - capture baseline
python server/simulate.py 500 > /tmp/ai_baseline.txt

# After each step
python server/simulate.py 500 > /tmp/ai_after_stepN.txt

# Compare key metrics:
# - Average scores per personality
# - "Swapped 8+ into unknown" rate (should stay < 0.1%)
# - Win rate distribution

---

Files Touched

• server/ai.py - major restructuring (same file, new internal organization)
• No new files needed (all changes within ai.py unless we decide to split constants out)

Risk Assessment

• Low risk if done mechanically (cut-paste into functions, update call sites)
• Medium risk if we accidentally change conditional logic order or miss an early return
• Simulation tests are the safety net - run after every step