Fix: Preserve subtree liveness across EquatableView cache hits - #64
Merged
Conversation
- EquatableEffectTests pins issue #14 end-to-end through the RenderLoop pipeline: lifecycle slots/tasks, key handlers, status-bar items, focus registrations, and nested cache entries all vanish across EquatableView cache hits (withKnownIssue markers) - Documents the first-frame hit source: the measurement pass stores into the live cache while effect sites are inert, so the same frame's main pass already hits and the subtree's effects never mount at all - Migrate the live-path marker test from RuntimeCharacterizationTests to the new FrameHarness end-to-end form
- EquatableView classifies its content on every cache miss: the pass's effect-registration probe is snapshotted around the rendering, and any delta flags the identity in RenderCache so lookups always miss and the subtree renders every frame - The probe (package env key, installed by RenderLoop.passEnvironment) sums all per-pass effect sinks: key handlers, preference writes, status-bar declarations, staged focus registrations, and deferred lifetime-effect records; new counters: PreferenceStorage.writeCount, StatusBarState.passRegistrationCount, FocusManager.stagedRegistrationCount - Measurement passes stay out of the cache entirely: a buffer stored in .measure let the same frame's output pass hit a subtree whose effects were never recorded (they never mounted at all) - Live path (no collectors) keeps its historical caching semantics - EquatableEffectTests lifecycle/handler/status-bar/focus markers flip green and are dropped; the nested-entry marker stays for Task 3
- RenderCache.markSubtreeActive keeps every entry at or below a cached subtree root active without traversal (ancestor logic like clearAffected(by:)) - TUIContext.applyFrameLiveness marks subtree roots via markSubtreeActive instead of markActive; the EquatableView live path gains the same call - The nested-entry characterization marker flips green and is dropped — EquatableEffectTests now asserts all issue #14 effect contracts directly
- A focus section declared without focusables inside cacheable content produced no probe delta, so the subtree was classified effect-free and the section vanished from committed Tab cycling on cache hits - stagedRegistrationCount now counts staged sections plus focusables; new end-to-end test pins the section survival
- EnvironmentFingerprint (type-erased Equatable snapshot, TUIkitView) joins identity, view value, and size in the RenderCache key; a mismatch misses so a style change above an .equatable() boundary can never serve a stale buffer - RenderLoop installs the per-pass fingerprint probe snapshotting the issue #14 audit members: foregroundStyle and focusIndicatorColor (pulse-derived, so cached entries inside an active focus section refresh every tick); palette/appearance stay covered by the global EnvironmentSnapshot clear, pulsePhase itself stays excluded - Live path stores and matches nil fingerprints, keeping its historical cache key - New end-to-end test: a foregroundStyle change between frames misses and re-stores, an unchanged style hits again
- Subtree Memoization rewrite: measure-pass exclusion, effect-bearing classification and bypass, liveness guarantees for hits, environment fingerprint in the cache key - Refresh the when-(not)-to-use guidance: effect-bearing subtrees are auto-bypassed and safe, wrapping them just adds nothing
23 tasks
wadetregaskis
added a commit
to wadetregaskis/TUIkit
that referenced
this pull request
Sep 2, 2026
…rified The closest upstream has come to our own code. Their effect-bearing bypass is already ours by another route — LifecycleModifier declares a render side effect at all three registration sites, and EquatableView refuses to store a buffer whose render tripped the tracker. The other two are recorded as `queued` rather than guessed: Nested cache entries below a cached root may be collectable while live. Upstream added RenderCache.markSubtreeActive; ours marks the subtree in StateStorage but only the root identity in the cache. Symptom would be a miss storm — bad, benign. Our cache key has no environment component at all. A .foregroundStyle change ABOVE an .equatable() boundary, view value unchanged, has nothing to miss on. Symptom would be wrong pixels, so this is the one that matters. Both need a written repro before a fix, and the session that reached them had no room to do that properly. A wrong `have` here would close a door on something that serves stale pixels, and PR-45.md already records what it costs when a probe agrees with its author by accident. Co-Authored-By: Claude <[email protected]>
wadetregaskis
added a commit
to wadetregaskis/TUIkit
that referenced
this pull request
Sep 2, 2026
An .equatable() cache hit returns the stored buffer and skips the subtree entirely, so no view inside ever reaches RenderCache.markActive. A nested .equatable() below it therefore looked absent from the tree, and removeInactive collected its entry at the end of every hit frame. The steady state was one entry where there should have been two, and the first frame the OUTER value changed, the inner subtree re-rendered from scratch even though nothing in it had changed. StateStorage already solves exactly this for the @State inside a cached subtree — retainSubtree, declared per pass by the same cache-hit path — so the cache gets the twin rather than upstream's markSubtreeActive naming. Following the pattern that exists beats importing a second one. Per-pass, so it does not leak: a subtree that stops being declared is collectable on the next frame, which the third test pins. Costs nothing measurable. Stress --bench, per-frame, before → after: megalist 400.5µs → 389.5µs deep 6842.3µs → 6688.6µs textwall 3791.4µs → 3834.7µs kitchensink 761.9µs → 762.4µs All four are inside run-to-run noise and every checksum is identical, so rendering is unchanged. Nor is that a strong result: the harness barely uses .equatable(), so it exercises the new retention path only lightly. What bounds the cost is the shape — the ancestor walk runs only for entries NOT visited this pass, against the handful of roots that hit. Also lands the repro for the OTHER PR phranck#64 concern as a characterization: the cache key has no environment component, so a scoped .foregroundStyle change above an .equatable() serves the buffer rendered under the old style. Those two tests assert the wrong behaviour on purpose and will fail the day it is fixed. The fix is a design fork, not a patch — see open-questions.md. Co-Authored-By: Claude <[email protected]>
wadetregaskis
added a commit
to wadetregaskis/TUIkit
that referenced
this pull request
Sep 2, 2026
…r from it All three of PR phranck#64's fixes are now resolved against our tree: the effect-bearing bypass we already had, and both cache contracts reproduced and fixed. The environment one is `adapt` rather than `have` because we did not take upstream's shape — a fixed fingerprint list would leave every style value nobody remembered to add still broken. open-questions.md is down to the branch note. Queued: 3 → 2, both the older FrameBuffer and AppState.shared questions. Records the measurement lesson with the verdict, since that is where someone revisiting the perf trade would look: the first "+10 to 20%" readings were cross-time comparisons on a machine that drifts ~20% between runs. Only interleaved A/B is worth anything here. Co-Authored-By: Claude <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the full issue #14 plan: cached subtrees can no longer lose descendant effects, and only provably effect-free content is ever served from the
RenderCache.EquatableEffectTests,FrameHarnessend-to-end): pinned five losses across cache hits — lifecycle slots/tasks, key handlers, status-bar items, focus registrations, nested cache entries. Notable root-cause finding: the first frame's measurement pass stored into the live cache while effect sites were inert, so the same frame's main pass already hit and the subtree's effects never mounted at all.EquatableViewsnapshots the pass's effect-registration probe (key handlers, preference writes, status-bar declarations, staged focus sections/focusables, deferred lifetime-effect records) around the rendering. Any delta flags the identity inRenderCache— flagged identities never hit, so effect-bearing subtrees render every frame and.equatable()becomes safe to apply anywhere. Measurement passes stay out of the cache entirely.RenderCache.markSubtreeActivekeeps entries below a cached root alive across outer hits (applyFrameLiveness+ live path).EnvironmentFingerprint(foregroundStyle,focusIndicatorColorfrom the audit); a style change above an.equatable()boundary now misses instead of serving a stale buffer. Palette/appearance stay covered by the globalEnvironmentSnapshot,pulsePhasestays excluded.RenderCycle.mdSubtree Memoization rewritten (bypass semantics, liveness guarantees, fingerprint invalidation, refreshed usage guidance).Public API is unchanged — all new plumbing is
package/internal.Fixes #14
Test plan
EquatableEffectTestsnow asserts the contracts directly (plus new section-survival and style-invalidation tests)./scripts/test-linux.sh: full warning-fatal gate green on macOS and Linux (1322 tests, zero known issues)