Skip to content

CTDC-1719: add ancestor node traceback feature#166

Open
nathanhuangnih wants to merge 4 commits intoCRDC-DHfrom
CTDC-1719-ancestor-traceback-feature
Open

CTDC-1719: add ancestor node traceback feature#166
nathanhuangnih wants to merge 4 commits intoCRDC-DHfrom
CTDC-1719-ancestor-traceback-feature

Conversation

@nathanhuangnih
Copy link

  • Clicking a node filters the graph to show only that node and its parent path to root
  • Filter clears when closing node or clicking canvas
  • Uses Redux state and BFS traversal to find ancestors

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds an ancestor node traceback feature that allows users to filter the graph visualization to show only a selected node and its parent path to the root. The feature integrates with the existing Redux state management and uses BFS (Breadth-First Search) traversal to identify ancestor relationships.

Key Changes:

  • Implements ancestor node filtering functionality using BFS traversal
  • Adds Redux actions and state management for the ancestor filter
  • Integrates filter clearing with existing UI interactions (node collapse, canvas click)

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
src/components/ModelNavigator/DataDictionary/Store/reducers/graph.js Adds ancestorFilterNodeIds state and reducer cases for ancestor filtering; computes ancestor nodes when a node is focused
src/components/ModelNavigator/DataDictionary/Store/actions/graph.js Defines setAncestorFilter and clearAncestorFilter action creators
src/components/ModelNavigator/DataDictionary/ReactFlowGraph/Node/ReduxNodeView.jsx Connects onClearAncestorFilter action to Redux dispatcher
src/components/ModelNavigator/DataDictionary/ReactFlowGraph/Node/NodeView.jsx Calls onClearAncestorFilter when the node view is collapsed
src/components/ModelNavigator/DataDictionary/ReactFlowGraph/Canvas/CanvasHelper.jsx Implements buildParentMap and getAncestorNodes helper functions for BFS traversal
src/components/ModelNavigator/DataDictionary/ReactFlowGraph/Canvas/CanvasController.jsx Filters nodes and edges based on ancestorFilterNodeIds; clears filter on canvas click

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -7,6 +7,7 @@ import {
onCnavasWidthChange,
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'Cnavas' to 'Canvas'.

Suggested change
onCnavasWidthChange,
onCanvasWidthChange,

Copilot uses AI. Check for mistakes.
Comment on lines +167 to +168
const parentMap = buildParentMap(state.dictionary);
const ancestorNodeIds = getAncestorNodes(action.nodeID, parentMap);
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

Building the parent map on every node focus is inefficient. Consider memoizing the parent map or computing it once when the dictionary changes, then storing it in Redux state to avoid recalculating on each node interaction.

Copilot uses AI. Check for mistakes.
});

export const setAncestorFilter = (ancestorNodeIds) => ({
type: 'SET_ANCESTOR_FILTER',
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

Action type 'SET_ANCESTOR_FILTER' should be defined as a constant in actionTypes file to maintain consistency with other actions like 'ON_REACT_FLOW_NODE_FOCUS'. This prevents typos and makes refactoring easier.

Suggested change
type: 'SET_ANCESTOR_FILTER',
type: actionTypes.SET_ANCESTOR_FILTER,

Copilot uses AI. Check for mistakes.
});

export const clearAncestorFilter = () => ({
type: 'CLEAR_ANCESTOR_FILTER',
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

Action type 'CLEAR_ANCESTOR_FILTER' should be defined as a constant in actionTypes file to maintain consistency with other actions like 'ON_REACT_FLOW_NODE_FOCUS'. This prevents typos and makes refactoring easier.

Suggested change
type: 'CLEAR_ANCESTOR_FILTER',
type: actionTypes.CLEAR_ANCESTOR_FILTER,

Copilot uses AI. Check for mistakes.
onViewTable: (hide) => dispatch(setOverlayPropertyTableHidden(hide)),
onCollapseNodeView: () => {dispatch(onPanelViewClick())}
onCollapseNodeView: () => {dispatch(onPanelViewClick())},
onClearAncestorFilter: () => dispatch({ type: 'CLEAR_ANCESTOR_FILTER' }),
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

Dispatching action with inline type string bypasses the imported action creator. Use dispatch(clearAncestorFilter()) from the actions/graph.js file to maintain consistency and type safety.

Copilot uses AI. Check for mistakes.
onGraphPanelClick: () => {dispatch(onPanelViewClick())},
onGraphPanelClick: () => {
dispatch(onPanelViewClick());
dispatch({ type: 'CLEAR_ANCESTOR_FILTER' });
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

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

Dispatching action with inline type string bypasses the imported action creator. Use dispatch(clearAncestorFilter()) from the actions/graph.js file to maintain consistency and type safety.

Copilot uses AI. Check for mistakes.
@amattu2 amattu2 requested a review from a team January 27, 2026 18:57
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +159 to +164
type: 'SET_ANCESTOR_FILTER',
ancestorNodeIds,
});

export const clearAncestorFilter = () => ({
type: 'CLEAR_ANCESTOR_FILTER',
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

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

The action types 'SET_ANCESTOR_FILTER' and 'CLEAR_ANCESTOR_FILTER' are hardcoded string literals. These should be defined as constants in the actionTypes file to maintain consistency with the existing pattern in the codebase and prevent typos.

Suggested change
type: 'SET_ANCESTOR_FILTER',
ancestorNodeIds,
});
export const clearAncestorFilter = () => ({
type: 'CLEAR_ANCESTOR_FILTER',
type: actionTypes.SET_ANCESTOR_FILTER,
ancestorNodeIds,
});
export const clearAncestorFilter = () => ({
type: actionTypes.CLEAR_ANCESTOR_FILTER,

Copilot uses AI. Check for mistakes.
Copy link

@amattu2 amattu2 left a comment

Choose a reason for hiding this comment

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

I'm doing some initial testing, and noticing an odd behavior with some nodes not showing the initial Node View popup. Maybe this is pre-existing behavior? It's probably worth fixing as apart of these changes if it's not a huge scope creep. LMK

Screen.Recording.2026-02-09.at.10.05.58.AM.mov

@amattu2
Copy link

amattu2 commented Mar 6, 2026

Discussed in a call an issue (??) where the graph resets the placement of nodes after dismissing the contextual popup that appears when clicking a node. Pending confirmation from CTDC if this needs to be fixed, or if it's expected behavior. If no changes are made, we can merge this in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants