Skip to content

Commit e77a7f3

Browse files
fix(fs_search): handle empty string file type from LLM tool calls (#2355)
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
1 parent 8308b1f commit e77a7f3

2 files changed

Lines changed: 159 additions & 15 deletions

File tree

crates/forge_json_repair/src/schema_coercion.rs

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,17 @@ fn coerce_value_with_schema_object(
4545
return coerce_value_with_schema(value, def_schema, root_schema);
4646
}
4747
}
48+
49+
// Coerce empty strings to null for nullable schemas.
50+
// LLMs often send "" for optional parameters instead of omitting them or
51+
// sending null. When the schema has "nullable": true (OpenAPI 3.0 style),
52+
// an empty string should be treated as null.
53+
if let Value::String(s) = &value
54+
&& s.is_empty()
55+
&& is_nullable(schema)
56+
{
57+
return Value::Null;
58+
}
4859
// Handle anyOf/oneOf schemas by trying each sub-schema
4960
if let Some(subschemas) = &schema.subschemas {
5061
if let Some(any_of) = &subschemas.any_of {
@@ -157,6 +168,17 @@ fn coerce_by_instance_type(
157168
value
158169
}
159170

171+
/// Checks if a schema is marked as nullable via the OpenAPI 3.0 "nullable"
172+
/// extension. This is set by schemars when `option_nullable = true` for
173+
/// `Option<T>` fields.
174+
fn is_nullable(schema: &SchemaObject) -> bool {
175+
schema
176+
.extensions
177+
.get("nullable")
178+
.and_then(|v| v.as_bool())
179+
.unwrap_or(false)
180+
}
181+
160182
fn type_matches(value: &Value, target_types: &[&InstanceType]) -> bool {
161183
target_types.iter().any(|t| match t {
162184
InstanceType::Null => value.is_null(),
@@ -1125,4 +1147,100 @@ mod tests {
11251147

11261148
assert_eq!(actual, expected);
11271149
}
1150+
1151+
#[test]
1152+
fn test_coerce_empty_string_to_null_for_nullable_field() {
1153+
// Simulates LLM sending "" for a nullable string field (e.g., file_type in
1154+
// fs_search). The schema uses "nullable: true" (OpenAPI 3.0 style).
1155+
#[derive(JsonSchema)]
1156+
#[allow(dead_code)]
1157+
struct NullableStringData {
1158+
required_field: String,
1159+
#[schemars(default)]
1160+
optional_field: Option<String>,
1161+
}
1162+
1163+
// Generate schema with option_nullable=true (matching project settings)
1164+
let r#gen = schemars::r#gen::SchemaSettings::default()
1165+
.with(|s| {
1166+
s.option_nullable = true;
1167+
s.option_add_null_type = false;
1168+
})
1169+
.into_generator();
1170+
let schema = r#gen.into_root_schema_for::<NullableStringData>();
1171+
1172+
let fixture = json!({
1173+
"required_field": "value",
1174+
"optional_field": ""
1175+
});
1176+
let actual = coerce_to_schema(fixture, &schema);
1177+
let expected = json!({
1178+
"required_field": "value",
1179+
"optional_field": null
1180+
});
1181+
assert_eq!(actual, expected);
1182+
}
1183+
1184+
#[test]
1185+
fn test_preserve_nonempty_string_for_nullable_field() {
1186+
// Non-empty strings should be preserved even for nullable fields
1187+
#[derive(JsonSchema)]
1188+
#[allow(dead_code)]
1189+
struct NullableStringData {
1190+
optional_field: Option<String>,
1191+
}
1192+
1193+
let r#gen = schemars::r#gen::SchemaSettings::default()
1194+
.with(|s| {
1195+
s.option_nullable = true;
1196+
s.option_add_null_type = false;
1197+
})
1198+
.into_generator();
1199+
let schema = r#gen.into_root_schema_for::<NullableStringData>();
1200+
1201+
let fixture = json!({"optional_field": "rust"});
1202+
let actual = coerce_to_schema(fixture, &schema);
1203+
let expected = json!({"optional_field": "rust"});
1204+
assert_eq!(actual, expected);
1205+
}
1206+
1207+
#[test]
1208+
fn test_preserve_empty_string_for_required_field() {
1209+
// Empty strings should NOT be converted to null for non-nullable fields
1210+
#[derive(JsonSchema)]
1211+
#[allow(dead_code)]
1212+
struct RequiredStringData {
1213+
name: String,
1214+
}
1215+
1216+
let schema = schema_for!(RequiredStringData);
1217+
1218+
let fixture = json!({"name": ""});
1219+
let actual = coerce_to_schema(fixture, &schema);
1220+
let expected = json!({"name": ""});
1221+
assert_eq!(actual, expected);
1222+
}
1223+
1224+
#[test]
1225+
fn test_coerce_empty_string_to_null_for_nullable_integer() {
1226+
// Empty string for a nullable integer should become null
1227+
#[derive(JsonSchema)]
1228+
#[allow(dead_code)]
1229+
struct NullableIntData {
1230+
count: Option<u32>,
1231+
}
1232+
1233+
let r#gen = schemars::r#gen::SchemaSettings::default()
1234+
.with(|s| {
1235+
s.option_nullable = true;
1236+
s.option_add_null_type = false;
1237+
})
1238+
.into_generator();
1239+
let schema = r#gen.into_root_schema_for::<NullableIntData>();
1240+
1241+
let fixture = json!({"count": ""});
1242+
let actual = coerce_to_schema(fixture, &schema);
1243+
let expected = json!({"count": null});
1244+
assert_eq!(actual, expected);
1245+
}
11281246
}

crates/forge_services/src/tool_services/fs_search.rs

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -101,21 +101,24 @@ impl<W: WalkerInfra + FileReaderInfra + FileInfoInfra> ForgeFsSearch<W> {
101101
params: &FSSearch,
102102
) -> anyhow::Result<Vec<PathBuf>> {
103103
// Build type matcher once if file_type is specified (for efficiency)
104-
let types_matcher = if let Some(file_type) = &params.file_type {
105-
use ignore::types::TypesBuilder;
106-
107-
let mut builder = TypesBuilder::new();
108-
builder.add_defaults();
109-
builder.select(file_type);
110-
111-
Some(
112-
builder
113-
.build()
114-
.with_context(|| format!("Failed to build type matcher for: {file_type}"))?,
115-
)
116-
} else {
117-
None
118-
};
104+
// Filter out empty strings that may arrive from LLM tool calls with nullable
105+
// parameters
106+
let types_matcher =
107+
if let Some(file_type) = params.file_type.as_deref().filter(|s| !s.is_empty()) {
108+
use ignore::types::TypesBuilder;
109+
110+
let mut builder = TypesBuilder::new();
111+
builder.add_defaults();
112+
builder.select(file_type);
113+
114+
Some(
115+
builder.build().with_context(|| {
116+
format!("Failed to build type matcher for: {file_type}")
117+
})?,
118+
)
119+
} else {
120+
None
121+
};
119122

120123
let paths = if self.infra.is_file(search_path).await? {
121124
vec![search_path.to_path_buf()]
@@ -682,6 +685,29 @@ mod test {
682685
);
683686
}
684687

688+
#[tokio::test]
689+
async fn test_empty_file_type_treated_as_none() {
690+
let fixture = create_test_directory().await.unwrap();
691+
let params = FSSearch {
692+
pattern: "test".to_string(),
693+
path: Some(fixture.path().to_string_lossy().to_string()),
694+
file_type: Some("".to_string()),
695+
output_mode: Some(OutputMode::FilesWithMatches),
696+
..Default::default()
697+
};
698+
699+
// Should not error - empty file_type should be treated as None
700+
let actual = ForgeFsSearch::new(Arc::new(MockInfra::default()))
701+
.search(params)
702+
.await
703+
.unwrap();
704+
705+
assert!(actual.is_some());
706+
let result = actual.unwrap();
707+
// Should match files across all types (not filtered)
708+
assert!(result.matches.len() >= 3);
709+
}
710+
685711
#[tokio::test]
686712
async fn test_glob_filter() {
687713
let fixture = create_test_directory().await.unwrap();

0 commit comments

Comments
 (0)