feat: wire up proactive memory compaction system#14524
Closed
battman21 wants to merge 6 commits into
Closed
Conversation
…dback (openclaw#14374) - Add warning message when audio transcription is skipped due to no models - Add verbose logging throughout auto-detection flow - Help users understand why transcription isn't working - Guide users to docs for setup instructions Fixes openclaw#14374 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add modelId field to before_agent_start hook event and result - Call hook before model selection in agent runner - Allow plugins to route messages to different models based on content - Support both provider/model and model formats for routing - Add verbose logging when routing occurs Fixes openclaw#14150 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…penclaw#14210) - Add isError, errorType, and originalError fields to PluginHookMessageSendingEvent - Add classifyErrorMessage() helper to categorize errors - Supports error types: rate_limit, overload, auth, network, provider, unknown - Enhanced documentation for hook result fields This provides the infrastructure for plugins to detect and suppress error messages. Note: The message_sending hook exists but is not yet integrated into the delivery pipeline - integration needed in follow-up. Partial fix for openclaw#14210 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…penclaw#14210) - Document how to use message_sending hook for error suppression - Provide examples: suppress rate limits, customize error messages, silent mode - Explain error classification types and hook result options - Note integration status: infrastructure ready, delivery integration pending Plugins can now intercept and suppress/modify error messages once the hook is called from the message delivery pipeline. Addresses openclaw#14210 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
## Problem Memory compaction infrastructure existed but was never triggered: - shouldRunMemoryFlush() implemented but never called - compactionCount never incremented - Memory flush prompt system orphaned - Only reactive compaction (after overflow) worked Result: Users hit "Context overflow" errors when proactive compaction should have prevented them. ## Solution Implemented complete proactive memory flush system: **1. Created memory-flush-runner.ts** - runMemoryFlushIfNeeded() checks threshold after each turn - Uses shouldRunMemoryFlush() to detect when approaching limit - Runs compaction before context overflow occurs - Tracks compactionCount and memoryFlushCompactionCount **2. Integrated into agent runner (run.ts)** - Proactive check: After successful turns, before return - Reactive tracking: Increments compactionCount on overflow compaction - Graceful degradation: Logs but doesn't fail on flush errors **3. Added comprehensive tests** - Tests threshold detection, disabled state, already-flushed scenarios ## Impact ✅ Memory compaction triggers proactively (before overflow) ✅ compactionCount properly tracked (no longer always 0) ✅ Reduces "Context overflow" errors for users ✅ Existing orphaned code now functional Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Comment on lines
+417
to
+429
| if (sessionEntry) { | ||
| const newCompactionCount = (sessionEntry.compactionCount ?? 0) + 1; | ||
| await import("../../config/sessions/store.js").then( | ||
| ({ updateSessionStoreEntry }) => | ||
| updateSessionStoreEntry({ | ||
| storePath, | ||
| sessionKey: params.sessionKey!, | ||
| update: async () => ({ | ||
| compactionCount: newCompactionCount, | ||
| updatedAt: Date.now(), | ||
| }), | ||
| }), | ||
| ); |
Contributor
There was a problem hiding this comment.
Lost compactionCount increments
This reactive compaction tracking computes newCompactionCount from an unlocked loadSessionStore() snapshot and then writes that absolute value inside updateSessionStoreEntry(). If another writer updates compactionCount between the snapshot and the locked update, this patch can overwrite a higher value (i.e., lose increments / potentially decrease the counter). To keep the counter monotonic, compute the increment inside the update callback using the locked entry (e.g. compactionCount: (entry.compactionCount ?? 0) + 1).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/run.ts
Line: 417:429
Comment:
**Lost compactionCount increments**
This reactive compaction tracking computes `newCompactionCount` from an unlocked `loadSessionStore()` snapshot and then writes that absolute value inside `updateSessionStoreEntry()`. If another writer updates `compactionCount` between the snapshot and the locked update, this patch can overwrite a higher value (i.e., lose increments / potentially decrease the counter). To keep the counter monotonic, compute the increment inside the `update` callback using the locked `entry` (e.g. `compactionCount: (entry.compactionCount ?? 0) + 1`).
How can I resolve this? If you propose a fix, please make it concise.Compute compactionCount increment inside locked update callback instead of from unlocked snapshot to prevent lost increments when multiple writers update the counter concurrently.
Contributor
Author
|
Closing: branch is >1600 commits behind main. Will recreate from fresh base if/when needed. |
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.
Problem
Memory compaction infrastructure fully implemented but never triggered:
shouldRunMemoryFlush()implemented but never calledcompactionCountnever incremented (always 0)Result: Users hit "Context overflow: prompt too large" errors when proactive compaction should have prevented them.
Solution
1. Created
memory-flush-runner.tsrunMemoryFlushIfNeeded()checks threshold after each successful turnshouldRunMemoryFlush()to detect approaching context limitcompactionCountandmemoryFlushCompactionCount2. Integrated into agent runner (
run.ts)compactionCounton overflow compaction3. Added comprehensive tests
Impact
✅ Memory compaction triggers proactively (before overflow)
✅
compactionCountproperly tracked (no longer always 0)✅ Reduces "Context overflow" errors for users
✅ Existing orphaned code now functional
Files Changed
src/agents/pi-embedded-runner/memory-flush-runner.ts(175 lines)src/agents/pi-embedded-runner/memory-flush-runner.test.ts(104 lines)src/agents/pi-embedded-runner/run.ts(proactive + reactive tracking)Co-Authored-By: Claude Sonnet 4.5 noreply@anthropic.com
Greptile Overview
Greptile Summary
This PR wires up proactive memory compaction/flush behavior for the embedded PI agent runner by adding a memory flush runner, invoking it after successful turns, and tracking compaction counts. It also extends plugin hook types/results (including
before_agent_startmodel routing and richermessage_sendingmetadata), adds error classification helper(s), and improves media-understanding auto-detection logging.Main integration points are in
src/agents/pi-embedded-runner/run.ts(proactive flush invocation + reactive tracking on overflow) andsrc/auto-reply/reply/agent-runner-execution.ts(allowbefore_agent_starthook to override provider/model before the fallback runner executes).Confidence Score: 4/5