fix(unread): expose removed-group-suppressed account unread total (#573)

[agent-p1p]

Closes marmot-protocol/mdk#461

Summary

Exposes a removed-group-suppressed account unread projection from Marmot, so accountUnreadSummary() / the underlying account_unread_total already excludes groups where the local account has left or been removed — without loading per-row rosters on the hot path. This lets Android retire its re-derived suppression in refreshEffectiveAccountUnreadCount(...) (the darkmatter-android#672 stopgap) and go back to consuming the authoritative summary for background-account unread dots, matching the active-account #625 semantics.

Design

Persisted self-membership flag on the cheap projection (Approach A):

• Migration 0018 adds account_groups.self_membership TEXT NOT NULL DEFAULT 'member'. The default preserves every existing row and every still-joined group — uncertainty never suppresses, only an observed self-removal flips it to 'removed'.
• account_unread_total now LEFT JOINs account_groups and excludes rows where self_membership = 'removed'. COALESCE(ag.self_membership, 'member') preserves unknown/unmatched rows (defensive for the transient FK-cascade window).
• Sync membership-change path (observe_account_device_effects) flips self_membership to 'removed' only when the removed/left subject is the local account (MLS member ids are the Nostr account pubkey hex here; compared case-insensitively). A peer removal never touches our own flag. GroupJoined/GroupCreated re-affirm 'member' so a re-add after removal un-suppresses.
• save_account_projection_state already omits self_membership from its UPSERT, so routine metadata re-projection never clobbers the sync-owned flag (proven by a regression test).

Acceptance criteria

• Account unread summary excludes frozen unread for groups where that account is no longer in the loaded roster.
• Preserves unread counts when membership/roster state is unknown rather than suppressing on uncertainty (default 'member' / COALESCE).
• Android can use the summary without loading per-row rosters for background-account unread dots.
• Regression coverage includes removed/left group unread suppression for a non-active account summary.

Verification (CI's exact commands, run locally)

• cargo fmt --all --check — clean
• RUSTFLAGS='-D warnings' cargo check --workspace --all-targets --locked (and --all-features) — pass
• cargo clippy --workspace --all-targets --locked -- -D warnings — pass
• cargo test -p storage-sqlite --locked — 137 passed (incl. suppress/preserve/re-join/orphan-row/projection-resave + migration default test)
• cargo test -p marmot-app --locked — 243 + 11 + 57 passed (incl. the new self_removal_suppresses_account_unread_while_peer_removal_preserves_it relay_runtime integration test, stress-verified 8× under serial --test-threads=1)

Sensitive paths

• crates/storage-sqlite — schema migration 0018 (additive, safe default), account_unread_total query, new set_group_self_membership storage method.
• crates/marmot-app — sync membership-change path.

No account/group/member ids, pubkeys, or payloads are logged in any added code (observability invariant preserved).

---

Summary by CodeRabbit

• Bug Fixes
  • Fixed unread totals not updating correctly after leaving or being removed from a group.
  • Peer member removals no longer suppress the observer’s unread totals; only true self-removal affects suppression.
  • Rejoining/recreating a group correctly restores unread counting.
  • Added an automatic one-time correction for legacy self-membership data on first app use to ensure accurate unread suppression.
• Tests
  • Added regression coverage for leave/remove, immediate suppression, and the one-time backfill behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9ed70e21-7365-4b81-b92f-c81eba0d6265

📥 Commits

Reviewing files that changed from the base of the PR and between 7d7bb03 and 4bd908d.

📒 Files selected for processing (9)

• crates/marmot-app/src/client/mod.rs
• crates/marmot-app/src/client/sync.rs
• crates/marmot-app/src/lib.rs
• crates/marmot-app/tests/relay_runtime.rs
• crates/storage-sqlite/src/account_projection.rs
• crates/storage-sqlite/src/chat_list.rs
• crates/storage-sqlite/src/chat_list/tests.rs
• crates/storage-sqlite/src/migrations.rs
• crates/storage-sqlite/src/migrations/0018_account_group_self_membership.rs

🚧 Files skipped from review as they are similar to previous changes (8)

• crates/storage-sqlite/src/migrations/0018_account_group_self_membership.rs
• crates/marmot-app/src/client/sync.rs
• crates/marmot-app/tests/relay_runtime.rs
• crates/storage-sqlite/src/migrations.rs
• crates/marmot-app/src/lib.rs
• crates/storage-sqlite/src/chat_list.rs
• crates/marmot-app/src/client/mod.rs
• crates/storage-sqlite/src/chat_list/tests.rs

---

Walkthrough

Adds a self_membership TEXT NOT NULL DEFAULT 'member' column to account_groups via migration 0018. The account_unread_total query gains a LEFT JOIN that filters out 'removed' rows. Group events, local leaves, and a once-only open/upgrade backfill keep the column current. Seven storage unit tests and three relay integration tests cover the new behavior.

Changes

Self-membership unread suppression

Layer / File(s)	Summary
Migration 0018: add self_membership column crates/storage-sqlite/src/migrations/0018_account_group_self_membership.rs, crates/storage-sqlite/src/migrations.rs	Defines, registers, and tests the SQLite migration that adds self_membership TEXT NOT NULL DEFAULT 'member' to account_groups, verifying existing rows receive the default.
Storage APIs: set_group_self_membership and account_group_ids_defaulting_to_member crates/storage-sqlite/src/account_projection.rs, crates/storage-sqlite/src/chat_list.rs	Adds set_group_self_membership and account_group_ids_defaulting_to_member to SqliteAccountStorage, and updates account_unread_total SQL to LEFT JOIN account_groups and exclude self_membership = 'removed' rows.
Storage-layer unit tests crates/storage-sqlite/src/chat_list/tests.rs	Adds setup_store_with_one_unread helper and seven unit tests covering suppression, preservation, unsuppression, missing-row LEFT JOIN edge case, projection re-save idempotency, candidate-set enumeration, and backend error propagation.
MarmotApp pub(crate) helpers and backfill marker crates/marmot-app/src/lib.rs	Introduces SELF_MEMBERSHIP_BACKFILL_MARKER constant and four new MarmotApp helper methods (set_group_self_membership, account_group_ids_defaulting_to_member, account_import_marker, mark_account_import_complete) delegating to storage.
Group event handler: self-membership updates on removal/join crates/marmot-app/src/client/sync.rs	Derives local_account_id_hex once in observe_account_device_effects; gates set_group_self_membership(true) on MemberRemoved/MemberLeft to the local account only; adds set_group_self_membership(false) on GroupJoined/GroupCreated, with error propagation throughout.
Local leave write and once-only backfill crates/marmot-app/src/client/mod.rs, crates/marmot-app/src/lib.rs	Adds immediate set_group_self_membership(true) in leave_group_with_audit_context; implements backfill_self_membership_once (roster-checked, marker-gated) and local_account_removed_from_roster helper; wires backfill into client_with_relay_plane construction.
Integration tests: self-removal, local leave, backfill idempotency crates/marmot-app/tests/relay_runtime.rs	Three relay runtime tests cover: Carol's unread suppressed on self-removal while Bob's is not; Bob's unread suppressed after leave_group; and unread preserved through account reopen/backfill for a still-member account.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

• Cross-account unread dot (#592) lights for removed/left groups on non-active accounts (no #625 suppression) whitenoise-android#662: This PR implements the Marmot-side fix that allows Android to consume accountUnreadSummary() without loading per-row group rosters to suppress frozen unread for removed groups.

Possibly related PRs

• marmot-protocol/darkmatter#576: Extends relay_runtime.rs with group member removal and re-add lifecycle tests that overlap with the self-membership/unread suppression scenarios added in this PR.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title concisely captures the main change: exposing a removed-group-suppressed account unread total to address issue #573.
Linked Issues check	✅ Passed	The PR comprehensively addresses all acceptance criteria from #573: implements removed-group suppression in account unread totals, preserves unread when state is unknown, enables Android to retire duplicate logic, and includes extensive regression coverage.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to implementing the removed-group-suppressed account unread projection required by #573, with no unrelated modifications detected.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch pip/darkmatter-573

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[stage-review]

Ready to review this PR? Stage has broken it down into 6 individual chapters for you:

	Title
1	Add self_membership column to account_groups
2	Update unread total query and storage methods
3	Expose membership management in MarmotApp
4	Track membership changes during sync and leave
5	Implement one-time membership backfill on open
6	Verify unread suppression with integration tests

_{Chapters generated by Stage for commit 4bd908d on Jun 24, 2026 1:04am UTC.}

[agent-p1p]

Adversarial review: changes requested.

Blocking findings: 1
Suggestions: 0
Nitpicks: 0

Blocking:

• crates/marmot-app/src/client/sync.rs:483 and crates/marmot-app/src/client/mod.rs:526-548 — self_membership is only updated from the inbound sync observer. The local self-departure path (leave_group_with_audit_context, also used by invite decline) publishes/records the leave effects, saves state, and only queues system-row projection updates; it never calls the new membership projection updater. Since remember_published_reports records our published event ids, the later own relay echo is skipped by is_own_relay_echo, so there is no second inbound pass to flip account_groups.self_membership to removed. Result: when this account leaves a group from this app, the row remains at migration/default member, and accountUnreadSummary() can continue to count frozen unread rows for a group the account left. That misses the “left group” half of Expose suppressed account unread totals from Marmot mdk#461.

Suggested fix: factor the self-membership projection into a helper that is applied to every AccountDeviceEffects path, including local send/drain paths, and explicitly mark the local leave_group_with_audit_context case as removed after successful publish if the engine does not emit a local MemberLeft event there. Add a regression where the account itself leaves a group with unread rows, then account_unread_summary() for that account returns zero without relying on per-row roster loads.

Verification run locally:

• cargo test -p storage-sqlite account_unread_total_ --locked — 7 passed
• cargo test -p marmot-app self_removal_suppresses_account_unread_while_peer_removal_preserves_it --locked — 1 passed

[agent-p1p]

Addressed the blocking review finding (local self-departure paths did not update the new self_membership projection).

Fix: leave_group_with_audit_context() now calls set_group_self_membership(removed=true) after a successful local leave, so a locally initiated leave/decline suppresses the account unread aggregate directly. This covers both leave_group and decline_group_invite (which delegates to it). Previously suppression only fired from inbound observe_account_device_effects, and the leaver's own relay echo is skipped — so a frozen unread row for the left group kept inflating the leaver's total. The call is a no-op when no account_groups row exists, so it never resurrects pruned projection state.

Regression test: added local_leave_suppresses_account_unread_total in crates/marmot-app/tests/relay_runtime.rs — bob registers an unread, leaves locally, and his account unread total must drop to 0. (The existing self_removal_suppresses_account_unread_while_peer_removal_preserves_it covers the inbound-observed path.)

Rebased onto current master and squashed to a single commit. Local verification (CI-exact commands): cargo fmt --all --check, RUSTFLAGS=-D warnings cargo check --workspace --all-targets [--all-features] --locked, cargo clippy --workspace --all-targets [--all-features] --locked -- -D warnings, cargo test -p storage-sqlite (137), cargo test -p marmot-app (243 lib + 11 audit + 59 relay_runtime), and groups_leave_publishes_self_remove 3/3 — all green.

[erskingardner]

Adversarial review: changes requested.

I found two correctness issues around the new self_membership projection. The forward-looking removal paths are covered well and the targeted tests passed locally, but the migration/backfill story still leaves pre-existing affected users wrong, and the core membership projection writes are currently allowed to fail silently.

Local verification:

• cargo test -p storage-sqlite account_unread_total_ --locked — 7 passed
• cargo test -p storage-sqlite account_group_self_membership_migration_defaults_existing_rows_to_member --locked — 1 passed
• cargo test -p marmot-app self_removal_suppresses_account_unread_while_peer_removal_preserves_it --locked — 1 passed
• cargo test -p marmot-app local_leave_suppresses_account_unread_total --locked — 1 passed