Repository navigation
feat: split a photo with several garments into items - #302
Mallard-Marsh wants to merge 3 commits into
Conversation
Reviewer's GuideThis PR adds an on-demand, owner-only workflow that uses one AI vision call to detect garments, lets users edit normalized crop boxes in a touch-capable dialog, creates and tags one item per confirmed crop with transactional cleanup, and archives the original item as restorable; it includes backend, frontend, localization, and extensive unit/integration coverage. Sequence diagram for detecting and splitting garmentssequenceDiagram
actor User
participant Dialog as SplitPhotoDialog
participant API as ItemsAPI
participant AI as AIService
participant Image as ImageService
participant Items as ItemService
participant Queue as TaggingQueue
User->>Dialog: Open split photo
Dialog->>API: POST /items/{id}/detect-garments
API->>AI: detect_garments(image_path)
AI-->>API: garment types, colors, normalized boxes
API-->>Dialog: DetectedGarments
User->>Dialog: Move, resize, add, or remove boxes
User->>Dialog: Create N items
Dialog->>API: POST /items/{id}/split
loop Each confirmed box
API->>Image: crop_to_jpeg(box)
API->>Image: process_and_store(crop)
API->>Items: create(item)
end
API->>Items: archive(source, split)
API->>Queue: enqueue tag_item_image for each new item
API-->>Dialog: source_id and item_ids
Dialog-->>User: Show success and close
Flow diagram for transactional photo splittingflowchart TD
A["Confirmed 1-8 garment boxes"] --> B["Crop stored photo with padding"]
B --> C["process_and_store crop"]
C --> D["Create new item"]
D --> E{More boxes?}
E -->|Yes| B
E -->|No| F["Archive source with reason split"]
F --> G["Commit transaction"]
G --> H["Queue normal tagging"]
C -->|Failure| I["Rollback database changes"]
I --> J["Delete already stored crop files"]
J --> K["No items created and source unchanged"]
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="backend/app/api/split.py" line_range="130" />
<code_context>
+ """
+ item_service = ItemService(db)
+ source = await _owned_item(item_service, item_id, current_user)
+ if source.is_archived:
+ raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="Item is archived")
+
</code_context>
<issue_to_address>
**Archived source becomes ready**
When the source has an active tagging job when the split commits, `tag_item_image` writes `ItemStatus.ready` after the source is archived, leaving the archived source with a ready status that status-based readers or analytics see as active.
Prevent the tagging success path from changing the status of an archived item.
Also at `backend/app/api/split.py:160-161`.
</issue_to_address>
### Comment 2
<location path="backend/app/api/split.py" line_range="161" />
<code_context>
+ )
+ )
+ await item_service.archive(source, SPLIT_ARCHIVE_REASON)
+ await db.commit()
+ except Exception:
+ await db.rollback()
</code_context>
<issue_to_address>
**Split can queue duplicate tagging jobs**
When a bulk-analyze request sees the new children after the split commit but before their tagging job IDs are recorded, the committed children are visible as `processing` with no `ai_job_id` before `_start_tagging` finishes. A bulk-analyze request in another tab treats those rows as eligible and queues jobs too; the split handler then queues its own jobs and overwrites the recorded IDs, leaving duplicate workers and an untracked job per affected item.
Atomically reserve each child for tagging before it becomes eligible for another enqueue, or use a shared job-claim mechanism for split and analyze requests.
Also at `backend/app/api/split.py:191-198`.
</issue_to_address>
### Comment 3
<location path="backend/app/services/garment_detector.py" line_range="130" />
<code_context>
+
+ # The prompt asks for percentages, but a model may answer in 0-1 fractions; a box that is
+ # really under 1% of the photo is not a garment, so "everything is at most 1" means fractions.
+ scale = 1.0 if parsed and max(max(e) for _, _, e in parsed) <= 1.0 else 100.0
+
+ garments: list[DetectedGarment] = []
</code_context>
<issue_to_address>
**Fraction boxes near edges disappear**
When the model returns fractional coordinates for a box that slightly extends beyond an edge, `parse_detection` treats any reply with a coordinate above 1 as percentages. A fractional box that slightly overflows the photo is therefore divided by 100 before clamping, making its sides fall below `MIN_BOX_SIDE` and dropping the garment instead of clamping it to the photo edge.
Determine the coordinate format without treating a small fractional overflow as a switch to percentage units, then clamp the box.
</issue_to_address>
### Comment 4
<location path="backend/tests/test_split_photo.py" line_range="103-104" />
<code_context>
+ def test_colours_are_normalised_and_deduplicated(self):
+ [g] = parse_detection(_reply(_g(0, 0, 50, 50, colors=("Blue", "blue", 3, "nonsense"))))
+
+ assert g.colors.count("blue") <= 1
+ assert all(isinstance(c, str) for c in g.colors)
+
+ @pytest.mark.parametrize(
</code_context>
<issue_to_address>
**Empty colors pass normalization test**
When the parser drops all colors for the mixed-color input, `test_colours_are_normalised_and_deduplicated` accepts an empty color list: `count("blue") <= 1` is true and `all(...)` is vacuously true, so dropping the valid `"Blue"` color passes despite the test input containing it.
Assert that the normalized result is exactly `["blue"]`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 4 findings to address first, and a split creates new item records and crop files, then archives the source photo. If the split is wrong, the source can be restored and the new items removed, but reverting the code will not undo records already created.
Blocking findings: backend/app/api/split.py:130, backend/app/api/split.py:161, backend/app/services/garment_detector.py:130, backend/tests/test_split_photo.py:104
POST /items/{id}/detect-garments asks the vision model for each garment's box (writes nothing);
POST /items/{id}/split crops the stored photo into one new item per confirmed box, queues normal
tagging, and archives the original. All or nothing, owner only, duplicate check skipped.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A Split photo button in the item dialog opens a dialog that asks the AI where the garments are, draws an editable box round each (pointer events, so touch works), and creates one item per box. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
From review: - An item archived while its tagging job was still running (the source of a split) was set back to ready when the job finished. The worker now leaves an archived item's status alone and only fills in its tags. - A reply in 0-1 fractions with a box running slightly past an edge (right: 1.03) was read as percentages and the garment dropped. Coordinates up to 2 are now read as fractions; a percentage box that small would be a speck either way. - The colour-normalisation test now checks the exact result instead of passing on an empty list. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
d805015 to
a839907
Compare
Description
Splits a photo that contains several garments into separate items, on demand. Bulk upload creates one item per file and the tagging prompt describes the main garment, so a photo of three tops became one blended item (#284). This adds a way to fix that from the item itself.
Item dialog -> Split photo (scissors button next to the existing photo tools)
split(restorable from the archive, not deleted).Backend
POST /items/{id}/detect-garments: owner only, rate-limited (10/min), one vision call, writes nothing. Returns each garment's type, colours and box (fractions of the photo). No garments found is an empty list; AI disabled is 503; AI unreachable is 502.POST /items/{id}/splitwith 1-8 confirmed boxes: crops the stored photo (already upright, so boxes line up) once per box with a little padding, stores each crop through the normalprocess_and_store, creates the items, queues normal tagging after the commit, and archives the source. All or nothing: if any crop fails, the files already written are removed and nothing changes. The upload duplicate check is skipped on purpose (a white tee and a cream tee look alike).services/garment_detector.py(parses the model's reply: clamps boxes into the photo, drops specks and near-duplicates, caps at 8, tolerates messy JSON and either percentages or fractions),prompts/garment_detection.txt,api/split.py,ImageService.crop_to_jpeg,AIService.detect_garments.archive_reasonis free text) and no new dependency (Pillow crops).Frontend:
components/split-photo-dialog.tsx,lib/split-boxes.ts(pure box maths),lib/hooks/use-split.ts, one button and one mount initem-detail-dialog.tsx. Strings in all 8 locales.Related Issue
Fixes #284 (the on-demand part). Detecting several garments during bulk upload (a "my photos may contain several garments" option and a review list) is not included; I left it out because it touches the upload dialog and manager that #277 and #283 rework, and would rather do it after those settle if you want it.
Type of Change
Checklist
Testing
Test Environment
Tests Performed
ruff, fullpyteston currentmain(same 4 macOS-only failures asmain). 47 new tests: the reply parser (percent/fraction boxes, list boxes, clamping, swapped corners, specks, duplicates, the cap, unknown types, colour normalisation, junk replies), the crop (padding, edges, missing file), and both endpoints with the AI mocked (N items created and queued, source archived, owner only, duplicate check skipped, rollback with files removed on a failure part-way, box validation, AI off/unreachable, rate limit, auth).tsc,vitest,i18n:check,vocab:check. 24 new tests: box maths, and the dialog (detection once, boxes positioned as percentages, remove/add up to the limit, drag to move, corner resize, create sends the current boxes, failure keeps the dialog open, retry after a failed detection).gpt-4.1-mini, read-only: collages built from real wardrobe photos found 3/3 tops side by side and 4/4 garments in a 2x2 grid; on 20 single-garment photos it returned one box each, except a jeans photo where it also boxed the shoes at the hem. Boxes are approximate (they vary by roughly 10% of the photo between runs), which is why they are editable.Additional Notes
Not checked: dragging on a phone's touch screen against this exact branch (it is the same code as the version I use day to day).
Summary by Sourcery
Enable users to split a photo containing multiple garments into separately tagged wardrobe items.
New Features:
Bug Fixes:
Enhancements:
Tests:
Chores: