Add comments throughout the codebase #355#500
Add comments throughout the codebase #355#500GovindBuildsAI wants to merge 4 commits intoAOSSIE-Org:mainfrom
Conversation
* chore: remove unused files from web-ui * file move from web-ui to landing-page and also add the framer package which was use but not in the package.json * remove unused frontend files
added comments throughout the codebase to make it more understandable
WalkthroughIntroduces typed image models and revised DB queries, a new FaceDetector integrating YOLO and FaceNet with DB persistence, FaceNet/YOLO utility modules, FastAPI lifespan with OpenAPI generation and logging, explicit cache invalidation in app entry points, refined microservice startup/health logic, and frontend wiring for global loader/info dialog. Mostly adds documentation/comments elsewhere. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant FastAPI as FastAPI App
participant Lifespan as Lifespan Ctx
participant DB as Database
participant Logger as Logger
participant MS as Sync Microservice
participant FS as Filesystem (docs path)
User->>FastAPI: Start server
activate FastAPI
FastAPI->>Lifespan: enter()
activate Lifespan
Lifespan->>Logger: init app logger
Lifespan->>DB: create tables (folders, images, yolo_mappings, clusters, faces, albums, album_images, metadata)
Lifespan->>FastAPI: generate_openapi_json()
FastAPI->>FS: write docs/backend/backend_python/openapi.json
Lifespan->>MS: start microservice utility
Lifespan-->>FastAPI: yield (app ready)
deactivate Lifespan
Note over User,FastAPI: App serves requests...
User-->>FastAPI: Shutdown signal
FastAPI->>Lifespan: exit()
activate Lifespan
Lifespan->>FastAPI: shutdown ProcessPoolExecutor
Lifespan-->>FastAPI: complete shutdown
deactivate Lifespan
deactivate FastAPI
sequenceDiagram
autonumber
participant Caller as Detector Client
participant FaceDetector as FaceDetector.detect_faces
participant YOLO as YOLO Detector
participant CV as Image Loader
participant FaceNet as FaceNet Model
participant DB as db_insert_face_embeddings_by_image_id
Caller->>FaceDetector: detect_faces(image_id, image_path)
FaceDetector->>CV: load image
alt Load fails
FaceDetector-->>Caller: None
else Loaded
FaceDetector->>YOLO: infer(image)
YOLO-->>FaceDetector: boxes, scores, class_ids
loop each detection (conf>=threshold)
FaceDetector->>CV: crop + pad face ROI
FaceDetector->>FaceNet: preprocess ROI
FaceDetector->>FaceNet: embed(ROI)
FaceNet-->>FaceDetector: embedding
end
alt any embeddings
FaceDetector->>DB: insert embeddings + bbox + confidence
DB-->>FaceDetector: ack
end
FaceDetector-->>Caller: {ids, processed_faces, num_faces}
end
sequenceDiagram
autonumber
actor Client
participant Health as /health (Sync MS)
participant DB as db_check_database_connection
participant Watcher as watcher_util_is_watcher_running
Client->>Health: GET /api/v1/health
Health->>DB: check connection
DB-->>Health: True/False
Health->>Watcher: is running?
Watcher-->>Health: True/False
Health-->>Client: {status: healthy|unhealthy, database, watcher}
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
backend/app/database/images.py (2)
233-236: Don’t swallow DB errors in db_get_all_images().Returning [] on exception hides real failures and makes the API return 200 with empty data.
Apply:
- except Exception as e: - print(f"Error getting all images: {e}") - return [] + except Exception as e: + print(f"Error getting all images: {e}") + raise
272-283: db_get_untagged_images() returns ImageRecord but omits isTagged.Add isTagged to match the declared TypedDict.
Apply:
for image_id, path, folder_id, thumbnail_path, metadata in results: untagged_images.append( { "id": image_id, "path": path, "folder_id": folder_id, "thumbnailPath": thumbnail_path, "metadata": metadata, + "isTagged": False, } )backend/app/routes/images.py (1)
46-52: folder_id should be int to match DB and schema.Database returns INTEGER; current model defines str.
Apply:
- folder_id: str + folder_id: intbackend/app/utils/YOLO.py (2)
241-249: Off-by-one excludes the last COCO class and lacks bounds checks.
class_id >= len(class_names) - 1incorrectly treats index 79 as unknown; negative ids also not handled.- if score < confidence_threshold or class_id >= len(class_names) - 1: - color = colors[-1] - label = "unknown" + if score < confidence_threshold or class_id < 0 or class_id >= len(class_names): + color = (0, 0, 0) # fallback color + label = "unknown" else: color = colors[class_id] label = class_names[class_id]
300-309: Potential IndexError on out-of-range class_id in mask drawing.Add a safe fallback to avoid crashes when class_id is invalid.
- for box, class_id in zip(boxes, classes): - color = colors[class_id] + for box, class_id in zip(boxes, classes): + if 0 <= int(class_id) < len(colors): + color = colors[int(class_id)] + else: + color = (0, 0, 0)
🧹 Nitpick comments (25)
app.py (2)
27-30: Startup invalidate_cache() is likely a no-op with current in-memory cacheGiven utils.cache uses a process-local dict, a cold start begins empty; this call is harmless but redundant. If you prefer to keep it for clarity/future persistence, tweak the comment to avoid implying stale state exists at boot.
Apply this small wording tweak:
- # Clear any stale cache data on startup to ensure fresh data - # This prevents issues with outdated cached information + # Defensive: ensure empty cache on process start. + # Current cache is in-memory and already empty on cold start; kept for clarity/future persistence. invalidate_cache()
46-50: Extract cache keys into constants and add logging
- Replace string literals with named constants to prevent typos:
- invalidate_cache("albums:get_all_albums") + invalidate_cache(ALBUMS_ALL_KEY) … - invalidate_cache("folder_structure:get_folder_structure") + invalidate_cache(FOLDER_STRUCTURE_KEY)- Define at top of app.py after imports:
ALBUMS_ALL_KEY = "albums:get_all_albums" FOLDER_STRUCTURE_KEY = "folder_structure:get_folder_structure" import logging logger = logging.getLogger(__name__)- Add observability for each invalidation:
logger.info("Cache invalidated: %s", ALBUMS_ALL_KEY) logger.info("Cache invalidated: %s", FOLDER_STRUCTURE_KEY)- If invalidate_cache may run alongside readers, ensure cache invalidations are thread-safe (e.g., add locking or use atomic swaps).
frontend/src/routes/AppRoutes.tsx (2)
24-24: Prefer react-router-dom for web imports.Use the DOM bundle consistently across the app.
-import { Routes, Route } from 'react-router'; +import { Routes, Route, Navigate } from 'react-router-dom';
46-73: Add a catch-all route for unknown paths.Prevents blank screens or confusing states.
<Route element={<Layout />}> @@ {/* Person-based image organization using face clustering */} <Route path={ROUTES.PERSON} element={<PersonImages />} /> + {/* Fallback: redirect unknown routes to home */} + <Route path="*" element={<Navigate to={ROUTES.HOME} replace />} /> </Route>frontend/src/app/store.ts (1)
40-60: Add typed Redux hooks to remove inline RootState annotations.Simplifies usage and avoids repetitive types.
Create a new file (outside this diff) at frontend/src/app/hooks.ts:
import { TypedUseSelectorHook, useDispatch, useSelector } from 'react-redux'; import type { RootState, AppDispatch } from './store'; export const useAppDispatch = () => useDispatch<AppDispatch>(); export const useAppSelector: TypedUseSelectorHook<RootState> = useSelector;Then replace useSelector((state: RootState) => ...) with useAppSelector(...) where used.
frontend/src/App.tsx (1)
56-65: Optional: Place overlays inside BrowserRouter.If InfoDialog/Loader ever navigate or use Links, nesting them under BrowserRouter avoids context issues.
sync-microservice/app/core/lifespan.py (4)
32-35: Avoid module-level globals; keep lifecycle state on app.state.Using a module global can bite under reloads, tests, or multiple app instances. Store the flag on app.state instead.
Apply this diff within the shown ranges:
-# Global variable to track file system watcher status -# Used to ensure proper cleanup during shutdown -watcher_started = False +# (moved to app.state to avoid module-level globals)- global watcher_started + if not hasattr(app.state, "watcher_started"): + app.state.watcher_started = False- watcher_started = watcher_util_start_folder_watcher() + app.state.watcher_started = watcher_util_start_folder_watcher()- if watcher_started: + if getattr(app.state, "watcher_started", False): watcher_util_stop_folder_watcher()Also applies to: 64-66, 83-84, 105-107
41-43: Add return type for async lifespan.Helps tooling and readability.
-from contextlib import asynccontextmanager +from contextlib import asynccontextmanager +from typing import AsyncIterator @@ -async def lifespan(app: FastAPI): +async def lifespan(app: FastAPI) -> AsyncIterator[None]:
70-81: Use structured logging instead of print.Switching to logging integrates with Uvicorn/Infra logs and supports levels/formatting.
Minimal changes:
- print("Starting PictoPy Sync Microservice...") + logger.info("Starting PictoPy Sync Microservice...") @@ - print("Failed to connect to PictoPy database") - print( - "Make sure the main PictoPy backend is set up and the database exists" - ) + logger.error("Failed to connect to PictoPy database") + logger.info("Ensure the main PictoPy backend is set up and the database exists") @@ - print("Database connection successful") + logger.info("Database connection successful") @@ - print("Sync microservice is ready!") + logger.info("Sync microservice is ready!") @@ - print("\nReceived keyboard interrupt (Ctrl+C)") - print("Initiating graceful shutdown...") + logger.warning("Received keyboard interrupt (Ctrl+C). Initiating graceful shutdown...") @@ - print(f"Unexpected error during startup: {e}") + logger.exception("Unexpected error during startup: %s", e) @@ - print("Shutting down sync microservice...") + logger.info("Shutting down sync microservice...") @@ - print("Folder watcher stopped") + logger.info("Folder watcher stopped") @@ - print("Shutdown complete") + logger.info("Shutdown complete")Outside the ranges above, add once at top:
import logging logger = logging.getLogger(__name__)Also applies to: 90-110
72-80: Startup hard-fails if DB is down — consider readiness vs. liveness.Aborting startup is fine if this service must not run without DB. If you want the health endpoint to be reachable even when dependencies are down, start the app and report “unhealthy” instead, letting orchestration use readiness to gate traffic.
Is the intended behavior to refuse to start without a DB, or should we expose /health as unhealthy until dependencies recover?
sync-microservice/main.py (1)
56-60: Don’t override Uvicorn’s default logging config with None.Passing log_config=None disables the default structured logs. Prefer omitting it so you inherit Uvicorn’s defaults.
- config = Config(app=app, host="0.0.0.0", port=8001, log_config=None) + config = Config(app=app, host="0.0.0.0", port=8001)Optional simplification:
-if __name__ == "__main__": - # Configure uvicorn server for direct execution - config = Config(app=app, host="0.0.0.0", port=8001, log_config=None) - server = Server(config) - - # Start the sync microservice - server.run() +if __name__ == "__main__": + import uvicorn + uvicorn.run(app, host="0.0.0.0", port=8001)sync-microservice/app/routes/health.py (3)
27-41: Document response codes and intent (liveness vs. readiness).Consider clarifying whether /health is liveness (always 200) or readiness (503 when unhealthy).
42-50: Offload the synchronous DB check to a threadpool.Avoid blocking the event loop by running the sqlite check in a threadpool.
-from app.database.folders import db_check_database_connection +from app.database.folders import db_check_database_connection +from starlette.concurrency import run_in_threadpool @@ - db_status = db_check_database_connection() + db_status = await run_in_threadpool(db_check_database_connection)
51-55: Optionally set HTTP 503 when unhealthy (readiness).If this endpoint is used by load balancers for readiness, set 503 when dependencies are down.
-from fastapi import APIRouter +from fastapi import APIRouter, Response, status @@ -@router.get("/health", response_model=HealthCheckResponse) -async def health_check(): +@router.get("/health", response_model=HealthCheckResponse) +async def health_check(response: Response): @@ - return HealthCheckResponse( + if overall_status != "healthy": + response.status_code = status.HTTP_503_SERVICE_UNAVAILABLE + return HealthCheckResponse( status=overall_status, database="connected" if db_status else "disconnected", watcher="running" if watcher_status else "stopped", )If you prefer liveness semantics, keep 200 but expose a separate
/readyendpoint that returns 503 when not ready.backend/main.py (2)
27-27: Remove duplicate import of CustomizeLogger.The module imports CustomizeLogger twice.
Apply this diff:
- from app.custom_logging import CustomizeLoggerAlso applies to: 36-36
41-54: Gate side effects at startup (OpenAPI write, sync service) via env flags.Avoid doing heavy I/O or spawning processes unconditionally on app start.
Apply this diff:
- generate_openapi_json() + if os.getenv("GENERATE_OPENAPI_JSON", "true").lower() == "true": + generate_openapi_json() @@ - microservice_util_start_sync_service() + if os.getenv("START_SYNC_SERVICE", "true").lower() == "true": + if not microservice_util_start_sync_service(): + app.logger.warning("Sync microservice failed to start") + else: + app.logger.info("START_SYNC_SERVICE disabled; skipping sync microservice startup")backend/app/database/images.py (3)
265-267: Use 1/0 instead of TRUE/FALSE for SQLite booleans.More portable across SQLite builds.
Apply:
- WHERE f.AI_Tagging = TRUE - AND i.isTagged = FALSE + WHERE f.AI_Tagging = 1 + AND i.isTagged = 0
74-100: Consider enabling foreign keys per-connection (PRAGMA).SQLite requires PRAGMA foreign_keys=ON per connection; otherwise CASCADE won’t apply.
Optional: execute
cursor.execute("PRAGMA foreign_keys = ON")after connect() in writers.
132-147: Replace prints with structured logging.Use the project logger for consistency and observability.
If permissible, add:
from loguru import loggerThen replace print(...) with logger.error/info as appropriate.
Also applies to: 233-235, 347-350, 385-387, 419-421
backend/app/routes/images.py (1)
117-126: 500 error body doesn’t match documented model.Raising HTTPException nests the model under "detail", diverging from responses={500: {"model": ErrorResponse}}.
Apply:
- raise HTTPException( - status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - detail=ErrorResponse( - success=False, - error="Internal server error", - message=f"Unable to retrieve images: {str(e)}", - ).model_dump(), - ) + return JSONResponse( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + content=ErrorResponse( + success=False, + error="Internal server error", + message=f"Unable to retrieve images: {str(e)}", + ).model_dump(), + )Add import (outside this hunk):
from fastapi.responses import JSONResponsebackend/app/config/settings.py (1)
41-49: Prefer Path objects and env-driven config for portability.Using pathlib.Path improves path joins and OS portability; consider moving these to a typed Settings (e.g., Pydantic) with env overrides.
backend/app/utils/YOLO.py (2)
120-124: Use integer colors compatible with OpenCV.Current colors are float; cv2 expects int tuples. Generate uint8 once for consistency.
-rng = np.random.default_rng(3) # Fixed seed for reproducible colors -colors = rng.uniform(0, 255, size=(len(class_names), 3)) +rng = np.random.default_rng(3) # Fixed seed for reproducible colors +colors = rng.integers(0, 256, size=(len(class_names), 3), dtype=np.uint8)
331-334: Normalize and validate user preference for model size.Lowercasing helps avoid silent fallbacks due to casing mismatches.
- user_prefs = metadata["user_preferences"] - model_size = user_prefs.get("YOLO_model_size", "small") + user_prefs = metadata["user_preferences"] + model_size = str(user_prefs.get("YOLO_model_size", "small")).lower()backend/app/models/FaceDetector.py (2)
86-90: Replace prints with structured logging.Use the project’s logger to control verbosity and avoid stdout noise.
127-132: Return JSON-friendly types for ids.Stringifying a numpy array is awkward for consumers. Return a list of ints.
- return { - "ids": f"{class_ids}", + return { + "ids": class_ids.tolist() if hasattr(class_ids, "tolist") else list(class_ids), "processed_faces": processed_faces, "num_faces": len(embeddings), }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
app.py(1 hunks)backend/app/config/settings.py(1 hunks)backend/app/database/images.py(7 hunks)backend/app/models/FaceDetector.py(3 hunks)backend/app/routes/images.py(3 hunks)backend/app/utils/FaceNet.py(1 hunks)backend/app/utils/YOLO.py(2 hunks)backend/main.py(7 hunks)frontend/src/App.tsx(1 hunks)frontend/src/app/store.ts(1 hunks)frontend/src/routes/AppRoutes.tsx(1 hunks)sync-microservice/app/core/lifespan.py(2 hunks)sync-microservice/app/routes/health.py(1 hunks)sync-microservice/main.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
frontend/src/routes/AppRoutes.tsx (2)
frontend/src/pages/InitialSteps/InitialSteps.tsx (1)
InitialSteps(7-21)frontend/src/constants/routes.ts (1)
ROUTES(1-11)
app.py (1)
utils/cache.py (1)
invalidate_cache(44-55)
sync-microservice/main.py (2)
backend/main.py (1)
lifespan(40-60)sync-microservice/app/core/lifespan.py (1)
lifespan(42-109)
sync-microservice/app/routes/health.py (3)
sync-microservice/app/database/folders.py (1)
db_check_database_connection(44-68)sync-microservice/app/utils/watcher.py (1)
watcher_util_is_watcher_running(211-213)sync-microservice/app/schemas/health.py (1)
HealthCheckResponse(5-10)
sync-microservice/app/core/lifespan.py (3)
sync-microservice/app/database/folders.py (1)
db_check_database_connection(44-68)sync-microservice/app/utils/watcher.py (2)
watcher_util_start_folder_watcher(216-271)watcher_util_stop_folder_watcher(274-302)backend/main.py (1)
lifespan(40-60)
backend/app/models/FaceDetector.py (6)
backend/app/models/YOLO.py (2)
YOLO(14-103)close(31-33)backend/app/utils/YOLO.py (1)
YOLO_util_get_model_path(312-353)backend/app/models/FaceNet.py (3)
FaceNet(8-25)get_embedding(16-21)close(23-25)backend/app/utils/FaceNet.py (2)
FaceNet_util_get_model_path(110-117)FaceNet_util_preprocess_image(30-63)backend/app/database/faces.py (1)
db_insert_face_embeddings_by_image_id(90-134)backend/app/models/ObjectClassifier.py (1)
close(24-30)
frontend/src/App.tsx (4)
frontend/src/app/store.ts (1)
RootState(73-73)frontend/src/contexts/ThemeContext.tsx (1)
ThemeProvider(23-64)frontend/src/routes/AppRoutes.tsx (1)
AppRoutes(44-75)frontend/src/components/Loader/GlobalLoader.tsx (1)
GlobalLoader(5-19)
backend/app/routes/images.py (2)
backend/app/database/images.py (1)
db_get_all_images(151-237)backend/app/schemas/images.py (1)
ErrorResponse(47-50)
backend/main.py (2)
backend/app/utils/microservice.py (1)
microservice_util_start_sync_service(13-43)backend/app/custom_logging.py (2)
CustomizeLogger(37-79)make_logger(39-51)
🪛 Biome (2.1.2)
frontend/src/App.tsx
[error] 32-32: Shouldn't redeclare 'useSelector'. Consider to delete it or rename it.
'useSelector' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (18)
app.py (3)
1-12: Clear, helpful module docstring — LGTMAccurately describes entry points and cache semantics.
19-24: Function docstring reads well — LGTMConcise and aligned with behavior.
35-42: Refresh docstring — LGTMGood guidance on when/why selective invalidation happens.
frontend/src/routes/AppRoutes.tsx (1)
1-20: Docs-only change LGTM.Helpful, accurate module and component JSDoc without altering behavior.
Also applies to: 37-43
frontend/src/app/store.ts (1)
73-81: Type exports LGTM.RootState and AppDispatch improve selector/dispatch typing across the app.
sync-microservice/app/core/lifespan.py (2)
1-13: Nice, clear module docs.The responsibilities and phases are well documented and easy to follow.
83-86: Watcher start failures: confirm desired readiness semantics.watcher_util_start_folder_watcher() can return False for “no folders” or “already running.” The app still announces readiness. That’s OK if “no folders” is acceptable; otherwise treat it as “unready.”
Do we want to fail startup when no folders are configured, or is it acceptable to run with watcher_status="stopped"?
sync-microservice/main.py (4)
1-19: Good high-level app docs.
33-39: Lifespan wiring looks correct.
45-49: Router registration is clear and versioned.
45-49: All routers correctly define APIRouter Verified that each module insync-microservice/app/routesexposesrouter = APIRouter(...)as expected.sync-microservice/app/routes/health.py (1)
1-13: Helpful endpoint/module docs.backend/main.py (3)
132-137: LGTM: helpful health endpoint docstring.
39-61: Scope drift vs PR title.This adds lifecycle management, OpenAPI emission, process pool, and microservice startup—beyond “add comments”. Please confirm scope or update PR description.
77-79: Remove logging config verification comment
app/logging_config.jsonis present and includes the requiredloggerfields (path,filename,level,rotation,retention,format).backend/app/routes/images.py (2)
93-115: LGTM: clear mapping to strongly-typed response model.
18-20: Pydantic v2 confirmed—model_dump usage is valid.Requirements pin Pydantic to 2.x (sync-microservice: 2.11.7; backend: 2.7.1), and there are no remaining
.dict()calls.backend/app/utils/FaceNet.py (1)
30-63: Preprocessing doc and steps look good.
| # YOLO models specifically trained for face detection | ||
| # Optimized for detecting human faces in images | ||
| SMALL_FACE_DETECTION_MODEL = f"{MODEL_EXPORTS_PATH}/YOLOv11_Small_Face.onnx" # Fast face detection | ||
| NANO_FACE_DETECTION_MODEL = f"{MODEL_EXPORTS_PATH}/YOLOv11_Nano_Face.onnx" # Fastest face detection | ||
| MEDIUM_FACE_DETECTION_MODEL = f"{MODEL_EXPORTS_PATH}/YOLOv11_Medium_Face.onnx" # Most accurate face detection |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Duplicate face-model constants — remove the second block.
You re-declare SMALL/NANO/MEDIUM_FACE_DETECTION_MODEL above, then again at Lines 27–31. The latter silently overrides the commented ones and invites drift.
Apply this diff to drop the duplicate block:
-# Face Detection Models:
-SMALL_FACE_DETECTION_MODEL = f"{MODEL_EXPORTS_PATH}/YOLOv11_Small_Face.onnx"
-NANO_FACE_DETECTION_MODEL = f"{MODEL_EXPORTS_PATH}/YOLOv11_Nano_Face.onnx"
-MEDIUM_FACE_DETECTION_MODEL = f"{MODEL_EXPORTS_PATH}/YOLOv11_Medium_Face.onnx"📝 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.
| # YOLO models specifically trained for face detection | |
| # Optimized for detecting human faces in images | |
| SMALL_FACE_DETECTION_MODEL = f"{MODEL_EXPORTS_PATH}/YOLOv11_Small_Face.onnx" # Fast face detection | |
| NANO_FACE_DETECTION_MODEL = f"{MODEL_EXPORTS_PATH}/YOLOv11_Nano_Face.onnx" # Fastest face detection | |
| MEDIUM_FACE_DETECTION_MODEL = f"{MODEL_EXPORTS_PATH}/YOLOv11_Medium_Face.onnx" # Most accurate face detection | |
| # YOLO models specifically trained for face detection | |
| # Optimized for detecting human faces in images | |
| SMALL_FACE_DETECTION_MODEL = f"{MODEL_EXPORTS_PATH}/YOLOv11_Small_Face.onnx" # Fast face detection | |
| NANO_FACE_DETECTION_MODEL = f"{MODEL_EXPORTS_PATH}/YOLOv11_Nano_Face.onnx" # Fastest face detection | |
| MEDIUM_FACE_DETECTION_MODEL = f"{MODEL_EXPORTS_PATH}/YOLOv11_Medium_Face.onnx" # Most accurate face detection |
🤖 Prompt for AI Agents
In backend/app/config/settings.py around lines 21–31 there are duplicate
declarations of SMALL_FACE_DETECTION_MODEL, NANO_FACE_DETECTION_MODEL, and
MEDIUM_FACE_DETECTION_MODEL: remove the second block (the re-declaration at
approximately lines 27–31) so the constants are defined only once (keep the
first, commented/annotated block at lines 21–25), ensuring no silent overrides
or drift.
| ImageId = str # Unique identifier for images | ||
| ImagePath = str # File system path to image | ||
| FolderId = str # Unique identifier for folders | ||
| ClassId = int # Object detection class identifier |
There was a problem hiding this comment.
Type mismatch: folder_id should be int (matches DB schema).
You declare FolderId as str, but the schema uses INTEGER and code treats it as int elsewhere. Align types.
Apply:
-ImageId = str # Unique identifier for images
-ImagePath = str # File system path to image
-FolderId = str # Unique identifier for folders
-ClassId = int # Object detection class identifier
+ImageId = str # Unique identifier for images
+ImagePath = str # File system path to image
+FolderId = int # Unique identifier for folders (SQLite INTEGER FK)
+ClassId = int # Object detection class identifier📝 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.
| ImageId = str # Unique identifier for images | |
| ImagePath = str # File system path to image | |
| FolderId = str # Unique identifier for folders | |
| ClassId = int # Object detection class identifier | |
| ImageId = str # Unique identifier for images | |
| ImagePath = str # File system path to image | |
| FolderId = int # Unique identifier for folders (SQLite INTEGER FK) | |
| ClassId = int # Object detection class identifier |
🤖 Prompt for AI Agents
In backend/app/database/images.py around lines 33 to 36, the FolderId type alias
is declared as str but the DB schema and other code expect an int; update the
alias to FolderId = int and adjust any local uses or annotations in this file to
reflect int (and run type-checks/quick search to fix any mismatched annotations
that assumed str).
| def detect_faces(self, image_id: int, image_path: str): | ||
| """ | ||
| Detect faces in an image and generate face embeddings. | ||
|
|
||
| This method performs the complete face detection and recognition pipeline: | ||
| 1. Loads the image using OpenCV | ||
| 2. Uses YOLO to detect face bounding boxes | ||
| 3. Extracts face regions with padding | ||
| 4. Generates face embeddings using FaceNet | ||
| 5. Stores face data in the database | ||
|
|
||
| Args: | ||
| image_id (int): Unique identifier for the image | ||
| image_path (str): Path to the image file | ||
|
|
||
| Returns: | ||
| dict: Dictionary containing detection results with: | ||
| - ids: Class IDs from YOLO detection | ||
| - processed_faces: Preprocessed face images | ||
| - num_faces: Number of faces detected | ||
| None: If image loading fails | ||
| """ |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
image_id type mismatch with DB layer.
DB expects an ImageId (uuid string), but signature/docstring use int. Align types to prevent subtle bugs and schema drift.
Apply this diff to update the API contract:
- def detect_faces(self, image_id: int, image_path: str):
+ def detect_faces(self, image_id: str, image_path: str):
@@
- image_id (int): Unique identifier for the image
+ image_id (str): Unique identifier for the image (uuid)Run to confirm usages:
🏁 Script executed:
#!/bin/bash
# Check DB API and call sites for image_id typing/usage
rg -nP 'db_insert_face_embeddings_by_image_id\s*\(' -C2
rg -nP '\bImageId\b|\bimage_id:\s*(int|str)' -C2Length of output: 5265
Refactor detect_faces signature to use string ImageId
Change image_id type from int to str in backend/app/models/FaceDetector.py detect_faces (line 58) and update its docstring to match the ImageId alias:
- def detect_faces(self, image_id: int, image_path: str):
+ def detect_faces(self, image_id: str, image_path: str):
@@
- image_id (int): Unique identifier for the image
+ image_id (str): Unique identifier for the image (uuid)🤖 Prompt for AI Agents
In backend/app/models/FaceDetector.py around lines 58-79, change the
detect_faces signature from image_id: int to image_id: str (or to the ImageId
alias if your codebase defines one) and update the docstring to mention ImageId
and that it is a string; update the type hint in the signature, replace any
references in the docstring that say int with ImageId/str, ensure you import or
reference the ImageId alias if used, and scan/update local usage and external
callers to accept/convert string IDs accordingly so types remain consistent.
| # Create bounding box dictionary in JSON format for database storage | ||
| bbox = {"x": x1, "y": y1, "width": x2 - x1, "height": y2 - y1} | ||
| bboxes.append(bbox) | ||
| confidences.append(float(score)) | ||
|
|
||
| # Extract face region with padding for better recognition | ||
| padding = 20 | ||
| face_img = img[ | ||
| max(0, y1 - padding) : min(img.shape[0], y2 + padding), | ||
| max(0, x1 - padding) : min(img.shape[1], x2 + padding), | ||
| ] | ||
|
|
||
| # Preprocess face image for FaceNet | ||
| processed_face = FaceNet_util_preprocess_image(face_img) | ||
| processed_faces.append(processed_face) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add guard for empty/invalid crops before preprocessing.
Zero-size crops cause cv2.resize to throw.
face_img = img[
max(0, y1 - padding) : min(img.shape[0], y2 + padding),
max(0, x1 - padding) : min(img.shape[1], x2 + padding),
]
-
+ # Skip invalid crops
+ if face_img.size == 0 or face_img.shape[0] == 0 or face_img.shape[1] == 0:
+ continue
# Preprocess face image for FaceNet
processed_face = FaceNet_util_preprocess_image(face_img)📝 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.
| # Create bounding box dictionary in JSON format for database storage | |
| bbox = {"x": x1, "y": y1, "width": x2 - x1, "height": y2 - y1} | |
| bboxes.append(bbox) | |
| confidences.append(float(score)) | |
| # Extract face region with padding for better recognition | |
| padding = 20 | |
| face_img = img[ | |
| max(0, y1 - padding) : min(img.shape[0], y2 + padding), | |
| max(0, x1 - padding) : min(img.shape[1], x2 + padding), | |
| ] | |
| # Preprocess face image for FaceNet | |
| processed_face = FaceNet_util_preprocess_image(face_img) | |
| processed_faces.append(processed_face) | |
| # Create bounding box dictionary in JSON format for database storage | |
| bbox = {"x": x1, "y": y1, "width": x2 - x1, "height": y2 - y1} | |
| bboxes.append(bbox) | |
| confidences.append(float(score)) | |
| # Extract face region with padding for better recognition | |
| padding = 20 | |
| face_img = img[ | |
| max(0, y1 - padding) : min(img.shape[0], y2 + padding), | |
| max(0, x1 - padding) : min(img.shape[1], x2 + padding), | |
| ] | |
| # Skip invalid crops | |
| if face_img.size == 0 or face_img.shape[0] == 0 or face_img.shape[1] == 0: | |
| continue | |
| # Preprocess face image for FaceNet | |
| processed_face = FaceNet_util_preprocess_image(face_img) | |
| processed_faces.append(processed_face) |
🤖 Prompt for AI Agents
In backend/app/models/FaceDetector.py around lines 101 to 116, the code extracts
a padded face crop then immediately preprocesses it, but zero-size or invalid
crops will cause cv2.resize (inside FaceNet_util_preprocess_image) to throw; add
a guard after extracting face_img to check its height and width are > 0 (e.g.,
face_img.size or face_img.shape) and skip this detection if not valid (append no
processed face / continue), optionally log a debug/warning with bbox info; only
call FaceNet_util_preprocess_image and append to processed_faces when the crop
is non-empty and valid.
| def FaceNet_util_normalize_embedding(embedding): | ||
| """ | ||
| Normalize a face embedding to unit length. | ||
|
|
||
| This function normalizes the face embedding vector to have unit length, | ||
| which is essential for accurate cosine similarity computation. | ||
|
|
||
| Args: | ||
| embedding: Face embedding vector | ||
|
|
||
| Returns: | ||
| Normalized embedding vector with unit length | ||
| """ | ||
| return embedding / np.linalg.norm(embedding) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid division by zero during embedding normalization.
Zero vectors can occur; add epsilon.
-def FaceNet_util_normalize_embedding(embedding):
+def FaceNet_util_normalize_embedding(embedding):
@@
- return embedding / np.linalg.norm(embedding)
+ norm = np.linalg.norm(embedding)
+ if norm == 0:
+ return embedding
+ return embedding / norm📝 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.
| def FaceNet_util_normalize_embedding(embedding): | |
| """ | |
| Normalize a face embedding to unit length. | |
| This function normalizes the face embedding vector to have unit length, | |
| which is essential for accurate cosine similarity computation. | |
| Args: | |
| embedding: Face embedding vector | |
| Returns: | |
| Normalized embedding vector with unit length | |
| """ | |
| return embedding / np.linalg.norm(embedding) | |
| def FaceNet_util_normalize_embedding(embedding): | |
| """ | |
| Normalize a face embedding to unit length. | |
| This function normalizes the face embedding vector to have unit length, | |
| which is essential for accurate cosine similarity computation. | |
| Args: | |
| embedding: Face embedding vector | |
| Returns: | |
| Normalized embedding vector with unit length | |
| """ | |
| norm = np.linalg.norm(embedding) | |
| if norm == 0: | |
| return embedding | |
| return embedding / norm |
🤖 Prompt for AI Agents
In backend/app/utils/FaceNet.py around lines 70 to 84, the normalization divides
by np.linalg.norm(embedding) which can be zero for zero vectors; change to
compute the norm, replace it with a small epsilon when it is zero (e.g. norm =
np.linalg.norm(embedding); norm = max(norm, 1e-10) or use np.maximum for arrays)
and then return embedding / norm so division-by-zero is avoided.
| # Compute IoU (handle division by zero) | ||
| iou = intersection_area / union_area | ||
|
|
||
| return iou |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard IoU division by zero.
Degenerate boxes can make union_area zero, yielding inf/NaN and breaking NMS.
- iou = intersection_area / union_area
+ iou = np.divide(
+ intersection_area,
+ union_area,
+ out=np.zeros_like(union_area, dtype=np.float32),
+ where=union_area > 0,
+ )📝 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.
| # Compute IoU (handle division by zero) | |
| iou = intersection_area / union_area | |
| return iou | |
| # Compute IoU (handle division by zero) | |
| iou = np.divide( | |
| intersection_area, | |
| union_area, | |
| out=np.zeros_like(union_area, dtype=np.float32), | |
| where=union_area > 0, | |
| ) | |
| return iou |
🤖 Prompt for AI Agents
In backend/app/utils/YOLO.py around lines 212 to 215, the IoU computation
divides by union_area which can be zero for degenerate boxes; change the
calculation to guard against division by zero by checking if union_area is zero
(or below a small epsilon) and return 0.0 in that case, otherwise compute
intersection_area / union_area; ensure you use a small epsilon (e.g., 1e-6) or
an explicit conditional to avoid inf/NaN and keep the return type as a float.
| CORSMiddleware, | ||
| allow_origins=["*"], # Allows all origins | ||
| allow_credentials=True, | ||
| allow_methods=["*"], | ||
| allow_headers=["*"], | ||
| allow_methods=["*"], # Allow all HTTP methods | ||
| allow_headers=["*"], # Allow all headers | ||
| ) |
There was a problem hiding this comment.
CORS: credentials with wildcard origins is insecure.
Browsers reject this per spec; also unsafe. Either disable credentials with "*" or specify explicit origins.
Quick safe fix:
- allow_origins=["*"], # Allows all origins
- allow_credentials=True,
+ allow_origins=["*"], # Allows all origins
+ allow_credentials=False, # credentials not allowed with wildcard originsOptionally, read allowed origins from env and keep credentials enabled.
📝 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.
| CORSMiddleware, | |
| allow_origins=["*"], # Allows all origins | |
| allow_credentials=True, | |
| allow_methods=["*"], | |
| allow_headers=["*"], | |
| allow_methods=["*"], # Allow all HTTP methods | |
| allow_headers=["*"], # Allow all headers | |
| ) | |
| CORSMiddleware, | |
| allow_origins=["*"], # Allows all origins | |
| allow_credentials=False, # credentials not allowed with wildcard origins | |
| allow_methods=["*"], # Allow all HTTP methods | |
| allow_headers=["*"], # Allow all headers | |
| ) |
🤖 Prompt for AI Agents
In backend/main.py around lines 121-126 the CORS config uses allow_origins=["*"]
while allow_credentials=True which is invalid/insecure; fix by either (A) set
allow_credentials=False when using "*" OR (B) replace the wildcard with an
explicit list of allowed origins (e.g., read a comma-separated ALLOWED_ORIGINS
from env, split into a list) and keep allow_credentials=True; implement the
chosen option and ensure the middleware uses that boolean/list accordingly.
| import { BrowserRouter } from 'react-router'; | ||
| import { useSelector } from 'react-redux'; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Quick verification script: ensure no other wrong BrowserRouter imports.
Run to find mis-imports across the repo.
Also applies to: 32-32
🏁 Script executed:
#!/bin/bash
# Find BrowserRouter imported from 'react-router' (should be none)
rg -nP "import\\s*\\{[^}]*BrowserRouter[^}]*\\}\\s*from\\s*['\"]react-router['\"]" -C2
# (Optional) Normalize Routes/Route to react-router-dom for consistency
rg -nP "import\\s*\\{[^}]*\\b(Routes|Route)\\b[^}]*\\}\\s*from\\s*['\"]react-router['\"]" -C2Length of output: 1237
Correct import targets from react-router to react-router-dom
Update all instances of:
import { BrowserRouter } from 'react-router';infrontend/src/App.tsx(line 23) andfrontend/src/pages/__tests__/allPages.test.tsx(line 10)import { Routes, Route } from 'react-router';infrontend/src/routes/AppRoutes.tsx(line 24)
to import from'react-router-dom'.
🤖 Prompt for AI Agents
In frontend/src/App.tsx around lines 23-24, the BrowserRouter import is
incorrectly sourced from 'react-router'; change it to import BrowserRouter from
'react-router-dom'. Also update the same incorrect BrowserRouter import in
frontend/src/pages/__tests__/allPages.test.tsx (line ~10) and change the Routes
and Route import in frontend/src/routes/AppRoutes.tsx (around line 24) to import
{ Routes, Route } from 'react-router-dom'. Ensure the named imports match their
exported names from 'react-router-dom' and run tests/build to verify no
remaining import errors.
|
|
||
| // React imports | ||
| import React from 'react'; | ||
| import { BrowserRouter } from 'react-router'; |
There was a problem hiding this comment.
Fix BrowserRouter import source.
BrowserRouter is exported from react-router-dom, not react-router.
-import { BrowserRouter } from 'react-router';
+import { BrowserRouter } from 'react-router-dom';📝 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.
| import { BrowserRouter } from 'react-router'; | |
| // frontend/src/App.tsx | |
| import { BrowserRouter } from 'react-router-dom'; |
🤖 Prompt for AI Agents
In frontend/src/App.tsx around line 23, the code imports BrowserRouter from
'react-router' but BrowserRouter is exported from 'react-router-dom'; change the
import source to 'react-router-dom' (i.e., replace the current import statement
to import BrowserRouter from 'react-router-dom') so the component resolves
correctly at runtime.
| import { useSelector } from 'react-redux'; | ||
|
|
There was a problem hiding this comment.
Remove duplicate useSelector import.
Redefinition causes a compile-time error (noRedeclare).
import { useSelector } from 'react-redux';
@@
-import { useSelector } from 'react-redux';Also applies to: 32-32
🤖 Prompt for AI Agents
In frontend/src/App.tsx around lines 24-25 (and also line 32), there's a
duplicate import of useSelector from 'react-redux' causing a noRedeclare compile
error; remove the redundant import so useSelector is imported only once (keep
the correctly placed import and delete the other duplicate line) and ensure any
references to useSelector still resolve to the remaining import.
rahulharpal1603
force-pushed
the
main
branch
from
f589c41 to
00d7a55
Compare
4 participants
added comments throughout the codebase to make it more understandable
Summary by CodeRabbit
New Features
Improvements
Documentation