Fixes #30361: Show failed rows sample when a test passes within its threshold

[TeddyCr]

Describe your changes:

Fixes #30361

A test case can pass while some rows still break its rule, as long as the failures stay under its failure threshold. For example, 7 failed rows with a threshold of 7 is a Success. Those rows are what a DQ team needs to see before the threshold is crossed, but three separate gates kept the failed rows sample for Failed results only:

Layer	Before	After
Ingestion (FailedSampleValidatorMixin)	sampled only on Failed	samples on Failed, or Success with failedRows > 0
Server (PUT /dataQuality/testCases/{id}/failedRowsSample)	400 unless the latest result is Failed	accepts under the same rule
UI (FailedTestCaseSampleData)	fetched only on Failed	fetches under the same rule

What stays the same:

• Ingestion still samples only when computePassedFailedRowCount is on.
• A clean pass (failedRows == 0 or unknown) still costs no extra query.
• A current Success result still deletes the stored sample on the server. The sink posts the result before the sample, so a Success within its threshold replaces the sample and a clean Success clears it.

The rule and the write order it depends on are recorded in docs/decisions/2026-10-09-failed-rows-sample-follows-failed-rows-not-status.md and cited at each gate.

Type of change:

• Improvement

High-level design:

Each layer applies the same check, "does the latest result have failed rows?", instead of "is the latest result Failed?". There is no schema change: TestCaseResult.failedRows already exists and the threshold work already fills it.

Alternative considered: dropping the server's delete-on-Success so that a sample outlives its run. I rejected it because the delete keeps the sample tied to the latest run, and the sink's write order already makes the delete correct for both kinds of Success.

Out of scope: the Warning status suggested in the issue thread. It needs a new testCaseStatus enum value plus decisions on incidents and alerting, so it is better tracked separately.

Tests:

Local Java test run: PASSED

• Commit: eadd865c47aa (base upstream/main) — uncommitted changes were present
• Finished: 2026-10-09 20:29 UTC · 13.1 min

Step	Scope	Classes	Tests	Failed	Skipped	Min	Result
unit	openmetadata-mcp (1 class)	1	4	0	0	1.2	passed
integration	mysql-elasticsearch · parallel	29	1934	0	131	6.0	passed
integration	mysql-elasticsearch · isolated	1	1	0	0	2.9	passed
integration	postgres-opensearch · search-it	1	1	0	0	3.0	passed

Tests run locally

• unit · openmetadata-mcp (1 class): CreateTestCaseToolTest (4 passed)
• integration · mysql-elasticsearch · parallel: AutoAssignIncidentIT (41 passed, 1 skipped), AutoCloseIncidentIT (95 passed, 12 skipped), BaseEntityIT$ListEntityHistoryByTimestampPaginationTest (27 passed, 2 skipped), ConfigResourceIT (12 passed), DataQualityDimensionMigrationIT (2 passed), DataQualityDimensionPropagationIT (10 passed), DataQualityLineageIT (24 passed), DatabaseSmokeIT (8 passed), EntityProfileResourceIT (11 passed), GlossaryTermResourceIT (82 passed, 2 skipped), IncidentGroupsIT (163 passed, 11 skipped), IncidentPaginationIT (19 passed, 1 skipped), IncidentTaskIntegrationIT (274 passed, 37 skipped), IncidentTaskNotificationIT (8 passed), LatestTestCaseResultListingIT (130 passed, 10 skipped), OrphanedTimeSeriesCleanupIT (44 passed, 2 skipped), SeedDataPresenceIT (1 passed), SqlTestDecoratorsIT (6 passed), StaleIncidentStatusIT (42 passed), TableResourceIT (5 passed), TestCaseCsvDomainIsolationIT (21 passed), TestCaseDeleteResilienceIT (25 passed), TestCaseLatestStatusIT (47 passed, 5 skipped), TestCaseRelationshipRetentionCleanupIT (27 passed), TestCaseResolutionStatusLegacyTestSuitesIT (1 passed), TestCaseResourceIT (358 passed, 11 skipped), TestCaseSoftDeleteSearchIT (1 passed), TestDefinitionResourceIT (31 passed, 4 skipped), TestSuiteResourceIT (31 passed, 2 skipped), UserResourceIT (62 passed, 7 skipped)
• integration · mysql-elasticsearch · isolated: TestSuiteReindexIT (1 passed)
• integration · postgres-opensearch · search-it: TestCaseStaleDefinitionReindexIT (1 passed)

Per-class counts come from the test reports, which can credit a test to the wrong class when classes run concurrently, and keep only the last run of a nested class that several test classes inherit. The step totals above are Maven's own counts.

Why these tests

• area data-quality ← openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java
• called from entity TestCase ← openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java
• changed test ← openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/TestCaseResourceIT.java
• entity TestCase ← openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java
• smoke ← code other than a test class changed

Commands

mvn -B package -pl openmetadata-mcp -am -Dspring-boot.repackage.skip=true -Dtest=CreateTestCaseToolTest -Dsurefire.failIfNoSpecifiedTests=false
mvn -B verify -pl :openmetadata-integration-tests -am -Dspring-boot.repackage.skip=true -Pmysql-elasticsearch -DintegrationTests.lane=parallel -Dit.test=AutoAssignIncidentIT,AutoCloseIncidentIT,ConfigResourceIT,DataQualityDimensionMigrationIT,DataQualityDimensionPropagationIT,DataQualityLineageIT,DatabaseSmokeIT,EntityProfileResourceIT,GlossaryTermResourceIT,IncidentGroupsIT,IncidentPaginationIT,IncidentTaskIntegrationIT,IncidentTaskNotificationIT,LatestTestCaseResultListingIT,OrphanedTimeSeriesCleanupIT,SeedDataPresenceIT,SqlTestDecoratorsIT,StaleIncidentStatusIT,TableResourceIT,TestCaseCsvDomainIsolationIT,TestCaseDeleteResilienceIT,TestCaseLatestStatusIT,TestCaseRelationshipRetentionCleanupIT,TestCaseResolutionStatusLegacyTestSuitesIT,TestCaseResourceIT,TestCaseSoftDeleteSearchIT,TestDefinitionResourceIT,TestSuiteResourceIT,UserResourceIT -Dfailsafe.failIfNoSpecifiedTests=false -Dtest=NoUnitTestsInThisRun -Dsurefire.failIfNoSpecifiedTests=false
mvn -B verify -pl :openmetadata-integration-tests -am -Dspring-boot.repackage.skip=true -Pmysql-elasticsearch -DintegrationTests.lane=global-state -Dit.test=TestSuiteReindexIT -Dfailsafe.failIfNoSpecifiedTests=false -Dtest=NoUnitTestsInThisRun -Dsurefire.failIfNoSpecifiedTests=false
mvn -B verify -pl :openmetadata-integration-tests -am -Dspring-boot.repackage.skip=true -Psearch-it -DdatabaseType=postgres -DsearchType=opensearch -Djpw.data.mode=ingest -Dit.test=TestCaseStaleDefinitionReindexIT -Dfailsafe.failIfNoSpecifiedTests=false -Dtest=NoUnitTestsInThisRun -Dsurefire.failIfNoSpecifiedTests=false

Use cases covered

• A test passing within its threshold with failed rows stores the sample and the inspection query, and the UI shows them.
• A test passing with no failed rows stores and fetches nothing.
• A failing test behaves as before.
• The server rejects a sample for a clean Success.

Unit tests

• ingestion/tests/unit/data_quality/validations/test_failed_sample_mixin.py: Success with failedRows > 0 is sampled; Success with failedRows 0 or None, the flag off, or Aborted is not. 16 passed.
• Whole DQ unit suite (tests/unit/data_quality, tests/unit/observability/data_quality): 778 passed. 9 failed in test_clickzetta_data_diff.py with ModuleNotFoundError: clickzetta, an optional dependency missing from my local venv and unrelated to this change.
• FailedTestCaseSampleData.test.tsx: added "passed within its threshold → fetches". Both suites in the folder pass (12 tests). Checked that the new test fails when the Success branch is reverted.

Backend integration tests

• TestCaseResourceIT#test_failedRowsSampleAcceptedWhenPassingWithinThreshold (new)
• TestCaseResourceIT#test_failedRowsSampleRejectedWhenPassingWithoutFailedRows (new)

Review fixes and verification (5c66ebb717c; supersedes earlier local-test notes)

• Fixed stale UI samples when a new result has the same status/failed-row count, or a Failed result becomes a Success with failed rows. The effect now also follows result timestamp, status, and failed-row count.
• Fixed an overlapping-request race: an older response no longer clears the latest request's loading state. Previous rows are cleared when a replacement fetch starts.
• Added API regressions for no result, unknown failed-row count, Aborted status, replacement samples, and clean-pass clearing; UI regressions for result changes, overlapping requests, permissions, version-page actions, and deletion; Python regressions for sampling/query-generation errors preserving the successful result.
• Python mixin unit tests: 18 passed, 94% line coverage. Broader DQ suite: 778 passed; 9 failed only because the optional clickzetta package is absent from the local environment.
• UI component and sample-table unit tests: 27 passed. The affected component's measured line coverage is 99.11%.
• Real PostgreSQL ingestion integration file against this PR's isolated backend: 11 passed, including the threshold-success sample and clean-pass cases.
• Both affected Playwright scenarios passed against this PR's isolated backend and rebuilt worktree component library (3 passed including authentication setup): failed-sample highlighting/deletion, and passing-with-failed-rows navigation/reload plus clean-pass/no-sample gates. Earlier attempts were blocked by API-test authentication configuration and a stale shared component-library build; both test-harness issues were corrected without tracked source changes.
• Java affected-test runner: all four steps passed; exact commands/counts are in the local Java block above (1,940 tests including 131 skipped).
• Java Spotless, Python Ruff, changed-file UI import/lint/format checks, full Playwright lint, Java impact-map completeness, and git diff --check passed. Lint has pre-existing warnings. The full Playwright TypeScript check still reports errors in untouched helpers, not this changed spec.
• CI is monitored separately on the live pushed commit; skipped checks are not counted as executed tests.

Ingestion integration tests

• ingestion/tests/integration/data_quality/test_failed_row_samples.py::test_passing_within_threshold_publishes_failed_samples (new): columnValuesToBeNotNull on bad_data_customer.email (about 10% null) with a 50% threshold passes, and the sample holds only null-email rows.
• Ran the file against a local server built without the server change: 10 passed, and the new test failed with 400 Failed rows can only be added to a failed test case. That is the guard this PR relaxes. The server side is covered by the ITs above; the end-to-end Python test has not yet been re-run against a server that includes this change.

Playwright (UI) tests

• TODO: add a spec that seeds a Success result with failedRows > 0 and checks that the sample table renders.

Manual testing performed

• TODO

UI screen recording / screenshots:

TODO: attach a recording of the sample table on a test case that passed within its threshold.

Checklist:

• I have read the CONTRIBUTING document.

• My PR title is Fixes <issue-number>: <short explanation>

• My PR is linked to a GitHub issue via Fixes #<issue-number> above.

• 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. (N/A, no schema change)

• For UI changes: I attached a screen recording and/or screenshots above.

• I have added tests (unit / integration / Playwright as applicable) and listed them above.

• Decision check: adds docs/decisions/2026-10-09-failed-rows-sample-follows-failed-rows-not-status.md.

• I have added tests around the new logic.

🤖 Generated with Claude Code

Final CI monitoring result (5c66ebb717c59a560d8cd706c6d0c7083591285b)

All checks reached a terminal state: 42 successful, 27 skipped, 2 failed. Skipped jobs are not counted as executed tests. Collate, the Python 3.10/3.11/3.12 unit/static matrix and coverage aggregation, service unit tests, UI coverage/quality gates, Java/UI/Python formatting, and Maven/Sonar completed successfully.

The two failures occurred before their tests ran, while pulling unchanged Docker images:

• Python operator build/test job: initial environment setup received HTTP 429 pulling eclipse-temurin:21-jdk. A single failed-job rerun also received HTTP 429 while building the existing PR-label action from python:3.6.10-alpine3.10.
• Visual regression job: environment setup received HTTP 429 pulling alpine:3; browser assertions never ran.

CI is not green. These infrastructure failures need a later successful rerun once registry pulls are available. No workflow, Dockerfile, credentials, or labels were changed to bypass the failures. No remaining blocking code defect was identified in the reviewed paths after the committed fixes; this is not a claim that the failed CI jobs passed.

Confidence Score: 4/5

Fix the custom SQL sampling override before merging so passing tests with failed rows receive the promised sample.

Findings

1.  Custom SQL samples stay missing ▶

Summary

Allows failed-row samples for Success results with failedRows > 0 across the shared ingestion mixin, server, and UI.

• Passing test runs with failed rows now keep and show their sample.

Diagram

sequenceDiagram
  participant Validator
  participant Sink
  participant Server
  participant UI
  Validator->>Validator: "Success with failedRows > 0"
  Validator->>Validator: Collect sample when row counts are enabled
  Validator->>Sink: Result and sample
  Sink->>Server: Write result
  Server->>Server: Delete previous sample on Success
  Sink->>Server: Write replacement sample
  UI->>Server: Fetch sample with view permission
  Server-->>UI: Sample rows

Loading

_{Reviews (1) · Last reviewed commit: "Merge branch 'main' into failed-rows-sam..." · Reviewed by Greptile}

[Copilot]

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

[Copilot]

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

[github-actions]

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 4 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), 4 warning(s) across 2 changed file(s).

Count	Rule
2	openmetadata-imports/no-lower-layer-page-imports
1	react-hooks/exhaustive-deps
1	openmetadata-ui-patterns/no-non-adaptive-palette

All findings

	Location	Rule	Message
🟡	src/components/DataQuality/IncidentManager/FailedTestCaseSampleData/FailedTestCaseSampleData.component.tsx:41:1	openmetadata-imports/no-lower-layer-page-imports	Pages are route-level composition modules. Move the shared implementation/type to a lower layer instead of importing a page from here.
🟡	src/components/DataQuality/IncidentManager/FailedTestCaseSampleData/FailedTestCaseSampleData.component.tsx:264:6	react-hooks/exhaustive-deps	React Hook useEffect has a missing dependency: 'fetchFailedTestCaseSampleData'. Either include it or remove the dependency array.
🟡	src/components/DataQuality/IncidentManager/FailedTestCaseSampleData/FailedTestCaseSampleData.component.tsx:366:27	openmetadata-ui-patterns/no-non-adaptive-palette	Raw palette class "tw:border-r-gray-blue-100" is static — it does not flip in dark mode. Use the theme-adapting "utility-" variant (tw:border-r-utility-gray-blu
🟡	src/components/DataQuality/IncidentManager/FailedTestCaseSampleData/FailedTestCaseSampleData.test.tsx:20:1	openmetadata-imports/no-lower-layer-page-imports	Pages are route-level composition modules. Move the shared implementation/type to a lower layer instead of importing a page from here.

---

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

make ui-checkstyle-changed

[github-actions]

Jest test Coverage

UI tests summary

Lines	Statements	Branches	Functions
	78.19% (128440/164259)	64.86% (81484/125620)	66.15% (27213/41136)

[Copilot]

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

[github-actions]

🚦 Removed from the merge queue — merge_conflict (2026-10-13T14:22:01Z)

The entry left the queue before it was built, so no checks ran against it.

Conflicts with the latest main — rebase, then re-queue.

[Copilot]

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

[github-actions]

🚦 Removed from the merge queue — failed_checks (2026-10-15T03:00:06Z)

Blocked the queue: integration-tests-postgres-opensearch

• Integration Tests - PostgreSQL + OpenSearch — integration-tests-postgres-opensearch, Integration Test Lane (parallel)

[github-actions]

🚦 Removed from the merge queue — failed_checks (2026-10-16T01:38:40Z)

Blocked the queue: ui-coverage

• Prune dead merge-queue refs — cancel-stale-runs
• SonarCloud + Jest Coverage — ui-coverage, ui-sonar-gate, ui-coverage-tests

[github-actions]

🚦 Removed from the merge queue — failed_checks (2026-10-16T16:19:03Z)

Blocked the queue: integration-tests-postgres-opensearch

• playwright-visual-regression — playwright-visual-regression
• Integration Tests - PostgreSQL + OpenSearch — integration-tests-postgres-opensearch, Integration Test Lane (rdf)

[gitar-bot]

Code Review ✅ Approved 1 closed / 1 findings

🟡 Medium risk · Allowing threshold-passing tests to retain and display failed-row samples changes ingestion, API, and UI behavior; incorrect lifecycle gating could show stale rows or hide diagnostics.

Extends failed-rows sample sampling, server acceptance, and UI fetching from Failed-only to any result with failed rows, so a test that passes within its threshold still shows its failing rows. Resolved a javadoc placement issue on toTestCaseStatus. No open issues found.

✅ 1 closed

✅ Quality: New javadoc inserted under toTestCaseStatus's doc, orphaning it

📄 openmetadata-service/src/main/java/org/openmetadata/service/resources/dqtests/TestCaseResource.java:1575-1580
hasFailedRows was added between toTestCaseStatus and its existing javadoc (/** Matching is case insensitive, the status is normalized to its canonical enum value. */, line 1575). That leaves two javadoc blocks stacked on hasFailedRows. Javadoc only attaches the closer one, so the case-insensitive matching note is now orphaned and no longer documents toTestCaseStatus. Move the one-line javadoc down so it sits directly above toTestCaseStatus again.

Verification

Rules No rules evaluated

Functional validation Not enabled

Set up

Auto-approval Not enabled

Set up

_{For more verification details, comment gitar display:verbose.}

Options

Display: compact → Showing compact verification results.

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

Compact

gitar display:verbose

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

[greptile-apps]

Custom SQL samples stay missing

Changing the shared mixin does not cover tableCustomSQLQuery, whose SQLAlchemy validator overrides result_with_failed_samples and still requires Failed. With strategy=ROWS, operator="<=", threshold=7, and computePassedFailedRowCount=True, a query returning seven rows produces Success with failedRows=7. The override then skips both the sample and inspection query, so users still cannot inspect those rows.

Apply _has_failed_rows to that override too, keeping its ROWS check, and add a regression test.

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ui'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ingestion'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
81.8% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud