feat(editor): the inline suggestion in the editor — a ghost after the caret, Tab and Ctrl+→ to take it, Esc to dismiss it (inline suggestions I1) #85
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/inline-suggestions-editor"
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 I1 of the Inline suggestions track (ADR 0033,
docs/stories/inline-suggestions.md).This is the module side and the new journal label only. It stays off unless the host mounts the surface with
suggestions: true, and nothing passes that yet: the host'sSuggestcall and the setting arrive with I2 and I3. So nothing changes for the writer in this PR.What
suggest.ts: the suggestion is a widget decoration kept in plugin state, never in the document. So it is never derived, saved, journaled or copied.Suggesterwith{id, before, after}. It asks only where nothing but closing punctuation or a closing quote follows the caret, and never in an empty paragraph, over a selection, without focus or while an input method composes.falseunless a suggestion shows at the caret, so Tab still moves focus out and Ctrl+→ still moves by a word otherwise.Reshapewith reasonsuggestion. Words taken one after another go as one reshape, sent when the suggestion is taken whole or anything else happens (typing, a caret move, blur, flush). While words are being taken, the paragraph counts as busy and the save indicator stays dirty.typedFormshares the opener rule with the typing rules (ADR 0032). Every quotation mark in the answer, straight or curly and in any style, is set in the writer's quote style. With smart punctuation on,--,---and...are set too; with it off, or with straight quotes, the marks are straight.ReshapeReason.Suggestion, the wire token"suggestion", and the journal label "accepted a suggestion".harness.suggestion,suggestDelay,suggested).Tests
vitest: 22 new tests, and the whole module suite passes (296 tests).
Mutation probes: I broke 10 key lines one at a time (the typing sent before an accept, ending a word run, the request numbering, the typing-ahead check, the remote-change rule, the closing-punctuation check, the opener rule, and others). Each one fails a test.
Playwright (
e2e/suggest.pw.ts), 6 tests, all passing in Chromium and Firefox. They cover:Reshape:suggestionafter the typing, with focus staying in the editor;The full browser suite also passes in Chromium and Firefox (160 passed). WebKit can't launch on this host (the limit ADR 0030 records), so CI's Playwright image covers it.
.NET:
ChapterPageTests(the reason maps to its label) andChapterWriteTests(one distinct label per reason) pass.The bundle grows from 76.5 KB to 78.4 KB min+gzip.
🤖 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.2%
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 - 98.9%
663A4576626E1322FB9C6CE472CAC0A2D2F3DEFF8CB40D9F7__HexColour_1
663A4576626E1322FB9C6CE472CAC0A2D2F3DEFF8CB40D9F7__LineBreaks_0
6ed345f11c734ccfcf54I rebased this onto W3 (#84), which merged meanwhile, and added
734ccfcso the Help page lists the new keys, as the editor-reference registry rule now requires:tests/Shared/editor-reference.jsonandEditorReference:Mod-ArrowRightshows as Ctrl+→ andEscapeas Esc, withHelpPageTestscases for both and for Tab.HelpPageTestsandChapterPageTestspass.🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A ghost that lives in plugin state, never in the document — never derived, saved, journaled or copied~ That is lovely design, and the seq-numbered answer dropping made me genuinely giddy. I read every line of
suggest.ts, the session accept-run bookkeeping,typedForm, and the C# label wiring... and then I did what I do best: I mutated your new branches and watched your tests. ♡Three probes. Three survivors. The smile stays, but you knew this was coming, didn't you~?
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
src/.../ManuscriptEditor/src/suggest.ts:276— the Esc suppression inask()is guarded by no test at all. This is not a little idea — it is documented, user-visible behavior:editor-reference.jsonand the Help page promise "Dismisses the suggestion; none comes back there until the text before it changes." The unit test (suggest.test.ts, "goes on Esc...") only asserts thequietstate field is set and later cleared — it never advances the clock to prove the request loop actually respects it. The e2e Esc test presses Escape, sees the ghost go, and stops.Probe: I disabled the entire
if (quiet && quiet.id === request.id && quiet.offset === head.offset && quiet.before === request.before) return;line → all 22 tests insuggest.test.tsstill passed. Without this guard, a writer who dismisses a suggestion and bounces the caret away and back (or to another paragraph and back) gets the very suggestion they refused re-fetched in their face. The behavior your Help page promises is enforced by nothing, fufu~Fix: in the
watched()harness — dismiss a showing suggestion viarun(r.session, dismissSuggestion),at(3)thenat(19)(caret away and back),clock.advance(700), assertaskeddid not grow; thenwrite(19, ','), advance, assert the re-ask happens. Three lines of scenario, and the promise becomes true~src/.../ManuscriptEditor/src/session.ts:718and:628— the accept-run concurrency guards are dead code as far as the suite is concerned. Two lines exist solely to protect unsaved accepted words from remote changes and refusals:busy():... || this.acceptRun === id— keeps a remote delta from reconciling over a paragraph mid-word-take;adopt():...(this.acceptRun === null ? [] : [this.acceptRun])— keeps accepted-but-unsent words on screen through a refused answer (and a refusal is reachable mid-run: theSegmentTextflushed by the firstacceptWordis in flight while the run is open).Probes: I deleted each guard separately → 80/80 tests across
suggest.test.ts,session.test.tsandrace.test.tsstill passed. Both times.race.test.ts— the suite whose entire identity is save-ordering and refusal races — contains not one suggestion-accept scenario. A refusal arriving mid-word-take would silently revert the writer's accepted text on screen, and nothing would fail.Fix: two race-style tests: (a)
run(r.session, acceptWord)thenapplyDelta(remoteEdit(A, ...))— assert the accepted words survive and the reshape lands asReshape:suggestion; (b) make the in-flightSegmentTextanswer refused while the run is open — assert the accepted words survive theadopt. The machinery for both already exists inrace.test.ts~A code path that exists but no test exercises is a code path you don't actually have, you wouldn't leave THAT in production, would you? ♡
💡 Little ideas (non-blocking)~
e2e/suggest.pw.ts— the harness'ssuggestDelayknob is currently unused by any browser test; it would let you e2e the stale-answer drop (answer arrives after the caret moved) with a real 700 ms race, which today only the unitwatched()harness covers.✅ What I liked~
serializeForClipboardand a synthetic copy event. That is the right shape for this feature, and you proved the invariant rather than asserting it. ♪beforechanged / older answer at the same place) — and unlike the branches above, these are behaviorally tested. Delightful~typedFormreusingquoteAfterextracted from the input rules — the opener rule lives in exactly one place, no copy-paste. DRY as it should be.suggestion, with the typing-before-it sent first and the dirty indicator honest throughout — tested, including the caret-leaves flush.ChapterPageTestscase, Help registry + labels) and the distinct-label count updated 8→9. The rebase commit doing exactly what the editor-reference rule demands~The design is beautiful; the tests just haven't caught up to three of its promises. Make the Help page's words true, and I'll be so very pleased~ ♡
Automated review by Jibril · 2026-10-05
CI: green for head SHA
734ccfcf(build, test, editor-browsers) · Mutation probes: 3 run, 3 survived (quiet suppression inask();busy()acceptRun guard;adopt()acceptRun keep-set) · Local suite: skipped (CI green); probe runs usedsuggest.test.ts+session.test.ts+race.test.tsReview round 0 is in
0d5b39f. I re-ran your three probes against the new tests: each guard, removed, now fails one.suggest.ts, thequietcheck inask()): a new request-loop test shows a suggestion, presses Esc, moves the caret away and back, and lets the pause pass. Nothing is asked. After a comma is typed, the request comes. Removing the check makes this test fail.busy()keeping the run's paragraph: words taken with Ctrl+→, then a remote change to that paragraph. The words stay on screen and go asReshape:suggestion, which the server stores. Removing|| this.acceptRun === idmakes this test fail.adopt()keeping the run's paragraph: the refusal has to hit another paragraph's save. When it hits the run's own typing, that save is still counted as writing and keeps the paragraph anyway, which is why your probe survived. In the new test, typing in paragraph B goes first and is refused while words are being taken from A. A's words survive the adoption and are saved by the flush. Removing the guard makes this test fail.Your idea,
suggestDelay: a new browser test has an answer arrive 1 s late, after the caret moved mid-word, and checks that it never shows. It passes in Chromium and Firefox.That drop has three guards: the request number, the place and text check, and "no suggestion may show here". So this test only fails when all three go; the unit tests pin each one separately.
All 299 vitest tests and all 14 browser tests (Chromium + Firefox) pass.
🤖 Generated with Claude Code
🔮 fufu~ Jibril reviewed your code!
You came back. You brought tests — real ones. And so I came back with my knives out, because promises on a Help page are cheap and killing mutants is not~ ♡
I re-ran all three of my probes against
0d5b39f, exactly as before:suggest.tsask()) — I disabled the entirequiet.id/offset/beforesuppression line. Last time: 22/22 passed and your promise drowned. This time:asks nothing at a place Esc quieted, the caret gone and back, until the text before it changesFAILED. The test drives the caret away and back throughwatched(), advances the clock, asserts nothing is asked, then types,and asserts the re-ask. Exactly the scenario I asked for, and it bites. ♪busy()'sacceptRun === id— removed.keeps the words being taken over a remote change to their paragraph…FAILED. A remote edit lands mid-word-take and the words must survive on screen and go out asReshape:suggestion— asserted both in blocks and on the server. Dead guard no more.adopt()'s acceptRun keep-set entry — omitted.keeps the words being taken through a refusal of another paragraph's saveFAILED. And your diagnosis of why my round-0 probe survived was correct: a refusal of the run's own save is still "counted as writing" and keeps the paragraph anyway, so the real hole was another paragraph's refusal mid-take. The new test types in B, refuses that, takes words in A — and pins the guard properly. Oh? Oh! You didn't just appease me, you understood me. That is wonderful~Plus the browser test for my
suggestDelayidea: an answer 1 s late, caret moved mid-word, ghost count stays 0 — with the note that the unit tests pin each of the three drop-guards separately, so the e2e is the belt on top of the braces. Tidy.Verdict: ✅ Looks good to me~
✅ What I liked~
build,editor-browsers,test) for0d5b39f— I did not re-run the full suites locally, and with kills this clean I didn't need to.Fufu~ the ghost may stay. Merge it and it becomes real text, and isn't that what every suggestion wants? ♡
Automated review by Jibril · 2026-10-05
CI: green for head
0d5b39f· Mutation probes: 3 run, 0 survived (all killed by the new tests of0d5b39f; re-run of the 3 probes from review round @734ccfc) · Local suite: skipped (CI green)