From e2e7a272d25743088fb08ccebe103ba64026e360 Mon Sep 17 00:00:00 2001 From: 1AbdullahButt <168034895+1AbdullahButt@users.noreply.github.com> Date: Thu, 12 Nov 2026 17:05:30 +0500 Subject: [PATCH] fix(config): let a flat --cfg-options key override an env setting With a structured config, apply_overrides stored an unmapped flat key such as workers=4 at the top level. 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. When the env section already has the key, write the override there. Fixes #409 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 ++ skillopt/config.py | 8 +++++ tests/test_cfg_options_flat_env_key.py | 43 ++++++++++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 tests/test_cfg_options_flat_env_key.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ef83812d9..defcb7ee8 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 12230d31e..1b3f13f64 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 000000000..68e586ae0 --- /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