ladder: rung 2 -- bookmarks - #90
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Split out of application-ladder (originally bundled with pastebin/polls and the shared foundation in #41) into its own PR against the rung-0 foundation (#88). Includes the pool-migration and cursor-race fixes folded in during review of the combined branch: - BookmarkModel/SharedFeedModel/TagModel acquire connections from Lightweight::GlobalDataMapperPool() per execute() call rather than holding one for its own lifetime (WithMapper removed). - GetChangesSince's millisecond cursor boundary race fixed (originally landed on master as its own commit; carried forward here since bookmarks is where the fix lives). Verified standalone against the ladder-foundation base: configures and builds with -DMORPH_LADDER_RUNGS=bookmarks and no other rung present. Full suite passes: 826 assertions in 121 test cases (SQLite default). Spec-citation and test-type-name lints clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Yaraslaut
force-pushed
the
ladder-bookmarks
branch
from
August 16, 2026 11:26
196a0b4 to
f6cb281
Compare
Yaraslaut
marked this pull request as ready for review
August 16, 2026 11:36
…ypes
Per docs/superpowers/specs/2026-08-11-strong-storage-types-design.md item 3
(string retyping; item 1 mapper pooling, item 2 pastebin id, and item 4
timestamp retyping are separate, out of scope here).
Every plain Light::Field<std::string, ...> in
examples/bookmarks/include/bookmarks/db/*.hpp moves to a Lightweight strong
string type:
- BookmarkRecord::ownerPrincipal, ImportedOpRecord::ownerPrincipal,
TagRecord::ownerPrincipal -> SqlAnsiString<64>, matching
bookmarks_authorizer.hpp's kMaxPrincipalBytes.
- BookmarkRecord::url, faviconPath -> SqlAnsiString<2048> (kMaxUrlBytes).
- BookmarkRecord::title -> SqlAnsiString<512> (kMaxTitleBytes).
- BookmarkRecord::description, notes -> SqlMaxDynamicAnsiString (unbounded,
matching the DTO's own unbounded std::string fields).
- ImportedOpRecord::opId -> SqlAnsiString<128> (small idempotency token).
- BookmarkOutboxRecord::modelType/entityKey/actionType/principal ->
SqlAnsiString<64> each; idempotencyKey -> SqlAnsiString<128>
(program-controlled identifiers, no existing named constant).
- BookmarkOutboxRecord::payload/result -> SqlMaxDynamicAnsiString
(serialized JSON, unbounded).
- TagRecord::name -> SqlAnsiString<128> (kMaxTagNameBytes, already existed
in tag_dto.hpp).
Every bounded field ties back to its DTO-level constant via a
static_assert(decltype(Entity::field)::ValueType{}.capacity() == kMaxFooBytes,
...) in the owning model .cpp (bookmark_model.cpp, tag_model.cpp), following
the precedent in examples/pastebin/src/models/paste_model.cpp's
kMaxSyntaxBytes assertion. Fields with no existing named constant (opId,
outbox columns) use the literal N directly with no new constant invented,
per the plan's instruction.
Updates every model-layer read that converts an entity field back to the
plain std::string DTO shape (SqlAnsiString/SqlMaxDynamicAnsiString's
std::string conversion operator is explicit, so each such read needs an
explicit std::string{...} or .str()/.ToStringView() call):
bookmark_model.cpp's toView()/readTagNames()/ListBookmarks/GetChangesSince/
ExportBookmarks, tag_model.cpp's ListTags, shared_feed_model.cpp's
ListSharedFeed, and app.cpp's relayOutboxOnce() (BookmarkOutboxRecord ->
journal::LogEntry). Writes (DTO std::string -> entity strong string) need
no changes: every strong string type's constructor from std::string/
std::string_view is non-explicit, so plain assignment already compiles.
Lightweight's Where(...) query-builder value argument is not constrained to
the column's declared type either (it binds through SqlVariant, which
accepts std::string natively), so no .Where(...) call site needed a change.
DTOs, glaze meta specializations, QML files, presenters/bridges, and every
test file are unaffected: test_bookmarks_schema.cpp's direct
db::*Record field assignments (string literals) and
test_bookmark_model.cpp/test_tag_model.cpp/test_app.cpp's read-only
BookmarkOutboxRecord .Value() == "literal" comparisons already compile
unchanged against the new types.
Verified via a from-scratch build of ladder_bookmarks_tests (MSVC/Ninja,
build/bookmarks) and the full suite run directly (121 test cases, 826
assertions, all passing against the SQLite test-fixture default).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds application ladder rung 2 (“bookmarks”): a linkding-inspired, multi-user bookmarks example that exercises Morph’s signed-token session pipeline, server-hosted models, a Qt/QML client (plus WASM shell), schema-driven forms, import/export, and a durable action log via a transactional outbox.
Changes:
- Introduces the bookmarks domain: DTOs, strong types/errors, Lightweight entities + migrations, and model implementations (bookmark/tag/shared-feed/auth).
- Adds a standalone server process (QtWebSocketServer) and shared desktop/WASM client shells with QML + presenter/bridge glue.
- Adds comprehensive unit/integration tests for DTO validation, schema, authorization, models, presenters, QML smoke, and import parsing.
Reviewed changes
Copilot reviewed 65 out of 65 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| examples/bookmarks/CMakeLists.txt | Rung build wiring and per-target sources/defs (incl. WASM URL define). |
| examples/bookmarks/README.md | Rung documentation: scope, running, design decisions, known gaps. |
| examples/bookmarks/gui/main.cpp | Desktop client shell: selects local/remote mode and boots QML with controllers. |
| examples/bookmarks/gui/qml/BookmarkListView.qml | Main post-login UI: list/detail panes + form submission handling. |
| examples/bookmarks/gui/qml/LoginView.qml | Login screen using schema-driven DynamicForm. |
| examples/bookmarks/gui/qml/Main.qml | Application window + navigation between login and main screen. |
| examples/bookmarks/gui_lib/bookmark_forms_controller.cpp | ActionType→model routing implementation for schema-driven submissions. |
| examples/bookmarks/gui_lib/bookmark_forms_controller.hpp | Multi-model FormsController glue over BridgeHandlers. |
| examples/bookmarks/gui_lib/bookmark_presenter.cpp | BookmarkPresenter implementation over BridgeHandler. |
| examples/bookmarks/gui_lib/bookmark_presenter.hpp | BookmarkPresenter API + signals for QML bridges. |
| examples/bookmarks/gui_lib/bookmark_qml_bridges.cpp | QML adapters: FormsBridge + presenter result shaping to QVariant. |
| examples/bookmarks/gui_lib/bookmark_qml_bridges.hpp | QML adapter class declarations and Q_INVOKABLE surface. |
| examples/bookmarks/gui_lib/bookmark_schemas.hpp | Assembles the schema JSON document for all rendered forms. |
| examples/bookmarks/gui_lib/shared_feed_presenter.cpp | SharedFeedPresenter implementation. |
| examples/bookmarks/gui_lib/shared_feed_presenter.hpp | SharedFeedPresenter API + signals. |
| examples/bookmarks/gui_lib/tag_presenter.cpp | TagPresenter implementation. |
| examples/bookmarks/gui_lib/tag_presenter.hpp | TagPresenter API + signals. |
| examples/bookmarks/gui_wasm/main_wasm.cpp | WASM client shell (remote-only) mirroring the desktop client boot. |
| examples/bookmarks/include/bookmarks/app/app.hpp | Server-side App wrapper: RemoteServer + timers + internal client jobs. |
| examples/bookmarks/include/bookmarks/app/metadata_fetcher.hpp | Metadata fetch seam + NullMetadataFetcher default. |
| examples/bookmarks/include/bookmarks/auth/bookmarks_authorizer.hpp | Rung authorizer: signed-token auth + register/instance policies + TokenIssuer slot. |
| examples/bookmarks/include/bookmarks/core/errors.hpp | Bookmarks domain exception hierarchy. |
| examples/bookmarks/include/bookmarks/core/types.hpp | Strong scalar types (ids, cursors, auth token, etc.). |
| examples/bookmarks/include/bookmarks/db/bookmark_entity.hpp | Lightweight BookmarkRecord entity definition. |
| examples/bookmarks/include/bookmarks/db/bookmark_tag_entity.hpp | Lightweight bookmark↔tag junction entity definition. |
| examples/bookmarks/include/bookmarks/db/database.hpp | DB bootstrap API (setup + migrations). |
| examples/bookmarks/include/bookmarks/db/imported_op_entity.hpp | Imported-ops table entity (import idempotency). |
| examples/bookmarks/include/bookmarks/db/outbox_entity.hpp | Transactional outbox entity for durable action logging. |
| examples/bookmarks/include/bookmarks/db/tag_entity.hpp | TagRecord entity definition. |
| examples/bookmarks/include/bookmarks/dto/auth_dto.hpp | Login/AuthToken DTOs + reflection. |
| examples/bookmarks/include/bookmarks/dto/bookmark_dto.hpp | Bookmark DTOs: CRUD, list, changes-since, import/export, metadata. |
| examples/bookmarks/include/bookmarks/dto/bulk_dto.hpp | BulkEdit DTOs and enum reflection. |
| examples/bookmarks/include/bookmarks/dto/import_export_dto.hpp | ImportBookmarks/ExportBookmarks DTOs and limits. |
| examples/bookmarks/include/bookmarks/dto/shared_feed_dto.hpp | Shared feed DTOs + pagination cursor. |
| examples/bookmarks/include/bookmarks/dto/tag_dto.hpp | Tag DTOs: rename/merge/list + TagSummary. |
| examples/bookmarks/include/bookmarks/import/netscape_bookmarks.hpp | Netscape bookmark chunk parser + HTML escaping API. |
| examples/bookmarks/include/bookmarks/models/auth_model.hpp | AuthModel interface + BRIDGE_REGISTER wiring. |
| examples/bookmarks/include/bookmarks/models/bookmark_model.hpp | BookmarkModel interface + BRIDGE_REGISTER wiring. |
| examples/bookmarks/include/bookmarks/models/shared_feed_model.hpp | SharedFeedModel interface + BRIDGE_REGISTER wiring. |
| examples/bookmarks/include/bookmarks/models/tag_model.hpp | TagModel interface + BRIDGE_REGISTER wiring. |
| examples/bookmarks/include/bookmarks/units.hpp | Bookmarks unit system (Count quantity). |
| examples/bookmarks/src/app/app.cpp | Server App implementation: background metadata + outbox relay + limit policy. |
| examples/bookmarks/src/db/schema.cpp | Lightweight migrations for bookmarks tables + outbox. |
| examples/bookmarks/src/dto/auth_dto.cpp | Login::validate implementation. |
| examples/bookmarks/src/import/netscape_bookmarks.cpp | Parser/escaper implementation for Netscape bookmarks format. |
| examples/bookmarks/src/models/auth_model.cpp | AuthModel execute(Login) implementation with reserved principal guard. |
| examples/bookmarks/src/models/bookmark_model.cpp | Bookmark model implementation (CRUD/list/bulk/import/export/changes). |
| examples/bookmarks/src/models/shared_feed_model.cpp | Shared feed model implementation (cross-user listing + paging). |
| examples/bookmarks/src/models/tag_model.cpp | Tag model implementation (rename/merge/list + outbox write). |
| examples/bookmarks/src/server/main.cpp | Standalone server binary main: env config + graceful shutdown + drains. |
| examples/bookmarks/tests/test_app.cpp | App-level tests (server-side orchestration/background jobs/outbox). |
| examples/bookmarks/tests/test_bookmark_dto.cpp | Bookmark DTO validation + schema-related guards. |
| examples/bookmarks/tests/test_bookmark_model.cpp | BookmarkModel behavior tests (ownership, tags, etc.). |
| examples/bookmarks/tests/test_bookmark_presenter.cpp | BookmarkPresenter wiring tests across backend modes. |
| examples/bookmarks/tests/test_bookmark_qml_bridges.cpp | QML bridge/controller glue tests (incl. login decode path). |
| examples/bookmarks/tests/test_bookmarks_authorizer.cpp | Authorizer behavior tests (authz/authn/register/instance). |
| examples/bookmarks/tests/test_bookmarks_schema.cpp | Schema/entity smoke tests and constraint checks. |
| examples/bookmarks/tests/test_bookmarks_types.cpp | Strong type serialization/ordering tests. |
| examples/bookmarks/tests/test_gui_qml_smoke.cpp | Offscreen QML engine-load smoke test for rung QML module. |
| examples/bookmarks/tests/test_netscape_bookmarks.cpp | Netscape import parser/escaping tests. |
| examples/bookmarks/tests/test_shared_feed_model.cpp | SharedFeedModel behavior tests. |
| examples/bookmarks/tests/test_shared_feed_presenter.cpp | SharedFeedPresenter wiring tests across backend modes. |
| examples/bookmarks/tests/test_tag_bulk_dto.cpp | Tag/Bulk/Import DTO validation tests. |
| examples/bookmarks/tests/test_tag_model.cpp | TagModel behavior tests (rename/merge/conflicts/outbox). |
| examples/bookmarks/tests/test_tag_presenter.cpp | TagPresenter wiring tests across backend modes. |
Suppressed comments (2)
examples/bookmarks/src/db/schema.cpp:33
favicon_pathis declared as TEXT in the migration, but BookmarkRecord::faviconPath is a bounded SqlAnsiString<2048>. Aligning the DDL type (e.g., Varchar(2048)) avoids divergence between the database schema and the entity’s storage contract on non-SQLite backends.
examples/bookmarks/src/db/schema.cpp:44tags.nameis created as TEXT in the migration, but TagRecord::name is SqlAnsiString<128> (and RenameTag validates against kMaxTagNameBytes=128). Using Varchar(128) in the schema keeps the database constraint aligned with the DTO/entity limits.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+23
to
+27
| .RequiredColumn("owner_principal", Varchar(64)) | ||
| .RequiredColumn("url", Text()) | ||
| .RequiredColumn("title", Text()) | ||
| .RequiredColumn("description", Text()) | ||
| .RequiredColumn("notes", Text()) |
Comment on lines
+71
to
+76
| .RequiredColumn("model_type", Varchar(64)) | ||
| .RequiredColumn("entity_key", Varchar(64)) | ||
| .RequiredColumn("action_type", Varchar(64)) | ||
| .RequiredColumn("payload", Text()) | ||
| .RequiredColumn("result", Text()) | ||
| .RequiredColumn("principal", Varchar(64)) |
Comment on lines
+62
to
+66
| .All(); | ||
| std::vector<std::string> tags; | ||
| for (const auto& jrow : junctionRows) { | ||
| auto tagRows = | ||
| mapper->Query<db::TagRecord>().Where(::Lightweight::FieldNameOf<&db::TagRecord::id>, "=", jrow.tag.Value()).All(); |
Comment on lines
+199
to
+203
| const auto count = mapper | ||
| ->Query<db::BookmarkTagRecord>() | ||
| .Where(::Lightweight::FieldNameOf<&db::BookmarkTagRecord::tag>, "=", rec.id.Value()) | ||
| .All() | ||
| .size(); |
Comment on lines
+13
to
+16
| /// @brief Longest tag name, in bytes, this rung accepts — a `validate()` | ||
| /// sanity bound only, not a storage-column width. See this task's | ||
| /// own header comment for why `TagRecord::name` carries no | ||
| /// `SqlAnsiString` capacity to check against. |
Comment on lines
+166
to
+187
| if (actionType == QLatin1String("Login")) { | ||
| const auto result = decodeLoginResult(resultJson); | ||
| if (!result) { | ||
| emit replyReceived(actionType, false, | ||
| QStringLiteral("login succeeded but its reply could not be decoded")); | ||
| return; | ||
| } | ||
| onLoginSucceeded(*result); | ||
| } | ||
| // NOTE: for `Login`, `resultJson` is the full `LoginResult` | ||
| // document — bearer token included — and this signal is broadcast | ||
| // to *every* bound QML handler. Both handlers this rung ships | ||
| // keep it off screen: BookmarkListView.qml returns early for | ||
| // `Login`, and LoginView.qml renders `payload` only when `ok` is | ||
| // false — and a failed login carries no token. A future handler | ||
| // must not render `payload` | ||
| // unconditionally: doing so would put a live credential on screen | ||
| // (and into any screenshot or screen recording of it). Narrowing | ||
| // the signal itself is the real fix and is deliberately not made | ||
| // here — it is a public QML surface change, not a review tweak. | ||
| emit replyReceived(actionType, true, QString::fromStdString(resultJson)); | ||
| }, |
- schema.cpp: match DDL column types to their entities' declared Lightweight strong-string capacities -- Varchar(N) for bounded url/title/favicon_path/ tag name, NVarchar(0) for unbounded description/notes/outbox payload+result (was Text() uniformly, which encoded no bound and mirrors none of pastebin's precedent) (per @Copilot) - shared_feed_model.cpp: batch ListSharedFeed's per-bookmark tag-name lookup into 2 queries for the whole page (WhereIn + in-memory grouping) instead of a junction query plus one tag query per junction row (N+1+M) (per @Copilot) - tag_model.cpp: batch ListTags' per-tag bookmark count into 1 query for all of the owner's tags, counted in-memory, instead of one full-row junction query per tag just to read its .size() (per @Copilot) - tag_dto.hpp: fix kMaxTagNameBytes's comment, which claimed TagRecord::name carries no SqlAnsiString capacity to check against -- it is SqlAnsiString<128>, and tag_model.cpp's own static_assert already pins it to this constant (per @Copilot) - bookmark_qml_bridges.cpp: redact the bearer token from a successful Login's replyReceived payload -- the token has already done its one job (installed onto the session) by the time the signal fires, and broadcasting it further invites a future QML handler to display or log a live credential; principal is preserved, and the signal's shape is unchanged (per @Copilot) - test_bookmark_qml_bridges.cpp: update the Login-decode regression test for the redaction above (asserts payload's token is absent, then reuses the sibling test's follow-up-refresh technique to confirm the *real* token was still installed onto the session) Verified: ladder_bookmarks_tests full suite passes against SQLite (260 cases; the 2 unrelated pre-existing "failures" ctest reports on RecordMetadata/ QtWebSocketBackend are a console em-dash encoding artifact in ctest's own -R re-invocation, not real failures -- both pass when run without a name filter).
Yaraslaut
pushed a commit
that referenced
this pull request
Aug 16, 2026
cpp-review finding: - schema.cpp: previous_votes_json/summary use NVarchar(0), not Text() -- both back a SqlMaxDynamicAnsiString entity field (genuinely unbounded), and Text() encodes no such contract; matches pastebin's own NVarchar(0) precedent for unbounded storage and the fix already applied to bookmarks' equivalent Text()-vs-entity-capacity mismatch (PR #90) Copilot findings: - VoteView.qml: each vote RadioButton's onToggled fired unconditionally, including on the checked->unchecked transition Qt's shared ButtonGroup triggers on the sibling that just lost the selection -- so clicking one choice could have its own setPick() call overwritten right back by the previously-checked button's own unconditional handler, depending on which of the two `toggled` signals QML fires second. Guarded all three (Yes/If need be/No) on `if (checked)`. - types.hpp: OptionId/PollEventId::operator*()'s doc comment claimed "UB when empty, exactly like std::optional::operator*" -- both wrap a plain std::int64_t with 0 as their own sentinel, not a std::optional, so reading either when "empty" just returns 0, never UB. Rewrote both comments to state the real contract (Copilot's suppressed-comments list named PollEventId only; OptionId carries the identical copy-pasted comment one type up in the same file, so fixed both). Verified: full ladder_polls_tests suite (68 cases) passes against SQLite; the 1 unrelated ctest-reported failure (QtWebSocketBackend concurrent dispatch, untouched by this diff) is the same console em-dash encoding artifact in ctest's -R/-I re-invocation seen on PR #90, not a real failure.
Yaraslaut
added a commit
that referenced
this pull request
Aug 16, 2026
* ladder: rung 3 -- polls Split out of application-ladder (originally bundled with pastebin/ bookmarks and the shared foundation in #41) into its own PR against the rung-0 foundation (#88). Includes the pool-migration fix folded in during review of the combined branch: - PollModel (this ladder's BRIDGE_MODEL_KEY-shared/keyed model) acquires connections from Lightweight::GlobalDataMapperPool() per execute() call rather than holding one for its own lifetime (WithMapper removed); applyVotes() (private helper, called from within execute() bodies that don't need to share its connection) makes its own independent acquisition. Verified standalone against the ladder-foundation base: configures and builds with -DMORPH_LADDER_RUNGS=polls and no other rung present. Full suite passes: 557 assertions in 68 test cases (SQLite default). Spec-citation and test-type-name lints clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * ci: nudge to trigger pull_request workflow run * polls: retype plain std::string entity fields to Lightweight strong string types Per docs/superpowers/specs/2026-08-11-strong-storage-types-design.md item 3's polls inventory (spec file since removed from the tree by 2851376, recovered from git history for this change). poll_entity.hpp: - PollRecord::title -> Light::SqlAnsiString<kMaxTitleBytes> (200) - OptionRecord::label -> Light::SqlAnsiString<kMaxOptionLabelBytes> (100) - VoteRecord/CommentRecord/VoteHistoryRecord::participantName -> Light::SqlAnsiString<kMaxParticipantNameBytes> (80) - CommentRecord::body -> Light::SqlAnsiString<kMaxCommentBytes> (500) - VoteHistoryRecord::previousVotesJson -> Light::SqlMaxDynamicAnsiString (serialized JSON, unbounded) - PollEventRecord::kind -> Light::SqlAnsiString<32> (short internal tag, no existing DTO constant) - PollEventRecord::summary -> Light::SqlMaxDynamicAnsiString (free text, no existing bound) pollId/adminToken/participantToken are untouched -- already SqlAnsiString<kTokenBytes> from an earlier pass. poll_model.cpp: added a templated textOf(SqlAnsiString<N>) plus a SqlMaxDynamicAnsiString overload (mirroring pastebin::PasteModel's textOf() precedent) and wrapped every entity->DTO read of these fields in it; DTO->entity writes needed no changes since Light::Field's assignment operator already accepts anything constructible into the field's value type. Added five static_asserts pinning each bounded field's SqlAnsiString capacity to its DTO-level kMax*Bytes constant (matching pastebin's kMaxSyntaxBytes static_assert), so title/label/ participantName/body's storage capacity and validate() bound can never drift apart silently. kind/summary have no DTO constant to pin against and are left unasserted, per the spec's own note on those two fields. schema.cpp: poll_events.kind's DDL widens from Varchar(16) to Varchar(32) and summary from Varchar(200) to Text(), to match the entity's new capacities (kind's entity comment already documented 32 as the chosen size; summary is now genuinely unbounded like previous_votes_json's existing Text() column). test_polls_schema.cpp and every other polls test file needed no changes: all *Record field assignments in these tests use string literals, which construct implicitly into SqlAnsiString<N>/SqlMaxDynamicAnsiString the same way they did into plain std::string, and its two Field::Value() comparisons against string literals resolve through SqlFixedString's operator==(string_view) overload. Verified via the existing build/polls tree (MORPH_LADDER_RUNGS=polls, MSVC/Ninja, Debug), reconfigured and rebuilt incrementally: full rebuild succeeded and all 68 ladder-polls-labeled ctest cases pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * polls: fix cpp-review + Copilot findings on PR #91 cpp-review finding: - schema.cpp: previous_votes_json/summary use NVarchar(0), not Text() -- both back a SqlMaxDynamicAnsiString entity field (genuinely unbounded), and Text() encodes no such contract; matches pastebin's own NVarchar(0) precedent for unbounded storage and the fix already applied to bookmarks' equivalent Text()-vs-entity-capacity mismatch (PR #90) Copilot findings: - VoteView.qml: each vote RadioButton's onToggled fired unconditionally, including on the checked->unchecked transition Qt's shared ButtonGroup triggers on the sibling that just lost the selection -- so clicking one choice could have its own setPick() call overwritten right back by the previously-checked button's own unconditional handler, depending on which of the two `toggled` signals QML fires second. Guarded all three (Yes/If need be/No) on `if (checked)`. - types.hpp: OptionId/PollEventId::operator*()'s doc comment claimed "UB when empty, exactly like std::optional::operator*" -- both wrap a plain std::int64_t with 0 as their own sentinel, not a std::optional, so reading either when "empty" just returns 0, never UB. Rewrote both comments to state the real contract (Copilot's suppressed-comments list named PollEventId only; OptionId carries the identical copy-pasted comment one type up in the same file, so fixed both). Verified: full ladder_polls_tests suite (68 cases) passes against SQLite; the 1 unrelated ctest-reported failure (QtWebSocketBackend concurrent dispatch, untouched by this diff) is the same console em-dash encoding artifact in ctest's -R/-I re-invocation seen on PR #90, not a real failure. --------- Co-authored-by: Yaraslau Tamashevich <y.tamashevich@lastrada.net> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Rung 2 of the application ladder: bookmarks, anchored on linkding. Split out of #41 into its own PR, targeting #88's
ladder-foundationbranch since it depends on the shared testkit/CMake infra there.Draft until #88 merges — retarget to
masterand un-draft once it does.Contents
examples/bookmarks/in full: bookmark/tag/shared-feed/auth models, DTOs, entities, schema migration, Netscape-bookmarks import/export, server, Qt/QML GUI, and tests — 65 files.Includes fixes folded in during review of the combined branch:
BookmarkModel/SharedFeedModel/TagModelacquire connections fromLightweight::GlobalDataMapperPool()perexecute()call rather than holding one for its own lifetime (WithMapperremoved).GetChangesSince's millisecond cursor boundary race (issue GetChangesSince's millisecond cursor comparison can miss a same-millisecond write #43):since/asOfbecome a compoundChangesCursor(timestamp + same-instant id tie-break) instead of a bareTimestamp, so a write landing in the exact same millisecond as the previous poll's cursor is no longer silently dropped.Verification
Configured and built standalone against the
ladder-foundationbase with-DMORPH_LADDER_RUNGS=bookmarks(no other rung present). Full suite passes: 826 assertions in 121 test cases (SQLite default). Spec-citation and test-type-name lints clean.