Upgrade embedding model to voyage-context-4 - #14
Merged
Conversation
Replace voyage-context-3 with voyage-context-4 as the default embedding model. It is a drop-in replacement: same 1024-dim vectors, same contextualized_embed API, better retrieval quality (+2.08% chunk-level NDCG@10 per Voyage), lower price ($0.12/1M vs $0.18/1M), and its own 200M free-token tier. - Resolve the model from a new VOYAGE_MODEL env var inside VoyageEmbedder (default voyage-context-4) so the server and indexer can never disagree; drop the hardcoded model in server.py - Pass VOYAGE_MODEL through docker-compose and document it in .env.example - Fix docker exec examples to use the image's .venv/bin/python (the system python has no dependencies installed) - Disable the file watcher in the documented MCP client config and add scripts/run_vault_mcp.sh: the main container process already owns watching, so exec'd stdio sessions should be query-only - Add .dockerignore so host __pycache__, .venv, and .env never leak into the image - Fix embedder init log to print the resolved model instead of the constructor argument Migration: embeddings from different models are not comparable. After upgrading, run a full re-index (docker exec -i obsidian-graph .venv/bin/python -m src.indexer). The embedding cache is keyed by model name and invalidates itself.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Pre-existing docs/TOOLS.md MCP config example is now inconsistent with README
The MCP config example at docs/TOOLS.md:371 still shows "args": ["exec", "-i", "obsidian-graph", "python", "-m", "src.server"] — using bare python instead of .venv/bin/python and missing the OBSIDIAN_WATCH_ENABLED=false flag. This is in unchanged lines so it's pre-existing, but the PR updated the equivalent example in README.md:182 to use .venv/bin/python and disable the watcher. Users following the TOOLS.md example would get a python: command not found error since the Docker image only has .venv/bin/python available.
(Refers to line 371)
Was this helpful? React with 👍 or 👎 to provide feedback.
Owner
Author
There was a problem hiding this comment.
Good catch. Fixed in d256395: TOOLS.md now matches the README example (.venv/bin/python plus OBSIDIAN_WATCH_ENABLED=false so exec'd MCP sessions don't spawn a duplicate watcher).
Review findings from the voyage-context-4 migration review:
- Detect model drift (Critical): new index_metadata table records which
embedding model built the index (auto-created for existing databases);
the server logs a loud error at startup when its configured model
differs, instead of silently comparing incomparable vectors
- Honest indexer accounting (High): report chunks actually upserted this
run, log an error instead of a success line when files failed, and
exit non-zero so partial migrations are visible; only record the
corpus model after a fully successful run
- Cover VOYAGE_MODEL resolution (High): new tests/test_embedder.py
covers default fallback, env override, explicit-arg precedence,
empty/whitespace values, model-keyed cache isolation, and
invalid-model retry behavior
- Fix documented test commands (High): the image ships without dev deps
or tests/, so in-container pytest could never work; docs now use
host-side uv commands and drop the nonexistent test_e2e.py reference
- Harden model resolution (Medium): empty or whitespace VOYAGE_MODEL
falls back to the default, matching compose ${VAR:-} semantics
- Fail fast on invalid model names (Medium): invalid-model API errors
are deterministic and no longer retried with backoff
- Replace broken deadlock diagnostic in docs/CONCURRENCY.md (a fresh
interpreter cannot inspect the server's event loop) with log-based
diagnosis
Owner
Author
Quality gate review (T3, 6 parallel lenses + clawpatch)Reviewed by six isolated reviewers (correctness, silent failures, tests, types, comments, simplify) plus the clawpatch CLI. Findings remediated in 3f68a51:
Deferred (pre-existing on master, out of scope): watcher swallowing per-file embed failures, stale chunk rows when notes shrink, |
Dogfooding against a real vault found two gaps: - Files that fail to read (e.g. iCloud dataless placeholders that Docker file sharing cannot materialize) were logged and skipped without counting as failures, so the indexer exited 0 and recorded the corpus model despite leaving files unindexed. Read errors now join the failure list, fail the run, and block the embedding_model metadata write. Regression test added. - scripts/run_e2e_tests.py (35-check standalone E2E suite) had a broken documented invocation: the image ships only src/ and no system-python deps. Docstring and README now show the working docker cp invocation.
actions/setup-python v5 and astral-sh/setup-uv v3 target the deprecated Node 20 runtime on GitHub runners; bump to v6/v7.
This was referenced Jul 13, 2026
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.
Summary
Replaces
voyage-context-3withvoyage-context-4as the default embedding model. Voyage positions context-4 as a drop-in replacement (announcement): same 1024-dim vectors, samecontextualized_embed()API, better retrieval quality (+2.08% chunk-level / +1.4% document-level NDCG@10 across 39 datasets), lower price ($0.12/1M vs $0.18/1M), and its own 200M free-token allowance (Voyage free tiers are per model).Changes
VOYAGE_MODELenv var (defaultvoyage-context-4), resolved insideVoyageEmbedderso the server and indexer always agree on the model; removes the hardcoded model inserver.pyVOYAGE_MODELthroughdocker-compose.yml; document in.env.exampledocker execexamples to use the image's.venv/bin/python(the systempythonin the container has no dependencies installed, so the documented indexer command failed)OBSIDIAN_WATCH_ENABLED=false) and a newscripts/run_vault_mcp.shwraps the invocation; the main container process already owns watching, so each connected client was spawning a duplicate watcher.dockerignore: host__pycache__(stale bytecode),.venv, and.envno longer leak into the imageMigration
Embeddings from different models are not comparable, so switching requires a full re-index:
The embedding cache is keyed by model name, so it invalidates itself; no schema change is needed (both models are 1024-dim).
Testing
voyageai0.3.7 (pin stays>=0.3.0); the model name is server-side