feat: implement Mem0 memory backend adapter#341
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
…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
mem0ai was moved to optional-dependencies — CI needs --extra mem0 in the uv sync command to install it for tests.
mem0ai is the only memory backend — no reason to make it optional. Reverts the optional-dependencies split and CI extra flag.
…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
- 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
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF ScorecardScorecard details
Scanned Files
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a complete Mem0 memory backend (adapter, config, mappers), wires it into the factory with an embedder parameter, updates observability events and docs, adds mem0 dependency and license allowances, and introduces extensive unit/integration tests plus test lint-cleanup across many files. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Backend as Mem0MemoryBackend
participant Mem0 as Mem0 Service
participant Qdrant as Qdrant (Vector Store)
participant SQLite as SQLite (History DB)
App->>Backend: store(agent_id, request)
activate Backend
Backend->>Backend: _require_connected()
Backend->>Backend: build_mem0_metadata(request)
Backend->>Mem0: add() in asyncio.to_thread()
activate Mem0
Mem0->>Qdrant: store embedding
Mem0->>SQLite: store metadata/history
Mem0-->>Backend: result (memory_id)
deactivate Mem0
Backend->>Backend: validate_add_result()
Backend-->>App: memory_id
deactivate Backend
App->>Backend: retrieve(agent_id, query)
activate Backend
Backend->>Backend: _require_connected()
alt query.text provided
Backend->>Mem0: search(text) in asyncio.to_thread()
activate Mem0
Mem0->>Qdrant: semantic search
Mem0->>SQLite: fetch metadata
Mem0-->>Backend: search results
deactivate Mem0
else no text
Backend->>Mem0: get_all() in asyncio.to_thread()
activate Mem0
Mem0->>SQLite: retrieve all
Mem0-->>Backend: results
deactivate Mem0
end
Backend->>Backend: apply_post_filters(results, query)
Backend-->>App: MemoryEntry[]
deactivate Backend
App->>Backend: publish(agent_id, request)
activate Backend
Backend->>Mem0: add() with shared namespace
activate Mem0
Mem0->>Qdrant: store shared embedding
Mem0->>SQLite: store shared metadata + publisher
Mem0-->>Backend: result
deactivate Mem0
Backend-->>App: shared memory_id
deactivate Backend
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #341 +/- ##
==========================================
+ Coverage 93.82% 93.93% +0.10%
==========================================
Files 441 446 +5
Lines 20243 20742 +499
Branches 1950 2002 +52
==========================================
+ Hits 18993 19484 +491
- Misses 966 972 +6
- Partials 284 286 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 the foundational Mem0 memory backend, significantly enhancing the system's ability to manage persistent agent memory. It provides a robust, pluggable solution for storing and retrieving agent knowledge, including vector search and temporal tracking capabilities, while ensuring proper isolation and shared access. This integration marks a major step towards a more sophisticated and scalable 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 implements the first concrete Key observations:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant Mem0MemoryBackend
participant asyncio.to_thread
participant Mem0Memory
participant mappers
Note over Caller,mappers: store() / publish() flow
Caller->>Mem0MemoryBackend: store(agent_id, request)
Mem0MemoryBackend->>Mem0MemoryBackend: _require_connected()
Mem0MemoryBackend->>Mem0MemoryBackend: _validate_agent_id(agent_id)
Mem0MemoryBackend->>mappers: build_mem0_metadata(request)
mappers-->>Mem0MemoryBackend: metadata dict
Mem0MemoryBackend->>asyncio.to_thread: Memory.add(messages, user_id, metadata, infer=False)
asyncio.to_thread->>Mem0Memory: add(...)
Mem0Memory-->>asyncio.to_thread: {results: [{id, ...}]}
asyncio.to_thread-->>Mem0MemoryBackend: result dict
Mem0MemoryBackend->>mappers: validate_add_result(result)
mappers-->>Mem0MemoryBackend: NotBlankStr(memory_id)
Mem0MemoryBackend-->>Caller: memory_id
Note over Caller,mappers: retrieve() flow
Caller->>Mem0MemoryBackend: retrieve(agent_id, query)
Mem0MemoryBackend->>asyncio.to_thread: Memory.search() or Memory.get_all()
asyncio.to_thread->>Mem0Memory: search/get_all(user_id, limit)
Mem0Memory-->>asyncio.to_thread: {results: [...]}
asyncio.to_thread-->>Mem0MemoryBackend: raw result
Mem0MemoryBackend->>mappers: mem0_result_to_entry(item, agent_id) × N
mappers-->>Mem0MemoryBackend: tuple[MemoryEntry]
Mem0MemoryBackend->>mappers: apply_post_filters(entries, query)
mappers-->>Mem0MemoryBackend: filtered tuple[MemoryEntry]
Mem0MemoryBackend-->>Caller: tuple[MemoryEntry]
Note over Caller,mappers: search_shared() + exclude_agent flow
Caller->>Mem0MemoryBackend: search_shared(query, exclude_agent)
Mem0MemoryBackend->>asyncio.to_thread: Memory.search/get_all(user_id=__synthorg_shared__)
asyncio.to_thread-->>Mem0MemoryBackend: raw results
Mem0MemoryBackend->>mappers: extract_publisher(item) → publisher
Mem0MemoryBackend->>mappers: mem0_result_to_entry(item, publisher)
Mem0MemoryBackend->>mappers: apply_post_filters(entries, query)
Mem0MemoryBackend->>Mem0MemoryBackend: filter out exclude_agent entries
Mem0MemoryBackend-->>Caller: tuple[MemoryEntry]
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/ai_company/memory/backends/mem0/adapter.py
Line: 638-676
Comment:
**Dead-code `if owner is not None` guards after unconditional raise**
After the `raise MemoryStoreError` at line 650 when `owner is None`, control flow never reaches lines 651 or 664. The `if owner is not None and …` prefix on both subsequent branches is therefore always `True` and constitutes dead code. While this causes no runtime bug today, it is misleading to future readers who might think the `None` case is "handled" again at those branches, and becomes a latent footgun if the guard at line 650 is ever restructured (the subsequent conditions would silently stop providing protection).
```suggestion
owner = existing.get("user_id")
if owner is None:
msg = (
f"Memory {memory_id} has no user_id — ownership "
f"unverifiable, refusing deletion"
)
logger.warning(
MEMORY_ENTRY_DELETE_FAILED,
agent_id=agent_id,
memory_id=memory_id,
reason="unverifiable_ownership",
)
raise MemoryStoreError(msg) # noqa: TRY301
if str(owner) == _SHARED_NAMESPACE:
msg = (
f"Memory {memory_id} belongs to the shared namespace — "
f"use retract() to remove shared entries"
)
logger.warning(
MEMORY_ENTRY_DELETE_FAILED,
agent_id=agent_id,
memory_id=memory_id,
reason="shared namespace entry",
)
raise MemoryStoreError(msg) # noqa: TRY301
# Verify ownership — reject cross-agent deletion.
if str(owner) != str(agent_id):
msg = (
f"Agent {agent_id} cannot delete memory "
f"{memory_id} owned by {owner}"
)
logger.warning(
MEMORY_ENTRY_DELETE_FAILED,
agent_id=agent_id,
memory_id=memory_id,
reason="ownership mismatch",
actual_owner=str(owner),
)
raise MemoryStoreError(msg) # noqa: TRY301
```
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/mappers.py
Line: 295-315
Comment:
**Using `expires_at` as `created_at` fallback produces semantically invalid timestamps**
When `created_at` and `updated_at` are both absent from a Mem0 result but `expires_at` is present, the code sets `created_at = expires_at`. This creates a `MemoryEntry` where the entry *appears to have been created at its own expiry time*, which is semantically incorrect and has a concrete downstream effect.
`apply_post_filters` applies the `since` filter as `entry.created_at >= query.since`. An entry with `created_at == expires_at = T_future` would satisfy `T_future >= since` for any `since < T_future`, appearing in results as if it were created in the future — even if it was actually created long before `since`. Callers relying on `since` to exclude old memories would receive stale entries.
The `datetime.now(UTC)` fallback (the else-branch below) is actually *safer* for `since` semantics: it anchors the entry to "approximately now", which is at least a conservative present-tense timestamp rather than a future one. Using `expires_at` as the fallback should either be dropped in favour of `datetime.now(UTC)`, or documented explicitly as a known source of `since`-filter inaccuracy.
```suggestion
if created_at is None:
# Pick the best available fallback to avoid violating the
# MemoryEntry invariants (updated_at >= created_at).
# NOTE: expires_at is intentionally excluded as a fallback —
# using a future timestamp as created_at would cause since-
# filter inaccuracies (entries appearing newer than they are).
if updated_at is not None:
fallback = updated_at
fallback_source = "updated_at"
else:
fallback = datetime.now(UTC)
fallback_source = "now()"
logger.warning(
MEMORY_MODEL_INVALID,
field="created_at",
memory_id=str(raw.get("id", "?")),
reason=f"missing or unparseable created_at, "
f"defaulting to {fallback_source}",
)
created_at = fallback
```
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: 883-896
Comment:
**`search_shared()` duplicates query-arg construction instead of using shared helpers**
`retrieve()` delegates to `query_to_mem0_search_args()` and `query_to_mem0_getall_args()` for building Mem0 call kwargs, keeping the adapter thin and the mapping logic in one place. `search_shared()` builds the same kwargs inline (lines 884–896), bypassing those helpers.
This creates a maintenance split: any future extension to the helpers (e.g. passing category hints, output fields, or other native Mem0 filters) would automatically benefit `retrieve()` but would silently leave `search_shared()` behind.
The helpers accept a plain `str` for `agent_id`, so using them here only requires passing `_SHARED_NAMESPACE` as that argument:
```python
if query.text is not None:
kwargs = query_to_mem0_search_args(_SHARED_NAMESPACE, query)
raw_result = await asyncio.to_thread(self._client.search, **kwargs)
else:
kwargs = query_to_mem0_getall_args(_SHARED_NAMESPACE, query)
raw_result = await asyncio.to_thread(self._client.get_all, **kwargs)
```
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: 6b69d77 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Implements the first concrete MemoryBackend by adding a Mem0-backed adapter (embedded Qdrant + SQLite), wiring it into the memory backend factory, and updating exports/docs. The PR also expands memory-specific unit/integration coverage and removes TC*-style noqa markers across many tests now that TC is ignored in test files.
Changes:
- Add
Mem0MemoryBackendadapter + Mem0 config/mapping utilities (CRUD + shared knowledge). - Wire
create_memory_backend()to instantiate the Mem0 backend (with embedder config) and add new observability event constants. - Add extensive Mem0 unit + integration tests; simplify numerous test imports by removing
# noqa: TC*.
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 | Adds Mem0 + Qdrant/SQLAlchemy dependency set to lockfile. |
| pyproject.toml | Adds mem0ai dependency; updates ruff/mypy config for mem0 + TC ignores in tests. |
| src/ai_company/observability/events/memory.py | Adds new memory backend event constants for config/agent-id guardrails. |
| src/ai_company/memory/factory.py | Implements Mem0 backend creation + error wrapping and logging. |
| src/ai_company/memory/backends/mem0/adapter.py | New Mem0 adapter implementing lifecycle, CRUD, and shared knowledge operations. |
| src/ai_company/memory/backends/mem0/config.py | New Mem0 backend + embedder config models and builders. |
| src/ai_company/memory/backends/mem0/mappers.py | New mapping/filtering utilities between domain models and Mem0 payloads. |
| src/ai_company/memory/backends/mem0/init.py | Exports Mem0 backend + config types. |
| src/ai_company/memory/backends/init.py | Adds concrete backend package export. |
| src/ai_company/memory/init.py | Re-exports Mem0 backend + embedder config in public memory API. |
| tests/unit/memory/test_init.py | Updates export expectations to include Mem0 symbols. |
| tests/unit/memory/test_factory.py | Adds Mem0 backend factory tests (embedder required, error wrapping, options passthrough). |
| tests/unit/memory/backends/mem0/conftest.py | Shared fixtures for Mem0 adapter unit tests. |
| tests/unit/memory/backends/mem0/test_adapter.py | Unit tests for lifecycle, capabilities, protocol conformance, connection guards. |
| tests/unit/memory/backends/mem0/test_adapter_crud.py | Unit tests for CRUD behavior + error wrapping/ownership checks. |
| tests/unit/memory/backends/mem0/test_adapter_shared.py | Unit tests for shared publish/search/retract flows. |
| tests/unit/memory/backends/mem0/test_config.py | Unit tests for Mem0 config validation and config dict builders. |
| tests/unit/memory/backends/mem0/test_mappers.py | Unit tests for mappers/parsing/filters/expiry enforcement. |
| tests/integration/memory/test_mem0_backend.py | Integration-style test validating adapter in retrieval pipeline with mocked Mem0 client. |
| docs/design/memory.md | Updates design doc to reflect Mem0 adapter is implemented + embedder passed via factory. |
| docs/roadmap/index.md | Updates roadmap to mark Mem0 adapter as implemented. |
| README.md | Updates status section to reflect Mem0 backend adapter completion. |
| CLAUDE.md | Updates architecture/deps notes to include Mem0 backend as required dependency. |
| .github/workflows/dependency-review.yml | Updates allowed license handling for new deps / missing SPDX metadata. |
| tests/unit/tools/sandbox/test_protocol.py | Removes # noqa: TC* markers from imports. |
| tests/unit/tools/sandbox/conftest.py | Removes # noqa: TC* markers from imports. |
| tests/unit/tools/git/test_git_sandbox_integration.py | Removes # noqa: TC* markers from imports. |
| tests/unit/tools/git/conftest.py | Removes # noqa: TC* markers from imports. |
| tests/unit/providers/test_protocol.py | Removes # noqa: TC* markers from imports. |
| tests/unit/providers/conftest.py | Removes # noqa: TC* markers from imports. |
| tests/unit/hr/test_registry.py | Removes # noqa: TC* markers from imports. |
| tests/unit/hr/test_onboarding_service.py | Removes # noqa: TC* markers from imports. |
| tests/unit/hr/test_offboarding_service.py | Removes # noqa: TC* markers from imports. |
| tests/unit/hr/test_hiring_service.py | Removes # noqa: TC* markers from imports. |
| tests/unit/hr/test_full_snapshot_strategy.py | Removes # noqa: TC* markers from imports. |
| tests/unit/engine/test_task_engine_mutations.py | Removes # noqa: TC* markers from imports. |
| tests/unit/engine/test_routing_models.py | Removes # noqa: TC* markers from imports. |
| tests/unit/engine/test_react_loop.py | Removes # noqa: TC* markers from imports. |
| tests/unit/engine/test_plan_execute_loop.py | Removes # noqa: TC* markers from imports. |
| tests/unit/engine/test_metrics.py | Removes # noqa: TC* markers from imports. |
| tests/unit/engine/test_loop_protocol.py | Removes # noqa: TC* markers from imports. |
| tests/unit/engine/test_context.py | Removes # noqa: TC* markers from imports. |
| tests/unit/engine/test_agent_engine_lifecycle.py | Removes # noqa: TC* markers from imports. |
| tests/unit/engine/test_agent_engine_errors.py | Removes # noqa: TC* markers from imports. |
| tests/unit/engine/test_agent_engine.py | Removes # noqa: TC* markers from imports. |
| tests/unit/engine/task_engine_helpers.py | Removes # noqa: TC* markers from imports. |
| tests/unit/communication/meeting/test_structured_phases.py | Removes # noqa: TC* markers from imports. |
| tests/unit/communication/meeting/test_round_robin.py | Removes # noqa: TC* markers from imports. |
| tests/unit/communication/meeting/test_protocol.py | Removes # noqa: TC* markers from imports. |
| tests/unit/communication/meeting/test_position_papers.py | Removes # noqa: TC* markers from imports. |
| tests/unit/communication/meeting/test_orchestrator.py | Removes # noqa: TC* markers from imports. |
| tests/unit/communication/meeting/conftest.py | Removes # noqa: TC* markers from imports. |
| tests/unit/communication/conflict_resolution/test_service.py | Removes # noqa: TC* markers from imports. |
| tests/unit/communication/conflict_resolution/test_hybrid_strategy.py | Removes # noqa: TC* markers from imports. |
| tests/unit/communication/conflict_resolution/test_helpers.py | Removes # noqa: TC* markers from imports. |
| tests/unit/communication/conflict_resolution/test_debate_strategy.py | Removes # noqa: TC* markers from imports. |
| tests/unit/communication/conflict_resolution/test_authority_strategy.py | Removes # noqa: TC* markers from imports. |
| tests/unit/budget/test_category_analytics.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/test_middleware.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/test_health.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/test_guards.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/test_app.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/controllers/test_tasks.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/controllers/test_providers.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/controllers/test_projects.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/controllers/test_messages.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/controllers/test_meetings.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/controllers/test_departments.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/controllers/test_company.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/controllers/test_budget.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/controllers/test_autonomy.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/controllers/test_artifacts.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/controllers/test_approvals.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/controllers/test_analytics.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/controllers/test_agents.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/conftest.py | Removes # noqa: TC* markers from imports. |
| tests/unit/api/auth/test_controller.py | Removes # noqa: TC* markers from imports. |
| tests/integration/tools/test_sandbox_integration.py | Removes # noqa: TC* markers from imports. |
| tests/integration/tools/conftest.py | Removes # noqa: TC* markers from imports. |
| tests/integration/communication/test_meeting_integration.py | Removes # noqa: TC* markers from imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| dt = datetime.fromisoformat(raw) | ||
| except ValueError, TypeError: | ||
| logger.warning( |
| try: | ||
| config_dict = build_mem0_config_dict(self._mem0_config) | ||
| client = await asyncio.to_thread(Memory.from_config, config_dict) | ||
| except builtins.MemoryError, RecursionError: |
| truncated = total == self._max_memories_per_agent | ||
| if truncated: | ||
| logger.warning( | ||
| MEMORY_ENTRY_COUNTED, | ||
| agent_id=agent_id, |
| MemoryConnectionError: If the backend is not connected. | ||
| MemoryStoreError: If the publish operation fails. | ||
| """ | ||
| self._require_connected() |
![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 comprehensive implementation of the Mem0 memory backend, including the adapter, configuration models, and data mappers. The changes are well-structured, with extensive test coverage and documentation updates. The code demonstrates robust error handling and adherence to the project's design principles.
My main feedback is a critical syntax error in exception handling that needs to be addressed. Specifically, the Python 2-style syntax for catching multiple exceptions is used, which will cause a SyntaxError in Python 3. I've left comments with suggestions to fix this across the new files.
| 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.
The syntax for catching multiple exceptions is incorrect for Python 3 and will cause a SyntaxError. In Python 3, multiple exceptions must be enclosed in a tuple.
This pattern is repeated throughout the file. Please correct it in all occurrences (e.g., lines 225, 368, 432, 505, 593, 667, 746, 824, 911).
except (builtins.MemoryError, RecursionError):| return None | ||
| try: | ||
| dt = datetime.fromisoformat(raw) | ||
| except ValueError, TypeError: |
There was a problem hiding this comment.
The syntax for catching multiple exceptions is incorrect for Python 3 and will cause a SyntaxError. In Python 3, multiple exceptions must be enclosed in a tuple.
This pattern is repeated in this file at lines 104 and 123. Please correct all occurrences.
except (ValueError, TypeError):![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 `@src/ai_company/memory/backends/mem0/adapter.py`:
- Around line 41-48: Consolidate the duplicate imports from
ai_company.memory.errors into a single import statement: replace the two
separate import blocks with one line that imports MemoryConnectionError,
MemoryRetrievalError, MemoryStoreError and aliases MemoryError as
DomainMemoryError so the module only imports from ai_company.memory.errors once.
- Around line 701-727: The publish() method is missing agent validation; call
the existing _validate_agent_id(agent_id) at the start of publish() (same as
store()) to reject reserved/invalid IDs (e.g., "__synthorg_shared__") before
building metadata; ensure this runs before setting _PUBLISHER_KEY so the
publisher field cannot be set to the reserved namespace and ownership/retract
checks remain intact.
- Around line 679-689: The truncation check is using the filtered count
(`total`) rather than the raw query size, so when a category filter is applied
truncation can be missed; update the logic in the block around `truncated =
total == self._max_memories_per_agent` to derive truncation from the
unfiltered/raw query result (e.g., use the length or a “limited” flag returned
by `get_all` / the initial query results) instead of `total`, and log
`MEMORY_ENTRY_COUNTED` with `truncated=True` when that raw result equals
`self._max_memories_per_agent`; keep the `agent_id`, `count` (still the filtered
`total`), `category` and `reason` fields but base `truncated` on `raw_count ==
self._max_memories_per_agent` or an explicit truncation flag.
In `@src/ai_company/memory/backends/mem0/config.py`:
- Around line 103-120: build_mem0_config_dict() currently always emits a local
Qdrant config causing callers that requested "qdrant-external" (allowed by
build_config_from_company_config()) to silently fall back; update
build_mem0_config_dict() to detect when the company config requests
"qdrant-external" and either (a) populate the returned dict with an external
Qdrant provider config including required connection fields from the company
config (host/port/api_key/collection_name/etc.), or (b) if external Qdrant is
not supported yet, raise a clear exception (e.g., ValueError) so callers fail
fast; ensure the change touches the same build_mem0_config_dict() code paths
that build_config_from_company_config() can produce and align the "vector_store"
-> "provider" value with the requested provider string.
In `@src/ai_company/memory/backends/mem0/mappers.py`:
- Around line 89-112: The function normalize_relevance_score currently converts
strings like "nan" or "inf" to float which bypasses the except and then gets
clamped to 0.0/1.0; update normalize_relevance_score to reject non-finite values
by using math.isfinite (or equivalent) after float(score) and, if not finite,
call logger.warning with MEMORY_MODEL_INVALID (field="score", raw_value=score,
reason="non-finite relevance score, returning None") and return None; ensure you
keep the existing ValueError/TypeError handling and final clamping logic. Apply
the same non-finite check and logging pattern to the other analogous function in
this file (the function covering lines 115-131) so both relevance/confidence
normalizers consistently reject NaN/inf.
- Around line 267-294: When created_at is missing we must not default to now()
because that can violate MemoryEntry's timestamp ordering; update the logic in
the mapper around parse_mem0_datetime, parse_mem0_metadata and the MemoryEntry
construction so that if created_at is None you derive it from other available
timestamps (prefer updated_at if parsed, else expires_at from
parse_mem0_metadata, or as last resort datetime.now(UTC)); additionally
normalize/adjust updated_at and expires_at to be >= created_at before calling
MemoryEntry(...) so the model's invariants hold (use parse_mem0_datetime output
for updated_at, the expires_at returned by parse_mem0_metadata, and then
clamp/assign values accordingly).
- Around line 229-294: The mem0_result_to_entry function is doing multiple
responsibilities (ID/content validation, timestamp repair, metadata/score
parsing and model construction); split it into small helpers to keep functions
<50 lines: extract and validate the id (e.g., new helper validate_mem0_id(raw)
returning NotBlankStr id and logging/raising like current block), extract and
validate content (validate_mem0_content(raw) returning NotBlankStr content and
logging/raising), extract timestamps (parse_and_fix_timestamps(raw) that wraps
parse_mem0_datetime for created_at/updated_at and defaults created_at to
datetime.now(UTC) with the same debug log), and extract/normalize score/metadata
(wrap normalize_relevance_score and parse_mem0_metadata if desired); then have
mem0_result_to_entry call these helpers and only construct and return
MemoryEntry, keeping existing symbols (mem0_result_to_entry,
parse_mem0_datetime, parse_mem0_metadata, normalize_relevance_score,
MemoryEntry) and preserving current logging and error behavior.
In `@src/ai_company/memory/factory.py`:
- Around line 94-107: The catch-all except in the Mem0MemoryBackend construction
is swallowing system-level errors; update the error handling so MemoryError and
RecursionError are not wrapped: either add an explicit re-raise for those (e.g.,
if isinstance(exc, (MemoryError, RecursionError)): raise) inside the current
except block, or change to two except clauses (except (MemoryError,
RecursionError): raise; except Exception as exc: ...). Keep the existing
logger.warning and raise MemoryConfigError only for non-fatal exceptions related
to Mem0MemoryBackend construction.
- Around line 26-114: The create_memory_backend function is too long and mixes
dispatching, validation, config building and instantiation for the "mem0" case;
extract the mem0-specific logic into a private helper (e.g.
_create_mem0_backend) that accepts the CompanyMemoryConfig and embedder and
performs the checks currently inside the if config.backend == "mem0" block (type
check for Mem0EmbedderConfig, call build_config_from_company_config, construct
Mem0MemoryBackend, and map exceptions to MemoryConfigError while logging with
MEMORY_BACKEND_CONFIG_INVALID and MEMORY_BACKEND_CREATED); update
create_memory_backend to call this new helper for the "mem0" branch so the
dispatcher remains short and each backend path is isolated.
In `@tests/unit/memory/backends/mem0/test_adapter_shared.py`:
- Around line 83-91: The tests currently reference MemoryError implicitly;
update the assertions and side_effects to explicitly use builtins.MemoryError to
avoid future shadowing: in test_publish_reraises_memory_error set
mock_client.add.side_effect = builtins.MemoryError("out of memory") and assert
pytest.raises(builtins.MemoryError) around backend.publish("test-agent-001",
make_store_request()); apply the same change pattern in
test_search_shared_reraises_memory_error (the mock that triggers backend.search)
and test_retract_reraises_memory_error (the mock that triggers backend.retract)
so all three tests reference builtins.MemoryError explicitly.
🪄 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: f8733413-2ae2-4546-afcd-83fed07e6b12
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (81)
.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/communication/test_meeting_integration.pytests/integration/memory/test_mem0_backend.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/__init__.pytests/unit/memory/backends/mem0/__init__.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/memory/backends/mem0/test_config.pytests/unit/memory/backends/mem0/test_mappers.pytests/unit/memory/test_factory.pytests/unit/memory/test_init.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). (3)
- GitHub Check: Agent
- 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/tools/git/conftest.pytests/unit/api/test_middleware.pytests/unit/hr/test_registry.pysrc/ai_company/observability/events/memory.pytests/unit/budget/test_category_analytics.pytests/unit/communication/conflict_resolution/test_helpers.pytests/unit/api/controllers/test_meetings.pytests/unit/communication/conflict_resolution/test_debate_strategy.pytests/unit/api/test_guards.pytests/unit/tools/git/test_git_sandbox_integration.pytests/unit/engine/test_task_engine_mutations.pytests/unit/api/controllers/test_autonomy.pytests/unit/tools/sandbox/test_protocol.pytests/unit/providers/test_protocol.pytests/unit/communication/conflict_resolution/test_service.pytests/unit/api/controllers/test_providers.pytests/unit/memory/test_factory.pysrc/ai_company/memory/backends/mem0/config.pytests/unit/communication/meeting/test_protocol.pytests/unit/engine/test_agent_engine_lifecycle.pytests/unit/communication/meeting/test_orchestrator.pytests/unit/engine/task_engine_helpers.pytests/unit/communication/meeting/conftest.pytests/unit/api/controllers/test_agents.pytests/unit/api/controllers/test_artifacts.pysrc/ai_company/memory/backends/mem0/mappers.pytests/unit/providers/conftest.pytests/unit/api/controllers/test_departments.pytests/unit/memory/backends/mem0/test_adapter_shared.pytests/unit/api/controllers/test_tasks.pytests/unit/api/auth/test_controller.pysrc/ai_company/memory/factory.pytests/unit/tools/sandbox/conftest.pytests/unit/api/controllers/test_approvals.pytests/unit/engine/test_agent_engine.pytests/unit/communication/meeting/test_round_robin.pytests/unit/engine/test_metrics.pytests/integration/tools/test_sandbox_integration.pytests/unit/hr/test_full_snapshot_strategy.pytests/unit/communication/conflict_resolution/test_authority_strategy.pytests/integration/communication/test_meeting_integration.pytests/unit/memory/backends/mem0/test_adapter_crud.pytests/integration/tools/conftest.pytests/unit/engine/test_loop_protocol.pytests/unit/api/controllers/test_analytics.pytests/unit/api/controllers/test_budget.pytests/unit/api/conftest.pytests/unit/hr/test_onboarding_service.pytests/unit/api/test_app.pytests/unit/engine/test_react_loop.pytests/unit/memory/backends/mem0/test_adapter.pysrc/ai_company/memory/backends/mem0/adapter.pytests/unit/api/controllers/test_projects.pytests/unit/api/test_health.pytests/unit/engine/test_plan_execute_loop.pytests/unit/communication/conflict_resolution/test_hybrid_strategy.pytests/unit/engine/test_context.pytests/integration/memory/test_mem0_backend.pytests/unit/engine/test_agent_engine_errors.pytests/unit/memory/backends/mem0/test_config.pytests/unit/communication/meeting/test_structured_phases.pysrc/ai_company/memory/backends/mem0/__init__.pytests/unit/hr/test_offboarding_service.pytests/unit/memory/test_init.pytests/unit/memory/backends/mem0/test_mappers.pytests/unit/communication/meeting/test_position_papers.pytests/unit/memory/backends/mem0/conftest.pysrc/ai_company/memory/backends/__init__.pytests/unit/api/controllers/test_company.pytests/unit/hr/test_hiring_service.pytests/unit/engine/test_routing_models.pysrc/ai_company/memory/__init__.pytests/unit/api/controllers/test_messages.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/tools/git/conftest.pytests/unit/api/test_middleware.pytests/unit/hr/test_registry.pytests/unit/budget/test_category_analytics.pytests/unit/communication/conflict_resolution/test_helpers.pytests/unit/api/controllers/test_meetings.pytests/unit/communication/conflict_resolution/test_debate_strategy.pytests/unit/api/test_guards.pytests/unit/tools/git/test_git_sandbox_integration.pytests/unit/engine/test_task_engine_mutations.pytests/unit/api/controllers/test_autonomy.pytests/unit/tools/sandbox/test_protocol.pytests/unit/providers/test_protocol.pytests/unit/communication/conflict_resolution/test_service.pytests/unit/api/controllers/test_providers.pytests/unit/memory/test_factory.pytests/unit/communication/meeting/test_protocol.pytests/unit/engine/test_agent_engine_lifecycle.pytests/unit/communication/meeting/test_orchestrator.pytests/unit/engine/task_engine_helpers.pytests/unit/communication/meeting/conftest.pytests/unit/api/controllers/test_agents.pytests/unit/api/controllers/test_artifacts.pytests/unit/providers/conftest.pytests/unit/api/controllers/test_departments.pytests/unit/memory/backends/mem0/test_adapter_shared.pytests/unit/api/controllers/test_tasks.pytests/unit/api/auth/test_controller.pytests/unit/tools/sandbox/conftest.pytests/unit/api/controllers/test_approvals.pytests/unit/engine/test_agent_engine.pytests/unit/communication/meeting/test_round_robin.pytests/unit/engine/test_metrics.pytests/integration/tools/test_sandbox_integration.pytests/unit/hr/test_full_snapshot_strategy.pytests/unit/communication/conflict_resolution/test_authority_strategy.pytests/integration/communication/test_meeting_integration.pytests/unit/memory/backends/mem0/test_adapter_crud.pytests/integration/tools/conftest.pytests/unit/engine/test_loop_protocol.pytests/unit/api/controllers/test_analytics.pytests/unit/api/controllers/test_budget.pytests/unit/api/conftest.pytests/unit/hr/test_onboarding_service.pytests/unit/api/test_app.pytests/unit/engine/test_react_loop.pytests/unit/memory/backends/mem0/test_adapter.pytests/unit/api/controllers/test_projects.pytests/unit/api/test_health.pytests/unit/engine/test_plan_execute_loop.pytests/unit/communication/conflict_resolution/test_hybrid_strategy.pytests/unit/engine/test_context.pytests/integration/memory/test_mem0_backend.pytests/unit/engine/test_agent_engine_errors.pytests/unit/memory/backends/mem0/test_config.pytests/unit/communication/meeting/test_structured_phases.pytests/unit/hr/test_offboarding_service.pytests/unit/memory/test_init.pytests/unit/memory/backends/mem0/test_mappers.pytests/unit/communication/meeting/test_position_papers.pytests/unit/memory/backends/mem0/conftest.pytests/unit/api/controllers/test_company.pytests/unit/hr/test_hiring_service.pytests/unit/engine/test_routing_models.pytests/unit/api/controllers/test_messages.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/observability/events/memory.pysrc/ai_company/memory/backends/mem0/config.pysrc/ai_company/memory/backends/mem0/mappers.pysrc/ai_company/memory/factory.pysrc/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/backends/mem0/__init__.pysrc/ai_company/memory/backends/__init__.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
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/design/memory.mddocs/roadmap/index.md
.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 (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 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 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
📚 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: 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.mddocs/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 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 (36)
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/communication/conflict_resolution/test_helpers.py (1)
src/ai_company/communication/delegation/hierarchy.py (1)
HierarchyResolver(16-277)
tests/unit/communication/conflict_resolution/test_debate_strategy.py (1)
src/ai_company/communication/delegation/hierarchy.py (1)
HierarchyResolver(16-277)
tests/unit/engine/test_task_engine_mutations.py (1)
src/ai_company/core/task.py (1)
Task(45-261)
tests/unit/communication/conflict_resolution/test_service.py (1)
src/ai_company/communication/delegation/hierarchy.py (1)
HierarchyResolver(16-277)
tests/unit/memory/test_factory.py (6)
src/ai_company/memory/backends/mem0/adapter.py (4)
Mem0MemoryBackend(125-930)is_connected(244-246)backend_name(249-251)max_memories_per_agent(281-283)src/ai_company/memory/backends/mem0/config.py (1)
Mem0EmbedderConfig(23-49)src/ai_company/memory/config.py (1)
CompanyMemoryConfig(139-195)src/ai_company/memory/errors.py (1)
MemoryConfigError(40-41)src/ai_company/memory/factory.py (1)
create_memory_backend(26-120)src/ai_company/memory/capabilities.py (1)
max_memories_per_agent(54-56)
src/ai_company/memory/backends/mem0/config.py (2)
src/ai_company/memory/config.py (1)
CompanyMemoryConfig(139-195)src/ai_company/observability/_logger.py (1)
get_logger(8-28)
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/engine/task_engine_helpers.py (1)
src/ai_company/core/task.py (1)
Task(45-261)
tests/unit/api/controllers/test_agents.py (1)
src/ai_company/api/auth/service.py (1)
AuthService(36-245)
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/api/auth/test_controller.py (1)
src/ai_company/api/auth/service.py (1)
AuthService(36-245)
src/ai_company/memory/factory.py (3)
src/ai_company/memory/config.py (1)
CompanyMemoryConfig(139-195)src/ai_company/memory/backends/mem0/config.py (2)
Mem0EmbedderConfig(23-49)build_config_from_company_config(124-172)src/ai_company/memory/backends/mem0/adapter.py (1)
max_memories_per_agent(281-283)
tests/unit/engine/test_agent_engine.py (1)
src/ai_company/core/agent.py (1)
AgentIdentity(266-342)
tests/unit/communication/meeting/test_round_robin.py (1)
src/ai_company/communication/meeting/models.py (1)
MeetingAgenda(77-96)
tests/unit/hr/test_full_snapshot_strategy.py (1)
src/ai_company/memory/consolidation/models.py (1)
ArchivalEntry(56-91)
tests/unit/communication/conflict_resolution/test_authority_strategy.py (1)
src/ai_company/communication/delegation/hierarchy.py (1)
HierarchyResolver(16-277)
tests/integration/communication/test_meeting_integration.py (1)
src/ai_company/communication/meeting/protocol.py (1)
MeetingProtocol(61-100)
tests/unit/engine/test_loop_protocol.py (1)
src/ai_company/engine/context.py (1)
AgentContext(87-307)
tests/unit/api/controllers/test_budget.py (2)
src/ai_company/budget/cost_record.py (1)
CostRecord(16-57)src/ai_company/budget/tracker.py (1)
CostTracker(68-455)
tests/unit/hr/test_onboarding_service.py (1)
tests/unit/hr/conftest.py (2)
onboarding_service(180-182)registry(174-176)
tests/unit/memory/backends/mem0/test_adapter.py (6)
src/ai_company/memory/backends/mem0/adapter.py (20)
Mem0MemoryBackend(125-930)backend_name(249-251)is_connected(244-246)supported_categories(256-258)supports_graph(261-263)supports_temporal(266-268)supports_vector_search(271-273)supports_shared_access(276-278)max_memories_per_agent(281-283)connect(149-189)disconnect(191-200)health_check(202-241)store(329-386)retrieve(388-449)get(451-524)delete(526-612)count(614-697)publish(701-763)search_shared(765-842)retract(844-930)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/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)
src/ai_company/memory/backends/mem0/adapter.py (2)
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 (7)
build_mem0_metadata(38-57)extract_category(436-458)extract_publisher(461-474)mem0_result_to_entry(229-294)query_to_mem0_getall_args(329-345)query_to_mem0_search_args(297-326)validate_add_result(393-433)
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_hybrid_strategy.py (1)
src/ai_company/communication/delegation/hierarchy.py (1)
HierarchyResolver(16-277)
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/memory/test_mem0_backend.py (4)
src/ai_company/memory/backends/mem0/config.py (2)
Mem0BackendConfig(52-91)Mem0EmbedderConfig(23-49)src/ai_company/memory/models.py (2)
MemoryQuery(153-230)MemoryStoreRequest(55-79)src/ai_company/memory/retrieval_config.py (1)
MemoryRetrievalConfig(20-132)src/ai_company/memory/retriever.py (1)
ContextInjectionStrategy(97-400)
tests/unit/engine/test_agent_engine_errors.py (2)
src/ai_company/core/agent.py (1)
AgentIdentity(266-342)src/ai_company/core/task.py (1)
Task(45-261)
tests/unit/memory/backends/mem0/test_config.py (4)
src/ai_company/memory/backends/mem0/config.py (4)
Mem0BackendConfig(52-91)Mem0EmbedderConfig(23-49)build_config_from_company_config(124-172)build_mem0_config_dict(94-121)src/ai_company/memory/config.py (2)
CompanyMemoryConfig(139-195)MemoryStorageConfig(25-104)tests/integration/memory/test_mem0_backend.py (1)
backend(42-51)tests/unit/memory/backends/mem0/conftest.py (1)
mem0_config(26-31)
tests/unit/communication/meeting/test_structured_phases.py (1)
src/ai_company/communication/meeting/models.py (1)
MeetingAgenda(77-96)
src/ai_company/memory/backends/mem0/__init__.py (1)
src/ai_company/memory/backends/mem0/config.py (2)
Mem0BackendConfig(52-91)Mem0EmbedderConfig(23-49)
tests/unit/communication/meeting/test_position_papers.py (1)
src/ai_company/communication/meeting/models.py (1)
MeetingAgenda(77-96)
src/ai_company/memory/backends/__init__.py (1)
src/ai_company/memory/backends/mem0/adapter.py (1)
Mem0MemoryBackend(125-930)
tests/unit/hr/test_hiring_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/engine/test_routing_models.py (1)
src/ai_company/core/agent.py (1)
AgentIdentity(266-342)
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(125-930)
🔇 Additional comments (89)
tests/integration/communication/test_meeting_integration.py (1)
29-30: Import cleanup looks correct.
AgentCallerandMeetingProtocolare still actively used in type annotations, so removing inline suppression here is safe and keeps the test clean.tests/unit/engine/test_agent_engine_lifecycle.py (1)
9-11: Import cleanup looks good.Removing the inline lint suppressions here is safe; both imported symbols are still actively used via test type annotations.
tests/unit/communication/meeting/conftest.py (1)
16-16: Import cleanup looks good.
AgentCalleris used by the function annotation, so keeping this direct import withoutnoqais correct.tests/integration/tools/test_sandbox_integration.py (1)
4-4: Good cleanup: removing the inline lint suppression is safe here.
Pathis still used at runtime in this module, so this change improves clarity without behavior impact.tests/unit/engine/test_metrics.py (1)
6-6: LGTM: inline lint-suppression removal is a clean non-functional improvement.
No behavior change here, and the import remains clearly declared for typed test signatures.As per coding guidelines,
tests/**/*.py: "Use@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowfor test markers" — this change does not violate those requirements.tests/unit/engine/test_agent_engine_errors.py (1)
8-10: Import cleanup is safe and improves lint hygiene.Line 8 and Line 10 only remove redundant inline suppression; symbol resolution and test behavior remain unchanged.
tests/unit/engine/test_context.py (1)
9-11: Import cleanup is safe and clear.These import changes are non-functional and remain consistent with how
AgentIdentityandTaskare used in annotations across this test module.tests/integration/tools/conftest.py (1)
5-5: LGTM: Formatting cleanupRemoving the unnecessary
noqacomment is appropriate and improves code cleanliness.tests/unit/engine/test_loop_protocol.py (1)
9-9: LGTM! Clean lint cleanup.Removing the
# noqa: TC001comment is appropriate here sinceAgentContextis used at runtime in test fixtures and method signatures, not just in type annotations.tests/unit/budget/test_category_analytics.py (1)
15-15: LGTM! Cleanup is correct.The removal of the
# noqa: TC001suppression is appropriate.CostTrackeris genuinely used at runtime in fixture parameters throughout the test methods, so the type-checking-only import warning (TC001) should not apply.tests/unit/communication/meeting/test_position_papers.py (1)
13-13: LGTM!The
noqa: TC001suppression removal is appropriate.MeetingAgendais used at runtime in type annotations for test method parameters, so the import correctly belongs at module level.tests/unit/communication/meeting/test_orchestrator.py (1)
31-31: LGTM!The
noqa: TC001suppression removal is correct.MeetingProtocolis used at runtime in theregistrydict type annotation (lines 48, 418), so the import properly belongs at module level.tests/unit/communication/meeting/test_structured_phases.py (1)
13-13: LGTM!The
noqa: TC001suppression removal is appropriate.MeetingAgendais used at runtime in test method type annotations throughout the file.tests/unit/communication/meeting/test_protocol.py (1)
6-9: LGTM!The
noqa: TC001suppression removal is correct. BothMeetingAgendaandMeetingMinutesare used at runtime in the_MockProtocolclass definition (lines 34, 39).tests/unit/communication/meeting/test_round_robin.py (1)
13-13: LGTM!The
noqa: TC001suppression removal is appropriate.MeetingAgendais used at runtime in test method type annotations throughout the file.tests/unit/api/auth/test_controller.py (1)
6-6: LGTM! Appropriate cleanup of unnecessary lint suppressions.Removing the
# noqa: TC001and# noqa: TC002comments is correct. With Python 3.14's PEP 649 lazy annotation evaluation, these imports no longer need TYPE_CHECKING guards or suppressions, as both are legitimately used at runtime.Also applies to: 47-47
tests/unit/providers/test_protocol.py (1)
3-3: LGTM! Lint suppression cleanup is appropriate.The removal of the
# noqa: TC003suppression aligns with Python 3.14's PEP 649 native lazy annotations. SinceAsyncIteratoris used only in type annotations and the coding guidelines discouragefrom __future__ import annotations, this import pattern is correct.tests/unit/hr/test_registry.py (1)
7-7: Import change is behavior-neutral.
AgentRegistryServiceis still used by the test parameter annotations throughout this module, so this cleanup keeps the file simpler without affecting execution.tests/unit/hr/test_onboarding_service.py (1)
8-9: Import change is behavior-neutral.These types are still referenced by the test parameter annotations in this module, so the cleanup does not change runtime behavior.
tests/unit/hr/test_offboarding_service.py (1)
8-9: Import change is behavior-neutral.These symbols are still used by the fake collaborator signatures and test parameter annotations here, so this cleanup does not affect execution.
Also applies to: 11-11, 22-22
tests/unit/hr/test_full_snapshot_strategy.py (1)
12-12: Import change is behavior-neutral.
ArchivalEntryis still used by the fake archival-store helpers in this module, so this cleanup is safe.tests/unit/hr/test_hiring_service.py (1)
15-16: Import change is behavior-neutral.
OnboardingServiceandAgentRegistryServiceare still referenced by the test parameter annotations in this module, so this cleanup is safe.tests/unit/tools/git/test_git_sandbox_integration.py (1)
3-3: No concerns with this import cleanup.
Pathis still referenced in the test signatures, so removing the inline suppression is non-behavioral.tests/unit/api/controllers/test_messages.py (1)
6-6: No concerns with this import cleanup.
TestClientis still referenced throughout the module's fixture annotations, so this remains non-behavioral.tests/unit/api/test_middleware.py (1)
6-6: No concerns with this import cleanup.
TestClientis still used by the test signatures, so dropping the inline suppression does not affect behavior.tests/unit/api/controllers/test_artifacts.py (1)
6-6: No concerns with this import cleanup.
TestClientremains referenced by the test signatures, so this change is non-behavioral.tests/unit/engine/test_task_engine_mutations.py (1)
8-8: No concerns with this import cleanup.
Taskis still referenced later in the module, so removing the inline suppression does not change behavior.tests/unit/api/test_health.py (1)
6-6: No concerns with this import cleanup.
TestClientis still used in the test signatures, so this remains a non-functional cleanup.tests/unit/engine/task_engine_helpers.py (1)
6-6: No concerns with this import cleanup.
Taskis still referenced by the helper annotations in this module, so the change is non-behavioral.tests/unit/api/controllers/test_providers.py (1)
6-6: No concerns with this import cleanup.
TestClientremains referenced by the test signatures, so removing the inline suppression does not affect behavior.tests/unit/api/controllers/test_agents.py (1)
31-31: Import cleanup is safe and clear.No functional concerns; this is a clean lint-oriented change.
tests/unit/api/controllers/test_budget.py (1)
7-10: LGTM — import-only cleanup.These changes are non-functional and keep the test module consistent.
tests/unit/api/controllers/test_projects.py (1)
6-6: Looks good.No issues with this import cleanup change.
tests/unit/api/test_guards.py (1)
6-6: Approved — no functional impact.This import-line cleanup is safe.
tests/unit/api/test_app.py (1)
7-7: LGTM.Import cleanup is straightforward and safe.
tests/unit/providers/conftest.py (1)
3-3: Good change.The import adjustment is clean and does not affect behavior.
tests/unit/api/controllers/test_tasks.py (1)
6-6: No concerns here.This is a safe import-line cleanup.
tests/unit/api/controllers/test_analytics.py (1)
6-6: Approved.Import cleanup is non-functional and consistent with the rest of the test suite.
tests/unit/engine/test_plan_execute_loop.py (1)
9-9: Import cleanup is safe and consistent.Line 9 keeps the needed symbol for typing and does not alter test behavior.
tests/unit/engine/test_react_loop.py (1)
8-8: Looks good — no functional impact.Line 8 is a clean import-only change and preserves existing typed test usage.
tests/unit/api/controllers/test_company.py (1)
6-6: Import change is clean and non-breaking.Line 6 keeps the same dependency surface and test signatures remain valid.
tests/unit/api/conftest.py (1)
5-5: Import updates are fine.These line-level import changes are consistent with current usage in the file and do not affect fixture behavior.
Also applies to: 18-18, 20-21, 31-33, 38-39
tests/unit/tools/sandbox/conftest.py (1)
3-3: Good cleanup.Line 3 remains correct for the typed fixture signatures and introduces no behavior change.
tests/unit/api/controllers/test_departments.py (1)
6-6: No concerns on this change.Line 6 is a straightforward import cleanup with unchanged test behavior.
tests/unit/communication/conflict_resolution/test_authority_strategy.py (1)
12-12: Approved — formatting-only import adjustment.Line 12 preserves the same symbol usage and does not alter test semantics.
tests/unit/communication/conflict_resolution/test_service.py (1)
23-23: Looks good.Line 23 is a safe import cleanup and keeps all typed test interfaces intact.
tests/unit/communication/conflict_resolution/test_hybrid_strategy.py (1)
17-19: LGTM!The inline
noqa: TC001suppression is correctly removed since TC rules are now globally ignored for test files viapyproject.toml.tests/unit/communication/conflict_resolution/test_helpers.py (1)
11-13: LGTM!Inline suppression removal aligns with the global TC ignore for test files.
tests/unit/communication/conflict_resolution/test_debate_strategy.py (1)
14-16: LGTM!Inline suppression removal aligns with the global TC ignore for test files.
tests/unit/api/controllers/test_meetings.py (1)
6-6: LGTM!Inline suppression removal aligns with the global TC ignore for test files.
tests/unit/api/controllers/test_autonomy.py (1)
6-6: LGTM!Inline suppression removal aligns with the global TC ignore for test files.
tests/unit/tools/git/conftest.py (1)
5-5: LGTM!Inline suppression removal aligns with the global TC ignore for test files.
pyproject.toml (3)
138-138: LGTM — TC ignore for test files is appropriate.Adding
"TC"(flake8-type-checking) to test file ignores is a sensible choice since test files commonly use type hints for fixtures which don't benefit fromTYPE_CHECKINGblocks. This eliminates the need for scattered inlinenoqa: TC001/TC002comments throughout the test suite.
189-192: LGTM — mypy override for mem0.The
ignore_missing_imports = trueoverride formem0.*is appropriate since the library lacks type stubs, consistent with handling of other untyped dependencies (litellm, aiodocker, etc.).
24-24: Dependencymem0ai==1.0.5is valid and correctly pinned.The version exists on PyPI and is the latest available release. The dependency adheres to the coding guidelines with exact version pinning using
==.tests/unit/engine/test_routing_models.py (1)
5-5: LGTM!Inline suppression removal aligns with the global TC ignore for test files.
README.md (1)
130-130: LGTM!The status update accurately reflects the completion of the Mem0 backend adapter implementation.
CLAUDE.md (2)
97-97: LGTM!Package structure documentation accurately reflects the new Mem0 backend adapter path.
208-209: LGTM!Dependency documentation correctly notes
mem0aias a required dependency for the memory backend.src/ai_company/memory/backends/__init__.py (1)
1-5: LGTM!Clean re-export module with proper docstring and
__all__definition. The structure follows the pattern for exposing concrete backend implementations.docs/design/memory.md (2)
33-33: LGTM!Status update correctly reflects Mem0 as implemented.
286-293: LGTM!Helpful guidance on programmatic embedder configuration via the factory. The example uses generic placeholder names appropriately.
tests/unit/memory/test_init.py (1)
19-20: LGTM!Test correctly updated to expect the new
Mem0EmbedderConfigandMem0MemoryBackendexports.src/ai_company/memory/__init__.py (2)
1-15: LGTM!Clean re-export of Mem0 components with updated docstring reflecting the new concrete backend.
82-83: LGTM!
__all__correctly updated with new exports in alphabetical order.src/ai_company/memory/backends/mem0/__init__.py (1)
1-6: LGTM!Clean package initializer with proper re-exports of Mem0 backend components. The
__all__correctly exposes the public API.tests/unit/memory/backends/mem0/test_adapter_shared.py (4)
1-23: LGTM!Well-structured test file with comprehensive coverage of shared knowledge store operations. Good use of fixtures from conftest.
30-92: Good coverage of publish error paths.Tests cover success, empty results, exception wrapping, missing ID, and MemoryError propagation. Well-structured assertions verify both return values and mock call arguments.
97-226: Thorough search_shared test coverage.Good coverage including text vs no-text queries, agent exclusion filtering, exception wrapping, and the edge case where entries lack publisher metadata (falling back to shared namespace).
231-327: Comprehensive retract test coverage.Tests cover ownership validation, not-found handling, exception wrapping in both get and delete phases. The ownership mismatch and no-publisher-raises tests properly verify the security constraints.
tests/integration/memory/test_mem0_backend.py (1)
54-288: Solid end-to-end integration coverage.This suite validates the critical Mem0 flows (personal retrieval pipeline + shared knowledge lifecycle) with clear assertions and realistic payload shapes.
tests/unit/memory/test_factory.py (1)
29-94: Factory path coverage is strong.The tests exercise both the happy path and key failure modes (missing/wrong embedder, config-build failure, backend init failure, unknown backend guard).
tests/unit/memory/backends/mem0/test_adapter.py (1)
25-346: Comprehensive protocol/lifecycle test surface.Great coverage of backend contract conformance plus connect/disconnect/health and disconnected-operation guards.
tests/unit/memory/backends/mem0/test_config.py (1)
27-229: Config validation coverage looks complete.The suite hits defaults, immutability, traversal defense, and unsupported storage combinations with clear assertions.
tests/unit/memory/backends/mem0/test_mappers.py (1)
60-577: Excellent mapper edge-case coverage.This file thoroughly exercises parsing, normalization, filtering, and validation branches, including malformed inputs and boundary conditions.
tests/unit/memory/backends/mem0/test_adapter_crud.py (1)
31-545: Strong CRUD and guard-rail coverage.The tests validate both normal operation and important protections (ownership checks, shared namespace restrictions, error wrapping/reraising).
tests/unit/memory/backends/mem0/conftest.py (1)
17-117: Fixture scaffolding is clean and practical.Good centralization of backend/mocked-client setup and canonical Mem0 payload builders for cross-test consistency.
src/ai_company/memory/backends/mem0/adapter.py (12)
91-123: LGTM!Well-structured validation helper with proper type hints, docstring, and error handling. The defensive checks for response structure are appropriate.
125-145: LGTM!Clean class initialization with proper docstrings, type hints, and use of keyword-only arguments.
149-189: LGTM!The
connect()method properly handles idempotency, imports the Mem0 SDK lazily, and wraps initialization errors appropriately. Good use of structured logging for state transitions.
202-241: LGTM!The
health_check()implementation correctly probes the backend with a lightweight call and handles exceptions gracefully, returningFalseon failure rather than propagating errors.
253-283: LGTM!Capabilities properties are correctly implemented with appropriate return types and documentation.
285-325: LGTM!Well-designed guard methods with parameterized error types for different operation contexts. The
_validate_agent_idmethod appropriately prevents namespace collision attacks.
329-386: LGTM!The
store()method correctly validates connection and agent ID, constructs the Mem0 payload appropriately, and handles errors with proper logging.
388-449: LGTM!The
retrieve()method correctly branches between semantic search and non-semantic retrieval based on query parameters, with proper post-filtering applied.
451-524: LGTM!The
get()method correctly verifies ownership before returning the entry, and appropriately returnsNonefor ownership mismatches rather than raising an error.
526-612: LGTM!The
delete()method properly enforces ownership verification and correctly blocks deletion of shared-namespace entries with a helpful error message directing users toretract().
765-842: LGTM!The
search_shared()method correctly searches the shared namespace and properly handles the optionalexclude_agentfiltering. Good use ofextract_publisherto attribute entries to their original publishers.
844-930: LGTM!The
retract()method properly verifies publisher ownership before deletion and distinguishes between non-shared entries (missing publisher) and ownership mismatches with appropriate error messages.
| from ai_company.memory.errors import ( | ||
| MemoryConnectionError, | ||
| MemoryRetrievalError, | ||
| MemoryStoreError, | ||
| ) | ||
| from ai_company.memory.errors import ( | ||
| MemoryError as DomainMemoryError, | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consolidate duplicate imports from the same module.
These two import statements from ai_company.memory.errors can be merged into one for cleaner organization.
♻️ Proposed consolidation
-from ai_company.memory.errors import (
- MemoryConnectionError,
- MemoryRetrievalError,
- MemoryStoreError,
-)
-from ai_company.memory.errors import (
- MemoryError as DomainMemoryError,
-)
+from ai_company.memory.errors import (
+ MemoryConnectionError,
+ MemoryError as DomainMemoryError,
+ MemoryRetrievalError,
+ MemoryStoreError,
+)🤖 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 41 - 48,
Consolidate the duplicate imports from ai_company.memory.errors into a single
import statement: replace the two separate import blocks with one line that
imports MemoryConnectionError, MemoryRetrievalError, MemoryStoreError and
aliases MemoryError as DomainMemoryError so the module only imports from
ai_company.memory.errors once.
| 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", |
There was a problem hiding this comment.
qdrant-external is accepted but never actually configured.
build_config_from_company_config() allows qdrant-external, but build_mem0_config_dict() always emits a local-path Qdrant config. A caller that selects an external vector store will silently fall back to embedded storage instead of failing fast.
🛠️ Minimal fix if external Qdrant is not supported yet
- 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 only supports embedded qdrant vector stores, "
f"got {config.storage.vector_store!r}"
)Also applies to: 143-155
🤖 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 103 - 120,
build_mem0_config_dict() currently always emits a local Qdrant config causing
callers that requested "qdrant-external" (allowed by
build_config_from_company_config()) to silently fall back; update
build_mem0_config_dict() to detect when the company config requests
"qdrant-external" and either (a) populate the returned dict with an external
Qdrant provider config including required connection fields from the company
config (host/port/api_key/collection_name/etc.), or (b) if external Qdrant is
not supported yet, raise a clear exception (e.g., ValueError) so callers fail
fast; ensure the change touches the same build_mem0_config_dict() code paths
that build_config_from_company_config() can produce and align the "vector_store"
-> "provider" value with the requested provider string.
| def mem0_result_to_entry( | ||
| raw: dict[str, Any], | ||
| agent_id: str, | ||
| ) -> MemoryEntry: | ||
| """Convert a single Mem0 result dict to a ``MemoryEntry``. | ||
|
|
||
| Args: | ||
| raw: Single result dict from Mem0 (``search``, ``get``, or | ||
| ``get_all``). | ||
| agent_id: Owning agent identifier. | ||
|
|
||
| Returns: | ||
| Domain ``MemoryEntry``. | ||
| """ | ||
| 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, | ||
| content=content, | ||
| metadata=metadata, | ||
| created_at=created_at, | ||
| updated_at=updated_at, | ||
| expires_at=expires_at, | ||
| relevance_score=relevance_score, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Split mem0_result_to_entry() into smaller helpers.
This mapper is already handling raw-field validation, timestamp repair, metadata parsing, and domain-model construction in one place. Extracting the timestamp and raw-field normalization logic will make the Mem0 boundary easier to test and keep the function within the repository limit. As per coding guidelines, Functions: < 50 lines.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/memory/backends/mem0/mappers.py` around lines 229 - 294, The
mem0_result_to_entry function is doing multiple responsibilities (ID/content
validation, timestamp repair, metadata/score parsing and model construction);
split it into small helpers to keep functions <50 lines: extract and validate
the id (e.g., new helper validate_mem0_id(raw) returning NotBlankStr id and
logging/raising like current block), extract and validate content
(validate_mem0_content(raw) returning NotBlankStr content and logging/raising),
extract timestamps (parse_and_fix_timestamps(raw) that wraps parse_mem0_datetime
for created_at/updated_at and defaults created_at to datetime.now(UTC) with the
same debug log), and extract/normalize score/metadata (wrap
normalize_relevance_score and parse_mem0_metadata if desired); then have
mem0_result_to_entry call these helpers and only construct and return
MemoryEntry, keeping existing symbols (mem0_result_to_entry,
parse_mem0_datetime, parse_mem0_metadata, normalize_relevance_score,
MemoryEntry) and preserving current logging and error behavior.
…and factory - Add Mem0Client protocol (TYPE_CHECKING) for structural typing documentation - Add asyncio.Lock + double-check locking to prevent concurrent connect() - Expand docstrings: adapter, config, design spec protocol snippets - Replace inline generator in search_shared with explicit loop + publisher fallback logging - Narrow factory exception catch to (ValueError, ValidationError) - Add YAML enum value annotations in design spec - Update CLAUDE.md logging event domains - Remove unused event constants (MEMORY_BACKEND_NOT_IMPLEMENTED, MEMORY_CAPABILITY_UNSUPPORTED) - Rename _PUBLISHER_KEY → PUBLISHER_KEY (public constant) - Add tests: RecursionError propagation, _validate_mem0_result, _coerce_confidence, _normalize_tags, count truncation warning, category post-filter, publisher fallback
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
src/ai_company/memory/backends/mem0/adapter.py (1)
41-48: 🧹 Nitpick | 🔵 TrivialConsolidate duplicate imports from the same module.
These two import statements from
ai_company.memory.errorscan be merged into one for cleaner organization.🤖 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 41 - 48, Consolidate the duplicate imports from ai_company.memory.errors into a single import statement: replace the two separate import blocks with one import that imports MemoryConnectionError, MemoryRetrievalError, MemoryStoreError and aliases MemoryError as DomainMemoryError so the symbols (MemoryConnectionError, MemoryRetrievalError, MemoryStoreError, MemoryError as DomainMemoryError) are all imported from ai_company.memory.errors in one line.tests/unit/memory/backends/mem0/test_adapter_shared.py (1)
83-91: 🧹 Nitpick | 🔵 TrivialConsider explicit
builtins.MemoryErrorreference for clarity.The test correctly verifies that
builtins.MemoryErroris re-raised without wrapping. While the current code works (no domainMemoryErroris imported), usingbuiltins.MemoryErrorexplicitly would prevent confusion if the domain error is later imported.🤖 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_shared.py` around lines 83 - 91, Update the test to reference the builtin MemoryError explicitly: in test_publish_reraises_memory_error, set mock_client.add.side_effect = builtins.MemoryError("out of memory") and assert with pytest.raises(builtins.MemoryError) when awaiting backend.publish("test-agent-001", make_store_request()); this clarifies the intent and avoids confusion if a domain-level MemoryError is later introduced (locate the changes in test_publish_reraises_memory_error, mock_client.add, and the pytest.raises assertion).src/ai_company/memory/backends/mem0/mappers.py (2)
90-113:⚠️ Potential issue | 🟡 MinorReject
NaNandinfbefore clamping.
float("nan")andfloat("inf")bypass theexceptblocks and themax/minclamp then silently converts corrupt input into a boundary value (0.0 or 1.0). This can inflate bad Mem0 scores to1.0and distort ranking/filtering.🛡️ Proposed fix
+import math from datetime import UTC, datetime ... def normalize_relevance_score(score: Any) -> float | None: ... try: numeric = float(score) except ValueError, TypeError: logger.warning(...) return None + if not math.isfinite(numeric): + logger.warning( + MEMORY_MODEL_INVALID, + field="score", + raw_value=score, + reason="non-finite relevance score, returning None", + ) + return None return max(0.0, min(1.0, numeric))The same guard should be applied to
_coerce_confidence(lines 116-135).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ai_company/memory/backends/mem0/mappers.py` around lines 90 - 113, normalize_relevance_score currently allows float("nan") and float("inf") to pass through and get clamped to 0.0/1.0; change the function to reject non-finite values by checking numeric with math.isfinite (or equivalent) after conversion, log using MEMORY_MODEL_INVALID with field="score" and raw_value=score and return None if not finite, then proceed to clamp; apply the same finite-check + logging/None-return pattern to the helper _coerce_confidence (use field="confidence" and raw_value as appropriate) so NaN/inf are consistently rejected before clamping.
272-280:⚠️ Potential issue | 🟡 MinorConsider deriving
created_atfallback from available timestamps.When Mem0 omits
created_atbut providesupdated_atorexpires_at, defaulting tonow()can violateMemoryEntry's timestamp ordering constraints (updated_at >= created_at,expires_at >= created_at). Deriving the fallback fromupdated_at(if available) or ensuring consistency before model construction would be safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ai_company/memory/backends/mem0/mappers.py` around lines 272 - 280, The fallback for created_at should be derived from any available timestamps instead of blindly using now(); parse updated_at and expires_at with parse_mem0_datetime, and if created_at is None set created_at = the minimum (earliest) of the non-None values among parsed updated_at, parsed expires_at, and datetime.now(UTC) so that created_at <= updated_at and created_at <= expires_at; update the MEMORY_MODEL_INVALID log to reflect that created_at was derived from other timestamps when applicable and keep using the same variables (created_at, parsed_updated_at, parsed_expires_at, parse_mem0_datetime) and the MemoryEntry construction unchanged.src/ai_company/memory/backends/mem0/config.py (1)
152-181:⚠️ Potential issue | 🟠 MajorDo not accept
qdrant-externaluntil it is actually serialized.
build_config_from_company_config()currently allows"qdrant-external", butbuild_mem0_config_dict()still always emits a localpath-based Qdrant config. That silently changes the requested storage mode instead of failing fast.Minimal fail-fast 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 only supports embedded qdrant vector stores, " f"got {config.storage.vector_store!r}" )🤖 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 152 - 181, The validation in build_config_from_company_config() wrongly accepts "qdrant-external" even though build_mem0_config_dict() only emits local path-based Qdrant configs; change the allowed vector_store check to only permit "qdrant" (remove "qdrant-external") so the function fails fast with the existing logger.warning + ValueError path when an unsupported value is supplied; ensure the change occurs in the validation block that raises ValueError and references config.storage.vector_store and Mem0BackendConfig.
🤖 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/backends/mem0/config.py`:
- Around line 26-30: The docstring in
src/ai_company/memory/backends/mem0/config.py that shows example identifiers for
the required parameters provider and model uses vendor-specific examples
("openai", "text-embedding-ada-002"); update those examples to the repository's
generic placeholders (e.g., use "example-provider" for provider and
"example-medium-001" for model) so the docstring for the required parameters
provider and model references only allowed generic identifiers per the coding
guidelines.
In `@tests/unit/memory/backends/mem0/test_mappers.py`:
- Around line 449-504: The tests under TestValidateAddResult repeat many failure
cases that only vary the input result and expected regex; replace those
individual failure test methods with a single `@pytest.mark.parametrize` test that
iterates tuples of (result, expected_regex) and asserts
validate_add_result(result, context="test") raises MemoryStoreError matching the
regex; keep the existing positive tests (test_valid_result and
test_numeric_id_coerced_to_string) as-is, and include the special non-dict case
("not-a-dict") and non-dict-first-item ({"results": ["not-a-dict"]}) in the
param table so all failure scenarios (missing/malformed results, missing/blank
id, unexpected type, non-dict first item) are covered by the one parameterized
test calling validate_add_result.
---
Duplicate comments:
In `@src/ai_company/memory/backends/mem0/adapter.py`:
- Around line 41-48: Consolidate the duplicate imports from
ai_company.memory.errors into a single import statement: replace the two
separate import blocks with one import that imports MemoryConnectionError,
MemoryRetrievalError, MemoryStoreError and aliases MemoryError as
DomainMemoryError so the symbols (MemoryConnectionError, MemoryRetrievalError,
MemoryStoreError, MemoryError as DomainMemoryError) are all imported from
ai_company.memory.errors in one line.
In `@src/ai_company/memory/backends/mem0/config.py`:
- Around line 152-181: The validation in build_config_from_company_config()
wrongly accepts "qdrant-external" even though build_mem0_config_dict() only
emits local path-based Qdrant configs; change the allowed vector_store check to
only permit "qdrant" (remove "qdrant-external") so the function fails fast with
the existing logger.warning + ValueError path when an unsupported value is
supplied; ensure the change occurs in the validation block that raises
ValueError and references config.storage.vector_store and Mem0BackendConfig.
In `@src/ai_company/memory/backends/mem0/mappers.py`:
- Around line 90-113: normalize_relevance_score currently allows float("nan")
and float("inf") to pass through and get clamped to 0.0/1.0; change the function
to reject non-finite values by checking numeric with math.isfinite (or
equivalent) after conversion, log using MEMORY_MODEL_INVALID with field="score"
and raw_value=score and return None if not finite, then proceed to clamp; apply
the same finite-check + logging/None-return pattern to the helper
_coerce_confidence (use field="confidence" and raw_value as appropriate) so
NaN/inf are consistently rejected before clamping.
- Around line 272-280: The fallback for created_at should be derived from any
available timestamps instead of blindly using now(); parse updated_at and
expires_at with parse_mem0_datetime, and if created_at is None set created_at =
the minimum (earliest) of the non-None values among parsed updated_at, parsed
expires_at, and datetime.now(UTC) so that created_at <= updated_at and
created_at <= expires_at; update the MEMORY_MODEL_INVALID log to reflect that
created_at was derived from other timestamps when applicable and keep using the
same variables (created_at, parsed_updated_at, parsed_expires_at,
parse_mem0_datetime) and the MemoryEntry construction unchanged.
In `@tests/unit/memory/backends/mem0/test_adapter_shared.py`:
- Around line 83-91: Update the test to reference the builtin MemoryError
explicitly: in test_publish_reraises_memory_error, set
mock_client.add.side_effect = builtins.MemoryError("out of memory") and assert
with pytest.raises(builtins.MemoryError) when awaiting
backend.publish("test-agent-001", make_store_request()); this clarifies the
intent and avoids confusion if a domain-level MemoryError is later introduced
(locate the changes in test_publish_reraises_memory_error, mock_client.add, and
the pytest.raises assertion).
🪄 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: e3024281-6bf4-45f9-92e4-626904b38a84
📒 Files selected for processing (14)
CLAUDE.mddocs/design/memory.mdsrc/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/memory/models.pysrc/ai_company/observability/events/memory.pytests/integration/memory/test_mem0_backend.pytests/unit/memory/backends/mem0/test_adapter_crud.pytests/unit/memory/backends/mem0/test_adapter_shared.pytests/unit/memory/backends/mem0/test_mappers.pytests/unit/memory/test_factory.pytests/unit/observability/test_events.py
💤 Files with no reviewable changes (1)
- tests/unit/observability/test_events.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 (4)
**/*.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/observability/events/memory.pytests/integration/memory/test_mem0_backend.pytests/unit/memory/test_factory.pysrc/ai_company/memory/factory.pytests/unit/memory/backends/mem0/test_adapter_shared.pytests/unit/memory/backends/mem0/test_adapter_crud.pysrc/ai_company/memory/backends/mem0/config.pysrc/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/models.pytests/unit/memory/backends/mem0/test_mappers.pysrc/ai_company/memory/backends/mem0/mappers.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/observability/events/memory.pysrc/ai_company/memory/factory.pysrc/ai_company/memory/backends/mem0/config.pysrc/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/models.pysrc/ai_company/memory/backends/mem0/mappers.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/integration/memory/test_mem0_backend.pytests/unit/memory/test_factory.pytests/unit/memory/backends/mem0/test_adapter_shared.pytests/unit/memory/backends/mem0/test_adapter_crud.pytests/unit/memory/backends/mem0/test_mappers.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/design/memory.md
🧠 Learnings (11)
📚 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.mddocs/design/memory.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:
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 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:
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 src/**/*.py : Always use structured logging kwargs: `logger.info(EVENT, key=value)` — never `logger.info("msg %s", val)`
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 src/**/*.py : Never use `import logging` / `logging.getLogger()` / `print()` in application code
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 src/**/*.py : Always use `logger` as the variable name (not `_logger`, not `log`)
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 src/**/*.py : All error paths must log at WARNING or ERROR with context before raising
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 src/**/*.py : All state transitions must log at INFO
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
📚 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 : Handle errors explicitly, never silently swallow them
Applied to files:
src/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/backends/mem0/mappers.py
🧬 Code graph analysis (7)
tests/unit/memory/test_factory.py (4)
src/ai_company/memory/backends/mem0/adapter.py (4)
Mem0MemoryBackend(179-1023)is_connected(306-308)backend_name(311-313)max_memories_per_agent(343-345)src/ai_company/memory/backends/mem0/config.py (1)
Mem0EmbedderConfig(23-50)src/ai_company/memory/errors.py (1)
MemoryConfigError(40-41)src/ai_company/memory/factory.py (1)
create_memory_backend(28-122)
src/ai_company/memory/factory.py (4)
src/ai_company/memory/config.py (1)
CompanyMemoryConfig(139-195)src/ai_company/memory/backends/mem0/config.py (2)
Mem0EmbedderConfig(23-50)build_config_from_company_config(130-181)src/ai_company/memory/errors.py (1)
MemoryConfigError(40-41)src/ai_company/memory/backends/mem0/adapter.py (2)
Mem0MemoryBackend(179-1023)max_memories_per_agent(343-345)
tests/unit/memory/backends/mem0/test_adapter_crud.py (1)
src/ai_company/memory/backends/mem0/adapter.py (1)
_validate_mem0_result(124-176)
src/ai_company/memory/backends/mem0/config.py (2)
src/ai_company/memory/config.py (1)
CompanyMemoryConfig(139-195)src/ai_company/observability/_logger.py (1)
get_logger(8-28)
src/ai_company/memory/backends/mem0/adapter.py (3)
src/ai_company/memory/backends/mem0/config.py (2)
Mem0BackendConfig(53-97)build_mem0_config_dict(100-127)src/ai_company/memory/backends/mem0/mappers.py (7)
apply_post_filters(353-395)extract_category(444-466)extract_publisher(469-482)mem0_result_to_entry(234-299)query_to_mem0_getall_args(334-350)query_to_mem0_search_args(302-331)validate_add_result(401-441)src/ai_company/memory/models.py (3)
MemoryEntry(82-150)MemoryQuery(153-230)MemoryStoreRequest(55-79)
tests/unit/memory/backends/mem0/test_mappers.py (4)
src/ai_company/core/enums.py (1)
MemoryCategory(101-108)src/ai_company/memory/backends/mem0/mappers.py (11)
_coerce_confidence(116-135)_normalize_tags(158-176)build_mem0_metadata(39-58)extract_publisher(469-482)mem0_result_to_entry(234-299)normalize_relevance_score(90-113)parse_mem0_datetime(61-87)parse_mem0_metadata(179-231)query_to_mem0_getall_args(334-350)query_to_mem0_search_args(302-331)validate_add_result(401-441)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/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)
🪛 LanguageTool
CLAUDE.md
[style] ~130-~130: A comma is missing here.
Context: ...nder ai_company.observability.events (e.g. PROVIDER_CALL_START from `events.prov...
(EG_NO_COMMA)
docs/design/memory.md
[style] ~313-~313: Consider using the typographical ellipsis character here instead.
Context: ...g). All are frozen Pydantic models. The create_memory_backend(config, *, embedder=...) factory returns an isolated `MemoryBa...
(ELLIPSIS)
🔇 Additional comments (26)
src/ai_company/memory/models.py (1)
195-198: LGTM!The updated description correctly reflects the half-open interval semantics implemented in
apply_post_filters(entries withcreated_at >= sinceare included).src/ai_company/observability/events/memory.py (1)
21-23: LGTM!The new event constants follow the established
memory.<entity>.<action>naming convention and support the validation pathways in the Mem0 adapter.src/ai_company/memory/backends/mem0/adapter.py (6)
179-201: LGTM!The
__init__properly initializes state with anasyncio.Lockfor connection idempotency and stores the configuration immutably.
204-251: LGTM!The
connect()method correctly implements the double-check locking pattern withasyncio.Lockto prevent duplicate client creation during concurrent calls. The# type: ignore[unreachable]comment on line 222 is appropriate for the concurrent state change edge case.
359-391: LGTM!The
_validate_agent_idhelper with parameterizederror_clsprovides clean error-type customization for read vs write operations while ensuring the reserved namespace is consistently rejected.
694-781: LGTM!The
count()method correctly bases truncation detection onlen(raw_list)(the raw query result size) rather than the filtered count, ensuring accurate truncation warnings regardless of category filtering.
785-848: LGTM!The
publish()method now includes_validate_agent_id(agent_id)to reject reserved namespace IDs, preventing agents from setting the publisher to the shared namespace value.
937-1023: LGTM!The
retract()method properly validates publisher ownership before deletion and correctly distinguishes between "not found" (returnsFalse) and "ownership mismatch" or "not a shared entry" (raisesMemoryStoreError).src/ai_company/memory/backends/mem0/mappers.py (4)
39-58: LGTM!The
build_mem0_metadatafunction correctly serializes store request metadata into Mem0-compatible format with prefixed keys.
353-395: LGTM!The
apply_post_filtersfunction correctly implements the half-open interval (created_at >= sinceandcreated_at < until) and handles all filter criteria with proper null-safety forrelevance_score.
401-441: LGTM!The
validate_add_resultfunction provides thorough validation of Mem0addresponses with clear error messages and appropriate logging.
469-482: LGTM!The
extract_publisherfunction safely extracts publisher metadata with proper null handling and string coercion.docs/design/memory.md (3)
209-223: LGTM!The
MemoryBackendprotocol documentation now clearly specifies which exceptions each method can raise, improving API clarity.
261-269: LGTM!The
SharedKnowledgeStoreprotocol documentation mirrors the exception annotations added toMemoryBackend, maintaining consistency across the memory API surface.
302-315: LGTM!The embedder configuration example and factory documentation update clearly communicate that
Mem0EmbedderConfigis required for the Mem0 backend.CLAUDE.md (2)
97-97: LGTM!The package structure documentation correctly reflects the new Mem0 backend location at
backends/mem0/.
208-208: LGTM!The dependency section correctly documents
mem0aias a required dependency for the Mem0 memory backend.tests/unit/memory/backends/mem0/test_adapter_shared.py (4)
1-25: LGTM!The test module is well-organized with proper imports, timeout marker, and clear separation of test fixtures from conftest.py.
30-102: LGTM!The
TestPublishclass provides comprehensive coverage including success paths, empty results, exceptions, missing IDs, and bothMemoryErrorandRecursionErrorre-raising behavior.
107-287: LGTM!The
TestSearchSharedclass thoroughly tests semantic and non-semantic search paths, agent exclusion, post-filtering, publisher fallback, and error propagation.
292-398: LGTM!The
TestRetractclass covers all key scenarios: success, not found, ownership mismatch, missing publisher, and error wrapping at both the get and delete phases.tests/integration/memory/test_mem0_backend.py (5)
41-51: LGTM!The fixture appropriately injects a mock client by setting internal state, which is acceptable for integration tests that need to bypass actual Mem0 initialization while testing the full retrieval pipeline flow.
54-106: LGTM!The
test_store_then_retrievetest validates the complete store → retrieve flow with proper assertions on memory ID, content, and relevance score.
107-161: LGTM!The
test_pipeline_prepare_messagestest exercises the full retrieval pipeline includingContextInjectionStrategy, validating that memories are properly ranked, filtered, and formatted into chat messages.
163-221: LGTM!The
test_shared_knowledge_flowtest validates the complete publish → search_shared → retract lifecycle for shared knowledge entries.
255-288: LGTM!The
test_count_after_storetest verifies both total count and category-filtered count operations.
| Both ``provider`` and ``model`` are required — callers must | ||
| supply them explicitly so that vendor-specific identifiers stay | ||
| out of source defaults. The values must be valid Mem0 SDK | ||
| identifiers (e.g. ``"openai"``, ``"text-embedding-ada-002"``); | ||
| see the Mem0 documentation for supported providers and models. |
There was a problem hiding this comment.
Replace the vendor-specific docstring examples.
The openai / text-embedding-ada-002 examples put concrete vendor/model names back into source docs. Please switch them to the repository's generic placeholders instead.
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."
🤖 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 26 - 30, The
docstring in src/ai_company/memory/backends/mem0/config.py that shows example
identifiers for the required parameters provider and model uses vendor-specific
examples ("openai", "text-embedding-ada-002"); update those examples to the
repository's generic placeholders (e.g., use "example-provider" for provider and
"example-medium-001" for model) so the docstring for the required parameters
provider and model references only allowed generic identifiers per the coding
guidelines.
| @pytest.mark.unit | ||
| class TestValidateAddResult: | ||
| def test_valid_result(self) -> None: | ||
| result = {"results": [{"id": "mem-001", "event": "ADD"}]} | ||
| memory_id = validate_add_result(result, context="test") | ||
| assert memory_id == "mem-001" | ||
|
|
||
| def test_empty_results_raises(self) -> None: | ||
| result: dict[str, Any] = {"results": []} | ||
| with pytest.raises(MemoryStoreError, match="no results"): | ||
| validate_add_result(result, context="test") | ||
|
|
||
| def test_missing_results_key_raises(self) -> None: | ||
| result = {"data": "something"} | ||
| with pytest.raises(MemoryStoreError, match="no results"): | ||
| validate_add_result(result, context="test") | ||
|
|
||
| def test_non_list_results_raises(self) -> None: | ||
| result = {"results": "not-a-list"} | ||
| with pytest.raises(MemoryStoreError, match="no results"): | ||
| validate_add_result(result, context="test") | ||
|
|
||
| def test_missing_id_raises(self) -> None: | ||
| result = {"results": [{"memory": "no id"}]} | ||
| with pytest.raises(MemoryStoreError, match="missing or blank 'id'"): | ||
| validate_add_result(result, context="test") | ||
|
|
||
| def test_none_id_raises(self) -> None: | ||
| result = {"results": [{"id": None, "event": "ADD"}]} | ||
| with pytest.raises(MemoryStoreError, match="missing or blank 'id'"): | ||
| validate_add_result(result, context="test") | ||
|
|
||
| def test_blank_id_raises(self) -> None: | ||
| result = {"results": [{"id": "", "event": "ADD"}]} | ||
| with pytest.raises(MemoryStoreError, match="missing or blank 'id'"): | ||
| validate_add_result(result, context="test") | ||
|
|
||
| def test_whitespace_only_id_raises(self) -> None: | ||
| result = {"results": [{"id": " ", "event": "ADD"}]} | ||
| with pytest.raises(MemoryStoreError, match="missing or blank 'id'"): | ||
| validate_add_result(result, context="test") | ||
|
|
||
| def test_numeric_id_coerced_to_string(self) -> None: | ||
| result = {"results": [{"id": 42, "event": "ADD"}]} | ||
| memory_id = validate_add_result(result, context="test") | ||
| assert memory_id == "42" | ||
|
|
||
| def test_non_dict_result_raises(self) -> None: | ||
| with pytest.raises(MemoryStoreError, match="unexpected type"): | ||
| validate_add_result("not-a-dict", context="test") | ||
|
|
||
| def test_non_dict_first_item_raises(self) -> None: | ||
| result: dict[str, Any] = {"results": ["not-a-dict"]} | ||
| with pytest.raises(MemoryStoreError, match="not a dict"): | ||
| validate_add_result(result, context="test") | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Parameterize the validate_add_result() failure matrix.
These cases only vary result and the expected regex. A small @pytest.mark.parametrize table would keep the same coverage with much less duplicated setup.
As per coding guidelines, "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_mappers.py` around lines 449 - 504, The
tests under TestValidateAddResult repeat many failure cases that only vary the
input result and expected regex; replace those individual failure test methods
with a single `@pytest.mark.parametrize` test that iterates tuples of (result,
expected_regex) and asserts validate_add_result(result, context="test") raises
MemoryStoreError matching the regex; keep the existing positive tests
(test_valid_result and test_numeric_id_coerced_to_string) as-is, and include the
special non-dict case ("not-a-dict") and non-dict-first-item ({"results":
["not-a-dict"]}) in the param table so all failure scenarios (missing/malformed
results, missing/blank id, unexpected type, non-dict first item) are covered by
the one parameterized test calling validate_add_result.
…ig, factory, and tests - Remove @runtime_checkable from TYPE_CHECKING-only Mem0Client protocol - Add _connect_lock to disconnect() to prevent race with connect() - Fix MEMORY_ENTRY_DELETE_FAILED event on non-failure path (unverifiable owner) - Add shared namespace verification in retract() (defense-in-depth) - Add _validate_agent_id() call in retract() - Add math.isfinite() guards in normalize_relevance_score() and _coerce_confidence() - Prefer updated_at over now() as created_at fallback in mem0_result_to_entry() - Reject qdrant-external in build_config_from_company_config() (not supported) - Extract _create_mem0_backend() helper from create_memory_backend() - Fix MemoryQuery class docstring: "after" → "at or after" for since field - Use explicit builtins.MemoryError in all test assertions
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
Implements the first concrete MemoryBackend by adding a Mem0-backed adapter (embedded Qdrant + SQLite history), wiring it into the memory backend factory, and updating public exports/docs. The PR also adds extensive unit/integration tests for Mem0 behavior and adjusts tooling/lint configuration to accommodate new deps and test import patterns.
Changes:
- Add
Mem0MemoryBackendadapter + Mem0 config/mapping utilities, and dispatch to it fromcreate_memory_backend(). - Add comprehensive Mem0 unit + integration tests (CRUD, shared knowledge, config, mappers, lifecycle/guards).
- Update dependency set (mem0ai + transitive deps), observability events, docs, and test import linting.
Reviewed changes
Copilot reviewed 81 out of 84 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Locks mem0ai and new transitive dependencies (qdrant/sqlalchemy/etc). |
| pyproject.toml | Adds mem0ai dependency, relaxes Ruff TC rules for tests, adds mypy ignore for mem0.*. |
| src/ai_company/memory/backends/mem0/adapter.py | New Mem0 backend implementation (lifecycle, CRUD, shared knowledge). |
| src/ai_company/memory/backends/mem0/config.py | New Mem0 backend + embedder config and builders. |
| src/ai_company/memory/backends/mem0/mappers.py | New domain↔Mem0 mapping/parsing + post-filtering helpers. |
| src/ai_company/memory/backends/mem0/init.py | Exports Mem0 backend/config types. |
| src/ai_company/memory/backends/init.py | Exposes concrete backends package exports. |
| src/ai_company/memory/factory.py | Dispatches factory to Mem0 backend; adds embedder requirement + error wrapping/logging. |
| src/ai_company/memory/init.py | Re-exports Mem0MemoryBackend and Mem0EmbedderConfig. |
| src/ai_company/memory/models.py | Clarifies since semantics (“at or after”) in doc/field description. |
| src/ai_company/observability/events/memory.py | Updates memory event constants (adds config/agent-id-rejected; removes unused). |
| tests/unit/memory/test_factory.py | Adds factory coverage for Mem0 creation and error wrapping. |
| tests/unit/memory/test_init.py | Updates public export assertions for Mem0 symbols. |
| tests/unit/memory/backends/mem0/conftest.py | Shared fixtures for Mem0 adapter tests. |
| tests/unit/memory/backends/mem0/test_adapter.py | Tests lifecycle, guards, protocol conformance, capabilities. |
| tests/unit/memory/backends/mem0/test_adapter_crud.py | Tests CRUD + _validate_mem0_result behavior. |
| tests/unit/memory/backends/mem0/test_adapter_shared.py | Tests publish/search_shared/retract flows and ownership logic. |
| tests/unit/memory/backends/mem0/test_config.py | Tests Mem0 config validation/builders. |
| tests/unit/memory/backends/mem0/test_mappers.py | Tests metadata/datetime/score parsing and post-filter behavior (incl expiry). |
| tests/integration/memory/test_mem0_backend.py | Integration-style test: adapter plugged into retrieval pipeline w/ mocked client. |
| tests/unit/observability/test_events.py | Updates expected memory event constants list. |
| docs/design/memory.md | Updates design doc to reflect Mem0 adapter implemented + factory embedder requirement. |
| docs/roadmap/index.md | Removes “Mem0 adapter” from remaining roadmap; marks it implemented. |
| README.md | Status section updated to include Mem0 adapter as implemented. |
| CLAUDE.md | Updates architecture notes + dependency expectations to include mem0ai. |
| .github/workflows/dependency-review.yml | Allows additional license IDs / “null license metadata” exceptions for new deps. |
| tests/unit/tools/sandbox/test_protocol.py | Removes noqa: TC* now that tests ignore TC rules. |
| tests/unit/tools/sandbox/conftest.py | Same: removes noqa: TC*. |
| tests/unit/tools/git/test_git_sandbox_integration.py | Same: removes noqa: TC*. |
| tests/unit/tools/git/conftest.py | Same: removes noqa: TC*. |
| tests/unit/providers/test_protocol.py | Same: removes noqa: TC*. |
| tests/unit/providers/conftest.py | Same: removes noqa: TC*. |
| tests/unit/hr/test_registry.py | Same: removes noqa: TC*. |
| tests/unit/hr/test_onboarding_service.py | Same: removes noqa: TC*. |
| tests/unit/hr/test_offboarding_service.py | Same: removes noqa: TC*. |
| tests/unit/hr/test_hiring_service.py | Same: removes noqa: TC*. |
| tests/unit/hr/test_full_snapshot_strategy.py | Same: removes noqa: TC*. |
| tests/unit/engine/test_task_engine_mutations.py | Same: removes noqa: TC*. |
| tests/unit/engine/test_routing_models.py | Same: removes noqa: TC*. |
| tests/unit/engine/test_react_loop.py | Same: removes noqa: TC*. |
| tests/unit/engine/test_plan_execute_loop.py | Same: removes noqa: TC*. |
| tests/unit/engine/test_metrics.py | Same: removes noqa: TC*. |
| tests/unit/engine/test_loop_protocol.py | Same: removes noqa: TC*. |
| tests/unit/engine/test_context.py | Same: removes noqa: TC*. |
| tests/unit/engine/test_agent_engine_lifecycle.py | Same: removes noqa: TC*. |
| tests/unit/engine/test_agent_engine_errors.py | Same: removes noqa: TC*. |
| tests/unit/engine/test_agent_engine.py | Same: removes noqa: TC*. |
| tests/unit/engine/task_engine_helpers.py | Same: removes noqa: TC*. |
| tests/unit/communication/meeting/test_structured_phases.py | Same: removes noqa: TC*. |
| tests/unit/communication/meeting/test_round_robin.py | Same: removes noqa: TC*. |
| tests/unit/communication/meeting/test_protocol.py | Same: removes noqa: TC*. |
| tests/unit/communication/meeting/test_position_papers.py | Same: removes noqa: TC*. |
| tests/unit/communication/meeting/test_orchestrator.py | Same: removes noqa: TC*. |
| tests/unit/communication/meeting/conftest.py | Same: removes noqa: TC*. |
| tests/unit/communication/conflict_resolution/test_service.py | Same: removes noqa: TC*. |
| tests/unit/communication/conflict_resolution/test_hybrid_strategy.py | Same: removes noqa: TC*. |
| tests/unit/communication/conflict_resolution/test_helpers.py | Same: removes noqa: TC*. |
| tests/unit/communication/conflict_resolution/test_debate_strategy.py | Same: removes noqa: TC*. |
| tests/unit/communication/conflict_resolution/test_authority_strategy.py | Same: removes noqa: TC*. |
| tests/unit/budget/test_category_analytics.py | Same: removes noqa: TC*. |
| tests/unit/api/test_middleware.py | Same: removes noqa: TC*. |
| tests/unit/api/test_health.py | Same: removes noqa: TC*. |
| tests/unit/api/test_guards.py | Same: removes noqa: TC*. |
| tests/unit/api/test_app.py | Same: removes noqa: TC*. |
| tests/unit/api/controllers/test_tasks.py | Same: removes noqa: TC*. |
| tests/unit/api/controllers/test_providers.py | Same: removes noqa: TC*. |
| tests/unit/api/controllers/test_projects.py | Same: removes noqa: TC*. |
| tests/unit/api/controllers/test_messages.py | Same: removes noqa: TC*. |
| tests/unit/api/controllers/test_meetings.py | Same: removes noqa: TC*. |
| tests/unit/api/controllers/test_departments.py | Same: removes noqa: TC*. |
| tests/unit/api/controllers/test_company.py | Same: removes noqa: TC*. |
| tests/unit/api/controllers/test_budget.py | Same: removes noqa: TC*. |
| tests/unit/api/controllers/test_autonomy.py | Same: removes noqa: TC*. |
| tests/unit/api/controllers/test_artifacts.py | Same: removes noqa: TC*. |
| tests/unit/api/controllers/test_approvals.py | Same: removes noqa: TC*. |
| tests/unit/api/controllers/test_analytics.py | Same: removes noqa: TC*. |
| tests/unit/api/controllers/test_agents.py | Same: removes noqa: TC*. |
| tests/unit/api/conftest.py | Same: removes noqa: TC*. |
| tests/unit/api/auth/test_controller.py | Same: removes noqa: TC*. |
| tests/integration/tools/test_sandbox_integration.py | Same: removes noqa: TC*. |
| tests/integration/tools/conftest.py | Same: removes noqa: TC*. |
| tests/integration/communication/test_meeting_integration.py | Same: removes noqa: TC*. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| config_dict = build_mem0_config_dict(self._mem0_config) | ||
| client = await asyncio.to_thread(Memory.from_config, config_dict) | ||
| except builtins.MemoryError, RecursionError: |
| if not raw: | ||
| return None | ||
| try: | ||
| dt = datetime.fromisoformat(raw) | ||
| except ValueError, TypeError: | ||
| logger.warning( | ||
| MEMORY_MODEL_INVALID, | ||
| field="datetime", | ||
| raw_value=raw, | ||
| reason="malformed ISO 8601 datetime, returning None", | ||
| ) | ||
| return None | ||
| if dt.tzinfo is None: |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (3)
tests/unit/memory/backends/mem0/test_mappers.py (1)
468-516: 🧹 Nitpick | 🔵 TrivialParameterize the
validate_add_result()failure matrix.These cases only vary
resultand the expected regex. A single@pytest.mark.parametrizetable would keep the same coverage with much less duplicated setup.Based on learnings,
tests/**/*.py: Preferpytest.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_mappers.py` around lines 468 - 516, Combine the repeated failure/success cases for validate_add_result in tests/unit/memory/backends/mem0/test_mappers.py into a single pytest.mark.parametrize table that iterates over pairs of (result, expected_regex_or_expected_value) and a flag indicating whether to expect an exception; locate the current individual tests named test_empty_results_raises, test_missing_results_key_raises, test_non_list_results_raises, test_missing_id_raises, test_none_id_raises, test_blank_id_raises, test_whitespace_only_id_raises, test_numeric_id_coerced_to_string, test_non_dict_result_raises, and test_non_dict_first_item_raises and replace them with one parametrized test that calls validate_add_result(result, context="test"), asserting an exception with pytest.raises(MemoryStoreError, match=expected_regex) for error cases and asserting the returned memory_id == "42" (coerced string) for the numeric success case; use clear param ids for readability.src/ai_company/memory/backends/mem0/adapter.py (1)
41-48: 🧹 Nitpick | 🔵 TrivialConsolidate duplicate imports from
ai_company.memory.errors.These two import statements from the same module can be merged for cleaner organization.
♻️ Proposed consolidation
-from ai_company.memory.errors import ( - MemoryConnectionError, - MemoryRetrievalError, - MemoryStoreError, -) -from ai_company.memory.errors import ( - MemoryError as DomainMemoryError, -) +from ai_company.memory.errors import ( + MemoryConnectionError, + MemoryError as DomainMemoryError, + MemoryRetrievalError, + MemoryStoreError, +)🤖 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 41 - 48, Two separate import blocks from ai_company.memory.errors should be consolidated into a single import line: combine MemoryConnectionError, MemoryRetrievalError, MemoryStoreError and MemoryError (aliased as DomainMemoryError) into one import statement in adapter.py so there are no duplicate imports and the module imports are cleaner.src/ai_company/memory/backends/mem0/mappers.py (1)
251-322: 🧹 Nitpick | 🔵 TrivialConsider splitting
mem0_result_to_entry()into smaller helpers.This function is ~72 lines, exceeding the < 50 line guideline. The past review comment suggesting extraction of ID/content validation, timestamp parsing, and metadata normalization into separate helpers would improve testability and maintainability. This is not blocking but recommended for future cleanup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ai_company/memory/backends/mem0/mappers.py` around lines 251 - 322, Split mem0_result_to_entry into smaller helper functions to reduce size and improve testability: extract ID validation into a helper (e.g., validate_mem0_id(raw) -> NotBlankStr or raise MemoryRetrievalError) that performs the raw.get("id") checks, logging (MEMORY_MODEL_INVALID) and NotBlankStr construction; extract content validation into a helper (e.g., validate_mem0_content(raw) -> NotBlankStr) that checks raw.get("memory")/raw.get("data"), logs and raises on error; extract timestamp fallback logic into a helper (e.g., resolve_mem0_timestamps(raw, parse_mem0_datetime) -> (created_at, updated_at)) that calls parse_mem0_datetime, applies the updated_at fallback and warning log; and extract metadata/score normalization into a helper that calls parse_mem0_metadata and normalize_relevance_score and returns category, metadata, expires_at, relevance_score; update mem0_result_to_entry to call these helpers and then construct the MemoryEntry with the returned values (preserving use of NotBlankStr, MemoryRetrievalError, parse_mem0_datetime, parse_mem0_metadata, normalize_relevance_score and existing logger usage).
🤖 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/backends/mem0/adapter.py`:
- Around line 549-568: The current handling in the mem0 adapter accepts entries
with owner == None; change this to fail-safe: in the block checking owner =
raw.get("user_id") (and the branch where owner is None) update the logger call
(MEMORY_ENTRY_FETCHED) to indicate found=False and a reason like "ownership
unverifiable" (retain warning level if desired), and ensure the function returns
None immediately instead of returning the entry; keep the existing ownership
mismatch branch (str(owner) != str(agent_id)) as-is.
In `@src/ai_company/memory/backends/mem0/config.py`:
- Around line 109-127: The path construction can create double slashes when
config.data_dir ends with a slash; update the dict construction in this module
so the qdrant path and history_db_path are built with pathlib.Path joins instead
of f-strings—use Path(config.data_dir) / "qdrant" for
vector_store.config["path"] and Path(config.data_dir) / "history.db" for
"history_db_path"; keep other keys (collection_name, embedder settings, version)
unchanged and ensure you convert the Path to a string where the caller expects a
str.
In `@src/ai_company/memory/backends/mem0/mappers.py`:
- Around line 289-322: The current mapper can set created_at to
datetime.now(UTC) when created_at and updated_at are missing, which can violate
the MemoryEntry invariant if expires_at (from parse_mem0_metadata) is earlier;
modify the logic in this function (around parse_mem0_datetime,
parse_mem0_metadata and the MemoryEntry construction) to ensure expires_at >=
created_at by either using expires_at as an additional fallback when computing
created_at (prefer expires_at if it exists and is later than updated_at) or by
clamping expires_at to be at least the chosen created_at before constructing
MemoryEntry; update the fallback selection and any logging
(MEMORY_MODEL_INVALID) accordingly so the MemoryEntry constructor no longer
raises ValueError.
In `@src/ai_company/memory/factory.py`:
- Around line 77-90: The try/except around build_config_from_company_config
currently only catches ValueError, so Pydantic ValidationError thrown while
constructing Mem0BackendConfig escapes; update the except to also catch
ValidationError (from pydantic.errors or pydantic import ValidationError) and
handle it the same way: construct the same msg, call logger.warning with
MEMORY_BACKEND_CONFIG_INVALID and the same kwargs (backend="mem0",
reason="config_build_failed", error=msg), then raise MemoryConfigError(msg) from
exc; target the try that calls build_config_from_company_config (symbols:
build_config_from_company_config, Mem0BackendConfig,
MEMORY_BACKEND_CONFIG_INVALID, MemoryConfigError).
In `@tests/unit/memory/backends/mem0/test_mappers.py`:
- Around line 426-433: The test test_exactly_expired_entry_excluded is
non-deterministic because apply_post_filters calls datetime.now(UTC) itself;
modify the test to control the module clock so expires_at == now is exact —
e.g., freeze time or monkeypatch the datetime.now used by the module under test
before creating entries (set a fixed now value, use that for expires_at and for
the patched datetime.now), then call _make_entry(...) with created_at and
expires_at based on that fixed now and run
apply_post_filters(query=MemoryQuery()); ensure you restore/unpatch the clock
after the assertion so other tests are unaffected.
---
Duplicate comments:
In `@src/ai_company/memory/backends/mem0/adapter.py`:
- Around line 41-48: Two separate import blocks from ai_company.memory.errors
should be consolidated into a single import line: combine MemoryConnectionError,
MemoryRetrievalError, MemoryStoreError and MemoryError (aliased as
DomainMemoryError) into one import statement in adapter.py so there are no
duplicate imports and the module imports are cleaner.
In `@src/ai_company/memory/backends/mem0/mappers.py`:
- Around line 251-322: Split mem0_result_to_entry into smaller helper functions
to reduce size and improve testability: extract ID validation into a helper
(e.g., validate_mem0_id(raw) -> NotBlankStr or raise MemoryRetrievalError) that
performs the raw.get("id") checks, logging (MEMORY_MODEL_INVALID) and
NotBlankStr construction; extract content validation into a helper (e.g.,
validate_mem0_content(raw) -> NotBlankStr) that checks
raw.get("memory")/raw.get("data"), logs and raises on error; extract timestamp
fallback logic into a helper (e.g., resolve_mem0_timestamps(raw,
parse_mem0_datetime) -> (created_at, updated_at)) that calls
parse_mem0_datetime, applies the updated_at fallback and warning log; and
extract metadata/score normalization into a helper that calls
parse_mem0_metadata and normalize_relevance_score and returns category,
metadata, expires_at, relevance_score; update mem0_result_to_entry to call these
helpers and then construct the MemoryEntry with the returned values (preserving
use of NotBlankStr, MemoryRetrievalError, parse_mem0_datetime,
parse_mem0_metadata, normalize_relevance_score and existing logger usage).
In `@tests/unit/memory/backends/mem0/test_mappers.py`:
- Around line 468-516: Combine the repeated failure/success cases for
validate_add_result in tests/unit/memory/backends/mem0/test_mappers.py into a
single pytest.mark.parametrize table that iterates over pairs of (result,
expected_regex_or_expected_value) and a flag indicating whether to expect an
exception; locate the current individual tests named test_empty_results_raises,
test_missing_results_key_raises, test_non_list_results_raises,
test_missing_id_raises, test_none_id_raises, test_blank_id_raises,
test_whitespace_only_id_raises, test_numeric_id_coerced_to_string,
test_non_dict_result_raises, and test_non_dict_first_item_raises and replace
them with one parametrized test that calls validate_add_result(result,
context="test"), asserting an exception with pytest.raises(MemoryStoreError,
match=expected_regex) for error cases and asserting the returned memory_id ==
"42" (coerced string) for the numeric success case; use clear param ids for
readability.
🪄 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: f2fd1aef-290f-478f-8d41-aa493db05810
📒 Files selected for processing (11)
src/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/memory/models.pytests/integration/memory/test_mem0_backend.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/memory/backends/mem0/test_config.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). (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:
tests/unit/memory/backends/mem0/test_adapter.pysrc/ai_company/memory/models.pytests/integration/memory/test_mem0_backend.pytests/unit/memory/backends/mem0/test_adapter_crud.pysrc/ai_company/memory/factory.pysrc/ai_company/memory/backends/mem0/config.pytests/unit/memory/backends/mem0/test_adapter_shared.pytests/unit/memory/backends/mem0/test_config.pysrc/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/backends/mem0/mappers.pytests/unit/memory/backends/mem0/test_mappers.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/integration/memory/test_mem0_backend.pytests/unit/memory/backends/mem0/test_adapter_crud.pytests/unit/memory/backends/mem0/test_adapter_shared.pytests/unit/memory/backends/mem0/test_config.pytests/unit/memory/backends/mem0/test_mappers.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/models.pysrc/ai_company/memory/factory.pysrc/ai_company/memory/backends/mem0/config.pysrc/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/backends/mem0/mappers.py
🧠 Learnings (4)
📚 Learning: 2026-03-13T06:54:28.702Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.702Z
Learning: Applies to src/**/*.py : 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. Tests must use `test-provider`, `test-small-001`, etc.
Applied to files:
src/ai_company/memory/backends/mem0/config.py
📚 Learning: 2026-03-13T06:54:28.702Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.702Z
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.702Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.702Z
Learning: Applies to src/**/*.py : Handle errors explicitly, never silently swallow them
Applied to files:
src/ai_company/memory/backends/mem0/adapter.pysrc/ai_company/memory/backends/mem0/mappers.py
📚 Learning: 2026-03-13T06:54:28.702Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-13T06:54:28.702Z
Learning: Applies to tests/**/*.py : Prefer `pytest.mark.parametrize` for testing similar cases
Applied to files:
tests/unit/memory/backends/mem0/test_mappers.py
🧬 Code graph analysis (10)
tests/unit/memory/backends/mem0/test_adapter.py (4)
src/ai_company/core/enums.py (1)
MemoryCategory(101-108)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/protocol.py (1)
MemoryBackend(20-182)
tests/integration/memory/test_mem0_backend.py (4)
src/ai_company/memory/backends/mem0/config.py (2)
Mem0BackendConfig(53-97)Mem0EmbedderConfig(23-50)src/ai_company/memory/models.py (2)
MemoryQuery(153-230)MemoryStoreRequest(55-79)src/ai_company/memory/retrieval_config.py (1)
MemoryRetrievalConfig(20-132)src/ai_company/memory/retriever.py (1)
ContextInjectionStrategy(97-400)
tests/unit/memory/backends/mem0/test_adapter_crud.py (1)
src/ai_company/memory/backends/mem0/adapter.py (1)
_validate_mem0_result(123-175)
src/ai_company/memory/factory.py (2)
src/ai_company/memory/config.py (1)
CompanyMemoryConfig(139-195)src/ai_company/memory/backends/mem0/config.py (2)
Mem0EmbedderConfig(23-50)build_config_from_company_config(130-181)
src/ai_company/memory/backends/mem0/config.py (4)
src/ai_company/memory/config.py (1)
CompanyMemoryConfig(139-195)src/ai_company/observability/_logger.py (1)
get_logger(8-28)tests/integration/memory/test_mem0_backend.py (1)
backend(45-54)tests/unit/memory/backends/mem0/conftest.py (1)
backend(41-49)
tests/unit/memory/backends/mem0/test_adapter_shared.py (5)
src/ai_company/memory/backends/mem0/adapter.py (8)
Mem0MemoryBackend(178-1043)add(94-96)publish(788-851)search(98-100)search_shared(853-938)delete(110-112)delete(600-695)retract(940-1043)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)
mem0_add_result(52-62)mem0_search_result(65-82)backend(41-49)mock_client(35-37)src/ai_company/core/enums.py (1)
MemoryCategory(101-108)
tests/unit/memory/backends/mem0/test_config.py (2)
src/ai_company/memory/backends/mem0/config.py (4)
Mem0BackendConfig(53-97)Mem0EmbedderConfig(23-50)build_config_from_company_config(130-181)build_mem0_config_dict(100-127)src/ai_company/memory/config.py (2)
CompanyMemoryConfig(139-195)MemoryStorageConfig(25-104)
src/ai_company/memory/backends/mem0/adapter.py (2)
src/ai_company/memory/backends/mem0/config.py (2)
Mem0BackendConfig(53-97)build_mem0_config_dict(100-127)src/ai_company/memory/backends/mem0/mappers.py (8)
apply_post_filters(376-418)build_mem0_metadata(40-59)extract_category(467-489)extract_publisher(492-505)mem0_result_to_entry(251-322)query_to_mem0_getall_args(357-373)query_to_mem0_search_args(325-354)validate_add_result(424-464)
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 (3)
MemoryEntry(82-150)MemoryMetadata(20-52)MemoryQuery(153-230)
tests/unit/memory/backends/mem0/test_mappers.py (2)
src/ai_company/memory/backends/mem0/mappers.py (12)
_coerce_confidence(125-152)_normalize_tags(175-193)apply_post_filters(376-418)build_mem0_metadata(40-59)extract_category(467-489)extract_publisher(492-505)mem0_result_to_entry(251-322)normalize_relevance_score(91-122)parse_mem0_datetime(62-88)parse_mem0_metadata(196-248)query_to_mem0_getall_args(357-373)query_to_mem0_search_args(325-354)src/ai_company/memory/models.py (4)
MemoryEntry(82-150)MemoryMetadata(20-52)MemoryQuery(153-230)MemoryStoreRequest(55-79)
🔇 Additional comments (11)
src/ai_company/memory/backends/mem0/config.py (2)
26-30: Replace vendor-specific examples in docstring.The docstring still references vendor-specific identifiers (
"openai","text-embedding-ada-002"). Per coding guidelines, use generic placeholders like"example-provider"and"example-medium-001"instead.Both ``provider`` and ``model`` are required — callers must supply them explicitly so that vendor-specific identifiers stay - out of source defaults. The values must be valid Mem0 SDK - identifiers (e.g. ``"openai"``, ``"text-embedding-ada-002"``); - see the Mem0 documentation for supported providers and models. + out of source defaults. The values must be valid Mem0 SDK + identifiers (e.g. ``"example-provider"``, ``"example-medium-001"``); + see the Mem0 documentation for supported providers and models.As per coding guidelines, "Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples."
130-181: LGTM!The
build_config_from_company_configfunction properly validates the storage configuration, logs failures with structured events, and constructs the config correctly. The validation forqdrantandsqlitestores is clear and fail-fast.src/ai_company/memory/backends/mem0/adapter.py (5)
237-238: LGTM on exception handling pattern.The
except builtins.MemoryError, RecursionError:syntax correctly follows PEP 758 (Python 3.14), and the pattern of re-raising system-level errors without wrapping is appropriate to avoid masking critical failures.
203-250: LGTM on connection lifecycle.The
connect()method correctly implements double-checked locking with_connect_lockto prevent duplicate client creation. The error handling properly distinguishes betweenImportError(missing package) and other initialization failures.
788-851: LGTM onpublish()implementation.The method correctly validates the agent ID (addressing the previous review comment), properly constructs metadata with
PUBLISHER_KEY, and uses the reserved_SHARED_NAMESPACEfor cross-agent visibility.
853-938: LGTM onsearch_shared()implementation.The method correctly handles the case where publisher metadata is missing by attributing entries to
_SHARED_NAMESPACE, applies post-filters, and supports optional agent exclusion. The fallback logging at debug level is appropriate.
13-17: File size justification is reasonable.The note explaining why this file exceeds the 800-line guideline is appreciated. The cohesive implementation of three protocols (
MemoryBackend,MemoryCapabilities,SharedKnowledgeStore) with shared connection lifecycle logic justifies keeping the code together.src/ai_company/memory/backends/mem0/mappers.py (4)
78-88: LGTM on exception syntax.The
except ValueError, TypeError:syntax correctly follows PEP 758 for Python 3.14. The fallback toNonefor malformed datetimes is appropriate.
91-152: LGTM on score and confidence normalization.Both
normalize_relevance_score()and_coerce_confidence()properly guard against non-finite values usingmath.isfinite(), addressing the previous review concern. The different defaults (None vs 0.5) are appropriate for their respective contexts.
376-418: LGTM onapply_post_filters()implementation.The filter logic correctly implements:
- Expiry exclusion using
<= now(half-open)- Category filtering with
frozensetmembership- Tag filtering with AND semantics via
all()- Time range with half-open interval
[since, until)- Min relevance only applied when
relevance_score is not NoneThe order preservation and explicit
Nonehandling for relevance scores are correct.
424-505: LGTM on adapter helper functions.
validate_add_result()has comprehensive validation of the Mem0 response structure with clear error messages.extract_category()andextract_publisher()handle missing/malformed metadata gracefully with appropriate defaults and logging.
| owner = raw.get("user_id") | ||
| if owner is None: | ||
| logger.warning( | ||
| MEMORY_ENTRY_FETCHED, | ||
| agent_id=agent_id, | ||
| memory_id=memory_id, | ||
| found=True, | ||
| reason="memory has no user_id — ownership " | ||
| "unverifiable, returning to requesting agent", | ||
| ) | ||
| elif str(owner) != str(agent_id): | ||
| logger.debug( | ||
| MEMORY_ENTRY_FETCHED, | ||
| agent_id=agent_id, | ||
| memory_id=memory_id, | ||
| found=False, | ||
| reason="ownership mismatch", | ||
| actual_owner=str(owner), | ||
| ) | ||
| return None |
There was a problem hiding this comment.
Ownership handling for missing user_id may be overly permissive.
When owner is None (line 550), the method logs a warning but still returns the entry to the requesting agent (line 569). This could inadvertently expose memories that belong to other agents if Mem0 returns corrupted data. Consider returning None for unverifiable ownership to fail-safe.
🛡️ Suggested defensive approach
owner = raw.get("user_id")
if owner is None:
logger.warning(
MEMORY_ENTRY_FETCHED,
agent_id=agent_id,
memory_id=memory_id,
found=True,
- reason="memory has no user_id — ownership "
- "unverifiable, returning to requesting agent",
+ reason="memory has no user_id — ownership unverifiable",
)
+ return None
elif str(owner) != str(agent_id):🤖 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 549 - 568, The
current handling in the mem0 adapter accepts entries with owner == None; change
this to fail-safe: in the block checking owner = raw.get("user_id") (and the
branch where owner is None) update the logger call (MEMORY_ENTRY_FETCHED) to
indicate found=False and a reason like "ownership unverifiable" (retain warning
level if desired), and ensure the function returns None immediately instead of
returning the entry; keep the existing ownership mismatch branch (str(owner) !=
str(agent_id)) as-is.
| 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", | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Path construction may produce double slashes if data_dir has a trailing slash.
If data_dir is provided with a trailing slash (e.g., "/data/memory/"), the resulting paths will contain double slashes ("/data/memory//qdrant"). While most file systems handle this correctly, using pathlib would be more robust.
♻️ Suggested fix using pathlib
+from pathlib import Path
+
def build_mem0_config_dict(config: Mem0BackendConfig) -> dict[str, Any]:
...
+ base_path = Path(config.data_dir)
return {
"vector_store": {
"provider": "qdrant",
"config": {
"collection_name": config.collection_name,
"embedding_model_dims": config.embedder.dims,
- "path": f"{config.data_dir}/qdrant",
+ "path": str(base_path / "qdrant"),
},
},
...
- "history_db_path": f"{config.data_dir}/history.db",
+ "history_db_path": str(base_path / "history.db"),
...
}🤖 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 109 - 127, The
path construction can create double slashes when config.data_dir ends with a
slash; update the dict construction in this module so the qdrant path and
history_db_path are built with pathlib.Path joins instead of f-strings—use
Path(config.data_dir) / "qdrant" for vector_store.config["path"] and
Path(config.data_dir) / "history.db" for "history_db_path"; keep other keys
(collection_name, embedder settings, version) unchanged and ensure you convert
the Path to a string where the caller expects a str.
| created_at = parse_mem0_datetime(raw.get("created_at")) | ||
| updated_at = parse_mem0_datetime(raw.get("updated_at")) | ||
| if created_at is None: | ||
| # Prefer updated_at as a fallback — it is a closer | ||
| # approximation than now() and avoids violating the | ||
| # MemoryEntry invariant (updated_at >= created_at). | ||
| fallback = updated_at or datetime.now(UTC) | ||
| fallback_source = "updated_at" if updated_at else "now()" | ||
| logger.warning( | ||
| MEMORY_MODEL_INVALID, | ||
| field="created_at", | ||
| memory_id=str(raw.get("id", "?")), | ||
| reason=f"missing or unparseable created_at, " | ||
| f"defaulting to {fallback_source}", | ||
| ) | ||
| created_at = fallback | ||
|
|
||
| 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=agent_id, | ||
| category=category, | ||
| content=content, | ||
| metadata=metadata, | ||
| created_at=created_at, | ||
| updated_at=updated_at, | ||
| expires_at=expires_at, | ||
| relevance_score=relevance_score, | ||
| ) |
There was a problem hiding this comment.
Potential timestamp ordering violation when expires_at is earlier than the fallback created_at.
When both created_at and updated_at are missing (lines 291-304), created_at defaults to datetime.now(UTC). If expires_at from metadata is earlier than now(), the MemoryEntry constructor will raise ValueError because expires_at >= created_at is required by the model validator (per src/ai_company/memory/models.py lines 137-143).
Consider using expires_at as an additional fallback source, or clamping expires_at to be at least created_at:
🛡️ Suggested fix to use expires_at as additional fallback
if created_at is None:
# Prefer updated_at as a fallback — it is a closer
# approximation than now() and avoids violating the
# MemoryEntry invariant (updated_at >= created_at).
- fallback = updated_at or datetime.now(UTC)
- fallback_source = "updated_at" if updated_at else "now()"
+ raw_metadata = raw.get("metadata")
+ _, _, expires_at_preview = parse_mem0_metadata(raw_metadata)
+ if updated_at is not None:
+ fallback = updated_at
+ fallback_source = "updated_at"
+ elif expires_at_preview is not None:
+ fallback = expires_at_preview
+ fallback_source = "expires_at"
+ else:
+ fallback = datetime.now(UTC)
+ fallback_source = "now()"
logger.warning(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/memory/backends/mem0/mappers.py` around lines 289 - 322, The
current mapper can set created_at to datetime.now(UTC) when created_at and
updated_at are missing, which can violate the MemoryEntry invariant if
expires_at (from parse_mem0_metadata) is earlier; modify the logic in this
function (around parse_mem0_datetime, parse_mem0_metadata and the MemoryEntry
construction) to ensure expires_at >= created_at by either using expires_at as
an additional fallback when computing created_at (prefer expires_at if it exists
and is later than updated_at) or by clamping expires_at to be at least the
chosen created_at before constructing MemoryEntry; update the fallback selection
and any logging (MEMORY_MODEL_INVALID) accordingly so the MemoryEntry
constructor no longer raises ValueError.
| 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 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python -m pip install --quiet 'pydantic==2.12.5'
python - <<'PY'
from pydantic import BaseModel
class Demo(BaseModel):
data_dir: str
try:
Demo(data_dir=None)
except Exception as exc:
print(type(exc).__name__)
print(str(exc).splitlines()[0])
PY
rg -n -C2 'def build_config_from_company_config|return Mem0BackendConfig\(|except ValueError|except \(ValueError, ValidationError\)' \
src/ai_company/memory/backends/mem0/config.py \
src/ai_company/memory/factory.pyRepository: Aureliolo/synthorg
Length of output: 1550
Catch ValidationError in the config-build path too.
build_config_from_company_config() constructs Mem0BackendConfig (line 178 in src/ai_company/memory/backends/mem0/config.py), so malformed values can fail with Pydantic ValidationError before backend construction starts. Right now that escapes as a raw dependency exception instead of the factory's documented MemoryConfigError. (Note: the second try-except block at line 96 already includes this fix, but the first block at line 82 does not.)
Suggested fix
- except ValueError as exc:
+ except (ValueError, ValidationError) 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🤖 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 77 - 90, The try/except around
build_config_from_company_config currently only catches ValueError, so Pydantic
ValidationError thrown while constructing Mem0BackendConfig escapes; update the
except to also catch ValidationError (from pydantic.errors or pydantic import
ValidationError) and handle it the same way: construct the same msg, call
logger.warning with MEMORY_BACKEND_CONFIG_INVALID and the same kwargs
(backend="mem0", reason="config_build_failed", error=msg), then raise
MemoryConfigError(msg) from exc; target the try that calls
build_config_from_company_config (symbols: build_config_from_company_config,
Mem0BackendConfig, MEMORY_BACKEND_CONFIG_INVALID, MemoryConfigError).
| def test_exactly_expired_entry_excluded(self) -> None: | ||
| """Entry with expires_at == now is excluded (<=).""" | ||
| now = datetime.now(UTC) | ||
| past = now - timedelta(days=7) | ||
| entries = (_make_entry(memory_id="m1", created_at=past, expires_at=now),) | ||
| query = MemoryQuery() | ||
| result = apply_post_filters(entries, query) | ||
| assert len(result) == 0 |
There was a problem hiding this comment.
Freeze the clock for the exact-expiry check.
apply_post_filters() reads its own datetime.now(UTC), so this test never deterministically hits the expires_at == now branch; changing the implementation from <= to < would still pass because the inner clock is already later. Patch the module clock or inject a fixed now here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/unit/memory/backends/mem0/test_mappers.py` around lines 426 - 433, The
test test_exactly_expired_entry_excluded is non-deterministic because
apply_post_filters calls datetime.now(UTC) itself; modify the test to control
the module clock so expires_at == now is exact — e.g., freeze time or
monkeypatch the datetime.now used by the module under test before creating
entries (set a fixed now value, use that for expires_at and for the patched
datetime.now), then call _make_entry(...) with created_at and expires_at based
on that fixed now and run apply_post_filters(query=MemoryQuery()); ensure you
restore/unpatch the clock after the assertion so other tests are unaffected.
…mes, pathlib - get(): return None for memories with no user_id (unverifiable ownership) - delete(): raise MemoryStoreError for orphan memories instead of allowing - search_shared(): reject exclude_agent matching reserved shared namespace - config: replace vendor-specific docstring examples with generic identifiers - config: use PurePosixPath for Docker-targeted path construction - factory: catch ValidationError alongside ValueError in config build - mappers: improved created_at fallback cascade (updated_at → expires_at → now) - tests: add orphan rejection tests (get/delete), namespace guard test - tests: freeze clock in expiry test, parameterize validate_add_result tests
|
Closing for fresh review — all review feedback from 8 rounds has been addressed. Opening a new PR for clean reviewer state. |
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
| owner = existing.get("user_id") | ||
| if owner is None: | ||
| msg = ( | ||
| f"Memory {memory_id} has no user_id — ownership " | ||
| f"unverifiable, refusing deletion" | ||
| ) | ||
| logger.warning( | ||
| MEMORY_ENTRY_DELETE_FAILED, | ||
| agent_id=agent_id, | ||
| memory_id=memory_id, | ||
| reason="unverifiable_ownership", | ||
| ) | ||
| raise MemoryStoreError(msg) # noqa: TRY301 | ||
| if owner is not None and str(owner) == _SHARED_NAMESPACE: | ||
| msg = ( | ||
| f"Memory {memory_id} belongs to the shared namespace — " | ||
| f"use retract() to remove shared entries" | ||
| ) | ||
| logger.warning( | ||
| MEMORY_ENTRY_DELETE_FAILED, | ||
| agent_id=agent_id, | ||
| memory_id=memory_id, | ||
| reason="shared namespace entry", | ||
| ) | ||
| raise MemoryStoreError(msg) # noqa: TRY301 | ||
| # Verify ownership — reject cross-agent deletion. | ||
| if owner is not None and str(owner) != str(agent_id): | ||
| msg = ( | ||
| f"Agent {agent_id} cannot delete memory " | ||
| f"{memory_id} owned by {owner}" | ||
| ) | ||
| logger.warning( | ||
| MEMORY_ENTRY_DELETE_FAILED, | ||
| agent_id=agent_id, | ||
| memory_id=memory_id, | ||
| reason="ownership mismatch", | ||
| actual_owner=str(owner), | ||
| ) | ||
| raise MemoryStoreError(msg) # noqa: TRY301 |
There was a problem hiding this comment.
Dead-code if owner is not None guards after unconditional raise
After the raise MemoryStoreError at line 650 when owner is None, control flow never reaches lines 651 or 664. The if owner is not None and … prefix on both subsequent branches is therefore always True and constitutes dead code. While this causes no runtime bug today, it is misleading to future readers who might think the None case is "handled" again at those branches, and becomes a latent footgun if the guard at line 650 is ever restructured (the subsequent conditions would silently stop providing protection).
| owner = existing.get("user_id") | |
| if owner is None: | |
| msg = ( | |
| f"Memory {memory_id} has no user_id — ownership " | |
| f"unverifiable, refusing deletion" | |
| ) | |
| logger.warning( | |
| MEMORY_ENTRY_DELETE_FAILED, | |
| agent_id=agent_id, | |
| memory_id=memory_id, | |
| reason="unverifiable_ownership", | |
| ) | |
| raise MemoryStoreError(msg) # noqa: TRY301 | |
| if owner is not None and str(owner) == _SHARED_NAMESPACE: | |
| msg = ( | |
| f"Memory {memory_id} belongs to the shared namespace — " | |
| f"use retract() to remove shared entries" | |
| ) | |
| logger.warning( | |
| MEMORY_ENTRY_DELETE_FAILED, | |
| agent_id=agent_id, | |
| memory_id=memory_id, | |
| reason="shared namespace entry", | |
| ) | |
| raise MemoryStoreError(msg) # noqa: TRY301 | |
| # Verify ownership — reject cross-agent deletion. | |
| if owner is not None and str(owner) != str(agent_id): | |
| msg = ( | |
| f"Agent {agent_id} cannot delete memory " | |
| f"{memory_id} owned by {owner}" | |
| ) | |
| logger.warning( | |
| MEMORY_ENTRY_DELETE_FAILED, | |
| agent_id=agent_id, | |
| memory_id=memory_id, | |
| reason="ownership mismatch", | |
| actual_owner=str(owner), | |
| ) | |
| raise MemoryStoreError(msg) # noqa: TRY301 | |
| owner = existing.get("user_id") | |
| if owner is None: | |
| msg = ( | |
| f"Memory {memory_id} has no user_id — ownership " | |
| f"unverifiable, refusing deletion" | |
| ) | |
| logger.warning( | |
| MEMORY_ENTRY_DELETE_FAILED, | |
| agent_id=agent_id, | |
| memory_id=memory_id, | |
| reason="unverifiable_ownership", | |
| ) | |
| raise MemoryStoreError(msg) # noqa: TRY301 | |
| if str(owner) == _SHARED_NAMESPACE: | |
| msg = ( | |
| f"Memory {memory_id} belongs to the shared namespace — " | |
| f"use retract() to remove shared entries" | |
| ) | |
| logger.warning( | |
| MEMORY_ENTRY_DELETE_FAILED, | |
| agent_id=agent_id, | |
| memory_id=memory_id, | |
| reason="shared namespace entry", | |
| ) | |
| raise MemoryStoreError(msg) # noqa: TRY301 | |
| # Verify ownership — reject cross-agent deletion. | |
| if str(owner) != str(agent_id): | |
| msg = ( | |
| f"Agent {agent_id} cannot delete memory " | |
| f"{memory_id} owned by {owner}" | |
| ) | |
| logger.warning( | |
| MEMORY_ENTRY_DELETE_FAILED, | |
| agent_id=agent_id, | |
| memory_id=memory_id, | |
| reason="ownership mismatch", | |
| actual_owner=str(owner), | |
| ) | |
| raise MemoryStoreError(msg) # noqa: TRY301 |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ai_company/memory/backends/mem0/adapter.py
Line: 638-676
Comment:
**Dead-code `if owner is not None` guards after unconditional raise**
After the `raise MemoryStoreError` at line 650 when `owner is None`, control flow never reaches lines 651 or 664. The `if owner is not None and …` prefix on both subsequent branches is therefore always `True` and constitutes dead code. While this causes no runtime bug today, it is misleading to future readers who might think the `None` case is "handled" again at those branches, and becomes a latent footgun if the guard at line 650 is ever restructured (the subsequent conditions would silently stop providing protection).
```suggestion
owner = existing.get("user_id")
if owner is None:
msg = (
f"Memory {memory_id} has no user_id — ownership "
f"unverifiable, refusing deletion"
)
logger.warning(
MEMORY_ENTRY_DELETE_FAILED,
agent_id=agent_id,
memory_id=memory_id,
reason="unverifiable_ownership",
)
raise MemoryStoreError(msg) # noqa: TRY301
if str(owner) == _SHARED_NAMESPACE:
msg = (
f"Memory {memory_id} belongs to the shared namespace — "
f"use retract() to remove shared entries"
)
logger.warning(
MEMORY_ENTRY_DELETE_FAILED,
agent_id=agent_id,
memory_id=memory_id,
reason="shared namespace entry",
)
raise MemoryStoreError(msg) # noqa: TRY301
# Verify ownership — reject cross-agent deletion.
if str(owner) != str(agent_id):
msg = (
f"Agent {agent_id} cannot delete memory "
f"{memory_id} owned by {owner}"
)
logger.warning(
MEMORY_ENTRY_DELETE_FAILED,
agent_id=agent_id,
memory_id=memory_id,
reason="ownership mismatch",
actual_owner=str(owner),
)
raise MemoryStoreError(msg) # noqa: TRY301
```
How can I resolve this? If you propose a fix, please make it concise.| if created_at is None: | ||
| # Pick the best available fallback to avoid violating the | ||
| # MemoryEntry invariants (updated_at >= created_at, | ||
| # expires_at >= created_at). | ||
| if updated_at is not None: | ||
| fallback = updated_at | ||
| fallback_source = "updated_at" | ||
| elif expires_at is not None: | ||
| fallback = expires_at | ||
| fallback_source = "expires_at" | ||
| else: | ||
| fallback = datetime.now(UTC) | ||
| fallback_source = "now()" | ||
| logger.warning( | ||
| MEMORY_MODEL_INVALID, | ||
| field="created_at", | ||
| memory_id=str(raw.get("id", "?")), | ||
| reason=f"missing or unparseable created_at, " | ||
| f"defaulting to {fallback_source}", | ||
| ) | ||
| created_at = fallback |
There was a problem hiding this comment.
Using expires_at as created_at fallback produces semantically invalid timestamps
When created_at and updated_at are both absent from a Mem0 result but expires_at is present, the code sets created_at = expires_at. This creates a MemoryEntry where the entry appears to have been created at its own expiry time, which is semantically incorrect and has a concrete downstream effect.
apply_post_filters applies the since filter as entry.created_at >= query.since. An entry with created_at == expires_at = T_future would satisfy T_future >= since for any since < T_future, appearing in results as if it were created in the future — even if it was actually created long before since. Callers relying on since to exclude old memories would receive stale entries.
The datetime.now(UTC) fallback (the else-branch below) is actually safer for since semantics: it anchors the entry to "approximately now", which is at least a conservative present-tense timestamp rather than a future one. Using expires_at as the fallback should either be dropped in favour of datetime.now(UTC), or documented explicitly as a known source of since-filter inaccuracy.
| if created_at is None: | |
| # Pick the best available fallback to avoid violating the | |
| # MemoryEntry invariants (updated_at >= created_at, | |
| # expires_at >= created_at). | |
| if updated_at is not None: | |
| fallback = updated_at | |
| fallback_source = "updated_at" | |
| elif expires_at is not None: | |
| fallback = expires_at | |
| fallback_source = "expires_at" | |
| else: | |
| fallback = datetime.now(UTC) | |
| fallback_source = "now()" | |
| logger.warning( | |
| MEMORY_MODEL_INVALID, | |
| field="created_at", | |
| memory_id=str(raw.get("id", "?")), | |
| reason=f"missing or unparseable created_at, " | |
| f"defaulting to {fallback_source}", | |
| ) | |
| created_at = fallback | |
| if created_at is None: | |
| # Pick the best available fallback to avoid violating the | |
| # MemoryEntry invariants (updated_at >= created_at). | |
| # NOTE: expires_at is intentionally excluded as a fallback — | |
| # using a future timestamp as created_at would cause since- | |
| # filter inaccuracies (entries appearing newer than they are). | |
| if updated_at is not None: | |
| fallback = updated_at | |
| fallback_source = "updated_at" | |
| else: | |
| fallback = datetime.now(UTC) | |
| fallback_source = "now()" | |
| logger.warning( | |
| MEMORY_MODEL_INVALID, | |
| field="created_at", | |
| memory_id=str(raw.get("id", "?")), | |
| reason=f"missing or unparseable created_at, " | |
| f"defaulting to {fallback_source}", | |
| ) | |
| created_at = fallback |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/ai_company/memory/backends/mem0/mappers.py
Line: 295-315
Comment:
**Using `expires_at` as `created_at` fallback produces semantically invalid timestamps**
When `created_at` and `updated_at` are both absent from a Mem0 result but `expires_at` is present, the code sets `created_at = expires_at`. This creates a `MemoryEntry` where the entry *appears to have been created at its own expiry time*, which is semantically incorrect and has a concrete downstream effect.
`apply_post_filters` applies the `since` filter as `entry.created_at >= query.since`. An entry with `created_at == expires_at = T_future` would satisfy `T_future >= since` for any `since < T_future`, appearing in results as if it were created in the future — even if it was actually created long before `since`. Callers relying on `since` to exclude old memories would receive stale entries.
The `datetime.now(UTC)` fallback (the else-branch below) is actually *safer* for `since` semantics: it anchors the entry to "approximately now", which is at least a conservative present-tense timestamp rather than a future one. Using `expires_at` as the fallback should either be dropped in favour of `datetime.now(UTC)`, or documented explicitly as a known source of `since`-filter inaccuracy.
```suggestion
if created_at is None:
# Pick the best available fallback to avoid violating the
# MemoryEntry invariants (updated_at >= created_at).
# NOTE: expires_at is intentionally excluded as a fallback —
# using a future timestamp as created_at would cause since-
# filter inaccuracies (entries appearing newer than they are).
if updated_at is not None:
fallback = updated_at
fallback_source = "updated_at"
else:
fallback = datetime.now(UTC)
fallback_source = "now()"
logger.warning(
MEMORY_MODEL_INVALID,
field="created_at",
memory_id=str(raw.get("id", "?")),
reason=f"missing or unparseable created_at, "
f"defaulting to {fallback_source}",
)
created_at = fallback
```
How can I resolve this? If you propose a fix, please make it concise.
Summary
Implements the Mem0 memory backend adapter as defined in the Memory & Persistence design page and ADR-001. This is the first concrete
MemoryBackendimplementation, providing persistent agent memory backed by embedded Qdrant (vector search) and SQLite (history).What's included
backends/mem0/adapter.py(~930 lines) —Mem0MemoryBackendimplementing three protocols:MemoryBackend— CRUD (store, retrieve, get, delete, count) with per-agent isolation viauser_idMemoryCapabilities— declares vector search, temporal tracking, shared access supportSharedKnowledgeStore— publish/search_shared/retract with__synthorg_shared__namespace and publisher ownership trackingbackends/mem0/config.py—Mem0BackendConfig,Mem0EmbedderConfig(vendor-agnostic),build_mem0_config_dict,build_config_from_company_configbackends/mem0/mappers.py— Bidirectional mapping between SynthOrg domain models and Mem0 dicts (metadata serialization, datetime parsing, score normalization, post-retrieval filtering including expiry enforcement)factory.py—create_memory_backend()factory dispatching to Mem0 backend with full error wrappingmemory/__init__.py— Updated public API exportsKey design decisions
asyncio.to_thread()to avoid blocking the event loopbuiltins.MemoryErrorandRecursionErrorpropagate unwrapped (system-level failures)user_idparameter in all Mem0 calls__synthorg_shared__with_synthorg_publishermetadata key for ownership_validate_agent_iduses parameterizederror_clsto raise correct error type for read vs write operationsTest coverage
tests/integration/memory/test_mem0_backend.py)builtins.MemoryErrorpropagation, ownership validation, config validation, mappers (metadata, datetime, scores, tags, categories, expiry filtering), factory error pathsReview history
This is a fresh PR replacing #338 (closed after 5 rounds of review incorporating feedback from CodeRabbit, Copilot, Greptile, and Gemini). All findings have been addressed:
Memory.reset()fromdisconnect()(data destruction risk)error_clsparameter)apply_post_filtersCloses #206
Test plan
uv run ruff check src/ tests/— all checks passeduv run mypy src/ tests/— 903 source files, no issuesuv run pytest tests/unit/memory/ -m unit -n auto— 647 passed