Fix Sonnet's regression with inserting `</parameter></invoke>` (#41800)

Oleksiy Syvokon created 1 day ago

Sometimes, inside the edit agent, Sonnet thinks that it's doing a tool
call and closes its response with `</parameter></invoke>` instead of
properly closing </new_text>.

A better but more labor-intensive way of fixing this would be switching
to streaming tool calls for LLMs that support it.

Closes #39921

Release Notes:

- Fixed Sonnet's regression with inserting `</parameter></invoke>`
sometimes

Change summary

crates/agent/src/edit_agent/edit_parser.rs | 46 +++++++++++++++++++++++
1 file changed, 45 insertions(+), 1 deletion(-)

Detailed changes

crates/agent/src/edit_agent/edit_parser.rs

@@ -13,7 +13,15 @@ const EDITS_END_TAG: &str = "</edits>";
 const SEARCH_MARKER: &str = "<<<<<<< SEARCH";
 const SEPARATOR_MARKER: &str = "=======";
 const REPLACE_MARKER: &str = ">>>>>>> REPLACE";
-const END_TAGS: [&str; 3] = [OLD_TEXT_END_TAG, NEW_TEXT_END_TAG, EDITS_END_TAG];
+const SONNET_PARAMETER_INVOKE_1: &str = "</parameter>\n</invoke>";
+const SONNET_PARAMETER_INVOKE_2: &str = "</parameter></invoke>";
+const END_TAGS: [&str; 5] = [
+    OLD_TEXT_END_TAG,
+    NEW_TEXT_END_TAG,
+    EDITS_END_TAG,
+    SONNET_PARAMETER_INVOKE_1, // Remove this after switching to streaming tool call
+    SONNET_PARAMETER_INVOKE_2,
+];
 
 #[derive(Debug)]
 pub enum EditParserEvent {
@@ -547,6 +555,37 @@ mod tests {
         );
     }
 
+    #[gpui::test(iterations = 1000)]
+    fn test_xml_edits_with_closing_parameter_invoke(mut rng: StdRng) {
+        // This case is a regression with Claude Sonnet 4.5.
+        // Sometimes Sonnet thinks that it's doing a tool call
+        // and closes its response with '</parameter></invoke>'
+        // instead of properly closing </new_text>
+
+        let mut parser = EditParser::new(EditFormat::XmlTags);
+        assert_eq!(
+            parse_random_chunks(
+                indoc! {"
+                    <old_text>some text</old_text><new_text>updated text</parameter></invoke>
+                "},
+                &mut parser,
+                &mut rng
+            ),
+            vec![Edit {
+                old_text: "some text".to_string(),
+                new_text: "updated text".to_string(),
+                line_hint: None,
+            },]
+        );
+        assert_eq!(
+            parser.finish(),
+            EditParserMetrics {
+                tags: 2,
+                mismatched_tags: 1
+            }
+        );
+    }
+
     #[gpui::test(iterations = 1000)]
     fn test_xml_nested_tags(mut rng: StdRng) {
         let mut parser = EditParser::new(EditFormat::XmlTags);
@@ -1035,6 +1074,11 @@ mod tests {
             last_ix = chunk_ix;
         }
 
+        if new_text.is_some() {
+            pending_edit.new_text = new_text.take().unwrap();
+            edits.push(pending_edit);
+        }
+
         edits
     }
 }