Skip to content

Commit eaac762

Browse files
edit tool: Support stringified mode parameter (#55498) (cherry-pick to stable) (#55500)
Cherry-pick of #55498 to stable ---- Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed an issue where agent failed to apply edits in some cases Co-authored-by: Agus Zubiaga <agus@zed.dev>
1 parent 5ec84a9 commit eaac762

1 file changed

Lines changed: 83 additions & 23 deletions

File tree

crates/agent/src/tools/streaming_edit_file_tool.rs

Lines changed: 83 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ pub struct StreamingEditFileToolInput {
8484
/// - 'edit': Make granular edits to an existing file. Requires 'edits' field.
8585
///
8686
/// When a file already exists or you just created it, prefer editing it as opposed to recreating it from scratch.
87+
#[serde(deserialize_with = "deserialize_maybe_stringified")]
8788
pub mode: StreamingEditFileMode,
8889

8990
/// The complete content for the new file (required for 'write' mode).
@@ -96,7 +97,7 @@ pub struct StreamingEditFileToolInput {
9697
#[serde(
9798
default,
9899
skip_serializing_if = "Option::is_none",
99-
deserialize_with = "deserialize_optional_vec_or_json_string"
100+
deserialize_with = "deserialize_maybe_stringified"
100101
)]
101102
pub edits: Option<Vec<Edit>>,
102103
}
@@ -133,11 +134,11 @@ struct StreamingEditFileToolPartialInput {
133134
display_description: Option<String>,
134135
#[serde(default)]
135136
path: Option<String>,
136-
#[serde(default)]
137+
#[serde(default, deserialize_with = "deserialize_maybe_stringified")]
137138
mode: Option<StreamingEditFileMode>,
138139
#[serde(default)]
139140
content: Option<String>,
140-
#[serde(default, deserialize_with = "deserialize_optional_vec_or_json_string")]
141+
#[serde(default, deserialize_with = "deserialize_maybe_stringified")]
141142
edits: Option<Vec<PartialEdit>>,
142143
}
143144

@@ -149,30 +150,23 @@ pub struct PartialEdit {
149150
pub new_text: Option<String>,
150151
}
151152

152-
/// Sometimes the model responds with a stringified JSON array of edits (`"[...]"`) instead of a regular array (`[...]`)
153-
fn deserialize_optional_vec_or_json_string<'de, T, D>(
154-
deserializer: D,
155-
) -> Result<Option<Vec<T>>, D::Error>
153+
#[derive(Deserialize)]
154+
#[serde(untagged)]
155+
enum ValueOrJsonString<T> {
156+
Value(T),
157+
String(String),
158+
}
159+
160+
fn deserialize_maybe_stringified<'de, T, D>(deserializer: D) -> Result<T, D::Error>
156161
where
157162
T: DeserializeOwned,
158163
D: Deserializer<'de>,
159164
{
160-
#[derive(Deserialize)]
161-
#[serde(untagged)]
162-
enum VecOrJsonString<T> {
163-
Vec(Vec<T>),
164-
String(String),
165-
}
166-
167-
let value = Option::<VecOrJsonString<T>>::deserialize(deserializer)?;
168-
match value {
169-
None => Ok(None),
170-
Some(VecOrJsonString::Vec(items)) => Ok(Some(items)),
171-
Some(VecOrJsonString::String(string)) => serde_json::from_str::<Vec<T>>(&string)
172-
.map(Some)
173-
.map_err(|error| {
174-
D::Error::custom(format!("failed to parse stringified edits array: {error}"))
175-
}),
165+
match ValueOrJsonString::<T>::deserialize(deserializer)? {
166+
ValueOrJsonString::Value(value) => Ok(value),
167+
ValueOrJsonString::String(string) => serde_json::from_str::<T>(&string).map_err(|error| {
168+
D::Error::custom(format!("failed to parse stringified value: {error}"))
169+
}),
176170
}
177171
}
178172

@@ -4180,6 +4174,72 @@ mod tests {
41804174
);
41814175
}
41824176

4177+
#[test]
4178+
fn test_input_deserializes_double_encoded_fields() {
4179+
let input = serde_json::from_value::<StreamingEditFileToolInput>(json!({
4180+
"display_description": "Edit",
4181+
"path": "root/file.txt",
4182+
"mode": "\"edit\"",
4183+
"edits": "[{\"old_text\": \"hello\\nworld\", \"new_text\": \"HELLO\\nWORLD\"}]"
4184+
}))
4185+
.expect("input should deserialize");
4186+
4187+
assert!(matches!(input.mode, StreamingEditFileMode::Edit));
4188+
let edits = input.edits.expect("edits should deserialize");
4189+
assert_eq!(edits.len(), 1);
4190+
assert_eq!(edits[0].old_text, "hello\nworld");
4191+
assert_eq!(edits[0].new_text, "HELLO\nWORLD");
4192+
4193+
let input = serde_json::from_value::<StreamingEditFileToolInput>(json!({
4194+
"display_description": "Edit",
4195+
"path": "root/file.txt",
4196+
"mode": "\"edit\""
4197+
}))
4198+
.expect("input should deserialize");
4199+
assert!(input.edits.is_none());
4200+
4201+
let input = serde_json::from_value::<StreamingEditFileToolInput>(json!({
4202+
"display_description": "Edit",
4203+
"path": "root/file.txt",
4204+
"mode": "\"edit\"",
4205+
"edits": null
4206+
}))
4207+
.expect("input should deserialize");
4208+
assert!(input.edits.is_none());
4209+
4210+
let input = serde_json::from_value::<StreamingEditFileToolPartialInput>(json!({
4211+
"display_description": "Edit",
4212+
"path": "root/file.txt",
4213+
"mode": "\"edit\"",
4214+
"edits": "[{\"old_text\": \"hello\\nworld\", \"new_text\": \"HELLO\\nWORLD\"}]"
4215+
}))
4216+
.expect("input should deserialize");
4217+
4218+
assert!(matches!(input.mode, Some(StreamingEditFileMode::Edit)));
4219+
let edits = input.edits.expect("edits should deserialize");
4220+
assert_eq!(edits.len(), 1);
4221+
assert_eq!(edits[0].old_text.as_deref(), Some("hello\nworld"));
4222+
assert_eq!(edits[0].new_text.as_deref(), Some("HELLO\nWORLD"));
4223+
4224+
let input = serde_json::from_value::<StreamingEditFileToolPartialInput>(json!({
4225+
"display_description": "Edit",
4226+
"path": "root/file.txt"
4227+
}))
4228+
.expect("input should deserialize");
4229+
assert!(input.mode.is_none());
4230+
assert!(input.edits.is_none());
4231+
4232+
let input = serde_json::from_value::<StreamingEditFileToolPartialInput>(json!({
4233+
"display_description": "Edit",
4234+
"path": "root/file.txt",
4235+
"mode": null,
4236+
"edits": null
4237+
}))
4238+
.expect("input should deserialize");
4239+
assert!(input.mode.is_none());
4240+
assert!(input.edits.is_none());
4241+
}
4242+
41834243
async fn setup_test_with_fs(
41844244
cx: &mut TestAppContext,
41854245
fs: Arc<project::FakeFs>,

0 commit comments

Comments
 (0)