RFC: deepen the piece-placement cluster behind a PieceReader port #117

New issue

Closed

opened 2026-05-22 12:35:04 +00:00 by root · 7 comments

root commented

2026-05-22 12:35:04 +00:00

(Migrated from gitlab.core-project.space)

Imported from GitHub amaurycoudr/chess-project#117, opened by @amaurycoudr on 2026-05-22 12:35 UTC.
Closed on GitHub on 2026-05-23 06:28 UTC as completed.

Problem

"Where the pieces are" is currently modeled by four overlapping things, all in packages/engine/src/:

Position.board: Board — the placement field of an immutable snapshot
Board — immutable class wrapping sets: Record<PieceSetKey, SquareSet>, a 12-slot record (one bitset per color+type)
PieceSets — that raw 12-slot record type, exported, used structurally by attacks.ts and search-board.ts
SearchBoard — mutable; holds both #sets: PieceSets (raw record it mutates in place) and #board: Board (a Board.wrapSets(#sets) aliasing the same record)

SquareSet itself is a deep module. The shallowness is the cluster around it — four near-synonymous types co-owning one concept, with leaky seams between them:

Three constructors that differ only by aliasing. Board.wrapSets aliases the ref, Board.fromSets shallow-copies, Board.toSets copies out. The choice is invisible at the call site — a copy-vs-alias slip silently corrupts a Game's history.
isAttackedIn bypasses Board. It takes raw { sets: PieceSets; occupancy: SquareSet }. Its two callers feed it differently: rules.isCheck passes a fresh position.board.toSets() copy, SearchBoard.legalMoves passes the live #sets ref.
The 12-slot layout is public. PieceSets and pieceSetKeyOf are exported; the internal shape leaks across board.ts → attacks.ts → search-board.ts.
occupancy() is an O(12) reduce recomputed on every call, including inside the make/unmake hot loop.

Understanding placement means bouncing between four files; the integration risk lives entirely in the aliasing seams between them.

Proposed Interface

A single read port every rule module depends on, with two implementations behind it — an immutable snapshot and a mutable search representation. (Ports & adapters pattern; the "port" here is internal — it decouples rule logic from placement representation.)

// piece-placement.ts — none of this is exported from the package (ADR 0001)

/** A square whose occupant changes across a move. */
export type PieceEdit = {
  readonly square: Square;
  readonly before: Piece | undefined;
  readonly after: Piece | undefined;
};

/**
 * The read-only port for piece placement. Every rule module depends ONLY on
 * this — never on the 12-slot record. A frozen snapshot and a live mutating
 * board are indistinguishable to a consumer. Promises correctness, NOT a
 * complexity class for occupancy() — that preserves the representation-swap
 * escape hatch (ADR 0007's Uint32Array option).
 */
export interface PieceReader {
  pieceAt(square: Square): Piece | undefined;
  squaresOf(color: Color, type: PieceType): SquareSet;
  occupancy(): SquareSet;
  occupancyOf(color: Color): SquareSet;
}

/** Immutable snapshot. Keeps the name `Board`; implements the port. */
export class Board implements PieceReader {
  static empty(): Board;
  static fromPieces(pieces: Iterable<{ square: Square; piece: Piece }>): Board;
  static fromReader(reader: PieceReader): Board;   // the ONLY snapshot-from-anything builder
  // ...PieceReader read methods; occupancy memoized at construction. NO mutators.
}

/** Mutable placement for the search hot path. Owned by SearchBoard, never leaked. */
export class MutablePlacement implements PieceReader {
  static fromReader(reader: PieceReader): MutablePlacement;   // always copies in
  // ...PieceReader read methods; occupancy maintained incrementally.
  applyEdits(edits: readonly PieceEdit[]): void;              // the ONLY mutation primitive
}

Consumer signatures lose the raw record entirely:

// attacks.ts — was ({ sets: PieceSets; occupancy: SquareSet })
isAttackedIn(target: { square: Square; byColor: Color }, pieces: PieceReader): boolean;

// movegen.ts / zobrist.ts — `board: Board` becomes `board: PieceReader`

Usage. Immutable callers receive a PieceReader and never know the implementation:

// rules.isCheck — no toSets() copy, no hand-rolled occupancy
return isAttackedIn({ square: kingSq, byColor: opponentOf(turn) }, position.board);

SearchBoard holds one MutablePlacement (replacing both #sets and #board) and passes it as the port to every rule module:

class SearchBoard {
  readonly #pieces: MutablePlacement;
  // makeMove:  this.#pieces.applyEdits(edits);
  // unmakeMove: this.#pieces.applyEdits(undo.inverse);
  // legalMoves: isAttackedIn({ square: kingSq, byColor: enemy }, this.#pieces);
  // snapshotBoard(): Board { return Board.fromReader(this.#pieces); }
}

What it hides: the 12-slot Record<PieceSetKey, SquareSet> layout (PieceSetKey, PIECE_SET_KEYS, PIECE_FOR_KEY, pieceSetKeyOf all become module-private); occupancy derivation and its cost; the aliasing/copying distinction (one snapshot builder, always copies — silent aliasing becomes unrepresentable). It also folds the three identical edit shapes (ApplyEdit in search-board.ts, ZobristEdit in zobrist.ts, SquareEdit in board.ts) into one PieceEdit.

This is a hybrid of four explored designs: a ports-&-adapters skeleton (keeps Board's name and identity → minimal churn, ADR-faithful), a deliberately minimal 4-method read surface (no speculative squares()/pieces() iteration — add only when a caller needs it), and a correctness-only occupancy() contract.

Dependency Strategy

In-process — pure computation, no I/O. The module is merged directly; no fakes, no adapters for external systems. The "port" is internal: one contract (PieceReader), two adapters (Board immutable, MutablePlacement mutable). isAttackedIn, movegen, and zobrist are rewritten to depend on PieceReader alone, so the same move-generation code runs over a frozen snapshot (cold path) and a live mid-mutation board (hot path) with byte-identical behavior.

The immutable/mutable duality is handled by interface segregation, not a mode flag: applyEdits lives only on MutablePlacement. The immutable Board cannot be mutated because the method is not on its type — the type-safe replacement for ADR 0007's rejected "@internal mutation method on Board". Crossing the boundary always copies: MutablePlacement.fromReader (cold, once per SearchBoard construction) and Board.fromReader (cold, once per played ply). The hot make/unmake loop never crosses.

Testing Strategy

New boundary tests to write:
- A shared, table-driven PieceReader conformance suite run against both Board and MutablePlacement — pieceAt, squaresOf, occupancy, occupancyOf over a range of positions.
- MutablePlacement occupancy invariant: after any sequence of applyEdits (and their inverses), occupancy() equals the recomputed union of all piece sets, and an apply-then-unapply round-trip restores the exact prior state.
- Board.fromReader / MutablePlacement.fromReader independence: mutating the source after a crossing never affects the copy.
- isAttackedIn tested directly against a PieceReader (no { sets, occupancy } assembly).
Old tests to delete / fold in: the Board-internal cases in board.test.ts that assert on wrapSets/fromSets/toSets aliasing semantics — those mechanics no longer exist. Placement-shape assertions in attacks.test.ts migrate to the conformance suite.
Test environment needs: none beyond Vitest. Perft is the correctness guard — node counts in perft.test.ts must be byte-identical at every step of the refactor (the refactor is behavior-preserving).

Implementation Recommendations

Durable guidance, not coupled to today's file paths:

What the module owns: the canonical representation of piece placement — the per-(color,type) bitsets, occupancy derivation (memoized for the immutable impl, incremental for the mutable one), the single mutation primitive (applyEdits), and the immutable↔mutable crossings (always copying).
What it hides: the 12-slot record and its keys; whether occupancy is recomputed, memoized, or maintained incrementally; the backing representation itself (so a future raw-word-array rewrite, ADR 0007's gated option, drops in behind the same port with zero consumer change).
What it exposes: a 4-method read port (pieceAt, squaresOf, occupancy, occupancyOf); an immutable builder family (empty, fromPieces, fromReader); a mutable entry (fromReader + applyEdits). The port promises correctness, never a complexity class. Resist adding iteration helpers until a concrete caller needs one.
How callers migrate: every signature currently typed Board or { sets, occupancy } is widened to PieceReader; bodies are unchanged. SearchBoard drops its #sets/#board pair for one MutablePlacement. pieceSetKeyOf and the PieceSets/PieceSetKey types stop being exported. It is one coordinated, mechanical change — leaving Board alongside a new type would preserve the four-overlapping-things friction, so the migration must land atomically.
ADR alignment: ADR 0001 (nothing exported; Game untouched), ADR 0007 (Board gains no mutation method; SquareSet instances stay immutable — MutablePlacement mutates its slot references and occupancy fields, never a SquareSet; snapshotBoard still pays one copy crossing back). Worth a short note in ADR 0007's lineage, or a new ADR, recording that PieceReader is now the seam between rule logic and placement representation.

> _Imported from GitHub [amaurycoudr/chess-project#117](https://github.com/amaurycoudr/chess-project/issues/117), opened by [@amaurycoudr](https://github.com/amaurycoudr) on 2026-05-22 12:35 UTC._ > _Closed on GitHub on 2026-05-23 06:28 UTC as completed._  --- ## Problem "Where the pieces are" is currently modeled by **four overlapping things**, all in `packages/engine/src/`: - `Position.board: Board` — the placement field of an immutable snapshot - `Board` — immutable class wrapping `sets: Record<PieceSetKey, SquareSet>`, a 12-slot record (one bitset per color+type) - `PieceSets` — that raw 12-slot record type, **exported**, used *structurally* by `attacks.ts` and `search-board.ts` - `SearchBoard` — mutable; holds **both** `#sets: PieceSets` (raw record it mutates in place) **and** `#board: Board` (a `Board.wrapSets(#sets)` aliasing the same record) `SquareSet` itself is a deep module. The shallowness is the *cluster around it* — four near-synonymous types co-owning one concept, with leaky seams between them: - **Three constructors that differ only by aliasing.** `Board.wrapSets` aliases the ref, `Board.fromSets` shallow-copies, `Board.toSets` copies out. The choice is invisible at the call site — a copy-vs-alias slip silently corrupts a `Game`'s history. - **`isAttackedIn` bypasses `Board`.** It takes raw `{ sets: PieceSets; occupancy: SquareSet }`. Its two callers feed it differently: `rules.isCheck` passes a fresh `position.board.toSets()` copy, `SearchBoard.legalMoves` passes the live `#sets` ref. - **The 12-slot layout is public.** `PieceSets` and `pieceSetKeyOf` are exported; the internal shape leaks across `board.ts → attacks.ts → search-board.ts`. - **`occupancy()` is an O(12) reduce** recomputed on every call, including inside the make/unmake hot loop. Understanding placement means bouncing between four files; the integration risk lives entirely in the aliasing seams between them. ## Proposed Interface A single **read port** every rule module depends on, with two implementations behind it — an immutable snapshot and a mutable search representation. (Ports & adapters pattern; the "port" here is internal — it decouples rule logic from placement representation.) ```ts // piece-placement.ts — none of this is exported from the package (ADR 0001) /** A square whose occupant changes across a move. */ export type PieceEdit = { readonly square: Square; readonly before: Piece | undefined; readonly after: Piece | undefined; }; /** * The read-only port for piece placement. Every rule module depends ONLY on * this — never on the 12-slot record. A frozen snapshot and a live mutating * board are indistinguishable to a consumer. Promises correctness, NOT a * complexity class for occupancy() — that preserves the representation-swap * escape hatch (ADR 0007's Uint32Array option). */ export interface PieceReader { pieceAt(square: Square): Piece | undefined; squaresOf(color: Color, type: PieceType): SquareSet; occupancy(): SquareSet; occupancyOf(color: Color): SquareSet; } /** Immutable snapshot. Keeps the name `Board`; implements the port. */ export class Board implements PieceReader { static empty(): Board; static fromPieces(pieces: Iterable<{ square: Square; piece: Piece }>): Board; static fromReader(reader: PieceReader): Board; // the ONLY snapshot-from-anything builder // ...PieceReader read methods; occupancy memoized at construction. NO mutators. } /** Mutable placement for the search hot path. Owned by SearchBoard, never leaked. */ export class MutablePlacement implements PieceReader { static fromReader(reader: PieceReader): MutablePlacement; // always copies in // ...PieceReader read methods; occupancy maintained incrementally. applyEdits(edits: readonly PieceEdit[]): void; // the ONLY mutation primitive } ``` Consumer signatures lose the raw record entirely: ```ts // attacks.ts — was ({ sets: PieceSets; occupancy: SquareSet }) isAttackedIn(target: { square: Square; byColor: Color }, pieces: PieceReader): boolean; // movegen.ts / zobrist.ts — `board: Board` becomes `board: PieceReader` ``` **Usage.** Immutable callers receive a `PieceReader` and never know the implementation: ```ts // rules.isCheck — no toSets() copy, no hand-rolled occupancy return isAttackedIn({ square: kingSq, byColor: opponentOf(turn) }, position.board); ``` `SearchBoard` holds **one** `MutablePlacement` (replacing both `#sets` and `#board`) and passes it as the port to every rule module: ```ts class SearchBoard { readonly #pieces: MutablePlacement; // makeMove: this.#pieces.applyEdits(edits); // unmakeMove: this.#pieces.applyEdits(undo.inverse); // legalMoves: isAttackedIn({ square: kingSq, byColor: enemy }, this.#pieces); // snapshotBoard(): Board { return Board.fromReader(this.#pieces); } } ``` **What it hides:** the 12-slot `Record<PieceSetKey, SquareSet>` layout (`PieceSetKey`, `PIECE_SET_KEYS`, `PIECE_FOR_KEY`, `pieceSetKeyOf` all become module-private); occupancy derivation and its cost; the aliasing/copying distinction (one snapshot builder, always copies — silent aliasing becomes unrepresentable). It also folds the three identical edit shapes (`ApplyEdit` in `search-board.ts`, `ZobristEdit` in `zobrist.ts`, `SquareEdit` in `board.ts`) into one `PieceEdit`. This is a hybrid of four explored designs: a ports-&-adapters skeleton (keeps `Board`'s name and identity → minimal churn, ADR-faithful), a deliberately minimal 4-method read surface (no speculative `squares()`/`pieces()` iteration — add only when a caller needs it), and a correctness-only `occupancy()` contract. ## Dependency Strategy **In-process** — pure computation, no I/O. The module is merged directly; no fakes, no adapters for external systems. The "port" is internal: one contract (`PieceReader`), two adapters (`Board` immutable, `MutablePlacement` mutable). `isAttackedIn`, `movegen`, and `zobrist` are rewritten to depend on `PieceReader` alone, so the same move-generation code runs over a frozen snapshot (cold path) and a live mid-mutation board (hot path) with byte-identical behavior. The immutable/mutable duality is handled by **interface segregation, not a mode flag**: `applyEdits` lives only on `MutablePlacement`. The immutable `Board` cannot be mutated because the method is not on its type — the type-safe replacement for ADR 0007's rejected "`@internal` mutation method on `Board`". Crossing the boundary always copies: `MutablePlacement.fromReader` (cold, once per `SearchBoard` construction) and `Board.fromReader` (cold, once per played ply). The hot make/unmake loop never crosses. ## Testing Strategy - **New boundary tests to write:** - A shared, table-driven `PieceReader` conformance suite run against **both** `Board` and `MutablePlacement` — `pieceAt`, `squaresOf`, `occupancy`, `occupancyOf` over a range of positions. - `MutablePlacement` occupancy invariant: after any sequence of `applyEdits` (and their inverses), `occupancy()` equals the recomputed union of all piece sets, and an apply-then-unapply round-trip restores the exact prior state. - `Board.fromReader` / `MutablePlacement.fromReader` independence: mutating the source after a crossing never affects the copy. - `isAttackedIn` tested directly against a `PieceReader` (no `{ sets, occupancy }` assembly). - **Old tests to delete / fold in:** the `Board`-internal cases in `board.test.ts` that assert on `wrapSets`/`fromSets`/`toSets` aliasing semantics — those mechanics no longer exist. Placement-shape assertions in `attacks.test.ts` migrate to the conformance suite. - **Test environment needs:** none beyond Vitest. **Perft is the correctness guard** — node counts in `perft.test.ts` must be byte-identical at every step of the refactor (the refactor is behavior-preserving). ## Implementation Recommendations Durable guidance, not coupled to today's file paths: - **What the module owns:** the canonical representation of piece placement — the per-(color,type) bitsets, occupancy derivation (memoized for the immutable impl, incremental for the mutable one), the single mutation primitive (`applyEdits`), and the immutable↔mutable crossings (always copying). - **What it hides:** the 12-slot record and its keys; whether occupancy is recomputed, memoized, or maintained incrementally; the backing representation itself (so a future raw-word-array rewrite, ADR 0007's gated option, drops in behind the same port with zero consumer change). - **What it exposes:** a 4-method read port (`pieceAt`, `squaresOf`, `occupancy`, `occupancyOf`); an immutable builder family (`empty`, `fromPieces`, `fromReader`); a mutable entry (`fromReader` + `applyEdits`). The port promises *correctness*, never a complexity class. Resist adding iteration helpers until a concrete caller needs one. - **How callers migrate:** every signature currently typed `Board` or `{ sets, occupancy }` is widened to `PieceReader`; bodies are unchanged. `SearchBoard` drops its `#sets`/`#board` pair for one `MutablePlacement`. `pieceSetKeyOf` and the `PieceSets`/`PieceSetKey` types stop being exported. It is one coordinated, mechanical change — leaving `Board` alongside a new type would preserve the four-overlapping-things friction, so the migration must land atomically. - **ADR alignment:** ADR 0001 (nothing exported; `Game` untouched), ADR 0007 (`Board` gains no mutation method; `SquareSet` instances stay immutable — `MutablePlacement` mutates its slot references and occupancy fields, never a `SquareSet`; `snapshotBoard` still pays one copy crossing back). Worth a short note in ADR 0007's lineage, or a new ADR, recording that `PieceReader` is now the seam between rule logic and placement representation.

root commented

2026-05-22 14:19:59 +00:00

(Migrated from gitlab.core-project.space)

Imported from a GitHub comment by @amaurycoudr on 2026-05-22 14:19 UTC.

Promoted to branch refactor/introduce-the-piecereader-read-port-118 (PR #122), not yet merged to main. A dependent slice should git fetch origin refactor/introduce-the-piecereader-read-port-118 and branch off it.

> _Imported from a [GitHub comment](https://github.com/amaurycoudr/chess-project/issues/117#issuecomment-4519525201) by [@amaurycoudr](https://github.com/amaurycoudr) on 2026-05-22 14:19 UTC._  Promoted to branch `refactor/introduce-the-piecereader-read-port-118` (PR #122), not yet merged to main. A dependent slice should `git fetch origin refactor/introduce-the-piecereader-read-port-118` and branch off it.

root (Migrated from gitlab.core-project.space) closed this issue

2026-05-23 06:28:34 +00:00

root commented

2026-10-13 08:38:43 +00:00

(Migrated from gitlab.core-project.space)

mentioned in issue #118