fix: resolve blank merge preview when selecting many records#19065
fix: resolve blank merge preview when selecting many records#19065jnMetaCode wants to merge 1 commit intotwentyhq:mainfrom
Conversation
The merge preview useEffect had two issues causing blank renders with multiple records: 1. isInitialized state flag prevented retries after initial fetch, even if the fetch failed silently (errorPolicy: 'ignore' swallows GraphQL errors). Once set to true, the preview could never recover. 2. isGeneratingPreview as React state was subject to stale closures. With many records, selectedRecords reference changes more frequently (each record atom update triggers a new array), causing the effect to re-run with stale guard values. Fix replaces isInitialized with a stable selectedRecordIds string dependency (sorted, joined IDs), uses a ref for the in-flight guard to avoid stale closures, and passes selectedRecords as a parameter to the fetch callback. Fixes twentyhq#18214
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Welcome!
Hello there, congrats on your first PR! We're excited to have you contributing to this project. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-front/src/modules/object-record/record-merge/hooks/usePerformMergePreview.ts">
<violation number="1" location="packages/twenty-front/src/modules/object-record/record-merge/hooks/usePerformMergePreview.ts:79">
P1: Selection updates that occur during an in-flight merge preview request can be dropped, causing stale preview data to persist.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| if ( | ||
| selectedRecords.length >= 2 && | ||
| !isMergeInProgress && | ||
| !isGeneratingPreviewRef.current |
There was a problem hiding this comment.
P1: Selection updates that occur during an in-flight merge preview request can be dropped, causing stale preview data to persist.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/object-record/record-merge/hooks/usePerformMergePreview.ts, line 79:
<comment>Selection updates that occur during an in-flight merge preview request can be dropped, causing stale preview data to persist.</comment>
<file context>
@@ -63,23 +65,23 @@ export const usePerformMergePreview = ({
+ if (
+ selectedRecords.length >= 2 &&
+ !isMergeInProgress &&
+ !isGeneratingPreviewRef.current
+ ) {
+ fetchPreview(selectedRecords);
</file context>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| !isMergeInProgress && | ||
| !isGeneratingPreviewRef.current | ||
| ) { | ||
| fetchPreview(selectedRecords); | ||
| } | ||
| }, [ | ||
| selectedRecords, | ||
| mergeSettings, | ||
| isMergeInProgress, | ||
| isGeneratingPreview, | ||
| mergeManyRecords, | ||
| upsertRecordsInStore, | ||
| isInitialized, | ||
| ]); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [selectedRecordIds, mergeSettings, isMergeInProgress, fetchPreview]); | ||
|
|
||
| return { | ||
| mergePreviewRecord, |
There was a problem hiding this comment.
Bug: Changing mergeSettings while a preview is generating will not trigger a refresh. The UI will show stale data from the previous settings due to a missing dependency.
Severity: MEDIUM
Suggested Fix
Re-introduce a dependency that changes when the preview generation completes. The previous implementation used isGeneratingPreview in the dependency array. Adding it back would ensure the effect re-runs after a fetch completes, picking up any mergeSettings changes that occurred during the fetch.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
packages/twenty-front/src/modules/object-record/record-merge/hooks/usePerformMergePreview.ts#L78-L87
Potential issue: Removing `isGeneratingPreview` from the `useEffect` dependency array
introduces a race condition. If `mergeSettings` are changed while a preview fetch is
in-flight, the effect's early return (`isGeneratingPreviewRef.current === true`)
prevents a new fetch. When the current fetch completes and sets `isGeneratingPreview` to
`false`, the component re-renders, but the effect dependencies have not changed since
the last run. The effect does not re-run, and the UI continues to display a stale
preview based on the old settings. This can lead to users performing a merge based on
incorrect preview data.
Did we get this right? 👍 / 👎 to inform future reviews.
2 participants
Summary
Fixes #18214 — the "Merge preview" tab stays blank when selecting many records (e.g. 5) for merging.
Root cause: The
usePerformMergePreviewhook had two issues:isInitializedflag prevented retries — Once set totrueafter the first fetch attempt (success or failure), the preview could never re-fetch. With many records, theselectedRecordsarray reference changes more frequently (each underlying record atom update creates a new array from the Jotai selector). If the initial fetch ran with stale/incomplete data or failed silently (the mutation useserrorPolicy: 'ignore'),isInitializedblocked all subsequent attempts.isGeneratingPreviewas React state was subject to stale closures — The guard check inside the effect used the state value captured at render time, not the latest value. With more records causing more frequent re-renders, concurrent effect invocations could slip past the guard.Fix:
isInitializedentirely — replaced with a stableselectedRecordIdsstring (sorted, comma-joined IDs) as the effect dependency, so the effect only re-runs when the actual set of records changesuseRefforisGeneratingPreviewRefto track in-flight fetches without stale closure issuesfetchPreviewinto auseCallbackthat receivesselectedRecordsas a parameter to avoid stale closuresisGeneratingPreviewstate for the return value (used byMergePreviewTabto show loading state)Test plan