resolve ambiguity regarding annotation inclusion in failed validation results

[karenetheridge]

The spec says (https://json-schema.org/draft/2019-09/json-schema-core.html#rfc.section.7.7.1.2):

Schema objects that produce a false assertion result MUST NOT produce any annotation results, whether from their own keywords or from keywords in subschemas.

However, this is contradicted by other parts of the spec
(https://json-schema.org/draft/2019-09/json-schema-core.html#rfc.section.10.4.4):

This includes sub-schema validation results that would otherwise be removed (e.g. annotations for failed validations, successful validations inside a not keyword, etc.).

So, it would appear there are cases where annotations should be included in a result even if the
overall validation result is false. (e.g. if one branch of an allOf was successful and produced annotations, but another branch failed, this means that the subschema at allOf fails, so it should discard all annotations collected below that point and simply return its valid: "failed" and errors: [..] results back up to the next-higher node. But in the end, annotations are still not returned in the final result.

This (section 10.4.4) also implies that an implementation cannot simply discard
all collected annotations at a schema position where validation fails: it must continue to collect
them in case the "verbose" output method is requested.

---

Now, another usecase:

I have a desire to include annotations in failed validation result, so as to provide more
information as to the nature of the failure (or perhaps offer extra guidance as to how to make
validation succeed).·

(cross-ref: https://github.com/karenetheridge/JSON-Schema-Draft201909/issues/22)

I can achieve this with a simple annotation-only keyword (e.g. "description", or a new one that acts
like "description" by returning data at the position it is evaluated), except for this item of the
spec that says that annotations are discarded on failed results.

Is there a way through this? At the very least, it seems as if there is some contradictory language
in the spec that could be clarified.

[ssilverman]

In my validator, I separate the concept of "collected annotations" from "error collection". I think the "annotations" referred to by section 10.4.4 are in the second, "error collection" set, where there are both "error" and "annotation" annotations representing "false" and "true" results, respectively. At least that's how I'm reading this part.

I agree the language could use work.

[handrews]

My intention was that failed validation always drops annotations, which means you can abandon processing once a schema object fails validation, and ignore its subschemas.

I did not write section 10, so you'll have to ask @gregsdennis about it (if I was involved in that point, I have since forgotten about it).

It's not feasible to fully specify annotations that might-have-been because you get into very complex questions about conditional schema application and exactly how much needs to be faked in order to find might-have-been annotations. That's one reason I didn't require it, in addition to the performance issues.

[handrews]

Section 7.7.2.1 paragraph one is correct as written, any change to the behavior it requires it would require substantial discussion beyond just harmonizing wording.

[karenetheridge]

It's not feasible to fully specify annotations that might-have-been because...

completely agreed! I think this means that the "verbose" output format might need to be reworked, or moved into the "not-required-but-some-implementations-may-support-it" category.

edit: #679 (comment) shows that at one point the core team did grasp what was going on here and did not see any contradictions in it :) Perhaps there is no issue here that needs resolving. I hope to determine that for myself soon when I get to implementing the "detailed" and "verbose" result formats.

[Relequestual]

Looking at 10.4.4, I don't see any examples of annotations being passed through to output.
It's not unreasonable to expect annotations of schemas that were applied but failed to be collected for errors.
I'd think of this as "error info collection" as opposed to "annotation collection", which is clear cannot happen for failed validation (and makes a lot of sense).

In terms of what @handrews said...

It's not feasible to fully specify annotations that might-have-been because you get into very complex questions about conditional schema application

Yeah, I believe the verbose error reporting can ONLY produce error results for schema parts which were actually APPLIED to the instance.

@ssilverman Can I see some verbose output examples from you when you have failing subschemas with annotations please? Also for allOf.

I haven't poked at @jdesrosiers hyperjump, but the web based version does fast failing, so you only get the first failure.

[ssilverman]

@Relequestual I'm not completely clear what the request is, so I created the following example to start. Let's start here and modify until it's what you'd like to see.

Schema:

{
  "$id": "http://github.com/json-schema-org/json-schema-spec/issues/939",
  "allOf": [
    {
      "not": true,
      "title": "The Title"
    }
  ]
}

Instance:

{

}

Basic errors:

{
    "valid": false,
    "errors": [
        {
            "keywordLocation": "",
            "absoluteKeywordLocation": "https://github.com/json-schema-org/json-schema-spec/issues/939",
            "instanceLocation": "",
            "error": "schema didn't validate"
        },
        {
            "keywordLocation": "/allOf",
            "absoluteKeywordLocation": "https://github.com/json-schema-org/json-schema-spec/issues/939#/allOf",
            "instanceLocation": "",
            "error": "invalid subschemas: 0"
        },
        {
            "keywordLocation": "/allOf/0/not",
            "absoluteKeywordLocation": "https://github.com/json-schema-org/json-schema-spec/issues/939#/allOf/0/not",
            "instanceLocation": "",
            "error": "\"not\" didn't validate"
        }
    ]
}

Annotations when not collecting annotations for failed schemas:

{
    "annotations": []
}

Annotations when collecting annotations for failed schemas (note the "valid=false"):

{
    "annotations": [
        {
            "instanceLocation": "",
            "keywordLocation": "/allOf/0/title",
            "absoluteKeywordLocation": "https://github.com/json-schema-org/json-schema-spec/issues/939#/allOf/0/title",
            "annotation": {
                "name": "title",
                "valid": false,
                "value": "The Title"
            }
        }
    ]
}

[jdesrosiers]

Hyperjump JSV doesn't collect annotations, so I'm not going to be any help.

[Relequestual]

Thanks @ssilverman
I think what I was implying was, I want to see what you expect to happen when there are multiple branches of the allOf.

Regardless, I've considered...

@karenetheridge Do you think there's any merit in collecting non-annotations (I'm calling them non-annotations because they are never attached to an instance location) for failed validation processes?

Given the failures provide a schema location, anyone wanting such non-annotations could look up in the schema to see what they are (based on the schema location attribute).

If you agree there's no point, then I'm going to suggest we remove the part where it talks about "annotations for failed validations", unless @gregsdennis pipes up to explain what this meant...

[gregsdennis]

That verbiage only pertains to the Verbose output format. The intent was that, if the client is requesting verbose, they probably want as much info as possible, so we give them everything, even annotations that would otherwise be removed.

Maybe this can be an implementation prerogative. If so, we should update the text as such.

[gregsdennis]

[@handrews] very complex questions about conditional schema application and exactly how much needs to be faked in order to find might-have-been annotations.

From an implementor's point of view, collecting annotations on a node you're already evaluating is trivial, unless you have shortcutting logic to quickly fail. But if they're requesting verbose output, performance is an unlikely concern as the output content take priority.

[Relequestual]

OK. I think we're getting there.

@ssilverman I think in your example, the validproperty was supposed to be on the node object as opposed to the annotation object? (up a level).

---

Addressing...

[@handrews] It's not feasible to fully specify annotations that might-have-been because you get into very complex questions about conditional schema application and exactly how much needs to be faked in order to find might-have-been annotations.

I wonder @handrews if you're thinking about subschemas which are not actually applied as part of processing? Obviously that would be insane. It's impractical to suggest that the verbose ouput somehow magically works out how to apply all subschemas (which might not even be possible with the instance provided).

I think, as @gregsdennis says here, if the user is wanting verbose output, the annotation collection would be enhanced, and collected regardless of the validation result, but STILL only based on applicability, and without short circiting validation (because verbose).

@gregsdennis @handrews Are we on the same page here?

---

I do have a follow up though...

It's actually a bit of a sidetrack... but I'd like to keep it here anyway

Given a schema and instance such as...

{
  "$schema": "https://json-schema.org/draft/2019-09/schema",
  "not": {
    "allOf":[
      {
        "title": "Something",
        "required": ["a", "b"]
        },{
          "title": "Something Else",
          "required": ["c", "d"]
      }
    ]
  }
}

and instance...

{
  "c": 1,
  "d": 1
}

How does the verbose result look?

The subschema allOf[1] is valid, but overall the schema applied to the instance is not valid.