feat: Add tag management methods to WebDAV client

- Add get_file_info() to get file info including file ID via PROPFIND - Add create_tag() to create system tags via OCS API - Add get_or_create_tag() for idempotent tag creation - Add assign_tag_to_file() to assign tags to files via WebDAV - Add remove_tag_from_file() to remove tags from files Also refactors RAG evaluation: - Add indexed_manual_pdf fixture using existing nc_client/nc_mcp_client - Remove manual tag creation steps from workflow (now handled by fixture) - Add comprehensive unit tests for new WebDAV methods 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-23 01:51:42 +01:00
parent 04251401aa
commit 2896fa1dc9
4 changed files with 572 additions and 171 deletions
@@ -10,6 +10,7 @@ Environment Variables:
    OPENAI_BASE_URL: Base URL override (e.g., "https://models.github.ai/inference")
    OPENAI_EMBEDDING_MODEL: Embedding model (default: "text-embedding-3-small")
    OPENAI_GENERATION_MODEL: Generation model for sampling (default: "gpt-4o-mini")
+    RAG_MANUAL_PATH: Path to manual PDF in Nextcloud (default: "Nextcloud_User_Manual.pdf")

 For GitHub CI, set:
    OPENAI_API_KEY: ${{ secrets.GITHUB_TOKEN }}
@@ -18,15 +19,17 @@ For GitHub CI, set:
    OPENAI_GENERATION_MODEL: openai/gpt-4o-mini

 Prerequisites:
-    - Nextcloud User Manual indexed in Qdrant (via vector sync)
+    - Nextcloud User Manual PDF uploaded to Nextcloud
    - VECTOR_SYNC_ENABLED=true on the MCP server
 """

 import json
+import logging
 import os
 from pathlib import Path
 from typing import Any, AsyncGenerator

+import anyio
 import pytest
 from mcp import ClientSession

@@ -34,6 +37,11 @@ from nextcloud_mcp_server.providers.openai import OpenAIProvider
 from tests.conftest import create_mcp_client_session
 from tests.integration.sampling_support import create_openai_sampling_callback

+logger = logging.getLogger(__name__)
+
+# Default path to the Nextcloud User Manual PDF
+DEFAULT_MANUAL_PATH = "Nextcloud Manual.pdf"
+
 # Skip all tests if OpenAI API key not configured
 pytestmark = [
    pytest.mark.integration,
@@ -58,6 +66,86 @@ def ground_truth_qa():
        return json.load(f)


+@pytest.fixture(scope="module")
+async def indexed_manual_pdf(nc_client, nc_mcp_client):
+    """Ensure the Nextcloud User Manual PDF is tagged and indexed for vector search.
+
+    This fixture:
+    1. Gets file info for the manual PDF
+    2. Creates/gets the 'vector-index' tag
+    3. Assigns the tag to the file
+    4. Waits for vector sync to complete indexing
+
+    Environment Variables:
+        RAG_MANUAL_PATH: Path to manual PDF in Nextcloud (default: Nextcloud Manual.pdf)
+    """
+    manual_path = os.getenv("RAG_MANUAL_PATH", DEFAULT_MANUAL_PATH)
+
+    logger.info(f"Setting up indexed manual PDF: {manual_path}")
+
+    # Get file info to verify file exists and get file ID
+    file_info = await nc_client.webdav.get_file_info(manual_path)
+    if not file_info:
+        pytest.skip(f"Manual PDF not found at '{manual_path}'")
+
+    file_id = file_info["id"]
+    logger.info(f"Found manual PDF: {manual_path} (file_id={file_id})")
+
+    # Create or get the vector-index tag
+    tag = await nc_client.webdav.get_or_create_tag("vector-index")
+    tag_id = tag["id"]
+    logger.info(f"Using tag 'vector-index' (tag_id={tag_id})")
+
+    # Assign tag to file
+    await nc_client.webdav.assign_tag_to_file(file_id, tag_id)
+    logger.info(f"Tagged file {file_id} with vector-index tag")
+
+    # Wait for vector sync to complete indexing
+    max_attempts = 60
+    poll_interval = 10
+
+    logger.info("Waiting for vector sync to index the manual...")
+
+    for attempt in range(1, max_attempts + 1):
+        try:
+            # Call the MCP tool via the existing client session
+            result = await nc_mcp_client.call_tool(
+                "nc_get_vector_sync_status",
+                arguments={},
+            )
+
+            if not result.isError:
+                content = result.structuredContent or {}
+                indexed = content.get("indexed_count", 0)
+                pending = content.get("pending_count", 1)
+
+                logger.info(
+                    f"Attempt {attempt}/{max_attempts}: "
+                    f"indexed={indexed}, pending={pending}"
+                )
+
+                if indexed > 0 and pending == 0:
+                    logger.info(
+                        f"Vector indexing complete: {indexed} documents indexed"
+                    )
+                    break
+        except Exception as e:
+            logger.warning(f"Attempt {attempt}: Error checking status: {e}")
+
+        if attempt < max_attempts:
+            await anyio.sleep(poll_interval)
+    else:
+        logger.warning(
+            f"Vector indexing may not be complete after {max_attempts} attempts"
+        )
+
+    yield {
+        "path": manual_path,
+        "file_id": file_id,
+        "tag_id": tag_id,
+    }
+
+
@pytest.fixture(scope="module")
 async def openai_provider():
    """OpenAI provider configured from environment (embeddings only)."""
@@ -129,7 +217,9 @@ async def test_openai_embeddings_work(openai_provider: OpenAIProvider):
    assert len(embedding) in [1536, 3072]


-async def test_semantic_search_retrieval(nc_mcp_client, ground_truth_qa):
+async def test_semantic_search_retrieval(
+    nc_mcp_client, ground_truth_qa, indexed_manual_pdf
+):
    """Test that semantic search retrieves relevant documents from the manual.

    This tests the retrieval component of RAG - ensuring that queries
@@ -167,7 +257,7 @@ async def test_semantic_search_retrieval(nc_mcp_client, ground_truth_qa):


 async def test_semantic_search_answer_with_sampling(
-    nc_mcp_client_with_sampling, ground_truth_qa
+    nc_mcp_client_with_sampling, ground_truth_qa, indexed_manual_pdf
 ):
    """Test semantic search with MCP sampling for answer generation.

@@ -243,7 +333,7 @@ async def test_semantic_search_answer_with_sampling(
    ],
 )
 async def test_retrieval_quality_all_queries(
-    nc_mcp_client, ground_truth_qa, qa_index, min_expected_results
+    nc_mcp_client, ground_truth_qa, indexed_manual_pdf, qa_index, min_expected_results
 ):
    """Test retrieval quality for all ground truth queries.

@@ -274,7 +364,7 @@ async def test_retrieval_quality_all_queries(
    )


-async def test_no_results_for_unrelated_query(nc_mcp_client):
+async def test_no_results_for_unrelated_query(nc_mcp_client, indexed_manual_pdf):
    """Test that completely unrelated queries return low/no scores.

    The Nextcloud manual shouldn't have relevant content for
@@ -117,3 +117,244 @@ def test_parse_search_response_with_empty_tags(mocker):
    assert len(results) == 1
    assert "tags" in results[0]
    assert results[0]["tags"] == []
+
+
+@pytest.mark.unit
+async def test_get_file_info_returns_file_details(mocker):
+    """Test that get_file_info returns file info including file ID."""
+    mock_http_client = AsyncMock()
+    client = WebDAVClient(mock_http_client, "testuser")
+
+    # Mock PROPFIND response
+    mock_response = AsyncMock()
+    mock_response.status_code = 207
+    mock_response.content = b"""<?xml version="1.0"?>
+    <d:multistatus xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns">
+        <d:response>
+            <d:href>/remote.php/dav/files/testuser/Documents/test.pdf</d:href>
+            <d:propstat>
+                <d:prop>
+                    <oc:fileid>12345</oc:fileid>
+                    <d:displayname>test.pdf</d:displayname>
+                    <d:getcontentlength>1024</d:getcontentlength>
+                    <d:getcontenttype>application/pdf</d:getcontenttype>
+                    <d:getlastmodified>Sat, 01 Jan 2025 00:00:00 GMT</d:getlastmodified>
+                    <d:getetag>"abc123"</d:getetag>
+                    <d:resourcetype/>
+                </d:prop>
+            </d:propstat>
+        </d:response>
+    </d:multistatus>"""
+    mock_response.raise_for_status = mocker.Mock()
+
+    mock_http_client.request = AsyncMock(return_value=mock_response)
+
+    # Call get_file_info
+    result = await client.get_file_info("Documents/test.pdf")
+
+    # Verify result
+    assert result is not None
+    assert result["id"] == 12345
+    assert result["name"] == "test.pdf"
+    assert result["path"] == "Documents/test.pdf"
+    assert result["content_type"] == "application/pdf"
+    assert result["size"] == 1024
+    assert result["etag"] == "abc123"
+    assert result["is_directory"] is False
+
+
+@pytest.mark.unit
+async def test_get_file_info_returns_none_for_missing_file(mocker):
+    """Test that get_file_info returns None for missing files."""
+    from httpx import HTTPStatusError, Response
+
+    mock_http_client = AsyncMock()
+    client = WebDAVClient(mock_http_client, "testuser")
+
+    # Mock 404 response
+    mock_response = mocker.Mock(spec=Response)
+    mock_response.status_code = 404
+    mock_http_client.request = AsyncMock(
+        side_effect=HTTPStatusError(
+            "Not Found", request=mocker.Mock(), response=mock_response
+        )
+    )
+
+    # Call get_file_info
+    result = await client.get_file_info("nonexistent.pdf")
+
+    # Verify result is None
+    assert result is None
+
+
+@pytest.mark.unit
+async def test_create_tag_creates_system_tag(mocker):
+    """Test that create_tag creates a system tag via OCS API."""
+    mock_http_client = AsyncMock()
+    client = WebDAVClient(mock_http_client, "testuser")
+
+    # Mock OCS response
+    mock_response = AsyncMock()
+    mock_response.status_code = 200
+    mock_response.json = mocker.Mock(
+        return_value={
+            "ocs": {
+                "data": {
+                    "id": 42,
+                    "name": "vector-index",
+                    "userVisible": True,
+                    "userAssignable": True,
+                }
+            }
+        }
+    )
+    mock_response.raise_for_status = mocker.Mock()
+
+    mock_http_client.post = AsyncMock(return_value=mock_response)
+
+    # Call create_tag
+    result = await client.create_tag("vector-index")
+
+    # Verify result
+    assert result["id"] == 42
+    assert result["name"] == "vector-index"
+    assert result["userVisible"] is True
+    assert result["userAssignable"] is True
+
+    # Verify API call
+    mock_http_client.post.assert_called_once()
+    call_args = mock_http_client.post.call_args
+    assert call_args[0][0] == "/ocs/v2.php/apps/systemtags/api/v1/tags"
+    assert call_args[1]["json"]["name"] == "vector-index"
+
+
+@pytest.mark.unit
+async def test_get_or_create_tag_returns_existing_tag(mocker):
+    """Test that get_or_create_tag returns existing tag without creating."""
+    mock_http_client = AsyncMock()
+    client = WebDAVClient(mock_http_client, "testuser")
+
+    # Mock existing tag
+    mocker.patch.object(
+        client,
+        "get_tag_by_name",
+        return_value={"id": 42, "name": "vector-index", "userVisible": True},
+    )
+    mock_create = mocker.patch.object(client, "create_tag")
+
+    # Call get_or_create_tag
+    result = await client.get_or_create_tag("vector-index")
+
+    # Verify existing tag returned without creating
+    assert result["id"] == 42
+    mock_create.assert_not_called()
+
+
+@pytest.mark.unit
+async def test_get_or_create_tag_creates_new_tag(mocker):
+    """Test that get_or_create_tag creates tag when not found."""
+    mock_http_client = AsyncMock()
+    client = WebDAVClient(mock_http_client, "testuser")
+
+    # Mock no existing tag
+    mocker.patch.object(client, "get_tag_by_name", return_value=None)
+    mocker.patch.object(
+        client,
+        "create_tag",
+        return_value={"id": 42, "name": "vector-index", "userVisible": True},
+    )
+
+    # Call get_or_create_tag
+    result = await client.get_or_create_tag("vector-index")
+
+    # Verify tag was created
+    assert result["id"] == 42
+    client.create_tag.assert_called_once_with("vector-index", True, True)
+
+
+@pytest.mark.unit
+async def test_assign_tag_to_file_success(mocker):
+    """Test that assign_tag_to_file assigns tag via WebDAV."""
+    mock_http_client = AsyncMock()
+    client = WebDAVClient(mock_http_client, "testuser")
+
+    # Mock 201 Created response
+    mock_response = AsyncMock()
+    mock_response.status_code = 201
+
+    mock_http_client.request = AsyncMock(return_value=mock_response)
+
+    # Call assign_tag_to_file
+    result = await client.assign_tag_to_file(12345, 42)
+
+    # Verify result
+    assert result is True
+
+    # Verify API call
+    mock_http_client.request.assert_called_once()
+    call_args = mock_http_client.request.call_args
+    assert call_args[0][0] == "PUT"
+    assert "/systemtags-relations/files/12345/42" in call_args[0][1]
+
+
+@pytest.mark.unit
+async def test_assign_tag_to_file_already_assigned(mocker):
+    """Test that assign_tag_to_file handles already assigned (409) gracefully."""
+    mock_http_client = AsyncMock()
+    client = WebDAVClient(mock_http_client, "testuser")
+
+    # Mock 409 Conflict response (already assigned)
+    mock_response = AsyncMock()
+    mock_response.status_code = 409
+
+    mock_http_client.request = AsyncMock(return_value=mock_response)
+
+    # Call assign_tag_to_file
+    result = await client.assign_tag_to_file(12345, 42)
+
+    # Verify result (should succeed even with 409)
+    assert result is True
+
+
+@pytest.mark.unit
+async def test_remove_tag_from_file_success(mocker):
+    """Test that remove_tag_from_file removes tag via WebDAV."""
+    mock_http_client = AsyncMock()
+    client = WebDAVClient(mock_http_client, "testuser")
+
+    # Mock 204 No Content response
+    mock_response = AsyncMock()
+    mock_response.status_code = 204
+
+    mock_http_client.request = AsyncMock(return_value=mock_response)
+
+    # Call remove_tag_from_file
+    result = await client.remove_tag_from_file(12345, 42)
+
+    # Verify result
+    assert result is True
+
+    # Verify API call
+    mock_http_client.request.assert_called_once()
+    call_args = mock_http_client.request.call_args
+    assert call_args[0][0] == "DELETE"
+    assert "/systemtags-relations/files/12345/42" in call_args[0][1]
+
+
+@pytest.mark.unit
+async def test_remove_tag_from_file_not_assigned(mocker):
+    """Test that remove_tag_from_file handles not assigned (404) gracefully."""
+    mock_http_client = AsyncMock()
+    client = WebDAVClient(mock_http_client, "testuser")
+
+    # Mock 404 Not Found response (tag wasn't assigned)
+    mock_response = AsyncMock()
+    mock_response.status_code = 404
+
+    mock_http_client.request = AsyncMock(return_value=mock_response)
+
+    # Call remove_tag_from_file
+    result = await client.remove_tag_from_file(12345, 42)
+
+    # Verify result (should succeed even with 404)
+    assert result is True