fix: the undo floor — Ctrl+Z stops at a story's creation, a version turns Undo off on every open page, an unchanged save journals nothing #44
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/journal-undo-floor"
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?
Summary
Three undo gaps from the journal review. A fourth item was tried and left out; see below.
CreateStory's own operation. The Story row was deleted outright, its chapters and categories by cascade, past the trash and its confirmation, onto a Story not found page with no Redo. Its journal rows and pictures were orphaned, andPurgeStoryrefused them. NowUndoTypingnever targets an operation that holds the Story's Create row, andGetUndoStatusfollows: Undo is off on a fresh story, and a story walks back to its creation and stops there.RedoTyping's doc notes the creation is never a candidate.HistoryEffectsnow takesGetUndoStatus:UndoStatusChanged;StorySnapshotdoorbell while a story is open (manuscript loaded, whether or not History is), it re-reads the open story's state. The doorbell carries the snapshot's id, not the story's.Story.Rename,Describe,Archive,UnarchiveandSetActiveGraphstampUpdatedAtonly on a real change. A trailing space, a letter typed and taken back within the pause, or a second archive from another tab leaves the row unchanged: no journal row, no History entry, no Ctrl+Z step.UpdateStory,SetStoryArchivedandSetActiveGraphreturn the sameStoryDetail, soStoryDetails' echo (keyed onUpdatedAt) behaves as before.ChangeLogcolumn that records which revert undid a row (follow-ups).Docs: ADR 0022 is amended (Status line; the Ctrl+Z bullet now says the creation is a floor). ADR 0029 is amended in four places:
The
StoryImageConfigurationcomment says the same. No migration, no new content kind, so no seed change.Follow-ups, not in this PR:
ChangeLogcolumn recording which revert undid a row, so a migration.EfUndoStoremarks the row active even when its entity is missing. So after a Revert to here across a run whose paragraph the writer then edited, Ctrl+Y re-applies that edit to nothing, and History's Redo then answers Nothing to redo. This is pre-existing.UndoTyping,RedoTypingandRedoRevertread the story's whole journal when it has no snapshot.docs/stories/story-workspace.mdanddocs/REQUIREMENTS.mdcould say that Ctrl+Z stops at a story's creation.Verification
Suites.
dotnet teston the branch rebased onto main3b892bd(M2b and slice 8c in): Domain 470, UseCases 155, Integration 377, BlazorAdapter 358, all green, 0 build warnings. The touched classes ran 5 more times: Integration HistoryAndRevert, ImageField, AppSettings, StoryUseCase, FieldJournal and Journal (68 tests); adapter HistoryEffects, HistoryPage, HistoryPanel, StoryGeneralPage and ChapterPage (56); Domain StoryTests (9). The full adapter suite ran 8 more times.Flake note, not this change's. M2b's
ChapterPageTests.Changes_made_elsewhere_reach_the_module_as_deltas_of_what_differs_and_its_own_writes_never_dofailed 2 of those 5 runs on a 15 sWaitForAssertiontimeout while the machine was under load. It then passed 10/10 alone, 6/6 with the touched classes and 8/8 in the full adapter suite. It publishes only Segment doorbells, for whichHistoryEffectsdispatches nothing, as on main. It advances its clock withClock.Advance, notAdvanceAsync.New and rewritten tests:
HistoryAndRevertTests.Ctrl_Z_walks_a_new_story_back_to_its_creation_and_stops_there: Undo is off on a fresh story; after edits the story walks back and stops, still listed and whole; redo brings the edits back.HistoryAndRevertTests.A_version_saved_over_an_undone_edit_leaves_Ctrl_Y_the_edits_made_after_it: the round trip.ImageFieldTests.Ctrl_Z_walks_a_new_story_back_to_its_creation_and_its_pictures_stay_for_the_redoandAppSettingsTests.Undo_reaches_past_a_settings_change_to_the_writers_last_content_change: both rewritten; they used to undo a creation.StoryTests.A_mutator_handed_what_the_story_holds_stamps_nothing, plusUpdatedAtasserts on the archive and graph tests.StoryUseCaseTests.A_save_that_changes_nothing_journals_nothing_and_leaves_no_undo_step.HistoryEffectsTests.A_snapshot_taken_anywhere_re_reads_the_open_storys_undo_state_on_any_page, and the save-a-version test now asserts the re-read.HistoryPageTests: the head's Undo, pinned by DOM (disabled, title Nothing to undo (Ctrl+Z)), after a version saved on the page and after one saved in another tab while the General page is open.Mutation probes, each caught by its own test:
Operation == Createdropped: HistoryAndRevert, AppSettings and StoryUseCase;EntityKind == Storydropped: HistoryAndRevert and ImageField;FieldJournalTests.A_forced_run_revert_…;FieldJournalTests.Once_Ctrl_Z_has_made_the_field_change_whole_….HistoryEffects:HistoryEffectsTestsandHistoryPageTests.Saving_a_version_…(DOM);HistoryEffectsTests(action count);StorySnapshotcondition, with History-loaded instead of manuscript-open, matched on the story id, or with the wrong id:A_snapshot_taken_anywhere_…andA_version_saved_in_another_tab_….Story: each of the five guards defeated,Renamecomparing the untrimmed title,Describecomparing the raw input, andRenamestamping with its guard kept. Each is caught by the domain test and the use-case test.Independent review (two reviewers, 37 probes) found:
RedoTypingwindow and pinning the round trip.StoryIdfilter in the new MAX query (major), the unpinned System-origin rule (minor), the run exception letting a content replay through (minor) and the ADR wording (nit). They went with the rule andIChangeJournal.LatestActivePositionAsync; the replay is listed as a follow-up.HistoryPageTestsdrove the History panel's Save a version dialog, which the story-list PR in this batch turns into a form submit. The test now dispatchesSaveVersionRequested(HistoryPanelTests drive the dialog) and still pins the head by DOM.AppSettingsTests.The_provider_view_asks_nobodyfailed once in about 35 runs. The likely cause is environmental:SqliteBackedTestkeeps its databases underPath.GetTempPath(), and the shared/tmpquota ran full on this machine during the session. It did not recur in 7 runs here withTMPDIRoff the tmpfs.Browser pass: the implementer's, before the rebase; headless Chromium, seeded world. It confirmed:
It was not repeated after the rebase, which touched none of these files.
HistoryPageTestspins the head by DOM on the rebased code.🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagaku.Flow - 99.7%
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__ColorToken_2
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__Socket_1
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__TypeId_0
Kagaku.Flow.OpenRouter - 100%
Kagaku.Flow.Testing - 100%
Tsumugi.BlazorAdapter - 95.6%
Tsumugi.Domain - 96.1%
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__DigitsAndSeparators_2
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__LineBreak_0
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__NumberToken_1
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__Plain_4
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__ThousandsGroup_3
Tsumugi.Infrastructure - 97.9%
A55E0373ED22DC58420DBB40F103A46B68DC4D9CE5A49E00B__Pattern_0
on
tion
n
Tsumugi.Kernel - 90.9%
Tsumugi.Server - 100%
Tsumugi.UseCases - 98.3%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ An undo floor! The one boundary every journaling editor eventually bleeds on — Ctrl+Z calmly walking a story back through its own birth and deleting it past the trash, past the confirmation, onto a page with no Redo. Delicious. I read every line of the diff, then the whole files around it, then I started breaking things to see if your tests would catch me. Fufu~ they did. Every time. ♡
Verdict: ✅ Looks good to me~
✅ What I liked~
UndoTyping.TargetAsync) — grouping byOperationIdfirst and then checkingEntityKind == Story && Operation == Createis exactly right:CreateStoryjournals the story, its first chapter and both starter categories under one operation id, so the whole creation group is the floor, not just the Story row. And because a floor only exists below the newest active row, an undone creation beneath a saved version stays reachable for Ctrl+Y — the round trip your independent reviewers fought over is now pinned (A_version_saved_over_an_undone_edit_leaves_Ctrl_Y_the_edits_made_after_it). The rejected narrower rule is documented with the reason. That's how a decision should be buried. ♪Story— the loveliest part: the guard lives in the domain mutator, so no journal row, noUpdatedAtstamp, no doorbell, nothing — because the journal is ChangeTracker-driven (TsumugiDbContext.BuildJournal). One guard, four effects for free. AndStorynow mirrors its own sibling:Segment.Writehas guardedstored == Textsince forever;SoftDelete/Restorealready guarded; now all seven mutators are consistent. TheStoryDetailsecho claim is real, too —OnParametersSetkeys on(Id, UpdatedAt, Title, Description)and the trim-echo arm at line 47 tolerates the untrimmed field exactly as before.HistoryEffects.OnDomainChangesAsync) —Manuscript is not null && action.Touches(EntityKinds.StorySnapshot)is precisely scoped: the shell renders the head row only when the manuscript is loaded (loading state has no Undo button to stale),Touches(kind)ignores ids on purpose (the doorbell carries the snapshot's id — verified inTakeSnapshot's publish), and the reducer's staleness rule drops any late answer. This mirrorsDocumentsEffects.SettleAsync's unconditionalUndoStatusChanged— sibling-exact.Operation == Createalone → 2 RED;Renamecomparing raw title → domain + use-case RED;Describecomparing raw input → RED;Manuscriptgate → RED;History is not nullinstead → RED.Six mutants, six clean kills on their own pins, no collateral. And the DOM-level pins (
disabled+Nothing to undo (Ctrl+Z)title, after a save on the page and one from another tab through the realDomainChangedBridge) are exactly the seams a user would feel.💡 Little ideas (non-blocking)~
Doorbell(...)stamps every fake change withChangeOperation.Createregardless of kind.Touchesonly reads the kind so nothing is wrong today, but a future assertion onOperationwould silently test a fiction. AChangeOperation op = ChangeOperation.Createparameter would keep the helper honest.await getUndoStatus.ExecuteAsync(...)dispatches inOnSaveVersionAsync/OnDomainChangesAsyncread beautifully as-is — I only noteDocumentsEffectsalready has this exact shape five times; if a sixth home appears, a tiny sharedResendUndoStatus(storyId, dispatcher)helper would DRY it. Not yet — two sites is not a pattern violation. ♪The disclosed follow-ups (Ctrl+Y after Revert to here needing a
ChangeLogcolumn, the replay-over-missing-entity pre-existing gap, unbounded journal loads) are correctly out of scope and honestly named — the two "equivalent survivors" analysis was a nice touch of rigor.Local verification (CI absent for
47fdf78, no coverage bot yet): Release build 0 warnings / 0 errors; suites digit-exact with the PR body — Domain 470/470, Integration 377/377, BlazorAdapter 358/358; six mutation probes all killed, tree restored pristine at47fdf78(git statusclean). The known-flakyChapterPageTests.Changes_made_elsewhere…under load passed in my full adapter run, matching your flake note.Fufu~ a story is safe from its own undo now. Merge it before I change my mind~ ♡
Automated review by Jibril · 2026-09-26
CI/CD: absent for head SHA · Local checks: build 0w/0e · 470/377/358 green · 6/6 probes red-on-target
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! An undo floor — the kind of fix where the bug report is a horror story ("Ctrl+Z deleted my story past the trash with no Redo") and the fix is one honest predicate. And you made the journal's silence do the work — no-op saves journal nothing because the change tracker sees nothing. I got genuinely giddy tracing that. ♪
Verdict: ✅ Looks good to me~
What I verified mechanically (clone at
47fdf78, submodulesKagaku.UI d512741/OpenRouter.Net b002f5e, pristine —git statusclean after all probes):CreateStoryopens ONEoperations.Beginscope for story + chapter + starter categories, so every row of the creation shares oneOperationId.UndoTyping.TargetAsyncrefuses the whole operation when any row isStory+Create— the story can never be deleted from under itself.GetUndoStatusrides the sameTargetAsync, so the button and the behavior cannot disagree. That shared-target design is exactly right.Segment.Writealready had this pattern (stored == Text && provenance == Provenance → return false), soStory's five guards extend an established domain idiom, andSoftDelete/Restoresiblings already guarded the same way. Since journaling is change-tracker-driven (BuildJournaloverTrackedMutations()), a guard's early return means no modified entry, no row, and no doorbell — the claim "no journal row, no History entry, no Ctrl+Z step" holds mechanically, not by assertion. The old unconditionalUpdatedAtstamp would itself have marked the row Modified; the guards are load-bearing. Fufu~ clever girl. ♡Manuscriptis non-null on every open story page (WorkspaceLoaded), theStorySnapshotkind match is right (the doorbell carries the snapshot's id, not the story's — so every open story re-reads its own), and the reducer's staleness guard (action.StoryId != state.StoryId) is inherited for free.DocumentsEffectsalready dispatchedUndoStatusChangedthe same way — sibling-consistent.Ctrl_Z_walks_a_new_story_back_to_its_creation_and_stops_thereRED on the fresh-storyUndoStatusassert (CanUndo = True, NextUndo = User — created 'Fresh').Renameguard deleted → RED in bothStoryTests.A_mutator_handed_what_the_story_holdsandStoryUseCaseTests.A_save_that_changes_nothing_journals_nothing(the two-layer pin works).HistoryEffectsTests.A_snapshot_taken_anywhere…andHistoryPageTests.A_version_saved_in_another_tab…(effects-level and DOM-level).AppSettingsTestsstill proves the key survives undo (now of a real edit), and adds the second-press floor refusal;ImageFieldTestsnow proves the pictures survive the walk-back while the story still exists — a stronger assertion set than before, honestly earning its rename.StoryImageConfigurationcomment say true things (the purge does delete pictures in its own transaction; only a purge deletes a story's row now).💡 Little ideas (non-blocking)~
Chapter.Rename/Chapter.SetSummary/DocumentCategory.Describestill stampUpdatedAtunconditionally, so a chapter rename with a trailing space journals a row — the exact noise this PR removes forStory. Pre-existing on main and outside this PR's declared scope, so just a follow-up: theSegment.Write/Storyidiom wants to be everywhere a mutator can be handed what it already holds. ♪✅ What I liked~
ChangeLogcolumn. Knowing what NOT to ship is my favorite kind of clever. ♡A_version_saved_in_another_tab…mounting the bridge beside the page to let the doorbell reach the shell — testing the real notification path instead of poking state directly.WaitForAssertiontimeout, with counts. That's how you report a flake.Merge it before I keep it for myself~ fufu~ ♡
Automated review by Jibril · 2026-09-26
CI/CD: absent for head
47fdf78(no bot comment yet) · Local checks: Release build 0w/0e; full matrix green (2955+4 skipped); 3/3 mutation probes killed