Skip to content

refactor(core): introduce TeamId enum for team identity (replace string-keyed scores) #2

Description

@valmathieu

Context

Round/game scores and trick piles are keyed by team name strings (Round.team_tricks: Dict[str, List[Trick]], RoundScore.scores: Dict[str, int], Game.scores). This works, but keys are stringly-typed: a typo is a runtime KeyError, not a type error, and consumers needing the actual Team must map name → object themselves.

Meanwhile contrai_core.Team has no value semantics (default identity __eq__/__hash__), so it is a fragile dict key: a Team rebuilt in a test or reconstructed from persisted data would silently not match the original.

Findings

  • Team.total_score / add_points is effectively dead weight: only core's own tests exercise it. The engine tracks cumulative scores in Game.scores and never calls add_points.
  • There is a circular reference BasePlayer.teamTeam.players, wired after construction.
  • play.py compares current_master.team == player.team by identity today.

Options considered

Option A — add __eq__/__hash__ (on name) to the current mutable Team

Cheap, and makes Dict[Team, int] safe. But:

  • A mutable object with value-based hash is a standing footgun — only safe as long as name never changes, and nothing enforces that.
  • Silently changes semantics elsewhere: play.py's current_master.team == player.team today means "the same team object" and would start meaning "any team with the same name", which could mask wiring bugs (two accidentally-distinct Team("North-South") instances) instead of surfacing them.
  • Doesn't solve any of the future needs (serialization, type-safe keys).

Option B — full frozen dataclass

Proper value semantics, immutability guaranteed by construction. But it is blocked by the player↔team reference cycle unless players reference their team by seat/name instead of object — a bigger refactor than the problem warrants today, though it points in the right long-term direction.

Option C — TeamId enum in contrai-core (recommended)

Introduce a small TeamId enum (e.g. NORTH_SOUTH / EAST_WEST) and key Round.team_tricks, RoundScore.scores, and Game.scores by it. Team keeps its role as the mutable roster object; identity lives in the enum. Drop the unused Team.total_score / add_points (the engine already owns scoring).

Why it wins for the roadmap:

  • RL/self-play: observations must be cheap, hashable, serializable values — an enum member is exactly that; a live Team dragging two Players and their Hands is what the seat-sealing follow-up is trying to eliminate.
  • Web server / persistence: enums serialize trivially (.value) into JSON keys and SQLite columns; reconstructed data compares correctly by value, which object identity never survives.
  • Type safety: fixes the real weakness (stringly-typed keys) that Option A would only paper over.

Decision

Status quo for now — string keys aren't hurting while there are exactly two teams with fixed names. Execute Option C when the PlayObservation seat-sealing follow-up happens (CLAUDE.md §10 — live BasePlayer refs leak through observations): team identity is the same problem one level up, so both should land in the same refactor.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions