feat(cg): any sibling's render is a valid retouch source (ADR 0058) #235
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/cg-sibling-source"
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 asked for the establishing shot's last privilege to go: a moment should be able to start from any sibling's picture, not only the scene's base — "After the rain" begins as "First refusal" with the umbrella gone.
ADR 0058 (amends 0057). Of the three shapes considered — this widening, a branch/duplicate verb, and a live per-variation base pointer — the pointer was rejected outright (cast chain-walking, cycles, dangling bases; the establishing shot is undeletable precisely so ADR 0055/0056 never face those questions), the branch verb stays open for later, and this is the cheap true one: a named retouch source is valid when it belongs to any variation of the same set. No pointers are stored — a sibling source is a per-gesture choice, nothing to walk, cycle, or dangle. Everything else of ADR 0057 stands: named source, enqueue capture, effective-render fallback, the result appending to the retoucher's own timeline.
The guard in both retouch pipelines and the vision scoping moves from own-plus-establishing to the set boundary — the source's variation is loaded and its
CgSetIdcompared; another set's row is still another picture, and still silently wrong ("That render belongs to another CG"). Proven end to end: the establishing shot builds from a sibling's retouch (the sibling's bytes on the wire, the result on the establishing timeline), while a fresh set's render is refused by name before the queue by all three consumers, and the scoping judges from a sibling's row without the model ever seeing a foreign one.The editor. The strip's head gains a source picker listing the set's other variations with a published render (the establishing shot stays out — its render is already the base entry). Choosing one projects that render onto the canvas exactly the way the base entry projects: a display-only row in the strip, badged with the sibling's title, publish/pin/delete asleep because its verbs live in its own editor, re-flagged unpublished because the dot does too. The mask rides it and the one verb builds from it — the shown-render rule, unchanged, with a wider catchment. A reload is fresh truth (the projection is re-picked, not carried blind), a sibling whose publish vanished fails with the reason, and only rendered siblings are listed.
Proof. 1828 green (323 / 820 / 685). Reducers (projection joins the strip, takes the canvas, clears on reload, stale-guarded), effects (published render re-flagged, titled, vanished-publish named; the load lists only rendered siblings), page (pick → badge → canvas → verbs asleep → Generate retouches from exactly that row), and the browser on the seeded world with two fabricated renders: the picker offers "From First refusal", picking swaps the canvas, both projections sit badged with Publish asleep.
🤖 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 establishing shot's last privilege, surrendered — "After the rain" begins as "First refusal" with the umbrella gone. That is a beautiful widening, and you did it by widening the guard, not by bolting on pointers or branch verbs. The ADR 0058 reasoning is exquisite: the pointer shape was rejected for exactly the chain-walking/cycle/dangle hazards that the undeletable establishing shot exists to prevent, and the branch verb is left open rather than half-built. No pointers stored, no relationships to dangle — a sibling source is a per-gesture choice. Chef's kiss. ♡
And the symmetry across all THREE guard sites —
RetouchAsync,RetouchRegionAsync,ScopeCgCutout.ExecuteAsync— is identical down to the comment phrasing. Every one moves from own-plus-establishing to the set boundary. Every one loads the source's variation and comparesCgSetId. That's how you keep three pipelines honest. ♪Verdict: ✅ Looks good to me~
I traced every changed line, read every sibling, ran the suite, and found nothing that produces wrong runtime behavior. This is clean. Let me show you what I checked~
✅ What I liked~
The guard is correct at the null edge.
sourceVariation?.CgSetId != variation.CgSetId— whensourceVariationis null (the variation was deleted between the source generation being created and now),null.Guid != realGuidistrue, so a dangling source variation is correctly refused as "belongs to another CG." The oldestablishingshortcut is gone and nothing regressed:source.CgVariationId != variation.Idstill short-circuits the own-variation case, and the cross-set case still fails before_gateway.Specsgrows. The integration testA_siblings_row_builds_and_another_sets_row_is_refusedproves all three consumers (whole retouch, region retouch, scoping) refuse a foreign set's row, AND it asserts_gateway.Specs.Countis unchanged — the refusal is before the queue, not a silent fallback. Sharp.The wire carries the sibling's bytes, the result lands on the retoucher's timeline. The integration test asserts
_gateway.Specs[^1].Inpaint!.ImageData == siblingPng— the sibling's PNG is what got inpainted, and the establishing timeline gets the result. That's exactly ADR 0058's promise: the merged image and frozen snapshot stay regardless of what happens to the sibling later. ThesourceVariationId = source.CgVariationId/sourceFileName = source.FileNamecapture in both pipelines correctly reads from the source generation, soOpenCgVariationGenerationAsyncopens the sibling's file path. No path confusion.The projection idiom is a faithful mirror of the base entry.
SourceEntryjoinsTimelineEntriesright afterBaseEntry, getsPublished = falsere-flagged (the dot belongs to the sibling's editor, exactly like the base entry's comment atCgEditorState.cs:541-542), is excluded from publish/pin/delete viaisBase, and clears on reload ("fresh truth, not carried blind"). The reducer testA_picked_source_takes_the_canvas_and_a_reload_or_clear_returns_itcovers pick→canvas, clear→fallback, reload→null, AND stale-variation guard. Four branches, four assertions. fufu~ you tested every arm~The scoping guard widening is consistent.
ScopeCgCutoutnow passessource.CgVariationIdtocutouts.RenderAsync(line 152) — the cutout image handed to the model comes from the source variation's file, not the retoucher's. The vision testAnother_sets_render_is_refused_as_a_source_but_a_siblings_is_judgedasserts the sibling judges from the establishing row (same set, allowed) and a second set's render is refused with_llm.Requests.Countunchanged — the model never sees a foreign picture. Correct and proven.The source picker UX is thoughtful. The establishing shot is excluded from siblings ("its render is already the base entry"), only
PublishedGenerationId is not nullvariations are listed, theGuid.Emptysentinel handles the librarySelect's non-null requirement, and the vanish case (published is nullafter unpublish) fails with a named reason. The(unnamed)fallback for empty titles is consistent acrossSourceOptions,OnPickSourceAsync, and the badge. ♪DI wiring is complete.
ListCgVariationsis registered inDependencyInjection.cs:85, added to theCgEditorEffectsprimary constructor, and the test fixture atCgEditorEffectsTests.cs:46constructs it with the realCgRenderResolver. No missing service, no test-only stub.💡 Little ideas (non-blocking)~
CgEditorLoaded.Siblingsdual-assignment pattern (CgEditorState.cs:134-136) — the record has both a constructor parameterIReadOnlyList<CgVariationDto>? Siblings = nullAND aninitpropertySiblingsthat defaults from it. This works (theinitshadowing the constructor param is valid C#), but it's the only action in this file with that shape — every otherCgEditorLoadedfield is a plain positional. A reader has to pause to understand whySiblingsgets special treatment. Not wrong, just slightly asymmetric; the null-coalescing is belt-and-suspenders since the caller always passes a concrete list now. Purely cosmetic.Automated review by Jibril · 2026-08-07
CI/CD: absent for head
f109abb(PR just opened, 0 comments) · Local checks: build 0 warnings/0 errors, 12/12 integration tests pass (CgRetouch + CgVisionScoping), 77/77 BlazorAdapter CgEditor tests pass💡1 taken, in
c783c00—Siblingsis a plain positional now, like every neighbour. The nullable-parameter-plus-shadowing-init shape only existed to keep pre-widening call sites compiling; with every caller passing a concrete list, the asymmetry bought nothing, so the call sites pass[]explicitly and the record reads flat again. Adapter suite re-run green (820).🔮 fufu~ Jibril reviewed your code!
Oh? A clean flat record, and every call site honest. Exactly the shape I asked for~ ♡
Verdict: ✅ Looks good to me~
This is a surgical
+5/-8across exactly 2 files (1 src, 1 test) — pure cleanup responding to non-blocking 💡1 from my last pass. Let me verify~✅ What I checked~
The record is flat now.
CgEditorLoaded'sSiblingsparameter is a plain required positional — no nullable default, no body, no shadowinginit. TheIReadOnlyList<CgVariationDto>? Siblings = null+Siblings { get; init; } = Siblings ?? []shape is gone, and the record reads like every neighbour. Clean.Every call site passes a concrete list. I checked all 4
new CgEditorLoaded(sites: the production effect (CgEditorState.cs:558) passessiblings, and all 3 test sites inCgEditorReducersTests.csnow pass[]explicitly. Zero compile errors. The adapter suite confirms: 820/820 pass locally atc783c00(matching your claim, matching CI's 820 from thef109abbcoverage comment).Zero behavioral drift. Stripped comments aside, the only change is removing a default + its shadowing init.
Siblingsis still non-null at every construction site (was[]by default before, now[]explicit at tests, real list at production). TheOnLoadedreducer still receives and storesaction.Siblingsidentically. Nothing moved in the guard, the projection, the picker, or the three pipelines. My full architectural review from the previous pass stands unchanged.CI: forgejo-actions coverage bot present for
f109abb(1828 green, 96.1%/77.3% — 323/820/685). Thec783c00delta is compile-and-test-only (record shape fix), verified locally.Automated review by Jibril · 2026-08-07
CI/CD: passed for
f109abb(stale forc783c00) · Local checks: 820/820 adapter tests pass atc783c00