Add an ordered, coalescing, identity-aware action log#5
Merged
Conversation
Records every executed action against a stateful model instance, coalesces
repeats into their net effect at an app-controlled checkpoint (e.g. a "Save"
action) while keeping full history for undo, tags every entry with the
model instance's own identity, and replays a log to reconstruct state —
reusing the existing ActionDispatcher/ModelRegistryFactory as the replay
engine rather than inventing a new one.
Core (registry.hpp, model.hpp, bridge.hpp, action_log.hpp, journal.hpp):
- Loggable{No,Yes} rides on the existing BRIDGE_REGISTER_ACTION macro as an
optional 4th argument (default Yes); ActionLogPolicy<A>::coalesce (default
false) is a lightweight, separately-specialised trait.
- IModelHolder::attachActionLog/recordIfAttached automatically records at the
two places Model::execute() is ever actually invoked (ActionDispatcher's
runner for every remote/Qt topology, Bridge::executeVia's localOp for local
mode) — recording is therefore always server-side wherever a client/server
split exists, with no extra plumbing.
- SessionLog keeps full-fidelity history for undo (via journal::replay() over
a shorter prefix — no inverse operations needed) and checkpoint()s a
coalesced view to a durable IActionLog sink.
Phase 2 (file_action_log.hpp, wire.hpp, backend.hpp, remote.hpp):
- FileActionLog: append-only NDJSON, real fsync on flush().
- Closes the remote-identity gap: wire::Envelope::contextKey +
HandlerBinding::contextKey + a non-breaking IBackend::registerModelWithContext
(default-implemented, so LocalBackend/QtWebSocketBackend need no changes) +
RemoteServer::setLogProvider, so a server-created instance behind
SimulatedRemoteBackend can get a real log attached with a real identity.
Phase 3 (kafka_action_log.hpp), per the "interface + fake broker" approach
(no librdkafka or live cluster in this environment):
- IProducer is the seam a real librdkafka-backed implementation plugs into
later; FakeProducer is the in-memory reference impl with a compactedView()
helper reproducing Kafka's own log-compaction semantics.
- KafkaActionLog's key scheme lets one compacted topic serve both coalescing
policies for free: coalesce==true omits a uniquifier (last-write-wins via
compaction); coalesce==false folds in `seq` so compaction never merges
distinct events.
Phase 4 (a Kafka Streams read-model) is left as documented future work in
docs/ARCHITECTURE.md — different tech stack, no live deployment to build it
against here.
280 tests pass. Precise per-line coverage analysis (de-duplicating across
template instantiations, the same approach scripts/aggregate_lcov_branches.py
already uses for this codebase) shows 100% of the new code is exercised.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GnLrDn28LXHHvJStTXHRfV
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
CI's "Build documentation" job runs Doxygen with WARN_NO_PARAMDOC/ WARN_AS_ERROR — several new public methods (IActionLog/InMemoryActionLog/ FileActionLog append+entries, SessionLog entries/undoLast, the Kafka IProducer/FakeProducer/KafkaActionLog surface, FileActionLog's constructor) were missing @param/@return tags. Verified locally with the same doxygen target (`cmake --build --target doc`) before pushing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnLrDn28LXHHvJStTXHRfV
TransactionModel's handler in bank_cli gets an audited HandlerBinding whose contextKey is the demo principal: on LocalBackend the factory attaches a FileActionLog directly; on SimulatedRemoteBackend the same contextKey travels through the register wire envelope and RemoteServer::setLogProvider attaches the identical log server-side. Same application code, same audit file, either backend — this is a runnable demonstration of the client/server placement guarantee the action log design relies on, not just a unit test of it. Deposit/Withdraw/Transfer are audited by the Loggable default; History (a pure read) opts out via the macro's 4th argument. Verified: `bank_cli` runs both scenarios and prints the resulting audit trail — each entry correctly attributed to its principal, with the real account id visible in the recorded payload. `bank_tests` (131 assertions) still passes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnLrDn28LXHHvJStTXHRfV
FileActionLog::~FileActionLog's `_file != nullptr` check was dead code: the constructor either finishes with a valid handle or throws before completing (destructor never runs on a failed construction), and copy/move are deleted, so _file can never be null when the destructor runs. Removed the check instead of writing an unreachable-by-design test for it. Added a real test for entries()'s line.empty() skip: a hand-appended blank line in the NDJSON file (not something FileActionLog itself would ever produce, but plausible from external editing) must be skipped, not passed to fromJson(). The third gap codecov flagged — action_log.hpp's toJson() throwing SerializationError when glz::write_json fails — is left uncovered: it has no realistic trigger for LogEntry's plain string/int fields, matching the same already-accepted pattern in BRIDGE_REGISTER_ACTION's own untested write-failure throw. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnLrDn28LXHHvJStTXHRfV
Kafka (IProducer/FakeProducer/KafkaActionLog and its tests) is removed — dropped for now per request. The IActionLog interface is unchanged, so a real sink can be added back later without touching call sites. The bigger change: morph::journal::setActionLog(log) installs a process-wide default action log. ModelFactory::create<Model>() — the single construction path behind every ordinary model registration, local *or* remote — attaches that default to each new instance automatically. Set it once in main(); no per-model, per-handler, or per-backend wiring is needed, and it reaches RemoteServer-owned instances the same way since they're built through the same factory. defaultActionLog() reads it back; ScopedActionLog (RAII, mirrors morph::log::ScopedLoggerOverride) installs one temporarily and restores the previous one on scope exit. IModelHolder::attachActionLog remains available for callers that need a specific instance identity (contextKey) — an explicit call always overrides the auto-attached default — so HandlerBinding::contextKey and RemoteServer::setLogProvider from the previous commits still work as the advanced per-instance escape hatch. examples/bank/src/cli/main.cpp now demonstrates the simplified path: one setActionLog() call in main() replaces the previous per-handler HandlerBinding/contextKey/setLogProvider wiring, and every one of the app's nine models is audited automatically, for both the local and simulated- remote scenarios. Read-only actions (ListAccounts, GetAccount, GetLoan, ListLoans, LoanScheduleRequest, ListBudgets, SpendingByKind, ListCards, WhoAmI, ListPayments, ListPayees, ListNotifications, GenerateStatement, plus the already-opted-out History) are marked Loggable::No so the resulting trail stays meaningful now that logging applies everywhere by default. 282 core tests pass (down from 287 after removing 5 Kafka-only tests, up 6 for the new default-action-log coverage — net -5, +6 relative to before this change). bank_tests (131 assertions) passes. Verified bank_cli end to end: the printed audit trail correctly attributes every mutation across all nine models to its principal, identically for both backends. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnLrDn28LXHHvJStTXHRfV
toJson()'s write-failure throw was structurally unreachable: I verified against Glaze's own source that dump_int_error (the only write-relevant error code that could apply to a flat string/int struct like LogEntry) is declared in the error_code enum but never actually raised anywhere in Glaze's write path, and the other write-relevant code (recursion-depth limit) doesn't apply to a non-recursive struct. No amount of crafted input could reach that branch through the public API. Rather than leave it as a documented exception, factored the Glaze-error-to- SerializationError conversion out of toJson/fromJson into one shared, non-template function (detail::throwOnGlazeError). Both call sites now route through the exact same compiled branch, and fromJson's failure path already has a real, easy-to-trigger test (malformed JSON) — so that branch is now genuinely exercised, not faked. action_log.hpp is 100% line/branch covered as a result, no new test needed; the existing fromJson test now proves the shared logic works for both directions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnLrDn28LXHHvJStTXHRfV
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.
Summary
Implements issue #3 ("Message queue"): an ordered log of actions executed against a model, with a pluggable durable sink, so state can be reproduced by replay and undone by replaying a shorter prefix.
Model::execute()is ever actually invoked in the codebase — so recording is always server-side wherever a client/server split exists (local vs. remote/Qt topologies), with no extra plumbing required.ActionDispatcher/ModelRegistryFactoryas the replay engine rather than building a new one.entityKey), attached once via the sameHandlerBindingcustom-factory seam already used for other injected dependencies.Loggablerides on the existingBRIDGE_REGISTER_ACTIONmacro as an optional, strongly-typed 4th argument (defaultYes) — no new registration macro.FileActionLog, append-only NDJSON, real fsync) and closes the remote-identity gap (wire::Envelope::contextKey+RemoteServer::setLogProvider, via a non-breakingIBackend::registerModelWithContextdefault method —LocalBackend/QtWebSocketBackendneed no changes).KafkaActionLog+FakeProducer) as an interface + in-memory fake broker — no librdkafka or live cluster dependency. Its key scheme lets one compacted Kafka topic serve both coalescing policies for free once a real producer is plugged into theIProducerseam.docs/ARCHITECTURE.md— different tech stack, no live deployment to build it against in this environment.Test plan
ctest— 280/280 tests pass, zero compiler warningsscripts/aggregate_lcov_branches.py's own approach for this codebase) shows 100% of the new code is exercised; the only untested lines in the whole diff are pre-existing, unrelated codeVERIFY_INTERFACE_HEADER_SETSpasses (every new header compiles standalone)Saveaction's completion checkpointsSessionLogto a real file on disk, andjournal::replay()from that file reproduces the live model's stateSimulatedRemoteBackendCloses #3.
🤖 Generated with Claude Code
https://claude.ai/code/session_01GnLrDn28LXHHvJStTXHRfV