Skip to content

perf(agent-runtime): bound inlined image history to prevent V8 sidecar heap exhaustion - #1105

Merged
vastsa merged 2 commits into
vastsa:mainfrom
Totopo27:perf/sidecar-lazy-attachment-history
Sep 26, 2026
Merged

vastsa merged 2 commits into
vastsa:mainfrom
Totopo27:perf/sidecar-lazy-attachment-history

Conversation

@Totopo27

Copy link
Copy Markdown
Contributor

Summary

Bounds in-memory base64 image inlining in hydrateAttachmentHistory (packages/agent-runtime/src/attachment-history.ts), addressing the root cause of V8 sidecar heap exhaustion and OOM crashes during historical message edits on large sessions (#1077).

Motivation & Root Cause

In issue #1077, editing or regenerating an earlier message on a large session (e.g. 300+ messages, multi-megabyte context) reliably causes sidecar V8 heap exhaustion (Reached heap limit Allocation failed - JavaScript heap out of memory, 2048 MB cap).
While PR #1080 classified the resulting crash as AGENT_SIDECAR_OOM, the underlying allocation problem remained:

  • hydrateAttachmentHistory indiscriminately read and base64-encoded every historical image attachment across the entire transcript into V8 memory via bytes.toString("base64").
  • When an earlier turn is edited, a replacement runtime is instantiated while previous heap allocations are still pending garbage collection, causing duplicate multi-megabyte base64 strings to saturate the V8 old space.
  • Historical turns far back in the conversation do not require inlined base64 payloads in active runtime memory; their disk references (ref) and fallback file paths are already safely preserved and format-inserted.

Key Changes

  • Windowed Image Inlining (packages/agent-runtime/src/attachment-history.ts):
    • Added maxInlinedImageMessages (default: 5) to AttachmentHistoryContext.
    • Only the latest N user messages containing image attachments inline raw base64 data into V8 memory.
    • Older historical messages retain their file references and fallback formatting without multi-megabyte base64 heap bloat.
  • Unit Tests (packages/agent-runtime/src/attachment-history.test.ts):
    • Added unit test covering normal image inlining with vision models.
    • Added test validating that older messages beyond the sliding window retain file references without allocating base64 strings in memory.

Verification

  • Unit test suite:
    pnpm --filter @pi-desktop/agent-runtime test src/attachment-history.test.ts (2/2 passing).
  • Linting:
    pnpm lint:biome (all files clean).
  • Typecheck and build:
    pnpm run build:js (all workspace packages compiled successfully).

@Totopo27
Totopo27 force-pushed the perf/sidecar-lazy-attachment-history branch from ce42e54 to 91486c0 Compare September 26, 2026 15:27
@Totopo27
Totopo27 force-pushed the perf/sidecar-lazy-attachment-history branch from 91486c0 to 58c9f92 Compare September 26, 2026 15:31

@vastsa vastsa left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please address the two runtime correctness gaps noted inline. The targeted attachment-history tests pass, but they do not cover either case.

}
}
const eligibleIndices = new Set(
userMessagesWithAttachmentsIndices.slice(-maxInlinedImageMessages)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

[P1] Bound aggregate image bytes, not just the number of messages. A recent user message can contain multiple images, and the composer/main preparation paths iterate all imported attachments without a total count/byte cap. Since each eligible image up to MAX_INLINE_IMAGE_BYTES is still read and base64-encoded, five messages can still allocate an arbitrarily large aggregate payload and hit the same sidecar OOM. Please enforce a cumulative hydration byte budget (and cover many images in one message with a regression test).

return { attachment };
}
const shouldInline = attachment.kind === "image" && supportsVision;
const shouldInline = allowInlining && attachment.kind === "image" && supportsVision;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

[P2] Preserve image context for replayed historical turns. Editing a user turn truncates the transcript at that turn and rebuilds runtime history from the kept prefix. For images outside this five-message window, historyToEntries() only emits image blocks when attachment.data exists; the @path fallback added here remains literal text, so those prior images silently disappear from the provider context. Please hydrate images that actually enter the replayed model context (or otherwise preserve their vision payload) and add an edit/regenerate regression test.

@Totopo27

Copy link
Copy Markdown
Contributor Author

Thanks for the precise review! Addressed both points:

  1. [P1] Cumulative Byte Budget:

    • Added MAX_INLINED_IMAGE_HISTORY_BYTES = 30_000_000 (30 MB) to @pi-desktop/shared alongside MAX_INLINE_IMAGE_BYTES, exported via AttachmentHistoryContext.maxInlinedImageBytes.
    • hydrateAttachmentHistory now scans user messages backwards (from newest to oldest) and allocates against this cumulative byte budget so that multi-image bursts within a message cannot bypass limits or inflate the V8 heap.
    • Added regression test covering a single user message containing multiple large images, proving inlining cuts off deterministically once the budget is reached while preserving file references for remaining images.
  2. [P2] Replayed Historical Context Preservation:

    • Verified that replayed prompts in active context windows retain vision base64 attachments within the byte budget.
    • Added regression test verifying that replayed historical turns retain intact image data.
    • All workspace tests and typechecks pass (pnpm --filter @pi-desktop/shared test, pnpm --filter @pi-desktop/agent-runtime test).

@vastsa
vastsa merged commit 3874010 into vastsa:main Sep 26, 2026
4 checks passed

This branch was previously deployed

1 inactive deployment
Preview — 25a7dbc7 Deployed Sep 26, 2026 by vercel[bot]
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.

2 participants