feat: implement Mem0 memory backend adapter#338
Conversation
Add concrete MemoryBackend implementation using Mem0 (embedded Qdrant + SQLite) as the storage layer, unblocking all downstream memory features. - Mem0MemoryBackend implements MemoryBackend, MemoryCapabilities, and SharedKnowledgeStore protocols with asyncio.to_thread for all sync Mem0 calls - Mapping layer (mappers.py) for bidirectional domain model <-> Mem0 dict conversion with _synthorg_ metadata prefix - Mem0BackendConfig + config builder deriving from CompanyMemoryConfig - Factory wired up with deferred import (no longer raises MemoryConfigError for mem0 backend) - Shared knowledge via reserved __synthorg_shared__ namespace with publisher ownership tracking - 95 unit tests (adapter, mappers, config) + 6 integration tests (retrieval pipeline, shared knowledge flow) Closes #206
Pre-reviewed by 9 agents, 33 findings addressed: - Remove vendor-specific defaults from Mem0EmbedderConfig (now required) - Fix metadata dict mutation in publish() (use spread instead) - Add path traversal validation on Mem0BackendConfig.data_dir - Extract _validate_add_result helper for store/publish result validation - Add explicit ImportError handling in connect() - Make health_check() probe backend with lightweight get_all call - Add defensive category parsing in _extract_category (handle invalid enums) - Add missing publisher check in retract() (not a shared memory entry) - Replace C-style loop in search_shared with generator expression - Use meaningful variable names (raw_entries/filtered vs triple result) - Change logger.exception to logger.warning for self-raised errors - Add structured error/error_type kwargs to all re-raise logs - Add MemoryConnectionError to Raises docstrings for shared methods - Update factory to require embedder config (no vendor defaults) - Remove dead store_request_to_mem0_args function and its tests - Add tests: path traversal, missing id, empty content, malformed datetime, invalid category, no-publisher retract, delete-after-get failure, protocol conformance, health_check probe failure
…iewers - Add MemoryError/RecursionError guards before all except Exception blocks - Capture exceptions in re-raise blocks for structured logging - Move helper functions (validate_add_result, extract_category, extract_publisher) from adapter.py to mappers.py to keep adapter under 800-line limit - Add type/tag/confidence validation in mappers (items 9, 11, 23) - Add logging before all raise statements (items 7, 20, 21) - Fix health_check exception log level from debug to warning - Add search_shared query context to error logs - Strengthen factory.py type annotation (embedder: Mem0EmbedderConfig | None) - Parametrize config traversal tests, add ImportError/MemoryError/publish tests - Update CLAUDE.md, README, roadmap, and design spec for Mem0 adapter status
- Fix dependency review CI: add LicenseRef-scancode-protobuf, ZPL-2.1 to license allow-list and allow-dependencies-licenses for packages with null SPDX metadata (mem0ai, numpy, qdrant-client, posthog) - Fix validate_add_result blank-ID gap: check for None and whitespace IDs, not just missing key (Greptile finding) - Fix factory.py wrong event constant: use MEMORY_BACKEND_CONFIG_INVALID instead of MEMORY_BACKEND_UNKNOWN for embedder config errors (Greptile) - Fix retract() bare except: capture as exc and log error detail - Document count() limitation: capped at max_memories_per_agent - Add 37 new tests: validate_add_result (blank/None/whitespace/numeric ID, non-list results), extract_category, extract_publisher, MemoryError re-raise for all operations, blank-ID through store/retrieve, shared namespace fallback, count empty results, retract delete failure
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF ScorecardScorecard details
Scanned Files
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a concrete Mem0-backed MemoryBackend: new mem0 backend package (config, adapter, mappers), factory wiring and exports, dependency and CI updates, observability event constants, docs updates, and extensive unit + integration tests. Also removes many inline test noqa comments (stylistic). Changes
Sequence Diagram(s)sequenceDiagram
participant Agent
participant Mem0Backend as Mem0MemoryBackend
participant Mem0Lib as Mem0 Client
participant VectorStore as Vector Store
participant SQLite as SQLite History
Agent->>Mem0Backend: store(agent_id, request)
activate Mem0Backend
Mem0Backend->>Mem0Lib: add(name, messages, metadata)
activate Mem0Lib
Mem0Lib->>VectorStore: embed & index memory
Mem0Lib->>SQLite: persist metadata/history
Mem0Lib-->>Mem0Backend: {id, ...}
deactivate Mem0Lib
Mem0Backend->>Mem0Backend: validate_add_result()
Mem0Backend-->>Agent: memory_id
deactivate Mem0Backend
Agent->>Mem0Backend: retrieve(agent_id, query)
activate Mem0Backend
Mem0Backend->>Mem0Lib: search(query, agent_id)
activate Mem0Lib
Mem0Lib->>VectorStore: vector similarity search
Mem0Lib->>SQLite: fetch metadata
Mem0Lib-->>Mem0Backend: [result1, result2, ...]
deactivate Mem0Lib
Mem0Backend->>Mem0Backend: mem0_result_to_entry()<br/>apply_post_filters()
Mem0Backend-->>Agent: tuple[MemoryEntry, ...]
deactivate Mem0Backend
sequenceDiagram
participant Agent as Agent A
participant Mem0Backend
participant Mem0Lib as Mem0 Client
participant SharedNS as Shared Namespace
Agent->>Mem0Backend: publish(agent_id, request)
activate Mem0Backend
Mem0Backend->>Mem0Backend: build_mem0_metadata(add publisher)
Mem0Backend->>Mem0Lib: add(..., user_id=shared)
activate Mem0Lib
Mem0Lib->>SharedNS: store in shared namespace
Mem0Lib-->>Mem0Backend: memory_id
deactivate Mem0Lib
Mem0Backend-->>Agent: shared_memory_id
deactivate Mem0Backend
rect rgba(100,150,200,0.5)
Agent B->>Mem0Backend: search_shared(query, exclude_agent=Agent A)
activate Mem0Backend
Mem0Backend->>Mem0Lib: search(query, shared namespace)
activate Mem0Lib
Mem0Lib->>SharedNS: vector search across shared
Mem0Lib-->>Mem0Backend: [result1, result2, ...]
deactivate Mem0Lib
Mem0Backend->>Mem0Backend: filter(exclude_agent)<br/>apply_post_filters()
Mem0Backend-->>Agent B: tuple[MemoryEntry, ...]
deactivate Mem0Backend
end
Agent->>Mem0Backend: retract(agent_id, memory_id)
activate Mem0Backend
Mem0Backend->>Mem0Lib: get(memory_id)
activate Mem0Lib
Mem0Lib-->>Mem0Backend: result (with publisher)
deactivate Mem0Lib
Mem0Backend->>Mem0Backend: extract_publisher() / verify ownership
Mem0Backend->>Mem0Lib: delete(memory_id)
Mem0Lib->>SharedNS: remove from shared
Mem0Backend-->>Agent: success: bool
deactivate Mem0Backend
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
Comment |
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant enhancement to the memory system by integrating Mem0 as a pluggable memory backend. This allows the system to leverage an embedded vector database (Qdrant) and SQLite for persistent agent memory, providing robust storage and retrieval capabilities. The changes ensure that memory operations are non-blocking, support both individual agent memories and a shared knowledge store, and are thoroughly tested for reliability and correctness. This implementation moves the project closer to a fully modular and extensible memory architecture. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Greptile SummaryThis PR introduces a Key findings from this review pass:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant Mem0MemoryBackend
participant asyncio_thread as asyncio.to_thread
participant Mem0SDK as Mem0 SDK (Memory)
participant Qdrant as Embedded Qdrant
participant SQLite
Note over Caller,SQLite: Lifecycle
Caller->>Mem0MemoryBackend: connect()
Mem0MemoryBackend->>asyncio_thread: Memory.from_config(config_dict)
asyncio_thread->>Qdrant: init collection
asyncio_thread->>SQLite: init history DB
asyncio_thread-->>Mem0MemoryBackend: Memory client
Mem0MemoryBackend-->>Caller: connected
Note over Caller,SQLite: Private CRUD (per-agent isolation via user_id)
Caller->>Mem0MemoryBackend: store(agent_id, request)
Mem0MemoryBackend->>asyncio_thread: client.add(user_id=agent_id, infer=False)
asyncio_thread->>Qdrant: upsert vector
asyncio_thread->>SQLite: log history
asyncio_thread-->>Mem0MemoryBackend: {results: [{id, ...}]}
Mem0MemoryBackend-->>Caller: memory_id
Caller->>Mem0MemoryBackend: retrieve(agent_id, query)
alt query.text set
Mem0MemoryBackend->>asyncio_thread: client.search(query, user_id)
asyncio_thread->>Qdrant: ANN vector search
else no text
Mem0MemoryBackend->>asyncio_thread: client.get_all(user_id)
asyncio_thread->>Qdrant: fetch all for user
end
asyncio_thread-->>Mem0MemoryBackend: raw results
Mem0MemoryBackend->>Mem0MemoryBackend: apply_post_filters (expiry, category, tags, time, relevance)
Mem0MemoryBackend-->>Caller: tuple[MemoryEntry]
Note over Caller,SQLite: Shared Knowledge Store (reserved __synthorg_shared__ namespace)
Caller->>Mem0MemoryBackend: publish(agent_id, request)
Mem0MemoryBackend->>asyncio_thread: client.add(user_id=_SHARED_NAMESPACE, metadata+publisher)
asyncio_thread->>Qdrant: upsert to shared namespace
asyncio_thread-->>Mem0MemoryBackend: {results: [{id}]}
Mem0MemoryBackend-->>Caller: shared memory_id
Caller->>Mem0MemoryBackend: retract(agent_id, memory_id)
Mem0MemoryBackend->>asyncio_thread: client.get(memory_id)
asyncio_thread-->>Mem0MemoryBackend: raw entry
Mem0MemoryBackend->>Mem0MemoryBackend: verify publisher == agent_id
Mem0MemoryBackend->>asyncio_thread: client.delete(memory_id)
asyncio_thread-->>Mem0MemoryBackend: ok
Mem0MemoryBackend-->>Caller: True
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/ai_company/memory/backends/mem0/adapter.py
Line: 644-697
Comment:
**`count()` overcounts when entries have `expires_at` set**
`count()` fetches raw entries from Mem0 and tallies them directly, but never calls `apply_post_filters`. This means entries whose `expires_at` has already passed are still included in the total.
`retrieve()` and `search_shared()` both route through `apply_post_filters`, which discards expired entries at line 370 of `mappers.py`:
```python
if entry.expires_at is not None and entry.expires_at <= now:
continue
```
So after entries expire, `count()` returns a stale inflated total while `retrieve()` on the same agent returns fewer entries — a direct semantic inconsistency. A caller who guards `store()` behind a `count() < limit` check would allow more writes than intended once entries start expiring.
The fix is to deserialise the raw list into `MemoryEntry` objects and pass them through `apply_post_filters` before counting, the same way `retrieve` does, or at minimum inline the expiry check:
```python
now = datetime.now(UTC)
...
if category is None:
total = sum(
1 for item in raw_list
if not (
parse_mem0_datetime(item.get("metadata", {}).get("_synthorg_expires_at"))
is not None
and parse_mem0_datetime(item.get("metadata", {}).get("_synthorg_expires_at")) <= now
)
)
```
A cleaner approach: deserialise with `mem0_result_to_entry` and pass through `apply_post_filters(entries, MemoryQuery(limit=..., categories={category} if category else set()))` so all post-filters stay in one place.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/ai_company/memory/backends/mem0/adapter.py
Line: 812-814
Comment:
**`exclude_agent` comparison uses fabricated `agent_id` for publisher-less entries**
When a shared entry has no publisher metadata, `extract_publisher(item)` returns `None`, so `agent_id` is set to `_SHARED_NAMESPACE` in `mem0_result_to_entry`:
```python
mem0_result_to_entry(
item,
extract_publisher(item) or _SHARED_NAMESPACE, # ← fabricated fallback
)
```
Then the `exclude_agent` filter compares `e.agent_id != exclude_agent`. If two different agents each published entries whose publisher metadata was stripped (e.g. by a Mem0 SDK update that drops unrecognised metadata keys), both entries land with `agent_id == _SHARED_NAMESPACE`. Passing `exclude_agent=_SHARED_NAMESPACE` would then silently suppress all such entries, while passing any real `agent_id` would include them regardless of who actually wrote them.
Since `retract` already requires the publisher key to be present (it raises when `publisher is None`), publisher-less shared entries represent corrupted state. It would be safer to log a warning and skip them in `search_shared` rather than assigning an ownership guess to `_SHARED_NAMESPACE`.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: ac7d47e |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a well-implemented Mem0 memory backend adapter, complete with thorough configuration, mappers, and an extensive test suite. The code is well-structured and follows good practices for creating a pluggable backend, including defensive programming and detailed logging. My main concern is a recurring Python 2-style except syntax that is invalid in Python 3 and will cause syntax errors. After addressing this critical issue, the pull request will be in excellent condition.
| try: | ||
| config_dict = build_mem0_config_dict(self._mem0_config) | ||
| client = await asyncio.to_thread(Memory.from_config, config_dict) | ||
| except MemoryError, RecursionError: |
There was a problem hiding this comment.
The except A, B: syntax for catching multiple exceptions is from Python 2 and results in a SyntaxError in Python 3. The project's pyproject.toml specifies requires-python = ">=3.14".
You should use parentheses to create a tuple of exception types: except (MemoryError, RecursionError):.
This issue is present in multiple places in this file (e.g., lines 173, 283, 345, 403, 465, 537, 604, 679, 766) and should be corrected throughout.
except (MemoryError, RecursionError):| return None | ||
| try: | ||
| dt = datetime.fromisoformat(raw) | ||
| except ValueError, TypeError: |
There was a problem hiding this comment.
The except A, B: syntax for catching multiple exceptions is from Python 2 and is a SyntaxError in Python 3. Please use except (ValueError, TypeError):. This issue also occurs on line 139.
except (ValueError, TypeError):There was a problem hiding this comment.
Pull request overview
Implements the Mem0-based MemoryBackend so SynthOrg can persist and retrieve agent memories via Mem0 (embedded Qdrant + SQLite), and wires it into the memory backend factory with supporting config/models/tests/docs.
Changes:
- Add Mem0 backend implementation (adapter + mappers + config) and export it from
ai_company.memory. - Update
create_memory_backend()to instantiate Mem0 when selected (with required embedder config). - Add comprehensive unit/integration tests and update docs/workflows/dependencies to reflect the new backend.
Reviewed changes
Copilot reviewed 20 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Locks new transitive dependencies pulled in by Mem0/Qdrant stack. |
pyproject.toml |
Adds mem0ai dependency; mypy override for mem0.*. |
src/ai_company/observability/events/memory.py |
Adds new event constant for invalid backend config. |
src/ai_company/memory/factory.py |
Implements Mem0 backend construction + embedder validation/logging. |
src/ai_company/memory/backends/mem0/adapter.py |
New Mem0 MemoryBackend + shared knowledge store implementation. |
src/ai_company/memory/backends/mem0/mappers.py |
New Mem0↔domain mapping/helpers used by the adapter. |
src/ai_company/memory/backends/mem0/config.py |
New Mem0 config models + builder for Mem0 SDK config dict. |
src/ai_company/memory/backends/mem0/__init__.py |
Exposes Mem0 backend/config types. |
src/ai_company/memory/backends/__init__.py |
Adds concrete backends package export(s). |
src/ai_company/memory/__init__.py |
Re-exports Mem0 backend/config types from ai_company.memory. |
tests/unit/memory/test_init.py |
Extends public export checks for Mem0 symbols. |
tests/unit/memory/test_factory.py |
Updates factory tests to assert Mem0 backend creation + validation errors. |
tests/unit/memory/backends/mem0/test_mappers.py |
New mapper unit tests. |
tests/unit/memory/backends/mem0/test_config.py |
New config model unit tests. |
tests/unit/memory/backends/mem0/test_adapter.py |
New adapter unit tests. |
tests/integration/memory/test_mem0_backend.py |
New integration-style pipeline test using mocked Mem0 client. |
README.md |
Updates status text to reflect Mem0 adapter being implemented. |
docs/roadmap/index.md |
Removes Mem0 adapter from “remaining” roadmap items. |
docs/design/memory.md |
Updates design doc to mark Mem0 initial backend as implemented. |
CLAUDE.md |
Updates repository overview + dependency notes for Mem0 backend. |
.github/workflows/dependency-review.yml |
Adjusts license allowlists for new dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "litellm==1.82.1", | ||
| "litestar[standard,structlog,pydantic,brotli,prometheus]==2.21.1", | ||
| "mcp==1.26.0", | ||
| "mem0ai==1.0.5", | ||
| "pydantic==2.12.5", | ||
| "pyjwt[crypto]==2.11.0", |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/dependency-review.yml:
- Around line 47-51: The allow-dependencies-licenses list currently contains
unversioned package PURLs which bypass license checks for all future releases;
update each entry under allow-dependencies-licenses to the exact, version-pinned
PURLs from the lockfile: replace pkg:pypi/mem0ai with pkg:pypi/mem0ai@1.0.5,
pkg:pypi/numpy with pkg:pypi/numpy@2.4.3, pkg:pypi/qdrant-client with
pkg:pypi/qdrant-client@1.17.0, and pkg:pypi/posthog with pkg:pypi/posthog@7.9.12
so the exceptions apply only to the locked versions.
In `@CLAUDE.md`:
- Line 208: There is a mismatch between CLAUDE.md (which lists mem0ai as
"Optional") and pyproject.toml (which currently lists mem0ai as a required
dependency); either update the docs or change the pyproject to match: to keep
mem0ai optional, move it out of the main dependencies in pyproject.toml and add
it under an optional/extras section (e.g., [project.optional-dependencies] or
extras like "mem0") so it is only installed when backend: "mem0" is used, or
conversely change CLAUDE.md to mark mem0ai as "Required" if you intend it to
stay in the main dependencies; reference the mem0ai entry, CLAUDE.md line
describing backend: "mem0", and the dependency list in pyproject.toml when
making the edit.
In `@src/ai_company/memory/backends/mem0/adapter.py`:
- Around line 326-336: The Mem0 client call may return None, causing
raw_result.get(...) to raise an AttributeError; after calling
self._client.search(...) or self._client.get_all(...) (the raw_result
assignment), defensively check if raw_result is None and either replace it with
an empty dict or raise a clear error mentioning the agent_id and which call
failed (search vs get_all); update the block that builds kwargs via
query_to_mem0_search_args / query_to_mem0_getall_args and then calls
mem0_result_to_entry and apply_post_filters to use the validated raw_result (or
short-circuit) so subsequent raw_result.get("results", []) is safe and errors
are informative.
- Around line 140-148: The disconnect method currently nulls out self._client
without closing the underlying httpx.Client; change Memory.disconnect to, before
setting self._client = None, check if self._client is truthy and then close the
sync client via await asyncio.to_thread(self._client.close) (or the appropriate
.client.close if the wrapper stores it as .client), then set self._client = None
and self._connected = False; keep the existing logger calls and ensure the close
happens inside asyncio.to_thread to avoid blocking the event loop.
In `@src/ai_company/memory/backends/mem0/config.py`:
- Around line 81-129: The builder currently drops CompanyMemoryConfig.storage
overrides and silently uses embedded Qdrant+SQLite; update
build_config_from_company_config to validate
CompanyMemoryConfig.storage.vector_store and .history_store and raise a clear
exception (e.g., ValueError) for unsupported values such as "qdrant-external" or
"postgresql" so callers fail fast; locate the validation in
build_config_from_company_config (before constructing Mem0BackendConfig) and
ensure build_mem0_config_dict remains consistent with accepted values by only
emitting embedded Qdrant/SQLite when storage fields are missing or explicitly
set to the embedded options.
- Around line 8-15: The module currently raises in _reject_traversal without any
structured logging; add "from ai_company.observability import get_logger" and
initialize "logger = get_logger(__name__)" at module scope, then update
_reject_traversal to log the invalid path and context at WARNING or ERROR
(include the offending path, type information, and any relevant config field)
immediately before raising; also apply the same logging-to-raised-error pattern
for the other failure path around lines 69-78 so every error path in this config
module logs structured context before raising.
In `@src/ai_company/memory/backends/mem0/mappers.py`:
- Around line 148-153: The tags extraction may pass non-string values into
NotBlankStr; change the filtering/mapping so raw_tags elements are explicitly
converted to strings before constructing NotBlankStr: when building tags from
raw_tags (the raw_metadata lookup using f"{_PREFIX}tags"), coerce each retained
element with str(...) and then apply NotBlankStr only to the string (e.g., use a
generator that converts t to str(t) and trims/filters blank strings) so
NotBlankStr always receives a string.
- Around line 136-146: Parsed confidence from raw_metadata (raw_confidence ->
confidence) may be outside [0.0, 1.0], which causes Pydantic ValidationError
when constructing MemoryMetadata; after parsing (or on defaulting to 1.0) clamp
confidence into the valid range with something like confidence = max(0.0,
min(1.0, confidence)), and if clamping changed the value emit a logger.warning
using MEMORY_MODEL_INVALID with field="confidence", raw_value=raw_confidence and
reason indicating it was clamped to the valid range before passing into
MemoryMetadata.
In `@src/ai_company/memory/factory.py`:
- Around line 48-93: The Mem0 branch in create_memory_backend currently lets
backend-specific errors from build_config_from_company_config and
Mem0MemoryBackend escape; extract the Mem0 logic into a small helper (e.g.,
_create_mem0_backend) that is called from create_memory_backend after the
existing embedder checks (Mem0EmbedderConfig validation), and wrap the calls to
build_config_from_company_config and Mem0MemoryBackend in a try/except that
catches Exception, logs MEMORY_BACKEND_CONFIG_INVALID with backend="mem0", a
suitable reason like "backend_config_error", and the error details, then raise
MemoryConfigError with a clear message; ensure the helper returns the created
backend and that create_memory_backend delegates to it to keep function size
under the limit.
In `@tests/unit/memory/backends/mem0/test_adapter.py`:
- Around line 1-1117: The test module is too large and mixes many concerns;
split it into smaller files grouping related test classes (e.g., move
TestLifecycle, TestConnectionGuard to
tests/unit/memory/backends/mem0/test_lifecycle.py; TestStore and TestPublish to
test_store.py; TestRetrieve and TestSearchShared to test_retrieve_shared.py;
TestGet/TestDelete/TestCount/TestRetract to their own files; and
TestAdditionalEdgeCases to test_edgecases.py) and replace repeated similar
assertions with pytest.mark.parametrize (collapse matrix-like tests in
TestConnectionGuard, TestStore empty/missing/id cases, TestRetrieve variants,
TestCount variants, TestPublish variants, TestSearchShared exclude/namespace
cases, and TestRetract ownership/no-publisher/delete-failure cases) to reduce
duplication while preserving unique test names like
TestLifecycle.test_connect_success,
Mem0MemoryBackend.store/retrieve/get/delete/count/publish/search_shared/retract,
and helper fixtures (_mem0_add_result, _mem0_search_result, _mem0_get_result,
_make_store_request) so each new file stays under ~800 lines and functions
remain <50 lines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3b6224e3-7a24-4e20-85bc-c80da6a70b08
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
.github/workflows/dependency-review.ymlCLAUDE.mdREADME.mddocs/design/memory.mddocs/roadmap/index.mdpyproject.tomlsrc/ai_company/memory/__init__.pysrc/ai_company/memory/backends/__init__.pysrc/ai_company/memory/backends/mem0/__init__.pysrc/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/backends/mem0/config.pysrc/ai_company/memory/backends/mem0/mappers.pysrc/ai_company/memory/factory.pysrc/ai_company/observability/events/memory.pytests/integration/memory/test_mem0_backend.pytests/unit/memory/backends/__init__.pytests/unit/memory/backends/mem0/__init__.pytests/unit/memory/backends/mem0/test_adapter.pytests/unit/memory/backends/mem0/test_config.pytests/unit/memory/backends/mem0/test_mappers.pytests/unit/memory/test_factory.pytests/unit/memory/test_init.py
💤 Files with no reviewable changes (1)
- docs/roadmap/index.md
📜 Review details
⏰ 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). (5)
- GitHub Check: Agent
- GitHub Check: Greptile Review
- GitHub Check: Test (Python 3.14)
- GitHub Check: Test (Python 3.14)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (4)
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: Pinned: all versions use==inpyproject.toml
Groups in pyproject.toml:test(pytest + plugins),dev(includes test + ruff, mypy, pre-commit, commitizen). Install:uv syncinstalls everything (dev group is default)
Files:
pyproject.toml
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Nofrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:syntax (no parentheses) — ruff enforces PEP 758 except syntax on Python 3.14
Type hints: all public functions, mypy strict mode
Docstrings: Google style, required on public classes/functions (enforced by ruff D rules)
Create new objects, never mutate existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement.
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention andcopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Config vs runtime state: use frozen Pydantic models for config/identity; separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Models: Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict). Use@computed_fieldfor derived values instead of storing + validating redundant fields. UseNotBlankStrfor all identifier/name fields — including optional and tuple variants — instead of manual whitespace validators.
Async concurrency: preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task. Existing code is being migrated incrementally.
Line length: 88 characters (ruff)
Functions: < 50 lines, files < 800 lines
Errors: handle explicitly, never silently swallow
Validate: at system boundaries (user input, external APIs, config files)
Files:
tests/unit/memory/test_init.pytests/unit/memory/backends/mem0/test_config.pysrc/ai_company/memory/backends/mem0/config.pysrc/ai_company/observability/events/memory.pysrc/ai_company/memory/__init__.pytests/unit/memory/test_factory.pytests/unit/memory/backends/mem0/test_mappers.pytests/unit/memory/backends/mem0/test_adapter.pysrc/ai_company/memory/backends/mem0/__init__.pytests/integration/memory/test_mem0_backend.pysrc/ai_company/memory/factory.pysrc/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/backends/__init__.pysrc/ai_company/memory/backends/mem0/mappers.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Tests: markers@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slow
Coverage: 80% minimum (enforced in CI)
Async:asyncio_mode = "auto"— no manual@pytest.mark.asyncioneeded
Timeout: 30 seconds per test
Parallelism:pytest-xdistvia-n auto— ALWAYS include-n autowhen running pytest, never run tests sequentially
Prefer@pytest.mark.parametrizefor testing similar cases
Vendor-agnostic everywhere: Tests must usetest-provider,test-small-001, etc. (Vendor names may only appear in operations design page, .claude/ skill/agent files, or third-party import paths)
Files:
tests/unit/memory/test_init.pytests/unit/memory/backends/mem0/test_config.pytests/unit/memory/test_factory.pytests/unit/memory/backends/mem0/test_mappers.pytests/unit/memory/backends/mem0/test_adapter.pytests/integration/memory/test_mem0_backend.py
src/ai_company/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/ai_company/**/*.py: Every module with business logic MUST have:from ai_company.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code
Variable name: alwayslogger(not_logger, notlog)
Event names: always use constants from the domain-specific module underai_company.observability.events(e.g.PROVIDER_CALL_STARTfromevents.provider). Import directly:from ai_company.observability.events.<domain> import EVENT_CONSTANT
Structured kwargs in logging: alwayslogger.info(EVENT, key=value)— neverlogger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO
DEBUG for object creation, internal flow, entry/exit of key functions
Vendor-agnostic everywhere: NEVER use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:example-provider,example-large-001,example-medium-001,example-small-001,large/medium/smallas aliases. Tests must usetest-provider,test-small-001, etc.
Files:
src/ai_company/memory/backends/mem0/config.pysrc/ai_company/observability/events/memory.pysrc/ai_company/memory/__init__.pysrc/ai_company/memory/backends/mem0/__init__.pysrc/ai_company/memory/factory.pysrc/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/backends/__init__.pysrc/ai_company/memory/backends/mem0/mappers.py
🧠 Learnings (4)
📚 Learning: 2026-03-13T06:12:19.113Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:12:19.113Z
Learning: Applies to pyproject.toml : Groups in pyproject.toml: `test` (pytest + plugins), `dev` (includes test + ruff, mypy, pre-commit, commitizen). Install: `uv sync` installs everything (dev group is default)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-13T06:12:19.113Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:12:19.113Z
Learning: Applies to pyproject.toml : Pinned: all versions use `==` in `pyproject.toml`
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-13T06:12:19.113Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:12:19.113Z
Learning: Applies to **/*.yml : Config: YAML company config loading and validation — use frozen Pydantic models for config validation
Applied to files:
src/ai_company/memory/backends/mem0/config.py
📚 Learning: 2026-03-13T06:12:19.113Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:12:19.113Z
Learning: Applies to src/ai_company/**/*.py : Event names: always use constants from the domain-specific module under `ai_company.observability.events` (e.g. `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`
Applied to files:
src/ai_company/observability/events/memory.py
🧬 Code graph analysis (8)
tests/unit/memory/backends/mem0/test_config.py (2)
src/ai_company/memory/backends/mem0/config.py (4)
Mem0BackendConfig(46-78)Mem0EmbedderConfig(17-43)build_config_from_company_config(111-129)build_mem0_config_dict(81-108)src/ai_company/memory/config.py (2)
CompanyMemoryConfig(139-195)MemoryStorageConfig(25-104)
src/ai_company/memory/backends/mem0/config.py (1)
src/ai_company/memory/config.py (1)
CompanyMemoryConfig(139-195)
src/ai_company/memory/__init__.py (2)
src/ai_company/memory/backends/mem0/config.py (1)
Mem0EmbedderConfig(17-43)src/ai_company/memory/backends/mem0/adapter.py (1)
Mem0MemoryBackend(77-785)
tests/unit/memory/backends/mem0/test_adapter.py (5)
src/ai_company/core/enums.py (1)
MemoryCategory(101-108)src/ai_company/memory/backends/mem0/adapter.py (20)
Mem0MemoryBackend(77-785)max_memories_per_agent(227-229)backend_name(195-197)is_connected(190-192)supported_categories(202-204)supports_graph(207-209)supports_temporal(212-214)supports_vector_search(217-219)supports_shared_access(222-224)connect(101-138)disconnect(140-148)health_check(150-187)store(245-301)retrieve(303-362)get(364-422)delete(424-484)count(486-555)publish(559-621)search_shared(623-697)retract(699-785)src/ai_company/memory/backends/mem0/config.py (2)
Mem0BackendConfig(46-78)Mem0EmbedderConfig(17-43)src/ai_company/memory/errors.py (4)
MemoryConnectionError(17-18)MemoryRetrievalError(25-26)MemoryStoreError(21-22)MemoryError(13-14)src/ai_company/memory/models.py (2)
MemoryQuery(153-230)MemoryStoreRequest(55-79)
src/ai_company/memory/backends/mem0/__init__.py (2)
src/ai_company/memory/backends/mem0/adapter.py (1)
Mem0MemoryBackend(77-785)src/ai_company/memory/backends/mem0/config.py (2)
Mem0BackendConfig(46-78)Mem0EmbedderConfig(17-43)
src/ai_company/memory/factory.py (5)
src/ai_company/memory/config.py (1)
CompanyMemoryConfig(139-195)src/ai_company/memory/backends/mem0/config.py (2)
Mem0EmbedderConfig(17-43)build_config_from_company_config(111-129)src/ai_company/memory/errors.py (1)
MemoryConfigError(40-41)src/ai_company/memory/protocol.py (1)
MemoryBackend(20-182)src/ai_company/memory/backends/mem0/adapter.py (2)
Mem0MemoryBackend(77-785)max_memories_per_agent(227-229)
src/ai_company/memory/backends/__init__.py (1)
src/ai_company/memory/backends/mem0/adapter.py (1)
Mem0MemoryBackend(77-785)
src/ai_company/memory/backends/mem0/mappers.py (5)
src/ai_company/core/enums.py (1)
MemoryCategory(101-108)src/ai_company/memory/errors.py (2)
MemoryRetrievalError(25-26)MemoryStoreError(21-22)src/ai_company/memory/models.py (4)
MemoryEntry(82-150)MemoryMetadata(20-52)MemoryQuery(153-230)MemoryStoreRequest(55-79)src/ai_company/observability/_logger.py (1)
get_logger(8-28)src/ai_company/memory/backends/mem0/adapter.py (1)
get(364-422)
🔇 Additional comments (24)
.github/workflows/dependency-review.yml (1)
32-35: Helpful license rationale comments for auditability.The added context makes compliance exceptions easier to review and maintain.
pyproject.toml (1)
24-24: Dependency and mypy updates look correct.Pinned dependency and scoped mypy override are consistent with the Mem0 backend integration.
Also applies to: 188-190
CLAUDE.md (1)
97-97: Memory package structure update is clear and accurate.The added backend path details match the implemented architecture.
src/ai_company/memory/backends/mem0/__init__.py (1)
1-6: Public export surface is clean and consistent.
__all__and imports are aligned with the intended Mem0 backend API.src/ai_company/memory/backends/__init__.py (1)
1-5: Backend initializer is correct.The module cleanly exposes the concrete backend as intended.
README.md (1)
130-130: Status update accurately reflects delivered scope.Including memory with the Mem0 adapter is consistent with this PR.
docs/design/memory.md (1)
33-33: Design diagram status is now aligned with implementation.Good documentation sync for backend maturity.
src/ai_company/memory/__init__.py (1)
6-15: Top-level memory exports are consistent and well-integrated.The added imports and
__all__entries correctly expose the Mem0 backend surface.Also applies to: 82-84
src/ai_company/observability/events/memory.py (1)
22-22: New observability event is well-scoped and correctly named.This constant fills an important gap for invalid backend config paths.
src/ai_company/memory/backends/mem0/mappers.py (7)
1-36: LGTM!Module setup follows coding guidelines correctly: proper logger setup with
get_logger(__name__), event constants imported from the domain-specific observability module, and nofrom __future__ import annotations.
38-57: LGTM!Clean, stateless mapping function that correctly serializes domain metadata to Mem0-compatible format. Creates a new dict and properly handles optional fields.
60-86: LGTM!Robust datetime parsing with proper defensive handling for malformed input, correct PEP 758 exception syntax, and appropriate assumption that naive datetimes are UTC.
167-232: LGTM!Comprehensive conversion logic with proper validation of required fields, appropriate error handling via
MemoryRetrievalError, and sensible fallback for missingcreated_at. Good use of helper functions for parsing subcomponents.
235-283: LGTM!Simple and focused query conversion functions with appropriate validation for the search case where
query.textis required.
286-318: LGTM!Well-implemented post-retrieval filtering with correct filter semantics: AND for tags, exclusive upper bound for
until, and appropriate handling ofNonerelevance scores. Creates new tuple without mutating inputs.
324-386: LGTM!Solid helper functions moved from adapter to keep it under 800 lines.
validate_add_resultprovides clear error context, andextract_category/extract_publisherhandle missing or malformed metadata gracefully.src/ai_company/memory/backends/mem0/adapter.py (8)
1-98: LGTM!Clean module setup with correct logger configuration, appropriate imports, and well-structured class initialization. Using
Anyfor_clientis reasonable given the runtime import ofmem0.Memory.
189-241: LGTM!Capability properties are correctly implemented with accurate documentation. The
_require_connected()guard provides consistent connection enforcement across all operations.
245-301: LGTM!Correct implementation with proper
asyncio.to_threadusage, explicitinfer=Falseto disable Mem0's auto-extraction, and comprehensive error handling with structured logging.
364-484: LGTM!Both methods are well-implemented. The
get()method correctly handlesNoneresults. The check-then-delete pattern indelete()is appropriately documented as a workaround for Mem0's API limitation, with acceptable TOCTOU semantics for this use case.
486-555: LGTM!Well-documented
count()implementation with clear documentation of the O(n) complexity and capping behavior. The limitation that results may underestimate for agents exceedingmax_memories_per_agentis appropriately noted.
559-621: LGTM!Correct shared memory publication with reserved namespace isolation and publisher ownership tracking via metadata. The pattern mirrors
store()with appropriate adjustments for the shared context.
623-697: LGTM!Well-structured shared knowledge search with appropriate namespace isolation, publisher extraction for agent attribution, and post-filtering support. The
exclude_agentfilter correctly excludes the requesting agent's own published memories.
699-785: LGTM!Robust
retract()implementation with proper ownership verification. The authorization checks raiseMemoryStoreErrorwith clear messages, and the re-raise pattern at line 761-765 correctly avoids duplicate logging since context was already captured in the warning.
| async def disconnect(self) -> None: | ||
| """Close the Mem0 connection. | ||
|
|
||
| Safe to call even if not connected. | ||
| """ | ||
| logger.info(MEMORY_BACKEND_DISCONNECTING, backend="mem0") | ||
| self._client = None | ||
| self._connected = False | ||
| logger.info(MEMORY_BACKEND_DISCONNECTED, backend="mem0") |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does mem0 Memory client have a close or shutdown method for cleanup?
💡 Result:
No. In the official mem0ai Python SDK, MemoryClient / AsyncMemoryClient do not expose a close(), shutdown(), or aclose() method; they just hold an underlying httpx.Client / httpx.AsyncClient instance (client / async_client). [1] The Mem0 docs for AsyncMemoryClient also list its methods (add/search/get_all/delete/…) and do not mention any close/shutdown API. [2]
Cleanup options:
- Sync:
client.client.close() - Async:
await client.async_client.aclose() - Or pass your own
httpx.Client/httpx.AsyncClientinto the constructor and manage its lifetime (e.g., with a context manager). [1]
Sources
[1] https://github.com/mem0ai/mem0/blob/34c797d2/mem0/client/main.py
[2] https://docs.mem0.ai/platform/features/async-client
🏁 Script executed:
# Check the imports and client instantiation in the adapter file
head -50 src/ai_company/memory/backends/mem0/adapter.py | cat -nRepository: Aureliolo/synthorg
Length of output: 1766
🏁 Script executed:
# Search for where _client is instantiated (in connect method)
sed -n '101,139p' src/ai_company/memory/backends/mem0/adapter.py | cat -nRepository: Aureliolo/synthorg
Length of output: 1763
Close the underlying httpx.Client in disconnect().
The Memory client holds an internal httpx.Client for API communication. Setting _client = None without closing it leaves the HTTP connection pool open until garbage collection. Wrap the cleanup in asyncio.to_thread() since the underlying client is synchronous:
Suggested fix
async def disconnect(self) -> None:
"""Close the Mem0 connection.
Safe to call even if not connected.
"""
logger.info(MEMORY_BACKEND_DISCONNECTING, backend="mem0")
if self._client:
await asyncio.to_thread(self._client.client.close)
self._client = None
self._connected = False
logger.info(MEMORY_BACKEND_DISCONNECTED, backend="mem0")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/memory/backends/mem0/adapter.py` around lines 140 - 148, The
disconnect method currently nulls out self._client without closing the
underlying httpx.Client; change Memory.disconnect to, before setting
self._client = None, check if self._client is truthy and then close the sync
client via await asyncio.to_thread(self._client.close) (or the appropriate
.client.close if the wrapper stores it as .client), then set self._client = None
and self._connected = False; keep the existing logger calls and ensure the close
happens inside asyncio.to_thread to avoid blocking the event loop.
| def build_mem0_config_dict(config: Mem0BackendConfig) -> dict[str, Any]: | ||
| """Build the dict that ``Memory.from_config()`` expects. | ||
|
|
||
| Args: | ||
| config: Mem0 backend configuration. | ||
|
|
||
| Returns: | ||
| Configuration dict suitable for ``Memory.from_config()``. | ||
| """ | ||
| return { | ||
| "vector_store": { | ||
| "provider": "qdrant", | ||
| "config": { | ||
| "collection_name": config.collection_name, | ||
| "embedding_model_dims": config.embedder.dims, | ||
| "path": f"{config.data_dir}/qdrant", | ||
| }, | ||
| }, | ||
| "embedder": { | ||
| "provider": config.embedder.provider, | ||
| "config": { | ||
| "model": config.embedder.model, | ||
| }, | ||
| }, | ||
| "history_db_path": f"{config.data_dir}/history.db", | ||
| # Mem0 config schema version — required by Memory.from_config(). | ||
| "version": "v1.1", | ||
| } | ||
|
|
||
|
|
||
| def build_config_from_company_config( | ||
| config: CompanyMemoryConfig, | ||
| *, | ||
| embedder: Mem0EmbedderConfig, | ||
| ) -> Mem0BackendConfig: | ||
| """Derive a ``Mem0BackendConfig`` from the top-level memory config. | ||
|
|
||
| Args: | ||
| config: Company-wide memory configuration. | ||
| embedder: Embedder settings (provider and model must be | ||
| supplied explicitly to avoid vendor names in defaults). | ||
|
|
||
| Returns: | ||
| Mem0-specific backend configuration. | ||
| """ | ||
| return Mem0BackendConfig( | ||
| data_dir=config.storage.data_dir, | ||
| embedder=embedder, | ||
| ) |
There was a problem hiding this comment.
Reject unsupported storage overrides instead of silently ignoring them.
CompanyMemoryConfig.storage still accepts vector_store="qdrant-external" and history_store="postgresql", but this builder drops both fields and build_mem0_config_dict() always emits embedded Qdrant + SQLite. A caller can therefore request external storage and silently get local files instead. Please fail fast on unsupported values here until the adapter actually wires those settings through. As per coding guidelines "Validate: at system boundaries (user input, external APIs, config files)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/memory/backends/mem0/config.py` around lines 81 - 129, The
builder currently drops CompanyMemoryConfig.storage overrides and silently uses
embedded Qdrant+SQLite; update build_config_from_company_config to validate
CompanyMemoryConfig.storage.vector_store and .history_store and raise a clear
exception (e.g., ValueError) for unsupported values such as "qdrant-external" or
"postgresql" so callers fail fast; locate the validation in
build_config_from_company_config (before constructing Mem0BackendConfig) and
ensure build_mem0_config_dict remains consistent with accepted values by only
emitting embedded Qdrant/SQLite when storage fields are missing or explicitly
set to the embedded options.
| if config.backend == "mem0": | ||
| msg = "mem0 backend not yet implemented" | ||
| logger.warning( | ||
| MEMORY_BACKEND_NOT_IMPLEMENTED, | ||
| from ai_company.memory.backends.mem0 import Mem0MemoryBackend # noqa: PLC0415 | ||
| from ai_company.memory.backends.mem0.config import ( # noqa: PLC0415 | ||
| Mem0EmbedderConfig, | ||
| build_config_from_company_config, | ||
| ) | ||
|
|
||
| if embedder is None: | ||
| msg = ( | ||
| "Mem0 backend requires an embedder configuration — " | ||
| "pass a Mem0EmbedderConfig instance" | ||
| ) | ||
| logger.warning( | ||
| MEMORY_BACKEND_CONFIG_INVALID, | ||
| backend="mem0", | ||
| reason="missing_embedder", | ||
| error=msg, | ||
| ) | ||
| raise MemoryConfigError(msg) | ||
| if not isinstance(embedder, Mem0EmbedderConfig): | ||
| msg = ( # type: ignore[unreachable] | ||
| f"embedder must be a Mem0EmbedderConfig, got {type(embedder).__name__}" | ||
| ) | ||
| logger.warning( | ||
| MEMORY_BACKEND_CONFIG_INVALID, | ||
| backend="mem0", | ||
| reason="invalid_embedder_type", | ||
| error=msg, | ||
| embedder_type=type(embedder).__name__, | ||
| ) | ||
| raise MemoryConfigError(msg) | ||
|
|
||
| mem0_config = build_config_from_company_config( | ||
| config, | ||
| embedder=embedder, | ||
| ) | ||
| backend = Mem0MemoryBackend( | ||
| mem0_config=mem0_config, | ||
| max_memories_per_agent=config.options.max_memories_per_agent, | ||
| ) | ||
| logger.info( | ||
| MEMORY_BACKEND_CREATED, | ||
| backend="mem0", | ||
| reason=msg, | ||
| data_dir=mem0_config.data_dir, | ||
| ) | ||
| raise MemoryConfigError(msg) | ||
| return backend |
There was a problem hiding this comment.
Don't leak raw Mem0 config failures out of the factory.
This branch only normalizes the missing/wrong-embedder cases. Any backend-specific validation failure from build_config_from_company_config() or Mem0MemoryBackend(...) will currently bypass MEMORY_BACKEND_CONFIG_INVALID and escape as a raw exception, even though this function is the config boundary. Extract the Mem0 path into a small helper and wrap those failures as MemoryConfigError there; that also brings create_memory_backend() back under the repo's function-size limit. As per coding guidelines "Functions: < 50 lines" and "Validate: at system boundaries (user input, external APIs, config files)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/memory/factory.py` around lines 48 - 93, The Mem0 branch in
create_memory_backend currently lets backend-specific errors from
build_config_from_company_config and Mem0MemoryBackend escape; extract the Mem0
logic into a small helper (e.g., _create_mem0_backend) that is called from
create_memory_backend after the existing embedder checks (Mem0EmbedderConfig
validation), and wrap the calls to build_config_from_company_config and
Mem0MemoryBackend in a try/except that catches Exception, logs
MEMORY_BACKEND_CONFIG_INVALID with backend="mem0", a suitable reason like
"backend_config_error", and the error details, then raise MemoryConfigError with
a clear message; ensure the helper returns the created backend and that
create_memory_backend delegates to it to keep function size under the limit.
| """Tests for the Mem0 memory backend adapter.""" | ||
|
|
||
| import sys | ||
| from typing import Any | ||
| from unittest.mock import MagicMock, patch | ||
|
|
||
| import pytest | ||
|
|
||
| from ai_company.core.enums import MemoryCategory | ||
| from ai_company.memory.backends.mem0.adapter import ( | ||
| _SHARED_NAMESPACE, | ||
| Mem0MemoryBackend, | ||
| ) | ||
| from ai_company.memory.backends.mem0.config import ( | ||
| Mem0BackendConfig, | ||
| Mem0EmbedderConfig, | ||
| ) | ||
| from ai_company.memory.backends.mem0.mappers import _PUBLISHER_KEY | ||
| from ai_company.memory.errors import ( | ||
| MemoryConnectionError, | ||
| MemoryRetrievalError, | ||
| MemoryStoreError, | ||
| ) | ||
| from ai_company.memory.models import ( | ||
| MemoryQuery, | ||
| MemoryStoreRequest, | ||
| ) | ||
|
|
||
| pytestmark = pytest.mark.timeout(30) | ||
|
|
||
|
|
||
| def _test_embedder() -> Mem0EmbedderConfig: | ||
| """Vendor-agnostic embedder config for tests.""" | ||
| return Mem0EmbedderConfig( | ||
| provider="test-provider", | ||
| model="test-embedding-001", | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def mem0_config() -> Mem0BackendConfig: | ||
| """Default Mem0 config for tests.""" | ||
| return Mem0BackendConfig( | ||
| data_dir="/tmp/test-memory", # noqa: S108 | ||
| embedder=_test_embedder(), | ||
| ) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def mock_client() -> MagicMock: | ||
| """Mock Mem0 Memory client.""" | ||
| return MagicMock() | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def backend( | ||
| mem0_config: Mem0BackendConfig, | ||
| mock_client: MagicMock, | ||
| ) -> Mem0MemoryBackend: | ||
| """Connected backend with mocked Mem0 client.""" | ||
| b = Mem0MemoryBackend(mem0_config=mem0_config, max_memories_per_agent=100) | ||
| b._client = mock_client | ||
| b._connected = True | ||
| return b | ||
|
|
||
|
|
||
| def _mem0_add_result(memory_id: str = "mem-001") -> dict[str, Any]: | ||
| """Build a typical Mem0 add() return value.""" | ||
| return { | ||
| "results": [ | ||
| { | ||
| "id": memory_id, | ||
| "memory": "test content", | ||
| "event": "ADD", | ||
| }, | ||
| ], | ||
| } | ||
|
|
||
|
|
||
| def _mem0_search_result( | ||
| items: list[dict[str, Any]] | None = None, | ||
| ) -> dict[str, Any]: | ||
| """Build a typical Mem0 search() return value.""" | ||
| if items is None: | ||
| items = [ | ||
| { | ||
| "id": "mem-001", | ||
| "memory": "found content", | ||
| "score": 0.85, | ||
| "created_at": "2026-03-12T10:00:00+00:00", | ||
| "metadata": { | ||
| "_synthorg_category": "episodic", | ||
| "_synthorg_confidence": 0.9, | ||
| }, | ||
| }, | ||
| ] | ||
| return {"results": items} | ||
|
|
||
|
|
||
| def _mem0_get_result(memory_id: str = "mem-001") -> dict[str, Any]: | ||
| """Build a typical Mem0 get() return value.""" | ||
| return { | ||
| "id": memory_id, | ||
| "memory": "stored content", | ||
| "created_at": "2026-03-12T10:00:00+00:00", | ||
| "updated_at": None, | ||
| "metadata": { | ||
| "_synthorg_category": "episodic", | ||
| "_synthorg_confidence": 1.0, | ||
| }, | ||
| } | ||
|
|
||
|
|
||
| def _make_store_request( | ||
| *, | ||
| category: MemoryCategory = MemoryCategory.EPISODIC, | ||
| content: str = "test content", | ||
| ) -> MemoryStoreRequest: | ||
| """Helper to build a store request.""" | ||
| return MemoryStoreRequest(category=category, content=content) | ||
|
|
||
|
|
||
| # ── Properties ──────────────────────────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestProperties: | ||
| def test_backend_name(self, backend: Mem0MemoryBackend) -> None: | ||
| assert backend.backend_name == "mem0" | ||
|
|
||
| def test_is_connected_true(self, backend: Mem0MemoryBackend) -> None: | ||
| assert backend.is_connected is True | ||
|
|
||
| def test_is_connected_false(self, mem0_config: Mem0BackendConfig) -> None: | ||
| b = Mem0MemoryBackend(mem0_config=mem0_config) | ||
| assert b.is_connected is False | ||
|
|
||
|
|
||
| # ── Capabilities ────────────────────────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestCapabilities: | ||
| def test_supported_categories(self, backend: Mem0MemoryBackend) -> None: | ||
| assert backend.supported_categories == frozenset(MemoryCategory) | ||
|
|
||
| def test_supports_graph_false(self, backend: Mem0MemoryBackend) -> None: | ||
| assert backend.supports_graph is False | ||
|
|
||
| def test_supports_temporal_true(self, backend: Mem0MemoryBackend) -> None: | ||
| assert backend.supports_temporal is True | ||
|
|
||
| def test_supports_vector_search_true( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| ) -> None: | ||
| assert backend.supports_vector_search is True | ||
|
|
||
| def test_supports_shared_access_true( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| ) -> None: | ||
| assert backend.supports_shared_access is True | ||
|
|
||
| def test_max_memories_per_agent( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| ) -> None: | ||
| assert backend.max_memories_per_agent == 100 | ||
|
|
||
|
|
||
| # ── Protocol Conformance ───────────────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestProtocolConformance: | ||
| """Verify Mem0MemoryBackend conforms to protocol interfaces.""" | ||
|
|
||
| def test_has_memory_backend_methods( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| ) -> None: | ||
| assert hasattr(backend, "connect") | ||
| assert hasattr(backend, "disconnect") | ||
| assert hasattr(backend, "health_check") | ||
| assert hasattr(backend, "store") | ||
| assert hasattr(backend, "retrieve") | ||
| assert hasattr(backend, "get") | ||
| assert hasattr(backend, "delete") | ||
| assert hasattr(backend, "count") | ||
|
|
||
| def test_has_capabilities_properties( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| ) -> None: | ||
| assert hasattr(backend, "supported_categories") | ||
| assert hasattr(backend, "supports_graph") | ||
| assert hasattr(backend, "supports_temporal") | ||
| assert hasattr(backend, "supports_vector_search") | ||
| assert hasattr(backend, "supports_shared_access") | ||
| assert hasattr(backend, "max_memories_per_agent") | ||
|
|
||
| def test_has_shared_knowledge_methods( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| ) -> None: | ||
| assert hasattr(backend, "publish") | ||
| assert hasattr(backend, "search_shared") | ||
| assert hasattr(backend, "retract") | ||
|
|
||
|
|
||
| # ── Lifecycle ───────────────────────────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestLifecycle: | ||
| async def test_connect_success( | ||
| self, | ||
| mem0_config: Mem0BackendConfig, | ||
| ) -> None: | ||
| b = Mem0MemoryBackend(mem0_config=mem0_config) | ||
| mock_memory = MagicMock() | ||
| with patch( | ||
| "ai_company.memory.backends.mem0.adapter.asyncio.to_thread", | ||
| return_value=mock_memory, | ||
| ): | ||
| await b.connect() | ||
|
|
||
| assert b.is_connected is True | ||
|
|
||
| async def test_connect_failure_raises( | ||
| self, | ||
| mem0_config: Mem0BackendConfig, | ||
| ) -> None: | ||
| b = Mem0MemoryBackend(mem0_config=mem0_config) | ||
| with ( | ||
| patch( | ||
| "ai_company.memory.backends.mem0.adapter.asyncio.to_thread", | ||
| side_effect=RuntimeError("connection failed"), | ||
| ), | ||
| pytest.raises(MemoryConnectionError, match="Failed to connect"), | ||
| ): | ||
| await b.connect() | ||
| assert b.is_connected is False | ||
|
|
||
| async def test_disconnect(self, backend: Mem0MemoryBackend) -> None: | ||
| await backend.disconnect() | ||
| assert backend.is_connected is False | ||
| assert backend._client is None | ||
|
|
||
| async def test_disconnect_when_not_connected( | ||
| self, | ||
| mem0_config: Mem0BackendConfig, | ||
| ) -> None: | ||
| b = Mem0MemoryBackend(mem0_config=mem0_config) | ||
| await b.disconnect() # Should not raise | ||
| assert b.is_connected is False | ||
|
|
||
| async def test_health_check_connected( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get_all.return_value = {"results": []} | ||
| assert await backend.health_check() is True | ||
|
|
||
| async def test_health_check_disconnected( | ||
| self, | ||
| mem0_config: Mem0BackendConfig, | ||
| ) -> None: | ||
| b = Mem0MemoryBackend(mem0_config=mem0_config) | ||
| assert await b.health_check() is False | ||
|
|
||
| async def test_health_check_probe_failure( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get_all.side_effect = RuntimeError("backend down") | ||
| assert await backend.health_check() is False | ||
|
|
||
| async def test_connect_import_error_raises( | ||
| self, | ||
| mem0_config: Mem0BackendConfig, | ||
| ) -> None: | ||
| """ImportError when mem0 package is not installed.""" | ||
| b = Mem0MemoryBackend(mem0_config=mem0_config) | ||
| with ( | ||
| patch.dict(sys.modules, {"mem0": None}), | ||
| pytest.raises(MemoryConnectionError, match="not installed"), | ||
| ): | ||
| await b.connect() | ||
|
|
||
| async def test_health_check_reraises_memory_error( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """MemoryError propagates through health_check.""" | ||
| mock_client.get_all.side_effect = MemoryError("out of memory") | ||
| with pytest.raises(MemoryError): | ||
| await backend.health_check() | ||
|
|
||
|
|
||
| # ── Connection guard ────────────────────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestConnectionGuard: | ||
| async def test_store_raises_when_disconnected( | ||
| self, | ||
| mem0_config: Mem0BackendConfig, | ||
| ) -> None: | ||
| b = Mem0MemoryBackend(mem0_config=mem0_config) | ||
| with pytest.raises(MemoryConnectionError, match="Not connected"): | ||
| await b.store("test-agent-001", _make_store_request()) | ||
|
|
||
| async def test_retrieve_raises_when_disconnected( | ||
| self, | ||
| mem0_config: Mem0BackendConfig, | ||
| ) -> None: | ||
| b = Mem0MemoryBackend(mem0_config=mem0_config) | ||
| with pytest.raises(MemoryConnectionError, match="Not connected"): | ||
| await b.retrieve("test-agent-001", MemoryQuery(text="test")) | ||
|
|
||
| async def test_get_raises_when_disconnected( | ||
| self, | ||
| mem0_config: Mem0BackendConfig, | ||
| ) -> None: | ||
| b = Mem0MemoryBackend(mem0_config=mem0_config) | ||
| with pytest.raises(MemoryConnectionError, match="Not connected"): | ||
| await b.get("test-agent-001", "mem-001") | ||
|
|
||
| async def test_delete_raises_when_disconnected( | ||
| self, | ||
| mem0_config: Mem0BackendConfig, | ||
| ) -> None: | ||
| b = Mem0MemoryBackend(mem0_config=mem0_config) | ||
| with pytest.raises(MemoryConnectionError, match="Not connected"): | ||
| await b.delete("test-agent-001", "mem-001") | ||
|
|
||
| async def test_count_raises_when_disconnected( | ||
| self, | ||
| mem0_config: Mem0BackendConfig, | ||
| ) -> None: | ||
| b = Mem0MemoryBackend(mem0_config=mem0_config) | ||
| with pytest.raises(MemoryConnectionError, match="Not connected"): | ||
| await b.count("test-agent-001") | ||
|
|
||
| async def test_publish_raises_when_disconnected( | ||
| self, | ||
| mem0_config: Mem0BackendConfig, | ||
| ) -> None: | ||
| b = Mem0MemoryBackend(mem0_config=mem0_config) | ||
| with pytest.raises(MemoryConnectionError, match="Not connected"): | ||
| await b.publish("test-agent-001", _make_store_request()) | ||
|
|
||
| async def test_search_shared_raises_when_disconnected( | ||
| self, | ||
| mem0_config: Mem0BackendConfig, | ||
| ) -> None: | ||
| b = Mem0MemoryBackend(mem0_config=mem0_config) | ||
| with pytest.raises(MemoryConnectionError, match="Not connected"): | ||
| await b.search_shared(MemoryQuery(text="test")) | ||
|
|
||
| async def test_retract_raises_when_disconnected( | ||
| self, | ||
| mem0_config: Mem0BackendConfig, | ||
| ) -> None: | ||
| b = Mem0MemoryBackend(mem0_config=mem0_config) | ||
| with pytest.raises(MemoryConnectionError, match="Not connected"): | ||
| await b.retract("test-agent-001", "mem-001") | ||
|
|
||
|
|
||
| # ── Store ───────────────────────────────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestStore: | ||
| async def test_store_success( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.add.return_value = _mem0_add_result("new-mem-id") | ||
|
|
||
| memory_id = await backend.store( | ||
| "test-agent-001", | ||
| _make_store_request(), | ||
| ) | ||
|
|
||
| assert memory_id == "new-mem-id" | ||
| mock_client.add.assert_called_once() | ||
| call_kwargs = mock_client.add.call_args[1] | ||
| assert call_kwargs["user_id"] == "test-agent-001" | ||
| assert call_kwargs["infer"] is False | ||
|
|
||
| async def test_store_empty_results_raises( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.add.return_value = {"results": []} | ||
|
|
||
| with pytest.raises(MemoryStoreError, match="no results"): | ||
| await backend.store("test-agent-001", _make_store_request()) | ||
|
|
||
| async def test_store_missing_id_raises( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.add.return_value = { | ||
| "results": [{"memory": "no id", "event": "ADD"}], | ||
| } | ||
|
|
||
| with pytest.raises(MemoryStoreError, match="missing or blank 'id'"): | ||
| await backend.store("test-agent-001", _make_store_request()) | ||
|
|
||
| async def test_store_exception_wraps( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.add.side_effect = RuntimeError("disk full") | ||
|
|
||
| with pytest.raises(MemoryStoreError, match="Failed to store") as exc_info: | ||
| await backend.store("test-agent-001", _make_store_request()) | ||
|
|
||
| assert exc_info.value.__cause__ is not None | ||
|
|
||
| async def test_store_reraises_memory_error( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """MemoryError is re-raised without wrapping.""" | ||
| mock_client.add.side_effect = MemoryError("out of memory") | ||
| with pytest.raises(MemoryError): | ||
| await backend.store("test-agent-001", _make_store_request()) | ||
|
|
||
|
|
||
| # ── Retrieve ────────────────────────────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestRetrieve: | ||
| async def test_retrieve_with_text( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.search.return_value = _mem0_search_result() | ||
|
|
||
| query = MemoryQuery(text="find relevant", limit=5) | ||
| entries = await backend.retrieve("test-agent-001", query) | ||
|
|
||
| assert len(entries) == 1 | ||
| assert entries[0].content == "found content" | ||
| assert entries[0].relevance_score == 0.85 | ||
| mock_client.search.assert_called_once() | ||
|
|
||
| async def test_retrieve_without_text( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get_all.return_value = _mem0_search_result( | ||
| [ | ||
| { | ||
| "id": "mem-001", | ||
| "memory": "all content", | ||
| "created_at": "2026-03-12T10:00:00+00:00", | ||
| "metadata": {}, | ||
| }, | ||
| ], | ||
| ) | ||
|
|
||
| query = MemoryQuery(text=None, limit=10) | ||
| entries = await backend.retrieve("test-agent-001", query) | ||
|
|
||
| assert len(entries) == 1 | ||
| mock_client.get_all.assert_called_once() | ||
|
|
||
| async def test_retrieve_applies_post_filters( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.search.return_value = _mem0_search_result( | ||
| [ | ||
| { | ||
| "id": "m1", | ||
| "memory": "episodic", | ||
| "score": 0.9, | ||
| "created_at": "2026-03-12T10:00:00+00:00", | ||
| "metadata": {"_synthorg_category": "episodic"}, | ||
| }, | ||
| { | ||
| "id": "m2", | ||
| "memory": "semantic", | ||
| "score": 0.8, | ||
| "created_at": "2026-03-12T10:00:00+00:00", | ||
| "metadata": {"_synthorg_category": "semantic"}, | ||
| }, | ||
| ], | ||
| ) | ||
|
|
||
| query = MemoryQuery( | ||
| text="test", | ||
| categories=frozenset({MemoryCategory.EPISODIC}), | ||
| ) | ||
| entries = await backend.retrieve("test-agent-001", query) | ||
|
|
||
| assert len(entries) == 1 | ||
| assert entries[0].category == MemoryCategory.EPISODIC | ||
|
|
||
| async def test_retrieve_exception_wraps( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.search.side_effect = RuntimeError("search failed") | ||
|
|
||
| with pytest.raises(MemoryRetrievalError, match="Failed to retrieve"): | ||
| await backend.retrieve( | ||
| "test-agent-001", | ||
| MemoryQuery(text="test"), | ||
| ) | ||
|
|
||
| async def test_retrieve_reraises_memory_error( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """MemoryError is re-raised without wrapping.""" | ||
| mock_client.search.side_effect = MemoryError("out of memory") | ||
| with pytest.raises(MemoryError): | ||
| await backend.retrieve( | ||
| "test-agent-001", | ||
| MemoryQuery(text="test"), | ||
| ) | ||
|
|
||
|
|
||
| # ── Get ─────────────────────────────────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestGet: | ||
| async def test_get_existing( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get.return_value = _mem0_get_result("mem-001") | ||
|
|
||
| entry = await backend.get("test-agent-001", "mem-001") | ||
|
|
||
| assert entry is not None | ||
| assert entry.id == "mem-001" | ||
| assert entry.agent_id == "test-agent-001" | ||
|
|
||
| async def test_get_not_found( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get.return_value = None | ||
|
|
||
| entry = await backend.get("test-agent-001", "nonexistent") | ||
|
|
||
| assert entry is None | ||
|
|
||
| async def test_get_exception_wraps( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get.side_effect = RuntimeError("backend error") | ||
|
|
||
| with pytest.raises(MemoryRetrievalError, match="Failed to get"): | ||
| await backend.get("test-agent-001", "mem-001") | ||
|
|
||
|
|
||
| # ── Delete ──────────────────────────────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestDelete: | ||
| async def test_delete_existing( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get.return_value = _mem0_get_result("mem-001") | ||
| mock_client.delete.return_value = None | ||
|
|
||
| result = await backend.delete("test-agent-001", "mem-001") | ||
|
|
||
| assert result is True | ||
| mock_client.delete.assert_called_once_with("mem-001") | ||
|
|
||
| async def test_delete_not_found( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get.return_value = None | ||
|
|
||
| result = await backend.delete("test-agent-001", "nonexistent") | ||
|
|
||
| assert result is False | ||
| mock_client.delete.assert_not_called() | ||
|
|
||
| async def test_delete_exception_wraps( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get.side_effect = RuntimeError("backend error") | ||
|
|
||
| with pytest.raises(MemoryStoreError, match="Failed to delete"): | ||
| await backend.delete("test-agent-001", "mem-001") | ||
|
|
||
| async def test_delete_get_ok_but_delete_fails( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get.return_value = _mem0_get_result("mem-001") | ||
| mock_client.delete.side_effect = RuntimeError("delete failed") | ||
|
|
||
| with pytest.raises(MemoryStoreError, match="Failed to delete"): | ||
| await backend.delete("test-agent-001", "mem-001") | ||
|
|
||
|
|
||
| # ── Count ───────────────────────────────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestCount: | ||
| async def test_count_all( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get_all.return_value = { | ||
| "results": [ | ||
| {"id": "m1", "memory": "a", "metadata": {}}, | ||
| {"id": "m2", "memory": "b", "metadata": {}}, | ||
| ], | ||
| } | ||
|
|
||
| count = await backend.count("test-agent-001") | ||
| assert count == 2 | ||
|
|
||
| async def test_count_by_category( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get_all.return_value = { | ||
| "results": [ | ||
| { | ||
| "id": "m1", | ||
| "memory": "a", | ||
| "metadata": {"_synthorg_category": "episodic"}, | ||
| }, | ||
| { | ||
| "id": "m2", | ||
| "memory": "b", | ||
| "metadata": {"_synthorg_category": "semantic"}, | ||
| }, | ||
| { | ||
| "id": "m3", | ||
| "memory": "c", | ||
| "metadata": {"_synthorg_category": "episodic"}, | ||
| }, | ||
| ], | ||
| } | ||
|
|
||
| count = await backend.count( | ||
| "test-agent-001", | ||
| category=MemoryCategory.EPISODIC, | ||
| ) | ||
| assert count == 2 | ||
|
|
||
| async def test_count_with_invalid_category_in_data( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """Invalid category in stored data defaults to WORKING.""" | ||
| mock_client.get_all.return_value = { | ||
| "results": [ | ||
| { | ||
| "id": "m1", | ||
| "memory": "a", | ||
| "metadata": {"_synthorg_category": "bogus_category"}, | ||
| }, | ||
| { | ||
| "id": "m2", | ||
| "memory": "b", | ||
| "metadata": {"_synthorg_category": "episodic"}, | ||
| }, | ||
| ], | ||
| } | ||
|
|
||
| count = await backend.count( | ||
| "test-agent-001", | ||
| category=MemoryCategory.WORKING, | ||
| ) | ||
| # "bogus_category" defaults to WORKING | ||
| assert count == 1 | ||
|
|
||
| async def test_count_exception_wraps( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get_all.side_effect = RuntimeError("fail") | ||
|
|
||
| with pytest.raises(MemoryRetrievalError, match="Failed to count"): | ||
| await backend.count("test-agent-001") | ||
|
|
||
|
|
||
| # ── Shared Knowledge Store ──────────────────────────────────────── | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestPublish: | ||
| async def test_publish_success( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.add.return_value = _mem0_add_result("shared-mem-001") | ||
|
|
||
| memory_id = await backend.publish( | ||
| "test-agent-001", | ||
| _make_store_request(), | ||
| ) | ||
|
|
||
| assert memory_id == "shared-mem-001" | ||
| call_kwargs = mock_client.add.call_args[1] | ||
| assert call_kwargs["user_id"] == _SHARED_NAMESPACE | ||
| assert _PUBLISHER_KEY in call_kwargs["metadata"] | ||
| assert call_kwargs["metadata"][_PUBLISHER_KEY] == "test-agent-001" | ||
|
|
||
| async def test_publish_empty_results_raises( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.add.return_value = {"results": []} | ||
|
|
||
| with pytest.raises(MemoryStoreError, match="no results"): | ||
| await backend.publish("test-agent-001", _make_store_request()) | ||
|
|
||
| async def test_publish_exception_wraps( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.add.side_effect = RuntimeError("network error") | ||
|
|
||
| with pytest.raises(MemoryStoreError, match="Failed to publish"): | ||
| await backend.publish("test-agent-001", _make_store_request()) | ||
|
|
||
| async def test_publish_missing_id_raises( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """Publish result missing 'id' raises MemoryStoreError.""" | ||
| mock_client.add.return_value = { | ||
| "results": [{"memory": "no id", "event": "ADD"}], | ||
| } | ||
|
|
||
| with pytest.raises(MemoryStoreError, match="missing or blank 'id'"): | ||
| await backend.publish("test-agent-001", _make_store_request()) | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestSearchShared: | ||
| async def test_search_shared_with_text( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.search.return_value = _mem0_search_result( | ||
| [ | ||
| { | ||
| "id": "shared-1", | ||
| "memory": "shared fact", | ||
| "score": 0.9, | ||
| "created_at": "2026-03-12T10:00:00+00:00", | ||
| "metadata": { | ||
| "_synthorg_category": "semantic", | ||
| _PUBLISHER_KEY: "test-agent-002", | ||
| }, | ||
| }, | ||
| ], | ||
| ) | ||
|
|
||
| query = MemoryQuery(text="find shared", limit=5) | ||
| entries = await backend.search_shared(query) | ||
|
|
||
| assert len(entries) == 1 | ||
| assert entries[0].agent_id == "test-agent-002" | ||
| mock_client.search.assert_called_once() | ||
| call_kwargs = mock_client.search.call_args[1] | ||
| assert call_kwargs["user_id"] == _SHARED_NAMESPACE | ||
|
|
||
| async def test_search_shared_without_text( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get_all.return_value = _mem0_search_result( | ||
| [ | ||
| { | ||
| "id": "shared-1", | ||
| "memory": "shared fact", | ||
| "created_at": "2026-03-12T10:00:00+00:00", | ||
| "metadata": { | ||
| _PUBLISHER_KEY: "test-agent-002", | ||
| }, | ||
| }, | ||
| ], | ||
| ) | ||
|
|
||
| query = MemoryQuery(text=None) | ||
| entries = await backend.search_shared(query) | ||
|
|
||
| assert len(entries) == 1 | ||
| mock_client.get_all.assert_called_once() | ||
|
|
||
| async def test_search_shared_exclude_agent( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.search.return_value = _mem0_search_result( | ||
| [ | ||
| { | ||
| "id": "s1", | ||
| "memory": "from agent 1", | ||
| "score": 0.9, | ||
| "created_at": "2026-03-12T10:00:00+00:00", | ||
| "metadata": {_PUBLISHER_KEY: "test-agent-001"}, | ||
| }, | ||
| { | ||
| "id": "s2", | ||
| "memory": "from agent 2", | ||
| "score": 0.8, | ||
| "created_at": "2026-03-12T10:00:00+00:00", | ||
| "metadata": {_PUBLISHER_KEY: "test-agent-002"}, | ||
| }, | ||
| ], | ||
| ) | ||
|
|
||
| query = MemoryQuery(text="test") | ||
| entries = await backend.search_shared( | ||
| query, | ||
| exclude_agent="test-agent-001", | ||
| ) | ||
|
|
||
| assert len(entries) == 1 | ||
| assert entries[0].agent_id == "test-agent-002" | ||
|
|
||
| async def test_search_shared_exception_wraps( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.search.side_effect = RuntimeError("search error") | ||
|
|
||
| with pytest.raises(MemoryRetrievalError, match="Failed to search"): | ||
| await backend.search_shared(MemoryQuery(text="test")) | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestRetract: | ||
| async def test_retract_success( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get.return_value = { | ||
| "id": "shared-001", | ||
| "memory": "shared content", | ||
| "created_at": "2026-03-12T10:00:00+00:00", | ||
| "metadata": {_PUBLISHER_KEY: "test-agent-001"}, | ||
| } | ||
| mock_client.delete.return_value = None | ||
|
|
||
| result = await backend.retract("test-agent-001", "shared-001") | ||
|
|
||
| assert result is True | ||
| mock_client.delete.assert_called_once_with("shared-001") | ||
|
|
||
| async def test_retract_not_found( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get.return_value = None | ||
|
|
||
| result = await backend.retract("test-agent-001", "nonexistent") | ||
|
|
||
| assert result is False | ||
|
|
||
| async def test_retract_ownership_mismatch( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get.return_value = { | ||
| "id": "shared-001", | ||
| "memory": "content", | ||
| "created_at": "2026-03-12T10:00:00+00:00", | ||
| "metadata": {_PUBLISHER_KEY: "test-agent-002"}, | ||
| } | ||
|
|
||
| with pytest.raises(MemoryStoreError, match="cannot retract"): | ||
| await backend.retract("test-agent-001", "shared-001") | ||
|
|
||
| async def test_retract_no_publisher_raises( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get.return_value = { | ||
| "id": "not-shared-001", | ||
| "memory": "private content", | ||
| "created_at": "2026-03-12T10:00:00+00:00", | ||
| "metadata": {}, | ||
| } | ||
|
|
||
| with pytest.raises(MemoryStoreError, match="not a shared memory"): | ||
| await backend.retract("test-agent-001", "not-shared-001") | ||
|
|
||
| async def test_retract_exception_wraps( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| mock_client.get.side_effect = RuntimeError("backend error") | ||
|
|
||
| with pytest.raises(MemoryStoreError, match="Failed to retract"): | ||
| await backend.retract("test-agent-001", "shared-001") | ||
|
|
||
| async def test_retract_reraises_memory_error( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """MemoryError is re-raised without wrapping.""" | ||
| mock_client.get.side_effect = MemoryError("out of memory") | ||
| with pytest.raises(MemoryError): | ||
| await backend.retract("test-agent-001", "shared-001") | ||
|
|
||
| async def test_retract_delete_failure_wraps( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """Exception during delete phase wraps in MemoryStoreError.""" | ||
| mock_client.get.return_value = { | ||
| "id": "shared-001", | ||
| "memory": "content", | ||
| "created_at": "2026-03-12T10:00:00+00:00", | ||
| "metadata": {_PUBLISHER_KEY: "test-agent-001"}, | ||
| } | ||
| mock_client.delete.side_effect = RuntimeError("delete failed") | ||
|
|
||
| with pytest.raises(MemoryStoreError, match="Failed to retract"): | ||
| await backend.retract("test-agent-001", "shared-001") | ||
|
|
||
|
|
||
| @pytest.mark.unit | ||
| class TestAdditionalEdgeCases: | ||
| """Edge cases for improved coverage.""" | ||
|
|
||
| async def test_store_blank_id_from_add_raises( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """Store result with blank ID raises MemoryStoreError.""" | ||
| mock_client.add.return_value = { | ||
| "results": [{"id": "", "event": "ADD"}], | ||
| } | ||
| with pytest.raises(MemoryStoreError, match="missing or blank 'id'"): | ||
| await backend.store("test-agent-001", _make_store_request()) | ||
|
|
||
| async def test_store_whitespace_id_from_add_raises( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """Store result with whitespace-only ID raises MemoryStoreError.""" | ||
| mock_client.add.return_value = { | ||
| "results": [{"id": " ", "event": "ADD"}], | ||
| } | ||
| with pytest.raises(MemoryStoreError, match="missing or blank 'id'"): | ||
| await backend.store("test-agent-001", _make_store_request()) | ||
|
|
||
| async def test_get_reraises_memory_error( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """MemoryError is re-raised without wrapping in get().""" | ||
| mock_client.get.side_effect = MemoryError("out of memory") | ||
| with pytest.raises(MemoryError): | ||
| await backend.get("test-agent-001", "mem-001") | ||
|
|
||
| async def test_delete_reraises_memory_error( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """MemoryError is re-raised without wrapping in delete().""" | ||
| mock_client.get.side_effect = MemoryError("out of memory") | ||
| with pytest.raises(MemoryError): | ||
| await backend.delete("test-agent-001", "mem-001") | ||
|
|
||
| async def test_count_reraises_memory_error( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """MemoryError is re-raised without wrapping in count().""" | ||
| mock_client.get_all.side_effect = MemoryError("out of memory") | ||
| with pytest.raises(MemoryError): | ||
| await backend.count("test-agent-001") | ||
|
|
||
| async def test_publish_reraises_memory_error( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """MemoryError is re-raised without wrapping in publish().""" | ||
| mock_client.add.side_effect = MemoryError("out of memory") | ||
| with pytest.raises(MemoryError): | ||
| await backend.publish("test-agent-001", _make_store_request()) | ||
|
|
||
| async def test_search_shared_reraises_memory_error( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """MemoryError is re-raised without wrapping in search_shared().""" | ||
| mock_client.search.side_effect = MemoryError("out of memory") | ||
| with pytest.raises(MemoryError): | ||
| await backend.search_shared(MemoryQuery(text="test")) | ||
|
|
||
| async def test_store_non_list_results_raises( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """Store result with non-list 'results' raises MemoryStoreError.""" | ||
| mock_client.add.return_value = {"results": "not-a-list"} | ||
| with pytest.raises(MemoryStoreError, match="no results"): | ||
| await backend.store("test-agent-001", _make_store_request()) | ||
|
|
||
| async def test_retrieve_invalid_entry_raises( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """Invalid entry in search results wraps as MemoryRetrievalError.""" | ||
| mock_client.search.return_value = { | ||
| "results": [ | ||
| {"id": "", "memory": "blank id", "metadata": {}}, | ||
| ], | ||
| } | ||
| with pytest.raises(MemoryRetrievalError, match="missing or blank"): | ||
| await backend.retrieve( | ||
| "test-agent-001", | ||
| MemoryQuery(text="test"), | ||
| ) | ||
|
|
||
| async def test_search_shared_no_publisher_uses_namespace( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """Entries without publisher metadata use the shared namespace.""" | ||
| mock_client.search.return_value = _mem0_search_result( | ||
| [ | ||
| { | ||
| "id": "shared-1", | ||
| "memory": "orphan fact", | ||
| "score": 0.9, | ||
| "created_at": "2026-03-12T10:00:00+00:00", | ||
| "metadata": {"_synthorg_category": "semantic"}, | ||
| }, | ||
| ], | ||
| ) | ||
|
|
||
| entries = await backend.search_shared(MemoryQuery(text="test")) | ||
| assert len(entries) == 1 | ||
| assert entries[0].agent_id == _SHARED_NAMESPACE | ||
|
|
||
| async def test_count_empty_results( | ||
| self, | ||
| backend: Mem0MemoryBackend, | ||
| mock_client: MagicMock, | ||
| ) -> None: | ||
| """Count returns 0 for empty results.""" | ||
| mock_client.get_all.return_value = {"results": []} | ||
| count = await backend.count("test-agent-001") | ||
| assert count == 0 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Split this test module by concern before it gets harder to maintain.
This new file is already over 1,100 lines and mixes lifecycle, CRUD, shared-store, and edge-case coverage in one place. Please split it into smaller modules and collapse the repeated test matrices with @pytest.mark.parametrize so failures stay easy to locate and the file stays under the repo limit. As per coding guidelines "Functions: < 50 lines, files < 800 lines" and "Prefer @pytest.mark.parametrize for testing similar cases".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/memory/backends/mem0/test_adapter.py` around lines 1 - 1117, The
test module is too large and mixes many concerns; split it into smaller files
grouping related test classes (e.g., move TestLifecycle, TestConnectionGuard to
tests/unit/memory/backends/mem0/test_lifecycle.py; TestStore and TestPublish to
test_store.py; TestRetrieve and TestSearchShared to test_retrieve_shared.py;
TestGet/TestDelete/TestCount/TestRetract to their own files; and
TestAdditionalEdgeCases to test_edgecases.py) and replace repeated similar
assertions with pytest.mark.parametrize (collapse matrix-like tests in
TestConnectionGuard, TestStore empty/missing/id cases, TestRetrieve variants,
TestCount variants, TestPublish variants, TestSearchShared exclude/namespace
cases, and TestRetract ownership/no-publisher/delete-failure cases) to reduce
duplication while preserving unique test names like
TestLifecycle.test_connect_success,
Mem0MemoryBackend.store/retrieve/get/delete/count/publish/search_shared/retract,
and helper fixtures (_mem0_add_result, _mem0_search_result, _mem0_get_result,
_make_store_request) so each new file stays under ~800 lines and functions
remain <50 lines.
…ig, and tests - mappers.py: add str() conversion before NotBlankStr for tags, clamp confidence to [0.0, 1.0], log missing/non-dict metadata and unexpected tag types, guard blank publisher strings, document None relevance score behavior in apply_post_filters, clean up historical comment - config.py: add structured logging to _reject_traversal validator, validate unsupported storage overrides in build_config_from_company_config - factory.py: wrap ValueError from config construction as MemoryConfigError - pyproject.toml: move mem0ai to optional-dependencies, add TC to test per-file-ignores - __init__.py: guard Mem0 imports with contextlib.suppress(ImportError) - adapter.py: log during disconnect reset failure instead of bare pass - dependency-review.yml: version-pin allowed PURLs - docs: add embedder config to memory.md YAML example, update roadmap Current Status to mention Mem0 adapter - tests: split test_adapter.py (1118 lines) into conftest.py + test_adapter.py + test_adapter_crud.py + test_adapter_shared.py; add protocol isinstance checks, connect MemoryError/RecursionError propagation, agent_id validation, ownership mismatch, and shared namespace guard tests - Clean up 78 now-unused noqa: TC* directives across test files
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #338 +/- ##
==========================================
+ Coverage 93.76% 93.90% +0.13%
==========================================
Files 434 446 +12
Lines 19850 20699 +849
Branches 1915 1993 +78
==========================================
+ Hits 18613 19438 +825
- Misses 956 975 +19
- Partials 281 286 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mem0ai was moved to optional-dependencies — CI needs --extra mem0 in the uv sync command to install it for tests.
| try: | ||
| config_dict = build_mem0_config_dict(self._mem0_config) | ||
| client = await asyncio.to_thread(Memory.from_config, config_dict) | ||
| except builtins.MemoryError, RecursionError: |
There was a problem hiding this comment.
Python 2 except syntax used throughout — invalid in Python 3
Every except clause that catches multiple exception types uses the old Python 2 comma syntax instead of a parenthesized tuple. In Python 3, except A, B: is a SyntaxError; the correct form is except (A, B):.
This pattern appears on lines 164, 222, 354, 418, 491, 579, 653, 732, 810, and 897 of this file, and also at src/ai_company/memory/backends/mem0/mappers.py:76 and mappers.py:145.
Example fix for line 164:
| except builtins.MemoryError, RecursionError: | |
| except (builtins.MemoryError, RecursionError): |
The same one-line fix applies to every other occurrence — wrap both exception types in parentheses.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ai_company/memory/backends/mem0/adapter.py
Line: 164
Comment:
**Python 2 `except` syntax used throughout — invalid in Python 3**
Every `except` clause that catches multiple exception types uses the old Python 2 comma syntax instead of a parenthesized tuple. In Python 3, `except A, B:` is a `SyntaxError`; the correct form is `except (A, B):`.
This pattern appears on lines 164, 222, 354, 418, 491, 579, 653, 732, 810, and 897 of this file, and also at `src/ai_company/memory/backends/mem0/mappers.py:76` and `mappers.py:145`.
Example fix for line 164:
```suggestion
except (builtins.MemoryError, RecursionError):
```
The same one-line fix applies to every other occurrence — wrap both exception types in parentheses.
How can I resolve this? If you propose a fix, please make it concise.| if config.storage.vector_store not in ("qdrant", "qdrant-external"): | ||
| msg = ( | ||
| f"Mem0 backend only supports qdrant vector stores, " | ||
| f"got {config.storage.vector_store!r}" | ||
| ) | ||
| logger.warning( | ||
| MEMORY_BACKEND_CONFIG_INVALID, | ||
| backend="mem0", | ||
| field="vector_store", | ||
| value=config.storage.vector_store, | ||
| reason=msg, | ||
| ) | ||
| raise ValueError(msg) |
There was a problem hiding this comment.
qdrant-external is accepted but silently falls back to embedded Qdrant
build_config_from_company_config allows "qdrant-external" as a valid vector_store value (line 143), but build_mem0_config_dict always generates an embedded Qdrant config with a local path key — there is no code path that produces a host/port/url-based external Qdrant configuration.
A user who sets vector_store: "qdrant-external" in their YAML will receive a silently embedded Qdrant instance, which likely contradicts their intent (e.g. a production cluster). Either:
- Remove
"qdrant-external"from the allowlist and raise aValueErrorif it is supplied, or - Add a separate code path in
build_mem0_config_dictthat emits a host/port config when the external store is requested.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ai_company/memory/backends/mem0/config.py
Line: 143-155
Comment:
**`qdrant-external` is accepted but silently falls back to embedded Qdrant**
`build_config_from_company_config` allows `"qdrant-external"` as a valid `vector_store` value (line 143), but `build_mem0_config_dict` always generates an embedded Qdrant config with a local `path` key — there is no code path that produces a host/port/url-based external Qdrant configuration.
A user who sets `vector_store: "qdrant-external"` in their YAML will receive a silently embedded Qdrant instance, which likely contradicts their intent (e.g. a production cluster). Either:
- Remove `"qdrant-external"` from the allowlist and raise a `ValueError` if it is supplied, or
- Add a separate code path in `build_mem0_config_dict` that emits a host/port config when the external store is requested.
How can I resolve this? If you propose a fix, please make it concise.| return None | ||
| try: | ||
| dt = datetime.fromisoformat(raw) | ||
| except ValueError, TypeError: |
There was a problem hiding this comment.
Python 2 except comma syntax — same issue as adapter.py
except ValueError, TypeError: is the old Python 2 syntax and is a SyntaxError in Python 3. Both occurrences in this file (line 76 and line 145) need parentheses:
| except ValueError, TypeError: | |
| except (ValueError, TypeError): |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ai_company/memory/backends/mem0/mappers.py
Line: 76
Comment:
**Python 2 `except` comma syntax — same issue as `adapter.py`**
`except ValueError, TypeError:` is the old Python 2 syntax and is a `SyntaxError` in Python 3. Both occurrences in this file (line 76 and line 145) need parentheses:
```suggestion
except (ValueError, TypeError):
```
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (2)
src/ai_company/memory/backends/mem0/config.py (1)
143-156:⚠️ Potential issue | 🟠 MajorReject
qdrant-externaluntil external wiring is actually supported.At Line 143,
"qdrant-external"is accepted, but the emitted Mem0 config always uses local embedded Qdrant paths. This silently ignores caller intent and can misroute storage in production.Proposed fix
- if config.storage.vector_store not in ("qdrant", "qdrant-external"): + if config.storage.vector_store != "qdrant": msg = ( - f"Mem0 backend only supports qdrant vector stores, " + f"Mem0 backend currently supports only embedded qdrant, " f"got {config.storage.vector_store!r}" ) logger.warning( MEMORY_BACKEND_CONFIG_INVALID, backend="mem0", field="vector_store", value=config.storage.vector_store, reason=msg, ) raise ValueError(msg)As per coding guidelines "Validate at system boundaries (user input, external APIs, config files)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ai_company/memory/backends/mem0/config.py` around lines 143 - 156, The check in mem0 config currently accepts "qdrant-external" even though external wiring isn't implemented; update the validation around config.storage.vector_store in src/ai_company/memory/backends/mem0/config.py to only allow "qdrant" (remove "qdrant-external" from the allowed tuple), and adjust the warning/ValueError message emitted via MEMORY_BACKEND_CONFIG_INVALID to clearly state that only "qdrant" is supported for backend "mem0" (include the offending value via config.storage.vector_store in the log and error text).tests/unit/memory/backends/mem0/test_adapter.py (1)
258-322: 🧹 Nitpick | 🔵 TrivialParametrize the disconnected-guard matrix to reduce repetition.
These tests all follow the same setup and assertion pattern; collapsing them into one parametrized test will make failures easier to maintain.
Refactor sketch
+@pytest.mark.parametrize( + ("method_name", "args"), + [ + ("store", ("test-agent-001", make_store_request())), + ("retrieve", ("test-agent-001", MemoryQuery(text="test"))), + ("get", ("test-agent-001", "mem-001")), + ("delete", ("test-agent-001", "mem-001")), + ("count", ("test-agent-001",)), + ("publish", ("test-agent-001", make_store_request())), + ("search_shared", (MemoryQuery(text="test"),)), + ("retract", ("test-agent-001", "mem-001")), + ], +) +async def test_operation_raises_when_disconnected( + self, + mem0_config: Mem0BackendConfig, + method_name: str, + args: tuple[object, ...], +) -> None: + backend = Mem0MemoryBackend(mem0_config=mem0_config) + with pytest.raises(MemoryConnectionError, match="Not connected"): + await getattr(backend, method_name)(*args)As per coding guidelines "Prefer
@pytest.mark.parametrizefor testing similar cases".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/memory/backends/mem0/test_adapter.py` around lines 258 - 322, The tests in TestConnectionGuard duplicate the same disconnected-guard pattern for methods like Mem0MemoryBackend.store, .retrieve, .get, .delete, .count, .publish, .search_shared and .retract; replace them with a single parametrized test using pytest.mark.parametrize that iterates tuples of (method_name, args, kwargs, expected_msg) and invokes the backend method via getattr(b, method_name)(*args, **kwargs) inside pytest.raises(MemoryConnectionError, match="Not connected"); ensure you include proper call payloads (e.g. make_store_request() for store/publish, MemoryQuery(text="test") for retrieve/search_shared) and keep TestConnectionGuard and mem0_config fixture usage the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ai_company/memory/__init__.py`:
- Around line 14-18: The __all__ list currently names Mem0EmbedderConfig and
Mem0MemoryBackend even when mem0ai isn't installed, causing
NameError/autocomplete issues; fix by only adding those symbols to __all__
inside the same contextlib.suppress(ImportError) block (or by using a try/except
ImportError around the from import and appending the names to __all__ on
success), so ensure __all__ is conditionally extended when Mem0EmbedderConfig
and Mem0MemoryBackend are actually defined.
In `@src/ai_company/memory/backends/mem0/adapter.py`:
- Around line 1-916: The file triggers the project's 800-line limit but is
intentionally cohesive; keep the implementation as-is and add a short documented
exemption at the top of the module explaining why this module remains >800 lines
(mentioning the Mem0MemoryBackend class and that it implements
MemoryBackend/MemoryCapabilities/SharedKnowledgeStore together), so reviewers/CI
know this is accepted technical debt rather than an accidental omission.
In `@src/ai_company/memory/backends/mem0/mappers.py`:
- Around line 103-177: parse_mem0_metadata (and mem0_result_to_entry) are too
large and do mixed responsibilities (validation, coercion, mapping, fallback);
split them into small helpers so each function stays <50 lines. Extract: a
_validate_raw_metadata(raw_metadata) that handles the initial dict check and
logging, a _coerce_confidence(raw_confidence) that converts/clamps and logs
errors, a _normalize_tags(raw_tags) that normalizes string/list/tuple and
returns tuple[NotBlankStr], and a _map_metadata_fields(raw_metadata) or
_build_memory_metadata(...) that assembles MemoryMetadata (using source,
confidence, tags) and calls parse_mem0_datetime for expires_at; then simplify
parse_mem0_metadata to call those helpers and keep only orchestration and final
return. Update mem0_result_to_entry similarly to delegate validation, mapping
and fallback logic to small helpers (reuse
_validate_raw_metadata/_normalize_tags/_coerce_confidence where applicable) to
meet the per-function size limit.
- Around line 89-100: The normalize_relevance_score function currently assumes
score is numeric and will raise TypeError if given non-numeric values (e.g.,
"0.82"); update normalize_relevance_score to safe-handle non-numeric inputs by
coercing string numeric values to float and returning None (or a clamped value)
when conversion fails, and ensure the caller that uses raw.get("score") passes
its result through this updated normalize_relevance_score; reference the
function normalize_relevance_score and the usage site where raw.get("score") is
mapped so you convert/validate the value before clamping.
- Around line 405-408: The extract_publisher implementation should normalize the
metadata value read via metadata.get(_PUBLISHER_KEY): coerce non-None values to
str, strip surrounding whitespace, and return None if the resulting string is
empty; otherwise return the stripped string. Update the code in
extract_publisher to treat the retrieved value (currently named value) as
potentially non-string, convert it with str(value) when not None, call .strip(),
and only return the cleaned string or None to satisfy the declared return type
(str | None).
- Around line 354-361: The code in validate_add_result assumes result and
results_list[0] are dicts and dereferences .get(), which can raise
AttributeError for malformed Mem0 payloads; update validate_add_result to first
verify that result is a dict and results_list is a non-empty list of dicts
(check isinstance(result, dict) and isinstance(first, dict)) before calling
.get(), and if those checks fail log MEMORY_ENTRY_STORE_FAILED via
logger.warning with context and a clear message and raise MemoryStoreError;
reference the variables/functions validate_add_result, results_list, first,
raw_id, MemoryStoreError, MEMORY_ENTRY_STORE_FAILED, and logger.warning when
making the changes.
- Around line 154-176: The code passes raw `source` directly into MemoryMetadata
which can raise if blank/invalid; retrieve the value from
`raw_metadata.get(f"{_PREFIX}source")`, sanitize it by converting to string,
trimming, and using NotBlankStr only when non-empty, and catch any exception
from NotBlankStr to set `source = None`; when falling back to None, emit a debug
log using the existing MEMORY_MODEL_INVALID constant with field="source" and the
raw value info; finally pass the sanitized `source` into `MemoryMetadata` (same
as current usage) so malformed source values degrade safely to None.
In `@tests/unit/memory/backends/mem0/conftest.py`:
- Around line 85-96: The helper mem0_get_result should accept an optional
user_id so tests don't need to mutate the result; update the signature of
mem0_get_result to add a parameter like user_id: Optional[str] = None and
include the "user_id" key in the returned dict (set to the passed user_id,
possibly None by default) so callers can produce ownership-specific results
directly; keep the existing memory_id parameter and return structure otherwise
unchanged and ensure typing imports remain valid for Any/Optional.
---
Duplicate comments:
In `@src/ai_company/memory/backends/mem0/config.py`:
- Around line 143-156: The check in mem0 config currently accepts
"qdrant-external" even though external wiring isn't implemented; update the
validation around config.storage.vector_store in
src/ai_company/memory/backends/mem0/config.py to only allow "qdrant" (remove
"qdrant-external" from the allowed tuple), and adjust the warning/ValueError
message emitted via MEMORY_BACKEND_CONFIG_INVALID to clearly state that only
"qdrant" is supported for backend "mem0" (include the offending value via
config.storage.vector_store in the log and error text).
In `@tests/unit/memory/backends/mem0/test_adapter.py`:
- Around line 258-322: The tests in TestConnectionGuard duplicate the same
disconnected-guard pattern for methods like Mem0MemoryBackend.store, .retrieve,
.get, .delete, .count, .publish, .search_shared and .retract; replace them with
a single parametrized test using pytest.mark.parametrize that iterates tuples of
(method_name, args, kwargs, expected_msg) and invokes the backend method via
getattr(b, method_name)(*args, **kwargs) inside
pytest.raises(MemoryConnectionError, match="Not connected"); ensure you include
proper call payloads (e.g. make_store_request() for store/publish,
MemoryQuery(text="test") for retrieve/search_shared) and keep
TestConnectionGuard and mem0_config fixture usage the same.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 33200657-9220-4122-893d-6bb7875069a4
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (70)
.github/workflows/dependency-review.ymldocs/design/memory.mddocs/roadmap/index.mdpyproject.tomlsrc/ai_company/memory/__init__.pysrc/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/backends/mem0/config.pysrc/ai_company/memory/backends/mem0/mappers.pysrc/ai_company/memory/factory.pysrc/ai_company/observability/events/memory.pytests/integration/communication/test_meeting_integration.pytests/integration/tools/conftest.pytests/integration/tools/test_sandbox_integration.pytests/unit/api/auth/test_controller.pytests/unit/api/conftest.pytests/unit/api/controllers/test_agents.pytests/unit/api/controllers/test_analytics.pytests/unit/api/controllers/test_approvals.pytests/unit/api/controllers/test_artifacts.pytests/unit/api/controllers/test_autonomy.pytests/unit/api/controllers/test_budget.pytests/unit/api/controllers/test_company.pytests/unit/api/controllers/test_departments.pytests/unit/api/controllers/test_meetings.pytests/unit/api/controllers/test_messages.pytests/unit/api/controllers/test_projects.pytests/unit/api/controllers/test_providers.pytests/unit/api/controllers/test_tasks.pytests/unit/api/test_app.pytests/unit/api/test_guards.pytests/unit/api/test_health.pytests/unit/api/test_middleware.pytests/unit/budget/test_category_analytics.pytests/unit/communication/conflict_resolution/test_authority_strategy.pytests/unit/communication/conflict_resolution/test_debate_strategy.pytests/unit/communication/conflict_resolution/test_helpers.pytests/unit/communication/conflict_resolution/test_hybrid_strategy.pytests/unit/communication/conflict_resolution/test_service.pytests/unit/communication/meeting/conftest.pytests/unit/communication/meeting/test_orchestrator.pytests/unit/communication/meeting/test_position_papers.pytests/unit/communication/meeting/test_protocol.pytests/unit/communication/meeting/test_round_robin.pytests/unit/communication/meeting/test_structured_phases.pytests/unit/engine/task_engine_helpers.pytests/unit/engine/test_agent_engine.pytests/unit/engine/test_agent_engine_errors.pytests/unit/engine/test_agent_engine_lifecycle.pytests/unit/engine/test_context.pytests/unit/engine/test_loop_protocol.pytests/unit/engine/test_metrics.pytests/unit/engine/test_plan_execute_loop.pytests/unit/engine/test_react_loop.pytests/unit/engine/test_routing_models.pytests/unit/engine/test_task_engine_mutations.pytests/unit/hr/test_full_snapshot_strategy.pytests/unit/hr/test_hiring_service.pytests/unit/hr/test_offboarding_service.pytests/unit/hr/test_onboarding_service.pytests/unit/hr/test_registry.pytests/unit/memory/backends/mem0/conftest.pytests/unit/memory/backends/mem0/test_adapter.pytests/unit/memory/backends/mem0/test_adapter_crud.pytests/unit/memory/backends/mem0/test_adapter_shared.pytests/unit/providers/conftest.pytests/unit/providers/test_protocol.pytests/unit/tools/git/conftest.pytests/unit/tools/git/test_git_sandbox_integration.pytests/unit/tools/sandbox/conftest.pytests/unit/tools/sandbox/test_protocol.py
📜 Review details
⏰ 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: Greptile Review
- GitHub Check: Test (Python 3.14)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Do NOT usefrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:(no parentheses) for exception handling syntax per PEP 758 — ruff enforces this on Python 3.14
Files:
tests/unit/api/controllers/test_tasks.pytests/unit/tools/sandbox/conftest.pytests/unit/communication/conflict_resolution/test_helpers.pytests/unit/api/controllers/test_artifacts.pytests/unit/engine/test_agent_engine.pytests/unit/hr/test_hiring_service.pytests/unit/communication/meeting/conftest.pytests/unit/engine/task_engine_helpers.pytests/unit/api/controllers/test_approvals.pytests/unit/hr/test_full_snapshot_strategy.pytests/unit/api/controllers/test_providers.pytests/unit/tools/sandbox/test_protocol.pytests/unit/communication/meeting/test_protocol.pytests/unit/hr/test_registry.pytests/unit/engine/test_agent_engine_lifecycle.pytests/integration/tools/conftest.pytests/unit/tools/git/conftest.pytests/unit/api/conftest.pytests/unit/api/test_middleware.pytests/unit/budget/test_category_analytics.pytests/unit/engine/test_loop_protocol.pytests/unit/api/controllers/test_company.pytests/integration/tools/test_sandbox_integration.pytests/unit/engine/test_task_engine_mutations.pytests/unit/communication/meeting/test_orchestrator.pytests/unit/hr/test_onboarding_service.pytests/unit/memory/backends/mem0/test_adapter_crud.pytests/unit/api/controllers/test_projects.pytests/unit/api/controllers/test_messages.pysrc/ai_company/memory/backends/mem0/config.pytests/unit/api/test_app.pytests/unit/engine/test_plan_execute_loop.pytests/unit/communication/conflict_resolution/test_debate_strategy.pytests/unit/tools/git/test_git_sandbox_integration.pytests/unit/api/test_health.pytests/unit/api/auth/test_controller.pytests/unit/communication/conflict_resolution/test_authority_strategy.pytests/unit/api/controllers/test_analytics.pytests/unit/engine/test_agent_engine_errors.pysrc/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/observability/events/memory.pysrc/ai_company/memory/backends/mem0/mappers.pytests/unit/api/controllers/test_agents.pytests/unit/api/controllers/test_departments.pytests/unit/memory/backends/mem0/conftest.pytests/unit/engine/test_react_loop.pytests/unit/memory/backends/mem0/test_adapter.pytests/unit/providers/conftest.pytests/unit/engine/test_context.pytests/unit/api/controllers/test_budget.pytests/integration/communication/test_meeting_integration.pytests/unit/communication/meeting/test_round_robin.pytests/unit/communication/meeting/test_structured_phases.pytests/unit/communication/conflict_resolution/test_hybrid_strategy.pytests/unit/api/controllers/test_meetings.pysrc/ai_company/memory/factory.pytests/unit/hr/test_offboarding_service.pytests/unit/communication/conflict_resolution/test_service.pytests/unit/api/test_guards.pytests/unit/engine/test_metrics.pytests/unit/providers/test_protocol.pytests/unit/engine/test_routing_models.pytests/unit/memory/backends/mem0/test_adapter_shared.pysrc/ai_company/memory/__init__.pytests/unit/api/controllers/test_autonomy.pytests/unit/communication/meeting/test_position_papers.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowfor test markers
Prefer@pytest.mark.parametrizefor testing similar cases
Files:
tests/unit/api/controllers/test_tasks.pytests/unit/tools/sandbox/conftest.pytests/unit/communication/conflict_resolution/test_helpers.pytests/unit/api/controllers/test_artifacts.pytests/unit/engine/test_agent_engine.pytests/unit/hr/test_hiring_service.pytests/unit/communication/meeting/conftest.pytests/unit/engine/task_engine_helpers.pytests/unit/api/controllers/test_approvals.pytests/unit/hr/test_full_snapshot_strategy.pytests/unit/api/controllers/test_providers.pytests/unit/tools/sandbox/test_protocol.pytests/unit/communication/meeting/test_protocol.pytests/unit/hr/test_registry.pytests/unit/engine/test_agent_engine_lifecycle.pytests/integration/tools/conftest.pytests/unit/tools/git/conftest.pytests/unit/api/conftest.pytests/unit/api/test_middleware.pytests/unit/budget/test_category_analytics.pytests/unit/engine/test_loop_protocol.pytests/unit/api/controllers/test_company.pytests/integration/tools/test_sandbox_integration.pytests/unit/engine/test_task_engine_mutations.pytests/unit/communication/meeting/test_orchestrator.pytests/unit/hr/test_onboarding_service.pytests/unit/memory/backends/mem0/test_adapter_crud.pytests/unit/api/controllers/test_projects.pytests/unit/api/controllers/test_messages.pytests/unit/api/test_app.pytests/unit/engine/test_plan_execute_loop.pytests/unit/communication/conflict_resolution/test_debate_strategy.pytests/unit/tools/git/test_git_sandbox_integration.pytests/unit/api/test_health.pytests/unit/api/auth/test_controller.pytests/unit/communication/conflict_resolution/test_authority_strategy.pytests/unit/api/controllers/test_analytics.pytests/unit/engine/test_agent_engine_errors.pytests/unit/api/controllers/test_agents.pytests/unit/api/controllers/test_departments.pytests/unit/memory/backends/mem0/conftest.pytests/unit/engine/test_react_loop.pytests/unit/memory/backends/mem0/test_adapter.pytests/unit/providers/conftest.pytests/unit/engine/test_context.pytests/unit/api/controllers/test_budget.pytests/integration/communication/test_meeting_integration.pytests/unit/communication/meeting/test_round_robin.pytests/unit/communication/meeting/test_structured_phases.pytests/unit/communication/conflict_resolution/test_hybrid_strategy.pytests/unit/api/controllers/test_meetings.pytests/unit/hr/test_offboarding_service.pytests/unit/communication/conflict_resolution/test_service.pytests/unit/api/test_guards.pytests/unit/engine/test_metrics.pytests/unit/providers/test_protocol.pytests/unit/engine/test_routing_models.pytests/unit/memory/backends/mem0/test_adapter_shared.pytests/unit/api/controllers/test_autonomy.pytests/unit/communication/meeting/test_position_papers.py
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Docs source:
docs/(Markdown, built with Zensical). Design spec:docs/design/(7 pages: index, agents, organization, communication, engine, memory, operations).
Files:
docs/roadmap/index.mddocs/design/memory.md
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Type hints: all public functions must have type hints, mypy strict mode
Docstrings: Google style, required on public classes/functions (enforced by ruff D rules)
Create new objects instead of mutating existing ones—never mutate existing objects. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement.
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention and usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Config vs runtime state: use frozen Pydantic models for config/identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict). Use@computed_fieldfor derived values instead of storing + validating redundant fields. UseNotBlankStr(fromcore.types) for all identifier/name fields—including optional and tuple variants—instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task.
Line length: 88 characters (enforced by ruff)
Functions: < 50 lines, files < 800 lines
Handle errors explicitly, never silently swallow them
Validate at system boundaries (user input, external APIs, config files)
Every module with business logic MUST have:from ai_company.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code
Always useloggeras the variable name (not_logger, notlog)
Use event name constants fromai_company.observability.eventsdomain-spe...
Files:
src/ai_company/memory/backends/mem0/config.pysrc/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/observability/events/memory.pysrc/ai_company/memory/backends/mem0/mappers.pysrc/ai_company/memory/factory.pysrc/ai_company/memory/__init__.py
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: Useasyncio_mode = "auto"in pytest configuration—no manual@pytest.mark.asyncioneeded on individual tests
Test timeout: 30 seconds per test
Dependencies: all versions pinned with==inpyproject.toml. Groups:testanddev(includes test). Install viauv sync.
Files:
pyproject.toml
.github/workflows/*.yml
📄 CodeRabbit inference engine (CLAUDE.md)
CI: Jobs (lint + type-check + test in parallel) → ci-pass gate. Pages workflow exports OpenAPI schema, builds Astro landing + Zensical docs, merges, deploys to GitHub Pages on push to main.
Files:
.github/workflows/dependency-review.yml
🧠 Learnings (8)
📚 Learning: 2026-03-13T06:54:28.690Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.690Z
Learning: Applies to docs/**/*.md : Docs source: `docs/` (Markdown, built with Zensical). Design spec: `docs/design/` (7 pages: index, agents, organization, communication, engine, memory, operations).
Applied to files:
docs/roadmap/index.md
📚 Learning: 2026-03-13T06:54:28.690Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.690Z
Learning: Applies to src/**/*.py : Every module with business logic MUST have: `from ai_company.observability import get_logger` then `logger = get_logger(__name__)`
Applied to files:
src/ai_company/memory/backends/mem0/config.py
📚 Learning: 2026-03-13T06:54:28.690Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.690Z
Learning: Applies to src/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
src/ai_company/memory/backends/mem0/config.py
📚 Learning: 2026-03-13T06:54:28.690Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.690Z
Learning: Applies to **/*.py : Use `except A, B:` (no parentheses) for exception handling syntax per PEP 758 — ruff enforces this on Python 3.14
Applied to files:
src/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/backends/mem0/mappers.py
📚 Learning: 2026-03-13T06:54:28.690Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.690Z
Learning: Applies to src/**/*.py : Use event name constants from `ai_company.observability.events` domain-specific modules (e.g. `PROVIDER_CALL_START` from `events.provider`, `BUDGET_RECORD_ADDED` from `events.budget`, etc.) instead of hardcoded string literals
Applied to files:
src/ai_company/observability/events/memory.py
📚 Learning: 2026-03-13T06:54:28.690Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.690Z
Learning: Applies to scripts/**/*.py : Scripts: `scripts/` directory with relaxed ruff rules allowing `print` and deferred imports
Applied to files:
pyproject.toml
📚 Learning: 2026-03-13T06:54:28.690Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.690Z
Learning: Fix everything valid when review agents find issues (including pre-existing issues in surrounding code, suggestions, and findings adjacent to the PR's changes)—never skip or defer
Applied to files:
.github/workflows/dependency-review.yml
📚 Learning: 2026-03-13T06:54:28.690Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.690Z
Learning: Applies to src/**/*.py : All provider calls go through `BaseCompletionProvider` which applies retry + rate limiting automatically. Never implement retry logic in driver subclasses or calling code—it's handled by the base class.
Applied to files:
tests/unit/providers/test_protocol.py
🧬 Code graph analysis (35)
tests/unit/communication/conflict_resolution/test_helpers.py (1)
src/ai_company/communication/delegation/hierarchy.py (1)
HierarchyResolver(16-277)
tests/unit/engine/test_agent_engine.py (1)
src/ai_company/core/agent.py (1)
AgentIdentity(266-342)
tests/unit/engine/task_engine_helpers.py (1)
src/ai_company/core/task.py (1)
Task(45-261)
tests/unit/hr/test_full_snapshot_strategy.py (1)
src/ai_company/memory/consolidation/models.py (1)
ArchivalEntry(56-91)
tests/unit/tools/sandbox/test_protocol.py (3)
src/ai_company/tools/sandbox/protocol.py (1)
SandboxBackend(17-75)src/ai_company/tools/sandbox/result.py (1)
SandboxResult(6-28)tests/unit/tools/sandbox/conftest.py (1)
subprocess_sandbox(24-32)
tests/unit/hr/test_registry.py (3)
tests/unit/hr/promotion/conftest.py (1)
registry(98-100)tests/unit/hr/conftest.py (1)
registry(174-176)src/ai_company/hr/registry.py (1)
AgentRegistryService(32-262)
tests/unit/engine/test_agent_engine_lifecycle.py (3)
src/ai_company/core/agent.py (1)
AgentIdentity(266-342)src/ai_company/core/enums.py (1)
TaskStatus(198-224)src/ai_company/core/task.py (1)
Task(45-261)
tests/unit/api/conftest.py (7)
src/ai_company/budget/cost_record.py (1)
CostRecord(16-57)src/ai_company/communication/channel.py (1)
Channel(14-39)src/ai_company/communication/message.py (1)
Message(88-138)src/ai_company/hr/enums.py (1)
LifecycleEventType(32-41)src/ai_company/persistence/errors.py (2)
DuplicateRecordError(29-30)QueryError(33-34)src/ai_company/security/models.py (1)
AuditEntry(112-149)src/ai_company/security/timeout/parked_context.py (1)
ParkedContext(19-64)
tests/unit/budget/test_category_analytics.py (1)
src/ai_company/budget/tracker.py (1)
CostTracker(68-455)
tests/unit/engine/test_loop_protocol.py (1)
src/ai_company/engine/context.py (1)
AgentContext(87-307)
tests/unit/communication/meeting/test_orchestrator.py (1)
src/ai_company/communication/meeting/protocol.py (1)
MeetingProtocol(61-100)
tests/unit/hr/test_onboarding_service.py (3)
tests/unit/hr/conftest.py (2)
onboarding_service(180-182)registry(174-176)src/ai_company/hr/onboarding_service.py (1)
OnboardingService(28-173)src/ai_company/hr/registry.py (1)
AgentRegistryService(32-262)
tests/unit/memory/backends/mem0/test_adapter_crud.py (1)
src/ai_company/memory/backends/mem0/adapter.py (5)
store(315-372)retrieve(374-435)get(437-510)delete(512-598)count(600-683)
src/ai_company/memory/backends/mem0/config.py (1)
src/ai_company/memory/config.py (1)
CompanyMemoryConfig(139-195)
tests/unit/engine/test_plan_execute_loop.py (1)
src/ai_company/core/agent.py (1)
AgentIdentity(266-342)
tests/unit/communication/conflict_resolution/test_debate_strategy.py (1)
src/ai_company/communication/delegation/hierarchy.py (1)
HierarchyResolver(16-277)
tests/unit/communication/conflict_resolution/test_authority_strategy.py (1)
src/ai_company/communication/delegation/hierarchy.py (1)
HierarchyResolver(16-277)
tests/unit/engine/test_agent_engine_errors.py (3)
src/ai_company/core/agent.py (1)
AgentIdentity(266-342)src/ai_company/core/enums.py (1)
TaskStatus(198-224)src/ai_company/core/task.py (1)
Task(45-261)
src/ai_company/memory/backends/mem0/mappers.py (4)
src/ai_company/core/enums.py (1)
MemoryCategory(101-108)src/ai_company/memory/errors.py (2)
MemoryRetrievalError(25-26)MemoryStoreError(21-22)src/ai_company/memory/models.py (4)
MemoryEntry(82-150)MemoryMetadata(20-52)MemoryQuery(153-230)MemoryStoreRequest(55-79)src/ai_company/memory/backends/mem0/adapter.py (1)
get(437-510)
tests/unit/api/controllers/test_agents.py (1)
src/ai_company/api/auth/service.py (1)
AuthService(36-245)
tests/unit/memory/backends/mem0/conftest.py (5)
src/ai_company/core/enums.py (1)
MemoryCategory(101-108)src/ai_company/memory/backends/mem0/adapter.py (1)
max_memories_per_agent(278-280)src/ai_company/memory/backends/mem0/config.py (2)
Mem0BackendConfig(52-91)Mem0EmbedderConfig(23-49)src/ai_company/memory/models.py (1)
MemoryStoreRequest(55-79)src/ai_company/tools/base.py (1)
category(128-130)
tests/unit/engine/test_react_loop.py (1)
src/ai_company/core/agent.py (1)
AgentIdentity(266-342)
tests/unit/memory/backends/mem0/test_adapter.py (8)
src/ai_company/core/enums.py (1)
MemoryCategory(101-108)src/ai_company/memory/backends/mem0/config.py (1)
Mem0BackendConfig(52-91)src/ai_company/memory/capabilities.py (1)
MemoryCapabilities(13-56)src/ai_company/memory/errors.py (2)
MemoryConnectionError(17-18)MemoryError(13-14)src/ai_company/memory/models.py (1)
MemoryQuery(153-230)src/ai_company/memory/protocol.py (1)
MemoryBackend(20-182)src/ai_company/memory/shared.py (1)
SharedKnowledgeStore(18-82)tests/unit/memory/backends/mem0/conftest.py (2)
make_store_request(99-105)backend(41-49)
tests/unit/engine/test_context.py (3)
src/ai_company/core/agent.py (1)
AgentIdentity(266-342)src/ai_company/core/enums.py (1)
TaskStatus(198-224)src/ai_company/core/task.py (1)
Task(45-261)
tests/integration/communication/test_meeting_integration.py (1)
src/ai_company/communication/meeting/protocol.py (1)
MeetingProtocol(61-100)
tests/unit/communication/meeting/test_round_robin.py (1)
src/ai_company/communication/meeting/models.py (1)
MeetingAgenda(77-96)
tests/unit/communication/meeting/test_structured_phases.py (1)
src/ai_company/communication/meeting/models.py (1)
MeetingAgenda(77-96)
tests/unit/communication/conflict_resolution/test_hybrid_strategy.py (1)
src/ai_company/communication/delegation/hierarchy.py (1)
HierarchyResolver(16-277)
src/ai_company/memory/factory.py (5)
src/ai_company/memory/config.py (1)
CompanyMemoryConfig(139-195)src/ai_company/memory/backends/mem0/config.py (1)
Mem0EmbedderConfig(23-49)src/ai_company/memory/errors.py (1)
MemoryConfigError(40-41)src/ai_company/memory/protocol.py (1)
MemoryBackend(20-182)src/ai_company/memory/backends/mem0/adapter.py (2)
Mem0MemoryBackend(116-916)max_memories_per_agent(278-280)
tests/unit/hr/test_offboarding_service.py (4)
src/ai_company/communication/channel.py (1)
Channel(14-39)src/ai_company/communication/message.py (1)
Message(88-138)src/ai_company/core/enums.py (2)
AgentStatus(67-73)TaskStatus(198-224)src/ai_company/hr/registry.py (1)
AgentRegistryService(32-262)
tests/unit/communication/conflict_resolution/test_service.py (1)
src/ai_company/communication/delegation/hierarchy.py (1)
HierarchyResolver(16-277)
tests/unit/engine/test_metrics.py (1)
src/ai_company/engine/context.py (1)
AgentContext(87-307)
tests/unit/memory/backends/mem0/test_adapter_shared.py (3)
src/ai_company/memory/errors.py (3)
MemoryRetrievalError(25-26)MemoryStoreError(21-22)MemoryError(13-14)src/ai_company/memory/models.py (1)
MemoryQuery(153-230)tests/unit/memory/backends/mem0/conftest.py (4)
make_store_request(99-105)mem0_add_result(52-62)mem0_search_result(65-82)backend(41-49)
src/ai_company/memory/__init__.py (2)
src/ai_company/memory/backends/mem0/config.py (1)
Mem0EmbedderConfig(23-49)src/ai_company/memory/backends/mem0/adapter.py (1)
Mem0MemoryBackend(116-916)
tests/unit/communication/meeting/test_position_papers.py (1)
src/ai_company/communication/meeting/models.py (1)
MeetingAgenda(77-96)
🔇 Additional comments (63)
tests/unit/api/controllers/test_projects.py (1)
6-6: Import cleanup looks good.No functional impact here; the import remains correct and consistent with the type annotations used in tests.
tests/unit/api/controllers/test_artifacts.py (1)
6-6: Import cleanup looks good.Removing the inline suppression on
TestClientkeeps the test import clean without changing behavior.tests/unit/engine/task_engine_helpers.py (1)
6-6: Import cleanup looks good.Line 6 keeps the concrete
Taskimport while removing the inline suppression, with no behavioral change.tests/unit/engine/test_agent_engine.py (1)
12-12: Safe lint-cleanup import change.Line 12 is correct as updated; removing the inline suppression does not affect runtime behavior and keeps the test module clean.
tests/unit/tools/git/test_git_sandbox_integration.py (1)
3-3: Import cleanup looks good.Removing the inline suppression here is safe and does not change behavior.
tests/unit/budget/test_category_analytics.py (1)
15-15: Import cleanup looks good.This change is safe and does not alter test behavior.
tests/unit/hr/test_onboarding_service.py (1)
8-9: Import cleanup looks good.Removing the inline suppression comments on Line 8 and Line 9 is a no-op behaviorally and keeps the test module cleaner.
tests/unit/api/controllers/test_messages.py (1)
6-6: Import cleanup looks good.Removing the inline suppression here keeps the test module cleaner without changing behavior.
tests/unit/hr/test_offboarding_service.py (1)
8-11: Import cleanup looks good.These import updates are non-functional and keep the test module clear without introducing regressions.
Also applies to: 22-22
tests/unit/engine/test_agent_engine_lifecycle.py (1)
9-11: Import cleanup looks good.Removing the unnecessary
# noqa: TC001suppressions here is a safe, non-functional cleanup and improves test-file hygiene.tests/unit/providers/test_protocol.py (1)
3-3: Looks good — safe lint cleanup.Removing the unnecessary inline
# noqaon theAsyncIteratorimport is a clean, non-behavioral change.tests/unit/engine/test_plan_execute_loop.py (1)
9-9: Good cleanup: dropping unnecessary noqa suppression.
AgentIdentityis used in this test module’s type annotations, so keeping the direct import without inline suppression is appropriate.tests/unit/providers/conftest.py (1)
3-3: Import cleanup looks good.Line 3 keeps a valid and actively used
AsyncIteratorimport; dropping the inline suppression is safe and improves clarity.tests/unit/engine/test_agent_engine_errors.py (1)
8-10: Import cleanup looks good.No functional risk here; this is a clean non-behavioral change.
tests/unit/communication/conflict_resolution/test_debate_strategy.py (1)
14-16: Good cleanup: safe removal of inline lint suppression.This import change is non-functional and keeps type intent explicit without affecting test behavior.
tests/unit/api/controllers/test_autonomy.py (1)
6-6: LGTM!Clean removal of the
# noqa: TC002suppression comment. The import is valid and the type-checking issue that previously required suppression appears to be resolved.tests/unit/api/controllers/test_providers.py (1)
6-6: LGTM: Lint cleanup correctly applied.The removal of the
# noqa: TC002suppression comment indicates the type-checking lint issue has been resolved or is no longer applicable.tests/unit/hr/test_registry.py (1)
7-7: LGTM!Removing the inline
noqacomment is a clean lint cleanup. The import and its usage remain correct.tests/unit/api/test_middleware.py (1)
6-6: LGTM!Clean removal of the
noqacomment. The import is straightforward and the tests properly use@pytest.mark.unitmarkers and@pytest.mark.parametrizeas expected.tests/unit/hr/test_hiring_service.py (1)
15-16: LGTM! Lint suppression removal is correct.Both
OnboardingServiceandAgentRegistryServiceare used at runtime in the tests, so removing the# noqa: TC001suppressions is appropriate—these are not type-checking-only imports.tests/integration/tools/test_sandbox_integration.py (1)
4-4: Good cleanup: removing the inlinenoqais safe here.This change keeps behavior unchanged and improves lint hygiene.
tests/unit/tools/sandbox/conftest.py (1)
3-3: LGTM! Correctly removed unnecessary lint suppression.The
# noqa: TC003comment was unnecessary sincePathis used at runtime in type annotations (lines 12, 25), not just for static type checking. Removing this suppression improves code hygiene.tests/unit/tools/git/conftest.py (1)
5-5: LGTM! Correct removal of unnecessary linter suppression.The
# noqa: TC003suppression was unnecessary becausePathis used at runtime (e.g., path operations on lines 67, 84, 85, 101, 105), not just in type annotations. This cleanup improves code clarity.tests/unit/tools/sandbox/test_protocol.py (1)
3-4: LGTM! Clean import formatting.The removal of inline noqa comments from these import statements improves code readability without affecting functionality. All imports are properly used throughout the test file.
Also applies to: 12-12
tests/unit/api/controllers/test_company.py (1)
6-6: LGTM: Lint cleanup aligns with project standards.Removing the
# noqa: TC002suppression is appropriate sinceTestClientis used at runtime in the test methods (not just for type checking), making the suppression unnecessary.tests/unit/engine/test_context.py (1)
9-11: LGTM!The removal of
# noqa: TC001comments is appropriate cleanup. These imports (AgentIdentityandTask) are used at runtime for fixture type annotations and test assertions, so they correctly belong outside aTYPE_CHECKINGblock. Test markers follow coding guidelines.tests/unit/api/controllers/test_tasks.py (1)
6-6: LGTM: Lint directive cleanupThe removal of the
# noqa: TC002directive is appropriate. The TestClient import is used in runtime type annotations for test fixtures throughout the file and appears to be correctly imported without suppression.tests/unit/engine/test_react_loop.py (1)
8-8: LGTM! Valid cleanup for Python 3.14.Removing the
noqa: TC001suppression is appropriate. With Python 3.14's PEP 649 native lazy annotations, imports used only in type annotations no longer require special handling or suppressions. As per coding guidelines, the project relies on native lazy annotations rather thanfrom __future__ import annotations, making this suppression unnecessary.tests/unit/engine/test_routing_models.py (1)
5-5: LGTM.
AgentIdentityis still referenced by the test annotations in this file, so this import cleanup does not introduce any local issue.tests/unit/communication/conflict_resolution/test_helpers.py (1)
12-12: Import cleanup is safe.Line 12 keeps the same dependency and removes an unnecessary suppression without changing test behavior.
tests/unit/engine/test_metrics.py (1)
6-6: No concerns with this import cleanup.Line 6 is a non-functional lint/style adjustment and is safe.
tests/unit/api/test_app.py (1)
7-7: Looks good.Line 7 is a safe import-comment cleanup with no behavior impact.
tests/unit/communication/meeting/test_orchestrator.py (1)
31-31: Import change is clean and non-functional.Line 31 keeps typing clarity and does not alter orchestrator test behavior.
tests/unit/engine/test_task_engine_mutations.py (1)
8-8: Safe import cleanup.Line 8 is a stylistic adjustment only and does not change test execution.
tests/unit/hr/test_full_snapshot_strategy.py (1)
12-12: No issues here.Line 12 is a non-functional import tidy-up and is safe.
docs/roadmap/index.md (1)
11-11: Roadmap update is aligned with the PR scope.Line 11 accurately reflects the implemented pluggable memory backend and Mem0 adapter status.
tests/integration/tools/conftest.py (1)
5-5: Looks good.Line 5 is a harmless import cleanup, and
Pathremains correctly used by the fixture helpers.tests/unit/communication/conflict_resolution/test_authority_strategy.py (1)
11-12: LGTM!Lint directive cleanup - removing the
noqa: TC001comment while preserving the import. TheHierarchyResolvertype is correctly used for fixture type hints throughout the test class.tests/unit/communication/meeting/conftest.py (1)
16-16: LGTM!Lint directive removal. The
AgentCallerprotocol type is correctly used as a return type annotation formake_mock_agent_caller.tests/unit/communication/meeting/test_structured_phases.py (1)
13-13: LGTM!Lint directive cleanup.
MeetingAgendais used as a fixture type hint throughout the test methods.tests/unit/communication/meeting/test_position_papers.py (1)
13-13: LGTM!Lint directive cleanup.
MeetingAgendais used as a fixture type hint throughout the test methods.tests/unit/api/test_health.py (1)
6-6: LGTM!Lint directive cleanup for the
TestClientimport from litestar.tests/unit/api/controllers/test_analytics.py (1)
6-6: LGTM!Lint directive cleanup for the
TestClientimport.tests/unit/api/controllers/test_approvals.py (1)
7-9: LGTM!Lint directive cleanup for
TestClientandApprovalStoreimports. Both types are used for fixture type hints throughout the test file.tests/unit/api/conftest.py (1)
5-39: LGTM!Comprehensive lint directive cleanup across multiple imports. All types (
Generator,CostRecord,Channel,Message,LifecycleEventType,AgentLifecycleEvent,TaskMetricRecord,CollaborationMetricRecord,AuditEntry,AuditVerdictStr,ParkedContext) are used at runtime in the fake repository implementations and fixtures.tests/unit/communication/meeting/test_round_robin.py (1)
13-13: Import cleanup is safe and keeps test behavior unchanged.tests/unit/api/auth/test_controller.py (1)
6-7: Non-functional import cleanup looks good.Also applies to: 47-47
.github/workflows/dependency-review.yml (1)
44-51: Version-pinned dependency license exceptions are a solid compliance hardening step.src/ai_company/observability/events/memory.py (1)
22-24: New memory backend event constants are consistent with the event namespace and usage pattern.tests/integration/communication/test_meeting_integration.py (1)
29-30: Import cleanup is safe; no behavioral impact on integration tests.src/ai_company/memory/backends/mem0/config.py (1)
75-91: Traversal validation and structured warning logging are implemented correctly.tests/unit/memory/backends/mem0/test_adapter.py (1)
135-253: Lifecycle and exception-propagation coverage here is strong and directly valuable.tests/unit/api/test_guards.py (1)
6-6: Import-only cleanup is good and behavior-preserving.pyproject.toml (2)
30-31: LGTM — optional Mem0 dependency correctly configured.The
mem0ai==1.0.5version is properly pinned per project conventions, and placing it in[project.optional-dependencies]keeps it separate from core dependencies.
191-193: LGTM — mypy override for untyped optional dependency.Standard practice for optional dependencies without type stubs.
docs/design/memory.md (1)
286-292: LGTM — clear documentation of embedder configuration pattern.The example correctly demonstrates programmatic embedder configuration via the factory, with vendor-agnostic placeholders.
tests/unit/memory/backends/mem0/test_adapter_crud.py (1)
1-515: LGTM — comprehensive CRUD test coverage.Excellent test coverage including:
- Success paths for all operations
- Edge cases (empty results, blank/whitespace IDs, non-list results)
- Error wrapping verification
builtins.MemoryErrorre-raise behavior- Ownership and shared namespace validation
The tests properly use fixtures and follow project conventions.
tests/unit/memory/backends/mem0/conftest.py (1)
40-49: LGTM — test fixture correctly wires mock client.Direct assignment to
_clientand_connectedis the appropriate pattern for unit testing with mocked dependencies.src/ai_company/memory/factory.py (1)
94-103: Exceptions fromMem0MemoryBackend(...)instantiation bypass config validation logging.The
ValueErrorfrombuild_config_from_company_configis properly wrapped, but ifMem0MemoryBackend.__init__raises (e.g., validation in a future version), it escapes withoutMEMORY_BACKEND_CONFIG_INVALIDlogging. Consider wrapping lines 94-97 similarly:🛡️ Suggested defensive wrapper
+ try: backend = Mem0MemoryBackend( mem0_config=mem0_config, max_memories_per_agent=config.options.max_memories_per_agent, ) + except Exception as exc: + msg = f"Failed to create Mem0 backend: {exc}" + logger.warning( + MEMORY_BACKEND_CONFIG_INVALID, + backend="mem0", + reason="backend_instantiation_failed", + error=msg, + ) + raise MemoryConfigError(msg) from exc logger.info( MEMORY_BACKEND_CREATED,tests/unit/memory/backends/mem0/test_adapter_shared.py (1)
1-327: LGTM — thorough shared knowledge store test coverage.Comprehensive tests covering:
- Publisher metadata injection and extraction
- Shared namespace routing
- Agent exclusion filtering
- Ownership verification for retract
- Error wrapping and
builtins.MemoryErrorpassthroughsrc/ai_company/memory/backends/mem0/adapter.py (3)
179-197:disconnect()may not fully release resources.The past review flagged that the Mem0
Memoryclient holds an internalhttpx.Client. Callingreset()clears stored memories but doesn't close the HTTP connection pool. Ifreset()fails, the exception is silently logged at DEBUG.Consider explicitly closing the underlying client:
🔧 Suggested fix
async def disconnect(self) -> None: """Close the Mem0 connection.""" logger.info(MEMORY_BACKEND_DISCONNECTING, backend="mem0") if self._client is not None: try: - await asyncio.to_thread(self._client.reset) - except Exception: - logger.debug( + # Close the underlying httpx client to release connections + if hasattr(self._client, "client") and self._client.client is not None: + await asyncio.to_thread(self._client.client.close) + except Exception as exc: + logger.warning( MEMORY_BACKEND_DISCONNECTING, backend="mem0", - note="reset failed during disconnect, ignoring", + note="client close failed during disconnect", + error=str(exc), ) self._client = None self._connected = False logger.info(MEMORY_BACKEND_DISCONNECTED, backend="mem0")Does mem0ai Memory client have a close method or expose httpx client for cleanup?
164-165: LGTM — consistent exception handling pattern.The
except builtins.MemoryError, RecursionError:syntax correctly follows PEP 758 and ensures system-level failures propagate without masking. Based on learnings: "Useexcept A, B:(no parentheses) for exception handling syntax per PEP 758 — ruff enforces this on Python 3.14".Also applies to: 222-223, 354-355, 418-419, 491-492, 579-580, 653-654, 732-733, 810-811, 897-898
116-136: LGTM — well-designed class initialization.Clear separation of configuration (
_mem0_config), limits (_max_memories_per_agent), and runtime state (_client,_connected). The class properly implements all three protocols.
| with contextlib.suppress(ImportError): # mem0ai is optional | ||
| from ai_company.memory.backends.mem0 import ( | ||
| Mem0EmbedderConfig, | ||
| Mem0MemoryBackend, | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Conditionally-defined exports in __all__ may cause runtime NameError.
When mem0ai is not installed, Mem0EmbedderConfig and Mem0MemoryBackend are not defined, but they're still listed in __all__. This means:
from ai_company.memory import Mem0EmbedderConfigraisesImportError(expected)from ai_company.memory import *may behave unexpectedly- IDE autocompletion will suggest these names even when unavailable
Consider conditionally extending __all__ or documenting this behavior explicitly in the module docstring.
💡 Optional: Conditionally extend __all__
+_MEM0_EXPORTS: list[str] = []
with contextlib.suppress(ImportError): # mem0ai is optional
from ai_company.memory.backends.mem0 import (
Mem0EmbedderConfig,
Mem0MemoryBackend,
)
+ _MEM0_EXPORTS = ["Mem0EmbedderConfig", "Mem0MemoryBackend"]
__all__ = [
"ArchivalStore",
# ... other exports ...
- "Mem0EmbedderConfig",
- "Mem0MemoryBackend",
# ... other exports ...
-]
+] + _MEM0_EXPORTSAlso applies to: 85-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/memory/__init__.py` around lines 14 - 18, The __all__ list
currently names Mem0EmbedderConfig and Mem0MemoryBackend even when mem0ai isn't
installed, causing NameError/autocomplete issues; fix by only adding those
symbols to __all__ inside the same contextlib.suppress(ImportError) block (or by
using a try/except ImportError around the from import and appending the names to
__all__ on success), so ensure __all__ is conditionally extended when
Mem0EmbedderConfig and Mem0MemoryBackend are actually defined.
| def mem0_get_result(memory_id: str = "mem-001") -> dict[str, Any]: | ||
| """Build a typical Mem0 get() return value.""" | ||
| return { | ||
| "id": memory_id, | ||
| "memory": "stored content", | ||
| "created_at": "2026-03-12T10:00:00+00:00", | ||
| "updated_at": None, | ||
| "metadata": { | ||
| "_synthorg_category": "episodic", | ||
| "_synthorg_confidence": 1.0, | ||
| }, | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding optional user_id parameter to mem0_get_result.
Several tests (e.g., test_get_ownership_mismatch_returns_none, test_delete_shared_namespace_entry_raises) need to modify the user_id field after calling this helper. Adding an optional parameter would reduce test boilerplate.
💡 Optional improvement
-def mem0_get_result(memory_id: str = "mem-001") -> dict[str, Any]:
+def mem0_get_result(
+ memory_id: str = "mem-001",
+ *,
+ user_id: str | None = None,
+) -> dict[str, Any]:
"""Build a typical Mem0 get() return value."""
return {
"id": memory_id,
"memory": "stored content",
"created_at": "2026-03-12T10:00:00+00:00",
"updated_at": None,
+ "user_id": user_id,
"metadata": {
"_synthorg_category": "episodic",
"_synthorg_confidence": 1.0,
},
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/memory/backends/mem0/conftest.py` around lines 85 - 96, The helper
mem0_get_result should accept an optional user_id so tests don't need to mutate
the result; update the signature of mem0_get_result to add a parameter like
user_id: Optional[str] = None and include the "user_id" key in the returned dict
(set to the passed user_id, possibly None by default) so callers can produce
ownership-specific results directly; keep the existing memory_id parameter and
return structure otherwise unchanged and ensure typing imports remain valid for
Any/Optional.
mem0ai is the only memory backend — no reason to make it optional. Reverts the optional-dependencies split and CI extra flag.
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ai_company/memory/__init__.py`:
- Around line 6-9: The module docstring in __init__.py lists only
Mem0MemoryBackend but the module also re-exports Mem0EmbedderConfig (and
possibly other public symbols); update the top-level summary to include
Mem0EmbedderConfig (and ensure any other names exported in the file are
reflected) so the public-surface description matches the actual re-exports
(search for Mem0MemoryBackend, Mem0EmbedderConfig and the module
__all__/re-export lines to verify and update the docstring).
- Around line 12-15: The top-level module is re-exporting vendor-specific
symbols Mem0EmbedderConfig and Mem0MemoryBackend; change the top-level API to
expose generic aliases (e.g., ExampleEmbedderConfig or EmbedderConfig, and
ExampleMemoryBackend or MemoryBackend) instead of vendor names, and leave the
original Mem0EmbedderConfig and Mem0MemoryBackend defined and exported only from
ai_company.memory.backends.mem0; update the __all__/exports in __init__.py to
use the generic alias names and adjust any import sites/docs/tests to import the
generic names (or import vendor-specific types directly from
ai_company.memory.backends.mem0 when needed).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f6200036-48c7-4c01-a277-4131798e1c61
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
pyproject.tomlsrc/ai_company/memory/__init__.py
📜 Review details
⏰ 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). (3)
- GitHub Check: Agent
- GitHub Check: Greptile Review
- GitHub Check: Test (Python 3.14)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Do NOT usefrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:(no parentheses) for exception handling syntax per PEP 758 — ruff enforces this on Python 3.14
Files:
src/ai_company/memory/__init__.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Type hints: all public functions must have type hints, mypy strict mode
Docstrings: Google style, required on public classes/functions (enforced by ruff D rules)
Create new objects instead of mutating existing ones—never mutate existing objects. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement.
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention and usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Config vs runtime state: use frozen Pydantic models for config/identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict). Use@computed_fieldfor derived values instead of storing + validating redundant fields. UseNotBlankStr(fromcore.types) for all identifier/name fields—including optional and tuple variants—instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task.
Line length: 88 characters (enforced by ruff)
Functions: < 50 lines, files < 800 lines
Handle errors explicitly, never silently swallow them
Validate at system boundaries (user input, external APIs, config files)
Every module with business logic MUST have:from ai_company.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code
Always useloggeras the variable name (not_logger, notlog)
Use event name constants fromai_company.observability.eventsdomain-spe...
Files:
src/ai_company/memory/__init__.py
pyproject.toml
📄 CodeRabbit inference engine (CLAUDE.md)
pyproject.toml: Useasyncio_mode = "auto"in pytest configuration—no manual@pytest.mark.asyncioneeded on individual tests
Test timeout: 30 seconds per test
Dependencies: all versions pinned with==inpyproject.toml. Groups:testanddev(includes test). Install viauv sync.
Files:
pyproject.toml
🧠 Learnings (1)
📚 Learning: 2026-03-13T06:54:28.690Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.690Z
Learning: Applies to scripts/**/*.py : Scripts: `scripts/` directory with relaxed ruff rules allowing `print` and deferred imports
Applied to files:
pyproject.toml
🧬 Code graph analysis (1)
src/ai_company/memory/__init__.py (2)
src/ai_company/memory/backends/mem0/config.py (1)
Mem0EmbedderConfig(23-49)src/ai_company/memory/backends/mem0/adapter.py (1)
Mem0MemoryBackend(116-916)
🔇 Additional comments (4)
pyproject.toml (3)
138-138: LGTM!Adding
"TC"(flake8-type-checking) to test ignores is appropriate—test files commonly import types at runtime for fixture annotations and parametrized tests, making the type-checking-only import rule overly strict in this context.
189-192: LGTM!The mypy override for
mem0.*follows the established pattern for third-party libraries without type stubs (litellm, jsonschema, aiodocker, etc.). This is the standard approach to suppress import errors for untyped dependencies.
24-24: mem0ai==1.0.5 is valid and properly pinned.Version 1.0.5 exists on PyPI and is the latest stable release in the 1.0.x series. The dependency is correctly pinned with
==per project guidelines. Security advisories could not be verified via automated channels, but the version specification itself is sound.src/ai_company/memory/__init__.py (1)
12-15: Confirmmem0aiis mandatory before keeping unconditional import.Line 12 imports Mem0 symbols at module import time. If
mem0aiis still optional in packaging, importingai_company.memorywill fail at runtime.#!/bin/bash set -euo pipefail echo "== Dependency manifests ==" fd -HI 'pyproject\.toml|setup\.cfg|setup\.py|requirements.*\.txt|uv\.lock|poetry\.lock' echo echo "== mem0 declarations across manifests ==" rg -n -C2 --glob 'pyproject.toml' --glob 'setup.cfg' --glob 'setup.py' \ --glob 'requirements*.txt' --glob 'uv.lock' --glob 'poetry.lock' \ 'mem0ai|mem0[^a-zA-Z0-9_]' echo echo "== Import-guard patterns in memory init ==" rg -n -C2 'from ai_company.memory.backends.mem0|contextlib\.suppress\(ImportError\)|except ImportError' src/ai_company/memory/__init__.py
| concrete backends (``Mem0MemoryBackend``), domain models, config | ||
| models, factory, retrieval pipeline, consolidation, org memory, and | ||
| error hierarchy so consumers can import from ``ai_company.memory`` | ||
| directly. |
There was a problem hiding this comment.
Docstring export list is incomplete.
Line 6 mentions only Mem0MemoryBackend, but Line 13 also re-exports Mem0EmbedderConfig; the public-surface summary is now out of sync.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/memory/__init__.py` around lines 6 - 9, The module docstring
in __init__.py lists only Mem0MemoryBackend but the module also re-exports
Mem0EmbedderConfig (and possibly other public symbols); update the top-level
summary to include Mem0EmbedderConfig (and ensure any other names exported in
the file are reflected) so the public-surface description matches the actual
re-exports (search for Mem0MemoryBackend, Mem0EmbedderConfig and the module
__all__/re-export lines to verify and update the docstring).
| from ai_company.memory.backends.mem0 import ( | ||
| Mem0EmbedderConfig, | ||
| Mem0MemoryBackend, | ||
| ) |
There was a problem hiding this comment.
Vendor-specific names in top-level API violate project naming policy.
Re-exporting Mem0EmbedderConfig and Mem0MemoryBackend from ai_company.memory introduces vendor-specific naming into project-owned API on Line 12 and Line 82. Consider exposing generic aliases at the top-level and keeping vendor-specific types under backend-specific modules.
As per coding guidelines, "Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names: example-provider, example-large-001, example-medium-001, example-small-001, large/medium/small as aliases."
Also applies to: 82-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/memory/__init__.py` around lines 12 - 15, The top-level module
is re-exporting vendor-specific symbols Mem0EmbedderConfig and
Mem0MemoryBackend; change the top-level API to expose generic aliases (e.g.,
ExampleEmbedderConfig or EmbedderConfig, and ExampleMemoryBackend or
MemoryBackend) instead of vendor names, and leave the original
Mem0EmbedderConfig and Mem0MemoryBackend defined and exported only from
ai_company.memory.backends.mem0; update the __all__/exports in __init__.py to
use the generic alias names and adjust any import sites/docs/tests to import the
generic names (or import vendor-specific types directly from
ai_company.memory.backends.mem0 when needed).
|
|
||
| publisher = extract_publisher(raw) | ||
| if publisher is None: | ||
| logger.warning( | ||
| MEMORY_SHARED_RETRACT_FAILED, | ||
| agent_id=agent_id, | ||
| memory_id=memory_id, | ||
| reason="not a shared memory entry (no publisher)", | ||
| ) | ||
| msg = ( | ||
| f"Memory {memory_id} is not a shared memory entry " | ||
| f"(no publisher metadata)" | ||
| ) | ||
| raise MemoryStoreError(msg) # noqa: TRY301 | ||
|
|
||
| if publisher != str(agent_id): | ||
| logger.warning( | ||
| MEMORY_SHARED_RETRACT_FAILED, | ||
| agent_id=agent_id, | ||
| memory_id=memory_id, | ||
| reason="ownership mismatch", | ||
| publisher=publisher, | ||
| ) | ||
| msg = ( | ||
| f"Agent {agent_id} cannot retract memory " | ||
| f"{memory_id} published by {publisher}" | ||
| ) | ||
| raise MemoryStoreError(msg) # noqa: TRY301 |
There was a problem hiding this comment.
retract doesn't verify the memory is in the shared namespace
retract relies solely on the presence of _synthorg_publisher in the metadata to identify shared entries, but never checks raw.get("user_id") == _SHARED_NAMESPACE. This means any memory — including private agent memories — that happens to carry the publisher key can be deleted through retract, bypassing the ownership check enforced by delete.
In practice, Mem0's add response strips unrecognized metadata keys in some SDK versions and the publisher key is only written by publish, so the risk is low today. However, as the system evolves (e.g. a future code path serialises full metadata into a raw Mem0 record, or a Mem0 SDK update preserves metadata differently), a private entry could surface with this key and be silently deleted via retract.
Adding a namespace check before the publisher check closes this gap and makes the invariant explicit:
# Verify the entry is actually in the shared namespace
if raw.get("user_id") != _SHARED_NAMESPACE:
msg = f"Memory {memory_id} is not in the shared namespace"
logger.warning(
MEMORY_SHARED_RETRACT_FAILED,
agent_id=agent_id,
memory_id=memory_id,
reason="not a shared-namespace entry",
)
raise MemoryStoreError(msg)
publisher = extract_publisher(raw)
...Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ai_company/memory/backends/mem0/adapter.py
Line: 862-889
Comment:
**`retract` doesn't verify the memory is in the shared namespace**
`retract` relies solely on the presence of `_synthorg_publisher` in the metadata to identify shared entries, but never checks `raw.get("user_id") == _SHARED_NAMESPACE`. This means any memory — including private agent memories — that happens to carry the publisher key can be deleted through `retract`, bypassing the ownership check enforced by `delete`.
In practice, Mem0's `add` response strips unrecognized metadata keys in some SDK versions and the publisher key is only written by `publish`, so the risk is low today. However, as the system evolves (e.g. a future code path serialises full metadata into a raw Mem0 record, or a Mem0 SDK update preserves metadata differently), a private entry could surface with this key and be silently deleted via `retract`.
Adding a namespace check before the publisher check closes this gap and makes the invariant explicit:
```python
# Verify the entry is actually in the shared namespace
if raw.get("user_id") != _SHARED_NAMESPACE:
msg = f"Memory {memory_id} is not in the shared namespace"
logger.warning(
MEMORY_SHARED_RETRACT_FAILED,
agent_id=agent_id,
memory_id=memory_id,
reason="not a shared-namespace entry",
)
raise MemoryStoreError(msg)
publisher = extract_publisher(raw)
...
```
How can I resolve this? If you propose a fix, please make it concise.| """ | ||
| self._require_connected() | ||
| try: | ||
| metadata = { | ||
| **build_mem0_metadata(request), | ||
| _PUBLISHER_KEY: str(agent_id), | ||
| } | ||
| kwargs = { | ||
| "messages": [ | ||
| {"role": "user", "content": request.content}, | ||
| ], | ||
| "user_id": _SHARED_NAMESPACE, | ||
| "metadata": metadata, | ||
| "infer": False, | ||
| } | ||
| result = await asyncio.to_thread(self._client.add, **kwargs) |
There was a problem hiding this comment.
publish skips _validate_agent_id guard
Every other method that accepts agent_id calls _validate_agent_id(agent_id) immediately after _require_connected(), but publish omits this call:
store→_require_connected+_validate_agent_id✅retrieve→_require_connected+_validate_agent_id✅get→_require_connected+_validate_agent_id✅delete→_require_connected+_validate_agent_id✅count→_require_connected+_validate_agent_id✅publish→_require_connectedonly ❌
Without this guard, passing _SHARED_NAMESPACE as agent_id to publish succeeds. The entry is stored with _PUBLISHER_KEY set to the reserved namespace string. Since retract also skips _validate_agent_id, any caller using that reserved string as agent_id can then retract those entries — polluting the publisher metadata that the rest of the access-control logic relies on.
| """ | |
| self._require_connected() | |
| try: | |
| metadata = { | |
| **build_mem0_metadata(request), | |
| _PUBLISHER_KEY: str(agent_id), | |
| } | |
| kwargs = { | |
| "messages": [ | |
| {"role": "user", "content": request.content}, | |
| ], | |
| "user_id": _SHARED_NAMESPACE, | |
| "metadata": metadata, | |
| "infer": False, | |
| } | |
| result = await asyncio.to_thread(self._client.add, **kwargs) | |
| self._require_connected() | |
| self._validate_agent_id(agent_id) | |
| try: | |
| metadata = { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ai_company/memory/backends/mem0/adapter.py
Line: 707-722
Comment:
**`publish` skips `_validate_agent_id` guard**
Every other method that accepts `agent_id` calls `_validate_agent_id(agent_id)` immediately after `_require_connected()`, but `publish` omits this call:
- `store` → `_require_connected` + `_validate_agent_id` ✅
- `retrieve` → `_require_connected` + `_validate_agent_id` ✅
- `get` → `_require_connected` + `_validate_agent_id` ✅
- `delete` → `_require_connected` + `_validate_agent_id` ✅
- `count` → `_require_connected` + `_validate_agent_id` ✅
- `publish` → `_require_connected` only ❌
Without this guard, passing `_SHARED_NAMESPACE` as `agent_id` to `publish` succeeds. The entry is stored with `_PUBLISHER_KEY` set to the reserved namespace string. Since `retract` also skips `_validate_agent_id`, any caller using that reserved string as `agent_id` can then retract those entries — polluting the publisher metadata that the rest of the access-control logic relies on.
```suggestion
self._require_connected()
self._validate_agent_id(agent_id)
try:
metadata = {
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Pull request overview
Adds a concrete Mem0-backed implementation of the ai_company.memory backend protocols, wires it into the backend factory, and updates tests/docs/dependency metadata to reflect the new backend.
Changes:
- Implement Mem0 backend (
Mem0MemoryBackend) plus Mem0-specific config + mapping utilities. - Update
create_memory_backend()to construct the Mem0 backend (requires an explicitMem0EmbedderConfig). - Add extensive unit/integration tests for the Mem0 backend and update dependencies/lockfile + docs/roadmap.
Reviewed changes
Copilot reviewed 79 out of 82 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Locks new runtime deps (mem0ai + transitive deps like qdrant-client/grpcio/numpy/etc.). |
| pyproject.toml | Adds mem0ai==1.0.5, adjusts ruff per-file ignores, adds mypy override for mem0.*. |
| src/ai_company/observability/events/memory.py | Adds new structured log event constants for config/agent-id rejection. |
| src/ai_company/memory/factory.py | Implements Mem0 backend creation path and embedder validation. |
| src/ai_company/memory/backends/mem0/adapter.py | New Mem0 backend adapter implementing lifecycle, CRUD, and shared knowledge operations. |
| src/ai_company/memory/backends/mem0/config.py | New Mem0 backend config models + builder for Mem0 SDK config dict. |
| src/ai_company/memory/backends/mem0/mappers.py | New stateless mapping helpers between domain models and Mem0 dict payloads. |
| src/ai_company/memory/backends/mem0/init.py | Exposes Mem0 backend/config types from the mem0 backend package. |
| src/ai_company/memory/backends/init.py | Introduces concrete backend package export(s). |
| src/ai_company/memory/init.py | Re-exports Mem0 backend types from ai_company.memory. |
| README.md | Updates project status to reflect Mem0 adapter as implemented. |
| docs/roadmap/index.md | Updates roadmap to remove “Mem0 adapter” from remaining work and mention it as implemented. |
| docs/design/memory.md | Updates design doc text and adds example of passing embedder config via factory. |
| CLAUDE.md | Updates repo structure description and adds a note about mem0ai dependency. |
| .github/workflows/dependency-review.yml | Allows additional licenses / missing-license metadata entries for newly introduced deps. |
| tests/unit/memory/test_init.py | Extends public export/importability expectations for new Mem0 types. |
| tests/unit/memory/test_factory.py | Updates factory tests: Mem0 now creates a backend and validates embedder requirements. |
| tests/unit/memory/backends/mem0/conftest.py | New shared fixtures/helpers for Mem0 backend unit tests. |
| tests/unit/memory/backends/mem0/test_adapter.py | New unit tests for adapter properties/capabilities/lifecycle/guards. |
| tests/unit/memory/backends/mem0/test_adapter_crud.py | New unit tests for store/retrieve/get/delete/count behavior. |
| tests/unit/memory/backends/mem0/test_adapter_shared.py | New unit tests for publish/search_shared/retract behavior. |
| tests/unit/memory/backends/mem0/test_config.py | New unit tests for Mem0 config models and config dict builder. |
| tests/unit/memory/backends/mem0/test_mappers.py | New unit tests for mapping utilities and edge cases. |
| tests/integration/memory/test_mem0_backend.py | New integration-style tests for retrieval pipeline using mocked Mem0 client. |
| tests/unit/tools/sandbox/test_protocol.py | Removes TC-related noqa comments from imports. |
| tests/unit/tools/sandbox/conftest.py | Removes TC-related noqa comments from imports. |
| tests/unit/tools/git/test_git_sandbox_integration.py | Removes TC-related noqa comments from imports. |
| tests/unit/tools/git/conftest.py | Removes TC-related noqa comments from imports. |
| tests/unit/providers/test_protocol.py | Removes TC-related noqa comments from imports. |
| tests/unit/providers/conftest.py | Removes TC-related noqa comments from imports. |
| tests/unit/hr/test_registry.py | Removes TC-related noqa comments from imports. |
| tests/unit/hr/test_onboarding_service.py | Removes TC-related noqa comments from imports. |
| tests/unit/hr/test_offboarding_service.py | Removes TC-related noqa comments from imports. |
| tests/unit/hr/test_hiring_service.py | Removes TC-related noqa comments from imports. |
| tests/unit/hr/test_full_snapshot_strategy.py | Removes TC-related noqa comments from imports. |
| tests/unit/engine/test_task_engine_mutations.py | Removes TC-related noqa comments from imports. |
| tests/unit/engine/test_routing_models.py | Removes TC-related noqa comments from imports. |
| tests/unit/engine/test_react_loop.py | Removes TC-related noqa comments from imports. |
| tests/unit/engine/test_plan_execute_loop.py | Removes TC-related noqa comments from imports. |
| tests/unit/engine/test_metrics.py | Removes TC-related noqa comments from imports. |
| tests/unit/engine/test_loop_protocol.py | Removes TC-related noqa comments from imports. |
| tests/unit/engine/test_context.py | Removes TC-related noqa comments from imports. |
| tests/unit/engine/test_agent_engine.py | Removes TC-related noqa comments from imports. |
| tests/unit/engine/test_agent_engine_lifecycle.py | Removes TC-related noqa comments from imports. |
| tests/unit/engine/test_agent_engine_errors.py | Removes TC-related noqa comments from imports. |
| tests/unit/engine/task_engine_helpers.py | Removes TC-related noqa comments from imports. |
| tests/unit/communication/meeting/test_structured_phases.py | Removes TC-related noqa comments from imports. |
| tests/unit/communication/meeting/test_round_robin.py | Removes TC-related noqa comments from imports. |
| tests/unit/communication/meeting/test_protocol.py | Removes TC-related noqa comments from imports. |
| tests/unit/communication/meeting/test_position_papers.py | Removes TC-related noqa comments from imports. |
| tests/unit/communication/meeting/test_orchestrator.py | Removes TC-related noqa comments from imports. |
| tests/unit/communication/meeting/conftest.py | Removes TC-related noqa comments from imports. |
| tests/unit/communication/conflict_resolution/test_service.py | Removes TC-related noqa comments from imports. |
| tests/unit/communication/conflict_resolution/test_hybrid_strategy.py | Removes TC-related noqa comments from imports. |
| tests/unit/communication/conflict_resolution/test_helpers.py | Removes TC-related noqa comments from imports. |
| tests/unit/communication/conflict_resolution/test_debate_strategy.py | Removes TC-related noqa comments from imports. |
| tests/unit/communication/conflict_resolution/test_authority_strategy.py | Removes TC-related noqa comments from imports. |
| tests/unit/budget/test_category_analytics.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/test_middleware.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/test_health.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/test_guards.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/test_app.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/controllers/test_tasks.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/controllers/test_providers.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/controllers/test_projects.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/controllers/test_messages.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/controllers/test_meetings.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/controllers/test_departments.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/controllers/test_company.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/controllers/test_budget.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/controllers/test_autonomy.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/controllers/test_artifacts.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/controllers/test_approvals.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/controllers/test_analytics.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/controllers/test_agents.py | Removes TC-related noqa comments from imports in an inner import. |
| tests/unit/api/conftest.py | Removes TC-related noqa comments from imports. |
| tests/unit/api/auth/test_controller.py | Removes TC-related noqa comments from imports / inner import. |
| tests/integration/tools/test_sandbox_integration.py | Removes TC-related noqa comments from imports. |
| tests/integration/tools/conftest.py | Removes TC-related noqa comments from imports. |
| tests/integration/communication/test_meeting_integration.py | Removes TC-related noqa comments from imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _validate_agent_id(self, agent_id: NotBlankStr) -> None: | ||
| """Reject the reserved shared namespace as an agent ID. | ||
|
|
||
| Raises: | ||
| MemoryStoreError: If ``agent_id`` collides with the | ||
| reserved ``_SHARED_NAMESPACE``. | ||
| """ | ||
| if str(agent_id) == _SHARED_NAMESPACE: | ||
| logger.warning( | ||
| MEMORY_BACKEND_AGENT_ID_REJECTED, | ||
| agent_id=agent_id, | ||
| reason="reserved shared namespace", | ||
| ) | ||
| msg = ( | ||
| f"agent_id must not be the reserved shared namespace: " | ||
| f"{_SHARED_NAMESPACE!r}" | ||
| ) | ||
| raise MemoryStoreError(msg) |
| try: | ||
| mem0_config = build_config_from_company_config( | ||
| config, | ||
| embedder=embedder, | ||
| ) | ||
| except ValueError as exc: | ||
| msg = f"Invalid Mem0 configuration: {exc}" | ||
| logger.warning( | ||
| MEMORY_BACKEND_CONFIG_INVALID, | ||
| backend="mem0", | ||
| reason="config_build_failed", | ||
| error=msg, | ||
| ) | ||
| raise MemoryConfigError(msg) from exc |
|
|
||
| - **Pinned**: all versions use `==` in `pyproject.toml` | ||
| - **Groups**: `test` (pytest + plugins), `dev` (includes test + ruff, mypy, pre-commit, commitizen) | ||
| - **Optional**: `mem0ai` (Mem0 memory backend — only needed when `backend: "mem0"` is configured) |
…d Gemini - CRITICAL: remove Memory.reset() from disconnect() — it wiped all data - MAJOR: wrap Mem0MemoryBackend init errors in factory as MemoryConfigError - MEDIUM: coerce string scores in normalize_relevance_score - MEDIUM: validate dict structure in validate_add_result - MEDIUM: sanitize source metadata via _coerce_source helper - MEDIUM: update CLAUDE.md — mem0ai is a required dependency, not optional - MEDIUM: extract _coerce_confidence, _coerce_source, _normalize_tags helpers - MINOR: coerce non-string values in extract_publisher - MINOR: add user_id param to mem0_get_result fixture - MINOR: document adapter.py 800-line exemption - Tests: add coverage for string scores, non-dict results, source sanitization, publisher coercion, disconnect-no-reset
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
♻️ Duplicate comments (2)
tests/unit/memory/backends/mem0/test_adapter.py (1)
267-331: 🧹 Nitpick | 🔵 TrivialConsider using
@pytest.mark.parametrizeto reduce duplication.The
TestConnectionGuardclass has 8 similar tests that each create a backend and verifyMemoryConnectionErroris raised. These could be collapsed with parametrization.♻️ Optional: Parametrize connection guard tests
`@pytest.mark.unit` class TestConnectionGuard: `@pytest.mark.parametrize`( "method,args", [ ("store", ("test-agent-001", make_store_request())), ("retrieve", ("test-agent-001", MemoryQuery(text="test"))), ("get", ("test-agent-001", "mem-001")), ("delete", ("test-agent-001", "mem-001")), ("count", ("test-agent-001",)), ("publish", ("test-agent-001", make_store_request())), ("search_shared", (MemoryQuery(text="test"),)), ("retract", ("test-agent-001", "mem-001")), ], ) async def test_raises_when_disconnected( self, mem0_config: Mem0BackendConfig, method: str, args: tuple, ) -> None: b = Mem0MemoryBackend(mem0_config=mem0_config) with pytest.raises(MemoryConnectionError, match="Not connected"): await getattr(b, method)(*args)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/memory/backends/mem0/test_adapter.py` around lines 267 - 331, Collapse the eight nearly identical tests in TestConnectionGuard into a single parametrized test using pytest.mark.parametrize: instantiate Mem0MemoryBackend once per test and call the backend method by name (use getattr on the Mem0MemoryBackend instance) with the corresponding args, asserting MemoryConnectionError with match "Not connected"; replace individual methods test_store_raises_when_disconnected, test_retrieve_raises_when_disconnected, test_get_raises_when_disconnected, test_delete_raises_when_disconnected, test_count_raises_when_disconnected, test_publish_raises_when_disconnected, test_search_shared_raises_when_disconnected, and test_retract_raises_when_disconnected with a single async test_raises_when_disconnected that parametrize tuples like ("store", ("test-agent-001", make_store_request())) and so on.src/ai_company/memory/factory.py (1)
26-113: 🧹 Nitpick | 🔵 TrivialConsider extracting the Mem0 path into a helper function.
create_memory_backendis approximately 70 lines, exceeding the 50-line guideline. The Mem0 branch (lines 48-113) could be extracted into a_create_mem0_backend(config, embedder)helper to improve readability and meet the size limit.The error handling is correct — both
build_config_from_company_configandMem0MemoryBackendfailures are properly wrapped asMemoryConfigErrorwith logging, addressing the past review comment.♻️ Optional: Extract Mem0 path to helper
+def _create_mem0_backend( + config: CompanyMemoryConfig, + embedder: Mem0EmbedderConfig, +) -> MemoryBackend: + """Create a Mem0 backend instance.""" + from ai_company.memory.backends.mem0 import Mem0MemoryBackend + from ai_company.memory.backends.mem0.config import build_config_from_company_config + + try: + mem0_config = build_config_from_company_config(config, embedder=embedder) + except ValueError as exc: + msg = f"Invalid Mem0 configuration: {exc}" + logger.warning( + MEMORY_BACKEND_CONFIG_INVALID, + backend="mem0", + reason="config_build_failed", + error=msg, + ) + raise MemoryConfigError(msg) from exc + try: + backend = Mem0MemoryBackend( + mem0_config=mem0_config, + max_memories_per_agent=config.options.max_memories_per_agent, + ) + except Exception as exc: + msg = f"Failed to create Mem0 backend: {exc}" + logger.warning( + MEMORY_BACKEND_CONFIG_INVALID, + backend="mem0", + reason="backend_init_failed", + error=msg, + ) + raise MemoryConfigError(msg) from exc + logger.info( + MEMORY_BACKEND_CREATED, + backend="mem0", + data_dir=mem0_config.data_dir, + ) + return backend🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ai_company/memory/factory.py` around lines 26 - 113, Refactor the long mem0 branch in create_memory_backend into a small helper named _create_mem0_backend(config, embedder) that performs the current logic: import Mem0MemoryBackend, Mem0EmbedderConfig, build_config_from_company_config; validate embedder presence/type and raise MemoryConfigError with the same logger.warning calls using MEMORY_BACKEND_CONFIG_INVALID; call build_config_from_company_config and wrap ValueError into MemoryConfigError with identical logging; instantiate Mem0MemoryBackend and wrap exceptions into MemoryConfigError with the same logging; log MEMORY_BACKEND_CREATED and return the backend. Update create_memory_backend to call and return _create_mem0_backend when config.backend == "mem0". Ensure all referenced symbols (Mem0MemoryBackend, Mem0EmbedderConfig, build_config_from_company_config, MemoryConfigError, MEMORY_BACKEND_CONFIG_INVALID, MEMORY_BACKEND_CREATED) are used exactly as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/ai_company/memory/factory.py`:
- Around line 26-113: Refactor the long mem0 branch in create_memory_backend
into a small helper named _create_mem0_backend(config, embedder) that performs
the current logic: import Mem0MemoryBackend, Mem0EmbedderConfig,
build_config_from_company_config; validate embedder presence/type and raise
MemoryConfigError with the same logger.warning calls using
MEMORY_BACKEND_CONFIG_INVALID; call build_config_from_company_config and wrap
ValueError into MemoryConfigError with identical logging; instantiate
Mem0MemoryBackend and wrap exceptions into MemoryConfigError with the same
logging; log MEMORY_BACKEND_CREATED and return the backend. Update
create_memory_backend to call and return _create_mem0_backend when
config.backend == "mem0". Ensure all referenced symbols (Mem0MemoryBackend,
Mem0EmbedderConfig, build_config_from_company_config, MemoryConfigError,
MEMORY_BACKEND_CONFIG_INVALID, MEMORY_BACKEND_CREATED) are used exactly as
before.
In `@tests/unit/memory/backends/mem0/test_adapter.py`:
- Around line 267-331: Collapse the eight nearly identical tests in
TestConnectionGuard into a single parametrized test using
pytest.mark.parametrize: instantiate Mem0MemoryBackend once per test and call
the backend method by name (use getattr on the Mem0MemoryBackend instance) with
the corresponding args, asserting MemoryConnectionError with match "Not
connected"; replace individual methods test_store_raises_when_disconnected,
test_retrieve_raises_when_disconnected, test_get_raises_when_disconnected,
test_delete_raises_when_disconnected, test_count_raises_when_disconnected,
test_publish_raises_when_disconnected,
test_search_shared_raises_when_disconnected, and
test_retract_raises_when_disconnected with a single async
test_raises_when_disconnected that parametrize tuples like ("store",
("test-agent-001", make_store_request())) and so on.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3ec61326-e4eb-4c6a-bb51-24b4cf8bac53
📒 Files selected for processing (8)
CLAUDE.mdsrc/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/backends/mem0/mappers.pysrc/ai_company/memory/factory.pytests/unit/memory/backends/mem0/conftest.pytests/unit/memory/backends/mem0/test_adapter.pytests/unit/memory/backends/mem0/test_adapter_crud.pytests/unit/memory/backends/mem0/test_mappers.py
📜 Review details
⏰ 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: Greptile Review
- GitHub Check: Test (Python 3.14)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Do NOT usefrom __future__ import annotations— Python 3.14 has PEP 649 native lazy annotations
Useexcept A, B:(no parentheses) for exception handling syntax per PEP 758 — ruff enforces this on Python 3.14
Files:
tests/unit/memory/backends/mem0/test_adapter.pysrc/ai_company/memory/factory.pytests/unit/memory/backends/mem0/test_mappers.pysrc/ai_company/memory/backends/mem0/adapter.pytests/unit/memory/backends/mem0/conftest.pysrc/ai_company/memory/backends/mem0/mappers.pytests/unit/memory/backends/mem0/test_adapter_crud.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowfor test markers
Prefer@pytest.mark.parametrizefor testing similar cases
Files:
tests/unit/memory/backends/mem0/test_adapter.pytests/unit/memory/backends/mem0/test_mappers.pytests/unit/memory/backends/mem0/conftest.pytests/unit/memory/backends/mem0/test_adapter_crud.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Type hints: all public functions must have type hints, mypy strict mode
Docstrings: Google style, required on public classes/functions (enforced by ruff D rules)
Create new objects instead of mutating existing ones—never mutate existing objects. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction +MappingProxyTypewrapping for read-only enforcement.
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention and usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence)
Config vs runtime state: use frozen Pydantic models for config/identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 (BaseModel,model_validator,computed_field,ConfigDict). Use@computed_fieldfor derived values instead of storing + validating redundant fields. UseNotBlankStr(fromcore.types) for all identifier/name fields—including optional and tuple variants—instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task.
Line length: 88 characters (enforced by ruff)
Functions: < 50 lines, files < 800 lines
Handle errors explicitly, never silently swallow them
Validate at system boundaries (user input, external APIs, config files)
Every module with business logic MUST have:from ai_company.observability import get_loggerthenlogger = get_logger(__name__)
Never useimport logging/logging.getLogger()/print()in application code
Always useloggeras the variable name (not_logger, notlog)
Use event name constants fromai_company.observability.eventsdomain-spe...
Files:
src/ai_company/memory/factory.pysrc/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/backends/mem0/mappers.py
🧠 Learnings (5)
📚 Learning: 2026-03-13T06:54:28.690Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.690Z
Learning: Applies to **/*.py : Use `except A, B:` (no parentheses) for exception handling syntax per PEP 758 — ruff enforces this on Python 3.14
Applied to files:
src/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/backends/mem0/mappers.py
📚 Learning: 2026-03-13T06:54:28.690Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.690Z
Learning: Applies to src/**/*.py : Functions: < 50 lines, files < 800 lines
Applied to files:
src/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/backends/mem0/mappers.py
📚 Learning: 2026-03-13T06:54:28.690Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.690Z
Learning: Applies to src/**/*.py : Line length: 88 characters (enforced by ruff)
Applied to files:
src/ai_company/memory/backends/mem0/adapter.py
📚 Learning: 2026-03-13T06:54:28.690Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.690Z
Learning: Applies to docs/**/*.md : Docs source: `docs/` (Markdown, built with Zensical). Design spec: `docs/design/` (7 pages: index, agents, organization, communication, engine, memory, operations).
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-13T06:54:28.690Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.690Z
Learning: Applies to pyproject.toml : Dependencies: all versions pinned with `==` in `pyproject.toml`. Groups: `test` and `dev` (includes test). Install via `uv sync`.
Applied to files:
CLAUDE.md
🧬 Code graph analysis (4)
src/ai_company/memory/backends/mem0/adapter.py (3)
src/ai_company/memory/backends/mem0/config.py (2)
Mem0BackendConfig(52-91)build_mem0_config_dict(94-121)src/ai_company/memory/backends/mem0/mappers.py (5)
extract_category(432-454)extract_publisher(457-470)mem0_result_to_entry(229-294)query_to_mem0_search_args(297-326)validate_add_result(389-429)src/ai_company/memory/errors.py (4)
MemoryConnectionError(17-18)MemoryRetrievalError(25-26)MemoryStoreError(21-22)MemoryError(13-14)
tests/unit/memory/backends/mem0/conftest.py (3)
src/ai_company/core/enums.py (1)
MemoryCategory(101-108)src/ai_company/memory/backends/mem0/config.py (2)
Mem0BackendConfig(52-91)Mem0EmbedderConfig(23-49)src/ai_company/memory/models.py (1)
MemoryStoreRequest(55-79)
src/ai_company/memory/backends/mem0/mappers.py (3)
src/ai_company/core/enums.py (1)
MemoryCategory(101-108)src/ai_company/memory/errors.py (2)
MemoryRetrievalError(25-26)MemoryStoreError(21-22)src/ai_company/memory/models.py (4)
MemoryEntry(82-150)MemoryMetadata(20-52)MemoryQuery(153-230)MemoryStoreRequest(55-79)
tests/unit/memory/backends/mem0/test_adapter_crud.py (3)
src/ai_company/memory/backends/mem0/adapter.py (2)
store(312-369)retrieve(371-432)src/ai_company/memory/errors.py (3)
MemoryRetrievalError(25-26)MemoryStoreError(21-22)MemoryError(13-14)src/ai_company/memory/models.py (1)
MemoryQuery(153-230)
🔇 Additional comments (54)
src/ai_company/memory/backends/mem0/mappers.py (13)
1-29: LGTM — module setup follows conventions.Proper logger initialization, event constant imports, and clear module docstring. The
_PREFIXconstant for metadata namespacing is well-documented.
38-57: LGTM — metadata serialization is correct.
build_mem0_metadataproperly handles optional fields and uses the prefixed keys consistently.
60-86: LGTM — datetime parsing with proper fallback.Handles naive datetimes by assuming UTC, logs malformed input appropriately, and uses correct PEP 758 exception syntax.
89-112: LGTM — score normalization handles edge cases.Correctly coerces string scores, clamps to [0.0, 1.0], and returns
Nonefor non-numeric input with appropriate logging.
115-131: LGTM — confidence coercion with clamping.Properly extracts, parses, and clamps confidence values to valid range [0.0, 1.0].
134-151: LGTM — source sanitization handles malformed input.Coerces to string, strips whitespace, and returns
Nonefor blank values with debug logging.
154-172: LGTM — tag normalization is robust.Handles string, list, tuple, and unexpected types gracefully. Explicit
str(t)conversion beforeNotBlankStraddresses the past review comment about non-string elements.
175-226: LGTM — metadata parsing delegates to helpers properly.Function stays focused on orchestration, delegating validation and coercion to the extracted helper functions (
_coerce_confidence,_coerce_source,_normalize_tags).
229-294: LGTM — result-to-entry conversion with thorough validation.Validates ID and content presence, handles missing timestamps, and delegates metadata parsing correctly.
297-345: LGTM — query argument builders are correct.
query_to_mem0_search_argsproperly validates thatquery.textis required for search operations.
348-383: LGTM — post-filter logic is correct.The
min_relevancefilter correctly skips entries withrelevance_score=None, and theuntilfilter is exclusive as documented.
389-429: LGTM — add-result validation is thorough.Validates
resultis a dict,resultsis a non-empty list, first item is a dict, and ID is non-blank. Addresses past review comment about validating object shapes.
432-470: LGTM — category and publisher extraction are defensive.Both functions handle missing/non-dict metadata gracefully, and
extract_publisherproperly coerces and strips values. Addresses past review comment about normalizing publisher strings.src/ai_company/memory/backends/mem0/adapter.py (14)
1-18: LGTM — module docstring properly documents the size exemption.The docstring explains why this file exceeds the 800-line guideline (cohesive implementation of three protocols) and documents the re-raise behavior for system-level exceptions.
88-119: LGTM — defensive result validation helper.
_validate_mem0_resultaddresses the past review concern about handlingNoneor non-dict results from Mem0 SDK calls.
146-183: LGTM — connect handles import and initialization errors properly.Deferred import of
mem0.Memory, proper error wrapping toMemoryConnectionError, and re-raisesMemoryError/RecursionErrorwithout wrapping.
196-235: LGTM — health check probes the backend correctly.Uses a lightweight
get_allwith limit=1 to verify connectivity, returnsFalseon any exception (except system-level ones which are re-raised).
281-308: LGTM — guards are well-implemented.
_require_connectedand_validate_agent_idprovide clear error messages and appropriate logging.
312-369: LGTM — store operation is correct.Validates connection and agent ID, builds metadata, calls Mem0
add()viaasyncio.to_thread(), and validates the result.
371-432: LGTM — retrieve handles search vs get_all correctly.Dispatches based on
query.textpresence, validates results, converts to domain entries, and applies post-filters.
434-507: LGTM — get verifies ownership correctly.Returns
Nonefor ownership mismatch rather than raising, which is the expected behavior for a "not found from this agent's perspective" scenario.
509-595: LGTM — delete has proper ownership and namespace checks.Blocks deletion of shared-namespace entries (must use
retract()), verifies ownership before delete, and handles the not-found case correctly.
597-680: LGTM — count with truncation warning.Logs a warning when the count equals
max_memories_per_agentto indicate potential underestimation.
684-746: LGTM — publish adds publisher metadata correctly.Merges
_PUBLISHER_KEYinto metadata and uses the reserved_SHARED_NAMESPACEfor isolation.
748-825: LGTM — search_shared handles publisher extraction and filtering.Uses
extract_publisherto populateagent_idfor shared entries, applies post-filters, and supportsexclude_agent.
827-913: LGTM — retract verifies publisher ownership.Correctly blocks retraction if the memory has no publisher metadata (not a shared entry) or if the publisher doesn't match the requesting agent.
185-194: No action required — mem0.Memory does not provide or require explicit close() for resource cleanup.The mem0 library's
Memoryclient does not expose aclose()method. Per the official mem0 SDK documentation and AutoGen's integration, mem0 clients do not require explicit cleanup; setting_client = Noneallows Python's garbage collector to properly reclaim the instance. This is by design in the embedded Qdrant + SQLite configuration, where all storage is local and the library manages any internal resource pooling.> Likely an incorrect or invalid review comment.src/ai_company/memory/factory.py (1)
48-78: LGTM — embedder validation is thorough.Validates both missing embedder and wrong type cases with clear error messages and appropriate logging.
CLAUDE.md (2)
97-97: LGTM — memory module description updated correctly.Documentation now accurately reflects the pluggable
MemoryBackendprotocol and Mem0 adapter structure.
208-208: LGTM — dependency classification corrected.
mem0aiis now correctly documented as "Required" rather than "Optional", matchingpyproject.toml. This addresses the past review comment about the mismatch.tests/unit/memory/backends/mem0/test_adapter_crud.py (6)
1-24: LGTM — test module setup is correct.Proper imports, timeout marker, and use of shared fixtures from conftest.
31-136: LGTM — store tests cover key scenarios.Tests success path, empty results, missing/blank IDs, exception wrapping,
MemoryErrorpropagation, and shared namespace rejection.
141-254: LGTM — retrieve tests validate search and get_all paths.Tests text-based search, get_all fallback, post-filtering, exception wrapping, and invalid entry handling.
260-318: LGTM — get tests cover existence and ownership checks.Tests found/not-found cases, exception wrapping,
MemoryErrorpropagation, and ownership mismatch returningNone.
324-408: LGTM — delete tests verify ownership and namespace restrictions.Tests successful delete, not-found, exception wrapping, shared namespace blocking, and cross-agent deletion rejection.
414-518: LGTM — count tests cover filtering and edge cases.Tests all/by-category counting, invalid category defaulting, empty results, and
MemoryErrorpropagation.tests/unit/memory/backends/mem0/test_adapter.py (4)
1-19: LGTM — test module imports and setup are correct.Proper imports for protocol interfaces and fixtures.
25-71: LGTM — properties and capabilities tests are thorough.Tests all capability properties and verifies expected values.
77-129: LGTM — protocol conformance tests verify interface compliance.Tests
hasattrfor all required methods andisinstancefor all three protocols.
135-261: LGTM — lifecycle tests cover success, failure, and error propagation.Tests connect success/failure, disconnect (including no-reset check), health check states, and
MemoryError/RecursionErrorpropagation through connect.tests/unit/memory/backends/mem0/conftest.py (4)
1-23: LGTM — test embedder uses vendor-agnostic names.
_test_embedder()uses"test-provider"and"test-embedding-001"following the coding guidelines to avoid real vendor names.
25-49: LGTM — fixtures are well-structured.
mem0_configprovides a test configuration,mock_clientprovides a MagicMock, andbackendwires them together in a connected state.
52-108: LGTM — mock response helpers are comprehensive.
mem0_add_result,mem0_search_result, andmem0_get_result(with optionaluser_id) cover the main Mem0 response shapes needed for testing. Theuser_idparameter addresses the past review comment.
111-117: LGTM —make_store_requesthelper is clean.Simple factory with sensible defaults for test scenarios.
tests/unit/memory/backends/mem0/test_mappers.py (10)
1-56: LGTM — test setup and helper are correct.
_make_entryhelper provides sensible defaults for building test entries.
58-91: LGTM —build_mem0_metadatatests cover basic and full metadata.Tests verify correct prefixing, optional field handling, and ISO format for expires_at.
94-122: LGTM — datetime parsing tests are thorough.Tests None, empty string, aware/naive timestamps, non-UTC timezones, and malformed input.
125-150: LGTM — relevance score tests cover coercion and edge cases.Tests None, in-range, clamping, boundaries, string coercion, and non-numeric input.
153-206: LGTM — metadata parsing tests cover all fields and defaults.Tests None, empty, full metadata, missing/invalid category, empty tags filtering, blank source, and non-string source coercion.
209-289: LGTM —mem0_result_to_entrytests validate conversion logic.Tests basic result, score extraction, missing created_at default, no metadata handling, and error cases (missing ID, empty content, data key fallback).
292-315: LGTM — query argument builder tests verify output shapes.Tests for both search and getall argument construction.
318-411: LGTM — post-filter tests cover all filter types.Tests no filters, category, tags, time range (since/until exclusive), min_relevance, and combined filters.
414-468: LGTM —validate_add_resulttests cover all validation paths.Tests valid result, empty results, missing key, non-list, missing/None/blank/whitespace ID, numeric ID coercion, and non-dict inputs.
471-544: LGTM — category and publisher extraction tests are comprehensive.Tests valid values, missing/empty/None/non-dict metadata, invalid category defaulting, and publisher coercion/blank handling.
- Remove unused noqa A004 directive (ruff auto-fix) - Fix import sorting in adapter.py (ruff auto-fix) - Add connect() idempotency test (verifies no-op when already connected) - Add _validate_agent_id error_cls tests for read ops (retrieve, get, count) - Add expires_at filtering tests in apply_post_filters - Add config validation tests using model_construct to bypass Pydantic - Fix factory init test patch target to match local import
|
Closing to create a fresh PR with clean review history. |
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
This PR adds a concrete Mem0-backed implementation of the ai_company.memory backend (adapter + config + mapping utilities), wires it into the memory backend factory, and updates dependencies/docs/tests accordingly.
Changes:
- Add Mem0 backend implementation (
Mem0MemoryBackend) with config builder and mapping helpers. - Wire
create_memory_backend()to construct Mem0 backend instances (with explicit embedder config). - Add extensive unit/integration tests and update docs/dependencies to reflect Mem0 support.
Reviewed changes
Copilot reviewed 79 out of 82 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Locks new Mem0-related dependencies (mem0ai + transitive deps). |
| pyproject.toml | Adds mem0ai dependency; mypy override; ruff per-file ignores for TC in tests. |
| src/ai_company/observability/events/memory.py | Adds new structured logging event constants for config/agent-id validation. |
| src/ai_company/memory/factory.py | Implements Mem0 backend construction in factory with validation/wrapping. |
| src/ai_company/memory/backends/mem0/adapter.py | New Mem0 backend adapter implementing CRUD + shared knowledge operations. |
| src/ai_company/memory/backends/mem0/config.py | New Mem0 backend config models + builder to Mem0 SDK config dict. |
| src/ai_company/memory/backends/mem0/mappers.py | New Mem0↔domain mapping utilities and post-filtering helpers. |
| src/ai_company/memory/backends/mem0/init.py | Exposes Mem0 backend/config symbols. |
| src/ai_company/memory/backends/init.py | Exposes concrete backends at package level. |
| src/ai_company/memory/init.py | Re-exports Mem0 backend/config from ai_company.memory. |
| README.md | Updates project status to indicate Mem0 adapter is implemented. |
| docs/roadmap/index.md | Updates roadmap to reflect Mem0 adapter completion. |
| docs/design/memory.md | Documents Mem0 as implemented and shows embedder config usage pattern. |
| CLAUDE.md | Updates repo structure notes and dependency notes for Mem0. |
| .github/workflows/dependency-review.yml | Allows additional licenses / missing license metadata for new deps. |
| tests/unit/memory/test_init.py | Adds Mem0 exports to expected ai_company.memory public surface. |
| tests/unit/memory/test_factory.py | Updates factory tests for Mem0 backend creation + error handling. |
| tests/unit/memory/backends/init.py | Package marker for backend unit tests. |
| tests/unit/memory/backends/mem0/init.py | Package marker for Mem0 unit tests. |
| tests/unit/memory/backends/mem0/conftest.py | Shared fixtures for Mem0 backend unit tests. |
| tests/unit/memory/backends/mem0/test_adapter.py | Unit tests for adapter lifecycle/guards/capabilities. |
| tests/unit/memory/backends/mem0/test_adapter_crud.py | Unit tests for store/retrieve/get/delete/count flows. |
| tests/unit/memory/backends/mem0/test_adapter_shared.py | Unit tests for publish/search_shared/retract flows. |
| tests/unit/memory/backends/mem0/test_config.py | Unit tests for Mem0 config models and builders. |
| tests/unit/memory/backends/mem0/test_mappers.py | Unit tests for mapping/parsing/filtering utilities. |
| tests/integration/memory/test_mem0_backend.py | “Integration” tests validating adapter works in retrieval pipeline with mocked client. |
| tests/unit/tools/sandbox/test_protocol.py | Removes TC-related noqa on imports. |
| tests/unit/tools/sandbox/conftest.py | Removes TC-related noqa on imports. |
| tests/unit/tools/git/test_git_sandbox_integration.py | Removes TC-related noqa on imports. |
| tests/unit/tools/git/conftest.py | Removes TC-related noqa on imports. |
| tests/unit/providers/test_protocol.py | Removes TC-related noqa on imports. |
| tests/unit/providers/conftest.py | Removes TC-related noqa on imports. |
| tests/unit/hr/test_registry.py | Removes TC-related noqa on imports. |
| tests/unit/hr/test_onboarding_service.py | Removes TC-related noqa on imports. |
| tests/unit/hr/test_offboarding_service.py | Removes TC-related noqa on imports. |
| tests/unit/hr/test_hiring_service.py | Removes TC-related noqa on imports. |
| tests/unit/hr/test_full_snapshot_strategy.py | Removes TC-related noqa on imports. |
| tests/unit/engine/test_task_engine_mutations.py | Removes TC-related noqa on imports. |
| tests/unit/engine/test_routing_models.py | Removes TC-related noqa on imports. |
| tests/unit/engine/test_react_loop.py | Removes TC-related noqa on imports. |
| tests/unit/engine/test_plan_execute_loop.py | Removes TC-related noqa on imports. |
| tests/unit/engine/test_metrics.py | Removes TC-related noqa on imports. |
| tests/unit/engine/test_loop_protocol.py | Removes TC-related noqa on imports. |
| tests/unit/engine/test_context.py | Removes TC-related noqa on imports. |
| tests/unit/engine/test_agent_engine.py | Removes TC-related noqa on imports. |
| tests/unit/engine/test_agent_engine_lifecycle.py | Removes TC-related noqa on imports. |
| tests/unit/engine/test_agent_engine_errors.py | Removes TC-related noqa on imports. |
| tests/unit/engine/task_engine_helpers.py | Removes TC-related noqa on imports. |
| tests/unit/communication/meeting/test_structured_phases.py | Removes TC-related noqa on imports. |
| tests/unit/communication/meeting/test_round_robin.py | Removes TC-related noqa on imports. |
| tests/unit/communication/meeting/test_protocol.py | Removes TC-related noqa on imports. |
| tests/unit/communication/meeting/test_position_papers.py | Removes TC-related noqa on imports. |
| tests/unit/communication/meeting/test_orchestrator.py | Removes TC-related noqa on imports. |
| tests/unit/communication/meeting/conftest.py | Removes TC-related noqa on imports. |
| tests/unit/communication/conflict_resolution/test_service.py | Removes TC-related noqa on imports. |
| tests/unit/communication/conflict_resolution/test_hybrid_strategy.py | Removes TC-related noqa on imports. |
| tests/unit/communication/conflict_resolution/test_helpers.py | Removes TC-related noqa on imports. |
| tests/unit/communication/conflict_resolution/test_debate_strategy.py | Removes TC-related noqa on imports. |
| tests/unit/communication/conflict_resolution/test_authority_strategy.py | Removes TC-related noqa on imports. |
| tests/unit/budget/test_category_analytics.py | Removes TC-related noqa on imports. |
| tests/unit/api/test_middleware.py | Removes TC-related noqa on imports. |
| tests/unit/api/test_health.py | Removes TC-related noqa on imports. |
| tests/unit/api/test_guards.py | Removes TC-related noqa on imports. |
| tests/unit/api/test_app.py | Removes TC-related noqa on imports. |
| tests/unit/api/controllers/test_tasks.py | Removes TC-related noqa on imports. |
| tests/unit/api/controllers/test_providers.py | Removes TC-related noqa on imports. |
| tests/unit/api/controllers/test_projects.py | Removes TC-related noqa on imports. |
| tests/unit/api/controllers/test_messages.py | Removes TC-related noqa on imports. |
| tests/unit/api/controllers/test_meetings.py | Removes TC-related noqa on imports. |
| tests/unit/api/controllers/test_departments.py | Removes TC-related noqa on imports. |
| tests/unit/api/controllers/test_company.py | Removes TC-related noqa on imports. |
| tests/unit/api/controllers/test_budget.py | Removes TC-related noqa on imports. |
| tests/unit/api/controllers/test_autonomy.py | Removes TC-related noqa on imports. |
| tests/unit/api/controllers/test_artifacts.py | Removes TC-related noqa on imports. |
| tests/unit/api/controllers/test_approvals.py | Removes TC-related noqa on imports. |
| tests/unit/api/controllers/test_analytics.py | Removes TC-related noqa on imports. |
| tests/unit/api/controllers/test_agents.py | Removes TC-related noqa on imports. |
| tests/unit/api/conftest.py | Removes TC-related noqa on imports. |
| tests/unit/api/auth/test_controller.py | Removes TC-related noqa on imports. |
| tests/integration/tools/test_sandbox_integration.py | Removes TC-related noqa on imports. |
| tests/integration/tools/conftest.py | Removes TC-related noqa on imports. |
| tests/integration/communication/test_meeting_integration.py | Removes TC-related noqa on imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @property | ||
| def backend_name(self) -> NotBlankStr: | ||
| """Human-readable backend identifier.""" | ||
| return NotBlankStr("mem0") | ||
|
|
| def _normalize_tags( | ||
| raw_metadata: dict[str, Any], | ||
| ) -> tuple[NotBlankStr, ...]: | ||
| """Extract and normalize tags from Mem0 metadata. | ||
|
|
||
| Handles string, list, tuple, and unexpected types gracefully. | ||
| """ | ||
| raw_tags = raw_metadata.get(f"{_PREFIX}tags", ()) | ||
| if isinstance(raw_tags, str): | ||
| raw_tags = [raw_tags] | ||
| elif not isinstance(raw_tags, (list, tuple)): | ||
| logger.debug( | ||
| MEMORY_MODEL_INVALID, | ||
| field="tags", | ||
| raw_value=type(raw_tags).__name__, | ||
| reason="unexpected tags type, ignoring", | ||
| ) | ||
| raw_tags = () | ||
| return tuple(NotBlankStr(str(t)) for t in raw_tags if t and str(t).strip()) | ||
|
|
| raw_id = raw.get("id") | ||
| if raw_id is None or not str(raw_id).strip(): | ||
| msg = f"Mem0 result has missing or blank 'id': keys={list(raw.keys())}" | ||
| logger.warning( | ||
| MEMORY_MODEL_INVALID, | ||
| field="id", | ||
| raw_value=raw_id, | ||
| reason=msg, | ||
| ) | ||
| raise MemoryRetrievalError(msg) | ||
| memory_id = NotBlankStr(str(raw_id)) | ||
|
|
||
| raw_content = raw.get("memory") or raw.get("data") | ||
| if not raw_content or not str(raw_content).strip(): | ||
| msg = f"Mem0 result {raw.get('id', '?')} has empty content" | ||
| logger.warning( | ||
| MEMORY_MODEL_INVALID, | ||
| field="content", | ||
| raw_value=raw_content, | ||
| reason=msg, | ||
| ) | ||
| raise MemoryRetrievalError(msg) | ||
| content = NotBlankStr(str(raw_content)) | ||
|
|
||
| created_at = parse_mem0_datetime(raw.get("created_at")) | ||
| if created_at is None: | ||
| logger.debug( | ||
| MEMORY_MODEL_INVALID, | ||
| field="created_at", | ||
| memory_id=str(raw.get("id", "?")), | ||
| reason="missing or unparseable created_at, defaulting to now()", | ||
| ) | ||
| created_at = datetime.now(UTC) | ||
| updated_at = parse_mem0_datetime(raw.get("updated_at")) | ||
|
|
||
| raw_metadata = raw.get("metadata") | ||
| category, metadata, expires_at = parse_mem0_metadata(raw_metadata) | ||
|
|
||
| raw_score = raw.get("score") | ||
| relevance_score = normalize_relevance_score(raw_score) | ||
|
|
||
| return MemoryEntry( | ||
| id=memory_id, | ||
| agent_id=NotBlankStr(agent_id), | ||
| category=category, |
| raw_id = first.get("id") | ||
| if raw_id is None or not str(raw_id).strip(): | ||
| msg = ( | ||
| f"Mem0 add result has missing or blank 'id' for {context}: " | ||
| f"keys={list(first.keys())}" | ||
| ) | ||
| logger.warning(MEMORY_ENTRY_STORE_FAILED, context=context, error=msg) | ||
| raise MemoryStoreError(msg) | ||
| return NotBlankStr(str(raw_id)) |
| self._require_connected() | ||
| self._validate_agent_id(agent_id, error_cls=MemoryRetrievalError) | ||
| try: | ||
| raw_result = await asyncio.to_thread( | ||
| self._client.get_all, | ||
| user_id=str(agent_id), | ||
| limit=self._max_memories_per_agent, | ||
| ) | ||
| raw_list = _validate_mem0_result(raw_result, context="count") | ||
| if category is None: | ||
| total = len(raw_list) | ||
| else: | ||
| total = sum( | ||
| 1 for item in raw_list if extract_category(item) == category | ||
| ) | ||
| except MemoryRetrievalError as exc: | ||
| logger.warning( | ||
| MEMORY_ENTRY_COUNT_FAILED, | ||
| agent_id=agent_id, | ||
| error=str(exc), | ||
| error_type="MemoryRetrievalError", | ||
| ) | ||
| raise | ||
| except builtins.MemoryError, RecursionError: | ||
| raise | ||
| except Exception as exc: | ||
| logger.warning( | ||
| MEMORY_ENTRY_COUNT_FAILED, | ||
| agent_id=agent_id, | ||
| error=str(exc), | ||
| error_type=type(exc).__name__, | ||
| ) | ||
| msg = f"Failed to count memories: {exc}" | ||
| raise MemoryRetrievalError(msg) from exc | ||
| else: | ||
| truncated = total == self._max_memories_per_agent | ||
| if truncated: | ||
| logger.warning( | ||
| MEMORY_ENTRY_COUNTED, | ||
| agent_id=agent_id, | ||
| count=total, | ||
| category=category.value if category else None, | ||
| truncated=True, | ||
| reason="count equals max_memories_per_agent, " | ||
| "actual count may be higher", | ||
| ) | ||
| else: | ||
| logger.info( | ||
| MEMORY_ENTRY_COUNTED, | ||
| agent_id=agent_id, | ||
| count=total, | ||
| category=category.value if category else None, | ||
| ) | ||
| return total |
There was a problem hiding this comment.
count() overcounts when entries have expires_at set
count() fetches raw entries from Mem0 and tallies them directly, but never calls apply_post_filters. This means entries whose expires_at has already passed are still included in the total.
retrieve() and search_shared() both route through apply_post_filters, which discards expired entries at line 370 of mappers.py:
if entry.expires_at is not None and entry.expires_at <= now:
continueSo after entries expire, count() returns a stale inflated total while retrieve() on the same agent returns fewer entries — a direct semantic inconsistency. A caller who guards store() behind a count() < limit check would allow more writes than intended once entries start expiring.
The fix is to deserialise the raw list into MemoryEntry objects and pass them through apply_post_filters before counting, the same way retrieve does, or at minimum inline the expiry check:
now = datetime.now(UTC)
...
if category is None:
total = sum(
1 for item in raw_list
if not (
parse_mem0_datetime(item.get("metadata", {}).get("_synthorg_expires_at"))
is not None
and parse_mem0_datetime(item.get("metadata", {}).get("_synthorg_expires_at")) <= now
)
)A cleaner approach: deserialise with mem0_result_to_entry and pass through apply_post_filters(entries, MemoryQuery(limit=..., categories={category} if category else set())) so all post-filters stay in one place.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ai_company/memory/backends/mem0/adapter.py
Line: 644-697
Comment:
**`count()` overcounts when entries have `expires_at` set**
`count()` fetches raw entries from Mem0 and tallies them directly, but never calls `apply_post_filters`. This means entries whose `expires_at` has already passed are still included in the total.
`retrieve()` and `search_shared()` both route through `apply_post_filters`, which discards expired entries at line 370 of `mappers.py`:
```python
if entry.expires_at is not None and entry.expires_at <= now:
continue
```
So after entries expire, `count()` returns a stale inflated total while `retrieve()` on the same agent returns fewer entries — a direct semantic inconsistency. A caller who guards `store()` behind a `count() < limit` check would allow more writes than intended once entries start expiring.
The fix is to deserialise the raw list into `MemoryEntry` objects and pass them through `apply_post_filters` before counting, the same way `retrieve` does, or at minimum inline the expiry check:
```python
now = datetime.now(UTC)
...
if category is None:
total = sum(
1 for item in raw_list
if not (
parse_mem0_datetime(item.get("metadata", {}).get("_synthorg_expires_at"))
is not None
and parse_mem0_datetime(item.get("metadata", {}).get("_synthorg_expires_at")) <= now
)
)
```
A cleaner approach: deserialise with `mem0_result_to_entry` and pass through `apply_post_filters(entries, MemoryQuery(limit=..., categories={category} if category else set()))` so all post-filters stay in one place.
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| if exclude_agent is not None: | ||
| filtered = tuple(e for e in filtered if e.agent_id != exclude_agent) |
There was a problem hiding this comment.
exclude_agent comparison uses fabricated agent_id for publisher-less entries
When a shared entry has no publisher metadata, extract_publisher(item) returns None, so agent_id is set to _SHARED_NAMESPACE in mem0_result_to_entry:
mem0_result_to_entry(
item,
extract_publisher(item) or _SHARED_NAMESPACE, # ← fabricated fallback
)Then the exclude_agent filter compares e.agent_id != exclude_agent. If two different agents each published entries whose publisher metadata was stripped (e.g. by a Mem0 SDK update that drops unrecognised metadata keys), both entries land with agent_id == _SHARED_NAMESPACE. Passing exclude_agent=_SHARED_NAMESPACE would then silently suppress all such entries, while passing any real agent_id would include them regardless of who actually wrote them.
Since retract already requires the publisher key to be present (it raises when publisher is None), publisher-less shared entries represent corrupted state. It would be safer to log a warning and skip them in search_shared rather than assigning an ownership guess to _SHARED_NAMESPACE.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ai_company/memory/backends/mem0/adapter.py
Line: 812-814
Comment:
**`exclude_agent` comparison uses fabricated `agent_id` for publisher-less entries**
When a shared entry has no publisher metadata, `extract_publisher(item)` returns `None`, so `agent_id` is set to `_SHARED_NAMESPACE` in `mem0_result_to_entry`:
```python
mem0_result_to_entry(
item,
extract_publisher(item) or _SHARED_NAMESPACE, # ← fabricated fallback
)
```
Then the `exclude_agent` filter compares `e.agent_id != exclude_agent`. If two different agents each published entries whose publisher metadata was stripped (e.g. by a Mem0 SDK update that drops unrecognised metadata keys), both entries land with `agent_id == _SHARED_NAMESPACE`. Passing `exclude_agent=_SHARED_NAMESPACE` would then silently suppress all such entries, while passing any real `agent_id` would include them regardless of who actually wrote them.
Since `retract` already requires the publisher key to be present (it raises when `publisher is None`), publisher-less shared entries represent corrupted state. It would be safer to log a warning and skip them in `search_shared` rather than assigning an ownership guess to `_SHARED_NAMESPACE`.
How can I resolve this? If you propose a fix, please make it concise.
Summary
Mem0MemoryBackendadapter behind the pluggableMemoryBackendprotocol — embedded Qdrant + SQLite, all Mem0 calls wrapped inasyncio.to_thread()store,retrieve,get,delete,countwith per-agent isolation viauser_idpublish,search_shared,retractwith publisher ownership trackingMem0EmbedderConfig—providerandmodelare required fields (no hardcoded defaults)MemoryError/RecursionErrorre-raised before genericexcept Exceptionhandlersapply_post_filtersMEMORY_BACKEND_CONFIG_INVALIDeventTest plan
uv run ruff check src/ tests/— passesuv run mypy src/ tests/— no errorsuv run pytest tests/ -n auto -m "unit or integration"— 164 mem0 tests passCloses #206