Merge pull request #39 from The-Obstacle-Is-The-Way/fix/staticfiles-cors-bug-004

.pre-commit-config.yaml

@@ -1,4 +1,7 @@
 repos:
+  # ============================================
+  # BACKEND HOOKS (Python)
+  # ============================================
   - repo: https://github.com/astral-sh/ruff-pre-commit
     rev: v0.14.8
     hooks:
@@ -17,6 +20,36 @@ repos:
         # Exclude auto-generated Gradio custom component files
         exclude: ^packages/niivueviewer/
 
+  # ============================================
+  # FRONTEND HOOKS (TypeScript/React)
+  # ============================================
+  - repo: local
+    hooks:
+      - id: frontend-lint
+        name: frontend-lint (eslint)
+        entry: bash -c 'cd frontend && npm run lint'
+        language: system
+        files: ^frontend/.*\.(ts|tsx|js|jsx)$
+        pass_filenames: false
+
+      - id: frontend-typecheck
+        name: frontend-typecheck (tsc)
+        entry: bash -c 'cd frontend && npx tsc --noEmit'
+        language: system
+        files: ^frontend/.*\.(ts|tsx)$
+        pass_filenames: false
+
+      - id: frontend-test
+        name: frontend-test (vitest coverage)
+        entry: bash -c 'cd frontend && npm run test:coverage'
+        language: system
+        files: ^frontend/.*\.(ts|tsx)$
+        pass_filenames: false
+        stages: [pre-push]  # Run on push only (tests are slow)
+
+  # ============================================
+  # GENERAL HOOKS
+  # ============================================
   - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v6.0.0
     hooks:

Dockerfile

@@ -75,9 +75,13 @@ EXPOSE 7860
 # Reset ENTRYPOINT from base image
 ENTRYPOINT []
 
-# Explicit frontend origin for CORS (backup to regex)
+# Explicit frontend origin for CORS
 ENV FRONTEND_ORIGIN=https://vibecodermcswaggins-stroke-viewer-frontend.hf.space
 
+# Explicit backend public URL for constructing file URLs
+# This ensures correct https:// URLs even if proxy headers aren't forwarded correctly
+ENV BACKEND_PUBLIC_URL=https://vibecodermcswaggins-stroke-deepisles-demo.hf.space
+
 # Run FastAPI with uvicorn (module path: stroke_deepisles_demo.api.main:app)
 # --proxy-headers: Trust X-Forwarded-Proto from HF Spaces proxy (ensures https:// in request.base_url)
 CMD ["uvicorn", "stroke_deepisles_demo.api.main:app", "--host", "0.0.0.0", "--port", "7860", "--proxy-headers"]

frontend/README.md

@@ -99,7 +99,7 @@ If you fork this repository, update these files before deploying:
    ```
 
 2. **Backend CORS** (`src/stroke_deepisles_demo/api/main.py`):
-   Update the `allow_origin_regex` to match your frontend Space URL.
+   Add your frontend URL to the `CORS_ORIGINS` list, or set `FRONTEND_ORIGIN` env var.
 
 3. **Rebuild frontend**:
    ```bash

frontend/src/components/CaseSelector.tsx

@@ -1,5 +1,10 @@
 import { useEffect, useState } from "react";
-import { apiClient } from "../api/client";
+import { apiClient, ApiError } from "../api/client";
+
+// Cold start retry configuration (matches useSegmentation.ts)
+const MAX_COLD_START_RETRIES = 5;
+const INITIAL_RETRY_DELAY = 2000;
+const MAX_RETRY_DELAY = 30000;
 
 interface CaseSelectorProps {
   selectedCase: string | null;
@@ -13,36 +18,84 @@ export function CaseSelector({
   const [cases, setCases] = useState<string[]>([]);
   const [isLoading, setIsLoading] = useState(true);
   const [error, setError] = useState<string | null>(null);
+  const [retryCount, setRetryCount] = useState(0);
+  const [isWakingUp, setIsWakingUp] = useState(false);
 
+  // Fetch cases on mount with cold-start retry logic
+  // Using inline async function pattern recommended by React docs for data fetching
   useEffect(() => {
+    let isActive = true;
     const abortController = new AbortController();
 
-    const fetchCases = async () => {
-      try {
-        const data = await apiClient.getCases(abortController.signal);
-        setCases(data.cases);
-      } catch (err) {
-        // Ignore abort errors - component unmounted
-        if (err instanceof Error && err.name === "AbortError") return;
+    async function fetchCases() {
+      let attempts = 0;
+
+      while (attempts <= MAX_COLD_START_RETRIES && isActive) {
+        try {
+          const data = await apiClient.getCases(abortController.signal);
+          if (!isActive) return;
+          setCases(data.cases);
+          setIsWakingUp(false);
+          setRetryCount(0);
+          setIsLoading(false);
+          return; // Success
+        } catch (err) {
+          if (!isActive) return;
+          if (err instanceof Error && err.name === "AbortError") return;
+
+          const is503 = err instanceof ApiError && err.status === 503;
+          const isNetworkError =
+            err instanceof TypeError &&
+            err.message.toLowerCase().includes("fetch");
 
-        const message = err instanceof Error ? err.message : "Unknown error";
-        setError(`Failed to load cases: ${message}`);
-      } finally {
-        if (!abortController.signal.aborted) {
+          // Retry on cold start (503) or network errors
+          if ((is503 || isNetworkError) && attempts < MAX_COLD_START_RETRIES) {
+            attempts++;
+            setRetryCount(attempts);
+            setIsWakingUp(true);
+
+            // Exponential backoff
+            const delay = Math.min(
+              INITIAL_RETRY_DELAY * Math.pow(2, attempts - 1),
+              MAX_RETRY_DELAY,
+            );
+            await new Promise((resolve) => setTimeout(resolve, delay));
+            continue;
+          }
+
+          // Max retries exceeded or non-retryable error
+          const message =
+            is503 || isNetworkError
+              ? "Backend failed to wake up. Please refresh the page."
+              : err instanceof Error
+                ? err.message
+                : "Unknown error";
+          setError(`Failed to load cases: ${message}`);
+          setIsWakingUp(false);
           setIsLoading(false);
+          return;
         }
       }
-    };
+    }
 
     fetchCases();
 
-    return () => abortController.abort();
+    return () => {
+      isActive = false;
+      abortController.abort();
+    };
   }, []);
 
   if (isLoading) {
     return (
       <div className="bg-gray-800 rounded-lg p-4">
-        <p className="text-gray-400">Loading cases...</p>
+        {isWakingUp ? (
+          <p className="text-yellow-400">
+            Backend waking up... Retry {retryCount}/{MAX_COLD_START_RETRIES}
+          </p>
+        ) : (
+          <p className="text-gray-400">Loading cases...</p>
+        )}
       </div>
     );
   }

frontend/src/components/__tests__/CaseSelector.test.tsx

@@ -2,7 +2,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest";
 import { render, screen, waitFor } from "@testing-library/react";
 import userEvent from "@testing-library/user-event";
 import { server } from "../../mocks/server";
-import { errorHandlers } from "../../mocks/handlers";
+import { errorHandlers, resetCasesAttempts } from "../../mocks/handlers";
 import { CaseSelector } from "../CaseSelector";
 
 describe("CaseSelector", () => {
@@ -10,6 +10,7 @@ describe("CaseSelector", () => {
 
   beforeEach(() => {
     mockOnSelectCase.mockClear();
+    resetCasesAttempts();
   });
 
   it("shows loading state initially", () => {
@@ -117,4 +118,32 @@ describe("CaseSelector", () => {
     const container = screen.getByRole("combobox").closest("div");
     expect(container).toHaveClass("bg-gray-800");
   });
+
+  it("retries on 503 cold-start and succeeds", async () => {
+    server.use(errorHandlers.casesColdStart);
+
+    render(
+      <CaseSelector selectedCase={null} onSelectCase={mockOnSelectCase} />,
+    );
+
+    // Should show waking up message during retry
+    await waitFor(
+      () => {
+        expect(screen.getByText(/waking up/i)).toBeInTheDocument();
+      },
+      { timeout: 3000 },
+    );
+
+    // Should eventually succeed and show cases
+    await waitFor(
+      () => {
+        expect(screen.getByRole("combobox")).toBeInTheDocument();
+      },
+      { timeout: 5000 },
+    );
+
+    expect(
+      screen.getByRole("option", { name: /sub-stroke0001/i }),
+    ).toBeInTheDocument();
+  });
 });

frontend/src/hooks/useSegmentation.ts

@@ -111,7 +111,19 @@ export function useSegmentation() {
         // Ignore abort errors
         if (err instanceof Error && err.name === "AbortError") return;
 
-        // Don't stop polling on transient network errors - retry next interval
+        // 404 = job expired or lost (HF restart) - this is TERMINAL, not transient
+        if (err instanceof ApiError && err.status === 404) {
+          stopPolling();
+          setIsLoading(false);
+          setJobStatus("failed");
+          setError(
+            "Job expired or lost. This can happen if the backend restarted. Please try again.",
+          );
+          setResult(null);
+          return;
+        }
+
+        // Other errors (network, 5xx) - retry next interval
         console.warn("Polling error (will retry):", err);
       }
     },

frontend/src/mocks/handlers.ts

@@ -173,6 +173,14 @@ export const handlers = [
   }),
 ];
 
+// Track retry attempts for cold-start testing
+let casesAttempts = 0;
+
+/** Reset the cases attempt counter (call in test beforeEach) */
+export function resetCasesAttempts(): void {
+  casesAttempts = 0;
+}
+
 // Error handlers for testing error states
 export const errorHandlers = {
   casesServerError: http.get(`${API_BASE}/api/cases`, () => {
@@ -186,6 +194,21 @@ export const errorHandlers = {
     return HttpResponse.error();
   }),
 
+  // 503 on first attempt, success on retry (tests cold-start retry)
+  casesColdStart: http.get(`${API_BASE}/api/cases`, async () => {
+    casesAttempts++;
+    if (casesAttempts === 1) {
+      return HttpResponse.json(
+        { detail: "Service Unavailable" },
+        { status: 503 },
+      );
+    }
+    // Succeed on retry
+    return HttpResponse.json({
+      cases: ["sub-stroke0001", "sub-stroke0002", "sub-stroke0003"],
+    });
+  }),
+
   segmentCreateError: http.post(`${API_BASE}/api/segment`, () => {
     return HttpResponse.json(
       { detail: "Failed to create job: case not found" },

src/stroke_deepisles_demo/api/config.py

@@ -0,0 +1,10 @@
+"""API configuration constants.
+
+Single source of truth for API configuration values.
+"""
+
+from pathlib import Path
+
+# Results directory for job outputs (must be in /tmp for HF Spaces)
+# CRITICAL: This is the single source of truth. Import this instead of hardcoding.
+RESULTS_DIR = Path("/tmp/stroke-results")

src/stroke_deepisles_demo/api/files.py

@@ -0,0 +1,85 @@
+"""File serving routes for NIfTI result files.
+
+BUG-004 FIX: This module replaces the StaticFiles mount approach.
+
+Previously, files were served via:
+    app.mount("/files", StaticFiles(directory=RESULTS_DIR))
+
+The problem: StaticFiles is a mounted sub-application, and FastAPI/Starlette
+middleware (including CORSMiddleware) does NOT propagate to mounted apps.
+This caused NiiVue's cross-origin fetch to fail with "Failed to fetch".
+
+Solution: Use explicit route handlers that go through the main app's middleware.
+Now CORS headers are correctly applied to file responses.
+
+Reference: https://github.com/fastapi/fastapi/discussions/7319
+"""
+
+from fastapi import APIRouter, HTTPException
+from fastapi.responses import FileResponse
+
+from stroke_deepisles_demo.api.config import RESULTS_DIR
+from stroke_deepisles_demo.core.logging import get_logger
+
+logger = get_logger(__name__)
+
+files_router = APIRouter(prefix="/files", tags=["files"])
+
+
+@files_router.get("/{job_id}/{case_id}/{filename}")
+async def get_result_file(job_id: str, case_id: str, filename: str) -> FileResponse:
+    """Serve NIfTI result files with proper CORS headers.
+
+    This route goes through the main FastAPI app's middleware stack,
+    ensuring CORS and CORP headers are applied to the response.
+
+    Args:
+        job_id: The job UUID from segmentation
+        case_id: The case identifier (e.g., sub-stroke0001)
+        filename: The NIfTI filename (e.g., dwi.nii.gz, prediction_fused.nii.gz)
+
+    Returns:
+        FileResponse with the NIfTI file
+
+    Raises:
+        404: File not found (job expired, invalid path, or doesn't exist)
+    """
+    # Construct file path
+    file_path = RESULTS_DIR / job_id / case_id / filename
+
+    # Security: Ensure path doesn't escape RESULTS_DIR (path traversal protection)
+    # Using is_relative_to() instead of startswith() to prevent prefix-collision bypass
+    # e.g., /tmp/stroke-results-evil/file.txt would pass startswith but fail is_relative_to
+    try:
+        base_dir = RESULTS_DIR.resolve()
+        resolved = file_path.resolve()
+        if not resolved.is_relative_to(base_dir):
+            logger.warning("Path traversal attempt blocked: %s", filename)
+            raise HTTPException(status_code=404, detail="File not found")
+    except (OSError, ValueError):
+        raise HTTPException(status_code=404, detail="Invalid file path") from None
+
+    # Check file exists
+    if not resolved.exists() or not resolved.is_file():
+        logger.debug("File not found: %s", resolved)
+        raise HTTPException(
+            status_code=404,
+            detail=f"File not found: {filename}. Job may have expired (1 hour TTL).",
+        )
+
+    # Determine media type based on extension
+    # NIfTI files are typically gzip-compressed
+    if filename.endswith(".nii.gz"):
+        media_type = "application/gzip"
+    elif filename.endswith(".nii"):
+        media_type = "application/octet-stream"
+    else:
+        media_type = "application/octet-stream"
+
+    logger.debug("Serving file: %s (type: %s)", resolved, media_type)
+
+    return FileResponse(
+        path=resolved,
+        media_type=media_type,
+        filename=filename,
+    )

src/stroke_deepisles_demo/api/job_store.py

@@ -23,11 +23,14 @@ import threading
 from dataclasses import dataclass
 from datetime import datetime, timedelta
 from enum import Enum
-from pathlib import Path
-from typing import Any
+from typing import TYPE_CHECKING, Any
 
+from stroke_deepisles_demo.api.config import RESULTS_DIR
 from stroke_deepisles_demo.core.logging import get_logger
 
+if TYPE_CHECKING:
+    from pathlib import Path
+
 logger = get_logger(__name__)
 
 # Regex for safe job IDs (alphanumeric, hyphens, underscores only)
@@ -135,7 +138,7 @@ class JobStore:
         self._jobs: dict[str, Job] = {}
         self._lock = threading.RLock()
         self._ttl = ttl
-        self._results_dir = results_dir or Path("/tmp/stroke-results")
+        self._results_dir = results_dir or RESULTS_DIR
         self._cleanup_thread: threading.Thread | None = None
         self._shutdown = threading.Event()
 

src/stroke_deepisles_demo/api/main.py

@@ -16,23 +16,20 @@ Architecture designed to work within HuggingFace Spaces constraints:
 import os
 from collections.abc import AsyncIterator
 from contextlib import asynccontextmanager
-from pathlib import Path
 from typing import Any
 
 from fastapi import FastAPI, Request, Response
 from fastapi.middleware.cors import CORSMiddleware
-from fastapi.staticfiles import StaticFiles
 from starlette.middleware.base import BaseHTTPMiddleware, RequestResponseEndpoint
 
+from stroke_deepisles_demo.api.config import RESULTS_DIR
+from stroke_deepisles_demo.api.files import files_router
 from stroke_deepisles_demo.api.job_store import init_job_store
 from stroke_deepisles_demo.api.routes import router
 from stroke_deepisles_demo.core.logging import get_logger
 
 logger = get_logger(__name__)
 
-# Results directory (must be in /tmp for HF Spaces)
-RESULTS_DIR = Path("/tmp/stroke-results")
-
 
 @asynccontextmanager
 async def lifespan(_app: FastAPI) -> AsyncIterator[None]:
@@ -115,24 +112,25 @@ if FRONTEND_ORIGIN and FRONTEND_ORIGIN not in CORS_ORIGINS:
 # Add CORP middleware first (for COEP compatibility)
 app.add_middleware(CORPMiddleware)
 
-# Add CORS middleware with strict security settings
+# Add CORS middleware with settings for NiiVue binary file fetching
 # Note: Using allow_origins list for exact matching (no regex needed)
 # This eliminates regex security concerns while maintaining single source of truth
 app.add_middleware(
     CORSMiddleware,
     allow_origins=CORS_ORIGINS,
     allow_credentials=False,  # Not needed - no cookies/auth
-    allow_methods=["GET", "POST"],  # Only methods we use
-    allow_headers=["Content-Type"],  # Only headers we need
+    allow_methods=["GET", "POST", "HEAD"],  # HEAD for preflight checks
+    allow_headers=["Content-Type", "Range"],  # Range needed for partial content requests
+    expose_headers=["Content-Range", "Content-Length", "Accept-Ranges"],  # NiiVue needs these
 )
 
-# API routes
+# API routes (includes /api/* endpoints)
 app.include_router(router, prefix="/api")
 
-# Static files for NIfTI results
-# Note: Mount happens at import time; ensure directory exists here as well.
-RESULTS_DIR.mkdir(parents=True, exist_ok=True)
-app.mount("/files", StaticFiles(directory=str(RESULTS_DIR)), name="files")
+# File routes (serves NIfTI results through main app's middleware for CORS)
+# BUG-004 FIX: Previously used StaticFiles mount which bypassed CORS middleware.
+# Now using explicit routes so CORS headers are applied to file responses.
+app.include_router(files_router)
 
 
 @app.get("/")

src/stroke_deepisles_demo/api/routes.py

@@ -12,10 +12,10 @@ from __future__ import annotations
 
 import os
 import uuid
-from pathlib import Path
 
 from fastapi import APIRouter, BackgroundTasks, HTTPException, Request
 
+from stroke_deepisles_demo.api.config import RESULTS_DIR
 from stroke_deepisles_demo.api.job_store import JobStatus, get_job_store
 from stroke_deepisles_demo.api.schemas import (
     CasesResponse,
@@ -33,9 +33,6 @@ logger = get_logger(__name__)
 
 router = APIRouter()
 
-# Base directory for results
-RESULTS_BASE = Path("/tmp/stroke-results")
-
 
 def get_backend_base_url(request: Request) -> str:
     """Get the backend's public URL for building absolute file URLs.
@@ -215,7 +212,7 @@ def run_segmentation_job(
         store.update_progress(job_id, 10, "Loading case data...")
 
         # Set up output directory
-        output_dir = RESULTS_BASE / job_id
+        output_dir = RESULTS_DIR / job_id
 
         store.update_progress(job_id, 20, "Staging files for DeepISLES...")