From 048a50153faff18f4b64c5da84392c74305fec43 Mon Sep 17 00:00:00 2001 From: a-rybnikov Date: Thu, 2 Jul 2026 04:29:20 +0200 Subject: [PATCH] Add parseOk status to contribution evaluation parsing (#22) Extract the evaluator-output parsing from routes.ts into a testable module (server/evaluationParse.ts) and add a parseOk flag, so a parse *failure* is visible instead of being silently scored as 1. - evaluateContribution now warns when the model output isn't valid JSON with a numeric score. Previously that case was indistinguishable from a genuine score of 1; a string-typed score (e.g. "8") also silently fell to 1. - Adds server/evaluationParse.test.ts (vitest, 10 cases): valid/fenced JSON, prose, string score, missing score, clamping, float rounding, empty, oversized, fence stripping. Score/summary behaviour is unchanged; this only surfaces parse failures. Co-Authored-By: Claude Opus 4.8 --- server/evaluationParse.test.ts | 61 ++++++++++++++++++++++++++++++++++ server/evaluationParse.ts | 38 +++++++++++++++++++++ server/routes.ts | 17 ++++------ 3 files changed, 105 insertions(+), 11 deletions(-) create mode 100644 server/evaluationParse.test.ts create mode 100644 server/evaluationParse.ts diff --git a/server/evaluationParse.test.ts b/server/evaluationParse.test.ts new file mode 100644 index 0000000..5cf48d4 --- /dev/null +++ b/server/evaluationParse.test.ts @@ -0,0 +1,61 @@ +import { test, expect } from "vitest"; +import { parseEvaluation, stripJsonFence } from "./evaluationParse"; + +test("clean JSON: score + summary parse, parseOk true", () => { + const r = parseEvaluation('{"score": 8, "summary": "solid dilemma"}'); + expect(r.score).toBe(8); + expect(r.summary).toBe("solid dilemma"); + expect(r.parseOk).toBe(true); +}); + +test("fenced ```json block is stripped and parsed", () => { + const r = parseEvaluation('```json\n{"score": 6, "summary": "ok"}\n```'); + expect(r.score).toBe(6); + expect(r.parseOk).toBe(true); +}); + +test("prose (non-JSON) → defaults to 1 but parseOk FALSE (distinguishes from a real 1)", () => { + const r = parseEvaluation("This is a thoughtful contribution about cooperation..."); + expect(r.score).toBe(1); + expect(r.parseOk).toBe(false); + expect(r.summary.startsWith("This is a thoughtful")).toBe(true); +}); + +test("string-typed score '8' is NOT accepted → 1, parseOk FALSE (the #22 gap)", () => { + const r = parseEvaluation('{"score": "8", "summary": "x"}'); + expect(r.score).toBe(1); + expect(r.parseOk).toBe(false); +}); + +test("missing score → 1, parseOk FALSE", () => { + const r = parseEvaluation('{"summary": "no score here"}'); + expect(r.score).toBe(1); + expect(r.parseOk).toBe(false); +}); + +test("out-of-range score is clamped to 1..10, parseOk true", () => { + expect(parseEvaluation('{"score": 99, "summary":"y"}').score).toBe(10); + expect(parseEvaluation('{"score": -4, "summary":"y"}').score).toBe(1); + expect(parseEvaluation('{"score": 99}').parseOk).toBe(true); +}); + +test("float score is rounded", () => { + expect(parseEvaluation('{"score": 7.6, "summary":"y"}').score).toBe(8); +}); + +test("empty / whitespace input → 1, parseOk FALSE", () => { + expect(parseEvaluation("").parseOk).toBe(false); + expect(parseEvaluation(" ").score).toBe(1); +}); + +test("summary falls back to raw content, capped at 2000 chars", () => { + const big = "x".repeat(5000); + const r = parseEvaluation(big); + expect(r.parseOk).toBe(false); + expect(r.summary.length).toBe(2000); +}); + +test("stripJsonFence handles bare and fenced input", () => { + expect(stripJsonFence('{"a":1}')).toBe('{"a":1}'); + expect(stripJsonFence('```json\n{"a":1}\n```')).toBe('{"a":1}'); +}); diff --git a/server/evaluationParse.ts b/server/evaluationParse.ts new file mode 100644 index 0000000..7708e0c --- /dev/null +++ b/server/evaluationParse.ts @@ -0,0 +1,38 @@ +// Parsing of the contribution-evaluator model output. +// Extracted from routes.ts so the parse path (Issue #22) is unit-testable and +// so a parse *failure* is visible (parseOk) rather than silently scored as 1. + +export function stripJsonFence(s: string): string { + return s.trim().replace(/^```(?:json)?\s*/i, "").replace(/```\s*$/i, "").trim(); +} + +export interface ParsedEvaluation { + score: number; // clamped integer 1..10 (defaults to 1 on failure, preserving prior behavior) + summary: string; + parseOk: boolean; // true only when the model returned a JSON object with a usable numeric score +} + +/** + * Parse the evaluator model's raw content into { score, summary, parseOk }. + * Behaviour is intentionally identical to the previous inline logic for score/summary, + * with one addition: `parseOk` distinguishes a real parse failure (bad/prose/string-score + * output → defaulted to 1) from a genuine model score of 1. + */ +export function parseEvaluation(rawContent: string): ParsedEvaluation { + const raw = rawContent ?? ""; + let parsed: any = null; + try { parsed = JSON.parse(stripJsonFence(raw)); } catch { parsed = null; } + + const scoreVal = parsed && typeof parsed === "object" ? parsed.score : undefined; + const scoreIsNumber = typeof scoreVal === "number" && Number.isFinite(scoreVal); + const parseOk = !!parsed && typeof parsed === "object" && scoreIsNumber; + + let score = scoreIsNumber ? Math.round(scoreVal) : 1; // preserve default-to-1 on failure + score = Math.max(1, Math.min(10, score)); + + const summary = parsed && parsed.summary != null && parsed.summary !== "" + ? String(parsed.summary) + : (raw || "No summary produced.").slice(0, 2000); + + return { score, summary, parseOk }; +} diff --git a/server/routes.ts b/server/routes.ts index f2a9208..1221d68 100644 --- a/server/routes.ts +++ b/server/routes.ts @@ -16,6 +16,7 @@ import { ESCALATION_LADDER, scenarioConfigs, escalationBeats } from "./wargameCo import { ModelClient, replayRun, type ArtifactStore, type ModelCallContext } from "./modelClient"; import { deriveEthicalSpace } from "@shared/ethicalSpace"; import { parseHistoryQuery, buildHistoryResponse } from "./historyQuery"; +import { parseEvaluation } from "./evaluationParse"; // Read app version from package.json once at startup let APP_VERSION = "1.0.0"; @@ -868,9 +869,7 @@ async function fetchAndExtractText(url: string): Promise { return text.trim(); } -function stripJsonFence(s: string): string { - return s.trim().replace(/^```(?:json)?\s*/i, "").replace(/```\s*$/i, "").trim(); -} +// stripJsonFence + evaluator-output parsing now live in ./evaluationParse (Issue #22) // Ask an AI model to rate a contribution 1–10 with a short written assessment. async function evaluateContribution(title: string, text: string): Promise<{ score: number; summary: string }> { @@ -886,14 +885,10 @@ Title: ${title} Contribution: ${text}`; const result = await runModel(EVALUATOR_PROVIDER, EVALUATOR_MODEL, [{ role: "user", content: prompt }]); - let parsed: any = null; - try { parsed = JSON.parse(stripJsonFence(result.content)); } catch { parsed = null; } - let score = parsed && typeof parsed.score === "number" ? Math.round(parsed.score) : NaN; - if (!Number.isFinite(score)) score = 1; - score = Math.max(1, Math.min(10, score)); - const summary = parsed?.summary - ? String(parsed.summary) - : (result.content || "No summary produced.").slice(0, 2000); + const { score, summary, parseOk } = parseEvaluation(result.content); + if (!parseOk) { + console.warn(`[evaluateContribution] evaluator output was not valid JSON with a numeric score; score defaulted to 1 (title: ${title.slice(0, 80)})`); + } return { score, summary }; }