refactor(perft): code-review polish for the perft rig + MCP server

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<DivergenceLevel>) {
 
-    /** 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<DivergenceLevel>) {
         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 {

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:") -> {

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 |

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"

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 <fen>` + `go perft <depth>`, returns {moves: Map<uci, count>, 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 ---")

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:") -> {