diff --git a/scripts/commands/search.py b/scripts/commands/search.py index a1c52d0..efe53d4 100644 --- a/scripts/commands/search.py +++ b/scripts/commands/search.py @@ -25,6 +25,7 @@ import argparse import os +import re import sys from typing import Any, Dict @@ -171,6 +172,34 @@ def _run_graph(args, workspace) -> Dict[str, Any]: return _qg_execute(sub_args, workspace) +_CYPHER_START_RE = re.compile(r"^\s*MATCH\s*\(", re.IGNORECASE) + + +def _detect_pattern_workspace_swap(pattern, workspace): + """Heuristic for issue #239: `search` is the only umbrella command with + pattern before workspace (opposite of every other command) — a very easy + mistake, and one that doesn't error, it just silently searches for the + real workspace path as the pattern and returns an empty "ok" result. + + Returns a hint string if ``pattern`` looks like it's actually a + workspace path (an existing directory) — a strong signal the two + arguments were swapped — else None. + """ + if not pattern or not isinstance(pattern, str): + return None + try: + if os.path.isdir(pattern): + return ( + f"pattern {pattern!r} is an existing directory — this looks like " + "the pattern/workspace arguments may be swapped. `search` takes " + "pattern first, workspace second (opposite of every other " + "umbrella command): `search \"query\" `." + ) + except (OSError, ValueError): + pass + return None + + def execute(args, workspace): """Dispatch to the selected search mode and normalize output shape. @@ -179,6 +208,25 @@ def execute(args, workspace): @MUTATES: nothing (read-only) """ mode = getattr(args, "mode", "semantic") or "semantic" + pattern = getattr(args, "pattern", None) + hints = [] + + swap_hint = _detect_pattern_workspace_swap(pattern, workspace) + if swap_hint: + hints.append(swap_hint) + + # Issue #239: auto-route Cypher-shaped patterns to graph mode. Only + # fires on high-confidence matches (starts with "MATCH (") to avoid + # second-guessing genuine regex/symbol/semantic queries. + if mode != "graph" and pattern and _CYPHER_START_RE.match(pattern): + hints.append( + f"pattern looks like a Cypher query but --mode was '{mode}' — " + "auto-routed to --mode graph. Pass --mode graph explicitly to " + "silence this hint." + ) + mode = "graph" + args.mode = "graph" + try: if mode == "semantic": result = _run_semantic(args, workspace) @@ -192,13 +240,19 @@ def execute(args, workspace): return {"s": "error", "st": {"mode": mode}, "r": [], "error": f"unknown mode '{mode}'"} except Exception as exc: - return {"s": "error", "st": {"mode": mode}, - "r": [], "error": str(exc), - "error_type": type(exc).__name__} + out = {"s": "error", "st": {"mode": mode}, + "r": [], "error": str(exc), + "error_type": type(exc).__name__} + if hints: + out["_hints"] = hints + return out # Normalize to {s, st, r} shape while preserving original payload. if not isinstance(result, dict): - return {"s": "ok", "st": {"mode": mode}, "r": [{"result": result}]} + out = {"s": "ok", "st": {"mode": mode}, "r": [{"result": result}]} + if hints: + out["_hints"] = hints + return out status = result.pop("status", "ok") # Move large payload lists into ``r`` if present, keep stats in ``st``. rows = None @@ -206,11 +260,14 @@ def execute(args, workspace): if key in result and isinstance(result[key], list): rows = result.pop(key) break - return { + out = { "s": status, "st": {"mode": mode, **result}, "r": rows if rows is not None else [], } + if hints: + out["_hints"] = hints + return out register_command( diff --git a/tests/test_search_command.py b/tests/test_search_command.py new file mode 100644 index 0000000..d8a39d5 --- /dev/null +++ b/tests/test_search_command.py @@ -0,0 +1,97 @@ +"""Tests for search command self-correction hints (issue #239). + +`search` is the only umbrella command with `pattern` before `workspace` +(opposite of every other command) — getting it backwards doesn't error, +it silently searches for the workspace path as the pattern and returns +an empty "ok" result. Separately, a Cypher-shaped pattern passed without +`--mode graph` gets misinterpreted by the default semantic mode instead +of erroring or hinting. Both are runtime self-correction, not just docs. +""" + +import argparse +import os +import sys +import tempfile +from unittest import mock + +import pytest + +SCRIPT_DIR = os.path.join( + os.path.dirname(os.path.dirname(os.path.abspath(__file__))), "scripts" +) +if SCRIPT_DIR not in sys.path: + sys.path.insert(0, SCRIPT_DIR) + +from commands.search import ( # noqa: E402 + _detect_pattern_workspace_swap, + execute, +) + + +class TestDetectPatternWorkspaceSwap: + def test_pattern_is_existing_directory_flags_swap(self): + with tempfile.TemporaryDirectory() as tmpdir: + hint = _detect_pattern_workspace_swap(tmpdir, "some pattern") + assert hint is not None + assert "swapped" in hint + + def test_pattern_is_normal_string_no_hint(self): + assert _detect_pattern_workspace_swap("getAccessMode", ".") is None + + def test_none_pattern_no_crash(self): + assert _detect_pattern_workspace_swap(None, ".") is None + + +class TestSearchExecuteHints: + def _args(self, pattern, mode="semantic"): + return argparse.Namespace( + pattern=pattern, mode=mode, top=None, db_path=None, + file_type=None, file=None, max_results=200, context=0, + ignore_case=False, whole_word=False, domain=None, fuzzy=False, + validate=False, limit=None, offset=0, + ) + + def test_argument_swap_produces_hint(self): + with tempfile.TemporaryDirectory() as tmpdir, \ + mock.patch("commands.search._run_semantic", return_value={"status": "ok"}): + result = execute(self._args(pattern=tmpdir), tmpdir) + assert result.get("_hints") + assert any("swapped" in h for h in result["_hints"]) + + def test_cypher_pattern_auto_routes_to_graph_mode(self): + with mock.patch("commands.search._run_graph", return_value={"status": "ok"}) as mock_graph, \ + mock.patch("commands.search._run_semantic") as mock_semantic: + args = self._args(pattern="MATCH (n) RETURN n LIMIT 5", mode="semantic") + result = execute(args, ".") + + mock_graph.assert_called_once() + mock_semantic.assert_not_called() + assert result["st"]["mode"] == "graph" + assert result.get("_hints") + assert any("auto-routed" in h for h in result["_hints"]) + + def test_cypher_pattern_with_explicit_graph_mode_no_hint(self): + """If the caller already passed --mode graph, no hint is needed — + the auto-route heuristic should be a no-op, not noisy.""" + with mock.patch("commands.search._run_graph", return_value={"status": "ok"}): + args = self._args(pattern="MATCH (n) RETURN n LIMIT 5", mode="graph") + result = execute(args, ".") + assert not result.get("_hints") + + def test_normal_symbol_query_no_hints(self): + with mock.patch("commands.search._run_symbol", return_value={"status": "ok", "results": []}): + args = self._args(pattern="getAccessMode", mode="symbol") + result = execute(args, ".") + assert "_hints" not in result + + def test_regex_pattern_resembling_but_not_cypher_not_rerouted(self): + """A regex pattern that merely contains the word MATCH somewhere + (not at the start followed by a paren) must not be reinterpreted + as Cypher — only high-confidence matches auto-route.""" + with mock.patch("commands.search._run_regex", return_value={"status": "ok", "matches": []}) as mock_regex, \ + mock.patch("commands.search._run_graph") as mock_graph: + args = self._args(pattern="function MATCHER(x) {", mode="regex") + result = execute(args, ".") + mock_graph.assert_not_called() + mock_regex.assert_called_once() + assert "_hints" not in result