-
Notifications
You must be signed in to change notification settings - Fork 183
fix: add dependencies for pg vector store #312
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
Changes from 5 commits
f1d185a
809f88a
418b180
5146713
8690ba2
cc62e6c
6351281
87e4cf2
6e248a2
fe645fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"create-llama": patch | ||
--- | ||
|
||
Add dependencies for PGVectorStore |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
# coderabbit.yml | ||
reviews: | ||
path_instructions: | ||
- path: "templates/**" | ||
instructions: | | ||
For files under the `templates` folder, do not report 'Missing Dependencies Detected' errors. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
import { expect, test } from "@playwright/test"; | ||
import { exec } from "child_process"; | ||
import fs from "fs"; | ||
import path from "path"; | ||
import util from "util"; | ||
import { TemplateFramework, TemplateVectorDB } from "../helpers/types"; | ||
import { createTestDir, runCreateLlama } from "./utils"; | ||
|
||
const execAsync = util.promisify(exec); | ||
|
||
const templateFramework: TemplateFramework = process.env.FRAMEWORK | ||
? (process.env.FRAMEWORK as TemplateFramework) | ||
: "nextjs"; | ||
const dataSource: string = process.env.DATASOURCE | ||
? process.env.DATASOURCE | ||
: "--example-file"; | ||
|
||
if ( | ||
templateFramework == "nextjs" || | ||
templateFramework == "express" // test is only relevant for TS projects | ||
) { | ||
// vectorDBs combinations to test | ||
const vectorDbs: TemplateVectorDB[] = [ | ||
"mongo", | ||
"pg", | ||
"qdrant", | ||
"pinecone", | ||
"milvus", | ||
"astra", | ||
"chroma", | ||
"llamacloud", | ||
"weaviate", | ||
]; | ||
|
||
test.describe("Test resolve TS dependencies", () => { | ||
for (const vectorDb of vectorDbs) { | ||
const optionDescription = `vectorDb: ${vectorDb}, dataSource: ${dataSource}`; | ||
|
||
test(`options: ${optionDescription}`, async () => { | ||
const cwd = await createTestDir(); | ||
|
||
const result = await runCreateLlama( | ||
cwd, | ||
"streaming", | ||
templateFramework, | ||
dataSource, | ||
vectorDb, | ||
3000, // port | ||
8000, // externalPort | ||
"none", // postInstallAction | ||
undefined, // ui | ||
templateFramework === "nextjs" ? "" : "--no-frontend", // appType | ||
undefined, // llamaCloudProjectName | ||
undefined, // llamaCloudIndexName | ||
); | ||
const name = result.projectName; | ||
|
||
// Check if the app folder exists | ||
const appDir = path.join(cwd, name); | ||
const dirExists = fs.existsSync(appDir); | ||
expect(dirExists).toBeTruthy(); | ||
|
||
// Install dependencies using pnpm | ||
try { | ||
const { stderr: installStderr } = await execAsync( | ||
"pnpm install --prefer-offline", | ||
{ | ||
cwd: appDir, | ||
}, | ||
); | ||
expect(installStderr).toBeFalsy(); | ||
} catch (error) { | ||
console.error("Error installing dependencies:", error); | ||
throw error; | ||
} | ||
|
||
// Run tsc type check and capture the output | ||
try { | ||
const { stdout, stderr } = await execAsync( | ||
"pnpm exec tsc -b --diagnostics", | ||
{ | ||
cwd: appDir, | ||
}, | ||
); | ||
// Check if there's any error output | ||
expect(stderr).toBeFalsy(); | ||
|
||
// Log the stdout for debugging purposes | ||
console.log("TypeScript type-check output:", stdout); | ||
} catch (error) { | ||
console.error("Error running tsc:", error); | ||
throw error; | ||
} | ||
}); | ||
} | ||
}); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,14 @@ | ||
/* eslint-disable turbo/no-undeclared-env-vars */ | ||
import { VectorStoreIndex } from "llamaindex"; | ||
import { ChromaVectorStore } from "llamaindex/storage/vectorStore/ChromaVectorStore"; | ||
import { ChromaVectorStore } from "llamaindex/vector-store/ChromaVectorStore"; | ||
import { checkRequiredEnvVars } from "./shared"; | ||
|
||
export async function getDataSource(params?: any) { | ||
checkRequiredEnvVars(); | ||
const chromaUri = `http://${process.env.CHROMA_HOST}:${process.env.CHROMA_PORT}`; | ||
|
||
const store = new ChromaVectorStore({ | ||
collectionName: process.env.CHROMA_COLLECTION, | ||
collectionName: process.env.CHROMA_COLLECTION!, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider a more robust approach for handling environment variables. While the non-null assertion operator ( Consider using a default value or throwing a more descriptive error: collectionName: process.env.CHROMA_COLLECTION ?? throw new Error("CHROMA_COLLECTION environment variable is not set"), This approach ensures type safety and provides a clear error message if the environment variable is missing. |
||
chromaClientParams: { path: chromaUri }, | ||
}); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
/* eslint-disable turbo/no-undeclared-env-vars */ | ||
import * as dotenv from "dotenv"; | ||
import { storageContextFromDefaults, VectorStoreIndex } from "llamaindex"; | ||
import { MongoDBAtlasVectorSearch } from "llamaindex/storage/vectorStore/MongoDBAtlasVectorStore"; | ||
import { MongoDBAtlasVectorSearch } from "llamaindex/vector-store/MongoDBAtlasVectorStore"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Import path points to a non-existent directory. The new import path Analysis chainLGTM! Verify the new import path. The change to the import path for To ensure this change is correct and functional, please run the following verification steps:
This script will check if Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify the new import path for MongoDBAtlasVectorSearch
# Test: Check if the new import path exists in the llamaindex package
npm list llamaindex | grep -q "llamaindex" && echo "llamaindex is installed" || echo "llamaindex is not installed"
# Note: The following command assumes TypeScript is set up in the project
# If TypeScript is not set up, you may need to adjust the command accordingly
npx tsc --noEmit templates/components/vectordbs/typescript/mongo/generate.ts
Length of output: 328 Script: #!/bin/bash
# Description: Verify the new import path for MongoDBAtlasVectorSearch using fd and ast-grep
# Check if MongoDBAtlasVectorStore exists in the new path
fd "MongoDBAtlasVectorStore" ./llamaindex/vector-store --type f
# Verify the import statement in generate.ts
ast-grep --lang typescript --pattern $'import { MongoDBAtlasVectorSearch } from "llamaindex/vector-store/MongoDBAtlasVectorStore";' templates/components/vectordbs/typescript/mongo/generate.ts
Length of output: 523 |
||
import { MongoClient } from "mongodb"; | ||
import { getDocuments } from "./loader"; | ||
import { initSettings } from "./settings"; | ||
|
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.
💡 Codebase verification
Missing Dependency:
llamaindex
is not listed inpackage.json
.The
ChromaVectorStore
is being imported from thellamaindex
package in multiple TypeScript files, butllamaindex
is not listed underdependencies
,devDependencies
, orpeerDependencies
inpackage.json
.llamaindex
to the appropriate dependency section inpackage.json
to ensure it is correctly installed and available during runtime.Analysis chain
Verify the updated import path for ChromaVectorStore.
The import path for
ChromaVectorStore
has been updated. This change likely reflects a reorganization in the llamaindex library structure.To ensure this change is correct and consistent, please run the following script:
This will help confirm that:
Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 177
Script:
Length of output: 637