feat(context): add compaction preservation hooks and queue#91
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR wires extension lifecycle hooks into session compaction, collects and preserves file-operation records into compaction metadata and summaries, and updates terminal logic to snapshot, resume, and restore queued prompts during compaction. ChangesCompaction Lifecycle & File Operations System
Terminal Queue Management & Compaction Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #91 +/- ##
==========================================
+ Coverage 69.40% 69.89% +0.49%
==========================================
Files 217 219 +2
Lines 19236 19607 +371
==========================================
+ Hits 13350 13705 +355
- Misses 4729 4737 +8
- Partials 1157 1165 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
internal/assistant/context_compaction_lifecycle.go (1)
181-198: 💤 Low valueConsider using
slices.Containsfrom the standard library.The
slicesContainsStringhelper reimplementsslices.Containswhich is available in Go 1.21+. Theslicespackage is already imported incontext_file_operations.goin this PR.♻️ Optional: Use stdlib slices.Contains
+import "slices" + -func slicesContainsString(values []string, target string) bool { - for _, value := range values { - if value == target { - return true - } - } - - return false -}Then replace
slicesContainsString(plan.KeptEntryIDs, decision.FirstKeptEntryID)withslices.Contains(plan.KeptEntryIDs, decision.FirstKeptEntryID).🤖 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 `@internal/assistant/context_compaction_lifecycle.go` around lines 181 - 198, Replace the custom helper slicesContainsString with the standard library function: remove the slicesContainsString function and update its call sites (e.g., replace slicesContainsString(plan.KeptEntryIDs, decision.FirstKeptEntryID) with slices.Contains(plan.KeptEntryIDs, decision.FirstKeptEntryID)); ensure the file imports golang.org's stdlib slices package if not already imported in this file.internal/assistant/context_file_operations.go (1)
180-193: 💤 Low valueSubstring matching may cause false positives for mutating command detection.
Commands like
echo "remove this"would match"rm"substring, andecho "this sed -i example"would match"sed -i". Consider using word boundary matching or checking only the first token (the actual command).♻️ Optional: Use word-boundary or first-token matching
func shellCommandLooksMutating(command string) bool { lower := strings.ToLower(command) if strings.Contains(lower, ">") { return true } - mutatingCommands := []string{"cp", "mv", "rm", "mkdir", "touch", "tee", "sed -i", "perl -i"} - for _, candidate := range mutatingCommands { - if strings.Contains(lower, candidate) { + fields := strings.Fields(lower) + if len(fields) == 0 { + return false + } + firstCmd := fields[0] + singleTokenMutating := []string{"cp", "mv", "rm", "mkdir", "touch", "tee"} + for _, candidate := range singleTokenMutating { + if firstCmd == candidate { return true } } + // Check for sed/perl with -i flag anywhere in command + if (firstCmd == "sed" || firstCmd == "perl") && strings.Contains(lower, " -i") { + return true + } return false }🤖 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 `@internal/assistant/context_file_operations.go` around lines 180 - 193, The current shellCommandLooksMutating function falsely flags commands by substring matching (e.g., "echo remove" matches "rm"); update it to only consider the actual command token or use word-boundary matching: split the input into tokens (e.g., by whitespace), inspect the first token (command) and any explicit redirection characters, and compare that token against the mutatingCommands list (or use regex with \b boundaries) instead of using strings.Contains on the whole lowercased string; ensure checks for redirection (">", ">>") still behave correctly and preserve the existing function name shellCommandLooksMutating and the mutatingCommands set.internal/terminal/prompt_send_test.go (1)
145-154: ⚡ Quick winConsider adding test coverage for command deferral during compaction.
The test verifies that regular prompts are queued during compaction, but there's no test case for the scenario where a command (prompt starting with
/) is submitted during compaction. Based on the implementation inprompt_submit.golines 22-26, commands should restore the composer text and defer until compaction finishes, rather than being queued.🧪 Suggested test case
Add a test case to
submitCases()like:{ setupApp: func(app *App) { app.compacting = true }, composerText: "/model", wantComposerText: "/model", wantQueued: nil, name: "defers command while compacting", wantMode: modeChat, wantPromptHistory: 0, wantConsumed: false, wantRequest: false, },🤖 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 `@internal/terminal/prompt_send_test.go` around lines 145 - 154, Add a test case to submitCases() that sets app.compacting = true via setupApp, supplies composerText starting with "/" (e.g. "/model"), and asserts the composer text is preserved (wantComposerText "/model"), no queued prompts (wantQueued nil), no prompt history increment (wantPromptHistory 0), and both wantConsumed and wantRequest are false; this verifies the command-deferral behavior implemented in prompt_submit.go (lines ~22-26) where commands are restored and deferred during compaction.Source: Learnings
🤖 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.
Nitpick comments:
In `@internal/assistant/context_compaction_lifecycle.go`:
- Around line 181-198: Replace the custom helper slicesContainsString with the
standard library function: remove the slicesContainsString function and update
its call sites (e.g., replace slicesContainsString(plan.KeptEntryIDs,
decision.FirstKeptEntryID) with slices.Contains(plan.KeptEntryIDs,
decision.FirstKeptEntryID)); ensure the file imports golang.org's stdlib slices
package if not already imported in this file.
In `@internal/assistant/context_file_operations.go`:
- Around line 180-193: The current shellCommandLooksMutating function falsely
flags commands by substring matching (e.g., "echo remove" matches "rm"); update
it to only consider the actual command token or use word-boundary matching:
split the input into tokens (e.g., by whitespace), inspect the first token
(command) and any explicit redirection characters, and compare that token
against the mutatingCommands list (or use regex with \b boundaries) instead of
using strings.Contains on the whole lowercased string; ensure checks for
redirection (">", ">>") still behave correctly and preserve the existing
function name shellCommandLooksMutating and the mutatingCommands set.
In `@internal/terminal/prompt_send_test.go`:
- Around line 145-154: Add a test case to submitCases() that sets app.compacting
= true via setupApp, supplies composerText starting with "/" (e.g. "/model"),
and asserts the composer text is preserved (wantComposerText "/model"), no
queued prompts (wantQueued nil), no prompt history increment (wantPromptHistory
0), and both wantConsumed and wantRequest are false; this verifies the
command-deferral behavior implemented in prompt_submit.go (lines ~22-26) where
commands are restored and deferred during compaction.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f4ddfed5-7e1d-4944-bf43-b3d8123b32e3
📒 Files selected for processing (16)
internal/assistant/context_compaction.gointernal/assistant/context_compaction_lifecycle.gointernal/assistant/context_compaction_lifecycle_test.gointernal/assistant/context_compaction_test.gointernal/assistant/context_file_operations.gointernal/assistant/lifecycle.gointernal/extension/lifecycle.gointernal/terminal/app.gointernal/terminal/async_events.gointernal/terminal/async_events_test.gointernal/terminal/compact_commands.gointernal/terminal/compact_commands_test.gointernal/terminal/prompt_queue.gointernal/terminal/prompt_send.gointernal/terminal/prompt_send_test.gointernal/terminal/prompt_submit.go
|
This was referenced Jun 9, 2026
Merged
Merged
Merged
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.
Adds file operation preservation for compaction summaries, extension lifecycle hooks around compaction, and prompt queueing while manual compaction is running.