Backend functionality updates #9
Conversation
…s for API framework; refactor UI components to streamline imports; enhance feature engineering for time-based data; improve credit score analysis and generation forms with better error handling and user input management.
…tions; bringing back the Readme.md
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bempong-Sylvester-Obese
changed the title
…d firebase admin auth middleware and local JSON auto detect; update frontend API to send Firebase ID tokens; Prune DB deps from requirements.txt and remove SQL files; persist predictions, profiles, and history to firestore
WalkthroughAdds Firebase auth (backend FastAPI middleware and frontend Firebase client), Firestore persistence, lifespan-managed model loading and new prediction/profile endpoints, protected React routes with Google sign‑in and AuthProvider, new Pydantic and TypeScript schemas/types, dependency updates, a project guidelines doc, and several formatting/export fixes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant Auth as AuthProvider
participant Firebase
participant Client as FrontendAPI
participant Backend
participant Firestore
User->>Browser: Click "Sign in with Google"
Browser->>Auth: signInWithGoogle()
Auth->>Firebase: OAuth flow
Firebase-->>Auth: ID token & user
Auth->>Browser: hold user, provide getAuthToken()
rect rgba(100,200,150,0.06)
Note over Browser,Backend: Protected request (Bearer token)
Browser->>Client: POST /predict (file)
Client->>Client: getAuthToken() -> attach Authorization: Bearer <id_token>
Client->>Backend: request with Authorization header
Backend->>Firebase: verify token (auth_middleware)
Firebase-->>Backend: decoded token
Backend->>Backend: run predictor (lifespan-managed)
Backend->>Firestore: save prediction & metadata (services_firestore)
Firestore-->>Backend: confirmation
Backend-->>Browser: prediction response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas to review closely:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/constants/credit.ts (1)
51-55: Remove the extra closing brace.The extra
};after Line 54 breaks TypeScript parsing—compilation will fail.export const getRiskThreshold = (score: number): RiskThreshold => { const category = getRiskCategory(score); return RISK_THRESHOLDS[category]; -}; -}; +};src/lib/api.ts (1)
86-99: Fix the duplicatedfetchblock forpredictCreditScoreThe new authorization header logic now creates a second
const response = await fetch…immediately before the original one. This both redeclaresresponsein the same scope and – more importantly – sends the request without theformDatabody or abort signal, so uploads fail. Merge the header handling into the existing request instead of creating a new fetch.- const response = await fetch(`${API_BASE_URL}/api/predict`, { - method: 'POST', - headers, - const response = await fetch(`${API_BASE_URL}/api/predict`, { - method: 'POST', - body: formData, - signal: controller.signal, - }); + const response = await fetch(`${API_BASE_URL}/api/predict`, { + method: 'POST', + headers, + body: formData, + signal: controller.signal, + });api/main.py (1)
62-104: Remove duplicate/old code causing syntax errors.Lines 62-104 contain old code that wasn't removed during the refactoring to use
lifespan. This includes a duplicateFastAPIapp instantiation (lines 74-78) and the deprecated@app.on_event("startup")handler (lines 92-104). The duplicate code causes the syntax error flagged at line 133.Remove these lines entirely:
-from fastapi import FastAPI, File, UploadFile, HTTPException -from fastapi.middleware.cors import CORSMiddleware -from pathlib import Path -import sys -from src.predict import CreditScorePredictor -from src.features.build_features import engineer_features -import pandas as pd -import io -import uvicorn - -sys.path.insert(0, str(Path(__file__).parent.parent)) - -app = FastAPI( - title="AI Credit Score API", - description="ML-powered credit score prediction API", - version="1.0.0" -) - # Enable CORS for React frontend app.add_middleware( CORSMiddleware, allow_origins=["http://localhost:5173", "http://localhost:3000", "http://127.0.0.1:5173"], allow_credentials=True, allow_methods=["*"], allow_headers=["*"], ) - -# Initialize predictor on startup -predictor = None - -@app.on_event("startup") -async def startup_event(): - global predictor - try: - predictor = CreditScorePredictor( - model_path='models/model.pkl', - scaler_path='models/scaler.pkl', - features_path='models/features.csv' - ) - print("Model loaded successfully") - except Exception as e: - print(f"Error loading model: {str(e)}") - raise
🧹 Nitpick comments (7)
api/schemas.py (2)
24-37: Consider usingEmailStrfor consistency in the response model.The
UserProfileResponseis typed asOptional[str](line 32), while the create/update models useEmailStr. For consistency and to maintain validation semantics, consider usingOptional[EmailStr]unless there's a specific reason to use plainstrin responses.Apply this diff if consistency is desired:
mobile: Optional[str] = None employment_status: Optional[str] = None - email: Optional[str] = None + email: Optional[EmailStr] = None created_at: datetime
40-53: Consider more explicit typing forfeature_values.Line 47 uses
Optional[dict]which, while valid in Python 3.9+, is less explicit thanOptional[Dict[str, Any]]. The more explicit form provides better IDE support and clarity about the dictionary's contents.Apply this diff for more explicit typing:
+from typing import Optional, List, Dict, Any -from typing import Optional, List from datetime import datetime class PredictionResponse(BaseModel): id: int user_id: int credit_score: int risk_probability: float risk_category: str interpretation: Optional[str] = None - feature_values: Optional[dict] = None + feature_values: Optional[Dict[str, Any]] = None transaction_count: Optional[int] = Nonesrc/contexts/AuthContext.tsx (1)
31-70: Well-implemented authentication provider.The
AuthProvidercorrectly:
- Manages user and loading state
- Subscribes to Firebase auth state changes with proper cleanup
- Handles sign-in and sign-out operations
- Provides error handling
Consider whether
console.errorin production (lines 39, 48) might expose sensitive error details. You may want to use a more structured logging approach or sanitize error messages for production environments.src/lib/firebase.ts (1)
5-15: Consider validating required environment variables.The Firebase configuration is built from environment variables without validation. If any required variable is missing or undefined, Firebase initialization will fail with a cryptic error at runtime.
Consider adding validation before initialization:
// Firebase configuration const requiredEnvVars = [ 'VITE_FIREBASE_API_KEY', 'VITE_FIREBASE_AUTH_DOMAIN', 'VITE_FIREBASE_PROJECT_ID', 'VITE_FIREBASE_STORAGE_BUCKET', 'VITE_FIREBASE_MESSAGING_SENDER_ID', 'VITE_FIREBASE_APP_ID', ] as const; const missingVars = requiredEnvVars.filter( (varName) => !import.meta.env[varName] ); if (missingVars.length > 0) { throw new Error( `Missing required Firebase environment variables: ${missingVars.join(', ')}` ); } const firebaseConfig = { apiKey: import.meta.env.VITE_FIREBASE_API_KEY, authDomain: import.meta.env.VITE_FIREBASE_AUTH_DOMAIN, projectId: import.meta.env.VITE_FIREBASE_PROJECT_ID, storageBucket: import.meta.env.VITE_FIREBASE_STORAGE_BUCKET, messagingSenderId: import.meta.env.VITE_FIREBASE_MESSAGING_SENDER_ID, appId: import.meta.env.VITE_FIREBASE_APP_ID, };api/services_firestore.py (2)
69-79: Consider using Firestore's native offset for pagination.The current implementation fetches
limit + offsetdocuments and then slices the results in Python (line 79). While this works, it transfers unnecessary data from Firestore.Consider using Firestore's native offset method:
def get_user_predictions(uid: str, limit: int = 10, offset: int = 0) -> List[dict]: db = _get_db() q = ( db.collection('predictions') .where('uid', '==', uid) .order_by('created_at', direction=firestore.Query.DESCENDING) - .limit(limit + offset) + .offset(offset) + .limit(limit) ) docs = list(q.stream()) - items = [d.to_dict() | {'id': d.id, 'user_id': uid} for d in docs] - return items[offset:] + return [d.to_dict() | {'id': d.id, 'user_id': uid} for d in docs]Note: Firestore offset still scans skipped documents for billing, but this avoids transferring unnecessary data.
31-43: Remove unnecessary empty dict check.Lines 35-36 check if
datais empty and reset it to an empty dict, but this is unnecessary because line 37 immediately setsupdated_at, ensuringdatais never empty when passed toref.set().Apply this diff to simplify:
def create_or_update_user_profile(uid: str, payload: dict) -> dict: db = _get_db() ref = db.collection('users').document(uid).collection('profile').document('profile') data = {k: v for k, v in payload.items() if v is not None} - if not data: - data = {} data['updated_at'] = firestore.SERVER_TIMESTAMP ref.set(data, merge=True) snap = ref.get() out = snap.to_dict() or {} out['id'] = 'profile' out['user_id'] = uid return outapi/main.py (1)
217-273: Consider consolidating POST and PUT profile endpoints.Both
POST /api/profile(line 217) andPUT /api/profile(line 257) call the same underlying functioncreate_or_update_user_profile, making them functionally identical. This might be intentional for client convenience, but it deviates from REST conventions where POST typically creates and PUT updates.If this is intentional for flexibility, consider documenting it in the docstrings. Otherwise, consider making POST create-only:
@app.post("/api/profile", response_model=UserProfileResponse) async def create_profile( profile_data: UserProfileCreate, current_user: dict = Depends(get_current_user) ): """ Create user profile (creates if doesn't exist, updates if exists) """ get_or_create_user(current_user["uid"], current_user.get("email")) # Check if profile exists existing = get_user_profile(current_user["uid"]) if existing: raise HTTPException(status_code=409, detail="Profile already exists. Use PUT to update.") profile = create_or_update_user_profile(current_user["uid"], profile_data.model_dump()) return profile
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
api/__pycache__/__init__.cpython-311.pycis excluded by!**/*.pycapi/__pycache__/auth_middleware.cpython-311.pycis excluded by!**/*.pycapi/__pycache__/main.cpython-313.pycis excluded by!**/*.pycapi/__pycache__/schemas.cpython-311.pycis excluded by!**/*.pycapi/__pycache__/services_firestore.cpython-311.pycis excluded by!**/*.pycpackage-lock.jsonis excluded by!**/package-lock.jsonsrc/__pycache__/predict.cpython-311.pycis excluded by!**/*.pycsrc/features/__pycache__/build_features.cpython-311.pycis excluded by!**/*.pyc
📒 Files selected for processing (30)
.cursorules(1 hunks).gitignore(1 hunks)api/auth_middleware.py(1 hunks)api/main.py(5 hunks)api/schemas.py(1 hunks)api/services_firestore.py(1 hunks)package.json(1 hunks)requirements.txt(1 hunks)src/App.tsx(2 hunks)src/components/ProtectedRoute.tsx(1 hunks)src/components/ui/badge-variants.ts(1 hunks)src/components/ui/badge.tsx(1 hunks)src/components/ui/button-variants.ts(1 hunks)src/components/ui/button.tsx(2 hunks)src/components/ui/card.tsx(1 hunks)src/components/ui/dropdown-menu.tsx(1 hunks)src/components/ui/file-upload.tsx(1 hunks)src/components/ui/input.tsx(1 hunks)src/constants/credit.ts(1 hunks)src/contexts/AuthContext.tsx(1 hunks)src/features/build_features.py(1 hunks)src/index.css(1 hunks)src/lib/api.ts(4 hunks)src/lib/firebase.ts(1 hunks)src/lib/utils.ts(1 hunks)src/main.tsx(1 hunks)src/predict.py(1 hunks)src/types/credit.ts(1 hunks)src/views/home/Header.tsx(2 hunks)src/views/login/index.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
src/main.tsx (1)
src/contexts/AuthContext.tsx (1)
AuthProvider(31-70)
src/components/ProtectedRoute.tsx (1)
src/contexts/AuthContext.tsx (1)
useAuth(19-25)
src/types/credit.ts (1)
api/schemas.py (5)
UserProfileCreate(6-12)UserProfileUpdate(15-21)PredictionResponse(40-53)PredictionListResponse(63-67)ScoreHistoryItem(56-60)
src/views/login/index.tsx (1)
src/contexts/AuthContext.tsx (1)
useAuth(19-25)
src/views/home/Header.tsx (2)
src/contexts/AuthContext.tsx (1)
useAuth(19-25)src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(239-239)DropdownMenuTrigger(241-241)DropdownMenuContent(242-242)DropdownMenuItem(245-245)
src/App.tsx (2)
src/views/creditScoreAnalyses/analyses.tsx (1)
location(23-150)src/views/takeCredit/takeCredit.tsx (1)
navigate(29-31)
api/schemas.py (1)
src/types/credit.ts (5)
UserProfileCreate(29-36)UserProfileUpdate(38-45)PredictionResponse(47-58)ScoreHistoryItem(67-72)PredictionListResponse(60-65)
api/main.py (5)
src/predict.py (1)
CreditScorePredictor(12-140)src/features/build_features.py (1)
engineer_features(4-66)api/auth_middleware.py (1)
get_current_user(89-105)api/services_firestore.py (6)
get_or_create_user(8-21)get_user_profile(24-28)create_or_update_user_profile(31-43)save_prediction(46-66)get_user_predictions(69-79)get_user_score_history(82-104)api/schemas.py (6)
UserProfileCreate(6-12)UserProfileUpdate(15-21)UserProfileResponse(24-37)PredictionResponse(40-53)PredictionListResponse(63-67)ScoreHistoryItem(56-60)
src/lib/api.ts (2)
src/lib/firebase.ts (1)
auth(18-18)src/types/credit.ts (6)
ApiError(10-14)UserProfileCreate(29-36)UserProfile(16-27)UserProfileUpdate(38-45)PredictionListResponse(60-65)ScoreHistoryItem(67-72)
src/contexts/AuthContext.tsx (1)
src/lib/firebase.ts (2)
auth(18-18)googleProvider(21-21)
api/auth_middleware.py (1)
src/lib/firebase.ts (1)
auth(18-18)
🪛 Biome (2.1.2)
src/components/ui/button-variants.ts
[error] 34-35: Expected a statement but instead found ')'.
Expected a statement here.
(parse)
src/constants/credit.ts
[error] 54-55: Expected a statement but instead found '}'.
Expected a statement here.
(parse)
src/components/ui/badge-variants.ts
[error] 22-23: Expected a statement but instead found ')'.
Expected a statement here.
(parse)
🪛 Ruff (0.14.3)
api/main.py
133-133: Expected an indented block after function definition
(invalid-syntax)
api/auth_middleware.py
40-40: Do not catch blind exception: Exception
(BLE001)
47-47: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
71-71: Consider moving this statement to an else block
(TRY300)
73-76: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
78-81: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
82-82: Do not catch blind exception: Exception
(BLE001)
83-86: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
85-85: Use explicit conversion flag
Replace with conversion flag
(RUF010)
90-90: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
132-132: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (17)
src/index.css (1)
159-159: No concerns.This change only adjusts end-of-file formatting (newline presence) without any functional CSS modifications. All theme variables, selectors, and styling remain unchanged and properly structured.
src/features/build_features.py (1)
34-37: Comment removal is fine.The DWR ratio calculation is unchanged, so no action needed.
src/components/ui/dropdown-menu.tsx (1)
238-254: Trailing newline is harmless.Satisfied that the export block remains untouched functionally.
src/components/ui/input.tsx (1)
20-20: No issues in Input export tweak.Formatting-only adjustment; behavior stays the same.
.gitignore (1)
24-24: Good: firebasecredentials.json was never committed.Verified that firebasecredentials.json has no traces in git history and is not currently tracked. The .gitignore entry at line 24 is correct and properly prevents future accidental commits. No remediation needed.
src/components/ui/card.tsx (1)
105-105: LGTM: Stylistic export terminator.The addition of a semicolon to the export statement is a valid stylistic choice with no functional impact.
src/predict.py (1)
178-178: LGTM: Formatting cleanup.Whitespace adjustment with no functional impact.
src/components/ui/button.tsx (1)
4-4: LGTM: Formatting adjustment.Adding a blank line in the imports section improves readability.
package.json (1)
18-18: Firebase dependency version is current and secure.The addition of firebase ^12.5.0 is appropriate. The version 12.5.0 is available in the npm registry, and it is well above the vulnerable version threshold. CVE-2024-11023 affects firebase versions below 10.9.0, so version 12.5.0 is protected from this and other known vulnerabilities in the firebase package.
.cursorules (1)
1-164: Excellent documentation for project standards.This comprehensive Cursor Rules document provides clear guidelines for technology stack, coding conventions, security practices, and development workflows. Well-structured and valuable for maintaining consistency across the codebase.
api/schemas.py (1)
6-12: LGTM: Well-structured profile creation model.The
UserProfileCreatemodel correctly usesEmailStrfor email validation and makes all fields optional, allowing flexible profile creation.src/main.tsx (1)
4-13: LGTM: Proper authentication context integration.The
AuthProvideris correctly positioned within the component tree (BrowserRouter > AuthProvider > App), ensuring authentication state is available throughout the application while maintaining proper router context.src/App.tsx (1)
29-52: LGTM: Protected routes correctly implemented.The three routes are properly wrapped with
ProtectedRouteto enforce authentication. However, see the critical issue below regarding duplicate route definitions.src/contexts/AuthContext.tsx (1)
19-25: LGTM: Proper context hook with guard.The
useAuthhook correctly validates that it's used within anAuthProvider, preventing runtime errors from undefined context access.src/lib/firebase.ts (1)
24-26: LGTM: Good UX configuration for Google sign-in.Setting
prompt: 'select_account'ensures users can explicitly choose which Google account to use, improving the authentication experience.api/services_firestore.py (1)
82-104: LGTM - Good defensive handling of timestamp conversion.The use of
hasattr(ts, 'isoformat')at line 96 safely handles timestamp conversion without assuming the timestamp type. The implementation correctly transforms Firestore data into the expected response format.api/main.py (1)
192-202: LGTM - Good integration of authentication and persistence.The predict endpoint correctly integrates the authentication dependency and saves predictions to Firestore with all relevant context (feature values, transaction count, filename). The user creation with
get_or_create_userensures the user document exists before saving the prediction.
api/auth_middleware.py
Outdated
| try: | ||
| decoded_token = auth.verify_id_token(token) | ||
| return decoded_token | ||
| except auth.InvalidIdTokenError: | ||
| raise HTTPException( | ||
| status_code=401, | ||
| detail="Invalid authentication token" | ||
| ) | ||
| except auth.ExpiredIdTokenError: | ||
| raise HTTPException( | ||
| status_code=401, | ||
| detail="Authentication token has expired" | ||
| ) | ||
| except Exception as e: | ||
| raise HTTPException( | ||
| status_code=401, | ||
| detail=f"Authentication error: {str(e)}" | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Improve exception handling with proper chaining.
The exception handlers lack proper exception chaining, which loses valuable debugging context. The bare Exception catch at line 82 is also too broad and could mask unexpected errors.
Apply this diff to improve error handling:
try:
decoded_token = auth.verify_id_token(token)
return decoded_token
except auth.InvalidIdTokenError:
raise HTTPException(
status_code=401,
detail="Invalid authentication token"
- )
+ ) from None
except auth.ExpiredIdTokenError:
raise HTTPException(
status_code=401,
detail="Authentication token has expired"
- )
- except Exception as e:
+ ) from None
+ except auth.RevokedIdTokenError as e:
+ raise HTTPException(
+ status_code=401,
+ detail="Authentication token has been revoked"
+ ) from e
+ except Exception as e:
raise HTTPException(
status_code=401,
- detail=f"Authentication error: {str(e)}"
- )
+ detail=f"Authentication error: {e!r}"
+ ) from eCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.3)
71-71: Consider moving this statement to an else block
(TRY300)
73-76: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
78-81: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
82-82: Do not catch blind exception: Exception
(BLE001)
83-86: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
85-85: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
In api/auth_middleware.py around lines 69-86, the current exception handlers
drop original traceback/context and the final broad except may mask unrelated
failures; update each raise of HTTPException to use exception chaining (raise
HTTPException(...) from e) so the original exception is preserved, and remove or
narrow the bare "except Exception" handler — either let non-auth errors bubble
up (re-raise) or explicitly catch only expected auth-related exceptions and
chain them when converting to a 401.
| async def get_optional_user( | ||
| authorization: Optional[str] = Header(None) | ||
| ) -> Optional[dict]: | ||
| """ | ||
| Optional authentication - returns user if token is present, None otherwise. | ||
| Useful for endpoints that work with or without authentication. | ||
| Args: | ||
| authorization: Optional Authorization header | ||
| Returns: | ||
| User information if authenticated, None otherwise | ||
| """ | ||
| if not authorization or not authorization.startswith("Bearer "): | ||
| return None | ||
|
|
||
| try: | ||
| token = authorization.split("Bearer ")[1] | ||
| decoded_token = auth.verify_id_token(token) | ||
| return { | ||
| "uid": decoded_token.get("uid"), | ||
| "email": decoded_token.get("email"), | ||
| "firebase_claims": decoded_token | ||
| } | ||
| except Exception: | ||
| return None No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add Firebase initialization check for consistency.
The function calls auth.verify_id_token without first checking if _firebase_initialized is True, which could lead to confusing errors if Firebase isn't configured. This is inconsistent with verify_firebase_token.
Apply this diff:
async def get_optional_user(
authorization: Optional[str] = Header(None)
) -> Optional[dict]:
"""
Optional authentication - returns user if token is present, None otherwise.
Useful for endpoints that work with or without authentication.
Args:
authorization: Optional Authorization header
Returns:
User information if authenticated, None otherwise
"""
+ if not _firebase_initialized:
+ return None
+
if not authorization or not authorization.startswith("Bearer "):
return None
try:
token = authorization.split("Bearer ")[1]
decoded_token = auth.verify_id_token(token)
return {
"uid": decoded_token.get("uid"),
"email": decoded_token.get("email"),
"firebase_claims": decoded_token
}
except Exception:
return None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async def get_optional_user( | |
| authorization: Optional[str] = Header(None) | |
| ) -> Optional[dict]: | |
| """ | |
| Optional authentication - returns user if token is present, None otherwise. | |
| Useful for endpoints that work with or without authentication. | |
| Args: | |
| authorization: Optional Authorization header | |
| Returns: | |
| User information if authenticated, None otherwise | |
| """ | |
| if not authorization or not authorization.startswith("Bearer "): | |
| return None | |
| try: | |
| token = authorization.split("Bearer ")[1] | |
| decoded_token = auth.verify_id_token(token) | |
| return { | |
| "uid": decoded_token.get("uid"), | |
| "email": decoded_token.get("email"), | |
| "firebase_claims": decoded_token | |
| } | |
| except Exception: | |
| return None | |
| async def get_optional_user( | |
| authorization: Optional[str] = Header(None) | |
| ) -> Optional[dict]: | |
| """ | |
| Optional authentication - returns user if token is present, None otherwise. | |
| Useful for endpoints that work with or without authentication. | |
| Args: | |
| authorization: Optional Authorization header | |
| Returns: | |
| User information if authenticated, None otherwise | |
| """ | |
| if not _firebase_initialized: | |
| return None | |
| if not authorization or not authorization.startswith("Bearer "): | |
| return None | |
| try: | |
| token = authorization.split("Bearer ")[1] | |
| decoded_token = auth.verify_id_token(token) | |
| return { | |
| "uid": decoded_token.get("uid"), | |
| "email": decoded_token.get("email"), | |
| "firebase_claims": decoded_token | |
| } | |
| except Exception: | |
| return None |
🧰 Tools
🪛 Ruff (0.14.3)
132-132: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In api/auth_middleware.py around lines 108 to 133, get_optional_user calls
auth.verify_id_token without checking whether Firebase has been initialized;
update the function to first check the module-level _firebase_initialized flag
(same way verify_firebase_token does) and if Firebase is not initialized, return
None immediately (since this is an optional auth path), otherwise proceed to
extract the Bearer token and call auth.verify_id_token inside the existing
try/except; ensure behavior remains to return None on any exception.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
api/auth_middleware.py (2)
68-74: Improve exception handling with proper chaining and specific exception types.The exception handlers still lack proper exception chaining, losing valuable debugging context. The code doesn't handle
ExpiredIdTokenErrorandRevokedIdTokenErrorspecifically, and the bareExceptioncatch at line 73 is too broad.Apply this diff:
try: decoded_token = auth.verify_id_token(token) return decoded_token except auth.InvalidIdTokenError: raise HTTPException( status_code=401, detail="Invalid authentication token" - ) - except Exception as e: + ) from None + except auth.ExpiredIdTokenError: + raise HTTPException( + status_code=401, + detail="Authentication token has expired" + ) from None + except auth.RevokedIdTokenError as e: + raise HTTPException( + status_code=401, + detail="Authentication token has been revoked" + ) from e + except Exception as e: raise HTTPException( status_code=401, - detail=f"Authentication error: {str(e)}" - ) + detail=f"Authentication error: {e!r}" + ) from e
86-102: Add Firebase initialization check before token verification.The function calls
auth.verify_id_tokenat line 95 without checking if_firebase_initializedis True, which could lead to confusing errors if Firebase isn't configured. This is inconsistent withverify_firebase_token.Apply this diff:
async def get_optional_user( authorization: Optional[str] = Header(None) ) -> Optional[dict]: """Optional authentication - returns user if token is present, None otherwise.""" + if not _firebase_initialized: + return None + if not authorization or not authorization.startswith("Bearer "): return None try: token = authorization.split("Bearer ")[1] decoded_token = auth.verify_id_token(token) return { "uid": decoded_token.get("uid"), "email": decoded_token.get("email"), "firebase_claims": decoded_token } except Exception: return None
🧹 Nitpick comments (2)
api/auth_middleware.py (1)
48-53: Narrow the exception handler for better error diagnosis.The bare
Exceptioncatch at line 48 masks specific initialization failures. Consider catching only expected exceptions (ValueError,json.JSONDecodeError,OSError,IOError) to allow unexpected errors to surface.Apply this diff:
-except Exception as e: +except (ValueError, json.JSONDecodeError, OSError, IOError) as e: if _is_production(): raise else: print(f"Warning: Firebase Admin initialization failed: {e}") print("Authentication will be disabled.")src/contexts/AuthProvider.tsx (1)
41-48: Consider the loading state design.The
loadingfield in the context value (line 45) will always befalsefrom any consumer's perspective because line 41 prevents the provider from rendering children until loading is false. This makes theloadingfield in the context redundant.Additionally, the early return on line 41 blocks the entire app during the initial auth check. Consider whether:
- The
loadingfield should be removed fromAuthContextTypesince it's not effectively used, or- The provider should render children immediately and let components individually handle the loading state via the context's
loadingfield.If you prefer option 2, you could refactor like this:
- if (loading) return <div>Loading authentication...</div>; - const value: AuthContextType = { user, loading, signInWithGoogle, signOutUser, }; return <AuthContext.Provider value={value}>{children}</AuthContext.Provider>;Then let individual components (like
ProtectedRoute) handle loading states based onloadingfrom the context.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
api/auth_middleware.py(1 hunks)src/contexts/AuthContext.tsx(1 hunks)src/contexts/AuthProvider.tsx(1 hunks)src/contexts/useAuth.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/contexts/useAuth.ts (1)
src/contexts/AuthContext.tsx (1)
AuthContext(12-12)
src/contexts/AuthProvider.tsx (2)
src/lib/firebase.ts (2)
auth(18-18)googleProvider(21-21)src/contexts/AuthContext.tsx (2)
AuthContextType(4-9)AuthContext(12-12)
api/auth_middleware.py (1)
src/lib/firebase.ts (1)
auth(18-18)
🪛 Ruff (0.14.3)
api/auth_middleware.py
41-44: Avoid specifying long messages outside the exception class
(TRY003)
58-58: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
70-70: Consider moving this statement to an else block
(TRY300)
72-72: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
73-73: Do not catch blind exception: Exception
(BLE001)
74-74: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
74-74: Use explicit conversion flag
Replace with conversion flag
(RUF010)
77-77: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
101-101: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (7)
api/auth_middleware.py (2)
9-10: LGTM! Production environment detection added.The
_is_production()helper addresses the previous concern about accidentally deploying with disabled authentication to production.
76-84: LGTM! Clean user extraction logic.The function correctly extracts user information from the verified token. The static analysis warning about
Dependsin the argument default is a false positive - this is the standard FastAPI dependency injection pattern.src/contexts/useAuth.ts (1)
1-10: LGTM! Standard React context hook pattern.The implementation correctly wraps
useContextwith a runtime check to ensure the hook is used within anAuthProvider. The error message is clear and helpful.src/contexts/AuthContext.tsx (1)
1-12: LGTM! Clean context and type definitions.The
AuthContextTypeinterface is well-structured with appropriate FirebaseUsertyping, and the context is correctly initialized withundefinedto enforce provider usage.src/contexts/AuthProvider.tsx (3)
1-8: LGTM! Proper imports and type definitions.All necessary dependencies are imported, and the
AuthProviderPropsinterface is correctly typed for the provider component.
14-30: LGTM! Solid error handling pattern.Both authentication functions properly use try-catch blocks, log errors for debugging, and re-throw to allow caller-level error handling. This gives consumers control over error UX while maintaining visibility.
32-39: LGTM! Proper auth state subscription and cleanup.The effect correctly subscribes to Firebase auth state changes and returns the unsubscribe function for cleanup. The empty dependency array is appropriate since
authis stable.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/api.ts (1)
67-100: Refactor to use authenticatedFetch for consistency.This function manually handles authentication (lines 75-77) instead of using the
authenticatedFetchwrapper, creating code duplication and inconsistency with other API methods. SinceauthenticatedFetchpasses through the body unchanged, it works fine with FormData.Apply this refactor to align with other methods:
try { - const token = await getAuthToken(); - const headers: HeadersInit = {}; - if (token) headers['Authorization'] = `Bearer ${token}`; - - const response = await fetch(`${API_BASE_URL}/api/predict`, { + const response = await authenticatedFetch(`${API_BASE_URL}/api/predict`, { method: 'POST', - headers, body: formData, signal: controller.signal, });
♻️ Duplicate comments (1)
api/main.py (1)
206-219: Fix misleadingtotalfield in pagination response.The
totalfield at line 216 returnslen(predictions), which represents the current page size (up tolimititems), not the total count of all predictions. This makes it impossible for clients to implement proper pagination UI.This issue was previously flagged but remains unresolved.
Solutions:
- Remove the
totalfield (simplest; clients can uselen(predictions) < limitto detect last page)- Rename to
countto clarify it's the current page count- Add a separate Firestore count query to get the true total (requires additional query)
Option 1 (simplest):
predictions = get_user_predictions(current_user["uid"], limit=limit, offset=offset) return { "predictions": predictions, - "total": len(predictions), "limit": limit, "offset": offset }Then update
PredictionListResponseinapi/schemas.pyto remove thetotalfield.
🧹 Nitpick comments (6)
src/lib/api.ts (2)
29-49: Consider cleaner type handling for headers.The type assertion on line 42 works but is slightly awkward. Consider initializing headers as a plain object from the start:
- const headers: HeadersInit = { - ...options.headers, - }; - - if (token) { - (headers as Record<string, string>)['Authorization'] = `Bearer ${token}`; - } + const headers: Record<string, string> = { + ...(options.headers as Record<string, string>), + }; + + if (token) { + headers['Authorization'] = `Bearer ${token}`; + }
105-177: Consider adding timeout handling to other API methods.Only
predictCreditScoreincludes timeout protection (30s). The other API methods (createUserProfile,getUserProfile,updateUserProfile,getUserPredictions,getScoreHistory) could hang indefinitely on network issues or slow backends, potentially degrading user experience.Consider adding a timeout option to
authenticatedFetchor wrapping these calls similarly:const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), 10000); try { const response = await authenticatedFetch(url, { ...options, signal: controller.signal, }); clearTimeout(timeoutId); // ... rest of logic } catch (error) { clearTimeout(timeoutId); // handle error }api/main.py (4)
67-77: Review CORS configuration before production deployment.The current CORS settings use wildcards for methods and headers, which is acceptable for local development. Before deploying to production, consider restricting
allow_methodsandallow_headersto only what's necessary, and updateallow_originsto include production domain(s).
162-167: Add exception chaining for better error traceability.The exception handlers at lines 163, 165, and 167 should use
from errorfrom Noneto preserve (or explicitly suppress) the original exception context.Apply this diff:
except pd.errors.EmptyDataError: - raise HTTPException(status_code=400, detail="Empty CSV file") + raise HTTPException(status_code=400, detail="Empty CSV file") from None except pd.errors.ParserError: - raise HTTPException(status_code=400, detail="Invalid CSV format") + raise HTTPException(status_code=400, detail="Invalid CSV format") from None except Exception as e: - raise HTTPException(status_code=500, detail=f"Prediction error: {str(e)}") + raise HTTPException(status_code=500, detail=f"Prediction error: {str(e)}") from e
184-190: Consider auto-creating empty profile instead of returning 404.The GET endpoint returns 404 when a profile doesn't exist, but the POST endpoint (line 179) auto-creates the user. This inconsistency might surprise API consumers who have successfully authenticated.
Consider either:
- Auto-create an empty profile in the GET handler (consistent with POST behavior)
- Document that users must POST to create their profile before GET will succeed
Option 1 (auto-create empty profile):
@app.get("/api/profile", response_model=UserProfileResponse) async def get_profile(current_user: dict = Depends(get_current_user)): """Get user profile""" + get_or_create_user(current_user["uid"], current_user.get("email")) profile = get_user_profile(current_user["uid"]) if not profile: - raise HTTPException(status_code=404, detail="Profile not found") + # Auto-create empty profile + profile = create_or_update_user_profile(current_user["uid"], {}) return profile
173-200: POST and PUT endpoints are functionally identical.Both the POST (lines 173-181) and PUT (lines 193-200) endpoints call
create_or_update_user_profilewithmerge=True, making them functionally identical. This violates REST conventions where POST creates and PUT updates/replaces.Consider either:
- Make POST fail if profile already exists (create-only)
- Make PUT use merge=False for full replacement
- Keep current behavior but document that both endpoints upsert
- Remove one endpoint to reduce confusion
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
api/main.py(3 hunks)src/components/ui/badge-variants.ts(1 hunks)src/components/ui/badge.tsx(0 hunks)src/components/ui/button-variants.ts(1 hunks)src/components/ui/button.tsx(1 hunks)src/components/ui/file-upload.tsx(1 hunks)src/lib/api.ts(2 hunks)
💤 Files with no reviewable changes (1)
- src/components/ui/badge.tsx
✅ Files skipped from review due to trivial changes (1)
- src/components/ui/badge-variants.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/ui/button.tsx
- src/components/ui/file-upload.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/lib/api.ts (2)
src/lib/firebase.ts (1)
auth(18-18)src/types/credit.ts (6)
ApiError(10-14)UserProfileCreate(29-36)UserProfile(16-27)UserProfileUpdate(38-45)PredictionListResponse(60-65)ScoreHistoryItem(67-72)
api/main.py (5)
src/predict.py (2)
CreditScorePredictor(12-140)predict_full_assessment(121-140)src/features/build_features.py (1)
engineer_features(4-66)api/auth_middleware.py (1)
get_current_user(76-84)api/services_firestore.py (6)
get_or_create_user(8-21)get_user_profile(24-28)create_or_update_user_profile(31-43)save_prediction(46-66)get_user_predictions(69-79)get_user_score_history(82-104)api/schemas.py (6)
UserProfileCreate(6-12)UserProfileUpdate(15-21)UserProfileResponse(24-37)PredictionResponse(40-53)PredictionListResponse(63-67)ScoreHistoryItem(56-60)
🪛 Ruff (0.14.4)
api/main.py
45-45: Unused function argument: app
(ARG001)
111-111: Do not perform function call File in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
112-112: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
128-128: Abstract raise to an inner function
(TRY301)
133-133: Abstract raise to an inner function
(TRY301)
160-160: Consider moving this statement to an else block
(TRY300)
163-163: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
165-165: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
166-166: Do not catch blind exception: Exception
(BLE001)
167-167: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
167-167: Use explicit conversion flag
Replace with conversion flag
(RUF010)
176-176: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
185-185: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
196-196: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
208-208: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
224-224: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🔇 Additional comments (8)
src/components/ui/button-variants.ts (1)
34-34: Previous syntax error resolved.The extraneous closing delimiter mentioned in the earlier review has been removed. The file now correctly ends with a single
);that properly closes thecva()call, and the syntax is valid.src/lib/api.ts (4)
1-12: LGTM: Imports and configuration are clean.The grouped type imports and Firebase auth integration are properly structured. The API_BASE_URL follows standard practices with environment variable fallback.
14-27: LGTM: Token retrieval is properly implemented.The function correctly retrieves the Firebase ID token with appropriate null handling for unauthenticated states and error cases.
51-62: LGTM: Error handling is robust.The function properly parses API errors with a sensible fallback structure and correct
Promise<never>return type.
102-177: LGTM: Profile and history methods are well-implemented.All methods consistently use
authenticatedFetch, proper HTTP methods, and centralized error handling. Default parameters and query string handling are appropriate.api/main.py (3)
83-95: LGTM: Root endpoint provides clear API documentation.The endpoint overview is helpful for API discovery and includes all the main endpoints with their HTTP methods.
98-103: LGTM: Health check endpoint is well-implemented.The
model_loadedflag provides useful diagnostic information for deployment health checks.
222-229: LGTM: Score history endpoint is simple and clear.The endpoint provides historical scores with a reasonable default limit of 12. The response model correctly uses
List[ScoreHistoryItem].
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/predict.py (1)
28-46: Exception handling converts specific FileNotFoundError to generic ValueError.The explicit
FileNotFoundErrorexceptions raised in lines 29-34 are immediately caught by the broadExceptionhandler on line 45 and re-wrapped asValueErroron line 46. This loses important type information and prevents callers from distinguishing between "file not found" errors and other failure modes (e.g., corrupt pickle files, permission errors).Consider one of these approaches:
Option 1: Let FileNotFoundError propagate
def _load_artifacts(self): + # Check file existence first + if not self.model_path.exists(): + raise FileNotFoundError(f"Model file not found at {self.model_path}") + if not self.scaler_path.exists(): + raise FileNotFoundError(f"Scaler file not found at {self.scaler_path}") + if not self.features_path.exists(): + raise FileNotFoundError(f"Features file not found at {self.features_path}") + try: - if not self.model_path.exists(): - raise FileNotFoundError(f"Model file not found at {self.model_path}") - if not self.scaler_path.exists(): - raise FileNotFoundError(f"Scaler file not found at {self.scaler_path}") - if not self.features_path.exists(): - raise FileNotFoundError(f"Features file not found at {self.features_path}") - self.model = joblib.load(self.model_path) self.scaler = joblib.load(self.scaler_path) features_df = pd.read_csv(self.features_path, header=None) self.features = features_df.iloc[:, 0].tolist() print(f"Model artifacts loaded successfully") print(f"Using {len(self.features)} features: {self.features}") except Exception as e: raise ValueError(f"Failed to load model artifacts: {str(e)}")Option 2: Remove redundant checks
Since
joblib.load()andpd.read_csv()will fail with clear errors anyway:def _load_artifacts(self): try: - if not self.model_path.exists(): - raise FileNotFoundError(f"Model file not found at {self.model_path}") - if not self.scaler_path.exists(): - raise FileNotFoundError(f"Scaler file not found at {self.scaler_path}") - if not self.features_path.exists(): - raise FileNotFoundError(f"Features file not found at {self.features_path}") - self.model = joblib.load(self.model_path) self.scaler = joblib.load(self.scaler_path) features_df = pd.read_csv(self.features_path, header=None) self.features = features_df.iloc[:, 0].tolist() print(f"Model artifacts loaded successfully") print(f"Using {len(self.features)} features: {self.features}") except Exception as e: raise ValueError(f"Failed to load model artifacts: {str(e)}")
Replace print statements with proper logging.
Lines 42-43 use
print()for operational messages. Use theloggingmodule instead for production-ready code.Apply this refactor:
+import logging + +logger = logging.getLogger(__name__) + class CreditScorePredictor: def __init__(self, model_path='models/model.pkl', scaler_path='models/scaler.pkl', features_path='models/features.csv'):features_df = pd.read_csv(self.features_path, header=None) self.features = features_df.iloc[:, 0].tolist() - print(f"Model artifacts loaded successfully") - print(f"Using {len(self.features)} features: {self.features}") + logger.info("Model artifacts loaded successfully") + logger.debug(f"Using {len(self.features)} features: {self.features}") except Exception as e:src/views/login/index.tsx (1)
76-118: Non-functional email/password fields may confuse users.The email/password input fields and login button are rendered but have no functionality. Users may attempt to use these fields expecting them to work. Consider either removing these elements until the feature is implemented, or adding visual indicators (disabled state, "Coming Soon" label) to set appropriate expectations.
Consider one of these approaches:
Option 1: Add disabled state and coming soon indicator
<div> <label htmlFor="email" className="text-[#828282] text-sm"> - Email + Email (Coming Soon) </label> <Input id="email" name="email" + disabled placeholder="[email protected]" className="w-full rounded-[5px] bg-transparent border border-[#E8E8E8] text-[#828282] shadow-none placeholder:text-[#E0E0E0] placeholder:text-xs py-5" /> </div>Option 2: Hide the email/password section temporarily
-<span className="text-[#A1A1A1] text-xs flex items-center justify-center mb-5"> - ------or Sign in with Email------ -</span> - -<div> - {/* email/password fields */} -</div> +{/* TODO: Implement email/password authentication */}
♻️ Duplicate comments (1)
src/views/home/Header.tsx (1)
22-31: ResetisSigningOutin afinallyblock (previously flagged).This issue was already identified in a previous review but remains unaddressed. The
setIsSigningOut(false)call only executes in thecatchblock, leaving the flag permanentlytrueafter successful sign-out. Additionally, becausenavigate('/')may unmount this component, consider guarding the state update with anisMountedref or cleanup to prevent React warnings.Apply this diff to fix both issues:
const handleSignOut = async () => { + let isMounted = true; try { setIsSigningOut(true); await signOutUser(); navigate('/'); } catch (error) { console.error('Error signing out:', error); - setIsSigningOut(false); + } finally { + if (isMounted) { + setIsSigningOut(false); + } } + return () => { isMounted = false; }; };Or use a ref-based approach with useEffect cleanup if this pattern repeats.
🧹 Nitpick comments (2)
src/views/home/Header.tsx (1)
22-31: Consider adding user-visible error feedback.Currently, sign-out errors are only logged to the console. If
signOutUser()fails, users may not realize an error occurred, especially after navigation completes. Consider displaying a toast notification or error message.Example enhancement (requires a toast/notification library):
const handleSignOut = async () => { try { setIsSigningOut(true); await signOutUser(); navigate('/'); } catch (error) { console.error('Error signing out:', error); // Add user-visible feedback toast.error('Failed to sign out. Please try again.'); } finally { setIsSigningOut(false); } };src/views/login/index.tsx (1)
13-23: Consider using a finally block for loading state cleanup.The loading state is only reset in the catch block. While navigation typically succeeds and unmounts the component, using a
finallyblock would ensure cleanup in all scenarios.Apply this diff for more robust state management:
const handleGoogleSignIn = async () => { try { setIsLoading(true); setError(''); await signInWithGoogle(); navigate('/'); } catch (err) { setError(err instanceof Error ? err.message : 'Failed to sign in with Google'); - setIsLoading(false); + } finally { + setIsLoading(false); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
requirements.txt(1 hunks)src/App.tsx(2 hunks)src/components/ProtectedRoute.tsx(1 hunks)src/constants/credit.ts(1 hunks)src/main.tsx(1 hunks)src/predict.py(1 hunks)src/views/home/Header.tsx(2 hunks)src/views/login/index.tsx(2 hunks)vercel.json(0 hunks)
💤 Files with no reviewable changes (1)
- vercel.json
✅ Files skipped from review due to trivial changes (1)
- src/constants/credit.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main.tsx
- src/components/ProtectedRoute.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/App.tsx (1)
src/contexts/AuthProvider.tsx (1)
AuthProvider(10-51)
src/views/home/Header.tsx (2)
src/contexts/useAuth.ts (1)
useAuth(4-10)src/components/ui/dropdown-menu.tsx (4)
DropdownMenu(239-239)DropdownMenuTrigger(241-241)DropdownMenuContent(242-242)DropdownMenuItem(245-245)
src/views/login/index.tsx (1)
src/contexts/useAuth.ts (1)
useAuth(4-10)
🔇 Additional comments (2)
src/App.tsx (1)
24-26: LGTM! Proper resource cleanup.The cleanup function correctly disconnects the IntersectionObserver when the component unmounts, preventing memory leaks.
src/views/login/index.tsx (1)
56-69: LGTM! Proper loading state handling.The Google sign-in button correctly handles loading states by disabling the button and showing appropriate feedback to users during the authentication process.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
src/views/home/Header.tsx (1)
22-33: Remove redundantsetIsSigningOut(false)inhandleSignOut.You now reset
isSigningOutin both thecatchand thefinallyblocks; thecatchcall is unnecessary sincefinallyalways runs and handles the reset.- } catch (error) { - console.error('Error signing out:', error); - setIsSigningOut(false); - } finally { + } catch (error) { + console.error('Error signing out:', error); + } finally { setIsSigningOut(false); }This keeps the logic minimal while preserving behavior.
api/main.py (2)
151-171: Remove duplicate Firestore calls in/api/predict.You’re calling
get_or_create_userandsave_predictiontwice: once unguarded, then again inside the new try/except. This can:
- Save two prediction records per request.
- Reintroduce the original issue where Firestore errors can abort the response (from the unguarded calls).
Keep only the guarded block:
- # Firestore operations - get_or_create_user(current_user["uid"], current_user.get("email")) - save_prediction( - uid=current_user["uid"], - assessment=assessment, - feature_values=feature_dict, - transaction_count=len(df), - file_name=file.filename - ) - - try: - get_or_create_user(current_user["uid"], current_user.get("email")) - save_prediction( - uid=current_user["uid"], - assessment=assessment, - feature_values=feature_dict, - transaction_count=len(df), - file_name=file.filename) + # Firestore operations (non-blocking) + try: + get_or_create_user(current_user["uid"], current_user.get("email")) + save_prediction( + uid=current_user["uid"], + assessment=assessment, + feature_values=feature_dict, + transaction_count=len(df), + file_name=file.filename + ) except Exception as db_error: print(f"Warning: failed to persist prediction: {db_error}") return assessmentThis restores the intended behavior: prediction result is always returned, and persistence failures are logged once.
217-231: Clarify or remove misleadingtotalfield in predictions pagination.
/api/predictionscurrently returns:return { "predictions": predictions, "total": len(predictions), "count": len(predictions), "limit": limit, "offset": offset }
len(predictions)is just the current page size, not the total number of predictions for the user, sototalis misleading even though you addedcount.Either:
- Implement a true total count (separate Firestore query), or
- Remove/rename
totalto something likepage_countand updatePredictionListResponseaccordingly.For the simple option (no count query), you can do:
predictions = get_user_predictions(current_user["uid"], limit=limit, offset=offset) return { "predictions": predictions, - "total": len(predictions), "count": len(predictions), "limit": limit, "offset": offset }And then drop
totalfromPredictionListResponse.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
api/__pycache__/__init__.cpython-313.pycis excluded by!**/*.pycapi/__pycache__/auth_middleware.cpython-313.pycis excluded by!**/*.pycapi/__pycache__/schemas.cpython-313.pycis excluded by!**/*.pycapi/__pycache__/services_firestore.cpython-313.pycis excluded by!**/*.pycsrc/__pycache__/predict.cpython-313.pycis excluded by!**/*.pycsrc/features/__pycache__/build_features.cpython-313.pycis excluded by!**/*.pyc
📒 Files selected for processing (7)
api/main.py(3 hunks)api/schemas.py(1 hunks)requirements.txt(1 hunks)src/App.tsx(2 hunks)src/components/ProtectedRoute.tsx(1 hunks)src/views/home/Header.tsx(2 hunks)src/views/login/index.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- api/schemas.py
🧰 Additional context used
🧬 Code graph analysis (5)
src/views/home/Header.tsx (2)
src/contexts/useAuth.ts (1)
useAuth(4-10)src/components/ui/dropdown-menu.tsx (2)
DropdownMenu(239-239)DropdownMenuSubContent(222-236)
src/App.tsx (1)
src/contexts/AuthProvider.tsx (1)
AuthProvider(10-51)
src/views/login/index.tsx (1)
src/contexts/useAuth.ts (1)
useAuth(4-10)
src/components/ProtectedRoute.tsx (1)
src/contexts/useAuth.ts (1)
useAuth(4-10)
api/main.py (5)
src/predict.py (2)
CreditScorePredictor(11-139)predict_full_assessment(120-139)src/features/build_features.py (1)
engineer_features(4-66)api/auth_middleware.py (1)
get_current_user(76-84)api/services_firestore.py (6)
get_or_create_user(8-21)get_user_profile(24-28)create_or_update_user_profile(31-43)save_prediction(46-66)get_user_predictions(69-79)get_user_score_history(82-104)api/schemas.py (6)
UserProfileCreate(6-12)UserProfileUpdate(15-21)UserProfileResponse(24-37)PredictionResponse(40-53)PredictionListResponse(63-68)ScoreHistoryItem(56-60)
🪛 Ruff (0.14.4)
api/main.py
46-46: Unused function argument: app
(ARG001)
112-112: Do not perform function call File in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
113-113: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
129-129: Abstract raise to an inner function
(TRY301)
134-134: Abstract raise to an inner function
(TRY301)
169-169: Do not catch blind exception: Exception
(BLE001)
171-171: Consider moving this statement to an else block
(TRY300)
174-174: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
176-176: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
177-177: Do not catch blind exception: Exception
(BLE001)
178-178: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
178-178: Use explicit conversion flag
Replace with conversion flag
(RUF010)
187-187: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
196-196: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
207-207: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
219-219: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
236-236: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🔇 Additional comments (3)
src/App.tsx (1)
11-26: AuthProvider + intersection observer cleanup look good.Wrapping routes in
AuthProviderand addingobserver.disconnect()in theuseEffectcleanup are both correct and prevent leaks; the guarded routes for/generate-credit,/analyses, and/take-creditare wired appropriately.Also applies to: 29-57
src/components/ProtectedRoute.tsx (1)
1-34: ProtectedRoute implementation is solid.The loading UI, redirect with
state={{ from: location }}, and authenticated render path all look correct and align with the AuthProvider/useAuth contract.requirements.txt (1)
17-22: I'll search for known vulnerabilities in these specific dependency versions.Dependency constraints are well-chosen and safe.
firebase-admin 7.1.0 has no direct vulnerabilities in Snyk's database. The pydantic constraint >=2.4.0 specifically targets the fixed version for CVE-2024-3772, addressing a ReDoS vulnerability in earlier versions. email-validator 2.0.0 has no direct vulnerabilities reported. These constraints are appropriate for the auth and validation layers.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/App.tsx (1)
11-26: IntersectionObserver effect is correctly wired with proper cleanup.Lines 11-26 set up the scroll animation observer cleanly and disconnect it in the effect cleanup, which prevents leaks and duplicate observers on remounts. If you ever render
Appin SSR or a non‑browser test environment, consider guarding the setup with a runtime check (e.g.,if (typeof window === "undefined" || !("IntersectionObserver" in window)) return;) to avoid reference errors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/App.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/App.tsx (1)
src/contexts/AuthProvider.tsx (1)
AuthProvider(10-51)
🔇 Additional comments (1)
src/App.tsx (1)
29-67: Verification confirmed—all protected routes are properly scoped to App.tsx and correctly wrapped. The rg search output confirms that/generate-credit,/analyses,/take-credit, and/settingsare defined only insrc/App.tsxat the expected lines (34, 42, 50, 59), all wrapped in<ProtectedRoute>. No stray unprotected definitions of these routes exist elsewhere. The code matches the review assessment.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
api/main.py (2)
291-318: Still misleading:totalrepresents current page count, not total predictions.Lines 308-310 acknowledge that
totalis set to the returned count rather than the full dataset size, but the issue flagged in a previous review comment remains unresolved. API consumers expecttotalto represent all predictions across all pages for pagination UI.As suggested in the earlier review, either:
- Implement a separate Firestore count query for the true total
- Remove the
totalfield from the response (keeping onlycount)- Rename it to avoid confusion
The current implementation will break pagination UIs that try to calculate page numbers using
total.
219-222: Reconsider error handling: DB failure loses successful prediction result.When Firestore persistence fails (line 219), the endpoint raises a 500 error (line 222), preventing users from receiving their prediction result even though the ML model ran successfully. A previous review comment suggested returning the prediction even if persistence fails.
Consider this approach to preserve the prediction result:
except Exception as db_error: print(f"Warning: failed to persist prediction: {db_error}") - # Return assessment without id/user_id/created_at if save fails - raise HTTPException(status_code=500, detail=f"Failed to save prediction: {str(db_error)}") + # Return prediction with fallback values if persistence fails + return PredictionResponse( + id="", # or generate a temporary ID + user_id=current_user["uid"], + credit_score=assessment['credit_score'], + risk_probability=assessment['risk_probability'], + risk_category=assessment['risk_category'], + interpretation=assessment.get('interpretation'), + feature_values=feature_dict, + transaction_count=len(df), + file_name=file.filename, + created_at=datetime.now() + )This ensures users always get their prediction result, while logging alerts you to persistence issues.
🧹 Nitpick comments (6)
api/schemas.py (2)
71-76: Clarify the distinction betweentotalandcountfields.
PredictionListResponseincludes bothtotalandcountfields. Based on the endpoint implementation (api/main.py lines 308-318), both are currently set to the same value (the number of items returned in the current page), making one of them redundant.Consider:
- If
totalshould represent the total number of predictions across all pages, implement a separate Firestore count query and document that it may be approximate.- If you only need the current page count, remove
totaland keepcount.- Document which field represents what in the docstring.
For now, the implementation sets both to
len(converted_predictions), which works but may confuse API consumers expectingtotalto be the full dataset size.
6-21: Consider consolidating UserProfileCreate and UserProfileUpdate.Both classes have identical field definitions. While keeping them separate provides clarity at the API boundary, you could reduce duplication by having both inherit from a common base class.
Apply this refactor if you prefer DRY:
+class UserProfileBase(BaseModel): + first_name: Optional[str] = None + last_name: Optional[str] = None + postal_address: Optional[str] = None + mobile: Optional[str] = None + employment_status: Optional[str] = None + email: Optional[EmailStr] = None + -class UserProfileCreate(BaseModel): - first_name: Optional[str] = None - last_name: Optional[str] = None - postal_address: Optional[str] = None - mobile: Optional[str] = None - employment_status: Optional[str] = None - email: Optional[EmailStr] = None +class UserProfileCreate(UserProfileBase): + pass -class UserProfileUpdate(BaseModel): - first_name: Optional[str] = None - last_name: Optional[str] = None - postal_address: Optional[str] = None - mobile: Optional[str] = None - employment_status: Optional[str] = None - email: Optional[EmailStr] = None +class UserProfileUpdate(UserProfileBase): + passAlternatively, keep them separate for explicit API contracts—both approaches are valid.
api/main.py (2)
6-6: Remove unused import.The
filenameimport fromfileinputis never used in the file.-from fileinput import filename from fastapi import FastAPI, File, UploadFile, HTTPException, Depends, Query
224-229: Add exception chaining for better error diagnostics.The exception handlers raise new HTTPExceptions without chaining the original exception, making it harder to debug root causes in production logs.
Add
from errto preserve the exception chain:except pd.errors.EmptyDataError: - raise HTTPException(status_code=400, detail="Empty CSV file") + raise HTTPException(status_code=400, detail="Empty CSV file") from None except pd.errors.ParserError: - raise HTTPException(status_code=400, detail="Invalid CSV format") + raise HTTPException(status_code=400, detail="Invalid CSV format") from None except Exception as e: - raise HTTPException(status_code=500, detail=f"Prediction error: {str(e)}") + raise HTTPException(status_code=500, detail=f"Prediction error: {str(e)}") from eUse
from Nonefor expected user errors (empty/invalid CSV) andfrom efor unexpected exceptions to preserve the traceback.src/views/login/index.tsx (2)
8-25: Remove redundantsetIsLoading(false)in thecatchblock.
setIsLoading(false)runs in bothcatch(line 21) andfinally(line 23); the call incatchis now redundant after addingfinally. You can simplify:const handleGoogleSignIn = async () => { try { setIsLoading(true); setError(''); await signInWithGoogle(); navigate('/'); } catch (err) { setError( err instanceof Error ? err.message : 'Failed to sign in with Google', ); - setIsLoading(false); } finally { setIsLoading(false); } };This keeps the behavior the same with less noise.
77-120: Clarify or wire up the email/password “Login” button behavior.The email/password inputs and “Login” button (lines 77–120) are currently uncontrolled and not connected to any handler, so clicking “Login” has no effect. If email/password auth is not ready yet, consider disabling the button or adding a placeholder handler/tooltip so users aren’t confused by a non-functional control.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
api/main.py(3 hunks)api/schemas.py(1 hunks)src/App.tsx(2 hunks)src/views/login/index.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
api/main.py (5)
src/predict.py (2)
CreditScorePredictor(11-139)predict_full_assessment(120-139)src/features/build_features.py (1)
engineer_features(4-66)api/auth_middleware.py (1)
get_current_user(76-84)api/services_firestore.py (7)
get_or_create_user(8-21)get_user_profile(24-28)create_or_update_user_profile(31-43)save_prediction(46-66)get_user_predictions(69-79)get_user_score_history(82-104)get_latest_prediction(107-119)api/schemas.py (7)
UserProfileCreate(6-12)UserProfileUpdate(15-21)UserProfileResponse(24-37)PredictionResponse(48-61)PredictionListResponse(71-76)ScoreHistoryItem(64-68)PredictionAssessment(40-45)
api/schemas.py (1)
src/types/credit.ts (5)
UserProfileCreate(29-36)UserProfileUpdate(38-45)PredictionResponse(47-58)ScoreHistoryItem(67-72)PredictionListResponse(60-65)
src/views/login/index.tsx (1)
src/contexts/useAuth.ts (1)
useAuth(4-10)
🪛 Ruff (0.14.4)
api/main.py
49-49: Unused function argument: app
(ARG001)
137-137: Do not perform function call File in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
138-138: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
154-154: Abstract raise to an inner function
(TRY301)
159-159: Abstract raise to an inner function
(TRY301)
219-219: Do not catch blind exception: Exception
(BLE001)
222-222: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
222-222: Use explicit conversion flag
Replace with conversion flag
(RUF010)
225-225: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
227-227: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
228-228: Do not catch blind exception: Exception
(BLE001)
229-229: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
229-229: Use explicit conversion flag
Replace with conversion flag
(RUF010)
238-238: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
254-254: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
274-274: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
293-293: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
323-323: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🔇 Additional comments (5)
api/main.py (4)
48-60: LGTM: Proper use of async lifespan context manager.The lifespan manager correctly initializes the predictor during startup and is properly wired to the FastAPI app. The global
predictorpattern works well for this use case.Note: Ruff's warning about unused
appparameter is a false positive—FastAPI requires this signature for lifespan context managers.
113-129: LGTM: Robust timestamp conversion helper.The
convert_firestore_timestampfunction properly handles multiple timestamp formats with appropriate fallbacks, which is essential for consistent datetime handling across the API.
235-285: LGTM: Profile endpoints properly handle Firestore timestamps and missing fields.The profile CRUD endpoints correctly:
- Convert Firestore timestamps to Python datetime objects
- Add missing
idanduser_idfields in GET responses- Use proper dependency injection for authentication
321-328: LGTM: Score history endpoint is straightforward and correct.The endpoint properly delegates to the Firestore service and returns the expected schema.
src/App.tsx (1)
10-25: IntersectionObserver setup and cleanup look solid.Creating a single
IntersectionObserver, observing.animate-on-scrollelements, and disconnecting in the cleanup avoids leaks and redundant observers; no changes needed here.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
api/main.py (1)
290-317:totalin predictions list still returns page size, not global total.As a previous review noted,
totalis currentlylen(converted_predictions), i.e., the count for this page, not the total number of predictions for the user. That’s misleading for clients implementing pagination, especially since you also return a separatecountfield with the same value.Either:
- implement a separate count query for the true total, or
- drop/rename
totalto reflect that it’s just the page count and updatePredictionListResponseaccordingly.For the simpler “rename” approach in this handler:
- total_count = len(converted_predictions) - - return { - "predictions": converted_predictions, - "total": total_count, - "count": len(converted_predictions), - "limit": limit, - "offset": offset - } + count = len(converted_predictions) + + return { + "predictions": converted_predictions, + "count": count, + "limit": limit, + "offset": offset, + }Then align
PredictionListResponseinapi/schemas.pywith the new shape.
🧹 Nitpick comments (3)
api/main.py (3)
47-59: Tidy up unusedappargument inlifespan(Ruff ARG001).FastAPI still requires the
lifespan(app: FastAPI)signature, but Ruff flagsappas unused. Easiest fix is to rename the parameter and keep behavior identical:-@asynccontextmanager -async def lifespan(app: FastAPI): +@asynccontextmanager +async def lifespan(_app: FastAPI):You may also want to switch the
112-129: Make Firestore timestamp conversion more robust and timezone-aware.
datetime.fromtimestamp(ts.timestamp())uses the server’s local timezone; Firestore timestamps are UTC. Consider preferring the SDK’s conversion when available and making the fallback explicit, e.g.:- if hasattr(ts, 'timestamp'): - try: - # Firestore Timestamp.timestamp() returns seconds since epoch as float - return datetime.fromtimestamp(ts.timestamp()) - except (ValueError, OSError, AttributeError): - # Fallback: try to_datetime if available (some SDK versions) - if hasattr(ts, 'to_datetime'): - return ts.to_datetime() - return None + if hasattr(ts, "to_datetime"): + # Preferred path when Firestore Timestamp exposes to_datetime() + return ts.to_datetime() + if hasattr(ts, "timestamp"): + try: + return datetime.fromtimestamp(ts.timestamp()) + except (ValueError, OSError, AttributeError): + return NoneThis keeps responses consistent with Firestore semantics and avoids local‑timezone surprises.
134-138: Ruff B008 vs FastAPI’sFile(...)/Depends(...)pattern.Ruff’s B008 warnings on
File(...)andDepends(...)as defaults are expected in FastAPI code; this is the idiomatic way to declare dependencies. To keep Ruff happy without changing behavior, either:
- disable B008 globally for this project, or
- add targeted ignores on these endpoints, e.g.:
async def predict( file: UploadFile = File(...), # noqa: B008 current_user: dict = Depends(get_current_user), # noqa: B008 ): ...Same applies to the other endpoints using
Depends(...)below.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
api/main.py(3 hunks)package.json(2 hunks)src/App.tsx(2 hunks)src/train.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/App.tsx
- package.json
🧰 Additional context used
🧬 Code graph analysis (2)
src/train.py (1)
src/features/build_features.py (1)
engineer_features(4-66)
api/main.py (5)
src/predict.py (2)
CreditScorePredictor(11-139)predict_full_assessment(120-139)src/features/build_features.py (1)
engineer_features(4-66)api/auth_middleware.py (1)
get_current_user(76-84)api/services_firestore.py (7)
get_or_create_user(8-21)get_user_profile(24-28)create_or_update_user_profile(31-43)save_prediction(46-66)get_user_predictions(69-79)get_user_score_history(82-104)get_latest_prediction(107-119)api/schemas.py (7)
UserProfileCreate(6-12)UserProfileUpdate(15-21)UserProfileResponse(24-37)PredictionResponse(48-61)PredictionListResponse(71-76)ScoreHistoryItem(64-68)PredictionAssessment(40-45)
🪛 Ruff (0.14.4)
api/main.py
48-48: Unused function argument: app
(ARG001)
136-136: Do not perform function call File in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
137-137: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
153-153: Abstract raise to an inner function
(TRY301)
158-158: Abstract raise to an inner function
(TRY301)
218-218: Do not catch blind exception: Exception
(BLE001)
221-221: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
221-221: Use explicit conversion flag
Replace with conversion flag
(RUF010)
224-224: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
226-226: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
227-227: Do not catch blind exception: Exception
(BLE001)
228-228: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
228-228: Use explicit conversion flag
Replace with conversion flag
(RUF010)
237-237: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
253-253: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
273-273: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
292-292: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
322-322: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🔇 Additional comments (1)
src/train.py (1)
10-10: Good cleanup of unused import.Removing the unused
confusion_matriximport improves code quality.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/types/credit.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/types/credit.ts (1)
api/schemas.py (5)
UserProfileCreate(6-12)UserProfileUpdate(15-21)PredictionResponse(48-61)PredictionListResponse(71-76)ScoreHistoryItem(64-68)
🔇 Additional comments (4)
src/types/credit.ts (4)
29-36: LGTM!The
UserProfileCreateinterface correctly matches the backend schema with appropriate optional fields.
38-45: LGTM!The
UserProfileUpdateinterface correctly matches the backend schema with all fields optional for partial updates.
47-58: LGTM!The
PredictionResponseinterface correctly matches the backend schema with proper string types for Firebase identifiers and appropriate optional fields.
67-72: LGTM!The
ScoreHistoryIteminterface correctly matches the backend schema with appropriate types and optional fields.
I added a Google Authentication System to the login page to facilitate authentication using Google.
Screen.Recording.2025-11-02.at.01.19.16.mov
I intend to add the email/password authentication to it soon!.
Summary by CodeRabbit
New Features
Documentation
Chores