fix: cannot load previous messages when lastMessage thread exceeds 50 messages

[OtavioStasiak]

Proposed changes

Issue(s)

https://rocketchat.atlassian.net/browse/NATIVE-1035

How to test or reproduce

• Open the app;
• Enter a room where the last message is a thread with more than 50 messages;
• The loading of the thread and previous messages must work;

Screenshots

Before	After

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• Improvement (non-breaking change which improves a current function)
• New feature (non-breaking change which adds functionality)
• Documentation update (if none of the other choices apply)

Checklist

• I have read the CONTRIBUTING doc
• I have signed the CLA
• Lint and unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works (if applicable)
• I have added necessary documentation (if applicable)
• Any dependent changes have been merged and published in downstream modules

Further comments

Summary by CodeRabbit

• Bug Fixes
  • More reliable, complete chat history loading when browsing older messages: backfilled messages now load in batches with limits and termination when enough main messages are collected, reducing gaps and stalls in large rooms.
• Tests
  • Added an automated end-to-end test that opens a room whose last message is a thread with over 50 replies and verifies thread count display and scroll-to-first-message behavior.

[coderabbitai]

Caution

Review failed

The head commit changed during the review from 9c8e8ea to 01d6c61.

Walkthrough

Implements batched history loading in loadMessagesForRoom: repeatedly calls fetchBatch across channel/group/IM endpoints, aggregates messages into an allMessages array, counts non-thread messages until COUNT or an overall cap, and returns the combined results. Adds a Maestro UI test for a last-message thread with >50 messages.

Changes

Cohort / File(s)	Summary of Changes
Batched message loading logic app/lib/methods/loadMessagesForRoom.ts	Replaces single fetch with looped fetchBatch calls; selects API per apiType (channels/groups/im); accumulates allMessages; tallies main messages (no tmid) until COUNT or termination (empty batches or cap COUNT * 10); updates startTimestamp per batch and returns aggregated array. No public signature changes.
Maestro UI test .maestro/tests/room/room-last-message-thread-50-plus.yaml	Adds a Maestro test that logs in, opens room maestro_test_load_threads, waits up to 60s for UI text message 50 and element id thread-count-55, then scrolls up to reveal the first message.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant Loader as loadMessagesForRoom
  participant API_channels as API (channels)
  participant API_groups as API (groups)
  participant API_im as API (im)

  Client->>Loader: loadMessagesForRoom(roomId, COUNT, latest?)
  activate Loader
  note right of Loader #DDEBF7: init allMessages=[], mainCount=0, start=latest?timestamp:null

  loop while mainCount < COUNT and not terminated
    alt apiType == channels
      Loader->>API_channels: fetchBatch({roomId, count, latest: start})
      API_channels-->>Loader: batch(messages[], metadata)
    else apiType == groups
      Loader->>API_groups: fetchBatch({roomId, count, latest: start})
      API_groups-->>Loader: batch(messages[], metadata)
    else apiType == im
      Loader->>API_im: fetchBatch({roomId, count, latest: start})
      API_im-->>Loader: batch(messages[], metadata)
    end
    note right of Loader #F7F3DD: append messages to allMessages\nincrement mainCount by messages without tmid\nupdate start to oldest timestamp\ncheck cap (COUNT * 10) or no more main messages
  end

  Loader-->>Client: allMessages (aggregated)
  deactivate Loader

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

• diegolmello

Poem

I hop through threads, past fifty so deep,
Batch by batch I gather each peep.
From latest stamp to oldest I roam,
Stitching messages back into home.
Thump-thump — history fetched sweet and neat. 🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title clearly identifies the bug being fixed—loading previous messages when the room’s last message is a thread exceeding 50 messages—and directly corresponds to the core change in the batch-fetch logic and new test case without extraneous detail.
Linked Issues Check	✅ Passed	The changes implement batched fetching in loadMessagesForRoom to restore paging for threads beyond 50 messages and introduce a Maestro test that reproduces the scenario, directly addressing the regression and verification steps outlined in NATIVE-1035.
Out of Scope Changes Check	✅ Passed	All modifications are confined to the loadMessagesForRoom method and the new test definition for long threads, with no unrelated files or features altered, so there are no out-of-scope changes.

---

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 and usage tips.}

[github-actions]

iOS Build Available

Rocket.Chat Experimental 4.65.0.107410

[github-actions]

iOS Build Available

Rocket.Chat Experimental 4.65.0.107411

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

app/lib/methods/loadMessagesForRoom.ts (1)

45-64: Fix regression: load-more sentinel never created

uniqueTmids is always truthy (arrays never coerce to falsy), so !uniqueTmids is now permanently false. That prevents us from ever pushing the synthetic load-more record, which in turn blocks pagination for rooms that hit the 50-message batch size—the very scenario this PR is addressing. Please gate on the array length instead so the sentinel is emitted when there are no thread replies, and tighten the later guard likewise.

-				const uniqueTmids = [...new Set(data.map(m => m.tmid).filter(Boolean))];
-				if (!lastMessageRecord && data.length === COUNT && !uniqueTmids) {
+				const uniqueTmids = [...new Set(data.map(m => m.tmid).filter(Boolean))];
+				if (!lastMessageRecord && data.length === COUNT && uniqueTmids.length === 0) {
 					const loadMoreMessage = {
 						_id: generateLoadMoreId(lastMessage._id as string),
 						rid: lastMessage.rid,
 						ts: moment(lastMessage.ts).subtract(1, 'millisecond').toString(),
 						t: MessageTypeLoad.MORE,
 						msg: lastMessage.msg
 					} as IMessage;
 					data.push(loadMoreMessage);
 				}
 				const onlyThreadMessages = !data.find(item => !item.tmid);
-				if (uniqueTmids && onlyThreadMessages) {
+				if (uniqueTmids.length > 0 && onlyThreadMessages) {

🧹 Nitpick comments (1)

app/views/RoomView/List/hooks/useMessages.ts (1)

98-100: Remove leftover debug log

This console.log will spam every subscription tick in production. Please drop it before merging.

-					console.log('here');

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 14b131c and b82fd67.

📒 Files selected for processing (3)

• app/lib/methods/loadMessagesForRoom.ts (2 hunks)
• app/lib/methods/loadPreviousMessages.ts (1 hunks)
• app/views/RoomView/List/hooks/useMessages.ts (4 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

app/lib/methods/loadPreviousMessages.ts (4)

app/lib/database/services/Subscription.ts (1)

• getSubscriptionByRoomId (8-17)

app/lib/store/auxStore.ts (1)

• store (6-6)

app/lib/methods/helpers/compareServerVersion.ts (1)

• compareServerVersion (10-15)

app/lib/methods/updateMessages.ts (1)

• updateMessages (20-214)

app/lib/methods/loadMessagesForRoom.ts (2)

app/containers/message/interfaces.ts (1)

• IMessage (116-129)

app/definitions/IMessage.ts (1)

• IMessage (126-156)

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (4)

app/views/RoomView/List/hooks/useMessages.ts (4)

89-89: Consider using a Set for improved performance.

The includes() check has O(n) complexity for each element, resulting in O(n²) overall complexity. For large message lists, this could impact performance.

Apply this diff to use a Set for O(n) complexity:

-			const addedIds = messagesIds.current?.filter(id => !prevIds.includes(id));
+			const prevIdsSet = new Set(prevIds);
+			const addedIds = messagesIds.current?.filter(id => !prevIdsSet.has(id));

---

93-93: Remove type assertion for better type safety.

The as any cast bypasses TypeScript's type checking. Consider using proper typing or a type guard instead to maintain type safety.

If the TAnyMessageModel type doesn't include the ts property in its type definition, consider:

1. Updating the type definition to include ts
2. Using a type guard to narrow the type
3. Accessing the property in a type-safe manner

Example using type guard:

const tsNumbers = newMessages.map(m => {
	if ('ts' in m && m.ts instanceof Date) {
		return m.ts.getTime();
	}
	return undefined;
}).filter((ts): ts is number => ts !== undefined);

---

98-98: Add error handling around loadPreviousMessages call.

If loadPreviousMessages throws an error, it could crash the subscription handler and prevent further message updates. Consider adding try-catch to handle potential failures gracefully.

Apply this diff to add error handling:

 			if (oldestTsNumber) {
-				await loadPreviousMessages({ rid, lastOpen: new Date(oldestTsNumber) });
+				try {
+					await loadPreviousMessages({ rid, lastOpen: new Date(oldestTsNumber) });
+				} catch (error) {
+					// Log error for debugging but don't break the subscription
+					console.error('Failed to load previous messages:', error);
+				}
 			}

---

88-100: Consider adding a comment to explain the logic.

The logic for detecting when to load previous messages (when no new IDs are added) is not immediately obvious. A brief comment would help future maintainers understand the intent and how this fixes the thread message loading issue.

Example comment:

// If no new messages were added to the local database, it means we've reached
// the limit of locally cached messages. This can happen when the last message
// is a thread with >50 messages. In this case, we need to fetch more messages
// from the server using the oldest timestamp as the reference point.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b82fd67 and 369e401.

📒 Files selected for processing (1)

• app/views/RoomView/List/hooks/useMessages.ts (4 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)

• GitHub Check: ESLint and Test / run-eslint-and-test
• GitHub Check: format

🔇 Additional comments (3)

app/views/RoomView/List/hooks/useMessages.ts (3)

12-12: LGTM!

The import is correctly added to support the new functionality.

---

36-36: LGTM!

Capturing the previous IDs before state update enables proper detection of newly added messages in the subscription handler.

---

71-71: LGTM!

Converting the subscription handler to async is necessary to support the await loadPreviousMessages call introduced later in this function.

[github-actions]

Android Build Available

Rocket.Chat Experimental 4.66.0.107570

Internal App Sharing: https://play.google.com/apps/test/RQVpXLytHNc/ahAO29uNTNmvO7jsoIXxrl6jKxQMIF5GtqA2LJa0T1jgqtgkcq2F9jWEtCB1cnqVs2FhlxQdG-stcQua0bBuC_Sz_D

[github-actions]

iOS Build Available

Rocket.Chat Experimental 4.66.0.107571

[github-actions]

iOS Build Available

Rocket.Chat Experimental 4.66.0.107584

[diegolmello]

lgtm