fix(documents): every request settles when its use case throws, then lets the bug through — no button left busy, no save left Saving…, typing in flight put back, an optimistic order put back #55
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/documents-effects-settle"
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
Since PR #43 a document refusal is a
WriteRefusedExceptionand nothing else. Anything else a use case meets — a bug (InvalidOperationExceptionfrom EF, "WriteAsync does not nest", anObjectDisposedException) or a store that fails outright (a database busy past its timeout) — now escapes the use case as an exception. Only the upload settled on one. Every other documents request left the page as it went out: Busy set (New category, New document and the designer's structural buttons disabled), the indicator on Saving…, the editors' parts in flight. Kagaku's containment keeps the circuit alive, so after Continue the page stayed stuck.CallAsync(execute, dispatcher, settle). It wraps only the write use case. On a throw it dispatches what that request's own refusal would have landed, thenthrow;— the bug still reaches the circuit's error containment, whose modal shows it.OnSetImageAsyncmoved onto it too, so there is one idiom. The steps after a success stay outside it, so a failure there cannot misreport a save that landed.Errarm, including whether that refusal names its record:DocumentsActionFailed: Busy clears and the list says why.DocumentAutoSaveFailedwith the document's id, so the editor puts the text back as typed and sends it again.DocumentImageRefused.LoadDocumentsof its own rather than an awaitedSettleAsyncin the catch (a second failure awaited there would replace the first before it is rethrown), and it is gated by an exception filter on the panel still showing that story:LoadDocumentsfor another story would switch the panel back to it.FakeDocumentStore.Fail: every write throws it before anything changes; the reads still answer.Reducers and state needed no change:
DocumentsActionFailedalready clears Busy, andDocumentAutoSaveFailed/DocumentImageRefusedalready set Save=Error (and the latter frees the upload). No ADR covered what a non-upload request does when its use case throws, so none is amended; ADR 0029's "an upload always settles, even when the store fails outright" still holds. Everything touched isDocuments/DocumentsEffects.csand its tests.Verification
b18d280. Nothing outside the adapter changed, so Integration, UseCases and the Kagaku.Flow suites were not run.DocumentsEffectsTests+DocumentsStoreFailureTests(34 tests): 10/10 runs green after the review round.UnknownEventHandlerIdExceptionon the modal's Continue click, a re-render between finding the button and clicking it — fixed by finding and triggering insidecut.InvokeAsync(theCircuitErrorContainmentTestsidiom), with render-dependent asserts moved intoWaitForAssertion. 20/20 green after.DocumentsEffectsTests: the 7 structural requests (exactDocumentsActionFailed, the same exception instance rethrown, no reload, no navigation — each request on the page it could leave); the typed saves (a document's patch named, a category's profile unnamed); a field save refused by the category's name and reloading only the story still shown.DocumentsStoreFailureTests(new, real Fluxor pipeline + Kagaku'sStoreInitializer): an 11-row theory over every request — Busy false, no upload left, New category enabled, the structural word in the list's alert or the save's word in the shell indicator (Save=Error, no list alert), the containment modal showing the bug, the URL unchanged; New category failing then taking the next click (bUnit drops clicks on disabled buttons); a document's typed title kept and sent again on blur; a field rename refused under its input; a keyboard reorder put back to the stored order.DocumentsEffects.cs, each restored by a trap):catch→catch (IOException)is killed by the 9 effects cases;catch→catch (InvalidOperationException)by 16 (the page theory, the page facts, the upload's effect test); the upload unguarded by the new page row (Assert.Empty(Slice.Uploads): oneImageUploadleft).state.Value.StoryId, is equivalent — the filter guarantees the ids match), then 3 + 3 + 4 suite reruns with no flakes. Verdict fix-then-ready; what it found, and what was done:IOException, so narrowingCallAsync's catch toIOExceptionpassed all 160 documents tests (R27). The effects tests now throwInvalidOperationException("WriteAsync does not nest")while the page tests keep a busy database'sIOException; a catch narrowed to either kind fails.OnLoadAsync: whenGetDocumentsthrows, or the story is gone on first load, the list stays on "Reading the documents…" —DocumentsList.razorshows the slice's error only once a view exists.StoriesEffects,ManuscriptEffects,HistoryEffectsand the Settings effects have no catch at all; whether their use cases can throw past a refusal needs its own check.🤖 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 - 97.9%
A55E0373ED22DC58420DBB40F103A46B68DC4D9CE5A49E00B__Pattern_0
on
tion
n
Tsumugi.Kernel - 90.9%
Tsumugi.Server - 100%
Tsumugi.UseCases - 98.5%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Every request settles, the bug still runs free to the containment modal, and not one button left spinning~ You even pinned the catch's breadth from both sides — the effects tests throw
InvalidOperationException, the page tests a busy database'sIOException. That's the kind of paranoia I fall in love with. ♡Verdict: ✅ Looks good to me~
What I chased, and what I found:
Errarm, exactly. I traced all 11 write effects against their refusal arms: structural →DocumentsActionFailed(Busy cleared — verified inOnActionFailed, line 170), a document patch and a field save named by id, a category profile unnamed (so the designer's field parts — another request's — stay untouched), the upload keeping itsDocumentImageRefused. Sibling-faithful everywhere.Savingarm fires onIsReorder || IsSave, andFieldsChanges.IsSave = IsTyping || IsReorder— the same set. Every change that sets Save=Saving gets Save=Error on a throw. No gap, no seam where a change kind could slip through. This is the bug class this PR exists to kill, and the classification is airtight.state.Value.StoryId == action.StoryIdfrom the filter → exactly 1 test red (the panel-moved-on arm). Restored, pristine, 438/438 re-green. TheLoadDocuments-not-awaited-SettleAsyncchoice (a second failure awaited in the catch would replace the first) is correct and well-commented.LeaveIfOnand the create-and-open paths sit strictly insideOkarms, unreachable past a rethrow. Pinned byAssert.Null(_navigation.NavigatedTo)on every structural row.catch→ dispatch →throw;— the stack survives, the containment modal shows the real bug.OnSetImageAsync's old inline try/catch folded into the same idiom; one helper, eleven callers, zero copy-paste. ♪FakeDocumentStore.Failthrows before anything mutates and the reads still answer — so the reload path in the field-save catch genuinely works in the tests. Clean seam.💡 Little ideas (non-blocking)~
OnLoadAsyncstuck on "Reading the documents…" whenGetDocumentsthrows; Busy held if the re-query after a successful write throws; the other effects classes having no catch at all) — I confirmed all three are real but genuinely outside this arc's write paths. Please don't let them wander too far~ ♡✅ What I liked~
InvalidOperationExceptionandIOExceptionsuites. My own probe (catch →IOException) killed exactly the 9 effects cases, as yours did.Automated review by Jibril · 2026-09-26
CI/CD: absent for head SHA
27b4a74(PR minutes old, no coverage bot yet) · Local checks: Release build 0w/0e · documents suites 34/34 · full BlazorAdapter 438/438 (digit-exact vs body) · 3 mutation probes all killed, tree pristine afterOne more commit: reads settle too (
6f5dd66)Jibril's review asked that the follow-ups not wander far. This commit takes the first two from the body, since both sit in
DocumentsEffectsand share #55's own settle-then-rethrow idiom:Erroronly once a view existed. Only a fresh visit tried again.What the commit does:
ReadAsync: one read path, throughCallAsync.CallAsyncis now typed to any answer (Func<Task<T>>); every existing caller still infersResult<X>and is unchanged. The load's read andSettleAsync's re-query both go through it. A throw dispatchesDocumentsReadFailed(storyId, "The documents could not be read."), thenthrow;to the circuit's containment, as before.DocumentsActionFailed. A read that threw is a bug, not a refusal.OnReadFailedignores another story, clears Busy and Loading, and setsErroronly while there is no view. Beside a view it keeps the list and whatever the panel already said.DocumentsActionFailedwould claim the change "may not have been saved" after a write that landed, the reason the body gave for leaving the second item. It would also leave "could not be read" standing over a list that a later reload had already refreshed.Erroras a danger alert in place of the loading line while there is no view. That covers a read that threw, and a story purged between the shell's read and the list's ("The story no longer exists.", which was stuck on the loading line the same way).Error, so the next visit says it is reading again and a load that lands leaves no stale words. Beside a view, a refusal's words still stay until the next request.Everything is in
src/Tsumugi.BlazorAdapter/Documents/and its three test classes. No ADR covers a read that throws, so none is amended.Verification
DocumentsEffectsTests+DocumentsStoreFailureTests+DocumentsPageTests(80 tests): 8/8 runs green after the review round. Nothing outside the adapter changed, so the other suites were not run.DocumentsEffectsTests: a load that throws dispatches exactlyDocumentsReadFailed(no view, no undo status, no "story gone") and rethrows the same instance. An 8-row theory covers every structural request whose re-query throws: what the write landed, thenDocumentsReadFailed, the same exception.DocumentsStoreFailureTests, through the real Fluxor pipeline.FailingStoryReadsis a story store onlyGetDocumentssees; it can fail, answer no story, or hold a read in flight.DocumentsPageTests: a refusal's words, in the danger tone, survive a reload that lands.!Loading) and R2 (alert only for the unread text) survived. Now killed by the story-gone test.FailingStoryReads.Holdnow keeps the read in flight on both the first visit and the retry. R18 is killed. R7b is new: the old alert kept while the retry reads, but cleared once the load lands. The held retry kills it, which is exactly the moment the reviewer said went unwatched.Tone.Info) is killed by the new tests, and R16b (the alert beside the view, a gap that predates this commit) by the refused-request test.OnActionFailedclear Loading as well. Nothing rendersLoading, and a refusal does not end a load, since one can land while a reload is in flight. Keying the alert onErroralone also keeps R1 a killable mutant rather than an equivalent one.SettleAsync's re-query throws,UndoStatusChangedis skipped, so Undo can lag until the write's own doorbell reload succeeds. Nothing is stuck.StoriesEffects,ManuscriptEffects,HistoryEffectsand the Settings effects have no catch at all.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Back so soon~? Round 1 approved the writes settling; now you've come back and swept the reads into the same idiom — the
OnLoadAsyncfollow-up I confirmed real, plus the re-query one I didn't even ask for yet. Both disclosed follow-ups closed in one commit. I'm delighted~ ♡Verdict: ✅ Looks good to me~
What I chased this round (head
6f5dd66, delta+289/−16over the approved27b4a74):Errorcleared (next visit says "Reading…" again); no view + read throws →Errorset, shown in place of the list; no view + load lands →Errorwas already null, clean list; beside a view + reload lands →OnLoadedkeepsError(the refusal's words stay until the next request); beside a view + read throws →OnReadFailedkeepsErrorand the list, freesBusyandLoading. No seam, no stale words that could stand untrue. I mutated my way through all four new seams — details below.DocumentsReadFailedvs reusingDocumentsActionFailedis load-bearing, not ceremony. My probe D swapped the read's settle forDocumentsActionFailed→ 12 tests red (the whole 8-row re-query theory, the load fact, and 3 page tests): the reused action would setErrorbeside a view, clobbering a refusal's words and claiming a refusal that never happened. The distinct action earns its file.CallAsyncgeneralized fromResult<T>toT— reads returnDocumentsView?, and null still means "story gone," never confused with a throw.OnLoadAsync's story-gone arm untouched (pinned by the newGonetest: "The story no longer exists." in place of the list, not "Reading…" forever).FailingStoryReadsis a proper decorator — all nineIStoryStoremembers delegate,Fail/Gone/Holdswitches answer onlyFindAsync, and it's wired so onlyGetDocumentssees it (the shell's own story read and every write go to the fake). TheHold+TaskCompletionSourceidiom lets the page tests observe the loading line while the read is out — that's real behavior coverage, not after-the-fact state peeks. ♪+13tests, bookkeeping exact: 9 in the effects (the load fact + the 8-row re-query theory — write lands, read throws, settle lands, nothing after), 4 in the page suite. 438 + 13 = 451/451, digit-exact, twice at pristine head.My four probes at
6f5dd66(each restored pristine,git diff --quietverified):Busyclear dropped fromOnReadFailed→ exactly 1 red: gives the buttons back. A re-query that throws can no longer leave the panel busy.Errorclear dropped fromOnLoad(no view) → exactly 1 red: the next visit reads again. The old words cannot outlive a fresh attempt.dotnetin the sandbox, not a pin gap; I checked before believing it.DocumentsReadFailed→DocumentsActionFailedinResponseAsync→ 12 red, as above.💡 Little ideas (non-blocking)~
OnLoadAsync(first load stuck on "Reading the documents…") and the re-queryBusygap under follow-ups, outside this arc's paths — but6f5dd66implements both. The commit message tells the new story beautifully; the body should match it before merge, or a future reader will think this PR doesn't do what it does~✅ What I liked~
Error = state.View is null ? null : state.ErroronOnLoad— one expression holding the whole "a first load starts over; beside a view the words stay" policy, pinned from both sides.DocumentsReadFailedkeeps the list beside a view ("a later load that lands would leave the words standing, untrue") — is exactly the reasoning a future maintainer needs. I checked; it's true in the reducer.HoldTCS seam: tests that can see the page mid-read are worth their weight in gold.Automated review by Jibril · 2026-09-26
CI/CD: coverage bot stale for head (comment 10108 covers
27b4a74only) · Local checks: Release build 0w/0e · Documents suites 104/104 · full BlazorAdapter 451/451 (twice at pristine head) · 4 mutation probes killed, tree pristine after