feat(expressions): adopt library emotions onto an outfit — the expression grid (ADR 0029) #95
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/expression-adoption"
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?
Slice 3a of the expression tier (ADR 0029, outfit-expressions story): the
Expressionrow — an outfit adopting a library emotion — and the card grid on the Expressions tab. Slice 3b (per-expression inpainting through the queue) completes the arc; the cards already carry the image slot it fills.Domain & persistence
Expression— one adopted emotion of an outfit: project-scoped like its outfit (the referenced emotion is the one global thing it points at), journaled and soft-deleted (removal is reversible, and a removed row keeps referencing its generated image file).ImageFileNameis presence-only likeOutfit.ImageFileName, set by generation in 3b, never by adoption.EfExpressionStorewith a batchAddManyAsync— one save = one journaled operation, so "add these five" undoes together (ADR 0020). No FKs, same composition reasoning as the outfit.AddExpressionsmigration (indexes onOutfitIdandProjectId).Use cases
AdoptEmotions— batched, opt-in: dedupes against what's already adopted, quietly skips unknown/soft-deleted library emotions (the picker only offers valid ones; a race no-ops), rows a tick apart so a batch keeps the library's order, and returns the refreshed grid so the UI needs no second round trip.ListExpressions— joins each row with its emotion's name through the newIEmotionStore.NamesAsync, which reads ignoring the soft-delete filter: deleting a library emotion removes it from pickers but never blanks the cards that adopted it (the ADR 0029 rule, crossing the project/global boundary).RemoveExpression— soft.UI
The Expressions tab gains the grid under the face region (both read their own Fluxor stores — the tab-fragment convention from #94):
EmotionLibraryStateslice — no new query), batch-added in library order; the Add button counts the picks.Tests
Browser-verified
Both themes: created an outfit, adopted 3 of the 8 seeded emotions from the picker (offered in curated order), re-opened the picker (exactly the 5 unadopted offered), removed one with confirm, and a full reload restored the grid from the database. Dev data cleaned afterwards.
🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagura.BlazorAdapter - 90.1%
Kagura.Domain - 93.4%
D04ADFED3A21D401C2764A1D17367E35BEB556CBB3B4B0B74__NonSlugChars_0
Kagura.Infrastructure - 98.3%
n
on
Kagura.Kernel - 90%
Kagura.Server - 92.8%
Kagura.UI - 96.3%
Kagura.UseCases - 96.3%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh~ A new expression grid slice for ADR 0029! The domain model is lovely — presence-only
ImageFileNamemirroring the outfit, the soft-delete-with-still-referenced-image semantics, the tick-apart adoption ordering, theNamesAsynccrossing the project/global boundary while ignoring soft-delete filters... wonderful. The reasoning in the XML docs reads like a love letter to the domain. I purred readingExpressionConfiguration. ♡But fufu~ you know I read everything, right? Every sibling, every effect, every reducer... I compared this slice against the whole KnowledgeBase family. And I found two things I absolutely cannot let slide~ ♡
Verdict: ⛔ I can't let this pass~
⛔ These need fixing before I'm satisfied~
ExpressionsState.cs— missingOnDomainChangesAsynchandler (ADR 0016 cross-session sync + undo refresh). This is the big one.Every other editor/list Effects class in KnowledgeBase implements
[EffectMethod] public Task OnDomainChangesAsync(DomainChangesReceived action, IDispatcher dispatcher)— I counted:OutfitEditorState,OutfitsState,CharacterEditorState,LocationsState,LoreState,RelationshipsState,TimelineState,LocationHierarchyState,LocationVariantsState,LocationVariantEditorState.ExpressionsStatehas none. This handler does two things across the codebase:DomainChangedbus event arrives (journal committed), sibling editors re-query so a remote origin (undo store replay, another tab) reflects. Without it, if the user undoes an adoption/removal from a different session or the undo is replayed by a reconnect, this grid goes stale and shows expressions that no longer exist (or hides ones that were restored).And
EntityKindsis missing theExpressionconstant. The codebase itself documents this exact trap insrc/Kagura.UseCases/Notifications/EntityKinds.cs:Fix:
Mirror exactly what
OutfitsState.OnDomainChangesAsync/OutfitEditorState.OnDomainChangesAsyncdo. This is a real runtime bug, not a nicety — the undo path is exercised by every user who hits Ctrl+Z while the Expressions tab is mounted.OutfitExpressions.razor— the picker briefly flashes the "everything adopted" empty state on first open. When the user clicks Add emotions for the first time,OpenPicker()dispatchesLoadEmotionLibrary(lazy) and sets_pickerOpen = trueimmediately. ButLibrary.Value.Emotionsis still[]untilEmotionLibraryLoadedarrives.Adoptablethen evaluatesLibrary.Value.Emotions.Where(...)→ empty → the picker renders "Every library emotion is already adopted — manage the library in settings to add more." for one render cycle, then flips to the real list. On a slow connection the flicker is jarring and the message is a lie.Fix: gate on
Library.Value.Loaded:(The
Loadedflag already exists onEmotionLibraryState— use it.)💡 Little ideas (non-blocking)~
OutfitExpressionsTests.cs— no test exercises the deleted-emotionAdoptableexclusion, only the picker-offered list. Consider one bUnit test that seeds two emotions, adopts one, deletes the other from the library, opens the picker, and asserts the deleted one is absent fromAdoptable(not just that the live ones are present). Right now the "deleted emotion leaves the picker" branch is only tested in the negative ("every adopted" path). Tightens the ADR-0029-crosses-boundary guarantee at the adapter layer.EfExpressionStore.ListForOutfitAsync— theAsNoTracking+OrderBy(CreatedAt)is correct, but unlike the outfit's sibling it has noOrderByDescending-style comment explaining the order contract. Tiny — a one-liner// Grid order = adoption order (oldest first); the seeder-style tick spacing makes it deterministic.would mirror the sibling's discipline.✅ What I liked~
AddManyAsync= one journaled operation design is exactly right — "add these five undoes together" (ADR 0020) is honored precisely because it's oneSaveChangesAsync. Beautiful~ ♡NamesAsyncreading withIgnoreQueryFiltersso an adopted card keeps its label after the library entry is soft-deleted — the ADR 0029 boundary-crossing rule, implemented at exactly the right layer (the emotion store, not patched up in the UI). chef's kiss ♪OnRemovedreducer removing by id rather than refetching is a nice touch — instant UI feedback, no round trip.Automated review by Jibril · 2026-07-12
CI/CD: passed for head SHA
f833062(1126 green, 96.2% line / 83.4% branch coverage) · Local checks: skipped (CI current and green)🔮 fufu~ Jibril reviewed your code!
Welcome back~ ♡ I see you pushed
10debfe— let me check every single thing I asked for last time. I'm thorough, you know that. Fufu~Verdict: ✅ Looks good to me~
Both blocking issues are resolved, and — oh? You even picked up my non-blocking suggestions? That makes me happy. ♡
✅ Fixed since last review~
EntityKinds.Expression+OnDomainChangesAsync— DONE. ✅EntityKinds.Expression = nameof(Domain.KnowledgeBase.Expression)is present.ExpressionsEffects.OnDomainChangesAsyncis implemented, guards onoutfitId != Guid.Emptyandaction.Touches(EntityKinds.Expression), dispatchesLoadExpressions. This matches the sibling pattern exactly. The cross-session sync (ADR 0016) and the undo/redo refresh path are now wired. The threeExpressionsEffectsTests(expression change reloads, unmounted grid stays quiet, unrelated change ignored) confirm it behaves correctly. Wonderful. ♪Picker "everything adopted" flash — FIXED. ✅ The picker now branches on
!Library.Value.Loadedfirst ("Loading the emotion library…"), thenAdoptable.Count == 0("Every library emotion is already adopted…"), then the list. The lie can no longer flash. Clean.Bonus — you adopted the non-blocking suggestions too! ✅
A_deleted_library_emotion_leaves_the_picker_while_live_ones_staynow directly tests the deleted-emotion exclusion fromAdoptable(Angry is soft-deleted, only Sad is offered). The ADR-0029 boundary is now exercised at the adapter layer.EfExpressionStore.ListForOutfitAsyncnow carries the// Grid order = adoption order (oldest first)comment matching its sibling's discipline.✅ What I liked~ (still true from last time)
AddManyAsync= one journaled operation — "add these five undoes together" (ADR 0020). Still beautiful~ ♡NamesAsyncwithIgnoreQueryFilters— the ADR 0029 boundary-crossing rule at exactly the right layer. The integration testAn_adopted_expression_keeps_its_name_after_the_emotion_is_deletedproves it over real SQLite.This slice is ready to ship~ fufu♪
Automated review by Jibril · 2026-07-12
CI/CD: passed for head SHA
10debfe(96.2% line / 83.4% branch coverage) · Local checks: skipped (CI current and green)