Addresses the four 🟡 important findings from claude-bot review on PR #773:
str (non-Optional) and the guard would silently skip the Qdrant lookup
for an empty string. Removing the guard matches the type signature.
(`all([…, doc_id, …])` rejects None and empty string, plus
`assert doc_id is not None`). No code change needed.
`get_qdrant_client()` with a module-level `anyio.Lock`. Double-checked
locking keeps the steady-state hot path lock-free. Without this,
parallel cold-start callers could all enter the init block and run
`_backfill_doc_id_to_string` + `_ensure_payload_indexes` redundantly
(idempotent, but noisy). Pattern matches `auth/storage.py:2071`.
behavior with three tests covering the float-warning path (the gap
called out in the review), the str/None silent-skip paths, and the
int-grouping happy path.
Verification:
- ruff check / format: clean
- ty check -- nextcloud_mcp_server: clean
- uv run pytest tests/unit/: 969 passed
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- pdf_highlighter.compute_chunk_bboxes_batch: drop unused chunk_text
destructure (SonarQube finding), and replace positional
page_boundaries[page_num - 1] with a key-based next() match so
reordered or non-1-indexed boundaries can't silently shift the bbox.
Convert touched f-string log to lazy %s formatting.
- vector/processor: rename the trace_operation span from
"vector_sync.generate_highlights" to "vector_sync.compute_chunk_bboxes"
to match what the function actually does.
- Add test_compute_chunk_bboxes_handles_unordered_page_boundaries —
reverses the boundaries list and asserts identical results to the
in-order case, guarding the boundary-lookup regression class.
- Pin pre-push-review skill to sonnet model.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switch the diff range from $BASE..HEAD to $BASE so the review covers
working-tree changes (committed + staged + unstaged), letting the skill
run usefully on in-progress work without requiring a commit first.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>