Skip to content

fix: async errors and implement missing methods in qdrantindex#663

Merged
jamescalam merged 1 commit into
mainfrom
ashraq/fix-qdrant
May 18, 2026
Merged

fix: async errors and implement missing methods in qdrantindex#663
jamescalam merged 1 commit into
mainfrom
ashraq/fix-qdrant

Conversation

@ashraq1455

@ashraq1455 ashraq1455 commented May 18, 2026

Copy link
Copy Markdown
Contributor

User description

Summary

  • Implemented _remove_and_sync and _async_remove_and_sync — previously stubbed with a logger error, causing runtime failures when auto_sync tried to delete utterances
  • Added _point_ids_for_utterances helper that derives deterministic point UUIDs from route/utterance strings without a Qdrant round-trip (reuses the same uuid5 scheme as add())
  • Implemented _get_all and _async_get_all as the proper BaseIndex extension points, replacing inline scroll logic that was duplicated across get_utterances and aget_utterances
  • Simplified get_utterances and aget_utterances to thin wrappers over _get_all/_async_get_all, consistent with how PineconeIndex is structured
  • Added aget_routes (async) which was missing from QdrantIndex but required by the router sync flow
  • Fixed aget_utterances fallback: in-memory Qdrant (no aclient) now correctly falls back to the sync path via _get_all

PR Type

Bug fix, Enhancement


Description

  • Implement Qdrant utterance deletion

  • Add shared sync/async scroll retrieval

  • Fix async route and utterance access

  • Preserve metadata inclusion behavior


Diagram Walkthrough

flowchart LR
  A["Route utterance changes"] -- "derive deterministic IDs" --> B["Qdrant point deletion"]
  C["Qdrant scroll"] -- "shared retrieval helpers" --> D["Utterance and route APIs"]
  E["Async client unavailable"] -- "fallback" --> F["Sync retrieval path"]
Loading

File Walkthrough

Relevant files
Bug fix
qdrant.py
Complete Qdrant deletion and retrieval APIs                           

semantic_router/index/qdrant.py

  • Adds _point_ids_for_utterances to derive deterministic Qdrant point
    IDs using the same uuid5 scheme as insertion.
  • Implements _remove_and_sync and _async_remove_and_sync to delete
    selected utterances from Qdrant instead of logging an unimplemented
    error.
  • Introduces _get_all and _async_get_all shared scroll helpers with
    namespace filtering, empty-index handling, and sync fallback for
    missing async clients.
  • Refactors get_utterances, aget_utterances, get_routes_tuples, and
    aget_routes to return route/utterance tuples and metadata
    consistently.
+155/-71

@ashraq1455 ashraq1455 changed the title fix: async errors and implement missing methods in QdrantIndex and fix: Async errors and implement missing methods in QdrantIndex May 18, 2026
@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ No major issues detected

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Prevent invalid utterance records

Defaulting missing route or utterance payload fields to empty strings can create
invalid Utterance objects that later sync or route operations may treat as real
records. Filter out malformed payloads or fail explicitly; apply the same guard in
aget_utterances.

semantic_router/index/qdrant.py [420-428]

 return [
     Utterance(
-        route=p.get(SR_ROUTE_PAYLOAD_KEY, ""),
-        utterance=p.get(SR_UTTERANCE_PAYLOAD_KEY, ""),
+        route=p[SR_ROUTE_PAYLOAD_KEY],
+        utterance=p[SR_UTTERANCE_PAYLOAD_KEY],
         function_schemas=None,
         metadata=p.get("metadata", {}) if include_metadata else {},
     )
     for p in payloads
+    if SR_ROUTE_PAYLOAD_KEY in p and SR_UTTERANCE_PAYLOAD_KEY in p
 ]
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that using empty-string defaults for missing SR_ROUTE_PAYLOAD_KEY or SR_UTTERANCE_PAYLOAD_KEY can create misleading Utterance records. This is a reasonable data-integrity improvement, though malformed payloads may be uncommon because records are expected to be written by this index implementation.

Low
Avoid blocking async fallback

Calling the synchronous delete path directly inside _async_remove_and_sync can block
the event loop during network I/O. Run the sync fallback in a worker thread so async
callers do not stall other tasks when aclient is unavailable.

semantic_router/index/qdrant.py [261-262]

 else:
-    self._remove_and_sync(routes_to_delete)
+    await asyncio.to_thread(self._remove_and_sync, routes_to_delete)
Suggestion importance[1-10]: 4

__

Why: The suggestion is conceptually valid because calling _remove_and_sync directly inside _async_remove_and_sync may block the event loop when falling back to the sync client. Its impact is moderate and the improved code would also require asyncio to be available in the module.

Low

@jamescalam jamescalam changed the title fix: Async errors and implement missing methods in QdrantIndex fix: async errors and implement missing methods in qdrantindex May 18, 2026
Comment thread semantic_router/index/qdrant.py
Comment thread semantic_router/index/qdrant.py
Comment thread semantic_router/index/qdrant.py
@jamescalam
jamescalam merged commit 753826a into main May 18, 2026
13 of 16 checks passed
@jamescalam
jamescalam deleted the ashraq/fix-qdrant branch May 18, 2026 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants