Refactor RoadmapService to use async database operations

- Replaced synchronous database calls with asynchronous counterparts in RoadmapService.
- Introduced a new helper method _run_db to handle blocking database operations in a non-blocking manner.
- Removed deprecated scripts for diagnosing schema, fixing missing columns, and running migrations.

Author: alimkhann

commitly-backend/app/api/roadmap.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import math
 from typing import Optional
 
 from fastapi import APIRouter, Depends, HTTPException, Query, Response, status
@@ -56,114 +57,27 @@ async def list_roadmaps(
     service: RoadmapService = Depends(get_roadmap_service),
 ) -> CatalogPage:
     """List catalog with filters, sorting, and pagination."""
-    import logging
-    import math
-
-    logger = logging.getLogger(__name__)
-    try:
-        logger.info(f"list_roadmaps: Starting with page={page}, page_size={page_size}")
-
-        # Call service method - wrap sync database call in executor
-        # SQLAlchemy sessions are not thread-safe, so we must use executor carefully
-        import asyncio
-        import time
-
-        start_time = time.time()
-        logger.info("list_roadmaps: Calling service.list_catalog")
-
-        # Wrap the sync database call in executor to avoid blocking event loop
-        # The service.list_catalog is async but calls sync DB code internally
-        def call_db():
-            try:
-                logger.info("list_roadmaps.executor: Starting database call")
-                # Use the service's internal result_store directly for executor
-                # This avoids issues with async/sync boundaries
-                if hasattr(service, "_result_store"):
-                    return service._result_store.list_catalog(
-                        page=page,
-                        page_size=page_size,
-                        language=language,
-                        tag=tag,
-                        difficulty=difficulty,
-                        min_rating=min_rating,
-                        min_views=min_views,
-                        min_syncs=min_syncs,
-                        sort=sort,
-                    )
-                else:
-                    # For test stubs, fallback to async service method
-                    # This won't work in executor, but should work in tests
-                    raise RuntimeError(
-                        "Service doesn't have _result_store and executor can't "
-                        "call async methods"
-                    )
-            except Exception as e:
-                elapsed = time.time() - start_time
-                logger.error(
-                    f"list_roadmaps.executor: Error after {elapsed:.2f}s - "
-                    f"{type(e).__name__}: {e}",
-                    exc_info=True,
-                )
-                raise
-
-        # Check if we have _result_store (production) or need to use async method (tests)
-        if hasattr(service, "_result_store"):
-            # Production: Use executor for sync DB call
-            loop = asyncio.get_event_loop()
-            try:
-                items, total_count = await asyncio.wait_for(
-                    loop.run_in_executor(None, call_db),
-                    timeout=25.0,  # 25 second timeout (less than Render's 30s)
-                )
-                elapsed = time.time() - start_time
-                logger.info(
-                    f"list_roadmaps: Query completed in {elapsed:.2f}s - "
-                    f"Retrieved {len(items)} items, total={total_count}"
-                )
-            except asyncio.TimeoutError:
-                elapsed = time.time() - start_time
-                logger.error(
-                    f"list_roadmaps: Database query timed out after {elapsed:.2f}s"
-                )
-                raise HTTPException(
-                    status_code=status.HTTP_504_GATEWAY_TIMEOUT,
-                    detail="Database query timed out",
-                )
-        else:
-            # Tests: Use async service method directly
-            logger.info("list_roadmaps: Using async service method (test mode)")
-            items, total_count = await service.list_catalog(
-                page=page,
-                page_size=page_size,
-                language=language,
-                tag=tag,
-                difficulty=difficulty,
-                min_rating=min_rating,
-                min_views=min_views,
-                min_syncs=min_syncs,
-                sort=sort,
-            )
-
-        total_pages = math.ceil(total_count / page_size) if total_count > 0 else 0
-
-        return CatalogPage(
-            items=items,
-            page=page,
-            page_size=page_size,
-            total_count=total_count,
-            total_pages=total_pages,
-        )
-    except HTTPException:
-        raise
-    except Exception as e:
-        logger.error(
-            f"list_roadmaps: Unexpected error - {type(e).__name__}: {e}",
-            exc_info=True,
-        )
-        raise HTTPException(
-            status_code=status.HTTP_500_INTERNAL_SERVER_ERROR,
-            detail=f"Failed to retrieve catalog: {str(e)}",
-        ) from e
+    items, total_count = await service.list_catalog(
+        page=page,
+        page_size=page_size,
+        language=language,
+        tag=tag,
+        difficulty=difficulty,
+        min_rating=min_rating,
+        min_views=min_views,
+        min_syncs=min_syncs,
+        sort=sort,
+    )
+
+    total_pages = math.ceil(total_count / page_size) if total_count > 0 else 0
+
+    return CatalogPage(
+        items=items,
+        page=page,
+        page_size=page_size,
+        total_count=total_count,
+        total_pages=total_pages,
+    )
 
 
 @router.get("/cached/{owner}/{repo}", response_model=RoadmapResponse)
@@ -276,7 +190,9 @@ async def set_repository_rating(
     current_user: ClerkClaims = Depends(require_clerk_auth),
     service: RoadmapService = Depends(get_roadmap_service),
 ) -> RatingResponse:
-    return await service.set_rating(get_user_id(current_user), owner, repo, payload.rating)
+    return await service.set_rating(
+        get_user_id(current_user), owner, repo, payload.rating
+    )
 
 
 @router.get("/{owner}/{repo}/rating", response_model=RatingResponse | None)

commitly-backend/app/core/auth.py

@@ -242,7 +242,9 @@ async def verify_clerk_token_async(token: str) -> ClerkClaims:
 
     # Use async version to avoid blocking
     jwk_data = await jwks_cache.get_key_async(kid)
-    logger.info("verify_clerk_token_async: Got JWKS data, starting signature verification")
+    logger.info(
+        "verify_clerk_token_async: Got JWKS data, starting signature verification"
+    )
 
     # Run CPU-intensive JWT operations in executor to avoid blocking
     def verify_signature():
@@ -297,7 +299,9 @@ def verify_signature():
     ]
     if audience_values:
         if not any(audience in audience_values for audience in allowed_audiences):
-            logger.error(f"verify_clerk_token_async: Invalid audience {audience_values}")
+            logger.error(
+                f"verify_clerk_token_async: Invalid audience {audience_values}"
+            )
             raise InvalidClerkToken("Invalid audience")
 
     if settings.clerk_authorized_parties:
@@ -308,7 +312,9 @@ def verify_signature():
                 _normalize_party(party) for party in settings.clerk_authorized_parties
             }
             if "*" not in allowed and normalized_azp not in allowed:
-                logger.error(f"verify_clerk_token_async: Unauthorized party {normalized_azp}")
+                logger.error(
+                    f"verify_clerk_token_async: Unauthorized party {normalized_azp}"
+                )
                 raise InvalidClerkToken("Token not issued for this application")
         else:
             logger.error("verify_clerk_token_async: Missing authorized party")
@@ -401,14 +407,16 @@ async def dispatch(
                 # Use async version to avoid blocking the event loop
                 # Add timeout to prevent hanging
                 request.state.clerk_claims = await asyncio.wait_for(
-                    verify_clerk_token_async(token),
-                    timeout=10.0
+                    verify_clerk_token_async(token), timeout=10.0
                 )
             except asyncio.TimeoutError:
                 import logging
+
                 logger = logging.getLogger(__name__)
                 logger.error("Clerk token verification timed out after 10 seconds")
-                request.state.clerk_auth_error = InvalidClerkToken("Token verification timeout")
+                request.state.clerk_auth_error = InvalidClerkToken(
+                    "Token verification timeout"
+                )
             except InvalidClerkToken as exc:
                 request.state.clerk_auth_error = exc
 

commitly-backend/app/main.py

@@ -38,70 +38,29 @@ def _configure_logging() -> None:
 
 @asynccontextmanager
 async def lifespan(app: FastAPI):
-    """Handle application startup and shutdown."""
+    """
+    Keep startup lightweight: only verify we can talk to the database.
+    Heavy schema fixes belong in Alembic migrations (run separately).
+    """
     import asyncio
 
-    # Startup: Test database connection
-    logger.info("Testing database connection...")
-    try:
-        def test_connection():
+    async def ping_database() -> None:
+        def _ping():
             with SessionLocal() as session:
                 session.execute(text("SELECT 1"))
-                session.commit()
 
-        # Run in executor to avoid blocking
         loop = asyncio.get_event_loop()
-        await asyncio.wait_for(
-            loop.run_in_executor(None, test_connection),
-            timeout=5.0
-        )
-        logger.info("✅ Database connection successful")
-    except asyncio.TimeoutError:
-        logger.error("❌ Database connection timeout after 5 seconds")
-    except Exception as e:
-        logger.error(f"❌ Database connection failed: {e}", exc_info=True)
+        await loop.run_in_executor(None, _ping)
 
-    # Verify schema: Check if required columns exist
-    # Note: Migrations are run in pre-deploy script, not here
-    logger.info("Verifying database schema...")
     try:
-        def verify_schema():
-            with SessionLocal() as session:
-                # Check if primary_language column exists
-                result = session.execute(
-                    text(
-                        """
-                        SELECT column_name
-                        FROM information_schema.columns
-                        WHERE table_name = 'generated_roadmaps'
-                        AND column_name = 'primary_language'
-                        """
-                    )
-                )
-                return result.fetchone() is not None
-
-        # Run in executor to avoid blocking
-        loop = asyncio.get_event_loop()
-        column_exists = await asyncio.wait_for(
-            loop.run_in_executor(None, verify_schema),
-            timeout=5.0
-        )
-
-        if column_exists:
-            logger.info("✅ Database schema verified - all required columns exist")
-        else:
-            logger.warning(
-                "⚠️ Required columns missing. Pre-deploy script should have fixed this. "
-                "If this persists, check pre-deploy command execution."
-            )
+        await asyncio.wait_for(ping_database(), timeout=5.0)
+        logger.info("✅ Database connection healthy at startup")
     except asyncio.TimeoutError:
-        logger.error("❌ Schema verification timeout after 5 seconds")
-    except Exception as e:
-        logger.error(f"❌ Schema verification failed: {e}", exc_info=True)
+        logger.error("❌ Database ping timed out (5s)")
+    except Exception as exc:  # pragma: no cover - startup diagnostic
+        logger.error(f"❌ Database ping failed: {exc}", exc_info=True)
 
-    logger.info("🚀 Application startup complete")
     yield
-    # Shutdown
     logger.info("Application shutting down")
 
 
@@ -116,25 +75,36 @@ def verify_schema():
 )
 
 # Add CORS middleware
-# Ensure allowed_origins is always a list
-# (validator should handle this, but type-safe check)
 if isinstance(settings.allowed_origins, str):
-    cors_origins = [settings.allowed_origins] if settings.allowed_origins else ["*"]
+    configured_origins = [settings.allowed_origins] if settings.allowed_origins else []
 else:
-    cors_origins = list(settings.allowed_origins or ["*"])
+    configured_origins = list(settings.allowed_origins or [])
 
 local_dev_origins = {
     "http://localhost:3000",
     "http://localhost:3700",
 }
-for origin in local_dev_origins:
-    if origin not in cors_origins:
-        cors_origins.append(origin)
+
+cors_origins = sorted({*configured_origins, *local_dev_origins} - {None, ""})
+
+allow_origin_regex = None
+allow_credentials = True
+
+if "*" in configured_origins:
+    # FastAPI disallows credentials when using wildcard origins. Fall back to
+    # a permissive regex and explicitly disable credentials to avoid misconfig.
+    cors_origins = sorted({*configured_origins, *local_dev_origins} - {"*", None, ""})
+    allow_origin_regex = r"https?://.*"
+    allow_credentials = False
+    logger.warning(
+        "CORS wildcard detected. Falling back to regex with credentials disabled."
+    )
 
 app.add_middleware(
     CORSMiddleware,
-    allow_origins=cors_origins or ["*"],
-    allow_credentials=True,
+    allow_origins=cors_origins,
+    allow_origin_regex=allow_origin_regex,
+    allow_credentials=allow_credentials,
     allow_methods=["*"],
     allow_headers=["*"],
 )