From 62826c8966bb352f9a314efa85641a21390d7e31 Mon Sep 17 00:00:00 2001 From: Agnik47 <140933190+Agnik47@users.noreply.github.com> Date: Sat, 15 Aug 2026 02:36:18 +0530 Subject: [PATCH] fix(benchmarks): make question order deterministic and load() idempotent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `LongMemEvalBenchmark.loadQuestions` took whatever order `readdirSync` returned. That order is unspecified — hash-ordered on ext4 with dir_index, roughly lexicographic on APFS/NTFS — and it decides which questions a run covers, because the orchestrator slices the front of the list for `--limit N`, for `sampling.mode === "limit"`, and for consecutive sampling. The same command therefore benchmarked a different subset on a maintainer's Mac than on CI, and the accuracy gap between the two was indistinguishable from a real provider difference. Sorting the file list makes a limited run cover the same questions everywhere. `load()` was also not idempotent: all three benchmarks pushed into `this.questions` (and LongMemEval into `this.data`) without clearing first, so calling it twice on one instance duplicated every question. `getQuestions()` would return each one twice, double-counting in the report and halving the effective coverage of `--limit`. The CLI creates a fresh instance per run, but the server caches benchmark instances in `routes/runs.ts`, so the footgun is reachable. Reset the accumulated state at the start of the load — for ConvoMem after the JSON parses, so a failed reload does not discard good data. Fixes #75 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Nwg8d2HEScsHTWpBgWewWX --- src/benchmarks/convomem/index.ts | 7 + src/benchmarks/locomo/index.ts | 6 + src/benchmarks/longmemeval/index.ts | 17 ++- src/benchmarks/question-order.test.ts | 198 ++++++++++++++++++++++++++ 4 files changed, 227 insertions(+), 1 deletion(-) create mode 100644 src/benchmarks/question-order.test.ts diff --git a/src/benchmarks/convomem/index.ts b/src/benchmarks/convomem/index.ts index 9c253fa..ab859bd 100644 --- a/src/benchmarks/convomem/index.ts +++ b/src/benchmarks/convomem/index.ts @@ -144,6 +144,13 @@ export class ConvoMemBenchmark implements Benchmark { const content = readFileSync(dataFile, "utf8") const items: { category: string; item: ConvoMemEvidence }[] = JSON.parse(content) + // Reset once the new data has parsed, so `load()` is idempotent without a + // failed reload discarding what was already loaded. The server caches + // benchmark instances, and a second `load()` would otherwise append a + // duplicate copy of every question. + this.questions = [] + this.sessionsMap.clear() + for (let i = 0; i < items.length; i++) { const { category, item } = items[i] this.processItem(item, category, i) diff --git a/src/benchmarks/locomo/index.ts b/src/benchmarks/locomo/index.ts index 7b8c86c..0da79b6 100644 --- a/src/benchmarks/locomo/index.ts +++ b/src/benchmarks/locomo/index.ts @@ -113,6 +113,12 @@ export class LoCoMoBenchmark implements Benchmark { } private processData(): void { + // Reset first so `load()` is idempotent — the server caches benchmark + // instances, and a second `load()` would otherwise append a duplicate copy + // of every question. + this.questions = [] + this.sessionsMap.clear() + for (const item of this.data) { const sessions = this.extractSessions(item) const sessionIds = sessions.map((s) => s.sessionId) diff --git a/src/benchmarks/longmemeval/index.ts b/src/benchmarks/longmemeval/index.ts index 158d3f9..fc794c6 100644 --- a/src/benchmarks/longmemeval/index.ts +++ b/src/benchmarks/longmemeval/index.ts @@ -184,7 +184,22 @@ export class LongMemEvalBenchmark implements Benchmark { } private loadQuestions(questionsDir: string): void { - const files = readdirSync(questionsDir).filter((f) => f.endsWith(".json")) + // Sorted: `readdirSync` order is unspecified — hash-ordered on ext4 with + // dir_index, roughly lexicographic on APFS/NTFS — and this order decides + // which questions a `--limit N` or sampled run actually covers. Without the + // sort the same command benchmarks a different subset on a different + // machine, and that difference is indistinguishable from a provider + // difference. + const files = readdirSync(questionsDir) + .filter((f) => f.endsWith(".json")) + .sort() + + // Reset first so `load()` is idempotent. The server caches benchmark + // instances, and a second `load()` would otherwise append a duplicate copy + // of every question. + this.data = [] + this.questions = [] + this.sessionsMap.clear() for (const file of files) { const item: LongMemEvalItem = JSON.parse(readFileSync(join(questionsDir, file), "utf8")) diff --git a/src/benchmarks/question-order.test.ts b/src/benchmarks/question-order.test.ts new file mode 100644 index 0000000..fcbf6a2 --- /dev/null +++ b/src/benchmarks/question-order.test.ts @@ -0,0 +1,198 @@ +import { afterAll, beforeEach, describe, expect, mock, test } from "bun:test" + +// Captured before the mock is installed so the real module can be restored for +// any other test file in the same run. +const realFs = { ...(await import("fs")) } + +/** + * Directory listing handed back by the mocked `readdirSync`, deliberately out of + * lexicographic order. This is what an ext4 directory with `dir_index` looks + * like: entries come back in hash order, not sorted. NTFS and APFS happen to + * return something close to sorted, which is why the bug hides on a maintainer's + * machine and shows up on CI. + */ +const UNSORTED_LISTING = ["q3.json", "q1.json", "q10.json", "q2.json"] + +/** questionId per file, so a selection can be traced back to its source file. */ +const QUESTION_IDS: Record = { + "q1.json": "q1", + "q2.json": "q2", + "q3.json": "q3", + "q10.json": "q10", +} + +function longMemEvalItem(questionId: string) { + return { + question_id: questionId, + question: `question ${questionId}?`, + question_type: "single-session-user", + answer: `answer ${questionId}`, + question_date: "2023/05/20 (Sat) 02:21", + haystack_dates: ["2023/05/20 (Sat) 02:21"], + haystack_sessions: [[{ role: "user", content: `content ${questionId}` }]], + } +} + +const locomoConversation = { + sample_id: "conv-1", + conversation: { + speaker_a: "Alice", + speaker_b: "Bob", + session_1_date_time: "1:00 pm on 20 May, 2023", + session_1: [ + { speaker: "Alice", text: "hello", dia_id: "D1:1" }, + { speaker: "Bob", text: "hi", dia_id: "D1:2" }, + ], + }, + qa: [ + { question: "who said hello?", answer: "Alice", category: 1, evidence: ["D1:1"] }, + { question: "who replied?", answer: "Bob", category: 1, evidence: ["D1:2"] }, + ], +} + +const convoMemItems = [ + { + category: "assistant_memory", + item: { + question: "what did they order?", + answer: "a flat white", + conversations: [{ messages: [{ speaker: "User", text: "I ordered a flat white" }] }], + message_evidences: [0], + }, + }, + { + category: "assistant_memory", + item: { + question: "where from?", + answer: "the corner cafe", + conversations: [{ messages: [{ speaker: "User", text: "from the corner cafe" }] }], + message_evidences: [0], + }, + }, +] + +/** Serves each benchmark's on-disk layout without touching a real filesystem. */ +function installFsMock(): void { + mock.module("fs", () => ({ + ...realFs, + existsSync: () => true, + mkdirSync: () => undefined, + writeFileSync: () => undefined, + readdirSync: () => [...UNSORTED_LISTING], + readFileSync: (path: string) => { + const file = String(path).split(/[\\/]/).pop() ?? "" + + const questionId = QUESTION_IDS[file] + if (questionId) return JSON.stringify(longMemEvalItem(questionId)) + + if (file.startsWith("locomo")) return JSON.stringify([locomoConversation]) + if (file.startsWith("convomem")) return JSON.stringify(convoMemItems) + + return "{}" + }, + })) +} + +installFsMock() + +afterAll(() => { + mock.module("fs", () => realFs) +}) + +beforeEach(() => { + installFsMock() +}) + +describe("LongMemEvalBenchmark question order", () => { + async function loadBenchmark() { + const { LongMemEvalBenchmark } = await import("./longmemeval/index") + const benchmark = new LongMemEvalBenchmark() + await benchmark.load() + return benchmark + } + + test("returns questions in a stable order regardless of directory order", async () => { + const benchmark = await loadBenchmark() + + // Lexicographic by filename: q1.json, q10.json, q2.json, q3.json. Without the + // sort this is the filesystem's order (q3, q1, q10, q2). + expect(benchmark.getQuestions().map((q) => q.questionId)).toEqual(["q1", "q10", "q2", "q3"]) + }) + + test("`--limit N` selects the same subset on any filesystem", async () => { + const benchmark = await loadBenchmark() + + // This is the slice the orchestrator takes for a limited run. On the + // unsorted listing it would have been ["q3", "q1"] — a different pair of + // questions, and so a different, incomparable accuracy number. + const limited = benchmark.getQuestions().slice(0, 2) + expect(limited.map((q) => q.questionId)).toEqual(["q1", "q10"]) + }) + + test("sessions stay attached to their question", async () => { + const benchmark = await loadBenchmark() + + for (const questionId of ["q1", "q2", "q3", "q10"]) { + const sessions = benchmark.getHaystackSessions(questionId) + expect(sessions).toHaveLength(1) + expect(sessions[0].sessionId).toBe(`${questionId}-session-0`) + expect(sessions[0].messages[0].content).toBe(`content ${questionId}`) + } + }) + + test("loading twice does not duplicate questions", async () => { + const { LongMemEvalBenchmark } = await import("./longmemeval/index") + const benchmark = new LongMemEvalBenchmark() + + await benchmark.load() + const first = benchmark.getQuestions().map((q) => q.questionId) + await benchmark.load() + const second = benchmark.getQuestions().map((q) => q.questionId) + + expect(second).toEqual(first) + expect(new Set(second).size).toBe(second.length) + }) +}) + +describe("LoCoMoBenchmark", () => { + test("loading twice does not duplicate questions", async () => { + const { LoCoMoBenchmark } = await import("./locomo/index") + const benchmark = new LoCoMoBenchmark() + + await benchmark.load() + const first = benchmark.getQuestions().map((q) => q.questionId) + await benchmark.load() + const second = benchmark.getQuestions().map((q) => q.questionId) + + expect(first).toEqual(["conv-1-q0", "conv-1-q1"]) + expect(second).toEqual(first) + expect(new Set(second).size).toBe(second.length) + }) + + test("keeps ground truth and sessions intact across a reload", async () => { + const { LoCoMoBenchmark } = await import("./locomo/index") + const benchmark = new LoCoMoBenchmark() + + await benchmark.load() + await benchmark.load() + + expect(benchmark.getGroundTruth("conv-1-q0")).toBe("Alice") + expect(benchmark.getHaystackSessions("conv-1-q0")).toHaveLength(1) + }) +}) + +describe("ConvoMemBenchmark", () => { + test("loading twice does not duplicate questions", async () => { + const { ConvoMemBenchmark } = await import("./convomem/index") + const benchmark = new ConvoMemBenchmark() + + await benchmark.load() + const first = benchmark.getQuestions().map((q) => q.questionId) + await benchmark.load() + const second = benchmark.getQuestions().map((q) => q.questionId) + + expect(first).toHaveLength(convoMemItems.length) + expect(second).toEqual(first) + expect(new Set(second).size).toBe(second.length) + }) +})