Repository navigation
feat(dashboard): Open long transcripts at the read position - #2635
sentry-junior[bot] wants to merge 18 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
3 screenshot changes — 3 changed · 0 added · 0 removed Review screenshots in Frameshift
|
This comment has been minimized.
This comment has been minimized.
When the read position is older than the latest page, Conversation detail returns an unreadCursor. The events API pages forward with after. The dashboard opens at the read position, loads newer pages in the background until they reach the latest page, and loads earlier pages on scroll from there. Refs #2568
…ned events A cache from a visit that read to the end has no unread cursor. The first fetched detail now supplies it, and the cached transcript stays until the unread pages load. Events that new events push out of the latest page now stay in the transcript, so the latest page does not unjoin after catch-up.
7a59c41 to
268b9b4
Compare
BYK
left a comment
There was a problem hiding this comment.
GET /api/conversations/:id/events takes after= as well as before=. An after page holds the earliest events after the cursor. It has a nextCursor when newer events follow, and a previousCursor when older events come before it.
We should also probably have an "around=" for this?
…t-read-position # Conflicts: # packages/junior-dashboard/src/client/conversations/ConversationPage.tsx
Co-Authored-By: BYK <burak.kaya@sentry.io>
…ng rows Read the latest page alone, not the transcript joined with cached earlier pages, and not the deferred copy that lags the fetched read cursor. Withhold inline sends while newer pages load. Memory jumps use the shown history and wait for newer pages before they look back.
The transcript does not end at the latest event until newer pages join, so an inline send has no place there. Keep it in the composer queue.
This comment has been minimized.
This comment has been minimized.
BYK
left a comment
There was a problem hiding this comment.
Also please ensure there are no perf regressions on the server side and ideally there is actually improvements.
Newer pages from the read position loaded one after another until they reached the latest page, so a reader far behind cost one server page read per 500 unread events, even without a scroll. Load them when the reader scrolls near them, like earlier pages. The server now reads only the pages that the reader reaches. Linked events, Turns, and memory jumps share one search step: load a page toward the row, or wait for the page that loads. The unread cursor comes only from the fetched detail, so the cache no longer needs it. After pages no longer read one row before the page: nothing uses it.
…tail The first detail request of a visit asks for the page around the read position with unreadPage=true. A reader far behind opens with one request, not two. Polls do not ask, so the server reads that page once. The server now reads the read position with the other detail reads, in parallel. A cached transcript holds without a marker until this visit's fetch tells where to open. Move the archive queries to conversationArchive.ts so queries.ts stays under the file length limit. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>
|
Might wanna rebase this |
Jump to latest, a send, or the mobile keyboard now skips the newer pages that are not loaded. The transcript shows the latest page from the polled detail at once, and earlier pages load above it as the reader scrolls up. A transcript that does not end at the latest event no longer follows its end. A newer page that loads below the reader keeps their place, so pages no longer load one after another, and the jump control stays visible. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>
…t-read-position # Conflicts: # packages/docs/src/content/docs/operate/dashboard.md
…led one A send or the mobile keyboard during a cached reopen now shows the latest page after the fetch too. The page owns the choice, so it works before the unread page loads. An explicit pin no longer opens at the "New" marker. A failed unread page no longer fails conversation detail. The dashboard then reads it with an around request, which reports the failure.
The held copy sits at its latest rows while the visit fetches where to open, so reporting them moved the read position past unread events. Co-Authored-By: BYK <burak.kaya@sentry.io>
The app shell started its own detail fetch on a direct load. The page's first fetch joined it and lost ?unreadPage=true, so far-behind readers paid a second, serial around request. The shell now reads the title from the cache without fetching. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>
…t-read-position # Conflicts: # packages/junior-dashboard/src/client/conversations/ConversationPage.tsx # packages/junior-dashboard/src/client/conversations/ConversationSidebar.tsx # packages/junior-dashboard/src/client/conversations/ConversationWorkspace.tsx # packages/junior-dashboard/src/client/conversations/conversationArchive.ts # packages/junior-dashboard/src/client/conversations/queries.ts # packages/junior/src/api/conversations/detail.ts # packages/junior/src/api/conversations/routes.ts # packages/junior/src/api/schema.ts Co-Authored-By: BYK <126780+BYK@users.noreply.github.com> Co-Authored-By: BYK <burak.kaya@sentry.io>
This reverts commit d156e15. The page mounts only after the dashboard core data loads, so the shell's early fetch is what runs the detail in parallel with it. Without it, every direct load waits one more round trip for the detail, to save an around request only for far-behind readers. It also made the offline reconnect e2e test flaky. Co-Authored-By: BYK <126780+BYK@users.noreply.github.com> Co-Authored-By: BYK <burak.kaya@sentry.io>
|
Found 2 test failures on Blacksmith runners: Failures
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5f119d6. Configure here.
| pages: [unreadPage], | ||
| }, | ||
| } | ||
| : undefined), |
There was a problem hiding this comment.
Unread pages leak across visits
Medium Severity
The unread newer-page query uses staleTime: Infinity and a key of conversation plus unreadCursor only. A later visit with the same cursor rehydrates every previously fetched newer page, including pages loaded toward a linked event that the reader never viewed, so the capped around page is replaced by the old full range.
Reviewed by Cursor Bugbot for commit 5f119d6. Configure here.
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)


First step of #2568: cap how far toward the present the first load goes.
Today a reader whose read position is older than the latest 500 events opens at the latest page. The "New" marker shows at the top of the loaded rows, not at the first unread row, and the reader has to scroll back through every page to get there.
API
GET /api/conversations/:id/eventstakesafter=<cursor>andaround=<cursor>as well asbefore=<cursor>. Anafterpage holds the earliest events after the cursor. Anaroundpage ends half of its events with the cursor event, and the rest follow it. Both have anextCursorwhen newer events follow. Anaroundpage also has apreviousCursorwhen older events come before it.unreadCursorwhen the viewer's read position is older thanevents. It is a normal signed cursor at the read position. It comes from the read position that detail already reads, so detail runs no extra query.GET /api/conversations/:id?unreadPage=truealso returns thearoundpage ofunreadCursorasunreadPage. Without the flag, or without a cursor, detail reads no extra page.Dashboard (new
transcriptUnreadHistory.ts)unreadCursor, and the reader opens at unread (the default), the transcript starts with the page around the read position, so the last read events show above the marker. It opens at the "New" marker as usual.unreadPage, so that page needs no second request. Polls do not ask. If the page does not come with the detail (for example, the fetch joined one that was already in flight), thearoundrequest reads it as before.Server cost
aroundpage (two half pages in parallel) next to its mention lookup. That is one round trip, one conversation read, and one access read fewer than a separatearoundrequest.The read-position journeys moved to
conversations-read-position.spec.ts, because the mobile spec went over the file length limit. The far-behind journey opens 120 events behind the latest page and checks these things. The reader opens at the first unread row, and the first detail request brings the read events above it, with no/eventsrequest. Polls do not ask for the page. No newer page loads before the reader scrolls. The latest page waits until the newer pages reach it. The pages before the latest page are never read. The other read-position journeys get nounreadPage, so they cover the separatearoundrequest.Detail mocks in the other e2e specs now match the detail URL with a query string. The archive queries moved to
conversationArchive.ts, soqueries.tsstays under the file length limit.Still open in #2568
maxPageson these queries is a natural fit. This also covers the layout cost that fix(dashboard): Stop transcript jumps while scrolling up on mobile #2634 adds back on phones.Refs #2568
via @BYK.
--
View Session in Sentry