Skip to content

Skip the unread-message subquery when listing joinable games - #1402

Merged
johnpooch merged 2 commits into
mainfrom
claude/quirky-pasteur-j1x1jb
Sep 25, 2026
Merged

johnpooch merged 2 commits into
mainfrom
claude/quirky-pasteur-j1x1jb

Conversation

@johnpooch

@johnpooch johnpooch commented Sep 25, 2026 •

Copy link
Copy Markdown
Owner

What this PR does

Speeds up the authenticated Find Games request (GET /games/?can_join=true&ordering=slots_remaining, sent from packages/web/src/screens/Home/FindGames.tsx). When can_join parses as true, GameFilter.filter_can_join now re-annotates total_unread_message_count as a constant integer zero. That replaces the correlated unread-message subquery before any SQL is compiled.

Root cause: the query shape

GameListView.get_queryset calls with_total_unread_counts(user) before django-filter runs. For authenticated users that adds a correlated ChannelMessage subquery (COALESCE((SELECT COUNT(DISTINCT …) FROM channel_channelmessage …), 0)). Then ordering=slots_remaining adds Count("members") and Count("variant__nations"), which join members and variant nations and GROUP BY the game. Because of that aggregate, Django's pagination COUNT(*) wraps the whole grouped query, unread subquery included. So the subquery ran once per member×nation row, in both the COUNT and the page query.

Production EXPLAIN (supplied diagnosis, not re-run for this PR):

  • 16 games expanded to 1,004 member×nation rows.
  • The unread subplan ran 1,004 times in both the pagination COUNT and the main list query.
  • With JIT, PostgreSQL spent about 420 ms compiling the COUNT and 1,713 ms compiling the main query.
  • Replacing the unread annotation with a constant zero cut full in-process view time from 2,651 ms to 375 ms.
  • The phase/unit/supply-centre hydration from Scope latest-phase hydration on the game list and retrieve to the requested games #1398 took only about 9 ms.

Why zero is correct for can_join=true

  • can_join=true selects pending, public games (GameFilter.filter_can_join).
  • For authenticated users, the same filter excludes games the user is already a member of.
  • The unread query needs the user to be a channel member (channel__member_channels__member__user=user), and non-members can't be channel members. So the subquery always returns 0 for these rows.
  • Find Games only shows pending games, and GameCard only shows its unread badge for active or finished games.

Where the change lives

  • GameFilter.filter_can_join (service/game/filters.py): the joinable branch ends with queryset.with_zero_unread_counts().
    • The filter already receives django-filter's parsed boolean, so request parsing stays in GameFilter and nothing parses the filterset a second time.
    • Querysets are lazy, so the re-annotation replaces the correlated total_unread_message_count expression before evaluation.
    • can_join is applied before ordering, so the slots_remaining aggregation and the pagination COUNT only see the constant.
  • GameQuerySet.with_zero_unread_counts() (service/game/models.py): new method for the constant-zero annotation. The anonymous-user branch of with_total_unread_counts now uses it too.
  • GameListView (service/game/views.py): identical to main. It stays thin, and pagination plus the scoped phase hydration from Scope latest-phase hydration on the game list and retrieve to the requested games #1398 (paginate_queryset → hydrate_list_phases(page)) are untouched.
  • Retrieve, ?mine, normal list requests and channel lists work exactly as before.

can_join parsing

The optimisation fires only when django-filter's cleaned boolean is true, because it lives in the if value: branch of filter_can_join. GameFilter declares django_filters.BooleanFilter, whose field is forms.NullBooleanField with Django's NullBooleanSelect widget. I checked this against the endpoint, not only by reading the source:

can_join value Parsed as Filter + zero annotation applied
true, True, 2 True yes
false, False, 3 False no
1, 0, yes, other malformed values, absent None (unknown) no

1→true and 0→false is how django_filters.rest_framework.BooleanFilter behaves, but GameFilter doesn't use that class. This PR leaves parsing as it is. Whatever the filter treats as true is exactly what gets the zero annotation, so the two can't disagree.

Response schema

Unchanged. I generated manage.py spectacular (camelCase) and the internal snake_case schema (INTERNAL_SCHEMA_SETTINGS) on main and on this branch, and both are byte-identical. No codegen changes, no database field, no migration.

Tests

New tests in TestGameListViewQueryPerformance (service/game/tests.py):

  • test_list_joinable_games_skips_unread_message_subquery: authenticated can_join=true&ordering=slots_remaining over four pending games filled 3/2/1/0 (created in a different order), plus an active game where the user has 2 unread messages. It checks:

    • the results come back in exact slots_remaining order;
    • every total_unread_message_count is 0;
    • exactly one pagination COUNT(*) query and one page query hit game_game, and neither contains channel_channelmessage.

    It fails if the with_zero_unread_counts() line in filter_can_join is reverted.

  • test_list_games_counts_unread_messages_unless_can_join_is_true[false|malformed|absent]: with ordering=slots_remaining, the game still reports 2 unread messages, and both the COUNT and the page query still contain the unread subquery.

Run with service/.venv/bin/python:

  • python -m pytest game/tests.py -k "skips_unread or counts_unread_messages_unless" -v: 4 passed.
  • python -m pytest game/ channel/ -n auto: 499 passed. This includes the existing unread tests in game/tests/test_games_list_unread.py and channel/tests/test_channel.py, the slots-remaining ordering tests, and the Scope latest-phase hydration on the game list and retrieve to the requested games #1398 hydration tests.
  • On the first commit, python -m pytest -n auto --reuse-db: 2584 passed, 8 skipped, 1 failed. The failure is harness/tests.py::TestQualityMetricAggregation::test_accuracy_covers_ranked_samples_and_skips_the_rest, which also fails on unmodified main (AttributeError: 'NoneType' object has no attribute 'scores') and is unrelated to this change.

Checklist

  • This PR does one thing — no unrelated fixes, refactors, or drive-by cleanups bundled in
  • For PRs of any significant complexity: I ran /review-pr against this PR in Claude Code and addressed (or responded to) its findings
  • Tests cover the change
  • Screenshots embedded in the PR description for any visual changes (see CLAUDE.md): no visual changes, backend only

🤖 Generated with Claude Code

https://claude.ai/code/session_01S5LGbFJXETpBtFju7A2RUQ

Find Games requests can_join=true, which only returns pending games the
user is not a member of. The unread count needs channel membership, so
it is always zero there, but the correlated ChannelMessage subquery ran
once per member x nation row produced by slots_remaining ordering in
both the pagination COUNT and the page query. Annotate a constant zero
when the filterset parses can_join as true.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5LGbFJXETpBtFju7A2RUQ
johnpooch pushed a commit that referenced this pull request Sep 25, 2026
Match can_join the way GameFilter does, so every spelling the filter
treats as true (true, True) takes the zero fast path, and the helper is
the same one #1402 introduces.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016sj2eM1KW37VqohqwKViPh
filter_can_join already receives django-filter's parsed boolean, so the
view no longer builds and validates the filterset a second time.
Re-annotating total_unread_message_count on the lazy queryset replaces
the correlated unread subquery before SQL is compiled, and can_join is
applied before ordering, so the slots_remaining aggregation sees the
constant zero. GameListView returns to its original shape.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5LGbFJXETpBtFju7A2RUQ
@johnpooch
johnpooch merged commit 9be7f0e into main Sep 25, 2026
22 checks passed
johnpooch pushed a commit that referenced this pull request Sep 25, 2026
filter_can_join() now annotates zero unread messages. The list queryset
no longer carries the unread subquery on any path, so #1402's test for
non-joinable lists asserts the unread count through the single
page-scoped query instead of through the game-list queries.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016sj2eM1KW37VqohqwKViPh
johnpooch added a commit that referenced this pull request Sep 25, 2026
* Count game-list unread messages for the page, after pagination

GameListView annotated every game with a correlated ChannelMessage
subquery before filtering and pagination. That subquery ran inside the
pagination COUNT, was evaluated over member x nation rows under
ordering=slots_remaining, and pushed PostgreSQL's cost estimates high
enough to spend seconds in JIT compilation.

The list queryset no longer carries the annotation. Once the page is
known, GameManager.hydrate_total_unread_counts() runs one grouped query
over the page's game ids and sets total_unread_message_count on every
game, zero included. Anonymous requests and can_join=true (the user is a
member of none of the listed games) get zero without a query.

The unread predicate (channels the user belongs to, messages after that
channel's last_read_at, not sent by the user) moves to
ChannelMessageQuerySet.unread_by() and is shared with the retrieve
annotation, whose SQL is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016sj2eM1KW37VqohqwKViPh

* Detect joinable listings through the game filter's own parsing

Match can_join the way GameFilter does, so every spelling the filter
treats as true (true, True) takes the zero fast path, and the helper is
the same one #1402 introduces.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016sj2eM1KW37VqohqwKViPh

* Let unread hydration respect an existing annotation

hydrate_total_unread_counts() now fills only games that do not already
carry total_unread_message_count. filter_can_join() annotates zero, so
joinable listings skip the unread query without the view re-parsing
can_join, and the joinable_only flag and lists_joinable_games() go.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016sj2eM1KW37VqohqwKViPh

---------

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants