From 3f614d4d2fa5d8bbeddda6f59a628f9b1bbc091d Mon Sep 17 00:00:00 2001 From: Gabor Berenyi <205867+ber4444@users.noreply.github.com> Date: Tue, 14 Jul 2026 17:57:33 -0700 Subject: [PATCH 1/2] refactor(perft): apply code-review polish to the perft rig + MCP server Six review findings from the merged perft/MCP work, no behavior change to the green gate: 1. Fix stale path in the divergence report an agent reads: point at chess-core/src/.../Move.kt, not the pre-move app/src path. 2. Split DivergenceReport.render() (now pure) from a new persist() so producing the report text has no hidden filesystem side effect; call sites persist then fail explicitly instead of writing inside an assert message. 3. Distinguish a Stockfish timeout from a real divergence: the rig (StockfishPerft) now throws instead of returning a partial divide, and the MCP tool (StockfishDivider) reports a new DivideResult.timedOut and withholds the partial map so a slow engine can't masquerade as a mismatch. 4. Bound the run_perft_gate subprocess (15m default / 45m deep) with a background stdout drainer, so a wedged gradle can't hang the agent forever. 5. De-duplicate the depth cap / timeout: tool descriptions now interpolate StockfishDivider.MAX_DEPTH / DEFAULT_TIMEOUT_MS instead of a separate constant that could drift from the real clamp. 6. Remove unused imports (kotlin.io.path.pathString, java.io.File). Verified: :perft-mcp:test green, :chess-core:desktopTest --tests "*Perft*" green with Stockfish present (Oracle 2 ran). Co-Authored-By: Claude Opus 4.8 --- .../perft/PerftVsStockfishTest.kt | 38 ++++++++++-------- .../myapplication/perft/StockfishPerft.kt | 9 ++++- .../perft/mcp/PerftGateRunner.kt | 39 +++++++++++++++---- .../perft/mcp/PerftMcpServerMain.kt | 11 ++++-- .../perft/mcp/StockfishDivider.kt | 12 +++++- 5 files changed, 79 insertions(+), 30 deletions(-) diff --git a/chess-core/src/desktopTest/kotlin/com/example/myapplication/perft/PerftVsStockfishTest.kt b/chess-core/src/desktopTest/kotlin/com/example/myapplication/perft/PerftVsStockfishTest.kt index ead24b5c..8ab75140 100644 --- a/chess-core/src/desktopTest/kotlin/com/example/myapplication/perft/PerftVsStockfishTest.kt +++ b/chess-core/src/desktopTest/kotlin/com/example/myapplication/perft/PerftVsStockfishTest.kt @@ -6,11 +6,10 @@ import com.example.myapplication.PromotionType import com.example.myapplication.Set import com.example.myapplication.UciMoveConverter import com.example.myapplication.applyMove -import java.io.File import kotlin.random.Random import kotlin.test.Test -import kotlin.test.assertNull import kotlin.test.assertTrue +import kotlin.test.fail import org.junit.Assume /** @@ -34,10 +33,11 @@ class PerftVsStockfishTest { val state = FenConverter.fenToGameState(position.fen) val depth = canonicalDivideDepth(position) val report = localizeDivergence(state, depth, sf) - assertNull( - report, - "Position ${position.name} diverged at depth $depth.\n${report?.render(state, depth)}", - ) + if (report != null) { + val rendered = report.render(state, depth) + report.persist(rendered) + fail("Position ${position.name} diverged at depth $depth.\n$rendered") + } } } } @@ -85,11 +85,11 @@ class PerftVsStockfishTest { // Diff every K accepted steps at a shallow depth that keeps Stockfish fast. if (accepted % DIFF_EVERY_N_ACCEPTED == 0) { val report = localizeDivergence(state, DIFF_DEPTH, sf) - assertNull( - report, - "Walk diverged at step=$step (accepted=$accepted, seed=$seed) at depth $DIFF_DEPTH.\n" + - report?.render(state, DIFF_DEPTH), - ) + if (report != null) { + val rendered = report.render(state, DIFF_DEPTH) + report.persist(rendered) + fail("Walk diverged at step=$step (accepted=$accepted, seed=$seed) at depth $DIFF_DEPTH.\n$rendered") + } } } assertTrue(accepted > 0, "Random walk accepted zero moves (seed=$seed) — generator is broken") @@ -193,7 +193,7 @@ private data class DivergenceLevel( /** A full trail of [DivergenceLevel]s from the original FEN down to the deepest mismatch. */ private class DivergenceReport(val trail: List) { - /** Multi-line, agent-friendly report. Persisted to `build/perft-divergence.txt` on failure. */ + /** Multi-line, agent-friendly report. Pure — call [persist] to write it to disk. */ fun render(rootState: GameUiState, rootDepth: Int): String { val sb = StringBuilder() sb.appendLine("Perft divergence detected at depth $rootDepth:") @@ -216,14 +216,20 @@ private class DivergenceReport(val trail: List) { sb.appendLine() sb.appendLine("Next step: read the deepest [ply N] above — its FEN is the position where the app's") sb.appendLine("generator produces a different subtree count than Stockfish for the listed move(s).") - sb.appendLine("Inspect the corresponding rule in app/src/commonMain/.../Move.kt or applyMove.") + sb.appendLine("Inspect the corresponding rule in chess-core/src/commonMain/.../Move.kt or applyMove.") + return sb.toString() + } - // Persist so the autonomous loop can pick it up via the filesystem. + /** + * Write a [render]ed report to `build/perft-divergence.txt` so the autonomous loop (and the + * `read_divergence` MCP tool) can pick it up. Kept separate from [render] so producing the text + * has no filesystem side effect. Best-effort: an IO failure here must not mask the divergence. + */ + fun persist(rendered: String) { runCatching { val out = projectRoot().resolve("build").apply { toFile().mkdirs() } - out.resolve("perft-divergence.txt").toFile().writeText(sb.toString()) + out.resolve("perft-divergence.txt").toFile().writeText(rendered) } - return sb.toString() } private companion object { diff --git a/chess-core/src/desktopTest/kotlin/com/example/myapplication/perft/StockfishPerft.kt b/chess-core/src/desktopTest/kotlin/com/example/myapplication/perft/StockfishPerft.kt index a8613c7e..e69b4780 100644 --- a/chess-core/src/desktopTest/kotlin/com/example/myapplication/perft/StockfishPerft.kt +++ b/chess-core/src/desktopTest/kotlin/com/example/myapplication/perft/StockfishPerft.kt @@ -43,7 +43,14 @@ class StockfishPerft( if (remaining <= 0) { throw IllegalStateException("Stockfish perft timed out after ${PERFT_TIMEOUT_MS}ms (fen=$fen, depth=$depth)") } - val line = readLineWithTimeout(remaining) ?: break + val line = readLineWithTimeout(remaining) + ?: throw IllegalStateException( + // A healthy `go perft` always ends with a "Nodes searched:" line, which breaks the + // loop explicitly below. A null here means timeout or early process exit — fail + // loudly rather than returning a partial divide that would masquerade as a + // divergence and send the localizer chasing a phantom (fen=$fen, depth=$depth). + "Stockfish perft ended without a 'Nodes searched:' summary (timeout or early exit)" + ) val trimmed = line.trim() when { trimmed.startsWith("Nodes searched:") -> { diff --git a/perft-mcp/src/main/kotlin/com/example/myapplication/perft/mcp/PerftGateRunner.kt b/perft-mcp/src/main/kotlin/com/example/myapplication/perft/mcp/PerftGateRunner.kt index 3998038e..d989f040 100644 --- a/perft-mcp/src/main/kotlin/com/example/myapplication/perft/mcp/PerftGateRunner.kt +++ b/perft-mcp/src/main/kotlin/com/example/myapplication/perft/mcp/PerftGateRunner.kt @@ -1,7 +1,8 @@ package com.example.myapplication.perft.mcp import java.nio.file.Path -import kotlin.io.path.pathString +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicReference /** * Result of a `run_perft_gate` tool call. @@ -34,6 +35,11 @@ object PerftGateRunner { private const val GATE_TEST_PATTERN = "*Perft*" private const val DEEP_FLAG = "-Dperft.deep=true" private const val SUMMARY_TAIL_LINES = 50 + // Bound the subprocess so a wedged gradle can't hang the MCP tool call (and thus the agent) + // indefinitely. The deep tier legitimately runs for minutes, so it gets a wider ceiling. + private const val DEFAULT_TIMEOUT_MINUTES = 15L + private const val DEEP_TIMEOUT_MINUTES = 45L + private const val DRAIN_JOIN_MS = 2_000L /** * @param deep when true, appends [DEEP_FLAG] so [PerftDeepTest] runs (nightly-tier depths; slow). @@ -47,19 +53,38 @@ object PerftGateRunner { val builder = ProcessBuilder(command).directory(root.toFile()).redirectErrorStream(true) val proc = builder.start() - val output = proc.inputStream.bufferedReader().readText() - val exitCode = proc.waitFor() - val tail = output.lineSequence().toList().let { lines -> - lines.takeLast(Math.min(SUMMARY_TAIL_LINES, lines.size)).joinToString("\n").trim() + // Drain stdout on a daemon thread: this both keeps a chatty gradle from filling the pipe + // buffer and wedging, and lets us bound the whole run with waitFor(timeout) instead of + // blocking forever inside readText() when the build hangs. + val output = AtomicReference("") + val drainer = Thread { output.set(proc.inputStream.bufferedReader().readText()) } + .apply { isDaemon = true; name = "perft-gate-stdout"; start() } + + val timeoutMinutes = if (deep) DEEP_TIMEOUT_MINUTES else DEFAULT_TIMEOUT_MINUTES + if (!proc.waitFor(timeoutMinutes, TimeUnit.MINUTES)) { + proc.destroyForcibly() + drainer.join(DRAIN_JOIN_MS) + return GateResult( + passed = false, + summary = "perft gate TIMED OUT after ${timeoutMinutes}m and was killed.\n" + tailOf(output.get()), + divergenceFileExists = PerftMcpPaths.divergenceExists(root), + ) } + drainer.join(DRAIN_JOIN_MS) return GateResult( - passed = exitCode == 0, - summary = tail, + passed = proc.exitValue() == 0, + summary = tailOf(output.get()), divergenceFileExists = PerftMcpPaths.divergenceExists(root), ) } + /** Last [SUMMARY_TAIL_LINES] lines of [output], trimmed — enough to see which test failed. */ + private fun tailOf(output: String): String = + output.lineSequence().toList().let { lines -> + lines.takeLast(minOf(SUMMARY_TAIL_LINES, lines.size)).joinToString("\n").trim() + } + /** `./gradlew` on unix, `gradlew.bat` on windows. The MCP host's platform decides. */ private fun gradleWrapper(): String = if (System.getProperty("os.name").startsWith("Windows")) "gradlew.bat" else "gradlew" diff --git a/perft-mcp/src/main/kotlin/com/example/myapplication/perft/mcp/PerftMcpServerMain.kt b/perft-mcp/src/main/kotlin/com/example/myapplication/perft/mcp/PerftMcpServerMain.kt index 0d8c5059..7238b6b3 100644 --- a/perft-mcp/src/main/kotlin/com/example/myapplication/perft/mcp/PerftMcpServerMain.kt +++ b/perft-mcp/src/main/kotlin/com/example/myapplication/perft/mcp/PerftMcpServerMain.kt @@ -107,7 +107,7 @@ class PerftMcpServer { } putJsonObject("depth") { put("type", "integer") - put("description", "Perft depth (clamped to $MAX_DEPTH_CAP).") + put("description", "Perft depth (clamped to ${StockfishDivider.MAX_DEPTH}).") put("default", 3) } }, @@ -144,8 +144,6 @@ class PerftMcpServer { const val TOOL_SF_DIVIDE = "stockfish_divide" const val TOOL_READ_DIV = "read_divergence" - const val MAX_DEPTH_CAP = 6 - // --- Tool descriptions (embed the hard rules verbatim) ------------------- private val ORACLE_RULE = """ @@ -182,7 +180,7 @@ class PerftMcpServer { val SF_DIVIDE_DESCRIPTION = """ Ask Stockfish for the per-move perft divide breakdown of a FEN: spawns `stockfish`, sends `position fen ` + `go perft `, returns {moves: Map, total, oracleUnavailable}. - Depth is clamped to $MAX_DEPTH_CAP; 120s timeout. If no `stockfish` binary is on PATH this + Depth is clamped to ${StockfishDivider.MAX_DEPTH}; ${StockfishDivider.DEFAULT_TIMEOUT_MS / 1000}s timeout. If no `stockfish` binary is on PATH this returns oracleUnavailable=true rather than throwing — Stockfish is a soft dependency. Use this to cross-check the app's generator against an independent oracle for an arbitrary @@ -221,6 +219,11 @@ class PerftMcpServer { appendLine("Install stockfish (Homebrew: brew install stockfish; apt: sudo apt-get install -y stockfish) and retry.") return@buildString } + if (result.timedOut) { + appendLine("Stockfish divide did not complete within the timeout (fen=$fen depth=$depth).") + appendLine("Partial results were withheld to avoid a false divergence — retry at a lower depth.") + return@buildString + } appendLine("Stockfish divide: fen=$fen depth=$depth") appendLine("total (Nodes searched): ${result.total ?: "(missing)"}") appendLine("--- per-move subtree counts ---") diff --git a/perft-mcp/src/main/kotlin/com/example/myapplication/perft/mcp/StockfishDivider.kt b/perft-mcp/src/main/kotlin/com/example/myapplication/perft/mcp/StockfishDivider.kt index 87f7a523..cc26becd 100644 --- a/perft-mcp/src/main/kotlin/com/example/myapplication/perft/mcp/StockfishDivider.kt +++ b/perft-mcp/src/main/kotlin/com/example/myapplication/perft/mcp/StockfishDivider.kt @@ -18,6 +18,13 @@ data class DivideResult( val total: Long?, /** True iff no launchable `stockfish` binary was found. See class docs. */ val oracleUnavailable: Boolean, + /** + * True iff the divide did not finish within [StockfishDivider.timeoutMs] — the deadline elapsed + * or the process exited before emitting `Nodes searched:`. Distinct from [oracleUnavailable] so a + * slow engine is not mistaken for a missing one; the partial map is withheld ([moves] is empty) + * because an incomplete divide looks like — but isn't — a real divergence. + */ + val timedOut: Boolean = false, ) /** @@ -60,8 +67,9 @@ class StockfishDivider( val deadline = System.currentTimeMillis() + timeoutMs while (true) { val remaining = deadline - System.currentTimeMillis() - if (remaining <= 0) return DivideResult(moves, total, oracleUnavailable = false) - val line = readLineWithTimeout(remaining) ?: break + if (remaining <= 0) return DivideResult(emptyMap(), null, oracleUnavailable = false, timedOut = true) + val line = readLineWithTimeout(remaining) + ?: return DivideResult(emptyMap(), null, oracleUnavailable = false, timedOut = true) val trimmed = line.trim() when { trimmed.startsWith("Nodes searched:") -> { From dd1c3aeb49218192ca3143d62922b8259f03c107 Mon Sep 17 00:00:00 2001 From: Gabor Berenyi <205867+ber4444@users.noreply.github.com> Date: Wed, 15 Jul 2026 05:29:58 -0700 Subject: [PATCH 2/2] docs(perft): sync MCP tool table with the polish + add Future work - run_perft_gate / stockfish_divide rows now match the new contract (gate subprocess timeout; stockfish_divide's timedOut field + partial- result withholding). - Future work: note The Grand Chess Tree's ~28k-position corpus as an Oracle 1 breadth extension, and why a no-divide speed counter (perft_cpu_2026) is the wrong tool for the localizer. Co-Authored-By: Claude Opus 4.8 --- docs/perft.md | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/docs/perft.md b/docs/perft.md index ded57642..ba0cc325 100644 --- a/docs/perft.md +++ b/docs/perft.md @@ -121,8 +121,8 @@ a thin stdio MCP server so an agent calls `run_perft_gate` instead of reading pr | Tool | What it does | |------|-------------| -| `run_perft_gate(deep=false)` | Runs `./gradlew :chess-core:desktopTest --tests "*Perft*"`. Returns `{passed, summary, divergenceFileExists}`. | -| `stockfish_divide(fen, depth=3)` | Spawns `stockfish`, returns `{moves, total, oracleUnavailable}`. Depth clamped to 6; 120s timeout. Never throws — degrades to structured "unavailable". | +| `run_perft_gate(deep=false)` | Runs `./gradlew :chess-core:desktopTest --tests "*Perft*"` (bounded at 15 min, 45 min when `deep`, so a wedged build can't hang the agent). Returns `{passed, summary, divergenceFileExists}`. | +| `stockfish_divide(fen, depth=3)` | Spawns `stockfish`, returns `{moves, total, oracleUnavailable, timedOut}`. Depth clamped to 6; 120s timeout — a timeout sets `timedOut` and withholds the partial map so it can't masquerade as a divergence. Never throws — degrades to structured "unavailable". | | `read_divergence()` | Reads `build/perft-divergence.txt`, or a structured "no divergence recorded" message. | It's an **adapter, not an engine** — no dependency on `:app` or `:chess-core`; it shells out to @@ -152,6 +152,27 @@ The perft gate runs in CI on every PR and nightly: See [`docs/plans/perft-ci-completion.md`](plans/perft-ci-completion.md) for the full CI story, including the breakage the chess-core move caused and how it was fixed. +## Future work: a broader Oracle 1 corpus + +Oracle 1 is deliberately small — six hand-picked positions. The natural extension is to swap those +constants for a slice of a large, independently-verified perft corpus. +[The Grand Chess Tree](https://grandchesstree.com/) is a distributed effort that publishes a +machine-readable results set and a ~28k-position corpus, categorized into exactly the buckets that +break hand-written generators: **captures, castles, promotions, checkmates**. Ingesting a low-depth +slice as extra `PerftPositions` entries would widen the always-on net from "a handful of constants" +to a broad, edge-case-weighted one — and make cheating the oracle even more pointless, since a large +categorized corpus is far harder to fake than six numbers. + +This enriches **Oracle 1 only.** Localization still needs a live divide-capable engine, so Stockfish +stays as Oracle 2 (the corpus is data, not a `go perft` server). Trust is the one caveat: +chessprogramming.org's counts are long-standing consensus, so ingest at depths where the corpus +agrees with known values rather than adopting frontier-depth results wholesale. + +> A pure-speed counter like [`perft_cpu_2026`](https://github.com/ankan-ban/perft_cpu_2026) is a +> *different* tool: per its README it has no divide output, so it can't localize a divergence — it +> would only make Oracle 1's totals faster, which the bounded-depth gate doesn't need. That's why the +> rig cross-checks against Stockfish's `go perft` divide rather than a raw perft speed record. + ## File map | File | Source set | Purpose |