agent: Improve initial file search quality (#29317)

This PR significantly improves the quality of the initial file search
that occurs when the model doesn't yet know the full path to a file it
needs to read/edit.

Previously, the assertions in file_search often failed on main as the
model attempted to guess full file paths. On this branch, it reliably
calls `find_path` (previously `path_search`) before reading files.

After getting the model to find paths first, I noticed it would try
using `grep` instead of `path_search`. This motivated renaming
`path_search` to `find_path` (continuing the analogy to unix commands)
and adding system prompt instructions about proper tool selection.

Note: I know the command is just called `find`, but that seemed too
general.

In my eval runs, the `file_search` example improved from 40% ± 10% to
98% ± 2%. The only assertion I'm seeing occasionally fail is "glob
starts with `**` or project". We can probably add some instructions in
that regard.

Release Notes:

- N/A
This commit is contained in:
Agus Zubiaga 2025-04-23 21:24:41 -03:00 committed by GitHub
parent 2124b7ea99
commit 8b5835de17
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 107 additions and 28 deletions

View file

@ -51,3 +51,9 @@ pub(crate) mod m_2025_04_21 {
pub(crate) use settings::SETTINGS_PATTERNS;
}
pub(crate) mod m_2025_04_23 {
mod settings;
pub(crate) use settings::SETTINGS_PATTERNS;
}

View file

@ -0,0 +1,27 @@
use std::ops::Range;
use tree_sitter::{Query, QueryMatch};
use crate::MigrationPatterns;
use crate::patterns::SETTINGS_ASSISTANT_TOOLS_PATTERN;
pub const SETTINGS_PATTERNS: MigrationPatterns =
&[(SETTINGS_ASSISTANT_TOOLS_PATTERN, rename_path_search_tool)];
fn rename_path_search_tool(
contents: &str,
mat: &QueryMatch,
query: &Query,
) -> Option<(Range<usize>, String)> {
let tool_name_capture_ix = query.capture_index_for_name("tool_name")?;
let tool_name_range = mat
.nodes_for_capture_index(tool_name_capture_ix)
.next()?
.byte_range();
let tool_name = contents.get(tool_name_range.clone())?;
if tool_name == "path_search" {
return Some((tool_name_range, "find_path".to_string()));
}
None
}

View file

@ -132,6 +132,10 @@ pub fn migrate_settings(text: &str) -> Result<Option<String>> {
migrations::m_2025_04_21::SETTINGS_PATTERNS,
&SETTINGS_QUERY_2025_04_21,
),
(
migrations::m_2025_04_23::SETTINGS_PATTERNS,
&SETTINGS_QUERY_2025_04_23,
),
];
run_migrations(text, migrations)
}
@ -214,6 +218,10 @@ define_query!(
SETTINGS_QUERY_2025_04_21,
migrations::m_2025_04_21::SETTINGS_PATTERNS
);
define_query!(
SETTINGS_QUERY_2025_04_23,
migrations::m_2025_04_23::SETTINGS_PATTERNS
);
// custom query
static EDIT_PREDICTION_SETTINGS_MIGRATION_QUERY: LazyLock<Query> = LazyLock::new(|| {
@ -639,7 +647,7 @@ mod tests {
"name": "Custom",
"tools": {
"diagnostics": true,
"path_search": true,
"find_path": true,
"read_file": true
}
}
@ -650,4 +658,40 @@ mod tests {
None,
)
}
#[test]
fn test_rename_path_search_to_find_path() {
assert_migrate_settings(
r#"
{
"assistant": {
"profiles": {
"default": {
"tools": {
"path_search": true,
"read_file": true
}
}
}
}
}
"#,
Some(
r#"
{
"assistant": {
"profiles": {
"default": {
"tools": {
"find_path": true,
"read_file": true
}
}
}
}
}
"#,
),
);
}
}