Fix: hoisted allOf branch contexts committed out of collector LIFO order - #838
Merged
Conversation
The code generator's HoistedAllOfPropertyValidationHandler pushes every
hoisted allOf branch context up front in forward order ([b0, b1, ... bN]),
then committed those child contexts in the SAME forward order. A results
collector requires strict LIFO commits (the last-pushed context must be
committed first), so committing b0 while bN is still open trips the
collector's context-sequence invariant.
In DEBUG this fires Debug.Assert "A context has been completed out-of-order"
and terminates the process; in RELEASE it silently produces corrupt results.
It only surfaces on schemas with two or more hoistable allOf branches AND a
results collector attached. The JSON Schema 2020-12 meta-schema (seven
vocabulary allOf branches) is the first common schema to expose it -- e.g.
validating any instance against {"$ref":"https://json-schema.org/draft/2020-12/schema"}
with a collector. The bool-only validation path is unaffected, which is why
the meta-schema "works" everywhere that never attaches a collector; shallow
schemas do not reproduce it at all.
Fix: commit the branch contexts in reverse of the push order, in both the
parent-hosted path (AppendValidationCode) and the standalone object-loop
path (AppendStandaloneObjectLoop). Branches are contiguous per keyword in
branchMetadataList, so reversing the keyword-group order and the within-group
order reproduces the exact reverse of the push order. All other per-branch
work (composedIsMatch accumulation, ApplyEvaluated, required checks) is
order-independent, so only the commit ordering changes.
Regenerating the full JSON Schema Test Suite with this change produces no
diff (no suite schema exercises the multi-hoisted-branch collector path), so
the change is compliance-neutral. Adds MetaSchemaCollectorTests as a
regression gate: six probes localise the trigger to "meta-schema + collector"
(independent of root vs sub-element and of collector level) and confirm the
fix.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LDvQ1iDziTdVBD6YFmrNxe
Contributor
Code Coverage Summary Report - Linux (net10.0)Summary
CoverageCorvus.Json.CodeGeneration - 80.3%
Corvus.Json.CodeGeneration.CSharp - 81.7%
Corvus.Json.ExtendedTypes - 74.9%
Corvus.Json.JsonReference - 76.7%
Corvus.Text.Json - 94.5%
Corvus.Text.Json.AsyncApi - 64.4%
Corvus.Text.Json.AsyncApi.Amqp - 0.6%
Corvus.Text.Json.AsyncApi.AzureServiceBus - 0%
Corvus.Text.Json.AsyncApi.CodeGeneration - 93.2%
Corvus.Text.Json.AsyncApi.Kafka - 0.6%
Corvus.Text.Json.AsyncApi.Mqtt - 0.9%
Corvus.Text.Json.AsyncApi.Nats - 0.4%
Corvus.Text.Json.AsyncApi.Polly - 100%
Corvus.Text.Json.AsyncApi.Testing - 96.6%
Corvus.Text.Json.AsyncApi.WebSocket - 0%
Corvus.Text.Json.CodeGeneration - 93.2%
Corvus.Text.Json.JMESPath - 93.8%
Corvus.Text.Json.JMESPath.CodeGeneration - 97.7%
Corvus.Text.Json.Jsonata - 91.6%
Corvus.Text.Json.Jsonata.CodeGeneration - 88.5%
Corvus.Text.Json.JsonLogic - 95.1%
Corvus.Text.Json.JsonLogic.CodeGeneration - 92.8%
Corvus.Text.Json.JsonPath - 95.2%
Corvus.Text.Json.JsonPath.CodeGeneration - 96.1%
Corvus.Text.Json.OpenApi - 95.7%
Corvus.Text.Json.OpenApi.CodeGeneration - 98.9%
|
Contributor
Contributor
Code Coverage Summary Report - Windows (net10.0)Summary
CoverageCorvus.Json.CodeGeneration - 80.3%
Corvus.Json.CodeGeneration.CSharp - 81.7%
Corvus.Json.ExtendedTypes - 74.9%
Corvus.Json.JsonReference - 76.7%
Corvus.Text.Json - 94.4%
Corvus.Text.Json.AsyncApi - 64.4%
Corvus.Text.Json.AsyncApi.Amqp - 0.6%
Corvus.Text.Json.AsyncApi.AzureServiceBus - 0%
Corvus.Text.Json.AsyncApi.CodeGeneration - 93.2%
Corvus.Text.Json.AsyncApi.Kafka - 0.6%
Corvus.Text.Json.AsyncApi.Mqtt - 0.9%
Corvus.Text.Json.AsyncApi.Nats - 0.4%
Corvus.Text.Json.AsyncApi.Polly - 100%
Corvus.Text.Json.AsyncApi.Testing - 96.6%
Corvus.Text.Json.AsyncApi.WebSocket - 0%
Corvus.Text.Json.CodeGeneration - 93.2%
Corvus.Text.Json.JMESPath - 93.8%
Corvus.Text.Json.JMESPath.CodeGeneration - 97.7%
Corvus.Text.Json.Jsonata - 91.6%
Corvus.Text.Json.Jsonata.CodeGeneration - 88.5%
Corvus.Text.Json.JsonLogic - 95.1%
Corvus.Text.Json.JsonLogic.CodeGeneration - 92.8%
Corvus.Text.Json.JsonPath - 95.2%
Corvus.Text.Json.JsonPath.CodeGeneration - 96.1%
Corvus.Text.Json.OpenApi - 95.7%
|
Contributor
Code Coverage Summary Report - Windows (net481)Summary
CoverageCorvus.Json.CodeGeneration - 80.4%
Corvus.Json.CodeGeneration.CSharp - 81.4%
Corvus.Json.ExtendedTypes - 72.6%
Corvus.Json.JsonReference - 73.8%
Corvus.Text.Json - 93.2%
Corvus.Text.Json.CodeGeneration - 86.2%
Corvus.Text.Json.JMESPath - 93.8%
Corvus.Text.Json.JMESPath.CodeGeneration - 97.7%
Corvus.Text.Json.Jsonata - 91.8%
Corvus.Text.Json.Jsonata.CodeGeneration - 88.6%
Corvus.Text.Json.JsonLogic - 95.1%
Corvus.Text.Json.JsonLogic.CodeGeneration - 92.8%
Corvus.Text.Json.JsonPath - 95.2%
Corvus.Text.Json.JsonPath.CodeGeneration - 96.1%
Corvus.Text.Json.Patch - 97.5%
Corvus.Text.Json.Toon - 88.2%
Corvus.Text.Json.Validator - 95.5%
Corvus.Text.Json.Yaml - 90.2%
Corvus.Toon.SystemTextJson - 90%
Corvus.Yaml.SystemTextJson - 88.8%
|
mwadams
added a commit
that referenced
this pull request
Jul 13, 2026
The additive OpenAPI rev for nested sub-workflow trace records (§15-8a): - SimulatedStep gains `skipped` (boolean; the overdue §8b marker) and `subTrace` (a `$ref` to SimulationTrace) — the document's first recursive schema (SimulatedStep.subTrace -> SimulationTrace -> steps -> SimulatedStep). Plain `$ref` with a sibling description, no allOf-with-$ref. - SimulationTrace gains optional `workflowId`, present only on a nested subTrace and absent on the root (the client's step-into ascent stops where it becomes absent). - SimulatedStep `status` extends with `paused`/`suspended` for a sub-workflow parent whose child did not finish. - The scoped step path string format is documented on `until.beforeStepId`, `pause.beforeSteps`, and the trace's `pausedBefore` (a bare id means a root step; `parent/child/...` targets a step inside a sub-workflow). Additive only: `SimulatedStep.required` stays `[stepId, status, attempt]`, no `additionalProperties: false` introduced. Server models regenerated (`corvusjson openapi-server`); the recursive SimulationTrace compiles and a new round-trip smoke test navigates a nested subTrace through the recursion. The one non-trace regenerated file is the hoisted-allOf commit-order fix (PR #838) propagating into the control-plane models on this first full regen since it landed. Gates: warning-free `dotnet build Corvus.Text.Json.slnx`; recursive-model smoke, workspace API (24), and simulator (11) tests green; client conformance (273) green; catalog `-Check` 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LDvQ1iDziTdVBD6YFmrNxe
Contributor
|
This was referenced Jul 13, 2026
mwadams
added a commit
that referenced
this pull request
Jul 13, 2026
…840) The docs-only #829 merge cut tag 5.2.3 without a version-history entry, leaving every subsequent heading one release behind its tag. Renumbers V5.2.3->V5.2.4 (HttpClientTransport), V5.2.4->V5.2.5 (nullable boolean), V5.2.5->V5.2.6 (numeric conversions); adds the missing documentation-only V5.2.3 stub; and adds the missing V5.2.8 entry for the hoisted-allOf collector-ordering fix (#838). Claude-Session: https://claude.ai/code/session_0194AjRoXpqKCsNGLq7jcCso (cherry picked from commit d76edf2) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
mwadams
added a commit
that referenced
this pull request
Jul 14, 2026
The additive OpenAPI rev for nested sub-workflow trace records (§15-8a): - SimulatedStep gains `skipped` (boolean; the overdue §8b marker) and `subTrace` (a `$ref` to SimulationTrace) — the document's first recursive schema (SimulatedStep.subTrace -> SimulationTrace -> steps -> SimulatedStep). Plain `$ref` with a sibling description, no allOf-with-$ref. - SimulationTrace gains optional `workflowId`, present only on a nested subTrace and absent on the root (the client's step-into ascent stops where it becomes absent). - SimulatedStep `status` extends with `paused`/`suspended` for a sub-workflow parent whose child did not finish. - The scoped step path string format is documented on `until.beforeStepId`, `pause.beforeSteps`, and the trace's `pausedBefore` (a bare id means a root step; `parent/child/...` targets a step inside a sub-workflow). Additive only: `SimulatedStep.required` stays `[stepId, status, attempt]`, no `additionalProperties: false` introduced. Server models regenerated (`corvusjson openapi-server`); the recursive SimulationTrace compiles and a new round-trip smoke test navigates a nested subTrace through the recursion. The one non-trace regenerated file is the hoisted-allOf commit-order fix (PR #838) propagating into the control-plane models on this first full regen since it landed. Gates: warning-free `dotnet build Corvus.Text.Json.slnx`; recursive-model smoke, workspace API (24), and simulator (11) tests green; client conformance (273) green; catalog `-Check` 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LDvQ1iDziTdVBD6YFmrNxe
mwadams
added a commit
that referenced
this pull request
Jul 14, 2026
…on with main's Create() feature Rebasing onto 5.2.7 main (43d84f4 #836 generated Create() factories, d80c19b #838 hoisted-allOf LIFO fix — the branch's cherry-pick of #838 was patch-identical and deduped automatically) merged textually clean but left two cross-side semantic gaps: - The branch added IJsonDocument.CloneElementAsBuilder(int, JsonWorkspace); main's new ParsedJsonDocumentBuilder implements IJsonDocument without it (CS0535 on all four TFMs). Implemented in the class's construction-only idiom — `throw ConstructionOnly()`, beside its sibling CloneElement<TElement>: the builder never hands out elements over its in-progress state. - CatalogVersion's remark referenced <see cref="Create"/>, unambiguous before the generated type gained the standard Create() overloads (#836) and now CS0419 under TreatWarningsAsErrors. Reworded to name the hand-written from-columns factory explicitly; a repo-wide sweep found no other unqualified cref="Create". Gates: full solution build 0 warnings; MetaSchemaCollectorTests 6/6 (#838 guard); ParsedJsonDocumentBuilderTests + GeneratedCreateTests 58/58 (#836); Arazzo Durability suite 462/462. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LDvQ1iDziTdVBD6YFmrNxe
mwadams
added a commit
that referenced
this pull request
Jul 14, 2026
…7 generator The Create() adoption prerequisite (handover pack, prerequisite 2): after rebasing onto main (#836 generated Create() factories + #838 hoisted-allOf LIFO fix), regenerate every checked-in corpus with the rebased Release CLI so the wave rows convert against current generated surface. - Arazzo ControlPlane.Server + ControlPlane.Cli + Kyc/Ledger/Onboarding/ Notifications samples: clean regen (delete + regenerate; the Notifications lock preserved). 799 modified files carry the additive Create() surface; 54 deleted files are stale mirrored Server/Cli models from earlier contract revisions that additive overlays never swept (none referenced). The only non-Create in-file changes are lock metadata and six numeric-operator lines (explicit -> implicit), the #834 numeric-operator fix reaching corpora last generated before it. - Example recipes 029-039: regenerated (12/12 steps); all 11 affected recipes build and run (servers verified to start cleanly). - Benchmark C/ corpora: regenerated; B/ untouched. Net +94,257/-14,900 across benchmarks; the regen script's large per-corpus removed-line counts were rename pairs (1,880 identical-content files under new collision-hash names, naming drift from the branch's accumulated generator changes). The worst-drift corpus (Ui5) compiles with 0 warnings. - Code-sample catalog refreshed for the moved/new blocks (JsonDocumentBuilder/CodeGenerator/SourceGenerator/001-DataObject); -Check exits 0. Gates: full solution build 0 warnings / 0 errors; ControlPlane.Server tests 245/245; ControlPlane.Cli tests 58/58; reserved-name check clean (no create/createArray/createObject schema properties, no renamed members). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LDvQ1iDziTdVBD6YFmrNxe
mwadams
added a commit
that referenced
this pull request
Jul 16, 2026
…der (#838) The code generator's HoistedAllOfPropertyValidationHandler pushes every hoisted allOf branch context up front in forward order ([b0, b1, ... bN]), then committed those child contexts in the SAME forward order. A results collector requires strict LIFO commits (the last-pushed context must be committed first), so committing b0 while bN is still open trips the collector's context-sequence invariant. In DEBUG this fires Debug.Assert "A context has been completed out-of-order" and terminates the process; in RELEASE it silently produces corrupt results. It only surfaces on schemas with two or more hoistable allOf branches AND a results collector attached. The JSON Schema 2020-12 meta-schema (seven vocabulary allOf branches) is the first common schema to expose it -- e.g. validating any instance against {"$ref":"https://json-schema.org/draft/2020-12/schema"} with a collector. The bool-only validation path is unaffected, which is why the meta-schema "works" everywhere that never attaches a collector; shallow schemas do not reproduce it at all. Fix: commit the branch contexts in reverse of the push order, in both the parent-hosted path (AppendValidationCode) and the standalone object-loop path (AppendStandaloneObjectLoop). Branches are contiguous per keyword in branchMetadataList, so reversing the keyword-group order and the within-group order reproduces the exact reverse of the push order. All other per-branch work (composedIsMatch accumulation, ApplyEvaluated, required checks) is order-independent, so only the commit ordering changes. Regenerating the full JSON Schema Test Suite with this change produces no diff (no suite schema exercises the multi-hoisted-branch collector path), so the change is compliance-neutral. Adds MetaSchemaCollectorTests as a regression gate: six probes localise the trigger to "meta-schema + collector" (independent of root vs sub-element and of collector level) and confirm the fix. Claude-Session: https://claude.ai/code/session_01LDvQ1iDziTdVBD6YFmrNxe Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mwadams
added a commit
that referenced
this pull request
Jul 16, 2026
…840) The docs-only #829 merge cut tag 5.2.3 without a version-history entry, leaving every subsequent heading one release behind its tag. Renumbers V5.2.3->V5.2.4 (HttpClientTransport), V5.2.4->V5.2.5 (nullable boolean), V5.2.5->V5.2.6 (numeric conversions); adds the missing documentation-only V5.2.3 stub; and adds the missing V5.2.8 entry for the hoisted-allOf collector-ordering fix (#838). Claude-Session: https://claude.ai/code/session_0194AjRoXpqKCsNGLq7jcCso (cherry picked from commit d76edf2) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
mwadams
added a commit
that referenced
this pull request
Jul 16, 2026
The additive OpenAPI rev for nested sub-workflow trace records (§15-8a): - SimulatedStep gains `skipped` (boolean; the overdue §8b marker) and `subTrace` (a `$ref` to SimulationTrace) — the document's first recursive schema (SimulatedStep.subTrace -> SimulationTrace -> steps -> SimulatedStep). Plain `$ref` with a sibling description, no allOf-with-$ref. - SimulationTrace gains optional `workflowId`, present only on a nested subTrace and absent on the root (the client's step-into ascent stops where it becomes absent). - SimulatedStep `status` extends with `paused`/`suspended` for a sub-workflow parent whose child did not finish. - The scoped step path string format is documented on `until.beforeStepId`, `pause.beforeSteps`, and the trace's `pausedBefore` (a bare id means a root step; `parent/child/...` targets a step inside a sub-workflow). Additive only: `SimulatedStep.required` stays `[stepId, status, attempt]`, no `additionalProperties: false` introduced. Server models regenerated (`corvusjson openapi-server`); the recursive SimulationTrace compiles and a new round-trip smoke test navigates a nested subTrace through the recursion. The one non-trace regenerated file is the hoisted-allOf commit-order fix (PR #838) propagating into the control-plane models on this first full regen since it landed. Gates: warning-free `dotnet build Corvus.Text.Json.slnx`; recursive-model smoke, workspace API (24), and simulator (11) tests green; client conformance (273) green; catalog `-Check` 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LDvQ1iDziTdVBD6YFmrNxe
mwadams
added a commit
that referenced
this pull request
Jul 16, 2026
…on with main's Create() feature Rebasing onto 5.2.7 main (43d84f4 #836 generated Create() factories, d80c19b #838 hoisted-allOf LIFO fix — the branch's cherry-pick of #838 was patch-identical and deduped automatically) merged textually clean but left two cross-side semantic gaps: - The branch added IJsonDocument.CloneElementAsBuilder(int, JsonWorkspace); main's new ParsedJsonDocumentBuilder implements IJsonDocument without it (CS0535 on all four TFMs). Implemented in the class's construction-only idiom — `throw ConstructionOnly()`, beside its sibling CloneElement<TElement>: the builder never hands out elements over its in-progress state. - CatalogVersion's remark referenced <see cref="Create"/>, unambiguous before the generated type gained the standard Create() overloads (#836) and now CS0419 under TreatWarningsAsErrors. Reworded to name the hand-written from-columns factory explicitly; a repo-wide sweep found no other unqualified cref="Create". Gates: full solution build 0 warnings; MetaSchemaCollectorTests 6/6 (#838 guard); ParsedJsonDocumentBuilderTests + GeneratedCreateTests 58/58 (#836); Arazzo Durability suite 462/462. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LDvQ1iDziTdVBD6YFmrNxe
mwadams
added a commit
that referenced
this pull request
Jul 16, 2026
…7 generator The Create() adoption prerequisite (handover pack, prerequisite 2): after rebasing onto main (#836 generated Create() factories + #838 hoisted-allOf LIFO fix), regenerate every checked-in corpus with the rebased Release CLI so the wave rows convert against current generated surface. - Arazzo ControlPlane.Server + ControlPlane.Cli + Kyc/Ledger/Onboarding/ Notifications samples: clean regen (delete + regenerate; the Notifications lock preserved). 799 modified files carry the additive Create() surface; 54 deleted files are stale mirrored Server/Cli models from earlier contract revisions that additive overlays never swept (none referenced). The only non-Create in-file changes are lock metadata and six numeric-operator lines (explicit -> implicit), the #834 numeric-operator fix reaching corpora last generated before it. - Example recipes 029-039: regenerated (12/12 steps); all 11 affected recipes build and run (servers verified to start cleanly). - Benchmark C/ corpora: regenerated; B/ untouched. Net +94,257/-14,900 across benchmarks; the regen script's large per-corpus removed-line counts were rename pairs (1,880 identical-content files under new collision-hash names, naming drift from the branch's accumulated generator changes). The worst-drift corpus (Ui5) compiles with 0 warnings. - Code-sample catalog refreshed for the moved/new blocks (JsonDocumentBuilder/CodeGenerator/SourceGenerator/001-DataObject); -Check exits 0. Gates: full solution build 0 warnings / 0 errors; ControlPlane.Server tests 245/245; ControlPlane.Cli tests 58/58; reserved-name check clean (no create/createArray/createObject schema properties, no renamed members). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LDvQ1iDziTdVBD6YFmrNxe
This was referenced Jul 26, 2026
This was referenced Aug 3, 2026
Bump Azure.Data.AppConfiguration, corvus.json.cli and Corvus.Text.Json
RESousa01/bmad-runtime-dev#14
Closed
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
1 participant
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.
Problem
The code generator's
HoistedAllOfPropertyValidationHandlerpushes every hoistedallOfbranch context up front in forward order ([b0, b1, ... bN]), then committed those child contexts in the same forward order. A results collector requires strict LIFO commits — the last-pushed context must be committed first — so committingb0whilebNis still open trips the collector's context-sequence invariant.In DEBUG this fires
Debug.Assert("A context has been completed out-of-order")and terminates the process; in RELEASE it silently produces corrupt results.It only surfaces on schemas with two or more hoistable
allOfbranches AND a results collector attached. The JSON Schema 2020-12 meta-schema (seven vocabularyallOfbranches) is the first common schema to expose it — e.g. validating any instance against{"$ref":"https://json-schema.org/draft/2020-12/schema"}with a collector. The bool-only validation path is unaffected, which is why the meta-schema "works" everywhere that never attaches a collector; shallow schemas do not reproduce it at all.Fix
Commit the branch contexts in reverse of the push order, in both the parent-hosted path (
AppendValidationCode) and the standalone object-loop path (AppendStandaloneObjectLoop). Branches are contiguous per keyword inbranchMetadataList, so reversing the keyword-group order and the within-group order reproduces the exact reverse of the push order. All other per-branch work (composedisMatchaccumulation,ApplyEvaluated, required checks) is order-independent, so only the commit ordering changes.Validation
MetaSchemaCollectorTests— six probes that localise the trigger to "2020-12 meta-schema + results collector" (independent of root vs nested sub-element, and independent of collector level: Basic and Detailed both failed before). RED before the fix (process-terminating assert), GREEN after.🤖 Generated with Claude Code
https://claude.ai/code/session_01LDvQ1iDziTdVBD6YFmrNxe