fix(messaging): stabilize message paging queries to prevent duplicate LazyColumn keys (COLUMBA-9W)

[sentry]

The LazyColumn in MessagingScreen.kt was crashing with an IllegalArgumentException: Key "..." was already used because pagingItems.itemKey { message -> message.id } received duplicate message.id values.

Root cause analysis revealed that Room's offset-based PagingSource queries (MessageDao.getMessagesForConversationPaged and getMessagesForConversationPagedBySentTime) were emitting duplicate rows across page boundaries. This was due to the ORDER BY clauses (COALESCE(receivedAt, timestamp) DESC and timestamp DESC) lacking a unique tiebreaker, leading to unstable pagination when new messages were inserted or timestamps collided.

This fix addresses the root cause by adding rowid DESC as a secondary sort key to both affected paging queries in MessageDao. SQLite's rowid is a unique, monotonic identifier, ensuring deterministic and stable ordering even when primary sort keys are identical. This prevents the PagingSource from emitting duplicate message.id values, thereby resolving the IllegalArgumentException in the LazyColumn while preserving its stable key for smooth scrolling and animations.

Fixes COLUMBA-9W

[greptile-apps]

Greptile Summary

Adds rowid DESC as a secondary sort key to both Room paging queries in MessageDao to stabilize OFFSET-based pagination and prevent duplicate message.id values from reaching pagingItems.itemKey { } in LazyColumn.

• getMessagesForConversationPaged and getMessagesForConversationPagedBySentTime both get rowid DESC appended to their ORDER BY clauses; SQLite's implicit rowid is unique and monotonically increasing, so it deterministically breaks timestamp ties without requiring a schema change.
• The root cause (non-unique sort key on an OFFSET-based PagingSource) is correctly identified and fixed; the choice of rowid is a lightweight, correct tiebreaker for this case.

Confidence Score: 4/5

Safe to merge; the fix correctly stabilises OFFSET-based paging and eliminates the LazyColumn crash. One edge case around REPLACE-based re-insertion changing rowid is worth watching but does not affect the happy path.

The two-line change is minimal and well-targeted. The rowid tiebreaker is the canonical SQLite solution for this class of pagination instability. The only subtle concern is that insertMessage/insertMessages use OnConflictStrategy.REPLACE, which performs a DELETE+INSERT and assigns a new rowid; this could reorder tied-timestamp messages after a bulk import or replay, though normal status-update flows use dedicated UPDATE queries and are unaffected.

The only changed file is data/src/main/java/network/columba/app/data/db/dao/MessageDao.kt; the REPLACE conflict strategy on insertMessage and insertMessages is worth a second look if import/replay scenarios are exercised.

Important Files Changed

Filename	Overview
data/src/main/java/network/columba/app/data/db/dao/MessageDao.kt	Adds rowid DESC tiebreaker to both paging queries; fix is correct but REPLACE conflict strategy on insertMessage/insertMessages could reassign rowid on re-insertion, subtly reordering tied-timestamp messages after an import or replay.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant UI as MessagingScreen (LazyColumn)
    participant VM as ViewModel
    participant Pager as Pager3
    participant DAO as MessageDao
    participant DB as SQLite (messages)

    UI->>VM: collect pagingData
    VM->>Pager: "flow<PagingData<MessageEntity>>"
    Pager->>DAO: getMessagesForConversationPaged(peerHash, identityHash)
    DAO->>DB: "SELECT * FROM messages WHERE ... ORDER BY COALESCE(receivedAt, timestamp) DESC, rowid DESC"
    Note over DB: rowid DESC breaks timestamp ties,<br/>ensuring stable OFFSET pages
    DB-->>DAO: deterministic, duplicate-free rows
    DAO-->>Pager: PagingSource (stable pages)
    Pager-->>VM: "PagingData<MessageEntity>"
    VM-->>UI: pagingItems (unique message.id keys)
    Note over UI: LazyColumn itemKey { message.id }<br/>no longer throws IllegalArgumentException

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant UI as MessagingScreen (LazyColumn)
    participant VM as ViewModel
    participant Pager as Pager3
    participant DAO as MessageDao
    participant DB as SQLite (messages)

    UI->>VM: collect pagingData
    VM->>Pager: "flow<PagingData<MessageEntity>>"
    Pager->>DAO: getMessagesForConversationPaged(peerHash, identityHash)
    DAO->>DB: "SELECT * FROM messages WHERE ... ORDER BY COALESCE(receivedAt, timestamp) DESC, rowid DESC"
    Note over DB: rowid DESC breaks timestamp ties,<br/>ensuring stable OFFSET pages
    DB-->>DAO: deterministic, duplicate-free rows
    DAO-->>Pager: PagingSource (stable pages)
    Pager-->>VM: "PagingData<MessageEntity>"
    VM-->>UI: pagingItems (unique message.id keys)
    Note over UI: LazyColumn itemKey { message.id }<br/>no longer throws IllegalArgumentException

Loading

Comments Outside Diff (1)

1. data/src/main/java/network/columba/app/data/db/dao/MessageDao.kt, line 39-40 (link)

   REPLACE conflict strategy silently reassigns rowid, undermining the tiebreaker

   insertMessage and insertMessages both use OnConflictStrategy.REPLACE, which SQLite implements as DELETE + INSERT. The new row receives the next auto-assigned rowid — the highest in the table at that moment. If a message is re-inserted via REPLACE (e.g., during import via insertMessages or an LXMF replay), its rowid jumps to the top of the ordering, so it will sort before all other messages that share the same COALESCE(receivedAt, timestamp) value. Dedicated update paths (updateMessage, updateMessageStatus, etc.) use plain UPDATE and are fine since they don't change rowid. The risk is narrower than it might look — but bulk import or a replay event can silently reorder tied-timestamp messages in a conversation.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: data/src/main/java/network/columba/app/data/db/dao/MessageDao.kt
   Line: 39-40
   
   Comment:
   **REPLACE conflict strategy silently reassigns `rowid`, undermining the tiebreaker**
   
   `insertMessage` and `insertMessages` both use `OnConflictStrategy.REPLACE`, which SQLite implements as DELETE + INSERT. The new row receives the next auto-assigned `rowid` — the highest in the table at that moment. If a message is re-inserted via REPLACE (e.g., during import via `insertMessages` or an LXMF replay), its `rowid` jumps to the top of the ordering, so it will sort before all other messages that share the same `COALESCE(receivedAt, timestamp)` value. Dedicated update paths (`updateMessage`, `updateMessageStatus`, etc.) use plain `UPDATE` and are fine since they don't change `rowid`. The risk is narrower than it might look — but bulk import or a replay event can silently reorder tied-timestamp messages in a conversation.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
data/src/main/java/network/columba/app/data/db/dao/MessageDao.kt:39-40
**REPLACE conflict strategy silently reassigns `rowid`, undermining the tiebreaker**

`insertMessage` and `insertMessages` both use `OnConflictStrategy.REPLACE`, which SQLite implements as DELETE + INSERT. The new row receives the next auto-assigned `rowid` — the highest in the table at that moment. If a message is re-inserted via REPLACE (e.g., during import via `insertMessages` or an LXMF replay), its `rowid` jumps to the top of the ordering, so it will sort before all other messages that share the same `COALESCE(receivedAt, timestamp)` value. Dedicated update paths (`updateMessage`, `updateMessageStatus`, etc.) use plain `UPDATE` and are fine since they don't change `rowid`. The risk is narrower than it might look — but bulk import or a replay event can silently reorder tied-timestamp messages in a conversation.

_{Reviews (1): Last reviewed commit: "fix(messaging): stabilize message paging..." | Re-trigger Greptile}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!