From 179c1a9dcc011ebdb9f034aa9a45cf2be438c9ef Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 28 Jun 2026 09:49:05 -0400 Subject: [PATCH] feat(suggestions): visible, reversible rejection in the modal rail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A red-✗ dismissal no longer makes the suggestion vanish. The rejected tag stays in the rail — dimmed, struck-through, with a "rejected" pill and a one-click undo (↶) in place of the ✗ — so a misclick is recoverable and the operator can see what they've said no to (operator-asked 2026-06-27). Backend: SuggestionService.for_image now KEEPS rejected tags, flagged rejected=True, sorted to the bottom of their category, instead of dropping them. New AllowlistService.undismiss + POST /suggestions/undismiss clears the TagSuggestionRejection. Rejected items are still excluded from bulk consensus (for_selection) and the type-to-add dropdown, whose jobs are unchanged. Frontend: store.dismiss flags in place (canonical tags) rather than dropping; new store.undismiss reverts. SuggestionItem renders the rejected state and swaps ✗→↶; ✓ still accepts (which clears the rejection server-side). Tests: rejected-surfaced-flagged-then-reversible (service) + undismiss endpoint idempotency (API). Completes #1134's reversible-rejection half. Heads-as-suggestion-source is the remaining piece. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01Ttrj5P7upUTueSfoJcxEqa --- backend/app/api/suggestions.py | 19 ++++++++ backend/app/services/ml/allowlist.py | 6 +++ backend/app/services/ml/suggestions.py | 29 +++++++++--- .../src/components/modal/SuggestionItem.vue | 47 +++++++++++++++++-- .../modal/SuggestionsCategoryGroup.vue | 3 +- .../src/components/modal/SuggestionsPanel.vue | 4 +- .../src/components/modal/TagAutocomplete.vue | 4 ++ frontend/src/stores/suggestions.js | 46 ++++++++++++++---- tests/test_api_suggestions.py | 19 ++++++++ tests/test_ml_suggestions.py | 34 ++++++++++++++ 10 files changed, 188 insertions(+), 23 deletions(-) diff --git a/backend/app/api/suggestions.py b/backend/app/api/suggestions.py index 46fa2e5..7a5ca1d 100644 --- a/backend/app/api/suggestions.py +++ b/backend/app/api/suggestions.py @@ -61,6 +61,10 @@ async def get_suggestions(image_id: int): # modal's "Treat as alias"/"Remove alias" affordances. "raw_name": s.raw_name, "via_alias": s.via_alias, + # operator dismissed this tag for this image — surfaced + # (not dropped) so the rail can show it rejected + offer + # one-click un-reject. + "rejected": s.rejected, } for s in items ] @@ -131,6 +135,21 @@ async def dismiss_suggestion(image_id: int): return "", 204 +@suggestions_bp.route( + "/images//suggestions/undismiss", methods=["POST"] +) +async def undismiss_suggestion(image_id: int): + """Reverse a per-image dismissal (reject-recovery). Idempotent — undoing a + tag that isn't rejected is a no-op delete.""" + body = await request.get_json() + if not body or "tag_id" not in body: + return jsonify({"error": "tag_id required"}), 400 + async with get_session() as session: + await AllowlistService(session).undismiss(image_id, body["tag_id"]) + await session.commit() + return "", 204 + + @suggestions_bp.route("/suggestions/bulk", methods=["POST"]) async def bulk_suggestions(): body = await request.get_json() diff --git a/backend/app/services/ml/allowlist.py b/backend/app/services/ml/allowlist.py index 8f7d14e..857d665 100644 --- a/backend/app/services/ml/allowlist.py +++ b/backend/app/services/ml/allowlist.py @@ -89,6 +89,12 @@ class AllowlistService: ) await self.session.execute(stmt) + async def undismiss(self, image_id: int, tag_id: int) -> None: + """Undo a per-image dismissal — drop the TagSuggestionRejection so the + suggestion reverts to a live (un-rejected) state. Backs the rail's + one-click reject-recovery (operator-asked 2026-06-27).""" + await self._clear_rejection(image_id, tag_id) + async def reject_applied_tag(self, image_id: int, tag_id: int) -> None: """Operator removed an applied tag from an image. Remove the image_tag row AND record a rejection so the allowlist won't diff --git a/backend/app/services/ml/suggestions.py b/backend/app/services/ml/suggestions.py index 7b0f996..43a488c 100644 --- a/backend/app/services/ml/suggestions.py +++ b/backend/app/services/ml/suggestions.py @@ -39,6 +39,11 @@ class Suggestion: # via_alias = this suggestion was surfaced because an operator alias remapped # the raw prediction to this canonical tag. Lets the UI mark it + offer undo. via_alias: bool = False + # rejected = the operator dismissed this tag for this image (a stored + # TagSuggestionRejection). It stays in the list — flagged, not dropped — so + # the rejection is VISIBLE and REVERSIBLE in the rail (misclick recovery, + # operator-asked 2026-06-27) instead of silently vanishing or re-suggesting. + rejected: bool = False @dataclass @@ -174,12 +179,15 @@ class SuggestionService: # augmentation, so it's always the first writer for a key. raw_name=existing.raw_name, via_alias=existing.via_alias, + # rejected is a property of the tag_id, so both writers for a + # key agree — preserve it through the higher-score rebuild. + rejected=existing.rejected, ) for raw, display, category, conf in candidates: canonical = alias_map.get((raw, category)) if canonical is not None: - if canonical.id in applied or canonical.id in rejected: + if canonical.id in applied: continue _merge( canonical.id, @@ -192,6 +200,7 @@ class SuggestionService: creates_new_tag=False, raw_name=raw, via_alias=True, + rejected=canonical.id in rejected, ), ) else: @@ -209,10 +218,7 @@ class SuggestionService: ) ).scalars().first() if existing_tag is not None: - if ( - existing_tag.id in applied - or existing_tag.id in rejected - ): + if existing_tag.id in applied: continue _merge( existing_tag.id, @@ -225,6 +231,7 @@ class SuggestionService: creates_new_tag=False, raw_name=raw, via_alias=False, + rejected=existing_tag.id in rejected, ), ) else: @@ -247,7 +254,7 @@ class SuggestionService: for hit in hits: if hit.similarity < settings.centroid_similarity_threshold: continue - if hit.tag_id in applied or hit.tag_id in rejected: + if hit.tag_id in applied: continue tag = await self.session.get(Tag, hit.tag_id) if tag is None: @@ -263,6 +270,7 @@ class SuggestionService: score=hit.similarity, source="centroid", creates_new_tag=False, + rejected=hit.tag_id in rejected, ), ) @@ -270,7 +278,9 @@ class SuggestionService: for sug in merged.values(): result.by_category.setdefault(sug.category, []).append(sug) for cat in result.by_category: - result.by_category[cat].sort(key=lambda s: s.score, reverse=True) + # Live suggestions first (by score), rejected ones sink to the + # bottom of the category — visible for recovery, out of the way. + result.by_category[cat].sort(key=lambda s: (s.rejected, -s.score)) return result async def for_selection( @@ -297,6 +307,11 @@ class SuggestionService: for s in items: if s.canonical_tag_id is None or s.creates_new_tag: continue + # for_image now keeps rejected tags (flagged) for the rail; + # bulk consensus must still ignore them — a tag dismissed on + # an image isn't a suggestion for that image. + if s.rejected: + continue st = stats.get(s.canonical_tag_id) if st is None: st = { diff --git a/frontend/src/components/modal/SuggestionItem.vue b/frontend/src/components/modal/SuggestionItem.vue index c99107a..b9df764 100644 --- a/frontend/src/components/modal/SuggestionItem.vue +++ b/frontend/src/components/modal/SuggestionItem.vue @@ -3,10 +3,12 @@ name, score, and action buttons as one "object" (operator-asked 2026-06-01). The row itself is informational; the green ✓ / red ✗ verdict pair + 3-dot alias menu are the action affordances. --> -
+
{{ suggestion.display_name }} - rejected + + new alias @@ -17,7 +19,8 @@ for this image (records a TagSuggestionRejection — a hard negative the heads train on). Together they occupy ~the footprint of the old single Accept pill, so rejecting is now a one-click peer of accepting rather - than buried in the kebab. --> + than buried in the kebab. When the row is already rejected the ✗ swaps + to an undo (↶) so the rejection is reversible in place. -->
+
@@ -33,7 +34,7 @@ const props = defineProps({ collapsible: { type: Boolean, default: false }, defaultOpen: { type: Boolean, default: true } }) -defineEmits(['accept', 'alias', 'remove-alias', 'dismiss']) +defineEmits(['accept', 'alias', 'remove-alias', 'dismiss', 'undismiss']) const open = ref(props.collapsible ? props.defaultOpen : true) diff --git a/frontend/src/components/modal/SuggestionsPanel.vue b/frontend/src/components/modal/SuggestionsPanel.vue index ffa2804..e62fc8b 100644 --- a/frontend/src/components/modal/SuggestionsPanel.vue +++ b/frontend/src/components/modal/SuggestionsPanel.vue @@ -21,14 +21,14 @@ v-show="store.byCategory[cat] && store.byCategory[cat].length" :label="labelFor(cat)" :items="store.byCategory[cat] || []" @accept="onAccept" @alias="onAlias" @remove-alias="onRemoveAlias" - @dismiss="store.dismiss" + @dismiss="store.dismiss" @undismiss="store.undismiss" />
diff --git a/frontend/src/components/modal/TagAutocomplete.vue b/frontend/src/components/modal/TagAutocomplete.vue index ba6fe25..43ab0e1 100644 --- a/frontend/src/components/modal/TagAutocomplete.vue +++ b/frontend/src/components/modal/TagAutocomplete.vue @@ -202,6 +202,10 @@ const suggestionHits = computed(() => { const out = [] for (const list of Object.values(suggestions.allByCategory)) { for (const s of list || []) { + // Rejected suggestions now stay in allByCategory (flagged) so the panel + // can show + un-reject them; keep them OUT of the type-to-add dropdown, + // whose job is finding a tag to ADD (un-reject lives in the panel). + if (s.rejected) continue const key = `${s.category}:${s.display_name.toLowerCase()}` if (!s.display_name.toLowerCase().includes(q)) continue if (seen.has(key)) continue diff --git a/frontend/src/stores/suggestions.js b/frontend/src/stores/suggestions.js index 36c926a..b7dd6c5 100644 --- a/frontend/src/stores/suggestions.js +++ b/frontend/src/stores/suggestions.js @@ -84,6 +84,19 @@ export const useSuggestionsStore = defineStore('suggestions', () => { } } + // Flip the `rejected` flag on the matching suggestion in BOTH lists in place, + // so a reject/un-reject shows immediately without dropping the row (visible, + // reversible rejection — misclick recovery, operator-asked 2026-06-27). + function _setRejectedEverywhere(suggestion, value) { + const key = _keyOf(suggestion) + const cat = suggestion.category + for (const map of [byCategory.value, allByCategory.value]) { + for (const s of map[cat] || []) { + if (_keyOf(s) === key) s.rejected = value + } + } + } + async function accept(suggestion) { // Capture imageId so a mid-flight prev/next can't reroute the // accept POST to a different image AND push the tag to that @@ -168,21 +181,36 @@ export const useSuggestionsStore = defineStore('suggestions', () => { async function dismiss(suggestion) { const imageId = currentImageId if (imageId == null) return - // Dismiss needs a tag_id; raw tags have none, so dismissing a raw - // suggestion just hides it client-side (nothing to persist a rejection - // against until the tag exists). - if (suggestion.canonical_tag_id != null) { - await api.post(`/api/images/${imageId}/suggestions/dismiss`, { - body: { tag_id: suggestion.canonical_tag_id } - }) + // Dismiss needs a tag_id. Raw tags (creates_new_tag) have none, so there's + // nothing to persist a rejection against — drop them client-side as before. + // Canonical tags persist a rejection and STAY in the list flagged rejected, + // so the operator can see it and one-click un-reject (misclick recovery). + if (suggestion.canonical_tag_id == null) { + if (currentImageId === imageId) _dropEverywhere(suggestion) + return } + await api.post(`/api/images/${imageId}/suggestions/dismiss`, { + body: { tag_id: suggestion.canonical_tag_id } + }) if (currentImageId === imageId) { - _dropEverywhere(suggestion) + _setRejectedEverywhere(suggestion, true) + } + } + + // Undo a per-image dismissal — the suggestion reverts to a live row. + async function undismiss(suggestion) { + const imageId = currentImageId + if (imageId == null || suggestion.canonical_tag_id == null) return + await api.post(`/api/images/${imageId}/suggestions/undismiss`, { + body: { tag_id: suggestion.canonical_tag_id } + }) + if (currentImageId === imageId) { + _setRejectedEverywhere(suggestion, false) } } return { byCategory, allByCategory, loading, error, - load, loadAll, accept, aliasAccept, removeAlias, dismiss + load, loadAll, accept, aliasAccept, removeAlias, dismiss, undismiss } }) diff --git a/tests/test_api_suggestions.py b/tests/test_api_suggestions.py index 2b1b425..165b43e 100644 --- a/tests/test_api_suggestions.py +++ b/tests/test_api_suggestions.py @@ -95,6 +95,25 @@ async def test_dismiss(client, db): assert resp.status_code == 204 +@pytest.mark.asyncio +async def test_undismiss_reverses_rejection(client, db): + img = await _img(db, {}) + tag = await TagService(db).find_or_create("UndismissMe", TagKind.general) + await db.commit() + await client.post( + f"/api/images/{img.id}/suggestions/dismiss", json={"tag_id": tag.id} + ) + resp = await client.post( + f"/api/images/{img.id}/suggestions/undismiss", json={"tag_id": tag.id} + ) + assert resp.status_code == 204 + # Idempotent: un-rejecting again (nothing to clear) is still a 204. + resp2 = await client.post( + f"/api/images/{img.id}/suggestions/undismiss", json={"tag_id": tag.id} + ) + assert resp2.status_code == 204 + + @pytest.mark.asyncio async def test_alias_requires_fields(client, db): img = await _img(db, {}) diff --git a/tests/test_ml_suggestions.py b/tests/test_ml_suggestions.py index 3f532bd..fbab396 100644 --- a/tests/test_ml_suggestions.py +++ b/tests/test_ml_suggestions.py @@ -3,6 +3,7 @@ import pytest from backend.app.models import ImageRecord, TagKind from backend.app.models.tag import image_tag from backend.app.services.ml.aliases import AliasService +from backend.app.services.ml.allowlist import AllowlistService from backend.app.services.ml.suggestions import SuggestionService from backend.app.services.tag_service import TagService @@ -147,3 +148,36 @@ async def test_applied_tag_not_suggested(db): ) sl = await SuggestionService(db).for_image(img.id) assert "character" not in sl.by_category or not sl.by_category["character"] + + +@pytest.mark.asyncio +async def test_rejected_tag_surfaced_flagged_then_reversible(db): + # A dismissed suggestion is NOT dropped: it stays in the list flagged + # rejected=True so the rail can show it + offer one-click un-reject + # (visible, reversible rejection — operator-asked 2026-06-27). A live + # suggestion sorts ahead of the rejected one. + tags = TagService(db) + rejected_tag = await tags.find_or_create("rejectme", TagKind.general) + img = await _seed_img( + db, + "f" * 64, + { + "rejectme": {"category": "general", "confidence": 0.96}, + "keepme": {"category": "general", "confidence": 0.90}, + }, + ) + await AllowlistService(db).dismiss(img.id, rejected_tag.id) + + sl = await SuggestionService(db).for_image(img.id) + general = sl.by_category["general"] + by_name = {s.display_name: s for s in general} + assert by_name["Rejectme"].rejected is True + assert by_name["Keepme"].rejected is False + # Live suggestions sort ahead of rejected ones regardless of score. + assert general[-1].display_name == "Rejectme" + + # Un-reject reverts it to a live suggestion. + await AllowlistService(db).undismiss(img.id, rejected_tag.id) + sl2 = await SuggestionService(db).for_image(img.id) + by_name2 = {s.display_name: s for s in sl2.by_category["general"]} + assert by_name2["Rejectme"].rejected is False