fixed: implement text search by AI-detected tags#1207
Open
ProthamD wants to merge 2 commits intoAOSSIE-Org:mainfrom
Open
fixed: implement text search by AI-detected tags#1207ProthamD wants to merge 2 commits intoAOSSIE-Org:mainfrom
ProthamD wants to merge 2 commits intoAOSSIE-Org:mainfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughAdded a text-based tag search feature: backend exposes a GET /images/search endpoint and db_search_images_by_tags; frontend adds API client, Navbar input handlers (Enter/Escape/Search), and Redux state to start/clear text searches. Responses map to existing ImageData shape. Changes
Sequence DiagramsequenceDiagram
participant User
participant Navbar as Navbar Component
participant API as Frontend API
participant Backend as Backend API
participant DB as Database
User->>Navbar: Type tags & press Enter / click Search
activate Navbar
Navbar->>Navbar: Validate non-empty tags
Navbar->>Navbar: Dispatch startTextSearch
Navbar->>API: searchImagesByTags(tags)
deactivate Navbar
activate API
API->>Backend: GET /images/search?tags=tag1,tag2
deactivate API
activate Backend
Backend->>Backend: Validate tags
Backend->>DB: db_search_images_by_tags(tags)
deactivate Backend
activate DB
DB->>DB: JOIN images, mappings, tags (case-insensitive) and aggregate
DB-->>Backend: image list
deactivate DB
Backend-->>API: GetAllImagesResponse (images)
API-->>Navbar: APIResponse with images
Navbar->>Navbar: Map results to ImageData & update display
Navbar-->>User: Show filtered images
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
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: 3
🧹 Nitpick comments (1)
backend/app/database/images.py (1)
297-339: Extract shared row-to-image mapping to avoid duplication.This block duplicates the aggregation/parsing flow already present in
db_get_all_images, which increases drift risk when response shape evolves.♻️ Suggested refactor direction
+def _rows_to_image_dicts(results: list[tuple]) -> list[dict]: + from app.utils.images import image_util_parse_metadata + images_dict = {} + for (...) in results: + ... + images = [] + for image_data in images_dict.values(): + if not image_data["tags"]: + image_data["tags"] = None + images.append(image_data) + images.sort(key=lambda x: x["path"]) + return imagesThen call the helper from both
db_get_all_imagesanddb_search_images_by_tags.As per coding guidelines, "Look for code duplication".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/database/images.py` around lines 297 - 339, Extract the duplicated row-to-image aggregation into a single helper (e.g., parse_image_rows or _aggregate_image_rows) that accepts the raw query results iterable and returns the list of image dicts sorted by "path"; implement the same logic currently in db_search_images_by_tags (use image_util_parse_metadata, build images_dict keyed by image_id, append unique tags, set tags to None if empty, cast folder_id to str, booleans for isTagged/isFavourite, and captured_at handling), then replace the aggregation block in both db_search_images_by_tags and db_get_all_images to call this new helper and return its result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/routes/images.py`:
- Around line 133-183: Add a new test_images.py following the existing test
patterns to cover the search_images_by_tags endpoint and its DB function: write
route tests that call search_images_by_tags (via test client) to assert 400 on
empty/invalid tags, assert OR semantics by seeding images with overlapping tags
and verifying GetAllImagesResponse contains any matching ImageData (check id,
path, tags, thumbnailPath, metadata via image_util_parse_metadata expectations),
and simulate an internal error by patching/db_search_images_by_tags to raise an
exception and assert a 500 with the ErrorResponse shape; also add unit tests for
db_search_images_by_tags to verify correct results for single/multiple tags and
edge cases (no matches), and ensure tests clean up/seed fixtures consistent with
test_albums.py pattern.
In `@frontend/src/components/Navigation/Navbar/Navbar.tsx`:
- Around line 26-29: The current split using /[,\s]+/ in Navbar.tsx breaks
multi-word tags (e.g., "traffic light"); change the logic in the search handling
around textQuery/trimmed/tags so you split only on commas (e.g., use
trimmed.split(',') ), then .map(tag => tag.trim()) and .filter(Boolean) to
preserve multi-word tags while removing empty entries; keep the existing early
return for empty trimmed.
- Around line 40-43: The text search clear handler (handleClearTextSearch) only
resets state but doesn’t reload the full gallery; add a useEffect in Home.tsx
that watches textSearchActive and when it becomes false dispatches the same
fetch/loader used by startTextSearch (or the existing "load all images" action
used by face search clearing) to restore the full gallery; locate the effect
near the face-search clearing effect and ensure it triggers on textSearchActive
changing to false and calls the same image-fetching logic (or action creator)
that startTextSearch uses so clearing via the button or Escape shows all images
again.
---
Nitpick comments:
In `@backend/app/database/images.py`:
- Around line 297-339: Extract the duplicated row-to-image aggregation into a
single helper (e.g., parse_image_rows or _aggregate_image_rows) that accepts the
raw query results iterable and returns the list of image dicts sorted by "path";
implement the same logic currently in db_search_images_by_tags (use
image_util_parse_metadata, build images_dict keyed by image_id, append unique
tags, set tags to None if empty, cast folder_id to str, booleans for
isTagged/isFavourite, and captured_at handling), then replace the aggregation
block in both db_search_images_by_tags and db_get_all_images to call this new
helper and return its result.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/app/database/images.pybackend/app/routes/images.pyfrontend/src/api/api-functions/images.tsfrontend/src/api/apiEndpoints.tsfrontend/src/components/Navigation/Navbar/Navbar.tsxfrontend/src/features/searchSlice.ts
Comment on lines
+133
to
+183
| @router.get( | ||
| "/search", | ||
| response_model=GetAllImagesResponse, | ||
| responses={400: {"model": ErrorResponse}, 500: {"model": ErrorResponse}}, | ||
| ) | ||
| def search_images_by_tags( | ||
| tags: str = Query( | ||
| ..., | ||
| description="Comma-separated tag names to search for (e.g. dog,beach,person)", | ||
| ) | ||
| ): | ||
| """Return images whose AI-detected tags contain any of the requested tags.""" | ||
| tag_list = [t.strip() for t in tags.split(",") if t.strip()] | ||
| if not tag_list: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_400_BAD_REQUEST, | ||
| detail=ErrorResponse( | ||
| success=False, | ||
| error="Bad request", | ||
| message="At least one tag must be provided.", | ||
| ).model_dump(), | ||
| ) | ||
| try: | ||
| images = db_search_images_by_tags(tag_list) | ||
| image_data = [ | ||
| ImageData( | ||
| id=image["id"], | ||
| path=image["path"], | ||
| folder_id=image["folder_id"], | ||
| thumbnailPath=image["thumbnailPath"], | ||
| metadata=image_util_parse_metadata(image["metadata"]), | ||
| isTagged=image["isTagged"], | ||
| isFavourite=image.get("isFavourite", False), | ||
| tags=image["tags"], | ||
| ) | ||
| for image in images | ||
| ] | ||
| return GetAllImagesResponse( | ||
| success=True, | ||
| message=f"Found {len(image_data)} image(s) matching tags: {', '.join(tag_list)}", | ||
| data=image_data, | ||
| ) | ||
| except Exception as e: | ||
| raise HTTPException( | ||
| status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, | ||
| detail=ErrorResponse( | ||
| success=False, | ||
| error="Internal server error", | ||
| message=f"Unable to search images: {str(e)}", | ||
| ).model_dump(), | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for tests covering the search_images_by_tags function and related functionality
rg -n -C2 "search_images_by_tags|db_search_images_by_tags|/images/search|tags.*dog.*beach" -g "*test*" -t pyRepository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
# Also search more broadly for test files related to images
fd -type f -name "*test*.py" | head -20Repository: AOSSIE-Org/PictoPy
Length of output: 232
🏁 Script executed:
# Check the structure of test directories
fd -type d -name "test*" -o -name "*test"Repository: AOSSIE-Org/PictoPy
Length of output: 232
🏁 Script executed:
# Find all test files
fd -type f -name "*test*.py"Repository: AOSSIE-Org/PictoPy
Length of output: 232
🏁 Script executed:
# Search for any test directory
find . -type d \( -name test -o -name tests -o -name "*test*" \) | head -20Repository: AOSSIE-Org/PictoPy
Length of output: 108
🏁 Script executed:
# Search for imports of the search function or route handler in any Python files
rg "search_images_by_tags|db_search_images_by_tags" --type pyRepository: AOSSIE-Org/PictoPy
Length of output: 395
🏁 Script executed:
# Check if there's a test file for the images route
find . -type f -name "*.py" -path "*test*" | xargs grep -l "search.*image\|image.*search" 2>/dev/nullRepository: AOSSIE-Org/PictoPy
Length of output: 46
🏁 Script executed:
# List contents of the backend/tests directory
find ./backend/tests -type f -name "*.py" | head -30Repository: AOSSIE-Org/PictoPy
Length of output: 259
🏁 Script executed:
# Search for any references to images in test files
rg "search.*image\|image.*search\|test.*search" ./backend/tests -A 3Repository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
# Look for test files specifically for the routes
find ./backend/tests -type f -name "*image*"Repository: AOSSIE-Org/PictoPy
Length of output: 44
🏁 Script executed:
# Check the structure of backend/tests
ls -la ./backend/tests/Repository: AOSSIE-Org/PictoPy
Length of output: 692
🏁 Script executed:
# Search all test files for any reference to /images endpoint
rg "/images|images_route|test.*image" ./backend/tests/Repository: AOSSIE-Org/PictoPy
Length of output: 1847
🏁 Script executed:
# Check the conftest.py to understand test setup
cat ./backend/tests/conftest.pyRepository: AOSSIE-Org/PictoPy
Length of output: 1602
🏁 Script executed:
# Look at one of the test files to understand the pattern and structure
head -50 ./backend/tests/test_folders.pyRepository: AOSSIE-Org/PictoPy
Length of output: 1411
Add automated tests for /images/search endpoint before merge.
No test coverage exists for the search_images_by_tags endpoint or its database function. This critical user-facing search feature lacks tests for validation (empty/invalid tags), OR semantics (matching any tag), and error scenarios. Given the established test pattern in the codebase (test_albums.py, test_folders.py, test_face_clusters.py), a test_images.py file with comprehensive route and database-level tests is required to meet project standards.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/routes/images.py` around lines 133 - 183, Add a new
test_images.py following the existing test patterns to cover the
search_images_by_tags endpoint and its DB function: write route tests that call
search_images_by_tags (via test client) to assert 400 on empty/invalid tags,
assert OR semantics by seeding images with overlapping tags and verifying
GetAllImagesResponse contains any matching ImageData (check id, path, tags,
thumbnailPath, metadata via image_util_parse_metadata expectations), and
simulate an internal error by patching/db_search_images_by_tags to raise an
exception and assert a 500 with the ErrorResponse shape; also add unit tests for
db_search_images_by_tags to verify correct results for single/multiple tags and
edge cases (no matches), and ensure tests clean up/seed fixtures consistent with
test_albums.py pattern.
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.
feat: implement text search by AI-detected tags
fixes #1048
🔍 What's the Problem?
The Navbar search bar in PictoPy was purely cosmetic — it accepted text input but did absolutely nothing with it. Clicking the search button or pressing Enter had zero effect. There was also no backend endpoint to support querying images by tag name.
This PR fully implements end-to-end text-based image search by AI-detected tags, making the search bar functional for the first time.
🐛 Root Cause
Frontend (
Navbar.tsx):<Input>had novalue,onChange, oronKeyDown— making it an uncontrolled, disconnected elementonClickhandlerBackend:
GET /images/only supported filtering bytagged(boolean)✅ Changes Made
Backend
backend/app/database/images.pydb_search_images_by_tags(tags: List[str])— performs a case-insensitive SQL query usingLOWER()to find all images whose AI-detected tags match any of the provided tag names. Returns the same data shape as the existingdb_get_all_images()for full compatibility.backend/app/routes/images.pyGET /images/search?tags=dog,beach,persontagsquery parameterGetAllImagesResponseFrontend
frontend/src/api/apiEndpoints.tssearchByTags: '/images/search'toimagesEndpointsfrontend/src/api/api-functions/images.tssearchImagesByTags(tags: string[])— joins the array as a comma-separated query param and calls the new endpointfrontend/src/features/searchSlice.tsSearchStateinterface withqueryText?: stringandtextSearchActive: booleanstartTextSearch(query)andclearTextSearch()startSearch/clearSearch) unchangedfrontend/src/components/Navigation/Navbar/Navbar.tsx<Input>withvalue,onChange, andonKeyDown:onClickto the Search (🔍) button"Add to your search"to"Search by tags (e.g. dog, beach)"🧪 How to Test
person,dog,car,bottle)dog beach) to search across multiple tags🏗️ Architecture Notes
LOWER()in SQL — searchingDogorDOGworks the same asdog📁 Files Changed
backend/app/database/images.pydb_search_images_by_tags()backend/app/routes/images.pyGET /images/searchendpointfrontend/src/api/apiEndpoints.tssearchByTagsendpoint constantfrontend/src/api/api-functions/images.tssearchImagesByTags()API functionfrontend/src/features/searchSlice.tsfrontend/src/components/Navigation/Navbar/Navbar.tsxASSIST:
i took assistance of githu copilot to code review, to solve some installation problems and fix my updated coding logics
Summary by CodeRabbit