Round-5 claude-review (merge-ready; all nits):
- 🟡 Added test_empty_doc_types_normalizes_to_null pinning doc_types=[] → None
in record_search_usage metadata (matches the None case).
- 🟡 record_search_usage docstring now notes nc_semantic_search_answer always
meters with doc_types=None (it exposes no doc_types parameter).
- 🟢 BM25HybridSearchAlgorithm.__init__ now sets query_embedding /
query_token_count alongside _embedded_query, so all three cache fields are
instance attributes from construction (was relying on the class-level
SearchAlgorithm defaults).
- 🟢 Ollama embed_batch_with_usage caches _dimension inline (mirrors
OpenAI/Mistral), so the dimension is set via any embed path.
- 🟢 record_indexing_usage documents the independent-record / partial-failure
semantics under SUM aggregation.
Deck #67.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round-4 claude-review findings (no blockers):
- 🟡 Untested server-layer metering hook (raised across rounds): extracted the
nc_semantic_search embeddings_queries recording into a module-level
record_search_usage() helper (mirroring record_indexing_usage) and added
tests/unit/server/test_semantic_metering.py — value = query token count,
flag-off no-op, None token → 0, doc_types metadata bounding, best-effort
failure swallowed.
- 🟡 Dedup-hit skipped metering invisibly: the existing dedup info log now
states "no embedding/usage recorded" so a "fewer embeddings_queries rows than
expected" audit lands on the dedup path directly.
Deferred 🟢 nits (stated on the PR): search 0-token rows are recorded
deliberately (the query embedding ran; zero is a sum no-op) — documented in the
helper; embed_tokens closure locality and the OpenAI embed() dual path are
unchanged (correct as-is / separate refactor).
Deck #67.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
