From 8dde23cc10904e993a3ffe5c835de45fa80537f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 17:00:40 +0000 Subject: [PATCH] feat(opal): tint a guess red once its guesser ran out of guesses Yellow says a guess led nowhere. It does not separate someone who wandered off from someone the puzzle actually shut out, which is the sharper signal: they wanted the answer and the guess limit took it away. Tint that case red -- an unsolved puzzle whose guesser has also spent its whole guess limit. It sits between the finisher and unsolved branches, so the hunt-wide tints still win and every red row is one that would otherwise be yellow. What "spent the limit" means now lives in one place. `_eligibility` was the only definition of which guesses count -- not correct, not close, not excused -- and the log has to agree with it, or a row goes red while the puzzle page is still taking answers. Both now read `COUNTS_AGAINST_GUESS_LIMIT`, and a test pins the two together by walking a guesser to the limit and checking `can_attempt` flips in step with the tint. Still one query for the standings. Rather than add a second, `_standings` now groups every guess on the hunt by guesser and puzzle and reads the first correct timestamp and the spent-guess count off each group, which carries all four facts a standing holds. Counts measured at 2, 50 and 100 rows are unchanged. --- opal/tests.py | 72 ++++++++++++++++++++++++++++++++++++----- opal/views.py | 88 +++++++++++++++++++++++++++++++++++---------------- 2 files changed, 124 insertions(+), 36 deletions(-) diff --git a/opal/tests.py b/opal/tests.py index 2e90f3b0..b46846c2 100644 --- a/opal/tests.py +++ b/opal/tests.py @@ -891,20 +891,76 @@ def styling(attempts) -> dict[int, tuple[str, str]]: pk: expected[pk] for pk in (bob_miss.pk, bob_feeder.pk, bob_stuck.pk) } - # once Bob finishes, the hunt-wide green wins over the per-puzzle yellow + # and the leaderboard the logs are mirroring agrees on both counts + resp = otis.get_20x("opal-leaderboard", "hunt") + leaders = {row["name"]: row for row in resp.context["rows"]} + assert leaders["Tess Solver"]["emoji_string"] == "☑️🆗" + assert leaders["Tess Solver"]["row_class"] == "table-primary" + assert leaders["Alice A"]["emoji_string"] == "✅🈴" + assert leaders["Alice A"]["row_class"] == "table-success" + + # spending the last guess on the meta turns Bob's rows on it red, and + # leaves his rows on the puzzle he did get alone + with freeze_time("2024-08-16"): + OpalAttemptFactory.create_batch( + meta.guess_limit - 1, user=bob, puzzle=meta, guess="nope" + ) + resp = otis.get_20x("opal-person-log", "hunt", bob.pk) + rows = {a.pk: a.row_class for a in resp.context["attempts"]} + assert rows.pop(bob_miss.pk) == "" + assert rows.pop(bob_feeder.pk) == "" + assert set(rows.values()) == {"table-danger"} + + # once Bob finishes, the hunt-wide green wins over both per-puzzle tints OpalAttemptFactory.create(user=bob, puzzle=meta, guess="two") resp = otis.get_20x("opal-person-log", "hunt", bob.pk) assert {a.pk: a.row_class for a in resp.context["attempts"]} == { a.pk: "table-success" for a in resp.context["attempts"] } - # and the leaderboard the logs are mirroring agrees on both counts - resp = otis.get_20x("opal-leaderboard", "hunt") - rows = {row["name"]: row for row in resp.context["rows"]} - assert rows["Tess Solver"]["emoji_string"] == "☑️🆗" - assert rows["Tess Solver"]["row_class"] == "table-primary" - assert rows["Alice A"]["emoji_string"] == "✅🈴" - assert rows["Alice A"]["row_class"] == "table-success" + +@pytest.mark.django_db +def test_guess_log_out_of_guesses_matches_puzzle_page(otis): + """A row goes red exactly when the puzzle page stops taking guesses.""" + verified_group = GroupFactory(name="Verified") + alice = UserFactory.create(username="alice", groups=(verified_group,)) + admin = UserFactory.create(username="admin", is_staff=True, is_superuser=True) + + hunt = OpalHuntFactory.create( + slug="hunt", start_date=datetime.datetime(2024, 8, 1, tzinfo=UTC) + ) + puzzle = OpalPuzzleFactory.create( + hunt=hunt, slug="puzzle", answer="right", partial_answers="warm", guess_limit=3 + ) + + def row_classes() -> set[str]: + otis.login(admin) + resp = otis.get_20x("opal-person-log", "hunt", alice.pk) + classes = {a.row_class for a in resp.context["attempts"]} + otis.login(alice) + return classes + + with freeze_time("2024-08-15"): + # guesses that do not eat the limit: a close one, and an excused one + OpalAttemptFactory.create(user=alice, puzzle=puzzle, guess="warm") + excused = OpalAttemptFactory.create(user=alice, puzzle=puzzle, guess="nope") + excused.excused = True + excused.save() + OpalAttemptFactory.create_batch(2, user=alice, puzzle=puzzle, guess="nope") + + # two of three spent, so the form is still up and nothing is red yet + otis.login(alice) + resp = otis.get_20x("opal-show-puzzle", "hunt", "puzzle") + assert resp.context["can_attempt"] is True + assert row_classes() == {"table-warning"} + + with freeze_time("2024-08-16"): + OpalAttemptFactory.create(user=alice, puzzle=puzzle, guess="nope") + + # the third spends the limit: the page shuts the form and the rows go red + resp = otis.get_20x("opal-show-puzzle", "hunt", "puzzle") + assert resp.context["can_attempt"] is False + assert row_classes() == {"table-danger"} @pytest.mark.django_db diff --git a/opal/views.py b/opal/views.py index 97dcfac6..42398ad2 100644 --- a/opal/views.py +++ b/opal/views.py @@ -11,7 +11,7 @@ from django.core.exceptions import PermissionDenied from django.db import transaction from django.db.models import Q -from django.db.models.aggregates import Max +from django.db.models.aggregates import Count, Max, Min from django.db.models.manager import Manager from django.db.models.query import QuerySet from django.http.response import HttpResponse, HttpResponseRedirect @@ -47,13 +47,20 @@ def has_early_access(u: User) -> bool: # guess repeated on one page and missing from the next. ATTEMPT_LOG_ORDERING = ("-created_at", "-pk") +# A guess eats one of the puzzle's guess limit unless it was right, close, or +# excused by an admin. `_eligibility` enforces this and the logs read it back, +# so what a row calls "out of guesses" is what the puzzle page acted on. +COUNTS_AGAINST_GUESS_LIMIT = Q(excused=False, is_close=False, is_correct=False) + # How a guesser's standing in a hunt tints their row, on the leaderboard and in # every guess log: blue for a testsolver, green for someone who has finished, -# yellow for a guess on a puzzle its guesser never did get. +# yellow for a guess on a puzzle its guesser never did get, and red once they +# also ran out of guesses on it. TESTSOLVER_ROW_CLASS = "table-primary" FINISHER_ROW_CLASS = "table-success" UNSOLVED_ROW_CLASS = "table-warning" +EXHAUSTED_ROW_CLASS = "table-danger" def correct_emoji(is_testsolver: bool, is_metapuzzle: bool) -> str: @@ -70,19 +77,26 @@ def correct_emoji(is_testsolver: bool, is_metapuzzle: bool) -> str: def standing_row_class( - is_testsolver: bool, has_finished: bool, puzzle_unsolved: bool = False + is_testsolver: bool, + has_finished: bool, + puzzle_unsolved: bool = False, + out_of_guesses: bool = False, ) -> str: """The Bootstrap tint for a row belonging to a guesser with this standing. `puzzle_unsolved` says this row is a guess on a puzzle its guesser has no - correct answer for anywhere in the hunt, so the guess led nowhere. It only - means something for a row about one puzzle: the leaderboard's rows span the - whole hunt, so it leaves the argument alone. + correct answer for anywhere in the hunt, so the guess led nowhere; + `out_of_guesses` says they also spent that puzzle's whole guess limit, so + it led nowhere and there is no way back. Both only mean something for a row + about one puzzle: the leaderboard's rows span the whole hunt, so it leaves + the two arguments alone. """ if is_testsolver: return TESTSOLVER_ROW_CLASS elif has_finished: return FINISHER_ROW_CLASS + elif puzzle_unsolved and out_of_guesses: + return EXHAUSTED_ROW_CLASS elif puzzle_unsolved: return UNSOLVED_ROW_CLASS else: @@ -95,11 +109,12 @@ class _Standing(NamedTuple): A testsolver is someone who solved something before the hunt opened, the same test the leaderboard uses, so the two pages agree on who is who. - `solved_puzzles` is kept whole rather than counted, since a row also wants - to know whether its own puzzle is in there. + The two puzzle sets are kept whole rather than counted, since a row also + wants to know whether its own puzzle is in either of them. """ solved_puzzles: frozenset[int] + exhausted_puzzles: frozenset[int] is_testsolver: bool has_finished: bool @@ -109,34 +124,53 @@ def solve_count(self) -> int: NO_SOLVES = _Standing( - solved_puzzles=frozenset(), is_testsolver=False, has_finished=False + solved_puzzles=frozenset(), + exhausted_puzzles=frozenset(), + is_testsolver=False, + has_finished=False, ) def _standings(hunt: OpalHunt, user_pks: Collection[int]) -> dict[int, _Standing]: """Each of those users' standing in `hunt`, in one query for the whole page. - Users with no correct guess on the hunt are absent; `NO_SOLVES` covers them. + The query groups every guess on the hunt by guesser and puzzle, so a row + comes back per puzzle a guesser has touched, saying when they first got it + right (never, if they did not) and how many of their guesses on it ate the + guess limit. That is enough for all four facts a standing carries, and it + stays one round trip however long the page is. + + Users who have not guessed on the hunt are absent; `NO_SOLVES` covers them. """ - solved_puzzles: defaultdict[int, set[int]] = defaultdict(set) + solved: defaultdict[int, set[int]] = defaultdict(set) + exhausted: defaultdict[int, set[int]] = defaultdict(set) testsolvers: set[int] = set() finishers: set[int] = set() - for d in OpalAttempt.objects.filter( - puzzle__hunt=hunt, user__in=user_pks, is_correct=True - ).values("user", "puzzle", "created_at", "puzzle__is_metapuzzle"): - user_pk = d["user"] - solved_puzzles[user_pk].add(d["puzzle"]) - if d["created_at"] < hunt.start_date: - testsolvers.add(user_pk) - if d["puzzle__is_metapuzzle"]: - finishers.add(user_pk) + for d in ( + OpalAttempt.objects.filter(puzzle__hunt=hunt, user__in=user_pks) + .values("user", "puzzle", "puzzle__is_metapuzzle", "puzzle__guess_limit") + .annotate( + first_correct=Min("created_at", filter=Q(is_correct=True)), + num_counted=Count("pk", filter=COUNTS_AGAINST_GUESS_LIMIT), + ) + ): + user_pk, puzzle_pk = d["user"], d["puzzle"] + if (first_correct := d["first_correct"]) is not None: + solved[user_pk].add(puzzle_pk) + if first_correct < hunt.start_date: + testsolvers.add(user_pk) + if d["puzzle__is_metapuzzle"]: + finishers.add(user_pk) + if d["num_counted"] >= d["puzzle__guess_limit"]: + exhausted[user_pk].add(puzzle_pk) return { user_pk: _Standing( - solved_puzzles=frozenset(puzzle_pks), + solved_puzzles=frozenset(solved.get(user_pk, ())), + exhausted_puzzles=frozenset(exhausted.get(user_pk, ())), is_testsolver=user_pk in testsolvers, has_finished=user_pk in finishers, ) - for user_pk, puzzle_pks in solved_puzzles.items() + for user_pk in solved.keys() | exhausted.keys() } @@ -154,7 +188,8 @@ def decorate_attempts( making the guess is. * `emoji` and `text_class`, how the guess itself was judged. * `row_class`, the tint for where the guesser stands in the hunt, and for - whether they ever solved the puzzle this row is a guess on. + whether they ever solved the puzzle this row is a guess on and whether + they still had guesses left on it. The standings take a single query for the whole page, since `OpalHunt.num_solves` would be one query per row. Pass a queryset that has @@ -170,6 +205,7 @@ def decorate_attempts( is_testsolver=standing.is_testsolver, has_finished=standing.has_finished, puzzle_unsolved=attempt.puzzle.pk not in standing.solved_puzzles, + out_of_guesses=attempt.puzzle.pk in standing.exhausted_puzzles, ) if attempt.is_correct: attempt.emoji = correct_emoji( # type: ignore[attr-defined] @@ -456,11 +492,7 @@ def _eligibility(puzzle: OpalPuzzle, user: User) -> _Eligibility: puzzle=puzzle, user=user, is_correct=True ).exists() incorrect_attempts = OpalAttempt.objects.filter( - puzzle=puzzle, - user=user, - excused=False, - is_close=False, - is_correct=False, + COUNTS_AGAINST_GUESS_LIMIT, puzzle=puzzle, user=user ).order_by("-created_at") return _Eligibility( is_solved=is_solved,