Memoize derived thread reads#1908
Conversation
- Extract shared thread derivation - Add existence selector for route checks - Cover memoization behavior with tests
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Removed effect breaks diff content persistence after close
- Restored a useEffect that watches
diffOpenand setshasOpenedDiff(true)whenever the diff becomes open, ensuring the DiffPanel stays mounted after close regardless of how the diff was opened.
- Restored a useEffect that watches
Or push these changes by commenting:
@cursor push f0ed1ba77a
Preview (f0ed1ba77a)
diff --git a/apps/web/src/routes/_chat.$environmentId.$threadId.tsx b/apps/web/src/routes/_chat.$environmentId.$threadId.tsx
--- a/apps/web/src/routes/_chat.$environmentId.$threadId.tsx
+++ b/apps/web/src/routes/_chat.$environmentId.$threadId.tsx
@@ -204,11 +204,15 @@
search: { diff: undefined },
});
}, [navigate, threadRef]);
+ useEffect(() => {
+ if (diffOpen) {
+ setHasOpenedDiff(true);
+ }
+ }, [diffOpen]);
const openDiff = useCallback(() => {
if (!threadRef) {
return;
}
- setHasOpenedDiff(true);
void navigate({
to: "/$environmentId/$threadId",
params: buildThreadRouteParams(threadRef),You can send follow-ups to the cloud agent here.
ApprovabilityVerdict: Approved This is a performance optimization refactor that centralizes thread derivation logic into a new module with WeakMap-based memoization. The changes are well-tested, functionally equivalent to existing behavior, and don't introduce new features or runtime behavior changes. You can customize Macroscope's approvability policy. Learn more. |
- Track diff-open state per thread key - Notify route when chat opens the diff panel
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Four exported functions are never imported externally
- Removed the
exportkeyword fromselectThreadMessages,selectThreadActivities,selectThreadProposedPlans, andselectThreadTurnDiffSummariessince they are only used internally withinthreadDerivation.ts.
- Removed the
Or push these changes by commenting:
@cursor push 9176389c68
Preview (9176389c68)
diff --git a/apps/web/src/threadDerivation.ts b/apps/web/src/threadDerivation.ts
--- a/apps/web/src/threadDerivation.ts
+++ b/apps/web/src/threadDerivation.ts
@@ -60,10 +60,7 @@
return nextValues;
}
-export function selectThreadMessages(
- state: EnvironmentState,
- threadId: ThreadId,
-): Thread["messages"] {
+function selectThreadMessages(state: EnvironmentState, threadId: ThreadId): Thread["messages"] {
return collectByIds(
state.messageIdsByThreadId[threadId],
state.messageByThreadId[threadId] ?? EMPTY_MESSAGE_MAP,
@@ -71,10 +68,7 @@
);
}
-export function selectThreadActivities(
- state: EnvironmentState,
- threadId: ThreadId,
-): Thread["activities"] {
+function selectThreadActivities(state: EnvironmentState, threadId: ThreadId): Thread["activities"] {
return collectByIds(
state.activityIdsByThreadId[threadId],
state.activityByThreadId[threadId] ?? EMPTY_ACTIVITY_MAP,
@@ -82,7 +76,7 @@
);
}
-export function selectThreadProposedPlans(
+function selectThreadProposedPlans(
state: EnvironmentState,
threadId: ThreadId,
): Thread["proposedPlans"] {
@@ -93,7 +87,7 @@
);
}
-export function selectThreadTurnDiffSummaries(
+function selectThreadTurnDiffSummaries(
state: EnvironmentState,
threadId: ThreadId,
): Thread["turnDiffSummaries"] {You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 594dd8f. Configure here.
…rivation.ts Applied via @cursor push command
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>


Summary
threadDerivation.tsto reduce duplication between store selectors and store updates.Testing
apps/web/src/store.test.tscoverage for memoized thread derivation and existence checks.bun fmtbun lintbun typecheckbun run testNote
Medium Risk
Introduces new WeakMap-based memoization for derived
Threadobjects and refactors selectors/store updates to use it, which could subtly affect referential equality and UI update behavior. Route-level diff panel mounting logic also changes state tracking per-thread, so regressions would mainly show up as stale/incorrect renders rather than data corruption.Overview
Memoizes derived thread reads by centralizing thread derivation in
getThreadFromEnvironmentState(newthreadDerivation.ts) and caching derivedThreadobjects/collected arrays to keep references stable when underlying environment state is unchanged.Updates
store.tsandstoreSelectors.tsto use the shared derivation helper, adds a lightweightselectThreadExistsByReffor existence checks without materializing threads, and adds tests instore.test.tscovering stability, invalidation, and existence.Tweaks the chat thread route/diff UX: tracks “diff panel has opened” per thread key, passes
onDiffPanelOpenintoChatView, and triggers it when opening/toggling diffs so diff content can stay mounted after first open.Reviewed by Cursor Bugbot for commit 081d29b. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Memoize derived thread reads to avoid redundant recomputation
getThreadFromEnvironmentStatein threadDerivation.ts, which uses aWeakMapkeyed on the thread shell to cache the fully composedThreadobject, returning the same reference when inputs are unchanged.collectByIdshelper that caches derived arrays for messages, activities, proposed plans, and turn diff summaries usingWeakMaps.createScopedThreadSelectorin storeSelectors.ts to trackEnvironmentStateandthreadIdinstead of individual per-thread slices, delegating derivation togetThreadFromEnvironmentState.selectThreadExistsByRefselector in store.ts to check thread existence without materializing the full derived thread.onDiffPanelOpencallback onChatView, avoiding unnecessary re-renders tied todiffOpen.Macroscope summarized 081d29b.