feat: compact - #22
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughA new context compaction module is introduced to manage and reduce agent conversation memory. It defines a pluggable async ChangesContext Compaction System
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/compact.rs`:
- Around line 253-265: The compacted constructor currently computes tokens_saved
incorrectly (tokens_after.saturating_sub(tokens_after)) and needs a real
tokens_before value; update pub fn compacted(messages: Vec<Message>,
tokens_after: u64) -> Self to accept a tokens_before: u64 parameter and set
tokens_saved = tokens_before.saturating_sub(tokens_after), leaving
messages_compacted = 0 (caller will set) and preserving success: true and error:
None; alternatively, if you prefer leaving tokens_saved to callers, remove the
tokens_saved field initialization here and document that callers must set it,
but the preferred fix is adding the tokens_before parameter and computing
tokens_saved in compacted.
- Around line 330-336: The docstring claims the compactor preserves the first
system prompt but the current compaction logic simply keeps the last
preserve_recent messages; modify the compaction routine (the code that currently
slices to keep the last preserve_recent messages) to also include the first
message when it has the expected role: check messages.first().map(|m| m.role)
and if it matches the prompt role (as described in the docstring) prepend that
first message to the preserved list and then append the last preserve_recent
messages without duplicating it (use preserve_recent, messages, and the
compaction function/logic identifiers to locate where to change the
slice/collection code); alternatively if you prefer the simpler fix, update the
docstring text to remove the claim about preserving the system prompt so it
matches the existing behavior.
🪄 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 Plus
Run ID: 62b2de40-7ee1-444e-8cba-78669c356360
📒 Files selected for processing (2)
src/compact.rssrc/lib.rs
No description provided.