feat(assets): event-CG renders join the project asset browser #237
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/cg-asset-browser"
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 owner spotted the gap: the asset browser predates the CG arc — it walked characters and locations and never learned the third image-owning family.
ProjectAssetKindhad no CG kind; a project full of event renders showed none of them.ListCgImageswalks each set the way the character and location walks do (the shared-walk split that keeps lists from disagreeing): one card per variation with a published render of its own — deliberately not the effective one. A render-less sibling shows the base picture everywhere it's staged, but the browser answers "which images exist", and the effective fallback would list one file as many cards. The generation row also vouches for the pointer (a rotated-out render can't become a broken card), and itsCreatedAttimestamps the card — the variation's ownUpdatedAtmoves with every mask stroke (ADR 0050) and would churn the newest-first ordering for no reason.The page gains the Event CGs chip, the widescreen ratio, the
imageplaceholder icon, a link straight to the CG editor, and a thumbnail URL with no cache-buster — each render is its own immutable file (ADR 0049), so the id is the whole cache key, unlike the replace-in-place kinds that need?v=.Proof. 1836 green (323 / 827 / 686). Integration: the establishing shot and a rendered sibling each get one card, the render-less sibling adds no duplicate, newest first by generation time. Page: badge, chip narrowing, editor href, immutable src. Browser-verified on the seeded world with a fabricated render — card, working thumbnail, filter, link all live.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.4%
Kagura.Domain - 94.5%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.6%
n
n
on
ng
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ArtifactTimestampRegex_2
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ProjectRoute_0
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__TabQuery_1
Kagura.Kernel - 90%
Kagura.Server - 81.8%
Kagura.UseCases - 95.5%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! The third image-owning family finally joins the browser~ ♪ The CG arc has been growing so beautifully, and this is the gap-filling piece that makes the asset grid whole.
ListCgImagesmirrors the character and location walks exactly — theinternal CollectAsyncsplit, theNamehelper, the per-set-per-variation traversal. A textbook sibling. I'm giddy~The design decision to list the OWN render (not ADR 0056's effective fallback) is sharp and well-argued: the browser answers "which images exist", and the effective fallback would list one file as many cards. Using
generation.CreatedAtinstead ofvariation.UpdatedAtto avoid mask-stroke churn (ADR 0050) is the kind of detail that shows deep understanding of the domain. And the immutable-URL thumbnail with no cache-buster (ADR 0049) matches every other CG-serving component (PlayerStage.razor:121,CgEditorPage.razor:503,CgSetGallery.razor:94,CgSetsPage.razor:74) down to the character. chef's kiss ♡Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
ListCgImages.cs:30-33— Thegenerations.FindAsync(published, ct) is not { } generationguard is an untested new code path. It's reachable for real:CgGenerationConfigurationhasHasQueryFilter(g => !g.IsDeleted), andICgGenerationStore.SoftDeleteAsyncexists — so a variation'sPublishedGenerationIdcan point at a soft-deleted generation, makingFindAsyncreturn null and triggering thecontinue. The comment even names the scenario ("a render rotated out from under a stale pointer"), yet no test exercises it. The sibling walks (ListCharacterImages,ListLocationImages) don't need this guard because they check a filename on the record itself — but the CG path uses a pointer-to-separate-row pattern (ADR 0049), and the dangling-pointer defense is unique to this walk. A branch that exists but no test exercises is a branch I can't let slide~ fufu~Fix: Add an integration test case where a variation's published generation is soft-deleted (via
ICgGenerationStore.SoftDeleteAsync), and assert that the variation's card does not appear in the asset list. This pins the defensive behavior so a future refactor can't silently break it.💡 Little ideas (non-blocking)~
ListProjectAssets.cs:38-39— The class doc comment lists the walks as "characters → outfits → expressions and locations → variants (each walk lives inListCharacterImages/ListLocationImages)" — now that CG sets are a third walk viaListCgImages, the summary is a touch stale. A quick "+ CG sets (ListCgImages)" would keep it honest.ProjectAssetsPage.razor:9-11— The page comment enumerates "base sprites, outfits, expressions, backgrounds, variants" but not event CGs. Same staleness — a one-word addition keeps the comment and the code in agreement.ListProjectAssetsTests.cs:16-18— The test-class doc lists "base sprite, outfit sprite, expression, background, variant" — now that event CGs have their own test, adding "event CG" to the roster would match.✅ What I liked~
ListCgImageswalk is a perfect structural clone of the sibling walks —internal CollectAsync, theName(title, fallback)helper, theforeachover children with an image-on-disk gate. If you squint, the three files are the same shape with different nouns. That's exactly how it should be~ ♪generation.CreatedAtfor ordering instead ofvariation.UpdatedAt— wonderful insight. Mask strokes (ADR 0050) churnUpdatedAtfor no visual reason; the generation's birth time is when the picture appeared. This keeps the newest-first ordering meaningful.?v=) is consistent with every other CG-serving component in the codebase. I traced all five call sites — they're identical.Event_cg_renders_join_the_browser_once_each_and_a_render_less_sibling_stays_out) is excellent: establishing shot + rendered sibling each get one card, the render-less sibling adds no duplicate, ordering is newest-first by generation time, subtitle and ids all verified. Genuinely directional assertions.ProjectAssetViewrecord extension (three new optionalGuidfields) follows the established "kind says which ids are meaningful" pattern cleanly.Automated review by Jibril · 2026-08-07
CI/CD: absent for head SHA
3110978(PR just opened, no coverage bot) · Local checks: 4/4 integration + 7/7 adapter tests pass (build 0 warnings/0 errors, .NET 10)All four taken, in
2f29ec6:⛔1 — the dangling-pointer guard is pinned. The integration test now soft-deletes the sibling's published render out from under its pointer and asserts the card vanishes rather than breaks (
afterlist carries only the establishing shot). Directional both ways: with the guard removed, the walk would throw on the missing row'sCreatedAt— red either way. You're right that this branch is unique to the CG walk's pointer-to-row shape; that's exactly why it deserved its own arm.💡1–3 — the rosters name the third family. The aggregator's doc comment now reads "characters → outfits → expressions, locations → variants, and CG sets → variations" with
ListCgImagesin the walk list (and notes only the first two are shared with the preview pickers); the page comment and the test-class doc both list event CGs alongside the kinds they already named.Full suite green: 323 / 827 / 686.
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! You came back so fast — and you brought exactly what I asked for~ ♪ The dangling-pointer guard finally has its own arm, and the three stale rosters now name the third family. A textbook response to review feedback. I'm beaming~
I verified every claim, naturally. The integration test soft-deletes the sibling's published render out from under its pointer and asserts the card vanishes — and it's directional both ways: without the guard, the walk dereferences
generation.CreatedAton a null reference (the EFHasQueryFilter(g => !g.IsDeleted)inCgVariationConfiguration.cs:61and the fake's!g.IsDeletedfilter inFakeCgGenerationStore.FindAsyncboth make the row genuinely disappear). Red without the guard, green with it. That's a real test, not a coverage-tick. ♡And the three doc fixes are exactly right — the aggregator's walk list now reads "characters → outfits → expressions, locations → variants, and CG sets → variations" with
ListCgImagesnamed alongside its siblings (and correctly notes only the first two are shared with the preview pickers). The page comment and test-class doc both carry "event CG" too. Consistency is love~Verdict: ✅ Looks good to me~
✅ What I liked~
ListCgImageswalks the pointer-to-row pattern (ADR 0049) that's unique among the three image-owning families, and thegenerations.FindAsync(published, ct) is not { }defense against a rotated-out render is the kind of detail that separates a working feature from a correct one.3110978and2f29ec6— theListProjectAssetsandProjectAssetsPagediffs are comment-only. Zero behavioral drift, exactly as a review-response commit should be.ListCgImagesmirrorsListCharacterImages/ListLocationImagesin every detail (theinternal CollectAsyncsplit, theNamehelper, the per-set-per-variation traversal). The immutable-URL thumbnail (no?v=) matches every other CG-serving component. The OWN-render-not-effective decision (ADR 0056) is sharp. chef's kiss ♡Automated review by Jibril · 2026-08-07
CI/CD: forgejo-actions coverage comment present for prior head
3110978(ListCgImages 100%/87.5% — the missing branch was this guard; now exercised) · Local checks: build 0/0, integration test 1/1 pass [24s], page test 1/1 pass [10s]