-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Compaction resiliency improvements #5618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…atz/tool-loop-cleanup * origin/dkatz/middle-truncation: fmt first pass # Conflicts: # crates/goose/src/context_mgmt/mod.rs
…eanup * 'main' of github.com:block/goose: Standardize CLI argument flags and update documentation (#5516) Release 1.13.0 fix: move goosehints/AGENTS.md handling to goose, and out of developer extension (#5575) fix: add standard context menu items to prevent empty right-click menu (#5616) Bump openapi in prepare-release (#5611) docs: add access control section to Developer tutorial (#5615)
…eanup * 'main' of github.com:block/goose: Use session IDs as task IDs for subagents instead of UUIDs (#5398) Fix the naming (#5628) fix: default tetrate model is broken, replace with haiku-4.5 (#5535) (#5587) Fetch less and use the right SHA (#5621) feat(ui): add custom macOS dock menu with New Window option (#5099) feat: remove hints from recipe prompts (#5622) docs: October 2025 Community All-Stars spotlight, Hacktoberfest edition (#5625) differentiate debug/release in cache key (#5613) Unify subrecipe and subagent execution through shared recipe pipeline (#5082)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the context management system to improve compaction handling and make it more testable by:
- Changing function signatures to accept
&dyn Providerinstead of&Agentfor better separation of concerns - Implementing progressive tool response removal when context limits are exceeded during summarization
- Introducing different continuation messages based on compaction type (manual, auto with recent user message, or during tool loop)
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| crates/goose/src/conversation/mod.rs | Made merge_consecutive_messages public to support merging summary and continuation messages |
| crates/goose/src/context_mgmt/mod.rs | Refactored compaction to use provider directly, added progressive tool response filtering, updated message preservation logic, and added comprehensive tests |
| crates/goose/src/agents/agent.rs | Updated calls to context management functions to pass provider reference instead of agent |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
crates/goose/src/agents/agent.rs
Outdated
| let needs_auto_compact = !is_manual_compact | ||
| && crate::context_mgmt::check_if_compaction_needed(self, &conversation, None, &session) | ||
| .await?; | ||
| && crate::context_mgmt::check_if_compaction_needed( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any particular reason to not have these imported with a use crate::context_mgmt::{...}?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
…eanup * 'main' of github.com:block/goose: (46 commits) Fix context progress bar not resetting after /clear command (#5652) docs: removing double announcements (#5714) docs: mcp sampling support (#5708) hackathon banner (#5710) Fix documentation-only change detection for push events (#5712) Added transaction commits to multi sql functions in session_manager (#5693) fix: improve and simplify tool call chain rendering (#5704) Fix: Always show autocompact threshold ui (#5701) chore: Update governance to include Discord (#5690) Ollama improvements (#5609) feat: add Supabase MCP server to registry (#5629) Unlist VS Code extension tutorials from MCP and experimental sections (#5677) fix: make image processing work in github copilot provider (#5687) fix: do not take into account gitignore in developer mcp (#5688) docs: session storage migration (#5682) New maintainers (#5685) chore: Update governance (#5660) chore(release): release version 1.14.0 (minor) (#5676) fix : action icons overlap session title in chat history (#5684) Document recent goose PRs (#5683) ...
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| && preserved_user_message.is_some() | ||
| { | ||
| // This is the most recent message and we're preserving it by adding a fresh copy | ||
| MessageMetadata::invisible() |
Copilot
AI
Nov 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the most recent message is being preserved, it's set to completely invisible. This means the original user message disappears from both user and agent view. Should use msg.metadata.with_agent_invisible() to keep it user-visible like other compacted messages.
| MessageMetadata::invisible() | |
| msg.metadata.with_agent_invisible() |
crates/goose/src/agents/agent.rs
Outdated
| ); | ||
|
|
||
| match crate::context_mgmt::compact_messages(self, &conversation_to_compact, false).await { | ||
| match crate::context_mgmt::compact_messages(self.provider().await?.as_ref(), &conversation_to_compact, is_manual_compact).await { |
Copilot
AI
Nov 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The third parameter should be false not is_manual_compact. This is for auto-compaction (needs_auto_compact path) and should preserve the user message. Using is_manual_compact here will incorrectly preserve messages during manual compacts when this branch shouldn't even execute.
| match crate::context_mgmt::compact_messages(self.provider().await?.as_ref(), &conversation_to_compact, is_manual_compact).await { | |
| match crate::context_mgmt::compact_messages(self.provider().await?.as_ref(), &conversation_to_compact, false).await { |
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| for i in 0..num_to_remove { | ||
| if i % 2 == 0 { | ||
| let offset = i / 2; | ||
| if middle > offset { | ||
| indices_to_remove.push(tool_indices[middle - offset - 1]); | ||
| } | ||
| } else { | ||
| let offset = i / 2; | ||
| if middle + offset < tool_indices.len() { | ||
| indices_to_remove.push(tool_indices[middle + offset]); | ||
| } | ||
| } | ||
| } |
Copilot
AI
Nov 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The middle-out removal logic fails when there's only one tool response. With a single tool response, middle = 0, and the condition middle > offset at line 253 is false for the first (and only) iteration. This means nothing gets added to indices_to_remove, so the tool response won't be removed even when remove_percent is 100. Consider adding: if i == 0 && tool_indices.len() == 1 { indices_to_remove.push(tool_indices[0]); continue; }
| const CONVERSATION_CONTINUATION_TEXT: &str = | ||
| "The previous message contains a summary that was prepared because a context limit was reached. | ||
| Do not mention that you read a summary or that conversation summarization occurred. | ||
| Just continue the conversation naturally based on the summarized context"; |
Copilot
AI
Nov 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent punctuation in continuation text constants. TOOL_LOOP_CONTINUATION_TEXT ends with a period (line 23), but CONVERSATION_CONTINUATION_TEXT (line 18) and MANUAL_COMPACT_CONTINUATION_TEXT (line 28) do not. Consider adding periods to lines 18 and 28 for consistency.
* origin/main: (51 commits) Compaction resiliency improvements (#5618) docs: ask goose button (#5730) Update prompt injection detection metrics (due to errors exporting to datadog) (#5692) Spence/icon2 tokyo drift (#5728) docs: logs rotation and misc updates (#5727) docs: automatic ollama model detection (#5725) Fix context progress bar not resetting after /clear command (#5652) docs: removing double announcements (#5714) docs: mcp sampling support (#5708) hackathon banner (#5710) Fix documentation-only change detection for push events (#5712) Added transaction commits to multi sql functions in session_manager (#5693) fix: improve and simplify tool call chain rendering (#5704) Fix: Always show autocompact threshold ui (#5701) chore: Update governance to include Discord (#5690) Ollama improvements (#5609) feat: add Supabase MCP server to registry (#5629) Unlist VS Code extension tutorials from MCP and experimental sections (#5677) fix: make image processing work in github copilot provider (#5687) fix: do not take into account gitignore in developer mcp (#5688) ...
* main: (65 commits) Fix: Recipes respect the quiet flag (#5743) docs: update cli commands (#5744) Run smoke tests on a free runner (#5745) faster, cheaper (pick two): improve CI workflow and switch to free github runner (#5702) Compaction resiliency improvements (#5618) docs: ask goose button (#5730) Update prompt injection detection metrics (due to errors exporting to datadog) (#5692) Spence/icon2 tokyo drift (#5728) docs: logs rotation and misc updates (#5727) docs: automatic ollama model detection (#5725) Fix context progress bar not resetting after /clear command (#5652) docs: removing double announcements (#5714) docs: mcp sampling support (#5708) hackathon banner (#5710) Fix documentation-only change detection for push events (#5712) Added transaction commits to multi sql functions in session_manager (#5693) fix: improve and simplify tool call chain rendering (#5704) Fix: Always show autocompact threshold ui (#5701) chore: Update governance to include Discord (#5690) Ollama improvements (#5609) ...
* main: scan recipe for security when saving recipe (#5747) feat: trying grok for live test (#5732) Platform Extension MOIM (Minus One Info Message) (#5027) docs: remove hackathon banner (#5756) Fix: Recipes respect the quiet flag (#5743) docs: update cli commands (#5744) Run smoke tests on a free runner (#5745) faster, cheaper (pick two): improve CI workflow and switch to free github runner (#5702) Compaction resiliency improvements (#5618) docs: ask goose button (#5730) Update prompt injection detection metrics (due to errors exporting to datadog) (#5692) Spence/icon2 tokyo drift (#5728) docs: logs rotation and misc updates (#5727) docs: automatic ollama model detection (#5725) Fix context progress bar not resetting after /clear command (#5652) docs: removing double announcements (#5714) docs: mcp sampling support (#5708)
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]> Signed-off-by: Blair Allan <[email protected]>
Additional tool calling tests
Progressively remove tool call outputs from the middle of the conversation to minimize context length.
Fix an issue where the manual compact marker was being preserved.