Fix: Derive the image placeholder instead of mutating state in traversal

Sources/TUIkit/Views/_ImageCore.swift

@@ -13,7 +13,11 @@ private enum StateIndex {
     /// Stores the loading phase (`ImageLoadingPhase`).
     static let phase = 0
 
-    /// Stores the last loaded source for change detection (`ImageSource`).
+    /// Stores the source of the last committed load (`ImageSource?`).
+    ///
+    /// Written exclusively by the mounted loading task, never during
+    /// traversal. A mismatch with the current source means the stored
+    /// phase belongs to an outdated source.
     static let lastSource = 1
 }
 
@@ -45,7 +49,6 @@ struct _ImageCore: View, Renderable, Layoutable {
 
     func renderToBuffer(context: RenderContext) -> FrameBuffer {
         let stateStorage = context.environment.stateStorage!
-        let lifecycle = context.environment.lifecycle!
         let identity = context.identity
 
         let width = context.availableWidth
@@ -63,10 +66,6 @@ struct _ImageCore: View, Renderable, Layoutable {
         let aspectRatioOverride = context.environment.imageAspectRatio
         let placeholderText = context.environment.imagePlaceholderText
         let showSpinner = context.environment.imagePlaceholderSpinner
-        let maxPixelCount = context.environment.imageMaxPixelCount
-        let urlTimeout = context.environment.imageURLTimeout
-        let imageLoader = context.environment.imageLoader
-        let imageCache = context.environment.imageCache
 
         // Retrieve or create persistent phase state
         let phaseKey = StateStorage.StateKey(identity: identity, propertyIndex: StateIndex.phase)
@@ -78,14 +77,18 @@ struct _ImageCore: View, Renderable, Layoutable {
             stateStorage.markActive(identity)
         }
 
-        // Track the last loaded source to detect changes
+        // Track the source of the last committed load
         let sourceKey = StateStorage.StateKey(identity: identity, propertyIndex: StateIndex.lastSource)
         let lastSourceBox: StateBox<ImageSource?> = stateStorage.storage(for: sourceKey, default: nil)
 
-        // Detect source change and show the placeholder for the new load.
-        // Task cancellation and restart are handled by the updateTask
-        // restart ID below (the source itself).
-        resetLoadingPhaseIfNeeded(phaseBox: phaseBox, lastSourceBox: lastSourceBox)
+        // The placeholder for a changed source is DERIVED instead of
+        // written: state writes during traversal violate the render-phase
+        // contract (they would emit RuntimeDiagnostics). The stored phase
+        // only counts while it belongs to the current source; the mounted
+        // task commits the new phase and source after the frame.
+        let phase: ImageLoadingPhase = lastSourceBox.value == source
+            ? phaseBox.value
+            : .loading
 
         // The loading task is a lifetime effect bound to this image's
         // structural identity. The SOURCE is the restart ID: an unchanged
@@ -95,18 +98,13 @@ struct _ImageCore: View, Renderable, Layoutable {
         if context.phase == .render {
             mountLoadingTask(
                 phaseBox: phaseBox,
-                identity: identity,
-                context: context,
-                lifecycle: lifecycle,
-                imageLoader: imageLoader,
-                imageCache: imageCache,
-                urlTimeout: urlTimeout,
-                maxPixelCount: maxPixelCount
+                lastSourceBox: lastSourceBox,
+                context: context
             )
         }
 
-        // Render based on current phase
-        switch phaseBox.value {
+        // Render based on the derived phase
+        switch phase {
         case .loading:
             return renderPlaceholder(
                 width: width,
@@ -141,24 +139,30 @@ extension _ImageCore {
     /// Mounts the async loading task for the current source (lifetime
     /// effect: recorded for the frame commit inside RenderLoop passes,
     /// immediate on the live path).
+    ///
+    /// The task is the only writer of `phaseBox` and `lastSourceBox`; it
+    /// commits them together once the load finishes, from outside the
+    /// traversal window. Loader, cache, and limits are read from the
+    /// render context's environment.
     private func mountLoadingTask(
         phaseBox: StateBox<ImageLoadingPhase>,
-        identity: ViewIdentity,
-        context: RenderContext,
-        lifecycle: LifecycleManager,
-        imageLoader: any ImageLoader,
-        imageCache: URLImageCache,
-        urlTimeout: TimeInterval,
-        maxPixelCount: Int?
+        lastSourceBox: StateBox<ImageSource?>,
+        context: RenderContext
     ) {
-        let taskIdentity = identity.scoped("image.load")
+        let lifecycle = context.environment.lifecycle!
+        let imageLoader = context.environment.imageLoader
+        let imageCache = context.environment.imageCache
+        let urlTimeout = context.environment.imageURLTimeout
+        let maxPixelCount = context.environment.imageMaxPixelCount
+        let taskIdentity = context.identity.scoped("image.load")
         let src = source
         let mount = { [imageLoader, imageCache] in
             _ = lifecycle.updateTask(
                 identity: taskIdentity,
                 id: src,
                 priority: .userInitiated
             ) {
+                let loadedPhase: ImageLoadingPhase
                 do {
                     let rawImage: RGBAImage
                     switch src {
@@ -174,14 +178,23 @@ extension _ImageCore {
                     }
 
                     // Store the raw image; conversion happens per render pass.
-                    // StateBox.didSet triggers setNeedsRender() automatically.
-                    // The runtime delivers that invalidation to its async event loop.
-                    phaseBox.value = .success(rawImage)
+                    loadedPhase = .success(rawImage)
                 } catch let loadError as ImageLoadError {
-                    phaseBox.value = .failure(loadError.description)
+                    loadedPhase = .failure(loadError.description)
                 } catch {
-                    phaseBox.value = .failure(error.localizedDescription)
+                    loadedPhase = .failure(error.localizedDescription)
                 }
+
+                // A cancelled task lost its slot to a newer source;
+                // committing its result would overwrite the replacement's.
+                guard !Task.isCancelled else { return }
+
+                // Phase first, then source: a render between the two writes
+                // still derives the placeholder instead of pairing the fresh
+                // phase with the outdated source. StateBox.didSet triggers
+                // setNeedsRender() for each write.
+                phaseBox.value = loadedPhase
+                lastSourceBox.value = src
             }
         }
         if let pendingEffects = context.environment.pendingFrameEffects {
@@ -190,20 +203,6 @@ extension _ImageCore {
             mount()
         }
     }
-
-    /// Resets the loading phase when the image source changes.
-    ///
-    /// Mutates state during rendering on purpose: the placeholder must show
-    /// in the SAME frame the new source appears, not one frame later.
-    private func resetLoadingPhaseIfNeeded(
-        phaseBox: StateBox<ImageLoadingPhase>,
-        lastSourceBox: StateBox<ImageSource?>
-    ) {
-        if let lastSource = lastSourceBox.value, lastSource != source {
-            phaseBox.value = .loading
-        }
-        lastSourceBox.value = source
-    }
 }
 
 // MARK: - Image Rendering

Tests/TUIkitTests/ImageRenderPhaseTests.swift

@@ -0,0 +1,131 @@
+//  🖥️ TUIKit — Terminal UI Kit for Swift
+//  ImageRenderPhaseTests.swift
+//
+//  License: MIT
+
+import Foundation
+import Testing
+
+@testable import TUIkit
+
+/// Specifies the render-phase contract for `_ImageCore` (issue #27 slice):
+/// rendering an image must never mutate state inside the traversal window.
+/// The placeholder for a changed source is DERIVED while rendering; the
+/// mounted loading task commits phase and last source from outside the
+/// window, so no frame ever emits a body-side-effect diagnostic.
+@MainActor
+@Suite("Image Render Phase", .serialized)
+struct ImageRenderPhaseTests {
+
+    @Test("Rendering an image emits no traversal diagnostics", .timeLimit(.minutes(1)))
+    func renderingEmitsNoTraversalDiagnostics() async throws {
+        let tuiContext = TUIContext(imageLoader: StubImageLoader())
+        let harness = FrameHarness(app: SwappableImageApp(), tuiContext: tuiContext)
+
+        harness.renderFrame()
+
+        #expect(tuiContext.runtimeDiagnostics.messages.isEmpty)
+
+        let phaseBox = try #require(imagePhaseBox(in: tuiContext))
+        while case .loading = phaseBox.value { await Task.yield() }
+        harness.renderFrame()
+
+        #expect(tuiContext.runtimeDiagnostics.messages.isEmpty)
+    }
+
+    @Test("Source change derives the placeholder without state writes", .timeLimit(.minutes(1)))
+    func sourceChangeDerivesPlaceholder() async throws {
+        let tuiContext = TUIContext(imageLoader: StubImageLoader())
+        let app = SwappableImageApp()
+        let harness = FrameHarness(app: app, tuiContext: tuiContext)
+
+        harness.renderFrame()
+        let phaseBox = try #require(imagePhaseBox(in: tuiContext))
+        while case .loading = phaseBox.value { await Task.yield() }
+        harness.renderFrame()
+
+        app.cell.source = .file("/stub/b.png")
+        let outputCountBeforeChange = harness.terminal.writtenOutput.count
+        harness.renderFrame()
+
+        // The placeholder appears in the SAME frame as the new source …
+        let changeFrameOutput = harness.terminal.writtenOutput[outputCountBeforeChange...].joined()
+        #expect(changeFrameOutput.contains(placeholderSpinnerGlyph))
+        // … and nothing mutated state during the traversal.
+        #expect(tuiContext.runtimeDiagnostics.messages.isEmpty)
+
+        // The replacement task commits phase and source after the frame.
+        let sourceBox = try #require(imageLastSourceBox(in: tuiContext))
+        while sourceBox.value != .file("/stub/b.png") { await Task.yield() }
+        harness.renderFrame()
+
+        #expect(tuiContext.runtimeDiagnostics.messages.isEmpty)
+    }
+}
+
+// MARK: - State Access
+
+/// The spinner glyph `_ImageCore.renderPlaceholder` draws while loading.
+private let placeholderSpinnerGlyph = "⠋"
+
+/// Returns the image view's phase box (`_ImageCore` property index 0).
+///
+/// The fixtures render exactly one image, so the single stored identity is
+/// the image's. Returns `nil` when no frame has hydrated state yet.
+@MainActor
+private func imagePhaseBox(in tuiContext: TUIContext) -> StateBox<ImageLoadingPhase>? {
+    guard let identity = tuiContext.stateStorage.storedIdentities.first else { return nil }
+    return tuiContext.stateStorage.storage(
+        for: StateStorage.StateKey(identity: identity, propertyIndex: 0),
+        default: .loading
+    )
+}
+
+/// Returns the image view's last-source box (`_ImageCore` property index 1).
+@MainActor
+private func imageLastSourceBox(in tuiContext: TUIContext) -> StateBox<ImageSource?>? {
+    guard let identity = tuiContext.stateStorage.storedIdentities.first else { return nil }
+    return tuiContext.stateStorage.storage(
+        for: StateStorage.StateKey(identity: identity, propertyIndex: 1),
+        default: nil
+    )
+}
+
+// MARK: - Fixtures
+
+/// Serves a deterministic single-pixel image for any request, keeping the
+/// file system and network out of the tests.
+private struct StubImageLoader: ImageLoader {
+    private let image = RGBAImage(
+        width: 1,
+        height: 1,
+        pixels: [RGBA(r: 255, g: 255, b: 255)]
+    )
+
+    func loadImage(from path: String) throws -> RGBAImage {
+        image
+    }
+
+    func loadImage(from data: Data) throws -> RGBAImage {
+        image
+    }
+}
+
+/// Mutable source slot shared between a test and its app fixture.
+@MainActor
+private final class ImageSourceCell {
+    var source: ImageSource = .file("/stub/a.png")
+}
+
+/// Renders one image whose source the test swaps between frames.
+private struct SwappableImageApp: App {
+    let cell = ImageSourceCell()
+
+    init() {}
+
+    var body: some Scene {
+        WindowGroup {
+            Image(cell.source)
+        }
+    }
+}

Tests/TUIkitTests/Support/FrameHarness.swift

@@ -15,6 +15,9 @@
 /// The default terminal is 40×24 with system status-bar items hidden, so a
 /// frame's content height is `24 − headerHeight` and height-gated fixtures
 /// can distinguish the measurement pass (full 24 rows) from output passes.
+///
+/// Tests that need deterministic services (stub image loaders, injected
+/// clocks, …) pass their own `TUIContext`; by default a fresh one is created.
 @MainActor
 final class FrameHarness<A: App> {
     let app: A
@@ -23,8 +26,7 @@ final class FrameHarness<A: App> {
 
     private let renderLoop: RenderLoop<A>
 
-    init(app: A, width: Int = 40, height: Int = 24) {
-        let tuiContext = TUIContext()
+    init(app: A, width: Int = 40, height: Int = 24, tuiContext: TUIContext = TUIContext()) {
         let terminal = MockTerminal()
         terminal.size = (width, height)
         tuiContext.statusBar.showSystemItems = false