fix(typing): split adapter vars in /readyz to satisfy mypy

.pre-commit-config.yaml

@@ -1,7 +1,7 @@
 repos:
   # --- Basic hygiene checks ---
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.6.0
+    rev: v6.0.0
     hooks:
       - id: check-merge-conflict
       - id: end-of-file-fixer
@@ -9,7 +9,7 @@ repos:
 
   # --- Ruff: linting and formatting ---
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.7.1
+    rev: v0.14.3
     hooks:
       - id: ruff
         args: [--fix, --exit-non-zero-on-fix]
@@ -17,7 +17,7 @@ repos:
 
   # --- Mypy: type-checking on staged Python files ---
   - repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v1.11.2
+    rev: v1.18.2
     hooks:
       - id: mypy
         args:

app/bootstrap.py

@@ -1,7 +1,17 @@
+"""App bootstrap: load .env and prepare environment paths."""
+
+import os
+from pathlib import Path
+
+# Change current dir to project root (so relative paths like data/ work)
+ROOT_DIR = Path(__file__).resolve().parent.parent
+os.chdir(ROOT_DIR)
+
+# Load .env if available
 try:
     from dotenv import load_dotenv
 
     load_dotenv()
-except Exception:
-    # optional: silently continue if python-dotenv is not installed
-    pass
+    print("✅ bootstrap: .env loaded")
+except Exception as e:
+    print(f"⚠️ bootstrap: could not load .env ({e})")

app/main.py

@@ -1,13 +1,7 @@
 import os
 import time
-from typing import Protocol, runtime_checkable, cast
-
 from fastapi import FastAPI, Request, Response, HTTPException
 from fastapi.responses import PlainTextResponse
-
-from app.routers import nl2sql
-
-# Prometheus
 from prometheus_client import (
     Counter,
     Histogram,
@@ -16,13 +10,14 @@ from prometheus_client import (
     CONTENT_TYPE_LATEST,
 )
 
+try:
+    from dotenv import load_dotenv
 
-@runtime_checkable
-class HasPing(Protocol):
-    """Minimal interface for adapters that support a connectivity check."""
-
-    def ping(self) -> None: ...
+    load_dotenv()
+except Exception:
+    pass
 
+from app.routers import nl2sql
 
 # ---- Optionally restore uploaded DB map ----
 try:
@@ -32,24 +27,22 @@ try:
 except Exception as e:
     print(f"⚠️ DB map not restored: {e}")
 
-app = FastAPI(
+application: FastAPI = FastAPI(
     title="NL2SQL Copilot Prototype",
     version=os.getenv("APP_VERSION", "0.1.0"),
     description="Convert natural language to safe & verified SQL",
 )
 
-app.include_router(nl2sql.router, prefix="/api/v1")
+application.include_router(nl2sql.router, prefix="/api/v1")
 
 # ---- Prometheus metrics ----
-REGISTRY: CollectorRegistry = CollectorRegistry()
-
+REGISTRY = CollectorRegistry()
 REQUEST_COUNT = Counter(
     "http_requests_total",
     "Total HTTP requests",
     ["path", "method", "status_code"],
     registry=REGISTRY,
 )
-
 REQUEST_LATENCY = Histogram(
     "http_request_latency_seconds",
     "Request latency",
@@ -58,20 +51,13 @@ REQUEST_LATENCY = Histogram(
 )
 
 
-@app.middleware("http")
+@application.middleware("http")
 async def metrics_middleware(request: Request, call_next):
     start = time.perf_counter()
     response: Response = await call_next(request)
     elapsed = time.perf_counter() - start
-
-    # Use route path if available, else raw path (typed guard for mypy)
     route = request.scope.get("route")
-    path = (
-        route.path
-        if (route is not None and hasattr(route, "path"))
-        else request.url.path
-    )
-
+    path = route.path if route else request.url.path
     REQUEST_COUNT.labels(
         path=path, method=request.method, status_code=str(response.status_code)
     ).inc()
@@ -79,14 +65,16 @@ async def metrics_middleware(request: Request, call_next):
     return response
 
 
-# --- Liveness (super light) ---
-@app.get("/healthz", response_class=PlainTextResponse, tags=["system"])
+# --- Liveness ---
+@application.get("/healthz", response_class=PlainTextResponse, tags=["system"])
 def healthz() -> str:
     return "ok"
 
 
-# --- Readiness (checks DB/env lightly) ---
-@app.get("/readyz", response_class=PlainTextResponse, tags=["system"])
+# --- Readiness ---
+
+
+@application.get("/readyz", response_class=PlainTextResponse, tags=["system"])
 def readyz() -> str:
     mode = os.getenv("DB_MODE", "sqlite").lower()
     try:
@@ -94,32 +82,34 @@ def readyz() -> str:
             from adapters.db.postgres_adapter import PostgresAdapter
 
             dsn = os.environ["POSTGRES_DSN"]
-            # Call ping inline; avoid cross-branch variable typing
-            cast(HasPing, PostgresAdapter(dsn)).ping()
+            pg = PostgresAdapter(dsn)
+            ping = getattr(pg, "ping", None)
+            if callable(ping):
+                ping()
         else:
             from adapters.db.sqlite_adapter import SQLiteAdapter
 
             db_path = os.getenv("SQLITE_DB_PATH", "data/chinook.db")
-            cast(HasPing, SQLiteAdapter(db_path)).ping()
-
-        # if not os.getenv("PROXY_API_KEY"): pass
+            sq = SQLiteAdapter(db_path)
+            ping = getattr(sq, "ping", None)
+            if callable(ping):
+                ping()
         return "ready"
     except Exception:
         raise HTTPException(status_code=503, detail="not ready")
 
 
-@app.get("/")
+@application.get("/")
 def root():
     return {"status": "ok", "message": "NL2SQL Copilot API is running"}
 
 
-@app.get("/health")
+@application.get("/health")
 def health():
-    # You might want to replace the placeholders with real checks later.
     return {"status": "ok", "db": "connected", "llm": "reachable", "uptime_sec": 123.4}
 
 
-@app.get("/metrics", tags=["system"])
+@application.get("/metrics", tags=["system"])
 def metrics():
     data = generate_latest(REGISTRY)
     return Response(content=data, media_type=CONTENT_TYPE_LATEST)

app/routers/nl2sql.py

@@ -22,46 +22,25 @@ from typing import Union, Optional, Dict, TypedDict, Any, cast
 
 router = APIRouter(prefix="/nl2sql")
 
-# --- Database adapter selection ---
-DB_MODE = os.getenv("DB_MODE", "sqlite").lower()
-
-_db: Union[PostgresAdapter, SQLiteAdapter]
-if DB_MODE == "postgres":
-    dsn = os.environ.get("POSTGRES_DSN")
-    if not dsn:
-        raise RuntimeError(
-            "POSTGRES_DSN environment variable is required in postgres mode"
-        )
-    _db = PostgresAdapter(dsn)
-else:
-    _db = SQLiteAdapter("data/chinook.db")
-
-
-# --- Build a single shared pipeline for all routes ---
-def _make_pipeline() -> Pipeline:
-    llm = OpenAIProvider()
-    return Pipeline(
-        detector=AmbiguityDetector(),
-        planner=Planner(llm=llm),
-        generator=Generator(llm=llm),
-        safety=Safety(),
-        executor=Executor(db=_db),
-        verifier=Verifier(),
-        repair=Repair(llm=llm),
-    )
-
-
-_pipeline: Pipeline = _make_pipeline()
-
-
 # -------------------------------
-# Runtime DB registry (for uploaded SQLite files)
-# Files are stored under /tmp, mapped by a short-lived db_id
+# Config / Defaults
 # -------------------------------
+DB_MODE = os.getenv("DB_MODE", "sqlite").lower()  # "sqlite" or "postgres"
+POSTGRES_DSN = os.getenv("POSTGRES_DSN")
+DEFAULT_SQLITE_PATH: str = os.getenv("DEFAULT_SQLITE_DB", "data/Chinook_Sqlite.sqlite")
+
+# Runtime upload storage
 _DB_UPLOAD_DIR = os.getenv("DB_UPLOAD_DIR", "/tmp/nl2sql_dbs")
 _DB_TTL_SECONDS: int = int(os.getenv("DB_TTL_SECONDS", "7200"))  # default 2 hours
 os.makedirs(_DB_UPLOAD_DIR, exist_ok=True)
 
+# Persisted map
+_DB_MAP_PATH = Path("data/uploads/db_map.json")
+_DB_MAP_PATH.parent.mkdir(parents=True, exist_ok=True)
+
+UPLOAD_DIR = Path("data/uploads")
+UPLOAD_DIR.mkdir(parents=True, exist_ok=True)
+
 
 class DBEntry(TypedDict):
     path: str
@@ -71,27 +50,8 @@ class DBEntry(TypedDict):
 # In-memory map: db_id -> {"path": str, "ts": float}
 _DB_MAP: Dict[str, DBEntry] = {}
 
-# -------------------------------
-# Default DB resolution
-# -------------------------------
-DB_MODE = os.getenv("DB_MODE", "sqlite").lower()  # "sqlite" or "postgres"
-POSTGRES_DSN = os.getenv("POSTGRES_DSN")
-DEFAULT_SQLITE_DB: str = os.getenv("DEFAULT_SQLITE_DB", "data/chinook.db")
-
-# -------------------------------
-# Path to persist db_id → file map
-# -------------------------------
-_DB_MAP_PATH = Path("data/uploads/db_map.json")
-_DB_MAP_PATH.parent.mkdir(parents=True, exist_ok=True)
-
-UPLOAD_DIR = Path("data/uploads")
-UPLOAD_DIR.mkdir(parents=True, exist_ok=True)  # ensure folder exists
-
-DEFAULT_SQLITE_PATH = "data/Chinook_Sqlite.sqlite"
-
 
 def _save_db_map() -> None:
-    """Persist the in-memory DB map to disk as JSON."""
     try:
         with open(_DB_MAP_PATH, "w") as f:
             json.dump(_DB_MAP, f)
@@ -100,13 +60,11 @@ def _save_db_map() -> None:
 
 
 def _load_db_map() -> None:
-    """Load the DB map from disk if it exists (called on startup)."""
     global _DB_MAP
     if _DB_MAP_PATH.exists():
         try:
             with open(_DB_MAP_PATH, "r") as f:
                 data = json.load(f)
-            # Be liberal in what we accept; validate into TypedDict
             if isinstance(data, dict):
                 restored: Dict[str, DBEntry] = {}
                 for k, v in data.items():
@@ -121,7 +79,6 @@ def _load_db_map() -> None:
 
 
 def _cleanup_db_map() -> None:
-    """Remove expired uploaded DB files (best-effort)."""
     now = time.time()
     expired = [k for k, v in _DB_MAP.items() if (now - v["ts"]) > _DB_TTL_SECONDS]
     for k in expired:
@@ -134,15 +91,20 @@ def _cleanup_db_map() -> None:
         _DB_MAP.pop(k, None)
 
 
-def _resolve_sqlite_path(db_id: Optional[str]) -> str:
-    """Resolve a SQLite file path from db_id or fallback to default."""
-    _cleanup_db_map()
-    if db_id and db_id in _DB_MAP:
-        return _DB_MAP[db_id]["path"]
-    return DEFAULT_SQLITE_DB
+# Call once at import (safe & light); heavy things remain lazy.
+_load_db_map()
 
 
-def _select_adapter(db_id: Optional[str]):
+# -------------------------------
+# Adapter selection (lazy)
+# -------------------------------
+def _select_adapter(db_id: Optional[str]) -> Union[PostgresAdapter, SQLiteAdapter]:
+    """
+    Resolve a DB adapter:
+      - postgres: requires POSTGRES_DSN
+      - sqlite with db_id: uploaded file or fallback locations
+      - sqlite default: DEFAULT_SQLITE_PATH must exist
+    """
     mode = os.getenv("DB_MODE", "sqlite").lower()
     if mode == "postgres":
         dsn = os.environ.get("POSTGRES_DSN")
@@ -151,79 +113,72 @@ def _select_adapter(db_id: Optional[str]):
         return PostgresAdapter(dsn)
 
     # sqlite mode
+    _cleanup_db_map()
     if db_id:
-        _cleanup_db_map()
-        db_path: Optional[str] = None
-        # first check runtime map
-        if db_id in _DB_MAP:
-            db_path = _DB_MAP[db_id]["path"]
-        # fallback: check /tmp or uploads
-        if not db_path or not os.path.exists(db_path):
-            fallback_tmp = os.path.join(_DB_UPLOAD_DIR, f"{db_id}.sqlite")
-            fallback_uploads = str(UPLOAD_DIR / f"{db_id}.sqlite")
-            for candidate in (fallback_tmp, fallback_uploads):
-                if os.path.exists(candidate):
-                    db_path = candidate
-                    break
-        if not db_path or not os.path.exists(db_path):
-            raise HTTPException(
-                status_code=400, detail="invalid db_id (file not found)"
-            )
-        return SQLiteAdapter(db_path)
-
-    # fallback to default Chinook
+        # Check runtime map
+        entry = _DB_MAP.get(db_id)
+        candidates = []
+        if entry and os.path.exists(entry["path"]):
+            candidates.append(entry["path"])
+        # Fallback locations based on convention
+        candidates.append(os.path.join(_DB_UPLOAD_DIR, f"{db_id}.sqlite"))
+        candidates.append(str(UPLOAD_DIR / f"{db_id}.sqlite"))
+
+        for p in candidates:
+            if p and os.path.exists(p):
+                return SQLiteAdapter(p)
+
+        raise HTTPException(status_code=400, detail="invalid db_id (file not found)")
+
+    # default sqlite
     if not Path(DEFAULT_SQLITE_PATH).exists():
         raise HTTPException(status_code=500, detail="default DB not found")
     return SQLiteAdapter(DEFAULT_SQLITE_PATH)
 
 
 # -------------------------------
-# LLM providers & shared components (stateless)
+# LLM & Pipeline builders (lazy)
 # -------------------------------
-def get_llm():
+def _get_llm() -> OpenAIProvider:
+    # Create provider on demand, after .env has been loaded in app.main
     return OpenAIProvider()
 
 
-_detector = AmbiguityDetector()
-_planner = Planner(get_llm())
-_generator = Generator(get_llm())
-_safety = Safety()
-_verifier = Verifier()
-_repair = Repair(get_llm())
-
-
 def _build_pipeline(adapter: Union[PostgresAdapter, SQLiteAdapter]) -> Pipeline:
-    """Build a fresh Pipeline with a per-request Executor bound to the chosen adapter."""
+    """
+    Build a fresh Pipeline bound to the given adapter.
+    All stateful/external pieces (LLM, executor) are instantiated here (lazy).
+    """
+    llm = _get_llm()
+    detector = AmbiguityDetector()
+    planner = Planner(llm=llm)
+    generator = Generator(llm=llm)
+    safety = Safety()
     executor = Executor(adapter)
+    verifier = Verifier()
+    repair = Repair(llm=llm)
     return Pipeline(
-        detector=_detector,
-        planner=_planner,
-        generator=_generator,
-        safety=_safety,
+        detector=detector,
+        planner=planner,
+        generator=generator,
+        safety=safety,
         executor=executor,
-        verifier=_verifier,
-        repair=_repair,
+        verifier=verifier,
+        repair=repair,
     )
 
 
 # -------------------------------
-# Helpers
+# Helpers (unchanged)
 # -------------------------------
 def _to_dict(obj: Any) -> Any:
-    """Safely convert dataclass instance → dict, otherwise return as-is.
-
-    Note: dataclasses.is_dataclass returns True for both classes and instances.
-    We must exclude classes; mypy cannot refine this perfectly, so we ignore arg-type.
-    """
     if is_dataclass(obj) and not isinstance(obj, type):
         return asdict(obj)  # type: ignore[arg-type]
     return obj
 
 
 def _round_trace(t: Dict[str, Any]) -> Dict[str, Any]:
-    """Round float fields to keep responses tidy and stable."""
     if t.get("cost_usd") is not None:
-        # Ensure numeric before rounding
         cost = t["cost_usd"]
         if isinstance(cost, (int, float)):
             t["cost_usd"] = round(float(cost), 6)
@@ -236,17 +191,9 @@ def _round_trace(t: Dict[str, Any]) -> Dict[str, Any]:
 
 # -------------------------------
 # Upload endpoint (SQLite only)
-# Path will be /api/nl2sql/upload_db if your root APIRouter is mounted at /api
 # -------------------------------
 @router.post("/upload_db")
 async def upload_db(file: UploadFile = File(...)):
-    """
-    Upload a SQLite database (.db/.sqlite). Returns a short-lived db_id.
-    Notes:
-    - Only SQLite files are allowed here (not for Postgres mode).
-    - Max size ~20MB recommended for demo environments like HF Spaces.
-    - Files are stored under /tmp and cleaned by TTL.
-    """
     if DB_MODE != "sqlite":
         raise HTTPException(
             status_code=400, detail="DB upload is only supported in sqlite mode"
@@ -280,48 +227,42 @@ async def upload_db(file: UploadFile = File(...)):
 
 # -------------------------------
 # Main NL2SQL endpoint
-# Path will be /api/nl2sql if your root APIRouter is mounted at /api
 # -------------------------------
 @router.post("", name="nl2sql_handler")
 def nl2sql_handler(request: NL2SQLRequest):
     db_id = getattr(request, "db_id", None)
 
-    # 1) Pick pipeline (+ optional per-request adapter)
-    pipeline: Pipeline
-    if db_id:
-        adapter = _select_adapter(db_id)  # returns PostgresAdapter | SQLiteAdapter
-        # If _select_adapter could theoretically return None, uncomment the next line:
-        # assert adapter is not None, "adapter must be set when db_id is provided"
-        pipeline = _build_pipeline(adapter)
-        derived_preview_val: str = _derive_schema_preview(adapter)
-    else:
-        pipeline = _pipeline
-        derived_preview_val = ""  # no adapter → no derive
-
-    # 2) Resolve schema_preview
+    # Pick adapter per-request (default or uploaded or postgres)
+    adapter = _select_adapter(db_id)
+
+    # Build pipeline lazily with this adapter
+    pipeline = _build_pipeline(adapter)
+
+    # Derive schema preview only for sqlite with a real path
+    derived_preview_val: str = (
+        _derive_schema_preview(adapter) if isinstance(adapter, SQLiteAdapter) else ""
+    )
+
+    # Resolve schema_preview
     provided_preview_any: Any = getattr(request, "schema_preview", None)
     provided_preview: Optional[str] = cast(Optional[str], provided_preview_any)
     final_preview: str = provided_preview or derived_preview_val
 
-    # 3) Run pipeline
+    # Run pipeline
     try:
         result = pipeline.run(
             user_query=request.query,
             schema_preview=final_preview,
         )
     except Exception as exc:
-        # Hard failure in pipeline itself
         raise HTTPException(status_code=500, detail=f"Pipeline crash: {exc!s}")
 
-    # 4) Type check
     if not isinstance(result, FinalResult):
         raise HTTPException(status_code=500, detail="Pipeline returned unexpected type")
 
-    # 5) Ambiguity → ask for clarification
     if result.ambiguous and result.questions:
         return ClarifyResponse(ambiguous=True, questions=result.questions)
 
-    # 6) Soft errors → bubble up details with 400
     if not result.ok or result.error:
         print("❌ Pipeline failure dump:")
         print("  ok:", result.ok)
@@ -333,7 +274,6 @@ def nl2sql_handler(request: NL2SQLRequest):
             detail="; ".join(result.details or []) or (result.error or "Unknown error"),
         )
 
-    # 7) Success
     traces = [_round_trace(t) for t in (result.traces or [])]
     return NL2SQLResponse(
         ambiguous=False,
@@ -345,13 +285,10 @@ def nl2sql_handler(request: NL2SQLRequest):
 
 def _derive_schema_preview(adapter: Union[PostgresAdapter, SQLiteAdapter]) -> str:
     """
-    Build a strict, exact-cased schema preview for the LLM.
-    Works for SQLite adapters by querying sqlite_master / pragma table_info.
+    Build a strict, exact-cased schema preview for the LLM (SQLite only).
     """
     import sqlite3
-    import os
 
-    # Adapters may expose db_path or path; both are str in our codebase
     db_path: Optional[str] = cast(
         Optional[str], getattr(adapter, "db_path", None)
     ) or cast(Optional[str], getattr(adapter, "path", None))
@@ -367,8 +304,7 @@ def _derive_schema_preview(adapter: Union[PostgresAdapter, SQLiteAdapter]) -> st
         lines = []
         for (tname,) in tables:
             cols = cur.execute(f"PRAGMA table_info('{tname}')").fetchall()
-            # sqlite: pragma columns → (cid, name, type, notnull, dflt_value, pk)
-            colnames = [c[1] for c in cols]
+            colnames = [c[1] for c in cols]  # (cid, name, type, notnull, dflt, pk)
             lines.append(f"{tname}({', '.join(colnames)})")
         conn.close()
         return "\n".join(lines)