fix(lore): embed-rebuild refuses when no embedder is wired#96
Open
mvanhorn wants to merge 1 commit into
Open
Conversation
Before this change, EmbedRebuildCommand checked meta.embedder_state, saw "enabled", then proceeded to wipe lore_vectors and re-encode using embed.NewNullEmbedder() as a placeholder. Every encode returned ErrEmbedderDisabled, so coverage stayed at 0% even while lore_health reported the embedder as enabled. Net effect: silent destruction of vectors with no usable rebuild. This change pulls *EmbedDeps via embedFromDeps(ctx, d) (the same contract appraise_cmd uses) and refuses to rebuild when the embedder is not wired: - meta says disabled OR embedder is not wired -> Disabled: true with a reason explaining the wiring gap, and existing vectors are not touched. - meta says enabled AND embedder is wired -> the real embedder is used for the encode loop and coverage returns >0%. The previous "placeholder NullEmbedder pending QUEST-212" block is removed. internal/lore/embed_rebuild_test.go adds two table-free tests: - TestEmbedRebuild_RefusesWhenEmbedderNotWired: seeds initial vectors, calls the handler with stubCommandDeps(nil), asserts Disabled=true and vector rows still match the pre-call count. - TestEmbedRebuild_HappyPathWithWiredEmbedder: seeds entries without vectors, calls the handler with a wired EmbedDeps holding a 384-dim fake embedder, asserts Encoded>0 and lore_vectors row count matches. Closes mathomhaus#88
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
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
guild lore embed-rebuildpreviously readmeta.embedder_state, sawenabled, then proceeded to wipelore_vectorsand re-encode usingembed.NewNullEmbedder()as a placeholder pending QUEST-212. Every encode returnedErrEmbedderDisabled, so coverage stayed at 0% even whilelore_healthreported the embedder as enabled. Net effect: silent destruction of vectors with no usable rebuild.This change pulls
*EmbedDepsviaembedFromDeps(ctx, d)(the same contractappraise_cmdalready uses) and refuses to rebuild when the embedder is not wired:metasays disabled OR embedder is not wired ->Disabled: truewith a reason explaining the wiring gap, and existing vectors are left untouched.metasays enabled AND embedder is wired -> the real embedder is used for the encode loop and coverage returns >0%.The previous "placeholder NullEmbedder pending QUEST-212" block is removed.
Acceptance (matches the issue's acceptance section)
embed-rebuildwith a wired embedder, vector coverage returns to >0% for projects with active embedders.lore_health"enabled" report no longer contradict: rebuild now refuses up-front when the embedder isn't wired and emits a clear reason instead of silently no-opping.Tests
internal/lore/embed_rebuild_test.go(new):TestEmbedRebuild_RefusesWhenEmbedderNotWired- seeds initial vectors through the existingInscribepath with a fake 384-dim embedder, then callsEmbedRebuildCommand.HandlerwithstubCommandDeps(nil). AssertsDisabled == true, a non-empty reason, and that thelore_vectorsrow count is unchanged (no silent wipe).TestEmbedRebuild_HappyPathWithWiredEmbedder- seeds entries without vectors (deps == nilduring inscribe), then calls the handler withstubCommandDeps(&EmbedDeps{Embedder: fake, ModelID: "test-model"}). AssertsDisabled == false,Encoded > 0, and that the row count after rebuild matches the reportedEncoded.The fake embedder is a small inline type returning a deterministic 384-dim float32 slice; 384 matches the production BGE dimension so the existing Tx2 vector-write path accepts the encode.
go test ./internal/lore/...,go build ./...,go vet ./...: clean.Files
internal/lore/embed_rebuild_cmd.go- replace the NullEmbedder placeholder with anembedFromDeps + !ed.Enabled() -> Disabledgate.internal/lore/embed_rebuild_test.go(new) - regression tests for both branches.Closes #88.