fix(opencode): skip O(N^2) diff computation on large files#20471
fix(opencode): skip O(N^2) diff computation on large files#20471FindTea wants to merge 2 commits into
Conversation
- write/edit/apply_patch: skip createTwoFilesPatch and diffLines for files > 100 KB to avoid blocking the Node.js event loop. The Myers diff algorithm is O(N^2) and was the root cause of 'Preparing write...' / 'Preparing patch...' hanging for several seconds on large files. For oversized files, diff preview is omitted and additions/deletions are approximated by line count. - edit: remove redundant Filesystem.readText + second diff recompute after write; use in-memory contentNew instead. - apply_patch: catch EEXIST from fs.mkdir on Windows (Bun 1.3.x bug where recursive:true still throws when directory already exists). - bash: throttle ctx.metadata() updates to at most once per 100 ms to prevent flooding the event loop during high-volume command output (e.g. compiler output, npm install). Final metadata is always flushed on process close. - write.txt: hint AI to prefer Edit over Write for large files.
github-actions
Bot
added
needs:compliance
|
Hey! Your PR title Please update it to start with one of:
Where See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: Potential Related PR Found:
All other search results returned only the current PR (#20471) itself. No duplicate PRs addressing the same specific issues (diff skipping on large files, bash metadata throttling, or the apply_patch EEXIST Windows fix) were found. |
FindTea
changed the title
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
Note: the e2e failures appear to be pre-existing flaky tests unrelated to this change — other recent PRs show the same failures on the same jobs. |
github-actions
Bot
removed
needs:issue
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
- edit.ts: restore Filesystem.readText after Format.file() so the stored contentNew reflects the formatter's output, not the pre-format content. Removing this was incorrect — the original intent was to capture the final on-disk state for accurate diff/snapshot tracking. - edit.ts, write.ts: restore await on LSP.touchFile so diagnostics are fetched after the LSP has processed the file change, not before.
|
Closing to revise the approach. The byte-size threshold is not the right fix — will switch to using structuredPatch with a timeout (matching how the diff library's timeout option works, similar to how other tools handle this) instead of skipping diff based on file size. |
1 participant
Issue for this PR
Closes #20477
Type of change
What does this PR do?
The write/edit/apply_patch tools call createTwoFilesPatch (Myers diff, O(N^2)) synchronously before showing the permission prompt. When content has many lines this blocks the Node.js event loop, causing the TUI to freeze on 'Preparing write...' / 'Preparing patch...'.
Fix: skip diff computation when content exceeds 100KB. The permission prompt still fires immediately, just without a line-level diff preview.
Also fixes apply_patch throwing EEXIST from fs.mkdir on Windows (Bun 1.3.x bug), and throttles bash tool metadata updates to avoid event-loop flooding during high-volume output.
How did you verify your code works?
Ran bun test test/tool/ locally. All pre-existing passes still pass. Manually tested large file writes and confirmed the hang is gone.
Screenshots / recordings
N/A - not a UI change
Checklist