Skip to content

feat(replay): default screenshotScale to null so native screens use the Android SDK default - #689

Open
ioannisj wants to merge 2 commits into
mainfrom
feat/replay-nullable-screenshot-scale
Open

ioannisj wants to merge 2 commits into
mainfrom
feat/replay-nullable-screenshot-scale

Conversation

@ioannisj

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

Follow-up to PostHog/posthog-android#906, released as posthog-android 3.74.0. That change made sessionReplayConfig.screenshotScale nullable on Android, with null as the default meaning one pixel per dp instead of native resolution.

Flutter screens were already fine: screenshot_capturer.dart captures them in Dart at one pixel per logical pixel. Native screens recorded with captureNativeScreens didn't get the new default though, because the Dart option defaulted to 1.0 and was always sent over the method channel, which pinned them to physical resolution.

Public API change: PostHogSessionReplayConfig.screenshotScale changes from double to double?.

  • null is the new default and is not sent to the native SDK, so native screens pick up posthog-android's one-pixel-per-dp default. Flutter screens still capture at 1.0.
  • An explicit value keeps its meaning (a multiplier of the logical size for Flutter screens and of the physical size for native ones), clamped to 0.1–1.0. screenshotScale = 1.0 keeps native screens at physical resolution.
  • NaN and infinity now reset to null instead of 1.0.
  • Code that reads the value into a double needs a null check.

The option is @experimental and posthog-android shipped the same type change as a minor, so I went with a minor here too and flagged the breaking type change in the changeset. Happy to make it a major if anyone feels otherwise.

Also bumps the posthog-android floor to [3.74.0,4.0.0). The Kotlin plugin already only sets screenshotScale when the key is present (PosthogFlutterPlugin.kt:662), so no plugin change was needed.

iOS doesn't read screenshotScale on Flutter, so this is Android only. No sdk-spec covers screenshot scale.

💚 How did you test it?

  • flutter test on replay_screenshot_config_test.dart, replay_screenshot_capture_test.dart and event_shape_snapshot_test.dart: 23 pass. New cases cover the default not being sent, an explicit 1.0 being sent, NaN/infinity/null resetting to null, and a null scale producing the same Flutter frame as 1.0.
  • ./gradlew :posthog_flutter:testDebugUnitTest against 3.74.0: 61 pass, including setup_screenshotControls_preserveDefaults now asserting null when the key is absent.
  • make analyzeDart, make checkFormatDart, make updateApiDart (the snapshot changes one line, double to double?).
  • I haven't run the native-screen path on a device after the change.

📝 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

DRI: @ioannisj
Autonomy: Human-driven (agent-assisted)

@ioannisj ioannisj self-assigned this Oct 30, 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 9070c4a: 1 Should fix.

This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.

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 .changeset/fresh-apples-knock.md Resolved
@github-actions

Copy link
Copy Markdown
Contributor

posthog-flutter Compliance Report

Date: 2026-10-30 14:40:56 UTC
Duration: 96431ms

✅ All Tests Passed!

45/45 tests passed


Capture Tests

✅ 29/29 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields ✅ 125ms
Format Validation.Event Has Uuid ✅ 111ms
Format Validation.Event Has Lib Properties ✅ 110ms
Format Validation.Distinct Id Is String ✅ 110ms
Format Validation.Token Is Present ✅ 110ms
Format Validation.Custom Properties Preserved ✅ 113ms
Format Validation.Event Has Timestamp ✅ 112ms
Retry Behavior.Retries On 503 ✅ 5323ms
Retry Behavior.Does Not Retry On 400 ✅ 2111ms
Retry Behavior.Does Not Retry On 401 ✅ 2113ms
Retry Behavior.Respects Retry After Header ✅ 8119ms
Retry Behavior.Implements Backoff ✅ 15438ms
Retry Behavior.Retries On 500 ✅ 5220ms
Retry Behavior.Retries On 502 ✅ 5217ms
Retry Behavior.Retries On 504 ✅ 5217ms
Retry Behavior.Max Retries Respected ✅ 15439ms
Deduplication.Generates Unique Uuids ✅ 117ms
Deduplication.Preserves Uuid On Retry ✅ 5218ms
Deduplication.Preserves Uuid And Timestamp On Retry ✅ 10330ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry ✅ 5223ms
Deduplication.No Duplicate Events In Batch ✅ 114ms
Deduplication.Different Events Have Different Uuids ✅ 111ms
Compression.Sends Gzip When Enabled ✅ 111ms
Batch Format.Uses Proper Batch Structure ✅ 109ms
Batch Format.Flush With No Events Sends Nothing ✅ 106ms
Batch Format.Multiple Events Batched Together ✅ 116ms
Error Handling.Does Not Retry On 403 ✅ 2110ms
Error Handling.Does Not Retry On 413 ✅ 2113ms
Error Handling.Retries On 408 ✅ 5220ms

Feature_Flags Tests

✅ 16/16 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id ✅ 9ms
Request Payload.Flags Request Uses V2 Query Param ✅ 5ms
Request Payload.Flags Request Hits Flags Path Not Decide ✅ 6ms
Request Payload.Flags Request Omits Authorization Header ✅ 5ms
Request Payload.Token In Flags Body Matches Init ✅ 6ms
Request Payload.Groups Round Trip ✅ 6ms
Request Payload.Groups Default To Empty Object ✅ 6ms
Request Payload.Person Properties Distinct Id Auto Populated When Caller Omits It ✅ 5ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False ✅ 5ms
Request Payload.Disable Geoip Omitted Defaults To False ✅ 6ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key ✅ 5ms
Request Lifecycle.No Flags Request On Init Alone ✅ 2ms
Request Lifecycle.No Flags Request On Normal Capture ✅ 107ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests ✅ 11ms
Request Lifecycle.Mock Response Value Is Returned To Caller ✅ 5ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event ✅ 109ms

@ioannisj
ioannisj marked this pull request as ready for review October 30, 2026 18:06
@ioannisj
ioannisj requested a review from a team as a code owner October 30, 2026 18:06

@turnipdabeets turnipdabeets 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.

Shape looks right: double? with a null default matches Android, and the key being left out means native screens pick up the new default while Flutter frames stay the same. Re-ran the three replay/snapshot test files at this head and they pass. Native-screen path has not had a device run yet, not blocking from my side.

@@ -0,0 +1,5 @@
---
"posthog_flutter": minor

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.

[suggestion] I would keep this as a minor, same as Android: the option is @experimental, only a few weeks old, and only code reading the getter breaks. RELEASING.md has no experimental exception though, so could you reply on the bot thread with that reasoning so the call is on record?

"posthog_flutter": minor
---

**Breaking:** Change `PostHogSessionReplayConfig.screenshotScale` from `double` to `double?` with a `null` default, so native screens recorded with `captureNativeScreens` on Android pick up posthog-android's one-pixel-per-dp default instead of physical resolution. Flutter screens still capture at one pixel per logical pixel. Set `screenshotScale: 1.0` to keep native screens at physical resolution. Code that reads the value into a `double` needs a null check.

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.

[nit] screenshotScale: 1.0 reads like a named parameter, but this is a property, so screenshotScale = 1.0?

testImplementation 'junit:junit:4.13.2'
testRuntimeOnly 'org.junit.vintage:junit-vintage-engine:5.10.1'
implementation 'com.posthog:posthog-android:[3.72.9,4.0.0)'
implementation 'com.posthog:posthog-android:[3.74.0,4.0.0)'

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.

[nit] Worth making the floor 3.74.2 to match React Native? It picks up the masking fixes for anyone on a lockfile. Fresh builds already resolve to it through the open range, so fine to skip.

This branch has not been deployed

No deployments
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.

2 participants