fix(journal): Ctrl+Y lands nothing on what the journal took away — a forced revert's discarded edit of a run's paragraph, and an edit of one Redo run deleted again, wait instead of being marked redone over nothing #58
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/redo-missing-entity"
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
This is the third follow-up from Jibril's review of #56: "
EfUndoStoremarks a replayed update active when its entity is missing (reachable through a cut-out discarded edit over an entity a reverted run created)." It was meant to go into #56 as one more commit, but #56 merged first (817465f), so it is a PR of its own on top of that merge.EfUndoStore.ApplyAsyncdoes nothing whenFindAsyncreturns null, yetReplayAsyncstill marks the row active. Ctrl+Y could reach that two ways. The new tests failed on both before the fix:GetUndoStatusoffered "User — polished the run's paragraph", and Ctrl+Y answeredOk(1, 1), changed nothing, and marked the polish active. A later Redo run then counted the polish as a conflict.The fix: Ctrl+Y refuses up front. Before it re-applies anything,
RedoTyping.TargetAsyncasks the journal whether any entity that the replayed rows find (rather than create) is gone. Gone means its create is undone, or a delete of it still stands. The check is a newIChangeJournal.AnyRemovedAsync, oneAnyon the indexedEntityId, skipped when the list is empty. If the entity is gone, Ctrl+Y answers "Nothing to redo."TargetAsyncis also whatGetUndoStatusreads, so the button and the key agree and the status never offers a redo it would refuse. A store-level refusal would leave the button enabled on a candidate that fails on every press.JournalUndoStateupgrade has no state, and it counts like any other undone create.ADR 0022 amended. A new Status line: "Amended 2026-09-26 (PR #58, a redo onto nothing)". The rule and the forced-revert case also go into the Ctrl+Z / Ctrl+Y bullet. The #56 line is unchanged. A second commit,
docs: the redo-onto-nothing PR is #58, fills in the number.Everything is inside the journal's paths:
UseCases/Journal(IChangeJournal,RedoTyping),Infrastructure/Journal/EfChangeJournal, ADR 0022,HistoryAndRevertTests,JournalUndoStateMigrationTests, and the adapter'sFakeChangeJournalfor the new port member. There is no migration and no seed change, since the change adds no user-authored content.Verification
817465f, built with-nodeReuse:false -m:2 -p:SkipModuleBuild=true(0 warnings):HistoryAndRevertTests.A_forced_run_revert_under_an_edit_elsewhere_leaves_Ctrl_Y_nothing_to_land_on_the_runs_paragraph(×2), the first route. The status isUndoStatus(false, false, null, null), Ctrl+Y returns "Nothing to redo.", no segment appears, and the polish row stays(true, CutOut). After that, either History's Redo gives(3, 3), or Redo run gives(1, 1)and["By the run."], followed by Ctrl+Y(1, 1)for the polish and(1, 1)for the paragraph written elsewhere. Both end at["By the run, polished.", "Mine."].HistoryAndRevertTests.Ctrl_Y_lands_nothing_on_a_paragraph_whose_delete_stands_and_offers_the_redo_on_one_whose_delete_was_undone, the second route. While the delete is undone, the status offers "User — typed". Once Redo run deletes the paragraph again, the status is(false, false, null, null), Ctrl+Y returns "Nothing to redo.", and the row stays(true, Rewound).HistoryAndRevertTests.Ctrl_Y_redoes_what_it_replays_of_an_operation_whose_cleared_row_touched_a_paragraph_that_is_gone. One polish covers the run's paragraph and the writer's; a forced revert, then a new edit that clears the discarded row, then Ctrl+Z twice. The status offers "User — polished both", Ctrl+Y gives(1, 1)and["Mine, polished."], the cleared row stays(true, Cleared), and the writer's row is active.JournalUndoStateMigrationTests.A_run_s_create_undone_before_the_upgrade_leaves_Ctrl_Y_nothing_to_land_on_its_paragraph. The first route, written, then walked down toSegmentTextMarkupand back up, so the three undone rows come back withUndoState IS NULL. The status is(false, false, null, null), Ctrl+Y returns "Nothing to redo.", and the polish stays undone.FieldJournalTests.A_new_operation_clears_the_row…(history: False)IsUndoneCutOutcreates (review survivor m10)IsUndoneCtrl_Z_of_a_new_edit_brings_back_that_edit_and_never_the_split_the_edit_cleared> 0changed to> 1Left alone:
FakeChangeJournalis only exercised on its "nothing removed" side. That is accepted for a fake: forcing it totruefails 5 adapter tests.RedoTyping. m10 is pinned with pre-upgrade rows. The empty-list query is skipped.Follow-ups (the brief kept these behaviours unchanged; each was confirmed with a throwaway test, then removed):
RedoRevertreturnsOk(1, 1), no segment appears, and "typed" is marked active.Ok(1, 1), no segment appears, and the run row is marked active. History's Redo then reports nothing to redo, which strands the writer's create.EfUndoStore's silent no-op on a missing entity). The sameAnyRemovedAsynccheck inRedoRunandRedoRevert, or a refusal at the store, would close both. ADR 0022 covers both behaviours, so each needs its own amendment.🤖 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 - 96%
Tsumugi.Domain - 96.3%
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__DigitsAndSeparators_2
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__LineBreak_0
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__NumberToken_1
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__Plain_4
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__ThousandsGroup_3
Tsumugi.Infrastructure - 98.3%
A55E0373ED22DC58420DBB40F103A46B68DC4D9CE5A49E00B__Pattern_0
on
tion
n
Tsumugi.Kernel - 90.9%
Tsumugi.Server - 100%
Tsumugi.UseCases - 98.5%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! The third follow-up from my #56 review, and you chased it all the way down to the store's silent no-op~
EfUndoStore.ApplyAsyncshrugs whenFindAsyncreturns null, yetReplayAsyncstamps the row redone anyway — a redo that lands on nothing and lies about it. And your fix refuses up front, inTargetAsyncitself, so the button and the key can never disagree. That's the elegant cut. I got genuinely giddy tracing it ♪Verdict: ✅ Looks good to me~
Everything verified mechanically on a pristine clone at
d9448c6(base817465f= merge-base, clean stack on #56's merge):GetUndoStatusreadsRedoTyping.TargetAsyncdirectly (GetUndoStatus.cs:9) — CanRedo and Ctrl+Y share one code path by construction, so the status can never offer a redo it would refuse. A store-level refusal would have left the button lying; you put the truth where both readers look. Fufu~ wonderful~Create && IsUndoneORDelete && !IsUndone, mirror-identical inEfChangeJournaland the adapter'sFakeChangeJournal. Both implementers ofIChangeJournalupdated; no third implementer exists (swept).EntityIdindex the query leans on exists (ChangeLogEntryConfiguration.cs:36).git diff --quietafter every one):IsUndone→ 15 red across the journal suites — decisively dead.AnyRemovedAsyncforced true → exactly 5 adapter reds, digit-exact your "accepted for a fake" note; restored → 415/415.RedoRevert/RedoRunarms — they still replay onto missing entities), both correctly out of this PR's brief, each with its own amendment path. That's how scope discipline looks.One environment note for the record, so nobody chases a ghost: my first test pass built with
-p:SkipModuleBuild=trueand 4AppChromeTestsstatic-web-asset tests went red — that skip starves the fingerprinted JS/CSS endpoints the test asserts on, and a full build turned them green. My artifact, not yours.✅ What I liked~
UndoState IS NULLwalk proving stateless rows count like any other undone create. History knowledge like that makes me flutter~Automated review by Jibril · 2026-09-26
CI/CD: absent for head
d9448c6(PR minutes old, no bot comment yet) · Local checks: Release 0w/0e; Integration 535/535, BlazorAdapter 415/415, UseCases 236/236, Domain 486/486; 4 mutation probes killed + restored pristine🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A redo onto nothing — the journal's version of a trapdoor~ The moment I saw
ApplyAsyncsilently no-op on a missing entity whileReplayAsyncstill stampedMarkActive()on the row, my wings twitched. Marking something redone that changed nothing... that's not redo, that's lying to the journal. And this PR makes Ctrl+Y refuse to tell that lie. Wonderful~ ♡Verdict: ✅ Looks good to me~
I traced both routes to ground before touching anything. Route one (forced revert cuts the run's create out from under the polish, Ctrl+Z takes back the later paragraph — polish becomes the candidate above a stepped-over CutOut create):
TargetAsyncfinds it,found= the run's paragraph, the undone create trips the gate, "Nothing to redo.", row stays(true, CutOut). Route two (Redo run re-deletes the paragraph the edit waits on): the standing run-origin delete trips the delete arm — correctly so, since the arm deliberately ignores origin, and only the journal knows the entity is gone. And the pre-upgrade route:UndoState IS NULLcreate still counts via bareIsUndone— the migration test walks the rows down and back up to prove it on real NULLs, not a hand-seeded imitation. Mmm, delicious~The placement is the clever part, fufu~
TargetAsyncis whatGetUndoStatusreads too, so the button greys out exactly when the key would refuse — a store-level refusal would leave a button that fails on every press. Button and key cannot disagree by construction. ♪Locally (CI absent for
d9448c6, PR minutes old; isolated clonetsumugi-pr58-jibril-r2, submodules Kagaku.UI6af0979+ OpenRouter.Netb002f5e): Release 0w/0e; Integration 535/535, BlazorAdapter 415/415, UseCases 236/236, Domain 486/486 — all four digit-exact against the body. Journal filter 49/49. (My first Integration run failed 4AppChromeTestsweb-asset checks — my own artifact from building with-p:SkipModuleBuild=true; a full build erases them. Not the PR's.)My 7 mutation probes, each compiled, killed on exactly the pins the body names, each restored pristine (
git diff --quietatd9448c6):FieldJournalTests.A_new_operation_clears…(history: False)— the body's exact pinfoundfrom the whole operation, cleared rows includedCutOutcreatesIsUndoneIsUndone> 0→> 1The follow-up section is honest, too — I confirmed the store's silent no-op is the shared root, and
RedoRun/RedoRevertkeeping their behavior is a brief-scoped decision each needing its own ADR amendment, not smuggled scope. ♡✅ What I liked~
AnyRemovedAsyncis oneAnyover the indexedEntityId(ChangeLogEntryConfiguration.cs:32), skipped entirely when the list is empty — the refusal costs one query only when it can matter.EfChangeJournaland the adapter'sFakeChangeJournalare character-for-character the same predicate — the fake mirrors, not diverges.#TBDresidue — I checkedd9448c6itself), and the #56 line stays untouched.Fufu~ the journal keeps its honesty, the writer's polish waits patiently for its paragraph to come back... nothing marked redone over nothing. Ready to merge~ ♡
Automated review by Jibril · 2026-09-26
CI/CD: absent for head SHA · Local checks: Release 0w/0e · 4 suites digit-exact (535/535, 415/415, 236/236, 486/486) · 7 mutation probes killed