Skip to content

feat: clean up stale announces, add PHONE NodeType and cross-link buttons#581

Merged
torlando-tech merged 5 commits intomainfrom
worktree-temporal-floating-blanket
Mar 11, 2026
Merged

feat: clean up stale announces, add PHONE NodeType and cross-link buttons#581
torlando-tech merged 5 commits intomainfrom
worktree-temporal-floating-blanket

Conversation

@torlando-tech
Copy link
Owner

@torlando-tech torlando-tech commented Feb 28, 2026

Summary

  • Stale announce cleanup: Add deleteStaleAnnounces() DAO query that removes announces with lastSeenTimestamp older than 30 days, preserving favorites and contacts (across all identities). Triggered once per service lifecycle in ReticulumService.onCreate()
  • PHONE NodeType: Replace call.audio aspect with lxst.telephony, add NodeType.PHONE enum variant, register Python announce handler for lxst.telephony
  • Cross-link buttons: On announce detail screen, show "View messaging destination" / "View telephony destination" buttons when a peer has both lxmf.delivery and lxst.telephony announces (matched by identity hash)
  • PHONE badge: Red badge distinguishing telephony destinations from regular nodes in the announce list
  • Flaky test fix: Move Dispatchers.resetMain() before viewModelScope.cancel() in OfflineMapDownloadViewModelTest

Test plan

  • AnnounceDaoTest — 6 new tests for stale cleanup (old deletion, recent/favorite/contact preservation, empty table, correct count)
  • NodeTypeDetectorTest — updated for lxst.telephonyPHONE
  • AnnounceDetailScreenTest — updated for onViewAnnounce and getLinkedAnnouncesFlow
  • :app:compileNoSentryDebugKotlin builds clean
  • Manual: verify announces older than 30 days are cleaned on service start
  • Manual: verify favorited/contact announces survive cleanup
  • Manual: verify cross-link buttons appear for peers with both telephony and messaging announces
  • Manual: verify PHONE badge renders red in announce list

Closes #273

🤖 Generated with Claude Code

@sentry
Copy link
Contributor

sentry bot commented Feb 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 28, 2026

Greptile Summary

This PR introduces four coordinated features: stale announce cleanup (DAO query + service lifecycle trigger), a new PHONE node type replacing the old call.audio detection, cross-link navigation buttons between telephony and messaging destinations for the same peer identity, and a fix for a flaky dispatcher-reset ordering bug in tests.

The overall implementation is solid — the reactive getLinkedAnnouncesFlow (via flatMapLatest) correctly addresses the previous non-reactive concern, the deleteStaleAnnounces SQL correctly excludes favorites and contact-joined rows, and the PHONE badge now uses colorScheme.error (red) as intended.

Key issues found:

  • Dead code in AnnounceRepository: Both deleteStaleAnnounces(maxAgeMillis) and the suspend getLinkedAnnounces() methods were added to the repository but are never called. ServicePersistenceManager calls announceDao.deleteStaleAnnounces() directly, which is consistent with its existing pattern but leaves the repository wrappers as unused code.

  • call.audio backward-compatibility regression: Existing DB rows with aspect = "call.audio" (present after a user upgrades before the 30-day stale cleanup expires them) will now fall through to OtherBadge() in the announce list. Additionally, the showAudio filter logic now uses nodeType == "PHONE" rather than aspect == "call.audio", so old call.audio rows (nodeType = "NODE") will bypass the audio toggle and show whenever NODE is selected — a subtle filter behavior change.

Confidence Score: 4/5

  • Safe to merge with minor dead code and a transitional UX regression for call.audio DB rows.
  • The core logic is correct and well-tested (6 new DAO tests, updated UI tests). No runtime crashes introduced. The two issues are: unused repository methods (no behavioral impact) and a cosmetic badge regression for existing call.audio announces that will self-resolve within the 30-day TTL. No data loss or security concerns.
  • data/src/main/java/com/lxmf/messenger/data/repository/AnnounceRepository.kt (dead code) and app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt (call.audio badge/filter regression).

Important Files Changed

Filename Overview
data/src/main/java/com/lxmf/messenger/data/db/dao/AnnounceDao.kt Adds deleteStaleAnnounces(cutoffTime) DELETE query (returns row count) and two variants of getAnnouncesByIdentityHashExcluding (suspend + reactive Flow) for cross-linking. SQL logic is correct; favorites and contacts subquery exclusion is well-formed.
data/src/main/java/com/lxmf/messenger/data/repository/AnnounceRepository.kt Adds getLinkedAnnouncesFlow, getLinkedAnnounces, and deleteStaleAnnounces wrappers. The suspend getLinkedAnnounces and deleteStaleAnnounces are never called from production code — dead code added in this PR.
app/src/main/java/com/lxmf/messenger/service/persistence/ServicePersistenceManager.kt Adds cleanupStaleAnnounces() that calls the DAO directly (consistent with this class's existing pattern). The 30-day TTL constant is clear. Error handling and fire-and-forget launch are appropriate for a service lifecycle callback.
app/src/main/java/com/lxmf/messenger/viewmodel/AnnounceStreamViewModel.kt Audio-filter logic correctly updated to use NodeType.PHONE. New getLinkedAnnouncesFlow uses flatMapLatest over the reactive announce flow, making cross-link detection fully reactive. FQN kotlinx.coroutines.flow.flowOf could be imported but is harmless.
app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceDetailScreen.kt Cross-link buttons added with correct aspect-based matching for both telephony→messaging and messaging→telephony directions. UI layout (56dp height, fillMaxWidth, RoundedCornerShape) is consistent with existing buttons in the screen.
app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt Removes call.audio from badge switch and adds lxst.telephony. Existing DB rows with call.audio aspect will fall through to OtherBadge(). The error("PHONE filtered above") exhaustiveness guard is correct but unexpected at runtime.

Sequence Diagram

sequenceDiagram
    participant RS as ReticulumService.onCreate()
    participant SPM as ServicePersistenceManager
    participant DAO as AnnounceDao
    participant DB as SQLite (announces)

    RS->>SPM: cleanupStaleAnnounces()
    SPM->>SPM: cutoffTime = now - 30 days
    SPM->>DAO: deleteStaleAnnounces(cutoffTime)
    DAO->>DB: DELETE WHERE lastSeenTimestamp < cutoff\nAND isFavorite = 0\nAND hash NOT IN contacts
    DB-->>DAO: deleted row count
    DAO-->>SPM: Int
    SPM->>SPM: Log.d if deleted > 0

    participant ADS as AnnounceDetailScreen
    participant VM as AnnounceStreamViewModel
    participant AR as AnnounceRepository

    ADS->>VM: getLinkedAnnouncesFlow(destinationHash)
    VM->>AR: getAnnounceFlow(destinationHash)
    AR-->>VM: Flow<Announce?>
    VM->>VM: flatMapLatest: computeIdentityHash(announce.publicKey)
    VM->>AR: getLinkedAnnouncesFlow(identityHash, excludeHash)
    AR->>DAO: getAnnouncesByIdentityHashExcludingFlow(identityHash, excludeHash)
    DAO-->>AR: Flow<List<AnnounceEntity>>
    AR-->>VM: Flow<List<Announce>>
    VM-->>ADS: Flow<List<Announce>>
    ADS->>ADS: Show "View messaging destination" or\n"View telephony destination" button
Loading

Comments Outside Diff (1)

  1. app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt, line 292-297 (link)

    call.audio announces silently degrade to OtherBadge after upgrade

    Any user upgrading with existing call.audio announces still in their database (not yet expired by the 30-day stale cleanup) will now see OtherBadge() for those entries instead of a meaningful badge. call.audio no longer matches any branch in the when expression, so it falls through to else -> OtherBadge().

    Similarly, in AnnounceStreamViewModel, the isAudioAnnounce check now uses nodeType == NodeType.PHONE.name instead of aspect == "call.audio". Because existing DB rows have nodeType = "NODE" (set by the old detector), they'll be treated as regular nodes and bypass the showAudio toggle, potentially appearing in results even when the user has disabled audio/telephony filtering.

    Consider adding "call.audio" back as an alias pointing to NodeTypeBadge until the stale-cleanup cycle (30 days) clears those rows:

    "lxmf.delivery", "lxmf.propagation", "nomadnetwork.node", "lxst.telephony", "call.audio", null -> {
        NodeTypeBadge(nodeType = announce.nodeType)
    }
    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: app/src/main/java/com/lxmf/messenger/ui/screens/AnnounceStreamScreen.kt
    Line: 292-297
    
    Comment:
    **`call.audio` announces silently degrade to `OtherBadge` after upgrade**
    
    Any user upgrading with existing `call.audio` announces still in their database (not yet expired by the 30-day stale cleanup) will now see `OtherBadge()` for those entries instead of a meaningful badge. `call.audio` no longer matches any branch in the `when` expression, so it falls through to `else -> OtherBadge()`.
    
    Similarly, in `AnnounceStreamViewModel`, the `isAudioAnnounce` check now uses `nodeType == NodeType.PHONE.name` instead of `aspect == "call.audio"`. Because existing DB rows have `nodeType = "NODE"` (set by the old detector), they'll be treated as regular nodes and bypass the `showAudio` toggle, potentially appearing in results even when the user has disabled audio/telephony filtering.
    
    Consider adding `"call.audio"` back as an alias pointing to `NodeTypeBadge` until the stale-cleanup cycle (30 days) clears those rows:
    ```kotlin
    "lxmf.delivery", "lxmf.propagation", "nomadnetwork.node", "lxst.telephony", "call.audio", null -> {
        NodeTypeBadge(nodeType = announce.nodeType)
    }
    ```
    
    How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: 3c4b9c4

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

12 files reviewed, 4 comments

Edit Code Review Agent Settings | Greptile

@torlando-tech torlando-tech changed the title feat: replace call.audio with lxst.telephony + PHONE NodeType feat: clean up stale announces older than 30 days Feb 28, 2026
@torlando-tech torlando-tech added this to the v0.10.0 milestone Mar 7, 2026
@torlando-tech torlando-tech force-pushed the worktree-temporal-floating-blanket branch from 82d160b to 9e9d21d Compare March 10, 2026 04:41
@torlando-tech torlando-tech changed the title feat: clean up stale announces older than 30 days feat: clean up stale announces, add PHONE NodeType and cross-link buttons Mar 10, 2026
@torlando-tech torlando-tech force-pushed the worktree-temporal-floating-blanket branch from 9e9d21d to 3c4b9c4 Compare March 11, 2026 03:26
@torlando-tech
Copy link
Owner Author

@greptileai

* @param maxAgeMillis Maximum age in milliseconds; announces last seen longer ago than this are deleted
* @return Number of deleted rows
*/
suspend fun deleteStaleAnnounces(maxAgeMillis: Long): Int = announceDao.deleteStaleAnnounces(System.currentTimeMillis() - maxAgeMillis)
Copy link
Contributor

Choose a reason for hiding this comment

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

deleteStaleAnnounces and getLinkedAnnounces are unused dead code

Both repository wrapper methods added in this PR are never called from production code:

  • deleteStaleAnnounces(maxAgeMillis: Long) (line 401) — ServicePersistenceManager.cleanupStaleAnnounces() calls announceDao.deleteStaleAnnounces(cutoffTime) directly, bypassing this method entirely.
  • getLinkedAnnounces(identityHash, excludeHash) (lines 269–271) — only the reactive getLinkedAnnouncesFlow variant is actually used in AnnounceStreamViewModel. The suspend overload has no callers.

Since both were added in this PR, either they should be wired up or removed. If ServicePersistenceManager is intentionally bypassing the repository layer (which is consistent with its existing pattern), then at minimum deleteStaleAnnounces in AnnounceRepository should be removed to avoid confusion about the intended cleanup path.

Prompt To Fix With AI
This is a comment left during a code review.
Path: data/src/main/java/com/lxmf/messenger/data/repository/AnnounceRepository.kt
Line: 401

Comment:
**`deleteStaleAnnounces` and `getLinkedAnnounces` are unused dead code**

Both repository wrapper methods added in this PR are never called from production code:

- `deleteStaleAnnounces(maxAgeMillis: Long)` (line 401) — `ServicePersistenceManager.cleanupStaleAnnounces()` calls `announceDao.deleteStaleAnnounces(cutoffTime)` directly, bypassing this method entirely.
- `getLinkedAnnounces(identityHash, excludeHash)` (lines 269–271) — only the reactive `getLinkedAnnouncesFlow` variant is actually used in `AnnounceStreamViewModel`. The suspend overload has no callers.

Since both were added in this PR, either they should be wired up or removed. If `ServicePersistenceManager` is intentionally bypassing the repository layer (which is consistent with its existing pattern), then at minimum `deleteStaleAnnounces` in `AnnounceRepository` should be removed to avoid confusion about the intended cleanup path.

How can I resolve this? If you propose a fix, please make it concise.

torlando-tech and others added 5 commits March 11, 2026 00:11
Drops the deprecated `call.audio` meshchat aspect and adds `lxst.telephony`
as a first-class `NodeType.PHONE` throughout the stack:

- python: register `lxst.telephony` announce handler so RNS delivers these
  announces to Kotlin (previously they were silently dropped)
- model: add `PHONE` to `NodeType` enum
- NodeTypeDetector: `lxst.telephony` → `NodeType.PHONE`; remove `call.audio` case
- AnnounceDao: add `getAnnouncesByIdentityHashExcluding` for identity cross-linking
- AnnounceRepository: expose `getLinkedAnnounces()` for cross-link lookups
- AnnounceStreamViewModel: type filter now uses `PHONE` instead of PEER-for-audio
  hack; `isAudioAnnounce` checks `nodeType == PHONE`; add `getLinkedAnnouncesFlow()`
- PeerCard: add "Phone" badge for PHONE nodeType; remove AudioBadge composable
- AnnounceStreamScreen: `lxst.telephony` falls through to NodeTypeBadge; PHONE
  excluded from type-filter checkboxes (controlled by audio toggle)
- AnnounceDetailScreen: update aspect subtitle; add `onViewAnnounce` param;
  show symmetric cross-link buttons between telephony and messaging destinations
- MainActivity: wire `onViewAnnounce` to announce detail navigation

No Room migration needed — NodeType is stored as a plain string; old `call.audio`
rows remain as `nodeType="NODE"` and age out naturally.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Announces table grows unbounded as peers are discovered. Add a cleanup
query that deletes announces with lastSeenTimestamp > 30 days old,
preserving favorites and contacts across all identities. Triggered once
per service lifecycle in ReticulumService.onCreate().

Closes #273

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ViewModelTest

Reset Dispatchers.Main before canceling viewModelScope in tearDown.
This ensures IO coroutine cleanup (geocoder check on Dispatchers.IO)
doesn't route cancellation exceptions through the test dispatcher,
which caused intermittent UncaughtExceptionsBeforeTest failures.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use aspect consistently for cross-link detection (lxst.telephony)
- Make dead PHONE branch explicit with error() guard
- Restore red badge color for PHONE to distinguish from NODE
- Update stale call.audio comment to lxst.telephony

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Replace cold flow{} in getLinkedAnnouncesFlow with flatMapLatest over
  Room's reactive queries so cross-link buttons appear live when a
  linked announce arrives after the detail screen is already open
- Add reactive DAO/repository methods for identity-hash-based lookups
- Fix misleading deleteStaleAnnounces docstring (parameter is a duration,
  not a pre-computed timestamp)
- Add missing onViewAnnounce callback to MainActivity's AnnounceDetailScreen
  call site (lost during rebase conflict resolution)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@torlando-tech torlando-tech force-pushed the worktree-temporal-floating-blanket branch from 3c4b9c4 to d604c4d Compare March 11, 2026 04:13
@torlando-tech torlando-tech merged commit 7269cd7 into main Mar 11, 2026
14 checks passed
@torlando-tech torlando-tech deleted the worktree-temporal-floating-blanket branch March 11, 2026 04:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clean up old announces

1 participant