From 4cc321498475dde78b4b9993683f9d623c44ce91 Mon Sep 17 00:00:00 2001 From: Ethan Date: Sat, 24 Oct 2026 13:48:28 +0800 Subject: [PATCH 1/2] fix(cli): bind a model's top-level extra like args.extra 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 --- CHANGELOG.md | 6 + docs/guide/configuration.md | 7 +- sieval/cli/resolution.py | 22 +++ sieval/cli/session.py | 27 +++- sieval/cli/validation.py | 5 + tests/unit/cli/test_session.py | 236 ++++++++++++++++++++++++++++++ tests/unit/cli/test_validation.py | 41 ++++++ 7 files changed, 337 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db9fab1e..ad04bc84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ 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..extra` was checked by `--dry-run` and then dropped when the model was built, so `Model.extra` was `{}` and the inference identity left it out; only `args.extra` worked. Both placements now bind the same value, on every dialect and on a derived model. Setting both is refused before any engine starts. A config that sets `args.extra` keeps its inference identity. A result directory written under 0.12.0 from a top-level `extra` ran without it, so resuming it is refused as an inference change; start it fresh (#227). + ## [0.12.0] - 2026-10-23 A minor bump: `meta.json` `identity` moves to schema 3, with one identity per diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 602574d4..e36f52cc 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -196,9 +196,10 @@ The reply grows by N entries per input token. `extra` is a dict on a model that tasks read and the API never sees: a task 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. +below, so a typo fails before the run. Set it on a model, at its top level +or in `args` but not both, or per task in `infer_args`. 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. ```yaml models: diff --git a/sieval/cli/resolution.py b/sieval/cli/resolution.py index 3cf91155..3a6ad48b 100644 --- a/sieval/cli/resolution.py +++ b/sieval/cli/resolution.py @@ -104,6 +104,27 @@ 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: either one may be the value the author meant, + and ``extra`` enters the inference identity, so a silent pick would record + a capability set nobody chose. ``None`` when neither is set. + """ + 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], *, @@ -237,6 +258,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( diff --git a/sieval/cli/session.py b/sieval/cli/session.py index 88e01be9..77c17ebe 100644 --- a/sieval/cli/session.py +++ b/sieval/cli/session.py @@ -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, @@ -2096,6 +2097,7 @@ def _validate_named_resource_config( ) -> None: """Reject resource overrides before reconciliation, allocation, or I/O.""" + model_config_extra(config, f"Model '{model_name}'") is_root = config.get("base") is None allowed_top_level = ( frozenset( @@ -4007,10 +4009,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", @@ -4021,7 +4025,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( @@ -4174,6 +4178,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, @@ -4223,7 +4231,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[ @@ -4268,7 +4284,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", {}) @@ -4319,6 +4335,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): @@ -4335,9 +4354,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 diff --git a/sieval/cli/validation.py b/sieval/cli/validation.py index 01efd380..41551947 100644 --- a/sieval/cli/validation.py +++ b/sieval/cli/validation.py @@ -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, @@ -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"] diff --git a/tests/unit/cli/test_session.py b/tests/unit/cli/test_session.py index 4f06352b..fc8e5945 100644 --- a/tests/unit/cli/test_session.py +++ b/tests/unit/cli/test_session.py @@ -7409,6 +7409,242 @@ class LegacyGenTask(MockTask): await session._close_owned_model_resources() +_WRAPPERS: dict[str, JSONValue] = {"dna": "{seq}"} +_RNA_WRAPPERS: dict[str, JSONValue] = {"rna": "{seq}"} + + +class TestModelExtraPlacement: + """A model's ``extra`` binds the same from its top level or from ``args``, + on the canonical and legacy paths, and both at once are refused.""" + + class ChatTask: + model_type = "chat" + + @classmethod + def model_requirements_for(cls, context): + return ( + TaskModelRequirement( + role="candidate", + binding=context.model_bindings["candidate"], + requires=TaskRequirements(input=InputKind.CHAT), + source_task="chat_task", + ), + ) + + class GenTask: + model_type = "gen" + + @classmethod + def model_requirements_for(cls, context): + return ( + TaskModelRequirement( + role="candidate", + binding=context.model_bindings["candidate"], + requires=TaskRequirements(input=InputKind.COMPLETION), + source_task="gen_task", + ), + ) + + @staticmethod + def _bound(tmp_path: Path, models: str, task_cls: type) -> EvalSession: + config_path = _write_yaml_config( + tmp_path, + "extra.yaml", + f"models:\n{models}" + "tasks:\n t:\n class: fake.T\n" + " dataset:\n class: fake.Dataset\n model: m\n", + ) + session = EvalSession(config_path) + with ( + patch("sieval.cli.session.resolve_task_class", return_value=task_cls), + patch("sieval.core.models.connection_factory.AsyncOpenAI"), + ): + session._setup_prelaunch_reconciliation() + session._setup_postlaunch_reconciliation() + session._setup_models() + return session + + _ROOT = " root:\n name: org/m\n api_base: http://x/v1\n api_key: k\n" + _LEGACY = " type: gen\n engine: sglang\n dialect: sglang_legacy\n" + + @pytest.mark.parametrize( + "placement", + [ + " extra:\n sequence_wrappers: {dna: '{seq}'}\n", + " args:\n extra:\n" + " sequence_wrappers: {dna: '{seq}'}\n", + ], + ids=["top-level", "args"], + ) + @pytest.mark.parametrize("legacy", [False, True], ids=["canonical", "legacy"]) + def test_top_level_and_args_bind_the_same_extra( + self, tmp_path: Path, placement: str, legacy: bool + ) -> None: + from sieval.core.tasks.run_meta import model_request_args + + root = self._ROOT.replace("root", "m") + (self._LEGACY if legacy else "") + session = self._bound( + tmp_path, + root + placement, + self.GenTask if legacy else self.ChatTask, + ) + model = session.models["m"] + assert model.extra == {"sequence_wrappers": _WRAPPERS} + # The inference identity reads ``extra`` from the bound model. + assert model_request_args(model)["extra"] == {"sequence_wrappers": _WRAPPERS} + assert "extra" not in model._kwargs + + @pytest.mark.parametrize("legacy", [False, True], ids=["canonical", "legacy"]) + def test_derived_model_inherits_top_level_extra( + self, tmp_path: Path, legacy: bool + ) -> None: + models = ( + self._ROOT + + (self._LEGACY if legacy else "") + + " extra:\n sequence_wrappers: {dna: '{seq}'}\n" + + " m:\n base: root\n args:\n temperature: 0.5\n" + ) + session = self._bound( + tmp_path, models, self.GenTask if legacy else self.ChatTask + ) + assert session.models["m"].extra == {"sequence_wrappers": _WRAPPERS} + assert session.models["m"]._kwargs["temperature"] == 0.5 + + @pytest.mark.parametrize("legacy", [False, True], ids=["canonical", "legacy"]) + def test_derived_top_level_extra_replaces_the_base( + self, tmp_path: Path, legacy: bool + ) -> None: + models = ( + self._ROOT + + (self._LEGACY if legacy else "") + + " extra:\n sequence_wrappers: {dna: '{seq}'}\n" + + " m:\n base: root\n" + + " extra:\n sequence_wrappers: {rna: '{seq}'}\n" + ) + session = self._bound( + tmp_path, models, self.GenTask if legacy else self.ChatTask + ) + assert session.models["m"].extra == {"sequence_wrappers": _RNA_WRAPPERS} + assert session.models["root"].extra == {"sequence_wrappers": _WRAPPERS} + + @pytest.mark.parametrize("derived", [False, True], ids=["root", "derived"]) + def test_both_placements_are_refused_before_binding( + self, tmp_path: Path, derived: bool + ) -> None: + both = ( + " extra:\n sequence_wrappers: {dna: '{seq}'}\n" + " args:\n extra:\n" + " sequence_wrappers: {rna: '{seq}'}\n" + ) + models = ( + self._ROOT + " m:\n base: root\n" + both + if derived + else self._ROOT.replace("root", "m") + both + ) + config_path = _write_yaml_config( + tmp_path, + "extra.yaml", + f"models:\n{models}" + "tasks:\n t:\n class: fake.T\n" + " dataset:\n class: fake.Dataset\n model: m\n", + ) + session = EvalSession(config_path) + with ( + patch("sieval.cli.session.resolve_task_class", return_value=self.ChatTask), + patch("sieval.core.models.connection_factory.AsyncOpenAI") as client, + pytest.raises(ValueError, match=r"Model 'm' sets 'extra' both"), + ): + session._setup_prelaunch_reconciliation() + client.assert_not_called() + + def test_task_infer_args_extra_replaces_top_level_extra( + self, tmp_path: Path + ) -> None: + from tests.unit.core.runners.test_runner import MockTask + + config_path = _write_yaml_config( + tmp_path, + "extra.yaml", + "models:\n" + + self._ROOT.replace("root", "m") + + " extra:\n sequence_wrappers: {dna: '{seq}'}\n" + "datasets:\n ds:\n class: fake.Dataset\n" + "tasks:\n t:\n class: fake.MockTask\n dataset: ds\n" + " model: m\n infer_args:\n extra:\n" + " sequence_wrappers: {rna: '{seq}'}\n", + ) + session = EvalSession(config_path) + with ( + patch("sieval.cli.session.resolve_task_class", return_value=MockTask), + patch("sieval.core.models.connection_factory.AsyncOpenAI"), + ): + session._setup_prelaunch_reconciliation() + session._setup_postlaunch_reconciliation() + session._setup_models() + session._init_runner() + session.datasets["ds"] = MagicMock() + session._setup_tasks() + + assert session.runner is not None + task_model = session.runner._runners[0]._task.model + assert task_model.extra == {"sequence_wrappers": _RNA_WRAPPERS} + assert session.models["m"].extra == {"sequence_wrappers": _WRAPPERS} + + @pytest.mark.parametrize( + "placement", + [ + " extra:\n sequence_wrappers: {dna: '{seq}'}\n", + " args:\n extra:\n" + " sequence_wrappers: {dna: '{seq}'}\n", + ], + ids=["top-level", "args"], + ) + def test_inline_role_binds_extra_from_either_placement( + self, tmp_path: Path, placement: str + ) -> None: + judge_task = TestPrelaunchReconciliation.JudgeTask + config_path = _write_yaml_config( + tmp_path, + "extra.yaml", + "models:\n" + + self._ROOT.replace("root", "m") + + "tasks:\n t:\n class: fake.JudgeTask\n" + " dataset:\n class: fake.Dataset\n model: m\n" + " args:\n grader:\n model: judge\n" + " api_base: http://judge/v1\n api_key: k\n" + placement, + ) + session = EvalSession(config_path) + with ( + patch("sieval.cli.session.resolve_task_class", return_value=judge_task), + patch("sieval.core.models.connection_factory.AsyncOpenAI"), + ): + session._setup_prelaunch_reconciliation() + session._setup_postlaunch_reconciliation() + session._setup_models() + grader = session._bound_task_role_models["t"]["grader"] + assert grader.extra == {"sequence_wrappers": _WRAPPERS} + + def test_inline_role_with_both_placements_is_refused(self, tmp_path: Path) -> None: + judge_task = TestPrelaunchReconciliation.JudgeTask + config_path = _write_yaml_config( + tmp_path, + "extra.yaml", + "models:\n" + + self._ROOT.replace("root", "m") + + "tasks:\n t:\n class: fake.JudgeTask\n" + " dataset:\n class: fake.Dataset\n model: m\n" + " args:\n grader:\n model: judge\n" + " api_base: http://judge/v1\n api_key: k\n" + " extra: {}\n args:\n extra: {}\n", + ) + session = EvalSession(config_path) + with ( + patch("sieval.cli.session.resolve_task_class", return_value=judge_task), + pytest.raises(ValueError, match=r"inline grader sets 'extra' both"), + ): + session._setup_prelaunch_reconciliation() + + class TestEvalSessionWrappers: @pytest.mark.anyio async def test_arun_session_delegates_to_runner(self): diff --git a/tests/unit/cli/test_validation.py b/tests/unit/cli/test_validation.py index 7be5e1f4..7bef1023 100644 --- a/tests/unit/cli/test_validation.py +++ b/tests/unit/cli/test_validation.py @@ -1074,6 +1074,19 @@ def test_well_formed_sequence_wrappers_pass(self): "sequence_wrappers" in e for e in validate_eval_config(cfg).errors ) + def test_extra_both_top_level_and_in_args_is_rejected(self): + extra = {"sequence_wrappers": {"dna": "{seq}"}} + cfg = { + "models": {"m": {"name": "m", "extra": extra, "args": {"extra": extra}}}, + "datasets": {"d": {"class": "X"}}, + "tasks": {"t": {"class": "X", "dataset": "d", "model": "m"}}, + } + errors = validate_eval_config(cfg).errors + assert any( + e.startswith("Model 'm' sets 'extra' both at its top level and in 'args'") + for e in errors + ), errors + def test_model_role_unsupported_by_task_constructor_is_rejected(self): # `sieval eval` must reject the same role config `sieval run --dry-run` # rejects; the two entry points disagreeing is what let the missing @@ -1714,6 +1727,34 @@ def test_explicit_sglang_legacy_bypass_reaches_prelaunch(self, tmp_path): assert checks["imports"]["ok"] is True assert checks["capability_reconcile"]["ok"] is True + @pytest.mark.parametrize( + ("extra_yaml", "ok"), + [ + (" extra:\n sequence_wrappers: {dna: ''}\n", False), + ( + " args:\n extra:\n sequence_wrappers: {dna: ''}\n", + False, + ), + (" extra: {}\n args:\n extra: {}\n", False), + (" extra:\n sequence_wrappers: {dna: '{seq}'}\n", True), + ], + ids=["top-level-malformed", "args-malformed", "both", "top-level-valid"], + ) + def test_schema_check_sees_top_level_extra(self, tmp_path, extra_yaml, ok): + from sieval.cli.validation import run_dry_run + + config = tmp_path / "eval.yaml" + config.write_text( + "models:\n m:\n name: org/m\n api_base: http://x/v1\n" + + extra_yaml + + "datasets:\n d:\n class: HumanEvalDataset\n" + "tasks:\n t:\n class: sieval.tasks.human_eval_0shot_base_gen." + "HumanEvalZeroShotBaseGenTask\n dataset: d\n model: m\n", + encoding="utf-8", + ) + checks = {c["name"]: c for c in run_dry_run(config)["checks"]} + assert checks["schema"]["ok"] is ok, checks["schema"] + def test_returns_dict_on_missing_file(self, tmp_path): from sieval.cli.validation import run_dry_run From 9e15b0337b73b6e873ea4b7a9f13a8fdec84046c Mon Sep 17 00:00:00 2001 From: Ethan Date: Sat, 24 Oct 2026 14:35:03 +0800 Subject: [PATCH 2/2] fix(cli): note the inline extra behaviour change; trim extra docs 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 --- CHANGELOG.md | 3 ++- docs/guide/configuration.md | 8 ++++---- sieval/cli/resolution.py | 7 +++---- sieval/cli/session.py | 1 + tests/unit/cli/test_resolution.py | 22 ++++++++++++++++++++++ 5 files changed, 32 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad04bc84..6ad5588c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ### Fixed -- **A model's top-level `extra` reaches the model.** `models..extra` was checked by `--dry-run` and then dropped when the model was built, so `Model.extra` was `{}` and the inference identity left it out; only `args.extra` worked. Both placements now bind the same value, on every dialect and on a derived model. Setting both is refused before any engine starts. A config that sets `args.extra` keeps its inference identity. A result directory written under 0.12.0 from a top-level `extra` ran without it, so resuming it is refused as an inference change; start it fresh (#227). +- **A model's top-level `extra` reaches the model.** `models..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 diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index e36f52cc..ef03d4f4 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -196,10 +196,10 @@ The reply grows by N entries per input token. `extra` is a dict on a model that tasks read and the API never sees: a task 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, at its top level -or in `args` but not both, or per task in `infer_args`. 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. +below, so a typo fails before the run. Set it on a model (top level or +`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: diff --git a/sieval/cli/resolution.py b/sieval/cli/resolution.py index 3a6ad48b..9d320cc2 100644 --- a/sieval/cli/resolution.py +++ b/sieval/cli/resolution.py @@ -105,11 +105,10 @@ 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``. + """The ``extra`` a model config sets at its top level or in ``args``. - Both at once are refused: either one may be the value the author meant, - and ``extra`` enters the inference identity, so a silent pick would record - a capability set nobody chose. ``None`` when neither is set. + 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 = ( diff --git a/sieval/cli/session.py b/sieval/cli/session.py index 77c17ebe..6b138f5f 100644 --- a/sieval/cli/session.py +++ b/sieval/cli/session.py @@ -2097,6 +2097,7 @@ 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 = ( diff --git a/tests/unit/cli/test_resolution.py b/tests/unit/cli/test_resolution.py index fc2f76c4..2d069e62 100644 --- a/tests/unit/cli/test_resolution.py +++ b/tests/unit/cli/test_resolution.py @@ -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, @@ -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."""