Feature : Added text-based search feature #667
Conversation
WalkthroughAdds end-to-end text-based image search: backend DB search function and GET /images/search API, frontend API + hook, Redux slice split for text/face searches, Navbar debounced text input, and Home integration to display search results. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Navbar as Navbar UI
participant Redux as Redux Store
participant Hook as useImageSearch
participant API as Frontend API Module
participant Backend as Backend API
participant DB as Database
participant Home as Home Component
User->>Navbar: Type query
Navbar->>Navbar: Debounce 500ms
Navbar->>Redux: dispatch startTextSearch(query)
Redux-->>Navbar: update searchState(type='text', query)
Home->>Hook: useImageSearch(query, enabled=true)
Hook->>API: searchImages(query)
API->>Backend: GET /images/search?query=...
Backend->>DB: db_search_images(query, tagged?)
DB-->>Backend: aggregated image list
Backend-->>API: 200 with images
API-->>Hook: return results
Hook-->>Home: searchData available
Home->>Redux: dispatch setImages(searchData)
Redux-->>Home: images state updated
Home-->>User: Render "Search Results for 'query'"
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related issues
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)
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/components/Navigation/Navbar/Navbar.tsx (1)
64-76: Close button hidden during face search.The close button only appears when
isSearchActive(text search), but the query image is also displayed during face search. This creates inconsistent UX where users cannot close the query image during face search.Consider showing the close button whenever a query image exists:
- {isSearchActive && ( + {queryImage && ( <button onClick={() => { setSearchInput(''); dispatch(clearSearch()); }}Or adjust the condition to cover both search types:
- {isSearchActive && ( + {searchState.active && (frontend/src/pages/Home/Home.tsx (1)
50-64: Missing error handling for text search.The feedback handler only uses
isErroranderrorfrom the normal fetch query (lines 54-55), ignoring errors from the text search hook. If text search fails, users won't see any error feedback.Include text search error states:
+ const { + data: searchData, + isLoading: searchLoading, + isSuccess: searchSuccess, + isError: searchError, + error: searchErrorObj, + } = useImageSearch(searchQuery, isTextSearchActive); + // LOADING STATUS const finalLoading = isTextSearchActive ? searchLoading : isLoading; + const finalError = isTextSearchActive ? searchError : isError; + const finalErrorObj = isTextSearchActive ? searchErrorObj : error; // FEEDBACK HANDLER useMutationFeedback( { isPending: finalLoading, isSuccess: isTextSearchActive ? searchSuccess : isSuccess, - isError, - error, + isError: finalError, + error: finalErrorObj, },
🧹 Nitpick comments (5)
frontend/src/api/api-functions/images.ts (1)
17-24: Use endpoint constant and improve type safety.Two issues:
- The endpoint
/images/searchis hardcoded whilefetchAllImagesusesimagesEndpoints.getAllImages. Add a constant toapiEndpointsfor consistency.- Return type
Promise<any>loses type safety. Consider usingPromise<APIResponse>to matchfetchAllImages.+// In apiEndpoints.ts, add: +// searchImages: '/images/search', -export const searchImages = async (query: string, tagged?: boolean): Promise<any> => { +export const searchImages = async (query: string, tagged?: boolean): Promise<APIResponse> => { const params = new URLSearchParams({ query }); if (tagged !== undefined) { params.append('tagged', tagged.toString()); } - const response = await apiClient.get(`/images/search?${params.toString()}`); + const response = await apiClient.get<APIResponse>( + `${imagesEndpoints.searchImages}?${params.toString()}` + ); return response.data; };frontend/src/hooks/useImageSearch.ts (1)
4-10: Consider adding staleTime to reduce refetch frequency.The hook works correctly, but since search results are unlikely to change rapidly, adding
staleTimecould improve UX by reducing unnecessary API calls during typing (even with debouncing in the UI).export const useImageSearch = (query: string, enabled: boolean = true) => { return usePictoQuery({ queryKey: ['images', 'search', query], queryFn: () => searchImages(query), enabled: enabled && query.length > 0, + staleTime: 30000, // 30 seconds }); };backend/app/routes/images.py (2)
112-120: Remove redundant validation.This manual check is unnecessary since
Query(..., min_length=1)on line 98 already ensures the query is non-empty. FastAPI will return a 422 validation error before this code executes if the query is empty or whitespace-only (after strip).try: - if not query or not query.strip(): - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=ErrorResponse( - success=False, - error="Validation Error", - message="Search query cannot be empty", - ).model_dump(), - ) - images = db_search_images(query.strip(), tagged=tagged)
124-136: Metadata is being parsed twice.
db_search_images(inbackend/app/database/images.py, line 468) already callsimage_util_parse_metadata(metadata)and stores the parsed dict inimages_dict[image_id]["metadata"]. Calling it again here on line 130 will pass an already-parsed dict through the function.While
image_util_parse_metadatahandles dict input gracefully (returns it as-is), this is inefficient and potentially confusing. Either remove the call here or document the double-parse behavior.image_data = [ ImageData( id=image["id"], path=image["path"], folder_id=image["folder_id"], thumbnailPath=image["thumbnailPath"], - metadata=image_util_parse_metadata(image["metadata"]), + metadata=image["metadata"], # Already parsed in db_search_images isTagged=image["isTagged"], isFavourite=image.get("isFavourite", False), tags=image["tags"], ) for image in images ]backend/app/database/images.py (1)
455-465: Unusedlocation_namevariable.
location_nameis fetched from the query results (line 427, 464) but is never used in the function body. Either remove it from the SELECT clause or use it (e.g., for enriching search results).- for ( - image_id, - path, - folder_id, - thumbnail_path, - metadata, - is_tagged, - is_favourite, - tag_name, - location_name, - ) in results: + for ( + image_id, + path, + folder_id, + thumbnail_path, + metadata, + is_tagged, + is_favourite, + tag_name, + _location_name, # Used in WHERE clause only + ) in results:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
backend/app/database/images.py(2 hunks)backend/app/routes/images.py(2 hunks)docs/backend/backend_python/openapi.json(1 hunks)frontend/src/api/api-functions/images.ts(1 hunks)frontend/src/components/Dialog/FaceSearchDialog.tsx(2 hunks)frontend/src/components/Navigation/Navbar/Navbar.tsx(4 hunks)frontend/src/components/WebCam/WebCamComponent.tsx(2 hunks)frontend/src/features/searchSlice.ts(1 hunks)frontend/src/hooks/useImageSearch.ts(1 hunks)frontend/src/pages/Home/Home.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
frontend/src/hooks/useImageSearch.ts (2)
frontend/src/hooks/useQueryExtension.ts (1)
usePictoQuery(80-108)frontend/src/api/api-functions/images.ts (1)
searchImages(17-24)
frontend/src/components/WebCam/WebCamComponent.tsx (1)
frontend/src/features/searchSlice.ts (1)
startFaceSearch(29-34)
backend/app/routes/images.py (2)
backend/app/database/images.py (1)
db_search_images(399-499)backend/app/utils/images.py (1)
image_util_parse_metadata(496-513)
frontend/src/api/api-functions/images.ts (1)
frontend/src/api/axiosConfig.ts (1)
apiClient(5-12)
frontend/src/components/Dialog/FaceSearchDialog.tsx (1)
frontend/src/features/searchSlice.ts (1)
startFaceSearch(29-34)
frontend/src/pages/Home/Home.tsx (6)
frontend/src/features/imageSelectors.ts (1)
selectImages(5-7)frontend/src/hooks/useQueryExtension.ts (1)
usePictoQuery(80-108)frontend/src/api/api-functions/images.ts (1)
fetchAllImages(5-14)frontend/src/hooks/useImageSearch.ts (1)
useImageSearch(4-10)frontend/src/types/Media.ts (1)
Image(13-23)frontend/src/features/imageSlice.ts (1)
setImages(18-20)
backend/app/database/images.py (1)
backend/app/utils/images.py (1)
image_util_parse_metadata(496-513)
🔇 Additional comments (13)
frontend/src/features/searchSlice.ts (1)
1-45: LGTM! Clean separation of text and face search states.The refactored slice correctly distinguishes between text and face search modes. Each action properly clears the other search type's data, preventing stale state conflicts.
frontend/src/components/Dialog/FaceSearchDialog.tsx (2)
14-14: LGTM!Import correctly updated to use
startFaceSearchfor the face detection search flow.
86-86: LGTM!Dispatch correctly uses
startFaceSearchfor face-based image search, aligning with the refactored search slice.docs/backend/backend_python/openapi.json (1)
890-973: LGTM!The OpenAPI specification for the new
/images/searchendpoint is well-documented with proper parameter validation, response schemas, and a clear description of limitations (face clusters not supported).frontend/src/components/WebCam/WebCamComponent.tsx (2)
14-14: LGTM!Import correctly updated for the face search action.
82-82: LGTM!Correctly dispatches
startFaceSearchfor webcam-captured face searches, consistent with the refactored search slice.backend/app/database/images.py (2)
414-414: Consider escaping SQL LIKE wildcards in user input.The search pattern
f"%{query}%"doesn't escape%and_characters in the user's query. If a user searches for "100%", it will match "100" followed by any characters, not the literal "100%". For a search feature, this may be acceptable behavior, but it's worth documenting.If literal wildcard matching is desired, escape them:
def escape_like(query: str) -> str: return query.replace('%', '\\%').replace('_', '\\_') search_pattern = f"%{escape_like(query)}%"And add
ESCAPE '\\'to each LIKE clause in the query.
399-499: I encountered a repository cloning failure that prevents me from accessing the codebase to verify the reviewer's concern. Without access to the database schema files, I cannot definitively determine whether themetadatatable exists or validate the query's correctness.Given this limitation and the nature of the concern (a potential runtime failure if the table is missing), I must output the review comment with a verification tag indicating that manual investigation is needed:
Verify the
metadatatable exists in the database schema.The query joins
LEFT JOIN metadata md ON i.id = md.image_id, but the reviewer could not locate ametadatatable creation in thedb_create_images_tablefunction. If this table doesn't exist in the database schema, the query will fail at runtime.frontend/src/components/Navigation/Navbar/Navbar.tsx (2)
80-93: LGTM!The search input implementation is solid: controlled state, clear placeholder text, and good keyboard accessibility with Escape key support.
18-18: Good separation of text and face search states.The type-specific check properly distinguishes text search from face search, enabling different UI behaviors for each search mode.
frontend/src/pages/Home/Home.tsx (3)
26-30: Clean search state management.The type-specific flags (
isTextSearchActive,isFaceSearchActive) properly separate different search modes, and thesearchQueryfallback prevents undefined issues.
32-44: Proper query coordination.Disabling the normal fetch during active searches (line 36) prevents redundant API calls, and React Query will automatically refetch when the search clears. The text search hook is correctly gated by
isTextSearchActive.
86-90: Excellent user feedback with dynamic title.The title clearly communicates the search context and result count, providing immediate feedback to users about what they're viewing.
| {isSearchActive && ( | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="h-6 w-6" | ||
| onClick={() => { | ||
| setSearchInput(''); | ||
| dispatch(clearSearch()); | ||
| }} | ||
| > | ||
| {/* <X className="h-4 w-4" /> */} | ||
| </Button> | ||
| )} |
There was a problem hiding this comment.
Clear button has no visible content.
Line 106 has the icon commented out, leaving the button completely empty. This creates an invisible button that users cannot see or interact with, breaking the UX.
Uncomment the icon or add alternative content:
>
- {/* <X className="h-4 w-4" /> */}
+ <X className="h-4 w-4" />
</Button>Don't forget to import X from lucide-react:
-import { Search, } from 'lucide-react';
+import { Search, X } from 'lucide-react';Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/components/Navigation/Navbar/Navbar.tsx around lines 96 to 108,
the clear Button rendered when isSearchActive is empty because the X icon is
commented out; restore visible content by uncommenting the X icon (or add a
suitable label/icon) inside the Button and ensure you import X from
'lucide-react' at the top of the file; keep the existing onClick handler
(clearing state and dispatching clearSearch) intact.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/src/components/Navigation/Navbar/Navbar.tsx (3)
16-21: Debounced effect is coupled to searchState and can be simplifiedRight now the effect depends on
searchState.active/searchState.typebut only uses them to gateclearSearch, which can:
- Dispatch
startTextSearchtwice per new query (once due tosearchInputchange, once afteractive/typeflip).- Auto‑clear a text search started elsewhere if
searchInputis still''whensearchStateflips to active.If the intent is “Navbar owns text search and
clearSearchis always safe when the input is empty”, you can decouple fromsearchStateentirely and dropisSearchActive:- const isSearchActive = searchState.active && searchState.type === 'text'; + // const isSearchActive = searchState.active && searchState.type === 'text'; const dispatch = useDispatch(); const [searchInput, setSearchInput] = useState(''); - // Debounce search - useEffect(() => { - const timer = setTimeout(() => { - if (searchInput.trim().length > 0) { - dispatch(startTextSearch(searchInput.trim())); - } else if (searchInput.trim().length === 0 && isSearchActive) { - dispatch(clearSearch()); - } - }, 500); - - return () => clearTimeout(timer); - - // use searchState - }, [searchInput, dispatch, searchState.active, searchState.type]); + // Debounce search based only on local input + useEffect(() => { + const trimmed = searchInput.trim(); + const timer = setTimeout(() => { + if (trimmed) { + dispatch(startTextSearch(trimmed)); + } else { + dispatch(clearSearch()); + } + }, 500); + return () => clearTimeout(timer); + }, [searchInput, dispatch]);This keeps behavior predictable (one dispatch per logical change, no hidden coupling to external text-search triggers).
Also applies to: 23-37
54-67: Consider whether Navbar should expose a way to clear the query imageThe query image preview looks good, and the
queryImage && ...guard keepsstartsWithsafe. However, with the inline close control removed, there’s no obvious way in the Navbar itself to “drop” the face/query image while staying on the same view.If FaceSearchDialog is the only place to clear it, that’s fine; otherwise, consider re‑adding a small close/clear affordance on the thumbnail so users can quickly revert to a pure text search.
69-81: Wire up the Search icon button or make it non-interactiveThe text input is already fully controlled and debounced, but the icon button is rendered as a
<button>with noonClick, so it appears interactive yet does nothing. That’s confusing and not great for accessibility.Either:
- Attach behavior (e.g., focus the input or immediately trigger a search), or
- Change it to a non-interactive element (e.g.,
<span>with appropriate classes andaria-hidden) so it’s clearly decorative.Also applies to: 84-93
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/src/components/Navigation/Navbar/Navbar.tsx(3 hunks)frontend/src/pages/Home/Home.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/pages/Home/Home.tsx
🔇 Additional comments (1)
frontend/src/components/Navigation/Navbar/Navbar.tsx (1)
1-4: Imports cleanly wire text search and icon usageThe new React/Redux imports plus
startTextSearch/clearSearchandSearchicon wiring are consistent with the rest of the file and the new text-search flow.Also applies to: 7-7
1 participant
This PR introduces a complete text-based search system to enhance image discovery within PictoPy.
Previously, search was limited to face search only; the text search bar in the navbar was non-functional.
With this PR, users can now search images seamlessly using tags, filename, metadata, and even month/year.
Demo: https://www.loom.com/share/bd608939c716492abf8db5265671e5ed
What’s Added-
fixes #661
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.