fix(replay): verify mask alignment instead of discarding frames on any redraw

[arnohillen]

💡 Motivation and Context

Closes #596.

In screenshot mode, a frame was discarded whenever its window redrew during the PixelCopy capture, unless an animation-type heuristic matched (hasTransientState from #529, surface/texture views from #649). Most animated content matches neither signal: indeterminate spinners (ProgressDialog), animated GIFs (Glide/Coil), Lottie (which never sets transient state, so the #529 exemption structurally could not fire for it), Material progress indicators, and Compose infinite animations all redraw per frame on the UI thread. On screens showing any of them, essentially every capture logged "Session Replay screenshot discarded due to screen changes" and the replay showed nothing. On top of that, the draw-dirty flags were single fields shared across all tracked windows, so an animating loader dialog also blanked captures of the static activity behind it.

The guard exists for a real reason (#254 / #234): mask rects are computed from live views after the pixels are frozen, so a structural change mid-capture can drift masks off sensitive content. This PR keeps that protection but stops proxying it with "did anything redraw":

• Per-window draw state: isOnDrawnCalled/didLayoutSinceReset move into a WindowDrawState on ViewTreeSnapshotStatus. PixelCopy copies a single window's surface and masks come from that window's own tree, so one window's draws say nothing about another window's mask alignment.
• Verified masks instead of heuristics: mask rects are sampled before PixelCopy.request and again in the callback. A dirty frame is kept only when both walks agree, no layout pass ran, and neither walk was poisoned. Pixel-only animation redraws pass this check no matter which library drives them; structural changes still discard. The hasTransientState/surface-view exemptions are deleted: frames they legitimately kept have stable geometry and pass rect equality anyway, and frames they kept with moving masked geometry were unsafe to keep at all.
• Fail closed on unknowable geometry: a walk is poisoned (frame discarded) when it meets a rendered view it cannot place (legacy view.animation, transient state mid-animation), and when the Compose semantics pass times out. Previously such views were silently pruned from the walk, which would have shipped them unmasked. A timed-out PixelCopy latch also no longer ships the bitmap before masks are painted.

Behavior is monotone for safety: no frame that was previously discarded for a genuine structural change is now kept, and the discard log line is unchanged for support diagnostics. The default wireframe mode is untouched, and the Flutter/RN forced-screenshot bridge inherits the fix through the same path.

💚 How did you test it?

• Unit tests for the keep/discard predicate: clean frame, stable rects, moved rects, appeared/disappeared rects, layout pass, poisoned walks.
• Deterministic end-to-end Robolectric tests through generateSnapshot + ShadowPixelCopy, using a hook view that injects state changes exactly between the pre- and post-copy walks: dirty-but-stable frame kept (the fix path), masked widget moved mid-capture discarded, layout mid-capture discarded, masked view mid legacy animation fails closed (regression test for the walk-pruning hole), and a redraw+layout in another window no longer discards this window's capture (fails under the old shared-flag code).
• Full posthog-android unit test suite, apiCheck, and spotlessCheck pass locally.

📝 Checklist

• I reviewed the submitted code.
• I added tests to verify the changes.
• I updated the docs if needed.
• No breaking change or entry added to the changelog.

If releasing new changes

• Ran pnpm changeset to generate a changeset file

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

• Authored with PostHog Code (Claude). Root cause was established with a multi-agent investigation over the SDK source, AOSP internals (ProgressBar/AnimatedVectorDrawable/ViewRootImpl), and the fix history of chore: do not capture screenshot during screen changes #254/fix: stop screenshot frames being dropped during animations #529/fix(replay): keep frames on screens with continuous surface rendering (e.g. Rive) #649, then adversarially verified claim by claim.
• Notable findings that shaped the design: setHasTransientState is only called by ViewPropertyAnimator, Editor, Transition, and view-translation in the framework, so the fix: stop screenshot frames being dropped during animations #529 exemption never fired for Lottie or any drawable-level animation; sibling SDKs (posthog-ios, Sentry Android) avoid this bug class by sampling masks synchronously with the frame, which is what the rect pre/post verification approximates while keeping PixelCopy off the main thread.
• An enumerate-more-animation-types fix (like closed PR fix: capture session replay screenshots during animated loaders #606) was considered and rejected: it cannot cover Compose or future libraries and keeps unverified keep paths. Rect verification subsumes the heuristics.
• The diff itself was adversarially reviewed by independent agents (PII, concurrency, regression, efficacy lenses); the confirmed findings (legacy-animation walk pruning, Compose walk timeout, stale exemption flag) are fixed and regression-tested in this PR.

---

Created with PostHog Code

[greptile-apps]

Security Review

Two fail-open paths remain in screenshot masking: poisoned walks are accepted when no redraw is observed, and a late timed-out PixelCopy callback can erase state belonging to a newer capture.

Prompt To Fix All With AI

### Issue 1
posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt:1174-1176
**Poisoned clean frames bypass masking**

When either mask walk is poisoned without an intervening redraw, this early return accepts the frame and paints only the incomplete post-walk rectangles, causing sensitive content to be emitted without its required mask.

**How this was verified:** The capture resets the draw flag before walking, while timeout and unstable-geometry paths poison a walk independently of that flag.

```suggestion
        if (preWalk.poisoned || postWalk.poisoned) {
            return false
        }
        if (!drawState.isOnDrawnCalled) {
            return true
        }
```

### Issue 2
posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt:1284-1287
**Stale callbacks erase capture state**

If a PixelCopy callback arrives after its one-second timeout and another capture for the same window has started, this old callback resets the shared `WindowDrawState`, erasing draw or layout changes from the newer capture and allowing a screenshot with misaligned masks to be accepted.

**How this was verified:** The timeout releases the capture executor while the pending callback retains the same per-window state and resets it from its `finally` block.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (1): Last reviewed commit: "fix(replay): verify mask alignment inste..." | Re-trigger Greptile}

[marandaneto]

@arnohillen, this requires manual testing; otherwise, we risk leaking PII. Have you tested this, or are you purely relying on the unit tests?

[marandaneto]

[P1] Equal endpoint masks do not prove copy-time alignment. A masked view can move from rect A to B, be captured by PixelCopy at B, then return to A before this comparison. Translation and Compose motion need not trigger global layout, so the equal A endpoints accept the frame while sensitive pixels remain unmasked at B. Any redraw involving movable masks needs stronger synchronization than endpoint equality.

[dustinbyrne]

Validated via test, will fix.

[marandaneto]

[P1] Do not bypass alignment checks before onDraw fires. Layout or mask geometry can change after pixels are frozen but before the next draw callback; in that interval this branch accepts the frame even when didLayoutSinceReset is true or the walks differ, then paints post-change masks over pre-change pixels. The clean path should still reject layouts and mismatched walks.

[dustinbyrne]

Validated via test, will fix.

[marandaneto]

[P1] Poison Compose walks when semantics traversal fails. completed only means the posted runnable ended: the RootForTest early return and caught semantics exceptions also count down the latch. Both walks can therefore become trustworthy empty lists and ship a redrawing Compose screen without masks. Mark the walk poisoned for every traversal failure, not only timeout or interruption.

[dustinbyrne]

Validated via test, will fix.

[marandaneto]

[P1] Do not reset capture tracking with snapshot metadata. clearSnapshotStates() can run while PixelCopy is pending, and this reset erases draw/layout evidence recorded since the pre-walk. The callback can then take the clean-frame path and paint current mask coordinates over older pixels. Either invalidate the pending capture or leave its draw state intact.

[dustinbyrne]

Validated via test, will fix.

[posthog]

🦔 ReviewHog reviewed this pull request

Found 0 must fix, 4 should fix, 2 consider.

Published 6 findings (view the review).

[posthog]

ReviewHog Alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

[posthog]

ReviewHog Report

Changes

Issues: 6 issues

Files (3)

• .changeset/replay-animated-screens-capture.md
• posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt
• posthog-android/src/main/java/com/posthog/android/replay/internal/ViewTreeSnapshotStatus.kt

[posthog]

stop()/uninstall() reset per-window draw state while a capture may be in-flight, letting a real structural change ship with misaligned masks

Why we think it's a valid issue

• Checked: The drawState reference flow (generateSnapshot passes status.drawState by reference to toScreenshotWireframe, PostHogReplayIntegration.kt:658-661), the reset call sites at uninstall() (line 506) and stop() (lines 1888-1891), the callback read + shouldKeepFrame (lines 1230, 1159-1166), and reachability of stop() plus any downstream gate.
• Found: The race is real and correctly analyzed. WindowDrawState flags are read by the PixelCopy callback on the pixelCopy handler thread while stop()/uninstall() write reset() on the caller's thread — the callback is not aborted by stop() (it holds its own drawState/view/window refs, and clearViewListeners removing the decorViews entry doesn't cancel the in-flight call). If a genuine structural change sets isOnDrawnCalled/didLayoutSinceReset true during the capture and a concurrent reset wipes them before the callback reads, shouldKeepFrame hits if (!drawState.isOnDrawnCalled) return true (line 1162), skipping the rect-equality check and painting post-change postWalk rects over pre-change frozen pixels → masked content shifts off sensitive pixels (PII leak).
• Found: It ships: after toScreenshotWireframe returns, generateSnapshot does NOT re-check isActive() before events.capture(postHog) (line 756); the only isActive() guard is at the top (line 644).
• Found: Reachable outside teardown — a session-rotation stop-then-restart path exists (if (isSessionReplayActive) stop(); start(resumeCurrent = false), lines 1991-1992), and uninstall()'s reset (506) runs on whatever thread calls the public API. The proposed fix (b) is sound: the per-attempt drawState.reset() at line 1214 already arms each capture, so the external resets in stop()/uninstall() are redundant for arming and only add race surface.
• Impact: A residual path that ships misaligned masks defeats the exact fail-closed PII guarantee this PR exists to provide (chore: do not capture screenshot during screen changes #254/Redacted text showing on android during session replay #234) — meets the keep bar (race corrupting shared state → privacy leak) with a concrete trigger and consequence.
• Priority: Lowered must_fix → should_fix: the consequence is severe (PII), but the trigger is a narrow multi-condition interleaving (in-flight PixelCopy + a masked view structurally moving + a concurrent stop/uninstall reset landing in the callback sub-window, screenshot mode only), not a deterministic defect, and it is explicitly the same class already flagged at the resetViewSnapshotStates/~420 call site — additive (new call sites, unifying fix) rather than an independent blocker.

Issue description

This is the same defect class already flagged on resetViewSnapshotStates/clearSnapshotStates() (the existing review comment anchored at line ~420), but it recurs at two more call sites that write directly to WindowDrawState instead of going through that function: uninstall() calls status.drawState.reset() for every tracked window (line 506), and stop() calls it.drawState.reset() for every tracked window (lines 1888-1891). Both run on whatever thread calls the public stop()/uninstall() API, which is not the single-threaded capture executor. toScreenshotWireframe(view, window, drawState) receives the same WindowDrawState object by reference (captured directly from status.drawState, not re-looked-up from decorViews), and an in-flight capture can be anywhere between its own drawState.reset() and its PixelCopy callback's shouldKeepFrame(...) check when stop()/uninstall() fires concurrently. If a genuine structural change (real isOnDrawnCalled/didLayoutSinceReset) happened during that in-flight capture and stop()/uninstall()'s reset wipes those flags back to false before the PixelCopy callback reads them, shouldKeepFrame takes the !drawState.isOnDrawnCalled -> return true "clean" branch unconditionally, skipping the rect-equality check entirely and painting the freshly-walked postWalk mask rects over pixels that were frozen before the structural change. Removing an entry from decorViews (via clearViewListeners, called just before the uninstall() reset) does not stop the in-flight call, since it already holds its own reference to the same drawState/view/window objects. stop() in particular is reachable from a routine "stop-then-restart" path (e.g. session/config-driven if (isSessionReplayActive) stop(); start(resumeCurrent = false)), so this is not a rare shutdown-only race.

Suggested fix

Don't let stop()/uninstall() silently wipe a window's WindowDrawState back to the 'clean' state while a capture for that window might be in-flight. Either (a) make the capture path invalidate itself against an external stop/uninstall (e.g. re-check isActive()/an installation flag right before shouldKeepFrame and discard unconditionally if it flipped since the capture started), or (b) stop resetting drawState from stop()/uninstall() at all — each toScreenshotWireframe call already resets its own drawState at the top of its own attempt (line ~1214), so the external reset from stop()/uninstall() isn't needed to arm the next capture and only exists to race with a capture that's already running.

Prompt to fix with AI (copy-paste)

## Context
@posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt#L506
@posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt#L1888-1891

<issue_description>
This is the same defect class already flagged on `resetViewSnapshotStates`/`clearSnapshotStates()` (the existing review comment anchored at line ~420), but it recurs at two more call sites that write directly to `WindowDrawState` instead of going through that function: `uninstall()` calls `status.drawState.reset()` for every tracked window (line 506), and `stop()` calls `it.drawState.reset()` for every tracked window (lines 1888-1891). Both run on whatever thread calls the public `stop()`/`uninstall()` API, which is not the single-threaded capture `executor`. `toScreenshotWireframe(view, window, drawState)` receives the *same* `WindowDrawState` object by reference (captured directly from `status.drawState`, not re-looked-up from `decorViews`), and an in-flight capture can be anywhere between its own `drawState.reset()` and its PixelCopy callback's `shouldKeepFrame(...)` check when `stop()`/`uninstall()` fires concurrently. If a genuine structural change (real `isOnDrawnCalled`/`didLayoutSinceReset`) happened during that in-flight capture and `stop()`/`uninstall()`'s reset wipes those flags back to `false` before the PixelCopy callback reads them, `shouldKeepFrame` takes the `!drawState.isOnDrawnCalled -> return true` "clean" branch unconditionally, skipping the rect-equality check entirely and painting the freshly-walked `postWalk` mask rects over pixels that were frozen before the structural change. Removing an entry from `decorViews` (via `clearViewListeners`, called just before the `uninstall()` reset) does not stop the in-flight call, since it already holds its own reference to the same `drawState`/`view`/`window` objects. `stop()` in particular is reachable from a routine "stop-then-restart" path (e.g. session/config-driven `if (isSessionReplayActive) stop(); start(resumeCurrent = false)`), so this is not a rare shutdown-only race.
</issue_description>

<issue_validation>
- **Checked:** The drawState reference flow (`generateSnapshot` passes `status.drawState` by reference to `toScreenshotWireframe`, PostHogReplayIntegration.kt:658-661), the reset call sites at `uninstall()` (line 506) and `stop()` (lines 1888-1891), the callback read + `shouldKeepFrame` (lines 1230, 1159-1166), and reachability of `stop()` plus any downstream gate.
- **Found:** The race is real and correctly analyzed. `WindowDrawState` flags are read by the PixelCopy callback on the pixelCopy handler thread while `stop()`/`uninstall()` write `reset()` on the caller's thread — the callback is not aborted by `stop()` (it holds its own `drawState`/`view`/`window` refs, and `clearViewListeners` removing the `decorViews` entry doesn't cancel the in-flight call). If a genuine structural change sets `isOnDrawnCalled`/`didLayoutSinceReset` true during the capture and a concurrent reset wipes them before the callback reads, `shouldKeepFrame` hits `if (!drawState.isOnDrawnCalled) return true` (line 1162), skipping the rect-equality check and painting post-change `postWalk` rects over pre-change frozen pixels → masked content shifts off sensitive pixels (PII leak).
- **Found:** It ships: after `toScreenshotWireframe` returns, `generateSnapshot` does NOT re-check `isActive()` before `events.capture(postHog)` (line 756); the only `isActive()` guard is at the top (line 644).
- **Found:** Reachable outside teardown — a session-rotation stop-then-restart path exists (`if (isSessionReplayActive) stop(); start(resumeCurrent = false)`, lines 1991-1992), and `uninstall()`'s reset (506) runs on whatever thread calls the public API. The proposed fix (b) is sound: the per-attempt `drawState.reset()` at line 1214 already arms each capture, so the external resets in `stop()`/`uninstall()` are redundant for arming and only add race surface.
- **Impact:** A residual path that ships misaligned masks defeats the exact fail-closed PII guarantee this PR exists to provide (#254/#234) — meets the keep bar (race corrupting shared state → privacy leak) with a concrete trigger and consequence.
- **Priority:** Lowered must_fix → should_fix: the consequence is severe (PII), but the trigger is a narrow multi-condition interleaving (in-flight PixelCopy + a masked view structurally moving + a concurrent stop/uninstall reset landing in the callback sub-window, screenshot mode only), not a deterministic defect, and it is explicitly the same class already flagged at the resetViewSnapshotStates/~420 call site — additive (new call sites, unifying fix) rather than an independent blocker.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Don't let `stop()`/`uninstall()` silently wipe a window's `WindowDrawState` back to the 'clean' state while a capture for that window might be in-flight. Either (a) make the capture path invalidate itself against an external stop/uninstall (e.g. re-check `isActive()`/an installation flag right before `shouldKeepFrame` and discard unconditionally if it flipped since the capture started), or (b) stop resetting `drawState` from `stop()`/`uninstall()` at all — each `toScreenshotWireframe` call already resets its own `drawState` at the top of its own attempt (line ~1214), so the external reset from `stop()`/`uninstall()` isn't needed to arm the next capture and only exists to race with a capture that's already running.
</potential_solution>

[dustinbyrne]

Couldn't validate this via test: stop/uninstall do reset the local draw flags, but their lifecycle and queue gates prevented the test from demonstrating that an unsafe frame can be emitted.

[posthog]

MaskWalk.poisoned is write-only during the recursive walk — poisoning never short-circuits the walk that produced it

Why we think it's a valid issue

• Checked: findMaskableWidgets traversal (PostHogReplayIntegration.kt:925-1024), the poison site (1015-1018), the Compose branch (940-944 → findMaskableComposeWidgets), its blocking await (1099-1113, latch.await(1000, ...)), and shouldKeepFrame's poison short-circuit (1159-1161).
• Found: The premise is correct and verified. walk.poisoned = true at line 1017 is followed by continue, and no code path in findMaskableWidgets reads walk.poisoned to stop — so once poisoned, the recursion keeps descending into every remaining visible subtree. A later isComposeView() node routes to findMaskableComposeWidgets, which posts to the main thread and blocks the (single-threaded) capture executor up to 1s. shouldKeepFrame returns false the instant either walk is poisoned (line 1159), so all of that traversal — including the main-thread semantics walk and its up-to-1s block — is spent on a frame already guaranteed to be discarded.
• Found: The suggested early-return is safe: when preWalk.poisoned, shouldKeepFrame never reaches the preWalk.rects == postWalk.rects comparison (poison check is first), so skipping the remaining rect collection cannot change the verdict.
• Impact: On mixed View+Compose hierarchies where a legacy View is mid-animation/transient (the poison trigger) and Compose content sits later in the tree, every capture during the animation needlessly runs a main-thread Compose semantics walk (worst case a ~1s executor stall) for a doomed frame — wasted work, and added main-thread load precisely during animation, on the hot capture path this PR exists to make less lossy. Real, concrete, with a trivial safe fix.
• Priority: Keeping the reviewer's consider — the premise is sound and the fix cheap, but the trigger is a specific interop+animation combination and the 1s figure is a worst-case ceiling (typically milliseconds), so it is a genuine-but-minor optimization rather than a scale problem.

Issue description

findMaskableWidgets sets walk.poisoned = true when it meets a rendered-but-unplaceable view, then continues into the rest of the sibling loop instead of returning — the recursive walk keeps descending into every remaining subtree even though the frame is now unconditionally going to be discarded (shouldKeepFrame returns false as soon as either walk is poisoned). Because a Compose view found later in the same tree routes through findMaskableComposeWidgets, which blocks the calling thread on a main-thread round trip for up to a full second, a walk that was poisoned early can still pay one or more of those 1-second stalls for a frame that was already dead — on the same hot capture path this PR is trying to make less lossy.

Suggested fix

Check walk.poisoned at the top of the recursive step (and before invoking findMaskableComposeWidgets) and return immediately once set, so a poisoned walk stops doing work instead of continuing to traverse (and potentially blocking on Compose semantics) for a result that will be discarded regardless.

Prompt to fix with AI (copy-paste)

## Context
@posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt#L1009-1025

<issue_description>
`findMaskableWidgets` sets `walk.poisoned = true` when it meets a rendered-but-unplaceable view, then `continue`s into the rest of the sibling loop instead of returning — the recursive walk keeps descending into every remaining subtree even though the frame is now unconditionally going to be discarded (`shouldKeepFrame` returns `false` as soon as either walk is poisoned). Because a Compose view found later in the same tree routes through `findMaskableComposeWidgets`, which blocks the calling thread on a main-thread round trip for up to a full second, a walk that was poisoned early can still pay one or more of those 1-second stalls for a frame that was already dead — on the same hot capture path this PR is trying to make less lossy.
</issue_description>

<issue_validation>
- **Checked:** `findMaskableWidgets` traversal (PostHogReplayIntegration.kt:925-1024), the poison site (1015-1018), the Compose branch (940-944 → `findMaskableComposeWidgets`), its blocking await (1099-1113, `latch.await(1000, ...)`), and `shouldKeepFrame`'s poison short-circuit (1159-1161).
- **Found:** The premise is correct and verified. `walk.poisoned = true` at line 1017 is followed by `continue`, and no code path in `findMaskableWidgets` reads `walk.poisoned` to stop — so once poisoned, the recursion keeps descending into every remaining visible subtree. A later `isComposeView()` node routes to `findMaskableComposeWidgets`, which posts to the main thread and blocks the (single-threaded) capture executor up to 1s. `shouldKeepFrame` returns false the instant either walk is poisoned (line 1159), so all of that traversal — including the main-thread semantics walk and its up-to-1s block — is spent on a frame already guaranteed to be discarded.
- **Found:** The suggested early-return is safe: when `preWalk.poisoned`, `shouldKeepFrame` never reaches the `preWalk.rects == postWalk.rects` comparison (poison check is first), so skipping the remaining rect collection cannot change the verdict.
- **Impact:** On mixed View+Compose hierarchies where a legacy View is mid-animation/transient (the poison trigger) and Compose content sits later in the tree, every capture during the animation needlessly runs a main-thread Compose semantics walk (worst case a ~1s executor stall) for a doomed frame — wasted work, and added main-thread load precisely during animation, on the hot capture path this PR exists to make less lossy. Real, concrete, with a trivial safe fix.
- **Priority:** Keeping the reviewer's `consider` — the premise is sound and the fix cheap, but the trigger is a specific interop+animation combination and the 1s figure is a worst-case ceiling (typically milliseconds), so it is a genuine-but-minor optimization rather than a scale problem.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Check `walk.poisoned` at the top of the recursive step (and before invoking `findMaskableComposeWidgets`) and return immediately once set, so a poisoned walk stops doing work instead of continuing to traverse (and potentially blocking on Compose semantics) for a result that will be discarded regardless.
</potential_solution>

[dustinbyrne]

Validated via test, will fix.

[posthog]

No early-out when the pre-copy walk is already poisoned — full PixelCopy capture and post-walk still run for a guaranteed discard

Why we think it's a valid issue

• Checked: The capture sequence in toScreenshotWireframe (PostHogReplayIntegration.kt:1214-1234): drawState.reset() → preWalk computed under maskWalkLock (1218-1221) → PixelCopy.request (1223) → callback post-walk (guarded only by alreadyDoomed, 1230-1235). Cross-checked shouldKeepFrame (1159: if (preWalk.poisoned || postWalk.poisoned) return false) and the single-threaded executor (line 137).
• Found: The premise is correct and verified. preWalk is fully materialized before PixelCopy.request, and preWalk.poisoned alone guarantees shouldKeepFrame returns false — yet nothing checks it between the pre-walk and the request. So a poisoned pre-walk still issues the full-window PixelCopy GPU readback and (unless alreadyDoomed) runs the entire post-copy findMaskableWidgets walk, including its own up-to-1s Compose main-thread round trips, all for a frame guaranteed to be discarded.
• Found: The impact is amplified by the single-threaded replay executor (line 137): toScreenshotWireframe blocks that one thread for the whole capture (PixelCopy latch up to 1s + any post-walk Compose stalls), so a wasted poisoned-frame cycle directly delays the next capture attempt app-wide — reducing the odds of landing a keepable frame on exactly the animating screens this PR targets. preWalk.poisoned fires for legacy view.animation, transient-state (ViewPropertyAnimator), and Compose timeouts, which are common during transitions/interactions.
• Found: The suggested fix is safe and mirrors the existing alreadyDoomed short-circuit: since a poisoned pre-walk is discarded regardless, returning early (log the existing discard line, skip PixelCopy + post-walk) changes no outcome while eliminating the GPU readback and second tree walk.
• Impact: Concrete, non-trivial wasted work (GPU readback + full post-walk + possible blocking Compose waits) on the single-threaded hot capture path, for guaranteed-discard frames — meets the performance bar with a clean, low-risk fix. Bigger, clearer win than the in-walk short-circuit, and complementary to it.

Issue description

shouldKeepFrame unconditionally discards the frame when preWalk.poisoned is true (line ~1157 area), and preWalk is fully computed at lines 1214-1221 before PixelCopy.request is invoked. However, the code does not check preWalk.poisoned before proceeding — it always allocates the destination bitmap, issues the PixelCopy GPU readback, and (unless alreadyDoomed via layout) still runs the entire post-copy findMaskableWidgets walk (including a second up-to-1s Compose main-thread round trip per Compose subtree) even though the outcome is already fully determined and will be discarded. preWalk.poisoned is set whenever the walk meets a view mid legacy view.animation, mid transient-state, or when any Compose semantics pass times out — none of which are rare on an actively-animating screen, which is precisely the population of screens this PR is trying to capture more successfully. Every poisoned pre-walk now pays for a full, wasted screenshot capture cycle.

Suggested fix

Check preWalk.poisoned immediately after computing the pre-walk and short-circuit (log the existing discard message and return) before allocating the bitmap or calling PixelCopy.request, mirroring the alreadyDoomed short-circuit already used for the post-walk.

Prompt to fix with AI (copy-paste)

## Context
@posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt#L1214-1222

<issue_description>
`shouldKeepFrame` unconditionally discards the frame when `preWalk.poisoned` is true (line ~1157 area), and `preWalk` is fully computed at lines 1214-1221 *before* `PixelCopy.request` is invoked. However, the code does not check `preWalk.poisoned` before proceeding — it always allocates the destination bitmap, issues the PixelCopy GPU readback, and (unless `alreadyDoomed` via layout) still runs the entire post-copy `findMaskableWidgets` walk (including a second up-to-1s Compose main-thread round trip per Compose subtree) even though the outcome is already fully determined and will be discarded. `preWalk.poisoned` is set whenever the walk meets a view mid legacy `view.animation`, mid transient-state, or when any Compose semantics pass times out — none of which are rare on an actively-animating screen, which is precisely the population of screens this PR is trying to capture more successfully. Every poisoned pre-walk now pays for a full, wasted screenshot capture cycle.
</issue_description>

<issue_validation>
- **Checked:** The capture sequence in `toScreenshotWireframe` (PostHogReplayIntegration.kt:1214-1234): `drawState.reset()` → `preWalk` computed under `maskWalkLock` (1218-1221) → `PixelCopy.request` (1223) → callback post-walk (guarded only by `alreadyDoomed`, 1230-1235). Cross-checked `shouldKeepFrame` (1159: `if (preWalk.poisoned || postWalk.poisoned) return false`) and the single-threaded executor (line 137).
- **Found:** The premise is correct and verified. `preWalk` is fully materialized before `PixelCopy.request`, and `preWalk.poisoned` alone guarantees `shouldKeepFrame` returns false — yet nothing checks it between the pre-walk and the request. So a poisoned pre-walk still issues the full-window PixelCopy GPU readback and (unless `alreadyDoomed`) runs the entire post-copy `findMaskableWidgets` walk, including its own up-to-1s Compose main-thread round trips, all for a frame guaranteed to be discarded.
- **Found:** The impact is amplified by the single-threaded replay executor (line 137): `toScreenshotWireframe` blocks that one thread for the whole capture (PixelCopy latch up to 1s + any post-walk Compose stalls), so a wasted poisoned-frame cycle directly delays the next capture attempt app-wide — reducing the odds of landing a keepable frame on exactly the animating screens this PR targets. `preWalk.poisoned` fires for legacy `view.animation`, transient-state (ViewPropertyAnimator), and Compose timeouts, which are common during transitions/interactions.
- **Found:** The suggested fix is safe and mirrors the existing `alreadyDoomed` short-circuit: since a poisoned pre-walk is discarded regardless, returning early (log the existing discard line, skip PixelCopy + post-walk) changes no outcome while eliminating the GPU readback and second tree walk.
- **Impact:** Concrete, non-trivial wasted work (GPU readback + full post-walk + possible blocking Compose waits) on the single-threaded hot capture path, for guaranteed-discard frames — meets the performance bar with a clean, low-risk fix. Bigger, clearer win than the in-walk short-circuit, and complementary to it.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Check `preWalk.poisoned` immediately after computing the pre-walk and short-circuit (log the existing discard message and return) before allocating the bitmap or calling `PixelCopy.request`, mirroring the `alreadyDoomed` short-circuit already used for the post-walk.
</potential_solution>

[dustinbyrne]

Validated via test, will fix.

[posthog]

New maskWalkLock invariant for shared scratch Rect/Point is not applied to the wireframe-mode isVisible() call

Why we think it's a valid issue

• Checked: toWireframe() (PostHogReplayIntegration.kt:1326-1332 — if (!view.isVisible()) return null, unguarded), isVisible()→hasGlobalVisibleRect() (789, 826-833) which calls getGlobalVisibleRect(reusableRect, reusablePoint) on the shared scratch fields, the screenshot path's guards (synchronized(maskWalkLock) at 1180-1184, 1219, 1232) and the new invariant comment (line 174: 'every user must hold maskWalkLock'), plus screenshot mutability (PostHogSessionReplayConfig.kt:38 public var, read fresh at line 655).
• Found: The invariant violation is real. The screenshot path acquires maskWalkLock around every reusableRect access, but toWireframe()'s isVisible() touches the same shared reusableRect/reusablePoint with no lock. getGlobalVisibleRect(Rect, Point) does a multi-level read-modify-write of the passed Rect while walking the parent chain, and its boolean result depends on that intermediate state, so a concurrent writer corrupts the visibility outcome.
• Found: Reachable cross-thread: a screenshot post-walk callback runs on the pixelCopy handler thread holding the lock, while the executor thread can run an unguarded toWireframe() walk — the same late-callback overlap the PR's own comment acknowledges (1168-1169). Because screenshot is a runtime-mutable var read per capture, a screenshot→wireframe mode transition (or a forceScreenshot/native interleave) can place a guarded screenshot callback and an unguarded wireframe walk on reusableRect simultaneously.
• Impact: A corrupted visibility boolean can wrongly drop or include a rendered view in the emitted frame; in the tail it can flip a masked view's visibility inside the screenshot mask walk (guarded side corrupted by the unguarded side), risking an unmasked sensitive view. Real race on shared mutable state violating a contract this PR introduces, with a trivial fix (lock the wireframe isVisible, or allocate a local Rect in hasGlobalVisibleRect mirroring globalVisibleRect).
• Priority: Lowered should_fix → consider: the defect is real and the fix cheap, but reachability is compound-rare — it needs a runtime capture-mode transition AND a >1s PixelCopy timeout AND the concurrent parent-chain RMW to overlap — and the usual consequence is a single mildly-corrupted frame, so it doesn't warrant should_fix urgency while remaining worth recording.

Issue description

This chunk changes the concurrency contract for the shared reusableRect/reusablePoint fields: the old comment ("Only accessed from the PixelCopy callback thread (or executor), so no synchronization needed") is replaced with "Shared scratch objects; every user must hold maskWalkLock", and a new maskWalkLock is introduced. toScreenshotWireframe()'s view.isVisible() check and the recursive findMaskableWidgets() walk are correctly moved under synchronized(maskWalkLock). However, View.toWireframe() (the non-screenshot / wireframe-mode path, unchanged by this PR at what is now approximately line 1361) also calls view.isVisible() → hasGlobalVisibleRect() → getGlobalVisibleRect(reusableRect, reusablePoint) on the same shared fields, with no lock at all. Since config.sessionReplayConfig.screenshot is read fresh on every draw (onDrawCallback) and can change at runtime via remote config, and since a timed-out PixelCopy callback for one window can keep running on the PixelCopy handler thread after the executor has moved on to the next generateSnapshot() call (exactly the overlap this PR's own comment calls out: "a timed-out capture's post-copy walk can overlap the next capture's pre-copy walk on a different thread"), it is possible for a screenshot-mode PixelCopy callback's guarded walk to run concurrently with a wireframe-mode toWireframe() call's unguarded walk on the executor thread. View.getGlobalVisibleRect(Rect, Point) performs a multi-level read-modify-write of the passed-in Rect as it walks up the parent chain, and the returned boolean itself depends on that intermediate state, so a concurrent write from the other thread can corrupt the intersection and silently return the wrong visibility result (a view incorrectly included in or excluded from the emitted wireframe/screenshot).

Suggested fix

Either give toWireframe()'s isVisible() call the same synchronized(maskWalkLock) treatment as toScreenshotWireframe(), or make hasGlobalVisibleRect() allocate a local Rect/Point instead of reusing the shared fields (mirroring what globalVisibleRect() already does). The per-call allocation avoided by reuse is a much smaller cost than a data race that can silently misplace or drop a rendered view.

Prompt to fix with AI (copy-paste)

## Context
@posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt#L174-178
@posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt#L1170-1172
@posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt#L1179-1184
@posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt#L1361-1363

<issue_description>
This chunk changes the concurrency contract for the shared `reusableRect`/`reusablePoint` fields: the old comment ("Only accessed from the PixelCopy callback thread (or executor), so no synchronization needed") is replaced with "Shared scratch objects; every user must hold maskWalkLock", and a new `maskWalkLock` is introduced. `toScreenshotWireframe()`'s `view.isVisible()` check and the recursive `findMaskableWidgets()` walk are correctly moved under `synchronized(maskWalkLock)`. However, `View.toWireframe()` (the non-screenshot / wireframe-mode path, unchanged by this PR at what is now approximately line 1361) also calls `view.isVisible()` → `hasGlobalVisibleRect()` → `getGlobalVisibleRect(reusableRect, reusablePoint)` on the same shared fields, with no lock at all. Since `config.sessionReplayConfig.screenshot` is read fresh on every draw (`onDrawCallback`) and can change at runtime via remote config, and since a timed-out PixelCopy callback for one window can keep running on the PixelCopy handler thread after the executor has moved on to the next `generateSnapshot()` call (exactly the overlap this PR's own comment calls out: "a timed-out capture's post-copy walk can overlap the next capture's pre-copy walk on a different thread"), it is possible for a screenshot-mode PixelCopy callback's guarded walk to run concurrently with a wireframe-mode `toWireframe()` call's unguarded walk on the executor thread. `View.getGlobalVisibleRect(Rect, Point)` performs a multi-level read-modify-write of the passed-in Rect as it walks up the parent chain, and the returned boolean itself depends on that intermediate state, so a concurrent write from the other thread can corrupt the intersection and silently return the wrong visibility result (a view incorrectly included in or excluded from the emitted wireframe/screenshot).
</issue_description>

<issue_validation>
- **Checked:** `toWireframe()` (PostHogReplayIntegration.kt:1326-1332 — `if (!view.isVisible()) return null`, unguarded), `isVisible()`→`hasGlobalVisibleRect()` (789, 826-833) which calls `getGlobalVisibleRect(reusableRect, reusablePoint)` on the shared scratch fields, the screenshot path's guards (`synchronized(maskWalkLock)` at 1180-1184, 1219, 1232) and the new invariant comment (line 174: 'every user must hold maskWalkLock'), plus `screenshot` mutability (PostHogSessionReplayConfig.kt:38 `public var`, read fresh at line 655).
- **Found:** The invariant violation is real. The screenshot path acquires `maskWalkLock` around every `reusableRect` access, but `toWireframe()`'s `isVisible()` touches the same shared `reusableRect`/`reusablePoint` with no lock. `getGlobalVisibleRect(Rect, Point)` does a multi-level read-modify-write of the passed Rect while walking the parent chain, and its boolean result depends on that intermediate state, so a concurrent writer corrupts the visibility outcome.
- **Found:** Reachable cross-thread: a screenshot post-walk callback runs on the pixelCopy handler thread holding the lock, while the executor thread can run an unguarded `toWireframe()` walk — the same late-callback overlap the PR's own comment acknowledges (1168-1169). Because `screenshot` is a runtime-mutable var read per capture, a screenshot→wireframe mode transition (or a forceScreenshot/native interleave) can place a guarded screenshot callback and an unguarded wireframe walk on `reusableRect` simultaneously.
- **Impact:** A corrupted visibility boolean can wrongly drop or include a rendered view in the emitted frame; in the tail it can flip a masked view's visibility inside the screenshot mask walk (guarded side corrupted by the unguarded side), risking an unmasked sensitive view. Real race on shared mutable state violating a contract this PR introduces, with a trivial fix (lock the wireframe `isVisible`, or allocate a local Rect in `hasGlobalVisibleRect` mirroring `globalVisibleRect`).
- **Priority:** Lowered should_fix → consider: the defect is real and the fix cheap, but reachability is compound-rare — it needs a runtime capture-mode transition AND a >1s PixelCopy timeout AND the concurrent parent-chain RMW to overlap — and the usual consequence is a single mildly-corrupted frame, so it doesn't warrant should_fix urgency while remaining worth recording.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Either give `toWireframe()`'s `isVisible()` call the same `synchronized(maskWalkLock)` treatment as `toScreenshotWireframe()`, or make `hasGlobalVisibleRect()` allocate a local `Rect`/`Point` instead of reusing the shared fields (mirroring what `globalVisibleRect()` already does). The per-call allocation avoided by reuse is a much smaller cost than a data race that can silently misplace or drop a rendered view.
</potential_solution>

[dustinbyrne]

Validated via test, will fix.

[posthog]

Compose semantics-traversal failure paths never poison the walk, and have zero test coverage

Why we think it's a valid issue

• Checked: findMaskableComposeWidgets (PostHogReplayIntegration.kt:1037-1113) — the (view as? RootForTest)?.semanticsOwner ?: run { ...; return@post } early-return, the catch (Throwable) around getAllSemanticsNodes, the shared finally { latch.countDown() }, and the if (completed) walk.rects.addAll(...) else walk.poisoned = true decision. Also grepped PostHogReplayIntegrationTest.kt.
• Found: The fail-open gap is real and present in shipped code. Both the RootForTest-mismatch early-return AND the caught semantics exception hit finally { latch.countDown() }, so latch.await returns true → completed = true → walk.rects.addAll(maskableWidgets) with the (empty/partial) list, and walk.poisoned is NEVER set. Only the timeout path (else { walk.poisoned = true }) fails closed. So the function fails CLOSED on timeout but fails OPEN on structural traversal failure — an internal inconsistency contradicting the PR's stated 'fail closed on unknowable geometry' thesis.
• Found: Consequence when it fires: on a Compose version where getAllSemanticsNodes throws (the catch exists precisely 'due to compose versioning'), every capture produces empty Compose rects, is not poisoned, and shouldKeepFrame keeps it (empty preWalk.rects == empty postWalk.rects) → all Compose sensitive content (text inputs, images) ships unmasked, consistently, for affected versions. That is a real PII leak, reachable via a foreseeable Compose upgrade the SDK lags — a single systemic condition, not a compound contrived one.
• Found: Test-coverage claim confirmed: PostHogReplayIntegrationTest.kt tests shouldKeepFrame with directly-constructed poisoned MaskWalks (lines 1550-1564) but has NO test that drives findMaskableComposeWidgets through its RootForTest/exception/timeout branches, so neither the intended fail-closed behavior nor a regression on these paths is guarded.
• Impact: A fail-open path on a PII-masking gate, present in the reviewed code and undercutting the PR's own safety model, with a foreseeable (version-gated) but severe and consistent leak when triggered — meets the security keep bar. The fix is concrete (poison on all non-completion branches) and the missing tests reinforce the risk. should_fix is appropriate: real and security-relevant, but version-gated rather than universal.

Issue description

findMaskableComposeWidgets() only sets walk.poisoned = true when the 1s latch.await times out. The (view as? RootForTest)?.semanticsOwner ?: run { ...; return@post } early-return and the catch (e: Throwable) handler around the semantics traversal both still just latch.countDown(), so completed is true and the walk is treated as fully trustworthy with an empty (or partial) rect list. This is exactly the gap already raised in review (marandaneto's comment on this file: 'RootForTest early return and caught semantics exceptions also count down the latch... mark the walk poisoned for every traversal failure, not only timeout or interruption'). Independent of whether that gets fixed, there is currently no test anywhere in PostHogReplayIntegrationTest.kt that exercises findMaskableComposeWidgets at all — not the new timeout-poisoning behavior this PR added, not the RootForTest-mismatch branch, not the exception branch. A Compose screen whose semantics traversal fails structurally (not just times out) would silently ship unmasked PII, and no test would catch a regression here or confirm the intended fail-closed behavior.

Suggested fix

Add unit/E2E coverage for findMaskableComposeWidgets' failure branches: (1) a view that is not a RootForTest reaching this method (early-return path) should poison the walk, with a test asserting shouldKeepFrame ultimately discards; (2) a thrown exception during getAllSemanticsNodes/traversal should poison the walk; (3) keep the existing-but-currently-absent timeout case tested too (e.g. by injecting a main-thread post that never completes within the latch window). At minimum, extend the completed check to also fail closed on the two silently-swallowed branches, and add a test proving each one discards the frame.

Prompt to fix with AI (copy-paste)

## Context
@posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt#L1044-1049
@posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt#L1095-1112

<issue_description>
findMaskableComposeWidgets() only sets walk.poisoned = true when the 1s latch.await times out. The `(view as? RootForTest)?.semanticsOwner ?: run { ...; return@post }` early-return and the `catch (e: Throwable)` handler around the semantics traversal both still just `latch.countDown()`, so `completed` is true and the walk is treated as fully trustworthy with an empty (or partial) rect list. This is exactly the gap already raised in review (marandaneto's comment on this file: 'RootForTest early return and caught semantics exceptions also count down the latch... mark the walk poisoned for every traversal failure, not only timeout or interruption'). Independent of whether that gets fixed, there is currently no test anywhere in PostHogReplayIntegrationTest.kt that exercises findMaskableComposeWidgets at all — not the new timeout-poisoning behavior this PR added, not the RootForTest-mismatch branch, not the exception branch. A Compose screen whose semantics traversal fails structurally (not just times out) would silently ship unmasked PII, and no test would catch a regression here or confirm the intended fail-closed behavior.
</issue_description>

<issue_validation>
- **Checked:** `findMaskableComposeWidgets` (PostHogReplayIntegration.kt:1037-1113) — the `(view as? RootForTest)?.semanticsOwner ?: run { ...; return@post }` early-return, the `catch (Throwable)` around `getAllSemanticsNodes`, the shared `finally { latch.countDown() }`, and the `if (completed) walk.rects.addAll(...) else walk.poisoned = true` decision. Also grepped PostHogReplayIntegrationTest.kt.
- **Found:** The fail-open gap is real and present in shipped code. Both the RootForTest-mismatch early-return AND the caught semantics exception hit `finally { latch.countDown() }`, so `latch.await` returns true → `completed = true` → `walk.rects.addAll(maskableWidgets)` with the (empty/partial) list, and `walk.poisoned` is NEVER set. Only the timeout path (`else { walk.poisoned = true }`) fails closed. So the function fails CLOSED on timeout but fails OPEN on structural traversal failure — an internal inconsistency contradicting the PR's stated 'fail closed on unknowable geometry' thesis.
- **Found:** Consequence when it fires: on a Compose version where `getAllSemanticsNodes` throws (the catch exists precisely 'due to compose versioning'), every capture produces empty Compose rects, is not poisoned, and `shouldKeepFrame` keeps it (empty preWalk.rects == empty postWalk.rects) → all Compose sensitive content (text inputs, images) ships unmasked, consistently, for affected versions. That is a real PII leak, reachable via a foreseeable Compose upgrade the SDK lags — a single systemic condition, not a compound contrived one.
- **Found:** Test-coverage claim confirmed: PostHogReplayIntegrationTest.kt tests `shouldKeepFrame` with directly-constructed poisoned MaskWalks (lines 1550-1564) but has NO test that drives `findMaskableComposeWidgets` through its RootForTest/exception/timeout branches, so neither the intended fail-closed behavior nor a regression on these paths is guarded.
- **Impact:** A fail-open path on a PII-masking gate, present in the reviewed code and undercutting the PR's own safety model, with a foreseeable (version-gated) but severe and consistent leak when triggered — meets the security keep bar. The fix is concrete (poison on all non-completion branches) and the missing tests reinforce the risk. should_fix is appropriate: real and security-relevant, but version-gated rather than universal.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Add unit/E2E coverage for findMaskableComposeWidgets' failure branches: (1) a view that is not a RootForTest reaching this method (early-return path) should poison the walk, with a test asserting shouldKeepFrame ultimately discards; (2) a thrown exception during getAllSemanticsNodes/traversal should poison the walk; (3) keep the existing-but-currently-absent timeout case tested too (e.g. by injecting a main-thread post that never completes within the latch window). At minimum, extend the `completed` check to also fail closed on the two silently-swallowed branches, and add a test proving each one discards the frame.
</potential_solution>

[dustinbyrne]

Validated via test, will fix.

[posthog]

isRenderedButUnplaceable() poisons the walk for any hasTransientState() view regardless of maskability, regressing common RecyclerView list-animation screens

Why we think it's a valid issue

• Checked: isRenderedButUnplaceable() (PostHogReplayIntegration.kt:1029-1033), its poison call site (1013-1018), isViewStateStableForMatrixOperations() (871-895, !hasTransientState() at line 880), isVisible() (765-803), and shouldKeepFrame's preWalk.poisoned || postWalk.poisoned -> false (1159). Cross-referenced the PR description's deletion of the fix: stop screenshot frames being dropped during animations #529 hasTransientState exemption.
• Found: Mechanics confirmed and maskability-agnostic. Any child with hasTransientState()==true → isViewStateStableForMatrixOperations() false → isVisible() false → isRenderedButUnplaceable() true (visibility==VISIBLE && w/h>0 && !stable) → walk.poisoned = true → whole frame discarded — with no check that the view or its subtree contains anything maskable.
• Found: This is a regression, not just a non-improvement. ViewPropertyAnimator (which RecyclerView's DefaultItemAnimator uses for add/remove/move/change) sets transient state for the animation's duration — the PR's own analysis confirms setHasTransientState is driven by ViewPropertyAnimator. Pre-PR the fix: stop screenshot frames being dropped during animations #529 hasTransientState exemption KEPT such frames; this PR deletes that exemption AND adds the poison, flipping them to discard. The PR's stated justification ('frames they kept have stable geometry and pass rect equality anyway') does not hold here: a transient-state view fails isVisible() and is POISONED before rect-equality is ever evaluated, so stable-geometry transient-state content is discarded, not rescued by rect equality.
• Found: Breadth is amplified by transient-state propagation — Android propagates transient state up the hierarchy (for recycling), so an animating descendant marks ancestor containers hasTransientState()==true too, poisoning the walk high in the tree. So essentially any ViewPropertyAnimator-driven animation anywhere in the window (RecyclerView item animations, FAB show/hide, Snackbar, many transitions) discards the whole capture for its duration — extremely common patterns.
• Impact: Intermittent frame discards during routine animations on lists/feeds/chats and other common screens — a safe (fail-closed, no PII) but real efficacy regression from prior behavior, directly undercutting the PR's goal, and plausibly a blind spot (the hasTransientState poison path is also untested per the sibling finding). Meets the keep bar as a behavioral regression real inputs will frequently hit.
• Note on fix: The suggested narrowing needs care — an unplaceable ViewGroup can hide maskable descendants that can't be safely enumerated, so naively skipping poison for non-'maskable-type' views would reintroduce the unmasked-PII hole this PR closed; a safe narrowing is limited to definitively non-maskable leaf views.
• Priority: Keeping should_fix — common and broad regression tied directly to the PR's changes, though safe and transient (brief animations, throttled captures) rather than a permanent blackout.

Issue description

isRenderedButUnplaceable() (new) fires whenever a child fails isVisible() while still having visibility == VISIBLE and non-zero width/height, and isVisible() routes through the unchanged isViewStateStableForMatrixOperations(), which returns false whenever hasTransientState() is true — with no requirement that the view is actually a masking candidate. AndroidX RecyclerView's item animators (DefaultItemAnimator and friends) call setHasTransientState(true) on a ViewHolder's itemView for the duration of add/remove/change/move animations — a routine, frequent event on essentially any list, feed, or chat screen (list diffing via notifyItemChanged/DiffUtil, swipe-to-dismiss, etc.), not an exotic case. This happens independently of whether the animating row contains masked content: a completely unrelated, non-sensitive thumbnail or divider mid-animation still satisfies visibility==VISIBLE && width>0 && height>0 && hasTransientState()==true, so it poisons the whole walk and the entire frame is discarded via shouldKeepFrame's preWalk.poisoned || postWalk.poisoned -> false branch. Pre-PR, this same hasTransientState()-driven isVisible()==false path only caused that one view to be silently skipped when collecting maskable rects (per the PR's own description: 'previously such views were silently pruned from the walk, which would have shipped them unmasked') — a narrower risk scoped to that view's own masking. Post-PR it escalates to a full-frame discard for every capture attempt while any list item anywhere on screen is mid-animation, which is a much broader and more frequent trigger than the Lottie/GIF/spinner cases this PR was written to fix, and works against its own stated goal for one of the most common Android UI patterns.

Suggested fix

Scope the fail-closed poisoning to views that could plausibly need masking (e.g. only poison when the unplaceable view is a TextView/EditText/ImageView/WebView/Compose-hosting subtree, or otherwise apply the same masking-candidate heuristics used elsewhere in findMaskableWidgets before deciding to poison), so an animating decorative RecyclerView row doesn't discard frames that have nothing to protect. Add a regression test with a RecyclerView item mid DefaultItemAnimator animation (hasTransientState()==true, non-sensitive content) to confirm the frame is still kept.

Prompt to fix with AI (copy-paste)

## Context
@posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt#L1015-1018
@posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt#L1027-1032

<issue_description>
isRenderedButUnplaceable() (new) fires whenever a child fails isVisible() while still having visibility == VISIBLE and non-zero width/height, and isVisible() routes through the unchanged isViewStateStableForMatrixOperations(), which returns false whenever hasTransientState() is true — with no requirement that the view is actually a masking candidate. AndroidX RecyclerView's item animators (DefaultItemAnimator and friends) call setHasTransientState(true) on a ViewHolder's itemView for the duration of add/remove/change/move animations — a routine, frequent event on essentially any list, feed, or chat screen (list diffing via notifyItemChanged/DiffUtil, swipe-to-dismiss, etc.), not an exotic case. This happens independently of whether the animating row contains masked content: a completely unrelated, non-sensitive thumbnail or divider mid-animation still satisfies visibility==VISIBLE && width>0 && height>0 && hasTransientState()==true, so it poisons the whole walk and the entire frame is discarded via shouldKeepFrame's `preWalk.poisoned || postWalk.poisoned -> false` branch. Pre-PR, this same hasTransientState()-driven isVisible()==false path only caused that one view to be silently skipped when collecting maskable rects (per the PR's own description: 'previously such views were silently pruned from the walk, which would have shipped them unmasked') — a narrower risk scoped to that view's own masking. Post-PR it escalates to a full-frame discard for every capture attempt while any list item anywhere on screen is mid-animation, which is a much broader and more frequent trigger than the Lottie/GIF/spinner cases this PR was written to fix, and works against its own stated goal for one of the most common Android UI patterns.
</issue_description>

<issue_validation>
- **Checked:** `isRenderedButUnplaceable()` (PostHogReplayIntegration.kt:1029-1033), its poison call site (1013-1018), `isViewStateStableForMatrixOperations()` (871-895, `!hasTransientState()` at line 880), `isVisible()` (765-803), and `shouldKeepFrame`'s `preWalk.poisoned || postWalk.poisoned -> false` (1159). Cross-referenced the PR description's deletion of the #529 `hasTransientState` exemption.
- **Found:** Mechanics confirmed and maskability-agnostic. Any child with `hasTransientState()==true` → `isViewStateStableForMatrixOperations()` false → `isVisible()` false → `isRenderedButUnplaceable()` true (visibility==VISIBLE && w/h>0 && !stable) → `walk.poisoned = true` → whole frame discarded — with no check that the view or its subtree contains anything maskable.
- **Found:** This is a regression, not just a non-improvement. `ViewPropertyAnimator` (which RecyclerView's DefaultItemAnimator uses for add/remove/move/change) sets transient state for the animation's duration — the PR's own analysis confirms `setHasTransientState` is driven by `ViewPropertyAnimator`. Pre-PR the #529 `hasTransientState` exemption KEPT such frames; this PR deletes that exemption AND adds the poison, flipping them to discard. The PR's stated justification ('frames they kept have stable geometry and pass rect equality anyway') does not hold here: a transient-state view fails `isVisible()` and is POISONED before rect-equality is ever evaluated, so stable-geometry transient-state content is discarded, not rescued by rect equality.
- **Found:** Breadth is amplified by transient-state propagation — Android propagates transient state up the hierarchy (for recycling), so an animating descendant marks ancestor containers `hasTransientState()==true` too, poisoning the walk high in the tree. So essentially any ViewPropertyAnimator-driven animation anywhere in the window (RecyclerView item animations, FAB show/hide, Snackbar, many transitions) discards the whole capture for its duration — extremely common patterns.
- **Impact:** Intermittent frame discards during routine animations on lists/feeds/chats and other common screens — a safe (fail-closed, no PII) but real efficacy regression from prior behavior, directly undercutting the PR's goal, and plausibly a blind spot (the hasTransientState poison path is also untested per the sibling finding). Meets the keep bar as a behavioral regression real inputs will frequently hit.
- **Note on fix:** The suggested narrowing needs care — an unplaceable ViewGroup can hide maskable descendants that can't be safely enumerated, so naively skipping poison for non-'maskable-type' views would reintroduce the unmasked-PII hole this PR closed; a safe narrowing is limited to definitively non-maskable leaf views.
- **Priority:** Keeping should_fix — common and broad regression tied directly to the PR's changes, though safe and transient (brief animations, throttled captures) rather than a permanent blackout.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Scope the fail-closed poisoning to views that could plausibly need masking (e.g. only poison when the unplaceable view is a TextView/EditText/ImageView/WebView/Compose-hosting subtree, or otherwise apply the same masking-candidate heuristics used elsewhere in findMaskableWidgets before deciding to poison), so an animating decorative RecyclerView row doesn't discard frames that have nothing to protect. Add a regression test with a RecyclerView item mid DefaultItemAnimator animation (hasTransientState()==true, non-sensitive content) to confirm the frame is still kept.
</potential_solution>

[dustinbyrne]

Couldn't validate this via test: propagated transient state is rejected by the existing root visibility gate on both the pre-PR base and current head, so the claimed new regression did not reproduce.

[dustinbyrne]

Verified: I ran the Android sample app in screenshot-based session replay mode with a continuously animated loading spinner visible. Before the patch, every capture attempt during the animation was discarded and no replay frames were sent; after the patch, frames were captured continuously with no discard errors, and the animation was represented by changing screenshot payloads.

[marandaneto]

Verified: I ran the Android sample app in screenshot-based session replay mode with a continuously animated loading spinner visible. Before the patch, every capture attempt during the animation was discarded and no replay frames were sent; after the patch, frames were captured continuously with no discard errors, and the animation was represented by changing screenshot payloads.

awesome, @dustinbyrne @arnohillen worth testing this on react native as well since react native relies on that and sometimes there are some incompatibilities