Skip to content

Fix compaction loop for small models or large input #5803

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.

Sign up for GitHub

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

Merged
katzdave merged 14 commits into from
Dec 16, 2025
Merged

Fix compaction loop for small models or large input #5803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
25c9354
v1
katzdave Nov 17, 2025
0609252
Cleanup extra error
katzdave Nov 18, 2025
e6f58cf
consecutive compaction single stop
katzdave Nov 18, 2025
5d4dbd3
merge
katzdave Nov 26, 2025
0917920
Merge branch 'main' of github.com:block/goose into dkatz/compation-doom
katzdave Nov 26, 2025
059bd33
merge
katzdave Dec 1, 2025
383e719
hopefully fix package lock
katzdave Dec 1, 2025
7864533
merge
katzdave Dec 3, 2025
5468906
rm empty file
katzdave Dec 3, 2025
3a304d1
variable simplification
katzdave Dec 3, 2025
7911c46
logging and comments
katzdave Dec 3, 2025
2dcc913
merge
katzdave Dec 16, 2025
657aa0a
merge agian
katzdave Dec 16, 2025
94eee11
fix complier
katzdave Dec 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions crates/goose/src/agents/agent.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,7 @@ impl Agent {
let _ = reply_span.enter();
let mut turns_taken = 0u32;
let max_turns = session_config.max_turns.unwrap_or(DEFAULT_MAX_TURNS);
let mut compaction_attempts = 0;

loop {
if is_token_cancelled(&cancel_token) {
Expand Down Expand Up @@ -991,6 +992,8 @@ impl Agent {

match next {
Ok((response, usage)) => {
compaction_attempts = 0;

// Emit model change event if provider is lead-worker
let provider = self.provider().await?;
if let Some(lead_worker) = provider.as_lead_worker() {
Expand Down Expand Up @@ -1202,6 +1205,19 @@ impl Agent {
}
Err(ref provider_err @ ProviderError::ContextLengthExceeded(_)) => {
crate::posthog::emit_error(provider_err.telemetry_type());
compaction_attempts += 1;

if compaction_attempts >= 2 {
Copy link
Collaborator

@alexhancock alexhancock Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could just check if the message length is a certain value? Conversation only contains a single user message and compaction failed?

This would be durable to other failure modes on the compaction request not working (like the provider connection is bad, or something else goes wrong)

What do you think?

Copy link
Collaborator Author

@katzdave katzdave Dec 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most common situation this occurs is when pasting something gigantic, so I don't think the single message case is all that general.

That triggers out of context error -> triggers compaction -> loop.

Agree on the sentiment of seeing if the message length is a certain size, but a bit messy in practice and would need to both estimate token counts + have a good understanding of context window size and system prompt size.

I think will go with as is for now, but might be something there using the data from canonical models.

error!("Context limit exceeded after compaction - prompt too large");
yield AgentEvent::Message(
Message::assistant().with_system_notification(
SystemNotificationType::InlineMessage,
"Unable to continue: Context limit still exceeded after compaction. Try using a shorter message, a model with a larger context window, or start a new session."
)
);
break;
}

yield AgentEvent::Message(
Message::assistant().with_system_notification(
SystemNotificationType::InlineMessage,
Expand All @@ -1222,15 +1238,10 @@ impl Agent {
conversation = compacted_conversation;
did_recovery_compact_this_iteration = true;
yield AgentEvent::HistoryReplaced(conversation.clone());
continue;
break;
}
Err(e) => {
error!("Error: {}", e);
yield AgentEvent::Message(
Message::assistant().with_text(
format!("Ran into this error trying to compact: {e}.\n\nPlease retry if you think this is a transient or recoverable error.")
)
);
error!("Compaction failed: {}", e);
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When compaction fails, the user no longer receives any feedback message. The old code yielded an AgentEvent::Message explaining the error and suggesting retry. Consider adding back a user-facing error message to inform them why the conversation stopped.

Suggested change
error!("Compaction failed: {}", e);
error!("Compaction failed: {}", e);
yield AgentEvent::Message(
Message::assistant().with_system_notification(
SystemNotificationType::InlineMessage,
"Unable to continue: Compaction failed due to an internal error. Try using a shorter message, a model with a larger context window, or start a new session."
)
);

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When compaction fails, no user-facing message is shown. The old code yielded an AgentEvent::Message explaining the error. Now users will see the conversation end without explanation, which is confusing. Consider adding back a user notification before the break.

Suggested change
error!("Compaction failed: {}", e);
error!("Compaction failed: {}", e);
yield AgentEvent::Message(
Message::assistant().with_system_notification(
SystemNotificationType::InlineMessage,
"Unable to continue: Compaction failed due to context limit. Try using a shorter message, a model with a larger context window, or start a new session."
)
);

Copilot uses AI. Check for mistakes.
break;
}
}
Expand Down
3 changes: 1 addition & 2 deletions crates/goose/src/context_mgmt/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,7 @@ async fn do_compact(
continue;
} else {
return Err(anyhow::anyhow!(
"Failed to compact messages: context length still exceeded after {} attempts with maximum removal",
removal_percentages.len()
"Failed to compact: context limit exceeded even after removing all tool responses"
));
}
}
Expand Down
Loading