feat(characters): base-image-only tags outside the inheritance (ADR 0061) #243
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/base-image-tags"
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?
A character's
BaseTagsserved two masters: as the base appearance they root every inheritance chain and must stay clothing-free, but as the base image's whole prompt they leave the render's dress to the model's whim — every regeneration re-rolls what the character happens to wear. The workaround (author clothes into the base, disable them on every outfit) is inverted effort that leaks through any outfit that forgets.Decision (ADR 0061)
Characters gain
BaseImageTags: a second authored tag list besideBaseTags, same tag-authoring surface, same scalar JSON column. It participates in exactly one prompt — the base sprite's, appended after the base tags — and in nothing else. Outfits inheritBaseTagsalone (nothing new shows in their inherited rows), and expressions and CG cast chains are untouched by construction. The pattern mirrors the region retouch's local scope (ADR 0051): tags scoped to one generation surface, never entering the record's inheritable set.What changed
Character.BaseImageTags+ profile/DTO fields (required positional, so no call site can silently drop it), one JSONTEXTcolumn with migration (defaultValue: "[]").[.. BaseTags, .. BaseImageTags]; the positive-tag precondition now checks the combined list, since it guards what actually generates.TagAuthoringwith a one-line contract distinguishing it from the base appearance ("outfits never inherit these"). Both authors carry stable ids now that the tab hosts two.create_characterthreads the blank's list;update_charactergainsbaseImageTags(null-keeps merge, wholesale replace) with the description explaining the split.get_characternow reports both tag lists — it previously reported neither, which left agents editing tags blind against the get-tools contract.sundressas a base-image-only tag;SeedDevDataTestsassert it (and that Ren has none).Verification
BaseImageTagswithout touchingBaseTags.🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.6%
Kagura.Domain - 94.3%
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 - 82%
Kagura.UseCases - 95.6%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Ohhh — ♡ A second tag list that divides by audience rather than by flag, so the inheritance root stays clean while the base render gets to wear clothes without leaking into every outfit? This is wonderful~ The split is elegant, the ADR names three rejected alternatives with honest reasoning, and the pattern consciously mirrors ADR 0051's local-scope precedent. Knowledge like this makes my wings flutter~ ♪
Verdict: ✅ Looks good to me~
I went looking for the seams where a bug like this could hide — a forgotten
.Concat, a sibling generator that still reads the old single list, an outfit chain that accidentally inherits the sundress — and found every one of them correct.The inheritance boundary holds at every consumer:
CharacterImageGeneratorbuilds[..BaseTags, ..BaseImageTags]✓OutfitImageGeneratorreadsoutfit.EffectiveTags(character.BaseTags)— BaseImageTags correctly absent ✓ExpressionImageGeneratorreadsoutfit.EffectiveTags(character.BaseTags)— correctly absent ✓CgVariationImageGenerator(CG cast chain) readscharacter.BaseTags/outfit.EffectiveTags(character.BaseTags)— correctly absent ✓CgCastMemberDto.Fromresolves inherited fromcharacter.BaseTagsonly — correctly absent ✓The one place the two lists meet is the base sprite's prompt, and the precondition guard checks the combined list because it guards what actually generates. Sharp~
The positional-arg trap is properly sprung:
BaseImageTagsis a required positional onCharacterProfile(slot 12, before the defaultedTier), so the compiler catches any caller that forgets it. Every call site — 2 inCharacterWriteTools, 3 inSeedDevData, 11 across tests — is updated correctly. And the two existing silent-reset traps (VoiceProviderId,StyleReference) are still threaded through both create and update tools. No regression there~Test coverage is genuine and directional, not tautological:
Base_image_only_tags_join_the_base_prompt_after_the_base_tags— pins the ordering[long_hair, sundress]at the gateway ✓A_base_image_only_positive_satisfies_the_precondition— negative-only BaseTags + positive BaseImageTags → generation proceeds ✓Base_image_only_tags_never_reach_an_outfit_prompt— the critical invariant, and it's proven right: outfit prompt asserts[long_hair, winter_coat], sundress nowhere. If BaseImageTags ever leaked intoEffectiveTags, this goes red. ♥Apply_stores_the_base_image_only_tags_clamped_beside_the_base_tags— clamp (99→MaxStrength) + separate-lists + round-trip ✓BaseImageTagsnotBaseTags✓Migration is sound:
nullable: falseTEXT withdefaultValue: "[]"— byte-for-byte theAsJsonListsibling pattern. ModelSnapshot consistent.The
TagAuthoringId addition on the base-appearance author (char-base-{id}) is a necessary companion to the new second author — two tag surfaces on one tab need distinct search-state keys, or their dropdowns cross-pollute. Well-caught proactively rather than waiting for the bug report~💡 Little ideas (non-blocking)~
create_characterhas nobaseImageTagsparam — this is consistent withbaseTags(neither is settable at create time; both requireupdate_character), so it's not a defect. But thecreate_characterdescription doesn't mention the concept at all, whileupdate_character's description explains the split thoroughly. An agent that only reads the create tool's schema might not discover the feature exists until it callsget_characterand sees the field. A one-line mention in create's description ("base-image-only tags are set via update_character") would close the discovery gap — purely additive, no behavior change.✅ What I liked~
character.BaseTagsconsumer. Naming why it's rejected shows the decision was earned, not guessed. ♡get_characterreporting both lists is flagged as fixing a pre-existing get-tools-contract gap, not smuggled in as a quiet drive-by. Honest changelog~Automated review by Jibril · 2026-08-09
CI/CD: absent for head
dc59ce0(PR just opened, no coverage bot yet) · Local checks: build 0 warnings/0 errors, 703/703 integration + 32/32 use-case + 113/113 blazor-adapter tests pass (full suite + targeted filters on changed files)Applied 💡1 in
93846f8:create_character's description now ends with a pointer at both tag lists — "Sprite tags are authored afterwards via update_character: baseTags for the inheritable base appearance, baseImageTags for base-render-only extras." That closes the discovery gap without adding create-time parameters, keeping create consistent with baseTags (neither list is settable at create time, deliberately — a blank character has no tags to merge against, and the tag tools should be consulted before authoring any).No behavior change, so no new tests; the build is clean.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Right on cue~ ♡ The discovery gap I flagged is now closed with a single well-placed sentence. No new parameters, no create-time coupling — just a pointer that says "hey, the tags live there, go look." That's exactly the lean fix I love to see~
Verdict: ✅ Looks good to me~
The new commit
93846f8is surgical: +3/-1 inCharacterWriteTools.csonly — theCreateCharacterTool.Descriptionstring gains one sentence pointing the agent atupdate_characterfor bothbaseTags(inheritable base appearance) andbaseImageTags(base-render-only extras). I verified the pointer is accurate against the actualupdate_characterschema (baseTagsat line 166,baseImageTagsat line 167 — both real[JsonPropertyName]params). The param names in the description match the wire names exactly. No behavioral drift —git diff dc59ce0..93846f8touches exactly 1 file, description string only, zero test changes.The sentence slots in naturally after the build-attributes paragraph and before the closing "applies immediately" line — reads cleanly in context. Stays consistent with
baseTagsitself (neither list settable at create time, deliberately — a blank character has no tags to merge against). ♪My full architectural review from
dc59ce0(comment 6061) stands unchanged in full: inheritance boundary verified at all 5 consumers, positional-arg trap properly sprung, migration sound, 6 directional tests pinning every invariant. No new issues introduced by this commit.✅ What I liked~
baseTags/baseImageTags) means an agent readingcreate_character's description can immediately construct the rightupdate_charactercall without guessing. Clean UX for the tool surface~Automated review by Jibril · 2026-08-09
CI/CD: coverage bot 6060 present for
dc59ce0(96.2%/77.6%) · stale for93846f8— change is description-string-only, coverage irrelevant · Local checks: skipped (comment-only delta from approveddc59ce0)