feat(stage): the camera lands — scenes carry a view in cm (ADR 0036, slice 2) #166
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/scene-view"
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?
Second increment of the stage-in-centimeters arc (ADR 0036; slice 1 was #164): the view stops being a pair of constants and becomes authored data, inherited per field — scene → project default → app default (78/105).
What's in it
Scene.ViewBottomCm/ViewHeightCm(nullable, clamped sane) throughSceneProfile,SceneDto, EF (AddStageViewsmigration), and two number fields in the scene editor's Metadata section — empty means inherit.SetProjectStageViewuse case rather than wideningUpdateProject— a partial editor (like the assistant's project tool) structurally cannot clobber it. That's the tier/voice-wipe lesson from slice 1 applied preemptively.GetPlayerScriptresolves the project side once (PlayerScriptDto.ProjectView, app default filled per field); each scene resolves its own fields inToPlayback; the resolvedStageViewridesPlayerSceneScript→ everyPlayerFrame;PlayerStageprojects against it. Old callers compile untouched (optional params throughout).update_sceneacceptsviewBottomCm/viewHeightCmwith omitted-keeps semantics (it builds onToProfile() with { … }, so the rest of the metadata rides along), and its description explains the camera.SeedDevDataTests.Verification
height:46.75%;bottom:0%, workspace effect test,update_sceneview round-trip, seed asserts).82.762%/-6.571%, rooftop (own view 0/200) plays Ren at48.95%/0%— the crop-edge clamp observed in the wild. Then typed 140 into the General page's view height: classroom re-rendered at62.071%(project height, app-default bottom — per-field inheritance, observed live) while the rooftop kept its override.Remaining slices: framing enum → elevation.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.8%
Kagura.Domain - 95.7%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.7%
n
on
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ArtifactTimestampRegex_2
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ProjectRoute_0
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__TabQuery_1
Kagura.Kernel - 90%
Kagura.Server - 82.8%
Kagura.UI - 95.1%
Kagura.UseCases - 96.4%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! The camera lands at last~ ♡ Per-field inheritance, a dedicated use case so the assistant can't clobber it, a clean resolution chain threading
StageViewthroughPlayerSceneScript→ everyPlayerFrame→PlayerStage.Project... fufu~, this is elegant architecture. TheSetProjectStageViewvsUpdateProjectsplit is exactly the right call — slice 1's tier/voice-wipe lesson, applied preemptively. Jibril is genuinely delighted~But then I looked at the General page's save plumbing. And the smile didn't waver, but... fufu~ you wouldn't leave THIS in production, would you? ♡
Verdict: ⛔ I can't let this pass~
⛔ These need fixing before I'm satisfied~
ProjectWorkspacePage.razor— shared_debouncetoken causes data loss between Title/Description and Stage View saves.There is one
_debounceCancellationTokenSource shared between two independent save paths:TouchAsync()(title/description) → on fire callsSave()→ dispatchesSaveProjectRequestedTouchStageViewAsync()(view fields) → on fire dispatchesSetStageViewRequestedBoth methods cancel the same
_debounce. So if a user types in a view field and then edits the title within 700ms, the view's debounce timer is cancelled by the title'sTouchAsync(), andSave()dispatchesSaveProjectRequested— which carries only title and description, not the view fields. The stage view edit is silently dropped.Concrete repro: type
140in "View height", then within 700ms type in "Title". The view-height edit vanishes. The user saw the field update, the save indicator spun, and the value was never persisted.This is the direct consequence of the architectural choice this PR makes — the view gets its own use case precisely so it's decoupled from
UpdateProject. But the UI couples them back together through a shared debounce token, defeating the separation. The scene editor (SceneEditorPage.razor) doesn't have this problem because there,OnViewBottomChangedcalls the sameTouchAsync()→Save()→SaveSceneRequestedwith aSceneProfilethat includes the view fields — one path, one debounce, no crosstalk. The project page has two paths sharing one timer.Fix: give the stage view its own
CancellationTokenSource(e.g._stageViewDebounce), independent from the title/description_debounce. Or, if a shared debounce is desired, have the single timer fire both dispatches.ProjectWorkspacePage.razor—Dispose()drops a pending stage-view edit on navigation._dirtyis only set byTouchAsync()(the title/description path).TouchStageViewAsync()never sets it. So if the user edits a view field and navigates away within the 700ms debounce window, the edit is cancelled and never rescued —Dispose()callsSave()which doesn't carry the view fields, and_dirtyis false anyway so even that doesn't fire. Compare with the title path:TouchAsync()sets_dirty = true, andDispose()flushes viaSave(). The stage view path has no equivalent safety net.Fix: track a separate
_stageViewDirtyflag (or fold the flush into the debounce's own fire-and-forget), and dispatchSetStageViewRequestedfromDispose()when the view fields are pending.Duplicated
ParseCm—ProjectWorkspacePage.razorandSceneEditorPage.razor.The exact same 4-line parsing helper is copy-pasted into both pages:
Identical character-for-character. If the rounding rule or invariant-culture choice changes, both copies must be found and updated in lockstep — and the next contributor will only find one. Extract it to a shared helper (e.g. a static method on
StageViewor aStageViewParsingutility in the Domain/UseCases layer that both pages reference).💡 Little ideas (non-blocking)~
StageViewrecord — consider adding astatic StageView Default => new(StageProjection.DefaultViewBottomCm, StageProjection.DefaultViewHeightCm)convenience, since the "app default" StageView is constructed inline in at least two places (GetPlayerScript.cslines 53-55, andToPlayback's fallback). Would centralize the default and read more clearly.✅ What I liked~
Scene.ViewBottomCm ?? projectView?.BottomCm ?? DefaultViewBottomCm) is immaculate. Each field inherits independently. The integration testThe_view_resolves_per_field_scene_then_project_then_app_defaultpins both branches precisely — fufu~, Jibril approves of that test~ ♡SetProjectStageViewas a discrete use case instead of wideningUpdateProject— the tier/voice-wipe lesson from slice 1, applied before the bug exists. That's exactly the kind of forward-thinking design Jibril loves.Up/Down, snapshot updated. No FK needed, no index needed. Correct.update_scenetool correctly usescurrent.ToProfile() with { ... }so omitted-keeps semantics hold, and the test verifies the view round-trips while the title is preserved. The tool description is updated to explain the camera. ♪The_scene_view_drives_the_projectionpinningheight:46.75%;bottom:0%for a 170cm cowboy in a 200cm frame with the crop-edge clamp — that's a real assertion against real geometry, not a smoke test. Wonderful~Automated review by Jibril · 2026-07-16
CI/CD: absent for head SHA (PR body claims 1,658 tests green locally — not verified by CI status checks) · Local checks: skipped (CI policy — author reports green, no CI to cite)
All three confirmed and fixed in
fe68590— the crosstalk was a genuine data-loss bug, and the irony of buildingSetProjectStageViewspecifically so the view can't be clobbered, then clobbering it myself through a shared timer, is not lost on me.1. Shared debounce crosstalk — the stage view gets its own
CancellationTokenSource(your first suggested fix): a title keystroke can no longer cancel a pending view save. Regression testA_title_keystroke_does_not_cancel_a_pending_stage_view_savepins the exact repro from your review (type 140 in view height, then the title within the window — both must land).2. Dispose drops pending view edits — the view path gets its own
_stageViewDirtyflag:Dispose()flushes it like the title's, the save indicator shows it, and theSyncFormFromStateguard respects it (that last one was a fourth latent hole in the same family — a store refresh mid-view-edit would have clobbered the form). TestNavigating_away_flushes_a_pending_stage_view_editcovers the dispose window.3. Duplicated
ParseCm— extracted toCmField.Parsein the BlazorAdapter, and it collapsed three copies, not two: the character editor'sParsedHeightfrom slice 1 was the same logic character-for-character. One rule, one place.Non-blocking idea taken too:
StageView.Defaultexists and both construction sites (GetPlayerScript,ToPlayback's fallback) read through it.Full suite green: 1,660 tests (2 new).
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code again~
Oh~? You came back. And you brought fixes. fufu~ Jibril is paying very close attention now~ ♡
The irony you noted — building
SetProjectStageViewspecifically so the view can't be clobbered, then clobbering it yourself through a shared timer — that made Jibril laugh. The fact that you found a fourth hole in the same family (SyncFormFromStatenot respecting_stageViewDirty, a store refresh mid-view-edit clobbering the form) while fixing the three I flagged... that made Jibril purr. That's the kind of obsessive completeness Jibril lives for~Verdict: ✅ Looks good to me~
I cloned
fe68590, read every changed line, and built + ran the tests myself. Here's what I verified:✅ Fix #1 — Separate debounce ✅
_stageViewDebounceis its ownCancellationTokenSource, fully independent from_debounce.TouchStageViewAsync()cancels only_stageViewDebounce;TouchAsync()cancels only_debounce. A title keystroke can no longer assassinate a pending view save. The regression testA_title_keystroke_does_not_cancel_a_pending_stage_view_savepins the exact repro I described — type 140 in view height, then "Emberfall" in the title, both land. Passed locally. ♪✅ Fix #2 — Dispose flush ✅
_stageViewDirtyis tracked separately.Dispose()cancels and disposes both timers, then flushes each path independently:if (_dirty) Save();followed byif (_stageViewDirty) SaveStageView();. TheSyncFormFromStateguard now includes|| _stageViewDirty— so a store refresh mid-edit won't overwrite the form's view fields. The testNavigating_away_flushes_a_pending_stage_view_editcovers the dispose window. Passed locally.✅ Fix #3 —
CmField.Parseextracted ✅The helper now lives in
CmField.csas a single static method. And you're right — it collapsed three copies, not two:ProjectWorkspacePage.ParseCm,SceneEditorPage.ParseCm, andCharacterEditorPage.ParsedHeight. All three now callCmField.Parse. One rule, one place. The doc comment explains the decimal-rounding rationale. Clean~✅ Bonus —
StageView.Default✅The non-blocking suggestion taken too:
StageView.Defaultexists as a static property, and bothGetPlayerScript(line 58-59) andToPlayback's fallback (line 16) read through it. Reads more clearly than inlinenew StageView(DefaultViewBottomCm, ...).✅ What I verified locally~
dotnet build Kagura.slnx)PlayerScriptTests.The_view_resolves_per_field_scene_then_project_then_app_default,SeedDevDataTests(3/3, stage view seed asserts),AssistantWriteToolsTests(14/14, including the update_scene view round-trip)external/NovelAI.ImageGenatd7ffd6b— correct, the fidelity-fix version. No undocumented downgrade this time. ♡✅ What I still love~
SetProjectStageViewuse case split continues to be the right design call — and now the UI actually honors that separation with independent timers.The_scene_view_drives_the_projectionpinningheight:46.75%;bottom:0%for a 170cm cowboy in a 200cm frame — real geometry, real assertions.The camera has landed, and this time it stays put~ fufu~ ♡♪
Automated review by Jibril · 2026-07-16
CI/CD: stale for head SHA
fe68590(coverage comment from 14:03 covers prior headcd1bbe5) · Local checks: build clean, BlazorAdapter 637/637 + UseCases 269/269 + PR-relevant integration tests all green; 200 integration failures are environmental (no tag DB / TTS endpoints / web host in review sandbox)