Skip to content

perf(llc): Reduce the number of DB reads in ChatPersistenceClient.getChannelStates()#2664

Open
VelikovPetar wants to merge 11 commits into
masterfrom
feature/optimize-query-channels-offline
Open

perf(llc): Reduce the number of DB reads in ChatPersistenceClient.getChannelStates()#2664
VelikovPetar wants to merge 11 commits into
masterfrom
feature/optimize-query-channels-offline

Conversation

@VelikovPetar
Copy link
Copy Markdown
Contributor

@VelikovPetar VelikovPetar commented May 19, 2026

Submit a pull request

Linear: FLU-484

CLA

  • I have signed the Stream CLA (required).
  • The code changes follow best practices
  • Code changes are tested (add some information if not applicable)

Description of the pull request

Reduces the amount of DB reads done in ChatPersistenceClient.getChannelStates by

  1. Fetch the lightweight channel data
  2. Sort / slice the required page
  3. Enrich only the relevant page (previously we enriched ALL fetched channels, not just the page)

Renames some leftover docs / private methods referencing Moor instead of Drift

Screenshots / Videos

NA

Testing

The following patch contains a benchmark test - comparing the new and old implementations. You can run the bench locally to verify the changes in performance.

Benchmark
Subject: [PATCH] [DB] GetChannelStates benchmark tests.
---
Index: packages/stream_chat_persistence/test/src/benchmark/counting_query_interceptor.dart
===================================================================
diff --git a/packages/stream_chat_persistence/test/src/benchmark/counting_query_interceptor.dart b/packages/stream_chat_persistence/test/src/benchmark/counting_query_interceptor.dart
new file mode 100644
--- /dev/null	(date 1779109117584)
+++ b/packages/stream_chat_persistence/test/src/benchmark/counting_query_interceptor.dart	(date 1779109117584)
@@ -0,0 +1,117 @@
+import 'package:drift/drift.dart';
+
+/// A [QueryInterceptor] that counts SQL statements executed against the
+/// underlying [QueryExecutor]. Use in benchmark tests via:
+///
+/// ```dart
+/// final counter = CountingQueryInterceptor();
+/// final executor = NativeDatabase.memory().interceptWith(counter);
+/// final db = DriftChatDatabase('user', executor);
+/// // ... run code ...
+/// print('Total queries: ${counter.totalStatements}');
+/// ```
+class CountingQueryInterceptor extends QueryInterceptor {
+  int selects = 0;
+  int inserts = 0;
+  int updates = 0;
+  int deletes = 0;
+  int customs = 0;
+  int batchedCalls = 0;
+  int batchedStatements = 0;
+  int rowsReturned = 0;
+
+  /// Total statements executed, counting each batched statement individually.
+  int get totalStatements =>
+      selects + inserts + updates + deletes + customs + batchedStatements;
+
+  /// Total executor invocations — useful to see how many fan-out calls were
+  /// made independent of batching. Treats one `runBatched` as one call.
+  int get totalInvocations =>
+      selects + inserts + updates + deletes + customs + batchedCalls;
+
+  void reset() {
+    selects = 0;
+    inserts = 0;
+    updates = 0;
+    deletes = 0;
+    customs = 0;
+    batchedCalls = 0;
+    batchedStatements = 0;
+    rowsReturned = 0;
+  }
+
+  @override
+  Future<List<Map<String, Object?>>> runSelect(
+    QueryExecutor executor,
+    String statement,
+    List<Object?> args,
+  ) async {
+    selects++;
+    final result = await executor.runSelect(statement, args);
+    rowsReturned += result.length;
+    return result;
+  }
+
+  @override
+  Future<int> runInsert(
+    QueryExecutor executor,
+    String statement,
+    List<Object?> args,
+  ) {
+    inserts++;
+    return executor.runInsert(statement, args);
+  }
+
+  @override
+  Future<int> runUpdate(
+    QueryExecutor executor,
+    String statement,
+    List<Object?> args,
+  ) {
+    updates++;
+    return executor.runUpdate(statement, args);
+  }
+
+  @override
+  Future<int> runDelete(
+    QueryExecutor executor,
+    String statement,
+    List<Object?> args,
+  ) {
+    deletes++;
+    return executor.runDelete(statement, args);
+  }
+
+  @override
+  Future<void> runCustom(
+    QueryExecutor executor,
+    String statement,
+    List<Object?> args,
+  ) {
+    customs++;
+    return executor.runCustom(statement, args);
+  }
+
+  @override
+  Future<void> runBatched(
+    QueryExecutor executor,
+    BatchedStatements statements,
+  ) {
+    batchedCalls++;
+    batchedStatements += statements.arguments.length;
+    return executor.runBatched(statements);
+  }
+
+  @override
+  String toString() => 'CountingQueryInterceptor('
+      'selects: $selects, '
+      'inserts: $inserts, '
+      'updates: $updates, '
+      'deletes: $deletes, '
+      'customs: $customs, '
+      'batchedCalls: $batchedCalls, '
+      'batchedStatements: $batchedStatements, '
+      'totalStatements: $totalStatements, '
+      'rowsReturned: $rowsReturned'
+      ')';
+}
Index: packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart b/packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart
--- a/packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart	(revision 9710f8a7d3075c92e6f0971564c25445e1e0701c)
+++ b/packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart	(date 1779177736535)
@@ -261,6 +261,41 @@
     );
   }
 
+  /// Legacy full-hydration variant of [getChannelStates], retained for
+  /// benchmark parity. Not part of the abstract contract.
+  Future<List<ChannelState>> getChannelStatesLegacy({
+    Filter? filter,
+    SortOrder<ChannelState>? channelStateSort,
+    PaginationParams? paginationParams,
+  }) async {
+    assert(_debugIsConnected, '');
+    _logger.info('getChannelStatesLegacy');
+
+    final channels = await db!.channelQueryDao.getChannels(filter: filter);
+
+    final channelStates = await Future.wait(
+      channels.map((e) => getChannelStateByCid(e.cid)),
+    );
+
+    // Sort the channel states
+    if (channelStateSort != null && channelStateSort.isNotEmpty) {
+      channelStates.sort(channelStateSort.compare);
+    }
+
+    // Apply offset
+    if (paginationParams?.offset case final paginationOffset?) {
+      final clampedOffset = paginationOffset.clamp(0, channelStates.length);
+      channelStates.removeRange(0, clampedOffset);
+    }
+
+    // Apply limit
+    if (paginationParams?.limit case final paginationLimit?) {
+      return channelStates.take(paginationLimit).toList();
+    }
+
+    return channelStates;
+  }
+
   @override
   Future<List<ChannelState>> getChannelStates({
     Filter? filter,
Index: packages/stream_chat_persistence/test/src/benchmark/RESULTS.md
===================================================================
diff --git a/packages/stream_chat_persistence/test/src/benchmark/RESULTS.md b/packages/stream_chat_persistence/test/src/benchmark/RESULTS.md
new file mode 100644
--- /dev/null	(date 1779176239885)
+++ b/packages/stream_chat_persistence/test/src/benchmark/RESULTS.md	(date 1779176239885)
@@ -0,0 +1,104 @@
+# `getChannelStates` Benchmark Results
+
+## TL;DR
+
+The new `getChannelStates` reduces cold-start SQL workload by **~3–10× depending on cache size and page size** compared to `getChannelStatesLegacy`. For the realistic "active user with a populated cache" scenario (250 cached channels × 100 messages each, page size 30):
+
+- **SQL statements**: 78,002 → 9,362 (**−88.0%, 8.3× fewer**)
+- **Rows fetched**: 28,750 → 3,890 (**−86.5%, 7.4× fewer**)
+- **Wall time** (in-memory SQLite, single isolate): 8,408 ms → 984 ms (**−88.3%, 8.5× faster**)
+
+Equivalence is asserted across all sort keys: both methods return the same channels in the same order for the page.
+
+## What's measured
+
+Three signals captured via `CountingQueryInterceptor` (Drift `QueryInterceptor` API) and `Stopwatch`:
+
+| Metric | How |
+|---|---|
+| Statements executed | Sum of `runSelect` + `runInsert` + `runUpdate` + `runDelete` + `runCustom` + batched statements |
+| Rows returned | Sum of `runSelect` result lengths |
+| Wall time | `Stopwatch.elapsed` around the call |
+
+Not measured: memory pressure, GC, isolate overhead. In-memory `NativeDatabase.memory()`, single isolate, no disk I/O — wall-time numbers represent CPU + SQLite engine cost only. On real devices with disk-backed SQLite, the wall-time advantage will be larger because the OLD method does ~3–10× more I/O.
+
+## How to reproduce
+
+```bash
+cd packages/stream_chat_persistence
+flutter test test/src/benchmark/get_channel_states_bench_test.dart
+```
+
+Output is printed by each test. Suite takes ~25 seconds locally.
+
+## Results
+
+| Scenario | OLD stmts | NEW stmts | stmts Δ | stmts speedup | OLD rows | NEW rows | rows Δ | OLD ms | NEW ms | time Δ | time speedup |
+|---|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|---:|
+| 100ch × 25msgs, lastUpdated sort, page 30 | 8702 | 2612 | −70.0% | 3.3× fewer | 4000 | 1340 | −66.5% | 1015 | 292 | −71.2% | 3.5× faster |
+| 100ch × 25msgs, lastMessageAt sort, page 30 | 8702 | 2612 | −70.0% | 3.3× fewer | 4000 | 1340 | −66.5% | 1012 | 371 | −63.3% | 2.7× faster |
+| 100ch × 25msgs, memberCount sort, page 30 | 8702 | 2612 | −70.0% | 3.3× fewer | 4000 | 1340 | −66.5% | 967 | 282 | −70.8% | 3.4× faster |
+| 100ch × 25msgs, pinnedAt sort, page 30 | 8702 | 2613 | −70.0% | 3.3× fewer | 4000 | 1440 | −64.0% | 926 | 275 | −70.3% | 3.4× faster |
+| 100ch × 25msgs, multi-field sort, page 30 | 8702 | 2613 | −70.0% | 3.3× fewer | 4000 | 1440 | −64.0% | 911 | 277 | −69.6% | 3.3× faster |
+| 100ch × 25msgs, extraData sort, page 30 | 8702 | 2612 | −70.0% | 3.3× fewer | 4000 | 1340 | −66.5% | 911 | 269 | −70.5% | 3.4× faster |
+| 100ch × 25msgs, **page 10** | 8702 | **872** | **−90.0%** | **10.0× fewer** | 4000 | 580 | −85.5% | 941 | **84** | **−91.1%** | **11.2× faster** |
+| 100ch × 25msgs, page 100 (whole cache) | 8702 | 8702 | 0% | tied | 4000 | 4000 | 0% | 899 | 901 | +0.2% | tied |
+| 100ch × 25msgs, page 2 (offset 30) | 8702 | 2612 | −70.0% | 3.3× fewer | 4000 | 1340 | −66.5% | 913 | 271 | −70.3% | 3.4× faster |
+| 50ch × 25msgs, page 30 | 4352 | 2612 | −40.0% | 1.7× fewer | 2000 | 1240 | −38.0% | 449 | 266 | −40.8% | 1.7× faster |
+| **250ch × 100msgs, page 30** | **78002** | **9362** | **−88.0%** | **8.3× fewer** | **28750** | **3890** | **−86.5%** | **8408** | **984** | **−88.3%** | **8.5× faster** |
+
+Run on macOS Darwin 25.4.0, single in-memory SQLite, single isolate. Wall times vary ±20% across runs; treat as orders-of-magnitude comparisons.
+
+## Where the speedup comes from
+
+**OLD** (`getChannelStatesLegacy`) loads full state for every cached channel before sorting and paginating in Dart:
+
+```
+1 (cids lookup)
++ 1 (channels with JOIN users)
++ N × 6 fan-out (members, reads, channel, messages, pinned, draft per cid)
++ messages × ~3 sub-queries each (reactions, quoted, polls, drafts)
+```
+
+For 100 channels × 25 messages: ≈ 1 + 1 + 100×6 + 100×25×3 = **~8200 statements**. Matches measured 8702.
+
+**NEW** (`getChannelStates`) loads only channel rows, sorts/paginates in Dart on lightweight envelopes, then hydrates state only for the visible page:
+
+```
+1 (cids lookup)
++ 1 (channels with JOIN users)
++ (1 if sort uses pinnedAt — batched memberships)
++ pageSize × 6 fan-out for the visible page
++ pageSize × messagesPerChannel × ~3 sub-queries
+```
+
+For 30 visible channels × 25 messages: ≈ 1 + 1 + 30×6 + 30×25×3 = **~2430 statements**. Matches measured 2612.
+
+The constant per-channel hydration cost is the floor; everything saved is the work that *isn't* done for off-page channels.
+
+## Key observations
+
+1. **The 250 ch × 100 msg case is the bug** — 78k → 9k SQL statements (−88.0%), 8.4s → 0.98s (−88.3%). That's a near-cap real-world cold start (the sync cap is 250) with chatty channels.
+
+2. **Speedup scales as `cache_size / page_size`** — page 10 yields 10× (−90.0%), page 30 yields 3.3× (−70.0%), page 100 (full cache) yields 1× (0%). The "wasted hydration" is what gets eliminated.
+
+3. **`pinnedAt` sort adds exactly 1 statement** (2613 vs 2612) for the batched membership preload. Sub-millisecond overhead. Negligible.
+
+4. **Custom (`extraData`) sort works identically to built-in keys** in NEW, because `ChannelState.getComparableField` falls through to `channel?.extraData[sortKey]` and `ChannelModel.extraData` is loaded with the row. No special handling needed.
+
+5. **Equivalence holds across all sort keys**, including the harder cases (`pinnedAt` with 75 nullsFirst ties; multi-field with primary + tiebreaker; extraData sort). Both methods return identical cid lists in identical order.
+
+6. **Small caches (50 ch) show smaller speedups** — only 1.7× fewer statements (−40.0%). When you cache 50 and show 30, the per-channel cost dominates; not much room to win. Wins grow with cache size, which is the case we care about.
+
+7. **Whole-cache pages tie**, which is the right behavior. When you ask for every channel, NEW must hydrate every channel. The `assertSpeedup: false` flag in the test codifies this.
+
+## Where NEW differs from OLD
+
+- **Message list per channel**: OLD loads *every* cached message; NEW caps at 25 (matches the API default). Returned `ChannelState.messages` may have a different `length` for channels with >25 cached messages.
+- **Order within ties**: Both rely on Dart's stable sort against rows from the same DAO call, so ties resolve consistently. Verified by equivalence assertions.
+
+## Future work (not in this PR)
+
+- **Alt B (iOS-style batched per-channel queries)**: replace the page's 6×pageSize fan-out with 6 batched IN-clause queries. Would drop the 250-channel scenario from ~9k to ~6 statements.
+- **Drift `.watch()`-based live observer**: replace the re-query-on-change approach with reactive streams (analog to iOS's `NSFetchedResultsController`). Bigger refactor.
+- **Filter → SQL `WHERE` translation**: not required for offline-first-paint (the `query_hash` cache scopes correctly).
Index: packages/stream_chat_persistence/test/src/benchmark/fixture_builder.dart
===================================================================
diff --git a/packages/stream_chat_persistence/test/src/benchmark/fixture_builder.dart b/packages/stream_chat_persistence/test/src/benchmark/fixture_builder.dart
new file mode 100644
--- /dev/null	(date 1779109311037)
+++ b/packages/stream_chat_persistence/test/src/benchmark/fixture_builder.dart	(date 1779109311037)
@@ -0,0 +1,206 @@
+import 'dart:math';
+
+import 'package:stream_chat/stream_chat.dart';
+import 'package:stream_chat_persistence/src/db/drift_chat_database.dart';
+
+/// Result of seeding the database with channel data.
+class SeedResult {
+  SeedResult({required this.cids, required this.userIds});
+
+  /// All channel CIDs that were seeded, in insertion order.
+  final List<String> cids;
+
+  /// All user IDs that were seeded (includes [currentUserId] if provided).
+  final List<String> userIds;
+}
+
+/// Seeds an in-memory [DriftChatDatabase] with channels and their full state
+/// for benchmark tests.
+///
+/// The data is generated deterministically (seeded random) and varied across
+/// channels so different sort orderings yield different permutations.
+///
+/// - [channelCount] channels are created.
+/// - Each channel gets [messagesPerChannel] messages, [membersPerChannel]
+///   members (including [currentUserId] when provided), [readsPerChannel]
+///   reads, [pinnedPerChannel] pinned messages.
+/// - The channel CIDs are registered under the given [filter]'s query hash.
+/// - Membership rows for [currentUserId] include a pinnedAt date so the
+///   `pinnedAt` sort can be exercised.
+Future<SeedResult> seedChannels(
+  DriftChatDatabase db, {
+  required int channelCount,
+  String currentUserId = 'testUserId',
+  int messagesPerChannel = 25,
+  int membersPerChannel = 5,
+  int readsPerChannel = 5,
+  int pinnedPerChannel = 2,
+  Filter? filter,
+  int seed = 0xC0DEBABE,
+}) async {
+  final rng = Random(seed);
+  final baseDate = DateTime.utc(2025, 1, 1);
+
+  // -------------------------------------------------------------------------
+  // Users
+  // -------------------------------------------------------------------------
+  // Pool of user IDs reused across channels as member/read authors.
+  final memberUserIds = List.generate(
+    max(membersPerChannel * 2, 10),
+    (i) => 'member_user_$i',
+  );
+
+  // The current user is always in the pool so its membership exists everywhere.
+  final allUserIds = <String>{currentUserId, ...memberUserIds}.toList();
+
+  final users = [
+    for (final id in allUserIds)
+      User(id: id, name: 'User $id', extraData: const {}),
+  ];
+  await db.userDao.updateUsers(users);
+
+  // -------------------------------------------------------------------------
+  // Channels — vary lastMessageAt, createdAt, updatedAt, memberCount, extraData
+  // so each sort key produces a distinct ordering.
+  // -------------------------------------------------------------------------
+  // Generate three independent permutations so lastMessageAt / createdAt /
+  // memberCount don't all sort the same way.
+  final lastMessagePermutation = _shuffledRange(channelCount, rng);
+  final createdAtPermutation = _shuffledRange(channelCount, rng);
+  final memberCountPermutation = _shuffledRange(channelCount, rng);
+
+  final cids = List.generate(channelCount, (i) => 'messaging:bench_$i');
+
+  final channels = List.generate(channelCount, (i) {
+    final lmaIndex = lastMessagePermutation[i];
+    final createdIndex = createdAtPermutation[i];
+    final memberCountIndex = memberCountPermutation[i];
+
+    return ChannelModel(
+      cid: cids[i],
+      type: 'messaging',
+      id: 'bench_$i',
+      config: ChannelConfig(),
+      createdBy: users.first,
+      createdAt: baseDate.add(Duration(minutes: createdIndex)),
+      updatedAt: baseDate.add(Duration(minutes: createdIndex + 5)),
+      lastMessageAt: baseDate.add(Duration(hours: lmaIndex)),
+      memberCount: memberCountIndex + 1,
+      extraData: {
+        'priority': memberCountIndex,
+        'group': i % 3 == 0 ? 'A' : (i % 3 == 1 ? 'B' : 'C'),
+      },
+    );
+  });
+  await db.channelDao.updateChannels(channels);
+
+  // -------------------------------------------------------------------------
+  // Members — current user always included; vary pinnedAt for sort coverage.
+  // -------------------------------------------------------------------------
+  final membersByCid = <String, List<Member>>{};
+  for (var i = 0; i < channelCount; i++) {
+    final cid = cids[i];
+    final memberList = <Member>[];
+
+    // Current user as first member; assign pinnedAt to some channels.
+    memberList.add(Member(
+      userId: currentUserId,
+      user: users.firstWhere((u) => u.id == currentUserId),
+      pinnedAt: i % 4 == 0 ? baseDate.add(Duration(days: i)) : null,
+      channelRole: 'member',
+    ));
+
+    // Fill in other members.
+    for (var m = 1; m < membersPerChannel; m++) {
+      final uid = memberUserIds[(i + m) % memberUserIds.length];
+      memberList.add(Member(
+        userId: uid,
+        user: users.firstWhere((u) => u.id == uid),
+        channelRole: 'member',
+      ));
+    }
+    membersByCid[cid] = memberList;
+  }
+  await db.memberDao.bulkUpdateMembers(membersByCid);
+
+  // -------------------------------------------------------------------------
+  // Reads
+  // -------------------------------------------------------------------------
+  final readsByCid = <String, List<Read>>{};
+  for (var i = 0; i < channelCount; i++) {
+    final cid = cids[i];
+    final readList = <Read>[];
+    for (var r = 0; r < readsPerChannel; r++) {
+      final uid = memberUserIds[(i + r) % memberUserIds.length];
+      readList.add(Read(
+        lastRead: baseDate.add(Duration(hours: i, minutes: r)),
+        user: users.firstWhere((u) => u.id == uid),
+        unreadMessages: r,
+      ));
+    }
+    readsByCid[cid] = readList;
+  }
+  await db.readDao.bulkUpdateReads(readsByCid);
+
+  // -------------------------------------------------------------------------
+  // Messages
+  // -------------------------------------------------------------------------
+  final messagesByCid = <String, List<Message>>{};
+  for (var i = 0; i < channelCount; i++) {
+    final cid = cids[i];
+    final author = users.firstWhere(
+      (u) => u.id == memberUserIds[i % memberUserIds.length],
+    );
+    final msgs = List.generate(messagesPerChannel, (m) {
+      return Message(
+        id: 'msg_${i}_$m',
+        text: 'Message $m in $cid',
+        type: 'regular',
+        user: author,
+        createdAt: baseDate.add(Duration(hours: i, minutes: m)),
+        updatedAt: baseDate.add(Duration(hours: i, minutes: m + 1)),
+      );
+    });
+    messagesByCid[cid] = msgs;
+  }
+  await db.messageDao.bulkUpdateMessages(messagesByCid);
+
+  // -------------------------------------------------------------------------
+  // Pinned messages (a subset of the regular messages)
+  // -------------------------------------------------------------------------
+  final pinnedByCid = <String, List<Message>>{};
+  for (var i = 0; i < channelCount; i++) {
+    final cid = cids[i];
+    final author = users.firstWhere(
+      (u) => u.id == memberUserIds[i % memberUserIds.length],
+    );
+    final pinned = List.generate(pinnedPerChannel, (p) {
+      return Message(
+        id: 'pinned_${i}_$p',
+        text: 'Pinned $p in $cid',
+        type: 'regular',
+        user: author,
+        createdAt: baseDate.add(Duration(hours: i, minutes: p * 5)),
+        updatedAt: baseDate.add(Duration(hours: i, minutes: p * 5 + 1)),
+        pinned: true,
+      );
+    });
+    pinnedByCid[cid] = pinned;
+  }
+  await db.pinnedMessageDao.bulkUpdateMessages(pinnedByCid);
+
+  // -------------------------------------------------------------------------
+  // Channel queries — register cids under the filter's query hash so
+  // ChannelQueryDao.getChannels(filter) returns them.
+  // -------------------------------------------------------------------------
+  await db.channelQueryDao.updateChannelQueries(filter, cids);
+
+  return SeedResult(cids: cids, userIds: allUserIds);
+}
+
+/// Returns a list `[0..count)` shuffled deterministically using [rng].
+List<int> _shuffledRange(int count, Random rng) {
+  final list = List<int>.generate(count, (i) => i);
+  list.shuffle(rng);
+  return list;
+}
Index: packages/stream_chat_persistence/test/src/benchmark/get_channel_states_bench_test.dart
===================================================================
diff --git a/packages/stream_chat_persistence/test/src/benchmark/get_channel_states_bench_test.dart b/packages/stream_chat_persistence/test/src/benchmark/get_channel_states_bench_test.dart
new file mode 100644
--- /dev/null	(date 1779175996004)
+++ b/packages/stream_chat_persistence/test/src/benchmark/get_channel_states_bench_test.dart	(date 1779175996004)
@@ -0,0 +1,485 @@
+import 'dart:math' show max;
+
+import 'package:drift/drift.dart';
+import 'package:drift/native.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:stream_chat/stream_chat.dart';
+import 'package:stream_chat_persistence/src/db/drift_chat_database.dart';
+import 'package:stream_chat_persistence/src/stream_chat_persistence_client.dart';
+
+import 'counting_query_interceptor.dart';
+import 'fixture_builder.dart';
+
+/// Snapshot of a single benchmark run.
+class BenchResult {
+  BenchResult({
+    required this.label,
+    required this.queryCount,
+    required this.invocationCount,
+    required this.rowsReturned,
+    required this.elapsed,
+    required this.resultCids,
+  });
+
+  final String label;
+  final int queryCount;
+  final int invocationCount;
+  final int rowsReturned;
+  final Duration elapsed;
+  final List<String> resultCids;
+}
+
+/// Runs [body] once, captures stats from [counter], and resets the counter.
+Future<BenchResult> runBench({
+  required String label,
+  required Future<List<ChannelState>> Function() body,
+  required CountingQueryInterceptor counter,
+}) async {
+  counter.reset();
+  final sw = Stopwatch()..start();
+  final result = await body();
+  sw.stop();
+  return BenchResult(
+    label: label,
+    queryCount: counter.totalStatements,
+    invocationCount: counter.totalInvocations,
+    rowsReturned: counter.rowsReturned,
+    elapsed: sw.elapsed,
+    resultCids: result.map((s) => s.channel?.cid).whereType<String>().toList(),
+  );
+}
+
+// ===========================================================================
+// Pretty-printing helpers
+// ===========================================================================
+
+const _ruleWidth = 76;
+
+String _rule([String? title]) {
+  if (title == null) return '─' * _ruleWidth;
+  final left = '─── $title ';
+  final padding = max(0, _ruleWidth - left.length);
+  return '$left${'─' * padding}';
+}
+
+String _signed(int v) => v >= 0 ? '+$v' : '$v';
+
+String _pct(num pct) {
+  final sign = pct >= 0 ? '+' : '';
+  return '$sign${pct.toStringAsFixed(1)}%';
+}
+
+/// Returns "3.3× fewer" / "tied" / "1.5× more" for count-style metrics where
+/// smaller is better.
+String _fewerPhrase(num oldValue, num newValue) {
+  if (oldValue == newValue) return 'tied';
+  if (newValue == 0) return '∞× fewer';
+  if (newValue < oldValue) {
+    return '${(oldValue / newValue).toStringAsFixed(1)}× fewer';
+  }
+  return '${(newValue / oldValue).toStringAsFixed(1)}× more';
+}
+
+/// Returns "4.2× faster" / "tied" / "1.5× slower" for time-style metrics.
+String _fasterPhrase(num oldMs, num newMs) {
+  if (oldMs == newMs) return 'tied';
+  if (newMs == 0) return '∞× faster';
+  if (newMs < oldMs) {
+    return '${(oldMs / newMs).toStringAsFixed(1)}× faster';
+  }
+  return '${(newMs / oldMs).toStringAsFixed(1)}× slower';
+}
+
+void _printSingleRun(String scenario, BenchResult r) {
+  // ignore: avoid_print
+  print('\n${_rule(scenario)}');
+  // ignore: avoid_print
+  print('  ${r.label.padRight(4)} '
+      '${r.queryCount.toString().padLeft(7)} stmts | '
+      '${r.rowsReturned.toString().padLeft(6)} rows | '
+      '${r.elapsed.inMilliseconds.toString().padLeft(6)} ms');
+  // ignore: avoid_print
+  print(_rule());
+}
+
+void _printComparison(String scenario, BenchResult old, BenchResult fresh) {
+  final stmtsDelta = fresh.queryCount - old.queryCount;
+  final rowsDelta = fresh.rowsReturned - old.rowsReturned;
+  final msDelta = fresh.elapsed.inMilliseconds - old.elapsed.inMilliseconds;
+
+  final stmtsPct = old.queryCount == 0 ? 0.0 : stmtsDelta / old.queryCount * 100;
+  final rowsPct =
+      old.rowsReturned == 0 ? 0.0 : rowsDelta / old.rowsReturned * 100;
+  final msPct = old.elapsed.inMilliseconds == 0
+      ? 0.0
+      : msDelta / old.elapsed.inMilliseconds * 100;
+
+  String row(String label, int stmts, int rows, int ms) =>
+      '  ${label.padRight(4)} '
+      '${stmts.toString().padLeft(7)} stmts | '
+      '${rows.toString().padLeft(6)} rows | '
+      '${ms.toString().padLeft(6)} ms';
+
+  String deltaRow() => '  Δ    '
+      '${_signed(stmtsDelta).padLeft(7)} (${_pct(stmtsPct).padLeft(6)}) | '
+      '${_signed(rowsDelta).padLeft(6)} (${_pct(rowsPct).padLeft(6)}) | '
+      '${_signed(msDelta).padLeft(6)} (${_pct(msPct).padLeft(6)})';
+
+  final speedup = '${_fewerPhrase(old.queryCount, fresh.queryCount)} stmts, '
+      '${_fewerPhrase(old.rowsReturned, fresh.rowsReturned)} rows, '
+      '${_fasterPhrase(old.elapsed.inMilliseconds, fresh.elapsed.inMilliseconds)}';
+
+  // ignore: avoid_print
+  print('\n${_rule(scenario)}');
+  // ignore: avoid_print
+  print(row(old.label, old.queryCount, old.rowsReturned,
+      old.elapsed.inMilliseconds));
+  // ignore: avoid_print
+  print(row(fresh.label, fresh.queryCount, fresh.rowsReturned,
+      fresh.elapsed.inMilliseconds));
+  // ignore: avoid_print
+  print(deltaRow());
+  // ignore: avoid_print
+  print('  →    $speedup');
+  // ignore: avoid_print
+  print(_rule());
+}
+
+/// Holder for the fresh setup used by tests within a benchmark group.
+class _BenchSetup {
+  late CountingQueryInterceptor counter;
+  late DriftChatDatabase database;
+  late StreamChatPersistenceClient client;
+}
+
+/// Registers a group of benchmark tests against a database seeded with the
+/// given parameters. Each test in the group gets a fresh in-memory DB.
+void _benchmarkGroup({
+  required String name,
+  required int channelCount,
+  required int messagesPerChannel,
+  required void Function(_BenchSetup setup, Filter filter) registerTests,
+  String userId = 'testUserId',
+}) {
+  group(name, () {
+    final filter = Filter.in_('members', [userId]);
+    final setup = _BenchSetup();
+
+    setUp(() async {
+      setup.counter = CountingQueryInterceptor();
+      final executor = NativeDatabase.memory().interceptWith(setup.counter);
+      setup.database = DriftChatDatabase(userId, executor);
+
+      setup.client = StreamChatPersistenceClient();
+      await setup.client.connect(
+        userId,
+        databaseProvider: (_, __) => setup.database,
+      );
+
+      await seedChannels(
+        setup.database,
+        channelCount: channelCount,
+        currentUserId: userId,
+        messagesPerChannel: messagesPerChannel,
+        filter: filter,
+      );
+    });
+
+    tearDown(() async {
+      await setup.client.disconnect(flush: true);
+    });
+
+    registerTests(setup, filter);
+  });
+}
+
+void main() {
+  // ===========================================================================
+  // Group 1 — 100 channels, 25 messages each (baselines + sort variations).
+  // ===========================================================================
+  _benchmarkGroup(
+    name: 'getChannelStates benchmark (100 channels, 25 msgs each)',
+    channelCount: 100,
+    messagesPerChannel: 25,
+    registerTests: (setup, filter) {
+      test('OLD getChannelStatesLegacy — default sort, page 30', () async {
+        const params = PaginationParams(limit: 30);
+        const sort = [
+          SortOption<ChannelState>.desc(ChannelSortKey.lastUpdated),
+        ];
+
+        final result = await runBench(
+          label: 'OLD',
+          body: () => setup.client.getChannelStatesLegacy(
+            filter: filter,
+            channelStateSort: sort,
+            paginationParams: params,
+          ),
+          counter: setup.counter,
+        );
+
+        _printSingleRun('100ch × 25msgs, default sort, page 30 (OLD only)',
+            result);
+
+        expect(result.resultCids.length, lessThanOrEqualTo(30));
+        expect(result.queryCount, greaterThan(0));
+      });
+
+      test('NEW getChannelStates — default sort, page 30', () async {
+        const params = PaginationParams(limit: 30);
+        const sort = [
+          SortOption<ChannelState>.desc(ChannelSortKey.lastUpdated),
+        ];
+
+        final result = await runBench(
+          label: 'NEW',
+          body: () => setup.client.getChannelStates(
+            filter: filter,
+            channelStateSort: sort,
+            paginationParams: params,
+          ),
+          counter: setup.counter,
+        );
+
+        _printSingleRun('100ch × 25msgs, default sort, page 30 (NEW only)',
+            result);
+
+        expect(result.resultCids.length, lessThanOrEqualTo(30));
+        expect(result.queryCount, greaterThan(0));
+      });
+
+      // ----- Sort variations ----------------------------------------------
+
+      test('OLD vs NEW — lastUpdated sort, page 30', () async {
+        await _runComparison(
+          scenario: '100ch × 25msgs, lastUpdated sort, page 30',
+          setup: setup,
+          filter: filter,
+          sort: const [
+            SortOption<ChannelState>.desc(ChannelSortKey.lastUpdated),
+          ],
+          params: const PaginationParams(limit: 30),
+        );
+      });
+
+      test('OLD vs NEW — lastMessageAt sort, page 30', () async {
+        await _runComparison(
+          scenario: '100ch × 25msgs, lastMessageAt sort, page 30',
+          setup: setup,
+          filter: filter,
+          sort: const [
+            SortOption<ChannelState>.desc(ChannelSortKey.lastMessageAt),
+          ],
+          params: const PaginationParams(limit: 30),
+        );
+      });
+
+      test('OLD vs NEW — memberCount sort, page 30', () async {
+        await _runComparison(
+          scenario: '100ch × 25msgs, memberCount sort, page 30',
+          setup: setup,
+          filter: filter,
+          sort: const [
+            SortOption<ChannelState>.desc(ChannelSortKey.memberCount),
+          ],
+          params: const PaginationParams(limit: 30),
+        );
+      });
+
+      test('OLD vs NEW — pinnedAt sort, page 30', () async {
+        await _runComparison(
+          scenario: '100ch × 25msgs, pinnedAt sort, page 30',
+          setup: setup,
+          filter: filter,
+          sort: const [
+            SortOption<ChannelState>.desc(ChannelSortKey.pinnedAt),
+          ],
+          params: const PaginationParams(limit: 30),
+        );
+      });
+
+      test('OLD vs NEW — multi-field sort (pinnedAt → lastMessageAt), page 30',
+          () async {
+        await _runComparison(
+          scenario:
+              '100ch × 25msgs, multi-field sort (pinnedAt → lastMessageAt), page 30',
+          setup: setup,
+          filter: filter,
+          sort: const [
+            SortOption<ChannelState>.desc(ChannelSortKey.pinnedAt),
+            SortOption<ChannelState>.desc(ChannelSortKey.lastMessageAt),
+          ],
+          params: const PaginationParams(limit: 30),
+        );
+      });
+
+      test('OLD vs NEW — custom extraData sort (priority), page 30', () async {
+        await _runComparison(
+          scenario: '100ch × 25msgs, extraData sort (priority), page 30',
+          setup: setup,
+          filter: filter,
+          sort: const [SortOption<ChannelState>.desc('priority')],
+          params: const PaginationParams(limit: 30),
+        );
+      });
+
+      // ----- Page size variations -----------------------------------------
+
+      test('OLD vs NEW — default sort, page 10', () async {
+        await _runComparison(
+          scenario: '100ch × 25msgs, default sort, page 10',
+          setup: setup,
+          filter: filter,
+          sort: const [
+            SortOption<ChannelState>.desc(ChannelSortKey.lastUpdated),
+          ],
+          params: const PaginationParams(limit: 10),
+        );
+      });
+
+      test('OLD vs NEW — default sort, page 100 (whole cache, no speedup)',
+          () async {
+        await _runComparison(
+          scenario:
+              '100ch × 25msgs, default sort, page 100 (whole cache — no speedup expected)',
+          setup: setup,
+          filter: filter,
+          sort: const [
+            SortOption<ChannelState>.desc(ChannelSortKey.lastUpdated),
+          ],
+          params: const PaginationParams(limit: 100),
+          assertSpeedup: false,
+        );
+      });
+
+      // ----- Offset variations --------------------------------------------
+
+      test('OLD vs NEW — default sort, page 2 (offset 30, limit 30)',
+          () async {
+        await _runComparison(
+          scenario:
+              '100ch × 25msgs, default sort, page 2 (offset 30, limit 30)',
+          setup: setup,
+          filter: filter,
+          sort: const [
+            SortOption<ChannelState>.desc(ChannelSortKey.lastUpdated),
+          ],
+          params: const PaginationParams(limit: 30, offset: 30),
+        );
+      });
+    },
+  );
+
+  // ===========================================================================
+  // Group 2 — 50 channels (small cache).
+  // ===========================================================================
+  _benchmarkGroup(
+    name: 'getChannelStates benchmark (50 channels, 25 msgs each)',
+    channelCount: 50,
+    messagesPerChannel: 25,
+    registerTests: (setup, filter) {
+      test('OLD vs NEW — default sort, page 30', () async {
+        await _runComparison(
+          scenario: '50ch × 25msgs, default sort, page 30',
+          setup: setup,
+          filter: filter,
+          sort: const [
+            SortOption<ChannelState>.desc(ChannelSortKey.lastUpdated),
+          ],
+          params: const PaginationParams(limit: 30),
+        );
+      });
+    },
+  );
+
+  // ===========================================================================
+  // Group 3 — 250 channels, 100 messages each (heavy / near sync-cap).
+  // ===========================================================================
+  _benchmarkGroup(
+    name: 'getChannelStates benchmark (250 channels, 100 msgs each)',
+    channelCount: 250,
+    messagesPerChannel: 100,
+    registerTests: (setup, filter) {
+      test('OLD vs NEW — default sort, page 30', () async {
+        await _runComparison(
+          scenario: '250ch × 100msgs, default sort, page 30',
+          setup: setup,
+          filter: filter,
+          sort: const [
+            SortOption<ChannelState>.desc(ChannelSortKey.lastUpdated),
+          ],
+          params: const PaginationParams(limit: 30),
+        );
+      });
+    },
+  );
+}
+
+/// Runs both methods against the same DB, asserts behavioral equivalence
+/// (same channels returned in the same sort order), and (by default) asserts
+/// NEW issues fewer SQL statements than OLD.
+///
+/// Set [assertSpeedup] to false for scenarios where the page covers the
+/// entire cache — NEW must hydrate every channel in that case, so query
+/// counts naturally match.
+Future<void> _runComparison({
+  required String scenario,
+  required _BenchSetup setup,
+  required Filter filter,
+  required SortOrder<ChannelState> sort,
+  required PaginationParams params,
+  bool assertSpeedup = true,
+}) async {
+  final oldResult = await runBench(
+    label: 'OLD',
+    body: () => setup.client.getChannelStatesLegacy(
+      filter: filter,
+      channelStateSort: sort,
+      paginationParams: params,
+    ),
+    counter: setup.counter,
+  );
+
+  final newResult = await runBench(
+    label: 'NEW',
+    body: () => setup.client.getChannelStates(
+      filter: filter,
+      channelStateSort: sort,
+      paginationParams: params,
+    ),
+    counter: setup.counter,
+  );
+
+  _printComparison(scenario, oldResult, newResult);
+
+  // Same channels — assert length first for a clearer failure message, then
+  // assert same cids in the same order.
+  expect(
+    newResult.resultCids.length,
+    equals(oldResult.resultCids.length),
+    reason: 'NEW must return the same number of channels as OLD',
+  );
+  expect(
+    newResult.resultCids,
+    orderedEquals(oldResult.resultCids),
+    reason: 'NEW must return the same channels in the same sort order as OLD',
+  );
+
+  if (assertSpeedup) {
+    // NEW should issue strictly fewer queries than OLD.
+    expect(
+      newResult.queryCount,
+      lessThan(oldResult.queryCount),
+      reason: 'NEW must execute fewer SQL statements than OLD',
+    );
+  } else {
+    // For whole-cache pages, NEW must not be worse.
+    expect(
+      newResult.queryCount,
+      lessThanOrEqualTo(oldResult.queryCount),
+      reason: 'NEW must not execute more SQL statements than OLD',
+    );
+  }
+
+  // Wall time is printed but not asserted (CI noise).
+}

Summary by CodeRabbit

  • Performance Improvements

    • Faster channel list loading: reduced database reads, paginated hydration, and batched membership loading for large channel sets
  • New Features

    • Added a DAO method to fetch memberships for multiple channels in a single call
  • Documentation

    • Replaced Moor with Drift as the disk cache implementation and updated Flutter Web offline-storage guidance
    • Added an "Upcoming" changelog entry highlighting the performance improvement
  • Tests

    • Added/expanded tests covering batched membership hydration, deterministic sorting + pagination, and new DAO behavior

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

📝 Walkthrough

Walkthrough

Migrate persistence isolate wiring from Moor to Drift, update entity docs to reference Drift, add a batched member DAO, refactor getChannelStates to envelope, sort, paginate, and hydrate only the requested page with conditional membership preloading, and update tests and changelog.

Changes

Drift migration and channel states optimization

Layer / File(s) Summary
Drift database isolate migration
packages/stream_chat_persistence/lib/src/db/shared/native_db.dart, packages/stream_chat_persistence/lib/src/db/drift_chat_database.dart
Background DB creation now uses _createDriftIsolate(...). The background bootstrap constructs a DriftIsolate from DatabaseConnection(executor) and _IsolateStartRequest now carries a drift-send callback. File comment updated to reference Drift.
Entity schema documentation updates to Drift
packages/stream_chat_persistence/lib/src/entity/*.dart
Documentation comments for table/entity files updated to reference DriftChatDatabase instead of MoorChatDatabase.
Member batch query support
packages/stream_chat_persistence/lib/src/dao/member_dao.dart
Added getMembershipsForChannels(List<String> cids, String userId) to batch-query members joined with users and return a Map<String, Member> keyed by channelCid.
Channel states lazy pagination and membership optimization
packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart, packages/stream_chat_persistence/test/stream_chat_persistence_client_test.dart, packages/stream_chat_persistence/CHANGELOG.md
getChannelStates now queries channel rows and builds envelopes, conditionally batch-attaches current-user memberships when sorting by pinnedAt, sorts and paginates envelopes, then hydrates only the paged channel cids via getChannelStateByCid. Added _sortRequiresMembership and _attachMemberships. Tests and changelog updated accordingly.
README and test updates
packages/stream_chat_persistence/README.md, packages/stream_chat_persistence/test/src/db/drift_chat_database_test.dart, packages/stream_chat_persistence/test/src/dao/member_dao_test.dart
README disk-cache and Flutter Web notes updated to reference Drift; test descriptions changed to DriftChatDatabase; tests register a List<String> fallback value, add getMembershipsForChannels DAO test, and reorganize getChannelStates tests.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

  • Brazol

Poem

A rabbit hops where Drift now streams,
I batch the memberships in my dreams,
Envelopes sorted, pages light,
Hydrate the few, keep reads tight,
Hoppity-hop — performance gleams! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main performance optimization: reducing DB reads in ChatPersistenceClient.getChannelStates() by lazy-loading and pagination.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/optimize-query-channels-offline

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 19, 2026

⚠️ Database Entity Files Modified

The following database entity files have been modified in this PR:

packages/stream_chat_persistence/lib/src/entity/channel_queries.dart
packages/stream_chat_persistence/lib/src/entity/channels.dart
packages/stream_chat_persistence/lib/src/entity/connection_events.dart
packages/stream_chat_persistence/lib/src/entity/draft_messages.dart
packages/stream_chat_persistence/lib/src/entity/members.dart
packages/stream_chat_persistence/lib/src/entity/messages.dart
packages/stream_chat_persistence/lib/src/entity/pinned_message_reactions.dart
packages/stream_chat_persistence/lib/src/entity/pinned_messages.dart
packages/stream_chat_persistence/lib/src/entity/poll_votes.dart
packages/stream_chat_persistence/lib/src/entity/polls.dart
packages/stream_chat_persistence/lib/src/entity/reactions.dart
packages/stream_chat_persistence/lib/src/entity/reads.dart
packages/stream_chat_persistence/lib/src/entity/users.dart

📝 Remember to:

  1. Update database version in db/drift_chat_database.dart.
  2. Update entity schema tests if necessary.

Note: This comment is automatically generated by the CI workflow.

@VelikovPetar VelikovPetar changed the title perf(llc): Reduce the number of DB reads in ChatPersistenceClient.getClientStates() perf(llc): Reduce the number of DB reads in ChatPersistenceClient.getChannelStates() May 19, 2026
@VelikovPetar VelikovPetar requested a review from a team May 19, 2026 10:42
@VelikovPetar VelikovPetar marked this pull request as ready for review May 21, 2026 13:59
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (5)
packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart (1)

294-299: 💤 Low value

Pagination slicing LGTM, but s.channel!.cid is doubly-asserted — consider tightening.

s.channel!.cid assumes both channel and its cid are non-null. The first is safe because envelopes are constructed locally as ChannelState(channel: m). The second relies on every row from channelQueryDao.getChannels having a non-null cid — which is true today, but a future schema/DAO change that filters or projects channels differently would surface here as a runtime Null check operator crash inside Future.wait. A whereType<String> on the cid stream (like you already do in _attachMemberships) would be consistent and defensive.

♻️ Minor consistency tweak
-    final pagedCids =
-        envelopes.skip(offset).take(limit).map((s) => s.channel!.cid).toList();
+    final pagedCids = envelopes
+        .skip(offset)
+        .take(limit)
+        .map((s) => s.channel?.cid)
+        .whereType<String>()
+        .toList(growable: false);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart`
around lines 294 - 299, The code is using a double null-assertion s.channel!.cid
when building pagedCids; replace this with a defensive projection that skips
null cids (e.g., map to s.channel?.cid and filter non-null) so we don't crash if
cid becomes null in future schema/DAO changes—apply the same pattern you used in
_attachMemberships (whereType<String>) to the pagedCids construction that reads
from envelopes and paginationParams to ensure only valid String cids are
collected.
packages/stream_chat_persistence/lib/src/dao/member_dao.dart (2)

50-56: 💤 Low value

Avoid reading the same row table twice.

row.readTable(members) is invoked twice for each row in the map literal. Cache it once for clarity and a tiny CPU win.

♻️ Proposed refactor
-    return {
-      for (final row in rows)
-        row.readTable(members).channelCid: row.readTable(members).toMember(
-              user: row.readTableOrNull(users)?.toUser(),
-            ),
-    };
+    return {
+      for (final row in rows)
+        if (row.readTable(members) case final memberEntity)
+          memberEntity.channelCid: memberEntity.toMember(
+            user: row.readTableOrNull(users)?.toUser(),
+          ),
+    };

Or use a plain for loop with a local final memberEntity = row.readTable(members);.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/stream_chat_persistence/lib/src/dao/member_dao.dart` around lines 50
- 56, The map literal is calling row.readTable(members) twice per row (in the
key and value); cache the result in a local variable (e.g., memberEntity) or
switch to an explicit for loop so you call row.readTable(members) once, then use
memberEntity.channelCid for the key and memberEntity.toMember(user:
row.readTableOrNull(users)?.toUser()) for the value; update references to
members, toMember, and readTableOrNull(users) accordingly.

42-48: 💤 Low value

Consider chunking cids in getMembershipsForChannels when preloading memberships
MemberDao.getMembershipsForChannels uses a single WHERE members.channelCid IN (cids) query; with large cids, this can hit SQLite bound-variable limits. The current offline path usually keeps cids small (default PaginationParams.limit = 10), but getChannelStates accepts nullable paginationParams and preloads memberships for all cached channels when it’s omitted—so chunking/batching would prevent failures in large/unpaginated reuse cases. (packages/stream_chat_persistence/lib/src/dao/member_dao.dart:42-48)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/stream_chat_persistence/lib/src/dao/member_dao.dart` around lines 42
- 48, getMembershipsForChannels currently builds a single query using
members.channelCid.isIn(cids) which can exceed SQLite bound-variable limits for
large cids; modify MemberDao.getMembershipsForChannels to chunk the cids list
into safe-sized batches (e.g., 500–900 ids per batch), run the
select(join(leftOuterJoin(users, members.userId.equalsExp(users.id)))) with the
existing where clause (members.channelCid.isIn(batch) &
members.userId.equals(userId)) for each batch, collect and concatenate all rows
across batches, and return the combined result so large/unpaginated preloads
won’t hit the bind-variable limit.
packages/stream_chat_persistence/test/stream_chat_persistence_client_test.dart (1)

332-360: ⚡ Quick win

Mock/verification updates are correct, but the new branches are uncovered.

The updated messagePagination: const PaginationParams(limit: 25) matchers match the implementation. However, the refactor introduced two notable new code paths that this test does not exercise:

  1. _sortRequiresMembership + _attachMemberships (triggered when channelStateSort contains pinnedAt) → memberDao.getMembershipsForChannels should be called exactly once with the page-relevant cids and the connected userId.
  2. paginationParams slicing (offset/limit/empty page) — previously hydration was unconditional, now an out-of-range offset should produce zero getChannelStateByCid calls.

Consider adding a couple of focused tests so future regressions in either branch are caught.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/stream_chat_persistence/test/stream_chat_persistence_client_test.dart`
around lines 332 - 360, The test misses two new branches introduced by the
refactor: add focused tests for getChannelStates that (1) exercise the
_sortRequiresMembership + _attachMemberships path by making channelStateSort
include "pinnedAt" and then verify memberDao.getMembershipsForChannels is called
exactly once with only the page-relevant cids and the connected userId, and (2)
exercise pagination slicing by calling getChannelStates with an out-of-range
offset so that paginationParams yields an empty page and verify
getChannelStateByCid is not called (zero calls); update the mocked responses and
verifications around getChannelStateByCid, memberDao.getMembershipsForChannels,
and paginationParams to assert these behaviors.
packages/stream_chat_persistence/CHANGELOG.md (1)

1-5: ⚡ Quick win

Document the offline-message-window behavior change.

The new getChannelStates implementation now loads at most 25 messages per channel from persistence (previously it returned all cached messages per channel). That's not strictly a perf-only change — consumers who relied on the persistence layer returning the full cached history will observe fewer messages after this upgrade. Worth adding a 🔄 Changed bullet noting the cap so users aren't surprised.

📝 Suggested addition
 # Upcoming
 
 🚀 Performance
 
 - Reduce the number of DB reads in the `ChatPersistenceClient.getChannelStates` method.
+
+🔄 Changed
+
+- `ChatPersistenceClient.getChannelStates` now hydrates at most 25 messages per channel
+  (matching the API default) instead of returning every cached message. Channels not in
+  the requested page are no longer hydrated at all.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/stream_chat_persistence/CHANGELOG.md` around lines 1 - 5, Update the
CHANGELOG.md to document the behavioral change in
ChatPersistenceClient.getChannelStates: add a "🔄 Changed" bullet under Upcoming
or Performance explaining that getChannelStates now loads at most 25 messages
per channel from persistence (previously returned full cached history), mention
the offline-message-window cap (25 messages) and note that consumers relying on
full history may see fewer messages after upgrading. Keep the note concise and
add a short migration/impact hint for consumers to adjust if they relied on full
cached history.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart`:
- Around line 301-308: getChannelStates currently hard-caps per-channel
hydration to 25 messages by calling getChannelStateByCid(cid, messagePagination:
const PaginationParams(limit: 25)); make this behavior opt-in by adding a
messagePagination (or at least limit) parameter to the public getChannelStates
signature and pass that through to getChannelStateByCid instead of the
hard-coded PaginationParams; also update
packages/stream_chat_persistence/CHANGELOG.md to call out the new default
per-channel truncation and document the new option so callers can opt in to
larger hydration windows.

---

Nitpick comments:
In `@packages/stream_chat_persistence/CHANGELOG.md`:
- Around line 1-5: Update the CHANGELOG.md to document the behavioral change in
ChatPersistenceClient.getChannelStates: add a "🔄 Changed" bullet under Upcoming
or Performance explaining that getChannelStates now loads at most 25 messages
per channel from persistence (previously returned full cached history), mention
the offline-message-window cap (25 messages) and note that consumers relying on
full history may see fewer messages after upgrading. Keep the note concise and
add a short migration/impact hint for consumers to adjust if they relied on full
cached history.

In `@packages/stream_chat_persistence/lib/src/dao/member_dao.dart`:
- Around line 50-56: The map literal is calling row.readTable(members) twice per
row (in the key and value); cache the result in a local variable (e.g.,
memberEntity) or switch to an explicit for loop so you call
row.readTable(members) once, then use memberEntity.channelCid for the key and
memberEntity.toMember(user: row.readTableOrNull(users)?.toUser()) for the value;
update references to members, toMember, and readTableOrNull(users) accordingly.
- Around line 42-48: getMembershipsForChannels currently builds a single query
using members.channelCid.isIn(cids) which can exceed SQLite bound-variable
limits for large cids; modify MemberDao.getMembershipsForChannels to chunk the
cids list into safe-sized batches (e.g., 500–900 ids per batch), run the
select(join(leftOuterJoin(users, members.userId.equalsExp(users.id)))) with the
existing where clause (members.channelCid.isIn(batch) &
members.userId.equals(userId)) for each batch, collect and concatenate all rows
across batches, and return the combined result so large/unpaginated preloads
won’t hit the bind-variable limit.

In
`@packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart`:
- Around line 294-299: The code is using a double null-assertion s.channel!.cid
when building pagedCids; replace this with a defensive projection that skips
null cids (e.g., map to s.channel?.cid and filter non-null) so we don't crash if
cid becomes null in future schema/DAO changes—apply the same pattern you used in
_attachMemberships (whereType<String>) to the pagedCids construction that reads
from envelopes and paginationParams to ensure only valid String cids are
collected.

In
`@packages/stream_chat_persistence/test/stream_chat_persistence_client_test.dart`:
- Around line 332-360: The test misses two new branches introduced by the
refactor: add focused tests for getChannelStates that (1) exercise the
_sortRequiresMembership + _attachMemberships path by making channelStateSort
include "pinnedAt" and then verify memberDao.getMembershipsForChannels is called
exactly once with only the page-relevant cids and the connected userId, and (2)
exercise pagination slicing by calling getChannelStates with an out-of-range
offset so that paginationParams yields an empty page and verify
getChannelStateByCid is not called (zero calls); update the mocked responses and
verifications around getChannelStateByCid, memberDao.getMembershipsForChannels,
and paginationParams to assert these behaviors.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7825573d-f714-4ea0-9fde-b95632444682

📥 Commits

Reviewing files that changed from the base of the PR and between 4103da3 and dd495eb.

📒 Files selected for processing (21)
  • packages/stream_chat_persistence/CHANGELOG.md
  • packages/stream_chat_persistence/README.md
  • packages/stream_chat_persistence/lib/src/dao/member_dao.dart
  • packages/stream_chat_persistence/lib/src/db/drift_chat_database.dart
  • packages/stream_chat_persistence/lib/src/db/shared/native_db.dart
  • packages/stream_chat_persistence/lib/src/entity/channel_queries.dart
  • packages/stream_chat_persistence/lib/src/entity/channels.dart
  • packages/stream_chat_persistence/lib/src/entity/connection_events.dart
  • packages/stream_chat_persistence/lib/src/entity/draft_messages.dart
  • packages/stream_chat_persistence/lib/src/entity/members.dart
  • packages/stream_chat_persistence/lib/src/entity/messages.dart
  • packages/stream_chat_persistence/lib/src/entity/pinned_message_reactions.dart
  • packages/stream_chat_persistence/lib/src/entity/pinned_messages.dart
  • packages/stream_chat_persistence/lib/src/entity/poll_votes.dart
  • packages/stream_chat_persistence/lib/src/entity/polls.dart
  • packages/stream_chat_persistence/lib/src/entity/reactions.dart
  • packages/stream_chat_persistence/lib/src/entity/reads.dart
  • packages/stream_chat_persistence/lib/src/entity/users.dart
  • packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart
  • packages/stream_chat_persistence/test/src/db/drift_chat_database_test.dart
  • packages/stream_chat_persistence/test/stream_chat_persistence_client_test.dart

Comment thread packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.33%. Comparing base (4103da3) to head (5da7f07).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2664      +/-   ##
==========================================
+ Coverage   65.27%   65.33%   +0.05%     
==========================================
  Files         423      423              
  Lines       26622    26646      +24     
==========================================
+ Hits        17377    17408      +31     
+ Misses       9245     9238       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@packages/stream_chat_persistence/test/stream_chat_persistence_client_test.dart`:
- Around line 382-387: The assertion uses the hardcoded fixture 'test-user-id'
instead of the connected client user variable; update the expectation for the
second captured argument from the literal 'test-user-id' to the testUserId
variable so verify(() =>
mockDatabase.memberDao.getMembershipsForChannels(captureAny(),
captureAny())).captured is asserted as first == cids and last == testUserId.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 59ac3d35-b6cb-4611-bcaa-90da821418fe

📥 Commits

Reviewing files that changed from the base of the PR and between dd495eb and f11700d.

📒 Files selected for processing (1)
  • packages/stream_chat_persistence/test/stream_chat_persistence_client_test.dart

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart`:
- Around line 301-303: The inline comment is stale: it claims messagePagination
is passed but getChannelStateByCid is invoked without it; either remove the
misleading text or actually thread a PaginationParams through getChannelStates
and into getChannelStateByCid so callers can opt into a limit. To fix, choose
one of two options: (A) update the comment above the
Future.wait(pagedCids.map(getChannelStateByCid)) line to remove the "Pass
messagePagination" clause so it accurately reflects current behavior, or (B) add
a messagePagination parameter to getChannelStates and modify
getChannelStateByCid (and its callers) to accept and use that PaginationParams
value when hydrating channel state; reference the functions getChannelStates and
getChannelStateByCid and the messagePagination/PaginationParams symbol when
making the change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cc90da6e-62d0-444a-ad9d-0d0699e17688

📥 Commits

Reviewing files that changed from the base of the PR and between f11700d and 14ea472.

📒 Files selected for processing (1)
  • packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart

Comment thread packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
packages/stream_chat_persistence/test/stream_chat_persistence_client_test.dart (1)

312-313: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align membership assertion with the connected client user id.

At Line 387, the assertion is tied to currentUserId (Line 312), which is a fixture value and can diverge from the connected user (userId). Use the connected id to keep this test validating the real contract of getChannelStates.

Proposed fix
-        const currentUserId = 'test-user-id';
+        const currentUserId = userId;

Also applies to: 320-329, 387-387

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/stream_chat_persistence/test/stream_chat_persistence_client_test.dart`
around lines 312 - 313, The test uses the fixture constant currentUserId when
building the membership Filter and asserting channel membership, but the test
connects a client as userId — replace references to currentUserId with the
actual connected user identifier (use the test's userId variable or the
connected client's id, e.g., client.state.currentUser!.id) so the
Filter.in_('members', [ ... ]) and all assertions that check membership
(including those in the same test's channel creation/verification blocks) assert
against the real connected user id instead of the fixture constant; update
occurrences around the getChannelStates test and the related assertions to use
userId/client.state.currentUser!.id accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In
`@packages/stream_chat_persistence/test/stream_chat_persistence_client_test.dart`:
- Around line 312-313: The test uses the fixture constant currentUserId when
building the membership Filter and asserting channel membership, but the test
connects a client as userId — replace references to currentUserId with the
actual connected user identifier (use the test's userId variable or the
connected client's id, e.g., client.state.currentUser!.id) so the
Filter.in_('members', [ ... ]) and all assertions that check membership
(including those in the same test's channel creation/verification blocks) assert
against the real connected user id instead of the fixture constant; update
occurrences around the getChannelStates test and the related assertions to use
userId/client.state.currentUser!.id accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bdb7a334-4fe9-4207-82c2-decd2b9be03e

📥 Commits

Reviewing files that changed from the base of the PR and between 14ea472 and 5da7f07.

📒 Files selected for processing (3)
  • packages/stream_chat_persistence/lib/src/stream_chat_persistence_client.dart
  • packages/stream_chat_persistence/test/src/dao/member_dao_test.dart
  • packages/stream_chat_persistence/test/stream_chat_persistence_client_test.dart

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.

1 participant