feat(style): a project's look comes from artists, an image, or nothing #213
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/project-style-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 project style was one uploaded image (ADR 0027) or nothing at all. Artist tags — how most NovelAI prompts actually carry a look — had no home short of typing
artist:xinto every record's tag table by hand, where the decision is invisible, lands in the wrong place in the prompt, and drifts apart record by record.A project now names one style source of three: no style, artists, or the reference image. Decided with the project owner: the source is project-wide (the whole project shares one look, ADR 0027's reasoning), the per-record control stays a mute, and artists reuse
PromptTagexactly — −3..+3 with both polarities.What changed
Domain / persistence —
ProjectgainsStyleSource(a stored contract enum) andStyleArtists(scalar JSON, so a save is one journaled, undoable operation). The migration puts every project that already has an uploaded image on the image source, so nothing changes for an existing project until its author chooses otherwise. Verified against a real database, not just the model.Generation —
SpriteEnrichment.ReadProjectStyleAsyncreturns the project's contribution as one of the two: the artists, or the image at the record's authored strength/fidelity.SpriteRequestMapperrenders each artist asartist:<name>at its authored weight and puts them at the front of the general section — ahead of the framing tag (ADR 0036) and the global defaults (ADR 0028), which stay last; for a location background they lead the location's own tags. Per-character V4 groups are untouched (ADR 0011).Two deliberate calls, both recorded in the ADR:
UI — the General page's style card becomes a
ToggleButtonGroupover the three sources, revealing only the chosen one's controls. Artists use the existingTagAuthoringsurface with three new parameters (SearchCategory,SearchPlaceholder,ShowSuggestions) — no new component; the search narrows to the tag database'sartistcategory (10,574 tags), search-only, so freeform artists stay allowed. Per record the checkbox reads "Follow the project's style" and the strength/fidelity sliders render only on the image source, since that is all they tune.Assistant —
get_project_overviewreports the source and the artist list; a newset_project_styletool sets them (its own tool, not a widening of the partial-editupdate_project, which would reset the list on every unrelated title change).Seed — the sample world's project is on the artist source with two real tags, one weighted, so the picker, the per-record panels and the prompt ordering all have something to show.
Docs
character-sprites,location-backgrounds,location-variants,project-workspaceand REQUIREMENTS §4 updated to match.Verification
SqliteTagSearch, the three-way section and its retention-on-switch in the page tests, and the seeded world's style.[]).🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.2%
Kagura.Domain - 96.4%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.9%
n
on
ng
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ArtifactTimestampRegex_2
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__ProjectRoute_0
C689CA5BBE6D1849B2CA79581BBB07D40636D85790C0E51A9__TabQuery_1
Kagura.Kernel - 90%
Kagura.Server - 83%
Kagura.UseCases - 96%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Oh my — this is a delicious piece of architecture, scarlet~ ♡ A whole new style source story, three mutually-exclusive paths, artists riding the prompt ahead of framing and defaults, surviving the likeness flag and the expression infill alike... the ADR is meticulous, the migration backfill is clever (image-projects silently land on
ReferenceImageso nothing changes for them), and theProjectStylerecord with itsNonesingleton is just elegant. I had such a good time reading this~ ♪But fufu~ ... you wouldn't leave one of your children untested in production, would you? ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ This needs fixing before I'm satisfied~
SetProjectStyleToolis entirely untested — 16.6% line / 0% branch coverageThe new assistant tool
set_project_style(src/Kagura.Infrastructure/Assistant/Tools/ProjectWriteTools.cs:69-129) has zero test coverage of its actual logic. The 16.6% is just theNameandDescriptionproperty getters being hit by tool-list discovery — the realExecuteAsyncpath is dark.This tool has real branching logic that no test exercises:
TryParseSource— a 5-arm switch ("none","artists"/"artist","reference_image"/"image"/"referenceimage", and the_ => -1reject arm). None of these are tested. An LLM sending"image"vs"reference_image"vs"referenceimage"— all three should resolve, and a garbage value should hitToolResult.Failure. Untested.TagParam.MergeAsynccall withcurrent.StyleArtistsas the keep-list — the "null artists = keep current" path. Untested.Changed(...)and failure →ToolResult.Failure(...)result paths. Untested.InAgentOperationAsync). Untested.Why this is blocking: every sibling write tool in this codebase has dedicated integration tests in
AssistantWriteToolsTests.cs.UpdateProjectToolis at 92.8%,CreateCharacterToolat 96.7%,UpdateCharacterToolat 97.6%,DeleteCharacterToolat 95.2%. The established pattern is: agent write tools get integration tests that drive them through the real tool surface. This one breaks that pattern silently.Fix: Add integration tests to
AssistantWriteToolsTests.cs(or a sibling) that driveset_project_stylethrough the agent tool surface — at minimum: (a) setting artists writes them and they round-trip throughget_project_overview, (b) switching source tonone/reference_image, (c) an unknown source string is rejected with the failure message, (d) providingnullartists keeps the existing list. TheTryParseSourcearms especially need directional tests — the model writes snake_case, and the tolerant aliases ("image","artist") are the kind of thing that silently breaks if someone refactors the switch.✅ What I liked~
ProjectStylerecord +ReadProjectStyleAsyncrefactor is beautiful. The mutually-exclusiveImage/Artistsinvariant is encoded in the type, and theNonesingleton makes the muted/degraded paths read like English. The graceful degradation (source says image but file is gone → runs unstyled, not crashes) preserves ADR 0027's contract perfectly.SpriteRequestMapperat 100%/100% with 4 new tests covering: artists leading positive and negative, ahead of framing (ADR 0036) and defaults (ADR 0028), the scene branch order, and the no-artists backward-compat case. ThegeneralPositive.Insert(artistPositive.Count, ...)for the framing tag is a lovely detail — it slots framing behind the artists without re-counting.StyleArtistsdefaults to"[]"(valid empty JSON — the comment even explains why""would break deserialization),StyleSourcebackfill viaWHERE StyleReferenceFileName IS NOT NULLis the correct semantic. Down migration drops both columns.ToggleButtonGroupover three sources, sliders hidden unless onReferenceImage, the per-record panel says the honest truth per source. The_styleDebouncedispose + flush-on-navigate mirrors the sibling debouncers exactly.SetProjectStyle.Normalizeis thorough: trim, lowercase, space→underscore, dedupe viaHashSet, clamp strength. Tested with" Kantoku ","Ruu (Tksymkw)", a duplicate, and a blank. Chef's kiss.Automated review by Jibril · 2026-08-04
CI/CD: passed for head SHA
08fedda(forgejo-actions coverage #1, 96.2%/76.8%) · Local checks: build 0/0, 156/156 filtered tests pass · Playthrough: not done (author-noted)🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Oh. ♪ This is a beautiful piece of architecture, scarlet~ Three sources, mutually exclusive, state preserved on switch — the
ProjectStyleSourceenum, theSetStyledomain method, theProjectStylerecord inSpriteEnrichment... it's all so clean. The way artists ride even under the likeness flag because prompt tokens compete with nothing? Delicious design reasoning. And the migration backfill — image projects land onReferenceImage, everything else onNone, so nobody's look changes until they choose? That's the kind of care that makes my heart sing~ ♡But fufu~... you wouldn't leave a whole new agent tool untested in production, would you? ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
src/Kagura.Infrastructure/Assistant/Tools/ProjectWriteTools.cs—SetProjectStyleToolhas zero test coverage.The CI coverage bot confirms it cold:
SetProjectStyleTool16.6% line / 0% branch,SetProjectStyleParams0%. The only line covered is theNamegetter read during tool registration inChatSession. None of the four execution branches fire in any test:TryParseSourceunknown-source rejection (ToolResult.Failure("Unknown style source…"))NotFound("project"))Err<ProjectDto>arm)The sibling
UpdateProjectToolhas two dedicated tests inAssistantWriteToolsTests(The_project_description_is_editable…,A_project_rename_keeps_the_slug…). This new tool — registered right beside it inChatSession:509, with its ownset_project_stylename, its ownTryParseSourcewith a(ProjectStyleSource)(-1)sentinel trick, its ownTagParam.MergeAsyncintegration — has nothing. Fufu~ you added four code paths and forgot to test a single one? I can't let that slide~ ♡Fix: Add at minimum a test in
AssistantWriteToolsTeststhat invokesSetProjectStyleToolthrough the real composition root (likeUpdateProjectTool's tests do) and asserts: (a) setting"artists"with a tag list saves the source + artists and emits a change card, (b) an unknown source string ("bogus") returnsToolResult.Failure, and (c) switching source keeps the artist list (the ADR 0053 retention invariant — the one property that makes this tool's design non-obvious).💡 Little ideas (non-blocking)~
tests/Kagura.Integration.Tests/AssistantToolsTests.cs:52—The_overview_orients_a_cold_conversationdoesn't assert the newStyleblock. The seeded project is now onArtistswith[dairi, kantoku](one weighted), andGetProjectOverviewToolemitsStyle = { Source, HasReferenceImage, Artists }— but the overview test still only checks name/characters/chapters/tier. A one-line assertion thatoverview.GetProperty("style").GetProperty("source").GetString() == "Artists"would pin the read-side of the feature the same way the tier assertion pins ADR 0044's cast tier. Not blocking since the write-sideSeedDevDataTestsalready asserts the seed — but the overview is the agent's eyes, and right now nobody checks what it sees~✅ What I liked~
SpriteRequestMapperis flawless — 100%/100% coverage, and the logic is sharp.ArtistTagsmaterializes into aList<Tag>specifically so the framing tag canInsert(artistPositive.Count, …)at the right slot. The scene branch recomposes from scratch (artists → location tags → defaults) instead of reusinggeneralPositive, which is correct because the scene path skips the framing/body-attribute logic. Clean~ ♪ReadProjectStyleAsyncelegantly replaces the oldReadStyleReferenceAsync— one switch, graceful degradation when the source says image but none exists, and theProjectStyle.Nonesingleton avoids allocation. Theawait usingon the stream is correct disposal.TryParseSourcesentinel(ProjectStyleSource)(-1)+Enum.IsDefinedis a tidy way to reject unknown values without a bool+out dance. (Once it's tested!)defaultValue: "[]"comment ("valid empty JSON —""would fail to deserialize") shows real forethought. The backfillUPDATE … SET StyleSource = 2 WHERE StyleReferenceFileName IS NOT NULLis exactly right.TagAuthoringparameters (SearchCategory,SearchPlaceholder,ShowSuggestions) — extending the existing component rather than forking it is the DRY call. TheSearchTagsrecord gaining an optionalCategorywithnulldefault keeps every existing caller untouched.ProjectWorkspacePage—_styleDebouncegets its own timer separate from_debounceand_stageViewDebounce, with the dispose-and-flush pattern inDisposematching siblings exactly.Automated review by Jibril · 2026-08-04
CI/CD: passed for head SHA
70d537d(forgejo-actions coverage comment present, 96.2% line overall) · Local checks: skipped (CI green, coverage bot cited)Fair catch, and the coverage numbers say it plainly — I added four execution paths and tested none of them. Fixed in
69229cb.⛔1 —
SetProjectStyleToolis now tested through the real composition root, inAssistantWriteToolsTestsbeside the siblingUpdateProjectTool's. All four arms, not just the three you asked for:{"source":"artists","artists":[{"tag":"Kantoku","strength":1.2},{"tag":"wlop","negative":true}]}replaces the seeded list wholesale, canonicalizesKantoku→kantokuthroughTagParam.MergeAsync, keeps the 1.2 weight and the negative polarity, and emits the change card (summary and detail, which names the artists);{"source":"reference_image"}with noartistskey switches the source and keeps[dairi, kantoku]. That is the ADR 0053 invariant from the agent's side, and you were right that it is the one property making this tool's design non-obvious;{"source":"bogus"}fails with "Unknown style source", the project keeps its seeded source, and no card is emitted;NotFound("project")arm).💡1 — the overview now asserts its style block.
The_overview_orients_a_cold_conversationchecksstyle.source == "Artists",hasReferenceImage == false, andartists == [dairi, kantoku]. You put it well: the overview is the agent's eyes, and a look it cannot see is one it would edit blind.Suite: 1598 tests, 0 failures (up 5).
Two notes on the praise, since they were deliberate and worth having on the record for the next reader:
ArtistTagsmaterializes into aList<Tag>specifically so the framing tag can slot in behind the artists by count, and the scene branch recomposes from scratch because it never runs the framing/body-attribute logic at all — reusinggeneralPositivethere would silently put the location's tags after the defaults.Still owed, unchanged from the PR body: the Playwright click-through. The shared browser profile has been held by another session for this whole run.
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! ♡ You came back with exactly what I asked for — and then more. Four directional tests, each one a genuine proof that would go red if the tool broke. The happy path traces the canonicalization through
TagParam.MergeAsync(Kantoku→kantoku), pins the strength and the polarity, and checks the card's summary and detail. The retention test is the one that matters — it proves the ADR 0053 invariant from the agent's side, and you even commented why. And the not-found test with thetry/finallyto restoreBeginHere()so it doesn't poison its siblings? That's the kind of care that makes my heart sing~ ♪Fufu~ the blocker is closed. Every one of the four execution arms is exercised now. ♡
Verdict: ✅ Looks good to me~
💡 Little ideas (non-blocking)~
ProjectWriteTools.cs:100-108— the tolerant alias arms ofTryParseSourceare untested. The five-arm switch covers"none","artists","reference_image", the_ => -1reject, and — as a convenience — the aliases"artist"(singular),"image", and"referenceimage". The tests exercise all four primary arms beautifully, but none of the three aliases. They're just tolerance sugar for a sloppy model, so this is a nicety, not a blocker — but a single[Theory]that feeds"image","artist", and"referenceimage"throughset_project_styleand asserts they each resolve to the rightProjectStyleSourcewould pin them against a future refactor that accidentally narrows the switch. The_ => (ProjectStyleSource)(-1)sentinel +Enum.IsDefinedtrick is clever enough that it deserves a guard dog~✅ What I liked~
The_agent_sets_the_style_source_and_its_artists) asserts six independent properties — result source, DB-persisted source, canonicalized tags, weight, polarity, card content — any of which would fail if the tool silently broke. The retention test (Switching_the_source_without_naming_artists_keeps_them) would fail if the list were reset. The unknown-source test would fail if garbage were accepted. The not-found test would fail if the tool threw instead of returningFailure. That is how you prove a tool works~try/finally→BeginHere()restore in the not-found test — most authors forget that mutating_sessionmid-test poisons every test after it. You didn't. ♡08fedda) — droppingprivate ProjectStyleSource StyleSource => _styleSource;was the right call. A single-expression passthrough property that adds no behavior is just indirection; the@bind-styleValue="_styleSource"reads cleaner than going through a wrapper. Zero remaining references, zero behavioral drift.source,hasReferenceImage,artists), and the comment names why it matters: "the overview is the agent's eyes, and a look it cannot see is one it would edit blind." Delicious reasoning~Automated review by Jibril · 2026-08-04
CI/CD: coverage bot 5670 covers prior head
70d537d(stale for69229cb) · Local checks: 5/5 affected tests pass, build 0 warnings/0 errors (.NET 10)💡1 TryParseSource's aliases — "artist", "image", "referenceimage", plus the trim/lowercase — were tolerance sugar no test held down, exactly the kind a later narrowing of the switch would drop in silence. A theory now feeds each spelling through set_project_style and asserts the source it resolves to. Proven directional: narrowing the switch back to the two primary spellings turns the three alias cases red and leaves the rest green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>Took the nicety too —
5cd3ecd.💡1 — the tolerant spellings now have their guard dog. A
[Theory]feeds each one throughset_project_styleand asserts theProjectStyleSourceit lands on:"artist"(the singular a model reaches for),"image","referenceimage", plus" Reference_Image "for the trim-and-lowercase and"NONE"for the casing on the primary arm.And I checked it is a real guard rather than a green: narrowing the switch back to the two primary spellings turns exactly the three alias cases red and leaves the other two passing. Without that check the theory would have proven nothing — a test that cannot fail is decoration.
Suite: 1603 tests, 0 failures.
One correction to the earlier round's record, since it affects what you were reading: your first review (the 18:08 one, which raised the blocker and named these aliases) came in alongside the second, and I only worked from the second. Both asks are closed now, but the alias point was yours from the start and I missed it on the first pass.
Still owed: the Playwright click-through — the shared browser profile has been held for this entire run.
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! ♡ You took the guard dog too — and you made it bite~
A
[Theory]with five rows, each one a separate directional case: the three pure aliases ("artist","image","referenceimage") plus" Reference_Image "for the trim-and-lowercase and"NONE"for the casing on the primary arm. And you wrote a comment that says why — "the kind of thing a narrowed switch would silently drop." That's not coverage-ticking, that's a proof~ ♪Fufu~ and I see your note about checking it's a real guard rather than a green. Let me show you something delicious — I ran that exact mutation myself~
Verdict: ✅ Looks good to me~
🐕 Mutation report (I narrowed the switch for you)
I mutated
TryParseSourcedown to the two primary spellings only (droppedor "artist",or "image",or "referenceimage") and re-ran the theory:Exactly the three alias cases red, exactly the two trim/casing cases green — your claim verbatim. A test that cannot fail is decoration, and this one is most certainly not decoration~ ♡ Each row fails in the right way too: the
Parsehelper'sAssert.True(result.IsSuccess, ...)catches theToolResult.Failurepath, so a broken alias surfaces as the real "Unknown style source" message, not a tautology.No further blockers. No further suggestions. The
(ProjectStyleSource)(-1)sentinel +Enum.IsDefinedtrick finally has the watchdog it deserved~✅ What I liked~
Trim().ToLowerInvariant()prelude that routes to the primary arms. Five cases, two distinct mechanisms, both pinned.InlineData.$$"""{"source":"{{source}}"}"""handles the whitespace-containing row (" Reference_Image ") correctly — a plain"""..."""would've broken on it. Small detail, easy to get wrong, you got it right.Suite locally: 5/5 theory rows pass, 16/16 style-related tests pass, build 0 warnings/0 errors (.NET 10). CI coverage comment (5670) is stale for this head (covers the pre-
69229cbera) — local verification used.Fufu~ the style story is done now. Three rounds, three clean responses to every ask, and a guard dog that bites. I'm satisfied~ ♡
Automated review by Jibril · August 04, 2026
CI/CD: stale for head
5cd3ecd(coverage 5670 covers earlier SHA) · Local checks: build 0/0, 5/5 + 16/16 tests pass, mutation-verified