Repository navigation
perf(dashboard): Read the mailbox with the first conversation detail - #2716
Merged
Merged
Conversation
A first load with no cached copy read /pending-messages and then the detail, so the transcript waited for two round trips. The detail route now takes mailbox=true and reads the mailbox before history in one request, which keeps the order that prevents a gap. Polls still read the mailbox apart so the detail can answer 304. Co-Authored-By: Thomas Jiang <20501235+thomasjiangcy@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
3 screenshot changes — 3 changed · 0 added · 0 removed Review screenshots in Frameshift
|
Guard the no-304 rule for ?mailbox=true in the core route and the mock reporting middleware, check that non-participants get a redacted mailbox on the combined read, and serve queued rows with the first e2e detail read so the queued-message specs do not wait for a poll. Co-Authored-By: Thomas Jiang <20501235+thomasjiangcy@users.noreply.github.com>
thomasjiangcy
approved these changes
Nov 5, 2026
BYK
pushed a commit
that referenced
this pull request
Nov 5, 2026
…ersation read (#2720) Opening a conversation built the whole detail report twice. `readConversationDetailEtag`, added in #2655, hashed the viewer's read position, and the transcript sends `PUT /read-position` soon after it opens and again as the reader scrolls. Each write changed the version token. So the next poll got a 200 with the full report, not a 304. That new token also made `conversationDetailQueryOptions` refresh agents, memories, watches and Review items. A prod browser capture of one conversation open showed it: `pending-messages` → detail (200, 77.8 kB, 1.41s) → `read-position` → `pending-messages` → detail (200 again, 77.9 kB, 1.44s) → `memories` and `spend` again. Change: - The read position is no longer part of the detail ETag, and that query no longer runs on each poll. `useMarkConversationRead` already puts the stored position into the cached detail, so the reader's own client stays correct. - Polls now answer 304 after a read, so the cache keeps the `unreadCursor` from the last full build. `useMarkConversationRead` now drops `unreadCursor` and `unreadPage` when the new position reaches the latest page. It uses the same rule as the server (`lastReadSeq + 1 < firstSeq`). Without this, a second visit in the same session would open at rows the reader already read. The current visit is not affected, because `useReadPositionAtOpen` keeps the values from when the conversation opened. - The read-position integration scenario now checks that a poll after a read returns 304. Trade-off: another tab or device does not see the new read position until the detail changes or the page reloads. A new page load has no cache, so it always gets the current position. If the reader is still behind the latest page after a write, the cached cursor stays at the older position until the next full build. A reopen in that tab then loads from slightly earlier rows. Not checked: the dashboard Playwright read-position spec, because Chrome is not installed in my sandbox. CI runs it. Mock reporting uses a content-hash ETag, so the e2e path still rebuilds after a read either way. `tests/dashboard-output.test.ts` also fails on `main` in my sandbox. Overlaps: #2716 changes the first detail read (mailbox in the same request) in `queries.ts` and the detail route, but not the ETag. #2657 (unread catch-up) touches read-position e2e and the unread flow. Whichever merges second may need a small rebase. <!-- junior-request-attribution:start --> via @thomasjiangcy. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:agent%3Ab48dee8440f2faa8ee84518a2b9ac612 --> -- [View Session](https://junior-prod.sentry.dev/conversations/agent%3Ab48dee8440f2faa8ee84518a2b9ac612) [[Sentry]](https://sentry.sentry.io/explore/conversations/agent%3Ab48dee8440f2faa8ee84518a2b9ac612/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Thomas Jiang <20501235+thomasjiangcy@users.noreply.github.com>
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a conversation opens with no cached copy, the transcript waits for two requests in a row:
/pending-messages, then/api/conversations/:id. #1932 added this order on purpose. Workers commit input before they acknowledge it, so reading the mailbox first means a message that moves into history between the two reads still appears in one of them.In prod traces, the detail request started only after the mailbox request returned:
3cffecfa…(navigation): mailbox 47→383ms, then detail from 385ms.0d26d374…(pageload): mailbox 713→856ms, then detail from 861ms.19019d5f…and40a68dfc…: the mailbox took 0.86s and 1.8s in the browser, and the detail waited for it.Change:
GET /api/conversations/:id?mailbox=truereads the mailbox first and then history, in one request. It returns the/pending-messagesreport asmailbox. A request withmailbox=truenever returns 304.withRequestedMailbox, adds the mailbox to mocked detail reports.Expected impact (inference): opening a conversation saves one round trip plus the server's mailbox read, about 150–350ms in typical traces and up to about 1.8s in the slow ones. Polling cost does not change.
Checks:
pnpm typecheck(junior, junior-dashboard), oxlint, prettier,file-length:check, juniorpending-messagesanddetailintegration tests, dashboard vitest, and the whole dashboard Playwright suite (87 passed).tests/dashboard-output.test.tsfails in my sandbox onmaintoo, during its Nitro build, so it is not related to this change.Overlaps: #2657, #2676, #2677, #2678, and #2428 change the conversation page UI. This PR changes only
queries.tsdata loading and the detail route, so I expect few conflicts. #2657 (catch up on unread activity) may touch the same detail options. This branch is based onmain. #2712 (Neon pool size) is independent, and it fixes most of the server-side wait in the slow traces.via @thomasjiangcy.
--
View Session [Sentry]