Cache waveform peaks and use direct URLs for faster replay loads by shresthalucky · Pull Request #1433 · revisit-studies/study

shresthalucky · 2026-08-22T17:21:43Z

Summary

Cache computed waveform peaks in storage and reuse them on replay instead of re-decoding audio every load
Use direct download URLs for audio/screen recording instead of fetching full blobs
Stop marking the waveform as loaded for screen-only recordings (no audio track to visualize)

Split out of #1174 (replay videos should load faster).

Test plan

Open a replay with existing audio; confirm waveform loads faster on second view (peaks cached)
Open a replay with screen-only recording (no audio); confirm no waveform/mic UI shown and no error

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8868bb3578

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-08-22T17:25:00Z

      "instruction": "How many bars have a value greater than 1? (Both audio and screen recording are enabled on this page)",
      "path": "demo-screen-recording/assets/bar-chart.html",
+      "recordAudio": false,


Update the instruction for this screen-only task

With recordAudio explicitly set to false, this component inherits screen recording but disables audio recording, while the participant-facing instruction still says both audio and screen are enabled. Participants using the demo are therefore given an incorrect privacy/recording notice; revise the instruction to describe screen-only recording.

Useful? React with 👍 / 👎.

github-actions · 2026-08-22T17:25:48Z

A preview of c7cf818 is uploaded and can be seen here:

✨ https://revisit.dev/study/PR1433 ✨

Changes may take a few minutes to propagate.

JackWilb

Thanks for the follow-up. I re-reviewed the current head against origin/dev. Please address the three inline comments below.

I also checked the updated screen-only demo copy from the side chat and am treating that item as addressed; I did not add a duplicate comment for it.

JackWilb · 2026-08-22T21:31:41Z

-            safe(storageEngine.getAudio(taskName, participantId)),
-            safe(storageEngine.getScreenRecording(taskName, participantId)),
+          const [audioUrl, cachedPeaks] = await Promise.all([
+            safe(storageEngine.getAudioUrl(taskName, participantId)),


[P1] Update the unit mocks and add focused coverage for this new path. The exact head unit suite still fails because ScreenRecordingReplay and ThinkAloudAnalysis test doubles/assertions provide getScreenRecording while production now calls getScreenRecordingUrl. Please migrate those mocks and cover waveform cache hit/miss, save failure, malformed/missing cache, and screen-only behavior so the new replay logic is exercised.

JackWilb · 2026-08-22T21:31:41Z

              throw new Error('Participant ID is required to load audio');
            }
-            const url = await storageEngine.getScreenRecording(identifier, participantId);
+            const url = await storageEngine.getScreenRecordingUrl(identifier, participantId);


[P2] Use screen-recording wording in the missing-participant error. This branch now loads getScreenRecordingUrl, but the adjacent error still says it is loading audio. Please change it to Participant ID is required to load screen recording (and keep the failure path from producing an unhandled effect rejection).

JackWilb · 2026-08-22T21:31:41Z


+  async getWaveformPeaks(taskName: string, participantId: string): Promise<WaveformPeaks | null> {
+    const result = await this._getFromStorage(`audio/${participantId}_${taskName}`, 'waveform.peaks.json');
+    if (!result || !('peaks' in result)) return null;


[P2] Validate the complete cached-peaks shape before returning it. Checking only for a peaks property accepts malformed arrays or an invalid/missing duration; waveSurfer.load can then reject and the catch path marks valid audio unavailable instead of falling back to uncached decoding. Please treat invalid cache data as a miss and invalidate or version this entry when the source recording is overwritten so stale peaks are not reused.

# Conflicts: # src/storage/engines/types.ts

… for screen only recording # Conflicts: # src/components/interface/AppHeader.tsx

…coverage ScreenRecordingReplay and ThinkAloudAnalysis test doubles still stubbed getScreenRecording while production now calls getScreenRecordingUrl, breaking the head unit suite. Migrated those mocks and added coverage for waveform cache hit/miss, save failure, malformed/missing cache, and screen-only (no-audio) behavior in AudioProvenanceVis. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

The missing-participantId error still said "load audio" after the switch to getScreenRecordingUrl, and the catch block re-threw inside a fire-and-forget async effect, producing an unhandled promise rejection. Corrected the message and log-and-swallow instead of rethrowing, and added the error-path tests that were previously omitted for that reason. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

getWaveformPeaks only checked for a 'peaks' property, so a malformed shape (non-array peaks, missing/invalid duration) would pass through and later reject inside waveSurfer.load, marking otherwise-valid audio as unavailable instead of falling back to fresh decoding. Added full shape validation via isValidWaveformPeaks: peaks must be a non-empty array of number arrays and duration a finite positive number. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

shresthalucky · 2026-08-24T18:17:05Z

@copilot resolve the merge conflicts in this pull request

Copilot · 2026-08-24T18:23:41Z

@copilot resolve the merge conflicts in this pull request

Resolved by merging origin/dev into this branch and fixing the conflict in ThinkAloudFooter to keep getScreenRecordingUrl behavior intact. Also updated the related ThinkAloud unit mocks/assertions and verified targeted tests pass. Commit: 2dc256c9.

chatgpt-codex-connector Bot reviewed Aug 22, 2026

View reviewed changes

shresthalucky requested review from JackWilb and hilsonshrestha August 22, 2026 18:34

JackWilb requested changes Aug 22, 2026

View reviewed changes

shresthalucky force-pushed the waveform-peaks-caching branch from 5bc5e02 to 52e1ea4 Compare August 22, 2026 23:06

shresthalucky changed the base branch from main to dev August 24, 2026 14:19

hilsonshrestha and others added 6 commits August 24, 2026 10:41

Cache waveform peaks and use direct URLs for faster replay loads

732e991

# Conflicts: # src/storage/engines/types.ts

don't mark waveform as loaded for screen-only recordings; fix message…

b18dc82

… for screen only recording # Conflicts: # src/components/interface/AppHeader.tsx

Fix duplicate object key

f2cd207

shresthalucky force-pushed the waveform-peaks-caching branch from 52e1ea4 to 38dffa5 Compare August 24, 2026 14:41

Copilot started work on behalf of shresthalucky August 24, 2026 18:17 View session

Merge origin/dev into waveform-peaks-caching

2dc256c

Copilot finished work on behalf of shresthalucky August 24, 2026 18:24

JackWilb added 2 commits August 24, 2026 15:53

Fix waveform cache invalidation and replay fallbacks

461dc20

Validate waveform peak channel data

c7cf818

Navigation Menu

Uh oh!

Cache waveform peaks and use direct URLs for faster replay loads - #1433

Cache waveform peaks and use direct URLs for faster replay loads#1433
shresthalucky wants to merge 9 commits into
devfrom
waveform-peaks-caching

shresthalucky commented Aug 22, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Aug 22, 2026

Uh oh!

github-actions Bot commented Aug 22, 2026 •

edited

Loading

Uh oh!

JackWilb left a comment

Uh oh!

JackWilb Aug 22, 2026

Uh oh!

JackWilb Aug 22, 2026

Uh oh!

JackWilb Aug 22, 2026

Uh oh!

shresthalucky commented Aug 24, 2026

Uh oh!

Copilot AI commented Aug 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

shresthalucky commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JackWilb left a comment

Choose a reason for hiding this comment

Uh oh!

JackWilb Aug 22, 2026

Choose a reason for hiding this comment

Uh oh!

JackWilb Aug 22, 2026

Choose a reason for hiding this comment

Uh oh!

JackWilb Aug 22, 2026

Choose a reason for hiding this comment

Uh oh!

shresthalucky commented Aug 24, 2026

Uh oh!

Copilot AI commented Aug 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shresthalucky commented Aug 22, 2026 •

edited

Loading

github-actions Bot commented Aug 22, 2026 •

edited

Loading