admin/mcp-nextcloud
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #767 from cbcoutinho/fix/chunk-context-indexed-lookup

Browse Source 
fix(chunk-context): use indexed chunk_index lookup, fix close-after-use bug
This commit is contained in:
Chris Coutinho
2026-05-09 13:35:08 +02:00
committed by GitHub
parent 4b6c556f39 7ef8760d27
commit 797a698274
7 changed files with 1168 additions and 270 deletions
Download Patch File Download Diff File Expand all files Collapse all files
nextcloud_mcp_server/api/visualization.py
+29 -43
View File
@@ -14,7 +14,6 @@ import logging
from typing import Any
import pymupdf
from qdrant_client.models import FieldCondition, Filter, MatchValue
from starlette.requests import Request
from starlette.responses import JSONResponse
@@ -31,13 +30,14 @@ from nextcloud_mcp_server.search import (
BM25HybridSearchAlgorithm,
SemanticSearchAlgorithm,
)
from nextcloud_mcp_server.search.context import get_chunk_with_context
from nextcloud_mcp_server.search.context import (
get_chunk_bbox_and_page_from_qdrant,
get_chunk_with_context,
)
from nextcloud_mcp_server.vector.oauth_sync import (
NotProvisionedError,
get_user_client_basic_auth,
)
from nextcloud_mcp_server.vector.placeholder import get_placeholder_filter
from nextcloud_mcp_server.vector.qdrant_client import get_qdrant_client
from nextcloud_mcp_server.vector.visualization import compute_pca_coordinates
logger = logging.getLogger(__name__)
@@ -479,6 +479,8 @@ async def get_chunk_context(request: Request) -> JSONResponse:
doc_id = request.query_params.get("doc_id")
start_str = request.query_params.get("start")
end_str = request.query_params.get("end")
chunk_index_str = request.query_params.get("chunk_index")
total_chunks_str = request.query_params.get("total_chunks")
# Validate required parameters
if not all([doc_type, doc_id, start_str, end_str]):
@@ -509,6 +511,14 @@ async def get_chunk_context(request: Request) -> JSONResponse:
end = _parse_int_param(end_str, 0, 0, 10000000, "end")
if end <= start:
raise ValueError("end must be greater than start")
chunk_index: int | None = None
if chunk_index_str is not None:
chunk_index = _parse_int_param(
chunk_index_str, 0, 0, 1000000, "chunk_index"
)
total_chunks = _parse_int_param(
total_chunks_str, 1, 1, 1000000, "total_chunks"
)
except ValueError as e:
return JSONResponse({"success": False, "error": str(e)}, status_code=400)
# Convert doc_id to int if possible (most IDs are int)
@@ -541,6 +551,8 @@ async def get_chunk_context(request: Request) -> JSONResponse:
doc_type=doc_type,
chunk_start=start,
chunk_end=end,
chunk_index=chunk_index,
total_chunks=total_chunks,
context_chars=context_chars,
)
@@ -555,49 +567,23 @@ async def get_chunk_context(request: Request) -> JSONResponse:
# For PDF files, also fetch the chunk's bounding box from Qdrant if
# available so the client can overlay a highlight on top of a
# render-on-demand page image (Deck #76).
# render-on-demand page image (Deck #76). Qdrant's page_number is
# trusted over the context-expansion fallback when present.
chunk_bbox = None
page_number = chunk_context.page_number
if doc_type == "file":
try:
settings = get_settings()
qdrant_client = await get_qdrant_client()
points_response = await qdrant_client.scroll(
collection_name=settings.get_collection_name(),
scroll_filter=Filter(
must=[
get_placeholder_filter(),
FieldCondition(
key="doc_id", match=MatchValue(value=doc_id_val)
),
FieldCondition(
key="user_id", match=MatchValue(value=user_id)
),
FieldCondition(
key="chunk_start_offset", match=MatchValue(value=start)
),
FieldCondition(
key="chunk_end_offset", match=MatchValue(value=end)
),
]
),
limit=1,
with_vectors=False,
with_payload=["chunk_bbox", "page_number"],
)
if points_response[0]:
payload = points_response[0][0].payload
if payload:
chunk_bbox = payload.get("chunk_bbox")
# Trust Qdrant page number if available (might be more accurate than context expansion logic)
if payload.get("page_number") is not None:
page_number = payload.get("page_number")
except Exception as e:
logger.warning(f"Failed to fetch chunk bbox: {e}")
qdrant_bbox, qdrant_page = await get_chunk_bbox_and_page_from_qdrant(
user_id=user_id,
doc_id=doc_id_val,
chunk_index=chunk_index,
chunk_start=start,
chunk_end=end,
)
if qdrant_bbox is not None:
chunk_bbox = qdrant_bbox
if qdrant_page is not None:
page_number = qdrant_page
# Build response
response_data = {
nextcloud_mcp_server/auth/viz_routes.py
+45 -50
View File
@@ -18,11 +18,11 @@ from pathlib import Path
import anyio
import numpy as np
from jinja2 import Environment, FileSystemLoader
from qdrant_client.models import FieldCondition, Filter, MatchValue
from starlette.authentication import requires
from starlette.requests import Request
from starlette.responses import HTMLResponse, JSONResponse
from nextcloud_mcp_server.api.management import _parse_int_param
from nextcloud_mcp_server.auth.userinfo_routes import (
_get_authenticated_client_for_userinfo,
)
@@ -33,13 +33,15 @@ from nextcloud_mcp_server.search import (
BM25HybridSearchAlgorithm,
SemanticSearchAlgorithm,
)
from nextcloud_mcp_server.search.context import get_chunk_with_context
from nextcloud_mcp_server.search.context import (
get_chunk_bbox_and_page_from_qdrant,
get_chunk_with_context,
)
from nextcloud_mcp_server.vector.oauth_sync import (
NotProvisionedError,
get_user_client_basic_auth,
)
from nextcloud_mcp_server.vector.pca import PCA
from nextcloud_mcp_server.vector.placeholder import get_placeholder_filter
from nextcloud_mcp_server.vector.qdrant_client import get_qdrant_client
logger = logging.getLogger(__name__)
@@ -535,7 +537,8 @@ async def chunk_context_endpoint(request: Request) -> JSONResponse:
doc_id = request.query_params.get("doc_id")
start_str = request.query_params.get("start")
end_str = request.query_params.get("end")
context_chars = int(request.query_params.get("context", "500"))
chunk_index_str = request.query_params.get("chunk_index")
total_chunks_str = request.query_params.get("total_chunks")
# Validate required parameters
if not all([doc_type, doc_id, start_str, end_str]):
@@ -553,8 +556,23 @@ async def chunk_context_endpoint(request: Request) -> JSONResponse:
assert start_str is not None
assert end_str is not None
start = int(start_str)
end = int(end_str)
context_chars = _parse_int_param(
request.query_params.get("context"),
500,
0,
10000,
"context_chars",
)
start = _parse_int_param(start_str, 0, 0, 10000000, "start")
end = _parse_int_param(end_str, 0, 0, 10000000, "end")
if end <= start:
raise ValueError("end must be greater than start")
chunk_index: int | None = None
if chunk_index_str is not None:
chunk_index = _parse_int_param(
chunk_index_str, 0, 0, 1000000, "chunk_index"
)
total_chunks = _parse_int_param(total_chunks_str, 1, 1, 1000000, "total_chunks")
# Convert doc_id to int (all document types use int IDs)
doc_id_int = int(doc_id)
@@ -584,6 +602,8 @@ async def chunk_context_endpoint(request: Request) -> JSONResponse:
doc_type=doc_type,
chunk_start=start,
chunk_end=end,
chunk_index=chunk_index,
total_chunks=total_chunks,
context_chars=context_chars,
)
@@ -606,50 +626,22 @@ async def chunk_context_endpoint(request: Request) -> JSONResponse:
# For PDF files, also fetch the chunk bbox from Qdrant so the client
# can overlay a highlight on top of a render-on-demand page image
# (Deck #76).
# (Deck #76). Qdrant's page_number is trusted over the
# context-expansion fallback when present.
chunk_bbox = None
page_number = None
page_number = chunk_context.page_number
if doc_type == "file":
try:
settings = get_settings()
qdrant_client = await get_qdrant_client()
username = request.user.display_name
points_response = await qdrant_client.scroll(
collection_name=settings.get_collection_name(),
scroll_filter=Filter(
must=[
get_placeholder_filter(),
FieldCondition(
key="doc_id", match=MatchValue(value=doc_id_int)
),
FieldCondition(
key="user_id", match=MatchValue(value=username)
),
FieldCondition(
key="chunk_start_offset", match=MatchValue(value=start)
),
FieldCondition(
key="chunk_end_offset", match=MatchValue(value=end)
),
]
),
limit=1,
with_vectors=False,
with_payload=["chunk_bbox", "page_number"],
)
points = points_response[0]
if points and points[0].payload:
chunk_bbox = points[0].payload.get("chunk_bbox")
page_number = points[0].payload.get("page_number")
if chunk_bbox:
logger.info(
f"Found chunk bbox: page={page_number}, "
f"rects={len(chunk_bbox)}"
)
except Exception as e:
logger.warning(f"Failed to fetch chunk bbox: {e}")
qdrant_bbox, qdrant_page = await get_chunk_bbox_and_page_from_qdrant(
user_id=user_id,
doc_id=doc_id_int,
chunk_index=chunk_index,
chunk_start=start,
chunk_end=end,
)
if qdrant_bbox is not None:
chunk_bbox = qdrant_bbox
if qdrant_page is not None:
page_number = qdrant_page
# Return response compatible with frontend expectations
response_data: dict = {
@@ -659,16 +651,19 @@ async def chunk_context_endpoint(request: Request) -> JSONResponse:
"after_context": chunk_context.after_context,
"has_more_before": chunk_context.has_before_truncation,
"has_more_after": chunk_context.has_after_truncation,
"page_number": page_number,
"chunk_index": chunk_context.chunk_index,
"total_chunks": chunk_context.total_chunks,
}
if chunk_bbox:
response_data["chunk_bbox"] = chunk_bbox
response_data["page_number"] = page_number
return JSONResponse(response_data)
except ValueError as e:
logger.error(f"Invalid parameter format: {e}")
# User-supplied bad input → 400, not a server error.
logger.warning("Invalid parameter format: %s", e)
return JSONResponse(
{"success": False, "error": f"Invalid parameter format: {e}"},
status_code=400,
nextcloud_mcp_server/search/context.py
+199 -177
View File
@@ -7,13 +7,12 @@ position markers for better visualization and understanding of search results.
import logging
from dataclasses import dataclass
import pymupdf
import pymupdf4llm
from qdrant_client.models import FieldCondition, Filter, MatchValue
from nextcloud_mcp_server.client import NextcloudClient
from nextcloud_mcp_server.config import get_settings
from nextcloud_mcp_server.vector.html_processor import html_to_markdown
from nextcloud_mcp_server.vector.placeholder import get_placeholder_filter
from nextcloud_mcp_server.vector.qdrant_client import get_qdrant_client
logger = logging.getLogger(__name__)
@@ -144,63 +143,6 @@ async def _get_chunk_by_index_from_qdrant(
return None
async def _get_file_path_from_qdrant(
user_id: str, file_id: int, chunk_start: int, chunk_end: int
) -> str | None:
"""Resolve file_id to file_path by querying Qdrant payload.
Args:
user_id: User ID who owns the file
file_id: Numeric file ID
chunk_start: Character offset where chunk starts
chunk_end: Character offset where chunk ends
Returns:
File path string, or None if not found in Qdrant
"""
try:
qdrant_client = await get_qdrant_client()
settings = get_settings()
# Query for the specific chunk
scroll_result = await qdrant_client.scroll(
collection_name=settings.get_collection_name(),
scroll_filter=Filter(
must=[
FieldCondition(key="user_id", match=MatchValue(value=user_id)),
FieldCondition(key="doc_id", match=MatchValue(value=file_id)),
FieldCondition(key="doc_type", match=MatchValue(value="file")),
FieldCondition(
key="chunk_start_offset", match=MatchValue(value=chunk_start)
),
FieldCondition(
key="chunk_end_offset", match=MatchValue(value=chunk_end)
),
]
),
limit=1,
with_payload=["file_path"],
with_vectors=False,
)
if scroll_result[0]:
point = scroll_result[0][0]
file_path = point.payload.get("file_path")
if file_path:
logger.debug(f"Resolved file_id {file_id} to file_path {file_path}")
return str(file_path)
logger.warning(
f"Could not find file_path in Qdrant for file_id {file_id}, "
f"chunk [{chunk_start}:{chunk_end}]"
)
return None
except Exception as e:
logger.error(f"Error querying Qdrant for file_path: {e}", exc_info=True)
return None
async def _get_deck_metadata_from_qdrant(
user_id: str, card_id: int
) -> dict[str, int] | None:
@@ -254,6 +196,98 @@ async def _get_deck_metadata_from_qdrant(
return None
async def get_chunk_bbox_and_page_from_qdrant(
user_id: str,
doc_id: int | str,
chunk_index: int | None,
chunk_start: int,
chunk_end: int,
) -> tuple[list | None, int | None]:
"""Fetch chunk_bbox and page_number for a chunk from Qdrant payload.
Prefers chunk_index for the lookup (always indexed); falls back to
(chunk_start_offset, chunk_end_offset) when chunk_index is not provided
— this is the legacy path for clients pre-cbcoutinho/astrolabe#75. The
fallback may 400 in Qdrant Cloud strict mode because those offset fields
aren't indexed there; that's logged as a warning and (None, None) is
returned so callers degrade gracefully.
Args:
user_id: User ID who owns the document
doc_id: Document ID (int for file/note, str for some doc types)
chunk_index: Zero-based chunk index, or None to use offset fallback
chunk_start: Character offset where chunk starts (used when
chunk_index is None)
chunk_end: Character offset where chunk ends (used when chunk_index
is None)
Returns:
Tuple of (chunk_bbox, page_number); either field may be None
independently if absent from the payload, or both may be None on
miss/error.
"""
try:
settings = get_settings()
qdrant_client = await get_qdrant_client()
if chunk_index is not None:
points_response = await qdrant_client.scroll(
collection_name=settings.get_collection_name(),
scroll_filter=Filter(
must=[
get_placeholder_filter(),
FieldCondition(key="doc_id", match=MatchValue(value=doc_id)),
FieldCondition(key="user_id", match=MatchValue(value=user_id)),
FieldCondition(
key="chunk_index", match=MatchValue(value=chunk_index)
),
]
),
limit=1,
with_vectors=False,
with_payload=["chunk_bbox", "page_number"],
)
else:
points_response = await qdrant_client.scroll(
collection_name=settings.get_collection_name(),
scroll_filter=Filter(
must=[
get_placeholder_filter(),
FieldCondition(key="doc_id", match=MatchValue(value=doc_id)),
FieldCondition(key="user_id", match=MatchValue(value=user_id)),
FieldCondition(
key="chunk_start_offset",
match=MatchValue(value=chunk_start),
),
FieldCondition(
key="chunk_end_offset",
match=MatchValue(value=chunk_end),
),
]
),
limit=1,
with_vectors=False,
with_payload=["chunk_bbox", "page_number"],
)
points = points_response[0]
if not points or not points[0].payload:
return None, None
payload = points[0].payload
chunk_bbox = payload.get("chunk_bbox")
page_number = payload.get("page_number")
if chunk_bbox:
logger.info(
"Found chunk bbox: page=%s, rects=%d", page_number, len(chunk_bbox)
)
return chunk_bbox, page_number
except Exception as e:
logger.warning("Failed to fetch chunk bbox: %s", e)
return None, None
@dataclass
class ChunkContext:
"""Expanded chunk with surrounding context and position markers.
@@ -265,7 +299,10 @@ class ChunkContext:
chunk_start_offset: Character position where chunk starts in document
chunk_end_offset: Character position where chunk ends in document
page_number: Page number for PDFs (None for other doc types)
chunk_index: Zero-based chunk index (N in "chunk N of M")
chunk_index: Zero-based chunk index (N in "chunk N of M"). None when
the caller didn't supply chunk_index and we couldn't determine it
from the lookup path — distinguishes "unknown position" from
"actually chunk 0".
total_chunks: Total number of chunks in document
marked_text: Full text with position markers around the chunk
has_before_truncation: True if before_context was truncated
@@ -278,7 +315,7 @@ class ChunkContext:
chunk_start_offset: int
chunk_end_offset: int
page_number: int | None
chunk_index: int
chunk_index: int | None
total_chunks: int
marked_text: str
has_before_truncation: bool
@@ -293,7 +330,7 @@ async def get_chunk_with_context(
chunk_start: int,
chunk_end: int,
page_number: int | None = None,
chunk_index: int = 0,
chunk_index: int | None = None,
total_chunks: int = 1,
context_chars: int = 300,
) -> ChunkContext | None:
@@ -311,7 +348,9 @@ async def get_chunk_with_context(
chunk_start: Character offset where chunk starts
chunk_end: Character offset where chunk ends
page_number: Optional page number for PDFs
chunk_index: Zero-based chunk index in document
chunk_index: Zero-based chunk index in document. When provided, used as
the primary Qdrant lookup key (uses the always-indexed chunk_index
field). When None, falls back to the (chunk_start, chunk_end) lookup.
total_chunks: Total number of chunks in document
context_chars: Number of characters to include before/after chunk
@@ -326,27 +365,46 @@ async def get_chunk_with_context(
else (doc_id if isinstance(doc_id, int) else None)
)
# Try to get chunk from Qdrant first (fast path)
# Try to get chunk from Qdrant (fast path).
# Prefer chunk_index lookup (always-indexed field) when caller supplied it;
# fall back to (chunk_start, chunk_end) lookup otherwise.
chunk_text: str | None = None
if doc_id_int is not None:
chunk_text = await _get_chunk_from_qdrant(
user_id, doc_id_int, doc_type, chunk_start, chunk_end
)
if chunk_text:
logger.info(
f"Retrieved chunk from Qdrant cache for {doc_type} {doc_id} "
f"(avoids document re-fetch/re-parse)"
if chunk_index is not None:
chunk_text = await _get_chunk_by_index_from_qdrant(
user_id, doc_id_int, doc_type, chunk_index
)
# Skip the offset fallback for files when the indexed chunk_index
# lookup already ran: chunk_start/end_offset aren't indexed in Qdrant
# Cloud strict mode, so the call returns 400 and surfaces a misleading
# logger.error. The file fast-fail below correctly handles the miss
# without it.
skip_offset_lookup = chunk_index is not None and doc_type == "file"
if chunk_text is None and not skip_offset_lookup:
chunk_text = await _get_chunk_from_qdrant(
user_id, doc_id_int, doc_type, chunk_start, chunk_end
)
# Fetch adjacent chunks for context expansion
# Get chunk overlap from config to remove duplicate text
settings = get_settings()
chunk_overlap = settings.document_chunk_overlap
if chunk_text:
# chunk_text can only be non-None inside the `if doc_id_int is not None:`
# block above, so doc_id_int is guaranteed non-None here. Narrow for ty.
assert doc_id_int is not None
logger.info(
f"Retrieved chunk from Qdrant cache for {doc_type} {doc_id} "
f"(avoids document re-fetch/re-parse)"
)
before_context = ""
after_context = ""
has_before_truncation = False
has_after_truncation = False
# Fetch adjacent chunks for context expansion
# Get chunk overlap from config to remove duplicate text
settings = get_settings()
chunk_overlap = settings.document_chunk_overlap
before_context = ""
after_context = ""
has_before_truncation = False
has_after_truncation = False
if chunk_index is not None:
# Fetch previous chunk if not first chunk
if chunk_index > 0:
before_chunk = await _get_chunk_by_index_from_qdrant(
@@ -388,58 +446,62 @@ async def get_chunk_with_context(
else:
# Could not fetch next chunk, but we're not at end
has_after_truncation = True
else:
# No chunk_index → can't fetch adjacent chunks via index arithmetic
# without risking wrong neighbours (a default of 0 would query the
# chunks at positions -1 and 1 even when the actual chunk is, say,
# 5/20). Mark both sides as truncated so the caller knows context
# wasn't expanded.
has_before_truncation = True
has_after_truncation = True
marked_text = _insert_position_markers(
before_context=before_context,
chunk_text=chunk_text,
after_context=after_context,
page_number=page_number,
chunk_index=chunk_index,
total_chunks=total_chunks,
has_before_truncation=has_before_truncation,
has_after_truncation=has_after_truncation,
)
return ChunkContext(
chunk_text=chunk_text,
before_context=before_context,
after_context=after_context,
chunk_start_offset=chunk_start,
chunk_end_offset=chunk_end,
page_number=page_number,
chunk_index=chunk_index,
total_chunks=total_chunks,
marked_text=marked_text,
has_before_truncation=has_before_truncation,
has_after_truncation=has_after_truncation,
)
marked_text = _insert_position_markers(
before_context=before_context,
chunk_text=chunk_text,
after_context=after_context,
page_number=page_number,
chunk_index=chunk_index,
total_chunks=total_chunks,
has_before_truncation=has_before_truncation,
has_after_truncation=has_after_truncation,
)
return ChunkContext(
chunk_text=chunk_text,
before_context=before_context,
after_context=after_context,
chunk_start_offset=chunk_start,
chunk_end_offset=chunk_end,
page_number=page_number,
chunk_index=chunk_index,
total_chunks=total_chunks,
marked_text=marked_text,
has_before_truncation=has_before_truncation,
has_after_truncation=has_after_truncation,
)
# Fallback: Fetch full document and extract chunk with context.
# For files this path requires downloading and re-parsing the PDF, which
# routinely exceeds 30s on large documents. Skip it: if the chunk wasn't
# found by chunk_index OR offsets, re-parsing the PDF won't find it either
# (the chunk has been removed or re-indexed with different offsets).
if doc_type == "file":
logger.warning(
"Chunk not found in Qdrant for file %s (chunk_index=%s, "
"offsets=%s-%s); skipping slow PDF re-parse fallback",
doc_id,
chunk_index,
chunk_start,
chunk_end,
)
return None
# Fallback: Fetch full document and extract chunk with context
# This path is taken for:
# 1. Legacy data with truncated excerpts in Qdrant
# 2. Failed Qdrant queries
logger.info(
f"Falling back to document fetch for {doc_type} {doc_id} "
f"(Qdrant cache miss, possibly legacy data)"
)
# For files, retrieve file_path from Qdrant payload
resolved_doc_id = doc_id
if doc_type == "file" and isinstance(doc_id, int):
file_path = await _get_file_path_from_qdrant(
user_id, doc_id, chunk_start, chunk_end
)
if not file_path:
logger.warning(
f"Could not resolve file_id {doc_id} to file_path from Qdrant"
)
return None
resolved_doc_id = file_path
logger.debug(f"Resolved file_id {doc_id} to file_path {file_path}")
# Fetch full document text
full_text = await _fetch_document_text(
nc_client, resolved_doc_id, doc_type, user_id
)
# Fetch full document text (notes, deck cards, news items, etc.)
full_text = await _fetch_document_text(nc_client, doc_id, doc_type, user_id)
if full_text is None:
logger.warning(
f"Could not fetch document text for {doc_type} {doc_id}, "
@@ -502,10 +564,14 @@ async def _fetch_document_text(
) -> str | None:
"""Fetch full text content of a document.
Note: doc_type=="file" is short-circuited in get_chunk_with_context before
this function is called (re-parsing PDFs is too slow for the request
timeout), so no file branch exists here.
Args:
nc_client: Authenticated Nextcloud client
doc_id: Document ID (note ID or file path)
doc_type: Type of document ("note", "file", etc.)
doc_id: Document ID
doc_type: Type of document ("note", "news_item", "deck_card")
Returns:
Full document text, or None if document cannot be retrieved
@@ -519,54 +585,6 @@ async def _fetch_document_text(
title = note.get("title", "")
content = note.get("content", "")
return f"{title}\n\n{content}"
elif doc_type == "file":
# Fetch file content via WebDAV
try:
file_path = str(doc_id)
file_content, content_type = await nc_client.webdav.read_file(file_path)
# Check if it's a PDF (by content type or file extension)
is_pdf = (
content_type and "pdf" in content_type.lower()
) or file_path.lower().endswith(".pdf")
if is_pdf:
# Extract text from PDF using PyMuPDF
# IMPORTANT: Use pymupdf4llm.to_markdown() to match indexing extraction
# This ensures character offsets align between indexed chunks and retrieval
logger.debug(f"Extracting text from PDF: {file_path}")
pdf_doc = pymupdf.open(stream=file_content, filetype="pdf")
text_parts = []
# Extract each page as markdown (same as indexing)
for page_num in range(pdf_doc.page_count):
page_md = pymupdf4llm.to_markdown(
pdf_doc,
pages=[page_num],
write_images=False, # Don't need images for context
page_chunks=False,
)
text_parts.append(page_md)
pdf_doc.close()
# Join pages (no separator - matches indexing)
full_text = "".join(text_parts)
logger.debug(
f"Extracted {len(full_text)} characters from "
f"{pdf_doc.page_count} pages in {file_path}"
)
return full_text
else:
# Assume it's a text file, decode to string
logger.debug(f"Decoding text file: {file_path}")
return file_content.decode("utf-8", errors="replace")
except Exception as e:
logger.error(
f"Error fetching file content for {doc_id}: {e}", exc_info=True
)
return None
elif doc_type == "news_item":
# Fetch news item by ID
item = await nc_client.news.get_item(int(doc_id))
@@ -668,7 +686,7 @@ def _insert_position_markers(
chunk_text: str,
after_context: str,
page_number: int | None,
chunk_index: int,
chunk_index: int | None,
total_chunks: int,
has_before_truncation: bool,
has_after_truncation: bool,
@@ -683,7 +701,8 @@ def _insert_position_markers(
chunk_text: The matched chunk
after_context: Text after chunk
page_number: Optional page number
chunk_index: Zero-based chunk index
chunk_index: Zero-based chunk index, or None when the caller didn't
supply it (rendered as "Chunk ?/N" instead of "Chunk 0/N").
total_chunks: Total chunks in document
has_before_truncation: Whether before_context is truncated
has_after_truncation: Whether after_context is truncated
@@ -695,7 +714,10 @@ def _insert_position_markers(
position_parts = []
if page_number is not None:
position_parts.append(f"Page {page_number}")
position_parts.append(f"Chunk {chunk_index + 1} of {total_chunks}")
if chunk_index is None:
position_parts.append(f"Chunk ?/{total_chunks}")
else:
position_parts.append(f"Chunk {chunk_index + 1} of {total_chunks}")
position_metadata = ", ".join(position_parts)
# Build marked text
tests/unit/test_chunk_bbox_helper.py
+215
View File
@@ -0,0 +1,215 @@
"""Unit tests for
`nextcloud_mcp_server.search.context.get_chunk_bbox_and_page_from_qdrant`.
Covers the two paths the helper handles:
- Indexed lookup via `chunk_index` (the preferred path post
cbcoutinho/astrolabe#75)
- Legacy offset fallback via `(chunk_start_offset, chunk_end_offset)`, which
may 400 in Qdrant Cloud strict mode
Plus the regression case from PR #767 review: when the payload has
`chunk_bbox` but no `page_number`, the helper must surface that as
`(bbox, None)` so callers can preserve their context-derived page_number
fallback.
"""
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
# Import via the auth surface first to side-step the known
# `nextcloud_mcp_server.search.__init__` circular-init issue (same workaround
# used in test_chunk_context_offset_gate.py).
import nextcloud_mcp_server.auth.viz_routes # noqa: F401
from nextcloud_mcp_server.search import context as context_module
from nextcloud_mcp_server.search.context import get_chunk_bbox_and_page_from_qdrant
pytestmark = pytest.mark.unit
def _make_point(payload: dict) -> MagicMock:
point = MagicMock()
point.payload = payload
return point
def _patch_qdrant(scroll_return=None, scroll_side_effect=None):
qdrant_client = MagicMock()
if scroll_side_effect is not None:
qdrant_client.scroll = AsyncMock(side_effect=scroll_side_effect)
else:
qdrant_client.scroll = AsyncMock(return_value=scroll_return)
return patch.object(
context_module,
"get_qdrant_client",
new_callable=AsyncMock,
return_value=qdrant_client,
), qdrant_client
class TestIndexedPath:
"""When `chunk_index` is supplied, the helper must use the indexed
`chunk_index` filter (not the offset fallback)."""
async def test_returns_bbox_and_page_when_payload_complete(self):
bbox = [[0, 0, 100, 50]]
point = _make_point({"chunk_bbox": bbox, "page_number": 7})
ctx, qdrant_client = _patch_qdrant(scroll_return=([point], None))
with ctx:
result = await get_chunk_bbox_and_page_from_qdrant(
user_id="alice",
doc_id=42,
chunk_index=3,
chunk_start=0,
chunk_end=100,
)
assert result == (bbox, 7)
# One scroll call, and the filter must include chunk_index (not offsets)
qdrant_client.scroll.assert_awaited_once()
scroll_kwargs = qdrant_client.scroll.await_args.kwargs
filter_keys = [c.key for c in scroll_kwargs["scroll_filter"].must]
assert "chunk_index" in filter_keys
assert "chunk_start_offset" not in filter_keys
assert "chunk_end_offset" not in filter_keys
class TestOffsetFallbackPath:
"""When `chunk_index` is None, the helper must use the offset filter."""
async def test_returns_bbox_and_page_when_payload_complete(self):
bbox = [[10, 20, 110, 70]]
point = _make_point({"chunk_bbox": bbox, "page_number": 2})
ctx, qdrant_client = _patch_qdrant(scroll_return=([point], None))
with ctx:
result = await get_chunk_bbox_and_page_from_qdrant(
user_id="bob",
doc_id=99,
chunk_index=None,
chunk_start=500,
chunk_end=600,
)
assert result == (bbox, 2)
scroll_kwargs = qdrant_client.scroll.await_args.kwargs
filter_keys = [c.key for c in scroll_kwargs["scroll_filter"].must]
assert "chunk_start_offset" in filter_keys
assert "chunk_end_offset" in filter_keys
assert "chunk_index" not in filter_keys
async def test_strict_mode_400_returns_none_pair_and_warns(self, caplog):
"""Qdrant Cloud strict mode 400s on unindexed offset filters; the
helper must swallow the exception, log a warning, and degrade
gracefully so the route can still return chunk text."""
ctx, _ = _patch_qdrant(scroll_side_effect=Exception("strict mode: 400"))
with ctx, caplog.at_level("WARNING"):
result = await get_chunk_bbox_and_page_from_qdrant(
user_id="bob",
doc_id=99,
chunk_index=None,
chunk_start=0,
chunk_end=100,
)
assert result == (None, None)
assert any("Failed to fetch chunk bbox" in r.message for r in caplog.records)
class TestPayloadShape:
"""Each payload field can be missing independently — callers rely on
that to decide whether to overwrite their fallback values."""
async def test_empty_points_returns_none_pair(self):
ctx, _ = _patch_qdrant(scroll_return=([], None))
with ctx:
result = await get_chunk_bbox_and_page_from_qdrant(
user_id="alice",
doc_id=1,
chunk_index=0,
chunk_start=0,
chunk_end=10,
)
assert result == (None, None)
async def test_missing_page_returns_bbox_only(self):
"""Regression for PR #767 review issue #1: when Qdrant returns a
point whose payload lacks `page_number`, the helper must return
`(bbox, None)` so callers preserve their `chunk_context.page_number`
fallback rather than clobbering it to None."""
bbox = [[0, 0, 100, 50]]
point = _make_point({"chunk_bbox": bbox}) # no page_number
ctx, _ = _patch_qdrant(scroll_return=([point], None))
with ctx:
result = await get_chunk_bbox_and_page_from_qdrant(
user_id="alice",
doc_id=42,
chunk_index=3,
chunk_start=0,
chunk_end=100,
)
assert result == (bbox, None)
async def test_missing_bbox_returns_page_only(self):
point = _make_point({"page_number": 5}) # no chunk_bbox
ctx, _ = _patch_qdrant(scroll_return=([point], None))
with ctx:
result = await get_chunk_bbox_and_page_from_qdrant(
user_id="alice",
doc_id=42,
chunk_index=3,
chunk_start=0,
chunk_end=100,
)
assert result == (None, 5)
async def test_empty_payload_returns_none_pair(self):
point = _make_point({})
ctx, _ = _patch_qdrant(scroll_return=([point], None))
with ctx:
result = await get_chunk_bbox_and_page_from_qdrant(
user_id="alice",
doc_id=42,
chunk_index=3,
chunk_start=0,
chunk_end=100,
)
assert result == (None, None)
async def test_falsy_payload_treated_as_no_point(self):
"""`if not points[0].payload` short-circuits when payload is None or
an empty dict, mirroring the original guards in the route handlers."""
point = MagicMock()
point.payload = None
ctx, _ = _patch_qdrant(scroll_return=([point], None))
with ctx:
result = await get_chunk_bbox_and_page_from_qdrant(
user_id="alice",
doc_id=42,
chunk_index=3,
chunk_start=0,
chunk_end=100,
)
assert result == (None, None)
class TestExceptionHandling:
"""Any error from Qdrant must produce `(None, None)` — never propagate."""
async def test_indexed_path_exception_returns_none_pair(self, caplog):
ctx, _ = _patch_qdrant(scroll_side_effect=RuntimeError("qdrant unavailable"))
with ctx, caplog.at_level("WARNING"):
result = await get_chunk_bbox_and_page_from_qdrant(
user_id="alice",
doc_id=42,
chunk_index=3,
chunk_start=0,
chunk_end=100,
)
assert result == (None, None)
assert any("Failed to fetch chunk bbox" in r.message for r in caplog.records)
tests/unit/test_chunk_context_offset_gate.py
+391
View File
@@ -0,0 +1,391 @@
"""Unit tests for `nextcloud_mcp_server.search.context.get_chunk_with_context`.
Focused on the chunk-lookup gate that decides whether to fall back from the
indexed `chunk_index` path to the unindexed `(chunk_start, chunk_end)` path.
The behaviour matters because Qdrant Cloud's strict mode rejects filters on
unindexed fields with HTTP 400 — a fall-through there surfaces a misleading
`logger.error` even when the caller's request would correctly resolve as a
404 via the file fast-fail.
"""
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
# Import via the auth surface first to side-step a known circular-init issue
# in `nextcloud_mcp_server.search.__init__` when `search` is imported as the
# first entry point (also affects pre-existing tests under tests/unit/search/).
import nextcloud_mcp_server.auth.viz_routes # noqa: F401 (init-order fixup)
from nextcloud_mcp_server.search import context as context_module
from nextcloud_mcp_server.search.context import get_chunk_with_context
pytestmark = pytest.mark.unit
@pytest.fixture
def mock_nc_client() -> MagicMock:
return MagicMock()
class TestOffsetFallbackGate:
"""When chunk_index is provided AND doc_type=='file', the offset fallback
must be skipped — see PR #767 review (🟡 spurious Qdrant error log).
"""
async def test_file_with_chunk_index_skips_offset_fallback_on_miss(
self, mock_nc_client
):
with (
patch.object(
context_module,
"_get_chunk_by_index_from_qdrant",
new_callable=AsyncMock,
return_value=None,
) as mock_indexed,
patch.object(
context_module,
"_get_chunk_from_qdrant",
new_callable=AsyncMock,
return_value="should-not-be-returned",
) as mock_offset,
):
result = await get_chunk_with_context(
nc_client=mock_nc_client,
user_id="alice",
doc_id=12345,
doc_type="file",
chunk_start=0,
chunk_end=100,
chunk_index=3,
total_chunks=20,
)
assert result is None, "file fast-fail must return None on Qdrant miss"
mock_indexed.assert_awaited_once()
mock_offset.assert_not_awaited()
async def test_note_with_chunk_index_still_uses_offset_fallback(
self, mock_nc_client
):
"""Notes/deck cards keep the offset fallback (cheap, useful for legacy
data): the gate is file-specific.
"""
with (
patch.object(
context_module,
"_get_chunk_by_index_from_qdrant",
new_callable=AsyncMock,
return_value=None,
) as mock_indexed,
patch.object(
context_module,
"_get_chunk_from_qdrant",
new_callable=AsyncMock,
return_value=None,
) as mock_offset,
patch.object(
context_module,
"_fetch_document_text",
new_callable=AsyncMock,
return_value=None,
),
):
await get_chunk_with_context(
nc_client=mock_nc_client,
user_id="alice",
doc_id=42,
doc_type="note",
chunk_start=0,
chunk_end=10,
chunk_index=2,
total_chunks=5,
)
mock_indexed.assert_awaited_once()
mock_offset.assert_awaited_once()
async def test_file_without_chunk_index_uses_offset_fallback(self, mock_nc_client):
"""Files with no chunk_index supplied still use the offset path —
the gate only kicks in once the indexed lookup has been attempted.
"""
with (
patch.object(
context_module,
"_get_chunk_by_index_from_qdrant",
new_callable=AsyncMock,
return_value=None,
) as mock_indexed,
patch.object(
context_module,
"_get_chunk_from_qdrant",
new_callable=AsyncMock,
return_value=None,
) as mock_offset,
):
await get_chunk_with_context(
nc_client=mock_nc_client,
user_id="alice",
doc_id=12345,
doc_type="file",
chunk_start=0,
chunk_end=100,
chunk_index=None,
total_chunks=20,
)
mock_indexed.assert_not_awaited()
mock_offset.assert_awaited_once()
class TestNullableChunkIndexPropagation:
"""When the caller doesn't supply chunk_index, it must propagate as None
through to ChunkContext and the position markers — distinguishing
"unknown position" from "actually chunk 0". See PR #767 review (🟡 issue 2).
"""
async def test_fast_path_without_chunk_index_returns_none_in_response(
self, mock_nc_client
):
"""Note retrieved via offset fallback (chunk_index=None) → response
chunk_index is None, markers render '?/N', and adjacent fetch is
skipped (would otherwise produce wrong neighbours from index 0).
"""
with (
patch.object(
context_module,
"_get_chunk_by_index_from_qdrant",
new_callable=AsyncMock,
return_value=None,
) as mock_indexed,
patch.object(
context_module,
"_get_chunk_from_qdrant",
new_callable=AsyncMock,
return_value="matched chunk text",
),
):
result = await get_chunk_with_context(
nc_client=mock_nc_client,
user_id="alice",
doc_id=42,
doc_type="note",
chunk_start=100,
chunk_end=200,
chunk_index=None,
total_chunks=8,
)
assert result is not None
assert result.chunk_index is None, (
"chunk_index must propagate as None, not default to 0"
)
assert "Chunk ?/8" in result.marked_text
assert "Chunk 1 of 8" not in result.marked_text
# Adjacent fetch must be skipped — index arithmetic from 0 would
# query the wrong neighbours when actual position isn't 0.
mock_indexed.assert_not_awaited()
assert result.has_before_truncation is True
assert result.has_after_truncation is True
async def test_fast_path_with_chunk_index_renders_position_correctly(
self, mock_nc_client
):
"""Counter-positive: when chunk_index is supplied, response carries
the value and markers render the explicit "Chunk N of M".
"""
with (
patch.object(
context_module,
"_get_chunk_by_index_from_qdrant",
new_callable=AsyncMock,
side_effect=[
"current chunk text", # primary lookup
"previous chunk text", # adjacent before
"next chunk text", # adjacent after
],
),
patch.object(
context_module,
"_get_chunk_from_qdrant",
new_callable=AsyncMock,
return_value=None,
),
):
result = await get_chunk_with_context(
nc_client=mock_nc_client,
user_id="alice",
doc_id=42,
doc_type="note",
chunk_start=0,
chunk_end=10,
chunk_index=5,
total_chunks=20,
)
assert result is not None
assert result.chunk_index == 5
assert "Chunk 6 of 20" in result.marked_text
async def test_doc_text_fallback_without_chunk_index_returns_none(
self, mock_nc_client
):
"""Doc-text fallback (Qdrant miss → re-fetch document) must also
propagate chunk_index=None into the response so callers can tell
the position is unknown.
"""
with (
patch.object(
context_module,
"_get_chunk_by_index_from_qdrant",
new_callable=AsyncMock,
return_value=None,
),
patch.object(
context_module,
"_get_chunk_from_qdrant",
new_callable=AsyncMock,
return_value=None,
),
patch.object(
context_module,
"_fetch_document_text",
new_callable=AsyncMock,
return_value="x" * 500,
),
):
result = await get_chunk_with_context(
nc_client=mock_nc_client,
user_id="alice",
doc_id=42,
doc_type="note",
chunk_start=100,
chunk_end=200,
chunk_index=None,
total_chunks=10,
)
assert result is not None
assert result.chunk_index is None
assert "Chunk ?/10" in result.marked_text
class TestAdjacentChunkBoundary:
"""Boundary cases for the `chunk_index > 0` / `chunk_index < total_chunks - 1`
gates that decide whether to fetch the previous / next chunk via Qdrant.
See PR #767 review (🟡 missing boundary tests).
"""
async def test_first_chunk_skips_before_fetch_only(self, mock_nc_client):
"""At chunk_index=0 the before-fetch gate is closed (no previous
chunk exists) but the after-fetch still runs.
"""
with (
patch.object(
context_module,
"_get_chunk_by_index_from_qdrant",
new_callable=AsyncMock,
side_effect=[
"current chunk text", # primary lookup
"next chunk text", # adjacent after only
],
) as mock_indexed,
patch.object(
context_module,
"_get_chunk_from_qdrant",
new_callable=AsyncMock,
return_value=None,
),
):
result = await get_chunk_with_context(
nc_client=mock_nc_client,
user_id="alice",
doc_id=42,
doc_type="note",
chunk_start=0,
chunk_end=10,
chunk_index=0,
total_chunks=10,
)
assert result is not None
assert result.chunk_index == 0
assert result.has_before_truncation is False
assert result.has_after_truncation is False
assert mock_indexed.await_count == 2, (
"expected primary lookup + after-fetch only (no before-fetch at index 0)"
)
assert "Chunk 1 of 10" in result.marked_text
async def test_last_chunk_skips_after_fetch_only(self, mock_nc_client):
"""At chunk_index=total_chunks-1 the after-fetch gate is closed (no
next chunk exists) but the before-fetch still runs.
"""
with (
patch.object(
context_module,
"_get_chunk_by_index_from_qdrant",
new_callable=AsyncMock,
side_effect=[
"current chunk text", # primary lookup
"previous chunk text", # adjacent before only
],
) as mock_indexed,
patch.object(
context_module,
"_get_chunk_from_qdrant",
new_callable=AsyncMock,
return_value=None,
),
):
result = await get_chunk_with_context(
nc_client=mock_nc_client,
user_id="alice",
doc_id=42,
doc_type="note",
chunk_start=0,
chunk_end=10,
chunk_index=9,
total_chunks=10,
)
assert result is not None
assert result.chunk_index == 9
assert result.has_before_truncation is False
assert result.has_after_truncation is False
assert mock_indexed.await_count == 2, (
"expected primary lookup + before-fetch only (no after-fetch at last index)"
)
assert "Chunk 10 of 10" in result.marked_text
class TestPositionMarkers:
"""Direct tests for `_insert_position_markers` rendering when chunk_index
is None vs explicit.
"""
def test_marker_renders_question_mark_when_chunk_index_is_none(self):
text = context_module._insert_position_markers(
before_context="",
chunk_text="x",
after_context="",
page_number=None,
chunk_index=None,
total_chunks=12,
has_before_truncation=False,
has_after_truncation=False,
)
assert "Chunk ?/12" in text
def test_marker_renders_explicit_index_when_supplied(self):
text = context_module._insert_position_markers(
before_context="",
chunk_text="x",
after_context="",
page_number=3,
chunk_index=4,
total_chunks=12,
has_before_truncation=False,
has_after_truncation=False,
)
assert "Page 3" in text
assert "Chunk 5 of 12" in text
tests/unit/test_management_chunk_context_endpoint.py
+95
View File
@@ -258,6 +258,101 @@ class TestChunkContextCredentialPath:
assert data["success"] is False
assert "failed to fetch chunk context" in data["error"].lower()
def test_file_doc_type_qdrant_miss_yields_fast_404(self):
"""For doc_type=file, a Qdrant miss must surface as 404 immediately
(no slow PDF re-parse fallback). Locks the proxy-timeout fix in.
At the unit level we only assert the response shape; the
no-fallback contract itself lives in `search/context.py` and is
exercised by chunk-context tests there.
"""
mock_nc_client = _make_mock_nc_client()
with (
patch(
"nextcloud_mcp_server.api.visualization.validate_token_and_get_user",
new_callable=AsyncMock,
return_value=("testuser", True),
),
patch(
"nextcloud_mcp_server.api.visualization.get_user_client_basic_auth",
new_callable=AsyncMock,
return_value=mock_nc_client,
),
patch(
"nextcloud_mcp_server.api.visualization.get_chunk_with_context",
new_callable=AsyncMock,
return_value=None,
) as mock_get_chunk,
):
app = create_test_app()
client = TestClient(app)
response = client.get(
"/api/v1/chunk-context?doc_type=file&doc_id=12345"
"&start=0&end=10&chunk_index=3&total_chunks=20",
headers={"Authorization": "Bearer test-token"},
)
assert response.status_code == 404
data = response.json()
assert data["success"] is False
# Confirm the handler called the resolver with doc_type=file
# (not a coerced/normalized value) so the fast-fail path engages.
kwargs = mock_get_chunk.await_args.kwargs
assert kwargs["doc_type"] == "file"
assert kwargs["chunk_index"] == 3
class TestChunkContextParameterForwarding:
"""Verify new chunk_index / total_chunks query params reach the lookup.
Regression guard for PR #767: the whole point of the fix is that callers
pass chunk_index, and it must arrive at get_chunk_with_context as the
primary Qdrant lookup key.
"""
def test_chunk_index_and_total_chunks_forwarded(self):
mock_nc_client = _make_mock_nc_client()
mock_ctx = _make_mock_chunk_context()
mock_ctx.chunk_index = 7
mock_ctx.total_chunks = 10
with (
patch(
"nextcloud_mcp_server.api.visualization.validate_token_and_get_user",
new_callable=AsyncMock,
return_value=("testuser", True),
),
patch(
"nextcloud_mcp_server.api.visualization.get_user_client_basic_auth",
new_callable=AsyncMock,
return_value=mock_nc_client,
),
patch(
"nextcloud_mcp_server.api.visualization.get_chunk_with_context",
new_callable=AsyncMock,
return_value=mock_ctx,
) as mock_get_chunk,
):
app = create_test_app()
client = TestClient(app)
response = client.get(
"/api/v1/chunk-context?doc_type=note&doc_id=42"
"&start=0&end=10&chunk_index=7&total_chunks=10",
headers={"Authorization": "Bearer test-token"},
)
assert response.status_code == 200
kwargs = mock_get_chunk.await_args.kwargs
assert kwargs["chunk_index"] == 7
assert kwargs["total_chunks"] == 10
data = response.json()
assert data["chunk_index"] == 7
assert data["total_chunks"] == 10
# page_number must be present even when None (frontend may scroll
# by it for non-file doc types)
assert "page_number" in data
class TestChunkContextConfigErrors:
"""Tests for configuration failure paths."""
tests/unit/test_viz_routes_chunk_context.py
+194
View File
@@ -0,0 +1,194 @@
"""Unit tests for the OAuth-session chunk-context endpoint
(`nextcloud_mcp_server.auth.viz_routes.chunk_context_endpoint`).
Mirrors the regression coverage of
`tests/unit/test_management_chunk_context_endpoint.py` (which targets the
management API route in `nextcloud_mcp_server.api.visualization`).
Both routes share the same purpose — fetch chunk text with surrounding
context for the viz pane — but live behind different auth surfaces:
* Management API: OAuth bearer validated by `validate_token_and_get_user`
* Viz route: Starlette session auth via `@requires("authenticated")`
Because of the auth-middleware difference, a separate file is cleaner than
mixing both styles into one test module.
"""
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from starlette.applications import Starlette
from starlette.authentication import (
AuthCredentials,
AuthenticationBackend,
SimpleUser,
)
from starlette.middleware import Middleware
from starlette.middleware.authentication import AuthenticationMiddleware
from starlette.routing import Route
from starlette.testclient import TestClient
from nextcloud_mcp_server.auth.viz_routes import chunk_context_endpoint
pytestmark = pytest.mark.unit
class _AlwaysAuthBackend(AuthenticationBackend):
"""Stub auth backend: every request is authenticated as `testuser`."""
async def authenticate(self, conn):
return AuthCredentials(["authenticated"]), SimpleUser("testuser")
def _make_app() -> Starlette:
return Starlette(
routes=[
Route("/app/chunk-context", chunk_context_endpoint, methods=["GET"]),
],
middleware=[
Middleware(AuthenticationMiddleware, backend=_AlwaysAuthBackend()),
],
)
def _make_mock_chunk_context(chunk_text="chunk", before="before", after="after"):
"""Mock a ChunkContext dataclass with enough fields for the handler."""
ctx = MagicMock()
ctx.chunk_text = chunk_text
ctx.before_context = before
ctx.after_context = after
ctx.has_before_truncation = False
ctx.has_after_truncation = False
ctx.page_number = None
ctx.chunk_index = 0
ctx.total_chunks = 1
return ctx
def _make_mock_nc_client():
"""Mock NextcloudClient that supports `async with`."""
mock_client = MagicMock()
mock_client.__aenter__ = AsyncMock(return_value=mock_client)
mock_client.__aexit__ = AsyncMock(return_value=None)
return mock_client
def _make_mock_settings(nextcloud_host: str = "http://localhost:8080") -> MagicMock:
"""Mock get_settings() return value with the fields the handler reads."""
settings = MagicMock()
settings.nextcloud_host = nextcloud_host
settings.get_collection_name.return_value = "test-collection"
return settings
class TestVizChunkContextParameterForwarding:
"""Regression guard mirroring TestChunkContextParameterForwarding for the
management API: chunk_index / total_chunks must reach get_chunk_with_context.
"""
def test_chunk_index_and_total_chunks_forwarded(self):
mock_nc_client = _make_mock_nc_client()
mock_ctx = _make_mock_chunk_context()
mock_ctx.chunk_index = 7
mock_ctx.total_chunks = 10
with (
patch(
"nextcloud_mcp_server.auth.viz_routes.get_settings",
return_value=_make_mock_settings(),
),
patch(
"nextcloud_mcp_server.auth.viz_routes.get_user_client_basic_auth",
new_callable=AsyncMock,
return_value=mock_nc_client,
),
patch(
"nextcloud_mcp_server.auth.viz_routes.get_chunk_with_context",
new_callable=AsyncMock,
return_value=mock_ctx,
) as mock_get_chunk,
):
with TestClient(_make_app()) as client:
response = client.get(
"/app/chunk-context?doc_type=note&doc_id=42"
"&start=0&end=10&chunk_index=7&total_chunks=10"
)
assert response.status_code == 200
kwargs = mock_get_chunk.await_args.kwargs
assert kwargs["chunk_index"] == 7
assert kwargs["total_chunks"] == 10
data = response.json()
assert data["chunk_index"] == 7
assert data["total_chunks"] == 10
# page_number must be present even when None — the response shape
# is unconditional so the frontend can rely on the key existing.
assert "page_number" in data
class TestVizChunkContextFile404:
"""When get_chunk_with_context returns None for doc_type=file, the route
must surface a fast 404 — no slow PDF re-parse fallback. This guards the
proxy-timeout fix from PR #767.
"""
def test_file_doc_type_qdrant_miss_yields_fast_404(self):
mock_nc_client = _make_mock_nc_client()
with (
patch(
"nextcloud_mcp_server.auth.viz_routes.get_settings",
return_value=_make_mock_settings(),
),
patch(
"nextcloud_mcp_server.auth.viz_routes.get_user_client_basic_auth",
new_callable=AsyncMock,
return_value=mock_nc_client,
),
patch(
"nextcloud_mcp_server.auth.viz_routes.get_chunk_with_context",
new_callable=AsyncMock,
return_value=None,
) as mock_get_chunk,
):
with TestClient(_make_app()) as client:
response = client.get(
"/app/chunk-context?doc_type=file&doc_id=12345"
"&start=0&end=10&chunk_index=3&total_chunks=20"
)
assert response.status_code == 404
data = response.json()
assert data["success"] is False
assert "failed to fetch chunk context" in data["error"].lower()
kwargs = mock_get_chunk.await_args.kwargs
assert kwargs["doc_type"] == "file"
assert kwargs["chunk_index"] == 3
assert kwargs["total_chunks"] == 20
class TestVizChunkContextValueErrorLogging:
"""Verify the route returns 400 for malformed integer params (and does
not crash with a 500). The log-level demotion (logger.warning) is
asserted indirectly via response shape — log-level itself is not a
behaviour the user can observe through HTTP.
"""
def test_invalid_int_param_returns_400(self):
with patch(
"nextcloud_mcp_server.auth.viz_routes.get_settings",
return_value=_make_mock_settings(),
):
with TestClient(_make_app()) as client:
response = client.get(
"/app/chunk-context?doc_type=note&doc_id=1"
"&start=not-a-number&end=10"
)
assert response.status_code == 400
data = response.json()
assert data["success"] is False
assert "invalid" in data["error"].lower()