fix(edit): search quote-normalized text with str::find - #2480
Open
thaildhe172591 wants to merge 1 commit into
Open
fix(edit): search quote-normalized text with str::find#2480thaildhe172591 wants to merge 1 commit into
thaildhe172591 wants to merge 1 commit into
Conversation
find_actual_string compared every char window of the file against the search string by hand. The comparison short-circuits on the first mismatch, so ordinary source stays fast, but a file holding long runs of one character (minified output, padded or aligned text) makes most windows share a deep prefix with the search string and the scan turns quadratic: 256KB of spaces with a 2KB old_string took over 20 seconds per candidate on this machine, and edit_string_candidates can probe several candidates per failed edit. Quote normalization maps one char to one char, so normalizing both sides once and searching with str::find finds the same leftmost match in linear time. The returned slice is cut from the line-ending normalized file by char offset, which byte offsets cannot do once multibyte text precedes the match; a new test covers that case, and a second test guards the runtime on long repeated runs. Validation: cargo test --locked -p tool-runtime --lib; cargo test --locked -p bitfun-core --lib; cargo check --locked --workspace; cargo fmt -p tool-runtime -- --check. Refs: GCWing#1650 AI: fully tested
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
find_actual_stringinedit_file.rscompared every character window of the file against the search string by hand. The comparison short-circuits on the first mismatch, so ordinary source files stay fast, but a file holding long runs of one character (minified output, padded or aligned text, whitespace-heavy data) makes most windows share a deep prefix with the search string and the scan turns quadratic. Measured on this machine (debug build): 256KB of spaces with a 2KBold_stringtook 32s for one call, andedit_string_candidatescan probe several candidates per failed edit, so a single failed Edit on such a file hangs for minutes.Quote normalization maps one char to one char, so this change normalizes both sides once and searches with
str::find, which finds the same leftmost match in linear time. The same 256KB input now completes in about 68ms, and a 512KB input went from 126s to 131ms. The returned slice is cut from the line-ending-normalized file by char offset, which byte offsets cannot do once multibyte text precedes the match; a new test covers exactly that case, and a second test guards against the scan regressing to quadratic on long repeated runs.This addresses the hang half of #1650. The token-amplification half of that report is about how many times file content re-enters the model context after a failed edit, which lives in the read-state/
validate_inputflow, not in this function; I have left it alone.References #1650
Type and Areas
Type:
Bug fix (performance).
Areas:
Rust core (
tool-runtime,src/crates/execution/tool-execution).Motivation / Impact
Editing large files with repetitive content could hang the Edit tool for minutes on a single failed match. After this change the fallback scan stays linear. No behavior change for edits that already matched: all existing candidates and fallbacks resolve to the same result, only faster.
Verification
cargo test --locked -p tool-runtime --lib- 132 passed, 0 failed (includes the two new tests)apply_edit_scans_long_repeated_runs_without_hangingfails at 23.7s; with this change it passes in well under a secondcargo test --locked -p bitfun-core --lib- 150 passed, 0 failedcargo check --locked --workspace- cleancargo fmt -p tool-runtime -- --check- cleanNote: CI's tool-runtime step only runs
--lib search::, so thefs::tests here do not run in CI; the counts above are from local runs on Windows.AI-assisted: fully tested (see verification above).
Reviewer Notes
The subtle point is the byte/char offset distinction:
str::findreturns a byte offset into the quote-normalized string, while the returned slice must come from the pre-normalization string. Char offsets line up between the two becausenormalize_quote_charis a char-to-char map; byte offsets do not once any multibyte character precedes the match.apply_edit_matches_curly_quotes_after_multibyte_contentfails if that is ever gotten wrong.Checklist