Fix Merge Conflict : Added text-based search feature to the search bar#1294
Draft
akshajtiwari wants to merge 6 commits into
Draft
Fix Merge Conflict : Added text-based search feature to the search bar#1294akshajtiwari wants to merge 6 commits into
akshajtiwari wants to merge 6 commits into
Conversation
Contributor
WalkthroughThis PR adds a complete text-based image search feature while refactoring the frontend search state to separately support both text and face search modes. The backend implements LIKE-based search across tags, metadata, and image paths. Frontend components are updated to dispatch the appropriate search type, and the Home page integrates both search flows with unified loading and display logic. ChangesUnified Text and Face Search Architecture
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
frontend/src/api/api-functions/images.ts (1)
19-25: ⚡ Quick winReplace
Promise<any>with a concrete response type.This violates the TS guideline to avoid
anyand weakens type safety on the API boundary.Proposed fix
export const searchImages = async ( query: string, tagged?: boolean, -): Promise<any> => { +): 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>(`/images/search?${params.toString()}`); return response.data; };As per coding guidelines: "
**/*.{ts,tsx,js,jsx}: Avoid 'any', use explicit types."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/api/api-functions/images.ts` around lines 19 - 25, The exported async function in frontend/src/api/api-functions/images.ts currently returns Promise<any>; replace this with a concrete response type (e.g., Promise<ImageSearchResponse> or Promise<ImageDto[]>) by defining or importing an appropriate interface (like ImageSearchResponse or ImageDto) that matches the API shape, then use the generic on apiClient.get (apiClient.get<ImageSearchResponse>(...)) and change the function signature from Promise<any> to the concrete type so response.data is strongly typed; update any callers if needed to the new type.frontend/src/components/Navigation/Navbar/Navbar.tsx (1)
17-17: ⚡ Quick winReplace
anyin selector withRootStatefor type-safe search state access.Using
anyhere weakens compile-time guarantees and can mask state-shape regressions.Suggested fix
+import { RootState } from '`@/app/store`'; ... - const queryImage = useSelector((state: any) => state.search.queryImage); + const queryImage = useSelector( + (state: RootState) => state.search.queryImage, + );As per coding guidelines, "
**/*.{ts,tsx,js,jsx}: Avoid 'any', use explicit types."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/Navigation/Navbar/Navbar.tsx` at line 17, The selector in Navbar.tsx uses an untyped `any` which weakens type-safety; change the useSelector call to use your app RootState (e.g., import { RootState } from your store/types) and replace `useSelector((state: any) => state.search.queryImage)` with a typed signature like `useSelector((state: RootState) => state.search.queryImage)` so `queryImage` is accessed with full type checking; ensure you add the appropriate RootState import and update any related imports in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/Dialog/FaceSearchDialog.tsx`:
- Around line 86-88: startFaceSearch is dispatched before calling
getSearchImages but failures don't rollback the search state, leaving
search.active=true; wrap the getSearchImages call in a try/catch (or handle its
promise rejection) and on error dispatch an action that clears the face-search
active state (e.g., stopFaceSearch / clearFaceSearch), hide the loader
(counterpart to showLoader) and surface/log the error; update the error path
where getSearchImages is invoked so the search mode is reset whenever the API
call fails.
In `@frontend/src/components/WebCam/WebCamComponent.tsx`:
- Around line 161-163: The code optimistically starts face search
(dispatch(startFaceSearch(capturedImageUrl))) before calling
searchByFaceMutation.mutate, so if the mutation fails the UI state may remain
active; fix by either moving dispatch(startFaceSearch(...)) to the mutation
success handler or by adding an onError rollback that dispatches a stop/clear
action (e.g., dispatch(stopFaceSearch()) or dispatch(clearFaceSearchState())) in
the mutation call (searchByFaceMutation.mutate(capturedImageUrl, { onError: ()
=> dispatch(stopFaceSearch()) })), and ensure onClose is only called after
success or is also paired with the rollback so state stays consistent.
In `@frontend/src/hooks/useImageSearch.ts`:
- Around line 6-9: The hook useImageSearch currently enables the query when
query.length > 0 which still allows whitespace-only strings and causes 400s;
change the enabled check to use a trimmed value (e.g., const trimmed =
query.trim()) and use enabled: enabled && trimmed.length > 0, update queryFn to
call searchImages(trimmed) and update queryKey to include trimmed (e.g.,
['images','search', trimmed]) so caching and requests use the normalized query;
make these changes in the useImageSearch hook around the existing queryKey,
queryFn, and enabled usage.
In `@frontend/src/pages/Home/Home.tsx`:
- Around line 41-57: When text-search is active the feedback hook still reads
the regular image query's error values; change the props passed to
useMutationFeedback so it selects the text-search error state when
isTextSearchActive is true. Specifically, use the error flags/objects returned
by useImageSearch (e.g., searchError and searchIsError or whatever names that
hook exposes) and pass isError: isTextSearchActive ? searchIsError : isError and
error: isTextSearchActive ? searchError : error while keeping the existing
finalLoading/isSuccess selection logic.
---
Nitpick comments:
In `@frontend/src/api/api-functions/images.ts`:
- Around line 19-25: The exported async function in
frontend/src/api/api-functions/images.ts currently returns Promise<any>; replace
this with a concrete response type (e.g., Promise<ImageSearchResponse> or
Promise<ImageDto[]>) by defining or importing an appropriate interface (like
ImageSearchResponse or ImageDto) that matches the API shape, then use the
generic on apiClient.get (apiClient.get<ImageSearchResponse>(...)) and change
the function signature from Promise<any> to the concrete type so response.data
is strongly typed; update any callers if needed to the new type.
In `@frontend/src/components/Navigation/Navbar/Navbar.tsx`:
- Line 17: The selector in Navbar.tsx uses an untyped `any` which weakens
type-safety; change the useSelector call to use your app RootState (e.g., import
{ RootState } from your store/types) and replace `useSelector((state: any) =>
state.search.queryImage)` with a typed signature like `useSelector((state:
RootState) => state.search.queryImage)` so `queryImage` is accessed with full
type checking; ensure you add the appropriate RootState import and update any
related imports in the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d0c3de52-52f7-42cc-957a-b33a073cff3b
📒 Files selected for processing (10)
backend/app/database/images.pybackend/app/routes/images.pydocs/backend/backend_python/openapi.jsonfrontend/src/api/api-functions/images.tsfrontend/src/components/Dialog/FaceSearchDialog.tsxfrontend/src/components/Navigation/Navbar/Navbar.tsxfrontend/src/components/WebCam/WebCamComponent.tsxfrontend/src/features/searchSlice.tsfrontend/src/hooks/useImageSearch.tsfrontend/src/pages/Home/Home.tsx
Comment on lines
+86
to
88
| dispatch(startFaceSearch(filePath)); | ||
| dispatch(showLoader('Searching faces...')); | ||
| getSearchImages(filePath); |
Contributor
There was a problem hiding this comment.
Clear face-search state on request failure to avoid stale active mode.
startFaceSearch is dispatched before the API call, but the error path does not rollback search state. A failed request can leave search.active=true with stale face mode.
Suggested fix
onError: () => {
dispatch(hideLoader());
+ dispatch(clearSearch());
dispatch(
showInfoDialog({
title: 'Search Failed',
message: 'There was an error while searching for faces.',
variant: 'error',
}),
);
},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/Dialog/FaceSearchDialog.tsx` around lines 86 - 88,
startFaceSearch is dispatched before calling getSearchImages but failures don't
rollback the search state, leaving search.active=true; wrap the getSearchImages
call in a try/catch (or handle its promise rejection) and on error dispatch an
action that clears the face-search active state (e.g., stopFaceSearch /
clearFaceSearch), hide the loader (counterpart to showLoader) and surface/log
the error; update the error path where getSearchImages is invoked so the search
mode is reset whenever the API call fails.
Comment on lines
+161
to
163
| dispatch(startFaceSearch(capturedImageUrl)); | ||
| searchByFaceMutation.mutate(capturedImageUrl); | ||
| onClose(); |
Contributor
There was a problem hiding this comment.
Rollback face-search state when webcam search fails.
This path activates face search before mutation. If the mutation errors, search state can remain active and inconsistent unless cleared in error handling.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/WebCam/WebCamComponent.tsx` around lines 161 - 163,
The code optimistically starts face search
(dispatch(startFaceSearch(capturedImageUrl))) before calling
searchByFaceMutation.mutate, so if the mutation fails the UI state may remain
active; fix by either moving dispatch(startFaceSearch(...)) to the mutation
success handler or by adding an onError rollback that dispatches a stop/clear
action (e.g., dispatch(stopFaceSearch()) or dispatch(clearFaceSearchState())) in
the mutation call (searchByFaceMutation.mutate(capturedImageUrl, { onError: ()
=> dispatch(stopFaceSearch()) })), and ensure onClose is only called after
success or is also paired with the rollback so state stays consistent.
Comment on lines
+6
to
+9
| queryKey: ['images', 'search', query], | ||
| queryFn: () => searchImages(query), | ||
| enabled: enabled && query.length > 0, | ||
| }); |
Contributor
There was a problem hiding this comment.
Guard against whitespace-only queries before firing requests.
query.length > 0 still allows " ", which triggers avoidable 400s from /images/search.
Proposed fix
export const useImageSearch = (query: string, enabled: boolean = true) => {
+ const normalizedQuery = query.trim();
return usePictoQuery({
- queryKey: ['images', 'search', query],
- queryFn: () => searchImages(query),
- enabled: enabled && query.length > 0,
+ queryKey: ['images', 'search', normalizedQuery],
+ queryFn: () => searchImages(normalizedQuery),
+ enabled: enabled && normalizedQuery.length > 0,
});
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/hooks/useImageSearch.ts` around lines 6 - 9, The hook
useImageSearch currently enables the query when query.length > 0 which still
allows whitespace-only strings and causes 400s; change the enabled check to use
a trimmed value (e.g., const trimmed = query.trim()) and use enabled: enabled &&
trimmed.length > 0, update queryFn to call searchImages(trimmed) and update
queryKey to include trimmed (e.g., ['images','search', trimmed]) so caching and
requests use the normalized query; make these changes in the useImageSearch hook
around the existing queryKey, queryFn, and enabled usage.
Comment on lines
+41
to
+57
| const { | ||
| data: searchData, | ||
| isLoading: searchLoading, | ||
| isSuccess: searchSuccess, | ||
| } = useImageSearch(searchQuery, isTextSearchActive); | ||
|
|
||
| // LOADING MERGE | ||
| const finalLoading = isTextSearchActive ? searchLoading : isLoading; | ||
|
|
||
| // FEEDBACK | ||
| useMutationFeedback( | ||
| { isPending: isLoading, isSuccess, isError, error }, | ||
| { | ||
| isPending: finalLoading, | ||
| isSuccess: isTextSearchActive ? searchSuccess : isSuccess, | ||
| isError, | ||
| error, | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced file and surrounding code
FILE="frontend/src/pages/Home/Home.tsx"
if [ ! -f "$FILE" ]; then
echo "MISSING_FILE: $FILE"
exit 0
fi
echo "== File: $FILE =="
sed -n '1,140p' "$FILE" | nl -ba | sed -n '30,120p'
# Extract the specific snippet region (around provided line numbers)
sed -n '1,220p' "$FILE" | nl -ba | sed -n '35,90p'
# Search for useImageSearch definition and where it's used
echo "== Search: useImageSearch usages =="
rg -n "useImageSearch" -S frontend/src | head -n 50 || true
# Search for useMutationFeedback definition/props
echo "== Search: useMutationFeedback usages =="
rg -n "useMutationFeedback" -S frontend/src | head -n 50 || trueRepository: AOSSIE-Org/PictoPy
Length of output: 169
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="frontend/src/pages/Home/Home.tsx"
if [ ! -f "$FILE" ]; then
echo "MISSING_FILE: $FILE"
exit 0
fi
echo "== Snippet around requested lines =="
# Use cat -n (available) instead of nl
cat -n "$FILE" | sed -n '1,200p' | sed -n '35,90p'
echo "== Search for useImageSearch =="
rg -n "useImageSearch" frontend/src/pages/Home/Home.tsx frontend/src -S | head -n 50 || true
echo "== Search for useMutationFeedback =="
rg -n "useMutationFeedback" frontend/src/pages/Home/Home.tsx frontend/src -S | head -n 50 || true
echo "== Show useMutationFeedback call with surrounding lines =="
cat -n "$FILE" | sed -n '1,260p' | sed -n '60,130p'Repository: AOSSIE-Org/PictoPy
Length of output: 6832
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="frontend/src/pages/Home/Home.tsx"
if [ ! -f "$FILE" ]; then
echo "MISSING_FILE: $FILE"
exit 0
fi
echo "== Snippet for normal (non-text) image query + error vars =="
# show earlier region where normal query likely defined
cat -n "$FILE" | sed -n '1,110p'
echo "== Snippet for imports (to locate vars/types) =="
head -n 60 "$FILE" | cat -n
echo "== Inspect useImageSearch hook =="
HOOK1="frontend/src/hooks/useImageSearch.ts"
if [ -f "$HOOK1" ]; then
cat -n "$HOOK1" | sed -n '1,200p'
else
echo "MISSING_FILE: $HOOK1"
fi
echo "== Inspect useMutationFeedback hook =="
HOOK2="frontend/src/hooks/useMutationFeedback.tsx"
if [ ! -f "$HOOK2" ]; then HOOK2="frontend/src/hooks/useMutationFeedback.ts"; fi
if [ -f "$HOOK2" ]; then
cat -n "$HOOK2" | sed -n '1,220p'
else
echo "MISSING_FILE: frontend/src/hooks/useMutationFeedback.ts(x)"
fiRepository: AOSSIE-Org/PictoPy
Length of output: 11196
Wire text-search error state into feedback handling.
When isTextSearchActive is true, useMutationFeedback still uses isError/error coming from the normal image query (usePictoQuery) instead of the text-search query (useImageSearch). Text-search failures can therefore be missed.
Suggested fix
const {
data: searchData,
isLoading: searchLoading,
isSuccess: searchSuccess,
+ isError: searchError,
+ error: searchErrorObj,
} = useImageSearch(searchQuery, isTextSearchActive);
...
useMutationFeedback(
{
isPending: finalLoading,
isSuccess: isTextSearchActive ? searchSuccess : isSuccess,
- isError,
- error,
+ isError: isTextSearchActive ? searchError : isError,
+ error: isTextSearchActive ? searchErrorObj : error,
},📝 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.
Suggested change
| const { | |
| data: searchData, | |
| isLoading: searchLoading, | |
| isSuccess: searchSuccess, | |
| } = useImageSearch(searchQuery, isTextSearchActive); | |
| // LOADING MERGE | |
| const finalLoading = isTextSearchActive ? searchLoading : isLoading; | |
| // FEEDBACK | |
| useMutationFeedback( | |
| { isPending: isLoading, isSuccess, isError, error }, | |
| { | |
| isPending: finalLoading, | |
| isSuccess: isTextSearchActive ? searchSuccess : isSuccess, | |
| isError, | |
| error, | |
| }, | |
| const { | |
| data: searchData, | |
| isLoading: searchLoading, | |
| isSuccess: searchSuccess, | |
| isError: searchError, | |
| error: searchErrorObj, | |
| } = useImageSearch(searchQuery, isTextSearchActive); | |
| // LOADING MERGE | |
| const finalLoading = isTextSearchActive ? searchLoading : isLoading; | |
| // FEEDBACK | |
| useMutationFeedback( | |
| { | |
| isPending: finalLoading, | |
| isSuccess: isTextSearchActive ? searchSuccess : isSuccess, | |
| isError: isTextSearchActive ? searchError : isError, | |
| error: isTextSearchActive ? searchErrorObj : error, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/pages/Home/Home.tsx` around lines 41 - 57, When text-search is
active the feedback hook still reads the regular image query's error values;
change the props passed to useMutationFeedback so it selects the text-search
error state when isTextSearchActive is true. Specifically, use the error
flags/objects returned by useImageSearch (e.g., searchError and searchIsError or
whatever names that hook exposes) and pass isError: isTextSearchActive ?
searchIsError : isError and error: isTextSearchActive ? searchError : error
while keeping the existing finalLoading/isSuccess selection logic.
akshajtiwari
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addressed Issues:
Added text-based search feature to the search bar
Fixes Pull Request #667
FIxes Issue : #661
Screenshots/Recordings:
NA
Additional Notes:
NA
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: Claude
Checklist
Summary by CodeRabbit
Release Notes