feat: introduce pluggable memory providers for agent workflows#29
Conversation
- Added `MemoryProvider` trait to allow customizable state compaction strategies. - Implemented default `SlidingWindowMemory` provider to maintain legacy behavior. - Updated `AgentWorkerBuilder` to support configurable memory providers. - Removed hardcoded compaction logic from `workflow.rs` and `state.rs`, and moved to `memory.rs`.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughThe PR refactors history compaction in ChangesPluggable Memory Provider System
Possibly related issues
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/builder.rs (1)
115-123:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not ignore
OnceCell::setfailures for workflow globals.Silently discarding failed sets can bind a worker to stale
WORKER_TOOL_CATALOG/WORKER_MEMORYvalues from a previous initialization in the same process.build_workershould fail fast (or validate exact equality) when initialization is not accepted.As per coding guidelines, "Set
WORKER_TOOL_CATALOG(process-globalOnceCell) once at worker init inbuild_worker; it must be set before the worker starts and never mutated after."🤖 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 `@src/builder.rs` around lines 115 - 123, The current code ignores failures from WORKER_TOOL_CATALOG.set(...) and WORKER_MEMORY.set(...), which can leave a worker bound to stale globals; in build_worker handle the Result returned by OnceCell::set: if set succeeds continue, if it fails retrieve the existing value with WORKER_TOOL_CATALOG.get()/WORKER_MEMORY.get() and either validate it is exactly equal to the value being set (and continue) or return an error / abort initialization (fail fast). Update build_worker to propagate an Err (or panic) when the existing global differs from the new value so initialization cannot silently proceed with stale state.
🤖 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 `@src/workflow.rs`:
- Around line 97-99: The workflow currently defers compaction entirely to
MemoryProvider::should_compact, allowing providers to bypass the mandated
continue-as-new behavior when AgentState::history.len() >
CONTINUE_AS_NEW_THRESHOLD (200); modify the workflow around ctx.state(|s| ...)
so it always enforces the invariant: if s.state.history.len() >
CONTINUE_AS_NEW_THRESHOLD then perform continue_as_new with a compacted state
(call memory.compact to produce a summary, prepend that summary to the system
prompt, and retain only the last 20 messages from s.state.history) and only
otherwise allow MemoryProvider::should_compact to influence additional
compaction strategy. Ensure you update the logic that currently uses
ctx.state(|s| memory.should_compact(&s.state)) and ctx.state(|s|
memory.compact(&s.state)) to implement this enforced path before delegating to
providers.
---
Outside diff comments:
In `@src/builder.rs`:
- Around line 115-123: The current code ignores failures from
WORKER_TOOL_CATALOG.set(...) and WORKER_MEMORY.set(...), which can leave a
worker bound to stale globals; in build_worker handle the Result returned by
OnceCell::set: if set succeeds continue, if it fails retrieve the existing value
with WORKER_TOOL_CATALOG.get()/WORKER_MEMORY.get() and either validate it is
exactly equal to the value being set (and continue) or return an error / abort
initialization (fail fast). Update build_worker to propagate an Err (or panic)
when the existing global differs from the new value so initialization cannot
silently proceed with stale state.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c969f53d-d46c-417e-b1c4-14a8948078e4
📒 Files selected for processing (7)
src/builder.rssrc/lib.rssrc/memory.rssrc/prelude.rssrc/state.rssrc/workflow.rstests/prelude_memory_exports.rs
💤 Files with no reviewable changes (1)
- src/state.rs
…r memory providers - Introduced the `tunable_memory_agent` example to demonstrate custom and aggressive `SlidingWindowMemory` configurations. - Expanded README with a "Pluggable memory backends" section describing memory provider traits and usage. - Added documentation for `MemoryProvider` determinism requirements in `AGENTS.md`. - Enforced consistency checks for `MemoryProvider` across workers in multi-worker setups.
MemoryProvidertrait to allow customizable state compaction strategies.SlidingWindowMemoryprovider to maintain legacy behavior.AgentWorkerBuilderto support configurable memory providers.workflow.rsandstate.rs, and moved tomemory.rs.Summary by CodeRabbit