Repository navigation
Package boundaries: training knows no AOP, rewrite knows no training; engine class, observable fan-out loop, rule-based gate - #15
Merged
Tyler-R-Kendrick merged 5 commits intoJul 12, 2026
Conversation
The training package defined MethodWeaver and SourcePromoter ports that structurally mirrored the rewrite API, so weaving, dispatch, hot-swap, and source-splicing concepts leaked into a package that should know nothing about them. Delete ports.ts and leave training with two seams in its own vocabulary: - captureTrainable(id, method, ...) routes one marked-method call through runtime capture; how calls get intercepted is not training's concern. - TrainingProviders.promote (PromotionApplier) applies a gate-approved candidate and returns how to undo it; activation checks the gate and delegates. Activation no longer exposes rewrite's PromotionResult. The root package is now the only place the siblings meet: it points the rewrite interceptor at captureTrainable (configureRewriteCapture), ships the digest-guarded applier with hot-swap (rewritePromotion), and hosts the trainable decorator plus load-time instrumentation helpers, which moved out of training. Also give the engine a real class: CandidateEngine owns request validation, implementation cleanup, TypeScript validation, and candidate identity as private methods, composing the injected TrainingEngine strategy. The pipeline formerly exposed as free functions (optimizeCandidate and friends) is internal; overrides remain pure composition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NG6fSKqgt8nD7JnEY9q245
|
Warning Review limit reached
Next review available in: 46 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR refactors training into observable fan-out rounds, replaces role-based harness dispatch with a gated append-only message bus, introduces guarded rewrite commit/revert APIs and composable promotion gates, and rewires root-package capture and promotion providers. ChangesTraining runtime and rewrite lifecycle
Harness integration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…romotion gate Loop: the default loop is now built on trainingRounds(), a cold observable round sequence — each subscription drives its own run, reviewed rounds are pushed to the observer as they settle, and unsubscribing aborts in-flight proposals and reviews through a derived AbortSignal. sequentialLoop collects the subscription into one TrainingLoopRun. Within a round, TrainingLoopInput.fanOut (and TrainInput.fanOut) caps how many candidate propose→review pipelines run concurrently: slots pipeline independently, duplicate proposals are skipped rather than re-reviewed, a round that reviews nothing new stalls the run, and when several candidates pass the gate the highest-scoring one is emitted last as the winner. ProposalTurn gains the 1-based slot so strategies can diversify concurrent proposals. Promotion: the gate is a rule set instead of cascading if statements. Each PromotionGate is a pure function over one shared PromotionGateContext (candidate, candidate-bound results, thresholds, aggregates) returning the failures it sees. The standard invariants live in defaultPromotionGates and always run; PromotionGateInput.gates (and TrainInput.gates) appends extension rules, and the configured policy participates as one more gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NG6fSKqgt8nD7JnEY9q245
Tyler-R-Kendrick
changed the title
ts-autocode-rewrite is abstract AOP and guarded source rewriting for
TypeScript; it carried training vocabulary anyway. promotion.ts defined
RewriteApproval (a structural subset of training's PromotionDecision)
and enforced the promotion gate inside promoteCandidate, and
RewriteCandidate carried a trainableId. All of it is gone:
- commitRewrite(source, candidate) applies a candidate and records a
RewriteSnapshot (rewriteId/targetId/updated); revertRewrite restores
it. Whether a candidate deserves committing is the consumer's call —
no decision parameter, no gate. promotion.ts is deleted; the guarded
replacement lives with applyCandidate in apply.ts.
- RewriteCandidate is { id, target, implementation }; error messages and
comments no longer mention trainables or training, and the rewrite
package's own tests use neutral markers.
The root package — the consumer — wires training promotion into its
rewrite usage: rewritePromotion now enforces the gate correspondence
(decision.promote and candidateId match) before committing, exactly
where training's decision meets the guarded rewrite. The training test
wiring mirrors the same check.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NG6fSKqgt8nD7JnEY9q245
Tyler-R-Kendrick
changed the title
…rness
The bus knew far too much: it enumerated the four roles, held an
ActionJudge, gated every dispatch through setJudge, and kept a private
judgeControl bootstrap path because a judge could not approve its own
actions. Now WriteAheadAgentBus is a plain durable append-only message
log: append({actor, kind, payload}) records a message with identity,
ordering, and time; read(actor?) returns the trailing window; an
optional allow hook decides whether an append or read may proceed. The
bus attaches no meaning to any actor.
The write-ahead convention moved into dispatchAction (dispatch.ts),
layered on top of the bus: record intent, ask the gate, record the
verdict — the judge is just another actor and its decision is an
ordinary agent.decision message — execute only after a pass, record the
outcome. defineTrainingHarness dispatches every student, teacher, and
adversary invocation through it with the run's judge callback as the
gate; MxcSandbox takes an actor string and an optional gate instead of
a role. No special judge path exists anywhere.
The harness also no longer creates agents: agents.ts (createTrainingAgents
with per-role Deep Agent models, system prompts, and role wiring) is
removed along with parseJudgeDecision. Callbacks are the whole contract,
which is what the documentation already claimed the harness was about.
The harness README, root README, and architecture doc are rewritten to
describe exactly this surface.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NG6fSKqgt8nD7JnEY9q245
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/training/src/loop.ts (1)
103-117: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueOptional: harden the floating
drivepromise against throwing observer callbacks.
drive()routes internal errors toobserver.error, but the wrappers here callobserver.next?./observer.complete?./observer.error?.directly. Sincedriveis launched asvoid drive(...), a throwing user-supplied callback (publictrainingRoundsAPI) surfaces as an unhandled rejection rather than being contained.sequentialLoop's own callbacks are safe, so this only matters for external subscribers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/training/src/loop.ts` around lines 103 - 117, Harden the observer callback wrappers in the floating drive invocation within trainingRounds so exceptions from external observer.next, observer.complete, or observer.error callbacks are contained rather than becoming unhandled rejections. Preserve the existing closed checks and settle behavior, while ensuring each user-supplied callback invocation is safely guarded.packages/harness/src/index.ts (1)
107-119: 🚀 Performance & Scalability | 🔵 TrivialEvery actor dispatch now also triggers a judge call for
subject: "action".Student/teacher/adversary calls are now all routed through the same
gate, so a round can invokeinput.judgeup to several times (once per actor dispatch, plus the existing candidate/adversary decisions) instead of just the candidate/adversary decisions. Ifjudgeis an expensive callback (e.g. an LLM call), custom implementations should short-circuitsubject === "action"the way the referencecreateHarnessLoopprovider does, or this refactor can noticeably increase judge invocation volume/cost per round.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/harness/src/index.ts` around lines 107 - 119, Update the judge handling in the harness loop around the action gate and actor dispatches so action-level evaluations do not invoke expensive judge logic by default; short-circuit requests with subject === "action" while preserving the existing candidate and adversary decision evaluations. Ensure custom judge behavior follows the reference createHarnessLoop provider’s action handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/harness/src/dispatch.ts`:
- Around line 41-50: Update dispatchAction’s error handling so failures while
appending .failed or .completed records never replace the original execution or
gate outcome. Keep execute() failures classified as execution failures, but
preserve and rethrow the original gate error when the gate failure record cannot
be appended; when execute() succeeds, retain and return its computed result even
if recording completion fails.
---
Nitpick comments:
In `@packages/harness/src/index.ts`:
- Around line 107-119: Update the judge handling in the harness loop around the
action gate and actor dispatches so action-level evaluations do not invoke
expensive judge logic by default; short-circuit requests with subject ===
"action" while preserving the existing candidate and adversary decision
evaluations. Ensure custom judge behavior follows the reference
createHarnessLoop provider’s action handling.
In `@packages/training/src/loop.ts`:
- Around line 103-117: Harden the observer callback wrappers in the floating
drive invocation within trainingRounds so exceptions from external
observer.next, observer.complete, or observer.error callbacks are contained
rather than becoming unhandled rejections. Preserve the existing closed checks
and settle behavior, while ensuring each user-supplied callback invocation is
safely guarded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ba79a8a3-363f-4c93-8a82-47b89b4fdcab
📒 Files selected for processing (38)
README.mddocs/architecture.mdpackages/harness/README.mdpackages/harness/src/agents.tspackages/harness/src/bus.tspackages/harness/src/dispatch.tspackages/harness/src/index.tspackages/harness/src/sandbox.tspackages/harness/test/harness.test.tspackages/rewrite/README.mdpackages/rewrite/src/apply.tspackages/rewrite/src/aspect.tspackages/rewrite/src/index.tspackages/rewrite/src/instrument.tspackages/rewrite/src/promotion.tspackages/rewrite/test/apply.test.tspackages/rewrite/test/instrument.test.tspackages/rewrite/test/weaving.test.tspackages/training/README.mdpackages/training/src/engine.tspackages/training/src/index.tspackages/training/src/loop.tspackages/training/src/ports.tspackages/training/src/promotion.tspackages/training/src/source.tspackages/training/src/training.tspackages/training/test/engine.test.tspackages/training/test/evaluation.test.tspackages/training/test/loop.test.tspackages/training/test/promotion.test.tspackages/training/test/training.test.tspackages/training/test/wiring.tssrc/index.tssrc/instrumentation.tssrc/providers/harness.tssrc/providers/rewrite.tssrc/register.tstest/instrumentation.test.ts
💤 Files with no reviewable changes (3)
- packages/rewrite/src/promotion.ts
- packages/training/src/ports.ts
- packages/harness/src/agents.ts
…vers Review follow-ups on the write-ahead dispatch and the observable loop: - dispatchAction: a successful execute() whose .completed append fails was recorded as .failed and its result discarded. Execution now has its own catch; the completion record is appended outside it, so a failing append surfaces as a bus error, never as a failed action. Failure-record appends in the gate and execution catches are best-effort so they cannot mask the original error. - trainingRounds: subscriber callbacks are guarded — the drive promise floats, so a throwing observer.next/complete/error would otherwise become an unhandled rejection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NG6fSKqgt8nD7JnEY9q245
Tyler-R-Kendrick
deleted the
claude/interface-redundancies-consolidate-8juzw3
branch
July 12, 2026 18:29
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
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
Boundary/design cleanups across
packages/training,packages/rewrite, andpackages/harness, with all cross-package wiring confined to the rootts-autocodepackage:Changes
Training package knows nothing about rewrite/AOP
ports.ts(theMethodWeaver/SourcePromotermirrors of the rewrite API) is deleted. Training exposes two seams in its own vocabulary:captureTrainable(...)andTrainingProviders.promote(aPromotionApplierwithrollback()). Thetrainabledecorator,wrapTrainable, andinstrumentTrainablemoved to the root package.Rewrite package knows nothing about training
promotion.tsis deleted:RewriteApprovaland the in-package gate check are gone, along withRewriteCandidate.trainableId. Rewrite is abstract AOP plus guarded rewriting:commitRewrite/revertRewritewithRewriteSnapshot; gating is the consumer's decision. The root package'srewritePromotionenforces the gate correspondence before committing, andconfigureRewriteCapture()points the interceptor atcaptureTrainable.Harness bus is a plain message log; the judge is not special
WriteAheadAgentBusno longer enumerates roles, holds anActionJudge, gates dispatches viasetJudge, or keeps a privatejudgeControlbootstrap path. It is a durable append-only message log:append({actor, kind, payload}),read(actor?), and an optionalallowhook deciding who may append or read.dispatchActionindispatch.ts): record intent → ask the gate → record the verdict (the judge is just another actor; its decision is an ordinaryagent.decisionmessage) → execute only on pass → record the outcome. The harness dispatches every actor invocation through it with the run's judge callback as the gate;MxcSandboxtakes anactorstring and optionalgateinstead of a role.agents.ts(createTrainingAgentswith per-role Deep Agent models, system prompts, and role wiring) andparseJudgeDecisionare removed. Callbacks are the whole contract. The harness README, root README, and architecture doc are rewritten to describe exactly this surface.A real engine class, composition over inheritance
CandidateEngineowns request validation, implementation cleanup, TypeScript validation, and candidate identity as private methods, composing the injectedTrainingEnginestrategy.Observable loop with fan-out
trainingRounds()is a cold observable round sequence;sequentialLoopcollects the subscription into one run.fanOutcaps concurrent candidate pipelines per round; duplicates are skipped, empty rounds stall, and the highest-scoring gated candidate wins.ProposalTurn.slotlets strategies diversify.Rule-based, extensible promotion gate
PromotionGaterules over one sharedPromotionGateContext; standard invariants indefaultPromotionGatesalways run,gatesappends extension rules, andpolicyparticipates as one more gate.Testing
npm run check(typecheck for all packages, full test suite — 77 tests across 15 files, and the package build) passes.🤖 Generated with Claude Code
https://claude.ai/code/session_01NG6fSKqgt8nD7JnEY9q245
Summary by CodeRabbit