diff --git a/CHANGELOG.md b/CHANGELOG.md index ef83812d..defcb7ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -129,6 +129,9 @@ All notable changes to SkillOpt are documented here. This project adheres to value is rejected with a clear error. ### Fixed +- A flat `--cfg-options` key for an `env` setting (for example `workers=4`) + now overrides the YAML value with a structured config instead of being + silently replaced by it (#409). - Close four ways invalid evidence could reach the SkillOpt-Sleep gate: recall no longer brings a content twin of tonight's held-out task into training, a measured tool route no longer credits a self-reported `TOOL_CALL:` marker, diff --git a/skillopt/config.py b/skillopt/config.py index 12230d31..1b3f13f6 100644 --- a/skillopt/config.py +++ b/skillopt/config.py @@ -397,6 +397,14 @@ def apply_overrides(cfg: dict, overrides: list[str]) -> None: if structured and dotted: section, subkey = dotted.split(".", 1) cfg.setdefault(section, {})[subkey] = val + elif ( + structured + and isinstance(cfg.get("env"), dict) + and canonical in cfg["env"] + ): + # flatten_config copies env keys over top-level ones, so a + # top-level copy of an env setting would be ignored. + cfg["env"][canonical] = val else: cfg[canonical] = val diff --git a/tests/test_cfg_options_flat_env_key.py b/tests/test_cfg_options_flat_env_key.py new file mode 100644 index 00000000..68e586ae --- /dev/null +++ b/tests/test_cfg_options_flat_env_key.py @@ -0,0 +1,43 @@ +"""Regression: a flat ``--cfg-options`` key for an ``env`` setting must win. + +``apply_overrides`` stored an unmapped flat key such as ``workers=4`` at the +top level of a structured config. ``flatten_config`` copies top-level keys +first and the ``env`` section's keys after them, so the YAML's ``env.workers`` +silently replaced the command-line value. +""" +from __future__ import annotations + +from skillopt.config import flatten_config, load_config + + +def _write_config(tmp_path): + config = tmp_path / "config.yaml" + config.write_text( + "train:\n" + " batch_size: 8\n" + "env:\n" + " name: searchqa\n" + " workers: 24\n" + " max_turns: 1\n", + encoding="utf-8", + ) + return str(config) + + +def test_flat_override_of_env_key_wins_over_yaml(tmp_path): + cfg = load_config(_write_config(tmp_path), overrides=["workers=4", "max_turns=3"]) + flat = flatten_config(cfg) + assert flat["workers"] == 4 + assert flat["max_turns"] == 3 + + +def test_flat_and_dotted_env_overrides_agree(tmp_path): + path = _write_config(tmp_path) + flat_form = flatten_config(load_config(path, overrides=["workers=4"])) + dotted_form = flatten_config(load_config(path, overrides=["env.workers=4"])) + assert flat_form == dotted_form + + +def test_new_flat_key_still_reaches_the_trainer(tmp_path): + cfg = load_config(_write_config(tmp_path), overrides=["limit=16"]) + assert flatten_config(cfg)["limit"] == 16