fix(core): bound auto-compaction when the buffer exceeds the window by reynold-hu · Pull Request #56786 · anomalyco/opencode · GitHub
Skip to content

fix(core): bound auto-compaction when the buffer exceeds the window - #56786

Open
reynold-hu wants to merge 1 commit into
anomalyco:devfrom
reynold-hu:compaction-buffer-guard
Open

reynold-hu wants to merge 1 commit into
anomalyco:devfrom
reynold-hu:compaction-buffer-guard

Conversation

@reynold-hu

@reynold-hu reynold-hu commented Oct 18, 2026 •

Copy link
Copy Markdown

Issue for this PR

Closes #56783

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

compaction.buffer is a single global value but is evaluated against each model's own window. A value at or above the window made window - buffer zero or negative, so automatic compaction was due on every turn, including the first request of a session. A fresh session could also publish a failed compaction when there was nothing to summarize.

  • A buffer at or above the window is treated as unset, so the default reserve (RESERVE_SHARE, at least RESERVE_MIN) applies. Clamping to a fraction instead can put the ceiling below the fixed prompt size (system, tools, instructions).
  • Automatic compaction waits until the history contains an assistant reply. The opening prompt alone is not a conversation to summarize, and it no longer produces a failed compaction. A manual /compact still summarizes whatever the history holds.
  • An automatic no-op is reported as skipped instead of publishing a failed compaction.

Rebased onto the current dev, including #57091's whole-conversation summarization. The guard checks for an assistant message instead of the removed split.older.

How did you verify your code works?

  • New test: with buffer: 600000 and a 100k window, 83,999 measured tokens are skipped and 84,000 compact (default 16k reserve); a session whose only message is a ~100k-token user prompt is skipped.
  • New provider-switch test (suggested by @intrepid37): large 1M → small 200k → large, asserting stale usage from another provider does not trigger compaction while real pressure on the small model still does.
  • Both tests return completed on the parent commit, so they fail before the fix.
  • bun test test/session-compaction.test.ts test/session-native-compaction.test.ts test/session-compact.test.ts test/session-runner.test.ts: 259 pass, 0 fail.
  • packages/core typecheck passes.

Screenshots / recordings

No UI change.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@github-actions

Copy link
Copy Markdown
Contributor

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This fixes the reported case, but I'd like one more change before merge.

I drove the TUI with the simulated model (128k window) and compaction.buffer: 600000. On the base, automatic compaction ran before the first reply and again on the next turn. With this PR, neither happened. The new test fails on the base and passes here. The compaction tests (23) and session-runner.test.ts (229) pass, and the packages/core type check passes.

Public API: unchanged. Only the effect of a compaction.buffer larger than 90% of the window changes.

The main concern is the 90% cap (inline): automatic compaction still runs on every turn whenever the system prompt, tools and instructions take more than 10% of the window. I reproduced this with a ~14k-token AGENTS.md and the same config.

The skipped outcome for an automatic run with nothing to summarize looks right. That failure has no other source, so no real error gets hidden, and the runner already handles skipped, so the StepFailedError no longer shows at startup.

Related: #43703

Before

After

Comment thread packages/core/src/session/compaction.ts Outdated
if (buffer !== undefined) return window - buffer
// Keep at most 90% of the window free. A larger buffer would make the ceiling negative and every estimate
// cross it, so automatic compaction would run on every step, including the first request of a session.
if (buffer !== undefined) return window - Math.min(buffer, Math.floor(window * 0.9))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Capping at 90% stops the first-request compaction, but the per-turn loop from #56783 can still happen. The threshold becomes 10% of the window: 12.8k for a 128k model, or 3.2k when a buffer tuned for a big model (e.g. 40000) is used on a 32k model. The system prompt, tools and project instructions alone often exceed that, and compacting can't shrink them. I reproduced it with the simulated 128k model, buffer: 600000 and a ~14k-token AGENTS.md: with this PR, compaction ran after the first reply and again on the next turn.

A buffer this large is almost certainly a misconfiguration. One fix is to fall back to the default reserve when buffer is at least the window, perhaps with a config warning. Another is a much smaller cap, e.g. half the window, together with a check that skips automatic compaction when it would not get the request below the threshold.

@reynold-hu

Copy link
Copy Markdown
Author

Addressed the 90% cap concern in cd4fd3d. A buffer at or above the window is now treated as unset, so the default reserve applies and the ceiling stays above the fixed prompt size. For the 128k window + buffer: 600000 + ~14k-token AGENTS.md case the threshold is now 112k (16k reserve), not 12.8k. Updated the test: 83,999 measured tokens are skipped and 84,000 compact on a 100k window; the no-exchange case now uses a prompt that actually crosses the ceiling. Compaction + runner suites: 252 pass, typecheck clean.

@intrepid37

Copy link
Copy Markdown

Additional synthetic regression test for this PR: verify that an oversized global compaction buffer remains safe when the same session switches large-context provider → small-context provider → large-context provider. Old usage measurements from a different provider should not trigger compaction on the new one, and real pressure on the small model should still trigger compaction normally.

The diff adds one test to packages/core/test/session-compaction.test.ts on this PR head (cd4fd3d). It uses only fake models, synthetic prompts, and synthetic usage. No real conversation, screenshot, session record, or user environment detail is included.

Ready-to-apply test diff (one test)
diff --git a/packages/core/test/session-compaction.test.ts b/packages/core/test/session-compaction.test.ts
index eeffeb4..80bb7e4 100644
--- a/packages/core/test/session-compaction.test.ts
+++ b/packages/core/test/session-compaction.test.ts
@@ -344,6 +344,65 @@ it.effect("clamps an oversized compaction buffer and waits for an exchange to su
   }),
 )
 
+it.effect("auto compaction stays model-aware when switching between differently sized providers", () =>
+  Effect.gen(function* () {
+    const compaction = yield* SessionCompaction.Service
+    const session = yield* insertSession(Session.ID.make("ses_synthetic_provider_switch"))
+    // A user may configure this for a large model, then change providers mid-thread.
+    yield* compaction.transform((editor) => editor.configure({ buffer: 600_000 }))
+
+    const models = {
+      large: LanguageModel.make({ id: "synthetic-large", provider: "synthetic-large", route: OpenAIChat.route }),
+      small: LanguageModel.make({ id: "synthetic-small", provider: "synthetic-small", route: OpenAIChat.route }),
+    }
+    const assistant = (provider: "large" | "small", inputTokens: number) =>
+      Schema.decodeUnknownSync(SessionMessage.Assistant)({
+        id: SessionMessage.ID.create(),
+        type: "assistant",
+        agent: Agent.defaultID,
+        model: { id: models[provider].id, providerID: models[provider].provider },
+        content: [{ type: "text", text: "Synthetic response" }],
+        tokens: { input: inputTokens, output: 0, reasoning: 0, cache: { read: 0, write: 0 } },
+        time: { created: 0, completed: 0 },
+      })
+    const user = (text: string) =>
+      SessionMessage.User.make({
+        id: SessionMessage.ID.create(),
+        type: "user",
+        text,
+        time: { created: DateTime.makeUnsafe(0) },
+      })
+    const status = (provider: "large" | "small", messages: readonly SessionMessage.Info[]) =>
+      compaction
+        .compact({
+          reason: "auto",
+          context: {
+            ...loaded(session, messages),
+            model: SessionRunnerModel.resolved(models[provider], {
+              capabilities: { tools: true, input: ["text"], output: ["text"] },
+              cost: [],
+              limit: { context: provider === "large" ? 1_000_000 : 200_000, output: 32_000 },
+            }),
+          },
+        })
+        .pipe(Effect.map((outcome) => outcome.status))
+
+    const big = assistant("large", 250_000)
+    const start = [user("Start synthetic task"), big]
+    expect(yield* status("large", start)).toBe("skipped") // 1M - 600k remains usable.
+
+    const switched = [...start, user("Switch to smaller provider")]
+    expect(yield* status("small", switched)).toBe("skipped") // Ignore large provider's token count.
+    const small = [...switched, assistant("small", 25_000)]
+    expect(yield* status("small", small)).toBe("skipped") // Oversized buffer falls back to default.
+
+    expect(yield* status("large", [...small, user("Switch back")])).toBe("skipped")
+
+    // Real pressure on the smaller provider still triggers normal auto-compaction (200k - 20k).
+    expect(yield* status("small", [...switched, assistant("small", 180_000)])).toBe("completed")
+  }),
+)
+
 /** Seeds the global project plus one session row, returning the projected session. */
 const insertSession = (id: Session.ID, overrides?: Partial<typeof SessionTable.$inferInsert>) =>
   Effect.gen(function* () {

Validation: git diff --check passed, and TypeScript syntax/transpilation reported zero errors. Bun tests and full typecheck were not run on this isolated checkout, so please run them in the PR environment before merging. This complements the existing oversized-buffer regression test rather than duplicating the fix.

@reynold-hu
reynold-hu force-pushed the compaction-buffer-guard branch from cd4fd3d to 5314686 Compare October 21, 2026 10:43
@reynold-hu

Copy link
Copy Markdown
Author

@reynold-hu
reynold-hu force-pushed the compaction-buffer-guard branch from 5314686 to 7846b32 Compare October 21, 2026 14:45

This branch has not been deployed

No deployments
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.

V2: compaction fires on first request / loops when compaction.buffer >= model context limit

2 participants