Repository navigation
feat: edit an item's AI tags and secondary and tertiary colours - #300
Open
Mallard-Marsh wants to merge 3 commits into
Open
Mallard-Marsh wants to merge 3 commits into
Mallard-Marsh wants to merge 3 commits into
Conversation
Reviewer's GuideThis PR adds end-to-end editing of AI tags and secondary/tertiary colours: the frontend builds sparse tag diffs and safely applies them optimistically, while the backend validates and normalizes supplied vocabulary values, merges edits without data loss, tracks user changes for future re-analysis, centralizes vocabulary generation, and broadens colour preference scoring. Sequence diagram for sparse item tag editingsequenceDiagram
actor User
participant Dialog as ItemDetailDialog
participant Editor as ItemTagsEditor
participant API as PATCH /items/{id}
participant Service as ItemService
participant DB as Item tags JSON
User->>Editor: Change tag or extra colour
Editor-->>Dialog: onChange(TagDraft)
User->>Dialog: Save item
Dialog->>Dialog: diffTags()
Dialog->>API: PATCH item with changed tags only
API->>Service: update(ItemUpdate)
Service->>Service: normalize_tag_edits()
Service->>DB: Merge tag changes
Service->>DB: Append changed keys to user_edited_keys
DB-->>API: Updated item
API-->>Dialog: Item response
Dialog->>Dialog: applyItemEdits()
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
9 of 15 tasks
github-actions
Bot
added
javascript
Contributor
|
Thanks for your first pull request to Wardrowbe! Please make sure you've read the CONTRIBUTING guide, especially the PR title format section. A maintainer will review this soon. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 5 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="frontend/components/item-detail-dialog.tsx" line_range="198" />
<code_context>
const handleSave = async () => {
+ // Only the tags that differ, and none at all when nothing was touched.
+ const tagChanges = diffTags(
+ item.tags,
+ tagDraft,
</code_context>
<issue_to_address>
**New tags disappear on save**
When tagging updates the item after the editor opens and before the user saves, `diffTags` compares the stale `tagDraft` with the refreshed `item.tags`, so tags added in the meantime are sent as clears or empty lists and saving erases them, even when the user changed only a plain field.
Reconcile `tagDraft` with refreshed item tags before calculating `tagChanges`.
Also at `frontend/components/item-detail-dialog.tsx:188`, `frontend/components/item-detail-dialog.tsx:200`.
</issue_to_address>
### Comment 2
<location path="backend/app/services/item_service.py" line_range="263-266" />
<code_context>
+
+ # What the AI produced and the person changed, so "re-analyse and replace" can keep it.
+ # Only real changes count: the edit form resends type and subtype on every save.
+ edited = [
+ field
+ for field in _EDITABLE_ITEM_FIELDS
+ if field in update_data and update_data[field] != getattr(item, field)
+ ]
+
</code_context>
<issue_to_address>
**Colour representations disagree**
When a PATCH changes top-level `primary_color` or `colors` without including the matching key in `tags`, `ItemService.update` records the top-level change but does not synchronize it into the tags JSON or the corresponding colour field, so those representations disagree and readers or recommendations use stale colours.
Synchronize top-level colour changes with the corresponding values in `tags` and `colors`.
Also at `backend/app/services/item_service.py:269-295`.
</issue_to_address>
### Comment 3
<location path="backend/app/services/item_service.py" line_range="255" />
<code_context>
+ # A null (or empty) tags object means "no change". Clearing one tag is a null
+ # *inside* the object; writing None for the whole thing would wipe every tag, the
+ # AI confidence and the user-owned keys.
+ incoming = update_data.pop("tags", None) or {}
+ if not isinstance(incoming, dict):
+ incoming = incoming.model_dump(exclude_unset=True)
</code_context>
<issue_to_address>
**Cleared tags retain AI provenance**
When a PATCH clears a tag with `null` or clears a list tag with an empty list, `_has_tag_content` treats the clear as no content, so the PATCH skips the manual-tagging metadata update and the item still appears automatically tagged.
Have the PATCH provenance check recognize explicit tag clears, including empty lists.
Also at `backend/app/services/item_service.py:281-284`, `frontend/lib/tag-edits.ts:74-75`.
</issue_to_address>
### Comment 4
<location path="frontend/lib/tag-edits.ts" line_range="80" />
<code_context>
+ const colors = colorsFrom(primary, draft.extraColors);
+ const currentColors = colorsFrom(currentPrimary, draftFromTags(original).extraColors);
+ if (!sameList(colors, currentColors)) changes.colors = colors;
+ if (primary && primary !== currentPrimary) changes.primary_color = primary;
+
+ return changes;
</code_context>
<issue_to_address>
**Clearing primary colour leaves it stale**
When a user clears the primary colour while secondary colours are present, `diffTags` omits `primary_color` when the form has cleared it, while it can still send a changed `colors` list without the old primary. `handleSave` also turns the empty form value into `undefined`, so the server retains the old primary colour while the colour list no longer contains it.
Represent a cleared primary colour as `null` in the save payload and have `diffTags` include `primary_color: null` when the primary changes to empty.
</issue_to_address>
### Comment 5
<location path="frontend/components/item-tags-editor.tsx" line_range="171-177" />
<code_context>
+ onChange={(pattern) => onChange({ ...draft, pattern })}
+ />
+ <TagSelect
+ label={t('material')}
+ noneLabel={t('none')}
+ value={draft.material}
+ options={labelled(MATERIAL_VALUES, materialLabel)}
+ describe={materialLabel}
+ onChange={(material) => onChange({ ...draft, material })}
+ />
+ <TagSelect
+ label={t('formality')}
</code_context>
<issue_to_address>
**Custom materials cannot be edited**
When a user needs to set a material value that is not in the predefined materials list, `ItemTagsEditor` only offers `MATERIAL_VALUES` in a select, so a user cannot enter or change a free-text material such as “100% linen”; existing custom values remain visible but can only be left as-is or replaced with one of the preset options.
Use an editable text input with the predefined materials as suggestions, or otherwise let users submit arbitrary material values.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 5 findings to address first, and a bad save can persist incorrect tags or colours in the item record and affect future recommendations; reverting the code will not restore those records. The impact is bounded to edited items and can be corrected by editing them again.
Blocking findings: frontend/components/item-detail-dialog.tsx:198, backend/app/services/item_service.py:266, backend/app/services/item_service.py:255, frontend/lib/tag-edits.ts:80, frontend/components/item-tags-editor.tsx:177
The item editor only covered name, type, subtype, brand, primary colour,
notes and wash interval. Pattern, material, formality, fit, style, season and
extra colours were read-only chips, so a wrong tag (a herringbone blazer
tagged "solid") could not be fixed on screen.
Editor (components/item-tags-editor.tsx, mounted in the edit form):
- selects for pattern, material, formality and fit, each with a None option
that clears the tag
- toggle chips for style and season
- secondary and tertiary colour selects after the existing primary colour
field (which keeps its eyedropper); colours past the third are preserved
- only the tags that differ are sent, and none at all when nothing changed
Backend:
- The pattern, style, season and fit lists move into
garment_vocabulary.json next to the other vocabulary, are generated for the
frontend, and fill new <<PATTERNS>> <<STYLES>> <<SEASONS>> <<FITS>> prompt
tokens. The rendered tagging prompt is byte-identical to before.
- PATCH validates the pattern, formality, fit, style and season values it is
given (422 for unknown ones) and stores them stripped and lower-cased. Only
keys in the request are checked, so an item that still holds an older
off-list value can be saved. Material stays free text and colours stay
lenient, as before.
- A null or empty `tags` object is now a no-op. It used to set the whole tags
JSON to NULL, dropping the AI confidence and the user-owned keys; clearing
one tag is a null inside the object.
- The server records the AI-produced keys a person changed in
tags.user_edited_keys (from the tags object and from type, subtype,
primary_color and colors), ignoring a value sent by the client. Nothing
reads it yet; it is for a later "re-analyse and replace".
- The colour preference score looks at every colour on the item, not only
the primary.
Edit form: changing the type drops a subtype that belongs to a different
type's list ("button-down" after a T-shirt becomes a polo) and keeps one that
fits the new type or that the app has no list for. The optimistic update now
merges edited tags instead of replacing the whole tags object.
No new database column and no migration: fit stays in the tags JSON.
Tests cover validation and cleaning, the null and empty no-ops, clearing a
tag and its column, edit tracking from both paths, the scorer, the prompt
tokens, the diffing helpers, the editor controls and the full edit dialog
(including that saving a plain field sends no tags).
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
CI type-checks the test files with tsc. The check read a property the Item type does not declare. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…rs in step From review: - The edit dialog compares the form with the item as it was when editing began and sends only the fields and tags that changed. Before, tags the AI added while the form was open were sent back as clears, and untouched fields were resent (and recorded as edits). Nothing changed means no request. - ItemService.update locks the row first and compares edits against it, not against the item loaded before the lock, so a re-analysis committing in between cannot slip past edit tracking. - A primary colour or colour list sent outside `tags` now reaches the tags JSON, and a new primary colour replaces the old one at the front of the colour list (Anyesh#294 for any client). - A style or season list may keep an off-list value the item already has, so an old value left switched on no longer makes the save fail with 422; a new off-list value is still rejected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Mallard-Marsh
force-pushed
the
pr/editable-tags
branch
from
October 28, 2026 07:04
ea7a42f to
494c9ea
Compare
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
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.
Description
Lets people edit an item's AI tags in the item editor. Until now only name, type, subtype, brand, primary colour, notes and wash interval were editable; pattern, material, formality, fit, style, season and the extra colours were read-only chips, so a wrong tag (a herringbone blazer tagged "solid") could not be fixed.
Editor (
components/item-tags-editor.tsx, mounted in the existing edit form)tagsat all when nothing changedBackend
garment_vocabulary.jsonwith the rest of the vocabulary, are generated for the frontend, and fill new<<PATTERNS>>,<<STYLES>>,<<SEASONS>>,<<FITS>>prompt tokens. The rendered tagging prompt is byte-identical to before.PATCH /items/{id}validates the pattern, formality, fit, style and season values it is given (422 for unknown ones) and stores them trimmed and lower-cased. Only keys in the request are checked, so an item holding an older off-list value can still be saved. Material stays free text and colours stay lenient, as before.nullor emptytagsobject is now a no-op. It used to set the whole tags JSON to NULL, dropping the AI confidence and the user-owned keys (size,care_instructions,source_url); clearing one tag is a null inside the object.tags.user_edited_keys(ignoring any value a client sends). Nothing in this PR reads it; it lets a later "re-analyse and replace" keep hand edits (see the follow-up PR).Edit form: changing the type drops a subtype that belongs to a different type's list and keeps one that fits the new type or that the app has no list for. The optimistic update merges edited tags instead of replacing the tags object. Changing the primary colour also updates the colour list (primary first, the rest kept), so suggestions and pairings stop using the old colour.
No new database column and no migration:
fitstays in the tags JSON.Related Issue
Fixes #270
Fixes #294 (editing an item's colour now updates its colour list; covered by
keeps the colour list and primary colour in step when the primary changesintests/tag-edits.test.ts)Type of Change
Checklist
Testing
Test Environment
Tests Performed
ruff check,ruff format --check, fullpytest(rebased on currentmain). Newtests/test_item_tag_edits.pycovers validation and cleaning, the null and empty no-ops, clearing a tag and its column, edit tracking from both paths; plus scorer and prompt-token tests. On macOS 4 tests fail exactly as they do onmain(2 intest_tagging_durability, 2[utc]timezone cases, which are filesystem-case related); CI runs on Linux.tsc --noEmit,vitest(743 passing),npm run i18n:check,npm run vocab:check. New tests for the diffing helpers, the editor controls and the full edit dialog (including that saving a plain field sends no tags).Additional Notes
tags.user_edited_keysand will be opened right after this one; it contains these commits until this one is merged.Summary by Sourcery
Enable people to correct AI-generated item tags and colours through the item editor while preserving unrelated metadata and supporting safe partial updates.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores: