Add ODCS type: library quality metric support

[Christopher-Lawford]

Summary

• Adds native mapping from ODCS type: library quality entries to DQX checks for the five supported metrics: rowCount, nullValues, missingValues, invalidValues, and duplicateValues.
• Each metric maps its eight shared ODCS threshold fields (mustBe, mustNotBe, mustBeGreaterOrEqualTo, mustBeLessOrEqualTo, mustBeGreaterThan, mustBeLessThan, mustBeBetween, mustNotBeBetween) onto exact-fit DQX aggregate checks where possible, with a dataset-level sql_query fallback for strict inequalities and the (both-bounds-exclusive, per ODCS) mustBeBetween/mustNotBeBetween forms.
• Malformed or unrecognized type: library entries (missing/unknown metric, no recognized threshold field, malformed arguments, unrecognized unit, misplaced property/schema-level entries) are warned-and-skipped per entry rather than failing the whole contract; this processing is unconditional, with no opt-out flag.
• Adds end-to-end integration coverage generating and applying rules from a contract exercising all five metrics.
• Adds a new "Metric Rule Generation" guide section (and updates the "Metadata Fields" table) documenting the feature for end users.

Test plan

• tests/unit/test_datacontract_generator.py::TestDataContractGeneratorLibraryRules — unit coverage per metric/threshold-field combination
• tests/unit/test_checks_semantic_validator.py — updated coverage for the semantic validator changes
• tests/integration/test_datacontract_integration.py — end-to-end generation + apply_checks_by_metadata against a real DataFrame for all five metrics
• make test / make lint run clean on this branch (please confirm in CI)

🤖 Generated with Claude Code

[github-actions]

All commits in PR should be signed ('git commit -S ...'). See https://docs.github.com/en/authentication/managing-commit-signature-verification/signing-commits

[CLAassistant]

All committers have signed the CLA.

[mwojtyczka]

Out of scope for this PR (should be a follow up): I would implement the missing functions and replace the sql query:

• mustBeGreaterThan (enforce metric > X): new function is_aggr_greater_than(limit=X)
• mustBeLessThan (enforce metric < X): new function is_aggr_less_than(limit=X)
• mustBeBetween (enforce lo < metric < hi): new function is_aggr_in_range(lo, hi)
• mustNotBeBetween (enforce metric ≤ lo OR metric ≥ hi): new function is_aggr_not_in_range(lo, hi)

Can you please create a follow up issue for this?

[Christopher-Lawford]

Filed #1495 for this, covering is_aggr_greater_than, is_aggr_less_than, is_aggr_in_range, and is_aggr_not_in_range.

[mwojtyczka]

Automated code review — ODCS type: library quality metrics

Verified against head d2b6471. One serious correctness bug plus a serialization/validation cluster and a few lower-severity consistency issues. Details are in the inline comments; summary:

Severity	Finding
🔴 High	duplicateValues (any non-zero threshold) nests COUNT(*) OVER (...) inside a SUM/AVG aggregate → Spark AnalysisException at apply time. Only mustBe: 0 is execution-tested.
🟠 Med	nullValues percent and missingValues forbidden embed live F.when/F.lit Column objects in the generated rule dicts → not YAML/JSON-serializable (save_checks fails). Other percent paths already use SQL strings to avoid this.
🟠 Med	Because of the above, ChecksSemanticValidator silently skips conflict detection for those rules (unhashable Column in the key → TypeError swallowed).
🟡 Low	RLIKE string literals don't escape backslashes → regex patterns mangled vs the row-level regex_match path.
🟡 Low	Numeric validValues stringified into NOT IN ('..') → string vs numeric comparison mismatch with is_in_list.
🟡 Low	mustBe == 0 matches boolean False, misses string "0".
🟡 Low	Multiple rowCount entries on one schema collide on rule name.

Cleared: the sql_query fallback for mustBeGreaterThan/mustBeLessThan/mustBeBetween/mustNotBeBetween is correct — DQX has no strict-inequality or between aggregate check (only is_aggr_not_greater_than/not_less_than/equal/not_equal). SQL-injection via threshold interpolation is not viable — those fields are typed float | int.

[mwojtyczka]

Conflict detection is silently skipped for Column-valued arguments.

When a generated check carries a Column as its column argument (the nullValues-percent path in contract_rules_generator.py), _make_hashable returns the Column unchanged (it is neither list/tuple/dict), so the _conflict_key tuple contains an unhashable Column. In _conflict_issue, conflict_key not in seen then raises TypeError, which detect_conflicts catches and skips — so two genuinely conflicting generated rules on the same column are never flagged.

(Duplicate detection via _full_key is unaffected — it stringifies through json.dumps(default=str).) Fixing the root cause — keeping generated column args as SQL strings rather than Column objects — resolves this as well.

[Christopher-Lawford]

The generator no longer emits a Column object as a check argument anywhere. The nullValues-percent path builds a SQL string indicator instead of an F.when(...) Column, so _make_hashable/_conflict_key never sees an unhashable Column here. We also reverted the checks_semantic_validator.py truthiness-hardening change and its test, since fixing the generator made it unnecessary, per vb-dbrks's separate comment on this file.

[mwojtyczka]

Going in the right direction. Left some comments

[vb-dbrks]

Thanks for taking this on, and for turning the last round around so quickly .. this is careful work and the previous fixes all landed cleanly (no pyspark import in the generator at all now, backslash and numeric literal handling covered with tests, mustBe normalisation handled, rowCount names carrying the threshold field).

Requesting changes. I went back to the ODCS docs and compared our mapping against other ODCS tooling, rather than just reading the code. There are a few places where we would report different numbers than other implementations for the same contract, which matters here because portability is the whole reason to support library rather than telling people to use type: custom. Details are inline; the summary:

• duplicateValues counts rows in duplicated groups, and ODCS tools disagree with each other on this. Ours matches vowl; datacontract-cli counts distinct recurring values instead. The spec does not settle it, so this needs documenting rather than changing.
• mustBeBetween / mustNotBeBetween are exclusive on both bounds, and the docs attribute that to the spec, which does not say it. Other tooling reads it inclusively.
• NULL is counted on the non-zero missingValues paths but not at mustBe: 0, so the metric means two different things depending on the threshold.
• invalidValues with both validValues and pattern becomes two independent thresholds rather than one invalid count.
• _is_dqx_library_rule only matches an explicit type: library, so the feature is inert for contracts written the way the spec documents them. One-line fix.
• duplicateValues still raises at apply time for every non-zero threshold, and the integration test written for it is misplaced and cannot fail.

Where the spec is genuinely silent (which is most of the above) I am not asking you to adopt anyone else's reading .. I would like the choice stated explicitly in the docs, and ideally raised with Bitol so the spec pins it down.

Two product asks

An opt-out flag. generate_rules_from_contract already has generate_predefined_rules and process_text_rules. Making library processing unconditional is inconsistent with that and leaves no escape hatch if a mapping misbehaves on someone's contract. Please add process_library_rules: bool = True alongside the existing flags.

Related: there is no dedup against the predefined path, so required: true plus a nullValues / mustBe: 0 entry on the same property produces two identical is_not_null checks under different names. Not a blocker, but we should pick a behaviour and document it.

Docs should position this as the fallback, not the recommended path. This is the part I feel strongest about, and it is not a criticism of the implementation.

There is no capability gain here. All five metrics were already expressible in DQX .. tolerances via is_aggr_* over a count or percentage indicator, rowCount via column: "*" with count, composite keys via is_unique with a column list, per-check severity via criticality. I checked each one. What this adds is authoring convenience, not new checking ability, and the docs currently read like it is the preferred way to express quality in a contract.

The coverage will also always be lopsided. ODCS library is five metrics. DQX has dozens of checks .. outliers, freshness, foreign keys, schema validation, dataset comparison. Anyone with real requirements outgrows library straight away and ends up in type: custom with engine: dqx, which is the extension point ODCS designed for exactly this. Specific doc asks are inline on the guide.

I would also like the five-metric surface treated as closed once this lands. Supporting library is fine because it is the portable, engine-agnostic part of ODCS, but I do not want it growing into a general ODCS interpreter. Every addition is permanent maintenance keyed to a vocabulary we do not control.

Scope

Per the principle Marcin set out for the new aggregate functions, the checks_semantic_validator.py change should also come out of this PR .. it is a core change and this PR no longer needs it. Comment inline.

+1 to Marcin's follow-up for is_aggr_greater_than / is_aggr_less_than / is_aggr_in_range / is_aggr_not_in_range. Worth having in core regardless of ODCS, and it would let most of these sql_query fallbacks go away.

One transparency note: parts of this review were machine-assisted, and the apply-time claims (the duplicateValues InvalidParameterError in particular) are read from the source rather than reproduced against Spark, so please sanity-check those as you go.

[vb-dbrks]

Good catch on the bare-string-resolves-as-a-column-expression trap, that is an easy one to miss.

One gap though: this escapes quotes but not backslashes, while _sql_scalar_literal does both. So a \N sentinel is compared differently on the mustBe: 0 path than on the non-zero paths for the same contract value. Worth aligning the two.

[Christopher-Lawford]

Fixed. _is_in_list_literal now escapes backslashes the same way _sql_scalar_literal does, doubling \ before doubling '. A \N-style sentinel now compares consistently between the mustBe: 0 path and the aggregate/sql_query paths for the same contract value.

[vb-dbrks]

Still open after this round. _is_in_list_literal escapes single quotes but not backslashes, while _sql_scalar_literal does both and its docstring explains why. So a \N sentinel, or a Windows-style path in validValues/missingValues, is compared one way on the mustBe: 0 path and another on every other threshold, for the same contract value.

This is the cheapest of the duplication fixes and it closes the whole class .. one literal escaper used by both paths. Of the three drift bugs we have hit (numeric literals, NULL inclusion, this one) it is the last one standing.

[vb-dbrks]

Really nice work on this round. The dispatch fix, the inclusive range semantics, the duplicateValues rewrite and the docs framing all landed exactly right, and the two admonitions in the guide are better than what I asked for. I have resolved eleven of my twelve earlier threads.

We are almost there. One blocker and a handful of smaller things, all inline. The blocker is that column: "*" combined with row_filter does not count what it looks like it counts, which hits the default unit: rows path for three of the five metrics.

I would like all of these addressed in this PR rather than as follow-ups, so the feature lands complete.

[vb-dbrks]

column: "*" with a row_filter does not count what it looks like it counts.

In _is_aggr_compare, a non-empty row_filter wraps the column as F.when(F.expr(row_filter), F.expr("*")), and Spark expands the star into the CASE WHEN's positional argument list instead of using it as the THEN value. Verified against serverless:

• 2-column DataFrame with 2 nulls: the analyzed plan is count(CASE WHEN isnull(c) THEN id ELSE c END) and the result is 4, not 2. Silently wrong, no error.
• 3-column DataFrame: apply_checks_by_metadata raises AnalysisException [DATATYPE_MISMATCH.UNEXPECTED_INPUT_TYPE], failing the whole run rather than just this check.

Most real DataFrames have three or more columns, so the usual outcome is a hard failure. Same shape at :2532 (invalidValues) and :2816 (missingValues). It covers the default unit: rows on mustBe (non-zero), mustNotBe, mustBeGreaterOrEqualTo and mustBeLessOrEqualTo .. 12 of the 40 metric/threshold combinations, and the path the new guide advertises. rowCount is unaffected since it never sets a row_filter.

The fix is lighter than what is there, and the right pattern is already one method below: _nullvalues_percent_aggregate_check passes an indicator SQL string. Do the same for the rows path, {"column": "CASE WHEN <cond> THEN 1 END", "aggr_type": "count"}, and the star/row_filter mechanism drops out entirely.

Separately, validate_star_aggregate explicitly permits * with count under a row filter, so core is wrong here too. That wants its own issue against _is_aggr_compare, but the generator should not wait on it.

[vb-dbrks]

The suite currently cannot tell a correct mapping from a broken one.

There are 24 assertions on row_filter in this file and they assert the column: "*" + row_filter shape as expected output, so they lock in the defect described on contract_rules_generator.py:2197 rather than catching it. With the fixture only covering mustBe: 0 (row level), one mustBeGreaterThan (sql_query) and a rowCount aggregate with no row filter, nothing executes the aggregate path at all.

This is the third round where a fix has introduced a new apply-time bug (window nested in an aggregate, then the one-row sql_query, now the star). That is a test-design gap, not anything about the changes themselves. Could we add integration coverage that actually calls apply_checks_by_metadata for each metric x threshold x unit, and treat the dict-shape assertions as secondary? The duplicateValues test you added this round is exactly the right model .. it would have caught all three.

[vb-dbrks]

Contract literals containing SQL keywords fail at apply time, and whether they do depends on unit.

The percent path interpolates contract-supplied literals into the sql_query string, and is_sql_query_safe rejects any query containing insert, update, delete, create, merge, replace, refresh, optimize ... as a whole word. A CDC-style entry is enough:

metric: invalidValues
arguments:
  validValues: [INSERT, UPDATE, DELETE]
unit: percent
mustBeLessThan: 5

Verified: validate_checks reports no errors, then apply_checks_by_metadata raises UnsafeSqlQueryError. The unit: rows variant is fine because the literals go into row_filter, which is not safety checked. So the same contract works or fails depending on unit, which nobody would predict. The missingValues percent path has the same issue. Keeping contract literals out of the query string avoids it.

[vb-dbrks]

Percent-unit thresholds take down the whole run on an empty DataFrame.

AVG(CASE WHEN ... END) is NULL over zero rows, so the generated condition returns NULL, _apply_dataset_level_sql_check produces a NullType column, and apply_checks fails with an AnalysisException that kills every other check in the run, not just this one. Verified on serverless with an empty input. An empty micro-batch or an empty first run is normal operation.

COALESCE(AVG(...), 0) in the generated query fixes it, and the NULLIF((SELECT COUNT(*) ...), 0) in _duplicate_values_count_expr needs the same treatment.

[vb-dbrks]

mustBe and mustNotBe are Any typed and interpolated raw into SQL.

The other six threshold fields are typed float | int by the ODCS model; these two are Any and go straight into the query string here and just below. So mustBe: "abc" is a SQL parse error at apply time, mustBe: [1,2] produces <> [1, 2], and mustBe: "0 OR 1=1" produces a check that can never fail. is_sql_query_safe only blocks destructive keywords, so it will not catch these. A numeric guard that warns and skips otherwise would match how unknown units and unknown metrics are already handled.

[vb-dbrks]

Library rules silently ignore the ODCS row filter, which widens the check's scope.

_quality_row_filter is consumed only in _build_explicit_rule_from_implementation, so a row_filter/rowFilter customProperty is honoured on a type: custom entry but dropped on a type: library one, and the check then runs against the whole dataset. Silently checking a wider population than the contract asked for is worse than skipping the rule, because the result looks valid. A call to _quality_row_filter here would make the two paths consistent.

[vb-dbrks]

metric is the current spelling, but the ODCS model still carries rule (annotated # Deprecated: Use metric instead) and contracts authored against 3.0.x use it .. those entries currently fall through and generate nothing. Accepting quality_rule.metric or quality_rule.rule here would cover them. One line, and it matches how the spec treats the two.

[vb-dbrks]

Each metric's predicate is implemented twice, and the copies keep drifting.

For four of the five metrics the "is this row bad" logic exists once as a delegation to a core DQX check and again as hand-written SQL here:

metric	core check (mustBe: 0)	hand-rolled SQL
nullValues	is_not_null	col IS NULL
invalidValues	is_in_list + regex_match	NOT IN (...) / NOT (col RLIKE ...)
missingValues	is_not_in_list	col IS NULL OR col IN (...)
duplicateValues	is_unique	GROUP BY/HAVING subquery

With regex_match it is three, counting the predefined path from logicalTypeOptions.pattern.

This is not a style point. The two copies have already disagreed three times and each one shipped as a bug: numeric literals stringified on one path only (fixed), NULL counted on one path only (fixed this round), and backslash escaping, still open on the thread below. Same failure three times from one cause.

The shape that removes it is one predicate per metric with two renderings: keep the row-level core check for mustBe: 0 because per-row reporting beats a count, and build the count/avg indicator from that same predicate. It also collapses most of the per-metric method families.

[vb-dbrks]

The module has taken on more responsibilities than it can hold.

DataContractRulesGenerator is now 2935 lines and 126 methods, covering ODCS loading, physical-type-to-DDL validation, schema validation rules, predefined rules from logicalTypeOptions, LLM text rules, explicit passthrough, library metrics, and a SQL generation and escaping layer. Library metrics alone are about half of it, 39 of the 126 methods.

Two things would help, and I think both belong in this PR while the code is fresh:

• Extract the library metric code into datacontract/library_metrics.py. Pure code motion, no behaviour change, and it stops the generator being half adapter. This is the best single answer to keeping the ODCS-specific surface contained.
• Make the per-metric families extension points rather than copies. Roughly 31 methods are per-metric against 12 shared, so adding a sixth ODCS metric means writing another 6 to 9 methods .. the class is open for modification rather than extension. A small handler per metric exposing its predicate and its argument expectations, with the threshold and unit rendering owned once by the caller, would turn that into one new class.

The vocabulary constants right here are exactly the right instinct .. _SUPPORTED_LIBRARY_METRICS, _THRESHOLD_FIELDS and _LIBRARY_METRIC_DEFAULT_DIMENSIONS keep spec churn contained to one place, which was my main worry earlier. This is about the logic around them.

Related: the seven SQL helpers (_safe_sql_identifier, _sql_scalar_literal, _is_in_list_literal, _is_library_pattern_safe, and the three condition/expression builders) are a small SQL generation subsystem living in a contract parsing module. Whatever survives the consolidation above wants to be in one place, with one literal escaper rather than two that disagree.

[vb-dbrks]

Withdrawing this one, my mistake .. you have already documented exactly this. The guide covers it under Malformed and Unrecognized Entries: "DQX does not deduplicate library metric rules against predefined rules generated from schema constraints", with the required: true plus nullValues/mustBe: 0 case spelled out and the generate_predefined_rules=False workaround. That is the "keep both and say so in the guide" option, which is what I was asking for. Nothing to do here.

[vb-dbrks]

The docs are in good shape .. I checked all seven YAML examples against the generator and every one parses, produces rules, and the inline → annotations name the check that is actually generated. The three admonitions, the threshold-field ordering and the new Metadata Fields rows are all genuinely useful.

Suggestions below rather than comments, so they can be applied directly. Two themes: keep a specific third-party tool out of the docs, and move implementation rationale out of user-facing prose into the code comments where it already lives. The one substantive change is the positioning admonition .. see that suggestion for the reasoning.

One thing I could not raise as a suggestion because it sits outside this PR's diff: the grouping snippet in Complete Usage Example (around lines 881-883) filters rule_type on predefined, explicit and text_llm only, so anyone copying it silently drops every metric rule from their counts. The inline comment in the metadata sample just above it ("rule_type": "predefined" # or "explicit" or "text_llm") has the same gap. The Metadata Fields table itself is correct and already lists metric, so it is only those two spots that are out of step.

[vb-dbrks]

Reframing this as an on-ramp rather than a fallback. The substance I would change: library metrics are for people who are already ODCS-native or have contracts in hand and want DQX enforcing them today, and the long-term home for checks is native DQX rules or text expectations, which is where the flexibility and the feature surface are. I have also dropped the "not a capability DQX otherwise lacks / already expressible with is_aggr_*" clause .. that is our internal reasoning about whether to build this, and in user docs it reads as DQX apologising for its own feature. The coverage boundary and the closed-surface sentence are the parts that genuinely help a reader, so both are kept.

Suggested change

	<Admonition type="note" title="A fallback for simple contracts, not the recommended path">
	Library metrics exist for **portability**: the same five-metric vocabulary is understood by any ODCS-compliant tool, including [datacontract-cli](https://github.com/datacontract/datacontract-cli). They are not a capability DQX otherwise lacks — every one of the five is already expressible directly with `is_aggr_*` checks, `is_unique`, and `column: "*"` — and they cover a narrow slice of what DQX can check (no outlier detection, freshness, foreign keys, schema validation, or dataset comparison). For anything beyond simple row-count/null/duplicate/allowlist checks on a contract that must stay portable across tools, prefer [explicit rules](#explicit-rule-generation) with `type: custom` and `engine: dqx`, which give you the full DQX check surface. Treat `type: library` as the option for the simple, common cases, not the default way to express quality in a contract.
	This five-metric surface is intentionally closed: DQX does not plan to grow this mapping into a general ODCS interpreter for arbitrary quality vocabularies.
	</Admonition>
	<Admonition type="note" title="A fast on-ramp, not the long-term home for your checks">
	Library metrics are for teams who are already ODCS-native, or who have existing ODCS contracts and want DQX enforcing them straight away. Point DQX at a contract you already have and you get checks immediately, with no rewrite — and the same five-metric vocabulary is understood by any other ODCS-compliant tool.
	Treat that as a starting point. The five metrics cover the common cases, but not the rest of what DQX can check: no outlier detection, freshness, foreign keys, schema validation, or dataset comparison. Longer term you will get more out of defining checks natively, either as [explicit rules](#explicit-rule-generation) with `type: custom` and `engine: dqx` or as [text-based expectations](#text-based-rule-generation) — both give you the full DQX check surface and far more flexibility than a fixed metric vocabulary.
	This five-metric surface is intentionally closed: DQX does not plan to grow this mapping into a general ODCS interpreter for arbitrary quality vocabularies.
	</Admonition>

[vb-dbrks]

Genericised .. we would rather not advertise or link a specific third-party tool here. The point stands without naming one.

Suggested change

Several of the mapping decisions below are not settled by the ODCS spec text. Where that's the case, DQX matches [datacontract-cli](https://github.com/datacontract/datacontract-cli)'s reference mapping (onto [Soda Core](https://github.com/sodadata/soda-core) checks) rather than inventing its own reading, so a contract produces the same verdict regardless of which tool evaluates it. Each judgment call is called out explicitly where it applies: `duplicateValues`'s counting unit, `mustBeBetween`/`mustNotBeBetween`'s bound inclusivity, and `invalidValues`'s combined-condition handling of `validValues` + `pattern`.

Several of the mapping decisions below are not settled by the ODCS spec text. Where that's the case, DQX follows the interpretation already established by other ODCS-compliant tooling rather than inventing its own reading, so a contract produces the same verdict regardless of which tool evaluates it. Each judgment call is called out explicitly where it applies: `duplicateValues`'s counting unit, `mustBeBetween`/`mustNotBeBetween`'s bound inclusivity, and `invalidValues`'s combined-condition handling of `validValues` + `pattern`.

[vb-dbrks]

Two things here. Dropping condition_column: "condition", the GROUP BY ... HAVING / PARTITION BY / SUM/AVG detail and the "Spark rejects a window function nested inside an aggregate" sentence: that is implementation rationale, it is already in the code comment where it belongs, and a contract author reading this is often a governance or architecture person who will never touch PySpark. Everything they need is kept, including the inclusivity behaviour. Also genericised the tool reference.

Suggested change

	- **`mustBeGreaterThan`, `mustBeLessThan`, `mustBeBetween`, `mustNotBeBetween`** have no strict-inequality/range equivalent among DQX's aggregate checks, so they fall back to a dataset-level [`sql_query`](/docs/reference/quality_checks#using-sql-query) check with `condition_column: "condition"` (`true` means a violation). For `mustBeBetween`/`mustNotBeBetween`, **both bounds are inclusive** — a value exactly equal to either bound counts as being "between" them. The ODCS spec text doesn't settle this either way; DQX matches datacontract-cli's reference mapping (SodaCL's plain `between`, which is inclusive on both ends unless a bound is written with a round bracket) rather than picking its own reading.
	- **`duplicateValues` is the one exception**: every non-`mustBe: 0` threshold (not just the four strict-inequality/range ones above) falls back to `sql_query`. The duplicate count/percentage is computed via a `GROUP BY ... HAVING` subquery rather than a plain aggregate expression, since Spark rejects a window function (the `PARTITION BY` used to detect duplicates) nested inside an aggregate function (`SUM`/`AVG`) — see [duplicateValues](#duplicatevalues) below for what it counts.
	- **`mustBeGreaterThan`, `mustBeLessThan`, `mustBeBetween`, `mustNotBeBetween`** have no strict-inequality or range equivalent among DQX's aggregate checks, so they fall back to a dataset-level [`sql_query`](/docs/reference/quality_checks#using-sql-query) check. For `mustBeBetween`/`mustNotBeBetween`, **both bounds are inclusive** — a value exactly equal to either bound counts as being "between" them. The ODCS spec doesn't settle this either way; DQX follows the interpretation already used by other ODCS-compliant tooling rather than picking its own reading.
	- **`duplicateValues` is the one exception**: every non-`mustBe: 0` threshold falls back to `sql_query`, not just the four above. See [duplicateValues](#duplicatevalues) below for exactly what it counts.

[vb-dbrks]

All nine library entries in the examples set type: library explicitly, even though the opener correctly says it can be omitted when metric is set .. and every example in the ODCS spec omits it, so contracts arriving from other tooling will look like this rather than like our examples. Worth demonstrating the canonical form at least once. Also drops "indicator", which is internal vocabulary.

Suggested change

	- type: library
	metric: nullValues
	mustBeLessOrEqualTo: 5
	unit: percent # → is_aggr_not_greater_than over the null-percentage indicator
	# `type: library` can be omitted when `metric` is set, which is how the ODCS
	# spec's own examples are written -- DQX recognizes both forms.
	- metric: nullValues
	mustBeLessOrEqualTo: 5
	unit: percent # → dataset-level check on the null percentage

[vb-dbrks]

Genericised the tool reference. The substance is unchanged .. pattern support is still flagged as going beyond what most implementations cover.

Suggested change

`invalidValues` accepts an allowlist (`arguments.validValues`), a regex (`arguments.pattern`), or both — a row fails if it matches neither. When both are present, DQX combines them into a single "invalid" condition (`NOT (in the allowlist) OR NOT (matches the pattern)`) rather than treating them as two independent thresholds, so `mustBeLessOrEqualTo: 5` means at most 5 rows failing *either* criterion, not up to 5 failing each independently. This combination is a DQX judgment call — datacontract-cli's reference mapping ignores `arguments.pattern` for `invalidValues` entirely and only honors `validValues`, so `pattern` support here is a DQX-only extension beyond what the reference implementation covers, even though the spec documents the field:

`invalidValues` accepts an allowlist (`arguments.validValues`), a regex (`arguments.pattern`), or both — a row fails if it matches neither. When both are present, DQX combines them into a single "invalid" condition (`NOT (in the allowlist) OR NOT (matches the pattern)`) rather than treating them as two independent thresholds, so `mustBeLessOrEqualTo: 5` means at most 5 rows failing *either* criterion, not up to 5 failing each independently. This combination is a DQX judgment call: other ODCS tooling commonly honors only `validValues` and ignores `arguments.pattern` for `invalidValues`, so `pattern` support here goes beyond what most implementations cover, even though the spec documents the field:

[vb-dbrks]

Genericised the tool reference. The counting choice and the worked example are unchanged.

Suggested change

For a non-`mustBe: 0` threshold, `duplicateValues` counts the number of *distinct values (or key combinations) that recur* — for a column holding `[A, A, A, B, B, C]` that's 2 (`A` and `B` each recur), not 5 (the rows sitting in those two groups). `unit: percent` divides that count by the total row count. This matches datacontract-cli's reference mapping onto Soda's `duplicate_count`; the ODCS spec text ("Counts duplicate values in a column") doesn't settle it either way, so DQX follows the reference rather than counting affected rows. `mustBe: 0` is unaffected by this choice, since both readings agree at zero.

For a non-`mustBe: 0` threshold, `duplicateValues` counts the number of *distinct values (or key combinations) that recur* — for a column holding `[A, A, A, B, B, C]` that's 2 (`A` and `B` each recur), not 5 (the rows sitting in those two groups). `unit: percent` divides that count by the total row count. This matches how other ODCS-compliant tooling counts duplicates; the ODCS spec text ("Counts duplicate values in a column") doesn't settle it either way, so DQX follows the established interpretation rather than counting affected rows. `mustBe: 0` is unaffected by this choice, since both readings agree at zero.