Skip to content

fix(cli): bind a model's top-level extra like args.extra - #228

Merged
ethan-scitix merged 2 commits into
mainfrom
fix/model-top-level-extra
Oct 24, 2026
Merged

ethan-scitix merged 2 commits into
mainfrom
fix/model-top-level-extra

Conversation

@ethan-scitix

@ethan-scitix ethan-scitix commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Type

  • fix — bug fix or alignment correction

Summary

  • Root cause: the configuration guide documents a model's extra as "top level or args", and the schema check validated both placements, but model binding read only args.extra. A top-level extra was dropped: Model.extra == {}, tasks rendered plain prompts, and the inference identity left it out. This affected the canonical path and both sglang_legacy paths (root and derived). Inline grader / extractor configs flatten args, so they already accepted both placements.
  • Fix: model_config_extra (sieval/cli/resolution.py) reads either placement. Every binding path uses it, so check_extra, Model.extra, the inference identity and --dry-run (which plans through the same session setup) all see the top-level value.
  • Precedence: setting both placements is refused. Named models let the top level win for other dual-placement keys and inline roles let args win, but none of those keys is identity; inline extra used to silently resolve to args.extra, so refusing both there is a behaviour change too (in the CHANGELOG). extra is part of the inference identity, so silently picking one of two values would record a capability set nobody chose. The refusal fires at schema validation and at named/inline normalization, before any engine starts. A derived model (base:) inherits its base's extra unless it sets its own. A task's infer_args.extra replaces the model's. Neither merges keys, as before.
  • Identity: unchanged for configs that set args.extra. The model_request_args digests for a root and a derived model match main. A 0.12.0 result directory written from a top-level extra ran without it, so a resume now sees an inference change and is refused (noted in the CHANGELOG).
  • Docs: the guide states the both-refused rule and the inheritance and replacement behaviour.

Related Issues

Closes #227

Test Plan

Automated

  • Lint/format clean (ruff check && ruff format --check)
  • Type check clean (ty check)
  • Unit tests pass: pytest tests/unit tests/integration tests/acceptance -m "not stress and not benchmark" -n auto: 9488 passed, 1 failed. The failure is test_exec_quotebench_anchor, a known macOS-only failure. Coverage on unit + integration: 97.11% (gate 95%).
  • pre-commit run -a, scripts/check_preflight.py, scripts/sync_package_stubs.py --check

New tests (tests/unit/cli/test_session.py::TestModelExtraPlacement, full session setup through binding):

  • top-level and args bind the same Model.extra and model_request_args()["extra"], on a canonical and an sglang_legacy model;
  • a derived base: inherits a top-level extra, and its own top-level extra replaces the base's (both paths);
  • both placements on a root or a derived model are refused at prelaunch, before any client is built;
  • a task's infer_args.extra replaces a top-level model extra on the task's model and leaves the named model unchanged;
  • inline grader: either placement binds, and both are refused.

tests/unit/cli/test_resolution.py::TestModelConfigExtra: either placement, null on one side defers to the other, both refused.

tests/unit/cli/test_validation.py: the schema rejects both placements, and the --dry-run schema check catches a malformed top-level or args extra, rejects both, and passes a valid top-level one.

Ten of the new session tests fail on main. The four that pass are the inline-placement cases and the args baseline.

Manual

  • Script check: model_request_args digests for an args.extra root and derived model are byte-identical before and after.

Checklist

Required (all PRs)

  • PR title follows conventional format (type(scope): description)
  • No internal paths, credentials, or personal info in committed files
  • AI-generated code has AI-Generated Code - <model> (<provider>) in module docstring (no new modules)
  • No new upper-layer dependencies added to core/
  • Deleted code verified — no remaining call sites depend on it

If: Breaking Change

  • Described what breaks and migration path in Summary: a config that sets extra at both placements is now refused; keep one.

🤖 Generated with Claude Code

ethan-scitix and others added 2 commits October 24, 2026 14:36
The configuration guide documents `extra` on a model "top level or
`args`", and the schema check validated both, but model binding read
only `args.extra`. A top-level `extra` was dropped: `Model.extra` was
`{}`, tasks rendered plain prompts, and the inference identity left it
out.

`model_config_extra` (cli/resolution.py) reads either placement and
refuses both at once. `extra` enters the inference identity, so a
silent pick between two values would record a capability set nobody
chose. For other dual-placement keys named models let the top level
win and inline roles let `args` win, but none of those keys is
identity. The refusal fires at schema validation and at
named/inline normalization, before any engine starts.

Every binding path reads it: canonical named models, the sglang_legacy
root and derived paths, and inline roles. A derived model inherits its
base's `extra` unless it sets its own; a task's `infer_args.extra`
replaces the model's.

A config that sets `args.extra` keeps its inference identity.

Closes #227

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Inline grader/extractor configs used to accept both `extra` placements
and silently use `args.extra` (`args` is laid over the top level there;
named models let the top level win). The CHANGELOG now lists the
both-placements refusal as breaking for them too.

Pin `model_config_extra`'s null fall-through with unit tests, note why
the prelaunch call exists, and tighten the docstring, guide paragraph
and CHANGELOG entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ethan-scitix
ethan-scitix force-pushed the fix/model-top-level-extra branch from d0819e1 to 9e15b03 Compare October 24, 2026 06:36
@ethan-scitix
ethan-scitix merged commit bd62ed7 into main Oct 24, 2026
9 checks passed
@ethan-scitix
ethan-scitix deleted the fix/model-top-level-extra branch October 24, 2026 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: a model's top-level extra is dropped; only args.extra reaches Model.extra

1 participant