diff --git a/CMakeLists.txt b/CMakeLists.txt index 1cf24690..aa5c5c99 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -225,6 +225,7 @@ target_sources(morph include/morph/forms/app.hpp include/morph/forms/flows.hpp include/morph/forms/layout.hpp + include/morph/forms/sections.hpp include/morph/forms/views.hpp include/morph/forms/widget_hints.hpp include/morph/journal/outbox.hpp diff --git a/docs/spec/forms/sections.md b/docs/spec/forms/sections.md new file mode 100644 index 00000000..c77d6557 --- /dev/null +++ b/docs/spec/forms/sections.md @@ -0,0 +1,303 @@ +# `morph::forms::SectionSet` — unordered sections + +`SectionSet` drives N independently editable action drafts +on one screen. Each section accumulates its own draft through `set<>` and +dispatches through `BridgeHandler::execute()` as soon as its own +`ActionValidator::ready` accepts it. There is no active section, no index, +and no sequence. Like [workflows_navigation.md](workflows_navigation.md)'s +wizards, this is additive metadata and client-side bookkeeping over the +dispatch path in [../core/bridge.md](../core/bridge.md) — no new wire format, +no new execution mode. + +## Contents + +- [The gap this closes](#the-gap-this-closes) +- [The `s-*` section-group document](#the-s--section-group-document) +- [C++ descriptors](#c-descriptors) +- [`SectionSet`](#sectionsetmodel-sections) +- [Prefill is a declaration, not a write](#prefill-is-a-declaration-not-a-write) +- [Concurrency and lifetime](#concurrency-and-lifetime) +- [Compile-time contract](#compile-time-contract) +- [Design decisions](#design-decisions) +- [Limitations](#limitations) +- [Testing](#testing) +- [Cross-references](#cross-references) + +## The gap this closes + +`FlowSession` has exactly one current step, and `FlowSession::set<>` throws +`std::logic_error` on a field belonging to any other. That is right for a +wizard and wrong for a screen whose blocks have no order — a settings page, a +tab strip, a column of cards — where a user may edit the third block first and +never touch the second. Before this layer such a screen either hand-wired one +`BridgeHandler::execute` call site per block, re-implementing draft +accumulation and the readiness gate each time, or misused a wizard and got a +`logic_error` for editing its own form out of order (morph#513). + +`SectionSet` keeps everything `FlowSession` does per action — per-action draft +accumulation, the readiness gate, result capture, error routing, the callback +lifetime gate — and drops only the position. + +Choose `FlowSession` when order carries meaning (a later step needs an earlier +step's result, or the user must not skip ahead) and `SectionSet` when it does +not. + +## The `s-*` section-group document + +`sectionGroupSchemaJson()` emits a small JSON document alongside each +section's ordinary action schema ([forms.md](forms.md)): + +```json +{ + "s-id": "AccountSettings", + "s-title": "Account settings", + "s-sections": [ + { "action": "UpdateProfile", "title": "Profile" }, + { "action": "UpdatePrefs", "title": "Preferences", + "prefill": { "profileId": "UpdateProfile.id" } } + ] +} +``` + +| Key | Where | JSON type | Meaning | +|---|---|---|---| +| `s-id` | top-level | string | The group's registered type-id (`SectionGroupTraits::typeId()`). | +| `s-title` | top-level | string | Human title for the whole group. | +| `s-sections` | top-level | array | The group's sections. Array order is declaration order and carries no meaning. | +| ↳ `action` | section | string | The section's registered action type-id (`ActionTraits::typeId()`). | +| ↳ `title` | section | string | Human title for the section (tab label, card header). | +| ↳ `prefill` | section | object | Field name → `"."` source path. Present only when the section declares at least one `Bind`. | + +There is deliberately **no index or order key**. A renderer arranges the +sections itself — as tabs, as a grid, collapsed, in a user-chosen order — and +a position in the wire format would suggest a sequence a section group does not +have. That is the one thing distinguishing this document from `w-*`, so +emitting an index would erase the distinction the type exists to make. + +`s-sections` is a JSON array only because JSON has no unordered collection that +also preserves duplicates-free keys usefully; consumers must not read meaning +into the order. + +## C++ descriptors + +```cpp +struct UpdateProfile { std::string name; bool validate() const { return !name.empty(); } }; +struct UpdatePrefs { std::int64_t profileId; std::string theme; + bool validate() const { return !theme.empty(); } }; + +BRIDGE_REGISTER_ACTION(SettingsModel, UpdateProfile, "UpdateProfile") +BRIDGE_REGISTER_ACTION(SettingsModel, UpdatePrefs, "UpdatePrefs") + +using ProfileSection = morph::forms::Section; +using PrefsSection = morph::forms::Section>; + +using AccountSettings = morph::forms::SectionGroup<"Account settings", + ProfileSection, PrefsSection>; +BRIDGE_REGISTER_SECTION_GROUP(AccountSettings, "AccountSettings") +``` + +| Type | Members | Meaning | +|---|---|---| +| `Section` | `action`, `binds`, `title()` | One section: a registered action, a display title, zero or more prefill declarations. | +| `SectionGroup` | `sections`, `title()` | A group of sections sharing one screen. | +| `SectionGroupTraits` | `typeId()` | Maps a group type to its stable string id. Specialise via `BRIDGE_REGISTER_SECTION_GROUP`; the default is a forward declaration, so using it unregistered is an incomplete-type error. | +| `Bind` | `field()`, `path()` | Shared with `morph::flows` — see [Prefill is a declaration, not a write](#prefill-is-a-declaration-not-a-write). | + +`Bind` and the declaration-walking helpers live in +`morph/forms/detail/session_common.hpp`, shared by both session types. +`morph::flows::Bind` remains as an alias, since that is the name existing +consumers write. + +Registration is metadata only. `BRIDGE_REGISTER_SECTION_GROUP` specialises a +traits template and registers nothing with the dispatcher — exactly as +`BRIDGE_REGISTER_WIZARD` does, and for the same reason ([../core/registry.md](../core/registry.md)). + +## `SectionSet` + +```cpp +morph::forms::SectionSet sections{handler}; + +sections.set<&UpdatePrefs::theme>("dark"); // fires UpdatePrefs -- no ordering +sections.set<&UpdateProfile::name>("ada"); // fires UpdateProfile +``` + +| Member | Contract | +|---|---| +| `SectionSet(handler, onError = nullptr)` | `handler` must outlive the set. `onError` receives every failed dispatch; when absent, failures are logged via `morph::log::logError` and never escape the completion. | +| `set(value)` | Assigns one field of its section's draft, then dispatches that section if `ActionValidator::ready` now accepts the draft. The field's action need only be *one of* the declared sections — there is no current one. | +| `reset()` | Clears section `A`'s draft to a default-constructed action. Touches no other section and dispatches nothing. Values already in `resolved()` stay: they describe what the model was told, which resetting an editor does not undo. | +| `draft()` | Returns a copy of section `A`'s draft, taken under the lock. | +| `resolved(path)` | Returns the JSON-encoded value captured at `"."`, or `std::nullopt` when that path was never captured. | + +`SectionSet` is neither copyable nor movable: its callbacks capture `this`. + +**No latch.** A ready section re-fires on *every* subsequent `set<>`, matching +`FlowSession`. A caller wanting one request per pause debounces on its own +side, where it knows what a pause means for its input widget. Coalescing here +would have to guess. + +**A ready draft is dispatched, a not-ready one is not sent at all.** The gate +is not a correctness backstop — `BridgeHandler::execute` enforces +`ActionValidator` on its own path ([../core/bridge.md](../core/bridge.md)), so +an ungated draft would come back as a validation failure rather than execute. +The gate exists to avoid a round trip that can only fail, and its absence is +observable as spurious `onError` calls. + +## Prefill is a declaration, not a write + +A `Bind` on a section says *where a field's initial value comes from*. Nothing +in the framework assigns it. `sectionGroupSchemaJson` emits it under `prefill` +for a renderer to act on, and `resolved(path)` exposes the captured values a +renderer resolves it against. `SectionSet` never writes a bound field into a +draft on its own. + +This is exact parity with `FlowSession`, which also emits `prefill` metadata +and captures values without ever assigning a bound field. A section set had a +stronger temptation to differ — with no ordering, "fill in the dependent field +the moment the source resolves" is a coherent design — but a reactive write +would silently overwrite a value the user had already typed into that field, +with no signal that it happened, and only for bound fields. The renderer knows +whether its widget is dirty; the session does not. + +Capture happens on success only, in the dispatch's completion: + +- the submitted draft's fields are recorded first, then the result's fields, + so a result field wins on a name collision — the result is what the model + actually settled on, and is therefore what a dependent field should show; +- keys are `"."`, the same vocabulary `Bind::path()` uses; +- values are JSON-encoded, so `resolved("Profile.name")` yields `"\"ada\""` + and `resolved("Profile.id")` yields `"3"`. + +A failed dispatch captures nothing. + +## Concurrency and lifetime + +One mutex guards the drafts and the captured values. `set<>` takes it to +assign the field and snapshot the draft, then **releases it before +dispatching**, so a slow dispatch of one section cannot block an edit to +another. + +Dispatch continuations run on whatever executor resolves the underlying +`BridgeHandler` completion, not necessarily the thread that called `set<>` — +see [../core/bridge.md](../core/bridge.md)'s executor/callback model. + +Every continuation is gated on one `morph::async::CallbackScope` +([../core/callback_scope.md](../core/callback_scope.md)), declared last so it +is the first member destroyed, and stopped explicitly at the top of +`~SectionSet`. A completion resolving after the set is gone finds the token +stopped and returns without touching anything. + +That covers a completion which has not yet started. It does not cover one +already past its token check: `requestStop()` does not wait, by design. So a +`SectionSet` may only be destroyed while a dispatch is outstanding if the +destroying thread is the one completions are delivered on — the ordinary case +for a UI-thread callback executor, and the boundary +[../core/callback_scope.md](../core/callback_scope.md) describes. + +**Polling `resolved()` is not a substitute for that.** Capture publishes each +key as it writes it, so a value becoming visible means the completion has +started, not that it has finished. A caller that destroys a set on one thread +the moment a value appears on another is destroying it mid-callback. `tests/test_sections.cpp` demonstrates the safe shape: deliver the completions on the +thread that owns the set. + +Unlike `FlowSession`, nothing here is keyed to a current position, so a reply +arriving late cannot be *stale*: there is no position for it to be stale +relative to. That is why `SectionSet` needs no equivalent of `FlowSession`'s +`_activeStep` guard. + +## Compile-time contract + +Two `static_assert`s, both on `SectionSet`: + +```cpp +// Rejected: a group must have at least one section. +morph::forms::SectionSet empty{handler}; + +// Rejected: two sections of the same action would share one draft slot, +// so an edit to either would silently clobber the other. +morph::forms::SectionSet duplicate{handler}; +``` + +and one on each of `set<>`, `reset()` and `draft()`: + +```cpp +// Rejected: UnrelatedAction is not a section of this group. +sections.set<&UnrelatedAction::field>(1); +``` + +This is where `SectionSet` still refuses a field — but at compile time, on +membership, not at run time on position. `FlowSession`'s `std::logic_error` +has no counterpart: with no current step, there is no run-time state that can +make a member field wrong to set. + +## Design decisions + +- **A separate type rather than a `FlowSession` mode.** A flag ("unordered + flow") would leave `advance()`, `back()`, `currentIndex()` and `finished()` + on an object for which none of them mean anything, and every one of them + would need a documented answer for the unordered case. A separate type has + only the members that make sense. +- **Shared declaration vocabulary.** `Bind`, the tuple/pack walkers and the + distinctness trait moved to `forms/detail/session_common.hpp` rather than + being duplicated. They describe how a form session declares its units, which + both types do identically; only the sequencing differs. +- **No index in the schema.** See [The `s-*` section-group document](#the-s--section-group-document). +- **No aggregate readiness and no "submit all".** Sections are independent by + construction; a group-level submit would reintroduce a coordination point + and raise questions this layer has no answer for (partial failure, ordering, + atomicity). Cross-action atomicity belongs in the outbox + ([../journal/journal.md](../journal/journal.md)), as it does for wizards. + +## Limitations + +- No renderer ships for `s-*` yet. `WizardView.qml` has no section-group + counterpart in `src/qt/forms`; a host consuming the document builds its own + layout for now. +- `resolved()` returns JSON-encoded strings, not typed values — the same shape + `FlowSession::resolved` has, and the same caller-side decode. +- A section that fires repeatedly issues one dispatch per `set<>`; there is no + in-flight coalescing or cancellation of a superseded request. +- Prefill is never applied by the framework, so a host that ignores the + `prefill` metadata gets no prefilling at all. + +## Testing + +`tests/test_sections.cpp` (`[sections]`), nine cases: + +- `sectionGroupSchemaJson` emits `s-id`, `s-title`, each section's `action` + and `title`, `prefill` only where a `Bind` is declared, and no `index` key. +- Sections fire independently in any order — the morph#513 regression: the + same edit sequence throws `std::logic_error` under `FlowSession`. +- A not-ready draft is not sent at all, observed through `onError` (a missing + gate is visible as a spurious validation failure, not as a bad execution). +- An already-fired section fires again on the next edit (the no-latch rule). +- `reset()` clears one section and leaves the others intact. +- A fired section's draft *and* result fields are resolvable, and a path + belonging to an unfired section — or to no field — is not. +- A failing dispatch reaches `onError`, and a succeeding one does not. +- An unhandled failure logs instead of escaping, and the set survives it. +- Destroying the set with a dispatch genuinely in flight (a section whose + model call blocks until the test releases it) delivers nothing afterwards. + +Every other case delivers its completions on the test thread through a +`StepExecutor` and drains before the set leaves scope, for the reason +[Concurrency and lifetime](#concurrency-and-lifetime) gives. + +## Cross-references + +- [workflows_navigation.md](workflows_navigation.md) — `FlowSession` and the + `w-*` document this layer is the unordered sibling of; the ordering + constraint whose absence defines `SectionSet`. +- [forms.md](forms.md) — the per-action schema each section renders, and + `FixedString`, which `Section`/`SectionGroup` titles and `Bind` paths use. +- [../core/bridge.md](../core/bridge.md) — `BridgeHandler::execute` and + `ActionValidator::ready`, the dispatch path and readiness gate `SectionSet` + reuses without extending. +- [../core/callback_scope.md](../core/callback_scope.md) — + `morph::async::CallbackScope`, the gate that refuses a completion resolving + after the set is destroyed. +- [../core/registry.md](../core/registry.md) — `ActionTraits::typeId()` and the + metadata-only registration `BRIDGE_REGISTER_SECTION_GROUP` mirrors. +- [../journal/journal.md](../journal/journal.md) — where cross-action + atomicity belongs; a section group deliberately does not provide it. diff --git a/docs/superpowers/plans/2026-09-10-unordered-sections.md b/docs/superpowers/plans/2026-09-10-unordered-sections.md new file mode 100644 index 00000000..a4177ef1 --- /dev/null +++ b/docs/superpowers/plans/2026-09-10-unordered-sections.md @@ -0,0 +1,1181 @@ +# Unordered Sections 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:** Add `morph::forms::SectionSet` — N independently editable action drafts, each gated +on its own `ActionValidator::ready`, dispatched through the handler with no sequencing. Closes morph#513. + +**Architecture:** Extract the machinery `FlowSession` and `SectionSet` share into +`forms/detail/session_common.hpp`, leaving `flows.hpp`'s public names intact. Add `forms/sections.hpp` with the +declaration types (`Section`, `SectionGroup`), the schema emitter, and the runtime session. `SectionSet` mirrors +`FlowSession` minus the current-step constraint. + +**Tech Stack:** C++23, header-only. Catch2 v3 for tests. glaze for JSON. No new dependencies. + +## Where execution diverged from this plan + +Recorded because the task steps below are wrong in one way that matters, and a +reader following them verbatim would reintroduce a real bug. + +1. **The tests here wait on `resolved()` before leaving scope. That is not + safe.** `captureResult` publishes its first key while still writing the + rest, so a value becoming visible means the completion has *started*, not + finished — and `CallbackScope::requestStop()` does not wait for a callback + already past its token check. A test that polls `resolved()` on one thread + and then destroys the set tears it down mid-callback. It segfaulted on + Windows CI and TSan reproduces it about once in fifteen runs. As shipped, + every case except the lifetime one uses `StepExecutor` and an explicit + `drain()`, so the thread that delivers the completion is the thread that + destroys the set. See `docs/spec/forms/sections.md`, "Concurrency and + lifetime". + +2. **Task 3's "not-ready draft" test measured nothing.** It checked the + recorder immediately after an incomplete `set<>`; dispatch is asynchronous, + so it passed with or without the gate. The shipped test watches `onError` + instead — `BridgeHandler::execute` enforces `ActionValidator` on its own + path, so a missing section-level gate shows up as a spurious validation + failure, not as a bad execution. + +3. **A gate this plan missed:** `CMakeLists.txt` fails configure for a public + header that belongs to no target's `FILE_SET HEADERS`, so + `include/morph/forms/sections.hpp` had to be added there (morph#230). + Headers under `detail/` are exempt. + +4. **`app.hpp` reached into `flows::detail`** for a walker, so Task 1 had to + repoint it at the shared header as well. + +## Global Constraints + +Every task's requirements implicitly include these. They are the gates CI applies. + +- **C++23, header-only.** No new third-party dependencies. +- **Build with clang AND gcc.** GCC has `-Werror=useless-cast`, which clang lacks; a clang-only check misses it. + Presets: `clang-debug`, `gcc-debug`. +- **`-Wdocumentation -Werror` clean.** Every public symbol needs complete `@param`/`@tparam`/`@return`. Never write + a Doxygen command name (`@throws`, `@par`) in prose — even in backticks — it parses as a command. +- **Tree-wide clang-format:** `git ls-files -z '*.hpp' '*.cpp' | xargs -0 clang-format --dry-run -Werror` must be + silent. +- **clang-tidy clean on changed lines**, diffed against `origin/master...HEAD` (not the working tree — that is + empty after a commit). +- **Header ↔ spec sync:** any change under `include/morph/forms/` requires a matching change under + `docs/spec/forms/`. Task 8 satisfies this and must land before the PR opens. +- **Invariant 7:** every test must be verified to FAIL without its implementation. A test that passes either way + measures nothing. +- **`scripts/branch_partial_allowlist.json`** must have every `line` landing on its own `source` text, and every + `line N` in `reason` prose resolving to a real entry. Re-pin if line numbers move. + +--- + +## File Structure + +| File | Responsibility | +|---|---| +| `include/morph/forms/detail/session_common.hpp` *(new)* | `Bind`, `forEachTupleElement`, `forPackElement`, `AllDistinct`, `emitBindsInto` — shared by both session types | +| `include/morph/forms/flows.hpp` *(modify)* | Unchanged public surface; helpers now come from the shared header, `morph::flows::Bind` becomes an alias | +| `include/morph/forms/sections.hpp` *(new)* | `Section`, `SectionGroup`, `SectionGroupTraits`, `BRIDGE_REGISTER_SECTION_GROUP`, `sectionGroupSchemaJson`, `SectionSet` | +| `tests/test_sections.cpp` *(new)* | All nine cases from the spec | +| `tests/CMakeLists.txt` *(modify)* | Register the new test file | +| `docs/spec/forms/sections.md` *(new)* | Design spec for the new surface; also satisfies the spec-sync gate | + +--- + +### Task 1: Extract the shared session helpers + +Pure refactor. `flows.hpp` must behave identically and its existing tests must pass untouched. + +**Files:** +- Create: `include/morph/forms/detail/session_common.hpp` +- Modify: `include/morph/forms/flows.hpp` (remove the moved definitions, add the include and the `Bind` alias) +- Test: `tests/test_flows_apps.cpp` (existing — must pass unchanged) + +**Interfaces:** +- Produces: `morph::forms::Bind` with `static constexpr std::string_view field()` and `path()`; + `morph::forms::detail::forEachTupleElement(Visitor&&)`; + `morph::forms::detail::forPackElement(std::size_t, Visitor&&)`; + `morph::forms::detail::AllDistinct::value`; + `morph::forms::detail::emitBindsInto(glz::generic_u64& node)`. +- Consumes: nothing. + +- [ ] **Step 1: Create the shared header** + +```cpp +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +#include +#include +#include +#include +#include +#include + +#include + +#include "../forms.hpp" + +namespace morph::forms { + +/// @brief One `field -> "."` prefill binding declared on a +/// wizard step or an unordered section. +/// +/// A declaration, not a write. Nothing in the framework assigns a bound field: +/// `wizardSchemaJson`/`sectionGroupSchemaJson` emit it for a renderer, and the +/// captured value is read back through `resolved()`. +/// @tparam Field The action's field name to prefill. +/// @tparam Path Source path, `"."`, into captured values. +template +struct Bind { + /// @brief The action field name this binding fills. + /// @return The declared field name. + [[nodiscard]] static constexpr std::string_view field() noexcept { return Field.view(); } + + /// @brief The source path into captured values. + /// @return The declared `"."` path. + [[nodiscard]] static constexpr std::string_view path() noexcept { return Path.view(); } +}; + +namespace detail { + +/// @brief Invokes `visitor.template operator(), I>()` +/// for every element of @p Tuple, in order. +/// @tparam Tuple A `std::tuple<...>` type (only its element types/arity are used). +/// @tparam Visitor Callable with a `template operator()()`. +/// @param visitor Callable invoked once per tuple element. +template +constexpr void forEachTupleElement(Visitor&& visitor) { + [](std::index_sequence, Visitor&& innerVisitor) { + (innerVisitor.template operator(), I>(), ...); + }(std::make_index_sequence>{}, std::forward(visitor)); +} + +/// @brief Invokes `visitor.template operator()()` for the pack element of +/// `Ts...` at runtime position @p index. A no-op when +/// `index >= sizeof...(Ts)`. +/// @tparam Ts The pack to index into. +/// @tparam Visitor Callable with a `template operator()()`. +/// @param index 0-based position to visit. +/// @param visitor Callable invoked for the element at @p index. +template +constexpr void forPackElement(std::size_t index, Visitor&& visitor) { + std::size_t i = 0; + (void)((i++ == index ? (visitor.template operator()(), true) : false) || ...); +} + +/// @brief Trait: `true` when every type in `Ts...` is pairwise distinct. +/// @tparam Ts Types to check for pairwise distinctness. +template +struct AllDistinct : std::true_type {}; + +/// @brief Recursive case: `T` distinct from every type in `Rest...`, and `Rest...` pairwise distinct. +/// @tparam T The type being checked against `Rest...`. +/// @tparam Rest The remaining types. +template +struct AllDistinct : std::bool_constant<(!std::is_same_v && ...) && AllDistinct::value> { +}; + +/// @brief Writes each `Bind` in @p BindsTuple into @p node as +/// `"": ""`. A no-op for an empty tuple. +/// @tparam BindsTuple `std::tuple...>`. +/// @param node Destination JSON object node. +template +void emitBindsInto(glz::generic_u64& node) { + forEachTupleElement([&]() { + static_cast(J); + node[std::string{BindT::field()}] = std::string{BindT::path()}; + }); +} + +} // namespace detail +} // namespace morph::forms +``` + +- [ ] **Step 2: Point flows.hpp at it** + +In `include/morph/forms/flows.hpp`: add `#include "detail/session_common.hpp"` beside the existing `#include +"forms.hpp"`. Delete the `Bind` struct definition and the `detail` block containing `forEachTupleElement`, +`forStep`, and `AllDistinct`. Add the compatibility alias in `namespace morph::flows`: + +```cpp +/// @brief Prefill binding for a wizard step. +/// +/// Alias for `morph::forms::Bind`, which both session types share. Kept in this +/// namespace because it is the name shipped consumers already write. +/// @tparam Field The step action's field name to prefill. +/// @tparam Path Source path, `"."`, into captured values. +template +using Bind = morph::forms::Bind; +``` + +Then replace the internal call sites: `detail::forEachTupleElement<...>` becomes +`::morph::forms::detail::forEachTupleElement<...>`, `detail::forStep<...>` becomes +`::morph::forms::detail::forPackElement<...>`, and `detail::AllDistinct<...>` becomes +`::morph::forms::detail::AllDistinct<...>`. + +- [ ] **Step 3: Build both compilers and run the existing suite** + +```bash +cmake --build build/clang-debug --target morph_tests -j 12 +./build/clang-debug/tests/morph_tests "[flows]" +cmake --build build/gcc-net --target morph_tests -j 12 2>&1 | grep -c "error:" +``` + +Expected: build clean under both; `[flows]` cases all pass. This is a refactor — a behaviour change here is a bug. + +- [ ] **Step 4: Commit** + +```bash +git add include/morph/forms/detail/session_common.hpp include/morph/forms/flows.hpp +git commit -m "forms: extract the session helpers both FlowSession and SectionSet need" +``` + +--- + +### Task 2: Declaration types and the schema document + +**Files:** +- Create: `include/morph/forms/sections.hpp` +- Create: `tests/test_sections.cpp` +- Modify: `tests/CMakeLists.txt:102` (add `test_sections.cpp` beside `test_flows_apps.cpp`) + +**Interfaces:** +- Consumes: `morph::forms::Bind`, `morph::forms::detail::forEachTupleElement`, + `morph::forms::detail::emitBindsInto` (Task 1). +- Produces: `morph::forms::Section` with `using action`, `using binds`, `static constexpr + std::string_view title()`; `morph::forms::SectionGroup` with `using sections`, `title()`; + `morph::forms::SectionGroupTraits::typeId()`; `BRIDGE_REGISTER_SECTION_GROUP(G, NAME)`; + `morph::forms::sectionGroupSchemaJson() -> std::string`. + +- [ ] **Step 1: Write the failing test** + +Append to `tests/test_sections.cpp` (create it with this content): + +```cpp +// SPDX-License-Identifier: Apache-2.0 +#include +#include +#include +#include +#include +#include +#include + +// Two independent sections of one screen: a profile block and a preferences +// block. Neither is a step of the other -- editing them in either order is the +// point of SectionSet. +struct SecProfile { + std::string name; + [[nodiscard]] bool validate() const { return !name.empty(); } +}; +struct SecProfileResult { + std::int64_t id = 0; +}; +struct SecPrefs { + std::int64_t profileId = 0; + std::string theme; + [[nodiscard]] bool validate() const { return !theme.empty(); } +}; +struct SecPrefsResult { + std::string summary; +}; + +struct SecModel { + SecProfileResult execute(SecProfile action) { return {.id = static_cast(action.name.size())}; } + SecPrefsResult execute(SecPrefs action) { return {.summary = action.theme}; } +}; + +BRIDGE_REGISTER_MODEL(SecModel, "SectionsTest_Model") +BRIDGE_REGISTER_ACTION(SecModel, SecProfile, "SectionsTest_Profile") +BRIDGE_REGISTER_ACTION(SecModel, SecPrefs, "SectionsTest_Prefs") + +using ProfileSection = morph::forms::Section; +using PrefsSection = + morph::forms::Section>; +using DemoGroup = morph::forms::SectionGroup<"Account settings", ProfileSection, PrefsSection>; +BRIDGE_REGISTER_SECTION_GROUP(DemoGroup, "SectionsTest_DemoGroup") + +TEST_CASE("sectionGroupSchemaJson carries each section's title, action and binds", "[sections][schema]") { + auto const json = morph::forms::sectionGroupSchemaJson(); + REQUIRE_FALSE(json.empty()); + + glz::generic_u64 dom{}; + REQUIRE_FALSE(glz::read_json(dom, json)); + + CHECK(dom["s-id"].get_string() == "SectionsTest_DemoGroup"); + CHECK(dom["s-title"].get_string() == "Account settings"); + + auto const& sections = dom["s-sections"].get_array(); + REQUIRE(sections.size() == 2); + CHECK(sections[0]["action"].get_string() == "SectionsTest_Profile"); + CHECK(sections[0]["title"].get_string() == "Profile"); + CHECK(sections[1]["action"].get_string() == "SectionsTest_Prefs"); + CHECK(sections[1]["prefill"]["profileId"].get_string() == "SectionsTest_Profile.id"); + + // No order is implied: a section carries no index field, because a renderer + // chooses its own arrangement and an emitted position would imply a + // sequence SectionSet does not have. + CHECK_FALSE(sections[0].contains("index")); +} +``` + +- [ ] **Step 2: Register the test and run it to verify it fails** + +Add `test_sections.cpp` to `tests/CMakeLists.txt` beside `test_flows_apps.cpp`, then: + +```bash +cmake --build build/clang-debug --target morph_tests -j 12 +``` + +Expected: FAIL to compile — `morph/forms/sections.hpp` does not exist. + +- [ ] **Step 3: Write the declaration types and emitter** + +Create `include/morph/forms/sections.hpp`: + +```cpp +// SPDX-License-Identifier: Apache-2.0 +#pragma once + +/// @file forms/sections.hpp +/// @brief Unordered sections: N independently editable action drafts on one +/// screen, each with its own readiness gate and no sequencing. +/// +/// The sibling to `morph::flows::FlowSession`. A wizard has one active step and +/// refuses a field belonging to any other; a section set has no active step at +/// all. Use this when step order carries no meaning -- tabs, cards, a settings +/// page -- and `FlowSession` when it does. + +#include +#include +#include +#include +#include +#include + +#include + +#include "../core/bridge.hpp" +#include "detail/session_common.hpp" + +namespace morph::forms { + +/// @brief One section of a `SectionGroup`: a registered action, a display +/// title, and zero or more `Bind` prefill declarations. +/// @tparam Action Registered action type (`BRIDGE_REGISTER_ACTION`) this section fires. +/// @tparam Title Human title for the section (tab label / card header). +/// @tparam Binds Zero or more `Bind` prefill declarations. +template +struct Section { + /// @brief The section's action type. + using action = Action; + + /// @brief Tuple of this section's `Bind<...>` declarations (possibly empty). + using binds = std::tuple; + + /// @brief The section's display title. + /// @return The declared title. + [[nodiscard]] static constexpr std::string_view title() noexcept { return Title.view(); } +}; + +/// @brief An unordered set of `Section`s sharing one screen. +/// @tparam Title Human title for the whole group. +/// @tparam Sections One or more `Section` types. +template +struct SectionGroup { + /// @brief Tuple of this group's `Section<...>` types. + using sections = std::tuple; + + /// @brief The group's display title. + /// @return The declared title. + [[nodiscard]] static constexpr std::string_view title() noexcept { return Title.view(); } +}; + +/// @brief Traits specialisation mapping a `SectionGroup` type to its string type-id. +/// +/// Specialise via `BRIDGE_REGISTER_SECTION_GROUP` rather than by hand. The +/// emitted schema needs a stable name for the group; deriving one from the C++ +/// type would tie the wire format to a mangled name. +/// @tparam G The `SectionGroup` type. +template +struct SectionGroupTraits; // forward — specialise or use BRIDGE_REGISTER_SECTION_GROUP + +/// @brief Generates the `s-*` JSON document for section group @p G. +/// +/// Carries the group id and title, and one entry per section with its action +/// type-id, title and declared `prefill` binds. Deliberately emits no index or +/// order field: a renderer chooses its own arrangement, and a position would +/// imply a sequence this type does not have. +/// @tparam G The `SectionGroup` type to describe. +/// @return The JSON document, or an empty string if serialization fails. +template +[[nodiscard]] std::string sectionGroupSchemaJson() { + glz::generic_u64 dom{}; + dom["s-id"] = std::string{SectionGroupTraits::typeId()}; + dom["s-title"] = std::string{G::title()}; + + glz::generic_u64::array_t sections{}; + detail::forEachTupleElement([&]() { + static_cast(I); + glz::generic_u64 entry{}; + entry["action"] = std::string{::morph::model::ActionTraits::typeId()}; + entry["title"] = std::string{SectionT::title()}; + if constexpr (std::tuple_size_v != 0) { + detail::emitBindsInto(entry["prefill"]); + } + sections.emplace_back(std::move(entry)); + }); + dom["s-sections"] = sections; + + return glz::write_json(dom).value_or(std::string{}); +} + +} // namespace morph::forms + +// clang-format off -- public macro surface; see CONTRIBUTING.md, "Formatting/linting". +/// @brief Specialises `morph::forms::SectionGroupTraits` with the string type-id @p NAME. +#define BRIDGE_REGISTER_SECTION_GROUP(G, NAME) \ + template <> \ + struct morph::forms::SectionGroupTraits { \ + static constexpr std::string_view typeId() noexcept { return NAME; } \ + }; +// clang-format on +``` + +- [ ] **Step 4: Run the test to verify it passes** + +```bash +cmake --build build/clang-debug --target morph_tests -j 12 +./build/clang-debug/tests/morph_tests "[sections][schema]" +``` + +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add include/morph/forms/sections.hpp tests/test_sections.cpp tests/CMakeLists.txt +git commit -m "forms: Section/SectionGroup declarations and their schema document" +``` + +--- + +### Task 3: SectionSet — independent, unordered firing + +The core of morph#513. Case 1 is the regression test: this shape throws under `FlowSession`. + +**Files:** +- Modify: `include/morph/forms/sections.hpp` (add `SectionSet` before the closing `} // namespace morph::forms`) +- Test: `tests/test_sections.cpp` + +**Interfaces:** +- Consumes: `Section`, `SectionGroup` (Task 2); `morph::forms::detail::AllDistinct` (Task 1). +- Produces: `SectionSet` with `explicit SectionSet(BridgeHandler&, + std::function = nullptr)`, `template void set(ValueType)`, and the + private `fire(A draft)`. + +- [ ] **Step 1: Write the failing tests** + +Append to `tests/test_sections.cpp`: + +```cpp +namespace { +/// Collects the actions the model actually executed, in order. +struct SecRecorder { + std::mutex mtx; + std::vector fired; + + void record(std::string what) { + std::scoped_lock const lock{mtx}; + fired.push_back(std::move(what)); + } + [[nodiscard]] std::vector snapshot() { + std::scoped_lock const lock{mtx}; + return fired; + } +}; +SecRecorder& recorder() { + static SecRecorder inst; + return inst; +} +} // namespace + +TEST_CASE("SectionSet: sections fire independently, in any order", "[sections][morph513]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::testing::InlineExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + morph::forms::SectionSet sections{handler}; + + recorder().fired.clear(); + + // Edit the SECOND section first. Under FlowSession this throws + // std::logic_error -- "field belongs to an action that is not the current + // step" -- which is exactly the gap morph#513 reports. + sections.set<&SecPrefs::theme>("dark"); + REQUIRE(morph::testing::waitUntil([&] { return recorder().snapshot().size() == 1; })); + + // Then the first. Both fire; neither is "current". + sections.set<&SecProfile::name>("ada"); + REQUIRE(morph::testing::waitUntil([&] { return recorder().snapshot().size() == 2; })); + + auto const fired = recorder().snapshot(); + CHECK(fired[0] == "SectionsTest_Prefs"); + CHECK(fired[1] == "SectionsTest_Profile"); +} + +TEST_CASE("SectionSet: a not-ready draft does not dispatch", "[sections]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::testing::InlineExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + morph::forms::SectionSet sections{handler}; + + recorder().fired.clear(); + + // SecPrefs::validate() requires a non-empty theme; profileId alone is not ready. + sections.set<&SecPrefs::profileId>(7); + CHECK(recorder().snapshot().empty()); + + // Completing it dispatches. + sections.set<&SecPrefs::theme>("light"); + REQUIRE(morph::testing::waitUntil([&] { return recorder().snapshot().size() == 1; })); +} + +TEST_CASE("SectionSet: an already-fired section fires again on the next edit", "[sections]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::testing::InlineExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + morph::forms::SectionSet sections{handler}; + + recorder().fired.clear(); + sections.set<&SecProfile::name>("ada"); + REQUIRE(morph::testing::waitUntil([&] { return recorder().snapshot().size() == 1; })); + + // No latch: still ready, so it dispatches again. + sections.set<&SecProfile::name>("grace"); + REQUIRE(morph::testing::waitUntil([&] { return recorder().snapshot().size() == 2; })); +} +``` + +Add `recorder().record(...)` calls to `SecModel::execute`: + +```cpp +struct SecModel { + SecProfileResult execute(SecProfile action) { + recorder().record("SectionsTest_Profile"); + return {.id = static_cast(action.name.size())}; + } + SecPrefsResult execute(SecPrefs action) { + recorder().record("SectionsTest_Prefs"); + return {.summary = action.theme}; + } +}; +``` + +Move the `SecRecorder` block above `SecModel`, and add these includes: ``, ``, ``, +``, ``, `"test_support.hpp"`. + +- [ ] **Step 2: Run to verify they fail** + +```bash +cmake --build build/clang-debug --target morph_tests -j 12 +``` + +Expected: FAIL to compile — `SectionSet` is not declared. + +- [ ] **Step 3: Implement SectionSet's core** + +Insert before the closing `} // namespace morph::forms` in `sections.hpp`: + +```cpp +/// @brief Drives N independently editable action drafts on one screen. +/// +/// Each section accumulates its own draft through `set<>`, and dispatches +/// through the handler as soon as `ActionValidator::ready` accepts it. +/// There is no active section and no sequence: a field belonging to any +/// declared section may be set at any time. +/// +/// A ready section re-fires on every subsequent `set<>`. There is no latch and +/// no coalescing, matching `FlowSession`; a caller wanting one request per +/// pause debounces on its own side. +/// @tparam Model The model the handler is bound to. +/// @tparam Sections One or more `Section` types. +template +class SectionSet { + static_assert(sizeof...(Sections) > 0, "SectionSet: a group needs at least one section"); + static_assert(detail::AllDistinct::value, + "SectionSet: section action types must be pairwise distinct"); + +public: + /// @brief Constructs a section set over @p handler. + /// @param handler Handler every section dispatches through. Must outlive + /// this `SectionSet` (only *destruction* order is + /// unconstrained; see bridge.md's Lifetime & ownership). + /// @param onError Optional callback invoked when a section's dispatch + /// fails. When absent the error is logged via + /// `morph::log::logError`. Stored and invoked for this + /// object's whole lifetime, so anything the callable refers + /// to must outlive it. + explicit SectionSet(::morph::bridge::BridgeHandler& handler MORPH_LIFETIMEBOUND, + std::function onError MORPH_LIFETIMEBOUND = nullptr) + : _handler{handler}, _onError{std::move(onError)} {} + + SectionSet(const SectionSet&) = delete; + SectionSet& operator=(const SectionSet&) = delete; + SectionSet(SectionSet&&) = delete; + SectionSet& operator=(SectionSet&&) = delete; + + /// @brief Sets one field of its section's draft and dispatches that section + /// if the draft is now ready. + /// + /// Unlike `FlowSession::set<>` this imposes no ordering: the field's action + /// need only be one of the declared sections. + /// @tparam FieldPtr Pointer-to-data-member of a declared section's action struct. + /// @param value New value for the field. + template + void set(typename ::morph::bridge::detail::MemberPointerTraits::ValueType value) { + using A = typename ::morph::bridge::detail::MemberPointerTraits::ClassType; + static_assert((std::is_same_v || ...), + "SectionSet::set<>: field's action is not a section of this group"); + A draft{}; + { + std::scoped_lock const lock{_mtx}; + std::get(_drafts).*FieldPtr = std::move(value); + draft = std::get(_drafts); + } + if (::morph::model::ActionValidator::ready(draft)) { + fire(std::move(draft)); + } + } + +private: + template + void fire(A draft) { + _handler.execute(std::move(draft)); + } + + ::morph::bridge::BridgeHandler& _handler; + std::function _onError; + mutable std::mutex _mtx; + std::tuple _drafts{}; +}; +``` + +Add `#include ` and `#include ` to the header's include block. + +- [ ] **Step 4: Run the tests to verify they pass** + +```bash +cmake --build build/clang-debug --target morph_tests -j 12 +./build/clang-debug/tests/morph_tests "[sections]" +``` + +Expected: PASS, all four cases. + +- [ ] **Step 5: Commit** + +```bash +git add include/morph/forms/sections.hpp tests/test_sections.cpp +git commit -m "forms: SectionSet dispatches each section independently (#513)" +``` + +--- + +### Task 4: reset and draft accessors + +**Files:** +- Modify: `include/morph/forms/sections.hpp` +- Test: `tests/test_sections.cpp` + +**Interfaces:** +- Consumes: `SectionSet` (Task 3). +- Produces: `template void reset()`; `template [[nodiscard]] A draft() const`. + +- [ ] **Step 1: Write the failing test** + +```cpp +TEST_CASE("SectionSet: reset clears one section and leaves the others intact", "[sections]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::testing::InlineExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + morph::forms::SectionSet sections{handler}; + + sections.set<&SecPrefs::profileId>(42); // not ready: no theme yet + sections.set<&SecProfile::name>("ada"); // ready: fires + + CHECK(sections.draft().profileId == 42); + CHECK(sections.draft().name == "ada"); + + sections.reset(); + + CHECK(sections.draft().profileId == 0); + // The other section is untouched -- per-section isolation is the point. + CHECK(sections.draft().name == "ada"); +} +``` + +- [ ] **Step 2: Run to verify it fails** + +```bash +cmake --build build/clang-debug --target morph_tests -j 12 +``` + +Expected: FAIL to compile — no member `reset`/`draft`. + +- [ ] **Step 3: Implement them** + +Add to `SectionSet`'s public section, after `set<>`: + +```cpp + /// @brief Clears one section's draft back to a default-constructed action. + /// + /// Touches no other section, and dispatches nothing. + /// @tparam A The section's action type. + template + void reset() { + static_assert((std::is_same_v || ...), + "SectionSet::reset<>: not a section of this group"); + std::scoped_lock const lock{_mtx}; + std::get(_drafts) = A{}; + } + + /// @brief Snapshots one section's current draft. + /// + /// A copy taken under the lock, not a reference into live state, so a + /// renderer can read it while another thread edits a different section. + /// @tparam A The section's action type. + /// @return The draft as it stands. + template + [[nodiscard]] A draft() const { + static_assert((std::is_same_v || ...), + "SectionSet::draft<>: not a section of this group"); + std::scoped_lock const lock{_mtx}; + return std::get(_drafts); + } +``` + +- [ ] **Step 4: Run to verify it passes** + +```bash +cmake --build build/clang-debug --target morph_tests -j 12 && ./build/clang-debug/tests/morph_tests "[sections]" +``` + +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add include/morph/forms/sections.hpp tests/test_sections.cpp +git commit -m "forms: SectionSet reset() and draft()" +``` + +--- + +### Task 5: Result capture and resolved() + +**Files:** +- Modify: `include/morph/forms/sections.hpp` +- Test: `tests/test_sections.cpp` + +**Interfaces:** +- Consumes: `SectionSet::fire` (Task 3). +- Produces: `[[nodiscard]] std::optional resolved(std::string_view path) const`. + +- [ ] **Step 1: Write the failing test** + +```cpp +TEST_CASE("SectionSet: a fired section's fields are resolvable; an unfired one is not", "[sections]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::testing::InlineExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + morph::forms::SectionSet sections{handler}; + + recorder().fired.clear(); + sections.set<&SecProfile::name>("ada"); + REQUIRE(morph::testing::waitUntil([&] { return recorder().snapshot().size() == 1; })); + REQUIRE(morph::testing::waitUntil([&] { return sections.resolved("SectionsTest_Profile.id").has_value(); })); + + // The result's field: SecProfileResult::id == name.size() == 3. + CHECK(sections.resolved("SectionsTest_Profile.id") == std::string{"3"}); + // The submitted draft's field is captured too. + CHECK(sections.resolved("SectionsTest_Profile.name") == std::string{"\"ada\""}); + + // Both halves matter: a path whose section never fired resolves to nothing. + // Without this, an implementation returning a value for everything passes. + CHECK_FALSE(sections.resolved("SectionsTest_Prefs.summary").has_value()); + CHECK_FALSE(sections.resolved("SectionsTest_Profile.nosuchfield").has_value()); +} +``` + +- [ ] **Step 2: Run to verify it fails** + +```bash +cmake --build build/clang-debug --target morph_tests -j 12 +``` + +Expected: FAIL to compile — no member `resolved`. + +- [ ] **Step 3: Implement capture and lookup** + +Replace `fire` and add `captureResult` plus the public `resolved`: + +```cpp + /// @brief Looks up a value captured from a section's submitted draft or result. + /// @param path `"."`, matching a section's declared `Bind::path()`. + /// @return The field's JSON-encoded value, or `std::nullopt` if @p path was + /// never captured (the section never fired, or has no such field). + [[nodiscard]] std::optional resolved(std::string_view path) const { + std::scoped_lock const lock{_mtx}; + auto iter = _resolvedValues.find(std::string{path}); + if (iter == _resolvedValues.end()) { + return std::nullopt; + } + return iter->second; + } +``` + +```cpp + template + void captureResult(const ::morph::model::ActionTraits::Result& result) { + std::scoped_lock const lock{_mtx}; + auto const typeId = ::morph::model::ActionTraits::typeId(); + auto record = [&](const auto& value) { + ::morph::forms::detail::forEachNamedMember( + value, [&](std::string_view name, const auto& member) { + static_cast(I); + std::string json; + if (!glz::write_json(member, json)) { + _resolvedValues[std::string{typeId} + "." + std::string{name}] = std::move(json); + } + }); + }; + record(std::get(_drafts)); // submitted draft fields first... + record(result); // ...result fields win on name collision + } + + template + void fire(A draft) { + _handler.execute(std::move(draft)).then(_callbacks, [this](::morph::model::ActionTraits::Result result) { + this->template captureResult(result); + }); + } +``` + +Add the member `std::unordered_map _resolvedValues;` and `::morph::async::CallbackScope +_callbacks;` (declared last), plus includes `` and ``. + +**Note on ordering:** `_callbacks` must be the last member so it is destroyed last; Task 7 adds the destructor that +stops it first. + +- [ ] **Step 4: Run to verify it passes** + +```bash +cmake --build build/clang-debug --target morph_tests -j 12 && ./build/clang-debug/tests/morph_tests "[sections]" +``` + +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add include/morph/forms/sections.hpp tests/test_sections.cpp +git commit -m "forms: SectionSet captures results and exposes resolved()" +``` + +--- + +### Task 6: Error routing + +**Files:** +- Modify: `include/morph/forms/sections.hpp` +- Test: `tests/test_sections.cpp` + +**Interfaces:** +- Consumes: `SectionSet::fire` (Task 5). +- Produces: the `onError` path and `logUnhandledError`. + +- [ ] **Step 1: Write the failing test** + +Add a throwing action to the fixture: + +```cpp +struct SecExplodes { + std::string label; + [[nodiscard]] bool validate() const { return !label.empty(); } +}; +struct SecExplodesResult { + std::int64_t id = 0; +}; +``` + +Add to `SecModel`: `SecExplodesResult execute(SecExplodes) { throw std::runtime_error{"section boom"}; }`, register +it with `BRIDGE_REGISTER_ACTION(SecModel, SecExplodes, "SectionsTest_Explodes")`, and declare `using +ExplodesSection = morph::forms::Section;`. + +```cpp +TEST_CASE("SectionSet: a failing dispatch reaches the onError callback", "[sections]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::testing::InlineExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + std::atomic errors{0}; + morph::forms::SectionSet sections{ + handler, [&](std::exception_ptr) { errors.fetch_add(1); }}; + + sections.set<&SecExplodes::label>("boom"); + REQUIRE(morph::testing::waitUntil([&] { return errors.load() == 1; })); + + // A section that succeeds does not route to onError. + sections.set<&SecProfile::name>("ada"); + CHECK(errors.load() == 1); +} +``` + +- [ ] **Step 2: Run to verify it fails** + +```bash +cmake --build build/clang-debug --target morph_tests -j 12 && ./build/clang-debug/tests/morph_tests "a failing dispatch reaches" +``` + +Expected: FAIL — `errors` stays 0, because `fire` installs no `.onError` continuation. + +- [ ] **Step 3: Implement error routing** + +```cpp + static void logUnhandledError(std::string_view typeId, const std::exception_ptr& err) { + try { + std::rethrow_exception(err); + } catch (const std::exception& exc) { + ::morph::log::logError(std::string{"[sections:"} + std::string{typeId} + "] " + exc.what()); + } catch (...) { + ::morph::log::logError(std::string{"[sections:"} + std::string{typeId} + "] unknown exception"); + } + } + + template + void fire(A draft) { + _handler.execute(std::move(draft)) + .then(_callbacks, [this](::morph::model::ActionTraits::Result result) { + this->template captureResult(result); + }) + .onError(_callbacks, [this](const std::exception_ptr& err) { + if (_onError) { + _onError(err); + return; + } + logUnhandledError(::morph::model::ActionTraits::typeId(), err); + }); + } +``` + +Add `#include ` and `#include `. + +- [ ] **Step 4: Run to verify it passes** + +```bash +cmake --build build/clang-debug --target morph_tests -j 12 && ./build/clang-debug/tests/morph_tests "[sections]" +``` + +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add include/morph/forms/sections.hpp tests/test_sections.cpp +git commit -m "forms: SectionSet routes a failed dispatch to onError, else logs" +``` + +--- + +### Task 7: Lifetime — the callback gate + +**Files:** +- Modify: `include/morph/forms/sections.hpp` +- Test: `tests/test_sections.cpp` + +**Interfaces:** +- Consumes: `_callbacks` (Task 5). +- Produces: `~SectionSet()`. + +- [ ] **Step 1: Write the failing test** + +```cpp +TEST_CASE("SectionSet: destroying it with a dispatch in flight delivers nothing", "[sections]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::testing::InlineExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + std::atomic errors{0}; + { + morph::forms::SectionSet sections{ + handler, [&](std::exception_ptr) { errors.fetch_add(1); }}; + sections.set<&SecExplodes::label>("boom"); + // Leaves scope with the dispatch possibly still in flight. The + // destructor stops the scope, so no continuation may run against the + // destroyed object. Same-thread delivery via InlineExecutor makes + // check-then-run atomic here -- see callback_scope.md's boundary. + } + // Nothing may arrive after destruction. A count of 1 recorded *before* the + // scope closed is possible and fine; what must not happen is a crash or a + // later increment. + auto const after = errors.load(); + CHECK(morph::testing::waitUntil([&] { return errors.load() == after; })); +} +``` + +- [ ] **Step 2: Run to verify it fails** + +```bash +cmake --build build/clang-debug --target morph_tests -j 12 +``` + +Expected: FAIL to compile only if the destructor is missing entirely; otherwise this case documents the gate. If it +compiles and passes without a destructor, that is because `_callbacks`' own destruction happens to cover it — add +the destructor anyway per Step 3 and keep the case as the regression guard for a future pumping teardown. + +- [ ] **Step 3: Add the destructor** + +```cpp + /// @brief Stops every callback this set installed from being delivered. + /// + /// There is nothing to detach: a section's continuations are owned by the + /// in-flight dispatch, not held in a map this object could remove itself + /// from. `_callbacks` gates each one on a token it checks before touching + /// `this`. + /// + /// `requestStop()` is called explicitly rather than left to the member's + /// own destruction, even though `_callbacks` is declared last: members are + /// destroyed only *after* the destructor body, so a body that later grew a + /// call pumping an event loop would otherwise deliver into a half-dead + /// object. The body does not do that today; stopping first keeps it correct + /// if one is added. + /// + /// The strength of the gate depends on which thread destroys this object, + /// exactly as callback_scope.md's "Thread safety and the boundary of + /// the guarantee" describes. + /// Destroying it off the delivery thread is advisory only, and that caller + /// owns its own synchronisation. + ~SectionSet() { _callbacks.requestStop(); } +``` + +- [ ] **Step 4: Run the full sections suite** + +```bash +cmake --build build/clang-debug --target morph_tests -j 12 && ./build/clang-debug/tests/morph_tests "[sections]" +``` + +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add include/morph/forms/sections.hpp tests/test_sections.cpp +git commit -m "forms: SectionSet stops its callback scope before teardown" +``` + +--- + +### Task 8: Spec document and the full gate run + +Required: the Header ↔ spec sync gate fails a PR that changes `include/morph/forms/` without changing +`docs/spec/forms/`. + +**Files:** +- Create: `docs/spec/forms/sections.md` +- Modify: `scripts/branch_partial_allowlist.json` (only if re-pinning is needed) + +- [ ] **Step 1: Write the spec document** + +`docs/spec/forms/sections.md` covering, in this order: what `SectionSet` is and when to choose it over +`FlowSession`; the declaration types; `set`/`reset`/`draft`/`resolved` with their contracts; the no-latch rule and +why; that binds are declarations the renderer acts on, never framework writes; the `s-*` schema shape and the +deliberate absence of an order field; concurrency and the callback-scope boundary; and the two `static_assert`s +(duplicate action types, and a field outside the group) as the compile-time contract, with a short negative example +for each. + +- [ ] **Step 2: Run every gate CI applies** + +```bash +# format, whole tree +git ls-files -z '*.hpp' '*.cpp' | xargs -0 clang-format --dry-run -Werror + +# documentation warnings, glaze as a system header the way CI treats it +clang++ -std=c++23 -fsyntax-only -Wdocumentation -Wno-documentation-unknown-command -Werror \ + -Iinclude -isystem build/clang-coverage/_deps/glaze-src/include \ + <(printf '#include \nint main(){}\n') -x c++ - + +# both compilers +cmake --build build/clang-debug --target morph_tests -j 12 +cmake --build build/gcc-net --target morph_tests -j 12 + +# suites +./build/clang-debug/tests/morph_tests +./build/clang-debug/tests/net/morph_net_tests + +# spec citations +bash scripts/check_spec_citations.sh + +# allowlist, both ways +python3 -c " +import json,pathlib,re +d=json.load(open('scripts/branch_partial_allowlist.json')); byline={e['line'] for e in d['entries']}; bad=0 +for e in d['entries']: + lines=pathlib.Path(e['file']).read_text().split(chr(10)) + if lines[e['line']-1].strip()!=e['source'].strip(): bad+=1; print('SOURCE',e['file'],e['line']) + for r in re.findall(r'(?:line |\.hpp:)([0-9]+)', e['reason']): + if int(r) not in byline: bad+=1; print('PROSE',e['file'],e['line'],r) +print(f'{len(d[\"entries\"])} entries, {bad} stale')" +``` + +Expected: format silent, no documentation warnings, both builds clean, both suites passing, spec lint OK, allowlist +0 stale. + +- [ ] **Step 3: clang-tidy on the branch's changed lines** + +```bash +git diff -U0 origin/master...HEAD -- '*.cpp' '*.hpp' > /tmp/br.diff +# Extract changed line numbers, run clang-tidy on changed .cpp files, and report +# only findings whose file:line appears in the diff. Diff against +# origin/master...HEAD, NOT the working tree -- the working tree is empty after +# a commit, which makes the filter silently vacuous. +``` + +Expected: no findings on changed lines. + +- [ ] **Step 4: Verify every test fails without its implementation** + +For each case added in Tasks 2-7, stash the corresponding part of `sections.hpp`, rebuild, and confirm the case +fails. A case that passes with the implementation removed is not a test. + +- [ ] **Step 5: Commit and open the PR** + +```bash +git add docs/spec/forms/sections.md scripts/branch_partial_allowlist.json +git commit -m "docs: spec for unordered sections (#513)" +``` + +PR body must state: what SectionSet is, that case 1 is the morph#513 regression, the measured before/after for each +test, and that binds are declarations rather than writes (with the reason, since the first design got this wrong). + +--- + +## Self-Review + +**Spec coverage.** Every section of `2026-09-10-unordered-sections-design.md` maps to a task: Headers → Task 1-2; +Declaration layer → Task 2; SectionSet surface → Tasks 3-5; Prefill → Tasks 2 (schema) and 5 (resolved); +Concurrency and lifetime → Tasks 5, 7; Schema document → Task 2; Testing cases 1-8 → Tasks 2-7; case 9 +(`static_assert`s) → Task 8's spec document, as the spec says it is compile-time and covered by documented negative +examples. + +**Placeholders.** None. Every code step carries the code. + +**Type consistency.** `Section::action`/`binds`/`title()`, `SectionGroup::sections`/`title()`, +`SectionGroupTraits::typeId()`, `sectionGroupSchemaJson()`, and `SectionSet`'s +`set`/`reset`/`draft`/`resolved`/`fire`/`captureResult`/`logUnhandledError` are spelled identically in every task +that references them. `forStep` is renamed to `forPackElement` in Task 1 and used under that name thereafter. diff --git a/docs/superpowers/specs/2026-09-10-unordered-sections-design.md b/docs/superpowers/specs/2026-09-10-unordered-sections-design.md new file mode 100644 index 00000000..3ed9c104 --- /dev/null +++ b/docs/superpowers/specs/2026-09-10-unordered-sections-design.md @@ -0,0 +1,212 @@ +# Unordered sections — design + +`morph::forms::SectionSet` gives a screen N independently +editable action drafts, each gated on its own `ActionValidator::ready` and +each dispatched through the handler's ordinary `execute()`. No sequence, no +current step, no advance/back. + +Closes [morph#513](https://github.com/LASTRADA-Software/morph/issues/513). + +## Contents + +- [The gap](#the-gap) +- [Headers](#headers) +- [Declaration layer](#declaration-layer) +- [SectionSet](#sectionset) +- [Prefill](#prefill) +- [Concurrency and lifetime](#concurrency-and-lifetime) +- [Schema document](#schema-document) +- [Testing](#testing) +- [Out of scope](#out-of-scope) + +## The gap + +`779bd8aa` removed the handler-side reactive draft (`BridgeHandler::set<&A::field>`, +`reset`, the action-keyed `subscribe`) and named +`morph::flows::FlowSession` as its replacement. `FlowSession` serves a wizard: +one active step at a time, enforced at `flows.hpp:293`, which throws +`std::logic_error` for a field belonging to any other step. + +That leaves the screen shape the removed mechanism also served — N sections a +user edits in any order, each with its own draft and its own readiness gate — +with no in-framework target. A consumer either reimplements draft accumulation +locally or restructures the screen into a wizard it is not. + +`SectionSet` is the sibling for that shape. + +## Headers + +| File | Holds | +| --- | --- | +| `forms/detail/session_common.hpp` *(new)* | `Bind`, `forEachTupleElement`, `forPackElement`, `AllDistinct`, prefill-path resolution, schema-emission helper | +| `forms/flows.hpp` | `Wizard`, `WizardStep`, `FlowSession` — unchanged behaviour | +| `forms/sections.hpp` *(new)* | `Section`, `SectionGroup`, `SectionGroupTraits`, `BRIDGE_REGISTER_SECTION_GROUP`, `sectionGroupSchemaJson`, `SectionSet` | + +The shared header is an extraction, not a rewrite: `flows.hpp` keeps every +public name it exports today and gains an include. Splitting it this way keeps +`flows.hpp` at its present size instead of doubling it, and gives `Bind` — which +both session types genuinely share — one home rather than two. + +## Declaration layer + +`Section` mirrors `WizardStep` structurally and is named for its own domain: + +```cpp +template +struct Section { + using action = Action; + using binds = std::tuple; + [[nodiscard]] static constexpr std::string_view title() noexcept; +}; + +template +struct SectionGroup { + using sections = std::tuple; + [[nodiscard]] static constexpr std::string_view title() noexcept; +}; +``` + +`SectionGroupTraits` maps a group type to its string type-id, specialised via +`BRIDGE_REGISTER_SECTION_GROUP(G, "name")`. It exists for the same reason +`WizardTraits` does: `sectionGroupSchemaJson()` needs a stable name for the +group in the emitted document, and deriving one from the C++ type would tie the +wire format to a mangled name. + +The two trait structs are the only duplication against `flows.hpp`; everything +substantive lives in the shared header. Reusing `WizardStep` for an unordered +screen was rejected: a consumer declaring sections would write `WizardStep`, +which is misleading in exactly the place morph#513 says the model is wrong. + +## SectionSet + +```cpp +template +class SectionSet { + static_assert(sizeof...(Sections) > 0); + static_assert(AllDistinct::value); + +public: + explicit SectionSet(BridgeHandler& handler, + std::function onError = nullptr); + ~SectionSet(); // requestStop() first + + template void set(ValueType value); + template void reset(); + template [[nodiscard]] A draft() const; + [[nodiscard]] std::optional resolved(std::string_view path) const; +}; +``` + +`set<>` keeps `FlowSession`'s compile-time check that the field's action belongs +to the set and drops the runtime current-step `throw`. It takes `_mtx`, writes +the field, copies the draft out, releases the lock, and dispatches through +`handler.execute()` when `ActionValidator::ready(draft)` holds — the same +sequence `FlowSession::set<>` runs, minus the step check. + +**A ready section re-fires on every subsequent `set<>`.** There is no latch and +no fire-once-per-ready-transition rule. This matches `FlowSession`, which +documents that a caller wanting one request per pause debounces on its own side; +a section that silently stopped dispatching after its first success would be a +surprising rule to carry. + +`reset()` returns that section's draft to `A{}` and re-applies any prefill +already resolved for it. It touches no other section. + +`draft()` returns a snapshot for a renderer to display. It is a copy taken +under the lock, not a reference into live state. + +## Prefill + +A section declaring `Bind<&B::field, "A.result">` publishes a *declaration*, not +a write. This matches `FlowSession` exactly, and the parity is worth stating +because it is easy to assume otherwise: `binds` are consumed in exactly one +place in `flows.hpp` — emitted into the schema document under a `prefill` node +for a renderer. `FlowSession` never writes a prefill value into a draft. + +`SectionSet` does the same two things: + +- `sectionGroupSchemaJson()` emits each section's binds under `prefill`, as + `{ "": "." }`. +- On a successful result, the submitted draft's fields and then the result's + fields are recorded into `_resolvedValues` under `"."`, + result fields winning on a name collision — the same order `FlowSession` + records them in. +- `resolved(path)` returns a captured field's JSON-encoded value, or + `std::nullopt` if that path was never captured. + +The renderer decides what to do with a resolved value: it is the component that +knows whether a field the user has already edited should be overwritten, and +the framework has no basis for that judgement. + +**No ordering is implied.** In a wizard a bind's source always precedes its +target, so `resolved` is populated by the time a step is entered. Here a bind +may name a section that has not fired, and `resolved` returns `std::nullopt` +for it — which is the same answer `FlowSession` gives for a path that was never +captured, so the accessor's contract is unchanged. + +## Concurrency and lifetime + +One `_mtx` guards `_drafts` and `_resolvedValues`. It is never held across +`handler.execute()`, and the draft is copied out before dispatch, matching +`FlowSession`. + +One `CallbackScope` gates every installed continuation, and the destructor calls +`requestStop()` explicitly as its first statement rather than leaving it to the +member's own destruction — the same reasoning `FlowSession`'s destructor +records, so a body that later grows a pumping call cannot deliver into a +half-dead session. + +The guarantee has the same boundary as `FlowSession`'s and the same caveat +applies verbatim: destroying the session on the thread its continuations are +delivered on makes check-then-run atomic; destroying it from another thread is +advisory, and that caller owns its own synchronisation. See +`docs/spec/core/callback_scope.md`, "Thread safety and the boundary of the guarantee". + +## Schema document + +`sectionGroupSchemaJson()` emits the group title and one entry per section +carrying its title, action type-id and declared binds. It mirrors +`wizardSchemaJson`'s shape with `s-` keys instead of `w-`, and deliberately +carries no index or order field: a renderer laying out sections chooses its own +arrangement, and emitting a position would imply a sequence the type does not +have. + +## Testing + +`tests/test_sections.cpp`. Every case is written to fail without the feature — +a suite that would pass either way measures nothing. + +| # | Case | Fails without | +| --- | --- | --- | +| 1 | Edit section B, then A; both fire | The feature entirely — this is the shape `FlowSession` throws on | +| 2 | A not-ready draft does not dispatch; a ready one does | The readiness gate | +| 3 | Editing an already-fired section fires again | The no-latch rule | +| 4 | `reset()` clears A, leaves B's draft intact | Per-section isolation | +| 5 | A fires → `resolved("A.field")` returns its value; a path whose section has not fired returns `nullopt` | Result capture and the resolved-values map | +| 6 | `onError` runs on dispatch failure; default path logs | Error routing | +| 7 | Destroying the set with a dispatch in flight delivers nothing | The `CallbackScope` gate | +| 8 | Schema carries each section's title and action id | Schema emission | +| 9 | Duplicate action types, and a field outside the set, are rejected | The two `static_assert`s | + +Case 5 asserts both halves — a captured path and an uncaptured one — because a +test that only checked the captured case would pass against an implementation +that returned a value for everything. + +Case 9 is compile-time. It is covered by a documented negative example rather +than a runtime assertion, since a `static_assert` that fires cannot also be +linked into the suite. + +## Out of scope + +- Changing `FlowSession`. Its single-active-step constraint is correct for a + wizard and stays. +- In-flight coalescing. It was removed with the reactive draft in `779bd8aa` + and is not reintroduced here; `FlowSession` does without it too. +- **Writing prefill values into drafts.** An earlier draft of this design had + the framework write a bound field when its source fired. That is not parity + with `FlowSession` — it is a mechanism that exists nowhere in the tree, and + would need JSON-to-typed-field deserialization keyed by field name. Rejected + in favour of matching the sibling type; if it is wanted later it is its own + piece of work, and the renderer can do it today from `resolved()`. +- A QML renderer for section groups. The schema document is emitted; consuming + it is a separate piece of work. diff --git a/include/morph/forms/app.hpp b/include/morph/forms/app.hpp index 23f25cdf..d4581182 100644 --- a/include/morph/forms/app.hpp +++ b/include/morph/forms/app.hpp @@ -18,6 +18,7 @@ #include #include +#include "detail/session_common.hpp" #include "flows.hpp" namespace morph::app { @@ -125,7 +126,7 @@ template dom["app-title"] = std::string{AppT::title()}; glz::generic_u64::array_t menu{}; - ::morph::flows::detail::forEachTupleElement([&]() { + ::morph::forms::detail::forEachTupleElement([&]() { static_cast(I); glz::generic_u64 entry{}; entry["label"] = std::string{Entry::label()}; @@ -135,7 +136,7 @@ template dom["app-menu"] = menu; auto& screensNode = dom["app-screens"]; - ::morph::flows::detail::forEachTupleElement([&]() { + ::morph::forms::detail::forEachTupleElement([&]() { static_cast(I); auto& screenNode = screensNode[std::string{S::id()}]; screenNode["kind"] = std::string{S::kind()}; diff --git a/include/morph/forms/detail/session_common.hpp b/include/morph/forms/detail/session_common.hpp new file mode 100644 index 00000000..9c4577a5 --- /dev/null +++ b/include/morph/forms/detail/session_common.hpp @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +/// @file forms/detail/session_common.hpp +/// @brief Machinery shared by the two form session types: `morph::flows::FlowSession` +/// (ordered wizard steps) and `morph::forms::SectionSet` (unordered sections). +/// +/// Both declare their units the same way — a registered action, a title, and +/// zero or more `Bind` prefill declarations — and both walk those declarations +/// to emit a schema document. Only the sequencing differs, so the declaration +/// vocabulary and the pack/tuple walkers live here rather than being written +/// twice. + +#include +#include +#include +#include +#include +#include +#include + +#include "../forms.hpp" + +namespace morph::forms { + +/// @brief One `field -> "."` prefill binding declared on a +/// wizard step or an unordered section. +/// +/// A declaration, not a write. Nothing in the framework assigns a bound field: +/// `wizardSchemaJson`/`sectionGroupSchemaJson` emit it for a renderer, and the +/// captured value is read back through `resolved()`. +/// @tparam Field The action's field name to prefill. +/// @tparam Path Source path, `"."`, into captured values. +template +struct Bind { + /// @brief The action field name this binding fills. + /// @return The declared field name. + [[nodiscard]] static constexpr std::string_view field() noexcept { return Field.view(); } + + /// @brief The source path into captured values. + /// @return The declared `"."` path. + [[nodiscard]] static constexpr std::string_view path() noexcept { return Path.view(); } +}; + +namespace detail { + +/// @brief Invokes `visitor.template operator(), I>()` +/// for every element of @p Tuple, in order. +/// @tparam Tuple A `std::tuple<...>` type (only its element types/arity are used). +/// @tparam Visitor Callable with a `template operator()()`. +/// @param visitor Callable invoked once per tuple element. +template +// NOLINTNEXTLINE(cppcoreguidelines-missing-std-forward) — invoked once per element, never moved from +constexpr void forEachTupleElement(Visitor&& visitor) { + // NOLINTNEXTLINE(cppcoreguidelines-rvalue-reference-param-not-moved) — same: one call per element + [](std::index_sequence, Visitor&& innerVisitor) { + (innerVisitor.template operator(), I>(), ...); + }(std::make_index_sequence>{}, std::forward(visitor)); +} + +/// @brief Invokes `visitor.template operator()()` for the pack element of +/// `Ts...` at runtime position @p index. A no-op when +/// `index >= sizeof...(Ts)`. +/// @tparam Ts The pack to index into. +/// @tparam Visitor Callable with a `template operator()()`. +/// @param index 0-based position to visit. +/// @param visitor Callable invoked for the element at @p index. +template +// NOLINTNEXTLINE(cppcoreguidelines-missing-std-forward) — the visited element is a run-time choice +constexpr void forPackElement(std::size_t index, Visitor&& visitor) { + std::size_t i = 0; + (void)((i++ == index ? (visitor.template operator()(), true) : false) || ...); +} + +/// @brief Trait: `true` when every type in `Ts...` is pairwise distinct. +/// @tparam Ts Types to check for pairwise distinctness. +template +struct AllDistinct : std::true_type {}; + +/// @brief Recursive case: `T` distinct from every type in `Rest...`, and `Rest...` pairwise distinct. +/// @tparam T The type being checked against `Rest...`. +/// @tparam Rest The remaining types. +template +struct AllDistinct : std::bool_constant<(!std::is_same_v && ...) && AllDistinct::value> { +}; + +/// @brief Writes each `Bind` in @p BindsTuple into @p node as `"": ""`. +/// +/// A no-op for an empty tuple, so callers guard on arity only to avoid +/// materialising an empty `prefill` object. +/// @tparam BindsTuple `std::tuple...>`. +/// @param node Destination JSON object node. +// NOLINTBEGIN(cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) -- glaze DOM requires operator[] +template +void emitBindsInto(glz::generic_u64& node) { + forEachTupleElement([&]() { + static_cast(J); + node[std::string{BindT::field()}] = std::string{BindT::path()}; + }); +} +// NOLINTEND(cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) + +} // namespace detail +} // namespace morph::forms diff --git a/include/morph/forms/flows.hpp b/include/morph/forms/flows.hpp index 16995668..bf559ab6 100644 --- a/include/morph/forms/flows.hpp +++ b/include/morph/forms/flows.hpp @@ -41,25 +41,20 @@ #include "../core/bridge.hpp" #include "../core/callback_scope.hpp" #include "../core/logger.hpp" +#include "detail/session_common.hpp" #include "forms.hpp" namespace morph::flows { -/// @brief One `field -> "."` prefill binding declared on -/// a wizard step. +/// @brief Prefill binding for a wizard step. +/// +/// Alias for `morph::forms::Bind`, which both session types share. Kept in this +/// namespace because it is the name shipped consumers already write. /// @tparam Field The step action's field name to prefill. /// @tparam Path Source path, `"."`, into an earlier /// step's captured values (see `FlowSession::resolved`). template -struct Bind { - /// @brief The step action's field name this binding fills. - /// @return The declared field name. - [[nodiscard]] static constexpr std::string_view field() noexcept { return Field.view(); } - - /// @brief The source path into an earlier step's captured values. - /// @return The declared `"."` path. - [[nodiscard]] static constexpr std::string_view path() noexcept { return Path.view(); } -}; +using Bind = morph::forms::Bind; /// @brief One step of a `Wizard`: a registered action, a display title, and /// zero or more `Bind` prefill declarations. @@ -101,47 +96,6 @@ struct Wizard { template struct WizardTraits; // forward — specialise or use BRIDGE_REGISTER_WIZARD -namespace detail { - -/// @brief Invokes `visitor.template operator(), I>()` -/// for every element of @p Tuple, in order. -/// @tparam Tuple A `std::tuple<...>` type (only its element types/arity are used). -/// @tparam Visitor Callable with a `template operator()()`. -/// @param visitor Callable invoked once per tuple element. -template -constexpr void forEachTupleElement(Visitor&& visitor) { - [](std::index_sequence, Visitor&& innerVisitor) { - (innerVisitor.template operator(), I>(), ...); - }(std::make_index_sequence>{}, std::forward(visitor)); -} - -/// @brief Invokes `visitor.template operator()()` for the pack element -/// of `Steps...` at runtime position @p index. A no-op when -/// `index >= sizeof...(Steps)`. -/// @tparam Steps The pack to index into. -/// @tparam Visitor Callable with a `template operator()()`. -/// @param index 0-based position to visit. -/// @param visitor Callable invoked for the step at @p index. -template -constexpr void forStep(std::size_t index, Visitor&& visitor) { - std::size_t i = 0; - (void)((i++ == index ? (visitor.template operator()(), true) : false) || ...); -} - -/// @brief Trait: `true` when every type in `Ts...` is pairwise distinct. -/// @tparam Ts Types to check for pairwise distinctness. -template -struct AllDistinct : std::true_type {}; - -/// @brief Recursive case: `T` distinct from every type in `Rest...`, and `Rest...` pairwise distinct. -/// @tparam T The type being checked against `Rest...`. -/// @tparam Rest The remaining types. -template -struct AllDistinct : std::bool_constant<(!std::is_same_v && ...) && AllDistinct::value> { -}; - -} // namespace detail - /// @brief Generates the `w-*` JSON document for wizard type @p W. /// /// Emits `w-title` and an ordered `w-steps` array; each step carries `action` @@ -158,17 +112,14 @@ template dom["w-title"] = std::string{W::title()}; glz::generic_u64::array_t steps{}; - detail::forEachTupleElement([&]() { + ::morph::forms::detail::forEachTupleElement([&]() { static_cast(I); glz::generic_u64 step{}; step["action"] = std::string{::morph::model::ActionTraits::typeId()}; step["title"] = std::string{StepT::title()}; if constexpr (std::tuple_size_v != 0) { - auto& prefillNode = step["prefill"]; - detail::forEachTupleElement([&]() { - static_cast(J); - prefillNode[std::string{BindT::field()}] = std::string{BindT::path()}; - }); + // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) -- glaze DOM requires operator[] + ::morph::forms::detail::emitBindsInto(step["prefill"]); } steps.emplace_back(std::move(step)); }); @@ -199,7 +150,8 @@ template template class FlowSession { static_assert(sizeof...(Steps) > 0, "FlowSession: a flow needs at least one step"); - static_assert(detail::AllDistinct::value, "FlowSession: step action types must be pairwise distinct"); + static_assert(::morph::forms::detail::AllDistinct::value, + "FlowSession: step action types must be pairwise distinct"); public: /// @brief Constructs a flow over @p handler, starting at step 0. @@ -406,7 +358,8 @@ class FlowSession { /// @return Empty when `finished()` (no current step). [[nodiscard]] std::string_view currentActionType() const noexcept { std::string_view id{}; - detail::forStep(_index, [&id] { id = ::morph::model::ActionTraits::typeId(); }); + ::morph::forms::detail::forPackElement( + _index, [&id] { id = ::morph::model::ActionTraits::typeId(); }); return id; } diff --git a/include/morph/forms/sections.hpp b/include/morph/forms/sections.hpp new file mode 100644 index 00000000..30a2ab7b --- /dev/null +++ b/include/morph/forms/sections.hpp @@ -0,0 +1,345 @@ +// SPDX-License-Identifier: Apache-2.0 + +#pragma once + +/// @file forms/sections.hpp +/// @brief Unordered sections: N independently editable action drafts on one +/// screen, each with its own readiness gate and no sequencing. +/// +/// The sibling to `morph::flows::FlowSession`. A wizard has one active step and +/// throws on a field belonging to any other; a section set has no active +/// section at all -- every declared section is editable at every moment, and +/// each dispatches on its own as soon as its draft validates. +/// +/// Choose this when order carries no meaning (tabs, cards, a settings page) and +/// `FlowSession` when it does. Like flows, this is additive metadata and +/// dispatch bookkeeping over the ordinary `BridgeHandler::execute()` +/// path: no new wire format, no new execution mode. +/// See docs/spec/forms/sections.md. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "../attributes.hpp" +#include "../core/bridge.hpp" +#include "../core/callback_scope.hpp" +#include "../core/logger.hpp" +#include "detail/session_common.hpp" + +namespace morph::forms { + +/// @brief One section of a `SectionGroup`: a registered action, a display +/// title, and zero or more `Bind` prefill declarations. +/// @tparam Action Registered action type (`BRIDGE_REGISTER_ACTION`) this section fires. +/// @tparam Title Human title for the section (tab label / card header). +/// @tparam Binds Zero or more `Bind` prefill declarations. +template +struct Section { + /// @brief The section's action type. + using action = Action; + + /// @brief Tuple of this section's `Bind<...>` prefill declarations (possibly empty). + using binds = std::tuple; + + /// @brief The section's display title. + /// @return The declared title. + [[nodiscard]] static constexpr std::string_view title() noexcept { return Title.view(); } +}; + +/// @brief An unordered set of `Section`s sharing one screen. +/// @tparam Title Human title for the whole group. +/// @tparam Sections One or more `Section` types. +template +struct SectionGroup { + /// @brief Tuple of this group's `Section<...>` types. + using sections = std::tuple; + + /// @brief The group's display title. + /// @return The declared title. + [[nodiscard]] static constexpr std::string_view title() noexcept { return Title.view(); } +}; + +/// @brief Traits specialisation mapping a `SectionGroup` type to its string type-id. +/// +/// Specialise via `BRIDGE_REGISTER_SECTION_GROUP` rather than by hand. The +/// default is a forward declaration — using it without a specialisation is an +/// incomplete-type error. The schema needs a stable name for the group; +/// deriving one from the C++ type would tie the wire format to a mangled name. +/// @tparam G Concrete `SectionGroup<...>` type. +template +struct SectionGroupTraits; // forward — specialise or use BRIDGE_REGISTER_SECTION_GROUP + +/// @brief Generates the `s-*` JSON document for section group @p G. +/// +/// Emits `s-id`, `s-title`, and an `s-sections` array; each entry carries +/// `action` (the section's registered action type-id), `title`, and — only when +/// the section declares at least one `Bind` — a `prefill` object mapping field +/// name to `"."` path. +/// +/// Deliberately emits no index or order field. A renderer arranges the sections +/// itself, and a position in the document would suggest a sequence this type +/// does not have. +/// @tparam G Concrete `SectionGroup` type. +/// @return The group's JSON document. Empty string only if glaze's own JSON +/// writer fails on the assembled DOM (schema generation never throws). +// NOLINTBEGIN(cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) -- glaze DOM requires operator[] +template +[[nodiscard]] std::string sectionGroupSchemaJson() { + glz::generic_u64 dom{}; + dom["s-id"] = std::string{SectionGroupTraits::typeId()}; + dom["s-title"] = std::string{G::title()}; + + glz::generic_u64::array_t sections{}; + detail::forEachTupleElement([&]() { + static_cast(I); + glz::generic_u64 entry{}; + entry["action"] = std::string{::morph::model::ActionTraits::typeId()}; + entry["title"] = std::string{SectionT::title()}; + if constexpr (std::tuple_size_v != 0) { + detail::emitBindsInto(entry["prefill"]); + } + sections.emplace_back(std::move(entry)); + }); + dom["s-sections"] = sections; + + return glz::write_json(dom).value_or(std::string{}); +} +// NOLINTEND(cppcoreguidelines-pro-bounds-avoid-unchecked-container-access) + +/// @brief Drives N independently editable action drafts on one screen. +/// +/// Each section accumulates its own draft through `set<>` and dispatches +/// through the handler as soon as `ActionValidator::ready` accepts it. +/// There is no active section and no sequence: a field belonging to any +/// declared section may be set at any time, in any order. This is the whole +/// difference from `morph::flows::FlowSession`, which throws on a field +/// outside its current step. +/// +/// A ready section re-fires on every subsequent `set<>`. There is no latch and +/// no coalescing, matching `FlowSession`; a caller that wants one request per +/// pause debounces on its own side, where it knows what a pause means. +/// @tparam Model The model the handler is bound to. +/// @tparam Sections One or more `Section` types. +template +class SectionSet { + static_assert(sizeof...(Sections) > 0, "SectionSet: a group needs at least one section"); + static_assert(::morph::forms::detail::AllDistinct::value, + "SectionSet: section action types must be pairwise distinct"); + +public: + /// @brief Constructs a section set dispatching through @p handler. + /// @param handler Handler every section dispatches through. Must outlive + /// this `SectionSet`. + /// @param onError Optional callback invoked when a section's dispatch + /// fails. When absent, the error is logged via + /// `morph::log::logError`. Stored and invoked for this + /// object's whole lifetime, so anything the callable refers + /// to must outlive it. + explicit SectionSet(::morph::bridge::BridgeHandler& handler MORPH_LIFETIMEBOUND, + std::function onError MORPH_LIFETIMEBOUND = nullptr) + : _handler{handler}, _onError{std::move(onError)} {} + + SectionSet(const SectionSet&) = delete; + SectionSet& operator=(const SectionSet&) = delete; + SectionSet(SectionSet&&) = delete; + SectionSet& operator=(SectionSet&&) = delete; + + /// @brief Stops every callback this set installed from being delivered. + /// + /// There is nothing to detach: a section's continuations are owned by the + /// in-flight dispatch, not held in a map this object could remove itself + /// from. `_callbacks` gates each one on a token it checks before touching + /// `this`, so a completion resolving after this object is gone finds the + /// token stopped and returns without dereferencing anything. + /// + /// `requestStop()` is called explicitly rather than left to the member's + /// own destruction, even though `_callbacks` is declared last: members are + /// destroyed only *after* the destructor body runs, so a body that later + /// grew a call pumping an event loop would otherwise deliver into a + /// half-dead object. The body does not do that today; stopping first keeps + /// it correct if one is ever added. + /// + /// How strong the gate is depends on which thread destroys this object, + /// exactly as `CallbackScope`'s "Boundary of the guarantee" describes. + /// Destroying it off the delivery thread is advisory only, and that caller + /// owns its own synchronisation. + ~SectionSet() { _callbacks.requestStop(); } + + /// @brief Sets one field of its section's draft, dispatching that section + /// if the draft is now ready. + /// + /// Unlike `FlowSession::set<>` this imposes no ordering: the field's action + /// need only be one of the declared sections. Readiness is evaluated on a + /// copy taken under the lock, so the dispatch happens with the lock + /// released and a concurrent edit to another section cannot block on it. + /// @tparam FieldPtr Pointer-to-data-member of a declared section's action struct. + /// @param value New value for the field. + template + void set(::morph::bridge::detail::MemberPointerTraits::ValueType value) { + using A = ::morph::bridge::detail::MemberPointerTraits::ClassType; + static_assert((std::is_same_v || ...), + "SectionSet::set<>: the field's action is not a section of this group"); + A draft{}; + { + std::scoped_lock const lock{_mtx}; + std::get(_drafts).*FieldPtr = std::move(value); + draft = std::get(_drafts); + } + if (::morph::model::ActionValidator::ready(draft)) { + fire(std::move(draft)); + } + } + + /// @brief Clears one section's draft back to a default-constructed action. + /// + /// Touches no other section and dispatches nothing. Values already captured + /// from a previous successful dispatch stay in `resolved()`: they describe + /// what the model was told, which resetting an editor does not undo. + /// @tparam A The section's action type. + template + void reset() { + static_assert((std::is_same_v || ...), + "SectionSet::reset<>: not a section of this group"); + std::scoped_lock const lock{_mtx}; + std::get(_drafts) = A{}; + } + + /// @brief Snapshots one section's current draft. + /// + /// A copy taken under the lock, not a reference into live state, so a + /// renderer can read one section while another thread edits a different one. + /// @tparam A The section's action type. + /// @return The draft as it stands. + template + [[nodiscard]] A draft() const { + static_assert((std::is_same_v || ...), + "SectionSet::draft<>: not a section of this group"); + std::scoped_lock const lock{_mtx}; + return std::get(_drafts); + } + + /// @brief Looks up a value captured from a section's submitted draft or result. + /// @param path `"."`, matching a section's declared `Bind::path()`. + /// @return The field's JSON-encoded value, or `std::nullopt` if @p path was + /// never captured — the section never fired successfully, or has no + /// such field. + [[nodiscard]] std::optional resolved(std::string_view path) const { + std::scoped_lock const lock{_mtx}; + auto iter = _resolvedValues.find(std::string{path}); + if (iter == _resolvedValues.end()) { + return std::nullopt; + } + return iter->second; + } + +private: + /// @brief Records section @p A's submitted draft and its result under + /// `"."` keys. + /// + /// Draft fields go in first and result fields overwrite them on a name + /// collision: the result is what the model actually settled on, so it is + /// the value a later section should prefill from. + /// @tparam A Section action type. + /// @param result The dispatch's successful result. + template + void captureResult(const ::morph::model::ActionTraits::Result& result) { + std::scoped_lock const lock{_mtx}; + auto const typeId = ::morph::model::ActionTraits::typeId(); + auto record = [&](const auto& value) { + ::morph::forms::detail::forEachNamedMember( + value, [&](std::string_view name, const auto& member) { + static_cast(I); + std::string json; + // The `!write_json(...)` guard's false arm (write failure) + // is not exercised by this file's own test suite: every + // section action's fields are plain, well-formed data + // glaze's JSON writer cannot fail on. The same + // "untestable line" flows.hpp's captureResult already + // carries, for the same reason. + if (!glz::write_json(member, json)) { + _resolvedValues[std::string{typeId} + "." + std::string{name}] = std::move(json); + } + }); + }; + record(std::get(_drafts)); + record(result); + } + + /// @brief Reports a dispatch failure no `onError` callback was given for. + /// @param typeId The failing section's action type-id. + /// @param err The captured exception. + static void logUnhandledError(std::string_view typeId, const std::exception_ptr& err) { + try { + std::rethrow_exception(err); + } catch (const std::exception& exc) { + ::morph::log::logError(std::string{"[section:"} + std::string{typeId} + + "] unhandled exception: " + exc.what()); + } catch (...) { + ::morph::log::logError(std::string{"[section:"} + std::string{typeId} + "] unhandled unknown exception"); + } + } + + /// @brief Dispatches section @p A's ready draft and routes its outcome. + /// + /// Both closures are gated on `_callbacks`, so neither touches anything on + /// `this` once the set has been destroyed — a completion can still resolve + /// after the set is gone. + /// + /// Nothing here is keyed to a "current" section, which is what makes + /// sections cheaper than flow steps: a late reply cannot be stale, because + /// there is no position for it to be stale relative to. + /// @tparam A Section action type. + /// @param draft The ready action to execute. + template + void fire(A draft) { + _handler.execute(std::move(draft)) + .then(_callbacks, + [this](const ::morph::model::ActionTraits::Result& result) { + this->template captureResult(result); + }) + .onError(_callbacks, [this](const std::exception_ptr& err) { + if (_onError) { + _onError(err); + } else { + logUnhandledError(::morph::model::ActionTraits::typeId(), err); + } + }); + } + + ::morph::bridge::BridgeHandler& _handler; + std::function _onError; + // _handler/_onError are set once at construction and never reassigned. + // Everything below is touched both by the owning thread (set/reset/draft) + // and by a dispatch's continuation, which runs on whatever thread resolves + // the BridgeHandler completion -- see docs/spec/core/bridge.md's + // executor/callback model. + mutable std::mutex _mtx; + std::tuple _drafts{}; + std::unordered_map _resolvedValues; + // Declared last, so it is the first member destroyed: every gated callback + // is stopped before the state those callbacks touch goes away. + ::morph::async::CallbackScope _callbacks; +}; + +} // namespace morph::forms + +// clang-format off -- public macro surface; see CONTRIBUTING.md, "Formatting/linting". +// NOLINTBEGIN(cppcoreguidelines-macro-usage) — registration macro is the intended public API +/// @brief Specialises `morph::forms::SectionGroupTraits` with the string type-id @p NAME. +#define BRIDGE_REGISTER_SECTION_GROUP(G, NAME) \ + template <> \ + struct morph::forms::SectionGroupTraits { \ + static constexpr std::string_view typeId() noexcept { return NAME; } \ + }; +// clang-format on +// NOLINTEND(cppcoreguidelines-macro-usage) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 054f1574..93d8ef24 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -100,6 +100,7 @@ add_executable(morph_tests test_quantity_decode_validation.cpp test_nested_forms.cpp test_flows_apps.cpp + test_sections.cpp test_views.cpp test_computed_fields.cpp test_forms_rules.cpp diff --git a/tests/test_sections.cpp b/tests/test_sections.cpp new file mode 100644 index 00000000..bdbbf97b --- /dev/null +++ b/tests/test_sections.cpp @@ -0,0 +1,413 @@ +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "test_support.hpp" + +// --------------------------------------------------------------------------- +// Fixture: two independent sections of one screen -- a profile block and a +// preferences block. Neither is a step of the other. Editing them in either +// order is the whole point of SectionSet, and is what FlowSession refuses. +// --------------------------------------------------------------------------- + +namespace { +/// Records which actions the model actually executed, in order. +struct SecRecorder { + std::mutex mtx; + std::vector fired; + + void record(std::string what) { + std::scoped_lock const lock{mtx}; + fired.push_back(std::move(what)); + } + void clear() { + std::scoped_lock const lock{mtx}; + fired.clear(); + } + [[nodiscard]] std::vector snapshot() { + std::scoped_lock const lock{mtx}; + return fired; + } +}; +SecRecorder& recorder() { + static SecRecorder inst; + return inst; +} +} // namespace + +struct SecProfile { + std::string name; + [[nodiscard]] bool validate() const { return !name.empty(); } +}; +struct SecProfileResult { + std::int64_t id = 0; +}; + +struct SecPrefs { + std::int64_t profileId = 0; + std::string theme; + [[nodiscard]] bool validate() const { return !theme.empty(); } +}; +struct SecPrefsResult { + std::string summary; +}; + +struct SecExplodes { + std::string label; + [[nodiscard]] bool validate() const { return !label.empty(); } +}; +struct SecExplodesResult { + std::int64_t id = 0; +}; + +// A section whose model call blocks until the test releases it, so a dispatch +// can be held genuinely in flight across the SectionSet's destruction. Both +// flags are function-local statics for the same reason as the recorder: the +// model's execute() and the test body both need them, and SecModel is +// stateless. Mirrors test_flows_apps.cpp's FlowStepSlow. +namespace { +std::atomic& secSlowStarted() { + static std::atomic flag{false}; + return flag; +} +std::atomic& secSlowRelease() { + static std::atomic flag{false}; + return flag; +} +} // namespace + +struct SecSlow { + std::string label; + [[nodiscard]] bool validate() const { return !label.empty(); } +}; +struct SecSlowResult { + std::int64_t id = 0; +}; + +struct SecModel { + SecProfileResult execute(const SecProfile& action) { + recorder().record("SectionsTest_Profile"); + return {.id = static_cast(action.name.size())}; + } + SecPrefsResult execute(const SecPrefs& action) { + recorder().record("SectionsTest_Prefs"); + return {.summary = action.theme}; + } + SecExplodesResult execute(const SecExplodes&) { + recorder().record("SectionsTest_Explodes"); + throw std::runtime_error{"section boom"}; + } + static SecSlowResult execute(const SecSlow& /*action*/) { + secSlowStarted().store(true, std::memory_order_relaxed); + while (!secSlowRelease().load(std::memory_order_relaxed)) { + std::this_thread::sleep_for(std::chrono::milliseconds{1}); + } + throw std::runtime_error{"late section boom"}; + } +}; + +BRIDGE_REGISTER_MODEL(SecModel, "SectionsTest_Model") +BRIDGE_REGISTER_ACTION(SecModel, SecProfile, "SectionsTest_Profile") +BRIDGE_REGISTER_ACTION(SecModel, SecPrefs, "SectionsTest_Prefs") +BRIDGE_REGISTER_ACTION(SecModel, SecExplodes, "SectionsTest_Explodes") +BRIDGE_REGISTER_ACTION(SecModel, SecSlow, "SectionsTest_Slow") + +using ProfileSection = morph::forms::Section; +using PrefsSection = + morph::forms::Section>; +using ExplodesSection = morph::forms::Section; +using SlowSection = morph::forms::Section; + +using DemoGroup = morph::forms::SectionGroup<"Account settings", ProfileSection, PrefsSection>; +BRIDGE_REGISTER_SECTION_GROUP(DemoGroup, "SectionsTest_DemoGroup") + +// Delivers every queued completion on the *test* thread and returns how many ran. +// +// SectionSet's callbacks are gated on a CallbackScope, which by contract does +// not wait for a callback already past its token check (callback_scope.md). +// Polling a published value is not enough to know one has finished: captureResult +// makes its first key visible through resolved() while it is still writing the +// rest, so a test that waits on resolved() and then leaves scope can destroy the +// set underneath its own completion. That is a real use-after-free -- it +// segfaulted on Windows CI and TSan reproduces it about once in fifteen runs. +// +// Draining on the test thread removes the race rather than narrowing it: the +// thread that delivers the callback is the thread that destroys the set, which +// is exactly the case CallbackScope's guarantee covers. +namespace { +std::size_t drain(morph::testing::StepExecutor& exec) { + REQUIRE(morph::testing::waitUntil([&exec] { return exec.pending() > 0; })); + std::size_t ran = exec.runAll(); + // A completion can post follow-up work; keep going while more arrives. + while (morph::testing::waitUntil([&exec] { return exec.pending() > 0; }, std::chrono::milliseconds{50})) { + ran += exec.runAll(); + } + return ran; +} +} // namespace + +TEST_CASE("sectionGroupSchemaJson carries each section's title, action and binds", "[sections]") { + auto const json = morph::forms::sectionGroupSchemaJson(); + REQUIRE_FALSE(json.empty()); + + glz::generic_u64 dom{}; + REQUIRE_FALSE(glz::read_json(dom, json)); + + CHECK(json.contains(R"("s-id":"SectionsTest_DemoGroup")")); + CHECK(json.contains(R"("s-title":"Account settings")")); + CHECK(json.contains(R"("action":"SectionsTest_Profile")")); + CHECK(json.contains(R"("title":"Profile")")); + CHECK(json.contains(R"("action":"SectionsTest_Prefs")")); + CHECK(json.contains(R"("prefill":{"profileId":"SectionsTest_Profile.id"})")); + + // A section with no Bind carries no prefill key at all, rather than an + // empty object a renderer would have to special-case. + auto const& sections = dom["s-sections"].get_array(); + REQUIRE(sections.size() == 2); + CHECK_FALSE(sections[0].contains("prefill")); + + // No order is implied: a section carries no index field, because a renderer + // chooses its own arrangement and an emitted position would suggest a + // sequence a SectionSet does not have. + CHECK_FALSE(sections[0].contains("index")); + CHECK_FALSE(sections[1].contains("index")); +} + +// --------------------------------------------------------------------------- +// Each case builds its own bridge/handler/section set: a SectionSet holds no +// global state, and a shared one would let a late dispatch from a previous case +// land in this one's recorder. +// --------------------------------------------------------------------------- + +TEST_CASE("SectionSet: sections fire independently, in any order", "[sections]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::testing::StepExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + morph::forms::SectionSet sections{handler}; + + recorder().clear(); + + // Edit the SECOND section first. Under FlowSession this throws + // std::logic_error -- "field belongs to an action that is not the current + // step" -- which is exactly the gap morph#513 reports. + sections.set<&SecPrefs::theme>("dark"); + drain(cbExec); + + // Then the first. Both fire; neither was ever "current". + sections.set<&SecProfile::name>("ada"); + drain(cbExec); + + auto const fired = recorder().snapshot(); + REQUIRE(fired.size() == 2); + CHECK(fired[0] == "SectionsTest_Prefs"); + CHECK(fired[1] == "SectionsTest_Profile"); + CHECK(sections.resolved("SectionsTest_Prefs.summary").has_value()); + CHECK(sections.resolved("SectionsTest_Profile.id").has_value()); +} + +TEST_CASE("SectionSet: a not-ready draft is not sent at all", "[sections]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::testing::StepExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + // The gate is observed through onError, not through the recorder. The + // bridge enforces ActionValidator on its own dispatch path (bridge.hpp), + // so an ungated draft would never reach SecModel::execute either -- it + // would come back as a validation failure. A round trip that can only fail + // is the cost this gate exists to avoid, and the error callback is where + // that cost is visible. + std::atomic errors{0}; + morph::forms::SectionSet sections{ + handler, [&errors](const std::exception_ptr&) { errors.fetch_add(1); }}; + + recorder().clear(); + + // SecPrefs::validate() requires a non-empty theme; profileId alone is not ready. + sections.set<&SecPrefs::profileId>(7); + CHECK(cbExec.pending() == 0); + CHECK(cbExec.runAll() == 0); + CHECK(errors.load() == 0); + + // Completing it dispatches, carrying the field set earlier, and succeeds. + // Waiting on the capture rather than on the recorder: the recorder is + // written inside execute(), which runs before the completion that captures. + sections.set<&SecPrefs::theme>("light"); + drain(cbExec); + CHECK(sections.resolved("SectionsTest_Prefs.summary").has_value()); + CHECK(recorder().snapshot().size() == 1); + CHECK(errors.load() == 0); + CHECK(sections.resolved("SectionsTest_Prefs.profileId") == std::string{"7"}); +} + +TEST_CASE("SectionSet: an already-fired section fires again on the next edit", "[sections]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::testing::StepExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + morph::forms::SectionSet sections{handler}; + + recorder().clear(); + sections.set<&SecProfile::name>("ada"); + drain(cbExec); + REQUIRE(recorder().snapshot().size() == 1); + + // No latch: the draft is still ready, so it dispatches again. Matching + // FlowSession, which also re-fires a ready step on every set<>. + sections.set<&SecProfile::name>("grace"); + drain(cbExec); + CHECK(recorder().snapshot().size() == 2); + // The second dispatch really carried the new value, not a replay of the first. + CHECK(sections.resolved("SectionsTest_Profile.name") == std::string{R"("grace")"}); +} + +TEST_CASE("SectionSet: reset clears one section and leaves the others intact", "[sections]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::testing::StepExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + morph::forms::SectionSet sections{handler}; + + recorder().clear(); + sections.set<&SecPrefs::profileId>(42); // not ready: no theme yet + sections.set<&SecProfile::name>("ada"); // ready: fires + + CHECK(sections.draft().profileId == 42); + CHECK(sections.draft().name == "ada"); + + sections.reset(); + + CHECK(sections.draft().profileId == 0); + // Per-section isolation is the point: resetting one editor must not wipe + // what the user typed into another. + CHECK(sections.draft().name == "ada"); + + // The profile dispatch above is still outstanding; let it land before the + // set is destroyed. + drain(cbExec); +} + +TEST_CASE("SectionSet: a fired section's fields are resolvable; an unfired one is not", "[sections]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::testing::StepExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + morph::forms::SectionSet sections{handler}; + + recorder().clear(); + sections.set<&SecProfile::name>("ada"); + drain(cbExec); + + // The result's field: SecProfileResult::id == name.size() == 3. + CHECK(sections.resolved("SectionsTest_Profile.id") == std::string{"3"}); + // The submitted draft's field is captured too, JSON-encoded. + CHECK(sections.resolved("SectionsTest_Profile.name") == std::string{R"("ada")"}); + + // Both halves matter. Without these an implementation that returned a value + // for every path would still pass the checks above. + CHECK_FALSE(sections.resolved("SectionsTest_Prefs.summary").has_value()); + CHECK_FALSE(sections.resolved("SectionsTest_Profile.nosuchfield").has_value()); +} + +TEST_CASE("SectionSet: a failing dispatch reaches the onError callback", "[sections]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::testing::StepExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + std::atomic errors{0}; + morph::forms::SectionSet sections{ + handler, [&errors](const std::exception_ptr&) { errors.fetch_add(1); }}; + + recorder().clear(); + sections.set<&SecExplodes::label>("boom"); + drain(cbExec); + CHECK(errors.load() == 1); + + // A section that succeeds does not route to onError, so the count above is + // the failure and not merely "some callback ran". + sections.set<&SecProfile::name>("ada"); + drain(cbExec); + CHECK(sections.resolved("SectionsTest_Profile.id").has_value()); + CHECK(errors.load() == 1); +} + +TEST_CASE("SectionSet: an unhandled failure logs instead of escaping", "[sections]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::testing::StepExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + // Capture the log rather than only asserting nothing threw: the logged + // line is also the signal that the error continuation has finished, which + // is what lets the set be destroyed safely at the end of this case. + std::atomic logged{false}; + morph::log::ScopedLoggerOverride const logGuard{[&logged](morph::log::LogLevel, std::string_view msg) { + if (msg.contains("SectionsTest_Explodes")) { + logged.store(true); + } + }}; + + { + // No onError: the failure has nowhere to go but the log. It must not + // propagate out of the completion and take the executor thread down. + morph::forms::SectionSet sections{handler}; + + recorder().clear(); + REQUIRE_NOTHROW(sections.set<&SecExplodes::label>("boom")); + REQUIRE_NOTHROW(drain(cbExec)); + CHECK(logged.load()); + + // The set survives its own failed section: an unrelated one still works. + sections.set<&SecProfile::name>("ada"); + drain(cbExec); + CHECK(sections.resolved("SectionsTest_Profile.id").has_value()); + } +} + +TEST_CASE("SectionSet: destroying it with a dispatch in flight delivers nothing", "[sections]") { + morph::exec::ThreadPoolExecutor pool{2}; + morph::testing::InlineExecutor cbExec; + morph::bridge::Bridge bridge{std::make_unique(pool)}; + morph::bridge::BridgeHandler handler{bridge, &cbExec}; + + secSlowStarted().store(false); + secSlowRelease().store(false); + + std::atomic errors{0}; + { + morph::forms::SectionSet sections{ + handler, [&errors](const std::exception_ptr&) { errors.fetch_add(1); }}; + sections.set<&SecSlow::label>("held"); + // Wait until the model call is genuinely inside execute() before + // leaving the scope. Without this the dispatch would usually finish + // first and the destructor would race nothing at all. + REQUIRE(morph::testing::waitUntil([] { return secSlowStarted().load(); })); + } + + // The set is gone; only now does the model return (by throwing). Its error + // continuation resolves against an object that no longer exists, and must + // find the callback scope stopped and do nothing. + secSlowRelease().store(true); + CHECK(morph::testing::waitUntil([&errors] { return errors.load() != 0; }, std::chrono::milliseconds{500}) == + false); + CHECK(errors.load() == 0); +}