Skip to content

Comments

Fix deepgram/parakeet workers bug #199

Merged
AnkushMalaker merged 5 commits intomainfrom
fix/openmemory-cache
Dec 18, 2025
Merged

Fix deepgram/parakeet workers bug #199
AnkushMalaker merged 5 commits intomainfrom
fix/openmemory-cache

Conversation

@AnkushMalaker
Copy link
Collaborator

@AnkushMalaker AnkushMalaker commented Dec 18, 2025

Summary by CodeRabbit

Release Notes

  • New Features

    • Conditional streaming worker startup optimizes deployment based on configuration availability.
  • Chores

    • Updated backend service infrastructure configuration.
    • Standardized Docker network naming conventions.
    • Simplified deployment process with removal of legacy initialization scripts.
    • Updated service endpoints and deployment instructions.

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

…se.yml for OpenMemory MCP

- Added conditional start for Deepgram worker based on DEEPGRAM_API_KEY presence.
- Updated shutdown and monitoring functions to handle AUDIO_STREAM_WORKER_PID safely.
- Changed OpenMemory MCP service to use the official Docker image instead of a local build.
- Removed deprecated init-cache.sh script and streamlined setup.sh for clarity.
- Introduced conditional starting of Parakeet worker based on PARAKEET_ASR_URL or OFFLINE_ASR_TCP_URI presence.
- Updated variable names for clarity, changing AUDIO_STREAM_WORKER_PID to AUDIO_STREAM_DEEPGRAM_WORKER_PID and added AUDIO_STREAM_PARAKEET_WORKER_PID.
- Improved shutdown and monitoring functions to handle new worker PIDs safely.
…ckend for all proxy configurations

- Changed reverse proxy targets for WebSocket, API, Auth, Health, Readiness, Users, and Audio endpoints from friend-backend:8000 to chronicle-backend:8000.
- Ensured consistency across all relevant sections of the Caddyfile.template.
- Changed the default network name in backends/advanced/docker-compose.yml from friend-network to chronicle-network.
- Updated extras/openmemory-mcp/docker-compose.yml to include chronicle-network as an external network and added it to the services configuration.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 18, 2025

Walkthrough

This PR updates infrastructure naming conventions and service configurations across the backend and extras modules. It renames the Docker network and backend service from friend-based names to chronicle-based names, introduces conditional startup logic for audio streaming workers based on environment variables, migrates OpenMemory MCP to use an official Docker image, and removes cache initialization scripts.

Changes

Cohort / File(s) Summary
Network and backend service renaming
backends/advanced/Caddyfile.template, backends/advanced/docker-compose.yml, extras/speaker-recognition/docker-compose.yml
Updated all references from friend-network/friend-backend to chronicle-network/chronicle-backend; added X-Forwarded-Proto header to WebSocket proxy block in Caddyfile.
Audio worker conditional startup
backends/advanced/start-workers.sh
Added conditional logic to start Deepgram and Parakeet audio stream workers only when their respective environment variables are set; introduced separate PID tracking variables for each worker; updated status reporting, self-healing monitor, and shutdown routines to handle conditional worker termination.
OpenMemory MCP migration to official image
extras/openmemory-mcp/docker-compose.yml
Changed from local build to official Docker image (mem0/openmemory-mcp:latest); added chronicle-network and updated UI configuration with restart policies and new default network definitions.
OpenMemory MCP setup simplification
extras/openmemory-mcp/setup.sh
Removed local mem0 fork cloning and checkout logic; updated startup instructions and changed web UI endpoint from localhost:8765 to localhost:3001.
Cache initialization removal
extras/openmemory-mcp/init-cache.sh
Entire script deleted; removed local cache initialization and update capability from external fork.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

  • start-workers.sh: Review conditional worker startup logic, PID tracking variables, and self-healing monitor adjustments carefully to ensure proper cleanup and process management.
  • OpenMemory MCP changes: Verify that migration to official image doesn't break existing workflows; confirm network connectivity with new chronicle-network.
  • Caddyfile header addition: Ensure X-Forwarded-Proto header addition for WebSocket proxy doesn't cause downstream issues.

Possibly related PRs

  • chronicler-ai/chronicle#150: Adds conditional Deepgram/Parakeet worker startup and speaker-recognition docker-compose network updates with closely aligned changes.
  • chronicler-ai/chronicle#74: Updates OpenMemory MCP configuration and scripts with similar infrastructure migration patterns.
  • chronicler-ai/chronicle#134: Modifies speaker-recognition/docker-compose.yml network configuration with related external network definitions.

Poem

🐰 A whisker-twitching tale of renaming spree,
From friend to chronicle, as it must be,
Workers leap conditionally, precise and neat,
Networks align, the infrastructure's complete!
Mem0 trades home for the Docker cloud,
A rabbit's delight—configuration quite proud!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title claims to fix a 'deepgram/parakeet workers bug', but the actual changes span multiple unrelated concerns: network name changes (friend-network to chronicle-network), backend proxy target changes (friend-backend to chronicle-backend), backend proxy header additions, OpenMemory MCP image migration, OpenMemory setup/cache script modifications, and speaker recognition network updates. Only one file (start-workers.sh) is directly related to the titled issue. Update the PR title to accurately reflect the broader scope of changes, such as: 'Update network names and backend configurations, fix worker startup conditions' or split into multiple focused PRs with specific titles.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/openmemory-cache

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

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: 2

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4178235 and 5e2220b.

📒 Files selected for processing (7)
  • backends/advanced/Caddyfile.template (3 hunks)
  • backends/advanced/docker-compose.yml (1 hunks)
  • backends/advanced/start-workers.sh (5 hunks)
  • extras/openmemory-mcp/docker-compose.yml (3 hunks)
  • extras/openmemory-mcp/init-cache.sh (0 hunks)
  • extras/openmemory-mcp/setup.sh (1 hunks)
  • extras/speaker-recognition/docker-compose.yml (1 hunks)
💤 Files with no reviewable changes (1)
  • extras/openmemory-mcp/init-cache.sh
🧰 Additional context used
📓 Path-based instructions (1)
**/docker-compose*.{yml,yaml}

📄 CodeRabbit inference engine (CLAUDE.md)

Check if src/ is volume mounted. If not, run docker compose build so that code changes are reflected. Do not simply run docker compose restart as it will not rebuild the image.

Files:

  • backends/advanced/docker-compose.yml
  • extras/speaker-recognition/docker-compose.yml
  • extras/openmemory-mcp/docker-compose.yml
⏰ 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: speaker-recognition-tests
🔇 Additional comments (11)
backends/advanced/start-workers.sh (5)

54-73: LGTM! Conditional worker startup logic is well-implemented.

The conditional startup for both Deepgram and Parakeet workers correctly checks for required environment variables before starting the respective workers. Setting PID variables to empty strings when workers are skipped ensures proper behavior in later conditional checks throughout the script.


83-84: LGTM! Status output properly reflects conditional workers.

The conditional echo statements correctly display worker PIDs only when the workers are actually started, providing accurate runtime information.


124-126: LGTM! Self-healing monitor properly handles conditional workers.

The monitor correctly kills only active workers based on their PID variables, ensuring that workers that were never started are not targeted. The use of || true properly prevents script failure when attempting to kill non-existent processes.


151-152: LGTM! Shutdown handler properly manages conditional workers.

Consistent with the self-healing monitor, the shutdown function correctly handles conditional workers by checking PID variables before attempting to kill processes.


185-186: LGTM! Final termination path consistently handles conditional workers.

The implementation maintains consistency with the other kill operations in the script, ensuring reliable cleanup of all active workers.

backends/advanced/docker-compose.yml (1)

229-229: LGTM! Network rename aligns with PR objectives.

The network name change from friend-network to chronicle-network is consistent with the infrastructure naming convention updates across the repository.

backends/advanced/Caddyfile.template (2)

22-27: LGTM! Backend rename and WebSocket header improvement.

The proxy target update to chronicle-backend:8000 aligns with the service name in docker-compose.yml. The addition of X-Forwarded-Proto {scheme} header ensures proper protocol scheme information is forwarded to the backend, which is particularly important for WebSocket connections.


32-56: LGTM! Consistent backend proxy target updates.

All backend proxy targets have been consistently updated to chronicle-backend:8000 across all API, auth, health, user, and audio endpoints, ensuring uniform routing to the renamed service.

extras/openmemory-mcp/setup.sh (1)

76-78: LGTM! Instructions updated for official Docker image usage.

The removal of --build flag is appropriate since the service now uses the official mem0/openmemory-mcp:latest image. The Web UI URL correctly reflects the new port mapping (3001) defined in docker-compose.yml.

extras/openmemory-mcp/docker-compose.yml (2)

34-44: LGTM! UI service improvements enhance reliability.

The addition of restart: unless-stopped policy improves service reliability, and the updated comment accurately reflects that the UI now starts by default alongside the MCP server.


11-32: Verify if chronicle-network connectivity is required for this MCP service.

The switch to the official mem0/openmemory-mcp:latest image is consistent with removing cache initialization scripts. However, the addition of chronicle-network connectivity creates a startup dependency that isn't documented in the standard OpenMemory MCP architecture. Please confirm whether the MCP service actually communicates with services on this network, or if this should be removed.

Comment on lines +46 to +50
networks:
default:
name: openmemory-mcp_default
chronicle-network:
external: true No newline at end of file
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

The external chronicle-network dependency requires backend services to start first.

The chronicle-network is marked external: true, which means this compose file depends on the network being pre-created by backend services. This matches the pattern in extras/speaker-recognition/docker-compose.yml and creates an implicit service startup order requirement. Users must start the backend services before running docker compose up in this directory.

🤖 Prompt for AI Agents
In extras/openmemory-mcp/docker-compose.yml around lines 46 to 50, the
chronicle-network is marked external: true which creates an implicit dependency
on backend services having pre-created that network; change the network to be
created by this compose file instead of requiring an external network by
removing external: true and defining the network locally (set an explicit name
and optional driver) so docker compose up will create it automatically; ensure
the network section uses a local definition consistent with other compose files
(e.g., provide name: openmemory-chronicle-network and remove external: true).

Comment on lines +111 to 112
name: chronicle-network
external: true No newline at end of file
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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for docker-compose files and look for chronicle-network
echo "=== Finding docker-compose files ==="
find . -name "docker-compose*.yml" -o -name "docker-compose*.yaml" | head -20

echo -e "\n=== Searching for chronicle-network in compose files ==="
rg -l "chronicle-network" -t yaml

echo -e "\n=== Examining network definitions ==="
rg "networks:" -A 5 -t yaml | head -50

echo -e "\n=== Looking for documentation about startup order ==="
find . -name "*.md" -o -name "*.txt" -o -name "README*" | xargs grep -l "startup\|start.*backend\|chronicle-network" 2>/dev/null | head -10

Repository: chronicler-ai/chronicle

Length of output: 3238


🏁 Script executed:

#!/bin/bash
# Check documentation files for startup order guidance
echo "=== Checking getting-started.md ==="
grep -i "startup\|start.*backend\|docker.*compose\|order\|first\|second" ./Docs/getting-started.md | head -15

echo -e "\n=== Checking quickstart.md ==="
grep -i "startup\|start.*backend\|docker.*compose\|order\|first\|second" ./backends/advanced/Docs/quickstart.md | head -15

echo -e "\n=== Checking README.md ==="
grep -i "startup\|start.*backend\|docker.*compose\|order\|first\|second" ./README.md | head -15

echo -e "\n=== Full content of getting-started.md (first 100 lines) ==="
head -100 ./Docs/getting-started.md

echo -e "\n=== Checking if speaker-recognition or extras are mentioned in getting-started ==="
grep -i "speaker\|extras\|asr\|recognition" ./Docs/getting-started.md

Repository: chronicler-ai/chronicle

Length of output: 5977


🏁 Script executed:

#!/bin/bash
# Check the specific lines 111-112 in speaker-recognition/docker-compose.yml
echo "=== Lines 111-112 of extras/speaker-recognition/docker-compose.yml ==="
sed -n '110,115p' ./extras/speaker-recognition/docker-compose.yml

echo -e "\n=== Context around network definition (lines 105-115) ==="
sed -n '105,115p' ./extras/speaker-recognition/docker-compose.yml

echo -e "\n=== Network definition in backends/advanced/docker-compose.yml ==="
grep -A 3 "^networks:" ./backends/advanced/docker-compose.yml

echo -e "\n=== Verify backends/advanced network does NOT have external: true ==="
grep -A 3 "^networks:" ./backends/advanced/docker-compose.yml | grep -i external || echo "No 'external' keyword found - network is created by this compose file"

Repository: chronicler-ai/chronicle

Length of output: 665


Document the chronicle-network external dependency requirement.

The chronicle-network is marked as external: true, requiring it to exist before this compose stack starts. While getting-started.md shows the correct startup sequence (backends/advanced first, then extras), add explicit documentation explaining that speaker-recognition depends on the network created by backends/advanced. This prevents confusion if users attempt to run speaker-recognition independently.

🤖 Prompt for AI Agents
In extras/speaker-recognition/docker-compose.yml around lines 111 to 112, the
compose file marks the chronicle-network as external:true but doesn't document
this dependency; update the repository docs by adding a brief note in
extras/speaker-recognition/README.md (or getting-started.md near the
speaker-recognition section) stating that speaker-recognition requires the
chronicle-network to be created by running backends/advanced first (include the
exact docker network name chronicle-network and the startup order:
backends/advanced -> extras/speaker-recognition), and add a one-line reminder in
the docker-compose.yml header comment referencing that external network and
linking to the doc for the startup steps.

@AnkushMalaker AnkushMalaker merged commit f8372d7 into main Dec 18, 2025
6 checks passed
@AnkushMalaker AnkushMalaker deleted the fix/openmemory-cache branch December 18, 2025 02:29
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