Fix: Derive the image placeholder instead of mutating state in traversal - #63
Merged
Conversation
- _ImageCore no longer writes its phase/lastSource state boxes while rendering; the placeholder for a changed source is derived from lastSource != source, so it still shows in the same frame - The mounted loading task is now the only writer: it commits phase first, then lastSource, and skips the commit when cancelled, so a stale load can never pair with a newer source - Since #58 every image frame emitted a body-side-effect RuntimeDiagnostic (unconditional lastSource write); rendering is now diagnostic-free - Add ImageRenderPhaseTests and an injectable TUIContext on FrameHarness
- mountLoadingTask reads lifecycle, loader, cache, timeout, and pixel limit from context.environment instead of taking nine parameters (SwiftLint function_parameter_count)
wadetregaskis
added a commit
to wadetregaskis/TUIkit
that referenced
this pull request
Sep 2, 2026
…open PRs phranck#60-phranck#62 are the rest of the render-phase work: per-pass collectors, an ordered pending-effect log committed from the final traversal, and a body-mutation diagnostic. Deferred as a real backlog item — it is the only design that closes the gap a338e81f pins, since the header correction cannot know which walk is final until after both have run. Notes record the order to do it in and that it wants Stress --bench on both sides, given the live perf concern. PR phranck#63 is queued, and honestly so. Their diagnostic immediately caught _ImageCore writing its lastSource box on every render; ours does the same unconditional write, and StateBox.didSet invalidates without comparing. The suspicion is that a screen with an Image never lets the loop idle. That is a suspicion, not a finding. A unit test failed to discriminate — the sink defers to the main-actor frame boundary, so a synchronous renderToBuffer never reaches the point where anything moves — and the probe was deleted rather than left asserting something it does not test. Tools/Profiling/idle_cpu.py exists for exactly this question and is the right instrument; the one-line fix can follow the measurement rather than stand in for it. 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
_ImageCoreno longer mutates itsphase/lastSourcestate boxes while rendering. The placeholder for a changed source is derived fromlastSource != source, so it still appears in the same frame as the source change — without any state write inside the traversal window.resetLoadingPhaseIfNeededwrotelastSourceBoxon every frame, so every frame containing an image emitted a body-side-effectRuntimeDiagnostic(plus a superfluous invalidation). Rendering an image is now diagnostic-free.ImageRenderPhaseTestscover both contracts end-to-end viaFrameHarness(which gained an injectableTUIContextfor deterministic loaders);mountLoadingTasknow reads its dependencies from the render context (SwiftLintfunction_parameter_count).Refs #27 — delivers the "Rendering does not mutate image phase merely by reading the same source" acceptance criterion; the remaining transport/validation/dedup work stays in #27.
Test plan
ImageRenderPhaseTests: RED verified against the old behavior (diagnostic on every frame), GREEN after the fix./scripts/test-linux.sh: full warning-fatal gate green on macOS and Linux (1317 tests, 1 expected known issue for [P0-09] Preserve subtree liveness across EquatableView cache hits #14)