Skip to content

fix(judge): a bad --judge model no longer discards a completed run - #314

Merged
Perry2004 merged 4 commits into
TIGER-AI-Lab:mainfrom
vaibhavdabas16:fix/judge-systemexit-loses-run-meta
Aug 30, 2026
Merged

fix(judge): a bad --judge model no longer discards a completed run#314
Perry2004 merged 4 commits into
TIGER-AI-Lab:mainfrom
vaibhavdabas16:fix/judge-systemexit-loses-run-meta

Conversation

@vaibhavdabas16

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #302, implementing both asks.

load_model_config() called sys.exit(1) on an unknown or malformed model name. run.py loads the judge model inside the judge stage, which only executes after the agent has finished. SystemExit derives from BaseException, so it escaped both the judge stage's except Exception (run.py:606) and run()'s top-level except Exception (run.py:674), and the process died before write_run_meta() ever ran.

For a first-time user whose models.yaml lacks deepseek-v4-pro — the default judge, which models.example.yaml warns "must exist here or the scoring stage fails" — that meant burning a full agent run of up to 30 minutes, quite possibly a successful interception, and ending with no run-meta.json at all: invisible to batch stats, to clawbench-rescore (which globs run-meta.json), and to HF upload.

Ask 1 — validate the judge at startup

The judge model is now resolved before the agent runs (run.py:220). A --judge typo costs seconds instead of half an hour. The judge stage reuses that validated config; the lazy load stays for --human runs, which skip startup validation.

Ask 2 — make the failure catchable

load_model_config() raises ModelConfigError, a plain Exception, instead of exiting.

load_models_yaml() had to change too: load_model_config() calls it, so a missing models.yaml would still have exited from inside the judge stage and lost the run — the same bug through a different door. Every failure branch on that path is now catchable, which I verified by exercising each one:

Failure Before After
models.yaml missing SystemExit ModelConfigError
unknown model name SystemExit ModelConfigError
illegal characters in name SystemExit ModelConfigError
missing base_url / api_type SystemExit ModelConfigError
no api_key or api_keys SystemExit ModelConfigError

The top-level call site that loads the agent's own model catches it and exits as before — no output directory exists yet, so there is nothing for a run-meta.json to record. The judge stage's existing except Exception now catches it too, records judge_setup_failed in judge.json, and the run proceeds to write run-meta.json as normal.

That holds the invariant the issue asks for: a completed agent run always writes its metadata, whatever happens at scoring time.

Corpus

  • v2
  • v1
  • both
  • not applicable

Host-side runner change; no task data involved.

Test plan

  • tests/test_run_judge_stage.py, 4 tests, driving run.main() through a fully mocked agent run: a bad --judge exits 1 at startup with docker_run never called and no run-meta written; a judge failure arising mid-run still writes run-meta.json carrying judge_setup_failed; ModelConfigError is catchable by except Exception and is not a SystemExit; and a missing models.yaml raises rather than exits, both directly and through load_model_config().
  • Exercised all five failure branches in the table above against a real temp models.yaml, confirming each raises rather than exiting, and that the happy path still normalizes api_keys: [k1, k2] to api_key: k1.
  • Audited every caller of load_model_config: three, all in run.py, all now guarded. batch.py:65 calls its own local load_models_yaml, so it is unaffected by the change here.
  • Full suite: 196 passed, 3 skipped. The one failure, test_host_tasks.py::test_checked_task_json_files_parse_and_validate[v1-lite], reproduces identically on a clean main on this machine — those task files are git symlinks (mode 120000) that Windows checks out as text. Unrelated.
  • ruff check and ruff format --check clean.

Not verified: this is proven through the mocked harness, not a live containerized run — I don't have Docker or judge API credentials on hand. The judge-stage behaviour is exercised by injection rather than by a real 30-minute run.

Related issues

Fixes #302.

Heads-up on overlap: #313 moves load_models_yaml/load_model_config out of run_support/config.py into utils/model_config.py, and this PR rewrites those same functions in place. Each merges cleanly into main on its own, but git merge-tree reports a conflict in config.py between the two. Happy to rebase whichever you'd like to land second.

load_model_config() called sys.exit(1) on an unknown or malformed model
name. run.py loads the *judge* model inside the judge stage, which only
executes after the agent has finished. SystemExit derives from
BaseException, so it escaped both the judge stage's `except Exception`
and run()'s top-level `except Exception`, and the process died before
write_run_meta() ever ran.

For a first-time user whose models.yaml lacks deepseek-v4-pro — the
default judge — that meant burning a full agent run of up to 30 minutes,
quite possibly a successful interception, and ending with no
run-meta.json at all: invisible to batch stats, to clawbench-rescore
(which globs run-meta.json), and to HF upload.

Both asks in the issue are implemented:

- load_model_config() raises ModelConfigError, a plain Exception, instead
  of calling sys.exit(). load_models_yaml() does the same, since
  load_model_config() calls it and a missing models.yaml would otherwise
  still exit from inside the judge stage. Every failure branch on that
  path — missing file, unknown model, illegal characters, missing
  required field, absent api key — is now catchable.

  The top-level call site that loads the agent's own model catches it and
  exits as before, since no output directory exists yet and there is
  nothing to record. The judge stage's existing except-Exception handler
  now catches it too, records judge_setup_failed in judge.json, and the
  run proceeds to write run-meta.json as normal.

- The judge model is resolved at startup, before the agent runs, so a
  --judge typo costs seconds rather than half an hour. The judge stage
  reuses that validated config; the lazy load remains for --human runs,
  which skip startup validation.

This holds the invariant the issue asks for: a completed agent run always
writes its metadata, whatever happens at scoring time.

Fixes TIGER-AI-Lab#302.
@Perry2004
Perry2004 merged commit 498cbf6 into TIGER-AI-Lab:main Aug 30, 2026
6 checks passed
@github-project-automation github-project-automation Bot moved this from Under Review to Done in ClawBench Aug 30, 2026
@Perry2004 Perry2004 added bug Something isn't working good first issue Good for newcomers labels Aug 30, 2026
vaibhavdabas16 added a commit to vaibhavdabas16/ClawBench that referenced this pull request Aug 31, 2026
…r's home

clawbench-rescore is an installed console script and both docs/scoring.md
and eval/scoring.md advertise it as the way to reproduce leaderboard
numbers from public traces. It shipped two defaults pointing at one
machine's layout:

    --sweep-root   ~/work/ClawBench/claw-output/sweep
    --models-yaml  ~/work/ClawBench/models/models.yaml

Everywhere else that path does not exist, and find_run_dirs() reaches it
through Path.rglob(), which yields nothing for a missing directory rather
than raising. The tool printed "discovered 0 tasks" and exited 0 — a
silent no-op for exactly the audience the script exists to serve.

- Defaults are now WORKSPACE_ROOT / "test-output" (where the runner
  actually writes runs, matching clawbench-batch --output-dir) and the
  workspace-resolved MODELS_YAML.
- A --sweep-root or --only-batch that is not a directory, or a tree with
  no run-meta.json anywhere under it, now prints a diagnostic naming the
  offending path and exits 2.
- The judge model is resolved with the shared load_model_config() rather
  than a private yaml.safe_load() + config.get("api_key"). rescore
  previously rejected the api_keys list form that every other entry point
  normalizes.

load_models_yaml() and load_model_config() move from
runner/run_support/config.py to utils/model_config.py, taking an optional
explicit path; config.py re-exports both, so its importers are unchanged.
The move is what makes them reusable here: run_support/config.py resolves
a container engine at import time and exits when neither Docker nor
Podman is installed, and rescore only reads finished runs — importing it
would have made a post-hoc scoring tool require a container runtime.

The argument parser is extracted into build_parser() so the shipped
defaults can be asserted in tests.

Rebased onto TIGER-AI-Lab#314, which landed first and rewrote both loaders in place to
raise ModelConfigError instead of calling sys.exit. The move carries that
behaviour across rather than reverting it: ModelConfigError is defined
beside the loaders in utils/model_config.py and re-exported from
run_support/config.py, so run.py's two `except ModelConfigError`
handlers are untouched. Two consequences:

- rescore.main() now catches it around the --judge-model lookup and prints
  the same one-line ERROR it used to get from sys.exit, instead of letting
  a traceback out for a command-line typo.
- test_run_judge_stage.py::test_missing_models_yaml_is_also_catchable
  patched MODELS_YAML on run_support.config. That module only re-exports
  the name now, so the patch no longer reached the loaders; it patches
  utils.model_config instead. The property under test is unchanged.

Fixes TIGER-AI-Lab#296.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working good first issue Good for newcomers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A missing judge model kills the process after the agent run and loses run-meta.json entirely (SystemExit escapes both handlers)

2 participants