-
Notifications
You must be signed in to change notification settings - Fork 0
Fix/rag pipeline #57
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
Fix/rag pipeline #57
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,33 +1,55 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import logging | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from langchain_core.documents import Document | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from app.core.config import settings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from app.core.database_connection import SessionLocal | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from app.models.document import Document | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from app.services.chunking_service import document_to_chunks | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from app.services.embedding_service import chunks_to_embeddings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from app.services.pdf_processor import pdf_to_document | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from app.services.vector_store import store_chunks_with_embeddings | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger = logging.getLogger(__name__) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _create_document_record(filename: str, minio_path: str) -> int: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Create a Document record in the database. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Args: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| filename: Original filename of the PDF | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| minio_path: Path to the PDF in MinIO bucket | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| int: The created document's ID | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+15
to
+22
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Create a Document record in the database. | |
| Args: | |
| filename: Original filename of the PDF | |
| minio_path: Path to the PDF in MinIO bucket | |
| Returns: | |
| int: The created document's ID | |
| Create a Document record in the database within a transaction and return its ID. | |
| This helper opens a new database session, inserts a row into the ``documents`` table, | |
| commits the transaction, and then returns the newly created document's primary key. | |
| Args: | |
| filename: Original filename of the PDF. | |
| minio_path: Path to the PDF in the MinIO bucket. | |
| Returns: | |
| int: The created document's ID. | |
| Raises: | |
| DatabaseError: If there is a problem connecting to or communicating with the database. | |
| IntegrityError: If inserting the document violates a database constraint | |
| (for example, a uniqueness or foreign-key constraint). |
Copilot
AI
Dec 17, 2025
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 database session is not properly handled in case of exceptions during commit or refresh. If db.commit() or db.refresh(document) raises an exception, the session should be rolled back before closing to avoid leaving the transaction in an inconsistent state. Consider wrapping the database operations in a try-except block that calls db.rollback() on exception before re-raising.
| return document.id | |
| return document.id | |
| except Exception: | |
| db.rollback() | |
| raise |
Copilot
AI
Dec 17, 2025
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.
If _create_document_record fails after the document is created but before returning, or if the subsequent store_chunks_with_embeddings call fails, the database will contain an orphaned document record with no associated chunks. This could lead to data inconsistency. Consider implementing a transaction boundary that spans both operations, or adding cleanup logic to remove the document record if chunk storage fails.
| chunks_stored = store_chunks_with_embeddings( | |
| document_id=document_id, | |
| filename=filename, | |
| chunks=chunks, | |
| ) | |
| logger.info(f"Stage 3 completed successfully. Stored {chunks_stored} chunks with embeddings") | |
| try: | |
| chunks_stored = store_chunks_with_embeddings( | |
| document_id=document_id, | |
| filename=filename, | |
| chunks=chunks, | |
| ) | |
| logger.info(f"Stage 3 completed successfully. Stored {chunks_stored} chunks with embeddings") | |
| except Exception as storage_error: | |
| logger.error( | |
| f"Failed to store chunks with embeddings for document_id={document_id}. " | |
| "Attempting to roll back created document record.", | |
| exc_info=True, | |
| ) | |
| cleanup_db = SessionLocal() | |
| try: | |
| document_record = cleanup_db.get(Document, document_id) | |
| if document_record is not None: | |
| cleanup_db.delete(document_record) | |
| cleanup_db.commit() | |
| logger.info(f"Rolled back document record with id={document_id} after chunk storage failure") | |
| except Exception: | |
| logger.error( | |
| f"Failed to roll back document record with id={document_id} after chunk storage failure", | |
| exc_info=True, | |
| ) | |
| finally: | |
| cleanup_db.close() | |
| # Re-raise the original error to be handled by the outer exception handler | |
| raise storage_error |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,162 @@ | ||||||||||||
| """ | ||||||||||||
| Vector Store Service - Handles embedding generation and storage using LangChain PGVector. | ||||||||||||
|
|
||||||||||||
| This service provides functionality to: | ||||||||||||
| 1. Initialize PGVector connection with OpenAI embeddings | ||||||||||||
| 2. Store document chunks with their embeddings in batches | ||||||||||||
| 3. Convert database URLs to psycopg3 format required by langchain-postgres | ||||||||||||
| """ | ||||||||||||
|
|
||||||||||||
| import logging | ||||||||||||
| from urllib.parse import urlparse, urlunparse | ||||||||||||
|
|
||||||||||||
| from langchain_core.documents import Document | ||||||||||||
| from langchain_openai import OpenAIEmbeddings | ||||||||||||
| from langchain_postgres import PGVector | ||||||||||||
|
|
||||||||||||
| from app.core.config import settings | ||||||||||||
|
|
||||||||||||
| logger = logging.getLogger(__name__) | ||||||||||||
|
|
||||||||||||
| # Collection name for the vector store | ||||||||||||
| COLLECTION_NAME = "document_chunks" | ||||||||||||
|
|
||||||||||||
| # Batch size for inserting documents (to handle large PDFs efficiently) | ||||||||||||
| DEFAULT_BATCH_SIZE = 100 | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def _convert_database_url_to_psycopg(database_url: str) -> str: | ||||||||||||
| """ | ||||||||||||
| Convert database URL to postgresql+psycopg format required by langchain-postgres. | ||||||||||||
|
|
||||||||||||
| LangChain PGVector requires postgresql+psycopg:// (psycopg3) format. | ||||||||||||
| This function converts common formats (postgresql://, postgresql+psycopg2://) to the required format. | ||||||||||||
|
|
||||||||||||
| Args: | ||||||||||||
| database_url: Original database URL | ||||||||||||
|
|
||||||||||||
| Returns: | ||||||||||||
| Database URL in postgresql+psycopg:// format | ||||||||||||
| """ | ||||||||||||
| parsed = urlparse(database_url) | ||||||||||||
|
|
||||||||||||
| # Replace driver with psycopg (psycopg3) | ||||||||||||
| if parsed.scheme.startswith("postgresql"): | ||||||||||||
| # Remove any existing driver (e.g., +psycopg2) | ||||||||||||
| base_scheme = "postgresql" | ||||||||||||
| if "+" in parsed.scheme: | ||||||||||||
| base_scheme = parsed.scheme.split("+")[0] | ||||||||||||
|
|
||||||||||||
| new_scheme = f"{base_scheme}+psycopg" | ||||||||||||
| new_parsed = parsed._replace(scheme=new_scheme) | ||||||||||||
| return urlunparse(new_parsed) | ||||||||||||
|
|
||||||||||||
| return database_url | ||||||||||||
|
Comment on lines
+28
to
+54
|
||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def _get_embeddings() -> OpenAIEmbeddings: | ||||||||||||
| """ | ||||||||||||
| Get OpenAI embeddings instance configured from settings. | ||||||||||||
|
|
||||||||||||
| Returns: | ||||||||||||
| OpenAIEmbeddings instance | ||||||||||||
| """ | ||||||||||||
| return OpenAIEmbeddings( | ||||||||||||
| model=settings.embedding_model, | ||||||||||||
| openai_api_key=settings.openai_api_key, | ||||||||||||
| ) | ||||||||||||
|
Comment on lines
+57
to
+67
|
||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def _get_vector_store() -> PGVector: | ||||||||||||
| """ | ||||||||||||
| Get or create PGVector instance for document storage. | ||||||||||||
|
|
||||||||||||
| Returns: | ||||||||||||
| PGVector instance configured with embeddings and connection | ||||||||||||
| """ | ||||||||||||
| connection_string = _convert_database_url_to_psycopg(settings.database_url) | ||||||||||||
| embeddings = _get_embeddings() | ||||||||||||
|
|
||||||||||||
| vector_store = PGVector( | ||||||||||||
| embeddings=embeddings, | ||||||||||||
| collection_name=COLLECTION_NAME, | ||||||||||||
| connection=connection_string, | ||||||||||||
| use_jsonb=True, | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| return vector_store | ||||||||||||
|
Comment on lines
+70
to
+87
|
||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def store_chunks_with_embeddings( | ||||||||||||
| document_id: int, | ||||||||||||
| filename: str, | ||||||||||||
| chunks: list[Document], | ||||||||||||
| batch_size: int = DEFAULT_BATCH_SIZE, | ||||||||||||
| ) -> int: | ||||||||||||
| """ | ||||||||||||
| Store document chunks with their embeddings in PGVector. | ||||||||||||
|
|
||||||||||||
| This function: | ||||||||||||
| 1. Prepares chunks with metadata (document_id, chunk_index, filename) | ||||||||||||
| 2. Uses LangChain PGVector to generate embeddings and store in batches | ||||||||||||
| 3. Returns the number of chunks stored | ||||||||||||
|
|
||||||||||||
| Args: | ||||||||||||
| document_id: ID of the parent document in the documents table | ||||||||||||
| filename: Original filename for metadata | ||||||||||||
| chunks: List of LangChain Document chunks to embed and store | ||||||||||||
| batch_size: Number of chunks to process per batch (default: 100) | ||||||||||||
|
|
||||||||||||
| Returns: | ||||||||||||
| int: Number of chunks successfully stored | ||||||||||||
|
|
||||||||||||
| Raises: | ||||||||||||
| Exception: If storage fails | ||||||||||||
| """ | ||||||||||||
| if not chunks: | ||||||||||||
| logger.warning("No chunks provided for storage") | ||||||||||||
| return 0 | ||||||||||||
|
|
||||||||||||
| logger.info(f"Storing {len(chunks)} chunks for document_id={document_id}") | ||||||||||||
|
|
||||||||||||
| # Prepare documents with metadata for PGVector | ||||||||||||
| prepared_docs = [] | ||||||||||||
| for idx, chunk in enumerate(chunks): | ||||||||||||
| # Create a new document with enriched metadata | ||||||||||||
| metadata = { | ||||||||||||
| "document_id": document_id, | ||||||||||||
| "chunk_index": idx, | ||||||||||||
| "filename": filename, | ||||||||||||
| # Preserve any existing metadata from chunking | ||||||||||||
| **chunk.metadata, | ||||||||||||
| } | ||||||||||||
| prepared_docs.append( | ||||||||||||
| Document( | ||||||||||||
| page_content=chunk.page_content, | ||||||||||||
| metadata=metadata, | ||||||||||||
| ) | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| # Get vector store instance | ||||||||||||
| vector_store = _get_vector_store() | ||||||||||||
|
|
||||||||||||
| # Store documents in batches | ||||||||||||
| total_stored = 0 | ||||||||||||
| for i in range(0, len(prepared_docs), batch_size): | ||||||||||||
| batch = prepared_docs[i : i + batch_size] | ||||||||||||
| batch_num = (i // batch_size) + 1 | ||||||||||||
| total_batches = (len(prepared_docs) + batch_size - 1) // batch_size | ||||||||||||
|
|
||||||||||||
| logger.info(f"Processing batch {batch_num}/{total_batches} ({len(batch)} chunks)") | ||||||||||||
|
|
||||||||||||
| try: | ||||||||||||
| # PGVector.add_documents handles embedding generation internally | ||||||||||||
| vector_store.add_documents(batch) | ||||||||||||
| total_stored += len(batch) | ||||||||||||
| logger.debug(f"Batch {batch_num} stored successfully") | ||||||||||||
| except Exception as e: | ||||||||||||
| logger.error(f"Error storing batch {batch_num}: {e}") | ||||||||||||
|
||||||||||||
| logger.error(f"Error storing batch {batch_num}: {e}") | |
| logger.error( | |
| f"Error storing batch {batch_num}: {e}. " | |
| f"Successfully stored {total_stored} chunks before failure" | |
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,11 +10,12 @@ dependencies = [ | |
| "langgraph>=1.0.4", | ||
| "langchain-core>=0.3.0", | ||
| "langchain-text-splitters>=0.3.0", | ||
| "langchain-postgres>=0.0.5", | ||
| "langchain-postgres>=0.0.14", | ||
| "langchain-openai>=0.3.0", | ||
| "typing-extensions>=4.15.0", | ||
| "uvicorn>=0.38.0", | ||
| "sqlalchemy>=2.0.0", | ||
| "psycopg2-binary>=2.9.0", | ||
| "psycopg[binary]>=3.0.0", | ||
|
||
| "pgvector>=0.3.0", | ||
| "pydantic-settings>=2.0.0", | ||
| "guardrails-ai>=0.5.10", | ||
|
|
||
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 comment states to "Use psycopg2 (sync driver), NOT asyncpg" but the actual DATABASE_URL now uses
postgresql+psycopg2://which contradicts the dependency change in pyproject.toml wherepsycopg2-binarywas replaced withpsycopg[binary]>=3.0.0(psycopg3). This creates confusion about which driver version should actually be used. The comment should be updated to reflect that psycopg3 is now being used, or the DATABASE_URL should use a different format if psycopg2 is still intended.