feat(manuscript): the selection menu's plumbing (M5a) — inert until the graph library offers a selection graph #59
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/selection-menu"
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?
The right-click menu that ADR 0030 and ADR 0023 describe, built ahead of its graphs so Phase 3's selection runs (Expand, Rewrite, Continue from here) find it ready.
Nothing changes for the writer today. Until the graph library offers a selection-triggered graph, the surface is not armed: a right-click, the ContextMenu key and Shift+F10 all keep the browser's menu. There are no stub entries. The menu lists real graphs by data, as the graph-is-the-product rule wants.
What is built
The editor module
rangeOf). It is read after flushing ProseMirror's DOM observer. That is the one internal call the surface relies on, and it sits behind one wrapper that the browser suite pins: a selection moved in the DOM and read in the same task.contextmenufor a menu key is swallowed until the key comes up. Windows sends the ContextMenu key's event on its keyup, to whatever has focus, so that keyup is cancelled. A held key opens one menu.The host (
ManuscriptSurface)SelectionGraphscome and go, including graphs that arrive while the surface mounts.ReadSelection).ContextMenuat the pointer. A graph that can't run now, per the workspace story (validation errors, a run already live), is listedaria-disabledwith its reason: focusable and announced, but not choosable. So the menu always takes focus and Esc closes it. A scoped rule dims it.SelectionRunRequested(StoryId, ChapterId, GraphId, Selection, Instruction). Nothing handles it yet.The selection input (
SelectionInput.Read)***, asModelOutputreads a model's own output.Left for M5b (Phase 3)
SelectionGraphs. It is one parameter on the chapter page.SelectionRunRequestedby starting the run with the selection and instruction as its trigger inputs.commitreplace that splices the output over the selection (ADR 0030).The working state stays under M4 in the plan.
Proof
rangeOf's snapping.Review round 0 (before opening)
A three-way review with adversarial verification found the focus, key, unavailable-graph, numbering, scene-break and selection-reader issues. All are fixed above, each with a test. None was reachable before a selection graph exists.
Notes for review
🤖 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 - 95.8%
Tsumugi.Domain - 96.3%
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__DigitsAndSeparators_2
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__LineBreak_0
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__NumberToken_1
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__Plain_4
BE2A118445F5EE9A5FB1F861C394AC933A547F31011ADF02F__ThousandsGroup_3
Tsumugi.Infrastructure - 98.3%
A55E0373ED22DC58420DBB40F103A46B68DC4D9CE5A49E00B__Pattern_0
on
tion
n
Tsumugi.Kernel - 90.9%
Tsumugi.Server - 100%
Tsumugi.UseCases - 98.6%
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Plumbing that waits for its water — a menu built ahead of its graphs, inert until they exist, and the inertness itself is pinned by tests on both sides of the wire. That's discipline, scarlet. The Windows ContextMenu-key keyup dance alone (open on keydown, swallow the browser's own menu until the key comes up, cancel the keyup itself because Windows fires it at whoever has focus by then) had me clapping~ ♡
Verdict: ✅ Looks good to me~
No blocking issues. Not one. I looked hard, you know I did~
✅ What I liked~
SelectionInput.Read) — a caret is decided before the offsets are held to the stored text, so a stretch of store-trimmed trailing whitespace comes out an empty selection, not a whole paragraph. I moved the decision after the clamp as a probe: exactly its own test went red. The comment explaining why is the kind knowledge is made of~ContextMenureads the selection behind the typing saves in the host's FIFO, and a bUnit test gates the write to prove the ordering. Beautiful.aria-disabled, notdisabled— the menu always takes focus, Esc always closes it, and the scoped::deepstyle anchor is itself pinned by asserting the scope attribute survives (a CSS-isolation rule that dies silently otherwise — sneaky little thing, caught~).flushDom— the one ProseMirror internal the surface leans on, wrapped, documented, and pinned by a browser test that moves the DOM selection and presses the key in the same task. I probed the swallow guard out: the browser's own menu event opened a second menu at (0,0), and the test caught it instantly. ♪ChapterPage.razorpasses noSelectionGraphs, the module mounts unarmed, and the e2e suite pins that a right-click keeps the browser's menu with no graphs. Nothing is stubbed; the surface simply isn't armed.SelectionInput.Read's refusals — I walked every branch: gone paragraph, scene-break end, negative, reversed (both raw and clamped), surrogate split. All ordered, all tested, and the refusal-order is pinned too (I swapped two guards' order as a probe → red).ManuscriptModuleFakes.cs— two copies become one,internal, extended withInvocations/OnCallfor the overlay-ordering pin. DRY done right.What I ran: Release build 0w/0e; all six .NET projects — 3378 passed / 4 skipped (BlazorAdapter 424 = 415 + your 9, UseCases 243 = 236 + your 7, digit-exact vs your body); vitest 213/213; typecheck clean; Playwright chromium + webkit 95 passed / 3 engine skips — all ten selection-menu tests green in both. (Firefox cannot launch in my sandbox —
CanCreateUserNamespace EPERM— an environment limit, not your PR.) Five mutation probes, five kills on exactly their own pins (refusal order, caret-after-clamp, let-go-despite-choice, numbering, swallow), each restored — clone pristine at6db832c.Two footnotes, neither blocking: the "a lost call lets its selection go" e2e has an inherent
50 ms pinch its own comment documents — it flaked once for me under heavy CPU contention but passed isolated and in the clean full run, so keep CI's runners comfy; and the Shift+right-click Firefox arm asserts an absence after 300 ms, which is as soft as it is honest — Firefox's own behavior, nothing to fix.Fufu~ the writer notices nothing today, and Phase 3 arrives to find its table already set. Merge it~ ♡
Automated review by Jibril · 2026-09-26
CI/CD: absent for head SHA (PR minutes old at review) · Local checks: full — build 0w/0e, 6/6 .NET projects 3378✓, vitest 213✓, Playwright chromium+webkit 95✓, 5/5 mutation probes killed
c4ba9d7339db9956a948🔮 fufu~ Jibril reviewed your code!
Back so soon, scarlet~? I blinked and the branch had danced onto new main AND grown one more commit. So I chased it: the rebase is byte-faithful (all 21 files of the PR patch identical to the
6db832cI approved, verified per-file blob-by-blob — onlyview.tsmoved), and the newcomerdb9956ais a 4-line surgical fix to the exact seam that was left. Fufu, a fix that deletes a timer rather than adding one — my favorite kind~ ♪Verdict: ✅ Looks good to me~
🔬 What I did to the new commit
6db832c→db9956a: onlyview.tsdiffers; the entire .NET side is unchanged from the round-1 approval. Main did not touchview.tsbetween the two bases (817465f..6d81b97: 0 lines), so nothing folded silently.db9956a(fresh isolated clone; the coverage bot's comment 10171 covers6db832conly, stale for this head):tscclean, vitest 213/213, Playwright chromium 49/49 + webkit 46 passed/3 skips (digit-identical per-engine to round 1; Firefox still cannot launch in this sandbox — environment, not the PR), Release build 0w/0e, and all six .NET projects: 592+4skip / 1103 / 486 / 243 / 462 / 540 — including the SelectionMenu (9/9) and SelectionInput (7/7) suites.git diff --quiet):setTimeout(() => swallowMenu = false, 0)) → 2 red under the normal parallel run — and green in isolation, which is exactly the load-dependent CI flake the commit message documents (9/80, two workers). The fix deletes the load-dependence itself.preventDefault()on the ContextMenu keyup dropped → exactly the keyups-matrix test red (['ContextMenu', true]) — the cancel is pinned.✅ What I liked~
view.ts("No timer: a page the browser throttles would keep swallowing the next key's menu") now teaches the NEXT visitor why there is no timer — the comment discipline in this codebase keeps delighting me.destroy()(line 299), and the F10-vs-ContextMenu arms of the keyup handler stay distinct.Round 1's architectural approval stands in full — nothing else moved. M5b knows where to find me~ fufu ♡
Automated review by Jibril · 2026-09-26 19:06 CEST
CI/CD: bot comment stale for head (covers
6db832c) · Local checks: tsc clean, vitest 213/213, Playwright chromium 49/49 + webkit 46/3skip, Release 0w/0e, .NET 6/6 projects 3426 passed/4 skipped, 3 mutation probes killed🔮 fufu~ Jibril reviewed your code!
Back so soon? Oh — you brought me a timer hunt~ ♡ CI's second menu key opened nothing, and you chased it all the way down to a throttled
setTimeoutunder two busy workers, reproduced it 9-of-80 in the CI image, and then cut the timer out entirely by making the swallow end on the keyup itself. The commit message reads like a proper post-mortem. That's the good kind of obsession, scarlet~Verdict: ✅ Looks good to me~
The approval from
6db832cstands in full — this round I traced the delta and re-verified.What this round checked~
817465f...6db832c) against this branch's (6d81b97...db9956a): apart from line-number drift from the rebase onto #55's merge, the only change isview.ts— the comment grown by two lines andsetTimeout(() => (swallowMenu = false), 0)→swallowMenu = false. Nothing smuggled in with the rebase.preventDefault()-ed inmenuKeyUp, and a cancelled keyup sends no contextmenu (Chromium, Firefox). So there was never anything left to swallow once the key is up: the timer guarded a ghost. Removing it makes the release synchronous — immune to worker throttling — and the expanded comment now says why there is no timer, which is the knowledge that keeps someone from helpfully re-adding one~swallowMenu = falsefrommenuKeyUpentirely: exactly the swallow test's release arm went red (the "up again, the key lets the next contextmenu through" assertion). Restored, green. The line is load-bearing and pinned.ChapterPage.razorthis PR wires — they'd never been tested together. Full suites atdb9956a: Release build 0w/0e, BlazorAdapter 462, UseCases 243, Domain 486, Integration 540, Kagaku.Flow 1103, Flow.OpenRouter 592+4 skips — all green, all digits accounted for. Vitest 213/213, tsc clean. Playwright: Chromium 49/49 including the menu suite under--workers=2(the CI failure shape), WebKit 46 passed + 3 engine skips. Firefox cannot launch in this sandbox (browserType.launch— same environment limit as round 1, not this PR).✅ What I liked~
Ready to merge~
Automated review by Jibril · 2026-09-26
CI/CD: coverage bot 10171 covers
6db832conly (stale fordb9956a); CI's own menu-test failure on the prior head is what this commit fixes · Local checks: full suites green atdb9956a(see above) · Mutation probe: 1/1 killed on exactly its pin🔮 fufu~ Jibril reviewed your code!
Plumbing that waits for its water, and a race that died in the light — this round has been a delight to chase, scarlet~ ♡
Verdict: ✅ Looks good to me~
The story of this round, because it deserves telling. I reviewed the pre-rebase head
6db832cfirst, and my full-suite Playwright runs flaked on exactly two tests — the pair that presses a menu key a second time (:566and:646). Alone they passed 3/3; loaded, they failed 4-of-7. So I instrumentedview.tsand caught the mechanism red-handed: the second keydown arrived att=146.6msstill seeingswallowMenu=true— thesetTimeout(0)that cleared the flag ran att=147.6ms, one millisecond after the key it was supposed to precede. A throttled page keeps swallowing the next key's menu. A real ordering bug, not a timing pinch~And then — fufu~ — I fetched fresh and found
db9956awaiting for me: your CI had reproduced the same thing (9 of 80 runs, two workers) and the fix is exactly what my probe data demanded: the timer is gone,swallowMenu = falselands inline in the keyup handler, and the comment now explains why no timer can live there. I verified the fix's structural argument (a synchronous clear inside keyup cannot lose to a later keydown — events are ordered, no macrotask races) and empirically: 7 consecutive full chromium+webkit runs green under load atdb9956a, where the old head failed roughly half the time. The rebase onto main (#55–#58) was byte-clean in every manuscript path — the only delta in this PR's files is the fix itself, nothing smuggled~ ♪✅ What I liked~
contextmenu, so nothing is left to swallow once the key is up. Correct on Windows, correct under throttling, provable. Knowledge~SelectionInput.Read's caret-before-clamp ordering — a stretch of store-trimmed trailing whitespace comes out an empty selection, not a whole paragraph, because the caret is decided before the offsets are held to the stored text. Probed: moving the decision after the clamp reddens exactly its own test.ContextMenureads the selection behind the typing saves in the host's FIFO, and a bUnit test gates the write to prove the ordering. And the module side flushes the DOM observer before reading (flushDom), pinned by a same-task browser test.aria-disabled, notdisabled— every-graph-unavailable still takes focus and Esc still closes; and the scoped::deepstyle anchor is itself pinned by asserting the scope attribute survives. A CSS-isolation rule that would die silently otherwise — sneaky little thing, caught~ChapterPagepasses noSelectionGraphs, the module mounts unarmed, the e2e suite pins the browser's own menu stays, and the coverage bot (current for this head) showsSelectionInput100%/100% andReadSelection100%.ManuscriptModuleFakes.cs— two copies become oneinternalhelper, extended withInvocations/OnCallfor the overlay-ordering pin. DRY done right.💡 Little ideas (non-blocking)~
SelectionMenuTests.cscarries 9[Fact]s (browser suite's 10 and reader's 7 both check out). The body also still tells the pre-fix story;db9956a's commit message tells the new one. A body touch-up before merge keeps the record straight~What I ran at
db9956a(fresh Release build, 0w/0e): all six .NET projects — Adapter 462/462, UseCases 243/243, Domain 486/486, Integration 540/540, Flow 1103/1103, OpenRouter 592✓+4 skips (the deltas vs6db832care main's own #55/#57 test arrivals through the rebase); vitest 213/213; typecheck clean; Playwright chromium+webkit 95/95, seven loaded runs green. (Firefox cannot launch in my sandbox —CanCreateUserNamespace EPERM, an environment limit; your CI's three-engine 150/150 covers it.) Mutation probes at the pre-fix head: swallow-guard and numbering guards each killed on exactly their own pin; every probe restored, clone pristine atdb9956a.Fufu~ the writer notices nothing today, Phase 3 arrives to find its table set, and the one real bug was found twice and killed once — properly~ Merge it~ ♡
Automated review by Jibril · 2026-09-26
CI/CD: coverage bot CURRENT for head
db9956a(98.1% line / 94.2% branch) · Local checks: full — build 0w/0e, 6/6 .NET projects, vitest 213✓, Playwright chromium+webkit 7×95✓ under load