Skip to content

Commit 1d5d032

Browse files
committed
Address Copilot review on PR #25
- Drop unused isJudgeTimeout flag from JudgeTimeoutError; instanceof is sufficient since the spawn function and the error class live in the same module realm. - Clamp bytesCapMultiplier to a finite positive number (defaulting to 1) so NaN / Infinity / negative inputs cannot produce empty or runaway prompt sections. - Strengthen the timeout-retry test: capture the full retry prompt and explicitly assert the schema-retry hint header is absent (not just that the prompt is shorter).
1 parent 67de52f commit 1d5d032

2 files changed

Lines changed: 12 additions & 6 deletions

File tree

src/server/judge.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ export interface JudgeInput {
3131
}
3232

3333
export class JudgeTimeoutError extends Error {
34-
readonly isJudgeTimeout = true;
3534
constructor(message: string) {
3635
super(message);
3736
this.name = 'JudgeTimeoutError';
@@ -128,7 +127,11 @@ export function buildJudgePrompt(
128127
opts: BuildJudgePromptOptions = {},
129128
): JudgePromptArtifacts {
130129
const { runConfig, transcript, variantContent, outputs } = input;
131-
const m = opts.bytesCapMultiplier ?? 1;
130+
// Default to 1 if the option is missing or non-finite/non-positive; otherwise
131+
// multiplier values like NaN or Infinity would propagate through Math.floor and
132+
// produce empty or runaway prompt sections.
133+
const rawMultiplier = opts.bytesCapMultiplier ?? 1;
134+
const m = Number.isFinite(rawMultiplier) && rawMultiplier > 0 ? rawMultiplier : 1;
132135
// Floors keep retries useful even if a future caller passes a very small multiplier.
133136
const promptCap = Math.max(256, Math.floor(JUDGE_PROMPT_CAP_BYTES * m));
134137
const variantCap = Math.max(512, Math.floor(JUDGE_VARIANT_CAP_BYTES * m));

test/judge.spec.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -523,9 +523,9 @@ async function withTmpRunDir<T>(fn: (runDir: string) => Promise<T>): Promise<T>
523523

524524
scenario('runJudge: timeout on first attempt triggers retry with halved caps', async () => {
525525
await withTmpRunDir(async (runDir) => {
526-
const calls: { promptLen: number }[] = [];
526+
const calls: string[] = [];
527527
const spawnFn: SpawnJudgeFn = async (_bin, prompt) => {
528-
calls.push({ promptLen: prompt.length });
528+
calls.push(prompt);
529529
if (calls.length === 1) {
530530
throw new JudgeTimeoutError('judge subprocess timed out after 120s');
531531
}
@@ -539,13 +539,16 @@ scenario('runJudge: timeout on first attempt triggers retry with halved caps', a
539539
throw new Error(`expected 2 spawn calls (initial + retry), got ${calls.length}`);
540540
}
541541
// Halved caps must yield a strictly shorter retry prompt.
542-
if (!(calls[1]!.promptLen < calls[0]!.promptLen)) {
542+
if (!(calls[1]!.length < calls[0]!.length)) {
543543
throw new Error(
544-
`retry prompt should be shorter (halved caps); first=${calls[0]!.promptLen} retry=${calls[1]!.promptLen}`,
544+
`retry prompt should be shorter (halved caps); first=${calls[0]!.length} retry=${calls[1]!.length}`,
545545
);
546546
}
547547
// The retry must NOT include the schema-retry hint — that hint is only for
548548
// parse failures, not timeouts.
549+
if (calls[1]!.includes('# Retry required')) {
550+
throw new Error('timeout-retry prompt must not include the schema-retry hint header');
551+
}
549552
const judgeFile = JSON.parse(readFileSync(join(runDir, 'judge.json'), 'utf8')) as {
550553
status: string;
551554
scores?: { accuracy: number };

0 commit comments

Comments
 (0)