chore: reintroduce clawloop run as a TrainConfig wrapper#60
Merged
bordeauxred merged 1 commit intoMay 21, 2026
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates the unified training runner into the clawloop CLI via a new run subcommand and adds support for the taubench environment. It also introduces a --dry-run flag to facilitate testing with mock LLM clients. The review feedback highlights that the current dry-run implementation may not cover environments that bypass the central LLM client factory and suggests more robust role identification and stricter configuration validation for the newly added environment.
bordeauxred
added a commit
that referenced
this pull request
May 22, 2026
…s-a-trainconfig-wrapper chore: reintroduce `clawloop run` as a TrainConfig wrapper
This was referenced Jun 10, 2026
dantp-ai
added a commit
that referenced
this pull request
Jun 12, 2026
…nch (#62) * fix: address PR #60 review comments on `clawloop run --dry-run` and taubench PR #60 review (gemini-code-assist) flagged four issues; this commit addresses all four with tests. 1. `_install_dry_run_clients` only patched `_make_llm_client`, so envs that bypass it (entropic, openclaw, taubench) could still make real API calls under `--dry-run`. The function now also swaps the env's builder in `ENV_BUILDERS` for a stub adapter whose `run_episode` / `run_batch` return canned episodes — no I/O, no LLM calls. Math keeps the real adapter path since `MathAdapter` routes through `_make_llm_client`. 2. Role identification used `id(cfg)`, which breaks if Pydantic copies or revalidates the `LLMClientConfig`. Replaced with a marker baked into `cfg.model`; the marker travels in field data so it survives `model_copy()`. New regression test covers a `model_copy()` round-trip. 3. `_build_taubench` return type widened from bare `tuple` to `tuple[Any, list[str]]` to match the rest of the builder family and enable static analysis. 4. `validate_config` now enforces taubench env_config: `env_config` required; `num_tasks` / `max_steps` / `max_concurrency` (if set) must be positive ints — `0` is no longer silently accepted via falsiness. Six new validation tests. Verification: 1036 + 49 = full suite passes (24s); manual smoke runs on math, taubench (no API keys set), and a bad-config rejection all behave as expected. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * style: ruff-format clawloop/train.py Collapses a short `raise ValueError(...)` to a single line. Fix for the lint CI on PR #62; no behavior change. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: address PR #62 review comments (dry_run_role field, strict int check) Follow-up review on PR #62 raised two concerns; both are now fixed. 1. Storing the dry-run role by overloading `LLMClientConfig.model` could pollute downstream consumers that read the field as a real model name (e.g. `_build_entropic` propagates `tc.model` into `entropic_cfg`). Replaced the marker-in-model trick with a dedicated `dry_run_role: str | None = None` field on `LLMClientConfig`. The field travels with the data, so it survives `model_copy()` like the marker did, but leaves `model` untouched. Test now asserts both the field round-trip AND that `model` is unmodified after `_install_dry_run_clients`. 2. `_positive_int` in `validate_config` accepted `bool` and `float` silently because `int(True) == 1` and `int(3.5) == 3`. Added an explicit `isinstance(v, (bool, float))` reject before the `int(v)` coercion. Six new tests cover bool / float rejection on `num_tasks`, `max_steps`, and `max_concurrency`. Verification: pre-commit clean; 56 passes in test_cli.py + test_train_config.py. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor: tighten dry-run wiring post-review Self-review follow-ups on the dry-run code added by the prior two commits. No behavior change for any existing test (56 still pass). - Move ENVS_USING_MAKE_LLM_CLIENT into clawloop/train.py next to ENV_BUILDERS, where the knowledge actually lives. Add a maintenance note: when registering a new builder, decide whether it routes through _make_llm_client and add the env_type if so; otherwise --dry-run falls back to _StubAdapter. Eliminates cross-module information leakage (cli.py asserting a property of train.py). - Collapse _make_stub_env_builder + _stub_episode into _StubAdapter methods. The factory was used in exactly one place; the helper had one caller. Two entities instead of three; depth unchanged. - Stub task count now tracks max(1, config.episodes_per_iter) instead of a magic 3, so --dry-run matches what the learning loop wants. - Drop default= args from validate_config._positive_int calls; TauBenchAdapter.setup already owns those defaults. Validation now only checks values the user supplied — removes duplicated knowledge and simplifies the helper signature. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What changed and why.
Test plan
pytest tests/ -xpasses