[Barbara Kudiess (bkudiess)]

Summary

Add opt-in, message-scoped selection across Markdown paragraphs, headings, lists, code and tables, with semantic custom surfaces, visibility gates, mouse/keyboard navigation, copy feedback and range-scoped UI Automation embedded controls. Default Markdown rendering remains unchanged; the guide and agent reference document structural scope ownership, and the generated API index includes the complete Advanced factory/modifier/hook/enum surface.

Linked issue / spec

Related: openclaw/openclaw-windows-node#1844 (downstream draft and port motivation; not closed by this PR).

Test plan

Current head: e107ff3f, based on 74cdd322. <artifact-output> redacts only local TRX storage.

• dotnet build tests\Reactor.Tests -c Debug -p:Platform=ARM64 -p:SkipSignaturesGen=true -p:SkipReactorApiGen=true -m:1 -nodereuse:false -v:q — passed; existing Pkcs advisory warnings remain.
• dotnet test tests\Reactor.Tests --no-build -p:Platform=ARM64 --filter-class '*MarkdownTextSelectionTests' — 73 passed, zero failures/skips, including wholesale replacement, streaming preservation and press-time modifier policy. With the new reset disabled, eight intended reset regressions fail; all pass with it restored.
• dotnet build tests\Reactor.AppTests.Host -c Debug -p:Platform=ARM64 -p:SkipSignaturesGen=true -m:1 -nodereuse:false -v:q — passed, zero warnings/errors; compiles the native host without launching UI.
• dotnet build src\Reactor.Advanced -c Release -p:Platform=ARM64 -p:SkipSignaturesGen=true -m:1 -nodereuse:false -v:q — passed, zero warnings/errors. git diff --check — passed.
• Current full managed run: dotnet test tests\Reactor.Tests --no-build -p:Platform=ARM64 --report-trx --results-directory <artifact-output> — 15,631 passed, 18 existing skips, zero failures.
• dotnet run --no-build --project tools\Reactor.DocPipeline -- compile --topic text-and-media --no-build --no-screenshots --skip-diagrams --no-ai — passed; regenerated the changed guide. dotnet test tests\Reactor.DocPipeline.Tests --no-build -p:Platform=ARM64 --filter-class '*InlineSnippetLedgerTests' — 15 passed, zero failures/skips.
• Both API-index copies regenerated through the existing UPDATE_API_INDEX=1 test path; exact-signature, duplicate-section, core-stability and committed-output gates passed.
• dotnet restore Reactor.slnx -v:q — passed.
• dotnet build Reactor.slnx --no-restore -c Release -v:q — failed locally with the same seven MSB3030 missing generated Copilot CLI assets in demo-script-tool/widget-creator samples. Current-head hosted Build solution passed; this local sample-asset limitation is not being represented as a local build pass.
• Current-head e107ff3f CI passed: CI run 42712579354 passed both E2E, both unpackaged/packaged selftest and both AOT selftest shards, plus unit/integration/docs/solution build and AOT Trim Proof. Coverage, Bootstrap, package and Node checks passed. The final current-head snapshot has 40 passing checks and two intentional workflow skips, with no pending/failing checks.
• Current-head Selection_RoutedInputAndExternalEvents passed with actual input: hyperlink click/drag and range containment, no extra activation, nested Ctrl+A/C with unchanged outer endpoints and a fresh clipboard write, exact selection-change events, Shift+click, exact streaming TextChanged, and three repeated scroll/selection advances with a physically stationary pointer. Logged samples increased scroll from 0% to 4.23%, 8.46% and 12.69%, and selected length from 142 to 178, 232 and 286 characters.
• Current-head AOT TAP explicitly passed the initial streaming document/selection positive controls, append preservation, prefix-insertion remapping with retained surface identity, and keyed scope replacement in a reused ItemsView row; total AOT shard-2 failures: zero. Native coverage also checks key isolation, keyboard/host-copy feedback, selection brushes, visual UIA lines/ranges, cache retention and clipped-surface exclusion.
• Missing-scope detection is asserted in every native lane; JIT lanes also require the actual warning and a direct warning-delivery positive control. AOT explicitly reports the emission assertion unsupported only when its positive control proves EventSource delivery is disabled. A green AOT job does not claim ETW emission was observed in an EventSource-disabled binary.
• Current-head native/AOT evidence passed MdSelection_UnkeyedRowInitialRealization, MdSelection_UnkeyedRowInitialSelection, MdSelection_UnkeyedRowStreamingKeepsSelection and MdSelection_UnkeyedRowWholesaleReplacementClearsSelection: the same unkeyed host and surface identities survive an append with selection preserved, then receive unrelated text with both endpoints reset to zero. AOT shard 2 reports zero failures; both selftest and packaged shards also passed. No local UI/input was launched.
• Earlier native/downstream evidence is preserved, not attributed to this head: ARM64 Markdown selftests at af79d853 passed 52 checks with no skips, with mutation-backed cache/collapse/value-equal-options regressions; sealed original 639d36ff downstream Debug and unsigned Release acceptance each passed 1/1 and mounted UI 18/18.

No local UI/window/input was launched for the current review fixes. Physical Narrator/High Contrast, signed downstream packaging, a performance benchmark and a fresh NativeAOT API-index byte comparison remain unverified. The maintainer approved baseline cc5fd5f and suggested this unkeyed-reuse follow-up. Nine fixed threads are resolved; two design-tradeoff discussions remain open. The follow-up is now verified by current-head CI and ready for maintainer consideration; the earlier approval is not being represented as a new review of this commit.

Risk / breaking changes

The new APIs are opt-in. Surfaces belong to their nearest enclosing Scope by structure; a non-null parser TextSelection only emits surfaces and only Scope.Options supplies copy feedback. A missing scope logs a Release-visible warning when the runtime supports EventSource. Visibility gates selection membership, not native visibility; virtualized rows should key the scope by logical message identity. As a fallback, wholesale text replacement clears selection/caret if no nonblank surface text survives and no reused surface retains nonblank prefix/suffix text; streaming and partial edits retain existing remapping. Shared text can belong to a different message, so this heuristic cannot replace explicit keys. Touch/pen remain available for native panning, and descendant controls retain handled keys.

Selection geometry, virtualized lifecycles, scrolling and UIA are native, performance-sensitive paths. Offscreen surfaces are culled before glyph queries, native highlight brushes replace fixed RGB, visual lines use native caret geometry, and geometry caches survive unchanged/viewport-only refreshes. Full membership/native-text scans remain a correctness tradeoff for ancestor collapse and inline mutation outside reconciliation; transcript-wide traversal cost is not claimed eliminated. The WindowsDesktop UIA/clipboard dependency remains confined to the E2E project. The Advanced index now widens the shipped agent reference, and its AOT gate roots both assemblies.

[Alexandre Zollinger Chohfi (azchohfi)]

The design holds up: the wrapper-as-decorator approach fits the registry, unload/reload and nested scopes are handled carefully, and the RTL glyph check against the native TextPattern is a good oracle. Two things block merge, and there are several smaller issues.

Blocking

• E2E shard 2 is red on the new Selection_RoutedInputAndExternalEvents. It fails at its own foreground guard, before it navigates or sends any input. See the inline comment. As written, this test will go red on unrelated PRs too.
• No passing test covers pointer selection: press, drag, Shift+click, the link-vs-drag threshold, or edge auto-scroll. The only test that routes real input is the red E2E, and the PR description says link-drag is still unresolved.

Worth fixing before this ships

• Setting TextSelection without a Scope silently makes the Markdown unselectable.
• The scope's KeyDown handler runs with handledEventsToo and uses a deny-list, so Ctrl+C can overwrite a copy that a focused descendant already made.
• Every scope runs a full subtree walk plus native text re-read on every LayoutUpdated. DrawSelection measures every selected grapheme before it clips to the viewport. In a long chat transcript both add up fast.
• The highlight is hardcoded at #5A0078D7, so High Contrast and accent colour are lost compared to native selection.
• Touch and pen presses start a selection drag, so a touch pan that starts on text collapses the selection and steals focus.
• UIA TextUnit.Line uses hard newlines, not visual lines.

API index / packaging

• Adding Reactor.Advanced to reactor.api.txt makes sense, but the new section leaves out every Advanced enum, fluent modifier and hook (MarkdownParserFlags {, UseDockHost, TargetFps all have 0 hits), because EmitPublicTypes assumes the core sections already emitted them. Since this widens what ships in the agent kit, please also update the header, the mur --regen-api help text and tools/Reactor.ApiIndex/README.md (they still say Reactor.dll only), and add a CHANGELOG Changed line.
• Reactor.SignaturesGen.csproj: the ReactorApiAot gate roots only Reactor (<TrimmerRootAssembly Include="Reactor" />). Now that it reflects over Advanced too, add Reactor.Advanced there or the "byte-identical" note at line 51 no longer holds.
• Reactor.AppTests.csproj turns on UseWPF just for System.Windows.Automation and Clipboard in one test. UiaPropertyReader already does UIA through COM interop. Not a blocker for a test project, but it's an extra dependency for one file.

Test gaps (non-blocking, but real)

• Keyboard handling (arrows, Shift-extend, Home/End, Escape, PageUp/Down, the IsChrome filter) and the host-level CopySelection -> OnCopyResult wiring have no tests at any tier.
• Streaming selftest (MarkdownSelectionFixtures.cs:127-133): an append at the end stays green even if Remap is replaced with Math.Min(offset, length). Assert surface identity survives the update, or stream an edit that shifts earlier offsets.
• Nothing covers ItemsView recycling. A recycled container keeps the same host and Anchor/Caret, so a selection could land on a different message.
• Most of the UIA range surface is untested: ExpandToEnclosingUnit, Word/Line/Paragraph boundaries, RangeFromPoint, GetVisibleRanges, RemoveFromSelection.

Docs

• skills/dsl-reference.md (agent kit) has nothing on the Scope/Surface/Visibility pattern or the missing-Scope pitfall. A short note there would help agents use this correctly.
• Small naming point: MarkdownTextSelection.Visibility(bool, Element) reads like visual visibility. The XML doc and guide both have to explain that it isn't. Something like Include(...) would make the call site read correctly without the doc.

[Alexandre Zollinger Chohfi (azchohfi)]

The docs say to use the same options instance for every leaf, but the builder only ever checks TextSelection is null. OnCopyResult comes solely from the Scope element, so an OnCopyResult set on a leaf's options is silently ignored. Either make this a plain flag on MarkdownOptions and keep the options object only on Scope, or have the scope register only surfaces whose options match its own by reference.

[Barbara Kudiess (bkudiess)]

Keeping structural ownership and the existing API shape. Reference matching previously dropped a child component's surfaces when its parent recreated value-equal options and ShouldUpdate skipped that child; MdSelection_ValueEqualOptionsRetainSkippedChild pins that regression. In 28f124b the XML docs, guide and DSL reference explicitly say that a non-null TextSelection only enables parser surfaces, options identity is irrelevant, and only Scope.Options supplies copy feedback.

[Alexandre Zollinger Chohfi (azchohfi)]

The docs are clear now. I still think a property whose type carries a callback that gets ignored will trip people up, and it's cheap to change before this ships: make MarkdownOptions.TextSelection a bool and keep MarkdownTextSelectionOptions only on Scope. Ownership stays structural either way, so MdSelection_ValueEqualOptionsRetainSkippedChild isn't affected. Not blocking, your call.

[Barbara Kudiess (bkudiess)]

No passing test covers pointer selection / keyboard and host-copy gaps / streaming and recycling / UIA range surface.

28f124b adds keyboard ownership/arrow/Home/End/Escape and host-copy callback checks, a prefix-insertion streaming check that asserts retained surface identity and shifted offsets, keyed scope replacement in a reused ItemsView row, native visual-line comparison, Word/Paragraph/RangeFromPoint/GetVisibleRanges/RemoveFromSelection coverage, and Shift+click plus hyperlink-range assertions in the external proof. The input harness now acquires owned foreground before guarding, uses the existing interactivity/retry policy, and preserves the shared host. It waits until the link press is delivered before moving the physical pointer. These native/input additions are built, not claimed passing yet; fresh CI must establish the pointer behaviors, including the still-unresolved link drag and stationary autoscroll.

API index / packaging.

Advanced factories, enums, modifiers and hooks are indexed; both copies, headers/help/README and the changelog are updated. The AOT index gate roots both assemblies. The managed index tests passed; I have not claimed a fresh NativeAOT index byte-comparison run.

UseWPF just for one test / Visibility naming.

I retained the test-only WindowsDesktop dependency: this proof needs managed TextPattern ranges/events and an STA clipboard reader beyond the existing UiaPropertyReader property interface; replacing that with new handwritten COM range/event interfaces would broaden this repair. It does not enter shipping packages. I also retained the agreed public Scope/Surface/Visibility shape, with explicit membership-only semantics and the missing-Scope warning in XML docs, guide and the shipped DSL reference.

The full membership/native-text scan remains a deliberate correctness tradeoff, explained in its inline reply; the geometry/cache and owner-transform costs are reduced, not eliminated. Local full managed validation passed 15,617 tests with 18 existing skips after merging current main (7277138), and app/host Debug plus Advanced Release builds passed. All human threads are left open for your re-review; the PR is not being presented as technically green before the new hosted input run.

[Alexandre Zollinger Chohfi (azchohfi)]

Thanks for working through all of this. CI is green on cc5fd5f and I checked the fixes behind the resolved threads. Approving.

One follow-up worth tracking: recycling an ItemsView row is now handled only by the docs telling people to key the scope with .WithKey(message.Id). Without the key, a reused row keeps the same host and markers, so Remap carries the old anchor/caret onto whatever message the row shows next. It would be nice if the scope reset the selection when the content was replaced wholesale (for example, when no entry's text overlaps the previous document), so the unkeyed case fails safe.

[Barbara Kudiess (bkudiess)]

It would be nice if the scope reset the selection when the content was replaced wholesale … so the unkeyed case fails safe.

Implemented in e107ff3. A text-changing rebuild resets anchor and caret to zero when no nonblank surface text survives and no reused surface retains nonblank prefix/suffix text. Empty/whitespace surfaces and shared surrogate halves do not count as continuity; retained text still uses the existing streaming/edit remapping. Message keys remain the reliable identity boundary when unrelated messages share text.

Added managed reset/preservation cases and a real ItemsView regression that requires the unkeyed host and markers to survive both a streaming append and an unrelated replacement. The 73 focused tests pass; disabling the reset produces eight intended failures. The full managed suite passed 15,631 tests with 18 existing skips, and the native host Debug/Advanced Release builds passed. Current-head CI is now green: run 42712579354 passed both E2E, selftest, packaged and AOT shards, and all other checks are green (40 passing, two intentional workflow skips). The saved AOT TAP explicitly passes unkeyed-row realization, initial selection, streaming preservation and wholesale-replacement reset with reused host/marker identities; the complete external input proof also passed. No local UI/input was launched.

[Alexandre Zollinger Chohfi (azchohfi)]

Thanks for adding the reset in e107ff3. It covers the unkeyed recycled-row case, and the ItemsView regression is a good test for it. Still approving at this head.