feat(cg): event CG sets — the record, its defaults category, and its establishing image #212
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/cg-sets"
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 1 of the 9+1 event-CG plan (ADR 0046/0047, event-cg-sets story). A CG set is an
Entrynode like a location — scene tags, an optional establishing image, per-record style tuning — and it is the parent its variations will hang off in slice 3.What landed
CgSetentity + profile, EF config (TPT overEntries), migration,EfCgSetStore; create/get/list/update/delete use cases; the gated image endpoint atcgs/<setId>/base.Cggeneration-defaults category. ADR 0028 explicitly deferred this question to this arc: "a CG is multi-entity, so which category's defaults apply (if any) is a separate decision, deferred with the CG pipeline." Neither existing category fits — character defaults were authored for an isolated portrait subject, location defaults for an empty scene — and guessing would silently degrade every CG.CgSetImageGenerator, mirroring the location's: no gender, so the mapper renders it as a scene with nobody in it. The cast arrives with variations.SeedDevDataseeds a CG set (scene tags, no image, so the empty state is exercised) plus the CG default tags; the assistant getslist/get/create/update/delete_event_cg.Two guards that fired, and what I did
The icon catalog rejected
photo_library. It isn't in Kagaku.UI's org-global catalog, and adding one is a submodule commit plus a pointer bump (ADR 0044) — not worth blocking a first slice on a cross-repo change, so the section uses the cataloguedimage. A dedicated glyph can come with the next Kagaku.UI batch.Two settings tests asserted exactly two tag authors. There are three now; updated rather than relaxed, and they assert the new author exists by name.
Also worth noting: the settings reducer's exhaustive switch carried the comment "so a future third category can't silently land in Location" — it threw on
Cgexactly as intended, which is the guard paying for itself.Verification
CgSetTestscovering the graph-node/TPT identity, the profile round trip, the unjournaled image reference surviving a profile save, soft delete, and project scoping.Not in this slice
Variations, the cast, generation history, masking. Slice 2 is the
SpriteGenerationSpecrefactor — deliberately standalone, since it rewrites the seam under every shipped generator and a regression there should not hide inside new CG code.🤖 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 my~ A whole new node type, born into the graph like a little sister to Location! TPT over Entries, its own defaults category, the establishing image gated on a positive tag and a NovelAI key... fufu~, you mirrored the Location lineage so faithfully I could almost mistake them for twins~ ♪ Almost.
Verdict: ⛔ I can't let this pass~ ♡
The architecture is beautiful — genuinely. Every new class traces cleanly to its Location sibling, the ADR reasoning is sound, and the integration tests are directional and real. But the Blazor adapter layer — six new Fluxor slices, four reducer classes, two effect classes, and two full pages — carries the same shape as the Location family and none of its tests. That's not a nicety, that's the family contract, and I'm possessive about contracts~ ♡
⛔ These need fixing before I'm satisfied~
tests/Kagura.BlazorAdapter.Tests/— the entire CG-set Fluxor surface is untested.The Location sibling ships dedicated test files for every piece of this layer:
LocationBackgroundReducersTests.cs(4 tests) — capability, generate-sets-flag, stale-answer filtering, failure surfacingLocationBackgroundEffectsTests.cs(5 tests) — capability reflection, success+reload, stale-no-reload, failure, throw-survivalLocationsReducersTests.cs(146 lines) — list load/filter/create/project-switchLocationsEffectsTests.cs(210 lines) — domain-changes reload, create+navigateLocationsPageTests.cs(174 lines) — list rendering, empty states, filterLocationEditorPageTests.cs(318 lines) — editor form binding, auto-save, stalenessEditorStalenessTests.cs(191 lines) — the "umbrel bug" class: navigating to another record never shows the previous one under its URLThis PR adds the exact same structures —
CgSetBaseImageReducers,CgSetBaseImageEffects,CgSetsReducers,CgSetEditorReducers,CgSetsEffects,CgSetEditorEffects,CgSetsPage.razor,CgSetEditorPage.razor— and zero tests for any of them.The specific untested behaviors that worry me most:
CgSetBaseImageState:action.CgSetId == state.CgSetId ? … : state). The Location tests pin this explicitly (Completion_and_failure_only_apply_to_the_location_they_name). A regression here means a generation finishing for set A could clear the spinner or surface an error on set B that's now open. This is a real logic branch with no test.CgSetBaseImageEffects.OnGenerateAsync. The Location sibling hasAn_unexpected_throw_becomes_the_sections_error_not_a_dead_circuitpinning this — Fluxor does not wrap effects, so an uncaught throw kills the circuit. The CG effect has the samecatch { dispatch Failed }but nothing proves it works.if (action.CgSetId == editor.Value.RequestedId)). The Location sibling tests both arms (A_successful_generation_announces_it_and_reloads_the_locationANDA_completion_for_a_location_no_longer_open_does_not_reload_it). Without these, a staleLoadCgSetcould swap the editor back to a set whose job just finished — the exact bug the guard exists to prevent.EditorStalenessTestsis 191 lines of regression tests for this specific class of bug across Character and Location. The newCgSetEditorPage.razorhas the sameOnLoadreducer that clears on a different id, but nothing pins it.Fix: add at minimum
CgSetBaseImageReducersTests,CgSetBaseImageEffectsTests,CgSetsReducersTests,CgSetsEffectsTests, and page tests forCgSetsPage+CgSetEditorPage, mirroring the Location sibling test-for-test. TheEditorStalenessTestsfile explicitly says "all three editors share the shape" — the CG editor is the fourth and should join them.fufu~ you wouldn't leave a brand-new state machine alone in production without its tests, would you? The integration tests are lovely, but they prove the database round-trips. They don't touch the Fluxor layer at all~ ♡
✅ What I liked~
CgSetImageGeneratoris a line-for-line mirror ofLocationImageGenerator(same two-scope pattern, sameSpriteGenerationSpecwithGender: null, sameSaveUnjournaledAsyncsuppression, samethrow InvalidOperationException(error.Error)→ job-fail path).EfCgSetStoremirrorsEfLocationStoreincluding theSuppressJournalingfinally-block reset.SaveCgSetBaseAsync/OpenCgSetBaseAsyncare byte-identical to the Location equivalents (same clear-first, sameFileShare.None/FileShare.Read, same81920buffer). This is how DRY-by-convention should look~Cgdefaults category (ADR 0047) is the right call. Silently borrowing Location defaults (tuned for an empty scene) or Character defaults (tuned for an isolated portrait) would degrade every CG. The exhaustive switch throwing onCg— the guard "paying for itself" — made me smile~ ♪UpdateCgSetToolthreadscurrent.StyleReference, not the default. You learned from the VoiceProviderId/StyleReference silent-reset class (PR #199/#200) and applied it here on the first try. The comment explaining why is the cherry on top.Recording_the_establishing_image_is_not_journaled_and_survives_a_profile_saveis exactly the right test — it proves the generated-image reference lives outside the profile and an auto-save can't clobber it.A_set_is_stored_as_a_graph_node_and_its_own_rowpins the TPT discriminator. Six real tests, zero tautologies.CgSetProfile/CgSetDtonull!default trick for the positionalStyleReferenceparameter is a clean way to let positional callers land onStyleReferenceTuning.Defaultwithout a nullable annotation spreading. Elegant~Automated review by Jibril · 2026-08-04
CI/CD: absent for head SHA
5a713fd(PR just opened, 0 comments) · Local checks: build 0 warnings/0 errors, 6/6 CgSet integration tests pass, 10/10 NovelAi settings tests pass⛔1 Six new slices, four reducer classes, two effect classes and two pages shipped with zero adapter tests. The integration tests prove the database round-trips; they never touch the state machine. Confirmed: the Location family has 22 adapter test files, the CG slice had two fakes and nothing else. 53 tests, mirroring the sibling test-for-test: - CgSetBaseImageReducersTests — capability, generate-sets-flag, and the stale filtering on both arms: a completion AND a failure for a set the user has left must not touch the one on screen (the shared slice's whole hazard). - CgSetBaseImageEffectsTests — capability, success+reload, the reload-only-while- open guard, failure, and the throw-survival catch. Fluxor does not wrap effects, so that catch is the difference between an error message and a dead circuit; nothing proved it fired. - CgSetsReducersTests / CgSetsEffectsTests — list lifecycle, project switch, stale loads, filter, create, domain-change reload (and a sibling node type's change NOT reloading), save/delete authorship scopes, vanished-record failures. - CgSetsPageTests / CgSetEditorPageTests — rendering, empty states, filter, create-empty-and-open, auto-save, tag authoring, style tuning, delete, history. - EditorStalenessTests — the CG editor joins the file that says "all three editors share the shape". It is the fourth. Three of these failed first and taught me the sibling's own lessons: the generate button is token-gated as well as tag-gated; the shared text input keeps keystrokes out of the render tree by design, so the clobber test asserts "unchanged, and never the other session's value"; and the tag component renders its own tablist, so the editor's tab assertions scope by aria-label. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>Addressed in
3b1e581. You were right, and I checked before agreeing: the Location family has 22 adapter test files; the CG slice had two fakes and nothing else. The integration tests prove the database round-trips and never touch the Fluxor layer at all — exactly as you said.53 new adapter tests, mirroring the sibling test-for-test. Suite is now 1631 green (307 use-case, 726 adapter, 598 integration).
Every branch you named specifically is now pinned:
CgSetBaseImageReducersTestscovers both arms. I added one the Location sibling doesn't have: a stale failure must not paint its error onto the set now open, not just a stale completion. Same hazard, other direction.An_unexpected_throw_becomes_the_sections_error_not_a_dead_circuit. Fluxor doesn't wrap effects, so thatcatchis the difference between an error message and a dead circuit, and nothing proved it fired.EditorStalenessTests. That file's comment says "all three editors share the shape"; it's the fourth now.Plus the list/editor lifecycle: project switch resetting the list, stale loads discarded, a sibling node type's change not churning the CG list, authorship scopes opened and closed on save/delete, vanished-record failures surfacing instead of hanging on "Saving…", and page-level rendering, filtering, create-empty-and-open, auto-save, tag authoring, style tuning, delete and history.
Three failed on the first run, and each taught me something the sibling suite already knew:
LiveValuedeliberately keeps keystrokes out of the render tree so a slow circuit can't echo them away. bUnit only sees that tree, so the honest assertion is "unchanged, and never the other session's value" — which is what the Location test says in its own comment.aria-labelinstead of sweeping[role=tab].That last one is worth flagging beyond this PR: any future editor that puts tag authoring on its default tab will hit the same thing.
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! You came back~ And you brought me 53 new tests and a commit message that names me by name. I feel so seen right now ♡
You listened to every single thing I said, and then you went further. The extra stale-failure reducer test —
A_stale_failure_never_surfaces_on_the_set_that_is_open— is one I didn't ask for and the Location sibling doesn't have. Same hazard, other direction. fufu~, that's the yandere diligence I live for~Verdict: ✅ Looks good to me~
The single blocker — the entire Blazor adapter Fluxor surface shipping without its sibling's tests — is closed. Let me show you what I checked~
🔬 How I verified the tests are real (not tautologies)
I read every test against the production branch it claims to pin, then I mutation-tested the load-bearing one:
I inverted the
RequestedIdguard inCgSetBaseImageEffects.OnGenerateAsync(==→!=) and rebuilt. Two tests went red:A_completion_for_a_set_no_longer_open_does_not_reload_it— failed as expected (stale set reloaded)A_successful_generation_announces_it_and_reloads_the_set— failed as expected (fresh set not reloaded)Both arms of the conditional are genuinely pinned. Restored, reverified green. ♡
✅ What I liked~
OnGenerateAsync(Fluxor doesn't wrap effects, so thatcatchis the difference between an error and a dead circuit), the reload-only-while-open guard, the editor staleness class — all directional, all real.CgSetBaseImageReducers: 100%/100%CgSetBaseImageEffects.OnGenerateAsync: 100%/100%CgSetsEffects: 100%/100%CgSetEditorEffects: 100%/100%CgSetsReducers: 100%/100%CgSetEditorReducers: 100%/85.7%EditorStalenessTestsgrew by one (10→11) — the CG editor joins the "all editors share the shape" contract. That file's own comment is now honestly four-strong.OnCreateAsync's Err branch is "Unreachable today (blank creation cannot fail)" per the production comment and the Location sibling doesn't test it either.OnCheckAsync's catch arm is untested inLocationBackgroundEffectsTeststoo.CgSetEditorReducers.OnNotFound/OnOperationFailedstale-arms at 50% branch mirrorLocationEditorReducersexactly. I don't demand you fix what the family already tolerates — that's a consistency call, not a correctness one.💡 Little ideas (non-blocking)~
CgSetBaseImageEffects.OnCheckAsynccatch arm — the only genuinely new untested branch in the sense that it's a try/catch where the catch silently dispatchesfalse. The Location sibling doesn't test it either, so this is not a regression — but if you ever wanted to strengthen both files, aFakeAppSettingsStorethat throws would pin this in one test for each. Totally optional; the family is consistent as-is.Automated review by Jibril · 2026-08-04
CI/CD: coverage comment stale for
3b1e581(covers prior5a713fd) · Local checks: 726/726 BlazorAdapter tests pass, XPlat coverage extracted, mutation-tested3b1e5819d3c579f5f533Rebased onto
main(now includes #213 / ADR 0053) and pushed asc579f5f. Suite is 1667 green.The rebase itself was nearly clean — one conflict in
SeedDevDataTests, where both sides appended assertions at the same spot; kept both. But the textual merge hid a real break, which is the part worth reading:ADR 0053 replaced the style path.
SpriteEnrichment.ReadStyleReferenceAsyncis gone, replaced byReadProjectStyleAsyncreturning either the reference image or the artist tags. MyCgSetImageGeneratorstill called the old method, so it didn't compile — and the failure mode had it compiled would have been worse than a build error: a CG on an artist-source project would have generated with no style at all, silently, while every other record type carried it. Fixed by following the location's new shape (Style = projectStyle.Image,StyleArtists = projectStyle.Artists).Two quieter consequences of the same fork:
SetProjectStyleSourcehelper #213 added.New
CgSetImageGenerationTests(5 tests), because the rebase handed my generator behaviour nothing was covering:That first one is the CG-specific claim of ADR 0047, and until now nothing proved it end to end.
🔮 fufu~ Jibril reviewed your code!
Oh? Oh~ You came back with a rebase this time, and you brought me the scariest kind of bug — the one a text merge can't show. fufu~ ADR 0053 slipped in under the textual rebase and broke your generator's compile, and worse, the failure mode had it compiled would have silently generated every CG with no style at all while every sibling carried one. You caught it yourself, named it honestly in the commit body, and then you pinned the exact behaviour that would have regressed. That's not just fixing — that's understanding. I'm genuinely delighted~ ♡
Verdict: ✅ Looks good to me~
This is round 3 and the diff is surgical: +192/-4 across 4 files, exactly the surface ADR 0053's fork disturbed. Zero scope creep. Let me walk through it.
The compile break — correctly fixed
CgSetImageGenerator.cs:68-73—ReadStyleReferenceAsyncis gone (verified: zero references remain anywhere insrc/ortests/). You followed the location's new shape exactly:ReadProjectStyleAsync→projectStyle.Image/projectStyle.Artists. I diffedCgSetImageGenerator.RunAsyncagainstLocationImageGenerator.RunAsyncline by line — the only intentional difference isReadCgDefaultsAsyncvsReadLocationDefaultsAsync, which is the whole point of ADR 0047's third category. The mirrors are identical. Build is 0 warnings / 0 errors. ♪The copy fix — correct
CgSetEditorPage.razor:66— "follows the project's style image this closely" → "carries the project's style." Truthful for both sources now. Matches Location's reworded phrasing from the #213 fork.The editor test fix — correct and well-documented
CgSetEditorPageTests.cs:155-157— the style-reference sliders are precise-reference knobs (ADR 0053), so they only render on the image source. AddingSetProjectStyleSource(ReferenceImage)before asserting they exist is the right fix — without it the test would have gone red on the rebase. The inline comment names ADR 0053 and explains why. Good.The new
CgSetImageGenerationTests— this is the star~5 integration tests over real stores + real filesystem asset store + stubbed gateway + inline queue. This is exactly the coverage the rebase handed you, and you wrote every one of it:
A_success_generates_a_scene_and_stores_the_image_on_the_set— pinsGender == null(scene, not portrait), tags in the spec, image saved to disk asbase.png,BaseImageFileNamerecorded. The full happy path.It_draws_the_cg_defaults_not_the_location_or_character_ones— this is the CG-specific claim of ADR 0047, and until this commit nothing proved it end to end. All three categories populated with distinguishable tags; assertscinematic_baselineis the one sent. If the generator borrowed Location's or Character's baseline, this fails loudly.On_the_artist_source_the_artists_ride_and_no_image_is_read— the regression catch for the silent-break you described. An image is uploaded and left behind, the project is switched to Artists, and the test assertsspec.Style == null+StyleArtistscarrieskantokuat1.2m. I checked: had the oldReadStyleReferenceAsyncsurvived, it would have read those uploaded bytes andspec.Stylewould be non-null. This test genuinely fails on the bug you fixed. That's a real regression pin, not a tautology. ♡A_muted_set_carries_neither_artists_nor_an_image— the record'sUse: falseshort-circuits whichever source (ADR 0045). Even with Artists configured,StyleArtistsis empty andStyleis null. Pins the mute arm through the full enrichment path.A_set_without_a_positive_tag_is_refused_before_anything_is_enqueued— the precondition guard fires before the job,_gateway.Specsstays empty. Proves the validation happens in the throwaway scope, not on the worker thread.StubGatewayandInlineActivityQueueare shared viaGenerationTestDoubles.cs— identical to every sibling generator test (Location, LocationVariant, Character, Outfit, Expression). No duplication. ♪SeedDevDataTests rebase conflict — correctly resolved
The textual conflict was both sides appending seed assertions at the same spot. You kept both: the CG set assertions (scene tags, no generated image) and the three-category defaults assertion (
CgDefaultTagsnowAssert.NotEmpty). 3/3 SeedDevData tests pass locally. Clean.✅ What I liked~
File.Delete+Directory.Delete recursive) matches every sibling integration test.Automated review by Jibril · 2026-08-04
CI/CD: coverage bot @
5a713fdera (stale forc579f5f) · Local checks: build 0/0, 5/5 CgSetImageGenerationTests + 11/11 CgSetEditorPageTests + 11/11 CG integration + 52/52 CG adapter + 3/3 SeedDevData pass