feat: add gRPC support with configurable options for Qdrant connection - #51
Merged
pavanjava merged 2 commits intoJun 13, 2026
Merged
Conversation
Copy link
Copy Markdown
Collaborator
Sorry, something went wrong.
Copy link
Copy Markdown
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agentsVerify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/qql/ast_nodes.py`:
- Around line 28-30: Parser currently initializes exact/acorn/indexed_only to
False and merges with "or", which loses explicit false; update
src/qql/parser.py::_parse_with_clause to initialize
SearchWith.exact/acorn/indexed_only as None (not False) and change merging logic
to coalesce using "if parsed_with.<field> is not None then parsed_with.<field>
else with_clause.<field>" (or equivalent is not None check) instead of "or" so
explicit False wins; also update src/qql/executor.py::_build_search_params to
map these three fields by checking "is not None" and passing True/False through
as-is (return value = field if field is not None else None) so False remains an
explicit disable for exact, acorn, and indexed_only.
In `@src/qql/cli.py`:
- Around line 243-250: The QdrantClient created in the try block may not be
closed if client.get_collections() raises; ensure the client is always closed by
restructuring to either use a context manager for QdrantClient or add a
try/finally that calls client.close() after creating the client (i.e., create
client = QdrantClient(**client_kwargs) then in finally call client.close()),
keeping the error logging in the except block and sys.exit(1) after the finally.
In `@src/qql/executor.py`:
- Around line 264-269: The current except block in executor.py that inspects
ValueError (the block referencing variable name and raising QQLRuntimeError)
uses a brittle literal substring check; replace it with a robust,
case-insensitive regex (or two-part check) that looks for "not found" and the
collection name allowing optional surrounding quotes (e.g. use
re.search(fr"collection\s+['\"]?{re.escape(name)}['\"]?\s+not\s+found", str(e),
re.I) or fall back to checking "not found" in str(e). If the regex matches,
return None; otherwise raise the QQLRuntimeError as before (preserving the
original exception chain).
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info ⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2d7d6693-1448-4233-ab2e-d177e9819b16
📥 CommitsReviewing files that changed from the base of the PR and between 9172127 and 5a9dfb6.
📒 Files selected for processing (9)- src/qql/__init__.py
- src/qql/ast_nodes.py
- src/qql/cli.py
- src/qql/config.py
- src/qql/connection.py
- src/qql/executor.py
- src/qql/parser.py
- tests/test_connection.py
- tests/test_executor.py
Sorry, something went wrong.
Comment thread
src/qql/ast_nodes.py
Comment on lines
+28
to
+30
| exact: bool | None = None | ||
| acorn: bool | None = None | ||
| indexed_only: bool | None = None |
Copy link
Copy Markdown
There was a problem hiding this comment.
⚠️ Potential issue | 🟠 Major
🧩 Analysis chain🏁 Script executed:
#!/bin/bash
# Check all references to exact/acorn/indexed_only fields in SearchWith
rg -n -C3 'with_clause\.(exact|acorn|indexed_only)' --type=pyRepository: pavanjava/qql
Length of output: 5734
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show SearchWith definition around the reported lines
sed -n '1,120p' src/qql/ast_nodes.py | nl -ba | sed -n '1,120p'
# Show executor section building SearchParams
sed -n '1300,1405p' src/qql/executor.py | nl -ba
# Show parser around merging/propagating with_clause options
sed -n '740,860p' src/qql/parser.py | nl -ba
# Search for explicit parsing of indexed_only/acorn/exact values
rg -n "indexed_only|acorn|exact" src/qql/parser.py | head -n 200Repository: pavanjava/qql
Length of output: 100
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== src/qql/ast_nodes.py (SearchWith definition) ==="
sed -n '1,120p' src/qql/ast_nodes.py | nl -ba | sed -n '1,120p'
echo "=== src/qql/executor.py (SearchParams construction) ==="
sed -n '1300,1405p' src/qql/executor.py | nl -ba
echo "=== src/qql/parser.py (with_clause merging) ==="
sed -n '740,860p' src/qql/parser.py | nl -ba
echo "=== src/qql/parser.py (mentions of indexed_only/acorn/exact) ==="
rg -n "indexed_only|acorn|exact" src/qql/parser.py | head -n 200Repository: pavanjava/qql
Length of output: 171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== src/qql/ast_nodes.py (SearchWith definition) ==="
sed -n '1,120p' src/qql/ast_nodes.py | nl -ba | sed -n '1,120p'
echo "=== src/qql/executor.py (SearchParams construction) ==="
sed -n '1300,1405p' src/qql/executor.py | nl -ba
echo "=== src/qql/parser.py (with_clause merging) ==="
sed -n '740,860p' src/qql/parser.py | nl -ba
echo "=== src/qql/parser.py (mentions of indexed_only/acorn/exact) ==="
rg -n "indexed_only|acorn|exact" src/qql/parser.py | head -n 200Repository: pavanjava/qql
Length of output: 171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== src/qql/ast_nodes.py (SearchWith definition) ==="
sed -n '1,120p' src/qql/ast_nodes.py | nl -ba | sed -n '1,120p'
echo "=== src/qql/executor.py (SearchParams construction) ==="
sed -n '1300,1405p' src/qql/executor.py | nl -ba
echo "=== src/qql/parser.py (with_clause merging) ==="
sed -n '740,860p' src/qql/parser.py | nl -ba
echo "=== src/qql/parser.py (mentions of indexed_only/acorn/exact) ==="
rg -n "indexed_only|acorn|exact" src/qql/parser.py | head -n 200Repository: pavanjava/qql
Length of output: 171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== src/qql/ast_nodes.py (SearchWith definition) ==="
sed -n '1,120p' src/qql/ast_nodes.py | nl -ba | sed -n '1,120p'
echo "=== src/qql/executor.py (SearchParams construction) ==="
sed -n '1300,1405p' src/qql/executor.py | nl -ba
echo "=== src/qql/parser.py (with_clause merging) ==="
sed -n '740,860p' src/qql/parser.py | nl -ba
echo "=== src/qql/parser.py (mentions of indexed_only/acorn/exact) ==="
rg -n "indexed_only|acorn|exact" src/qql/parser.py | head -n 200Repository: pavanjava/qql
Length of output: 171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== src/qql/ast_nodes.py (SearchWith definition) ==="
sed -n '1,120p' src/qql/ast_nodes.py | nl -ba | sed -n '1,120p'
echo "=== src/qql/executor.py (SearchParams construction) ==="
sed -n '1300,1405p' src/qql/executor.py | nl -ba
echo "=== src/qql/parser.py (with_clause merging) ==="
sed -n '740,860p' src/qql/parser.py | nl -ba
echo "=== src/qql/parser.py (mentions of indexed_only/acorn/exact) ==="
rg -n "indexed_only|acorn|exact" src/qql/parser.py | head -n 200Repository: pavanjava/qql
Length of output: 171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== src/qql/ast_nodes.py (SearchWith definition) ==="
sed -n '1,120p' src/qql/ast_nodes.py | nl -ba | sed -n '1,120p'
echo "=== src/qql/executor.py (SearchParams construction) ==="
sed -n '1300,1405p' src/qql/executor.py | nl -ba
echo "=== src/qql/parser.py (with_clause merging) ==="
sed -n '740,860p' src/qql/parser.py | nl -ba
echo "=== src/qql/parser.py (mentions of indexed_only/acorn/exact) ==="
rg -n "indexed_only|acorn|exact" src/qql/parser.py | head -n 200Repository: pavanjava/qql
Length of output: 171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== src/qql/ast_nodes.py (SearchWith fields around lines 20-50) ==="
sed -n '1,120p' src/qql/ast_nodes.py | cat -n | sed -n '20,70p'
echo "=== src/qql/ast_nodes.py (SearchWith definition area) ==="
rg -n "class SearchWith|SearchWith\s*\(" src/qql/ast_nodes.py
echo "=== src/qql/executor.py (SearchParams construction around 1320-1370) ==="
sed -n '1310,1370p' src/qql/executor.py | cat -n
echo "=== src/qql/parser.py (with_clause merging around 760-825) ==="
sed -n '760,830p' src/qql/parser.py | cat -n
echo "=== src/qql/parser.py (lines mentioning exact/acorn/indexed_only) ==="
rg -n "exact\s*=|acorn\s*=|indexed_only\s*=|\.exact\b|\.acorn\b|\.indexed_only\b|parsed_with\.(exact|acorn|indexed_only)|with_clause\.(exact|acorn|indexed_only)" src/qql/parser.pyRepository: pavanjava/qql
Length of output: 9807
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== src/qql/parser.py (parse_with_clause around 1360-1450) ==="
sed -n '1340,1460p' src/qql/parser.py | cat -n
echo "=== tests for indexed_only false / acorn false ==="
rg -n "indexed_only:\s*false|acorn:\s*false" tests -S || true
echo "=== tests for indexed_only false / acorn false (with_clause keywords) ==="
rg -n "indexed_only.*false|acorn.*false" tests/test_parser.py -S || trueRepository: pavanjava/qql
Length of output: 5912
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== rg: SearchWith( constructions in src/qql/parser.py ==="
rg -n "SearchWith\(" src/qql/parser.py
echo "=== rg: SearchWith( constructions in src/qql/**/*.py ==="
rg -n "SearchWith\(" src/qql
echo "=== rg: _build_search_params call sites / with_clause usage ==="
rg -n "_build_search_params\(" -S src/qql/executor.py src/qql -SRepository: pavanjava/qql
Length of output: 1503
Fix SearchWith.exact/acorn/indexed_only handling of false vs None end-to-end
- src/qql/ast_nodes.py now defines SearchWith.exact, SearchWith.acorn, and SearchWith.indexed_only as bool | None = None, but src/qql/parser.py::_parse_with_clause initializes exact/acorn/indexed_only to False and always returns booleans—so omitting those keys in WITH { ... } yields False (not None).
- src/qql/parser.py merges with_clause values using parsed_with.<field> or with_clause.<field>, which makes explicit false unable to override a previously-set true.
- src/qql/executor.py::_build_search_params treats indexed_only and acorn as True if ... else None, so false and None both become “unset” for Qdrant (while exact is passed through directly).
Adjust parsing/merging/executor mapping to use is not None / coalescing semantics so None remains “unset” and false remains an explicit disable for all three fields.
🤖 Prompt for AI AgentsVerify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/qql/ast_nodes.py` around lines 28 - 30, Parser currently initializes exact/acorn/indexed_only to False and merges with "or", which loses explicit false; update src/qql/parser.py::_parse_with_clause to initialize SearchWith.exact/acorn/indexed_only as None (not False) and change merging logic to coalesce using "if parsed_with.<field> is not None then parsed_with.<field> else with_clause.<field>" (or equivalent is not None check) instead of "or" so explicit False wins; also update src/qql/executor.py::_build_search_params to map these three fields by checking "is not None" and passing True/False through as-is (return value = field if field is not None else None) so False remains an explicit disable for exact, acorn, and indexed_only.
Sorry, something went wrong.
Comment thread
src/qql/cli.py
Comment on lines
243
to
250
| try: | ||
| client = QdrantClient(url=url, api_key=secret, verify=verify_val) | ||
| client = QdrantClient(**client_kwargs) | ||
| client.get_collections() | ||
| except Exception as e: | ||
| err_console.print(f"[bold red]Connection failed:[/bold red] {e}") | ||
| sys.exit(1) | ||
| else: | ||
| client.close() |
Copy link
Copy Markdown
There was a problem hiding this comment.
⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
Client not closed on connection test failure.
When the connection test at line 244 succeeds, client.close() is called at line 250. However, if client.get_collections() at line 245 raises an exception, the except block at line 246 prints an error and exits, but the client connection is never closed.
🔒 Proposed fix to ensure client is closed try:
client = QdrantClient(**client_kwargs)
client.get_collections()
except Exception as e:
+ client.close()
err_console.print(f"[bold red]Connection failed:[/bold red] {e}")
sys.exit(1)
else:
client.close()Alternatively, use a context manager or try/finally.
📝 Committable suggestion‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try: | |
| client = QdrantClient(url=url, api_key=secret, verify=verify_val) | |
| client = QdrantClient(**client_kwargs) | |
| client.get_collections() | |
| except Exception as e: | |
| err_console.print(f"[bold red]Connection failed:[/bold red] {e}") | |
| sys.exit(1) | |
| else: | |
| client.close() | |
| client = None | |
| try: | |
| client = QdrantClient(**client_kwargs) | |
| client.get_collections() | |
| except Exception as e: | |
| if client: | |
| client.close() | |
| err_console.print(f"[bold red]Connection failed:[/bold red] {e}") | |
| sys.exit(1) | |
| else: | |
| client.close() |
[warning] 246-246: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI AgentsVerify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/qql/cli.py` around lines 243 - 250, The QdrantClient created in the try block may not be closed if client.get_collections() raises; ensure the client is always closed by restructuring to either use a context manager for QdrantClient or add a try/finally that calls client.close() after creating the client (i.e., create client = QdrantClient(**client_kwargs) then in finally call client.close()), keeping the error logging in the except block and sys.exit(1) after the finally.
Sorry, something went wrong.
Comment thread
src/qql/executor.py
Comment on lines
+264
to
+269
| except ValueError as e: | ||
| if f"Collection {name} not found" in str(e): | ||
| return None | ||
| raise QQLRuntimeError( | ||
| f"Qdrant error fetching collection '{name}': {e}" | ||
| ) from e |
Copy link
Copy Markdown
There was a problem hiding this comment.
⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
String matching may miss collection-not-found ValueError.
Line 265 checks if f"Collection {name} not found" is in str(e), but this literal string interpolation happens at check-time with the actual collection name. If the ValueError message uses a different format (e.g., quotes around the name: Collection 'docs' not found), this check will fail.
🛡️ Proposed fix for robust pattern matching except ValueError as e:
- if f"Collection {name} not found" in str(e):
+ if "not found" in str(e).lower() and name in str(e):
return None
raise QQLRuntimeError(
f"Qdrant error fetching collection '{name}': {e}"
) from eThis checks for both "not found" (case-insensitive) and the collection name, making it more robust to message format variations.
🤖 Prompt for AI AgentsVerify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/qql/executor.py` around lines 264 - 269, The current except block in
executor.py that inspects ValueError (the block referencing variable name and
raising QQLRuntimeError) uses a brittle literal substring check; replace it with
a robust, case-insensitive regex (or two-part check) that looks for "not found"
and the collection name allowing optional surrounding quotes (e.g. use
re.search(fr"collection\s+['\"]?{re.escape(name)}['\"]?\s+not\s+found", str(e),
re.I) or fall back to checking "not found" in str(e). If the regex matches,
return None; otherwise raise the QQLRuntimeError as before (preserving the
original exception chain).
Sorry, something went wrong.
…ction and executor
pavanjava
approved these changes
Jun 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat: add gRPC transport support and NULL filter handling
What
Adds optional gRPC transport for faster Qdrant communication and proper SQL-style NULL comparison handling in QQL filter expressions.
Why
Changes
gRPC transport:
NULL handling:
Error handling:
Tests
Summary by CodeRabbit
Release Notes