Skip to content

[Bug]: embed_graph embeds 0 nodes and reports "Semantic search is now active" — embed_all_nodes walks nodes via a path spelling get_all_files never normalizes #945

Description

@mostartbambi

Summary

On a graph whose nodes.file_path rows are stored with native Windows separators, embed_graph embeds nothing and reports success:

{"status":"ok",
 "summary":"Embedded 0 new node(s). Total embeddings: 0. Semantic search is now active.",
 "newly_embedded":0,"total_embeddings":0}

The graph itself is healthy by every visible measure — list_graph_stats reported 205 files / 2632 nodes / 24644 edges at the same moment. Nothing in the output says the operation was a no-op, and the closing sentence asserts the opposite of what happened.

The user-visible consequence is not "no embeddings". It is that semantic_search_nodes then silently degrades to search_mode: "keyword" (tools/query.py:778, which defaults to "keyword" when no vectors match), so the caller believes they are getting vector similarity while getting BM25. On an agent-driven workflow that difference is invisible until someone inspects search_mode.

This is distinct from #911. #911 is about build failing to purge a pre-2.3.8 Windows graph (stale selection uses the stored spelling, deletion uses the normalized one). This report is a different code path with a different failure: the embedding walker never finds any node at all, and unlike build it emits an explicit success claim. Fixing #911 removes one way to reach this state; it does not make embed_graph honest, and it does not help a graph that is already in that spelling.

Root cause

embed_all_nodes reaches nodes by round-tripping through file paths instead of asking for nodes directly (embeddings.py:1307):

def embed_all_nodes(graph_store: GraphStore, embedding_store: EmbeddingStore) -> int:
    """Purge deleted nodes, then embed all current non-file nodes."""
    embedding_store.purge_orphans()
    if not embedding_store.available:
        return 0

    all_files = graph_store.get_all_files()
    all_nodes: list[GraphNode] = []
    for f in all_files:
        all_nodes.extend(graph_store.get_nodes_by_file(f))

    return embedding_store.embed_nodes(all_nodes)

The two halves of that round-trip disagree about spelling, exactly as in #911 but with the asymmetry reversed:

  • get_all_files (graph.py:1187) returns the raw stored file_path values — the SELECT file_path FROM nodes ... UNION SELECT file_path FROM edges applies no normalization.
  • get_nodes_by_fileiter_nodes_by_file (graph.py:441) matches on the normalized spelling:
rows = self._conn.execute(
    "SELECT * FROM nodes WHERE file_path = ?", (normalize_file_path(file_path),)
)

So for a natively-spelled row, get_all_files hands back C:\repo\src\App.tsx, normalize_file_path turns it into C:/repo/src/App.tsx, and the equality test matches zero rows. Every file yields an empty list, all_nodes is [], and embed_nodes returns 0 through its if not to_embed: return 0 guard (embeddings.py:1222) — the same value it would return on a genuinely up-to-date graph, which is why the caller cannot tell the two apart.

embed_graph (tools/docs.py:93) then formats that 0 into an unconditional success sentence:

newly_embedded = embed_all_nodes(store, emb_store)
total = emb_store.count()
return {
    "status": "ok",
    "summary": (
        f"Embedded {newly_embedded} new node(s). "
        f"Total embeddings: {total}. "
        "Semantic search is now active."
    ),
    ...
}

"Semantic search is now active." is appended even when total == 0, i.e. precisely when it is false.

Note the provider guard above it passes cleanly — emb_store.available is True and self.provider is a live LocalEmbeddingProvider. The dependency is installed and working; nothing in the embedding stack is at fault. I verified this directly:

files: 205
sample file: C:\...\admin\postcss.config.js
nodes in sample: 0
total nodes via files: 0
available: True provider: <code_review_graph.embeddings.LocalEmbeddingProvider object at 0x...>

2632 rows in nodes, 0 reachable through the documented walker.

Reproduction

No Windows and no pre-2.3.8 install are needed — the stored spelling is the only thing that matters, so the same UPDATE trick from #911 reproduces it anywhere:

# 1. clean 2.3.8 graph
code-review-graph build --repo . --data-dir /tmp/repro
sqlite3 /tmp/repro/graph.db "SELECT count(*) FROM nodes;"           # e.g. 2854

# 2. put it into the spelling a pre-2.3.8 Windows build produced
sqlite3 /tmp/repro/graph.db \
  "UPDATE nodes SET file_path = replace(file_path,'/','\\');
   UPDATE edges SET file_path = replace(file_path,'/','\\');"

# 3. embed
code-review-graph embed --repo . --data-dir /tmp/repro
# Embedded 0 new node(s). Total embeddings: 0. Semantic search is now active.
# exit 0

sqlite3 /tmp/repro/graph.db "SELECT count(*) FROM embeddings;"      # 0

Expected: either every node is embedded, or the command reports that it could not reach the graph.
Actual: exit 0, zero work done, and an explicit claim that the feature is now active.

Suggested fix

Two independent changes; the first is the actual bug, the second is what made it cost an hour to find.

1. Do not route node enumeration through path spelling. GraphStore already exposes exactly the query embed_all_nodes wants (graph.py:449):

all_nodes = graph_store.get_all_nodes(exclude_files=True)

That is one SELECT * FROM nodes WHERE kind != 'File', it is spelling-agnostic by construction, it drops the per-file loop, and it makes the if node.kind == "File": continue filter in embed_nodes redundant rather than load-bearing. It also cannot regress the moment some other writer introduces a spelling the reader does not expect.

2. Stop asserting the postcondition unconditionally. "Semantic search is now active." should be conditional on total > 0. A graph with nodes but zero embeddings after an embed run is a contradiction the tool is in a position to notice and report, e.g. status: "error" (or at minimum a warning) along the lines of "graph has N nodes but none were reachable for embedding — the store may hold paths in a spelling this version does not read; rebuild from scratch". Anything is better than a success string that is false exactly when the user most needs the truth.

Worth noting for whoever picks this up: a full_rebuild does not recover a graph in this state (that is #911) — the rebuild writes the new spelling but the old rows survive, leaving both halves in the table (I measured 5475 rows where the build reported 2854). The only recovery I found was deleting graph.db and building from scratch, after which embed_graph embedded 2638 nodes and semantic_search_nodes returned search_mode: "semantic". A fix for this issue should probably be paired with a rebuild path that actually purges, or users will hit #911 while trying to escape this one.

Environment

  • code-review-graph 2.3.8 (via uvx --from "code-review-graph[embeddings]" code-review-graph serve, MCP stdio)
  • Windows 11 Pro 26200, Python 3.13
  • sentence-transformers 5.7.0, torch 2.13.0+cpu, numpy 2.5.2
  • Repo: 212 files, TypeScript/TSX/JS monorepo

Related

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions