Skip to content

Application ladder: rungs 0-3 (pastebin, bookmarks, polls) + shared infrastructure - #41

Closed
Yaraslaut wants to merge 15 commits into
masterfrom
application-ladder
Closed

Application ladder: rungs 0-3 (pastebin, bookmarks, polls) + shared infrastructure#41
Yaraslaut wants to merge 15 commits into
masterfrom
application-ladder

Conversation

@Yaraslaut

@Yaraslaut Yaraslaut commented Aug 6, 2026

Copy link
Copy Markdown
Member

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:

  • Rung 0 — shared infrastructure: MORPH_BUILD_LADDER build 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 presenter
    architecture (Presenter, AppContext, the WASM-remote spike proving QtWebSocketBackend
    works from Emscripten).
  • Rung 1 — pastebin: 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).
  • Rung 2 — bookmarks: multi-user bookmarks with tags, bulk edit, Netscape-format import,
    a cross-user shared feed, and session-based auth.
  • Rung 3 — polls: 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 core framework additions (see below).

Framework changes surfaced by building the rungs (not rung-scoped, land in include/morph/core/):

  • A client-side execute deadline (Bridge::setExecuteDeadline).
  • An async register-or-attach/attach path for shared/keyed models, closing a staleness race and
    two exception-safety gaps in switchBackend (found while building polls' shared poll state).
  • A browser-timer build of TimeoutScheduler for single-threaded WASM.

Findings: docs/findings/001-033 — each a minimal failing test or a spec-cited
impossibility, 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
DynamicForm array-field gap, 033's poisoned-attach reproduction).

CI: a path-filtered ladder-tests job, coverage measurement extended to the ladder's
hand-written code, the GUI/WASM build+gate legs, and (this session) a full pass fixing every
standing CI failure on the branch — a bridge.hpp unused-capture warning, an MSVC /bigobj
gap, a morph_add_rung() check that unconditionally required a testkit target Emscripten
correctly never builds, and half a dozen Clang -Weverything diagnostics the WASM leg's
older 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
(DataMapperPool adoption, GUID ids, Lightweight strong string/timestamp types) from PR review
comments, not yet implemented.

Testing

  • Every rung's model layer is 100% unit tested against a real on-disk SQLite database via the
    shared testkit fixtures — no mocked persistence anywhere.
  • Each rung's GUI has the one offscreen engine-load smoke test examples/TESTING.md requires,
    plus full presenter/bridge coverage in C++.
  • Verified locally this session: ladder_pastebin_tests, ladder_bookmarks_tests,
    ladder_polls_tests, ladder_common_tests all rebuild clean and pass against a real SQLite
    ODBC backend (one known pre-existing Windows temp-file-lock flake in bookmarks' test_app.cpp,
    unrelated to any change in this PR).
  • CI fixes in this PR are being verified against a live run; WASM ladder gate has never passed
    on this branch and is being brought up incrementally, commit by commit, as each new layer of
    breakage surfaces.

🤖 Generated with Claude Code

@Yaraslaut Yaraslaut changed the title Document the application ladder: eight stress-test example apps Application ladder: design docs + rung 0 shared infrastructure Aug 6, 2026
Comment thread examples/pastebin/include/pastebin/db/db_model.hpp Outdated
Comment thread examples/pastebin/include/pastebin/db/paste_entity.hpp
Comment thread examples/pastebin/include/pastebin/db/paste_entity.hpp Outdated
Comment thread examples/pastebin/include/pastebin/db/paste_entity.hpp
Yaraslaut pushed a commit that referenced this pull request Aug 11, 2026
…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>
@Yaraslaut Yaraslaut changed the title Application ladder: design docs + rung 0 shared infrastructure Application ladder: rungs 0-3 (pastebin, bookmarks, polls) + shared infrastructure Aug 11, 2026
Yaraslaut pushed a commit that referenced this pull request Aug 13, 2026
…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>
Yaraslau Tamashevich and others added 4 commits August 13, 2026 14:26
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>
…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.
Yaraslau Tamashevich and others added 10 commits August 13, 2026 21:03
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>
…t lint)

Brings in the file-scope test-type-name collision CI gate (scripts/check_test_type_names.sh)
merged to master as PR #85, closing issue #84.
…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>
@Yaraslaut

Copy link
Copy Markdown
Member Author

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.

@Yaraslaut

Copy link
Copy Markdown
Member Author

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.

Yaraslaut pushed a commit that referenced this pull request Aug 15, 2026
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>
Yaraslaut added a commit that referenced this pull request Aug 15, 2026
* 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>
Yaraslaut pushed a commit that referenced this pull request Aug 16, 2026
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 pushed a commit that referenced this pull request Aug 16, 2026
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>
Yaraslaut added a commit that referenced this pull request Aug 16, 2026
* 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>
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>
@Yaraslaut

Copy link
Copy Markdown
Member Author

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.

@Yaraslaut Yaraslaut closed this Aug 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant