Skip to content

Commit 7f815b9

Browse files
fix: review follow-ups — verdict value-type validation, hard parse budget, O(1) span eviction
- isEvalVerdict requires usable section shapes (audits/patterns arrays; necessity array or object) so a quoted example like {"audits": "..."} can't shadow a real verdict and silently default the category scores - a span larger than the remaining parse budget is skipped instead of parsed (the budget is now a hard cap), and later smaller spans still get their turn - matched-span retention uses a ring buffer: shift() re-indexed the whole array and made a {}{}{} pair flood quadratic
1 parent 69991cf commit 7f815b9

3 files changed

Lines changed: 41 additions & 4 deletions

File tree

src/scoring/deep-eval.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,16 @@ function formatFullEntry(entry: NormalizedEntry): string {
348348
// A judge reply can quote example objects in prose; only the schema tells the
349349
// verdict apart. Any of the three known sections marks an eval verdict.
350350
function isEvalVerdict(c: Record<string, unknown>): boolean {
351-
return "audits" in c || "necessity" in c || "patterns" in c;
351+
// Key presence alone lets a quoted example like {"audits": "..."} shadow a
352+
// real verdict — the section values must carry usable shapes: arrays for
353+
// audits/patterns; necessity is an array (legacy all-category eval) or a
354+
// single object (per-category eval).
355+
return (
356+
Array.isArray(c.audits) ||
357+
Array.isArray(c.patterns) ||
358+
Array.isArray(c.necessity) ||
359+
(typeof c.necessity === "object" && c.necessity !== null)
360+
);
352361
}
353362

354363
export function parseCategoryEvalResponse(

src/scoring/parse-json.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ export function parseJsonFromText(
3838
for (const span of spans) {
3939
if (parseBudget <= 0) break;
4040
if (accepted.some((a) => a.start <= span.start && a.end >= span.end)) continue;
41+
// A span larger than the remaining budget is skipped, not parsed — the
42+
// budget is a hard cap — and later (smaller) spans still get their turn.
43+
if (span.end - span.start > parseBudget) continue;
4144
parseBudget -= span.end - span.start;
4245
const parsed = tryParseObject(text.slice(span.start, span.end + 1));
4346
if (parsed) {
@@ -90,7 +93,11 @@ function tryParseObject(candidate: string): Record<string, unknown> | null {
9093
* pass: push on `{`, pop on `}`. Braces inside JSON string values don't
9194
* count, and an unmatched open simply never pops. */
9295
function matchedSpans(text: string): { start: number; end: number }[] {
93-
const spans: { start: number; end: number }[] = [];
96+
// Ring buffer: keeps the last SPAN_LIMIT spans in O(1) per span (shift()
97+
// re-indexes the whole array and turns a {}{}{} flood quadratic) and caps
98+
// memory on span-heavy replies.
99+
const ring: ({ start: number; end: number } | undefined)[] = new Array(SPAN_LIMIT);
100+
let count = 0;
94101
const stack: number[] = [];
95102
let inString = false;
96103
for (let i = 0; i < text.length; i += 1) {
@@ -105,10 +112,13 @@ function matchedSpans(text: string): { start: number; end: number }[] {
105112
else if (ch === "}") {
106113
const start = stack.pop();
107114
if (start !== undefined) {
108-
spans.push({ start, end: i });
109-
if (spans.length > SPAN_LIMIT) spans.shift();
115+
ring[count % SPAN_LIMIT] = { start, end: i };
116+
count += 1;
110117
}
111118
}
112119
}
120+
const spans: { start: number; end: number }[] = [];
121+
const from = Math.max(0, count - SPAN_LIMIT);
122+
for (let n = from; n < count; n += 1) spans.push(ring[n % SPAN_LIMIT]!);
113123
return spans;
114124
}

test/unit/scoring/parse-json.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,3 +170,21 @@ End.`;
170170
expect(result).toEqual({ flaggedInteractions: [], patterns: [] });
171171
});
172172
});
173+
174+
describe("parseJsonFromText hardening (review follow-ups)", () => {
175+
it("a {}{}{} pair flood stays fast and keeps the trailing verdict", () => {
176+
const input = `${"{}".repeat(500_000)}{"score": 7}`;
177+
const started = Date.now();
178+
expect(parseJsonFromText(input, (c) => typeof c.score === "number")).toEqual({ score: 7 });
179+
expect(Date.now() - started).toBeLessThan(2_000);
180+
});
181+
182+
it("a span larger than the parse budget is skipped, smaller later spans still parse", () => {
183+
// 9MB of digits inside one balanced unparseable span exceeds the 8M
184+
// budget — it must be skipped without being fed to JSON.parse, and the
185+
// small verdict after it must still be found.
186+
const big = `{${"9".repeat(9_000_000)} not json}`;
187+
const input = `${big}\n{"score": 4}`;
188+
expect(parseJsonFromText(input)).toEqual({ score: 4 });
189+
});
190+
});

0 commit comments

Comments
 (0)