Repository navigation
fix(core): tell the model what file tools skipped and accept numbers written as strings - #352
Merged
Merged
Conversation
…written as strings grep passed over files above 1.5 MB and files that are not UTF-8 without a word, so a search whose answer sat in one of them read as "no matches". read_file refused a file above 1.5 MB before it looked at offset and limit, while the refusal told the model to retry with offset and limit, and it reported a directory or a call with no path as a file that is not UTF-8. glob and grep read .gitignore only inside a git repository, so in a project without one they walked node_modules and build output. offset, limit, timeout_secs and replace_all were read as JSON numbers and booleans only, so "5" or "true" was dropped and the default ran. grep now counts the files it skipped by reason in one closing line. read_file streams the requested window of a large file line by line, holding only the window and a bounded prefix of each line, and renders it exactly as a file read whole; a directory, and a missing path in any file tool, get errors that name them. glob and grep share one walker that applies .gitignore outside a repository and keeps the default inside one, so a .gitignore above the repository root still cannot hide the project. One helper reads those four arguments from their JSON type or from a string. The tool schemas are unchanged.
Outside a repository nothing marks where git's rules stop, so glob and grep read every .gitignore up to the filesystem root. git applies none of them there, and a parent directory's .gitignore that ignores "*" made the whole project look empty. Outside a repository the walker now reads no ignore file above the walk root, and a filter applies the project's own .gitignore files from its root down plus the .ignore files it always read, so a glob or grep scoped below the root keeps the root's rules. read_file's refusal of a file over 1.5 MB still pointed at grep, which skips files that large; it now names only a range read with offset and limit. A blank path in read_file, write_file or edit_file resolved to the project root and produced an error that named nothing; it is now reported as a missing path.
Outside a repository the walker reads .gitignore regardless, and that also turned on the user's global excludes, which git applies only inside a repository. A project without git lost every file they name, such as *.log or .env, from glob and grep without a word. Global excludes now apply only inside a repository. In a project without git, a glob or grep scoped below the root pruned any entry a .gitignore above the walk root ignored, even when a nearer .gitignore inside the walk whitelisted it, so the scoped search answered "no matches" for a file the whole-project search found. When a rule from above the walk root would prune an entry, the filter now ranks it with the ignore files from the entry's directory up, nearest first and .ignore ahead of .gitignore, as the walker ranks the files it reads.
In a project without git the walker read the .gitignore files inside the walk itself, so it pruned an entry they ignored before the filter could rank it against a .ignore above the walk root that whitelists it. A glob or grep scoped below the root then missed a file the whole-project search found, and a .ignore above the project root lost to the project's own .gitignore. The filter also parsed the ignore files of every directory above an entry once for each entry it was about to prune, so a scoped search over many such entries reread the same files thousands of times. Outside a repository the walker now reads only .ignore files, from every ancestor, and the filter applies the project's .gitignore files from its root down, ranked below every .ignore. Each directory's rules are read once per walk and shared by every entry below it.
A range read of a file over 1.5 MB streams the file to its end to count its lines. Cancelling the call stopped the caller from waiting, but the read ran on in the background through the rest of the file, however large, spending disk and CPU on a result nothing would use. The streamed read now checks the call's cancel token before each buffer fill and stops once the call is cancelled.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The file tools hid what they left out, so the model drew the wrong conclusions:
grepsilently skipped files over 1.5 MB, files that are not UTF-8 text, and files it could not read. A "no matches" result could therefore mean "never searched", and the model would decide that code did not exist.read_filerefused every file over 1.5 MB, even when the model asked for a small range of lines. The refusal told it to use grep, but grep also skips those files, so the advice led nowhere.read_fileon a directory, or a failed read for any other reason, reported "not a UTF-8 text file". A call with no path failed with a confusing error about an empty path.globandgrepignored the project's.gitignore, sonode_modules/and build output flooded results. Turning that on naively would let a.gitignoreabove the project (for example a home directory kept in git that ignores*), or the user's global git excludes, hide the whole project."offset": "200","replace_all": "true"). Those values were silently dropped and the defaults used instead, so the model read the wrong lines or edited only the first match.Summary
grepcounts the files it skipped and adds one line at the end, for example… not searched: 1 file over 1.5 MB, 2 files not UTF-8 text. Unreadable files are counted too. The size check measures what the read actually gets, which is a symlink's target.read_filewithoffset/limiton a file over 1.5 MB reads the file one line at a time, keeps only the requested lines (and the first 500 bytes of each), and formats them through the same code as small files, so the output is identical. The read stops as soon as the call is cancelled instead of running on to the end of the file. A whole-file read of a large file is still refused, now with advice that works: "read a range of lines with offset and limit".read_fileon a directory says<path> is a directory; use list_dir to see its entries. A missing or blank path toread_file,write_fileoredit_filesaysmissing required argument: path. Other read errors report the real OS error.globandgrepshare one walker builder:.gitignorefiles apply from the project root down. A.gitignoreabove the project and the user's global git excludes do not apply, as git would not apply them..ignorefiles keep applying from every ancestor..ignore(from any ancestor) outranks every.gitignore. A search scoped to a subdirectory therefore finds the same files as a whole-project search, including files a.ignoreabove the walk or above the project whitelists..gitis still excluded by name.offset,limit,timeout_secsandreplace_allaccept either their JSON type or a string. Strings are trimmed and matched without regard to case, and unusable values still fall back to the default.Test Plan
New tests in
crates/core/src/tools.rs. Each regression test was seen failing before the fix; the line reader test pins the new helper:grep_counts_the_files_it_could_not_search: the exact skipped-files line for oversized and non-UTF-8 files, with and without matches, and no line for a scoped search that skipped nothing.read_file_reads_a_window_of_a_file_too_large_to_load_whole: a range from a large file is byte-identical to the same range from a small file, and the whole-file refusal text is exact.a_cancelled_read_of_a_large_file_stops_reading: a range read of a large file returns as cancelled once its call is cancelled, instead of reading the file to its end.streamed_lines_split_like_str_lines: the streaming line reader splits lines exactly asstr::linesdoes, across buffer boundaries and with CRLF endings.file_tools_name_a_directory_and_a_missing_path: the directory message and the missing or blank path message for all three file tools.numeric_and_boolean_arguments_written_as_strings_are_honored: string forms ofoffset,limit,timeout_secsandreplace_alltake effect.grep_and_glob_honor_gitignore_without_a_git_repository: ignored directories are left out of whole and scoped walks, a path named directly is still searched, and a nested repository is not hidden by an outer.gitignore.a_gitignore_above_a_project_without_git_cannot_hide_it: a.gitignoreof*above the project hides nothing, while an ancestor.ignorestill applies.a_scoped_walk_ranks_ignore_files_as_a_whole_project_walk_does: a nearer whitelist beats a farther ignore rule in a scoped walk.an_ignore_file_above_the_walk_outranks_a_gitignore_inside_it: a.ignorewhitelist at the project root, and one above the project, keep a file a.gitignoreignores, in whole and scoped glob and grep.global_git_excludes_apply_only_inside_a_repository: global excludes hide files inside a repository and not outside one.Results:
cargo test --workspace --locked: passes (1089 passed, 0 failed), includingfrozen_prompt_fits_token_budget. The tools tests also pass with a global excludes file containing*.log,.env,.idea/andnode_modules/.cargo +1.97.0 clippy --workspace --all-targets --locked -- -D warnings: passes.No outstanding findings block merging.
Summary
The PR clarifies file-tool results, supports ranged reads of large files, accepts string-form tool arguments, and applies project .gitignore rules outside repositories. The current code addresses the three previously reported search and cancellation issues.
Reviews (2) · Last reviewed commit: "fix(core): stop a cancelled read of a la..."