Backend: Create new message thread queries and add public trips by nabramow · Pull Request #9219 · Couchers-org/couchers

nabramow · 2026-07-01T17:28:09Z

Closes #8962

Adds new message thread query that better fits the new UI. Also adds public trips to it.

FYI - I have the frontend in another branch off this one na/web/messages-pub-trips and tested this there.

Testing

Explain how you tested this PR and give clear steps so the reviewer can replicate.

Backend checklist

Added tests for any new code or added a regression test if fixing a bug
Run the backend locally and it works
Added migrations if there are any database changes, rebased onto develop if necessary for linear migration history

For maintainers

Maintainers can push commits to my branch
Maintainers can merge this PR for me

CouchersBot · 2026-07-01T17:52:18Z

Mobile

Install the Dev Tool (iOS via TestFlight, iOS Simulator, or Android .apk) here.

Scan the QR with your phone camera, or tap Open in Dev Tool on the device, to open this branch in the installed Dev Tool dev client.

iOS	Android

Open in Dev Tool	Open in Dev Tool

Deep links

iOS

couchers-devtool://expo-development-client/?url=https%3A%2F%2Fe08821c7--ota.preview.couchershq.org%2Fios%2Fmanifest

Android

couchers-devtool://expo-development-client/?url=https%3A%2F%2Fe08821c7--ota.preview.couchershq.org%2Fandroid%2Fmanifest

Web (Vercel)

View the Vercel web preview for this branch.

Backend

Schema	Schema diff	Sample emails

Other

Protos	Backend coverage	Web coverage

aapeliv

this is quite a huge PR. i added some initial comments but didn't have the chance to go through the entirety of the new logic in detail yet.

aapeliv · 2026-07-06T05:09:50Z

            unseen_sent_host_request_count=unseen_sent_host_request_count,
            unseen_received_host_request_count=unseen_received_host_request_count,
+            unseen_hosting_host_request_count=unseen_hosting_host_request_count,
+            unseen_surfing_host_request_count=unseen_surfing_host_request_count,
+            unseen_public_trip_offer_count=unseen_public_trip_offer_count,


unseen_sent_host_request_count should equal unseen_surfing_host_request_count pre-public trips, right? is there a reason to have them separate?

You're right unseen_sent_host_request_count should unseen_surfing_host_request_count compute the same number today as right now every request's surfer is also its initiator.

However the code keeps them separate anticipating public trips, where that will stop being true.

I think once public trips is out we could migrate the clients off sent/received onto surfing/hosting?

aapeliv · 2026-07-06T05:11:18Z

+        # Role-based unread host-request counts. Unlike sent/received above, these
+        # classify by stay-role, so public-trip offers (role reversal) bucket
+        # correctly: the offering host counts under hosting, the traveller under surfing.
+        def role_based_unseen_host_request_count(party_predicate: ColumnElement[bool]) -> int:


i am concerned about the performance of the Ping endpoint, and this will add a good chunk of extra work. we should optimize these queries to instead subquery all requests where the given user is a party and that have unread messages, then use that as a subquery to count by type.

i can do it as a follow up, so doesn't need to block this.

makes sense!

aapeliv · 2026-07-06T05:13:38Z

+                .group_by(HostRequest.status)
+            ).all()
+        )
+        pb.offer_tally.CopyFrom(


i'm not sure you need CopyFrom here, i think you can just assign?

yeah looks like I can as long as it's not the whole message

aapeliv · 2026-07-06T05:14:37Z

 }

+enum MessageThreadFilter {
+  MESSAGE_THREAD_FILTER_ALL = 0;


needs an unspecified

Hmm but there is no unspecified in practice or you mean just use ALL as unspecified?

I will add it but have it be an invalid argument I think

aapeliv · 2026-07-06T05:15:28Z

+  Message latest_message = 9;
+  string hosting_city = 10;
+  bool is_archived = 11;
+  // true if this thread is a public-trip offer (public_trip_id is set)


also requires current user to be recipient?

Good catch, this made me realize that the flag is redundant anyway.

is_public_trip_offer just duplicated public_trip_id's presence, and viewer_is_host is derivable from host_user_id == myId. Since roles are already sent viewer-independently, I dropped both.

Now we can tell :

is it an offer? → public_trip_id present

am I the host? → host_user_id == me

is it an offer to me? → public_trip_id present and host_user_id != me.

nabramow · 2026-07-16T16:35:18Z

this is quite a huge PR. i added some initial comments but didn't have the chance to go through the entirety of the new logic in detail yet.

Yeah I wasn't sure how else to break it up as it's basically one big query!

tristanlabelle

I still need to go through conversations.py due to its size but so far I left comments about code duplication and a few API shape thoughts.

tristanlabelle · 2026-07-17T11:33:14Z


        return host_request_to_pb(host_request, session, context)

+    # TODO(remove after FE migrates to ListMessageThreads)


Can you have this TODO refer to a GitHub issue? (# TODO(#1234): blah blah blah)

tristanlabelle · 2026-07-17T11:35:46Z

  bool same_gender_only = 10;
  // Number of non-cancelled host offers linked to this trip.
  // Only populated when the viewer is the trip owner.
  optional int32 offers_count = 11;


Should we deprecate in favor of offer_tally?

tristanlabelle · 2026-07-17T15:42:50Z

+// A host request (or public-trip offer) shaped for the unified thread list.
+// surfer_user_id/host_user_id are ROLE-based: for a public-trip offer (role
+// reversal) they are swapped relative to the underlying initiator/recipient.
+message HostRequestThread {


This is duplicating HostRequest's every field, with subtle changes. We should be referencing it instead. For example now I'd need to add a timezone in two places.

tristanlabelle · 2026-07-17T15:47:37Z

  bool no_more = 3;
 }

+enum MessageThreadFilter {


Even if the UI has those mutually exclusive filters, would it make sense for the backend to model those as combinable? E.g. support specifying searching for Hosting + Surfing?

tristanlabelle · 2026-07-17T16:00:48Z

+            node_id=node.id,
+            from_date=today() + timedelta(days=5),
+            to_date=today() + timedelta(days=10),
+            description="x" * 200,


Avoid the 200 magic number, can we reference a constant?

tristanlabelle · 2026-07-17T16:02:47Z

+        node = Node(
+            geom=to_multi(create_polygon_lat_lng([[60, 24], [60, 26], [62, 26], [62, 24], [60, 24]])),
+            node_type=NodeType.locality,
+        )
+        session.add(node)
+        session.flush()
+        session.add(
+            Cluster(
+                name="Test community",
+                description="Test",
+                parent_node_id=node.id,
+                is_official_cluster=True,
+                small_community_features_enabled=True,
+            )
+        )
+        session.flush()


Let's (extract and) reuse create_community from test_communities.py.

tristanlabelle · 2026-07-17T16:05:07Z

+            )
+        )
+        session.flush()
+        trip = PublicTrip(


Would be great to reuse _create_trip_directly and just have one place creating DB entries if we can't use the API.

tristanlabelle · 2026-07-17T16:09:29Z

+        expected_ids.add(_create_group_chat(token1, [other.id], moderator))
+        expected_ids.add(_create_host_request(other_token, user1.id, moderator))


Are group chat and host request IDs necessarily non-overlapping? I think we should track them separately.

tristanlabelle · 2026-07-17T16:21:02Z

+        unseen_hosting_host_request_count = role_based_unseen_host_request_count(
+            or_(
+                and_(HostRequest.public_trip_id.is_(None), HostRequest.recipient_user_id == me),
+                and_(HostRequest.public_trip_id.isnot(None), HostRequest.initiator_user_id == me),
+            )
+        )
+        unseen_surfing_host_request_count = role_based_unseen_host_request_count(
+            or_(
+                and_(HostRequest.public_trip_id.is_(None), HostRequest.initiator_user_id == me),
+                and_(HostRequest.public_trip_id.isnot(None), HostRequest.recipient_user_id == me),
+            )
+        )


I think we could make this a single database query for entries where either the initiator or recipient is "me", then do the different types of counting in Python. Maybe we can even cover the 5 counts that way in a single query.

aapeliv

had a look through the big function and left a lot of comments.

overall, it's quite a big piece. this is touching a high volume surface, so some perf comments as well.

there is a lot of "if public trip, then switch surfer/host", which is something i was hoping we would avoid with the public trips design. i wonder if we are shoehorning it in in a way that is going to be difficult down the line.

aapeliv · 2026-07-20T04:56:32Z

+    if unread:
+        # restrict to chats with at least one message newer than the user's last-seen
+        candidate_query = candidate_query.join(
+            GroupChatSubscription, GroupChatSubscription.id == visible_group_chats.c.subscription_id


you are double-joining here on GroupChatSubscription; you can move this above where you wrap in the select, so then you don't need this join. similar to only_archived

will do, good catch

aapeliv · 2026-07-20T05:00:56Z

+            and_(HostRequest.public_trip_id.is_(None), HostRequest.initiator_user_id == viewer_id),
+            and_(HostRequest.public_trip_id.isnot(None), HostRequest.recipient_user_id == viewer_id),
+        )
+    if thread_filter == conversations_pb2.MESSAGE_THREAD_FILTER_PUBLIC_TRIPS:


let's call this MY_PUBLIC_TRIPS, or SURFING_PUBLIC_TRIPS; it is confusing otherwise

I'll do MY_PUBLIC_TRIPS

aapeliv · 2026-07-20T05:09:35Z

+def _host_request_viewer_last_seen(host_request: HostRequest, user_id: int) -> int:
+    """The viewer's role-specific last-seen message id for a host request."""
+    if host_request.initiator_user_id == user_id:
+        return host_request.initiator_last_seen_message_id


please add a similar if for recipient_user_id, and throw if the user_id matches neither; that's safer

aapeliv · 2026-07-20T05:11:19Z

+    return conversations_pb2.GroupChat(
+        group_chat_id=group_chat.conversation_id,
+        title=group_chat.title,  # TODO: proper title for DMs, etc
+        member_user_ids=_get_visible_members_for_subscription(subscription),
+        admin_user_ids=_get_visible_admins_for_subscription(subscription),
+        only_admins_invite=group_chat.only_admins_invite,
+        is_dm=group_chat.is_dm,
+        created=Timestamp_from_datetime(group_chat.conversation.created),
+        unseen_message_count=unseen_message_count,
+        last_seen_message_id=subscription.last_seen_message_id,
+        latest_message=_message_to_pb(message) if message else None,
+        mute_info=_mute_info(subscription),
+        can_message=_user_can_message(session, context, group_chat),
+        is_archived=subscription.is_archived,
+    )


We could take this opportunity to prune stuff that we don't use in the frontend; e.g. the visible members/admins is N queries per group chat.

aapeliv · 2026-07-20T05:17:01Z

+    latest_per_group_chat = (
+        select(
+            GroupChatSubscription.group_chat_id.label("group_chat_id"),
+            func.max(GroupChatSubscription.id).label("subscription_id"),
+            func.max(Message.id).label("message_id"),
+        )
+        .join(Message, Message.conversation_id == GroupChatSubscription.group_chat_id)
+        .where(GroupChatSubscription.user_id == context.user_id)
+        .where(GroupChatSubscription.group_chat_id.in_(group_chat_ids))
+        .where(Message.time >= GroupChatSubscription.joined)
+        .where(or_(Message.time <= GroupChatSubscription.left, GroupChatSubscription.left == None))
+        .group_by(GroupChatSubscription.group_chat_id)
+        .subquery()
+    )


I think this logic is duplicated now multiple times? In existing code as well. Given the complexity of this query, we should only construct it once. (Similar with group chats)

aapeliv · 2026-07-20T05:22:14Z

+        page_size = request.page_size if request.page_size != 0 else DEFAULT_PAGINATION_LENGTH
+        page_size = min(page_size, MAX_PAGE_SIZE)
+        only_archived = request.only_archived if request.HasField("only_archived") else None
+        unread = thread_filter == conversations_pb2.MESSAGE_THREAD_FILTER_UNREAD


only_unread?

aapeliv · 2026-07-20T05:24:34Z

+
+        page_rows = candidate_rows[:page_size]
+        no_more = len(candidate_rows) <= page_size
+        next_page_token = str(min(row.latest_message_id for row in page_rows)) if (page_rows and not no_more) else ""


could we encrypt the page token please? there's a helper function. that way it's truly opaque

Ah yes done.

aapeliv · 2026-07-20T05:26:24Z

+        group_chats_by_id = _build_group_chats_pb(session, context, group_chat_ids)
+        host_request_threads_by_id = _build_host_request_threads_pb(
+            session, context, host_request_ids, latest_message_id_by_conversation
+        )


you are running those queries twice now for each group chat/host request. let's avoid that if we can, since this API will be called a lot. i have done some optimization on the exsting group chats queries in the past given it was taking lots of compute.

aapeliv · 2026-07-20T05:27:06Z

+        if include_chats:
+            latest_message_id_by_conversation = {
+                row.conversation_id: row.latest_message_id
+                for row in session.execute(_group_chat_candidate_query(context, only_archived, unread)).all()


it looks like you are using the same logic and same candidate query building; why not unify further?

aapeliv · 2026-07-20T05:27:53Z

+                    latest_message_id = latest_message_id_by_conversation[host_request.conversation_id]
+                    if host_request.initiator_user_id == context.user_id:
+                        if host_request.initiator_last_seen_message_id < latest_message_id:
+                            host_request.initiator_last_seen_message_id = latest_message_id
+                    elif host_request.recipient_last_seen_message_id < latest_message_id:
+                        host_request.recipient_last_seen_message_id = latest_message_id


This will issue queries for every host request separately; we can use a bulk update to reduce the roundtrips

nabramow added 2 commits June 30, 2026 17:37

Add new backend messages queries

e3dc4b5

Fix status change error

cf9ebf7

nabramow requested a review from aapeliv as a code owner July 1, 2026 17:28

Improve naming in queries

30da027

vercel Bot deployed to Preview July 1, 2026 17:56 View deployment

nabramow changed the title ~~Backend: Refactor messages screen queries and add public trips~~ Backend: Create new message thread queries and add public trips Jul 2, 2026

aapeliv reviewed Jul 6, 2026

View reviewed changes

Address review comments

9c6d184

nabramow requested a review from tristanlabelle as a code owner July 16, 2026 17:46

Merge branch 'develop' into na/backend/messages-queries

110f16a

vercel Bot deployed to Preview July 16, 2026 17:52 View deployment

tristanlabelle reviewed Jul 17, 2026

View reviewed changes

aapeliv reviewed Jul 20, 2026

View reviewed changes

Merge branch 'develop' into na/backend/messages-queries

8ef8921

vercel Bot deployed to Preview July 22, 2026 14:24 View deployment

Address quick win review comments

e08821c

vercel Bot deployed to Preview July 22, 2026 15:57 View deployment

nabramow mentioned this pull request Jul 22, 2026

Backend: Split shared conversation types into a leaf proto file #9306

Open

5 tasks


		return host_request_to_pb(host_request, session, context)

		# TODO(remove after FE migrates to ListMessageThreads)

		expected_ids.add(_create_group_chat(token1, [other.id], moderator))
		expected_ids.add(_create_host_request(other_token, user1.id, moderator))

Uh oh!

Conversation

nabramow commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

For maintainers

Uh oh!

CouchersBot commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Mobile

Web (Vercel)

Backend

Other

Uh oh!

aapeliv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nabramow commented Jul 16, 2026

Uh oh!

tristanlabelle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aapeliv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

nabramow commented Jul 1, 2026 •

edited

Loading

CouchersBot commented Jul 1, 2026 •

edited

Loading