Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/coverage-acks.txt
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,10 @@ coverage-ack: frontend/tests/ContrastAudit.spec.ts::Link colour audit (owner-app
coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - Level 2 illustration grouping > every candidate in a mixed illustration set still renders in the grid - none silently dropped — PR #730: renamed to "every candidate in a mixed illustration set is accounted for - a group collapses to its one representative tile, ungrouped candidates still render individually" now that a group renders one representative tile, not one tile per member.
coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - Level 2 illustration grouping > candidates sharing an illustration render inside one illustration-group container; unique/null-illustration candidates don't — PR #730: renamed to "only the illustration group's representative candidate renders inside its one illustration-group container; unique/null-illustration candidates don't", same reason as the sibling ack above.
coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - Level 2 illustration grouping > clustered tiles render each candidate's art crop, falling back to the printing scan when absent; ungrouped tiles keep the printing scan regardless — PR #730: renamed to "the illustration group's one tile renders its representative candidate's art crop; ungrouped tiles keep the printing scan regardless", same reason as the sibling acks above.

# PR #731 (issue #712) - Level 1 "Not sure" now fires a fire-and-forget POST to the new
# /2/submitQuestionAbstention/ endpoint before transitioning to Level 2, in addition to the
# unchanged "no printing vote cast" behavior this test already covered. Renamed (not deleted) to
# describe both halves of the contract, and extended with a mocked handler + payload assertion
# for the new POST - same surface (the NOT SURE button), same "no printing vote" claim kept.
coverage-ack: frontend/tests/QuestionFeed.spec.ts::question feed - confirm_suggestion question type > NOT SURE drops to Level 2's candidate grid without casting a vote — PR #731 (issue #712): renamed to "NOT SURE drops to Level 2's candidate grid without casting a printing vote, but does POST an abstention" now that NOT SURE fires a fire-and-forget POST to the new /2/submitQuestionAbstention/ endpoint; the "no printing vote" assertion is kept, extended with coverage of the new abstention POST.
9 changes: 9 additions & 0 deletions MPCAutofill/cardpicker/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
CardArtistVote,
CardIllustrationVote,
CardPrintingTag,
CardQuestionAbstention,
CardReport,
CardScanLog,
CardTagVote,
Expand Down Expand Up @@ -300,6 +301,14 @@ class AdminCardScanLog(admin.ModelAdmin[CardScanLog]):
ordering = ("-scanned_at",)


@admin.register(CardQuestionAbstention)
class AdminCardQuestionAbstention(admin.ModelAdmin[CardQuestionAbstention]):
list_display = ("card", "anonymous_id", "question_type", "created_at")
list_filter = ("question_type",)
search_fields = ("anonymous_id",)
ordering = ("-created_at",)


@admin.register(EnvelopeTrip)
class AdminEnvelopeTrip(admin.ModelAdmin[EnvelopeTrip]):
# Stage E Phase 1 (docs/proposals/stage-e-streaming.md §3 decision (5)/§10(a)) - this is the
Expand Down
39 changes: 39 additions & 0 deletions MPCAutofill/cardpicker/migrations/0104_cardquestionabstention.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# Hand-written (not `manage.py makemigrations`-generated - same convention as migration 0080
# `questionfeedservedlog.py`) to exactly match `cardpicker.models.CardQuestionAbstention` as of
# this migration.

import django.db.models.deletion
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("cardpicker", "0103_canonicalprintingmetadata_layout"),
]

operations = [
migrations.CreateModel(
name="CardQuestionAbstention",
fields=[
("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")),
("anonymous_id", models.CharField(max_length=40)),
("question_type", models.CharField(max_length=32)),
("created_at", models.DateTimeField(auto_now_add=True)),
(
"card",
models.ForeignKey(
on_delete=django.db.models.deletion.CASCADE,
related_name="question_abstentions",
to="cardpicker.card",
),
),
],
),
migrations.AddConstraint(
model_name="cardquestionabstention",
constraint=models.UniqueConstraint(
fields=("card", "anonymous_id", "question_type"), name="cardquestionabstention_unique"
),
),
]
55 changes: 55 additions & 0 deletions MPCAutofill/cardpicker/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2150,6 +2150,16 @@ class CardScanLog(models.Model):
call sites) - not a separately-invented vocabulary - so a `grep` for a skip reason in the
log output and a `WHERE skip_reason = '...'` query agree on what string to look for.

MACHINE abstention only. `CardQuestionAbstention` (below) is this model's HUMAN counterpart -
issue #712's "Not sure" signal on a `cardpicker.question_feed` item - and is deliberately a
separate model rather than a shared one: this model's `run_id`/`skip_reason`/
`evidence_types_used`/`survivor_pks` are all calculator-run bookkeeping with no human
equivalent, and the resume-exclusion query this model serves
(`local_identify_printing_tags._eligible_base_queryset`) must never be satisfied by a human
tapping "Not sure" - the two express different facts ("this engine did not vote" vs. "this
person looked and could not tell") and conflating them would let one silently stand in for
the other in either direction.

A card can have at most one CURRENT scan-log row per (card, anonymous_id) that actually
matters for the resume-exclusion query (see local_identify_printing_tags._eligible_base_
queryset) - older rows for the same pair are historical (multiple runs can each abstain on
Expand Down Expand Up @@ -2216,6 +2226,50 @@ def __str__(self) -> str:
return f"card={self.card_id} anonymous_id={self.anonymous_id} skip_reason={self.skip_reason}"


class CardQuestionAbstention(models.Model):
"""
Issue #712. Records that a voter ENGAGED with a `cardpicker.question_feed` item and found it
genuinely ambiguous ("Not sure") - real information about the card (this question is hard
to answer for this specific image), unlike a "Skip" tap, which carries no signal about the
card at all and writes nothing here or anywhere else (see QuestionFeed.tsx's `skip`).

This is a HUMAN abstention - the counterpart to `CardScanLog`'s MACHINE abstention (see that
model's own docstring for why the two are deliberately separate models). Like `CardScanLog`,
deliberately NOT a subclass of `AbstractWeightedVote`: an abstention is not a vote, carries
no source/confidence/user/polarity, and must never be reachable via `vote_consensus`'s
resolution machinery even by accident.

`question_type` mirrors `QuestionFeedItem.type` (e.g. "confirm_suggestion" /
"identify_printing") - the same free-text convention `QuestionFeedServedLog.question_type`
already uses, for the same reason: the question feed's own type vocabulary is the single
source of truth, and duplicating it as a second closed enum here would just be a second
place for the two to drift apart.

Unique on (card, anonymous_id, question_type); the write path is `get_or_create`, so a voter
tapping "Not sure" more than once on the same pair (e.g. across repeat serves) records the
fact once, not once per tap. This is also exactly the shape a future exclusion query needs
(issue #713, not built here): "has this anonymous_id already abstained on this card for this
question_type" is a single indexed equality lookup against this table's own unique
constraint, e.g. `CardQuestionAbstention.objects.filter(card_id=..., anonymous_id=...,
question_type=...).exists()`.
"""

card = models.ForeignKey(to=Card, on_delete=models.CASCADE, related_name="question_abstentions")
anonymous_id = models.CharField(max_length=40)
question_type = models.CharField(max_length=32)
created_at = models.DateTimeField(auto_now_add=True)

class Meta:
constraints = [
models.UniqueConstraint(
fields=["card", "anonymous_id", "question_type"], name="cardquestionabstention_unique"
),
]

def __str__(self) -> str:
return f"card={self.card_id} anonymous_id={self.anonymous_id} question_type={self.question_type}"


class SavedDeckKind(models.TextChoices):
"""
See docs/proposals/proposal-g-user-accounts-saved-decks.md §3/decision 7. DECK rows are
Expand Down Expand Up @@ -2792,6 +2846,7 @@ def __str__(self) -> str:
"ProjectMember",
"PilotRunLedger",
"CardScanLog",
"CardQuestionAbstention",
"SavedDeckKind",
"SavedDeck",
"UserCryptoProfile",
Expand Down
46 changes: 46 additions & 0 deletions MPCAutofill/cardpicker/schema_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -2940,6 +2940,52 @@ def to_dict(self) -> dict:
return result


class SubmitQuestionAbstentionRequest(BaseModel):
"""
2/submitQuestionAbstention/ (issue #712) - hand-maintained, same provenance as
CastImplicitVoteRequest/RetractImplicitVoteRequest above (no JSON schema source exists for
request types in this repo - typed by hand against the real Python pydantic model, not
quicktype-generated).
"""

anonymousId: str
identifier: str
questionType: str

@staticmethod
def from_dict(obj: Any) -> "SubmitQuestionAbstentionRequest":
assert isinstance(obj, dict)
anonymousId = from_str(obj.get("anonymousId"))
identifier = from_str(obj.get("identifier"))
questionType = from_str(obj.get("questionType"))
return SubmitQuestionAbstentionRequest(anonymousId, identifier, questionType)

def to_dict(self) -> dict:
result: dict = {}
result["anonymousId"] = from_str(self.anonymousId)
result["identifier"] = from_str(self.identifier)
result["questionType"] = from_str(self.questionType)
return result


class SubmitQuestionAbstentionResponse(BaseModel):
"""Mirrors SubmitQuestionAbstentionRequest above - a single-purpose ack, no vote tally to
return since an abstention isn't a vote and never participates in consensus."""

recorded: bool

@staticmethod
def from_dict(obj: Any) -> "SubmitQuestionAbstentionResponse":
assert isinstance(obj, dict)
recorded = from_bool(obj.get("recorded"))
return SubmitQuestionAbstentionResponse(recorded)

def to_dict(self) -> dict:
result: dict = {}
result["recorded"] = from_bool(self.recorded)
return result


class ArtistExternalLinksResponse(BaseModel):
"""
2/artistExternalLinks/ - hand-maintained, NOT quicktype-generated, same provenance/reasoning
Expand Down
107 changes: 107 additions & 0 deletions MPCAutofill/cardpicker/tests/test_question_abstention.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import pytest

from django.core.cache import cache
from django.urls import reverse

from cardpicker import views
from cardpicker.models import CardQuestionAbstention
from cardpicker.tests.factories import CardFactory


@pytest.fixture(autouse=True)
def _clear_rate_limit_cache():
cache.clear()
yield
cache.clear()


class TestPostSubmitQuestionAbstention:
def test_unknown_card_identifier_is_a_bad_request(self, client, django_settings):
response = client.post(
reverse(views.post_submit_question_abstention),
{"identifier": "does-not-exist", "anonymousId": "anon-1", "questionType": "confirm_suggestion"},
content_type="application/json",
)
assert response.status_code == 400

def test_records_exactly_one_abstention_with_the_right_voter_card_and_question_type(self, client, django_settings):
card = CardFactory()

response = client.post(
reverse(views.post_submit_question_abstention),
{"identifier": card.identifier, "anonymousId": "anon-1", "questionType": "confirm_suggestion"},
content_type="application/json",
)

assert response.status_code == 200
assert response.json()["recorded"] is True
assert CardQuestionAbstention.objects.count() == 1
abstention = CardQuestionAbstention.objects.get()
assert abstention.card_id == card.pk
assert abstention.anonymous_id == "anon-1"
assert abstention.question_type == "confirm_suggestion"

def test_repeat_taps_from_the_same_voter_record_the_fact_once(self, client, django_settings):
card = CardFactory()
body = {"identifier": card.identifier, "anonymousId": "anon-1", "questionType": "identify_printing"}

for _ in range(3):
response = client.post(
reverse(views.post_submit_question_abstention), body, content_type="application/json"
)
assert response.status_code == 200

assert CardQuestionAbstention.objects.count() == 1

def test_different_question_types_on_the_same_card_and_voter_record_separately(self, client, django_settings):
card = CardFactory()

for question_type in ("confirm_suggestion", "identify_printing"):
response = client.post(
reverse(views.post_submit_question_abstention),
{"identifier": card.identifier, "anonymousId": "anon-1", "questionType": question_type},
content_type="application/json",
)
assert response.status_code == 200

assert CardQuestionAbstention.objects.count() == 2

def test_different_voters_on_the_same_card_and_question_type_record_separately(self, client, django_settings):
card = CardFactory()

for anonymous_id in ("anon-1", "anon-2"):
response = client.post(
reverse(views.post_submit_question_abstention),
{"identifier": card.identifier, "anonymousId": anonymous_id, "questionType": "confirm_suggestion"},
content_type="application/json",
)
assert response.status_code == 200

assert CardQuestionAbstention.objects.count() == 2

def test_skip_writes_no_abstention_row(self, db):
# Documents the "Skip" side of the contract at the model layer: nothing in this codebase
# ever calls CardQuestionAbstention.objects.create/get_or_create from a skip path (the
# frontend's `skip` handler never calls `2/submitQuestionAbstention/` at all - see
# QuestionFeed.tsx), so an untouched card simply has zero rows here.
CardFactory()

assert CardQuestionAbstention.objects.count() == 0

def test_abstention_is_queryable_by_voter_card_and_question_type(self, db):
# The shape a future exclusion query (issue #713) needs: "has this anonymous_id already
# abstained on this card for this question_type" as a single indexed equality lookup.
card = CardFactory()
CardQuestionAbstention.objects.get_or_create(
card=card, anonymous_id="anon-1", question_type="confirm_suggestion"
)

assert CardQuestionAbstention.objects.filter(
card_id=card.pk, anonymous_id="anon-1", question_type="confirm_suggestion"
).exists()
assert not CardQuestionAbstention.objects.filter(
card_id=card.pk, anonymous_id="anon-1", question_type="identify_printing"
).exists()
assert not CardQuestionAbstention.objects.filter(
card_id=card.pk, anonymous_id="anon-2", question_type="confirm_suggestion"
).exists()
1 change: 1 addition & 0 deletions MPCAutofill/cardpicker/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
path("2/printingCandidates/", views.post_printing_candidates),
path("2/printingConsensus/", views.post_printing_consensus),
path("2/submitPrintingTag/", views.post_submit_printing_tag),
path("2/submitQuestionAbstention/", views.post_submit_question_abstention),
path("2/printingTagQueue/", views.get_printing_tag_queue),
path("2/artistCandidates/", views.post_artist_candidates),
path("2/artistConsensus/", views.post_artist_consensus),
Expand Down
33 changes: 33 additions & 0 deletions MPCAutofill/cardpicker/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
Card,
CardArtistVote,
CardPrintingTag,
CardQuestionAbstention,
CardReport,
CardScanLog,
CardTagVote,
Expand Down Expand Up @@ -218,6 +219,8 @@
SubmitIllustrationVoteRequest,
SubmitIllustrationVoteResponse,
SubmitPrintingTagRequest,
SubmitQuestionAbstentionRequest,
SubmitQuestionAbstentionResponse,
SubmitTagVoteRequest,
TagConsensusEntry,
TagConsensusRequest,
Expand Down Expand Up @@ -1114,6 +1117,36 @@ def post_submit_printing_tag(request: HttpRequest) -> HttpResponse:
return JsonResponse(_build_printing_consensus_response(card, resolved).model_dump())


@ratelimit( # type: ignore # `django-ratelimit` does not implement decorator typing correctly
key=_printing_tag_rate_limit_key, rate=_printing_tag_rate_limit_rate, method="POST", block=False
)
@ErrorWrappers.to_json
def post_submit_question_abstention(request: HttpRequest) -> HttpResponse:
"""
Issue #712. Records a human abstention ("Not sure") on a question-feed item - the counterpart
to `CardScanLog`'s machine abstention, see `CardQuestionAbstention`'s own docstring for why
the two are separate models. `get_or_create` makes repeat taps on the same (card,
anonymousId, questionType) idempotent. Reuses the printing-tag submission's rate-limit
plumbing (`_printing_tag_rate_limit_key`/`_printing_tag_rate_limit_rate`), the same rate
budget every other question-feed write already shares, not a separate budget.
"""

if request.method != "POST":
raise BadRequestException("Expected POST request.")
if getattr(request, "limited", False):
return JsonResponse(
ErrorResponse(name="Rate limited", message="Too many abstentions - please slow down.").model_dump(),
status=429,
)

req = SubmitQuestionAbstentionRequest.model_validate(json.loads(request.body))
card = _get_card_or_400(req.identifier)
CardQuestionAbstention.objects.get_or_create(
card=card, anonymous_id=req.anonymousId, question_type=req.questionType
)
return JsonResponse(SubmitQuestionAbstentionResponse(recorded=True).model_dump())


def _build_artist_consensus_response(
card: Card, resolved: CanonicalArtist | Literal["UNKNOWN"] | None
) -> ArtistConsensusResponse:
Expand Down
17 changes: 17 additions & 0 deletions docs/features/printing-tags.md
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,23 @@ printings, artists, tags, and moderation from one screen.
`question_feed._served_mix_ratio` reads it back as two cheap indexed
`COUNT`s, never a per-row scan. Append-only, same convention as
`CardScanLog` — never read by any consensus computation.
- **"Not sure" abstention** (issue #712): Level 1's "Yes"/"No, different
printing" both cast a real vote (see `selectCandidate`/`rejectSuggestion`
in `QuestionFeed.tsx`); "Not sure" and "Skip" used to be indistinguishable
no-ops — neither wrote anything. They are now split: "Not sure" means the
voter engaged and found the image genuinely ambiguous, real information
about the CARD, so it POSTs `2/submitQuestionAbstention/` and is recorded
in `CardQuestionAbstention` (`card`, `anonymous_id`, `question_type`,
unique together, `get_or_create`-idempotent per repeat tap) before the
same `setStage("level2")` transition it always did. "Skip" carries no
signal about the card at all and still writes nothing anywhere — that
stays a deliberate no-op, not a bug. `CardQuestionAbstention` is the
HUMAN counterpart to `CardScanLog`'s MACHINE abstention (see that
model's own docstring for why they're separate tables) and is, like it,
NOT an `AbstractWeightedVote` subclass — an abstention never enters
`vote_consensus`. A future exclusion query (issue #713, not built by
this addition) reads it back as a single indexed equality lookup:
`CardQuestionAbstention.objects.filter(card_id=..., anonymous_id=..., question_type=...).exists()`.
- **Remaining-work count**: `get_remaining_estimate()` returns
`QuestionFeedCounts` (`schemas/schemas/QuestionFeedCounts.json`), not a
single number. `total` is a `.distinct().count()` union across
Expand Down
Loading
Loading