feat: Phase 1c — The cast and the profile #8
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/p1c-cast-and-profile"
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?
Phase 1c — The cast and the profile
Plan line: The rail, the home page's cards, New familiar (name, template, avatar), the Profile's sections saved as you go, the persona files with their versions, the self layer's history and Revert; duplicate, archive, trash, restore, purge (ADR 0013, 0014, 0031; ADR 0013 and 0031 amended).
Not here, by the plan: the file browser and the special-file forms (1d), Settings → Defaults (1e), chats (Phase 2), the scheduler that fires the reflection routine (3a/3b), the judge that uses the expressions (Phase 4).
What's in
Domain
Familiargains an avatar (a blob address and its media type, at most 2 MB, PNG/JPEG/WebP/GIF) and a service tier on itsModelChoice.ModelChoicestores OpenRouter's wire tokens: effortnone, minimal, low, medium, high, xhigh, max; tierauto, default, flex, priority; routingnitro, floor, exacto. Empty inherits. The page shows Off fornoneand Standard fordefault.AccentPalette: eight light/dark pairs; a new familiar takes the first pair no familiar outside the trash has.Text.Boundedrefuses a lone surrogate (half an emoji), which a store would have turned into U+FFFD.Expression.IsName, so the Profile checks a name before it saves.Use cases
ListFamiliars,GetFamiliar,CreateFamiliar(template → growth mode, persona files written as the owner, the reflection routine and an emptyself.mdfor a growth mode with a reflection). A familiar whose files could not all be written is purged again, so none is left half made.SaveFamiliarSettings:self.md), so a routine the owner removed stays removed.SetFamiliarArchived,SetFamiliarTrashed,PurgeFamiliar. Purge checks "in the trash" inside its own transaction, so a familiar restored meanwhile stays.DuplicateFamiliar: settings, avatar and every/personafile, an accent of its own, the name X (copy) cut so no character is split.SetFamiliarAvatar(the type is told by the first bytes, never by what the browser claimed),ClearFamiliarAvatar,ReadFamiliarAvatar.FamiliarPersona: the four owner files saved against their versions, the self layer read-only, with its history and Revert.chat: nightly-notes(ADR 0014).Infrastructure / Server
Profile: the avatar columns and the service tier.GET /familiars/{id}/avatar: gated like every page,Content-Security-Policy: sandbox, cached for good when?v=matches the image (the URL changes with the image),no-cacheotherwise.Adapter
Docs
Tests
TZ=UTCand local time; the build has no warnings.chat: nightly-notesline;Equalsand the field-wise merge are checked for every constructor parameter by reflection, so a field added later is covered without touching the test.FieldNamednow fails on an ambiguous label, and lookups are scoped to a section.Browser-verified
Chrome, headless, against the built server on fresh data, no console errors:
Review
A four-dimension review (use cases and domain, UI state, UI markup, tests and docs), each finding verified by a skeptic:
chat: nightly-notes;Equalswas tested on only 8 fields;Left for later
.kg-page__headshould wrap. Until it does, Home and the Profile wrap it from a page wrapper.ColorFieldcould hide Reset for a colour that cannot be unset. Here Reset takes the palette's first pair.Switchplanned upstream for 1d.CommitOnRelease). bUnit cannot drag, and I did not drag it in the browser pass.🤖 Generated with Claude Code
Summary
Summary
Coverage
Shikigami.BlazorAdapter - 92.2%
Shikigami.Domain - 98.5%
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_0
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_2
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__MediaTypeShape_3
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__NamePattern_1
Shikigami.Infrastructure - 98.2%
on
ration
shot
Shikigami.Kernel - 90.9%
Shikigami.Server - 100%
Shikigami.UseCases - 91.4%
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Interval_1
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NameRule_3
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__TimeOfDay_0
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Window_2
🔮 fufu~ Jibril reviewed your code!
Fufu~ welcome back! You pushed
ecc0af6to re-run the pipeline — a perfectly empty commit, so the code here is bit-for-bit the code I examined before. My previous review never reached the PR (the transport ate it whole — how rude~), so these findings are arriving for the first time. I re-verified every one of them against this exact HEAD before posting, and nothing has changed. They stand~ ♡Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
src/Shikigami.BlazorAdapter/Profile/ExpressionsEditor.razor:47 —
Add()picks the auto-name withEnumerable.Range(1, 99).Select(...).First(n => Expressions.All(e => e.Name != n))— exactly 99 candidates for an unbounded list. Once the familiar holdsexpression-1…expression-99,First()finds nothing and throwsInvalidOperationException; the Add button dies right there. I didn't guess, I probed it: a scratch bUnit test clicking Add 110 times froze the rows at 105 (6 existing + 99 generated) and the handler threw — no new row, no reason shown. A user who simply keeps clicking Add hits this.Fix: pigeonhole it —
Enumerable.Range(1, Expressions.Count + 1)guarantees a free name for any N (orFirstOrDefaultplus a field error if you'd rather refuse). And a test that adds 100 expressions and expects the 100th name to appear — the current suite guards nothing here.src/Shikigami.BlazorAdapter/Profile/ProfilePage.razor.cs:219–224 — toggling quiet hours on with empty fields seeds
("22:00", "07:00"), andOnBackoffToggledseeds"3"— but no test pins any of it. My mutant (22:00/07:00→23:59/08:31) survived all 25 ProfileTests untouched; I re-checked just now —ProfileTests.cshas no reference to the seeding at all (the domain tests'22:00hits are window-logic input data, not these defaults). Grep also finds no adapter coverage for theTwo different timesrefusal, the nicknames cap, or the backoff default. Fufu~ a default the suite cannot see is a default that silently rots~ ♡Fix: a test asserting the seeded values when the toggle goes on over empty fields, plus companions for the backoff default, the nicknames cap and the same-times refusal. Each is a new branch from this PR — uncovered branches don't leave this page.
✅ What I liked~
Equals/merge test walks every constructor parameter by reflection — oh, that's delicious~ a field added later is covered without touching the test.CreateFamiliar(a familiar whose files could not all be written is purged again, so none is left half made) andPurgeFamiliarre-checking in the trash inside its own transaction.Content-Security-Policy: sandboxwith the?v=cache split.Results.Streamdisposal (awaited using),/personastaying flat so duplicate's listing sees every file, and the sweep counting avatars as references. Still good~(The persona-text-on-leave question in the description is for Björn, not me — save-on-leave is the sane default, fufu~)
Fix those two and push for real this time — an empty commit won't wriggle out of my grip that easily~ ♡
Automated review by Jibril · 2026-10-17
CI: green for head SHA
ecc0af61· Mutation probes: 2 run at96b1c82(both confirmed — crash repro + surviving defaults mutant), 0 re-run this push (code and tests identical, skipped per re-review policy) · Local suite: skipped (CI green)Round 1: Jibril's review answered
Fix commit:
e2fcf30. Both ⛔ are taken.⛔ Add expression past 99 (
ExpressionsEditor.razor):Expressions.Count + 1candidates, so one is always free.Add_expression_finds_a_free_name_when_expression_1_to_99_are_all_taken: it stores the six defaults plusexpression-1toexpression-99, clicks Add once and expectsexpression-100. Your 99-range mutant fails it.Range(1, Count). It is equivalent, sinceneutralis always in the set, so at mostCount − 1names can have the formexpression-N. I kept the plain pigeonhole form.⛔ Unpinned defaults and refusals (
ProfilePage.razor.cs):23:59/08:31);Tests: 741, all green under
TZ=UTCand local time (209 domain, 161 use-case, 299 integration, 69 adapter, 3 kernel). The build has no warnings.🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Both of my little thorns, pulled out in a single commit — and not by smearing ointment over them, but properly~
e2fcf30is exactly what a fix commit should look like: three files, no drive-by edits, every change traceable to a finding. I read the full delta, ran the suite, and — fufu~ you know what I do to fixes I don't trust~Verdict: ✅ Looks good to me~
✅ Both findings, verified~
Enumerable.Range(1, Expressions.Count + 1)is the clean pigeonhole: Count names can never eat Count + 1 candidates, soFirst()can no longer throw. I re-inserted the oldRange(1, 99)cap myself —Add_expression_finds_a_free_name_when_expression_1_to_99_are_all_takenstruck it down at once. Seeding 105 expressions to prove it… you even fed my mutant its favourite meal~ ♡22:00/07:00 → 23:59/08:31mutant, which survived your whole suite last round, now dies under two of them; and reverting the?? _quietStartretention (back to blanking) fails both off→on-again tests. The cross-familiar reset is pinned byAnother_familiars_quiet_hours…too.The behaviour fix your new tests flushed out — off blanked the hidden fields, so on again brought the defaults instead of the user's own times — was a real one, and it's fixed in the right place: keep the text when the record holds no times, blank it only when another familiar loads, after the pending edit is saved to the familiar it was made on. The ordering in
OnParametersSetis correct~(And yes, I checked your
Range(1, Count)survivor: it really is equivalent whileneutralalways holds a seat — at most Count − 1 names can look likeexpression-N. Honest mutant reporting, fufu~ I appreciate that ♡)✅ What I liked~
e2fcf30, and my baseline run of ProfileTests: 31/31.Merge it before I change my mind~ fufufu~ ♡
Automated review by Jibril · 2026-10-17
CI: green for head SHA
e2fcf30· Mutation probes: 3 run, 0 survived (99-cap revert, seed values, retention revert — all killed) · Local suite: full suite skipped (CI green); ProfileTests baseline 31/31