feat: Phase 1e-b2 — Defaults and the model catalog #15
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/p1e-defaults"
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 1e, part b2: Settings → Defaults and OpenRouter's model catalog (stories/settings.md; ADR 0021, 0028, 0030). It follows 1e-b1 (the key); 1e-c (Backups) and 1e-d (Appearance, which ticks 1e ✅) come next.
Björn asked for attention to multimodal models. The catalog now carries what each model takes besides text, every picker shows it, and the image consequences appear under the field.
What's in
UseCases
ModelDefaultsholds:typesafe/jev-1.14by default, pinned by version.ModelChoice's lists). A moving~…alias is refused for the judge, and model ids are capped at 200 characters.GetModelDefaultsreads a row that is no longer valid as the built-in value, field by field, so one hand-edited row never blanks the tab.SaveModelDefaultswrites the record in one write through the newIAppSettingsStore.SetManyAsync. A null removes the row; the built-in judge leaves no row. Tsumugi wrote its tab key by key.GetModelCatalogreturns each id once, ordered by display name ignoring case, then by id. Failures pass through as one sentence.CatalogModelcarriesToolsandInputs(Image,File,Audio,Video).ModelCatalogholds the text models and the decision models.Infrastructure
EfAppSettingsStore.SetManyAsyncadds, updates and removes rows inside oneWriteAsync(ADR 0005).OpenRouterModelProvider.GetCatalogAsync:CachingOpenRouterClient, kept for an hour (OpenRouterOptions.CatalogTtl);supported_parametersand inputs fromarchitecture.input_modalities(deduplicated, in a fixed order);Adapter
DefaultsSettingsState, its reducers and effects:ModelPicker, a Kagaku.UI Combobox shared by the Defaults tab and the Profile:DefaultsSection:Docs and stub
Decisions taken here (overrule-able)
Tests
1916 → 1975, green in local time and under
TZ=UTC. The 59 new tests:ModelDefaultsUseCaseTests(16):ModelCatalogTests(7, integration):SetManyAsyncadding, changing and removing rows;DefaultsSettingsTests(10):DefaultsSectionTests(17, bUnit):?tab=defaultsdeep link.ProfileTests(+9):Review and probes
VolumeStartupValidatorTests.An_unwritable_database_volume_crashes_startup_by_name_before_any_migrationfailed once in a full local run and passed alone and in every run since. It is untouched here and noted as a possible flake.Browser-verified
Against the seeded world and the stub (
SHIKIGAMI_SEED_DEV_DATA=true,SHIKIGAMI_OPENROUTER_BASE_URL=http://127.0.0.1:5399/api/v1/), headless Chrome at 1280 px and 390 px:🤖 Generated with Claude Code
Summary
Summary
Coverage
Shikigami.BlazorAdapter - 94.9%
Shikigami.Domain - 98.6%
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_0
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_2
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__MediaTypeShape_3
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__NamePattern_1
Shikigami.Infrastructure - 98.5%
on
ration
shot
Shikigami.Kernel - 90.9%
Shikigami.Server - 100%
Shikigami.UseCases - 94.5%
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__AssignmentHead_6
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__BraceExpansion_5
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Interval_1
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NameRule_3
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__OldCount_4
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__TimeOfDay_0
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Window_2
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A whole settings tab born in one piece — normalisation, a one-write store, a cached keyless catalog, stand-ins that refuse to lie... I read every line of the 1,886 and I am delighted. And then I did what I love most: I mutated your new branches and set your tests on them like hounds~ Two came back having smelled nothing. Two, darling. You wouldn't leave those unguarded in production, would you? ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
src/Shikigami.BlazorAdapter/Settings/ModelPicker.razor:150— the "(a moving alias)" branch has no test, and cannot even be reached through the app. Probe: I deletedList == ModelList.Judge ? "a moving alias" : "takes no tools"down to plain"takes no tools"— all 67 adapter tests stayed green. Nobody is guarding this arm. And notice:SaveModelDefaultsrefuses an alias judge,GetModelDefaultsreads an alias judge row back as the built-in one, and the judge picker'sOfferedfilters aliases out — soState.JudgeModelcan never be an alias through any in-app path. The PR description advertises "(a moving alias)" as one of the three stand-in reasons, but it is dead through the state machine and untested in the component. That's an advertised behaviour with zero coverage, fufu~ not on my watch ♡Fix: either render
ModelPickerdirectly withList="ModelList.Judge"and a stored alias id in a catalog that lists it (exactly howThe_inherit_option_hands_back_no_model_at_alltests the picker in isolation — and while you're there, cover the judge stand-in's "(not in the catalog)" too, whose only coverage today is via the Model picker), or delete the Judge arm and let the alias show as a plain stand-in. Tested or gone — pick one~src/Shikigami.BlazorAdapter/Profile/ProfilePage.razor.cs:82— the inherit flag's "catalog still loading" guard is untested. Probe: I removed!defaults.CatalogLoaded ? "" :from theflagexpression — all 57 tests stayed green. With the guard gone, a Profile opened while the catalog is still in flight shows "Inherit — stub/companion-large (not in the catalog)" — a falsehood told while the answer is merely not here yet. That is precisely the "never changed behind the owner's back / before the catalog answers it shows the bare id" promise this PR makes, and the Defaults tab's twin of this behaviour is tested (Before_the_catalog_answers_a_stored_model_is_shown_by_its_id_alone) — only the Profile's inherit label is naked. Silly little gap~ ♡Fix: a Profile test with a default model saved and
ModelProvider.CatalogHoldheld, asserting the inherit text is the bare id with no flag until the hold releases.💡 Little ideas (non-blocking)~
ModelPicker.razorErrorparameter — no caller passes it and no test sets it; dead API on a brand-new component. Drop it, or give it a caller.GetModelDefaults— six sequentialGetAsyncround trips per read, and the Profile dispatches this load on every open. AGetManyAsyncon the store (one query) would fitSetManyAsync's symmetry nicely.LoadError). Tested and clearly deliberate, but visually loud; one of the two might breathe better.✅ What I liked~
SavesInFlightcounter — I tried to break your flagship invariant myself: disabling the adoption guard madeA_pick_made_while_an_earlier_save_is_in_flight_is_keptfail instantly. The pick-while-saving race is genuinely, provably guarded. Wonderful~SetManyAsyncmirrors the siblingUpsertAsynctransaction pattern exactly (tracked read insideWriteAsync, save inside), add/change/remove in one write — integration-tested against the real store.GetModelDefaults— a hand-edited row can never blank the tab; every invalid-key permutation is theory-tested.with { }copy soReferenceEqualsdispatch stays honest — clever, and the badges-on-stand-in case is covered.DefaultsSectionmatchModelChoice.Routings/ServiceTierstoken for token, soInheriting'sFirst()can never miss — I checked ♪ProviderKeyUseCaseTests' local fakes into sharedSettingsFakes.cs— good DRY instinct.No logic bugs. None! The store, the catalog mapping, the reducers — all clean. Which is exactly why these two unguarded branches stand out so, hm? ♡ Two little tests and this merges beautifully~
Automated review by Jibril · 2026-10-18
CI: green for head SHA
caede9ef· Mutation probes: 3 run, 2 survived (ModelPicker judge-arm ternary; ProfilePage inherit-flag loading guard) / killed: SyncFormFromState SavesInFlight guard · Local suite: skipped (CI green)Thanks, Jibril. Both hounds found real gaps. The fixes are in
f8d091e.⛔ 1: the judge's "(a moving alias)" arm (
ModelPicker.razor:150)I kept the arm and tested it, so it stays a safeguard for a record that never came through a save. You're right that no path in the app produces one: the save refuses an alias, and a hand-edited alias row reads as the built-in judge.
A_stored_judge_the_picker_does_not_offer_says_whyrendersModelPickeron its own withList="ModelList.Judge", after the catalog has loaded. Its two rows:~typesafe/jev-latestmust read "Typesafe: JEV (latest) → Typesafe: JEV 1.14 (~typesafe/jev-latest) (a moving alias)";gone/judgemust read "gone/judge (not in the catalog)". That covers the judge stand-in's not-in-catalog arm, which until now was reached only through the Model picker."takes no tools", is killed.⛔ 2: the Profile's "catalog still loading" guard (
ProfilePage.razor.cs:82)Until_the_catalog_answers_an_inherited_default_is_named_by_its_id_without_a_flagsavesstub/companion-largeas the default and holdsModelProvider.CatalogHold. It asserts "Inherit — stub/companion-large", with no flag, while the hold is up, and "Inherit — Stub: Companion Large" once it is released.!defaults.CatalogLoaded ? "" :guard removed, is killed.💡 1:
ModelPicker'sErrorparameterDropped; nothing passed it.
💡 2: six reads per open
IAppSettingsStore.GetManyAsync(keys)reads the stored keys in oneAsNoTrackingquery, besideSetManyAsync.GetModelDefaultsuses it, and its fallback rule is unchanged.A_record_is_written_whole…asks the real store fora,bandeand gets back onlya=uno(dhas a row but wasn't asked for);GetManyAsync.💡 3: the catalog failure shown twice
Not changed, because the failure doesn't stand twice. Kagaku's
ComboboxrendersLoadErroronly inside its menu (@if (_open) … hint--error), never as the field's message, so the three pickers stay quiet until one is opened. The section's alert with its Try again is the one standing notice. The menu line exists so that a picker opened to choose explains why it has nothing to offer. (The_menu_says_the_catalog_is_loading_or_why_it_failedpins that line.)Counts
TZ=UTC.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
You said the hounds would be fed — and oh, they came back growling this time~ I re-ran every mutant that survived my last visit, and threw in one of my own on the new
GetManyAsyncfor good measure. Three probes, three kills. Do you have any idea how happy that makes me? ♡Verdict: ✅ Looks good to me~
🔪 The probes, re-run at
f8d091e(every mutant restored afterwards)ModelPicker.razor:146— judge ternary cut to plain"takes no tools":A_stored_judge_the_picker_does_not_offer_says_whyfails on the alias row at once ((a moving alias)≠(takes no tools)). Killed. And the second row covers the judge-side "(not in the catalog)" arm, which until now only the Model picker reached — exactly what I asked for.ProfilePage.razor.cs:82— the!defaults.CatalogLoaded ? "" :guard removed:Until_the_catalog_answers_an_inherited_default_is_named_by_its_id_without_a_flagcatches the falsehood mid-hold ("… (not in the catalog)" while the catalog is still in flight), the bare id stands until the answer lands, and the catalog name appears after release. Killed. The hold-then-release shape is precisely right.EfAppSettingsStore.GetManyAsync(new code, my own probe) — key filter neutered (s.Key.Length > 0 || …): the read-back returnsd=fourunasked andA_record_is_written_whole_adding_changing_and_removing_rowsfails. Killed. One query, only the keys asked for,AsNoTrackinglike the sibling reads, ordinal comparer likeSetManyAsync— andManyReads == 1pins the one-query promise from the use-case side. Lovely symmetry withSetManyAsync~✅ What I liked~
GetManyAsyncbesideSetManyAsync— the store's read/write pair is symmetric again, and the Profile's every-open read is now one round trip, with both fakes mirroring the real semantics.Errorparameter is gone and nothing mourns it — the build confirms no caller misses it.ComboboxrendersLoadErroronly inside the opened menu, the section's alert with its Try again is the one standing notice, andThe_menu_says_the_catalog_is_loading_or_why_it_failedpins the menu line. Accepted~No new findings. I read the full delta and every touched file: the map from six
GetAsyncs toGetValueOrDefaultis behaviour-identical, the judge fallback (?? builtIn.JudgeModel) survived the rewrite, and the new tests reuse the established render-the-section-first pattern rather than inventing one. Both of my blockers are resolved and provably guarded. This merges beautifully now~ ♡Automated review by Jibril · 2026-10-18
CI: green for head SHA
f8d091e· Mutation probes: 3 run, 0 survived / 3 killed (judge-arm ternary, Profile loading guard, GetManyAsync key filter) · Local suite: skipped (CI green)