Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/),
and this project adheres to [Semantic Versioning](https://semver.org/).

## [Unreleased]

### Fixed

- **A model's top-level `extra` reaches the model.** `models.<m>.extra` passed `--dry-run` and was then dropped at binding, so `Model.extra` was `{}` and the inference identity omitted it. It now binds like `args.extra` on every dialect and on derived models; `args.extra` identities are unchanged. A 0.12.0 result directory written from a top-level `extra` ran without it, so its resume is refused as an inference change; start it fresh (#227).
- **Breaking: `extra` at both placements is refused** before any engine starts, on named models and inline `grader`/`extractor` configs. Inline configs used to accept both and silently use `args.extra`. Keep one (#227).

## [0.12.0] - 2026-10-23

A minor bump: `meta.json` `identity` moves to schema 3, with one identity per
Expand Down
5 changes: 3 additions & 2 deletions docs/guide/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,9 @@ The reply grows by N entries per input token.
asks for a capability by key and does nothing different when it is absent.
SiEval records it in the inference identity and checks the shape of the keys
below, so a typo fails before the run. Set it on a model (top level or
`args`) or per task in `infer_args`; a task's `infer_args.extra` replaces the
model's.
`args`, not both) or per task in `infer_args`. A derived model inherits its
base's `extra` unless it sets its own, and `infer_args.extra` replaces the
model's; keys never merge.

```yaml
models:
Expand Down
21 changes: 21 additions & 0 deletions sieval/cli/resolution.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,26 @@ def validate_task_config_args(
)


def model_config_extra(config: Mapping[str, object], subject: str) -> object:
"""The ``extra`` a model config sets at its top level or in ``args``.

Both at once are refused: ``extra`` is identity, so no silent pick.
``None`` (unset or null) on either side defers to the other.
"""
raw_args = config.get("args")
nested = (
cast(Mapping[str, object], raw_args).get("extra")
if isinstance(raw_args, Mapping)
else None
)
top = config.get("extra")
if top is not None and nested is not None:
raise ValueError(
f"{subject} sets 'extra' both at its top level and in 'args'; keep one"
)
return top if top is not None else nested


def binding_resource_argument_paths(
arguments: Mapping[str, object],
*,
Expand Down Expand Up @@ -237,6 +257,7 @@ def normalize_inline_model_binding(
f"on an unsupported surface: {', '.join(paths)}"
)

model_config_extra(raw_config, f"Task '{task_name}' inline {role}")
requested_model_id = raw_config.get("model")
if not isinstance(requested_model_id, str) or not requested_model_id:
raise ValueError(
Expand Down
28 changes: 24 additions & 4 deletions sieval/cli/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
derive_model_type,
finalize_inline_model_binding,
is_task_model_role_sentinel,
model_config_extra,
normalize_inline_model_binding,
resolve_dataset_class,
resolve_key_function,
Expand Down Expand Up @@ -2096,6 +2097,8 @@ def _validate_named_resource_config(
) -> None:
"""Reject resource overrides before reconciliation, allocation, or I/O."""

# Refuse both ``extra`` placements before any engine starts.
model_config_extra(config, f"Model '{model_name}'")
is_root = config.get("base") is None
allowed_top_level = (
frozenset(
Expand Down Expand Up @@ -4007,10 +4010,12 @@ def _setup_postlaunch_reconciliation(self) -> None:
@staticmethod
def _request_builder_args(
config: Mapping[str, object],
subject: str,
) -> tuple[dict[str, Any], Any]:
raw = config.get("args", {})
if not isinstance(raw, Mapping):
raise ValueError("model args must be a dictionary")
extra = model_config_extra(config, subject)
args = dict(raw)
for key in (
"api_base",
Expand All @@ -4021,7 +4026,7 @@ def _request_builder_args(
"concurrency_limit",
):
args.pop(key, None)
extra = args.pop("extra", None)
args.pop("extra", None)
return args, extra

def _create_owned_pool(
Expand Down Expand Up @@ -4174,6 +4179,10 @@ def _setup_bound_models(self) -> None:
continue
cfg = chain[0][1]
args = self._normalize_dict(cfg.get("args"), f"Model '{name}' args")
args.pop("extra", None)
extra = model_config_extra(cfg, f"Model '{name}'")
if extra is not None:
args["extra"] = extra
if self.deterministic:
_apply_request_seed_decision_to_args(
args,
Expand Down Expand Up @@ -4223,7 +4232,15 @@ def _setup_bound_models(self) -> None:
model = self.models[base_name]
args = self._normalize_dict(cfg.get("args"), f"Model '{name}' args")
concurrency_limit = args.pop("concurrency_limit", None)
model = model.with_args(concurrency_limit=concurrency_limit, **args)
args.pop("extra", None)
model = model.with_args(
concurrency_limit=concurrency_limit,
extra=cast(
dict[str, JSONValue] | None,
model_config_extra(cfg, f"Model '{name}'"),
),
**args,
)
else:
runtime_plan = result.runtime_plans[binding.binding_id]
deployment = self._realized_deployments_by_root[
Expand Down Expand Up @@ -4268,7 +4285,7 @@ def _setup_bound_models(self) -> None:
**inherited_defaults,
)

args, extra = self._request_builder_args(cfg)
args, extra = self._request_builder_args(cfg, f"Model '{name}'")
concurrency_limit = None
if not is_root:
raw_args = cfg.get("args", {})
Expand Down Expand Up @@ -4319,6 +4336,9 @@ def _setup_bound_models(self) -> None:
nested_args=True,
)
model = Model.bind(deployment, pool, runtime_plan)
extra = model_config_extra(
typed_source, f"Inline binding '{binding.binding_id}'"
)
direct_config: dict[str, Any] = dict(typed_source)
nested = direct_config.pop("args", None)
if isinstance(nested, Mapping):
Expand All @@ -4335,9 +4355,9 @@ def _setup_bound_models(self) -> None:
"service_role",
"capabilities",
"engine",
"extra",
):
direct_config.pop(key, None)
extra = direct_config.pop("extra", None)
if direct_config or extra is not None:
model = model.with_args(
extra=cast(dict[str, JSONValue] | None, extra), **direct_config
Expand Down
5 changes: 5 additions & 0 deletions sieval/cli/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
)
from sieval.cli.resolution import (
binding_resource_argument_paths,
model_config_extra,
resolve_dataset_class,
resolve_task_class,
validate_configured_engine_id,
Expand Down Expand Up @@ -221,6 +222,10 @@ def _validate_models(cfg: dict, result: ValidationResult) -> None:
),
):
_check_model_extra(extra, where, result)
try:
model_config_extra(mcfg, f"Model '{name}'")
except ValueError as exc:
result.errors.append(str(exc))

if has_base:
base_ref = mcfg["base"]
Expand Down
22 changes: 22 additions & 0 deletions tests/unit/cli/test_resolution.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
finalize_inline_model_binding,
load_class_from_name,
load_class_from_path,
model_config_extra,
normalize_inline_model_binding,
resolve_class,
resolve_config_model_types,
Expand Down Expand Up @@ -1048,6 +1049,27 @@ def _fake_import_module(module_name: str):
resolve_task_class("SomeTask")


class TestModelConfigExtra:
@pytest.mark.parametrize(
("config", "expected"),
[
({}, None),
({"extra": {"a": 1}}, {"a": 1}),
({"args": {"extra": {"a": 1}}}, {"a": 1}),
({"extra": None, "args": {"extra": {"a": 1}}}, {"a": 1}),
({"extra": {"a": 1}, "args": {"extra": None}}, {"a": 1}),
({"extra": {"a": 1}, "args": "not-a-mapping"}, {"a": 1}),
],
ids=["unset", "top", "args", "top-null", "args-null", "args-malformed"],
)
def test_reads_either_placement(self, config, expected):
assert model_config_extra(config, "Model 'm'") == expected

def test_both_placements_are_refused(self):
with pytest.raises(ValueError, match=r"^Model 'm' sets 'extra' both"):
model_config_extra({"extra": {}, "args": {"extra": {}}}, "Model 'm'")


class TestNormalizeInlineModelBinding:
"""Guards on the inline-binding normalizer shared by both entry points."""

Expand Down
Loading
Loading