-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat: Add Qdrant collection name field to codebase indexing settings #7729
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add UI field in CodeIndexPopover for viewing/editing collection name - Update configuration interfaces to include qdrantCollectionName - Modify QdrantVectorStore to accept optional collection name parameter - Update config manager to handle collection name in settings - Add translation keys for new UI labels - Update tests to account for new parameter Fixes #7728
"searchMaxResultsDescription": "查询代码库索引时返回的最大搜索结果数。较高的值提供更多上下文,但可能包含相关性较低的结果。", | ||
"resetToDefault": "恢复默认值" | ||
"resetToDefault": "恢复默认值", | ||
"qdrantCollectionNameLabel": "Qdrant Collection Name", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
新添加的 Qdrant collection name 字段(qdrantCollectionNameLabel, qdrantCollectionNamePlaceholder, qdrantCollectionNameDescription)目前保持英文。考虑提供中文翻译以保证本地化一致性,或者在注释中说明暂未翻译的原因。
"qdrantCollectionNameLabel": "Qdrant Collection Name", | |
"qdrantCollectionNameLabel": "Qdrant 集合名称", |
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
"searchMaxResultsDescription": "查詢程式碼庫索引時傳回的最大搜尋結果數。較高的值提供更多上下文,但可能包含相關性較低的結果。", | ||
"resetToDefault": "重設為預設值" | ||
"resetToDefault": "重設為預設值", | ||
"qdrantCollectionNameLabel": "Qdrant Collection Name", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
在此文件中,新加入的 Qdrant collection name 字段仍為英文(qdrantCollectionNameLabel、qdrantCollectionNamePlaceholder、qdrantCollectionNameDescription)。建議為繁體中文用戶提供相應翻譯,或在註釋中標明其暫時保留英文。
"qdrantCollectionNameLabel": "Qdrant Collection Name", | |
"qdrantCollectionNameLabel": "Qdrant 集合名稱", |
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed my own code and found issues. Classic case of 'it worked on my machine syndrome'.
|
||
// Check for model dimension changes (generic for all providers) | ||
if (prevModelDimension !== currentModelDimension) { | ||
return true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical Issue: Missing restart detection for collection name changes.
The doesConfigChangeRequireRestart
method doesn't check if qdrantCollectionName
has changed. When users change the collection name, it requires a restart to take effect (since it affects which Qdrant collection is used), but this won't be detected.
Could we add this check around line 351?
return true | |
if ( | |
prevQdrantUrl !== currentQdrantUrl || | |
prevQdrantCollectionName !== currentQdrantCollectionName || | |
prevQdrantApiKey !== currentQdrantApiKey | |
) { | |
return true | |
} |
modelDimension: undefined, | ||
qdrantUrl: "http://localhost:6333", | ||
qdrantApiKey: "", | ||
qdrantCollectionName: "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important: Missing test coverage for collection name functionality.
While the test file has been updated to include qdrantCollectionName
in assertions, there are no specific tests for:
- Collection name changes triggering restart detection
- Collection name being properly loaded and saved
- Edge cases like empty or invalid collection names
Would it make sense to add dedicated test cases for this new feature?
// Generate collection name from workspace path | ||
const hash = createHash("sha256").update(workspacePath).digest("hex") | ||
// Use provided collection name or generate from workspace path | ||
if (collectionName && collectionName.trim()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice sanitization logic! However, could we add a comment explaining the regex pattern for future maintainers?
if (collectionName && collectionName.trim()) { | |
// Use provided collection name or generate from workspace path | |
if (collectionName && collectionName.trim()) { | |
// Sanitize the collection name to ensure it's valid for Qdrant | |
// Qdrant collection names must match: ^[a-zA-Z0-9_-]+$ | |
// Invalid characters are replaced with hyphens | |
this.collectionName = collectionName.trim().replace(/[^a-zA-Z0-9_-]/g, "-") | |
} else { |
onInput={(e: any) => | ||
updateSetting("codebaseIndexQdrantCollectionName", e.target.value) | ||
} | ||
placeholder={t("settings:codeIndex.qdrantCollectionNamePlaceholder")} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: The UI doesn't validate collection names before saving.
While the backend sanitizes invalid characters, it might be better UX to warn users in the UI when they enter invalid characters. Could we add validation feedback similar to other fields?
Also, is the placeholder text informative enough about valid characters? Maybe something like: "Enter collection name (letters, numbers, -, _ only)"?
"http://localhost:6333", | ||
3072, | ||
"test-key", | ||
undefined, // collectionName |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test correctly passes undefined
for the new collectionName
parameter, but we're missing tests that verify:
- Collection name is passed when configured
- Sanitization works correctly
- Empty/invalid collection names fall back to auto-generation
Should we add test coverage for the new parameter?
Fixes #7728
Summary
This PR adds the ability to view and edit Qdrant collection names in the codebase indexing popup, allowing users to specify custom collection names instead of relying on auto-generated names from workspace path hashes.
Changes
qdrantCollectionName
fieldBenefits
Testing
Screenshots
The new field appears in the codebase indexing settings popup with proper labels and placeholder text.
Important
Adds support for custom Qdrant collection names in codebase indexing settings, including UI, configuration, and backend updates.
codebaseIndexQdrantCollectionName
field tocodebase-index.ts
,webviewMessageHandler.ts
, andconfig-manager.ts
for custom Qdrant collection names.CodeIndexPopover.tsx
to include a new input field for Qdrant collection name.QdrantVectorStore
constructor inqdrant-client.ts
to accept and sanitize custom collection names.qdrantCollectionName
.service-factory.ts
to handle optional collection name.This description was created by
for 71e464b. You can customize this summary. It will automatically update as commits are pushed.