Conversation
This reverts commit 52a8a57. good
…og after selecting a file
|
|
WalkthroughReplaces the legacy image-tagging flow with a unified face-search pipeline supporting path and base64 inputs, centralizes detection/embedding/matching in a new utility, updates API schemas/endpoints, adds webcam capture in the frontend, and adds macOS camera entitlements and CSP updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as FaceSearchDialog / WebcamComponent
participant API as Backend API (/face-clusters/face-search)
participant FS as perform_face_search()
User->>UI: Open dialog
alt Upload file (path)
User->>UI: Select file
UI->>API: POST /face-clusters/face-search?input_type=path (body: path)
API->>FS: validate path -> detect -> embed -> compare
else Capture photo (webcam)
User->>UI: Open webcam & capture
UI->>API: POST /face-clusters/face-search?input_type=base64 (body: base64_data)
API->>API: decode base64 -> write temp file -> validate
API->>FS: detect -> embed -> compare
end
FS->>FS: compute embeddings, cosine similarity, filter by threshold
FS-->>API: matches / message
API-->>UI: return results or error
UI-->>User: display matches or "no matches"
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
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. 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/backend/backend_python/openapi.json (1)
1069-1076: Specify the 200 response schema.
Expose GetAllImagesResponse for clients; today it’s {}.Prefer to generate via FastAPI by adding response_model on the route (see route diff in backend/app/routes/face_clusters.py).
backend/app/routes/face_clusters.py (1)
208-211: Add response_model for typed 200s (improves OpenAPI).
Aligns docs with actual return.-@router.post( - "/face-search", - responses={code: {"model": ErrorResponse} for code in [400, 500]}, -) +from app.schemas.images import GetAllImagesResponse +@router.post( + "/face-search", + response_model=GetAllImagesResponse, + responses={code: {"model": ErrorResponse} for code in [400, 500]}, +)
🧹 Nitpick comments (12)
frontend/src-tauri/Info.plist (1)
5-6: Clarify camera usage description for users.The description mentions "WebRTC" which may be unclear to end users who see this in the permission dialog. Consider a more user-friendly explanation that describes the feature benefit.
Apply this diff to improve the user-facing message:
<key>NSCameraUsageDescription</key> - <string>Request camera access for WebRTC</string> + <string>PictoPy needs camera access to capture photos for face search</string>frontend/src/api/apiEndpoints.ts (1)
7-8: Unify endpoints with a param builder to avoid duplication.
A single factory keeps callers consistent and simplifies future query params.- searchForFaces: '/face-clusters/face-search?input_type=path', - searchForFacesBase64: '/face-clusters/face-search?input_type=base64', + searchForFaces: (type: 'path' | 'base64' = 'path') => + `/face-clusters/face-search?input_type=${type}`,frontend/src/components/Dialog/FaceSearchDialog.tsx (3)
62-79: Add feature detection and clearer permission error.
Guard for missing mediaDevices; keep dialog open on failure.const handleWebCam = async () => { - try { - const stream = await navigator.mediaDevices.getUserMedia({ video: true }); + try { + if (!('mediaDevices' in navigator) || !navigator.mediaDevices?.getUserMedia) { + throw new Error('MediaDevices API not available'); + } + const stream = await navigator.mediaDevices.getUserMedia({ video: true }); stream.getTracks().forEach((track) => track.stop()); navigate(`/${ROUTES.HOME}`); setIsDialogOpen(false); setShowCamera(true); } catch (error) { dispatch( showInfoDialog({ - title: 'Webcam Not Supported', - message: - 'Webcam is not supported or access was denied on this device.', + title: 'Webcam Not Available', + message: 'Camera access is unavailable or was denied.', variant: 'error', }), ); } };
81-89: Navigate only after a file is chosen.
Avoid unintended page change when user cancels.- navigate(`/${ROUTES.HOME}`); const filePath = await pickSingleFile(); if (filePath) { + navigate(`/${ROUTES.HOME}`); setIsDialogOpen(false); dispatch(startSearch(filePath)); dispatch(showLoader('Searching faces...')); getSearchImages(filePath); }
116-142: Remove redundant disabled={false}.
Keeps JSX terse; defaults to enabled.- <Button - onClick={handlePickFile} - disabled={false} + <Button + onClick={handlePickFile}(Same for the Webcam button.)
frontend/src/components/WebCam/WebCamComponent.tsx (2)
40-65: Avoid double notifications; let onSuccess decide.
Prevent a “Success” dialog followed by “No Match Found”.-useMutationFeedback(getSearchImagesBase64, { - showLoading: true, - loadingMessage: 'Searching for images...', - successTitle: 'Search Complete', - successMessage: 'Images matching your search have been found.', - errorTitle: 'Search Error', - errorMessage: 'Failed to search images. Please try again.', - onSuccess: () => { +useMutationFeedback(getSearchImagesBase64, { + showLoading: true, + loadingMessage: 'Searching for images...', + showSuccess: false, + errorTitle: 'Search Error', + errorMessage: 'Failed to search images. Please try again.', + onSuccess: () => { const result = getSearchImagesBase64.data?.data as Image[]; if (result && result.length > 0) { dispatch(setResults(result)); + dispatch( + showInfoDialog({ + title: 'Search Complete', + message: `Found ${result.length} matching image(s).`, + variant: 'info', + }), + ); } else { dispatch( showInfoDialog({ title: 'No Match Found', message: 'We couldn’t find any matching faces in your gallery for this photo.', variant: 'info', }), ); dispatch(setResults([])); dispatch(clearSearch()); } getSearchImagesBase64.reset(); }, });
125-131: Reduce payload size from the webcam.
Set JPEG quality to control base64 size and speed up upload.- <Webcam + <Webcam audio={false} ref={webcamRef} - screenshotFormat="image/jpeg" + screenshotFormat="image/jpeg" + screenshotQuality={0.9} videoConstraints={videoConstraints} className="w-full max-w-md rounded-lg border" />backend/app/utils/faceSearch.py (2)
45-69: Avoid double embedding computation.
FaceDetector already initializes FaceNet and computes embeddings; consider returning embeddings from detect_faces (when forSearch=True) to eliminate the extra FaceNet session here.
78-83: Rename CONFIDENCE_PERCENT for clarity—currently misleading naming.The constant is defined as
0.6inbackend/app/config/settings.py(using [0,1] scale), which correctly matches cosine similarity ranges. However, the name "CONFIDENCE_PERCENT" suggests a [0,100] scale. Rename toCONFIDENCE_THRESHOLDorSIMILARITY_THRESHOLDin settings and all usages.docs/backend/backend_python/openapi.json (2)
1045-1056: Drop duplicate descriptions on parameter.
In some tools, $ref + sibling schema keys cause issues. Keep description at the parameter level.
1516-1543: Capture conditional requirements.
Consider oneOf to express: when input_type=path require path; when input_type=base64 require base64_data.backend/app/routes/face_clusters.py (1)
277-284: Consider allowing HEIC/HEIF if needed.
iOS captures may be HEIC. If supported by your pipeline, add to allowed list.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
backend/app/routes/face_clusters.py(3 hunks)backend/app/schemas/images.py(1 hunks)backend/app/utils/faceSearch.py(1 hunks)docs/backend/backend_python/openapi.json(4 hunks)frontend/package.json(1 hunks)frontend/src-tauri/Entitlements.plist(1 hunks)frontend/src-tauri/Info.plist(1 hunks)frontend/src-tauri/tauri.conf.json(2 hunks)frontend/src/api/api-functions/face_clusters.ts(2 hunks)frontend/src/api/apiEndpoints.ts(1 hunks)frontend/src/components/Dialog/FaceSearchDialog.tsx(3 hunks)frontend/src/components/Navigation/Navbar/Navbar.tsx(1 hunks)frontend/src/components/WebCam/WebCamComponent.tsx(1 hunks)package.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
frontend/src/api/api-functions/face_clusters.ts (3)
frontend/src/types/API.ts (1)
APIResponse(1-8)frontend/src/api/axiosConfig.ts (1)
apiClient(5-12)frontend/src/api/apiEndpoints.ts (1)
faceClustersEndpoints(5-11)
frontend/src/components/WebCam/WebCamComponent.tsx (6)
frontend/src/hooks/useQueryExtension.ts (1)
usePictoMutation(26-78)frontend/src/api/api-functions/face_clusters.ts (1)
fetchSearchedFacesBase64(58-66)frontend/src/hooks/useMutationFeedback.tsx (1)
useMutationFeedback(60-135)frontend/src/types/Media.ts (1)
Image(13-22)frontend/src/features/searchSlice.ts (3)
setResults(24-26)clearSearch(27-31)startSearch(20-23)frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(16-30)
backend/app/routes/face_clusters.py (2)
backend/app/schemas/images.py (3)
FaceSearchRequest(11-13)InputType(6-8)ErrorResponse(59-62)backend/app/utils/faceSearch.py (1)
perform_face_search(35-106)
backend/app/utils/faceSearch.py (4)
backend/app/database/faces.py (1)
get_all_face_embeddings(144-216)backend/app/models/FaceDetector.py (2)
FaceDetector(15-79)detect_faces(26-67)backend/app/models/FaceNet.py (2)
FaceNet(12-29)get_embedding(20-25)backend/app/utils/FaceNet.py (1)
FaceNet_util_cosine_similarity(20-23)
frontend/src/components/Dialog/FaceSearchDialog.tsx (4)
frontend/src/constants/routes.ts (1)
ROUTES(1-11)frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(16-30)frontend/src/features/searchSlice.ts (1)
startSearch(20-23)frontend/src/features/loaderSlice.ts (1)
showLoader(17-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Tauri Build Check (windows-latest)
- GitHub Check: Backend Tests
- GitHub Check: Tauri Build Check (macos-latest, --target aarch64-apple-darwin)
- GitHub Check: Tauri Build Check (ubuntu-22.04)
🔇 Additional comments (8)
package.json (1)
20-20: LGTM!Standard formatting improvement - adds trailing newline per convention.
frontend/src/components/Navigation/Navbar/Navbar.tsx (1)
36-40: LGTM!The conditional logic correctly handles both data URLs (from webcam/base64) and file paths. Optional chaining safely handles undefined values.
frontend/src-tauri/tauri.conf.json (2)
29-30: LGTM!The macOS configuration correctly references the entitlements file and uses ad-hoc signing (appropriate for development builds).
66-66: LGTM!The CSP correctly adds
media-srcwith required sources for webcam access and includesipc:for Tauri communication. The directive syntax is correct.backend/app/schemas/images.py (1)
6-8: LGTM!The
InputTypeenum clearly defines the two supported input modalities for face search.frontend/src/api/api-functions/face_clusters.ts (2)
18-20: LGTM!The request type follows the established pattern and correctly defines the base64 input structure.
58-66: LGTM!The function implementation follows the established pattern, correctly posts to the base64 endpoint, and maintains type safety.
frontend/package.json (1)
64-64: Test react-webcam functionality with React 19 in your application.react-webcam@7.2.0 declares peerDependencies "react": ">=16.2.0" and "react-dom": ">=16.2.0", so React 19 satisfies the requirement. However, the package author may not have explicitly tested on React 19, so verify that the webcam functionality works as expected in your app before merging.
| MAX_B64_LEN = 14_000_000 # 10MB | ||
| if len(base64_data) > MAX_B64_LEN: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=ErrorResponse( | ||
| success=False, | ||
| error="Payload too large", | ||
| message="Base64 image exceeds maximum allowed size.", | ||
| ).model_dump(), | ||
| ) | ||
| try: | ||
| image_bytes = base64.b64decode(base64_data.split(",")[-1]) | ||
| except (Base64Error, ValueError): |
There was a problem hiding this comment.
Decode first, enforce byte-size, and use a secure temp file.
Current length check is on the base64 string (imprecise), and writes to a predictable path. Decode, validate bytes, respond with 413, and use tempfile.NamedTemporaryFile.
- MAX_B64_LEN = 14_000_000 # 10MB
- if len(base64_data) > MAX_B64_LEN:
- raise HTTPException(
- status_code=status.HTTP_400_BAD_REQUEST,
- detail=ErrorResponse(
- success=False,
- error="Payload too large",
- message="Base64 image exceeds maximum allowed size.",
- ).model_dump(),
- )
- try:
- image_bytes = base64.b64decode(base64_data.split(",")[-1])
+ try:
+ # Decode first; then validate exact byte size.
+ image_bytes = base64.b64decode(base64_data.split(",")[-1])
except (Base64Error, ValueError):
raise HTTPException(
status_code=status.HTTP_400_BAD_REQUEST,
detail=ErrorResponse(
success=False,
error="Invalid base64 data",
message="The provided base64 image data is malformed or invalid.",
).model_dump(),
)
+ MAX_IMAGE_BYTES = 10 * 1024 * 1024 # 10 MiB
+ if len(image_bytes) > MAX_IMAGE_BYTES:
+ raise HTTPException(
+ status_code=status.HTTP_413_REQUEST_ENTITY_TOO_LARGE,
+ detail=ErrorResponse(
+ success=False,
+ error="Payload too large",
+ message="Image exceeds maximum allowed size (10 MiB).",
+ ).model_dump(),
+ )
@@
- image_id = str(uuid.uuid4())[:8]
- temp_dir = "temp_uploads"
- os.makedirs(temp_dir, exist_ok=True)
- local_image_path = os.path.join(temp_dir, f"{image_id}.{extension}")
-
- with open(local_image_path, "wb") as f:
- f.write(image_bytes)
+ import tempfile
+ with tempfile.NamedTemporaryFile(
+ prefix="pictopy_", suffix=f".{extension}", delete=False
+ ) as tmp:
+ tmp.write(image_bytes)
+ local_image_path = tmp.nameAlso applies to: 277-294
🤖 Prompt for AI Agents
In backend/app/routes/face_clusters.py around lines 255-267 (also apply same
changes to 277-294): the current logic checks the length of the base64 string
and writes to a predictable path; instead decode the base64 first, then enforce
a byte-size limit (respond with HTTP 413 Payload Too Large), and only after
validating size write to a secure temporary file using
tempfile.NamedTemporaryFile (delete=False if needed) to avoid predictable paths;
also catch decoding errors and return a 400 with a clear error payload as
before.
| class FaceSearchRequest(BaseModel): | ||
| path: Optional[str] = None | ||
| base64_data: Optional[str] = None |
There was a problem hiding this comment.
Add validation to ensure at least one input field is provided.
Both path and base64_data are optional with no constraint enforcement. The API could receive requests with both fields set to None, leading to errors downstream.
Apply this diff to add validation using Pydantic's model validator:
+from pydantic import model_validator
+
class FaceSearchRequest(BaseModel):
path: Optional[str] = None
base64_data: Optional[str] = None
+
+ @model_validator(mode='after')
+ def check_at_least_one_field(self):
+ if not self.path and not self.base64_data:
+ raise ValueError("Either 'path' or 'base64_data' must be provided")
+ if self.path and self.base64_data:
+ raise ValueError("Only one of 'path' or 'base64_data' should be provided")
+ return self📝 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.
| class FaceSearchRequest(BaseModel): | |
| path: Optional[str] = None | |
| base64_data: Optional[str] = None | |
| from pydantic import model_validator | |
| class FaceSearchRequest(BaseModel): | |
| path: Optional[str] = None | |
| base64_data: Optional[str] = None | |
| @model_validator(mode='after') | |
| def check_at_least_one_field(self): | |
| if not self.path and not self.base64_data: | |
| raise ValueError("Either 'path' or 'base64_data' must be provided") | |
| if self.path and self.base64_data: | |
| raise ValueError("Only one of 'path' or 'base64_data' should be provided") | |
| return self |
🤖 Prompt for AI Agents
In backend/app/schemas/images.py around lines 11 to 13, the FaceSearchRequest
model declares path and base64_data as Optional but does not validate that at
least one is provided; add a Pydantic root validator (or @root_validator) to
check that either path or base64_data is not None/empty and raise a ValueError
with a clear message if both are missing/empty so the API rejects invalid
requests early.
| class BoundingBox(BaseModel): | ||
| x: float | ||
| y: float | ||
| width: float | ||
| height: float | ||
|
|
||
|
|
||
| class ImageData(BaseModel): | ||
| id: str | ||
| path: str | ||
| folder_id: str | ||
| thumbnailPath: str | ||
| metadata: Dict[str, Any] | ||
| isTagged: bool | ||
| tags: Optional[List[str]] = None | ||
| bboxes: BoundingBox | ||
|
|
||
|
|
||
| class GetAllImagesResponse(BaseModel): | ||
| success: bool | ||
| message: str | ||
| data: List[ImageData] | ||
|
|
There was a problem hiding this comment.
Fix schema mismatch and avoid duplicate models.
Local ImageData defines bboxes: BoundingBox (singular) while DB returns a list; also duplicates schemas already in app.schemas.images.
-from pydantic import BaseModel
-from app.config.settings import CONFIDENCE_PERCENT, DEFAULT_FACENET_MODEL
+from app.config.settings import CONFIDENCE_PERCENT, DEFAULT_FACENET_MODEL
+from app.schemas.images import ImageData, GetAllImagesResponse
@@
-class BoundingBox(BaseModel):
- x: float
- y: float
- width: float
- height: float
-
-
-class ImageData(BaseModel):
- id: str
- path: str
- folder_id: str
- thumbnailPath: str
- metadata: Dict[str, Any]
- isTagged: bool
- tags: Optional[List[str]] = None
- bboxes: BoundingBox
-
-
-class GetAllImagesResponse(BaseModel):
- success: bool
- message: str
- data: List[ImageData]
+# Reuse shared schemas to prevent drift with OpenAPI.
@@
matches.append(
ImageData(
id=image["id"],
path=image["path"],
folder_id=image["folder_id"],
thumbnailPath=image["thumbnailPath"],
metadata=image["metadata"],
isTagged=image["isTagged"],
tags=image["tags"],
- bboxes=image["bbox"],
+ # Note: bbox/bboxes omitted here to match ImageData schema.
)
)Follow-up: If bbox must be returned, extend the shared ImageData schema and OpenAPI accordingly instead of redefining models locally. Based on learnings
Also applies to: 18-27, 29-33, 84-93
🤖 Prompt for AI Agents
backend/app/utils/faceSearch.py lines ~11-33 (and also affecting ranges 18-27,
29-33, 84-93): the local ImageData and BoundingBox schemas are incorrect and
duplicate definitions that already exist in app.schemas.images; change bboxes
from a single BoundingBox to a List[BoundingBox], remove the duplicated model
definitions, and import the shared BoundingBox and ImageData schemas from
app.schemas.images instead; if you need to include thumbnailPath, metadata or
isTagged fields not present in the shared schema, extend the imported ImageData
(or create a subclass) so the OpenAPI schema reflects the additional fields, and
update GetAllImagesResponse to use the shared/extended ImageData type. Ensure
typing and imports are adjusted and that the OpenAPI exposure uses the
shared/extended model rather than locally redefining it.
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> | ||
| <plist version="1.0"> | ||
| <dict> | ||
| <key>com.apple.security.app-sandbox</key> | ||
| <true/> | ||
| </dict> | ||
| </plist> |
There was a problem hiding this comment.
Add camera entitlement for sandboxed macOS app.
The App Sandbox is enabled, but the camera-specific entitlement is missing. For webcam functionality to work in a sandboxed macOS app, you need to explicitly grant camera access.
Apply this diff to add the required camera entitlement:
<dict>
<key>com.apple.security.app-sandbox</key>
<true/>
+ <key>com.apple.security.device.camera</key>
+ <true/>
</dict>📝 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.
| <?xml version="1.0" encoding="UTF-8"?> | |
| <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> | |
| <plist version="1.0"> | |
| <dict> | |
| <key>com.apple.security.app-sandbox</key> | |
| <true/> | |
| </dict> | |
| </plist> | |
| <?xml version="1.0" encoding="UTF-8"?> | |
| <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> | |
| <plist version="1.0"> | |
| <dict> | |
| <key>com.apple.security.app-sandbox</key> | |
| <true/> | |
| <key>com.apple.security.device.camera</key> | |
| <true/> | |
| </dict> | |
| </plist> |
🤖 Prompt for AI Agents
In frontend/src-tauri/Entitlements.plist around lines 1 to 8, the App Sandbox is
enabled but the camera-specific entitlement is missing; add the
com.apple.security.device.camera key set to true in the entitlements plist to
grant webcam access for the sandboxed macOS app, and ensure you also add a
user-facing NSCameraUsageDescription string in Info.plist (if not already
present) so the app can request camera permission at runtime.
| @@ -19,8 +19,11 @@ import { fetchSearchedFaces } from '@/api/api-functions'; | |||
| import { showInfoDialog } from '@/features/infoDialogSlice'; | |||
| import { useNavigate } from 'react-router'; | |||
| import { ROUTES } from '@/constants/routes'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
Verify import source for useNavigate.
Many setups use react-router-dom; ensure this matches your project.
🏁 Script executed:
#!/bin/bash
rg -nC2 -P "import\\s+\\{\\s*useNavigate\\s*\\}\\s+from\\s+'react-router[^']*';" --glob "frontend/**"Length of output: 1424
---
🌐 Web query:
```web
React Router useNavigate import source react-router-dom correct package
💡 Result:
Use this import:
import { useNavigate } from 'react-router-dom';
React Router v6 adds the useNavigate hook and react-router-dom re-exports it (ensure you have react-router-dom v6+ and that your component is inside a ). [1][2]
Fix incorrect import source for useNavigate.
The import should be from 'react-router-dom', not 'react-router'. React Router v6+ requires useNavigate to be imported from the web-specific package (react-router-dom), which re-exports the hook. Update line 20 to:
import { useNavigate } from 'react-router-dom';This same issue affects frontend/src/pages/InitialSteps/InitialSteps.tsx (line 2) and frontend/src/components/FaceCollections.tsx (line 3).
🤖 Prompt for AI Agents
In frontend/src/components/Dialog/FaceSearchDialog.tsx around line 21 (and
similarly in frontend/src/pages/InitialSteps/InitialSteps.tsx line 2 and
frontend/src/components/FaceCollections.tsx line 3), the useNavigate hook is
incorrectly imported from 'react-router'; change the import source to
'react-router-dom' so useNavigate is imported from the web-specific package
(replace the module string only, keeping the named import).
| @@ -0,0 +1,176 @@ | |||
| import { useState, useRef, useCallback } from 'react'; | |||
| import { useMutationFeedback } from '../../hooks/useMutationFeedback.tsx'; | |||
There was a problem hiding this comment.
Fix import path (drop extension; use alias).
The .tsx extension and relative path can break builds and inconsistently bypass tsconfig paths.
-import { useMutationFeedback } from '../../hooks/useMutationFeedback.tsx';
+import { useMutationFeedback } from '@/hooks/useMutationFeedback';📝 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 { useMutationFeedback } from '../../hooks/useMutationFeedback.tsx'; | |
| import { useMutationFeedback } from '@/hooks/useMutationFeedback'; |
🤖 Prompt for AI Agents
In frontend/src/components/WebCam/WebCamComponent.tsx around line 2, the import
currently uses a relative path with a .tsx extension which can break builds and
bypass tsconfig path aliases; update the import to remove the .tsx extension and
use the project alias (e.g. replace "../../hooks/useMutationFeedback.tsx" with
the configured alias path to useMutationFeedback, ensuring the import relies on
tsconfig/webpack aliases and not a relative file-extension import).
| const capture = useCallback(() => { | ||
| if (webcamRef.current) { | ||
| const imageSrc = webcamRef.current.getScreenshot(); | ||
| setCapturedImageUrl(imageSrc); | ||
| setShowCamera(false); | ||
| } | ||
| }, [webcamRef]); | ||
|
|
There was a problem hiding this comment.
Handle null from getScreenshot to avoid a blank dialog.
When the camera isn’t ready, getScreenshot() can return null; UI currently becomes empty.
-const capture = useCallback(() => {
- if (webcamRef.current) {
- const imageSrc = webcamRef.current.getScreenshot();
- setCapturedImageUrl(imageSrc);
- setShowCamera(false);
- }
-}, [webcamRef]);
+const capture = useCallback(() => {
+ if (!webcamRef.current) return;
+ const imageSrc = webcamRef.current.getScreenshot();
+ if (!imageSrc) {
+ dispatch(
+ showInfoDialog({
+ title: 'Camera Not Ready',
+ message: 'Please wait for the camera to initialize, then try again.',
+ variant: 'error',
+ }),
+ );
+ return;
+ }
+ setCapturedImageUrl(imageSrc);
+ setShowCamera(false);
+}, [dispatch]);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/components/WebCam/WebCamComponent.tsx around lines 67 to 74,
getScreenshot() can return null when the camera isn’t ready; update the capture
callback to check the returned imageSrc for null before calling
setCapturedImageUrl and setShowCamera(false). If imageSrc is null, do not close
the camera or set the captured URL; instead handle the failure path (e.g., keep
camera open, optionally show a user-facing error/toast or retry) so the dialog
does not become blank.
|
|
…and settled callbacks
|
|
1 similar comment
|
|
|
|
1 similar comment
|
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/src/components/Dialog/FaceSearchDialog.tsx (1)
20-20: Fix incorrect import source for useNavigate (previously flagged).As noted in the previous review, the import should be from
'react-router-dom', not'react-router'. For React Router v6+ web applications,useNavigatemust be imported from the web-specific package.Apply this diff:
-import { useNavigate } from 'react-router'; +import { useNavigate } from 'react-router-dom';
🧹 Nitpick comments (2)
frontend/src/components/Dialog/FaceSearchDialog.tsx (2)
62-79: Verify the navigation and state update timing.The function navigates to home (line 66) before showing the camera component (line 68). Since state updates are asynchronous, this sequencing could lead to unexpected UI behavior if the navigation interrupts the camera initialization. Consider whether navigation should occur after the camera permission check or if the current flow is intentional for UX reasons.
Optionally, you can provide more specific error feedback by checking the error type:
} catch (error) { + const errorName = error instanceof DOMException ? error.name : 'Unknown'; + let message = 'Webcam is not supported or access was denied on this device.'; + + if (errorName === 'NotAllowedError') { + message = 'Camera access was denied. Please enable camera permissions in your browser settings.'; + } else if (errorName === 'NotFoundError') { + message = 'No camera device was found on this device.'; + } + dispatch( showInfoDialog({ title: 'Webcam Not Supported', - message: - 'Webcam is not supported or access was denied on this device.', + message, variant: 'error', }), );
94-103: Remove redundant props for cleaner code.Several props can be removed as they're either redundant or handled by the parent component:
- Line 95: The
onClickhandler is unnecessary sinceDialogTriggerautomatically handles opening the dialog.- Lines 118 and 132:
disabled={false}is the default value and can be omitted.Apply this diff:
<DialogTrigger asChild> <Button - onClick={() => setIsDialogOpen(true)} variant="ghost" size="icon" className="h-8 w-8 cursor-pointer p-1" > <ScanFace className="h-4 w-4" /> <span className="sr-only">Face Detection Search</span> </Button> </DialogTrigger> <DialogContent className="sm:max-w-md"> <DialogHeader> <DialogTitle>Face Detection Search</DialogTitle> <DialogDescription> Search for images containing specific faces by uploading a photo or using your webcam. </DialogDescription> </DialogHeader> <div className="grid grid-cols-2 gap-4 py-4"> {/* Upload Button */} <Button onClick={handlePickFile} - disabled={false} className="flex h-32 cursor-pointer flex-col items-center justify-center gap-2 p-0" variant="outline" > <ScanFace className="text-muted-foreground mb-1 h-8 w-8" /> <span className="text-sm font-medium">Upload Photo</span> <span className="text-muted-foreground text-center text-xs"> Select a file from your device </span> </Button> {/* Webcam Button */} <Button onClick={handleWebCam} - disabled={false} className="flex h-32 cursor-pointer flex-col items-center justify-center gap-2 p-0" variant="outline" > <Camera className="text-muted-foreground mb-1 h-8 w-8" /> <span className="text-sm font-medium">Use Webcam</span> <span className="text-muted-foreground text-center text-xs"> Capture with camera </span> </Button> </div> </DialogContent>Also applies to: 116-127, 130-141
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/Dialog/FaceSearchDialog.tsx(3 hunks)frontend/src/components/Navigation/Navbar/Navbar.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/Dialog/FaceSearchDialog.tsx (4)
frontend/src/constants/routes.ts (1)
ROUTES(1-11)frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(16-30)frontend/src/features/searchSlice.ts (1)
startSearch(20-23)frontend/src/features/loaderSlice.ts (1)
showLoader(17-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Tauri Build Check (windows-latest)
- GitHub Check: Tauri Build Check (ubuntu-22.04)
- GitHub Check: Backend Tests
🔇 Additional comments (4)
frontend/src/components/Navigation/Navbar/Navbar.tsx (1)
36-40: LGTM! Correctly handles both data URLs and file paths.The conditional logic appropriately distinguishes between base64-encoded data URLs (from webcam captures) and file system paths, ensuring proper image rendering for both input sources.
frontend/src/components/Dialog/FaceSearchDialog.tsx (3)
2-2: LGTM! Import changes align with new webcam feature.The updated imports correctly reflect the new UI elements (Camera, ScanFace icons) and webcam functionality (WebcamComponent).
Also applies to: 22-22
26-26: LGTM! State management for webcam component is correct.
80-89: LGTM! File picker flow improvements enhance UX.The added navigation and dialog closing after file selection create a cleaner user experience during the face search workflow.
|
|
1 similar comment
|
|
2 participants
Issue - #502
Added a backend API route to handle face searches using Base64-encoded image data.
Integrated frontend support for capturing images directly from the webcam and sending them to the backend for processing.
Video Ref :- https://drive.google.com/file/d/1hjGRv23Jhul-N_f9bcG4MfUmCyuWt3kT/view?usp=sharing
Summary by CodeRabbit
New Features
Face search accepts either file path or base64 image input; base64 searches (including webcam captures) supported.
Added a webcam capture component and a base64-based face-search API call.
User Experience
Face search dialog redesigned with "Upload Photo" and "Use Webcam", including capture review and retake.
Image rendering now correctly handles data URL images.
Security & Configuration
macOS camera entitlement and tightened content-security policy for camera/media access.
Summary by CodeRabbit
New Features
User Experience
Documentation / Permissions