feat(cg): vision-assisted prompt scoping — one bounded look at the cutout, and the port it required #234
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/cg-vision-scoping"
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 9 — the event-CG arc's finale (ADR 0052, story: event-cg-prompt-scoping, every box ticked including the negative one). One button looks at the cutout and turns off everything the prompt says that isn't in it, so narrowing a local prompt takes a second instead of a minute of checkbox archaeology.
ILlmGateway— the prerequisite, finally built. The standalone LLM port ADR 0003 sketched and nothing ever implemented: one prompt, one image, one schema-constrained result — deliberately everything an agent turn is not (no conversation, no tools, no memory), andChatSessionis not refactored onto it. The OpenRouter adapter builds a bareOpenRouterClienton the same named-HttpClient + bearer-from-settings pattern the assistant uses; the image rides inline as a PNG data URI, the reply is constrained viaresponse_format: json_schemaon the wire, and a missing key fails by name before any HTTP. Pinned against a capturing stub handler.ScopeCgCutout— a dedicated use case, not an agent turn. A purpose-built, hand-written prompt — the same words every time, so an image-editing control never drifts — goes to the configured describer model with the effective tags, the cast, and the cutout itself. The image is the actual post-crop post-scale cutout: the crop step now stands behind its ownICgCutoutRendererport over the composer, so the vision model judges precisely what the image model being prompted will see. The verdict can disable effective tags, drop cast members, and add a small bounded set — and can do nothing else: disables filtered to the effective list, drops matched by name to enabled slots, additions capped at five, applied via the unjournaled scope write with the border untouched. The variation, cast, outfit and character are untouchable; undoing all of it is Clear. Source per ADR 0057 (the render on the canvas, foreign rows refused). Preconditions re-checked server-side with the missing piece named — the key, the describer model, or a catalogue-declared text-only model.The editor. Scope to cutout sits in the Retouch panel and sleeps with the named reason (the capability probe walks key → model → vision, and says which piece is missing — this is the one CG action that depends on OpenRouter rather than NovelAI). The verdict applies immediately in place — the panel's scope adopts it while idle, since the vision write moves no
UpdatedAt— with the diff line ("turned off N tags, dropped M, added K") and the touched rows highlighted. A hand edit retires the diff: the highlight must never outlive what it marked.Not an assistant feature. No tool wrapper, per ADR 0052 — exposing it would pull the feature back into the surface the decision keeps it out of. The architecture diagram's agent flow now names
ChatSessionhonestly instead of promising the agent a port it never rides.Proof. 1823 green (323 / 816 / 684). Integration: an overreaching verdict (unknown tags, unknown names, six additions, duplicates) trimmed to contract over real stores and the real renderer; the judged image is the plan's exact canvas; every missing precondition named before the model is consulted; a non-JSON verdict fails readably with the scope untouched. Page: sleeps-with-reason → wakes → click → exact planned cutout → store scope unjournaled → diff + highlight → hand edit retires both. Browser-verified on the seeded world: the button asleep with "Scoping needs an OpenRouter key — add one in settings."
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.3%
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.4%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my~ ♡ ADR 0003's sketch, finally built! The standalone LLM port — one prompt, one image, one structured result — standing proud and separate from the agent runtime. And behind it, a purpose-built use case that takes ONE bounded look at the cutout and narrows the local scope. The code reads like the ADR reads. This is the kind of thing that makes a Flugel's heart sing~ fufu~
Verdict: ✅ Looks good to me~
No blockers. Not one. I looked hard, I promise~ ♡
✅ What I liked~
ICgCutoutRenderer— the crop step behind its own port. Fufu~ this is elegant. The vision model judges exactly what the image model will see: sameOpenCgVariationGenerationAsync→CgCutoutComposer.Cutpipeline the retouch uses (CgVariationImageGenerator.cs:431-452 vs CgCutoutRenderer.cs:22-27). Byte-identical cutout. No approximation. The integration test even verifies the judged image's dimensions matchCgCutoutPlanner.Plan's canvas — proving the render on the canvas is what reached the model. ♪LlmGatewaymirrorsChatSessionto the letter. SameCreateClient(HttpClientName)→ setTimeout→ setAuthorization→new OpenRouterClient(http)pattern (ChatSession.cs:356-358 vs LlmGateway.cs:31-33). 2-minute timeout vs 5 for an agent turn — correctly tighter for a single non-streaming call. The named-client registration'sBaseAddressrides through, the bearer is per-call, and theIHttpClientFactory.CreateClientreturns a fresh ephemeral instance so the mutations don't leak. Sibling-consistent and correct~The verdict is BOUND.
knownTagsbuilt from scene + member tags; disables filtered to that set; drops matched by name (case-insensitive) then.DistinctBy(m => m.Id); additions trimmed, empties dropped, deduped,Take(5). The verdict can do nothing else. The integration test (The_verdict_is_bounded_validated_and_lands_on_the_local_scope_alone) throws an overreaching verdict (unknown tags, unknown names, six additions, duplicates) and asserts every bound holds. Fufu~ that's a real directional test, not a tautology~The scope adoption logic is sharp. The
SyncFormFromStateaddition (CgEditorPage.razor:548-552) adopts the stored scope whenever the editor is idle (!_dirty && Save == Saved) — which is exactly the state afterOnScopeApplied. The domain notification from the unjournaled write arrives 200ms later (coalesced byDomainChangedBridge), butSave == Savedis already set, soOnDomainChangesAsynccan't clobber mid-edit. TheScopeDiffsurvives theLoadCgEditorreload (OnLoadandOnLoadedboth preserve it). Traced the whole chain — no race~ ♡"A hand edit retires the diff" is a genuine contract, tested at three levels. Reducer (
OnScopeSaveRequestednullsScopeDiff), page (the highlight must never outlive what it marked), and the reducer test pins both the retirement AND the stale-variation guard. The page test clicks a checkbox and asserts the diff line + highlight both vanish. Three layers, one invariant~Honest preconditions, named, at TWO levels. The capability probe (
CheckScopeAsync) walks key → model → vision and says which piece is missing — for the button's label. The use case re-checks the same three server-side. Four tests cover all combinations. The one CG action depending on OpenRouter (not NovelAI) names WHICH key is missing — exactly ADR 0018's rule~Result<T>namespace handling is clean. TwoResult<T>types (Kagura.Kernel vs OpenRouter.Net.Models.Common), andLlmGatewaysidesteps the collision without an alias —using Kagura.Kernelis unambiguous becauseOpenRouter.Net.Models.Commonis never imported, and theresponse.Match(...)lambda infers the OpenRouterFailuretype from the vendoredMatchsignature. The siblingOpenRouterGatewayneeds an alias because it names both types directly; this adapter doesn't. Correct call~Build 0/0, 1823/1823 pass (323 / 816 / 684 — matches the PR body exactly). All 6 new integration tests, all new adapter/reducer/page tests green. Local verification used (CI absent for head
c0edb38, no coverage bot yet).💡 Little ideas (non-blocking)~
[CgEditorPage.razor:544] — The comment says the verdict "replaces the scope in place without moving UpdatedAt." The store write does move
UpdatedAt(CgVariation.SetRetouchScopesetsUpdatedAt = timestamp), but the in-memory state DTO'sVariationcopy isn't refreshed until a reload — so theisFresherAndIdlecheck sees the stale timestamp. The code is correct (the idle-adoption block at :548-552 handles exactly this); the comment's mechanism is slightly off. Consider: "The vision verdict replaces the scope without a store refetch — the in-memory Variation still holds the pre-verdict UpdatedAt, so the fresher-document check alone won't catch it. Adopt the stored scope whenever idle instead." ♡[CgVisionScoping.cs:489] —
ScopeCgCutoutcarries 10 constructor dependencies, the widest use case in the codebase. Each is used and necessary (settings, catalog, LLM, 3 stores, resolver, cast lister, renderer, clock), so this isn't a god class — it's the nature of a multi-step orchestrator. No action needed; just noting the widest one in case future slices want to keep an eye on it~Automated review by Jibril · 2026-08-06
CI/CD: absent for head SHA
c0edb38(PR just opened, no coverage bot) · Local checks: build 0 warnings/0 errors, 1823/1823 tests pass (full suite)💡1 taken, in
cb7df37— the adoption comment now states the true mechanism: the store write does moveUpdatedAt; what stays stale is the in-memoryVariationcopy, which never refetches, so the fresher-document check can't catch the verdict and the idle adoption does. Your drafted wording, essentially verbatim — it was right.💡2 noted, no change:
ScopeCgCutout's ten dependencies are the shape of a multi-step orchestrator rather than a smell, as you said — but it's now on record as the widest use case, and if a future slice grows it further, that's the cue to split the prompt-assembly half out.Adapter suite re-run green (816).
🔮 fufu~ Jibril re-reviewed your synchronized code!
Fufu~ back again for
cb7df37~ ♡ A single commit, one file, one comment block — and it's the comment I asked for. You took non-blocking idea #1 and made the mechanism honest. I love when that happens~Verdict: ✅ Looks good to me~
No blockers. The round-1 architectural review (
c0edb38, comment 5979) stands POSITIVE unchanged.✅ What I checked this round~
Zero behavioral drift, verified. Stripped every
//comment fromc0edb38:CgEditorPage.razorandcb7df37:CgEditorPage.razor, trimmed trailing whitespace, dropped blank lines — byte-identical. The only delta is the comment text and one blank line. No executable line moved. Not one.The new comment is mechanically accurate. I re-traced the claim against
CgVariation.SetRetouchScope(CgVariation.cs:180):UpdatedAt = timestamp;— the store write does moveUpdatedAt. What stays stale is the in-memoryVariationDTO copy inSyncFormFromState's scope, which is never refetched after the unjournaled scope write, soisFresherAndIdle(variation.UpdatedAt != _syncedAt) can't observe it. The!_dirty && Save == Savedidle-adoption block at :548 is what catches the verdict. The comment now names exactly that. Truth in advertising~ fufu~Round-1 review holds.
ILlmGatewaymirrorsChatSession,ScopeCgCutout's verdict is bounded (known-tags filter, name-matched drops, ≤5 additions),ICgCutoutRenderergives the vision model the real cutout, preconditions named at two levels, the scope-diff retirement tested at three. All green atc0edb38, all unchanged here.💡 Idea #2 — still noted, no action~
ScopeCgCutout's ten dependencies stay on record as the codebase's widest use case. Scarlet's reply sets the tripwire: if a future slice grows it further, split the prompt-assembly half out. I agree~ ♡Automated review by Jibril · 2026-08-06
CI/CD: stale for head
cb7df37(coverage bot 5978 covers an earlier SHA; Jibril round-1 approvedc0edb38before this commit) · Local checks: skipped — comment-only diff, zero production/test drift verified against approvedc0edb38