golfgame/docs/v3/refactor-misc.md

Plan 3: Miscellaneous Refactoring & Improvements

Overview

Everything that doesn't fall under the main.py/game.py or ai.py refactors: shared utilities, dead code, test improvements, and structural cleanup.

---

M1. Duplicate get_card_value Functions

There are currently three functions that compute card values:

1. game.py:get_card_value(card: Card, options) - Takes Card objects
2. constants.py:get_card_value_for_rank(rank_str, options_dict) - Takes rank strings
3. ai.py:get_ai_card_value(card, options) - AI-specific wrapper (also handles face-down estimation)

Problem: game.py and constants.py do the same thing with different interfaces, and neither handles all house rules identically. The AI version adds face-down logic but duplicates the base value lookup.

Fix:

• Keep game.py:get_card_value() as the canonical Card-based function (it already is the most complete)
• Keep constants.py:get_card_value_for_rank() for string-based lookups from logs/JSON
• Have ai.py:get_ai_card_value() delegate to game.py:get_card_value() for the base value, only adding its face-down estimation on top
• Add a brief comment in each noting which is canonical and why each variant exists

This is a minor cleanup - the current code works, it's just slightly confusing to have three entry points.

M2. GameOptions Boilerplate Reduction

GameOptions currently has 17+ boolean fields. Every time a new house rule is added, you have to update:

1. GameOptions dataclass definition
2. _options_to_dict() in game.py
3. get_active_rules() logic in get_state()
4. from_client_data() (proposed in Plan 1)
5. start_game handler in main.py (currently, will move to handlers.py)

Fix: Use dataclasses.fields() introspection to auto-generate the dict and client data parsing:

from dataclasses import fields, asdict

# _options_to_dict becomes:
def _options_to_dict(self) -> dict:
    return asdict(self.options)

# from_client_data becomes:
@classmethod
def from_client_data(cls, data: dict) -> "GameOptions":
    field_defaults = {f.name: f.default for f in fields(cls)}
    kwargs = {}
    for f in fields(cls):
        if f.name in data:
            kwargs[f.name] = data[f.name]
    # Special validation
    kwargs["initial_flips"] = max(0, min(2, kwargs.get("initial_flips", 2)))
    return cls(**kwargs)

This means adding a new house rule only requires adding the field to GameOptions and its entry in the active_rules display table (from Plan 1's B1).

M3. Consolidate Game Logger Pattern in AI

ai.py:process_cpu_turn() has the same logger boilerplate as main.py's human handlers. After Plan 1's A2 creates log_human_action(), create a parallel:

def log_cpu_action(game_id, player, action, card=None, position=None, game=None, reason=""):
    game_logger = get_logger()
    if game_logger and game_id:
        game_logger.log_move(
            game_id=game_id,
            player=player,
            is_cpu=True,
            action=action,
            card=card,
            position=position,
            game=game,
            decision_reason=reason,
        )

This appears ~4 times in process_cpu_turn().

M4. Player.get_player() Linear Search

Game.get_player() does a linear scan of the players list:

def get_player(self, player_id: str) -> Optional[Player]:
    for player in self.players:
        if player.id == player_id:
            return player
    return None

With max 6 players this is fine performance-wise, but it's called frequently. Could add a _player_lookup: dict[str, Player] cache maintained by add_player/remove_player. Very minor optimization - only worth doing if we're already touching these methods.

M5. Room Code Collision Potential

RoomManager._generate_code() generates random 4-letter codes and retries on collision. With 26^4 = 456,976 possibilities this is fine now, but if we ever scale, the while-True loop could theoretically spin. Low priority, but a simple improvement:

def _generate_code(self, max_attempts=100) -> str:
    for _ in range(max_attempts):
        code = "".join(random.choices(string.ascii_uppercase, k=4))
        if code not in self.rooms:
            return code
    raise RuntimeError("Could not generate unique room code")

M6. Test Coverage Gaps

Current test files:

• test_game.py - Core game logic (good coverage)
• test_house_rules.py - House rule scoring
• test_v3_features.py - New v3 features
• test_maya_bug.py - Specific regression test
• tests/test_event_replay.py, test_persistence.py, test_replay.py - Event system

Missing:

• No tests for room.py (Room, RoomManager, RoomPlayer)
• No tests for WebSocket message handlers (will be much easier to test after Plan 1's handler extraction)
• No unit tests for individual AI decision functions (will be much easier after Plan 2's decomposition)

Recommendation: After Plans 1 and 2 are complete, add:

• test_handlers.py - Test each message handler with mock WebSocket/Room
• test_ai_decisions.py - Test individual AI sub-functions (go-out logic, denial, etc.)
• test_room.py - Test Room/RoomManager CRUD operations

M7. Unused/Dead Code Audit

Things to verify and potentially remove:

• score_analysis.py - Is this used anywhere or was it a one-off analysis tool?
• game_analyzer.py - Same question
• auth.py (top-level, not in routers/) - Appears to be an old file superseded by services/auth_service.py?
• models/game_state.py - Check if used or leftover from earlier design

M8. Type Hints Consistency

Some functions have full type hints, others don't. The AI functions especially are loosely typed. After the ai.py refactor (Plan 2), ensure all new sub-functions have proper type hints:

def _check_go_out_swap(
    player: Player,
    drawn_card: Card,
    profile: CPUProfile,
    game: Game,
    game_state: dict,
) -> Optional[int]:

This helps with IDE navigation and catching bugs during future changes.

---

Execution Order

1. M3 (AI logger helper) - Do alongside Plan 1's A2
2. M2 (GameOptions introspection) - Do alongside Plan 1's B2/B3
3. M1 (card value consolidation) - Quick cleanup
4. M7 (dead code audit) - Quick investigation
5. M5 (room code safety) - 2 lines
6. M6 (tests) - After Plans 1 and 2 are complete
7. M4 (player lookup) - Only if touching add/remove_player for other reasons
8. M8 (type hints) - Ongoing, do as part of Plan 2

Files Touched

• server/ai.py - logger helper, card value delegation
• server/game.py - GameOptions introspection
• server/constants.py - comments clarifying role
• server/room.py - room code safety (minor)
• server/test_room.py - new file (eventually)
• server/test_handlers.py - new file (eventually)
• server/test_ai_decisions.py - new file (eventually)
• Various files checked in dead code audit