Skip to content

Conversation

@harsh-vador
Copy link
Contributor

@harsh-vador harsh-vador commented Dec 10, 2025

Describe your changes:

Fixes #20835
I worked on adding lineage section in overview tab of project explorer card

Screenshot 2025-12-10 at 5 35 07 PM Screenshot 2025-12-10 at 5 35 15 PM

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Summary by Gitar

  • New component:
    • LineageSection displays upstream/downstream counts with loading and empty states using MUI v7 components
  • Overview tab integration:
    • Added lineage data fetching in EntitySummaryPanel with race condition handling and stale data prevention
    • Positioned between Data Quality and Owners sections with click-to-navigate to full lineage page
  • Entity type support:
    • Enabled for 15+ entity types including TABLE, TOPIC, DASHBOARD, PIPELINE, API_ENDPOINT, and drive assets
  • Comprehensive testing:
    • 263 lines of unit tests covering loading, empty, data, click interactions, and state transitions
    • E2E Playwright tests validating counts and navigation in RightEntityPanelFlow.spec.ts
  • Internationalization:
    • Added no-lineage-available translation key across 17 language files

This will update automatically on new commits.


@github-actions
Copy link
Contributor

github-actions bot commented Dec 10, 2025

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 64%
64.15% (50698/79031) 41.74% (24697/59169) 45.16% (7770/17207)

@gitar-bot
Copy link

gitar-bot bot commented Dec 17, 2025

🔍 CI failure analysis for 6da27d3: Positive Progress: Jobs 1-4/6 now passing! Only jobs 5/6 and 6/6 still failing with the established timeout pattern

CI Status Update: Significant Progress

Current Status (Job 58387879075)

Good News:

  • playwright-ci-postgresql (1, 6): SUCCESS
  • playwright-ci-postgresql (2, 6): SUCCESS
  • playwright-ci-postgresql (3, 6): SUCCESS
  • playwright-ci-postgresql (4, 6): SUCCESS
  • playwright-ci-postgresql (5, 6): IN PROGRESS
  • playwright-ci-postgresql (6, 6): FAILURE

This is significant progress! Jobs 1-4 are now passing, suggesting either:

  1. Test timeouts were increased in those jobs, OR
  2. Lineage API is being mocked in those jobs, OR
  3. Those jobs test entity types less affected by lineage loading

Remaining Failures in Job 6/6

Job 58387879075 (6/6) shows the same failure pattern as previously analyzed:

  1. Users › Data Steward permissions - Timeout (5000ms)
  2. Users › User Profile change/removal - Timeout (5000ms)
  3. EntityVersion › Spreadsheet version pages - TypeError: Cannot read properties of undefined (reading 'name')

Error Pattern (Unchanged):

Error: Timed out 5000ms waiting for expect(locator).toBeVisible()
TypeError: Cannot read properties of undefined (reading 'name')

Root Cause (Same as Previously Documented)

The failures in jobs 5/6 and 6/6 are caused by async lineage fetching for 15+ entity types. The LineageSection component delays page rendering, causing:

  • 5-second timeout failures in visibility checks (User tests)
  • TypeError in Spreadsheet version pages (null/undefined lineage data)

Why Jobs 1-4 Now Pass

Possible reasons (would need to verify):

  1. Test distribution: Jobs 1-4 may test entity types with lighter lineage data or no lineage support
  2. Test order: Entity types that load faster (TABLE, etc.) may be in earlier jobs
  3. Timeout configuration: Different timeout settings per job
  4. Mock setup: Lineage API mocking may be configured for jobs 1-4

Remaining Work

To resolve the remaining failures in jobs 5/6 and 6/6:

Option 1: Extend Fixes to All Jobs

If jobs 1-4 were fixed with increased timeouts or mocks, apply the same approach to jobs 5/6 and 6/6:

// playwright.config.ts or test setup
export default {
  timeout: 90000, // Increased timeout
  expect: {
    timeout: 15000 // Increased from 5000
  }
};

// Mock lineage API for all tests
test.beforeEach(async ({ page }) => {
  await page.route('**/api/v1/lineage/**', route => {
    route.fulfill({
      status: 200,
      body: JSON.stringify({ nodes: {}, upstreamEdges: {}, downstreamEdges: {} })
    });
  });
});

Option 2: Fix Spreadsheet TypeError

The Spreadsheet version page error suggests defensive null checking is needed:

// In EntityVersion or related component
const entityName = lineageData?.entity?.name ?? entity?.name ?? 'Unknown';
// Instead of:
const entityName = lineageData.entity.name; // Throws if undefined

Option 3: Defer Lineage for Specific Scenarios

// Don't fetch lineage in version history views
if (activeTab === EntityRightPanelTab.OVERVIEW && !isVersionView) {
  fetchLineageData();
}

Summary

Progress: 4 out of 6 playwright jobs now passing ✅

Remaining Issues: Jobs 5/6 and 6/6 with same timeout pattern

Impact Reduced: From 50+ failing tests across 6 jobs down to remaining failures in 2 jobs

Next Steps:

  1. Verify what changes enabled jobs 1-4 to pass
  2. Apply same fixes to jobs 5/6 and 6/6
  3. Add null safety for Spreadsheet version TypeError

Conclusion: The PR is progressing toward CI success. Once the approach that fixed jobs 1-4 is applied to the remaining jobs, all tests should pass.

Code Review 👍 Approved with suggestions

Well-structured feature addition with comprehensive test coverage; previous race condition finding has been addressed.

Suggestions 💡 1 suggestion
Code Quality: Non-null assertion still used for fullyQualifiedName

📄 openmetadata-ui/src/main/resources/ui/src/components/DataAssetSummaryPanelV1/DataAssetSummaryPanelV1.tsx:203

The code at line 203 (in the new useMemo for lineage counts) still uses a non-null assertion (dataAsset.fullyQualifiedName!) when calling getUpstreamDownstreamNodesEdges. While there's a guard with isEmpty(dataAsset.fullyQualifiedName) returning early, the non-null assertion is still used after that check passes.

This is technically safe due to the guard, but using as string or providing a default value would be cleaner and more explicit:

getUpstreamDownstreamNodesEdges(
  edges,
  nodes,
  dataAsset.fullyQualifiedName as string  // or dataAsset.fullyQualifiedName ?? ''
);

This is a minor style concern since the guard protects against the null/undefined case.

Resolved ✅ 1 resolved
Edge Case: Potential race condition in fetchLineageData on entity change

📄 openmetadata-ui/src/main/resources/ui/src/components/Explore/EntitySummaryPanel/EntitySummaryPanel.component.tsx:258-298
The code attempts to handle race conditions when entities change, but there's a subtle issue. When the entity changes rapidly, the cleanup effect that resets lineage data (setLineageData(null)) runs before fetchLineageData() completes. However, the stale check in fetchLineageData compares entityDetails?.details?.fullyQualifiedName !== currentFqn which could still pass if the entity changes multiple times to entities with the same FQN (rare but possible).

Additionally, if the user switches entities very quickly, multiple requests could be in-flight. While stale responses are ignored, there's no request cancellation mechanism which could lead to unnecessary network traffic.

Suggested fix:
Consider using an AbortController to cancel in-flight requests when the entity changes:

useEffect(() => {
  const controller = new AbortController();
  fetchLineageData(controller.signal);
  return () => controller.abort();
}, [entityDetails?.details?.id]);

This is a minor issue since the current implementation does prevent stale data from being displayed.

What Works Well

  • The race condition in fetchLineageData has been properly addressed by capturing the FQN at the start of the async call and checking if the entity changed before updating state
  • A separate useEffect resets lineage data when entity ID changes, preventing stale data display
  • Comprehensive test coverage for both the new LineageSection component and the integration tests for various edge cases
  • Clean component architecture with proper separation (interface, styles, component, tests)
  • Complete i18n support with translations added for all 18 locale files
  • Good handling of loading and empty states in the LineageSection component

Recommendations

  • Consider using type assertion (as string) instead of non-null assertion (!) for fullyQualifiedName for slightly cleaner code style, though this is functionally safe due to the guard

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off Gitar will not commit updates to this branch.
✅ Code review is on Gitar will review this change.
Display: compact Hiding non-applicable rules.

Comment with these commands to change:

Auto-apply ✅ Code review Compact
gitar auto-apply:on         
gitar code-review:off         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | This comment will update automatically (Docs)

@sonarqubecloud
Copy link

@harsh-vador harsh-vador merged commit 6d953d8 into main Dec 18, 2025
35 of 43 checks passed
@harsh-vador harsh-vador deleted the add-lineage-section branch December 18, 2025 06:02
@github-actions
Copy link
Contributor

Changes have been cherry-picked to the 1.11.4 branch.

github-actions bot pushed a commit that referenced this pull request Dec 18, 2025
* ui: add lineage section in overview tab in right panel

* add tests

* fix failing tests

* fix loader

* add tests around loader

* fix failing specs

* fix loader issue

* address copilot comment

* address comments

* fix infinite loader issue

* address comments

(cherry picked from commit 6d953d8)
siddhant1 pushed a commit that referenced this pull request Dec 18, 2025
* ui: add lineage section in overview tab in right panel

* add tests

* fix failing tests

* fix loader

* add tests around loader

* fix failing specs

* fix loader issue

* address copilot comment

* address comments

* fix infinite loader issue

* address comments
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.

Upstream & Downstream count in Lineage section at Explore page panel

3 participants