From b5350d24147b80eb961e9e4dc2229e759badaca3 Mon Sep 17 00:00:00 2001 From: phranck Date: Tue, 21 Jul 2026 21:21:08 +0200 Subject: [PATCH 1/5] Test: Specify pass-collector adoption semantics - Pin ghost effects from discarded passes: measure-only header reaches live state, superseded-pass status-bar items and focusables persist, preference callbacks accumulate across passes - Pin focus-change ordering: auto-focus fires during traversal instead of after it - Extract the shared FrameHarness into test support - Add PreferenceStorage.callbackCount for tests and diagnostics - Mark desired behavior withKnownIssue until the collectors land (#56) --- .../Environment/PreferenceKey.swift | 5 + .../RenderPassCollectorTests.swift | 265 ++++++++++++++++++ .../RenderPhaseCharacterizationTests.swift | 42 --- Tests/TUIkitTests/Support/FrameHarness.swift | 50 ++++ 4 files changed, 320 insertions(+), 42 deletions(-) create mode 100644 Tests/TUIkitTests/RenderPassCollectorTests.swift create mode 100644 Tests/TUIkitTests/Support/FrameHarness.swift diff --git a/Sources/TUIkitCore/Environment/PreferenceKey.swift b/Sources/TUIkitCore/Environment/PreferenceKey.swift index 36c5e34d2..c0d3d49c6 100644 --- a/Sources/TUIkitCore/Environment/PreferenceKey.swift +++ b/Sources/TUIkitCore/Environment/PreferenceKey.swift @@ -174,6 +174,11 @@ public extension PreferenceStorage { callbacks[keyId]?.append(wrappedCallback) } + /// Number of registered change callbacks for tests and diagnostics. + var callbackCount: Int { + callbacks.values.reduce(0) { $0 + $1.count } + } + /// Prepares preference storage for a new render pass. /// /// Clears all accumulated callbacks and resets the value stack diff --git a/Tests/TUIkitTests/RenderPassCollectorTests.swift b/Tests/TUIkitTests/RenderPassCollectorTests.swift new file mode 100644 index 000000000..37d8bbf50 --- /dev/null +++ b/Tests/TUIkitTests/RenderPassCollectorTests.swift @@ -0,0 +1,265 @@ +// 🖥️ TUIKit — Terminal UI Kit for Swift +// RenderPassCollectorTests.swift +// +// License: MIT + +import Testing +import TUIkitTestSupport + +@testable import TUIkit + +/// Specifies the pass-collector adoption semantics of issue #56: effects +/// that do not outlive the frame (key handlers, preference callbacks, +/// status-bar items, header buffer, focus registrations) must reach live +/// runtime state exclusively from the frame's FINAL pass. Discarded passes +/// (first-frame measurement, superseded main pass before a header +/// correction) leave no trace. +/// +/// Tests assert the DESIRED behavior and carry `withKnownIssue` markers +/// until the collectors land (#56 Tasks 2-4); then the markers drop. +@MainActor +@Suite("Render Pass Collectors", .serialized) +struct RenderPassCollectorTests { + + @Test("A header declared only in the measure pass never reaches live state") + func abandonedHeaderDoesNotReachLiveState() { + let harness = FrameHarness(app: MeasureOnlyHeaderApp()) + + harness.renderFrame() + + withKnownIssue("Issue #56: the measure pass writes the live header buffer") { + #expect(harness.tuiContext.appHeader.hasContent == false) + #expect(harness.tuiContext.appHeader.height == 0) + } matching: { issue in + guard case .expectationFailed = issue.kind else { return false } + return true + } + } + + @Test("Preference callbacks do not accumulate across passes of one frame") + func preferenceCallbacksDoNotAccumulateAcrossPasses() { + let harness = FrameHarness(app: CorrectionPreferenceApp()) + + // Frame 1 establishes the header height estimate; growing the header + // forces frame 2 through the correction pass. + harness.renderFrame() + harness.app.model.lineCount = 3 + harness.renderFrame() + + withKnownIssue("Issue #56: each pass appends its callbacks to the live storage") { + #expect(harness.tuiContext.preferences.callbackCount == 1) + } matching: { issue in + guard case .expectationFailed = issue.kind else { return false } + return true + } + } + + @Test("Status-bar items from a superseded pass do not persist") + func statusBarItemsFromSupersededPassDoNotPersist() { + let harness = FrameHarness(app: CorrectionGatedStatusBarApp()) + + harness.renderFrame() + harness.app.model.lineCount = 3 + harness.renderFrame() + + // The gated declaration exists at content height 22 (main pass of + // frame 2) but not at 20 (correction pass) — the final tree has no + // user items. + withKnownIssue("Issue #56: the superseded main pass writes live status-bar items") { + #expect(harness.tuiContext.statusBar.hasUserItems == false) + } matching: { issue in + guard case .expectationFailed = issue.kind else { return false } + return true + } + } + + @Test("Focus registrations come from the final pass only") + func focusRegistrationsComeFromFinalPass() { + let harness = FrameHarness(app: CorrectionGatedButtonApp()) + + harness.renderFrame() + harness.app.model.lineCount = 3 + harness.renderFrame() + + // The button exists in the superseded main pass (height 22) but not + // in the corrected final tree (height 20): nothing may stay focused. + withKnownIssue("Issue #56: focusables from the superseded pass stay registered") { + #expect(harness.tuiContext.focusManager.currentFocusedID == nil) + } matching: { issue in + guard case .expectationFailed = issue.kind else { return false } + return true + } + } + + @Test("Focus changes fire after traversal, not during it") + func focusChangeFiresOnlyAtCommit() { + let harness = FrameHarness(app: TraceOrderingApp()) + let trace = harness.app.trace + let previousHandler = harness.tuiContext.focusManager.onFocusChange + harness.tuiContext.focusManager.onFocusChange = { + previousHandler?() + trace.record("focusChange") + } + + harness.renderFrame() + + let events = trace.snapshot() + let lastRenderIndex = events.lastIndex { $0.hasPrefix("render:") } + let firstFocusChangeIndex = events.firstIndex(of: "focusChange") + + withKnownIssue("Issue #56: auto-focus fires onFocusChange during traversal") { + if let lastRenderIndex, let firstFocusChangeIndex { + #expect(lastRenderIndex < firstFocusChangeIndex) + } + } matching: { issue in + guard case .expectationFailed = issue.kind else { return false } + return true + } + } +} + +// MARK: - Shared Fixtures + +/// Mutable header size shared between the test and the app fixtures. +@MainActor +final class GrowableHeaderModel { + var lineCount = 1 +} + +/// Renders `content` only when the available height is at least `threshold`. +/// +/// Records nothing and has no effects of its own; used to make a subtree +/// exist in one pass of a frame but not in another (measure vs. main, or +/// main vs. correction). +private struct HeightGate: View, Renderable { + let threshold: Int + let content: Content + + var body: Never { + fatalError("HeightGate renders via Renderable") + } + + func renderToBuffer(context: RenderContext) -> FrameBuffer { + guard context.availableHeight >= threshold else { + return FrameBuffer(text: "below-gate") + } + return TUIkit.renderToBuffer(content, context: context) + } +} + +/// Grows the app header via `GrowableHeaderModel` so frame 2 runs through +/// the header-correction pass (estimate 2, actual 4 → content 22 → 20). +private struct GrowingHeader: View { + let model: GrowableHeaderModel + + var body: some View { + VStack { + ForEach(Array(0..() + + init() {} + + var body: some Scene { + WindowGroup { + VStack { + TraceLeaf(label: "first", trace: trace) + Button("Focus me") {} + TraceLeaf(label: "last", trace: trace) + } + } + } +} + +/// Leaf that records each output-phase render; measurement traversals are +/// intentionally silent so layout probing does not pollute the trace. +private struct TraceLeaf: View, Renderable { + let label: String + let trace: TraceRecorder + + var body: Never { + fatalError("TraceLeaf renders via Renderable") + } + + func renderToBuffer(context: RenderContext) -> FrameBuffer { + if context.phase == .render { + trace.record("render:\(label)") + } + return FrameBuffer(text: label) + } +} diff --git a/Tests/TUIkitTests/RenderPhaseCharacterizationTests.swift b/Tests/TUIkitTests/RenderPhaseCharacterizationTests.swift index 6debc6639..4a6bf98f0 100644 --- a/Tests/TUIkitTests/RenderPhaseCharacterizationTests.swift +++ b/Tests/TUIkitTests/RenderPhaseCharacterizationTests.swift @@ -74,48 +74,6 @@ struct RenderPhaseCharacterizationTests { } } -// MARK: - Frame Harness - -/// Drives a `RenderLoop` against a `MockTerminal` for one app instance. -/// -/// Unlike `RuntimeCharacterizationHarness` (which renders single views via -/// `renderToBuffer`), this harness exercises the full frame pipeline — -/// including the first-frame header measurement and the header-correction -/// pass — which is exactly where phase separation matters. -@MainActor -private final class FrameHarness { - let app: A - let tuiContext: TUIContext - let terminal: MockTerminal - - private let renderLoop: RenderLoop - - init(app: A, width: Int = 40, height: Int = 24) { - let tuiContext = TUIContext() - let terminal = MockTerminal() - terminal.size = (width, height) - tuiContext.statusBar.showSystemItems = false - - self.app = app - self.tuiContext = tuiContext - self.terminal = terminal - self.renderLoop = RenderLoop( - app: app, - terminal: terminal, - statusBar: tuiContext.statusBar, - appHeader: tuiContext.appHeader, - focusManager: tuiContext.focusManager, - paletteManager: tuiContext.paletteManager, - appearanceManager: tuiContext.appearanceManager, - tuiContext: tuiContext - ) - } - - func renderFrame() { - renderLoop.render() - } -} - // MARK: - Measure-Only Gating /// Content height in the main pass: 24 (terminal) − 0 (status bar hidden) diff --git a/Tests/TUIkitTests/Support/FrameHarness.swift b/Tests/TUIkitTests/Support/FrameHarness.swift new file mode 100644 index 000000000..c683fe847 --- /dev/null +++ b/Tests/TUIkitTests/Support/FrameHarness.swift @@ -0,0 +1,50 @@ +// 🖥️ TUIKit — Terminal UI Kit for Swift +// FrameHarness.swift +// +// License: MIT + +@testable import TUIkit + +/// Drives a `RenderLoop` against a `MockTerminal` for one app instance. +/// +/// Unlike `RuntimeCharacterizationHarness` (which renders single views via +/// `renderToBuffer`), this harness exercises the full frame pipeline — +/// including the first-frame header measurement and the header-correction +/// pass — which is exactly where phase separation matters. +/// +/// 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. +@MainActor +final class FrameHarness { + let app: A + let tuiContext: TUIContext + let terminal: MockTerminal + + private let renderLoop: RenderLoop + + init(app: A, width: Int = 40, height: Int = 24) { + let tuiContext = TUIContext() + let terminal = MockTerminal() + terminal.size = (width, height) + tuiContext.statusBar.showSystemItems = false + + self.app = app + self.tuiContext = tuiContext + self.terminal = terminal + self.renderLoop = RenderLoop( + app: app, + terminal: terminal, + statusBar: tuiContext.statusBar, + appHeader: tuiContext.appHeader, + focusManager: tuiContext.focusManager, + paletteManager: tuiContext.paletteManager, + appearanceManager: tuiContext.appearanceManager, + tuiContext: tuiContext + ) + } + + func renderFrame() { + renderLoop.render() + } +} From f7baf7905720bc1d37f39cfc4bd484baa26f1658 Mon Sep 17 00:00:00 2001 From: phranck Date: Tue, 21 Jul 2026 21:24:20 +0200 Subject: [PATCH 2/5] Feat: Add per-pass effect collectors - Introduce RenderPassCollectors bundling fresh scratch instances per traversal - Add adopt(from:) to KeyEventDispatcher, PreferenceStorage, and AppHeaderState - Record declarative status-bar registrations per pass and replay only the final pass via adoptPassRegistrations(from:) - Document the collector half of the effect classification rule on every adopt API --- Sources/TUIkit/App/RenderPassCollectors.swift | 87 +++++++++++++++++++ Sources/TUIkit/AppHeader/AppHeaderState.swift | 18 ++++ Sources/TUIkit/Focus/KeyEvent.swift | 14 +++ Sources/TUIkit/StatusBar/StatusBarState.swift | 60 +++++++++++++ .../Environment/PreferenceKey.swift | 15 ++++ 5 files changed, 194 insertions(+) create mode 100644 Sources/TUIkit/App/RenderPassCollectors.swift diff --git a/Sources/TUIkit/App/RenderPassCollectors.swift b/Sources/TUIkit/App/RenderPassCollectors.swift new file mode 100644 index 000000000..f8ba73cbc --- /dev/null +++ b/Sources/TUIkit/App/RenderPassCollectors.swift @@ -0,0 +1,87 @@ +// 🖥️ TUIKit — Terminal UI Kit for Swift +// RenderPassCollectors.swift +// +// Created by LAYERED.work +// License: MIT + +// MARK: - Render Pass Collectors + +/// Per-pass scratch sinks for effects that do **not** outlive the frame. +/// +/// A frame may traverse the view tree several times (first-frame header +/// measurement, main pass, header-correction pass), but only ONE traversal +/// produces the frame's output. Effect sites must therefore never write +/// per-frame registrations directly into the live managers — a discarded +/// pass would leave ghost handlers, items, or buffers behind. +/// +/// `RenderPassCollectors` solves this with the collector half of the effect +/// classification rule (see ``RenderPhase``): +/// +/// > Does the effect outlive the frame? **No → pass collector (this type).** +/// > Yes → pending diff (issue #57). +/// +/// ## How it works +/// +/// `RenderLoop` creates one `RenderPassCollectors` per traversal and injects +/// its members into that pass's environment. Effect sites stay completely +/// unchanged — they keep writing to `context.environment.keyEventDispatcher` +/// and friends; the environment simply hands them the scratch instance of +/// the current pass instead of the live manager. +/// +/// At frame commit, `RenderLoop` adopts the FINAL pass's collectors into the +/// live managers (`adopt(from:)` on each manager type). Collectors of +/// discarded passes are dropped without further ceremony: because nothing +/// live was ever touched, discarding requires no rollback. +/// +/// ## What is collected here +/// +/// - Key handlers (`KeyEventDispatcher`) +/// - Preference values and change callbacks (`PreferenceStorage`) +/// - Declarative status-bar registrations (`StatusBarState`) +/// - The app-header buffer (`AppHeaderState`) — the first-frame measurement +/// pass reads the header height from its scratch instance, which is then +/// discarded, so sizing never mutates live state +/// +/// Focus registrations are the deliberate exception: focus queries +/// (`isFocused`) must read live state during traversal, so `FocusManager` +/// stages registrations internally instead of using a scratch instance +/// (see `FocusManager.beginPass()`). +@MainActor +struct RenderPassCollectors { + /// Scratch sink for key handlers registered by this pass's tree. + let keyEventDispatcher = KeyEventDispatcher() + + /// Scratch sink for preference values and change callbacks. + let preferences = PreferenceStorage() + + /// Scratch sink for declarative status-bar registrations. + let statusBar: StatusBarState + + /// Scratch sink for the app-header buffer. + let appHeader = AppHeaderState() + + /// Creates fresh collectors for one render pass. + /// + /// - Parameter appState: The runtime's render-state sink; required by + /// `StatusBarState` for its (unsupported but non-crashing) imperative + /// re-render paths. + init(appState: AppState) { + self.statusBar = StatusBarState(appState: appState) + } + + /// Adopts this pass's collected registrations into the live managers. + /// + /// This is commit step "6a" of the frame choreography (see `RenderLoop`): + /// the single point where per-frame effect state reaches the live + /// runtime, called exactly once per frame with the FINAL pass's + /// collectors. + /// + /// - Parameter tuiContext: The runtime whose live managers adopt the + /// collected state. + func adoptIntoLiveManagers(of tuiContext: TUIContext) { + tuiContext.keyEventDispatcher.adopt(from: keyEventDispatcher) + tuiContext.preferences.adopt(from: preferences) + tuiContext.statusBar.adoptPassRegistrations(from: statusBar) + tuiContext.appHeader.adopt(from: appHeader) + } +} diff --git a/Sources/TUIkit/AppHeader/AppHeaderState.swift b/Sources/TUIkit/AppHeader/AppHeaderState.swift index 7566bc7cb..0988bb808 100644 --- a/Sources/TUIkit/AppHeader/AppHeaderState.swift +++ b/Sources/TUIkit/AppHeader/AppHeaderState.swift @@ -84,6 +84,24 @@ extension AppHeaderState { previousHeight = height contentBuffer = nil } + + /// Replaces the content buffer with the one collected by a per-pass + /// scratch instance. + /// + /// The header buffer does not outlive the frame: `AppHeaderModifier` + /// re-renders it on every traversal. At frame commit the live state + /// therefore adopts the FINAL pass's buffer (which may be `nil` when the + /// final tree declares no header); buffers written by discarded passes — + /// including the first-frame measurement pass, whose buffer only exists + /// to size the header — are dropped with their collector. + /// + /// The ``estimatedHeight`` bookkeeping stays on the live instance, so + /// the estimate always reflects the last COMMITTED frame. + /// + /// - Parameter collector: The scratch state of the frame's final pass. + func adopt(from collector: AppHeaderState) { + contentBuffer = collector.contentBuffer + } } // MARK: - Environment Key diff --git a/Sources/TUIkit/Focus/KeyEvent.swift b/Sources/TUIkit/Focus/KeyEvent.swift index 0b44ee1a4..32af0525a 100644 --- a/Sources/TUIkit/Focus/KeyEvent.swift +++ b/Sources/TUIkit/Focus/KeyEvent.swift @@ -40,6 +40,20 @@ extension KeyEventDispatcher { handlers.count } + /// Replaces all handlers with the ones collected by a per-pass scratch + /// dispatcher. + /// + /// Key handlers do not outlive the frame: views re-register them on + /// every traversal. At frame commit the live dispatcher therefore adopts + /// the FINAL pass's handler list wholesale; handler lists from discarded + /// passes (measurement, superseded main pass) are simply dropped with + /// their collector. + /// + /// - Parameter collector: The scratch dispatcher of the frame's final pass. + func adopt(from collector: KeyEventDispatcher) { + handlers = collector.handlers + } + /// Dispatches a key event to handlers. /// /// - Parameter event: The key event to dispatch. diff --git a/Sources/TUIkit/StatusBar/StatusBarState.swift b/Sources/TUIkit/StatusBar/StatusBarState.swift index 647e9d633..64d082f9f 100644 --- a/Sources/TUIkit/StatusBar/StatusBarState.swift +++ b/Sources/TUIkit/StatusBar/StatusBarState.swift @@ -60,6 +60,27 @@ public final class StatusBarState: @unchecked Sendable { /// Rebuilt every render pass by ``StatusBarItemsModifier``. private var sectionItems: [(sectionID: String, items: [any StatusBarItemProtocol], composition: StatusBarItemComposition)] = [] + /// One declarative status-bar registration made by a view during a + /// render pass. + /// + /// Status-bar declarations do not outlive the frame: modifiers re-issue + /// them on every traversal. Each silent registration method records what + /// the pass declared so that ``adoptPassRegistrations(from:)`` can + /// replay exactly the FINAL pass's declarations onto the live state. + enum PassRegistration { + /// `.statusBarItems { … }` without a focus section (global items). + case globalItems([any StatusBarItemProtocol]) + + /// Legacy `.statusBarItems(context:…)` push. + case pushContext(String, [any StatusBarItemProtocol]) + + /// `.statusBarItems { … }` inside a focus section. + case sectionItems(String, [any StatusBarItemProtocol], StatusBarItemComposition) + } + + /// Declarative registrations collected during the current render pass. + private(set) var passRegistrations: [PassRegistration] = [] + /// The focus manager used to determine the active section. /// /// Set by `RenderLoop` at the start of each render pass. @@ -274,6 +295,7 @@ public extension StatusBarState { extension StatusBarState { /// Sets the global user items without triggering a re-render. func setItemsSilently(_ items: [any StatusBarItemProtocol]) { + passRegistrations.append(.globalItems(items)) userGlobalItems = items } @@ -283,6 +305,7 @@ extension StatusBarState { items: [any StatusBarItemProtocol], composition: StatusBarItemComposition ) { + passRegistrations.append(.sectionItems(sectionID, items, composition)) sectionItems.removeAll { $0.sectionID == sectionID } sectionItems.append((sectionID, items, composition)) } @@ -290,13 +313,50 @@ extension StatusBarState { /// Clears all section items at the start of a render pass. func clearSectionItems() { sectionItems.removeAll() + passRegistrations.removeAll() } /// Pushes a new user context without triggering a re-render. func pushSilently(context: String, items: [any StatusBarItemProtocol]) { + passRegistrations.append(.pushContext(context, items)) userContextStack.removeAll { $0.context == context } userContextStack.append((context, items)) } + + /// Replays the declarative registrations of a per-pass scratch instance + /// onto this (live) state. + /// + /// Declarative state is replaced wholesale by the FINAL pass: + /// - Section items become exactly the final pass's section declarations. + /// - Global user items become the final pass's `.statusBarItems` + /// declaration, or empty when the final tree declares none. Items set + /// imperatively via ``setItems(_:)-1qhcp`` are therefore superseded by + /// the next committed frame — the tree owns the status bar. + /// - Legacy push contexts are re-applied with push semantics; contexts + /// removed only via ``pop(context:)`` keep their imperative lifecycle. + /// + /// Registrations from discarded passes (measurement, superseded main + /// pass) are dropped with their collector and can never reach this + /// state. + /// + /// - Parameter collector: The scratch state of the frame's final pass. + func adoptPassRegistrations(from collector: StatusBarState) { + sectionItems.removeAll() + userGlobalItems = [] + + for registration in collector.passRegistrations { + switch registration { + case .globalItems(let items): + userGlobalItems = items + case .pushContext(let name, let items): + userContextStack.removeAll { $0.context == name } + userContextStack.append((name, items)) + case .sectionItems(let sectionID, let items, let composition): + sectionItems.removeAll { $0.sectionID == sectionID } + sectionItems.append((sectionID, items, composition)) + } + } + } } // MARK: - Private Helpers diff --git a/Sources/TUIkitCore/Environment/PreferenceKey.swift b/Sources/TUIkitCore/Environment/PreferenceKey.swift index c0d3d49c6..8eae9a70e 100644 --- a/Sources/TUIkitCore/Environment/PreferenceKey.swift +++ b/Sources/TUIkitCore/Environment/PreferenceKey.swift @@ -179,6 +179,21 @@ public extension PreferenceStorage { callbacks.values.reduce(0) { $0 + $1.count } } + /// Replaces the collected values and change callbacks with the ones from + /// a per-pass scratch storage. + /// + /// Preference values and their change callbacks do not outlive the + /// frame: the tree re-declares them on every traversal. At frame commit + /// the live storage therefore adopts the FINAL pass's state wholesale; + /// storages of discarded passes are dropped, so their callbacks can + /// never accumulate on the live instance. + /// + /// - Parameter collector: The scratch storage of the frame's final pass. + func adopt(from collector: PreferenceStorage) { + stack = collector.stack + callbacks = collector.callbacks + } + /// Prepares preference storage for a new render pass. /// /// Clears all accumulated callbacks and resets the value stack From 857fb9357053ae12f657508a6923f793b0b01c93 Mon Sep 17 00:00:00 2001 From: phranck Date: Tue, 21 Jul 2026 21:27:46 +0200 Subject: [PATCH 3/5] Refactor: Stage focus registrations per pass - Route register/registerSection/activateSection into a staging area between beginPass and commitPass/discardPass - Defer auto-focus, section activation, and onFocusChange to commitPass so discarded passes leave no trace - Keep live semantics for imperative use outside a collecting pass (ViewRenderer, tests) - Extract the shared post-commit validation from endRenderPass --- Sources/TUIkit/Focus/Focus.swift | 169 +++++++++++++++++++++++++++---- 1 file changed, 149 insertions(+), 20 deletions(-) diff --git a/Sources/TUIkit/Focus/Focus.swift b/Sources/TUIkit/Focus/Focus.swift index fd9d5b37e..76d12961d 100644 --- a/Sources/TUIkit/Focus/Focus.swift +++ b/Sources/TUIkit/Focus/Focus.swift @@ -60,6 +60,33 @@ public final class FocusManager: @unchecked Sendable { /// Callback triggered when focus changes (element or section). public var onFocusChange: (() -> Void)? + // MARK: Per-Pass Staging + // + // Focus registrations do not outlive the frame, but unlike other + // per-frame effects they cannot use a scratch manager instance: focus + // QUERIES (`isFocused`, `isActiveSection`) must keep reading the live + // state of the last committed frame while the tree renders. The manager + // therefore stages registrations internally: between `beginPass()` and + // `commitPass()`/`discardPass()` all registrations and section + // activations go into the staging area, and only `commitPass()` replaces + // the committed sections and runs focus side effects (auto-focus, + // `onFocusReceived`, `onFocusChange`). A discarded pass leaves no trace. + + /// Sections registered by the current render pass, not yet committed. + private var stagedSections: [FocusSection] = [] + + /// Section activation requested by the current pass (e.g. by an alert), + /// applied at commit. The last request of the final pass wins. + private var stagedActivationID: String? + + /// Whether registrations are currently routed into the staging area. + /// + /// `true` between ``beginPass()`` and ``commitPass()`` / + /// ``discardPass()``. Outside a collecting pass (imperative use, + /// `ViewRenderer`, tests) registrations keep their immediate live + /// semantics. + private var isCollectingPass = false + /// Creates a new focus manager instance. public init() {} @@ -131,6 +158,16 @@ public extension FocusManager { func register(_ element: Focusable, inSection sectionID: String? = nil) { let targetID = sectionID ?? activeSectionID ?? Self.defaultSectionID + // During a collecting pass, registrations only reach the staging + // area. Focus side effects (auto-activation, auto-focus, + // onFocusChange) are deferred to commitPass() so a discarded pass + // can never mutate live focus state. + if isCollectingPass { + let section = stagedSection(id: targetID) + section.register(element) + return + } + // Ensure section exists if !sections.contains(where: { $0.id == targetID }) { registerSection(id: targetID) @@ -179,6 +216,9 @@ public extension FocusManager { /// section and focused element, use `beginRenderPass()` instead. func clear() { sections.removeAll() + stagedSections.removeAll() + stagedActivationID = nil + isCollectingPass = false activeSectionID = nil focusedID = nil } @@ -337,6 +377,13 @@ extension FocusManager { /// /// - Parameter id: The unique section identifier. func registerSection(id: String) { + // Collecting pass: sections are staged; activation is deferred to + // commitPass() so a discarded pass leaves no trace. + if isCollectingPass { + _ = stagedSection(id: id) + return + } + guard !sections.contains(where: { $0.id == id }) else { return } let section = FocusSection(id: id) sections.append(section) @@ -375,8 +422,17 @@ extension FocusManager { /// /// The first focusable element in the section receives focus. /// + /// During a collecting pass (e.g. an alert or modal activating its own + /// section while the tree renders) the activation is staged and applied + /// at ``commitPass()``; a discarded pass never changes the live section. + /// /// - Parameter id: The section identifier to activate. func activateSection(id: String) { + if isCollectingPass { + stagedActivationID = id + return + } + guard sections.contains(where: { $0.id == id }) else { return } // Notify current focused element @@ -417,28 +473,57 @@ extension FocusManager { /// registered section is activated. If the previously focused element /// no longer exists, the first focusable in the active section is focused. func endRenderPass() { - // Validate active section - if let activeID = activeSectionID, - !sections.contains(where: { $0.id == activeID }) - { - activeSectionID = sections.first?.id + validateCommittedFocus() + } + + // MARK: - Per-Pass Staging API + + /// Starts collecting focus registrations for one render pass. + /// + /// Until ``commitPass()`` or ``discardPass()``, all registrations and + /// section activations go into the staging area while queries keep + /// reading the last committed state. `RenderLoop` calls this before + /// every traversal (measurement, main, correction). + func beginPass() { + stagedSections.removeAll() + stagedActivationID = nil + isCollectingPass = true + } + + /// Drops everything the current pass registered. + /// + /// Called for traversals whose tree never becomes the frame's output + /// (the measurement pass, or a main pass superseded by a header + /// correction). Because nothing live was touched, discarding is free. + func discardPass() { + stagedSections.removeAll() + stagedActivationID = nil + isCollectingPass = false + } + + /// Replaces the committed sections with the staged ones and runs the + /// deferred focus side effects. + /// + /// This is the focus part of the frame commit: the FINAL pass's + /// registrations become the live sections, a staged section activation + /// (alert/modal) is applied, and the auto-focus validation runs — + /// firing `onFocusReceived`/`onFocusChange` at most once per frame, + /// never during traversal. + func commitPass() { + sections = stagedSections + stagedSections = [] + isCollectingPass = false + + if let requestedID = stagedActivationID { + stagedActivationID = nil + activateSection(id: requestedID) + return } - // Validate focused element - if let focusID = focusedID, let section = activeSection { - if !section.focusables.contains(where: { $0.focusID == focusID }) { - // Previously focused element is gone — auto-focus first available - self.focusedID = nil - if let firstFocusable = section.focusables.first(where: { $0.canBeFocused }) { - self.focusedID = firstFocusable.focusID - firstFocusable.onFocusReceived() - } - } - } else if focusedID == nil, let section = activeSection, - let firstFocusable = section.focusables.first(where: { $0.canBeFocused }) - { - self.focusedID = firstFocusable.focusID - firstFocusable.onFocusReceived() + let previousFocusedID = focusedID + validateCommittedFocus() + if focusedID != previousFocusedID { + onFocusChange?() } } } @@ -527,6 +612,50 @@ private extension FocusManager { } } } + + /// Returns the staged section with the given ID, creating it if needed. + func stagedSection(id: String) -> FocusSection { + if let existing = stagedSections.first(where: { $0.id == id }) { + return existing + } + let section = FocusSection(id: id) + stagedSections.append(section) + return section + } + + /// Repairs the committed focus state after sections were replaced. + /// + /// Shared by ``endRenderPass()`` (live path: `ViewRenderer`, tests) and + /// ``commitPass()``: reactivates a surviving section when the active one + /// disappeared, and auto-focuses the first available element when the + /// focused one is gone. `onFocusChange` is intentionally NOT fired here; + /// `commitPass()` detects focus changes itself so the live path keeps + /// its historical semantics. + func validateCommittedFocus() { + // Validate active section + if let activeID = activeSectionID, + !sections.contains(where: { $0.id == activeID }) + { + activeSectionID = sections.first?.id + } + + // Validate focused element + if let focusID = focusedID, let section = activeSection { + if !section.focusables.contains(where: { $0.focusID == focusID }) { + // Previously focused element is gone — auto-focus first available + self.focusedID = nil + if let firstFocusable = section.focusables.first(where: { $0.canBeFocused }) { + self.focusedID = firstFocusable.focusID + firstFocusable.onFocusReceived() + } + } + } else if focusedID == nil, let section = activeSection, + let firstFocusable = section.focusables.first(where: { $0.canBeFocused }) + { + self.focusedID = firstFocusable.focusID + firstFocusable.onFocusReceived() + } + } } // MARK: - Focus Manager Environment Key From 21699e84f33012432a5ad8f04a52b2434d66ac67 Mon Sep 17 00:00:00 2001 From: phranck Date: Tue, 21 Jul 2026 21:34:05 +0200 Subject: [PATCH 4/5] Fix: Adopt only the final pass into live managers - Render every pass into fresh RenderPassCollectors via a per-pass environment; live managers adopt the final pass atomically before writeFrame - Discard the measurement pass and superseded main pass together with their collectors and staged focus registrations - Read header heights from the pass collectors instead of live state - Repair two uncovered focus validation holes: activate the first committed section when none is active, and clear focus when no sections survive - Promote the collector and correction-pass tests to hard expectations; align the runtime-isolation test with declarative focus semantics --- Sources/TUIkit/App/RenderLoop.swift | 90 ++++++++++++++----- Sources/TUIkit/Focus/Focus.swift | 12 +++ .../RenderPassCollectorTests.swift | 67 ++++++-------- .../RenderPhaseCharacterizationTests.swift | 7 +- Tests/TUIkitTests/ViewRendererTests.swift | 9 +- 5 files changed, 118 insertions(+), 67 deletions(-) diff --git a/Sources/TUIkit/App/RenderLoop.swift b/Sources/TUIkit/App/RenderLoop.swift index 28a7c045e..c1402a2df 100644 --- a/Sources/TUIkit/App/RenderLoop.swift +++ b/Sources/TUIkit/App/RenderLoop.swift @@ -79,22 +79,29 @@ internal struct RenderBackgroundCodes: Equatable { /// 12. End lifecycle tracking (fires onDisappear for removed views) /// ``` /// -/// ## Render phases +/// ## Render phases and per-pass collectors /// /// A frame can traverse the view tree up to three times. Each traversal -/// carries a ``RenderPhase`` on its `RenderContext`: +/// carries a ``RenderPhase`` on its `RenderContext` and writes its +/// frame-scoped effects into fresh ``RenderPassCollectors``: /// /// - **First-frame header sizing** runs in `.measure`: it only discovers the -/// app header height, so phase-guarded effect sites (lifecycle, task, -/// focus registration) stay inert. `AppHeaderModifier` intentionally -/// remains unguarded — the header buffer IS the measured output. +/// app header height. Phase-guarded effect sites (lifecycle, task, focus +/// registration) stay inert, and the header buffer it renders lives in a +/// scratch collector that is dropped right after the height is read. /// - **Main pass** and, when the header height changed, the **correction /// pass** run in `.render` and produce the frame's candidate buffers. +/// A superseded main pass is discarded together with its collectors and +/// staged focus registrations. +/// - **Commit**: the FINAL pass's collectors are adopted into the live +/// managers (key handlers, preferences, status bar, header) and the +/// staged focus registrations are committed — the only point in a frame +/// where per-frame effect state reaches the live runtime. /// -/// Not yet guaranteed (tracked by issues #56/#57): the correction pass still -/// re-registers per-frame effects (e.g. key handlers) on top of the main -/// pass, and lifetime effects still apply during traversal instead of at a -/// single commit point. See ``RenderPhase`` for the target model. +/// Not yet guaranteed (tracked by issue #57): lifetime effects (`onAppear`, +/// `.task`, `onChange`, `onPreferenceChange` actions, GC liveness) still +/// apply during traversal instead of at the commit point. See +/// ``RenderPhase`` for the target model. /// /// ## Diff-Based Rendering /// @@ -225,20 +232,21 @@ extension RenderLoop { // This prevents visible content jumping. let appHeaderHeight: Int if isFirstFrame { + // This traversal only exists to size the app header — it runs in + // the measure phase (guarded effect sites stay inert) and writes + // into its own collectors, which are dropped right after the + // height is read. Live state is never touched. + let measureCollectors = RenderPassCollectors(appState: tuiContext.appState) var measureContext = RenderContext( availableWidth: terminalWidth, availableHeight: terminalHeight - statusBarHeight, - environment: environment + environment: passEnvironment(environment, collectors: measureCollectors) ) - // This traversal only exists to size the app header — run it in - // the measure phase so guarded effect sites (lifecycle, task, - // focus) stay inert. Deliberate exception: AppHeaderModifier is - // NOT phase-guarded, because writing the header buffer is the - // very output this pass measures. Isolating that write into a - // discardable per-pass collector is issue #56. measureContext.phase = .measure + focusManager.beginPass() _ = renderScene(scene, context: measureContext.withChildIdentity(type: type(of: scene))) - appHeaderHeight = appHeader.height + focusManager.discardPass() + appHeaderHeight = measureCollectors.appHeader.height isFirstFrame = false } else { appHeaderHeight = appHeader.estimatedHeight @@ -246,33 +254,47 @@ extension RenderLoop { let contentHeight = terminalHeight - statusBarHeight - appHeaderHeight + // Main pass: evaluate the frame's candidate tree into fresh + // collectors. Nothing reaches live state until the commit below. + var collectors = RenderPassCollectors(appState: tuiContext.appState) var context = RenderContext( availableWidth: terminalWidth, availableHeight: contentHeight, - environment: environment + environment: passEnvironment(environment, collectors: collectors) ) context.hasExplicitWidth = true context.hasExplicitHeight = true + focusManager.beginPass() var buffer = renderScene(scene, context: context.withChildIdentity(type: type(of: scene))) // If the header height changed after rendering, re-render with the - // correct height so centering is accurate. - let actualHeaderHeight = appHeader.height + // correct height so centering is accurate. The superseded main + // pass's collectors and staged focus registrations are discarded; + // the correction pass starts from clean scratch state. + let actualHeaderHeight = collectors.appHeader.height if actualHeaderHeight != appHeaderHeight { diffWriter.invalidate() + focusManager.discardPass() + collectors = RenderPassCollectors(appState: tuiContext.appState) let actualContentHeight = terminalHeight - statusBarHeight - actualHeaderHeight var correctedContext = RenderContext( availableWidth: terminalWidth, availableHeight: actualContentHeight, - environment: environment + environment: passEnvironment(environment, collectors: collectors) ) correctedContext.hasExplicitWidth = true correctedContext.hasExplicitHeight = true + focusManager.beginPass() buffer = renderScene(scene, context: correctedContext.withChildIdentity(type: type(of: scene))) } - focusManager.endRenderPass() + // COMMIT (step 6a): the single point where per-frame effect state + // reaches the live runtime — the FINAL pass's collectors replace the + // live managers' state, and the staged focus registrations are + // committed with their deferred side effects. + collectors.adoptIntoLiveManagers(of: tuiContext) + focusManager.commitPass() writeFrame( buffer: buffer, @@ -299,6 +321,30 @@ extension RenderLoop { func buildEnvironment() -> EnvironmentValues { tuiContext.environmentValues() } + + /// Derives a per-pass environment that routes frame-scoped effect sinks + /// into the pass's scratch collectors. + /// + /// Effect sites keep writing to `context.environment.keyEventDispatcher` + /// and friends; only the instance behind those keys changes per pass. + /// All other services (state storage, lifecycle, focus queries, palette, + /// …) stay on the live runtime. + /// + /// - Parameters: + /// - base: The frame's live environment. + /// - collectors: The scratch collectors of the current pass. + /// - Returns: The environment to render this pass with. + private func passEnvironment( + _ base: EnvironmentValues, + collectors: RenderPassCollectors + ) -> EnvironmentValues { + var environment = base + environment.keyEventDispatcher = collectors.keyEventDispatcher + environment.preferenceStorage = collectors.preferences + environment.statusBar = collectors.statusBar + environment.appHeader = collectors.appHeader + return environment + } } // MARK: - Private Helpers diff --git a/Sources/TUIkit/Focus/Focus.swift b/Sources/TUIkit/Focus/Focus.swift index 76d12961d..369310006 100644 --- a/Sources/TUIkit/Focus/Focus.swift +++ b/Sources/TUIkit/Focus/Focus.swift @@ -632,6 +632,13 @@ private extension FocusManager { /// `commitPass()` detects focus changes itself so the live path keeps /// its historical semantics. func validateCommittedFocus() { + // Activate the first section when none is active yet. On the live + // path register() already did this; on the staged path activation is + // deferred to the commit, so it happens here. + if activeSectionID == nil { + activeSectionID = sections.first?.id + } + // Validate active section if let activeID = activeSectionID, !sections.contains(where: { $0.id == activeID }) @@ -639,6 +646,11 @@ private extension FocusManager { activeSectionID = sections.first?.id } + // No sections at all: nothing can be focused. + if activeSectionID == nil { + focusedID = nil + } + // Validate focused element if let focusID = focusedID, let section = activeSection { if !section.focusables.contains(where: { $0.focusID == focusID }) { diff --git a/Tests/TUIkitTests/RenderPassCollectorTests.swift b/Tests/TUIkitTests/RenderPassCollectorTests.swift index 37d8bbf50..3b1b8d82c 100644 --- a/Tests/TUIkitTests/RenderPassCollectorTests.swift +++ b/Tests/TUIkitTests/RenderPassCollectorTests.swift @@ -14,9 +14,6 @@ import TUIkitTestSupport /// runtime state exclusively from the frame's FINAL pass. Discarded passes /// (first-frame measurement, superseded main pass before a header /// correction) leave no trace. -/// -/// Tests assert the DESIRED behavior and carry `withKnownIssue` markers -/// until the collectors land (#56 Tasks 2-4); then the markers drop. @MainActor @Suite("Render Pass Collectors", .serialized) struct RenderPassCollectorTests { @@ -27,13 +24,8 @@ struct RenderPassCollectorTests { harness.renderFrame() - withKnownIssue("Issue #56: the measure pass writes the live header buffer") { - #expect(harness.tuiContext.appHeader.hasContent == false) - #expect(harness.tuiContext.appHeader.height == 0) - } matching: { issue in - guard case .expectationFailed = issue.kind else { return false } - return true - } + #expect(harness.tuiContext.appHeader.hasContent == false) + #expect(harness.tuiContext.appHeader.height == 0) } @Test("Preference callbacks do not accumulate across passes of one frame") @@ -46,12 +38,7 @@ struct RenderPassCollectorTests { harness.app.model.lineCount = 3 harness.renderFrame() - withKnownIssue("Issue #56: each pass appends its callbacks to the live storage") { - #expect(harness.tuiContext.preferences.callbackCount == 1) - } matching: { issue in - guard case .expectationFailed = issue.kind else { return false } - return true - } + #expect(harness.tuiContext.preferences.callbackCount == 1) } @Test("Status-bar items from a superseded pass do not persist") @@ -65,12 +52,7 @@ struct RenderPassCollectorTests { // The gated declaration exists at content height 22 (main pass of // frame 2) but not at 20 (correction pass) — the final tree has no // user items. - withKnownIssue("Issue #56: the superseded main pass writes live status-bar items") { - #expect(harness.tuiContext.statusBar.hasUserItems == false) - } matching: { issue in - guard case .expectationFailed = issue.kind else { return false } - return true - } + #expect(harness.tuiContext.statusBar.hasUserItems == false) } @Test("Focus registrations come from the final pass only") @@ -83,12 +65,7 @@ struct RenderPassCollectorTests { // The button exists in the superseded main pass (height 22) but not // in the corrected final tree (height 20): nothing may stay focused. - withKnownIssue("Issue #56: focusables from the superseded pass stay registered") { - #expect(harness.tuiContext.focusManager.currentFocusedID == nil) - } matching: { issue in - guard case .expectationFailed = issue.kind else { return false } - return true - } + #expect(harness.tuiContext.focusManager.currentFocusedID == nil) } @Test("Focus changes fire after traversal, not during it") @@ -107,13 +84,9 @@ struct RenderPassCollectorTests { let lastRenderIndex = events.lastIndex { $0.hasPrefix("render:") } let firstFocusChangeIndex = events.firstIndex(of: "focusChange") - withKnownIssue("Issue #56: auto-focus fires onFocusChange during traversal") { - if let lastRenderIndex, let firstFocusChangeIndex { - #expect(lastRenderIndex < firstFocusChangeIndex) - } - } matching: { issue in - guard case .expectationFailed = issue.kind else { return false } - return true + #expect(firstFocusChangeIndex != nil) + if let lastRenderIndex, let firstFocusChangeIndex { + #expect(lastRenderIndex < firstFocusChangeIndex) } } } @@ -163,15 +136,33 @@ private struct GrowingHeader: View { // MARK: - Per-Test Apps -/// Declares an app header ONLY in the measure pass (full 24 rows); the -/// visible tree at 22 rows has no header at all. +/// Declares an app header ONLY during the measurement phase; no output +/// pass of any frame ever declares one. A height-based gate would re-open +/// in the correction pass (which runs at full height once the header is +/// gone), so this fixture gates on ``RenderPhase`` directly. private struct MeasureOnlyHeaderApp: App { init() {} var body: some Scene { WindowGroup { - HeightGate(threshold: 23, content: Text("body").appHeader { Text("ghost header") }) + MeasurePhaseOnlyHeaderView() + } + } +} + +private struct MeasurePhaseOnlyHeaderView: View, Renderable { + var body: Never { + fatalError("MeasurePhaseOnlyHeaderView renders via Renderable") + } + + func renderToBuffer(context: RenderContext) -> FrameBuffer { + guard context.phase == .measure else { + return FrameBuffer(text: "body") } + return TUIkit.renderToBuffer( + Text("body").appHeader { Text("ghost header") }, + context: context + ) } } diff --git a/Tests/TUIkitTests/RenderPhaseCharacterizationTests.swift b/Tests/TUIkitTests/RenderPhaseCharacterizationTests.swift index 4a6bf98f0..cecd1fdc4 100644 --- a/Tests/TUIkitTests/RenderPhaseCharacterizationTests.swift +++ b/Tests/TUIkitTests/RenderPhaseCharacterizationTests.swift @@ -51,12 +51,7 @@ struct RenderPhaseCharacterizationTests { harness.app.model.lineCount = 3 harness.renderFrame() - withKnownIssue("Issue #56: main and correction pass both register handlers") { - #expect(harness.tuiContext.keyEventDispatcher.handlerCount == 1) - } matching: { issue in - guard case .expectationFailed = issue.kind else { return false } - return true - } + #expect(harness.tuiContext.keyEventDispatcher.handlerCount == 1) } @Test("onChange(initial:) fires exactly once in the first frame") diff --git a/Tests/TUIkitTests/ViewRendererTests.swift b/Tests/TUIkitTests/ViewRendererTests.swift index 09b666378..26fd0e7ac 100644 --- a/Tests/TUIkitTests/ViewRendererTests.swift +++ b/Tests/TUIkitTests/ViewRendererTests.swift @@ -79,6 +79,11 @@ struct ViewRendererTests { firstContext.paletteManager.cycleNext() firstContext.focusManager.register(MockFocusable(id: "first-focus")) + // Focus is per-runtime: the imperative registration reaches only the + // first context. + #expect(firstContext.focusManager.currentFocusedID == "first-focus") + #expect(secondContext.focusManager.currentFocusedID == nil) + let firstRenderer = ViewRenderer(terminal: firstTerminal, tuiContext: firstContext) let secondRenderer = ViewRenderer(terminal: secondTerminal, tuiContext: secondContext) @@ -90,7 +95,9 @@ struct ViewRendererTests { #expect(firstContext.notificationService.activeEntries().map(\.message) == ["first notification"]) #expect(secondContext.notificationService.activeEntries().map(\.message) == ["second notification"]) #expect(firstContext.paletteManager.current.id != secondContext.paletteManager.current.id) - #expect(firstContext.focusManager.currentFocusedID == "first-focus") + // The rendered tree declares no focusables, so the committed frame + // clears the stale imperative focus in both runtimes. + #expect(firstContext.focusManager.currentFocusedID == nil) #expect(secondContext.focusManager.currentFocusedID == nil) let stateKey = StateStorage.StateKey( From c63034dc8a285b1d8fe5fbba9315238722419c85 Mon Sep 17 00:00:00 2001 From: phranck Date: Tue, 21 Jul 2026 22:36:23 +0200 Subject: [PATCH 5/5] Fix: Keep per-pass preference adoption out of the public API - Move PreferenceStorage.adopt(from:) and callbackCount into a package extension so the collector plumbing does not widen the public surface --- .../Environment/PreferenceKey.swift | 40 ++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/Sources/TUIkitCore/Environment/PreferenceKey.swift b/Sources/TUIkitCore/Environment/PreferenceKey.swift index 8eae9a70e..9f1c76d67 100644 --- a/Sources/TUIkitCore/Environment/PreferenceKey.swift +++ b/Sources/TUIkitCore/Environment/PreferenceKey.swift @@ -174,6 +174,28 @@ public extension PreferenceStorage { callbacks[keyId]?.append(wrappedCallback) } + /// Prepares preference storage for a new render pass. + /// + /// Clears all accumulated callbacks and resets the value stack + /// to a single empty context. Called at the start of each frame + /// by `RenderLoop.render()` to prevent callback accumulation. + func beginRenderPass() { + callbacks.removeAll() + stack = [PreferenceValues()] + } + + /// Resets all preference state. + /// + /// Called once during app shutdown by `TUIContext.reset()`. + func reset() { + stack = [PreferenceValues()] + callbacks.removeAll() + } +} + +// MARK: - Per-Pass Adoption + +package extension PreferenceStorage { /// Number of registered change callbacks for tests and diagnostics. var callbackCount: Int { callbacks.values.reduce(0) { $0 + $1.count } @@ -193,22 +215,4 @@ public extension PreferenceStorage { stack = collector.stack callbacks = collector.callbacks } - - /// Prepares preference storage for a new render pass. - /// - /// Clears all accumulated callbacks and resets the value stack - /// to a single empty context. Called at the start of each frame - /// by `RenderLoop.render()` to prevent callback accumulation. - func beginRenderPass() { - callbacks.removeAll() - stack = [PreferenceValues()] - } - - /// Resets all preference state. - /// - /// Called once during app shutdown by `TUIContext.reset()`. - func reset() { - stack = [PreferenceValues()] - callbacks.removeAll() - } }