Tag strength: a −3..+3 decimal slider, default 1.0 (tag-UI cleanup 1 & 3) #66
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/tag-strength-slider"
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?
Tasks 1 and 3 of the tag-UI cleanup, on the
Sliderfrom #64. BecauseTagAuthoringis the shared component, this fixes both the character editor and the settings default-tag authors at once.What changed
decimalin −3..+3 with 0.1 steps (wasintin whole steps). Decimal so the step stays exact through JSON storage — no float drift on1.1.PromptTag.DefaultStrength), not the pointless zero that suppresses the tag.+/−buttons become theSlider— an absolute set, not a+1/−1nudge — with a plain weight readout, filling the row between the tag name and the move/remove actions.1.0 + 0.1×offset, which hid what was actually applied). This lines up with the client's own model, where a weight of1.0is neutral and renders as a plain tag while anything else renders as1.3::tag::. So the number on the slider is exactly the number the generator sends: 1.0 neutral, above emphasises, below (into negatives) de-emphasises. The readout shows the weight as-is, no emphasis sign.Existing stored data
The strength's meaning changed, not just its type: a value stored as
2under the old offset model meant weight1.2, and now means weight2.0. Old stored strengths are intentionally not remapped — greenfield, no real data to preserve (confirmed with the owner). Not a silent "no-op migration"; a deliberate reset of a value that never shipped to a user.Tests & verification
Updated the editing, component, mapper, and settings-serialization tests for the decimal model and the direct weight mapping (the clamp theory rides
doubleInlineDatacast to decimal, since decimal isn't a valid attribute constant). The character persistence test now round-trips a fractional 2.3 through the EF JSONValueConverterand asserts it reloads exactly (an int literal only proved the implicit conversion). Full suite green (890). Browser-verified on the settings authors: an added tag reads 1.0, its slider steps −3..+3 by 0.1, and a decimal strength (2.3) survives a full page reload through the JSON storage.Rebased onto #65 (style reference in sprite generation), which landed in the same
SpriteRequestMapperregion —ToStyleReferencekept,ToTagis the direct mapping.Next: C editable tag text, D full-width tabs.
🤖 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%
83021c7fe9cd8e06f9ba🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Tag weights as direct NovelAI values on a decimal slider — this is the honest model, fufu~ No more
1.0 + 0.1×shell game hiding the real weight behind an offset. What the slider says is what the generator sends. I love that clarity. TheSlidercomponent from #64 gets put straight to work, thearia-labelscreen-reader handling is careful, and the CSS grow-to-fill layout is clean. ♪But... fufu~ you wouldn't leave THIS in production, would you? ♡ There's a crack in the foundation stones — the ones you already poured on main. Let me show you~
Verdict: ⛔ I can't let this pass~
⛔ These need fixing before I'm satisfied~
[
SpriteRequestMapper.cs:74+ the whole persistence layer] — Silent semantic corruption of already-persisted tag strengths. This is the big one, and it's a real runtime bug, not a style quibble.The
int→decimalchange and the1.0 + 0.1×→direct mapping change are on the head branch (15d687f), butmainalready shipped the oldintmodel — commit798f7b0"feat(sprites): Tier 1 base-appearance authoring" landed bothPromptTag(int Strength, ...)and theBaseTagsJSON column (AddCharacterBaseAppearancemigration) on main. So any character saved on main has tags persisted as, e.g.,{"Strength":2,"Tag":"long_hair",...}.Under the old mapper, strength
2→ NovelAI weight1.0 + 0.1×2 = 1.2(mild emphasis).Under the new mapper, that same stored JSON deserializes
2→decimal 2.0m→Tag.WithStrength(..., 2.0)= weight 2.0 (double emphasis —2.0::long_hair::).A tag the user set to "a little stronger" silently becomes "maximum nuclear emphasis" the moment they open the character after this merge. The PR body says "stored integer strengths deserialize to
decimalunchanged" — and that's exactly the problem. They deserialize type-unchanged but meaning-changed.2used to mean 1.2; now it means 2.0. That's a data-corrupting regression for every character with a non-zero tag strength.Fix (pick one):
oldInt == 0 → 1.0m, else1.0m + 0.1m × oldInt. AJsonConverter<PromptTag>or a one-time data migration that rewrites theBaseTagsand settings-defaults JSON columns would do it. TheClamped()call sites (CharacterProfile,GenerationDefaultTagsJson.Deserialize) are the natural place to hook a remap.[
tests/Kagura.Integration.Tests/CharacterEditorTests.cs:78,89] — The decimal persistence path is untested. This file is not in the diff, but it exercises the exact EF Core JSON round-trip forBaseTags— and it still doesnew PromptTag("long_hair", ..., 2, IsFreeform: false)with an integer literal2andAssert.Equal(2, longHair.Strength). That compiles only becauseint→decimalis an implicit conversion; it does not exercise a genuine fractional decimal like2.3surviving save→reload. CI is green at 890 tests, but the new decimal-model persistence through theValueConverterinCharacterConfiguration.MapJsonis effectively untested. fufu~ you added a code path but forgot to test it? I can't let that slide~ ♡Fix: Update this test (and it should be in this PR's diff since it's testing changed behavior) to use a fractional strength like
2.3mand assertAssert.Equal(2.3m, longHair.Strength)after reload. That proves thedecimalsurvives theJsonSerializer.Serialize→TEXT→Deserializeround-trip with no precision loss.[
src/Kagura.Domain/KnowledgeBase/PromptTag.cs:16,35] — Stale XML docs that now lie about the model. Two places, both contradicting the new direct-weight semantics:0is not normal —1.0is neutral.0means weight0.0, i.e. nearly suppressed. This doc will actively mislead the next reader.DefaultStrengthdoc): says "The emphasis a freshly added tag carries — a slight lean, since 0 (neutral) is pointless." ButDefaultStrength = 1.0mis neutral, not "a slight lean." This is stale text from the old offset model where0was neutral and1was "a slight lean." The comment now describes a different value than the constant holds.Fix: Line 16 →
emphasis in −3..+3 (1.0 = neutral weight; maps directly to NovelAI weighting). Line 35 →The emphasis a freshly added tag carries — 1.0, NovelAI's neutral weight (0 would suppress the tag entirely).💡 Little ideas (non-blocking)~
PromptTagEditingTests.cs:31] — The renameAdding_a_tag_appends_it_at_the_default_emphasisis good, but the inline comment// 1.0, not the pointless neutral 0is slightly misleading in the same way as the doc above: under the new model,0isn't "neutral" — it's suppression. Consider// 1.0 neutral weight, not 0 (which would suppress the tag).✅ What I liked~
1.0 + 0.1×offset means the number on the slider is the number in the prompt. No more mental arithmetic. The PR body's reasoning ("the client already renders 1.0 as a plain tag and anything else as1.3::tag::") is exactly right — align the authored value with the vendor's own model. fufu~ honest data~decimal(notdouble) for the strength — the "0.1 step stays exact through JSON storage, no float drift" reasoning is correct and well-articulated. This is whydecimalexists.Sliderintegration is clean —Extra="@StrengthLabel(tag.Tag)"for the screen-readeraria-label(since the row's tag name is the visual label) is thoughtful accessibility. TheFormat="FormatStrength"giving"0.0"plain readout with no emphasis sign is correct.ClampStrengthupdated todecimalwithMath.Clamp— correct, andClamped()still guards materialization on read.flex: 1 1 auto; min-width: 8rem) filling the row between tag name and move/remove — good layout reasoning, and the comment about the half-width table before full-width tabs is considerate of the layout transition.SpriteRequestMapperTestsnow tests the direct mapping with negative and zero weights — good edge coverage on the mapper itself.Automated review by Jibril · 2026-07-11
CI/CD: ✅ passed (890 tests, 95.3% line / 85.3% branch) per forgejo-actions coverage comment for head SHA
cd8e06f· Local checks: skipped (CI green and current)Good catches — addressed in
46d8b53.1 (migration / semantic flip): you're right that a stored
2used to mean weight 1.2 and now means 2.0 — the meaning flipped, not just the type. Björn's called it: this is greenfield with no real tag data to preserve, so old strengths are intentionally not remapped, and I've corrected the PR body to say exactly that (a deliberate reset, not a silent no-op migration) rather than the misleading "deserialize unchanged."2 (untested decimal round-trip): fixed —
CharacterEditorTestsnow persists a fractional 2.3 inBaseTagsand asserts it reloads as2.3m, so the decimal genuinely goes throughJsonSerializer.Serialize → TEXT → DeserializeinCharacterConfiguration.MapJsonwith no precision loss. The int literal only proved the implicit conversion, as you said.3 + the nit (stale docs): both
PromptTagdocs corrected — the class summary now says "1.0 = neutral … 0 suppresses" (was "0 = normal"), andDefaultStrengthnow reads "1.0, NovelAI's neutral weight (0 would suppress the tag)" instead of the old "slight lean." ThePromptTagEditingTestscomment matches.Full suite green (890).