feat(compaction): auto-compact and retry on context overflow errors#91
Merged
Conversation
) Add a reactive compaction path: when a provider call fails with a context-length/overflow error, compact the conversation and replay the turn once instead of surfacing a hard failure. Proactive ShouldCompact() relies on token estimates that inevitably drift, and a single huge mid-turn tool result can overflow even a turn that started under the limit — this is the safety net that makes an imprecise estimator acceptable. - new pkg/kit/overflow.go: isContextOverflow detection via the ErrContextOverflow sentinel, prepareOverflowRetry (compact via existing compactInternal, rebuild context, re-run ContextPrepare hooks), and stripMediaParts (media attachments replaced with text placeholders on replay — they cannot be shrunk by summarization) - runTurn: persist completed steps from the failed attempt first so the replay resumes rather than restarts; single-retry guard prevents compact/overflow loops; second overflow fails with a clear 'conversation too large to compact' error wrapping the sentinel - always on, independent of AutoCompact (which stays proactive-only); compaction failure or cancellation surfaces the original error - extract persistGenerationRemainder helper (deduplicates three copies) - tests: overflow classification, media stripping, and prepareOverflowRetry against an in-memory session (summary injection, hook execution, failure paths) - docs: README, pkg/kit README, www (sessions, cli/flags, sdk/options, sdk/overview), kit-sdk skill, godoc on ErrContextOverflow/AutoCompact Fixes #85
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds reactive compact-and-replay handling for provider context-overflow errors, including replay-context rebuilding, media stripping, retry wiring in the turn loop, tests, and documentation updates that distinguish proactive from reactive compaction. ChangesReactive compaction feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant runTurn
participant Provider
participant prepareOverflowRetry
participant ContextPrepare
runTurn->>Provider: generate turn
Provider-->>runTurn: context-overflow error
runTurn->>prepareOverflowRetry: build retry context
prepareOverflowRetry->>prepareOverflowRetry: compact and rebuild messages
prepareOverflowRetry->>ContextPrepare: run hooks on replay context
ContextPrepare-->>prepareOverflowRetry: prepared messages
prepareOverflowRetry-->>runTurn: replay messages
runTurn->>Provider: retry generate
alt overflow again
Provider-->>runTurn: overflow error
runTurn-->>runTurn: surface ErrContextOverflow
else success
Provider-->>runTurn: result
end
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@pkg/kit/overflow_test.go`:
- Around line 148-159: The test setup in overflow_test is ignoring AppendMessage
failures, which can make the fixture invalid and the retry assertions
unreliable. Add a small helper in the test file around k.session.AppendMessage
that checks each append error and returns a wrapped fmt.Errorf with context,
then use it for the setup calls in the affected test cases. Focus on the
repeated message-building code in the test helpers/cases so failures are
surfaced immediately with useful context.
In `@pkg/kit/overflow.go`:
- Around line 36-41: `prepareOverflowRetry` is currently returning only a
success flag, which causes `runTurn` to pass through raw provider failures
instead of the expected `ErrContextOverflow`-wrapped error when compaction/retry
prep fails. Update `prepareOverflowRetry` to return an error (or otherwise
classify/wrap the failure) alongside the replay messages, and adjust `runTurn`
to surface `ErrContextOverflow` consistently whenever this helper cannot produce
usable retry context.
- Around line 49-55: After rerunning ContextPrepare hooks in overflow replay,
the returned messages may reintroduce media parts via fantasy.FilePart
replacements. Update the overflow flow in stripMediaParts and the
ContextPrepareHook path so any hook-returned messages are passed through
stripMediaParts again before proceeding, ensuring replayed requests stay
media-free.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 88458464-8628-46c1-97c6-c7793e60f087
📒 Files selected for processing (11)
README.mdpkg/kit/README.mdpkg/kit/errors.gopkg/kit/kit.gopkg/kit/overflow.gopkg/kit/overflow_test.goskills/kit-sdk/SKILL.mdwww/pages/cli/flags.mdwww/pages/sdk/options.mdwww/pages/sdk/overview.mdwww/pages/sessions.md
- prepareOverflowRetry now returns an error instead of a bool so runTurn can wrap the original provider error with the recovery-failure reason; the surfaced error keeps the ErrContextOverflow classification via ClassifyProviderError even when the provider error was raw text - strip media again from ContextPrepare hook results — a hook can replace the messages and reintroduce attachments, defeating recovery - tests: check fixture AppendMessage errors via appendTestMessage helper; new TestPrepareOverflowRetry_StripsMediaFromHookResult; failure-path tests now assert the error message content
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.
Description
Adds reactive compaction: when a provider call fails with a context-length/overflow error, Kit automatically compacts the conversation and replays the turn once instead of surfacing a hard failure.
Kit previously only compacted proactively —
runTurnchecksShouldCompact()before building context. But token estimation inevitably drifts from real tokenizer counts (tool-heavy traffic, non-English text, images), and a single huge mid-turn tool result can overflow the context even when the turn started well under the limit.ErrContextOverflowexisted inpkg/kit/errors.gobut nothing recovered from it. The reactive path is the safety net that makes an imprecise estimator acceptable in practice (mirroring opencode'scompactAfterOverflow).Before: provider rejects the request → turn fails with a raw overflow error.
After: overflow → completed steps from the failed attempt are persisted → compaction runs (reusing
compactInternal(ctx, opts, "", true)) → context is rebuilt from the session (so the replay resumes from where it overflowed rather than restarting) → media attachments are replaced with text placeholders (they can't be shrunk by summarization) → the turn replays once. If the replay also overflows, the turn fails with a clear"conversation too large to compact — still exceeds the model context window after compaction"error wrappingErrContextOverflow.Key properties:
AutoCompact, which stays proactive-only. The reactive path only fires where the turn would otherwise hard-fail.BeforeCompacthooks run (withIsAutomatic=true) and can still cancel;ContextPreparehooks re-run on the rebuilt context so extensions observe every outgoing request. Compaction failure or cancellation surfaces the original provider error.TurnResult.Streamreflects only the replay.Fixes #85
Type of Change
Checklist
go test -race ./...,golangci-lint runclean)Additional Information
New files
pkg/kit/overflow.go—isContextOverflow(detection via the existingClassifyProviderError/ErrContextOverflowsentinel),prepareOverflowRetry(compact → rebuild context → strip media → re-runContextPreparehooks),stripMediaParts(non-mutatingFilePart→ text-placeholder replacement)pkg/kit/overflow_test.go— 8 tests: overflow classification (raw provider text, pre-wrapped sentinel, negatives), media stripping (replacement, filename/media-type fallbacks, passthrough, input immutability), andprepareOverflowRetryend-to-end against an in-memory session (summary injection,IsAutomatic=true, hook execution, impossible/cancelled-compaction failure paths)Modified
pkg/kit/kit.go— reactive retry inrunTurn; extractedpersistGenerationRemainderhelper (deduplicates three copies of the remainder-persistence logic); expandedOptions.AutoCompactgodocpkg/kit/errors.go—ErrContextOverflowgodoc now documents the automatic recoveryREADME.md,pkg/kit/README.md,www/pages/(sessions, cli/flags, sdk/options, sdk/overview),skills/kit-sdk/SKILL.mdBackward compatibility
kit.ErrContextOverflowviaerrors.Is— it just fires only after recovery also failedCompactionEntryper recovered overflow (non-destructive, same as any compaction)Out of scope (issue's optional follow-ons, suggested there as splittable): mid-turn
ShouldCompact()checks between agentic steps, and the post-compaction synthetic continuation nudge.Summary by CodeRabbit
New Features / Behavior Updates
Bug Fixes
Documentation
--auto-compact/AutoCompact: proactive compaction is optional, while reactive compact-and-retry on overflow is always enabled.Tests