Skip to content

fix(qdrant): add URL prefix handling for QdrantClient initialization #5033

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Jun 23, 2025
Merged

fix(qdrant): add URL prefix handling for QdrantClient initialization #5033

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9b09f08
feat(qdrant): add URL prefix handling for QdrantClient initialization
CW-B-W Jun 23, 2025
6450215
Merge remote-tracking branch 'upstream/main' into fix_qdrant_connecti…
CW-B-W Jun 23, 2025
32ffeb4
fix: Normalize Qdrant URL pathname to remove trailing slashes
CW-B-W Jun 23, 2025
e522eb3
fix: Normalize Qdrant URL pathname to remove multiple trailing slashes
CW-B-W Jun 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions src/services/code-index/vector-store/__tests__/qdrant-client.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ describe("QdrantVectorStore", () => {
host: "qdrant.ashbyfam.com",
https: true,
port: 443,
prefix: undefined, // No prefix for root path
apiKey: undefined,
headers: {
"User-Agent": "Roo-Code",
Expand All @@ -127,6 +128,7 @@ describe("QdrantVectorStore", () => {
host: "example.com",
https: true,
port: 9000,
prefix: undefined, // No prefix for root path
apiKey: undefined,
headers: {
"User-Agent": "Roo-Code",
Expand All @@ -145,6 +147,7 @@ describe("QdrantVectorStore", () => {
host: "example.com",
https: true,
port: 443,
prefix: "/api/v1", // Should have prefix
apiKey: undefined,
headers: {
"User-Agent": "Roo-Code",
Expand All @@ -161,6 +164,7 @@ describe("QdrantVectorStore", () => {
host: "example.com",
https: false,
port: 80,
prefix: undefined, // No prefix for root path
apiKey: undefined,
headers: {
"User-Agent": "Roo-Code",
Expand All @@ -175,6 +179,7 @@ describe("QdrantVectorStore", () => {
host: "localhost",
https: false,
port: 8080,
prefix: undefined, // No prefix for root path
apiKey: undefined,
headers: {
"User-Agent": "Roo-Code",
Expand All @@ -193,6 +198,7 @@ describe("QdrantVectorStore", () => {
host: "example.com",
https: false,
port: 80,
prefix: "/api/v1", // Should have prefix
apiKey: undefined,
headers: {
"User-Agent": "Roo-Code",
Expand Down Expand Up @@ -337,6 +343,65 @@ describe("QdrantVectorStore", () => {
})
})

describe("URL Prefix Handling", () => {
it("should pass the URL pathname as prefix to QdrantClient if not root", () => {
const vectorStoreWithPrefix = new QdrantVectorStore(
mockWorkspacePath,
"http://localhost:6333/some/path",
mockVectorSize,
)
expect(QdrantClient).toHaveBeenLastCalledWith({
host: "localhost",
https: false,
port: 6333,
prefix: "/some/path",
apiKey: undefined,
headers: {
"User-Agent": "Roo-Code",
},
})
expect((vectorStoreWithPrefix as any).qdrantUrl).toBe("http://localhost:6333/some/path")
})

it("should not pass prefix if the URL pathname is root ('/')", () => {
const vectorStoreWithoutPrefix = new QdrantVectorStore(
mockWorkspacePath,
"http://localhost:6333/",
mockVectorSize,
)
expect(QdrantClient).toHaveBeenLastCalledWith({
host: "localhost",
https: false,
port: 6333,
prefix: undefined,
apiKey: undefined,
headers: {
"User-Agent": "Roo-Code",
},
})
expect((vectorStoreWithoutPrefix as any).qdrantUrl).toBe("http://localhost:6333/")
})

it("should handle HTTPS URL with path as prefix", () => {
const vectorStoreWithHttpsPrefix = new QdrantVectorStore(
mockWorkspacePath,
"https://qdrant.ashbyfam.com/api",
mockVectorSize,
)
expect(QdrantClient).toHaveBeenLastCalledWith({
host: "qdrant.ashbyfam.com",
https: true,
port: 443,
prefix: "/api",
apiKey: undefined,
headers: {
"User-Agent": "Roo-Code",
},
})
expect((vectorStoreWithHttpsPrefix as any).qdrantUrl).toBe("https://qdrant.ashbyfam.com/api")
})
})

describe("initialize", () => {
it("should create a new collection if none exists and return true", async () => {
// Mock getCollection to throw a 404-like error
Expand Down
2 changes: 2 additions & 0 deletions src/services/code-index/vector-store/qdrant-client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,15 @@ export class QdrantVectorStore implements IVectorStore {
host: urlObj.hostname,
https: useHttps,
port: port,
prefix: urlObj.pathname === "/" ? undefined : urlObj.pathname,
Copy link
Member

@daniel-lxs daniel-lxs Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that URLs with trailing slashes aren't normalized. Would http://localhost:6333/api/ and http://localhost:6333/api result in different prefix values (/api/ vs /api)? If so, should we consider normalizing the pathname to ensure consistent behavior?

For example:

prefix: urlObj.pathname === "/" ? undefined : urlObj.pathname.replace(/\/$/, '')

Copy link
Contributor Author

@CW-B-W CW-B-W Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate for you suggestion, I apprently ignored that.
I have revised it and committed.

apiKey,
headers: {
"User-Agent": "Roo-Code",
},
})
} catch (urlError) {
// If URL parsing fails, fall back to URL-based config
// Note: This fallback won't correctly handle prefixes, but it's a last resort for malformed URLs.
this.client = new QdrantClient({
url: parsedUrl,
apiKey,
Expand Down