Skip to content

Commit 0940135

Browse files
committed
Fixed: Surface grep/glob partial errors end-to-end
Capture per-file traversal and search failures in core grep/glob as structured partial metadata (partial, errors) instead of silently dropping them. Propagate partial state through serdesAI absolute/allowed wrappers using explicit JSON payloads while preserving existing non-partial text behavior. Deduplicate wrapper conversion logic by extracting shared helpers in common::glob and common::grep so absolute and allowed variants stay consistent.
1 parent 5aa069b commit 0940135

9 files changed

Lines changed: 334 additions & 76 deletions

File tree

src/llm-coding-tools-core/src/tools/glob.rs

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@ pub struct GlobOutput {
1717
/// Whether results were truncated due to limit.
1818
#[serde(skip_serializing_if = "std::ops::Not::not")]
1919
pub truncated: bool,
20+
/// Whether one or more paths could not be traversed or processed.
21+
#[serde(skip_serializing_if = "std::ops::Not::not")]
22+
pub partial: bool,
23+
/// Per-path traversal errors encountered while collecting matches.
24+
#[serde(skip_serializing_if = "Vec::is_empty")]
25+
pub errors: Vec<String>,
2026
}
2127

2228
/// Finds files matching a glob pattern in the given directory.
@@ -39,6 +45,7 @@ pub fn glob_files<R: PathResolver>(
3945
let matcher = Glob::new(pattern)?.compile_matcher();
4046

4147
let mut files_with_mtime: Vec<(String, SystemTime)> = Vec::new();
48+
let mut errors: Vec<String> = Vec::with_capacity(8);
4249

4350
let walker = WalkBuilder::new(&path)
4451
.hidden(false)
@@ -50,7 +57,10 @@ pub fn glob_files<R: PathResolver>(
5057
for entry_result in walker {
5158
let entry = match entry_result {
5259
Ok(e) => e,
53-
Err(_) => continue,
60+
Err(err) => {
61+
errors.push(format!("walk error: {err}"));
62+
continue;
63+
}
5464
};
5565

5666
if let Some(ft) = entry.file_type() {
@@ -63,7 +73,14 @@ pub fn glob_files<R: PathResolver>(
6373

6474
let rel_path = match entry.path().strip_prefix(&path) {
6575
Ok(p) => p.to_string_lossy().into_owned(),
66-
Err(_) => continue,
76+
Err(err) => {
77+
errors.push(format!(
78+
"failed to make '{}' relative to '{}': {err}",
79+
entry.path().display(),
80+
path.display()
81+
));
82+
continue;
83+
}
6784
};
6885

6986
// Normalize Windows backslashes to forward slashes for glob pattern matching
@@ -97,7 +114,12 @@ pub fn glob_files<R: PathResolver>(
97114
.map(|(path, _)| path)
98115
.collect();
99116

100-
Ok(GlobOutput { files, truncated })
117+
Ok(GlobOutput {
118+
files,
119+
truncated,
120+
partial: !errors.is_empty(),
121+
errors,
122+
})
101123
}
102124

103125
#[cfg(test)]
@@ -196,4 +218,34 @@ mod tests {
196218
}
197219
assert!(result.files.iter().any(|f| f.contains('/')));
198220
}
221+
222+
#[test]
223+
fn glob_output_serializes_partial_metadata() {
224+
let output = GlobOutput {
225+
files: vec!["src/lib.rs".to_string()],
226+
truncated: false,
227+
partial: true,
228+
errors: vec!["walk error: denied".to_string()],
229+
};
230+
231+
let json = serde_json::to_value(&output).unwrap();
232+
233+
assert_eq!(json["partial"], true);
234+
assert_eq!(json["errors"][0], "walk error: denied");
235+
}
236+
237+
#[test]
238+
fn glob_output_omits_partial_metadata_when_not_partial() {
239+
let output = GlobOutput {
240+
files: vec!["src/lib.rs".to_string()],
241+
truncated: false,
242+
partial: false,
243+
errors: Vec::new(),
244+
};
245+
246+
let json = serde_json::to_value(&output).unwrap();
247+
248+
assert!(json.get("partial").is_none());
249+
assert!(json.get("errors").is_none());
250+
}
199251
}

src/llm-coding-tools-core/src/tools/grep.rs

Lines changed: 96 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,12 @@ pub struct GrepOutput {
4747
pub match_count: usize,
4848
/// Whether results were truncated due to limit.
4949
pub truncated: bool,
50+
/// Whether one or more files could not be searched.
51+
#[serde(skip_serializing_if = "std::ops::Not::not")]
52+
pub partial: bool,
53+
/// Per-file traversal/search errors encountered while collecting matches.
54+
#[serde(skip_serializing_if = "Vec::is_empty")]
55+
pub errors: Vec<String>,
5056
}
5157

5258
impl GrepOutput {
@@ -86,6 +92,14 @@ impl GrepOutput {
8692
let _ = write!(&mut output, "\n(Results truncated at {} matches)", limit);
8793
}
8894

95+
if self.partial {
96+
let _ = write!(
97+
&mut output,
98+
"\n(Partial results: {} file error(s) encountered)",
99+
self.errors.len()
100+
);
101+
}
102+
89103
output
90104
}
91105
}
@@ -116,6 +130,7 @@ pub fn grep_search<R: PathResolver>(
116130
.build();
117131

118132
let mut files: Vec<GrepFileMatches> = Vec::with_capacity(64);
133+
let mut errors: Vec<String> = Vec::with_capacity(8);
119134

120135
let walker = WalkBuilder::new(&path)
121136
.hidden(false)
@@ -127,7 +142,10 @@ pub fn grep_search<R: PathResolver>(
127142
for entry_result in walker {
128143
let entry = match entry_result {
129144
Ok(e) => e,
130-
Err(_) => continue,
145+
Err(err) => {
146+
errors.push(format!("walk error: {err}"));
147+
continue;
148+
}
131149
};
132150

133151
// Skip directories and non-regular files.
@@ -149,8 +167,13 @@ pub fn grep_search<R: PathResolver>(
149167
}
150168
}
151169

152-
let matches = collect_file_matches(&matcher, &mut searcher, entry_path);
153-
if matches.is_empty() {
170+
let search_result = collect_file_matches(&matcher, &mut searcher, entry_path);
171+
172+
if let Some(error) = search_result.error {
173+
errors.push(error);
174+
}
175+
176+
if search_result.matches.is_empty() {
154177
continue;
155178
}
156179

@@ -162,7 +185,7 @@ pub fn grep_search<R: PathResolver>(
162185

163186
files.push(GrepFileMatches {
164187
path: entry_path.to_string_lossy().into_owned(),
165-
matches,
188+
matches: search_result.matches,
166189
mtime,
167190
});
168191
}
@@ -193,30 +216,40 @@ pub fn grep_search<R: PathResolver>(
193216
files,
194217
match_count,
195218
truncated,
219+
partial: !errors.is_empty(),
220+
errors,
196221
})
197222
}
198223

224+
struct FileSearchResult {
225+
matches: Vec<GrepLineMatch>,
226+
error: Option<String>,
227+
}
228+
199229
#[inline]
200230
fn collect_file_matches(
201231
matcher: &RegexMatcher,
202232
searcher: &mut Searcher,
203233
path: &Path,
204-
) -> Vec<GrepLineMatch> {
234+
) -> FileSearchResult {
205235
let mut matches = Vec::new();
206236

207-
let _ = searcher.search_path(
208-
matcher,
209-
path,
210-
UTF8(|line_num, line| {
211-
matches.push(GrepLineMatch {
212-
line_num,
213-
line_text: line.trim_end().to_string(),
214-
});
215-
Ok(true)
216-
}),
217-
);
218-
219-
matches
237+
let error = searcher
238+
.search_path(
239+
matcher,
240+
path,
241+
UTF8(|line_num, line| {
242+
matches.push(GrepLineMatch {
243+
line_num,
244+
line_text: line.trim_end().to_string(),
245+
});
246+
Ok(true)
247+
}),
248+
)
249+
.err()
250+
.map(|err| format!("failed to search '{}': {err}", path.display()));
251+
252+
FileSearchResult { matches, error }
220253
}
221254

222255
#[cfg(test)]
@@ -257,4 +290,49 @@ mod tests {
257290
assert_eq!(result.files.len(), 1);
258291
assert!(result.files[0].path.ends_with(".rs"));
259292
}
293+
294+
#[test]
295+
fn grep_format_includes_partial_marker() {
296+
let output = GrepOutput {
297+
files: Vec::new(),
298+
match_count: 0,
299+
truncated: false,
300+
partial: true,
301+
errors: vec!["walk error: denied".to_string()],
302+
};
303+
304+
let formatted = output.format::<true>(10, DEFAULT_MAX_LINE_LENGTH);
305+
306+
assert!(formatted.contains("Partial results"));
307+
}
308+
309+
#[test]
310+
fn collect_file_matches_reports_error_for_missing_file() {
311+
let temp = tempdir().unwrap();
312+
let missing = temp.path().join("missing.txt");
313+
let matcher = RegexMatcher::new("hello").unwrap();
314+
let mut searcher = SearcherBuilder::new()
315+
.binary_detection(BinaryDetection::quit(0))
316+
.build();
317+
318+
let result = collect_file_matches(&matcher, &mut searcher, &missing);
319+
320+
assert!(result.matches.is_empty());
321+
assert!(result.error.is_some());
322+
}
323+
324+
#[test]
325+
fn grep_marks_results_partial_when_walker_reports_error() {
326+
let temp = tempdir().unwrap();
327+
let missing_root = temp.path().join("missing-root");
328+
let resolver = AbsolutePathResolver;
329+
330+
let result =
331+
grep_search(&resolver, "hello", None, missing_root.to_str().unwrap(), 10).unwrap();
332+
333+
assert!(result.partial);
334+
assert_eq!(result.match_count, 0);
335+
assert!(!result.truncated);
336+
assert!(!result.errors.is_empty());
337+
}
260338
}

src/llm-coding-tools-serdesai/src/absolute/glob.rs

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
//! Glob pattern file finding tool using [`AbsolutePathResolver`].
22
33
use async_trait::async_trait;
4+
use llm_coding_tools_core::ToolContext;
45
use llm_coding_tools_core::path::AbsolutePathResolver;
56
use llm_coding_tools_core::tool_names;
67
use llm_coding_tools_core::tools::glob_files;
7-
use llm_coding_tools_core::{ToolContext, ToolOutput};
88
use serde::Deserialize;
99
use serdes_ai::tools::{RunContext, SchemaBuilder, Tool, ToolDefinition, ToolError, ToolResult};
1010

11+
use crate::common::glob::output_to_return as glob_output_to_return;
1112
use crate::convert::to_serdes_result;
1213

1314
/// Internal args for JSON deserialization.
@@ -60,22 +61,10 @@ impl<Deps: Send + Sync> Tool<Deps> for GlobTool {
6061
let resolver = AbsolutePathResolver;
6162
let result = glob_files(&resolver, &args.pattern, &args.path);
6263

63-
// Convert GlobOutput to ToolOutput for consistent error handling
64-
to_serdes_result(
65-
tool_names::GLOB,
66-
result.map(|output| {
67-
let content = if output.files.is_empty() {
68-
"No files found matching the pattern.".to_string()
69-
} else {
70-
output.files.join("\n")
71-
};
72-
if output.truncated {
73-
ToolOutput::truncated(content)
74-
} else {
75-
ToolOutput::new(content)
76-
}
77-
}),
78-
)
64+
match result {
65+
Err(e) => to_serdes_result(tool_names::GLOB, Err(e)),
66+
Ok(output) => Ok(glob_output_to_return(output)),
67+
}
7968
}
8069
}
8170

@@ -90,6 +79,7 @@ impl ToolContext for GlobTool {
9079
#[cfg(test)]
9180
mod tests {
9281
use super::*;
82+
use llm_coding_tools_core::tools::GlobOutput;
9383
use serde_json::json;
9484
use serdes_ai::tools::RunContext;
9585
use std::fs::{self, File};
@@ -122,4 +112,18 @@ mod tests {
122112
assert!(text.contains("lib.rs"));
123113
assert!(text.contains("main.rs"));
124114
}
115+
116+
#[test]
117+
fn partial_glob_output_returns_json_payload() {
118+
let payload = glob_output_to_return(GlobOutput {
119+
files: vec!["src/lib.rs".to_string()],
120+
truncated: false,
121+
partial: true,
122+
errors: vec!["walk error: denied".to_string()],
123+
});
124+
125+
let json = payload.as_json().unwrap();
126+
assert_eq!(json["partial"], true);
127+
assert_eq!(json["errors"][0], "walk error: denied");
128+
}
125129
}

0 commit comments

Comments
 (0)