Split wiring.go builders and eliminate ContextManager state sync dance - #174
Merged
Conversation
|
Warning Review limit reached
Next review available in: 19 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 (25)
📝 WalkthroughWalkthroughThe change extracts session wiring into prompt, MCP, and OpenTelemetry helpers. ContextManager now shares mutable LoopState with Loop for messages and compaction data. Compaction callers no longer synchronize duplicate state. ChangesSession wiring and context management
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Task #11: Split newAgentSessionWithOptions into focused builders - wiring_otel.go: initOtel + wrapProviderWithOtel (extracted inline provider wrapping) - wiring_mcp.go: initMCP (moved to its own file) - wiring_prompt.go: buildSystemPrompt + buildMainPrompt (extracted inline prompt layers, memory enrichment, guidelines, directive injection, and quick-ref assembly) - wiring.go slimmed from 396 to ~210 lines; removed dead layers.Skills assignment (was set after prompts.Build, never read) Task #12: Complete ContextManager extraction — eliminate state sync dance - Added State *LoopState pointer to ContextManager; compaction methods now read/write mutable state (Messages, PreviousSummary, token counts, compaction tracking) directly through the pointer instead of copy-in/copy-out - Removed 9 duplicate state fields from ContextManager (Messages, PreviousSummary, LastPromptTokens, LastCachedPromptTokens, IneffectiveCompactions, LastCompactionTokens, CompactionBudgetMultiplier, CompactionSavingsHistory, CompactionForcedByOverflow) - Eliminated 19-line sync dance in Loop.compactContext and 3-line dance in Loop.trimContext - Removed redundant CtxMgr.Messages assignments in loop.go, tools.go, turn.go (now no-ops since CtxMgr.State points to Loop.State) - ctxMgr() lazily sets State = &l.State when nil (backward compatible with tests)
buchenberg
force-pushed
the
feat/split-wiring-context-manager
branch
from
August 7, 2026 02:32
b7cab7d to
dea6353
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@cmd/yaah/wiring_otel.go`:
- Around line 17-19: Update the OTel enablement logic in the wiring function
around skipOtel and the early return so YAAH_OTEL_ENABLED=true is evaluated
before the disabled-configuration check. Ensure the environment flag can enable
OTel even when cfg.Observability.Otel.Enabled is false and extraOtelProcessors
is empty, while preserving the existing skipOtel behavior.
In `@cmd/yaah/wiring_prompt.go`:
- Around line 13-21: Move the package-level memoryGuidelines constant into the
buildSystemPrompt function scope, or have a helper return its value. Preserve
the existing prompt text and appending behavior while removing the global
declaration.
- Around line 31-72: Move the private prompt-building implementations used by
buildSystemPrompt and buildMainPrompt from cmd/yaah/wiring_prompt.go (lines
31-72) into a focused internal package or file, leaving only command composition
in cmd/yaah and preserving their current behavior and call sites. Apply the same
boundary change to MCP initialization in cmd/yaah/wiring_mcp.go (lines 17-40)
and OTel initialization/provider instrumentation in cmd/yaah/wiring_otel.go
(lines 16-64); each site requires moving its implementation behind internal
while retaining equivalent functionality.
In `@internal/agent/lifecycle_init.go`:
- Around line 51-56: Update the lazy initialization path in ctxMgr() to set
CompactionBudgetMultiplier to 1.0 when initializing CtxMgr.State, ensuring
Loop.Compact has a nonzero preservation budget before applyDefaults() runs.
🪄 Autofix
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: eb5705b6-14d9-4440-8b88-c3d7fbdc14a6
📒 Files selected for processing (10)
cmd/yaah/wiring.gocmd/yaah/wiring_mcp.gocmd/yaah/wiring_otel.gocmd/yaah/wiring_prompt.gointernal/agent/agent_context.gointernal/agent/context_manager.gointernal/agent/lifecycle_init.gointernal/agent/loop.gointernal/agent/tools.gointernal/agent/turn.go
💤 Files with no reviewable changes (1)
- internal/agent/turn.go
Comment on lines
+31
to
+72
| func buildSystemPrompt(cfg *config.Config, cwd string, db *memory.DB, resumeSessionID string) string { | ||
| layers := prompts.Layers{ | ||
| Identity: prompts.IdentityPrompt, | ||
| Environment: prompts.DetectEnvironment(cwd), | ||
| UserContext: prompts.LoadUserContext(config.HomeDir()), | ||
| Project: instructions.FormatForSystem(instructions.Load(cwd, cwd)), | ||
| MaxSubAgentConcurrency: cfg.Agent.SubAgent.MaxConcurrency, | ||
| } | ||
|
|
||
| if db != nil { | ||
| if entries, memErr := db.ListMemory(50); memErr == nil && len(entries) > 0 { | ||
| var memLines []string | ||
| for _, entry := range entries { | ||
| if strings.Contains(entry.Tags, `"user_info"`) { | ||
| continue | ||
| } | ||
| memLines = append(memLines, "- "+entry.Text) | ||
| } | ||
| if len(memLines) > 0 { | ||
| layers.Memory = "You have the following stored information about the user and project:\n" + strings.Join(memLines, "\n") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| systemPrompt := prompts.Build(layers) | ||
| if db != nil && resumeSessionID == "" { | ||
| systemPrompt += memoryGuidelines | ||
| } | ||
|
|
||
| return systemPrompt | ||
| } | ||
|
|
||
| // buildMainPrompt derives the top-level agent prompt from the system prompt | ||
| // by injecting session directives after the identity block and appending the | ||
| // tool quick-reference card. The systemPrompt stays clean so child sub-agent | ||
| // prompts never inherit top-level directives. | ||
| func buildMainPrompt(cfg *config.Config, systemPrompt string, toolReg *tools.Registry) string { | ||
| mainPrompt := prompts.InjectAfterIdentity(systemPrompt, resolveDirectives(cfg)) | ||
| if quickRef := buildToolQuickRef(toolReg); quickRef != "" { | ||
| mainPrompt += "\n\n" + quickRef | ||
| } | ||
| return mainPrompt |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Move the extracted private builders under internal/.
These new helpers are private implementation code under cmd/yaah. Keep command composition in cmd/yaah and move the implementations to focused internal/ packages or files.
cmd/yaah/wiring_prompt.go#L31-L72: move prompt construction behind an internal package boundary.cmd/yaah/wiring_mcp.go#L17-L40: move MCP initialization behind an internal package boundary.cmd/yaah/wiring_otel.go#L16-L64: move OTel initialization and provider instrumentation behind an internal package boundary.
📍 Affects 3 files
cmd/yaah/wiring_prompt.go#L31-L72(this comment)cmd/yaah/wiring_mcp.go#L17-L40cmd/yaah/wiring_otel.go#L16-L64
🤖 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 `@cmd/yaah/wiring_prompt.go` around lines 31 - 72, Move the private
prompt-building implementations used by buildSystemPrompt and buildMainPrompt
from cmd/yaah/wiring_prompt.go (lines 31-72) into a focused internal package or
file, leaving only command composition in cmd/yaah and preserving their current
behavior and call sites. Apply the same boundary change to MCP initialization in
cmd/yaah/wiring_mcp.go (lines 17-40) and OTel initialization/provider
instrumentation in cmd/yaah/wiring_otel.go (lines 16-64); each site requires
moving its implementation behind internal while retaining equivalent
functionality.
Source: Coding guidelines
… multiplier init - wiring_otel.go: evaluate YAAH_OTEL_ENABLED before the early return so the env flag can enable OTel when config disables it (pre-existing bug) - wiring_prompt.go: move memoryGuidelines from package-level const into buildSystemPrompt function scope (no globals per AGENTS.md) - lifecycle_init.go: initialize CompactionBudgetMultiplier=1.0 in ctxMgr() lazy path so Loop.Compact has a nonzero preservation budget before applyDefaults() runs
Delete plan files for fully-implemented features (max iterations dialog, TUI-MCP bridge, quiet mode, task pane separation) and the four ADRs (engine-view separation, middleware pipeline, functional options, event-driven architecture) whose content is now covered by docs/architecture.md. Update doc references across CONTRIBUTING.md, architecture.md, and code-organization.md to point at architecture.md in place of the retired ADRs, and fix stale file paths to reflect the recent runner refactor (cmd/yaah/subagent_runner.go -> internal/agent/runner/).
- Deleted 4 implemented ADRs (0001-0004: engine-view separation, middleware pipeline, functional options, event-driven architecture) — all Accepted and fully implemented; content covered by architecture.md - Deleted 5 implemented plan docs from .agents/plans/ (tui-mcp-bridge #159, tui-quiet-mode #159, tui2-task-pane-separation #160, max-iterations-dialog, web-ui-commands #162) — all features shipped - Updated docs/adr/README.md and CONTRIBUTING.md to remove dead ADR links - Fixed stale references to cmd/yaah/subagent_runner.go in architecture.md, PROMPT-INJECTION.md, and code-organization.md (now internal/agent/runner)
This was referenced Aug 7, 2026
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
1 participant
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.
Task #11: Split newAgentSessionWithOptions into focused builders
Task #12: Complete ContextManager extraction — eliminate state sync dance
Summary by CodeRabbit
New Features
Bug Fixes