fix: an ingest refusal at a typed union names the admitted types - #1262
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Contributor
Author
|
Still valid, with one edit pending. The diagnostic change stands on its own - a typed node union should name the admitted types rather than the first branch's required field. But the new test uses a |
dereuromark
force-pushed
the
fix/ingest-names-the-admitted-type
branch
from
August 15, 2026 01:59
d18be65 to
79043d4
Compare
`figure.target` admits an image, a table, a code block or a paragraph. Feed it a
`block_quote` - the type PART 9 §4a removed when a caption on a quote became its
attribution - and this engine refused the payload, correctly, and then described
the wrong problem:
$.children[0].target is missing `src`, which the schema requires
`src` is the required property of the IMAGE branch, which is simply the first
alternative in the `oneOf`. A producer reading that message would add `src` to a
block quote. carve-js, from the same schema, says the useful thing:
a "block_quote" node sits where the schema admits only code_block, image,
paragraph, table
THE CAUSE IS ONE LINE. `checkComposition` returns `$first` - the failure of the
first branch that failed - whenever no branch of an `anyOf` / `oneOf` matched.
For a union of typed node definitions that is nearly always the wrong story,
because the branches differ by TYPE and the first one's missing field is an
artifact of branch order rather than a fact about the payload.
So a union of typed node definitions now reports the type mismatch:
$.children[0].target holds a "block_quote" node where the schema admits only
code_block, image, paragraph, table
BOTH CONDITIONS ARE REQUIRED before the message is built - the value identifies
itself as a node, and every branch pins a `type` constant - and anything else
keeps `$first` exactly as before. That is what stops the change from swallowing
a useful message: an `image` at `figure.target` with no `src` is a type the
position DOES admit, so the missing field is the real problem and is still what
gets reported. A target that is not even an object keeps its missing-`type`
report.
Which payloads are accepted and refused does not change; only what the refusal
says. The four tests pin the four paths: a refused type, an admitted type
missing a field, a value with no type at all, and a legitimate image target.
The reciprocal engine gap is markup-carve/carve-rs#982 - carve-rs accepted that
same payload rather than refusing it - and markup-carve/carve#1211 adds the
schema-side test for the admitted set.
The new case used a `block_quote` at `figure.target` as its example of a type the schema refuses. That was true of the schema on the day it was written and is about to stop being true: markup-carve/carve#1161 removed `block_quote` from the admitted set and markup-carve/carve#1213 puts it back, so the assertion would have flipped on a submodule bump and read as a regression in this diff. A `heading` is not a captionable host under any version of the clause, so the case pins the behavior under test - a typed union names the admitted set rather than the first branch's required field - and nothing else. The assertion on the set is a substring for the same reason. The CHANGELOG entry drops its verbatim message quote, which carried the same dependency and had a stray escape inside the code span.
codecov flags three `return null` lines in the new typed-union helper. They are type narrowing rather than guards: both unions the published schema writes today are typed node unions, so the branch shapes always resolve, and the checks exist because decoded JSON reaches PHP as `mixed` and the function has to be total for a union some later schema writes differently. Saying so in the file is the honest resolution. Removing them to satisfy the patch threshold would trade a coverage number for a crash on the first union that is not a typed one, and PHPStan requires the narrowing regardless.
The expectation named "code_block, image, paragraph, table" while the spec this branch pins already admits block_quote too: markup-carve/carve#1213 withdrew the attribution model and put a captioned quote back to a figure, so figure.target regained the type carve#1161 had removed. The refused type stays a heading, which is the point of choosing it - a heading is not a captionable host under any version of the clause, so only the admitted list moves with the pin. The docblock now says that rather than describing the churn as still pending. The changelog example moves off a block quote for the same reason: it is admissible again, so it no longer illustrates a node the union rejects.
dereuromark
force-pushed
the
fix/ingest-names-the-admitted-type
branch
from
August 16, 2026 01:05
79043d4 to
c44f458
Compare
dereuromark
marked this pull request as ready for review
August 16, 2026 01:06
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
When no branch of a typed node union matches, the refusal names the offending type and the admitted set instead of a required field from whichever branch happens to be first.
Why
figure.targetadmits an image, a block quote, a table, a code block or a paragraph. Feed it a node of a type the position does not admit - aheading, say - and this engine refused the payload, correctly, and then described the wrong problem:srcis the required property of the image branch, which is simply the first alternative in theoneOf. A producer reading that message would addsrcto a heading.carve-js, from the same schema, says the useful thing:
The cause
AstSchema::checkCompositionreturns$first, the failure of the first branch that failed, whenever no branch of ananyOforoneOfmatched. For a union of typed node definitions that is nearly always the wrong story: the branches differ by TYPE, so the first one's missing field is an artifact of branch order rather than a fact about the payload.What changed
A union of typed node definitions now reports the type mismatch:
Two conditions are required before that message is built, and anything else keeps
$firstexactly as before:type;typeconstant.That is what stops the change from swallowing a useful message. An
imageatfigure.targetwith nosrcis a type the position DOES admit, so the missing field is the real problem there and is still what gets reported. A target that is not even an object keeps its missing-typereport.Which payloads are accepted and refused does not change; only what the refusal says.
The fixture is a heading on purpose
A
block_quotewould have read better and is deliberately not used: markup-carve/carve#1161 removed it from the admitted set and markup-carve/carve#1213 has since put it back, so a case built on it would have asserted the opposite of the pinned schema within days. A heading is not a captionable host under any version of the clause. The admitted set inside the expectation moves with the spec pin; the refused type does not.The four tests pin the four paths: a refused type, an admitted type missing a field, a value with no type at all, and a legitimate image target.
Related
markup-carve/carve-rs#982 is the reciprocal gap: carve-rs accepted that same payload rather than refusing it. markup-carve/carve#1211 adds the schema-side test for the admitted set.