feat(embeddings): hybrid fusion strategies (RRF, weighted, normalized)#41
Merged
plind-junior merged 18 commits intoMay 21, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements embedding-first semantic search across the knowledge base by adding SQLite-backed vector persistence, query caching, result-list fusion, ingest-time embedding hooks, and multi-backend search routing with optional hybrid mode combining semantic and keyword search. ChangesSemantic Search Implementation
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Store as KBStore
participant IndexDB as index_db
participant Cache as cache
participant Embedder as Embedder
App->>Store: put_claim(text)
Store->>Store: _embed_and_store(kind, id, text)
Store->>IndexDB: search_semantic requires embeddings
Note over Store: Compute content_hash, resolve embedder
Store->>Embedder: encode(text)
Embedder-->>Store: embedding vector
Store->>Cache: lookup_query_vec(text)
Cache-->>Store: None (cache miss)
Store->>IndexDB: put_embedding(vector, hash, model)
IndexDB->>IndexDB: Store in embedding_index
IndexDB->>IndexDB: Update embedding metadata
sequenceDiagram
participant Client as Client
participant Server as server.kb_search
participant IndexDB as index_db
participant Fusion as fusion
Client->>Server: kb_search(query, backend="hybrid")
Server->>IndexDB: search_semantic(query)
IndexDB->>IndexDB: Cache or encode query
IndexDB-->>Server: semantic results
Server->>IndexDB: search(query) [FTS5]
IndexDB-->>Server: fts5 results
Server->>Fusion: rrf_fuse(semantic, fts5)
Fusion->>Fusion: Accumulate reciprocal ranks
Fusion->>Fusion: Deduplicate by key
Fusion-->>Server: fused results
Server-->>Client: {backend: "hybrid", hits: [...]}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/vouch/index_db.py (1)
89-94:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
reset()should also clear embedding tables.
reset()claims to drop everything, but it currently leavesembedding_index,query_embedding_cache,embedding_dupes, and embedding meta intact. That can return stale semantic hits after a rebuild.Proposed fix
def reset(kb_dir: Path) -> None: """Drop everything; the rebuild caller re-populates.""" with open_db(kb_dir) as conn: conn.executescript( - "DELETE FROM claims_fts; DELETE FROM pages_fts; DELETE FROM entities_fts;" + "DELETE FROM claims_fts; " + "DELETE FROM pages_fts; " + "DELETE FROM entities_fts; " + "DELETE FROM embedding_index; " + "DELETE FROM query_embedding_cache; " + "DELETE FROM embedding_dupes; " + "DELETE FROM index_meta WHERE key LIKE 'embedding_%';" )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/index_db.py` around lines 89 - 94, The reset function (reset) currently deletes only FT tables and must also clear all embedding-related tables to avoid stale semantic hits; update the block that opens the DB via open_db and the conn.executescript call to also DELETE FROM embedding_index, query_embedding_cache, embedding_dupes and the embedding metadata table (e.g., embedding_meta or whatever the repo uses) in the same script so the rebuild starts from a clean embedding state; keep the changes inside the same transaction via conn.executescript and ensure the SQL uses DELETE FROM <table>;.
🧹 Nitpick comments (5)
src/vouch/server.py (1)
77-134: 🏗️ Heavy liftConsider extracting shared search-routing logic used by MCP and JSONL servers.
The backend-selection/fusion code is duplicated and already diverged across transports. Moving this into one internal helper would prevent future contract drift.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/server.py` around lines 77 - 134, Extract the backend-selection and fusion logic from kb_search into a shared internal helper (e.g., _route_kb_search or kb_search_impl) that takes (query, limit, backend, min_score, store, index_db) and returns the same dict shape used by kb_search (_to_dicts-like output); the helper should call index_db.search_semantic, index_db.search, store.search_substring, and embeddings.fusion.rrf_fuse as currently done, handle exceptions and backend-specific empty-return behavior, and raise on unknown backend—then have kb_search call that helper (so the JSONL server can reuse it) to remove the duplicated routing code.tests/embeddings/test_search.py (1)
98-137: ⚡ Quick winAdd direct integration coverage for
backend="hybrid"andmin_scorebehavior.Current tests validate default routing, but they don’t assert the hybrid execution path or threshold propagation, which are central to this change set.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/embeddings/test_search.py` around lines 98 - 137, Add tests that explicitly exercise the hybrid path and min_score propagation: create a new test (or extend existing tests) that uses jsonl_server.handle_request (and/or server.kb_search) with params including "backend":"hybrid" and a "min_score" threshold, seed two claims (one high-sim, one low-sim) via store.put_claim, call health.rebuild_index(store), then assert the response backend is "hybrid" and that results respect min_score (i.e., low-scoring claim is excluded when min_score is high and included when min_score is low). Use the existing symbols jsonl_server.handle_request, server.kb_search, health.rebuild_index, store.put_claim and index_db.search_semantic to locate where to add the assertions.docs/superpowers/specs/2026-05-20-semantic-search-design.md (1)
33-47: 💤 Low valueAdd language identifier to the fenced code block.
The directory tree structure should specify a language identifier (e.g.,
textor leave it asplaintext) to satisfy markdown linting rules.📝 Proposed fix
-``` +```text src/vouch/embeddings/🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-05-20-semantic-search-design.md` around lines 33 - 47, The fenced code block showing the embeddings directory tree is missing a language identifier; update the opening fence (the triple backticks before "src/vouch/embeddings/") to include a language label such as "text" or "plaintext" (e.g., ```text) so the markdown linter accepts the block while keeping the directory listing unchanged.tests/embeddings/test_integration.py (2)
37-43: 💤 Low valueConsider adding norm check for consistency.
The
test_st_mpnet_loads_and_encodestest verifies that embeddings are normalized to unit length (line 21), but this test doesn't include the same check. SinceSTMinilmEmbeddernormalizes embeddings in its implementation, adding a norm assertion would improve test consistency.✅ Optional: Add norm verification
def test_st_minilm_loads_and_encodes() -> None: from vouch.embeddings.st_minilm import STMinilmEmbedder e = STMinilmEmbedder() vec = e.encode("hello world") assert vec.shape == (384,) assert vec.dtype == np.float32 + assert abs(float(np.linalg.norm(vec)) - 1.0) < 1e-3🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/embeddings/test_integration.py` around lines 37 - 43, The test test_st_minilm_loads_and_encodes currently checks shape and dtype but omits verifying that embeddings are unit-normalized; update the test to compute the vector norm from STMinilmEmbedder().encode (call e.encode("hello world")) and assert the norm is approximately 1.0 (use np.linalg.norm and np.isclose or an equivalent assertion) to match the normalization check in test_st_mpnet_loads_and_encodes.
46-53: 💤 Low valueConsider adding norm check for consistency.
Similar to the other integration tests, adding a norm assertion would verify that
FastembedBgeEmbedderproduces unit-length embeddings as expected.✅ Optional: Add norm verification
def test_fastembed_bge_loads_and_encodes() -> None: pytest.importorskip("fastembed") from vouch.embeddings.fastembed_bge import FastembedBgeEmbedder e = FastembedBgeEmbedder() vec = e.encode("hello world") assert vec.shape == (384,) assert vec.dtype == np.float32 + assert abs(float(np.linalg.norm(vec)) - 1.0) < 1e-3🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/embeddings/test_integration.py` around lines 46 - 53, Update the integration test test_fastembed_bge_loads_and_encodes to also verify embedding normalization: after creating FastembedBgeEmbedder and calling e.encode("hello world") (the vec variable), compute the L2 norm of vec and assert it is close to 1.0 (use an appropriate tolerance) to ensure FastembedBgeEmbedder produces unit-length embeddings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vouch/embeddings/cache.py`:
- Line 10: The module-level unconditional "import numpy as np" in
src/vouch/embeddings/cache.py causes mypy to fail when numpy isn't installed;
move the numpy import into the local scope where it's actually used (e.g.,
inside any functions/methods that call numpy in this module) or wrap it with a
TYPE_CHECKING guard so only type-checking imports occur (use
typing.TYPE_CHECKING and import numpy under that guard), and update usages
accordingly (refer to the module-level import and any functions in cache.py that
call numpy to locate where to move/guard the import).
In `@src/vouch/embeddings/fastembed_bge.py`:
- Line 11: CI mypy fails because src/vouch/embeddings/fastembed_bge.py imports
numpy unconditionally but the CI only installs [dev] deps; either have the CI
install the optional [embeddings-fast] extras before running mypy (update the
workflow step that installs dependencies to include extras) or add mypy config
to ignore missing imports (e.g., add an appropriate [mypy] section in
pyproject.toml with ignore_missing_imports = True or per-module settings for the
fastembed_bge module) so imports like the numpy import in fastembed_bge.py do
not cause type-check failures.
In `@src/vouch/index_db.py`:
- Around line 340-344: lookup_query_vec currently returns cached vectors keyed
only by query text which can be stale if the embedder model or embedding
dimensionality changes; update the logic around lookup_query_vec,
cache_query_vec and the embedder usage so that you validate the cached vector
against the current embedder (e.g., check an associated model identifier and
length/dimension) and treat it as invalid if it doesn't match. Specifically,
when calling lookup_query_vec(kb_dir, query=query) verify the returned vec has
the expected dimension (and/or stored model id) matching embedder.encode; if it
fails the check, call embedder.encode(query) to produce a fresh vector and then
cache_query_vec(kb_dir, query=query, vec=qvec, metadata={model_id, dim}) before
passing qvec into search_embedding.
In `@src/vouch/jsonl_server.py`:
- Around line 79-83: The code reads backend_arg and currently falls through to
returning an empty hits list for invalid values; update the validation around
backend_arg (the code that assigns backend_arg and sets used) to explicitly
check against the allowed backend options (e.g., include "auto" and the known
backend names used elsewhere) and reject unknown values instead of silently
returning hits: when backend_arg is not in the allowed set, return a clear error
(HTTP 400 / raise ValueError) indicating the invalid backend instead of
proceeding, ensuring consumers see consistent MCP-like behavior; adjust the code
that currently initializes hits and used to only proceed after this validation.
- Around line 99-103: In the hybrid branch (where backend_arg == "hybrid") apply
the same min_score filtering and FTS error handling as other backends: call
index_db.search_semantic(...) into emb and index_db.search(...) into fts inside
a try/except so any exception from index_db.search is caught and handled (e.g.,
log the error and set fts = [] to avoid failing the request), then filter both
emb and fts by the min_score parameter before passing them to rrf_fuse(emb, fts,
limit=limit). Ensure you reference the existing symbols:
index_db.search_semantic, index_db.search, rrf_fuse, emb, fts, hits, min_score,
q, and limit when making these changes.
In `@src/vouch/server.py`:
- Around line 126-131: In the hybrid branch (the block handling backend ==
"hybrid"), make the FTS call resilient and apply the semantic min_score: wrap
the index_db.search(store.kb_dir, query, limit=limit * 2) call in a try/except
and fall back to an empty list if it raises, then call
index_db.search_semantic(...) as before but filter its results using the
provided min_score threshold before passing them into rrf_fuse; after rrf_fuse,
also filter the fused hits by score >= min_score (or the same score key your
semantic results use) and then return _to_dicts(filtered_hits, "hybrid"). Ensure
you reference rrf_fuse, index_db.search_semantic, index_db.search, and _to_dicts
when making the changes.
In `@src/vouch/storage.py`:
- Around line 483-505: The embedding/indexing hook can still raise after the
caller persisted the file; wrap the whole block that calls
_index_db.get_embedding, get_embedder(), embedder.encode(...), the with
_index_db.open_db(...) / _index_db.put_embedding(...),
_index_db.set_embedding_meta(...), and the dedup check_and_log(...) in a single
try/except Exception as e so any DB/encode/dedup errors are caught and logged
rather than propagated; log the exception with context (e.g., which kb_dir,
kind, id and that embedding/indexing failed) and then return silently so the
write path never aborts.
- Around line 502-503: The mypy failure is caused by importing an untyped local
module (.embeddings.dedup) in storage.py; to fix quickly, suppress the mypy
import error by appending a type ignore to the local import line (from
.embeddings.dedup import check_and_log # type: ignore[import-untyped]) so CI
passes, or alternatively make the dedup module a typed package (add py.typed and
type annotations) if you prefer a permanent fix; target the import of
check_and_log in storage.py for the change.
---
Outside diff comments:
In `@src/vouch/index_db.py`:
- Around line 89-94: The reset function (reset) currently deletes only FT tables
and must also clear all embedding-related tables to avoid stale semantic hits;
update the block that opens the DB via open_db and the conn.executescript call
to also DELETE FROM embedding_index, query_embedding_cache, embedding_dupes and
the embedding metadata table (e.g., embedding_meta or whatever the repo uses) in
the same script so the rebuild starts from a clean embedding state; keep the
changes inside the same transaction via conn.executescript and ensure the SQL
uses DELETE FROM <table>;.
---
Nitpick comments:
In `@docs/superpowers/specs/2026-05-20-semantic-search-design.md`:
- Around line 33-47: The fenced code block showing the embeddings directory tree
is missing a language identifier; update the opening fence (the triple backticks
before "src/vouch/embeddings/") to include a language label such as "text" or
"plaintext" (e.g., ```text) so the markdown linter accepts the block while
keeping the directory listing unchanged.
In `@src/vouch/server.py`:
- Around line 77-134: Extract the backend-selection and fusion logic from
kb_search into a shared internal helper (e.g., _route_kb_search or
kb_search_impl) that takes (query, limit, backend, min_score, store, index_db)
and returns the same dict shape used by kb_search (_to_dicts-like output); the
helper should call index_db.search_semantic, index_db.search,
store.search_substring, and embeddings.fusion.rrf_fuse as currently done, handle
exceptions and backend-specific empty-return behavior, and raise on unknown
backend—then have kb_search call that helper (so the JSONL server can reuse it)
to remove the duplicated routing code.
In `@tests/embeddings/test_integration.py`:
- Around line 37-43: The test test_st_minilm_loads_and_encodes currently checks
shape and dtype but omits verifying that embeddings are unit-normalized; update
the test to compute the vector norm from STMinilmEmbedder().encode (call
e.encode("hello world")) and assert the norm is approximately 1.0 (use
np.linalg.norm and np.isclose or an equivalent assertion) to match the
normalization check in test_st_mpnet_loads_and_encodes.
- Around line 46-53: Update the integration test
test_fastembed_bge_loads_and_encodes to also verify embedding normalization:
after creating FastembedBgeEmbedder and calling e.encode("hello world") (the vec
variable), compute the L2 norm of vec and assert it is close to 1.0 (use an
appropriate tolerance) to ensure FastembedBgeEmbedder produces unit-length
embeddings.
In `@tests/embeddings/test_search.py`:
- Around line 98-137: Add tests that explicitly exercise the hybrid path and
min_score propagation: create a new test (or extend existing tests) that uses
jsonl_server.handle_request (and/or server.kb_search) with params including
"backend":"hybrid" and a "min_score" threshold, seed two claims (one high-sim,
one low-sim) via store.put_claim, call health.rebuild_index(store), then assert
the response backend is "hybrid" and that results respect min_score (i.e.,
low-scoring claim is excluded when min_score is high and included when min_score
is low). Use the existing symbols jsonl_server.handle_request, server.kb_search,
health.rebuild_index, store.put_claim and index_db.search_semantic to locate
where to add the assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ae6a6c73-d807-4fbe-a4c7-575cf396576b
📒 Files selected for processing (21)
docs/superpowers/plans/2026-05-20-semantic-search.mddocs/superpowers/specs/2026-05-20-semantic-search-design.mdpyproject.tomlsrc/vouch/embeddings/__init__.pysrc/vouch/embeddings/base.pysrc/vouch/embeddings/cache.pysrc/vouch/embeddings/fastembed_bge.pysrc/vouch/embeddings/fusion.pysrc/vouch/embeddings/st_minilm.pysrc/vouch/embeddings/st_mpnet.pysrc/vouch/index_db.pysrc/vouch/jsonl_server.pysrc/vouch/server.pysrc/vouch/storage.pytests/embeddings/__init__.pytests/embeddings/_fakes.pytests/embeddings/test_core.pytests/embeddings/test_fusion.pytests/embeddings/test_integration.pytests/embeddings/test_search.pytests/embeddings/test_storage.py
dripsmvcp
force-pushed
the
feat/semantic-search-phase-5-fusion
branch
from
May 20, 2026 05:31
ec626ee to
e0b1e96
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/embeddings/test_search.py`:
- Line 9: The test module imports MockEmbedder (from tests.embeddings._fakes
import MockEmbedder) which triggers a ModuleNotFoundError at collection when
numpy is not installed; guard the import by calling pytest.importorskip("numpy")
at the top of tests/embeddings/test_search.py before importing MockEmbedder (or
replace the direct import with pytest.importorskip("numpy") followed by the
import) so the entire test module is skipped if numpy is unavailable.
In `@tests/embeddings/test_storage.py`:
- Around line 7-8: Replace the unconditional NumPy import in
tests/embeddings/test_storage.py with a pytest.importorskip call so tests are
skipped at collection when NumPy isn't installed; locate the top-level "import
numpy as np" and change it to call pytest.importorskip("numpy") (and assign to a
local name like np if needed) so the module uses pytest.importorskip to detect
and skip the tests gracefully.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e504035-8e0c-4a5a-b596-aaae31bd36c1
📒 Files selected for processing (11)
pyproject.tomlsrc/vouch/embeddings/__init__.pysrc/vouch/embeddings/cache.pysrc/vouch/embeddings/fusion.pysrc/vouch/index_db.pysrc/vouch/jsonl_server.pysrc/vouch/server.pysrc/vouch/storage.pytests/embeddings/test_fusion.pytests/embeddings/test_search.pytests/embeddings/test_storage.py
|
|
||
| import pytest | ||
|
|
||
| from tests.embeddings._fakes import MockEmbedder |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Check test module import order:"
sed -n '1,30p' tests/embeddings/test_search.py
echo "Confirm numpy dependency in fake embedder:"
sed -n '1,80p' tests/embeddings/_fakes.py
echo "Inspect dependency declarations for numpy:"
fd -i 'pyproject.toml' -x rg -n 'numpy|optional-dependencies|dependencies|pytest' {}Repository: vouchdev/vouch
Length of output: 2439
Guard test module import when numpy is unavailable.
MockEmbedder imports numpy at the module level in _fakes.py. When this test imports MockEmbedder at line 9, pytest collection fails with ModuleNotFoundError if numpy is not installed, preventing the entire test suite from loading. Gate the import with pytest.importorskip() to skip the test module instead of crashing collection.
💡 Suggested patch
import pytest
-from tests.embeddings._fakes import MockEmbedder
+pytest.importorskip("numpy")
+from tests.embeddings._fakes import MockEmbedder
from vouch import index_db📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| from tests.embeddings._fakes import MockEmbedder | |
| import pytest | |
| pytest.importorskip("numpy") | |
| from tests.embeddings._fakes import MockEmbedder | |
| from vouch import index_db |
🧰 Tools
🪛 GitHub Actions: ci / 1_test (py3.11).txt
[error] 9-9: pytest collection failed due to import error while importing MockEmbedder from tests.embeddings._fakes: ModuleNotFoundError: No module named 'numpy'.
🪛 GitHub Actions: ci / 2_test (py3.13).txt
[error] 9-9: Pytest collection failed due to missing dependency in imported test utilities: ModuleNotFoundError: No module named 'numpy'
🪛 GitHub Actions: ci / 3_test (py3.12).txt
[error] 9-9: pytest collection failed due to missing dependency: ModuleNotFoundError: No module named 'numpy' (indirectly from tests/embeddings/_fakes.py).
🪛 GitHub Actions: ci / test (py3.11)
[error] 9-9: pytest collection failed because referenced fakes import fails with ImportError: No module named 'numpy'.
🪛 GitHub Actions: ci / test (py3.12)
[error] 9-9: pytest import error during test collection due to missing dependency: ModuleNotFoundError: No module named 'numpy'.
🪛 GitHub Actions: ci / test (py3.13)
[error] 9-9: pytest collection failed because tests/embeddings/_fakes.py imports numpy, but ModuleNotFoundError: No module named 'numpy'.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/embeddings/test_search.py` at line 9, The test module imports
MockEmbedder (from tests.embeddings._fakes import MockEmbedder) which triggers a
ModuleNotFoundError at collection when numpy is not installed; guard the import
by calling pytest.importorskip("numpy") at the top of
tests/embeddings/test_search.py before importing MockEmbedder (or replace the
direct import with pytest.importorskip("numpy") followed by the import) so the
entire test module is skipped if numpy is unavailable.
Comment on lines
+7
to
+8
| import numpy as np | ||
| import pytest |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read it
cat -n tests/embeddings/test_storage.pyRepository: vouchdev/vouch
Length of output: 7258
🏁 Script executed:
# Look for CI configuration files
git ls-files | grep -E '\.(yml|yaml|toml|cfg|ini)$' | head -20Repository: vouchdev/vouch
Length of output: 1014
🏁 Script executed:
# Check pyproject.toml or setup.py for optional dependencies
fd -e toml -e py | grep -E '(pyproject|setup)' | head -5Repository: vouchdev/vouch
Length of output: 73
🏁 Script executed:
cat -n pyproject.toml | head -100Repository: vouchdev/vouch
Length of output: 3397
🏁 Script executed:
cat -n .github/workflows/ci.ymlRepository: vouchdev/vouch
Length of output: 1859
Use pytest.importorskip to skip tests gracefully when NumPy isn't installed.
Line 7 imports NumPy unconditionally, causing test collection to fail when the [embeddings] extra is not installed. The default CI job installs only the [dev] extra (as documented in pyproject.toml lines 79–84), so NumPy is unavailable during collection.
Suggested fix
-import numpy as np
import pytest
+np = pytest.importorskip("numpy")🧰 Tools
🪛 GitHub Actions: ci / 1_test (py3.11).txt
[error] 7-7: pytest collection failed due to import error: ModuleNotFoundError: No module named 'numpy' (import numpy as np).
🪛 GitHub Actions: ci / 2_test (py3.13).txt
[error] 7-7: Pytest collection failed due to missing dependency: ModuleNotFoundError: No module named 'numpy'
🪛 GitHub Actions: ci / 3_test (py3.12).txt
[error] 7-7: pytest collection failed: ModuleNotFoundError: No module named 'numpy' (import numpy as np).
🪛 GitHub Actions: ci / test (py3.11)
[error] 7-7: pytest collection failed due to ImportError: No module named 'numpy' while importing the test module.
🪛 GitHub Actions: ci / test (py3.12)
[error] 7-7: pytest import error during test collection: ModuleNotFoundError: No module named 'numpy'.
🪛 GitHub Actions: ci / test (py3.13)
[error] 7-7: pytest collection failed due to missing dependency: ModuleNotFoundError: No module named 'numpy' (import numpy as np).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/embeddings/test_storage.py` around lines 7 - 8, Replace the
unconditional NumPy import in tests/embeddings/test_storage.py with a
pytest.importorskip call so tests are skipped at collection when NumPy isn't
installed; locate the top-level "import numpy as np" and change it to call
pytest.importorskip("numpy") (and assign to a local name like np if needed) so
the module uses pytest.importorskip to detect and skip the tests gracefully.
4 tasks
Member
|
LGTM with tiny nits. Fix CI and check comments from codrabbits. I think thoes are all valid |
dripsmvcp
force-pushed
the
feat/semantic-search-phase-5-fusion
branch
from
May 20, 2026 13:04
e0b1e96 to
56bd11b
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
tests/embeddings/test_search.py (1)
9-9:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winGuard the MockEmbedder import to prevent pytest collection failures.
This issue was flagged in a previous review and is blocking CI across all Python versions. The
MockEmbedderimport triggers aModuleNotFoundErrorfor numpy during pytest collection when the optional embedding dependencies are not installed, preventing the entire test suite from loading.🔧 Apply the guard immediately to unblock CI
import pytest +pytest.importorskip("numpy") from tests.embeddings._fakes import MockEmbedder from vouch import index_db🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/embeddings/test_search.py` at line 9, Guard the problematic import of MockEmbedder from tests.embeddings._fakes to avoid ModuleNotFoundError during collection: wrap the import in a safe guard (use pytest.importorskip for the optional dependency like "numpy" or a try/except ImportError around the from tests.embeddings._fakes import MockEmbedder and set a fallback/skip behavior) so tests/embeddings/test_search.py does not fail to import when embedding extras are not installed; locate the import statement referencing MockEmbedder in that file and replace it with the guarded import pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vouch/index_db.py`:
- Around line 40-67: The reset() routine currently only removes the three FTS
tables and leaves the new embedding tables intact, causing stale
embedding_index, query_embedding_cache, index_meta and embedding_dupes rows to
persist; update reset() to also clear those derived tables by running the
appropriate DELETE/DELETE FROM or DROP/CREATE statements for embedding_index,
query_embedding_cache, embedding_dupes and index_meta (and any associated
indexes) so reindexing starts from a clean slate, ensuring you reference the
reset() function and the table names embedding_index, query_embedding_cache,
embedding_dupes, and index_meta when making the change.
- Around line 311-314: The loop computing scores uses a raw dot product (score =
float(q @ vec)) which biases by magnitude; change it to compute cosine
similarity by dividing the dot product by the product of vector norms (i.e.,
score = (q · vec) / (||q|| * ||vec||)). Ensure you handle zero-length vectors
safely (skip or treat norm zero as yielding score below min_score). Update the
code where _blob_to_vec is used and the loop that iterates over rows
(referencing variables kind, id_, blob, dim, vec, q, min_score) so the fallback
path matches sqlite-vec cosine rankings.
In `@src/vouch/storage.py`:
- Around line 482-489: The code currently skips re-embedding when
content_hash(text) equals existing[1], which allows embeddings from a different
model to persist; change the check to also verify the active embedder identity
before returning: call get_embedder() (or its identifier like
embedder.name/model_id) and compare it to the embedder id stored in the existing
record returned by _index_db.get_embedding (augment _index_db.get_embedding to
include stored_embedder if it doesn't already). Only return early if both the
content hash and the stored embedder id match the current embedder; otherwise
proceed to re-embed and overwrite the stored embedding and embedder id.
---
Duplicate comments:
In `@tests/embeddings/test_search.py`:
- Line 9: Guard the problematic import of MockEmbedder from
tests.embeddings._fakes to avoid ModuleNotFoundError during collection: wrap the
import in a safe guard (use pytest.importorskip for the optional dependency like
"numpy" or a try/except ImportError around the from tests.embeddings._fakes
import MockEmbedder and set a fallback/skip behavior) so
tests/embeddings/test_search.py does not fail to import when embedding extras
are not installed; locate the import statement referencing MockEmbedder in that
file and replace it with the guarded import pattern.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 00e7dbdb-accc-4e6c-ad70-0bda5402746c
📒 Files selected for processing (11)
src/vouch/embeddings/__init__.pysrc/vouch/embeddings/cache.pysrc/vouch/embeddings/fusion.pysrc/vouch/index_db.pysrc/vouch/jsonl_server.pysrc/vouch/server.pysrc/vouch/storage.pytests/embeddings/conftest.pytests/embeddings/test_fusion.pytests/embeddings/test_search.pytests/embeddings/test_storage.py
✅ Files skipped from review due to trivial changes (2)
- tests/embeddings/conftest.py
- src/vouch/embeddings/init.py
dripsmvcp
force-pushed
the
feat/semantic-search-phase-5-fusion
branch
from
May 20, 2026 13:11
56bd11b to
6543417
Compare
dripsmvcp
force-pushed
the
feat/semantic-search-phase-5-fusion
branch
from
May 20, 2026 13:16
6543417 to
4ed27e6
Compare
…embedding Per CodeRabbit Critical on PR vouchdev#39/40/43: SQLite does not allow a column alias defined in the SELECT list to be referenced in the WHERE clause of the same query. The old form SELECT ... 1.0 - vec_distance_cosine(vec, ?) AS score ... WHERE ... AND score >= ? raised `no such column: score` at runtime; the catch-all `except sqlite3.OperationalError` silently swallowed it and made every call fall through to the NumPy brute-force path, which works but defeats the whole point of installing sqlite-vec. Wrap the projection in a subquery so the WHERE clause sees the alias. No behavior change for callers that have already been using the fallback (NumPy still produces correct results); the ANN code path now actually runs when sqlite-vec is loaded.
The KBStore._embed_and_store helper lazily imports vouch.embeddings.dedup (added in Phase 6). On Phase 3's branch that module does not yet exist, so mypy treats it as untyped and errors out. Mark the import with `# type: ignore[import-not-found,import-untyped, unused-ignore]` so the type check passes here and stays silent once Phase 6 lands.
The hybrid backend branch in kb_search (server.py) and _h_search (jsonl_server.py) lazily imports vouch.embeddings.fusion (added in Phase 5). On Phase 4's branch that module doesn't exist yet, so mypy emits import-untyped. Mark each import with `# type: ignore` and let ruff auto-format the line.
dripsmvcp
force-pushed
the
feat/semantic-search-phase-5-fusion
branch
from
May 20, 2026 13:44
4ed27e6 to
379297a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (11)
src/vouch/storage.py (2)
482-509:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
_embed_and_store()truly best-effort.The docstring says embedding failures are swallowed, but
get_embedding(),encode(), the DB writes,set_embedding_meta(), andcheck_and_log()can still raise after the artifact file has already been persisted. That breaks the “embeddings are an enhancement” contract and makes create/update calls report failure after a successful durable write. Wrap the remaining embedding/indexing work in a broadtry/except Exception, log the failure withkind/id, and return silently here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/storage.py` around lines 482 - 509, _wrap the remainder of _embed_and_store after obtaining h and ensuring the artifact file exists in a broad try/except Exception block so any error from get_embedder, embedder.encode, _index_db.put_embedding, _index_db.set_embedding_meta, or check_and_log does not propagate; on exception log a concise error including kind and id and then return silently, preserving the already-persisted artifact and keeping embedding/indexing strictly best-effort (refer to symbols: _embed_and_store, get_embedder, embedder.encode, _index_db.put_embedding, _index_db.set_embedding_meta, check_and_log).
482-488:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't skip re-embedding on hash alone.
The early return only checks
content_hash, so switching the active embedder can leave an old vector on disk for the same text. That mixes document embeddings from one model with queries from another and will skew semantic ranking. Compare the stored model identity toget_embedder()before returning early.Suggested fix
- h = content_hash(text) - existing = _index_db.get_embedding(self.kb_dir, kind=kind, id=id) - if existing is not None and existing[1] == h: - return try: embedder = get_embedder() except KeyError: return + h = content_hash(text) + existing = _index_db.get_embedding(self.kb_dir, kind=kind, id=id) + if ( + existing is not None + and existing[1] == h + and existing[2] == embedder.name + ): + return🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/storage.py` around lines 482 - 488, The early-return after computing h currently only checks content_hash(text) and can skip re-embedding when the active embedder changed; modify the logic in the block that fetches existing = _index_db.get_embedding(self.kb_dir, kind=kind, id=id) so that you also obtain the current embedder identity from get_embedder() (e.g., a model_name or id property) and compare it to the stored model identity in existing (the record returned by get_embedding); only return early if both the stored content hash equals h and the stored model identity matches the current embedder; otherwise fall through to re-embed and update the index.src/vouch/jsonl_server.py (2)
99-105:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply the same hybrid safeguards as the other backends.
The hybrid branch ignores
min_scoreon the semantic leg and letsindex_db.search()exceptions abort the request. That makeshybridless reliable thanauto/fts5for the same KB state.Suggested fix
- emb = index_db.search_semantic(s.kb_dir, q, limit=limit * 2) - fts = index_db.search(s.kb_dir, q, limit=limit * 2) + emb = index_db.search_semantic( + s.kb_dir, q, limit=limit * 2, min_score=min_score, + ) + try: + fts = index_db.search(s.kb_dir, q, limit=limit * 2) + except Exception: + fts = [] hits = rrf_fuse(emb, fts, limit=limit)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/jsonl_server.py` around lines 99 - 105, The hybrid branch (backend_arg == "hybrid") currently calls index_db.search_semantic and index_db.search without the same safeguards as other backends: it must apply the min_score cutoff to the semantic results and guard index_db.search with the same try/except fallback logic. Modify the hybrid block to (1) call index_db.search_semantic(..., limit=limit*2) and filter out semantic hits below min_score before passing to rrf_fuse, and (2) wrap index_db.search(...) in the same try/except used elsewhere so exceptions don’t abort the request (fall back to an empty FTS result on error). Ensure you still pass the filtered semantic list and the safe FTS list into rrf_fuse(..., limit=limit).
79-83:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject unknown
backendvalues up front.Invalid backend names currently fall through to a successful empty response, which hides client bugs and diverges from
server.kb_search(). Validatebackend_argagainst the supported set and raiseValueErrorfor anything else.Suggested fix
backend_arg = p.get("backend", "auto") + valid_backends = {"auto", "embedding", "fts5", "substring", "hybrid"} + if backend_arg not in valid_backends: + raise ValueError(f"unknown backend: {backend_arg}") min_score = float(p.get("min_score", 0.0))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/jsonl_server.py` around lines 79 - 83, Reject unknown backend values by validating backend_arg against the canonical supported set used by server.kb_search(); if backend_arg not in that set (and not the special "auto"), raise a ValueError. Update the code around the backend_arg/used assignment to fetch or reference the same supported-backends collection used by server.kb_search() (or define a small SUPPORTED_BACKENDS set if one doesn’t exist), then perform an if backend_arg not in SUPPORTED_BACKENDS: raise ValueError(f"unknown backend: {backend_arg}") before continuing and setting used.src/vouch/server.py (1)
126-133:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMirror
min_scoreand FTS fallback inhybrid.The explicit hybrid branch bypasses the semantic threshold and can still fail on
index_db.search()exceptions, even though the other backend paths already treat FTS errors as empty hits. Apply the same guards here before fusing.Suggested fix
- emb = index_db.search_semantic(store.kb_dir, query, limit=limit * 2) - fts = index_db.search(store.kb_dir, query, limit=limit * 2) + emb = index_db.search_semantic( + store.kb_dir, query, limit=limit * 2, min_score=min_score, + ) + try: + fts = index_db.search(store.kb_dir, query, limit=limit * 2) + except Exception: + fts = [] hits = rrf_fuse(emb, fts, limit=limit)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/server.py` around lines 126 - 133, In the hybrid branch where you call index_db.search_semantic and index_db.search and then rrf_fuse, apply the same semantic min_score filtering and FTS fallback used by other backends: filter the semantic results from index_db.search_semantic by the existing min_score variable before fusion, and wrap index_db.search in a try/except so any exception yields an empty FTS hits list instead of bubbling up; then call rrf_fuse(emb_filtered, fts_or_empty, limit=limit) and return _to_dicts(hits, "hybrid"). Ensure you reference the same symbols used here: index_db.search_semantic, index_db.search, rrf_fuse, min_score, and _to_dicts.tests/embeddings/test_search.py (1)
7-10:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winSkip this module cleanly when NumPy isn't installed.
Importing
MockEmbedderat module load time makes pytest collection fail when the optionalnumpydependency is absent. Gate the import withpytest.importorskip("numpy")so CI skips this module instead of hard-failing collection.Suggested fix
import pytest +pytest.importorskip("numpy") from tests.embeddings._fakes import MockEmbedder🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/embeddings/test_search.py` around lines 7 - 10, The test module imports MockEmbedder at import time which causes collection to fail when numpy is not present; call pytest.importorskip("numpy") at the top of the module before importing MockEmbedder so pytest will skip the whole module cleanly, then import MockEmbedder from tests.embeddings._fakes (leave vouch.index_db import as-is); ensure the importorskip call precedes any use/import of MockEmbedder or other numpy-dependent test utilities.src/vouch/index_db.py (3)
344-348:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate cached query vector against current embedding metadata.
Lines 344-348 reuse cached vectors keyed only by query text; model/dimension drift can produce stale vectors and potential shape errors downstream.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/index_db.py` around lines 344 - 348, The cached query vector returned by lookup_query_vec(query) must be validated against the current embedder metadata before reuse: call the embedder's metadata (e.g., model name and embedding dimension from embedder or embedder.metadata) and verify the cached vector's stored model/dimension match; if they differ or the vector shape is incorrect, discard the cached vector, re-run embedder.encode(query) and call cache_query_vec to store the new vector, then proceed to search_embedding; update the logic around lookup_query_vec, embedder.encode, cache_query_vec, and search_embedding to perform this validation and recaching.
315-318:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPython fallback scoring is not cosine-consistent.
At Line 317, raw dot product can skew ranking by vector magnitude versus the sqlite-vec cosine path. Normalize consistently in fallback scoring.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/index_db.py` around lines 315 - 318, The fallback scoring currently uses raw dot product (score = float(q @ vec)) which is inconsistent with sqlite-vec's cosine scoring; change the fallback in the loop that iterates over rows to compute cosine similarity instead by normalizing both the query vector q and the document vector produced by _blob_to_vec (or divide the dot by (||q|| * ||vec||)), precompute ||q|| once, guard against zero norms (skip or set score to 0 if either norm is zero), and then compare that normalized cosine score against min_score so ranking matches the sqlite-vec path; update references to q, _blob_to_vec, vec, score, and min_score accordingly.
89-94:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
reset()doesn’t clear embedding-derived tables.Line 93 only clears FTS tables; embedding tables/meta/cache remain and can leak stale semantic state after rebuilds.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/index_db.py` around lines 89 - 94, The reset(kb_dir) function currently only deletes FTS tables (claims_fts, pages_fts, entities_fts) but leaves embedding-derived tables and meta/cache behind; update reset (the function using open_db) to also clear all embedding-related tables and metadata/cache (for example embeddings, embedding_meta, embedding_cache and any *_embeddings tables or other embedding index tables your schema uses) by executing DELETE (or DROP/CREATE as appropriate) against those tables in the same DB transaction, and optionally run VACUUM or equivalent cleanup after to remove leftover state.tests/embeddings/test_storage.py (1)
7-8:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
pytest.importorskip("numpy")for optional dependency safety.Line 7 performs an unconditional NumPy import; this can fail collection in environments that don’t install embeddings extras.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/embeddings/test_storage.py` around lines 7 - 8, The unconditional "import numpy as np" should be replaced with a conditional import using pytest.importorskip to avoid test collection failures when the optional numpy dependency isn't installed; update the top of tests/embeddings/test_storage.py by calling pytest.importorskip("numpy") (or assigning numpy = pytest.importorskip("numpy")) instead of the bare "import numpy as np" so the tests are skipped gracefully when numpy is unavailable.src/vouch/embeddings/cache.py (1)
10-10:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard NumPy import behind optional-dependency boundaries.
Line 10 still imports NumPy unconditionally; this can break CI/type-check/collection in environments without embeddings extras. Move NumPy imports into local call sites (or a
TYPE_CHECKINGpattern) so optional embeddings code remains import-safe.#!/bin/bash # Verify whether numpy is guaranteed in default/dev install paths and CI checks. set -euo pipefail fd -HI 'pyproject.toml|requirements.*|constraints.*|ci.yml|*.yaml|*.yml' rg -n "numpy|embeddings|dev|mypy|pytest" pyproject.toml .github/workflows/ci.yml rg -n "import numpy as np" src/vouch/embeddings/cache.py tests/embeddings/test_storage.py🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/embeddings/cache.py` at line 10, Top-level "import numpy as np" in src.vouch.embeddings.cache must be removed and guarded: delete the unconditional import and instead import numpy locally inside the functions/methods that actually use it (e.g., any functions that manipulate arrays or call np.* in this module); for type hints use "from typing import TYPE_CHECKING" and put "if TYPE_CHECKING: import numpy as np" so annotations remain valid without requiring numpy at import time. Ensure all references to np within the file still work by adding the local import at each call site or the start of the function that needs it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/specs/2026-05-20-semantic-search-design.md`:
- Around line 33-47: The fenced code block that lists files under
"src/vouch/embeddings/" is missing a language tag which triggers markdownlint
MD040; update the opening fence to include an explicit language (e.g., replace
``` with ```text or ```bash) so the block is recognized as plain text, and scan
the rest of the file for other ``` blocks (e.g., any README examples or config
snippets) and add appropriate language identifiers (text/sql/toml/bash) to each.
In `@src/vouch/embeddings/fusion.py`:
- Around line 22-27: The rrf_fuse function can divide by zero when k is
negative; add an upfront validation in rrf_fuse to ensure k is non-negative (or
more strictly that k >= 0) and raise a ValueError with a clear message if
invalid, so the loop that uses 1.0 / (k + rank) never encounters (k + rank) ==
0; update any docstring or error text accordingly and keep the rest of the logic
(including use of _key and limit) unchanged.
---
Duplicate comments:
In `@src/vouch/embeddings/cache.py`:
- Line 10: Top-level "import numpy as np" in src.vouch.embeddings.cache must be
removed and guarded: delete the unconditional import and instead import numpy
locally inside the functions/methods that actually use it (e.g., any functions
that manipulate arrays or call np.* in this module); for type hints use "from
typing import TYPE_CHECKING" and put "if TYPE_CHECKING: import numpy as np" so
annotations remain valid without requiring numpy at import time. Ensure all
references to np within the file still work by adding the local import at each
call site or the start of the function that needs it.
In `@src/vouch/index_db.py`:
- Around line 344-348: The cached query vector returned by
lookup_query_vec(query) must be validated against the current embedder metadata
before reuse: call the embedder's metadata (e.g., model name and embedding
dimension from embedder or embedder.metadata) and verify the cached vector's
stored model/dimension match; if they differ or the vector shape is incorrect,
discard the cached vector, re-run embedder.encode(query) and call
cache_query_vec to store the new vector, then proceed to search_embedding;
update the logic around lookup_query_vec, embedder.encode, cache_query_vec, and
search_embedding to perform this validation and recaching.
- Around line 315-318: The fallback scoring currently uses raw dot product
(score = float(q @ vec)) which is inconsistent with sqlite-vec's cosine scoring;
change the fallback in the loop that iterates over rows to compute cosine
similarity instead by normalizing both the query vector q and the document
vector produced by _blob_to_vec (or divide the dot by (||q|| * ||vec||)),
precompute ||q|| once, guard against zero norms (skip or set score to 0 if
either norm is zero), and then compare that normalized cosine score against
min_score so ranking matches the sqlite-vec path; update references to q,
_blob_to_vec, vec, score, and min_score accordingly.
- Around line 89-94: The reset(kb_dir) function currently only deletes FTS
tables (claims_fts, pages_fts, entities_fts) but leaves embedding-derived tables
and meta/cache behind; update reset (the function using open_db) to also clear
all embedding-related tables and metadata/cache (for example embeddings,
embedding_meta, embedding_cache and any *_embeddings tables or other embedding
index tables your schema uses) by executing DELETE (or DROP/CREATE as
appropriate) against those tables in the same DB transaction, and optionally run
VACUUM or equivalent cleanup after to remove leftover state.
In `@src/vouch/jsonl_server.py`:
- Around line 99-105: The hybrid branch (backend_arg == "hybrid") currently
calls index_db.search_semantic and index_db.search without the same safeguards
as other backends: it must apply the min_score cutoff to the semantic results
and guard index_db.search with the same try/except fallback logic. Modify the
hybrid block to (1) call index_db.search_semantic(..., limit=limit*2) and filter
out semantic hits below min_score before passing to rrf_fuse, and (2) wrap
index_db.search(...) in the same try/except used elsewhere so exceptions don’t
abort the request (fall back to an empty FTS result on error). Ensure you still
pass the filtered semantic list and the safe FTS list into rrf_fuse(...,
limit=limit).
- Around line 79-83: Reject unknown backend values by validating backend_arg
against the canonical supported set used by server.kb_search(); if backend_arg
not in that set (and not the special "auto"), raise a ValueError. Update the
code around the backend_arg/used assignment to fetch or reference the same
supported-backends collection used by server.kb_search() (or define a small
SUPPORTED_BACKENDS set if one doesn’t exist), then perform an if backend_arg not
in SUPPORTED_BACKENDS: raise ValueError(f"unknown backend: {backend_arg}")
before continuing and setting used.
In `@src/vouch/server.py`:
- Around line 126-133: In the hybrid branch where you call
index_db.search_semantic and index_db.search and then rrf_fuse, apply the same
semantic min_score filtering and FTS fallback used by other backends: filter the
semantic results from index_db.search_semantic by the existing min_score
variable before fusion, and wrap index_db.search in a try/except so any
exception yields an empty FTS hits list instead of bubbling up; then call
rrf_fuse(emb_filtered, fts_or_empty, limit=limit) and return _to_dicts(hits,
"hybrid"). Ensure you reference the same symbols used here:
index_db.search_semantic, index_db.search, rrf_fuse, min_score, and _to_dicts.
In `@src/vouch/storage.py`:
- Around line 482-509: _wrap the remainder of _embed_and_store after obtaining h
and ensuring the artifact file exists in a broad try/except Exception block so
any error from get_embedder, embedder.encode, _index_db.put_embedding,
_index_db.set_embedding_meta, or check_and_log does not propagate; on exception
log a concise error including kind and id and then return silently, preserving
the already-persisted artifact and keeping embedding/indexing strictly
best-effort (refer to symbols: _embed_and_store, get_embedder, embedder.encode,
_index_db.put_embedding, _index_db.set_embedding_meta, check_and_log).
- Around line 482-488: The early-return after computing h currently only checks
content_hash(text) and can skip re-embedding when the active embedder changed;
modify the logic in the block that fetches existing =
_index_db.get_embedding(self.kb_dir, kind=kind, id=id) so that you also obtain
the current embedder identity from get_embedder() (e.g., a model_name or id
property) and compare it to the stored model identity in existing (the record
returned by get_embedding); only return early if both the stored content hash
equals h and the stored model identity matches the current embedder; otherwise
fall through to re-embed and update the index.
In `@tests/embeddings/test_search.py`:
- Around line 7-10: The test module imports MockEmbedder at import time which
causes collection to fail when numpy is not present; call
pytest.importorskip("numpy") at the top of the module before importing
MockEmbedder so pytest will skip the whole module cleanly, then import
MockEmbedder from tests.embeddings._fakes (leave vouch.index_db import as-is);
ensure the importorskip call precedes any use/import of MockEmbedder or other
numpy-dependent test utilities.
In `@tests/embeddings/test_storage.py`:
- Around line 7-8: The unconditional "import numpy as np" should be replaced
with a conditional import using pytest.importorskip to avoid test collection
failures when the optional numpy dependency isn't installed; update the top of
tests/embeddings/test_storage.py by calling pytest.importorskip("numpy") (or
assigning numpy = pytest.importorskip("numpy")) instead of the bare "import
numpy as np" so the tests are skipped gracefully when numpy is unavailable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: af8c2c97-c9a6-490e-9ca5-e8c1fbd52779
📒 Files selected for processing (23)
docs/superpowers/plans/2026-05-20-semantic-search.mddocs/superpowers/specs/2026-05-20-semantic-search-design.mdpyproject.tomlsrc/vouch/bundle.pysrc/vouch/embeddings/__init__.pysrc/vouch/embeddings/base.pysrc/vouch/embeddings/cache.pysrc/vouch/embeddings/fastembed_bge.pysrc/vouch/embeddings/fusion.pysrc/vouch/embeddings/st_minilm.pysrc/vouch/embeddings/st_mpnet.pysrc/vouch/index_db.pysrc/vouch/jsonl_server.pysrc/vouch/server.pysrc/vouch/storage.pytests/embeddings/__init__.pytests/embeddings/_fakes.pytests/embeddings/conftest.pytests/embeddings/test_core.pytests/embeddings/test_fusion.pytests/embeddings/test_integration.pytests/embeddings/test_search.pytests/embeddings/test_storage.py
✅ Files skipped from review due to trivial changes (1)
- tests/embeddings/init.py
Comment on lines
+33
to
+47
| ``` | ||
| src/vouch/embeddings/ | ||
| __init__.py # public API: encode, search, register | ||
| base.py # Embedder ABC + adapter registry | ||
| st_mpnet.py # default impl (sentence-transformers all-mpnet-base-v2) | ||
| st_minilm.py # alternative impl | ||
| fastembed_bge.py # alternative impl (no-torch path via fastembed) | ||
| cache.py # query embedding LRU + content-hash skip cache | ||
| rerank.py # cross-encoder reranker (ms-marco-MiniLM-L6-v2) | ||
| hyde.py # Hypothetical Document Embedding query expansion | ||
| dedup.py # cosine-threshold duplicate detection at ingest | ||
| fusion.py # RRF, weighted-sum, normalized-cosine fusion strategies | ||
| eval.py # recall@k / MRR / nDCG harness | ||
| migration.py # model-identity check + backfill orchestration | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks to satisfy markdownlint.
The fence starting at Line 33 is missing a language tag (MD040), which can fail docs linting in CI. Add explicit languages (e.g., text, sql, toml, bash) for each fenced block in this file.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-05-20-semantic-search-design.md` around lines 33
- 47, The fenced code block that lists files under "src/vouch/embeddings/" is
missing a language tag which triggers markdownlint MD040; update the opening
fence to include an explicit language (e.g., replace ``` with ```text or
```bash) so the block is recognized as plain text, and scan the rest of the file
for other ``` blocks (e.g., any README examples or config snippets) and add
appropriate language identifiers (text/sql/toml/bash) to each.
dripsmvcp
force-pushed
the
feat/semantic-search-phase-5-fusion
branch
from
May 21, 2026 05:34
379297a to
993e7da
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/vouch/embeddings/fusion.py (1)
22-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate
kto prevent division-by-zero in RRF.
k < 0can make(k + rank)equal zero (e.g.,k=-1, first item), causing a runtime crash.Proposed fix
def rrf_fuse(a: Hits, b: Hits, *, limit: int = 10, k: int = 60) -> Hits: """Reciprocal Rank Fusion: score = sum(1 / (k + rank)) across lists.""" + if k < 0: + raise ValueError("k must be >= 0") scores: dict[tuple[str, str], float] = {}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/embeddings/fusion.py` around lines 22 - 27, The rrf_fuse function can divide by zero when k is negative (1.0 / (k + rank)); add validation at the start of rrf_fuse to ensure k is non-negative (or at least k + 1 > 0) and raise a clear ValueError if invalid. Update the function that accepts parameter k (rrf_fuse) to validate k before the loop that uses rank and the expression 1.0/(k + rank) so the division can never receive zero or a negative denominator.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vouch/embeddings/fusion.py`:
- Around line 22-23: The rrf_fuse function currently slices results with a
negative limit which yields unexpected behavior; add a guard at the start of
rrf_fuse (checking the limit parameter) so that if limit <= 0 you return an
empty Hits object (or raise a ValueError if that API is preferred) instead of
performing slicing; apply the same guard to the other fusion functions in this
module (the other fuse implementations referenced around the other ranges) so
all functions consistently handle negative or zero limit values by returning no
results or failing fast.
In `@src/vouch/jsonl_server.py`:
- Line 100: The import of rrf_fuse in the import block is not formatted/sorted
per project lint rules (Ruff I001); update the import formatting and order to
match the module import group conventions and type-ignore comment style (for
example ensure "from .embeddings.fusion import rrf_fuse # type:
ignore[import-not-found,import-untyped]" is placed in the correct import section
and any unused-ignore is removed), or simply run "ruff check --fix" to auto-fix
the import formatting in src/vouch/jsonl_server.py so the rrf_fuse import
complies with the project's import styling rules.
In `@src/vouch/server.py`:
- Line 127: Import line "from .embeddings.fusion import rrf_fuse" in
src/vouch/server.py is unsorted causing Ruff I001; fix by sorting/formatting the
module imports (or running the auto-fixer) so import ordering complies with
ruff/isort rules, preserving the type-ignore comment (type:
ignore[import-not-found,import-untyped,unused-ignore]) and the symbol rrf_fuse;
run `ruff check --fix` (or `ruff format`/`isort`) and commit the updated import
block.
---
Duplicate comments:
In `@src/vouch/embeddings/fusion.py`:
- Around line 22-27: The rrf_fuse function can divide by zero when k is negative
(1.0 / (k + rank)); add validation at the start of rrf_fuse to ensure k is
non-negative (or at least k + 1 > 0) and raise a clear ValueError if invalid.
Update the function that accepts parameter k (rrf_fuse) to validate k before the
loop that uses rank and the expression 1.0/(k + rank) so the division can never
receive zero or a negative denominator.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8aba9b3f-8cd2-4e99-91d1-8c8fd6de8ad1
📒 Files selected for processing (4)
src/vouch/embeddings/fusion.pysrc/vouch/jsonl_server.pysrc/vouch/server.pytests/embeddings/test_fusion.py
dripsmvcp
force-pushed
the
feat/semantic-search-phase-5-fusion
branch
from
May 21, 2026 05:37
993e7da to
f6b8427
Compare
… hook Bundles the substantive correctness fixes flagged by CodeRabbit on PR vouchdev#41. CI was already green; these are latent bugs the review caught. storage._embed_and_store - Re-embed when the active embedder model changes, not only when the content hash changes. The previous short-circuit kept stale vectors on disk after a model swap, mixing document embeddings from one model with queries from another. - Truly best-effort: catch any exception during encode/put/meta/dedup so a hook failure can't bubble back to the caller and leave an artifact persisted-but-API-errored. index_db.reset - Also clears embedding_index, query_embedding_cache, embedding_dupes, and embedding_* keys from index_meta. Otherwise `vouch index` left orphaned hits behind after artifacts were removed. index_db.search_embedding (Python fallback) - Normalize the stored vector by its own L2 norm before scoring, so rankings match the sqlite-vec `1 - vec_distance_cosine` path. The previous raw dot product made magnitude leak into the score. index_db.search_semantic - Invalidate the query-vector cache entry when the embedder dim no longer matches what's on disk; otherwise a model swap returned stale vectors living in the wrong space. server.kb_search + jsonl_server._h_search (hybrid branch) - Hybrid now passes min_score into search_semantic (consistent with embedding/auto branches) and wraps the FTS lookup in try/except so an FTS5 error doesn't fail the whole request. - JSONL transport rejects unknown backend values with a clear error instead of silently returning []; matches MCP transport behavior. embeddings.fusion - rrf_fuse validates limit >= 0 and k >= 0 (k = -rank would divide by zero). weighted_fuse validates limit >= 0. Tests - Three new fusion guard tests cover the negative-limit and negative-k paths.
2 tasks
plind-junior
added a commit
that referenced
this pull request
May 22, 2026
… hook Bundles the substantive correctness fixes flagged by CodeRabbit on PR #41. CI was already green; these are latent bugs the review caught. storage._embed_and_store - Re-embed when the active embedder model changes, not only when the content hash changes. The previous short-circuit kept stale vectors on disk after a model swap, mixing document embeddings from one model with queries from another. - Truly best-effort: catch any exception during encode/put/meta/dedup so a hook failure can't bubble back to the caller and leave an artifact persisted-but-API-errored. index_db.reset - Also clears embedding_index, query_embedding_cache, embedding_dupes, and embedding_* keys from index_meta. Otherwise `vouch index` left orphaned hits behind after artifacts were removed. index_db.search_embedding (Python fallback) - Normalize the stored vector by its own L2 norm before scoring, so rankings match the sqlite-vec `1 - vec_distance_cosine` path. The previous raw dot product made magnitude leak into the score. index_db.search_semantic - Invalidate the query-vector cache entry when the embedder dim no longer matches what's on disk; otherwise a model swap returned stale vectors living in the wrong space. server.kb_search + jsonl_server._h_search (hybrid branch) - Hybrid now passes min_score into search_semantic (consistent with embedding/auto branches) and wraps the FTS lookup in try/except so an FTS5 error doesn't fail the whole request. - JSONL transport rejects unknown backend values with a clear error instead of silently returning []; matches MCP transport behavior. embeddings.fusion - rrf_fuse validates limit >= 0 and k >= 0 (k = -rank would divide by zero). weighted_fuse validates limit >= 0. Tests - Three new fusion guard tests cover the negative-limit and negative-k paths.
plind-junior
added a commit
that referenced
this pull request
May 22, 2026
…sion feat(embeddings): hybrid fusion strategies (RRF, weighted, normalized)
plind-junior
added a commit
that referenced
this pull request
Jul 7, 2026
* fix(server): block path traversal in register_source_from_path kb.register_source_from_path read the contents of any file the process could access, letting an agent register /etc/passwd, ~/.ssh/id_rsa, ~/.aws/credentials etc. as a "source" and then retrieve the bytes via kb.cite or kb.list_sources. Resolve the path (following symlinks) and require it to be inside the KB root before reading. Adds KBStore.resolve_under_root() so both the JSONL handler and the MCP tool share one containment check. Fixes #10 * fix(cli): translate domain errors into clean ClickException output The CLI handlers for approve and reject caught only (ArtifactNotFoundError, ValueError) but proposals.approve() / proposals.reject() raise ProposalError -- a RuntimeError subclass. The four propose-* shortcuts caught nothing at all. Result: a double- approve, an empty rejection reason, an empty claim text, or an unknown source id surfaced a raw Python traceback instead of the one-line `Error: ...` the rest of the CLI emits. Add a small `_cli_errors` context manager that translates ArtifactNotFoundError / ValueError / ProposalError / LifecycleError into click.ClickException, and apply it to every command that calls into proposals, lifecycle, sessions, or storage. The MCP and JSONL servers already do the equivalent in their own envelopes; this brings the human-facing surface in line. Adds tests/test_cli.py with regressions for approve, reject, propose- claim, propose-entity, and show. * chore(lint): chain FileExistsError cause in storage put_ methods ruff B904 requires `raise ... from err` inside an except block so the original exception is preserved on the chained __cause__. Without it, the CI lint step rejects the file. The seven sites are pre-existing (put_claim, put_page, put_entity, put_relation, put_evidence, put_session, put_proposal); this PR inherited the failure from main rather than introducing it, but the path traversal fix cannot land until lint is green, so the cleanup happens here. No behavior change: the chained exception carries .__cause__ but the public ValueError message and type are unchanged. * fix(verify): catch ArtifactNotFoundError and OSError on stored read verify_source() caught FileNotFoundError, but store.read_source_content() raises ArtifactNotFoundError (a KeyError subclass) when the content blob is missing -- so the "stored content missing" graceful path never ran and a single broken source crashed the entire verify_all() sweep, breaking `vouch source verify` and `vouch doctor`. The same call can also raise OSError (permission denied, TOCTOU race between exists() and read_bytes(), underlying I/O error) which was likewise unhandled. Catch both: the existing "missing" path keeps its note, and OSError surfaces as "stored content unreadable: <reason>". Adds three regression tests: - missing-content blob -> graceful per-source failure - unreadable stored content (monkeypatched PermissionError) -> graceful per-source failure with the underlying reason in the note - mixed sweep with one good + one broken source -> verify_all returns both results instead of aborting at the first failure Fixes #30 * chore(ci): unblock lint/type/test gates so the CLI fix can land The CI workflow runs `ruff check`, `mypy`, then `pytest`, and the fix/cli-clean-domain-errors branch was failing all three on pre-existing main-branch issues unrelated to the CLI change itself. * ruff: add `from e` to the seven `raise ValueError(...) from FileExistsError` re-raises added by the recent exclusive-create guards in storage.put_* (B904), and let ruff re-sort the cli.py import block (I001). * mypy: narrow the `kind` value flowing into `ContextItem(type=...)` with a `Literal` cast so the strict-typed field accepts what the search backends actually return. * pytest: `session_end()` mutates a session that `session_start()` has already written, but `put_session()` now uses exclusive create and rejects the second write. Add `KBStore.update_session()` mirroring `update_claim` and have `session_end` call it; existing test_sessions coverage now passes again. No behavior change for the CLI surface — these are infrastructure fixes so the existing test_sessions / lint / type assertions pass on this branch. * chore(ci): unblock mypy + test gates for path-traversal PR The B904 lint failures were already addressed in 2a439a5, but the CI matrix still fails on two pre-existing main-branch issues: * mypy: context.py:64 passed `kind: str` into ContextItem.type, which is Literal["claim","page","entity","relation","source"]. Narrow it with a typing.cast so strict mode accepts the search-backend output. * pytest: session_end() mutates a session that session_start() already wrote, but put_session() switched to exclusive create and rejects the second write. Add KBStore.update_session() mirroring update_claim and have session_end call it; test_sessions coverage passes again. No behavior change for the path-traversal fix itself. * fix(server): close TOCTOU window between path check and read CodeRabbit on PR #28 noted that resolve_under_root() only validated a pathname snapshot. Callers then re-opened the same name with .is_file() and .read_bytes(), so an attacker who can swap the resolved path for a symlink between the containment check and the read can still exfiltrate an out-of-root file via kb.register_source_from_path. Collapse the validate-then-read into a single trusted helper `KBStore.read_under_root(path)` that returns `(resolved, bytes)`: 1. Path.resolve() chases pre-existing symlinks and the resulting target is checked for containment (existing behaviour -- legitimate in-root symlinks still work). 2. The read goes through `os.open(resolved, O_RDONLY | O_NOFOLLOW)` so a fresh symlink placed at the resolved name *after* the check fails with ELOOP rather than following the swap. 3. `fstat` + `S_ISREG` rejects directories / device nodes / pipes atomically on the same fd, replacing the racy `is_file()` test the callers used to do. Both register_source_from_path handlers (MCP + JSONL) switch to the new helper and drop their now-redundant follow-up checks. Adds tests: - symlink swapped into the resolved name -> rejected via ELOOP - directory at a valid path -> rejected via S_ISREG Existing "outside the root" and "inside the root" tests still pass. * chore(lint): chain FileExistsError cause in storage put_ methods ruff B904 requires `raise ... from err` inside an except block so the original exception is preserved on the chained __cause__. Without it, the CI lint step rejects the file. The seven sites are pre-existing (put_claim, put_page, put_entity, put_relation, put_evidence, put_session, put_proposal); this PR inherited the failure from main rather than introducing it, but the path traversal fix cannot land until lint is green, so the cleanup happens here. No behavior change: the chained exception carries .__cause__ but the public ValueError message and type are unchanged. * chore(ci): unblock mypy + test gates for path-traversal PR The B904 lint failures were already addressed in 2a439a5, but the CI matrix still fails on two pre-existing main-branch issues: * mypy: context.py:64 passed `kind: str` into ContextItem.type, which is Literal["claim","page","entity","relation","source"]. Narrow it with a typing.cast so strict mode accepts the search-backend output. * pytest: session_end() mutates a session that session_start() already wrote, but put_session() switched to exclusive create and rejects the second write. Add KBStore.update_session() mirroring update_claim and have session_end call it; test_sessions coverage passes again. No behavior change for the path-traversal fix itself. * docs: add banner diagram to README * docs(spec): semantic search as primary retrieval backend Approved design for feat/semantic-search. Embedding (sentence- transformers all-mpnet-base-v2) becomes the primary search backend with FTS5 as fallback. Synchronous-at-write indexing across all six artifact types (claim, page, source, entity, relation, evidence). Maximally functional scope (~3000 LOC): pluggable model adapter registry, sqlite-vec ANN with NumPy fallback, cross-encoder rerank, HyDE query expansion, ingest-time duplicate detection, model-identity migration, recall/MRR/nDCG eval harness, and full CLI/MCP/JSONL parity. The writing-plans step will turn the rollout order in section 16 into concrete phased tasks. * docs(plan): semantic search implementation plan 32-task TDD plan for the semantic-search feature: foundation, storage, write hooks across all six artifact types, semantic-primary search integration in MCP/JSONL/CLI, RRF fusion + hybrid, cross-encoder rerank, HyDE expansion, ingest-time duplicate detection, model-identity migration, recall/MRR/nDCG scorer, end-to-end integration test, and user docs. Each task: failing test, minimal implementation, passing test, commit. MockEmbedder test double keeps the unit suite fast; the real model is exercised only under @pytest.mark.integration. The evaluation module is named scorer.py rather than eval.py to avoid shadowing the Python builtin and to keep static analysers quiet; the user-facing CLI subcommand remains `vouch eval embedding` (the Click group is registered under the name "eval", with the Python identifier eval_group). * feat(embeddings): add optional-deps extras and pytest markers * feat(embeddings): create package skeleton * feat(embeddings): Embedder ABC, registry, content_hash, MockEmbedder * feat(embeddings): sentence-transformers all-mpnet-base-v2 default adapter * feat(embeddings): sentence-transformers MiniLM-L6 alternative adapter * feat(embeddings): fastembed BGE alternative (no-torch) adapter * fix(ci): satisfy ruff SIM105 + add mypy overrides for optional deps CI ran ruff which flagged SIM105 on the three try/except ImportError guard blocks in src/vouch/embeddings/__init__.py. Replace each with `contextlib.suppress(ImportError)` -- same semantics, satisfies ruff. Also add mypy overrides for numpy / sqlite_vec / sentence_transformers / fastembed so the type check passes in the base [dev] CI install where those optional extras aren't present. The embedding code paths that import these libraries are only reached when the extras are installed at runtime; for the static type check the missing stubs are noise. * fix(ci): skip embeddings test suite when numpy isn't installed CI runs `pip install -e '.[dev]'` which deliberately excludes the optional `[embeddings]` extras. tests/embeddings/test_*.py modules import numpy at top level, so pytest collection fails with ModuleNotFoundError before any test can be deselected. Add tests/embeddings/conftest.py with `pytest.importorskip("numpy")` so the entire embeddings test directory skips gracefully when numpy is absent. Once `pip install vouch[embeddings]` (or numpy itself) is present, the skip is a no-op and the tests run normally. * fix(bundle): skip Pydantic validation for opaque source content files `_validate_content` in bundle.py uses the path's first directory component to look up a Pydantic validator. For sources, both `sources/<sha>/meta.yaml` (the Source model) and `sources/<sha>/content` (raw opaque bytes) hit the same "sources" key, so the validator was being run on the raw content bytes and failing with "1 validation error for Source". Add an early return for any non-meta.yaml path under `sources/` so opaque content bytes are not Pydantic-validated. Only the Source metadata file is checked, which matches the original intent of the PR #13 validation. Unblocks three pre-existing test_bundle.py failures inherited from upstream main. * fix(embeddings): MockEmbedder uses uint32 scaling to avoid NaN/Inf Per CodeRabbit on PR #37: decoding sha256 chunks as raw float32 (via struct.unpack("<f", ...)) produced NaN/Inf for ~1 in 2^7 chunks because most 4-byte patterns aren't finite IEEE754 floats. Norm overflow then made unit normalization unreliable and broke cosine similarity asserts in downstream phases (the dedup test in Phase 6 already had to work around this with a custom _HashEmbedder). Decode each chunk as an unsigned 32-bit integer, scale to [-1.0, 1.0], accumulate in float64 for exact norm, and cast to float32 on return. Deterministic, finite, well-behaved. * feat(embeddings): state.db schema for embedding storage + put/get helpers * feat(embeddings): NumPy brute-force cosine search over embedding_index * feat(embeddings): sqlite-vec ANN path with NumPy fallback * feat(embeddings): query embedding LRU cache * style: fix ruff E402/SIM105/E702/I001 violations from Phase 2 storage tasks * fix(embeddings): use outer-query for WHERE on cosine alias in search_embedding Per CodeRabbit Critical on PR #39/40/43: SQLite does not allow a column alias defined in the SELECT list to be referenced in the WHERE clause of the same query. The old form SELECT ... 1.0 - vec_distance_cosine(vec, ?) AS score ... WHERE ... AND score >= ? raised `no such column: score` at runtime; the catch-all `except sqlite3.OperationalError` silently swallowed it and made every call fall through to the NumPy brute-force path, which works but defeats the whole point of installing sqlite-vec. Wrap the projection in a subquery so the WHERE clause sees the alias. No behavior change for callers that have already been using the fallback (NumPy still produces correct results); the ANN code path now actually runs when sqlite-vec is loaded. * feat(embeddings): write-time embedding hook + wire put_claim * feat(embeddings): search_semantic wrapper with query cache * feat(embeddings): hook all six artifact write paths * feat(embeddings): kb_search defaults to embedding primary, fts5 fallback * feat(embeddings): re-embed on update_claim * feat(embeddings): JSONL _h_search parity with MCP semantic-primary dispatch * style: fix ruff SIM105/E501/I001/RUF059 violations from Phase 3 * fix(ci): silence mypy import-untyped for forward-ref fusion import The hybrid backend branch in kb_search (server.py) and _h_search (jsonl_server.py) lazily imports vouch.embeddings.fusion (added in Phase 5). On Phase 4's branch that module doesn't exist yet, so mypy emits import-untyped. Mark each import with `# type: ignore` and let ruff auto-format the line. * fix(ci): silence mypy import-untyped for forward-ref dedup import The KBStore._embed_and_store helper lazily imports vouch.embeddings.dedup (added in Phase 6). On Phase 3's branch that module does not yet exist, so mypy treats it as untyped and errors out. Mark the import with `# type: ignore[import-not-found,import-untyped, unused-ignore]` so the type check passes here and stays silent once Phase 6 lands. * spec: cut dated 2026-05-21 snapshot; promote schema generator to script * ci(mypy): collapse fusion import to single line for type-ignore * feat(embeddings): RRF/weighted/normalized fusion strategies * docs: add 'What ships today' table to README * kb: approve review-gate fact * docs: add 'When to use vouch' section * fix(cli): honor VOUCH_AGENT in _whoami for consistent actor attribution The CLI propose-* commands and most actor sites called _whoami(), which only checked VOUCH_USER and the OS user — VOUCH_AGENT was silently ignored. The MCP and JSONL servers (server.py, jsonl_server.py) and session_start already honour VOUCH_AGENT, so multi-agent attribution broke as soon as you used the CLI to propose. Prefer VOUCH_AGENT over VOUCH_USER over the OS user so the recorded proposed_by / audit actor matches across all three transports. * docs: add captured example session walkthrough Real propose -> review -> commit -> retrieve loop captured from a sandbox run on 2026-05-21. Includes on-disk YAML, audit log lines, and an honest note about the literal substring backend limitation pre-embeddings. * docs: add screen recording of full vouch loop (VHS tape + GIF) * docs: embed demo GIF under Quick start in README * fix(embeddings): address CodeRabbit review on hybrid + fusion + write hook Bundles the substantive correctness fixes flagged by CodeRabbit on PR #41. CI was already green; these are latent bugs the review caught. storage._embed_and_store - Re-embed when the active embedder model changes, not only when the content hash changes. The previous short-circuit kept stale vectors on disk after a model swap, mixing document embeddings from one model with queries from another. - Truly best-effort: catch any exception during encode/put/meta/dedup so a hook failure can't bubble back to the caller and leave an artifact persisted-but-API-errored. index_db.reset - Also clears embedding_index, query_embedding_cache, embedding_dupes, and embedding_* keys from index_meta. Otherwise `vouch index` left orphaned hits behind after artifacts were removed. index_db.search_embedding (Python fallback) - Normalize the stored vector by its own L2 norm before scoring, so rankings match the sqlite-vec `1 - vec_distance_cosine` path. The previous raw dot product made magnitude leak into the score. index_db.search_semantic - Invalidate the query-vector cache entry when the embedder dim no longer matches what's on disk; otherwise a model swap returned stale vectors living in the wrong space. server.kb_search + jsonl_server._h_search (hybrid branch) - Hybrid now passes min_score into search_semantic (consistent with embedding/auto branches) and wraps the FTS lookup in try/except so an FTS5 error doesn't fail the whole request. - JSONL transport rejects unknown backend values with a clear error instead of silently returning []; matches MCP transport behavior. embeddings.fusion - rrf_fuse validates limit >= 0 and k >= 0 (k = -rank would divide by zero). weighted_fuse validates limit >= 0. Tests - Three new fusion guard tests cover the negative-limit and negative-k paths. * fix(lifecycle): only catch missing-artifact errors in cite() the bare except was treating every error as a missing citation, which hid real bugs. narrow it to ArtifactNotFoundError so unexpected failures actually surface. * fix(context): only catch missing-artifact errors in citation lookup same issue as cite() — a bare except was hiding real errors behind an empty citation list. narrow it to ArtifactNotFoundError. * refactor(sessions): keep tracebacks when crystallize() approve fails we were dropping the traceback and only stringifying the error, which made it painful to diagnose anything other than ProposalError. now logs via logger.exception and includes error_type in the failures list. * fix(context): only catch sqlite errors when FTS5 falls back the retrieval helper had a bare except that would also swallow bugs in the substring fallback path or in our own code. narrow it to sqlite3.Error so only the intended cases (FTS5 missing, db missing, schema mismatch) trigger the fallback. * refactor(health): use typed status filter for pending proposal count we were listing every proposal and comparing p.status.value == 'pending' as a string. list_proposals() already takes a ProposalStatus filter — use it. shorter, type-checked, and avoids the stringly-typed compare. * chore(capabilities): pin pluginApi compat baseline, fail CI on host-compat drift * feat(dual-solve): surface engine logs * fix(sessions): make crystallize retry idempotent for summary pages Partial crystallize runs that already wrote session-{id} summary pages failed on retry with "page already exists". Upsert the summary page and derive its artifact list from all approved session proposals. Fixes #139 Co-authored-by: Cursor <cursoragent@cursor.com> * fix(models): reject empty text/name/title on claim/entity/page the non-empty contract for `Claim.text`, `Entity.name`, and `Page.title` lived only in the `propose_*` helpers, so `store.put_*`, `store.update_*`, and bundle/sync import (via `_validate_content`) all silently accepted empty or whitespace-only values and landed artifacts carrying zero of the field's semantic content. add `@field_validator`s on each field, mirroring the existing `Claim.evidence` min-citation validator (#81/#82): they reject blank input at construction time, so every write path — direct construction, put/update, and import — inherits the check at once. the validators gate on emptiness only and preserve surrounding whitespace of non-blank values. `Source.locator` is deliberately left out of scope — its format question is bigger and deserves its own issue, as the report notes. closes #155 * refactor(models): extract shared _require_non_empty validator helper address review on #300: the three #155 field validators (Claim.text / Entity.name / Page.title) were structurally identical strip-checks. extract a module-level `_require_non_empty(v, label)` helper they all delegate to, keeping the per-field error messages. also fix a mid-entry sentence casing in the CHANGELOG. no behaviour change. * fix: resolve RPC traceback leakage on internal errors Unexpected exceptions in handle_request() included full stack traces in the JSON error envelope, exposing paths and internals to HTTP /rpc clients. Log server-side and return message only. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(review): kb.triage_pending — advisory triage scoring for the pending queue a long `kb.list_pending` forces the reviewer to reconstruct, per proposal, whether the claim fits the existing kb, whether its citations resolve, whether it duplicates something already filed, and whether it contradicts an approved claim. this adds an optional triage pass that scores each pending proposal on those four signals and attaches a `_meta.vouch_triage` block (recommendation/score/signals/rationale) to help a reviewer prioritize, without ever deciding anything itself. read-only by construction: the pass never calls proposals.approve/reject, store.put_*, or store.move_proposal_to_decided — a human still calls kb.approve/kb.reject. citation_quality reuses proposals._payload_block_reason; duplication_risk reuses the propose-time embedding similarity path (embeddings.similarity.find_similar_on_propose) and degrades to a difflib heuristic when the embeddings extra isn't installed. fit uses a separate, lower-threshold embedding search so a near-duplicate hit doesn't also inflate fit and cancel out its own duplication penalty. opt-in via `triage.enabled: true` in config.yaml (default false). registered at all four kb.* surface sites (server.py, jsonl_server.py, capabilities.py, cli.py) plus `vouch triage [proposal-id...]` with `--json` and `--reverse`. * feat(diff): register kb.diff at all four kb.* surface sites vouch diff shipped CLI-only in 0.1.0, an explicit non-goal at the time ("MCP/JSONL parity ... kb.* surface unchanged"). that leaves it the only read method skipping the four-site registration convention documented in CLAUDE.md, so agents talking MCP/JSONL have no way to fetch a revision diff. adds the kb_diff MCP tool, the kb.diff JSONL handler, and the capabilities.METHODS entry, next to the other by-id read tools (kb_read_claim/kb_read_page) with the same unrestricted-read posture. also makes new_id optional for a superseded claim: diff_artifacts and the CLI both resolve it from superseded_by when omitted, erroring clearly when there's no successor (pages still require an explicit new_id — they have no successor pointer). closes #327. * fix(jsonl): define module logger after the import block #361 inserted `_log = logging.getLogger(...)` between two import groups in jsonl_server.py. that statement-among-imports trips ruff's E402 on every import that follows it, so `ruff check src tests` — the lint gate in ci — now fails on the whole repo. move the logger definition below the imports (where storage.py already keeps its own module logger); no behaviour change. * feat(adapters): toml_merge install strategy for codex config.toml .codex/config.toml is codex's primary config file, so the plain-copy path silently skipped any project where codex was already configured and vouch never got wired. add a toml_merge entry flag mirroring json_merge: parse the existing destination with tomllib, deep-merge the template's tables into it (existing user values always win on conflict), and write back. flip the codex T1 entry to toml_merge. writing uses a minimal hand-rolled serializer (tables, arrays, inline tables in arrays, scalars, datetimes) so the dependency set stays unchanged; the output must survive a tomllib round-trip back to the merged data, and anything the serializer can't faithfully re-emit degrades to skipped rather than risking the user's config. closes #384 * feat(adapters): codex T2 agents.md fenced snippet codex reads AGENTS.md for project instructions the way cursor does, but the codex adapter stopped at T1, so a codex session got the kb tools with no standing guidance on recall-first or the review gate. ship adapters/codex/AGENTS.md.snippet as a T2 tier with the standard fence markers, kept in lockstep with cursor's snippet modulo the host name (enforced by a sync test). also close the edited-fence gap in _install_fenced per the ticket's acceptance criteria: a fence body that drifted from the shipped snippet is replaced within the markers (reported as merged) instead of being skipped forever, while user content outside the fence stays untouched. a begin marker without an end marker is treated as corrupt and left alone. closes #385 * feat(adapters): codex T3 skills mirroring the vouch slash commands claude-code T3 ships nine slash commands; codex users got none of the guided flows. ship them as codex skills under the project-local .codex/skills/ as a T3 tier. scope decision the ticket asked to resolve first: codex loads custom prompts only from ~/.codex/prompts/ (user-global) and has deprecated them upstream in favour of skills, which do have a project-local home at <project>/.codex/skills/ in trusted projects. the #179 rule forbids a project-scoped install from touching home-directory state, so prompts are out and skills are in — recorded in the manifest comment and the adapter readme, with a manual-copy pointer for users who still want ~/.codex/prompts. the SKILL.md files are referenced from the openclaw mirror rather than duplicated (same reuse pattern openclaw itself applies to the claude-code commands), so one edit updates every host. a parametrized sync test asserts each installed codex skill body equals the matching claude-code command body, and a scope test asserts every installed path stays under the project target. closes #386 * feat(capture): ingest codex session rollouts into review-gated summaries session auto-capture was claude-code-only: hooks drive capture observe and capture finalize live. codex has no hook stream, but it persists every session as a rollout jsonl under $CODEX_HOME/sessions containing user messages, tool calls, and outputs — everything the existing rollup needs, just after the fact. new cli command `vouch capture ingest-codex [<rollout> | --latest]` parses one rollout through a small dedicated parser (codex_rollout.py) that maps function_call records into the same observation shape capture.observe produces — shell commands with failure detection from exit codes, apply_patch heredocs surfaced as file edits, mcp tools under their own names, session mechanics skipped — then reuses the existing build_summary_body -> propose_page rollup. one code path from observation to proposal, two front doors. the rollout format is not a stable public contract: unknown record types are tolerated, and unreadable, compressed, or meta-less files degrade to a CodexRolloutError with an actionable message and a non-zero exit, never a stack trace. re-ingesting a session is a no-op keyed on the rollout's session id; --latest resolves the newest rollout whose recorded cwd matches the current project. proposals are attributed to the codex actor (VOUCH_AGENT wins when set), respect capture's enabled/min_observations config, and never touch approve(). fixture rollouts use placeholder data only, enforced by a test. closes #387 * feat(adapters): codex T4 hook wiring for automatic session capture with `vouch capture ingest-codex` available, codex can self-capture the way claude-code T4 does. the ticket assumed the `notify` setting in config.toml, but codex only honours notify in user-global config — it is a restricted key in project-local layers — and the #179 rule forbids touching ~/.codex. codex's hooks system is the project-local equivalent: `.codex/hooks.json` (loaded in trusted projects) fires Stop when a turn completes, with a payload carrying the session id and transcript path. T4 ships that file through json_merge, so an existing user hooks.json is deep-merged into, never overwritten, and re-runs don't duplicate the hook. the handler is a new --hook mode on ingest-codex: read the stop payload from stdin, resolve the session's rollout (transcript_path when present, else by session id in the rollout filename), ingest idempotently, and exit 0 no matter what — the same never-break-the- host rule capture observe follows. stop fires per turn, not per session end, so the finalize policy the ticket left open is resolved as idempotent re-ingest: the dedup guard now refreshes a session's still-PENDING proposal in place when the rollout grew (same id, updated summary, audit-logged), reports an unchanged rollout as a no-op, and never resurrects a proposal a human already decided. storage gains update_proposal, a pure-I/O in-place rewrite of a pending proposal file. wiring only — everything still lands through propose_page; nothing touches approve(). closes #388 * test(adapters): live codex install gate against the real cli tests/test_openclaw_plugin_load_real.py set the pattern: exercise the real host cli when it's on PATH, skip cleanly where it isn't. the codex adapter had no equivalent, so regressions in the shipped config only surfaced when a user hit them — the old T1 silent-skip no-op is exactly the class of bug a live gate would have caught. the new suite installs the adapter into a temp project at T4, marks the project trusted inside an isolated CODEX_HOME, and asserts through `codex mcp list --json` that the vouch server is visible: next to a pre-seeded unrelated server on the merge path, alone on the fresh path, and absent for an untrusted project (the config must stay inert where codex says it does). a full-tier check covers the fenced AGENTS.md, the skills, and the Stop hook, and an isolation check pins every written artifact under the temp project. assertions target the observable contract — server listed, config parses, snippet present — not codex internals, so version bumps shouldn't break the suite. no network, no credentials, never touches the real ~/.codex; runs in the normal pytest invocation. closes #389 * docs(adapters): codex adapter docs reflect the tiered install three surfaces went stale once the codex tier work landed. the adapters table row described codex as a one-file config.toml snippet; it now lists the tiered install. the codex adapter readme led with the manual ~/.codex edit; it now leads with `vouch install-mcp codex`, explains the project-local .codex/config.toml choice and the trust requirement, summarizes what each tier adds (mcp wire / AGENTS.md snippet / skills / capture hook) with the merge-safety guarantees, and keeps the manual edit as an explicit fallback section. getting-started treated codex as a one-liner aside inside the claude instructions; the install section now shows both hosts side by side. the prompts-location decision from the T3 ticket and the notify restriction from the T4 ticket stay documented in their sections. no code; placeholder examples only. closes #390 * docs(mcp): design a friendlier mcp surface — profiles + auto-recall the closest competitor (pmb) exposes 10 mcp tools by default and hides the rest behind a profile flag; vouch shows all 58 every turn, which is the main first-touch friendliness gap. this spec adds a minimal-by-default tool profile, fusion-by-default retrieval, a per-prompt auto-recall hook, and compact tool descriptions. all of it is presentation-only — the review gate, the yaml storage, and the protocol method surface are untouched. it also closes the current 2-of-3 surface-parity gap as a bonus. * docs(mcp): plan the friendlier-mcp-surface build six TDD tasks: tool profiles (minimal default), real mcp↔methods parity, fusion-by-default retrieval, near-duplicate drop, per-prompt auto-recall hook, and compact tool descriptions. reconcile the spec: minimal is 8 tools (kb_capabilities kept as the discovery escape hatch) and the recency multiplier is deferred (per-hit timestamps aren't plumbed through _retrieve). * feat(mcp): add tool profiles, expose a minimal surface by default agents saw all 58 kb.* tools every turn — the main first-touch friendliness gap vs pmb, which exposes ~10 by default. add a profile layer applied in run_stdio: minimal (8 core tools) by default, standard (16), or full (58), selected by VOUCH_TOOL_PROFILE / config mcp.tool_profile. exposure only — the protocol surface, jsonl/cli, and the review gate are unchanged. * fix(mcp): harden profile resolution against a non-dict config section a bare `mcp:` key in config.yaml parses to None in YAML, and a non-dict mcp section is also possible from hand-edited configs. the previous config.get("mcp", {}).get("tool_profile") chain only supplied the {} default when the key was absent, so a present-but-None or non-dict value passed through and the chained .get raised AttributeError. run_stdio calls resolve_profile_name outside its try/except, so this would crash vouch serve instead of degrading to the minimal default. guard each nesting level with isinstance, matching the established reflex_cfg pattern in salience.py. * test(capabilities): assert mcp tool set matches the method list the parity test only compared capabilities.METHODS to the jsonl handlers; the 58 mcp tools were never checked, so mcp drift passed ci. enumerate the unfiltered server tools and assert they equal METHODS — the real 3-surface check for the mcp side. * feat(retrieval): fuse embedding + fts5 by default instead of a waterfall _retrieve tried embedding, then fts5, then substring, returning the first non-empty list — so lexical and semantic hits never combined. auto and hybrid now fuse both retrievers with the already-built rrf_fuse and tag hits "hybrid"; explicit embedding/fts5/substring pins are unchanged. existing KBs (config says "auto") benefit with no migration. * fix(volunteer): treat hybrid relevance as rank-relative, not pre-normalized normalize_relevance() grouped "hybrid" with "embedding", returning the raw score unclamped-by-batch on the assumption it was already a 0-1 similarity. now that _retrieve's auto/hybrid path tags fused hits "hybrid" with a rrf_fuse score (bounded by ~2/(k+rank), typically 0.01-0.03), that assumption stopped holding: every fused relevance landed far below DEFAULT_THRESHOLD (0.85), so kb.volunteer_context stopped firing for any KB using the new default backend. batch-normalize hybrid like fts5 and substring instead — restores the confidence-gated push channel (#236). * feat(retrieval): drop near-duplicate items from the context pack a fused pack could surface the same fact from two claims. add a cheap greedy jaccard pass (>=0.85 over the first 40 tokens) that keeps the highest-scored of a near-duplicate cluster, so an agent never reads the same thing twice. * fix(retrieval): dedupe by score so the highest-scored near-dup wins _dedupe_near_duplicates assumed its input arrived in descending-score order, which only holds for the plain retrieval path. when build_context_pack runs with expand_graph=True, graph neighbours are appended in bfs order with decayed scores after the sorted main items, so a higher-scored neighbour could be dropped in favor of an earlier, lower-scored main item. sort inside the function so the invariant holds regardless of caller order. * feat(hooks): inject per-prompt kb context via a userpromptsubmit hook recall used to be either a session-start firehose or an explicit tool call. add a pure, never-raising helper + a hidden `vouch context-hook` command that reads the host's prompt payload on stdin and prints an additionalContext envelope, and wire it into the claude-code adapter's UserPromptSubmit hook — so relevant, approved knowledge is injected every turn with zero tool calls. * fix(hooks): never raise on non-dict payload or missing kb a reviewer reproduced two crash paths that violated task 5's own contract that the prompt hook must never block a turn: build_claude_prompt_hook raised AttributeError on non-dict JSON (null/number/bool/array/string all decode fine but lack .get), and the context-hook CLI command called _load_store(), whose sys.exit(2) on a missing KB is a SystemExit that slips past `except Exception` and exits the process nonzero. guard the payload with an isinstance check before .get, log a breadcrumb when build_context_pack fails so a broken KB isn't silently indistinguishable from "no hits", and swap _load_store() for the existing non-exiting _capture_store() helper so the command has no sys.exit on its path at all. * feat(mcp): serve one-line tool descriptions under non-full profiles full docstrings for every exposed tool are paid on every turn. under minimal and standard, trim each tool's description to its first line (full keeps the complete docstrings), cutting the per-turn context cost. * docs(mcp): document tool profiles note VOUCH_TOOL_PROFILE / mcp.tool_profile (default minimal, values minimal|standard|full) in the transports reference and the claude-code adapter guide; this repo has no mintlify/ tree yet so both live under docs/ and adapters/claude-code/ instead. * fix(retrieval): dedupe by score but keep caller order for the pack _dedupe_near_duplicates sorted by score and returned that score-sorted order, which fought graph-expansion: build_context_pack appends decayed-score neighbours after the ranked hits, and fused primary hits now carry small rrf scores. re-sorting let an irrelevant depth-2 neighbour outrank a real match, and the max_chars tail-pop then evicted the real match instead of the neighbour. keep the keep-decision in descending-score order so the highest-scored member of a near-duplicate cluster still survives, but return survivors in the caller's original order so budget eviction drops the tail (appended neighbours), not the ranked hits. * docs(changelog): note minimal mcp profile, fusion, auto-recall summarize this branch's user-visible changes under [Unreleased]: the minimal-by-default mcp tool profile, rrf fusion for auto/hybrid retrieval, and the per-prompt auto-recall hook in the claude-code adapter. * docs(readme): note the fourth hook, per-prompt recall, and lean mcp profile install-mcp now writes a fourth hook (UserPromptSubmit -> vouch context-hook), and recall fires per prompt, not only at session start. also note the kb.* mcp surface defaults to a lean profile that widens via VOUCH_TOOL_PROFILE. * fix(capabilities): read openclaw.compat from package.json (#417) the host-compat drift check (#237) read openclaw.compat.pluginApi from openclaw.plugin.json. that block moved to package.json when the plugin packaging was split, and openclaw.* is now banned from the manifest (enforced by test_manifest_carries_no_dead_dialect_fields). the #237 reader was left pointing at the old, now-empty location, so host_compat silently degraded to {} and both host-compat assertions failed on the test branch — which every open pr targeting test inherited, including #413. repoint _load_host_compat and the test helpers at package.json. the openclaw.compat.pluginApi key path is identical in both files, so this is purely a source-file change with no behaviour difference. * chore(repo): untrack owner-local web/ and .claude/ (#413) these two directories were committed by accident and reach every first-time contributor on clone. neither is project source: - web/ is the netlify-deployed marketing landing page. it is shipped via the netlify cli and is not meant to live in the repo tree. - .claude/ is a personal claude code config: a settings.json with owner-specific hooks, plus command files that are byte-identical duplicates of adapters/claude-code/.claude/commands/. untrack both (files stay on disk) and gitignore them, anchored to the repo root so src/vouch/web and adapters/claude-code/.claude stay tracked. also ignore coverage.xml and the .netlify/ and .playwright-mcp/ tool-scratch dirs. * chore(merge): resolve changelog and version conflicts with main * chore(merge): update changelog to resolve merge conflicts * chore(merge): update to main's latest versions for clean merge --------- Co-authored-by: plind-junior <138900956+dripsmvcp@users.noreply.github.com> Co-authored-by: alpurkan17 <alpurkan9@gmail.com> Co-authored-by: Tet-9 <igeparallax@gmail.com> Co-authored-by: Yaroslav98214 <diakovichyaroslav30@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: minion1227 <romantymkiv1999@gmail.com> Co-authored-by: RealDiligent <brave.challenge007@gmail.com> Co-authored-by: jsdevninja <topit89807@gmail.com>
plind-junior
added a commit
that referenced
this pull request
Jul 7, 2026
* fix(server): block path traversal in register_source_from_path kb.register_source_from_path read the contents of any file the process could access, letting an agent register /etc/passwd, ~/.ssh/id_rsa, ~/.aws/credentials etc. as a "source" and then retrieve the bytes via kb.cite or kb.list_sources. Resolve the path (following symlinks) and require it to be inside the KB root before reading. Adds KBStore.resolve_under_root() so both the JSONL handler and the MCP tool share one containment check. Fixes #10 * fix(cli): translate domain errors into clean ClickException output The CLI handlers for approve and reject caught only (ArtifactNotFoundError, ValueError) but proposals.approve() / proposals.reject() raise ProposalError -- a RuntimeError subclass. The four propose-* shortcuts caught nothing at all. Result: a double- approve, an empty rejection reason, an empty claim text, or an unknown source id surfaced a raw Python traceback instead of the one-line `Error: ...` the rest of the CLI emits. Add a small `_cli_errors` context manager that translates ArtifactNotFoundError / ValueError / ProposalError / LifecycleError into click.ClickException, and apply it to every command that calls into proposals, lifecycle, sessions, or storage. The MCP and JSONL servers already do the equivalent in their own envelopes; this brings the human-facing surface in line. Adds tests/test_cli.py with regressions for approve, reject, propose- claim, propose-entity, and show. * chore(lint): chain FileExistsError cause in storage put_ methods ruff B904 requires `raise ... from err` inside an except block so the original exception is preserved on the chained __cause__. Without it, the CI lint step rejects the file. The seven sites are pre-existing (put_claim, put_page, put_entity, put_relation, put_evidence, put_session, put_proposal); this PR inherited the failure from main rather than introducing it, but the path traversal fix cannot land until lint is green, so the cleanup happens here. No behavior change: the chained exception carries .__cause__ but the public ValueError message and type are unchanged. * fix(verify): catch ArtifactNotFoundError and OSError on stored read verify_source() caught FileNotFoundError, but store.read_source_content() raises ArtifactNotFoundError (a KeyError subclass) when the content blob is missing -- so the "stored content missing" graceful path never ran and a single broken source crashed the entire verify_all() sweep, breaking `vouch source verify` and `vouch doctor`. The same call can also raise OSError (permission denied, TOCTOU race between exists() and read_bytes(), underlying I/O error) which was likewise unhandled. Catch both: the existing "missing" path keeps its note, and OSError surfaces as "stored content unreadable: <reason>". Adds three regression tests: - missing-content blob -> graceful per-source failure - unreadable stored content (monkeypatched PermissionError) -> graceful per-source failure with the underlying reason in the note - mixed sweep with one good + one broken source -> verify_all returns both results instead of aborting at the first failure Fixes #30 * chore(ci): unblock lint/type/test gates so the CLI fix can land The CI workflow runs `ruff check`, `mypy`, then `pytest`, and the fix/cli-clean-domain-errors branch was failing all three on pre-existing main-branch issues unrelated to the CLI change itself. * ruff: add `from e` to the seven `raise ValueError(...) from FileExistsError` re-raises added by the recent exclusive-create guards in storage.put_* (B904), and let ruff re-sort the cli.py import block (I001). * mypy: narrow the `kind` value flowing into `ContextItem(type=...)` with a `Literal` cast so the strict-typed field accepts what the search backends actually return. * pytest: `session_end()` mutates a session that `session_start()` has already written, but `put_session()` now uses exclusive create and rejects the second write. Add `KBStore.update_session()` mirroring `update_claim` and have `session_end` call it; existing test_sessions coverage now passes again. No behavior change for the CLI surface — these are infrastructure fixes so the existing test_sessions / lint / type assertions pass on this branch. * chore(ci): unblock mypy + test gates for path-traversal PR The B904 lint failures were already addressed in 2a439a5, but the CI matrix still fails on two pre-existing main-branch issues: * mypy: context.py:64 passed `kind: str` into ContextItem.type, which is Literal["claim","page","entity","relation","source"]. Narrow it with a typing.cast so strict mode accepts the search-backend output. * pytest: session_end() mutates a session that session_start() already wrote, but put_session() switched to exclusive create and rejects the second write. Add KBStore.update_session() mirroring update_claim and have session_end call it; test_sessions coverage passes again. No behavior change for the path-traversal fix itself. * fix(server): close TOCTOU window between path check and read CodeRabbit on PR #28 noted that resolve_under_root() only validated a pathname snapshot. Callers then re-opened the same name with .is_file() and .read_bytes(), so an attacker who can swap the resolved path for a symlink between the containment check and the read can still exfiltrate an out-of-root file via kb.register_source_from_path. Collapse the validate-then-read into a single trusted helper `KBStore.read_under_root(path)` that returns `(resolved, bytes)`: 1. Path.resolve() chases pre-existing symlinks and the resulting target is checked for containment (existing behaviour -- legitimate in-root symlinks still work). 2. The read goes through `os.open(resolved, O_RDONLY | O_NOFOLLOW)` so a fresh symlink placed at the resolved name *after* the check fails with ELOOP rather than following the swap. 3. `fstat` + `S_ISREG` rejects directories / device nodes / pipes atomically on the same fd, replacing the racy `is_file()` test the callers used to do. Both register_source_from_path handlers (MCP + JSONL) switch to the new helper and drop their now-redundant follow-up checks. Adds tests: - symlink swapped into the resolved name -> rejected via ELOOP - directory at a valid path -> rejected via S_ISREG Existing "outside the root" and "inside the root" tests still pass. * chore(lint): chain FileExistsError cause in storage put_ methods ruff B904 requires `raise ... from err` inside an except block so the original exception is preserved on the chained __cause__. Without it, the CI lint step rejects the file. The seven sites are pre-existing (put_claim, put_page, put_entity, put_relation, put_evidence, put_session, put_proposal); this PR inherited the failure from main rather than introducing it, but the path traversal fix cannot land until lint is green, so the cleanup happens here. No behavior change: the chained exception carries .__cause__ but the public ValueError message and type are unchanged. * chore(ci): unblock mypy + test gates for path-traversal PR The B904 lint failures were already addressed in 2a439a5, but the CI matrix still fails on two pre-existing main-branch issues: * mypy: context.py:64 passed `kind: str` into ContextItem.type, which is Literal["claim","page","entity","relation","source"]. Narrow it with a typing.cast so strict mode accepts the search-backend output. * pytest: session_end() mutates a session that session_start() already wrote, but put_session() switched to exclusive create and rejects the second write. Add KBStore.update_session() mirroring update_claim and have session_end call it; test_sessions coverage passes again. No behavior change for the path-traversal fix itself. * docs: add banner diagram to README * docs(spec): semantic search as primary retrieval backend Approved design for feat/semantic-search. Embedding (sentence- transformers all-mpnet-base-v2) becomes the primary search backend with FTS5 as fallback. Synchronous-at-write indexing across all six artifact types (claim, page, source, entity, relation, evidence). Maximally functional scope (~3000 LOC): pluggable model adapter registry, sqlite-vec ANN with NumPy fallback, cross-encoder rerank, HyDE query expansion, ingest-time duplicate detection, model-identity migration, recall/MRR/nDCG eval harness, and full CLI/MCP/JSONL parity. The writing-plans step will turn the rollout order in section 16 into concrete phased tasks. * docs(plan): semantic search implementation plan 32-task TDD plan for the semantic-search feature: foundation, storage, write hooks across all six artifact types, semantic-primary search integration in MCP/JSONL/CLI, RRF fusion + hybrid, cross-encoder rerank, HyDE expansion, ingest-time duplicate detection, model-identity migration, recall/MRR/nDCG scorer, end-to-end integration test, and user docs. Each task: failing test, minimal implementation, passing test, commit. MockEmbedder test double keeps the unit suite fast; the real model is exercised only under @pytest.mark.integration. The evaluation module is named scorer.py rather than eval.py to avoid shadowing the Python builtin and to keep static analysers quiet; the user-facing CLI subcommand remains `vouch eval embedding` (the Click group is registered under the name "eval", with the Python identifier eval_group). * feat(embeddings): add optional-deps extras and pytest markers * feat(embeddings): create package skeleton * feat(embeddings): Embedder ABC, registry, content_hash, MockEmbedder * feat(embeddings): sentence-transformers all-mpnet-base-v2 default adapter * feat(embeddings): sentence-transformers MiniLM-L6 alternative adapter * feat(embeddings): fastembed BGE alternative (no-torch) adapter * fix(ci): satisfy ruff SIM105 + add mypy overrides for optional deps CI ran ruff which flagged SIM105 on the three try/except ImportError guard blocks in src/vouch/embeddings/__init__.py. Replace each with `contextlib.suppress(ImportError)` -- same semantics, satisfies ruff. Also add mypy overrides for numpy / sqlite_vec / sentence_transformers / fastembed so the type check passes in the base [dev] CI install where those optional extras aren't present. The embedding code paths that import these libraries are only reached when the extras are installed at runtime; for the static type check the missing stubs are noise. * fix(ci): skip embeddings test suite when numpy isn't installed CI runs `pip install -e '.[dev]'` which deliberately excludes the optional `[embeddings]` extras. tests/embeddings/test_*.py modules import numpy at top level, so pytest collection fails with ModuleNotFoundError before any test can be deselected. Add tests/embeddings/conftest.py with `pytest.importorskip("numpy")` so the entire embeddings test directory skips gracefully when numpy is absent. Once `pip install vouch[embeddings]` (or numpy itself) is present, the skip is a no-op and the tests run normally. * fix(bundle): skip Pydantic validation for opaque source content files `_validate_content` in bundle.py uses the path's first directory component to look up a Pydantic validator. For sources, both `sources/<sha>/meta.yaml` (the Source model) and `sources/<sha>/content` (raw opaque bytes) hit the same "sources" key, so the validator was being run on the raw content bytes and failing with "1 validation error for Source". Add an early return for any non-meta.yaml path under `sources/` so opaque content bytes are not Pydantic-validated. Only the Source metadata file is checked, which matches the original intent of the PR #13 validation. Unblocks three pre-existing test_bundle.py failures inherited from upstream main. * fix(embeddings): MockEmbedder uses uint32 scaling to avoid NaN/Inf Per CodeRabbit on PR #37: decoding sha256 chunks as raw float32 (via struct.unpack("<f", ...)) produced NaN/Inf for ~1 in 2^7 chunks because most 4-byte patterns aren't finite IEEE754 floats. Norm overflow then made unit normalization unreliable and broke cosine similarity asserts in downstream phases (the dedup test in Phase 6 already had to work around this with a custom _HashEmbedder). Decode each chunk as an unsigned 32-bit integer, scale to [-1.0, 1.0], accumulate in float64 for exact norm, and cast to float32 on return. Deterministic, finite, well-behaved. * feat(embeddings): state.db schema for embedding storage + put/get helpers * feat(embeddings): NumPy brute-force cosine search over embedding_index * feat(embeddings): sqlite-vec ANN path with NumPy fallback * feat(embeddings): query embedding LRU cache * style: fix ruff E402/SIM105/E702/I001 violations from Phase 2 storage tasks * fix(embeddings): use outer-query for WHERE on cosine alias in search_embedding Per CodeRabbit Critical on PR #39/40/43: SQLite does not allow a column alias defined in the SELECT list to be referenced in the WHERE clause of the same query. The old form SELECT ... 1.0 - vec_distance_cosine(vec, ?) AS score ... WHERE ... AND score >= ? raised `no such column: score` at runtime; the catch-all `except sqlite3.OperationalError` silently swallowed it and made every call fall through to the NumPy brute-force path, which works but defeats the whole point of installing sqlite-vec. Wrap the projection in a subquery so the WHERE clause sees the alias. No behavior change for callers that have already been using the fallback (NumPy still produces correct results); the ANN code path now actually runs when sqlite-vec is loaded. * feat(embeddings): write-time embedding hook + wire put_claim * feat(embeddings): search_semantic wrapper with query cache * feat(embeddings): hook all six artifact write paths * feat(embeddings): kb_search defaults to embedding primary, fts5 fallback * feat(embeddings): re-embed on update_claim * feat(embeddings): JSONL _h_search parity with MCP semantic-primary dispatch * style: fix ruff SIM105/E501/I001/RUF059 violations from Phase 3 * fix(ci): silence mypy import-untyped for forward-ref fusion import The hybrid backend branch in kb_search (server.py) and _h_search (jsonl_server.py) lazily imports vouch.embeddings.fusion (added in Phase 5). On Phase 4's branch that module doesn't exist yet, so mypy emits import-untyped. Mark each import with `# type: ignore` and let ruff auto-format the line. * fix(ci): silence mypy import-untyped for forward-ref dedup import The KBStore._embed_and_store helper lazily imports vouch.embeddings.dedup (added in Phase 6). On Phase 3's branch that module does not yet exist, so mypy treats it as untyped and errors out. Mark the import with `# type: ignore[import-not-found,import-untyped, unused-ignore]` so the type check passes here and stays silent once Phase 6 lands. * spec: cut dated 2026-05-21 snapshot; promote schema generator to script * ci(mypy): collapse fusion import to single line for type-ignore * feat(embeddings): RRF/weighted/normalized fusion strategies * docs: add 'What ships today' table to README * kb: approve review-gate fact * docs: add 'When to use vouch' section * fix(cli): honor VOUCH_AGENT in _whoami for consistent actor attribution The CLI propose-* commands and most actor sites called _whoami(), which only checked VOUCH_USER and the OS user — VOUCH_AGENT was silently ignored. The MCP and JSONL servers (server.py, jsonl_server.py) and session_start already honour VOUCH_AGENT, so multi-agent attribution broke as soon as you used the CLI to propose. Prefer VOUCH_AGENT over VOUCH_USER over the OS user so the recorded proposed_by / audit actor matches across all three transports. * docs: add captured example session walkthrough Real propose -> review -> commit -> retrieve loop captured from a sandbox run on 2026-05-21. Includes on-disk YAML, audit log lines, and an honest note about the literal substring backend limitation pre-embeddings. * docs: add screen recording of full vouch loop (VHS tape + GIF) * docs: embed demo GIF under Quick start in README * fix(embeddings): address CodeRabbit review on hybrid + fusion + write hook Bundles the substantive correctness fixes flagged by CodeRabbit on PR #41. CI was already green; these are latent bugs the review caught. storage._embed_and_store - Re-embed when the active embedder model changes, not only when the content hash changes. The previous short-circuit kept stale vectors on disk after a model swap, mixing document embeddings from one model with queries from another. - Truly best-effort: catch any exception during encode/put/meta/dedup so a hook failure can't bubble back to the caller and leave an artifact persisted-but-API-errored. index_db.reset - Also clears embedding_index, query_embedding_cache, embedding_dupes, and embedding_* keys from index_meta. Otherwise `vouch index` left orphaned hits behind after artifacts were removed. index_db.search_embedding (Python fallback) - Normalize the stored vector by its own L2 norm before scoring, so rankings match the sqlite-vec `1 - vec_distance_cosine` path. The previous raw dot product made magnitude leak into the score. index_db.search_semantic - Invalidate the query-vector cache entry when the embedder dim no longer matches what's on disk; otherwise a model swap returned stale vectors living in the wrong space. server.kb_search + jsonl_server._h_search (hybrid branch) - Hybrid now passes min_score into search_semantic (consistent with embedding/auto branches) and wraps the FTS lookup in try/except so an FTS5 error doesn't fail the whole request. - JSONL transport rejects unknown backend values with a clear error instead of silently returning []; matches MCP transport behavior. embeddings.fusion - rrf_fuse validates limit >= 0 and k >= 0 (k = -rank would divide by zero). weighted_fuse validates limit >= 0. Tests - Three new fusion guard tests cover the negative-limit and negative-k paths. * fix(lifecycle): only catch missing-artifact errors in cite() the bare except was treating every error as a missing citation, which hid real bugs. narrow it to ArtifactNotFoundError so unexpected failures actually surface. * fix(context): only catch missing-artifact errors in citation lookup same issue as cite() — a bare except was hiding real errors behind an empty citation list. narrow it to ArtifactNotFoundError. * refactor(sessions): keep tracebacks when crystallize() approve fails we were dropping the traceback and only stringifying the error, which made it painful to diagnose anything other than ProposalError. now logs via logger.exception and includes error_type in the failures list. * fix(context): only catch sqlite errors when FTS5 falls back the retrieval helper had a bare except that would also swallow bugs in the substring fallback path or in our own code. narrow it to sqlite3.Error so only the intended cases (FTS5 missing, db missing, schema mismatch) trigger the fallback. * refactor(health): use typed status filter for pending proposal count we were listing every proposal and comparing p.status.value == 'pending' as a string. list_proposals() already takes a ProposalStatus filter — use it. shorter, type-checked, and avoids the stringly-typed compare. * chore(capabilities): pin pluginApi compat baseline, fail CI on host-compat drift * feat(dual-solve): surface engine logs * fix(sessions): make crystallize retry idempotent for summary pages Partial crystallize runs that already wrote session-{id} summary pages failed on retry with "page already exists". Upsert the summary page and derive its artifact list from all approved session proposals. Fixes #139 Co-authored-by: Cursor <cursoragent@cursor.com> * fix(models): reject empty text/name/title on claim/entity/page the non-empty contract for `Claim.text`, `Entity.name`, and `Page.title` lived only in the `propose_*` helpers, so `store.put_*`, `store.update_*`, and bundle/sync import (via `_validate_content`) all silently accepted empty or whitespace-only values and landed artifacts carrying zero of the field's semantic content. add `@field_validator`s on each field, mirroring the existing `Claim.evidence` min-citation validator (#81/#82): they reject blank input at construction time, so every write path — direct construction, put/update, and import — inherits the check at once. the validators gate on emptiness only and preserve surrounding whitespace of non-blank values. `Source.locator` is deliberately left out of scope — its format question is bigger and deserves its own issue, as the report notes. closes #155 * refactor(models): extract shared _require_non_empty validator helper address review on #300: the three #155 field validators (Claim.text / Entity.name / Page.title) were structurally identical strip-checks. extract a module-level `_require_non_empty(v, label)` helper they all delegate to, keeping the per-field error messages. also fix a mid-entry sentence casing in the CHANGELOG. no behaviour change. * fix: resolve RPC traceback leakage on internal errors Unexpected exceptions in handle_request() included full stack traces in the JSON error envelope, exposing paths and internals to HTTP /rpc clients. Log server-side and return message only. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(review): kb.triage_pending — advisory triage scoring for the pending queue a long `kb.list_pending` forces the reviewer to reconstruct, per proposal, whether the claim fits the existing kb, whether its citations resolve, whether it duplicates something already filed, and whether it contradicts an approved claim. this adds an optional triage pass that scores each pending proposal on those four signals and attaches a `_meta.vouch_triage` block (recommendation/score/signals/rationale) to help a reviewer prioritize, without ever deciding anything itself. read-only by construction: the pass never calls proposals.approve/reject, store.put_*, or store.move_proposal_to_decided — a human still calls kb.approve/kb.reject. citation_quality reuses proposals._payload_block_reason; duplication_risk reuses the propose-time embedding similarity path (embeddings.similarity.find_similar_on_propose) and degrades to a difflib heuristic when the embeddings extra isn't installed. fit uses a separate, lower-threshold embedding search so a near-duplicate hit doesn't also inflate fit and cancel out its own duplication penalty. opt-in via `triage.enabled: true` in config.yaml (default false). registered at all four kb.* surface sites (server.py, jsonl_server.py, capabilities.py, cli.py) plus `vouch triage [proposal-id...]` with `--json` and `--reverse`. * feat(diff): register kb.diff at all four kb.* surface sites vouch diff shipped CLI-only in 0.1.0, an explicit non-goal at the time ("MCP/JSONL parity ... kb.* surface unchanged"). that leaves it the only read method skipping the four-site registration convention documented in CLAUDE.md, so agents talking MCP/JSONL have no way to fetch a revision diff. adds the kb_diff MCP tool, the kb.diff JSONL handler, and the capabilities.METHODS entry, next to the other by-id read tools (kb_read_claim/kb_read_page) with the same unrestricted-read posture. also makes new_id optional for a superseded claim: diff_artifacts and the CLI both resolve it from superseded_by when omitted, erroring clearly when there's no successor (pages still require an explicit new_id — they have no successor pointer). closes #327. * fix(jsonl): define module logger after the import block #361 inserted `_log = logging.getLogger(...)` between two import groups in jsonl_server.py. that statement-among-imports trips ruff's E402 on every import that follows it, so `ruff check src tests` — the lint gate in ci — now fails on the whole repo. move the logger definition below the imports (where storage.py already keeps its own module logger); no behaviour change. * feat(adapters): toml_merge install strategy for codex config.toml .codex/config.toml is codex's primary config file, so the plain-copy path silently skipped any project where codex was already configured and vouch never got wired. add a toml_merge entry flag mirroring json_merge: parse the existing destination with tomllib, deep-merge the template's tables into it (existing user values always win on conflict), and write back. flip the codex T1 entry to toml_merge. writing uses a minimal hand-rolled serializer (tables, arrays, inline tables in arrays, scalars, datetimes) so the dependency set stays unchanged; the output must survive a tomllib round-trip back to the merged data, and anything the serializer can't faithfully re-emit degrades to skipped rather than risking the user's config. closes #384 * feat(adapters): codex T2 agents.md fenced snippet codex reads AGENTS.md for project instructions the way cursor does, but the codex adapter stopped at T1, so a codex session got the kb tools with no standing guidance on recall-first or the review gate. ship adapters/codex/AGENTS.md.snippet as a T2 tier with the standard fence markers, kept in lockstep with cursor's snippet modulo the host name (enforced by a sync test). also close the edited-fence gap in _install_fenced per the ticket's acceptance criteria: a fence body that drifted from the shipped snippet is replaced within the markers (reported as merged) instead of being skipped forever, while user content outside the fence stays untouched. a begin marker without an end marker is treated as corrupt and left alone. closes #385 * feat(adapters): codex T3 skills mirroring the vouch slash commands claude-code T3 ships nine slash commands; codex users got none of the guided flows. ship them as codex skills under the project-local .codex/skills/ as a T3 tier. scope decision the ticket asked to resolve first: codex loads custom prompts only from ~/.codex/prompts/ (user-global) and has deprecated them upstream in favour of skills, which do have a project-local home at <project>/.codex/skills/ in trusted projects. the #179 rule forbids a project-scoped install from touching home-directory state, so prompts are out and skills are in — recorded in the manifest comment and the adapter readme, with a manual-copy pointer for users who still want ~/.codex/prompts. the SKILL.md files are referenced from the openclaw mirror rather than duplicated (same reuse pattern openclaw itself applies to the claude-code commands), so one edit updates every host. a parametrized sync test asserts each installed codex skill body equals the matching claude-code command body, and a scope test asserts every installed path stays under the project target. closes #386 * feat(capture): ingest codex session rollouts into review-gated summaries session auto-capture was claude-code-only: hooks drive capture observe and capture finalize live. codex has no hook stream, but it persists every session as a rollout jsonl under $CODEX_HOME/sessions containing user messages, tool calls, and outputs — everything the existing rollup needs, just after the fact. new cli command `vouch capture ingest-codex [<rollout> | --latest]` parses one rollout through a small dedicated parser (codex_rollout.py) that maps function_call records into the same observation shape capture.observe produces — shell commands with failure detection from exit codes, apply_patch heredocs surfaced as file edits, mcp tools under their own names, session mechanics skipped — then reuses the existing build_summary_body -> propose_page rollup. one code path from observation to proposal, two front doors. the rollout format is not a stable public contract: unknown record types are tolerated, and unreadable, compressed, or meta-less files degrade to a CodexRolloutError with an actionable message and a non-zero exit, never a stack trace. re-ingesting a session is a no-op keyed on the rollout's session id; --latest resolves the newest rollout whose recorded cwd matches the current project. proposals are attributed to the codex actor (VOUCH_AGENT wins when set), respect capture's enabled/min_observations config, and never touch approve(). fixture rollouts use placeholder data only, enforced by a test. closes #387 * feat(adapters): codex T4 hook wiring for automatic session capture with `vouch capture ingest-codex` available, codex can self-capture the way claude-code T4 does. the ticket assumed the `notify` setting in config.toml, but codex only honours notify in user-global config — it is a restricted key in project-local layers — and the #179 rule forbids touching ~/.codex. codex's hooks system is the project-local equivalent: `.codex/hooks.json` (loaded in trusted projects) fires Stop when a turn completes, with a payload carrying the session id and transcript path. T4 ships that file through json_merge, so an existing user hooks.json is deep-merged into, never overwritten, and re-runs don't duplicate the hook. the handler is a new --hook mode on ingest-codex: read the stop payload from stdin, resolve the session's rollout (transcript_path when present, else by session id in the rollout filename), ingest idempotently, and exit 0 no matter what — the same never-break-the- host rule capture observe follows. stop fires per turn, not per session end, so the finalize policy the ticket left open is resolved as idempotent re-ingest: the dedup guard now refreshes a session's still-PENDING proposal in place when the rollout grew (same id, updated summary, audit-logged), reports an unchanged rollout as a no-op, and never resurrects a proposal a human already decided. storage gains update_proposal, a pure-I/O in-place rewrite of a pending proposal file. wiring only — everything still lands through propose_page; nothing touches approve(). closes #388 * test(adapters): live codex install gate against the real cli tests/test_openclaw_plugin_load_real.py set the pattern: exercise the real host cli when it's on PATH, skip cleanly where it isn't. the codex adapter had no equivalent, so regressions in the shipped config only surfaced when a user hit them — the old T1 silent-skip no-op is exactly the class of bug a live gate would have caught. the new suite installs the adapter into a temp project at T4, marks the project trusted inside an isolated CODEX_HOME, and asserts through `codex mcp list --json` that the vouch server is visible: next to a pre-seeded unrelated server on the merge path, alone on the fresh path, and absent for an untrusted project (the config must stay inert where codex says it does). a full-tier check covers the fenced AGENTS.md, the skills, and the Stop hook, and an isolation check pins every written artifact under the temp project. assertions target the observable contract — server listed, config parses, snippet present — not codex internals, so version bumps shouldn't break the suite. no network, no credentials, never touches the real ~/.codex; runs in the normal pytest invocation. closes #389 * docs(adapters): codex adapter docs reflect the tiered install three surfaces went stale once the codex tier work landed. the adapters table row described codex as a one-file config.toml snippet; it now lists the tiered install. the codex adapter readme led with the manual ~/.codex edit; it now leads with `vouch install-mcp codex`, explains the project-local .codex/config.toml choice and the trust requirement, summarizes what each tier adds (mcp wire / AGENTS.md snippet / skills / capture hook) with the merge-safety guarantees, and keeps the manual edit as an explicit fallback section. getting-started treated codex as a one-liner aside inside the claude instructions; the install section now shows both hosts side by side. the prompts-location decision from the T3 ticket and the notify restriction from the T4 ticket stay documented in their sections. no code; placeholder examples only. closes #390 * docs(mcp): design a friendlier mcp surface — profiles + auto-recall the closest competitor (pmb) exposes 10 mcp tools by default and hides the rest behind a profile flag; vouch shows all 58 every turn, which is the main first-touch friendliness gap. this spec adds a minimal-by-default tool profile, fusion-by-default retrieval, a per-prompt auto-recall hook, and compact tool descriptions. all of it is presentation-only — the review gate, the yaml storage, and the protocol method surface are untouched. it also closes the current 2-of-3 surface-parity gap as a bonus. * docs(mcp): plan the friendlier-mcp-surface build six TDD tasks: tool profiles (minimal default), real mcp↔methods parity, fusion-by-default retrieval, near-duplicate drop, per-prompt auto-recall hook, and compact tool descriptions. reconcile the spec: minimal is 8 tools (kb_capabilities kept as the discovery escape hatch) and the recency multiplier is deferred (per-hit timestamps aren't plumbed through _retrieve). * feat(mcp): add tool profiles, expose a minimal surface by default agents saw all 58 kb.* tools every turn — the main first-touch friendliness gap vs pmb, which exposes ~10 by default. add a profile layer applied in run_stdio: minimal (8 core tools) by default, standard (16), or full (58), selected by VOUCH_TOOL_PROFILE / config mcp.tool_profile. exposure only — the protocol surface, jsonl/cli, and the review gate are unchanged. * fix(mcp): harden profile resolution against a non-dict config section a bare `mcp:` key in config.yaml parses to None in YAML, and a non-dict mcp section is also possible from hand-edited configs. the previous config.get("mcp", {}).get("tool_profile") chain only supplied the {} default when the key was absent, so a present-but-None or non-dict value passed through and the chained .get raised AttributeError. run_stdio calls resolve_profile_name outside its try/except, so this would crash vouch serve instead of degrading to the minimal default. guard each nesting level with isinstance, matching the established reflex_cfg pattern in salience.py. * test(capabilities): assert mcp tool set matches the method list the parity test only compared capabilities.METHODS to the jsonl handlers; the 58 mcp tools were never checked, so mcp drift passed ci. enumerate the unfiltered server tools and assert they equal METHODS — the real 3-surface check for the mcp side. * feat(retrieval): fuse embedding + fts5 by default instead of a waterfall _retrieve tried embedding, then fts5, then substring, returning the first non-empty list — so lexical and semantic hits never combined. auto and hybrid now fuse both retrievers with the already-built rrf_fuse and tag hits "hybrid"; explicit embedding/fts5/substring pins are unchanged. existing KBs (config says "auto") benefit with no migration. * fix(volunteer): treat hybrid relevance as rank-relative, not pre-normalized normalize_relevance() grouped "hybrid" with "embedding", returning the raw score unclamped-by-batch on the assumption it was already a 0-1 similarity. now that _retrieve's auto/hybrid path tags fused hits "hybrid" with a rrf_fuse score (bounded by ~2/(k+rank), typically 0.01-0.03), that assumption stopped holding: every fused relevance landed far below DEFAULT_THRESHOLD (0.85), so kb.volunteer_context stopped firing for any KB using the new default backend. batch-normalize hybrid like fts5 and substring instead — restores the confidence-gated push channel (#236). * feat(retrieval): drop near-duplicate items from the context pack a fused pack could surface the same fact from two claims. add a cheap greedy jaccard pass (>=0.85 over the first 40 tokens) that keeps the highest-scored of a near-duplicate cluster, so an agent never reads the same thing twice. * fix(retrieval): dedupe by score so the highest-scored near-dup wins _dedupe_near_duplicates assumed its input arrived in descending-score order, which only holds for the plain retrieval path. when build_context_pack runs with expand_graph=True, graph neighbours are appended in bfs order with decayed scores after the sorted main items, so a higher-scored neighbour could be dropped in favor of an earlier, lower-scored main item. sort inside the function so the invariant holds regardless of caller order. * feat(hooks): inject per-prompt kb context via a userpromptsubmit hook recall used to be either a session-start firehose or an explicit tool call. add a pure, never-raising helper + a hidden `vouch context-hook` command that reads the host's prompt payload on stdin and prints an additionalContext envelope, and wire it into the claude-code adapter's UserPromptSubmit hook — so relevant, approved knowledge is injected every turn with zero tool calls. * fix(hooks): never raise on non-dict payload or missing kb a reviewer reproduced two crash paths that violated task 5's own contract that the prompt hook must never block a turn: build_claude_prompt_hook raised AttributeError on non-dict JSON (null/number/bool/array/string all decode fine but lack .get), and the context-hook CLI command called _load_store(), whose sys.exit(2) on a missing KB is a SystemExit that slips past `except Exception` and exits the process nonzero. guard the payload with an isinstance check before .get, log a breadcrumb when build_context_pack fails so a broken KB isn't silently indistinguishable from "no hits", and swap _load_store() for the existing non-exiting _capture_store() helper so the command has no sys.exit on its path at all. * feat(mcp): serve one-line tool descriptions under non-full profiles full docstrings for every exposed tool are paid on every turn. under minimal and standard, trim each tool's description to its first line (full keeps the complete docstrings), cutting the per-turn context cost. * docs(mcp): document tool profiles note VOUCH_TOOL_PROFILE / mcp.tool_profile (default minimal, values minimal|standard|full) in the transports reference and the claude-code adapter guide; this repo has no mintlify/ tree yet so both live under docs/ and adapters/claude-code/ instead. * fix(retrieval): dedupe by score but keep caller order for the pack _dedupe_near_duplicates sorted by score and returned that score-sorted order, which fought graph-expansion: build_context_pack appends decayed-score neighbours after the ranked hits, and fused primary hits now carry small rrf scores. re-sorting let an irrelevant depth-2 neighbour outrank a real match, and the max_chars tail-pop then evicted the real match instead of the neighbour. keep the keep-decision in descending-score order so the highest-scored member of a near-duplicate cluster still survives, but return survivors in the caller's original order so budget eviction drops the tail (appended neighbours), not the ranked hits. * docs(changelog): note minimal mcp profile, fusion, auto-recall summarize this branch's user-visible changes under [Unreleased]: the minimal-by-default mcp tool profile, rrf fusion for auto/hybrid retrieval, and the per-prompt auto-recall hook in the claude-code adapter. * docs(readme): note the fourth hook, per-prompt recall, and lean mcp profile install-mcp now writes a fourth hook (UserPromptSubmit -> vouch context-hook), and recall fires per prompt, not only at session start. also note the kb.* mcp surface defaults to a lean profile that widens via VOUCH_TOOL_PROFILE. * fix(capabilities): read openclaw.compat from package.json (#417) the host-compat drift check (#237) read openclaw.compat.pluginApi from openclaw.plugin.json. that block moved to package.json when the plugin packaging was split, and openclaw.* is now banned from the manifest (enforced by test_manifest_carries_no_dead_dialect_fields). the #237 reader was left pointing at the old, now-empty location, so host_compat silently degraded to {} and both host-compat assertions failed on the test branch — which every open pr targeting test inherited, including #413. repoint _load_host_compat and the test helpers at package.json. the openclaw.compat.pluginApi key path is identical in both files, so this is purely a source-file change with no behaviour difference. * chore(repo): untrack owner-local web/ and .claude/ (#413) these two directories were committed by accident and reach every first-time contributor on clone. neither is project source: - web/ is the netlify-deployed marketing landing page. it is shipped via the netlify cli and is not meant to live in the repo tree. - .claude/ is a personal claude code config: a settings.json with owner-specific hooks, plus command files that are byte-identical duplicates of adapters/claude-code/.claude/commands/. untrack both (files stay on disk) and gitignore them, anchored to the repo root so src/vouch/web and adapters/claude-code/.claude stay tracked. also ignore coverage.xml and the .netlify/ and .playwright-mcp/ tool-scratch dirs. * chore(merge): resolve changelog and version conflicts with main * chore(merge): update changelog to resolve merge conflicts * chore(merge): update to main's latest versions for clean merge * docs(readme): restore incubated by gittensor acknowledgment * chore: remove banner.svg and its reference in README * chore: remove desktop, spec, migrations dirs and pre-commit config * fix(ci): restore missing storage and context constants Add back KB_FORMAT_VERSION, SCHEMA_VERSION, SCHEMA_VERSION_FILENAME, _starter_config to storage.py and _RETRACTED_CLAIM_STATUSES to context.py to fix import errors in test suite. * fix(ci): restore coherent backing modules to match callers the test-branch merges (2ddd1f9 onward) took main's older, smaller storage.py / context.py / index_db.py / cli.py / server.py / health.py / jsonl_server.py / bundle.py / sessions.py while keeping the newer caller modules (lifecycle, proposals, graph, notify, sync, provenance, codex_rollout, triage). that desync left 31 mypy errors — callers referencing methods the truncated backing no longer defined (put_relation_idempotent, _validate_claim_refs, update_page, update_proposal, index_prov_edge, get_meta) — so the test job failed at mypy before pytest ever ran. restore src/ and tests/ to bb06565, the last coherent snapshot, where the fuller backing matches the callers. this also brings back the reindex cli command the recall-eval job invokes (the truncated cli only had index), fixing the second red job. keep __version__ at 1.2.2 so the four version sites stay in lockstep. --------- Co-authored-by: plind-junior <138900956+dripsmvcp@users.noreply.github.com> Co-authored-by: alpurkan17 <alpurkan9@gmail.com> Co-authored-by: Tet-9 <igeparallax@gmail.com> Co-authored-by: Yaroslav98214 <diakovichyaroslav30@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: minion1227 <romantymkiv1999@gmail.com> Co-authored-by: RealDiligent <brave.challenge007@gmail.com> Co-authored-by: jsdevninja <topit89807@gmail.com>
dripsmvcp
pushed a commit
to dripsmvcp/vouch
that referenced
this pull request
Jul 10, 2026
… hook Bundles the substantive correctness fixes flagged by CodeRabbit on PR vouchdev#41. CI was already green; these are latent bugs the review caught. storage._embed_and_store - Re-embed when the active embedder model changes, not only when the content hash changes. The previous short-circuit kept stale vectors on disk after a model swap, mixing document embeddings from one model with queries from another. - Truly best-effort: catch any exception during encode/put/meta/dedup so a hook failure can't bubble back to the caller and leave an artifact persisted-but-API-errored. index_db.reset - Also clears embedding_index, query_embedding_cache, embedding_dupes, and embedding_* keys from index_meta. Otherwise `vouch index` left orphaned hits behind after artifacts were removed. index_db.search_embedding (Python fallback) - Normalize the stored vector by its own L2 norm before scoring, so rankings match the sqlite-vec `1 - vec_distance_cosine` path. The previous raw dot product made magnitude leak into the score. index_db.search_semantic - Invalidate the query-vector cache entry when the embedder dim no longer matches what's on disk; otherwise a model swap returned stale vectors living in the wrong space. server.kb_search + jsonl_server._h_search (hybrid branch) - Hybrid now passes min_score into search_semantic (consistent with embedding/auto branches) and wraps the FTS lookup in try/except so an FTS5 error doesn't fail the whole request. - JSONL transport rejects unknown backend values with a clear error instead of silently returning []; matches MCP transport behavior. embeddings.fusion - rrf_fuse validates limit >= 0 and k >= 0 (k = -rank would divide by zero). weighted_fuse validates limit >= 0. Tests - Three new fusion guard tests cover the negative-limit and negative-k paths.
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.
Summary
Phase 5: enable the
backend="hybrid"code path stubbed out in Phase 4. Adds three fusion strategies — Reciprocal Rank Fusion (default), min-max-normalized weighted blend, and equal-weight normalized — for combining FTS5 and embedding result lists into a single ranked list.Tracks Task 17 in the plan.
Commit
ec626eesrc/vouch/embeddings/fusion.pywithrrf_fuse,weighted_fuse,normalized_fuse; tests intests/embeddings/test_fusion.pyDesign notes
RRF default with k=60 — Cormack et al.'s canonical parameter-free fusion. Robust against score-scale differences between BM25 (negative log-odds) and cosine similarity (
[-1, 1]).Weighted fuse min-max-normalizes each list first, then linearly blends with caller-supplied weights. Useful when one signal is known to be higher-quality for a workload.
Snippet coalescing — fused hits inherit the snippet from whichever input list had one (embedding hits have
""snippets since the backend returns IDs and cosine only; FTS5 supplies the snippet).Test Plan
.venv/bin/pytest tests/embeddings/test_fusion.py -v→ 4 passed (RRF top-of-both-lists, empty inputs, weight dominance, zero-score normalization).venv/bin/pytest tests/embeddings -v→ no regression.venv/bin/python -m ruff check src/vouch/embeddings/fusion.py tests/embeddings/test_fusion.py→ cleanWhat's NOT in this PR
hybridbranch ofkb.search/_h_search(added in Phase 4) — no CLI flag exposes it yet. Phase 8 addsvouch search --backend hybrid.Summary by CodeRabbit
New Features
Tests