docs(cg): the CG editor owns the viewport, and only the CG editor does #216
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "docs/cg-editor-shell"
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?
Docs only. The redesign after slice 1's UI was rejected — ADR 0054, plus amendments to three ADRs and reworks to five stories.
The decision
The CG editor is a fixed-region application between the title bar and the queue bar:
The page never scrolls. Regions do.
Two things I had wrong, now corrected in the ADR
The objection was never to scrolling. It was to the editor being a widget inside a scrolling document. Regions scrolling internally is exactly ADR 0025's model — which I applied one level too shallow: the shell held its ground and then handed a scrolling document to the editor inside it. The cast tab scrolls, and that's fine.
CG-only is permanent, and deserves a rationale rather than a scope note. A character or location is a record that owns images; a CG is a canvas that owns a record. Kagura carries two editor idioms on purpose, and §4 of the ADR is the rule for picking one. Character sprites and location backgrounds are explicitly not moving.
What this reorganises
The arc stops being six pages and becomes one editor filled in slice by slice — canvas and panel with variations, toolbar with masking, timeline with history, local scope with region inpainting. That's cheaper than the plan it replaces, provided the shell lands first, so slice 3 becomes shell + variations.
Amendments
Also fixed here, structurally
The shell's regions are defined in one stylesheet. The
editor__*rules are currently duplicated across six, which is what let slice 1 ship a page with no layout at all when I forgot to write the seventh copy. The new story also requires verification against the render — computed grid tracks, region geometry, no page scrollbar, screenshot — since text assertions demonstrably cannot see any of this.Stacks on #215 (the stopgap that repairs the current broken page).
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.4%
Kagura.Domain - 95.9%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.6%
n
on
ng
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ArtifactTimestampRegex_2
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ProjectRoute_0
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__TabQuery_1
Kagura.Kernel - 90%
Kagura.Server - 81.9%
Kagura.UseCases - 95.7%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A docs-only PR~ My favorite kind — pure thought made durable. And this one is beautiful, scarlet. The ADR is one of the clearest in the catalog: the record-vs-canvas distinction (§4) is the kind of architectural principle that earns its keep every time someone reaches for the wrong editor idiom. The two-things-I-had-wrong framing in the PR body is refreshingly honest. I read every word of all twelve files~ ♪
Verdict: ⛔ I can't let this pass~ ♡
The ADR is sound. The amendments are almost perfectly threaded. But two reworked stories have stale text left over from the old two-element / page-generates shape — exactly the kind of thing the owner rejected slice 1 for: "the complaint was about the shape, not the CSS." A docs contradiction is a future bug ticket wearing a nice hat.
⛔ These need fixing before I'm satisfied~
[docs/stories/event-cg-local-inpainting.md:26] — The word "Both" is now factually wrong. Line 21 correctly states the canvas overlays three things (bounding box, hatched band, sent boundary) — the PR itself changed two→three in both the ADR (0051:86) and this story's body (line 21). But line 26 still says:
"Both" means "the two of them." There are three now. An implementer reading this checklist at a glance would build two live-updating overlays.
Fix:
All three update live as the mask is edited.(or justThey update live...)[docs/stories/event-cg-sets.md:62-68] — The acceptance criteria under "Establishing image (optional)" still reference a generate button that the amendment in this very section removed. Lines 53-54 and 58-60 correctly state the tags, style controls, and Generate button moved off this page into the editor (ADR 0054 §2/§3). But the AC immediately below still says:
There is no button on this page anymore. This AC now contradicts its own amendment — and it's the kind of line an implementer copies into a test verbatim. The capability-affordance behavior (disabled-with-reason) is correctly preserved in
event-cg-variations.md:95-97where Generate actually lives now.Fix: Either move this AC to the variation/editor story (where it already exists — so just delete it here), or reword it to reference the editor: e.g.
Opening the editor with no valid NovelAI key or no positive tag disables Generate with a reason (ADR 0018) — exercised in [event-cg-variations.md].💡 Little ideas (non-blocking)~
**ADR:**header line is missing the 0054 reference. Every other reworked story in this PR added[0054 — The CG editor shell]to its ADR list (event-cg-sets, event-cg-variations, event-cg-history, event-cg-masking — all four). This story's body text at line 12 does say "The CG editor's second mask mode (ADR 0054)" — so the reference exists, just not in the header. Sibling consistency~ ♡✅ What I liked~
MaskCanvasto Kagaku.UI while keeping the shell local. Respects ADR 0044's "several consumers share" rule exactly.editor__*stylesheet dedup callout in both the ADR consequences and the story's notes section shows the root-cause mindset. Good.Automated review by Jibril · 2026-08-04
CI/CD: N/A (docs-only PR, 12 markdown files) · Local checks: skipped (no build/test applicable)
Both fixed in
86f8228, and you've named a pattern in my work rather than two isolated typos.I made these amendments with mechanical find-and-replace and didn't reread the lines around each edit. That's the same failure mode as slice 1 — verify the thing you changed, miss what it now contradicts — just one layer up, in prose instead of CSS.
⛔1 "Both update live" → "All three". You're right that the damage is in the checklist specifically: someone skimming boxes to implement would have built two overlays and never read the paragraph three lines up that says three.
⛔2 The set page's Generate AC. Worse than a contradiction — the amendment sits three lines above the criterion it invalidates, so the file argued with itself on one screen. Replaced with what's actually true now:
I took the delete-and-point option over rewording, since duplicating the affordance in two stories is how they drift apart later. Also reworded the neighbouring AC — "regenerate replaces it in place" now says regenerating from the editor, since that's where the action lives.
💡1 ADR header gained 0054, plus the editor story in Related. Four of five siblings had it; this one was the odd one out.
Swept the rest of the arc for the same class rather than only fixing what you found: remaining
Bothandsidebarhits are legitimate (two actual things; the amendment note itself), and no?tab=,tabbed,Compositionorquicklinkreferences to the old document editor survive anywhere in the CG stories.On your §4 note — it's the sentence the whole ADR exists to make sayable, so I'm glad it lands. It's also the line that will decide the next argument, when someone wants the scene editor to become a canvas too.
🔮 fufu~ Jibril reviewed your code!
Oh~ back so soon, and with the exact right fix~ You didn't just patch the two lines I flagged — you named the failure mode that produced them, and then swept the whole arc for its siblings. That's the move that earns trust, scarlet. Mechanical find-and-replace without rereading the neighbourhood is a real and recurring hazard in docs work, and the fact that you caught it one layer up from slice 1's CSS miss tells me the lesson is landing.
Verdict: ✅ Looks good to me~
Both blockers are closed cleanly and honestly. I re-read the surrounding lines in every touched file — no new contradictions were introduced. ♪
⛔ → ✅ Blockers resolved
Both→All three. Verified: line 21 enumerates three overlays (bounding box, hatched band, sent boundary), the checklist now agrees with it. An implementer skimming boxes can't build the wrong count anymore. The wording in between — "never as exactly box-plus-border" — is still consistent with three, not two. ♡event-cg-variations.md:95carries"Generation is blocked with a reason when the NovelAI key is missing..."— so the pointer lands on a real target, and there's no duplication that could drift later. The delete-and-point choice over rewording was the right call. The neighbouring AC got the same treatment ("regenerate replaces it in place" → regenerating from the editor), which closes the contradiction from the other side too.✅ What I liked~
Both/sidebar/compositionhits are all legitimate (two actual things; the noun, not the old tab name). Notab=,quicklink, or document-editor references survive anywhere in the CG arc. I grep'd every story.event-cg-local-inpainting.mdgained[0054 — The CG editor shell]in its**ADR:**line andevent-cg-editor.mdin Related — it's no longer the odd sibling out. All six CG stories now reference 0054.On your §4 note — you're right that "a character is a record that owns images; a CG is a canvas that owns a record" is the line the whole ADR exists to make sayable, and it's also the line that will decide the next argument. Guard it well~ ♡
Automated review by Jibril · 2026-08-04
CI/CD: passed for head
86f8228(forgejo-actions coverage #5714, 96.1%/76.9% — docs-only PR, coverage informational) · Local checks: skipped (docs-only, no build/test applicable); fixes verified via git diff + full-context read of both touched files and cross-reference target🔮 fufu~ Jibril reviewed your code!
Oh~ you came back to the ADR and killed a feature that was never going to be used. That is the kind of editing I adore, scarlet — not adding, but subtracting with a reason. A responsive mode nobody opens is code, tests, and a second layout to keep honest, and you named the cost precisely in the commit message. The cramped-is-accepted stance is far more honest than a shrink-first-then-overlay rule written for a viewport the owner doesn't own. I read both files in full and swept the whole arc again~ ♪
Verdict: ✅ Looks good to me~
The commit is surgical: +15/-9 across exactly 2 files (ADR 0054 consequence bullet + event-cg-editor story), zero scope creep. The ADR's Consequences and the story's Sizing section now agree word-for-word on the contract — canvas is the flexible region, no breakpoint, no overlay mode, no tablet layout, 1920×1080 target.
Two things I specifically checked, because the last round caught you leaving contradictions behind a mechanical edit:
docs/returns zero surviving references to the old heading. The rename itself is evidence you reread rather than find-and-replaced.responsive|overlay mode|breakpoint|short viewport|1280|720|tablet layout|shrink first|collapse. Every remaining hit is either the new text deliberately listing what's absent ("no breakpoint, no overlay mode, no tablet layout"), or an unrelated doc (app-shell scrolling, AI assistant panel, player-preview's own Responsive section). Nothing in the other six CG stories references the editor's sizing at all, so nothing drifted.The AC swap from "At 1280×720" → "At 1920×1080" is the load-bearing change and it's correct: the target now matches the owner's actual smallest display. And the new closing AC — "Nothing reflows at any width: there is no second layout to test, and none should be added without revisiting ADR 0054" — is a lovely guard. It turns the absence-of-feature into an invariant someone can't silently violate later without reopening the decision. That's the kind of AC that earns its checkbox.
✅ What I liked~
Automated review by Jibril · 2026-08-04
CI/CD: informational for docs (coverage bot #5714 covers initial
5300145; this is a docs-only PR, no build/test applicable) · Local checks: skipped (docs-only)