refactor: apply OOP design principles to core design#8
Merged
shenxiangzhuang merged 2 commits intomasterfrom Mar 10, 2026
Merged
refactor: apply OOP design principles to core design#8shenxiangzhuang merged 2 commits intomasterfrom
shenxiangzhuang merged 2 commits intomasterfrom
Conversation
- SRP: decompose Castle.create() into _ensure_dirs(), _build_skill_manager(), and _build_channels() private helpers — each with a single responsibility - DRY + Delegation: extract Session._assemble() classmethod shared by Session.create() and Session.resume(), eliminating duplicated setup code - SRP + Delegation: extract _record_turn_end() helper from agent_loop() so trace recording is isolated from hook dispatch and loop orchestration Co-authored-by: shenxiangzhuang <17157965+shenxiangzhuang@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Refactor code to align with design principles
refactor: apply OOP design principles to core design
Mar 10, 2026
There was a problem hiding this comment.
Pull request overview
This PR refactors three frequently used orchestration paths to better separate responsibilities and remove duplicated assembly logic, improving readability without changing expected behavior.
Changes:
- Refactors
Castle.create()into a small orchestrator by extracting directory creation, skill manager construction, and channel construction helpers. - Removes duplicated
Session.create()/Session.resume()wiring by extracting shared construction intoSession._assemble(). - Extracts trace-recording from
agent_loop()into_record_turn_end()to keep the main loop focused on control flow and hooks.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
packages/kcastle/src/kcastle/session/session.py |
Introduces Session._assemble() to DRY up session construction in create() and resume(). |
packages/kcastle/src/kcastle/castle.py |
Decomposes Castle.create() into _ensure_dirs, _build_skill_manager, and _build_channels helpers. |
packages/kagent/src/kagent/loop.py |
Extracts per-turn trace appends and usage extraction into _record_turn_end() and simplifies usage accumulation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Applies SRP, DRY, and Delegation principles to reduce cognitive load in three high-traffic areas:
Castle.create(),Session, andagent_loop().Changes
Castle.create()— SRP decomposition (castle.py)A 70-line monolithic factory handling dirs, skill discovery, provider setup, channels, and session/model wiring. Extracted three focused private helpers:
create()is now a readable orchestrator; each helper carries a single responsibility.Session.create()/Session.resume()— DRY fix (session/session.py)Both classmethods ended with identical code: calling
agent_factory(trace)and constructingSession. Extracted into_assemble():agent_loop()— trace recording extraction (kagent/loop.py)The
TurnEndhandler mixed hook dispatch, trace appending, and usage accumulation. Extracted_record_turn_end(state, event, *, run_id, turn_index) -> TokenUsage | Noneto own the trace concern, keeping the loop focused on orchestration.🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.