Skip to content

engine: resolve the seven open TODO markers in contrai-engine #9

Description

@valmathieu

Inventory of every TODO marker currently living in the two first-party game packages.
contrai-core is clean — zero markers. All seven sit in contrai-engine, and they group into
four independent themes. Each box below can be picked off on its own branch.

Line numbers are as of docs/diagrams-review @ 50a7996.


1. Seat configuration is hardcoded (2 markers)

  • packages/contrai-engine/src/contrai_engine/cli.py:17
  • packages/contrai-engine/src/contrai_engine/view/screens/landing.py:124
# cli.py
# TODO: replace with a seat picker on the landing screen. For now the
# layout matches the design handoff exactly: South is the human, the
# other three seats are AI (expert — the default strategies).
HUMAN_SEAT = "South"
SEATS = ("North", "East", "South", "West")
# landing.py — _panel_players()
TODO: replace with a configurable seat picker when we expose
difficulty / player config on the landing screen.

Two halves of the same feature: _build_game() always seats the human at South with three
AiPlayers, and _panel_players() renders that same layout as a literal table
(("S", "You", "human", …)). The two are only consistent by hand — nothing forces the landing
panel to describe the game the CLI actually builds.

Resolving this means a seat/difficulty picker on the landing screen feeding a single setup
description that both the view and _build_game() read. It also unlocks the 4-AI simulation mode
(§2 of CLAUDE.md), which today has no CLI entry point.

MVC note: the picker is user input, so it belongs on the view/controller side; the resulting
configuration should reach Game as data, not as view state.


2. Game.__init__ requires every player to carry a position (1 marker)

  • packages/contrai-engine/src/contrai_engine/model/game.py:73
#TODO: Accept no position and assign positions automatically

Game.__init__ rejects any player list whose positions aren't exactly
{'North', 'West', 'South', 'East'}, so every caller — CLI, tests, future training harnesses —
must pre-assign seats. Accepting positionless players and assigning seats in list order (with the
existing validation kept for the explicit case) would make Game([p1, p2, p3, p4]) work.

Worth deciding at the same time whether the auto-assignment is deterministic (list order) or
randomised (shuffled seating between games), since a self-play harness would want the latter and
tests want the former.

Related: this code still keys seats by raw strings. Issue #5 / #2 (team identity enums) cover the
same "stringly-typed domain identity" smell on the team side, and there is a Position enum in
flight on the integration branch — worth landing that first so the auto-assignment is written
against the enum rather than string literals.


3. Leading-card strategy doesn't exploit a known trump-void table (1 marker)

  • packages/contrai-engine/src/contrai_engine/model/player/rule_based/card_play.py:214
# TODO: exclude trump from logic if we know opponents have no trump left
# No trump left with opponents - play ace from the longest suit
aces = [c for c in playable_cards if c.rank == Rank.ACE]

In _play_leading_card, once _opponents_might_have_trump() returns False the comment claims
opponents are out of trump — but the fallbacks below it (aces, master_cards) still search all
playable_cards, trump included. The AI can therefore lead a trump Ace, or count a trump card as
"master", in a position where a plain-suit winner is strictly better and the trump should be kept.

Fix is to filter playable_cards down to non-trump before the ace/master search when opponents are
known trump-void, keeping trump as the last resort. Both _opponents_might_have_trump and
_is_master_card already receive the fallen / voids tracking they need, so this is local to
_play_leading_card.

Behaviour change to the expert card-play strategy, so it needs new cases in
packages/contrai-engine/tests/test_model/ pinning: (a) known trump-void → plain-suit ace preferred
over a trump ace, (b) opponents might hold trump → current behaviour unchanged.


4. _is_master_card takes a suit where a boolean would do (1 marker)

  • packages/contrai-engine/src/contrai_engine/model/player/rule_based/card_play.py:554
# TODO: replace trump_suit with a boolean is_trump parameter
def _is_master_card(self, card, trump_suit, fallen: dict[Suit, set]) -> bool:

Pure signature cleanup. _is_master_card only uses trump_suit to hand it to _get_higher_ranks,
which itself only asks suit == trump_suit. Passing is_trump: bool makes the dependency explicit
and removes a parameter that reads as "the card might be compared against a different suit".

Note the method is also missing type hints on card / trump_suit (CLAUDE.md §4.1 asks for hints
everywhere including private helpers) — worth fixing in the same pass. _get_higher_ranks has the
same gap and also names its local variable trump_order in the non-trump branch, which is
misleading.


5. Solo Slam is gated exactly like Slam (1 marker)

  • packages/contrai-engine/src/contrai_engine/model/player/rule_based/bidding.py:62
(SLAM_NUMERIC, {}, 0, 0, 8, False),  # Slam — only the trick estimator gates it.
# TODO: tune SoloSlam gate — currently shares Slam's gate. A
# stricter rule (e.g. holds the 8 top trumps in trump-led play,
# or all aces + trump master) would make this conservative.
(SOLO_SLAM_NUMERIC, {}, 0, 0, 8, False),  # Solo Slam — same gate as Slam for now.

Both all-tricks rows in BIDDING_TABLE carry an identical gate (tricks_min=8, no trump/ace
requirement), but the two bids are not equivalent: Slam only asks that the team take all eight
tricks, Solo Slam that the bidder personally does. A hand strong enough for Slam with partner
support is routinely not strong enough for Solo Slam, so the AI will over-bid 500 whenever it
reaches 250.

The comment already sketches candidate rules (8 top trumps in trump-led play, or all aces + trump
master). Deciding between them needs a hand-strength argument rather than a code change — see
contree-domain.md for what Solo Slam actually commits the bidder to.


6. Redouble strategy is a permanent False (1 marker)

  • packages/contrai-engine/src/contrai_engine/model/player/rule_based/bidding.py:236
@staticmethod
def _should_redouble():
    """Determine if we should redouble after being doubled."""

    # TODO: Implement a redouble strategy
    return False

The Redouble plumbing is complete — choose_bid routes a frozen auction to
_choose_under_double, which builds a RedoubleBid, checks auction.is_legal(...), and consults
_should_redouble(). The strategy behind it is the stub: the expert AI never redoubles, and the
tests that cover the redouble path do so by patching this method.

Its sibling _should_double has a working heuristic (_estimate_tricks(suit) * 20 > 162 - value);
_should_redouble needs the mirror argument — how confident the contracting side is that it makes
the contract, given an opponent has just doubled it. Unlike _should_double it currently takes no
arguments, so implementing it means giving it access to the standing contract and the hand.


Notes

  • Per CLAUDE.md §9, items 3, 5 and 6 change engine behaviour and ship with pytest coverage.
    Item 4 is a refactor covered by the existing suite. Items 1 and 2 need new tests for the
    setup/assignment logic (rendering itself is exempt).
  • No TODO markers exist in contrai-analyzer or contrai-scraper either — the seven above are the
    complete inventory for the whole workspace.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions