From 53e6dba5a2e272067851bb8eefff61cde428ae29 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Fri, 8 May 2026 19:14:43 +0200 Subject: [PATCH] =?UTF-8?q?fix(viz=5Froutes):=20address=20PR=20#767=20revi?= =?UTF-8?q?ew=20=E2=80=94=20param=20parity=20+=20always-on=20page=5Fnumber?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace bare int() casts for start/end/context_chars in chunk_context_endpoint with _parse_int_param, matching visualization.py bounds (0–10M for offsets, 0–10K for context_chars), and add the missing end > start guard. - Initialize page_number from chunk_context.page_number so non-file doc_types surface it; include page_number, chunk_index, and total_chunks unconditionally in the response. Only highlighted_page_image stays gated on its own truthiness. - Add a chunk_index forwarding regression test that asserts the new kwargs reach get_chunk_with_context and appear in the response payload. Co-Authored-By: Claude Opus 4.7 (1M context) --- nextcloud_mcp_server/auth/viz_routes.py | 21 +++++--- .../test_management_chunk_context_endpoint.py | 52 +++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/nextcloud_mcp_server/auth/viz_routes.py b/nextcloud_mcp_server/auth/viz_routes.py index 331173b2..95256286 100644 --- a/nextcloud_mcp_server/auth/viz_routes.py +++ b/nextcloud_mcp_server/auth/viz_routes.py @@ -538,7 +538,6 @@ async def chunk_context_endpoint(request: Request) -> JSONResponse: 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") - context_chars = int(request.query_params.get("context", "500")) # Validate required parameters if not all([doc_type, doc_id, start_str, end_str]): @@ -556,8 +555,17 @@ 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( @@ -617,7 +625,7 @@ async def chunk_context_endpoint(request: Request) -> JSONResponse: # For PDF files, also fetch the highlighted page image from Qdrant highlighted_page_image = None - page_number = None + page_number = chunk_context.page_number if doc_type == "file": try: settings = get_settings() @@ -697,12 +705,13 @@ 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, } - # Add image data if available if highlighted_page_image: response_data["highlighted_page_image"] = highlighted_page_image - response_data["page_number"] = page_number return JSONResponse(response_data) diff --git a/tests/unit/test_management_chunk_context_endpoint.py b/tests/unit/test_management_chunk_context_endpoint.py index e387a91a..c335fce0 100644 --- a/tests/unit/test_management_chunk_context_endpoint.py +++ b/tests/unit/test_management_chunk_context_endpoint.py @@ -259,6 +259,58 @@ class TestChunkContextCredentialPath: assert "failed to fetch chunk context" in data["error"].lower() +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."""