Skip to content

Replace monkey-patching of train._make_llm_client with dependency injection #64

Description

@dantp-ai

Context

PR #62 wires clawloop run --dry-run by mutating module-level state in clawloop/train.py:

# clawloop/cli.py:_install_dry_run_clients
_train._make_llm_client = _mock_make
_train.ENV_BUILDERS[env_type] = _stub_builder

This works for the CLI's one-shot process model but creates implicit contracts that future refactors can silently break:

  1. Every dry-run caller depends on _make_llm_client being a module-level reassignable function. If train.py ever inlines it, moves it into a class, or wraps it in a decorator, dry-run silently stops mocking.
  2. ENV_BUILDERS is mutated globally, so the modification persists for the lifetime of the process. Two dry-run runs in the same process would patch on top of each other.
  3. The next "intercept this" feature (judge-LLM mocking, deterministic seeds, etc.) will follow this monkey-patch precedent and compound the problem.

Proposal

Replace the monkey-patching with explicit dependency injection through train():

def train(
    config: TrainConfig,
    *,
    make_llm_client: Callable[[LLMClientConfig], LLMClient] = _make_llm_client,
    env_builders: dict[str, EnvBuilder] = ENV_BUILDERS,
) -> tuple[AgentState, StateID]:
    ...

Each _build_* helper takes make_llm_client as a parameter instead of importing it from module scope. The CLI's cmd_run builds a mock factory and an overridden builder dict, then calls train(config, make_llm_client=mock_factory, env_builders=stub_builders).

Benefits:

  • No global mutation; multiple parallel train() calls are safe
  • The injection points are documented in the train() signature — discoverable via type hints / docs
  • Tests that already do with patch(\"clawloop.train._make_llm_client\") as mock_make (tests/test_train_config.py::TestTrainEndToEnd) can switch to passing the mock directly
  • Future intercepts (judge LLM, seeds, sampling client) follow the same pattern: add a kwarg, default to the production thing, document

Scope

  • Modifies the public signature of train(). All current call sites (examples/train_runner.py, clawloop/cli.py:cmd_run, tests/test_train_config.py::TestTrainEndToEnd) need updating.
  • Each _build_* helper in clawloop/train.py (~6 functions) takes the factory as a parameter.
  • Deletes _install_dry_run_clients's monkey-patching block; the CLI builds factories and passes them in.
  • _StubAdapter stays as-is; ENVS_USING_MAKE_LLM_CLIENT becomes a property of how the CLI builds the override dict.

Acceptance

  • train() accepts make_llm_client and env_builders as keyword-only parameters with sensible defaults
  • No code outside train.py mutates train._make_llm_client or train.ENV_BUILDERS
  • test_dry_run_role_* and existing TestTrainEndToEnd patterns work without unittest.mock.patch
  • All existing --dry-run smoke tests still pass
  • examples/train_runner.py (the deprecated shim) keeps working unchanged via the CLI passthrough

Related

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions