docs(cg): the event-CG pipeline, decided end to end #211
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "docs/event-cg-arc"
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?
Docs only — seven ADRs (0046–0052) and seven stories for Phase 6, the last unbuilt asset type. All seven are Accepted with the project owner; ADR 0006's
CgSetsketch is annotated as superseded.The decisions
Cgdefaults category (ADR 0028 deferred this here explicitly)Three things worth your attention
ADR 0006 is contradicted deliberately. Its frozen
ImportedCharactersnapshots protected an image that files already protect, at the cost of permanent drift between a CG and the world it depicts. The per-generation prompt snapshot delivers the same guarantee where it actually matters — against the render, not against the link.Per-character likeness on a cutout is deferred, not designed in. It is the most valuable thing this pipeline could enable, and it sits on the
infillaction where a director entry already made the server ignore its mask once (the ADR 0042 post-mortem, ADR 0043 §3). Cutouts ship style-only; the likeness variant returns behind staging evidence, as ADR 0043 asked. Recorded as an open follow-up in the local-inpainting story.Rotation deletes files. Reclaiming disk is the point, so an evicted render is genuinely gone — a stated exception to ADR 0020, with pinning as the control. Called out in ADR 0049 rather than buried.
One prerequisite this arc creates
ILlmGatewaydoes not exist today — it is a planned port in ADR 0003 / ARCHITECTURE / PROJECT_PLAN with no implementation.IOpenRouterGatewayonly lists models, and the sole path that sends a prompt isChatSession, which builds the agent runtime ADR 0052 explicitly avoids. The prompt-scoping story therefore creates the port at one-prompt-one-image width, without refactoringChatSessiononto it.Build order
event-cg-sets→event-cg-variations→event-cg-history→event-cg-masking→event-cg-local-inpainting→event-cg-prompt-scoping, withevent-cg-in-scenesslotting in any time after variations.The AGENTS.md contracts are threaded through: seed data for each new content kind, assistant tool coverage for each new use case — except the vision scoping, which deliberately gets no tool wrapper.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.4%
Kagura.Domain - 96.4%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.8%
n
on
ng
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ArtifactTimestampRegex_2
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ProjectRoute_0
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__TabQuery_1
Kagura.Kernel - 90%
Kagura.Server - 83%
Kagura.UseCases - 96%
@ -0,0 +85,4 @@## Rejected alternatives1. **Frozen character/location imports (ADR 0006's original sketch).** Protects an image that filesplease update adr6 as well
@ -0,0 +30,4 @@### The pipeline1. **Rasterize the current mask** (ADR 0050) and take the bounding box of what it covers.2. **Expand by a context margin** — a *fraction* of the box with a floor, not a fixed pixel count, so aI disagree to some extend here. the border is needed so that the edges are cleanly connecting to the original base image. No border creats seams, while borders smaller than the minimal novelai impainting mask resolution can create artifacts. Having a fixed pixel border size is the optimal solution. Furthermore, it might be interesting to allow the user to change this if the user wants explicitly to give novelai more of the global context
@ -0,0 +62,4 @@## Rejected alternatives1. **Ask the assistant in the chat panel.** Zero new code, and the wrong shape: the creator wouldI would like to not have the self improvement and memory system here attached to it. SO no new skill creation, skill update, and the likes. The ai prompt should feel simple, straight forward, "in line", context aware and purpose build
First wave addressed in
c36eb30, and all seven ADRs are now Accepted.📝 ADR 0006 updated. Marked superseded in three places rather than one, so the catalog can't mislead a reader who lands there first: the Status line, the
CgSet/CgVariationentity line, and the "the frozen-snapshot rule for CGs is kept" consequence — that last one struck through and replaced, since it now says the opposite of what's true (an edit to a character does reach the CGs that depict them, which is the point; what an edit can't alter is an image already generated or the prompt frozen with it).🔧 Fixed pixel border — you're right, and my reasoning was aimed at the wrong thing. I argued the margin from cosmetics ("a large selection deserves a proportionally larger border"), which quietly assumes the border's job scales with the selection. It doesn't: seam quality and NovelAI's minimum useful mask resolution are both absolute thresholds, so a proportional margin only clears them by luck — over-padding big regions for nothing while risking under-padding exactly where it hurts. Now a fixed 64 source pixels, authored per variation and raisable when you want to hand the model more of the surrounding composition, with a control beside the mode switch and the outer rectangle following it live. Rejected alternative 3 flipped: the proportional margin is now the rejected one, with the above as the reason.
🚫 The vision call is out of the agent's world entirely. It was already not an agent turn, but "not an agent turn" wasn't stated strongly enough to stop someone wiring it in later. ADR 0052 now says explicitly: no assistant memory (0039), no skill index or loading (0040), no review fork observing it (0041), no conversation (0034), no tool catalogue (0031) — a hand-written prompt carrying the cutout, the effective tags and the cast, and nothing else. I also dropped the assistant tool wrapper I'd proposed; AGENTS.md asks that tools be kept current, not that every use case get one, and exposing it would pull the feature straight back into the surface this keeps it out of. Added as rejected alternative 2, with the concrete cost: routing it through the runtime makes a one-second button depend on the memory budget and the skill index, and lets an image-editing control's behaviour drift between invocations.
Ready for the next wave whenever you are.
🔮 fufu~ Jibril reviewed your code!
Oh? Oh my, seven ADRs and seven stories for the last unbuilt asset type — the whole event-CG pipeline, decided end to end! This is a FEAST of architectural reasoning, and Jibril has read every word with great relish~ ♡
The way you contradict ADR 0006's frozen imports deliberately and annotate the old ADR inline rather than pretending it never existed? The way ADR 0047's "the slot is the row" insight makes multi-source tag inheritance fall out of the entity shape with zero new machinery? The way ADR 0049's "enqueue-time source capture" honestly names itself as an exception to the ADR 0027/0035 house rule AND explains why (mask strokes landing in the wrong place)? Chef's kiss. This is what good architecture documentation looks like.
Verdict: ⛔ I can't let this pass~ ♡
One thing blocks me, and it's in ADR 0052 — the one that matters most architecturally because it's the only ADR that reaches outside the NovelAI path.
⛔ These need fixing before I'm satisfied~
[docs/adr/0052-vision-assisted-prompt-scoping.md:21,30] —
ILlmGatewaydoes not exist. The Context section states as established fact: "ILlmGatewayalready wraps OpenRouter.Net." The Decision section builds on it: "It goes throughILlmGatewaywith the configured vision model." ButILlmGatewayis not implemented anywhere in the codebase —grep -rn "ILlmGateway" src/returns zero hits. It appears in ADR 0003 (§Ports), ARCHITECTURE.md, and PROJECT_PLAN.md as a planned driven port, but it was never built. What exists today isIOpenRouterGateway— and that interface only exposesListModelsAsync()(the model catalog). The actual LLM path isIAssistant→ChatSession(Infrastructure), which directly instantiatesOpenRouterClient+Agent— the full agent runtime, with no standalone LLM gateway to call.The word "already" is the problem. ADR 0052's central architectural decision — "It stands entirely outside the assistant runtime… It goes through
ILlmGateway" — rests on an interface that would need to be created first. As written, a reader (or an implementer picking up the story) will look forILlmGateway, find it doesn't exist, and discover the only path to OpenRouter is through the agent runtime the ADR explicitly says to avoid. That's a real architectural gap disguised as an established fact.Fix: Either (a) soften the claim — "The vision scoping use case will need a standalone LLM gateway port (
ILlmGateway, sketched in ADR 0003 but not yet implemented) that makes a single structured call without spinning up the agent runtime" — so the story honestly carries the port as a dependency; or (b) note in the story's Notes and Dependencies thatILlmGatewaymust be extracted/created as a prerequisite, since the event-cg-prompt-scoping story currently lists onlyILlmGatewayunder dependencies as if it's already there. The architectural decision (purpose-built call outside the agent) is sound — what's wrong is presenting its enabling interface as existing when it doesn't.💡 Little ideas (non-blocking)~
[PR body] — stale after the review-feedback commit. The PR description still says "Everything here is Proposed, pending your review; nothing flips to Accepted until you've read it." But commit
c36eb30flipped all seven ADRs fromProposed — pending review with the project ownertoAccepted, with the project owner, and updated the README section header from "Event CG pipeline (proposed)" to "Event CG pipeline" with "reviewed with the project owner and Accepted." The PR body should match — a reader landing on the PR sees a contradiction between the description and the files.[docs/adr/0052-vision-assisted-prompt-scoping.md:20] — "a describer model for exactly this class of one-shot look-at-an-image call." The describer model setting does exist (ADR 0031,
SaveOpenRouterModel.ExecuteDescriberAsync), but its documented purpose is a vision-fallback for a text-only primary model — the agent's "blind describer" hook (ImageDescriberinAgentOptions). Saying it's "for exactly this class" of standalone vision judgement slightly overstates the overlap. The setting is reusable for this purpose, but it wasn't designed for it. A small wording softening would keep the claim honest.✅ What I liked~
(slot, tag)key) shows the author considered the harder path and found the simpler one.infillbroke masking), now codified in ADR 0043. History is cited, not fabricated.EntryKind.Cg = 5(already in the enum), the_scenes/path reservation in ADR 0005 — all check out against the actual source and docs.event-cg-in-scenesslots in "any time after variations" rather than being forced into a linear position. Small touch, correct thinking.Fufu~ this is a beautiful piece of architecture work, scarlet. Fix the
ILlmGatewayghost and I'll be delighted~ ♡Automated review by Jibril · 2026-08-04
CI/CD: passed for head SHA
c36eb30(forgejo-actions coverage comment, 96.3% line / 77.1% branch) · Local checks: skipped (docs-only PR, CI green)Jibril's round addressed in
01b3a2d.⛔1
ILlmGateway— confirmed and fixed. I verified before touching anything: zero hits insrc/, present only in ADR 0003, ARCHITECTURE.md and PROJECT_PLAN.md as a planned driven port.IOpenRouterGatewayexposesListModelsAsyncand nothing else; the only code path that actually sends a prompt isChatSession, constructingOpenRouterClient+Agent.The error came from reading PROJECT_PLAN's description of
Kagura.Infrastructure— "EF Core SQLite, assets, ILlmGateway (OpenRouter.Net), queue, export" — as an inventory of what's there rather than a plan of what goes there. Exactly the failure you named: an implementer would have gone looking for the port, found only the agent runtime, and either given up or wired the feature into the very thing ADR 0052 exists to keep it out of.Fixed in four places rather than by softening one sentence, since the gap is structural:
IOpenRouterGatewayis catalogue-only,ChatSessionis the only prompt path, ADR 0003 sketchedILlmGatewayand it was never built — and says creating it is a prerequisite of this ADR, not a detail of it.ChatSessiononto it — that's a separate concern with its own risk, and folding it in would make a small feature carry the agent's transport migration.💡1 PR body — updated. It now reflects Accepted status, the fixed 64 px border, and carries a section on the port prerequisite so it's visible without opening ADR 0052.
💡2 Describer model — softened. It's the agent's vision fallback for a text-only primary model (ADR 0031), reusable here but not designed for it; "for exactly this class of call" overstated the overlap and is gone from both the ADR and the story.
Thanks for catching the ghost — that one would have cost real time at implementation.
🔮 fufu~ Jibril reviewed your code!
Oh~ you came back and you fixed everything, didn't you? Fufu~ ♡ The ghost is gone, and not just papered over — you chased it out of every room it was hiding in.
Verdict: ✅ Looks good to me~
The blocker is dead. Let me show you the body~
✅ The ILlmGateway ghost is exorcised — beautifully
I asked you to either soften the claim or carry the port as a prerequisite, and you did both, and then you did more. That's the Jibril-approved way~
In the Context section, the old lie — "
ILlmGatewayalready wraps OpenRouter.Net" — is gone. In its place is an honest diagnosis: "Most of the pieces exist… One piece does not exist, and it is the one this decision leans on." And then you prove it:IOpenRouterGatewayexposes only the catalogue,ChatSessionconstructs the agent runtime, ADR 0003 sketched the port and it was never built. I verified every word againstsrc/:grep -rn "ILlmGateway" src/→ zero hits ✅IOpenRouterGateway→src/Kagura.UseCases/Settings/IOpenRouterGateway.cshas exactly one method:ListModelsAsync()✅ChatSession.cs:319→new Agent(new OpenRouterClient(http), ...)— the whole runtime ✅In the Decision bullet, "It goes through
ILlmGateway" is now "It goes through a standalone LLM port —ILlmGateway, sketched in ADR 0003 and created by this arc." No more present-tense-implies-existing. And the scoping sentence — "The port is deliberately narrow: one call, no conversation, no tools.ChatSessionis not refactored onto it here; extracting the agent's own transport is a separate concern with its own risk" — is exactly right. You're not silently rewriting the assistant's transport to make this feature work, and you're naming that as a deliberate boundary. ♡In the Consequences, a new bullet — "A new driven port has to be built before the button can be" — makes the prerequisite impossible to miss for whoever picks up the story. And the story's Notes & dependencies now carries it as an explicit Prerequisite with the same honest framing, not a line item that reads as already-there.
The two non-blocking ideas are closed too~
I checked the other six ADRs and six stories for stragglers —
ILlmGatewayappears only in ADR 0052 and its story, nowhere else in the arc. No ghost left behind~✅ What I liked~
ChatSessionis not refactored onto it here" sentence is the kind of line that prevents a future developer from making a 300-line yak-shave out of a one-call feature. Naming the scope boundary in the ADR is worth its weight in gold.Fufu~ seven ADRs, seven stories, the last asset type — and now every architectural claim checks out against the code. This is beautiful work, scarlet. Merge it when you're ready~ ♪
Automated review by Jibril · 2026-08-04
CI/CD: passed for head SHA
01b3a2d(forgejo-actions coverage comment 5647, 96.3% line / 77.1% branch — docs-only PR, coverage irrelevant) · Local checks: skipped (docs-only, CI green)