test(cg): the autosave rename rides a real debounce — headroom for a starved runner #252
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "test/cg-title-autosave-hardening"
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 push CI of #251's merge (run 926) failed on
Typing_a_new_title_renames_the_variation_through_the_autosave— a flake, not a bug in #251 (which only added a toolbar button and an unrelated test).Why it flaked. The rename wait is the suite's only assertion gated behind the editor's real 700 ms debounce (
CgEditorPage.TouchAsync) — a thread-pool timer. The bunit diagnostics in the failure (three checks against five lifetime renders, then silence until the 5 s timeout) are the signature of a stalled thread pool on the loaded runner, which executes all three test projects concurrently under coverage instrumentation. The save chain itself is deterministic end to end (all fakes synchronous, single renderer context); locally the test passed ~50 attempts including full-suite runs pinned to one core, so only the timer-riding waits need headroom.What changed.
await Task.Delay(900)passes vacuously when a starved runner hasn't fired the debounce yet. It now waits for the variation'sUpdatedAtto move past the rename — the blank save provably landed — before asserting it kept the name. Side effect: the test returns as soon as the save lands instead of always sleeping 900 ms.Verified: the hardened test 5× green, the full
CgEditorPageTestsclass green pinned to a single core.(Run 926 itself just needs a re-run from the web UI — the API endpoint for reruns 404s on this Forgejo version.)
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.8%
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 - 82%
Kagura.UseCases - 95.7%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A flake autopsy! You dug into bunit diagnostics, spotted the thread-pool starvation signature, and turned a vacuous
Task.Delay(900)into a provable landing condition — theUpdatedAtgate is genuinely elegant. I got so excited I mutation-tested it myself~ ♡Verdict: ⛔ I can't let this pass~ ♡
The changed lines are correct — I verified every hop of the save chain. But the PR's premise is wrong, and the premise is what decided its scope.
⛔ These need fixing before I'm satisfied~
[CgEditorPageTests.cs — the "only assertion" premise is false; the hardening is incomplete] — The PR body claims the rename wait is "the suite's only assertion gated behind the editor's real 700 ms debounce (
CgEditorPage.TouchAsync)." I traced every edit path in this class, and that is simply not true.TagAuthoring.EmitfiresTagsChanged/DisabledInheritedChangedimmediately (TagAuthoring.razor:252, :355 — its 250 ms timer is search-only), which lands inOnTagsChanged/OnDisabledInheritedChanged/OnSceneTagsChanged(CgEditorPage.razor:610–644) → the sameTouchAsync()→ the same 700 msTask.Delaythread-pool timer →Save(). So these siblings assert persisted store state behind the identical timer, each with a 5 s window:Adding_an_own_tag_auto_saves_it_without_a_save_button(:106 —AddTagByHand("sunset")→ store assert, 5 s)The_sets_scene_tags_appear_inherited_and_unticking_one_persists_the_disable(:125 — untick → store assert, 5 s)The_establishing_editor_owns_the_scene_tags_every_variation_inherits(:558 —AddTagByHand("rooftop")→CgSetStoreassert, 5 s)A_saved_edit_keeps_the_base_entry_workable_on_a_render_less_sibling(:643 —AddTagByHand("blush")→ store assert, 5 s)A_member_card_shows_the_inherited_chain_and_unticking_persists_the_disable(:331) ridesCgCastMemberCard's ownSaveDelayMs = 700debounce (CgCastMemberCard.razor:108) — same timer class, same 5 s window.By the PR's own diagnosis — a starved runner stalls the debounce timer well past the window — every one of these can flake exactly like run 926 did. The rename test was merely the one that drew the short straw this time; nothing about the mechanism discriminates. Fixing one instance and declaring the class handled leaves the failure armed~ fufu~ you wouldn't leave a loaded flake in CI, would you? ♡
Fix: extend the same 15 s headroom to all the debounce-riding waits above (a shared
static readonly TimeSpan DebounceRideWait = TimeSpan.FromSeconds(15);— or a smallWaitForDebouncedSavehelper — would keep the file from sprouting a third kind of magic window; right now the file mixes 5 s and 15 s waits on the same timer, which is its own little trap for the next reader).💡 Little ideas (non-blocking)~
✅ What I liked~
UpdatedAtgate is masterful. I verifiedCgVariation.ApplybumpsUpdatedAtunconditionally while refusing the blank title (CgVariation.cs:124–148), so the wait can neither dead-end nor pass vacuously — and I mutation-tested it: killing the blank-refusal guard makes the test go red precisely at the title assertion. Directional, not a tautology~ ♪>(not>=) is safe: the two saves are ≥700 ms apart on the real clock, so timestamp collision is impossible — you clearly thought about why it can't tie.Verification: hardened test 5/5 green locally, full
CgEditorPageTestsclass 39/39 green pinned to a single core (taskset -c 0), mutation probe red as designed, submodule pin3dc59a0(Kagaku.UI) initializes clean — my first build error was my own uninitialized submodule, not your tree.Automated review by Jibril · 2026-08-15
CI/CD: absent for head SHA
f90ed23(PR just opened, no coverage bot yet) · Local checks: build 0 errors, 5/5 + 39/39 single-core green, mutation-verifiedRound 1 addressed in
c6f4cae.⛔1 — accepted, premise corrected. You're right, and I verified each claim before folding:
TagAuthoring's only timer is the 250 ms search debounce — tag adds/disables emit immediately and land in the sameTouchAsync700 ms timer, andCgCastMemberCardcarries its ownSaveDelayMs = 700for tag edits. So all five sibling waits you listed were armed with the exact failure mode this PR diagnoses. All seven debounce-riding waits (your five + the two from round 0) now share oneDebouncedSaveWait = 15 s, documented once at the declaration — which also absorbs 💡1, so the file no longer mixes two magic windows on the same timer.Deliberately left at 5 s: the cast card's discrete controls (outfit pick, enabled/mute/likeness toggles, sliders — "the discrete controls save at once; only typing debounces"), the scope panel (per-gesture saves, no debounce, ADR 0051), the cast add select (immediate dispatch), and the "Saved" badge wait at :127 — it can only run after the store wait above it passed, i.e. after the save already landed.
Follow-up worth noting, out of scope here: the other editor-page suites (Character, Lore, Location, Chapter, Scene, CgSet…) have the same 700 ms
SaveDelayMsand presumably the same 5 s windows on their store asserts. If the runner starves again on one of those, the same constant wants to move toAdapterTestContextand spread — I'd rather do that migration when a second class actually needs it than churn ~10 files on one data point.Verification: full
CgEditorPageTestsclass 39/39 green pinned to a single core after the change.🔍 fufu~ Jibril reviewed your code!
Oh? Oh~! This is how you answer a blocker, scarlet. You didn't just widen the waits — you re-traced my premise yourself, corrected it in the reply, mapped every exemption to source, and then resisted the temptation to churn ten files on one data point. I checked every single claim. Every one held. I'm genuinely delighted~ ♡
Verdict: ✅ Looks good to me~
✅ Blocker ⛔1 — closed, and then some
Every debounce-riding wait in this class now shares
DebouncedSaveWait = 15 s, declared once with an XML doc that explains why (thread-pool timer, starved runner, triple headroom) — which also swallowed non-blocker 💡1, so the file no longer mixes two magic windows on the same timer. I verified all seven sites atc6f4cae: your five siblings (:128 own-tag, :149 untick, :356 cast-card tags, :580 establishing scene-tags, :674 base-entry) plus the original two (:1122 rename, :1133 blank-save). And the now-false "unlike the suite's other waits" inline comment is gone — the documentation lives once, at the constant. Clean~✅ The exemptions you left at 5 s — I traced every one to source
OnOutfitPicked/OnEnabledChanged/OnLikenessChanged/OnStrengthChanged/OnFidelityChangedall callSave(...)directly (CgCastMemberCard.razor:172–193), immediate dispatch, no timer. The "discrete controls save at once; only typing debounces" comment is accurate. The mute-diverges test (:551) rides this immediate path too — 5 s is honest there.SaveScope()→Dispatcher.Dispatch(new SaveCgRetouchScopeRequested(...)), with the in-source "One save per gesture… ADR 0051, no debounce" note (:804–810). Correct.Add()dispatchesAddCgCastMemberRequestedimmediately (CgCastPanel.razor:64–69). Correct.CgVariationSaved; nothing timer-gated remains between. Correct.💡 Little ideas (non-blocking)~
AdapterTestContextdeferral — agreed, for the record.ProjectWorkspacePage,SceneLineRow,ChapterEditorPage,SceneEditorPageall carry the same 700 msSaveDelayMsand presumably 5 s store-waits. Migrating the constant when a second class actually flakes is the right call — one data point shouldn't churn ~10 files. When that day comes,DebouncedSaveWaitwants to live in the shared context. ♪✅ What I liked~
CgCastMemberCard's ownSaveDelayMs = 700bonus exposure I'd flagged, now covered at :356.Verified locally: full
CgEditorPageTests39/39 green pinned to a single core (taskset -c 0, 1 m 29 s) — matching your claim. The premise correction in the thread stands as the record; nothing in the code still encodes the old "only assertion" story.Automated review by Jibril · 2026-08-15
CI/CD: absent for head
c6f4cae(coverage bot 6242 coversf90ed23only) · Local checks: CgEditorPageTests 39/39 pass, single-core