refactor: extract helpers from learning_loop — fixes #39#58
Merged
bordeauxred merged 1 commit intoApr 21, 2026
Merged
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 refactors the learning_loop by extracting its core logic into specialized components: ArchiveRecorder for metrics and logging, EpisodeCollectorRunner for collection, and LayerTransaction for the training protocol. New unit tests are included for these modules. Feedback identifies a bug in ArchiveRecorder where cost tracking is incorrectly nested within a logging try-block, and notes an encapsulation violation in LayerTransaction regarding private harness state.
kiranannadatha8
force-pushed
the
39-extract-loop-helpers
branch
from
d9bb8e2 to
dc738b8
Compare
Split the 486-line learning_loop() god-function into three focused
helper classes plus five module-private glue functions.
New modules:
- clawloop/core/runner.py (EpisodeCollectorRunner — task sampling
+ 3-way adapter dispatch)
- clawloop/core/archive_recorder.py (ArchiveRecorder — owns run-level
counters + all archive writes)
- clawloop/core/transaction.py (LayerTransaction — two-phase
fb→optim→rollback protocol with cross-layer rollback invariant)
learning_loop() body: 486 → 100 lines. loop.py total: 742 → 337 lines.
Also:
- Add Harness.pending_paradigm_insights() so LayerTransaction can
query paradigm-tagged insights without touching _pending directly.
- Move iter_cost accumulation outside the archive try-block in
ArchiveRecorder so total_cost_tokens is tracked even when
log_iteration fails.
No public API change on learning_loop. Full suite unchanged
(961 passed / 42 skipped) plus 28 new unit tests covering the
extracted helpers (runner 97%, archive_recorder 92%,
transaction 91% coverage).
kiranannadatha8
force-pushed
the
39-extract-loop-helpers
branch
from
dc738b8 to
03a9301
Compare
Contributor
|
tested and run LLM end-to-end against Gemini: Thank you @kiranannadatha8! |
dantp-ai
pushed a commit
to dantp-ai/clawloop
that referenced
this pull request
Apr 24, 2026
…nthos#58) Split the 486-line learning_loop() god-function into three focused helper classes plus five module-private glue functions. New modules: - clawloop/core/runner.py (EpisodeCollectorRunner — task sampling + 3-way adapter dispatch) - clawloop/core/archive_recorder.py (ArchiveRecorder — owns run-level counters + all archive writes) - clawloop/core/transaction.py (LayerTransaction — two-phase fb→optim→rollback protocol with cross-layer rollback invariant) learning_loop() body: 486 → 100 lines. loop.py total: 742 → 337 lines. Also: - Add Harness.pending_paradigm_insights() so LayerTransaction can query paradigm-tagged insights without touching _pending directly. - Move iter_cost accumulation outside the archive try-block in ArchiveRecorder so total_cost_tokens is tracked even when log_iteration fails. No public API change on learning_loop. Full suite unchanged (961 passed / 42 skipped) plus 28 new unit tests covering the extracted helpers (runner 97%, archive_recorder 92%, transaction 91% coverage).
bordeauxred
pushed a commit
that referenced
this pull request
May 3, 2026
* feat: Weights & Biases sink integration * refactor: extract common base for purple-agent adapters (#40) (#51) * refactor: extract _PurpleAgentBase for CAR + entropic adapters — fixes #40 Pulls the shared A2A scaffolding (tool schema conversion, assistant-msg normalization, session state, tool-call id reconciliation, harness update) into clawloop/environments/_purple_base.py. CAR and entropic adapters now override only the two bench-specific seams: _build_initial_messages and _format_a2a_response. No behavior change. 600 lines deleted, 379 added across the three files. * refactor: hoist stdlib imports in _purple_base, clarify reconcile scope Addresses Gemini review on #51: - Move socket, time, httpx to module-level imports (PEP 8). - Expand docstring + comment on _reconcile_tool_call_id to explain why it intentionally stops at the most-recent assistant message. No behavior change. * refactor: extract helpers from learning_loop — fixes #39 (#58) Split the 486-line learning_loop() god-function into three focused helper classes plus five module-private glue functions. New modules: - clawloop/core/runner.py (EpisodeCollectorRunner — task sampling + 3-way adapter dispatch) - clawloop/core/archive_recorder.py (ArchiveRecorder — owns run-level counters + all archive writes) - clawloop/core/transaction.py (LayerTransaction — two-phase fb→optim→rollback protocol with cross-layer rollback invariant) learning_loop() body: 486 → 100 lines. loop.py total: 742 → 337 lines. Also: - Add Harness.pending_paradigm_insights() so LayerTransaction can query paradigm-tagged insights without touching _pending directly. - Move iter_cost accumulation outside the archive try-block in ArchiveRecorder so total_cost_tokens is tracked even when log_iteration fails. No public API change on learning_loop. Full suite unchanged (961 passed / 42 skipped) plus 28 new unit tests covering the extracted helpers (runner 97%, archive_recorder 92%, transaction 91% coverage). * fix: sync log_iteration with self._step * Ignore wandb dir --------- Co-authored-by: kiranannadatha8 <87536091+kiranannadatha8@users.noreply.github.com>
bordeauxred
pushed a commit
that referenced
this pull request
May 22, 2026
Split the 486-line learning_loop() god-function into three focused
helper classes plus five module-private glue functions.
New modules:
- clawloop/core/runner.py (EpisodeCollectorRunner — task sampling
+ 3-way adapter dispatch)
- clawloop/core/archive_recorder.py (ArchiveRecorder — owns run-level
counters + all archive writes)
- clawloop/core/transaction.py (LayerTransaction — two-phase
fb→optim→rollback protocol with cross-layer rollback invariant)
learning_loop() body: 486 → 100 lines. loop.py total: 742 → 337 lines.
Also:
- Add Harness.pending_paradigm_insights() so LayerTransaction can
query paradigm-tagged insights without touching _pending directly.
- Move iter_cost accumulation outside the archive try-block in
ArchiveRecorder so total_cost_tokens is tracked even when
log_iteration fails.
No public API change on learning_loop. Full suite unchanged
(961 passed / 42 skipped) plus 28 new unit tests covering the
extracted helpers (runner 97%, archive_recorder 92%,
transaction 91% coverage).
bordeauxred
pushed a commit
that referenced
this pull request
May 22, 2026
* feat: Weights & Biases sink integration * refactor: extract common base for purple-agent adapters (#40) (#51) * refactor: extract _PurpleAgentBase for CAR + entropic adapters — fixes #40 Pulls the shared A2A scaffolding (tool schema conversion, assistant-msg normalization, session state, tool-call id reconciliation, harness update) into clawloop/environments/_purple_base.py. CAR and entropic adapters now override only the two bench-specific seams: _build_initial_messages and _format_a2a_response. No behavior change. 600 lines deleted, 379 added across the three files. * refactor: hoist stdlib imports in _purple_base, clarify reconcile scope Addresses Gemini review on #51: - Move socket, time, httpx to module-level imports (PEP 8). - Expand docstring + comment on _reconcile_tool_call_id to explain why it intentionally stops at the most-recent assistant message. No behavior change. * refactor: extract helpers from learning_loop — fixes #39 (#58) Split the 486-line learning_loop() god-function into three focused helper classes plus five module-private glue functions. New modules: - clawloop/core/runner.py (EpisodeCollectorRunner — task sampling + 3-way adapter dispatch) - clawloop/core/archive_recorder.py (ArchiveRecorder — owns run-level counters + all archive writes) - clawloop/core/transaction.py (LayerTransaction — two-phase fb→optim→rollback protocol with cross-layer rollback invariant) learning_loop() body: 486 → 100 lines. loop.py total: 742 → 337 lines. Also: - Add Harness.pending_paradigm_insights() so LayerTransaction can query paradigm-tagged insights without touching _pending directly. - Move iter_cost accumulation outside the archive try-block in ArchiveRecorder so total_cost_tokens is tracked even when log_iteration fails. No public API change on learning_loop. Full suite unchanged (961 passed / 42 skipped) plus 28 new unit tests covering the extracted helpers (runner 97%, archive_recorder 92%, transaction 91% coverage). * fix: sync log_iteration with self._step * Ignore wandb dir --------- Co-authored-by: kiranannadatha8 <87536091+kiranannadatha8@users.noreply.github.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
Pure refactor of clawloop/core/loop.py — no public API change, no behavioral drift. Splits the 486-line
learning_loop()god-function into three focused helper classes plus five module-private glue functions. Full suite unchanged (960 passed / 42 skipped), plus 27 new unit tests covering the extracted helpers.What moved
EpisodeCollectorRunner— task sampling + 3-way adapter dispatchArchiveRecorder— owns run-level counters + all archive writesLayerTransaction— two-phase fb→optim→rollback protocolFive module-private helpers stay in
loop.py(each <40 lines, single-use):_avg_reward,_set_evolver_context,_flush_generation_if_advanced,_log_evolution_entry,_run_after_iteration.Before / After
loop.pytotal lineslearning_loop()bodyAcceptance criterion from #39 (
learning_loop ≤ 100 lines) met exactly.Test plan
uv run pytest -q→ 960 passed, 42 skipped (matches baseline)tests/unit/core/test_runner.py(8),test_archive_recorder.py(11),test_transaction.py(8)test_transaction.py::test_optim_error_rolls_back_all_snapshotted_layerstest_paradigm_shift_on_harness_fb_mutates_tried_paradigmstest_archive_integration.pygreenclawloop/__init__.py,clawloop/core/__init__.pyuntouched; all 11learning_loopcall-sites unchangedruff check clawloop/core/clean