fix(agent): grep-only wiki search — fix dialect, encoding, partial-error, exclusions, guards (addresses code review)

Author: KylinMountain

openkb/agent/query.py

@@ -46,7 +46,8 @@
    yet seen on a wiki page. Because grep is lexical (not semantic), try a
    few term variants: acronym and expansion, singular/plural, close
    synonyms. For any matching page you have NOT already read, read_file it
-   and fold in relevant content. If grep surfaces a claim that contradicts
+   (grep_wiki lines are `path:line:text`; pass only the path, before the
+   first colon) and fold in relevant content. If grep surfaces a claim that contradicts
    your draft, note both claims with their citations rather than silently
    choosing one. Do at most 3 grep rounds (a round = one concept and its
    variants); stop once a round surfaces no new page. grep_wiki is a check,
@@ -111,12 +112,15 @@ def grep_wiki(pattern: str, ignore_case: bool = True, fixed_string: bool = False
         away, pages you never opened, or contradicting mentions. It does NOT
         search long-document page content (use get_page_content for that).
 
-        Returns up to 50 matches as 'relative/path.md:LINE: text'. Feed any
-        new path into read_file. Try a few term variants (acronym/expansion,
+        Returns up to 50 matches, one per line as 'path.md:LINE:text'. The
+        path is everything before the FIRST colon — pass only that path to
+        read_file (not the whole line). Pattern is an extended regex (ERE):
+        alternation 'a|b', '?', '+', '()' work; set fixed_string=True for a
+        literal search. Try a few term variants (acronym/expansion,
         singular/plural, synonyms) — this is lexical, not semantic.
 
         Args:
-            pattern: Search pattern (regex by default).
+            pattern: Search pattern (extended regex by default).
             ignore_case: Case-insensitive (default True).
             fixed_string: Treat pattern as a literal string, not a regex.
         """

openkb/agent/tools.py

@@ -7,11 +7,15 @@
 from __future__ import annotations
 
 import contextlib
+import functools
 import json as _json
+import os
 import shutil
 import subprocess
 from pathlib import Path
 
+from openkb.schema import EXCLUDED_WIKI_FILES
+
 # grep_wiki_files tuning
 _GREP_MAX_LINES = 50
 _GREP_TIMEOUT_S = 10
@@ -60,91 +64,99 @@ def read_wiki_file(path: str, wiki_root: str) -> str:
     return full_path.read_text(encoding="utf-8")
 
 
+@functools.cache
+def _grep_binary() -> str | None:
+    """Locate the system grep once per process (PATH does not change at runtime)."""
+    return shutil.which("grep")
+
+
 def grep_wiki_files(
     pattern: str,
     wiki_root: str,
     *,
     ignore_case: bool = True,
     fixed_string: bool = False,
 ) -> str:
-    """Lexically search the wiki's markdown layer for ``pattern``.
+    """Lexically search the wiki's markdown layer for ``pattern`` using grep.
+
+    A completeness sweep over every ``*.md`` file under *wiki_root* —
+    summaries, concepts, entities, explorations, ``index.md``, and short-doc
+    ``sources/*.md``. Long-doc per-page ``*.json`` (PageIndex's domain) is
+    excluded (only ``*.md`` is searched), as are the wiki's bookkeeping /
+    scaffolding files (``log.md``, ``AGENTS.md``, ``SCHEMA.md`` — see
+    :data:`openkb.schema.EXCLUDED_WIKI_FILES`).
 
-    A completeness sweep: shells out to ripgrep (preferred) or grep
-    (fallback) over every ``*.md`` file under *wiki_root* — summaries,
-    concepts, entities, explorations, ``index.md``, and short-doc
-    ``sources/*.md``. Long-doc per-page ``*.json`` (PageIndex's domain) and
-    ``log.md`` bookkeeping are excluded.
+    Shells out to the system ``grep`` (POSIX, ubiquitous on macOS/Linux) with
+    ``shell=False``, so a hostile *pattern* cannot inject commands. ``pattern``
+    is an **extended** regular expression (ERE) by default — alternation
+    ``a|b``, ``?``, ``+``, ``()`` all work — or a literal string when
+    *fixed_string* is True.
 
     Args:
-        pattern: Search pattern. Regex by default; literal when
-            *fixed_string* is True.
+        pattern: Search pattern. ERE by default; literal when *fixed_string*.
         wiki_root: Absolute path to the wiki root directory.
         ignore_case: Case-insensitive match (default True).
         fixed_string: Treat *pattern* as a literal string, not a regex.
 
     Returns:
-        Up to :data:`_GREP_MAX_LINES` matches as ``relative/path.md:LINE: text``
-        lines, plus a truncation notice if capped. On no match / missing
-        binary / timeout / error, returns an explicit message string. Never
-        raises and never invokes a shell (``shell=False``), so a hostile
-        *pattern* cannot inject commands.
+        Up to :data:`_GREP_MAX_LINES` matches, each line ``relative/path.md:LINE:text``
+        (the path is everything before the first colon), plus a truncation
+        notice if capped. On empty pattern / no match / missing grep / timeout /
+        error-with-no-results, returns an explicit message string. Never raises.
     """
+    if not pattern or not pattern.strip():
+        return "Provide a non-empty search pattern."
+
     root = Path(wiki_root).resolve()
     if not root.exists():
         return f"Wiki root not found: {wiki_root}"
 
-    rg = shutil.which("rg")
-    grep = shutil.which("grep")
-
-    if rg:
-        # --no-ignore: the wiki dir is often gitignored; without this rg
-        # silently returns zero matches inside a real OpenKB checkout.
-        cmd = [
-            rg, "--line-number", "--no-heading", "--color", "never",
-            "--no-ignore", "-g", "*.md", "-g", "!log.md",
-        ]
-        if ignore_case:
-            cmd.append("-i")
-        if fixed_string:
-            cmd.append("-F")
-        cmd += ["-e", pattern, str(root)]
-    elif grep:
-        cmd = [grep, "-rn", "--include=*.md", "--exclude-dir=images"]
-        if ignore_case:
-            cmd.append("-i")
-        if fixed_string:
-            cmd.append("-F")
-        cmd += ["-e", pattern, str(root)]
-    else:
+    grep = _grep_binary()
+    if not grep:
         return "grep unavailable on this system."
 
+    cmd = [grep, "-rn", "--include=*.md"]
+    for name in sorted(EXCLUDED_WIKI_FILES):
+        cmd.append(f"--exclude={name}")
+    if ignore_case:
+        cmd.append("-i")
+    cmd.append("-F" if fixed_string else "-E")
+    cmd += ["-e", pattern, str(root)]
+
     try:
         proc = subprocess.run(
-            cmd, capture_output=True, text=True,
+            cmd, capture_output=True, text=True, errors="replace",
             timeout=_GREP_TIMEOUT_S, check=False,
         )
     except subprocess.TimeoutExpired:
         return "grep timed out; narrow the pattern."
 
-    # rg/grep convention: 0 = matches, 1 = no matches, >=2 = real error.
-    if proc.returncode >= 2:
-        stderr_lines = (proc.stderr or "").strip().splitlines()
-        first = stderr_lines[0] if stderr_lines else "unknown error"
-        return f"grep error: {first}."
-
-    prefix = str(root) + "/"
+    prefix = str(root) + os.sep
     results: list[str] = []
     for line in proc.stdout.splitlines():
-        if not line.strip():
+        if not line:
             continue
-        rel = line[len(prefix):] if line.startswith(prefix) else line
+        if not line.startswith(prefix):
+            continue  # defensive: only surface paths under wiki_root
+        rel = line[len(prefix):]
         path_part = rel.split(":", 1)[0]
-        # Defensive: grep --include=*.md still matches log.md; drop it.
-        if path_part == "log.md" or path_part.endswith("/log.md"):
+        # Defense in depth: --exclude already drops these basenames; this also
+        # catches a same-named file in a subdirectory.
+        if Path(path_part).name in EXCLUDED_WIKI_FILES:
             continue
         results.append(rel)
+        if len(results) > _GREP_MAX_LINES:
+            break  # only need 51 to detect truncation; stop processing
 
     if not results:
+        # grep exit codes: 0 = match, 1 = no match, >=2 = error. grep can exit
+        # >=2 (e.g. one unreadable file) while still printing valid matches —
+        # those were collected above. Only report an error when nothing usable
+        # came back.
+        if proc.returncode >= 2:
+            stderr_lines = (proc.stderr or "").strip().splitlines()
+            first = stderr_lines[0] if stderr_lines else "unknown error"
+            return f"grep error: {first}."
         return f"No matches for {pattern}."
 
     truncated = len(results) > _GREP_MAX_LINES

openkb/lint.py

@@ -15,13 +15,13 @@
 
 import yaml
 
-from openkb.schema import PAGE_CONTENT_DIRS
+from openkb.schema import EXCLUDED_WIKI_FILES, PAGE_CONTENT_DIRS
 
 # Matches [[wikilink]] or [[subdir/link]]
 _WIKILINK_RE = re.compile(r"\[\[([^\]]+)\]\]")
 
 # Files to exclude from lint scanning (schema, logs, etc.)
-_EXCLUDED_FILES = {"AGENTS.md", "SCHEMA.md", "log.md"}
+_EXCLUDED_FILES = EXCLUDED_WIKI_FILES
 
 
 def _normalize_target(target: str) -> str:

openkb/schema.py

@@ -6,6 +6,11 @@
 # for surfaces that enumerate page content (list, lint, status, skill gate).
 PAGE_CONTENT_DIRS = ("summaries", "concepts", "entities")
 
+# Bookkeeping / scaffolding files that live under wiki/ but are NOT content.
+# Single source of truth shared by the structural linter and the grep search
+# tool so their exclusion policy can never drift.
+EXCLUDED_WIKI_FILES: frozenset[str] = frozenset({"AGENTS.md", "SCHEMA.md", "log.md"})
+
 # Canonical empty index.md seed. Used by `openkb init` and the compiler's
 # lazy-create path so they never drift.
 INDEX_SEED = "# Knowledge Base Index\n\n## Documents\n\n## Concepts\n\n## Entities\n\n## Explorations\n"