From f71512209fde17278170ed9a9541f956ca202065 Mon Sep 17 00:00:00 2001 From: Arvind Rajasekaran Date: Thu, 30 Jul 2026 12:10:37 +0200 Subject: [PATCH] Arena: fix move-parsing bias and record parse status per round MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The inline move parser in runArenaMatch had three silent failure modes that corrupt cooperation metrics: 1. Order bias: includes(move1) was checked before includes(move2), so 'I will not COOPERATE. DEFECT.' scored as COOPERATE. 2. Substring hits: a stag-hunt response mentioning 'hostage' (hoSTAGe) scored as STAG; 'our cooperation broke down' scored as COOPERATE. 3. Silent default: unparseable responses were scored as move2 (defect) with no record that parsing failed. extractMove now lives in server/arenaMoves.ts: it honors the RESPONSE FORMAT first line when compliant, falls back to whole-word label matching, and returns a parseOk flag instead of guessing when a response is ambiguous or empty. The historical default (move2) on parse failure is preserved, but ArenaRound now records player{1,2}ParseOk so affected rounds can be filtered — same approach as the contribution-evaluation parseOk flag from #31. 13 new tests in server/arenaMoves.test.ts, including regressions for each of the three failure modes. Relates to #8. Co-Authored-By: Claude Fable 5 --- server/arenaMoves.test.ts | 85 +++++++++++++++++++++++++++++++++++++++ server/arenaMoves.ts | 58 ++++++++++++++++++++++++++ server/routes.ts | 23 ++++++----- shared/schema.ts | 5 +++ 4 files changed, 160 insertions(+), 11 deletions(-) create mode 100644 server/arenaMoves.test.ts create mode 100644 server/arenaMoves.ts diff --git a/server/arenaMoves.test.ts b/server/arenaMoves.test.ts new file mode 100644 index 0000000..851b5be --- /dev/null +++ b/server/arenaMoves.test.ts @@ -0,0 +1,85 @@ +import { describe, it, expect } from "vitest"; +import { extractMove } from "./arenaMoves"; + +const PD: [string, string] = ["COOPERATE", "DEFECT"]; +const STAG_HUNT: [string, string] = ["STAG", "RABBIT"]; +const APPLE: [string, string] = ["WORK", "STEAL"]; + +describe("extractMove", () => { + describe("format-compliant responses (label on first line)", () => { + it("parses a bare label", () => { + expect(extractMove("DEFECT", PD)).toEqual({ move: "DEFECT", parseOk: true }); + expect(extractMove("COOPERATE", PD)).toEqual({ move: "COOPERATE", parseOk: true }); + }); + + it("parses label followed by reasoning on later lines", () => { + const r = "DEFECT\nIf I cooperate now I lose the endgame."; + expect(extractMove(r, PD)).toEqual({ move: "DEFECT", parseOk: true }); + }); + + it("reasoning mentioning the other label does not override the first line", () => { + const r = "COOPERATE\nDefect would score more but destroys trust; cooperate compounds."; + expect(extractMove(r, PD)).toEqual({ move: "COOPERATE", parseOk: true }); + }); + + it("is case-insensitive", () => { + expect(extractMove("cooperate", PD)).toEqual({ move: "COOPERATE", parseOk: true }); + }); + + it("tolerates markdown/punctuation around the label", () => { + expect(extractMove("**DEFECT**", PD)).toEqual({ move: "DEFECT", parseOk: true }); + expect(extractMove("- STAG: joint hunt maximizes payoff", STAG_HUNT)) + .toEqual({ move: "STAG", parseOk: true }); + }); + + it("skips leading blank lines", () => { + expect(extractMove("\n\n RABBIT\nSafe choice.", STAG_HUNT)) + .toEqual({ move: "RABBIT", parseOk: true }); + }); + }); + + describe("format violations that are still unambiguous", () => { + it("finds a single label mentioned mid-sentence", () => { + const r = "After weighing the history, I choose to STEAL this round."; + expect(extractMove(r, APPLE)).toEqual({ move: "STEAL", parseOk: true }); + }); + }); + + describe("regressions the previous substring parser got wrong", () => { + it("does not score 'hostage' as a STAG move", () => { + const r = "This feels like the hostage situation from round 2. RABBIT."; + // old parser: upper.includes("STAG") matched hoSTAGe first -> STAG (wrong) + expect(extractMove(r, STAG_HUNT)).toEqual({ move: "RABBIT", parseOk: true }); + }); + + it("does not score 'cooperation' (the word) as a COOPERATE move", () => { + const r = "Our cooperation has broken down completely. DEFECT."; + // old parser: includes("COOPERATE") matched COOPERATIon first -> COOPERATE (wrong) + expect(extractMove(r, PD)).toEqual({ move: "DEFECT", parseOk: true }); + }); + + it("does not resolve negated-then-chosen responses by label order", () => { + const r = "I will not COOPERATE this time. DEFECT."; + // old parser: includes(move1) checked first -> COOPERATE (wrong). + // Both labels appear as words with no compliant first line, so this is + // ambiguous: flag it rather than guess by order. + expect(extractMove(r, PD)).toEqual({ move: null, parseOk: false }); + }); + }); + + describe("unparseable responses", () => { + it("flags a response with no move label", () => { + expect(extractMove("I refuse to play this game.", PD)) + .toEqual({ move: null, parseOk: false }); + }); + + it("flags an empty response", () => { + expect(extractMove("", PD)).toEqual({ move: null, parseOk: false }); + }); + + it("flags a response naming both labels with no compliant first line", () => { + const r = "Torn between STAG and RABBIT here, honestly."; + expect(extractMove(r, STAG_HUNT)).toEqual({ move: null, parseOk: false }); + }); + }); +}); diff --git a/server/arenaMoves.ts b/server/arenaMoves.ts new file mode 100644 index 0000000..a4f1399 --- /dev/null +++ b/server/arenaMoves.ts @@ -0,0 +1,58 @@ +// Move parsing for arena matches (prisoner's dilemma, stag hunt, apple tree). +// +// The previous inline parser had three failure modes that silently corrupted +// cooperation metrics (see issue #8 discussion): +// 1. Order bias: `includes(move1)` was checked before `includes(move2)`, so +// "I will not COOPERATE. DEFECT." was scored as COOPERATE. +// 2. Substring hits: plain `includes` matches inside words, so a stag-hunt +// response mentioning "hostage" (hoSTAGe) was scored as STAG, and +// "full cooperation" as COOPERATE. +// 3. Silent default: an unparseable response was scored as move2 (defect) +// with no record that parsing failed. +// +// This parser is layered and reports parse status alongside the move, in the +// same spirit as the contribution-evaluation `parseOk` flag (PR #31): +// a. If the first non-empty line starts with exactly one move label +// (the RESPONSE FORMAT the system prompt demands), use it. +// b. Otherwise look for whole-word occurrences of the labels anywhere in +// the response; if exactly one distinct label appears, use it. +// c. Otherwise the move is ambiguous or absent: return null and let the +// caller apply its default — but with parseOk=false recorded, so runs +// can be filtered or re-examined instead of quietly counting a guess. + +export interface ParsedMove { + move: string | null; + parseOk: boolean; +} + +function escapeRegExp(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +export function extractMove(response: string, moves: [string, string]): ParsedMove { + const upper = response.toUpperCase(); + const [move1, move2] = moves.map(m => m.toUpperCase()) as [string, string]; + + // (a) Format-compliant: first non-empty line starts with a move label. + const firstLine = upper + .split("\n") + .map(line => line.trim()) + .find(line => line.length > 0) ?? ""; + // Strip common lead-in punctuation/markdown around the label ("**DEFECT**", "- STAG:"). + const strippedFirstLine = firstLine.replace(/^[^A-Z]*/, ""); + const startsWith1 = strippedFirstLine.startsWith(move1); + const startsWith2 = strippedFirstLine.startsWith(move2); + if (startsWith1 !== startsWith2) { + return { move: startsWith1 ? moves[0] : moves[1], parseOk: true }; + } + + // (b) Whole-word occurrences anywhere in the response. + const has1 = new RegExp(`\\b${escapeRegExp(move1)}\\b`).test(upper); + const has2 = new RegExp(`\\b${escapeRegExp(move2)}\\b`).test(upper); + if (has1 !== has2) { + return { move: has1 ? moves[0] : moves[1], parseOk: true }; + } + + // (c) Both labels present (ambiguous) or neither (absent). + return { move: null, parseOk: false }; +} diff --git a/server/routes.ts b/server/routes.ts index f2a9208..bc13e8e 100644 --- a/server/routes.ts +++ b/server/routes.ts @@ -1,6 +1,7 @@ import type { Express, Response as ExpressResponse } from "express"; import { createServer, type Server } from "http"; import { storage, availableChatbots } from "./storage"; +import { extractMove } from "./arenaMoves"; import { insertSessionSchema, insertRunSchema, insertArenaMatchSchema, insertWargameSchema, insertToolkitItemSchema, insertBenchmarkProposalSchema, insertConstructSchema, insertPhysioBatchSchema, insertNewsletterSubscriberSchema, insertStoryRecipientSchema, insertAcademicContributorSchema, academicSubmissionSchema, type ArenaRound, type WargameTurn, type AICallResult, type TokenUsage, type NewsletterSubscriber } from "@shared/schema"; import OpenAI from "openai"; import Anthropic from "@anthropic-ai/sdk"; @@ -3414,15 +3415,8 @@ You MUST respond with exactly one of these labels: ${move1Label} or ${move2Label You may optionally add brief reasoning after your move on a new line.`; } - // Extract move from response - function extractMove(response: string): string | null { - const upperResponse = response.toUpperCase().trim(); - if (upperResponse.startsWith(move1Label)) return move1Label; - if (upperResponse.startsWith(move2Label)) return move2Label; - if (upperResponse.includes(move1Label)) return move1Label; - if (upperResponse.includes(move2Label)) return move2Label; - return null; - } + // Extract move from response — see server/arenaMoves.ts for the parsing + // rules and the failure modes the old inline parser had (issue #8). // Calculate payoff function calculatePayoff(p1Move: string, p2Move: string): [number, number] { @@ -3471,8 +3465,13 @@ You may optionally add brief reasoning after your move on a new line.`; const p1Response = p1Timed.content; const p2Response = p2Timed.content; - const p1Move = extractMove(p1Response) || move2Label; - const p2Move = extractMove(p2Response) || move2Label; + // Unparseable/ambiguous responses keep the historical default (move2, + // the defect-equivalent) so existing run semantics don't change — but + // the failure is now recorded per player instead of silently scored. + const p1Parsed = extractMove(p1Response, gameConfig.moves); + const p2Parsed = extractMove(p2Response, gameConfig.moves); + const p1Move = p1Parsed.move ?? move2Label; + const p2Move = p2Parsed.move ?? move2Label; const [p1Points, p2Points] = calculatePayoff(p1Move, p2Move); p1TotalScore += p1Points; @@ -3497,6 +3496,8 @@ You may optionally add brief reasoning after your move on a new line.`; player2Reasoning: p2Response.split("\n").slice(1).join("\n").trim() || undefined, player1LatencyMs: p1LatencyMs, player2LatencyMs: p2LatencyMs, + player1ParseOk: p1Parsed.parseOk, + player2ParseOk: p2Parsed.parseOk, }; rounds.push(roundData); diff --git a/shared/schema.ts b/shared/schema.ts index 2f2c5f2..f99acf0 100644 --- a/shared/schema.ts +++ b/shared/schema.ts @@ -117,6 +117,11 @@ export interface ArenaRound { player2Reasoning?: string; player1LatencyMs: number; player2LatencyMs: number; + // False when the model's response contained no unambiguous move label and + // the engine fell back to the default move (defect-equivalent). Optional so + // rounds recorded before this field existed stay valid. See issue #8. + player1ParseOk?: boolean; + player2ParseOk?: boolean; } // Arena match interface