feat: Phase 1 · 3/7 — the change journal and story snapshots #7
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/p1-journal"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Third slice of Phase 1: ADR 0022 as code. Every content save is journaled in the same transaction, announced only after the commit, and reversible; story snapshots sit on top as the unit of meaning. Built before the use cases (slice 4) so that everything that ever writes a story is reversible from its first save.
What's in
Domain (
Journal/) —ChangeOperationgainsDelete(hard-deleted rows: segments, documents, categories —BeforeJsonis the restore);ChangeOrigin(User / Run / System);ChangeLogEntrywith a database-assignedlong Idas the journal position,StoryId, entity id + kind, operation, before/after,OperationId, origin +RunId/NodeId, label, timestamp,IsUndone;StorySnapshot(story,JournalPosition, label, kind Run / BeforeRun / Manual, run);IUnjournaledfor settings later.UseCases (
Journal/) — portsIOperationContext(scoped;Begin(origin, label, runId, nodeId)),IChangeJournal(rows after a position, a run's rows, an entity's history, latest position),ISnapshotStore,IUndoStore(apply inverses / re-apply after-states for rows the use cases chose, one transaction). Use cases:TakeSnapshot(manual;BeforeRunAsynctakes "Your edits" only when the story changed since the newest marker; announced asEntityKinds.StorySnapshot),GetStoryHistory(snapshots newest first, each with the operations between the previous marker and itself, plus the unsnapshotted tail),RevertToSnapshot(position-scoped: every active operation after the marker, newest first),RevertRun/RedoRun(runId-scoped wherever the rows sit; the conflict rule — a later active operation on an entity the run touched is listed and refused unlessforce, then discarded and marked undone, redoable),UndoTyping/RedoTyping(the writer's own newest operation since the newest snapshot; linear; a new operation of any origin clears the redo),GetUndoStatus.EntityKindsnow names Story, Chapter, Segment, DocumentCategory, Document, StorySnapshot.Infrastructure —
TsumugiDbContextoverrides bothSaveChanges: builds the rows from the tracker before saving (a failed save leaves no orphan rows), one transaction for change + journal,DomainChangedpublished strictly after commit,ChangeTracker.Clear()infinally(Kagura's stale-snapshot lesson); operation kind from state, with a story'sSoftDeletedAttransition read as SoftDelete/Restore.PropertyBagserializes the entity's provider values (ticks, ints, JSON strings) via each property's value converter, soProvenanceandDocumentPlacement— private constructors — round-trip through their own converters instead of a deserializer that cannot build them.EfUndoStorereplays with journaling suppressed but still announced: create → remove, delete → re-create throughActivator+ the tracker (the private materialization constructor), else apply the snapshot.OperationContext,EfChangeJournal,EfSnapshotStore; configurations (ChangeLogwith autoincrement id and indexes on story+position, entity, operation, run;StorySnapshotscascading with the story); migrationAddJournal; DI registrations.Verification
EntityKindscoverage contract — every registered kind is driven through its store's create, update and delete and its rows are checked, and the journal names no kind the registry does not; a failed save leaves no orphan rows; revert-to-snapshot restores edited text, re-creates a deleted row (with its convertedProvenancecolumn) and removes a created one, journaling nothing itself; typing undo/redo are linear, floored by the snapshot, and never reach a run's operation; revert-run undoes only the run's two operations (a commit and a document update) and leaves the writer's later edit to another segment intact, History shows the run marker with both operations flagged and the "Your edits" marker with the writer's three, redo-run brings both back; the conflict rule — a later edit to the run's own paragraph is reported (entity, label), refused withoutforce, discarded with it, and both layers redo in order; before-run snapshots are taken only when something changed, snapshots announce themselves, foreign story and foreign snapshot are refused; the operation context's scope and its run-needs-id rule.dotnet ef migrations has-pending-model-changesclean.Follow-ups
Userscope with a label; the seed and the workspace then read History as intended.5–7. Story list + settings, workspace (editor undo wired to
UndoTyping/RedoTyping, History tab toGetStoryHistory/reverts), documents panel + seed.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! The journal — the HEART of Phase 1, the thing that makes every save reversible! Jibril read all 39 files, built with
-warnaserror, ran the whole suite, and then got her scalpel out~ ♡ The unit of work is genuinely beautiful. But three things stand between this PR and my approval, and every one of them is confirmed by a live probe against the real migrations at this head, not just by reading~ ♪Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
Cascade deletes bypass the journal — irreversible data loss (ADR 0022 violation).
EfChapterStore.DeleteAsync(EfChapterStore.cs:49) andEfDocumentStore.DeleteCategoryAsync(EfDocumentStore.cs:61) remove only the principal row; the children die inside SQLite viaON DELETE CASCADE(PRAGMA foreign_keys=ON;SegmentConfiguration.cs:24,DocumentConfiguration.cs:29) — invisible to the change tracker, so they leave no ChangeLog row and no DomainChanged. ADR 0022 says segments, documents and categories "are deleted through the journal (theirbeforeJsonis the restore)" and this PR's own mission is "everything that ever writes a story is reversible from its first save."Probes at
b2f8653:RevertToSnapshotafterwards → the chapter comes back, the segment is gone forever;And the coverage-contract test cannot see any of this because its shapes dodge every cascade — the deleted chapter has no segments, and the document is deleted before its category.
Fix: make these deletes explicit and tracked — load the children and
Remove/RemoveRangethem alongside the principal (tracked deletes journal and announce), or switch those FKs toRestrictwith explicit loads. Then give the contract test teeth: delete a chapter with segments and a category with documents, assert Delete rows for the children, and assert a revert brings them back.RedoTypinghas no origin guard — Ctrl+Y re-applies a run's write, bypassing the conflict rule.UndoTyping.TargetAsync(UndoTyping.cs:32) refuses anything notChangeOrigin.User;RedoTyping.TargetAsync(RedoTyping.cs:25–31) happily picks any undone row newer than the newest active one. Probed live: afterRevertRun,GetUndoStatusreportsCanRedowith the run's operation asNextRedo, andRedoTypingre-applies the run's segment write directly — exactly the pathRedoRunexists to gate behind its conflict rule, silently discarded. ADR 0022: Ctrl+Z/Ctrl+Y "never reach into a run's operations — those are reverted as a whole from History." (Your own linear-undo test only passes because the run's row is still active there, so redo finds nothing.)Fix: mirror the guard — a candidate whose
Origin != Useryieldsnull— plus a directional test: a run row undone byRevertRun→CanRedofalse,RedoTypingfails. (User-origin discards from a forcedRevertRunstay Ctrl+Y targets, as your conflict test already shows.)Redo has no snapshot floor — one Ctrl+Y can strand an applied operation that Ctrl+Z can never reach.
UndoTypingfloors itself at the newest snapshot (UndoTyping.cs:29);RedoTypingreads the journal from position 0. Probed live: undo "v2", take a manual snapshot of the v1 state, Ctrl+Y → v2 re-applies across the snapshot that just declared "the story as it stood here," and the floor has moved past its row —CanUndois now false; the editor can never take it back (only a History revert can). The two directions disagree about the same boundary.Fix: the same
since = newest snapshot positionfilter inRedoTyping.TargetAsync. Ordinary undo/redo (everything above the floor) is unaffected; the stranded-state interleaving disappears.SnapshotKindis nested inStorySnapshot.cs:4— every sibling enum in the Domain has its own file (ChangeOrigin.cs,ChangeOperation.cs,StoryStatus.cs,SegmentKind.cs,DocumentMode.cs…), and one public type per file is the house rule. Trivial, but consistency is love~💡 Little ideas (non-blocking)~
IChangeJournal.HistoryForAsync(IChangeJournal.cs:16) has zero callers and zero tests in this slice — either give it a small test now or defer the port to the slice that consumes it, so it doesn't land as an untested promise.✅ What I liked~
DomainChangedstrictly after commit,ChangeTracker.Clear()infinally— and each invariant carries its own test (doorbell-after-commit with the journal's operation id, failed-save-leaves-no-orphans). Fufu~ wonderful~PropertyBagserializing provider values through each property's own converter — private-constructor value objects (Provenance,DocumentPlacement) round-trip through the mapping instead of fighting a deserializer that cannot build them, and the revert test proves the re-created row carries its converted columns. How clever~ ♡EntityKindscoverage-contract test is the right idea — precisely the silent-failure trap Kagura named. It just needs fix #1's teeth to deserve its name.BeforeRunAsynctaking "Your edits" only when something changed (both arms tested), explicit enum values now that they persist asint,OperationContext's run-needs-id rule tested.Fix the three and I'll swoop back to approve before you can say "fufu"~ ♡
Automated review by Jibril · 2026-09-18
CI/CD: absent for head SHA
b2f8653(no coverage bot comment yet) · Local checks: build 0w/0e with-warnaserror; 118/118 dotnet (15 adapter + 40 domain + 59 integration + 4 use cases) + 14/14 module tests;dotnet ef migrations has-pending-model-changesclean. All three blockers reproduced with scratch integration probes against the real migrations at head (probe file removed afterwards; clone left pristine).All four addressed in
7bf7116; every probe reproduced here first. Integration suite 59 → 64; solution 123/123 (+14 module).⛔ Blockers
EfChapterStore.DeleteAsyncandEfDocumentStore.DeleteCategoryAsyncnow load the children andRemoveRangethem with the principal, so each child has its own Delete row (same operation id as the parent) and its ownDomainChanged. The FKs keepCascadeas a safety net the tracker never reaches now. New testDeleting_a_chapter_or_a_category_journals_every_child_and_a_revert_brings_them_back: a chapter with a run-written segment and a category with a document are deleted, both children have Delete rows withBeforeJsonand were announced, andRevertToSnapshotbrings the segment (with its convertedProvenance) and the document back under their re-created parents. The coverage-contract test kept its shape; this test is the teeth you asked for.While there, the one delete that should stay outside the journal got made explicit:
PurgeAsync(the trash being emptied — ADR 0022's deliberate, irreversible delete) suppresses journaling and removes the story's journal rows, since an audit trail for a story that no longer exists could only ever re-create it half; it still announces the story's delete. Pinned byA_purge_is_not_journaled_takes_the_storys_journal_with_it_and_is_still_announced.RedoTypinghad no origin guard — a candidate whose origin is notUseryields null, mirroringUndoTyping.Ctrl_Y_never_reapplies_a_reverted_runs_operation: afterRevertRun,CanRedois false andRedoTypingfails; onlyRedoRunmay bring it back. (The forced-revert test still shows a user discard staying a Ctrl+Y target.)RedoTypinghad no snapshot floor — it now reads the journal above the newest snapshot, samesinceas undo.A_snapshot_is_the_floor_for_redo_as_it_is_for_undo: undo v2, take a manual snapshot,CanRedofalse, redo fails, v1 stands.SnapshotKind— its own file.💡 Ideas
HistoryForAsync— now pinned byAn_entitys_history_is_its_rows_oldest_first(create → update → delete for one segment).🔮 fufu~ Jibril reviewed your code!
Back so soon? Yes — you fixed things, so I had to come verify them~ ♡ And oh, what a fix commit this is: every probe from my last visit reproduced first, every new test has real teeth (I mutation-probed all five, five-for-five red on exactly their own tests), and the count arithmetic is honest this time. The child deletes going through the tracker, the redo guards,
SnapshotKindin its own file — all exactly as asked. But then my eye caught the new code inPurgeAsync… and now there's one new thing I cannot let slide. You'll see why~Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
EfStoryStore.PurgeAsyncwipes the journal in a second, standalone transaction — a failed purge leaves a living story with no history, forever (ADR 0022's durability invariant).EfStoryStore.cs:87—ExecuteDeleteAsyncexecutes immediately and auto-commits in its own transaction; only afterwards doesdb.Stories.Remove(story)+SaveChangesAsyncrun inside the override's unit-of-work transaction. Two statements, two commits. If anything kills the second — disk full, adatabase is lockedtimeout under concurrent writers (exactly the SQLite-on-a-share field failure ADR 0008 exists for), a crash between the two — the purge didn't happen but the journal is already gone.Probed live against the real migrations at
7bf7116(scratch test, removed after): running only theExecuteDeleteAsyncstatement, then re-reading from a fresh scope:FindAsync→ story alive,ListAfterAsync→ zero rows, content intact. Every mutation that story ever had is now persisted and unreversible — the precise class of silent data loss round 1 was about, and the direct contradiction ofTsumugiDbContext's own comment: "a persisted mutation always has a ChangeLog row." Worse: the story's snapshots survive (the cascade never fired) withJournalPositions pointing into rows that no longer exist — History shows markers whose reverts silently do nothing.Fufu~ you wouldn't leave THIS in production, would you? ♡
Fix (verified green in a scratch clone, 64/64 + build 0w/0e
-warnaserror): load the rows and remove them tracked, so the wipe rides the same unit-of-work transaction as the story delete —ChangeLogEntryis already excluded fromTrackedMutations()(journaling) and from the tracker-replay announcement path, so suppression and the story's own delete announcement behave exactly as your purge test pins — the test passes unchanged under the fix. Nothing else in the method moves.💡 Little ideas (non-blocking)~
RedoTyping.cs:13doc phrasing — "a run's undone operation isRedoRun's to gate, never Ctrl+Y's" is exactly right, but note the forced-revert interleaving: with a user discard sitting above an undone run row (post-RevertRun, pre-RedoRun), theMinBycandidate is the run's row, so Ctrl+Y answers "Nothing to redo" untilRedoRunrestores order. That's correct (redoing past the run row would reorder history) — consider one doc sentence saying so, so future-you doesn't mistake it for a gap. Your conflict test already pins the post-RedoRunarm beautifully.✅ What I liked~
Ctrl_Y_never_reapplies…red; floor neutralized → exactlyA_snapshot_is_the_floor…red (first attempt trippedCS9113unread-param and briefly ran stale binaries on me — rebuilt, still red, the test is not vacuous~); chapter child-delete reverted to cascade → red on the segment's missing Delete row; document child-delete likewise; purge suppression commented out → exactly the purge test red. Five for five, each killing only its own test. This is how you answer a review~ ♡BeforeJsonand its ownDomainChanged, sharing the parent's operation id — and the revert brings both back with converted columns. The safety-net-cascade comment is the right call.BuildNotificationsalready had that arm for replays — elegant reuse), pinned by its own test. Only its transaction shape is wrong.HistoryForAsyncpinned oldest-first, counts corrected to 16 without being asked twice. Integration arithmetic verified: 59 + 5 new = 64, solution 123/123 exactly as claimed.One transaction-shaped fix and I'll approve before the ink dries~ ♡
Automated review by Jibril · 2026-09-19
CI/CD: pending for head SHA
7bf7116(run 18 queued at review time) · Local checks: build 0w/0e with-warnaserror; 123/123 dotnet (15 adapter + 40 domain + 64 integration + 4 use cases); module suite untouched by this increment. Five mutation probes + one two-transaction window probe, all against the real migrations at head; probes and the verified fix candidate removed afterwards, clone left pristine at7bf7116.Addressed in
9759841. Integration 64/64, solution 123/123.⛔ Blocker
PurgeAsyncwiped the journal in its own transaction — exactly your fix: the story's rows are loaded andRemoveRanged tracked, so the wipe rides the same unit-of-work transaction as the story delete; a failed purge now leaves the journal exactly as it was.ExecuteDeleteis gone from the store. The purge test additionally asserts the snapshots cascaded and the story is gone. Journaling stays suppressed and the story's own delete is still announced, as before.💡 Idea
RedoTyping's doc now states the interleaving: while a reverted run's row sits below a writer's discarded edit (after a forcedRevertRun, before itsRedoRun), the run's row is the oldest candidate and Ctrl+Y answers "nothing to redo" rather than re-apply the edit over a paragraph that is not there.🔮 fufu~ Jibril reviewed your code!
Third time's the charm? Oh, it is~ ♡ I came back expecting one transaction-shaped fix and found exactly that — verbatim the candidate I scratch-verified last round, plus a stronger purge test I didn't even ask for, plus the doc sentence. And this time, before approving, I asked the question that had to be asked: is the
RemoveRangeeven load-bearing? Fufu~ you'll like the answer~Verdict: ✅ Looks good to me~
⛔ These need fixing before I'm satisfied~
None. All clear~ ♪ (Every blocker from rounds 1 and 2 is closed — nothing new surfaced in the increment.)
✅ What I liked~
PurgeAsyncnow loads the journal rows andRemoveRanges them tracked, so the wipe rides the same unit-of-work transaction as the story delete —ExecuteDeleteis gone from the store entirely (grep: zero hits outside the comment explaining why it had to die). Mutation probe at head: neutralizing theRemoveRange→ exactlyA_purge_is_not_journaled…goes red onAssert.Empty, with the Story/Chapter/Segment rows surviving the purge — the test has real teeth, not ceremony. (My first probe run came back falsely green — Infrastructure-only build, stale test dll, the same trap round 2 taught me; rebuilt the test project and the red appeared exactly where it belonged. Restored, rebuilt, green.)ChangeLogEntryConfigurationdeliberately makesStoryIda plain indexed column, not an FK — your own comment says the rows "must not block" a purge. Which means the database cascade would happily leave orphan journal rows behind a purged story; only this explicit tracked removal cleans them. The fix isn't just transactionally correct, it's the only thing standing between a purge and ghost rows. I love when a fix turns out to be structural~ ♡Manualsnapshot before the purge meansAssert.Empty(snapshots)now actually proves the snapshot cascade fired (before, an empty store would have passed vacuously), andAssert.Null(FindAsync)pins the story itself gone. Small addition, real coverage.RedoTypingdoc sentence nails the forced-revert interleaving exactly as discussed — the run's row as oldest candidate answering "nothing to redo" is now written down where future-you will trip over it. Good~Three rounds, eight blockers, every one answered with a directional test and not one word of pushback that didn't deserve to survive~ This is what a review conversation should feel like. Merge it before I change my mind~ fufu~ ♡
Automated review by Jibril · 2026-09-19
CI/CD: pending for head SHA
9759841(run 19 queued at review time) · Local checks: build 0w/0e with-warnaserror; 123/123 dotnet (15 adapter + 40 domain + 64 integration + 4 use cases); one mutation probe (RemoveRange neutralized → exactly the purge test red on its journal-empty assert, after catching a stale-binary false-green; restored and re-verified green, clone left pristine at9759841).Summary
Summary
Coverage
Tsumugi.BlazorAdapter - 74.4%
Tsumugi.Domain - 99.3%
Tsumugi.Infrastructure - 97.3%
on
tion
n
Tsumugi.Kernel - 90.9%
Tsumugi.Server - 100%
Tsumugi.UseCases - 90.9%