Repository navigation
fix(core): bound auto-compaction when the buffer exceeds the window - #56786
reynold-hu wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
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
| 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)) |
There was a problem hiding this comment.
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.
|
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 + |
|
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 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: |
cd4fd3d to
5314686
Compare
5314686 to
7846b32
Compare



Issue for this PR
Closes #56783
Type of change
What does this PR do?
compaction.bufferis a single global value but is evaluated against each model's own window. A value at or above the window madewindow - bufferzero 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.RESERVE_SHARE, at leastRESERVE_MIN) applies. Clamping to a fraction instead can put the ceiling below the fixed prompt size (system, tools, instructions)./compactstill summarizes whatever the history holds.skippedinstead 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 removedsplit.older.How did you verify your code works?
buffer: 600000and 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.completedon 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/coretypecheck passes.Screenshots / recordings
No UI change.
Checklist