From db8145fb97bc13ffa6f0cc2786dd4b36db4cf323 Mon Sep 17 00:00:00 2001 From: Joey Yakimowich-Payne Date: Mon, 20 Apr 2026 20:08:54 -0600 Subject: [PATCH] feat(engine): filterLegalMoves hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase A.2 of the rule-variants epic — adds the post-aggregation legal-move filter pipeline. - Adds FilterLegalMovesContext + filterLegalMoves hook on PresetDef. Hook runs AFTER the self-check filter; iterated in activePresets list order, each preset sees the prior's output. Subset-only contract (drop, don't synthesize); documented + pinned via test. - Wires the iteration into engine.getAllLegalMoves — immediately after the royal-aware self-check filter lands. - Adds filter-legal-moves.test.ts (6 tests): baseline unchanged, no-a-file drop filter, compulsory-capture prototype (proves the suicide-chess rule surface before D.1 lands), two-preset composition, and subset-contract doc test. Tests: 1434 passing (was 1428, +6 new). Blocks: unblocks D.1 (suicide-chess). --- .sisyphus/notepads/rule-variants/learnings.md | 29 +++ packages/chess/src/engine.ts | 21 +- .../src/presets/filter-legal-moves.test.ts | 213 ++++++++++++++++++ packages/chess/src/presets/registry.ts | 49 ++++ 4 files changed, 311 insertions(+), 1 deletion(-) create mode 100644 packages/chess/src/presets/filter-legal-moves.test.ts diff --git a/.sisyphus/notepads/rule-variants/learnings.md b/.sisyphus/notepads/rule-variants/learnings.md index c2a58c5..85b8303 100644 --- a/.sisyphus/notepads/rule-variants/learnings.md +++ b/.sisyphus/notepads/rule-variants/learnings.md @@ -124,3 +124,32 @@ section). - `packages/chess/src/presets/royal-pieces.test.ts` (new, 300+ lines, 11 tests) **Blocks**: A.1 now clears B.1, C.1, C.2, C.3, D.1, D.2. Phase A remaining: A.2 (filterLegalMoves), A.3 (shouldAdvanceTurn + HalfMovesThisTurn), A.4 (overridePieceMoves), A.5 (gate + PRESET-API.md docs). + +## [2026-04-20 20:08] Task: A.2 — filterLegalMoves hook + +**Shipped**: +- `FilterLegalMovesContext` type + `filterLegalMoves` hook on + `PresetDef` (`packages/chess/src/presets/registry.ts`). Context + carries `engine`, `color`, `moves`. Hook signature returns + `readonly LegalMove[]` (subset of input). +- `engine.getAllLegalMoves`: after the self-check filter + royal + resolution, iterate every active preset's `filterLegalMoves` in + `activePresets.list()` order. Each preset sees the prior's output. +- New test file: `packages/chess/src/presets/filter-legal-moves.test.ts` + (6 tests). Covers: baseline unchanged (FIDE opening = 20 moves), + prototype "no-a-file" drop filter, compulsory-capture prototype + (suicide-chess surface — proven before D.1 lands), two-preset + composition (a-file + h-file = 14 surviving), and the subset- + contract documentation test. + +**Verification**: `bun run check` green. 1434 tests (was 1428, +6). + +**Gotchas**: +- `EntityId` is branded — tests creating synthetic `LegalMove` + literals need `999 as EntityId`. Vitest was happy; `tsc -b` caught + it on the full check. +- Hook contract is subset-only; the engine does NOT enforce it at + runtime (documented + pinned via a test). Future enforcement + would be a visible diff. Keeping enforcement out for now + because the 1 call site per legal-move request is hot-path- + adjacent and a `Set`-based prune would show up. diff --git a/packages/chess/src/engine.ts b/packages/chess/src/engine.ts index 087efb6..99fe9d7 100644 --- a/packages/chess/src/engine.ts +++ b/packages/chess/src/engine.ts @@ -50,6 +50,7 @@ import type { DamageContext, DamageHookContext, DescribeMoveEffectContext, + FilterLegalMovesContext, GameResultHookContext, LifecycleContext, MoveHookContext, @@ -1005,9 +1006,27 @@ export class ChessEngine { } } const royalIds = this.getActiveRoyalEntityIds(color); - return applyFilter + let finalMoves = applyFilter ? filterSelfCheckMoves(this.session, moves, color, royalIds) : moves; + + // Phase A.2: post-aggregation filter pass. Each active preset's + // `filterLegalMoves` runs in registration order, receiving the + // accumulated list from the prior. Used by `suicide-chess` to + // enforce compulsory capture. See `filterLegalMoves` docs on + // `PresetDef` for composition semantics. + for (const entry of this.activePresets.list()) { + const def = PRESET_REGISTRY.get(entry.id); + if (def?.filterLegalMoves === undefined) continue; + const ctx: FilterLegalMovesContext = { + engine: this, + color, + moves: finalMoves, + }; + finalMoves = [...def.filterLegalMoves(ctx)]; + } + + return finalMoves; } applyMove(move: LegalMove, promoteTo: PieceType = "queen"): GameResult { diff --git a/packages/chess/src/presets/filter-legal-moves.test.ts b/packages/chess/src/presets/filter-legal-moves.test.ts new file mode 100644 index 0000000..50c6bf9 --- /dev/null +++ b/packages/chess/src/presets/filter-legal-moves.test.ts @@ -0,0 +1,213 @@ +/** + * Tests for the `filterLegalMoves` hook (Phase A.2 of the rule-variants + * epic). + * + * The hook runs AFTER: + * - per-piece move generation (type registry + transformMoveGenerator) + * - per-piece contributions (getExtraMoves) and filters (filterMoves) + * - the self-check filter (unless opted out) + * and BEFORE: + * - `getAllLegalMoves` returns its result to callers + * + * Each active preset's `filterLegalMoves` is iterated in registration + * order; each sees the OUTPUT of the prior. A preset returning a + * subset DROPS the unlisted moves. Synthesizing new moves is NOT + * supported — that's what `getExtraMoves` is for. + * + * Coverage: + * - Prototype "no-a-file" preset drops every move whose source or + * destination is on the a-file. Affects multiple pieces. + * - No active preset with the hook → getAllLegalMoves unchanged + * (regression coverage for the 1428-test baseline). + * - Compulsory-capture prototype (stand-in for suicide-chess): if + * any move is a capture, drop everything else. Used to prove the + * hook is enough to implement the motivating use case before + * suicide-chess lands in Phase D. + * - Two presets stacking: first drops one class, second drops + * another — each sees the prior's output. Documents the + * composition model. + */ +import { describe, it, expect, beforeEach } from "vitest"; +import "./index.js"; +import type { EntityId } from "@paratype/rete"; +import { ChessEngine } from "../engine.js"; +import { PRESET_REGISTRY } from "./registry.js"; +import type { LegalMove } from "../rules/types.js"; +import { clearBoard, placePiece } from "./test-utils.js"; +import { fileOf } from "../coord.js"; + +const NO_A_FILE_ID = "test-no-a-file"; +const COMPULSORY_CAPTURE_ID = "test-compulsory-capture"; +const NO_H_FILE_ID = "test-no-h-file"; + +beforeEach(() => { + // Drops any move touching the a-file (either `from` or `to`). + PRESET_REGISTRY.register({ + id: NO_A_FILE_ID, + name: "No a-file (test)", + description: "Drops every move starting on or landing on the a-file.", + incompatibleWith: [], + requires: [], + filterLegalMoves({ moves }) { + return moves.filter(m => fileOf(m.from) !== 0 && fileOf(m.to) !== 0); + }, + }); + + // If any capture is legal, keep only captures — the suicide-chess + // rule surface, proven here against the hook directly. + PRESET_REGISTRY.register({ + id: COMPULSORY_CAPTURE_ID, + name: "Compulsory capture (test)", + description: + "If any move in the aggregated list is a capture, restrict to captures.", + incompatibleWith: [], + requires: [], + filterLegalMoves({ moves }) { + const anyCapture = moves.some(m => m.isCapture === true); + if (!anyCapture) return moves; + return moves.filter(m => m.isCapture === true); + }, + }); + + // Drops any move touching the h-file. Used alongside NO_A_FILE_ID + // to verify two presets stack deterministically. + PRESET_REGISTRY.register({ + id: NO_H_FILE_ID, + name: "No h-file (test)", + description: "Drops every move starting on or landing on the h-file.", + incompatibleWith: [], + requires: [], + filterLegalMoves({ moves }) { + return moves.filter(m => fileOf(m.from) !== 7 && fileOf(m.to) !== 7); + }, + }); +}); + +describe("filterLegalMoves — baseline back-compat", () => { + it("no active preset with the hook → getAllLegalMoves unchanged", () => { + // This test exists purely as a regression check: if someone adds + // an always-on filter downstream, this fails. We compare the + // initial white move count to the well-known FIDE opening number. + const engine = new ChessEngine(); + const whiteMoves = engine.getAllLegalMoves(); + // Standard chess opening: 8 pawn pushes × 2 (1- or 2-square) = 16 + // pawn moves + 4 knight moves = 20 legal moves for white. + expect(whiteMoves).toHaveLength(20); + }); +}); + +describe("filterLegalMoves — drop-filter prototype", () => { + it("no-a-file preset drops every move touching file 0", () => { + const engine = new ChessEngine(); + engine.setActivePresets([ + { id: NO_A_FILE_ID, scope: "both", turnsRemaining: null }, + ]); + + const moves = engine.getAllLegalMoves(); + // None of the surviving moves touch the a-file. + for (const m of moves) { + expect(fileOf(m.from)).not.toBe(0); + expect(fileOf(m.to)).not.toBe(0); + } + // Sanity: some moves DID survive (we didn't nuke the whole list). + expect(moves.length).toBeGreaterThan(0); + // And we DID lose moves compared to the baseline — opening has + // a-pawn + knight-to-a3 = 3 moves touching the a-file. + expect(moves.length).toBeLessThan(20); + }); +}); + +describe("filterLegalMoves — compulsory-capture prototype (suicide-chess surface)", () => { + it("no captures available → list unchanged", () => { + // Minimal position with no captures possible. + const engine = new ChessEngine(); + engine.setActivePresets([ + { id: COMPULSORY_CAPTURE_ID, scope: "both", turnsRemaining: null }, + ]); + const before = engine.getAllLegalMoves(); + // FIDE opening has no captures → compulsory-capture hook is a no-op. + expect(before.length).toBe(20); + expect(before.every(m => m.isCapture !== true)).toBe(true); + }); + + it("at least one capture available → only captures survive", () => { + const engine = new ChessEngine(); + clearBoard(engine, { preserveKings: false }); + + placePiece(engine, "king", "white", "e1"); + placePiece(engine, "king", "black", "e8"); + // White rook on a1 can capture black pawn on a5. It also has + // many non-capture sliding moves. The hook should discard those. + placePiece(engine, "rook", "white", "a1"); + placePiece(engine, "pawn", "black", "a5"); + + engine.setActivePresets([ + { id: COMPULSORY_CAPTURE_ID, scope: "both", turnsRemaining: null }, + ]); + + const moves = engine.getAllLegalMoves(); + expect(moves.length).toBeGreaterThan(0); + // Every surviving move MUST be a capture. + expect(moves.every(m => m.isCapture === true)).toBe(true); + // And specifically the rook-captures-pawn move must be present. + const rxa5 = moves.find(m => m.to === 32); // a5 = rank 4 * 8 + file 0 = 32 + expect(rxa5).toBeDefined(); + expect(rxa5?.isCapture).toBe(true); + }); +}); + +describe("filterLegalMoves — composition (registration order)", () => { + it("stacked no-a-file + no-h-file drops moves touching either edge file", () => { + const engine = new ChessEngine(); + engine.setActivePresets([ + { id: NO_A_FILE_ID, scope: "both", turnsRemaining: null }, + { id: NO_H_FILE_ID, scope: "both", turnsRemaining: null }, + ]); + + const moves = engine.getAllLegalMoves(); + for (const m of moves) { + expect(fileOf(m.from)).not.toBe(0); + expect(fileOf(m.from)).not.toBe(7); + expect(fileOf(m.to)).not.toBe(0); + expect(fileOf(m.to)).not.toBe(7); + } + // FIDE opening: 20 moves; a-file costs 3 (a2-a3, a2-a4, Na3); + // h-file costs 3 (h2-h3, h2-h4, Nh3); no overlap → 14. + expect(moves.length).toBe(14); + }); + + it("subset property preserved — hook cannot synthesize moves", () => { + // Register a malicious preset that tries to return a move NOT in + // the input. The engine is defensive-by-contract: we document the + // hook's contract in `PresetDef.filterLegalMoves` but don't + // enforce at runtime. This test captures current behaviour so a + // future change to ADD enforcement is a visible diff. + const BAD_ID = "test-synthesize-move"; + PRESET_REGISTRY.register({ + id: BAD_ID, + name: "Synthesize (test)", + description: "Tries to add a fake move. Documented as violating the contract.", + incompatibleWith: [], + requires: [], + filterLegalMoves({ moves }) { + const fakeMove: LegalMove = { + pieceId: 999 as EntityId, + from: 0, + to: 1, + isCapture: false, + }; + return [...moves, fakeMove]; + }, + }); + + const engine = new ChessEngine(); + engine.setActivePresets([ + { id: BAD_ID, scope: "both", turnsRemaining: null }, + ]); + const moves = engine.getAllLegalMoves(); + // Current behaviour: the engine does NOT prune the fake move. + // The contract docs this as a caller responsibility. Test + // asserts 20 (baseline) + 1 (fake) = 21 to lock the semantics. + expect(moves.length).toBe(21); + }); +}); diff --git a/packages/chess/src/presets/registry.ts b/packages/chess/src/presets/registry.ts index f719562..3c729f9 100644 --- a/packages/chess/src/presets/registry.ts +++ b/packages/chess/src/presets/registry.ts @@ -229,6 +229,28 @@ export interface RoyalContext extends HookContext { readonly color: "white" | "black"; } +/** + * Context passed to `filterLegalMoves`. Carries the aggregated legal- + * move list (post-self-check-filter) + the color whose moves are being + * generated, so the preset can filter / reorder / drop entries. + * + * Phase A.2 (rule-variants epic): this is the post-aggregation + * pipeline stage. The `moves` array is already fully filtered for + * self-check (unless a preset opted out via `shouldFilterSelfCheck`); + * presets iterate in registration order, each seeing the OUTPUT of + * the prior. Composition is safe iff the transforms are commutative + * — for "drop" filters this is trivially true; for reorderings, use + * `incompatibleWith` to keep conflicting presets from stacking. + * + * Used by `suicide-chess` to enforce compulsory capture (if any + * capture is legal, return only captures) — the canonical motivating + * example. + */ +export interface FilterLegalMovesContext extends HookContext { + readonly color: "white" | "black"; + readonly moves: readonly LegalMove[]; +} + /** * Why a piece is being spawned. Used by `onPieceSpawn` hooks to * decide whether to participate (e.g., a preset that resurrects @@ -497,6 +519,33 @@ export interface PresetDef { ctx: RoyalContext, ) => readonly EntityId[] | undefined; + /** + * Post-aggregation filter on the per-color legal-move list. + * + * Runs AFTER the engine's self-check filter (and after every + * preset's `getExtraMoves` + `filterMoves` have contributed to the + * per-piece list) but BEFORE the engine returns the final list to + * callers. Iterated in preset-registration order; each preset sees + * the output of the previous. + * + * The returned array MUST be a subset of `ctx.moves` (or an equal + * reordering) — the hook can DROP entries but cannot SYNTHESIZE + * new ones. Use `getExtraMoves` for contribution; use this hook for + * filtration. + * + * Canonical use: `suicide-chess`. If any move in `ctx.moves` has + * `isCapture === true`, return only the captures → enforces + * compulsory capture without per-piece reasoning. + * + * Composition: SAFE for pure-subset filters (commutative across + * presets). For stricter filters (e.g. "only moves into the top + * half of the board") that might interact non-commutatively with + * another drop filter, declare `incompatibleWith` between them. + */ + readonly filterLegalMoves?: ( + ctx: FilterLegalMovesContext, + ) => readonly LegalMove[]; + /** * Phase hook: fires AFTER the move has been confirmed legal but * BEFORE any state mutation. Return `{ cancel: true, reason }` to