From 5963aa44b6b3cb6d325b775d9e60ff457e3c108a Mon Sep 17 00:00:00 2001 From: KraHsu Date: Fri, 22 May 2026 11:51:21 +0800 Subject: [PATCH] =?UTF-8?q?refactor:=20SimulationCfg.play=5Fretargeted=5Fk?= =?UTF-8?q?eys()=20owns=20the=20retarget=20list=20(ROADMAP=20=C2=A79=20PR?= =?UTF-8?q?=20R3.2=20/=20ADR-0005)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the play-mode shortcut-retargeting key list off the private `cli/__init__.py:_PLAY_RETARGETED_KEYS` constant onto `SimulationCfg.play_retargeted_keys()` (static method on the domain config). The CLI's `env.` → `play_env.` retarget loop in play mode now calls the method; the set of play-retargetable simulation override paths lives next to the SimulationCfg fields the --vis / --gpu / --steps / --dt shortcuts target. Returns the four paths verbatim (`env.simulation.{vis,gpu,steps,dt}`) so the CLI use-site is unchanged except for the constant → method swap — the lowest-risk faithful replacement per ADR-0005's framing. All four are real SimulationCfg fields (test_configs.py asserts this). Completes ADR-0005 / R3 (R3.1 EvalCallbackCfg.from_args shipped in PR #90; this is R3.2). New `tests/test_configs.py` (3 tests): exact ordered set, every key names a real SimulationCfg field, and class/instance call equivalence. Reference audit (per CLAUDE.md §9.1 rule 1): `_PLAY_RETARGETED_KEYS` had a single use site (the play-mode retarget loop); removed. `SimulationCfg` added to the existing module-top `from genelab.configs import …` line in cli/__init__.py (cli → configs is the allowed layering direction). Verified: ruff ✓, pyright 0/0/0, full suite 395 passed (was 392; +3 from test_configs.py), `genelab play --help` snapshots byte-identical (R0.1 gate), configs.py stays torch-free at import (invariant #5), lint-imports baseline unchanged at 2 kept / 2 broken. cli/__init__.py LoC: 1051 (refactor start) → 1020 after R3.1 + R3.2 (−31). ADR-0005 §10 estimated ≥35; the actual parse logic was a touch smaller. Both parsers (eval-callback args, play-retarget keys) are now domain-owned, which was the substantive goal. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 11 +++++++++++ src/genelab/cli/__init__.py | 12 ++---------- src/genelab/configs.py | 19 +++++++++++++++++++ tests/test_configs.py | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 10 deletions(-) create mode 100644 tests/test_configs.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 46360c36..82ba3f98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,17 @@ trajectory so breaking changes can land in any minor release until the 1.0 stabi ### Changed +- **Internal restructuring** (no behaviour change): the play-mode + shortcut-retargeting key list moved off the private + `cli/__init__.py:_PLAY_RETARGETED_KEYS` constant onto + `SimulationCfg.play_retargeted_keys()` (a static method on the domain + config in `genelab.configs`). The CLI's `env.` → `play_env.` retarget + loop now calls the method; the set of play-retargetable simulation + override paths (`env.simulation.{vis,gpu,steps,dt}`) lives next to the + `SimulationCfg` fields the `--vis` / `--gpu` / `--steps` / `--dt` + shortcuts target. `genelab play --help` is unchanged (R0.1 snapshot + gate green); `configs.py` stays torch-free at import (invariant #5). + Lands as ROADMAP §9 PR R3.2 — completes ADR-0005 (R3). - **Internal restructuring** (no behaviour change): `--eval-*` runner-arg parsing for in-training eval moved from `cli/__init__.py:_build_eval_callback` onto the domain config as `EvalCallbackCfg.from_args(runner_args) -> diff --git a/src/genelab/cli/__init__.py b/src/genelab/cli/__init__.py index 8bb93365..9990aa17 100644 --- a/src/genelab/cli/__init__.py +++ b/src/genelab/cli/__init__.py @@ -38,7 +38,7 @@ render_registry, ) from genelab.cli._scaffold import create_project_skeleton -from genelab.configs import apply_overrides +from genelab.configs import SimulationCfg, apply_overrides from genelab.registry import ( TASKS, load_bundled_asset_zoo, @@ -80,14 +80,6 @@ class _RegistryKindArg(str, Enum): _AGENT_KINDS: Final[frozenset[str]] = frozenset({"zero", "random", "trained"}) -_PLAY_RETARGETED_KEYS: Final[tuple[str, ...]] = ( - "env.simulation.vis", - "env.simulation.gpu", - "env.simulation.steps", - "env.simulation.dt", -) - - _RUN_FLAGS_HELP: Final[str] = """\ Shorthand flags rewritten into env overrides: @@ -556,7 +548,7 @@ def _configured_task( # task's play_env when one is configured. Keeps `genelab play TASK --vis` working # without forcing users to spell `play_env.simulation.vis`. if command == "play" and getattr(task.cfg, "play_env", None) is not None: - for short_key in _PLAY_RETARGETED_KEYS: + for short_key in SimulationCfg.play_retargeted_keys(): if short_key in overrides: overrides[short_key.replace("env.", "play_env.", 1)] = overrides.pop(short_key) diff --git a/src/genelab/configs.py b/src/genelab/configs.py index 09522684..5f876243 100644 --- a/src/genelab/configs.py +++ b/src/genelab/configs.py @@ -37,6 +37,25 @@ class SimulationCfg: # raise ``decimation`` if that's not what you want. render_fps: int | None = 60 + @staticmethod + def play_retargeted_keys() -> tuple[str, ...]: + """Override paths the CLI rewrites ``env.`` → ``play_env.`` in play mode. + + The ``--vis`` / ``--gpu`` / ``--steps`` / ``--dt`` shorthand flags expand + to ``env.simulation.`` overrides. When a task defines a separate + ``play_env``, ``genelab play TASK --vis`` should target *that* env, so the + CLI retargets these keys onto ``play_env.simulation.``. Owning the + list here (rather than as a private constant in ``cli/__init__.py``) keeps + the set of play-retargetable simulation overrides next to the fields + themselves (ADR-0005 / R3.2). + """ + return ( + "env.simulation.vis", + "env.simulation.gpu", + "env.simulation.steps", + "env.simulation.dt", + ) + @dataclass class InteractiveSceneCfg: diff --git a/tests/test_configs.py b/tests/test_configs.py new file mode 100644 index 00000000..800eb4d0 --- /dev/null +++ b/tests/test_configs.py @@ -0,0 +1,36 @@ +"""Tests for ``genelab.configs`` domain-config helpers. + +Currently covers ``SimulationCfg.play_retargeted_keys`` (ROADMAP §9 R3.2 / +ADR-0005) — the set of override paths the CLI rewrites ``env.`` → +``play_env.`` in play mode. The list moved off a private constant in +``cli/__init__.py`` onto the domain config so it lives next to the +``SimulationCfg`` fields the shortcut flags target. +""" + +from __future__ import annotations + +from genelab.configs import SimulationCfg + + +def test_play_retargeted_keys_exact_set() -> None: + """The four ``env.simulation.*`` shortcut-override paths, verbatim and ordered.""" + assert SimulationCfg.play_retargeted_keys() == ( + "env.simulation.vis", + "env.simulation.gpu", + "env.simulation.steps", + "env.simulation.dt", + ) + + +def test_play_retargeted_keys_target_real_simulation_fields() -> None: + """Every retargeted key names an actual ``SimulationCfg`` field (no stale paths).""" + fields = SimulationCfg.__dataclass_fields__ + for key in SimulationCfg.play_retargeted_keys(): + assert key.startswith("env.simulation."), key + field_name = key.rsplit(".", 1)[1] + assert field_name in fields, f"{field_name!r} is not a SimulationCfg field" + + +def test_play_retargeted_keys_callable_on_class_and_instance() -> None: + """Static method — same result whether called on the class or an instance.""" + assert SimulationCfg.play_retargeted_keys() == SimulationCfg().play_retargeted_keys()