Fix tasks disappearing on Dashboard click (empty API guard)

[kodjima33]

Summary

• Root cause: Rust backend's get_action_items Firestore query silently swallowed errors (break instead of return Err), returning Ok(vec![]). The handler's HTTP 500 fix (commit 35a8d4d) was bypassed because it only caught Err results.
• When user clicked Dashboard, refreshTasksIfNeeded() and reconcileWithAPIIfNeeded() ran reconciliation against the empty response, hard-deleting all synced tasks from SQLite.
• Same break pattern existed in get_memories_filtered.

Changes

1. Rust backend (firestore.rs): Changed break to return Err(...) on Firestore query failures in get_action_items and get_memories_filtered, so errors propagate to handlers and result in HTTP 500
2. Swift (TasksStore.swift): Added !response.items.isEmpty guard to refreshTasksIfNeeded() and allApiIds.isEmpty guard to reconcileWithAPIIfNeeded() — matches existing guard in forceReconcileOnLoad()
3. Swift (ActionItemStorage.swift): Added empty-set guards to hardDeleteAbsentTasks() and markAbsentTasksAsStaged() as last line of defense

Test plan

• Swift app builds clean
• Rust backend compiles clean
• Verified affected user (Salman, UID 6HbrDL1WZ4PYqwa6O7N4uWT0nPj1) has 17 incomplete tasks in Firestore and macOS FCM token
• After release, verify Salman's tasks persist across Dashboard navigation

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR fixes a data-loss bug where switching to the Dashboard caused all locally-cached tasks to be silently hard-deleted. The root cause was a Rust Firestore pagination loop that breaked on query failures (returning Ok(vec![])) instead of propagating the error, causing the Swift client to reconcile against an empty API response and wipe all synced SQLite records.

Key changes:

• Rust firestore.rs: Changed break to return Err(...) on Firestore HTTP failures in both get_action_items and get_memories_filtered, ensuring HTTP 500 is returned to the client instead of an empty 200.
• TasksStore.swift: Added !response.items.isEmpty guard in refreshTasksIfNeeded() and allApiIds.isEmpty guard in reconcileWithAPIIfNeeded() to skip destructive reconciliation on empty API responses.
• ActionItemStorage.swift: Added identical empty-set guards to hardDeleteAbsentTasks() and markAbsentTasksAsStaged() as a last line of defense.

The Rust fix is the correct primary repair; the Swift guards are defense-in-depth for future regressions. One minor edge case to be aware of: the Swift empty guards also fire when a user genuinely has 0 incomplete tasks (all completed/deleted on mobile), preventing stale SQLite records from being cleaned up until the user adds a new task.

Confidence Score: 4/5

• Safe to merge — the root-cause Rust fix is correct and the Swift guards are sound defensive layers with one known edge case for users with 0 tasks.
• The primary fix (Rust return Err instead of break) is precisely targeted and correct. The Swift guards add appropriate defense-in-depth. The only concern is that the empty guards are over-broad — they also suppress legitimate reconciliation for users with 0 genuine incomplete tasks, meaning stale SQLite records can persist for that cohort until a new task is created. This is a minor behavioral regression but not a data-loss risk.
• No files require special attention, but reviewers should note the edge case in TasksStore.swift reconcileWithAPIIfNeeded() where allApiIds.isEmpty can arise from legitimate 0-task state, not just errors.

Important Files Changed

Filename	Overview
desktop/Backend-Rust/src/services/firestore.rs	Root-cause fix: replaced break (silent swallow) with return Err(...) on Firestore HTTP failure in both get_memories_filtered and get_action_items pagination loops, ensuring errors propagate to HTTP handlers as 500s instead of returning an empty Ok(vec![]).
desktop/Desktop/Sources/Stores/TasksStore.swift	Added two empty-response guards: one in refreshTasksIfNeeded() preventing reconciliation on an empty incomplete-task page, and one in reconcileWithAPIIfNeeded() skipping full pagination-based reconciliation when allApiIds is empty. Correctly does not update lastReconciliationDate on skip so retries still occur. Edge case: guard also fires for users with legitimately 0 incomplete tasks, preventing stale SQLite records from ever being cleaned up.
desktop/Desktop/Sources/Rewind/Core/ActionItemStorage.swift	Last-line-of-defense empty-set guards added to hardDeleteAbsentTasks and markAbsentTasksAsStaged. Guards are logically correct. Pre-existing issue: markAbsentTasksAsStaged performs a hard delete (not a staged delete) and lacks the backendSynced == true filter present in hardDeleteAbsentTasks, creating a potential footgun for locally-created unsynced tasks.
desktop/CHANGELOG.json	Changelog entry added for the tasks disappearing fix. No issues.

Sequence Diagram

sequenceDiagram
    participant UI as Dashboard (Swift)
    participant TS as TasksStore
    participant API as Rust Backend
    participant FS as Firestore
    participant DB as Local SQLite

    UI->>TS: onAppear / page activation
    TS->>TS: refreshTasksIfNeeded()
    TS->>API: GET /action_items (incomplete)
    API->>FS: runQuery (action_items)

    alt BEFORE FIX — Firestore error
        FS-->>API: HTTP 4xx/5xx
        API-->>API: tracing::error + break loop
        API-->>TS: HTTP 200, items: []
        TS->>DB: hardDeleteAbsentTasks(apiIds: {})
        DB-->>DB: DELETE all synced incomplete tasks 💥
        TS-->>UI: incompleteTasks = []
    else AFTER FIX — Firestore error
        FS-->>API: HTTP 4xx/5xx
        API-->>API: tracing::error + return Err(...)
        API-->>TS: HTTP 500
        TS-->>TS: catch block — log error, skip reconcile ✅
        TS-->>UI: incompleteTasks unchanged
    else AFTER FIX — Success, user has tasks
        FS-->>API: HTTP 200, documents
        API-->>TS: HTTP 200, items: [...]
        TS->>DB: syncTaskActionItems(items)
        TS->>DB: hardDeleteAbsentTasks(apiIds: {...}) if !empty
        DB-->>TS: deleted count
        TS-->>UI: incompleteTasks updated
    else AFTER FIX — Success, 0 tasks (edge case)
        FS-->>API: HTTP 200, 0 documents
        API-->>TS: HTTP 200, items: []
        TS->>TS: empty guard fires — skip reconcile ⚠️
        Note over TS,DB: Stale SQLite records not cleaned up
        TS-->>UI: incompleteTasks unchanged
    end

Loading

_{Last reviewed commit: "fix(desktop): add em..."}

[greptile-apps]

Empty guard silently breaks reconciliation for users with 0 legitimate tasks

The guard correctly defends against a stale backend error returning an empty 200. However, now that the Rust fix properly propagates failures as Err (returning HTTP 500), an empty 200 is also the correct response for a user who has genuinely deleted or completed all their tasks on mobile.

For that user, allApiIds will be empty, the guard fires, lastReconciliationDate is not updated (because the function returns early), so reconciliation is retried every 5 minutes — but it always hits the guard, meaning stale SQLite records (incomplete tasks that were completed/deleted on mobile) will persist in local storage and remain visible in the UI indefinitely.

The root fix (Rust return Err) is the right primary mechanism; the Swift guard is intended as a fallback for future regressions. Consider adding a warning log that makes the distinction explicit, or check whether this is actually a user with 0 legitimate tasks vs. a potential error — for example by checking lastReconciliationDate == nil (first run) before skipping:

// Safety guard: skip if API returned zero task IDs (possible backend error / empty 200).
// Note: also skips for users who genuinely have 0 incomplete tasks; stale local records
// will not be cleaned up in that case until they add a new task.
if allApiIds.isEmpty {
    log("TasksStore: Periodic reconciliation skipped — API returned zero task IDs (possible backend error or user has 0 tasks)")
    return
}

The existing comment on line 345 implies this is only for errors, but it also silently applies to legitimate empty states.

[greptile-apps]

markAbsentTasksAsStaged actually hard-deletes records — misleading function name

This is pre-existing code, but the guard added by this PR draws attention to it. The function is named markAbsentTasksAsStaged (suggesting a soft/staged deletion), yet its implementation calls try record.delete(database) — a hard delete. Additionally, unlike hardDeleteAbsentTasks, this function does not filter by backendSynced == true, meaning it can delete locally-created tasks that have never been pushed to the backend.

Since this PR adds an identical empty-set guard to both functions, clarifying the naming and the backendSynced gap would reduce confusion about which callers should use which function.