Skip to content

fix(context): recover from provider overflow#87

Merged
omarluq merged 4 commits into
mainfrom
fix/context-overflow-compaction
Jun 7, 2026
Merged

fix(context): recover from provider overflow#87
omarluq merged 4 commits into
mainfrom
fix/context-overflow-compaction

Conversation

@omarluq

@omarluq omarluq commented Jun 6, 2026

Copy link
Copy Markdown
Owner

No description provided.

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 05e41707-d2f3-4b10-a0a2-805f296f3866

📥 Commits

Reviewing files that changed from the base of the PR and between 65bbfd4 and 22c7b3a.

📒 Files selected for processing (2)
  • internal/assistant/context_overflow_compaction_test.go
  • internal/assistant/runtime_model.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/assistant/runtime_model.go

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Automatic provider-overflow recovery: compacts conversation context, emits compaction events, rebuilds and retries the completion once, records compaction metadata, and preserves the original provider error when no compaction occurs; optional rebuilt-budget validation.
  • Improved Detection

    • Enhanced detection of provider "context window" errors to reliably trigger recovery.
  • Refactor

    • Test harness and runtime setup reorganized for clearer test construction.
  • Tests

    • Expanded coverage and helpers for overflow recovery, event emission, error paths, input guards, and detection logic.

Walkthrough

Routes completions through a recovery flow that detects provider context-window errors, compacts session context, rebuilds the completion request, optionally validates rebuilt budget, emits compaction/usage events, and retries once; adds IsContextWindowError and comprehensive tests.

Changes

Provider Context Window Overflow Recovery

Layer / File(s) Summary
Context window error detection helpers
internal/assistant/retry.go
Adds IsContextWindowError, normalizedErrorMessage, and contextWindowProviderMessage and refactors error normalization used by retry logic.
Overflow recovery data structures and functions
internal/assistant/context_overflow_compaction.go
Adds providerOverflowRecoveryInput, Runtime.completeWithProviderOverflowRecovery, and Runtime.recoverProviderContextOverflow to detect context-window failures, emit compaction events, compact session context (CompactSessionFrom), rebuild completion requests (buildCompletionRequest), emit rebuilt usage, optionally validate rebuilt budget, and retry completion.
Runtime model completion routing
internal/assistant/runtime_model.go
modelResponse now calls completeWithProviderOverflowRecovery instead of completeWithRetry, enabling overflow recovery for model completions.
Tests, harness updates, and test helpers
internal/assistant/context_overflow_compaction_test.go, internal/assistant/context_auto_compaction_*.go, internal/assistant/export_test.go, internal/assistant/retry_test.go
Adds integration and unit tests for overflow recovery, IsContextWindowError cases, recovery error paths and input guards, test clients/helpers (including recordingCompletionClient), small test-harness refactors, and three exported test helpers on Runtime to exercise the recovery entry points.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • omarluq/librecode#46: Related retry/error-classification refactors in internal/assistant/retry.go.
  • omarluq/librecode#83: Related work on context compaction and request rebuilding; overlaps at compaction + rebuild integration.

Poem

🐰 When tokens crowd and providers say "too wide",
I nibble branches, keep the meaning inside.
Rebuild the prompt from a compacted root,
Retry once gently — hop back to the route.
Tests hum in burrows, proving recovery's stride.

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess relevance to the changeset. Add a pull request description explaining the purpose, behavior, and testing of the provider overflow recovery implementation.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(context): recover from provider overflow' clearly and concisely summarizes the main change: adding provider overflow recovery functionality to handle context-window exhaustion.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/context-overflow-compaction

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov

codecov Bot commented Jun 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.54839% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.24%. Comparing base (e1a0488) to head (22c7b3a).

Files with missing lines Patch % Lines
internal/assistant/context_overflow_compaction.go 91.30% 4 Missing and 2 partials ⚠️
internal/assistant/retry.go 94.59% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #87      +/-   ##
==========================================
+ Coverage   69.11%   69.24%   +0.12%     
==========================================
  Files         214      215       +1     
  Lines       18998    19104     +106     
==========================================
+ Hits        13131    13229      +98     
- Misses       4717     4722       +5     
- Partials     1150     1153       +3     
Flag Coverage Δ
unittests 69.24% <93.54%> (+0.12%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@internal/assistant/context_overflow_compaction.go`:
- Around line 62-85: Change the compaction event flow so the pre-compaction
event is clearly an attempt and the post-compaction event is emitted from the
rebuilt context state: update the first runtime.emitContextCompaction call
(before runtime.CompactSessionFrom) to use wording like "attempting compaction
before retry" so it doesn't promise success, call runtime.CompactSessionFrom as
now, then on success emit the post-compaction event using compactionMessage with
the recovered session/build budget from the rebuilt state (use the budget value
on recoveredEntry rather than input.build.Budget) and only emit that
post-success event after err == nil; leave the
isCompactNothingToDoError(providerErr) early-return behavior unchanged but
ensure it does not cause a misleading pre-compaction success message.

In `@internal/assistant/retry.go`:
- Around line 258-266: The current contextWindowProviderMessage function is too
permissive because the "token limit" substring matches quota/billing or
plan-limit errors; narrow this by replacing the generic "token limit" pattern
with more specific context-window phrases (e.g., "token limit exceeded for
request", "token limit for this request", or "context token limit") and add
explicit negative/regression checks to ignore billing/quota messages such as
"daily token limit exceeded", "quota exceeded", "billing", or "plan limit" so
IsContextWindowError/nonRetryableProviderMessage won't misclassify those errors;
update the patterns slice in contextWindowProviderMessage accordingly and add
unit tests covering a regression case like "daily token limit exceeded".
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 265f782a-a101-424f-a442-7ddb0b4938d4

📥 Commits

Reviewing files that changed from the base of the PR and between e1a0488 and ba463ab.

📒 Files selected for processing (4)
  • internal/assistant/context_overflow_compaction.go
  • internal/assistant/context_overflow_compaction_test.go
  • internal/assistant/retry.go
  • internal/assistant/runtime_model.go

Comment thread internal/assistant/context_overflow_compaction.go Outdated
Comment thread internal/assistant/retry.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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 `@internal/assistant/context_overflow_compaction_test.go`:
- Around line 129-139: The test expectations assert the inner error codes
instead of the runtime wrapper codes returned by recoverProviderContextOverflow;
update the failing cases that use providerOverflowFailingSummaryClient and
providerOverflowLargeSummaryClient to expect the wrapped codes: replace wantCode
"compact_summarize" with "context_overflow_compact" for the
providerOverflowFailingSummaryClient case, and replace the inner provider code
(testContextWindowExceededOopsCode) with
"context_budget_after_provider_overflow_compact" for the
providerOverflowLargeSummaryClient case — also update the identical expectation
in the other occurrence noted (the second test block around the same clients) so
all tests reflect the wrapper error codes from recoverProviderContextOverflow.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 54d15641-70d6-4a50-baa1-c43d5e2b5cfc

📥 Commits

Reviewing files that changed from the base of the PR and between ba463ab and 67b15e2.

📒 Files selected for processing (7)
  • internal/assistant/context_auto_compaction_internal_extra_test.go
  • internal/assistant/context_auto_compaction_test.go
  • internal/assistant/context_overflow_compaction.go
  • internal/assistant/context_overflow_compaction_test.go
  • internal/assistant/export_test.go
  • internal/assistant/retry.go
  • internal/assistant/retry_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • internal/assistant/retry.go
  • internal/assistant/context_overflow_compaction.go

Comment thread internal/assistant/context_overflow_compaction_test.go
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 7, 2026
@sonarqubecloud

sonarqubecloud Bot commented Jun 7, 2026

Copy link
Copy Markdown

@omarluq
omarluq merged commit af2512c into main Jun 7, 2026
15 checks passed
@omarluq
omarluq deleted the fix/context-overflow-compaction branch June 7, 2026 02:13
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.

1 participant