fix(reconciler): re-render context consumers behind reference-stable child skips (#811)

[nsreehari]

Bug: pure context changes lost behind reference-stable child skip paths

Fixes #811.

Root cause

A UseContext consumer nested inside a reference-stable subtree stopped
re-rendering on pure context changes. When a parent re-rendered with a changed
provider value but returned the same child Element instance, the
reconciler's shallow-equality fast paths short-circuited before descending
into the consumer, so HasConsumedContextChanged was never consulted. The
consumer kept rendering the stale context value, and event handlers it created
captured that stale value.

The skip was reachable at two independent layers, so a single-site fix was
insufficient (verified: removing either guard re-breaks the repro).

Fix

Both skip fast paths are now context-aware:

• Reconciler.Update.cs — before the element-level ShallowEquals skip,
  decline the skip when a consumed context value changed in the subtree.
• ChildReconciler.cs (UpdateCommonChild) — before the per-child
  CanSkipUpdate skip, run the same check and fall through to a real update.

Both are gated by the new ContextScope.HasActiveValues so no subtree walk
happens when no provider is active, and are driven by
Reconciler.HasConsumedContextChangedInSubtree(control), which checks the
control's ComponentNode and recurses via ForEachReactorChildControl.

Tests

New self-test regression fixture
Issue811ContextConsumerSkipFixtures.ReferenceStableChildSkip_ContextConsumerRerenders
(registered in SelfTestFixtureRegistry) covering mount, provider-toggle
re-render, and current-vs-stale context use from an event handler.

Validation

• Focused host selftest Issue811_ReferenceStableChildSkip_ContextConsumerRerenders: 8/8 checks pass.
• Reactor.Tests slice (ChildReconcilerStructuralSkip, ContextSystemSelfHost, MemoizationSelfHost, ComponentModelIntegration): 56 passed / 0 failed.
• Full Reactor.SelfTests batch: passed.

[Copilot]

Pull request overview

Fixes a reconciler correctness bug where UseContext consumers inside a reference-stable child subtree could miss pure context updates due to early skip fast-paths, leaving UI and captured event handlers stale.

Changes:

• Make both element-level and per-child skip paths context-aware by consulting subtree context-consumer invalidation before skipping.
• Add ContextScope.HasActiveValues/Reconciler.HasActiveContextValues gating and a HasConsumedContextChangedInSubtree(UIElement) traversal helper.
• Add a self-test regression fixture for Issue #811 and register it in SelfTestFixtureRegistry.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs	Registers the new Issue #811 self-test fixture name and constructor mapping.
tests/Reactor.AppTests.Host/SelfTest/Fixtures/Issue811ContextConsumerSkipFixtures.cs	New self-test reproducer validating context-driven rerender behind reference-stable child skips.
src/Reactor/Core/Reconciler.Update.cs	Declines element-level shallow skip when a consumed context value changed in the subtree.
src/Reactor/Core/Reconciler.cs	Exposes HasActiveContextValues and adds HasConsumedContextChangedInSubtree traversal helper.
src/Reactor/Core/ContextScope.cs	Adds HasActiveValues to allow gating context-related subtree checks.
src/Reactor/Core/ChildReconciler.cs	Declines per-child CanSkipUpdate skip when a consumed context value changed in the existing child subtree.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[azchohfi]

Good catch, fixed in f7f2485. I folded the walk into the skip predicate as the last && term: !(HasActiveContextValues && HasConsumedContextChangedInSubtree(control)). Now short-circuiting means the recursive subtree walk only runs after the cheap structural gates (ShallowEquals/ModifiersEqual/etc.) all pass, so a non-skippable element never pays for it, and it's skipped entirely when no provider is on the stack.

Note this only covered the element-level Update path. The per-child arm in ChildReconciler already gates the walk behind CanSkipUpdate + HasActiveContextValues before the children.Get(i) fetch, so it's fine. The deeper follow-up is that the gate keys off HasActiveValues (any provider present) rather than whether a provided value actually changed this render, so stable subtrees still get walked even when no context changed. Worth a follow-up using a context version signal.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[azchohfi]

Review summary — context-consumer re-render fix (#811)

Thanks for tackling this. The core mechanism — declining the skip when a consumed context changed, via HasConsumedContextChangedInSubtree — is sound, and the primary case (a reference-stable component sibling under .Provide(...)) is fixed and covered by the new selftest.

My main concern is that the fix is incomplete for keyed lists, which I'd treat as a blocker. Details are in the inline comments; summarizing here:

Blocking

• Keyed + hint skip paths bypass the new check. The context-aware decline only lives in the positional UpdateCommonChild arm and the element-level shallow skip. The keyed prefix (ChildReconciler.cs ~L313) and suffix (~L362) arms, and the ChildDiffHints fast-path (~L111–143), still skip on CanSkipUpdate with no context check — so a keyed stable child (grid rows, keyed list items) that consumes context still goes stale on a provider change. That's the same #811 bug via the keyed path. I'd centralize the decision into one ShouldDeclineSkip(oldEl, newEl, control) helper shared by all four skip sites — that closes the gap in one place and lets the positional arm drop the one-off goto.

Non-blocking

• Traversal blind spot. ForEachReactorChildControl doesn't cover SplitView (Pane/Content) or Viewbox (Child), both real Reactor child hosts — consumers under them inside a stable wrapper are missed.
• Coarse gate / perf. The walk is gated on HasActiveValues (any provider present), not on whether a value actually changed, so stable subtrees get walked speculatively on every skip while a provider is active. A _version-derived change signal would avoid it. (The earlier hot-path ordering nit from the Copilot review is already handled in f7f2485d.)
• Test coverage. Add keyed, deeper-nested (stable non-component container), and negative (unchanged-context ⇒ no re-render) selftest variants.
• CHANGELOG. No ### Fixed entry for #811 (user-visible bug).

Happy to pair on the ShouldDeclineSkip refactor if useful.

[azchohfi]

This context-aware skip decline only lives here in the positional UpdateCommonChild arm, so the fix is incomplete for keyed lists. The other skip sites in this file don't have it:

• keyed prefix skip (~L313) and keyed suffix skip (~L362) still continue on CanSkipUpdate with no HasConsumedContextChangedInSubtree check, so a keyed stable child (grid rows, keyed list items) that consumes context still goes stale on a provider change — the exact Bug: pure context changes can be lost behind reference-stable child skip paths #811 bug via the keyed path;
• the ChildDiffHints fast-path (~L111-143) skips untouched indices wholesale and only calls UpdateCommonChild for changed indices, so it bypasses the check too.

The rule now exists in three shapes (this goto perform_update, the keyed continue, and the && chain in Reconciler.Update.cs). I'd pull it into a single ShouldDeclineSkip(oldEl, newEl, control) helper used by all four sites — that closes the keyed/hint gaps in one place and lets this arm drop the one-off goto. Please add a keyed selftest variant to lock it in.

[azchohfi]

HasActiveValues gates the subtree walk on any provider being on the stack, not on whether a provided value actually changed this render. With a provider near the app root (the common case), every reference-stable skip now pays a recursive HasConsumedContextChangedInSubtree walk even when no context changed — a real cost on the hot skip-floor.

There's already a _version here that bumps on every push/pop. Consider deriving a cheaper change signal from it (or a per-node "consumed-context version at last render") so stable subtrees are only walked when a provided value actually changed, rather than whenever any provider is present.

[azchohfi]

This walk is only as complete as ForEachReactorChildControl, which covers Panel / ItemsRepeater / Border / ScrollViewer / UserControl / ContentControl but not SplitView (Pane/Content) or Viewbox (Child) — both real Reactor child hosts. A context consumer under one of those inside a reference-stable wrapper will be missed and stay stale, so the fix has a blind spot.

Better to drive traversal off the descriptor child metadata / GetCurrentChild accessors so every V1 child-hosting strategy is covered, rather than an ad-hoc control-type switch. (Unmount is unaffected — it uses strategy-driven teardown — but this walk and the nav-lifecycle traversals that reuse the helper share the gap.)

[azchohfi]

Solid repro for the positional/unkeyed case. To cover the rest of the fix, worth adding three variants:

1. Keyed: put .Key(...) on stableOverlay so it routes through the keyed prefix/suffix skip arms (currently uncovered — see the ChildReconciler comment).
2. Nested: wrap the overlay in a reference-stable non-component container (a stable Border/ScrollViewer) so the recursive walk in HasConsumedContextChangedInSubtree is actually exercised — here the consumer is a direct child, one level deep.
3. Negative: re-render from unrelated state with the provider value unchanged and assert probe.RenderCount does not increase, so the coarse HasActiveValues gate can't regress into over-rendering.