feat: cross-collection Qdrant search#143
Conversation
Allows searching any Qdrant collection directly, bypassing the knowledge_ prefix and metadata mapping. Enables querying external collections like music_events from the know CLI. Usage: know search "punk rock" --collection=music_events
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe changes introduce a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📊 Coverage Report
Files Below Threshold
🏆 Synapse Sentinel Gate |
🔧 Synapse Sentinel: 2 checks need attentionThe following issues must be resolved before this PR can be merged: All tests passed.--- Security AuditReview the output and fix any issues.Quick Reference:
🤖 Generated by Synapse Sentinel - View Run |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Commands/KnowledgeSearchCommand.php (1)
50-55:⚠️ Potential issue | 🟠 MajorRaw collection path should require a non-empty query explicitly.
Current ordering lets
--collectionrun with an empty query if any regular filter is supplied (e.g.,--tag), because those filters satisfy the earlier validation even though raw search ignores them.Suggested fix
+ $rawCollection = $this->option('collection'); + if (is_string($rawCollection) && trim($rawCollection) !== '') { + if (! is_string($query) || trim($query) === '') { + $this->error('Please provide a query when using --collection.'); + return self::FAILURE; + } + + return $this->searchRawCollection($qdrant, trim($rawCollection), $query, $limit); + } + // Require at least one search parameter for entries if ($query === null && $tag === null && $category === null && $module === null && $priority === null && $status === null) { $this->error('Please provide at least one search parameter.'); return self::FAILURE; } @@ - $rawCollection = $this->option('collection'); - if (is_string($rawCollection) && $rawCollection !== '') { - return $this->searchRawCollection($qdrant, $rawCollection, $searchQuery, $limit); - }Also applies to: 73-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Commands/KnowledgeSearchCommand.php` around lines 50 - 55, When the raw/collection search path is requested, the code currently allows an empty $query if other filters (like $tag) are provided; update the validation in the command (the handler that checks $query, $tag, $category, $module, $priority, $status and $collection) to explicitly require a non-empty query when $collection is truthy: add a guard such as if ($collection && (is_null($query) || trim($query) === '')) { $this->error('Please provide a non-empty query when using --collection.'); return self::FAILURE; } and apply the same explicit check in the other validation block that performs the entries vs collection decision.
🧹 Nitpick comments (1)
app/Services/QdrantService.php (1)
986-987: Add basic input guardrails forcollectionandlimit.Since this is a public service method, reject blank collection names and non-positive limits early to avoid avoidable Qdrant calls.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Services/QdrantService.php` around lines 986 - 987, The public method searchRawCollection has no input validation; add guardrails at the start of searchRawCollection(string $collection, string $query, int $limit = 10): Collection to reject blank collection names and non‑positive limits: check that trim($collection) !== '' and $limit > 0 and if either check fails throw an InvalidArgumentException (with a brief message identifying the invalid param), so invalid requests are rejected before making Qdrant calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Commands/KnowledgeSearchCommand.php`:
- Around line 175-182: The output shows raw "Array" or "Object" when non-scalar
payload values are interpolated; update KnowledgeSearchCommand to safely
serialize non-scalar values before display: in the map closure that builds
$fields, check each value's type and leave scalars as-is but replace
arrays/objects by json_encode($v,
JSON_UNESCAPED_UNICODE|JSON_INVALID_UTF8_SUBSTITUTE) (or a similar safe
serializer), and update the fallback on the score line to use
json_encode($payload, JSON_UNESCAPED_UNICODE|JSON_INVALID_UTF8_SUBSTITUTE) so
Unicode and invalid UTF-8 are handled; target the map closure and the
score/description print logic to implement these changes.
In `@app/Services/QdrantService.php`:
- Line 995: The SearchPoints call is hard-coding the relevance threshold as 0.0
which bypasses semantic filtering; update the constructor call new
SearchPoints($collection, $queryVector, $limit, 0.0) to use the configured
threshold $this->scoreThreshold so it respects the service-level relevance
setting (same behavior as searchByProject and hybridSearch). Locate the call in
QdrantService (the new SearchPoints invocation) and replace the literal 0.0 with
$this->scoreThreshold, ensuring the instance property is available in the
method.
---
Outside diff comments:
In `@app/Commands/KnowledgeSearchCommand.php`:
- Around line 50-55: When the raw/collection search path is requested, the code
currently allows an empty $query if other filters (like $tag) are provided;
update the validation in the command (the handler that checks $query, $tag,
$category, $module, $priority, $status and $collection) to explicitly require a
non-empty query when $collection is truthy: add a guard such as if ($collection
&& (is_null($query) || trim($query) === '')) { $this->error('Please provide a
non-empty query when using --collection.'); return self::FAILURE; } and apply
the same explicit check in the other validation block that performs the entries
vs collection decision.
---
Nitpick comments:
In `@app/Services/QdrantService.php`:
- Around line 986-987: The public method searchRawCollection has no input
validation; add guardrails at the start of searchRawCollection(string
$collection, string $query, int $limit = 10): Collection to reject blank
collection names and non‑positive limits: check that trim($collection) !== ''
and $limit > 0 and if either check fails throw an InvalidArgumentException (with
a brief message identifying the invalid param), so invalid requests are rejected
before making Qdrant calls.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1cfeae18-f8dc-4e63-aa9d-14139fae833e
📒 Files selected for processing (2)
app/Commands/KnowledgeSearchCommand.phpapp/Services/QdrantService.php
| $this->line('<fg=yellow>'.number_format($score, 3).'</> | '.($payload['description'] ?? json_encode($payload))); | ||
|
|
||
| // Show key payload fields | ||
| $fields = collect($payload) | ||
| ->except(['description', 'vector']) | ||
| ->filter(fn ($v): bool => $v !== null && $v !== '') | ||
| ->map(fn ($v, $k): string => "<fg=gray>{$k}:</> {$v}") | ||
| ->implode(' | '); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In PHP 8.x, what happens when an array is interpolated into a double-quoted string (e.g., "{$value}")?
💡 Result:
In PHP 8.x, when an array $value is interpolated into a double-quoted string using "{$value}", the array is converted to the string "Array" and inserted into the string. This behavior occurs because PHP automatically converts arrays to the string "Array" in string contexts, such as interpolation in double-quoted strings. For example: $value = ['a' => 'hello', 'b' => 'world']; echo "{$value}"; // Outputs: Array To interpolate a specific array element, use array access syntax within the curly braces, such as "{$value['key']}", which evaluates the expression and inserts the element's string representation. Simple syntax like "$value[key]" works only for unquoted keys in basic interpolation, but complex access requires curly braces. This has been consistent across PHP versions, including 8.x, with no changes specific to array interpolation in double-quoted strings (PHP 8 introduced dereferencing for interpolated strings like "foo$bar"[0], but not for whole arrays).
Citations:
- 1: https://www.php.net/manual/en/language.types.string.php
- 2: https://www.php.net/language.types.string
- 3: https://zetcode.com/php/string-interpolation/
- 4: https://www.php.net/manual/en/language.types.array.php
- 5: https://www.php.net/manual/en/language.types.type-juggling.php
- 6: https://stackoverflow.com/questions/66072934/is-the-behaviour-of-associative-array-keys-in-php-8-0-2-different-to-php-7-shou
- 7: https://stackoverflow.com/questions/4738850/interpolation-double-quoted-string-of-associative-arrays-in-php
- 8: https://stackoverflow.com/questions/43437121/php-string-interpolation-syntax
- 9: https://stackoverflow.com/questions/44349224/syntax-for-accessing-nested-array-elements-in-php-double-quotes-string
- 10: https://stackoverflow.com/questions/37273113/double-quoted-nested-array-wont-work
🏁 Script executed:
cat -n app/Commands/KnowledgeSearchCommand.php | sed -n '170,190p'Repository: conduit-ui/knowledge
Length of output: 912
🏁 Script executed:
rg -n '\$payload' app/Commands/KnowledgeSearchCommand.php -B 2 -A 2Repository: conduit-ui/knowledge
Length of output: 564
🏁 Script executed:
head -30 app/Commands/KnowledgeSearchCommand.phpRepository: conduit-ui/knowledge
Length of output: 1173
🏁 Script executed:
rg -n '\$payload\s*=' app/Commands/KnowledgeSearchCommand.php -B 5 -A 1 | head -40Repository: conduit-ui/knowledge
Length of output: 366
🏁 Script executed:
rg -n 'strict_types' app/ -iRepository: conduit-ui/knowledge
Length of output: 6235
🏁 Script executed:
rg -n 'class QdrantService' app/Services/QdrantService.php -A 30 | head -50Repository: conduit-ui/knowledge
Length of output: 1247
🏁 Script executed:
rg -n 'search\(' app/Services/QdrantService.php -A 20 | head -60Repository: conduit-ui/knowledge
Length of output: 2106
🏁 Script executed:
rg -n 'payload.*=>' app/Services/QdrantService.php -B 3 -A 3Repository: conduit-ui/knowledge
Length of output: 636
🏁 Script executed:
rg -n '"payload"' app/Services/QdrantService.php -B 2 -A 2Repository: conduit-ui/knowledge
Length of output: 46
🏁 Script executed:
rg -n '\$payload\s*\[' app/Services/QdrantService.php -B 2 -A 2 | head -80Repository: conduit-ui/knowledge
Length of output: 2246
🏁 Script executed:
sed -n '150,210p' app/Services/QdrantService.phpRepository: conduit-ui/knowledge
Length of output: 2840
🏁 Script executed:
rg -n "description" app/Services/QdrantService.phpRepository: conduit-ui/knowledge
Length of output: 46
🏁 Script executed:
rg -n "description" app/Commands/KnowledgeSearchCommand.php -B 2 -A 2Repository: conduit-ui/knowledge
Length of output: 764
Safely serialize non-scalar payload values before displaying.
Array and object fields in $payload (like tags or evidence) will display as the literal string "Array" when directly interpolated, producing unreadable output. Line 181's map closure accepts any non-null, non-empty value and interpolates it directly without type checking.
Additionally, line 175's json_encode($payload) fallback should include JSON flags to handle edge cases like unescaped unicode and invalid UTF-8 sequences.
Suggested fix
- $this->line('<fg=yellow>'.number_format($score, 3).'</> | '.($payload['description'] ?? json_encode($payload)));
+ $description = $payload['description'] ?? json_encode($payload, JSON_UNESCAPED_UNICODE | JSON_INVALID_UTF8_SUBSTITUTE);
+ $this->line('<fg=yellow>'.number_format($score, 3).'</> | '.($description ?: '[payload unavailable]'));
@@
- ->map(fn ($v, $k): string => "<fg=gray>{$k}:</> {$v}")
+ ->map(function ($v, $k): string {
+ $formatted = is_scalar($v)
+ ? (string) $v
+ : (json_encode($v, JSON_UNESCAPED_UNICODE | JSON_INVALID_UTF8_SUBSTITUTE) ?: '[unserializable]');
+
+ return "<fg=gray>{$k}:</> {$formatted}";
+ })📝 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.
| $this->line('<fg=yellow>'.number_format($score, 3).'</> | '.($payload['description'] ?? json_encode($payload))); | |
| // Show key payload fields | |
| $fields = collect($payload) | |
| ->except(['description', 'vector']) | |
| ->filter(fn ($v): bool => $v !== null && $v !== '') | |
| ->map(fn ($v, $k): string => "<fg=gray>{$k}:</> {$v}") | |
| ->implode(' | '); | |
| $description = $payload['description'] ?? json_encode($payload, JSON_UNESCAPED_UNICODE | JSON_INVALID_UTF8_SUBSTITUTE); | |
| $this->line('<fg=yellow>'.number_format($score, 3).'</> | '.($description ?: '[payload unavailable]')); | |
| // Show key payload fields | |
| $fields = collect($payload) | |
| ->except(['description', 'vector']) | |
| ->filter(fn ($v): bool => $v !== null && $v !== '') | |
| ->map(function ($v, $k): string { | |
| $formatted = is_scalar($v) | |
| ? (string) $v | |
| : (json_encode($v, JSON_UNESCAPED_UNICODE | JSON_INVALID_UTF8_SUBSTITUTE) ?: '[unserializable]'); | |
| return "<fg=gray>{$k}:</> {$formatted}"; | |
| }) | |
| ->implode(' | '); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Commands/KnowledgeSearchCommand.php` around lines 175 - 182, The output
shows raw "Array" or "Object" when non-scalar payload values are interpolated;
update KnowledgeSearchCommand to safely serialize non-scalar values before
display: in the map closure that builds $fields, check each value's type and
leave scalars as-is but replace arrays/objects by json_encode($v,
JSON_UNESCAPED_UNICODE|JSON_INVALID_UTF8_SUBSTITUTE) (or a similar safe
serializer), and update the fallback on the score line to use
json_encode($payload, JSON_UNESCAPED_UNICODE|JSON_INVALID_UTF8_SUBSTITUTE) so
Unicode and invalid UTF-8 are handled; target the map closure and the
score/description print logic to implement these changes.
| } | ||
|
|
||
| $response = $this->connector->send( | ||
| new SearchPoints($collection, $queryVector, $limit, 0.0) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Qdrant /collections/{name}/points/search, what is the effect of setting score_thresholdto0.0compared with a positive threshold like0.7?
💡 Result:
Setting score_threshold to 0.0 in Qdrant's /collections/{name}/points/search endpoint effectively disables the score filtering, returning up to the specified limit of the top closest points regardless of their similarity score (even scores as low as 0 or negative, depending on the distance metric). In contrast, a positive threshold like 0.7 filters out results below that score, potentially returning fewer than limit points if insufficient matches exceed the threshold. This behavior holds for similarity metrics where higher scores indicate better matches (e.g., Cosine: 0-1, Dot: -∞ to ∞), ensuring all returned results meet the minimum quality while 0.0 prioritizes quantity up to the limit.
Citations:
- 1: https://www.mintlify.com/qdrant/qdrant/guides/search
- 2: Does incremental indexing have any effect on recall.? qdrant/qdrant#8111
- 3: https://qdrant.tech/documentation/concepts/search
- 4: https://reference.langchain.com/python/langchain-qdrant/vectorstores/Qdrant/similarity_search_with_score
- 5: [Local Mode] score_threshold not applied to FusionQuery results qdrant/qdrant-client#1136
🏁 Script executed:
# First, let's examine the QdrantService.php file around line 995
sed -n '985,1005p' app/Services/QdrantService.phpRepository: conduit-ui/knowledge
Length of output: 729
🏁 Script executed:
# Search for scoreThreshold property definition in the file
rg 'scoreThreshold' app/Services/QdrantService.php -nRepository: conduit-ui/knowledge
Length of output: 163
🏁 Script executed:
# Check the constructor and property initialization
sed -n '1,100p' app/Services/QdrantService.php | grep -n 'class\|function __construct\|scoreThreshold\|private\|protected\|public'Repository: conduit-ui/knowledge
Length of output: 905
🏁 Script executed:
# Check context around line 445 where scoreThreshold is used
sed -n '440,450p' app/Services/QdrantService.phpRepository: conduit-ui/knowledge
Length of output: 379
🏁 Script executed:
# Search for all SearchPoints instantiations to see the pattern
rg 'new SearchPoints' app/Services/QdrantService.php -n -A 1 -B 1Repository: conduit-ui/knowledge
Length of output: 481
Use configured relevance threshold instead of hard-coding 0.0.
Line 995 hard-codes 0.0 in SearchPoints, which disables semantic filtering and returns all closest points regardless of similarity score—potentially including low-quality results. Other methods in this class (searchByProject at line 268 and hybridSearch at line 440) correctly use $this->scoreThreshold (default 0.7). For consistency and to apply the configured relevance threshold, replace 0.0 with $this->scoreThreshold.
Suggested fix
- $response = $this->connector->send(
- new SearchPoints($collection, $queryVector, $limit, 0.0)
- );
+ $response = $this->connector->send(
+ new SearchPoints($collection, $queryVector, $limit, $this->scoreThreshold)
+ );📝 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.
| new SearchPoints($collection, $queryVector, $limit, 0.0) | |
| new SearchPoints($collection, $queryVector, $limit, $this->scoreThreshold) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Services/QdrantService.php` at line 995, The SearchPoints call is
hard-coding the relevance threshold as 0.0 which bypasses semantic filtering;
update the constructor call new SearchPoints($collection, $queryVector, $limit,
0.0) to use the configured threshold $this->scoreThreshold so it respects the
service-level relevance setting (same behavior as searchByProject and
hybridSearch). Locate the call in QdrantService (the new SearchPoints
invocation) and replace the literal 0.0 with $this->scoreThreshold, ensuring the
instance property is available in the method.
🔧 Synapse Sentinel: 2 checks need attentionThe following issues must be resolved before this PR can be merged: Test Failures (1 total)Fix these failing tests: 1. searchRawCollection → it returns results from any collection by nam… 0.02sFAIL at Fix: Review the test expectation vs actual behavior. Check the tested code logic. Security AuditReview the output and fix any issues.Quick Reference:
🤖 Generated by Synapse Sentinel - View Run |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
app/Commands/AnthropicImportCommand.php (2)
167-177:CreateCollectionresponse not checked.If collection creation fails, the command will proceed and fail on subsequent upsert operations. Consider checking the response.
💡 Suggested improvement
private function ensureCollection(string $collection): void { $response = $this->qdrant->send(new GetCollectionInfo($collection)); if ($response->successful()) { return; } - $this->qdrant->send(new CreateCollection($collection, self::VECTOR_SIZE)); - info("Created collection '{$collection}'"); + $createResponse = $this->qdrant->send(new CreateCollection($collection, self::VECTOR_SIZE)); + if (! $createResponse->successful()) { + throw new \RuntimeException("Failed to create collection '{$collection}'"); + } + info("Created collection '{$collection}'"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Commands/AnthropicImportCommand.php` around lines 167 - 177, The ensureCollection method calls CreateCollection via $this->qdrant but does not check the creation response; update ensureCollection to capture the response from $this->qdrant->send(new CreateCollection($collection, self::VECTOR_SIZE)), verify response->successful(), and if it fails log the response/error details (use info/error with the response body or status) and abort further execution (throw an exception or exit) so subsequent upserts won't run on a failed collection creation; reference ensureCollection, CreateCollection, GetCollectionInfo, $this->qdrant, and VECTOR_SIZE when making the change.
429-438: UpsertPoints responses not checked for success.Failed upserts will silently lose data. Consider checking the response and tracking failures, similar to how embedding failures are tracked.
💡 Suggested improvement
// Flush to Qdrant in batches of 200 if (count($pointsBuffer) >= 200) { - $this->qdrant->send(new UpsertPoints($collection, $pointsBuffer)); + $upsertResponse = $this->qdrant->send(new UpsertPoints($collection, $pointsBuffer)); + if (! $upsertResponse->successful()) { + warning('Failed to upsert batch of '.count($pointsBuffer).' points'); + } $pointsBuffer = []; } } // Flush remaining if (! empty($pointsBuffer)) { - $this->qdrant->send(new UpsertPoints($collection, $pointsBuffer)); + $upsertResponse = $this->qdrant->send(new UpsertPoints($collection, $pointsBuffer)); + if (! $upsertResponse->successful()) { + warning('Failed to upsert final batch of '.count($pointsBuffer).' points'); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Commands/AnthropicImportCommand.php` around lines 429 - 438, The UpsertPoints responses from $this->qdrant->send(new UpsertPoints($collection, $pointsBuffer)) are not inspected, so failed upserts can be lost; update the flush logic (both inside the batch-size conditional and the final flush) to capture the response returned by qdrant->send(UpsertPoints), check its success status or error fields, and on failure record the failed point IDs (or the full payload) into the same failure-tracking mechanism used for embedding errors and/or log the error via the command logger; optionally implement a retry loop or increment a failure counter so failures are visible and can be retried or reported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Commands/AnthropicImportCommand.php`:
- Around line 62-66: The QdrantConnector instantiation in AnthropicImportCommand
omits the secure parameter which can break HTTPS connections; update the
constructor call for new QdrantConnector in AnthropicImportCommand to pass the
secure flag (e.g. secure: (bool) config('search.qdrant.secure', false)) similar
to the fix in AnthropicStatusCommand so the connector uses the configured TLS
setting.
- Line 405: Remove the unused variable declaration $results from the scope where
it is declared (it is never read or returned); delete the line creating $results
and ensure there are no remaining references to $results elsewhere in the
surrounding function/method (adjust logic if something was intended to use it).
After removal, run static analysis/tests to confirm no usage remains.
- Around line 227-230: Check the return value of ZipArchive::open($zipPath) in
AnthropicImportCommand (where $zip = new \ZipArchive) and handle failure before
calling extractTo/close: if open() does not return true, log or throw a clear
error mentioning $zipPath and the returned error code/message, clean up $tmpDir
if created, and abort the import flow; only call $zip->extractTo($tmpDir) and
$zip->close() when open() succeeded.
In `@app/Commands/AnthropicStatusCommand.php`:
- Around line 27-31: The QdrantConnector instantiation is missing the secure
parameter which other services (e.g., CodeIndexerService, AgentHealthService)
supply; update the QdrantConnector(...) call in AnthropicStatusCommand to
include secure: config('search.qdrant.secure', false) (or the appropriate config
key) so the connector uses HTTPS when configured, preserving the existing host,
port, and apiKey arguments.
---
Nitpick comments:
In `@app/Commands/AnthropicImportCommand.php`:
- Around line 167-177: The ensureCollection method calls CreateCollection via
$this->qdrant but does not check the creation response; update ensureCollection
to capture the response from $this->qdrant->send(new
CreateCollection($collection, self::VECTOR_SIZE)), verify
response->successful(), and if it fails log the response/error details (use
info/error with the response body or status) and abort further execution (throw
an exception or exit) so subsequent upserts won't run on a failed collection
creation; reference ensureCollection, CreateCollection, GetCollectionInfo,
$this->qdrant, and VECTOR_SIZE when making the change.
- Around line 429-438: The UpsertPoints responses from $this->qdrant->send(new
UpsertPoints($collection, $pointsBuffer)) are not inspected, so failed upserts
can be lost; update the flush logic (both inside the batch-size conditional and
the final flush) to capture the response returned by qdrant->send(UpsertPoints),
check its success status or error fields, and on failure record the failed point
IDs (or the full payload) into the same failure-tracking mechanism used for
embedding errors and/or log the error via the command logger; optionally
implement a retry loop or increment a failure counter so failures are visible
and can be retried or reported.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3d48082c-e6f6-47ff-9d49-5a151146d9b1
📒 Files selected for processing (3)
app/Commands/AnthropicImportCommand.phpapp/Commands/AnthropicStatusCommand.phptests/Unit/Services/QdrantServiceTest.php
| $this->qdrant = new QdrantConnector( | ||
| host: config('search.qdrant.host', 'localhost'), | ||
| port: (int) config('search.qdrant.port', 6333), | ||
| apiKey: config('search.qdrant.api_key'), | ||
| ); |
There was a problem hiding this comment.
Missing secure parameter in QdrantConnector instantiation.
Same issue as in AnthropicStatusCommand — the secure parameter is omitted, which could cause connection failures when Qdrant uses HTTPS.
🔧 Proposed fix
$this->qdrant = new QdrantConnector(
host: config('search.qdrant.host', 'localhost'),
port: (int) config('search.qdrant.port', 6333),
apiKey: config('search.qdrant.api_key'),
+ secure: (bool) config('search.qdrant.secure', false),
);📝 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.
| $this->qdrant = new QdrantConnector( | |
| host: config('search.qdrant.host', 'localhost'), | |
| port: (int) config('search.qdrant.port', 6333), | |
| apiKey: config('search.qdrant.api_key'), | |
| ); | |
| $this->qdrant = new QdrantConnector( | |
| host: config('search.qdrant.host', 'localhost'), | |
| port: (int) config('search.qdrant.port', 6333), | |
| apiKey: config('search.qdrant.api_key'), | |
| secure: (bool) config('search.qdrant.secure', false), | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Commands/AnthropicImportCommand.php` around lines 62 - 66, The
QdrantConnector instantiation in AnthropicImportCommand omits the secure
parameter which can break HTTPS connections; update the constructor call for new
QdrantConnector in AnthropicImportCommand to pass the secure flag (e.g. secure:
(bool) config('search.qdrant.secure', false)) similar to the fix in
AnthropicStatusCommand so the connector uses the configured TLS setting.
| $zip = new \ZipArchive; | ||
| $zip->open($zipPath); | ||
| $zip->extractTo($tmpDir); | ||
| $zip->close(); |
There was a problem hiding this comment.
ZipArchive::open() return value not checked.
ZipArchive::open() returns true on success or an error code on failure. Without checking this, corrupted or invalid zip files will cause confusing downstream errors.
🔧 Proposed fix
$zip = new \ZipArchive;
- $zip->open($zipPath);
+ if ($zip->open($zipPath) !== true) {
+ error("Failed to open zip file: {$zipPath}");
+
+ return [];
+ }
$zip->extractTo($tmpDir);
$zip->close();📝 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.
| $zip = new \ZipArchive; | |
| $zip->open($zipPath); | |
| $zip->extractTo($tmpDir); | |
| $zip->close(); | |
| $zip = new \ZipArchive; | |
| if ($zip->open($zipPath) !== true) { | |
| $this->error("Failed to open zip file: {$zipPath}"); | |
| return []; | |
| } | |
| $zip->extractTo($tmpDir); | |
| $zip->close(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Commands/AnthropicImportCommand.php` around lines 227 - 230, Check the
return value of ZipArchive::open($zipPath) in AnthropicImportCommand (where $zip
= new \ZipArchive) and handle failure before calling extractTo/close: if open()
does not return true, log or throw a clear error mentioning $zipPath and the
returned error code/message, clean up $tmpDir if created, and abort the import
flow; only call $zip->extractTo($tmpDir) and $zip->close() when open()
succeeded.
| $qdrant = new QdrantConnector( | ||
| host: config('search.qdrant.host', 'localhost'), | ||
| port: (int) config('search.qdrant.port', 6333), | ||
| apiKey: config('search.qdrant.api_key'), | ||
| ); |
There was a problem hiding this comment.
Missing secure parameter in QdrantConnector instantiation.
Other services like CodeIndexerService and AgentHealthService include the secure parameter when instantiating QdrantConnector. This omission could cause connection failures when Qdrant is configured to use HTTPS.
🔧 Proposed fix
$qdrant = new QdrantConnector(
host: config('search.qdrant.host', 'localhost'),
port: (int) config('search.qdrant.port', 6333),
apiKey: config('search.qdrant.api_key'),
+ secure: (bool) config('search.qdrant.secure', false),
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Commands/AnthropicStatusCommand.php` around lines 27 - 31, The
QdrantConnector instantiation is missing the secure parameter which other
services (e.g., CodeIndexerService, AgentHealthService) supply; update the
QdrantConnector(...) call in AnthropicStatusCommand to include secure:
config('search.qdrant.secure', false) (or the appropriate config key) so the
connector uses HTTPS when configured, preserving the existing host, port, and
apiKey arguments.
🔧 Synapse Sentinel: 2 checks need attentionThe following issues must be resolved before this PR can be merged: Test Failures (1 total)Fix these failing tests: 1. searchRawCollection → it returns results from any collection by nam… 0.02sFAIL at Fix: Review the test expectation vs actual behavior. Check the tested code logic. Security AuditReview the output and fix any issues.Quick Reference:
🤖 Generated by Synapse Sentinel - View Run |
🔧 Synapse Sentinel: 2 checks need attentionThe following issues must be resolved before this PR can be merged: All tests passed.--- Security AuditReview the output and fix any issues.Quick Reference:
🤖 Generated by Synapse Sentinel - View Run |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
app/Commands/AnthropicSearchCommand.php (5)
77-79: Consider using multibyte-safe string functions.Using
strlen()andsubstr()on UTF-8 text may truncate mid-character, potentially displaying garbled characters. This is a minor cosmetic issue for CLI output.♻️ Proposed fix using mb_* functions
- if (strlen($text) > 300) { - $text = substr($text, 0, 300).'...'; + if (mb_strlen($text) > 300) { + $text = mb_substr($text, 0, 300).'...'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Commands/AnthropicSearchCommand.php` around lines 77 - 79, Replace the byte-oriented strlen/substr calls in AnthropicSearchCommand (the block that truncates $text) with multibyte-safe equivalents: use mb_strlen($text, 'UTF-8') to check length and mb_substr($text, 0, 300, 'UTF-8') to truncate, then append the ellipsis; update the condition and truncation logic where $text is trimmed to ensure UTF-8-safe slicing.
39-53: Consider reusingQdrantService::searchRawCollection()to reduce duplication.The embedding generation and
SearchPointsflow duplicates logic already present inQdrantService::searchRawCollection(). Using the service method would:
- Reduce code duplication
- Benefit from cached embeddings via
getCachedEmbedding()- Centralize search logic for easier maintenance
If there's a specific reason for keeping this separate (e.g., different error handling needs), consider adding a comment to clarify.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Commands/AnthropicSearchCommand.php` around lines 39 - 53, The current code creates a new QdrantConnector and builds a SearchPoints request (using QdrantConnector, SearchPoints) duplicating logic that exists in QdrantService::searchRawCollection(); update this command to call QdrantService::searchRawCollection($collection, $vectorOrInput, $limit, $minScore?) so it reuses getCachedEmbedding() and centralized search/error handling, handle the returned result/response in the same way the service does (or propagate its status to return self::FAILURE on error), and if you intentionally need different behavior keep the QdrantConnector flow but add a clarifying comment above it explaining why it diverges from QdrantService::searchRawCollection().
68-68: Remove unused loop variable$i.The
$iindex variable is declared but never used in the loop body.♻️ Proposed fix
- foreach ($results as $i => $result) { + foreach ($results as $result) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Commands/AnthropicSearchCommand.php` at line 68, The foreach in AnthropicSearchCommand.php declares an unused index variable ($i) — change the loop from "foreach ($results as $i => $result)" to use the value-only form "foreach ($results as $result)" so the unused $i is removed; update any surrounding references if they mistakenly relied on $i and run tests/lint to confirm no usages remain.
61-64: Client-side filtering may return fewer results than requested.The
--senderfilter is applied after fetching$limitresults from Qdrant. If the user requests 10 results but 7 don't match the sender filter, only 3 results are shown.For more accurate results, consider passing the filter to Qdrant's
SearchPointsrequest, or over-fetching and then limiting client-side.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Commands/AnthropicSearchCommand.php` around lines 61 - 64, The current sender filter in AnthropicSearchCommand is applied after fetching $limit from Qdrant which can yield fewer than the requested results; update the SearchPoints request sent from the same command to include a Qdrant filter on the payload.sender field (or, if adding the filter to SearchPoints is not possible, over-fetch by requesting a larger batch from Qdrant and then apply the existing client-side filter before slicing to $limit). Locate the code that builds and calls SearchPoints (within AnthropicSearchCommand) and either add the filter clause to the request payload using the sender value (matching the stored field key used in results: payload.sender) or increase the requested points (e.g., multiply $limit by a safe factor) and then perform the current filter and final array_slice to ensure exactly $limit results are returned.
27-30: Consider validating thatlimitis a positive integer.The limit is cast to int but there's no validation. A value like
--limit=0or--limit=-5would be passed directly to the Qdrant search request, which may result in unexpected behavior or errors.🛡️ Proposed validation
$query = $this->argument('query'); $limit = (int) $this->option('limit'); + if ($limit < 1) { + $limit = 10; + } $collection = $this->option('collection'); $sender = $this->option('sender');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Commands/AnthropicSearchCommand.php` around lines 27 - 30, The code currently casts the --limit option to an int ($limit) without validating it; ensure in the AnthropicSearchCommand (handle method) that $limit is a positive integer (>=1) before calling the Qdrant search: validate $limit after $this->option('limit'), and if it's <= 0 either set a safe default (e.g. 10) or emit an error via $this->error(...) and return a non-zero exit code; update any downstream usage of $limit to rely on this sanitized value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Commands/AnthropicSearchCommand.php`:
- Around line 39-43: QdrantConnector is being instantiated without passing the
secure flag so the config value (QDRANT_SECURE / search.qdrant.secure) is
ignored; update the QdrantConnector(...) call to include the secure parameter by
reading config('search.qdrant.secure', false) (cast to bool if needed) and pass
it as secure: so HTTPS configuration is honored.
---
Nitpick comments:
In `@app/Commands/AnthropicSearchCommand.php`:
- Around line 77-79: Replace the byte-oriented strlen/substr calls in
AnthropicSearchCommand (the block that truncates $text) with multibyte-safe
equivalents: use mb_strlen($text, 'UTF-8') to check length and mb_substr($text,
0, 300, 'UTF-8') to truncate, then append the ellipsis; update the condition and
truncation logic where $text is trimmed to ensure UTF-8-safe slicing.
- Around line 39-53: The current code creates a new QdrantConnector and builds a
SearchPoints request (using QdrantConnector, SearchPoints) duplicating logic
that exists in QdrantService::searchRawCollection(); update this command to call
QdrantService::searchRawCollection($collection, $vectorOrInput, $limit,
$minScore?) so it reuses getCachedEmbedding() and centralized search/error
handling, handle the returned result/response in the same way the service does
(or propagate its status to return self::FAILURE on error), and if you
intentionally need different behavior keep the QdrantConnector flow but add a
clarifying comment above it explaining why it diverges from
QdrantService::searchRawCollection().
- Line 68: The foreach in AnthropicSearchCommand.php declares an unused index
variable ($i) — change the loop from "foreach ($results as $i => $result)" to
use the value-only form "foreach ($results as $result)" so the unused $i is
removed; update any surrounding references if they mistakenly relied on $i and
run tests/lint to confirm no usages remain.
- Around line 61-64: The current sender filter in AnthropicSearchCommand is
applied after fetching $limit from Qdrant which can yield fewer than the
requested results; update the SearchPoints request sent from the same command to
include a Qdrant filter on the payload.sender field (or, if adding the filter to
SearchPoints is not possible, over-fetch by requesting a larger batch from
Qdrant and then apply the existing client-side filter before slicing to $limit).
Locate the code that builds and calls SearchPoints (within
AnthropicSearchCommand) and either add the filter clause to the request payload
using the sender value (matching the stored field key used in results:
payload.sender) or increase the requested points (e.g., multiply $limit by a
safe factor) and then perform the current filter and final array_slice to ensure
exactly $limit results are returned.
- Around line 27-30: The code currently casts the --limit option to an int
($limit) without validating it; ensure in the AnthropicSearchCommand (handle
method) that $limit is a positive integer (>=1) before calling the Qdrant
search: validate $limit after $this->option('limit'), and if it's <= 0 either
set a safe default (e.g. 10) or emit an error via $this->error(...) and return a
non-zero exit code; update any downstream usage of $limit to rely on this
sanitized value.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 477b6f27-8dae-489a-8a88-3233b6a7e485
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
app/Commands/AnthropicSearchCommand.phpcomposer.jsonphpunit.xmltests/Unit/Services/QdrantServiceTest.php
💤 Files with no reviewable changes (1)
- phpunit.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- composer.json
- tests/Unit/Services/QdrantServiceTest.php
| $qdrant = new QdrantConnector( | ||
| host: config('search.qdrant.host', 'localhost'), | ||
| port: (int) config('search.qdrant.port', 6333), | ||
| apiKey: config('search.qdrant.api_key'), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if 'secure' config key exists in search.php
rg -n 'secure' config/search.phpRepository: conduit-ui/knowledge
Length of output: 115
🏁 Script executed:
fd -type f -name "*QdrantConnector*" -o -name "*qdrant*" | grep -i connectorRepository: conduit-ui/knowledge
Length of output: 234
🏁 Script executed:
rg -l "class QdrantConnector" --type phpRepository: conduit-ui/knowledge
Length of output: 107
🏁 Script executed:
rg -A 20 "class QdrantConnector" --type phpRepository: conduit-ui/knowledge
Length of output: 1520
Pass secure config to QdrantConnector constructor.
The secure parameter is not being passed when instantiating QdrantConnector, causing it to default to false. This means any QDRANT_SECURE=true configuration will be ignored, potentially causing HTTPS connection failures.
🔧 Proposed fix
$qdrant = new QdrantConnector(
host: config('search.qdrant.host', 'localhost'),
port: (int) config('search.qdrant.port', 6333),
apiKey: config('search.qdrant.api_key'),
+ secure: (bool) config('search.qdrant.secure', false),
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Commands/AnthropicSearchCommand.php` around lines 39 - 43,
QdrantConnector is being instantiated without passing the secure flag so the
config value (QDRANT_SECURE / search.qdrant.secure) is ignored; update the
QdrantConnector(...) call to include the secure parameter by reading
config('search.qdrant.secure', false) (cast to bool if needed) and pass it as
secure: so HTTPS configuration is honored.
🔧 Synapse Sentinel: 1 check need attentionThe following issues must be resolved before this PR can be merged: Security AuditReview the output and fix any issues.Quick Reference:
🤖 Generated by Synapse Sentinel - View Run |
🏆 Sentinel Certified✅ Tests & Coverage: 0 tests passed Add this badge to your README: [](https://github.com/conduit-ui/knowledge/actions/workflows/gate.yml) |
1 participant
Summary
--collectionflag toknow searchfor querying any Qdrant collection directlyQdrantService::searchRawCollection()— semantic search without knowledge_ prefix or metadata mappingmusic_eventsfrom the know CLIUsage
know search "punk rock" --collection=music_eventsTest plan
--collectionflag works with a populated Qdrant collectionSummary by CodeRabbit
New Features
--collectionCLI option to the search command, allowing users to directly query collections with unfiltered raw results instead of standard formatted output.Tests
Chores