Fix AutoDetectionDraft falling back to Draft 07 for every property-level schema - #187
Merged
Merged
Conversation
…vel schema $schema is a document-root-only keyword, but AutoDetectionDraft re-derived it from whichever JsonSchema node it was handed. Every navigated subschema (properties, $ref targets, composition branches, ...) never carries $schema, so draft detection always fell back to Draft 07 outside the literal document root, silently dropping Draft 2019-09-only behavior like minContains/maxContains. JsonSchema now tracks the $schema URI in effect for each node: the node's own $schema if declared, otherwise inherited from the nearest ancestor. navigate() and withJson() re-derive it after cloning, so the inherited value survives down the whole tree while a local override on an embedded schema resource (a subschema with its own $id, per the JSON Schema core spec) still takes effect for its own descendants. Fixes #186 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nLMK2e753YDaXexRr1Z1B
An allOf branch failure surfaces as a generic AllOfException in fail-fast mode, discarding the underlying MinContainsException/MaxContainsException; the specific error is only preserved in the branch's error collection when collecting errors. Switch the AutoDetectionDraft regression test to collect errors and unwrap the branch-specific exception, matching the existing RequiredPropertyTest pattern for allOf branch assertions. Verified against master: the full suite (2971 tests) passes, and the new test fails as expected when the fix in JsonSchema/AutoDetectionDraft is reverted, confirming it actually exercises the regression. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nLMK2e753YDaXexRr1Z1B
Claude's session-local issue/topic tracking notes under .claude/ are working context only and must never be committed (per this repo's own CLAUDE.md policy); ignoring the directory keeps it out of git status without requiring it to be staged or tracked. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nLMK2e753YDaXexRr1Z1B
Coverage Report for CI Build 31141920609Coverage increased (+0.001%) to 98.76%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
The previous tests only covered inheritance (a node with no $schema of its own picking up the document root's declared draft). They didn't cover the other half of the fix: a node that IS itself an embedded schema resource (carries its own $id) may re-declare $schema per the JSON Schema core spec, and that override must win locally and keep propagating to its own descendants rather than reverting to the root's value one level down. Adds unit coverage on JsonSchema directly (inherit / override / propagate to descendants / withJson parity) plus a pipeline-level test proving AutoDetectionDraft actually honours the override end-to-end: a $defs entry that re-declares the draft-07 $schema drops minContains/maxContains enforcement while a sibling $defs entry with no override keeps it, despite both being reached from the same 2019-09 document root. All 6 new assertions fail against the pre-fix source, confirming they exercise the fix rather than passing vacuously. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nLMK2e753YDaXexRr1Z1B
testAutoDetectionDraftAppliesRootDraftToPropertyLevelRefSiblingMerge and its fixture reached a $ref target through an allOf composition branch, but that path funnels through the same JsonSchema::navigate()/withJson() the fix lives in — the composition wrapper's synthetic JSON carries no $schema of its own, so it just re-derives the already-inherited value and hands off to the same navigate() call the plain-$ref test already exercises. testAutoDetectionDraftHonoursASchemaOverrideDeclaredByAnEmbeddedResource covers inheritance AND override more directly (and without the AllOfException-unwrapping boilerplate this test needed), so the allOf variant added no coverage beyond mirroring the issue's literal reproduction shape. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nLMK2e753YDaXexRr1Z1B
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nLMK2e753YDaXexRr1Z1B
Relocates the $schema-override regression coverage out of ArrayContainsTest into tests/Issues/Issue/Issue186Test.php with its fixture under tests/Schema/Issues/186/, matching this repo's convention for issue-driven regression tests (see Issue179Test, Issue169Test, etc.) instead of living alongside the general contains/minContains/maxContains behavior tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019nLMK2e753YDaXexRr1Z1B
wol-soft
added a commit
that referenced
this pull request
Aug 7, 2026
…-draft-aware Pulls in the AutoDetectionDraft fix (#187) for property-level $schema inheritance/override, which this branch's own investigation (see .claude/topics/property-merger-consolidation/) had independently found and was about to fix separately. # Conflicts: # src/Model/SchemaDefinition/JsonSchema.php
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
3 participants
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.
$schema is a document-root-only keyword, but AutoDetectionDraft re-derived it
from whichever JsonSchema node it was handed. Every navigated subschema
(properties, $ref targets, composition branches, ...) never carries $schema,
so draft detection always fell back to Draft 07 outside the literal document
root, silently dropping Draft 2019-09-only behavior like minContains/maxContains.
JsonSchema now tracks the $schema URI in effect for each node: the node's own
$schema if declared, otherwise inherited from the nearest ancestor. navigate()
and withJson() re-derive it after cloning, so the inherited value survives
down the whole tree while a local override on an embedded schema resource
(a subschema with its own $id, per the JSON Schema core spec) still takes
effect for its own descendants.
Fixes #186
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_019nLMK2e753YDaXexRr1Z1B