From cdc2b129c006755bd923e75462c610df2815b383 Mon Sep 17 00:00:00 2001 From: Tom Brow Date: Tue, 20 Oct 2026 13:13:15 -0700 Subject: [PATCH] fix(mobile): resolve DM display labels once per sort (#8488) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Home's DM sort resolves participant identity names again for every comparator call. This work appeared in a captured 82.163 ms UI frame after foregrounding the full app (Debug/JIT iOS simulator, shared host). Resolve each lowercase display label once per sort, retaining the same label ordering and channel-ID tie-breaker. Sort keys are invocation-local, so later viewer, profile, owner, participant, and community changes cannot reuse stale labels. No matching open mobile sorting PR or issue found. ### Validation - Added synthetic regression coverage for bounded contextual resolution and ordering parity across identity/name/channel changes, ties, unnamed DMs, and empty/singleton lists. The 80-DM fixture performs 80 contextual resolutions instead of the original comparator’s 982; restoring that comparator makes the regression fail. - 133 focused tests passed, including existing Home scrolling/reconnect widget tests and identity-name tests. - Mobile static analysis passed; 3,697 tests passed with 16 existing skips, plus four no-gateway configuration tests. - No new full-app or hardware capture. The existing trace establishes repeated synchronous work, not its release-build magnitude or the cause of every scrolling stall. Signed-off-by: Tom Brow --- .../features/channels/dm_channel_labels.dart | 33 ++-- .../channels/dm_channel_labels_test.dart | 170 ++++++++++++++++++ 2 files changed, 187 insertions(+), 16 deletions(-) create mode 100644 mobile/test/features/channels/dm_channel_labels_test.dart diff --git a/mobile/lib/features/channels/dm_channel_labels.dart b/mobile/lib/features/channels/dm_channel_labels.dart index 5e1a35e8ed..f5d4b4b3ef 100644 --- a/mobile/lib/features/channels/dm_channel_labels.dart +++ b/mobile/lib/features/channels/dm_channel_labels.dart @@ -128,28 +128,29 @@ String dmAvatarInitial(Channel channel, {String? currentPubkey}) { return label.isNotEmpty ? label[0].toUpperCase() : '?'; } +/// Sorts by the visible label, resolving each label only once per invocation. List sortDmChannelsByDisplayLabel( Iterable channels, { String? currentPubkey, IdentityNameSources? names, }) { - final sorted = channels.toList(); + // Keep sort keys local: the next call must reflect current viewer, channel + // participants, and community profile/owner names. + final sorted = [ + for (final channel in channels) + ( + channel: channel, + label: resolveDmChannelDisplayLabel( + channel, + currentPubkey: currentPubkey, + names: names, + ).toLowerCase(), + ), + ]; sorted.sort((left, right) { - final leftLabel = resolveDmChannelDisplayLabel( - left, - currentPubkey: currentPubkey, - names: names, - ); - final rightLabel = resolveDmChannelDisplayLabel( - right, - currentPubkey: currentPubkey, - names: names, - ); - final labelCompare = leftLabel.toLowerCase().compareTo( - rightLabel.toLowerCase(), - ); + final labelCompare = left.label.compareTo(right.label); if (labelCompare != 0) return labelCompare; - return left.id.compareTo(right.id); + return left.channel.id.compareTo(right.channel.id); }); - return sorted; + return [for (final entry in sorted) entry.channel]; } diff --git a/mobile/test/features/channels/dm_channel_labels_test.dart b/mobile/test/features/channels/dm_channel_labels_test.dart new file mode 100644 index 0000000000..8132a53c2d --- /dev/null +++ b/mobile/test/features/channels/dm_channel_labels_test.dart @@ -0,0 +1,170 @@ +import 'package:buzz/features/channels/channel.dart'; +import 'package:buzz/features/channels/dm_channel_labels.dart'; +import 'package:buzz/shared/identity_names/identity_names.dart'; +import 'package:buzz/shared/profile/user_profile.dart'; +import 'package:flutter_test/flutter_test.dart'; + +String _key(int number) => number.toRadixString(16).padLeft(64, '0'); + +Channel _dm( + String id, { + String name = 'DM', + List keys = const [], + List labels = const [], +}) => Channel( + id: id, + name: name, + channelType: 'dm', + visibility: 'private', + description: '', + createdBy: _key(1), + createdAt: DateTime(2026), + memberCount: keys.length, + participantPubkeys: keys, + participants: labels, + isMember: true, +); + +// Count real contextual resolutions, without replacing the naming policy. +class _CountingSources extends IdentityNameSources { + final List> scopes = []; + + _CountingSources({required super.profiles}); + + @override + IdentityNames scope( + Iterable candidates, { + Set agentPubkeys = const {}, + Map fallbackNames = const {}, + Map ownerPubkeys = const {}, + }) { + scopes.add(candidates.toList()); + return super.scope( + candidates, + agentPubkeys: agentPubkeys, + fallbackNames: fallbackNames, + ownerPubkeys: ownerPubkeys, + ); + } +} + +void main() { + test('sorting resolves each DM context once, independent of comparisons', () { + final channels = [ + for (var index = 0; index < 80; index++) + _dm('dm-$index', keys: [_key(1), _key(index + 2)]), + ]; + final names = _CountingSources( + profiles: { + for (var index = 0; index < channels.length; index++) + _key(index + 2): UserProfile( + pubkey: _key(index + 2), + displayName: 'Person ${(79 - index).toString().padLeft(2, '0')}', + ), + }, + ); + + final sorted = sortDmChannelsByDisplayLabel( + channels, + currentPubkey: _key(1), + names: names, + ); + + expect(sorted, channels.reversed.toList()); + expect(names.scopes, hasLength(channels.length)); + expect( + names.scopes, + unorderedEquals(channels.map((channel) => channel.participantPubkeys)), + ); + expect(channels.first.id, 'dm-0', reason: 'Do not mutate the source list'); + }); + + test( + 'each sort uses current labels, identity, participants and community', + () { + final me = _key(1); + final alice = _key(2); + final agent = _key(3); + final otherAgent = _key(4); + final owner = _key(5); + final channels = [ + _dm('direct', keys: [me, alice], labels: ['Zoe', 'Alice']), + _dm( + 'group', + name: 'Group DM (4)', + keys: [me, alice, agent, otherAgent], + labels: ['Zoe', 'Alice', 'Scout', 'Scout'], + ), + _dm('self', keys: [me], labels: ['Zoe']), + _dm('empty'), + _dm('legacy', keys: ['legacy-key'], labels: ['Bob']), + _dm('tie-b', name: 'ALICE'), + _dm('tie-a', name: 'alice'), + ]; + final updatedNames = IdentityNameSources( + viewer: me, + profiles: { + me: UserProfile(pubkey: me, displayName: 'Zoe'), + alice: UserProfile(pubkey: alice, displayName: 'Yvonne'), + agent: UserProfile( + pubkey: agent, + displayName: 'Scout', + ownerPubkey: me, + ), + otherAgent: UserProfile( + pubkey: otherAgent, + displayName: 'Scout', + ownerPubkey: owner, + ), + owner: UserProfile(pubkey: owner, displayName: 'Aaron'), + }, + ); + final cases = [ + (channels: channels, viewer: me, names: null), + (channels: channels, viewer: me, names: updatedNames), + (channels: channels, viewer: alice, names: updatedNames), + ( + channels: [ + channels.first.copyWith(name: 'Renamed'), + _dm('group', keys: [agent, owner], labels: ['Scout', 'Aaron']), + ...channels.skip(2), + ], + viewer: me, + names: updatedNames, + ), + (channels: channels, viewer: me, names: const IdentityNameSources()), + (channels: [], viewer: me, names: updatedNames), + (channels: [channels.first], viewer: me, names: updatedNames), + ]; + + for (final scenario in cases) { + // Independent reference: the previous comparator's ordering contract. + final expected = scenario.channels.toList() + ..sort((left, right) { + String label(Channel channel) => resolveDmChannelDisplayLabel( + channel, + currentPubkey: scenario.viewer, + names: scenario.names, + ).toLowerCase(); + final order = label(left).compareTo(label(right)); + return order != 0 ? order : left.id.compareTo(right.id); + }); + expect( + sortDmChannelsByDisplayLabel( + scenario.channels, + currentPubkey: scenario.viewer, + names: scenario.names, + ), + expected, + ); + } + expect( + sortDmChannelsByDisplayLabel( + channels, + currentPubkey: me, + ).map((channel) => channel.id), + ['direct', 'tie-a', 'tie-b', 'group', 'legacy', 'empty', 'self'], + ); + }, + ); +}