feat: wire the-shit/vector as official Qdrant connector#145
Merged
Conversation
Drop the hand-rolled app/Integrations/Qdrant Saloon connector and all its request classes in favour of the-shit/vector ^0.1.1, which provides typed data objects (ScoredPoint, CollectionInfo, ScrollResult, UpsertResult) and a clean VectorClient contract. - QdrantService and CodeIndexerService now inject TheShit\Vector\Qdrant - Register VectorServiceProvider in config/app.php (Laravel Zero) - Delete app/Integrations/Qdrant/ and tests/Unit/Integrations/ - Rewrite QdrantServiceTest (67), HybridSearchTest (9), and CodeIndexerServiceTest (56) to mock Qdrant directly — no more reflection connector injection All 1093 tests pass.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (26)
📝 WalkthroughWalkthroughRemoved the custom Saloon-based Qdrant connector and request DTOs and replaced their usage with the TheShit\Vector\Qdrant client. QdrantService and CodeIndexerService now call typed Qdrant client methods; configuration, DI, and many tests were updated to reflect the client-based integration. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CodeIndexerService
participant QS as QdrantService
participant QC as Qdrant Client\n(TheShit\Vector\Qdrant)
participant QAPI as Qdrant API
rect rgba(200,230,255,0.5)
CI->>QC: upsert/search/scroll/delete (now injected Qdrant)
end
rect rgba(200,255,220,0.5)
QC->>QAPI: HTTP requests to Qdrant (typed DTOs)
QAPI-->>QC: typed responses / errors
end
rect rgba(255,240,200,0.5)
QC-->>CI: typed results / exceptions propagated
note right of CI: CodeIndexerService / QdrantService map ScoredPoint\ndata and handle RequestException
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.46)PHPStan was skipped because the config uses disallowed 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: 1 check need attentionThe following issues must be resolved before this PR can be merged: All tests passed.--- Quick Reference:
🤖 Generated by Synapse Sentinel - View Run |
…runeStaleSymbols Adds tests for RequestException catch branches to bring coverage from 94.8% back to 95.0% (Sentinel Gate threshold).
🔧 Synapse Sentinel: 1 check need attentionThe following issues must be resolved before this PR can be merged: All tests passed.--- 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Services/QdrantService.php (1)
333-367:⚠️ Potential issue | 🟠 Major
$prefetchLimitis currently a no-op.The public method exposes a custom prefetch window, but this value never reaches the client call. Callers tuning recall/latency will always get the library default, so either thread it through or remove the parameter from this service API.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Services/QdrantService.php` around lines 333 - 367, The public hybridSearch method exposes $prefetchLimit but never uses it; update hybridSearch to thread $prefetchLimit into the Qdrant client call by passing it through to $this->qdrant->hybridSearch (e.g., add the named argument prefetch_limit: $prefetchLimit or the client’s expected param name such as prefetchLimit: $prefetchLimit) so the client actually uses the requested prefetch window; if the Qdrant client has no such option, remove the $prefetchLimit parameter from hybridSearch and update callers accordingly.
🧹 Nitpick comments (3)
app/Services/CodeIndexerService.php (1)
36-40: RegisterCodeIndexerServiceexplicitly.The constructor now depends on the external Qdrant client, but
App\Providers\AppServiceProviderstill does not bind this service. Adding an explicit registration keeps command/tool resolution aligned with the repo convention instead of depending on implicit autowiring.As per coding guidelines:
app/Services/**/*.php: Register services inapp/Providers/AppServiceProvider.php.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/Services/CodeIndexerService.php` around lines 36 - 40, The CodeIndexerService constructor now requires EmbeddingServiceInterface and Qdrant (and an optional vector size), so register CodeIndexerService explicitly in AppServiceProvider by adding a container binding that resolves EmbeddingServiceInterface and Qdrant from the container and constructs a new CodeIndexerService (injecting a configurable vector size if applicable); reference the CodeIndexerService class name, EmbeddingServiceInterface and Qdrant so the factory uses $app->make(...) for those dependencies and returns the new instance in the provider's register method.tests/Unit/Services/QdrantServiceTest.php (2)
36-88: Consider extracting test helpers to a dedicated support file.The
if (! function_exists(...))guards work but are unconventional for Pest tests. A cleaner approach would be to extract these factory functions to a dedicated test support file (e.g.,tests/Support/QdrantTestHelpers.php) and include it via Pest'suses()or autoload, or define them as closures inbeforeEach.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/Services/QdrantServiceTest.php` around lines 36 - 88, The test helpers (makeCollectionInfo, makeRequestException, makeUpsertResult, makeScoredPoint, makeScrollResult, mockCollectionExists) are defined inline with function_exists guards; extract these factory helpers into a dedicated test support file (e.g., tests/Support/QdrantTestHelpers.php) and either autoload that file via composer or include it with Pest's uses()/beforeEach so they are globally available to tests; update tests to remove the function_exists blocks and reference the same helper names (makeCollectionInfo, makeRequestException, makeUpsertResult, makeScoredPoint, makeScrollResult, mockCollectionExists) from the new support file.
1368-1377: Consider using named arguments for constructor clarity.The
QdrantServiceconstructor with 7 positional arguments is difficult to read. Named arguments would improve test maintainability.✨ Suggested improvement
$serviceWithCache = new QdrantService( - $this->mockEmbedding, - $this->mockQdrant, - 384, - 0.7, - 604800, - false, - $mockCacheService, + embedding: $this->mockEmbedding, + qdrant: $this->mockQdrant, + vectorDimension: 384, + similarityThreshold: 0.7, + cacheTtl: 604800, + cacheEmbeddings: false, + cacheService: $mockCacheService, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Unit/Services/QdrantServiceTest.php` around lines 1368 - 1377, The test constructs QdrantService with seven positional args which is hard to read; refactor the instantiation in QdrantServiceTest to use PHP named arguments when calling new QdrantService (e.g. specify parameter names like embedding:, qdrantClient:, vectorSize:, scoreThreshold:, ttlSeconds:, useCache:, cacheService:) so the Mockery mock ($mockCacheService) is explicitly passed as cacheService: and all other values are labeled for clarity in the test.
🤖 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/Providers/AppServiceProvider.php`:
- Around line 159-165: The QdrantService binding leaves hybrid mode off because
the factory doesn't supply the sparse embedding dependency or enable hybrid;
update the singleton factory for QdrantService to resolve and pass the sparse
embedding service (e.g., app->make(SparseEmbeddingService::class) or
SparseEmbeddingServiceInterface) and set the hybridEnabled/hybridEmbedding
argument to true when constructing QdrantService (instead of relying on runtime
setSparseEmbeddingService calls), so DI-resolved instances are hybrid-capable by
default.
In `@app/Services/QdrantService.php`:
- Around line 642-664: mapScoredPointToEntry currently drops the payload's
commit field causing read-modify-write flows (e.g. getById →
updateFields/incrementUsage/markSuperseded and subsequent upsert) to erase
commit and break findByTitleAndCommit; update mapScoredPointToEntry(ScoredPoint
$point) to include the payload's commit (e.g. add a 'commit' => $p['commit'] ??
null entry) so the commit is preserved when remapping points back into entries.
In `@composer.json`:
- Around line 24-25: composer.json currently requires "symfony/uid": "^8.0"
which needs PHP >=8.4 while the package platform "php" is declared as ^8.2; to
fix, update composer.json either by lowering the UID dependency to a PHP
8.2-compatible release (e.g., change "symfony/uid" to a 7.x constraint) or by
raising the "php" platform requirement to >=8.4 so constraints resolve; modify
the "symfony/uid" version string or the "php" requirement accordingly and run
composer update to verify dependency resolution.
In `@config/vector.php`:
- Around line 7-13: Update the 'url' entry in config/vector.php to preserve
legacy inputs by building the URL from the older environment keys when
QDRANT_URL is not provided: check env('QDRANT_URL') first, otherwise read
env('QDRANT_HOST') and env('QDRANT_PORT') (and a secure flag like
env('QDRANT_SECURE') or the existing search.qdrant.secure setting used by
App\Providers\AppServiceProvider::loadUserConfig()) and construct the
scheme://host:port fallback (defaulting to http and 6333 as before); ensure the
'url' value uses that constructed string so existing deployments using
QDRANT_HOST/PORT continue to work, and update the config comments/documentation
in the same file to mention both legacy (QDRANT_HOST/QDRANT_PORT/QDRANT_SECURE)
and new (QDRANT_URL/QDRANT_API_KEY) variables.
---
Outside diff comments:
In `@app/Services/QdrantService.php`:
- Around line 333-367: The public hybridSearch method exposes $prefetchLimit but
never uses it; update hybridSearch to thread $prefetchLimit into the Qdrant
client call by passing it through to $this->qdrant->hybridSearch (e.g., add the
named argument prefetch_limit: $prefetchLimit or the client’s expected param
name such as prefetchLimit: $prefetchLimit) so the client actually uses the
requested prefetch window; if the Qdrant client has no such option, remove the
$prefetchLimit parameter from hybridSearch and update callers accordingly.
---
Nitpick comments:
In `@app/Services/CodeIndexerService.php`:
- Around line 36-40: The CodeIndexerService constructor now requires
EmbeddingServiceInterface and Qdrant (and an optional vector size), so register
CodeIndexerService explicitly in AppServiceProvider by adding a container
binding that resolves EmbeddingServiceInterface and Qdrant from the container
and constructs a new CodeIndexerService (injecting a configurable vector size if
applicable); reference the CodeIndexerService class name,
EmbeddingServiceInterface and Qdrant so the factory uses $app->make(...) for
those dependencies and returns the new instance in the provider's register
method.
In `@tests/Unit/Services/QdrantServiceTest.php`:
- Around line 36-88: The test helpers (makeCollectionInfo, makeRequestException,
makeUpsertResult, makeScoredPoint, makeScrollResult, mockCollectionExists) are
defined inline with function_exists guards; extract these factory helpers into a
dedicated test support file (e.g., tests/Support/QdrantTestHelpers.php) and
either autoload that file via composer or include it with Pest's
uses()/beforeEach so they are globally available to tests; update tests to
remove the function_exists blocks and reference the same helper names
(makeCollectionInfo, makeRequestException, makeUpsertResult, makeScoredPoint,
makeScrollResult, mockCollectionExists) from the new support file.
- Around line 1368-1377: The test constructs QdrantService with seven positional
args which is hard to read; refactor the instantiation in QdrantServiceTest to
use PHP named arguments when calling new QdrantService (e.g. specify parameter
names like embedding:, qdrantClient:, vectorSize:, scoreThreshold:, ttlSeconds:,
useCache:, cacheService:) so the Mockery mock ($mockCacheService) is explicitly
passed as cacheService: and all other values are labeled for clarity in the
test.
🪄 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: 8b4fde64-8757-4030-8ff8-691ff140bedb
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
app/Integrations/Qdrant/QdrantConnector.phpapp/Integrations/Qdrant/Requests/CreateCollection.phpapp/Integrations/Qdrant/Requests/DeletePoints.phpapp/Integrations/Qdrant/Requests/GetCollectionInfo.phpapp/Integrations/Qdrant/Requests/GetPoints.phpapp/Integrations/Qdrant/Requests/HybridSearchPoints.phpapp/Integrations/Qdrant/Requests/ListCollections.phpapp/Integrations/Qdrant/Requests/ScrollPoints.phpapp/Integrations/Qdrant/Requests/SearchPoints.phpapp/Integrations/Qdrant/Requests/UpsertPoints.phpapp/Providers/AppServiceProvider.phpapp/Services/CodeIndexerService.phpapp/Services/QdrantService.phpcomposer.jsonconfig/app.phpconfig/vector.phptests/Unit/Integrations/Qdrant/QdrantConnectorTest.phptests/Unit/Integrations/Qdrant/Requests/CreateCollectionHybridTest.phptests/Unit/Integrations/Qdrant/Requests/CreateCollectionTest.phptests/Unit/Integrations/Qdrant/Requests/DeletePointsTest.phptests/Unit/Integrations/Qdrant/Requests/GetCollectionInfoTest.phptests/Unit/Integrations/Qdrant/Requests/GetPointsTest.phptests/Unit/Integrations/Qdrant/Requests/HybridSearchPointsTest.phptests/Unit/Integrations/Qdrant/Requests/ListCollectionsTest.phptests/Unit/Integrations/Qdrant/Requests/SearchPointsTest.phptests/Unit/Integrations/Qdrant/Requests/UpsertPointsTest.phptests/Unit/Services/CodeIndexerServiceTest.phptests/Unit/Services/HybridSearchTest.phptests/Unit/Services/QdrantServiceTest.php
💤 Files with no reviewable changes (20)
- tests/Unit/Integrations/Qdrant/Requests/ListCollectionsTest.php
- app/Integrations/Qdrant/Requests/GetCollectionInfo.php
- app/Integrations/Qdrant/Requests/ListCollections.php
- tests/Unit/Integrations/Qdrant/QdrantConnectorTest.php
- tests/Unit/Integrations/Qdrant/Requests/HybridSearchPointsTest.php
- tests/Unit/Integrations/Qdrant/Requests/SearchPointsTest.php
- tests/Unit/Integrations/Qdrant/Requests/CreateCollectionTest.php
- app/Integrations/Qdrant/Requests/ScrollPoints.php
- app/Integrations/Qdrant/Requests/GetPoints.php
- tests/Unit/Integrations/Qdrant/Requests/UpsertPointsTest.php
- tests/Unit/Integrations/Qdrant/Requests/GetCollectionInfoTest.php
- app/Integrations/Qdrant/Requests/SearchPoints.php
- app/Integrations/Qdrant/QdrantConnector.php
- tests/Unit/Integrations/Qdrant/Requests/DeletePointsTest.php
- tests/Unit/Integrations/Qdrant/Requests/GetPointsTest.php
- app/Integrations/Qdrant/Requests/DeletePoints.php
- app/Integrations/Qdrant/Requests/UpsertPoints.php
- app/Integrations/Qdrant/Requests/CreateCollection.php
- app/Integrations/Qdrant/Requests/HybridSearchPoints.php
- tests/Unit/Integrations/Qdrant/Requests/CreateCollectionHybridTest.php
Comment on lines
159
to
+165
| $this->app->singleton(QdrantService::class, fn ($app): \App\Services\QdrantService => new QdrantService( | ||
| $app->make(EmbeddingServiceInterface::class), | ||
| (int) config('search.embedding_dimension', 1024), | ||
| (float) config('search.minimum_similarity', 0.7), | ||
| (int) config('search.qdrant.cache_ttl', 604800), | ||
| (bool) config('search.qdrant.secure', false), | ||
| cacheService: $app->make(KnowledgeCacheService::class) | ||
| embeddingService: $app->make(EmbeddingServiceInterface::class), | ||
| qdrant: $app->make(Qdrant::class), | ||
| vectorSize: (int) config('search.embedding_dimension', 1024), | ||
| scoreThreshold: (float) config('search.minimum_similarity', 0.7), | ||
| cacheTtl: (int) config('search.qdrant.cache_ttl', 604800), | ||
| cacheService: $app->make(KnowledgeCacheService::class), |
There was a problem hiding this comment.
This factory leaves hybrid mode disabled.
QdrantService still defaults hybridEnabled to false, and this binding does not attach a sparse embedding service. If DI-resolved instances are supposed to support hybrid search, wire both dependencies here instead of relying on the test-only manual setSparseEmbeddingService() setup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Providers/AppServiceProvider.php` around lines 159 - 165, The
QdrantService binding leaves hybrid mode off because the factory doesn't supply
the sparse embedding dependency or enable hybrid; update the singleton factory
for QdrantService to resolve and pass the sparse embedding service (e.g.,
app->make(SparseEmbeddingService::class) or SparseEmbeddingServiceInterface) and
set the hybridEnabled/hybridEmbedding argument to true when constructing
QdrantService (instead of relying on runtime setSparseEmbeddingService calls),
so DI-resolved instances are hybrid-capable by default.
Comment on lines
+642
to
664
| private function mapScoredPointToEntry(ScoredPoint $point): array | ||
| { | ||
| $payload = $point['payload'] ?? []; | ||
| $p = $point->payload; | ||
|
|
||
| return [ | ||
| 'id' => $point['id'], | ||
| 'title' => $payload['title'] ?? '', | ||
| 'content' => $payload['content'] ?? '', | ||
| 'tags' => $this->normalizeTags($payload['tags'] ?? []), | ||
| 'category' => $payload['category'] ?? null, | ||
| 'module' => $payload['module'] ?? null, | ||
| 'priority' => $payload['priority'] ?? null, | ||
| 'status' => $payload['status'] ?? null, | ||
| 'confidence' => $payload['confidence'] ?? 0, | ||
| 'usage_count' => $payload['usage_count'] ?? 0, | ||
| 'created_at' => $payload['created_at'] ?? '', | ||
| 'updated_at' => $payload['updated_at'] ?? '', | ||
| 'last_verified' => $payload['last_verified'] ?? null, | ||
| 'evidence' => $payload['evidence'] ?? null, | ||
| 'superseded_by' => $payload['superseded_by'] ?? null, | ||
| 'superseded_date' => $payload['superseded_date'] ?? null, | ||
| 'superseded_reason' => $payload['superseded_reason'] ?? null, | ||
| 'id' => $point->id, | ||
| 'title' => $p['title'] ?? '', | ||
| 'content' => $p['content'] ?? '', | ||
| 'tags' => $this->normalizeTags($p['tags'] ?? []), | ||
| 'category' => $p['category'] ?? null, | ||
| 'module' => $p['module'] ?? null, | ||
| 'priority' => $p['priority'] ?? null, | ||
| 'status' => $p['status'] ?? null, | ||
| 'confidence' => $p['confidence'] ?? 0, | ||
| 'usage_count' => $p['usage_count'] ?? 0, | ||
| 'created_at' => $p['created_at'] ?? '', | ||
| 'updated_at' => $p['updated_at'] ?? '', | ||
| 'last_verified' => $p['last_verified'] ?? null, | ||
| 'evidence' => $p['evidence'] ?? null, | ||
| 'superseded_by' => $p['superseded_by'] ?? null, | ||
| 'superseded_date' => $p['superseded_date'] ?? null, | ||
| 'superseded_reason' => $p['superseded_reason'] ?? null, | ||
| ]; |
There was a problem hiding this comment.
Preserve commit when remapping points.
upsert() still writes a commit payload key, but mapScoredPointToEntry() drops it. Any read-modify-write path (getById() → updateFields() / incrementUsage() / markSuperseded()) will erase the stored commit on the next upsert and break findByTitleAndCommit() for that record.
💡 Minimal fix
private function mapScoredPointToEntry(ScoredPoint $point): array
{
$p = $point->payload;
return [
'id' => $point->id,
'title' => $p['title'] ?? '',
'content' => $p['content'] ?? '',
'tags' => $this->normalizeTags($p['tags'] ?? []),
'category' => $p['category'] ?? null,
'module' => $p['module'] ?? null,
'priority' => $p['priority'] ?? null,
'status' => $p['status'] ?? null,
'confidence' => $p['confidence'] ?? 0,
'usage_count' => $p['usage_count'] ?? 0,
'created_at' => $p['created_at'] ?? '',
'updated_at' => $p['updated_at'] ?? '',
'last_verified' => $p['last_verified'] ?? null,
'evidence' => $p['evidence'] ?? null,
+ 'commit' => $p['commit'] ?? null,
'superseded_by' => $p['superseded_by'] ?? null,
'superseded_date' => $p['superseded_date'] ?? null,
'superseded_reason' => $p['superseded_reason'] ?? null,
];
}📝 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
| private function mapScoredPointToEntry(ScoredPoint $point): array | |
| { | |
| $payload = $point['payload'] ?? []; | |
| $p = $point->payload; | |
| return [ | |
| 'id' => $point['id'], | |
| 'title' => $payload['title'] ?? '', | |
| 'content' => $payload['content'] ?? '', | |
| 'tags' => $this->normalizeTags($payload['tags'] ?? []), | |
| 'category' => $payload['category'] ?? null, | |
| 'module' => $payload['module'] ?? null, | |
| 'priority' => $payload['priority'] ?? null, | |
| 'status' => $payload['status'] ?? null, | |
| 'confidence' => $payload['confidence'] ?? 0, | |
| 'usage_count' => $payload['usage_count'] ?? 0, | |
| 'created_at' => $payload['created_at'] ?? '', | |
| 'updated_at' => $payload['updated_at'] ?? '', | |
| 'last_verified' => $payload['last_verified'] ?? null, | |
| 'evidence' => $payload['evidence'] ?? null, | |
| 'superseded_by' => $payload['superseded_by'] ?? null, | |
| 'superseded_date' => $payload['superseded_date'] ?? null, | |
| 'superseded_reason' => $payload['superseded_reason'] ?? null, | |
| 'id' => $point->id, | |
| 'title' => $p['title'] ?? '', | |
| 'content' => $p['content'] ?? '', | |
| 'tags' => $this->normalizeTags($p['tags'] ?? []), | |
| 'category' => $p['category'] ?? null, | |
| 'module' => $p['module'] ?? null, | |
| 'priority' => $p['priority'] ?? null, | |
| 'status' => $p['status'] ?? null, | |
| 'confidence' => $p['confidence'] ?? 0, | |
| 'usage_count' => $p['usage_count'] ?? 0, | |
| 'created_at' => $p['created_at'] ?? '', | |
| 'updated_at' => $p['updated_at'] ?? '', | |
| 'last_verified' => $p['last_verified'] ?? null, | |
| 'evidence' => $p['evidence'] ?? null, | |
| 'superseded_by' => $p['superseded_by'] ?? null, | |
| 'superseded_date' => $p['superseded_date'] ?? null, | |
| 'superseded_reason' => $p['superseded_reason'] ?? null, | |
| ]; | |
| private function mapScoredPointToEntry(ScoredPoint $point): array | |
| { | |
| $p = $point->payload; | |
| return [ | |
| 'id' => $point->id, | |
| 'title' => $p['title'] ?? '', | |
| 'content' => $p['content'] ?? '', | |
| 'tags' => $this->normalizeTags($p['tags'] ?? []), | |
| 'category' => $p['category'] ?? null, | |
| 'module' => $p['module'] ?? null, | |
| 'priority' => $p['priority'] ?? null, | |
| 'status' => $p['status'] ?? null, | |
| 'confidence' => $p['confidence'] ?? 0, | |
| 'usage_count' => $p['usage_count'] ?? 0, | |
| 'created_at' => $p['created_at'] ?? '', | |
| 'updated_at' => $p['updated_at'] ?? '', | |
| 'last_verified' => $p['last_verified'] ?? null, | |
| 'evidence' => $p['evidence'] ?? null, | |
| 'commit' => $p['commit'] ?? null, | |
| 'superseded_by' => $p['superseded_by'] ?? null, | |
| 'superseded_date' => $p['superseded_date'] ?? null, | |
| 'superseded_reason' => $p['superseded_reason'] ?? null, | |
| ]; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Services/QdrantService.php` around lines 642 - 664, mapScoredPointToEntry
currently drops the payload's commit field causing read-modify-write flows (e.g.
getById → updateFields/incrementUsage/markSuperseded and subsequent upsert) to
erase commit and break findByTitleAndCommit; update
mapScoredPointToEntry(ScoredPoint $point) to include the payload's commit (e.g.
add a 'commit' => $p['commit'] ?? null entry) so the commit is preserved when
remapping points back into entries.
Comment on lines
+24
to
+25
| "symfony/uid": "^8.0", | ||
| "the-shit/vector": "^0.1.1" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n composer.json | head -30Repository: conduit-ui/knowledge
Length of output: 1283
🌐 Web query:
symfony/uid 8.0 PHP requirements packagist
💡 Result:
On Packagist, symfony/uid v8.0.x (e.g., v8.0.8) declares:
- PHP requirement:
>= 8.4 - Runtime dependency:
symfony/polyfill-uuid: ^1.15 - Dev requirement:
symfony/console: ^7.4|^8.0[1]
symfony/uid 8.x breaks the advertised PHP floor.
The repo declares PHP ^8.2, but symfony/uid 8.x requires PHP >=8.4. This constraint set will not resolve on PHP 8.2 or 8.3 installations. Either downgrade UID to a PHP 8.2-compatible version or raise the declared PHP minimum before merging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@composer.json` around lines 24 - 25, composer.json currently requires
"symfony/uid": "^8.0" which needs PHP >=8.4 while the package platform "php" is
declared as ^8.2; to fix, update composer.json either by lowering the UID
dependency to a PHP 8.2-compatible release (e.g., change "symfony/uid" to a 7.x
constraint) or by raising the "php" platform requirement to >=8.4 so constraints
resolve; modify the "symfony/uid" version string or the "php" requirement
accordingly and run composer update to verify dependency resolution.
Comment on lines
+7
to
+13
| 'url' => env('QDRANT_URL', 'http://localhost:6333'), | ||
|
|
||
| 'api_key' => env('QDRANT_API_KEY'), | ||
|
|
||
| 'timeout' => [ | ||
| 'connect' => (int) env('QDRANT_CONNECT_TIMEOUT', 10), | ||
| 'request' => (int) env('QDRANT_REQUEST_TIMEOUT', 30), |
There was a problem hiding this comment.
Keep the legacy Qdrant config inputs working.
This file now reads only QDRANT_URL, but .env.example:1-25 still documents QDRANT_HOST / QDRANT_PORT, and App\Providers\AppServiceProvider::loadUserConfig() still rewrites search.qdrant.host/port/secure. Any deployment using those existing inputs will now fall back to http://localhost:6333 unless it also adds the new URL var. Please bridge the old keys here and document the new ones in the same PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@config/vector.php` around lines 7 - 13, Update the 'url' entry in
config/vector.php to preserve legacy inputs by building the URL from the older
environment keys when QDRANT_URL is not provided: check env('QDRANT_URL') first,
otherwise read env('QDRANT_HOST') and env('QDRANT_PORT') (and a secure flag like
env('QDRANT_SECURE') or the existing search.qdrant.secure setting used by
App\Providers\AppServiceProvider::loadUserConfig()) and construct the
scheme://host:port fallback (defaulting to http and 6333 as before); ensure the
'url' value uses that constructed string so existing deployments using
QDRANT_HOST/PORT continue to work, and update the config comments/documentation
in the same file to mention both legacy (QDRANT_HOST/QDRANT_PORT/QDRANT_SECURE)
and new (QDRANT_URL/QDRANT_API_KEY) variables.
…ands Add 43 new tests covering every remaining uncovered code path: - MCP tool schema() methods (8 tools) - ContextTool edge cases: invalid dates, null categories, token budget overflow - RecallTool global search across multiple collections - McpServiceProvider container callback for mcp.request propagation - ProjectDetectorService: URL extraction fallback, sanitize-to-empty - OllamaService: non-200 status, malformed JSON, invalid categories - EnhancementQueueService: directory creation paths - TieredSearchService: deduplication across tiers - SymbolIndexService: getSymbolSourceByNameAndFile paths - Command coverage: --project flag, --collection, --global, --uninstall, install path, missing index files, ensureCollection failure, progress callbacks, prune output, enhance worker failure counting Mark truly untestable defensive guards with @codeCoverageIgnore: - tempnam() failure, file_get_contents race after file_exists, non-string CLI argument guard, createClient() fallback 1140 tests, 3281 assertions, 100.0% coverage.
🏆 Sentinel Certified✅ Tests & Coverage: 0 tests passed Add this badge to your README: [](https://github.com/conduit-ui/knowledge/actions/workflows/gate.yml) |
5 tasks
4 tasks
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
1 participant
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.
Summary
app/Integrations/Qdrant/Saloon connector (10 request classes + connector) withthe-shit/vector:^0.1.1QdrantServiceandCodeIndexerServicenow injectTheShit\Vector\Qdrantwith typed return objects (ScoredPoint,CollectionInfo,ScrollResult,UpsertResult)VectorServiceProviderinconfig/app.phpfor Laravel Zeroapp/Integrations/Qdrant/andtests/Unit/Integrations/(all coverage now lives in the package)QdrantServiceTest,HybridSearchTest, andCodeIndexerServiceTestto mockQdrantdirectly — no more reflection connector injectionTest plan
vendor/bin/pint --dirtycleanknow ingestworks against live Qdrantknow searchreturns results from existing collectionsSummary by CodeRabbit
New Features
Chores
Tests