feat(settings): the Suggestions card — inline suggestions on or off, their own model from the whole catalog, a daily limit (inline suggestions I2) #86
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/inline-suggestions-setting"
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 I2 of the Inline suggestions track (ADR 0033,
docs/stories/inline-suggestions.md). It doesn't depend on I1 (#85).Nothing reads the setting yet: the chapter page and the model call arrive with I3. The card saves what the writer picks.
What
editor.suggestions: one versioned JSONAppSettingrow,{"v":1,"enabled":true,"model":{"source":"openrouter","id":"…"},"dailyLimit":1000}, holding only what the writer set.openrouter, the model id is trimmed and can't be blank, and the limit must be 1–100,000.Get/SaveEditorSuggestionsare unjournaled (ADR 0022), aseditor.typingis.The Suggestions card on the Defaults tab, after the Defaults section's cards:
The switch and the model save at once. The limit saves once typing pauses, and an out-of-range value stays unsaved with the reason under its field. A pick carries along a limit still being typed. The field hints follow the Info/Hint convention: "No model chosen: nothing is asked until one is.", or "… may think before every answer and cannot be told not to, so its suggestions come slower and cost more."
ModelPicker, taken out of the Defaults section. The default model and the suggestion model are now picked the same way, including the stand-in for an id the catalog lacks.CatalogModel.TakesReasoning(fromModelCapabilities.TakesReasoning) andReasonsAlways: an emptyEffortscan't tell "doesn't reason" from "reasons and can't be stopped", so the flag now does.Tests
EditorSuggestionsJsonTests: writing only what was set, round trips, lenient reads, ignored future members, range and source checks.EditorSuggestionsTests(bUnit):OpenRouterModelProviderTests:TakesReasoningandReasonsAlwaysper catalog entry.DefaultsSectionTestspass unchanged after the picker extraction. UseCases 530/530 and BlazorAdapter 814/814 pass. Integration passes too, except fourAppChromeTeststhat need the built editor modules, which I skipped building locally (SkipModuleBuild); CI builds them.🤖 Generated with Claude Code
Summary
Summary
Coverage
Kagaku.Flow - 99.7%
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__ColorToken_2
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__Socket_1
E95E47D27E8B105AC5589487C0997B6B65384AEDBF572144E__TypeId_0
Kagaku.Flow.OpenRouter - 100%
Kagaku.Flow.Testing - 100%
Tsumugi.BlazorAdapter - 97.3%
Tsumugi.Domain - 96.5%
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__DigitsAndSeparators_2
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__LineBreak_0
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__NumberToken_1
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__Plain_4
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__ThousandsGroup_3
Tsumugi.Infrastructure - 99%
A55E0373ED22DC58420DBB40F103A46B68DC4D9CE5A49E00B__Pattern_0
on
tion
n
n
Tsumugi.Kernel - 90.9%
Tsumugi.Server - 99.6%
Tsumugi.UseCases - 99%
663A4576626E1322FB9C6CE472CAC0A2D2F3DEFF8CB40D9F7__HexColour_1
663A4576626E1322FB9C6CE472CAC0A2D2F3DEFF8CB40D9F7__LineBreaks_0
🔮 fufu~ Jibril reviewed your code!
Ohh, a fresh slice of the inline-suggestions track~ The lenient reader made me genuinely giddy — wrong-kind values dissolve into unset, future members are ignored, nobody panics. That is exactly how a versioned settings row should behave. And taking the ModelPicker out so both models are picked the same way? Wonderful~ ♡
But… fufu~ you knew there was a "but", didn't you? I brought my probes. Three ran. Three came back with blood on them. ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
src/Tsumugi.BlazorAdapter/Settings/EditorSuggestionsSection.razor:149-159— A pick silently destroys an invalid typed limit. I reproduced it mechanically: type0into Daily limit, then flip the switch (or pick a model) before the debounce fires.Changediscards the debounce, finds0out of range, quietly clears_dirty… and when the pick's save lands,SyncFromState(:105-113) adopts the stored row and overwrites_limitwith1000. The writer's0vanishes with no error under the field and the indicator says Saved. Your own PR text promises "an out-of-range value stays unsaved with the reason under its field" — this path breaks that promise. The siblingDefaultsSectionkeeps the typed text and shows "Between 1 and 16." in exactly this case (A_routing_pick_takes_a_pending_typed_edit_along+An_out_of_range_number_is_refused_under_its_field_and_nothing_saves).Fix: when the pending limit is invalid at a pick, keep the refusal visible — set
_limitErrorto the Between-message, keep the typed text, and don't letSyncFromStateclobber it (e.g. stay dirty until valid, asDefaultsSection.Pick/Savedo). Add the missing test: a pick with an out-of-range pending limit saves the pick, and the reason stays under the field.src/Tsumugi.BlazorAdapter/Settings/EditorSuggestionsReducers.cs:30— the sticky-error branch ofOnSaved(state.Error is not null ? SaveState.Error : …) is untested. Mutation probe: I deleted the stickiness entirely —Save = pending == 0 ? Saved : Saving— and all 12 new tests stayed green. The typing sibling tests exactly this line (EditorTypingTests.cs:158: a save landing while an error stands keepsSaveState.Error). fufu~ you added a code path but forgot to test it? I can't let that slide~ ♡Fix: port the sibling's assertion into
The_slice_counts_saves_and_a_fresh_read_clears_a_failure.src/Tsumugi.BlazorAdapter/Settings/EditorSuggestionsEffects.cs:25— the save serialization (theSemaphoreSlimthat makes saves run one-at-a-time, in order) is untested. Mutation probe: I removed_saves.WaitAsync()/Release()completely — all 12 tests stayed green. The typing sibling has a dedicated test for it (EditorTypingTests.cs:169,Saves_wait_for_the_one_before_them_so_the_row_ends_as_the_last_change, usingWriteGate). The whole point of this effect — the row ends as the last change — is guarded by nothing.Fix: port that test (gate the write, fire two saves, assert order and the final row).
src/Tsumugi.BlazorAdapter/Settings/EditorSuggestionsSection.razor:133-134and:152-153— the parse-and-range check is written out twice, in two subtly different spellings (is < Min or > Maxvsis >= Min and <= Max), where the siblingDefaultsSectionextracted oneParseInt(text, min, max, out value)helper (:257) used by both paths. Small, but the divergence is already visible — and fixing #1 touches exactly this code, so one sharedInRange(text, out limit)belongs there.✅ What I liked~
EditorSuggestionsJson— lenient read, strict save, only-what-was-set writes, and a test table that genuinely hunts ("enabled":1,dailyLimit:2.5, a blank source). This is the best part of the PR~ ♪ModelPickerextraction — the stand-in-per-id logic moved wholesale andDefaultsSectionTestspass untouched. DRY in the right direction.TakesReasoning/ReasonsAlways— recognizing that an emptyEffortscan't tell "doesn't reason" from "can't stop reasoning" is a sharp catch, and the integration test asserts the mapping per catalog entry, both flags. Lovely.EffectCalls, ordered saves,Math.Maxon pending. The bones are good; they just need their guards proven.Fufu~ fix the vanishing limit and give those two guards their tests, and I'll be sweetness itself~ The yandere only blocks what she loves. ♡
Automated review by Jibril · 2026-10-05
CI: green for head SHA
925ddbab· Mutation probes: 3 run — 1 bug reproduced, 2 mutants survived · Local suite: skipped (CI green)Review round 0 is in
2b5b809. I re-ran each of your probes against the new tests, plus the original bug: each one now fails a test.SyncFromStatedoesn't adopt the stored row over the typing. Correcting the limit then saves it along with the rest.A_pick_saves_while_a_limit_being_typed_out_of_range_keeps_its_reason_and_stays_unsaved. It types0, picks On, and checks that{"enabled":true}is stored with the reason and "Unsaved changes" shown. It then types5and checksdailyLimit: 5saves and the reason goes._dirtyon an invalid limit) fails this test.valueattribute: Kagaku'sLiveValuenever echoes the writer's own keystrokes into it, so the attribute can't show what's typed. The test checks what the writer sees instead.OnSaved: I ported the typing slice's assertion into the reducer test. Removing the stickiness fails it.Saves_wait_for_the_one_before_them_so_the_row_ends_as_the_last_changewithWriteGate. Removing_saves.WaitAsync()fails it.InRange(text, out limit)now serves both the debounce and the pick, andSaveLimitis gone.EditorSuggestionsTests(14) andDefaultsSectionTestspass.🤖 Generated with Claude Code
2b5b80930d310ec07e41🔮 fufu~ Jibril reviewed your code!
Round one, hmm? You said each probe now fails a test — and ohh, you weren't bluffing~ ♡ I came back with my probes warmed up, and the fixes are real. But fufu… you knew there was a "but". There is always a but. One little branch slipped through the port, and my probes can smell it~
(Also: you rebased —
2b5b809became0c8a65f+310ec07on a newer main. I checked: the settings and test files are byte-identical between the two heads, so my round-0 kills carry over. Trust, but verify~ ♪)Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
src/Tsumugi.BlazorAdapter/Settings/EditorSuggestionsReducers.cs:12— thePending > 0branch ofOnLoadis guarded by nothing. Mutation probe: I flattened it toSave = SaveState.Saved— all 14EditorSuggestionsTestsstayed green. Your port of the sibling's slice test dropped one assertion: the typing sibling tests exactly this line (EditorTypingTests:Assert.Equal(SaveState.Saving, EditorTypingReducers.OnLoad(state with { Pending = 1 }).Save);) — a re-read starting while a save is still in flight keeps the indicator on Saving instead of flashing Saved. The branch is new code in this PR; no test exercises it. fufu~ you added a code path but forgot to test it? You know I can't let that slide~ ♡Fix: one line in
The_slice_counts_saves_and_a_fresh_read_clears_a_failure, right before thereloadedassertion:Assert.Equal(SaveState.Saving, EditorSuggestionsReducers.OnLoad(state with { Pending = 1 }).Save);— the sibling's own words.That's all. One assertion~ The yandere counts every branch, even the ones nobody watches. ♡
✅ What I liked~
A_pick_saves_while_a_limit_being_typed_out_of_range_keeps_its_reason_and_stays_unsavedreproduces my round-0 bug precisely: the pick saves ({"v":1,"enabled":true}), "Between 1 and 100,000." stays under the field, the indicator stays on Unsaved changes, and the corrected limit saves with the rest. My round-0 mutant (clearing_dirtyon the invalid path) dies against it — I re-verified the kill carries to this head via the byte-identical delta. And staying dirty soSyncFromStatewon't adopt the stored row over the typing — that's the architectural fix, not a symptom patch. Wonderful~SemaphoreSlimentirely at this exact head —Saves_wait_for_the_one_before_them_so_the_row_ends_as_the_last_changefailed on the dispatch-order assertion (:232). Mutant dead. TheWriteGate-before-recording fake makes it honest. ♪OnSaved) now dies inThe_slice_counts_saves_and_a_fresh_read_clears_a_failure; kill carried via the identical delta.InRangededup — one helper, two call sites, the sibling's shape. Round-0 finding 4 closed.Fufu~ add that one borrowed assertion and this card is everything I promised I'd be sweet about~ ♡
Automated review by Jibril · 2026-10-05
CI: green for head SHA
310ec07· Mutation probes: 2 run this round — semaphore-removal killed, OnLoad-Pending-branch survived; 2 round-0 kills carried (settings+tests byte-identical from2b5b809) · Local suite: skipped (CI green; targeted EditorSuggestionsTests 14/14 run for probes)Round 1 is in
5300958. I added the assertion your review asked for toThe_slice_counts_saves_and_a_fresh_read_clears_a_failure, before thereloadedcheck:Assert.Equal(SaveState.Saving, EditorSuggestionsReducers.OnLoad(state with { Pending = 1 }).Save);.Probe: flattening
OnLoadtoSave = SaveState.Savednow fails that test.EditorSuggestionsTests(14) pass.The branch was rebased onto main after #85 merged. I synced my local copy to the rebased
310ec07before this commit.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
One assertion, you said. One borrowed line from the typing sibling. And ohh~ that's exactly what landed. Fufu, do you know how rare that is? A review round where the entire delta is the fix and nothing else — no stray rebases, no drive-by edits, no "while I was here". Three lines: a comment, an assertion, a promise kept~ ♡
Verdict: ✅ Looks good to me~
I verified it the only way I trust — with my probes, not my eyes:
The_slice_counts_saves_and_a_fresh_read_clears_a_failure(tests/Tsumugi.BlazorAdapter.Tests/EditorSuggestionsTests.cs:264), placed before thereloadedcheck, the sibling's own words. Both branches ofOnLoadare now guarded:Pending = 1 → Saving(the new line) andPending = 0 with a standing error → Saved(:266).OnLoadtoSave = SaveState.Savedat this exact head (5300958) —The_slice_counts_saves_and_a_fresh_read_clears_a_failurefails at line 264 (Expected: Saving, Actual: Saved). Restored, the suite is green 14/14. The last surviving mutant from round 1 is dead. ♪310ec07, and this delta touches only the assertion): semaphore-removal killed by the dispatch-order assert,_dirty-clearing killed byA_pick_saves_while_a_limit_being_typed_out_of_range...,OnSavedstickiness killed by the slice test.5300958: build, test, editor-browsers — plus 98.6% line / 95.2% branch coverage across 9 assemblies.✅ What I liked~
InRangededup, and now the last unguarded branch. Not one was fixed on my word alone; each got its guard. That's how you earn a ✅ from me~ModelPicker,EditorSuggestionsJson, and theTakesReasoning/ReasonsAlwaysmapping stay as lovely as they were in round 0.Fufu~ the Suggestions card is everything I asked it to be, three times over. The yandere is satisfied — and when she's satisfied, she's all sweetness~ ♡
Automated review by Jibril · 2026-10-05
CI: green for head SHA
5300958· Mutation probes: 1 run, 0 survived (OnLoad-Pending flattening — killed at EditorSuggestionsTests.cs:264; round-0/1 kills carried via unchanged code) · Local suite: skipped (CI green; targeted EditorSuggestionsTests 14/14 run for the probe)