From 1bf55fef39811949f2a7f9cd32733b36c4491dd4 Mon Sep 17 00:00:00 2001
From: Ken Riley <kenr@nodots.com>
Date: Tue, 25 Aug 2026 18:04:11 -0600
Subject: [PATCH] fix(pr): pin analysis engine config per calculateGamePR run
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The gnubg engine config is a mutable process-global shared with robot
turn execution. Once robots configure per-skill settings, PR analysis
inherited whatever the last robot turn left behind — including the case
where the robot's own move-selection settings graded the robot's moves,
yielding equity loss 0 and PR 0.0 for every robot.

calculateGamePR now re-asserts a complete ANALYSIS_HINTS_CONFIG
(2-ply, Huge filter, noise 0, pruning) at the start of every run via an
optional configure hook on AiModuleInterface.

Claude-Session: https://claude.ai/code/session_01JUcWgZDmavXuK6Xkeejd5W
---
 src/Services/PerformanceRatingCalculator.ts   | 29 ++++++++++++++++
 .../PerformanceRatingCalculator.test.ts       | 33 ++++++++++++++++++-
 2 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/src/Services/PerformanceRatingCalculator.ts b/src/Services/PerformanceRatingCalculator.ts
index c5c9ccb..4833b9a 100644
--- a/src/Services/PerformanceRatingCalculator.ts
+++ b/src/Services/PerformanceRatingCalculator.ts
@@ -8,6 +8,14 @@ export interface AiModuleInterface {
   gnubgHints: {
     isAvailable: () => Promise<boolean>
     getBuildInstructions: () => string
+    // The engine config is a mutable process-global shared with robot turn
+    // execution; analysis must be able to re-assert its own config per run.
+    configure?: (config: {
+      evalPlies?: number
+      moveFilter?: number
+      usePruning?: boolean
+      noise?: number
+    }) => void | Promise<void>
     getMoveHints: (request: any, maxHints?: number) => Promise<MoveHint[]>
     // Prefer positionId-based hints when available to match robot path
     getHintsFromPositionId?: (
@@ -23,6 +31,18 @@ export interface AiModuleInterface {
   }
 }
 
+// Engine settings PR analysis runs at, independent of any robot's skill
+// config. 2-ply, deterministic, with the widest move filter (numeric value of
+// MoveFilterSetting.Huge — not imported to keep @nodots/gnubg-hints a
+// type-only dependency). Matches what the api's pr routes have historically
+// configured.
+export const ANALYSIS_HINTS_CONFIG = {
+  evalPlies: 2,
+  moveFilter: 4,
+  usePruning: true,
+  noise: 0,
+} as const
+
 // Lazy-loaded ai module reference
 let aiModule: AiModuleInterface | null = null
 
@@ -145,6 +165,15 @@ export class PerformanceRatingCalculator {
         )
       }
 
+      // Re-assert the analysis config for every run. The engine config is a
+      // process-global that robot turns overwrite with per-robot skill
+      // settings; without this, a game against "GNU Beginner" would be
+      // analyzed at the robot's weakened settings (or the robot's play
+      // analyzed by the exact engine call that chose it, yielding PR 0.0).
+      // The config must be complete — partial configs merge into whatever
+      // the previous caller left behind.
+      await ai.gnubgHints.configure?.(ANALYSIS_HINTS_CONFIG)
+
       const playerStats: Record<string, PlayerPR> = {}
 
       // Group consecutive actions into full-turn sequences by (player, dice)
diff --git a/src/Services/__tests__/PerformanceRatingCalculator.test.ts b/src/Services/__tests__/PerformanceRatingCalculator.test.ts
index 150b4c0..031cf25 100644
--- a/src/Services/__tests__/PerformanceRatingCalculator.test.ts
+++ b/src/Services/__tests__/PerformanceRatingCalculator.test.ts
@@ -6,7 +6,7 @@
  */
 
 import { describe, expect, test, jest, beforeEach, afterEach } from '@jest/globals'
-import { PerformanceRatingCalculator, setAiModuleForTesting, AiModuleInterface } from '../PerformanceRatingCalculator'
+import { PerformanceRatingCalculator, setAiModuleForTesting, AiModuleInterface, ANALYSIS_HINTS_CONFIG } from '../PerformanceRatingCalculator'
 
 // Create mock AI module for testing
 const createMockAiModule = (overrides: Partial<AiModuleInterface> = {}): AiModuleInterface => ({
@@ -226,6 +226,37 @@ describe('PerformanceRatingCalculator', () => {
       expect(result.playerResults['player1']?.totalMoves).toBe(2)
       expect(result.playerResults['player2']?.totalMoves).toBe(1)
     })
+
+    test('re-asserts the full analysis config on every run', async () => {
+      // Regression: the engine config is a process-global that robot turns
+      // overwrite with per-robot skill settings. Analysis must pin its own
+      // complete config each run or it inherits the robot's.
+      const configureMock = jest
+        .fn<(config: Record<string, unknown>) => Promise<void>>()
+        .mockResolvedValue(undefined)
+      const moduleWithConfigure = createMockAiModule()
+      moduleWithConfigure.gnubgHints.configure = configureMock
+      setAiModuleForTesting(moduleWithConfigure)
+
+      await calculator.calculateGamePR('test-game', [], [])
+      await calculator.calculateGamePR('test-game', [], [])
+
+      expect(configureMock).toHaveBeenCalledTimes(2)
+      expect(configureMock).toHaveBeenCalledWith(ANALYSIS_HINTS_CONFIG)
+      // The config must be complete: partial configs merge into whatever the
+      // previous engine caller left behind.
+      expect(ANALYSIS_HINTS_CONFIG).toEqual({
+        evalPlies: 2,
+        moveFilter: 4,
+        usePruning: true,
+        noise: 0,
+      })
+    })
+
+    test('tolerates an ai module without configure', async () => {
+      const result = await calculator.calculateGamePR('test-game', [], [])
+      expect(result.analysisComplete).toBe(true)
+    })
   })
 })