Merge train: 8 tested PRs and a test-gate fix (2026-10-15)

CHANGELOG.md

@@ -2,6 +2,33 @@
 
 ## [Unreleased]
 
+### Changed
+
+- The MCP server instructions now route by question type instead of introducing
+  the server as a code-review tool. With deferred tool loading an agent sees
+  only the tool names and these instructions, so "for code reviews" kept it on
+  Grep for callers, importers and covering tests everywhere else. The new text
+  maps each relationship question to its tool and leaves plain-text search to
+  Grep. Measured with Claude Code on one TypeScript monorepo: across 8 ordinary
+  tasks that never mention the graph (implement, review, brainstorm, plan), the
+  graph was used in 1 run before and 6 after; plain-text questions stayed on
+  Grep (0 of 4 moved). `tests/test_server_instructions.py` keeps every tool and
+  query pattern the text names registered.
+
+### Fixed
+
+- An explicit embedding refresh (`--embedding-provider` with
+  `--embedding-model`, or the `embedding_provider` and `embedding_model`
+  arguments of `build_or_update_graph_tool` and `run_postprocess_tool`) on a
+  graph with no embeddings yet now reports that it embedded nothing. `build`,
+  `update` and `postprocess` return `embeddings_refresh_skipped: true` and a
+  warning to run `code-review-graph embed` once, and so does an `update`
+  that finds no changes. `watch` logs the same warning after each update and
+  keeps running; a refresh that fails against an existing index still stops
+  it, as before. The refresh used to return `ok` with no embedding field,
+  the same result as a build that never asked for one. It still loads no
+  model and makes no network call in that case (#1070).
+
 ## [2.3.9] - 2026-09-18
 
 ### Added

CONTRIBUTING.md

@@ -19,6 +19,11 @@ uv run pytest tests/test_parser.py -v
 
 CI runs the suite on Python 3.10, 3.11, 3.12 and 3.13 and fails below 65% coverage.
 
+`tests/test_parser_grammar_contract.py` checks every built-in class, function, import,
+and call dispatch name against its bundled Tree-sitter grammar. When updating a grammar
+or adding dispatch entries, use the grammar's actual named node types. This contract
+checks configured dispatch names; language fixtures still verify extraction behavior.
+
 ## Linting and Type Checking
 
 ```bash

code_review_graph/changes.py

@@ -787,10 +787,13 @@ def analyze_changes(
     # still gives the right answer: those rows carry ``is_test = 0`` and used
     # to be reported back to the author as their own tests needing tests
     # (issue #1014).
+    # TypeScript interfaces and type aliases have no runtime behavior to test,
+    # so they are never gap candidates (issue #1109).
     gap_candidates = [
         node for node in changed_funcs
         if not (node.is_test or is_test_file(node.file_path, gap_root))
         and node.name not in _TEST_GAP_EXEMPT_NAMES
+        and not node.extra.get("typescript_kind")
     ]
     # TESTED_BY edges are stored as source=production, target=test by the
     # parser, so a changed production function finds its tests by source.

code_review_graph/embeddings.py

@@ -1315,6 +1315,33 @@ def embed_all_nodes(graph_store: GraphStore, embedding_store: EmbeddingStore) ->
     return embedding_store.embed_nodes(all_nodes)
 
 
+# Reported by build and post-process paths when an explicit refresh finds no
+# vectors, so a caller who asked for a refresh learns that nothing was embedded.
+REFRESH_SKIPPED_WARNING = (
+    "Embedding refresh skipped: this graph has no embeddings yet. Run "
+    "`code-review-graph embed` (MCP: embed_graph_tool) once with the same "
+    "provider and model to create them."
+)
+
+
+def has_embeddings(graph_store: GraphStore) -> bool:
+    """True when the graph database holds at least one stored vector.
+
+    Reads the schema first, so the check never creates the ``embeddings``
+    table and never resolves a provider.
+    """
+    has_table = graph_store._conn.execute(
+        "SELECT 1 FROM sqlite_master "
+        "WHERE type = 'table' AND name = 'embeddings'",
+    ).fetchone()
+    if has_table is None:
+        return False
+    has_rows = graph_store._conn.execute(
+        "SELECT 1 FROM embeddings LIMIT 1",
+    ).fetchone()
+    return has_rows is not None
+
+
 def refresh_embeddings(
     graph_store: GraphStore,
     *,
@@ -1325,8 +1352,9 @@ def refresh_embeddings(
 
     This function is deliberately not called by default build paths.  Callers
     must supply both provider and model explicitly.  A graph with no existing
-    vectors returns before provider resolution, so routine builds cannot load
-    a local model, contact a cloud service, or incur API cost.
+    vectors returns ``None`` before provider resolution, so routine builds
+    cannot load a local model, contact a cloud service, or incur API cost;
+    the build paths report that case with ``REFRESH_SKIPPED_WARNING``.
 
     Existing vectors must all use the identity resolved from the requested
     provider/model (including the endpoint for OpenAI-compatible providers).
@@ -1339,16 +1367,7 @@ def refresh_embeddings(
             "Embedding refresh requires an explicit provider and model.",
         )
 
-    has_table = graph_store._conn.execute(
-        "SELECT 1 FROM sqlite_master "
-        "WHERE type = 'table' AND name = 'embeddings'",
-    ).fetchone()
-    if has_table is None:
-        return None
-    has_rows = graph_store._conn.execute(
-        "SELECT 1 FROM embeddings LIMIT 1",
-    ).fetchone()
-    if has_rows is None:
+    if not has_embeddings(graph_store):
         return None
     try:
         rows = graph_store._conn.execute(

code_review_graph/incremental.py

@@ -2185,10 +2185,22 @@ def _raise_watch_update_errors(result: dict, context: str) -> None:
 
 
 def _raise_watch_postprocess_warnings(result: object) -> None:
-    """Treat structured post-processing warnings as a failed watch update."""
+    """Treat structured post-processing warnings as a failed watch update.
+
+    One warning is exempt: an explicit embedding refresh that found no index
+    to refresh. The graph update itself succeeded and the post-processing step
+    has already logged the notice, so the watch keeps running. A refresh that
+    fails against an existing index still fails the update.
+    """
     if not isinstance(result, dict):
         return
-    warnings = result.get("warnings") or []
+    from .embeddings import REFRESH_SKIPPED_WARNING
+
+    warnings = [
+        warning
+        for warning in result.get("warnings") or []
+        if warning != REFRESH_SKIPPED_WARNING
+    ]
     if warnings:
         details = "; ".join(str(warning) for warning in warnings)
         raise RuntimeError(f"post-processing reported warnings: {details}")

code_review_graph/jsonc.py

@@ -301,6 +301,28 @@ def remove_paths(raw: str, paths: Iterable[Sequence[str | int]]) -> str:
             merged.append((start, end))
     for start, end in reversed(merged):
         raw = raw[:start] + raw[end:]
+    # Adjacent removals use one snapshot: removing the last two siblings can
+    # leave the separator after the surviving sibling. Drop only newly exposed
+    # trailing commas; keep pre-existing JSONC trailing commas and all comments.
+    original_trailing = {
+        token.start
+        for index, token in enumerate(tokens[:-1])
+        if token.kind == "," and tokens[index + 1].kind in ("}", "]")
+    }
+    surviving = [
+        token for token in tokens
+        if not any(start <= token.start < end for start, end in merged)
+    ]
+    exposed = [
+        token.start
+        for index, token in enumerate(surviving[:-1])
+        if token.kind == "," and surviving[index + 1].kind in ("}", "]")
+        and token.start not in original_trailing
+    ]
+    # Offsets above refer to the original document, before the removed spans.
+    for offset in reversed(exposed):
+        rewritten_offset = offset - sum(end - start for start, end in merged if end <= offset)
+        raw = raw[:rewritten_offset] + raw[rewritten_offset + 1:]
     return raw
 
 

code_review_graph/main.py

@@ -210,14 +210,38 @@ def _run() -> dict:
         )
 
 
+# What an agent reads before it loads any of these tools: with deferred tool
+# loading (Claude Code's default once many tools are installed) it sees the
+# tool names and this text, nothing else, while Grep's description is always
+# in context. So this text decides when the graph gets picked over text
+# search. It routes by question type rather than introducing the server as a
+# code-review tool, which kept agents on Grep for callers, importers and
+# covering tests everywhere outside a review. Claude Code truncates server
+# instructions at 2,048 characters.
+SERVER_INSTRUCTIONS = (
+    "Code knowledge graph of this repo (Tree-sitter AST + call / import / test edges). "
+    "Use it whenever the question is about RELATIONSHIPS between pieces of code, in any "
+    "phase of work - coding, debugging, planning, brainstorming, refactoring, review - "
+    "not only code review:\n"
+    "- who calls / references symbol X -> query_graph_tool pattern=callers_of / "
+    "references_to\n"
+    "- which files import module or file X -> query_graph_tool pattern=importers_of\n"
+    "- which tests cover X -> query_graph_tool pattern=tests_for\n"
+    "- what breaks if X changes (blast radius) -> get_impact_radius_tool\n"
+    "- what a diff or branch touches -> detect_changes_tool, get_review_context_tool\n"
+    "- how modules depend on each other -> get_architecture_overview_tool, "
+    "list_communities_tool\n"
+    "- follow an execution flow -> list_flows_tool, get_flow_tool\n"
+    "Graph answers include dependents reached through re-exports and barrel files that "
+    "never spell the name, which text search silently misses.\n"
+    "For plain text - string literals, log messages, env var names, comments, config "
+    "keys - use Grep; the graph does not index text."
+)
+
 mcp = FastMCP(
     "code-review-graph",
     version=__version__,
-    instructions=(
-        "Persistent incremental knowledge graph for token-efficient, "
-        "context-aware code reviews. Parses your codebase with Tree-sitter, "
-        "builds a structural graph, and provides smart impact analysis."
-    ),
+    instructions=SERVER_INSTRUCTIONS,
 )
 
 
@@ -257,6 +281,8 @@ async def build_or_update_graph_tool(
             When None (default), falls back to CRG_RECURSE_SUBMODULES env var.
         embedding_provider: Exact provider for an explicit post-build embedding
             refresh. Must be supplied with embedding_model. Default: disabled.
+            Refreshes an existing index only: on a graph with no embeddings
+            the result sets embeddings_refresh_skipped; run embed_graph_tool first.
         embedding_model: Exact model for an explicit post-build embedding
             refresh. Must be supplied with embedding_provider. Default: disabled.
     """
@@ -300,6 +326,8 @@ async def run_postprocess_tool(
         repo_root: Repository root path. Auto-detected if omitted.
         embedding_provider: Exact provider for an explicit embedding refresh.
             Must be supplied with embedding_model. Default: disabled.
+            Refreshes an existing index only: on a graph with no embeddings
+            the result sets embeddings_refresh_skipped; run embed_graph_tool first.
         embedding_model: Exact model for an explicit embedding refresh.
             Must be supplied with embedding_provider. Default: disabled.
     """