Conversation
…s an identity Natural-language search missed symbols whose names carry the words being searched for. `getUserById` reaches the embedder as one rare token, so a query of "get user by id" had little to match. Identifier splitting Measured on a doc->symbol retrieval eval, BGE-small, pure semantic R@1: camelCase (TypeScript, 377 symbols) 0.355 -> 0.637 +79% PascalCase (Rust, 200 symbols) 0.520 -> 0.640 +23% snake_case (Rust, 723 symbols) 0.683 -> 0.692 +1.3% snake_case (SystemVerilog, 696) 0.510 -> 0.497 -2.5% So it applies only where words run together. snake_case and kebab-case already tokenise into the same words; a leading or trailing delimiter separates nothing, so `_handleClick` splits like `handleClick`. The Rust snake-vs-Pascal pair is the controlled comparison: same repo, same docs, only the casing differs. Exposed as --split-identifiers, codegraph.splitIdentifiers in VS Code and a checkbox in JetBrains, wired through the CLI, MCP builder, engine config, daemon, LSP initializationOptions and the cross-client parity check. Flags that could not be turned off `--full-body-embedding` was `#[arg(long, default_value = "true")]` on a bool, which clap parses as a flag that rejects a value: always true, and `=false` errored. Both it and --split-identifiers now take an optional value. Vector identity Stored vectors had no record of what produced them, so nothing could tell whether they were comparable to the ones a process was about to make. Switching --embedding-model was the sharpest case: there is no dimension check anywhere, and cosine_similarity zips its inputs, so a 768d query against a stored 384d vector silently scored a dot product over the first 384 dimensions against norms of different lengths. Semantic ranking was garbage, with no error, until a manual reindex. Present before this change. Vectors are now stamped with the schema, model, full-body and split settings that built them, and a mismatched set is never loaded. - A rebuild takes ownership of the project once, after it knows it has something to write, in one atomic step that replaces the stored set and writes its stamp. Saves only add; writes are chunked so a save under memory pressure does not triple the footprint; checkpoints keep a crashed rebuild resumable. - A store that cannot be read is left alone. graph.db is one RocksDB shared by every project, and a lock held elsewhere reads exactly like an absent store; treating it as absent claimed the project and cleared a valid set. An unreadable store now embeds in memory for the session. - A live --watch daemon owns its project's vectors; sessions never claim over it, and the daemon carries the same embed-text settings as the sessions that read from it. - An auto-spawned engine is passed --full-body-embedding and --split-identifiers with their values. Both are in the stamp, so an engine started with defaults would have served its client no vectors at all. An index written by 0.20.1 or earlier carries no stamp and is re-embedded the first time this version opens it. A --watch daemon left running across the upgrade keeps writing unstamped vectors that this build will not load; restart it after upgrading. Known trade-off: an LSP client that omits fullBodyEmbedding still defaults it off, unlike every other client. VS Code and JetBrains always send it; a bare nvim/emacs/helix client and an IDE client on the same project would replace each other's vector set. Aligning the default would move every bare client to ~3x slower indexing, so it is left and documented in place. Also: the eval harness read CODEGRAPH_SPLIT_IDS with is_ok(), so `=0` enabled it and an A/B run had two identical arms; StorageBackend gains scan_prefix_keys with a default body so out-of-tree backends still compile. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017rVbt7rENTwXkdHt3Bpgb5
🔍 CodeGraph PR Review23 files changed (+1223/−185, 68 functions) · Risk: 🔴 high Blast radius117 direct callers affected (55 breaking) across
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
The developer asked to rebase the feat/split-identifiers-by-default branch onto the new main (after PR #25, the watcher/workspace-filter fix, was merged) and address the remaining review findings so it could go back through the no-mistakes gate. The branch makes embeddings split camelCase identifiers by default and gives stored vectors an identity stamp (covering embed-text schema, full-body, split-identifiers and embedding model), so vectors built under a different configuration are not mixed or loaded as current. The three pending findings to fix were: tell an unreadable RocksDB apart from an absent one, so a transient failure does not trigger a destructive claim that deletes a project's vectors; claim the project only after confirming there is rebuild work; and keep the model in the stamp, accepting that changing models re-embeds. Conflicts with main.rs, mcp/engine.rs and mcp/server.rs had to be resolved, keeping both sides' changes. The LSP fullBodyEmbedding default stays false. Implied constraints are no outside contributor code, plain dashes instead of em dashes, conventional commit messages, and pushing only through the no-mistakes gate.
What Changed
--split-identifiersoption is on by default. It also embeds the word-split form of identifiers whose words run together, sogetUserByIdis embedded as "get user by id" as well. Names already separated by_or-are left as they are. The option is wired through the CLI, the MCP builder, engine config, the--watchdaemon and LSPinitializationOptions. VS Code gets acodegraph.splitIdentifierssetting and JetBrains gets a settings checkbox.--full-body-embeddingand--split-identifiersnow take an optional value, so--full-body-embedding=falseworks; before this change that flag was always true. An auto-spawned engine is now passed both values.--watchdaemon keeps ownership of its project's vectors. Sessions attached to a daemon whose vectors don't match now report the mismatch instead of "embeddings building".StorageBackendgainsscan_prefix_keys, which has a default implementation and is overridden in the memory, namespaced and RocksDB backends. Theembed_evalexample now parsesCODEGRAPH_SPLIT_IDSas a value, not just whether it is present. The READMEs and VS Code setting descriptions explain the stamp, automatic re-embedding when settings change, and that--watchmust be restarted after upgrading.🤖 Generated with Claude Code
Risk Assessment
Testing
I rebuilt the server at the target commit and ran five live MCP sessions plus a
--watchdaemon on an isolated HOME and workspace. Vectors are stamped split=on by default and reload without re-embedding on restart. Changing the split flag makes the stored set Mismatched, so it is rebuilt rather than mixed in. A session whose flags differ from the daemon's now gets the "built with different embedding settings ... Restart the daemon" status instead of "Embeddings are building". A session whose flags match the daemon loads its 8 vectors and returns semantic results. All five passed. The JSON responses, server logs and a combined transcript are in the evidence directory. In the daemon-attached runs every symbol came back twice: the daemon resolved the workspace to /private/tmp (macOS's real path for /tmp), and this change does not touch path handling. Nothing in the worktree changed; only /tmp scratch dirs and evidence files were created.Evidence: Round 2 combined transcript (all scenarios)
Evidence: Session vs mismatched daemon: symbol_search response
embedding_status: "The --watch daemon's stored vectors were built with different embedding settings, so semantic matching is unavailable this session - results are from name/text search only. Restart the daemon with the same --full-body-embedding / --split-identifiers / --embedding-model flags as this session."Evidence: Session vs mismatched daemon: server log
Evidence: Session matching daemon: response
Evidence: Watcher daemon log
Evidence: Fresh default session log (split on, stamp)
~/.no-mistakes/evidence/01M3TSRN3TH8FYM6MTAP3ZSWB5/r2-run2-restart.log)~/.no-mistakes/evidence/01M3TSRN3TH8FYM6MTAP3ZSWB5/r2-run3-split-off.log)Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ No issues found.
crates/codegraph-server/src/mcp/server.rs:360- Pre-existing on main (not introduced by this change), but it defeats this change's 'an unreadable store is left alone' guarantee one layer up. If graph.db's RocksDB lock is held by another live process when an MCP session starts, load_persistent_graph treats the failure as corruption. It bumps graph.generation and sweeps the old db to graph.db.corrupt, which drops every project's graph and vectors from the shared store. The session then indexed nothing, because index_state said the files were unchanged, so persisted 0 nodes and symbol_search returned empty results. Reproduced live in run6-locked-store.log. Consider telling lock contention apart from corruption before quarantining.crates/codegraph-server/src/ai_query/engine.rs:1409- When a session attaches to a --watch daemon whose vectors carry a different stamp (a state new in this change), the server logs that semantic search is unavailable for the whole session. But codegraph_symbol_search still tells the agent 'Embeddings are building in the background. Semantic matching is temporarily unavailable', and no build will ever run. Seen live in run13-session-vs-daemon.json. The status should say the vectors were built with different settings and the daemon needs a restart.cargo build -p codegraph-server(dev profile, CARGO_TARGET_DIR=/tmp/cg-nm-target)codegraph-server --full-body-embedding=false | --split-identifiers=false | --split-identifiers | --split-identifiers=maybe --graph-only --run-tool codegraph_symbol_search(CLI parsing)MCP stdio driver (mcp-driver.py): initialize + codegraph_symbol_search over a 12-function TS workspace with HOME=/tmp/cgnm-home and CODEGRAPH_SKIP_MEMORY_CHECK=1Run 1-2: default config on a fresh store, then a restart (stamp written, then reloaded without re-embedding)Run 3-4:--split-identifiers=false, then--full-body-embedding=falsewith split on/off (Mismatched detected, re-embedded under new stamp)Run 5:--embedding-model jina-code-v2then back tobge-small(384d/768d sets never mixed)Run 6: graph.db RocksDB LOCK held by another process from before startup (found the pre-existing quarantine behaviour)Run 8-9: RocksDB lock taken after graph load and before the vector read, session with--split-identifiers=false; then unlocked default sessionRun 10-11: workspace emptied of symbols plus mismatched config, then symbols restored with the default configRun 12-13:--watchdaemon (split=on) live, then an MCP session with--split-identifiers=falseattaches🔧 Fix applied.
1 info still open:
crates/codegraph-server/src/backend.rs- Unrelated to this change: when MCP sessions and a --watch daemon reach the same workspace through a symlinked path (on macOS, /tmp is a link to /private/tmp), the daemon stores the files under the resolved path. The graph then holds both copies and symbol_search returns every symbol twice (8 vectors for 4 functions). The diff touches no path handling. I did not check whether the base commit behaves the same.cargo build -p codegraph-server(CARGO_TARGET_DIR=/tmp/cg-nm-target) at target commit 3a05fcapython3 mcp-driver.py '["--full-body-embedding"]': fresh MCP session with default flags on an isolated HOME and git workspace; codegraph_symbol_search for 'combine two ordered lists' / 'fetch user by identifier'Same session restarted with identical flags to check vectors are reloaded, not re-embedded--split-identifiers=falsestandalone session against the split=on storecodegraph-server --watch --full-body-embedding --extension-path /tmpdaemon (split on by default) owning the workspaceMCP session with--split-identifiers=falseattached to the daemon; checked the symbol_search embedding_status textMCP session with flags matching the daemon attached to it; checked that semantic results come back✅ **Document** - passed
✅ No issues found.
crates/codegraph-memory/examples/embed_eval.rs-cargo fmt --all -- --checkfails across about 60 files that existed before this change, mostly the language parser crates, codegraph-harness and codegraph-memory. That includes parts of embed_eval.rs that this change did not touch. The files this change edits in codegraph-server and codegraph storage are rustfmt-clean. Clippy's warnings in backend.rs and mcp/server.rs are also on lines this change did not touch. These were left alone to keep the diff scoped. A separatecargo fmt --alland clippy cleanup commit is worth doing as a follow-up.✅ **Push** - passed
✅ No issues found.