Fix editor panic from overlapping V4A diff deltas (WARP-CLIENT-DEV-NYY)#10186
Fix editor panic from overlapping V4A diff deltas (WARP-CLIENT-DEV-NYY)#10186kevinyang372 merged 3 commits intomasterfrom
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
kevinyang372
changed the title
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
| delta.replacement_line_range.start < prev.replacement_line_range.end | ||
| }); | ||
| if dominated { | ||
| log::warn!( |
There was a problem hiding this comment.
should this be an error? Just wondering if this is a thing quality should be aware of/worrying about or if it doesn't really matter
There was a problem hiding this comment.
I think of error as an unexpected state we shouldn't have gotten into
This is totally a reasonable state that could have happened if LLM outputs the diffs this way. It also self-heals since LLM will see the echoed back resulting state. I am not worried about putting this as warn level
|
|
||
| // If every action in the batch was skipped (e.g. all had inverted ranges), | ||
| // there is nothing to commit — return early. | ||
| if new_range_anchors.is_empty() { |
There was a problem hiding this comment.
could we surface this in some way to the agent (maybe in the form of a failed tool call or something?). Just feels like it should be aware if the edit wasn't applied
There was a problem hiding this comment.
Related to my comment above. The agent will see the resulting file state after the applied diff
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This change deduplicates overlapping V4A diff deltas before editor application and adds a defensive editor-side skip for inverted ranges.
Concerns
- Overlap deduplication currently drops every later overlapping delta, even when the later range only partially overlaps or extends beyond the previous accepted range, so requested changes can be lost while the diff is reported as successfully matched.
- Security pass: no actionable security concerns found.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
2 participants
Problem
Sentry issue
WARP-CLIENT-DEV-NYY:panic: Invalid edit range 4042..3982inBuffer::edit.When the AI agent produces a multi-hunk V4A diff where one hunk's matched range subsumes another's (e.g. a large deletion whose context window overlaps a nearby single-line edit),
fuzzy_match_v4a_diffsproducesDiffDeltas with overlappingreplacement_line_rangevalues. WhenCodeEditorModel::apply_diffslater converts these to char offsets and feeds them toinsert_at_offsets, the editor'sBuffer::editpanics on the invalid range.Confirmed via MAA conversation
d71bf84b(requestb621adb3): a 17-hunk V4A diff againstmod.rsproduced overlapping deltas (lines 46..71 subsuming 64..65).Fix
Diff layer (
crates/ai/src/diff_validation/mod.rs):deduplicate_overlapping_deltas()— after matching all hunks, sort deltas by start line and drop any delta whose range overlaps with a preceding one.Editor safeguard (
crates/editor/src/content/core.rs):apply_core_edit_actions(), skip any action whose anchor-resolved range hasstart > endinstead of panicking. ReturnsEditResult::default()if all actions are skipped.Tests
test_v4a_maa_crash_d71bf84b_no_overlapping_deltas— uses the exact V4A hunks from the crash to verify the subsumed delta is dropped (1 delta survives, not 2).test_insert_at_offsets_overlapping_ranges_skipped— passes inverted char-offset range (4042..3982) toinsert_at_offsetsand verifies the buffer is unchanged (edit gracefully skipped).Fixes WARP-CLIENT-DEV-NYY