Skip to content

feat: brady bunch#816

Merged
deardarlingoose merged 25 commits intomainfrom
feat/brady-bunch
Jan 23, 2026
Merged

feat: brady bunch#816
deardarlingoose merged 25 commits intomainfrom
feat/brady-bunch

Conversation

@deardarlingoose
Copy link
Contributor

@deardarlingoose deardarlingoose commented Jan 13, 2026

User description

Screenshot 2026-01-13 at 1 30 26 PM

both "raw" and "merged" recordings at the same time

an alternative of this suggestion https://discord.com/channels/1217145424381743145/1281060888966598747/1281060888966598747

KyleOP — 9/4/24, 9:18 PM
We have an app right now that is using raw tracks where we are encoding each using MediaConvert but we also need the Brady Bunch style grid video that daily outputs as its cloud output option. We're at a cross roads right now where we've been told in the past it's possible to get both but when applying the process suggested (start the recording from JS as well as our backend) it doesn't work and throws no logs to troubleshoot on. The other approach is to do this ourselves with either a bot or a something bespoke through ffmpeg and python which we want to avoid. Right now we're trying to weigh our options and figure out how to either get both raw track and cloud assets, or it this is a task for a meeting bot that we can use instead.

rajneesh — 9/5/24, 12:19 AM
Hi @Kyle It is possible to get both "raw-tracks" and "cloud-recording" at the same time.

to run both type of recording, start the cloud recording using daily-js API and then start raw-tracks using the REST API endpoint.
Please note you need to pass a unique instanceId in the [startRecording](https://docs.daily.co/reference/daily-js/instance-methods/start-recording#main) call.
if you face and issue, pls provide the the "domain" and the mtgSessionId, so that we can check the logs.

why not just merge: no approach we tried so far works reliably, even official and half-official ones

accounted for cross-feature conflicts with:

  • whereby
  • calendar
  • poll vs webhook
  • daily.co undocumented empty documented fields

EDIT - docs: https://www.loom.com/share/98c4d8ee95cc41e682181a06e116a3ea


PR Type

Enhancement, Bug fix


Description

  • Brady Bunch grid video alongside raw tracks

  • Time-based recording-to-meeting matching

  • Dual recording via REST API

  • Race condition fixes for webhook/polling


Changes walkthrough 📝

Relevant files
Database migration
1 files
1b1e6a6fc465_add_cloud_recording_support.py
Add database columns for cloud recording                                 
+40/-0   
Enhancement
15 files
__init__.py
Export RecordingType type                                                               
+2/-1     
client.py
Add REST API recording start method                                           
+36/-1   
instance_id.py
Add deterministic instanceId generation                                   
+37/-0   
responses.py
Add S3 key fields for cloud recordings                                     
+7/-0     
meetings.py
Add time-based meeting matching and cloud recording storage
+130/-2 
recordings.py
Add method to link recordings to meetings                               
+14/-0   
daily.py
Update recording start logic for dual recordings                 
+23/-13 
daily.py
Handle both cloud and raw-tracks recording webhooks           
+45/-26 
meetings.py
Add REST API endpoint to start recordings                               
+100/-1 
rooms.py
Remove auto-start recording from token creation                   
+2/-1     
process.py
Add cloud recording handling and time-based matching         
+282/-39
DailyRoom.tsx
Implement dual recording start with retry logic                   
+93/-20 
apiHooks.ts
Add hook for starting recordings                                                 
+14/-0   
types.ts
Add DailyRecordingType type                                                           
+2/-0     
reflector-api.d.ts
Add recording start API endpoint types                                     
+79/-0   
Cleanup
1 files
requests.py
Remove unused token properties                                                     
+0/-7     
Configuration changes
2 files
app.py
Adjust polling interval based on webhook availability       
+6/-1     
.gitleaksignore
Add exception for API key in process.py                                   
+1/-0     
Tests
2 files
test_dailyco_instance_id.py
Add tests for instanceId generation                                           
+147/-0 
test_time_based_meeting_matching.py
Add tests for time-based meeting matching                               
+374/-0 
Bug fix
6 files
finalSummary.tsx
Fix type reference                                                                             
+3/-2     
createTranscript.ts
Fix type reference                                                                             
+3/-2     
shareAndPrivacy.tsx
Fix type reference                                                                             
+3/-2     
shareZulip.tsx
Fix type reference                                                                             
+3/-2     
transcriptTitle.tsx
Fix type reference                                                                             
+3/-2     
transcript.ts
Fix type reference                                                                             
+2/-1     
Dependencies
2 files
package.json
Add react-uuid-hook dependency                                                     
+1/-0     
pnpm-lock.yaml
Update lock file with react-uuid-hook                                       
+25/-0   

Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @deardarlingoose deardarlingoose changed the title Feat/brady bunch feat: brady bunch Jan 14, 2026
    @deardarlingoose deardarlingoose marked this pull request as ready for review January 14, 2026 22:50
    @pr-agent-monadical
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 PR contains tests
    🔒 Security concerns

    The endpoint for starting recordings (/v1/meetings/{meeting_id}/recordings/start) explicitly notes "No authentication required - anonymous users supported. TODO this is a DOS vector" in both the code comments and API documentation. This creates a potential Denial of Service vulnerability where unauthenticated users could start recordings for any meeting, potentially exhausting resources or hitting API rate limits.

    ⚡ Recommended focus areas for review

    Time-based Matching

    The time-based matching logic for linking recordings to meetings is complex and potentially fragile. The 1-week window might be too large in busy environments, and the approach relies on temporal proximity which could fail if multiple meetings occur close together.

        logger.warning(
            f"Could not parse recorded_at from keys, using now() {recorded_at}",
            e,
            exc_info=True,
        )
    
    # Check if recording already exists (reprocessing path)
    recording = await recordings_controller.get_by_id(recording_id)
    
    if recording and recording.meeting_id:
        # Reprocessing: recording exists with meeting already linked
        meeting = await meetings_controller.get_by_id(recording.meeting_id)
        if not meeting:
            logger.error(
                "Reprocessing: meeting not found for recording - skipping",
                meeting_id=recording.meeting_id,
                recording_id=recording_id,
            )
            return
    
        logger.info(
            "Reprocessing: using existing recording.meeting_id",
            recording_id=recording_id,
            meeting_id=meeting.id,
            room_name=daily_room_name,
        )
    else:
        # First processing: recording doesn't exist, need time-based matching
        # (Daily.co doesn't return instanceId in API, must match by timestamp)
        recording_start = datetime.fromtimestamp(recording_start_ts, tz=timezone.utc)
        meeting = await meetings_controller.get_by_room_name_and_time(
            room_name=daily_room_name,
            recording_start=recording_start,
            time_window_hours=168,  # 1 week
        )
        if not meeting:
            logger.error(
                "Raw-tracks: no meeting found within 1-week window (time-based match) - skipping",
                recording_id=recording_id,
                room_name=daily_room_name,
                recording_start_ts=recording_start_ts,
                recording_start=recording_start.isoformat(),
            )
            return  # Skip processing, will retry on next poll
        logger.info(
            "First processing: found meeting via time-based matching",
            meeting_id=meeting.id,
            room_name=daily_room_name,
            recording_id=recording_id,
            time_delta_seconds=abs(
                (meeting.start_date - recording_start).total_seconds()
            ),
        )
    Type Inference

    The code infers recording type from structural properties since the Daily.co API doesn't return explicit type information. This inference logic could break if the API changes its response format.

    # DAILY.CO API LIMITATION:
    # GET /recordings response does NOT include type field.
    # Daily.co docs mention type field exists, but API never returns it.
    # Verified: 84 recordings from Nov 2025 - Jan 2026 ALL have type=None.
    #
    # This is not a recent API change - Daily.co has never returned type.
    # Must infer from structural properties.
    #
    # Inference heuristic (reliable for finished recordings):
    # - Has tracks array → raw-tracks
    # - Has s3key but no tracks → cloud
    # - Neither → failed/incomplete recording
    if len(rec.tracks) > 0:
        recording_type = "raw-tracks"
    elif rec.s3key and len(rec.tracks) == 0:
        recording_type = "cloud"
    else:
        logger.warning(
            "Recording has no type, no s3key, and no tracks - likely failed recording",
            recording_id=rec.id,
            room_name=rec.room_name,
            status=rec.status,
            duration=rec.duration,
            mtg_session_id=rec.mtgSessionId,
        )
        continue
    Error Handling

    The recording start retry mechanism has limited error handling. It retries on 404 errors but could benefit from more robust error detection and recovery for other failure scenarios.

    const startRecordingWithRetry = (
      type: DailyRecordingType,
      instanceId: NonEmptyString,
      attempt: number = 1,
    ) => {
      setTimeout(() => {
        startRecordingMutation.mutate(
          {
            params: {
              path: {
                meeting_id: meeting.id,
              },
            },
            body: {
              type,
              instanceId,
            },
          },
          {
            onError: (error: any) => {
              const errorText = error?.detail || error?.message || "";
              const is404NotHosting = errorText.includes(
                "does not seem to be hosting a call",
              );
              const isActiveStream = errorText.includes(
                "has an active stream",
              );
    
              if (is404NotHosting && attempt < RECORDING_START_MAX_RETRIES) {
                console.log(
                  `${type}: Call not hosting yet, retry ${attempt + 1}/${RECORDING_START_MAX_RETRIES} in ${RECORDING_START_DELAY_MS}ms...`,
                );
                startRecordingWithRetry(type, instanceId, attempt + 1);
              } else if (isActiveStream) {
                console.log(
                  `${type}: Recording already active (started by another participant)`,
                );
              } else {
                console.error(`Failed to start ${type} recording:`, error);
              }
            },
          },
        );
      }, RECORDING_START_DELAY_MS);
    };

    @deardarlingoose
    Copy link
    Contributor Author

    Addressed the matching concern in a separate PR #824

    @deardarlingoose deardarlingoose merged commit 6c175a1 into main Jan 23, 2026
    8 checks passed
    @deardarlingoose deardarlingoose deleted the feat/brady-bunch branch January 23, 2026 17:33
    @tito tito mentioned this pull request Jan 23, 2026
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    3 participants