Skip to content

feat: bounded message window for centered Jump to Message#7388

Open
diegolmello wants to merge 9 commits into
developfrom
oasis-ocean
Open

feat: bounded message window for centered Jump to Message#7388
diegolmello wants to merge 9 commits into
developfrom
oasis-ocean

Conversation

@diegolmello

@diegolmello diegolmello commented Jun 8, 2026

Copy link
Copy Markdown
Member

Proposed changes

Rebuilds Jump to Message on a bounded message window so a deep jump re-anchors in one step (O(1)) instead of growing the window page-by-page (O(pages)) — removing the root cause of the old 5s-race flakiness — without migrating off the custom native inverted list.

The whole capability is one optional upper ts bound (highTs) on the existing WatermelonDB observation:

  • highTs == nullLive Window (newest-first, follows the Live Tail) — unchanged default behavior.
  • finite highTsAnchored Window pinned around the target, below the Live Tail.

A pure, DB/React-free anchor resolver decides the bound (anchorForTarget) and climbs it back toward live as Newer Loaders are consumed (raiseOrRelease), releasing to a Live Window only once the Gap to the Live Tail has fully closed (invariant: never release across an open Gap). "Rejoin live" is now explicit — the Load Newer chain or the jump-to-bottom FAB (which stays visible while anchored).

Also included:

  • Reading-position preservation on release — on RELEASE, grow the Live Window by the count of cached messages above the old bound so a deep target isn't evicted by take(count) and the list doesn't snap to the Live Tail.
  • Scroll-to-index recursion guard — defer one frame + cap retries for onScrollToIndexFailed (no getItemLayout → the inverted list re-fires it synchronously → stack overflow).
  • Jump orchestration — gate anchoring on window membership (isMessageInWindow), not message origin, so a locally-cached-but-out-of-window target no longer silently aborts; three roads (in-window / cached-out-of-window / server) plus a target-ts fallback.
  • Supporting fixes: keep the live socket on deeplink login; tolerate an empty 2FA code on the DDP loadSurroundingMessages call; one-shot jump param so re-searching the same message re-fires.

Design rationale and the rejected alternative (FlashList v2 migration) are in the ADR at app/views/RoomView/docs/adr/0001-bounded-window-over-flashlist.md. New domain vocabulary is in UBIQUITOUS_LANGUAGE.md (§ Message Loading).

Issue(s)

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

How to test or reproduce

  1. Open a room with a long history (hundreds of messages, with gaps).
  2. Search for / tap a target far from the Live Tail. It should re-anchor and center on the target in one step — no long page-by-page scroll, no abort.
  3. Climb back via "Load newer" / scrolling up; the window rejoins live only when the gap closes; the jump-to-bottom FAB is available throughout.
  4. Jump to a deep target, then keep loading newer until release — the reading position is preserved (no snap to the Live Tail).
  5. Jump to the same message twice in a row via Search — it re-fires both times.
  6. Jump to a quoted reply pointing at an on-screen message — it scrolls in place, leaving the Live Tail intact.

Screenshots

No static UI redesign; changes are behavioral (centered jump; FAB visibility while anchored).

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

The list engine is unchanged; a future FlashList migration remains possible but is neither required nor blocked by this work. Ordering stays ts-only (a ts + _id composite is a deferred follow-up noted in the ADR). New unit coverage: anchorResolver, useMessages (release-window growth), useScroll (deferred + capped retry), getLocalAnchor.

Summary by CodeRabbit

Release Notes

New Features

  • Improved "Jump to Message" functionality with anchored window support for better navigation within message history
  • Enhanced message window display modes (live and anchored views) for flexible message viewing

Bug Fixes

  • Fixed TOTP 2FA code argument handling to prevent unwanted trailing parameters
  • Improved WebSocket connection reliability with better disconnection detection and recovery

Documentation

  • Added messaging terminology guide and architectural documentation

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR implements anchored message observation windows for Jump-to-Message O(1) performance, refactors scroll orchestration as a state machine, and hardens connection handling. It adds highTs bounds to message queries, derives anchors via pure resolver functions or cached database queries, and conditions deep-linking and WebSocket reconnection logic on explicit connection state transitions.

Changes

Bounded Message Window

Layer / File(s) Summary
Domain terms and architectural decision
UBIQUITOUS_LANGUAGE.md, app/views/RoomView/docs/adr/*
Message Loading terminology defines Windows, Loaders, Gaps, and Jump-to-Message; ADR documents bounded-window approach over list-engine migration to avoid page-by-page growth and maintain O(1) jumps.
Pure anchor-resolution utilities and tests
app/views/RoomView/List/hooks/anchorResolver.ts, app/views/RoomView/List/hooks/anchorResolver.test.ts
AnchorMessage interface and two pure resolvers: anchorForTarget computes nearest newer-loader timestamp above a target; raiseOrRelease determines climb vs release when boundary loader is consumed. Unit tests cover target discovery, loader selection, gap closure, and timestamp normalization.
Anchored message observation with rejoin logic
app/views/RoomView/List/hooks/useMessages.ts
Adds highTs state, boundary-loader tracking, and raiseOrReleaseAnchor callback; applies ts <= highTs filter when anchored; detects boundary consumption and either raises anchor toward live tail or releases to live window with count growth.
useMessages anchoring behavior tests
app/views/RoomView/List/hooks/useMessages.test.tsx
Mock support for targeted one-shot reads; helpers to detect query's ts <= clause; validates raise/release transitions, take sizing, and hook's highTs/setHighTs exposure.
Jump-to-message state machine and retry logic
app/views/RoomView/List/hooks/useScroll.ts
Replaces recursive visibility polling with pendingJump state machine; adds jump/highlight timeouts, bounded onScrollToIndexFailed retries, and reactive re-observation; jumpToBottom now clears anchor before scrolling.
useScroll jump behavior tests
app/views/RoomView/List/hooks/useScroll.test.tsx
Test harness for anchored/live modes; validates anchor set/re-observe/scroll flow, bounded retry behavior, safety timeout abort, anchor release on jump-to-bottom, and contiguous target one-scroll case.
List component props and ref interface definitions
app/views/RoomView/List/definitions.ts
Adds isAnchored prop to IListProps; extends IListContainerRef with isMessageInWindow(messageId) and updates jumpToMessage(messageId, highTs) signature.
List component FAB visibility logic
app/views/RoomView/List/components/List.tsx
FAB remains visible when anchored (isAnchored || scrolledPastLimit) independent of scroll position.
ListContainer integration and ref exposure
app/views/RoomView/List/index.tsx
Consumes highTs/setHighTs from useMessages, passes to useScroll, exposes isMessageInWindow, and passes isAnchored={highTs != null} to List.
RoomView jump orchestration and anchor derivation
app/views/RoomView/index.tsx
Determines if target is in-window; derives highTs via anchorForTarget (server-fetched) or getLocalAnchorTs (cached); clears one-shot jumpToMessageId param; calls list.jumpToMessage(id, highTs) without timeout racing.
Local anchor resolution service and message info
app/views/RoomView/services/getLocalAnchor.ts, app/views/RoomView/services/getMessageInfo.ts, app/views/RoomView/services/index.ts
getLocalAnchorTs queries cached newer-loader bounds from WatermelonDB; getMessageInfo extended with ts field for anchor derivation; service exported from index.

Connection and Sync Hardening

Layer / File(s) Summary
Deep-linking socket connection synchronization
app/sagas/deepLinking.js, app/sagas/__tests__/deepLinking.test.ts
Adds yield take(METEOR.SUCCESS) gate before loginRequest in token-resume and video-call push flows; prevents login during CONNECTING state; all regression tests now dispatch connectSuccess() before login sequencing.
SDK TOTP parameter and WebSocket reconnection hardening
app/lib/services/sdk.ts, patches/@rocket.chat+sdk+1.3.3-mobile.patch
Conditionally appends TOTP code (via spread operator) only when present, preventing unwanted trailing arguments. WebSocket: detaches stale handlers, closes orphaned sockets before replacement, adds probe() liveness check, introduces forceReopen() with concurrent coalescing, rewrites checkAndReopen() as async bucketed decision; DDP subscription topics extended with media-signal and media-calls.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • RocketChat/Rocket.Chat.ReactNative#7380: Explicitly implements the same "wait for METEOR.SUCCESS before dispatching loginRequest" deep-link socket synchronization logic that this PR adds to app/sagas/deepLinking.js.

Suggested labels

type: feature

Suggested reviewers

  • Rohit3523
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The PR title 'feat: bounded message window for centered Jump to Message' accurately captures the main feature: implementing a bounded message window to support deterministic Jump to Message functionality.

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

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • NATIVE-1224: Request failed with status code 401

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@diegolmello diegolmello changed the title feat(RoomView): bounded message window for centered Jump to Message feat: bounded message window for centered Jump to Message Jun 8, 2026
diegolmello and others added 9 commits June 8, 2026 17:00
…calls

sdk.methodCall appended `this.code || ''` (the TOTP code) as a trailing
positional argument to every DDP method call. That empty string was a
harmless ignored extra until the server's loadSurroundingMessages method
grew a typed `showThreadMessages: boolean` 3rd param (RocketChat/Rocket.Chat
#39092). The '' then fails the server's `check(showThreadMessages, Boolean)`,
returning a 500 that rejects loadSurroundingMessages and aborts the whole
jump-to-message flow (loading shows, no navigation).

Append the TOTP code only when a 2FA flow is in progress, so non-2FA DDP
calls send exactly their declared params — matching the REST path and the
server default. The 2FA flow is preserved: the codeless first call still
gets totp-required, then the retry appends the resolved code.

Covered by the existing .maestro/tests/room/jump-to-message.yaml flow.
… guard scroll-to-index retry

NATIVE-1229 bounded-window jump validation checkpoint.

- useMessages: on Anchored Window RELEASE, grow the Live Window by the count of
  cached messages above the old bound (count += aboveCount) so a deep jump target
  is not evicted by take(count) and the view no longer snaps to the Live Tail.
- useScroll: defer one frame + cap the onScrollToIndexFailed retry to break the
  synchronous VirtualizedList re-fire recursion (stack overflow) on an unmeasurable
  target; re-read the target at fire time so a stale retry cannot scroll wrong.
- getLocalAnchor / getMessageInfo / services: anchored-jump re-seed wiring.
- Regression tests: useMessages (release window growth), useScroll (deferred+capped
  retry), getLocalAnchor.

Includes temporary [JUMP-DBG] instrumentation; to be stripped before merge.
…ntation

Strip the in-runtime debug buffer, DB-primitive globals, and the deterministic
jump/climb drivers used to validate the bounded-window jump on-device. The real
fixes (anchored-window release growth, scroll-to-index retry guard,
isMessageInWindow) are unaffected.
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

iOS Build Available

Rocket.Chat 4.74.0.109044

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant