From 1b95b09ed4948b1c1eb306b5135c0eb0c06fd6b9 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 24 Aug 2026 11:44:55 +0300 Subject: [PATCH 1/3] ladder: make the zero-sentinel id constraint explicit and enforced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit polls::OptionId, polls::PollEventId and kanban::BoardEventId use `value == 0` as their "not entered" state, so an id of 0 is unrepresentable: construct one and it reports hasValue() == false and behaves as absent everywhere downstream. Nothing is broken today -- all three come from SQLite row ids, which start at 1 -- but the constraint was implicit, and the failure mode if a 0 ever did arrive (a seeded row, a migrated dataset, an externally supplied key, a sequence reset) is that a real record reads as "no record". #215 offers three options. This takes (2), documented-and-enforced, not (1), convert-to-optional-backed, for two reasons the issue does not account for: - kanban's types.hpp already documents the zero-sentinel shape as a deliberate choice ("it is always looked up already-assigned", citing design spec ยง7), and PollEventId{} is the natural spelling of "no cursor yet, replay from the beginning" for GetEventsSince. Converting would reverse a recorded decision. - The optional-backed shape changes the empty state's wire form from 0 to null for two shipped rungs. Enforcement is a checked factory, fromRowId(), which every conversion from a stored row id now goes through; it rejects 0 loudly instead of letting it collapse into the empty state one layer below the QML surface, where no conversion helper can restore the distinction. Six conversion sites in poll_model.cpp and board_model.cpp adopt it. One site deliberately does not: decodeVotes() in poll_qml_bridges.cpp builds an OptionId from *QML-supplied* input, where toLongLong() yields 0 for a missing or non-numeric field -- which is precisely the "not entered" state the action's validate() exists to reject, as a clean ValidationError rather than an exception. That distinction now has a comment. Also corrects polls/core/types.hpp's file comment, which claimed these two follow BookmarkId's pattern. They do not -- BookmarkId is optional-backed, and that mismatch is the documentation defect underneath this issue. Verified: polls + kanban ladder suites 198/198, including the new fromRowId cases and their control cases (a factory that rejected everything would pass the rejection test alone). clang-tidy-diff and -Wdocumentation clean. Closes #215 --- examples/kanban/include/kanban/core/types.hpp | 27 ++++++++ examples/kanban/src/models/board_model.cpp | 2 +- examples/kanban/tests/test_board_dto.cpp | 18 +++++ examples/polls/gui_lib/poll_qml_bridges.cpp | 6 ++ examples/polls/include/polls/core/types.hpp | 66 +++++++++++++++++-- examples/polls/src/models/poll_model.cpp | 10 +-- examples/polls/tests/test_polls_types.cpp | 22 +++++++ 7 files changed, 141 insertions(+), 10 deletions(-) diff --git a/examples/kanban/include/kanban/core/types.hpp b/examples/kanban/include/kanban/core/types.hpp index 9da760cd..05d1571f 100644 --- a/examples/kanban/include/kanban/core/types.hpp +++ b/examples/kanban/include/kanban/core/types.hpp @@ -7,6 +7,8 @@ #include #include +#include "errors.hpp" + /// @file /// Kanban's strong id types and the `Role` enum. Every id wraps an /// auto-incrementing SQLite row id -- `BookmarkId`'s shape @@ -97,11 +99,36 @@ enum class Role : std::uint8_t { Viewer, Member, Manager }; /// log. Zero-sentinel shape (not `fromOptional`'s optional shape) -- /// it is always looked up already-assigned, per `polls::PollEventId`'s /// identical precedent. +/// +/// The shape carries a constraint the type cannot enforce on its own: because +/// `value == 0` *is* the "not entered" state, **an event id of `0` is +/// unrepresentable** -- construct one and it reports `hasValue() == false`, +/// so a real event would read as "no event" (morph#215). The constraint holds +/// because these ids are SQLite row ids, which start at 1. `fromRowId()` is +/// the enforcement; use it for every conversion from a stored value. struct BoardEventId { std::int64_t value{0}; [[nodiscard]] constexpr bool hasValue() const { return value != 0; } [[nodiscard]] constexpr std::int64_t operator*() const { return value; } [[nodiscard]] constexpr bool operator==(const BoardEventId&) const = default; + + /// @brief Wraps a stored row id, rejecting the one value this type cannot + /// represent. + /// + /// Never fires in practice (row ids start at 1); it exists so a seeded + /// row, a migrated dataset, or a sequence reset fails loudly at the + /// boundary rather than collapsing into the empty state one layer below + /// the surface, where no conversion helper can restore the distinction. + /// @param rowId Stored row id; must be non-zero. + /// @return An engaged `BoardEventId` wrapping @p rowId. + /// @throws KanbanError if @p rowId is `0`. + [[nodiscard]] static BoardEventId fromRowId(std::int64_t rowId) { + if (rowId == 0) { + throw KanbanError{"BoardEventId::fromRowId: an event row id of 0 is unrepresentable -- 0 is this " + "type's \"not entered\" sentinel (morph#215)"}; + } + return BoardEventId{.value = rowId}; + } }; } // namespace kanban diff --git a/examples/kanban/src/models/board_model.cpp b/examples/kanban/src/models/board_model.cpp index 7f3dfcfb..57728150 100644 --- a/examples/kanban/src/models/board_model.cpp +++ b/examples/kanban/src/models/board_model.cpp @@ -1059,7 +1059,7 @@ GetEventsSinceResult BoardModel::execute(const GetEventsSince& action) { GetEventsSinceResult result; result.events.reserve(rows.size()); for (const auto& row : rows) { - result.events.push_back({.id = BoardEventId{.value = static_cast(row.id.Value())}, + result.events.push_back({.id = BoardEventId::fromRowId(static_cast(row.id.Value())), .kind = std::string{row.kind.Value()}, .summary = std::string{row.summary.Value()}}); } diff --git a/examples/kanban/tests/test_board_dto.cpp b/examples/kanban/tests/test_board_dto.cpp index d04631cb..e7b25d68 100644 --- a/examples/kanban/tests/test_board_dto.cpp +++ b/examples/kanban/tests/test_board_dto.cpp @@ -1,4 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 +#include "kanban/core/errors.hpp" +#include "kanban/core/types.hpp" #include "kanban/dto/board_dto.hpp" #include @@ -50,3 +52,19 @@ TEST_CASE("AddComment requires an engaged taskId and non-empty body", "[kanban][ CHECK(kanban::AddComment{.taskId = kanban::TaskId{1}, .body = "hi"}.validate()); } + +TEST_CASE("kanban::BoardEventId: fromRowId rejects the one value it cannot represent", "[kanban][types]") { + // 0 is BoardEventId's "not entered" sentinel, so an event id of 0 would + // arrive as *absent* and a real event would read as "no event" + // (morph#215). Row ids start at 1, so this never fires in practice -- it + // turns a silent collapse into a loud failure at the boundary. + CHECK_THROWS_AS(kanban::BoardEventId::fromRowId(0), kanban::KanbanError); +} + +TEST_CASE("kanban::BoardEventId: fromRowId wraps an ordinary row id unchanged", "[kanban][types]") { + // Control case: without it the check above would pass against a factory + // that rejected every input. + auto const event = kanban::BoardEventId::fromRowId(42); + CHECK(event.hasValue()); + CHECK(*event == 42); +} diff --git a/examples/polls/gui_lib/poll_qml_bridges.cpp b/examples/polls/gui_lib/poll_qml_bridges.cpp index 3ee13858..1cdeea40 100644 --- a/examples/polls/gui_lib/poll_qml_bridges.cpp +++ b/examples/polls/gui_lib/poll_qml_bridges.cpp @@ -77,6 +77,12 @@ using ::morph::ladder::gui::idNumber; out.reserve(static_cast(votes.size())); for (const QVariant& entry : votes) { const QVariantMap row = entry.toMap(); + // Deliberately not `OptionId::fromRowId`: this is QML-supplied input, + // not a stored row id. A missing or non-numeric `optionId` yields 0 + // from toLongLong(), which is exactly the "not entered" state the + // action's validate() is there to reject -- a clean ValidationError, + // rather than the exception fromRowId raises for a corrupt *stored* + // id (morph#215). out.push_back(OneVote{.optionId = OptionId{.value = row.value(QStringLiteral("optionId")).toLongLong()}, .choice = parseChoice(row.value(QStringLiteral("choice")).toString())}); } diff --git a/examples/polls/include/polls/core/types.hpp b/examples/polls/include/polls/core/types.hpp index 5b17fe39..54a0257e 100644 --- a/examples/polls/include/polls/core/types.hpp +++ b/examples/polls/include/polls/core/types.hpp @@ -6,12 +6,29 @@ #include #include +#include "errors.hpp" + /// @file /// Polls' strong id types and constants. `OptionId` and `PollEventId` wrap -/// auto-incrementing integers (SQLite row ids), following `BookmarkId`'s -/// pattern. `PollId` itself is not a strong type (see Global Constraints), -/// but `kTokenBytes` is shared by implementations and tests to ensure -/// consistency on generated token lengths. +/// auto-incrementing integers (SQLite row ids). `PollId` itself is not a +/// strong type (see Global Constraints), but `kTokenBytes` is shared by +/// implementations and tests to ensure consistency on generated token lengths. +/// +/// These two use a **zero sentinel**, not `BookmarkId`'s +/// `std::optional`-backed shape: `value == 0` *is* the "not entered" state. +/// That is deliberate -- neither is ever handed a nullable payload to adopt, +/// and `PollEventId{}` is the natural spelling of "no cursor yet, start from +/// the beginning" for `GetEventsSince` -- but it carries a constraint the +/// type cannot enforce on its own: **an id of `0` is unrepresentable**. +/// Construct one and it reports `hasValue() == false` and behaves as absent +/// everywhere downstream, so a real record would read as "no record" +/// (morph#215). +/// +/// The constraint holds because both ids come from SQLite row ids, which +/// start at 1. `fromRowId()` is the enforcement: every conversion from a +/// stored row id goes through it, and it rejects `0` loudly rather than +/// letting it collapse into the empty state. Use it instead of constructing +/// these ids directly from database values. namespace polls { @@ -47,6 +64,26 @@ struct OptionId { /// @brief Equality on the payload. [[nodiscard]] constexpr bool operator==(const OptionId&) const = default; + + /// @brief Wraps a stored row id, rejecting the one value this type cannot + /// represent. + /// + /// `0` is this type's "not entered" sentinel, so an id of `0` would arrive + /// as *absent* and a real option would read as "no option selected" + /// (morph#215). SQLite row ids start at 1, so this never fires in + /// practice -- it exists so that a seeded row, a migrated dataset, an + /// externally supplied key, or a sequence reset fails loudly at the + /// boundary instead of collapsing silently one layer below the surface. + /// @param rowId Stored row id; must be non-zero. + /// @return An engaged `OptionId` wrapping @p rowId. + /// @throws PollsError if @p rowId is `0`. + [[nodiscard]] static OptionId fromRowId(std::int64_t rowId) { + if (rowId == 0) { + throw PollsError{"OptionId::fromRowId: an option row id of 0 is unrepresentable -- 0 is this " + "type's \"not entered\" sentinel (morph#215)"}; + } + return OptionId{.value = rowId}; + } }; /// @brief Strong identifier for one row in the `poll_events` append-only log. @@ -72,6 +109,27 @@ struct PollEventId { /// @brief Equality on the payload. [[nodiscard]] constexpr bool operator==(const PollEventId&) const = default; + + /// @brief Wraps a stored row id, rejecting the one value this type cannot + /// represent. + /// + /// `0` is this type's "not entered" sentinel -- the spelling + /// `GetEventsSince` uses for "no cursor yet, replay from the beginning" -- + /// so an event row id of `0` would arrive as *absent* and the reader would + /// silently rewind to the start of the log (morph#215). SQLite row ids + /// start at 1, so this never fires in practice; it exists so that a + /// seeded row, a migrated dataset, or a sequence reset fails loudly at the + /// boundary rather than collapsing silently. + /// @param rowId Stored row id; must be non-zero. + /// @return An engaged `PollEventId` wrapping @p rowId. + /// @throws PollsError if @p rowId is `0`. + [[nodiscard]] static PollEventId fromRowId(std::int64_t rowId) { + if (rowId == 0) { + throw PollsError{"PollEventId::fromRowId: an event row id of 0 is unrepresentable -- 0 is this " + "type's \"not entered\" sentinel (morph#215)"}; + } + return PollEventId{.value = rowId}; + } }; /// @brief One participant's answer for one option. diff --git a/examples/polls/src/models/poll_model.cpp b/examples/polls/src/models/poll_model.cpp index f08e9cb5..f96fb891 100644 --- a/examples/polls/src/models/poll_model.cpp +++ b/examples/polls/src/models/poll_model.cpp @@ -222,7 +222,7 @@ void requireOptionBelongsToPoll(::Lightweight::DataMapper& mapper, const db::Pol result.title = textOf(poll.title.Value()); result.finalized = poll.finalized.Value() ? Finalized::Yes : Finalized::No; if (result.finalized == Finalized::Yes) { - result.finalizedOptionId = OptionId{.value = poll.finalizedOptionId.Value()}; + result.finalizedOptionId = OptionId::fromRowId(poll.finalizedOptionId.Value()); } const std::uint64_t pollDbId = poll.id.Value(); @@ -235,7 +235,7 @@ void requireOptionBelongsToPoll(::Lightweight::DataMapper& mapper, const db::Pol .All(); for (const auto& opt : options) { PollOptionView view; - view.id = OptionId{.value = static_cast(opt.id.Value())}; + view.id = OptionId::fromRowId(static_cast(opt.id.Value())); view.label = textOf(opt.label.Value()); // Explicit zero, not default-constructed: a default `Count{}` is // Quantity's *empty* state (no payload), and Quantity arithmetic @@ -285,7 +285,7 @@ void requireOptionBelongsToPoll(::Lightweight::DataMapper& mapper, const db::Pol ::Lightweight::SqlResultOrdering::DESCENDING) .First(); result.lastEventId = - lastEvent ? PollEventId{.value = static_cast(lastEvent->id.Value())} : PollEventId{}; + lastEvent ? PollEventId::fromRowId(static_cast(lastEvent->id.Value())) : PollEventId{}; return result; } @@ -463,7 +463,7 @@ GetPollStateResult PollModel::applyVotes(const std::string& participantName, con std::vector previousVotes; previousVotes.reserve(priorVotes.size()); for (const auto& v : priorVotes) { - previousVotes.push_back({.optionId = OptionId{.value = static_cast(v.option.Value())}, + previousVotes.push_back({.optionId = OptionId::fromRowId(static_cast(v.option.Value())), .choice = static_cast(v.choice.Value())}); } const std::string previousVotesJson = encodeVotesJson(previousVotes); @@ -730,7 +730,7 @@ GetEventsSinceResult PollModel::execute(const GetEventsSince& action) { GetEventsSinceResult result; result.events.reserve(rows.size()); for (const auto& row : rows) { - result.events.push_back({.id = PollEventId{.value = static_cast(row.id.Value())}, + result.events.push_back({.id = PollEventId::fromRowId(static_cast(row.id.Value())), .kind = textOf(row.kind.Value()), .summary = textOf(row.summary.Value())}); } diff --git a/examples/polls/tests/test_polls_types.cpp b/examples/polls/tests/test_polls_types.cpp index be3f0333..37561c7c 100644 --- a/examples/polls/tests/test_polls_types.cpp +++ b/examples/polls/tests/test_polls_types.cpp @@ -26,3 +26,25 @@ TEST_CASE("PollsError hierarchy: each derived type carries its own message", "[p CHECK(std::string_view{polls::Forbidden{"not the admin"}.what()} == "not the admin"); CHECK(std::string_view{polls::Conflict{"already finalized"}.what()} == "already finalized"); } + +TEST_CASE("polls id types: fromRowId rejects the one value they cannot represent", "[polls][types]") { + // 0 is these types' "not entered" sentinel, so an id of 0 would arrive as + // *absent* and a real record would read as "no record" (morph#215). Row + // ids start at 1, so this never fires in practice -- the point is that a + // seeded, migrated, or externally supplied 0 fails loudly at the boundary + // instead of collapsing silently one layer below the QML surface. + CHECK_THROWS_AS(polls::OptionId::fromRowId(0), polls::PollsError); + CHECK_THROWS_AS(polls::PollEventId::fromRowId(0), polls::PollsError); +} + +TEST_CASE("polls id types: fromRowId wraps an ordinary row id unchanged", "[polls][types]") { + // The control case: without it the check above would pass against a + // factory that rejected everything. + auto const option = polls::OptionId::fromRowId(7); + CHECK(option.hasValue()); + CHECK(*option == 7); + + auto const event = polls::PollEventId::fromRowId(9223372036854775807); + CHECK(event.hasValue()); + CHECK(*event == 9223372036854775807); +} From 434edb3fc043329940d49e62154e23578eabbfc2 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Mon, 24 Aug 2026 14:09:48 +0300 Subject: [PATCH 2/3] ci: re-trigger checks The original run was cancelled by the supersede-obsolete-runs concurrency rule (#257) shortly after it merged, and re-running the cancelled workflows produced attempts that were themselves cancelled within minutes. An empty commit gives the PR a fresh head so its checks run from a clean slate. No content change. From 4ea9be5dacf19466e3a6df2ef9a2714377c5b2a1 Mon Sep 17 00:00:00 2001 From: Yaraslau Tamashevich Date: Tue, 25 Aug 2026 03:50:30 +0300 Subject: [PATCH 3/3] ladder: clang-format the fromRowId call sites fromRowId(...) is longer than the BoardEventId{.value = ...} brace-init it replaced, so the designated-initialiser alignment under it shifted, and the KanbanError message no longer fits the way it was wrapped. Found by running the changed-lines clang-format check from morph#210 against this branch before proposing that gate -- it flagged 5 files here and nothing else across my other twelve branches. Fixed by applying the changed-lines patch rather than running clang-format over whole files: 472 of 676 files in the tree do not match .clang-format, so a whole-file pass would have buried this change in unrelated churn. polls + kanban ladder: 198/198. --- examples/kanban/include/kanban/core/types.hpp | 5 +++-- examples/kanban/src/models/board_model.cpp | 4 ++-- examples/kanban/tests/test_board_dto.cpp | 5 ++--- examples/polls/include/polls/core/types.hpp | 10 ++++++---- examples/polls/src/models/poll_model.cpp | 6 +++--- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/examples/kanban/include/kanban/core/types.hpp b/examples/kanban/include/kanban/core/types.hpp index 05d1571f..2ae2716f 100644 --- a/examples/kanban/include/kanban/core/types.hpp +++ b/examples/kanban/include/kanban/core/types.hpp @@ -124,8 +124,9 @@ struct BoardEventId { /// @throws KanbanError if @p rowId is `0`. [[nodiscard]] static BoardEventId fromRowId(std::int64_t rowId) { if (rowId == 0) { - throw KanbanError{"BoardEventId::fromRowId: an event row id of 0 is unrepresentable -- 0 is this " - "type's \"not entered\" sentinel (morph#215)"}; + throw KanbanError{ + "BoardEventId::fromRowId: an event row id of 0 is unrepresentable -- 0 is this " + "type's \"not entered\" sentinel (morph#215)"}; } return BoardEventId{.value = rowId}; } diff --git a/examples/kanban/src/models/board_model.cpp b/examples/kanban/src/models/board_model.cpp index 57728150..3ac6d712 100644 --- a/examples/kanban/src/models/board_model.cpp +++ b/examples/kanban/src/models/board_model.cpp @@ -1060,8 +1060,8 @@ GetEventsSinceResult BoardModel::execute(const GetEventsSince& action) { result.events.reserve(rows.size()); for (const auto& row : rows) { result.events.push_back({.id = BoardEventId::fromRowId(static_cast(row.id.Value())), - .kind = std::string{row.kind.Value()}, - .summary = std::string{row.summary.Value()}}); + .kind = std::string{row.kind.Value()}, + .summary = std::string{row.summary.Value()}}); } return result; } diff --git a/examples/kanban/tests/test_board_dto.cpp b/examples/kanban/tests/test_board_dto.cpp index e7b25d68..dbb9eb1e 100644 --- a/examples/kanban/tests/test_board_dto.cpp +++ b/examples/kanban/tests/test_board_dto.cpp @@ -1,10 +1,10 @@ // SPDX-License-Identifier: Apache-2.0 +#include + #include "kanban/core/errors.hpp" #include "kanban/core/types.hpp" #include "kanban/dto/board_dto.hpp" -#include - TEST_CASE("OpenBoard requires an engaged projectId", "[kanban][dto]") { CHECK_FALSE(kanban::OpenBoard{.projectId = {}}.validate()); CHECK(kanban::OpenBoard{.projectId = kanban::ProjectId{1}}.validate()); @@ -52,7 +52,6 @@ TEST_CASE("AddComment requires an engaged taskId and non-empty body", "[kanban][ CHECK(kanban::AddComment{.taskId = kanban::TaskId{1}, .body = "hi"}.validate()); } - TEST_CASE("kanban::BoardEventId: fromRowId rejects the one value it cannot represent", "[kanban][types]") { // 0 is BoardEventId's "not entered" sentinel, so an event id of 0 would // arrive as *absent* and a real event would read as "no event" diff --git a/examples/polls/include/polls/core/types.hpp b/examples/polls/include/polls/core/types.hpp index 54a0257e..d4b87e08 100644 --- a/examples/polls/include/polls/core/types.hpp +++ b/examples/polls/include/polls/core/types.hpp @@ -79,8 +79,9 @@ struct OptionId { /// @throws PollsError if @p rowId is `0`. [[nodiscard]] static OptionId fromRowId(std::int64_t rowId) { if (rowId == 0) { - throw PollsError{"OptionId::fromRowId: an option row id of 0 is unrepresentable -- 0 is this " - "type's \"not entered\" sentinel (morph#215)"}; + throw PollsError{ + "OptionId::fromRowId: an option row id of 0 is unrepresentable -- 0 is this " + "type's \"not entered\" sentinel (morph#215)"}; } return OptionId{.value = rowId}; } @@ -125,8 +126,9 @@ struct PollEventId { /// @throws PollsError if @p rowId is `0`. [[nodiscard]] static PollEventId fromRowId(std::int64_t rowId) { if (rowId == 0) { - throw PollsError{"PollEventId::fromRowId: an event row id of 0 is unrepresentable -- 0 is this " - "type's \"not entered\" sentinel (morph#215)"}; + throw PollsError{ + "PollEventId::fromRowId: an event row id of 0 is unrepresentable -- 0 is this " + "type's \"not entered\" sentinel (morph#215)"}; } return PollEventId{.value = rowId}; } diff --git a/examples/polls/src/models/poll_model.cpp b/examples/polls/src/models/poll_model.cpp index f96fb891..97e8e310 100644 --- a/examples/polls/src/models/poll_model.cpp +++ b/examples/polls/src/models/poll_model.cpp @@ -464,7 +464,7 @@ GetPollStateResult PollModel::applyVotes(const std::string& participantName, con previousVotes.reserve(priorVotes.size()); for (const auto& v : priorVotes) { previousVotes.push_back({.optionId = OptionId::fromRowId(static_cast(v.option.Value())), - .choice = static_cast(v.choice.Value())}); + .choice = static_cast(v.choice.Value())}); } const std::string previousVotesJson = encodeVotesJson(previousVotes); @@ -731,8 +731,8 @@ GetEventsSinceResult PollModel::execute(const GetEventsSince& action) { result.events.reserve(rows.size()); for (const auto& row : rows) { result.events.push_back({.id = PollEventId::fromRowId(static_cast(row.id.Value())), - .kind = textOf(row.kind.Value()), - .summary = textOf(row.summary.Value())}); + .kind = textOf(row.kind.Value()), + .summary = textOf(row.summary.Value())}); } return result; }