fix(tools): return zero-match probe paths - #80714
Conversation
monerostar
left a comment
There was a problem hiding this comment.
Ubuntu 26.04 on linux-5800x, rg 15.1.0. Live public search_tool entry on main vs this tip.
Synthetic tree with a.py/b.py (TOKEN_ALPHA), meta.py (lookup[key+1]), and .secret/conf.cfg (HIDDEN_ONLY_TOKEN):
main:
- case mismatch: warning counts only, no
case_insensitive_matchesfield - literal / hidden: same, counts in warning text only
this PR:
- case mismatch:
total_count=0pluscase_insensitive_matchesfora.pyandb.py(count 1 each) - literal:
literal_matches->meta.pycount 1 - hidden:
hidden_matches->.secret/conf.cfgcount 1
Credential filter check: search for a token only in .env on the PR tip keeps total_count=0, omits hidden_matches, and sets _omitted to the secret-bearing-files message. Good.
Focused tests:
pytest tests/tools/test_search_zero_match_and_multipath.py -q -o addopts=
-> 16 passed in 1.53s
Sibling note: #80525 puts probe paths into the warning string (smaller). This tip keeps structured fields plus the read-block omit path, which is stronger for tool JSON consumers. #79605 / #77157 are more about grep fallback / Windows rg paths. Prefer this shape for #80522 on Linux here.
Looks good.
a24988f to
51cffe4
Compare
|
Thanks @monerostar for the independent Linux validation and detailed review. #80525 has since been merged, and #80522 is closed as completed. That merged change addresses the reported user-visible failure by returning the paths found by the zero-match probes. To keep the review queue clear and avoid maintaining a competing implementation for a resolved issue, I am closing this PR as superseded. This PR also explored structured probe fields and routing them through the existing credential/read-block filter. I will not ask maintainers to evaluate that additional API shape after the reported issue has been resolved. If maintainers later want that separate contract, it can be proposed independently with a fresh issue and current-main evidence. Prepared with Codex assistance; evidence verified against the stated commits. |
What / Why
A zero-result content search already runs bounded recovery probes, but those probes reduce ripgrep per-file counts to warning text and discard the paths. The model therefore learns that differently-cased, hidden/ignored, or literal matches exist without receiving an actionable location.
This change preserves the bounded path/count rows in structured result fields while keeping the primary exact-search result at
total_count=0.Fixes #80522
Product impact path
User/model invokes the supported core
search_filestool ->tools.file_tools.search_tool->ShellFileOperations.search->_search_content->_zero_match_probe->SearchResult.to_dict(densify=True)-> tool JSON returned to the model.On the base commit, searching for
token_alphaagainst files containingTOKEN_ALPHAreports two case-insensitive matches but omits both paths. On this head, the same public entry returnscase_insensitive_matcheswith each path and count. Hidden/ignored and literal recovery probes receive equivalent structured fields.Root cause and scope
_zero_match_probeparsed every ripgreppath:countrow, summed the counts, and discarded the path variable before returning a warning string. The fix introduces a smallSearchProbeMatchvalue, parses those already-bounded rows once, attaches the relevant structured field, serializes it, and applies the existing credential/read-block filter before model delivery.The existing
head -50bound remains in place. No additional searches are introduced.Commit evidence
c0106e50e7ecedb3ce34e785d949725dc4e0e457(upstream/mainfetched immediately before rebase)51cffe422bacc3c71ba3719b8c040fbbfea1a302git range-diffreports the pre-rebasea24988f6patch and this head as equivalentMain / head / mutation proof
Canonical command:
.envprobe path never reached the read-block omission path.setattrthat attaches probe matches, while leaving warnings and search commands intact, 4 failed, 12 passed for the same four contracts.The base production-file state was checked with:
Additional verification
scripts/run_tests.sh tests/agent/test_file_safety_credentials.py -q: 9 passedscripts/run_tests.sh tests/tools/test_file_operations.py -k TestSearchResult -q: 5 passedscripts/run_tests.sh tests/tools/test_file_tools.py -k TestSearch -q: 4 passedruff 0.15.10 checkon all three changed files: passedpython scripts/check-windows-footguns.py --all: passed, 944 files scannedgit diff --check: passedChecks not passing or not run
ty 0.0.21 checkdid not complete: Ty panicked in unmodifiedtools/checkpoint_manager.py(too many cycle iterations) and reported missing optional dependencies throughout the repository.pytest/yamland pre-existingNoneannotations); none points to an added line. This is not represented as a passing type check.Platform and test harness
Windows NT 10.0.26200.0; Windows PowerShell 5.1.26100.7920; Python 3.12.2; Git 2.52.0.windows.1.
The focused public-entry tests used the official ripgrep 15.1.0 portable binary through a temporary, non-repository Git Bash shim that only cleared Hermes'
MSYS_NO_PATHCONV/MSYS2_ARG_CONV_EXCLvariables before launching nativerg.exe. The initial attempts without that shim were invalid environment failures and are not counted as code evidence. All temporary files were removed afterward.Independent review and overlapping work
monerostarindependently exercised the live publicsearch_toolentry on Ubuntu 26.04 with ripgrep 15.1.0 against the pre-rebase heada24988f6: main returned warning counts only; the PR returned all three structured path/count fields, and the.envcredential filter omitted the field while setting_omitted. Their focused suite passed 16/16. That review is attached to the old commit; despite the equivalentrange-diff, this PR does not claim a current formal approval and may need review refresh.Non-goals
This does not change exact regex matching, automatically include hidden/ignored files, alter pagination, add probe invocations, change search engine selection, or solve the separate Windows native-rg path issue.
Prepared with Codex assistance; evidence verified against the stated commits.