Skip to content

fix(file-ops): keep spaced filenames in files_only search results - #40707

Closed
TabooHarmony wants to merge 1 commit into
NousResearch:mainfrom
TabooHarmony:fix/search-files-only-space-paths
Closed

fix(file-ops): keep spaced filenames in files_only search results#40707
TabooHarmony wants to merge 1 commit into
NousResearch:mainfrom
TabooHarmony:fix/search-files-only-space-paths

Conversation

@TabooHarmony

Copy link
Copy Markdown

What does this PR do?

Fixes a regression from #39858 where search_files in files_only mode silently drops any matching file whose name contains a space.

#39858 added _split_tool_diagnostics to separate rg/grep error text from match output by line shape (since _exec merges stderr into stdout). The bare-path shape regex (^[^\s:][^\s]*$) forbids whitespace, so a files-only output line like My Document.md or sub/Meeting Notes.txt fails the regex, carries no rg:/grep: prefix, and gets folded into the "diagnostics" bucket — stripped from the result. No error is raised, so the loss is silent.

Content and count modes are unaffected: their path:line:content / path:count shapes satisfy the regex via the :N anchor. Only files_only, with its bare unanchored path lines, is hit.

The fix makes _split_tool_diagnostics mode-aware. In files_only mode it tracks rg's multi-line regex parse error: block and strips a line only while inside an active block. A regex parse error aborts the search, so no real matches are interleaved with its continuation lines — every other files_only line is treated as a real path and preserved verbatim, including names with spaces and (as a bonus) names that happen to start with error: or with leading whitespace. The hard-error path (rg:/grep: prefixed lines and the parse-error block) is still classified as diagnostics, so genuine failures are surfaced unchanged.

Related Issue

No separate issue — the regression and its repro are self-contained below.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/file_operations.py:
    • _split_tool_diagnostics takes an output_mode argument (default "content", backward compatible). In files_only mode it classifies by parse-error-block tracking instead of the whitespace-hostile content shape regex.
    • Both call sites in _search_with_rg / _search_with_grep pass output_mode.
  • tests/tools/test_search_error_guard.py:
    • test_files_only_preserves_spaced_paths — spaced names survive.
    • test_files_only_still_drops_regex_parse_error — hard error still classified as diagnostics in files_only mode.
    • test_files_only_keeps_path_named_like_error_summary — a file named error: notes.md is preserved.
    • test_files_only_keeps_indented_path_outside_error_block — a path with leading whitespace is preserved outside a parse-error block.

How to Test

Reproduce the drop on main:

from tools.file_operations import ShellFileOperations
from tools.environments.local import LocalEnvironment
import os, tempfile

root = tempfile.mkdtemp(); os.makedirs(f"{root}/sub")
for n in ["normal.py", "My Document.md", "sub/Meeting Notes.txt", "sub/clean.txt"]:
    open(f"{root}/{n}", "w").write("NEEDLE\n")

ops = ShellFileOperations(LocalEnvironment(cwd=root), cwd=root)
print(sorted(ops._search_with_rg("NEEDLE", root, None, 50, 0, "files_only", 0).files))

Before: only normal.py and sub/clean.txt (two spaced names dropped, no error). After: all four files returned. Reproduces on both _search_with_rg and _search_with_grep.

Run the tests:

pytest tests/tools/test_search_error_guard.py::TestSplitToolDiagnostics -q

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(file-ops):)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run the relevant tests and they pass
  • I've added tests for my changes
  • I've tested on my platform: Linux

Documentation & Housekeeping

  • N/A — no docs/config/architecture changes

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists tool/file File tools (read, write, patch, search) labels Jun 6, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for isolating the regression. The premise is confirmed on current main: tools/file_operations.py:401 rejects whitespace in bare files_only paths, while both backend paths call the splitter at tools/file_operations.py:2304 and tools/file_operations.py:2432.

Problems

  • The new tests in tests/tools/test_search_error_guard.py call _split_tool_diagnostics directly. They do not verify either production backend forwards output_mode or returns a spaced file path through its files_only parsing path.

Suggested changes

  • Add a parameterized end-to-end test using the existing _METHODS and _search helpers in tests/tools/test_search_error_guard.py:63-73; create spaced filenames and assert each available backend returns them in files_only mode.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 14, 2026
match output by line shape. The bare-path shape regex (^[^\s:][^\s]*$)
forbids whitespace, so a files_only line like "My Document.md" is
misclassified as a diagnostic and silently stripped from results. No error
is raised. Content and count modes are unaffected: their path:line and
path:count shapes satisfy the regex via the :N anchor.

In files_only mode, classify by tracking rg's multi-line "regex parse
error:" block instead of the whitespace-hostile shape regex. A parse error
aborts the search, so no real matches are interleaved with its continuation
lines; a line is stripped only while inside an active parse-error block.
Every other files_only line is treated as a real path and preserved
verbatim — including names containing spaces, or names that happen to start
with "error:". The hard-error path (rg:/grep: prefix and the parse-error
block) is still classified as diagnostics, so genuine failures are surfaced
unchanged.

Adds regression tests for spaced-path preservation, continued hard-error
detection, and the two awkward-but-valid filename cases (leading "error:",
leading whitespace) in files_only mode.
@TabooHarmony
TabooHarmony force-pushed the fix/search-files-only-space-paths branch from bb21f4c to e45cc63 Compare July 15, 2026 04:47
@TabooHarmony

Copy link
Copy Markdown
Author

Addressed the review feedback and rebased onto current main. Added a parameterized end-to-end regression test using the existing _METHODS and _search helpers. It creates filenames containing spaces and verifies that both available production backends return them in files_only mode.

The new test fails against current main for both grep and ripgrep, and passes with this fix. Relevant file-operation and search tests pass locally, aside from the existing unreadable-file cases that cannot reproduce under root because root can read chmod(000) files.

@TabooHarmony TabooHarmony closed this by deleting the head repository Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform tool/file File tools (read, write, patch, search) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants