Skip to content

Add game-over detection and PGN save support - #4

Merged
BrandonZacharie merged 3 commits into
mainfrom
claude/review-chess-py-wktEg
May 24, 2026
Merged

Add game-over detection and PGN save support#4
BrandonZacharie merged 3 commits into
mainfrom
claude/review-chess-py-wktEg

Conversation

@BrandonZacharie

Copy link
Copy Markdown
Owner

Summary

Two-part change:

  1. Game-over detection in the engineis_in_check, has_legal_moves, is_checkmate, is_stalemate, is_game_over, and result() on Game. The TODO at game/game.py:458 (# TODO: Append annotation for Checkmate) is now implemented: after a check move commits, if the position is mate, the trailing + on the just-logged algebraic token is swapped for #.
  2. PGN save supportGame.save_pgn(filename, headers=None) writes the Seven Tag Roster plus PlyCount, with sensible defaults. The save_pgn menu entry (previously a dimmed stub) is now wired to a real handler. Result is derived from Game.result().

Commits

  • 07a376d — Add game-over detection and PGN save support.
    • Game.is_in_check / has_legal_moves / is_checkmate / is_stalemate / is_game_over / result. has_legal_moves tries the king's escape squares first as a fast path so non-mate check positions exit after a handful of probes instead of scanning every piece.
    • Move-time # swap inside Game.move after a check move commits. Removes the # TODO: Append annotation for Checkmate marker.
    • Game.parse_pgn_move now also strips # and accepts * as a result token, so anything save_pgn writes round-trips through PGNFile.
    • Game.save_pgn — STR + PlyCount, defaults for unset tags (Casual Game / Chess.py vX.Y.Z / today / ? / ? / ? / derived / counted). Movetext wrapped at ≤ 80 chars. Headers overridable.
    • UI: save_pgn entry de-dimmed in the save menu; Chess._save_pgn mirrors _save_json (no game / FileNotFound / generic exception branches).
    • ~10 new engine tests (Fool's mate, Sam Loyd stalemate, starting position, check-but-not-mate, STR / PlyCount / Result derivation / header override / line wrapping / save→PGNFile roundtrip for both regular and mate-ending games) plus a _save_pgn UI test matrix mirroring _save_json.
  • 1f06861 — Strip +/# in test_load_pgn_file equality assertion. Recorded PGN games vary on whether mates are written Qh4+ or Qh4# — Carlsen never uses # even on actual mates. With the engine's new mate detection emitting #, the assertion now strips both annotations before comparing. The test is about move sequence equivalence, not annotation style.
  • 0f42b90 — Re-derive promotion's check/mate annotation from post-promote state. Six Carlsen games (promotion while in check, or promote-into-check) emitted malformed tokens like exd8#=R because the annotate() helper in promote() only knew how to relocate +, not #. Now strips any pre-existing +/# first, appends =PIECE, then re-derives the marker from the post-promotion state (running has_legal_moves() against the opponent to distinguish mate from check).

Why the existing engine tests didn't surface this

The test_load_pgn_file parametrize on Carlsen/Nakamura was the safety net, but it was annotation-strict. Before this PR the engine never emitted #, so the assertion only ever compared engine + against PGN + and the gap was invisible. Once mate detection switched things on, the slow test caught 6 promotion-related mismatches in Carlsen which led to the annotate() fix. The new fast-suite tests (Fool's mate, Sam Loyd stalemate, save→reload roundtrip on a mate game) keep this regression net in place going forward.

Performance impact

has_legal_moves runs once per +-annotated move (~5% of moves typically) plus once per promotion. The king-first fast path makes the common "check, king has an escape" case exit fast. Empirically:

Sweep Before After
Carlsen.pgn (5662 games) ~2:00 ~3:30
Carlsen + Nakamura ~7:00 ~8:15

Daily pytest (skips slow) unchanged: ~1.2 s.

Test plan

  • make test — fast suite passes (207 passed, 2 deselected).
  • make test-all — full sweep including Carlsen/Nakamura passes in ~8.3 min.
  • coverage report after make test shows TOTAL 100%.
  • Manual: play to Fool's mate (1.f3 e5 2.g4 Qh4#), open the save menu, select PGN, type a path, hit Enter — file written with [Result "0-1"] and movetext ending Qh4#.
  • Manual: same flow on a mid-game (no mate) — file written with [Result "*"].
  • Manual: save → quit → load PGN → confirm position resumes correctly.

https://claude.ai/code/session_01RnQvxtXfn9JFTN8jpNyriL


Generated by Claude Code

claude added 3 commits May 24, 2026 03:36
Engine additions on game/game.py:

- is_in_check / has_legal_moves / is_checkmate / is_stalemate /
  is_game_over / result. Detection walks every legal move for the
  player to move via game.move(.., can_commit=False). has_legal_moves
  tries the king's own moves first as a fast path so the common
  "check, but the king has an escape" case exits after a handful of
  probes instead of probing all 16 pieces.
- After move() commits, if the position is checkmate, the trailing
  '+' in the just-logged elog token is swapped for '#'. Removes the
  long-standing `# TODO: Append annotation for Checkmate` marker.
- parse_pgn_move now also strips '#' and accepts '*' as a result
  token so a save_pgn output can be re-read by PGNFile.

PGN serialization:

- Game.save_pgn(filename, headers=None) writes the Seven Tag Roster
  (Event, Site, Date, Round, White, Black, Result) plus PlyCount,
  with sensible defaults (Casual Game / Chess.py vX.Y.Z / today /
  '?' / '?' / '?' / derived / counted). Result is derived from
  self.result(); '*' for games in progress, '0-1' / '1-0' / '1/2-1/2'
  for terminal positions. Movetext is wrapped at <= 80 chars.
- Headers can be overridden by caller.

UI wiring on chess.py:

- save_pgn entry de-dimmed in the save menu and wired to a
  Chess._save_pgn handler that mirrors _save_json (no game / generic
  exception / FileNotFoundError branches each draw an error message,
  successful save returns True so _fileprompt closes and the
  enclosing save_pgn raises KeyboardInterrupt to exit).
- A_DIM import removed (no remaining use site).

Tests (~10 new):
- engine: Fool's mate detected as checkmate (with '#' annotation),
  Sam Loyd 1899 stalemate detected as draw, starting position and
  in-progress check positions return '*', save_pgn writes the STR
  and PlyCount, Result reflects engine state, headers override,
  movetext contains the log and wraps at <=80, save_pgn -> PGNFile
  roundtrip preserves move sequence both for a regular opening and
  a mate-ending game.
- UI: _save_pgn handler matrix (no game, success, FileNotFound,
  generic exception), save_pgn UI no-op + success flows.
Recorded PGN games vary on whether checkmate moves are written 'Qh4+'
or 'Qh4#' — Carlsen.pgn never uses '#' even on positions that are
actual mates. With the engine's new mate detection emitting '#', the
sequence-equality assertion in test_load_pgn_file now sees 'Rd1+' on
the PGN side vs 'Rd1#' on the log side at game 9 / ply 75 (and
several others).

Normalize both sides by stripping trailing '+' / '#' before
comparing. The test is about whether the replay reaches the same
move sequence, not about annotation style.
The annotate() helper inside promote() handled the case where the
pre-promotion move() had left a '+' on the elog token, but didn't
account for the '#' marker mate detection now produces. Six Carlsen
games (with promotion-while-in-check or promote-into-check patterns)
were emitting malformed tokens like 'exd8#=R' / 'c8#=Q' — the marker
landed before '=PIECE' instead of after it.

Strip any pre-existing '+' / '#' first, append '=PIECE', then
re-derive the marker by checking opponent.king.is_safe and (if in
check) calling has_legal_moves() to distinguish mate from check.
Self.turn correctly points at the opponent at this point in the
flow (move() set _last_turn; promote() doesn't touch it), so the
mate probe runs on the right team. Verified against all six failing
Carlsen positions.
@BrandonZacharie
BrandonZacharie merged commit e4daa22 into main May 24, 2026
3 checks passed
@BrandonZacharie
BrandonZacharie deleted the claude/review-chess-py-wktEg branch May 24, 2026 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants