-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix(core): memory-based chat compression to prevent heap OOM #4127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1349,6 +1349,28 @@ export class AgentCore { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| return this.messages; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Prune oldest messages from the message history when `heapUsed` exceeds | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * 1.5 GB. Returns the number of messages pruned (0 if no pruning was | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * needed). Call this after each reasoning round to prevent unbounded | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * memory growth in long-lived interactive sessions. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Messages are pruned in chunks — removing ~20% of the oldest messages | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * each time, which naturally scales with memory pressure rather than | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * a hard count cap. This catches the actual root cause (heap usage) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * regardless of how the memory ballooned (many small entries vs few huge). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pruneMessages(): number { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const threshold = 1.5 * 1024 * 1024 * 1024; // 1.5 GB | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (process.memoryUsage().heapUsed < threshold) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return 0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Remove ~20% of the oldest messages to relieve heap pressure. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical]
Suggested change
— DeepSeek/deepseek-v4-pro via Qwen Code /review
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion]
Suggested change
— DeepSeek/deepseek-v4-pro via Qwen Code /review |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const toPrune = Math.max(1, Math.ceil(this.messages.length * 0.2)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.messages.splice(0, toPrune); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return toPrune; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Tool calls currently awaiting user approval. Mutated by | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * AgentInteractive's TOOL_WAITING_APPROVAL handler; headless agents | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -123,6 +123,10 @@ export class AgentInteractive { | |||||||||||||||||||||||
| while (message !== null && !this.masterAbortController.signal.aborted) { | ||||||||||||||||||||||||
| this.addMessage('user', message); | ||||||||||||||||||||||||
| await this.runOneRound(message); | ||||||||||||||||||||||||
| // Prune old messages to prevent unbounded memory growth in | ||||||||||||||||||||||||
| // long-lived interactive sessions (81+ minute sessions with | ||||||||||||||||||||||||
| // hundreds of rounds can hit 4 GB without pruning). | ||||||||||||||||||||||||
| this.core.pruneMessages(); | ||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] The sequence is:
Suggested change
— DeepSeek/deepseek-v4-pro via Qwen Code /review
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] 12 tests fail because
Suggested change
— DeepSeek/deepseek-v4-pro via Qwen Code /review |
||||||||||||||||||||||||
| message = this.queue.dequeue(); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -388,6 +388,19 @@ export class GeminiChat { | |||||||||||||||||||||||||||||||||||||||||||||
| // model. | ||||||||||||||||||||||||||||||||||||||||||||||
| private sendPromise: Promise<void> = Promise.resolve(); | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||
| * Heap memory threshold (2 GB). When `heapUsed` exceeds this, chat | ||||||||||||||||||||||||||||||||||||||||||||||
| * compression is forced regardless of `hasFailedCompressionAttempt`. | ||||||||||||||||||||||||||||||||||||||||||||||
| * This is a memory safety net independent of the 70% token compaction | ||||||||||||||||||||||||||||||||||||||||||||||
| * threshold — catching the actual root cause (heap pressure) rather than | ||||||||||||||||||||||||||||||||||||||||||||||
| * a proxy (entry count). Protects against both many small entries AND | ||||||||||||||||||||||||||||||||||||||||||||||
| * few huge entries (large file reads, shell outputs). | ||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||
| * Note: set below Node's default 4 GB heap limit so there is headroom | ||||||||||||||||||||||||||||||||||||||||||||||
| * for one more GC cycle before the process is killed. | ||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||
| private static readonly HEAP_MEMORY_THRESHOLD = 2 * 1024 * 1024 * 1024; // 2 GB | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] Hardcoded 2GB threshold is a no-op on small-memory containers, and the safety net has zero observability. In Docker/K8s with
Suggested change
Additionally, neither this heap trigger nor — DeepSeek/deepseek-v4-pro via Qwen Code /review |
||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||
| * Per-chat last-prompt-token-count, populated from `usageMetadata` on each | ||||||||||||||||||||||||||||||||||||||||||||||
| * model response. Used by the compaction threshold check so that subagents | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -464,6 +477,18 @@ export class GeminiChat { | |||||||||||||||||||||||||||||||||||||||||||||
| signal?: AbortSignal, | ||||||||||||||||||||||||||||||||||||||||||||||
| options?: TryCompressOptions, | ||||||||||||||||||||||||||||||||||||||||||||||
| ): Promise<ChatCompressionInfo> { | ||||||||||||||||||||||||||||||||||||||||||||||
| // Force compression when heapUsed exceeds the memory threshold, | ||||||||||||||||||||||||||||||||||||||||||||||
| // regardless of `hasFailedCompressionAttempt`. This is a memory | ||||||||||||||||||||||||||||||||||||||||||||||
| // safety net — catches the actual root cause (heap pressure) rather | ||||||||||||||||||||||||||||||||||||||||||||||
| // than a proxy (entry count). Protects against both many small entries | ||||||||||||||||||||||||||||||||||||||||||||||
| // AND few huge entries (large file reads, shell outputs). | ||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] Force compression death loop: when heap > 2GB,
Suggested change
— DeepSeek/deepseek-v4-pro via Qwen Code /review
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] Heap-triggered
Suggested change
— DeepSeek/deepseek-v4-pro via Qwen Code /review |
||||||||||||||||||||||||||||||||||||||||||||||
| if ( | ||||||||||||||||||||||||||||||||||||||||||||||
| !force && | ||||||||||||||||||||||||||||||||||||||||||||||
| process.memoryUsage().heapUsed > GeminiChat.HEAP_MEMORY_THRESHOLD | ||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Critical] Heap-based (1) OOM race: (2) API retry loop:
Suggested change
— DeepSeek/deepseek-v4-pro via Qwen Code /review |
||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||
| force = true; | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| const service = new ChatCompressionService(); | ||||||||||||||||||||||||||||||||||||||||||||||
| const { newHistory, info } = await service.compress(this, { | ||||||||||||||||||||||||||||||||||||||||||||||
| promptId, | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Critical]
pruneMessages()has two structural issues:agent-interactive.ts:129. All subagents, background tasks, andAgentHeadlessusage never prune — exactly the long-running scenarios that need it most.Also add
this.core.pruneMessages()inAgentHeadless.execute()after each reasoning round.— DeepSeek/deepseek-v4-pro via Qwen Code /review