fix: sort messages by received time (backport to v0.9.x) by torlando-tech · Pull Request #656 · torlando-tech/columba

torlando-tech · 2026-03-11T02:21:46Z

Summary

Backport of PR fix: sort messages by local receive time to prevent clock-drift reordering #645 to release/v0.9.x
Adds sortMessagesBySentTime stubs to all test classes mocking SettingsRepository (including inline mocks that use clearAllMocks())
Adds performScrollTo() to hop count card assertions in MessageDetailScreenTest to account for the new "Sent by Sender" timestamp card pushing content below the viewport

Context

The feature commit and CI fixes (dependency retry, dual-scheduler race fix) were already on release/v0.9.x via PR #651. This PR carries over the two remaining commits that weren't included in that merge.

Test plan

CI passes all 4 test shards
MessageDetailScreenTest hop count tests pass with performScrollTo()
All SettingsRepository mock consumers have sortMessagesBySentTime stub

🤖 Generated with Claude Code

…tingsRepository The new sortMessagesBySentTime property is read during ViewModel init, so every strict MockK mock of SettingsRepository needs the stub — including the inline failingSettingsRepository in MessagingViewModelTest. Also updates ServicePersistenceManagerDatabaseTest to assert wall-clock time rather than sender timestamp, matching the production behavior change in the parent commit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

The new "Sent by Sender" timestamp card for received messages pushes the hop count card below the Robolectric viewport. Add performScrollTo() before assertIsDisplayed() so the card is scrolled into view first. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

greptile-apps · 2026-03-11T02:24:34Z

Greptile Summary

This PR backports two test-layer commits from the main branch to release/v0.9.x, adding sortMessagesBySentTime flow stubs to all SettingsRepository mocks and updating MessageDetailScreenTest to scroll to hop-count cards that are now pushed below the viewport by the new "Sent by Sender" timestamp card.

sortMessagesBySentTime stub (flowOf(false)) added to 9 test classes that mock SettingsRepository, covering both class-level and inline mockk() instances — coverage looks complete across the changed files.
performScrollTo() added before assertIsDisplayed() for all three hop-count test blocks in MessageDetailScreenTest, correctly accounting for the new UI card that shifts content below the viewport.
ServicePersistenceManagerDatabaseTest: lastMessageTimestamp assertion relaxed from assertEquals(1000L, ...) to assertTrue(... > 500L) to reflect that the field now stores System.currentTimeMillis() rather than the sender's timestamp; the threshold is weaker than needed and could miss regressions.
MessagingViewModelTest and MessagingViewModelImageLoadingTest both gained an unused ReceivedLocationRepository import that was not present in the original and is not referenced anywhere in either file.

Confidence Score: 4/5

Safe to merge; all issues are minor test-quality concerns with no impact on production behaviour.
All changes are confined to test files. The sortMessagesBySentTime stubs are consistently applied, and the performScrollTo() additions are correct. Two unused imports and one overly-permissive assertion reduce the score slightly, but none of these affect runtime correctness.
ServicePersistenceManagerDatabaseTest.kt (weak > 500L assertion) and MessagingViewModelTest.kt / MessagingViewModelImageLoadingTest.kt (unused ReceivedLocationRepository imports).

Important Files Changed

Filename	Overview
app/src/test/java/com/lxmf/messenger/service/persistence/ServicePersistenceManagerDatabaseTest.kt	Updates `lastMessageTimestamp` assertion to reflect new wall-clock behavior; assertion threshold (`> 500L`) is weaker than ideal but not a correctness regression.
app/src/test/java/com/lxmf/messenger/ui/screens/MessageDetailScreenTest.kt	Adds `performScrollTo()` before `assertIsDisplayed()` for hop count card assertions to account for new sender timestamp card pushing content off-screen; change is correct.
app/src/test/java/com/lxmf/messenger/viewmodel/MessagingViewModelTest.kt	Adds `sortMessagesBySentTime` stub to both the main and inline `failingSettingsRepository` mocks; also includes an unused `ReceivedLocationRepository` import.
app/src/test/java/com/lxmf/messenger/viewmodel/MessagingViewModelImageLoadingTest.kt	Adds `sortMessagesBySentTime` stub correctly; contains an unused `ReceivedLocationRepository` import that should be removed.
app/src/test/java/com/lxmf/messenger/viewmodel/NotificationSettingsViewModelTest.kt	Adds missing `flowOf` import and `sortMessagesBySentTime` stub; change is correct and complete.
app/src/test/java/com/lxmf/messenger/viewmodel/ThemeEditorViewModelTest.kt	Adds `flowOf` import and `sortMessagesBySentTime` stub to setup; change is correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[SettingsRepository mock] -->|sortMessagesBySentTime stub added| B[MapTileSourceManagerTest]
    A -->|sortMessagesBySentTime stub added| C[InterfaceConfigManagerTest]
    A -->|sortMessagesBySentTime stub added| D[PropagationNodeManagerTest]
    A -->|sortMessagesBySentTime stub added| E[MessagingViewModelTest\nmain + inline mock]
    A -->|sortMessagesBySentTime stub added| F[MessagingViewModelImageLoadingTest]
    A -->|sortMessagesBySentTime stub added| G[MapViewModelTest]
    A -->|sortMessagesBySentTime stub added| H[NotificationSettingsViewModelTest]
    A -->|sortMessagesBySentTime stub added| I[SettingsViewModelIncomingMessageLimitTest]
    A -->|sortMessagesBySentTime stub added| J[ThemeEditorViewModelTest]

    K[MessageDetailScreenTest] -->|performScrollTo added\nbefore assertIsDisplayed| L[Hop Count card assertions\ndirect / 1 hop / 3 hops]

    M[ServicePersistenceManagerDatabaseTest] -->|lastMessageTimestamp assertion\nchanged from ==1000L to >500L| N[Reflects wall-clock\nreceive time behaviour]

_{Last reviewed commit: c188dfc}

greptile-apps · 2026-03-11T02:24:37Z

app/src/test/java/com/lxmf/messenger/viewmodel/MessagingViewModelTest.kt

 import com.lxmf.messenger.data.repository.ContactRepository
 import com.lxmf.messenger.data.repository.ConversationRepository
 import com.lxmf.messenger.data.repository.IdentityRepository
+import com.lxmf.messenger.data.repository.ReceivedLocationRepository


Unused import added

ReceivedLocationRepository is imported on this line but is never referenced anywhere in the test file body. This import appears to have been accidentally included during the backport.

Suggested change

import com.lxmf.messenger.data.repository.ReceivedLocationRepository

import com.lxmf.messenger.data.repository.IdentityRepository

import com.lxmf.messenger.data.repository.ReplyPreview

Prompt To Fix With AI

This is a comment left during a code review. Path: app/src/test/java/com/lxmf/messenger/viewmodel/MessagingViewModelTest.kt Line: 16 Comment: **Unused import added** `ReceivedLocationRepository` is imported on this line but is never referenced anywhere in the test file body. This import appears to have been accidentally included during the backport. ```suggestion import com.lxmf.messenger.data.repository.IdentityRepository import com.lxmf.messenger.data.repository.ReplyPreview ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-03-11T02:24:38Z

app/src/test/java/com/lxmf/messenger/viewmodel/MessagingViewModelImageLoadingTest.kt

 import com.lxmf.messenger.data.repository.ContactRepository
 import com.lxmf.messenger.data.repository.ConversationRepository
 import com.lxmf.messenger.data.repository.IdentityRepository
+import com.lxmf.messenger.data.repository.ReceivedLocationRepository


Unused import added

ReceivedLocationRepository is imported here but is never used anywhere in the test file. The same unused import was also introduced in MessagingViewModelTest.kt (line 16). Both can be safely removed.

Prompt To Fix With AI

This is a comment left during a code review. Path: app/src/test/java/com/lxmf/messenger/viewmodel/MessagingViewModelImageLoadingTest.kt Line: 16 Comment: **Unused import added** `ReceivedLocationRepository` is imported here but is never used anywhere in the test file. The same unused import was also introduced in `MessagingViewModelTest.kt` (line 16). Both can be safely removed. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-03-11T02:24:39Z

...rc/test/java/com/lxmf/messenger/service/persistence/ServicePersistenceManagerDatabaseTest.kt

+            assertTrue(
+                "lastMessageTimestamp should be recent wall-clock time",
+                updated!!.lastMessageTimestamp > 500L,
+            )


Assertion is weaker than necessary

The new assertion updated!!.lastMessageTimestamp > 500L only checks that the timestamp is greater than the old conversation's lastMessageTimestamp (500L). Any value from 501L upward would pass, so this does not meaningfully verify that the timestamp is a "recent wall-clock time" as the comment states.

A more useful assertion would capture System.currentTimeMillis() just before calling persistMessage, then assert that lastMessageTimestamp >= beforePersist. This ensures regressions (e.g. accidentally reverting to sender timestamp 1000L, or writing 0L) would be caught, while still accommodating the wall-clock behavior introduced by the production change.

Prompt To Fix With AI

This is a comment left during a code review. Path: app/src/test/java/com/lxmf/messenger/service/persistence/ServicePersistenceManagerDatabaseTest.kt Line: 218-221 Comment: **Assertion is weaker than necessary** The new assertion `updated!!.lastMessageTimestamp > 500L` only checks that the timestamp is greater than the old conversation's `lastMessageTimestamp` (500L). Any value from 501L upward would pass, so this does not meaningfully verify that the timestamp is a "recent wall-clock time" as the comment states. A more useful assertion would capture `System.currentTimeMillis()` just before calling `persistMessage`, then assert that `lastMessageTimestamp >= beforePersist`. This ensures regressions (e.g. accidentally reverting to sender timestamp 1000L, or writing 0L) would be caught, while still accommodating the wall-clock behavior introduced by the production change. How can I resolve this? If you propose a fix, please make it concise.

torlando-tech and others added 2 commits March 10, 2026 22:21

greptile-apps bot reviewed Mar 11, 2026

View reviewed changes

torlando-tech merged commit 438e7f1 into release/v0.9.x Mar 11, 2026
2 checks passed

torlando-tech deleted the fix/sort-messages-by-received-time-v0.9.x branch March 11, 2026 02:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: sort messages by received time (backport to v0.9.x)#656

fix: sort messages by received time (backport to v0.9.x)#656
torlando-tech merged 2 commits intorelease/v0.9.xfrom
fix/sort-messages-by-received-time-v0.9.x

torlando-tech commented Mar 11, 2026

Uh oh!

greptile-apps bot commented Mar 11, 2026

Uh oh!

greptile-apps bot Mar 11, 2026

Uh oh!

greptile-apps bot Mar 11, 2026

Uh oh!

greptile-apps bot Mar 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	import com.lxmf.messenger.data.repository.ReceivedLocationRepository
	import com.lxmf.messenger.data.repository.IdentityRepository
	import com.lxmf.messenger.data.repository.ReplyPreview

Uh oh!

Conversation

torlando-tech commented Mar 11, 2026

Summary

Context

Test plan

Uh oh!

greptile-apps bot commented Mar 11, 2026

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps bot Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant