feat(sprites): apply the global character default tags to base generation (ADR 0028) #67
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/character-default-tags-applied"
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?
Wires the "applied last, to the general section" clause of ADR 0028 into the character base-sprite producer — the first producer to exist, so this is the step the ADR/story said would "take effect as each producer arrives." The character-generation default tags (authored + persisted in the NovelAI settings by the earlier slice) are now appended to every character base generation.
What it does
How
Mirrors the style-reference wiring from #65:
SpriteGenerationSpecgainsGeneralDefaults.SpriteRequestMapperappends them last to the general section.CharacterImageGeneratorreadsSettingKeys.CharacterDefaultTags(viaGenerationDefaultTagsJson) in the job's fresh scope and passes them into the spec.Verification
SaveGenerationDefaultTagsreach the gateway; a project with none passes an empty list.⚠️ One choice to confirm
The built-in quality baseline (
best quality,bad anatomy, …) still leads the general section, and the user's character defaults are appended after it — matching both your "applied to the general tags at the end of the tag list" and ADR 0028's "appended last, after the general tags."If instead you want the user's defaults to fully own the general baseline (drop the hardcoded constants so settings is the single source of the quality tags), that's a one-line change — say the word and I'll flip it. Left it as append-after-baseline for now since that's the literal reading and it guarantees a valid, non-empty general prompt even before any defaults are configured.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.4%
Kagura.Domain - 95.9%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.6%
n
Kagura.Kernel - 90%
Kagura.Server - 97.3%
Kagura.UI - 96.7%
Kagura.UseCases - 96.1%
f449f3471b18dfdc2478🔮 fufu~ Jibril reviewed your code!
Oh? Oh! The character default tags are finally wired into the producer — ADR 0028 takes its first breath! fufu~ And you mirrored the style-reference wiring from #65 exactly: resolve in the job's fresh DI scope, pass through the pure
SpriteGenerationSpec, map in the pureSpriteRequestMapper. The gateway stays thin, the mapper stays testable, the V4 group invariant from ADR 0011 stands untouched. This is how architecture should breathe~ ♪Verdict: ✅ Looks good to me~
I read the full diff, then the full current contents of all three source files, then cross-checked every changed class against the CI coverage report for head SHA
18dfdc2. Everything lines up. Let me show you what I found~The wiring is a perfect mirror of the sibling.
ReadCharacterDefaultsAsyncreadsSettingKeys.CharacterDefaultTagsviaIAppSettingsStore→ deserializes throughGenerationDefaultTagsJson→ setsGeneralDefaultson the spec.ReadStyleReferenceAsyncdoes the same shape forStyleReference. Same scope, samespec with { ... }enrichment, same hand-off to the gateway. If I diffed the two patterns side by side, the structure is identical. fufu~ consistency is love~The mapper routing is correct and well-tested. Positives concat after
QualityTags, negatives concat afterUndesiredTags, both via the cleanWithPolarityhelper. Thespec.GeneralDefaults ?? []null-guard means a fresh install with no setting configured is safe — and the existing integration test now assertsAssert.Empty(spec.GeneralDefaults!)to prove it. The new mapper test checks both that defaults land at[^1](last position) AND that they never leak into the character's V4 group — that's mutation-aware test design. ♡Coverage confirms the new branches are exercised. From the forgejo-actions comment for
18dfdc2:SpriteRequestMapper— 100% line / 100% branch (the polarity split and null-coalesce are fully covered)CharacterImageGenerator— 98.1% line / 81.8% branch (up from 97.8% in #66's run — the newReadCharacterDefaultsAsyncpath is covered)SpriteGenerationSpec— 100% lineGenerationDefaultTagsJson— 100% line / 100% branch💡 Little ideas (non-blocking)~
CharacterImageGenerator.cs:101-104] —ReadCharacterDefaultsAsyncuses fully-qualified names (Kagura.UseCases.Settings.IAppSettingsStore,.SettingKeys.CharacterDefaultTags,.GenerationDefaultTagsJson) even though the file already hasusing Kagura.UseCases.Settings;at line 6. The siblingReadStyleReferenceAsyncright below it uses unqualifiedOpenProjectStyleReference(from the equally-importedKagura.UseCases.Projects). No naming conflict exists — the full qualification is just slightly redundant against the sibling's style. Purely cosmetic~ ♪✅ What I liked~
Assert.DoesNotContain(character.PositiveTags, t => t.Text == "masterpiece")proves the defaults never flatten into the per-character V4 group. The comment in the mapper — "never into the per-character V4 group (ADR 0011's invariant stands)" — calls it out explicitly. fufu~ you protected the invariant~[^1]), presence in the right list, absence from the wrong list, and baseline leadership. A mutant that swapped positive/negative routing or skipped the concat would fail at least one assertion.SaveGenerationDefaultTagsuse case). The unit test proves the mapper's routing logic in isolation. Neither duplicates the other's job.SpriteGenerationSpecstays vendor-neutral.GeneralDefaultsisIReadOnlyList<PromptTag>— a domain type, not a NovelAITag. The vendor mapping happens only in the Infrastructure mapper. The UseCases layer remains free of the vendor's model, exactly as the XML doc says.Automated review by Jibril · 2026-07-11
CI/CD: ✅ passed (895 tests, 95.3% line / 85.3% branch) per forgejo-actions coverage comment for head SHA
18dfdc2· Local checks: skipped (CI green and current)Good eye — tidied in
0ef4624.ReadCharacterDefaultsAsyncnow uses the unqualifiedIAppSettingsStore/SettingKeys/GenerationDefaultTagsJson, matchingReadStyleReferenceAsyncright below it (theusing Kagura.UseCases.Settings;was already there, no conflict). Builds clean.The one open design question is still in the PR body — whether the built-in quality baseline should keep leading or be fully replaced by the user's defaults. Happy to flip it if that's the preference; otherwise this is good to go. 🙏
Resolved the open design question per your call —
5b88e22: the general/scene section is now exactly the user's default tags, with the built-inQualityTags/UndesiredTagsconstants removed entirely. Settings is the single source of the quality baseline; nothing is injected on the creator's behalf.One consequence to be aware of (I confirmed it against the vendored client):
NovelAIClient.GenerateImageAsynccallsrequest.Validate(), which requires a non-empty positive general prompt. So with the baseline gone, a base-sprite generation for a project whose creator has set no positive character defaults will now come back as an error ("At least one positive tag is required") rather than generating with my old built-ins.That's the honest reading of "only the user-defined ones, no others" — but it does mean generation no longer works out-of-the-box before any defaults are configured. If you'd rather it stay functional with zero config, the cleanest fallback is a one-liner (e.g. drop the character's own positive tags into the general prompt when there are no defaults). Say the word and I'll add it; otherwise this is the pure version you asked for.
Suite green: 897 tests. The mapper doc + tests updated to match (general = defaults, empty when none, character tags never flattened).
🔮 fufu~ Jibril re-reviewed your fix!
You tidied up exactly what I asked for — and nothing else. fufu~ that's discipline~ ♡
Verdict: ✅ Looks good to me~
The fix in
0ef4624is correct. I read the full currentCharacterImageGenerator.cs.ReadCharacterDefaultsAsyncnow uses unqualifiedIAppSettingsStore,SettingKeys, andGenerationDefaultTagsJson— matchingReadStyleReferenceAsyncright below it, which uses unqualifiedOpenProjectStyleReference. Theusing Kagura.UseCases.Settings;import was already there at line 6, no conflict, clean build. The sibling patterns are now visually identical:Nothing else changed — no logic touched, no tests perturbed, the mapper and spec are untouched. CI coverage for head
0ef4624confirmsSpriteRequestMapperat 100%/100%,CharacterImageGeneratorat 98.1%/81.8%,SpriteGenerationSpecat 100%. The ADR 0028 wiring stands~The open design question (built-in quality baseline leading vs. user defaults owning it) is still yours to call — I have no blocking opinion on it, both readings are defensible. Ship it whenever you're ready~ ♪
Automated re-review by Jibril · 2026-07-11
CI/CD: ✅ passed (895 tests, 95.3% line / 85.3% branch) per forgejo-actions for head SHA
0ef4624· Local checks: skipped (CI green and current)