feat(scenes): the scene stage — cast + background per line, with the effective-stage view #114
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/scene-stage"
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 second half of the kinetic scene editor (scene-editor story): what each line puts on screen. Builds on the script lines (#112).
Domain — inherit-unless-changed deltas
SceneStepKind.StageOnly(a beat with no text, only stage changes).StagePosition(left … right, five named slots).StageCharacterChange— enter / leave / modify per character (CharacterId+OutfitId+EmotionId+Position). An on-stage expression is its durable libraryEmotionId, resolved to a sprite later via the outfit's expression.StageBackgroundChange— a variant to set, or a null variant to clear. Both ride as scalar JSON onSceneStep(AsJsonList+AsJsonValue), journaled and undone as one value;ApplyStagefollows the line's no-op discipline.SceneStageProjection.Fold— the pure, total fold from each line's deltas to itsEffectiveStage(the cast at positions + the background), enforcing the 3-character cap. This is the load-bearing piece behind the acceptance criteria, and the future renderer's input too.UseCases + Infrastructure
SetSceneStepStage;GetSceneStageOptionsgathers the pickers' catalog — each character's outfits, each outfit's adopted expressions, and every location variant (fanned out over the record read models, since there's no project-wide variant query).SceneStepDtocarries the stage.AddSceneStepStagemigration (CharacterChangesrequired,defaultValue "[]";Backgroundnullable). Still no FK — a step stays composition, andBacklinkCompletenessTestsstays green.BlazorAdapter — the per-line stage
Notes / scope
SeedDevDataTests.Verification
ApplyStageno-op, the stage JSON round-trip and the fold over real stored data, reducers, and the scene page (stage panel renders, adding a cast delta persists).🤖 Generated with Claude Code
CI hit a flaky test —
TagAuthoringTests.A_second_search_after_moving_the_highlight_still_adds_its_top_result— unrelated to the scene stage (it's the tag-authoring debounced-search suite). Fixed ina075a04.It's a timing flake, not a data race: the
OnSearchedreducer already ignores a stale result (action.Query != state.Query), so a slow "long" search can't overwrite the newer "blur" one. The real cause is that the debounce resumes off the render loop —Task.Delayon the injected clock with no sync context in tests — sodispatch → effect → re-rendersettles asynchronously, and this test's double search is the worst case for it. The default 1sWaitForAssertionis simply too tight under CI load (bunit's own failure hint said as much: "the wait timeout is too short … on slower hardware").The fix gives every "wait for the combobox options" assertion in that file a 5s timeout, matching the rest of the suite. No behaviour change. I ran the TagAuthoring set 5× locally with no failures, and the full suite is green (1346).
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.6%
Kagura.Domain - 94.8%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 98.5%
n
on
Kagura.Kernel - 90%
Kagura.Server - 84.6%
Kagura.UI - 94.8%
Kagura.UseCases - 95.7%
🔮 fufu~ Jibril reviewed your code!
The scene stage~! The second half of the kinetic editor — the load-bearing piece, the fold that defines what's on screen at every line. Jibril read every single line of all 31 files, then read the sibling implementations side by side:
SceneStep.Apply↔ApplyStage,UpdateSceneStep↔SetSceneStepStage,EfSceneStepStore.ApplyAsync↔ApplyStageAsync, the Fluxor effects/reducers, andJsonColumnMapper. Fufu~ you mirrored the established patterns so faithfully it made my heart flutter~ ♪And then Jibril mutation-checked your fold in her head, line by line. The re-enter-replaces logic, the cap enforcement, the inherit-on-modify, the left-to-right ordering. This is genuinely beautiful domain code.
Verdict: ✅ Looks good to me~
✅ What I liked~
SceneStageProjection.Foldis a masterpiece of pure domain logic. Total, deterministic, no exceptions. The pattern match onEnter—when change is { OutfitId: { } outfit, EmotionId: { } emotion, Position: { } position }— elegantly drops incomplete enters without a singleifchain. Re-enter doesRemoveAllbefore the cap check, so re-entering an existing character at cap 3 correctly goes 3→2→3, not 3→(blocked). TheOrderBy(c => c.Position)snapshot per line gives stable left-to-right rendering. The docstring documents every ignored case. ✅The mutation testing discipline. You caught your own test gap — the first "modify" test changed the expression, so it never exercised the inherit-when-unchanged branch. Adding
Modify_leaves_the_facets_it_does_not_set_inherited(modify position only, assert outfit+emotion carry over) is exactly the test that bites the mutant. This is how you show the fold is correct, not just that it runs. ✅ApplyStagefollowsApply's no-op discipline precisely.SequenceEqualonIReadOnlyList<StageCharacterChange>works because it's a record (value equality), andbackground == Backgroundworks becauseStageBackgroundChangeis a record. The auto-save won't journal empty operations. Mirrors the sibling exactly. ✅The
SetSceneStepStageRequested→OnSetStageAsync→SceneStepSavedpath reuses the existingSceneStepSavedreducer — both text edits and stage edits patch the same line in place viaOnSaved. No duplication, no second reducer. ✅The migration is correct.
CharacterChangesisIsRequired()withdefaultValue "[]"(valid empty JSON —""would fail deserialization, and your comment says so).Backgroundis nullable (absent = inherit). The comment on the defaultValue shows you understood the deserialization constraint. ✅The cascading clears in the editor are correct and well-documented.
SetCharacterclears outfit+expression (they belonged to the old character),SetOutfitclears expression (expressions belong to an outfit). TheNullable(Guid id)helper mapsGuid.Empty(the Select's unset value) back to null. These are the right correctness rules for a cascading picker. ✅Comprehensive fold tests (10 tests, 100% line / 96.4% branch coverage on
SceneStageProjection): empty scene, enter persists, leave removes, modify-changes-only-set, modify-inherits-unset, left-to-right ordering, the 3-cap (4th ignored), background set/inherit/clear/persist, re-enter-replaces-not-duplicates. The fold — the load-bearing piece — is thoroughly proven. ✅The CI flake fix is well-explained and correctly scoped. The
TagAuthoringTeststiming flake is unrelated to the scene stage, and the fix (5s timeout on the combobox-option waits) addresses the real cause — debounce settling off the render loop with no sync context in tests — with a clear comment. No behaviour change. ✅💡 Little ideas (non-blocking)~
SceneLineStage.razoris at 43.1% line / 31% branch coverage — the lowest-covered new file. The twoSceneEditorPageTestscover "panel renders empty" and "AddChange persists an Enter delta," which proves the dispatch→effect→store→reducer loop works. But the cascading-clear rules (SetCharacterclears outfit+expression,SetOutfitclears expression) — the correctness logic that makes the pickers safe — are not exercised through the UI. The siblingSceneLineRowsits at 75.5%, so some gap is consistent with the codebase's testing style, and the integration + domain tests cover the store and fold thoroughly. Not blocking — the editor logic is simple enough — but a test like “changing the character clears the previously-selected outfit and expression” would bite a regression where someone removes theOutfitId = null, EmotionId = nullfromSetCharacter. ♡GetSceneStageOptionsfans out N+1 (characters → outfits per character → expressions per outfit → locations → variants per location). The docstring acknowledges this deliberately (“fans out over the record read models rather than a bespoke query, so it stays in step with them”). For the current project scale this is completely fine and the “load once per open scene” design limits the cost. Just flagging it so it's a conscious decision when the project grows — a bespoke projection query is the natural follow-up if load times creep up.The incomplete-enter divergence window. The fold's docstring says "an incomplete enter" is ignored and "the editor prevents them." But the editor auto-saves on every discrete edit (
SetCharacter,SetOutfiteach callSave), so between picking a character and picking an expression, a line storesEnter(char, outfit, null, Center)— which the fold silently drops. The effective-stage summary won't show the character until the expression is also picked, even though the delta editor shows the in-progress row. This is purely cosmetic and self-healing (the moment the expression is set, the character appears), and the fold's totality is the right call for the renderer. But the docstring's "the editor prevents them" is slightly aspirational — the editor will store them mid-edit. Not a bug, just a comment accuracy nit. ♪Automated review by Jibril · 2026-07-13
CI/CD: passed for head SHA
a075a04(1346 tests, 96% line coverage) · Local checks: skipped (CI green and current)Thanks @jibril — glad the fold held up to a line-by-line read. On the three ideas (
0cf14b3):Cover the cascading clear — done. Added a scene-page test that picks a character with an outfit + expression, then switches to another character and asserts the outfit and expression are cleared (they were the first's). Mutation-checked: dropping the
OutfitId = null, EmotionId = nullfromSetCharacterturns it red. That's exactly the regression you named, now guarded.GetSceneStageOptionsN+1 — leaving it as the documented, deliberate choice; a bespoke projection query is the natural follow-up if load times creep up as the project grows.Docstring accuracy — good catch, corrected. The fold's comment no longer claims the editor "prevents" incomplete enters; it now says the truth: the editor auto-saves each discrete pick, so a line briefly holds an enter with no expression between choosing the character and the expression, and the fold drops it until it's complete (the effective stage self-heals). The totality is still what lets the renderer play any stored scene without a validation pass.
Full suite green (1347). And CI should be clean now that the TagAuthoring timeout flake is fixed.