Reindex Card into ES on printing/tag consensus transitions

MPCAutofill/cardpicker/documents.py

@@ -1,3 +1,5 @@
+import logging
+
 from django_elasticsearch_dsl import Document, fields
 from django_elasticsearch_dsl.registries import registry
 from elasticsearch_dsl import analyzer
@@ -6,6 +8,8 @@
 
 from cardpicker.models import Card
 
+logger = logging.getLogger(__name__)
+
 # custom elasticsearch analysers are configured here to add the `asciifolding` filter, which handles accents:
 # https://www.elastic.co/guide/en/elasticsearch/reference/7.17/analysis-asciifolding-tokenfilter.html
 # https://www.elastic.co/guide/en/elasticsearch/reference/current/analysis-standard-analyzer.html
@@ -52,3 +56,19 @@ def get_queryset(self) -> QuerySet[Card]:
                 "inferred_canonical_card__expansion",
             )
         )
+
+
+def reindex_card_safely(card: Card) -> None:
+    """
+    Pushes `card`'s current state into the Elasticsearch index, catching and logging any
+    failure rather than raising. Postgres is the source of truth for vote/consensus state -
+    by the time this runs, that write has already committed - so a search-index hiccup (ES
+    down, a transient connection error, etc.) must never break vote submission or roll back
+    a DB write that already succeeded. Shared by every vote-consensus module that needs to
+    push a single card's change into the index immediately, rather than waiting for the next
+    scheduled `update_database` re-scan or a manual `search_index --rebuild`.
+    """
+    try:
+        CardSearch().update([card], action="index")
+    except Exception:
+        logger.exception("Failed to reindex card %s into Elasticsearch after a vote-consensus update", card.identifier)

MPCAutofill/cardpicker/printing_consensus.py

@@ -106,6 +106,19 @@ def resolve_printing(card: Card) -> CanonicalCard | Literal["NO_MATCH"] | None:
     return printings_by_id[winning_key]
 
 
+def _effective_indexed_printing_id(status: str, printing_id: int | None) -> int | None:
+    """
+    The printing id that actually reaches Elasticsearch for a card in `status`: `Card.
+    get_expansion_code`/`get_collector_number` (the fields `documents.py` indexes) only fall
+    back to `inferred_canonical_card` while `printing_tag_status == RESOLVED` - a card that's
+    UNRESOLVED or NO_MATCH is indexed as if `inferred_canonical_card` were `None`, regardless
+    of what's actually stored there. Comparing this derived value (not the raw fields) before
+    and after a consensus run is what lets a status flip with no printing change, or a
+    printing change with no status flip, both correctly count as "the index needs updating."
+    """
+    return printing_id if status == PrintingTagStatus.RESOLVED else None
+
+
 def resolve_and_persist_printing(card: Card) -> CanonicalCard | Literal["NO_MATCH"] | None:
     """
     Runs `resolve_printing(card)` and writes the outcome onto `card.inferred_canonical_card`
@@ -116,7 +129,20 @@ def resolve_and_persist_printing(card: Card) -> CanonicalCard | Literal["NO_MATC
     vote is submitted for `card` - cheap, since it only touches this one card's own votes.
     Returns the same outcome `resolve_printing` returned, so callers don't need to
     recompute it again immediately afterwards.
+
+    Also pushes `card` into Elasticsearch, but only when the outcome actually changes what's
+    indexed (see `_effective_indexed_printing_id`) - entering RESOLVED, leaving RESOLVED
+    (contested/unresolved again after new votes), or the resolved printing itself changing
+    while remaining RESOLVED. A re-resolve that lands on the same outcome as before (the
+    common case whenever this runs against a card that already has a settled consensus) does
+    not touch the index. The push itself is failure-isolated (`reindex_card_safely`) - an ES
+    hiccup is logged, never raised; this function's own DB write has already committed by
+    that point regardless.
     """
+    prior_status = card.printing_tag_status
+    prior_printing_id = card.inferred_canonical_card_id
+    prior_effective = _effective_indexed_printing_id(prior_status, prior_printing_id)
+
     result = resolve_printing(card)
     if result is None:
         card.inferred_canonical_card = None
@@ -128,6 +154,15 @@ def resolve_and_persist_printing(card: Card) -> CanonicalCard | Literal["NO_MATC
         card.inferred_canonical_card = result
         card.printing_tag_status = PrintingTagStatus.RESOLVED
     card.save(update_fields=["inferred_canonical_card", "printing_tag_status"])
+
+    new_effective = _effective_indexed_printing_id(card.printing_tag_status, card.inferred_canonical_card_id)
+    if new_effective != prior_effective:
+        from cardpicker.documents import (
+            reindex_card_safely,  # local import - avoids a top-level ES dependency in this module
+        )
+
+        reindex_card_safely(card)
+
     return result
 
 

MPCAutofill/cardpicker/tag_consensus.py

@@ -54,9 +54,12 @@ def resolve_and_persist_tag_votes(card: Card) -> None:
     rather than a single outcome), and merges the result directly into `card.tags`: a resolved
     APPLY adds the tag name if not already present; a resolved NOT_APPLICABLE removes it if
     present. Saves `card.tags` and pushes the change into Elasticsearch immediately - unlike
-    printing/artist consensus (whose denormalised fields aren't ES-indexed), `tags` *is* an
-    ES-indexed field (`documents.py`'s `KeywordField`), so a vote-triggered change has to reach
-    the search index directly rather than waiting for the next scheduled re-scan.
+    artist consensus (whose denormalised fields aren't ES-indexed), `tags` *is* an ES-indexed
+    field (`documents.py`'s `KeywordField`), so a vote-triggered change has to reach the
+    search index directly rather than waiting for the next scheduled re-scan. Only fires when
+    `tags_changed` is actually true, and the push itself is failure-isolated
+    (`reindex_card_safely`) - same rationale and mechanism as
+    `printing_consensus.resolve_and_persist_printing`'s equivalent hook.
 
     Also writes `card.tag_vote_statuses` (a JSONField, not ES-indexed, so no re-index needed
     for this part alone): for every voted tag, one of RESOLVED_APPLY/RESOLVED_REJECT/CONTESTED/
@@ -65,7 +68,7 @@ def resolve_and_persist_tag_votes(card: Card) -> None:
     unresolved means only one side has voted so far, or thresholds simply aren't cleared yet.
     """
     from cardpicker.documents import (
-        CardSearch,  # local import - avoids a top-level ES dependency in this module
+        reindex_card_safely,  # local import - avoids a top-level ES dependency in this module
     )
 
     votes_by_tag_id: dict[int, set[int]] = defaultdict(set)
@@ -107,7 +110,7 @@ def resolve_and_persist_tag_votes(card: Card) -> None:
     if update_fields:
         card.save(update_fields=update_fields)
         if tags_changed:
-            CardSearch().update([card], action="index")
+            reindex_card_safely(card)
 
 
 class TagVoteTallyEntry(TypedDict):

MPCAutofill/cardpicker/tests/test_printing_consensus.py

@@ -1,9 +1,12 @@
+from unittest.mock import patch
+
 import pytest
 
 from cardpicker.models import PrintingTagStatus, VoteSource
 from cardpicker.printing_consensus import (
     NO_MATCH,
     get_resolved_printings,
+    resolve_and_persist_printing,
     resolve_printing,
 )
 from cardpicker.tests.factories import (
@@ -108,6 +111,88 @@ def test_ai_only_insufficient(self, db):
         assert resolve_printing(card) is None
 
 
+class TestResolveAndPersistPrintingReindex:
+    """
+    `resolve_and_persist_printing`'s ES side effect: reindex exactly when the outcome changes
+    what `documents.py` actually indexes (see `_effective_indexed_printing_id`), and never let
+    an ES failure take down the vote-submission DB write it rides in on.
+    """
+
+    def test_unresolved_to_resolved_fires_one_reindex_call(self, db):
+        card = CardFactory()
+        printing = CanonicalCardFactory()
+        CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.USER)
+        CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.USER)
+
+        with patch("cardpicker.documents.reindex_card_safely") as mock_reindex:
+            result = resolve_and_persist_printing(card)
+
+        assert result == printing
+        assert card.printing_tag_status == PrintingTagStatus.RESOLVED
+        mock_reindex.assert_called_once_with(card)
+
+    def test_re_resolve_to_same_outcome_fires_zero_calls(self, db):
+        card = CardFactory()
+        printing = CanonicalCardFactory()
+        CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.USER)
+        CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.USER)
+
+        with patch("cardpicker.documents.reindex_card_safely"):
+            resolve_and_persist_printing(card)  # first call: UNRESOLVED -> RESOLVED, establishes the baseline
+
+        with patch("cardpicker.documents.reindex_card_safely") as mock_reindex:
+            result = resolve_and_persist_printing(card)  # same votes, same outcome, re-resolved
+
+        assert result == printing
+        assert card.printing_tag_status == PrintingTagStatus.RESOLVED
+        mock_reindex.assert_not_called()
+
+    def test_resolved_to_contested_fires_a_call_and_indexed_fields_clear(self, db):
+        card = CardFactory()
+        printing = CanonicalCardFactory()
+        CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.USER)
+        CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.USER)
+
+        with patch("cardpicker.documents.reindex_card_safely"):
+            resolve_and_persist_printing(card)
+        assert card.printing_tag_status == PrintingTagStatus.RESOLVED
+
+        # an equal-weight conflicting printing splits consensus (2 vs 2: share is exactly
+        # 0.5, below PRINTING_TAG_MIN_SHARE of 0.6 - same tie shape as test_tie_returns_none)
+        printing_b = CanonicalCardFactory()
+        CardPrintingTagFactory(card=card, printing=printing_b, source=VoteSource.USER)
+        CardPrintingTagFactory(card=card, printing=printing_b, source=VoteSource.USER)
+
+        with patch("cardpicker.documents.reindex_card_safely") as mock_reindex:
+            result = resolve_and_persist_printing(card)
+
+        assert result is None
+        assert card.printing_tag_status == PrintingTagStatus.UNRESOLVED
+        mock_reindex.assert_called_once_with(card)
+        # `card.canonical_card` is unset (no confirmed indexing match), so once RESOLVED is
+        # lost, the fields `documents.py` actually indexes fall all the way back to None -
+        # exactly what "the index needs updating" was gated on.
+        assert card.get_expansion_code() is None
+        assert card.get_collector_number() is None
+
+    def test_es_failure_inside_reindex_does_not_block_the_db_write(self, db, caplog):
+        card = CardFactory()
+        printing = CanonicalCardFactory()
+        CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.USER)
+        CardPrintingTagFactory(card=card, printing=printing, source=VoteSource.USER)
+
+        with patch("cardpicker.documents.CardSearch") as mock_card_search:
+            mock_card_search.return_value.update.side_effect = Exception("ES is down")
+            result = resolve_and_persist_printing(card)  # must not raise
+
+        assert result == printing
+        assert "Failed to reindex card" in caplog.text
+
+        card.refresh_from_db()
+        assert card.printing_tag_status == PrintingTagStatus.RESOLVED
+        assert card.inferred_canonical_card_id == printing.pk
+
+
 class TestGetResolvedPrintings:
     """
     `get_resolved_printings` is the shared hard-gate helper consumed by both the search

docs/features/printing-tags.md

@@ -214,14 +214,49 @@ Orama-indexed) search gets no re-rank/filter/indicator parity — that path
 has no ES/DB access to consult `printing_tag_status`. Flagged explicitly,
 not silently built.
 
+## Stage 3.5: immediate reindex on vote transition
+
+Stage 3's re-rank/filters read `printing_tag_status` and the indexed
+`expansion_code`/`collector_number` (see the fallback widening above) —
+but nothing pushed a changed card into ES until the next scheduled
+`update_database` re-scan. A vote that just resolved (or un-resolved) a
+printing was invisible to search until that next scan ran.
+
+`cardpicker/documents.py::reindex_card_safely(card)` — the shared,
+failure-isolated push (`CardSearch().update([card], action="index")`,
+exception caught and logged, never raised: Postgres is truth, ES is a
+projection, so an ES hiccup must never break vote submission or roll back
+a write that already committed).
+
+`printing_consensus.py::resolve_and_persist_printing` calls it, but only
+when the _effective indexed_ printing id actually changes
+(`_effective_indexed_printing_id` — the same RESOLVED-gated fallback
+`get_expansion_code`/`get_collector_number` use). Covers both directions:
+entering RESOLVED, leaving RESOLVED (contested/unresolved again), and the
+resolved printing itself changing while staying RESOLVED. A re-resolve
+landing on the same outcome (the common case for an already-settled card)
+touches the DB but not the index.
+
+`tag_consensus.py::resolve_and_persist_tag_votes` already had an
+ES push for `tags` (an ES-indexed field, unlike the printing/artist
+denormalised columns) gated on its own `tags_changed` flag — switched to
+`reindex_card_safely` for the same failure isolation, no change to when
+it fires.
+
+Artist resolution (`artist_consensus.py`) never touches an ES-indexed
+field (`inferred_canonical_artist`/`artist_vote_status` only,
+serialise-time-only) — confirmed against `documents.py`'s field list, no
+hook needed.
+
 ## Key files
 
 - Backend: `cardpicker/printing_consensus.py`,
   `cardpicker/printing_metadata_import.py`,
   `cardpicker/integrations/game/mtg.py`, `cardpicker/models.py` (migration
   `0050_canonicalprintingmetadata_cardprintingtag_and_more.py`),
   `cardpicker/search/search_functions.py` (Stage 3 re-rank/filter),
-  `cardpicker/documents.py` (Stage 3 widened indexing)
+  `cardpicker/documents.py` (Stage 3 widened indexing; Stage 3.5
+  `reindex_card_safely`), `cardpicker/tag_consensus.py` (Stage 3.5)
 - Frontend: `frontend/src/features/printingTags/` (`PrintingTagQueue.tsx`,
   `PrintingTagPicker.tsx`, `starburstShape.ts`, `useStickyTop`),
   `frontend/src/features/filters/ResolvedAttributeFilter.tsx` (Stage 3),