Skip to content

Commit d2cff75

Browse files
rgarciaclaude
andcommitted
webvoyager: validate --max-images and reject negative --limit
A 0/negative/non-numeric --max-images made lastShots' slice(-k) attach every screenshot instead of the last k, risking judge token blowups and spurious 0 rewards; parse it to a positive integer, falling back to the 15 default otherwise. A negative --limit likewise made tasks[:limit] drop tasks off the end instead of taking the first N, so reject it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent ac7f750 commit d2cff75

4 files changed

Lines changed: 42 additions & 2 deletions

File tree

benchmarks/adapters/webvoyager/judge/src/judge.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,19 @@ export interface Args {
3232
detailsOut?: string;
3333
}
3434

35+
const DEFAULT_MAX_IMAGES = 15;
36+
37+
/**
38+
* Parse `--max-images` into a positive integer last-k window. A 0, negative, or
39+
* non-numeric value (e.g. a bad `WEBVOYAGER_MAX_IMAGES` env override) would make
40+
* `lastShots`' `slice(-k)` attach *all* screenshots — blowing the judge's token
41+
* budget — so anything that isn't a positive integer falls back to the default.
42+
*/
43+
export function parseMaxImages(raw: string | undefined): number {
44+
const value = Number(raw ?? DEFAULT_MAX_IMAGES);
45+
return Number.isInteger(value) && value > 0 ? value : DEFAULT_MAX_IMAGES;
46+
}
47+
3548
function parseArgs(argv: string[]): Args {
3649
const flags = new Map<string, string>();
3750
for (let i = 0; i < argv.length; i += 1) {
@@ -51,7 +64,7 @@ function parseArgs(argv: string[]): Args {
5164
answer: flags.get("answer") ?? "/logs/agent/answer.txt",
5265
shots: flags.get("shots") ?? "/logs/agent/shots",
5366
judgeModel: flags.get("judge-model") ?? "claude-sonnet-4-5",
54-
maxImages: Number(flags.get("max-images") ?? "15"),
67+
maxImages: parseMaxImages(flags.get("max-images")),
5568
rewardOut: required("reward-out"),
5669
detailsOut: flags.get("details-out"),
5770
};

benchmarks/adapters/webvoyager/judge/test/judge.test.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { tmpdir } from "node:os";
33
import { join } from "node:path";
44
import { describe, expect, it } from "vitest";
55
import type { Args } from "../src/judge.ts";
6-
import { run } from "../src/judge.ts";
6+
import { parseMaxImages, run } from "../src/judge.ts";
77
import type { GradingDetails, JudgeContent, JudgeModel } from "../src/types.ts";
88

99
/** A /logs/agent + /tests layout, plus the verifier output paths run() writes. */
@@ -108,3 +108,20 @@ describe("run", () => {
108108
expect(readDetails(args).error).toContain("ANTHROPIC_API_KEY");
109109
});
110110
});
111+
112+
describe("parseMaxImages", () => {
113+
it("keeps a valid positive integer", () => {
114+
expect(parseMaxImages("3")).toBe(3);
115+
expect(parseMaxImages("15")).toBe(15);
116+
});
117+
118+
it("defaults to 15 when unset", () => {
119+
expect(parseMaxImages(undefined)).toBe(15);
120+
});
121+
122+
// A 0/negative/non-numeric last-k makes slice(-k) attach ALL screenshots, so
123+
// anything that isn't a positive integer must fall back to the default.
124+
it.each(["0", "-5", "abc", "", "2.5"])("falls back to 15 for invalid %o", (raw) => {
125+
expect(parseMaxImages(raw)).toBe(15);
126+
});
127+
});

benchmarks/adapters/webvoyager/src/webvoyager/adapter.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,10 @@ def __init__(
9999
refresh: bool = False,
100100
**kwargs: object,
101101
):
102+
if limit is not None and limit < 0:
103+
# tasks[:limit] with a negative limit drops tasks off the *end* instead
104+
# of taking the first N, so reject it rather than silently mis-selecting.
105+
raise ValueError(f"limit must be non-negative, got {limit}")
102106
self.output_dir = Path(output_dir)
103107
self.limit = limit
104108
self.overwrite = overwrite

benchmarks/adapters/webvoyager/tests/test_adapter.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,12 @@ def test_limit_and_task_ids_select(tmp_path: Path) -> None:
132132
assert ids == {"Amazon--3", "Apple--1"}
133133

134134

135+
def test_negative_limit_rejected(tmp_path: Path) -> None:
136+
# tasks[:limit] with a negative limit would drop tasks off the end, so it must error.
137+
with pytest.raises(ValueError, match="non-negative"):
138+
WebVoyagerAdapter(output_dir=tmp_path / "out", limit=-1)
139+
140+
135141
def test_overwrite_false_skips_existing(adapter: WebVoyagerAdapter) -> None:
136142
adapter.run()
137143
target = adapter.output_dir / "webvoyager-allrecipes--0" / "instruction.md"

0 commit comments

Comments
 (0)