Fixes #33231: [DQ Threshold W2.4] Build row-level violation counts for columnValuesToBeBetween / LengthsToBeBetween

[TeddyCr]

This pull request was opened by an olivaw agent. It is an input to review, not an approved change — read the diff, not this description.

Fixes #33231

What olivaw checked (machine-observed, §8.2/A.1)

• diff: non-empty
• build: not_run
• tests: not_run
• base: 4399f56ef732ca7e92381a50626fe31916529214
• head: 2fcf6e26324cc4f09e52073cffb79b0c2f5cb9ff

No build or test command is configured for this workspace, so only the diff's existence was checked. Set run_config.build_cmd/test_cmd to have olivaw verify its own work.

What the agent says it did (unverified)

Done — committed on agent/issue-33231.

What I built

The counting. BetweenBoundsChecker already had both halves (build_row_level_violations_sqa() and get_violations_mask()); they were only used by the dimensional queries. Each engine now has a _run_violation_count(column, test_params) that uses them for the overall result too:

• SQA — one aggregate query via a new SQAValidatorMixin._compute_row_violations(): COUNT(*) and the SUM(CASE …) in the same scan, so the count and its denominator can't disagree.
• pandas — one vectorized mask per dataframe, accumulated without concatenating, matching how every other aggregate in these validators is computed.

Both are wired from _run_validation() in the two base validators, and only when a tolerance is configured — with none, a value outside the window and a MIN/MAX outside it are the same verdict, so nothing extra is queried and the result is unchanged.

The verdict. _evaluate_test_condition() falls back to MIN/MAX as before, but with a tolerance it goes through the shared _apply_row_threshold(). MIN/MAX stay in testResultValue; the message leads with the count and keeps the window and extremes as context; the counts are also reported as passed/failed rows since they're already in hand.

Two fixes the counting needed: the infinite-bound check now only asks math.isinf of a float (a datetime window used to raise, which silently emptied dimensional results for date columns), and a COALESCE around the SUM so an empty table counts zero rather than NULL.

Decisions I made

These two no longer widen their bounds with the threshold. W2.2 had them call get_bounds(), which applies the deviation tolerance. Counting rows against an already-widened window would spend one threshold twice. Both test definitions describe the parameter as "number of failures tolerated … or a share of the evaluated rows", so the row reading is the declared contract. Bounds still resolve through get_min_bound/get_max_bound, so dynamic assertion keeps working, and run_validation() is untouched — the Collate MRO constraint holds. Caveat worth checking on the Collate side: if those subclasses override _get_test_parameters() and call get_bounds() themselves, they'd re-introduce the double application and should follow suit.

I put the wiring in _run_validation rather than _get_metrics_to_compute. The issue suggested the latter, but that method maps names to registry Metrics enums and only feeds the dimensional path; there is no registry metric for "values outside this window". Adding a fake entry would have injected a dead key into dimensional metric values. I documented the reasoning in both _get_metrics_to_compute docstrings instead. This is reversible if you'd rather have the literal shape.

Verification

I verified the checker empirically — SQLite and pandas over the same 7-row fixture both return 3 violations, NULLs excluded, one-sided bounds correct, empty table → 0, datetime bounds no longer raising. Ruff and format pass.

I could not run the repo's unit tests: metadata.generated is build-time output, and make generate fetches schemas from open-metadata.org and 404s here (installing the package outright also fails on an unrelated dbt dependency that downloads a wheel from GitHub). So the new test module — ingestion/tests/unit/observability/data_quality/validations/test_between_row_violations.py, covering hand-computed counts, SQA/pandas agreement, the four threshold cases, MIN/MAX presence, and the no-tolerance path — is written against the existing fixture patterns but has not been executed. It needs a CI run before this is trusted.

Confidence Score: 5/5

The PR appears safe to merge, with no outstanding correctness or repository-rule violations identified.

Summary

This PR adds row-level violation counting for the value and value-length “between” validators when a failure threshold is configured.

• Applies absolute or percentage thresholds to the number of values outside the configured bounds.
• Implements equivalent counting for SQLAlchemy and pandas runners while excluding null values from violations.
• Reuses the verdict’s counts for passed/failed-row reporting instead of performing a potentially inconsistent second scan.
• Preserves the existing MIN/MAX-based path when no threshold is configured.
• Adds coverage for threshold outcomes, one-sided bounds, empty datasets, datetime bounds, and SQL/pandas agreement.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    Input[Column values] --> Metrics[Compute MIN/MAX]
    Input --> Count{Threshold configured?}
    Count -->|No| Extremes[Evaluate MIN/MAX against bounds]
    Count -->|Yes| Violations[Count total and out-of-range rows]
    Violations --> Threshold[Apply absolute or percentage threshold]
    Extremes --> Result[Test result]
    Threshold --> Result
    Violations --> Reporting[Populate passed and failed rows]

Loading

_{Reviews (2) · Last reviewed commit: "Review: report the counted violations, a..."}

[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]

✅ Playwright Results — workflow succeeded

Validated commit 59581d6226feff182909c914a49af5202390e0a2 in Playwright run 36505994597, attempt 1.

✅ 614 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) 55m 14s

⏱️ Max setup 5m 3s · max shard execution 15m 37s · max shard-job elapsed before upload 21m 24s · reporting 5s

🌐 239.36 requests/attempt · 2.22 app boots/UI scenario · 12.66% common-shard skew

Optimization targets still in progress:

• Browser traffic was 239.36 requests per attempt (convergence target: fewer than 200).
• Application boot ratio was 2.22 per UI scenario (1429 boots / 644 scenarios; convergence target: at most 1).

Shard	Passed	Failed	Flaky	Skipped	Lifecycle failed	Lifecycle flaky
✅ Shard chromium-01	127	0	0	0	0	0
🟡 Shard chromium-02	129	0	1	0	0	0
✅ Shard chromium-03	136	0	0	0	0	0
✅ Shard data-asset-rules-01	65	0	0	0	0	0
✅ Shard domain-isolation-01	16	0	0	0	0	0
✅ Shard global-state-01	34	0	0	0	0	0
✅ Shard ingestion-01	35	0	0	0	0	0
✅ Shard ingestion-02	30	0	0	0	0	0
✅ Shard reindex-01	2	0	0	0	0	0
✅ Shard search-01	11	0	0	0	0	0
✅ Shard search-rbac-01	29	0	0	0	0	0

🟡 1 flaky test(s) (passed on retry)

• Pages/Entity.spec.ts › Announcement create, edit & delete (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

[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.

[Copilot]

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

[gitar-bot]

Code Review ✅ Approved

🟡 Medium risk · Adds row-count-based tolerance verdicts to two data quality validators.

Adds row-level violation counting for columnValuesToBeBetween and lengthsToBeBetween validators when a failure threshold is configured, with equivalent implementations for SQLAlchemy and pandas engines that exclude nulls and reuse the verdict's counts for passed/failed-row reporting. No issues found.

Review coverage

📋 Rules No rules evaluated

🧪 Functional validation Not enabled · Set up

🤖 Auto-approval Not enabled · Set up

Options

Display: compact → Counting what did not apply, without listing it.

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}

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ingestion'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
90.8% Coverage on New Code
3.4% Duplication on New Code

See analysis details on SonarQube Cloud