Skip to content
Closed
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { useMergeManyRecords } from '@/object-record/hooks/useMergeManyRecords';
import { useMergeRecordsSelectedRecords } from '@/object-record/record-merge/hooks/useMergeRecordsSelectedRecords';
import { useUpsertRecordsInStore } from '@/object-record/record-store/hooks/useUpsertRecordsInStore';
import { type ObjectRecord } from '@/object-record/types/ObjectRecord';
import { useEffect, useState } from 'react';
import { useCallback, useEffect, useRef, useState } from 'react';
import { isMergeInProgressState } from '@/object-record/record-merge/states/mergeInProgressState';
import { mergeSettingsState } from '@/object-record/record-merge/states/mergeSettingsState';
import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue';
Expand All @@ -18,7 +18,8 @@ export const usePerformMergePreview = ({
const [mergePreviewRecord, setMergePreviewRecord] =
useState<ObjectRecord | null>(null);
const [isGeneratingPreview, setIsGeneratingPreview] = useState(false);
const [isInitialized, setIsInitialized] = useState(false);

const isGeneratingPreviewRef = useRef(false);

const mergeSettings = useAtomStateValue(mergeSettingsState);
const isMergeInProgress = useAtomStateValue(isMergeInProgressState);
Expand All @@ -31,21 +32,22 @@ export const usePerformMergePreview = ({

const { upsertRecordsInStore } = useUpsertRecordsInStore();

useEffect(() => {
const fetchPreview = async () => {
if (
selectedRecords.length < 2 ||
isMergeInProgress ||
isInitialized ||
isGeneratingPreview
) {
const selectedRecordIds = selectedRecords
.map((record) => record.id)
.sort()
.join(',');

const fetchPreview = useCallback(
async (records: ObjectRecord[]) => {
if (records.length < 2 || isGeneratingPreviewRef.current) {
return;
}

isGeneratingPreviewRef.current = true;
setIsGeneratingPreview(true);
try {
const previewRecord = await mergeManyRecords({
recordIds: selectedRecords.map((record) => record.id),
recordIds: records.map((record) => record.id),
mergeSettings,
preview: true,
});
Expand All @@ -63,23 +65,23 @@ export const usePerformMergePreview = ({
} catch {
setMergePreviewRecord(null);
} finally {
isGeneratingPreviewRef.current = false;
setIsGeneratingPreview(false);
setIsInitialized(true);
}
};
},
[mergeManyRecords, mergeSettings, upsertRecordsInStore],
);

if (selectedRecords.length > 0 && !isMergeInProgress) {
fetchPreview();
useEffect(() => {
if (
selectedRecords.length >= 2 &&
!isMergeInProgress &&
!isGeneratingPreviewRef.current
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Fix with Cubic

) {
fetchPreview(selectedRecords);
}
}, [
selectedRecords,
mergeSettings,
isMergeInProgress,
isGeneratingPreview,
mergeManyRecords,
upsertRecordsInStore,
isInitialized,
]);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [selectedRecordIds, mergeSettings, isMergeInProgress, fetchPreview]);

return {
mergePreviewRecord,
Comment on lines +78 to 87
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Expand Down
Loading