From 38b958f14896759323bbd31da1910c1ebf310659 Mon Sep 17 00:00:00 2001 From: Brian O'Kelley Date: Sun, 3 May 2026 10:09:18 -0400 Subject: [PATCH] feat(validation): oneOf near-miss validator hints + issues[].hint on every VALIDATION_ERROR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a discriminated-union (`oneOf`) shape fails validation because the caller used the wrong key as the discriminator (the v3 reference-seller `pricing_options` regression: `{"type": "cpm", ...}` instead of `{"pricing_model": "cpm", ...}`), an additive `hint` field on the `VALIDATION_ERROR` issue names the closest matching variant and the wrong / expected discriminator keys: Looks like you may have meant the 'cpm' variant. Use 'pricing_model' instead of 'type' as the discriminator. The hint is best-effort: when no clear winner exists across variants, no hint is emitted (silent is better than misleading). Clients that ignore the new field behave exactly as before — `hint` is only added to the wire envelope's `issues[i]` dict when populated. Heuristic (in `adcp.validation.oneof_hints.compute_oneof_hint`): 1. Walk to the schema's `oneOf` keyword via the failing issue's `absolute_schema_path`. 2. Detect the discriminator field — a property pinned by `const` in at least two variants. No discriminator -> no hint. 3. Score each variant by `(const_match, required_present, total_present)`: the strongest signal is whether any value in the payload matches a variant's discriminator `const` (the wrong-key case); shape match is the tiebreaker. Tie at the top -> no hint. 4. Identify the wrong key in the payload — first key whose value matches the best variant's `const`, or first key not declared by any variant's `properties`. Plumbed through `validate_request` / `validate_response` (which now forward the validator's resolved schema + payload to `_format_error`) and surfaced via the existing `_issue_to_wire` helper used by both `SchemaValidationError.details` and `build_adcp_validation_error_payload`. Closes #460. Refs #452. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/adcp/validation/oneof_hints.py | 276 +++++++++++++ src/adcp/validation/schema_errors.py | 16 +- src/adcp/validation/schema_validator.py | 64 +++- tests/test_oneof_hints.py | 489 ++++++++++++++++++++++++ 4 files changed, 822 insertions(+), 23 deletions(-) create mode 100644 src/adcp/validation/oneof_hints.py create mode 100644 tests/test_oneof_hints.py diff --git a/src/adcp/validation/oneof_hints.py b/src/adcp/validation/oneof_hints.py new file mode 100644 index 000000000..333b8db79 --- /dev/null +++ b/src/adcp/validation/oneof_hints.py @@ -0,0 +1,276 @@ +"""Heuristic ``hint`` strings for ``oneOf`` near-miss validation failures. + +When a payload fails a discriminated-union (``oneOf``) shape because the +caller used the wrong key as the discriminator (the v3 ref seller +``pricing_options`` regression: ``{"type": "cpm", ...}`` instead of +``{"pricing_model": "cpm", ...}``), the standard jsonschema diagnostic is +``" is not valid under any of the given schemas"`` — accurate but +unactionable for an LLM client. + +This module computes an additive ``hint`` string that names the closest +matching variant and the wrong / expected discriminator keys: + + Looks like you may have meant the 'cpm' variant. Use 'pricing_model' + instead of 'type' as the discriminator. + +The hint is best-effort: if no clear winner exists across variants, no +hint is emitted (silent is better than misleading). +""" + +from __future__ import annotations + +from typing import Any + + +def _navigate(schema: Any, path_segments: list[Any]) -> Any | None: + """Walk ``schema`` along ``path_segments`` (jsonschema absolute_schema_path). + + Returns the sub-schema at the path or ``None`` if any segment misses. + """ + node: Any = schema + for seg in path_segments: + if isinstance(node, dict): + if seg in node: + node = node[seg] + continue + # jsonschema sometimes emits int-as-string segments + if isinstance(seg, int) and str(seg) in node: + node = node[str(seg)] + continue + return None + if isinstance(node, list): + try: + node = node[int(seg)] + continue + except (ValueError, IndexError, TypeError): + return None + return None + return node + + +def _navigate_input(payload: Any, path_segments: list[Any]) -> Any | None: + """Walk the request/response payload along an instance path.""" + node: Any = payload + for seg in path_segments: + if isinstance(node, dict): + if seg in node: + node = node[seg] + continue + return None + if isinstance(node, list): + try: + node = node[int(seg)] + continue + except (ValueError, IndexError, TypeError): + return None + return None + return node + + +def _detect_discriminator(variants: list[dict[str, Any]]) -> str | None: + """Identify the discriminator field across ``oneOf`` variants. + + A field qualifies when at least two variants pin it to a literal + ``const``. Ties broken by the field with the most variants pinning + it; further ties broken by lexical order so the result is stable. + + The ``count >= 2`` floor distinguishes a real discriminator (a key + that genuinely partitions the union) from incidental ``const`` + pinning on a single variant. A union with only one variant pinning + a field is not discriminated by that field — applying near-miss + heuristics there would just guess. + """ + counts: dict[str, int] = {} + for variant in variants: + if not isinstance(variant, dict): + continue + props = variant.get("properties") + if not isinstance(props, dict): + continue + for field_name, field_schema in props.items(): + if isinstance(field_schema, dict) and "const" in field_schema: + counts[field_name] = counts.get(field_name, 0) + 1 + if not counts: + return None + # Pick the field pinned by the most variants (>=2 to be a real discriminator). + best = sorted(counts.items(), key=lambda kv: (-kv[1], kv[0])) + field_name, count = best[0] + if count < 2: + return None + return field_name + + +def _variant_const_value(variant: dict[str, Any], field: str) -> Any | None: + props = variant.get("properties") + if not isinstance(props, dict): + return None + field_schema = props.get(field) + if not isinstance(field_schema, dict): + return None + return field_schema.get("const") + + +def _score_variant( + variant: dict[str, Any], + value: dict[str, Any], + discriminator: str | None = None, +) -> tuple[int, int, int, str | None]: + """Score how close ``value`` is to a ``oneOf`` variant. + + Returns ``(const_match, required_present, total_present, seen_key)`` where: + + * ``const_match`` — 1 when the variant's discriminator ``const`` + value appears as the value of some top-level key in the payload + *other than* the expected discriminator. Strongest signal: the + caller picked this variant by value but used the wrong key + (the v3 ref-seller ``pricing_options`` regression). + * ``required_present`` — count of the variant's ``required`` fields + present in ``value``. The variant the caller most nearly hit by + shape. + * ``total_present`` — count of the variant's declared ``properties`` + present in ``value``. Tiebreaker. + * ``seen_key`` — the top-level key that carried the matching + ``const`` value, if any. Recorded so the hint can name the exact + key the caller misused rather than guessing later. + + The exact-pairing requirement (``key != discriminator AND + val == const_value``) replaces a membership scan against + ``value.values()``. The looser scan would mark a const_match when + an unrelated field happened to carry the same scalar (e.g., a + variant pinning ``"type": "object"`` matching a payload's + ``"label": "object"``), producing a misleading hint. + """ + required = variant.get("required") or [] + if not isinstance(required, list): + required = [] + required_present = sum(1 for r in required if isinstance(r, str) and r in value) + + properties = variant.get("properties") or {} + if not isinstance(properties, dict): + properties = {} + total_present = sum(1 for p in properties if p in value) + + const_match = 0 + seen_key: str | None = None + if discriminator is not None: + const_value = _variant_const_value(variant, discriminator) + if const_value is not None: + for key, val in value.items(): + if key == discriminator: + continue + if val == const_value: + const_match = 1 + seen_key = key + break + + return const_match, required_present, total_present, seen_key + + +def _fallback_seen_key( + value: dict[str, Any], + expected_discriminator: str, + variants: list[dict[str, Any]], +) -> str | None: + """Pick a likely "wrong discriminator" key when no const_match was found. + + Used only when the variant's score did not record a ``seen_key`` + via exact (key, val) pairing — i.e., the caller didn't carry the + expected variant's ``const`` value at all. In that case we fall + back to the first top-level key that isn't declared by any variant + (an extraneous key, plausibly the caller's misnamed discriminator). + """ + declared: set[str] = set() + for variant in variants: + if not isinstance(variant, dict): + continue + props = variant.get("properties") + if isinstance(props, dict): + declared.update(props.keys()) + + for key in value: + if key == expected_discriminator: + continue + if key not in declared: + return key + + return None + + +def compute_oneof_hint( + schema: dict[str, Any], + schema_path_segments: list[Any], + instance_path_segments: list[Any], + payload: Any, +) -> str | None: + """Compute a near-miss hint for an ``oneOf`` failure. + + Args: + schema: The compiled validator's root schema (refs already inlined). + schema_path_segments: ``absolute_schema_path`` from the validation + error — points at the ``oneOf`` keyword. + instance_path_segments: ``absolute_path`` from the validation + error — points at the offending value in the payload. + payload: The full request/response payload that failed validation. + + Returns the hint string, or ``None`` if the heuristic can't pick a + clear winner (no detectable discriminator, no clear best variant, + or no obvious wrong-discriminator key). + """ + if not schema_path_segments or schema_path_segments[-1] != "oneOf": + return None + + parent = _navigate(schema, list(schema_path_segments[:-1])) + if not isinstance(parent, dict): + return None + variants_raw = parent.get("oneOf") + if not isinstance(variants_raw, list) or len(variants_raw) < 2: + return None + variants: list[dict[str, Any]] = [v for v in variants_raw if isinstance(v, dict)] + if len(variants) < 2: + return None + + value = _navigate_input(payload, list(instance_path_segments)) + if not isinstance(value, dict): + return None + + discriminator = _detect_discriminator(variants) + if discriminator is None: + return None + + # Skip the hint when the caller already used the right discriminator + # — they merely picked a value that doesn't match any variant. The + # default "value not in allowed enum" message is more accurate there. + if discriminator in value: + return None + + scored = [(_score_variant(v, value, discriminator), idx, v) for idx, v in enumerate(variants)] + # Sort by const_match (strongest), then required_present, then total_present. + # seen_key (index 3 of the score tuple) is metadata, not a ranking signal. + scored.sort(key=lambda s: (-s[0][0], -s[0][1], -s[0][2], s[1])) + + best_score, _, best_variant = scored[0] + if len(scored) > 1: + runner_up = scored[1][0] + # Compare the ranking signals only — ignore seen_key. + if best_score[:3] == runner_up[:3]: + # No clear winner; silent rather than misleading. + return None + + if best_score[:3] == (0, 0, 0): + return None + + expected_const = _variant_const_value(best_variant, discriminator) + if expected_const is None: + return None + + # Prefer the seen_key recorded during scoring (exact key/value match + # against the winning variant's const). Fall back to "extraneous + # top-level key" only when no const-carrying key was found. + seen_key = best_score[3] or _fallback_seen_key(value, discriminator, variants) + if seen_key is None: + return None + + return ( + f"Looks like you may have meant the {expected_const!r} variant. " + f"Use {discriminator!r} instead of {seen_key!r} as the discriminator." + ) diff --git a/src/adcp/validation/schema_errors.py b/src/adcp/validation/schema_errors.py index 2ad200b8c..29e09f3af 100644 --- a/src/adcp/validation/schema_errors.py +++ b/src/adcp/validation/schema_errors.py @@ -6,7 +6,11 @@ from dataclasses import dataclass from typing import Any -from adcp.validation.schema_validator import SchemaValidationError, ValidationIssue +from adcp.validation.schema_validator import ( + SchemaValidationError, + ValidationIssue, + _issue_to_wire, +) @dataclass(frozen=True) @@ -72,15 +76,7 @@ def build_adcp_validation_error_payload( "details": { "tool": tool, "side": side, - "issues": [ - { - "pointer": i.pointer, - "message": i.message, - "keyword": i.keyword, - "schema_path": i.schema_path, - } - for i in issues - ], + "issues": [_issue_to_wire(i) for i in issues], }, } if first is not None and first.pointer: diff --git a/src/adcp/validation/schema_validator.py b/src/adcp/validation/schema_validator.py index 5bec81970..102900b63 100644 --- a/src/adcp/validation/schema_validator.py +++ b/src/adcp/validation/schema_validator.py @@ -21,6 +21,7 @@ from itertools import islice from typing import Any +from adcp.validation.oneof_hints import compute_oneof_hint from adcp.validation.schema_loader import Direction, ResponseVariant, get_validator # Cap the number of issues returned. A hostile peer sending a deeply- @@ -48,12 +49,18 @@ class ValidationIssue: keyword: jsonschema keyword that rejected the payload (``required``, ``type``, ``enum``, etc.). schema_path: Path inside the schema that rejected the payload. + hint: Optional near-miss diagnostic naming the closest matching + ``oneOf`` variant and the wrong discriminator key. Only + populated when the heuristic in :mod:`adcp.validation.oneof_hints` + picks a clear winner; ``None`` otherwise. Additive — clients + that ignore the field behave as before. """ pointer: str message: str keyword: str schema_path: str + hint: str | None = None @dataclass(frozen=True) @@ -101,15 +108,7 @@ def __init__( self.details = { "tool": tool, "side": side, - "issues": [ - { - "pointer": i.pointer, - "message": i.message, - "keyword": i.keyword, - "schema_path": i.schema_path, - } - for i in issues - ], + "issues": [_issue_to_wire(i) for i in issues], } if message is None: first = issues[0] if issues else None @@ -126,6 +125,23 @@ def __init__( _OK_SKIPPED = ValidationOutcome(valid=True, issues=[], variant="skipped") +def _issue_to_wire(issue: ValidationIssue) -> dict[str, Any]: + """Serialize a :class:`ValidationIssue` for the wire envelope. + + ``hint`` is only included when populated — clients ignoring the + field see exactly the pre-hint envelope shape. + """ + payload: dict[str, Any] = { + "pointer": issue.pointer, + "message": issue.message, + "keyword": issue.keyword, + "schema_path": issue.schema_path, + } + if issue.hint is not None: + payload["hint"] = issue.hint + return payload + + def _path_to_pointer(path: Any) -> str: """Convert a jsonschema ``deque(['packages', 0, 'targeting'])`` to ``/packages/0/targeting``. Empty path maps to ``/`` per RFC 6901 @@ -208,8 +224,18 @@ def _missing_required_key(err: Any) -> str | None: return None -def _format_error(err: Any) -> ValidationIssue: - """Turn a ``jsonschema.exceptions.ValidationError`` into a ``ValidationIssue``.""" +def _format_error( + err: Any, + root_schema: Any | None = None, + payload: Any = None, +) -> ValidationIssue: + """Turn a ``jsonschema.exceptions.ValidationError`` into a ``ValidationIssue``. + + When ``root_schema`` and ``payload`` are supplied and the failing + keyword is ``oneOf``, attaches a near-miss ``hint`` naming the closest + matching variant and the wrong discriminator key (see + :mod:`adcp.validation.oneof_hints`). + """ pointer = _path_to_pointer(list(err.absolute_path)) keyword = str(err.validator or "validation") @@ -220,11 +246,21 @@ def _format_error(err: Any) -> ValidationIssue: schema_path = "#/" + "/".join(str(seg) for seg in err.absolute_schema_path) + hint: str | None = None + if keyword == "oneOf" and root_schema is not None: + hint = compute_oneof_hint( + root_schema, + list(err.absolute_schema_path), + list(err.absolute_path), + payload, + ) + return ValidationIssue( pointer=pointer, message=_safe_message(err, keyword), keyword=keyword, schema_path=schema_path, + hint=hint, ) @@ -278,9 +314,10 @@ def validate_request(tool_name: str, payload: Any) -> ValidationOutcome: errors = _iter_errors_bounded(validator, payload) if not errors: return ValidationOutcome(valid=True, issues=[], variant="request") + root_schema = getattr(validator, "schema", None) return ValidationOutcome( valid=False, - issues=[_format_error(e) for e in errors], + issues=[_format_error(e, root_schema, payload) for e in errors], variant="request", ) @@ -332,9 +369,10 @@ def validate_response(tool_name: str, payload: Any) -> ValidationOutcome: errors = _iter_errors_bounded(validator, payload) if not errors: return ValidationOutcome(valid=True, issues=[], variant=used_variant) + root_schema = getattr(validator, "schema", None) return ValidationOutcome( valid=False, - issues=[_format_error(e) for e in errors], + issues=[_format_error(e, root_schema, payload) for e in errors], variant=used_variant, ) diff --git a/tests/test_oneof_hints.py b/tests/test_oneof_hints.py new file mode 100644 index 000000000..9156c5f01 --- /dev/null +++ b/tests/test_oneof_hints.py @@ -0,0 +1,489 @@ +"""Tests for the ``oneOf`` near-miss validator hint. + +When a discriminated-union shape fails validation because the caller used +the wrong key as the discriminator (the v3 reference-seller +``pricing_options`` regression: ``{"type": "cpm"}`` instead of +``{"pricing_model": "cpm"}``), an additive ``hint`` field on the +``VALIDATION_ERROR`` issue names the closest matching variant and the +wrong / expected discriminator keys. +""" + +from __future__ import annotations + +from typing import Any + +from adcp.validation.oneof_hints import compute_oneof_hint +from adcp.validation.schema_errors import build_adcp_validation_error_payload +from adcp.validation.schema_validator import ( + SchemaValidationError, + ValidationIssue, + _issue_to_wire, + validate_response, +) + +# Mirrors the AdCP `pricing-option` oneOf shape — three variants pinning +# `pricing_model` via `const`. Kept inline so the test isn't coupled to +# the bundled schema's exact variant count. +_PRICING_LIKE_SCHEMA: dict[str, Any] = { + "type": "object", + "properties": { + "pricing_options": { + "type": "array", + "items": { + "oneOf": [ + { + "type": "object", + "properties": { + "pricing_option_id": {"type": "string"}, + "pricing_model": {"type": "string", "const": "cpm"}, + "currency": {"type": "string"}, + }, + "required": ["pricing_option_id", "pricing_model", "currency"], + }, + { + "type": "object", + "properties": { + "pricing_option_id": {"type": "string"}, + "pricing_model": {"type": "string", "const": "cpc"}, + "currency": {"type": "string"}, + }, + "required": ["pricing_option_id", "pricing_model", "currency"], + }, + { + "type": "object", + "properties": { + "pricing_option_id": {"type": "string"}, + "pricing_model": {"type": "string", "const": "flat_rate"}, + "currency": {"type": "string"}, + }, + "required": ["pricing_option_id", "pricing_model", "currency"], + }, + ] + }, + } + }, +} + + +class TestComputeOneofHint: + def test_pricing_options_type_vs_pricing_model_regression(self) -> None: + """The v3 ref-seller regression: caller used ``type`` instead of + ``pricing_model`` as the discriminator. Hint must name the ``cpm`` + variant and call out ``pricing_model`` vs ``type``.""" + payload = { + "pricing_options": [{"pricing_option_id": "po1", "type": "cpm", "currency": "USD"}] + } + hint = compute_oneof_hint( + _PRICING_LIKE_SCHEMA, + ["properties", "pricing_options", "items", "oneOf"], + ["pricing_options", 0], + payload, + ) + assert hint is not None + assert "'cpm'" in hint + assert "'pricing_model'" in hint + assert "'type'" in hint + assert "instead of" in hint + + def test_no_hint_when_discriminator_present_but_value_invalid(self) -> None: + """Caller used the right discriminator key but an invalid value — + the standard enum-like message is more accurate than a hint.""" + payload = { + "pricing_options": [ + { + "pricing_option_id": "po1", + "pricing_model": "not_a_real_model", + "currency": "USD", + } + ] + } + hint = compute_oneof_hint( + _PRICING_LIKE_SCHEMA, + ["properties", "pricing_options", "items", "oneOf"], + ["pricing_options", 0], + payload, + ) + assert hint is None + + def test_no_hint_when_no_clear_winner(self) -> None: + """When the caller's payload doesn't carry any variant's + discriminator value and shapes tie, the heuristic stays silent.""" + payload = {"pricing_options": [{"pricing_option_id": "po1", "currency": "USD"}]} + hint = compute_oneof_hint( + _PRICING_LIKE_SCHEMA, + ["properties", "pricing_options", "items", "oneOf"], + ["pricing_options", 0], + payload, + ) + assert hint is None + + def test_no_hint_when_no_discriminator_in_schema(self) -> None: + """Schemas whose ``oneOf`` variants don't pin a ``const`` + discriminator field are out of scope for this heuristic.""" + non_disc_schema: dict[str, Any] = { + "oneOf": [ + {"type": "object", "properties": {"a": {"type": "string"}}, "required": ["a"]}, + {"type": "object", "properties": {"b": {"type": "string"}}, "required": ["b"]}, + ] + } + hint = compute_oneof_hint(non_disc_schema, ["oneOf"], [], {"c": "x"}) + assert hint is None + + def test_no_hint_when_value_is_not_object(self) -> None: + """oneOf failures on scalars (e.g., string-or-array unions) don't + carry a discriminator — hint stays silent.""" + scalar_oneof = {"oneOf": [{"type": "string"}, {"type": "integer"}]} + hint = compute_oneof_hint(scalar_oneof, ["oneOf"], [], True) + assert hint is None + + def test_picks_correct_variant_among_many(self) -> None: + """With nine pricing-model variants, the hint names the one whose + ``const`` matches the value the caller supplied.""" + payload = { + "pricing_options": [ + {"pricing_option_id": "po1", "type": "flat_rate", "currency": "USD"} + ] + } + hint = compute_oneof_hint( + _PRICING_LIKE_SCHEMA, + ["properties", "pricing_options", "items", "oneOf"], + ["pricing_options", 0], + payload, + ) + assert hint is not None + assert "'flat_rate'" in hint + + def test_seen_key_is_the_one_carrying_winning_variant_const(self) -> None: + """When multiple top-level keys carry values matching different + variants' consts, the hint's ``seen_key`` must be the one tied + to the *winning* variant — not whichever key the old fallback + scan happened to find first. + + Payload: ``alias: "cpc"``, ``flavor: "cpm"``, plus shape-fields + only present in the ``cpc`` variant. The cpc variant wins on + required_present, so the hint must say "use ``pricing_model`` + instead of ``alias``" (the key carrying ``"cpc"``), not + ``flavor``. + """ + schema: dict[str, Any] = { + "oneOf": [ + { + "type": "object", + "properties": { + "pricing_model": {"type": "string", "const": "cpm"}, + "cpm_only": {"type": "string"}, + }, + "required": ["pricing_model", "cpm_only"], + }, + { + "type": "object", + "properties": { + "pricing_model": {"type": "string", "const": "cpc"}, + "cpc_only": {"type": "string"}, + }, + "required": ["pricing_model", "cpc_only"], + }, + ] + } + # cpc-shape (cpc_only present) — but BOTH "cpm" and "cpc" appear + # as values under unrelated keys. The winner is cpc on shape; the + # seen_key reported in the hint must be the one carrying "cpc". + payload = {"alias": "cpc", "flavor": "cpm", "cpc_only": "x"} + hint = compute_oneof_hint(schema, ["oneOf"], [], payload) + assert hint is not None + assert "'cpc'" in hint + assert "'pricing_model'" in hint + assert "'alias'" in hint + assert "'flavor'" not in hint + + def test_no_hint_on_tie_without_const_match(self) -> None: + """Two variants score identically on shape (no const_match for + either) — the heuristic must not promote one over the other.""" + schema: dict[str, Any] = { + "oneOf": [ + { + "type": "object", + "properties": { + "kind": {"type": "string", "const": "alpha"}, + "shared": {"type": "string"}, + }, + "required": ["kind", "shared"], + }, + { + "type": "object", + "properties": { + "kind": {"type": "string", "const": "beta"}, + "shared": {"type": "string"}, + }, + "required": ["kind", "shared"], + }, + ] + } + # Payload supplies `shared` (in both variants' required + properties) + # but no `kind` and no value matching either const. Both variants + # score (0, 1, 1, None) — a tie. + payload = {"shared": "x"} + hint = compute_oneof_hint(schema, ["oneOf"], [], payload) + assert hint is None + + def test_runner_up_with_extra_unmatched_property_does_not_win(self) -> None: + """Variant A is a true near-miss (caller supplied A's const value + via the wrong key). Variant B has more declared properties but + the payload doesn't carry them. const_match must dominate so A + wins, even though B has higher ``total`` declared properties.""" + schema: dict[str, Any] = { + "oneOf": [ + { + "type": "object", + "properties": { + "kind": {"type": "string", "const": "alpha"}, + "value": {"type": "string"}, + }, + "required": ["kind", "value"], + }, + { + "type": "object", + "properties": { + "kind": {"type": "string", "const": "beta"}, + "value": {"type": "string"}, + "extra1": {"type": "string"}, + "extra2": {"type": "string"}, + "extra3": {"type": "string"}, + }, + "required": ["kind", "value"], + }, + ] + } + payload = {"category": "alpha", "value": "v"} + hint = compute_oneof_hint(schema, ["oneOf"], [], payload) + assert hint is not None + assert "'alpha'" in hint + assert "'kind'" in hint + assert "'category'" in hint + + def test_nested_oneof_failures_each_get_their_own_hint(self) -> None: + """``oneOf`` inside ``oneOf``: outer variants choose by ``mode``, + and one outer variant has a nested ``oneOf`` keyed by + ``pricing_model``. Both layers must produce independent hints + when each is queried with its own schema/instance path.""" + nested_schema: dict[str, Any] = { + "type": "object", + "properties": { + "config": { + "oneOf": [ + { + "type": "object", + "properties": { + "mode": {"type": "string", "const": "auto"}, + "pricing": { + "oneOf": [ + { + "type": "object", + "properties": { + "pricing_model": { + "type": "string", + "const": "cpm", + }, + "currency": {"type": "string"}, + }, + "required": ["pricing_model", "currency"], + }, + { + "type": "object", + "properties": { + "pricing_model": { + "type": "string", + "const": "cpc", + }, + "currency": {"type": "string"}, + }, + "required": ["pricing_model", "currency"], + }, + ] + }, + }, + "required": ["mode", "pricing"], + }, + { + "type": "object", + "properties": { + "mode": {"type": "string", "const": "manual"}, + "knob": {"type": "string"}, + }, + "required": ["mode", "knob"], + }, + ] + } + }, + } + payload = { + "config": { + "type": "auto", + "pricing": {"type": "cpm", "currency": "USD"}, + } + } + # Outer hint: caller used `type` instead of `mode` to pick `auto`. + outer_hint = compute_oneof_hint( + nested_schema, + ["properties", "config", "oneOf"], + ["config"], + payload, + ) + assert outer_hint is not None + assert "'auto'" in outer_hint + assert "'mode'" in outer_hint + assert "'type'" in outer_hint + + # Inner hint: caller used `type` instead of `pricing_model` to pick `cpm`. + inner_hint = compute_oneof_hint( + nested_schema, + ["properties", "config", "oneOf", 0, "properties", "pricing", "oneOf"], + ["config", "pricing"], + payload, + ) + assert inner_hint is not None + assert "'cpm'" in inner_hint + assert "'pricing_model'" in inner_hint + assert "'type'" in inner_hint + + def test_two_independent_oneof_failures_each_get_their_own_hint(self) -> None: + """Two array entries each fail their own ``oneOf`` for different + reasons. Each instance path must be hinted independently.""" + payload = { + "pricing_options": [ + {"pricing_option_id": "po1", "type": "cpm", "currency": "USD"}, + {"pricing_option_id": "po2", "type": "cpc", "currency": "USD"}, + ] + } + hint_0 = compute_oneof_hint( + _PRICING_LIKE_SCHEMA, + ["properties", "pricing_options", "items", "oneOf"], + ["pricing_options", 0], + payload, + ) + hint_1 = compute_oneof_hint( + _PRICING_LIKE_SCHEMA, + ["properties", "pricing_options", "items", "oneOf"], + ["pricing_options", 1], + payload, + ) + assert hint_0 is not None + assert "'cpm'" in hint_0 + assert hint_1 is not None + assert "'cpc'" in hint_1 + # Hints must not cross-contaminate. + assert "'cpc'" not in hint_0 + assert "'cpm'" not in hint_1 + + +class TestValidationIntegration: + def test_clean_validation_passes_silently(self) -> None: + """A valid payload yields no issues at all — no hint plumbing + side-effect on the success path. The bundled ``get_products`` + response schema must be present (we assert ``variant != skipped`` + so this stays a real signal rather than vacuous).""" + payload: dict[str, Any] = {"products": []} + outcome = validate_response("get_products", payload) + assert ( + outcome.variant != "skipped" + ), "expected the bundled get_products schema to be loaded; got skipped" + assert outcome.issues == [] + + def test_pricing_options_regression_against_real_schema(self) -> None: + """End-to-end against the bundled ``get_products`` schema. The + wrong-discriminator failure on ``pricing_options[0]`` carries a + hint naming ``cpm`` and ``pricing_model``.""" + payload: dict[str, Any] = { + "products": [ + { + "product_id": "p1", + "name": "P", + "description": "d", + "format_ids": [{"format_id": "display_300x250"}], + "delivery_type": "guaranteed", + "pricing_options": [ + {"pricing_option_id": "po1", "type": "cpm", "currency": "USD"} + ], + "reporting_capabilities": {"available_metrics": []}, + "publisher_properties": {"property_list_id": "pl1"}, + } + ] + } + outcome = validate_response("get_products", payload) + oneof_issues = [i for i in outcome.issues if i.keyword == "oneOf"] + assert oneof_issues, ( + "expected at least one oneOf issue against the bundled schema; " + f"got keywords={[i.keyword for i in outcome.issues]}" + ) + pricing_issue = next((i for i in oneof_issues if "pricing_options" in i.pointer), None) + assert pricing_issue is not None + assert pricing_issue.hint is not None + assert "'cpm'" in pricing_issue.hint + assert "'pricing_model'" in pricing_issue.hint + assert "'type'" in pricing_issue.hint + + +class TestWireEnvelope: + def test_hint_is_optional_in_wire_payload(self) -> None: + """Issues without hints serialize without the field — clients + that ignore the new key see exactly the pre-hint envelope.""" + issue = ValidationIssue( + pointer="/foo", + message="expected type 'string'", + keyword="type", + schema_path="#/properties/foo/type", + ) + wire = _issue_to_wire(issue) + assert "hint" not in wire + assert wire == { + "pointer": "/foo", + "message": "expected type 'string'", + "keyword": "type", + "schema_path": "#/properties/foo/type", + } + + def test_hint_included_when_present(self) -> None: + issue = ValidationIssue( + pointer="/pricing_options/0", + message="oneOf composition failed", + keyword="oneOf", + schema_path="#/properties/pricing_options/items/oneOf", + hint="Looks like you may have meant the 'cpm' variant. " + "Use 'pricing_model' instead of 'type' as the discriminator.", + ) + wire = _issue_to_wire(issue) + assert wire.get("hint") == issue.hint + + def test_build_adcp_validation_error_payload_carries_hint(self) -> None: + """The wire envelope produced by ``build_adcp_validation_error_payload`` + — the canonical projection point for ``VALIDATION_ERROR`` — surfaces + ``hint`` per-issue when set.""" + issues = [ + ValidationIssue( + pointer="/pricing_options/0", + message="oneOf composition failed", + keyword="oneOf", + schema_path="#/properties/pricing_options/items/oneOf", + hint="Looks like you may have meant the 'cpm' variant. " + "Use 'pricing_model' instead of 'type' as the discriminator.", + ), + ] + payload = build_adcp_validation_error_payload("get_products", "response", issues) + assert payload["code"] == "VALIDATION_ERROR" + wire_issues = payload["details"]["issues"] + assert wire_issues[0]["hint"].startswith("Looks like") + + def test_schema_validation_error_details_carries_hint(self) -> None: + issues = [ + ValidationIssue( + pointer="/pricing_options/0", + message="oneOf composition failed", + keyword="oneOf", + schema_path="#/properties/pricing_options/items/oneOf", + hint="Looks like you may have meant the 'cpm' variant. " + "Use 'pricing_model' instead of 'type' as the discriminator.", + ), + ] + err = SchemaValidationError("get_products", "response", issues) + wire_issues = err.details["issues"] + assert wire_issues[0]["hint"].startswith("Looks like")