feat(characters): the character is a tuning row too (ADR 0063) #245
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/base-attribute-tuning"
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?
Follow-up to #244, from the owner's report: after the merge, the injected attribute tags were visible on every inheriting row except the character's own Base appearance prompt — the root of the chain was the one place where the head of the prompt stayed invisible, and the one row that couldn't tune what it injects.
Decision (ADR 0063)
The character joins the idiom. The Base appearance surface shows the attribute bundle as inherited rows — computed live from the generation-identity selects, so picking a body type surfaces its tags immediately — and the character stores the same two lists every other row has (
DisabledInheritedTags,InheritedTagStrengths), applied to the bundle at the root ofAppearanceChain().Because the tuning sits at the root, it cascades: outfits and CG members inherit the tuned values as what arrives and tune further on top — the same layering an outfit's tuning already has over a CG member's. The shared composition lives in one place (
BodyAttributePrompt.Chain) behind bothCharacter.AppearanceChain()and a newCharacterDto.AppearanceChain()twin, so the page's generate gate and the engine's precondition read the identical list — the #244 review's drift lesson made structural.What changed
Character(same normalization as every sibling row), threaded required-positional through profile and DTO, two JSON columns with migration (defaultValue: "[]").TagAuthoringgains the inherited rows and both tuning callbacks; the section hint explains the cascade. The generate gate now readsCharacterDto.AppearanceChain().update_charactergainsdisabledInheritedTags+inheritedTagStrengths(null-keeps merge);get_characterreports the tuned bundle (AttributeTags) plus both raw lists.fatcounter-tag to −1.5 at the root;SeedDevDataTestsassert the list and the tuned chain.Verification
AppearanceChainwith stale entries inert (including a base-tag name in the re-weight list staying bundle-only), the editor's attribute rows save onto the character and cascade, the base render honors root tuning at the gateway, and the outfit prompt receives the root-tuned values as its inherited baseline.skinny1.0 /fat−1.5 (the seeded root re-weight) /small breasts1.0 above the authored base tags — and the School Uniform editor inheritsfatat −1.5 while its ownsmall breasts0.9 re-weight still applies on top.🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.6%
Kagura.Domain - 94.4%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 97.7%
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! The root of the chain finally gets to tune its own injection~ How delightful! ♪ You took the #244 drift lesson and made it structural — one
BodyAttributePrompt.Chain(...)behind bothCharacter.AppearanceChain()and the DTO twin, so the gate and the engine read the identical list. That's exactly the kind of architectural fix I adore. The whole cascade threading —OutfitImageGenerator→character.AppearanceChain()→Outfit.EffectiveTags(...)→ root-tuned values arrive as the inherited baseline — is beautiful. I traced every generator path and the cascade is real. ♡Verdict: ⛔ I can't let this pass just yet~ ♡
The architecture is gorgeous and I want it merged, but you left a door unlocked, fufu~
⛔ These need fixing before I'm satisfied~
[tests/Kagura.Integration.Tests/AssistantWriteToolsTests.cs — no new test] — The
update_charactertool gained two new write params (disabledInheritedTags,inheritedTagStrengths) with null-keeps merge semantics, andget_charactergained three new output fields (AttributeTags,DisabledInheritedTags,InheritedTagStrengths). None of this is tested through the tools. The domain layer is well-covered (BodyAttributesTests,CharacterImageGenerationTests,OutfitImageGenerationTests), but the assistant tool surface — the agent's only way to set root tuning — has zero coverage for the new params.Your own established pattern from PRs #199 and #200 says otherwise: those PRs added explicit
AssistantWriteToolsTestsfortier(write-through + survives-omit) andStyleReference(survives partial edit through the profile-rebuild). The sibling testBody_attributes_write_through_the_tools_and_a_cup_size_on_a_boy_refuses_loudlyalready proves body-type/cup-size write-through is tool-tested — the new tuning params deserve the same treatment.The
InheritedStrengthParam.Mergepath (normalize term + clamp) and thep.DisabledInheritedTags ?? current.DisabledInheritedTagsnull-keeps arm are both new branches that no test exercises. Andget_character'sAttributeTagsfield — which callsBodyAttributePrompt.Chain(...)with empty base tags to show the agent the tuned bundle — isn't asserted inA_character_reads_in_full_with_outfits_relationships_and_route.Fix: Add at minimum:
The_root_attribute_tuning_writes_through_the_tools_and_survives_an_edit_that_omits_ittest (create character with body type, setdisabledInheritedTags+inheritedTagStrengthsviaupdate_character, assert the values land on the entity, then do a partial edit omitting them and assert they survive — mirrors the tier test exactly).AttributeTags/DisabledInheritedTags/InheritedTagStrengthsin the existingA_character_reads_in_full...test (the seeded Aoi hasInheritedTagStrengths: [{"fat", -1.5m}]— assert it's visible to the agent).💡 Little ideas (non-blocking)~
outfit.EffectiveTags(character.BaseTags)instead ofcharacter.AppearanceChain(), so expression inpainting renders from the pre-0062 chain (no attribute bundle, no root tuning). This is a pre-existing gap from ADR 0062 (#244), not introduced by this PR — but now that root tuning cascades through every other path, the inconsistency is starker. Not blocking because it predates this PR, but worth a follow-up issue so expressions don't drift further from the dressed sprite they sit on top of.✅ What I liked~
BodyAttributePrompt.Chain(...)is the one composition, shared by entity and DTO, so the #244 drift lesson is now enforced by the type system, not by convention. This is how you kill a class of bug permanently. ♡Character.Apply()—.Distinct(StringComparer.Ordinal)/.DistinctBy(s => s.Tag, StringComparer.Ordinal).Select(s => s.Clamped())— is byte-identical toOutfit.Apply(),CgCastMember.Apply(),LocationVariant.Apply(), andCgVariation.Apply(). The JSON-column migration matches the established scalar-JSON rule withdefaultValue: "[]". The tool descriptions are exemplary.The_characters_root_tuning_shapes_the_chain_and_stale_entries_are_inertproves three things at once:fatdisabled removes it,small breastsre-weighted carries through, and crucially a stale disable (gone_tag) and a base-tag-name re-weight (long_hair) are both inert — that's the ADR 0047 contract pinned.The_root_attribute_tuning_cascades_into_the_outfit_promptprovesskinnyarrives at 0.8 (the root's value, not the bundle's 1.0) andfatis gone — so the outfit inherits the tuned baseline, exactly as the ADR promises.CharacterProfileconstruction site across src + tests was updated consistently (13 sites), and the two new required positionals land before the defaulted params. TheVoiceConfigs/StyleReferencenull-keeps defaults were preserved.Automated review by Jibril · 2026-08-10
CI/CD: absent for head
27649b7(PR just opened, no coverage bot yet) · Local checks: build 0 warnings/0 errors, 98/98 relevant tests pass (BodyAttributes 23, CharacterTests 14, CharacterImageGeneration 16, OutfitImageGeneration 17, SeedDevData 3, CharacterEditorPage 25)Applied ⛔1 in
655110a— and writing those tests earned their keep immediately: the write-through test's spaced curated name exposed a real defect.InheritedStrengthParam.Mergebooru-normalized its key ("Small Breasts"→small_breasts), which can never match a curated bundle name — so an agent's attribute re-weight through any tool (character, outfit, CG member) would have landed inert. Merge now keeps the name as typed, trimmed and lowercased only; the doc comment explains the two name families and that copying a get-tool-reported canonical name always keys correctly. Nothing pinned the old underscoring.The demanded coverage:
The_root_attribute_tuning_writes_through_the_tools_and_survives_an_edit_that_omits_it— create with build+cup, tune viaupdate_characterusing the spaced name deliberately (proving the un-underscoring) with an out-of-range strength (proving the clamp), assert the entity and the shapedAppearanceChain, then a partial edit omitting both proves the null-keeps arm.A_character_reads_in_full…now assertsattributeTags(the tuned bundle, with Aoi's seededfatat −1.5),disabledInheritedTags, andinheritedTagStrengths.💡1 (expression infill still reading the pre-0062 chain) is filed as #246 with the consistency argument laid out — it predates this PR and deserves its own small decision.
Full suite green: 330 + 837 + 705.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh!
655110a— you came back with the tests AND an un-underscore, in one surgical pass. Fufu~ you spoil me~ ♡This is the re-review of the synchronized head (round 2). My single blocker from round 1 (comment 6089 — the assistant tool surface was entirely untested) is closed, and you bundled a real bug fix into the bargain. Let me show you what I found.
Verdict: ✅ Looks good to me~
⛔ Round-1 blocker — CLOSED ♪
The_root_attribute_tuning_writes_through_the_tools_and_survives_an_edit_that_omits_itcreates Mio (slim + small), disablesfat, re-weights the spaced curated nameSmall Breaststo 9, then asserts: the storedInheritedTagStrengthis("small breasts", MaxStrength)(clamped from 9 → 3), the chain the generators read is[skinny, small breasts](fat gone, small breasts re-weighted), AND a partial personality-only edit keeps both lists. The read-side test pins Aoi's seeded chain head —fatat −1.5 surfacing inattributeTags, the rawinheritedTagStrengthsalongside. Every newupdate_character/get_characterarm now has a witness.✨ The bonus fix — and I mutation-checked it
You didn't just add tests; you fixed a latent inconsistency in
InheritedStrengthParam.Merge. The old code ranTagParam.NormalizeTerm(p.Tag)— the booru path that lowercases and replaces spaces with underscores. That's right for authored tags ("Long Hair" →long_hair), but the inherited bundle is keyed by the verbatim curated name (BodyAttributePromptkeeps "spaces and all" per its own doc comment,CupSize.Small→"small breasts",BodyType.Musculargirl →"muscular female").InheritedTagStrength.Applymatches onStringComparer.Ordinalagainstt.Tag, andDisabledInheritedTagswas already a raw passthrough — so the re-weight list was the one outlier being underscored, silently inert for every multi-word attribute tag. And because all five sibling tools (Outfit/CgVariation/Variant/bothCgCastarms) route through this same sharedMerge, the fix lands uniformly. The new doc comment naming both families (underscored booru vs spaced curated) is exactly right.I verified it's load-bearing. I reverted
TagParam.csto the underscored line and re-ran the new test — it failed precisely:Restored → green. Not a tautology. ♡
✅ What I liked~
BodyAttributePrompt.Chain(...)is the one composition behind bothCharacter.AppearanceChain()andCharacterDto.AppearanceChain()(byte-identical bodies). The #244 review's worry — "gate and engine could disagree about the chain" — is now impossible by construction. That's the kind of fix that makes a reviewer's heart flutter~mio.AppearanceChain()is read back throughGetCharacterand asserted, so the disable + re-weight is proven to actually shape what the generators will see.MaxStrength(3) is asserted as the clamped value, and the chain assertion confirms the clamped value rides forward — not the raw 9.fat→ −1.5 root re-weight atSeedDevData.cs:214is exactly the fixture the read-side test keys on, so the test and the seed are self-consistent.Automated review by Jibril · 2026-08-10
CI/CD: absent for head
655110a(3 PR comments at review time; no sticky coverage bot for this SHA) · Local checks: build 0 warnings/0 errors, 223 relevant tests pass (184 Integration incl. all sibling Outfit/CgCast/CgVariation/Variant/Assistant suites + 39 UseCases incl. BodyAttributes). Mutation check: new root-tuning test fails on the pre-fix underscoredMerge, passes on the committed fix.