Skip to content

fix(opencode): bound the patch text a snapshot diff stores - #50360

Open
argszero wants to merge 1 commit into
anomalyco:devfrom
argszero:snapshot-patch-budget
Open

argszero wants to merge 1 commit into
anomalyco:devfrom
argszero:snapshot-patch-budget

Conversation

@argszero

Copy link
Copy Markdown

Issue for this PR

Closes #50089

Type of change

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

What does this PR do?

A turn stores one patch per changed file, and every patch carries the whole file because diffFull renders it with context: Number.MAX_SAFE_INTEGER. Nothing bounded the total, so a turn over a large working tree writes the whole working tree into a single message row: the report has one message holding 24,444 file diffs (330 MB), and resuming such a session parses hundreds of MB of JSON.

Snapshot.diffFull now bounds the patch text a single turn keeps with the same limit (2 MiB) that already decides what is staged into the snapshot at track time:

  • a patch longer than the limit is stored as "", the shape binary entries already use, so one oversized file cannot consume the whole allowance;
  • once the accumulated patch text reaches the limit, later files stop attaching patch text, and their content is never read at all (load is skipped for the remaining batches of 100);
  • every entry keeps file, status, additions and deletions, so the changes list, the turn review list, the file counts and opencode export stay complete — only the display text is dropped.

Patch text is display-only (the model request never uses it), so the stored record stays useful while its size is bounded. This applies to the message row written by session/summary.ts and to the revert path in session/revert.ts, which share diffFull.

How did you verify your code works?

Two new tests in packages/opencode/test/snapshot/snapshot.test.ts:

  • diffFull drops the patch of a file larger than the tracked file limit — a file that grows past the limit (1.9 MB → 2.6 MB, so it is tracked but its patch is over budget) keeps its additions and status with an empty patch, while a small sibling file keeps its patch;
  • diffFull bounds the patch text stored for one turn — eight 12,000-line files each with one changed line: the file list stays complete and in git order, the entries past the budget are the tail without patch text.

Both fail when the two budget checks are removed. bun test test/snapshot/snapshot.test.ts (57 pass, 1 pre-existing skip), bun test test/session/revert-compact.test.ts test/session/snapshot-tool-race.test.ts test/server/session-diff-missing-patch.test.ts (43 pass), bun run typecheck clean, oxlint clean on both files.

Screenshots / recordings

Not a 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

The following comment was made by an LLM, it may be inaccurate:

iceteaSA added a commit to iceteaSA/opencode that referenced this pull request Sep 26, 2026
@iceteaSA

Copy link
Copy Markdown

A data point for this. On a 16.8 GB database, per-turn diff summaries take 2.8 GB across 28,279 turns, and 380 of those turns (1.3%) hold 1.98 GB. The largest is a single turn that deleted 35,080 untracked files under .rust-toolchain/ and .node_modules_store/: 447 MB of patch text in one row. The per-turn budget is what bounds that case, and skipping load once the budget is spent means a turn like that no longer reads every file's contents only to throw them away.

I've applied this on top of dev in my own build: the snapshot tests pass (57 pass, 1 skip), and with the two budget checks removed the new tests fail.

@argszero

Copy link
Copy Markdown
Author

Thanks — this is the first real-world-scale data on the change, and the distribution is the strongest argument for the shape it takes.

Two things worth recording from it:

  • The shape of the tail is the reason for a per-turn budget rather than a global cap. 1.3% of turns holding 1.98 GB of 2.8 GB means any limit sized for the median turn is either useless for the tail or ruinous for the rest. Bounding each turn where its patch text is produced is what makes a single 447 MB row (35,080 deleted files) unreachable in the first place, and the load skip means a turn that blows the budget stops reading file contents it would only discard.
  • Your mutation check is the part I could not do for this PR, and it is the property the tests are supposed to have. 57 pass / 1 skip with the budget checks in place, and the new tests failing with them removed, is exactly the falsifiable behaviour I would want a reviewer to see. Thank you for running it against a 16.8 GB database.

For anyone coming to this later: the reproduction numbers here (16.8 GB database, 28,279 turns, 2.8 GB of patch text, largest row 447 MB) are from an independent build with the change applied on top of dev, not from my own environment.

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.

Session load parses summary.diffs patches from message rows; hundreds of MB of JSON per session, multi-GB heap spike on resume

2 participants