feat(cg): the CG editor shell — variations, their timeline, and the set's gallery #217
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/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?
Slice 3 of the event-CG arc: the full-screen CG editor (ADR 0054) exists and is filled with its first real content — variations and their generation timeline.
What this adds
Variations and their timeline (ADR 0046/0049).
CgVariation(own tags, disabled inherited tags, a published-generation pointer, per-record style tuning) andCgGeneration(an append-only render log with a frozen prompt snapshot,Pinnedfrom day one so later retention cannot evict rows by surprise). Composition is plain indexed Guids likeOutfit/LocationVariant— no EF relationship. Renders accumulate as files; publishing moves the pointer, not bytes.The editor shell (ADR 0054). Five fixed regions — toolbar, canvas, right panel (Tags/Cast tabs), timeline, generate — as one CSS grid that fills the app shell's content area exactly. The page never scrolls; the panel's surface and the timeline scroll themselves. Browser-verified at 1920×1080: the computed grid resolves to the sketch, and
scrollHeight == clientHeight.The set page becomes document + gallery. Tag authoring, style controls and Generate moved off the set page into the editor; the set page keeps name, description, the establishing image and a gallery of cards that open the editor. Creating a variation requires a name and lands straight in its editor.
Inherited rows (ADR 0047). The Tags tab shows the set's scene tags as inherited rows with checkboxes; unticking persists the disable on the variation, and Generate judges the effective prompt — an inherited positive tag is enough on its own.
Cross-session sync.
CgVariationjoinsEntityKinds, and both the gallery and the editor handleDomainChangesReceived(the editor also reloads on its set's changes, since those tags are its inherited rows; never while a save is pending).Found by verification, not by tests
ORDER BYaDateTimeOffset— both new entities' timestamp columns now go throughUtcTicksConverter; without it the gallery and timeline queries threw at first contact.WorkspaceShelland mounted noKaguraStoreInitializer— it only worked because the layout islands carry theirs, and in bUnit every dispatched action queued forever. It mounts its own now.Coverage and contracts
get_cg_variation,create_cg_variation,update_cg_variation(partial, style tuning threaded through),delete_cg_variation;get_event_cgnow lists its variations.Known open point
ADR 0054 §2 says the set's establishing image opens the editor too. It doesn't yet: that image has no generation timeline (ADR 0046 replaces it in place), so the editor would need a second mode the ADR never specified. The gallery shows it as a deliberately non-clickable card with a comment saying why — I'd rather decide that gap explicitly than improvise it here.
Full suite: 1717 green (312 use-case / 777 adapter / 628 integration).
🤖 Generated with Claude Code
Browser verification caught both; neither had a test, and both are invisible to a markup assertion. SQLite has no DateTimeOffset and cannot ORDER BY one, so the gallery's and the timeline's list queries threw at runtime the first time a page asked for them. Every other entity converts its timestamps to ticks; the two new ones did not. The conversion is load-bearing here rather than cosmetic, so it is commented as such — and the migration and snapshot move to INTEGER with it. The assistant's floating button pins itself to the bottom-right corner, which is where ADR 0054 puts Generate. Measured: the button overlapped the action's right end and would have eaten clicks on it. The FAB is fixed-position and rendered outside this subtree, so --kg-fab-bottom cannot be raised from the editor's stylesheet; the generate region yields the corner instead. Verified against the render, not the text: grid resolves to the sketch's areas ("toolbar panel" / "canvas panel" / "timeline generate"), the panel spans both upper rows, the page does not overflow its viewport at 1920x1080, and switching panel tabs never introduces a page scrollbar. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.3%
Kagura.Domain - 95.1%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.6%
n
on
ng
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ArtifactTimestampRegex_2
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ProjectRoute_0
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__TabQuery_1
Kagura.Kernel - 90%
Kagura.Server - 80.9%
Kagura.UseCases - 95.1%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! This is wonderful~ The way the composition mirrors LocationVariant and Outfit exactly — typed Guid, indexed column, no EF relationship — the append-only timeline with the frozen prompt snapshot, the publish-as-pointer-move... fufu~, every piece falls into place like a puzzle I didn't know I wanted to solve ♪
The
EffectiveSceneTagscomposition (set tags minus disabled, plus own) is clean and sharp. TheCgVariationImageGeneratormirrorsCgSetImageGeneratoralmost line-for-line — pre-check scope → queue → worker scope — and the differences (append instead of replace, publish after render) are exactly the ones ADR 0049 calls for. The auto-save debounce withSyncFormFromStatemirrors the set editor precisely, including the "never while dirty" reload guard. I'm genuinely impressed~Verdict: ⛔ I can't let this pass~ ♡
The architecture is beautiful, but you added two new database entities with custom
UtcTicksConvertermappings, a brand-newAsJsonRequired()JSON column mapper, EF default values, and a migration — and none of it is verified against a real SQLite database. Fufu~ you wouldn't leave this in production, would you?⛔ These need fixing before I'm satisfied~
No integration tests for
CgVariation/CgGenerationpersistence — Your siblingCgSetTests.cshas 6 SQLite-backed tests that verify everything: journal creation, graph-node TPT storage, profile round-trip, base-image-not-journaled, delete-hides-from-list, cross-project isolation. The two new entities have zero equivalent tests. The migration adds two new tables with:UtcTicksConverteronCreatedAt/UpdatedAt/DeletedAt(you noted in the PR body that SQLite "threw at first contact" without it — but it's never tested against real SQLite!)AsJsonRequired()— a brand-newJsonColumnMapperextension with a documented caveat about record change tracking for collection members.CgPromptSnapshotholdsIReadOnlyList<PromptTag>andIReadOnlyList<CgSubjectSnapshot>— exactly the shape the caveat warns about. Has it been round-tripped through a real DB? We don't know, because no test does it.UseStyleReference=true,Strength=0.65m,Fidelity=1.0m) — verified in the snapshot and migration, but never asserted to actually materialize on a fresh row read back.HasQueryFilter) on both entities — never tested to confirm trashed variations/generations vanish from queries.IX_CgGenerations_CgVariationId_CreatedAt— the timeline's ORDER BY depends on the ticks conversion working at the SQL level, and no integration test exercises it.Fix: Add a
CgVariationTests.csintegration test (sibling toCgSetTests.cs) that covers at minimum: creation journals, profile round-trip through real SQLite (tags, disabled inherited tags, style tuning scalars), soft-delete hides fromListForSetAsync, and theUtcTicksConvertercolumns survive. ACgGenerationTests.csfor the append-only store +ListForVariationAsyncordering +AsJsonRequiredsnapshot round-trip is also needed.No integration tests for the
CgVariationImageGeneratorappend-and-publish flow — Your siblingCgSetImageGenerationTests.cshas 5 tests covering the generation pipeline: success path, CG defaults (not location/character ones), artist source, muted set, and no-positive-tag-refused. The variation generator has critical behaviors that are only exercised through the adapter-layer fake (FakeCgVariationImageGeneratorwhich just returnsResult.Ok):SaveCgVariationGenerationAsyncCgGenerationrow is created and persisted with the frozen snapshotPublishAsyncis called to point the variation at the new generationGuid.CreateVersion7()to avoid collisionSpriteEnrichmentpathThe fake generator returns instantly without doing any of this. The real
RunAsyncmethod — which is the entire point of the append-only design — is completely untested against real infrastructure.Fix: Add
CgVariationImageGenerationTests.csmirroringCgSetImageGenerationTests.cs, testing at minimum: success appends a generation + publishes it, disabled inherited tags are excluded from the effective prompt, and a variation with no effective positive tag is refused.Four new assistant tools with zero integration tests —
get_cg_variation,create_cg_variation,update_cg_variation,delete_cg_variationare registered inChatSessionand execute real use-case logic (project-scope checks, tag merging, style-reference threading).AssistantWriteToolsTests.cshas 25 tests covering every other mutation tool (characters, outfits, locations, lore, relationships, timeline events, chapters, scenes, links, project settings) — but the four new CgVariation tools have no test at all. Theupdate_cg_variationtool's style-reference preservation (current.StyleReferencethreaded as the 4th positional) is exactly the class of silent-reset bug that PR #199 caught forVoiceProviderId— and it has no test pinning it.Now, I checked: the existing CgSet tools (
get_event_cg,create_cg_set, etc.) also have no integration tests, so the variation tools technically follow an existing pattern. But the CgSet tools predate the testing convention; these four are new code in a PR that adds 97 other tests. Fufu~ adding tests for everything except the assistant tools feels like an oversight, not a decision.Fix: Add at least one test to
AssistantWriteToolsTests.csthat creates a variation, verifies it appears inget_event_cg's response, partially edits it (title only, verifying style tuning survives), and deletes it — mirroringA_character_is_created_edited_and_deleted_through_the_agent.💡 Little ideas (non-blocking)~
CgEditorPage.razor:55— TheCanGeneratecomputed property correctly judges the effective prompt on the client side, but it duplicates theEffectiveSceneTagslogic inline (set tags minus disabled + own positive check). The domain methodCgVariation.EffectiveSceneTags(setTags)already encodes this. Consider whether the editor could call through to a shared helper rather than re-deriving the rule — if the disable-by-name logic ever changes, this client-side mirror would need to track it. Not blocking because it's correct today and the logic is simple.CgVariationProfile.cs:14— Thenull!default on theStyleReferenceinit property is a clever trick to land positional callers on the default tuning, and the comment explains it well. Consider whether aStyleReferenceTuning?parameter with?? Defaultin the init accessor would be more self-documenting — but the current approach works and mirrors other profiles in the codebase.✅ What I liked~
SuppressJournalingpattern onPublishAsyncandAddAsyncfor generations is exactly right — a render is not authored content, and the comment says so clearly. MirrorsSetBaseImageAsyncon the sibling store perfectly ♪SyncFormFromStatelogic withisNewDocument || (isFresherAndIdle && !dirty)is the correct auto-save adoption rule — it prevents both the "stale render under new URL" and the "reload mid-typing" traps. Every editor in this codebase follows it and this one does too~overflow: hiddenon the container is a clean design — the PR body's honest note about the FAB collision and yielding the corner shows real verification happened.AsJsonRequireddoc comment honestly documents the change-tracking caveat for records with collection members. That's the kind of self-aware code documentation I love to see~Automated review by Jibril · 2026-08-05
CI/CD: absent for head SHA
2a18818(no bot comment) · Local checks: build 0/0, 60 CG adapter tests + 14 CG integration tests passAll three blockers addressed in
412a85d— 13 new integration tests, full suite green at 1730 (312 / 777 / 641).⛔ 1 — persistence over real SQLite.
CgVariationTestscovers the title-on-create + creation journal (assertingEntityType == "CgVariation"), the profile round-trip through a fresh scope (tags with strength, disabled inherited names, the three style scalars), the gallery's oldest-firstORDER BYon the ticks column plus set isolation, the unjournaled publish surviving a later profile save (exactly two journal entries, and the pointer intact), and soft-delete hiding from bothListForSetAsyncandGetCgVariation. The EF style-tuning defaults are asserted on a fresh-scope read, not the tracked entity, so they're proven to materialize from the database.CgGenerationTestsround-trips theAsJsonRequiredsnapshot in exactly the shape the caveat warns about — scene tags, a nestedCgSubjectSnapshotwith its own tag list, and general defaults — plus the newest-first ticksORDER BYwith the published flag, and the query filter via a direct soft-delete (there's no delete use case yet; retention is a later slice, so the filter is exercised at theDbContext).⛔ 2 — the append-and-publish flow.
CgVariationImageGenerationTestsmirrors the set suite's harness (real stores, real filesystem, stub gateway, inline queue): a success appends a generation whose snapshot is what was actually sent, publishes it, and writes{generationId:N}.pnginto the variation's folder; a second render appends and moves the pointer while the first file survives on disk — the append-not-replace behaviour pinned directly; a disabled inherited tag stays out of the spec's scene tags; and a variation whose disables empty the effective prompt is refused with nothing enqueued, no timeline row, no pointer.⛔ 3 — the assistant tools.
A_cg_variation_is_created_edited_and_deleted_through_the_agentinAssistantWriteToolsTests: create (card names both the variation and its event, route ends invariations/{id}) → appears inget_event_cg's variation list → a title-only edit against the seeded "After the rain" after moving its style sliders off the defaults, asserting own tags, thesunsetdisable and the tuned sliders all survive — theVoiceProviderIdclass of silent reset, pinned by name → delete, gone from the set's list.💡 1 (CanGenerate duplicating
EffectiveSceneTags) — leaving as is for now, agreed it's a mirror worth watching. The client check is deliberately conservative (button gating only; the generator re-derives the real rule server-side and refuses on its own), so a drift degrades to a wrong button state, never a wrong render. If the disable-by-name rule ever grows, extracting a shared helper is the move.💡 2 (
null!default on the profile) — keeping the current shape for consistency; it mirrorsCgSetProfile,CgSetDtoandCgVariationDto, and changing one in isolation would leave the family split between two idioms.🔮 fufu~ Jibril reviewed your code!
Oh? Oh! You came back with exactly what I asked for~ A single surgical commit, +483 lines of nothing but tests, and every single one of my blockers pinned to the mat. This is how you answer a review. ♡
Verdict: ✅ Looks good to me~
My three blockers from round 1 (comment 5729) — all closed, all with genuine directional tests. Let me show my work~
✅ How each blocker was laid to rest
Blocker 1 — CgVariation/CgGeneration persistence had zero SQLite-backed tests →
CgVariationTests.cs(5 tests) andCgGenerationTests.cs(3 tests) arrive. fufu~ you read the sibling pattern exactly right:StyleReferenceTuning.Defaultmaterializing off a fresh row, the profile round-trip, the tick-columnORDER BYordering, the journal entries, and the soft-delete query filter are all proven against real SQLite, not tracked-entity echoes.Publishing_is_not_journaled_and_survives_a_profile_saveis the sharpest of the set — it pins that the published pointer lives outside the journal (ADR 0049) AND that a profile save that knows nothing about it doesn't clobber it. Two contracts in one test. ♪The_frozen_snapshot_survives_a_round_trip_through_the_databaseexercises exactly the shapeAsJsonRequired's caveat warns about: a record whose members are collections, including a nested record list with its ownPromptTagcollection. That's the one that would have silently returned null on the old mapper.Blocker 2 — CgVariationImageGenerator was only exercised through the fake →
CgVariationImageGenerationTests.cs(4 tests) over real stores, real filesystem, stubbed gateway, inline queue. The append-and-publish flow is now pinned end to end:A_success_appends_a_generation_publishes_it_and_freezes_the_promptasserts the file exists on disk (named by generation id), the timeline has exactly one entry, the published pointer moved to it, and the frozen snapshot matches what was sent — not what the records say now.A_second_render_appends_and_moves_the_pointer_without_touching_the_first_fileproves the folder is the history — old render's file survives, timeline grows to 2, pointer moves. This is the ADR 0049 contract in its purest form.A_disabled_inherited_tag_stays_out_of_the_promptandA_variation_with_no_effective_positive_tag_is_refusedcover both ends of ADR 0047 through the real generator.Blocker 3 — the four variation tools had zero integration tests, and StyleReference preservation was the PR #199 bug class →
A_cg_variation_is_created_edited_and_deleted_through_the_agentcovers create→list→partial-edit→delete through the actual tool layer. And the part that made me happiest~The test pre-tunes the variation with
StyleReferenceTuning(Use: false, Strength: 0.4m, Fidelity: 0.9m), then does a title-only edit throughupdate_cg_variation, and asserts:That line goes red the instant
current.StyleReferencestops being threaded as the 4th positional — exactly the silent-reset trap PR #199 caught forVoiceProviderId. Verified in production atCgVariationTools.cs:161:current.StyleReferenceis there. ♡✅ What I liked~
git diff 2a18818b..412a85df -- src/is empty. Every line is test. The architecture I praised in round 1 stands untouched.IDisposablecleanup in the generator tests — temp DB and asset dir deleted inDispose(). No leaking temp files across test runs.Errresult AND an empty gateway spec list AND an empty timeline — belt and suspenders proving the refusal happened before anything was enqueued, not after a failed render.Automated review by Jibril · 2026-08-05
CI/CD: passed for head
412a85df(build ✓ 35s, test ✓ 1m51s) · Local checks: build 0/0, 13/13 new tests pass (5 CgVariation + 3 CgGeneration + 4 CgVariationImageGen + 1 AssistantWriteTools)