From 23a406c6b307c69e8d17331e060cc6ba22090097 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 3 Aug 2026 20:03:13 +0300 Subject: [PATCH 01/13] forms: recurse one level into nested-aggregate members' schemas mergeSchemaExtras previously annotated (x-order, required, title, Quantity/ Choice/widget hints) only an action's own top-level members. A member that was itself a reflectable aggregate -- a nested struct, or std::vector -- got none of that on its own sub-members, and its $defs entry got no required array at all, per docs/spec/forms/forms.md's documented "flat actions only" boundary. Domains that are naturally nested (a measurement with a repeated specimen sub-record) had to either flatten the action type (impossible for a repeated sub-record without a fixed max count) or hand-write the form, bypassing the schema-driven generator for exactly the screens that would benefit most from it. Recurse one level into a nested-aggregate member's own schema, applying the same annotation rules the top level already applies (factored into a shared annotateBasicMemberProperty helper). Two schema shapes exist for a nested aggregate, both handled: glaze deduplicates via a shared $defs entry referenced by $ref when the nested type is used two or more times anywhere in the schema, or inlines the object schema directly into the property when it is used exactly once -- annotateNestedAggregateRef resolves whichever form applies before recursing. Deliberately capped at exactly one level, matching the design doc's own suggested bound: a nested aggregate's own nested-aggregate members are left unannotated, and computed fields/formLayout/fieldSpans/formRules stay top-level-only. Purely additive -- an action with no nested-aggregate member has nothing to trigger on, so its schema is byte-for-byte unchanged (the full existing suite, including every prior schema-generation test, passes unmodified). Render-side add/remove affordances for a repeated-aggregate array are a separate, downstream renderer concern the issue explicitly called out as distinct from the annotation-reach gap this closes, and are out of scope here. Closes #25 Signed-off-by: Yaraslau Tamashevich --- docs/spec/forms/forms.md | 73 +++++++-- include/morph/forms/forms.hpp | 219 +++++++++++++++++++++++++++ tests/CMakeLists.txt | 1 + tests/test_nested_forms.cpp | 273 ++++++++++++++++++++++++++++++++++ 4 files changed, 550 insertions(+), 16 deletions(-) create mode 100644 tests/test_nested_forms.cpp diff --git a/docs/spec/forms/forms.md b/docs/spec/forms/forms.md index 1ed5fbf1..19daa5a5 100644 --- a/docs/spec/forms/forms.md +++ b/docs/spec/forms/forms.md @@ -908,9 +908,9 @@ so it never satisfies `EmptyCapableField` in the first place. (This differs from the `required`-array derivation in `mergeSchemaExtras`, which checks `isStdOptional` **explicitly** — see [Required-ness rule](#required-ness-rule).) The predicate is `noexcept` and `constexpr`, and it inspects only the action's -**own top-level members** — the same flat-actions-only scope as schema -generation ([Scope: flat actions only](#scope-flat-actions-only)); it does not -recurse into nested aggregates. +**own top-level members**; unlike `schemaJson()`'s schema generation (see +[Nested aggregates (one level)](#nested-aggregates-one-level)), it does **not** +recurse into a nested aggregate member's own fields. ## Cross-field rules — the `x-rules` vocabulary @@ -1257,19 +1257,60 @@ for the exhaustive tables and design rationale. ## Failure modes -### Scope: flat actions only - -Annotation and `required`-derivation operate **exclusively on the action's -top-level members**. `mergeSchemaExtras` reflects `A`'s members with -`forEachNamedMember(probe, …)` and patches `dom["properties"][name]` for each — -it never descends into member types. A member that is itself an aggregate is -emitted by glaze into `$defs` and referenced by `$ref`; the generator does not -recurse into that definition, so its sub-members receive **none** of the `x-*` -annotations and are **not** part of any synthesised `required` array (the nested -`$def` gets no `required` at all). Actions meant to drive a generated form must -therefore be **flat**: every field the renderer should understand has to be a -direct member of the action type. Nesting is not a documented form-generation -path. +### Nested aggregates (one level) + +A member whose type is itself a reflectable aggregate — a plain nested +struct, or `std::vector` (a repeated aggregate) — gets its **own** +members annotated too, one level down: `x-order`, `title`/`FieldMeta`, +`required`, and the `Quantity`/`Choice`/widget/ranged-bounds rules the top +level already applies. This closes the gap a flat-only generator has for +domains that are naturally nested (a measurement with a repeated specimen +sub-record, a document with a nested address) — previously such a member's +sub-fields were silently unannotated and absent from the generated form. + +Two schema shapes exist for a nested aggregate, and both are recursed into: + +- **Deduplicated (`$ref`/`$defs`)** — glaze shares one `$defs` entry, `$ref`'d + from every property, when the nested type is used **two or more times** + anywhere in the schema. The shared `$defs` entry is annotated once; every + property that `$ref`s it sees the same annotations. +- **Inlined** — glaze writes the object schema directly into the property + itself (no `$ref`/`$defs` at all) when the nested type is used **exactly + once**. The property node itself is annotated in place. + +`mergeSchemaExtras` resolves whichever form applies (`annotateNestedAggregateRef`, +`forms.hpp`) and hands the resolved node to the same per-member annotation +logic the top level uses (`annotateBasicMemberProperty`), applied against the +nested type's own reflection. + +**Depth is capped at exactly one level.** If the nested type itself has a +member that is itself an aggregate, that member's own sub-members are left +exactly as glaze emitted them — unannotated, matching this generator's +original flat-only behaviour for anything past one level. Computed fields, +`formLayout`/`fieldSpans`, and `formRules` also remain **top-level only**: a +nested aggregate declaring any of those has no effect on the generated +schema. Both limits keep generated forms comprehensible and keep the +generator itself simple, rather than becoming a general recursive-descent +schema compiler. + +**Purely additive.** An action with no nested-aggregate member has nothing +here to trigger on, so its generated schema is byte-for-byte unchanged. A +pre-existing action that *does* have a nested-aggregate member sees its +schema gain annotations it previously lacked — the whole point of this +feature — with no change to any of its flat top-level members. + +The nested type must be **default-constructible**, exactly like the +top-level action type (see below): the recursion builds its own probe +instance purely to enumerate its members via reflection. + +### Scope: flat actions only (form layout, computed fields, and rules) + +`formLayout`/`fieldSpans` ([Layout & grouping](#layout--grouping)) and +`formRules` ([Cross-field rules](#cross-field-rules-x-rules)) are read only +from the top-level action type — they are not consulted on a nested +aggregate, even at the one level `mergeSchemaExtras` does otherwise recurse +into (see [Nested aggregates (one level)](#nested-aggregates-one-level) +above). Computed fields (`computedFields`) are likewise top-level only. The action type must also be **default-constructible**: `mergeSchemaExtras` builds a probe instance (`A probe{}`) purely to enumerate member names and types diff --git a/include/morph/forms/forms.hpp b/include/morph/forms/forms.hpp index 4e750e4b..f806376a 100644 --- a/include/morph/forms/forms.hpp +++ b/include/morph/forms/forms.hpp @@ -68,6 +68,16 @@ /// `morph::time::Timestamp` members need no extension keys: their schema /// carries the standard `"format": "date-time"` annotation. /// +/// **Nested aggregates (one level).** A member whose type is itself a +/// reflectable aggregate — a plain nested struct, or `std::vector` — gets +/// its own members annotated too, one level down: `x-order`, title/`FieldMeta`, +/// `required`, and the `Quantity`/`Choice`/widget/ranged-bounds rules above, +/// applied against the nested type's own reflection. Recursion stops after +/// this one level (a nested aggregate's own nested-aggregate members are left +/// unannotated), and computed fields/`formLayout`/`fieldSpans`/`formRules` +/// remain top-level-only. See docs/spec/forms/forms.md, "Nested aggregates +/// (one level)", and `detail::annotateNestedAggregateRef`. +/// /// @par Declaring optional fields /// Required is the default. An action opts individual fields out with a /// static member list: @@ -1534,6 +1544,194 @@ void collectComputedInputs(const A& probe, const ComputedField{resolveMemberName(probe, Inputs)...}); } +/// @brief Trait: is `T` a `std::vector<...>`? Exposes the element type as +/// `ValueType` (`void` when `T` is not a vector). +template +struct IsStdVector : std::false_type { + using ValueType = void; +}; + +template +struct IsStdVector> : std::true_type { + using ValueType = T; +}; + +/// @brief Concept: `T` is glaze-reflectable as a JSON object -- the same test +/// that decides whether glaze emits a member into `$defs`/`$ref` +/// rather than inline. Shared by the one-level nested-aggregate +/// recursion below and `reconcileDeclaredPrecision` elsewhere. +template +concept ReflectableAggregate = glz::reflectable || glz::glaze_object_t; + +/// @brief Applies the same title/`FieldMeta`/`Quantity`/`Choice`/widget/ +/// ranged-bounds annotations `mergeSchemaExtras`'s top-level pass +/// applies, to one property node. Shared by that top-level pass and +/// `annotateNestedAggregate` below (the one-level nested-aggregate pass) so +/// both apply identical per-member rules. +/// +/// Deliberately excludes computed-field annotations (`x-computed`/ +/// `x-readonly`) and `x-order`: computed fields are not supported inside a +/// nested aggregate (see `annotateNestedAggregate`), and `x-order`'s source index +/// differs by caller, so each caller sets it itself. +/// @tparam Owner The type declaring @p name (drives `FieldMeta`/widget-override lookup). +/// @tparam Member The static type of the member itself (drives type-driven annotations). +/// @param property DOM node for this one property; annotations are merged in, not replacing. +/// @param name Wire (JSON) name of the member, for `FieldMeta`/widget-override lookup. +template +void annotateBasicMemberProperty(glz::generic_u64& property, std::string_view name) { + const FieldMeta* fieldMeta = findFieldMeta(name); + std::string_view const declaredLabel = fieldMeta != nullptr ? fieldMeta->label : std::string_view{}; + property["title"] = declaredLabel.empty() ? inferTitle(name) : std::string{declaredLabel}; + if (fieldMeta != nullptr) { + if (!fieldMeta->help.empty()) { + property["description"] = std::string{fieldMeta->help}; + } + if (!fieldMeta->placeholder.empty()) { + property["x-placeholder"] = std::string{fieldMeta->placeholder}; + } + if (fieldMeta->readOnly) { + property["x-readonly"] = true; + } + if (fieldMeta->hidden) { + property["x-hidden"] = true; + } + if (!fieldMeta->i18nKey.empty()) { + property["x-i18nKey"] = std::string{fieldMeta->i18nKey}; + } + } + + if constexpr (units::isQuantity) { + property["x-decimalPlaces"] = std::uint64_t{Member::declaredDecimals}; + auto const alternatives = Member::unitAlternatives(); + if (!alternatives.empty()) { + glz::generic_u64::array_t list{}; + for (auto const& alternative : alternatives) { + auto const meta = + units::UnitTraits>::meta(alternative.unit); + glz::generic_u64 entry{}; + entry["id"] = std::string{meta.id}; + entry["display"] = std::string{meta.display}; + entry["decimals"] = std::uint64_t{meta.defaultDecimals}; + entry["num"] = alternative.num; + entry["den"] = alternative.den; + list.emplace_back(std::move(entry)); + } + property["x-unitAlternatives"] = list; + } + } + if constexpr (isChoice) { + property["x-optionsAction"] = std::string{Member::optionsAction()}; + property["x-optionValue"] = std::string{Member::valueField()}; + property["x-optionLabel"] = std::string{Member::labelField()}; + if constexpr (!Member::optionsDependsOn().empty()) { + glz::generic_u64::array_t dependsOn{}; + for (auto const& parentName : Member::optionsDependsOn()) { + dependsOn.emplace_back(std::string{parentName}); + } + property["x-optionsDependsOn"] = dependsOn; + } + } + + std::string_view widgetHint{}; + if constexpr (DeclaresWidget) { + widgetHint = Member::widget(); + } + if constexpr (HasFieldMetadataWidgets) { + if (auto const overrideWidget = widgetOverride(name); !overrideWidget.empty()) { + widgetHint = overrideWidget; + } + } + if (!widgetHint.empty()) { + property["x-widget"] = std::string{widgetHint}; + } + if constexpr (DeclaresRangedBounds) { + using Bound = std::remove_cvref_t; + if constexpr (std::floating_point) { + property["x-min"] = static_cast(Member::min()); + property["x-max"] = static_cast(Member::max()); + property["x-step"] = static_cast(Member::step()); + } else { + property["x-min"] = static_cast(Member::min()); + property["x-max"] = static_cast(Member::max()); + property["x-step"] = static_cast(Member::step()); + } + } +} + +/// @brief One level of recursion (see `docs/spec/forms/forms.md`, "Nested +/// aggregates (one level)"): annotates @p node -- the object-schema +/// DOM node for a nested-aggregate member -- applying `required` and +/// `annotateBasicMemberProperty`'s rules to its own properties. +/// +/// @p node is @e which DOM node depends on how many places in the whole +/// schema reference `Sub`: glaze **inlines** the object schema directly into +/// the referencing property when `Sub` is used exactly once (so @p node +/// *is* that property node), but **deduplicates** via `$defs`/`$ref` when +/// `Sub` is used two or more times (so @p node is the shared `$defs` entry, +/// resolved by the caller). Both forms have the identical `{"properties": +/// {...}}` shape this function needs, so one implementation handles both -- +/// see the call site in `mergeSchemaExtras` for how @p node is resolved. +/// +/// Deliberately does **not** recurse again: if `Sub` itself has a member that +/// is itself an aggregate, that member is left exactly as glaze emitted it -- +/// unannotated, matching today's behaviour beyond one level. This bounds the +/// generator to one level of nesting, as `docs/spec/forms/forms.md` documents. +/// Computed fields, `formLayout`/`fieldSpans`, and `formRules` also stay +/// top-level-only; a nested `Sub` declaring any of those has no effect here. +/// +/// @tparam Sub Nested aggregate type (default-constructible, glaze-reflectable +/// -- the same requirements the top-level action type already has). +/// @param node The object-schema DOM node to annotate in place (see above). +template +void annotateNestedAggregate(glz::generic_u64& node) { + Sub probe{}; + glz::generic_u64::array_t requiredNames{}; + forEachNamedMember(probe, [&](std::string_view name, const auto& member) { + using Member = std::remove_cvref_t; + if (!(isStdOptional || declaredOptional(name))) { + requiredNames.emplace_back(std::string{name}); + } + auto& property = node["properties"][std::string{name}]; + property["x-order"] = std::uint64_t{I}; + annotateBasicMemberProperty(property, name); + }); + // Idempotent if two members (or two actions sharing this schema call) + // resolve to the same $defs entry: re-deriving the identical required + // array is harmless. + node["required"] = requiredNames; +} + +/// @brief Resolves the object-schema DOM node for a nested-aggregate member, +/// given the property (or array `items`) node glaze wrote for it, and +/// annotates it via `annotateNestedAggregate`. +/// +/// Handles both forms `Sub` can take in the schema (see +/// `annotateNestedAggregate`'s doc comment): a `$ref` into `$defs` (`Sub` used +/// 2+ times somewhere in the schema) resolves to that shared def; anything +/// else is assumed to be the inlined object schema itself (`Sub` used exactly +/// once). A property that is neither -- glaze emitted something other than an +/// object schema for a type this function's caller already confirmed is a +/// `ReflectableAggregate` -- is left untouched rather than guessed at. +/// @tparam Sub Nested aggregate type, as `annotateNestedAggregate` requires. +/// @param dom The whole schema DOM (so a `$ref`'s `$defs` entry can be found). +/// @param propertyOrItems The property node itself (single nested member) or its +/// array `items` node (`std::vector` member). +template +void annotateNestedAggregateRef(glz::generic_u64& dom, glz::generic_u64& propertyOrItems) { + constexpr std::string_view kDefsPrefix = "#/$defs/"; + if (propertyOrItems.contains("$ref")) { + if (auto const* ref = propertyOrItems["$ref"].get_if()) { + if (std::string_view{*ref}.starts_with(kDefsPrefix)) { + annotateNestedAggregate(dom["$defs"][std::string{ref->substr(kDefsPrefix.size())}]); + } + } + return; + } + if (propertyOrItems.contains("properties")) { + annotateNestedAggregate(propertyOrItems); + } +} + /// @brief The DOM post-merge behind `schemaJson`: adds the derived `required` /// array, `x-order`, `x-decimalPlaces`, and (for actions declaring /// `computedFields`) `x-computed`/`x-readonly` to a glaze-produced schema. @@ -1694,6 +1892,27 @@ template property["x-step"] = static_cast(Member::step()); } } + + // Nested aggregates (one level -- docs/spec/forms/forms.md, "Nested + // aggregates (one level)"): a member whose type is itself a + // reflectable aggregate gets an object schema from glaze -- either + // inlined directly into this property (the type is used exactly once + // in the whole schema) or shared via `$defs`/`$ref` (used 2+ times); + // `annotateNestedAggregateRef` resolves whichever form it is. Recurse + // one level so that object schema's own members get `x-order`/ + // `required`/title/Quantity/Choice/widget annotations too, instead of + // being silently unannotated. Purely additive: an action with no + // nested aggregate member has nothing here to trigger on, so its + // schema is byte-for-byte unchanged. + if constexpr (ReflectableAggregate) { + annotateNestedAggregateRef(dom, property); + } else if constexpr (IsStdVector::value && + ReflectableAggregate::ValueType>) { + using ItemType = typename IsStdVector::ValueType; + if (property.contains("items")) { + annotateNestedAggregateRef(dom, property["items"]); + } + } }); // Always assign — an explicit empty array beats leaving whatever the // schema writer may have emitted (or omitted) for `required`. diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 86bb7ff2..fb534c86 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -60,6 +60,7 @@ add_executable(morph_tests test_rational.cpp test_quantity.cpp test_quantity_forms.cpp + test_nested_forms.cpp test_flows_apps.cpp test_views.cpp test_computed_fields.cpp diff --git a/tests/test_nested_forms.cpp b/tests/test_nested_forms.cpp new file mode 100644 index 00000000..1a98e690 --- /dev/null +++ b/tests/test_nested_forms.cpp @@ -0,0 +1,273 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Coverage for issue #25: form generation recurses one level into a +// nested-aggregate member's object schema -- a directly-nested struct member +// or a `std::vector` repeated aggregate -- applying the same +// title/x-order/required/widget rules the top level already applies, instead +// of leaving it entirely unannotated. Two distinct schema shapes exist for a +// nested aggregate (see forms.hpp's `annotateNestedAggregateRef`): glaze +// *inlines* the object schema directly into the property when the nested +// type is used exactly once in the whole schema, and *deduplicates* it via a +// shared `$defs` entry (referenced by `$ref`) when it is used two or more +// times. Both are exercised below. Recursion stops after one level (see +// docs/spec/forms/forms.md, "Nested aggregates (one level)"). + +#include +#include +#include +#include +#include +#include +#include +#include + +// Named namespace (not anonymous): glaze reflection requires the reflected +// type to have linkage (see test_bridge_fixes.cpp for the same note). +namespace nestedforms { + +struct Specimen { + double massDry = 0.0; + double massWet = 0.0; + std::optional note; // never required, one level down either +}; + +struct Attachment { + std::string filename; + std::int64_t sizeBytes = 0; +}; + +struct Provenance { + std::string collectedBy; +}; + +// A nested aggregate whose own member is itself a nested aggregate -- the +// depth-limit case: `provenance`'s own sub-members must stay unannotated. +struct DeepSpecimen { + double massDry = 0.0; + Provenance provenance; +}; + +// Specimen and Attachment are each used from two places below, so glaze +// deduplicates both via a shared `$defs` entry referenced by `$ref`. +struct Record { + std::string operatorName; // flat -- unaffected by this feature + double temperature = 0.0; // flat -- unaffected by this feature + Specimen reference; // single nested aggregate ($ref form) + Specimen secondary; // second use of Specimen -> forces $ref/$defs + Attachment primary; // single nested aggregate ($ref form) + std::vector files; // second use of Attachment -> forces $ref/$defs +}; + +// Specimen used exactly once here -- glaze inlines the object schema +// directly into the "reference" property instead of using $defs/$ref. +struct SingleUseRecord { + Specimen reference; +}; + +// Attachment used exactly once here (only via the vector) -- glaze inlines +// the object schema into the "files" property's "items" instead of $defs/$ref. +struct SingleUseVectorRecord { + std::vector files; +}; + +struct DeepRecord { + DeepSpecimen sample; +}; + +} // namespace nestedforms + +using nestedforms::Attachment; +using nestedforms::DeepRecord; +using nestedforms::DeepSpecimen; +using nestedforms::Provenance; +using nestedforms::Record; +using nestedforms::SingleUseRecord; +using nestedforms::SingleUseVectorRecord; +using nestedforms::Specimen; + +namespace { + +// Resolves the object-schema DOM node for a nested-aggregate member, given +// the property (or array `items`) node glaze wrote for it -- mirroring +// exactly what forms.hpp's `annotateNestedAggregateRef` resolves in +// production: a `$ref` into `$defs` (2+ uses) or the node itself, inlined +// (exactly one use). +const glz::generic_u64& resolveNestedSchema(const glz::generic_u64& dom, const glz::generic_u64& propertyOrItems) { + if (propertyOrItems.contains("$ref")) { + constexpr std::string_view kPrefix = "#/$defs/"; + std::string const ref = propertyOrItems["$ref"].get(); + REQUIRE(ref.starts_with(kPrefix)); + return dom["$defs"][ref.substr(kPrefix.size())]; + } + REQUIRE(propertyOrItems.contains("properties")); + return propertyOrItems; +} + +std::vector requiredNamesOf(const glz::generic_u64& node) { + std::vector out; + for (auto const& entry : node["required"].get()) { + out.push_back(entry.get()); + } + return out; +} + +} // namespace + +// ── Flat top-level fields are unaffected ──────────────────────────────────── + +TEST_CASE("Forms::SchemaJson::NestedAggregate: flat top-level members still render exactly as before", + "[forms][nested]") { + auto const schema = morph::forms::schemaJson(); + REQUIRE_FALSE(schema.empty()); + + glz::generic_u64 dom{}; + REQUIRE_FALSE(glz::read_json(dom, schema)); + + CHECK(dom["properties"]["operatorName"]["x-order"].as() == 0); + CHECK(dom["properties"]["temperature"]["x-order"].as() == 1); + CHECK(dom["properties"]["operatorName"]["title"].get() == "Operator Name"); +} + +// ── Single nested aggregate member, deduplicated ($ref/$defs form) ───────── + +TEST_CASE("Forms::SchemaJson::NestedAggregate: a nested struct member's $defs entry gets annotated ($ref form)", + "[forms][nested][issue25]") { + auto const schema = morph::forms::schemaJson(); + glz::generic_u64 dom{}; + REQUIRE_FALSE(glz::read_json(dom, schema)); + + REQUIRE(dom["properties"].contains("reference")); + auto const& def = resolveNestedSchema(dom, dom["properties"]["reference"]); + REQUIRE(dom["properties"]["reference"].contains("$ref")); // Specimen used twice -> deduplicated + + CHECK(def["properties"]["massDry"]["x-order"].as() == 0); + CHECK(def["properties"]["massWet"]["x-order"].as() == 1); + CHECK(def["properties"]["massDry"]["title"].get() == "Mass Dry"); + CHECK(def["properties"]["massWet"]["title"].get() == "Mass Wet"); + + // required derives the same way one level down: massDry/massWet are + // required, the std::optional note is not. + REQUIRE(def.contains("required")); + auto const requiredNames = requiredNamesOf(def); + CHECK(std::find(requiredNames.begin(), requiredNames.end(), "massDry") != requiredNames.end()); + CHECK(std::find(requiredNames.begin(), requiredNames.end(), "massWet") != requiredNames.end()); + CHECK(std::find(requiredNames.begin(), requiredNames.end(), "note") == requiredNames.end()); +} + +// ── Repeated aggregate (std::vector), deduplicated ($ref/$defs form) ── + +TEST_CASE( + "Forms::SchemaJson::NestedAggregate: a std::vector repeated-aggregate member's items def is annotated " + "($ref form)", + "[forms][nested][issue25]") { + auto const schema = morph::forms::schemaJson(); + glz::generic_u64 dom{}; + REQUIRE_FALSE(glz::read_json(dom, schema)); + + REQUIRE(dom["properties"].contains("files")); + REQUIRE(dom["properties"]["files"].contains("items")); + auto const& itemsNode = dom["properties"]["files"]["items"]; + REQUIRE(itemsNode.contains("$ref")); // Attachment used twice -> deduplicated + auto const& def = resolveNestedSchema(dom, itemsNode); + + CHECK(def["properties"]["filename"]["x-order"].as() == 0); + CHECK(def["properties"]["sizeBytes"]["x-order"].as() == 1); + CHECK(def["properties"]["filename"]["title"].get() == "Filename"); + + REQUIRE(def.contains("required")); + auto const requiredNames = requiredNamesOf(def); + CHECK(std::find(requiredNames.begin(), requiredNames.end(), "filename") != requiredNames.end()); + CHECK(std::find(requiredNames.begin(), requiredNames.end(), "sizeBytes") != requiredNames.end()); +} + +// ── Single nested aggregate member, inlined (used exactly once) ──────────── + +TEST_CASE("Forms::SchemaJson::NestedAggregate: a singly-used nested struct member is annotated in place (inline form)", + "[forms][nested][issue25]") { + auto const schema = morph::forms::schemaJson(); + glz::generic_u64 dom{}; + REQUIRE_FALSE(glz::read_json(dom, schema)); + + REQUIRE(dom["properties"].contains("reference")); + CHECK_FALSE(dom["properties"]["reference"].contains("$ref")); // inlined, not deduplicated + auto const& def = resolveNestedSchema(dom, dom["properties"]["reference"]); + + CHECK(def["properties"]["massDry"]["x-order"].as() == 0); + CHECK(def["properties"]["massDry"]["title"].get() == "Mass Dry"); + REQUIRE(def.contains("required")); + auto const requiredNames = requiredNamesOf(def); + CHECK(std::find(requiredNames.begin(), requiredNames.end(), "massDry") != requiredNames.end()); + CHECK(std::find(requiredNames.begin(), requiredNames.end(), "note") == requiredNames.end()); +} + +// ── Repeated aggregate (std::vector), inlined (used exactly once) ───── + +TEST_CASE( + "Forms::SchemaJson::NestedAggregate: a singly-used std::vector member's items are annotated in place " + "(inline form)", + "[forms][nested][issue25]") { + auto const schema = morph::forms::schemaJson(); + glz::generic_u64 dom{}; + REQUIRE_FALSE(glz::read_json(dom, schema)); + + REQUIRE(dom["properties"].contains("files")); + REQUIRE(dom["properties"]["files"].contains("items")); + auto const& itemsNode = dom["properties"]["files"]["items"]; + CHECK_FALSE(itemsNode.contains("$ref")); // inlined, not deduplicated + auto const& def = resolveNestedSchema(dom, itemsNode); + + CHECK(def["properties"]["filename"]["x-order"].as() == 0); + CHECK(def["properties"]["sizeBytes"]["x-order"].as() == 1); + REQUIRE(def.contains("required")); +} + +// ── Depth limit: exactly one level ────────────────────────────────────────── + +TEST_CASE("Forms::SchemaJson::NestedAggregate: recursion stops after one level (depth cap)", + "[forms][nested][issue25]") { + auto const schema = morph::forms::schemaJson(); + glz::generic_u64 dom{}; + REQUIRE_FALSE(glz::read_json(dom, schema)); + + REQUIRE(dom["properties"].contains("sample")); + auto const& outerDef = resolveNestedSchema(dom, dom["properties"]["sample"]); + + // Level 1 (DeepSpecimen's own members) IS annotated. + CHECK(outerDef["properties"]["massDry"].contains("x-order")); + CHECK(outerDef["properties"]["massDry"].contains("title")); + REQUIRE(outerDef.contains("required")); + + // Level 2 (Provenance, nested inside DeepSpecimen) is NOT annotated: its + // object schema has no "required" key and its own properties carry no + // x-order/title -- exactly today's pre-existing behaviour for anything + // deeper than one level. "provenance" itself (a level-1 member) DOES get + // an x-order/title on its own property node, same as any other member; + // only its *inner* properties are left untouched. + REQUIRE(outerDef["properties"].contains("provenance")); + CHECK(outerDef["properties"]["provenance"].contains("x-order")); + CHECK(outerDef["properties"]["provenance"].contains("title")); + auto const& innerDef = resolveNestedSchema(dom, outerDef["properties"]["provenance"]); + CHECK_FALSE(innerDef.contains("required")); + CHECK_FALSE(innerDef["properties"]["collectedBy"].contains("x-order")); + CHECK_FALSE(innerDef["properties"]["collectedBy"].contains("title")); +} + +// ── Idempotence: two members sharing the same nested type ────────────────── + +TEST_CASE("Forms::SchemaJson::NestedAggregate: re-annotating a shared $defs entry is harmless", "[forms][nested]") { + // Record reuses Specimen across "reference" and "secondary" (and + // Attachment across "primary" and "files"): the annotation pass runs once + // per property that resolves to a given def, so this proves multiple + // triggers into the same def produce one consistent, non-corrupted result. + auto const schema = morph::forms::schemaJson(); + glz::generic_u64 dom{}; + REQUIRE_FALSE(glz::read_json(dom, schema)); + + std::string const refA = dom["properties"]["reference"]["$ref"].get(); + std::string const refB = dom["properties"]["secondary"]["$ref"].get(); + CHECK(refA == refB); // same underlying type -> same $defs entry + + auto const& def = resolveNestedSchema(dom, dom["properties"]["reference"]); + CHECK(def["properties"]["massDry"]["x-order"].as() == 0); + REQUIRE(def.contains("required")); +} From 2bd39b8121562b4941df3a2cdf99ee51a691e76b Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 3 Aug 2026 22:04:50 +0300 Subject: [PATCH 02/13] test(forms): cover FieldMeta/Quantity/Choice annotations one level down The nested-aggregate annotation pass (issue #25) shares annotateBasicMemberProperty with the top-level pass, so it applies the same FieldMeta/Quantity/Choice/widget/ranged-bounds rules to a nested member's own properties -- but every existing nested-forms fixture (Specimen, Attachment, Provenance) only has plain scalar members, so those branches were never exercised through the nested path. Add RichSub/RichRecord, a nested type whose own members carry a FieldMeta override, a Quantity, and a Choice, and assert the resolved nested schema carries the expected x-placeholder/ x-readonly/x-hidden/x-decimalPlaces/x-optionsAction annotations. Signed-off-by: Yaraslau Tamashevich --- tests/test_nested_forms.cpp | 96 +++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/tests/test_nested_forms.cpp b/tests/test_nested_forms.cpp index 1a98e690..c87ccf5a 100644 --- a/tests/test_nested_forms.cpp +++ b/tests/test_nested_forms.cpp @@ -13,14 +13,37 @@ // docs/spec/forms/forms.md, "Nested aggregates (one level)"). #include +#include #include #include #include +#include #include +#include #include #include #include +// A minimal application unit system, purely so a nested-aggregate member can +// carry a Quantity field (see RichSub below) -- exercises the +// `annotateBasicMemberProperty` Quantity branch one level down, which none of +// the plain-scalar nested types above (Specimen/Attachment/Provenance) +// touch. No `relations`: unitAlternatives() is empty either way, and that is +// not what this file is testing. +enum class NestedFormUnit : std::uint8_t { scalar, kg }; + +template <> +struct morph::units::UnitTraits { + static constexpr morph::units::UnitMeta meta(NestedFormUnit unit) noexcept { + switch (unit) { + case NestedFormUnit::kg: + return {.id = "kg", .display = "kg", .defaultDecimals = 3}; + default: + return {.id = "scalar", .display = "", .defaultDecimals = 3}; + } + } +}; + // Named namespace (not anonymous): glaze reflection requires the reflected // type to have linkage (see test_bridge_fixes.cpp for the same note). namespace nestedforms { @@ -74,6 +97,32 @@ struct DeepRecord { DeepSpecimen sample; }; +// A nested aggregate whose own members carry the same annotation-worthy +// shapes the top-level pass already covers elsewhere (FieldMeta, Quantity, +// Choice): proves `annotateBasicMemberProperty` applies those rules one level +// down too, not just title/x-order/required (the only things Specimen/ +// Attachment/Provenance above exercise). +struct RichSub { + std::int64_t code = 0; + morph::units::Quantity mass{}; + morph::forms::Choice option; + + static constexpr std::array fieldMetadata{ + morph::forms::FieldMeta{.field = "code", + .label = "Custom Code", + .help = "Help text", + .placeholder = "e.g. 42", + .readOnly = true, + .hidden = true}, + }; +}; + +// RichSub used exactly once -- inlined, not deduplicated via $defs/$ref (see +// the "inline form" tests above for why that matters to resolution). +struct RichRecord { + RichSub rich; +}; + } // namespace nestedforms using nestedforms::Attachment; @@ -81,6 +130,7 @@ using nestedforms::DeepRecord; using nestedforms::DeepSpecimen; using nestedforms::Provenance; using nestedforms::Record; +using nestedforms::RichRecord; using nestedforms::SingleUseRecord; using nestedforms::SingleUseVectorRecord; using nestedforms::Specimen; @@ -252,6 +302,52 @@ TEST_CASE("Forms::SchemaJson::NestedAggregate: recursion stops after one level ( CHECK_FALSE(innerDef["properties"]["collectedBy"].contains("title")); } +// ── FieldMeta/Quantity/Choice rules apply one level down too ──────────────── + +TEST_CASE( + "Forms::SchemaJson::NestedAggregate: FieldMeta/Quantity/Choice annotations apply to a nested member's own " + "properties", + "[forms][nested][issue25]") { + auto const schema = morph::forms::schemaJson(); + glz::generic_u64 dom{}; + REQUIRE_FALSE(glz::read_json(dom, schema)); + + REQUIRE(dom["properties"].contains("rich")); + CHECK_FALSE(dom["properties"]["rich"].contains("$ref")); // RichSub used exactly once -> inlined + auto const& def = resolveNestedSchema(dom, dom["properties"]["rich"]); + + // FieldMeta: label/help/placeholder/readOnly/hidden, from RichSub's own + // fieldMetadata (looked up against RichSub, the Owner one level down — + // not against RichRecord). + auto const& codeProp = def["properties"]["code"]; + CHECK(codeProp["title"].get() == "Custom Code"); + CHECK(codeProp["description"].get() == "Help text"); + CHECK(codeProp["x-placeholder"].get() == "e.g. 42"); + CHECK(codeProp["x-readonly"].get() == true); + CHECK(codeProp["x-hidden"].get() == true); + // code has no FieldMeta-driven readOnly/hidden peers to compare against in + // this fixture, so also confirm a field the fieldMetadata list does not + // name -- mass -- carries none of these keys. + auto const& massProp = def["properties"]["mass"]; + CHECK_FALSE(massProp.contains("x-readonly")); + CHECK_FALSE(massProp.contains("x-hidden")); + CHECK_FALSE(massProp.contains("x-placeholder")); + + // Quantity: x-decimalPlaces from the unit's declared decimals. + CHECK(massProp["x-decimalPlaces"].as() == 3); + + // Choice: x-optionsAction/x-optionValue/x-optionLabel. + auto const& optionProp = def["properties"]["option"]; + CHECK(optionProp["x-optionsAction"].get() == "NestedFormListOptions"); + CHECK(optionProp.contains("x-optionValue")); + CHECK(optionProp.contains("x-optionLabel")); + + // required still derives correctly one level down alongside all of the above. + REQUIRE(def.contains("required")); + auto const requiredNames = requiredNamesOf(def); + CHECK(std::find(requiredNames.begin(), requiredNames.end(), "code") != requiredNames.end()); +} + // ── Idempotence: two members sharing the same nested type ────────────────── TEST_CASE("Forms::SchemaJson::NestedAggregate: re-annotating a shared $defs entry is harmless", "[forms][nested]") { From 547ad60acd56947c0f67db7f505206699b469e9b Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Tue, 4 Aug 2026 19:56:13 +0300 Subject: [PATCH 03/13] tests(forms): fix -Wswitch-enum and close nested-annotation coverage gaps Re-applies two fixes that were accidentally dropped by a stale-branch rebase (the local branch used for the rebase predated these commits on origin, so `git rebase origin/master` + force-push clobbered them). 1. UnitTraits::meta()'s switch had only a `kg` case plus `default`, tripping -Wswitch-enum (part of -Weverything, and not in apply_warnings()'s opt-out list) since it requires every enumerator named regardless of a default label. Add an explicit `case NestedFormUnit::scalar:` falling through to the existing default body. 2. codecov/patch still failed (79.41%, target 97.11%) after fix #1 because three annotation branches in the nested-aggregate path this PR adds -- FieldMeta::i18nKey, FieldMeta::widget, and Quantity::unitAlternatives() -- were structurally unreachable by the RichSub/RichRecord fixture. Declare a g<->kg unit relation and set i18nKey/widget on the fixture, and assert on x-i18nKey/x-widget/x-unitAlternatives at the nested level. Verified locally with llvm-cov: all three branches now execute at least once; full suite (826 cases) still passes. Signed-off-by: Yaraslau Tamashevich --- tests/test_nested_forms.cpp | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/tests/test_nested_forms.cpp b/tests/test_nested_forms.cpp index c87ccf5a..2adc4c4f 100644 --- a/tests/test_nested_forms.cpp +++ b/tests/test_nested_forms.cpp @@ -20,17 +20,24 @@ #include #include #include +#include #include #include #include +using morph::math::DecimalPlaces; +using morph::math::Denominator; +using morph::math::Numerator; +using morph::math::Rational; + // A minimal application unit system, purely so a nested-aggregate member can // carry a Quantity field (see RichSub below) -- exercises the // `annotateBasicMemberProperty` Quantity branch one level down, which none of // the plain-scalar nested types above (Specimen/Attachment/Provenance) -// touch. No `relations`: unitAlternatives() is empty either way, and that is -// not what this file is testing. -enum class NestedFormUnit : std::uint8_t { scalar, kg }; +// touch. `relations` declares a g<->kg conversion so RichSub's `mass` field +// also exercises the `unitAlternatives()`-non-empty branch one level down +// (see the "FieldMeta/Quantity/Choice" test case below). +enum class NestedFormUnit : std::uint8_t { scalar, kg, g }; template <> struct morph::units::UnitTraits { @@ -38,10 +45,16 @@ struct morph::units::UnitTraits { switch (unit) { case NestedFormUnit::kg: return {.id = "kg", .display = "kg", .defaultDecimals = 3}; + case NestedFormUnit::g: + return {.id = "g", .display = "g", .defaultDecimals = 1}; + case NestedFormUnit::scalar: default: return {.id = "scalar", .display = "", .defaultDecimals = 3}; } } + + static constexpr std::array, 1> relations{ + {{NestedFormUnit::g, NestedFormUnit::kg, Rational{Numerator{1}, Denominator{1000}, DecimalPlaces{3}}}}}; }; // Named namespace (not anonymous): glaze reflection requires the reflected @@ -112,6 +125,8 @@ struct RichSub { .label = "Custom Code", .help = "Help text", .placeholder = "e.g. 42", + .i18nKey = "custom.code", + .widget = "custom-widget", .readOnly = true, .hidden = true}, }; @@ -325,6 +340,8 @@ TEST_CASE( CHECK(codeProp["x-placeholder"].get() == "e.g. 42"); CHECK(codeProp["x-readonly"].get() == true); CHECK(codeProp["x-hidden"].get() == true); + CHECK(codeProp["x-i18nKey"].get() == "custom.code"); + CHECK(codeProp["x-widget"].get() == "custom-widget"); // code has no FieldMeta-driven readOnly/hidden peers to compare against in // this fixture, so also confirm a field the fieldMetadata list does not // name -- mass -- carries none of these keys. @@ -333,8 +350,15 @@ TEST_CASE( CHECK_FALSE(massProp.contains("x-hidden")); CHECK_FALSE(massProp.contains("x-placeholder")); - // Quantity: x-decimalPlaces from the unit's declared decimals. + // Quantity: x-decimalPlaces from the unit's declared decimals, plus + // x-unitAlternatives from the g<->kg relation declared above. CHECK(massProp["x-decimalPlaces"].as() == 3); + REQUIRE(massProp.contains("x-unitAlternatives")); + auto const& alternatives = massProp["x-unitAlternatives"].get(); + REQUIRE(alternatives.size() == 1); + CHECK(alternatives[0]["id"].get() == "g"); + CHECK(alternatives[0]["num"].as() == 1); + CHECK(alternatives[0]["den"].as() == 1000); // Choice: x-optionsAction/x-optionValue/x-optionLabel. auto const& optionProp = def["properties"]["option"]; From bc5b387a18a0147088d6435536562cb290b36990 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Wed, 5 Aug 2026 07:44:43 +0300 Subject: [PATCH 04/13] test(forms): close remaining nested-aggregate branch coverage gaps Six new cases: a FieldMeta entry with no optional attributes set, a nested Quantity member with no unit alternatives, optionalFields applied one level down, and annotateNestedAggregateRef's three defensive fallbacks (non-string $ref, $ref outside #/$defs/, neither $ref nor properties) via direct detail:: calls, since glaze itself never produces those malformed shapes through the public schemaJson() path. Co-Authored-By: Claude Sonnet 5 --- tests/test_nested_forms.cpp | 149 ++++++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) diff --git a/tests/test_nested_forms.cpp b/tests/test_nested_forms.cpp index 2adc4c4f..caefd83d 100644 --- a/tests/test_nested_forms.cpp +++ b/tests/test_nested_forms.cpp @@ -138,11 +138,70 @@ struct RichRecord { RichSub rich; }; +// A nested aggregate whose FieldMeta entry sets only .field/.label -- the +// mirror of RichSub's "code" above: fieldMeta is found, but every optional +// attribute (help/placeholder/readOnly/hidden/i18nKey) is left at its default, +// so each of their "found but set" branches must resolve false one level down. +struct PlainMetaSub { + std::int64_t code = 0; + + static constexpr std::array fieldMetadata{ + morph::forms::FieldMeta{.field = "code", .label = "Plain Code"}, + }; +}; + +struct PlainMetaRecord { + PlainMetaSub plain; +}; + +// A nested aggregate declaring a non-`std::optional`-typed member optional via +// `optionalFields` -- exercises `declaredOptional` one level down +// (Specimen/DeepSpecimen above only ever exercise the std::optional path). +struct DeclaredOptionalSub { + std::string label; + + static constexpr std::array optionalFields{"label"}; +}; + +struct DeclaredOptionalRecord { + DeclaredOptionalSub sub; +}; + +} // namespace nestedforms + +// A unit with no declared relations, so `unitAlternatives()` is empty -- +// exercises the "no unit alternatives" branch of `annotateBasicMemberProperty` +// one level down (RichSub's `mass` above only ever exercises the non-empty +// case). +enum class BareFormUnit : std::uint8_t { scalar }; + +template <> +struct morph::units::UnitTraits { + static constexpr morph::units::UnitMeta meta(BareFormUnit) noexcept { + return {.id = "scalar", .display = "", .defaultDecimals = 2}; + } + + static constexpr std::array, 0> relations{}; +}; + +namespace nestedforms { + +struct BareQuantitySub { + morph::units::Quantity amount{}; +}; + +struct BareQuantityRecord { + BareQuantitySub bare; +}; + } // namespace nestedforms using nestedforms::Attachment; +using nestedforms::BareQuantityRecord; +using nestedforms::DeclaredOptionalRecord; using nestedforms::DeepRecord; using nestedforms::DeepSpecimen; +using nestedforms::PlainMetaRecord; using nestedforms::Provenance; using nestedforms::Record; using nestedforms::RichRecord; @@ -391,3 +450,93 @@ TEST_CASE("Forms::SchemaJson::NestedAggregate: re-annotating a shared $defs entr CHECK(def["properties"]["massDry"]["x-order"].as() == 0); REQUIRE(def.contains("required")); } + +// ── FieldMeta found but no optional attributes set ────────────────────────── + +TEST_CASE( + "Forms::SchemaJson::NestedAggregate: a FieldMeta entry with no optional attributes leaves them all unset", + "[forms][nested][issue25]") { + auto const schema = morph::forms::schemaJson(); + glz::generic_u64 dom{}; + REQUIRE_FALSE(glz::read_json(dom, schema)); + + REQUIRE(dom["properties"].contains("plain")); + auto const& def = resolveNestedSchema(dom, dom["properties"]["plain"]); + auto const& codeProp = def["properties"]["code"]; + CHECK(codeProp["title"].get() == "Plain Code"); + CHECK_FALSE(codeProp.contains("description")); + CHECK_FALSE(codeProp.contains("x-placeholder")); + CHECK_FALSE(codeProp.contains("x-readonly")); + CHECK_FALSE(codeProp.contains("x-hidden")); + CHECK_FALSE(codeProp.contains("x-i18nKey")); +} + +// ── Quantity member with no unit alternatives ─────────────────────────────── + +TEST_CASE( + "Forms::SchemaJson::NestedAggregate: a nested Quantity member with no unit alternatives omits " + "x-unitAlternatives", + "[forms][nested][issue25]") { + auto const schema = morph::forms::schemaJson(); + glz::generic_u64 dom{}; + REQUIRE_FALSE(glz::read_json(dom, schema)); + + REQUIRE(dom["properties"].contains("bare")); + auto const& def = resolveNestedSchema(dom, dom["properties"]["bare"]); + auto const& amountProp = def["properties"]["amount"]; + CHECK(amountProp.contains("x-decimalPlaces")); + CHECK_FALSE(amountProp.contains("x-unitAlternatives")); +} + +// ── declaredOptional applies one level down too ───────────────────────────── + +TEST_CASE( + "Forms::SchemaJson::NestedAggregate: optionalFields marks a non-std::optional nested member as not required", + "[forms][nested][issue25]") { + auto const schema = morph::forms::schemaJson(); + glz::generic_u64 dom{}; + REQUIRE_FALSE(glz::read_json(dom, schema)); + + REQUIRE(dom["properties"].contains("sub")); + auto const& def = resolveNestedSchema(dom, dom["properties"]["sub"]); + REQUIRE(def.contains("required")); + auto const requiredNames = requiredNamesOf(def); + CHECK(std::find(requiredNames.begin(), requiredNames.end(), "label") == requiredNames.end()); +} + +// ── annotateNestedAggregateRef's defensive fallbacks (issue #25) ─────────── +// +// These call the detail function directly with hand-built DOM fragments, +// rather than through schemaJson(), because glaze itself never actually +// produces the malformed shapes these branches guard against -- see the +// function's own doc comment ("left untouched rather than guessed at"). + +TEST_CASE("Forms::SchemaJson::NestedAggregate: annotateNestedAggregateRef leaves a non-string $ref untouched", + "[forms][nested][issue25]") { + glz::generic_u64 dom{}; + glz::generic_u64 property{}; + property["$ref"] = std::uint64_t{42}; // malformed: $ref present but not a string + morph::forms::detail::annotateNestedAggregateRef(dom, property); + CHECK_FALSE(property.contains("required")); +} + +TEST_CASE("Forms::SchemaJson::NestedAggregate: annotateNestedAggregateRef leaves a $ref outside #/$defs/ untouched", + "[forms][nested][issue25]") { + glz::generic_u64 dom{}; + glz::generic_u64 property{}; + property["$ref"] = std::string{"#/other/Specimen"}; + morph::forms::detail::annotateNestedAggregateRef(dom, property); + CHECK_FALSE(dom.contains("$defs")); +} + +TEST_CASE( + "Forms::SchemaJson::NestedAggregate: annotateNestedAggregateRef leaves a schema with neither $ref nor " + "properties untouched", + "[forms][nested][issue25]") { + glz::generic_u64 dom{}; + glz::generic_u64 property{}; + property["type"] = std::string{"string"}; // glaze emitted something other than an object schema + morph::forms::detail::annotateNestedAggregateRef(dom, property); + CHECK_FALSE(property.contains("required")); + CHECK(property["type"].get() == "string"); +} From eda3c512cb0d446b4b51d1fee7f2261e3ab63ec9 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Wed, 5 Aug 2026 11:29:05 +0300 Subject: [PATCH 05/13] forms: spec unbounded-depth nested-aggregate recursion Replaces the "one level" cap documented for mergeSchemaExtras with cycle-guarded recursion to arbitrary depth: a nested-aggregate member's own nested-aggregate members are now annotated too, however deep the type graph goes, stopping only (via a static_assert) when a member's type would repeat a type already on the current ancestor chain. Signed-off-by: Yaraslau Tamashevich --- docs/spec/forms/forms.md | 69 +++++++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/docs/spec/forms/forms.md b/docs/spec/forms/forms.md index 19daa5a5..9e3e9ec9 100644 --- a/docs/spec/forms/forms.md +++ b/docs/spec/forms/forms.md @@ -909,8 +909,8 @@ the `required`-array derivation in `mergeSchemaExtras`, which checks `isStdOptional` **explicitly** — see [Required-ness rule](#required-ness-rule).) The predicate is `noexcept` and `constexpr`, and it inspects only the action's **own top-level members**; unlike `schemaJson()`'s schema generation (see -[Nested aggregates (one level)](#nested-aggregates-one-level)), it does **not** -recurse into a nested aggregate member's own fields. +[Nested aggregates (recursive, cycle-guarded)](#nested-aggregates-recursive-cycle-guarded)), +it does **not** recurse into a nested aggregate member's own fields. ## Cross-field rules — the `x-rules` vocabulary @@ -1257,16 +1257,19 @@ for the exhaustive tables and design rationale. ## Failure modes -### Nested aggregates (one level) +### Nested aggregates (recursive, cycle-guarded) A member whose type is itself a reflectable aggregate — a plain nested struct, or `std::vector` (a repeated aggregate) — gets its **own** -members annotated too, one level down: `x-order`, `title`/`FieldMeta`, -`required`, and the `Quantity`/`Choice`/widget/ranged-bounds rules the top -level already applies. This closes the gap a flat-only generator has for -domains that are naturally nested (a measurement with a repeated specimen -sub-record, a document with a nested address) — previously such a member's -sub-fields were silently unannotated and absent from the generated form. +members annotated too: `x-order`, `title`/`FieldMeta`, `required`, and the +`Quantity`/`Choice`/widget/ranged-bounds rules the top level already applies. +Unlike the top level, this recurses to **whatever depth the type graph +actually has** — a nested aggregate's own nested-aggregate member is +annotated in turn, and so on — rather than stopping after one level. This +closes the gap a flat-only generator has for domains that are naturally +nested (a measurement with a repeated specimen sub-record, a document with a +nested address, a category tree), including domains nested more than one +level deep (an address with a nested geo-coordinate sub-record, say). Two schema shapes exist for a nested aggregate, and both are recursed into: @@ -1281,17 +1284,33 @@ Two schema shapes exist for a nested aggregate, and both are recursed into: `mergeSchemaExtras` resolves whichever form applies (`annotateNestedAggregateRef`, `forms.hpp`) and hands the resolved node to the same per-member annotation logic the top level uses (`annotateBasicMemberProperty`), applied against the -nested type's own reflection. - -**Depth is capped at exactly one level.** If the nested type itself has a -member that is itself an aggregate, that member's own sub-members are left -exactly as glaze emitted them — unannotated, matching this generator's -original flat-only behaviour for anything past one level. Computed fields, -`formLayout`/`fieldSpans`, and `formRules` also remain **top-level only**: a -nested aggregate declaring any of those has no effect on the generated -schema. Both limits keep generated forms comprehensible and keep the -generator itself simple, rather than becoming a general recursive-descent -schema compiler. +nested type's own reflection. Each recursive step passes along the chain of +nested-aggregate types already being annotated on the current path — starting +with the action type `A` itself — as a variadic template parameter pack, so a +deeper call can tell whether it is about to revisit a type already on that +path. + +**Cyclic nested aggregates are a compile error, not infinite recursion.** A +member whose type — or, for `std::vector`, `Sub` — equals the action +type or any nested-aggregate type already on the current ancestor chain +(a self-referential type such as `struct Node { std::vector +children; };`, or a mutual reference between two distinct types) trips a +`static_assert` at the point that specific recursive instantiation would +occur, instead of recursing forever. This only rejects genuine cycles: a +"diamond" — the same type reused from two unrelated places in the schema, +e.g. an `Address` nested under both a `Company` and a `Person` member of the +same action — is not a cycle (neither `Address` nor any of its members is its +own ancestor) and recurses normally into both. Restructure a domain type +that trips this (e.g. flatten the self-reference, or represent the recursive +edge as an opaque id instead of a nested value) if you need one; there is no +runtime opt-out. + +Computed fields, `formLayout`/`fieldSpans`, and `formRules` remain **top-level +only** regardless of nesting depth: a nested aggregate declaring any of those +has no effect on the generated schema. This keeps the generator focused on +what a nested-aggregate schema actually needs (per-field annotations) rather +than becoming a general recursive-descent schema compiler that also +re-derives layout/rules/computed-field semantics at every level. **Purely additive.** An action with no nested-aggregate member has nothing here to trigger on, so its generated schema is byte-for-byte unchanged. A @@ -1299,17 +1318,17 @@ pre-existing action that *does* have a nested-aggregate member sees its schema gain annotations it previously lacked — the whole point of this feature — with no change to any of its flat top-level members. -The nested type must be **default-constructible**, exactly like the -top-level action type (see below): the recursion builds its own probe -instance purely to enumerate its members via reflection. +Every nested-aggregate type in the chain must be **default-constructible**, +exactly like the top-level action type (see below): the recursion builds its +own probe instance purely to enumerate its members via reflection. ### Scope: flat actions only (form layout, computed fields, and rules) `formLayout`/`fieldSpans` ([Layout & grouping](#layout--grouping)) and `formRules` ([Cross-field rules](#cross-field-rules-x-rules)) are read only from the top-level action type — they are not consulted on a nested -aggregate, even at the one level `mergeSchemaExtras` does otherwise recurse -into (see [Nested aggregates (one level)](#nested-aggregates-one-level) +aggregate, no matter how deep `mergeSchemaExtras` otherwise recurses (see +[Nested aggregates (recursive, cycle-guarded)](#nested-aggregates-recursive-cycle-guarded) above). Computed fields (`computedFields`) are likewise top-level only. The action type must also be **default-constructible**: `mergeSchemaExtras` From 0e31b235ecb0a8f6f0fc8eab074d9c6458307fcb Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Wed, 5 Aug 2026 11:47:23 +0300 Subject: [PATCH 06/13] forms: tighten nested-aggregate recursion spec wording Three review passes: fixed a terminology wobble (the ancestor chain's first element is the action type, not itself a "nested-aggregate type"), tightened the cycle-detection paragraph's phrasing, and closed an ambiguity about when the static_assert actually fires (only when the offending type is nested under an action's schema, not for the type standing alone). Signed-off-by: Yaraslau Tamashevich --- docs/spec/forms/forms.md | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/docs/spec/forms/forms.md b/docs/spec/forms/forms.md index 9e3e9ec9..85b95e58 100644 --- a/docs/spec/forms/forms.md +++ b/docs/spec/forms/forms.md @@ -1284,17 +1284,18 @@ Two schema shapes exist for a nested aggregate, and both are recursed into: `mergeSchemaExtras` resolves whichever form applies (`annotateNestedAggregateRef`, `forms.hpp`) and hands the resolved node to the same per-member annotation logic the top level uses (`annotateBasicMemberProperty`), applied against the -nested type's own reflection. Each recursive step passes along the chain of -nested-aggregate types already being annotated on the current path — starting -with the action type `A` itself — as a variadic template parameter pack, so a -deeper call can tell whether it is about to revisit a type already on that -path. +nested type's own reflection. Each recursive step passes along the **ancestor +chain** — the action type `A`, followed by every nested-aggregate type +visited since, in order, ending with the type currently being annotated — as +a variadic template parameter pack, so a deeper call can tell whether a +member's type is already somewhere on that chain. **Cyclic nested aggregates are a compile error, not infinite recursion.** A -member whose type — or, for `std::vector`, `Sub` — equals the action -type or any nested-aggregate type already on the current ancestor chain -(a self-referential type such as `struct Node { std::vector -children; };`, or a mutual reference between two distinct types) trips a +member whose type (or, for a `std::vector` member, `Sub` itself) matches +any type already on the ancestor chain — the action type, the +nested-aggregate type currently being annotated, or anything annotated in +between (a self-referential type such as `struct Node { std::vector +children; };`, or a mutual reference between two distinct types) — trips a `static_assert` at the point that specific recursive instantiation would occur, instead of recursing forever. This only rejects genuine cycles: a "diamond" — the same type reused from two unrelated places in the schema, @@ -1303,7 +1304,10 @@ same action — is not a cycle (neither `Address` nor any of its members is its own ancestor) and recurses normally into both. Restructure a domain type that trips this (e.g. flatten the self-reference, or represent the recursive edge as an opaque id instead of a nested value) if you need one; there is no -runtime opt-out. +runtime opt-out. The `static_assert` only fires where the offending type is +actually reached as a nested-aggregate member of some `schemaJson()` (or +`mergeSchemaExtras()`) instantiation — a self-referential type that is +never nested under an action this way compiles and works fine on its own. Computed fields, `formLayout`/`fieldSpans`, and `formRules` remain **top-level only** regardless of nesting depth: a nested aggregate declaring any of those From df8970e1de9fb5bbdc25db19a89b3a42785b581b Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Wed, 5 Aug 2026 11:56:11 +0300 Subject: [PATCH 07/13] plan: implementation plan for unbounded-depth nested-aggregate recursion Three tasks: extend test fixtures with a failing three-level-deep test (TDD red), implement the Ancestors-threaded cycle-guarded recursion in forms.hpp (TDD green), then add a standalone self-referential-type test plus a Doxygen docs-build verification pass. Signed-off-by: Yaraslau Tamashevich --- ...-08-05-nested-aggregate-recursion-depth.md | 825 ++++++++++++++++++ 1 file changed, 825 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-05-nested-aggregate-recursion-depth.md diff --git a/docs/superpowers/plans/2026-08-05-nested-aggregate-recursion-depth.md b/docs/superpowers/plans/2026-08-05-nested-aggregate-recursion-depth.md new file mode 100644 index 00000000..f4eb6483 --- /dev/null +++ b/docs/superpowers/plans/2026-08-05-nested-aggregate-recursion-depth.md @@ -0,0 +1,825 @@ +# Nested-Aggregate Recursion Depth Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace `mergeSchemaExtras`'s one-level cap on nested-aggregate schema +annotation (`include/morph/forms/forms.hpp`) with cycle-guarded recursion to +whatever depth the type graph actually has. + +**Architecture:** Thread a variadic `Ancestors...` template parameter pack +(the chain of nested-aggregate types already being annotated on the current +path, starting with the action type) through `annotateNestedAggregate` and +`annotateNestedAggregateRef`. Factor the "is this member itself a nested +aggregate, and should I recurse into it" decision — previously duplicated +inline in `mergeSchemaExtras`'s loop and absent from `annotateNestedAggregate` +entirely (since it never went past one level) — into one new shared function, +`recurseIntoNestedAggregateIfAny`, used by both loops. That function's cycle +guard is a `static_assert` whose condition depends on the member's type and +the ancestor chain, so it only fires for the specific cyclic instantiation +that would otherwise recurse forever. + +**Tech Stack:** C++23, Glaze (JSON reflection/schema), Catch2 (tests), CMake + +Ninja, `clang-release` preset. + +## Global Constraints + +- Spec lives at `docs/spec/forms/forms.md`, section "Nested aggregates + (recursive, cycle-guarded)" (already written and committed — read it before + starting; this plan implements it verbatim). If any step here turns out to + conflict with that spec, the spec wins — update this plan's approach, not + the spec. +- Computed fields, `formLayout`/`fieldSpans`, and `formRules` stay + **top-level-only** regardless of nesting depth — out of scope for this + change, do not touch that logic. +- Doxygen's `WARN_AS_ERROR` docs build fails on any undocumented public + `@param`/`@tparam`/`@return` — and this codebase already fully documents + even `morph::forms::detail`-namespace functions (see the existing + `annotateNestedAggregate`/`annotateNestedAggregateRef` comments being + replaced below), so every new/changed function needs complete Doxygen + comments too. Reproduce the docs build locally with: + `cmake -S . -B build -G Ninja -DMORPH_BUILD_DOCUMENTATION=ON -DMORPH_BUILD_TESTS=OFF -DMORPH_BUILD_EXAMPLES=OFF` + then `cmake --build build --target doc`. +- Build/test commands (see `README.md`, "Building & dependencies"): + `cmake --preset clang-release`, then + `cmake --build build/clang-release --target morph_tests`, then + `./build/clang-release/tests/morph_tests`. `VCPKG_ROOT` must be set in the + environment (it already is: `/Users/yaraslau/.local/share/vcpkg`). +- Filter to just this file's tests with Catch2's tag filter: + `./build/clang-release/tests/morph_tests "[forms][nested]"`. +- Work happens in the existing worktree at + `/Users/yaraslau/repo/morph/.claude/worktrees/agent-a504219143d710713` + (branch `feature/25-nested-aggregate-forms`, already rebased onto latest + `origin/master`). Every `git`/`cmake`/build command in this plan assumes + that directory is the working directory. + +--- + +### Task 1: Extend the test fixtures and add a failing "recursion continues past one level" test + +**Files:** +- Modify: `tests/test_nested_forms.cpp:1-13` (file header comment) +- Modify: `tests/test_nested_forms.cpp:75-84` (`Provenance`/`DeepSpecimen` fixtures) +- Modify: `tests/test_nested_forms.cpp:348-377` (replace the "depth cap" test) + +**Interfaces:** +- Consumes: `morph::forms::schemaJson()` (existing public API, unchanged + signature), the file's existing `resolveNestedSchema`/`requiredNamesOf` + helpers (unchanged). +- Produces: nothing new consumed by later tasks — this task only changes test + code. Task 2 makes the test added here pass. + +- [ ] **Step 1: Update the file header comment** + +Replace: + +```cpp +// SPDX-License-Identifier: Apache-2.0 +// +// Coverage for issue #25: form generation recurses one level into a +// nested-aggregate member's object schema -- a directly-nested struct member +// or a `std::vector` repeated aggregate -- applying the same +// title/x-order/required/widget rules the top level already applies, instead +// of leaving it entirely unannotated. Two distinct schema shapes exist for a +// nested aggregate (see forms.hpp's `annotateNestedAggregateRef`): glaze +// *inlines* the object schema directly into the property when the nested +// type is used exactly once in the whole schema, and *deduplicates* it via a +// shared `$defs` entry (referenced by `$ref`) when it is used two or more +// times. Both are exercised below. Recursion stops after one level (see +// docs/spec/forms/forms.md, "Nested aggregates (one level)"). +``` + +With: + +```cpp +// SPDX-License-Identifier: Apache-2.0 +// +// Coverage for issue #25: form generation recurses into a nested-aggregate +// member's object schema -- a directly-nested struct member or a +// `std::vector` repeated aggregate -- applying the same +// title/x-order/required/widget rules the top level already applies, instead +// of leaving it entirely unannotated. Two distinct schema shapes exist for a +// nested aggregate (see forms.hpp's `annotateNestedAggregateRef`): glaze +// *inlines* the object schema directly into the property when the nested +// type is used exactly once in the whole schema, and *deduplicates* it via a +// shared `$defs` entry (referenced by `$ref`) when it is used two or more +// times. Both are exercised below. Recursion continues to whatever depth the +// type graph actually has, stopping only at a genuine cycle -- a compile-time +// `static_assert`, not something this runtime test suite can exercise +// directly (see docs/spec/forms/forms.md, "Nested aggregates (recursive, +// cycle-guarded)"). +``` + +- [ ] **Step 2: Extend the fixtures to a three-level chain and add a self-referential standalone type** + +Replace: + +```cpp +struct Provenance { + std::string collectedBy; +}; + +// A nested aggregate whose own member is itself a nested aggregate -- the +// depth-limit case: `provenance`'s own sub-members must stay unannotated. +struct DeepSpecimen { + double massDry = 0.0; + Provenance provenance; +}; +``` + +With: + +```cpp +struct Origin { + std::string country; +}; + +// Three levels deep: DeepSpecimen -> Provenance -> Origin. Provenance's own +// member (origin) is itself a nested aggregate too -- proving recursion +// continues past one level. +struct Provenance { + std::string collectedBy; + Origin origin; +}; + +struct DeepSpecimen { + double massDry = 0.0; + Provenance provenance; +}; + +// A self-referential nested-aggregate type (a tree node). Never passed to +// morph::forms::schemaJson() anywhere in this file -- neither as the +// top-level action type itself nor nested inside another action's member -- +// either use would trip forms.hpp's cycle-guard static_assert (see +// docs/spec/forms/forms.md, "Nested aggregates (recursive, cycle-guarded)"). +// This only proves the type itself, and ordinary glaze JSON round-tripping +// over it, are completely unaffected by that guard. +struct TreeNode { + std::string name; + std::vector children; +}; +``` + +- [ ] **Step 3: Add the `using` declarations for the new types** + +In the `using nestedforms::...;` block (currently lines 199-210), add two +lines, keeping the existing alphabetical order: + +```cpp +using nestedforms::Origin; +``` + +(insert alphabetically between `using nestedforms::DeepSpecimen;` and +`using nestedforms::PlainMetaRecord;`) + +```cpp +using nestedforms::TreeNode; +``` + +(`TreeNode` sorts after `Specimen` — 'S' < 'T' — so append it as the new +last line, after the current last entry, `using nestedforms::Specimen;`) + +- [ ] **Step 4: Replace the "depth cap" test with a three-level recursion test** + +Replace the entire section (from the `// ── Depth limit: exactly one level ──` +comment through the end of that `TEST_CASE`, i.e. the old lines 348-377): + +```cpp +// ── Depth limit: exactly one level ────────────────────────────────────────── + +TEST_CASE("Forms::SchemaJson::NestedAggregate: recursion stops after one level (depth cap)", + "[forms][nested][issue25]") { + auto const schema = morph::forms::schemaJson(); + glz::generic_u64 dom{}; + REQUIRE_FALSE(glz::read_json(dom, schema)); + + REQUIRE(dom["properties"].contains("sample")); + auto const& outerDef = resolveNestedSchema(dom, dom["properties"]["sample"]); + + // Level 1 (DeepSpecimen's own members) IS annotated. + CHECK(outerDef["properties"]["massDry"].contains("x-order")); + CHECK(outerDef["properties"]["massDry"].contains("title")); + REQUIRE(outerDef.contains("required")); + + // Level 2 (Provenance, nested inside DeepSpecimen) is NOT annotated: its + // object schema has no "required" key and its own properties carry no + // x-order/title -- exactly today's pre-existing behaviour for anything + // deeper than one level. "provenance" itself (a level-1 member) DOES get + // an x-order/title on its own property node, same as any other member; + // only its *inner* properties are left untouched. + REQUIRE(outerDef["properties"].contains("provenance")); + CHECK(outerDef["properties"]["provenance"].contains("x-order")); + CHECK(outerDef["properties"]["provenance"].contains("title")); + auto const& innerDef = resolveNestedSchema(dom, outerDef["properties"]["provenance"]); + CHECK_FALSE(innerDef.contains("required")); + CHECK_FALSE(innerDef["properties"]["collectedBy"].contains("x-order")); + CHECK_FALSE(innerDef["properties"]["collectedBy"].contains("title")); +} +``` + +With: + +```cpp +// ── Recursion continues past one level (no depth cap) ─────────────────────── + +TEST_CASE("Forms::SchemaJson::NestedAggregate: recursion continues past one level to whatever depth exists", + "[forms][nested][issue25]") { + auto const schema = morph::forms::schemaJson(); + glz::generic_u64 dom{}; + REQUIRE_FALSE(glz::read_json(dom, schema)); + + REQUIRE(dom["properties"].contains("sample")); + auto const& level1Def = resolveNestedSchema(dom, dom["properties"]["sample"]); + + // Level 1 (DeepSpecimen's own members) is annotated. + CHECK(level1Def["properties"]["massDry"].contains("x-order")); + CHECK(level1Def["properties"]["massDry"].contains("title")); + REQUIRE(level1Def.contains("required")); + + // Level 2 (Provenance, nested inside DeepSpecimen) is now annotated too -- + // both its own property node (x-order/title, same as any level-1 member) + // and, unlike the old one-level cap, its own "required" array. + REQUIRE(level1Def["properties"].contains("provenance")); + CHECK(level1Def["properties"]["provenance"].contains("x-order")); + CHECK(level1Def["properties"]["provenance"].contains("title")); + auto const& level2Def = resolveNestedSchema(dom, level1Def["properties"]["provenance"]); + REQUIRE(level2Def.contains("required")); + CHECK(level2Def["properties"]["collectedBy"].contains("x-order")); + CHECK(level2Def["properties"]["collectedBy"].contains("title")); + + // Level 3 (Origin, nested inside Provenance) is annotated too -- proving + // recursion does not stop at two levels either. + REQUIRE(level2Def["properties"].contains("origin")); + CHECK(level2Def["properties"]["origin"].contains("x-order")); + CHECK(level2Def["properties"]["origin"].contains("title")); + auto const& level3Def = resolveNestedSchema(dom, level2Def["properties"]["origin"]); + REQUIRE(level3Def.contains("required")); + CHECK(level3Def["properties"]["country"].contains("x-order")); + CHECK(level3Def["properties"]["country"].contains("title")); +} +``` + +- [ ] **Step 5: Configure the build (first time only) and build the test binary** + +```bash +cd /Users/yaraslau/repo/morph/.claude/worktrees/agent-a504219143d710713 +export VCPKG_ROOT=/Users/yaraslau/.local/share/vcpkg +cmake --preset clang-release +cmake --build build/clang-release --target morph_tests +``` + +- [ ] **Step 6: Run the new test and confirm it fails** + +```bash +./build/clang-release/tests/morph_tests "recursion continues past one level to whatever depth exists" +``` + +Expected: **FAIL**. `level2Def` will not contain `"required"` (current code +leaves `Provenance`'s own object schema — and everything past it — completely +unannotated, since it never recurses past `DeepSpecimen`). `REQUIRE(level2Def.contains("required"))` +is expected to trip first. + +- [ ] **Step 7: Commit** + +```bash +git add tests/test_nested_forms.cpp +git commit -m "$(cat <<'EOF' +test(forms): extend nested-aggregate fixtures to three levels + +Replaces the one-level "depth cap" test with a failing test proving +recursion should continue past one level: DeepSpecimen -> Provenance -> +Origin, three levels deep. Also adds a self-referential TreeNode +fixture (never passed to schemaJson() here -- that would trip the +forthcoming cycle guard) proving such a type is unaffected on its own. + +Signed-off-by: Yaraslau Tamashevich +EOF +)" +``` + +--- + +### Task 2: Implement cycle-guarded unbounded-depth recursion in `forms.hpp` + +**Files:** +- Modify: `include/morph/forms/forms.hpp:1559-1564` (`ReflectableAggregate` doc comment) +- Modify: `include/morph/forms/forms.hpp:1660-1733` (replace `annotateNestedAggregate`/`annotateNestedAggregateRef`, add `recurseIntoNestedAggregateIfAny`) +- Modify: `include/morph/forms/forms.hpp:1896-1915` (`mergeSchemaExtras`'s nested-aggregate branch) + +**Interfaces:** +- Consumes: `ReflectableAggregate` concept, `IsStdVector` trait, + `annotateBasicMemberProperty(property, name)`, + `forEachNamedMember`, `declaredOptional(name)`, `isStdOptional` — all + pre-existing, all unchanged (defined earlier in the same file). +- Produces: + - `template void annotateNestedAggregate(glz::generic_u64& dom, glz::generic_u64& node)` + — **signature changed**: gains `dom` as its first parameter and an + `Ancestors...` pack. Anything outside this file calling it directly would + need updating; nothing does (only `annotateNestedAggregateRef` and the + tests in `test_nested_forms.cpp` — which call `annotateNestedAggregateRef`, + not this — reference it). + - `template void annotateNestedAggregateRef(glz::generic_u64& dom, glz::generic_u64& propertyOrItems)` + — signature gains the `Ancestors...` pack (variadic, so existing + single-template-argument call sites, e.g. + `annotateNestedAggregateRef(dom, property)` in + `test_nested_forms.cpp:519,528,539`, keep compiling unchanged with an + empty `Ancestors` pack). + - `template void recurseIntoNestedAggregateIfAny(glz::generic_u64& dom, glz::generic_u64& property)` + — new function, used by both `mergeSchemaExtras` and + `annotateNestedAggregate`. + +- [ ] **Step 1: Update `ReflectableAggregate`'s doc comment** + +Replace: + +```cpp +/// @brief Concept: `T` is glaze-reflectable as a JSON object -- the same test +/// that decides whether glaze emits a member into `$defs`/`$ref` +/// rather than inline. Shared by the one-level nested-aggregate +/// recursion below and `reconcileDeclaredPrecision` elsewhere. +template +concept ReflectableAggregate = glz::reflectable || glz::glaze_object_t; +``` + +With: + +```cpp +/// @brief Concept: `T` is glaze-reflectable as a JSON object -- the same test +/// that decides whether glaze emits a member into `$defs`/`$ref` +/// rather than inline. Shared by the cycle-guarded nested-aggregate +/// recursion below and `reconcileDeclaredPrecision` elsewhere. +template +concept ReflectableAggregate = glz::reflectable || glz::glaze_object_t; +``` + +- [ ] **Step 2: Replace `annotateNestedAggregate`/`annotateNestedAggregateRef` with the cycle-guarded, `Ancestors`-threaded versions, plus the new shared helper** + +Replace this entire block (from the blank line right after +`annotateBasicMemberProperty`'s closing brace through the closing brace of the +old `annotateNestedAggregateRef`): + +```cpp + +/// @brief One level of recursion (see `docs/spec/forms/forms.md`, "Nested +/// aggregates (one level)"): annotates @p node -- the object-schema +/// DOM node for a nested-aggregate member -- applying `required` and +/// `annotateBasicMemberProperty`'s rules to its own properties. +/// +/// @p node is @e which DOM node depends on how many places in the whole +/// schema reference `Sub`: glaze **inlines** the object schema directly into +/// the referencing property when `Sub` is used exactly once (so @p node +/// *is* that property node), but **deduplicates** via `$defs`/`$ref` when +/// `Sub` is used two or more times (so @p node is the shared `$defs` entry, +/// resolved by the caller). Both forms have the identical `{"properties": +/// {...}}` shape this function needs, so one implementation handles both -- +/// see the call site in `mergeSchemaExtras` for how @p node is resolved. +/// +/// Deliberately does **not** recurse again: if `Sub` itself has a member that +/// is itself an aggregate, that member is left exactly as glaze emitted it -- +/// unannotated, matching today's behaviour beyond one level. This bounds the +/// generator to one level of nesting, as `docs/spec/forms/forms.md` documents. +/// Computed fields, `formLayout`/`fieldSpans`, and `formRules` also stay +/// top-level-only; a nested `Sub` declaring any of those has no effect here. +/// +/// @tparam Sub Nested aggregate type (default-constructible, glaze-reflectable +/// -- the same requirements the top-level action type already has). +/// @param node The object-schema DOM node to annotate in place (see above). +template +void annotateNestedAggregate(glz::generic_u64& node) { + Sub probe{}; + glz::generic_u64::array_t requiredNames{}; + forEachNamedMember(probe, [&](std::string_view name, const auto& member) { + using Member = std::remove_cvref_t; + if (!(isStdOptional || declaredOptional(name))) { + requiredNames.emplace_back(std::string{name}); + } + auto& property = node["properties"][std::string{name}]; + property["x-order"] = std::uint64_t{I}; + annotateBasicMemberProperty(property, name); + }); + // Idempotent if two members (or two actions sharing this schema call) + // resolve to the same $defs entry: re-deriving the identical required + // array is harmless. + node["required"] = requiredNames; +} + +/// @brief Resolves the object-schema DOM node for a nested-aggregate member, +/// given the property (or array `items`) node glaze wrote for it, and +/// annotates it via `annotateNestedAggregate`. +/// +/// Handles both forms `Sub` can take in the schema (see +/// `annotateNestedAggregate`'s doc comment): a `$ref` into `$defs` (`Sub` used +/// 2+ times somewhere in the schema) resolves to that shared def; anything +/// else is assumed to be the inlined object schema itself (`Sub` used exactly +/// once). A property that is neither -- glaze emitted something other than an +/// object schema for a type this function's caller already confirmed is a +/// `ReflectableAggregate` -- is left untouched rather than guessed at. +/// @tparam Sub Nested aggregate type, as `annotateNestedAggregate` requires. +/// @param dom The whole schema DOM (so a `$ref`'s `$defs` entry can be found). +/// @param propertyOrItems The property node itself (single nested member) or its +/// array `items` node (`std::vector` member). +template +void annotateNestedAggregateRef(glz::generic_u64& dom, glz::generic_u64& propertyOrItems) { + constexpr std::string_view kDefsPrefix = "#/$defs/"; + if (propertyOrItems.contains("$ref")) { + if (auto const* ref = propertyOrItems["$ref"].get_if()) { + if (std::string_view{*ref}.starts_with(kDefsPrefix)) { + annotateNestedAggregate(dom["$defs"][std::string{ref->substr(kDefsPrefix.size())}]); + } + } + return; + } + if (propertyOrItems.contains("properties")) { + annotateNestedAggregate(propertyOrItems); + } +} +``` + +With: + +```cpp + +// annotateNestedAggregate, annotateNestedAggregateRef, and +// recurseIntoNestedAggregateIfAny are mutually recursive (each nested +// aggregate found while annotating one may itself contain another), so all +// three need forward declarations before any of their bodies can reference +// the others. +template +void annotateNestedAggregate(glz::generic_u64& dom, glz::generic_u64& node); + +template +void annotateNestedAggregateRef(glz::generic_u64& dom, glz::generic_u64& propertyOrItems); + +template +void recurseIntoNestedAggregateIfAny(glz::generic_u64& dom, glz::generic_u64& property); + +/// @brief Recurses into @p property's own object schema if @p Member (or, for +/// `std::vector`, its element type) is itself a +/// `ReflectableAggregate` -- the single decision point shared by +/// `mergeSchemaExtras`'s top-level loop and `annotateNestedAggregate`'s +/// own loop, so the cycle guard below has exactly one implementation. +/// +/// @p Ancestors is the chain of nested-aggregate types already being +/// annotated on the current path, **including** the type that declares this +/// member (the caller appends its own `Sub`/`A` before calling this). If the +/// type to recurse into matches any entry already on that chain, recursing +/// further would eventually re-enter this same instantiation and try to do +/// so again -- forever. Rather than let that happen, a `static_assert` (whose +/// condition depends on @p Member and @p Ancestors, so it only fires for the +/// specific cyclic instantiation, not every use of this generator) rejects it +/// at compile time instead: a self-referential nested-aggregate type (e.g. +/// `struct Node { std::vector children; };`), or a mutual reference +/// between two distinct types, fails to build with a clear message rather +/// than exhausting the compiler's template-instantiation depth. This only +/// rejects genuine cycles -- the same type reused from two unrelated places +/// in the schema (a "diamond") is not on either path's ancestor chain and +/// recurses normally into both. See `docs/spec/forms/forms.md`, "Nested +/// aggregates (recursive, cycle-guarded)". +/// @tparam Member The static type of the member `annotateBasicMemberProperty` +/// was just applied to. +/// @tparam Ancestors The ancestor chain so far, ending with the type that +/// declares this member. +/// @param dom The whole schema DOM (so a `$ref`'s `$defs` entry can be found). +/// @param property The property node for this member (or, for `std::vector`, +/// the property whose `"items"` node is the one to check). +template +void recurseIntoNestedAggregateIfAny(glz::generic_u64& dom, glz::generic_u64& property) { + if constexpr (ReflectableAggregate) { + if constexpr ((std::same_as || ...)) { + static_assert(!(std::same_as || ...), + "morph::forms: cyclic nested-aggregate schema -- this member's type already " + "appears in its own chain of enclosing nested-aggregate types (a self- or " + "mutually-referential type). Recursion depth is otherwise unbounded, but cycles " + "are not supported: restructure the domain type (flatten the self-reference, or " + "represent the recursive edge as an opaque id instead of a nested value)."); + } else { + annotateNestedAggregateRef(dom, property); + } + } else if constexpr (IsStdVector::value && + ReflectableAggregate::ValueType>) { + using ItemType = typename IsStdVector::ValueType; + if constexpr ((std::same_as || ...)) { + static_assert(!(std::same_as || ...), + "morph::forms: cyclic nested-aggregate schema -- this std::vector member's " + "element type already appears in its own chain of enclosing nested-aggregate " + "types (a self- or mutually-referential type). Recursion depth is otherwise " + "unbounded, but cycles are not supported: restructure the domain type (flatten " + "the self-reference, or represent the recursive edge as an opaque id instead of " + "a nested value)."); + } else if (property.contains("items")) { + annotateNestedAggregateRef(dom, property["items"]); + } + } +} + +/// @brief Annotates @p node -- the object-schema DOM node for a +/// nested-aggregate member -- applying `required` and +/// `annotateBasicMemberProperty`'s rules to its own properties, then +/// recursing into any of *its* members that are themselves nested +/// aggregates (see `recurseIntoNestedAggregateIfAny`), to whatever +/// depth the type graph actually has. +/// +/// @p node is @e which DOM node depends on how many places in the whole +/// schema reference `Sub`: glaze **inlines** the object schema directly into +/// the referencing property when `Sub` is used exactly once (so @p node +/// *is* that property node), but **deduplicates** via `$defs`/`$ref` when +/// `Sub` is used two or more times (so @p node is the shared `$defs` entry, +/// resolved by the caller). Both forms have the identical `{"properties": +/// {...}}` shape this function needs, so one implementation handles both -- +/// see the call site in `mergeSchemaExtras` for how @p node is resolved. +/// +/// Computed fields, `formLayout`/`fieldSpans`, and `formRules` stay +/// top-level-only regardless of depth; a nested `Sub` declaring any of those +/// has no effect here. +/// +/// @tparam Sub Nested aggregate type (default-constructible, glaze-reflectable +/// -- the same requirements the top-level action type already has). +/// @tparam Ancestors The ancestor chain so far, ending with `Sub` itself, passed +/// through to `recurseIntoNestedAggregateIfAny` for each of +/// `Sub`'s own members (see that function's doc comment). +/// @param dom The whole schema DOM (so a deeper `$ref`'s `$defs` entry can be found). +/// @param node The object-schema DOM node to annotate in place (see above). +template +void annotateNestedAggregate(glz::generic_u64& dom, glz::generic_u64& node) { + Sub probe{}; + glz::generic_u64::array_t requiredNames{}; + forEachNamedMember(probe, [&](std::string_view name, const auto& member) { + using Member = std::remove_cvref_t; + if (!(isStdOptional || declaredOptional(name))) { + requiredNames.emplace_back(std::string{name}); + } + auto& property = node["properties"][std::string{name}]; + property["x-order"] = std::uint64_t{I}; + annotateBasicMemberProperty(property, name); + recurseIntoNestedAggregateIfAny(dom, property); + }); + // Idempotent if two members (or two actions sharing this schema call) + // resolve to the same $defs entry: re-deriving the identical required + // array is harmless. + node["required"] = requiredNames; +} + +/// @brief Resolves the object-schema DOM node for a nested-aggregate member, +/// given the property (or array `items`) node glaze wrote for it, and +/// annotates it via `annotateNestedAggregate`. +/// +/// Handles both forms `Sub` can take in the schema (see +/// `annotateNestedAggregate`'s doc comment): a `$ref` into `$defs` (`Sub` used +/// 2+ times somewhere in the schema) resolves to that shared def; anything +/// else is assumed to be the inlined object schema itself (`Sub` used exactly +/// once). A property that is neither -- glaze emitted something other than an +/// object schema for a type this function's caller already confirmed is a +/// `ReflectableAggregate` -- is left untouched rather than guessed at. +/// @tparam Sub Nested aggregate type, as `annotateNestedAggregate` requires. +/// @tparam Ancestors The ancestor chain so far (excluding `Sub`), forwarded +/// to `annotateNestedAggregate` unchanged. +/// @param dom The whole schema DOM (so a `$ref`'s `$defs` entry can be found). +/// @param propertyOrItems The property node itself (single nested member) or its +/// array `items` node (`std::vector` member). +template +void annotateNestedAggregateRef(glz::generic_u64& dom, glz::generic_u64& propertyOrItems) { + constexpr std::string_view kDefsPrefix = "#/$defs/"; + if (propertyOrItems.contains("$ref")) { + if (auto const* ref = propertyOrItems["$ref"].get_if()) { + if (std::string_view{*ref}.starts_with(kDefsPrefix)) { + annotateNestedAggregate( + dom, dom["$defs"][std::string{ref->substr(kDefsPrefix.size())}]); + } + } + return; + } + if (propertyOrItems.contains("properties")) { + annotateNestedAggregate(dom, propertyOrItems); + } +} +``` + +- [ ] **Step 3: Simplify `mergeSchemaExtras`'s nested-aggregate branch to call the new shared helper** + +Replace: + +```cpp + // Nested aggregates (one level -- docs/spec/forms/forms.md, "Nested + // aggregates (one level)"): a member whose type is itself a + // reflectable aggregate gets an object schema from glaze -- either + // inlined directly into this property (the type is used exactly once + // in the whole schema) or shared via `$defs`/`$ref` (used 2+ times); + // `annotateNestedAggregateRef` resolves whichever form it is. Recurse + // one level so that object schema's own members get `x-order`/ + // `required`/title/Quantity/Choice/widget annotations too, instead of + // being silently unannotated. Purely additive: an action with no + // nested aggregate member has nothing here to trigger on, so its + // schema is byte-for-byte unchanged. + if constexpr (ReflectableAggregate) { + annotateNestedAggregateRef(dom, property); + } else if constexpr (IsStdVector::value && + ReflectableAggregate::ValueType>) { + using ItemType = typename IsStdVector::ValueType; + if (property.contains("items")) { + annotateNestedAggregateRef(dom, property["items"]); + } + } +``` + +With: + +```cpp + // Nested aggregates (recursive, cycle-guarded -- docs/spec/forms/forms.md, + // "Nested aggregates (recursive, cycle-guarded)"): a member whose type + // is itself a reflectable aggregate gets an object schema from glaze -- + // either inlined directly into this property (the type is used exactly + // once in the whole schema) or shared via `$defs`/`$ref` (used 2+ + // times). `recurseIntoNestedAggregateIfAny` resolves whichever form it + // is and recurses so that object schema's own members get + // `x-order`/`required`/title/Quantity/Choice/widget annotations too, + // however deep the type graph goes (guarding against cycles at compile + // time -- see that function's doc comment). Purely additive: an action + // with no nested aggregate member has nothing here to trigger on, so + // its schema is byte-for-byte unchanged. + recurseIntoNestedAggregateIfAny(dom, property); +``` + +- [ ] **Step 4: Rebuild and run the full nested-forms test suite** + +```bash +cmake --build build/clang-release --target morph_tests +./build/clang-release/tests/morph_tests "[forms][nested]" +``` + +Expected: **PASS** — every test tagged `[forms][nested]`, including the new +three-level test from Task 1 and the pre-existing idempotence/`$ref`/inline/ +`FieldMeta`/`Quantity`/`Choice`/`declaredOptional`/defensive-fallback tests +(none of their expectations changed; the earlier levels' behavior is +unaffected by extending recursion further). + +- [ ] **Step 5: Run the complete test suite to check for regressions elsewhere** + +```bash +./build/clang-release/tests/morph_tests +``` + +Expected: **PASS** — no other test exercises `forms.hpp`'s nested-aggregate +path with a type graph deep enough to be affected, so this is a regression +check, not expected to surface anything new. + +- [ ] **Step 6: Commit** + +```bash +git add include/morph/forms/forms.hpp +git commit -m "$(cat <<'EOF' +forms: recurse into nested aggregates to any depth, not just one + +Threads an Ancestors... template parameter pack through +annotateNestedAggregate/annotateNestedAggregateRef -- the chain of +nested-aggregate types already being annotated on the current path, +starting with the action type. Factors the "is this member itself a +nested aggregate, and should I recurse into it" decision (previously +duplicated inline in mergeSchemaExtras's loop, and entirely absent from +annotateNestedAggregate since it never went past one level) into one +shared recurseIntoNestedAggregateIfAny, used by both loops. + +A member whose type -- or, for std::vector, Sub -- already appears +on the ancestor chain would recurse into this same instantiation again, +forever. Rather than let that happen, a static_assert (dependent on the +member type and the chain, so it only fires for the actual cyclic +instantiation) rejects a self- or mutually-referential nested-aggregate +type graph at compile time instead. + +See docs/spec/forms/forms.md, "Nested aggregates (recursive, +cycle-guarded)". + +Signed-off-by: Yaraslau Tamashevich +EOF +)" +``` + +--- + +### Task 3: Add the standalone self-referential-type test and verify the docs build + +**Files:** +- Modify: `tests/test_nested_forms.cpp` (add one `TEST_CASE`, after the last + existing test case in the file) + +**Interfaces:** +- Consumes: `nestedforms::TreeNode` (added in Task 1, Step 2), `glz::write_json`, + `glz::read_json` (Glaze's own API, already used elsewhere in this test + suite — see `tests/test_bridge_fixes.cpp:252` for the `.value_or(std::string{})` + pattern this step follows). +- Produces: nothing consumed by anything later — this is the plan's last task. + +- [ ] **Step 1: Add the standalone self-referential-type test** + +Append to the end of `tests/test_nested_forms.cpp`: + +```cpp + +// ── Self-referential nested-aggregate type, standalone ───────────────────── + +TEST_CASE("Forms::SchemaJson::NestedAggregate: a self-referential nested-aggregate type round-trips fine on its own", + "[forms][nested][issue25]") { + // TreeNode is never passed to morph::forms::schemaJson() in this file + // -- see its doc comment. This only proves the type itself, and ordinary + // glaze JSON round-tripping over it, are completely unaffected by + // forms.hpp's cycle-guard static_assert, which fires only when a type + // like this is actually nested under some schemaJson() instantiation. + TreeNode root{}; + root.name = "root"; + TreeNode child{}; + child.name = "child"; + root.children.push_back(child); + + std::string const json = glz::write_json(root).value_or(std::string{}); + REQUIRE_FALSE(json.empty()); + + TreeNode decoded{}; + REQUIRE_FALSE(glz::read_json(decoded, json)); + CHECK(decoded.name == "root"); + REQUIRE(decoded.children.size() == 1); + CHECK(decoded.children[0].name == "child"); +} +``` + +- [ ] **Step 2: Rebuild and run** + +```bash +cmake --build build/clang-release --target morph_tests +./build/clang-release/tests/morph_tests "[forms][nested]" +``` + +Expected: **PASS**, including the new test. + +- [ ] **Step 3: Verify the Doxygen docs build succeeds with the new/changed comments** + +```bash +cmake -S . -B build/docs -G Ninja -DMORPH_BUILD_DOCUMENTATION=ON -DMORPH_BUILD_TESTS=OFF -DMORPH_BUILD_EXAMPLES=OFF +cmake --build build/docs --target doc +``` + +Expected: build succeeds (exit code 0). If it fails on a missing +`@param`/`@tparam`/`@return`, compare the failing function's signature against +Task 2 Step 2/3's replacement text above and add the missing tag — every +parameter and template parameter introduced there already has one documented, +so a failure here means a transcription slip, not a design gap. + +- [ ] **Step 4: Commit (only if Step 3 required a fix)** + +```bash +git add include/morph/forms/forms.hpp +git commit -m "$(cat <<'EOF' +forms: fix missing Doxygen tag on nested-aggregate recursion helper + +Signed-off-by: Yaraslau Tamashevich +EOF +)" +``` + +If Step 3 passed cleanly with no changes needed, skip this commit — +Step 1's test addition was already committed as part of a normal +add-test-then-verify cycle; commit just that: + +```bash +git add tests/test_nested_forms.cpp +git commit -m "$(cat <<'EOF' +test(forms): cover a self-referential nested-aggregate type standalone + +Proves TreeNode -- a tree-shaped, self-referential nested-aggregate +type -- round-trips through glaze JSON encode/decode normally on its +own. It is never passed to schemaJson() in this file; doing so +would trip forms.hpp's cycle-guard static_assert, which this test +suite has no harness to exercise directly (see +docs/spec/forms/forms.md, "Nested aggregates (recursive, +cycle-guarded)"). + +Signed-off-by: Yaraslau Tamashevich +EOF +)" +``` + +--- + +## Self-Review Notes + +- **Spec coverage:** Every behavior the spec (`docs/spec/forms/forms.md`, + "Nested aggregates (recursive, cycle-guarded)") describes has a + corresponding task: unbounded acyclic recursion depth (Task 1's test, Task + 2's implementation), the cycle-guard `static_assert` and its scoping to only + the offending instantiation (Task 2 Step 2's doc comment and code), the + "diamond is not a cycle" guarantee (unchanged `$defs`/`$ref` dedup logic, + covered by the pre-existing `$ref`-form tests that keep passing in Task 2 + Step 4), computed-fields/`formLayout`/`formRules` staying top-level-only + (untouched code, no task needed), and "a self-referential type not nested + under an action compiles fine" (Task 1 Step 2's `TreeNode` fixture, Task 3's + test). +- **Type consistency:** `annotateNestedAggregate`'s signature + (`glz::generic_u64& dom, glz::generic_u64& node` plus `Sub, Ancestors...`) + matches every call site introduced across Task 2 (`annotateNestedAggregateRef`'s + two internal calls; no other caller exists). `recurseIntoNestedAggregateIfAny`'s + signature matches both of its call sites (`mergeSchemaExtras`'s + ``, `annotateNestedAggregate`'s ``). + `annotateNestedAggregateRef`'s existing test call sites + (`test_nested_forms.cpp:519,528,539`, `annotateNestedAggregateRef(dom, property)`) + remain valid: `Ancestors...` is variadic and defaults to empty when only + `Sub` is given explicitly. +- **No placeholders:** every step above contains the literal before/after code + or the literal shell command to run; none say "add appropriate X" or defer + detail to another task. From 4d315eec5929d1e9505dbfde53521cb8e093d820 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Wed, 5 Aug 2026 12:16:54 +0300 Subject: [PATCH 08/13] test(forms): extend nested-aggregate fixtures to three levels Replaces the one-level "depth cap" test with a failing test proving recursion should continue past one level: DeepSpecimen -> Provenance -> Origin, three levels deep. Also adds a self-referential TreeNode fixture (never passed to schemaJson() here -- that would trip the forthcoming cycle guard) proving such a type is unaffected on its own. Signed-off-by: Yaraslau Tamashevich --- tests/test_nested_forms.cpp | 88 +++++++++++++++++++++++++------------ 1 file changed, 59 insertions(+), 29 deletions(-) diff --git a/tests/test_nested_forms.cpp b/tests/test_nested_forms.cpp index caefd83d..7b2edc45 100644 --- a/tests/test_nested_forms.cpp +++ b/tests/test_nested_forms.cpp @@ -1,16 +1,19 @@ // SPDX-License-Identifier: Apache-2.0 // -// Coverage for issue #25: form generation recurses one level into a -// nested-aggregate member's object schema -- a directly-nested struct member -// or a `std::vector` repeated aggregate -- applying the same +// Coverage for issue #25: form generation recurses into a nested-aggregate +// member's object schema -- a directly-nested struct member or a +// `std::vector` repeated aggregate -- applying the same // title/x-order/required/widget rules the top level already applies, instead // of leaving it entirely unannotated. Two distinct schema shapes exist for a // nested aggregate (see forms.hpp's `annotateNestedAggregateRef`): glaze // *inlines* the object schema directly into the property when the nested // type is used exactly once in the whole schema, and *deduplicates* it via a // shared `$defs` entry (referenced by `$ref`) when it is used two or more -// times. Both are exercised below. Recursion stops after one level (see -// docs/spec/forms/forms.md, "Nested aggregates (one level)"). +// times. Both are exercised below. Recursion continues to whatever depth the +// type graph actually has, stopping only at a genuine cycle -- a compile-time +// `static_assert`, not something this runtime test suite can exercise +// directly (see docs/spec/forms/forms.md, "Nested aggregates (recursive, +// cycle-guarded)"). #include #include @@ -72,17 +75,35 @@ struct Attachment { std::int64_t sizeBytes = 0; }; +struct Origin { + std::string country; +}; + +// Three levels deep: DeepSpecimen -> Provenance -> Origin. Provenance's own +// member (origin) is itself a nested aggregate too -- proving recursion +// continues past one level. struct Provenance { std::string collectedBy; + Origin origin; }; -// A nested aggregate whose own member is itself a nested aggregate -- the -// depth-limit case: `provenance`'s own sub-members must stay unannotated. struct DeepSpecimen { double massDry = 0.0; Provenance provenance; }; +// A self-referential nested-aggregate type (a tree node). Never passed to +// morph::forms::schemaJson() anywhere in this file -- neither as the +// top-level action type itself nor nested inside another action's member -- +// either use would trip forms.hpp's cycle-guard static_assert (see +// docs/spec/forms/forms.md, "Nested aggregates (recursive, cycle-guarded)"). +// This only proves the type itself, and ordinary glaze JSON round-tripping +// over it, are completely unaffected by that guard. +struct TreeNode { + std::string name; + std::vector children; +}; + // Specimen and Attachment are each used from two places below, so glaze // deduplicates both via a shared `$defs` entry referenced by `$ref`. struct Record { @@ -201,6 +222,7 @@ using nestedforms::BareQuantityRecord; using nestedforms::DeclaredOptionalRecord; using nestedforms::DeepRecord; using nestedforms::DeepSpecimen; +using nestedforms::Origin; using nestedforms::PlainMetaRecord; using nestedforms::Provenance; using nestedforms::Record; @@ -208,6 +230,7 @@ using nestedforms::RichRecord; using nestedforms::SingleUseRecord; using nestedforms::SingleUseVectorRecord; using nestedforms::Specimen; +using nestedforms::TreeNode; namespace { @@ -345,35 +368,42 @@ TEST_CASE( REQUIRE(def.contains("required")); } -// ── Depth limit: exactly one level ────────────────────────────────────────── +// ── Recursion continues past one level (no depth cap) ─────────────────────── -TEST_CASE("Forms::SchemaJson::NestedAggregate: recursion stops after one level (depth cap)", +TEST_CASE("Forms::SchemaJson::NestedAggregate: recursion continues past one level to whatever depth exists", "[forms][nested][issue25]") { auto const schema = morph::forms::schemaJson(); glz::generic_u64 dom{}; REQUIRE_FALSE(glz::read_json(dom, schema)); REQUIRE(dom["properties"].contains("sample")); - auto const& outerDef = resolveNestedSchema(dom, dom["properties"]["sample"]); - - // Level 1 (DeepSpecimen's own members) IS annotated. - CHECK(outerDef["properties"]["massDry"].contains("x-order")); - CHECK(outerDef["properties"]["massDry"].contains("title")); - REQUIRE(outerDef.contains("required")); - - // Level 2 (Provenance, nested inside DeepSpecimen) is NOT annotated: its - // object schema has no "required" key and its own properties carry no - // x-order/title -- exactly today's pre-existing behaviour for anything - // deeper than one level. "provenance" itself (a level-1 member) DOES get - // an x-order/title on its own property node, same as any other member; - // only its *inner* properties are left untouched. - REQUIRE(outerDef["properties"].contains("provenance")); - CHECK(outerDef["properties"]["provenance"].contains("x-order")); - CHECK(outerDef["properties"]["provenance"].contains("title")); - auto const& innerDef = resolveNestedSchema(dom, outerDef["properties"]["provenance"]); - CHECK_FALSE(innerDef.contains("required")); - CHECK_FALSE(innerDef["properties"]["collectedBy"].contains("x-order")); - CHECK_FALSE(innerDef["properties"]["collectedBy"].contains("title")); + auto const& level1Def = resolveNestedSchema(dom, dom["properties"]["sample"]); + + // Level 1 (DeepSpecimen's own members) is annotated. + CHECK(level1Def["properties"]["massDry"].contains("x-order")); + CHECK(level1Def["properties"]["massDry"].contains("title")); + REQUIRE(level1Def.contains("required")); + + // Level 2 (Provenance, nested inside DeepSpecimen) is now annotated too -- + // both its own property node (x-order/title, same as any level-1 member) + // and, unlike the old one-level cap, its own "required" array. + REQUIRE(level1Def["properties"].contains("provenance")); + CHECK(level1Def["properties"]["provenance"].contains("x-order")); + CHECK(level1Def["properties"]["provenance"].contains("title")); + auto const& level2Def = resolveNestedSchema(dom, level1Def["properties"]["provenance"]); + REQUIRE(level2Def.contains("required")); + CHECK(level2Def["properties"]["collectedBy"].contains("x-order")); + CHECK(level2Def["properties"]["collectedBy"].contains("title")); + + // Level 3 (Origin, nested inside Provenance) is annotated too -- proving + // recursion does not stop at two levels either. + REQUIRE(level2Def["properties"].contains("origin")); + CHECK(level2Def["properties"]["origin"].contains("x-order")); + CHECK(level2Def["properties"]["origin"].contains("title")); + auto const& level3Def = resolveNestedSchema(dom, level2Def["properties"]["origin"]); + REQUIRE(level3Def.contains("required")); + CHECK(level3Def["properties"]["country"].contains("x-order")); + CHECK(level3Def["properties"]["country"].contains("title")); } // ── FieldMeta/Quantity/Choice rules apply one level down too ──────────────── From c521c92d0ff12530b5bceb8f8cb8e0e58c634014 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Wed, 5 Aug 2026 12:24:22 +0300 Subject: [PATCH 09/13] forms: recurse into nested aggregates to any depth, not just one Threads an Ancestors... template parameter pack through annotateNestedAggregate/annotateNestedAggregateRef -- the chain of nested-aggregate types already being annotated on the current path, starting with the action type. Factors the "is this member itself a nested aggregate, and should I recurse into it" decision (previously duplicated inline in mergeSchemaExtras's loop, and entirely absent from annotateNestedAggregate since it never went past one level) into one shared recurseIntoNestedAggregateIfAny, used by both loops. A member whose type -- or, for std::vector, Sub -- already appears on the ancestor chain would recurse into this same instantiation again, forever. Rather than let that happen, a static_assert (dependent on the member type and the chain, so it only fires for the actual cyclic instantiation) rejects a self- or mutually-referential nested-aggregate type graph at compile time instead. See docs/spec/forms/forms.md, "Nested aggregates (recursive, cycle-guarded)". Signed-off-by: Yaraslau Tamashevich --- include/morph/forms/forms.hpp | 151 +++++++++++++++++++++++++--------- 1 file changed, 112 insertions(+), 39 deletions(-) diff --git a/include/morph/forms/forms.hpp b/include/morph/forms/forms.hpp index f806376a..2a2f90fe 100644 --- a/include/morph/forms/forms.hpp +++ b/include/morph/forms/forms.hpp @@ -1558,7 +1558,7 @@ struct IsStdVector> : std::true_type { /// @brief Concept: `T` is glaze-reflectable as a JSON object -- the same test /// that decides whether glaze emits a member into `$defs`/`$ref` -/// rather than inline. Shared by the one-level nested-aggregate +/// rather than inline. Shared by the cycle-guarded nested-aggregate /// recursion below and `reconcileDeclaredPrecision` elsewhere. template concept ReflectableAggregate = glz::reflectable || glz::glaze_object_t; @@ -1658,10 +1658,85 @@ void annotateBasicMemberProperty(glz::generic_u64& property, std::string_view na } } -/// @brief One level of recursion (see `docs/spec/forms/forms.md`, "Nested -/// aggregates (one level)"): annotates @p node -- the object-schema -/// DOM node for a nested-aggregate member -- applying `required` and -/// `annotateBasicMemberProperty`'s rules to its own properties. +// annotateNestedAggregate, annotateNestedAggregateRef, and +// recurseIntoNestedAggregateIfAny are mutually recursive (each nested +// aggregate found while annotating one may itself contain another), so all +// three need forward declarations before any of their bodies can reference +// the others. +template +void annotateNestedAggregate(glz::generic_u64& dom, glz::generic_u64& node); + +template +void annotateNestedAggregateRef(glz::generic_u64& dom, glz::generic_u64& propertyOrItems); + +template +void recurseIntoNestedAggregateIfAny(glz::generic_u64& dom, glz::generic_u64& property); + +/// @brief Recurses into @p property's own object schema if @p Member (or, for +/// `std::vector`, its element type) is itself a +/// `ReflectableAggregate` -- the single decision point shared by +/// `mergeSchemaExtras`'s top-level loop and `annotateNestedAggregate`'s +/// own loop, so the cycle guard below has exactly one implementation. +/// +/// @p Ancestors is the chain of nested-aggregate types already being +/// annotated on the current path, **including** the type that declares this +/// member (the caller appends its own `Sub`/`A` before calling this). If the +/// type to recurse into matches any entry already on that chain, recursing +/// further would eventually re-enter this same instantiation and try to do +/// so again -- forever. Rather than let that happen, a `static_assert` (whose +/// condition depends on @p Member and @p Ancestors, so it only fires for the +/// specific cyclic instantiation, not every use of this generator) rejects it +/// at compile time instead: a self-referential nested-aggregate type (e.g. +/// `struct Node { std::vector children; };`), or a mutual reference +/// between two distinct types, fails to build with a clear message rather +/// than exhausting the compiler's template-instantiation depth. This only +/// rejects genuine cycles -- the same type reused from two unrelated places +/// in the schema (a "diamond") is not on either path's ancestor chain and +/// recurses normally into both. See `docs/spec/forms/forms.md`, "Nested +/// aggregates (recursive, cycle-guarded)". +/// @tparam Member The static type of the member `annotateBasicMemberProperty` +/// was just applied to. +/// @tparam Ancestors The ancestor chain so far, ending with the type that +/// declares this member. +/// @param dom The whole schema DOM (so a `$ref`'s `$defs` entry can be found). +/// @param property The property node for this member (or, for `std::vector`, +/// the property whose `"items"` node is the one to check). +template +void recurseIntoNestedAggregateIfAny(glz::generic_u64& dom, glz::generic_u64& property) { + if constexpr (ReflectableAggregate) { + if constexpr ((std::same_as || ...)) { + static_assert(!(std::same_as || ...), + "morph::forms: cyclic nested-aggregate schema -- this member's type already " + "appears in its own chain of enclosing nested-aggregate types (a self- or " + "mutually-referential type). Recursion depth is otherwise unbounded, but cycles " + "are not supported: restructure the domain type (flatten the self-reference, or " + "represent the recursive edge as an opaque id instead of a nested value)."); + } else { + annotateNestedAggregateRef(dom, property); + } + } else if constexpr (IsStdVector::value && + ReflectableAggregate::ValueType>) { + using ItemType = typename IsStdVector::ValueType; + if constexpr ((std::same_as || ...)) { + static_assert(!(std::same_as || ...), + "morph::forms: cyclic nested-aggregate schema -- this std::vector member's " + "element type already appears in its own chain of enclosing nested-aggregate " + "types (a self- or mutually-referential type). Recursion depth is otherwise " + "unbounded, but cycles are not supported: restructure the domain type (flatten " + "the self-reference, or represent the recursive edge as an opaque id instead of " + "a nested value)."); + } else if (property.contains("items")) { + annotateNestedAggregateRef(dom, property["items"]); + } + } +} + +/// @brief Annotates @p node -- the object-schema DOM node for a +/// nested-aggregate member -- applying `required` and +/// `annotateBasicMemberProperty`'s rules to its own properties, then +/// recursing into any of *its* members that are themselves nested +/// aggregates (see `recurseIntoNestedAggregateIfAny`), to whatever +/// depth the type graph actually has. /// /// @p node is @e which DOM node depends on how many places in the whole /// schema reference `Sub`: glaze **inlines** the object schema directly into @@ -1672,18 +1747,19 @@ void annotateBasicMemberProperty(glz::generic_u64& property, std::string_view na /// {...}}` shape this function needs, so one implementation handles both -- /// see the call site in `mergeSchemaExtras` for how @p node is resolved. /// -/// Deliberately does **not** recurse again: if `Sub` itself has a member that -/// is itself an aggregate, that member is left exactly as glaze emitted it -- -/// unannotated, matching today's behaviour beyond one level. This bounds the -/// generator to one level of nesting, as `docs/spec/forms/forms.md` documents. -/// Computed fields, `formLayout`/`fieldSpans`, and `formRules` also stay -/// top-level-only; a nested `Sub` declaring any of those has no effect here. +/// Computed fields, `formLayout`/`fieldSpans`, and `formRules` stay +/// top-level-only regardless of depth; a nested `Sub` declaring any of those +/// has no effect here. /// -/// @tparam Sub Nested aggregate type (default-constructible, glaze-reflectable -/// -- the same requirements the top-level action type already has). +/// @tparam Sub Nested aggregate type (default-constructible, glaze-reflectable +/// -- the same requirements the top-level action type already has). +/// @tparam Ancestors The ancestor chain so far, ending with `Sub` itself, passed +/// through to `recurseIntoNestedAggregateIfAny` for each of +/// `Sub`'s own members (see that function's doc comment). +/// @param dom The whole schema DOM (so a deeper `$ref`'s `$defs` entry can be found). /// @param node The object-schema DOM node to annotate in place (see above). -template -void annotateNestedAggregate(glz::generic_u64& node) { +template +void annotateNestedAggregate(glz::generic_u64& dom, glz::generic_u64& node) { Sub probe{}; glz::generic_u64::array_t requiredNames{}; forEachNamedMember(probe, [&](std::string_view name, const auto& member) { @@ -1694,6 +1770,7 @@ void annotateNestedAggregate(glz::generic_u64& node) { auto& property = node["properties"][std::string{name}]; property["x-order"] = std::uint64_t{I}; annotateBasicMemberProperty(property, name); + recurseIntoNestedAggregateIfAny(dom, property); }); // Idempotent if two members (or two actions sharing this schema call) // resolve to the same $defs entry: re-deriving the identical required @@ -1703,7 +1780,7 @@ void annotateNestedAggregate(glz::generic_u64& node) { /// @brief Resolves the object-schema DOM node for a nested-aggregate member, /// given the property (or array `items`) node glaze wrote for it, and -/// annotates it via `annotateNestedAggregate`. +/// annotates it via `annotateNestedAggregate`. /// /// Handles both forms `Sub` can take in the schema (see /// `annotateNestedAggregate`'s doc comment): a `$ref` into `$defs` (`Sub` used @@ -1713,22 +1790,25 @@ void annotateNestedAggregate(glz::generic_u64& node) { /// object schema for a type this function's caller already confirmed is a /// `ReflectableAggregate` -- is left untouched rather than guessed at. /// @tparam Sub Nested aggregate type, as `annotateNestedAggregate` requires. +/// @tparam Ancestors The ancestor chain so far (excluding `Sub`), forwarded +/// to `annotateNestedAggregate` unchanged. /// @param dom The whole schema DOM (so a `$ref`'s `$defs` entry can be found). /// @param propertyOrItems The property node itself (single nested member) or its /// array `items` node (`std::vector` member). -template +template void annotateNestedAggregateRef(glz::generic_u64& dom, glz::generic_u64& propertyOrItems) { constexpr std::string_view kDefsPrefix = "#/$defs/"; if (propertyOrItems.contains("$ref")) { if (auto const* ref = propertyOrItems["$ref"].get_if()) { if (std::string_view{*ref}.starts_with(kDefsPrefix)) { - annotateNestedAggregate(dom["$defs"][std::string{ref->substr(kDefsPrefix.size())}]); + annotateNestedAggregate( + dom, dom["$defs"][std::string{ref->substr(kDefsPrefix.size())}]); } } return; } if (propertyOrItems.contains("properties")) { - annotateNestedAggregate(propertyOrItems); + annotateNestedAggregate(dom, propertyOrItems); } } @@ -1893,26 +1973,19 @@ template } } - // Nested aggregates (one level -- docs/spec/forms/forms.md, "Nested - // aggregates (one level)"): a member whose type is itself a - // reflectable aggregate gets an object schema from glaze -- either - // inlined directly into this property (the type is used exactly once - // in the whole schema) or shared via `$defs`/`$ref` (used 2+ times); - // `annotateNestedAggregateRef` resolves whichever form it is. Recurse - // one level so that object schema's own members get `x-order`/ - // `required`/title/Quantity/Choice/widget annotations too, instead of - // being silently unannotated. Purely additive: an action with no - // nested aggregate member has nothing here to trigger on, so its - // schema is byte-for-byte unchanged. - if constexpr (ReflectableAggregate) { - annotateNestedAggregateRef(dom, property); - } else if constexpr (IsStdVector::value && - ReflectableAggregate::ValueType>) { - using ItemType = typename IsStdVector::ValueType; - if (property.contains("items")) { - annotateNestedAggregateRef(dom, property["items"]); - } - } + // Nested aggregates (recursive, cycle-guarded -- docs/spec/forms/forms.md, + // "Nested aggregates (recursive, cycle-guarded)"): a member whose type + // is itself a reflectable aggregate gets an object schema from glaze -- + // either inlined directly into this property (the type is used exactly + // once in the whole schema) or shared via `$defs`/`$ref` (used 2+ + // times). `recurseIntoNestedAggregateIfAny` resolves whichever form it + // is and recurses so that object schema's own members get + // `x-order`/`required`/title/Quantity/Choice/widget annotations too, + // however deep the type graph goes (guarding against cycles at compile + // time -- see that function's doc comment). Purely additive: an action + // with no nested aggregate member has nothing here to trigger on, so + // its schema is byte-for-byte unchanged. + recurseIntoNestedAggregateIfAny(dom, property); }); // Always assign — an explicit empty array beats leaving whatever the // schema writer may have emitted (or omitted) for `required`. From 12762a255933ca25ae45fb2a07bef4468cd8daff Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Wed, 5 Aug 2026 12:36:33 +0300 Subject: [PATCH 10/13] forms: fix stale/incorrect nested-aggregate recursion doc comments Two doc-only fixes caught by task review: annotateNestedAggregate's @tparam Ancestors said the chain ends with Sub, when Sub is actually appended only when recursing one level deeper (contradicting the sibling comment on annotateNestedAggregateRef, which already had it right). The file's top-of-header public API doc still described the old one-level cap and pointed at a spec section title that no longer exists. Signed-off-by: Yaraslau Tamashevich --- include/morph/forms/forms.hpp | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/include/morph/forms/forms.hpp b/include/morph/forms/forms.hpp index 2a2f90fe..ea136168 100644 --- a/include/morph/forms/forms.hpp +++ b/include/morph/forms/forms.hpp @@ -68,15 +68,18 @@ /// `morph::time::Timestamp` members need no extension keys: their schema /// carries the standard `"format": "date-time"` annotation. /// -/// **Nested aggregates (one level).** A member whose type is itself a -/// reflectable aggregate — a plain nested struct, or `std::vector` — gets -/// its own members annotated too, one level down: `x-order`, title/`FieldMeta`, -/// `required`, and the `Quantity`/`Choice`/widget/ranged-bounds rules above, -/// applied against the nested type's own reflection. Recursion stops after -/// this one level (a nested aggregate's own nested-aggregate members are left -/// unannotated), and computed fields/`formLayout`/`fieldSpans`/`formRules` -/// remain top-level-only. See docs/spec/forms/forms.md, "Nested aggregates -/// (one level)", and `detail::annotateNestedAggregateRef`. +/// **Nested aggregates (recursive, cycle-guarded).** A member whose type is +/// itself a reflectable aggregate — a plain nested struct, or +/// `std::vector` — gets its own members annotated too: `x-order`, +/// title/`FieldMeta`, `required`, and the `Quantity`/`Choice`/widget/ +/// ranged-bounds rules above, applied against the nested type's own +/// reflection. Unlike the top level, this recurses to whatever depth the +/// type graph actually has, stopping only at a genuine cycle (a self- or +/// mutually-referential nested-aggregate type), which is a compile-time +/// `static_assert` rather than infinite recursion. Computed fields/ +/// `formLayout`/`fieldSpans`/`formRules` remain top-level-only regardless of +/// depth. See docs/spec/forms/forms.md, "Nested aggregates (recursive, +/// cycle-guarded)", and `detail::annotateNestedAggregateRef`. /// /// @par Declaring optional fields /// Required is the default. An action opts individual fields out with a @@ -1753,9 +1756,10 @@ void recurseIntoNestedAggregateIfAny(glz::generic_u64& dom, glz::generic_u64& pr /// /// @tparam Sub Nested aggregate type (default-constructible, glaze-reflectable /// -- the same requirements the top-level action type already has). -/// @tparam Ancestors The ancestor chain so far, ending with `Sub` itself, passed -/// through to `recurseIntoNestedAggregateIfAny` for each of -/// `Sub`'s own members (see that function's doc comment). +/// @tparam Ancestors The ancestor chain so far (excluding `Sub`); `Sub` is +/// appended before recursing into each of `Sub`'s own +/// members via `recurseIntoNestedAggregateIfAny` (see +/// that function's doc comment). /// @param dom The whole schema DOM (so a deeper `$ref`'s `$defs` entry can be found). /// @param node The object-schema DOM node to annotate in place (see above). template From d899a048653c338664edb9bb17ecccc88b9616e9 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Wed, 5 Aug 2026 12:40:06 +0300 Subject: [PATCH 11/13] test(forms): cover a self-referential nested-aggregate type standalone Proves TreeNode -- a tree-shaped, self-referential nested-aggregate type -- round-trips through glaze JSON encode/decode normally on its own. It is never passed to schemaJson() in this file; doing so would trip forms.hpp's cycle-guard static_assert, which this test suite has no harness to exercise directly (see docs/spec/forms/forms.md, "Nested aggregates (recursive, cycle-guarded)"). Signed-off-by: Yaraslau Tamashevich --- tests/test_nested_forms.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_nested_forms.cpp b/tests/test_nested_forms.cpp index 7b2edc45..b120345b 100644 --- a/tests/test_nested_forms.cpp +++ b/tests/test_nested_forms.cpp @@ -570,3 +570,28 @@ TEST_CASE( CHECK_FALSE(property.contains("required")); CHECK(property["type"].get() == "string"); } + +// ── Self-referential nested-aggregate type, standalone ───────────────────── + +TEST_CASE("Forms::SchemaJson::NestedAggregate: a self-referential nested-aggregate type round-trips fine on its own", + "[forms][nested][issue25]") { + // TreeNode is never passed to morph::forms::schemaJson() in this file + // -- see its doc comment. This only proves the type itself, and ordinary + // glaze JSON round-tripping over it, are completely unaffected by + // forms.hpp's cycle-guard static_assert, which fires only when a type + // like this is actually nested under some schemaJson() instantiation. + TreeNode root{}; + root.name = "root"; + TreeNode child{}; + child.name = "child"; + root.children.push_back(child); + + std::string const json = glz::write_json(root).value_or(std::string{}); + REQUIRE_FALSE(json.empty()); + + TreeNode decoded{}; + REQUIRE_FALSE(glz::read_json(decoded, json)); + CHECK(decoded.name == "root"); + REQUIRE(decoded.children.size() == 1); + CHECK(decoded.children[0].name == "child"); +} From 820a993c20b8282436f7adcde6a0694580c8c14f Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Wed, 5 Aug 2026 12:57:53 +0300 Subject: [PATCH 12/13] forms: fix final-review findings (dedup, use-after-free guard, spec) Three Important findings from the final whole-branch review: mergeSchemaExtras duplicated annotateBasicMemberProperty's ~90-line body instead of calling it (the doc comment already falsely claimed this sharing existed); annotateNestedAggregateRef's $defs lookup inserted an empty entry on a miss, which the new recursion turned from dormant into a live use-after-free risk (glz::generic_u64's object storage reallocates on insert); and the spec's normative renderer-contract section still asserted nested $defs are never patched, which this branch's whole point contradicts. Also fixes a broken anchor and adds a one-sentence compatibility caveat the same review flagged as cheap Minor fixes. Signed-off-by: Yaraslau Tamashevich --- docs/spec/forms/forms.md | 32 ++++++--- include/morph/forms/forms.hpp | 131 ++++++---------------------------- 2 files changed, 43 insertions(+), 120 deletions(-) diff --git a/docs/spec/forms/forms.md b/docs/spec/forms/forms.md index 85b95e58..b0ff9ef3 100644 --- a/docs/spec/forms/forms.md +++ b/docs/spec/forms/forms.md @@ -216,7 +216,7 @@ output of `glz::write_json_schema()` to add seven annotation groups: | Annotation | Scope | Contents | |---|---|---| -| `required` | Top-level | Array of field names that are **not** `std::optional<...>` and not listed in `A::optionalFields`. | +| `required` | Top-level, and every nested-aggregate object schema (see [Nested aggregates (recursive, cycle-guarded)](#nested-aggregates-recursive-cycle-guarded)) | Array of field names that are **not** `std::optional<...>` and not listed in `A::optionalFields`. | | `x-order` | Every property | The member's declaration index (0‑based), so a renderer lays fields out in declaration order regardless of JSON key ordering. | | `x-decimalPlaces` | `Quantity` properties | The field's declared precision (`Quantity::declaredDecimals`). | | `x-unitAlternatives` | `Quantity` properties | Convertible display/entry units derived from `UnitTraits::relations`, each with `{id, display, decimals, num, den}` — `id`/`display`/`decimals` come from the alternative unit's `UnitMeta`, and `num`/`den` are the exact alternative-to-canonical ratio. Omitted entirely when the field's unit declares no convertible units. | @@ -518,9 +518,15 @@ must resolve the `$ref` to see both: same unit type share one `$def` and therefore one `ExtUnits`. - **`x-order`, `x-decimalPlaces`, `x-unitAlternatives`, `x-optionsAction` / `x-optionValue` / `x-optionLabel` / `x-optionsDependsOn` are siblings of the - `$ref` on the property** — `mergeSchemaExtras` patches - `dom["properties"][name]`, which is the property node holding the `$ref`, - never the referenced `$def`. + `$ref` on *this* property** — `mergeSchemaExtras` patches + `dom["properties"][name]`, which is the property node holding the `$ref`. + This is still true for a `Quantity`/`Choice` property's own `$def` (the + `quantity_kg_per_m3`-style def shown above never gets `x-order`/`required`/ + title — only `ExtUnits` and glaze's own `type`/bounds/`description` live + there). It is **not** true for a *nested-aggregate* member's `$def`: see + [Nested aggregates (recursive, cycle-guarded)](#nested-aggregates-recursive-cycle-guarded) + below — that `$def` **does** get `required`/`x-order`/title/etc. patched + directly into it, the same as any other object schema. The **"Where"** column below names the node each key is written to. A renderer resolves the `$ref` into `$defs`, then merges: per-property `x-*` keys (from the @@ -530,7 +536,7 @@ below) `DynamicForm.qml`'s `resolveProp` does exactly this dual read. | Key | Where | JSON type | Meaning / renderer obligation | |---|---|---|---| -| `required` | top-level (object) | array of strings | Names of members that must be engaged before submit. A member is listed unless it is a `std::optional<...>`, appears in `A::optionalFields`, or is a `computedFields` destination (see the [Required-ness rule](#required-ness-rule)). Always emitted (an explicit `[]` when nothing is required). The renderer blocks submission until every listed field has a value. | +| `required` | top-level (object), and every nested-aggregate object schema (inlined property or `$defs` entry) — see [Nested aggregates (recursive, cycle-guarded)](#nested-aggregates-recursive-cycle-guarded) | array of strings | Names of members that must be engaged before submit. A member is listed unless it is a `std::optional<...>`, appears in `A::optionalFields`, or is a `computedFields` destination (see the [Required-ness rule](#required-ness-rule)). Always emitted (an explicit `[]` when nothing is required). The renderer blocks submission until every listed field has a value. | | `x-order` | property node (sibling of `$ref`) | non-negative integer | The member's 0-based **declaration index**. Renderers lay fields out in ascending `x-order`, not in JSON key order (object key order is not preserved across DOMs). | | `x-decimalPlaces` | property node (sibling of `$ref`) | non-negative integer | The field's *declared* precision (`Quantity::declaredDecimals`, unit default unless the type overrides it). The numeric input step / rounding granularity for entry in the canonical unit. **Enforced, not merely advisory:** the request/reply dispatch path retags each submitted `Quantity` to this precision before storing it (see [Advertised precision is enforced on dispatch](#advertised-precision-is-enforced-on-dispatch)). | | `x-unitAlternatives` | property node (sibling of `$ref`) | array of objects | Convertible display/entry units for the field, derived from `UnitTraits::relations`. **Omitted entirely** when the unit declares no convertible peers. Each element has the five subfields below. The renderer offers these as a unit selector and recomputes the entered value *exactly* on switch; the submitted payload is always in the canonical unit (the one named by `ExtUnits`). | @@ -1316,11 +1322,15 @@ what a nested-aggregate schema actually needs (per-field annotations) rather than becoming a general recursive-descent schema compiler that also re-derives layout/rules/computed-field semantics at every level. -**Purely additive.** An action with no nested-aggregate member has nothing -here to trigger on, so its generated schema is byte-for-byte unchanged. A -pre-existing action that *does* have a nested-aggregate member sees its -schema gain annotations it previously lacked — the whole point of this -feature — with no change to any of its flat top-level members. +**Purely additive, with one source-compatibility exception.** An action with +no nested-aggregate member has nothing here to trigger on, so its generated +schema is byte-for-byte unchanged. A pre-existing action that *does* have a +nested-aggregate member sees its schema gain annotations it previously +lacked — the whole point of this feature — with no change to any of its flat +top-level members. The one exception: an action with a self- or +mutually-referential nested-aggregate member (see the cycle-guard paragraph +above) now fails to *compile*, where it previously compiled (recursion used +to stop before reaching the cycle). No such action exists in this repo today. Every nested-aggregate type in the chain must be **default-constructible**, exactly like the top-level action type (see below): the recursion builds its @@ -1329,7 +1339,7 @@ own probe instance purely to enumerate its members via reflection. ### Scope: flat actions only (form layout, computed fields, and rules) `formLayout`/`fieldSpans` ([Layout & grouping](#layout--grouping)) and -`formRules` ([Cross-field rules](#cross-field-rules-x-rules)) are read only +`formRules` ([Cross-field rules](#cross-field-rules--the-x-rules-vocabulary)) are read only from the top-level action type — they are not consulted on a nested aggregate, no matter how deep `mergeSchemaExtras` otherwise recurses (see [Nested aggregates (recursive, cycle-guarded)](#nested-aggregates-recursive-cycle-guarded) diff --git a/include/morph/forms/forms.hpp b/include/morph/forms/forms.hpp index ea136168..3a0ef2a7 100644 --- a/include/morph/forms/forms.hpp +++ b/include/morph/forms/forms.hpp @@ -1566,11 +1566,12 @@ struct IsStdVector> : std::true_type { template concept ReflectableAggregate = glz::reflectable || glz::glaze_object_t; -/// @brief Applies the same title/`FieldMeta`/`Quantity`/`Choice`/widget/ -/// ranged-bounds annotations `mergeSchemaExtras`'s top-level pass -/// applies, to one property node. Shared by that top-level pass and -/// `annotateNestedAggregate` below (the one-level nested-aggregate pass) so -/// both apply identical per-member rules. +/// @brief Applies the title/`FieldMeta`/`Quantity`/`Choice`/widget/ +/// ranged-bounds annotations to one property node. Shared by +/// `mergeSchemaExtras`'s top-level pass and `annotateNestedAggregate` +/// below (the nested-aggregate recursion, to whatever depth the type +/// graph has) so both apply identical per-member rules -- this is the +/// single implementation of those rules; neither caller duplicates it. /// /// Deliberately excludes computed-field annotations (`x-computed`/ /// `x-readonly`) and `x-order`: computed fields are not supported inside a @@ -1805,8 +1806,18 @@ void annotateNestedAggregateRef(glz::generic_u64& dom, glz::generic_u64& propert if (propertyOrItems.contains("$ref")) { if (auto const* ref = propertyOrItems["$ref"].get_if()) { if (std::string_view{*ref}.starts_with(kDefsPrefix)) { - annotateNestedAggregate( - dom, dom["$defs"][std::string{ref->substr(kDefsPrefix.size())}]); + auto const key = std::string{ref->substr(kDefsPrefix.size())}; + // Checked, not indexed-and-hope: glz::generic_u64's object + // storage reallocates on insert, so indexing a missing key + // here would both fabricate a bogus empty $defs entry AND -- + // now that annotateNestedAggregate recurses -- risk dangling + // a `node` reference an enclosing frame still holds into this + // same $defs map. Well-formed glaze output never names a + // $defs key that doesn't exist, so this only changes behavior + // for malformed input, which is left untouched instead. + if (dom.contains("$defs") && dom["$defs"].contains(key)) { + annotateNestedAggregate(dom, dom["$defs"][key]); + } } } return; @@ -1874,108 +1885,10 @@ template property["x-computed"] = computedMeta; } - // Label/help/placeholder/read-only/hidden: an explicit FieldMeta - // entry overrides the inferred title and adds the rest; absent, every - // field still gets an inferred title and nothing else (Field - // metadata is additive/optional per gui_overview.md's versioning - // stance — a renderer that ignores these keys shows the raw wire key - // as the caption, no helper/placeholder text, every field editable - // and visible, exactly as before this feature). - const FieldMeta* fieldMeta = findFieldMeta(name); - std::string_view const declaredLabel = fieldMeta != nullptr ? fieldMeta->label : std::string_view{}; - property["title"] = declaredLabel.empty() ? inferTitle(name) : std::string{declaredLabel}; - if (fieldMeta != nullptr) { - if (!fieldMeta->help.empty()) { - property["description"] = std::string{fieldMeta->help}; - } - if (!fieldMeta->placeholder.empty()) { - property["x-placeholder"] = std::string{fieldMeta->placeholder}; - } - if (fieldMeta->readOnly) { - property["x-readonly"] = true; - } - if (fieldMeta->hidden) { - property["x-hidden"] = true; - } - if (!fieldMeta->i18nKey.empty()) { - property["x-i18nKey"] = std::string{fieldMeta->i18nKey}; - } - } - - if constexpr (units::isQuantity) { - // The field's *declared* precision: the unit default unless the - // field's type overrides it (Quantity). - property["x-decimalPlaces"] = std::uint64_t{Member::declaredDecimals}; - - // Convertible display/entry units with their exact ratios. - auto const alternatives = Member::unitAlternatives(); - if (!alternatives.empty()) { - glz::generic_u64::array_t list{}; - for (auto const& alternative : alternatives) { - auto const meta = - units::UnitTraits>::meta(alternative.unit); - glz::generic_u64 entry{}; - entry["id"] = std::string{meta.id}; - entry["display"] = std::string{meta.display}; - entry["decimals"] = std::uint64_t{meta.defaultDecimals}; - entry["num"] = alternative.num; - entry["den"] = alternative.den; - list.emplace_back(std::move(entry)); - } - property["x-unitAlternatives"] = list; - } - } - if constexpr (isChoice) { - // Which action serves the options, and which result-row fields - // carry the submitted value / display label. - property["x-optionsAction"] = std::string{Member::optionsAction()}; - property["x-optionValue"] = std::string{Member::valueField()}; - property["x-optionLabel"] = std::string{Member::labelField()}; - - // Sibling fields whose current values parameterise the options - // action (cascading picklists). Omitted entirely for an - // independent Choice, so the emitted schema is byte-for-byte - // unchanged from before this key existed. - if constexpr (!Member::optionsDependsOn().empty()) { - glz::generic_u64::array_t dependsOn{}; - for (auto const& parentName : Member::optionsDependsOn()) { - dependsOn.emplace_back(std::string{parentName}); - } - property["x-optionsDependsOn"] = dependsOn; - } - } - - // Widget hint: the field type's own widget() (e.g. Multiline, - // Ranged), overridden — if present — by a fieldMetadata-shaped entry - // naming this field. The override always wins over the type-derived - // default. - std::string_view widgetHint{}; - if constexpr (DeclaresWidget) { - widgetHint = Member::widget(); - } - if constexpr (HasFieldMetadataWidgets) { - if (auto const overrideWidget = widgetOverride(name); !overrideWidget.empty()) { - widgetHint = overrideWidget; - } - } - if (!widgetHint.empty()) { - property["x-widget"] = std::string{widgetHint}; - } - if constexpr (DeclaresRangedBounds) { - // The slider's control track: a UI hint, not a validation bound - // (glaze's own minimum/maximum, when present, stay authoritative - // for validation regardless of what x-widget ends up here). - using Bound = std::remove_cvref_t; - if constexpr (std::floating_point) { - property["x-min"] = static_cast(Member::min()); - property["x-max"] = static_cast(Member::max()); - property["x-step"] = static_cast(Member::step()); - } else { - property["x-min"] = static_cast(Member::min()); - property["x-max"] = static_cast(Member::max()); - property["x-step"] = static_cast(Member::step()); - } - } + // Label/title/FieldMeta/Quantity/Choice/widget/ranged-bounds: shared + // with the nested-aggregate recursion's per-member pass so both apply + // identical rules (see annotateBasicMemberProperty's doc comment). + annotateBasicMemberProperty(property, name); // Nested aggregates (recursive, cycle-guarded -- docs/spec/forms/forms.md, // "Nested aggregates (recursive, cycle-guarded)"): a member whose type From 014f75a8b3c8f5763c7d252a20f10465c785d48c Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Wed, 5 Aug 2026 15:56:58 +0300 Subject: [PATCH 13/13] docs: drop the nested-aggregate-recursion-depth implementation plan The plan document served its purpose during implementation; the spec (docs/spec/forms/forms.md) and git history are the lasting record. Signed-off-by: Yaraslau Tamashevich --- ...-08-05-nested-aggregate-recursion-depth.md | 825 ------------------ 1 file changed, 825 deletions(-) delete mode 100644 docs/superpowers/plans/2026-08-05-nested-aggregate-recursion-depth.md diff --git a/docs/superpowers/plans/2026-08-05-nested-aggregate-recursion-depth.md b/docs/superpowers/plans/2026-08-05-nested-aggregate-recursion-depth.md deleted file mode 100644 index f4eb6483..00000000 --- a/docs/superpowers/plans/2026-08-05-nested-aggregate-recursion-depth.md +++ /dev/null @@ -1,825 +0,0 @@ -# Nested-Aggregate Recursion Depth Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Replace `mergeSchemaExtras`'s one-level cap on nested-aggregate schema -annotation (`include/morph/forms/forms.hpp`) with cycle-guarded recursion to -whatever depth the type graph actually has. - -**Architecture:** Thread a variadic `Ancestors...` template parameter pack -(the chain of nested-aggregate types already being annotated on the current -path, starting with the action type) through `annotateNestedAggregate` and -`annotateNestedAggregateRef`. Factor the "is this member itself a nested -aggregate, and should I recurse into it" decision — previously duplicated -inline in `mergeSchemaExtras`'s loop and absent from `annotateNestedAggregate` -entirely (since it never went past one level) — into one new shared function, -`recurseIntoNestedAggregateIfAny`, used by both loops. That function's cycle -guard is a `static_assert` whose condition depends on the member's type and -the ancestor chain, so it only fires for the specific cyclic instantiation -that would otherwise recurse forever. - -**Tech Stack:** C++23, Glaze (JSON reflection/schema), Catch2 (tests), CMake + -Ninja, `clang-release` preset. - -## Global Constraints - -- Spec lives at `docs/spec/forms/forms.md`, section "Nested aggregates - (recursive, cycle-guarded)" (already written and committed — read it before - starting; this plan implements it verbatim). If any step here turns out to - conflict with that spec, the spec wins — update this plan's approach, not - the spec. -- Computed fields, `formLayout`/`fieldSpans`, and `formRules` stay - **top-level-only** regardless of nesting depth — out of scope for this - change, do not touch that logic. -- Doxygen's `WARN_AS_ERROR` docs build fails on any undocumented public - `@param`/`@tparam`/`@return` — and this codebase already fully documents - even `morph::forms::detail`-namespace functions (see the existing - `annotateNestedAggregate`/`annotateNestedAggregateRef` comments being - replaced below), so every new/changed function needs complete Doxygen - comments too. Reproduce the docs build locally with: - `cmake -S . -B build -G Ninja -DMORPH_BUILD_DOCUMENTATION=ON -DMORPH_BUILD_TESTS=OFF -DMORPH_BUILD_EXAMPLES=OFF` - then `cmake --build build --target doc`. -- Build/test commands (see `README.md`, "Building & dependencies"): - `cmake --preset clang-release`, then - `cmake --build build/clang-release --target morph_tests`, then - `./build/clang-release/tests/morph_tests`. `VCPKG_ROOT` must be set in the - environment (it already is: `/Users/yaraslau/.local/share/vcpkg`). -- Filter to just this file's tests with Catch2's tag filter: - `./build/clang-release/tests/morph_tests "[forms][nested]"`. -- Work happens in the existing worktree at - `/Users/yaraslau/repo/morph/.claude/worktrees/agent-a504219143d710713` - (branch `feature/25-nested-aggregate-forms`, already rebased onto latest - `origin/master`). Every `git`/`cmake`/build command in this plan assumes - that directory is the working directory. - ---- - -### Task 1: Extend the test fixtures and add a failing "recursion continues past one level" test - -**Files:** -- Modify: `tests/test_nested_forms.cpp:1-13` (file header comment) -- Modify: `tests/test_nested_forms.cpp:75-84` (`Provenance`/`DeepSpecimen` fixtures) -- Modify: `tests/test_nested_forms.cpp:348-377` (replace the "depth cap" test) - -**Interfaces:** -- Consumes: `morph::forms::schemaJson()` (existing public API, unchanged - signature), the file's existing `resolveNestedSchema`/`requiredNamesOf` - helpers (unchanged). -- Produces: nothing new consumed by later tasks — this task only changes test - code. Task 2 makes the test added here pass. - -- [ ] **Step 1: Update the file header comment** - -Replace: - -```cpp -// SPDX-License-Identifier: Apache-2.0 -// -// Coverage for issue #25: form generation recurses one level into a -// nested-aggregate member's object schema -- a directly-nested struct member -// or a `std::vector` repeated aggregate -- applying the same -// title/x-order/required/widget rules the top level already applies, instead -// of leaving it entirely unannotated. Two distinct schema shapes exist for a -// nested aggregate (see forms.hpp's `annotateNestedAggregateRef`): glaze -// *inlines* the object schema directly into the property when the nested -// type is used exactly once in the whole schema, and *deduplicates* it via a -// shared `$defs` entry (referenced by `$ref`) when it is used two or more -// times. Both are exercised below. Recursion stops after one level (see -// docs/spec/forms/forms.md, "Nested aggregates (one level)"). -``` - -With: - -```cpp -// SPDX-License-Identifier: Apache-2.0 -// -// Coverage for issue #25: form generation recurses into a nested-aggregate -// member's object schema -- a directly-nested struct member or a -// `std::vector` repeated aggregate -- applying the same -// title/x-order/required/widget rules the top level already applies, instead -// of leaving it entirely unannotated. Two distinct schema shapes exist for a -// nested aggregate (see forms.hpp's `annotateNestedAggregateRef`): glaze -// *inlines* the object schema directly into the property when the nested -// type is used exactly once in the whole schema, and *deduplicates* it via a -// shared `$defs` entry (referenced by `$ref`) when it is used two or more -// times. Both are exercised below. Recursion continues to whatever depth the -// type graph actually has, stopping only at a genuine cycle -- a compile-time -// `static_assert`, not something this runtime test suite can exercise -// directly (see docs/spec/forms/forms.md, "Nested aggregates (recursive, -// cycle-guarded)"). -``` - -- [ ] **Step 2: Extend the fixtures to a three-level chain and add a self-referential standalone type** - -Replace: - -```cpp -struct Provenance { - std::string collectedBy; -}; - -// A nested aggregate whose own member is itself a nested aggregate -- the -// depth-limit case: `provenance`'s own sub-members must stay unannotated. -struct DeepSpecimen { - double massDry = 0.0; - Provenance provenance; -}; -``` - -With: - -```cpp -struct Origin { - std::string country; -}; - -// Three levels deep: DeepSpecimen -> Provenance -> Origin. Provenance's own -// member (origin) is itself a nested aggregate too -- proving recursion -// continues past one level. -struct Provenance { - std::string collectedBy; - Origin origin; -}; - -struct DeepSpecimen { - double massDry = 0.0; - Provenance provenance; -}; - -// A self-referential nested-aggregate type (a tree node). Never passed to -// morph::forms::schemaJson() anywhere in this file -- neither as the -// top-level action type itself nor nested inside another action's member -- -// either use would trip forms.hpp's cycle-guard static_assert (see -// docs/spec/forms/forms.md, "Nested aggregates (recursive, cycle-guarded)"). -// This only proves the type itself, and ordinary glaze JSON round-tripping -// over it, are completely unaffected by that guard. -struct TreeNode { - std::string name; - std::vector children; -}; -``` - -- [ ] **Step 3: Add the `using` declarations for the new types** - -In the `using nestedforms::...;` block (currently lines 199-210), add two -lines, keeping the existing alphabetical order: - -```cpp -using nestedforms::Origin; -``` - -(insert alphabetically between `using nestedforms::DeepSpecimen;` and -`using nestedforms::PlainMetaRecord;`) - -```cpp -using nestedforms::TreeNode; -``` - -(`TreeNode` sorts after `Specimen` — 'S' < 'T' — so append it as the new -last line, after the current last entry, `using nestedforms::Specimen;`) - -- [ ] **Step 4: Replace the "depth cap" test with a three-level recursion test** - -Replace the entire section (from the `// ── Depth limit: exactly one level ──` -comment through the end of that `TEST_CASE`, i.e. the old lines 348-377): - -```cpp -// ── Depth limit: exactly one level ────────────────────────────────────────── - -TEST_CASE("Forms::SchemaJson::NestedAggregate: recursion stops after one level (depth cap)", - "[forms][nested][issue25]") { - auto const schema = morph::forms::schemaJson(); - glz::generic_u64 dom{}; - REQUIRE_FALSE(glz::read_json(dom, schema)); - - REQUIRE(dom["properties"].contains("sample")); - auto const& outerDef = resolveNestedSchema(dom, dom["properties"]["sample"]); - - // Level 1 (DeepSpecimen's own members) IS annotated. - CHECK(outerDef["properties"]["massDry"].contains("x-order")); - CHECK(outerDef["properties"]["massDry"].contains("title")); - REQUIRE(outerDef.contains("required")); - - // Level 2 (Provenance, nested inside DeepSpecimen) is NOT annotated: its - // object schema has no "required" key and its own properties carry no - // x-order/title -- exactly today's pre-existing behaviour for anything - // deeper than one level. "provenance" itself (a level-1 member) DOES get - // an x-order/title on its own property node, same as any other member; - // only its *inner* properties are left untouched. - REQUIRE(outerDef["properties"].contains("provenance")); - CHECK(outerDef["properties"]["provenance"].contains("x-order")); - CHECK(outerDef["properties"]["provenance"].contains("title")); - auto const& innerDef = resolveNestedSchema(dom, outerDef["properties"]["provenance"]); - CHECK_FALSE(innerDef.contains("required")); - CHECK_FALSE(innerDef["properties"]["collectedBy"].contains("x-order")); - CHECK_FALSE(innerDef["properties"]["collectedBy"].contains("title")); -} -``` - -With: - -```cpp -// ── Recursion continues past one level (no depth cap) ─────────────────────── - -TEST_CASE("Forms::SchemaJson::NestedAggregate: recursion continues past one level to whatever depth exists", - "[forms][nested][issue25]") { - auto const schema = morph::forms::schemaJson(); - glz::generic_u64 dom{}; - REQUIRE_FALSE(glz::read_json(dom, schema)); - - REQUIRE(dom["properties"].contains("sample")); - auto const& level1Def = resolveNestedSchema(dom, dom["properties"]["sample"]); - - // Level 1 (DeepSpecimen's own members) is annotated. - CHECK(level1Def["properties"]["massDry"].contains("x-order")); - CHECK(level1Def["properties"]["massDry"].contains("title")); - REQUIRE(level1Def.contains("required")); - - // Level 2 (Provenance, nested inside DeepSpecimen) is now annotated too -- - // both its own property node (x-order/title, same as any level-1 member) - // and, unlike the old one-level cap, its own "required" array. - REQUIRE(level1Def["properties"].contains("provenance")); - CHECK(level1Def["properties"]["provenance"].contains("x-order")); - CHECK(level1Def["properties"]["provenance"].contains("title")); - auto const& level2Def = resolveNestedSchema(dom, level1Def["properties"]["provenance"]); - REQUIRE(level2Def.contains("required")); - CHECK(level2Def["properties"]["collectedBy"].contains("x-order")); - CHECK(level2Def["properties"]["collectedBy"].contains("title")); - - // Level 3 (Origin, nested inside Provenance) is annotated too -- proving - // recursion does not stop at two levels either. - REQUIRE(level2Def["properties"].contains("origin")); - CHECK(level2Def["properties"]["origin"].contains("x-order")); - CHECK(level2Def["properties"]["origin"].contains("title")); - auto const& level3Def = resolveNestedSchema(dom, level2Def["properties"]["origin"]); - REQUIRE(level3Def.contains("required")); - CHECK(level3Def["properties"]["country"].contains("x-order")); - CHECK(level3Def["properties"]["country"].contains("title")); -} -``` - -- [ ] **Step 5: Configure the build (first time only) and build the test binary** - -```bash -cd /Users/yaraslau/repo/morph/.claude/worktrees/agent-a504219143d710713 -export VCPKG_ROOT=/Users/yaraslau/.local/share/vcpkg -cmake --preset clang-release -cmake --build build/clang-release --target morph_tests -``` - -- [ ] **Step 6: Run the new test and confirm it fails** - -```bash -./build/clang-release/tests/morph_tests "recursion continues past one level to whatever depth exists" -``` - -Expected: **FAIL**. `level2Def` will not contain `"required"` (current code -leaves `Provenance`'s own object schema — and everything past it — completely -unannotated, since it never recurses past `DeepSpecimen`). `REQUIRE(level2Def.contains("required"))` -is expected to trip first. - -- [ ] **Step 7: Commit** - -```bash -git add tests/test_nested_forms.cpp -git commit -m "$(cat <<'EOF' -test(forms): extend nested-aggregate fixtures to three levels - -Replaces the one-level "depth cap" test with a failing test proving -recursion should continue past one level: DeepSpecimen -> Provenance -> -Origin, three levels deep. Also adds a self-referential TreeNode -fixture (never passed to schemaJson() here -- that would trip the -forthcoming cycle guard) proving such a type is unaffected on its own. - -Signed-off-by: Yaraslau Tamashevich -EOF -)" -``` - ---- - -### Task 2: Implement cycle-guarded unbounded-depth recursion in `forms.hpp` - -**Files:** -- Modify: `include/morph/forms/forms.hpp:1559-1564` (`ReflectableAggregate` doc comment) -- Modify: `include/morph/forms/forms.hpp:1660-1733` (replace `annotateNestedAggregate`/`annotateNestedAggregateRef`, add `recurseIntoNestedAggregateIfAny`) -- Modify: `include/morph/forms/forms.hpp:1896-1915` (`mergeSchemaExtras`'s nested-aggregate branch) - -**Interfaces:** -- Consumes: `ReflectableAggregate` concept, `IsStdVector` trait, - `annotateBasicMemberProperty(property, name)`, - `forEachNamedMember`, `declaredOptional(name)`, `isStdOptional` — all - pre-existing, all unchanged (defined earlier in the same file). -- Produces: - - `template void annotateNestedAggregate(glz::generic_u64& dom, glz::generic_u64& node)` - — **signature changed**: gains `dom` as its first parameter and an - `Ancestors...` pack. Anything outside this file calling it directly would - need updating; nothing does (only `annotateNestedAggregateRef` and the - tests in `test_nested_forms.cpp` — which call `annotateNestedAggregateRef`, - not this — reference it). - - `template void annotateNestedAggregateRef(glz::generic_u64& dom, glz::generic_u64& propertyOrItems)` - — signature gains the `Ancestors...` pack (variadic, so existing - single-template-argument call sites, e.g. - `annotateNestedAggregateRef(dom, property)` in - `test_nested_forms.cpp:519,528,539`, keep compiling unchanged with an - empty `Ancestors` pack). - - `template void recurseIntoNestedAggregateIfAny(glz::generic_u64& dom, glz::generic_u64& property)` - — new function, used by both `mergeSchemaExtras` and - `annotateNestedAggregate`. - -- [ ] **Step 1: Update `ReflectableAggregate`'s doc comment** - -Replace: - -```cpp -/// @brief Concept: `T` is glaze-reflectable as a JSON object -- the same test -/// that decides whether glaze emits a member into `$defs`/`$ref` -/// rather than inline. Shared by the one-level nested-aggregate -/// recursion below and `reconcileDeclaredPrecision` elsewhere. -template -concept ReflectableAggregate = glz::reflectable || glz::glaze_object_t; -``` - -With: - -```cpp -/// @brief Concept: `T` is glaze-reflectable as a JSON object -- the same test -/// that decides whether glaze emits a member into `$defs`/`$ref` -/// rather than inline. Shared by the cycle-guarded nested-aggregate -/// recursion below and `reconcileDeclaredPrecision` elsewhere. -template -concept ReflectableAggregate = glz::reflectable || glz::glaze_object_t; -``` - -- [ ] **Step 2: Replace `annotateNestedAggregate`/`annotateNestedAggregateRef` with the cycle-guarded, `Ancestors`-threaded versions, plus the new shared helper** - -Replace this entire block (from the blank line right after -`annotateBasicMemberProperty`'s closing brace through the closing brace of the -old `annotateNestedAggregateRef`): - -```cpp - -/// @brief One level of recursion (see `docs/spec/forms/forms.md`, "Nested -/// aggregates (one level)"): annotates @p node -- the object-schema -/// DOM node for a nested-aggregate member -- applying `required` and -/// `annotateBasicMemberProperty`'s rules to its own properties. -/// -/// @p node is @e which DOM node depends on how many places in the whole -/// schema reference `Sub`: glaze **inlines** the object schema directly into -/// the referencing property when `Sub` is used exactly once (so @p node -/// *is* that property node), but **deduplicates** via `$defs`/`$ref` when -/// `Sub` is used two or more times (so @p node is the shared `$defs` entry, -/// resolved by the caller). Both forms have the identical `{"properties": -/// {...}}` shape this function needs, so one implementation handles both -- -/// see the call site in `mergeSchemaExtras` for how @p node is resolved. -/// -/// Deliberately does **not** recurse again: if `Sub` itself has a member that -/// is itself an aggregate, that member is left exactly as glaze emitted it -- -/// unannotated, matching today's behaviour beyond one level. This bounds the -/// generator to one level of nesting, as `docs/spec/forms/forms.md` documents. -/// Computed fields, `formLayout`/`fieldSpans`, and `formRules` also stay -/// top-level-only; a nested `Sub` declaring any of those has no effect here. -/// -/// @tparam Sub Nested aggregate type (default-constructible, glaze-reflectable -/// -- the same requirements the top-level action type already has). -/// @param node The object-schema DOM node to annotate in place (see above). -template -void annotateNestedAggregate(glz::generic_u64& node) { - Sub probe{}; - glz::generic_u64::array_t requiredNames{}; - forEachNamedMember(probe, [&](std::string_view name, const auto& member) { - using Member = std::remove_cvref_t; - if (!(isStdOptional || declaredOptional(name))) { - requiredNames.emplace_back(std::string{name}); - } - auto& property = node["properties"][std::string{name}]; - property["x-order"] = std::uint64_t{I}; - annotateBasicMemberProperty(property, name); - }); - // Idempotent if two members (or two actions sharing this schema call) - // resolve to the same $defs entry: re-deriving the identical required - // array is harmless. - node["required"] = requiredNames; -} - -/// @brief Resolves the object-schema DOM node for a nested-aggregate member, -/// given the property (or array `items`) node glaze wrote for it, and -/// annotates it via `annotateNestedAggregate`. -/// -/// Handles both forms `Sub` can take in the schema (see -/// `annotateNestedAggregate`'s doc comment): a `$ref` into `$defs` (`Sub` used -/// 2+ times somewhere in the schema) resolves to that shared def; anything -/// else is assumed to be the inlined object schema itself (`Sub` used exactly -/// once). A property that is neither -- glaze emitted something other than an -/// object schema for a type this function's caller already confirmed is a -/// `ReflectableAggregate` -- is left untouched rather than guessed at. -/// @tparam Sub Nested aggregate type, as `annotateNestedAggregate` requires. -/// @param dom The whole schema DOM (so a `$ref`'s `$defs` entry can be found). -/// @param propertyOrItems The property node itself (single nested member) or its -/// array `items` node (`std::vector` member). -template -void annotateNestedAggregateRef(glz::generic_u64& dom, glz::generic_u64& propertyOrItems) { - constexpr std::string_view kDefsPrefix = "#/$defs/"; - if (propertyOrItems.contains("$ref")) { - if (auto const* ref = propertyOrItems["$ref"].get_if()) { - if (std::string_view{*ref}.starts_with(kDefsPrefix)) { - annotateNestedAggregate(dom["$defs"][std::string{ref->substr(kDefsPrefix.size())}]); - } - } - return; - } - if (propertyOrItems.contains("properties")) { - annotateNestedAggregate(propertyOrItems); - } -} -``` - -With: - -```cpp - -// annotateNestedAggregate, annotateNestedAggregateRef, and -// recurseIntoNestedAggregateIfAny are mutually recursive (each nested -// aggregate found while annotating one may itself contain another), so all -// three need forward declarations before any of their bodies can reference -// the others. -template -void annotateNestedAggregate(glz::generic_u64& dom, glz::generic_u64& node); - -template -void annotateNestedAggregateRef(glz::generic_u64& dom, glz::generic_u64& propertyOrItems); - -template -void recurseIntoNestedAggregateIfAny(glz::generic_u64& dom, glz::generic_u64& property); - -/// @brief Recurses into @p property's own object schema if @p Member (or, for -/// `std::vector`, its element type) is itself a -/// `ReflectableAggregate` -- the single decision point shared by -/// `mergeSchemaExtras`'s top-level loop and `annotateNestedAggregate`'s -/// own loop, so the cycle guard below has exactly one implementation. -/// -/// @p Ancestors is the chain of nested-aggregate types already being -/// annotated on the current path, **including** the type that declares this -/// member (the caller appends its own `Sub`/`A` before calling this). If the -/// type to recurse into matches any entry already on that chain, recursing -/// further would eventually re-enter this same instantiation and try to do -/// so again -- forever. Rather than let that happen, a `static_assert` (whose -/// condition depends on @p Member and @p Ancestors, so it only fires for the -/// specific cyclic instantiation, not every use of this generator) rejects it -/// at compile time instead: a self-referential nested-aggregate type (e.g. -/// `struct Node { std::vector children; };`), or a mutual reference -/// between two distinct types, fails to build with a clear message rather -/// than exhausting the compiler's template-instantiation depth. This only -/// rejects genuine cycles -- the same type reused from two unrelated places -/// in the schema (a "diamond") is not on either path's ancestor chain and -/// recurses normally into both. See `docs/spec/forms/forms.md`, "Nested -/// aggregates (recursive, cycle-guarded)". -/// @tparam Member The static type of the member `annotateBasicMemberProperty` -/// was just applied to. -/// @tparam Ancestors The ancestor chain so far, ending with the type that -/// declares this member. -/// @param dom The whole schema DOM (so a `$ref`'s `$defs` entry can be found). -/// @param property The property node for this member (or, for `std::vector`, -/// the property whose `"items"` node is the one to check). -template -void recurseIntoNestedAggregateIfAny(glz::generic_u64& dom, glz::generic_u64& property) { - if constexpr (ReflectableAggregate) { - if constexpr ((std::same_as || ...)) { - static_assert(!(std::same_as || ...), - "morph::forms: cyclic nested-aggregate schema -- this member's type already " - "appears in its own chain of enclosing nested-aggregate types (a self- or " - "mutually-referential type). Recursion depth is otherwise unbounded, but cycles " - "are not supported: restructure the domain type (flatten the self-reference, or " - "represent the recursive edge as an opaque id instead of a nested value)."); - } else { - annotateNestedAggregateRef(dom, property); - } - } else if constexpr (IsStdVector::value && - ReflectableAggregate::ValueType>) { - using ItemType = typename IsStdVector::ValueType; - if constexpr ((std::same_as || ...)) { - static_assert(!(std::same_as || ...), - "morph::forms: cyclic nested-aggregate schema -- this std::vector member's " - "element type already appears in its own chain of enclosing nested-aggregate " - "types (a self- or mutually-referential type). Recursion depth is otherwise " - "unbounded, but cycles are not supported: restructure the domain type (flatten " - "the self-reference, or represent the recursive edge as an opaque id instead of " - "a nested value)."); - } else if (property.contains("items")) { - annotateNestedAggregateRef(dom, property["items"]); - } - } -} - -/// @brief Annotates @p node -- the object-schema DOM node for a -/// nested-aggregate member -- applying `required` and -/// `annotateBasicMemberProperty`'s rules to its own properties, then -/// recursing into any of *its* members that are themselves nested -/// aggregates (see `recurseIntoNestedAggregateIfAny`), to whatever -/// depth the type graph actually has. -/// -/// @p node is @e which DOM node depends on how many places in the whole -/// schema reference `Sub`: glaze **inlines** the object schema directly into -/// the referencing property when `Sub` is used exactly once (so @p node -/// *is* that property node), but **deduplicates** via `$defs`/`$ref` when -/// `Sub` is used two or more times (so @p node is the shared `$defs` entry, -/// resolved by the caller). Both forms have the identical `{"properties": -/// {...}}` shape this function needs, so one implementation handles both -- -/// see the call site in `mergeSchemaExtras` for how @p node is resolved. -/// -/// Computed fields, `formLayout`/`fieldSpans`, and `formRules` stay -/// top-level-only regardless of depth; a nested `Sub` declaring any of those -/// has no effect here. -/// -/// @tparam Sub Nested aggregate type (default-constructible, glaze-reflectable -/// -- the same requirements the top-level action type already has). -/// @tparam Ancestors The ancestor chain so far, ending with `Sub` itself, passed -/// through to `recurseIntoNestedAggregateIfAny` for each of -/// `Sub`'s own members (see that function's doc comment). -/// @param dom The whole schema DOM (so a deeper `$ref`'s `$defs` entry can be found). -/// @param node The object-schema DOM node to annotate in place (see above). -template -void annotateNestedAggregate(glz::generic_u64& dom, glz::generic_u64& node) { - Sub probe{}; - glz::generic_u64::array_t requiredNames{}; - forEachNamedMember(probe, [&](std::string_view name, const auto& member) { - using Member = std::remove_cvref_t; - if (!(isStdOptional || declaredOptional(name))) { - requiredNames.emplace_back(std::string{name}); - } - auto& property = node["properties"][std::string{name}]; - property["x-order"] = std::uint64_t{I}; - annotateBasicMemberProperty(property, name); - recurseIntoNestedAggregateIfAny(dom, property); - }); - // Idempotent if two members (or two actions sharing this schema call) - // resolve to the same $defs entry: re-deriving the identical required - // array is harmless. - node["required"] = requiredNames; -} - -/// @brief Resolves the object-schema DOM node for a nested-aggregate member, -/// given the property (or array `items`) node glaze wrote for it, and -/// annotates it via `annotateNestedAggregate`. -/// -/// Handles both forms `Sub` can take in the schema (see -/// `annotateNestedAggregate`'s doc comment): a `$ref` into `$defs` (`Sub` used -/// 2+ times somewhere in the schema) resolves to that shared def; anything -/// else is assumed to be the inlined object schema itself (`Sub` used exactly -/// once). A property that is neither -- glaze emitted something other than an -/// object schema for a type this function's caller already confirmed is a -/// `ReflectableAggregate` -- is left untouched rather than guessed at. -/// @tparam Sub Nested aggregate type, as `annotateNestedAggregate` requires. -/// @tparam Ancestors The ancestor chain so far (excluding `Sub`), forwarded -/// to `annotateNestedAggregate` unchanged. -/// @param dom The whole schema DOM (so a `$ref`'s `$defs` entry can be found). -/// @param propertyOrItems The property node itself (single nested member) or its -/// array `items` node (`std::vector` member). -template -void annotateNestedAggregateRef(glz::generic_u64& dom, glz::generic_u64& propertyOrItems) { - constexpr std::string_view kDefsPrefix = "#/$defs/"; - if (propertyOrItems.contains("$ref")) { - if (auto const* ref = propertyOrItems["$ref"].get_if()) { - if (std::string_view{*ref}.starts_with(kDefsPrefix)) { - annotateNestedAggregate( - dom, dom["$defs"][std::string{ref->substr(kDefsPrefix.size())}]); - } - } - return; - } - if (propertyOrItems.contains("properties")) { - annotateNestedAggregate(dom, propertyOrItems); - } -} -``` - -- [ ] **Step 3: Simplify `mergeSchemaExtras`'s nested-aggregate branch to call the new shared helper** - -Replace: - -```cpp - // Nested aggregates (one level -- docs/spec/forms/forms.md, "Nested - // aggregates (one level)"): a member whose type is itself a - // reflectable aggregate gets an object schema from glaze -- either - // inlined directly into this property (the type is used exactly once - // in the whole schema) or shared via `$defs`/`$ref` (used 2+ times); - // `annotateNestedAggregateRef` resolves whichever form it is. Recurse - // one level so that object schema's own members get `x-order`/ - // `required`/title/Quantity/Choice/widget annotations too, instead of - // being silently unannotated. Purely additive: an action with no - // nested aggregate member has nothing here to trigger on, so its - // schema is byte-for-byte unchanged. - if constexpr (ReflectableAggregate) { - annotateNestedAggregateRef(dom, property); - } else if constexpr (IsStdVector::value && - ReflectableAggregate::ValueType>) { - using ItemType = typename IsStdVector::ValueType; - if (property.contains("items")) { - annotateNestedAggregateRef(dom, property["items"]); - } - } -``` - -With: - -```cpp - // Nested aggregates (recursive, cycle-guarded -- docs/spec/forms/forms.md, - // "Nested aggregates (recursive, cycle-guarded)"): a member whose type - // is itself a reflectable aggregate gets an object schema from glaze -- - // either inlined directly into this property (the type is used exactly - // once in the whole schema) or shared via `$defs`/`$ref` (used 2+ - // times). `recurseIntoNestedAggregateIfAny` resolves whichever form it - // is and recurses so that object schema's own members get - // `x-order`/`required`/title/Quantity/Choice/widget annotations too, - // however deep the type graph goes (guarding against cycles at compile - // time -- see that function's doc comment). Purely additive: an action - // with no nested aggregate member has nothing here to trigger on, so - // its schema is byte-for-byte unchanged. - recurseIntoNestedAggregateIfAny(dom, property); -``` - -- [ ] **Step 4: Rebuild and run the full nested-forms test suite** - -```bash -cmake --build build/clang-release --target morph_tests -./build/clang-release/tests/morph_tests "[forms][nested]" -``` - -Expected: **PASS** — every test tagged `[forms][nested]`, including the new -three-level test from Task 1 and the pre-existing idempotence/`$ref`/inline/ -`FieldMeta`/`Quantity`/`Choice`/`declaredOptional`/defensive-fallback tests -(none of their expectations changed; the earlier levels' behavior is -unaffected by extending recursion further). - -- [ ] **Step 5: Run the complete test suite to check for regressions elsewhere** - -```bash -./build/clang-release/tests/morph_tests -``` - -Expected: **PASS** — no other test exercises `forms.hpp`'s nested-aggregate -path with a type graph deep enough to be affected, so this is a regression -check, not expected to surface anything new. - -- [ ] **Step 6: Commit** - -```bash -git add include/morph/forms/forms.hpp -git commit -m "$(cat <<'EOF' -forms: recurse into nested aggregates to any depth, not just one - -Threads an Ancestors... template parameter pack through -annotateNestedAggregate/annotateNestedAggregateRef -- the chain of -nested-aggregate types already being annotated on the current path, -starting with the action type. Factors the "is this member itself a -nested aggregate, and should I recurse into it" decision (previously -duplicated inline in mergeSchemaExtras's loop, and entirely absent from -annotateNestedAggregate since it never went past one level) into one -shared recurseIntoNestedAggregateIfAny, used by both loops. - -A member whose type -- or, for std::vector, Sub -- already appears -on the ancestor chain would recurse into this same instantiation again, -forever. Rather than let that happen, a static_assert (dependent on the -member type and the chain, so it only fires for the actual cyclic -instantiation) rejects a self- or mutually-referential nested-aggregate -type graph at compile time instead. - -See docs/spec/forms/forms.md, "Nested aggregates (recursive, -cycle-guarded)". - -Signed-off-by: Yaraslau Tamashevich -EOF -)" -``` - ---- - -### Task 3: Add the standalone self-referential-type test and verify the docs build - -**Files:** -- Modify: `tests/test_nested_forms.cpp` (add one `TEST_CASE`, after the last - existing test case in the file) - -**Interfaces:** -- Consumes: `nestedforms::TreeNode` (added in Task 1, Step 2), `glz::write_json`, - `glz::read_json` (Glaze's own API, already used elsewhere in this test - suite — see `tests/test_bridge_fixes.cpp:252` for the `.value_or(std::string{})` - pattern this step follows). -- Produces: nothing consumed by anything later — this is the plan's last task. - -- [ ] **Step 1: Add the standalone self-referential-type test** - -Append to the end of `tests/test_nested_forms.cpp`: - -```cpp - -// ── Self-referential nested-aggregate type, standalone ───────────────────── - -TEST_CASE("Forms::SchemaJson::NestedAggregate: a self-referential nested-aggregate type round-trips fine on its own", - "[forms][nested][issue25]") { - // TreeNode is never passed to morph::forms::schemaJson() in this file - // -- see its doc comment. This only proves the type itself, and ordinary - // glaze JSON round-tripping over it, are completely unaffected by - // forms.hpp's cycle-guard static_assert, which fires only when a type - // like this is actually nested under some schemaJson() instantiation. - TreeNode root{}; - root.name = "root"; - TreeNode child{}; - child.name = "child"; - root.children.push_back(child); - - std::string const json = glz::write_json(root).value_or(std::string{}); - REQUIRE_FALSE(json.empty()); - - TreeNode decoded{}; - REQUIRE_FALSE(glz::read_json(decoded, json)); - CHECK(decoded.name == "root"); - REQUIRE(decoded.children.size() == 1); - CHECK(decoded.children[0].name == "child"); -} -``` - -- [ ] **Step 2: Rebuild and run** - -```bash -cmake --build build/clang-release --target morph_tests -./build/clang-release/tests/morph_tests "[forms][nested]" -``` - -Expected: **PASS**, including the new test. - -- [ ] **Step 3: Verify the Doxygen docs build succeeds with the new/changed comments** - -```bash -cmake -S . -B build/docs -G Ninja -DMORPH_BUILD_DOCUMENTATION=ON -DMORPH_BUILD_TESTS=OFF -DMORPH_BUILD_EXAMPLES=OFF -cmake --build build/docs --target doc -``` - -Expected: build succeeds (exit code 0). If it fails on a missing -`@param`/`@tparam`/`@return`, compare the failing function's signature against -Task 2 Step 2/3's replacement text above and add the missing tag — every -parameter and template parameter introduced there already has one documented, -so a failure here means a transcription slip, not a design gap. - -- [ ] **Step 4: Commit (only if Step 3 required a fix)** - -```bash -git add include/morph/forms/forms.hpp -git commit -m "$(cat <<'EOF' -forms: fix missing Doxygen tag on nested-aggregate recursion helper - -Signed-off-by: Yaraslau Tamashevich -EOF -)" -``` - -If Step 3 passed cleanly with no changes needed, skip this commit — -Step 1's test addition was already committed as part of a normal -add-test-then-verify cycle; commit just that: - -```bash -git add tests/test_nested_forms.cpp -git commit -m "$(cat <<'EOF' -test(forms): cover a self-referential nested-aggregate type standalone - -Proves TreeNode -- a tree-shaped, self-referential nested-aggregate -type -- round-trips through glaze JSON encode/decode normally on its -own. It is never passed to schemaJson() in this file; doing so -would trip forms.hpp's cycle-guard static_assert, which this test -suite has no harness to exercise directly (see -docs/spec/forms/forms.md, "Nested aggregates (recursive, -cycle-guarded)"). - -Signed-off-by: Yaraslau Tamashevich -EOF -)" -``` - ---- - -## Self-Review Notes - -- **Spec coverage:** Every behavior the spec (`docs/spec/forms/forms.md`, - "Nested aggregates (recursive, cycle-guarded)") describes has a - corresponding task: unbounded acyclic recursion depth (Task 1's test, Task - 2's implementation), the cycle-guard `static_assert` and its scoping to only - the offending instantiation (Task 2 Step 2's doc comment and code), the - "diamond is not a cycle" guarantee (unchanged `$defs`/`$ref` dedup logic, - covered by the pre-existing `$ref`-form tests that keep passing in Task 2 - Step 4), computed-fields/`formLayout`/`formRules` staying top-level-only - (untouched code, no task needed), and "a self-referential type not nested - under an action compiles fine" (Task 1 Step 2's `TreeNode` fixture, Task 3's - test). -- **Type consistency:** `annotateNestedAggregate`'s signature - (`glz::generic_u64& dom, glz::generic_u64& node` plus `Sub, Ancestors...`) - matches every call site introduced across Task 2 (`annotateNestedAggregateRef`'s - two internal calls; no other caller exists). `recurseIntoNestedAggregateIfAny`'s - signature matches both of its call sites (`mergeSchemaExtras`'s - ``, `annotateNestedAggregate`'s ``). - `annotateNestedAggregateRef`'s existing test call sites - (`test_nested_forms.cpp:519,528,539`, `annotateNestedAggregateRef(dom, property)`) - remain valid: `Ancestors...` is variadic and defaults to empty when only - `Sub` is given explicitly. -- **No placeholders:** every step above contains the literal before/after code - or the literal shell command to run; none say "add appropriate X" or defer - detail to another task.