refactor(search): address PR #750 round 11 review feedback
Three minor fixes from the round-11 review on PR #750: - bm25_hybrid.py:209 — Comment said `doc_id` is `int (notes) or str (files)`, which is backwards. Notes, news_items, and deck_cards are stored as `str` (scanner.py:241, 666, 867); files are stored as `int` (scanner.py:425). Updated to point readers at scanner.py as the source of truth. - verification.py:338 — Lowered the News-API 403/404 log line from `info` to `debug`. The News app being uninstalled or disabled is a predictable operational state (matching the other verifiers' debug-on-not-found paths), so this should not generate operator-dashboard noise. Transient errors immediately below stay at `warning` because they're unexpected. - semantic.py:809 — `nc_get_vector_sync_status` was reading `document_receive_stream` via `getattr(..., None)`, but the attribute is guaranteed-defined on both `AppContext` and `OAuthAppContext` (as a field with `None` default). The defensive `getattr` masked typos that the eviction_task_group access at semantic.py:197-199 deliberately surfaces. Switched to direct access; the `if … is None:` value-check below is preserved (the attribute can legitimately be None before sync starts). Items deliberately deferred (with rationale in the plan file): - News verifier semaphore-hold during get_items (reviewer: "not required here, just worth tracking"; ADR already lists follow-ups). - Hardcoded 2× over-fetch / VERIFICATION_OVERFETCH (TODO already in code). - Integration test for the real Qdrant eviction filter (reviewer marked low-priority; type-preservation chain is unit-tested). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
15ffeca312
commit
1ed8362f78
@@ -804,11 +804,15 @@ def configure_semantic_tools(mcp: FastMCP):
|
||||
)
|
||||
|
||||
try:
|
||||
# Get document receive stream from lifespan context
|
||||
# Get document receive stream from lifespan context. Direct
|
||||
# attribute access matches the eviction_task_group pattern at
|
||||
# ``nc_semantic_search`` (see comment there): both AppContext
|
||||
# and OAuthAppContext define ``document_receive_stream``, so a
|
||||
# missing attribute is a typo that should fail loudly. The
|
||||
# value itself can legitimately be ``None`` before sync starts,
|
||||
# which the check below handles.
|
||||
lifespan_ctx = ctx.request_context.lifespan_context
|
||||
document_receive_stream = getattr(
|
||||
lifespan_ctx, "document_receive_stream", None
|
||||
)
|
||||
document_receive_stream = lifespan_ctx.document_receive_stream
|
||||
|
||||
if document_receive_stream is None:
|
||||
logger.debug(
|
||||
|
||||
Reference in New Issue
Block a user