From 446f9b14f6a0bfafad9ea575da2da1d89c44b0e4 Mon Sep 17 00:00:00 2001 From: Logan Johnson Date: Wed, 9 Sep 2026 13:12:56 -0400 Subject: [PATCH] fix(mobile): stop inferring historical mentions from recipient tags Signed-off-by: Logan Johnson --- .../lib/shared/mentions/mention_bindings.dart | 18 +-- .../message_content_exact_mentions_test.dart | 125 ++++++++++++------ .../mentions/mention_bindings_test.dart | 80 ++++++----- 3 files changed, 138 insertions(+), 85 deletions(-) diff --git a/mobile/lib/shared/mentions/mention_bindings.dart b/mobile/lib/shared/mentions/mention_bindings.dart index 0a95ece9f4..2d29afd4ca 100644 --- a/mobile/lib/shared/mentions/mention_bindings.dart +++ b/mobile/lib/shared/mentions/mention_bindings.dart @@ -83,13 +83,12 @@ Map> renderedMentionBindings( } // Untagged qualified literals are recognition blockers, not bindings. bindings.putIfAbsent(label, () => {}); - if (!keys.contains(key)) continue; qualified.add(( label: label, base: match.group(1)!.toLowerCase(), key: key, )); - add(label, key); + if (keys.contains(key)) add(label, key); bindings.putIfAbsent(match.group(1)!.toLowerCase(), () => {}); } final winning = mentionOccurrences( @@ -98,17 +97,12 @@ Map> renderedMentionBindings( ).map((range) => range.label).toSet(); final claimed = qualified.where((q) => winning.contains(q.label)).toList(); final bases = claimed.map((q) => q.base).where(winning.contains).toSet(); - final remaining = keys.difference(claimed.map((q) => q.key).toSet()); - // Ordinary sibling mentions claim their own identities before recovering a - // historical namesake. Current aliases must not retarget that namesake. - for (final label in winning.difference(bases)) { - final candidates = bindings[label]!; - if (candidates.length == 1) remaining.removeAll(candidates); - } for (final base in bases) { - // Multiple historical labels or leftover tagged keys cannot be paired - // deterministically. Leave them as non-clickable references, not guesses. - bindings[base] = bases.length == 1 ? remaining.toSet() : {}; + // Recipient tags do not encode which literal selected a key. In DMs they + // also include non-mentioned recipients, and missing profiles can leave + // an unrelated singleton. Even current aliases cannot establish the + // historical plain target. Only the qualified literal supplies its key. + bindings[base] = {}; } return bindings; } diff --git a/mobile/test/features/channels/message_content_exact_mentions_test.dart b/mobile/test/features/channels/message_content_exact_mentions_test.dart index 32f1bac1f9..ecf49c3399 100644 --- a/mobile/test/features/channels/message_content_exact_mentions_test.dart +++ b/mobile/test/features/channels/message_content_exact_mentions_test.dart @@ -1,16 +1,68 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:buzz/features/channels/message_content.dart'; +import 'package:buzz/features/channels/channel.dart'; +import 'package:buzz/features/channels/message_mention_pubkeys.dart'; +import 'package:buzz/shared/mentions/mention_tags.dart'; import '../../helpers/widget_helpers.dart'; void main() { + testWidgets('DM recipients cannot supply a missing historical identity', ( + tester, + ) async { + final first = 'a' * 64, second = 'b' * 64, bob = 'c' * 64; + final recipients = messageMentionPubkeys( + channel: Channel( + id: 'dm', + name: 'DM', + channelType: 'dm', + visibility: 'private', + description: '', + createdBy: 'self', + createdAt: DateTime(2026), + memberCount: 4, + ), + senderPubkey: 'self', + explicitMentions: [first, second], + dmRecipientPubkeys: [first, second, bob], + ); + final keys = mentionedPubkeysFromTags([ + for (final key in recipients) ['p', key], + ]); + // MessageBubble omits profiles with no displayName. Bob is only a DM + // recipient, not the author-selected plain Scout. + final profiles = {second: 'Scout', bob: 'Bob'}; + String? tapped; + await tester.pumpWidget( + WidgetHelpers.testable( + child: MessageContent( + content: '@Scout @Scout ($second)', + mentionNames: { + for (final key in keys) + if (profiles[key] != null) key: profiles[key]!, + }, + onMentionTap: (key) => tapped = key, + ), + ), + ); + await tester.pumpAndSettle(); + if (find.text('Bob').evaluate().isNotEmpty) { + await tester.tap(find.text('Bob')); + } + expect(tapped, isNot(bob)); + expect(find.text('Bob'), findsNothing); + expect(find.text('Scout'), findsNothing); + await tester.tap(find.text('Scout (bbbbbbbb…bbbb)')); + expect(tapped, second); + }); + testWidgets('untagged qualifiers cannot become shorter clickable aliases', ( tester, ) async { await tester.pumpWidget( WidgetHelpers.testable( child: MessageContent( - content: '@Scout (${'b' * 64})', + content: '@Scout @Scout (${'b' * 64})', mentionNames: {'a' * 64: 'Scout'}, onMentionTap: (_) => fail('unbound text is not a profile target'), ), @@ -42,7 +94,7 @@ void main() { expect(tester.takeException(), isNull); }); - testWidgets('namesake chips resolve exact tagged keys, not tag order', ( + testWidgets('only qualified namesake chips have an exact historical target', ( tester, ) async { final semantics = tester.ensureSemantics(); @@ -59,54 +111,49 @@ void main() { ), ); await tester.pumpAndSettle(); - await tester.tap(find.text('Scout')); - expect(tapped, first); + expect(find.text('Scout'), findsNothing); await tester.tap(find.text('Scout (bbbbbbbb…bbbb)')); expect(tapped, second); expect(find.bySemanticsLabel(RegExp('Scout.*$second')), findsOneWidget); expect(tester.takeException(), isNull); semantics.dispose(); }); - testWidgets('historical namesakes preserve targets with ordinary siblings', ( - tester, - ) async { - final first = 'a' * 64, second = 'b' * 64, sibling = 'c' * 64; - for (final firstName in ['Scout', 'Renamed Scout', first]) { - for (final secondName in ['Scout', 'Renamed Scout', second]) { - for (final reverse in [false, true]) { - for (final ambiguous in [false, true]) { - final entries = { - first: firstName, - second: secondName, - sibling: 'Alice', - if (ambiguous) 'd' * 64: 'Unknown', - }.entries.toList(); - String? tapped; - await tester.pumpWidget( - WidgetHelpers.testable( - child: MessageContent( - content: '@Scout @Scout ($second) @Alice', - mentionNames: Map.fromEntries( - reverse ? entries.reversed : entries, + testWidgets( + 'historical plain namesakes stay unbound with ordinary siblings', + (tester) async { + final first = 'a' * 64, second = 'b' * 64, sibling = 'c' * 64; + for (final firstName in ['Scout', 'Renamed Scout', first]) { + for (final secondName in ['Scout', 'Renamed Scout', second]) { + for (final reverse in [false, true]) { + for (final ambiguous in [false, true]) { + final entries = { + first: firstName, + second: secondName, + sibling: 'Alice', + if (ambiguous) 'd' * 64: 'Unknown', + }.entries.toList(); + String? tapped; + await tester.pumpWidget( + WidgetHelpers.testable( + child: MessageContent( + content: '@Scout @Scout ($second) @Alice', + mentionNames: Map.fromEntries( + reverse ? entries.reversed : entries, + ), + onMentionTap: (key) => tapped = key, ), - onMentionTap: (key) => tapped = key, ), - ), - ); - await tester.pumpAndSettle(); - if (ambiguous) { + ); + await tester.pumpAndSettle(); expect(find.text(firstName), findsNothing); - } else { - await tester.tap(find.text(firstName)); - expect(tapped, first); + await tester.tap(find.text('Scout (bbbbbbbb…bbbb)')); + expect(tapped, second); + await tester.tap(find.text('Alice')); + expect(tapped, sibling); } - await tester.tap(find.text('Scout (bbbbbbbb…bbbb)')); - expect(tapped, second); - await tester.tap(find.text('Alice')); - expect(tapped, sibling); } } } - } - }); + }, + ); } diff --git a/mobile/test/shared/mentions/mention_bindings_test.dart b/mobile/test/shared/mentions/mention_bindings_test.dart index e73a64e5d7..0b355e99c9 100644 --- a/mobile/test/shared/mentions/mention_bindings_test.dart +++ b/mobile/test/shared/mentions/mention_bindings_test.dart @@ -29,39 +29,51 @@ void main() { isEmpty, ); }); - test( - 'tagged qualified identity narrows a namesake independently of tag order', - () { - for (final names in [ - {'a': 'Scout', second: 'Scout'}, - {second: 'Scout', 'a': 'Scout'}, - ]) { - final bindings = renderedMentionBindings( - '@Scout @Scout ($second)', - names, - ); - expect(bindings['scout'], {'a'}); - expect(bindings['scout ($second)'], {second}); - } - expect( - renderedMentionBindings('@Scout', { - first: 'Scout', - second: 'Scout', - })['scout'], - {first, second}, + test('multiple historical bases remain unbound despite current aliases', () { + final bindings = renderedMentionBindings( + '@Scout @Scout ($second) @Zed @Zed ($first)', + { + first: 'Old Zed', + second: 'Old Scout', + 'c' * 64: 'Scout', + 'd' * 64: 'Zed', + }, + ); + expect(bindings['scout'], isEmpty); + expect(bindings['zed'], isEmpty); + expect(bindings['scout ($second)'], {second}); + expect(bindings['zed ($first)'], {first}); + }); + test('qualified identity does not authorize an inferred plain namesake', () { + for (final names in [ + {'a': 'Scout', second: 'Scout'}, + {second: 'Scout', 'a': 'Scout'}, + ]) { + final bindings = renderedMentionBindings( + '@Scout @Scout ($second)', + names, ); - expect( - renderedMentionBindings('@Scout ($second)', { - first: 'Scout', - })['scout ($second)'], - isEmpty, - ); - expect( - renderedMentionBindings('@Old ($second)', { - second: 'New', - })['old ($second)'], - {second}, - ); - }, - ); + expect(bindings['scout'], isEmpty); + expect(bindings['scout ($second)'], {second}); + } + expect( + renderedMentionBindings('@Scout', { + first: 'Scout', + second: 'Scout', + })['scout'], + {first, second}, + ); + expect( + renderedMentionBindings('@Scout ($second)', { + first: 'Scout', + })['scout ($second)'], + isEmpty, + ); + expect( + renderedMentionBindings('@Old ($second)', { + second: 'New', + })['old ($second)'], + {second}, + ); + }); }