Skip to content

fix(app): reduce reactive cascade overhead on session switch #36853

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
BYK wants to merge 1 commit into
base: dev
Choose a base branch
from
+22 −13
Open

fix(app): reduce reactive cascade overhead on session switch #36853

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions packages/app/src/pages/session.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ import { Identifier } from "@/utils/id"
import { diffs as list } from "@/utils/diffs"
import { Persist, persisted } from "@/utils/persist"
import { extractPromptFromParts } from "@/utils/prompt"
import { same } from "@/utils/same"
import { formatServerError, isLocalSessionNotFoundError, isSessionNotFoundError } from "@/utils/server-errors"
import { legacySessionHref, requireServerKey, sessionHref } from "@/utils/session-route"
import { useUsageExceededDialogs } from "./session/usage-exceeded-dialogs"
Expand Down Expand Up @@ -532,7 +533,7 @@ export default function Page() {

const info = createMemo(() => (params.id ? sync().session.get(params.id) : undefined))
const isChildSession = createMemo(() => !!info()?.parentID)
const diffs = createMemo(() => (params.id ? list(sync().data.session_diff[params.id]) : []))
const diffs = createMemo(() => (params.id ? list(sync().data.session_diff[params.id]) : []), [], { equals: same })
const canReview = createMemo(() => !!sync().project)
const reviewTab = createMemo(() => isDesktop())
const tabState = createSessionTabs({
Expand Down Expand Up @@ -587,11 +588,15 @@ export default function Page() {

createEffect(
on(
() => ({ dir: sdk().directory, id: params.id }),
(next, prev) => {
// Use a primitive string key instead of creating a new object on every
// evaluation. SolidJS on() uses === comparison, so a string avoids
// the always-different-reference problem that objects have.
() => `${params.dir}\0${params.id ?? ""}`,
(_next, prev) => {
if (!prev) return
if (next.dir === prev.dir && next.id === prev.id) return
if (prev.id && !next.id) local.session.reset()
// prev had a session ID (non-empty after the separator) and now there's none
const prevHadId = prev.indexOf("\0") < prev.length - 1
if (prevHadId && !params.id) local.session.reset()
},
{ defer: true },
),
Expand Down
20 changes: 12 additions & 8 deletions packages/app/src/pages/session/use-session-hash-scroll.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { UserMessage } from "@opencode-ai/sdk/v2"
import { useLocation, useNavigate } from "@solidjs/router"
import { createEffect, createMemo, onCleanup, onMount } from "solid-js"
import { createEffect, onCleanup, onMount } from "solid-js"
import { messageIdFromHash } from "./message-id-from-hash"

export const useSessionHashScroll = (input: {
Expand All @@ -22,8 +22,10 @@ export const useSessionHashScroll = (input: {
scheduleScrollState: (el: HTMLDivElement) => void
consumePendingMessage: (key: string) => string | undefined
}) => {
const visibleUserMessages = createMemo(() => input.visibleUserMessages())
const messageById = createMemo(() => new Map(visibleUserMessages().map((m) => [m.id, m])))
// Use input directly instead of wrapping in a memo without an equality comparator.
// Replace the Map (rebuilt on every change) with a lazy lookup — user message
// counts are small enough that linear scan is fine for the point lookups here.
const findMessage = (id: string) => input.visibleUserMessages().find((m) => m.id === id)
let pendingKey = ""
let clearing = false

Expand Down Expand Up @@ -110,7 +112,7 @@ export const useSessionHashScroll = (input: {
const messageId = messageIdFromHash(hash)
if (messageId) {
input.autoScroll.pause()
const msg = messageById().get(messageId)
const msg = findMessage(messageId)
if (msg) {
scrollToMessage(msg, behavior)
return
Expand Down Expand Up @@ -141,7 +143,8 @@ export const useSessionHashScroll = (input: {
createEffect(() => {
if (!input.sessionID() || !input.messagesReady()) return

visibleUserMessages()
// Subscribe to message list changes
input.visibleUserMessages()

let targetId = input.pendingMessage()
if (!targetId) {
Expand All @@ -160,7 +163,7 @@ export const useSessionHashScroll = (input: {
if (!targetId) return

const pending = input.pendingMessage() === targetId
const msg = messageById().get(targetId)
const msg = findMessage(targetId)
if (!msg) return

if (pending) input.setPendingMessage(undefined)
Expand All @@ -175,12 +178,13 @@ export const useSessionHashScroll = (input: {
const sessionID = input.sessionID()
if (!sessionID || !input.messagesReady()) return

visibleUserMessages()
// Subscribe to message list changes
input.visibleUserMessages()

let targetId = input.pendingMessage()
if (!targetId && !clearing) targetId = messageIdFromHash(location.hash)
if (!targetId) return
if (messageById().has(targetId)) return
if (findMessage(targetId)) return
if (!input.historyMore() || input.historyLoading()) return

void input.loadMore(sessionID)
Expand Down
Loading