Skip to content

fix: apply thread reloads and queries to the registered thread - #1929

Merged
isekovanic merged 4 commits into
release-v10from
fix/thread-reload-not-working
Oct 21, 2026
Merged

isekovanic merged 4 commits into
release-v10from
fix/thread-reload-not-working

Conversation

@isekovanic

Copy link
Copy Markdown
Contributor

CLA

  • I have signed the Stream CLA (required).
  • Code changes are tested

Description of the changes, What, Why and How?

Since #1913, getThreadAndHydrate returns the thread registered in client.threads instead of the copy it builds, and skips hydrating it when the thread is open. thread.reload() went through getThreadAndHydrate, so an open thread got itself back and hydrateState(this) returned early. Connection recovery reloads every open thread, and none of them caught up: replies sent while the user was offline never appeared, even after closing and reopening the thread.

Reload

thread.reload() calls client.getThread and hydrates itself with the answer, merged into the loaded window as before. It no longer depends on the thread being registered.

Hydrating from a response

thread.hydrateState takes the server response (ThreadStateResponse) instead of another Thread, like client.hydrateChannels and client.reminders.hydrateState take data. The mapping from response to state is shared with the constructor (formatThreadState). skipReplies updates the state and leaves the replies and isStateStale alone.

Queried threads

ThreadManager.resolveQueried takes the response instead of a copy. It returns a new Thread when none is registered, otherwise the registered one with its state updated:

  • open: replies left to its own reload()
  • stale and not open (every closed thread after a reconnect): replies replaced by the answer
  • current and not open: replies left alone, events kept them current

Queries no longer build a copy for threads that are already registered. Each copy used to keep its replies held in the shared message store, so every thread list reload pinned the listed threads' newest replies for good. ThreadPaginator.toStoredInstance is removed, since queryThreadsAndHydrate already returns the stored instance.

Behaviour changes

  • A registered thread that is open, or not stale, now gets its state (title, read state, reply count, participants, parent message) from the query. Before, it was returned untouched.
  • Thread responses no longer refresh the channel's custom data and members for threads that are already registered. That only happened as a side effect of building the discarded copy.
  • A second ThreadManager, or a ThreadPaginator with a private store, no longer dedupes queried threads against its own store.

Tests

The reload and thread list tests stubbed getThreadAndHydrate / queryThreadsAndHydrate with ready-made Thread copies, which is why none of them caught the regression. They now stub the raw getThread / queryThreads, so the registry lookup runs. New tests cover reloading a registered open thread and the three resolveQueried cases.

Changelog

  • Fixed thread.reload() not applying the fetched replies to an open thread, so open threads catch up after reconnecting
  • Fixed thread queries leaving the replies of already registered threads held in the message store
  • thread.hydrateState now takes a thread response (ThreadStateResponse) instead of another Thread (breaking)
  • Thread queries now update the state of registered threads in place, including open ones

@oliverlaz

Copy link
Copy Markdown
Member

Review: regressions, edge cases, side effects

The reload() fix itself looks right. The findings below are mostly about the new behaviour of hydrating registered threads from query answers.

Main concerns

  1. Query answers can roll back newer live state on open threads (src/thread.ts:422-428, src/thread_manager.ts:180)
    resolveQueried now hydrates open and non-stale registered threads too. hydrateState writes the answer's read, replyCount, participants and parentMessage into state and upserts the parent into the shared messageStore, with no freshness check (EntityStore.upsert has none). If a thread list reload is in flight (for example on connection recovery) and a message.new reply or a message.read arrives for an open thread, the older answer lands afterwards and reverts them: the parent (shared with the channel message list) loses the newer reply_count, reactions or edits, and the read state brings back unread counts. That re-fires markRead for the active thread and shows a wrong badge on inactive ones. Before this PR these threads were left untouched.

  2. A list reload cuts an open thread's participants to 10 (src/thread_manager.ts:180)
    queryThreadsAndHydrate defaults participant_limit to 10, while reload() / getThreadAndHydrate use 100. Open threads now take the list answer's thread_participants, so a list reload shrinks them (fully so when the thread has no subscriptions and the parent-message store projection does not run).

  3. The reply-pinning leak is only fixed for registered threads (src/client.ts:1856)
    Unregistered threads from direct queryThreadsAndHydrate / getThreadAndHydrate calls still get a fresh Thread whose constructor seeds the reply paginator from latest_replies, linking those replies in client.messageStore. Nothing disposes the superseded instances. An app that renders its own list via client.queryThreadsAndHydrate(), or calls getThreadAndHydrate for previews without activate(), still pins replies for the session.

  4. A ThreadPaginator with a private store now churns Thread instances (src/pagination/paginators/ThreadPaginator.ts:105)
    With toStoredInstance gone, resolveQueried only dedupes against client.threads' store. new ThreadPaginator({ client, store: myStore }) (publicly exported) gets new instances on every reload, and the upsert replaces the stored ones, which is exactly what the removed comment warned about. UI holding the old instance is orphaned, and the old instance keeps its replies pinned (see 3).

Edge cases

  1. skipReplies updates replyCount without the replies or the stale flag (src/thread.ts:430)
    It relies on events having kept the replies current. When they did not (ThreadManager subscriptions not registered, or a missed watch gap), a re-query sets replyCount to 5 while the paginator still holds 2 replies with isLastPage: true. Since isStateStale stays false, opening the thread never fetches the missing replies.

  2. isLastPage is computed differently from the constructor (src/thread.ts:437)
    hydrateState compares against the parent's reply_count (includes soft-deleted replies), while the constructor uses threadData.reply_count (excludes them). With 3 replies, 1 soft-deleted and 2 in latest_replies, the constructor marks the last page but hydrateState does not, which triggers a useless older-page fetch and a "load more" affordance.

  3. Registered threads no longer seed their channel's members and custom data (src/thread_manager.ts:177)
    Acknowledged in the description, but untested. A thread registered via ensure() on a channel that was never queried (for example opened from search) and then listed with watch: true gets watchStatus: Watching, but its channel's members and data.custom stay empty until the channel is queried separately (mention suggestions etc.).

Cleanup

  1. reload() duplicates getThreadAndHydrate's defaults (src/thread.ts:358-366): participant_limit: 100 and the inline watchStatus write repeat getThreadAndHydrate / markThreadChannelWatched, so the two can drift.
  2. Duplicate check and stale error text (src/thread.ts:139): the constructor still checks parent_message although formatThreadState now throws the same error, and hydrateState's error still says "using thread with different threadId" though it now takes a response.
  3. Stale source reference (ai-docs/v10-breaking-changes.md:1971): the reload entry still cites src/thread.ts:413-466; reload() is now around 344-399.

Items marked SOLVED in ai-docs/v10-followups.md

25d24e8 (this PR): resolved. toStoredInstance is removed and nothing references it; the trade-off is concern 4.

The other 15: all fixed in the code. None of the cited SHAs is in the release-v10 history, though: they were squash-merged as 98a298d (#1920), and 25d24e8 will dangle the same way once this PR is squash-merged.

Worth a second look:

  • show_in_channel reply edits (dd1b8c7): partly fixed. In src/channel.ts (around line 2405) isAppliedByThread skips the channel's whole update when the parent's thread is registered. Text edits still reach the channel list through the shared store, but pinning or unpinning such a reply never updates channel.pinnedMessagesPaginator (store fan-out does not re-run matchesFilter), and channel messages quoting the edited reply keep the old quoted_message. Neither case is tested. Skipping only messagePaginator.ingestItem would be enough.
  • Request date filters (202e690): fixed, but by 4fb4b52, which replaced that commit and typed the filters as Date only. The cited SHA no longer matches the code.
  • Bulk-rename artifacts (line 1015): the doc says the offline-support/types.ts ones remain, but they are gone too.
  • connectionRecovery.enabled: false (311c28a): documented in ai-docs only; the enabled JSDoc (ConnectionRecoveryManager.ts:14, client.ts:146-150, configuration/shape.ts:457-459) does not say it also suppresses connection.recovered and thread reloads.
  • Positional wrappers (be73434): the listed ones are done; unlisted positional siblings remain (unbanUser, blockUser / unblockUser, channel.banUser / unbanUser, poll.castVote / removeVote / addAnswer / removeAnswer).
  • Retry error (e295664): resolved; minor footgun: runWithRetry can now throw null when a call resolves falsy on its last attempt (not reachable from the API client today).

@isekovanic
isekovanic merged commit acb33c4 into release-v10 Oct 21, 2026
4 checks passed
@isekovanic
isekovanic deleted the fix/thread-reload-not-working branch October 21, 2026 15:18
github-actions Bot pushed a commit that referenced this pull request Oct 22, 2026
## [10.0.0-rc.24](v10.0.0-rc.23...v10.0.0-rc.24) (2026-10-22)

### ⚠ BREAKING CHANGES

* remove messageSetToAddToIfDoesNotExist and let the message paginator place every page (#1926)
* open api hot fixes (#1927)

### Bug Fixes

* apply thread reloads and queries to the registered thread ([#1929](#1929)) ([acb33c4](acb33c4)), closes [#1913](#1913)
* open api hot fixes ([#1927](#1927)) ([4fb4b52](4fb4b52))

### Features

* remove messageSetToAddToIfDoesNotExist and let the message paginator place every page ([#1926](#1926)) ([2b7960d](2b7960d))
@stream-ci-bot

Copy link
Copy Markdown

🎉 This PR is included in version 10.0.0-rc.24 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

4 participants