diff --git a/cosmos_framework/configs/toml_config/sft_config_test.py b/cosmos_framework/configs/toml_config/sft_config_test.py index 8ff8f3f0..cd274fe9 100644 --- a/cosmos_framework/configs/toml_config/sft_config_test.py +++ b/cosmos_framework/configs/toml_config/sft_config_test.py @@ -101,6 +101,50 @@ def test_other_keys_still_emitted(self) -> None: assert "experiment=vision_sft_nano" in overrides assert any(o.startswith("optimizer.lr=") for o in overrides), overrides + @pytest.mark.parametrize( + "value", + ["20260913", "1e-4", "true", "null", "lr=1e-5", "run(v2)", "run[1]", "exp#3", "bob's run", "a\\'b", "ends\\"], + ) + def test_string_value_reaches_hydra_as_the_same_string(self, value: str) -> None: + """A TOML string must not be re-typed (int/float/bool/None) or rejected by the override grammar.""" + from hydra.core.override_parser.overrides_parser import OverridesParser + + raw = {"job": {"task": "vfm", "experiment": "vision_sft_nano", "name": value}} + (override,) = [o for o in build_hydra_overrides(raw) if o.startswith("job.name=")] + parsed = OverridesParser.create().parse_override(override).value() + assert isinstance(parsed, str) and parsed == value, override + + def test_string_list_items_reach_hydra_as_the_same_strings(self) -> None: + from hydra.core.override_parser.overrides_parser import OverridesParser + + warmup = ["480", "bob's"] + raw = { + "job": {"task": "vfm", "experiment": "vision_sft_nano"}, + "trainer": {"callbacks": {"compile_tokenizer": {"warmup_resolutions": warmup}}}, + } + (override,) = [o for o in build_hydra_overrides(raw) if "warmup_resolutions=" in o] + assert OverridesParser.create().parse_override(override).value() == warmup, override + + def test_non_string_values_keep_their_types(self) -> None: + """Numbers, bools, lists, ``${...}`` interpolation and the ``???`` skip are unchanged.""" + from hydra.core.override_parser.overrides_parser import OverridesParser + + raw = { + "job": {"task": "vfm", "experiment": "vision_sft_nano"}, + "optimizer": {"lr": 1.0e-5, "betas": [0.9, 0.99]}, + "trainer": {"max_iter": 200}, + "model": {"ema": {"enabled": False}, "tokenizer": {"vae_path": "${oc.env:WAN_VAE_PATH}"}}, + "checkpoint": {"load_path": "???"}, + } + overrides = OverridesParser.create().parse_overrides(build_hydra_overrides(raw)[1:]) + parsed = {o.key_or_group: o.value() for o in overrides} + assert parsed["optimizer.lr"] == 1.0e-5 + assert parsed["optimizer.betas"] == [0.9, 0.99] + assert parsed["trainer.max_iter"] == 200 and isinstance(parsed["trainer.max_iter"], int) + assert parsed["model.config.ema.enabled"] is False + assert parsed["model.config.tokenizer.vae_path"] == "${oc.env:WAN_VAE_PATH}" + assert "checkpoint.load_path" not in parsed + # --------------------------------------------------------------------------- # # 3. end-to-end load_experiment_from_toml on the shipped vision_sft_nano recipe # diff --git a/cosmos_framework/configs/toml_config/toml_config_helper.py b/cosmos_framework/configs/toml_config/toml_config_helper.py index 4d1535c5..719c3508 100644 --- a/cosmos_framework/configs/toml_config/toml_config_helper.py +++ b/cosmos_framework/configs/toml_config/toml_config_helper.py @@ -16,6 +16,7 @@ from __future__ import annotations +import re from typing import Any @@ -181,14 +182,19 @@ def _emit_with_remap( out.append(f"{'.'.join(new_path)}={_hydra_format(value)}") -def _hydra_format(v: Any, in_list: bool = False) -> str: - """Convert a Python value to a Hydra CLI override RHS. +def _hydra_quote(s: str) -> str: + """Single-quote a string for a Hydra override RHS. - ``in_list=True`` indicates the value is being emitted inside a list - literal (``[a,b,c]``); strings then get single-quoted unconditionally - so numeric-looking entries like ``"480"`` stay strings rather than - being coerced to int by Hydra's list parser. + Inside quotes Hydra keeps backslashes literally except where they precede a + quote or end the string, so only those runs are doubled; quotes are escaped. """ + s = re.sub(r"(\\*)'", lambda m: m.group(1) * 2 + "\\'", s) + s = re.sub(r"(\\+)$", lambda m: m.group(1) * 2, s) + return f"'{s}'" + + +def _hydra_format(v: Any) -> str: + """Convert a Python value to a Hydra CLI override RHS.""" if v is None: return "null" if isinstance(v, bool): @@ -196,17 +202,13 @@ def _hydra_format(v: Any, in_list: bool = False) -> str: if isinstance(v, (int, float)): return str(v) if isinstance(v, list): - return "[" + ",".join(_hydra_format(x, in_list=True) for x in v) + "]" + return "[" + ",".join(_hydra_format(x) for x in v) + "]" if isinstance(v, str): - # Inside a list literal, always quote so numeric-looking strings - # ("480") aren't parsed as int. At top level, quote only when the - # string contains characters Hydra would otherwise interpret — - # commas (sweep / list marker) or whitespace. Env-interpolation - # strings like ``${oc.env:NAME}`` are safe unquoted because Hydra - # recognizes the ``${...}`` form even with a colon inside. - if in_list or "," in v or " " in v: - return f"'{v}'" - return v + # Always quote. Hydra re-types an unquoted RHS, so a string field set to + # "20260913", "true" or "null" would arrive as int/bool/None, and one + # containing "=", "(", "[" or "#" is rejected by the override grammar. + # Quoted values still resolve ``${oc.env:NAME}`` interpolation. + return _hydra_quote(v) return str(v)