coverage: measure every built rung, not just rung 1 - #142
Merged
Conversation
Closes morph#141.
`scripts/coverage.sh` named exactly two rung test binaries -- rung 0's
`ladder_common_tests` and rung 1's `ladder_pastebin_tests`. Rungs 2, 3 and 4
were never added, so bookmarks, polls and kanban contributed nothing to the
coverage report and were gated by nothing:
grep -c <rung> scripts/coverage.sh
pastebin: 4 bookmarks: 0 polls: 0 kanban: 0
That is ~15k lines of models, presenters and QML adapters -- the code the
ladder exists to exercise -- outside the number entirely. Confirmed against
the live report: Codecov's file list for the tree at master contains no
`examples/kanban/**` entry at all.
Nothing failed when a rung was forgotten. The script ran, the report
uploaded, and the percentage stayed healthy-looking while describing a
shrinking fraction of the ladder as more rungs landed -- the same shape as
morph#133 (a coverage control that looks like it works while measuring
nothing), and the reason morph#136's sanitizer job asserts its own
instrumentation.
So this replaces the per-rung hand-written blocks with a loop over a known-
rungs list mirroring `examples/CMakeLists.txt`'s own `_morph_known_rungs`. A
rung that was not configured still contributes nothing, via the same `-x`
guard as before; a new rung needs its name in one list and nowhere else,
rather than two blocks that can be half-added or skipped.
`codecov.yml` gains the matching `ignore:` entries for rungs 2-4 on exactly
rung 1's terms -- a rung's own `tests/` excluded so the suite does not score
itself, `gui/` and `gui_wasm/` excluded as `main()` shells with no
unit-testable seam.
Deliberately not included: any change to the gate targets. Those numbers are
about to move, because this is the commit that puts three rungs into the
denominator for the first time, and morph#133 (which also has to fix the
`comment.layout` typo that currently invalidates this whole file) should set
them against the completed data rather than against a denominator that is
mid-change.
Expect the headline coverage figure to shift when this lands. That is the
point: it will be the first number that describes the whole ladder.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Yaraslaut
added a commit
that referenced
this pull request
Aug 21, 2026
The QML-facing half of the ledger screen: `LedgerPresenter`'s typed signals become `QVariantList`/`QString`/`bool` properties, and its calls become `Q_INVOKABLE`s taking the plain types QML can pass. Money never becomes a float crossing this boundary. Balances are published as their exact `Rational` triple (numerator, denominator, decimalPlaces) and `storeTransaction` takes minor units as an integer, so nothing is rounded on the way through -- design spec §7's no-float rule applies here exactly as it does on the wire, and a QML boundary is where that most easily leaks. The second test asserts it directly: 50.00 in, `-5000/1` at 2 decimal places out. Idempotency keys are minted here, one per gesture, and the presenter never generates one -- `BoardBridge::moveTask`'s division of labour, including the lesson its fix round recorded: the key belongs to the call it was minted for, captured alongside that call, never stashed on a shared member. `accountOpened` triggers a re-read rather than appending the new account locally. The signal does not carry the whole ledger, and the model is the authority on the list's contents; appending would make the bridge a second source of truth for something it does not own. `kindToText`'s `default:` shares the `Asset` arm rather than standing alone after it. The warning policy requires a default (-Wswitch-default), but a standalone one is unreachable with every enumerator handled, and would be a permanently-uncovered line -- the artefact codecov.yml's own comments already catalogue for `UnitTraits<Unit>::meta`. Sharing the arm satisfies the warning with no dead code, which matters more now that morph#141/#142 put this rung's code into the coverage number at all. Tests: three bridge cases (publishing accounts as QML maps, exact balances, and a model refusal reaching `lastError`). Full ledger suite green at 49 cases / 204 assertions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Yaraslaut
added a commit
that referenced
this pull request
Aug 21, 2026
…143) Closes morph#133. `comment.layout` named `reference`, which is not a member of Codecov's closed layout vocabulary. Codecov rejects an invalid file *wholesale*, so every `status`, `component_management` and `ignore` rule here has been discarded and Codecov's defaults -- a blocking `codecov/patch` at `target: auto` -- silently took their place. Nothing errored; the checks still ran; the file read as though it were in force. That is why it survived: an invalid config behaves like a permissive one. Verified against Codecov's own validator, which now answers `Valid!` for the first time. With the file actually applying, the question becomes what it should enforce. Every status here is `informational: true`, deliberately: ladder (examples/common) 95.85% pastebin (rung 1) 95.99% bookmarks (rung 2) 86.79% polls (rung 3) 85.99% kanban (rung 4) 85.09% The ~10-point split is not chance. The two components that had a gate watching them sit near 96%; the three that never did -- invisible to the coverage number entirely until morph#141/#142 -- sit near 85%. A blocking gate today would halt work on rungs 5-8 to pay down debt from rungs 2-4, which is the wrong order. Finish the ladder, then raise these; flipping `informational` to `false` is a one-line change per component once the numbers support it. Adds components for rungs 2-4 so each rung's number is visible on its own rather than averaged into a single figure. That visibility is the point of keeping the statuses informational rather than dropping them: the gap stays in front of whoever opens a PR while it is being carried, instead of being quietly forgotten. Targets are each rung's measured floor, not an aspiration. An informational status is a reference line, and a line drawn where coverage actually is makes a regression legible on the next PR. An aspirational target would render every PR as failing-but-ignored, which reads as noise and gets tuned out -- the same way this file's own invalidity did. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Yaraslaut
added a commit
that referenced
this pull request
Aug 21, 2026
The QML-facing half of the ledger screen: `LedgerPresenter`'s typed signals become `QVariantList`/`QString`/`bool` properties, and its calls become `Q_INVOKABLE`s taking the plain types QML can pass. Money never becomes a float crossing this boundary. Balances are published as their exact `Rational` triple (numerator, denominator, decimalPlaces) and `storeTransaction` takes minor units as an integer, so nothing is rounded on the way through -- design spec §7's no-float rule applies here exactly as it does on the wire, and a QML boundary is where that most easily leaks. The second test asserts it directly: 50.00 in, `-5000/1` at 2 decimal places out. Idempotency keys are minted here, one per gesture, and the presenter never generates one -- `BoardBridge::moveTask`'s division of labour, including the lesson its fix round recorded: the key belongs to the call it was minted for, captured alongside that call, never stashed on a shared member. `accountOpened` triggers a re-read rather than appending the new account locally. The signal does not carry the whole ledger, and the model is the authority on the list's contents; appending would make the bridge a second source of truth for something it does not own. `kindToText`'s `default:` shares the `Asset` arm rather than standing alone after it. The warning policy requires a default (-Wswitch-default), but a standalone one is unreachable with every enumerator handled, and would be a permanently-uncovered line -- the artefact codecov.yml's own comments already catalogue for `UnitTraits<Unit>::meta`. Sharing the arm satisfies the warning with no dead code, which matters more now that morph#141/#142 put this rung's code into the coverage number at all. Tests: three bridge cases (publishing accounts as QML maps, exact balances, and a model refusal reaching `lastError`). Full ledger suite green at 49 cases / 204 assertions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Yaraslaut
added a commit
that referenced
this pull request
Aug 22, 2026
* docs: correct the DecimalPlaces floor-of-1 claim in LADDER.md and ledger's README
Verified against include/morph/util/rational.hpp's own doc comment,
docs/spec/util/rational.md, docs/spec/util/quantity_type.md, and
tests/test_quantity.cpp (lines asserting DecimalPlaces{0} round-trips):
DecimalPlaces has no floor of 1. Quantity<U, 0> is a legal, tested,
first-class configuration -- zero-decimal currencies (JPY/KRW) are
natively representable with no app-side convention or x-rules gate.
This was a stale claim in both the round-5 forms-gaps summary
(LADDER.md) and ledger's own "Expected strain points" section,
discovered while grounding the rung-5 design spec in the actual
framework API rather than repeating the README's draft claims
verbatim.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: add the ledger (rung 5) implementation design spec
Resolves examples/ledger/README.md's open design questions in writing,
per LADDER.md's discipline rule, covering steps 1-7 of the build order
plus the step-8 sync-philosophy write-up (produced alongside, no new
model code required):
- The per-currency zero-sum invariant, defined precisely (legs sum to
zero within each currency; foreign-amount pairs balance across, never
entering the zero-sum check itself).
- Multi-currency: currency as an account property, not a per-transaction
choice; exact Rational exchange rates; the corrected DecimalPlaces{0}
fact for JPY/KRW (see the companion docs fix in the prior commit).
- Budget aggregation strategy (in-model summation, not SQL) and why the
sanctioned-escape-tier doesn't apply here.
- Rules: reuses kanban's cascade-journaling decision verbatim (cited from
its unmerged design spec), with rule-version pinning as the additional
money-grade requirement layered on top, not a competing option.
- The one framework dependency this branch carries ahead of PR #121
(causalParentId/isReplaying, cherry-picked framework-only).
- Undo as a compensating action, and why undoLast() is disqualified.
- The Rational overflow fuzz test and the pre-decode validation gap,
each routed to a named finding rather than an app-side workaround.
- CSV/OFX import: bookmarks' op-id ledger pattern reused verbatim, plus
a distinct content-hash dedup layer for cross-import duplicate
detection.
- Reports: submit->poll shape, WAL-read-transaction snapshot semantics
(the pre-cleared escape-tier case), and UTC-storage vs. local-month
boundary handling.
- The sync-philosophy benchmark's three scenarios and the explicit
"server arrival order, full stop" statement.
- Empty-principal refusal at the model, per LADDER.md's binding
cross-rung convention.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: expand ledger's cherry-pick set from one commit to four (testkit deps)
Discovered while writing the implementation plan: Tasks 17/23/24 depend
on examples/common/testkit/{action_driver,offline_rig,client_pool,
convergence}.hpp, which TESTING.md's ownership table says predate rung 5
but which, like causalParentId, only exist on the unmerged
ladder-kanban-impl branch as of this writing. Each introducing commit
(ad491c4, 66717e7, 3630a15) was verified scoped strictly to
examples/common/testkit/ + examples/common/CMakeLists.txt, ships its own
test file, and has no kanban app-code entanglement -- cherry-picked
alongside the original causalParentId commit, same rationale, same
verification standard (builds, own tests pass: 7 test cases / 48
assertions, all green).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: add the ledger (rung 5) implementation plan
27 tasks (Task 0 = already-applied cherry-picks, Tasks 1-25 = TDD
implementation, Task 26 = deferred post-merge rebase), covering the
design spec's steps 1-7 plus the step-8 sync-benchmark write-up, backend
and GUI together (unlike rung 4's backend/GUI split):
- Tasks 1-5: scaffolding, strong ids/errors, Currency unit system,
schema, entities.
- Tasks 6-9: account/transaction DTOs, LedgerModel skeleton,
StoreTransaction's per-currency zero-sum invariant, foreign-amount
pairs.
- Task 10-11: BudgetModel, empty-principal refusal.
- Task 12: RuleModel + cascade-journaling with causalParentId and
rule-version pinning, including the named divergence test.
- Task 13: Rational overflow fuzz test + two named framework findings.
- Task 14-16: undo as a compensating action, CSV import with dedup,
reports' submit->poll job idiom with WAL-snapshot semantics.
- Task 17: local-time month boundary handling + offline-stack test.
- Tasks 18-22: presenters/bridges for ledger/budget/rules, the
ReportJobPoller (a new poll-one-job-to-terminal-state idiom, distinct
from EventPoller's open-ended stream shape), QML views.
- Tasks 23-25: multi-client stress test, sync-benchmark write-up +
Scenario A/B/clock-skew tests, coverage gate + reconciliation.
Self-review pass: filled in every "follow Task N's structure" reference
with real inline code (no bare cross-references left, per the
no-placeholders rule), fixed step renumbering after expansion, verified
spec-section coverage (all 12 design-spec sections map to a task).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: rung scaffolding (empty lib/server/tests targets)
* ledger: strong ids, enums, error hierarchy
* ledger: Currency unit system (dp=2 USD/EUR, dp=0 JPY/KRW)
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: database.hpp + schema migrations for every table
Adds ledger::db::configure/applyMigrations/setup (matching bank::db's
three-function split) and every LIGHTWEIGHT_SQL_MIGRATION for the rung's
11 tables: ledgers, accounts, transaction_journals, transaction_legs,
categories, budgets, budget_limits, rules, ledger_imported_ops,
ledger_imported_txn_hashes, ledger_report_jobs.
Also opts ladder_ledger_lib out of the local fastcache-cc compiler-cache
launcher: it was observed serving a stale, empty object for
src/db/schema.cpp.obj regardless of the file's actual content (reproduced
with fastcache-cc invoked directly, in both direct and
FASTCACHE_NO_DIRECT=1 modes -- same poisoned key either way; only
changing the object's output path produced a fresh key). This is a local
machine-cache bug unrelated to morph's own build; disabling the launcher
for this one target is the minimal, reversible workaround.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: correct ledger plan's Task 4 (db::setup signature, migration DDL)
Found during SDD execution, before dispatching Task 4: the original
Task 4 text specified a parameterless `ledger::db::setup()` called
directly from a test -- this contradicted the established convention.
bank/polls both declare `setup(const std::string& connectionString)`,
and polls::db::database.hpp's own doc comment states outright "tests
never call this" -- the real test-time pattern is
morph::ladder::testkit::DbFixture, which configures its own connection
and applies migrations independently.
Also replaced the migration DDL's prose bullet-point description with
real, verified code: cross-checked every Lightweight::SqlMigration
method against bank/bookmarks/pastebin's actual schema.cpp files
(RequiredForeignKey/ForeignKey + SqlForeignKeyReferenceDefinition,
Column vs RequiredColumn for nullability, CreateUniqueIndex as a
separate plan call, NVarchar(0) as the unbounded-text convention).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: correct ledger plan's Task 5 (setup() call, entity code, real API)
Found during pre-dispatch verification: Task 5's test still called
ledger::db::setup() directly (the same error already fixed out of Task 4),
and its entity file was left with a "follow the same shape" placeholder
for 9 of 11 entities instead of complete code.
Replaced with the full ledger_entity.hpp (all 11 entities) and a
DataMapper-based schema test, both verified against real, already-
compiling code: Field<std::optional<T>, ...> for plain nullable columns
(confirmed via Lightweight/DataBinder/StdOptional.hpp's
SqlDataBinder<std::optional<T>> specialization), BelongsTo<> assignment
(`accountRow.ledger = ledgerRow;`) and Query<T>().Where(...).All() copied
verbatim from examples/polls/tests/test_polls_schema.cpp's real usage.
One field (ReportJobRecord::resultJson, nullable + unbounded) has no
existing precedent to copy verbatim -- flagged explicitly for the
implementer to build-verify rather than trust as given.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: entities for every table (Light::Field records)
Adds ledger_entity.hpp with one Light::Field<>-wrapped struct per table
(LedgerRecord, AccountRecord, TransactionJournalRecord,
TransactionLegRecord, CategoryRecord, BudgetRecord, BudgetLimitRecord,
RuleRecord, ImportedOpRecord, ImportedTxnHashRecord, ReportJobRecord),
plus the schema test's AccountRecord round-trip extension.
Deviations from the brief:
- Header include: the brief's <Lightweight/DataMapper/BelongsTo.hpp> +
<Lightweight/DataMapper/Field.hpp> pair does not transitively provide
Light::SqlAnsiString/SqlRealName/PrimaryKey, confirmed by a real
compile failure. Switched to
<Lightweight/DataMapper/DataMapper.hpp>, matching
bookmarks::db::ImportedOpRecord's real, already-compiling include.
- examples/ledger/CMakeLists.txt: extended the existing fastcache-cc
stale-object workaround (previously only ladder_ledger_lib) to also
cover ladder_ledger_tests, which hit the same bug -- a rebuild after
editing test_ledger_schema.cpp kept linking a stale object missing
the new TEST_CASE, verified via strings/--list-tests, not just a
passing ctest run.
ReportJobRecord::resultJson's Light::Field<std::optional<
Light::SqlMaxDynamicAnsiString>, ...> composition (nullable + unbounded,
flagged by the brief as having no direct precedent) compiled and
round-tripped as written -- no adjustment needed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: fix stale schema comment on causal_parent_id (nullable, not empty-string sentinel)
Task 5's reviewer caught a Task-4-authored comment inconsistency: the
migration comment described causal_parent_id as an empty-string
sentinel, but the actual DDL declares it nullable and Task 5's entity
wraps it as std::optional<SqlAnsiString<64>> -- the comment, not the
behavior, was wrong.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: correct ledger plan's Task 6 (AccountInfo.balance as plain Rational, not Quantity<Currency::USD,2>)
Found and resolved before dispatch, per design spec §2's own answer and
Task 3's Money<C> precedent: Quantity<Unit,dp>'s Unit parameter is a
concrete enumerator value, not the enum type, so AccountInfo cannot hold
a single Quantity generic over an account's actual currency. Fixed to a
plain morph::math::Rational field alongside the sibling currency field.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: account_dto.hpp -- OpenAccount, GetLedger
* docs: bulk-fix recurring ledger::db::setup() defect across Tasks 7-23
Found while pre-verifying Task 7: the same setup()-called-directly-in-a-
test error (already caught and fixed in Tasks 4/5's own text) recurred
13 more times across Tasks 7, 8 (x2), 10, 11, 12, 14, 15 (x2), 16, and 23
-- drafted before the pattern was first caught, never swept back through
the rest of the plan. Replaced every occurrence with
morph::ladder::testkit::DbFixture fixture; and added the missing
#include "testkit/db_fixture.hpp" to every freshly-created test file
that was still missing it (test_budget_model.cpp, test_rule_model.cpp,
test_ledger_import.cpp, test_ledger_reports.cpp, test_multiclient.cpp).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: correct ledger plan's Task 7 (keyed-model construction, BRIDGE_MODEL_KEY vs BRIDGE_KEY_FROM)
Found while pre-verifying: the plan assumed a keyed model takes its key
as a constructor argument (LedgerModel model{LedgerId{1}}) and that
BRIDGE_KEY_FROM applies to every keyed action including the first. Both
wrong, verified against polls::PollModel's real shape: a keyed model is
plain default-constructible (PollModel model;, no key argument); the key
routes shared instances at the Bridge/registry layer, read fresh from
each action's own field. BRIDGE_MODEL_KEY is used exactly once (the
model's first keyed action, which also establishes ModelKeyTraits<M>);
every other action sharing the key type uses BRIDGE_KEY_FROM instead.
Also fixed: the OpenAccount execute() body was fabricating a stub
LedgerRecord for a BelongsTo assignment instead of loading the real
persisted parent row (BelongsTo assignment needs an actually-queried
record, per polls::db::OptionRecord's own usage); added the missing
ledger/core/errors.hpp include ledger_model.cpp actually needs for
ValidationError/NotFound; noted that ledger provisioning (no CreateLedger
action in scope) means the test seeds its own ledgers row.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: correct ledger plan's Tasks 8-10 (real code, seeded ledgers, key-per-model caveat)
Same class of pre-existing plan defects: Tasks 8-10's tests either called
the wrong constructor pattern or hardcoded LedgerId{1} without seeding a
ledgers row first (fixed via SQLite's per-test autoincrement reset,
confirmed via DbFixture's drop-and-remigrate behavior); Task 8's
StoreTransaction implementation was left as prose with no code; Task 9's
foreign-amount test was an unbalanceable 2-leg sketch in comments; Task
10's BudgetModel had no DTOs/model code at all and a test that was pure
prose. All replaced with real, verified code. Task 10 also surfaces a
new open question (flagged, not resolved): whether BudgetModel's mixed
per-action key types (LedgerId vs BudgetId vs Account/CategoryId pairs)
can use BRIDGE_MODEL_KEY/BRIDGE_KEY_FROM the way LedgerModel's uniform
keying does, or whether it must run unkeyed -- left for the implementer
to resolve against morph::model::ModelKeyTraits's real requirements.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: LedgerModel skeleton -- OpenAccount, GetLedger
- Adds transaction_dto.hpp's TransactionLeg (StoreTransaction lands in
Task 8), and LedgerModel implementing execute(OpenAccount)/
execute(GetLedger), registered via BRIDGE_REGISTER_MODEL/ACTION.
- OpenAccount's key is wired by hand-written ModelKeyTraits/
ActionKeyTraits rather than BRIDGE_MODEL_KEY/BRIDGE_KEY_FROM: those
macros route through keyToString<K>, constrained to std::integral/
std::string by morph::model::ModelKey, and LedgerId wraps
std::optional<std::int64_t> (like every LEDGER_DEFINE_STRONG_ID
type), so it does not satisfy that concept. PrimaryKey is declared
std::int64_t and key() unwraps LedgerId's payload directly -- the
plain, non-macro customisation point model_key.hpp's own doc
comments anticipate.
- execute(OpenAccount) returns the newly created AccountInfo, not
void: ActionTraits<A>::Result deduces via decltype(execute(...)),
and the registry runner unconditionally does
`auto result = model.execute(action);`, which cannot bind void.
Matches bank::CustomerModel::execute(const OpenAccount&)'s own
dto::AccountInfo return.
- ledger_model.hpp now includes <morph/core/bridge.hpp>, required by
BRIDGE_REGISTER_ACTION's own documented hard requirement
(registerActionExecutorOnce is only defined there) -- same
precedent as polls::PollModel's header.
- types.hpp gains a glz::meta<T> specialisation for every
LEDGER_DEFINE_STRONG_ID type (LedgerId, AccountId, JournalId,
CategoryId, BudgetId, RuleId, ReportJobId), matching
bookmarks::BookmarkId's wire-codec shape -- without it, any DTO
carrying one of these fields fails deep inside glaze's to/from
templates the first time BRIDGE_REGISTER_ACTION tries to serialise
it, which this task's OpenAccount/GetLedger registration is the
first to trigger.
- units.hpp adds currencyToCode/codeToCurrency: header-only constexpr,
matching bank::currencyCode's identical shape (a pure switch over a
small enum) rather than bank::format()'s .cpp split, which exists
only because that function does non-trivial work (std::format,
arithmetic) -- a different complexity class from a bare switch.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: fix review findings on Task 7 (return-value coverage + balance precision)
Two Important findings from independent review of Task 7's LedgerModel:
1. test_ledger_model.cpp discarded OpenAccount's AccountInfo return value
entirely, only verifying creation indirectly via a follow-up GetLedger
call. That return exists specifically because the framework's Result
deduction can't register a void execute() -- add a direct CHECK on
created.id.hasValue() to close the coverage gap on that field.
2. AccountInfo::balance's placeholder zero was hardcoded to
DecimalPlaces{2} in both execute(OpenAccount) and execute(GetLedger),
regardless of the account's actual currency. This rung exists to
exercise both dp=2 (USD/EUR) and dp=0 (JPY/KRW) currencies (per
units.hpp's own doc comment), so a freshly opened JPY/KRW account was
reporting its zero balance tagged at the wrong precision. Derive
DecimalPlaces from UnitTraits<Currency>::meta(currency).defaultDecimals
in both places instead -- the same customization point units.hpp
defines. The real balance computation (summing legs) remains out of
scope for this task; Task 8 now inherits a correctly precision-tagged
zero baseline instead of a wrong one.
Verified with a full build + the complete ladder_ledger_tests suite
(36 assertions, 14 test cases, all passing).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: correct ledger plan's Task 8 (real SqlTransaction/DateTime API, execute() return-type propagation)
Found while pre-verifying: (1) SqlTransaction's guessed constructor
(single-arg) was wrong -- real shape is SqlTransaction{connection,
SqlTransactionMode::ROLLBACK}, verified against bank::LoanModel's own
multi-row commit. (2) DateTime::toEpochMillis() does not exist -- real
conversion is (*timestamp.value).value.time_since_epoch().count(),
verified against bookmarks::db's own nowMs()/fromEpochMs() helpers.
(3) Documented the ladder-wide morph::ladder::now() injectable-clock
convention (examples/common/clock.hpp, LADDER.md framework prerequisite
3) for future tasks with server-stamped timestamps (Tasks 15/16) --
confirmed StoreTransaction's own client-supplied date field is correctly
exempt from that convention. Also propagated Task 10's void-execute()
fixes (LinkAccountToCategory/SetBudgetLimit now return ids, per Task 7's
verified execute()-cannot-return-void discovery) and added the
AccountRecord.category schema addition LinkAccountToCategory needs.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: StoreTransaction -- per-currency zero-sum invariant
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: verify Task 10's ALTER TABLE migration against real Lightweight API
Confirmed AlterTable()/AddNotRequiredForeignKeyColumn() directly against
Lightweight/SqlQuery/Migrate.hpp -- the exact method for a nullable FK
column via ALTER TABLE, replacing the plain AddColumn() guess (which
would have needed a separate AddForeignKey call and wasn't verified as
correct). Reuses Task 4's own categoriesRef() helper.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: foreign-amount pairs -- multi-currency, per-currency zero-sum stays intact
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: correct ledger plan's Task 11 (real Context/session API, ScopedPrincipal test helper)
Found while pre-verifying: morph::session::Context::principal is a
plain std::string (empty means unauthenticated), never
std::optional/.hasValue() as the plan guessed. The real accessor is
morph::session::current() returning const Context* (nullptr outside any
dispatch). The real test-time mechanism to drive an empty-principal
scenario is morph::session::detail::ScopedContext, following
bookmarks::tests::test_bookmark_model.cpp's own real ScopedPrincipal
helper pattern (contextFor()+ScopedContext RAII) verbatim rather than
reinventing it. Also added the missing BudgetModel-side test (the brief
named BudgetModel in scope but only had a LedgerModel test) and extended
the fix instruction to cover every mutating execute() overload on both
models, not just StoreTransaction.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: BudgetModel -- budgets, limits, in-model spent-so-far aggregation
Adds CreateCategory/LinkAccountToCategory/CreateBudget/SetBudgetLimit/
GetBudgetReport actions on a new BudgetModel, following the plain
default-constructible + hand-written ModelKeyTraits/ActionKeyTraits
pattern Task 7 established.
Schema: AccountRecord gains a nullable category_id FK, added via the
first ALTER TABLE migration in this codebase
(AlterTable("accounts").AddNotRequiredForeignKeyColumn(...)).
CategoryRecord's definition moves ahead of AccountRecord in
ledger_entity.hpp since BelongsTo<&CategoryRecord::id, ...> requires a
complete type, not just a forward declaration.
GetBudgetReport's spent computation joins in code (never a raw SQL
SUM() over the Rational columns, per design spec §3): accounts linked
to the budget's category, journals whose date falls in the requested
UTC month (parsed from "YYYY-MM"), then legs matching both id sets via
WhereIn, summed with Rational::operator+ in a loop.
LinkAccountToCategory gets no ActionKeyTraits specialization: the
primary template's hasKey = false is already the correct default for
an action with two co-equal ids and no single natural key.
* ledger: fix Task 10 review findings -- untested date filter, missing ledger_id scope, unvalidated month
Addresses three Important findings from independent review of
BudgetModel::execute(const GetBudgetReport&) (Task 10,
31f267ad35f5a458a9fc238f0e40703381050a88):
1. The date-range filter on the journal query was untested -- both
StoreTransaction calls in the existing test landed inside the query
month, so nothing would catch a regression that dropped the
date-range predicate entirely. Added a third, out-of-month
(February 2026) StoreTransaction against the same Groceries account;
the existing spent == 7550 assertion now only holds if the
out-of-month leg is correctly excluded.
2. The journal-collecting query filtered only by date range, not by
the budget's own ledger -- collecting every journal across every
ledger in the database for that month before building an unbounded
WhereIn list. Added
.Where(FieldNameOf<&db::TransactionJournalRecord::ledger>, "=",
ledgerId) using the budget's own ledger, already available from the
existing budgetRows.front() lookup.
3. monthRangeMs discarded std::from_chars's return status and never
checked the parsed month was in [1, 12] or that the resulting
year_month_day was ok() -- a malformed month like "2026-13" silently
produced a ~255-day range instead of an error. Strengthened
SetBudgetLimit::validate()/GetBudgetReport::validate() in
budget_dto.hpp with a new detail::isValidYearMonth helper (digit
positions, literal '-' at index 4, month in [1, 12]), matching this
rung's existing validate()-at-the-DTO-boundary convention. Also
hardened monthRangeMs itself to check from_chars's ec and
year_month_day::ok(), throwing ValidationError, as defense in depth
for any caller that bypasses validate(). Added a test asserting
GetBudgetReport/SetBudgetLimit both throw ValidationError given
month = "2026-13".
Full ladder_ledger_tests suite: 50 assertions in 19 test cases, all
passing (up from 48/18 before this fix).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: insert Task 11a -- LedgerModel/BudgetModel self-journaling infrastructure
Discovered while pre-verifying Task 12: cascade-journaling requires
LedgerModel to append a manually-constructed LogEntry with
causalParentId set, which is only possible if the model already
journals its own triggering actions. A plain-constructed model (every
test in this plan uses LedgerModel model; with no constructor argument,
per Task 7's own established pattern) is never wrapped by the
framework's registry IModelHolder, so the automatic per-call journaling
never fires for it -- confirmed against kanban's real, already-
implemented attachActionLog/logAction pair (unmerged
ladder-kanban-impl branch), which this task retrofits verbatim into
LedgerModel and BudgetModel before Task 12 needs to build the cascade on
top of it. No behavior change for any existing test (none attach a log,
so logAction stays a no-op exactly as before).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: refuse empty-principal writes at the model (design spec §11)
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: self-journaling infrastructure on LedgerModel/BudgetModel (attachActionLog/logAction, retrofit for Task 12)
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: insert Task 11b (StoreTransaction exactly-once) + fully correct Task 12 (RuleModel/cascade)
Pre-verifying Task 12's divergence test surfaced a real gap: StoreTransaction
is a pure insert, so morph::journal::replay() would double-insert it (unlike
kanban's naturally-idempotent MoveTaskPosition). Inserted Task 11b, copying
kanban's real, verified opId + applied-ops-ledger pattern
(execute(MoveTaskPosition)'s lookup-before-mutate, write-after-commit shape)
onto StoreTransaction.
Also fully corrected Task 12 itself, which had three real defects: (1)
RuleModel's constructor and principal check both used the plan's pre-Task-7/
pre-Task-11 stale patterns; (2) the causal-parent-id minting mechanism was
left as 'resolve the exact mechanism' instead of specified -- now copied
verbatim from kanban's real evaluateRules (mint from a real DB row's own
autoincrement id, e.g. TransactionJournalRecord's, never LogEntry::seq; call
the cascade's *implementation* directly, bypassing any public execute()
overload, which would double-log); (3) SetCategory needs BRIDGE_REGISTER_ACTION
and a public execute() overload even though no client is expected to dispatch
it directly, because morph::journal::replay()'s dispatcher requires the
action type to be registered regardless of who created the entry -- verified
against kanban's own ApplyTagMutation, which is registered for exactly this
reason. Also resolved the two 'which account/which category' design
questions concretely instead of leaving them speculative, and wrote out the
real divergence test (previously all comments), including the replay
read-back API (IModelHolder::into<Model>(), copied from kanban's own real
divergence test) that this plan had not independently verified before.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: StoreTransaction exactly-once via opId + applied-ops ledger (replay-safety prerequisite for Task 12)
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: RuleModel + cascade-journaling with causalParentId and rule-version pinning
Adds RuleModel (CreateRule/UpdateRule) with the same attachActionLog/
logAction self-journaling shape as LedgerModel/BudgetModel. RuleModel is
plain default-constructible and keyed by LedgerId via hand-written
ModelKeyTraits/ActionKeyTraits (LedgerId fails the ModelKey concept, same
as every other keyed model in this rung); UpdateRule is deliberately left
keyless (carries ruleId, not ledgerId -- same shape as LinkAccountToCategory).
Wires rule evaluation into LedgerModel::execute(StoreTransaction): after the
journal+legs commit loop but still inside the same SqlTransaction, a
RuleTrigger::DescriptionContains match cascades into a SetCategory mutation
(links the transaction's first Expense/Revenue leg to the rule's named
category). The category is looked up, never auto-created -- a rule naming a
nonexistent category silently doesn't fire. The cascade's LogEntry carries
causalParentId minted from TransactionJournalRecord's own row id (never
LogEntry::seq, which is sink-local and not stable across restarts/forwarding)
and its payload carries ruleId/ruleVersion, pinning which rule version fired
so a later edit to the rule never changes what replay() reproduces.
SetCategory gets both a public, directly-dispatchable execute() overload
(needed only so replay()'s dispatcher lookup can route a recorded
"SetCategory" entry -- not because a client dispatches it that way) and a
shared setCategoryImpl(mapper, action) the cascade path calls directly,
bypassing the public overload to avoid double-logging. setCategoryImpl takes
the DataMapper by reference so the cascade's mutation commits atomically
with the triggering transaction rather than through a second connection.
Rule evaluation is gated on !morph::journal::isReplaying(): replaying a
StoreTransaction entry must stay a pure no-op for rule purposes, since the
cascade it originally produced is already its own separate recorded entry
later in the same log. Cascade LogEntry emission is deferred until after the
trigger's own logAction call so the trigger always precedes its cascade in
seq order.
Tests: rule CRUD (version bump on update), a cascade test asserting
causalParentId != LogEntry::seq and payload contains ruleId/ruleVersion, and
a divergence test proving replay after editing a rule still reproduces the
original (v1) cascade outcome, never the edited (v2) one.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: Rational overflow fuzz test + two named framework findings (design spec §7)
- tests/test_ledger_rational_fuzz.cpp: fuzz test measuring int64 overflow boundary
when summing 10^9-unit Rational legs
- docs/findings/001-rational-checked-arithmetic-mode.md: no checked-arithmetic mode
in Rational operators; overflow can occur silently at ~9B row sum
- docs/findings/002-rational-no-predecode-validation-seam.md: Rational::setWire
clamps hostile den==0 to den==1 instead of rejecting; bypass for pre-decode
validation
- examples/ledger/tests/test_ledger_model.cpp: test verifying clamped legs are
caught by zero-sum invariant, not by explicit validation
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: fix Task 13 review findings C1/C2 -- real wire decode, fast exact overflow boundary
C1: the pre-decode-gap test now genuinely decodes {"num":5,"den":0,"dp":2}
through glz::read_json (reaching Rational::setWire via the glz::meta<Rational>
wire-codec specialisation) rather than the plain in-process 3-arg constructor,
so it actually exercises finding #002's claim that the wire-decode path
clamps hostile input rather than rejecting it.
C2: the overflow-boundary fuzz test now finds the exact boundary
(9,223,372,037 rows) via a binary search over real Rational::operator+
calls (O(log N) ~33 additions) instead of a naive count<N walk (O(N) ~9.2
billion additions, several minutes at -O2 -- verified directly: an
implementer's first attempt at raising the loop cap to reach the true
boundary via brute force ran for 6+ minutes without finishing and was
killed). Each binary-search step still calls the real Rational::operator+
(via exponentiation-by-squaring over a running 'doubling' term, not a
literal N-object loop) and cross-checks it against a closed-form int64
oracle, so the measurement stays empirical (the type's actual arithmetic
decides the outcome) while running in well under a second. Finding #001
now cites the exact measured boundary instead of a hand-computed estimate
the original test could not have produced.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: fix real signed-overflow UB in Task 13's binary-search fuzz test
Scoped re-review of the C1/C2 fix (commit c5994b5) found a new Critical
bug in sumOfNLegs's doubling helper: 'term = term + term' ran
unconditionally every loop iteration, including the final one where its
result is never consumed (n has already reached 0 after the shift).
For any candidate mid with bit-length >= 34 -- which includes the
binary search's own top-of-range probes and the true measured boundary
itself -- that trailing double computes perLeg * 2^34, overflowing
int64_t: real undefined behavior, not the type's own documented
overflow-at-the-boundary being measured.
Fixed by breaking out of the loop immediately once n reaches 0 after
the shift, before the next double -- every term this function now
computes is one a candidate actually consumes. Re-verified: same exact
boundary (9,223,372,037), same ~0.2s runtime, full morph_tests suite
still 1077/1077 (20139 assertions).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: fix second real signed-overflow UB in Task 13's binary-search fuzz test
Second scoped re-review found the sibling bug to the one just fixed in
4eebe69: sumOfNLegs was still called unconditionally on every binary-
search probe, including probes the oracle had already determined would
overflow (wouldOverflow == true, roughly half the ~33 probes while
bracketing the boundary from above). Even though the result was
discarded on that side, sumOfNLegs's own 'result = result + term'
accumulation still ran Rational::operator+= on values already known to
exceed int64_t's range -- real UB, one step earlier than the boundary
this test claims to safely observe.
Fixed by moving the wouldOverflow check to guard the sumOfNLegs call
itself, not just what happens with its result: the function is now only
ever invoked on probes the closed-form oracle has already certified as
overflow-free, before any Rational arithmetic runs. The overflow side
of the search updates highOverflow directly from the oracle's verdict,
with no Rational call at all. Re-verified: same exact boundary
(9,223,372,037), ~0.15s runtime, full morph_tests suite still 1077/1077
(20139 assertions).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: fully correct and concretize Task 14 (UndoTransaction)
- Fixed a wrong API reference: Rational::operator-() const is a MEMBER
unary negation, not the free binary operator-(lhs,rhs) subtraction
also declared in rational.hpp -- the plan cited the wrong one.
- Resolved a real design gap the plan left as 'reusing that private
implementation, not duplicating it' with no concrete mechanism:
extracted a new storeJournalImpl private helper (mirroring Task 12's
setCategoryImpl precedent exactly) that UndoTransaction calls with
negated legs, rather than either duplicating execute(StoreTransaction)'s
insert logic or reentrantly calling its public overload (which would
double-log and drag in opId/cascade logic meaningless for a reversal).
- Resolved a genuinely novel key-resolution question (UndoTransaction
only naturally carries journalId, but every keyed action in this file
derives its key from a ledgerId field) by adding a redundant ledgerId
field to the action itself, keeping ActionKeyTraits::key() a trivial
field read instead of introducing an unprecedented DB-lookup-inside-
key() pattern.
- Fixed the reversal's own date to morph::time::Timestamp::now() (the
same client-observable-date convention StoreTransaction.date already
uses) -- the plan had cited morph::ladder::now(), the server-audit-
stamp convention reserved for LogEntry::timestampMs, not a journal's
own date field.
- Wrote out the Step 1 test's full body (was a placeholder comment with
no code) using the DB-lookup-for-journal-id pattern this file's own
DTOs require, since GetLedgerResult/StoreTransaction's return value
never exposes a journal id.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: UndoTransaction -- compensating action, never undoLast()
Adds UndoTransaction { ledgerId, journalId } and
LedgerModel::execute(UndoTransaction) -> GetLedgerResult, which inserts a
second, reversing TransactionJournalRecord whose legs are the original
legs negated via Rational::operator-() const (member unary negation),
with causalParentId pointing at the undone journal row. Never uses
morph::journal::undoLast() -- the ledger's own journal is an audit
trail, so undo is a new, visible entry.
ledgerId is redundant with journalId but keeps ActionKeyTraits::key() a
trivial field read like every other keyed action in this file; execute()
independently verifies the looked-up journal's own ledger matches
action.ledgerId.
Extracts LedgerModel::storeJournalImpl (mirroring setCategoryImpl's
role as a single-caller helper) from execute(StoreTransaction)'s
journal-insert + leg-insert + buildLedgerState rebuild, minus the
opId-ledger-write and cascade-evaluation blocks that stay inline in
execute(StoreTransaction) itself. UndoTransaction is the sole caller.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: fully correct and concretize Task 15 (CSV import with dedup)
Four real gaps found and resolved before dispatch:
1. ImportOpId already exists (Task 11b created it specifically for this
task's reuse) -- the brief said to define a new one.
2. ledger_imported_ops's real unique key is (owner_principal, op_id),
confirmed against the actual entity/migration -- the brief said
(ledgerId, opId), a key the table has no column for.
3. No account information anywhere in the brief's CSV format or
ImportLedgerChunk, despite every transaction needing >=2 legs against
real accounts -- raised to the user, ruled: add a required
counterAccountId field, extend the CSV format with an account_id
column, each row posts a two-leg entry against its own account and
the chunk-wide counter-account.
4. Test snippets hardcoded LedgerId{1} with no backing LedgerRecord ever
created -- every other test in this file creates a real row first;
fixed in the rewritten test code.
Also: reuses Task 14's storeJournalImpl (not a new insert-path
duplicate); specifies exact-arithmetic decimal-string-to-Rational
parsing (never std::stod/atof, which would reintroduce the float
imprecision Rational's entire design avoids); scopes the opId-ledger
table to be populated but not yet read back for an early-return this
task's own test doesn't actually need (recorded as a ruling, not a
TODO) -- content-hash dedup alone already gives both of the task's real
tests their correct behavior.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: CSV import -- content-hash cross-import dedup, opId ledger populated
Adds ImportLedgerChunk/ImportResult (import_dto.hpp) and
LedgerModel::execute(ImportLedgerChunk): parses date,description,
account_id,amount CSV rows, posts a two-leg entry per row against the
row's own account and the chunk's shared counterAccountId, and skips
(never throws) any row whose content hash already exists in
ledger_imported_txn_hashes for this ledger. Amounts are parsed by hand
into morph::math::Rational -- never through std::stod/atof.
ledger_imported_ops is populated per chunk (guarded by a lookup so a
replayed opId does not violate its (owner_principal, op_id) UNIQUE
index) but deliberately not read back for an early-return: it stores
no result payload, so an early return could only produce a zeroed
ImportResult, under-reporting a genuine replay's real counts. A
replay is still a safe no-op -- the content-hash check catches the
re-parsed identical rows on its own. This is a deliberate scope
narrowing, not an unresolved TODO.
Per-row commits (via the existing storeJournalImpl helper) rather
than one transaction wrapping the whole chunk: storeJournalImpl opens
and commits its own Lightweight::SqlTransaction on the same
connection, and nesting a second one around the loop would have the
inner Commit() silently end the outer transaction early.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: fully correct and concretize Task 16 (reports: submit->poll, WAL snapshot)
Dispatched a dedicated research pass before writing this task's brief,
since the brief's own text flagged a genuine, unresolved uncertainty
("confirm the exact API against whatever rung 2's own README/spec
documents; if unavailable... use ThreadPoolExecutor::post directly").
Findings, all load-bearing:
1. No worker-pool-from-inside-a-model seam exists anywhere in this
codebase -- exhaustively confirmed (every model file in bank/
bookmarks/pastebin/polls). The design spec's own claim that rung 2
"establishes" one for this purpose is not actually true: bookmarks'
real background job lives entirely at the App/Bridge/RemoteServer
layer, re-entering the model as a fresh client dispatch, never
callable from inside a bare model's own execute(). Raised to the
user; ruled: LedgerModel gets its own IExecutor member, a genuinely
new local pattern -- filed as finding 003.
2. Confirmed the real raw-query API for WAL snapshot pinning:
Lightweight::SqlStatement{connection}.ExecuteDirect(rawSql), with a
raw BEGIN DEFERRED needed first (Lightweight::SqlTransaction itself
issues no BEGIN, only toggles ODBC autocommit -- confirmed against
the vendored source and an existing raw-BEGIN precedent in
db_busy_fixture.hpp).
3. ReportJobRecord::jobId (a string column) and ReportJobId (an int64
strong id) are a genuine type mismatch nothing exercised before this
task -- resolved by storing the row's own stringified id.
4. Ledger's own model code hasn't adopted the pooled-DataMapper
convention every later rung uses -- adopted for this task's own new
worker-thread code specifically, not retrofitted onto existing
execute() methods.
5. Confirmed no deferred/deterministic executor test double exists for
testing the worker-pool side of an async job -- every real precedent
genuinely spins a real thread pool with bounded polling, matching
the brief's own already-correct test shape.
Also fixed two smaller issues found while writing out the full
implementation: GetReportStatus's key derivation (repeating Task 14's
already-rejected DB-lookup-inside-key() pattern was considered and
rejected again, in favor of keying directly on jobId), and the nullable
resultJson field's assignment shape (needs an explicit std::optional
wrap, unlike the existing non-nullable AppliedOpRecord::resultJson).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: reports -- submit->poll job idiom, snapshot semantics, model-owned executor
Adds SubmitReport/GetReportStatus (design spec §9): SubmitReport creates a
Pending ledger_report_jobs row, returns its ReportJobId immediately, and
posts the aggregation to a background executor; GetReportStatus polls that
row and hands back the serialized body once the job reaches Done.
This is the first cross-thread code in LedgerModel, and the first place in
this codebase where a model's own execute() posts background work. No
framework seam exists for that -- every other "background job" here lives
at the App/Bridge/RemoteServer layer and re-enters its model as an ordinary
client dispatch, a layer this rung does not have. LedgerModel therefore
grows its own shared_ptr<morph::exec::IExecutor> (a single-thread
ThreadPoolExecutor by default), filed as
docs/findings/003-no-model-level-background-job-seam.md.
Thread-boundary discipline: the posted lambda captures only plain values
(the job's integer id and the ledger id, both copied). Nothing from
execute()'s stack frame crosses over -- not its DataMapper, not the
thread-local session Context -- since execute() returns long before the
worker runs. The worker acquires its own pooled DataMapper on the worker
thread (the later-rung GlobalDataMapperPool convention; existing execute()
overloads in this file are deliberately left on their bare DataMapper).
The aggregation runs inside a pinned read snapshot: a raw BEGIN DEFERRED
issued via SqlStatement::ExecuteDirect as the first statement on the
worker's connection, before any Query<T>(), then COMMIT on both the success
and the throw path (a read transaction left open holds a SHARED lock that
blocks every writer on every other connection). Lightweight::SqlTransaction
cannot substitute -- it only toggles SQL_ATTR_AUTOCOMMIT and issues no
BEGIN of its own. The job row's own status/result write happens only after
that snapshot is released, so the connection never holds a read lock while
asking for a write one. A catch-all around the whole worker records Failed
so a poller can never spin against Pending forever.
ReportJobRecord::job_id (a string column) and ReportJobId (an int64 strong
id) are reconciled here for the first time: job_id stores the row's own
stringified id, keeping the column consistent with `id` rather than dead
schema, with no migration needed.
GetReportStatus keys on jobId rather than a ledgerId it does not carry --
resolving one via a DB lookup inside key() is the pattern Task 14 already
rejected, and ModelKeyTraits<LedgerModel>::PrimaryKey is std::int64_t
either way.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: fix Task 16 review finding -- leaked read transaction can stall a pooled connection for 60s
Task 16's own review found a real, production-shaped bug: the sequential
ExecuteDirect("BEGIN DEFERRED")/ExecuteDirect("COMMIT") pair left two
paths where a connection could be returned to the pool with the read
transaction still open -- BEGIN DEFERRED itself throwing (outside any
try block), or the recovery COMMIT on the exception path itself throwing
(a real SQLITE_BUSY-on-commit possibility, which replaces the in-flight
exception and used to propagate with the transaction still live).
Lightweight::DataMapperPool::Return performs no transaction cleanup on
a returned connection, so either path silently hands the open read lock
to whichever unrelated caller acquires that connection next, which then
blocks for the full 60s busy_timeout on its first write.
Fixed with WalSnapshotGuard, an RAII wrapper whose constructor issues
BEGIN DEFERRED and whose destructor issues COMMIT unconditionally,
swallowing any commit failure (nothing left to report at that point,
and it must never mask whatever exception is already propagating).
Every path out of the pinned scope -- normal return, or any exception
from computeReportJson -- now runs exactly one COMMIT, with no window
where the connection could be returned mid-transaction.
Re-verified: same 120/120 assertions across the full suite, [reports]
subset stable across 3 repeated runs.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: fix Task 16 Minor review note -- comments stated fix history, not just current behavior
CLAUDE.md's own documentation rule: comments state only current
behavior + rationale, never 'used to'/'had thrown'/changelog framing.
WalSnapshotGuard's doc comment and its call site's comment both
violated this (referencing the prior sequential-ExecuteDirect shape
and its specific failure history) -- rewritten to state only why the
guard's unconditional-COMMIT-on-destruction behavior matters now, not
what it replaced.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: link filed GitHub issues to findings 001-003, add finding 004
Filed four issues from this session's discoveries:
- morph#129: no framework seam for a model's own execute() to post
background work (finding 003)
- morph#130: Rational has no checked-arithmetic mode (finding 001)
- morph#131: Rational's setWire clamps hostile wire input instead of
rejecting (finding 002)
- Lightweight#583: DataMapperPool::Return performs no transaction
cleanup on a returned connection (new finding 004 -- discovered and
fixed at the application layer during Task 16's review, filed
against the vendored Lightweight dependency since the gap is in its
own pool contract, not morph's)
Also recorded fastcached#51 (the stale-cache-entry detour investigated
during Task 13) in the SDD progress ledger.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* tests: opt morph_tests out of fastcache-cc, matching ledger's own existing precedent
examples/ledger/CMakeLists.txt already disables the machine-local
fastcache-cc/fastcached compiler-cache launcher for ladder_ledger_lib/
ladder_ledger_tests, having independently discovered the exact same
stale-cache-entry bug class this session's Task 13 investigation later
rediscovered for tests/test_ledger_rational_fuzz.cpp (which lives under
the separate, un-opted-out morph_tests target) -- a debug print added
directly to the source never appeared in the executed binary, across
repeated rebuilds, surviving a full FastCached service restart.
Applies the identical opt-out to morph_tests for the same reason. CI is
unaffected: fastcache-cc is only found/enabled when a daemon actually
answers on the build machine, never true in CI runners -- this is a
local-development-experience fix only. Filed as
https://github.com/LASTRADA-Software/fastcached/issues/51.
Verified: reconfigured with FASTCACHE_ADDR re-enabled, confirmed
morph_tests's real link command no longer references fastcache-cc,
full rebuild clean, both morph_tests (1077/1077, 20139 assertions) and
ladder_ledger_tests (38/38, 120 assertions) pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs: commit a snapshot of the SDD progress ledger for rung-5 (ledger)
The working ledger at .superpowers/sdd/2026-08-19-ledger-rung5/progress.md
is git-ignored scratch workspace per the SDD skill's own convention, so
it never made it into this branch/PR despite being the full detailed
record of every task's outcome, every review finding, and every ruling
made across Tasks 1-16. Committing a point-in-time snapshot alongside
the plan/spec it belongs with, so the reasoning behind this PR survives
the local checkout it was produced in.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: fix CI-only -Wsign-compare error on GCC/Clang (Linux legs)
test_ledger_model.cpp compared category.Value().value() (an unsigned
long -- the raw FK scalar Light::BelongsTo::Value() returns) directly
against *categoryA/*categoryB (std::int64_t, from CategoryId's
dereference) at three call sites. clang-cl on Windows didn't flag this
under -Weverything, but GCC and Linux Clang's -Wsign-compare (both
under -Werror) correctly caught it -- confirmed as the sole cause of
four failed CI legs (Application ladder, Linux/all-optional-features
on both gcc and clang, Linux/clang-coverage), all failing on the
identical three lines.
Fixed with an explicit static_cast<std::int64_t> on the unsigned side
at each comparison, matching this file's own existing idiom elsewhere
(static_cast<std::int64_t>(row.id.Value()) at every AccountId/LedgerId/
etc. construction site in this same file).
Re-verified locally: ladder_ledger_tests rebuilds clean, 120/120
assertions still pass (unchanged from before this fix -- this was a
warning-level compile issue, not a logic change).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ledger: scope a monthly statement to its local month (Task 17)
Design spec §9's local-time-vs-UTC-storage requirement. Transaction dates are
stored as UTC instants, but "January's statement" is a claim about the
client's calendar: at UTC-5 a transaction booked 2026-01-31T23:30 local is
already 2026-02-01T04:30 UTC and belongs to January regardless.
Adds `ledger/core/time_util.hpp`'s `localMonthToUtcRange()`, which converts
the month's own boundaries to a half-open UTC range once, so stored UTC
instants are compared against them -- never local-time strings against
UTC-stored rows row-by-row, which is the shape that misfiles every
transaction in the boundary hours. Half-open matters concretely: an instant
exactly at `end` is the next month's first moment, so consecutive statements
tile without counting the boundary transaction twice.
`SubmitReport` decodes `params` into `MonthlyStatementParams` on the caller's
thread, keeping the worker's plain-values-only capture discipline intact
(three ints, copied like `jobIdValue` and `ledgerId`), and the aggregation
filters journals to that range.
`MonthlyStatementParams` lives in report_dto.hpp rather than privately in
ledger_model.cpp for the same reason `ReportLine` does -- a client, or a
test, encodes `params` from the same type the model decodes it into instead
of both sides hand-writing the same JSON shape and drifting. It also has to
have external linkage for glaze's reflection to see it at all, which an
anonymous-namespace type does not; the compile error that surfaced this is
the one documented in test files across this tree.
`ReportLine` gains `transactionCount`, and that is the part worth reading
twice. `StoreTransaction` enforces a per-currency zero-sum (design spec §1),
so *any* whole set of transactions nets to exactly zero per currency: the
report body was mathematically incapable of reflecting the filter, and a
January report and an all-time report produced byte-identical bodies. The
filter would have been correct, fully wired, entirely untestable, and
worthless -- with every test passing. The count varies with the period, which
is what makes §9's own stated assertion ("a transaction at 23:30 local time
lands in the report for its local month") expressible at all. Counted over
journals, so a two-leg transaction counts once and one touching two
same-currency accounts still counts once.
Tests: `localMonthToUtcRange` against the boundary case, half-open tiling,
year rollover, leap February, a half-hour zone, and offset direction; plus an
end-to-end case storing both a 23:30-local and a 00:01-local transaction and
asserting each lands in exactly one month, with a third month reporting zero
to prove the filter is applied rather than merely computed.
Both are mutation-verified: inverting the offset sign fails 3 of 4 helper
cases, and forcing the journal filter to null fails the end-to-end case.
Full ledger suite green at 43 cases / 171 assertions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* ledger: add LedgerPresenter, and stop the model header dragging Lightweight into moc (Task 18)
The transport half of the ledger screen: `LedgerPresenter` dispatches
`GetLedger`/`OpenAccount`/`StoreTransaction`/`UndoTransaction`/
`ImportLedgerChunk` through a `BridgeHandler<LedgerModel, AllowShared>` and
re-emits each typed result as a Qt signal. `AllowShared`, not a plain
handler, because `LedgerModel` is keyed per ledger: a `NoSharing` handler
registers its own private instance and never attaches to another's, which
would hand every client a private ledger. Same rationale `BoardPresenter`
and `PollPresenter` already document.
It translates and routes and decides nothing (examples/IMPLEMENTATION.md
rule 2) -- idempotency keys arrive from the bridge above, never minted here,
and a model refusal (the per-currency zero-sum, say) is relayed as `failed`
rather than interpreted.
The larger part of this commit is the include fix that made it compile at
all. `ledger_model.hpp` included `ledger/db/ledger_entity.hpp`, which pulls
in `<Lightweight/DataMapper/DataMapper.hpp>`. moc mis-parses Lightweight's
namespace structure, concludes every namespace after it is nested inside
`Lightweight`, and emits `Lightweight::ledger::gui::LedgerPresenter` --
failing with "no member named 'ledger' in namespace 'Lightweight'".
`ledger_presenter.hpp` is the first `Q_OBJECT` header in this rung to include
the model header, so nothing had made moc parse it before; the trap was
latent from Task 7 onward. Rung 4 never hit it because
`kanban/models/board_model.hpp` includes only core and dto headers and never
its own entity header -- the shape this now matches. The only entity use in
the header is a `const std::vector<db::AccountRecord>&` parameter, so a
forward declaration suffices, and `ledger_model.cpp` already included the
real header directly. Checked before removing: no other translation unit
relied on the transitive include.
Worth having independently of moc -- it takes a heavy ORM header off every
consumer of the model header -- and it means Tasks 19-21's three remaining
presenter/bridge pairs will not each rediscover this.
Tests: three presenter cases (successful read, a result-carrying signal, and
a model refusal surfacing as `failed` rather than an exception). Full ledger
suite green at 46 cases / 183 assertions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* ledger: guard LedgerPresenter's includes from moc, per rung 4's convention
Corrects the explanation in 8741a17, which was wrong about the cause.
That commit fixed the moc failure by removing `ledger/db/ledger_entity.hpp`
from `ledger_model.hpp` and said kanban avoided the problem because its own
model header includes only core and dto headers. The real reason is simpler
and already established here: `board_presenter.hpp` wraps its model and
`morph/core/bridge.hpp` includes in `#ifndef Q_MOC_RUN`, documented in
`board_qml_bridge.hpp` as "moc must not be pointed at morph's template-heavy
bridge.hpp". `ledger_presenter.hpp` simply lacked that guard.
Adds it, matching the convention. Verified load-bearing rather than assumed:
with the guard in place the entity include can be restored to
`ledger_model.hpp` and the build still succeeds, so the guard alone is what
fixes it.
The include removal in 8741a17 is kept anyway -- it takes a heavy ORM header
off every consumer of the model header and matches
`kanban/models/board_model.hpp`'s shape -- but it is a tidy-up, not the fix,
and the history should not claim otherwise.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* ledger: add LedgerQmlBridge, completing Task 18
The QML-facing half of the ledger screen: `LedgerPresenter`'s typed signals
become `QVariantList`/`QString`/`bool` properties, and its calls become
`Q_INVOKABLE`s taking the plain types QML can pass.
Money never becomes a float crossing this boundary. Balances are published as
their exact `Rational` triple (numerator, denominator, decimalPlaces) and
`storeTransaction` takes minor units as an integer, so nothing is rounded on
the way through -- design spec §7's no-float rule applies here exactly as it
does on the wire, and a QML boundary is where that most easily leaks. The
second test asserts it directly: 50.00 in, `-5000/1` at 2 decimal places out.
Idempotency keys are minted here, one per gesture, and the presenter never
generates one -- `BoardBridge::moveTask`'s division of labour, including the
lesson its fix round recorded: the key belongs to the call it was minted for,
captured alongside that call, never stashed on a shared member.
`accountOpened` triggers a re-read rather than appending the new account
locally. The signal does not carry the whole ledger, and the model is the
authority on the list's contents; appending would make the bridge a second
source of truth for something it does not own.
`kindToText`'s `default:` shares the `Asset` arm rather than standing alone
after it. The warning policy requires a default (-Wswitch-default), but a
standalone one is unreachable with every enumerator handled, and would be a
permanently-uncovered line -- the artefact codecov.yml's own comments already
catalogue for `UnitTraits<Unit>::meta`. Sharing the arm satisfies the warning
with no dead code, which matters more now that morph#141/#142 put this rung's
code into the coverage number at all.
Tests: three bridge cases (publishing accounts as QML maps, exact balances,
and a model refusal reaching `lastError`). Full ledger suite green at 49
cases / 204 assertions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* ledger: add BudgetPresenter (Task 19, presenter half)
Dispatches `BudgetModel`'s action surface and re-emits each result as a Qt
signal, following `LedgerPresenter`'s shape exactly -- including the
`#ifndef Q_MOC_RUN` guard around the model and bridge includes, which Task 18
established as this rung's convention after moc mis-parsed Lightweight's
namespaces.
`AllowShared` for the same reason `LedgerPresenter` uses it: `BudgetModel` is
keyed, so a second client working the same ledger's budgets must join the
shared-instance directory rather than register a private instance.
Exposes five actions where the plan listed three. `CreateCategory` and
`LinkAccountToCategory` are not optional extras: a budget is defined *over a
category*, and a category only accumulates spend once accounts are linked to
it, so without them `GetBudgetReport` would report zero spend for every
budget forever. That is the same failure shape as Task 17's month filter --
correct code whose output could not vary -- and worth avoiding twice.
Tests: the create flow through category and budget, an exact limit-vs-spent
report (300.00 arriving as `30000/1` at 2 decimal places, never a float, per
design spec §7), and a refusal surfacing as `failed` rather than an
exception. Full ledger suite green at 52 cases / 225 assertions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* ledger: add BudgetQmlBridge, completing Task 19
The QML-facing half of the budget screen, following `LedgerQmlBridge`'s shape:
typed presenter signals become `QVariantMap`/`bool`/`QString` properties, and
its calls become `Q_INVOKABLE`s over the plain types QML can pass.
Money keeps the same discipline as the ledger bridge. Limits arrive as integer
minor units, and both `limit` and `spent` are published as exact
numerator/denominator/decimalPlaces triples through a shared `putRational`
helper rather than one pre-divided number -- design spec §7's no-float rule
holds at the QML boundary, and the view formats from the exact parts.
Adds `lastCategoryId()`/`lastBudgetId()` beyond the plan's sketch. A view
creating a category and then a budget over it needs the id it just created,
and the presenter's signals carry typed strong ids QML cannot hold; without
these the view would have to re-query to learn what it had just made.
Tests: the full QML-surface chain -- create category, create budget over it,
set a 300.00 limit, read the report -- asserting `30000/1` at 2 decimal places
comes back exactly, plus a refusal reaching `lastError` while `report` stays
empty. Full ledger suite green at 54 cases / 239 assertions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* ledger: add RulePresenter and RuleQmlBridge (Task 20)
The categorisation-rules screen's transport and QML halves, following the
shape Tasks 18 and 19 established -- `AllowShared` handler, `#ifndef
Q_MOC_RUN` around the model and bridge includes, typed results translated to
`QVariant` bags at the boundary.
`ruleUpdated` carries the whole `RuleInfo`, not just an id, because of
`version`. An already-categorised transaction is stamped with the rule version
that categorised it, and editing a rule bumps the version rather than
retroactively recategorising history -- so `version` is exactly what a view
needs to show "rules changed since you last reviewed". Emitting only the id
would leave the GUI unable to answer that without a re-query, which is the
same shape of loss as Task 17's unobservable filter.
`createRule` deliberately takes only `matchText` and `categoryId`; trigger and
action stay fixed. The model has exactly one of each today
(`DescriptionContains`, `SetCategory`), so exposing them as free-form QML
strings would invent a vocabulary the model cannot honour -- QML could pass
"amountGreaterThan" and be silently coerced. A second trigger arrives with a
real parameter and a real mapping.
Tests: presenter and bridge in one file, since the rule surface is small
enough that splitting would duplicate scaffolding without adding coverage.
They assert the version reaches QML after an update, and that a refusal
surfaces on `lastError`. Full ledger suite green at 57 cases / 257
assertions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* ledger: add the report submit->poll GUI (Task 21)
`SubmitReport` returns a job id, not a body, so a view that wants a report has
to wait for one without blocking. This adds the three pieces that do the
waiting.
`ReportJobPoller` is a new idiom rather than a reuse of `EventPoller`, as the
design brief asks. `EventPoller` is shaped for an open-ended stream -- apply N
events, advance a cursor, keep going, `resume()` after a resync. A report job
has one id, one terminal answer, and no meaningful "next"; forcing it into
that shape would mean a cursor that never advances and a `resume()` that must
never be called. It reuses the *pattern* -- the `Dispatch` closure, the
`_liveness` weak token declared last, `&_timer` as the connection's context
object -- while staying a distinct class.
Three of its decisions are load-bearing:
* It disarms *before* invoking the terminal callback, so a handler that
reacts by destroying the poller does not return into a live timer, and a
late reply from an already-dispatched tick cannot deliver a second
terminal callback.
* A dispatch error is terminal, not retried. The execute deadline already
bounds one attempt, and silently retrying a failing call forever is how a
"stuck at Pending" bug hides from a user. The presenter decides whether to
resubmit.
* There is no `resume()`. A finished job is final, unlike an event stream's
resyncable cursor.
`ReportPresenter` holds the poller by `unique_ptr` so a second submission
replaces -- and therefore destroys -- the first, which is wha…
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #141.
The problem
scripts/coverage.shnamed two rung test binaries — rung 0'sladder_common_testsand rung 1'sladder_pastebin_tests. Rungs 2, 3 and 4were never added:
~15k lines of models, presenters and QML adapters — the code the ladder exists
to exercise — outside the report entirely. Verified against live Codecov data
for the tree currently on
master: noexamples/kanban/**file appears at all.Why it went unnoticed
Nothing fails when a rung is forgotten. The script runs, the report uploads,
and the percentage keeps looking healthy while describing a steadily smaller
fraction of the ladder as rungs land.
That is the same failure shape as #133 — a coverage control that appears to be
working while measuring nothing — and the reason #136's sanitizer job asserts
its own instrumentation before trusting a green run.
The change
Replaces the hand-written per-rung blocks with a loop over a known-rungs list
mirroring
examples/CMakeLists.txt's own_morph_known_rungs. Unconfiguredrungs still contribute nothing via the same
-xguard; a new rung now needsits name in one list rather than two blocks that can be half-added.
codecov.ymlgains the matchingignore:entries for rungs 2–4 on rung 1'sexact terms — each rung's
tests/excluded so the suite doesn't score itself,gui/andgui_wasm/excluded asmain()shells.Verified the selection logic against a synthetic build tree with one rung
binary deliberately absent: built rungs are picked up, the missing one is
skipped, and sources expand to
include/,src/,gui_lib/per rung.Deliberately not included
Any change to the gate targets. This is the commit that puts three rungs into
the denominator for the first time, so those numbers are mid-change. #133 —
which must also fix the
comment.layouttypo that currently invalidates thewhole
codecov.yml, so none of its gates run at all — should set them againstthe completed data.
Expect the headline coverage figure to move when this merges. That is the
point: it will be the first number that describes the whole ladder rather than
a quarter of it.
🤖 Generated with Claude Code