quarto-dev / quarto-dev/quarto-cli

yaml validation error heuristics are not tuned for schema changes

Open
#2,190 1 comment 0 reactions 1 assignee View on GitHub

@cscheid is already working on this.

Since Aug 29, 2022.

bug yaml-validation
Dominant language
JavaScript
Stars
6k
Forks
458
Avg merge
1d 9h
Merged PRs (30d)
41

Description

We had a bad validation error come from https://github.com/rstudio-conf-2022/teach-ds-course-website/blob/ef90e6111411bc9d2b6feb4b2e5b49dc6c24e55b/_quarto.yml under our v1.1 schema closure.

image

Context

We recently closed a number of schemas, which introduces a large-scale change in the validation behavior. In this case specifically, text was mistakenly not a part of the navigation-item schema, which means that under the new schema, yaml validation fails (correctly, although using a wrong schema) :

  • every array entry in website:sidebar:contents fails
  • same for every array entry in website:sidebar:tools
  • as a result, the entries website:sidebar and website fail
Heuristics gone bad

We have heuristics that try to find "small" changes to fix the problem. Unfortunately, in this case, the small change suggested is to replace the website:sidebar object with true or false. It does this because we have a heuristic that says "a lot of internal errors to one object probably mean a single larger error in a containing object".

That heuristic is good in the case of interactive editing, where we expect errors to show up one at a time. But in this case, the schema change caused all errors to show up at once.

Possible solutions

(All of these are a significant amount of work, but I'm writing them here so I don't need to keep them in my head for when I come back to this problem later.)

  • We have a mechanism for identifying the "type" of error (schemaPath). Many errors with the same schemaPath should be an indication that it's actually the same error, and we should consider reporting that one instead.
  • The bad heuristic comes from looking at the entire YAML at once. In this case, if we forced ourselves to look at "the first error to happen", then the error would be better, as long as we had a good definition of "first". Currently, we scan the full YAML object for errors. Instead, we could run the entire validation procedure on a sequence of YAML objects, from empty object to full YAML object, and then report the errors as they come. This is an attempt to mimic the "incremental typing" behavior, under which our validation works well.
    • The logic of this is that there's always a path from a valid YAML (the empty object {}) to an invalid YAML (this object, this). Finding an error on a simpler partial YAML on the path from {} to this might make better validation errors.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.