Add EmbeddingGemma retrieval support#155
Merged
Merged
Conversation
Scope prose-aware Text labels to EmbeddingGemma Markdown and plain-text chunks. Keep the historical Code label for all existing models so incremental indexing cannot mix document representations. Warn when explicitly selecting models with larger vector dimensions.
test_sanitize_strips_single_char_escape passed a String argument to sanitize_for_terminal(s: &str), breaking cargo test --lib. Pass &str literals to match the sibling sanitize tests. (only surfaces under --lib test compilation, not plain cargo check) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
CI status noteThis PR's own feature code is validated — test-windows, Analyze (rust), and CodeQL all pass. A compile error in a lib test ( The remaining red (test-linux + csharp-integration-tests) is a pre-existing
Proof it's not from here: |
Five tests hardcoded Windows absolute paths (C:\..., \?\C:\..., backslash separators) and asserted separator-rewriting semantics that normalize_path_str deliberately applies ONLY on Windows (backslash is a legal filename char on Unix — see file_meta.rs Aikido 30641757 rationale). They therefore failed on the Linux CI jobs (test-linux, csharp-integration-tests) while passing on test-windows. Gate the Windows-specific tests with #[cfg(windows)] and add #[cfg(unix)] counterparts using native forward-slash paths for the three path-matching tests, preserving Linux coverage. The two pure separator-handling tests (backslashes / mixed) are Windows-only concepts; forward-slash behaviour is already covered by test_path_prefix_no_alias/_empty_alias on all platforms. Pre-existing develop breakage, unrelated to the EmbeddingGemma feature (src/mcp/mod.rs is untouched by that work). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
.map(|l| sanitize_for_terminal(l)) -> .map(sanitize_for_terminal). .lines() yields &str and sanitize_for_terminal takes &str, so the direct function reference is valid. clippy -D warnings (Linux CI) flagged it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
missing_db_not_cached_as_conflicted opened SharedStores directly in the test setup and then let get_or_open_stores open the same LMDB env again — two opens of one env in a single process, which AGENTS.md's LMDB rule forbids. On Linux the first env is not always released before the reopen, so try_open_stores' open failed intermittently -> readonly -> Conflicted -> Err (flaky). try_open_stores creates the env itself (see try_open_stores_creates_db_for_brand_new_repo), so the direct pre-open was redundant. Dropping it leaves a single deterministic open on both platforms. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jul 23, 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
2 participants
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.
Add EmbeddingGemma retrieval support
Lands the EmbeddingGemma retrieval work on
develop(this repo's integration branch), rebased clean onto currentdevelop. Supersedes #147 — full credit to @markschroedr for the original implementation; this PR only re-targets the base branch (master→develop) and incorporates review feedback.What it does
EmbeddingGemma300MQ4model variant as a selectable embedding model.ModelType::content_label(path)emitsText:for Markdown-family files (.md/.markdown/.txt, viaLanguage::from_path) andCode:otherwise — but only for EmbeddingGemma. Every pre-existing model keeps its exact historical input representation (Code:), so existing indexes are byte-for-byte unaffected and need no rebuild.is_heavier_than_default()compares embedding dimensions against the default). Motivated by OOM risk on large corpora / small serve containers.--modelthat mismatches the model an index was built with, pointing at--force(prevents silently mixing embedding spaces).ModelType::valid_short_names()instead of a hardcoded list;model_overrideusesshort_name().Review points addressed
.txtconfirmed to map toLanguage::Markdown, so it is labelledText:consistently under Gemma.tests/cli_model_errors.rscover the model-mismatch rejection path.Validation
cargo fmt --check,cargo clippy -D warnings, andcargo test --libwere clean on the identical source prior to rebase. Finalcargo check/build + full QC gate run in CI (local MSVC toolchain unavailable in the authoring environment).Closes #147.