Skip to content

fix(schema): do not invent max attempts - #258

Open
reacher-z wants to merge 1 commit into
evaleval:mainfrom
reacher-z:fix/max-attempts-default
Open

fix(schema): do not invent max attempts#258
reacher-z wants to merge 1 commit into
evaleval:mainfrom
reacher-z:fix/max-attempts-default

Conversation

@reacher-z

Copy link
Copy Markdown

What / source

Changes the optional generation_args.max_attempts schema default from 1 to null and regenerates eval_types.py. Sources that do not report an attempt limit no longer materialize a claim that one attempt was allowed. Explicitly observed Inspect values remain unchanged.

Fixes #236.

Review lane

  • Fast
  • Needs a human — this is a material output semantics change across converters.

Design agreed in: #236, including collaborator confirmation that this is worth doing.

Checklist

  • Validation is clean — 2 lm-eval aggregate records and 1 Inspect aggregate record, with 0 warnings
  • Offline regression tests added; full test suite green: 759 passed, 20 skipped
  • Ruff check clean
  • Content spot-checked: absent lm-eval value is omitted; explicit Inspect value remains 1

The adapter report, registry, and source-row checklist items are not applicable because this changes schema default serialization and drops no source records.

Decisions & coverage

  • Decision / where: schema_version
    Chose / instead of: keep the current version because the field remains optional and accepts the same integer values; only its non-required default annotation changes
    Confidence: high
    General: no
  • Decision / where: generated model
    Chose / instead of: regenerate eval_types.py from the schema and run post_codegen, rather than hand-editing generated behavior
    Confidence: high
    General: yes
  • Decision / where: explicit values
    Chose / instead of: preserve source-provided Inspect max_attempts values while omitting unobserved values
    Confidence: high
    General: yes

Coverage: 2 lm-eval source tasks to 2 valid records, 0 dropped; 1 Inspect source run to 1 valid record, 0 dropped. The lm-eval outputs contain no max_attempts key; the Inspect fixture that explicitly reports 1 still emits 1.

Commands run:

  • uv run ruff check every_eval_ever tests
  • env -u NO_COLOR TERM=xterm-256color uv run pytest tests -q
  • uv run --extra inspect pytest tests/test_inspect_adapter.py -q
  • real lm-eval and Inspect CLI conversions followed by validation at final collection/developer/model paths

AI assistance

OpenAI Codex assisted with implementation, tests, and validation. I reviewed the diff and the generated output.

Leave max_attempts unset when a source does not report an attempt limit, while preserving explicitly observed values.

Assisted-by: OpenAI Codex
@borgr borgr added the auto-review Cleared for fast review/merge, including by an agent (maintainers only) label Aug 26, 2026
@borgr

borgr commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

CI was never run here either — the workflow was queued as action_required for a first-time contributor. Approved, and all four rows are green.

This closes #236. The change is right: a record carrying max_attempts: 1 asserted an attempt count no source stated, and the two new tests pin both directions — Inspect keeps a real 1 it read from the log, lm-eval omits the field entirely rather than inventing it.

Flagging the lane rather than the content: this edits eval.schema.json and the generated types, and it changes what existing converters emit, so by CONTRIBUTING's review policy it wants a human read and not only an agent pass — the diff is four files but the blast radius is every record with a generation_args. Labelled auto-review so the pass happens too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-review Cleared for fast review/merge, including by an agent (maintainers only)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

generation_args.max_attempts defaults to 1, so records assert an attempt count no source stated

2 participants