From a2bb30e99575f7a1ea1cdedcbb4e4beffff28cc1 Mon Sep 17 00:00:00 2001 From: danvitv Date: Mon, 7 Sep 2026 01:22:29 +0300 Subject: [PATCH] fix(chat): open notification targets and allow memory overlap --- assets/translations/en.json | 1 - assets/translations/ru.json | 1 - docs/ARCHITECTURE.md | 4 +- docs/INVARIANTS.md | 43 +- docs/rules/generation.md | 21 +- docs/rules/race-conditions.md | 24 +- lib/app.dart | 20 +- lib/features/chat/chat_provider.dart | 20 +- .../services/stages/post_gen_coordinator.dart | 5 +- .../services/generation_dispatcher.dart | 13 - .../controllers/memory_book_controller.dart | 2 +- .../memory_draft_generation_controller.dart | 68 +++- .../memory_draft_mutex_test.dart | 21 +- test/chat_input_bar_test.dart | 38 +- test/helpers/pump_glaze_app.dart | 33 +- test/memory_chat_concurrency_test.dart | 374 ++++++++++++++++++ test/notification_navigation_test.dart | 81 ++++ test/trigger_generation_test.dart | 8 +- 18 files changed, 640 insertions(+), 137 deletions(-) create mode 100644 test/memory_chat_concurrency_test.dart create mode 100644 test/notification_navigation_test.dart diff --git a/assets/translations/en.json b/assets/translations/en.json index 97f4c1bc..136f92ba 100644 --- a/assets/translations/en.json +++ b/assets/translations/en.json @@ -2044,7 +2044,6 @@ "memory_books_all_covered": "All messages are already covered", "memory_books_need_more_uncovered": "Need more uncovered messages before creating a draft", "memory_books_drafts_created": "{arg0} drafts created", - "memory_books_chat_generation_active": "Chat generation is active — wait for it to finish before generating a memory draft", "memory_books_messages_not_found": "Messages not found for this draft", "memory_books_setup_embedding_first": "Set up embedding API in Embedding Settings first", "memory_books_reindex_result": "Indexed: {indexed}, Skipped: {skipped}, Failed: {failed}", diff --git a/assets/translations/ru.json b/assets/translations/ru.json index 3af72647..3ba04015 100644 --- a/assets/translations/ru.json +++ b/assets/translations/ru.json @@ -1980,7 +1980,6 @@ "imggen_match_mode_match": "совпадение", "imggen_ref_keyword": "ключевое слово", "memory_books_all_covered": "Все сообщения уже покрыты", - "memory_books_chat_generation_active": "Генерация чата активна — дождитесь её завершения перед генерацией черновика памяти", "memory_books_current_llm_model": "Текущая LLM модель", "memory_books_drafts_created": "Создано черновиков: {arg0}", "memory_books_messages_not_found": "Сообщения для этого черновика не найдены", diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2470c84f..698f7301 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -578,7 +578,7 @@ the result to recover which blocks landed where. | Image gen | `ChatState.isGeneratingImage` + `_imgGenCancelToken` | No (one-shot) | `_imgGenCancelToken` in `ChatNotifier` | | Summary (manual) | Widget-local in `summary_tab.dart` | No | Not abortable (INV-S2) | | Summary (auto) | `AutoSummaryStage`, from `PostGenCoordinator` | No | Not abortable (INV-S2) | -| Memory draft | `MemoryDraftGenerationController` (delegated by `MemoryBookController`) | No | Per-draft `CancelToken`; mutex via `memory_active_drafts_provider` | +| Memory draft | `MemoryDraftGenerationController` (delegated by `MemoryBookController`) | No | Per-draft `CancelToken`; memory-workflow leases via `memory_active_drafts_provider` | ### Reasoning / Thinking @@ -1564,7 +1564,7 @@ Resolved (kept for history; details in git / PR notes): - **prompt_payload_builder split** — `prompt_inputs_collector` + `prompt_payload_assembler`. - **chat_provider decomposition** — controllers + `generation_pipeline` + `saved_message_writer` (~420 lines; further splits possible). - **lorebook_vector_search providers** — moved to `core/state/lorebook_embedding_provider.dart`. -- **Chat ↔ memory draft mutex** — `memory_active_drafts_provider` + `MemoryBookController` (INV-M3/INV-M4). +- **Chat ↔ memory draft isolation** — independent request ownership plus targeted persistence; `memory_active_drafts_provider` now coordinates memory workflows only (INV-M3/INV-M4). - **Session vars on abort/error** — only a successful guarded commit applies the isolate variable delta (INV-C5). - **Memory injection token budget** — `memory_budget.dart` + INV-PS4. diff --git a/docs/INVARIANTS.md b/docs/INVARIANTS.md index 8dfaf9cf..6639cb00 100644 --- a/docs/INVARIANTS.md +++ b/docs/INVARIANTS.md @@ -312,31 +312,27 @@ It never reads or writes `ChatState.isGenerating`. `MemoryDraftGenerator.generate()` calls the API with `stream: false` unconditionally. -### INV-M3: Memory draft cannot start while chat generation is active ✅ ENFORCED (PR-B C12) +### INV-M3: Chat and memory draft generation may overlap ✅ ENFORCED -`MemoryBookController.generateDraft()` rejects a start request -when `chatProvider(_charId).value?.isGenerating == true` for the -target character. The user gets a "Chat generation is active" -error message via the existing `onError` callback. +The two pipelines own separate transports, callbacks, response accumulators, +cancel tokens, and persistence targets. A memory result may mutate only its +target `MemoryDraft`; a chat result may mutate only its owned chat generation +and session state. Cancelling either operation must not cancel or publish into +the other. -The check is read-only on the chat notifier — it does not wait for -the generation to finish; the user must explicitly abort the chat -generation or wait for it to complete. +This contract is exercised in `test/memory_chat_concurrency_test.dart` with +distinct marker responses and reversed completion order. -### INV-M4: Chat generation cannot start while memory draft is active ✅ ENFORCED (PR-B C12) +### INV-M4: Memory draft ownership remains exclusive ✅ ENFORCED -`ChatNotifier.sendMessage()`, `ChatNotifier.regenerateLastAssistant()`, -and `ChatNotifier.continueMessage()` reject a start request when a -memory draft is currently being generated for the same `sessionId`. - -Both invariants share a single new state container: +Starting the same draft twice remains prohibited. Memory workflows use: `lib/features/memory/state/memory_active_drafts_provider.dart` -(`StateNotifierProvider>`). -Drafts are added to the set when generation starts and removed when -it ends (success, error, or cancel). +to coordinate manual and automatic memory work for a session. The lease is not +a chat-generation mutex and chat entry points must not reject because it is +active. Shared state contract is pinned by -`test/characterization/memory_draft_mutex_test.dart` (7 tests). +`test/characterization/memory_draft_mutex_test.dart`. ### INV-M5: Memory draft approval preserves source range ✅ ENFORCED @@ -1053,8 +1049,8 @@ extend: a trailing user message is delegated to `regenerateLastAssistant()`, which generates a normal reply through `GenerationPipeline`; any other trailing role is a no-op. -Mutex: `continueMessage()` rejects when `_isMemoryDraftActive` (same as -`sendMessage` / `regenerateLastAssistant`) — see INV-M4. +`continueMessage()` may overlap memory draft generation under the same +ownership and persistence isolation contract as other chat entry points (INV-M3). ### INV-CM2: Continue runs the same post-generation stages as a send @@ -1270,7 +1266,7 @@ read-modify-write in a Drift transaction: (no NaN, finite numbers, string keys, ≤ 64 KiB total per payload) and surfaces failures as `ArgumentError` → bridge `invalid_request` code. -### INV-JS3: `glaze.triggerGeneration` respects generation mutexes (INV-C1, INV-M3/M4) ✅ ENFORCED +### INV-JS3: `glaze.triggerGeneration` respects chat ownership (INV-C1) ✅ ENFORCED `GenerationDispatcher.dispatch(charId, rawMode, reason)` is the only entry point that touches the chat notifier from a JS call. The @@ -1278,9 +1274,10 @@ dispatcher returns `TriggerResult`: * `TriggerNoSession` — no chat state for `charId` * `TriggerBusy(busyKind: 'chat')` — INV-C1 violated -* `TriggerBusy(busyKind: 'memory_draft')` — INV-M3/M4 violated * `TriggerAccepted` / `TriggerError` +An active memory draft does not make chat busy (INV-M3). + `auto` mode resolves to `continue` (last msg = assistant) or `regenerate` (last msg = user). The dispatcher never auto-aborts; the JS side decides whether to retry. See @@ -1524,7 +1521,7 @@ Before merging any structural PR: - [x] Memory injection respects token budget (PR-B C13 / INV-PS4) - [ ] History cutoff trims oldest messages first - [ ] Summary returns a string without affecting chat state -- [x] Memory draft mutex with chat generation (PR-B C12 / INV-M3, INV-M4) +- [x] Chat/memory concurrency keeps independent ownership and persistence (INV-M3, INV-M4) - [ ] Image generation completes after text generation (continue included — INV-CM2) - [ ] Extensions post-gen runs after normal/regen/continue (INV-EG1, INV-CM2) - [ ] Continue injects one system turn after the extended reply (INV-CM3) diff --git a/docs/rules/generation.md b/docs/rules/generation.md index 1c37fde1..b648068b 100644 --- a/docs/rules/generation.md +++ b/docs/rules/generation.md @@ -34,16 +34,18 @@ Full formal invariants with code references: `docs/INVARIANTS.md` --- -## Mutual exclusion ✅ ENFORCED (PR-B C12) +## Chat and memory concurrency -Chat generation and memory draft **cannot** overlap for the same session/character: +Chat generation and memory draft generation may overlap, including when they +use the same session: -- `MemoryBookController.generateDraft()` rejects when `chatProvider(charId).isGenerating`. -- `sendMessage` / `regenerateLastAssistant` / `continueMessage` reject when - `memoryActiveDraftsProvider` contains the session id. +- Each request owns its transport, callbacks, accumulator/completer, and cancel token. +- Chat persists only its owned session result; memory uses targeted draft mutation. +- Duplicate generation of the same draft remains prohibited. +- `memoryActiveDraftsProvider` coordinates memory workflows only; it does not block chat. See `docs/INVARIANTS.md` INV-M3, INV-M4 and -`test/characterization/memory_draft_mutex_test.dart`. +`test/memory_chat_concurrency_test.dart`. Image generation runs after text generation completes on the normal/regen path (`GenerationPipeline` → `processImageTags()`). Summary is independent. @@ -470,14 +472,15 @@ on expiry. The token is independent of the chat text generation token — aborting the chat does NOT cancel in-flight JS generate calls. `glaze.triggerGeneration` reuses the chat path entirely — see -`GenerationDispatcher.dispatch` for the mutex / abort chain. +`GenerationDispatcher.dispatch` for the ownership / abort chain. --- ## Adding a new generation path 1. Define abort mechanism (`AbortHandler` or separate `CancelToken`). -2. Add mutual exclusion in **both** directions if it shares a `charId` / session. +2. Add mutual exclusion only for shared mutable ownership or a concrete shared + resource; sharing a `charId` / session alone is not sufficient. 3. Verify `isCurrentGen(genId)` before mutating shared state after every `await`. 4. Clear `isGenerating*` on every exit path. 5. Decide whether post-SSE steps (image tags, extensions) must run — use @@ -498,7 +501,7 @@ Before merging any generation-related PR: - [x] Memory injection respects token budget (INV-PS4) - [ ] History cutoff trims oldest first - [ ] Summary does not touch `ChatState.isGenerating` or messages -- [x] Memory draft mutex enforced (INV-M3, INV-M4) +- [ ] Chat and memory overlap without cross-cancellation or cross-persistence (INV-M3, INV-M4) - [ ] Image tags run after text on send/regen (not on continue unless changed) - [ ] Extensions post-gen on send/regen only (INV-EG1) - [ ] Block chain does not start on aborted or errored generation (INV-EG4) diff --git a/docs/rules/race-conditions.md b/docs/rules/race-conditions.md index cb9e3226..c0625ab1 100644 --- a/docs/rules/race-conditions.md +++ b/docs/rules/race-conditions.md @@ -65,14 +65,13 @@ Rule of thumb: if there's an `await` before the mutation, there's a potential ra --- -## Rule 5: Mutual exclusion for concurrent operations - -- Chat generation and memory draft generation **are** mutually exclusive for the same session/character: - - `MemoryBookController.generateDraft()` rejects when `chatProvider(charId).isGenerating`. - - `sendMessage` / `regenerateLastAssistant` / `continueMessage` reject when - `memoryActiveDraftsProvider` contains the session id. - - `glaze.triggerGeneration` reuses `GenerationDispatcher`, which enforces the same - mutex (INV-JS3). The dispatcher returns `TriggerBusy` instead of auto-aborting. +## Rule 5: Isolate concurrent operation ownership + +- Chat generation and memory draft generation may overlap for the same session. +- Each operation must retain independent transport callbacks, response state, + cancellation ownership, staleness checks, and targeted persistence. +- `memoryActiveDraftsProvider` prevents conflicting memory workflows; it is not + a chat mutex. Duplicate generation of one draft remains prohibited. - Image generation runs only after text generation completes (enforced by call order). - Background operations (auto-sync, embedding indexing) should check `isGenerating` for the relevant `charId` before starting. @@ -81,8 +80,9 @@ Rule of thumb: if there's an `await` before the mutation, there's a potential ra contend with chat generation but the `jsRunner` ticks share `SseClient` with chat — keep heavy ticks ≤ 1 per preset at a time. -If adding a new request type alongside chat generation, add mutual exclusion guards -in **both** directions. +If adding a new request type alongside chat generation, serialize only a +concrete shared mutable owner or resource. Otherwise test concurrent completion +and cancellation in both orders. --- @@ -107,9 +107,9 @@ Verify: after pressing Stop, the network tab shows the request was actually term | Two memory drafts start for same draft ID | No in-flight ID tracking in generator | Tracked in widget: `memory_books_tab.dart._generatingDrafts` map | | `apiListProvider` null on cold start | Sync provider read before async load | `await ref.read(apiListProvider.future)` first; also used by `MemoryDraftGenerator` | | Image retry state corruption | A late retry could overwrite newer chat state | ✅ **Fixed** — operation generation IDs plus targeted `mutateMessage` and durable-state publication | -| Chat ↔ memory draft mutual exclusion | Neither side checks the other | ✅ **Fixed** — `memory_active_drafts_provider` enforces mutex in both directions; `glaze.triggerGeneration` reuses the same mutex via `GenerationDispatcher` (INV-M3, INV-M4, INV-JS3) | +| Chat ↔ memory draft output isolation | Concurrent requests could be routed or persisted into the wrong owner | ✅ **Covered** — independent ownership and targeted persistence; production concurrency markers in `memory_chat_concurrency_test.dart` (INV-M3, INV-M4) | | Character deletion orphan rows | Independent provider-level deletion lists missed newer session tables | ✅ **Fixed** — `SessionDeletionQueries` is the shared complete session cascade; `CharacterDeletionRepo` composes it atomically with character lorebooks, folders, rows, and variation promotion. | -| `glaze.triggerGeneration` racing chat generation | JS call while chat is generating | ✅ **Fixed** — `GenerationDispatcher.dispatch` returns `TriggerBusy` when `isGenerating` or `memoryActiveDrafts` is set (INV-JS3). | +| `glaze.triggerGeneration` racing chat generation | JS call while chat is generating | ✅ **Fixed** — `GenerationDispatcher.dispatch` returns `TriggerBusy` for active chat generation; memory generation may overlap (INV-JS3). | | Stale periodic ticks after app background | `Timer.periodic` keeps firing while app is paused | ✅ **Fixed** — `PeriodicTriggerScheduler` pauses on `paused`/`inactive`/`hidden`/`detached` (INV-JS6). No catch-up tick on resume. | | Rapid session switch — stale switch overwrites newer one | `ChatSessionController.switchSession` has no epoch/switchId guard; two concurrent calls race, last `_setState` wins | ✅ **Fixed** — `_switchEpoch` counter in `ChatSessionController`; after each `await`, stale-epoch operations bail out without calling `_setState`. Covers `switchSession`, `createNewSession`, `branchSession`. Tests in `test/characterization/session_switch_race_test.dart` | | `_applySessionPreference` — no cancellation of in-flight switch | `didUpdateWidget` resets `_sessionApplied` and starts a new `_applySessionPreference` without cancelling the old one; shared `_sessionSwitchPending` flag cleared prematurely | ✅ **Fixed** — `_applyEpoch` counter in `_ChatScreenState`; only the latest apply clears `_sessionSwitchPending` in its `finally` block | diff --git a/lib/app.dart b/lib/app.dart index ec15378e..fc2f91d4 100644 --- a/lib/app.dart +++ b/lib/app.dart @@ -53,7 +53,15 @@ class GlazeApp extends ConsumerStatefulWidget { /// without waiting for network-bound initialization. final bool skipStartup; - const GlazeApp({super.key, this.restart, this.skipStartup = false}); + @visibleForTesting + final NotificationNavigationData? notificationForTesting; + + const GlazeApp({ + super.key, + this.restart, + this.skipStartup = false, + this.notificationForTesting, + }); static VoidCallback? _restart; @@ -84,6 +92,14 @@ class _GlazeAppState extends ConsumerState _initInBackground(loadLorebookSettings(ref), 'lorebook settings'); _initInBackground(seedDefaultPresets(ref), 'default preset seeding'); _initInBackground(seedFeaturedPresets(ref), 'featured preset seeding'); + final notificationForTesting = widget.notificationForTesting; + if (notificationForTesting != null) { + WidgetsBinding.instance.addPostFrameCallback((_) { + if (mounted) { + unawaited(_openChatFromNotification(notificationForTesting)); + } + }); + } if (!widget.skipStartup) { _warmInitialListProviders(); } @@ -278,7 +294,7 @@ class _GlazeAppState extends ConsumerState path: '/chat/${data.charId}', queryParameters: query.isEmpty ? null : query, ); - context.push(uri.toString()); + unawaited(ref.read(routerProvider).push(uri.toString())); } @override diff --git a/lib/features/chat/chat_provider.dart b/lib/features/chat/chat_provider.dart index c5651b09..bfe34db4 100644 --- a/lib/features/chat/chat_provider.dart +++ b/lib/features/chat/chat_provider.dart @@ -15,7 +15,6 @@ import '../../core/utils/time_helpers.dart'; import '../../core/state/db_provider.dart'; import '../../core/state/persona_resolution.dart'; import '../chat_history/chat_history_provider.dart'; -import '../memory/state/memory_active_drafts_provider.dart'; import 'abort_handler.dart'; import 'chat_session_service.dart'; import 'chat_state.dart'; @@ -405,8 +404,7 @@ class ChatNotifier extends AsyncNotifier { if (current == null || current.isGenerating || current.isGeneratingImage || - current.isPostGenRunning || - _isMemoryDraftActive(current)) { + current.isPostGenRunning) { return Future.value(false); } @@ -472,10 +470,6 @@ class ChatNotifier extends AsyncNotifier { durableAcceptance?.complete(false); return; } - if (_isMemoryDraftActive(current)) { - durableAcceptance?.complete(false); - return; - } _sendInFlight = true; // Claimed before the try so the `finally` sweep can see it. Bumped here // rather than at the optimistic paint: the claim order is the tap order. @@ -791,8 +785,6 @@ class ChatNotifier extends AsyncNotifier { current.isPostGenRunning) { return; } - if (_isMemoryDraftActive(current)) return; - final lastIdx = current.messages.length - 1; if (lastIdx < 0) return; @@ -911,8 +903,6 @@ class ChatNotifier extends AsyncNotifier { current.isPostGenRunning) { return; } - if (_isMemoryDraftActive(current)) return; - final session = current.session!; // Impersonation never restores a chat message on abort — clear any stale // restoration target left by a prior regenerate so Stop only drops the @@ -1025,8 +1015,6 @@ class ChatNotifier extends AsyncNotifier { current.isPostGenRunning) { return; } - if (_isMemoryDraftActive(current)) return; - final lastIdx = current.messages.length - 1; if (lastIdx < 0) return; final lastMsg = current.messages[lastIdx]; @@ -1058,12 +1046,6 @@ class ChatNotifier extends AsyncNotifier { await _runGeneration(promptSession, current, continueTargetId: lastMsg.id); } - bool _isMemoryDraftActive(ChatState current) { - final sessionId = current.session?.id; - if (sessionId == null) return false; - return ref.read(memoryActiveDraftsProvider).contains(sessionId); - } - Future _runGeneration( ChatSession session, ChatState current, { diff --git a/lib/features/chat/services/stages/post_gen_coordinator.dart b/lib/features/chat/services/stages/post_gen_coordinator.dart index ad0df8fd..78cba018 100644 --- a/lib/features/chat/services/stages/post_gen_coordinator.dart +++ b/lib/features/chat/services/stages/post_gen_coordinator.dart @@ -153,9 +153,8 @@ class PostGenCoordinator { final sessionId = result.session!.id; final studioEnabled = studioTurnConfig?.enabled == true; - // Normal chat has no foreground post-gen hold. Claim the memory session - // before the first await so a new send cannot enter while auto-generation - // is waiting to be scheduled. + // Reserve the memory session before the first await so another memory + // workflow cannot claim the same auto-generation batch while scheduling. final ordinaryMemoryLease = studioEnabled ? null : draftStage.reserveAutoGeneration(result.session); diff --git a/lib/features/extensions/services/generation_dispatcher.dart b/lib/features/extensions/services/generation_dispatcher.dart index df00f984..5680414a 100644 --- a/lib/features/extensions/services/generation_dispatcher.dart +++ b/lib/features/extensions/services/generation_dispatcher.dart @@ -8,7 +8,6 @@ import '../../../core/utils/error_format.dart'; import '../../chat/chat_provider.dart'; import '../../chat/chat_state.dart'; import '../../chat/editing_message_provider.dart'; -import '../../memory/state/memory_active_drafts_provider.dart'; import '../models/trigger_mode.dart'; import '../models/trigger_result.dart'; @@ -20,8 +19,6 @@ import '../models/trigger_result.dart'; /// - INV-C1: at most one active generation per `charId`. The call is /// rejected (not auto-aborted) when `isGenerating == true` so the JS /// script can decide whether to retry / await. -/// - INV-M3 / INV-M4: memory draft mutex. The call is rejected when a -/// memory draft is currently being generated for the same session id. /// - INV-CM1 / INV-CM2 / INV-A3: `continue` and `regenerate` delegate to /// the regular [ChatNotifier.continueMessage] / /// [ChatNotifier.regenerateLastAssistant] entry points so the same @@ -64,13 +61,6 @@ class GenerationDispatcher { return TriggerBusy(busyKind: 'message_edit', mode: mode); } - final memoryActive = ref - .read(memoryActiveDraftsProvider) - .contains(current.session!.id); - if (memoryActive) { - return TriggerBusy(busyKind: 'memory_draft', mode: mode); - } - if (current.isGenerating || current.isPostGenRunning) { return TriggerBusy(busyKind: 'chat', mode: mode); } @@ -132,9 +122,6 @@ class GenerationDispatcher { if (current == null || current.session == null) return null; if (ref.read(editingMessageIdProvider(charId)) != null) return null; if (current.isGenerating || current.isPostGenRunning) return null; - if (ref.read(memoryActiveDraftsProvider).contains(current.session!.id)) { - return null; - } return _resolveAuto(current, TriggerMode.parse(rawMode)); } diff --git a/lib/features/memory/controllers/memory_book_controller.dart b/lib/features/memory/controllers/memory_book_controller.dart index 7f30a945..00f56302 100644 --- a/lib/features/memory/controllers/memory_book_controller.dart +++ b/lib/features/memory/controllers/memory_book_controller.dart @@ -19,7 +19,7 @@ import 'memory_settings_mapper.dart'; /// /// Thin orchestrator: owns the [MemoryBook] + entry/index CRUD + settings /// mapping + reindex, and delegates the draft-generation lifecycle (active -/// set, cancel tokens, elapsed timer, INV-M3 mutex) to +/// set, cancel tokens, elapsed timer, memory-workflow leases) to /// [MemoryDraftGenerationController]. class MemoryBookController { final WidgetRef _ref; diff --git a/lib/features/memory/controllers/memory_draft_generation_controller.dart b/lib/features/memory/controllers/memory_draft_generation_controller.dart index 187ec530..f3f9659d 100644 --- a/lib/features/memory/controllers/memory_draft_generation_controller.dart +++ b/lib/features/memory/controllers/memory_draft_generation_controller.dart @@ -1,9 +1,13 @@ import 'dart:async'; +// Public named constructor arguments intentionally initialize private fields. +// ignore_for_file: prefer_initializing_formals + import 'package:dio/dio.dart'; import 'package:easy_localization/easy_localization.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; +import '../../../core/models/chat_message.dart'; import '../../../core/models/memory_book.dart'; import '../../../core/models/pipeline_settings.dart'; import '../../../core/state/memory_settings_provider.dart'; @@ -15,17 +19,26 @@ import 'memory_book_write_queue.dart'; import 'memory_draft_generation_update.dart'; import 'memory_settings_mapper.dart'; +typedef GenerateMemoryDraft = + Future Function({ + required MemoryDraft draft, + required MemoryBookSettings settings, + required PipelineSettings pipeline, + required List messages, + required String charId, + required String sessionId, + required Map sessionVars, + CancelToken? cancelToken, + }); + /// Owns the memory-draft generation lifecycle for a single chat session: /// the active/generating sets, cancel tokens, and the elapsed-timer. Extracted /// from [MemoryBookController] (plan §6) so the host controller stays a thin /// orchestrator for entry/index CRUD + settings mapping. /// -/// INV-M3 (chat vs memory-draft mutual exclusion) is preserved: `generateDraft` -/// refuses to start when `chatProvider(charId).value?.isGenerating == true` -/// and marks the sessionId active on `memoryActiveDraftsProvider` for the -/// duration of the generation (matching the chat-side INV-M4 guard). The mutex -/// contract is characterized by `test/characterization/memory_draft_mutex_test.dart` -/// (which pins the shared provider, not this controller directly). +/// Chat and memory generation may overlap. The session lease coordinates +/// memory workflows with each other; per-draft ownership keeps late or +/// cancelled results from mutating another operation. /// /// The controller does not own the [MemoryBook] — the host does. It reads the /// book via [bookGetter] and atomically applies targeted updates via @@ -36,6 +49,7 @@ class MemoryDraftGenerationController { final String _charId; final String _sessionId; final MemorySettingsMapper _settingsMapper; + final GenerateMemoryDraft _generate; /// Returns the host's current [MemoryBook] (or `null` if not loaded). final MemoryBook? Function() bookGetter; @@ -53,13 +67,38 @@ class MemoryDraftGenerationController { Timer? _genElapsedTimer; MemoryDraftGenerationController({ - required this._ref, - required this._charId, - required this._sessionId, - required this._settingsMapper, + required WidgetRef ref, + required String charId, + required String sessionId, + required MemorySettingsMapper settingsMapper, required this.bookGetter, required this.persistMutation, - }); + GenerateMemoryDraft? generate, + }) : _ref = ref, + _charId = charId, + _sessionId = sessionId, + _settingsMapper = settingsMapper, + _generate = + generate ?? + (({ + required draft, + required settings, + required pipeline, + required messages, + required charId, + required sessionId, + required sessionVars, + cancelToken, + }) => MemoryDraftGenerator.widget(ref).generate( + draft: draft, + settings: settings, + pipeline: pipeline, + messages: messages, + charId: charId, + sessionId: sessionId, + sessionVars: sessionVars, + cancelToken: cancelToken, + )); Map get generatingDrafts => Map.unmodifiable(_generatingDrafts); Map get genStartTimes => Map.unmodifiable(_genStartTimes); @@ -105,10 +144,6 @@ class MemoryDraftGenerationController { final book = bookGetter(); if (book == null || _activeDraftIds.contains(draftId)) return; final chatState = _ref.read(chatProvider(_charId)); - if (chatState.value?.isGenerating == true) { - onError('memory_books_chat_generation_active'.tr()); - return; - } final draftIndex = book.pendingDrafts.indexWhere((d) => d.id == draftId); if (draftIndex < 0) return; @@ -137,8 +172,7 @@ class MemoryDraftGenerationController { onStart(); try { - final generator = MemoryDraftGenerator.widget(_ref); - final result = await generator.generate( + final result = await _generate( draft: draft, settings: _bookSettings, pipeline: _pipeline, diff --git a/test/characterization/memory_draft_mutex_test.dart b/test/characterization/memory_draft_mutex_test.dart index c1bb95c3..8d0465bb 100644 --- a/test/characterization/memory_draft_mutex_test.dart +++ b/test/characterization/memory_draft_mutex_test.dart @@ -3,24 +3,11 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:glaze_flutter/features/memory/state/memory_active_drafts_provider.dart'; -/// Characterization test for INV-M3 / INV-M4: memory draft generation and -/// chat generation must be mutually exclusive per (charId, sessionId). -/// -/// The fix (PR-B C12) introduces a global -/// [memoryActiveDraftsProvider] — a `StateNotifier>` of -/// sessionIds whose memory drafts are currently generating. Two -/// guards in production code observe this set: -/// -/// * `MemoryBookController.generateDraft` (INV-M3) refuses to start if -/// `chatProvider(charId).value?.isGenerating == true`, and marks the -/// sessionId active for the duration of the generation. -/// * `ChatNotifier.sendMessage`/`regenerateLastAssistant`/ -/// `continueMessage` (INV-M4) refuse to start if the sessionId is -/// in the active set. -/// -/// This test pins down the contract of the shared state container. +/// Characterization test for the memory-workflow lease registry. Manual and +/// automatic memory jobs use [memoryActiveDraftsProvider] to coordinate work +/// for a session. Chat generation does not read this registry and may overlap. void main() { - group('MemoryActiveDraftsNotifier (INV-M3/INV-M4 shared state)', () { + group('MemoryActiveDraftsNotifier memory workflow leases', () { test('initial state is empty', () { final container = ProviderContainer(); addTearDown(container.dispose); diff --git a/test/chat_input_bar_test.dart b/test/chat_input_bar_test.dart index c21031b1..02b74c6f 100644 --- a/test/chat_input_bar_test.dart +++ b/test/chat_input_bar_test.dart @@ -153,16 +153,18 @@ void main() { 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8' 'z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==', ); + File? clipboardImageFile; + var waitForWindowsImage = false; /// What the platform side of `pasteboard` hands back for `image` on this /// host: bytes everywhere except Windows, which passes a path to a file /// the plugin reads and deletes. Object clipboardImagePayload(Uint8List bytes) { if (!Platform.isWindows) return bytes; - final file = File( + clipboardImageFile = File( '${Directory.systemTemp.createTempSync('glaze_pb').path}/clip.png', )..writeAsBytesSync(bytes); - return file.path; + return clipboardImageFile!.path; } /// Puts an image and/or text on the fake clipboard. No image is what @@ -172,6 +174,7 @@ void main() { List files = const [], String? text, }) { + waitForWindowsImage = Platform.isWindows && image != null; final messenger = TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; messenger.setMockMethodCallHandler(const MethodChannel('pasteboard'), ( @@ -203,6 +206,24 @@ void main() { } Future pressPaste(WidgetTester tester) async { + if (waitForWindowsImage) { + // pasteboard turns the mocked path back into bytes with real file + // I/O. Start the unawaited paste in the real async zone so that I/O + // can finish before pumpAndSettle looks for the resulting setState. + await tester.runAsync(() async { + clipboardImageFile = null; + await tester.sendKeyDownEvent(LogicalKeyboardKey.controlLeft); + await tester.sendKeyDownEvent(LogicalKeyboardKey.keyV); + await tester.sendKeyUpEvent(LogicalKeyboardKey.keyV); + await tester.sendKeyUpEvent(LogicalKeyboardKey.controlLeft); + while (clipboardImageFile == null || + await clipboardImageFile!.exists()) { + await Future.delayed(const Duration(milliseconds: 1)); + } + }); + await tester.pumpAndSettle(); + return; + } await tester.sendKeyDownEvent(LogicalKeyboardKey.controlLeft); await tester.sendKeyDownEvent(LogicalKeyboardKey.keyV); await tester.sendKeyUpEvent(LogicalKeyboardKey.keyV); @@ -364,7 +385,18 @@ void main() { expect(paste, hasLength(1)); - paste.single.onPressed!(); + if (waitForWindowsImage) { + await tester.runAsync(() async { + clipboardImageFile = null; + paste.single.onPressed!(); + while (clipboardImageFile == null || + await clipboardImageFile!.exists()) { + await Future.delayed(const Duration(milliseconds: 1)); + } + }); + } else { + paste.single.onPressed!(); + } await tester.pumpAndSettle(); expect(find.byType(Image), findsOneWidget); diff --git a/test/helpers/pump_glaze_app.dart b/test/helpers/pump_glaze_app.dart index fc5dda13..3862d698 100644 --- a/test/helpers/pump_glaze_app.dart +++ b/test/helpers/pump_glaze_app.dart @@ -5,6 +5,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:shared_preferences/shared_preferences.dart'; import 'package:glaze_flutter/app.dart'; +import 'package:glaze_flutter/core/services/generation_notification_service.dart'; /// Call once in setUpAll to initialise EasyLocalization's static state /// (device locale + saved locale from SharedPreferences). @@ -31,6 +32,7 @@ Future pumpGlazeApp( required ProviderContainer container, VoidCallback? restart, Map prefsSeed = const {}, + NotificationNavigationData? notificationForTesting, }) async { SharedPreferences.setMockInitialValues({ 'onboarding_complete': true, @@ -44,7 +46,9 @@ Future pumpGlazeApp( }); await tester.runAsync(() async { - await tester.pumpWidget(_buildApp(container, restart)); + await tester.pumpWidget( + _buildApp(container, restart, notificationForTesting), + ); await Future.delayed(Duration.zero); for (var i = 0; i < 3; i++) { await tester.pump(Duration.zero); @@ -69,13 +73,20 @@ Future pumpNavigation(WidgetTester tester) async { }); } -Widget _buildApp(ProviderContainer container, VoidCallback? restart) => - UncontrolledProviderScope( - container: container, - child: EasyLocalization( - supportedLocales: const [Locale('en'), Locale('ru')], - path: 'assets/translations', - fallbackLocale: const Locale('en'), - child: GlazeApp(restart: restart, skipStartup: true), - ), - ); +Widget _buildApp( + ProviderContainer container, + VoidCallback? restart, + NotificationNavigationData? notificationForTesting, +) => UncontrolledProviderScope( + container: container, + child: EasyLocalization( + supportedLocales: const [Locale('en'), Locale('ru')], + path: 'assets/translations', + fallbackLocale: const Locale('en'), + child: GlazeApp( + restart: restart, + skipStartup: true, + notificationForTesting: notificationForTesting, + ), + ), +); diff --git a/test/memory_chat_concurrency_test.dart b/test/memory_chat_concurrency_test.dart new file mode 100644 index 00000000..bda8e3aa --- /dev/null +++ b/test/memory_chat_concurrency_test.dart @@ -0,0 +1,374 @@ +import 'dart:async'; + +import 'package:dio/dio.dart'; +import 'package:drift/native.dart'; +import 'package:flutter/widgets.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +import 'package:glaze_flutter/core/db/app_db.dart'; +import 'package:glaze_flutter/core/llm/studio_turn_config_snapshot.dart'; +import 'package:glaze_flutter/core/models/character.dart'; +import 'package:glaze_flutter/core/models/chat_message.dart'; +import 'package:glaze_flutter/core/models/memory_book.dart'; +import 'package:glaze_flutter/core/state/db_provider.dart'; +import 'package:glaze_flutter/features/chat/chat_generation_service.dart'; +import 'package:glaze_flutter/features/chat/chat_provider.dart'; +import 'package:glaze_flutter/features/chat/chat_session_service.dart'; +import 'package:glaze_flutter/features/chat/chat_state.dart'; +import 'package:glaze_flutter/features/memory/controllers/memory_book_write_queue.dart'; +import 'package:glaze_flutter/features/memory/controllers/memory_draft_generation_controller.dart'; +import 'package:glaze_flutter/features/memory/controllers/memory_settings_mapper.dart'; +import 'package:glaze_flutter/features/memory/state/memory_active_drafts_provider.dart'; + +class _ControlledChatGenerationService extends ChatGenerationService { + _ControlledChatGenerationService(super.ref); + + final started = Completer(); + final result = Completer(); + int calls = 0; + + @override + Future generate({ + required ChatSession session, + ChatSession? saveSession, + required String charId, + required int genId, + required ChatState currentState, + required void Function(ChatState) onStateUpdate, + required bool Function() isAborted, + List? previousSwipes, + int previousSwipeId = 0, + String? previousReasoning, + String? previousGenTime, + int? previousTokens, + List>? previousSwipesMeta, + String? guidanceText, + String? regenTargetId, + String? continueTargetId, + StudioTurnConfigSnapshot? studioTurnConfig, + }) { + calls++; + if (!started.isCompleted) started.complete(session); + return result.future; + } + + @override + Future processImageTags({ + required ChatState currentState, + required String charId, + String? targetMessageId, + CancelToken? cancelToken, + bool Function()? isCurrentOperation, + required void Function(ChatState) onStateUpdate, + }) async {} +} + +class _Harness { + _Harness({ + required this.container, + required this.chatService, + required this.chatNotifier, + required this.memoryController, + required this.memoryStarted, + required this.memoryResult, + required this.memoryCalls, + required this.charId, + required this.sessionId, + }); + + final ProviderContainer container; + final _ControlledChatGenerationService chatService; + final ChatNotifier chatNotifier; + final MemoryDraftGenerationController memoryController; + final Completer memoryStarted; + final Completer memoryResult; + final int Function() memoryCalls; + final String charId; + final String sessionId; +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + Future pumpUntil( + WidgetTester tester, + bool Function() condition, + String stage, + ) async { + for (var i = 0; i < 300 && !condition(); i++) { + await tester.pump(const Duration(milliseconds: 10)); + } + if (!condition()) throw TestFailure('Timed out waiting for $stage'); + } + + Future pumpUntilPersisted( + WidgetTester tester, + Future Function() condition, + String stage, + ) async { + for (var i = 0; i < 300; i++) { + await tester.pump(const Duration(milliseconds: 10)); + if (await condition()) return; + } + throw TestFailure('Timed out waiting for $stage'); + } + + const initialDraft = MemoryDraft( + id: 'draft-1', + messageIds: ['m1'], + content: '', + ); + var harnessSequence = 0; + + Future pumpRef( + WidgetTester tester, + ProviderContainer container, + ) async { + late WidgetRef captured; + await tester.pumpWidget( + UncontrolledProviderScope( + container: container, + child: Consumer( + builder: (context, ref, child) { + captured = ref; + return const SizedBox(); + }, + ), + ), + ); + return captured; + } + + Future<_Harness> createHarness(WidgetTester tester) async { + SharedPreferences.setMockInitialValues({}); + ChatSessionService.clearCache(); + final fixtureId = ++harnessSequence; + final charId = 'c$fixtureId'; + final sessionId = 's$fixtureId'; + final db = AppDatabase.forTesting(NativeDatabase.memory()); + late _ControlledChatGenerationService chatService; + final container = ProviderContainer( + overrides: [ + appDbProvider.overrideWithValue(db), + chatGenerationServiceProvider.overrideWith((ref) { + return chatService = _ControlledChatGenerationService(ref); + }), + ], + ); + addTearDown(() async { + container.dispose(); + ChatSessionService.clearCache(); + await db.close(); + }); + + final session = ChatSession( + id: sessionId, + characterId: charId, + sessionIndex: 0, + messages: [ + ChatMessage(id: 'm1', role: 'user', content: 'Hello', timestamp: 1), + ], + ); + final book = MemoryBook( + id: 'memorybook_$sessionId', + sessionId: sessionId, + pendingDrafts: const [initialDraft], + ); + await container + .read(characterRepoProvider) + .put(Character(id: charId, name: 'Alice')); + await container.read(chatRepoProvider).put(session); + await container.read(memoryBookRepoProvider).put(book); + await container.read(chatProvider(charId).future); + container.read(chatGenerationServiceProvider); + + final widgetRef = await pumpRef(tester, container); + addTearDown(() => tester.pumpWidget(const SizedBox())); + final memoryStarted = Completer(); + final memoryResult = Completer(); + var memoryCalls = 0; + MemoryBook? currentBook = book; + final repo = container.read(memoryBookRepoProvider); + final writeQueue = MemoryBookWriteQueue( + readLatest: () => currentBook, + publish: (book) => currentBook = book, + persist: repo.put, + ); + final memoryController = MemoryDraftGenerationController( + ref: widgetRef, + charId: charId, + sessionId: sessionId, + settingsMapper: const MemorySettingsMapper(), + bookGetter: () => currentBook, + persistMutation: writeQueue.mutate, + generate: + ({ + required draft, + required settings, + required pipeline, + required messages, + required charId, + required sessionId, + required sessionVars, + cancelToken, + }) { + memoryCalls++; + if (!memoryStarted.isCompleted) memoryStarted.complete(); + return memoryResult.future; + }, + ); + addTearDown(memoryController.dispose); + + return _Harness( + container: container, + chatService: chatService, + chatNotifier: container.read(chatProvider(charId).notifier), + memoryController: memoryController, + memoryStarted: memoryStarted, + memoryResult: memoryResult, + memoryCalls: () => memoryCalls, + charId: charId, + sessionId: sessionId, + ); + } + + Future startMemory(_Harness harness) => + harness.memoryController.generateDraft( + initialDraft.id, + onStart: () {}, + onComplete: () {}, + onError: (error) => fail('memory generation failed: $error'), + ); + + testWidgets( + 'chat and manual memory overlap and persist only their own marker', + (tester) async { + final harness = await createHarness(tester); + final existingMemoryLease = harness.container + .read(memoryActiveDraftsProvider.notifier) + .acquire(harness.sessionId); + final chatFuture = harness.chatNotifier.regenerateLastAssistant(); + await pumpUntil( + tester, + () => harness.chatService.started.isCompleted, + 'chat generation start', + ); + final chatSession = await harness.chatService.started.future; + existingMemoryLease.release(); + final memoryFuture = startMemory(harness); + await pumpUntil( + tester, + () => harness.memoryStarted.isCompleted, + 'memory generation start', + ); + + expect( + harness.memoryController.isDraftGenerating(initialDraft.id), + isTrue, + ); + expect( + harness.container + .read(chatProvider(harness.charId)) + .requireValue + .isGenerating, + isTrue, + ); + + harness.memoryResult.complete( + initialDraft.copyWith( + content: 'MEMORY_MARKER', + keys: ['memory-key'], + generatedAt: 3, + updatedAt: 3, + ), + ); + var memoryDone = false; + unawaited(memoryFuture.whenComplete(() => memoryDone = true)); + await pumpUntil(tester, () => memoryDone, 'memory generation completion'); + await memoryFuture; + + final memoryCompletedBook = await harness.container + .read(memoryBookRepoProvider) + .getBySessionId(harness.sessionId); + expect( + memoryCompletedBook!.pendingDrafts.single.content, + 'MEMORY_MARKER', + ); + + harness.chatService.result.complete( + ChatState( + session: chatSession.copyWith( + messages: [ + ...chatSession.messages, + const ChatMessage( + id: 'chat-result', + role: 'assistant', + content: 'CHAT_MARKER', + timestamp: 2, + isError: true, + ), + ], + ), + isGenerating: false, + ), + ); + await pumpUntilPersisted(tester, () async { + final session = await harness.container + .read(chatRepoProvider) + .getById(harness.sessionId); + return session?.messages.lastOrNull?.content == 'CHAT_MARKER'; + }, 'chat result persistence'); + await chatFuture; + + final persistedChat = await harness.container + .read(chatRepoProvider) + .getById(harness.sessionId); + final persistedBook = await harness.container + .read(memoryBookRepoProvider) + .getBySessionId(harness.sessionId); + expect(persistedChat!.messages.last.content, 'CHAT_MARKER'); + expect( + persistedChat.messages.map((message) => message.content), + isNot(contains('MEMORY_MARKER')), + ); + expect(persistedBook!.pendingDrafts.single.content, 'MEMORY_MARKER'); + expect(persistedBook.pendingDrafts.single.keys, ['memory-key']); + expect( + persistedBook.pendingDrafts.single.content, + isNot(contains('CHAT_MARKER')), + ); + }, + ); + + testWidgets('same draft cannot start twice while its request is active', ( + tester, + ) async { + final harness = await createHarness(tester); + final first = startMemory(harness); + await pumpUntil( + tester, + () => harness.memoryStarted.isCompleted, + 'memory generation start', + ); + final second = startMemory(harness); + + await second; + expect(harness.memoryCalls(), 1); + expect(harness.memoryController.isDraftGenerating(initialDraft.id), isTrue); + + harness.memoryResult.complete( + initialDraft.copyWith(content: 'ONLY_RESULT', updatedAt: 6), + ); + var memoryDone = false; + unawaited(first.whenComplete(() => memoryDone = true)); + await pumpUntil(tester, () => memoryDone, 'memory generation completion'); + await first; + + final persistedBook = await harness.container + .read(memoryBookRepoProvider) + .getBySessionId(harness.sessionId); + expect(persistedBook!.pendingDrafts.single.content, 'ONLY_RESULT'); + }); +} diff --git a/test/notification_navigation_test.dart b/test/notification_navigation_test.dart new file mode 100644 index 00000000..f3aca536 --- /dev/null +++ b/test/notification_navigation_test.dart @@ -0,0 +1,81 @@ +import 'package:drift/native.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:go_router/go_router.dart'; + +import 'package:glaze_flutter/core/db/app_db.dart'; +import 'package:glaze_flutter/core/models/chat_message.dart'; +import 'package:glaze_flutter/core/navigation/router.dart'; +import 'package:glaze_flutter/core/services/generation_notification_service.dart'; +import 'package:glaze_flutter/core/state/db_provider.dart'; + +import 'helpers/pump_glaze_app.dart'; + +void main() { + late AppDatabase db; + late ProviderContainer container; + Uri? matchedUri; + + setUpAll(initLocalizationOnce); + + setUp(() { + db = AppDatabase.forTesting(NativeDatabase.memory()); + final router = GoRouter( + initialLocation: '/', + routes: [ + GoRoute(path: '/', builder: (_, _) => const SizedBox()), + GoRoute( + path: '/chat/:charId', + builder: (_, state) { + matchedUri = state.uri; + return const SizedBox(); + }, + ), + ], + ); + container = ProviderContainer( + overrides: [ + appDbProvider.overrideWithValue(db), + routerProvider.overrideWithValue(router), + ], + ); + }); + + tearDown(() async { + container.dispose(); + await db.close(); + }); + + testWidgets('notification opens its session and target message', ( + tester, + ) async { + await container + .read(chatRepoProvider) + .put( + const ChatSession( + id: 'notification-session', + characterId: 'notification-character', + sessionIndex: 7, + ), + ); + + await pumpGlazeApp( + tester, + container: container, + notificationForTesting: const NotificationNavigationData( + charId: 'notification-character', + sessionId: 'notification-session', + msgId: 'notification-message', + ), + ); + await pumpNavigation(tester); + + final location = matchedUri; + expect(location, isNotNull); + expect(location!.path, '/chat/notification-character'); + expect(location.queryParameters['session'], '7'); + expect(location.queryParameters['msg'], 'notification-message'); + expect(tester.takeException(), isNull); + }); +} diff --git a/test/trigger_generation_test.dart b/test/trigger_generation_test.dart index fd29955b..9ff062f3 100644 --- a/test/trigger_generation_test.dart +++ b/test/trigger_generation_test.dart @@ -152,7 +152,7 @@ void main() { expect((result as TriggerBusy).busyKind, 'chat'); }); - test('rejects with TriggerBusy when a memory draft is active', () async { + test('accepts while a memory draft is active', () async { final state = ChatState( session: ChatSession( id: 's1', @@ -169,8 +169,10 @@ void main() { final dispatcher = container.read(generationDispatcherProvider); final result = await dispatcher.dispatch(charId: 'c1'); - expect(result, isA()); - expect((result as TriggerBusy).busyKind, 'memory_draft'); + expect(result, isA()); + final notifier = container.read(chatProvider('c1').notifier); + expect((notifier as _MockChatNotifier).calls, ['regenerate']); + expect(dispatcher.peekResolvedMode(charId: 'c1'), TriggerMode.regenerate); }); test('rejects with TriggerBusy while a message is being edited', () async {