fix: safer progressive loading #40768

pauldambra · 2025-11-01T11:37:49Z

had to get this out of my head while #4 is watching TV

the timestamp range of rrweb data in blob data returned from the API
does not match the playable time range in the data returned from the API
which means a blob can advertise it covers up to time X
but actually it falls short
so, we can't let the snapshotDataLogic manage progressive loading based on desired timestamp
this is why as we fixed bugs in progressive loading we got more and more reports of endless buffering because we increased the likelihood we'd be endlessly waiting on data because of the timestamp mismatch
instead the player already knows how much data is playably loaded and where it is up to
so it can just ping for the next snapshot source

there are problems with this

current player timestamp does not monotonically increase
in fact it pretty often jumps towards the end of the recording for a tick or two and then back
this means we load more data than we need to - not necessarily a bad thing since we'd rather load more than not at all 🙈

i've wrapped this in a flag so we can turn it off easily!

pauldambra · 2025-11-01T11:38:07Z

fix: trigger recalculation when snapshots change #40772
fix: safer progressive loading #40768 👈 (View in Graphite)
master

This stack of pull requests is managed by Graphite. Learn more about stacking.

github-actions · 2025-11-01T11:44:01Z

Size Change: +56 B (0%)

Total Size: 3.34 MB

ℹ️ View Unchanged

Filename	Size	Change
`frontend/dist/toolbar.js`	3.34 MB	+56 B (0%)

_{compressed-size-action}

github-actions · 2025-11-01T12:13:51Z

⚠️ Flapping Screenshots Detected

The following screenshots have been updated multiple times by the bot, indicating test instability:

playwright/__snapshots__/pageview-trends-insight.png

What this means:

These screenshots are changing on each test run (timing issues, rendering differences, etc.)
This prevents reliable verification and blocks auto-merge
Human intervention is required

How to fix:

Investigate flakiness: Check test timing, wait for elements, stabilize animations
Fix the underlying issue: Don't just update snapshots repeatedly
Verify locally: Run tests multiple times to ensure stability
Push your fix: This will reset the flap counter

If you need to proceed anyway:

Fix the test flakiness first (recommended)
Or manually verify snapshots are acceptable and revert the bot commits, then push a fix

The workflow has been failed to prevent merging unstable tests.

posthog-bot · 2025-11-01T17:54:58Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 15)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2025-11-01T18:03:42Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 15)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

github-actions · 2025-11-01T18:07:01Z

⚠️ Flapping Screenshots Detected

The following screenshots have been updated multiple times by the bot, indicating test instability:

playwright/__snapshots__/pageview-trends-insight.png

What this means:

These screenshots are changing on each test run (timing issues, rendering differences, etc.)
This prevents reliable verification and blocks auto-merge
Human intervention is required

How to fix:

Investigate flakiness: Check test timing, wait for elements, stabilize animations
Fix the underlying issue: Don't just update snapshots repeatedly
Verify locally: Run tests multiple times to ensure stability
Push your fix: This will reset the flap counter

If you need to proceed anyway:

Fix the test flakiness first (recommended)
Or manually verify snapshots are acceptable and revert the bot commits, then push a fix

The workflow has been failed to prevent merging unstable tests.

github-actions · 2025-11-01T18:16:13Z

⚠️ Flapping Screenshots Detected

The following screenshots have been updated multiple times by the bot, indicating test instability:

playwright/__snapshots__/pageview-trends-insight.png

What this means:

These screenshots are changing on each test run (timing issues, rendering differences, etc.)
This prevents reliable verification and blocks auto-merge
Human intervention is required

How to fix:

Investigate flakiness: Check test timing, wait for elements, stabilize animations
Fix the underlying issue: Don't just update snapshots repeatedly
Verify locally: Run tests multiple times to ensure stability
Push your fix: This will reset the flap counter

If you need to proceed anyway:

Fix the test flakiness first (recommended)
Or manually verify snapshots are acceptable and revert the bot commits, then push a fix

The workflow has been failed to prevent merging unstable tests.

posthog-bot · 2025-11-01T20:21:37Z

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

chromium: 0 added, 3 modified, 0 deleted (diff for shard 8, diff for shard 15)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

greptile-apps

_{4 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2025-11-01T20:26:44Z

frontend/src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts

+        percentPlayedOfBuffered: [
+            (s) => [s.dataBufferedUntilPlayerTime, s.currentPlayerTime],
+            (dataBufferedUntilPlayerTime, currentPlayerTime) => {
+                const bufferedSeconds = Math.floor(dataBufferedUntilPlayerTime / 1000)
+                const playerSeconds = Math.floor(currentPlayerTime / 1000)
+                return playerSeconds / bufferedSeconds
+            },


logic: division by zero when bufferedSeconds is 0 (early recording start). returns Infinity or NaN, causing line 1577's check to always pass and trigger excessive loading

Suggested change

percentPlayedOfBuffered: [

(s) => [s.dataBufferedUntilPlayerTime, s.currentPlayerTime],

(dataBufferedUntilPlayerTime, currentPlayerTime) => {

const bufferedSeconds = Math.floor(dataBufferedUntilPlayerTime / 1000)

const playerSeconds = Math.floor(currentPlayerTime / 1000)

return playerSeconds / bufferedSeconds

},

percentPlayedOfBuffered: [

(s) => [s.dataBufferedUntilPlayerTime, s.currentPlayerTime],

(dataBufferedUntilPlayerTime, currentPlayerTime) => {

const bufferedSeconds = Math.floor(dataBufferedUntilPlayerTime / 1000)

const playerSeconds = Math.floor(currentPlayerTime / 1000)

return bufferedSeconds === 0 ? 0 : playerSeconds / bufferedSeconds

},

],

Prompt To Fix With AI

This is a comment left during a code review. Path: frontend/src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts Line: 889:895 Comment: **logic:** division by zero when `bufferedSeconds` is 0 (early recording start). returns `Infinity` or `NaN`, causing line 1577's check to always pass and trigger excessive loading ```suggestion percentPlayedOfBuffered: [ (s) => [s.dataBufferedUntilPlayerTime, s.currentPlayerTime], (dataBufferedUntilPlayerTime, currentPlayerTime) => { const bufferedSeconds = Math.floor(dataBufferedUntilPlayerTime / 1000) const playerSeconds = Math.floor(currentPlayerTime / 1000) return bufferedSeconds === 0 ? 0 : playerSeconds / bufferedSeconds }, ], ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2025-11-01T20:26:44Z

frontend/src/scenes/session-recordings/player/snapshotDataLogic.tsx

                if (!cache.snapshotsBySource) {
                    return {}
                }
+                // TODO: if we change the data without changing the object instance then dependents don't recalculate


logic: mutating cache.snapshotsBySource without creating new object instance prevents selector recalculation, potentially causing stale UI state

Prompt To Fix With AI

This is a comment left during a code review. Path: frontend/src/scenes/session-recordings/player/snapshotDataLogic.tsx Line: 358:358 Comment: **logic:** mutating `cache.snapshotsBySource` without creating new object instance prevents selector recalculation, potentially causing stale UI state How can I resolve this? If you propose a fix, please make it concise.

posthog-bot · 2025-11-01T20:32:41Z

📸 UI snapshots have been updated

4 snapshot changes in total. 0 added, 4 modified, 0 deleted:

chromium: 0 added, 4 modified, 0 deleted (diff for shard 14, diff for shard 8)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

github-actions · 2025-11-01T20:34:07Z

⚠️ Flapping Screenshots Detected

The following screenshots have been updated multiple times by the bot, indicating test instability:

playwright/__snapshots__/pageview-trends-insight.png

What this means:

These screenshots are changing on each test run (timing issues, rendering differences, etc.)
This prevents reliable verification and blocks auto-merge
Human intervention is required

How to fix:

Investigate flakiness: Check test timing, wait for elements, stabilize animations
Fix the underlying issue: Don't just update snapshots repeatedly
Verify locally: Run tests multiple times to ensure stability
Push your fix: This will reset the flap counter

If you need to proceed anyway:

Fix the test flakiness first (recommended)
Or manually verify snapshots are acceptable and revert the bot commits, then push a fix

The workflow has been failed to prevent merging unstable tests.

posthog-bot · 2025-11-01T20:41:31Z

📸 UI snapshots have been updated

5 snapshot changes in total. 0 added, 5 modified, 0 deleted:

chromium: 0 added, 5 modified, 0 deleted (diff for shard 14, diff for shard 15)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

github-actions · 2025-11-01T20:43:17Z

⚠️ Flapping Screenshots Detected

The following screenshots have been updated multiple times by the bot, indicating test instability:

playwright/__snapshots__/pageview-trends-insight.png

What this means:

These screenshots are changing on each test run (timing issues, rendering differences, etc.)
This prevents reliable verification and blocks auto-merge
Human intervention is required

How to fix:

Investigate flakiness: Check test timing, wait for elements, stabilize animations
Fix the underlying issue: Don't just update snapshots repeatedly
Verify locally: Run tests multiple times to ensure stability
Push your fix: This will reset the flap counter

If you need to proceed anyway:

Fix the test flakiness first (recommended)
Or manually verify snapshots are acceptable and revert the bot commits, then push a fix

The workflow has been failed to prevent merging unstable tests.

github-actions · 2025-11-01T20:52:35Z

⚠️ Flapping Screenshots Detected

The following screenshots have been updated multiple times by the bot, indicating test instability:

playwright/__snapshots__/pageview-trends-insight.png

What this means:

These screenshots are changing on each test run (timing issues, rendering differences, etc.)
This prevents reliable verification and blocks auto-merge
Human intervention is required

How to fix:

Investigate flakiness: Check test timing, wait for elements, stabilize animations
Fix the underlying issue: Don't just update snapshots repeatedly
Verify locally: Run tests multiple times to ensure stability
Push your fix: This will reset the flap counter

If you need to proceed anyway:

Fix the test flakiness first (recommended)
Or manually verify snapshots are acceptable and revert the bot commits, then push a fix

The workflow has been failed to prevent merging unstable tests.

posthog-bot · 2025-11-01T20:52:55Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 14)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2025-11-01T21:01:52Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 15)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

github-actions · 2025-11-01T21:02:01Z

⚠️ Flapping Screenshots Detected

The following screenshots have been updated multiple times by the bot, indicating test instability:

playwright/__snapshots__/pageview-trends-insight.png

What this means:

These screenshots are changing on each test run (timing issues, rendering differences, etc.)
This prevents reliable verification and blocks auto-merge
Human intervention is required

How to fix:

Investigate flakiness: Check test timing, wait for elements, stabilize animations
Fix the underlying issue: Don't just update snapshots repeatedly
Verify locally: Run tests multiple times to ensure stability
Push your fix: This will reset the flap counter

If you need to proceed anyway:

Fix the test flakiness first (recommended)
Or manually verify snapshots are acceptable and revert the bot commits, then push a fix

The workflow has been failed to prevent merging unstable tests.

github-actions · 2025-11-01T21:11:26Z

⚠️ Flapping Screenshots Detected

The following screenshots have been updated multiple times by the bot, indicating test instability:

playwright/__snapshots__/pageview-trends-insight.png

What this means:

These screenshots are changing on each test run (timing issues, rendering differences, etc.)
This prevents reliable verification and blocks auto-merge
Human intervention is required

How to fix:

Investigate flakiness: Check test timing, wait for elements, stabilize animations
Fix the underlying issue: Don't just update snapshots repeatedly
Verify locally: Run tests multiple times to ensure stability
Push your fix: This will reset the flap counter

If you need to proceed anyway:

Fix the test flakiness first (recommended)
Or manually verify snapshots are acceptable and revert the bot commits, then push a fix

The workflow has been failed to prevent merging unstable tests.

posthog-bot · 2025-11-01T21:16:39Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 15)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

github-actions · 2025-11-01T21:20:41Z

⚠️ Flapping Screenshots Detected

The following screenshots have been updated multiple times by the bot, indicating test instability:

playwright/__snapshots__/pageview-trends-insight.png

What this means:

These screenshots are changing on each test run (timing issues, rendering differences, etc.)
This prevents reliable verification and blocks auto-merge
Human intervention is required

How to fix:

Investigate flakiness: Check test timing, wait for elements, stabilize animations
Fix the underlying issue: Don't just update snapshots repeatedly
Verify locally: Run tests multiple times to ensure stability
Push your fix: This will reset the flap counter

If you need to proceed anyway:

Fix the test flakiness first (recommended)
Or manually verify snapshots are acceptable and revert the bot commits, then push a fix

The workflow has been failed to prevent merging unstable tests.

pauldambra force-pushed the fix/prog-safer branch from 9041dc4 to e0bddc7 Compare November 1, 2025 17:34

pauldambra and others added 4 commits November 1, 2025 20:12

fix: safer progressive loading

008f0ab

Update UI snapshots for chromium (15)

525579e

Update E2E screenshots for Playwright

7ddb6e1

Update UI snapshots for chromium (15)

6d411c2

pauldambra force-pushed the fix/prog-safer branch from 68e2eaa to 6d411c2 Compare November 1, 2025 20:12

pauldambra mentioned this pull request Nov 1, 2025

fix: trigger recalculation when snapshots change #40772

Open

github-actions bot added 2 commits November 1, 2025 20:21

Update UI snapshots for chromium (8)

9a5e130

Update UI snapshots for chromium (15)

c2ef7c7

pauldambra marked this pull request as ready for review November 1, 2025 20:22

Update E2E screenshots for Playwright

9f3d055

greptile-apps bot reviewed Nov 1, 2025

View reviewed changes

github-actions bot added 2 commits November 1, 2025 20:29

Update UI snapshots for chromium (14)

19ba602

Update UI snapshots for chromium (8)

efcdecc

github-actions bot added 2 commits November 1, 2025 20:40

Update UI snapshots for chromium (14)

32e3633

Update UI snapshots for chromium (15)

88ae721

Update UI snapshots for chromium (14)

7c582ed

Update UI snapshots for chromium (15)

529bb63

Update UI snapshots for chromium (15)

c9a39b6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: safer progressive loading #40768

fix: safer progressive loading #40768

pauldambra commented Nov 1, 2025 •

edited

Loading

pauldambra commented Nov 1, 2025 •

edited

Loading

github-actions bot commented Nov 1, 2025 •

edited

Loading

github-actions bot commented Nov 1, 2025

posthog-bot commented Nov 1, 2025

posthog-bot commented Nov 1, 2025

github-actions bot commented Nov 1, 2025

github-actions bot commented Nov 1, 2025

posthog-bot commented Nov 1, 2025

greptile-apps bot left a comment

greptile-apps bot Nov 1, 2025

greptile-apps bot Nov 1, 2025

posthog-bot commented Nov 1, 2025

github-actions bot commented Nov 1, 2025

posthog-bot commented Nov 1, 2025

github-actions bot commented Nov 1, 2025

github-actions bot commented Nov 1, 2025

posthog-bot commented Nov 1, 2025

posthog-bot commented Nov 1, 2025

github-actions bot commented Nov 1, 2025

github-actions bot commented Nov 1, 2025

posthog-bot commented Nov 1, 2025

github-actions bot commented Nov 1, 2025

Labels

3 participants

fix: safer progressive loading #40768

Are you sure you want to change the base?

fix: safer progressive loading #40768

Conversation

pauldambra commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

pauldambra commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

github-actions bot commented Nov 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

github-actions bot commented Nov 1, 2025

⚠️ Flapping Screenshots Detected

posthog-bot commented Nov 1, 2025

📸 UI snapshots have been updated

posthog-bot commented Nov 1, 2025

📸 UI snapshots have been updated

github-actions bot commented Nov 1, 2025

⚠️ Flapping Screenshots Detected

github-actions bot commented Nov 1, 2025

⚠️ Flapping Screenshots Detected

posthog-bot commented Nov 1, 2025

📸 UI snapshots have been updated

greptile-apps bot left a comment

Choose a reason for hiding this comment

greptile-apps bot Nov 1, 2025

Choose a reason for hiding this comment

greptile-apps bot Nov 1, 2025

Choose a reason for hiding this comment

posthog-bot commented Nov 1, 2025

📸 UI snapshots have been updated

github-actions bot commented Nov 1, 2025

⚠️ Flapping Screenshots Detected

posthog-bot commented Nov 1, 2025

📸 UI snapshots have been updated

github-actions bot commented Nov 1, 2025

⚠️ Flapping Screenshots Detected

github-actions bot commented Nov 1, 2025

⚠️ Flapping Screenshots Detected

posthog-bot commented Nov 1, 2025

📸 UI snapshots have been updated

posthog-bot commented Nov 1, 2025

📸 UI snapshots have been updated

github-actions bot commented Nov 1, 2025

⚠️ Flapping Screenshots Detected

github-actions bot commented Nov 1, 2025

⚠️ Flapping Screenshots Detected

posthog-bot commented Nov 1, 2025

📸 UI snapshots have been updated

github-actions bot commented Nov 1, 2025

⚠️ Flapping Screenshots Detected

Labels

3 participants

pauldambra commented Nov 1, 2025 •

edited

Loading

pauldambra commented Nov 1, 2025 •

edited

Loading

github-actions bot commented Nov 1, 2025 •

edited

Loading