Feat: Global Search Support for Face Collections and AI Tags#1338
Open
rohan-pandeyy wants to merge 6 commits into
Open
Feat: Global Search Support for Face Collections and AI Tags#1338rohan-pandeyy wants to merge 6 commits into
rohan-pandeyy wants to merge 6 commits into
Conversation
Contributor
WalkthroughAdds end-to-end tag-based image search from backend to frontend. A new ChangesTag-based global image search
OpenAPI spec reformatting
Sequence Diagram(s)sequenceDiagram
actor User
participant Navbar
participant AppRouter as React Router
participant SearchResults
participant searchImagesByTag as searchImagesByTag API
participant FastAPI as GET /images/search
participant db_search as db_search_images_by_tag
User->>Navbar: Enter query, press Enter or click search
Navbar->>Navbar: Trim input, check clusters for name match
alt Cluster name match found
Navbar->>AppRouter: navigate(/person/:cluster_id)
else No cluster match
Navbar->>AppRouter: navigate(/search?value=encoded_query)
AppRouter->>SearchResults: Render with value param
SearchResults->>searchImagesByTag: GET /images/search?tag=query
searchImagesByTag->>FastAPI: HTTP GET request
FastAPI->>db_search: tag lookup (case-insensitive JOIN)
db_search-->>FastAPI: List[dict] with deduplicated tags per image
FastAPI-->>SearchResults: GetAllImagesResponse
SearchResults->>SearchResults: dispatch setImages, render ImageCard grid
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 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 |
github-actions
Bot
added
GSoC 2026
backend
enhancement
![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: 5
🤖 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 `@backend/app/database/images.py`:
- Around line 577-579: The sqlite3.Error exception handler in the image search
function is catching database errors but returning an empty list, which makes
the failure appear as a successful query with zero results. Instead of returning
an empty list when a sqlite3.Error is caught in the except block, you should
re-raise the exception or raise an appropriate exception type to properly
propagate the database error up to the caller, ensuring the API can distinguish
between a legitimate empty search result and an actual backend failure.
- Around line 551-555: In the images_dict dictionary construction at the line
containing "folder_id": str(folder_id) if folder_id is not None else None, the
ternary expression is allowing None to be set when folder_id is None, which
violates the API contract requiring folder_id to be a non-null string. Replace
the None fallback in this ternary expression with an appropriate non-null
default value (such as an empty string) to ensure folder_id is never null in the
returned response dictionary.
In `@backend/app/routes/images.py`:
- Around line 125-132: The exception handler for the image search endpoint is
exposing raw exception details to the client by including str(e) in the
ErrorResponse message field, which can leak sensitive backend information.
Replace the detailed message containing str(e) with a generic client-safe error
message, and add a server-side logger call (using the appropriate logging
mechanism for your application) to capture and log the actual exception details
before raising the HTTPException. This ensures debugging information remains on
the server while clients only receive generic error messages.
In `@frontend/src/components/Navigation/Navbar/Navbar.tsx`:
- Around line 76-88: The handleSearchSubmit function only checks the in-memory
clusters list but does not verify that clusters have been fully loaded before
processing a search query. If a user submits a search before the clusters list
is hydrated, exact face-cluster matches will incorrectly route to the search
page instead of the person page. Add a guard check at the start of
handleSearchSubmit to ensure the clusters array exists and is not empty before
attempting to find a match; if clusters are not yet loaded, return early or
provide appropriate user feedback to prevent misrouting.
In `@frontend/src/pages/SearchResults/SearchResults.tsx`:
- Around line 27-37: Add a return statement with a cleanup function to the
useEffect hook that contains the showLoader and hideLoader logic. The cleanup
function should dispatch hideLoader() to ensure the loader is properly hidden
when the SearchResults component unmounts, preventing the loader from remaining
in an active state if the component is unmounted while a search request is still
pending.
🪄 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: ec852948-8abf-4048-964f-00b00911e7c3
📒 Files selected for processing (9)
backend/app/database/images.pybackend/app/routes/images.pydocs/backend/backend_python/openapi.jsonfrontend/src/api/api-functions/images.tsfrontend/src/api/apiEndpoints.tsfrontend/src/components/Navigation/Navbar/Navbar.tsxfrontend/src/constants/routes.tsfrontend/src/pages/SearchResults/SearchResults.tsxfrontend/src/routes/AppRoutes.tsx
rohan-pandeyy
changed the title
Member
Author
|
@coderabbitai, in addition to the existing review, please check for:
|
Member
Author
|
@coderabbitai, your suggestions aren't within the scope of the PR and it's touched files. I was hoping for something within that boundary |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
1. 🔴 Double-parse of
|
| Issue | Severity | Touched file |
|---|---|---|
Double-parse of metadata |
🔴 Bug | database/images.py + routes/images.py |
displayImages bypasses Redux |
🟡 Medium | SearchResults.tsx |
Hardcoded /person/ path, not using ROUTES |
🟡 Medium | Navbar.tsx |
searchImagesByTag missing from API barrel |
🟢 Low | api-functions/images.ts |
✏️ Learnings added
Learnt from: rohan-pandeyy
URL: https://github.com/AOSSIE-Org/PictoPy/pull/1338
Timestamp: 2026-06-22T18:56:41.379Z
Learning: For PictoPy (AOSSIE-Org/PictoPy), when reviewing PRs, restrict suggestions to files that are actually touched/changed in the PR. Do not suggest refactors or abstractions involving untouched files or broader codebase patterns unless the user explicitly asks for a broader review.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
- Remove metadata double-parsing in image DB search - Read search results from Redux store via selector - Replace hardcoded routes with ROUTES constants - Import searchImagesByTag from API barrel
- Return empty string when folder_id is missing to prevent schema validation failures - Re-raise sqlite3 errors instead of masking them as empty search results - Log detailed exceptions server-side while returning safe client-facing errors - Prevent search navigation while clusters are still loading in Navbar - Add loader cleanup on SearchResults unmount to avoid stuck loading state - Verify frontend type safety with successful TypeScript check
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/database/images.py (1)
496-571:⚠️ Potential issue | 🟠 MajorExtract shared image grouping and tag deduplication logic to reduce duplication.
The Python-level grouping and tag deduplication pattern in
db_search_images_by_tag(lines 531–571) is nearly identical to the same pattern indb_get_all_images(lines 195–240). Both functions:
- Query images with
LEFT JOIN image_classes+LEFT JOIN mappings- Group results by image_id using
images_dict- Deduplicate tags with the same condition check
- Convert to list with identical logic for handling empty tag lists
- Sort by path
Consider extracting this shared logic into a helper function (e.g.,
_process_images_with_tags) to improve maintainability and reduce duplication.🤖 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 `@backend/app/database/images.py` around lines 496 - 571, Extract the shared image grouping and tag deduplication logic that appears in both db_search_images_by_tag and db_get_all_images into a separate helper function (e.g., _process_images_with_tags). This helper should accept the cursor results as a parameter and return the processed list of images with deduplicated tags, sorted by path. Then replace the identical logic blocks in both db_search_images_by_tag (around lines 531-571) and db_get_all_images (around lines 195-240) with calls to this new helper function.Source: Path instructions
🧹 Nitpick comments (1)
backend/app/database/images.py (1)
574-574: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winUse
logger.exception()for richer error context.When logging before re-raising an exception,
logger.exception()automatically includes the full stack trace, making debugging easier.📊 Suggested improvement
- logger.error(f"Error searching images by tag: {e}") + logger.exception(f"Error searching images by tag: {e}")🤖 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 `@backend/app/database/images.py` at line 574, The error logging statement in the image search function uses logger.error() which only logs the exception message without the stack trace. Replace logger.error(f"Error searching images by tag: {e}") with logger.exception() instead, passing a descriptive message as the argument. This will automatically capture and include the full stack trace in the logs, providing richer context for debugging.
🤖 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.
Outside diff comments:
In `@backend/app/database/images.py`:
- Around line 496-571: Extract the shared image grouping and tag deduplication
logic that appears in both db_search_images_by_tag and db_get_all_images into a
separate helper function (e.g., _process_images_with_tags). This helper should
accept the cursor results as a parameter and return the processed list of images
with deduplicated tags, sorted by path. Then replace the identical logic blocks
in both db_search_images_by_tag (around lines 531-571) and db_get_all_images
(around lines 195-240) with calls to this new helper function.
---
Nitpick comments:
In `@backend/app/database/images.py`:
- Line 574: The error logging statement in the image search function uses
logger.error() which only logs the exception message without the stack trace.
Replace logger.error(f"Error searching images by tag: {e}") with
logger.exception() instead, passing a descriptive message as the argument. This
will automatically capture and include the full stack trace in the logs,
providing richer context for debugging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f72e33ef-5e2d-4681-88cc-60b2506ae94f
📒 Files selected for processing (4)
backend/app/database/images.pybackend/app/routes/images.pyfrontend/src/components/Navigation/Navbar/Navbar.tsxfrontend/src/pages/SearchResults/SearchResults.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/components/Navigation/Navbar/Navbar.tsx
- frontend/src/pages/SearchResults/SearchResults.tsx
- backend/app/routes/images.py
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
1 participant
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.
Fixes #1337
Summary
This PR introduces global search support for face collections and AI-generated tags through the Navbar search workflow.
Users can now submit a search query from the navigation bar and either:
The implementation adds a backend-powered image tag search endpoint and a new Search Results page, providing a unified search experience across the photo library.
Changes
Backend
Frontend API Layer
Search Results Page
SearchResultspage for displaying search matches.Navbar Integration
User Experience
Face Collection Search
Searching for a face collection name:
navigates directly to the existing face collection view for that cluster.
AI Tag Search
Searching for an AI-generated object tag:
opens a dedicated Search Results page displaying all matching images.
Why a Dedicated Search Results Page?
Rather than filtering the currently visible gallery, search results are displayed in a dedicated view similar to the search experience found in modern photo management applications.
This provides:
Future Scope
This PR focuses exclusively on search result retrieval and display.
The following are intentionally out of scope:
Screenshots/Recordings:
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, Gemini
Checklist
Summary by CodeRabbit
Release Notes
New Features
Documentation