feat(data-layer): DL-10 - Vector Index Operations by spuentesp · Pull Request #104 · spuentesp/monitor_dm_system

spuentesp · 2026-01-05T14:17:48Z

Summary

Implements full Qdrant vector storage and semantic search support with 6 MCP tools for embedding operations across scenes, memories, and snippets.

Changes

Qdrant Client (db/qdrant.py)

New QdrantClient with singleton pattern
Auto-creates collections (scenes, memories, snippets) with 1536 dimensions
Supports both remote server and embedded local storage
Environment configuration (QDRANT_URL, QDRANT_API_KEY, QDRANT_PATH)
Lazy collection initialization with ensure_collection()
Cosine distance metric for semantic similarity

Vector Schemas (schemas/vectors.py)

VectorPoint/VectorUpsertRequest: Single vector storage
VectorBatchUpsertRequest: Batch operations
VectorSearchRequest/Response: Semantic search with filtering
VectorFilter: Multi-field filtering (story_id, scene_id, entity_id, type)
VectorDeleteRequest/ByFilterRequest: Point deletion
CollectionInfo: Collection metadata and stats

Qdrant Tools (tools/qdrant_tools.py)

6 MCP tools for vector operations:

qdrant_upsert: Store single vector with payload
qdrant_upsert_batch: Efficient batch storage
qdrant_search: Semantic search with score threshold + filters
qdrant_delete: Delete by ID
qdrant_delete_by_filter: Batch delete by filter
qdrant_get_collection_info: Collection stats

Helper function _build_qdrant_filter converts VectorFilter to Qdrant Filter DSL.

Authority Rules (middleware/auth.py)

All 6 operations have ["*"] authority (open access)

Tests (test_qdrant_tools.py)

15 comprehensive tests with proper mocking:

Upsert: single and batch operations, empty vector/points validation
Search: basic, with filters (story_id, entity_id, type), score threshold, multiple filters
Delete: by ID, by filter, empty filter validation
Collection info: metadata retrieval for populated and empty collections

Test Results

✅ All 294 tests passing (15 new tests for DL-10)

Design Decisions

Collections auto-created on first use (no manual setup)
UUID-to-string conversion at Qdrant boundary
Flexible filtering with custom filter support
Count before delete for accurate deletion counts
Pydantic validation catches schema errors early

Implementation Details

Implements: DL-10
Blocks: DL-7 (Memory embeddings), Q-3 (Semantic search), Q-5 (Character memory recall)
Files added: 4 (qdrant.py, vectors.py, qdrant_tools.py, test_qdrant_tools.py)
Files modified: 4 (db/init.py, schemas/init.py, tools/init.py, auth.py)

🤖 Generated with Claude Code

Implements full Qdrant vector storage and semantic search support with 6 MCP tools for embedding operations across scenes, memories, and snippets. ## Qdrant Client (db/qdrant.py) New QdrantClient with singleton pattern: - Auto-creates collections (scenes, memories, snippets) with 1536 dimensions - Supports both remote server and embedded local storage - Environment configuration (QDRANT_URL, QDRANT_API_KEY, QDRANT_PATH) - Lazy collection initialization with ensure_collection() - Cosine distance metric for semantic similarity ## Vector Schemas (schemas/vectors.py) New Pydantic schemas for vector operations: - VectorPoint/VectorUpsertRequest: Single vector storage - VectorBatchUpsertRequest: Batch operations - VectorSearchRequest/Response: Semantic search with filtering - VectorFilter: Multi-field filtering (story_id, scene_id, entity_id, type) - VectorDeleteRequest/ByFilterRequest: Point deletion - CollectionInfo: Collection metadata and stats ## Qdrant Tools (tools/qdrant_tools.py) 6 MCP tools for vector operations: - qdrant_upsert: Store single vector with payload - qdrant_upsert_batch: Efficient batch storage - qdrant_search: Semantic search with score threshold + filters - qdrant_delete: Delete by ID - qdrant_delete_by_filter: Batch delete by filter - qdrant_get_collection_info: Collection stats Helper function _build_qdrant_filter converts VectorFilter to Qdrant Filter DSL. ## Authority Rules (middleware/auth.py) All 6 operations have ["*"] authority (open access): - qdrant_upsert, qdrant_upsert_batch - qdrant_search - qdrant_delete, qdrant_delete_by_filter - qdrant_get_collection_info ## Tests (test_qdrant_tools.py) 15 comprehensive tests with proper mocking: - Upsert: single and batch operations, empty vector/points validation - Search: basic, with filters (story_id, entity_id, type), score threshold, multiple filters - Delete: by ID, by filter, empty filter validation - Collection info: metadata retrieval for populated and empty collections All 294 tests passing ✅ ## Design Decisions - Collections auto-created on first use (no manual setup) - UUID-to-string conversion at Qdrant boundary - Flexible filtering with custom filter support - Count before delete for accurate deletion counts - Pydantic validation catches schema errors early Implements: DL-10 Blocks: DL-7, Q-3, Q-5 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR implements comprehensive Qdrant vector storage and semantic search capabilities for the MONITOR data layer, adding 6 MCP tools for managing embeddings across scenes, memories, and snippets. The implementation includes a singleton Qdrant client with auto-collection creation, flexible filtering support, and complete test coverage.

Added QdrantClient with singleton pattern and lazy collection initialization for scenes, memories, and snippets collections
Implemented 6 vector operations: upsert (single/batch), semantic search with filtering, delete (by ID/filter), and collection metadata retrieval
Created comprehensive Pydantic schemas for vector operations with validation and type safety

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
packages/data-layer/src/monitor_data/db/qdrant.py	New Qdrant client with singleton pattern, collection management, and environment-based configuration
packages/data-layer/src/monitor_data/schemas/vectors.py	Pydantic schemas for vector operations including upsert, search, delete, and filter specifications
packages/data-layer/src/monitor_data/tools/qdrant_tools.py	Six MCP tools for vector operations with helper function for filter building
packages/data-layer/tests/test_tools/test_qdrant_tools.py	Comprehensive test suite with 15 tests covering all operations and edge cases
packages/data-layer/src/monitor_data/db/init.py	Added QdrantClient exports to db module
packages/data-layer/src/monitor_data/schemas/init.py	Added vector schema exports to schemas module
packages/data-layer/src/monitor_data/tools/init.py	Added 6 Qdrant tool exports to tools module
packages/data-layer/src/monitor_data/middleware/auth.py	Added authorization rules for all 6 Qdrant operations with open access

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-05T14:20:55Z

+    # Delete points matching filter
+    if deleted_count > 0:
+        qdrant.delete(
+            collection_name=params.collection,
+            points_selector=qdrant_filter,
+        )


The conditional check for deleted_count > 0 before calling delete may cause issues if the count operation succeeds but the delete operation is never attempted due to timing. If points are added between the count and delete operations, they won't be deleted. Consider whether this is the intended behavior or if the delete should always be attempted and rely on the filter match at delete time.

Suggested change

# Delete points matching filter

if deleted_count > 0:

qdrant.delete(

collection_name=params.collection,

points_selector=qdrant_filter,

)

# Delete points matching filter (always attempt delete; rely on filter match at delete time)

qdrant.delete(

collection_name=params.collection,

points_selector=qdrant_filter,

)

Copilot · 2026-01-05T14:20:56Z

+def get_qdrant_client() -> QdrantClient:
+    """
+    Get or create the singleton Qdrant client.
+
+    Returns:
+        QdrantClient instance
+
+    Thread-safe singleton pattern for database connections.
+    """
+    global _qdrant_client_instance
+
+    if _qdrant_client_instance is None:
+        _qdrant_client_instance = QdrantClient()
+        _qdrant_client_instance.connect()
+
+    return _qdrant_client_instance


The singleton pattern implementation is not thread-safe. Multiple threads calling get_qdrant_client() simultaneously could create multiple instances of QdrantClient. Consider using a lock or other thread-safe mechanism to protect the singleton creation, especially since the docstring claims it's "Thread-safe singleton pattern for database connections."

Copilot · 2026-01-05T14:20:56Z

+    def ensure_collection(self, collection_name: str) -> None:
+        """
+        Ensure collection exists with correct configuration.
+
+        Creates collection if it doesn't exist, using predefined configs.
+
+        Args:
+            collection_name: Name of the collection (scenes, memories, snippets)
+
+        Raises:
+            ValueError: If collection name is not in COLLECTION_CONFIGS
+        """
+        if collection_name in self._collections_initialized:
+            return
+
+        client = self.get_client()
+
+        # Check if collection exists
+        try:
+            client.get_collection(collection_name)
+            self._collections_initialized.add(collection_name)
+            return
+        except Exception:
+            # Collection doesn't exist, create it
+            pass
+
+        # Get configuration
+        if collection_name not in COLLECTION_CONFIGS:
+            raise ValueError(
+                f"Unknown collection '{collection_name}'. "
+                f"Valid collections: {', '.join(COLLECTION_CONFIGS.keys())}"
+            )
+
+        config = COLLECTION_CONFIGS[collection_name]
+
+        # Create collection
+        client.create_collection(
+            collection_name=collection_name,
+            vectors_config=VectorParams(
+                size=int(config["vector_size"]),
+                distance=Distance(config["distance"]),
+            ),
+        )
+
+        self._collections_initialized.add(collection_name)


The _collections_initialized set is not thread-safe. If multiple threads call ensure_collection() with the same collection name simultaneously, they could both pass the check at line 143 and attempt to create the collection concurrently. This could lead to race conditions or duplicate creation attempts. Consider using a thread-safe set or adding synchronization.

Copilot · 2026-01-05T14:20:56Z

+    """
+    Qdrant client for MONITOR vector storage and semantic search.
+
+    Thread-safe singleton client for Qdrant operations.


The docstring states "Thread-safe singleton client for Qdrant operations" but the implementation is not actually thread-safe. This is misleading documentation. Either make the implementation thread-safe or update the documentation to accurately reflect the current behavior.

Suggested change

Thread-safe singleton client for Qdrant operations.

Client for Qdrant operations.

Copilot · 2026-01-05T14:20:56Z

+    "qdrant_upsert": ["*"],
+    "qdrant_upsert_batch": ["*"],
    "qdrant_search": ["*"],
-    "qdrant_search_memories": ["*"],
-    "qdrant_delete_vectors": ["Indexer"],
+    "qdrant_delete": ["*"],
+    "qdrant_delete_by_filter": ["*"],
+    "qdrant_get_collection_info": ["*"],


All 6 Qdrant operations have open access (["*"]) which allows any agent to perform vector operations including deletion. Consider whether qdrant_delete, qdrant_delete_by_filter, and qdrant_upsert_batch should have more restricted access similar to other operations in the codebase (e.g., "Indexer" or "Orchestrator"). Unrestricted deletion could lead to accidental or malicious data loss.

Copilot · 2026-01-05T14:20:57Z

+            collection_name=collection_name,
+            vectors_config=VectorParams(
+                size=int(config["vector_size"]),
+                distance=Distance(config["distance"]),


The Distance enum value is being used directly in the dictionary at line 171, but it's already a Distance enum. The code converts it with Distance(config["distance"]) which is redundant. This works but is unnecessary since config["distance"] is already a Distance enum instance. Consider removing the redundant conversion or storing the string value in COLLECTION_CONFIGS.

Suggested change

distance=Distance(config["distance"]),

distance=config["distance"],

Copilot · 2026-01-05T14:20:57Z

+def test_upsert_batch_empty_points():
+    """Test batch upsert fails with empty points list."""
+    # Pydantic validation will catch this before reaching the function
+    from pydantic import ValidationError
+
+    with pytest.raises(ValidationError):
+        VectorBatchUpsertRequest(
+            collection="memories",
+            points=[],  # Empty list - Pydantic min_length=1
+        )


The test validates that Pydantic raises a ValidationError for empty points list due to min_length=1, but the test comment on line 124 says "Empty list - Pydantic min_length=1" while line 118 says "Pydantic validation will catch this before reaching the function". However, the actual function qdrant_upsert_batch also has a manual check at line 203-204 that raises ValueError with a different message. This creates redundant validation. Consider removing the manual check in the function since Pydantic already enforces it, or update the test to verify both validation paths.

Copilot · 2026-01-05T14:20:57Z

+        return None
+
+    # Use cast to fix type variance issue
+    return Filter(must=must_conditions)  # type: ignore[arg-type]


Type ignore comment is used to suppress a variance issue, but there's a potential bug: if must_conditions is empty after checking all filter parameters, the function returns None at line 106. However, if custom filter is provided but evaluates as empty after conversion, this could lead to unexpected behavior. Consider adding validation for the custom filter case.

Copilot · 2026-01-05T14:20:57Z

+    results = [
+        ScoredVector(
+            id=UUID(result.id),  # Convert string ID back to UUID
+            score=result.score,
+            payload=result.payload or {},
+        )
+        for result in search_results
+    ]


The UUID conversion from string at line 300 assumes that result.id is a valid UUID string. If Qdrant returns an invalid UUID string (which shouldn't happen in normal operation but could occur with data corruption or manual manipulation), this will raise a ValueError. Consider adding error handling or validation to provide a more informative error message.

Suggested change

results = [

ScoredVector(

id=UUID(result.id), # Convert string ID back to UUID

score=result.score,

payload=result.payload or {},

)

for result in search_results

]

results = []

for result in search_results:

try:

vector_id = UUID(str(result.id)) # Convert string ID back to UUID

except (ValueError, TypeError) as exc:

raise ValueError(

f"Invalid UUID '{result.id}' returned from Qdrant "

f"for collection '{params.collection}'."

) from exc

results.append(

ScoredVector(

id=vector_id,

score=result.score,

payload=result.payload or {},

)

)

Addressed all 9 review comments from PR #104: ## Thread Safety Fixes (P1) - Added thread-safe singleton pattern with double-checked locking - Added threading.Lock for _collections_initialized set - Fixed race conditions in ensure_collection() - Updated docstring to accurately reflect thread-safety ## Authority Restriction (Security) - Restricted qdrant_upsert, qdrant_upsert_batch to ["Indexer"] - Restricted qdrant_delete, qdrant_delete_by_filter to ["Indexer"] - Kept qdrant_search, qdrant_get_collection_info as ["*"] ## Delete Logic Fix - Removed conditional check before delete in delete_by_filter - Now always attempts delete (relies on filter match at delete time) - Count before AND after to calculate accurate deleted_count - Handles race conditions: new points added between operations won't be deleted ## Code Quality Improvements - Removed redundant Distance() conversion (already Distance enum) - Removed redundant empty points validation (Pydantic already enforces) - Added validation for custom filter case - Added error handling for invalid UUID conversion with informative messages ## Test Updates - Updated test_delete_by_filter_success: now counts twice (before/after) - Updated test_delete_by_filter_no_matches: delete always called All 294 tests passing ✅ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Copilot AI review requested due to automatic review settings January 5, 2026 14:17

Copilot started reviewing on behalf of spuentesp January 5, 2026 14:18 View session

github-actions Bot added area/data-layer Data layer changes type/tests Tests touched labels Jan 5, 2026

Copilot AI reviewed Jan 5, 2026

View reviewed changes

spuentesp merged commit 165317c into master Jan 5, 2026
1 check passed

spuentesp deleted the feature/DL-10-vector-index-operations branch April 25, 2026 00:14

spuentesp restored the feature/DL-10-vector-index-operations branch July 25, 2026 00:13

spuentesp deleted the feature/DL-10-vector-index-operations branch July 25, 2026 00:14

Navigation Menu

feat(data-layer): DL-10 - Vector Index Operations - #104

feat(data-layer): DL-10 - Vector Index Operations#104
spuentesp merged 2 commits into
masterfrom
feature/DL-10-vector-index-operations

spuentesp commented Jan 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	Thread-safe singleton client for Qdrant operations.
	Client for Qdrant operations.

	distance=Distance(config["distance"]),
	distance=config["distance"],

Conversation

spuentesp commented Jan 5, 2026

Summary

Changes

Qdrant Client (db/qdrant.py)

Vector Schemas (schemas/vectors.py)

Qdrant Tools (tools/qdrant_tools.py)

Authority Rules (middleware/auth.py)

Tests (test_qdrant_tools.py)

Test Results

Design Decisions

Implementation Details

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants