Snapshot testing: review-comment cleanup (structure, product-code de-leak, bridging)

AGENTS.md

@@ -65,7 +65,7 @@ by `./sync-agents`.
   `AGENTS.md` says what it is and how it may be used.
 - Add SPM library targets in `Package.swift` and wire apps/tests in `Project.swift` (see existing `unitTests` helper). A new module also ships a root `README.md` and `AGENTS.md` — see [Per-module docs](#per-module-docs).
 - **CI scheme**: CI runs the explicit shared **Stuff-iOS-Tests** scheme (all test bundles) rather than the autogenerated `Stuff-Workspace` scheme. New test bundles must be added to the `Stuff-iOS-Tests` scheme in `Project.swift` or CI won't run them.
-- **Snapshot test bundles are the exception**: image snapshot bundles (currently **WhereUISnapshotTests**) are slow and LFS-backed, so each runs via its own standalone scheme (declared in `Project.swift`, e.g. `WhereUISnapshotTests`) and a dedicated CI `snapshot` job — deliberately **not** in `Stuff-iOS-Tests`. Reference images under any `__Snapshots__/` directory are stored in **Git LFS** (see `.gitattributes`); the CI job checks out with `lfs: true`. The framework halves live in `Shared/SnapshotKit` (shippable matrix + previews) and `Shared/SnapshotKitTesting` (test-only capture/compare pipeline).
+- **Snapshot test bundles are the exception**: image snapshot bundles (currently **WhereUISnapshotTests**) are slow and LFS-backed, so each runs via its own standalone scheme (declared in `Project.swift`, e.g. `WhereUISnapshotTests`) and a dedicated CI `snapshot` job — deliberately **not** in `Stuff-iOS-Tests`. Reference images under any `__Snapshots__/` directory are stored in **Git LFS** (see `.gitattributes`); the CI job checks out with `lfs: true`. The framework halves live in `Shared/SnapshotKit` (shippable matrix + previews) and `Shared/SnapshotKitTesting` (test-only capture/compare pipeline). The pipeline's own regression tests are a *separate* bundle, **SnapshotKitTestingTests**, which pixel-probes captures (no LFS references) and so runs in the normal `Stuff-iOS-Tests` scheme / `test` job — only the image-reference bundles above take the standalone-scheme + `snapshot`-job route.
 - **Never double-link a package product into a test bundle that already gets it through a dynamic-framework dependency.** Xcode's default SPM integration (how `Package.local` is wired here) embeds a product's code into *every* image that links it. **WhereUI** is a dynamic framework that statically embeds its own dependencies (WhereCore, BroadwayCore/BroadwayUI, LifecycleKit, LogViewerUI, SwiftDataInspector, …), so a `*Tests` bundle that depends on **WhereUI** *and* re-lists one of those in `extraPackageProducts` ends up with a **second copy** of it. When the shared **StuffTestHost** loads several `.xctest` bundles into one process, that leaves duplicate **type metadata** for the module, and any *type-keyed runtime lookup that crosses the WhereUI boundary* — SwiftUI `EnvironmentKey`s, `UITraitBridgedEnvironmentKey` bridging, the type-keyed `BTraits`/`BThemes`/`BStylesheets` containers — silently resolves against the wrong copy and returns the default (the writer stores under one copy's key *type*, the reader looks it up under another's). It only reproduces in the **full multi-bundle scheme** (not isolated `tuist test WhereUITests` runs) and is papered over by newer Xcode linkers, so it is brutal to diagnose (it cost ~2 hours once). **Depend on such products only transitively via `WhereUI`; keep them out of `extraPackageProducts`.** `WhereStylesheetTests.resolvesTraitAwareTokensFromTheBroadwayRoot` is the regression guard — it exercises a `\.stylesheet` (→ `\.bContext`) read across the boundary and fails if a duplicate copy returns.
 
 ## Deployment

Project.swift

@@ -333,6 +333,22 @@ let project = Project(
             productDependency: "SnapshotKit",
             sources: ["Shared/SnapshotKit/Tests/**"],
         ),
+        // The capture/compare pipeline's own regression tests. They render
+        // through `renderSnapshotImage` (so they need the `StuffTestHost` key
+        // window) but assert on probed pixels rather than LFS reference images,
+        // so — unlike `WhereUISnapshotTests` — this bundle is fast, has no
+        // `__Snapshots__/`, and runs in the main `Stuff-iOS-Tests` scheme /
+        // `test` CI job. `SnapshotKitTesting` embeds its dependency closure
+        // (SnapshotKit, SnapshotTesting, AccessibilitySnapshot) into the
+        // `.xctest`; that's fine here since no WhereUI dynamic-framework
+        // boundary is crossed (the duplicate-metadata hazard is specific to the
+        // WhereUISnapshotTests topology).
+        unitTests(
+            name: "SnapshotKitTestingTests",
+            bundleIdSuffix: "snapshotkittesting",
+            productDependency: "SnapshotKitTesting",
+            sources: ["Shared/SnapshotKitTesting/Tests/**"],
+        ),
         unitTests(
             name: "RegionKitTests",
             bundleIdSuffix: "regionkit",
@@ -485,6 +501,7 @@ let project = Project(
                 "PeriscopeToolsTests",
                 "SwiftDataInspectorTests",
                 "SnapshotKitTests",
+                "SnapshotKitTestingTests",
                 "RegionKitTests",
                 "WhereCoreTests",
                 "WhereTests",
@@ -506,6 +523,7 @@ let project = Project(
                 "PeriscopeToolsTests",
                 "SwiftDataInspectorTests",
                 "SnapshotKitTests",
+                "SnapshotKitTestingTests",
                 "RegionKitTests",
                 "WhereCoreTests",
                 "WhereTests",
@@ -526,6 +544,7 @@ let project = Project(
         testScheme(name: "PeriscopeToolsTests"),
         testScheme(name: "SwiftDataInspectorTests"),
         testScheme(name: "SnapshotKitTests"),
+        testScheme(name: "SnapshotKitTestingTests"),
         testScheme(name: "RegionKitTests"),
         testScheme(name: "WhereCoreTests"),
         testScheme(name: "WhereTests"),

Shared/SnapshotKit/AGENTS.md

@@ -38,9 +38,13 @@ Complements the root [`AGENTS.md`](../../AGENTS.md) — read that first.
   deterministic — externally-loaded substrates, system controls that render
   relative to wall-clock state, wall-clock timers that flip visible state
   (skipped under capture; an explicit per-case seam pins each state) — may
-  substitute a placeholder of identical layout. It is a UIKit trait bridge
-  (not `@Entry`) so it reaches SwiftUI through the re-hosting the capture
-  pipeline does.
+  substitute a placeholder of identical layout. It is a **hybrid** accessor
+  (like Broadway's `BContext+SwiftUI`): a pure-SwiftUI `EnvironmentKey` read
+  first — set synchronously by the preview cutsheet / any `.environment`
+  override, no `UITraitCollection` round-trip — falling back to a
+  `UITraitBridgedEnvironmentKey` (not `@Entry`), which is how the test
+  pipeline's `traitOverrides` value reaches SwiftUI through the re-hosting the
+  capture does. The setter mirrors into both.
 - **Design-system-agnostic.** SnapshotKit never imports Broadway/WhereUI; the
   Broadway root wrap is a consumer concern (`WhereUI`'s `whereSnapshot(...)`).
 

Shared/SnapshotKit/Sources/SnapshotCaptureFlag.swift

@@ -32,6 +32,18 @@ private struct IsCapturingSnapshotKey: UITraitBridgedEnvironmentKey {
     }
 }
 
+/// A pure-SwiftUI capture flag set by SwiftUI-native callers (the preview
+/// cutsheet's `.environment(\.isCapturingSnapshot, true)`, or any `.environment`
+/// override). Because it's an ordinary `EnvironmentKey` — not trait-bridged — it
+/// propagates synchronously through the SwiftUI tree with no `UITraitCollection`
+/// round-trip or first-frame lag. `nil` means "no SwiftUI ancestor set one", so
+/// the accessor falls back to the UIKit-bridged value. This mirrors Broadway's
+/// `BContext+SwiftUI` bridging, and is why the capture pipeline's
+/// `traitOverrides`-set value (UIKit) still reaches SwiftUI via the fallback.
+private struct SwiftUIIsCapturingSnapshotKey: EnvironmentKey {
+    static let defaultValue: Bool? = nil
+}
+
 extension EnvironmentValues {
     /// `true` while `SnapshotKitTesting` captures this view for a snapshot test,
     /// and in the ``SnapshotProviding/snapshotPreviews`` cutsheet (previews
@@ -53,7 +65,7 @@ extension EnvironmentValues {
     /// Keep the placeholder honest: the view's own chrome (markers, overlays,
     /// legends, row titles) must still render for real; only the
     /// nondeterministic element is substituted (see the Where app's
-    /// `RegionMapView` and `SnapshotDatePickerStandIn`).
+    /// `RegionMapView` and `WhereDatePicker`).
     ///
     /// The same determinism rationale covers **wall-clock timers that flip
     /// visible state**: whether such a timer has fired by capture time races
@@ -62,8 +74,18 @@ extension EnvironmentValues {
     /// each state then gets its own snapshot case (see the Where app's launch
     /// splash, whose slow-launch caption shows iff its `previewShowsCaption`
     /// seam says so). Everything else must render real content.
+    ///
+    /// A SwiftUI-set value (the preview cutsheet, or any `.environment`
+    /// override) is read synchronously from a pure-SwiftUI key; with none set,
+    /// it falls back to the value bridged from the nearest UIKit ancestor's
+    /// trait collection (how the test pipeline's `traitOverrides` reaches
+    /// SwiftUI). Writing mirrors into both so a SwiftUI-set flag also reaches
+    /// nested UIKit.
     public var isCapturingSnapshot: Bool {
-        get { self[IsCapturingSnapshotKey.self] }
-        set { self[IsCapturingSnapshotKey.self] = newValue }
+        get { self[SwiftUIIsCapturingSnapshotKey.self] ?? self[IsCapturingSnapshotKey.self] }
+        set {
+            self[SwiftUIIsCapturingSnapshotKey.self] = newValue
+            self[IsCapturingSnapshotKey.self] = newValue
+        }
     }
 }

Shared/SnapshotKitTesting/AGENTS.md

@@ -77,5 +77,17 @@ Complements the root [`AGENTS.md`](../../AGENTS.md) — read that first.
 
 ## Testing
 
-No dedicated test bundle; exercised by every `*SnapshotTests` bundle that calls
-`assertSnapshots` (currently `WhereUISnapshotTests`).
+`SnapshotKitTestingTests` (`Tests/`, wired in `Project.swift`, in the
+`Stuff-iOS-Tests` scheme) owns the pipeline's own regression tests: async-content
+settle, concurrent-capture serialization, duplicate-identifier detection,
+tile-and-stitch / full-content sizing, the pre-capture hook, the same-image
+capture-flag surface, and safe-area composition (the swizzle zeroes the captured
+root while an interior `safeAreaInset` still composes). They render through
+`renderSnapshotImage` (so they need the `StuffTestHost` key window) but assert on
+probed pixels via the `@_spi(Testing)` `PixelSample`/`probePixel` API rather than
+LFS reference images — so the bundle is fast, has no `__Snapshots__/`, and runs
+in the main `test` job, not the snapshot job. The matrixed image assertions
+themselves are still exercised by consumer bundles (currently
+`WhereUISnapshotTests`); the WhereUI↔bundle cross-boundary flag probe stays there
+(`SnapshotCaptureFlagProbeTests`), since only a WhereUI-defined view can detect a
+duplicate-`SnapshotKit` split.

Shared/SnapshotKitTesting/Sources/SnapshotImageRendering.swift

@@ -96,6 +96,10 @@ private func renderSnapshotImageLocked(
     onReadyToSnapshot: (@MainActor () async -> Void)?,
 ) async -> UIImage {
     func capture() async -> UIImage {
+        // `hostKeyWindow()` is the specific window `StuffTestHost` stamps with
+        // `isMainTestHostWindow` — the guaranteed root window we set up, not
+        // merely whatever window happens to be key. So this is stable regardless
+        // of any transient key-window changes during a capture.
         guard let window = hostKeyWindow(), let hostRoot = window.rootViewController else {
             preconditionFailure(
                 "SnapshotKitTesting requires the StuffTestHost key window. Run snapshot tests in a hosted bundle.",

Where/WhereUI/SnapshotTests/SnapshotPixelProbe.swift → Shared/SnapshotKitTesting/Sources/SnapshotPixelProbe.swift

@@ -3,18 +3,22 @@ import UIKit
 /// A single RGBA pixel sampled from a rendered snapshot, with components in
 /// `0...1`. Shared by the pipeline regression tests that assert on rendered
 /// colors rather than reference images.
-struct PixelSample {
-    let red: CGFloat
-    let green: CGFloat
-    let blue: CGFloat
-    let alpha: CGFloat
+///
+/// `@_spi(Testing)`: a probe for snapshot-pipeline tests, not shipping API.
+@_spi(Testing)
+public struct PixelSample {
+    public let red: CGFloat
+    public let green: CGFloat
+    public let blue: CGFloat
+    public let alpha: CGFloat
 }
 
+@_spi(Testing)
 extension UIImage {
     /// Samples the pixel at a unit-space point (`(0.5, 0.5)` is the center),
     /// clamped to the image bounds. Returns transparent black if the image has
     /// no bitmap backing.
-    func probePixel(atUnitPoint unit: CGPoint) -> PixelSample {
+    public func probePixel(atUnitPoint unit: CGPoint) -> PixelSample {
         guard let cgImage else { return PixelSample(red: 0, green: 0, blue: 0, alpha: 0) }
         let x = min(cgImage.width - 1, max(0, Int(CGFloat(cgImage.width) * unit.x)))
         let y = min(cgImage.height - 1, max(0, Int(CGFloat(cgImage.height) * unit.y)))

Shared/SnapshotKitTesting/Sources/SnapshotRenderingSupport.swift

@@ -86,6 +86,13 @@ func settleContent(
         }
         CATransaction.performWithoutAnimation(view.layoutIfNeeded)
         let sample = view.renderedContentSample()
+        // Byte-exact on purpose — not the tolerance the final image compare uses.
+        // The final compare answers "does this match the reference?", where
+        // sub-pixel/gamut noise warrants a perceptual threshold. This loop answers
+        // a different question — "has rendering *stopped changing*?" — and the
+        // quarter-resolution sample already absorbs sub-pixel jitter, so exact
+        // equality is the right settled signal. A tolerance here would let a slow,
+        // still-drifting animation read as settled between adjacent samples.
         if sample != nil, sample == anchorSample {
             stablePasses += 1
         } else {
@@ -152,6 +159,12 @@ func drainInFlightAnimations(timeout: TimeInterval = 1) -> Bool {
 extension UIView {
     /// Clears the tint color of every text input in the tree so the blinking caret
     /// doesn't flake captures. Not restored — capture hosts are transient.
+    ///
+    /// It walks the tree and targets only `UITextField`/`UITextView` rather than
+    /// clearing the tint once at the root: `tintColor` inherits, so a clear root
+    /// tint would also blank every control that draws with the accent tint —
+    /// buttons, links, toggles, `Label` glyphs — silently changing the captured
+    /// image. Only the text inputs (whose caret *is* the tint) may be cleared.
     @MainActor
     func hideTextInputCursors() {
         recursiveForEach(UITextField.self) { $0.tintColor = .clear }

Where/WhereUI/SnapshotTests/AsyncContentCaptureTests.swift → Shared/SnapshotKitTesting/Tests/AsyncContentCaptureTests.swift

@@ -1,4 +1,4 @@
-import SnapshotKitTesting
+@_spi(Testing) import SnapshotKitTesting
 import SwiftUI
 import TestHostSupport
 import Testing

Where/WhereUI/SnapshotTests/ConcurrentCaptureTests.swift → Shared/SnapshotKitTesting/Tests/ConcurrentCaptureTests.swift

@@ -1,4 +1,4 @@
-import SnapshotKitTesting
+@_spi(Testing) import SnapshotKitTesting
 import SwiftUI
 import TestHostSupport
 import Testing

Where/WhereUI/SnapshotTests/LargeViewCaptureTests.swift → Shared/SnapshotKitTesting/Tests/LargeViewCaptureTests.swift

@@ -1,4 +1,4 @@
-import SnapshotKitTesting
+@_spi(Testing) import SnapshotKitTesting
 import SwiftUI
 import TestHostSupport
 import Testing

Where/WhereUI/SnapshotTests/PreCaptureHookTests.swift → Shared/SnapshotKitTesting/Tests/PreCaptureHookTests.swift

@@ -1,5 +1,5 @@
 import Observation
-import SnapshotKitTesting
+@_spi(Testing) import SnapshotKitTesting
 import SwiftUI
 import TestHostSupport
 import Testing

Shared/SnapshotKitTesting/Tests/SafeAreaCompositionTests.swift

@@ -0,0 +1,63 @@
+@_spi(Testing) import SnapshotKitTesting
+import SwiftUI
+import TestHostSupport
+import Testing
+import UIKit
+
+/// Regression guard for `SafeAreaInsetsSwizzling`. The swizzle zeroes the
+/// captured **root**'s `safeAreaInsets` — decoupling the image from the
+/// simulator's notch/home indicator — so a safe-area-respecting view reaches the
+/// top edge instead of being pushed down by the device insets
+/// (`zeroedRootLeavesNoDeviceInset`). It must do that *without* erasing
+/// contributions layered mid-tree: a `safeAreaInset` bar (the same mechanism a
+/// nav bar / toolbar uses) still composes on top of the zeroed base, so the bar
+/// sits flush at the top and its content lays out below it — not crammed under
+/// it and not offset by the device insets (`safeAreaInsetBarComposesOnTheZeroedRoot`).
+/// An earlier design that returned the override for *every* in-tree view erased
+/// such contributions; this pins that it doesn't.
+@MainActor
+struct SafeAreaCompositionTests {
+    @Test func zeroedRootLeavesNoDeviceInset() async throws {
+        try waitFor { hostKeyWindow() != nil }
+        let host = UIHostingController(rootView: Color.red)
+        host.view.frame = CGRect(x: 0, y: 0, width: 100, height: 100)
+        let image = await renderSnapshotImage(of: host, safeAreaInsets: .zero)
+
+        // A safe-area-respecting fill reaches the very top: the device's real
+        // top inset was zeroed at the root rather than leaking into the image.
+        let top = image.probePixel(atUnitPoint: CGPoint(x: 0.5, y: 0.05))
+        #expect(top.red > 0.5)
+        #expect(top.green < 0.5)
+    }
+
+    @Test func safeAreaInsetBarComposesOnTheZeroedRoot() async throws {
+        try waitFor { hostKeyWindow() != nil }
+        let host = UIHostingController(rootView: BarProbeView())
+        host.view.frame = CGRect(x: 0, y: 0, width: 100, height: 100)
+        let image = await renderSnapshotImage(of: host, safeAreaInsets: .zero)
+
+        // The 30pt bar sits flush at the top (not offset by device insets) —
+        // the interior contribution composed on the zeroed base.
+        let inBar = image.probePixel(atUnitPoint: CGPoint(x: 0.5, y: 0.15))
+        #expect(inBar.green > 0.5)
+        #expect(inBar.red < 0.5)
+
+        // Below the bar: the content the bar inset, still present rather than
+        // crammed under the bar.
+        let belowBar = image.probePixel(atUnitPoint: CGPoint(x: 0.5, y: 0.6))
+        #expect(belowBar.red > 0.5)
+        #expect(belowBar.green < 0.5)
+    }
+}
+
+/// A red content pane with a 30pt green `safeAreaInset` bar pinned to the top —
+/// the bar's contribution must compose on top of the captured root's zeroed
+/// safe area.
+private struct BarProbeView: View {
+    var body: some View {
+        Color.red
+            .safeAreaInset(edge: .top, spacing: 0) {
+                Color.green.frame(height: 30)
+            }
+    }
+}

Where/WhereUI/SnapshotTests/SnapshotCaptureFlagTests.swift → Shared/SnapshotKitTesting/Tests/SnapshotCaptureFlagTests.swift

@@ -1,4 +1,4 @@
-import SnapshotKitTesting
+@_spi(Testing) import SnapshotKitTesting
 import SwiftUI
 import TestHostSupport
 import Testing