Fix draw handling: illegal castling, repetition, 50-move, insufficient material#27
Merged
Conversation
Two bugs caused losses on Lichess: 1. TT-cached castling moves bypassed legality checks in is_pseudo_legal_compact — no verification that the king wasn't in check or passing through attacked squares. This caused the engine to send illegal moves, triggering lichess-bot to resign from winning positions. 2. Search had zero repetition awareness, so the engine would shuffle pieces indefinitely in won positions, drawing by threefold repetition. Fixes: - Add opponent attack map checks for castle-out-of-check and castle-through-check in is_pseudo_legal_compact - Thread position_history (Vec<u64> of zobrist hashes) through negamax search; treat twofold repetition as score 0 (draw) - Build position history from UCI "position ... moves" command - Push current hash at node entry, pop on exit, so descendants see all ancestor positions for repetition detection Verified against 5 actual Lichess games that exhibited these bugs. No performance regression (±3%, identical node counts).
Both draw types were tracked in the board but never checked during search. The engine could waste time searching K vs K endgames at full depth, or miss draws by the fifty-move rule. Both checks are O(1) (bitboard count_ones and integer comparison) and sit right next to the existing repetition detection.
Runs full test suite on push/PR to master, skipping tests that require the Cerebellum opening book file (not in repo).
The old src/bin/uci.rs used raw minimax without time control, position history, or draw detection. All production use goes through uci_movepicker. Retarget integration tests accordingly and relax movetime lower bound (iterative deepening finishes faster than fixed-depth search).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
is_pseudo_legal_compactnow checks that the king isn't in check and doesn't pass through attacked squares when validating cached castling moves. Previously the engine would send illegal castling moves to Lichess, causing lichess-bot to resign from winning positions.halfmove_clock >= 100at each search node, returning draw score.Test plan