From d1b4a273d27d60e09840b5d1b940c44a68beb445 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 8 May 2026 08:36:41 +0000 Subject: [PATCH 1/8] perf(frontend): drop subscribed:true from low-value views Several view-only components requested an active discovery subscription just to display cached metadata (breadcrumbs, sidebar listings, notification rows, header chrome). Each of those calls opened a tRPC stream and a discovery state subscription per row; in long lists or deep breadcrumbs that multiplied daemon work for data the user does not actually need to keep fresh. Remove the subscribed flag from these sites; data still flows through React Query, and the entities the user is actively viewing are still covered by the main resource page sub or by joined-site / followed-account derived subscriptions. --- .../desktop/src/components/edit-nav-header-pane.tsx | 4 +++- .../desktop/src/components/import-doc-button.tsx | 6 ++++-- frontend/apps/desktop/src/components/sidebar.tsx | 12 ++++++++---- frontend/packages/ui/src/notification-list-item.tsx | 7 +++++-- frontend/packages/ui/src/resource-page-common.tsx | 6 +++++- 5 files changed, 25 insertions(+), 10 deletions(-) diff --git a/frontend/apps/desktop/src/components/edit-nav-header-pane.tsx b/frontend/apps/desktop/src/components/edit-nav-header-pane.tsx index d87a95e29..1d5de8df6 100644 --- a/frontend/apps/desktop/src/components/edit-nav-header-pane.tsx +++ b/frontend/apps/desktop/src/components/edit-nav-header-pane.tsx @@ -8,7 +8,9 @@ export function EditNavHeaderPane({homeId}: {homeId: UnpackedHypermediaId}) { const {canEdit, beginEditIfNeeded} = useEditorGate() const send = useDocumentSend() const machineNavigation = useDocumentNavigationOptional() - const homeResource = useResource(homeId, {subscribed: true}) + // The home doc is already subscribed by the active resource page; this pane + // only reads cached navigation metadata and does not need its own discovery. + const homeResource = useResource(homeId) const homeDocument = homeResource.data?.type === 'document' ? homeResource.data.document : null if (!canEdit) return null diff --git a/frontend/apps/desktop/src/components/import-doc-button.tsx b/frontend/apps/desktop/src/components/import-doc-button.tsx index 58390ce45..3e8ddc7f6 100644 --- a/frontend/apps/desktop/src/components/import-doc-button.tsx +++ b/frontend/apps/desktop/src/components/import-doc-button.tsx @@ -211,8 +211,10 @@ export function useImporting(parentId: UnpackedHypermediaId) { mutationFn: (url: string) => client.webImporting.checkWebUrl.mutate(url), }) - // Private documents require a site URL and must be at the home doc level - const siteHomeResource = useResource(hmId(parentId.uid), {subscribed: true}) + // Private documents require a site URL and must be at the home doc level. + // The home doc's metadata is already cached by the active resource page or + // joined-site recursive sub; this button only reads the site URL field. + const siteHomeResource = useResource(hmId(parentId.uid)) const siteUrl = siteHomeResource.data?.type === 'document' ? siteHomeResource.data.document?.metadata?.siteUrl : undefined const isHomeDoc = !parentId.path?.length diff --git a/frontend/apps/desktop/src/components/sidebar.tsx b/frontend/apps/desktop/src/components/sidebar.tsx index 65c1cd9f1..24a736641 100644 --- a/frontend/apps/desktop/src/components/sidebar.tsx +++ b/frontend/apps/desktop/src/components/sidebar.tsx @@ -299,9 +299,11 @@ function SubscriptionsSection() { ) : undefined - // Fetch site resources for all joined sites to ensure metadata is available + // Fetch site resources for all joined sites to ensure metadata is available. + // Joined sites are already covered by derived-subscriptions' root recursive + // subscription, so this listing only needs cached metadata for display. const siteIds = siteSubscribed?.map((contact) => hmId(contact.subject)) || [] - const siteResources = useResources(siteIds, {subscribed: true}) + const siteResources = useResources(siteIds) // Sort by activity using the backend's account order (already sorted by activity desc) const accounts = accountList.data?.accounts || [] @@ -482,9 +484,11 @@ function FollowingSection() { ) : undefined - // Fetch profile resources for all followed contacts to ensure metadata is available + // Fetch profile resources for all followed contacts to ensure metadata is + // available. Followed accounts are already covered by derived-subscriptions' + // root recursive subscription, so this listing only needs cached metadata. const profileIds = profileSubscribed?.map((contact) => hmId(contact.subject)) || [] - const profileResources = useResources(profileIds, {subscribed: true}) + const profileResources = useResources(profileIds) // Sort by activity using the backend's account order const accounts = accountList.data?.accounts || [] diff --git a/frontend/packages/ui/src/notification-list-item.tsx b/frontend/packages/ui/src/notification-list-item.tsx index 9464c3d0b..e3d7a3099 100644 --- a/frontend/packages/ui/src/notification-list-item.tsx +++ b/frontend/packages/ui/src/notification-list-item.tsx @@ -25,8 +25,11 @@ export type NotificationListItemProps = { export function NotificationListItem({item, isRead, onOpen, onToggleRead}: NotificationListItemProps) { const authorId = item.author.uid ? hmId(item.author.uid) : null const targetId = item.target.uid ? hmId(item.target.uid, {path: item.target.path ?? undefined}) : null - const author = useAccount(item.author.uid || undefined, {subscribe: true}) - const target = useResource(targetId, {subscribed: true}) + // Notification list rows only need cached metadata for display. Subscribing + // every row in a long list multiplies daemon discovery work; the row's target + // is fetched on-demand when the user opens it. + const author = useAccount(item.author.uid || undefined) + const target = useResource(targetId) const resolvedName = author.data?.metadata?.name const authorName = resolvedName || item.author.name || (item.author.uid ? abbreviateUid(item.author.uid) : undefined) diff --git a/frontend/packages/ui/src/resource-page-common.tsx b/frontend/packages/ui/src/resource-page-common.tsx index 694a3f42c..8846e4521 100644 --- a/frontend/packages/ui/src/resource-page-common.tsx +++ b/frontend/packages/ui/src/resource-page-common.tsx @@ -1004,7 +1004,11 @@ function DocumentBody({ return getBreadcrumbDocumentIds(docId) }, [docId, isHomeDoc]) - const breadcrumbResults = useResources(breadcrumbIds, {subscribed: true}) + // Breadcrumbs only need cached metadata for display; ancestors of docs the user + // cares about are already covered by joined-site recursive subscriptions or by + // the main resource page subscription. Avoid creating one renderer-side + // subscription per ancestor on every page load. + const breadcrumbResults = useResources(breadcrumbIds) const breadcrumbs = useMemo((): BreadcrumbEntry[] | undefined => { if (isHomeDoc) return undefined From 3f5a8f286df484f8c513d699afc0e5021c7ff251 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 8 May 2026 08:41:20 +0000 Subject: [PATCH 2/8] perf(ui): gate embed subscriptions on viewport visibility Embeds in long documents previously each opened an active discovery subscription on mount, regardless of whether the embed was on screen. Documents with many embeds (or recursive comment embeds) multiplied that into many concurrent daemon discovery loops. Add a useInViewport hook that uses IntersectionObserver with a 30 s grace period to avoid churn on rapid scroll, and gate the subscribed flag on each of the three embed renderers (BlockEmbedCard, BlockEmbedContent, BlockEmbedComments) on it. The render path stays identical; only the subscription kicks in when the embed comes into (or near) the viewport. --- frontend/packages/ui/src/embed-views.tsx | 63 +++++++++++++++--- frontend/packages/ui/src/use-in-viewport.ts | 73 +++++++++++++++++++++ 2 files changed, 128 insertions(+), 8 deletions(-) create mode 100644 frontend/packages/ui/src/use-in-viewport.ts diff --git a/frontend/packages/ui/src/embed-views.tsx b/frontend/packages/ui/src/embed-views.tsx index b7ca3b970..887e2f6ca 100644 --- a/frontend/packages/ui/src/embed-views.tsx +++ b/frontend/packages/ui/src/embed-views.tsx @@ -29,6 +29,7 @@ import {DocumentCard} from './newspaper' import {Spinner} from './spinner' import {SizableText} from './text' import {Tooltip} from './tooltip' +import {useInViewport} from './use-in-viewport' /** Props shared by embed block renderers. */ type BlockContentProps = { @@ -105,13 +106,26 @@ export function DeletedEmbedBanner({children, entityLabel = 'document'}: {childr } /** Renders an embedded document as a card view. */ -export function BlockEmbedCard({ +export function BlockEmbedCard(props: BlockContentProps & {openOnClick?: boolean}) { + // Gate active discovery on viewport visibility so off-screen embeds in long + // documents do not each open a discovery loop. Grace period defaults inside + // the hook avoid churn on rapid scroll. + const {ref, isVisible} = useInViewport() + return ( +
+ +
+ ) +} + +function BlockEmbedCardInner({ block, parentBlockId, openOnClick = true, -}: BlockContentProps & {openOnClick?: boolean}) { + subscribed, +}: BlockContentProps & {openOnClick?: boolean; subscribed: boolean}) { const id = unpackHmId(block.link) ?? undefined - const doc = useResource(id, {subscribed: true}) + const doc = useResource(id, {subscribed}) // Check tombstone on latest version for version-pinned embeds. // Version-specific fetches skip the backend's tombstone check. const latestCheckId = id?.version && !id?.latest ? hmId(id.uid, {path: id.path}) : undefined @@ -193,14 +207,34 @@ export function BlockEmbedCard({ } /** Renders full embedded document or comment content. */ -export function BlockEmbedContent({ +export function BlockEmbedContent( + props: BlockContentProps & { + openOnClick?: boolean + renderDocumentContent?: (props: { + embedBlocks: HMBlockNode[] + document: HMDocument | null | undefined + id: UnpackedHypermediaId + }) => React.ReactNode + }, +) { + const {ref, isVisible} = useInViewport() + return ( +
+ +
+ ) +} + +function BlockEmbedContentInner({ block, depth, parentBlockId, openOnClick = true, renderDocumentContent, + subscribed, }: BlockContentProps & { openOnClick?: boolean + subscribed: boolean renderDocumentContent?: (props: { embedBlocks: HMBlockNode[] document: HMDocument | null | undefined @@ -213,7 +247,7 @@ export function BlockEmbedContent({ const [showReferenced, setShowReferenced] = useState(false) const id = unpackHmId(block.link) - const resource = useResource(id, {subscribed: true}) + const resource = useResource(id, {subscribed}) // Check tombstone on latest version for version-pinned embeds. // Version-specific fetches skip the backend's tombstone check. const latestCheckId = id?.version && !id?.latest ? hmId(id.uid, {path: id.path}) : null @@ -315,17 +349,30 @@ export function BlockEmbedContent({ } /** Renders embedded comments section for a document. */ -export function BlockEmbedComments({ +export function BlockEmbedComments(props: BlockContentProps & {openOnClick?: boolean}) { + const {ref, isVisible} = useInViewport() + return ( +
+ +
+ ) +} + +function BlockEmbedCommentsInner({ parentBlockId, block, openOnClick = true, -}: BlockContentProps & {openOnClick?: boolean}) { + subscribed, +}: BlockContentProps & {openOnClick?: boolean; subscribed: boolean}) { const client = useUniversalClient() const id = unpackHmId(block.link) + // recursive is meaningful only when subscribed — when off-screen, no daemon + // sub is created regardless. When the embed scrolls into view we re-subscribe + // recursively so the discussion thread's blobs are discovered as before. const resource = useResource(id, { recursive: true, - subscribed: true, + subscribed, }) // Check tombstone on latest version for version-pinned embeds. const latestCheckId = id?.version && !id?.latest ? hmId(id.uid, {path: id.path}) : null diff --git a/frontend/packages/ui/src/use-in-viewport.ts b/frontend/packages/ui/src/use-in-viewport.ts new file mode 100644 index 000000000..98948fe24 --- /dev/null +++ b/frontend/packages/ui/src/use-in-viewport.ts @@ -0,0 +1,73 @@ +import {RefObject, useEffect, useRef, useState} from 'react' + +type UseInViewportOptions = { + /** rootMargin passed to the underlying IntersectionObserver. */ + rootMargin?: string + /** Keep `isVisible` true for this many ms after the element exits the viewport. */ + graceMs?: number + /** Initial value of `isVisible` before the observer reports. */ + initialVisible?: boolean +} + +/** + * Tracks whether a DOM element is in (or near) the viewport. + * + * Returns a ref to attach to the element being observed and a boolean that + * stays true for `graceMs` after the element scrolls out of view, to prevent + * thrash on rapid scroll. The grace period defaults to 30 seconds, which is + * long enough that quick back-and-forth scrolling does not churn the + * subscription state of consumers. + */ +export function useInViewport({ + rootMargin = '200px', + graceMs = 30_000, + initialVisible = false, +}: UseInViewportOptions = {}): { + ref: RefObject + isVisible: boolean +} { + const ref = useRef(null) + const [isVisible, setIsVisible] = useState(initialVisible) + + useEffect(() => { + const el = ref.current + if (!el || typeof IntersectionObserver === 'undefined') return + + let graceTimer: ReturnType | null = null + const clearGrace = () => { + if (graceTimer) { + clearTimeout(graceTimer) + graceTimer = null + } + } + + const observer = new IntersectionObserver( + (entries) => { + const entry = entries[0] + if (!entry) return + if (entry.isIntersecting) { + clearGrace() + setIsVisible(true) + return + } + if (graceMs <= 0) { + setIsVisible(false) + return + } + clearGrace() + graceTimer = setTimeout(() => { + graceTimer = null + setIsVisible(false) + }, graceMs) + }, + {rootMargin}, + ) + observer.observe(el) + return () => { + observer.disconnect() + clearGrace() + } + }, [rootMargin, graceMs]) + + return {ref, isVisible} +} From 8f0bfe62643380b54df4a9cba4bed83094e451e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 8 May 2026 08:42:14 +0000 Subject: [PATCH 3/8] perf(frontend): normalize mismatched subscription options The renderer-side dedup key in entities.ts is composed of id + recursive + priority + scope, so callers that disagree on those flags for the same entity create separate ref-counted entries and separate tRPC streams. Two cases were forking unnecessarily: - doc-navigation subscribed to the active document with {recursive: true} (key: id/*) while the resource page already subscribes the same id with {recursive: true, priority: 'high'} (key: id/*!high). Drop the redundant subscription; the navigation pane only needs cached outline metadata. - query-block toggled `recursive` based on the display mode ('AllDescendants' vs 'Children'), forking the same query target between id and id/* keys. Always subscribe recursively so the display mode does not influence the dedup key. --- frontend/apps/desktop/src/components/doc-navigation.tsx | 6 +++++- frontend/apps/desktop/src/editor/query-block.tsx | 5 ++++- frontend/packages/editor/src/query-block.tsx | 5 ++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/frontend/apps/desktop/src/components/doc-navigation.tsx b/frontend/apps/desktop/src/components/doc-navigation.tsx index 8fac95981..3e3208cfc 100644 --- a/frontend/apps/desktop/src/components/doc-navigation.tsx +++ b/frontend/apps/desktop/src/components/doc-navigation.tsx @@ -13,7 +13,11 @@ export function DocNavigation({showCollapsed}: {showCollapsed: boolean}) { const route = useNavRoute() if (route.key !== 'document') throw new Error('DocNavigation only supports document route') const {id} = route - const entity = useResource(id, {subscribed: true, recursive: true}) // recursive subscriptions to make sure children get loaded + // The active document route already opens a high-priority recursive + // subscription on this id, so the navigation pane just needs cached metadata. + // Subscribing again here forks the dedup key (no priority vs `!high`) into + // a second renderer-side entry without adding any daemon coverage. + const entity = useResource(id) const navigate = useNavigate('replace') const document = // @ts-ignore diff --git a/frontend/apps/desktop/src/editor/query-block.tsx b/frontend/apps/desktop/src/editor/query-block.tsx index 886d08d96..326183660 100644 --- a/frontend/apps/desktop/src/editor/query-block.tsx +++ b/frontend/apps/desktop/src/editor/query-block.tsx @@ -99,10 +99,13 @@ function Render(block: Block, editor: BlockNoteEditor, editor: BlockNoteEditor Date: Fri, 8 May 2026 08:45:58 +0000 Subject: [PATCH 4/8] perf(desktop): dedup entity subscriptions by id and fix stream cleanup Two related issues with the previous renderer-side subscription ledger: 1) The dedup key combined id + recursive + priority + scope, but the daemon subscribe API only accepts {id, recursive} and ignores priority/scope. Callers that disagreed on priority or scope (e.g. the resource page asking for high priority and an embed asking for normal) created independent ref-counted entries even though they pointed at the same daemon work. 2) Each entry installed its own discovery-state tRPC sub on a shared per-entity stream and overwrote the prior unsubscribe handle. When the first caller cleaned up it would unsubscribe the wrong handle and delete the shared stream entry, leaking the live sub and breaking discovery state for the still-active caller. Replace the multi-key ledger with a per-entity state that tracks the set of active callers (keyed by sub object identity), merges their options to the strongest requested values, and owns a single daemon sub plus a single discovery-state sub. Re-issue the daemon sub only when the merged recursive flag actually changes, so callers can disagree on priority/scope without churn. Removal stays deferred via queueMicrotask so React 18 batched unmount/remount does not flap the daemon sub. --- frontend/apps/desktop/src/models/entities.ts | 209 +++++++++++-------- 1 file changed, 127 insertions(+), 82 deletions(-) diff --git a/frontend/apps/desktop/src/models/entities.ts b/frontend/apps/desktop/src/models/entities.ts index 2cfd8b0c2..5a83f8826 100644 --- a/frontend/apps/desktop/src/models/entities.ts +++ b/frontend/apps/desktop/src/models/entities.ts @@ -136,22 +136,26 @@ export const fetchResource = createResourceFetcher(grpcClient) export const fetchQuery = createQueryResolver(grpcClient) -// Discovery state streams - managed locally but updated via tRPC subscriptions +// Discovery state streams - one per entity ID, persists for the lifetime of +// the renderer process. The live tRPC update subscription is owned by the +// matching entity state (see `entityStates` below) so that multiple callers +// for the same entity share one update stream rather than racing each other. const discoveryStreams = new Map< string, { write: (state: DiscoveryState | null) => void stream: StateStream - unsubscribe?: () => void } >() function getOrCreateDiscoveryStream(entityId: string) { - if (!discoveryStreams.has(entityId)) { + let entry = discoveryStreams.get(entityId) + if (!entry) { const [write, stream] = writeableStateStream(null) - discoveryStreams.set(entityId, {write, stream}) + entry = {write, stream} + discoveryStreams.set(entityId, entry) } - return discoveryStreams.get(entityId)! + return entry } export function getDiscoveryStream(entityId: string): StateStream { @@ -215,20 +219,12 @@ export function cleanupAllEntitySubscriptions() { activeDiscoveriesSubscription = null } - // Clean up all entity subscriptions - for (const key of Object.keys(entitySubscriptions)) { - entitySubscriptions[key]?.unsubscribe() - delete entitySubscriptions[key] + // Tear down per-entity tRPC subs + for (const state of entityStates.values()) { + state.daemonSub?.unsubscribe() + state.discoveryStateSub?.unsubscribe() } - // Reset counts - for (const key of Object.keys(entitySubscriptionCounts)) { - delete entitySubscriptionCounts[key] - } - - // Clean up all discovery streams - discoveryStreams.forEach((entry) => { - entry.unsubscribe?.() - }) + entityStates.clear() discoveryStreams.clear() } @@ -241,9 +237,38 @@ export type EntitySubscription = { scope?: 'all' | 'profile' } -// Entity subscription management - delegates to main process via tRPC -const entitySubscriptions: Record void}> = {} -const entitySubscriptionCounts: Record = {} +// Entity subscription management - dedupes by entity ID across all callers. +// +// Why entity-ID-only (and not also recursive/priority/scope)? +// - The daemon-side `subscribe` tRPC API only accepts `{id, recursive}`, and +// internally collapses non-recursive subs that are covered by a parent +// recursive sub. Priority and scope have no representation on the wire. +// - When the dedup key included priority/scope, two callers for the same +// entity (e.g. the resource page with priority 'high' and an embed with +// priority 'normal') created separate ref-counted entries, separate tRPC +// streams, and — worse — overwrote each other's discovery-state subscription +// handle, leaking one of them and causing the shared stream entry to be +// deleted out from under the still-active caller. +// - Dedup by entity ID lets us run one tRPC sub and one discovery-state sub +// per entity, and merge caller options (currently just `recursive`) up to +// the strongest requested value. The daemon dedups again on its side. +type CallerOptions = { + recursive: boolean + priority: 'normal' | 'high' + scope: 'all' | 'profile' +} + +type EntityState = { + id: UnpackedHypermediaId + /** Map keyed by sub object identity so add/remove of the same caller pair correctly. */ + callers: Map + daemonSub: {unsubscribe: () => void} | null + discoveryStateSub: {unsubscribe: () => void} | null + /** The recursive value currently applied to the daemon sub. */ + currentRecursive: boolean +} + +const entityStates = new Map() /** Stream of current subscription display keys for the footer panel. */ const [writeSubscriptionKeys, subscriptionKeysStream] = writeableStateStream([]) @@ -254,42 +279,47 @@ export function getSubscriptionKeysStream(): StateStream { } function emitSubscriptionKeys() { - const keys = new Set() - for (const key of Object.keys(entitySubscriptionCounts)) { - if (entitySubscriptionCounts[key] > 0) { - // Strip !high priority suffix — it's an internal detail, not relevant for display - const displayKey = key.replace('!high', '') - keys.add(displayKey) - } + const keys: string[] = [] + for (const state of entityStates.values()) { + if (state.callers.size === 0) continue + keys.push(state.id.id + (state.currentRecursive ? '/*' : '')) } - const sorted = Array.from(keys).sort() - writeSubscriptionKeys(sorted) + keys.sort() + writeSubscriptionKeys(keys) } -function getEntitySubscriptionKey(sub: EntitySubscription) { - const {id, recursive} = sub - if (!id) return null - // Priority is part of the key so a normal-priority sub doesn't shadow a - // high-priority one (or vice versa) when both are added concurrently. - const priorityKey = sub.priority === 'high' ? '!high' : '' - const scopeKey = sub.scope === 'profile' ? ':profile' : '' - return id.id + (recursive ? '/*' : '') + priorityKey + scopeKey +function mergeCallerOptions(callers: Iterable): {recursive: boolean} { + let recursive = false + for (const c of callers) { + if (c.recursive) { + recursive = true + break + } + } + return {recursive} } -export function addSubscribedEntity(sub: EntitySubscription) { - const key = getEntitySubscriptionKey(sub) - if (!key || !sub.id) return +function syncEntityState(state: EntityState) { + if (state.callers.size === 0) { + state.daemonSub?.unsubscribe() + state.daemonSub = null + state.discoveryStateSub?.unsubscribe() + state.discoveryStateSub = null + entityStates.delete(state.id.id) + return + } - const currentCount = entitySubscriptionCounts[key] || 0 - entitySubscriptionCounts[key] = currentCount + 1 + const merged = mergeCallerOptions(state.callers.values()) - // Only create subscription on first reference - if (currentCount === 0) { - // Subscribe via tRPC to the main process - const subscription = client.sync.subscribe.subscribe( + if (!state.daemonSub || state.currentRecursive !== merged.recursive) { + // (Re)issue the daemon sub when the merged options change. This is the + // only place we touch the daemon sub, so we never end up with two streams + // racing for the same entity. + state.daemonSub?.unsubscribe() + state.daemonSub = client.sync.subscribe.subscribe( { - id: sub.id, - recursive: sub.recursive, + id: state.id, + recursive: merged.recursive, }, { onData: () => { @@ -297,45 +327,60 @@ export function addSubscribedEntity(sub: EntitySubscription) { }, }, ) - entitySubscriptions[key] = subscription + state.currentRecursive = merged.recursive + } - // Also subscribe to discovery state updates for this entity - const discoveryStreamEntry = getOrCreateDiscoveryStream(sub.id.id) - const discoveryStateSub = client.sync.discoveryState.subscribe(sub.id.id, { - onData: (state) => { - discoveryStreamEntry.write(state) - }, + if (!state.discoveryStateSub) { + const stream = getOrCreateDiscoveryStream(state.id.id) + const sub = client.sync.discoveryState.subscribe(state.id.id, { + onData: (s) => stream.write(s), }) - discoveryStreamEntry.unsubscribe = () => discoveryStateSub.unsubscribe() + state.discoveryStateSub = sub + } +} + +export function addSubscribedEntity(sub: EntitySubscription) { + if (!sub.id) return + const entityId = sub.id.id + + let state = entityStates.get(entityId) + if (!state) { + state = { + id: sub.id, + callers: new Map(), + daemonSub: null, + discoveryStateSub: null, + currentRecursive: false, + } + entityStates.set(entityId, state) } + state.callers.set(sub, { + recursive: !!sub.recursive, + priority: sub.priority || 'normal', + scope: sub.scope || 'all', + }) + + syncEntityState(state) emitSubscriptionKeys() } export function removeSubscribedEntity(sub: EntitySubscription) { - const key = getEntitySubscriptionKey(sub) - if (!key) return - if (!entitySubscriptionCounts[key]) return - - entitySubscriptionCounts[key] = entitySubscriptionCounts[key] - 1 - if (entitySubscriptionCounts[key] === 0) { - queueMicrotask(() => { - // microtask lets React 18 batch cleanup+setup in the same commit, - // so rapid unmount/remount won't trigger unnecessary unsubscribe+resubscribe. - // Unlike the previous 300ms setTimeout, this completes before window destruction. - if (entitySubscriptionCounts[key] === 0) { - entitySubscriptions[key]?.unsubscribe() - delete entitySubscriptions[key] - - // Also cleanup discovery state subscription AND delete from map to prevent memory leak - if (sub.id) { - const discoveryStreamEntry = discoveryStreams.get(sub.id.id) - discoveryStreamEntry?.unsubscribe?.() - discoveryStreams.delete(sub.id.id) - } - - emitSubscriptionKeys() - } - }) - } + if (!sub.id) return + const entityId = sub.id.id + const state = entityStates.get(entityId) + if (!state) return + + state.callers.delete(sub) + + // Defer the actual sync so React 18 can batch cleanup+setup in the same + // commit without churning the daemon subscription on rapid unmount/remount. + // If a new caller is added before the microtask runs, the current sub stays + // in place; if no caller arrives, the microtask tears it down. + queueMicrotask(() => { + const cur = entityStates.get(entityId) + if (!cur) return + syncEntityState(cur) + emitSubscriptionKeys() + }) } From 7bea70e13cbe0bd86bd12bc13fad9eb4ed9c1583 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 8 May 2026 08:48:30 +0000 Subject: [PATCH 5/8] perf(desktop): pause entity subs while the window is blurred MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a renderer window is not focused, its callers are by definition not consuming fresh state — but the daemon kept polling discovery for every subscribed entity at the same cadence (just with the global 10x slowdown). With many windows or many subscriptions per window, that turns into substantial idle work. Track the focus state of the local renderer window via the standard focus/blur events, and on a sustained blur (30 s grace) tear down all of this window's outbound entity subs and discovery-state streams. The state and caller list are preserved, so when the window regains focus we re-issue the subs without losing any callers. The grace period absorbs quick alt-tabs without churning the daemon. --- frontend/apps/desktop/src/models/entities.ts | 65 +++++++++++++++++++ .../apps/desktop/src/models/window-focus.ts | 47 ++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 frontend/apps/desktop/src/models/window-focus.ts diff --git a/frontend/apps/desktop/src/models/entities.ts b/frontend/apps/desktop/src/models/entities.ts index 5a83f8826..99b8d32a6 100644 --- a/frontend/apps/desktop/src/models/entities.ts +++ b/frontend/apps/desktop/src/models/entities.ts @@ -15,6 +15,7 @@ import {hmIdPathToEntityQueryPath} from '@shm/shared/utils/path-api' import {StateStream, writeableStateStream} from '@shm/shared/utils/stream' import {useMutation, UseMutationOptions, useQuery} from '@tanstack/react-query' import {usePushResource} from './documents' +import {isThisWindowFocused, onThisWindowFocusChange} from './window-focus' type DeleteEntitiesInput = { ids: UnpackedHypermediaId[] @@ -270,6 +271,14 @@ type EntityState = { const entityStates = new Map() +// Per-window pause state: when this renderer window has been blurred for +// longer than BLUR_PAUSE_GRACE_MS we tear down all of its outbound daemon +// subscriptions; on focus we re-issue them. The grace window absorbs quick +// alt-tabs without churning the daemon. +const BLUR_PAUSE_GRACE_MS = 30_000 +let blurPauseTimer: ReturnType | null = null +let isPaused = false + /** Stream of current subscription display keys for the footer panel. */ const [writeSubscriptionKeys, subscriptionKeysStream] = writeableStateStream([]) @@ -309,6 +318,20 @@ function syncEntityState(state: EntityState) { return } + if (isPaused) { + // Window-blurred: tear down outbound subs but keep callers and state so we + // can resume on focus without losing the caller list. + if (state.daemonSub) { + state.daemonSub.unsubscribe() + state.daemonSub = null + } + if (state.discoveryStateSub) { + state.discoveryStateSub.unsubscribe() + state.discoveryStateSub = null + } + return + } + const merged = mergeCallerOptions(state.callers.values()) if (!state.daemonSub || state.currentRecursive !== merged.recursive) { @@ -339,6 +362,48 @@ function syncEntityState(state: EntityState) { } } +function syncAllEntityStates() { + // Iterate over a snapshot — syncEntityState may delete from entityStates + // when a state has zero callers. + for (const state of Array.from(entityStates.values())) { + syncEntityState(state) + } +} + +// Wire window focus to the pause flag. Module-level so the listener registers +// once per renderer process. Initial value reflects whether the window is +// focused at module load time. +if (typeof window !== 'undefined') { + if (!isThisWindowFocused()) { + // Started blurred — schedule the same grace timer we would on a transition. + blurPauseTimer = setTimeout(() => { + blurPauseTimer = null + isPaused = true + syncAllEntityStates() + }, BLUR_PAUSE_GRACE_MS) + } + onThisWindowFocusChange((focused) => { + if (focused) { + if (blurPauseTimer) { + clearTimeout(blurPauseTimer) + blurPauseTimer = null + } + if (isPaused) { + isPaused = false + syncAllEntityStates() + } + return + } + if (blurPauseTimer) clearTimeout(blurPauseTimer) + blurPauseTimer = setTimeout(() => { + blurPauseTimer = null + if (isPaused) return + isPaused = true + syncAllEntityStates() + }, BLUR_PAUSE_GRACE_MS) + }) +} + export function addSubscribedEntity(sub: EntitySubscription) { if (!sub.id) return const entityId = sub.id.id diff --git a/frontend/apps/desktop/src/models/window-focus.ts b/frontend/apps/desktop/src/models/window-focus.ts new file mode 100644 index 000000000..0e6793063 --- /dev/null +++ b/frontend/apps/desktop/src/models/window-focus.ts @@ -0,0 +1,47 @@ +/** + * Renderer-side window focus tracking. + * + * Each Electron window has its own focus state (independent of whether other + * Seed windows are focused). When the local window loses focus we pause its + * outbound entity subscriptions so the daemon does not keep polling discovery + * for documents the user is not actively looking at; when the window regains + * focus we resume them. A grace period absorbs quick focus-stealing events so + * a brief alt-tab does not churn the daemon. + */ + +const FOCUSED_AT_LOAD = + typeof document !== 'undefined' && typeof document.hasFocus === 'function' ? document.hasFocus() : true + +let windowFocused = FOCUSED_AT_LOAD + +const focusListeners = new Set<(focused: boolean) => void>() + +if (typeof window !== 'undefined') { + window.addEventListener('focus', () => { + if (windowFocused) return + windowFocused = true + focusListeners.forEach((l) => l(true)) + }) + window.addEventListener('blur', () => { + if (!windowFocused) return + windowFocused = false + focusListeners.forEach((l) => l(false)) + }) +} + +/** Returns the current focus state of this Electron renderer window. */ +export function isThisWindowFocused(): boolean { + return windowFocused +} + +/** + * Subscribes to focus transitions of this window. The handler receives `true` + * when the window gains focus and `false` when it loses focus. Returns a + * cleanup callback. + */ +export function onThisWindowFocusChange(handler: (focused: boolean) => void): () => void { + focusListeners.add(handler) + return () => { + focusListeners.delete(handler) + } +} From 162349f4374c2cac33ffda97b65a908c510327d9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 8 May 2026 08:49:48 +0000 Subject: [PATCH 6/8] perf(desktop): cap concurrent discovery RPCs in the sync loop Previously every subscription's discoveryLoop fired its discoverEntity RPC independently, so when many subs reach a polling tick close together (typical at app startup or after focus regain) the renderer issues a burst of RPCs to the daemon. The daemon has its own worker pool, but stacked tRPC + grpc-client overhead on the renderer side adds up. Add a small FIFO slot pool sized to MAX_CONCURRENT_DISCOVERY=4 around runDiscovery so additional RPCs queue rather than pile in flight. Re-check cancellation before consuming a slot so that a sub which was unsubscribed while waiting does not still hit the wire. --- frontend/apps/desktop/src/app-sync.ts | 35 ++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/frontend/apps/desktop/src/app-sync.ts b/frontend/apps/desktop/src/app-sync.ts index ea6ecb76c..1df166f7e 100644 --- a/frontend/apps/desktop/src/app-sync.ts +++ b/frontend/apps/desktop/src/app-sync.ts @@ -81,6 +81,34 @@ const DISCOVERY_POLL_INTERVAL_MS = 14_000 const ACTIVITY_POLL_INTERVAL_MS = 1_000 const DELETED_POLL_INTERVAL_MS = 60_000 // Slower polling for deleted/redirected resources +// Cap concurrent in-flight discovery RPCs. The backend has its own worker pool +// (config.MaxWorkers, default 6), but capping on this side keeps grpc-client +// and tRPC overhead bounded when many subs reach a polling tick at once. +const MAX_CONCURRENT_DISCOVERY = 4 +let discoveryInFlight = 0 +const discoveryQueue: Array<() => void> = [] + +/** Runs `fn` immediately if a slot is free, otherwise queues it FIFO. */ +function withDiscoverySlot(fn: () => Promise): Promise { + return new Promise((resolve, reject) => { + const run = () => { + discoveryInFlight++ + fn() + .then(resolve, reject) + .finally(() => { + discoveryInFlight-- + const next = discoveryQueue.shift() + if (next) next() + }) + } + if (discoveryInFlight < MAX_CONCURRENT_DISCOVERY) { + run() + } else { + discoveryQueue.push(run) + } + }) +} + /** Returns 1 when app is focused, 10 when backgrounded. */ function getPollingMultiplier(): number { return isAnyWindowFocused() ? 1 : 10 @@ -825,7 +853,12 @@ function createSubscription(sub: ResourceSubscription): SubscriptionState { return } - runDiscovery(sub) + withDiscoverySlot(async () => { + // Re-check cancellation before consuming a slot — the loop may have been + // unsubscribed while waiting in the queue. + if (cancelled) return null + return runDiscovery(sub) + }) .then((result) => { if (cancelled) return From 597d9e72fe824d6c8d4660f169d5f8d1de9a2ae4 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 8 May 2026 08:55:45 +0000 Subject: [PATCH 7/8] test(desktop): cover entity subscription dedup and blur-pause Add targeted Vitest unit tests for the entity subscription ledger: - two callers on the same entity with different priorities open one daemon sub and one discovery-state sub - a recursive caller arriving later upgrades the merged daemon sub - removing the recursive caller downgrades it back - the discovery-state sub stays alive while any caller remains - rapid unmount/remount of a single caller does not churn the daemon - separate entity ids are tracked independently Plus two tests for the blur-pause behaviour: subs are torn down once the grace timer expires after a blur and re-issued on focus, and the grace timer is cancelled when focus returns before it fires. --- .../__tests__/entities.subscriptions.test.ts | 208 ++++++++++++++++++ 1 file changed, 208 insertions(+) create mode 100644 frontend/apps/desktop/src/models/__tests__/entities.subscriptions.test.ts diff --git a/frontend/apps/desktop/src/models/__tests__/entities.subscriptions.test.ts b/frontend/apps/desktop/src/models/__tests__/entities.subscriptions.test.ts new file mode 100644 index 000000000..703a488f4 --- /dev/null +++ b/frontend/apps/desktop/src/models/__tests__/entities.subscriptions.test.ts @@ -0,0 +1,208 @@ +import {beforeEach, describe, expect, it, vi} from 'vitest' +import {hmId} from '@shm/shared/utils/entity-id-url' + +// vi.mock is hoisted, so its factory is evaluated before the rest of this file +// runs. Declare the mock fns via vi.hoisted so they are available to the +// factory while still being stable references the test cases can reset. +const {subscribeMock, discoveryStateSubscribeMock, focusHandlers} = vi.hoisted(() => ({ + subscribeMock: vi.fn(), + discoveryStateSubscribeMock: vi.fn(), + focusHandlers: [] as Array<(focused: boolean) => void>, +})) + +vi.mock('@/grpc-client', () => ({grpcClient: {}})) + +vi.mock('@/trpc', () => ({ + client: { + sync: { + subscribe: {subscribe: subscribeMock}, + discoveryState: {subscribe: discoveryStateSubscribeMock}, + }, + }, +})) + +vi.mock('../documents', () => ({usePushResource: () => vi.fn()})) + +vi.mock('../window-focus', () => ({ + isThisWindowFocused: () => true, + onThisWindowFocusChange: (handler: (focused: boolean) => void) => { + focusHandlers.push(handler) + return () => { + const idx = focusHandlers.indexOf(handler) + if (idx >= 0) focusHandlers.splice(idx, 1) + } + }, +})) + +import {addSubscribedEntity, cleanupAllEntitySubscriptions, removeSubscribedEntity} from '../entities' + +type Handle = {unsubscribe: ReturnType} + +describe('entity subscription dedup', () => { + let daemonHandles: Handle[] + let discoveryHandles: Handle[] + + beforeEach(() => { + cleanupAllEntitySubscriptions() + subscribeMock.mockReset() + discoveryStateSubscribeMock.mockReset() + daemonHandles = [] + discoveryHandles = [] + subscribeMock.mockImplementation(() => { + const handle: Handle = {unsubscribe: vi.fn()} + daemonHandles.push(handle) + return handle + }) + discoveryStateSubscribeMock.mockImplementation(() => { + const handle: Handle = {unsubscribe: vi.fn()} + discoveryHandles.push(handle) + return handle + }) + }) + + it('opens a single daemon sub for two callers on the same entity with different priorities', () => { + const id = hmId('alice', {path: ['doc']}) + addSubscribedEntity({id, recursive: true, priority: 'high'}) + addSubscribedEntity({id, recursive: true, priority: 'normal'}) + + expect(subscribeMock).toHaveBeenCalledTimes(1) + expect(subscribeMock).toHaveBeenCalledWith({id, recursive: true}, expect.anything()) + expect(discoveryStateSubscribeMock).toHaveBeenCalledTimes(1) + }) + + it('merges options up: a recursive caller arriving later upgrades the daemon sub', () => { + const id = hmId('alice', {path: ['doc']}) + addSubscribedEntity({id, recursive: false}) + expect(subscribeMock).toHaveBeenLastCalledWith({id, recursive: false}, expect.anything()) + + addSubscribedEntity({id, recursive: true}) + expect(subscribeMock).toHaveBeenCalledTimes(2) + expect(subscribeMock).toHaveBeenLastCalledWith({id, recursive: true}, expect.anything()) + // Old non-recursive sub torn down when re-issued. + expect(daemonHandles[0]!.unsubscribe).toHaveBeenCalledTimes(1) + expect(daemonHandles[1]!.unsubscribe).not.toHaveBeenCalled() + }) + + it('does not churn the daemon sub on rapid unmount/remount of a single caller', async () => { + const id = hmId('alice', {path: ['doc']}) + const oldSub = {id} + const newSub = {id} + + addSubscribedEntity(oldSub) + removeSubscribedEntity(oldSub) + addSubscribedEntity(newSub) + // Flush the queueMicrotask that defers the removal sync. + await Promise.resolve() + await Promise.resolve() + + expect(subscribeMock).toHaveBeenCalledTimes(1) + expect(daemonHandles[0]!.unsubscribe).not.toHaveBeenCalled() + }) + + it('keeps the shared discovery-state sub alive until the last caller leaves', async () => { + const id = hmId('alice', {path: ['doc']}) + const subA = {id, priority: 'high' as const} + const subB = {id, priority: 'normal' as const} + + addSubscribedEntity(subA) + addSubscribedEntity(subB) + expect(discoveryStateSubscribeMock).toHaveBeenCalledTimes(1) + + removeSubscribedEntity(subA) + await Promise.resolve() + await Promise.resolve() + // B is still active, the shared discovery sub must NOT have been torn down. + expect(discoveryHandles[0]!.unsubscribe).not.toHaveBeenCalled() + + removeSubscribedEntity(subB) + await Promise.resolve() + await Promise.resolve() + expect(discoveryHandles[0]!.unsubscribe).toHaveBeenCalledTimes(1) + expect(daemonHandles[0]!.unsubscribe).toHaveBeenCalledTimes(1) + }) + + it('downgrades the merged options when the recursive caller leaves', async () => { + const id = hmId('alice', {path: ['doc']}) + const recSub = {id, recursive: true} + const nonRecSub = {id, recursive: false} + + addSubscribedEntity(nonRecSub) + addSubscribedEntity(recSub) + expect(subscribeMock).toHaveBeenCalledTimes(2) + // After the recursive caller arrives, current sub is recursive: true. + + removeSubscribedEntity(recSub) + await Promise.resolve() + await Promise.resolve() + + // Should have re-issued back to recursive: false now that only non-rec caller remains. + expect(subscribeMock).toHaveBeenCalledTimes(3) + expect(subscribeMock).toHaveBeenLastCalledWith({id, recursive: false}, expect.anything()) + }) + + it('tracks separate entity ids independently', () => { + const aliceDoc = hmId('alice', {path: ['doc']}) + const bobDoc = hmId('bob', {path: ['doc']}) + addSubscribedEntity({id: aliceDoc}) + addSubscribedEntity({id: bobDoc}) + + expect(subscribeMock).toHaveBeenCalledTimes(2) + expect(discoveryStateSubscribeMock).toHaveBeenCalledTimes(2) + }) +}) + +describe('window blur pause', () => { + beforeEach(() => { + cleanupAllEntitySubscriptions() + subscribeMock.mockReset() + discoveryStateSubscribeMock.mockReset() + subscribeMock.mockImplementation(() => ({unsubscribe: vi.fn()})) + discoveryStateSubscribeMock.mockImplementation(() => ({unsubscribe: vi.fn()})) + }) + + it('tears down daemon subs after the grace period and re-issues on focus', () => { + vi.useFakeTimers() + try { + const id = hmId('alice', {path: ['doc']}) + addSubscribedEntity({id}) + expect(subscribeMock).toHaveBeenCalledTimes(1) + const initialDaemonHandle = subscribeMock.mock.results[0]!.value as {unsubscribe: ReturnType} + + // Simulate window blur. Grace timer should be scheduled but not yet fired. + focusHandlers.forEach((h) => h(false)) + vi.advanceTimersByTime(29_000) + expect(initialDaemonHandle.unsubscribe).not.toHaveBeenCalled() + + // Past the grace window the daemon sub should be torn down. + vi.advanceTimersByTime(2_000) + expect(initialDaemonHandle.unsubscribe).toHaveBeenCalledTimes(1) + + // Regaining focus re-issues the daemon sub. + focusHandlers.forEach((h) => h(true)) + expect(subscribeMock).toHaveBeenCalledTimes(2) + } finally { + vi.useRealTimers() + } + }) + + it('cancels the grace timer when focus returns before it fires', () => { + vi.useFakeTimers() + try { + const id = hmId('alice', {path: ['doc']}) + addSubscribedEntity({id}) + const handle = subscribeMock.mock.results[0]!.value as {unsubscribe: ReturnType} + + focusHandlers.forEach((h) => h(false)) + vi.advanceTimersByTime(10_000) + focusHandlers.forEach((h) => h(true)) + // Advance past where the timer would have fired without the cancel. + vi.advanceTimersByTime(60_000) + + expect(handle.unsubscribe).not.toHaveBeenCalled() + // No unnecessary re-issue either, since we never actually paused. + expect(subscribeMock).toHaveBeenCalledTimes(1) + } finally { + vi.useRealTimers() + } + }) +}) From 7dd79e406b0febc6c0c5432338058183f18029c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 8 May 2026 09:00:26 +0000 Subject: [PATCH 8/8] fix(desktop): use Map.forEach in entity ledger for downlevel TS target The desktop tsconfig has no explicit target so emits ES3 + lib ES2020, which means MapIterator and Iterable values cannot be iterated with for-of without --downlevelIteration. Switch the new entity-ledger iteration sites (cleanup, syncAllEntityStates, emitSubscriptionKeys) to Map.forEach and pass the Map itself into mergeCallerOptions so typecheck stays happy without enabling downlevelIteration globally. --- frontend/apps/desktop/src/models/entities.ts | 33 ++++++++++---------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/frontend/apps/desktop/src/models/entities.ts b/frontend/apps/desktop/src/models/entities.ts index 99b8d32a6..59a9d23c0 100644 --- a/frontend/apps/desktop/src/models/entities.ts +++ b/frontend/apps/desktop/src/models/entities.ts @@ -221,10 +221,10 @@ export function cleanupAllEntitySubscriptions() { } // Tear down per-entity tRPC subs - for (const state of entityStates.values()) { + entityStates.forEach((state) => { state.daemonSub?.unsubscribe() state.discoveryStateSub?.unsubscribe() - } + }) entityStates.clear() discoveryStreams.clear() } @@ -289,22 +289,19 @@ export function getSubscriptionKeysStream(): StateStream { function emitSubscriptionKeys() { const keys: string[] = [] - for (const state of entityStates.values()) { - if (state.callers.size === 0) continue + entityStates.forEach((state) => { + if (state.callers.size === 0) return keys.push(state.id.id + (state.currentRecursive ? '/*' : '')) - } + }) keys.sort() writeSubscriptionKeys(keys) } -function mergeCallerOptions(callers: Iterable): {recursive: boolean} { +function mergeCallerOptions(callers: Map): {recursive: boolean} { let recursive = false - for (const c of callers) { - if (c.recursive) { - recursive = true - break - } - } + callers.forEach((c) => { + if (c.recursive) recursive = true + }) return {recursive} } @@ -332,7 +329,7 @@ function syncEntityState(state: EntityState) { return } - const merged = mergeCallerOptions(state.callers.values()) + const merged = mergeCallerOptions(state.callers) if (!state.daemonSub || state.currentRecursive !== merged.recursive) { // (Re)issue the daemon sub when the merged options change. This is the @@ -363,10 +360,12 @@ function syncEntityState(state: EntityState) { } function syncAllEntityStates() { - // Iterate over a snapshot — syncEntityState may delete from entityStates - // when a state has zero callers. - for (const state of Array.from(entityStates.values())) { - syncEntityState(state) + // Snapshot first — syncEntityState may delete from entityStates when a state + // has zero callers, which would invalidate a live iterator. + const snapshot: EntityState[] = [] + entityStates.forEach((state) => snapshot.push(state)) + for (let i = 0; i < snapshot.length; i++) { + syncEntityState(snapshot[i]!) } }