fix(vector): address PR review round 14 — accurate offset-skip comment + news_item doc_id guard

Round-14 review surfaced one blocking and one important issue.

context.py: the comment justifying `skip_offset_lookup` claimed
chunk_start/end_offset weren't in _PAYLOAD_INDEX_FIELDS — round 13
indexed both as INTEGER, so the comment now actively misleads. Replace
with the real reason: an indexed chunk_index miss is canonical (both
paths hit the same Qdrant collection), and skipping the offset filter
avoids a redundant round-trip.

verification.py: hoist an is_valid_nextcloud_doc_id guard before the
`int(d)` cast in _verify_news_items, mirroring the boundary-validation
pattern already in _fetch_document_text. Coerce via `str(d)` because
SearchResult.id is `int | str` (D1 forward-compat widening). Malformed
ids now surface as a logger.warning rather than a generic debug line;
fail-open semantics are preserved.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Chris Coutinho
2026-05-10 11:50:46 +02:00
co-authored by Claude Opus 4.7
parent ae23bbe8b8
commit c5020d9629
2 changed files with 17 additions and 8 deletions
@@ -42,6 +42,7 @@ from nextcloud_mcp_server.search.algorithms import (
NextcloudClientProtocol,
SearchResult,
)
from nextcloud_mcp_server.utils.validation import is_valid_nextcloud_doc_id
from nextcloud_mcp_server.vector.eviction import delete_document_points
logger = logging.getLogger(__name__)
@@ -388,6 +389,15 @@ async def _verify_news_items(
# above for why this is narrower than the API-response failure path.
accessible: set[str] = set()
for d in doc_ids:
# SearchResult.id is `int | str` (D1: forward-compat widening). Coerce
# to str so the validator's regex applies consistently to both shapes.
if not is_valid_nextcloud_doc_id(str(d)):
logger.warning(
"Malformed news_item doc_id %r in verifier; keeping (cannot verify)",
d,
)
accessible.add(d)
continue
try:
if int(d) in present_ids:
accessible.add(d)