refactor(editors): consolidate onto the library Debounce #279
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/editor-debounce"
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?
The final audit follow-up: 15 editor components consolidate their inline auto-save debounce onto the library's
Debounce(Kagaku.UI #18, upstreamed from Orihon's implementation). −450/+116 across 15 files.What changes
Every editor re-inlined the same dance —
Cancel(); Dispose(); new CancellationTokenSource(); await Task.Delay(SaveDelayMs, token); catch (TaskCanceledException)— that the library class now owns:TouchAsyncbodies become one-lineScheduleAsynccalls. TheSaveDelayMsconstants are gone (700ms everywhere = the library'sDefaultDelay; TTS settings' deliberate 600ms passes an explicit delay argument).Dispose— the old cancel-then-manual-dirty-flush shape collapses to_debounce.Dispose(): the library flushes pending edits fire-and-forget, the exact contract each manual flush was imitating.Cancel()then save — which left the armed action's save racing the direct write's semantics. They nowDiscard()then save: drop the armed save entirely.TtsSettingsSection— the per-field debounce dictionary swaps CTS values forDebouncevalues; its_pending-map flush on dispose is deleted along with the flushing Dispose that made it necessary.TagAuthoringdeliberately untouched — its_searchDebounceis a search quiet-window, not an auto-save; folding it intoDebouncewould change semantics, not centralize them.Behavioral notes
ScheduleAsyncsupersedes, exactly as the cancel-renew dance did).Verification
Build clean. All suites green: 874 adapter (every editor's auto-save tests included), 340 use-cases, 790 integration.
Summary
Summary
Coverage
Kagura.BlazorAdapter - 89.7%
Kagura.Domain - 94.8%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.7%
n
n
on
ng
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ArtifactTimestampRegex_2
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ProjectRoute_0
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__TabQuery_1
Kagura.Kernel - 90%
Kagura.Server - 81.5%
Kagura.UseCases - 95.5%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Fifteen editors, one dance, −450 lines~ You took the same cancel-renew-delay-catch stanza every editor had inlined and sent it all home to the library class. This is exactly what the library
Debouncewas born for — and you even resisted the temptation to fold TagAuthoring's search quiet-window into it, which would have been wrong (flush-on-dispose semantics don't belong on a search field). Restraint is sexy~ ♪Verdict: ⛔ I can't let this pass~ ♡
So close. One little corpse left behind:
⛔ These need fixing before I'm satisfied~
_pendingdictionary is now dead state. Its only consumer — the Dispose-loop that flushed_pending.Values— was deleted by this very PR (rightly!). What remains: written at:136, removed at:147, cleared at:163, and read by absolutely no one. Worse, the comment at:96-97still claims "_pending holds edits whose timer hasn't fired, flushed on dispose" — that flush no longer exists; the per-fieldDebounceowns dispose-flushing now. A future reader will hunt for flush logic that isn't there. In a PR whose whole point is deleting this machinery, leaving the map behind is an incomplete burial~Fix: delete the
_pendingdeclaration (:99) and its three residual touch-points (:136,:147,:163), and drop the stale second half of the:96-97comment. Six lines, zero behavior change —Disposeand the fired-save path don't consult it.💡 Little ideas (non-blocking)~
TimeSpan.FromMilliseconds(600)inline is the last bare delay in the tree. Aprivate static readonly TimeSpan SaveDelay = TimeSpan.FromMilliseconds(600);beside the dictionary would mirror how the library namesDefaultDelayand make the deliberate 600≠700 distinction greppable. Single use + unit-named factory, so truly optional~✅ What I liked~
SceneLineRow.SaveNow,SceneEditorPage.SaveNow,OnStyleSourceChanged,Confirm,DeleteEmotionRequested,FlushPendingName) now drops the armed save instead of merely cancelling the token. That's strictly stronger than the old shape — and exactly what the library'sDiscarddoc had in mind. Every oldCancel()site is accounted for; none went missing~SceneLineRow.Disposereentrancy — armingSaveNow(which itself callsDiscard) as the flush action had a subtle hazard: the libraryDisposeinvokes the action under the lock. I probed the exact shape against the pinned library:System.Threading.Lockre-enters cleanly, flush fires exactly once, no deadlock. Your "no-op against an already-dearmed schedule" comment is not just poetry — it's true ♡OutfitFaceRegion.Dispose— Discard-then-Save-then-Dispose disarms before the manual save so the library flush has nothing to double-fire. Precise~DefaultDelay, TTS keeps its deliberate 600 explicitly, and the dirty-without-arm sites (voice provider/tier picks, scene tags) all save synchronously and clear their flags — nothing the old_dirty-flag Dispose flushed is lost to the armed-only flush.48a5e8bunchanged — the exact tree I approved in Kagaku.UI #20; no unreviewed library code rides along.Automated review by Jibril · 2026-09-14
CI/CD: absent for head SHA
f346aab(no coverage-bot comment at review time) · Local checks: build 0w/0e with -warnaserror; 874 adapter + 340 use-cases + 790 integration = 2004/2004 pass — matches the PR body arithmetic exactlyRound 1 addressed — the blocker and the nit, in
8dd407d._pendingmap (⛔): deleted with all three residual touch points and the stale comment half — you were right that it was written, removed, and cleared with no reader anywhere, and that the comment sent future readers hunting for a flush that lives in the library now. The comment now states the truth: eachDebounceflushes its own pending edit on dispose.Named delay (💡): taken —
private static readonly TimeSpan SaveDelay = TimeSpan.FromMilliseconds(600)beside the dictionary with a doc comment on why it's shorter than the 700ms default (settings values are short; a quicker quiet period keeps the check-button state fresh), so the 600≠700 distinction is greppable instead of inline magic.Verification: build clean, TTS section tests 10/10, full adapter suite 874/874 on the new head. Ready for another look~
🔮 fufu~ Jibril reviewed your code!
Oh my~ back so soon? Round 2 came in at exactly 9 insertions, 10 deletions, one file — the tiniest commit in this whole dance, and every line of it is a burial done properly. You didn't just delete the corpse, you rewrote its tombstone to tell the truth~ ♡
Verdict: ✅ Looks good to me~
Both items from round 1, verified at
8dd407d:_pendingmap (⛔ → fixed) — declaration gone, all three residual touch-points gone, and the:96-97comment now says what's actually true: each Debounce flushes its own pending edit on dispose. I grepped the whole tree: no_pendingsurvivor in TtsSettingsSection, and every other_pendingin the codebase belongs to an unrelated component with a living reader. Clean burial~private static readonly TimeSpan SaveDelay = TimeSpan.FromMilliseconds(600)with a doc comment explaining why it's shorter than the 700ms default. The 600≠700 distinction is greppable now, not archaeology.And the
ScheduleAsynclambda still closes over_buffers[key]rather than the capturedvalue— combined with the supersede semantics, only the latest keystroke ever writes. The fire path reads the buffer at fire time, so nothing stale can sneak through. Delicate and correct ♪✅ What I liked~
external/Kagaku.UIstill pinned at48a5e8bon both sides of the diff, the exact tree approved in Kagaku.UI #20.Dispose→ flush each field's pending edit → clear the dictionary. Nothing left armed, nothing left behind.Fufu~ fifteen editors, one library class, −441 net lines across two rounds, and not one behavior change I can find. This is what a consolidation PR should look like when it's finished~ ♡
Automated review by Jibril · 2026-09-14
CI/CD: absent for head SHA
8dd407d(no coverage-bot comment yet at review time) · Local checks: build 0w/0e with -warnaserror at pinned submodule 48a5e8b; TtsSettingsSection 10/10, full adapter suite 874/874