fix: improve schema validation error relevance

[dk3yyyy]

Fix for #3752

Problem

I found that Altair's error-selection heuristic could choose the wrong branch of a Vega-Lite anyOf union. In positional channels this could report value itself as unexpected instead of reporting the options that are invalid for the value branch.

Reproduce

import altair as alt

channel = alt.value(1, bin=True, aggregate="sum")
alt.Chart().mark_point().encode(y=channel).to_dict()

Current output:

SchemaValidationError: `YValue` has no parameter named 'value'

With this PR:

SchemaValidationError: `YValue` has no parameters named 'aggregate', 'bin'

The same underlying heuristic could also hide an array-size error. For example:

alt.selection_interval(
    resolve="global",
    value={"x": ["Europe"], "y": [10, 20]},
)

With this PR, the error points to the actual constraint:

'['Europe']' is an invalid value for `x`. Valid values are of type `Sequence` with at least 2 items.

Proposed solution

I changed union-error selection so a branch is preferred only when root-level required properties and their discriminator values reliably identify it. I resolve local Vega-Lite schema references against the effective validation root before making that decision, including Altair's internal urn:vega-lite-schema form on current jsonschema. If multiple branches can accept the same properties, or a reference cannot be resolved locally, I keep the competing errors instead of guessing.

Local JSON Pointer resolution handles URI-fragment percent decoding, strict ~0/~1 escapes, chained references, and cycle detection. Malformed, external, cyclic, or otherwise unresolved references remain unknown rather than becoming branch-selection evidence.

I also compare JSON paths by path segments, prioritize minItems and maxItems constraints over nested item errors, and preserve all unexpected parameters while respecting patternProperties.

My main uncertainty was how aggressively to choose a branch when the schema is ambiguous. I chose the conservative option: only narrow the error tree with positive evidence and otherwise retain the available branch errors. This is intended to generalize across Vega-Lite's referenced unions rather than special-case YValue.

Tests

I replaced the synthetic union-schema matrix with regressions that exercise Altair's shipped Vega-Lite schema:

• the YValue cases reported in SchemaValidationError relevance heuristic is flawed #3752, including multiple invalid parameters;
• the interval-selection minItems case reported in SchemaValidationError relevance heuristic is flawed #3752;
• Vega-Lite's referenced Binding union, including its const, enum, and general input branches;
• separate schema/rootschema validation and strict local JSON Pointer handling;
• multiple unexpected parameters and patternProperties;
• sibling JSON paths and minItems/maxItems precedence.

Local validation:

• uv and pytest are available in the review environment;
• uv run task test passes: 1,943 main tests and 2 serial tests, plus Ruff, formatting, MyPy, and Ty;
• focused resolver coverage passes with the minimum supported jsonschema==3.0.2;
• the generated altair/utils/schemapi.py is synchronized with tools/schemapi/schemapi.py and regeneration is idempotent.

Closes #3752

[mattijn]

Thanks for the PR! Could you instruct your agent to first fix your review environment so the uv toolchain and pytest are available locally?

Also can you update the PR description? Currently it feels more like feedback of your code generator to you than an invitation for a review. Maybe something like this as template?:

---

<!-- Title: fix: <short imperative> -->

Fix for #<issue>

# Problem
One or two sentences: what breaks, and why the current code does it.

# Reproduce
```python
# minimal runnable Altair snippet

Current output:
<actual error message>

With this PR:
<new error message>

Proposed solution
What changed and why, in prose or a short list. 

Please write in first person, and explain briefly your trail of thought (make your uncertainty visible).

---

Also a reminder to try we generalize to the varies reported cases and not have a hotfix for a single usecase.

I'm also a bit confused on the Draft7Validator tests added, these seem more appropriate for jsonschema itself? Please use the vegalite jsonschema itself where necessary.

Thanks for your work!🦾🙌

[dk3yyyy]

Thanks, this was helpful. I fixed my local setup first and can now run the full uv test, lint, formatting, and type-checking workflow.

I also rewrote the PR description around the actual problem, included runnable examples with the before/after errors, and explained why I kept the branch selection conservative.

You were right about the synthetic tests too. I replaced the broad Draft7Validator matrix with regressions using Altair’s Vega-Lite schema, including the reported YValue and interval-selection cases and the referenced Binding branches. I’ve pushed the updated implementation and tests.

[mattijn]

Thanks for the ongoing work. I'm still on mobile, so I can't check the details but looking to the code diff I don't think we need to add another schema-resolution layer for this, my gut feeling says we can use existing functions that are available for this, including usage of referencing package.

Please expect some scrutiny, since we have aimed to simplify these routines previously, see for reference #2771 and other mentioned issues/PR in there.

Some crumbs from my LLM assistant:

Instead of resolving pointers would it be possible to thread the registry/resolver that _get_errors_from_spec already builds into _get_leaves_of_error_tree? For jsonschema ≥4.18 that’s registry.resolver().lookup(ref).contents, on older versions resolver.resolve(ref)[1]. Hopefully with this we can reduce the added code like _decode_json_pointer_*, _resolve_json_pointer and _resolve_local_schema_reference all go, along with their tests, also we add the urn:vega-lite-schema prefix by ourselves, so it feels off to strip it again too

Thanks again for the ongoing effort. We will get there🤗

[dk3yyyy]

Thanks for the guidance — I've reworked the reference resolution to reuse the registry, as suggested.

• _get_errors_from_spec now returns the registry it builds, and _get_leaves_of_error_tree receives it and threads it down; _union_branch_schemas resolves branch refs through registry.resolver().
• The pointer helpers (_decode_json_pointer_*, _resolve_json_pointer, _resolve_local_schema_reference) and their unit tests are gone.
• _resolve_references now uses registry.resolver().lookup(...), falling back to jsonschema.RefResolver on older jsonschema versions. Since we build the registry with the urn:vega-lite-schema prefix ourselves, refs are prefixed when missing rather than stripped.
• Compatibility with the minimum supported jsonschema==3.0.2 is covered by a focused test.

On CI: the current failures are pre-existing network flakes in tests/test_datasets.py (external dataset downloads returning URLError: Connection reset by peer, plus a polars BufferError under xdist) — unrelated to these changes. Locally, tests/utils/test_schemapi.py passes in full, and make lint/make typecheck/make test are green.

Happy to iterate further if the branch-selection heuristics need tightening.

[joelostblom]

Thanks for working on this! It will be great to have improved error heuristics. I found one smaller issue that I think would be good to fix in addition to any additional comments from mattijn.

[joelostblom]

I think that this branch-selection heuristic can hide the actual enum error. A required property is not necessarily an unambiguous discriminator. In particular, a generic fallback branch may accept the property's type while a more specialized branch restricts it with enum or const. If another property identifies the specialized branch, selecting the generic branch produces a misleading additionalProperties error.

Take this example:

import altair as alt

slider = alt.param(
    name="threshold",
    value=5,
    bind={
        "input": "ragne",  # Intentional typo: should be "range"
        "min": 0,
        "max": 10,
    },
)

chart = (
    alt.Chart({"values": [{"x": 1}]})
    .mark_point()
    .encode(x="x:Q")
    .add_params(slider)
)

On this PR, the result is:

`VariableParameter` has no parameters named 'max', 'min'

That is misleading because min and max are valid properties of a range binding. They also provide evidence that the intended branch is BindRange. The actual error is the misspelled value "ragne".

On main, the result correctly identifies the invalid input value (although the error message could still be improved):

'ragne' is an invalid value for `input`. Valid values are one of ['radio', 'select'].

'checkbox' was expected
'range' was expected

The same behavior can be demonstrated with a reduced schema:

from altair.utils.schemapi import validate_jsonschema

schema = {
    "anyOf": [
        {
            "type": "object",
            "required": ["kind"],
            "properties": {
                "kind": {"enum": ["allowed"]},
                "payload": {"type": "integer"},
            },
            "additionalProperties": False,
        },
        {
            "type": "object",
            "required": ["kind"],
            "properties": {
                "kind": {"type": "string"},
            },
            "additionalProperties": False,
        },
    ]
}

error = validate_jsonschema(
    {"kind": "typo", "payload": 1},
    schema,
    raise_error=False,
)

assert error is not None
print(error.message)

This PR reports:

Additional properties are not allowed ('payload' was unexpected)

main reports the more actionable error:

'typo' is not one of ['allowed']

Could we adjust the heuristic so that an enum/const mismatch does not automatically make a specialized branch irrelevant when other properties identify that branch? Please also add the real binding example as a regression test and mirror the fix in tools/schemapi/schemapi.py.

generated with codex

[dk3yyyy]

Thanks for the review. I've addressed all three points: the heuristic now keeps a specialized branch when its only mismatch is a const/enum on a required property (so the BindRange / 'allowed' enum errors surface instead of the misleading additionalProperties / unknown-parameters ones), the real binding example is added as a regression test, and the fix is mirrored in tools/schemapi/schemapi.py. Both examples now produce the expected messages, and the full suite is green. Re-requesting review — happy to iterate further.

[joelostblom]

Thanks @dk3yyyy, it seems like the fix migh overcorrect by no longer treating const/enum mismatches as branch-exclusion evidence in _branch_excludes_required_value (altair/utils/schemapi.py:556). This can retain a branch that is clearly contradicted by a valid discriminator.

For example, with branches distinguishing kind="integer" from kind="string":

from altair.utils.schemapi import SchemaBase, SchemaValidationError

schema = {
    "anyOf": [
        {
            "type": "object",
            "required": ["kind"],
            "properties": {
                "kind": {"const": "integer"},
                "payload": {"type": "integer"},
            },
            "additionalProperties": False,
        },
        {
            "type": "object",
            "required": ["kind"],
            "properties": {
                "kind": {"const": "string"},
                "payload": {"type": "string"},
            },
            "additionalProperties": False,
        },
    ]
}


class Example(SchemaBase):
    _schema = schema


try:
    Example(
        kind="integer",
        payload="bad",  # Invalid: the "integer" branch requires an integer.
    ).to_dict()
except SchemaValidationError as error:
    print(error)

At commit c886518, this prints:

Multiple errors were found.

Error 1: 'bad' is an invalid value for `payload`. Valid values are of type `int`.

Error 2: 'integer' is an invalid value for `kind`.

    'string' was expected

The second error should not appear. kind="integer" positively identifies the first branch, so the only relevant error is:
'bad' is an invalid value for payload. Valid values are of type int.
The parent commit reports only that relevant error.

---

Also note that The new regression test fails mypy at tests/utils/test_schemapi.py:1046:

Argument "bind" to "param" has incompatible type "dict[str, object]";
expected "Binding | UndefinedType"

[dk3yyyy]

Thanks — I've refined the heuristic so a const/enum mismatch is only treated as ambiguous when the value satisfies no branch's discriminator; when it matches another branch's const/enum, the contradicted branch is still excluded. The overcorrection example now reports only the payload error, the original enum-typo and binding cases are unchanged, and the mypy issue in the test is fixed with a scoped # type: ignore[arg-type] (mirroring the existing test pattern). Full suite, ruff, and mypy are green.

[mattijn]

While reviewing last evening, I think I found something that may account for half of this issue. In our _reduce function (inside _invert_group_channels) we currently have this:

altair/altair/utils/core.py

Lines 957 to 973 in 1db4156

	def _reduce(it: Iterator[tuple[type[Any], str]]) -> Any:
	"""
	Returns a 1-2 item dict, per channel.
	Never includes `datum`, as it is never utilized in `wrap_in_channel`.
	"""
	item: dict[Any, type[SchemaBase]] = {}
	for tp, _ in it:
	name = tp.__name__
	if name.endswith("Datum"):
	continue
	elif name.endswith("Value"):
	sub_key = "value"
	else:
	sub_key = "field"
	item[sub_key] = tp
	return item

with a docstring saying: "Never includes datum, as it is never utilised in wrap_in_channel." If we actually accept datum, it gets wrapped in a *Datum channel class, just as value is wrapped in a *Value class (which we then subsequently can use in the error validation).

The other thing, I think this PR's current approach of reconstructing the relevant branch after the fact is a bit risky, since it has to guess. Altair already has the object's own type available at that point, so we can re-validate against that directly instead.

My branch with applied changes is here: main...fix/3752-validate-against-live-object-schema (or code.diff)

Because I can reuse the existing object the introduced new LOC is much lower.

The only thing I haven't get to is this example from original post (and is working correctly in this PR), is this example

alt.selection_interval(
    resolve="global",
    value={"x": ["Europe"], "y": [10, 20]},
)

Technically speaking, while still super relevant to be fixed, I think it is a bit of a different issue than #3752, since its because x has two true complaints at once (the list is too short, and its one item is the wrong type), and the current path-picking rule shows the more specific one (about the item) instead of the more useful one (about the length).

Its also defined in the doctsring of this _subset_to_most_specific_json_paths function:

altair/altair/utils/schemapi.py

Lines 330 to 340 in fde4b47

	def _subset_to_most_specific_json_paths(
	errors_by_json_path: GroupedValidationErrors,
	) -> GroupedValidationErrors:
	"""
	Removes key (json path), value (errors) pairs where the json path is fully contained in another json path.
	For example if `errors_by_json_path` has two keys, `$.encoding.X` and `$.encoding.X.tooltip`,
	then the first one will be removed and only the second one is returned.
	This is done under the assumption that more specific json paths give more helpful error messages to the user.
	"""

This is done under the assumption that more specific json paths give more helpful error messages to the user.

All errors can be retrieved from this code:

from altair.utils.schemapi import (
    _get_errors_from_spec,
    _get_leaves_of_error_tree,
    _group_errors_by_json_path,
)
from altair.vegalite.v6.schema.core import SelectionParameter

spec = {
    "name": "__TEMP__",
    "select": {"type": "interval", "resolve": "global"},
    "value": {"x": ["Europe"], "y": [10, 20]},
}

errors = _get_errors_from_spec(spec, SelectionParameter._schema, rootschema=SelectionParameter._rootschema)
leaves = _get_leaves_of_error_tree(errors)
grouped = _group_errors_by_json_path(leaves)

grouped

{
 '$.value': [
  <ValidationError: "{'x': ['Europe'], 'y': [10, 20]} is not of type 'number', 'string', 'boolean', 'null'">,
  <ValidationError: "Additional properties are not allowed ('x', 'y' were unexpected)">,
  <ValidationError: "{'x': ['Europe'], 'y': [10, 20]} is not of type 'array'">
],
 '$.value.x[0]': [
  <ValidationError: "'Europe' is not of type 'boolean'">,
  <ValidationError: "'Europe' is not of type 'number'">,
  <ValidationError: "'Europe' is not of type 'object'">
],
 '$.value.x': [
  <ValidationError: "['Europe'] is too short">,
  <ValidationError: "['Europe'] is too short">,
  <ValidationError: "['Europe'] is too short">,
  <ValidationError: "['Europe'] is too short">
]}

I'm wondering what you think @dk3yyyy here

[dk3yyyy]

Thanks for the detailed look — you were right that guessing branches is the fragile part. I've switched to your approach: SchemaValidationError now re-validates against the live object's own schema first, so it uses the type Altair already knows instead of reconstructing it. I also applied your _invert_group_channels observation, so datum= now gets wrapped in its *Datum class the same way value= is — that covers the other half of #3752.

The only thing I kept from the previous approach is a fallback for cases where there is no live object at the error path, like a raw dict passed to bind=. With alt.param(value=1, bind={"input": "range", "min": "bad"}) there's no Binding object to re-validate against, so it needs branch selection to get to 'bad' is an invalid value for 'min' instead of an additionalProperties message. It's a much smaller surface than before, but I kept it for exactly that case, and the new tests cover both paths.

On the selection_interval example — agreed, that's a separate issue: x has two genuine complaints and the path-picking rule intentionally shows the more specific one, so I've left it out of scope here.

CI is green on the current head. Happy to adjust anything else if you have suggestions.

[dk3yyyy]

@joelostblom just a heads-up that both points from the last round are addressed on the current head:

• the overcorrection on {"kind": "integer", "payload": "bad"} no longer leaks a spurious 'integer' is an invalid value for 'kind' — a value that satisfies another branch's discriminator now counts as evidence against the current branch, and the message only reports the payload error
• the mypy failure on the bind= dict is fixed (typed with # type: ignore[arg-type], mirroring the existing pattern)

The branch is also rebased on the latest main (including the referencing/registry refactor) and CI is green. Would appreciate another look whenever you have a chance.

[mattijn]

Heads up: not forgotten this PR and so now and then I'm reading this thread and the code again. But my feeling is that it has become a bit messy, since we are piling fixes.

Maybe good idea if we can get a overview of the different Altair examples discussed and discovered in this thread that we feel can improve in its error message justification.

So it's easier to create a few follow up issues if these don't touch the same heuristic as the original linked issue aimed to be fixed.