fix: prioritize bot DM in updateBotOptions, handle DM outside _targetBotChats (#5698)

Closes #5697

- Extract buildUpdatedBotOptions (pure) and applyBotOptionUpdatesInOrder
  (async orchestration) as top-level @visibleForTesting functions
- Handle bot DM independently of _targetBotChats filter — the DM may
  lack a botOptions state event or have a stale activityPlan, but it's
  the most important room to keep current
- Update remaining rooms sequentially (not Future.wait) to avoid rate-limiting
- DM errors propagate; other room errors are logged and isolated
- Add 17 unit tests covering both extracted functions
- Add profile.instructions.md design doc for profile settings architecture
This commit is contained in:
wcjord 2026-02-14 14:47:05 -05:00 committed by GitHub
parent 5fed2df7f2
commit 75a237b90c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 509 additions and 39 deletions

View file

@ -0,0 +1,110 @@
---
applyTo: "lib/pangea/user/user_model.dart, lib/pangea/user/user_controller.dart, lib/pangea/user/public_profile_model.dart, lib/pangea/user/analytics_profile_model.dart, lib/pangea/chat_settings/utils/bot_client_extension.dart, lib/pangea/chat_settings/models/bot_options_model.dart, lib/pangea/bot/utils/bot_room_extension.dart, lib/pangea/bot/widgets/bot_chat_settings_dialog.dart, lib/pangea/learning_settings/**, lib/pangea/common/controllers/pangea_controller.dart"
---
# Profile Settings — Architecture & Contracts
How profile settings are structured, stored, propagated, and surfaced to other users.
## Data Model
`Profile` (in [user_model.dart](lib/pangea/user/user_model.dart)) is the top-level container. It wraps three sub-models:
- **`UserSettings`** — learning prefs: target/source language, CEFR level, gender, voice, country, about, etc.
- **`UserToolSettings`** — per-tool on/off toggles (interactive translator, grammar, immersion mode, definitions, auto-IGC, autocorrect, TTS).
- **`InstructionSettings`** — which instructional tooltips the user has dismissed.
A separate **`PublicProfileModel`** (in [public_profile_model.dart](lib/pangea/user/public_profile_model.dart)) holds data visible to other users: analytics level/XP per language, country, and about. It lives on the Matrix user profile (public), not in account data.
> **Open question**: `country` and `about` are the only fields that cross the private → public boundary (they live in `UserSettings` but get synced to `PublicProfileModel`). It might be cleaner to keep them solely in `PublicProfileModel` and edit them in a "public profile" editor, making the privacy boundary explicit.
## Storage & Sync
| Concern | Design Decision | Why |
|---|---|---|
| **Format** | Single JSON blob in Matrix account data under key `profile` | Atomic writes; no partial-update races between fields |
| **Cross-device sync** | Rides on standard Matrix sync | No extra infrastructure; every logged-in device gets updates automatically |
| **Caching** | `UserController` reads account data on first sync, caches in memory, and refreshes on subsequent sync events | Avoids repeated deserialization; single source of truth in-process |
| **Change detection** | Two separate streams: **`languageStream`** (source/target language changed) and **`settingsUpdateStream`** (everything else) | Language changes have heavyweight side-effects (cache clearing, bot option updates, public profile sync) that other setting changes don't need |
| **Side-effect orchestration** | `PangeaController` subscribes to both streams and triggers bot option propagation + public profile sync | Keeps `UserController` focused on data; orchestration lives in the central controller |
| **Migration** | Legacy users stored individual account data keys; a migration path reads those keys and re-saves in the unified format | One-time upgrade, no ongoing cost |
## Entry Points for Changing Settings
### Full settings UI (opens `SettingsLearning`)
1. **Settings page → Learning** — full-page at `/settings/learning`
2. **IGC button long-press** — modal dialog from the writing-assistance button in the chat input
3. **IT bar gear icon** — modal dialog from the interactive translation bar
4. **Analytics language indicator** — modal dialog from the language-pair chip (e.g. "EN → ES") on the learning progress widget
All four use the same `SettingsLearning` widget and the same save flow: write to account data → wait for sync round-trip → stream dispatch → side-effects.
### Per-room bot settings
5. **Bot member menu**`BotChatSettingsDialog`, opened from the bot's member profile in a room. Updates the profile *and* immediately calls bot option propagation rather than waiting for the stream (avoids perceived lag in the room the user is looking at).
### Inline language-switch prompts
These bypass the full settings UI and only change `targetLanguage`:
6. **Activity session mismatch** — When the user tries to send a message in an activity room whose target language differs from their current L2, a popup offers to switch. Rate-limited to once per 30 minutes per room (via `LanguageMismatchRepo`) to avoid nagging. On confirm, updates the profile and auto-sends the pending message.
7. **Reading toolbar mismatch** — When the user taps a message in a language that doesn't match their L2 and the selected toolbar mode is unavailable, a snackbar offers a "Learn" button to switch their target language.
### Contract all paths must satisfy
Every path that changes settings **must** write to account data via `UserController.updateProfile`. The sync-driven stream is the canonical trigger for propagating changes to the bot and public profile. The only exception is the bot-settings dialog (path 5), which additionally calls bot propagation eagerly for responsiveness.
## Bot Option Propagation
The bot reads the user's settings from a `pangea.bot_options` room state event. The client is responsible for keeping this event current.
### Priority ordering
1. **Bot DM first** — The user's 1:1 chat with the bot is updated first, synchronously, with errors propagating to the caller. This is the room the user is most likely actively using.
2. **Other eligible rooms sequentially** — Updated one-by-one (not in parallel) to avoid Matrix rate-limiting. Individual failures are logged but don't block other rooms.
### Eligible room criteria
A room receives bot option updates if:
- It has a `pangea.bot_options` state event
- It has **no** `pangea.activity_plan` state event (activity rooms manage their own options)
- It has exactly 2 joined members, one of which is the bot
### Retry policy
Each room state write retries up to 3× with exponential backoff (5 s → 10 s → 20 s).
### Known limitation
The activity-plan filter uses state event presence, but Matrix state events persist after an activity ends. Rooms with stale activity plans won't get their options updated. The DM-first strategy mitigates this since the most important room is always covered.
## Public vs. Private Boundary
| Data | Where it lives | Who can see it |
|---|---|---|
| `UserSettings` (language, gender, CEFR, voice, tool toggles) | Matrix account data | Only the owning user |
| `PublicProfileModel` (analytics level/XP, country, about) | Matrix user profile | Anyone in shared rooms |
`country` and `about` are synced from `UserSettings``PublicProfileModel` on every settings update. All other settings remain private. Other users see *derived* analytics levels (computed from chat activity), not the self-reported CEFR level.
## Key Files
| Concern | Location |
|---|---|
| Profile / UserSettings models | [lib/pangea/user/user_model.dart](lib/pangea/user/user_model.dart) |
| UserController (cache, streams, updateProfile) | [lib/pangea/user/user_controller.dart](lib/pangea/user/user_controller.dart) |
| Side-effect subscriptions | [lib/pangea/common/controllers/pangea_controller.dart](lib/pangea/common/controllers/pangea_controller.dart) |
| Bot option propagation | [lib/pangea/chat_settings/utils/bot_client_extension.dart](lib/pangea/chat_settings/utils/bot_client_extension.dart) |
| BotOptionsModel | [lib/pangea/chat_settings/models/bot_options_model.dart](lib/pangea/chat_settings/models/bot_options_model.dart) |
| Language mismatch popup + rate limiter | [lib/pangea/learning_settings/](lib/pangea/learning_settings/) |
| Public profile model | [lib/pangea/user/public_profile_model.dart](lib/pangea/user/public_profile_model.dart) |
| Settings UI | [lib/pangea/learning_settings/settings_learning.dart](lib/pangea/learning_settings/settings_learning.dart) |
| Bot chat settings dialog | [lib/pangea/bot/widgets/bot_chat_settings_dialog.dart](lib/pangea/bot/widgets/bot_chat_settings_dialog.dart) |
## Future Work
- **Bio / about editing** — Users currently have no UI to set or edit their `about` field. Add an input to either the learning settings page or a dedicated public-profile editor.
- **Bio / about display** — Decide where other users see the bio. Candidates: user profile sheet in a room, member list hover card, space member directory. Also resolve whether `about` should stay in `UserSettings` (private, synced to public) or move entirely to `PublicProfileModel`.
- **Public learning stats** — Surface vocab count, grammar construct progress, and completed activities on a user's public profile so classmates and teachers can see learning outcomes, not just XP/level.

View file

@ -1,5 +1,6 @@
import 'package:collection/collection.dart';
import 'package:matrix/matrix.dart';
import 'package:meta/meta.dart';
import 'package:fluffychat/pangea/activity_sessions/activity_room_extension.dart';
import 'package:fluffychat/pangea/bot/utils/bot_name.dart';
@ -13,6 +14,65 @@ import 'package:fluffychat/pangea/learning_settings/gender_enum.dart';
import 'package:fluffychat/pangea/user/user_model.dart';
import 'package:fluffychat/widgets/matrix.dart';
/// Builds updated [BotOptionsModel] if any bot-relevant user setting differs
/// from [currentOptions]. Returns null when no changes are needed.
@visibleForTesting
BotOptionsModel? buildUpdatedBotOptions({
required BotOptionsModel? currentOptions,
required UserSettings userSettings,
required String? userId,
}) {
final botOptions = currentOptions ?? const BotOptionsModel();
final targetLanguage = userSettings.targetLanguage;
final languageLevel = userSettings.cefrLevel;
final voice = userSettings.voice;
final gender = userSettings.gender;
if (botOptions.targetLanguage == targetLanguage &&
botOptions.languageLevel == languageLevel &&
botOptions.targetVoice == voice &&
botOptions.userGenders[userId] == gender) {
return null;
}
final updatedGenders = Map<String, GenderEnum>.from(botOptions.userGenders);
if (userId != null && updatedGenders[userId] != gender) {
updatedGenders[userId] = gender;
}
return botOptions.copyWith(
targetLanguage: targetLanguage,
languageLevel: languageLevel,
targetVoice: voice,
userGenders: updatedGenders,
);
}
/// Executes async update functions in priority order:
/// 1. [priorityUpdate] runs first errors propagate to the caller.
/// 2. [remainingUpdates] run sequentially errors go to [onError].
///
/// This ensures the bot DM (most important room) is updated first and
/// rate-limiting from parallel requests doesn't block it.
@visibleForTesting
Future<void> applyBotOptionUpdatesInOrder({
required Future<void> Function()? priorityUpdate,
required List<Future<void> Function()> remainingUpdates,
void Function(Object error, StackTrace stack)? onError,
}) async {
if (priorityUpdate != null) {
await priorityUpdate();
}
for (final update in remainingUpdates) {
try {
await update();
} catch (e, s) {
onError?.call(e, s);
}
}
}
extension BotClientExtension on Client {
bool get hasBotDM => rooms.any((r) => r.isBotDM);
Room? get botDM => rooms.firstWhereOrNull((r) => r.isBotDM);
@ -52,52 +112,51 @@ extension BotClientExtension on Client {
);
Future<void> updateBotOptions(UserSettings userSettings) async {
final targetBotRooms = [..._targetBotChats];
if (targetBotRooms.isEmpty) return;
final dm = botDM;
Future<void> Function()? dmUpdate;
try {
final futures = <Future>[];
for (final targetBotRoom in targetBotRooms) {
final botOptions = targetBotRoom.botOptions ?? const BotOptionsModel();
final targetLanguage = userSettings.targetLanguage;
final languageLevel = userSettings.cefrLevel;
final voice = userSettings.voice;
final gender = userSettings.gender;
if (botOptions.targetLanguage == targetLanguage &&
botOptions.languageLevel == languageLevel &&
botOptions.targetVoice == voice &&
botOptions.userGenders[userID] == gender) {
continue;
}
final updatedGenders = Map<String, GenderEnum>.from(
botOptions.userGenders,
);
if (updatedGenders[userID] != gender) {
updatedGenders[userID!] = gender;
}
final updated = botOptions.copyWith(
targetLanguage: targetLanguage,
languageLevel: languageLevel,
targetVoice: voice,
userGenders: updatedGenders,
);
futures.add(targetBotRoom.setBotOptions(updated));
// Handle the bot DM independently of _targetBotChats.
// The DM may not pass _targetBotChats filters (e.g., botOptions is null,
// or a stale activityPlan state event exists), but it's the most important
// room to keep current.
if (dm != null) {
final updated = buildUpdatedBotOptions(
currentOptions: dm.botOptions,
userSettings: userSettings,
userId: userID,
);
if (updated != null) {
dmUpdate = () => dm.setBotOptions(updated);
}
}
await Future.wait(futures);
} catch (e, s) {
ErrorHandler.logError(
// Remaining eligible rooms, excluding the DM (already handled above).
final otherUpdates = <Future<void> Function()>[];
for (final room in _targetBotChats) {
if (room == dm) continue;
final updated = buildUpdatedBotOptions(
currentOptions: room.botOptions,
userSettings: userSettings,
userId: userID,
);
if (updated == null) continue;
otherUpdates.add(() => room.setBotOptions(updated));
}
if (dmUpdate == null && otherUpdates.isEmpty) return;
await applyBotOptionUpdatesInOrder(
priorityUpdate: dmUpdate,
remainingUpdates: otherUpdates,
onError: (e, s) => ErrorHandler.logError(
e: e,
s: s,
data: {
'userSettings': userSettings.toJson(),
'targetBotRooms': targetBotRooms.map((r) => r.id).toList(),
},
);
}
),
);
}
}

View file

@ -0,0 +1,301 @@
import 'package:fluffychat/pangea/chat_settings/models/bot_options_model.dart';
import 'package:fluffychat/pangea/chat_settings/utils/bot_client_extension.dart';
import 'package:fluffychat/pangea/learning_settings/gender_enum.dart';
import 'package:fluffychat/pangea/learning_settings/language_level_type_enum.dart';
import 'package:fluffychat/pangea/user/user_model.dart';
import 'package:flutter_test/flutter_test.dart';
void main() {
// ---------------------------------------------------------------------------
// buildUpdatedBotOptions pure logic tests
// ---------------------------------------------------------------------------
group('buildUpdatedBotOptions', () {
final baseSettings = UserSettings(
targetLanguage: 'es',
cefrLevel: LanguageLevelTypeEnum.b1,
voice: 'voice_1',
gender: GenderEnum.woman,
);
const userId = '@user:server';
test('returns null when all relevant fields already match', () {
final currentOptions = BotOptionsModel(
targetLanguage: 'es',
languageLevel: LanguageLevelTypeEnum.b1,
targetVoice: 'voice_1',
userGenders: const {userId: GenderEnum.woman},
);
final result = buildUpdatedBotOptions(currentOptions: currentOptions, userSettings: baseSettings, userId: userId);
expect(result, isNull);
});
test('returns updated model when targetLanguage differs', () {
final currentOptions = BotOptionsModel(
targetLanguage: 'fr',
languageLevel: LanguageLevelTypeEnum.b1,
targetVoice: 'voice_1',
userGenders: const {userId: GenderEnum.woman},
);
final result = buildUpdatedBotOptions(currentOptions: currentOptions, userSettings: baseSettings, userId: userId);
expect(result, isNotNull);
expect(result!.targetLanguage, 'es');
// Other fields carried over
expect(result.languageLevel, LanguageLevelTypeEnum.b1);
expect(result.targetVoice, 'voice_1');
});
test('returns updated model when languageLevel differs', () {
final currentOptions = BotOptionsModel(
targetLanguage: 'es',
languageLevel: LanguageLevelTypeEnum.a1,
targetVoice: 'voice_1',
userGenders: const {userId: GenderEnum.woman},
);
final result = buildUpdatedBotOptions(currentOptions: currentOptions, userSettings: baseSettings, userId: userId);
expect(result, isNotNull);
expect(result!.languageLevel, LanguageLevelTypeEnum.b1);
});
test('returns updated model when voice differs', () {
final currentOptions = BotOptionsModel(
targetLanguage: 'es',
languageLevel: LanguageLevelTypeEnum.b1,
targetVoice: 'voice_2',
userGenders: const {userId: GenderEnum.woman},
);
final result = buildUpdatedBotOptions(currentOptions: currentOptions, userSettings: baseSettings, userId: userId);
expect(result, isNotNull);
expect(result!.targetVoice, 'voice_1');
});
test('returns updated model when gender differs', () {
final currentOptions = BotOptionsModel(
targetLanguage: 'es',
languageLevel: LanguageLevelTypeEnum.b1,
targetVoice: 'voice_1',
userGenders: const {userId: GenderEnum.man},
);
final result = buildUpdatedBotOptions(currentOptions: currentOptions, userSettings: baseSettings, userId: userId);
expect(result, isNotNull);
expect(result!.userGenders[userId], GenderEnum.woman);
});
test('defaults to empty BotOptionsModel when currentOptions is null', () {
final result = buildUpdatedBotOptions(currentOptions: null, userSettings: baseSettings, userId: userId);
expect(result, isNotNull);
expect(result!.targetLanguage, 'es');
expect(result.languageLevel, LanguageLevelTypeEnum.b1);
expect(result.targetVoice, 'voice_1');
expect(result.userGenders[userId], GenderEnum.woman);
});
test('preserves gender entries for other users', () {
final currentOptions = BotOptionsModel(
targetLanguage: 'fr', // different triggers update
languageLevel: LanguageLevelTypeEnum.b1,
targetVoice: 'voice_1',
userGenders: const {'@other:server': GenderEnum.man, userId: GenderEnum.woman},
);
final result = buildUpdatedBotOptions(currentOptions: currentOptions, userSettings: baseSettings, userId: userId);
expect(result, isNotNull);
expect(result!.userGenders['@other:server'], GenderEnum.man);
expect(result.userGenders[userId], GenderEnum.woman);
});
test('handles null userId gracefully', () {
const currentOptions = BotOptionsModel(targetLanguage: 'fr');
final result = buildUpdatedBotOptions(currentOptions: currentOptions, userSettings: baseSettings, userId: null);
expect(result, isNotNull);
expect(result!.targetLanguage, 'es');
// Gender not set because userId is null
expect(result.userGenders, isEmpty);
});
});
// ---------------------------------------------------------------------------
// applyBotOptionUpdatesInOrder async orchestration tests
// ---------------------------------------------------------------------------
group('applyBotOptionUpdatesInOrder', () {
test('executes priority update before remaining updates', () async {
final callLog = <String>[];
await applyBotOptionUpdatesInOrder(
priorityUpdate: () async {
callLog.add('dm');
},
remainingUpdates: [
() async {
callLog.add('room_a');
},
() async {
callLog.add('room_b');
},
],
);
expect(callLog, ['dm', 'room_a', 'room_b']);
});
test('executes remaining updates sequentially, not in parallel', () async {
final timestamps = <String, int>{};
var counter = 0;
await applyBotOptionUpdatesInOrder(
priorityUpdate: () async {
timestamps['dm_start'] = counter++;
await Future.delayed(const Duration(milliseconds: 30));
timestamps['dm_end'] = counter++;
},
remainingUpdates: [
() async {
timestamps['a_start'] = counter++;
await Future.delayed(const Duration(milliseconds: 30));
timestamps['a_end'] = counter++;
},
() async {
timestamps['b_start'] = counter++;
await Future.delayed(const Duration(milliseconds: 30));
timestamps['b_end'] = counter++;
},
],
);
// Sequential order: dm completes before a starts, a before b
expect(timestamps['dm_end']!, lessThan(timestamps['a_start']!));
expect(timestamps['a_end']!, lessThan(timestamps['b_start']!));
});
test('propagates priority update errors to caller', () async {
expect(
() => applyBotOptionUpdatesInOrder(
priorityUpdate: () async {
throw Exception('DM update failed');
},
remainingUpdates: [],
),
throwsA(isA<Exception>()),
);
});
test('remaining updates do NOT execute when priority update fails', () async {
final callLog = <String>[];
try {
await applyBotOptionUpdatesInOrder(
priorityUpdate: () async {
throw Exception('DM failed');
},
remainingUpdates: [
() async {
callLog.add('room_a');
},
],
);
} catch (_) {}
expect(callLog, isEmpty);
});
test('isolates errors in remaining updates and continues to next room', () async {
final callLog = <String>[];
final errors = <Object>[];
await applyBotOptionUpdatesInOrder(
priorityUpdate: () async {
callLog.add('dm');
},
remainingUpdates: [
() async {
callLog.add('room_a');
},
() async {
throw Exception('room_b failed');
},
() async {
callLog.add('room_c');
},
],
onError: (e, _) => errors.add(e),
);
// room_b's error didn't prevent room_c from running
expect(callLog, ['dm', 'room_a', 'room_c']);
expect(errors, hasLength(1));
expect(errors.first, isA<Exception>());
});
test('works correctly when priority update is null', () async {
final callLog = <String>[];
await applyBotOptionUpdatesInOrder(
priorityUpdate: null,
remainingUpdates: [
() async {
callLog.add('room_a');
},
() async {
callLog.add('room_b');
},
],
);
expect(callLog, ['room_a', 'room_b']);
});
test('handles empty remaining updates list', () async {
final callLog = <String>[];
await applyBotOptionUpdatesInOrder(
priorityUpdate: () async {
callLog.add('dm');
},
remainingUpdates: [],
);
expect(callLog, ['dm']);
});
test('handles all null / empty gracefully', () async {
// Should complete without error
await applyBotOptionUpdatesInOrder(priorityUpdate: null, remainingUpdates: []);
});
test('multiple remaining errors are all reported', () async {
final errors = <Object>[];
await applyBotOptionUpdatesInOrder(
priorityUpdate: null,
remainingUpdates: [
() async {
throw Exception('fail_1');
},
() async {
throw Exception('fail_2');
},
() async {
throw Exception('fail_3');
},
],
onError: (e, _) => errors.add(e),
);
expect(errors, hasLength(3));
});
});
}