Skip to content

fix(replay): fill screenshot masks to the full rectangle - #969

Merged
turnipdabeets merged 6 commits into
mainfrom
fix/replay-square-mask-fill
Oct 27, 2026
Merged

turnipdabeets merged 6 commits into
mainfrom
fix/replay-square-mask-fill

Conversation

@turnipdabeets

@turnipdabeets turnipdabeets commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

Screenshot replay painted each mask as a rounded rect (UIBezierPath(roundedRect: rect, cornerRadius: 10)). UIKit draws that as a continuous corner about 13–14px across, so the pixels in the four corners of every masked region weren't covered and were uploaded. #873 (4.0) makes screenshot mode mandatory, so every masked region in every recording has this gap. This PR fills the whole rect (UIRectFill(rect)).

Alternative, if we want to keep the rounded look: grow each mask a little and keep the 10pt rounded fill, so the corner arc falls outside the original rect:

UIBezierPath(roundedRect: rect.insetBy(dx: -4, dy: -4), cornerRadius: 10).fill()

A 10pt radius covers the original corner once each side grows by about 2.9pt (10 × (1 − 1/√2)). 4pt leaves room for UIKit's continuous corners. The cost is that every mask is 4pt bigger on each side, so nearby masks can merge and more of the surrounding UI gets covered. It would also need the goldens re-recorded again. This PR goes with the square fill because it's simpler and covers exactly the rect. Wireframe mode never had mask shapes (masked text became *** and sensitive images were dropped), so the rounded boxes were only ever a screenshot-mode look.

This came out of the bot review thread on #873: #873 (comment). The fix lands on main and will merge forward into the v4 branch.

Spec / parity:

  • sdk-specs session-replay-privacy says native screenshot replay "paints opaque masks over screenshots" for the computed mask rectangles. It doesn't specify a corner shape, so a full-rect fill is the conforming behavior.
  • posthog-flutter already paints square masks (canvas.drawRect), so iOS now matches it.
  • Divergence: posthog-android still uses canvas.drawRoundRect(maskRect, 10f * scaleX, 10f * scaleY, paint) in PostHogReplayIntegration.kt, so it has the same corner gap. It isn't changed here.

💚 How did you test it?

  • New regression test (RRWireframeTest › maskImage, iOS): checks that all 4 corner pixels of a mask rect are filled, and that the pixel just outside each edge isn't.
    • It fails on the old rounded fill (all 4 corners unmasked) and passes with the fix.
    • It also fails if the mask grows by 1px (checked by temporarily changing the fill).
  • Masking goldens are re-recorded by CI on the pinned Xcode 26.3 / iOS 26.2 (MASK_SNAPSHOT_XCODE / MASK_SNAPSHOT_OS), because that toolchain isn't installed locally.
    • A temporary commit set the snapshot tolerance to 0 so CI rendered every changed case and uploaded the actual PNGs. A later commit reverts it.
    • The actual PNGs are written with the same write(composite) call recordGoldens() uses.
  • Golden diff review: 21 of 24 goldens changed. The 3 unchanged ones (15, 16, 19) are the scenarios where nothing is masked.
    • For every changed case, a pixel-level check confirmed that each change darkens a pixel in the masked panel and sits inside a corner blob of at most 14×14px. There are exactly 4 blobs per mask.
    • The only other differences are antialiasing changes of 2 levels or less on fractional mask edges.
    • Renders were identical across two CI runs.
  • make lint passes.
  • Focused iOS simulator tests pass on iOS 26.4: RRWireframeTest, PostHogMaskFailClosedTest, PostHogReactNativeMaskingTest (39 tests).
  • make buildSdk couldn't run locally because xcpretty isn't installed. The CI Build SDK jobs cover it.

📝 Checklist

  • All commits are signed (see Commit signing)
  • 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)

  • Claude Code (Opus 5.5) wrote the change, directed by turnipdabeets. The pre-PR review loop ran with Claude Code review subagents.
  • Kept the change to the fill call. No mask geometry, padding or rect discovery changed.
  • I recorded the goldens from CI renders on the pinned runtime rather than bumping the pins, so the toolchain stays out of this diff. The temporary tolerance-0 commit and its revert should go away when this is squash-merged.

🤖 Generated with Claude Code

Masks were painted as 10pt rounded rects, so the pixels in each mask's
four corners stayed visible in the uploaded screenshot. Fill the whole
rect instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@turnipdabeets
turnipdabeets requested a review from a team as a code owner October 27, 2026 15:57
@turnipdabeets turnipdabeets self-assigned this Oct 27, 2026

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed d3bea8c: 1 Should fix.

See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.

Comment thread PostHog/Replay/RRWireframe.swift Resolved
turnipdabeets and others added 2 commits October 27, 2026 12:10
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… (reverted next)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

posthog-ios Compliance Report

Date: 2026-10-27T16:48:59.613969+00:00
Duration: 241095ms

⚠️ Some Tests Failed

46/47 tests passed, 1 failed


Capture Tests

⚠️ 29/30 tests passed, 1 failed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 3104ms
Format Validation.Event Has Uuid ✅ 2934ms
Format Validation.Event Has Lib Properties ✅ 2900ms
Format Validation.Distinct Id Is String ✅ 2972ms
Format Validation.Token Is Present ✅ 2904ms
Format Validation.Custom Properties Preserved ✅ 3003ms
Format Validation.Event Has Timestamp ✅ 2848ms
Format Validation.Non Utc Event Timestamp Is Converted To Utc ✅ 2947ms
Retry Behavior.Retries On 503 ✅ 11980ms
Retry Behavior.Does Not Retry On 400 ✅ 4800ms
Retry Behavior.Does Not Retry On 401 ✅ 4903ms
Retry Behavior.Respects Retry After Header ✅ 7825ms
Retry Behavior.Implements Backoff ✅ 21417ms
Retry Behavior.Retries On 500 ✅ 8830ms
Retry Behavior.Retries On 502 ✅ 5375ms
Retry Behavior.Retries On 504 ✅ 9257ms
Retry Behavior.Max Retries Respected ❌ 19164ms
Deduplication.Generates Unique Uuids ✅ 2992ms
Deduplication.Preserves Uuid On Retry ✅ 5412ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 16836ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 7789ms
Deduplication.No Duplicate Events In Batch ✅ 3169ms
Deduplication.Different Events Have Different Uuids ✅ 3067ms
Compression.Sends Gzip When Enabled ✅ 3140ms
Batch Format.Uses Proper Batch Structure ✅ 2855ms
Batch Format.Flush With No Events Sends Nothing ✅ 446ms
Batch Format.Multiple Events Batched Together ✅ 3163ms
Error Handling.Does Not Retry On 403 ✅ 4957ms
Error Handling.Does Not Retry On 413 ✅ 5004ms
Error Handling.Retries On 408 ✅ 9442ms

Failures

retry_behavior.max_retries_respected

Expected 4 requests, got 6

Feature_Flags Tests

✅ 17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 2957ms
Request Payload.Flags Request Uses V2 Query Param ✅ 3044ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 2997ms
Request Payload.Flags Request Omits Authorization Header ✅ 3008ms
Request Payload.Token In Flags Body Matches Init ✅ 3001ms
Request Payload.Groups Round Trip ✅ 3005ms
Request Payload.Groups Default To Empty Object ✅ 2997ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 3035ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 2972ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 2996ms
Request Lifecycle.No Flags Request On Init Alone ✅ 137ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 2900ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 5802ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 2981ms
Retry Behavior.Retries Flags On 502 ✅ 3007ms
Retry Behavior.Retries Flags On 504 ✅ 3144ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 3157ms

turnipdabeets and others added 3 commits October 27, 2026 12:24
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rendered by CI on the pinned Xcode 26.3 / iOS 26.2 runtime, which isn't
installed locally. Every changed pixel is inside a mask's corner. The 3
unchanged cases mask nothing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@dustinbyrne dustinbyrne left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The full-rectangle fill closes the screenshot-mask corner gap without changing discovery, encoding or public APIs. The focused corner/edge regression and refreshed pinned goldens cover the change; the earlier golden-image finding is resolved.

AI-assisted review.

@turnipdabeets
turnipdabeets merged commit 05c1201 into main Oct 27, 2026
51 of 52 checks passed
@turnipdabeets
turnipdabeets deleted the fix/replay-square-mask-fill branch October 27, 2026 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants