feat: support multiple vector store backends - #11
Open
ashrobertsdragon wants to merge 1 commit into
Open
Conversation
… Pinecone, MongoDB)
Reviewer's GuideDecouples the core RAG pipeline from a hard‑coded pgvector/Postgres dependency by introducing a VectorStoreAdapter protocol, wiring main/rag to this abstraction, and adding concrete adapters, config, and docs for multiple optional vector store backends (Postgres, Qdrant, Chroma, Milvus, Pinecone, MongoDB). File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Review Summary by QodoSupport multiple vector store backends with pluggable adapter pattern
WalkthroughsDescription• Implement pluggable vector store architecture supporting 6 backends - PostgreSQL (pgvector), Qdrant, Chroma, Milvus, Pinecone, MongoDB • Move database dependencies to optional extras in pyproject.toml • Add lazy loading of database drivers based on VECTOR_STORE configuration • Refactor RAG core logic to use VectorStoreAdapter protocol interface • Update documentation with vector store setup and configuration guide Diagramflowchart LR
Config["VECTOR_STORE Config"]
Main["main.py<br/>start_store()"]
Adapter["VectorStoreAdapter<br/>Protocol"]
PG["PostgresAdapter"]
QD["QdrantAdapter"]
CH["ChromaAdapter"]
MV["MilvusAdapter"]
PN["PineconeAdapter"]
MG["MongoDBAdapter"]
RAG["MarkdownRAG<br/>Core Logic"]
Config -->|selects backend| Main
Main -->|lazy loads| PG
Main -->|lazy loads| QD
Main -->|lazy loads| CH
Main -->|lazy loads| MV
Main -->|lazy loads| PN
Main -->|lazy loads| MG
PG -->|implements| Adapter
QD -->|implements| Adapter
CH -->|implements| Adapter
MV -->|implements| Adapter
PN -->|implements| Adapter
MG -->|implements| Adapter
Adapter -->|decouples| RAG
File Changes1. src/markdown_rag/config.py
|
Code Review by Qodo
Context used✅ Tickets:
🎫 Add support for other vector stores 1. delete_filename() always returns True
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The
PineconeAdapter’slist_filenamesandexistsmethods rely on a hard-coded dummy vector, largetop_k, and aget_pinecone_index()call, which may be brittle and inefficient for larger indexes; consider either using Pinecone’s metadata filtering APIs directly (without embedding) or clearly documenting and constraining this implementation (e.g., configurable vector size/top_k) to avoid performance surprises. - In
MilvusAdapter.existsthe filter expression is built via string interpolation (e.g.,f"{k} == '{v}'"), which will break if values contain quotes and is hard to make safe; prefer using Milvus’ recommended expression-building patterns or sanitizing/escaping values before constructing the expression. - The
start_storefunction inmain.pyhas grown into a largematchblock with backend-specific wiring; consider extracting acreate_vector_store_adapter(env, embeddings, directory)factory to keepmainfocused on orchestration and make adding/removing backends simpler.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `PineconeAdapter`’s `list_filenames` and `exists` methods rely on a hard-coded dummy vector, large `top_k`, and a `get_pinecone_index()` call, which may be brittle and inefficient for larger indexes; consider either using Pinecone’s metadata filtering APIs directly (without embedding) or clearly documenting and constraining this implementation (e.g., configurable vector size/top_k) to avoid performance surprises.
- In `MilvusAdapter.exists` the filter expression is built via string interpolation (e.g., `f"{k} == '{v}'"`), which will break if values contain quotes and is hard to make safe; prefer using Milvus’ recommended expression-building patterns or sanitizing/escaping values before constructing the expression.
- The `start_store` function in `main.py` has grown into a large `match` block with backend-specific wiring; consider extracting a `create_vector_store_adapter(env, embeddings, directory)` factory to keep `main` focused on orchestration and make adding/removing backends simpler.
## Individual Comments
### Comment 1
<location path="src/markdown_rag/vector_stores/pinecone.py" line_range="44-52" />
<code_context>
+ # A common workaround is to use a zero vector for a broad search.
+ # Note: This might be slow for very large datasets.
+
+ # Access the raw index client
+ index = self.vector_store.get_pinecone_index()
+
+ # We can't easily list all without a query in standard Pinecone.
+ # Using a workaround: list all by prefix if possible, or query with zero vector.
+ # Here we'll just try to fetch a large sample.
+
+ results = index.query(
+ vector=[0] * 1536, # Dummy vector size
+ top_k=10000,
+ include_metadata=True
</code_context>
<issue_to_address>
**issue (performance):** Avoid hardcoding the Pinecone embedding dimension and large broad queries in list_filenames.
This assumes a 1536‑dimensional embedding and issues a single `top_k=10000` query, which ties this code to a specific model and can become slow/expensive on large indexes. It also depends on `get_pinecone_index()`, which may not be a stable public API. Consider deriving the dimension from index metadata or the embeddings object and iterating with pagination over the raw client (e.g., `list` or repeated `query` calls with smaller `top_k`) instead of one broad dummy‑vector query. If full refactoring isn’t feasible now, at least make the dimension configurable rather than hard‑coded.
</issue_to_address>
### Comment 2
<location path="src/markdown_rag/vector_stores/pinecone.py" line_range="73-74" />
<code_context>
+ def exists(self, metadata: dict[str, str]) -> bool:
+ """Check if documents with given metadata exist."""
+ # Use similarity search with filter to check existence
+ docs = self.vector_store.similarity_search(
+ "dummy", k=1, filter=metadata
+ )
+ return len(docs) > 0
</code_context>
<issue_to_address>
**suggestion (performance):** Using similarity_search with a dummy query for existence checks is likely inefficient.
This implementation always performs an embedding + similarity query just to check presence, which can add avoidable latency and cost if `exists` is called often. Prefer using the underlying Pinecone index client for a minimal `query` with a fixed/cheap vector, or (if available) a metadata-only lookup. As another option, you could make this existence check optional/best-effort for Pinecone-backed stores to avoid unnecessary embedding calls.
</issue_to_address>
### Comment 3
<location path="src/markdown_rag/vector_stores/milvus.py" line_range="47-52" />
<code_context>
+
+ def delete_filename(self, filename: str) -> bool:
+ """Delete all documents associated with a filename."""
+ self.vector_store.col.delete(expr=f"filename == '{filename}'")
+ return True
+
+ def exists(self, metadata: dict[str, str]) -> bool:
+ """Check if documents with given metadata exist."""
+ expr = " and ".join([f"{k} == '{v}'" for k, v in metadata.items()])
+ res = self.vector_store.col.query(
+ expr=expr,
</code_context>
<issue_to_address>
**🚨 issue (security):** String-interpolated expressions for Milvus queries risk quoting bugs and injection issues.
`delete_filename` and `exists` build Milvus `expr` strings directly from user data via f-strings (e.g., `filename == '{filename}'`, `" and ".join([f"{k} == '{v}'" ...])`). If any value contains quotes or special characters, the query can break or be exploitable. These values should be safely escaped or validated, ideally via a Milvus query builder or parameterized API rather than manual string concatenation.
</issue_to_address>
### Comment 4
<location path="src/markdown_rag/vector_stores/pinecone.py" line_range="52" />
<code_context>
+ # Here we'll just try to fetch a large sample.
+
+ results = index.query(
+ vector=[0] * 1536, # Dummy vector size
+ top_k=10000,
+ include_metadata=True
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the Pinecone adapter to centralize the dummy vector logic, infer the embedding dimension dynamically, and use direct Pinecone queries for metadata checks instead of similarity_search.
You can reduce the “surprising” complexity without dropping functionality by:
1. Removing the hard‑coded embedding dimension.
2. Avoiding similarity_search for non‑similarity operations.
3. Centralizing the “dummy query” behavior in a small internal helper so callers see a clear, honest API.
### 1. Avoid hard‑coded 1536 and expose the approximation
Derive the dimension once from the configured embeddings instead of baking in `1536`, and make the “broad scan” behavior explicit:
```python
class PineconeAdapter(VectorStoreAdapter):
def __init__(self, vector_store: "PineconeVectorStore"):
self.vector_store = vector_store
# Infer dimension from the configured embeddings
sample_vec = self.vector_store._embedding.embed_query("") # or similar API
self._dim = len(sample_vec)
def _dummy_vector(self) -> list[float]:
# Single place where we define the "broad search" dummy vector
return [0.0] * self._dim
def list_filenames(self) -> list[str]:
"""Best-effort listing of unique filenames using a broad query.
Note: This uses a dummy vector to approximate "scan all" and may be slow
for large indexes.
"""
index = self.vector_store.get_pinecone_index()
results = index.query(
vector=self._dummy_vector(),
top_k=10_000,
include_metadata=True,
)
filenames: set[str] = set()
for match in results.get("matches", []):
metadata = match.get("metadata") or {}
filename = metadata.get("filename")
if filename:
filenames.add(filename)
return sorted(filenames)
```
This removes the magic `1536`, keeps the behavior, and documents that it is an approximation with potential performance cost.
### 2. Use Pinecone query directly for `exists`, not similarity_search
You can keep semantics but avoid the “dummy text” similarity_search and make the metadata‑filter behavior obvious:
```python
class PineconeAdapter(VectorStoreAdapter):
...
def exists(self, metadata: dict[str, str]) -> bool:
"""Check if documents with given metadata exist.
This relies on a minimal Pinecone query with a dummy vector, using only
the metadata filter for existence.
"""
index = self.vector_store.get_pinecone_index()
res = index.query(
vector=self._dummy_vector(),
top_k=1,
filter=metadata,
include_metadata=False,
)
return bool(res.get("matches"))
```
This:
- Decouples existence from a semantic “similarity search” API.
- Keeps the same functional behavior (metadata‑filtered query with `top_k=1`).
- Moves all the “hackiness” into `_dummy_vector` + clear docstrings, so callers aren’t surprised by magic dimensions or dummy queries.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+44
to
+52
| # Access the raw index client | ||
| index = self.vector_store.get_pinecone_index() | ||
|
|
||
| # We can't easily list all without a query in standard Pinecone. | ||
| # Using a workaround: list all by prefix if possible, or query with zero vector. | ||
| # Here we'll just try to fetch a large sample. | ||
|
|
||
| results = index.query( | ||
| vector=[0] * 1536, # Dummy vector size |
There was a problem hiding this comment.
issue (performance): Avoid hardcoding the Pinecone embedding dimension and large broad queries in list_filenames.
This assumes a 1536‑dimensional embedding and issues a single top_k=10000 query, which ties this code to a specific model and can become slow/expensive on large indexes. It also depends on get_pinecone_index(), which may not be a stable public API. Consider deriving the dimension from index metadata or the embeddings object and iterating with pagination over the raw client (e.g., list or repeated query calls with smaller top_k) instead of one broad dummy‑vector query. If full refactoring isn’t feasible now, at least make the dimension configurable rather than hard‑coded.
Comment on lines
+73
to
+74
| docs = self.vector_store.similarity_search( | ||
| "dummy", k=1, filter=metadata |
There was a problem hiding this comment.
suggestion (performance): Using similarity_search with a dummy query for existence checks is likely inefficient.
This implementation always performs an embedding + similarity query just to check presence, which can add avoidable latency and cost if exists is called often. Prefer using the underlying Pinecone index client for a minimal query with a fixed/cheap vector, or (if available) a metadata-only lookup. As another option, you could make this existence check optional/best-effort for Pinecone-backed stores to avoid unnecessary embedding calls.
Comment on lines
+47
to
+52
| self.vector_store.col.delete(expr=f"filename == '{filename}'") | ||
| return True | ||
|
|
||
| def exists(self, metadata: dict[str, str]) -> bool: | ||
| """Check if documents with given metadata exist.""" | ||
| expr = " and ".join([f"{k} == '{v}'" for k, v in metadata.items()]) |
There was a problem hiding this comment.
🚨 issue (security): String-interpolated expressions for Milvus queries risk quoting bugs and injection issues.
delete_filename and exists build Milvus expr strings directly from user data via f-strings (e.g., filename == '{filename}', " and ".join([f"{k} == '{v}'" ...])). If any value contains quotes or special characters, the query can break or be exploitable. These values should be safely escaped or validated, ideally via a Milvus query builder or parameterized API rather than manual string concatenation.
| # Here we'll just try to fetch a large sample. | ||
|
|
||
| results = index.query( | ||
| vector=[0] * 1536, # Dummy vector size |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the Pinecone adapter to centralize the dummy vector logic, infer the embedding dimension dynamically, and use direct Pinecone queries for metadata checks instead of similarity_search.
You can reduce the “surprising” complexity without dropping functionality by:
- Removing the hard‑coded embedding dimension.
- Avoiding similarity_search for non‑similarity operations.
- Centralizing the “dummy query” behavior in a small internal helper so callers see a clear, honest API.
1. Avoid hard‑coded 1536 and expose the approximation
Derive the dimension once from the configured embeddings instead of baking in 1536, and make the “broad scan” behavior explicit:
class PineconeAdapter(VectorStoreAdapter):
def __init__(self, vector_store: "PineconeVectorStore"):
self.vector_store = vector_store
# Infer dimension from the configured embeddings
sample_vec = self.vector_store._embedding.embed_query("") # or similar API
self._dim = len(sample_vec)
def _dummy_vector(self) -> list[float]:
# Single place where we define the "broad search" dummy vector
return [0.0] * self._dim
def list_filenames(self) -> list[str]:
"""Best-effort listing of unique filenames using a broad query.
Note: This uses a dummy vector to approximate "scan all" and may be slow
for large indexes.
"""
index = self.vector_store.get_pinecone_index()
results = index.query(
vector=self._dummy_vector(),
top_k=10_000,
include_metadata=True,
)
filenames: set[str] = set()
for match in results.get("matches", []):
metadata = match.get("metadata") or {}
filename = metadata.get("filename")
if filename:
filenames.add(filename)
return sorted(filenames)This removes the magic 1536, keeps the behavior, and documents that it is an approximation with potential performance cost.
2. Use Pinecone query directly for exists, not similarity_search
You can keep semantics but avoid the “dummy text” similarity_search and make the metadata‑filter behavior obvious:
class PineconeAdapter(VectorStoreAdapter):
...
def exists(self, metadata: dict[str, str]) -> bool:
"""Check if documents with given metadata exist.
This relies on a minimal Pinecone query with a dummy vector, using only
the metadata filter for existence.
"""
index = self.vector_store.get_pinecone_index()
res = index.query(
vector=self._dummy_vector(),
top_k=1,
filter=metadata,
include_metadata=False,
)
return bool(res.get("matches"))This:
- Decouples existence from a semantic “similarity search” API.
- Keeps the same functional behavior (metadata‑filtered query with
top_k=1). - Moves all the “hackiness” into
_dummy_vector+ clear docstrings, so callers aren’t surprised by magic dimensions or dummy queries.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+45
to
+48
| def delete_filename(self, filename: str) -> bool: | ||
| """Delete all documents associated with a filename.""" | ||
| self.vector_store.col.delete(expr=f"filename == '{filename}'") | ||
| return True |
There was a problem hiding this comment.
1. delete_filename() always returns true 📎 Requirement gap ≡ Correctness
Several new vector store adapters return True from delete_filename() unconditionally, while others return True only when at least one document was actually deleted. This makes deletion behavior inconsistent across providers and can mislead callers relying on the boolean result.
Agent Prompt
## Issue description
`VectorStoreAdapter.delete_filename()` is expected to provide consistent semantics across providers, but multiple adapters always return `True` even when nothing may have been deleted. This breaks uniform behavior and can cause incorrect UX/logic (e.g., reporting a delete succeeded when no documents matched).
## Issue Context
`MarkdownRAG.delete_document()` directly returns `self.vector_store.delete_filename(filename)`, so this inconsistency propagates to user-visible behavior.
## Fix Focus Areas
- src/markdown_rag/vector_stores/milvus.py[45-48]
- src/markdown_rag/vector_stores/qdrant.py[60-75]
- src/markdown_rag/vector_stores/mongodb.py[42-46]
- src/markdown_rag/vector_stores/pinecone.py[65-68]
## Notes
Update each adapter to return `True` only if at least one record/vector was deleted (or otherwise document a consistent contract, e.g., `True` means the delete operation executed successfully and include a separate count).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+51
to
+55
| results = index.query( | ||
| vector=[0] * 1536, # Dummy vector size | ||
| top_k=10000, | ||
| include_metadata=True | ||
| ) |
There was a problem hiding this comment.
2. pineconeadapter hardcodes vector size 📎 Requirement gap ☼ Reliability
PineconeAdapter.list_filenames() queries Pinecone using a hardcoded 1536-dimension dummy vector, which can fail for indexes built with different embedding dimensions and diverges from other providers’ dimension-agnostic listing behavior. It also uses top_k=10000, which may be expensive on large indexes, undermining a consistent provider-agnostic interface.
Agent Prompt
## Issue description
`PineconeAdapter.list_filenames()` currently issues a Pinecone vector query using a dummy vector with a hardcoded length of `1536`, which can raise runtime errors when the Pinecone index dimension differs due to configurable embedding models. The method also requests `top_k=10000`, which can be unnecessarily expensive for large indexes and further undermines consistent, provider-agnostic behavior.
## Issue Context
- Compliance requires consistent operation semantics across vector store providers.
- Other adapters can list filenames without assuming embedding dimensionality, but the Pinecone implementation depends on a vector query and thus implicitly assumes an embedding dimension.
- Embedding model selection is configurable (e.g., `GOOGLE_MODEL` / `OLLAMA_MODEL`), and there is no code ensuring those embeddings (or the Pinecone index) are 1536-dimensional.
## Fix Focus Areas
- src/markdown_rag/vector_stores/pinecone.py[35-63]
- src/markdown_rag/config.py[132-170]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+37
to
+45
| case VectorStoreType.PGVECTOR: | ||
| try: | ||
| from langchain_postgres import PGVector | ||
| except ImportError: | ||
| logger.error( | ||
| "Postgres support is not installed. " | ||
| "Please install with 'uv add markdown-rag --optional postgres'" | ||
| ) | ||
| sys.exit(1) |
There was a problem hiding this comment.
3. Eager pgvector import 🐞 Bug ☼ Reliability
src/markdown_rag/main.py still imports langchain_postgres.PGVector at module import time, so installs without the postgres extra will crash before the VECTOR_STORE match/case can lazy-load backends. This defeats the PR’s optional-deps design and prevents using non-Postgres stores without also installing Postgres deps.
Agent Prompt
### Issue description
`src/markdown_rag/main.py` imports `langchain_postgres.PGVector` at module import time, but `langchain-postgres` was moved to an optional extra. This causes an immediate `ImportError` for users who install without the `postgres` extra (even if they configure `VECTOR_STORE` to `qdrant/chroma/milvus/pinecone/mongodb`).
### Issue Context
The code already has a `try/except ImportError` lazy import inside the `VectorStoreType.PGVECTOR` match/case branch, but the unconditional import at the top triggers first.
### Fix Focus Areas
- src/markdown_rag/main.py[1-10]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+28
to
34
| VECTOR_STORE: VectorStoreType = Field(default=VectorStoreType.PGVECTOR) | ||
|
|
||
| POSTGRES_USER: str = Field(default="postgres") | ||
| POSTGRES_PASSWORD: SecretStr = Field(default=...) | ||
| POSTGRES_HOST: str = Field(default="localhost") | ||
| POSTGRES_PORT: str = Field(default="5432") | ||
| POSTGRES_DB: str | None = Field(default=None) |
There was a problem hiding this comment.
4. Postgres password always required 🐞 Bug ≡ Correctness
Env.POSTGRES_PASSWORD is required in the base settings class, so configuration validation fails even when VECTOR_STORE is not pgvector. This blocks startup for Qdrant/Chroma/Milvus/Pinecone/MongoDB unless users still provide POSTGRES_PASSWORD.
Agent Prompt
### Issue description
`POSTGRES_PASSWORD` is declared as required on the base `Env` settings class, so Pydantic validates it on every startup regardless of `VECTOR_STORE`. This makes non-Postgres backends unusable unless Postgres credentials are still provided.
### Issue Context
`main()` instantiates the `Env` subclass before `start_store()` selects the vector store backend.
### Fix Focus Areas
- src/markdown_rag/config.py[23-35]
- src/markdown_rag/main.py[200-206]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+45
to
+58
| def delete_filename(self, filename: str) -> bool: | ||
| """Delete all documents associated with a filename.""" | ||
| self.vector_store.col.delete(expr=f"filename == '{filename}'") | ||
| return True | ||
|
|
||
| def exists(self, metadata: dict[str, str]) -> bool: | ||
| """Check if documents with given metadata exist.""" | ||
| expr = " and ".join([f"{k} == '{v}'" for k, v in metadata.items()]) | ||
| res = self.vector_store.col.query( | ||
| expr=expr, | ||
| limit=1, | ||
| output_fields=["pk"] | ||
| ) | ||
| return len(res) > 0 |
There was a problem hiding this comment.
5. Milvus expr string injection 🐞 Bug ⛨ Security
MilvusAdapter.delete_filename and exists build Milvus expr strings via direct interpolation, so filenames/metadata containing quotes can break the expression or alter its meaning. This can cause failed deletes/dedup checks or unintended query behavior.
Agent Prompt
### Issue description
`MilvusAdapter` constructs Milvus filter expressions using raw string interpolation (e.g., `filename == '{filename}'`). If `filename` (derived from filesystem-relative paths) contains `'` or other special characters, the expression becomes invalid or can be manipulated.
### Issue Context
`MarkdownRAG` uses filesystem-derived relative paths as the `filename` metadata key and passes it into `exists()` and `delete_filename()`.
### Fix Focus Areas
- src/markdown_rag/vector_stores/milvus.py[45-58]
- src/markdown_rag/rag.py[71-77]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a pluggable vector store architecture, enabling support for multiple backends such as Qdrant, Chroma, Milvus, Pinecone, and MongoDB alongside the existing PostgreSQL implementation. The core RAG logic has been refactored to use a VectorStoreAdapter protocol, and dependencies are now managed via optional extras. Review feedback identifies critical security vulnerabilities regarding string injection in the Milvus adapter, logic errors in the return values for document deletion in Milvus and MongoDB, and a hardcoded vector dimension in the Pinecone adapter that should be dynamically retrieved.
Comment on lines
+47
to
+48
| self.vector_store.col.delete(expr=f"filename == '{filename}'") | ||
| return True |
There was a problem hiding this comment.
The deletion expression is vulnerable to injection if the filename contains single quotes. Additionally, the protocol expects a boolean indicating if any documents were deleted. Milvus delete returns a MutationResult which contains a delete_count.
Suggested change
| self.vector_store.col.delete(expr=f"filename == '{filename}'") | |
| return True | |
| safe_filename = filename.replace("'", "''") | |
| res = self.vector_store.col.delete(expr=f"filename == '{safe_filename}'") | |
| return res.delete_count > 0 |
References
- Code changes must pass MyPy type checking, even if a suggestion is functionally correct at runtime.
|
|
||
| def exists(self, metadata: dict[str, str]) -> bool: | ||
| """Check if documents with given metadata exist.""" | ||
| expr = " and ".join([f"{k} == '{v}'" for k, v in metadata.items()]) |
There was a problem hiding this comment.
The query expression is vulnerable to injection if metadata values contain single quotes. Values should be sanitized by escaping single quotes.
Suggested change
| expr = " and ".join([f"{k} == '{v}'" for k, v in metadata.items()]) | |
| expr = " and ".join([f"{k} == '{str(v).replace("'", "''")}'" for k, v in metadata.items()]) |
References
- Code changes must pass MyPy type checking, even if a suggestion is functionally correct at runtime.
Comment on lines
+52
to
+53
| vector=[0] * 1536, # Dummy vector size | ||
| top_k=10000, |
There was a problem hiding this comment.
The vector dimension is hardcoded to 1536, which will cause a runtime error if the index was created with a different dimension. You should retrieve the dimension from the index statistics. Using getattr ensures MyPy compatibility as the response object from Pinecone is not a dictionary.
stats = index.describe_index_stats()
dimension = getattr(stats, "dimension", 1536)
results = index.query(
vector=[0.0] * dimension,References
- Code changes must pass MyPy type checking, even if a suggestion is functionally correct at runtime.
Comment on lines
+45
to
+46
| collection.delete_many({"metadata.filename": filename}) | ||
| return True |
There was a problem hiding this comment.
The method should return a boolean indicating whether any documents were actually deleted, rather than always returning True.
Suggested change
| collection.delete_many({"metadata.filename": filename}) | |
| return True | |
| result = collection.delete_many({"metadata.filename": filename}) | |
| return result.deleted_count > 0 |
References
- Code changes must pass MyPy type checking, even if a suggestion is functionally correct at runtime.
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.
This PR implements support for multiple vector store backends, decoupling the core RAG logic from specific database implementations.
Key Changes:
Configuration:
Users can now set the VECTOR_STORE environment variable to any of the supported values and provide the corresponding connection settings.
Resolves #5
Summary by Sourcery
Add a pluggable vector store layer and update configuration and docs to support multiple backend providers.
New Features:
Enhancements:
Build:
Documentation: