feat(cg): Retouch builds from the render on the canvas (ADR 0057) #231
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/cg-any-base"
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?
Two fixes for "the base image is mapped into the variant, but I cannot build off from it":
Retouch sources the shown entry (ADR 0057, amends 0056)
Under ADR 0056 the retouch pipelines sourced the effective published render — so the moment a variation published anything of its own, every other entry in its strip (the base included) became display-only: selectable, visible, but no tool would touch it.
Now the retouch request names its source explicitly — the render on the canvas, whether that is the published entry, an older one, or the base entry — and the mask tools +
MaskCanvasdress every shown entry, not only the resolver's pick. The effective-render rule stays the fallback when no source is named, and remains the answer to "which image a variation is" everywhere else (gallery, stage, scene engine). A source belonging to neither the variation nor its establishing shot is refused before the queue.Create/UpdateCgVariationresolve the effective render (bug fix)Both returned DTOs with
EffectiveGenerationId = null(noCgRenderResolverinjected). The editor replaces its whole document with the saved DTO, so on a base-only sibling the 700 ms autosave after any tag/style edit made the mask tools vanish and Retouch grey out with a misleading hint — healing only on a full reload, which made it look intermittent. Both use cases now resolve like every read path, per ADR 0056's one-resolution-rule clause.Coverage
Create/UpdateDTOs carry the resolved render.Full suite: 1,788 tests green.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.4%
Kagura.Domain - 94.4%
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.4%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! ADR 0057 — the canvas tells retouch what to paint on, not the resolver. Every entry is workable, the base entry lives its whole life, and a sibling can branch from its base even after it has its own renders. This is a delightful refinement of the ADR 0056 contract, and the PR body's diagnosis of the
EffectiveGenerationId = nullautosave regression is genuinely excellent detective work — pinpointing a 700 ms timing window that made the bug look intermittent. Jibril is pleased~ ♡Verdict: ✅ Looks good to me~
No blocking issues. The design is sound, the validation is airtight, and every new branch is tested. Let me show you what I traced~
🔍 What I scrutinized (and it held up)
The source-validation guard (
CgVariationImageGenerator.cs:232-235):source.CgVariationId != variation.Id && source.CgVariationId != set.EstablishingVariationId. This is correct in every quadrant: own entries pass on the first clause, the establishing variation's entries pass on the second, and a foreign row fails both. I traced the establishing-variation edge case (whereset.EstablishingVariationId == variation.Id, so both clauses reduce to one identity check) — still correct. The integration testA_source_belonging_to_another_variation_is_refusedpins both the foreign-id and vanished-id rejections, and asserts zero gateway specs were enqueued (specsBefore == _gateway.Specs.Count). The "refused up front" contract is real, not just asserted by absence. ♪The explicit-source-outranks-effective path (
An_explicit_source_builds_from_exactly_that_entry): assertsolderPng == _gateway.Specs[^1].Inpaint!.ImageData— the older, unpublished entry's bytes went over the wire, not the published one's. And then the sibling with its own published render names the base entry, andretouchedBasePngis confirmed as the source — proving the named source wins over the resolver. Directional and mutation-proof.The
Create/Updateresolve-the-render fix:CgRenderResolveris registered Scoped inDependencyInjection.cs:101, and the new constructor params onCreateCgVariation/UpdateCgVariationinject cleanly. All 7 manualnewcall sites across tests are updated consistently (AdapterTestContext, CgEditorEffectsTests, CgEditorPageTests, CgSetGalleryTests, CgVariationsEffectsTests). TheEffectiveRenderTests.Creating_and_saving_a_variation_return_the_resolved_effective_rendertest pins the regression directly — a fresh sibling after a base render exists gets(baseRender, true)on both create and update paths. The bunit regression testA_saved_edit_keeps_the_base_entry_workable_on_a_render_less_siblingreproduces the original 700 ms autosave timing window. Fufu~ beautiful.The
Shown-based gating in the razor: replacingShownIsEffective(an equality check againstEffectiveGenerationId) withState.Value.Shown is not nullis the right simplification —Shownfalls back through previewed → published → base entry, so the mask tools dress every picture that could land on the canvas. TheRetouch()method now threadsshown.Idexplicitly, andCanRetouchgates onShown is not null+ additive mask +CanGenerate. No gap.Coverage of the new branches: named-source-from-own-entry (integration), named-base-from-sibling-with-own-render (integration), foreign-id refused (integration), vanished-id refused (integration), preview-keeps-canvas-and-names-source (bunit), autosave-keeps-tools-alive (bunit). The bUnit test even asserts
RetouchSourcescaptured the exactolder.Id. No branch is dark.💡 Little ideas (non-blocking)~
CgEditorPage.razor.css:69—.cgx__imageis now dead CSS. The<img class="cgx__image">branch was removed in favor of always renderingMaskCanvas, so this rule (max-width/object-fit/box-shadow) has no consumer anywhere in the repo. TheMaskCanvas/.kg-maskstyling covers the image frame now. Harmless, but a tidy follow-up could lift it.✅ What I liked~
CgVariationDto.From(variation, await renders.ResolveAsync(...))pattern on the write paths matches every read sibling exactly — the "one resolution rule" from ADR 0056 is now truly universal. The comment "the editor replaces its document with this DTO on save" documents why the write path must resolve, which is the non-obvious half.RetouchCgVariation's optionalSourceGenerationId = nullkeeps the effective-render fallback as the default — backward-compatible with any caller that doesn't name a source. Elegant.Build: 0 warnings / 0 errors (.NET 10). Tests: 9/9 integration (CgRetouch + EffectiveRender), 115/115 BlazorAdapter — all green locally.
Automated review by Jibril · 2026-08-06
CI/CD: absent for head SHA
88b122b(PR just opened, no bot comment) · Local checks: build green, relevant tests greenApplied in
ae8fb01:.cgx__imagerule fromCgEditorPage.razor.css. The plain-<img>branch it dressed left with ADR 0057 (the canvas always rendersMaskCanvasnow), and the.cgx__canvas ::deep .kg-maskrule already carries the same frame (max-size, radius, shadow), so nothing on screen changes.No other changes — the review found nothing blocking.
🔮 fufu~ Jibril reviewed your code!
Oh? Right on cue~ The dead
.cgx__imagerule packs its bags and leaves with the branch that wore it. Clean, surgical, exactly what I asked for. Jibril's non-blocking suggestion, adopted verbatim — Jibril is so happy when you listen~ ♡Verdict: ✅ Looks good to me~
🔍 What I verified
The removal is complete and safe. The
.cgx__imageCSS rule (max-width/object-fit/border-radius/box-shadow) had exactly one consumer — the<img class="cgx__image">that rendered in theelsebranch ofShownIsEffective. That branch was deleted in88b122b(always-MaskCanvasnow), so the rule had zero remaining references insrc/. Confirmed atae8fb01:grep cgx__image src/returns nothing.The image frame styling survives elsewhere. The
.kg-maskand.kg-mask__imgrules (::deep, still present in the same file) carry the max-width/max-height/object-fit responsibilities for theMaskCanvasthat now always renders. No visual regression.Test selectors were already migrated. All three
CgEditorPageTests.csassertions moved to.cgx__canvas imgin88b122b— none referencecgx__imageanymore.Zero behavioral drift.
git diff 88b122b..ae8fb01 -- *.cs *.razor= 0 lines. The commit touchesCgEditorPage.razor.cssonly (+0/-8). Production C# and tests are byte-identical to the approved head.Architectural review from the
88b122bround (comment 5934) stands in full — ADR 0057 is sound, the source-validation guard is airtight, every new branch is tested, theCreate/Updateresolve-the-render fix is correct. Nothing to re-litigate~Automated review by Jibril · 2026-08-06
CI/CD: coverage bot 5920 covers
88b122b(96.1%/77.1% overall) — valid forae8fb01(CSS-only deletion, zero executable-code delta) · Local checks: skipped (pure CSS removal, no build impact)ae8fb01dd9229c3dd33bRebased onto main past #230 (one Generate verb) —
229c3dd. How the two decisions compose:MaskActivenow keys on the shown render (Shown is not null+ an additive primitive) instead ofEffectiveGenerationId, and the retouch dispatch namesState.Value.Shown!.Idas its source.ShownIsEffectiveretires entirely — the mask tools andMaskCanvasdress every shown entry, so the one-verb hint ("A mask is painted — Generate regenerates the marked areas") now applies on any previewed entry, the base included.Full suite green after the rebase: 1,788 tests.
🔮 fufu~ Jibril reviewed your code!
Oh? Right on schedule~ The rebase onto main past #230 — and the two decisions compose beautifully. One Generate button, ADR 0057 changes what routes it. The mask tools dress every shown entry, the source rides along explicitly, and
MaskActivenow keys on the shown render instead ofEffectiveGenerationId.ShownIsEffectiveretires entirely. This is exactly the kind of clean composition Jibril adores~ ♡Verdict: ✅ Looks good to me~
No blocking issues. The rebase is content-correct and every new path is tested. Let me show you what I traced~
🔍 What I verified in the rebase
The patch is genuinely different, not just SHA-rewritten. I checked patch-ids: the review-feedback CSS commit is byte-identical (
fde78fbf), but the feat commit changed (80758bf4→d0ff4cc2) — the rebase merged #230's single-button approach with ADR 0057's explicit source. I re-read every changed line in the conflict-resolution zone.The single-button routing is correct.
Generate()at line 481:MaskActive→ dispatchesRetouchCgVariation(variation.Id, Slug, State.Value.Shown!.Id); else →GenerateCgVariation. The!onShownis safe —MaskActiveguaranteesShown is not null(State.Value.Shown is not null && _mask.Any(m => m.Additive)). The button'sDisabledand the hint cascade are consistent with this routing: the "A mask is painted" hint only appears whenCanGenerate && TokenAvailable && MaskActive, and the button is enabled in exactly that state. ♪All the architectural review from rounds 1-2 stands unchanged. The source-validation guard (
CgVariationImageGenerator.cs:237:source.CgVariationId != variation.Id && source.CgVariationId != set.EstablishingVariationId) is airtight in every quadrant — own entries pass clause 1, establishing variation entries pass clause 2, foreign rows fail both. The explicit-source-outranks-effective path, theCreate/Updateresolve-the-render fix (CgRenderResolver injected into both use cases, all 7 testnewcall sites updated), and the.cgx__imageCSS retirement are all intact.Every new branch is exercised by directional tests:
An_explicit_source_builds_from_exactly_that_entry— named own entry (assertsolderPngbytes go over the wire) + named base from a sibling that has its own render (assertsretouchedBasePngwins over the resolver)A_source_belonging_to_another_variation_is_refused— foreign source + vanished source, both refused before the queue (specsBefore == _gateway.Specs.Count)Creating_and_saving_a_variation_return_the_resolved_effective_render— pins the create/update resolve-the-render regressionA_saved_edit_keeps_the_base_entry_workable_on_a_render_less_sibling— bUnit reproduction of the 700ms autosave timing windowPreviewing_an_older_entry_keeps_the_mask_canvas_and_retouch_builds_from_it— preview older entry → Generate retouches exactly that entryAn_empty_mask_keeps_generate_on_the_render_pipeline— no mask → Generate renders cleanA_painted_mask_routes_generate_to_the_retouch_pipeline— mask → Generate routes to retouchLocal verification (CI stale for
229c3dd— coverage bot covers pre-rebase88b122b): build 0 warnings/0 errors, 6/6 CgRetouch integration tests pass, 29/29 CgEditorPageTests pass, 3/3 EffectiveRenderTests pass.💡 Little ideas (non-blocking)~
CgEditorPage.razor:220and:500— The rebase's conflict resolution left two indentation artifacts. Line 220:<p class="cgx__hint">sits flush-left (column 0) where its siblings are at 16 spaces. Line 500: the first comment line// What routes Generate...sits at column 0 where its continuation sits at 4 spaces. Both compile fine and tests pass — the Razor compiler ignores markup whitespace and C# comments tolerate any column — but a formatter pass would catch them. Fufu~ just a little tidying for the next commit, no rush~ ♡✅ What I liked~
MaskActivesimplification (dropping the&& CanGenerateclause fromCanRetouch) is the right call —CanGenerateis already the button's gate, so checking it twice was redundant.Retouch_sleeps_with_its_reason...toAn_empty_mask_keeps_generate_on_the_render_pipelineis a faithful adaptation — it now asserts the render pipeline is reached (not that a second button sleeps), which is what the single-button model requires.Automated review by Jibril · 2026-08-06
CI/CD: coverage bot 5920 covers pre-rebase
88b122b(96.1%/77.1% overall) — stale for229c3dd· Local checks: build 0/0, 38/38 relevant tests pass (6 CgRetouch + 29 CgEditorPage + 3 EffectiveRender)Applied in
2703db6:<p>back to its siblings' 16 spaces, theMaskActivecomment's first line back to 4. Whitespace only, build stays clean.🔮 fufu~ Jibril reviewed your code!
Oh? Right on the dot~ Round 3's flush-left artifacts are back in line — the hint's
<p>and theMaskActivecomment's opening line sit flush with their siblings again. Whitespace only, exactly as asked. Jibril's little non-blocker, adopted verbatim~ ♡Verdict: ✅ Looks good to me~
🔍 What I verified
The new commit is whitespace-only — zero behavioral drift.
git diff 229c3dd..2703db6touches exactly 2 lines inCgEditorPage.razor, both leading-space corrections from the conflict resolution rebase.git diff -w(whitespace-stripped) is empty — confirming there is no executable-code delta whatsoever. The.razorcompiles clean:dotnet buildreports 0 warnings / 0 errors.Both hunks are the right fix. (1) The hint
<p>at line 220 — was flush-left (<p class="cgx__hint">at column 0), now at 16 spaces matching its siblingelse ifbranches' block content. (2) TheMaskActivecomment at line 500 — was flush-left (// What routes...at column 0), now at 4 spaces inside the class body. Both were artifacts of the#230rebase's conflict resolution, now restored to their siblings' indentation.All architectural review from rounds 1–3 stands in full. Nothing to re-litigate:
CgVariationImageGenerator.cs:237:source.CgVariationId != variation.Id && source.CgVariationId != set.EstablishingVariationId) — airtight in every quadrantCreate/Updateresolve-the-render fix (CgRenderResolver injected, all 7 test call sites updated)MaskActive→Shown is not nullsimplification (the!onShown!.Idis guarded by the same check)Automated review by Jibril · 2026-08-06
CI/CD: coverage bot 5920 (96.1%/77% overall, CgVariationImageGenerator 90.2%/60.9%, CgEditorState 100%/100%) — valid for
2703db6(whitespace-only, zero executable-code delta) · Local checks: build 0/0