Add API server setup, authentication, and RAG pipeline integration#21
Add API server setup, authentication, and RAG pipeline integration#21
Conversation
- Updated README.md to include instructions for starting the API server and setting up authentication. - Introduced debug_import.py for testing import paths and module availability. - Added SQLModel and aiosqlite dependencies to pyproject.toml. - Implemented database initialization and RAG pipeline setup in app.py. - Created authentication logic in auth.py, including API key verification. - Configured API settings in config.py to include an optional API key. - Developed database connection and session management in database.py. - Defined database models for analysis and findings in db_models.py. - Established API routes for analysis and RAG functionalities in routes/analysis.py and routes/rag.py. - Implemented in-memory and PostgreSQL vector store interfaces for RAG in store/base.py, store/memory.py, and store/postgres.py. - Added tests for API functionality and authentication in test_api.py and test_auth.py. - Updated pytest.ini to include necessary python paths for testing. - Modified requirements.txt to include SQLModel and aiosqlite dependencies.
There was a problem hiding this comment.
Pull request overview
This pull request introduces a comprehensive API server setup with authentication, database persistence, and RAG (Retrieval-Augmented Generation) pipeline integration. The changes establish a REST API for code analysis with API key authentication, SQLite database storage, and support for both in-memory and PostgreSQL vector stores.
Key changes:
- Added FastAPI server with API key authentication and database-backed analysis tracking
- Implemented vector store abstraction with in-memory and PostgreSQL implementations for RAG
- Created database models and session management using SQLModel and SQLAlchemy async
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 21 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_auth.py | New test suite for API authentication with various scenarios (disabled, missing key, invalid key, valid key) |
| packages/api/tests/test_api.py | New tests for API endpoints including analysis creation, listing, and RAG search |
| requirements.txt | Added SQLModel and aiosqlite dependencies for database support |
| pytest.ini | Added python paths for API and RAG packages to pytest configuration |
| packages/rag/src/omniscient_rag/store/postgres.py | Extended PostgresVectorStore to inherit from VectorStore base class and implement document ingestion and hybrid search |
| packages/rag/src/omniscient_rag/store/memory.py | New in-memory vector store implementation for testing and development |
| packages/rag/src/omniscient_rag/store/base.py | New abstract base class defining vector store interface |
| packages/rag/src/omniscient_rag/store/init.py | Updated exports to include VectorStore base class and InMemoryVectorStore |
| packages/rag/src/omniscient_rag/pipeline.py | Updated to support both in-memory and PostgreSQL stores via factory pattern |
| packages/api/src/omniscient_api/routes/rag.py | New RAG endpoints for file ingestion and knowledge base search |
| packages/api/src/omniscient_api/routes/analysis.py | Refactored to use database persistence instead of in-memory storage |
| packages/api/src/omniscient_api/routes/init.py | New routes package initialization |
| packages/api/src/omniscient_api/db_models.py | New database models for Analysis and FindingModel using SQLModel |
| packages/api/src/omniscient_api/database.py | Database connection and session management with async SQLAlchemy |
| packages/api/src/omniscient_api/config.py | Added API key configuration field and config caching function |
| packages/api/src/omniscient_api/auth.py | New authentication logic with API key verification dependency |
| packages/api/src/omniscient_api/app.py | Updated app initialization to include database and RAG pipeline setup |
| packages/api/pyproject.toml | Added SQLModel and aiosqlite to project dependencies |
| debug_import.py | New debugging script for testing import paths and module availability |
| README.md | Added documentation for API server setup and authentication usage |
Comments suppressed due to low confidence (9)
packages/api/src/omniscient_api/routes/analysis.py:153
- Bare except clause suppresses all exceptions silently. This error handling catches all exceptions without logging them or handling them in any meaningful way, making it impossible to diagnose issues when analysis status/error updates fail.
packages/api/src/omniscient_api/routes/analysis.py:154 - Database session handling in background task uses incorrect pattern. The async generator is iterated with 'async for' but then breaks after one iteration, which is unusual. The background task should properly manage the session lifecycle, either by creating a new session directly or using a context manager.
packages/api/src/omniscient_api/routes/analysis.py:89 - Incomplete TODO comment. The comment states 'Implement full analysis pipeline integration' but doesn't provide any guidance on what needs to be implemented or what the expected integration points are. Consider expanding this TODO to include specific tasks or linking to an issue tracker.
packages/api/src/omniscient_api/routes/analysis.py:92 - Incorrect import path for database module. The import uses a relative path from the current module (routes.analysis), but database.py is in the parent package. This should be
from ..database import get_sessioninstead offrom .database import get_session.
packages/api/src/omniscient_api/routes/analysis.py:181 - Type mismatch between database model and response model. The function returns an Analysis database model directly, but the response_model is AnalysisResponse which expects different types for summary (AnalysisSummary) and metrics (AnalysisMetrics) fields. The database model stores these as plain dicts (JSON), which will cause serialization issues. The database model should be converted to the response model before returning.
packages/api/src/omniscient_api/routes/analysis.py:211 - Type mismatch between database model and response model. The function returns a list of Analysis database models directly, but the response_model expects List[AnalysisResponse]. The database model's summary and metrics fields are plain dicts (JSON) while AnalysisResponse expects AnalysisSummary and AnalysisMetrics objects. This will cause serialization issues. Each database model should be converted to the response model before returning.
packages/api/src/omniscient_api/routes/analysis.py:24 - Import of 'Finding' is not used.
Import of 'AnalysisSummary' is not used.
Import of 'AnalysisMetrics' is not used.
packages/api/src/omniscient_api/routes/analysis.py:152 - Except block directly handles BaseException.
packages/api/src/omniscient_api/routes/analysis.py:152 - 'except' clause does nothing but pass and there is no explanatory comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| DATABASE_URL = "sqlite+aiosqlite:///./omniscient.db" | ||
|
|
||
| engine = create_async_engine(DATABASE_URL, echo=True, future=True) |
There was a problem hiding this comment.
Database echo enabled in production code. The engine is created with echo=True which logs all SQL statements. This is useful for debugging but should not be enabled in production as it can expose sensitive data and reduce performance. Consider making this configurable or disabling it by default.
| engine = create_async_engine(DATABASE_URL, echo=True, future=True) | |
| engine = create_async_engine(DATABASE_URL, future=True) |
|
|
||
| DATABASE_URL = "sqlite+aiosqlite:///./omniscient.db" |
There was a problem hiding this comment.
Hardcoded database URL in production code. The DATABASE_URL is hardcoded to a SQLite file path without any configuration option. This should be configurable through environment variables or configuration to support different databases in different environments (development, testing, production).
| DATABASE_URL = "sqlite+aiosqlite:///./omniscient.db" | |
| import os | |
| DATABASE_URL = os.getenv("DATABASE_URL", "sqlite+aiosqlite:///./omniscient.db") |
|
|
||
| async def init_db(): | ||
| async with engine.begin() as conn: | ||
| # await conn.run_sync(SQLModel.metadata.drop_all) |
There was a problem hiding this comment.
Commented-out code should be removed. The line await conn.run_sync(SQLModel.metadata.drop_all) is commented out but left in the codebase. If this code is not needed, it should be removed entirely rather than left as a comment. If it's needed for development/testing purposes, consider making it a parameter or configuration option.
| # await conn.run_sync(SQLModel.metadata.drop_all) |
| except Exception as e: | ||
| # Log error but continue | ||
| pass |
There was a problem hiding this comment.
Silent error handling suppresses all exceptions without logging. When file ingestion fails, the exception is caught but not logged or reported. This makes debugging difficult and provides no feedback about which files failed to ingest or why.
| env_file=".env", | ||
| extra="ignore", | ||
| env_nested_delimiter="__", | ||
| case_sensitive=False |
There was a problem hiding this comment.
Missing settings for removed configuration options. The SettingsConfigDict removed 'env_file' and 'extra' settings but these may be needed for backward compatibility and to prevent errors when unknown environment variables are present.
| case_sensitive=False | |
| case_sensitive=False, | |
| env_file=None, | |
| extra="ignore", |
| @@ -0,0 +1,67 @@ | |||
| import math | |||
| from typing import List, Dict, Any, Optional | |||
There was a problem hiding this comment.
Import of 'Optional' is not used.
Import of 'Any' is not used.
| from typing import List, Dict, Any, Optional | |
| from typing import List, Dict |
| import math | ||
| from typing import List, Dict, Any, Optional | ||
| from uuid import UUID | ||
| from datetime import datetime |
There was a problem hiding this comment.
Import of 'datetime' is not used.
| from datetime import datetime |
| @@ -0,0 +1,63 @@ | |||
| from typing import List, Optional | |||
There was a problem hiding this comment.
Import of 'Optional' is not used.
| from typing import List, Optional | |
| from typing import List |
| @@ -0,0 +1,63 @@ | |||
| from typing import List, Optional | |||
| from fastapi import APIRouter, HTTPException, UploadFile, File, Form, Depends, Request | |||
There was a problem hiding this comment.
Import of 'Form' is not used.
| from fastapi import APIRouter, HTTPException, UploadFile, File, Form, Depends, Request | |
| from fastapi import APIRouter, HTTPException, UploadFile, File, Depends, Request |
|
|
||
| from omniscient_api.app import create_app | ||
| from omniscient_api.database import get_session | ||
| from omniscient_api.models import AnalysisRequest |
There was a problem hiding this comment.
Import of 'AnalysisRequest' is not used.
| from omniscient_api.models import AnalysisRequest |
Pull Request
Description
Brief description of the changes.
Type of Change
Related Issues
Fixes #(issue number)
Changes Made
Testing
test_packages.pyChecklist