Skip to content

fix: support trailing globs in path search - #1881

Open
mikemikimike wants to merge 7 commits into
repowise-dev:mainfrom
mikemikimike:fix/path-trailing-glob
Open

fix: support trailing globs in path search#1881
mikemikimike wants to merge 7 commits into
repowise-dev:mainfrom
mikemikimike:fix/path-trailing-glob

Conversation

@mikemikimike

Copy link
Copy Markdown

Summary

  • Treat trailing * and ? in path-mode searches as boundary glob markers.
  • Add regression coverage for both trailing glob forms against a real SQLite-backed search.

Related Issues

Fixes #1871

Implementation

Normalize the path query by stripping only leading/trailing * and ? after path normalization. This preserves the existing substring matching behavior and deliberately avoids interpreting mid-path glob syntax as a broader glob language.

Compatibility

Queries without trailing glob markers are unchanged. A query containing only glob markers continues to return no results.

Test Plan

  • PYTHONPATH=packages/server/src;packages/core/src python -m pytest tests/unit/server/mcp/test_search.py -k trailing_glob -q — 2 passed
  • python -m ruff check packages/server/src/repowise/server/mcp_server/tool_search_symbols.py tests/unit/server/mcp/test_search.py — passed
  • git diff --check — passed
  • The full test_search.py module was attempted but exceeded the local wait window and was stopped; the focused regression tests completed successfully.
  • Frontend build not run; this change is server-side Python only.

@repowise-bot

repowise-bot Bot commented Aug 23, 2026

Copy link
Copy Markdown

✅ Health of changed files: 5.8 (unchanged)

📋 At a glance
2 hotspots touched · 1 file with recent fix history.

Files & modules (2)
  • packages (1 file)
    • .../mcp_server/tool_search_symbols.py
  • tests (1 file)
    • .../mcp/test_search.py

✅ Health gate: passed

🔎 More signals (2)

🗺️ Change map

flowchart LR
  subgraph PR ["Changed in this PR (2 with dependents)"]
    f_packages_server_src_repowise_server_mcp_server_tool_search_symbols_py[".../mcp_server/tool_search_symbols.py 🔥"]:::changed
    f_tests_unit_server_mcp_test_search_py[".../mcp/test_search.py"]:::changed
  end
  f_packages_server_src_repowise_server_mcp_server__prose_symbols_py[".../mcp_server/_prose_symbols.py"]
  f_packages_server_src_repowise_server_mcp_server_tool_search_symbols_py --> f_packages_server_src_repowise_server_mcp_server__prose_symbols_py
  f_packages_server_src_repowise_server_mcp_server_tool_search_py[".../mcp_server/tool_search.py"]
  f_packages_server_src_repowise_server_mcp_server_tool_search_symbols_py --> f_packages_server_src_repowise_server_mcp_server_tool_search_py
  f_packages_server_src_repowise_server_mcp_server__answer_pipeline_py[".../mcp_server/_answer_pipeline.py"]
  f_tests_unit_server_mcp_test_search_py --> f_packages_server_src_repowise_server_mcp_server__answer_pipeline_py
  f_packages_server_src_repowise_server_mcp_server_tool_answer_answer_py[".../tool_answer/answer.py"]
  f_tests_unit_server_mcp_test_search_py --> f_packages_server_src_repowise_server_mcp_server_tool_answer_answer_py
  classDef changed fill:#dbeafe,stroke:#1d4ed8,color:#1e3a5f
  classDef warn fill:#fef3c7,stroke:#b45309,color:#78350f
  classDef guard fill:#dcfce7,stroke:#15803d,color:#14532d
Loading

Solid arrows: code that imports the changed files (4 direct dependents, from the last indexed snapshot). Dashed: history/tests.

🔥 Hotspots touched (2)

  • .../mcp_server/tool_search_symbols.py: 3 commits/90d, 3 dependents · primary owner: Raghav Chamadiya (96%)
  • .../mcp/test_search.py: 15 commits/90d, 3 dependents · primary owner: Raghav Chamadiya (86%)

👀 Suggested reviewers @RaghavChamadiya


📊 See the full report for this PR
Your repo map with this PR's blast radius lit up, every caller of the contracts it changes, and health before and after. No sign-in. · ⭐ Star Repowise · 📥 Install bot · Silence on a single PR with [skip repowise] in the title · Per-repo toggle on repowise.dev/settings?tab=bot · Updated 2026-08-24 08:47 UTC

@mikemikimike

Copy link
Copy Markdown
Author

Added boundary coverage for ordinary filenames, whitespace plus leading/trailing markers, marker-only queries, and mid-path glob markers. Focused path-search tests now pass 8/8; Ruff and git diff --check also pass.

@mikemikimike
mikemikimike force-pushed the fix/path-trailing-glob branch from 93cf105 to 711df1a Compare August 23, 2026 15:51
@RaghavChamadiya

Copy link
Copy Markdown
Member

@mikemikimike, this is the trailing-glob fix I am taking for #1871, over the other PR on the same issue. Yours arrived first and links the issue, and the parametrized table covering *, ?, a bare *, a bare ? and the mid-string tool_*_overview case is broader than the alternative.

Your CI red is not yours. Only Python 3.13 failed while 3.11 and 3.12 passed, which is already the shape of something unrelated. The failure is:

tests/unit/workspace/test_extraction_walk_budget.py::test_each_file_is_read_once_per_repo
    assert counters["reads"] <= 2 * len(REPO_SOURCE)

That assertion counts Path.read_text / read_bytes calls process-wide with no path filter, so reads by unrelated threads spend the same budget and it reds on PRs that cannot reach the code. It is filed as #1854, and #1859 fixed it by counting per path and scoping to the fixture tree. #1859 merged at 04:00 today; your run started at 03:52, so it was built against a base without the fix.

git fetch origin && git rebase origin/main and the run should go green. Ping me when it has and I will merge without another round.

One thing I would like from the other PR when you rebase, and it is a comment rather than code. Nothing on main will say why .strip("*?") only touches the ends. The next person reads strip as a mistake that should have been replace, "fixes" it, and turns src/*/main.py into a substring search for src//main.py. A line saying that a substring match cannot honour a mid-string glob, so stripping there would answer a different question, is the guard against that. Take it or leave it, it is not a blocker.

@Georgefifth

Copy link
Copy Markdown

Heads up before merging — the current head (ac6da050) is broken, and I have a ready-to-apply patch.

What happened. The docs commit f02cac1901 folded the comment and the qnorm assignment into a single physical line — the in-line separators are literal \n text, not real newlines. The whole assignment is now part of the comment:

    # Path mode is substring matching, ... meaningful;\n    # stripping mid-path globs ... path.\n    qnorm = query.strip().lower().replace("\\", "/").strip("*?")

Consequence. qnorm is referenced (line 275 if not qnorm:) but never assigned — no module-level binding, no global. Every path search now raises:

NameError: name 'qnorm' is not defined

and the two glob tests this PR adds fail on the branch itself:

uv run pytest tests/unit/server/mcp/test_search.py -k glob
FAILED test_path_search_accepts_trailing_glob[*]  # NameError at line 275

Verified three ways against the exact head SHA via the raw blob (not a local checkout): AST walk shows qnorm referenced-but-unassigned in search_paths_single, executing the function body raises NameError, and the branch's own test suite reproduces it. The code-health check doesn't execute the module, so nothing caught this. Commits up to 94cfa412 were fine; the breakage is specific to f02cac1901.

Ready patch. Based on ac6da050, branch docs/explain-path-glob-strip in my fork (single commit, +5/−1) — it restores the assignment as its own line and uses the comment wording I proposed from the closed #1887 (why mid-string globs are deliberately left alone), which is the documentation the review asked for:

-    # Path mode is substring matching, so only boundary markers are meaningful;\n    # stripping mid-path globs would change the query into a different literal path.\n    qnorm = query.strip().lower().replace("\\", "/").strip("*?")
+    # Path mode is substring matching, so boundary ``*?`` markers carry no
+    # information and are stripped. Mid-string globs (``src/*/main.py``) are
+    # left alone: a substring match cannot honour them, so stripping there
+    # would change the query.
+    qnorm = query.strip().lower().replace("\\", "/").strip("*?")

On that branch: glob tests 7/7 pass, ruff check clean. Happy to hand off the commit directly if easier than cherry-picking.

@Georgefifth

Copy link
Copy Markdown

Verified the new head (c2d503b) locally: the qnorm assignment is restored, the 4-line comment landed as written, and the glob suite passes (7/7). Ready to merge from my side. Thanks for the quick turnaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] search_codebase path mode cannot resolve a trailing glob, because escape_like does not strip it

3 participants