-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Break compaction back into check_ and do_ compaction #5212
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
| /// * `CompactionCheckResult` containing detailed information about compaction needs | ||
| async fn check_compaction_needed( | ||
| /// * `CompactionCheckResult` containing whether compaction is needed and usage details | ||
| pub async fn 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.
can't we just return a boolean here? also delete the intro comments
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.
Yeah good call, I don't think we're actually doing anything interesting with CompactionCheckResult. Mostly just want to display the GOOSE_AUTO_COMPACT_THRESHOLD that we exceeded.
| .rev() | ||
| .find(|msg| matches!(msg.role, rmcp::model::Role::User)) | ||
| .cloned(); | ||
| (messages.as_slice(), most_recent_user_message) |
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.
this whole block strikes me as weird. there's two cases here, either the last message is a user message (i.e. the user said something and we decided to auto compact or the user said something, our autocompacting didn't trigger but we ran out tokens never the less, i.e. user has compact limit at 99% and posts something long) or it is not (i.e. we ran out of context while processing a tool call reply)
we set preserve_last_user_message to true if this is triggered from the tool calling loop, but in all other cases, the last message should be a user message. so I don't think we need this flag at all.
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 first case where the last message is a user one, is from auto-compact.
The second exists for the difference of manual summarize vs tool calling case you described.
In manual compaction we don't actually want to re-trigger the agent loop (and the last message is probably an agent message from the last response), but if we hit a tool call ContextLimitExceeded, I want to find and re-process the most recent user message and continue the agent loop automatically.
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.
yes I understand. I'm just saying there's no difference.
but if we hit a tool call ContextLimitExceeded, I want to find and re-process the most recent user message and continue the agent loop automatically.
that is one way, but the other is that we ran out of context with a user mesage. so in either case you are looking for the last user message.
The other thing we're missing here I think is that tool responses are also user mesages, so you'll typically return that for the tool loop case.
* 'main' of github.com:block/goose: Break compaction back into check_ and do_ compaction (#5212) fix: revert built app name to uppercase Goose (#5206) feat: add Code Documentation Generator recipe (#5121) (#5125) Revert "feat: enhance goose to search sessions for easy recall (#5177)" (#5209)
* main: (119 commits) Break compaction back into check_ and do_ compaction (#5212) fix: revert built app name to uppercase Goose (#5206) feat: add Code Documentation Generator recipe (#5121) (#5125) Revert "feat: enhance goose to search sessions for easy recall (#5177)" (#5209) Blog: Best Practices for Prompt Engineering with goose (#5204) force WAL sync after session create (#5202) Feat: goose Apify MCP integration docs (#5047) feat: enhance goose to search sessions for easy recall (#5177) Skip hidden & real format (#5194) docs: Hacktoberfest blog submission - Best Practices for Using Goose in Enterprise Environments by Anudhyan Datta. (#5184) docs: prompt injection detection (#5193) Fix mcp large response race condition (#5065) Compaction overhaul (#5186) fix: #3960 better approach to input schema for dynamic task params (#5189) used recipe id or deeplink to start agent (#5154) [docs] Add Blog Post: "Designing AI for Users, Not Just LLMs" (#5190) docs: update cognee, jetbrains, mbot extensions config (#5172) Minimally disable subagents when not in autonomous model (#5149) Fix provider sort (#5188) blog: Getting Started with Goose on Windows (#5156) ...
* main: (143 commits) Break compaction back into check_ and do_ compaction (#5212) fix: revert built app name to uppercase Goose (#5206) feat: add Code Documentation Generator recipe (#5121) (#5125) Revert "feat: enhance goose to search sessions for easy recall (#5177)" (#5209) Blog: Best Practices for Prompt Engineering with goose (#5204) force WAL sync after session create (#5202) Feat: goose Apify MCP integration docs (#5047) feat: enhance goose to search sessions for easy recall (#5177) Skip hidden & real format (#5194) docs: Hacktoberfest blog submission - Best Practices for Using Goose in Enterprise Environments by Anudhyan Datta. (#5184) docs: prompt injection detection (#5193) Fix mcp large response race condition (#5065) Compaction overhaul (#5186) fix: #3960 better approach to input schema for dynamic task params (#5189) used recipe id or deeplink to start agent (#5154) [docs] Add Blog Post: "Designing AI for Users, Not Just LLMs" (#5190) docs: update cognee, jetbrains, mbot extensions config (#5172) Minimally disable subagents when not in autonomous model (#5149) Fix provider sort (#5188) blog: Getting Started with Goose on Windows (#5156) ...
* 'main' of github.com:block/goose: (22 commits) Rewrite extension management tools (#5057) fix: re-sync package-lock.json (#5235) docs: Hacktoberfest MCP youtube short entry to community-content.json (#5150) feat: add schedule button to recipe entries (#5217) Autocompact threshold UI cleanup (#5232) fix: correct schema for openai tools (#5229) Break compaction back into check_ and do_ compaction (#5212) fix: revert built app name to uppercase Goose (#5206) feat: add Code Documentation Generator recipe (#5121) (#5125) Revert "feat: enhance goose to search sessions for easy recall (#5177)" (#5209) Blog: Best Practices for Prompt Engineering with goose (#5204) force WAL sync after session create (#5202) Feat: goose Apify MCP integration docs (#5047) feat: enhance goose to search sessions for easy recall (#5177) Skip hidden & real format (#5194) docs: Hacktoberfest blog submission - Best Practices for Using Goose in Enterprise Environments by Anudhyan Datta. (#5184) docs: prompt injection detection (#5193) Fix mcp large response race condition (#5065) Compaction overhaul (#5186) fix: #3960 better approach to input schema for dynamic task params (#5189) ...
* main: delete flaky pricing integration tests (block#5207) chore: upgrade most npm packages to latest (block#5185) Release/1.11.0 (block#5224) Rewrite extension management tools (block#5057) fix: re-sync package-lock.json (block#5235) docs: Hacktoberfest MCP youtube short entry to community-content.json (block#5150) feat: add schedule button to recipe entries (block#5217) Autocompact threshold UI cleanup (block#5232) fix: correct schema for openai tools (block#5229) Break compaction back into check_ and do_ compaction (block#5212) fix: revert built app name to uppercase Goose (block#5206) feat: add Code Documentation Generator recipe (block#5121) (block#5125)
* origin/main: (66 commits) Revert "Rewrite extension management tools" (#5243) Standardize Session Name Attribute (#5085) Docs: Include step-by-step model configuration instructions for first… (#5239) Delete message visibility filter (#5216) delete flaky pricing integration tests (#5207) chore: upgrade most npm packages to latest (#5185) Release/1.11.0 (#5224) Rewrite extension management tools (#5057) fix: re-sync package-lock.json (#5235) docs: Hacktoberfest MCP youtube short entry to community-content.json (#5150) feat: add schedule button to recipe entries (#5217) Autocompact threshold UI cleanup (#5232) fix: correct schema for openai tools (#5229) Break compaction back into check_ and do_ compaction (#5212) fix: revert built app name to uppercase Goose (#5206) feat: add Code Documentation Generator recipe (#5121) (#5125) Revert "feat: enhance goose to search sessions for easy recall (#5177)" (#5209) Blog: Best Practices for Prompt Engineering with goose (#5204) force WAL sync after session create (#5202) Feat: goose Apify MCP integration docs (#5047) ...
* main: (32 commits) turn off WAL (#5203) Skip subagents for gemini (#5257) Revert "Standardize Session Name Attribute" (#5250) improve provider request logging a bit (#5236) Fix OpenAI empty choices panic (#5248) Revert "Rewrite extension management tools" (#5243) Standardize Session Name Attribute (#5085) Docs: Include step-by-step model configuration instructions for first… (#5239) Delete message visibility filter (#5216) delete flaky pricing integration tests (#5207) chore: upgrade most npm packages to latest (#5185) Release/1.11.0 (#5224) Rewrite extension management tools (#5057) fix: re-sync package-lock.json (#5235) docs: Hacktoberfest MCP youtube short entry to community-content.json (#5150) feat: add schedule button to recipe entries (#5217) Autocompact threshold UI cleanup (#5232) fix: correct schema for openai tools (#5229) Break compaction back into check_ and do_ compaction (#5212) fix: revert built app name to uppercase Goose (#5206) ...
This exposes clear points (marked with TODOs) as to where we can notify the client before + after compaction.
Also move the public api to use conversation instead of messages
Following this up with more clear messaging on
Adding GooseThinkingMessage. Non agent visible message, that can set the state of the goose is thinking spinner. Will use this to say 'Compacting conversation...'
Refactor ConversationCompacted into SystemNotification; General messages that display in-line in the client, but aren't agent_visible. No special logic on trying to process these, they just render inline with the conversation wherever they fall.