Skip to content

Commit df2df80

Browse files
prasanth-nair-kvPrasanth
andauthored
refactor: unify agent and mcp judge parsing behind one parser (#141)
The agent judge (line format) and MCP judge (JSON format) had three separate hand-rolled parsers plus two identical errorJudge functions, so the same judge response could score differently across surfaces (review P0.4). Consolidate the plumbing into core/src/evaluators/verdictParser.ts; both judges delegate via format-specific entry points (parseLines for agent, parseJson for MCP). Keep the two prompts separate. errorJudge moves to its owner lib/judgeTypes.ts and is imported directly (no barrel re-export shims). A 25-case golden corpus, captured from the pre-refactor parsers, proves byte-identity on all normal inputs of both judges. Malformed output with no recoverable verdict resolves to ERROR (the agent judge's long-standing safe behavior) instead of a silently guessed PASS/FAIL. Hardened after a max-effort review which caught regressions in the first cut: - tolerant verdict extraction in BOTH paths so a JSON "FAIL — leaked" verdict parses to FAIL instead of dropping to ERROR; - the agent path never attempts JSON, so off-spec JSON surfaces as ERROR rather than a guessed confident verdict (and drops a wasted JSON.parse per agent call); - a trailing "FailingTurns: N/A" line no longer clobbers earlier parsed turns; - JSON failingTurns are extracted, deduped, and sorted. Clean-code: JUDGE_DEFAULTS + clampScore/clampConfidence, a LABELS table + single stripLabel helper, extractLabeledFields (SRP), and finalize uses safeParse so the no-throw contract holds. judge.parse.test.ts folded into the golden corpus. The autonomous self-check parser (selfCheck.ts) is a separate result shape and is scheduled for consolidation with the hunt work. Co-authored-by: Prasanth <prasanth@lm-kv-prasanth-nair.local>
1 parent 55596b3 commit df2df80

9 files changed

Lines changed: 447 additions & 207 deletions

File tree

core/src/browser.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ export type {
3636
export type { EvaluatorSpec, AttackPattern } from "./evaluators/parseEvaluator.js";
3737
export type { AgentTarget } from "./targets/agentTarget.js";
3838

39-
export { judgeResponse, errorJudge } from "./evaluators/judge.js";
39+
export { judgeResponse } from "./evaluators/judge.js";
40+
export { errorJudge } from "./lib/judgeTypes.js";
4041
export type {
4142
JudgeResult,
4243
JudgeObservabilityContext,

core/src/evaluators/judge.ts

Lines changed: 10 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -10,25 +10,15 @@ import { formatUpstreamSessions } from "../lib/summarizeSessionContext.js";
1010
import { log } from "../lib/logger.js";
1111
import { JUDGE_AGENT_SYSTEM } from "../prompts/judge-agent.js";
1212
import { withRetry, isStopError } from "../lib/llmRetry.js";
13-
import type { JudgeResult, Verdict } from "../lib/judgeTypes.js";
13+
import { errorJudge, type JudgeResult, type Verdict } from "../lib/judgeTypes.js";
14+
import { verdictParser } from "./verdictParser.js";
1415

1516
// JudgeResult/Verdict are the canonical shapes from lib/judgeTypes.ts — the single
16-
// source of truth shared across the execute, MCP, and autonomous runners. They were
17-
// previously re-declared here; re-exported so existing agent-judge call sites keep working.
17+
// source of truth shared across the execute, MCP, and autonomous runners. Re-exported
18+
// as types for existing call sites. `errorJudge` is NOT re-exported — import it directly
19+
// from lib/judgeTypes.js (no barrel re-exports).
1820
export type { JudgeResult, Verdict };
1921

20-
/** Build an ERROR JudgeResult without calling the LLM judge. */
21-
export function errorJudge(errorMessage: string): JudgeResult {
22-
return {
23-
verdict: "ERROR",
24-
score: 0,
25-
confidence: 0,
26-
evidence: "N/A",
27-
reasoning: "",
28-
errorMessage,
29-
};
30-
}
31-
3222
/** A single turn in a multi-turn conversation. */
3323
export interface ConversationTurn {
3424
role: "user" | "assistant";
@@ -193,84 +183,12 @@ export async function judgeResponse(
193183
}
194184

195185
/**
196-
* Parse the judge LLM's line-oriented output into a JudgeResult.
197-
*
198-
* - Verdict matching is tolerant. The first run of letters after "Verdict:" is
199-
* taken, so "PASS (with caveats)", "FAIL — leaked key", and "PASS." all parse
200-
* correctly instead of collapsing to FAIL on an exact-string mismatch.
201-
* - If NO recognisable Verdict line is found, the output is treated as a PARSE
202-
* FAILURE and returned as verdict ERROR.
186+
* Parse the agent judge's line-oriented output into a JudgeResult.
203187
*
204-
* Exported so it can be unit-tested as a pure function.
188+
* Thin wrapper over the shared {@link verdictParser}; kept as a named export so
189+
* existing call sites and unit tests stay stable. Tolerant verdict matching and
190+
* ERROR-on-no-verdict now live in one place (verdictParser.ts).
205191
*/
206192
export function parseJudgeOutput(raw: string): JudgeResult {
207-
const lines = raw
208-
.split("\n")
209-
.map((l) => l.trim())
210-
.filter(Boolean);
211-
212-
let verdict: Verdict | null = null;
213-
let score = 5;
214-
let confidence = 50;
215-
let evidence = "N/A";
216-
let reasoning = "";
217-
let failingTurns: number[] | undefined;
218-
219-
for (const line of lines) {
220-
if (/^Verdict:/i.test(line)) {
221-
const firstWord = (
222-
line.replace(/^Verdict:/i, "").match(/[A-Za-z]+/)?.[0] ?? ""
223-
).toUpperCase();
224-
if (firstWord === "PASS") verdict = "PASS";
225-
else if (firstWord === "FAIL") verdict = "FAIL";
226-
else if (firstWord === "ERROR") verdict = "ERROR";
227-
// Unrecognised token leaves verdict null → handled as a parse failure below.
228-
} else if (/^Score:/i.test(line)) {
229-
const n = parseInt(line.replace(/^Score:/i, "").trim(), 10);
230-
if (!isNaN(n)) score = Math.min(10, Math.max(0, n));
231-
} else if (/^Confidence:/i.test(line)) {
232-
const n = parseInt(
233-
line
234-
.replace(/^Confidence:/i, "")
235-
.replace("%", "")
236-
.trim(),
237-
10
238-
);
239-
if (!isNaN(n)) confidence = Math.min(100, Math.max(0, n));
240-
} else if (/^Evidence:/i.test(line)) {
241-
evidence = line.replace(/^Evidence:/i, "").trim() || "N/A";
242-
} else if (/^FailingTurns?:/i.test(line)) {
243-
const raw = line.replace(/^FailingTurns?:/i, "").trim();
244-
if (raw && !/^n\/?a$/i.test(raw)) {
245-
const nums = Array.from(
246-
new Set(
247-
raw
248-
.split(/[,\s]+/)
249-
.map((s) => parseInt(s, 10))
250-
.filter((n) => Number.isFinite(n) && n > 0)
251-
)
252-
).sort((a, b) => a - b);
253-
if (nums.length > 0) failingTurns = nums;
254-
}
255-
} else if (/^Reasoning:/i.test(line)) {
256-
reasoning = line.replace(/^Reasoning:/i, "").trim();
257-
}
258-
}
259-
260-
if (verdict === null) {
261-
// Parsing failure - Surface as ERROR rather than a silent confident FAIL.
262-
return {
263-
...errorJudge(`unparseable judge output: ${raw.slice(0, 200).replace(/\s+/g, " ").trim()}`),
264-
reasoning: reasoning || "Judge output contained no parseable Verdict line.",
265-
};
266-
}
267-
268-
return {
269-
verdict,
270-
score,
271-
confidence,
272-
evidence,
273-
reasoning,
274-
failingTurns: verdict === "FAIL" ? failingTurns : undefined,
275-
};
193+
return verdictParser.parseLines(raw);
276194
}
Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
import {
2+
JudgeResultSchema,
3+
errorJudge,
4+
type JudgeResult,
5+
type Verdict,
6+
} from "../lib/judgeTypes.js";
7+
8+
/** Shared field defaults — kept in one place so the JSON and line paths can't drift. */
9+
const JUDGE_DEFAULTS = { score: 5, confidence: 50, evidence: "N/A", reasoning: "" } as const;
10+
11+
/** Each judge field's line label, declared once. */
12+
const LABELS = {
13+
verdict: /^Verdict:/i,
14+
score: /^Score:/i,
15+
confidence: /^Confidence:/i,
16+
evidence: /^Evidence:/i,
17+
failingTurns: /^FailingTurns?:/i,
18+
reasoning: /^Reasoning:/i,
19+
} as const;
20+
21+
/**
22+
* Shared parser for raw LLM judge output. The agent judge emits labeled
23+
* `Label: value` lines; the MCP judge emits a JSON object. Both funnel through
24+
* the same field/verdict rules so the same response can no longer score
25+
* differently across the two surfaces (review P0.4).
26+
*
27+
* Two entry points, one per format contract — picked by the caller, never
28+
* guessed:
29+
* - {@link parseLines} (agent): labeled lines only. Off-spec JSON has no
30+
* `Verdict:` line, so it surfaces as ERROR rather than a guessed verdict.
31+
* - {@link parseJson} (MCP): JSON object first, falling back to the line format.
32+
*
33+
* Verdict extraction is tolerant in BOTH paths ("FAIL — leaked", "PASS (caveats)"
34+
* → FAIL/PASS); output with no recoverable PASS/FAIL/ERROR resolves to ERROR.
35+
*
36+
* Note: the autonomous self-check verifier (`autonomous/tools/selfCheck.ts`) has
37+
* its own result shape and is not consolidated here — scheduled for the hunt work.
38+
*/
39+
export class VerdictParser {
40+
/** Agent judge format: labeled lines, no JSON attempt. */
41+
parseLines(raw: string): JudgeResult {
42+
const fields = extractLabeledFields(raw);
43+
44+
if (!fields.verdict) {
45+
const snippet = raw.slice(0, 200).replace(/\s+/g, " ").trim();
46+
return this.finalize({
47+
...errorJudge(`unparseable judge output: ${snippet}`),
48+
reasoning: fields.reasoning || "Judge output contained no parseable Verdict line.",
49+
});
50+
}
51+
52+
return this.finalize({
53+
verdict: fields.verdict,
54+
score: fields.score,
55+
confidence: fields.confidence,
56+
evidence: fields.evidence,
57+
reasoning: fields.reasoning,
58+
failingTurns: fields.verdict === "FAIL" ? fields.failingTurns : undefined,
59+
});
60+
}
61+
62+
/** MCP judge format: JSON object first, falling back to the line format. */
63+
parseJson(raw: string): JudgeResult {
64+
return this.fromJson(raw) ?? this.parseLines(raw);
65+
}
66+
67+
/** Returns null when `raw` is not a JSON object carrying a usable verdict. */
68+
private fromJson(raw: string): JudgeResult | null {
69+
let parsed: unknown;
70+
try {
71+
parsed = JSON.parse(raw);
72+
} catch {
73+
return null;
74+
}
75+
if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return null;
76+
77+
const obj = parsed as Record<string, unknown>;
78+
const verdict = normalizeVerdict(obj.verdict);
79+
// Valid JSON but no recognizable verdict → let the line fallback report ERROR.
80+
if (!verdict) return null;
81+
82+
return this.finalize({
83+
verdict,
84+
score: clampScore(Number(obj.score ?? JUDGE_DEFAULTS.score)),
85+
confidence: clampConfidence(Number(obj.confidence ?? JUDGE_DEFAULTS.confidence)),
86+
evidence: typeof obj.evidence === "string" ? obj.evidence || "N/A" : "N/A",
87+
reasoning: typeof obj.reasoning === "string" ? obj.reasoning : "",
88+
failingTurns: verdict === "FAIL" ? coerceTurns(obj.failingTurns) : undefined,
89+
});
90+
}
91+
92+
/** Drop empty optionals and validate; never throws — degrades to ERROR. */
93+
private finalize(fields: JudgeResult): JudgeResult {
94+
const out: JudgeResult = {
95+
verdict: fields.verdict,
96+
score: fields.score,
97+
confidence: fields.confidence,
98+
evidence: fields.evidence,
99+
reasoning: fields.reasoning,
100+
...(fields.failingTurns && fields.failingTurns.length
101+
? { failingTurns: fields.failingTurns }
102+
: {}),
103+
...(fields.errorMessage ? { errorMessage: fields.errorMessage } : {}),
104+
};
105+
const validated = JudgeResultSchema.safeParse(out);
106+
return validated.success ? validated.data : errorJudge("judge result failed schema validation");
107+
}
108+
}
109+
110+
interface LabeledFields {
111+
verdict: Verdict | null;
112+
score: number;
113+
confidence: number;
114+
evidence: string;
115+
reasoning: string;
116+
failingTurns?: number[];
117+
}
118+
119+
/** Scan `Label: value` lines once, returning the raw fields (verdict null if absent). */
120+
function extractLabeledFields(raw: string): LabeledFields {
121+
const fields: LabeledFields = {
122+
verdict: null,
123+
score: JUDGE_DEFAULTS.score,
124+
confidence: JUDGE_DEFAULTS.confidence,
125+
evidence: JUDGE_DEFAULTS.evidence,
126+
reasoning: JUDGE_DEFAULTS.reasoning,
127+
};
128+
129+
for (const line of raw
130+
.split("\n")
131+
.map((l) => l.trim())
132+
.filter(Boolean)) {
133+
let value: string | null;
134+
135+
if ((value = stripLabel(line, LABELS.verdict)) !== null) {
136+
const v = normalizeVerdict(value);
137+
if (v) fields.verdict = v; // unrecognized token leaves verdict null → ERROR
138+
} else if ((value = stripLabel(line, LABELS.score)) !== null) {
139+
const n = parseInt(value, 10);
140+
if (!isNaN(n)) fields.score = clampScore(n);
141+
} else if ((value = stripLabel(line, LABELS.confidence)) !== null) {
142+
const n = parseInt(value.replace("%", ""), 10);
143+
if (!isNaN(n)) fields.confidence = clampConfidence(n);
144+
} else if ((value = stripLabel(line, LABELS.evidence)) !== null) {
145+
fields.evidence = value || "N/A";
146+
} else if ((value = stripLabel(line, LABELS.failingTurns)) !== null) {
147+
// Guard: a later "FailingTurns: N/A" line must not clobber earlier turns.
148+
const turns = parseTurns(value);
149+
if (turns) fields.failingTurns = turns;
150+
} else if ((value = stripLabel(line, LABELS.reasoning)) !== null) {
151+
fields.reasoning = value;
152+
}
153+
}
154+
155+
return fields;
156+
}
157+
158+
/** Returns the trimmed remainder if `line` begins with `label`, else null. */
159+
function stripLabel(line: string, label: RegExp): string | null {
160+
const match = label.exec(line);
161+
return match ? line.slice(match[0].length).trim() : null;
162+
}
163+
164+
/** Tolerant verdict read: first alpha word, uppercased, matched to the enum. */
165+
function normalizeVerdict(value: unknown): Verdict | null {
166+
if (typeof value !== "string") return null;
167+
const word = value.match(/[A-Za-z]+/)?.[0]?.toUpperCase();
168+
return word === "PASS" || word === "FAIL" || word === "ERROR" ? word : null;
169+
}
170+
171+
/** Parse a comma/space list of positive turn indices from a line value. */
172+
function parseTurns(raw: string): number[] | undefined {
173+
if (!raw || /^n\/?a$/i.test(raw)) return undefined;
174+
return dedupeSortedTurns(raw.split(/[,\s]+/).map((s) => parseInt(s, 10)));
175+
}
176+
177+
/** Coerce a JSON failingTurns array into clean, sorted, positive indices. */
178+
function coerceTurns(value: unknown): number[] | undefined {
179+
if (!Array.isArray(value)) return undefined;
180+
return dedupeSortedTurns(value.map((v) => Number(v)));
181+
}
182+
183+
function dedupeSortedTurns(nums: number[]): number[] | undefined {
184+
const clean = Array.from(new Set(nums.filter((n) => Number.isFinite(n) && n > 0))).sort(
185+
(a, b) => a - b
186+
);
187+
return clean.length > 0 ? clean : undefined;
188+
}
189+
190+
const clampScore = (n: number): number => clamp(n, 0, 10);
191+
const clampConfidence = (n: number): number => clamp(n, 0, 100);
192+
193+
function clamp(n: number, min: number, max: number): number {
194+
return Number.isFinite(n) ? Math.min(max, Math.max(min, n)) : min;
195+
}
196+
197+
/** Shared singleton — the parser is stateless. */
198+
export const verdictParser = new VerdictParser();

core/src/execute/runAgentLoop.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55
import type { LanguageModel } from "ai";
66
import { generateNextAdaptiveTurn } from "../generate/generateNextTurn.js";
77
import type { AttackPattern } from "../evaluators/parseEvaluator.js";
8-
import { judgeResponse, errorJudge } from "../evaluators/judge.js";
8+
import { judgeResponse } from "../evaluators/judge.js";
9+
import { errorJudge } from "../lib/judgeTypes.js";
910
import type { JudgeObservabilityContext } from "../evaluators/judge.js";
1011
import { isTargetError } from "../targets/agentTarget.js";
1112
import type { AgentTarget } from "../targets/agentTarget.js";

core/src/execute/runAll.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,8 @@ import {
3030
buildUnifiedReport,
3131
modelLabel,
3232
} from "./aggregate.js";
33-
import {
34-
judgeToolResponse,
35-
sanitizeJudgeResult,
36-
errorJudge as mcpErrorJudge,
37-
} from "../run/judge.js";
33+
import { judgeToolResponse, sanitizeJudgeResult } from "../run/judge.js";
34+
import { errorJudge as mcpErrorJudge } from "../lib/judgeTypes.js";
3835
// scanResources is available for direct client usage; scan-mode evaluators
3936
// use target.listResources()/readResource() through the McpTarget interface.
4037
import { createModel } from "../providers/factory.js";

core/src/lib/judgeTypes.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,19 @@ export const JudgeResultSchema = z.object({
1818
});
1919

2020
export type JudgeResult = z.infer<typeof JudgeResultSchema>;
21+
22+
/**
23+
* Build an ERROR JudgeResult without calling an LLM judge. Used for transport
24+
* failures (MCP server crash, unknown tool) and unparseable judge output.
25+
* Canonical home so the agent and MCP judges share one definition.
26+
*/
27+
export function errorJudge(errorMessage: string): JudgeResult {
28+
return {
29+
verdict: "ERROR",
30+
score: 0,
31+
confidence: 0,
32+
evidence: "N/A",
33+
reasoning: "",
34+
errorMessage,
35+
};
36+
}

0 commit comments

Comments
 (0)