Skip to content

Improve compaction#1681

Closed
dgageot wants to merge 1 commit intodocker:mainfrom
dgageot:improve-compaction
Closed

Improve compaction#1681
dgageot wants to merge 1 commit intodocker:mainfrom
dgageot:improve-compaction

Conversation

@dgageot
Copy link
Copy Markdown
Member

@dgageot dgageot commented Feb 10, 2026

I had cagent review the compaction code. Here's what it says:

Scenario Behavior Status
Successful auto-compaction Summary + continuation added in Compact(). Tokens reset in runtime.go. Model gets a valid user message. OK
Failed compaction (API error) Compact() returns early — no summary, no continuation, no token reset. OK
Empty summary Compact() returns early at if summary == "". Same as above. OK
Re-compaction loop Tokens reset to 0 after success — check does not re-trigger next iteration. OK
User-triggered /compact Continuation added inside Compact(), then user sends a real message — both appear after the summary, model sees both. Harmless. OK
Double compaction (very long session) Each compaction produces its own summary + continuation, tokens reset each time. OK

@dgageot dgageot requested a review from a team as a code owner February 10, 2026 14:04
Copy link
Copy Markdown

@docker-agent docker-agent Bot left a comment

Choose a reason for hiding this comment

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

Code Review Summary

No issues found

I've completed a thorough review of the compaction improvements. The implementation looks solid:

What was reviewed:

  • Session compaction logic - Correctly checks if compaction succeeded by comparing message counts before/after
  • Continuation message handling - Only adds the implicit user message when compaction actually produces a summary
  • Token counter reset - Properly resets counters to 0 after successful compaction to prevent immediate re-triggering
  • Error handling - Gracefully handles failed compaction by not adding stray messages

Test coverage:

The PR includes comprehensive tests covering:

  • ✅ Continuation message is sent after successful compaction
  • ✅ Token counters are reset and don't cause compaction loops
  • ✅ Failed compaction doesn't add stray messages

The logic is sound and well-tested. Great work on improving the compaction feature!

@dgageot dgageot requested a review from rumpl February 10, 2026 14:33
Comment thread pkg/runtime/runtime.go
Signed-off-by: David Gageot <david.gageot@docker.com>
// would be zero conversation messages. Providers (e.g. Anthropic)
// reject requests with no non-system messages, so we add a
// continuation message to bridge the gap.
sess.AddMessage(session.ImplicitUserMessage("The conversation was automatically compacted. Please continue where you left off."))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this is actually the issue, we are putting the session summary as a system prompt, but it should be a user message https://github.com/docker/cagent/blob/main/pkg/session/session.go#L602

@dgageot
Copy link
Copy Markdown
Member Author

dgageot commented Feb 11, 2026

@rumpl please take a look at fixing this right. I'm closing the PR

@dgageot dgageot closed this Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants