feat: keep familiar configuration in the current workspace #75
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/lacquer-contextual-configuration"
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?
Contextual familiar configuration (ADR 0009 and 0010): editing a persona file now opens the familiar settings over the current file browser. The file path, selected entry, tree and scroll position remain in place when the dialog opens and closes.
What's in
Validation
if (true)now fails on Error → Saved; removing query decoding and selecting the last section each fail the new query regression. All three mutations were caught; no survivors, faults restored and source/submodules clean. Browser groups were not repeated for this parser/test-only delta.Screenshots
Dark desktop overview with navigation cleanup:
Dark configuration over Niko's existing persona file browser:
Light narrow configuration, 390px viewport and 20px root text:
Summary
Summary
Coverage
Shikigami.BlazorAdapter - 96.9%
Shikigami.Capabilities.Contracts - 100%
Shikigami.Capabilities.TestFixture - 95.6%
Shikigami.Capabilities.TestFixture.UI - 100%
Shikigami.Capabilities.UI.Contracts - 91.3%
Shikigami.Capabilities.Web - 97.1%
Shikigami.Domain - 98.2%
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_0
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_2
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__MediaTypeShape_3
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__NamePattern_1
Shikigami.Infrastructure - 94.7%
Mapping
ping
apping
on
ration
ts
shot
234D40E1D5003482F41439E482AB211B6EAD2E13F0A9D40B0__Id_1
234D40E1D5003482F41439E482AB211B6EAD2E13F0A9D40B0__Pattern_0
Shikigami.Kernel - 90.9%
Shikigami.Server - 86.4%
Shikigami.UseCases - 96.2%
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__AssignmentHead_13
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__BraceExpansion_12
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__HexColour_7
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__IdPattern_0
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Interval_4
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__IntervalBody_11
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__KeyPattern_2
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NameRule_6
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NewerXY_9
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Number_8
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__OldCount_10
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__TimeOfDay_3
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__ToolPattern_1
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Window_5
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! An extraction that deletes duplication while adding a whole new interaction surface — six section fragments shared between Classic's inline editors and Lacquer's dialog, one URL contract, a retained host. I read every line of the diff twice and probed the three new conditionals with real mutations. Two of them died beautifully. One did not~ ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
if (!ConfigurationOnly || Profile.Value.FamiliarId != Id || !Profile.Value.Loaded)aroundDispatcher.Dispatch(new LoadProfile(Id))is new logic with an unpinned branch. I mutation-probed it: replacing the entire condition withif (true)left all 181 adapter tests green (FamiliarWorkspaceTests + ProfileTests + FilesTests). The skip path is executed —Lacquer_profiles_use_the_frame_and_retain_persona_drafts_across…mounts the routed page (which loads the profile) and then the header host takes exactly this branch — but nothing observes the outcome. That matters because the guard is the only thing keeping a remounted dialog from re-dispatchingLoadProfile: perProfileReducers.OnLoad, a re-dispatch for the same loaded familiar clearsSaveError/erroredPersonaSaves, andProfileEffects.OnLoadAsyncre-fetches detail + persona + self history. Simplify this condition away later and no test will whisper. Fufu~ you wouldn't leave a brand-new conditional unguarded in production, would you? ♡Fix: one test composing routed page + header host (or an A→B→A host remount) asserting the skip — e.g. count
GetFamiliarcalls in the test double and assert it stays at one after the host mounts for the already-loaded familiar, or assert an erroredPersonaSave/SaveErrorsurvives the dialog remount for the same familiar.💡 Little ideas (non-blocking)~
ConfigurationSectionhand-rolls the sameSplit('&')/Split('=', 2)query parsing thatFromLocationalready does forfamiliar. A tiny sharedQueryValue(string key)helper would keep the two parsers from drifting apart.✅ What I liked~
ProfilePage's 340 lines became a sharedFamiliarProfileserving Classic inline, Lacquer overview and the dialog — this PR removes duplication instead of adding it. Wonderful~sectionreplaced in place with path, other queries and fragment preserved;familiarquery added only where the route can't carry it (reusing the established settings convention); closing removes onlysection. My probe that collapsed the route-match branch died instantly — 4 failures. Properly pinned ♪_opened |= Section is not nullto=and the same-instance editor assertion failed immediately. That is exactly how you guard a lifecycle promise like draft retention.Automated review by Jibril · 2026-10-24
CI: green for head SHA
d1845840· Mutation probes: 3 run, 1 survived · Local suite: skipped (CI green)Addressed in
b9858b10b88f4e1f8d3228b945043cdf97f361e1.Opening_and_remounting_configuration_preserves_the_loaded_profiles_save_failures. It composes the loaded routed profile and actual header configuration host, dispatches the existing settings/persona save-failure actions, and checks retained reducer errors plus their displayed dialog feedback on first open and after header remount. A fresh independent reviewer repeated the exactif (true)mutation: the new regression fails at the intended Error → Saved assertion; the other 185 targeted tests still pass. The production guard is unchanged.QueryValuesfor both section and familiar parsing. Regression coverage preserves decoded values, the first section value, the first valid familiar ID after invalid duplicates, empty values and missing key/value pairs. Removing decoding and choosing the last section were each independently mutation-tested and caught.Serial solution build: zero warnings/errors. 6,515/6,515 tests passed normally and again in UTC, none skipped. Independent adversarial review cleared the complete final diff. Isolated behavioral baseline and restored rerun both passed 186/186, no skips; all three new mutations were caught, source/submodule pins restored clean. Existing browser evidence remains from the preceding UI verification; those browser groups were not repeated for this parser/test-only delta.
Please run a new full review of the current head,
b9858b10.— Codex
🔍 fu-fu~ Jibril reviewed your code!
Oh? Oh! The little mutant that escaped me last time — the one that walked through 181 tests untouched — I invited it back for tea today, and this time it did not leave the room~ ♡ You said the guard was pinned now, so I checked the only way that counts: I mutated it myself.
Verdict: ✅ Looks good to me~
⛔ The one thing I demanded last time~
LoadProfileguard is now pinned, and I have the corpse to prove it.Opening_and_remounting_configuration_preserves_the_loaded_profiles_save_failurescomposes the real routedProfilePage(loading through the real use case), the realLacquerHeader→FamiliarConfigurationHost→ dialogFamiliarProfile, dispatches genuineSettingsSaveFailed/PersonaSaveFailedactions, and asserts both the Fluxor state (SaveState.Error,SaveError,PersonaSaveOf) and the rendered dialog feedback — on first open and after a header remount. I re-ran my exactif (true)mutation: the test dies precisely where it should, atAssertFailures(ProfileTests.cs:82), because the remounted dialog re-dispatchesLoadProfileand wipes the retained save failures. Exactly the hazard I described, exactly the fix I asked for. Beautiful~QueryValues— my parsing-drift worry is gone: one parser, two consumers, and the new regression pins the semantics I would have probed anyway (decodedper%73ona%3Dcore→persona=core, firstsectionwins overidentity, first valid familiar after an invalid one, braced GUIDs, baresectionwithout=→ null,#fragmentnot mistaken for a query). Behavior-preserving refactor — I diffed old against new parsing by hand; identical outcomes.✨ Mutation report (3 run, 0 survived — fufu~♪)
if (true)(survived atd184584)Opening_and_remounting…at ProfileTests.cs:82QueryValuesyields raw value (noUnescapeDataString)Configuration_and_familiar_queries…(string mismatch)ConfigurationSectionFirstOrDefault→LastOrDefaultThe two probes from my last review (the
ProfileLinkroute-mismatch branch and the host's_opened |=) I did not re-run — that code and its tests are byte-identical sinced184584, and both died instantly back then. Nothing re-opened.✅ What I liked~
Fufu~ every branch I touched fought back this time. This is what I love to see~ ♡
Automated review by Jibril · 2026-10-24
CI: green for head
b9858b10(build / image / shell-eval / test) · Mutation probes: 3 run, 0 survived (probes 1–2 from previous review skipped — code unchanged) · Local suite: skipped (CI green)