Context
PR #62 added a dry_run_role: str | None = None field to LLMClientConfig in clawloop/train.py:39-42 so that clawloop run --dry-run can identify which mock client to substitute for each role. This was chosen over the prior approach (mutating LLMClientConfig.model with a marker string) on reviewer feedback (PR #62 review comment 1) because mutating model could pollute downstream consumers like _build_entropic, which reads tc.model and propagates it into entropic_cfg.
The current design works and is well-tested (tests/test_cli.py::test_dry_run_role_field_survives_pydantic_copy), but it still has a minor smell flagged in self-review: a CLI testing affordance now lives on a public Pydantic model. Every model_dump_json() of a TrainConfig emits \"dry_run_role\": null for each LLM client. JSON Schema generators surface it. Future config-validation UIs see it.
Proposal
Move the role mapping into a side-channel owned entirely by clawloop/cli.py:
# in cli.py
from weakref import WeakKeyDictionary
_dry_run_roles: WeakKeyDictionary[LLMClientConfig, str] = WeakKeyDictionary()
_install_dry_run_clients populates this dict; the patched _make_llm_client reads from it via _dry_run_roles.get(cfg). This:
- Removes
dry_run_role from the public LLMClientConfig schema entirely
- Survives Pydantic
model_copy() the same way the field does, as long as the original object stays referenced (it does — config.llm_clients holds it)
- Keeps all dry-run vocabulary in
cli.py where it belongs
Trade-off: WeakKeyDictionary requires the cfg objects to be hashable. Pydantic v2 models are hashable by default, so this should work.
Acceptance
Related
Context
PR #62 added a
dry_run_role: str | None = Nonefield toLLMClientConfiginclawloop/train.py:39-42so thatclawloop run --dry-runcan identify which mock client to substitute for each role. This was chosen over the prior approach (mutatingLLMClientConfig.modelwith a marker string) on reviewer feedback (PR #62 review comment 1) because mutatingmodelcould pollute downstream consumers like_build_entropic, which readstc.modeland propagates it intoentropic_cfg.The current design works and is well-tested (
tests/test_cli.py::test_dry_run_role_field_survives_pydantic_copy), but it still has a minor smell flagged in self-review: a CLI testing affordance now lives on a public Pydantic model. Everymodel_dump_json()of aTrainConfigemits\"dry_run_role\": nullfor each LLM client. JSON Schema generators surface it. Future config-validation UIs see it.Proposal
Move the role mapping into a side-channel owned entirely by
clawloop/cli.py:_install_dry_run_clientspopulates this dict; the patched_make_llm_clientreads from it via_dry_run_roles.get(cfg). This:dry_run_rolefrom the publicLLMClientConfigschema entirelymodel_copy()the same way the field does, as long as the original object stays referenced (it does —config.llm_clientsholds it)cli.pywhere it belongsTrade-off:
WeakKeyDictionaryrequires the cfg objects to be hashable. Pydantic v2 models are hashable by default, so this should work.Acceptance
LLMClientConfigno longer carriesdry_run_rolemodel_dump_json()ofLLMClientConfigdoes not emit any dry-run markerstest_run_*_dry_run_*,test_dry_run_role_*_survives_pydantic_copy)Related
clawloop runas a TrainConfig wrapper #60 review comment 1 (gemini-code-assist) — the prior round