refactor: replace monkey-patching with dependency injection in train() — closes #64

[dantp-ai]

Summary

Closes #64. train() now accepts keyword-only make_llm_client and env_builders parameters; the CLI dry-run path builds them as closures over the user's config and passes them in directly to train(...). This eliminates two pieces of implicit global state that the prior dry-run wiring had introduced:

• _train._make_llm_client = _mock_make (from PR fix: address PR #60 review comments on clawloop run --dry-run + taubench #62) — module-level reassignment.
• _train.ENV_BUILDERS[env_type] = stub_builder (from PR fix: address PR #60 review comments on clawloop run --dry-run + taubench #62) — dict mutation.

The _DryRunLLMClientConfig subclass added in PR #65 also goes away: with DI the role lookup lives in a closure created fresh per train() call, so no cfg tag is needed at all.

Why this is a real improvement, not just a refactor

The previous design had implicit contracts that DI removes:

1. Every dry-run caller depended on _make_llm_client being a module-level reassignable function. If train.py ever inlined it, moved it into a class, or decorated it, dry-run would silently stop mocking. New test test_dry_run_does_not_mutate_module_state asserts the no-mutation contract directly.
2. ENV_BUILDERS was mutated globally, so the change persisted for the process lifetime — parallel train() calls would have raced.
3. Tests resorted to with patch(\"clawloop.train._make_llm_client\") — unnecessary ceremony with DI. The TestTrainEndToEnd test in this PR drops the patch and passes make_llm_client=_pick_client directly.

What changed

File	Change
clawloop/train.py	train() gains *, make_llm_client=None, env_builders=None (defaults preserve current behavior). All six _build_* helpers accept make_llm_client as a third positional parameter — only _build_math uses it today; others accept it for signature uniformity and future use. Added LLMClientFactory = Callable[[LLMClientConfig], Any] type alias.
clawloop/cli.py	Deleted _DryRunLLMClientConfig and its LLMClientConfig import. Replaced _install_dry_run_clients (mutates module state) with _build_dry_run_overrides(config) (pure: returns (factory, builders)). cmd_run passes overrides into train(...). _StubAdapter and ENVS_USING_MAKE_LLM_CLIENT unchanged.
tests/test_train_config.py	TestTrainEndToEnd::test_harness_learning_math drops with patch(...) ceremony for direct DI.
tests/test_cli.py	Removed subclass round-trip test. Added three tests: test_dry_run_overrides_map_roles_correctly, test_dry_run_overrides_stub_bypassing_env, test_dry_run_does_not_mutate_module_state. Kept the schema-cleanliness test.
tests/unit/test_train.py	Builder invocations pass None as the third positional arg.

PR layering

This builds on #62 and #65 (both still open at time of writing). The diff against main will include all three PRs' commits until #62 and #65 merge. Suggested merge order: #62 → #65 → #64. Each PR's diff against the post-merge main is a clean forward-looking step.

If you'd rather skip #65 (since DI here removes both the field from #62 and the subclass from #65 anyway), you could merge #62 → #64 and close #65. Either path is reachable.

Test plan

• pre-commit run --all-files clean (ruff + ruff-format)
• pytest tests/test_cli.py tests/test_train_config.py tests/unit/test_train.py → 68 pass
• Full suite: 1046 passed, 49 skipped, 0 failed (+10 vs upstream main)
• clawloop run examples/configs/math_harness.json --dry-run exits 0 (real MathAdapter + injected mock factory)
• clawloop run examples/configs/taubench_harness.json --dry-run with OPENAI_API_KEY / ANTHROPIC_API_KEY / GEMINI_API_KEY unset exits 0 (stub adapter via injected env_builders override)
• clawloop eval still exits 2 with the truthful redirect
• test_dry_run_does_not_mutate_module_state directly asserts clawloop.train._make_llm_client and ENV_BUILDERS are unchanged after a dry-run

Out of scope (future follow-ups)

• Refactoring _build_entropic, _build_openclaw, _build_taubench, _build_openspiel to use the injected make_llm_client for their internal LLM construction. They accept the parameter for signature uniformity but still drive LLMs internally. Routing them through make_llm_client would let dry-run drop _StubAdapter for those envs and exercise the real adapter logic with mocks.
• Threading make_llm_client through Reflector, LocalEvolver, or other harness internals.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request refactors the --dry-run mechanism to use dependency injection instead of global monkey-patching, introducing a _StubAdapter to safely mock environments that bypass _make_llm_client (such as taubench, entropic, and openclaw). It also adds up-front validation for taubench configurations and includes comprehensive tests to verify dry-run behavior and configuration validation. The review feedback correctly highlights that passing None as the make_llm_client argument in several tests violates the expected non-optional LLMClientFactory type signature, and suggests replacing None with a dummy lambda to maintain type safety.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.