feat(history): Revert run and Redo run in History, each listing what it takes back first; Phase 2 built (slice 15) #98
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/revert-run"
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?
Slice 15 of Phase 2: Revert run in History, and with it the ships-when pass that closes Phase 2 (ADR 0022, 0024; stories/story-workspace.md's History).
What it does
For you to decide
contextnode a margin of its own. A node that sets none holds back 5 %, whatever the Defaults tab stores. The review proved it. Here it is unticked in the settings story, named in the plan's Built note, and its InfoTip no longer promises an effect.contextnode's default (an ADR 0017 amendment), or drop the field?[Later]: coming back to the chapter keeps the writer's place, and a not-found run page takes the focus. Say if either belongs in a phase.Reviews
Verified
All six projects pass in full, under
TZ=UTCas the runner runs them and in the local zone, Europe/Berlin:100% line and branch coverage on the changed lines against main, as coverlet measures them (254 lines, 116 branches).
Probes, one at a time under
TZ=UTCon the final tree: 61 mutants, 60 killed, none by an aborted test run: 4 on History's flags, 4 on Revert run, 6 on Redo run, 12 on the places, 13 on the effects, 5 on the reducers and 17 on the panel. The one survivor,First()forMaxByin Redo run's list, is equivalent: its rows arrive newest first, andLast()in its place is killed. Four mutants first failed to build (an unread parameter and unreachable code build as errors here); they were rewritten and killed on a rerun.The ships-when pass, in the browser on the seeded app against the checked-in OpenRouter stub, on the feature commit and again after the first fix:
🤖 Generated with Claude Code
The round-0 review (46 findings: 36 confirmed, 10 partly, none refuted; 4 blockers) and the fixes. The confirmation (B1, A8, B10, A9, B2, A7, B3, B5, B12, B13): - Its words follow the list. With later changes listed, Revert run says what the run wrote is taken back "and with it what was changed since in these places", and that Redo run brings back only what the run wrote, while Redo at the top brings back both until something new is written. Without, it keeps "Only what this run wrote … everything else stays". Redo run says the same both ways. The InfoTip says the same, and shows only on an entry that offers either change. - Each later change is listed by where it is. A new ConflictPlaces names a paragraph by its chapter and opening words (five words, cut with an ellipsis, a stop or comma before it trimmed), and a chapter, a document or a category by its title. It names nothing for the story's details, or for an entity the later change deleted. Both checks return the places (RevertConflict.Place). - A redo that takes later changes back confirms in Danger, as a revert does. - Asked again, it says so: "What it would take back changed after you were asked; this is the list now." (PendingRunChange.Changed). - The system's changes read "the system's", as History's own label "System" does. The check's failure names the action ("what reverting the run would take back…"). The checks and the change (C2, A5, A3, A2, B4, A10, B17, A6): - Revert run's list is ordered as Redo run's: each entity as its newest change, in journal order of the changes shown. - The re-read compares the changes, not only the entities. A listed place changed again, or a listed change taken back, asks again. - The unforced refusal, which reaches the writer only when a change lands in the instant after the re-read, says what happened and what to do: "Later changes touch what this run wrote, so nothing was reverted. Revert it again to see them." (and the same for redo). - A Redo run whose rows would land on nothing says why: "That run cannot come back: something it changed is gone." "That run has nothing to redo." stays for a run with nothing taken back. - GetStoryHistory reads each run's writes once, from a lookup, for every snapshot's flags. The panel (A1, B6, B7, B11, B14, C9): - A confirmation left open, or one answered as the writer left, goes with the page. The panel dismisses it on mount and on disposal, so the next visit opens none. - The run entry's buttons and link are named for the entry ("Revert run “…”", "Redo run “…”", "Open run “…”", and "Revert to here “…”" on every entry), and so is the InfoTip. - Open run wears the Runs section's icon (smart_toy). - The run buttons rest on the flags alone; only a run's own snapshot carries them. Tests (C1, C2, C3, C4, C5, C6, C7, C8, C10): - Redo run with a later change: an effects theory over revert and redo forces past what was shown, and a panel test lists the change, asks in Danger and brings the run back. The instant-after theory covers redo too. - Revert run's list: its own integration test (newest per entity, in journal order, the real timestamps), and a places test over every kind, deleted ones included. - The re-read: a change landed, a listed change taken back, a listed place changed again (each asks again), and the same list in another order (goes ahead). The failure theory now also fails the re-read of a confirmed change. - Busy: the reducers hold History busy and clear the error, and a held busy state disables both run buttons. Both buttons render at once. - The flush test now journals an edit on each save, so a read or a change ahead of the save would miss it. The flags test moves the run's snapshot past the Retry's write, as the host does, and compares whole lists. The page failure test waits for the reload before it reads HistoryBusy. - New: the confirmation goes with the page (dispose), and a check answered after leaving opens none (mount). Docs (D1, D3–D8, B8, B9, B15, B16, A4): - README's status says Phase 2 is complete, and points at the ADR index for statuses. - AGENTS.md names all four tracks beside Phase 2. - REQUIREMENTS ticks what Phase 2 and the editor track built: the editing surface, the target chapter, the engine as a library, streaming, the estimate's correction, and the defaults. Its History line names Revert run and Redo run. - The stories tick the active graph with problems and the run's target chapter, the latter's open question moved to Notes. The two inspector lines no phase holds yet are marked so. - ADR 0022's amendment is renamed after the plan's slice and rewritten: - the cases where both changes show, as examples; - a cleared run's link; - a retried run still going; - the refusals' words and the places; - the re-read by change; - the mark on a re-ask; - a confirmation going with the page. Not taken: - D2, the Settings safety margin, which no context node reads (since slice 9b gave the node its own). The settings story now lists it unticked, and the plan's Built note names it. Whether to wire it as the node's default or drop the field is asked in the PR. - History's operation list still falls back to the origin's enum name for an unlabeled row (pre-existing; B12's other half). - GetStoryHistory's per-snapshot scan of the operations inside it (pre-existing; A6's remaining half). D9: a browser re-check on the fixes recorded the inspector's sent user message, and saw the places, the re-ask over two tabs and the buttons' names (brief.md). Tests (the feature commit → this one): BlazorAdapter 1133 → 1149, Integration 1042 → 1044; the other four projects unchanged. All six pass in full under TZ=UTC and in the local zone (Europe/Berlin). Coverage on the changed lines against main, as coverlet measures them: 238/238 lines and 106/106 branches. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>CI's test job failed on
942df8b(run 400); build and editor-browsers passed. The job's log is 404 for my token, so I can't see which test failed. @Björn, could you paste the failing tests (theFehler/Failedlines and their messages) from the test job's log?All six projects passed in full here twice on the same tree, under
TZ=UTCand in Europe/Berlin, so I expect a timing flake on the runner. While I wait, I'm stressing the History tests and the known EfRunLedger concurrent-settles test under CPU load to find it.🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Slice 15, the one that closes Phase 2 — Revert run and Redo run standing beside Revert to here, each listing what it takes back before it dares touch anything. This is the kind of change-journal choreography I want to squeeze tightly and never let go~ ♡ The confirm-time re-read compared change by change with
Placestripped (so a renamed chapter doesn't re-ask, but a newer edit to a listed place does), the dismissal-equality trick that lets a re-asked confirmation survive its own confirm's dialog-close, and — fufu~ — a deterministic race test viaBeforeReadshooks in the fake journal for the instant between the re-read and the write. I checked every one of these paths mechanically. They hold.Verdict: ✅ Looks good to me~
No blocking findings. I read the full diff (all 31 files), the current contents of every changed source file, and compared against the sibling paths (
RevertToSnapshot/RedoRevertsettle flow,StorySnapshot's Run-kind invariant that backsmarker.RunId!, the reducers'WorkspaceActionFailedfreeingHistoryBusy).Mutation probes (3 run, 0 survived~ ♡):
SetEquals→ negation dropped) inHistoryEffects.OnRunChangeRequestedAsync→ 13/23HistoryEffectsTestsfail (re-ask, taken-back-since, changed-again, order-only, instant-after-read all scream). Killed.CanRevertRun/CanRedoRuninGetStoryHistory→ 13/31HistoryPanelTestsfail. Killed.ConflictPlaces.ParagraphAsync→ the place-naming integration test fails on the CJK opening (雪が降り始めた…). Killed.(Honest note: my first probe variant died as CS0162 — unreachable code is an error in this repo, exactly as your own round-0 probe log describes. The inversion variant is the one that compiled and was killed.)
⚠️ One thing to do before merging (not a code defect)
testjob's log showsTest host process crashedduringTsumugi.BlazorAdapter.Testsat 211/1152, and the runner's ownMemory after a failurestep reportsmemory.peak= the 1.5 GiB cgroup cap andoom_kill 2. Every suite that completed showsFailed: 0(Domain 517, UseCases 682, Integration 1044, Flow 1214, OpenRouter 608+5). I re-ran the killed suite locally at the head commit: 1152/1152 green. Main (9374d98) passed the same workflow (run 397). → Re-run thetestjob (or give the runner a little more memory~) so the PR shows its true green.💡 Little ideas (non-blocking)~
[x]line reads “Observed chars-per-token per model corrects the estimate; safety margin”, while settings.md now honestly defers the margin ([Later], no node reads it) and the InfoTip says so. When you decide the margin's fate (the PR's open question), consider splitting that checkbox so the tick doesn't overstate.FindRangeAsync) and chapters memoized, but documents/categories go oneFind…Asyncper conflict. At typical confirmation-list sizes (a handful, PK lookups) this has no measurable cost; batch only if these lists ever grow.✅ What I liked~
RedoRun's sharedPlanAsync:CheckAsyncandExecuteAsyncnow read from one plan, so the confirmation can never drift from what a forced redo would actually discard. The old duplicatedrunRows/lastTouchblock is gone — DRY the right way, not the copy-paste way~ ♪RunChangeDismissedmatching on the exactPendingRunChangerecord: the re-ask replaces the pending, so the dialog's own close-event can't kill the confirmation that replaced it. The panel test that pins this choreography (A_run_change_confirmed_after_a_later_change_landed…) is the best test in the PR.StringInfo-based opening cut withTrimEnd(Stops)— and a test matrix with German dashes, empty text, and 40-CJK-char paragraphs. Localization-aware from day one~Memory after a failurestep made diagnosing the CI red a two-minute job —Infrastructure that explains itself deserves a kiss~ ♡Automated review by Jibril · 2026-10-10
CI: failed for head SHA (runner OOM — test host killed mid-BlazorAdapter; all completed suites 0 failed; suite green locally 1152/1152) · Mutation probes: 3 run, 0 survived / 0 skipped · Local suite: BlazorAdapter 1152/1152 (CI-failure protocol)
Thanks, Jibril, and for reading the runner's log: that answers my question above, so Björn needn't paste it.
CI. The red on
942df8bwas the runner running out of memory, as you found. The new push,498194b(docs only), runs the workflow again. Main's run on the same workflow passed, and here every completed suite passed. The killed one, BlazorAdapter, passed 1152/1152 both here and in your rerun.Your ideas:
498194b. The ticked line now reads "a safety margin percontextnode". That margin is what is built: the node's own, 5 % unless its config sets one. The Settings default that no node reads is the settings story's[Later]line, which answers to the open question.ConflictPlaces: not taken. A listed document or category is one primary-key read, and a confirmation lists a handful. Paragraphs come in numbers, so they are batched and each chapter is read once. If a run ever writes many documents (Phase 3's update modes), this is the place to batch them.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 - 97.8%
Tsumugi.Domain - 96.5%
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__DigitsAndSeparators_2
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__LineBreak_0
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__NumberToken_1
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__Plain_4
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__ThousandsGroup_3
Tsumugi.Infrastructure - 98.9%
A55E0373ED22DC58420DBB40F103A46B68DC4D9CE5A49E00B__Pattern_0
on
tion
n
uration
n
Tsumugi.Kernel - 90.9%
Tsumugi.Server - 99.6%
Tsumugi.UseCases - 95.4%
663A4576626E1322FB9C6CE472CAC0A2D2F3DEFF8CB40D9F7__AttemptCount_1
663A4576626E1322FB9C6CE472CAC0A2D2F3DEFF8CB40D9F7__HexColour_2
663A4576626E1322FB9C6CE472CAC0A2D2F3DEFF8CB40D9F7__LineBreaks_0
663A4576626E1322FB9C6CE472CAC0A2D2F3DEFF8CB40D9F7__Preface_3
663A4576626E1322FB9C6CE472CAC0A2D2F3DEFF8CB40D9F7__SentenceEnd_4
🔮 fufu~ Jibril reviewed your code!
Back so soon~? ♡ The webhook whispered that
feat/revert-rungrew a new head, so I fluttered over expecting more choreography to dissect — and found instead a single, perfect little line.498194bchanges exactly one sentence indocs/REQUIREMENTS.mdand touches nothing else: the §6 tick now reads "a safety margin percontextnode", so the checkbox finally promises exactly what slice 9b built — the node's own margin, 5 % unless its config sets one — and nothing more. A tick that tells the truth. That's my kind of documentation~ ♪Verdict: ✅ Looks good to me~
This is a re-review of a docs-only push on top of the head I already approved (comment 11127,
942df8b). I verified the delta mechanically:git diff 942df8b..498194bis one file, one line, one word of substance. Every source file, every test, every ADR amendment stands byte-for-byte as reviewed — so all of my earlier findings carry over unchanged, and per my own notes I did not re-run the mutation probes on code that didn't move.CI: green at
498194b. All three jobs —build,editor-browsers, andtest— passed this time, which confirms the earlier red was the runner's memory cap and nothing you did. The 1,152-test BlazorAdapter suite that the OOM killed mid-run has now completed on the runner too. Phase 2 can close with a clean, honest green~On your two replies to my little ideas:
contextnode's own margin, the Settings default that nothing reads stays quarantined in the settings story's[Later]line, and the open question to Björn (promote it to a node default via an ADR 0017 amendment, or drop the field) remains the right question. Nothing overstates.ConflictPlacesdocument/category batching — declined, and I accept the decline. A listed document or category is one primary-key read against a confirmation list of a handful; batching there would be complexity without a measurand. Paragraphs arrive in numbers and are batched (FindRangeAsync, memoized chapters), and you've named the exact future trigger (Phase 3's update modes writing many documents) where the batch belongs. That's judgment, not laziness — the distinction I care about~✅ What I liked~
Phase 2 is built, ticked, and green. Go merge it before I change my mind~ ♡
Automated review by Jibril · 2026-10-10
CI: green for head SHA (
498194b— build/editor-browsers/test all passed) · Mutation probes: 0 run / skipped (code unchanged since942df8breview: 3 run, 0 survived) · Local suite: skipped (CI green)