Summary
embed_graph_tool returns {"status": "ok"} and reports nodes embedded, but list_graph_stats_tool continues to show Embeddings: 0 afterwards because the local embedding provider silently mis-reports availability when sentence-transformers is not installed. The tool should either succeed-and-actually-embed, or fail loudly with an actionable install hint. Right now it does neither — it returns "ok" and embeds nothing.
Version
code-review-graph==2.3.2 (also reproducible on main as of 4b6293f)
Environment
- Linux (Ubuntu 24.04, Python 3.12)
sentence-transformers NOT installed (the package is from pip install code-review-graph without the [embeddings] extra)
- No
CRG_OPENAI_*, GOOGLE_API_KEY, or MINIMAX_API_KEY env vars set
- Default provider (local)
Reproduction
pip install code-review-graph (no extras)
code-review-graph build --repo /some/repo — graph builds normally
- Call
embed_graph_tool — returns {"status": "ok", "newly_embedded": N, "total_embeddings": N} (or 0/0)
- Call
list_graph_stats_tool — reports Embeddings: 0 nodes embedded
graph.db mtime hasn't moved; no embed/sentence-transformers Python process was ever spawned
Root cause
In code_review_graph/embeddings.py:
class LocalEmbeddingProvider(EmbeddingProvider):
def __init__(self, model_name: str | None = None) -> None:
self._model_name = ...
self._model = None # Lazy-loaded <-- import deferred
def _get_model(self):
if self._model is None:
try:
from sentence_transformers import SentenceTransformer
...
except ImportError:
raise ImportError(...)
return self._model
__init__ does NOT import sentence_transformers. The import is deferred to _get_model().
In get_provider():
# Default: local
try:
return LocalEmbeddingProvider(model_name=model)
except ImportError:
return None
This try/except ImportError never fires because __init__ doesn't import anything. So get_provider() returns a non-None provider, EmbeddingStore.available = True, and embed_graph_tool's availability guard is bypassed.
Then in EmbeddingStore.embed_nodes, the deferred import would fire when self.provider.embed(texts) is called inside the if to_embed: branch — but here's the second issue: if to_embed is empty (which can happen if _node_to_text produces deduplicated text-hashes the first run finds zero novel entries, or if the table is somehow already partially populated from a previous half-finished run), the function returns 0 silently with no provider call ever made — and the user sees "ok, 0 embedded" with no diagnostic.
In our repro the more direct path was: embed_graph_tool reported success without writing any rows, and no Python child process was ever spawned (verified with ps/pidstat). That implies the ImportError did fire somewhere upstream and was swallowed, OR the to_embed list was empty due to a pre-existing schema row, OR embed raised an exception that was caught and returned as success.
Suggested fix
Make get_provider("local") actually verify availability:
# Default: local
try:
import sentence_transformers # noqa: F401
except ImportError:
return None
try:
return LocalEmbeddingProvider(model_name=model)
except ImportError:
return None
(There's already a _check_available() helper in the file that does exactly this, but it's never called from get_provider().)
Once get_provider() correctly returns None when the local backend isn't installed, EmbeddingStore.available becomes False, and embed_graph_tool's existing guard fires the helpful error:
The local embedding provider needs sentence-transformers.
Install with: pip install code-review-graph[embeddings] —
or switch provider to 'openai' / 'google' / 'minimax'.
That's the desired UX.
Bonus diagnostic tool
A get_embedding_status_tool (or extending list_graph_stats_tool) to report:
- Currently configured provider (local / openai / google / minimax)
- Whether the provider is
available (i.e., its backend imports succeed)
- The model name in use
- The Python interpreter / venv it would run in (since the MCP server can be run from various venvs and the provider's backend needs to be installed in the same one)
would help users diagnose this kind of mismatch quickly. We had to read source to figure out why "ok, 0 embedded" actually meant "no backend installed".
Acceptance criteria
- With
sentence-transformers not installed and no cloud creds, embed_graph_tool returns status=error with the install hint, not status=ok with 0 embeddings.
- With
sentence-transformers installed, embed_graph_tool actually embeds N>0 nodes and list_graph_stats_tool reflects the count.
Summary
embed_graph_toolreturns{"status": "ok"}and reports nodes embedded, butlist_graph_stats_toolcontinues to showEmbeddings: 0afterwards because the local embedding provider silently mis-reports availability whensentence-transformersis not installed. The tool should either succeed-and-actually-embed, or fail loudly with an actionable install hint. Right now it does neither — it returns "ok" and embeds nothing.Version
code-review-graph==2.3.2(also reproducible on main as of4b6293f)Environment
sentence-transformersNOT installed (the package is frompip install code-review-graphwithout the[embeddings]extra)CRG_OPENAI_*,GOOGLE_API_KEY, orMINIMAX_API_KEYenv vars setReproduction
pip install code-review-graph(no extras)code-review-graph build --repo /some/repo— graph builds normallyembed_graph_tool— returns{"status": "ok", "newly_embedded": N, "total_embeddings": N}(or 0/0)list_graph_stats_tool— reportsEmbeddings: 0 nodes embeddedgraph.dbmtime hasn't moved; no embed/sentence-transformers Python process was ever spawnedRoot cause
In
code_review_graph/embeddings.py:__init__does NOT importsentence_transformers. The import is deferred to_get_model().In
get_provider():This
try/except ImportErrornever fires because__init__doesn't import anything. Soget_provider()returns a non-None provider,EmbeddingStore.available = True, andembed_graph_tool's availability guard is bypassed.Then in
EmbeddingStore.embed_nodes, the deferred import would fire whenself.provider.embed(texts)is called inside theif to_embed:branch — but here's the second issue: ifto_embedis empty (which can happen if_node_to_textproduces deduplicated text-hashes the first run finds zero novel entries, or if the table is somehow already partially populated from a previous half-finished run), the function returns 0 silently with no provider call ever made — and the user sees "ok, 0 embedded" with no diagnostic.In our repro the more direct path was:
embed_graph_toolreported success without writing any rows, and no Python child process was ever spawned (verified withps/pidstat). That implies the ImportError did fire somewhere upstream and was swallowed, OR the to_embed list was empty due to a pre-existing schema row, ORembedraised an exception that was caught and returned as success.Suggested fix
Make
get_provider("local")actually verify availability:(There's already a
_check_available()helper in the file that does exactly this, but it's never called fromget_provider().)Once
get_provider()correctly returns None when the local backend isn't installed,EmbeddingStore.availablebecomes False, andembed_graph_tool's existing guard fires the helpful error:That's the desired UX.
Bonus diagnostic tool
A
get_embedding_status_tool(or extendinglist_graph_stats_tool) to report:available(i.e., its backend imports succeed)would help users diagnose this kind of mismatch quickly. We had to read source to figure out why "ok, 0 embedded" actually meant "no backend installed".
Acceptance criteria
sentence-transformersnot installed and no cloud creds,embed_graph_toolreturnsstatus=errorwith the install hint, notstatus=okwith 0 embeddings.sentence-transformersinstalled,embed_graph_toolactually embeds N>0 nodes andlist_graph_stats_toolreflects the count.