diff --git a/crates/agent/src/tools/edit_file_tool.rs b/crates/agent/src/tools/edit_file_tool.rs index 4734dd6b36..bca682c038 100644 --- a/crates/agent/src/tools/edit_file_tool.rs +++ b/crates/agent/src/tools/edit_file_tool.rs @@ -324,6 +324,79 @@ mod tests { assert_eq!(new_text, "line 1\nmodified line 2\nline 3\n"); } + #[gpui::test] + async fn test_streaming_edit_exact_fragments(cx: &mut TestAppContext) { + let content = concat!( + "fn spaces() {\n", + " spaces_old();\n", + "}\n", + "fn tabs() {\n", + "\ttabs_old();\n", + "}\n", + "controls: keyboard WASD, voxel-based\n", + "prefix OLD suffix\n", + "foo suffix\n", + "foo\n", + ); + let (edit_tool, _project, _action_log, _fs, _thread) = + setup_test(cx, json!({"file.rs": content})).await; + let result = cx + .update(|cx| { + edit_tool.clone().run( + ToolInput::resolved(EditFileToolInput { + path: "root/file.rs".into(), + edits: vec![ + Edit { + old_text: "keyboard WASD, voxel-based".into(), + new_text: "arrow keys".into(), + }, + Edit { + old_text: "spaces_old();".into(), + new_text: "spaces_new();\nspaces_more();".into(), + }, + Edit { + old_text: "tabs_old();".into(), + new_text: "tabs_new();\ntabs_more();".into(), + }, + Edit { + old_text: "OLD".into(), + new_text: "NEW\n".into(), + }, + Edit { + old_text: "foo\n".into(), + new_text: "bar\n".into(), + }, + ], + }), + ToolCallEventStream::test().0, + cx, + ) + }) + .await; + + let EditFileToolOutput::Success { new_text, .. } = result.unwrap() else { + panic!("expected success"); + }; + assert_eq!( + new_text, + concat!( + "fn spaces() {\n", + " spaces_new();\n", + " spaces_more();\n", + "}\n", + "fn tabs() {\n", + "\ttabs_new();\n", + "\ttabs_more();\n", + "}\n", + "controls: arrow keys\n", + "prefix NEW\n", + " suffix\n", + "foo suffix\n", + "bar\n", + ) + ); + } + #[gpui::test] async fn test_streaming_edit_first_line_missing_indent(cx: &mut TestAppContext) { // Reproduces https://github.com/zed-industries/zed/issues/60302: the @@ -627,6 +700,33 @@ mod tests { ); } + #[gpui::test] + async fn test_streaming_edit_rejects_overlapping_matches(cx: &mut TestAppContext) { + let (edit_tool, _project, _action_log, _fs, _thread) = + setup_test(cx, json!({"file.txt": "aaaaa"})).await; + let result = cx + .update(|cx| { + edit_tool.clone().run( + ToolInput::resolved(EditFileToolInput { + path: "root/file.txt".into(), + edits: vec![Edit { + old_text: "aaaa".into(), + new_text: "replacement".into(), + }], + }), + ToolCallEventStream::test().0, + cx, + ) + }) + .await; + + let EditFileToolOutput::Error { error, diff, .. } = result.unwrap_err() else { + panic!("expected error"); + }; + assert!(error.contains("matched multiple locations")); + assert!(diff.is_empty()); + } + /// When the edit fails after a session is created but before any edits are /// actually applied (e.g., the first `old_text` doesn't match), the empty /// diff placeholder in the UI should be replaced with the error message. diff --git a/crates/agent/src/tools/edit_session.rs b/crates/agent/src/tools/edit_session.rs index bf5308e90d..f96f84c7f6 100644 --- a/crates/agent/src/tools/edit_session.rs +++ b/crates/agent/src/tools/edit_session.rs @@ -16,14 +16,14 @@ use language::{Buffer, BufferEditSource, BufferEvent, LanguageRegistry}; use language_model::LanguageModelToolResultContent; use project::lsp_store::{FormatTrigger, LspFormatTarget}; use project::{AgentLocation, Project, ProjectPath}; -use reindent::{Reindenter, compute_indent_delta, compute_rest_indent_delta}; +use reindent::{IndentDelta, Reindenter, compute_indent_delta, compute_rest_indent_delta}; use schemars::JsonSchema; use serde::{Deserialize, Serialize, de::DeserializeOwned}; use std::ops::Range; use std::path::PathBuf; use std::sync::Arc; use streaming_diff::{CharOperation, StreamingDiff}; -use streaming_fuzzy_matcher::StreamingFuzzyMatcher; +use streaming_fuzzy_matcher::{SearchMatch, SearchMatches, StreamingFuzzyMatcher}; use streaming_parser::{EditEvent, StreamingParser, WriteEvent}; use text::ToOffset; use ui::SharedString; @@ -393,6 +393,7 @@ enum EditPipelineEntry { edit_cursor: usize, reindenter: Reindenter, original_snapshot: text::BufferSnapshot, + strip_trailing_newline: bool, }, } @@ -506,7 +507,10 @@ impl EditPipeline { if !chunk.is_empty() { matcher.push(chunk, None); } - let range = extract_match( + let ResolvedMatch { + search_match: SearchMatch { range, line_pairs }, + is_exact, + } = extract_match( matcher.finish(), buffer, edit_index, @@ -535,16 +539,20 @@ impl EditPipeline { .unwrap_or("") .chars(), ); - let first_line_delta = compute_indent_delta(buffer_indent, query_indent); - + let contextual_delta = compute_indent_delta(buffer_indent, query_indent); + let first_line_delta = if is_exact { + // An exact range may start mid-line, where the retained prefix already + // provides the first replacement line's indentation. + IndentDelta::Spaces(0) + } else { + contextual_delta + }; // Query row 0 is excluded: its delta is `first_line_delta`, // which intentionally differs when the model stripped the // first line's indentation. let rest_delta = compute_rest_indent_delta( - first_line_delta, - matcher - .line_pairs(&range) - .unwrap_or(&[]) + contextual_delta, + line_pairs .iter() .filter(|(query_row, _)| *query_row != 0) .filter_map(|(query_row, buffer_row)| { @@ -556,6 +564,7 @@ impl EditPipeline { }), ); + let strip_trailing_newline = snapshot.contains_str_at(range.end, "\n"); let old_text_in_buffer = snapshot.text_for_range(range.clone()).collect::(); log::debug!( @@ -572,6 +581,7 @@ impl EditPipeline { edit_cursor: range.start, reindenter: Reindenter::with_deltas(first_line_delta, rest_delta), original_snapshot: text_snapshot, + strip_trailing_newline, }); cx.update(|cx| { @@ -625,11 +635,17 @@ impl EditPipeline { mut edit_cursor, mut reindenter, original_snapshot, + strip_trailing_newline, }) = self.current_edit.take() else { return Ok(()); }; + let chunk = if strip_trailing_newline { + chunk.strip_suffix('\n').unwrap_or(chunk) + } else { + chunk + }; let mut final_text = reindenter.push(chunk); final_text.push_str(&reindenter.finish()); @@ -933,32 +949,46 @@ fn apply_char_operations( } } +struct ResolvedMatch { + search_match: SearchMatch, + is_exact: bool, +} + fn extract_match( - matches: Vec>, + matches: SearchMatches, buffer: &Entity, edit_index: &usize, file_changed_since_last_read: bool, cx: &mut AsyncApp, -) -> Result, String> { +) -> Result { + let (matches, is_exact) = match matches { + SearchMatches::Exact(matches) => (matches, true), + SearchMatches::Fuzzy(matches) => (matches, false), + }; let file_changed_since_last_read_message = if file_changed_since_last_read { " The file has changed on disk since you last read it." } else { "" }; - match matches.len() { - 0 => Err(format!( + match matches.as_slice() { + [] => Err(format!( "Could not find matching text for edit at index {}. \ The old_text did not match any content in the file.{} \ Please read the file again to get the current content.", edit_index, file_changed_since_last_read_message, )), - 1 => Ok(matches.into_iter().next().unwrap()), + [search_match] => Ok(ResolvedMatch { + search_match: search_match.clone(), + is_exact, + }), _ => { let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot()); let lines = matches .iter() - .map(|range| (snapshot.offset_to_point(range.start).row + 1).to_string()) + .map(|search_match| { + (snapshot.offset_to_point(search_match.range.start).row + 1).to_string() + }) .collect::>() .join(", "); Err(format!( diff --git a/crates/agent/src/tools/edit_session/streaming_fuzzy_matcher.rs b/crates/agent/src/tools/edit_session/streaming_fuzzy_matcher.rs index 3515275f70..321fb1c353 100644 --- a/crates/agent/src/tools/edit_session/streaming_fuzzy_matcher.rs +++ b/crates/agent/src/tools/edit_session/streaming_fuzzy_matcher.rs @@ -4,6 +4,9 @@ use std::{cmp, ops::Range}; const REPLACEMENT_COST: u32 = 1; const INSERTION_COST: u32 = 3; const DELETION_COST: u32 = 10; +// Two matches are enough to prove ambiguity, so stop scanning early; exact +// matches are byte-identical and can't be ranked. +const MAX_EXACT_MATCHES: usize = 2; /// A streaming fuzzy matcher that can process text chunks incrementally /// and return the best match found so far at each step. @@ -12,6 +15,7 @@ pub struct StreamingFuzzyMatcher { query_lines: Vec, line_hint: Option, incomplete_line: String, + raw_query: String, matches: Vec, matrix: SearchMatrix, } @@ -19,9 +23,15 @@ pub struct StreamingFuzzyMatcher { /// A match candidate: the matched byte range plus the 0-based /// `(query_row, buffer_row)` line pairs the search aligned to produce it. #[derive(Clone, Debug)] -struct SearchMatch { - range: Range, - line_pairs: Vec<(u32, u32)>, +pub(super) struct SearchMatch { + pub(super) range: Range, + pub(super) line_pairs: Vec<(u32, u32)>, +} + +#[derive(Debug)] +pub(super) enum SearchMatches { + Exact(Vec), + Fuzzy(Vec), } impl StreamingFuzzyMatcher { @@ -32,6 +42,7 @@ impl StreamingFuzzyMatcher { query_lines: Vec::new(), line_hint: None, incomplete_line: String::new(), + raw_query: String::new(), matches: Vec::new(), matrix: SearchMatrix::new(buffer_line_count + 1), } @@ -42,23 +53,6 @@ impl StreamingFuzzyMatcher { &self.query_lines } - /// Returns the 0-based `(query_row, buffer_row)` line pairs that the - /// search aligned for the match with the given range. Lines that were - /// skipped on either side of the alignment are absent. - pub fn line_pairs(&self, range: &Range) -> Option<&[(u32, u32)]> { - self.matches - .iter() - .find(|search_match| search_match.range == *range) - .map(|search_match| search_match.line_pairs.as_slice()) - } - - fn match_ranges(&self) -> Vec> { - self.matches - .iter() - .map(|search_match| search_match.range.clone()) - .collect() - } - /// Push a new chunk of text and get the best match found so far. /// /// This method accumulates text chunks and processes complete lines. @@ -70,6 +64,7 @@ impl StreamingFuzzyMatcher { /// query so far, or `None` if no suitable match exists yet. pub fn push(&mut self, chunk: &str, line_hint: Option) -> Option> { // Add the chunk to our incomplete line buffer + self.raw_query.push_str(chunk); self.incomplete_line.push_str(chunk); self.line_hint = line_hint; @@ -98,7 +93,26 @@ impl StreamingFuzzyMatcher { /// /// This processes any remaining incomplete line before returning the final /// match result. - pub fn finish(&mut self) -> Vec> { + pub fn finish(&mut self) -> SearchMatches { + let exact_ranges = find_exact_matches(&self.snapshot, &self.raw_query); + if !exact_ranges.is_empty() { + if !self.incomplete_line.is_empty() { + self.query_lines + .push(std::mem::take(&mut self.incomplete_line)); + } + let matches = exact_ranges + .into_iter() + .map(|range| { + let start_row = self.snapshot.offset_to_point(range.start).row; + let line_pairs = (0..self.query_lines.len()) + .map(|query_row| (query_row as u32, start_row + query_row as u32)) + .collect(); + SearchMatch { range, line_pairs } + }) + .collect(); + return SearchMatches::Exact(matches); + } + // Process any remaining incomplete line if !self.incomplete_line.is_empty() { if let [only_match] = self.matches.as_mut_slice() { @@ -118,7 +132,7 @@ impl StreamingFuzzyMatcher { only_match .line_pairs .push(((self.query_lines.len() - 1) as u32, extended_row)); - return self.match_ranges(); + return SearchMatches::Fuzzy(self.matches.clone()); } } @@ -126,7 +140,7 @@ impl StreamingFuzzyMatcher { .push(std::mem::take(&mut self.incomplete_line)); self.matches = self.resolve_location_fuzzy(); } - self.match_ranges() + SearchMatches::Fuzzy(self.matches.clone()) } fn resolve_location_fuzzy(&mut self) -> Vec { @@ -285,6 +299,62 @@ impl StreamingFuzzyMatcher { } } +// Aho-Corasick's overlapping search requires a contiguous haystack, while its +// streaming search skips overlapping matches. KMP detects overlapping ambiguity +// while scanning rope chunks without copying the buffer. +fn find_exact_matches(snapshot: &TextBufferSnapshot, query: &str) -> Vec> { + if query.is_empty() { + return Vec::new(); + } + + let query = query.as_bytes(); + let trailing_line_ending_len = if query.ends_with(b"\r\n") { + 2 + } else if query.ends_with(b"\n") { + 1 + } else { + 0 + }; + let mut prefix_lengths = vec![0; query.len()]; + let mut prefix_length = 0; + for query_index in 1..query.len() { + while prefix_length > 0 && query[query_index] != query[prefix_length] { + prefix_length = prefix_lengths[prefix_length - 1]; + } + if query[query_index] == query[prefix_length] { + prefix_length += 1; + prefix_lengths[query_index] = prefix_length; + } + } + + let mut matches = Vec::new(); + let mut matched_length = 0; + for (offset, byte) in snapshot + .bytes_in_range(0..snapshot.len()) + .flatten() + .copied() + .enumerate() + { + while matched_length > 0 && byte != query[matched_length] { + matched_length = prefix_lengths[matched_length - 1]; + } + if byte == query[matched_length] { + matched_length += 1; + } + if matched_length == query.len() { + let raw_end = offset + 1; + let start = raw_end - query.len(); + let end = raw_end - trailing_line_ending_len; + matches.push(start..end); + if matches.len() == MAX_EXACT_MATCHES { + break; + } + matched_length = prefix_lengths[matched_length - 1]; + } + } + matches +} + fn fuzzy_eq(left: &str, right: &str) -> bool { const THRESHOLD: f64 = 0.8; @@ -563,7 +633,7 @@ mod tests { assert_location_resolution( concat!( " Lorem\n", - "« ipsum»\n", + " «ipsum»\n", " dolor sit amet\n", " consecteur", ), @@ -776,6 +846,111 @@ mod tests { ); } + #[gpui::test] + fn test_exact_match_takes_precedence_over_fuzzy_match() { + let buffer = TextBuffer::new( + ReplicaId::LOCAL, + BufferId::new(1).unwrap(), + concat!( + "prefix keyboard WASD, voxel-based suffix\n", + "keyboard WASD, voxel-baseX\n", + ), + ); + let snapshot = buffer.snapshot(); + let mut matcher = StreamingFuzzyMatcher::new(snapshot.clone()); + + assert_eq!(matcher.push("keyboard WASD, voxel-based", None), None); + let SearchMatches::Exact(matches) = matcher.finish() else { + panic!("expected an exact match"); + }; + let [search_match] = matches.as_slice() else { + panic!("expected one match, got {}", matches.len()); + }; + assert_eq!( + snapshot + .text_for_range(search_match.range.clone()) + .collect::(), + "keyboard WASD, voxel-based" + ); + } + + #[gpui::test] + fn test_exact_match_uses_trailing_newline_to_disambiguate() { + let buffer = TextBuffer::new( + ReplicaId::LOCAL, + BufferId::new(1).unwrap(), + "foo suffix\nfoo\n", + ); + let snapshot = buffer.snapshot(); + let mut matcher = StreamingFuzzyMatcher::new(snapshot.clone()); + + matcher.push("foo\n", None); + let SearchMatches::Exact(matches) = matcher.finish() else { + panic!("expected an exact match"); + }; + let [search_match] = matches.as_slice() else { + panic!("expected one match, got {}", matches.len()); + }; + assert_eq!( + snapshot + .text_for_range(search_match.range.clone()) + .collect::(), + "foo" + ); + } + + #[gpui::test] + fn test_exact_newline_only_match_excludes_line_ending() { + let buffer = TextBuffer::new(ReplicaId::LOCAL, BufferId::new(1).unwrap(), "a\nb"); + let mut matcher = StreamingFuzzyMatcher::new(buffer.snapshot().clone()); + + matcher.push("\n", None); + let SearchMatches::Exact(matches) = matcher.finish() else { + panic!("expected an exact match"); + }; + let [search_match] = matches.as_slice() else { + panic!("expected one match, got {}", matches.len()); + }; + assert_eq!(search_match.range, 1..1); + } + + #[gpui::test] + fn test_exact_overlapping_matches_are_ambiguous() { + let buffer = TextBuffer::new(ReplicaId::LOCAL, BufferId::new(1).unwrap(), "aaaaa"); + let mut matcher = StreamingFuzzyMatcher::new(buffer.snapshot().clone()); + + matcher.push("aaaa", None); + let matches = matcher.finish(); + + assert!(matches!(matches, SearchMatches::Exact(_))); + assert_eq!(match_ranges(&matches), vec![0..4, 1..5]); + } + + #[gpui::test] + fn test_exact_multiline_match_does_not_extend_incomplete_line() { + let buffer = TextBuffer::new( + ReplicaId::LOCAL, + BufferId::new(1).unwrap(), + "prefix fragment\nnext\nnext\n", + ); + let snapshot = buffer.snapshot(); + let mut matcher = StreamingFuzzyMatcher::new(snapshot.clone()); + + assert_eq!(matcher.push("fragment\nnext", None), None); + let SearchMatches::Exact(matches) = matcher.finish() else { + panic!("expected an exact match"); + }; + let [search_match] = matches.as_slice() else { + panic!("expected one match, got {}", matches.len()); + }; + assert_eq!( + snapshot + .text_for_range(search_match.range.clone()) + .collect::(), + "fragment\nnext" + ); + } + #[gpui::test] fn test_prefix_of_last_line_resolves_to_correct_range() { let text = indoc! {r#" @@ -797,26 +972,32 @@ mod tests { ); let snapshot = buffer.snapshot(); - // Query with a partial last line. + // Query with a partial last line. This is a verbatim substring of the + // buffer, so it resolves through the exact-match path. let query = "}\n\n\n\nfn render_search"; let mut matcher = StreamingFuzzyMatcher::new(snapshot.clone()); matcher.push(query, None); let matches = matcher.finish(); + let matched = search_matches(&matches); // The match should include the line containing "fn render_search". - let matched_text = matches - .first() - .map(|range| snapshot.text_for_range(range.clone()).collect::()); + let matched_text = matched.first().map(|search_match| { + snapshot + .text_for_range(search_match.range.clone()) + .collect::() + }); assert!( - matches.len() == 1, + matched.len() == 1, "Expected exactly one match, got {}: {:?}", - matches.len(), + matched.len(), matched_text, ); - let matched_text = matched_text.unwrap(); + let Some(matched_text) = matched_text else { + panic!("expected a match"); + }; pretty_assertions::assert_eq!( matched_text, "}\n\n\n\nfn render_search", @@ -840,7 +1021,8 @@ mod tests { matcher.push(chunk, None); } - let actual_ranges = matcher.finish(); + let actual_matches = matcher.finish(); + let actual_ranges = match_ranges(&actual_matches); // If no expected ranges, we expect no match if expected_ranges.is_empty() { @@ -904,13 +1086,13 @@ mod tests { ), None, ); - let matches = matcher.finish(); - - assert_eq!(matches.len(), 1); - assert_eq!( - matcher.line_pairs(&matches[0]), - Some(&[(0, 3), (1, 5), (2, 6), (3, 7)][..]) - ); + let SearchMatches::Fuzzy(matches) = matcher.finish() else { + panic!("expected a fuzzy match"); + }; + let [search_match] = matches.as_slice() else { + panic!("expected one match, got {}", matches.len()); + }; + assert_eq!(search_match.line_pairs, [(0, 3), (1, 5), (2, 6), (3, 7)]); } #[test] @@ -934,14 +1116,19 @@ mod tests { let mut matcher = StreamingFuzzyMatcher::new(buffer.snapshot().clone()); // The last query line is incomplete and gets appended to the match by - // `finish` via verbatim comparison rather than the fuzzy search. - matcher.push("}\n\n\n\nfn render_search", None); - let matches = matcher.finish(); - - assert_eq!(matches.len(), 1); + // `finish` via verbatim comparison rather than the fuzzy search. The + // trailing space after `}` keeps the query from being an exact + // substring of the buffer, so the fuzzy extension path is exercised. + matcher.push("} \n\n\n\nfn render_search", None); + let SearchMatches::Fuzzy(matches) = matcher.finish() else { + panic!("expected a fuzzy match"); + }; + let [search_match] = matches.as_slice() else { + panic!("expected one match, got {}", matches.len()); + }; assert_eq!( - matcher.line_pairs(&matches[0]), - Some(&[(0, 2), (1, 3), (2, 4), (3, 5), (4, 6)][..]) + search_match.line_pairs, + [(0, 2), (1, 3), (2, 4), (3, 5), (4, 6)] ); assert_eq!(matcher.query_lines().len(), 5); } @@ -967,11 +1154,26 @@ mod tests { .map(|range| finder.snapshot.text_for_range(range).collect::()) } + fn search_matches(matches: &SearchMatches) -> &[SearchMatch] { + match matches { + SearchMatches::Exact(matches) | SearchMatches::Fuzzy(matches) => matches, + } + } + + fn match_ranges(matches: &SearchMatches) -> Vec> { + search_matches(matches) + .iter() + .map(|search_match| search_match.range.clone()) + .collect() + } + fn finish(mut finder: StreamingFuzzyMatcher) -> Option { let snapshot = finder.snapshot.clone(); let matches = finder.finish(); - matches - .first() - .map(|range| snapshot.text_for_range(range.clone()).collect::()) + search_matches(&matches).first().map(|search_match| { + snapshot + .text_for_range(search_match.range.clone()) + .collect::() + }) } } diff --git a/crates/agent/src/tools/edit_session/streaming_parser.rs b/crates/agent/src/tools/edit_session/streaming_parser.rs index b210dd76e9..62b5c5e370 100644 --- a/crates/agent/src/tools/edit_session/streaming_parser.rs +++ b/crates/agent/src/tools/edit_session/streaming_parser.rs @@ -114,7 +114,7 @@ impl StreamingParser { if partial.new_text.is_some() && !state.buffer_new_text_until_old_text_done { // new_text appeared after old_text, so old_text is done — emit everything. let start = find_char_boundary(old_text, state.old_text_emitted_len); - let chunk = normalize_done_chunk(old_text[start..].to_string()); + let chunk = old_text[start..].to_string(); state.old_text_done = true; state.old_text_emitted_len = old_text.len(); events.push(EditEvent::OldTextChunk { @@ -217,12 +217,12 @@ impl StreamingParser { state.buffer_new_text_until_old_text_done = false; events.push(EditEvent::OldTextChunk { edit_index: index, - chunk: normalize_done_chunk(edit.old_text.clone()), + chunk: edit.old_text.clone(), done: true, }); events.push(EditEvent::NewTextChunk { edit_index: index, - chunk: normalize_done_chunk(edit.new_text.clone()), + chunk: edit.new_text.clone(), done: true, }); continue; @@ -230,7 +230,7 @@ impl StreamingParser { if !state.old_text_done { let start = find_char_boundary(&edit.old_text, state.old_text_emitted_len); - let chunk = normalize_done_chunk(edit.old_text[start..].to_string()); + let chunk = edit.old_text[start..].to_string(); state.old_text_done = true; state.old_text_emitted_len = edit.old_text.len(); events.push(EditEvent::OldTextChunk { @@ -242,7 +242,7 @@ impl StreamingParser { if !state.new_text_done { let start = find_char_boundary(&edit.new_text, state.new_text_emitted_len); - let chunk = normalize_done_chunk(edit.new_text[start..].to_string()); + let chunk = edit.new_text[start..].to_string(); state.new_text_done = true; state.new_text_emitted_len = edit.new_text.len(); events.push(EditEvent::NewTextChunk { @@ -301,12 +301,12 @@ impl StreamingParser { state.buffer_new_text_until_old_text_done = false; events.push(EditEvent::OldTextChunk { edit_index: previous_index, - chunk: normalize_done_chunk(old_text.to_string()), + chunk: old_text.to_string(), done: true, }); events.push(EditEvent::NewTextChunk { edit_index: previous_index, - chunk: normalize_done_chunk(new_text.to_string()), + chunk: new_text.to_string(), done: true, }); return Some(events); @@ -319,7 +319,7 @@ impl StreamingParser { state.old_text_emitted_len = old_text.len(); events.push(EditEvent::OldTextChunk { edit_index: previous_index, - chunk: normalize_done_chunk(old_text[start..].to_string()), + chunk: old_text[start..].to_string(), done: true, }); } @@ -332,7 +332,7 @@ impl StreamingParser { state.buffer_new_text_until_old_text_done = false; events.push(EditEvent::NewTextChunk { edit_index: previous_index, - chunk: normalize_done_chunk(new_text[start..].to_string()), + chunk: new_text[start..].to_string(), done: true, }); } @@ -387,13 +387,6 @@ fn find_char_boundary(text: &str, target: usize) -> usize { pos } -fn normalize_done_chunk(mut chunk: String) -> String { - if chunk.ends_with('\n') { - chunk.pop(); - } - chunk -} - #[cfg(test)] mod tests { use super::*; @@ -555,7 +548,7 @@ mod tests { } #[test] - fn test_done_chunks_strip_trailing_newline() { + fn test_done_chunks_preserve_trailing_newlines() { let mut parser = StreamingParser::default(); let events = parser.finalize_edits(&[Edit { @@ -567,12 +560,12 @@ mod tests { &[ EditEvent::OldTextChunk { edit_index: 0, - chunk: "before".into(), + chunk: "before\n".into(), done: true, }, EditEvent::NewTextChunk { edit_index: 0, - chunk: "after".into(), + chunk: "after\n".into(), done: true, }, ] @@ -580,7 +573,7 @@ mod tests { } #[test] - fn test_partial_edit_chunks_hold_back_trailing_newline() { + fn test_partial_edit_preserves_trailing_newlines() { let mut parser = StreamingParser::default(); let events = parser.push_edits(&[PartialEdit { @@ -598,12 +591,12 @@ mod tests { &[ EditEvent::OldTextChunk { edit_index: 0, - chunk: "before".into(), + chunk: "before\n".into(), done: true, }, EditEvent::NewTextChunk { edit_index: 0, - chunk: "after".into(), + chunk: "after\n".into(), done: true, }, ]