feat(memory,brain,knowledge): multi-engine embed, FeedbackEventKind, reindex (#376, #377, #268, #380, #484)#502
Conversation
MAJ-001: Add unit tests for SemanticFeedback fold paths (ExplicitPositive, ImplicitNegative, Correction) verifying exact posterior deltas in fold.rs. Add FeedbackEventKind unit tests in event.rs: from_signal_str all variants, update_weight values, is_positive classification, and interpret integration. Add BetaPosterior weighted update tests in state.rs. MAJ-002: Add debug_assert!(weight > 0.0) to update_success_weighted and update_failure_weighted in BetaPosterior to catch negative/zero weight callers in debug builds. Include corresponding should_panic tests. MAJ-003: knowledge.reindex no longer swallows embed/insert errors silently. Both entity and note loops now match on the embed result, count failures into an errors_skipped counter, and emit tracing::warn on each failure. The final response JSON includes errors_skipped so callers can detect partial failures. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Merge sequence
Merge #499 first — PRs #500, #502, #503 depend on it (GitHub auto-retargets on merge). |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7eccb92e9
ℹ️ 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".
| let dims = vec.len(); | ||
| // Use the primary/default model's embedding as the top-level field | ||
| // for backward compatibility. | ||
| if primary_embedding.is_none() || model_name == &primary_model { |
There was a problem hiding this comment.
Do not fall back to a non-default embedding
When multiple embedders are registered and the default/primary model fails, this condition lets the first successful non-default model become the top-level embedding; EmbedderRegistry::names() is documented as unspecified order, and the success response does not include a top-level model name. That changes the previous single-default behavior from failing/null to returning a vector with potentially different dimensions, so existing callers that treat memory.embed.embedding as the default model's vector can pass it to the wrong vector store or compare incompatible embeddings.
Useful? React with 👍 / 👎.
…reindex (#376, #377, #268, #380, #484) - RecallConfig: remove scaffolded fields, add BrainProfileHint (#376, #484) - Multi-engine embed envelope in recall handlers (#377) - FeedbackEventKind: 5 semantic variants with weighted update magnitudes (#268) - knowledge.reindex subhandler with ReindexParams (#380) Closes #376, #377, #268, #380, #484 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
MAJ-001: Add unit tests for SemanticFeedback fold paths (ExplicitPositive, ImplicitNegative, Correction) verifying exact posterior deltas in fold.rs. Add FeedbackEventKind unit tests in event.rs: from_signal_str all variants, update_weight values, is_positive classification, and interpret integration. Add BetaPosterior weighted update tests in state.rs. MAJ-002: Add debug_assert!(weight > 0.0) to update_success_weighted and update_failure_weighted in BetaPosterior to catch negative/zero weight callers in debug builds. Include corresponding should_panic tests. MAJ-003: knowledge.reindex no longer swallows embed/insert errors silently. Both entity and note loops now match on the embed result, count failures into an errors_skipped counter, and emit tracing::warn on each failure. The final response JSON includes errors_skipped so callers can detect partial failures. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
bc75afe to
59ce783
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ntract test Entity CRUD was inconsistent: create/list/search used graph namespace (shared), but get/update/delete used caller namespace. This meant even the entity creator couldn't retrieve their own entity by UUID. Now handle_get takes both caller and graph tokens, trying graph first for entities/edges and caller for notes/events. Update and delete use graph_token since they target entities/edges. Contract test updated to reflect ADR-007 shared graph design: entities are visible cross-namespace, notes remain isolated. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…test depends_on is not valid between two concepts per ADR-002. Use extends instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Note create returns "id" not "note_id". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Closes #376, #377, #268, #380, #484
Stacked on #499 (authorize → Result), independent of #500/#501
Test plan
cargo check --workspacepassescargo test -p khive-pack-memory -p khive-pack-brain -p khive-pack-knowledgepasses🤖 Generated with Claude Code