Restructure engine, package as src/pychess, and add tooling - #4
Conversation
## Search: composition instead of a mixin stack `NegamaxEngine` was one class fused from seven mixins via the MRO, with collaborators communicating through `self` and hook-method overrides. That was fragile (alphabetising the base list silently disabled the TT), untestable in pieces, and required no-op stub hooks on the base. - `Negamax` now takes an evaluator, move orderer, transposition table, and clock as constructor arguments. Quiescence folds into it. - `TranspositionTable` and `SharedTT` share a real `key`/`probe`/`store` interface; the searcher is handed whichever one. - `Clock` replaces the `_can_abort`/`_deadline`/`_stop_flag` attribute-poking. - `lazy_smp.search` returns a `SearchResult`; all UCI `info`/`bestmove` formatting moved to the UCI layer. The engine never prints. - The single-process search path is gone: `go` always runs Lazy SMP. - Score/flag constants centralised in `constants.py`. ## Packaging: src/pychess layout - `pyproject.toml` (hatchling) replaces `requirements.txt` and `pytest.ini`; version single-sourced from `__init__.py`. - `pychess` console entry point; `python -m pychess` also works. - `py.typed`; `GoLimits` TypedDict; `SupportsSearch` protocol. - License changed to Apache-2.0. ## Tooling - Ruff (lint + format), Mypy (strict on `src/`), pytest-cov with an 85% gate - all run in CI on Python 3.12 and 3.13, mirrored by pre-commit. - CI: `permissions`, `concurrency`, SHA-pinned actions, a `ci-ok` aggregate job to gate branch protection on, and a Codecov upload step. - Coverage is 92% (80 tests, up from 22); every module has a `test_<module>.py`. ## Docs and repo hygiene - Deleted the legacy `archive/` tree and unused evaluation mixins. - README rewritten; deep reference lists moved to `docs/design.md`. `docs/` files lowercased; `STRENGTH.md` -> `docs/engine-strength.md`. - Added CONTRIBUTING, CHANGELOG, SECURITY, CODE_OF_CONDUCT, .editorconfig, .gitattributes, dependabot, issue/PR templates, `opening_book/README.md`. - `CLAUDE.md` + `AGENTS.md` symlink; skills in `.claude/skills/` (`update-docs`, `bench`, `pr-check`, `release`). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Lazy SMP currently time-caps depth-only searches and ignores the go nodes limit, which can break UCI limit semantics.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR performs a repo-wide restructure of the chess engine into a typed src/pychess/ package, replacing the prior mixin-based engine with composed collaborators (Negamax + evaluator + move ordering + TT + clock) and standardizing tooling/CI around Ruff, Mypy, and pytest-cov.
Changes:
- Rebuilt the search stack into
Negamax+ collaborators and madegoalways run Lazy SMP (lazy_smp.search→SearchResult, no engine-layer printing). - Migrated to
src/packaging withpyproject.toml, typed public surfaces (py.typed,GoLimits), and apychessconsole entry point. - Added/updated tests, docs, templates, and CI/tooling (Ruff/Mypy/pytest-cov + coverage gate, Codecov upload, pre-commit, repo hygiene files).
File summaries
| File | Description |
|---|---|
| transposition.py | Removed legacy top-level transposition-table mixin implementation. |
| tests/test_uci.py | Added UCI protocol layer tests. |
| tests/test_transposition.py | Added unit tests for the new in-process TT API. |
| tests/test_perft.py | Updated perft imports to the new pychess.perft module path. |
| tests/test_negamax.py | Added unit tests for composed Negamax search and helpers. |
| tests/test_move_ordering.py | Added unit tests for MoveOrderer. |
| tests/test_evaluation.py | Added unit tests for PestoEvaluator and incremental eval consistency. |
| tests/test_engine.py | Added integration tests for Lazy SMP coordinator/worker and assembled engines. |
| tests/test_clock.py | Added unit tests for time management and stop predicate. |
| tests/init.py | Added tests package marker. |
| src/pychess/types.py | Added shared type aliases (GoLimits). |
| src/pychess/transposition.py | Added in-process TranspositionTable implementation. |
| src/pychess/shared_tt.py | Updated shared-memory TT implementation and typing; aligned constants imports. |
| src/pychess/py.typed | Declared the package as typed for type checkers. |
| src/pychess/perft.py | Refactored perft utilities and typing; clarified manual entrypoint. |
| src/pychess/negamax.py | Added the composed Negamax searcher (alpha-beta + quiescence). |
| src/pychess/move_ordering.py | Replaced mixin with MoveOrderer class and typed heuristics state. |
| src/pychess/lazy_smp.py | Added Lazy SMP coordinator + worker protocol and SearchResult. |
| src/pychess/evaluation.py | Replaced legacy eval mixins with PeSTO tables and PestoEvaluator. |
| src/pychess/eval_board.py | Updated incremental evaluation board with typing and new imports. |
| src/pychess/engine.py | Added assembled Engine and RandomEngine implementations. |
| src/pychess/constants.py | Added shared scoring and TT-flag constants. |
| src/pychess/clock.py | Added UCI time management and stop predicate (Clock). |
| src/pychess/main.py | Added UCI protocol loop/handlers as the only printing layer. |
| src/pychess/init.py | Added package init and version single source of truth. |
| SECURITY.md | Added security policy and reporting guidance. |
| search.py | Removed legacy search mixin stack implementation. |
| requirements.txt | Removed legacy requirements file (moved to pyproject.toml). |
| README.md | Rewritten README to match new architecture, usage, and repo layout. |
| pytest.ini | Removed legacy pytest config (moved to pyproject.toml). |
| pyproject.toml | Added hatchling packaging + unified tool configuration (ruff/mypy/pytest/cov). |
| parallel.py | Removed legacy parallel/multiprocessing implementation. |
| opening_book/README.md | Documented the Polyglot opening book usage and provenance notes. |
| main.py | Removed legacy entrypoint (replaced by pychess / python -m pychess). |
| LICENSE | Switched licensing to Apache-2.0. |
| engines.py | Removed legacy mixin-engine assembly. |
| docs/tasks.md | Added roadmap/tasks document for search/eval follow-ups. |
| docs/performance.md | Added performance documentation and benchmark tables (with caveats). |
| docs/engine-strength.md | Updated engine strength doc naming and references to new roadmap. |
| docs/design.md | Added design/architecture documentation for the new composed engine. |
| CONTRIBUTING.md | Added contributor setup and PR hygiene guidance. |
| codecov.yml | Added Codecov project/patch status targets and comment config. |
| CODE_OF_CONDUCT.md | Added community code of conduct. |
| CLAUDE.md | Added repo “agent guide” describing architecture and workflows. |
| CHANGELOG.md | Added changelog with unreleased entries capturing the restructure. |
| build.bat | Removed legacy PyInstaller build script. |
| archive/transposition_table.py | Removed legacy archived TT implementation. |
| archive/test/test_quiescence_search.py | Removed archived tests. |
| archive/test/test_puzzles.py | Removed archived tests. |
| archive/test/test_evaluation.py | Removed archived tests. |
| archive/test/puzzles.py | Removed archived puzzle corpus and helpers. |
| archive/src/tt.py | Removed archived engine implementation. |
| archive/src/search.py | Removed archived engine implementation. |
| archive/src/psqt.py | Removed archived engine implementation. |
| archive/src/limits.py | Removed archived engine implementation. |
| archive/src/helpers.py | Removed archived engine implementation. |
| archive/src/evaluation.py | Removed archived engine implementation. |
| archive/quiescence_search.py | Removed archived quiescence implementation. |
| archive/pyChess.py | Removed archived GUI/self-play harness. |
| archive/negamax.py | Removed archived negamax engine implementation. |
| archive/move_ordering.py | Removed archived move ordering implementation. |
| archive/helper.py | Removed archived helper utilities. |
| archive/evaluation.py | Removed archived evaluation implementation. |
| archive/config.py | Removed archived config. |
| archive/init.py | Removed archived import path hack. |
| .pre-commit-config.yaml | Added pre-commit hooks mirroring CI’s lint/format/type checks. |
| .gitignore | Simplified/standardized ignore patterns for Python + tooling caches. |
| .github/workflows/ci.yml | Added CI workflow for lint/typecheck + test matrix + Codecov upload + aggregate gate. |
| .github/PULL_REQUEST_TEMPLATE.md | Added PR template aligned to CI gate and docs/changelog expectations. |
| .github/ISSUE_TEMPLATE/feature_request.yml | Added feature request issue template. |
| .github/ISSUE_TEMPLATE/config.yml | Added issue template configuration and contact links. |
| .github/ISSUE_TEMPLATE/bug_report.yml | Added bug report issue template. |
| .github/dependabot.yml | Added dependabot config for pip + GitHub Actions. |
| .gitattributes | Added attrs for binary files and linguist classification. |
| .editorconfig | Added editorconfig for consistent formatting rules. |
| .claude/skills/update-docs/SKILL.md | Added documentation update workflow skill definition. |
| .claude/skills/release/SKILL.md | Added release workflow skill definition. |
| .claude/skills/pr-check/SKILL.md | Added PR gate workflow skill definition. |
| .claude/skills/bench/SKILL.md | Added benchmark workflow skill definition. |
Review details
- Files reviewed: 76/80 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: cjunius <41166228+cjunius@users.noreply.github.com>
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
🟡 Changes recommended
The UCI go nodes limit is parsed but not enforced under Lazy SMP, and the UCI info time output uses seconds (not milliseconds), which can break protocol consumers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
src/pychess/lazy_smp.py:118
go nodes <n>is parsed (andClocksupportsnode_limit), but Lazy SMP never propagates thenodeslimit into the workers, so node-limited searches effectively run unbounded.
src/pychess/main.py:129- UCI
info ... time <t>is specified in milliseconds, but this prints elapsed time in seconds (a rounded float), which can confuse GUIs and downstream tooling.
This issue also appears on line 132 of the same file.
src/pychess/main.py:134
- UCI
info ... time <t>should report time in milliseconds; this currently prints the raw seconds float returned byperft().
- Files reviewed: 76/80 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: cjunius <41166228+cjunius@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The UCI layer currently has crash-on-invalid-input paths and non-standard info time formatting, and Lazy SMP worker stop/clock arming can violate short time controls.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
Previously missed (2) — in code that hasn't changed since the last review.
src/pychess/main.py:90
- Invalid FEN strings will raise
ValueErrorinEvalBoard(fen_string)and crash the UCI loop. Since UCI input is untrusted, the protocol layer should handle bad FEN gracefully (e.g., reset to startpos or ignore the command).
This issue also appears on line 93 of the same file.
src/pychess/main.py:129
- UCI
infolines expecttimeto be an integer number of milliseconds; printing a seconds float (round(result.elapsed, 3)) can confuse GUIs and makes output inconsistent with the UCI protocol field semantics.
This issue also appears on line 131 of the same file.
src/pychess/main.py:97
- Invalid UCI moves in a
position ... moves ...command will raiseValueErrorinpush_uciand crash the engine. The UCI layer should suppress/handle parse errors so malformed input can’t take down the process.
src/pychess/main.py:134 perftoutput uses aninfo ... timefield too; for consistency with UCI output (and most GUIs), this should also be milliseconds rather than a raw seconds float.
- Files reviewed: 76/80 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It’s a large structural rewrite plus tooling/CI changes, and at least one confirmed functional issue remains (depth-limit handling in lazy_smp.search).
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/pychess/lazy_smp.py:127
max_depthis computed withlimits.get("depth") or MAX_DEPTH, so an explicitgo depth 0(or any falsy depth) is treated as "no depth limit" and ends up searching toMAX_DEPTH. This can make a supposedly minimal search run much longer than requested.
- Files reviewed: 76/80 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
A full pass over the repo:
Negamaxtakes an evaluator, move orderer, transposition table, and clock asconstructor arguments;
TranspositionTableandSharedTTshare oneinterface;
Clockreplaces the old attribute-poking.lazy_smp.searchreturns a
SearchResultand the engine no longer prints — all UCIinfo/bestmoveformatting is in the UCI layer.src/pychesslayout with a hatchlingpyproject.toml(replacing
requirements.txt/pytest.ini), apychessconsole script,py.typed, and aGoLimitsTypedDict. License → Apache-2.0.src/), pytest-cov with an85% gate — in CI on 3.12/3.13 and mirrored by pre-commit. CI gained
permissions,concurrency, SHA-pinned actions, aci-okaggregate job, anda Codecov upload step.
archive/tree and unused eval mixins; READMErewritten with reference material moved to
docs/design.md; addedCONTRIBUTING / CHANGELOG / SECURITY / CODE_OF_CONDUCT / editorconfig /
gitattributes / dependabot / issue + PR templates /
opening_book/README.md;CLAUDE.md+AGENTS.mdand.claude/skills/.See the commit message for the file-level detail.
Behaviour change: the single-process search path is removed —
goalwaysruns Lazy SMP. Moves, scores, and UCI I/O are otherwise unchanged.
Testing
ruff check .,ruff format --check .,mypy(strict onsrc/) — cleanpytest— 80 passing, 92% coverage (gate 85%), on Python 3.12 and 3.13pychessandpython -m pychessverified end-to-end including the Lazy SMPmultiprocessing path; editable and non-editable installs verified
Checklist
ruff check .andruff format --check .passmypypasses (strict onsrc/)pytestpasses; coverage stays >= 85%CHANGELOG.mdupdated under## [Unreleased]README.md/docs/*.mdupdateddocs/performance.md— n/a; no measured speed change, numbers pending a/benchrunFollow-ups (tracked in
docs/tasks.md)pyChess→pychessCODECOV_TOKENActions secret to activate the coverage badgemainrequiring theci-okcheckv0.1.0and cut a Release🤖 Generated with Claude Code