Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 55 additions & 3 deletions src/llm-coding-tools-core/src/tools/glob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ pub struct GlobOutput {
/// Whether results were truncated due to limit.
#[serde(skip_serializing_if = "std::ops::Not::not")]
pub truncated: bool,
/// Whether one or more paths could not be traversed or processed.
#[serde(skip_serializing_if = "std::ops::Not::not")]
pub partial: bool,
/// Per-path traversal errors encountered while collecting matches.
#[serde(skip_serializing_if = "Vec::is_empty")]
pub errors: Vec<String>,
}

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

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

let walker = WalkBuilder::new(&path)
.hidden(false)
Expand All @@ -50,7 +57,10 @@ pub fn glob_files<R: PathResolver>(
for entry_result in walker {
let entry = match entry_result {
Ok(e) => e,
Err(_) => continue,
Err(err) => {
errors.push(format!("walk error: {err}"));
continue;
}
Comment thread
Sewer56 marked this conversation as resolved.
};

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

let rel_path = match entry.path().strip_prefix(&path) {
Ok(p) => p.to_string_lossy().into_owned(),
Err(_) => continue,
Err(err) => {
errors.push(format!(
"failed to make '{}' relative to '{}': {err}",
entry.path().display(),
path.display()
));
continue;
}
};

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

Ok(GlobOutput { files, truncated })
Ok(GlobOutput {
files,
truncated,
partial: !errors.is_empty(),
errors,
})
}

#[cfg(test)]
Expand Down Expand Up @@ -196,4 +218,34 @@ mod tests {
}
assert!(result.files.iter().any(|f| f.contains('/')));
}

#[test]
fn glob_output_serializes_partial_metadata() {
let output = GlobOutput {
files: vec!["src/lib.rs".to_string()],
truncated: false,
partial: true,
errors: vec!["walk error: denied".to_string()],
};

let json = serde_json::to_value(&output).unwrap();

assert_eq!(json["partial"], true);
assert_eq!(json["errors"][0], "walk error: denied");
}

#[test]
fn glob_output_omits_partial_metadata_when_not_partial() {
let output = GlobOutput {
files: vec!["src/lib.rs".to_string()],
truncated: false,
partial: false,
errors: Vec::new(),
};

let json = serde_json::to_value(&output).unwrap();

assert!(json.get("partial").is_none());
assert!(json.get("errors").is_none());
}
}
114 changes: 96 additions & 18 deletions src/llm-coding-tools-core/src/tools/grep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ pub struct GrepOutput {
pub match_count: usize,
/// Whether results were truncated due to limit.
pub truncated: bool,
/// Whether one or more files could not be searched.
#[serde(skip_serializing_if = "std::ops::Not::not")]
pub partial: bool,
/// Per-file traversal/search errors encountered while collecting matches.
#[serde(skip_serializing_if = "Vec::is_empty")]
pub errors: Vec<String>,
}

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

if self.partial {
let _ = write!(
&mut output,
"\n(Partial results: {} file error(s) encountered)",
self.errors.len()
);
}

output
}
}
Expand Down Expand Up @@ -116,6 +130,7 @@ pub fn grep_search<R: PathResolver>(
.build();

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

let walker = WalkBuilder::new(&path)
.hidden(false)
Expand All @@ -127,7 +142,10 @@ pub fn grep_search<R: PathResolver>(
for entry_result in walker {
let entry = match entry_result {
Ok(e) => e,
Err(_) => continue,
Err(err) => {
errors.push(format!("walk error: {err}"));
continue;
}
Comment thread
Sewer56 marked this conversation as resolved.
};

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

let matches = collect_file_matches(&matcher, &mut searcher, entry_path);
if matches.is_empty() {
let search_result = collect_file_matches(&matcher, &mut searcher, entry_path);

if let Some(error) = search_result.error {
errors.push(error);
}

if search_result.matches.is_empty() {
continue;
}

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

files.push(GrepFileMatches {
path: entry_path.to_string_lossy().into_owned(),
matches,
matches: search_result.matches,
mtime,
});
}
Expand Down Expand Up @@ -193,30 +216,40 @@ pub fn grep_search<R: PathResolver>(
files,
match_count,
truncated,
partial: !errors.is_empty(),
errors,
})
}

struct FileSearchResult {
matches: Vec<GrepLineMatch>,
error: Option<String>,
}

#[inline]
fn collect_file_matches(
matcher: &RegexMatcher,
searcher: &mut Searcher,
path: &Path,
) -> Vec<GrepLineMatch> {
) -> FileSearchResult {
let mut matches = Vec::new();

let _ = searcher.search_path(
matcher,
path,
UTF8(|line_num, line| {
matches.push(GrepLineMatch {
line_num,
line_text: line.trim_end().to_string(),
});
Ok(true)
}),
);

matches
let error = searcher
.search_path(
matcher,
path,
UTF8(|line_num, line| {
matches.push(GrepLineMatch {
line_num,
line_text: line.trim_end().to_string(),
});
Ok(true)
}),
)
.err()
.map(|err| format!("failed to search '{}': {err}", path.display()));

FileSearchResult { matches, error }
}

#[cfg(test)]
Expand Down Expand Up @@ -257,4 +290,49 @@ mod tests {
assert_eq!(result.files.len(), 1);
assert!(result.files[0].path.ends_with(".rs"));
}

#[test]
fn grep_format_includes_partial_marker() {
let output = GrepOutput {
files: Vec::new(),
match_count: 0,
truncated: false,
partial: true,
errors: vec!["walk error: denied".to_string()],
};

let formatted = output.format::<true>(10, DEFAULT_MAX_LINE_LENGTH);

assert!(formatted.contains("Partial results"));
}

#[test]
fn collect_file_matches_reports_error_for_missing_file() {
let temp = tempdir().unwrap();
let missing = temp.path().join("missing.txt");
let matcher = RegexMatcher::new("hello").unwrap();
let mut searcher = SearcherBuilder::new()
.binary_detection(BinaryDetection::quit(0))
.build();

let result = collect_file_matches(&matcher, &mut searcher, &missing);

assert!(result.matches.is_empty());
assert!(result.error.is_some());
}

#[test]
fn grep_marks_results_partial_when_walker_reports_error() {
let temp = tempdir().unwrap();
let missing_root = temp.path().join("missing-root");
let resolver = AbsolutePathResolver;

let result =
grep_search(&resolver, "hello", None, missing_root.to_str().unwrap(), 10).unwrap();

assert!(result.partial);
assert_eq!(result.match_count, 0);
assert!(!result.truncated);
assert!(!result.errors.is_empty());
}
}
38 changes: 21 additions & 17 deletions src/llm-coding-tools-serdesai/src/absolute/glob.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
//! Glob pattern file finding tool using [`AbsolutePathResolver`].

use async_trait::async_trait;
use llm_coding_tools_core::ToolContext;
use llm_coding_tools_core::path::AbsolutePathResolver;
use llm_coding_tools_core::tool_names;
use llm_coding_tools_core::tools::glob_files;
use llm_coding_tools_core::{ToolContext, ToolOutput};
use serde::Deserialize;
use serdes_ai::tools::{RunContext, SchemaBuilder, Tool, ToolDefinition, ToolError, ToolResult};

use crate::common::glob::output_to_return as glob_output_to_return;
use crate::convert::to_serdes_result;

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

// Convert GlobOutput to ToolOutput for consistent error handling
to_serdes_result(
tool_names::GLOB,
result.map(|output| {
let content = if output.files.is_empty() {
"No files found matching the pattern.".to_string()
} else {
output.files.join("\n")
};
if output.truncated {
ToolOutput::truncated(content)
} else {
ToolOutput::new(content)
}
}),
)
match result {
Err(e) => to_serdes_result(tool_names::GLOB, Err(e)),
Ok(output) => Ok(glob_output_to_return(output)),
}
}
}

Expand All @@ -90,6 +79,7 @@ impl ToolContext for GlobTool {
#[cfg(test)]
mod tests {
use super::*;
use llm_coding_tools_core::tools::GlobOutput;
use serde_json::json;
use serdes_ai::tools::RunContext;
use std::fs::{self, File};
Expand Down Expand Up @@ -122,4 +112,18 @@ mod tests {
assert!(text.contains("lib.rs"));
assert!(text.contains("main.rs"));
}

#[test]
fn partial_glob_output_returns_json_payload() {
let payload = glob_output_to_return(GlobOutput {
files: vec!["src/lib.rs".to_string()],
truncated: false,
partial: true,
errors: vec!["walk error: denied".to_string()],
});

let json = payload.as_json().unwrap();
assert_eq!(json["partial"], true);
assert_eq!(json["errors"][0], "walk error: denied");
}
}
Loading