From dedc82eb6a2985a6135090f7f03743269534553d Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Wed, 11 Nov 2026 22:01:53 -0500 Subject: [PATCH 1/2] Publish the outbox's summary and its waiting work together Storing a change published the account's summary before the open strategy's waiting work included it. For that moment the outbox read as empty, the canvas was marked saved, and a redraw from the server waiting for that ran over the edit just stored. The edit still landed; the older page then replaced it on the canvas, and the next save sent the old value back. --- .../collab/strategy_op_queue_provider.dart | 42 +++------------- test/strategy_page_session_provider_test.dart | 48 +++++++++++++++++++ 2 files changed, 54 insertions(+), 36 deletions(-) diff --git a/lib/providers/collab/strategy_op_queue_provider.dart b/lib/providers/collab/strategy_op_queue_provider.dart index 1fe501bf9..9da476270 100644 --- a/lib/providers/collab/strategy_op_queue_provider.dart +++ b/lib/providers/collab/strategy_op_queue_provider.dart @@ -3675,42 +3675,12 @@ class StrategyOpQueueNotifier extends Notifier { return Uri.decodeComponent(parts[1]); } - void _publishAccountSummary() { - if (_isDisposed) return; - final summary = _accountSummary(state.accountId); - final hasDurabilityFailure = _hasDurabilityFailureForCurrentAccount; - if (_sameAccountSummary(state.accountOutbox, summary) && - state.hasDurabilityFailure == hasDurabilityFailure) { - return; - } - state = state.copyWith( - accountOutbox: summary, - hasDurabilityFailure: hasDurabilityFailure, - ); - } - - bool _sameAccountSummary( - AccountStrategyOutboxSummary left, - AccountStrategyOutboxSummary right, - ) { - if (left.accountId != right.accountId || - left.strategies.length != right.strategies.length) { - return false; - } - for (final entry in left.strategies.entries) { - final other = right.strategies[entry.key]; - if (other == null || - other.queuedCount != entry.value.queuedCount || - other.inFlightCount != entry.value.inFlightCount || - other.pausedCount != entry.value.pausedCount || - other.attentionCount != entry.value.attentionCount || - other.successorCount != entry.value.successorCount || - other.reason != entry.value.reason) { - return false; - } - } - return true; - } + /// Publishes the outbox as its records now stand: the account's summary + /// together with the open strategy's waiting work. Never one without the + /// other: a state that counted a newly stored change in the summary while + /// its waiting work still left it out read as nothing left to send, and + /// the canvas was redrawn from the server over the edit just stored. + void _publishAccountSummary() => _refreshActiveQueueView(); Future _serializeWrite(Future Function() action) { final next = _writeTail.then((_) => action()); diff --git a/test/strategy_page_session_provider_test.dart b/test/strategy_page_session_provider_test.dart index 2dfb1f4fd..12de7c260 100644 --- a/test/strategy_page_session_provider_test.dart +++ b/test/strategy_page_session_provider_test.dart @@ -9997,6 +9997,54 @@ void main() { }); }); + test( + "an edit made offline lands when a teammate's edit to another item " + 'arrived first', () async { + final page = _page('page-1', 0); + server = _FakeServer(page.publicId, elements: [ + _textElement(page.publicId, 'a', 'alpha', worldSized: true), + _textElement(page.publicId, 'c', 'charlie', + sortIndex: 1, worldSized: true), + ]) + ..mergesByField = true; + final (container, _) = await openOnRealQueue(page); + for (var i = 0; i < 10; i++) { + await _settle(); + } + + container.read(_online.notifier).state = false; + container.read(textProvider.notifier).commitText('a', 'mine'); + // The teammate's edit waits to be drawn until the user's is saved. + server.teammateEditElement('c', (data) => data['text'] = 'theirs'); + showServer(); + for (var i = 0; i < 4; i++) { + await _settle(); + } + + container.read(_online.notifier).state = true; + for (var round = 0; round < 3; round++) { + await container + .read(strategyProvider.notifier) + .forceSaveNow('cloud-strategy'); + await container.read(strategyOpQueueProvider.notifier).flushNow(); + for (var i = 0; i < 6; i++) { + await _settle(); + } + showServer(); + for (var i = 0; i < 6; i++) { + await _settle(); + } + } + + String? drawn(String id) => container + .read(textProvider) + .firstWhere((text) => text.id == id) + .text; + expect(cloudPayloadData(server.element('a').payload)['text'], 'mine'); + expect(drawn('a'), 'mine'); + expect(drawn('c'), 'theirs'); + }); + test('offline edits to a group nobody changed land on reconnect', () async { final page = _page('page-1', 0); server = _FakeServer(page.publicId, lineups: [fanIn(page.publicId)]); From 0b97921b20e576940943ae3ee0414fa5b61659a2 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Wed, 11 Nov 2026 22:15:51 -0500 Subject: [PATCH 2/2] Publish a page's stored work once, not once per item Each item stored while a page is handed to the outbox republished the whole outbox. A 2,000 item page took 7.8 s to queue; it takes 0.2 s. --- .../collab/strategy_op_queue_provider.dart | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/providers/collab/strategy_op_queue_provider.dart b/lib/providers/collab/strategy_op_queue_provider.dart index 9da476270..928d90999 100644 --- a/lib/providers/collab/strategy_op_queue_provider.dart +++ b/lib/providers/collab/strategy_op_queue_provider.dart @@ -1275,6 +1275,7 @@ class StrategyOpQueueNotifier extends Notifier { state.successorByEntityKey, ); var changed = false; + _storingMany = true; try { for (final key in keys) { if (_awaitingRemoteAdoption.contains(key)) continue; @@ -1589,9 +1590,11 @@ class StrategyOpQueueNotifier extends Notifier { changed = true; } } catch (error, stackTrace) { + _storedMany(); _recordPersistenceFailure(error, stackTrace); return; } + _storedMany(); if (!changed) return; _pruneCanvasOpIds(); final attentionMessage = _loadedAttentionMessage( @@ -3680,7 +3683,27 @@ class StrategyOpQueueNotifier extends Notifier { /// other: a state that counted a newly stored change in the summary while /// its waiting work still left it out read as nothing left to send, and /// the canvas was redrawn from the server over the edit just stored. - void _publishAccountSummary() => _refreshActiveQueueView(); + /// + /// While a page's work is being stored record by record ([_storingMany]), + /// it waits until all of it is, so a page of many items publishes once. + void _publishAccountSummary() { + if (_storingMany) { + _publishWaits = true; + return; + } + _refreshActiveQueueView(); + } + + bool _storingMany = false; + bool _publishWaits = false; + + /// Ends [_storingMany], publishing if a publish waited for it. + void _storedMany() { + _storingMany = false; + if (!_publishWaits) return; + _publishWaits = false; + _refreshActiveQueueView(); + } Future _serializeWrite(Future Function() action) { final next = _writeTail.then((_) => action());