Skip to content

Fix filters impact analysis#24877

Merged
chirag-madlani merged 15 commits intomainfrom
fix-filters-impact-analysis
Dec 24, 2025
Merged

Fix filters impact analysis#24877
chirag-madlani merged 15 commits intomainfrom
fix-filters-impact-analysis

Conversation

@chirag-madlani
Copy link
Copy Markdown
Collaborator

@chirag-madlani chirag-madlani commented Dec 17, 2025

This pull request introduces several improvements and refactorings to the lineage and impact analysis features, focusing on streamlining column-level lineage representation, enhancing search/filter functionality, and updating related tests and constants. The most significant changes include standardizing the data model for column-level lineage, improving quick filter performance, and refactoring code for clarity and maintainability.

Column-level lineage and impact analysis improvements:

  • Standardized the column-level lineage data structure by introducing explicit fromColumn and toColumn fields in the ColumnLevelLineageNode interface, replacing the previous nested column property. Updated all relevant usages, including table columns, constants, and test mocks, to use the new structure. [1] [2] [3] [4] [5]
  • Refactored the rendering logic for column names and entity names to use the new structure and utility methods, ensuring consistent highlighting and display. [1] [2] [3]

Search and filter enhancements:

  • Added an optionPageSize prop to ExploreQuickFilters and passed a large page size (AGGREGATE_PAGE_SIZE_LARGE) for aggregate field options, improving performance and usability when filtering large datasets. [1] [2] [3] [4] [5] [6]
  • Increased the typing interval for search input debounce from 0 to 300ms to reduce unnecessary searches and improve responsiveness.

Codebase and API refactoring:

  • Simplified and clarified the aggregate field options API by refactoring postAggregateFieldOptions to accept a single SearchRequest object, consolidating parameters and improving maintainability.
  • Removed unused imports and streamlined utility usage in LineageTable, including switching from getPartialNameFromTableFQN to Fqn.split for FQN parsing. [1] [2] [3]

Testing improvements:

  • Updated and expanded mocks in LineageTable.test.tsx to match the new data structure and utility usage, ensuring tests remain accurate and maintainable. Also improved test selectors for menu interactions. [1] [2] [3] [4]

Minor UI/UX fixes:

  • Removed unnecessary margin from .selectable-list-item in the user-select dropdown for a cleaner appearance.This pull request introduces several improvements and refactors to the lineage and quick filter components, focusing on enhancing column-level lineage analysis, optimizing quick filter aggregation, and improving code maintainability. The most impactful changes include restructuring the data model for column-level lineage, optimizing aggregation option fetching, and updating UI components for clarity and performance.

Column-level lineage improvements:

  • Refactored the ColumnLevelLineageNode interface to use flat fromColumn and toColumn properties instead of nested structures, simplifying data handling and improving clarity in the lineage table. [1] [2] [3] [4] [5] [6] [7]
  • Updated the lineage table rendering logic to use the new fromColumn and toColumn fields, improved sorting, and streamlined the rendering of column and entity names, removing unnecessary utility dependencies. [1] [2] [3] [4] [5]

Quick filter and aggregation optimization:

  • Added an optionPageSize prop to quick filter components, allowing for larger aggregation result sets (defaulting to 1000), and updated the aggregation fetching logic to support customizable page sizes. [1] [2] [3] [4] [5] [6] [7]
  • Refactored aggregation API calls to accept a unified request object, improving maintainability and flexibility for future enhancements. [1] [2]

UI/UX and performance enhancements:

  • Increased the typing interval for search input debounce to 300ms in lineage controls, reducing unnecessary search requests and improving performance.
  • Removed unnecessary margins in user-selectable list items for a cleaner UI.

Code cleanup and maintainability:

  • Removed unused imports and legacy code related to FQN utilities, further streamlining the codebase. [1] [2]
  • Improved error logging when fetching markdown files, aiding in debugging.

These changes collectively improve the scalability, maintainability, and user experience of the lineage and quick filter features.

Describe your changes:

Fixes

I worked on ... because ...

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

  • Fixed filter performance:
    • Increased aggregation page size from 10 to 1000 using AGGREGATE_PAGE_SIZE_LARGE constant for lineage impact analysis filters
  • Column-level lineage refactor:
    • Simplified data structure from nested column: { fromColumns, toColumn } to flat fromColumn and toColumn properties in ColumnLevelLineageNode interface
  • Fixed sorting:
    • Replaced broken server-side sorting with client-side localeCompare functions for all columns in column impact table
  • Enhanced search:
    • Made getSearchNameEsQuery column-level aware, searching columns.name.keyword and columns.displayName.keyword when in column-level mode
  • API refactoring:
    • Changed postAggregateFieldOptions from positional parameters to object destructuring pattern for better maintainability

This will update automatically on new commits.


@github-actions github-actions Bot added safe to test Add this label to run secure Github workflows on PRs UI UI specific issues labels Dec 17, 2025
Copy link
Copy Markdown
Contributor

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 16 out of 16 changed files in this pull request and generated 4 comments.

Comment thread openmetadata-ui/src/main/resources/ui/src/utils/ExploreUtils.tsx
Comment thread openmetadata-ui/src/main/resources/ui/src/rest/miscAPI.ts
@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented Dec 23, 2025

🔍 CI failure analysis for 6f573ee: Playwright tests failed due to timeouts in Glossary, Bulk Import, and Landing Page Widget tests - completely unrelated to this PR's lineage/filter changes. Likely infrastructure or pre-existing flaky test issues.

Issue

The playwright-ci-postgresql (2, 6) job failed with multiple E2E test timeout and browser closure errors. Five specific tests encountered failures:

  1. Glossary - Remove domain from glossary (failed twice including retry)

    • page.waitForResponse: Test timeout of 180000ms exceeded
    • locator.click: Test timeout of 180000ms exceeded
  2. Bulk Import - Export Database service

    • page.click: Test timeout of 300000ms exceeded
  3. Glossary - Root of different glossary

    • locator.click: Test timeout of 60000ms exceeded
  4. Landing Page Widget - Present in following widget

    • page.goto: Target page, context or browser has been closed

Root Cause

These failures are NOT related to this PR's changes. This PR exclusively modifies:

  • Lineage components (EntityLineage, LineageTable, LineageUtils)
  • Explore/QuickFilters components
  • Lineage and search utilities
  • API utilities (miscAPI.ts)

The failing tests cover completely different functionality:

  • Glossary operations (domain management, term navigation)
  • Bulk import/export operations
  • Landing page widgets

None of these test areas overlap with lineage impact analysis or explore filters functionality modified in this PR.

Details

The timeout errors indicate:

  1. Infrastructure/performance issues - Tests exceeded timeouts ranging from 60s to 300s (5 minutes)
  2. Possible backend slowness - waitForResponse timeouts suggest API calls not completing
  3. Environment instability - "Target page, context or browser has been closed" indicates test environment problems
  4. Potential flakiness - The "Remove domain from glossary" test was retried but failed again with the same timeout, which can indicate either consistent infrastructure issues or a pre-existing problem unrelated to this PR

These patterns suggest either:

  • CI environment resource constraints or slowness
  • Pre-existing test flakiness or bugs in the Glossary/Bulk Import/Widget features
  • Backend service performance degradation during this test run

The failures are environment/infrastructure-related rather than code defects introduced by this PR's lineage and filter improvements.

Code Review 👍 Approved with suggestions

Good refactor addressing pagination bug and simplifying lineage data structures, with one minor logging concern.

Suggestions 💡 1 suggestion
Code Quality: Logging expected fallback behavior to console is unnecessary

📄 openmetadata-ui/src/main/resources/ui/src/rest/miscAPI.ts:233-238

The console.info added in fetchMarkdownFile logs when a relative path fails URL parsing. This is expected behavior (relative paths will fail new URL()) and the fallback to '/' is intentional. Logging this creates console noise during normal operation.

Impact: Minor - adds unnecessary console output during normal app operation.

Suggestion: Remove the console.info statement since this is expected behavior, not an error condition:

} catch (error) {
  // Relative paths will fail URL parsing, fall back to default
  baseURL = '/';
}

If debugging is needed, consider using a debug-level logger that can be disabled in production.

Resolved ✅ 5 resolved
Bug: Pagination slicing logic is incorrect (off-by-one)

📄 openmetadata-ui/src/main/resources/ui/src/components/LineageTable/LineageTable.tsx:746
The pagination slicing logic at line 746 uses slice(currentPage - 1, currentPage - 1 + pageSize) which is incorrect. For page-based pagination:

  • Page 1 with pageSize 10: should return items 0-9 (index 0 to 10)
  • Page 2 with pageSize 10: should return items 10-19 (index 10 to 20)

Current code for page 2: slice(2 - 1, 2 - 1 + 10) = slice(1, 11) - returns items 1-10 instead of 10-19.

Correct formula:

const source = nodes.slice((currentPage - 1) * pageSize, currentPage * pageSize);

This will cause users to see incorrect data on pages beyond the first page, with results overlapping and missing items.

Bug: Missing `impactLevel` in useMemo dependency for `columnImpactColumns`

📄 openmetadata-ui/src/main/resources/ui/src/components/LineageTable/LineageTable.tsx:1129
The columnImpactColumns definition uses lineageDirection in the dependency array, but the column definitions now have hardcoded field names (fromEntity, toEntity, fromColumn, toColumn) instead of conditionally using different fields based on direction.

While this simplification might be intentional, if impactLevel is being used anywhere within columnImpactColumns rendering (indirectly through searchValue which now uses impactLevel), the memoization won't capture all dependencies.

However, looking more closely, the main issue is that the combinedQuery useMemo now correctly includes impactLevel (line 1129), which is good. But columnImpactColumns doesn't need impactLevel since it no longer conditionally switches columns based on direction - this is actually a correct simplification.

No action required - upon closer inspection, this is a valid refactor that simplifies the code by using fixed column keys and the memoization is correct.

Code Quality: Test mock data uses incorrect property name

📄 openmetadata-ui/src/main/resources/ui/src/components/LineageTable/LineageTable.test.tsx:347-367
In LineageTable.test.tsx, the mock data was changed from column to columns:

-        column: { fromColumns: ['col1'], toColumn: ['col2'] },
+        columns: { fromColumns: ['col1'], toColumn: ['col2'] },

However, based on the production code changes in LineageUtils.tsx, the new structure uses fromColumn and toColumn as top-level properties (not nested under column or columns):

fromColumn: fromCol,
toColumn: col.toColumn,

The test mock should align with the actual production data structure to ensure meaningful test coverage. The current mock may cause tests to pass while not actually validating the correct data structure.

Suggested fix: Update the mock to use the flattened structure:

{
  fromColumn: 'col1',
  toColumn: 'col2',
}
Bug: getAggregateFieldOptions call uses incorrect signature

📄 openmetadata-ui/src/main/resources/ui/src/utils/ExploreUtils.tsx:364-380
The getAggregateFieldOptions function call was changed to use an object parameter ({ index, fieldName, ... }), but looking at the original function signature from the diff context, getAggregateFieldOptions appears to take positional arguments ((index, key, value, filter, undefined, deleted)), not an object.

In the diff at lines 363-380, the function now calls:

getAggregateFieldOptions({
  index,
  fieldName: key,
  fieldValue: value,
  query: filter,
  deleted,
})

But only postAggregateFieldOptions was refactored to accept an object parameter. The getAggregateFieldOptions function still uses positional parameters based on the original code shown at line 203-205.

Impact: This will cause a runtime error as the function will receive an object where it expects individual string/boolean arguments.

Suggested fix: Either update getAggregateFieldOptions to also accept an object parameter (like was done for postAggregateFieldOptions), or keep using positional arguments for getAggregateFieldOptions while adding the size parameter where needed.

Edge Case: Sorter function may crash on undefined fullyQualifiedName

📄 openmetadata-ui/src/main/resources/ui/src/components/LineageTable/LineageTable.tsx:651-652 📄 openmetadata-ui/src/main/resources/ui/src/components/LineageTable/LineageTable.tsx:662
The sorter functions use optional chaining for the first operand but could still throw if fromEntity or toEntity is undefined:

sorter: (a, b) =>
  a.fromEntity.fullyQualifiedName?.localeCompare(
    b.fromEntity.fullyQualifiedName ?? ''
  ),

If a.fromEntity is undefined/null, this will throw a TypeError since we're accessing .fullyQualifiedName on it.

Impact: Potential runtime crash when sorting if any row has missing entity references.

Suggested fix: Add null checks for the entity objects themselves:

sorter: (a, b) =>
  (a.fromEntity?.fullyQualifiedName ?? '').localeCompare(
    b.fromEntity?.fullyQualifiedName ?? ''
  ),

What Works Well

  • Pagination fix: The off-by-one bug in column lineage pagination slicing has been correctly fixed with proper index calculation: (currentPage - 1) * pageSize to currentPage * pageSize
  • Data structure simplification: Flattening column: { fromColumns, toColumn } to direct fromColumn/toColumn properties improves clarity and simplifies column rendering
  • Comprehensive test coverage: Extensive new tests added for ExploreQuickFilters covering props handling, URL parsing, search functionality, tier field handling, and error cases
  • API improvements: The postAggregateFieldOptions signature refactored to accept a single SearchRequest object is cleaner and more extensible

Recommendations

  • Consider removing the console.info for expected fallback behavior in fetchMarkdownFile to reduce console noise

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
Copy Markdown

@chirag-madlani chirag-madlani merged commit e2e1474 into main Dec 24, 2025
28 of 29 checks passed
@chirag-madlani chirag-madlani deleted the fix-filters-impact-analysis branch December 24, 2025 12:13
ShaileshParmar11 pushed a commit that referenced this pull request Dec 26, 2025
* fix(ui): impact analysis filter and sorting

* support sorting and highlight to column level

* fix dropdown issue

* add debounce to search value

* add coverage

* fix tests

* fix tests

* update tests

* fix sonar comment

* Address PR review comments: fix useMemo dependencies and remove deprecated properties (#24981)

* Initial plan

* Address review comments: fix useMemo deps, remove deprecated props, fix test mocks

Co-authored-by: chirag-madlani <12962843+chirag-madlani@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: chirag-madlani <12962843+chirag-madlani@users.noreply.github.com>

* fix pagination logic

---------

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
chirag-madlani added a commit that referenced this pull request Feb 10, 2026
* fix(ui): impact analysis filter and sorting

* support sorting and highlight to column level

* fix dropdown issue

* add debounce to search value

* add coverage

* fix tests

* fix tests

* update tests

* fix sonar comment

* Address PR review comments: fix useMemo dependencies and remove deprecated properties (#24981)

* Initial plan

* Address review comments: fix useMemo deps, remove deprecated props, fix test mocks

Co-authored-by: chirag-madlani <12962843+chirag-madlani@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: chirag-madlani <12962843+chirag-madlani@users.noreply.github.com>

* fix pagination logic

---------

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
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 UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants