Python: feat(memory): add DakeraMemoryStore connector for self-hosted Dakera memory server#14131
Python: feat(memory): add DakeraMemoryStore connector for self-hosted Dakera memory server#14131ferhimedamine wants to merge 2 commits into
Conversation
Adds a new memory connector for Dakera — a self-hosted, decay-weighted vector memory server (https://dakera.ai). ## What this adds - `DakeraMemoryStore` implementing `MemoryStoreBase` (the legacy SK memory interface used by `SemanticTextMemory` pipelines) - All 10 abstract methods implemented using Dakera's REST API: - `create_collection` / `delete_collection` / `does_collection_exist` / `get_collections` / `upsert` / `upsert_batch` / `get` / `get_batch` / `remove` / `remove_batch` / `get_nearest_matches` / `get_nearest_match` - `collection_name` maps to Dakera's `agent_id` namespace — each collection is an isolated memory silo inside the same server - Async I/O via `aiohttp` (already a core SK dependency) - 23 unit tests in `tests/unit/connectors/memory/test_dakera.py` that mock the HTTP layer and require no live server - Optional dep `dakera = ["aiohttp >= 3.9"]` added to `pyproject.toml` ## Quick start ```python from semantic_kernel.connectors.memory_stores.dakera import DakeraMemoryStore from semantic_kernel.memory import SemanticTextMemory store = DakeraMemoryStore(url="http://localhost:3300", api_key="demo") memory = SemanticTextMemory(storage=store, embeddings_generator=kernel) ``` Run Dakera locally: ```bash docker run -p 3300:3300 -e DAKERA_API_KEY=demo ghcr.io/dakera-ai/dakera:latest ``` Co-Authored-By: Paperclip <noreply@paperclip.ing>
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Python DakeraMemoryStore connector to integrate a Dakera self-hosted memory server with Semantic Kernel’s legacy MemoryStoreBase / SemanticTextMemory interfaces, along with unit tests and packaging updates.
Changes:
- Adds a
DakeraMemoryStoreimplementation (HTTP client, CRUD, and search helpers) undersemantic_kernel.connectors.memory_stores.dakera. - Adds unit tests for Dakera connector behavior using mocked HTTP calls.
- Adds a
dakeraoptional dependency group inpyproject.toml.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| python/semantic_kernel/connectors/memory_stores/dakera/dakera_memory_store.py | Implements the Dakera-backed MemoryStoreBase connector and related payload/record conversions. |
| python/semantic_kernel/connectors/memory_stores/dakera/init.py | Exposes DakeraMemoryStore from the dakera connector package. |
| python/tests/unit/connectors/memory/test_dakera.py | Adds unit tests for Dakera connector behavior (mocked HTTP). |
| python/pyproject.toml | Adds a dakera optional dependency group. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…arch, exact round-trip Reworks the DakeraMemoryStore connector to resolve all review feedback on the initial revision and to make the integration correct and idiomatic. ## Dependency (unsatisfiable extra → official SDK) - `pyproject.toml` extra was `dakera = ["aiohttp >= 3.9"]`, which both re-pinned a core dependency and read oddly against convention. Replaced with the official async SDK: `dakera = ["dakera[async] >= 0.12.9, < 1.0"]`, mirroring how every other connector declares its vendor library (`chroma → chromadb`, `milvus → pymilvus`). The connector now talks to Dakera through `dakera.AsyncDakeraClient` instead of hand-rolled aiohttp. ## Correctness - **Exact text round-trip.** Writes store the raw SK `text` as Dakera `content` (the description lives only in metadata), so reads reconstruct `MemoryRecord.text` without the previous `"text | description"` pollution. - **`get_nearest_matches` honours the query embedding.** The old code sent `query=collection_name` to a text-search endpoint, so results ignored the caller's query. `MemoryStoreBase` only provides an embedding (never the query text), so the connector now persists the SK-side embedding at write time and ranks candidates client-side with cosine similarity (wiring up the previously-unused `_cosine_similarity` helper) — ranking in the caller's own embedding space. - **`with_embeddings=True` now returns real vectors** hydrated from the persisted embedding instead of an empty array. - **`delete_collection` no longer fails the server safety guard.** It now issues a tag-filtered batch forget (`tags=[collection]`) rather than an unfiltered forget, which the server rejects. - **`upsert` docstring** now matches behaviour: it returns the SK record id (the addressing key); the Dakera UUID is kept on `record._key`. - Removed unused imports (`deepcopy`, `ServiceResourceNotFoundError`, and `json` in tests). ## Docs - Corrected the self-host instructions to the `dakera-deploy` compose stack on port 3000 (the server needs the object store the compose provisions; a bare `docker run` is not sufficient). - Added a Dakera-native `search_text()` path documented alongside the drop-in `get_nearest_matches` compatibility path. ## Tests - 24 unit tests mock the async client (no live server / no network) and cover round-trip fidelity, embedding-based ranking, min-relevance filtering, tag-filtered delete, both-ids batch removal, error wrapping, and client lifecycle ownership. Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
Thanks for the thorough review — all points are addressed in a10ab6d, along with a reworked design that fixes the underlying issues rather than just the symptoms. Design change: the connector now talks to Dakera through the official async SDK (
One extra bug fixed while here: Verification: 24 unit tests (mock the async client, no live server), plus |
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 89%
✓ Correctness
The connector is well-structured and handles the happy-path correctly, but
upsertonly inserts without checking for existing records with the same SK id. TheMemoryStoreBasecontract explicitly requires update-if-exists semantics. Repeated upserts with the same key will accumulate duplicate memories in Dakera, causing non-deterministic reads and incomplete deletes.
✗ Security Reliability
The connector has two reliability issues: (1)
upsertalways inserts without checking for existing records, violating theMemoryStoreBasecontract that says 'If the record already exists, it will be updated,' leading to duplicate accumulation and inconsistent reads/deletes over time; (2)_list_collectionsilently truncates results atfetch_limitwithout logging a warning, so all dependent operations (get,remove,get_nearest_matches) silently miss records in large collections.
✓ Test Coverage
The test suite (24 tests) covers the happy paths well — round-trip fidelity, upsert, get, remove, cosine ranking, min-relevance filtering, and lifecycle management are all addressed. However, error-wrapping behavior is only verified for the
store_memorycall path (viatest_store_error_is_wrapped). The implementation has three other distinct DakeraError→ServiceResponseException wrapping sites (_list_collection/batch_recall,search_text/recall,delete_collection/batch_forget) that lack test coverage. A regression that removes or breaks any of those try/except blocks would go undetected.
✓ Failure Modes
The connector has three concrete failure modes: (1)
upsertalways creates a new Dakera memory without checking for an existing record with the same SK id, violating theMemoryStoreBasecontract ("If the record already exists, it will be updated") and causing silent duplicate accumulation that corruptsget(),remove(), and similarity search results; (2)upsert_batchusesasyncio.gatherwithout error isolation, so a partial failure (e.g., 3rd of 5 writes fails) raises a single exception while already-persisted records become unrecoverable orphans — the caller cannot distinguish partial from total failure; (3)_list_collectionsilently truncates atfetch_limit(default 1000) with no warning, causingget(),remove(), andget_nearest_matches()to silently miss records in larger collections.
✓ Design Approach
I found two blocking design mismatches and one contract-level compatibility gap. The connector currently turns every collection scan into a fixed first-page view, so large collections silently become partially unreadable/unsearchable, and reference records do not round-trip faithfully because
save_referencerecords withtext=Nonecome back with synthesized text. In addition,get_colections/does_collection_existonly reflect process-local bookkeeping rather than backend state, which breaks the legacyMemoryStoreBasecontract after a restart.
Flagged Issues
-
upsertviolates theMemoryStoreBaseinterface contract (memory_store_base.py:74-88: 'If the record already exists, it will be updated'). Repeated calls with the same SK id create duplicates, causinggetto return stale data,removeto leave orphans, andget_nearest_matchesto score the same logical record multiple times.
Automated review by ferhimedamine's agents
| content = record._text or record._description or record._id | ||
| try: | ||
| memory = await self._client.store_memory( | ||
| agent_id=collection_name, | ||
| content=content, | ||
| metadata=_record_to_metadata(record), | ||
| tags=[collection_name, _SK_TAG], | ||
| ) | ||
| except DakeraError as exc: | ||
| raise ServiceResponseException(f"Dakera store_memory failed: {exc}") from exc | ||
|
|
||
| dakera_id = memory.get("id") if isinstance(memory, dict) else None | ||
| if not dakera_id: | ||
| raise ServiceResponseException(f"Dakera did not return a memory id after store. Response: {memory!r}") | ||
|
|
||
| record._key = dakera_id | ||
| self._known_collections.add(collection_name) | ||
| return record._id |
There was a problem hiding this comment.
upsert always inserts a new memory without checking whether a record with the same SK id already exists. The MemoryStoreBase contract explicitly states: "If the record already exists, it will be updated. If the record does not exist, it will be created." Other connectors (e.g. Postgres uses ON CONFLICT (key) DO UPDATE) handle this correctly. Here, repeated calls with the same record._id silently accumulate duplicates. Downstream: get() returns whichever duplicate is found first (non-deterministic), remove() only deletes the first match leaving orphans, and get_nearest_matches() may return the same logical record multiple times.
Fix: resolve and forget the existing record before storing the replacement:
existing_dakera_id = await self._resolve_dakera_id(collection_name, record._id)
if existing_dakera_id is not None:
await self._forget(collection_name, existing_dakera_id)| content = record._text or record._description or record._id | |
| try: | |
| memory = await self._client.store_memory( | |
| agent_id=collection_name, | |
| content=content, | |
| metadata=_record_to_metadata(record), | |
| tags=[collection_name, _SK_TAG], | |
| ) | |
| except DakeraError as exc: | |
| raise ServiceResponseException(f"Dakera store_memory failed: {exc}") from exc | |
| dakera_id = memory.get("id") if isinstance(memory, dict) else None | |
| if not dakera_id: | |
| raise ServiceResponseException(f"Dakera did not return a memory id after store. Response: {memory!r}") | |
| record._key = dakera_id | |
| self._known_collections.add(collection_name) | |
| return record._id | |
| content = record._text or record._description or record._id | |
| # Honour upsert semantics: delete any existing record with the same SK id | |
| # before inserting, so repeated saves don't accumulate duplicates. | |
| existing_dakera_id = await self._resolve_dakera_id(collection_name, record._id) | |
| if existing_dakera_id is not None: | |
| await self._forget(collection_name, existing_dakera_id) | |
| try: | |
| memory = await self._client.store_memory( | |
| agent_id=collection_name, | |
| content=content, | |
| metadata=_record_to_metadata(record), | |
| tags=[collection_name, _SK_TAG], | |
| ) | |
| except DakeraError as exc: | |
| raise ServiceResponseException(f"Dakera store_memory failed: {exc}") from exc | |
| dakera_id = memory.get("id") if isinstance(memory, dict) else None | |
| if not dakera_id: | |
| raise ServiceResponseException(f"Dakera did not return a memory id after store. Response: {memory!r}") | |
| record._key = dakera_id | |
| self._known_collections.add(collection_name) | |
| return record._id |
| try: | ||
| response = await self._client.batch_recall(request) | ||
| except NotFoundError: | ||
| return [] | ||
| except DakeraError as exc: | ||
| raise ServiceResponseException(f"Dakera batch_recall failed for '{collection_name}': {exc}") from exc | ||
| return list(response.memories) |
There was a problem hiding this comment.
_list_collection uses a fixed limit=self._fetch_limit with no pagination, making every caller (get, get_batch, remove_batch, get_nearest_matches) operate on only the first page of a collection. A collection exceding the limit will have records silently unreachable, violating the legacy store contract's expectation that these methods operate over the full collection. At minimum, add a warning log when the result size equals the limit to alert operators of potential truncation.
| try: | |
| response = await self._client.batch_recall(request) | |
| except NotFoundError: | |
| return [] | |
| except DakeraError as exc: | |
| raise ServiceResponseException(f"Dakera batch_recall failed for '{collection_name}': {exc}") from exc | |
| return list(response.memories) | |
| try: | |
| response = await self._client.batch_recall(request) | |
| except NotFoundError: | |
| return [] | |
| except DakeraError as exc: | |
| raise ServiceResponseException(f"Dakera batch_recall failed for '{collection_name}': {exc}") from exc | |
| memories = list(response.memories) | |
| if len(memories) >= self._fetch_limit: | |
| logger.warning( | |
| "DakeraMemoryStore: collection '%s' returned %d memories (at fetch_limit); " | |
| "results may be truncated. Increase fetch_limit to access all records.", | |
| collection_name, | |
| len(memories), | |
| ) | |
| return memories |
| subsequent ``get``/``remove`` calls. (Dakera's own server-assigned | ||
| UUID is retained on ``record._key`` for internal use.) | ||
| """ | ||
| content = record._text or record._description or record._id |
There was a problem hiding this comment.
SemanticTextMemory.save_reference() builds a MemoryRecord.reference_record(...) whose text is None. The fallback here to record._description or record._id for Dakera content, combined with _to_record() reconstructing with text=content, means a reference record read back from Dakera acquires synthetic text that was never in the original record. This breaks the PR's stated 'exact round-trip' design for reference records.
|
Flagged issue
Source: automated DevFlow PR review |
|
Thanks for the contribution, @ferhimedamine. We're not looking to add support for this memory connector to SK right now. Please feel free to host support for it on your end. |
Problem
Semantic Kernel has no connector for Dakera, a self-hosted memory server that adds persistent, decay-weighted vector recall across agent sessions: memories are importance-scored and decay over time, so stale context stops competing with fresh, relevant facts. This PR adds a first-class
DakeraMemoryStoreso any pipeline built onSemanticTextMemorycan use Dakera as its backing store with no code changes outside the constructor.Design
DakeraMemoryStoreimplements the legacyMemoryStoreBasecontract (still used bySemanticTextMemory) on top of the official async Dakera SDK (dakera.AsyncDakeraClient).agent_id. An SK collection maps 1-to-1 to a Dakeraagent_idnamespace — a fully isolated memory silo inside one server.create_collectionis a lazy no-op;delete_collectionpurges the namespace with a single tag-filtered batch forget.get_nearest_matches.MemoryStoreBaseonly hands the store a query embedding (never the query text). The connector therefore persists the SK-side embedding alongside each memory at write time and ranks candidates client-side with cosine similarity — i.e. in the caller's own embedding space. This keeps a correct drop-in experience forSemanticTextMemory.search_text(). For the richer path,search_text()sends the raw query text to Dakera'srecallendpoint and benefits from server-side decay- and importance-weighted ranking.textis stored as Dakeracontent; description and the otherMemoryRecordfields live in metadata, so reads reconstruct records losslessly.dakeraoptional group declares the vendor SDK (dakera[async]), mirroring every other connector (chroma → chromadb,milvus → pymilvus).Usage
Run Dakera locally with the public
dakera-deploycompose stack (the server needs the object store the compose provisions, so a baredocker runis not sufficient):Install the extra:
Testing
python/tests/unit/connectors/memory/test_dakera.py— 24 unit tests that inject a mockAsyncDakeraClient(no live server, no network).delete_collection, both-ids batch removal, error wrapping, and owned-vs-injected client lifecycle.ruff check,ruff format --check, andmypyall clean on the connector and tests.Notes
MemoryStoreBaseis deprecated upstream; this connector targets it for compatibility with existingSemanticTextMemorypipelines and is marked@deprecatedaccordingly, pointing users to theVectorStoreAPI for new code.Checklist
rufflint + format passmypypasses