Skip to content

feat(status): report indexed embedding metadata#48

Open
saagpatel wants to merge 1 commit into
devwhodevs:mainfrom
saagpatel:agent/index-model-metadata
Open

feat(status): report indexed embedding metadata#48
saagpatel wants to merge 1 commit into
devwhodevs:mainfrom
saagpatel:agent/index-model-metadata

Conversation

@saagpatel

Copy link
Copy Markdown

Refs #44.

What changed

  • Store the configured embedding model URI and the embedder's actual dimension during indexing.
  • Report those stored values from engraph status instead of the obsolete hard-coded all-MiniLM-L6-v2 label.
  • Add a model-free indexing test for the persisted metadata.

Why

The status output could claim a model that the current llama.cpp implementation no longer uses. That makes index provenance and dimension troubleshooting misleading.

Impact

New indexing runs record the model identity used to build the index. Existing indexes report <unknown> until they are indexed again; this PR does not force a rebuild.

Validation

  • cargo fmt --check
  • cargo clippy -- -D warnings
  • cargo test --lib indexer::tests::test_index_records_embedding_metadata
  • cargo test --lib search::tests::test_format_status — 2 passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1c74670843

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/indexer.rs
.embed
.as_deref()
.unwrap_or(&defaults.embed_uri);
store.set_meta("embedding_model_uri", embedding_uri)?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Rebuild when the embedding URI changes

When a user changes models.embed to another model with the same output dimension, engraph index can take the non-rebuild path and diff_vault will skip unchanged files, but this line still overwrites embedding_model_uri. engraph status then reports the new model even though existing vectors were produced by the old model, and searches embed queries with the new model against stale embeddings; the CLI also tells users the next index will re-embed the vault at src/main.rs:700. Compare the stored URI before writing it and force a rebuild/reset when it changes.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant