Skip to content

Conversation

@AnkushMalaker
Copy link
Collaborator

@AnkushMalaker AnkushMalaker commented Jan 12, 2026

… management

  • Replaced the legacy AudioFile model with AudioChunkDocument for storing audio data in MongoDB, optimizing storage and retrieval.
  • Introduced CleanupSettings dataclass for managing soft-deletion configurations, including auto-cleanup and retention days.
  • Added admin API routes for retrieving and saving cleanup settings, ensuring better control over data retention policies.
  • Updated audio processing workflows to utilize MongoDB chunks, removing dependencies on disk-based audio files.
  • Enhanced tests to validate the new audio chunk storage and cleanup functionalities, ensuring robust integration with existing systems.

Summary by CodeRabbit

Release Notes

  • New Features

    • Audio storage now uses MongoDB chunks with automatic Opus compression instead of disk files.
    • Soft-delete and restore functionality for conversations with retention-based cleanup policies.
    • Admin cleanup settings management endpoints with dry-run preview capabilities.
    • Streaming audio playback and time-range retrieval directly from MongoDB.
  • Refactor

    • Migrated audio persistence layer from filesystem to MongoDB-backed storage.
    • Updated conversation deletion to support soft-delete with optional permanent removal.
  • Tests

    • Added comprehensive MongoDB audio storage integration tests and Robot Framework test suites.

✏️ Tip: You can customize this high-level summary in your review settings.

… management

- Replaced the legacy AudioFile model with AudioChunkDocument for storing audio data in MongoDB, optimizing storage and retrieval.
- Introduced CleanupSettings dataclass for managing soft-deletion configurations, including auto-cleanup and retention days.
- Added admin API routes for retrieving and saving cleanup settings, ensuring better control over data retention policies.
- Updated audio processing workflows to utilize MongoDB chunks, removing dependencies on disk-based audio files.
- Enhanced tests to validate the new audio chunk storage and cleanup functionalities, ensuring robust integration with existing systems.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 12, 2026

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

The pull request introduces MongoDB-backed audio chunk storage, replacing disk-based file persistence. Audio is now stored as Opus-encoded chunks in MongoDB rather than WAV files on disk. Cleanup settings enable retention-based purging of soft-deleted conversations. Admin endpoints manage cleanup configuration. Audio upload, retrieval, and worker jobs are refactored to use the new chunk-based architecture.

Changes

Cohort / File(s) Summary
Audio Chunk Model
backends/advanced/src/advanced_omi_backend/models/audio_chunk.py
New AudioChunkDocument with Beanie ODM support for storing Opus audio chunks, chunk metadata, soft-delete flags, and computed compression metrics
Model Migrations
backends/advanced/src/advanced_omi_backend/models/audio_file.py, backends/advanced/src/advanced_omi_backend/models/job.py, backends/advanced/src/advanced_omi_backend/models/conversation.py, backends/advanced/src/advanced_omi_backend/app_factory.py
Removed legacy AudioFile model; updated Beanie registration and Conversation schema to reference AudioChunkDocument and expose audio_chunks_count, audio_total_duration, audio_compression_ratio fields
Cleanup Settings
backends/advanced/src/advanced_omi_backend/config.py
New CleanupSettings dataclass with JSON file persistence, in-memory caching, and getter/setter functions for cleanup configuration (auto_cleanup_enabled, retention_days)
Audio Processing Utilities
backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py
Comprehensive module (739 lines) for Opus encoding/decoding, WAV construction, MongoDB chunk storage/retrieval, concatenation, and reconstruction from PCM and WAV sources
Audio Utilities Updates
backends/advanced/src/advanced_omi_backend/utils/audio_utils.py, backends/advanced/src/advanced_omi_backend/utils/conversation_utils.py, backends/advanced/src/advanced_omi_backend/utils/gdrive_audio_utils.py
Removed AudioFile persistence in write_audio_file; removed wait_for_audio_file polling; updated gdrive dedup logic to use Conversation instead of AudioFile
Audio Upload & Retrieval Controllers
backends/advanced/src/advanced_omi_backend/controllers/audio_controller.py
Replaced write_audio_file with validate_and_prepare_audio; added convert_audio_to_chunks for MongoDB storage; removed disk audio path handling; removed get_conversation_audio_path helper
Conversation Controller
backends/advanced/src/advanced_omi_backend/controllers/conversation_controller.py
Implemented soft-delete/hard-delete pathways; added restore_conversation; updated delete_conversation with permanent flag; removed legacy disk cleanup; expanded responses with audio chunk metadata
System Controller
backends/advanced/src/advanced_omi_backend/controllers/system_controller.py
New admin endpoints for get_cleanup_settings and save_cleanup_settings with validation
WebSocket Controller
backends/advanced/src/advanced_omi_backend/controllers/websocket_controller.py
Updated batch audio completion to convert audio to MongoDB chunks via convert_audio_to_chunks instead of writing files
Audio Routes
backends/advanced/src/advanced_omi_backend/routers/modules/audio_routes.py
Replaced file-based audio serving with WAV reconstruction from MongoDB chunks; added stream_conversation_audio and get_audio_chunk_range endpoints for progressive and range-based playback
Conversation Routes
backends/advanced/src/advanced_omi_backend/routers/modules/conversation_routes.py
Added permanent parameter to delete_conversation; introduced restore_conversation route
Admin Routes
backends/advanced/src/advanced_omi_backend/routers/modules/admin_routes.py
New admin router module with require_admin dependency and endpoints for cleanup settings retrieval, trigger_cleanup (with job enqueue), and preview_cleanup
System Routes
backends/advanced/src/advanced_omi_backend/routers/modules/system_routes.py
New admin-only routes for GET/POST cleanup settings with validation
Router Registration
backends/advanced/src/advanced_omi_backend/routers/api_router.py, backends/advanced/src/advanced_omi_backend/routers/modules/__init__.py
Registered admin_router in main API router and module exports
Cleanup Jobs
backends/advanced/src/advanced_omi_backend/workers/cleanup_jobs.py
New module with purge_old_deleted_conversations job (hard-delete by retention) and schedule_cleanup_job function
Conversation Jobs
backends/advanced/src/advanced_omi_backend/workers/conversation_jobs.py
Replaced wait_for_audio_file with wait_for_audio_chunks in open_conversation_job; updated deletion handling
Audio Persistence Jobs
backends/advanced/src/advanced_omi_backend/workers/audio_jobs.py
Replaced disk-based WAV buffering with 10-second PCM chunk buffering, Opus encoding, and MongoDB storage; updated metadata tracking and conversation updates
Transcription Jobs
backends/advanced/src/advanced_omi_backend/workers/transcription_jobs.py
Replaced direct audio file reading with WAV reconstruction from MongoDB chunks; added temporary file management and error handling
Speaker Jobs
backends/advanced/src/advanced_omi_backend/workers/speaker_jobs.py
Replaced direct audio path usage with reconstruction from MongoDB chunks; added temporary file lifecycle management and granular error handling
Python Tests
backends/advanced/tests/test_audio_persistence_mongodb.py
New integration test suite for Opus codec, MongoDB chunk storage/retrieval, WAV reconstruction, and chunk waiting mechanism
Robot Framework Tests
tests/endpoints/audio_upload_tests.robot, tests/infrastructure/infra_tests.robot, tests/integration/mongodb_audio_storage_tests.robot
Updated/added tests for MongoDB audio storage; removed audio_path validation; reworked WebSocket test flow; added MongoDB chunk verification keywords
Test Infrastructure
tests/libs/mongodb_helper.py, tests/resources/mongodb_keywords.robot
New MongoDB helper utilities and Robot Framework keywords for chunk retrieval, verification, and metadata validation
Test Configuration
tests/.env.test, tests/test-requirements.txt
Added MongoDB test environment variables and pymongo dependency

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant AudioController
    participant AudioUtils
    participant AudioChunkUtils
    participant MongoDB
    participant Conversation

    Client->>AudioController: POST /audio/upload
    AudioController->>AudioUtils: validate_and_prepare_audio()
    AudioUtils-->>AudioController: audio_data, metadata
    AudioController->>AudioChunkUtils: convert_audio_to_chunks()
    AudioChunkUtils->>AudioChunkUtils: Split into 10s chunks
    AudioChunkUtils->>AudioChunkUtils: Encode PCM to Opus
    AudioChunkUtils->>MongoDB: Insert AudioChunkDocument(s)
    MongoDB-->>AudioChunkUtils: Chunk IDs
    AudioChunkUtils->>Conversation: Update audio_chunks_count, duration, compression_ratio
    AudioChunkUtils-->>AudioController: chunk_count
    AudioController->>Conversation: Create Conversation (audio_file_path=None)
    AudioController-->>Client: 200 OK, conversation_id
Loading
sequenceDiagram
    participant Client
    participant AudioRoutes
    participant AudioChunkUtils
    participant MongoDB
    participant Conversation

    Client->>AudioRoutes: GET /audio/{conversation_id}
    AudioRoutes->>AudioChunkUtils: retrieve_audio_chunks(conversation_id)
    AudioChunkUtils->>MongoDB: Query AudioChunkDocument by conversation_id
    MongoDB-->>AudioChunkUtils: List[AudioChunkDocument]
    AudioChunkUtils->>AudioChunkUtils: concatenate_chunks_to_pcm()
    AudioChunkUtils->>AudioChunkUtils: Decode Opus chunks to PCM
    AudioChunkUtils->>AudioChunkUtils: build_wav_from_pcm()
    AudioChunkUtils-->>AudioRoutes: WAV bytes
    AudioRoutes->>Client: StreamingResponse(WAV data)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: refactoring audio storage to MongoDB chunks and adding cleanup settings management, which are the core objectives of this pull request.
Docstring Coverage ✅ Passed Docstring coverage is 98.80% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@AnkushMalaker
Copy link
Collaborator Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 12, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)
backends/advanced/src/advanced_omi_backend/models/job.py (1)

48-52: Bug: The raise statement negates the fallback logic.

The exception handler assigns database = client[mongodb_database] as a fallback when get_default_database() fails, but then immediately re-raises the exception, making the fallback useless. The code never reaches init_beanie() in this case.

This appears to be an unintended raise that should be removed to allow the fallback to work correctly.

🐛 Proposed fix
             try:
                 database = client.get_default_database(mongodb_database)
             except ConfigurationError:
                 database = client[mongodb_database]
-                raise
-            _beanie_initialized = True
backends/advanced/src/advanced_omi_backend/controllers/websocket_controller.py (1)

873-903: Misleading error handling - chunk conversion failure will cause downstream job failures.

The comment on line 891 states "Continue anyway - transcription job will handle it", but this is incorrect. Based on transcription_jobs.py, the transcription job now reconstructs audio from MongoDB chunks via reconstruct_wav_from_conversation(). If chunk conversion fails here, the transcription job will raise FileNotFoundError("No audio chunks found...").

Consider either:

  1. Re-raising the exception to fail fast and avoid enqueuing jobs that will fail
  2. Updating the comment to accurately describe the behavior (jobs will fail, but conversation record exists for retry)
🔧 Option 1: Fail fast to avoid orphaned jobs
         except Exception as chunk_error:
             application_logger.error(
                 f"Failed to convert batch audio to chunks: {chunk_error}",
                 exc_info=True
             )
-            # Continue anyway - transcription job will handle it
+            # Cannot proceed without audio chunks - transcription job will fail
+            raise
📝 Option 2: Update comment to be accurate
         except Exception as chunk_error:
             application_logger.error(
                 f"Failed to convert batch audio to chunks: {chunk_error}",
                 exc_info=True
             )
-            # Continue anyway - transcription job will handle it
+            # Note: Transcription will fail with FileNotFoundError, but conversation exists for debugging
backends/advanced/src/advanced_omi_backend/workers/speaker_jobs.py (1)

121-128: Remove the unused audio_path parameter.

The audio_path parameter is never used in the function. Audio is reconstructed from MongoDB chunks (line 198), and the speaker recognition service receives temp_wav_path instead (line 248). This requires updating the two call sites as well.

♻️ Remove unused parameter
 @async_job(redis=True, beanie=True)
 async def recognise_speakers_job(
     conversation_id: str,
     version_id: str,
-    audio_path: str,
     transcript_text: str,
     words: list,
     *,
     redis_client=None
 ) -> Dict[str, Any]:

Update callers:

  • backends/advanced/src/advanced_omi_backend/controllers/conversation_controller.py line 454: Remove str(full_audio_path),
  • backends/advanced/src/advanced_omi_backend/controllers/queue_controller.py line 442: Remove audio_file_path,
backends/advanced/src/advanced_omi_backend/workers/conversation_jobs.py (1)

527-534: Critical: file_path is undefined, causing a NameError at runtime.

The refactoring replaced wait_for_audio_file() (which returned a file path) with wait_for_audio_chunks() (which returns a boolean), but line 530 still references the undefined file_path variable in the start_post_conversation_jobs call.

The downstream jobs (transcribe_full_audio_job and recognise_speakers_job) both reconstruct audio from MongoDB chunks and do not use the audio_file_path parameter, so this parameter can be safely removed or set to None if the function signature is updated to accept Optional[str].

🐛 Suggested fix

Either remove the parameter entirely:

     job_ids = start_post_conversation_jobs(
         conversation_id=conversation_id,
         audio_uuid=session_id,
-        audio_file_path=file_path,
         user_id=user_id,
         post_transcription=True,  # Run batch transcription for streaming audio
         client_id=client_id  # Pass client_id for UI tracking
     )

Or update start_post_conversation_jobs signature to accept Optional[str] and pass None.

backends/advanced/src/advanced_omi_backend/controllers/audio_controller.py (1)

134-163: Processing continues after chunk conversion failure.

If convert_audio_to_chunks fails (lines 147-151), the error is logged but execution continues to enqueue transcription jobs (lines 156-163). These downstream jobs will fail since no audio chunks were stored.

Consider failing the upload for this file when chunk conversion fails:

🐛 Proposed fix
                 try:
                     num_chunks = await convert_audio_to_chunks(
                         conversation_id=conversation_id,
                         audio_data=audio_data,
                         sample_rate=sample_rate,
                         channels=channels,
                         sample_width=sample_width,
                     )
                     audio_logger.info(
                         f"📦 Converted uploaded file to {num_chunks} MongoDB chunks "
                         f"(conversation {conversation_id[:12]})"
                     )
                 except Exception as chunk_error:
                     audio_logger.error(
                         f"Failed to convert uploaded file to chunks: {chunk_error}",
                         exc_info=True
                     )
+                    # Delete the conversation since we can't store audio
+                    await conversation.delete()
+                    processed_files.append({
+                        "filename": file.filename,
+                        "status": "error",
+                        "error": f"Failed to store audio: {chunk_error}",
+                    })
+                    continue
🤖 Fix all issues with AI agents
In
@backends/advanced/src/advanced_omi_backend/controllers/conversation_controller.py:
- Around line 181-216: The _soft_delete_conversation function uses
datetime.utcnow() but datetime is not imported; fix by adding the missing import
(e.g., add "from datetime import datetime") at the top of the module so calls in
_soft_delete_conversation (and its update_many payload) resolve correctly and
avoid a NameError.

In @backends/advanced/src/advanced_omi_backend/routers/modules/audio_routes.py:
- Around line 321-331: The end_byte calculation can go negative for partial
final chunks because bytes_from_end assumes CHUNK_DURATION; update the logic
around start_offset_in_chunk/end_offset_in_chunk/bytes_from_end/end_byte to use
the actual retrieved chunk duration (e.g., use each chunk's stored duration
field) when converting to bytes, and then clamp the resulting end_byte to valid
bounds via end_byte = max(start_byte, min(end_byte, len(pcm_buffer))) to prevent
negative or out-of-range slices of pcm_buffer.

In @tests/infrastructure/infra_tests.robot:
- Around line 282-310: The variable ${conversation_id} may be undefined if the
FOR loop finishes without finding a job; before the loop starts initialize
${conversation_id} to a known value (e.g., Set Variable ${None}) so the final
assertion (Should Not Be Equal ${conversation_id} ${None}) fails with the
intended message; keep the rest of the logic (Get Conversation ID From Job Meta,
Close Audio Stream, Send Audio Chunks To Stream) unchanged.
🧹 Nitpick comments (23)
backends/advanced/src/advanced_omi_backend/models/audio_chunk.py (1)

121-137: Consider annotating indexes with ClassVar to satisfy static analysis.

The static analysis tool flags mutable class attributes without ClassVar annotation (Ruff RUF012). While this works correctly at runtime with Beanie, adding the annotation improves type safety.

♻️ Suggested fix
+from typing import ClassVar, List, Optional, Union
 ...
     class Settings:
         """Beanie document settings."""
         name = "audio_chunks"

-        indexes = [
+        indexes: ClassVar[List[Union[str, List[tuple]]]] = [
             # Primary query: Retrieve chunks in order for a conversation
             [("conversation_id", 1), ("chunk_index", 1)],
             ...
         ]
backends/advanced/src/advanced_omi_backend/config.py (3)

150-154: Consider deferring directory creation to the save function.

get_cleanup_config_path() unconditionally creates the data directory as a side effect, whereas the analogous get_diarization_config_path() only checks existence. Creating directories in a getter can be unexpected behavior.

♻️ Suggested refactor
 def get_cleanup_config_path() -> Path:
     """Get path to cleanup settings JSON file."""
     data_dir = Path(os.getenv("DATA_DIR", "/app/data"))
-    data_dir.mkdir(parents=True, exist_ok=True)
     return data_dir / "cleanup_config.json"

Then ensure save_cleanup_settings_to_file creates the directory before writing:

 def save_cleanup_settings_to_file(settings: CleanupSettings) -> None:
     ...
     config_path = get_cleanup_config_path()
 
     try:
+        # Ensure parent directory exists
+        config_path.parent.mkdir(parents=True, exist_ok=True)
+        
         # Save to JSON file
         with open(config_path, "w") as f:

180-181: Use logging.exception to capture the stack trace.

When catching exceptions, logging.exception automatically includes the traceback, which aids debugging.

         except Exception as e:
-            logger.error(f"❌ Failed to load cleanup settings from {config_path}: {e}")
+            logger.exception(f"❌ Failed to load cleanup settings from {config_path}: {e}")

212-214: Use logging.exception before re-raising.

Since the exception is being re-raised, using logging.exception captures the full traceback in the log before propagation.

     except Exception as e:
-        logger.error(f"❌ Failed to save cleanup settings to {config_path}: {e}")
+        logger.exception(f"❌ Failed to save cleanup settings to {config_path}: {e}")
         raise
backends/advanced/src/advanced_omi_backend/models/job.py (1)

53-60: Minor: Redundant _beanie_initialized = True assignment.

The flag is set to True at line 53 (before init_beanie) and again at line 60 (after). The first assignment at line 53 should be removed since it's set prematurely before initialization completes.

♻️ Suggested cleanup
-            _beanie_initialized = True
             # Initialize Beanie
             await init_beanie(
                 database=database,
                 document_models=[User, Conversation, AudioChunkDocument],
             )

             _beanie_initialized = True
backends/advanced/src/advanced_omi_backend/workers/transcription_jobs.py (1)

232-237: Add exception chaining for better traceability.

The raise statements should use raise ... from e to preserve the original exception context in the traceback. This makes debugging easier.

♻️ Proposed fix with exception chaining
     except ValueError as e:
         # No chunks found for conversation
-        raise FileNotFoundError(f"No audio chunks found for conversation {conversation_id}: {e}")
+        raise FileNotFoundError(f"No audio chunks found for conversation {conversation_id}: {e}") from e
     except Exception as e:
         logger.error(f"Failed to reconstruct audio from MongoDB: {e}", exc_info=True)
-        raise RuntimeError(f"Audio reconstruction failed: {e}")
+        raise RuntimeError(f"Audio reconstruction failed: {e}") from e
tests/integration/mongodb_audio_storage_tests.robot (1)

23-45: Consider replacing fixed Sleep 5s with a polling mechanism.

Using a fixed 5-second sleep is brittle - chunk persistence could take longer under load or complete faster. Consider using a Wait Until Keyword Succeeds pattern to poll for chunks instead of a fixed delay.

♻️ Suggested improvement
-    # Wait for chunks to be written to MongoDB
-    Sleep    5s
+    # Wait for chunks to be written to MongoDB (poll with timeout)
+    Wait Until Keyword Succeeds    30s    2s    Verify Audio Chunks Exist    ${conversation_id}    min_chunks=5
-
-    # Verify chunks exist in MongoDB (expect ~6 chunks for 1-minute audio)
-    ${chunks}=    Verify Audio Chunks Exist    ${conversation_id}    min_chunks=5
+    ${chunks}=    Get Audio Chunks For Conversation    ${conversation_id}

Apply similar changes to other test cases (lines 57, 79, 98).

tests/libs/mongodb_helper.py (1)

37-61: Consider using a context manager for cleaner connection handling.

The current try/finally pattern works correctly, but a context manager would be slightly cleaner and could be reused across all functions.

♻️ Optional: Use contextlib for connection management
from contextlib import contextmanager

@contextmanager
def get_mongo_client():
    """Context manager for MongoDB connections."""
    client = MongoClient(get_mongodb_uri())
    try:
        yield client[get_db_name()]
    finally:
        client.close()


def get_audio_chunks(conversation_id):
    """Get all audio chunks for a conversation from MongoDB."""
    with get_mongo_client() as db:
        chunks = list(db.audio_chunks.find(
            {"conversation_id": conversation_id},
            sort=[("chunk_index", 1)]
        ))
        # ... rest of processing
        return chunks
backends/advanced/src/advanced_omi_backend/workers/cleanup_jobs.py (1)

135-137: Consider using logger.exception for better error context.

When catching exceptions, logger.exception automatically includes the stack trace, which aids debugging.

♻️ Improved exception logging
     except Exception as e:
-        logger.error(f"Failed to schedule cleanup job: {e}")
+        logger.exception("Failed to schedule cleanup job")
         return None
backends/advanced/src/advanced_omi_backend/workers/audio_jobs.py (3)

214-214: Remove extraneous f prefix from string without placeholders.

♻️ Fix
-            logger.info(f"🛑 Session finalizing detected, flushing final chunks...")
+            logger.info("🛑 Session finalizing detected, flushing final chunks...")

228-228: Rename unused loop variable to indicate it's intentionally unused.

♻️ Fix
-                    for stream_name, msgs in final_messages:
+                    for _stream_name, msgs in final_messages:

353-353: Remove extraneous f prefix from string without placeholders.

♻️ Fix
-                        logger.info(f"✅ Stream empty after END signal - stopping")
+                        logger.info("✅ Stream empty after END signal - stopping")
backends/advanced/src/advanced_omi_backend/routers/modules/audio_routes.py (1)

97-107: Add exception chaining for better error traceability.

Using raise ... from e preserves the original exception context, which aids debugging.

♻️ Improved exception handling
     try:
         wav_data = await reconstruct_wav_from_conversation(conversation_id)
     except ValueError as e:
         # No chunks found for conversation
-        raise HTTPException(status_code=404, detail=str(e))
+        raise HTTPException(status_code=404, detail=str(e)) from e
     except Exception as e:
         # Reconstruction failed
-        raise HTTPException(
-            status_code=500,
-            detail=f"Failed to reconstruct audio: {str(e)}"
-        )
+        raise HTTPException(
+            status_code=500,
+            detail=f"Failed to reconstruct audio: {e!s}"
+        ) from e
backends/advanced/src/advanced_omi_backend/workers/speaker_jobs.py (2)

244-244: Remove extraneous f prefix from string without placeholders.

♻️ Fix
-            logger.info(f"🎤 Calling speaker recognition service...")
+            logger.info("🎤 Calling speaker recognition service...")

261-279: Consider using logger.exception for the ValueError case.

When logging exceptions, logger.exception provides the full stack trace automatically.

♻️ Improved exception logging
     except ValueError as e:
         # No chunks found for conversation
-        logger.error(f"No audio chunks found for conversation {conversation_id}: {e}")
+        logger.exception(f"No audio chunks found for conversation {conversation_id}")
         return {
backends/advanced/src/advanced_omi_backend/controllers/audio_controller.py (1)

95-95: Remove unused timestamp variable.

The timestamp variable is assigned but never used. Based on static analysis hint.

♻️ Proposed fix
-                timestamp = int(time.time() * 1000)
backends/advanced/src/advanced_omi_backend/controllers/conversation_controller.py (1)

249-297: Delete function handles both soft and permanent deletion correctly.

The function properly gates permanent deletion behind user.is_superuser check (line 286). However, if a non-superuser passes permanent=True, the behavior silently falls through to soft delete. Consider logging a warning when a non-admin requests permanent deletion.

♻️ Consider warning when permanent=True without admin rights
         # Hard delete (admin only, permanent flag)
         if permanent and user.is_superuser:
             return await _hard_delete_conversation(conversation)
+        elif permanent:
+            logger.warning(
+                f"User {user.user_id} requested permanent deletion without admin rights, "
+                f"falling back to soft delete for conversation {conversation_id}"
+            )

         # Soft delete (default)
         return await _soft_delete_conversation(conversation, user)
backends/advanced/src/advanced_omi_backend/routers/modules/admin_routes.py (1)

86-122: Preview endpoint looks good, consider adding audit logging.

The preview endpoint correctly counts conversations that would be deleted. Consider adding an audit log entry similar to trigger_cleanup for consistency and security audit trails.

♻️ Add audit logging for preview actions
         count = await Conversation.find(
             Conversation.deleted == True,
             Conversation.deleted_at < cutoff_date
         ).count()

+        logger.info(f"Admin {admin.email} previewed cleanup: {count} conversations (retention={retention_days} days)")
+
         return {
             "retention_days": retention_days,
backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py (5)

26-114: Opus encoding implementation is solid.

Good use of temp files with proper cleanup in finally block. FFmpeg command is well-documented with comments. One minor issue: stdout is captured but unused.

♻️ Prefix unused stdout with underscore
-            stdout, stderr = await process.communicate()
+            _stdout, stderr = await process.communicate()

117-196: Opus decoding implementation is consistent with encoding.

Same solid patterns as encoding. Same minor issue with unused stdout.

♻️ Prefix unused stdout with underscore
-            stdout, stderr = await process.communicate()
+            _stdout, stderr = await process.communicate()

507-515: Remove debug log statements before production.

Lines 507, 513, and 515 contain DEBUG-level logs with the "🔍 DEBUG:" prefix. These appear to be temporary debugging aids that should be removed or converted to proper debug-level logs without the prefix.

♻️ Clean up debug statements
-        logger.info(f"🔍 DEBUG: Setting metadata - chunks={chunk_index}, duration={total_duration:.2f}s, ratio={compression_ratio:.3f}")
-
         conversation.audio_chunks_count = chunk_index
         conversation.audio_total_duration = total_duration
         conversation.audio_compression_ratio = compression_ratio

-        logger.info(f"🔍 DEBUG: Before save - chunks={conversation.audio_chunks_count}, duration={conversation.audio_total_duration}")
         await conversation.save()
-        logger.info(f"🔍 DEBUG: After save - metadata should be persisted")

530-676: Significant code duplication with convert_audio_to_chunks.

convert_wav_to_chunks duplicates the chunking and storage logic from convert_audio_to_chunks (lines 443-527 vs 587-674). Since this function is marked as deprecated, consider refactoring it to call convert_audio_to_chunks internally after reading the WAV file.

Also contains the same debug log statements that should be removed (lines 655, 661, 663).

♻️ Reduce duplication by calling convert_audio_to_chunks
 async def convert_wav_to_chunks(
     conversation_id: str,
     wav_file_path: Path,
     chunk_duration: float = 10.0,
 ) -> int:
     """
     Convert an existing WAV file to MongoDB audio chunks.

     DEPRECATED: Use convert_audio_to_chunks() instead to avoid disk I/O.
     ...
     """
     if not wav_file_path.exists():
         raise FileNotFoundError(f"WAV file not found: {wav_file_path}")

     logger.info(f"📦 Converting WAV file to MongoDB chunks: {wav_file_path}")

     # Read WAV file
     import wave
     with wave.open(str(wav_file_path), "rb") as wav:
         sample_rate = wav.getframerate()
         channels = wav.getnchannels()
         sample_width = wav.getsampwidth()
         total_frames = wav.getnframes()
         pcm_data = wav.readframes(total_frames)

     logger.info(
         f"📁 Read WAV: {len(pcm_data)} bytes PCM, "
         f"{sample_rate}Hz, {channels}ch, {sample_width*8}-bit"
     )

+    # Delegate to in-memory conversion
+    return await convert_audio_to_chunks(
+        conversation_id=conversation_id,
+        audio_data=pcm_data,
+        sample_rate=sample_rate,
+        channels=channels,
+        sample_width=sample_width,
+        chunk_duration=chunk_duration,
+    )
-    # ... rest of duplicated code ...

704-705: Redundant imports inside function.

time and asyncio are imported inside wait_for_audio_chunks but asyncio is already imported at module level (line 13). time is not imported at module level but could be.

♻️ Use module-level imports

Add time to module-level imports:

 import asyncio
 import io
 import logging
 import tempfile
+import time
 import wave

And remove the function-level imports:

-    import time
-    import asyncio
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 387385f and 32e2e47.

📒 Files selected for processing (33)
  • backends/advanced/src/advanced_omi_backend/app_factory.py
  • backends/advanced/src/advanced_omi_backend/config.py
  • backends/advanced/src/advanced_omi_backend/controllers/audio_controller.py
  • backends/advanced/src/advanced_omi_backend/controllers/conversation_controller.py
  • backends/advanced/src/advanced_omi_backend/controllers/system_controller.py
  • backends/advanced/src/advanced_omi_backend/controllers/websocket_controller.py
  • backends/advanced/src/advanced_omi_backend/models/audio_chunk.py
  • backends/advanced/src/advanced_omi_backend/models/audio_file.py
  • backends/advanced/src/advanced_omi_backend/models/conversation.py
  • backends/advanced/src/advanced_omi_backend/models/job.py
  • backends/advanced/src/advanced_omi_backend/routers/api_router.py
  • backends/advanced/src/advanced_omi_backend/routers/modules/__init__.py
  • backends/advanced/src/advanced_omi_backend/routers/modules/admin_routes.py
  • backends/advanced/src/advanced_omi_backend/routers/modules/audio_routes.py
  • backends/advanced/src/advanced_omi_backend/routers/modules/conversation_routes.py
  • backends/advanced/src/advanced_omi_backend/routers/modules/system_routes.py
  • backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py
  • backends/advanced/src/advanced_omi_backend/utils/audio_utils.py
  • backends/advanced/src/advanced_omi_backend/utils/conversation_utils.py
  • backends/advanced/src/advanced_omi_backend/utils/gdrive_audio_utils.py
  • backends/advanced/src/advanced_omi_backend/workers/audio_jobs.py
  • backends/advanced/src/advanced_omi_backend/workers/cleanup_jobs.py
  • backends/advanced/src/advanced_omi_backend/workers/conversation_jobs.py
  • backends/advanced/src/advanced_omi_backend/workers/speaker_jobs.py
  • backends/advanced/src/advanced_omi_backend/workers/transcription_jobs.py
  • backends/advanced/tests/test_audio_persistence_mongodb.py
  • tests/.env.test
  • tests/endpoints/audio_upload_tests.robot
  • tests/infrastructure/infra_tests.robot
  • tests/integration/mongodb_audio_storage_tests.robot
  • tests/libs/mongodb_helper.py
  • tests/resources/mongodb_keywords.robot
  • tests/test-requirements.txt
💤 Files with no reviewable changes (3)
  • backends/advanced/src/advanced_omi_backend/utils/audio_utils.py
  • backends/advanced/src/advanced_omi_backend/utils/conversation_utils.py
  • backends/advanced/src/advanced_omi_backend/models/audio_file.py
🧰 Additional context used
🧬 Code graph analysis (17)
backends/advanced/src/advanced_omi_backend/models/job.py (1)
backends/advanced/src/advanced_omi_backend/models/audio_chunk.py (1)
  • AudioChunkDocument (16-158)
backends/advanced/src/advanced_omi_backend/app_factory.py (1)
backends/advanced/src/advanced_omi_backend/models/audio_chunk.py (1)
  • AudioChunkDocument (16-158)
backends/advanced/src/advanced_omi_backend/routers/modules/system_routes.py (2)
backends/advanced/src/advanced_omi_backend/config.py (1)
  • get_cleanup_settings (217-228)
backends/advanced/src/advanced_omi_backend/controllers/system_controller.py (2)
  • get_cleanup_settings_controller (130-142)
  • save_cleanup_settings_controller (145-191)
backends/advanced/src/advanced_omi_backend/controllers/websocket_controller.py (2)
backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py (1)
  • convert_audio_to_chunks (398-527)
backends/advanced/src/advanced_omi_backend/controllers/queue_controller.py (1)
  • start_post_conversation_jobs (366-515)
backends/advanced/src/advanced_omi_backend/workers/audio_jobs.py (2)
backends/advanced/src/advanced_omi_backend/models/audio_chunk.py (2)
  • AudioChunkDocument (16-158)
  • compression_ratio (140-144)
backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py (1)
  • encode_pcm_to_opus (26-114)
backends/advanced/src/advanced_omi_backend/models/audio_chunk.py (2)
backends/advanced/src/advanced_omi_backend/models/conversation.py (1)
  • Settings (323-330)
backends/advanced/src/advanced_omi_backend/models/user.py (1)
  • Settings (66-68)
backends/advanced/src/advanced_omi_backend/controllers/system_controller.py (2)
backends/advanced/src/advanced_omi_backend/config.py (3)
  • get_cleanup_settings (217-228)
  • CleanupSettings (141-144)
  • save_cleanup_settings_to_file (189-214)
backends/advanced/src/advanced_omi_backend/routers/modules/system_routes.py (1)
  • get_cleanup_settings (61-65)
backends/advanced/src/advanced_omi_backend/workers/transcription_jobs.py (2)
backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py (1)
  • reconstruct_wav_from_conversation (331-395)
backends/advanced/src/advanced_omi_backend/services/transcription/base.py (3)
  • name (64-66)
  • transcribe (45-60)
  • transcribe (125-133)
backends/advanced/src/advanced_omi_backend/workers/cleanup_jobs.py (4)
backends/advanced/src/advanced_omi_backend/models/conversation.py (1)
  • Conversation (17-330)
backends/advanced/src/advanced_omi_backend/models/audio_chunk.py (1)
  • AudioChunkDocument (16-158)
backends/advanced/src/advanced_omi_backend/config.py (1)
  • load_cleanup_settings_from_file (157-186)
backends/advanced/src/advanced_omi_backend/controllers/queue_controller.py (1)
  • get_queue (51-59)
backends/advanced/src/advanced_omi_backend/routers/modules/conversation_routes.py (1)
backends/advanced/src/advanced_omi_backend/controllers/conversation_controller.py (2)
  • delete_conversation (249-297)
  • restore_conversation (300-370)
backends/advanced/src/advanced_omi_backend/routers/modules/admin_routes.py (5)
backends/advanced/src/advanced_omi_backend/config.py (2)
  • get_cleanup_settings (217-228)
  • load_cleanup_settings_from_file (157-186)
backends/advanced/src/advanced_omi_backend/routers/modules/system_routes.py (1)
  • get_cleanup_settings (61-65)
backends/advanced/src/advanced_omi_backend/workers/cleanup_jobs.py (1)
  • purge_old_deleted_conversations (20-94)
backends/advanced/src/advanced_omi_backend/controllers/queue_controller.py (1)
  • get_queue (51-59)
backends/advanced/src/advanced_omi_backend/models/conversation.py (1)
  • Conversation (17-330)
backends/advanced/src/advanced_omi_backend/config.py (1)
backends/advanced/src/advanced_omi_backend/routers/modules/system_routes.py (1)
  • get_cleanup_settings (61-65)
backends/advanced/tests/test_audio_persistence_mongodb.py (3)
backends/advanced/src/advanced_omi_backend/models/audio_chunk.py (1)
  • AudioChunkDocument (16-158)
backends/advanced/src/advanced_omi_backend/models/conversation.py (1)
  • Conversation (17-330)
backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py (7)
  • encode_pcm_to_opus (26-114)
  • decode_opus_to_pcm (117-195)
  • build_wav_from_pcm (198-243)
  • retrieve_audio_chunks (246-288)
  • concatenate_chunks_to_pcm (291-328)
  • reconstruct_wav_from_conversation (331-395)
  • wait_for_audio_chunks (678-739)
backends/advanced/src/advanced_omi_backend/utils/gdrive_audio_utils.py (1)
backends/advanced/src/advanced_omi_backend/models/conversation.py (1)
  • Conversation (17-330)
backends/advanced/src/advanced_omi_backend/controllers/audio_controller.py (3)
backends/advanced/src/advanced_omi_backend/utils/audio_utils.py (1)
  • validate_and_prepare_audio (33-99)
backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py (1)
  • convert_audio_to_chunks (398-527)
backends/advanced/src/advanced_omi_backend/controllers/queue_controller.py (1)
  • start_post_conversation_jobs (366-515)
backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py (2)
backends/advanced/src/advanced_omi_backend/models/audio_chunk.py (2)
  • AudioChunkDocument (16-158)
  • compression_ratio (140-144)
backends/advanced/src/advanced_omi_backend/models/conversation.py (1)
  • Conversation (17-330)
backends/advanced/src/advanced_omi_backend/workers/conversation_jobs.py (2)
backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py (1)
  • wait_for_audio_chunks (678-739)
backends/advanced/src/advanced_omi_backend/utils/conversation_utils.py (1)
  • mark_conversation_deleted (518-540)
🪛 Ruff (0.14.10)
backends/advanced/src/advanced_omi_backend/routers/modules/system_routes.py

62-62: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


72-72: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)

backends/advanced/src/advanced_omi_backend/workers/speaker_jobs.py

244-244: f-string without any placeholders

Remove extraneous f prefix

(F541)


258-258: Do not catch blind exception: Exception

(BLE001)


263-263: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

backends/advanced/src/advanced_omi_backend/workers/audio_jobs.py

214-214: f-string without any placeholders

Remove extraneous f prefix

(F541)


228-228: Loop control variable stream_name not used within loop body

Rename unused stream_name to _stream_name

(B007)


248-248: Do not catch blind exception: Exception

(BLE001)


353-353: f-string without any placeholders

Remove extraneous f prefix

(F541)


359-359: Do not catch blind exception: Exception

(BLE001)

backends/advanced/src/advanced_omi_backend/models/audio_chunk.py

125-137: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

backends/advanced/src/advanced_omi_backend/controllers/system_controller.py

130-130: Unused function argument: user

(ARG001)


168-168: Prefer TypeError exception for invalid type

(TRY004)


168-168: Avoid specifying long messages outside the exception class

(TRY003)


171-171: Prefer TypeError exception for invalid type

(TRY004)


171-171: Avoid specifying long messages outside the exception class

(TRY003)


174-174: Avoid specifying long messages outside the exception class

(TRY003)

tests/libs/mongodb_helper.py

102-104: Avoid specifying long messages outside the exception class

(TRY003)

backends/advanced/src/advanced_omi_backend/workers/transcription_jobs.py

229-229: Do not catch blind exception: Exception

(BLE001)


234-234: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


234-234: Avoid specifying long messages outside the exception class

(TRY003)


237-237: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


237-237: Avoid specifying long messages outside the exception class

(TRY003)

backends/advanced/src/advanced_omi_backend/workers/cleanup_jobs.py

45-45: Avoid equality comparisons to True; use Conversation.deleted: for truth checks

Replace with Conversation.deleted

(E712)


133-133: Consider moving this statement to an else block

(TRY300)


135-135: Do not catch blind exception: Exception

(BLE001)


136-136: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

backends/advanced/src/advanced_omi_backend/routers/modules/conversation_routes.py

96-96: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


105-105: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)

backends/advanced/src/advanced_omi_backend/routers/modules/admin_routes.py

21-21: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


33-33: Unused function argument: admin

(ARG001)


33-33: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


49-49: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


78-78: Do not catch blind exception: Exception

(BLE001)


79-79: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


82-82: Use explicit conversion flag

Replace with conversion flag

(RUF010)


89-89: Unused function argument: admin

(ARG001)


89-89: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


106-106: Avoid equality comparisons to True; use Conversation.deleted: for truth checks

Replace with Conversation.deleted

(E712)


117-117: Do not catch blind exception: Exception

(BLE001)


118-118: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


121-121: Use explicit conversion flag

Replace with conversion flag

(RUF010)

backends/advanced/src/advanced_omi_backend/controllers/conversation_controller.py

188-188: Undefined name datetime

(F821)


196-196: Avoid equality comparisons to False; use not AudioChunkDocument.deleted: for false checks

Replace with not AudioChunkDocument.deleted

(E712)


200-200: Undefined name datetime

(F821)


292-292: Do not catch blind exception: Exception

(BLE001)


293-293: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


296-296: Use explicit conversion flag

Replace with conversion flag

(RUF010)


341-341: Avoid equality comparisons to True; use AudioChunkDocument.deleted: for truth checks

Replace with AudioChunkDocument.deleted

(E712)


365-365: Do not catch blind exception: Exception

(BLE001)


366-366: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


369-369: Use explicit conversion flag

Replace with conversion flag

(RUF010)

backends/advanced/src/advanced_omi_backend/config.py

180-180: Do not catch blind exception: Exception

(BLE001)


181-181: Use logging.exception instead of logging.error

Replace with exception

(TRY400)


213-213: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

backends/advanced/src/advanced_omi_backend/routers/modules/audio_routes.py

101-101: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


102-102: Do not catch blind exception: Exception

(BLE001)


104-107: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


106-106: Use explicit conversion flag

Replace with conversion flag

(RUF010)


126-126: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)


239-239: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

(B008)

backends/advanced/tests/test_audio_persistence_mongodb.py

81-81: Unused function argument: init_db

(ARG001)


119-119: Unused method argument: clean_db

(ARG002)


136-136: Unused method argument: clean_db

(ARG002)


159-159: Unused method argument: clean_db

(ARG002)


188-188: Unused method argument: clean_db

(ARG002)


220-220: Unused method argument: clean_db

(ARG002)


258-258: Unused method argument: clean_db

(ARG002)


287-287: Unused method argument: clean_db

(ARG002)


320-320: Unused method argument: clean_db

(ARG002)


330-330: Unused method argument: clean_db

(ARG002)


365-365: Unused method argument: clean_db

(ARG002)


397-397: Unused method argument: clean_db

(ARG002)


420-420: Unused method argument: clean_db

(ARG002)

backends/advanced/src/advanced_omi_backend/controllers/audio_controller.py

95-95: Local variable timestamp is assigned to but never used

Remove assignment to unused variable timestamp

(F841)

backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py

93-93: Unpacked variable stdout is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)


98-98: Avoid specifying long messages outside the exception class

(TRY003)


175-175: Unpacked variable stdout is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)


180-180: Avoid specifying long messages outside the exception class

(TRY003)


371-373: Avoid specifying long messages outside the exception class

(TRY003)


515-515: f-string without any placeholders

Remove extraneous f prefix

(F541)


564-564: Avoid specifying long messages outside the exception class

(TRY003)


663-663: f-string without any placeholders

Remove extraneous f prefix

(F541)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: robot-tests

Comment on lines +321 to +331
# Calculate byte offsets within concatenated buffer
start_offset_in_chunk = start_time - (start_chunk * CHUNK_DURATION)
end_offset_in_chunk = end_time - (end_chunk * CHUNK_DURATION)

start_byte = int(start_offset_in_chunk * bytes_per_second)
# Calculate end byte from the end of buffer
bytes_from_end = int((CHUNK_DURATION - end_offset_in_chunk) * bytes_per_second)
end_byte = len(pcm_buffer) - bytes_from_end

# Trim PCM data
trimmed_pcm = pcm_buffer[start_byte:end_byte]
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the full function around these lines to understand the context
cd backends/advanced/src/advanced_omi_backend/routers/modules && wc -l audio_routes.py

Repository: SimpleOpenSoftware/chronicle

Length of output: 91


🏁 Script executed:

# Get the function containing these lines - read a larger section
sed -n '280,360p' backends/advanced/src/advanced_omi_backend/routers/modules/audio_routes.py

Repository: SimpleOpenSoftware/chronicle

Length of output: 3079


🏁 Script executed:

# Search for the function definition to understand what precedes this code
rg -n "def.*get_audio|def.*chunk" backends/advanced/src/advanced_omi_backend/routers/modules/audio_routes.py -A 2

Repository: SimpleOpenSoftware/chronicle

Length of output: 374


🏁 Script executed:

# Check for CHUNK_DURATION definition
rg -n "CHUNK_DURATION" backends/advanced/src/advanced_omi_backend/ -B 2 -A 2

Repository: SimpleOpenSoftware/chronicle

Length of output: 4096


🏁 Script executed:

# Search for test files related to audio
find . -path "*/test*" -name "*audio*" -type f

Repository: SimpleOpenSoftware/chronicle

Length of output: 364


🏁 Script executed:

# Let's examine the test file for audio persistence
cat backends/advanced/tests/test_audio_persistence_mongodb.py

Repository: SimpleOpenSoftware/chronicle

Length of output: 14151


🏁 Script executed:

# Search for any edge case tests related to time boundaries
rg -n "end_time\|boundary\|edge" backends/advanced/tests/test_audio_persistence_mongodb.py -i -A 2

Repository: SimpleOpenSoftware/chronicle

Length of output: 54


🏁 Script executed:

# Look for test functions that test the get_audio_chunk_range function
rg -n "test.*chunk.*range\|test.*audio.*time\|get_audio_chunk_range" backends/advanced/tests/ -i

Repository: SimpleOpenSoftware/chronicle

Length of output: 54


🏁 Script executed:

# Check if there are any tests covering the specific scenario
rg -n "10\.0\|CHUNK_DURATION" backends/advanced/tests/ -A 3 -B 1

Repository: SimpleOpenSoftware/chronicle

Length of output: 54


🏁 Script executed:

# Search for tests specifically for get_audio_chunk_range in robot test files
rg -n "get_audio_chunk_range\|boundary\|edge.*case" tests/ -i -A 3

Repository: SimpleOpenSoftware/chronicle

Length of output: 54


🏁 Script executed:

# Check the concatenate_chunks_to_pcm function to understand buffer construction
rg -n "def concatenate_chunks_to_pcm" backends/advanced/src/ -A 20

Repository: SimpleOpenSoftware/chronicle

Length of output: 2210


🏁 Script executed:

# Look for any existing edge case handling or validation in the audio routes
rg -n "end_time.*=.*CHUNK_DURATION\|boundary\|edge" backends/advanced/src/advanced_omi_backend/routers/modules/audio_routes.py -i -B 2 -A 2

Repository: SimpleOpenSoftware/chronicle

Length of output: 54


🏁 Script executed:

# Get more of the concatenate_chunks_to_pcm function
sed -n '291,330p' backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py

Repository: SimpleOpenSoftware/chronicle

Length of output: 1081


🏁 Script executed:

# Let's look at the full context of the get_audio_chunk_range function
sed -n '234,340p' backends/advanced/src/advanced_omi_backend/routers/modules/audio_routes.py

Repository: SimpleOpenSoftware/chronicle

Length of output: 3959


🏁 Script executed:

# Check if there's any validation or clamping of end_time
rg -n "end_time\|num_chunks" backends/advanced/src/advanced_omi_backend/routers/modules/audio_routes.py | head -20

Repository: SimpleOpenSoftware/chronicle

Length of output: 54


🏁 Script executed:

# Let me trace through the mathematical logic with concrete values
python3 << 'EOF'
# Test case 1: end_time exactly on chunk boundary
CHUNK_DURATION = 10.0
end_time = 10.0

end_chunk = int(end_time / CHUNK_DURATION)
end_offset_in_chunk = end_time - (end_chunk * CHUNK_DURATION)

print("Case 1: end_time = 10.0 (on boundary)")
print(f"  end_chunk = int({end_time}/{CHUNK_DURATION}) = {end_chunk}")
print(f"  end_offset_in_chunk = {end_time} - ({end_chunk} * {CHUNK_DURATION}) = {end_offset_in_chunk}")

# Test case 2: end_time just before chunk boundary
end_time = 9.99
end_chunk = int(end_time / CHUNK_DURATION)
end_offset_in_chunk = end_time - (end_chunk * CHUNK_DURATION)

print("\nCase 2: end_time = 9.99 (just before boundary)")
print(f"  end_chunk = int({end_time}/{CHUNK_DURATION}) = {end_chunk}")
print(f"  end_offset_in_chunk = {end_time} - ({end_chunk} * {CHUNK_DURATION}) = {end_offset_in_chunk}")

# Test case 3: start_time=5.0, end_time=10.0
print("\nCase 3: start_time=5.0, end_time=10.0")
start_time = 5.0
end_time = 10.0
start_chunk = int(start_time / CHUNK_DURATION)
end_chunk = int(end_time / CHUNK_DURATION)
num_chunks = end_chunk - start_chunk + 1

print(f"  start_chunk = {start_chunk}, end_chunk = {end_chunk}")
print(f"  num_chunks = {end_chunk} - {start_chunk} + 1 = {num_chunks}")
print(f"  This retrieves chunks [{start_chunk}, {end_chunk}]")

# Test case 4: Boundary analysis
print("\nBoundary analysis for different end_time values:")
for end_time in [10.0, 10.1, 19.9, 20.0, 20.1]:
    end_chunk = int(end_time / CHUNK_DURATION)
    end_offset_in_chunk = end_time - (end_chunk * CHUNK_DURATION)
    bytes_per_second = 32000
    bytes_from_end = int((CHUNK_DURATION - end_offset_in_chunk) * bytes_per_second)
    print(f"  end_time={end_time}: end_chunk={end_chunk}, end_offset={end_offset_in_chunk:.1f}, bytes_from_end={bytes_from_end}")

EOF

Repository: SimpleOpenSoftware/chronicle

Length of output: 158


🏁 Script executed:

# Check if there's any validation or boundary condition handling
rg -n "int.*end_time\|end_chunk\|num_chunks" backends/advanced/src/advanced_omi_backend/routers/modules/audio_routes.py -B 2 -A 2

Repository: SimpleOpenSoftware/chronicle

Length of output: 54


🏁 Script executed:

# Let me do a simpler analysis of the mathematical logic
rg -n "end_chunk\|start_chunk\|num_chunks" backends/advanced/src/advanced_omi_backend/routers/modules/audio_routes.py

Repository: SimpleOpenSoftware/chronicle

Length of output: 54


🏁 Script executed:

# Check what happens when requesting chunks
rg -n "retrieve_audio_chunks" backends/advanced/src/advanced_omi_backend/routers/modules/audio_routes.py -B 5 -A 5

Repository: SimpleOpenSoftware/chronicle

Length of output: 1338


🏁 Script executed:

# Let me verify the edge case logic by examining what happens with partial last chunks
# Search for how last chunks are handled
rg -n "audio_total_duration\|last.*chunk\|partial" backends/advanced/src/advanced_omi_backend/ -i -B 2 -A 2

Repository: SimpleOpenSoftware/chronicle

Length of output: 54


🏁 Script executed:

# Let's check if there's any clipping or rounding of end_time to actual available duration
sed -n '280,295p' backends/advanced/src/advanced_omi_backend/routers/modules/audio_routes.py

Repository: SimpleOpenSoftware/chronicle

Length of output: 797


🏁 Script executed:

# Look for how chunks are created and if they always have CHUNK_DURATION seconds
rg -n "audio_total_duration\|end_time.*=\|duration.*=" backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py | head -20

Repository: SimpleOpenSoftware/chronicle

Length of output: 54


🏁 Script executed:

# Check how chunks are stored - do they always represent exactly CHUNK_DURATION?
rg -n "duration.*=\|end_time.*=" backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py -B 2 -A 2 | head -40

Repository: SimpleOpenSoftware/chronicle

Length of output: 54


🏁 Script executed:

# Look at convert_wav_to_chunks to see how the last chunk is handled
rg -n "def convert_wav_to_chunks" backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py -A 40

Repository: SimpleOpenSoftware/chronicle

Length of output: 1625


🏁 Script executed:

# Look at the actual chunk creation to see if chunks can have partial duration
sed -n '530,620p' backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py

Repository: SimpleOpenSoftware/chronicle

Length of output: 3008


🏁 Script executed:

# Check for any comments about last chunk handling
rg -n "last.*chunk\|partial\|remainder" backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py -i -B 2 -A 2

Repository: SimpleOpenSoftware/chronicle

Length of output: 54


🏁 Script executed:

# Get the rest of the chunk creation to see the stored duration
sed -n '600,650p' backends/advanced/src/advanced_omi_backend/utils/audio_chunk_utils.py

Repository: SimpleOpenSoftware/chronicle

Length of output: 1610


🏁 Script executed:

# Search for where chunks store their actual duration
rg -n "AudioChunkDocument\|duration\|end_time" backends/advanced/src/advanced_omi_backend/models/audio_chunk.py

Repository: SimpleOpenSoftware/chronicle

Length of output: 54


🏁 Script executed:

# Let me verify by checking the audio model to confirm chunks store actual duration
cat backends/advanced/src/advanced_omi_backend/models/audio_chunk.py

Repository: SimpleOpenSoftware/chronicle

Length of output: 4806


🏁 Script executed:

# Let me verify the exact problematic scenario with a concrete test
python3 << 'EOF'
# Critical edge case: When requesting end_time on a partial chunk boundary

# Scenario: Audio is 25 seconds total (chunks: 0-10, 10-20, 20-25)
# Request: start_time=10.0, end_time=20.0 (exactly on chunk 2 boundary, which is partial)

CHUNK_DURATION = 10.0
bytes_per_second = 32000

# Request parameters
start_time = 10.0
end_time = 20.0

# Calculate chunk indices
start_chunk = int(start_time / CHUNK_DURATION)  # 1
end_chunk = int(end_time / CHUNK_DURATION)      # 2
num_chunks = end_chunk - start_chunk + 1        # 2

print(f"Request: start_time={start_time}, end_time={end_time}")
print(f"Chunks retrieved: {list(range(start_chunk, end_chunk + 1))}")
print(f"Num chunks to retrieve: {num_chunks}")

# Buffer construction (chunks 1 and 2, where 2 is partial at 5 seconds)
chunk1_bytes = 10 * bytes_per_second  # 320000
chunk2_bytes = 5 * bytes_per_second   # 160000 (partial)
pcm_buffer_len = chunk1_bytes + chunk2_bytes  # 480000

print(f"\nBuffer construction:")
print(f"  Chunk 1: {chunk1_bytes} bytes (10s)")
print(f"  Chunk 2: {chunk2_bytes} bytes (5s partial)")
print(f"  Total buffer: {pcm_buffer_len} bytes")

# Trim calculation
start_offset_in_chunk = start_time - (start_chunk * CHUNK_DURATION)  # 0
end_offset_in_chunk = end_time - (end_chunk * CHUNK_DURATION)        # 0

start_byte = int(start_offset_in_chunk * bytes_per_second)
bytes_from_end = int((CHUNK_DURATION - end_offset_in_chunk) * bytes_per_second)
end_byte = pcm_buffer_len - bytes_from_end

print(f"\nTrim calculation:")
print(f"  start_offset_in_chunk: {start_offset_in_chunk}")
print(f"  end_offset_in_chunk: {end_offset_in_chunk}")
print(f"  start_byte: {start_byte}")
print(f"  bytes_from_end: {bytes_from_end}")
print(f"  end_byte: {end_byte}")
print(f"  Result slice: [{start_byte}:{end_byte}]")

if end_byte < start_byte:
    print(f"\n⚠️  NEGATIVE SLICE! end_byte ({end_byte}) < start_byte ({start_byte})")
elif end_byte > pcm_buffer_len:
    print(f"\n⚠️  OVERFLOW! end_byte ({end_byte}) > buffer length ({pcm_buffer_len})")
else:
    print(f"\n✓ Slice is valid")

print("\n" + "="*60)
print("CRITICAL CASE: end_time exactly on partial chunk boundary")
print("="*60)

# More dangerous scenario
start_time = 15.0
end_time = 20.0
start_chunk = int(start_time / CHUNK_DURATION)
end_chunk = int(end_time / CHUNK_DURATION)

print(f"\nRequest: start_time={start_time}, end_time={end_time}")
print(f"Chunks: {start_chunk} to {end_chunk}")

start_offset = start_time - (start_chunk * CHUNK_DURATION)
end_offset = end_time - (end_chunk * CHUNK_DURATION)
print(f"Offsets: start={start_offset}, end={end_offset}")

bytes_from_end_calc = int((CHUNK_DURATION - end_offset) * bytes_per_second)
print(f"bytes_from_end = int(({CHUNK_DURATION} - {end_offset}) * {bytes_per_second}) = {bytes_from_end_calc}")

# If we only got chunk 2 (5 seconds = 160000 bytes)
if end_chunk == 2:  # Partial chunk
    buffer_len = 5 * bytes_per_second
    end_byte_calc = buffer_len - bytes_from_end_calc
    print(f"\nIf buffer has only chunk 2 (partial, {buffer_len} bytes):")
    print(f"  end_byte = {buffer_len} - {bytes_from_end_calc} = {end_byte_calc}")
    if end_byte_calc < 0:
        print(f"  ⚠️  NEGATIVE END_BYTE! This will cause incorrect slicing!")

EOF

Repository: SimpleOpenSoftware/chronicle

Length of output: 929


Add bounds checking for the end_byte calculation to handle partial final chunks.

When end_time falls exactly on a chunk boundary and the last retrieved chunk is partial (shorter than CHUNK_DURATION), the calculation of bytes_from_end will overflow, resulting in a negative end_byte. For example, requesting audio from a 25-second conversation with end_time=20.0 causes bytes_from_end = int((10.0 - 0.0) * 32000) = 320000, but the buffer only contains 160000 bytes (chunk 2 is 5 seconds), yielding end_byte = -160000. This creates an incorrect reverse slice.

The fix should account for actual chunk durations rather than assuming all chunks are exactly CHUNK_DURATION seconds. Consider using the stored duration field from retrieved chunks or clamping end_byte to valid bounds: end_byte = max(start_byte, min(end_byte, len(pcm_buffer))).

🤖 Prompt for AI Agents
In @backends/advanced/src/advanced_omi_backend/routers/modules/audio_routes.py
around lines 321 - 331, The end_byte calculation can go negative for partial
final chunks because bytes_from_end assumes CHUNK_DURATION; update the logic
around start_offset_in_chunk/end_offset_in_chunk/bytes_from_end/end_byte to use
the actual retrieved chunk duration (e.g., use each chunk's stored duration
field) when converting to bytes, and then clamp the resulting end_byte to valid
bounds via end_byte = max(start_byte, min(end_byte, len(pcm_buffer))) to prevent
negative or out-of-range slices of pcm_buffer.

Comment on lines +282 to +310
# Keep sending audio in a loop to prevent inactivity timeout while waiting for conversation
# We need to continuously send audio because SPEECH_INACTIVITY_THRESHOLD_SECONDS=2
FOR ${i} IN RANGE 20 # Send 20 batches while waiting
# Try to get conversation job
${conv_jobs}= Get Jobs By Type And Client open_conversation ${client_id}
${has_job}= Evaluate len($conv_jobs) > 0

IF ${has_job}
# Conversation job exists, try to get conversation_id
TRY
${conversation_id}= Get Conversation ID From Job Meta open_conversation ${client_id}
# Got conversation_id! Close websocket immediately to trigger disconnect
Log To Console Conversation created (${conversation_id}), closing websocket NOW
Close Audio Stream ${stream_id}
BREAK
EXCEPT
# conversation_id not set yet, keep sending audio
Send Audio Chunks To Stream ${stream_id} ${TEST_AUDIO_FILE} num_chunks=50
Sleep 1s
END
ELSE
# No conversation job yet, keep sending audio
Send Audio Chunks To Stream ${stream_id} ${TEST_AUDIO_FILE} num_chunks=50
Sleep 1s
END
END

# Simulate WebSocket disconnect (Bluetooth dropout)
Close Audio Stream ${stream_id}
# Verify we got the conversation_id before loop ended
Should Not Be Equal ${conversation_id} ${None} Failed to get conversation_id within timeout
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Bug: ${conversation_id} may be undefined when the loop exhausts iterations.

If the loop completes all 20 iterations without successfully retrieving a conversation_id, line 310's assertion will fail with a "variable not found" error rather than the intended failure message. Initialize the variable before the loop.

🐛 Proposed fix
     # Keep sending audio in a loop to prevent inactivity timeout while waiting for conversation
     # We need to continuously send audio because SPEECH_INACTIVITY_THRESHOLD_SECONDS=2
+    ${conversation_id}=    Set Variable    ${None}
     FOR    ${i}    IN RANGE    20    # Send 20 batches while waiting
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Keep sending audio in a loop to prevent inactivity timeout while waiting for conversation
# We need to continuously send audio because SPEECH_INACTIVITY_THRESHOLD_SECONDS=2
FOR ${i} IN RANGE 20 # Send 20 batches while waiting
# Try to get conversation job
${conv_jobs}= Get Jobs By Type And Client open_conversation ${client_id}
${has_job}= Evaluate len($conv_jobs) > 0
IF ${has_job}
# Conversation job exists, try to get conversation_id
TRY
${conversation_id}= Get Conversation ID From Job Meta open_conversation ${client_id}
# Got conversation_id! Close websocket immediately to trigger disconnect
Log To Console Conversation created (${conversation_id}), closing websocket NOW
Close Audio Stream ${stream_id}
BREAK
EXCEPT
# conversation_id not set yet, keep sending audio
Send Audio Chunks To Stream ${stream_id} ${TEST_AUDIO_FILE} num_chunks=50
Sleep 1s
END
ELSE
# No conversation job yet, keep sending audio
Send Audio Chunks To Stream ${stream_id} ${TEST_AUDIO_FILE} num_chunks=50
Sleep 1s
END
END
# Simulate WebSocket disconnect (Bluetooth dropout)
Close Audio Stream ${stream_id}
# Verify we got the conversation_id before loop ended
Should Not Be Equal ${conversation_id} ${None} Failed to get conversation_id within timeout
# Keep sending audio in a loop to prevent inactivity timeout while waiting for conversation
# We need to continuously send audio because SPEECH_INACTIVITY_THRESHOLD_SECONDS=2
${conversation_id}= Set Variable ${None}
FOR ${i} IN RANGE 20 # Send 20 batches while waiting
# Try to get conversation job
${conv_jobs}= Get Jobs By Type And Client open_conversation ${client_id}
${has_job}= Evaluate len($conv_jobs) > 0
IF ${has_job}
# Conversation job exists, try to get conversation_id
TRY
${conversation_id}= Get Conversation ID From Job Meta open_conversation ${client_id}
# Got conversation_id! Close websocket immediately to trigger disconnect
Log To Console Conversation created (${conversation_id}), closing websocket NOW
Close Audio Stream ${stream_id}
BREAK
EXCEPT
# conversation_id not set yet, keep sending audio
Send Audio Chunks To Stream ${stream_id} ${TEST_AUDIO_FILE} num_chunks=50
Sleep 1s
END
ELSE
# No conversation job yet, keep sending audio
Send Audio Chunks To Stream ${stream_id} ${TEST_AUDIO_FILE} num_chunks=50
Sleep 1s
END
END
# Verify we got the conversation_id before loop ended
Should Not Be Equal ${conversation_id} ${None} Failed to get conversation_id within timeout
🤖 Prompt for AI Agents
In @tests/infrastructure/infra_tests.robot around lines 282 - 310, The variable
${conversation_id} may be undefined if the FOR loop finishes without finding a
job; before the loop starts initialize ${conversation_id} to a known value
(e.g., Set Variable ${None}) so the final assertion (Should Not Be Equal
${conversation_id} ${None}) fails with the intended message; keep the rest of
the logic (Get Conversation ID From Job Meta, Close Audio Stream, Send Audio
Chunks To Stream) unchanged.

@github-actions
Copy link

⚠️ Robot Framework Test Results

Status: ❌ Some tests failed

Metric Count
✅ Passed 103
❌ Failed 12
📊 Total 115

📊 View Reports

GitHub Pages (Live Reports):

Download Artifacts:


View full workflow run

…andling

- Removed audio file path parameters from various functions, transitioning to audio data retrieval from MongoDB chunks.
- Updated the `start_post_conversation_jobs` function to reflect changes in audio handling, ensuring jobs reconstruct audio from database chunks.
- Enhanced the `transcribe_full_audio_job` and `recognise_speakers_job` to process audio directly from memory, eliminating the need for temporary files.
- Improved error handling and logging for audio data retrieval, ensuring better feedback during processing.
- Added a new utility function for converting PCM data to WAV format in memory, streamlining audio format handling.
- Updated methods to accept audio data as bytes instead of file paths, enhancing performance by eliminating disk I/O.
- Improved logging to reflect in-memory audio processing, providing better insights during speaker identification and diarization.
- Streamlined audio data handling in the `diarize_identify_match` and `diarize_and_identify` methods, ensuring consistency across the client.
- Removed temporary file handling, simplifying the audio processing workflow and reducing potential file system errors.
@github-actions
Copy link

⚠️ Robot Framework Test Results

Status: ❌ Some tests failed

Metric Count
✅ Passed 111
❌ Failed 4
📊 Total 115

📊 View Reports

GitHub Pages (Live Reports):

Download Artifacts:


View full workflow run

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