Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/changelog/unreleased/task-indent-persistence.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"change": "Indented sub-tasks and reordered tasks keep their position when the list refreshes, and a failed save is retried instead of being lost."}
77 changes: 31 additions & 46 deletions app/lib/providers/action_items_provider.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import 'package:omi/utils/logger.dart';
import 'package:omi/utils/platform/platform_service.dart';
import 'package:omi/utils/analytics/product_telemetry.dart';
import 'package:omi/ui/feedback/omi_feedback.dart';
import 'package:omi/providers/task_layout_writes.dart';
import 'package:omi/providers/task_suggestions_controller.dart';

typedef ActionItemsFetcher = Future<ActionItemsResponse?> Function({
Expand Down Expand Up @@ -57,6 +58,7 @@ class ActionItemsProvider extends ChangeNotifier {
BulkDeleteActionItemsRequest? bulkDeleteActionItemsRequest,
CreateActionItemRequest? createActionItemRequest,
UpdateDueDateRequest? updateDueDateRequest,
BatchUpdateActionItemsRequest? batchUpdateActionItemsRequest,
api.ActionItemsApi? actionItemsApi,
TaskSuggestionsApi? taskSuggestionsApi,
AutomaticAppleRemindersExport? automaticAppleRemindersExport,
Expand All @@ -67,6 +69,7 @@ class ActionItemsProvider extends ChangeNotifier {
_bulkDeleteActionItemsRequest = bulkDeleteActionItemsRequest ?? api.bulkDeleteActionItems,
_createActionItemRequest = createActionItemRequest ?? api.createActionItem,
_updateDueDateRequest = updateDueDateRequest ?? api.updateActionItem,
_layoutWrites = TaskLayoutWrites(batchUpdateActionItemsRequest ?? api.batchUpdateActionItems),
_actionItemsApi = actionItemsApi,
_automaticAppleRemindersExport = automaticAppleRemindersExport ?? AutomaticAppleRemindersExport(),
_exportActionItem = exportActionItem ?? ActionItemExportService.export {
Expand All @@ -83,6 +86,7 @@ class ActionItemsProvider extends ChangeNotifier {
final DeleteActionItemRequest _deleteActionItemRequest;
final UpdateActionItemRequest _updateActionItemRequest;
final BulkDeleteActionItemsRequest _bulkDeleteActionItemsRequest;
final TaskLayoutWrites _layoutWrites;
final CreateActionItemRequest _createActionItemRequest;
final UpdateDueDateRequest _updateDueDateRequest;
final api.ActionItemsApi? _actionItemsApi;
Expand Down Expand Up @@ -123,12 +127,6 @@ class ActionItemsProvider extends ChangeNotifier {
DateTime? _lastRefreshTime;
static const Duration _refreshCooldown = Duration(seconds: 30);

// Debounced persistence for sort order and indent level
final Map<String, int> _pendingSortUpdates = {};
final Map<String, int> _pendingIndentUpdates = {};
Timer? _sortDebounce;
Timer? _indentDebounce;

// Multi-selection state
bool _isSelectionMode = false;
Set<String> _selectedItems = {};
Expand Down Expand Up @@ -307,6 +305,7 @@ class ActionItemsProvider extends ChangeNotifier {
final writeRevision = _taskWriteRevision;
final readOrdinal = ++_listReadOrdinal;
_activeListReads.add(readOrdinal);
final layoutMark = _layoutWrites.readMark;
final startOfTomorrow = DateTime(now.year, now.month, now.day + 1);
final sevenDaysAgo = now.subtract(const Duration(days: 7));
try {
Expand All @@ -318,11 +317,13 @@ class ActionItemsProvider extends ChangeNotifier {
dueEndDate: startOfTomorrow.subtract(const Duration(microseconds: 1)),
);
if (response != null && generation == _sessionGeneration && writeRevision == _taskWriteRevision) {
_homeDayItems = _pendingDeletionIds.isEmpty
? List.of(response.actionItems)
: response.actionItems.where((item) => !_pendingDeletionIds.contains(item.id)).toList();
_homeDayItems = response.actionItems
.where((item) => !_pendingDeletionIds.contains(item.id))
.map(_layoutWrites.overlayFor(layoutMark))
.toList();
SiriIntegration.current.queueUpsertTasks(_homeDayItems);
_homeDayLoaded = true;
_layoutWrites.retryPending();
}
} catch (e) {
Logger.debug('Error fetching home today tasks: $e');
Expand Down Expand Up @@ -414,6 +415,7 @@ class ActionItemsProvider extends ChangeNotifier {
final readOrdinal = ++_listReadOrdinal;
_latestTaskListRead = readOrdinal;
_activeListReads.add(readOrdinal);
final layoutMark = _layoutWrites.readMark;
final query = (includeCompleted: _includeCompleted, startDate: _startDate, endDate: _endDate);
bool ownsQuery() =>
!_disposed &&
Expand Down Expand Up @@ -448,6 +450,7 @@ class ActionItemsProvider extends ChangeNotifier {
response,
decodedPageIsComplete: decodedPageIsComplete,
readOrdinal: readOrdinal,
layoutMark: layoutMark,
);
loaded = true;
}
Expand Down Expand Up @@ -491,6 +494,7 @@ class ActionItemsProvider extends ChangeNotifier {
ActionItemsResponse response, {
required bool decodedPageIsComplete,
required int readOrdinal,
required int layoutMark,
}) async {
// Snapshot server IDs before filtering so tombstone retirement is
// based on the full server response, not the filtered subset.
Expand All @@ -499,7 +503,9 @@ class ActionItemsProvider extends ChangeNotifier {
// in-place: the response list may be unmodifiable, and aliasing it
// would cause removeWhere to throw in deleteActionItem/deleteSelectedItems
// and retire tombstones prematurely.
_actionItems = TaskCategorizationList(response.actionItems.where((item) => !_pendingDeletionIds.contains(item.id)));
_actionItems = TaskCategorizationList(response.actionItems
.where((item) => !_pendingDeletionIds.contains(item.id))
.map(_layoutWrites.overlayFor(layoutMark)));
_loadedTaskPageIds
..clear()
..addAll(serverIds);
Expand Down Expand Up @@ -539,6 +545,7 @@ class ActionItemsProvider extends ChangeNotifier {
_showCompletedView = true;
_completedViewWasAutoRevealed = true;
}
_layoutWrites.retryPending();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Retry pending writes after every successful task read

When a failed layout write is pending and the next successful task request is pagination via loadMoreActionItems or a saved-task point read via receiveSavedTask, neither path reaches this sole retryPending() call even though both now apply overlay(). The edit therefore remains only local unless the user later makes another edit or performs a first-page fetch; invoke the retry hook from each successful read path so restored connectivity actually persists the pending layout.

Useful? React with 👍 / 👎.

}

void _revealNewIncompleteTasks() {
Expand All @@ -555,6 +562,7 @@ class ActionItemsProvider extends ChangeNotifier {
final writeRevision = _taskWriteRevision;
final readOrdinal = ++_listReadOrdinal;
_activeListReads.add(readOrdinal);
final layoutMark = _layoutWrites.readMark;

setFetching(true);

Expand Down Expand Up @@ -585,9 +593,11 @@ class ActionItemsProvider extends ChangeNotifier {
final seenIds = _actionItems.map((item) => item.id).toSet();
final filtered = response.actionItems
.where((item) => !_pendingDeletionIds.contains(item.id) && seenIds.add(item.id))
.map(_layoutWrites.overlayFor(layoutMark))
.toList();
_actionItems.addAll(filtered);
_revealNewIncompleteTasks();
_layoutWrites.retryPending();
_hasMore = response.hasMore;
if (!_hasMore &&
!hasActiveFilter &&
Expand Down Expand Up @@ -1003,7 +1013,7 @@ class ActionItemsProvider extends ChangeNotifier {

if (newItem != null) {
_actionItems.removeWhere((item) => item.id == optimisticItem.id);
_receiveCanonicalTask(newItem);
_receiveCanonicalTask(newItem, _layoutWrites.readMark);
unawaited(() async {
final exported = await _automaticAppleRemindersExport.export(newItem,
canContinue: () => !_disposed && generation == _sessionGeneration);
Expand Down Expand Up @@ -1033,17 +1043,20 @@ class ActionItemsProvider extends ChangeNotifier {
final generation = _sessionGeneration;
final typedApi = _actionItemsApi;
if (typedApi == null || _disposed) return false;
final layoutMark = _layoutWrites.readMark;
final result = await typedApi.getById(id);
if (_disposed || generation != _sessionGeneration) return false;
if (result case ApiSuccess(:final data)) {
if (data.id != id) return false;
_receiveCanonicalTask(data);
_receiveCanonicalTask(data, layoutMark);
_layoutWrites.retryPending();
return true;
}
return false;
}

void _receiveCanonicalTask(ActionItemWithMetadata item) {
void _receiveCanonicalTask(ActionItemWithMetadata item, int layoutMark) {
item = _layoutWrites.overlayFor(layoutMark)(item);
_taskWriteRevision++;
if (_pendingDeletionIds.contains(item.id)) return;
// An explicit Add Task gesture should reveal its new incomplete task,
Expand Down Expand Up @@ -1098,46 +1111,22 @@ class ActionItemsProvider extends ChangeNotifier {

void updateItemSortOrder(String id, int sortOrder) {
_updateItemInPlace(id, sortOrder: sortOrder);
_pendingSortUpdates[id] = sortOrder;
_layoutWrites.setSortOrder(id, sortOrder);
notifyListeners();
_sortDebounce?.cancel();
_sortDebounce = Timer(const Duration(milliseconds: 500), _flushSortUpdates);
}

void updateItemIndentLevel(String id, int indentLevel) {
_updateItemInPlace(id, indentLevel: indentLevel);
_pendingIndentUpdates[id] = indentLevel;
_layoutWrites.setIndentLevel(id, indentLevel);
notifyListeners();
_indentDebounce?.cancel();
_indentDebounce = Timer(const Duration(milliseconds: 500), _flushIndentUpdates);
}

void batchUpdateSortOrders(Map<String, int> updates) {
for (final entry in updates.entries) {
_updateItemInPlace(entry.key, sortOrder: entry.value);
_pendingSortUpdates[entry.key] = entry.value;
_layoutWrites.setSortOrder(entry.key, entry.value);
}
notifyListeners();
_sortDebounce?.cancel();
_sortDebounce = Timer(const Duration(milliseconds: 500), _flushSortUpdates);
}

Future<void> _flushSortUpdates() async {
if (_pendingSortUpdates.isEmpty) return;
final updates = Map<String, int>.from(_pendingSortUpdates);
_pendingSortUpdates.clear();

final items = updates.entries.map((e) => {'id': e.key, 'sort_order': e.value}).toList();
await api.batchUpdateActionItems(items);
}

Future<void> _flushIndentUpdates() async {
if (_pendingIndentUpdates.isEmpty) return;
final updates = Map<String, int>.from(_pendingIndentUpdates);
_pendingIndentUpdates.clear();

final items = updates.entries.map((e) => {'id': e.key, 'indent_level': e.value}).toList();
await api.batchUpdateActionItems(items);
}

ActionItemWithMetadata? _findAndUpdateItemState(String itemId, bool newState) {
Expand Down Expand Up @@ -1324,8 +1313,7 @@ class ActionItemsProvider extends ChangeNotifier {
_pendingDeletionIds.clear();
_deletionAcknowledgedAtRead.clear();
_activeListReads.clear();
_pendingSortUpdates.clear();
_pendingIndentUpdates.clear();
_layoutWrites.clear();
suggestions?.reset();
notifyListeners();
}
Expand Down Expand Up @@ -1524,10 +1512,7 @@ class ActionItemsProvider extends ChangeNotifier {
suggestions?.removeListener(notifyListeners);
suggestions?.dispose();
_refreshDebounceTimer?.cancel();
_sortDebounce?.cancel();
_indentDebounce?.cancel();
_flushSortUpdates();
_flushIndentUpdates();
_layoutWrites.flush();
super.dispose();
}
}
135 changes: 135 additions & 0 deletions app/lib/providers/task_layout_writes.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
import 'dart:async';

import 'package:omi/backend/schema/schema.dart';
import 'package:omi/utils/logger.dart';

// Owns debounced persistence of task sort order and indent level.
//
// An edit stays pending until the server acknowledges that exact value, so a
// list read cannot revert it on screen and a failed write is retried. One
// write is in flight at a time, so an older layout cannot land after a newer
// one. Acknowledged values are still overlaid on reads that started before the
// acknowledgement, because their server snapshot may predate the write.

typedef BatchUpdateActionItemsRequest = Future<bool> Function(List<Map<String, dynamic>> items);

class TaskLayoutWrites {
TaskLayoutWrites(this._send);

final BatchUpdateActionItemsRequest _send;
final Map<String, int> _sortOrders = {};
final Map<String, int> _indentLevels = {};
// ponytail: kept for the session (one entry per edited task); prune by in-flight read marks if this ever grows.
final Map<String, ({int value, int ack})> _ackedSortOrders = {};
final Map<String, ({int value, int ack})> _ackedIndentLevels = {};
int _acks = 0;
Timer? _debounce;
Future<void>? _flush;
bool _flushRequestedDuringSend = false;
int _generation = 0;

/// Capture before a task read starts; pass to [overlayFor] for its rows.
int get readMark => _acks;

void setSortOrder(String id, int sortOrder) {
_sortOrders[id] = sortOrder;
_schedule();
}

void setIndentLevel(String id, int indentLevel) {
_indentLevels[id] = indentLevel;
_schedule();
}

/// Re-applies unacknowledged edits, and edits acknowledged after [readMark],
/// to rows from a read that started at [readMark].
ActionItemWithMetadata Function(ActionItemWithMetadata) overlayFor(int readMark) => (item) {
final ackedSort = _ackedSortOrders[item.id];
final ackedIndent = _ackedIndentLevels[item.id];
final sortOrder =
_sortOrders[item.id] ?? (ackedSort != null && ackedSort.ack > readMark ? ackedSort.value : null);
final indentLevel =
_indentLevels[item.id] ?? (ackedIndent != null && ackedIndent.ack > readMark ? ackedIndent.value : null);
if (sortOrder == null && indentLevel == null) return item;
return item.copyWith(sortOrder: sortOrder, indentLevel: indentLevel);
};

/// Called after any successful task read: the server is reachable again.
void retryPending() {
if (_sortOrders.isEmpty && _indentLevels.isEmpty) return;
// A send in flight may still fail; remember that the server answered meanwhile.
if (_flush != null) {
_flushRequestedDuringSend = true;
} else {
_schedule();
}
}

/// Drops pending edits, e.g. on sign-out; an in-flight write cannot restore them.
void clear() {
_generation++;
_debounce?.cancel();
_sortOrders.clear();
_indentLevels.clear();
_ackedSortOrders.clear();
_ackedIndentLevels.clear();
}

Future<void> flush() {
_debounce?.cancel();
if (_flush != null) {
_flushRequestedDuringSend = true;
return _flush!;
}
return _flush = _sendPending().whenComplete(() {
_flush = null;
// An edit made while the send was in flight lost its timer to this send.
// If the send failed, give that edit its own attempt.
final retry = _flushRequestedDuringSend && (_sortOrders.isNotEmpty || _indentLevels.isNotEmpty);
_flushRequestedDuringSend = false;
if (retry) _schedule();
});
}

void _schedule() {
_debounce?.cancel();
_debounce = Timer(const Duration(milliseconds: 500), flush);
}

Future<void> _sendPending() async {
final generation = _generation;
while (generation == _generation) {
final sortOrders = Map<String, int>.of(_sortOrders);
final indentLevels = Map<String, int>.of(_indentLevels);
if (sortOrders.isEmpty && indentLevels.isEmpty) return;

final items = [
for (final id in {...sortOrders.keys, ...indentLevels.keys})
{
'id': id,
if (sortOrders.containsKey(id)) 'sort_order': sortOrders[id],
if (indentLevels.containsKey(id)) 'indent_level': indentLevels[id],
},
];
bool saved;
try {
saved = await _send(items);
} catch (e) {
Logger.debug('Error saving task layout: $e');
saved = false;
}
// Failed edits stay pending; the next edit or list read retries them.
if (!saved || generation != _generation) return;
_acks++;
sortOrders.forEach((id, value) {
if (_sortOrders[id] == value) _sortOrders.remove(id);
_ackedSortOrders[id] = (value: value, ack: _acks);
});
indentLevels.forEach((id, value) {
if (_indentLevels[id] == value) _indentLevels.remove(id);
Comment on lines +125 to +129

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Fence reads that predate the layout acknowledgement

If a list request takes its server snapshot before the PATCH but completes after the PATCH succeeds, these removals clear the only overlay before that response is applied. Layout edits never advance _taskWriteRevision, so fetchActionItems still accepts the older response and _applyFetchedActionItems replaces the local row with stale order/indent values, visibly reverting an acknowledged edit until another fetch. Retain the acknowledged overlay until pre-write reads drain, or invalidate those reads when the layout write is acknowledged.

Useful? React with 👍 / 👎.

_ackedIndentLevels[id] = (value: value, ack: _acks);
});
_flushRequestedDuringSend = false;
}
}
}
Loading
Loading