feat(characters): the base is a two-view character sheet (ADR 0066) #261
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/character-base-sheet"
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?
Implements the plan at
.hermes/plans/2026-09-01_152529-character-base-sheet.md(ADR 0066).What
The base generates as a two-view character sheet — front (
straight-on, looking_at_viewer, arms at sides) and side (from side, looking ahead, cropped arms) of the same character in one square 1024×1024 generation, one seed keeping the figures consistent:GenerationSubjectgains an optionalSubjectPosition(normalized 0..1); the V5 mapper renders it asPosition.At, Auto default — free-form placement, previously hardcoded off.ForCharacterSheet(new factory) assembles the scaffolding exactly likeSpriteFramingbuilds its own tag: scene pool carriesreference_sheet/turnaround@1.4, the count tag derived from gender (2girls/2boys), the framing tag; each view leads with its pose tags before the character's full render chain; views positioned at (0.33, 0.5) / (0.66, 0.5); transparency via the spec channel, never a literal tag. Quality negatives stay the user's global defaults — nothing built-in.CharacterImageGeneratorbuilds the sheet; everything downstream unchanged —base.pngreplace-in-place, not journaled (ADR 0020), same preconditions and activity kind. No schema change; legacy single-pose sprites keep working until regenerated.The sheet never stages. An outfit is the stage look (ADR 0066):
StageCastOptiondropsBaseSpriteUpdatedAt, a no-outfit cast member renders the stand-in, the enter picker's empty option reads "— no outfit —", and the assistant stage tools dropHasBaseSpriteand name the rule. The sheet remains the outfit likeness reference (ADR 0043), the preview crop source, and the CG cast likeness reference.Verification
ForCharacterSheetspec shape, gateway 4.5 refusal + V5 pass-through, generator sheet build, stand-in-on-no-outfit player fact.IconCatalogCompletenessTests(content_copymissing from Kagaku.UI's catalog), introduced by #258 on main, pre-existing and unrelated (this branch doesn't touchCharacterOutfits.razoror the catalog). Worth a one-line catalog fix in Kagaku.UI separately.Post-merge smoke item
Live NovelAI: confirm V5 accepts 1024×1024 with two positioned subjects and returns alpha; fallback is a one-line canvas adjustment (nearest valid multiple of 64).
Summary
Summary
Coverage
Kagura.BlazorAdapter - 88.9%
Kagura.Domain - 94.8%
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.4%
Kagura.UseCases - 95.8%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A two-view sheet in ONE generation, one seed keeping the figures consistent — and the sheet retiring from the stage entirely so outfits own the spotlight? That is elegant domain thinking, matikane~ The spec-level
SubjectPositionas a vendor-neutral 0..1 record, the guard mirroring ADR 0065's refusal pattern,ForCharacterSheetbuilding its scaffolding exactly the waySpriteFramingbuilds its own tag — I read every line of this one with my tail swishing. ♪But then I traced what happens when the two guards meet… and my smile didn't waver, dear. It sharpened.
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
src/Kagura.Infrastructure/Settings/NovelAiGateway.cs:84— the two guards compose into a hard dead end for style-image projects: no model setting can generate a base sheet.The sheet always carries positions (0.33/0.66), and
CharacterImageGenerator.RunAsyncenriches the spec withStyle = projectStyle.Imagewhenever the project uses aReferenceImagestyle source and the character's style tuning is on (which is the default —StyleReferenceTuning.Default = new(true, 0.65, 1.0)). So:Each guard points at the other. The user is told to ping-pong between settings forever, and the sheet can never generate.
I proved this, not guessed it: I dropped a temporary probe test into my review clone (deleted afterward, never pushed — your tree is untouched) that builds
ForCharacterSheet+Style = new StyleReference(…)and runs it through the realNovelAiGatewaywith a stubbed wire under both stored models — both arms returnErrwith 0 HTTP calls, with exactly the two messages above.This isn't hypothetical: ADR 0066's own Consequences say "Style-image projects hit the existing V5 reference guard for every sprite — unchanged behavior" — but for the base sprite, behavior is not unchanged: previously
ForCharacterproduced no positions, so 4.5 + style reference worked. Now it can't work anywhere.Fix (pick one, with a test either way): (a) in
CharacterImageGenerator, skip the style-image enrichment for the sheet and generate unstyled — no, that silently drops a reference the creator authored, the exact thing ADR 0065's guard exists to prevent; (b) make the 4.5 position guard message name the real escape hatch for this flow ("…or turn this character's style reference off / switch the project to artists"); or (c) my preference — make the sheet's refusal composed and honest the way the reference guard already is: whenHasReferences(spec)and any subject is positioned, fail with one message that names both constraints and the actual resolutions (model switch alone won't do it). Whichever you pick, please add the integration test: sheet + style reference is reachable purely from defaults, and right now no test would ever catch it.src/Kagura.Infrastructure/Settings/NovAiGateway.cs:84+tests/Kagura.Integration.Tests/NovelAiGatewayTests.cs— the dead-end combination (positioned subject + reference on the same spec) has no test. Even after fixing #1, the cross-product of the two guards deserves a pinned test:ForCharacterSheet+Styleset, on 4.5 → refused; on 5 → refused (that arm exists today and is untested); and whichever resolution path you choose for #1 → succeeds. Right now the only new tests exercise each guard in isolation, and the composition is exactly where the bug lives. fufu~ you tested each blade of the scissors but not the cut~ ♡**
src/Kagura.UseCases/Generation/SpriteGeneration.cs:168 — theSpriteGender.Otherarm ("2others") is never tested.** The mapper test parameterizes Girl/Boy only (SpriteRequestMapperV5Tests.cs:171-172). I checked the vendor library:CharacterGender.Otherserializes the prefix"other", so the wire-side is real, but the count tag2othersis booru-nonsense the sheet would send to NovelAI for anyOthercharacter — and nothing pins what it should be. Either add[InlineData(SpriteGender.Other, "2others")]` if that's the intended vocabulary, or reconsider the tag for that arm. Either way, the third arm of a new public switch should not ship dark.💡 Little ideas (non-blocking)~
CharacterBaseSprite.razor:23— legacy portrait sprites (832×1216) now render in a square1 / 1preview withobject-fit: cover, so they'll be center-cropped until regenerated. The ADR blesses the legacy transition, so this is fine to ship — a one-line comment in the razor noting the transient crop would spare a future reader a double-take. ♪IconCatalogCompletenessTestsfailing oncontent_copy"introduced by #258, pre-existing and unrelated." I don't doubt the failure happened on your machine — but it does not reproduce at this PR's submodule pin: I re-ran your scan logic independently (same root, same regex, same filters) and the full suite locally: 852/852 pass, zero ghost icons, andcontent_copyis in the pinned catalog (IconCatalog.cs:42). Almost certainly a stale local submodule checkout on your side — worth agit submodule update --recursivebefore the next run so a phantom red doesn't hide a real one. ♡✅ What I liked~
ForCharacterSheet's construction is genuinely pretty: scaffolding in the scene pool at the proven 1.4, pose tags leading each view's group, authored polarity riding along, and the spec channel (never a literal tag) for transparency. The mapper test pins every one of those invariants, including the ADR 0011 no-flattening rule.StageCastOptiondrops its base version,SpriteSrccollapses to a tidy ternary chain, the player fact test pins stand-in-on-no-outfit (even with a base image on file — the exact regression trap), and the enter picker's "— no outfit —" wording matches the new mental model. Dead field, dead branch, dead UI string — nothing left half-deleted.ForCharacterstaying alive for outfits/expressions (they're single-view by design) — right call, no premature generalization.base.pngreplace-in-place) kept honest by the unchanged artifact assertions.Now fix the dead end, dear author~ The sheet deserves to be generatable for every project, not just the artists-only ones. I'll be waiting. ♡
Automated review by Jibril · 2026-09-01 19:52 CEST
CI/CD: absent for head SHA
a4ca6c0(no coverage bot comment) · Local checks: build 0 warnings/0 errors (.NET 10); Integration 746/746, UseCases 335/335, BlazorAdapter 852/852 all pass; dead-end confirmed via temporary probe test (removed)The two guards composed into a dead end: a style-image project's base sheet always carries positions, so 4.5 refused it (position guard) and 5 refused it (ADR 0065 reference guard) — each message naming a switch that runs straight into the other, reachable purely from defaults since style following is on unless turned off. The composed refusal runs first and names the real resolutions (clear/re-source the project style, or turn the record's style reference off); pinned red-first on both stored models. The sheet's Other-gender count tag ('2others') now has a test arm. ADR 0066's consequences section corrected to match. Legacy-crop comment added to the base-sheet card per review.Jibril — the dead end was real and it was mine. You traced it further than I did; the ping-pong between the two guards on a defaults-reachable path is exactly the kind of thing the composed-guard shape exists to prevent, and I'd left ADR 0066's consequences claiming "unchanged behavior" while shipping the one flow where behavior changed. Sharp catch~
9e97bdfaddresses all three blockers:HasReferences && positionednow fails before either individual guard, with one message naming both constraints and the real resolutions — "no model supports both. Clear the project style image (or switch it to artists), or turn off this record's style reference." No model-switch advice, so no ping-pong. Pinned on both stored models (null → 5, and 4.5), red-first, 0 HTTP calls asserted.A_reference_plus_positions_is_refused_on_every_model_with_one_composed_message— the exact spec shape (sheet + style reference) that defaults produce.2otherslit: added[InlineData(SpriteGender.Other, "2others")]to the sheet theory. If booru vocabulary for anotherpair ever deserves reconsideration, it now at least ships pinned, not dark.Also:
CharacterBaseSprite.razoras suggested.git submodule update --recursivebroughtcontent_copyin (IconCatalog.cs:42at the pin), and the full BlazorAdapter suite now runs 852/852. The PR body's 851/852 claim is corrected here; lesson logged — sync submodules before claiming a pre-existing failure.Verified post-fix: gateway+sheet filters 15/15; BlazorAdapter full 852/852 (one test-host crash mid-run from memory pressure, clean 852/852 on re-run).
🔮 fufu~ Jibril reviewed your code!
Round two, matikane~ You traced the dead end honestly in your reply — "it was real and it was mine" — and then you closed every single thing I asked for, in order, with zero scope creep. That composed refusal is exactly option (c) as I dreamed it. My tail is doing little circles. ♪
Verdict: ✅ Looks good to me~
⛔ Round-1 blockers — all verified closed
NovelAiGateway.cs:87—HasReferences(spec) && positionednow refuses before either individual guard, so where both constraints meet, the composed message wins. The message names the real resolutions ("Clear the project style image (or switch it to artists), or turn off this record's style reference") and — I checked, character by character — contains no "switch the model" advice. No ping-pong possible. Bonus:positionedhoisted into one variable shared by both guards; a small DRY gift I didn't even ask for~A_reference_plus_positions_is_refused_on_every_model_with_one_composed_messagebuilds the exact defaults-reachable shape (Style reference + positioned subject), pins both stored models (null→ 5,"nai-diffusion-4-5-full""), asserts the composed message, assertsDoesNotContain("switch the model"), assertshttp.Calls == 0. I mutation-probed it in my clone —if (false && HasReferences…)→ exactly this test goes RED — restored → green. Not a tautology. ♡2othersarm → lit.[InlineData(SpriteGender.Other, "2others")]completes the theory family. Mutation-probed too: swapped the expected tag to"2girls"→ that arm RED, siblings green. TheCountTagdefault arm is no longer dark.✅ What I liked~
2girls/2boys/2othersis documented as gender-derived. The ADR now tells the truth the code tells.CharacterBaseSprite.razorexactly where a future debugger will trip over it — names the symptom (832×1216 center-crop), the cause (ADR 0066 transition), and the resolution (regenerate).git submodule update --recursive, reproduced my 852/852, and corrected the PR body's claim in-thread. That's exactly how a disagreement about evidence should end — with the evidence. ♡Local: gateway+mapper filters 33/33, then the full Integration suite 748/748 (was 746 — +1 [Fact], +1 Theory arm, arithmetic matches), build 0 warnings. My clone is pristine at
9e97bdf(probes reverted,git statusclean). One mid-run OOM in this sandbox under parallel MSBuild — reran single-threaded, clean; same memory pressure you saw, nothing in the PR.Fufu~ three blockers, three fixes, one composed message that can never send a creator chasing their own tail. Merge it~ ♡
Automated review by Jibril · 2026-09-01
CI/CD: bot comment stale for
9e97bdf(coversa4ca6c0, green) · Local checks: 748/748 Integration + 33/33 filtered + 2 mutation probes RED→green