Skip to content

fix(ui): actually drop the removed column pair when removing a column-level lineage edge - #31107

Merged
chirag-madlani merged 4 commits into
mainfrom
fix-column-lineage-edge-removal
Aug 7, 2026
Merged

fix(ui): actually drop the removed column pair when removing a column-level lineage edge#31107
chirag-madlani merged 4 commits into
mainfrom
fix-column-lineage-edge-removal

Conversation

@chirag-madlani

@chirag-madlani chirag-madlani commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Fix: #31109

Describe your changes:

getColumnLineageData was reading sourceHandle/targetHandle from edge.data.*, but React Flow stores those handles at the top level of the edge — so both reads returned undefined, the filter never matched, and the "trimmed" columnsLineage array was actually the original array. removeColumnEdge then PUT that unchanged array to /api/v1/lineage, whose backend upsert (ON DUPLICATE KEY UPDATE json = :json) fully replaces LineageDetails — so the same set of columns was written back. The UI looked correct because LineageProvider optimistically updated local state (setEntityLineage / removeEdgeById / setColumnsHavingLineage), but a refresh brought the removed edge back.

The Playwright removeColumnLineage helper missed this because it asserted not.toBeVisible() immediately after the click, against the same optimistically-mutated DOM. It now reloads, waits for a fresh /api/v1/lineage/getLineage?* response, and re-asserts — so every caller (including the cross-entity sweep in DataAssetLineage.spec.ts) now validates server state, not local state.

Type of change:

  • Bug fix

High-level design:

N/A — small change.

Tests:

Use cases covered

  • User in edit mode clicks the delete button on a column-level lineage edge; after saving and refreshing, the edge stays gone.
  • User deletes a node-to-node lineage edge; the deletion persists across a page refresh.

Playwright (UI) tests

  • Added Edge removal persists across refresh describe in openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineage/LineageInteraction.spec.ts with two focused tests:
    • Node-to-node edge deletion persists across a page refresh
    • Column-level edge deletion persists across a page refresh (regression for this bug)
  • Hardened removeColumnLineage in openmetadata-ui/src/main/resources/ui/playwright/utils/lineage.ts to reload + re-fetch getLineage + re-assert.

Manual testing performed

  1. Attached a column-level lineage edge in edit mode, saved, refreshed — edge stays.
  2. Removed the same edge in edit mode, saved, refreshed — edge is gone (previously came back).
  3. Ran the two new Playwright tests locally against the fixed code — both pass; reverting only the client fix flips the column-edge test to fail on the post-reload assertion.

UI screen recording / screenshots:

Screen.Recording.2026-08-06.at.6.28.45.PM.mov

Checklist:

  • I have read the CONTRIBUTING document.
  • I have commented on my code, particularly in hard-to-understand areas.
  • I have added tests (unit / integration / Playwright as applicable) and listed them above.
  • I have added a test that covers the exact scenario we are fixing.

🤖 Generated with Claude Code

… edge

getColumnLineageData was reading sourceHandle/targetHandle from
edge.data.* — but React Flow stores those handles at the top level of
the edge, so both reads returned undefined and the filter never matched.
removeColumnEdge then PUT the unchanged columnsLineage array back to
/api/v1/lineage, and the server (which fully replaces LineageDetails
JSON on upsert) persisted the same set. The UI looked correct because
LineageProvider optimistically updated local state, but on refresh the
removed edge reappeared.

The Playwright removeColumnLineage helper missed this because it
asserted the edge was not visible immediately after the click, against
the same optimistically-mutated DOM. Reload it once and it now checks
server state.

- Read sourceHandle/targetHandle from the edge itself in
  getColumnLineageData.
- Harden removeColumnLineage to reload + re-fetch getLineage + re-assert.
- Add focused Edge-removal-persists-across-refresh describe in
  LineageInteraction.spec.ts covering both node-to-node and column-level
  edges, so the regression is caught without disturbing the existing
  cross-entity add/remove sweep in DataAssetLineage.spec.ts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@chirag-madlani
chirag-madlani requested a review from a team as a code owner August 6, 2026 12:21
Copilot AI review requested due to automatic review settings August 6, 2026 12:21

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

✅ PR checks passed

The linked issue has a description and all required Shipping project fields set. Thanks!

@github-actions github-actions Bot added safe to test Add this label to run secure Github workflows on PRs UI UI specific issues labels Aug 6, 2026
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 66%
66.2% (78087/117954) 50.19% (47172/93977) 51.39% (14198/27623)

…tion button disappears

EdgeInteractionOverlay reads selectedEdge from useLineageStore and
renders the floating edit/delete button at the edge's midpoint. Neither
removeEdgeHandler nor removeColumnEdge cleared selectedEdge after the
removal, so the button kept rendering at the last-known position of a
now-deleted edge until the user clicked somewhere else on the canvas.

Set selectedEdge back to undefined once the removal has finished — the
overlay's pathData/buttonPosition then resolve to null and the button
unmounts cleanly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 6, 2026 12:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

…ression

Cover the pure-function core of the column-edge removal fix. The
regression case constructs a React Flow edge with sourceHandle /
targetHandle on the TOP LEVEL (and no such fields on edge.data) and
asserts the matching entry is dropped — the old implementation read
data.data?.sourceHandle, missed it, and returned the array unchanged.
Reverting the fix in EntityLineageEdgeUtils.ts flips all four tests to
red, so the regression is properly pinned.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 6, 2026 12:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 3904ac01603c380e805db5c2c56d9e3b3ac14753 in Playwright run 31108389323, attempt 1.

✅ 566 passed · ❌ 0 failed · 🟡 1 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 56m 46s

⏱️ Max setup 3m 23s · max shard execution 18m 52s · max shard-job elapsed before upload 24m 10s · reporting 6s

🌐 203.35 requests/attempt · 2.84 app boots/UI scenario · 13.43% common-shard skew

Optimization targets still in progress:

  • Browser traffic was 203.35 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.84 per UI scenario (1673 boots / 589 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 140 0 0 0 0 0
🟡 Shard chromium-02 138 0 1 0 0 0
✅ Shard chromium-03 137 0 0 0 0 0
✅ Shard data-asset-rules-01 61 0 0 0 0 0
✅ Shard domain-isolation-01 14 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
✅ Shard ingestion-01 1 0 0 0 0 0
✅ Shard reindex-01 2 0 0 0 0 0
✅ Shard search-01 10 0 0 0 0 0
✅ Shard search-rbac-01 29 0 0 0 0 0
🟡 1 flaky test(s) (passed on retry)
  • Pages/Lineage/LineageInteraction.spec.tsVerify node panel opens on click (shard chromium-02, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

The confirmation modal's Dialog.Content renders
getModalBodyText(selectedEdge), which unconditionally destructures
selectedEdge.data. My previous change cleared selectedEdge from inside
removeEdgeHandler/removeColumnEdge — running before the caller
(`onRemove`) called setShowDeleteModal(false). Any intermediate render
that observed `showDeleteModal=true && selectedEdge=undefined` therefore
threw during render and took the LineageProvider tree down. That is
exactly what the flaky `Verify edge delete button in drawer` Playwright
test was reporting: "Target page, context or browser has been closed"
followed by "element(s) not found" on the retry.

Move setSelectedEdge(undefined) out of the individual handlers and into
onRemove where it is called back-to-back with setShowDeleteModal(false)
— so both updates land in the same React 18 batch and the modal
unmounts before the selection is dropped. Add a defensive JSX guard
(`showDeleteModal && selectedEdge`) so the modal cannot render at all
when either half of the state is missing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 6, 2026 13:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 46 warning(s)

Errors block the build. Warnings do not yet — they are rules whose backlog is still
being worked down, listed so this PR does not add to it. See docs/ui-code-quality-gate.md.

0 error(s), 46 warning(s) across 2 changed file(s).

Count Rule
33 react-hooks/exhaustive-deps
5 sonarjs/cyclomatic-complexity
3 sonarjs/no-duplicate-string
2 sonarjs/cognitive-complexity
1 sonarjs/no-nested-conditional
1 sonarjs/no-nested-functions
1 @typescript-eslint/no-non-null-assertion
All findings
Location Rule Message
🟡 src/context/LineageProvider/LineageProvider.tsx:205:62 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 13 which is greater than 10 authorized.","cost":3,"secondaryLocations":[{"line":205,"column":61,"endLine":205,"endColum
🟡 src/context/LineageProvider/LineageProvider.tsx:336:9 sonarjs/no-nested-conditional Extract this nested ternary operation into an independent statement.
🟡 src/context/LineageProvider/LineageProvider.tsx:470:6 react-hooks/exhaustive-deps React Hook useMemo has an unnecessary dependency: 'isEditMode'. Either exclude it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:503:7 sonarjs/cognitive-complexity Refactor this function to reduce its Cognitive Complexity from 20 to the 15 allowed.
🟡 src/context/LineageProvider/LineageProvider.tsx:503:7 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 19 which is greater than 10 authorized.","cost":9,"secondaryLocations":[{"line":503,"column":6,"endLine":503,"endColumn
🟡 src/context/LineageProvider/LineageProvider.tsx:622:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'setColumnsHavingLineage', 'setEdges', 'setIsRepositioning', and 'setNodes'. Either include them or remove the
🟡 src/context/LineageProvider/LineageProvider.tsx:681:13 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 4 times.
🟡 src/context/LineageProvider/LineageProvider.tsx:682:23 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 4 times.
🟡 src/context/LineageProvider/LineageProvider.tsx:690:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 't'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:749:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'isTourOpen', 'setEdges', 'setNodes', 't', and 'timeFilter'. Either include them or remove the dependency array
🟡 src/context/LineageProvider/LineageProvider.tsx:778:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'navigate', 'setActiveLayer', and 'setPlatformView'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:819:26 sonarjs/no-duplicate-string Define a constant instead of duplicating this literal 4 times.
🟡 src/context/LineageProvider/LineageProvider.tsx:850:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'exportLineageData', 'showModal', and 't'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:862:75 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 20 which is greater than 10 authorized.","cost":10,"secondaryLocations":[{"line":862,"column":74,"endLine":862,"endColu
🟡 src/context/LineageProvider/LineageProvider.tsx:984:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'lineageData?.downstreamEdges', 'lineageData?.upstreamEdges', 't', and 'updateLineageData'. Either include them
🟡 src/context/LineageProvider/LineageProvider.tsx:1036:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'setTracedColumns' and 'setTracedNodes'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1057:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'onPlatformViewChange' and 'setIsPlatformLineage'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1073:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'setSelectedEdge', 'setTracedColumns', and 'setTracedNodes'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1083:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'setTracedColumns'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1241:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'edges', 'removeEdgesBySourceTarget', and 'removeNodeById'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1433:5 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'setSelectedNode'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1456:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'selectLoadMoreNode', 'setActiveNode', 'setSelectedEdge', and 'setSelectedNode'. Either include them or remove
🟡 src/context/LineageProvider/LineageProvider.tsx:1466:6 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'setActiveNode', 'setSelectedColumn', 'setSelectedNode', 'setTracedColumns', and 'setTracedNodes'. Either inclu
🟡 src/context/LineageProvider/LineageProvider.tsx:1478:6 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'setActiveNode', 'setSelectedEdge', 'setSelectedNode', 'setTracedColumns', and 'setTracedNodes'. Either include
🟡 src/context/LineageProvider/LineageProvider.tsx:1526:6 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'setSelectedEdge'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1529:33 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 14 which is greater than 10 authorized.","cost":4,"secondaryLocations":[{"line":1529,"column":32,"endLine":1529,"endCol
🟡 src/context/LineageProvider/LineageProvider.tsx:1626:31 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/context/LineageProvider/LineageProvider.tsx:1653:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'addTracedColumns', 'setColumnsHavingLineage', and 'setTracedNodes'. Either include them or remove the dependen
🟡 src/context/LineageProvider/LineageProvider.tsx:1672:6 react-hooks/exhaustive-deps React Hook useCallback has a missing dependency: 'setSelectedEdge'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1752:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'handleModalCancel' and 'updateEdge'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1815:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'setSelectedEdge' and 'updateEdge'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1878:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'setActiveNode' and 'updateLineageData'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1883:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'redraw'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:1885:47 sonarjs/cognitive-complexity Refactor this function to reduce its Cognitive Complexity from 16 to the 15 allowed.
🟡 src/context/LineageProvider/LineageProvider.tsx:1885:47 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 21 which is greater than 10 authorized.","cost":11,"secondaryLocations":[{"line":1885,"column":46,"endLine":1885,"endCo
🟡 src/context/LineageProvider/LineageProvider.tsx:1938:6 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'fetchLineageData', 'fetchPlatformLineage', and 'timeFilter'. Either include them or remove the dependency arra
🟡 src/context/LineageProvider/LineageProvider.tsx:1965:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'setActiveLayer' and 'setLineageConfig'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:2004:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'removeEdgeHandler' and 'removeNodeHandler'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:2010:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'redraw'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:2014:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'onPlatformViewUpdate'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:2062:6 react-hooks/exhaustive-deps React Hook useMemo has a missing dependency: 'queryFilter'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:2116:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'updateActiveLayer'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:2122:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'fetchDataQualityLineage'. Either include it or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:2164:6 react-hooks/exhaustive-deps React Hook useMemo has missing dependencies: 'platformView' and 't'. Either include them or remove the dependency array.
🟡 src/context/LineageProvider/LineageProvider.tsx:2169:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'reset'. Either include it or remove the dependency array.
🟡 src/utils/EntityLineageEdgeUtils.ts:172:27 @typescript-eslint/no-non-null-assertion Forbidden non-null assertion.

Fix locally (fast - only checks files changed in this branch):

make ui-checkstyle-changed

@sonarqubecloud

sonarqubecloud Bot commented Aug 6, 2026

Copy link
Copy Markdown

@chirag-madlani
chirag-madlani added this pull request to the merge queue Aug 6, 2026
Merged via the queue into main with commit 34c9708 Aug 7, 2026
85 of 86 checks passed
@chirag-madlani
chirag-madlani deleted the fix-column-lineage-edge-removal branch August 7, 2026 01:07
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Changes have been cherry-picked to the 2.0 branch.

github-actions Bot pushed a commit that referenced this pull request Aug 7, 2026
…-level lineage edge (#31107)

* fix(ui): actually drop the removed column pair when removing a column edge

getColumnLineageData was reading sourceHandle/targetHandle from
edge.data.* — but React Flow stores those handles at the top level of
the edge, so both reads returned undefined and the filter never matched.
removeColumnEdge then PUT the unchanged columnsLineage array back to
/api/v1/lineage, and the server (which fully replaces LineageDetails
JSON on upsert) persisted the same set. The UI looked correct because
LineageProvider optimistically updated local state, but on refresh the
removed edge reappeared.

The Playwright removeColumnLineage helper missed this because it
asserted the edge was not visible immediately after the click, against
the same optimistically-mutated DOM. Reload it once and it now checks
server state.

- Read sourceHandle/targetHandle from the edge itself in
  getColumnLineageData.
- Harden removeColumnLineage to reload + re-fetch getLineage + re-assert.
- Add focused Edge-removal-persists-across-refresh describe in
  LineageInteraction.spec.ts covering both node-to-node and column-level
  edges, so the regression is caught without disturbing the existing
  cross-entity add/remove sweep in DataAssetLineage.spec.ts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(ui): clear selectedEdge after removing an edge so the floating action button disappears

EdgeInteractionOverlay reads selectedEdge from useLineageStore and
renders the floating edit/delete button at the edge's midpoint. Neither
removeEdgeHandler nor removeColumnEdge cleared selectedEdge after the
removal, so the button kept rendering at the last-known position of a
now-deleted edge until the user clicked somewhere else on the canvas.

Set selectedEdge back to undefined once the removal has finished — the
overlay's pathData/buttonPosition then resolve to null and the button
unmounts cleanly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(ui): unit-test getColumnLineageData against the sourceHandle regression

Cover the pure-function core of the column-edge removal fix. The
regression case constructs a React Flow edge with sourceHandle /
targetHandle on the TOP LEVEL (and no such fields on edge.data) and
asserts the matching entry is dropped — the old implementation read
data.data?.sourceHandle, missed it, and returned the array unchanged.
Reverting the fix in EntityLineageEdgeUtils.ts flips all four tests to
red, so the regression is properly pinned.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(ui): close the delete-edge modal before clearing selectedEdge

The confirmation modal's Dialog.Content renders
getModalBodyText(selectedEdge), which unconditionally destructures
selectedEdge.data. My previous change cleared selectedEdge from inside
removeEdgeHandler/removeColumnEdge — running before the caller
(`onRemove`) called setShowDeleteModal(false). Any intermediate render
that observed `showDeleteModal=true && selectedEdge=undefined` therefore
threw during render and took the LineageProvider tree down. That is
exactly what the flaky `Verify edge delete button in drawer` Playwright
test was reporting: "Target page, context or browser has been closed"
followed by "element(s) not found" on the retry.

Move setSelectedEdge(undefined) out of the individual handlers and into
onRemove where it is called back-to-back with setShowDeleteModal(false)
— so both updates land in the same React 18 batch and the modal
unmounts before the selection is dropped. Add a defensive JSX guard
(`showDeleteModal && selectedEdge`) so the modal cannot render at all
when either half of the state is missing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
(cherry picked from commit 34c9708)
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Failed to cherry-pick changes to the 1.13 branch.
Please cherry-pick the changes manually.
You can find more details here.

@gitar-bot

gitar-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Fixes column-level lineage edge removal by reading source and target handles correctly from React Flow edges and hardening Playwright tests to validate server-side persistence. No issues found.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source

chirag-madlani added a commit that referenced this pull request Aug 7, 2026
…-level lineage edge (#31107)

* fix(ui): actually drop the removed column pair when removing a column edge

getColumnLineageData was reading sourceHandle/targetHandle from
edge.data.* — but React Flow stores those handles at the top level of
the edge, so both reads returned undefined and the filter never matched.
removeColumnEdge then PUT the unchanged columnsLineage array back to
/api/v1/lineage, and the server (which fully replaces LineageDetails
JSON on upsert) persisted the same set. The UI looked correct because
LineageProvider optimistically updated local state, but on refresh the
removed edge reappeared.

The Playwright removeColumnLineage helper missed this because it
asserted the edge was not visible immediately after the click, against
the same optimistically-mutated DOM. Reload it once and it now checks
server state.

- Read sourceHandle/targetHandle from the edge itself in
  getColumnLineageData.
- Harden removeColumnLineage to reload + re-fetch getLineage + re-assert.
- Add focused Edge-removal-persists-across-refresh describe in
  LineageInteraction.spec.ts covering both node-to-node and column-level
  edges, so the regression is caught without disturbing the existing
  cross-entity add/remove sweep in DataAssetLineage.spec.ts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(ui): clear selectedEdge after removing an edge so the floating action button disappears

EdgeInteractionOverlay reads selectedEdge from useLineageStore and
renders the floating edit/delete button at the edge's midpoint. Neither
removeEdgeHandler nor removeColumnEdge cleared selectedEdge after the
removal, so the button kept rendering at the last-known position of a
now-deleted edge until the user clicked somewhere else on the canvas.

Set selectedEdge back to undefined once the removal has finished — the
overlay's pathData/buttonPosition then resolve to null and the button
unmounts cleanly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(ui): unit-test getColumnLineageData against the sourceHandle regression

Cover the pure-function core of the column-edge removal fix. The
regression case constructs a React Flow edge with sourceHandle /
targetHandle on the TOP LEVEL (and no such fields on edge.data) and
asserts the matching entry is dropped — the old implementation read
data.data?.sourceHandle, missed it, and returned the array unchanged.
Reverting the fix in EntityLineageEdgeUtils.ts flips all four tests to
red, so the regression is properly pinned.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(ui): close the delete-edge modal before clearing selectedEdge

The confirmation modal's Dialog.Content renders
getModalBodyText(selectedEdge), which unconditionally destructures
selectedEdge.data. My previous change cleared selectedEdge from inside
removeEdgeHandler/removeColumnEdge — running before the caller
(`onRemove`) called setShowDeleteModal(false). Any intermediate render
that observed `showDeleteModal=true && selectedEdge=undefined` therefore
threw during render and took the LineageProvider tree down. That is
exactly what the flaky `Verify edge delete button in drawer` Playwright
test was reporting: "Target page, context or browser has been closed"
followed by "element(s) not found" on the retry.

Move setSelectedEdge(undefined) out of the individual handlers and into
onRemove where it is called back-to-back with setShowDeleteModal(false)
— so both updates land in the same React 18 batch and the modal
unmounts before the selection is dropped. Add a defensive JSX guard
(`showDeleteModal && selectedEdge`) so the modal cannot render at all
when either half of the state is missing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
(cherry picked from commit 34c9708)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs To release Will cherry-pick this PR into the release branch UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Removing a column-level lineage edge does not persist; edge reappears on refresh; stale delete button lingers

3 participants