Fix duplicate rows in schema explorer tree on 'load more' #6843

gilluminate · 2025-10-24T22:24:16Z

Description Of Changes

Fixes duplicate rows appearing in the monitor tree schema explorer by implementing a dual-query pattern with intelligent merge logic. The component now fires both fast (without details) and detailed queries simultaneously, displaying fast results immediately while updating with detailed results when available. Added race condition protection to ensure stale queries don't overwrite newer ones.

Code Changes

Added mergeTreeNodeData function to intelligently merge detailed query results with existing fast query results by matching node keys
Refactored fetchTreeDataWithDetails to accept separate fastUpdateFn and detailedUpdateFn callbacks
Wrapped onLoadData and onLoadMore in useCallback with proper dependencies to prevent stale closures
Updated pagination state updates to use updater function pattern for consistency
Optimized merge algorithm using Map and Set data structures for lookups
Added sequence tracking to prevent race conditions when rapid expansions/paginations occur

Steps to Confirm

Navigate to a data discovery monitor's action center with many tables
Expand nodes in the schema explorer tree
Click "Load more..." button at bottom of long table list
Verify no duplicate rows appear when detailed query results load

Pre-Merge Checklist

vercel · 2025-10-24T22:24:22Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
fides-plus-nightly	Ready	Preview	Comment	Oct 24, 2025 11:21pm

1 Skipped Deployment

Project	Deployment	Preview	Comments	Updated (UTC)
fides-privacy-center	Ignored			Oct 24, 2025 11:21pm

greptile-apps

Greptile Overview

Greptile Summary

This PR fixes duplicate rows appearing in the monitor tree schema explorer by implementing a dual-query pattern. The component now fires both a fast query (without details) and a detailed query simultaneously, displaying fast results immediately while updating with detailed results when available.

Key changes:

Added mergeTreeNodeData function that intelligently merges detailed query results with existing fast query results by matching node keys
Refactored fetchTreeDataWithDetails to accept separate fastUpdateFn and detailedUpdateFn callbacks
Wrapped onLoadData and onLoadMore in useCallback with proper dependencies to prevent stale closures
For "load more" operations, the fast query appends new nodes while the detailed query merges/updates them to prevent duplicates
Optimized merge algorithm using Map data structures for O(1) lookups

Issues found:

One critical pagination state update issue that could cause incorrect page tracking under rapid successive "load more" clicks

Confidence Score: 3/5

This PR has a critical race condition that could cause data loading issues under rapid user interaction
The PR successfully implements the dual-query pattern and merge logic to fix duplicates, but contains a race condition in pagination state management that could cause incorrect page loading if users rapidly click "Load more" multiple times. The race condition tracking via requestSequenceRef prevents stale data updates, but doesn't prevent duplicate page fetches due to stale pagination reads.
MonitorTree.tsx requires attention for the pagination race condition in the onLoadMore callback

Important Files Changed

File Analysis

Filename	Score	Overview
clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx	4/5	Implements dual-query pattern with merge logic to fix duplicate rows, uses useCallback for stability, good race condition protection

_{1 file reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx

gilluminate · 2025-10-24T22:50:33Z

@greptile

greptile-apps

Greptile Overview

Greptile Summary

Implements a dual-query pattern (fast + detailed) with intelligent merge logic to fix duplicate rows in the schema explorer tree. The approach fires both queries simultaneously and uses sequence tracking to prevent stale results from overwriting newer ones.

Key changes:

Added mergeTreeNodeData function to intelligently merge detailed query results with fast query results by matching node keys
Refactored fetchTreeDataWithDetails to accept separate callbacks for fast and detailed query results
Wrapped onLoadData and onLoadMore in useCallback with proper dependencies
Added sequence tracking refs to handle race conditions

Issues found:

Critical: Pagination state not updated when detailed query completes before fast query (line 340-345), which breaks subsequent "Load more" operations

Confidence Score: 3/5

This PR has a solid approach but contains a critical pagination bug that needs to be fixed before merge
The dual-query pattern and merge logic are well-designed and should fix the duplicate rows issue. The sequence tracking correctly prevents race conditions. However, there's a critical bug where pagination state isn't updated if the detailed query completes before the fast query, which will break subsequent paginations. This needs to be fixed by also updating pagination state in the detailedUpdateFn callback.
clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx requires attention for the pagination state update bug in the onLoadMore function

Important Files Changed

File Analysis

Filename	Score	Overview
clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx	3/5	Implements dual-query pattern to fix duplicates, but has a critical bug where pagination state isn't updated if detailed query completes before fast query

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx

gilluminate · 2025-10-24T23:01:19Z

@greptile

greptile-apps

Greptile Overview

Greptile Summary

Implements a dual-query pattern to fix duplicate rows in the schema explorer tree by firing fast (without details) and detailed queries simultaneously, displaying fast results immediately and updating with detailed results when available.

Key Changes:

Added mergeTreeNodeData function to intelligently merge detailed query results with existing fast results
Refactored fetchTreeDataWithDetails to accept separate fastUpdateFn and detailedUpdateFn callbacks
Added race condition protection using sequence tracking
Wrapped onLoadData and onLoadMore in useCallback for proper dependency management

Issues Found:

Critical: Pagination state incremented twice in onLoadMore - once when fast query completes and again when detailed query completes, causing page index to be incorrect after both queries finish

Confidence Score: 2/5

This PR has a critical pagination bug that will cause incorrect page tracking
The dual-query pattern and merge logic are well-designed, but the pagination state is incremented twice in the onLoadMore function (once in fastUpdateFn and again in detailedUpdateFn), which will cause the page index to be 2 higher than expected after both queries complete. This breaks the pagination logic and could lead to missing data or incorrect "Load more" behavior.
MonitorTree.tsx requires attention - the detailedUpdateFn in onLoadMore should not update pagination state

Important Files Changed

File Analysis

Filename	Score	Overview
clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx	2/5	Implements dual-query pattern with merge logic to fix duplicate rows. Critical bug: pagination state incremented twice in `onLoadMore` (lines 332-351), causing incorrect page tracking.

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

gilluminate · 2025-10-24T23:09:50Z

@greptile

greptile-apps

Greptile Overview

Greptile Summary

This PR implements a dual-query pattern to fix duplicate rows in the schema explorer tree when clicking "Load more". The solution fires both a fast query (without details) and a detailed query simultaneously, displaying fast results immediately while updating with detailed results when available.

Key Changes:

Added mergeTreeNodeData function to intelligently merge detailed results with existing fast results by matching node keys
Refactored fetchTreeDataWithDetails to accept separate fastUpdateFn and detailedUpdateFn callbacks
Wrapped onLoadData and onLoadMore in useCallback with proper dependencies
Updated pagination state to use updater function pattern for consistency
Added sequence tracking to prevent race conditions from rapid expansions/paginations

Critical Issue:

Lines 340-351 contain a syntax error with duplicate detailedUpdateFn property definition and malformed object structure that will prevent the code from compiling

Confidence Score: 0/5

This PR cannot be merged due to a critical syntax error that will prevent compilation
The code contains a duplicate property definition (detailedUpdateFn on lines 340-351) with malformed object syntax that will cause TypeScript compilation to fail. This is a blocking syntax error that must be resolved before the PR can be merged.
MonitorTree.tsx requires immediate attention to fix the syntax error on lines 340-351

Important Files Changed

File Analysis

Filename	Score	Overview
clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx	0/5	Implements dual-query pattern to fix duplicate rows, but contains critical syntax error with duplicate `detailedUpdateFn` property (lines 340-351) that will prevent compilation

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx

This reverts commit 7d82a8b.

gilluminate · 2025-10-24T23:22:42Z

@greptile

greptile-apps

Greptile Overview

Greptile Summary

Fixes duplicate rows in schema explorer by implementing a dual-query pattern (fast without details, detailed with details) with intelligent merge logic and race condition protection.

Key Changes:

Added mergeTreeNodeData function using Map-based O(1) lookups to merge detailed results into existing fast results without duplication
Refactored fetchTreeDataWithDetails to accept separate fastUpdateFn and detailedUpdateFn callbacks for different update strategies
Wrapped onLoadData and onLoadMore in useCallback to prevent stale closures
Implemented sequence tracking via refs to prevent stale queries from overwriting newer ones
Updated onLoadMore to use appendTreeNodeData for fast query (immediate display) and mergeTreeNodeData for detailed query (updates with details)

Analysis:
The implementation correctly addresses the duplicate rows issue. The race condition protection ensures that only the latest request updates state, preventing corruption from out-of-order query completions. The merge logic preserves order and correctly handles matching keys between fast and detailed results.

Minor inefficiency: rapid double-clicks on "Load more" read stale pagination state and request the same page twice before state updates, though race protection prevents actual corruption.

Confidence Score: 4/5

Safe to merge with minor inefficiency that doesn't affect correctness
The PR successfully fixes the duplicate rows bug with solid race condition protection and merge logic. All previous review comments appear to have been addressed. One minor inefficiency exists with rapid clicks reading stale pagination state, but this doesn't cause correctness issues due to sequence tracking. The implementation is well-documented and uses appropriate data structures (Maps) for performance.
No files require special attention

Important Files Changed

File Analysis

Filename	Score	Overview
clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx	4/5	Implements dual-query pattern with race condition protection to fix duplicate rows issue; logic is mostly sound with minor inefficiency in rapid clicks

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx

speaker-ender · 2025-10-27T14:25:40Z

clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx

+      // Create maps for efficient O(1) lookups
+      const newChildrenMap = new Map(
+        newChildren.map((child) => [child.key, child]),
+      );
+      const existingChildrenMap = new Map(
+        filteredChildren.map((child) => [child.key, child]),
+      );
+
+      // Merge new children: update existing ones with detailed data, keep order
+      const mergedChildren = filteredChildren.map((existingChild) => {
+        const matchingNewChild = newChildrenMap.get(existingChild.key);
+        return matchingNewChild ?? existingChild;
+      });
+
+      // Add any new children that weren't in the existing list
+      newChildren.forEach((newChild) => {
+        if (!existingChildrenMap.has(newChild.key)) {
+          mergedChildren.push(newChild);
+        }
+      });


Not a blocker but it would be nice to have a standardized way of doing these operations + using generic utilities to reduce the amount of boilerplate + reduce bugs.
Not sure if lodash has something for this.

speaker-ender

Small comment but def not a blocker.
Looks good.
Def want to revisit this component after initial launch.
I think there are some inherit issues with the data returned from the BE + how ant expects the data to be handled that are incompatible.

vercel bot deployed to Preview – fides-plus-nightly October 24, 2025 22:24 View deployment

Fix duplicate rows in schema explorer tree on 'load more'

0ce0e78

gilluminate force-pushed the gill/ENG-1758/duplicate-rows-in-schema branch from 12e5ee8 to 0ce0e78 Compare October 24, 2025 22:29

gilluminate requested a review from speaker-ender October 24, 2025 22:30

gilluminate marked this pull request as ready for review October 24, 2025 22:30

gilluminate requested a review from a team as a code owner October 24, 2025 22:30

vercel bot deployed to Preview – fides-plus-nightly October 24, 2025 22:33 View deployment

greptile-apps bot reviewed Oct 24, 2025

View reviewed changes

clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx Show resolved Hide resolved

clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx Outdated Show resolved Hide resolved

PR feedback updates

5c4f5f2

vercel bot deployed to Preview – fides-plus-nightly October 24, 2025 22:54 View deployment

greptile-apps bot reviewed Oct 24, 2025

View reviewed changes

clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx Outdated Show resolved Hide resolved

gilluminate enabled auto-merge October 24, 2025 22:58

PR review fix

1e6d43b

vercel bot deployed to Preview – fides-plus-nightly October 24, 2025 23:04 View deployment

greptile-apps bot reviewed Oct 24, 2025

View reviewed changes

clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx Show resolved Hide resolved

PR review feedback fix

7d82a8b

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>

vercel bot had a problem deploying to Preview – fides-plus-nightly October 24, 2025 23:10 Failure

greptile-apps bot reviewed Oct 24, 2025

View reviewed changes

clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx Outdated Show resolved Hide resolved

gilluminate added 2 commits October 24, 2025 17:14

Revert "PR review feedback fix"

41f106b

This reverts commit 7d82a8b.

PR review feedback fix

1a61f95

vercel bot deployed to Preview – fides-plus-nightly October 24, 2025 23:21 View deployment

greptile-apps bot reviewed Oct 24, 2025

View reviewed changes

clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/MonitorTree.tsx Show resolved Hide resolved

speaker-ender reviewed Oct 27, 2025

View reviewed changes

speaker-ender approved these changes Oct 27, 2025

View reviewed changes

gilluminate added this pull request to the merge queue Oct 27, 2025

Merged via the queue into main with commit 6a17b32 Oct 27, 2025
46 checks passed

gilluminate deleted the gill/ENG-1758/duplicate-rows-in-schema branch October 27, 2025 14:40

Fix duplicate rows in schema explorer tree on 'load more' #6843

Fix duplicate rows in schema explorer tree on 'load more' #6843

Uh oh!

Conversation

gilluminate commented Oct 24, 2025 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description Of Changes

Code Changes

Steps to Confirm

Pre-Merge Checklist

Uh oh!

vercel bot commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Overview

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

gilluminate commented Oct 24, 2025

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Overview

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Uh oh!

Uh oh!

gilluminate commented Oct 24, 2025

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Overview

Greptile Summary

Confidence Score: 2/5

Important Files Changed

Uh oh!

Uh oh!

gilluminate commented Oct 24, 2025

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Overview

Greptile Summary

Confidence Score: 0/5

Important Files Changed

Uh oh!

Uh oh!

gilluminate commented Oct 24, 2025

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Greptile Overview

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Uh oh!

Uh oh!

speaker-ender Oct 27, 2025

Choose a reason for hiding this comment

Uh oh!

speaker-ender left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gilluminate commented Oct 24, 2025 •

edited by atlassian bot

Loading

vercel bot commented Oct 24, 2025 •

edited

Loading