🐛 fix(index): stamp model on serve/git-hook index path (worktree "model: unknown") - #168
Merged
Merged
Conversation
Fixes the "model: unknown" worktree bug. When a repo is registered via POST /repos (the git-hook path), the store is opened first and ensure_schema_version pre-creates a metadata.json containing only schema_version — no model fields. force_reindex's Step 0 then saw the file already existed and skipped the default-model stamp, so the index was left with no model_short_name. Every reader showed "model: unknown", and read_model_metadata's "unknown" sentinel disabled the empty-index live-chunk-count self-heal — making the worktree index look empty so the agent fell back to grep. Fix A (force_reindex_with_stores): when the preserved metadata.json has no model_short_name, stamp ModelType::default() (short_name/name/dims) before the merge write. Fix B (perform_incremental_refresh_with_stores): persist the resolved embed_model alongside the chunk/file stats so incremental refreshes also keep the model recorded. Both use ModelType::default() rather than hardcoded strings, mirroring the working CLI index path (src/index/mod.rs). Adds a regression test reproducing the schema-version-only bootstrap state. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…metadata_fields Addresses reviewer Important remark on df1e504: the ModelType -> 3 JSON fields (model_short_name/model_name/dimensions) block was duplicated across four index-creation sites (force_reindex override + Fix A + Fix B, and the CLI index_with_options save + final save). The keys and value derivation could drift and the sites already differed in style (obj.insert closures vs Value indexing). Extracts a single source of truth, ModelType::write_metadata_fields(obj), and routes all four sites through it: - force_reindex_with_stores: model override + default-stamp (via as_object_mut) - perform_incremental_refresh_with_stores: Fix B write - index_with_options: partial-cancel save + final save The CLI final-save previously captured model_{short_name,name,dimensions} strings from embedding_service before dropping the ONNX model; since the service is built directly from model_type (EmbeddingService::with_cache_dir), those values are identical to model_type.*, so the capture block is removed and model_type is used directly. EmbeddingService::model_name() thereby loses its last caller and gets #[allow(dead_code)] to match the sibling accessor convention in embed/mod.rs. No behavior change: same keys, same values. cargo check/clippy/test green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…a_fields Addresses reviewer Important remark on 50c9397: the create-minimal-DB path in serve (src/mcp/mod.rs) was a 5th, un-consolidated copy of the three-key model stamp — and it had drifted, writing model_name as the Debug variant name (format!("{:?}", model_type) → "AllMiniLML6V2Q") instead of model_type.name() ("all-MiniLM-L6-v2-q") that every other path writes. Display-only (readers key on model_short_name), so no resolution defect, but it contradicted write_metadata_fields' own "cannot drift" contract. Routes this site through model_type.write_metadata_fields(obj) too, so the helper's "every index-creation path" claim now holds literally and model_name is consistent across all five sites. Drops the now-unused local model_name; model_short_name/dimensions are still used below. No functional change beyond correcting the drifted model_name value. cargo check/clippy/test (mcp: 196, index: 21) green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add [Unreleased] CHANGELOG entry for the serve/git-hook "model: unknown" worktree-index fix and the write_metadata_fields consolidation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
model: unknownbug on indexes created via the serve / git-hook (POST /repos) path — git worktrees especially.ensure_schema_versionpre-createsmetadata.jsonwith onlyschema_version(no model fields); force-reindex then saw the file existed and skipped stamping the default model. Readers showedmodel: unknown, and that sentinel disabled the empty-index live-chunk-count self-heal → a good worktree index looked empty and agents fell back to grep.ModelType::write_metadata_fieldssource of truth across all five index-creation sites, correcting a pre-existing drift (auto-create-DB path wrote the Debug variant nameAllMiniLML6V2Qasmodel_name).Changes
src/index/manager.rs— Fix A (force_reindex_with_stores: stamp default model when metadata lacks it) + Fix B (perform_incremental_refresh_with_stores: persist resolved model) + regression test.src/embed/embedder.rs— newModelType::write_metadata_fieldshelper (single source of truth).src/index/mod.rs— CLI partial-cancel + final save routed through helper; dead capture block removed.src/mcp/mod.rs— auto-create-DB stamp routed through helper (drift fixed).src/embed/mod.rs—#[allow(dead_code)]on now-unusedmodel_name()accessor.CHANGELOG.md—[Unreleased]entry.Testing
cargo fmt --check,cargo check --all-targets,cargo clippy --all-targets -D warningscargo test --lib— 596 passed, 20 ignored (pre-push QC gate)force_reindex_stamps_model_when_metadata_has_only_schema_versionNote
Existing worktree indexes (e.g.
HUSQ.Aprimo-209169) need one reindex to pick up the stamped model.🤖 Generated with Claude Code