Application ladder: rungs 0-3 (pastebin, bookmarks, polls) + shared infrastructure - #41
Application ladder: rungs 0-3 (pastebin, bookmarks, polls) + shared infrastructure#41Yaraslaut wants to merge 15 commits into
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…r-wide Scopes DataMapperPool adoption, pastebin's animal-name-id -> SqlGuid conversion, plain std::string -> Lightweight strong string types, and int64 epoch-ms -> SqlDateTime across bank/bookmarks/pastebin/polls. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…r-wide Scopes DataMapperPool adoption, pastebin's animal-name-id -> SqlGuid conversion, plain std::string -> Lightweight strong string types, and int64 epoch-ms -> SqlDateTime across bank/bookmarks/pastebin/polls. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2101867 to
4722098
Compare
The application ladder -- a planned sequence of stateful example apps of gradually increasing complexity, each stressing a distinct set of morph subsystems (examples/LADDER.md). This commit carries rung 0: everything the rungs themselves share, not any one rung's own model/DTO/GUI code. - MORPH_BUILD_LADDER build wiring, morph_add_rung() per-rung scaffolding (examples/CMakeLists.txt, cmake/morph_add_rung.cmake). - Shared testkit (examples/common/testkit/): pump.hpp, db_fixture.hpp/ db_fault_fixture.hpp/db_busy_fixture.hpp, backend_rig.hpp, fault_proxy, strand_interleaver.hpp -- the fixtures every rung's own tests build on. - Shared presenter architecture (examples/common/gui/): Presenter, AppContext, event_poller.hpp/.cpp, plus the WASM-remote spike (examples/common/wasm_spike/) proving QtWebSocketBackend works from Emscripten. - Framework changes the rungs surfaced, landing in include/morph/core/ rather than any rung's own tree: RemoteServer's same-model execute reordering, fixed with a per-model ticket gate (tests/test_remote_execute_ordering.cpp). - CI: a path-filtered ladder-tests job, coverage measurement extended to the ladder's hand-written code, the WASM build+gate leg (.github/workflows/wasm-ladder.yml), and every standing CI fix found building the rungs (an MSVC /bigobj gap, a morph_add_rung() Emscripten check, several Clang -Weverything diagnostics the WASM leg's older bundled Clang surfaces that Linux/Windows Clang never did). - docs/findings/001-036: each a minimal failing test or a spec-cited impossibility, opened and closed as the rungs' own construction surfaced or resolved them. - Planning docs (docs/superpowers/plans/) and a design doc scoping a follow-up storage-types pass from PR review comments, not yet implemented. - README-only stubs for five not-yet-built future rungs (crm, forge, kanban, ledger, lims). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Animal-name-keyed pastes with burn-after-read, expiry, and edit conflict detection -- the first rung exercised end-to-end (server, desktop GUI, WASM client). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Multi-user bookmarks with tags, bulk edit, Netscape-format import, a cross-user shared feed, and session-based auth. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Multi-participant polling with vote/comment history, undo, finalization, and a Zulip-pattern event log (GetEventsSince) -- the first rung to exercise the framework's async keyed/shared-model attach path, which drove the core framework additions in the shared-infrastructure commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
4722098 to
46cda21
Compare
…ts, remove settled findings Goes through every finding filed against rung 0-3 (36 files total) and either migrates the ladder's own code onto the public seam that closes it, corrects every stale in-code comment/README passage that still described a closed gap as open, or removes the finding file once its content is durably captured elsewhere (a spec section, a GitHub issue, or the fix's own code comments) -- never leaving a bare historical record behind. Code migrations (findings 019, 024): - Testkit reach-ins onto public seams: Completion<T>::makeSettleable()/ Promise replaces direct CompletionState<T> construction (test_pump.cpp); BridgeHandler::isBound()/whenBound() replaces manual HandlerBinding construction + registerHandler + currentId polling (test_wasm_registration_path_native.cpp, wasm_spike/main_wasm.cpp); QtWebSocketBackend(url, tls, cfg) replaces the defaultDispatcher()/defaultRegistry() padding five call sites no longer need (app_context.cpp, test_fault_proxy.cpp, and the above). StrandExecutor/ModelId reach-in (strand_interleaver.hpp) is kept on purpose, documented as testkit-layer-by-design: those tests prove StrandExecutor's own ordering guarantee and a stand-in would prove nothing. - Presenter::bound()/trackBound() (examples/common/gui/presenter.hpp), backed by Bridge::whenBound(), gates every rung's bootstrap dispatch (PastePresenter/BookmarkPresenter/TagPresenter/SharedFeedPresenter and their QML bridges/Main.qml/BookmarkListView.qml) instead of a 150ms polling Timer. Guarded with QPointer<Presenter>, not a bare `this` capture -- whenBound()'s Completion resolves through the executor asynchronously even in Local mode, so a short-lived presenter torn down before that post runs would otherwise be dereferenced after destruction (caught via a genuine SIGSEGV repro under cdb during this work, in a presenter destroyed between test cases). PollPresenter is untouched: nothing in polls' QML dispatches on Component.onCompleted, so that rung never hits the window this closes. - Along the way, verified and fixed bookmarks' own tags field (CreateBookmark::tags/EditBookmark::tags, a JSON array of strings) now renders and submits correctly through DynamicForm's array-field control -- the README's "tagging is not reachable from the GUI at all" claim was stale; neither Main.qml nor BookmarkListView.qml special-cases the field. Findings removed after independent re-verification against current source (not their own disposition: labels, which are known to drift): 003, 005-009, 011-014, 019-021, 023-026, 028-034 (framework fixes confirmed landed, each with its own commit/regression test), 001, 002, 004 (rung-0 framework prerequisites, all confirmed shipped), 017 (its queueing behavior was already fully documented in docs/spec/core/backend.md -- the finding file was pure redundant history), 018 (folded directly into examples/TESTING.md's testkit section and examples/IMPLEMENTATION.md rule 5, since it's a genuine, still-current limitation that belongs in the governing docs, not a separate finding file), 035 (RemoteServer execute-ordering gate; its full design history, including the reverted first attempt, is now condensed into remote.hpp's own comment rather than a separate file). Two genuine, still-open technical gaps were filed as GitHub issues instead of finding files, since they're real work items someone should eventually pick up: - 022 (sqliteodbc's UPDATE...RETURNING reports success but SQLFetch throws SQLSTATE 24000) -- a third-party driver bug, filed as LASTRADA-Software/Lightweight#545 with the full reproducer; cross-linked with morph's own tracking issue (#58, already existed). - 036 (BookmarkModel::execute(GetChangesSince)'s millisecond-resolution cursor can miss a same-millisecond write) -- already tracked as morph#43; added the GetEventsSince id-cursor precedent and remaining design-work detail from the finding file as a follow-up comment before removing it. Left untouched, correctly: 010 (forms sum-type gap), 015 (reconcileDeclaredPrecision spec/code agreement, already verified matching), 016 (FileOfflineQueue linear-scan dedup) -- all three `documented-limitation`, independently re-checked against docs/spec/forms/forms.md and docs/spec/offline/offline.md, both specs already stating the identical content verbatim. And 022/036's replacement issues are new work items, not closed findings -- left open on GitHub for whoever picks them up next. Verified throughout: full core suite (morph_tests: 9774 assertions, morph_qt_tests: 496 assertions) plus every ladder rung (ladder_common/pastebin/bookmarks/polls_tests: 2423 assertions total), all green. The only non-comment production-code changes are the Presenter::bound()/whenBound() migration and its QPointer lifetime fix; everything else is documentation, test-comment, or finding-file churn.
812834a to
cf68403
Compare
Root cause: BookmarkModel::execute(const GetChangesSince&) filtered on a strict `updatedAtMs > since` millisecond-resolution comparison. A write landing in the exact same millisecond as the previous poll's asOf cursor was silently excluded -- `>` treats "equal" as "not new" -- even though the write happened strictly after asOf was captured in wall-clock terms. Plausible whenever poll -> write -> poll executes within one clock tick (a fast machine, or a loaded CI runner), not a contrived timing window. Fix: GetChangesSince.since / GetChangesSinceResult.asOf become a compound ChangesCursor (timestampMs + a same-instant id tie-break) instead of a bare Timestamp. The query becomes `updatedAtMs > since OR (updatedAtMs = since AND id > lastId)`, ordered (updatedAtMs, id) ascending -- a same-millisecond write with a higher id is included, and the row that established lastId is never re-delivered on a later poll at the same instant. lastId is derived from the highest id among this poll's own returned rows that share asOf's exact timestamp (rows strictly before asOf need no tie-break; no row can exist strictly after asOf, since asOf is captured before the query runs, per the existing, unchanged ordering argument in this function's own comment). Deliberately not the id/sequence-cursor redesign issue #43 leaves open as unresolved ("GetChangesSince's bulk-summary shape... would need to cursor on something like max(id) at the time of the previous poll per bookmark, or move to an outbox/event-log shape of its own") -- that is a larger design change to the wire contract's return shape, not a boundary-condition fix, and rung 3's poll_events (GetEventsSince) already demonstrates the event-log alternative for a genuinely different DTO shape. Tests: two new regression cases in test_bookmark_model.cpp, following the same frozen-single-instant idiom as the existing BulkEdit same-millisecond regression test. Verified the "does not miss a write" case fails against the pre-fix strict-`>` query (0 == 1) and passes after the fix. Full bookmarks suite (121 cases, 826 assertions) and polls/pastebin suites pass with no regressions. Fixes #43 Signed-off-by: Yaraslau Tamashevich <y.tamashevich@lastrada.net>
…e docs); close findings 010/015/016 docs/superpowers/ (6 files: rung 0-3 implementation plans, one design-spec draft) held dated planning-process documents, not the "one file per feature, compressed reference documentation, present tense only" convention CLAUDE.md/CONTRIBUTING.md describe for that path -- git history already covers implementation plans, so these are redundant with it, the same reasoning the earlier docs/findings/ cleanup applied. Removed the whole directory; CLAUDE.md/CONTRIBUTING.md's docs/superpowers/ convention itself is unchanged, since it was never actually violated by the rule, only by these files. Fixed the two comments that cited a deleted plan file by name: examples/polls/gui_wasm/main_wasm.cpp's doc comment (also dropped two dead "finding 017"/"finding 032" bare-number references in the same passage -- both files were already deleted in the earlier findings cleanup) and tests/test_quantity_forms.cpp's i18nKey comment, which cited docs/superpowers/plans/2026-07-20-gui-i18n.md -- a file that did not exist even before this change. Both now state the current behavior directly instead of citing a planning doc. Findings 010 (forms has no sum types), 015 (reconcileDeclaredPrecision retag-vs-round verification), and 016 (FileOfflineQueue linear-scan depth) are all disposition: documented-limitation, test: spec-cited -- already fully covered by docs/spec/forms/forms.md and docs/spec/offline/offline.md citations, the same closed-disposition category the earlier findings-queue cleanup (docs+ladder: close out the findings queue) deleted ~25 of. Deleted; nothing to migrate since the spec already states each limitation. Signed-off-by: Yaraslau Tamashevich <y.tamashevich@lastrada.net>
…lDataMapperPool per call WithMapper (a per-model mixin: one lazily-opened Lightweight::DataMapper, held for the model instance's entire lifetime, never released) is replaced by acquiring a Lightweight::GlobalDataMapperPool() PooledDataMapper at the top of each execute() call and letting it go out of scope at the end of that call. Models hold no database state themselves anymore. Motivation: WithMapper's design meant every live model instance held a dedicated ODBC connection open for as long as it existed, with no idle timeout or release short of full destruction. pastebin has no cap on concurrent model instances at all (no RemoteServer::setLimitPolicy call), so this was an unbounded connection count against whatever a real (non-SQLite) DB server's connection limit turned out to be; bookmarks caps at 256 live instances (examples/bookmarks/src/app/app.cpp), which is still 256 permanently-held connections rather than a shared, reusable pool. Mechanics: - PasteModel/BookmarkModel/SharedFeedModel/TagModel no longer inherit db::WithMapper; db_model.hpp deleted in both rungs. - Each execute() acquires exactly one PooledDataMapper for its own duration -- multiple mapper()-shaped accesses within one execute() (a transaction spanning several statements, a query built up across several .Where() calls) all route through that single acquisition, not a fresh one per access, since two accesses on different pooled connections would silently split a transaction across two connections. - Free helper functions that already took `Lightweight::DataMapper&` (applyTagSet, loadOwned, readTagNames, findOrCreateTagId, addTagAssociationIfAbsent, writeOutboxEntry, loadOwnedTag) now receive `pooledMapper.Get()` at their call sites instead of the old mapper() accessor's return value -- same reference type, different source. - Presenter headers' moc-guard comments (paste/bookmark/shared_feed/tag _presenter.hpp) updated: the model headers no longer pull in Lightweight at all (WithMapper was the only dependency), so the guard now exists purely for morph/core/bridge.hpp's own template machinery, not a DataMapper transitive dependency that no longer exists. Test fix: two SQLITE_BUSY tests per rung relied on "the model's connection opens fresh, under a short busy-timeout hook" -- true by construction under WithMapper (always a brand-new DataMapper on first use), no longer guaranteed once execute() acquires from a shared, potentially-pre-warmed pool (Lightweight::SqlConnection::PostConnect(), which the hook overrides, only fires when the pool actually creates a new connection, not when it hands back an idle one). New shared testkit helper examples/common/testkit/db_pool_drain.hpp's drainPoolIdleMappers() forces the pool's idle list empty immediately before the acquisition each test cares about (holding Config.maxSize acquisitions live across the racy call -- BoundedOverflow's own Return() never idles more than maxSize at once, so that count is always sufficient regardless of the pool's prior state), turning "usually fresh" back into "always fresh." Covered by its own unit test (test_db_pool_drain.cpp) proving PostConnect fires exactly once on the guarded acquisition. Verified: pastebin (832 assertions/50 cases), bookmarks (826/121), and ladder_common_tests (212/69, including the new pool-drain test) all pass, each run twice for reliability. Server binaries build clean. Not yet converted: polls (PollModel, the one keyed/shared-instance model) and bank (11 models, no server) -- separate follow-up commits. Signed-off-by: Yaraslau Tamashevich <y.tamashevich@lastrada.net>
…ol per call Same conversion as pastebin/bookmarks (previous commit): PollModel no longer inherits db::WithMapper or holds a permanent DataMapper. Each execute() acquires exactly one PooledDataMapper from Lightweight::GlobalDataMapperPool() for its own duration. PollModel is the one keyed/shared-instance model in this codebase (BRIDGE_MODEL_KEY, BridgeHandler<PollModel, AllowShared> -- every participant of the same poll shares one instance). This does not change the conversion's correctness: dispatched calls against a shared instance are still serialized one at a time on its own strand, so no two execute() calls ever contend for one acquisition, and no execute()/applyVotes() body holds a connection or transaction spanning across two separately dispatched calls -- each is self-contained (acquire, work, commit, return) within its own call. applyVotes() (the shared helper behind SubmitVotes/UpdateVotes/ UndoLastVoteChange's restore path) makes its own acquisition rather than receiving one from its caller: execute(UndoLastVoteChange&)'s own preliminary read (finding the most recent VoteHistoryRecord) only passes a plain integer (historyRowId) across the boundary, so it has no correctness dependency on sharing a physical connection with applyVotes()'s own transaction. db/db_model.hpp deleted. poll_entity.hpp's WASM-stub branch comment fixed to explain its own #ifndef __EMSCRIPTEN__ guard directly rather than citing the now-deleted file. poll_presenter.hpp/poll_qml_bridges.hpp's moc-guard comments updated: poll_model.hpp no longer pulls in Lightweight at all, so the guard now exists purely for morph/core/bridge.hpp's template machinery. No busy-timeout tests to fix here (unlike pastebin/bookmarks) -- polls has no SQLITE_BUSY-under-a-short-timeout test relying on connection freshness. Verified: full polls suite (557 assertions, 68 cases) passes, run twice for reliability. Server binary builds clean. Lint scripts pass. Not yet converted: bank (11 models, no server) -- separate follow-up commit. Signed-off-by: Yaraslau Tamashevich <y.tamashevich@lastrada.net>
…l per call Same conversion as pastebin/bookmarks/polls (previous commits), across all 11 bank models (AccountModel, AuthModel, BudgetModel, CardModel, CustomerModel, LoanModel, NotificationModel, PayeeModel, PaymentModel, StatementModel, TransactionModel): no model inherits db::WithMapper or holds a permanent DataMapper anymore. Each execute() acquires exactly one PooledDataMapper from Lightweight::GlobalDataMapperPool() for its own duration. AccountModel (and CustomerModel, its keyed sibling) is bank's one genuinely *stateful* model -- hydrate() caches a row snapshot (_row/_owner/_loadedId/ _seenVersion) across separately dispatched execute() calls, invalidated via a process-wide RowVersions counter (db/row_versions.hpp) when another model's write lands behind the cache's back. That caching is of *data*, not of a connection: nothing in this design ever relied on hydrate() and a later mutation sharing a physical connection, so converting to a per-call pool acquisition changes nothing about its correctness -- hydrate() and execute(CloseAccount&) each simply acquire their own connection now instead of reaching into a member `_mapper`. Several files already used a local `auto& dm = mapper();` idiom (loan_model.cpp, payment_model.cpp, transaction_model.cpp) purely for readability under WithMapper; converted to `auto dm = GlobalDataMapperPool().Acquire();` with `dm->`/`dm.Get()` at call sites, preserving that same local-variable structure. Confirmed customer_model.cpp/statement_model.cpp's use of `UserRecord::accounts` (a HasMany relation)'s `.All()` is unaffected by which connection loaded the parent `UserRecord`: Lightweight's HasMany lazy-loader (DataMapper/DataMapper.hpp) calls `DataMapper::AcquireThreadLocal()` internally, entirely independent of both the old WithMapper and the new pool. db/db_model.hpp deleted (bank has no Emscripten build, unlike the other three examples -- no WASM branch to account for here). README/header doc comments updated. bank/gui_wasm/models/auth_model.hpp (bank's own pre-existing WASM shadow-header, unrelated to WithMapper -- it already had no database dependency) had one stale comment fixed. Also fixes an unrelated, pre-existing CMake bug this work surfaced: examples/bank/CMakeLists.txt's own FetchContent_MakeAvailable(Lightweight) call had no CMAKE_SKIP_INSTALL_RULES guard around Lightweight's $<TARGET_PDB_FILE:Lightweight> install-rule generator expression (invalid for a static-library build), unlike examples/common/CMakeLists.txt's identical fetch, which already carries this guard with its own comment explaining why. Only reachable when bank is configured alongside MORPH_BUILD_LADDER=ON, which is what building bank's models against this change required doing for the first time in this tree. Verified: bank_lib, bank_cli, and bank_tests (145 assertions, 21 cases, including test_stateful_account.cpp's AllowShared/hydrate coverage) all build and pass, run twice for reliability. bank_gui (Qt/QML) builds clean too (not launched, per policy on GUI binaries). Lint scripts pass. This completes the WithMapper -> GlobalDataMapperPool migration across all four ladder examples (pastebin, bookmarks, polls, bank). Signed-off-by: Yaraslau Tamashevich <y.tamashevich@lastrada.net>
…er -> pool migration
Both files described the now-removed WithMapper mixin pattern as the
current convention across all four ladder examples (pastebin, bookmarks,
polls, bank). Rewritten to state the actual current behavior: a model
holds no database connection state of its own -- each execute() acquires
one from Lightweight::GlobalDataMapperPool() for its own duration.
TESTING.md's WASM section also simplifies materially, not just in wording:
since no rung's model *header* pulls in Lightweight/ODBC anymore (that
dependency existed only through WithMapper, which lived in a header), the
two-thing WASM story ("configure with -DMORPH_CLIENT_ONLY=ON, AND give
db_model.hpp a persistence-free WithMapper branch under __EMSCRIPTEN__")
collapses to one: -DMORPH_CLIENT_ONLY=ON alone is sufficient, because
cmake/morph_add_rung.cmake's own if(NOT EMSCRIPTEN) guard already means the
model's .cpp (where the real ODBC-backed bodies live) is never compiled
for Emscripten -- there is nothing left in the header for a stub branch to
guard against.
Deferred until now (rather than done alongside each rung's own conversion
commit) since editing a shared, cross-rung doc mid-migration would have
described a state that was only true for whichever rungs had already
converted at that point.
Signed-off-by: Yaraslau Tamashevich <y.tamashevich@lastrada.net>
…eMappers CI (clang-coverage leg) caught what local runs didn't: "CreatePaste surfaces a real SQLITE_BUSY rather than mistaking it for an id collision" timed out at 120s. This test predates the drainPoolIdleMappers fix applied to this file's other two SQLITE_BUSY tests (GetPaste/EditPaste contention) in the WithMapper -> GlobalDataMapperPool commit -- it has the identical structural bug (contendedModel's execute() needs a genuinely fresh pooled connection under the short busy-timeout hook, but the preceding `warmup` model's own acquisition can leave an idle, already-connected mapper in the pool for contendedModel to receive instead), just missed because whether it actually reproduces depends on the pool's prior state, which happened to differ between the interactive runs used to verify locally and this CI leg's own test ordering/timing. Same fix as the other two: drain the pool's idle mappers immediately before installing the busy-timeout hook and constructing contendedModel, so the next acquisition is guaranteed fresh rather than incidentally so. Verified: the specific test passes immediately (no timeout) and the full pastebin suite (832 assertions, 50 cases) passes reliably across three consecutive runs. Signed-off-by: Yaraslau Tamashevich <y.tamashevich@lastrada.net>
PasteRecord::content held a bare std::string, which Lightweight binds to
Varchar(255) by default -- only schema.cpp's explicit .Column("content",
Text()) migration call made that correct for an unbounded paste body.
Light::SqlText is Lightweight's dedicated type for this case: it
self-declares Text() as its column type instead of relying on the
migration to override the C++-side default.
id/syntax already used Light::SqlAnsiString<32>, matching bank's
convention for fixed-width/ASCII/token-shaped columns; content is the
one field that needed the free-form-text counterpart instead.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… safety
Light::SqlText (the previous fix) is still char-based underneath -- its
column type renders as VARCHAR(MAX) on the SQL Server backend, a
single-byte-collation column that would not round-trip non-ASCII paste
content correctly. This example's DbFixture lets ODBC_CONNECTION_STRING
point the same suite at SQL Server instead of its SQLite default, so
that backend is a real target, not hypothetical.
Light::SqlMaxDynamicWideString is wchar_t-based, so its
SqlBasicStringOperations specialization self-declares NVarchar as its
column type -- NVARCHAR(MAX) on SQL Server, and an inert type-affinity
NVARCHAR(N) on SQLite (which stores UTF-8 natively regardless). The
model converts at the DTO boundary (Lightweight::ToStdWideString /
ToUtf8) in both directions, including the raw prepared statement
EditPaste's compare-and-swap binds by hand (kEditPasteSql) -- that bind
needed the same wide value, not just the Field<>-mapped read/write path.
schema.cpp's migration column changes from Text() to NVarchar(0) to
match (0 is the "unbounded" sentinel on both formatters, same role
Text{}'s default size{} played before). Added a round-trip test
covering non-ASCII content through both the DataMapper-bound write path
(CreatePaste) and the raw-statement path (EditPaste's CAS).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Splitting this into smaller, independently-reviewable PRs:
Leaving this PR open as reference until all the split PRs are confirmed merged; will close it then. |
|
All four split PRs are up:
Each of #89-91 is verified standalone (configures/builds/passes its full test suite with no other rung present). Will close this PR once all four are merged. |
Split out of application-ladder (originally bundled with bookmarks/polls and the shared foundation in #41) into its own PR against the rung-0 foundation (#88). Includes the pool-migration and Unicode-content fixes folded in during review of the combined branch: - PasteModel acquires connections from Lightweight::GlobalDataMapperPool() per execute() call rather than holding one for its own lifetime (WithMapper removed). - content is Light::SqlMaxDynamicWideString, not std::string/SqlText -- both of those are char-based and would render as VARCHAR(MAX) (a single-byte-collation column) on the SQL Server backend this same test suite can target via ODBC_CONNECTION_STRING; SqlMaxDynamicWideString's wchar_t-based storage self-declares NVarchar, giving real Unicode columns on every backend. Verified standalone against the ladder-foundation base: configures and builds with -DMORPH_LADDER_RUNGS=pastebin and no other rung present. Full suite passes: 834 assertions in 51 test cases (SQLite default). Spec-citation and test-type-name lints clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ladder: rung 1 -- pastebin Split out of application-ladder (originally bundled with bookmarks/polls and the shared foundation in #41) into its own PR against the rung-0 foundation (#88). Includes the pool-migration and Unicode-content fixes folded in during review of the combined branch: - PasteModel acquires connections from Lightweight::GlobalDataMapperPool() per execute() call rather than holding one for its own lifetime (WithMapper removed). - content is Light::SqlMaxDynamicWideString, not std::string/SqlText -- both of those are char-based and would render as VARCHAR(MAX) (a single-byte-collation column) on the SQL Server backend this same test suite can target via ODBC_CONNECTION_STRING; SqlMaxDynamicWideString's wchar_t-based storage self-declares NVarchar, giving real Unicode columns on every backend. Verified standalone against the ladder-foundation base: configures and builds with -DMORPH_LADDER_RUNGS=pastebin and no other rung present. Full suite passes: 834 assertions in 51 test cases (SQLite default). Spec-citation and test-type-name lints clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * tests: close two real EditPaste coverage gaps, document a third Investigating PR #89's codecov/patch gap (85.04% vs 97.24% target) found two genuinely reachable, previously-uncovered branches in EditPaste's post-CAS-miss classification (paste_model.cpp's "Zero rows matched: classify why" block) -- distinct from the pre-CAS checks the existing "refuses an immutable paste, an unknown id" test already covers, since those never reach the guarded UPDATE at all: - A concurrent delete between EditPaste's first read and its guarded write throws NotFound, not Conflict -- the row genuinely vanished underneath the pending edit. - A concurrent is_editable flip (simulated directly through the locking connection, since no ordinary action un-edits a paste) throws ValidationError instead. Both forced deterministically via the same WaitForGuardedUpdate SqlLogger hook idiom the existing Conflict test already established -- no sleep_for, no guessing at scheduling. Also investigated (but left undone, with a comment explaining why) a third gap: app.cpp's sweep .onError() path, which decrements sweepInFlight when a swept ExpirePaste fails. Forcing a real SQLITE_BUSY inside sweepExpiredOnce()'s worker-thread-dispatched execute() needs drainPoolIdleMappers()'s "next Acquire() is fresh" guarantee to hold across that async dispatch; confirmed by direct instrumentation that it currently does not (some other Acquire()/Return() pair repopulates the idle pool first), and there is no way to observe which path a given Acquire() took to root-cause that further. Filed LASTRADA-Software/Lightweight#548 requesting SqlLogger::OnConnectionIdle/ OnConnectionReuse (already declared, never called anywhere in the library) actually get wired up, which would answer this directly. Two other apparent gaps (paste_qml_bridges.hpp, app.hpp -- Q_OBJECT/ constructor/destructor declaration lines) are the same llvm-cov per-declaration-line reporting artifact confirmed twice already this session: both classes are thoroughly exercised via their .cpp definitions (0 uncovered lines each), and their constructors are directly instantiated in this file's own App-level tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Yaraslau Tamashevich <y.tamashevich@lastrada.net> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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>
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>
* ladder: rung 2 -- bookmarks 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> * ci: nudge to trigger pull_request workflow run * bookmarks: retype entity string fields to Lightweight strong string types 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> * Address code review feedback on PR #90 - 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). --------- Co-authored-by: Yaraslau Tamashevich <y.tamashevich@lastrada.net> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* 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>
|
Superseded — rungs 1-3 (pastebin #89, bookmarks #90, polls #91) have all merged into master individually. This branch's content is now fully a subset of master (diff shows only deletions relative to master, i.e. master has moved ahead), so nothing here needs merging separately. Closing without merge. |
Summary
The application ladder — a planned sequence of stateful example apps of gradually increasing
complexity, each stressing a distinct set of morph subsystems. This PR now carries rung 0
(shared infrastructure) through rung 3 (polls), not just rung 0 as originally scoped:
MORPH_BUILD_LADDERbuild wiring,morph_add_rung()per-rung scaffolding, the shared testkit (
pump.hpp,db_fixture.hpp/db_fault_fixture.hpp,backend_rig.hpp,fault_proxy,strand_interleaver.hpp), and the shared presenterarchitecture (
Presenter,AppContext, the WASM-remote spike provingQtWebSocketBackendworks from Emscripten).
conflict detection; the first rung exercised end-to-end (server, desktop GUI, WASM client).
a cross-user shared feed, and session-based auth.
and a Zulip-pattern event log (
GetEventsSince) — the first rung to exercise the framework'sasync keyed/shared-model attach path, which drove core framework additions (see below).
Framework changes surfaced by building the rungs (not rung-scoped, land in
include/morph/core/):Bridge::setExecuteDeadline).two exception-safety gaps in
switchBackend(found while building polls' shared poll state).TimeoutSchedulerfor single-threaded WASM.Findings:
docs/findings/001-033— each a minimal failing test or a spec-citedimpossibility, opened and closed as the rungs' own construction surfaced or resolved them
(e.g. 017's async-registration-before-connect bug, 030's attachModel staleness, 031's
DynamicFormarray-field gap, 033's poisoned-attach reproduction).CI: a path-filtered
ladder-testsjob, coverage measurement extended to the ladder'shand-written code, the GUI/WASM build+gate legs, and (this session) a full pass fixing every
standing CI failure on the branch — a
bridge.hppunused-capture warning, an MSVC/bigobjgap, a
morph_add_rung()check that unconditionally required a testkit target Emscriptencorrectly never builds, and half a dozen Clang
-Weverythingdiagnostics the WASM leg'solder bundled Clang surfaces that the Linux/Windows Clang never did — plus a design doc
(
docs/superpowers/specs/2026-08-11-strong-storage-types-design.md) scoping a follow-up pass(
DataMapperPooladoption, GUID ids, Lightweight strong string/timestamp types) from PR reviewcomments, not yet implemented.
Testing
shared testkit fixtures — no mocked persistence anywhere.
examples/TESTING.mdrequires,plus full presenter/bridge coverage in C++.
ladder_pastebin_tests,ladder_bookmarks_tests,ladder_polls_tests,ladder_common_testsall rebuild clean and pass against a real SQLiteODBC backend (one known pre-existing Windows temp-file-lock flake in bookmarks'
test_app.cpp,unrelated to any change in this PR).
WASM ladder gatehas never passedon this branch and is being brought up incrementally, commit by commit, as each new layer of
breakage surfaces.
🤖 Generated with Claude Code