diff --git a/CHANGELOG.md b/CHANGELOG.md index 37524762..eae73e78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -160,6 +160,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 parameter was renamed `entity` → `entity_id` (all in-tree callers pass it positionally); code using the `entity=` keyword must switch to `entity_id=`. +### Fixed + +- **NAMS `add_entity` no longer raises `ValidationError` when the server + auto-merges the create onto an existing entity.** NAMS resolves-before-create: + a sufficiently similar name responds `{id, resolution: "merged", merged_into, + confidence}` with no `name`/`type`, which previously failed Pydantic `Entity` + parsing. The client now follows up with `GET /entities/{id}` and returns the + canonical merged-into entity. Fallback is limited to 404 or empty/incomplete + canonical responses with a valid merge ID; authentication, rate-limit, + transport, and malformed-data errors propagate. HTTP 404 now raises the + exported `NotFoundError`, a `MemoryError` subclass. Merge details are preserved + in `metadata.nams_resolution`, including a `fallback` flag and the separate + `merge_confidence` score; fallback entity confidence uses the model default. + Null confidence and collection fields use meaningful defaults, while invalid + server IDs and explicit null/invalid creation timestamps fail validation. + > **Docs note:** when this ships, flip the "REST-only / no SDK method" notes in > `reference/rest-api.adoc`, `reference/ontology-api.adoc`, and > `reference/authentication.adoc`, and the Python↔TS parity note in diff --git a/docs/modules/ROOT/pages/how-to/migrate-to-nams.adoc b/docs/modules/ROOT/pages/how-to/migrate-to-nams.adoc index 8a2650b8..5e70a597 100644 --- a/docs/modules/ROOT/pages/how-to/migrate-to-nams.adoc +++ b/docs/modules/ROOT/pages/how-to/migrate-to-nams.adoc @@ -221,7 +221,7 @@ These methods exist on `NamsLongTermMemory` / `NamsShortTermMemory` but not on t === Deduplication * **Bolt**: client-side deduplication runs on every `add_entity` (configurable via `DeduplicationConfig`). `add_entity` returns `(Entity, DeduplicationResult)` — the tuple carries info about whether a merge happened. -* **NAMS**: dedup is server-side and opaque to the client. `add_entity` returns just `Entity`. Existing code that unpacks the tuple (`entity, dedup = await client.long_term.add_entity(...)`) **will break** on NAMS — the result isn't iterable as a 2-tuple. Migrate to: +* **NAMS**: dedup is server-side. `add_entity` returns just `Entity`; auto-merge details are available in `entity.metadata["nams_resolution"]`. Existing code that unpacks the tuple (`entity, dedup = await client.long_term.add_entity(...)`) **will break** on NAMS — the result isn't iterable as a 2-tuple. Migrate to: + ```python result = await client.long_term.add_entity(...) @@ -231,6 +231,12 @@ These methods exist on `NamsLongTermMemory` / `NamsShortTermMemory` but not on t entity = result # NAMS ``` +For auto-merges, `nams_resolution` contains `merged_into`, the separate +`merge_confidence` score when present, and a `fallback` flag. A fallback uses +request/default-derived fields when the canonical read returns 404 or an +incomplete response. See xref:reference/rest-api.adoc#auto-merged-entity-creates[Auto-merged entity creates] +for the return and error behavior. + === Embeddings and vector indexes * **Bolt**: you configure an embedder (OpenAI, sentence-transformers, etc.) on `MemorySettings.embedding`; the client embeds text before storage; vector indexes are created at `connect()` sized to the embedder's dimensions. diff --git a/docs/modules/ROOT/pages/reference/rest-api.adoc b/docs/modules/ROOT/pages/reference/rest-api.adoc index c73b3744..2bd087a1 100644 --- a/docs/modules/ROOT/pages/reference/rest-api.adoc +++ b/docs/modules/ROOT/pages/reference/rest-api.adoc @@ -99,6 +99,43 @@ excluding any `loaded_ids` you already have — it backs incremental graph-visualization ("expand this node"). Exposed as `long_term.expand_graph` / `longTerm.expandGraph`. +[#auto-merged-entity-creates] +=== Auto-merged entity creates + +When `POST /entities` resolves a name to an existing entity, it returns a +`resolution: "merged"` envelope instead of a complete entity. Python +`add_entity` and TypeScript `addEntity` fetch the canonical entity and return it. +Both expose the create operation's resolution in `entity.metadata` under the +reserved `nams_resolution` key, preserving other entity metadata: + +[source,json] +---- +{ + "nams_resolution": { + "resolution": "merged", + "merged_into": "00000000-0000-0000-0000-000000000001", + "merge_confidence": 0.93, + "fallback": false + } +} +---- + +`merge_confidence` is the service's name-match score and is omitted when absent +or null. It does not replace the canonical entity's own `confidence`. + +If the canonical GET returns 404, an empty body, or an incomplete response +without a usable name, the SDK can return a fallback using the server-provided +merge ID and requested name/type. `fallback: true` identifies this result; +its creation timestamp is generated by the client, not read from the server. +Python uses its entity confidence default of `1.0`; TypeScript leaves entity +confidence undefined. These fallback values are not estimates of stored +canonical values. Missing merge IDs and malformed populated fields are +validation errors. Authentication, rate-limit, network, and server errors +propagate to the caller instead of returning a fallback. + +Ordinary creates and `review_pending` responses return their entity directly +without a follow-up GET. + == Reasoning [cols="2,3,1,1,1,1,1"] @@ -187,7 +224,7 @@ Error responses use standard HTTP status codes with a JSON body of the shape | `400` | Invalid request, write Cypher attempted, validation failure | `ValidationError` (carries `details`) | `401`, `403` | Missing/invalid token, or workspace not accessible | `AuthenticationError` -| `404` | Resource not found | `MemoryError` (base class) +| `404` | Resource not found | `NotFoundError` (subclasses `MemoryError`) | `405`, `501` | Operation not available on this backend | `NotSupportedError` | `429` | Rate limited | `RateLimitError` (carries `retry_after`; honors `Retry-After`) | `5xx` | Service error | `TransportError` (after retries exhausted) @@ -198,6 +235,9 @@ All SDK exceptions subclass `MemoryError`. The SDK retries `429`, `5xx`, and network errors with exponential backoff, honoring any `Retry-After` header. See xref:reference/nams-limits.adoc[NAMS Limits & Behavior] for rate-limit and quota specifics. +The TypeScript REST transport reports 404 as `TransportError` with +`statusCode === 404`; its exported `NotFoundError` is not used by that transport. + == Pagination List endpoints accept `limit` and `offset` query parameters for offset-based diff --git a/src/neo4j_agent_memory/__init__.py b/src/neo4j_agent_memory/__init__.py index c636543d..c139d8a8 100644 --- a/src/neo4j_agent_memory/__init__.py +++ b/src/neo4j_agent_memory/__init__.py @@ -100,6 +100,7 @@ ExtractionError, MemoryError, NotConnectedError, + NotFoundError, NotSupportedError, RateLimitError, ResolutionError, @@ -318,6 +319,7 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: # NAMS exceptions (v0.4) "TransportError", "AuthenticationError", + "NotFoundError", "NotSupportedError", "RateLimitError", "ValidationError", diff --git a/src/neo4j_agent_memory/core/__init__.py b/src/neo4j_agent_memory/core/__init__.py index 22d22a2c..a782830e 100644 --- a/src/neo4j_agent_memory/core/__init__.py +++ b/src/neo4j_agent_memory/core/__init__.py @@ -5,6 +5,7 @@ EmbeddingError, ExtractionError, MemoryError, + NotFoundError, ResolutionError, SchemaError, ) @@ -17,6 +18,7 @@ __all__ = [ # Exceptions "MemoryError", + "NotFoundError", "ConnectionError", "SchemaError", "ExtractionError", diff --git a/src/neo4j_agent_memory/core/exceptions.py b/src/neo4j_agent_memory/core/exceptions.py index 7379d73d..e2997b3c 100644 --- a/src/neo4j_agent_memory/core/exceptions.py +++ b/src/neo4j_agent_memory/core/exceptions.py @@ -73,6 +73,12 @@ class AuthenticationError(MemoryError): pass +class NotFoundError(MemoryError): + """Raised when the NAMS backend cannot find a resource (HTTP 404).""" + + pass + + class RateLimitError(MemoryError): """Raised when the NAMS backend rate-limits the client. diff --git a/src/neo4j_agent_memory/nams/long_term.py b/src/neo4j_agent_memory/nams/long_term.py index 13d3fa40..da183c65 100644 --- a/src/neo4j_agent_memory/nams/long_term.py +++ b/src/neo4j_agent_memory/nams/long_term.py @@ -32,10 +32,13 @@ import asyncio import time from collections.abc import Callable +from datetime import datetime, timezone from typing import TYPE_CHECKING, Any from uuid import UUID -from neo4j_agent_memory.core.exceptions import NotSupportedError +from pydantic import TypeAdapter + +from neo4j_agent_memory.core.exceptions import NotFoundError, NotSupportedError, ValidationError from neo4j_agent_memory.memory.long_term import ( Entity, Fact, @@ -167,18 +170,17 @@ def _normalize_entity(payload: dict[str, Any] | None) -> dict[str, Any]: doesn't provide — we default them so Pydantic parsing succeeds. NAMS types come back lowercase; uppercase them so package-side consumers see the same type values they sent. - """ - from datetime import datetime, timezone + Known nullable confidence/collection fields use model defaults. Identity + and explicit timestamps remain subject to validation; a missing server ID + must never invoke the model's UUID factory. Missing creation timestamps + retain the existing model-default behavior for compatibility. + """ data = snakeize_keys(payload) if isinstance(payload, dict) else {} - if "created_at" not in data: - data["created_at"] = datetime.now(timezone.utc).isoformat() - if "metadata" not in data: - data["metadata"] = {} - if "aliases" not in data: - data["aliases"] = [] - if "attributes" not in data: - data["attributes"] = {} + for field in ("confidence", "metadata", "aliases", "attributes"): + if data.get(field) is None: + data.pop(field, None) + data.setdefault("id", None) if isinstance(data.get("type"), str): data["type"] = data["type"].upper() return data @@ -215,6 +217,15 @@ async def add_entity( ``{name, type, description?}`` per spec. Bolt-only kwargs (``subtype``, ``aliases``, ``attributes``, ``confidence``, ``deduplicate``, ``geocode``, ``enrich``, etc.) are silently dropped. + + NAMS resolves-before-create: when the name is close enough to an + existing entity, the server merges onto it and responds + ``{id, resolution: "merged", merged_into, confidence}`` — no + ``name``/``type``. We follow up with ``GET /entities/{id}`` and + return the canonical merged-into entity. A 404 or incomplete read + falls back to the request's name/type with an explicit local UTC + timestamp. Other read failures propagate. ``metadata.nams_resolution`` + preserves the merge outcome and score and identifies fallback records. """ et = entity_type or kwargs.get("type") or kwargs.get("label") if et is None: @@ -227,8 +238,87 @@ async def add_entity( } ) payload = await self._transport.request(_SPEC_ADD_ENTITY, json=body) + if isinstance(payload, dict) and payload.get("resolution") == "merged": + payload = await self._resolve_merged_entity(payload, name=name, nams_type=body["type"]) return payload_to_model(_normalize_entity(payload), Entity) + async def _resolve_merged_entity( + self, + payload: dict[str, Any], + *, + name: str, + nams_type: str, + ) -> dict[str, Any]: + """Turn a ``resolution: "merged"`` create response into entity fields. + + A missing canonical record (404, empty body, or missing name) may use + request fields, but malformed populated fields and other read errors + remain visible. The merge score describes similarity, not the returned + entity's confidence, and is preserved separately in metadata. + """ + raw_merged_id = None + for key in ("merged_into", "mergedInto", "id"): + value = payload.get(key) + if value is None or (isinstance(value, str) and not value.strip()): + continue + raw_merged_id = value + break + merged_id = TypeAdapter(UUID).validate_python(raw_merged_id) + envelope_metadata = snakeize_keys( + TypeAdapter(dict[str, Any]).validate_python( + {} if payload.get("metadata") is None else payload["metadata"] + ) + ) + try: + detail = await self._transport.request( + _SPEC_GET_ENTITY, + path_params={"entity_id": str(merged_id)}, + ) + except NotFoundError: + detail = None + + data = ( + {} + if detail is None + else snakeize_keys(TypeAdapter(dict[str, Any]).validate_python(detail)) + ) + canonical_name = data.get("name") + fallback = canonical_name is None or ( + isinstance(canonical_name, str) and not canonical_name.strip() + ) + if fallback: + # Validate fields that did arrive before building a fallback. Only + # absent identity/type and missing names can use request context. + candidate = {"id": str(merged_id), "type": nams_type, **data, "name": name} + else: + candidate = data + canonical = payload_to_model(_normalize_entity(candidate), Entity) + if canonical.id != merged_id: + raise ValidationError("NAMS canonical entity ID does not match the merge target") + if not canonical.type.strip(): + raise ValidationError("NAMS canonical entity type must not be blank") + + if fallback: + data = { + "id": str(merged_id), + "name": name, + "type": nams_type, + "created_at": datetime.now(timezone.utc).isoformat(), + } + resolution: dict[str, Any] = { + "resolution": "merged", + "merged_into": str(merged_id), + "fallback": fallback, + } + if payload.get("confidence") is not None: + resolution["merge_confidence"] = payload["confidence"] + data["metadata"] = { + **envelope_metadata, + **canonical.metadata, + "nams_resolution": resolution, + } + return data + async def add_preference(self, category: str, preference: str, **kwargs: Any) -> Preference: raise NotSupportedError( backend="nams", diff --git a/src/neo4j_agent_memory/nams/transport.py b/src/neo4j_agent_memory/nams/transport.py index e148fda6..b2ca1a75 100644 --- a/src/neo4j_agent_memory/nams/transport.py +++ b/src/neo4j_agent_memory/nams/transport.py @@ -28,7 +28,7 @@ from neo4j_agent_memory.config.settings import NamsConfig from neo4j_agent_memory.core.exceptions import ( AuthenticationError, - MemoryError, + NotFoundError, NotSupportedError, RateLimitError, TransportError, @@ -356,7 +356,7 @@ def _raise_for_status( if status in (401, 403): raise AuthenticationError(message) if status == 404: - raise MemoryError(message) + raise NotFoundError(message) if status in (405, 501): raise NotSupportedError( backend="nams", diff --git a/tests/integration/nams/test_framework_smoke.py b/tests/integration/nams/test_framework_smoke.py index d0e8e662..3e31aa0d 100644 --- a/tests/integration/nams/test_framework_smoke.py +++ b/tests/integration/nams/test_framework_smoke.py @@ -141,11 +141,11 @@ async def test_crewai_smoke(nams_client: MemoryClient, nams_session: str) -> Non except (ImportError, AttributeError): pytest.skip("crewai integration unavailable") - # CrewAI's adapter is crew-scoped (no session_id) and uses an internal - # crew id rather than a pre-created NAMS conversation, so we assert it - # constructs against a NAMS-backed client and round-trip the write path - # through the shared client. - _ = Neo4jCrewMemory(memory_client=nams_client) + # CrewAI's adapter is crew-scoped: crew_id (not session_id) names the + # conversation it writes under. We assert it constructs against a + # NAMS-backed client and round-trip the write path through the shared + # client. + _ = Neo4jCrewMemory(memory_client=nams_client, crew_id=f"{nams_session}-crew") marker = _marker() async def store() -> None: diff --git a/tests/integration/nams/test_smoke.py b/tests/integration/nams/test_smoke.py index 7d4054a2..6c955f25 100644 --- a/tests/integration/nams/test_smoke.py +++ b/tests/integration/nams/test_smoke.py @@ -45,7 +45,13 @@ async def test_smoke_full_flow(nams_client: MemoryClient, nams_session: str) -> entity_name = f"SmokeTest-{uuid.uuid4().hex[:8]}" entity = await nams_client.long_term.add_entity(entity_name, "PERSON") entity = entity[0] if isinstance(entity, tuple) else entity - assert entity.name == entity_name + assert entity.name.strip() + if entity.name != entity_name: + # Similar smoke-test names can merge across runs in the shared sandbox. + resolution = entity.metadata.get("nams_resolution", {}) + assert resolution.get("resolution") == "merged" + assert resolution.get("merged_into") == str(entity.id) + assert resolution.get("fallback") is False # Reasoning: tied to the session. trace = await nams_client.reasoning.start_trace(nams_session, "smoke task") diff --git a/tests/integration/nams/test_tck_gold.py b/tests/integration/nams/test_tck_gold.py index 6c6a3948..2f0fa37e 100644 --- a/tests/integration/nams/test_tck_gold.py +++ b/tests/integration/nams/test_tck_gold.py @@ -177,10 +177,11 @@ async def test_entity_visible_across_sessions( e = await nams_client.long_term.add_entity(entity_name, "PERSON") e = e[0] if isinstance(e, tuple) else e - # Query from session B context. NAMS search is async-indexed; poll. + # Query the returned canonical name: the create may have merged onto a + # prior run's entity. NAMS search is async-indexed; poll. found = None for _ in range(10): # ~5s - found = await nams_client.long_term.get_entity_by_name(entity_name) + found = await nams_client.long_term.get_entity_by_name(e.name) if found is not None: break await asyncio.sleep(0.5) @@ -191,4 +192,5 @@ async def test_entity_visible_across_sessions( "Cross-session visibility is verified by the write succeeding " f"with entity id {e.id} from session_a={session_a}." ) - assert found.name == entity_name + assert found.name == e.name + assert found.id == e.id diff --git a/tests/integration/nams/test_tck_silver.py b/tests/integration/nams/test_tck_silver.py index 7a5522d8..37d9ece3 100644 --- a/tests/integration/nams/test_tck_silver.py +++ b/tests/integration/nams/test_tck_silver.py @@ -36,7 +36,14 @@ async def test_add_entity_returns_entity(nams_client: MemoryClient, unique_name: # NAMS may return Entity directly OR wrapped in a tuple — accept both. actual = entity[0] if isinstance(entity, tuple) else entity assert isinstance(actual, Entity) - assert actual.name == name + assert actual.name.strip() + if actual.name != name: + # Even UUID-suffixed names can resolve to a prior run's entity. A + # different name is valid only for a fetched canonical merge target. + resolution = actual.metadata.get("nams_resolution", {}) + assert resolution.get("resolution") == "merged" + assert resolution.get("merged_into") == str(actual.id) + assert resolution.get("fallback") is False assert actual.type == "PERSON" @@ -66,16 +73,18 @@ async def test_get_entity_by_name_found(nams_client: MemoryClient, unique_name: impl calls ``POST /entities/search`` and filters for exact match. That search is vector-backed and indexed asynchronously — a freshly-written entity may not be returned by search for a brief window. We poll a - handful of times before giving up to absorb that lag. + handful of times before giving up to absorb that lag. If creation merges + onto an existing entity, look up its canonical name. """ import asyncio name = unique_name("charlie") - await nams_client.long_term.add_entity(name, "PERSON") + entity = await nams_client.long_term.add_entity(name, "PERSON") + actual = entity[0] if isinstance(entity, tuple) else entity found = None for _ in range(10): # ~5s total - found = await nams_client.long_term.get_entity_by_name(name) + found = await nams_client.long_term.get_entity_by_name(actual.name) if found is not None: break await asyncio.sleep(0.5) @@ -85,7 +94,8 @@ async def test_get_entity_by_name_found(nams_client: MemoryClient, unique_name: "NAMS search index appears to lag behind writes for " "get_entity_by_name; treating as eventual-consistency limitation." ) - assert found.name == name + assert found.name == actual.name + assert found.id == actual.id @pytest.mark.asyncio diff --git a/tests/unit/nams/test_exceptions.py b/tests/unit/nams/test_exceptions.py index a9701f4a..0b4daf65 100644 --- a/tests/unit/nams/test_exceptions.py +++ b/tests/unit/nams/test_exceptions.py @@ -1,9 +1,10 @@ """Phase 1 unit tests: new exception classes for the NAMS backend. -Covers the five new exceptions added in v0.4: +Covers the HTTP/backend-specific exceptions: * ``TransportError`` (subclass of ``ConnectionError``) * ``AuthenticationError`` +* ``NotFoundError`` * ``NotSupportedError`` — structured (backend, method, workaround) * ``RateLimitError`` — carries ``retry_after`` * ``ValidationError`` — carries ``details`` @@ -17,6 +18,7 @@ AuthenticationError, ConnectionError, MemoryError, + NotFoundError, NotSupportedError, RateLimitError, TransportError, @@ -35,6 +37,14 @@ def test_transport_error_is_connection_error(self): def test_authentication_error_is_memory_error(self): assert issubclass(AuthenticationError, MemoryError) + def test_not_found_error_is_memory_error(self): + assert issubclass(NotFoundError, MemoryError) + + def test_not_found_error_is_exported_from_core(self): + from neo4j_agent_memory.core import NotFoundError as CoreNotFoundError + + assert CoreNotFoundError is NotFoundError + def test_not_supported_error_is_memory_error(self): assert issubclass(NotSupportedError, MemoryError) diff --git a/tests/unit/nams/test_long_term.py b/tests/unit/nams/test_long_term.py index 8b6b7541..86b833bb 100644 --- a/tests/unit/nams/test_long_term.py +++ b/tests/unit/nams/test_long_term.py @@ -9,15 +9,24 @@ from __future__ import annotations import json +from datetime import datetime, timezone import httpx import pytest import respx - -from neo4j_agent_memory.core.exceptions import NotSupportedError +from pydantic import ValidationError as PydanticValidationError + +from neo4j_agent_memory.core.exceptions import ( + AuthenticationError, + NotSupportedError, + RateLimitError, + TransportError, + ValidationError, +) from neo4j_agent_memory.core.protocols import LongTermProtocol from neo4j_agent_memory.memory.long_term import Entity, Relationship from neo4j_agent_memory.nams import HttpTransport, NamsLongTermMemory, StaticApiKeyAuth +from neo4j_agent_memory.nams.long_term import _normalize_entity @pytest.fixture @@ -154,6 +163,110 @@ async def test_basic_returns_entity_only(self, long_term): # Outbound type is mapped to NAMS' lowercase enum. assert body == {"name": "Alice", "type": "person", "description": "Test entity"} + @respx.mock + async def test_merged_resolution_fetches_canonical_entity(self, long_term): + # NAMS resolves-before-create: a near-duplicate name merges onto the + # existing entity and the response carries no name/type — the client + # must follow up with GET /entities/{id} for the canonical record. + respx.post("https://memory.test/v1/entities").respond( + 200, + json={ + "id": SAMPLE_ENTITY["id"], + "resolution": "merged", + "merged_into": SAMPLE_ENTITY["id"], + "confidence": 0.93, + }, + ) + get_route = respx.get(f"https://memory.test/v1/entities/{SAMPLE_ENTITY['id']}").respond( + 200, json={**SAMPLE_ENTITY, "relationships": []} + ) + entity = await long_term.add_entity("Alice Smith", "PERSON") + assert get_route.called + assert isinstance(entity, Entity) + assert str(entity.id) == SAMPLE_ENTITY["id"] + # The canonical (merged-into) record wins over the requested name. + assert entity.name == "Alice" + assert entity.type == "PERSON" + + @respx.mock + async def test_merged_resolution_tolerates_null_fields_on_canonical_entity(self, long_term): + # NAMS projects unset node properties as JSON null — a manually + # created entity has no confidence/sourceStage/updatedAt. Those + # nulls must fall back to model defaults, not fail float parsing. + respx.post("https://memory.test/v1/entities").respond( + 200, + json={ + "id": SAMPLE_ENTITY["id"], + "resolution": "merged", + "merged_into": SAMPLE_ENTITY["id"], + "confidence": 0.93, + }, + ) + respx.get(f"https://memory.test/v1/entities/{SAMPLE_ENTITY['id']}").respond( + 200, + json={ + "id": SAMPLE_ENTITY["id"], + "name": "Alice", + "type": "person", + "description": None, + "confidence": None, + "sourceStage": None, + "createdAt": "2026-05-17T12:00:00Z", + "updatedAt": None, + "relationships": [], + }, + ) + entity = await long_term.add_entity("Alice Smith", "PERSON") + assert isinstance(entity, Entity) + assert entity.name == "Alice" + assert entity.confidence == 1.0 # model default + assert entity.description is None + + @respx.mock + async def test_merged_resolution_falls_back_to_request_fields(self, long_term): + # If the follow-up read fails, synthesize a parseable Entity from the + # request instead of raising a ValidationError. + respx.post("https://memory.test/v1/entities").respond( + 200, + json={ + "id": SAMPLE_ENTITY["id"], + "resolution": "merged", + "merged_into": SAMPLE_ENTITY["id"], + "confidence": 0.93, + }, + ) + respx.get(f"https://memory.test/v1/entities/{SAMPLE_ENTITY['id']}").respond( + 404, json={"error": "entity not found"} + ) + entity = await long_term.add_entity("Alice Smith", "PERSON") + assert isinstance(entity, Entity) + assert str(entity.id) == SAMPLE_ENTITY["id"] + assert entity.name == "Alice Smith" + assert entity.type == "PERSON" + assert entity.confidence == 1.0 + assert entity.metadata["nams_resolution"] == { + "resolution": "merged", + "merged_into": SAMPLE_ENTITY["id"], + "merge_confidence": 0.93, + "fallback": True, + } + + @respx.mock + async def test_review_pending_resolution_parses(self, long_term): + # review_pending responses include full entity fields plus the + # resolution/duplicate_of extras — those must parse cleanly. + respx.post("https://memory.test/v1/entities").respond( + 201, + json={ + **SAMPLE_ENTITY, + "resolution": "review_pending", + "duplicate_of": "00000000-0000-0000-0000-000000000002", + }, + ) + entity = await long_term.add_entity("Alice", "PERSON") + assert isinstance(entity, Entity) + assert entity.name == "Alice" + @respx.mock async def test_bolt_only_kwargs_dropped(self, long_term): route = respx.post("https://memory.test/v1/entities").respond(201, json=SAMPLE_ENTITY) @@ -173,6 +286,280 @@ async def test_bolt_only_kwargs_dropped(self, long_term): assert k not in body +def _mock_merged_create(**overrides): + return respx.post("https://memory.test/v1/entities").respond( + 200, + json={ + "id": SAMPLE_ENTITY["id"], + "resolution": "merged", + "merged_into": SAMPLE_ENTITY["id"], + "confidence": 0.93, + **overrides, + }, + ) + + +class TestMergedEntityResolution: + @pytest.mark.parametrize( + ("status", "error_type"), + [ + (400, ValidationError), + (401, AuthenticationError), + (403, AuthenticationError), + (405, NotSupportedError), + (429, RateLimitError), + (500, TransportError), + (501, NotSupportedError), + ], + ) + @respx.mock + async def test_canonical_read_errors_propagate(self, long_term, status, error_type): + _mock_merged_create() + route = respx.get(f"https://memory.test/v1/entities/{SAMPLE_ENTITY['id']}").respond( + status, json={"error": "canonical read failed"}, headers={"Retry-After": "0"} + ) + with pytest.raises(error_type): + await long_term.add_entity("Alice Smith", "PERSON") + assert route.call_count == (3 if status in (429, 500) else 1) + + @pytest.mark.parametrize("error_type", [httpx.ConnectError, httpx.ReadTimeout]) + @respx.mock + async def test_canonical_network_errors_propagate(self, long_term, error_type): + _mock_merged_create() + route = respx.get(f"https://memory.test/v1/entities/{SAMPLE_ENTITY['id']}").mock( + side_effect=error_type("canonical read failed") + ) + with pytest.raises(TransportError) as exc_info: + await long_term.add_entity("Alice Smith", "PERSON") + assert isinstance(exc_info.value.__cause__, error_type) + assert route.call_count == 3 + + @pytest.mark.parametrize("score", [0.93, 1.00001, -0.1, "unscaled", None]) + @pytest.mark.parametrize("fallback", [False, True]) + @respx.mock + async def test_score_and_metadata_are_separate_from_entity_confidence( + self, long_term, score, fallback + ): + _mock_merged_create(confidence=score) + route = respx.get(f"https://memory.test/v1/entities/{SAMPLE_ENTITY['id']}") + if fallback: + route.respond(404) + else: + route.respond(200, json={**SAMPLE_ENTITY, "metadata": {"source": "canonical"}}) + entity = await long_term.add_entity("Alice Smith", "PERSON") + assert entity.confidence == (1.0 if fallback else SAMPLE_ENTITY["confidence"]) + expected_resolution = { + "resolution": "merged", + "merged_into": SAMPLE_ENTITY["id"], + "fallback": fallback, + } + if score is not None: + expected_resolution["merge_confidence"] = score + assert entity.metadata["nams_resolution"] == expected_resolution + if not fallback: + assert entity.metadata["source"] == "canonical" + + @respx.mock + async def test_absent_merge_score_stays_absent(self, long_term): + respx.post("https://memory.test/v1/entities").respond( + 200, json={"id": SAMPLE_ENTITY["id"], "resolution": "merged"} + ) + respx.get(f"https://memory.test/v1/entities/{SAMPLE_ENTITY['id']}").respond( + 200, json=SAMPLE_ENTITY + ) + entity = await long_term.add_entity("Alice Smith", "PERSON") + assert "merge_confidence" not in entity.metadata["nams_resolution"] + + @pytest.mark.parametrize("target_key", ["merged_into", "mergedInto", "id"]) + @respx.mock + async def test_merge_target_aliases(self, long_term, target_key): + respx.post("https://memory.test/v1/entities").respond( + 200, json={target_key: SAMPLE_ENTITY["id"], "resolution": "merged"} + ) + route = respx.get(f"https://memory.test/v1/entities/{SAMPLE_ENTITY['id']}").respond( + 200, json=SAMPLE_ENTITY + ) + entity = await long_term.add_entity("Alice Smith", "PERSON") + assert route.call_count == 1 + assert str(entity.id) == SAMPLE_ENTITY["id"] + + @pytest.mark.parametrize("missing_target", [None, "", " \t "]) + @respx.mock + async def test_empty_merge_target_uses_valid_id(self, long_term, missing_target): + _mock_merged_create(merged_into=missing_target) + route = respx.get(f"https://memory.test/v1/entities/{SAMPLE_ENTITY['id']}").respond( + 200, json=SAMPLE_ENTITY + ) + entity = await long_term.add_entity("Alice Smith", "PERSON") + assert route.call_count == 1 + assert str(entity.id) == SAMPLE_ENTITY["id"] + + @pytest.mark.parametrize("fallback", [False, True]) + @respx.mock + async def test_envelope_and_canonical_metadata_are_preserved(self, long_term, fallback): + _mock_merged_create(metadata={"request": "create", "source": "envelope"}) + detail = {"metadata": {"source": "canonical"}} + if not fallback: + detail.update(SAMPLE_ENTITY) + respx.get(f"https://memory.test/v1/entities/{SAMPLE_ENTITY['id']}").respond( + 200, json=detail + ) + entity = await long_term.add_entity("Alice Smith", "PERSON") + assert entity.metadata["request"] == "create" + assert entity.metadata["source"] == "canonical" + assert entity.metadata["nams_resolution"]["fallback"] is fallback + + @pytest.mark.parametrize( + "identity", + [ + {}, + {"id": None}, + {"id": ""}, + {"id": "bad-id"}, + {"merged_into": []}, + {"merged_into": [], "id": SAMPLE_ENTITY["id"]}, + {"merged_into": False, "id": SAMPLE_ENTITY["id"]}, + {"merged_into": 0, "id": SAMPLE_ENTITY["id"]}, + ], + ) + @respx.mock + async def test_invalid_merge_identity_fails_before_read(self, long_term, identity): + respx.post("https://memory.test/v1/entities").respond( + 200, json={"resolution": "merged", **identity} + ) + with pytest.raises(PydanticValidationError): + await long_term.add_entity("Alice Smith", "PERSON") + assert len(respx.calls) == 1 + + @pytest.mark.parametrize( + "detail", + [ + None, + {}, + {"name": None}, + {"name": ""}, + {"name": " \t "}, + {"metadata": {"source": "partial"}}, + ], + ) + @respx.mock + async def test_incomplete_canonical_response_uses_marked_fallback(self, long_term, detail): + _mock_merged_create() + respx.get(f"https://memory.test/v1/entities/{SAMPLE_ENTITY['id']}").respond( + 200, json=detail + ) + before = datetime.now(timezone.utc) + entity = await long_term.add_entity("Alice Smith", "PERSON") + after = datetime.now(timezone.utc) + assert str(entity.id) == SAMPLE_ENTITY["id"] + assert entity.name == "Alice Smith" + assert entity.type == "PERSON" + assert entity.confidence == 1.0 + assert before <= entity.created_at <= after + assert entity.created_at.utcoffset().total_seconds() == 0 + assert entity.metadata["nams_resolution"]["fallback"] is True + if detail and detail.get("metadata"): + assert entity.metadata["source"] == "partial" + + @pytest.mark.parametrize("status", [200, 204]) + @respx.mock + async def test_empty_canonical_body_uses_marked_fallback(self, long_term, status): + _mock_merged_create() + respx.get(f"https://memory.test/v1/entities/{SAMPLE_ENTITY['id']}").respond(status) + entity = await long_term.add_entity("Alice Smith", "PERSON") + assert entity.name == "Alice Smith" + assert str(entity.id) == SAMPLE_ENTITY["id"] + assert entity.metadata["nams_resolution"]["fallback"] is True + + @pytest.mark.parametrize("entity_type", ["", " \t "]) + @respx.mock + async def test_blank_canonical_type_is_invalid(self, long_term, entity_type): + _mock_merged_create() + respx.get(f"https://memory.test/v1/entities/{SAMPLE_ENTITY['id']}").respond( + 200, json={"type": entity_type} + ) + with pytest.raises(ValidationError, match="type must not be blank"): + await long_term.add_entity("Alice Smith", "PERSON") + + @pytest.mark.parametrize( + "detail", + [ + [], + "not an entity", + {"name": 17}, + {"id": None}, + {"id": "bad-id"}, + {"name": "Alice"}, + {"createdAt": None}, + {"createdAt": "bad-date"}, + {"confidence": 1.1}, + {"type": None}, + {"aliases": "Alice"}, + {"metadata": []}, + ], + ) + @respx.mock + async def test_malformed_canonical_fields_do_not_fall_back(self, long_term, detail): + _mock_merged_create() + respx.get(f"https://memory.test/v1/entities/{SAMPLE_ENTITY['id']}").respond( + 200, json=detail + ) + with pytest.raises(PydanticValidationError): + await long_term.add_entity("Alice Smith", "PERSON") + + @pytest.mark.parametrize("name", ["Alice", None]) + @respx.mock + async def test_canonical_identity_must_match_merge_target(self, long_term, name): + _mock_merged_create() + respx.get(f"https://memory.test/v1/entities/{SAMPLE_ENTITY['id']}").respond( + 200, json={**SAMPLE_ENTITY, "name": name, "id": "00000000-0000-0000-0000-000000000002"} + ) + with pytest.raises(ValidationError, match="does not match the merge target"): + await long_term.add_entity("Alice Smith", "PERSON") + + +class TestNormalizeEntity: + def test_known_null_fields_use_defaults(self): + entity = Entity.model_validate( + _normalize_entity( + { + **SAMPLE_ENTITY, + "confidence": None, + "aliases": None, + "attributes": None, + "metadata": None, + "description": None, + "updatedAt": None, + } + ) + ) + assert entity.confidence == 1.0 + assert entity.aliases == [] + assert entity.attributes == {} + assert entity.metadata == {} + assert entity.description is None + assert entity.updated_at is None + assert entity.created_at == datetime(2026, 5, 17, 12, tzinfo=timezone.utc) + + @pytest.mark.parametrize( + "changes", [{"id": None}, {"id": "bad-id"}, {"createdAt": None}, {"createdAt": "bad-date"}] + ) + def test_invalid_identity_and_timestamps_are_not_defaulted(self, changes): + with pytest.raises(PydanticValidationError): + Entity.model_validate(_normalize_entity({**SAMPLE_ENTITY, **changes})) + + def test_missing_id_never_generates_a_uuid(self): + payload = {key: value for key, value in SAMPLE_ENTITY.items() if key != "id"} + with pytest.raises(PydanticValidationError): + Entity.model_validate(_normalize_entity(payload)) + + def test_missing_timestamp_keeps_existing_default_behavior(self): + payload = {key: value for key, value in SAMPLE_ENTITY.items() if key != "createdAt"} + before = datetime.now(timezone.utc) + entity = Entity.model_validate(_normalize_entity(payload)) + assert before <= entity.created_at <= datetime.now(timezone.utc) + + class TestSearchEntities: @respx.mock async def test_with_envelope(self, long_term): diff --git a/tests/unit/nams/test_transport.py b/tests/unit/nams/test_transport.py index 011b8642..1aad62f1 100644 --- a/tests/unit/nams/test_transport.py +++ b/tests/unit/nams/test_transport.py @@ -5,7 +5,7 @@ * Auto-protocol detection (REST vs bridge). * Auth header application. * Happy path: 200, 201, 204 (empty body). -* Error mapping: 400→Validation, 401→Auth, 403→Auth, 404→MemoryError, +* Error mapping: 400→Validation, 401→Auth, 403→Auth, 404→NotFoundError, 405/501→NotSupportedError, 429→RateLimitError, 5xx→TransportError, network failures→TransportError. * Retry policy: 429 honors Retry-After, 5xx uses exponential backoff, @@ -27,6 +27,7 @@ from neo4j_agent_memory.core.exceptions import ( AuthenticationError, MemoryError, + NotFoundError, NotSupportedError, RateLimitError, TransportError, @@ -261,18 +262,19 @@ async def test_403_raises_authentication_error(self, nams_config, auth): ) @respx.mock - async def test_404_raises_memory_error(self, nams_config, auth): + async def test_404_raises_not_found_error(self, nams_config, auth): respx.post("https://memory.test/v1/conversations/abc/messages").respond( 404, json={"error": "session not found"} ) async with HttpTransport.from_config(nams_config, auth=auth) as t: - with pytest.raises(MemoryError, match="not found"): + with pytest.raises(NotFoundError, match="not found") as exc_info: await t.request( ADD_MESSAGE_SPEC, path_params={"session_id": "abc"}, json={"role": "user", "content": "hi"}, ) + assert isinstance(exc_info.value, MemoryError) @respx.mock async def test_405_raises_not_supported(self, nams_config, auth): diff --git a/typescript/CHANGELOG.md b/typescript/CHANGELOG.md index 07a4efcf..7edd4b67 100644 --- a/typescript/CHANGELOG.md +++ b/typescript/CHANGELOG.md @@ -9,6 +9,24 @@ appear in minor versions with a callout in this file. ## [Unreleased] +### Fixed + +- **`longTerm.addEntity` no longer returns a malformed Entity when the + hosted service auto-merges the create onto an existing entity.** NAMS + resolves-before-create: a sufficiently similar name responds + `{id, resolution: "merged", merged_into, confidence}` with no + `name`/`type`, which previously flowed into an Entity with `undefined` + name and type. The client now follows up with `GET /entities/{id}` and + returns the canonical merged-into entity. Fallback is limited to HTTP 404 + or empty/incomplete canonical responses with a valid merge ID; other errors + propagate. Fallback entities use a lowercase hosted type and a client-generated + ISO creation timestamp. The additive optional `Entity.metadata` preserves + canonical metadata and exposes merge details in `nams_resolution`, including + a `fallback` flag and separate `merge_confidence`; merge scores no longer + populate entity confidence. Missing IDs and malformed canonical fields are + rejected before conversion. Optional null fields are normalized to `undefined` + on entities, inline relationship references, preferences, facts, and mentions. + ## 0.4.0 — NAMS alignment Adds workspace addressing, a first-class ontology surface diff --git a/typescript/src/long-term/index.ts b/typescript/src/long-term/index.ts index 0ddbb95d..1cb3a7ca 100644 --- a/typescript/src/long-term/index.ts +++ b/typescript/src/long-term/index.ts @@ -5,7 +5,7 @@ * entity feedback, history, merge-by-id, graph view, and provenance. */ -import { ValidationError } from "../errors.js"; +import { TransportError, ValidationError } from "../errors.js"; import type { Transport } from "../transport/index.js"; import type { AddRelationshipOptions, @@ -35,31 +35,45 @@ interface WireEntity { id: string; name: string; type: string; - subtype?: string; - description?: string; - embedding?: number[]; - canonical_name?: string; - created_at?: string; - updated_at?: string; - confidence?: number; - source_stage?: string; - relationships?: WireEntityRelRef[]; + subtype?: string | null; + description?: string | null; + embedding?: number[] | null; + canonical_name?: string | null; + created_at?: string | null; + updated_at?: string | null; + confidence?: number | null; + source_stage?: string | null; + relationships?: WireEntityRelRef[] | null; + metadata?: Record | null; } interface WireEntityRelRef { id: string; type: string; target_id: string; - target_name?: string; - properties?: Record; + target_name?: string | null; + properties?: Record | null; +} + +/** + * Hosted create response when NAMS resolves-before-create merges the new + * name onto an existing entity: `{id, resolution: "merged", merged_into, + * confidence}` — no name/type fields. + */ +interface WireMergedResolution { + id?: string; + resolution: string; + merged_into?: string; + confidence?: unknown; + metadata?: Record | null; } interface WirePreference { id: string; category: string; preference: string; - context?: string; - embedding?: number[]; + context?: string | null; + embedding?: number[] | null; } interface WireFact { @@ -67,7 +81,7 @@ interface WireFact { subject: string; predicate: string; object: string; - embedding?: number[]; + embedding?: number[] | null; } interface WireRelationship { @@ -75,7 +89,7 @@ interface WireRelationship { source_id: string; target_id: string; relationship_type: string; - properties?: Record; + properties?: Record | null; } interface WireEntityHistory { @@ -85,7 +99,7 @@ interface WireEntityHistory { interface WireMention { conversation_id: string; - message_id?: string; + message_id?: string | null; content: string; timestamp: string; } @@ -108,20 +122,24 @@ interface WireGraph { edges?: WireGraphEdge[]; } +// NAMS projects unset node properties as JSON null (e.g. `confidence` on a +// manually-created entity) — normalize those to undefined so Entity's +// optional fields stay `T | undefined` at runtime, matching their types. function toEntity(w: WireEntity): Entity { return { id: w.id, name: w.name, type: w.type, - subtype: w.subtype, - description: w.description, - embedding: w.embedding, - canonicalName: w.canonical_name, + subtype: w.subtype ?? undefined, + description: w.description ?? undefined, + embedding: w.embedding ?? undefined, + canonicalName: w.canonical_name ?? undefined, createdAt: w.created_at ?? "", - updatedAt: w.updated_at, - confidence: w.confidence, - sourceStage: w.source_stage, - relationships: w.relationships?.map(toRelRef), + updatedAt: w.updated_at ?? undefined, + confidence: w.confidence ?? undefined, + sourceStage: w.source_stage ?? undefined, + relationships: w.relationships?.map(toRelRef) ?? undefined, + metadata: w.metadata ?? undefined, }; } @@ -130,8 +148,8 @@ function toRelRef(w: WireEntityRelRef): EntityRelationshipRef { id: w.id, type: w.type, targetId: w.target_id, - targetName: w.target_name, - properties: w.properties, + targetName: w.target_name ?? undefined, + properties: w.properties ?? undefined, }; } @@ -140,8 +158,8 @@ function toPreference(w: WirePreference): Preference { id: w.id, category: w.category, preference: w.preference, - context: w.context, - embedding: w.embedding, + context: w.context ?? undefined, + embedding: w.embedding ?? undefined, }; } @@ -151,7 +169,7 @@ function toFact(w: WireFact): Fact { subject: w.subject, predicate: w.predicate, object: w.object, - embedding: w.embedding, + embedding: w.embedding ?? undefined, }; } @@ -168,7 +186,7 @@ function toRelationship(w: WireRelationship): Relationship { function toMention(w: WireMention): EntityMention { return { conversationId: w.conversation_id, - messageId: w.message_id, + messageId: w.message_id ?? undefined, content: w.content, timestamp: w.timestamp, }; @@ -182,6 +200,84 @@ function toGraphEdge(w: WireGraphEdge): EntityGraphEdge { return { id: w.id, source: w.source, target: w.target, type: w.type }; } +function isRecord(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function isNonemptyString(value: unknown): value is string { + return typeof value === "string" && value.trim().length > 0; +} + +function isConfidence(value: unknown): value is number { + return typeof value === "number" && Number.isFinite(value) && value >= 0 && value <= 1; +} + +function isIsoTimestamp(value: unknown): value is string { + if (typeof value !== "string") return false; + const parts = /^(\d{4})-(\d{2})-(\d{2})T\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[+-]\d{2}:\d{2})$/.exec(value); + if (!parts || !Number.isFinite(Date.parse(value))) return false; + const year = Number(parts[1]); + const month = Number(parts[2]); + const day = Number(parts[3]); + const leap = year % 4 === 0 && (year % 100 !== 0 || year % 400 === 0); + const days = [31, leap ? 29 : 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31]; + return month >= 1 && month <= 12 && day >= 1 && day <= days[month - 1]!; +} + +function requireMergedField(valid: boolean, field: string): asserts valid { + if (!valid) throw new ValidationError(`Invalid merged entity response: ${field}`); +} + +/** Validate populated fields before deciding whether an incomplete GET can fall back. */ +function mergedEntityDetail(detail: unknown, mergedId: string): WireEntity | undefined { + if (detail === undefined || detail === null) return undefined; + requireMergedField(isRecord(detail), "expected an entity object"); + if ("id" in detail) { + requireMergedField(isNonemptyString(detail.id) && detail.id === mergedId, "canonical id"); + } + if ("type" in detail) requireMergedField(isNonemptyString(detail.type), "type"); + if (detail.name !== undefined && detail.name !== null) { + requireMergedField(typeof detail.name === "string", "name"); + } + if ("created_at" in detail) { + requireMergedField(isIsoTimestamp(detail.created_at), "created_at"); + } + if (detail.updated_at != null) { + requireMergedField(isIsoTimestamp(detail.updated_at), "updated_at"); + } + for (const field of ["subtype", "description", "canonical_name", "source_stage"]) { + if (detail[field] != null) requireMergedField(typeof detail[field] === "string", field); + } + if (detail.confidence != null) requireMergedField(isConfidence(detail.confidence), "confidence"); + if (detail.embedding != null) { + requireMergedField( + Array.isArray(detail.embedding) && + detail.embedding.every((value) => typeof value === "number" && Number.isFinite(value)), + "embedding", + ); + } + if (detail.metadata != null) requireMergedField(isRecord(detail.metadata), "metadata"); + if (detail.relationships != null) { + requireMergedField(Array.isArray(detail.relationships), "relationships"); + for (const ref of detail.relationships) { + requireMergedField(isRecord(ref), "relationship reference"); + for (const field of ["id", "type", "target_id"]) { + requireMergedField(isNonemptyString(ref[field]), `relationship ${field}`); + } + if (ref.target_name != null) { + requireMergedField(typeof ref.target_name === "string", "relationship target_name"); + } + if (ref.properties != null) { + requireMergedField(isRecord(ref.properties), "relationship properties"); + } + } + } + if (!isNonemptyString(detail.name)) return undefined; + requireMergedField(isNonemptyString(detail.id), "missing canonical id"); + requireMergedField(isNonemptyString(detail.type), "missing type"); + return detail as unknown as WireEntity; +} + export class LongTermMemory { constructor(private readonly transport: Transport) {} @@ -192,13 +288,67 @@ export class LongTermMemory { entityType: string, options?: { description?: string }, ): Promise { - const wire = await this.transport.request("add_entity", { + const wire = await this.transport.request("add_entity", { name, entity_type: entityType, type: entityType, description: options?.description, }); - return toEntity(wire); + // NAMS resolves-before-create: a sufficiently similar name merges onto an + // existing entity and the response carries no name/type — follow up with + // a GET for the canonical merged-into record. + if (wire && typeof wire === "object" && "resolution" in wire && wire.resolution === "merged") { + return this.resolveMergedEntity(wire, name, entityType); + } + return toEntity(wire as WireEntity); + } + + /** + * Turn a `resolution: "merged"` create response into the canonical Entity. + * + * A 404 or an empty/missing-name canonical response falls back to request + * fields, provided the response includes a valid merge identifier. Other + * failures propagate. A fallback's createdAt is its local construction + * time; metadata.nams_resolution records whether fallback was necessary. + */ + private async resolveMergedEntity( + wire: WireMergedResolution, + name: string, + entityType: string, + ): Promise { + const mergedId = [wire.merged_into, wire.id].find( + (value) => value != null && !(typeof value === "string" && value.trim() === ""), + ); + requireMergedField(isNonemptyString(mergedId), "missing merge identifier"); + if (wire.metadata != null) requireMergedField(isRecord(wire.metadata), "metadata"); + + let detail: unknown; + try { + detail = await this.transport.request("get_entity", { entity_id: mergedId }); + } catch (error) { + if (!(error instanceof TransportError) || error.statusCode !== 404) throw error; + } + const canonical = mergedEntityDetail(detail, mergedId); + const canonicalMetadata = isRecord(detail) && isRecord(detail.metadata) ? detail.metadata : undefined; + const entity = canonical ?? { + id: mergedId, + name, + type: entityType.toLowerCase(), + created_at: new Date().toISOString(), + }; + return toEntity({ + ...entity, + metadata: { + ...wire.metadata, + ...canonicalMetadata, + nams_resolution: { + resolution: "merged", + merged_into: mergedId, + ...(wire.confidence == null ? {} : { merge_confidence: wire.confidence }), + fallback: canonical === undefined, + }, + }, + }); } async addPreference( diff --git a/typescript/src/types.ts b/typescript/src/types.ts index 689e36f5..75e8ae72 100644 --- a/typescript/src/types.ts +++ b/typescript/src/types.ts @@ -117,6 +117,8 @@ export interface Entity { sourceStage?: string; /** Hosted service: relationships referenced by getEntity. */ relationships?: EntityRelationshipRef[]; + /** Entity metadata; hosted merge provenance is stored under `nams_resolution`. */ + metadata?: Record; } export interface EntityRelationshipRef { diff --git a/typescript/test/integration/rest-transport.test.ts b/typescript/test/integration/rest-transport.test.ts index 9901c783..d2ad333d 100644 --- a/typescript/test/integration/rest-transport.test.ts +++ b/typescript/test/integration/rest-transport.test.ts @@ -10,7 +10,7 @@ import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest import { http, HttpResponse } from "msw"; import { setupServer } from "msw/node"; import { MemoryClient } from "../../src/client.js"; -import { AuthenticationError, NotSupportedError, TransportError } from "../../src/errors.js"; +import { AuthenticationError, ConnectionError, NotSupportedError, TransportError, ValidationError } from "../../src/errors.js"; const ENDPOINT = "https://memory.test/v1"; const API_KEY = "nams_test_key"; @@ -178,6 +178,111 @@ describe("RestTransport — long-term", () => { }); }); +describe("RestTransport — merged entity creation", () => { + const entityId = "e1"; + const mergedResponse = { id: entityId, resolution: "merged", mergedInto: entityId, confidence: 0.93 }; + + function mockMergedRead(response: () => Response) { + const created = vi.fn(() => HttpResponse.json(mergedResponse)); + const fetched = vi.fn(response); + server.use( + http.post(`${ENDPOINT}/entities`, created), + http.get(`${ENDPOINT}/entities/${entityId}`, fetched), + ); + return { created, fetched }; + } + + it("fetches and validates the camelCase canonical response without replacing entity confidence", async () => { + const calls = mockMergedRead(() => HttpResponse.json({ + id: entityId, name: "Alice", type: "person", confidence: 0.5, + createdAt: "2026-09-10T12:00:00Z", updatedAt: null, + metadata: { source: "original", nested: { retained: null } }, + relationships: [{ id: "r1", type: "KNOWS", targetId: "e2", targetName: null, properties: null }], + })); + const client = newClient(); + const entity = await client.longTerm.addEntity("Alice Smith", "person"); + await client.close(); + expect(entity).toMatchObject({ id: entityId, name: "Alice", type: "person", confidence: 0.5 }); + expect(entity.updatedAt).toBeUndefined(); + expect(entity.relationships?.[0]?.targetName).toBeUndefined(); + expect(entity.relationships?.[0]?.properties).toBeUndefined(); + expect(entity.metadata).toEqual({ + source: "original", nested: { retained: null }, + nams_resolution: { resolution: "merged", merged_into: entityId, merge_confidence: 0.93, fallback: false }, + }); + expect(calls.created).toHaveBeenCalledTimes(1); + expect(calls.fetched).toHaveBeenCalledTimes(1); + }); + + it("falls back after the actual REST transport maps HTTP 404 to TransportError", async () => { + const calls = mockMergedRead(() => HttpResponse.json({ error: "not found" }, { status: 404 })); + const client = newClient(); + const entity = await client.longTerm.addEntity("Alice Smith", "PERSON"); + await client.close(); + expect(entity).toMatchObject({ id: entityId, name: "Alice Smith", type: "person" }); + expect(entity.confidence).toBeUndefined(); + expect(Number.isFinite(Date.parse(entity.createdAt))).toBe(true); + expect(entity.metadata?.nams_resolution).toEqual({ + resolution: "merged", merged_into: entityId, merge_confidence: 0.93, fallback: true, + }); + expect(calls.created).toHaveBeenCalledTimes(1); + expect(calls.fetched).toHaveBeenCalledTimes(1); + }); + + it.each([ + ["204", () => new HttpResponse(null, { status: 204 })], + ["empty 200", () => new HttpResponse(null, { status: 200 })], + ["null 200", () => HttpResponse.json(null)], + ["empty object", () => HttpResponse.json({})], + ["missing name", () => HttpResponse.json({ id: entityId, metadata: { retained: null } })], + ] as const)("falls back for a %s canonical response", async (_, response) => { + mockMergedRead(response); + const client = newClient(); + const entity = await client.longTerm.addEntity("Alice Smith", "person"); + await client.close(); + expect(entity).toMatchObject({ id: entityId, name: "Alice Smith", type: "person" }); + expect(entity.metadata?.nams_resolution).toHaveProperty("fallback", true); + if (_ === "missing name") expect(entity.metadata).toHaveProperty("retained", null); + }); + + it.each([401, 403, 429, 500, 503])("propagates HTTP %s from the canonical read with requestId", async (status) => { + mockMergedRead(() => HttpResponse.json({ error: "canonical read failed" }, { + status, headers: { "x-request-id": "canonical-request" }, + })); + const client = newClient(); + try { + await expect(client.longTerm.addEntity("Alice Smith", "person")).rejects.toMatchObject({ + name: status === 401 || status === 403 ? "AuthenticationError" : "TransportError", + requestId: "canonical-request", + ...(status === 401 || status === 403 ? {} : { statusCode: status }), + }); + } finally { + await client.close(); + } + }); + + it("propagates network failures from the canonical read", async () => { + mockMergedRead(() => HttpResponse.error()); + const client = newClient(); + await expect(client.longTerm.addEntity("Alice Smith", "person")).rejects.toBeInstanceOf(ConnectionError); + await client.close(); + }); + + it("propagates invalid JSON from the canonical read", async () => { + mockMergedRead(() => new HttpResponse("{", { headers: { "Content-Type": "application/json" } })); + const client = newClient(); + await expect(client.longTerm.addEntity("Alice Smith", "person")).rejects.toBeInstanceOf(SyntaxError); + await client.close(); + }); + + it("rejects a populated malformed canonical payload", async () => { + mockMergedRead(() => HttpResponse.json({ id: entityId, name: "Alice", type: "person", createdAt: null })); + const client = newClient(); + await expect(client.longTerm.addEntity("Alice Smith", "person")).rejects.toBeInstanceOf(ValidationError); + await client.close(); + }); +}); + describe("RestTransport — error handling", () => { it("returns AuthenticationError on 401", async () => { server.use( diff --git a/typescript/test/unit/add-entity-merged.test.ts b/typescript/test/unit/add-entity-merged.test.ts new file mode 100644 index 00000000..81b08014 --- /dev/null +++ b/typescript/test/unit/add-entity-merged.test.ts @@ -0,0 +1,275 @@ +/** + * Unit test — LongTermMemory.addEntity merged-resolution handling + * (transport mocked). + * + * NAMS resolves-before-create: a sufficiently similar name merges onto an + * existing entity and POST /entities responds + * `{id, resolution: "merged", merged_into, confidence}` with no name/type. + * addEntity must fetch the canonical merged-into entity instead of returning + * a malformed one (mirrors the Python SDK fix). + */ + +import { afterEach, describe, it, expect, vi } from "vitest"; +import { LongTermMemory } from "../../src/long-term/index.js"; +import { + AuthenticationError, + ConnectionError, + NotFoundError, + NotSupportedError, + TransportError, + ValidationError, +} from "../../src/errors.js"; + +const ENTITY_ID = "00000000-0000-0000-0000-000000000001"; + +const CANONICAL_WIRE = { + id: ENTITY_ID, + name: "Alice", + type: "person", + description: "Test entity", + confidence: 0.95, + created_at: "2026-05-17T12:00:00Z", + relationships: [], +}; + +const MERGED_WIRE = { + id: ENTITY_ID, + resolution: "merged", + merged_into: ENTITY_ID, + confidence: 0.93, +}; + +function mockTransport(handler: (method: string, params: Record) => unknown) { + return { request: vi.fn(async (m: string, p: Record) => handler(m, p)) }; +} + +describe("LongTermMemory.addEntity merged resolution", () => { + afterEach(() => vi.useRealTimers()); + + it("fetches the canonical entity when the create merged", async () => { + const t = mockTransport((method) => { + if (method === "add_entity") return MERGED_WIRE; + if (method === "get_entity") return CANONICAL_WIRE; + throw new Error(`unexpected method ${method}`); + }); + const lt = new LongTermMemory(t as never); + const entity = await lt.addEntity("Alice Smith", "person"); + const methods = t.request.mock.calls.map((c) => c[0]); + expect(methods).toEqual(["add_entity", "get_entity"]); + expect(t.request.mock.calls[1]?.[1]).toMatchObject({ entity_id: ENTITY_ID }); + // The canonical (merged-into) record wins over the requested name. + expect(entity.id).toBe(ENTITY_ID); + expect(entity.name).toBe("Alice"); + expect(entity.type).toBe("person"); + expect(entity.confidence).toBe(0.95); + expect(entity.metadata?.nams_resolution).toEqual({ + resolution: "merged", + merged_into: ENTITY_ID, + merge_confidence: 0.93, + fallback: false, + }); + }); + + it("normalizes null fields on the canonical entity to undefined", async () => { + // NAMS projects unset node properties as JSON null — a manually created + // entity has no confidence/sourceStage/updatedAt. + const t = mockTransport((method) => { + if (method === "add_entity") return MERGED_WIRE; + if (method === "get_entity") { + return { + id: ENTITY_ID, + name: "Alice", + type: "person", + description: null, + confidence: null, + source_stage: null, + created_at: "2026-05-17T12:00:00Z", + updated_at: null, + relationships: [], + }; + } + throw new Error(`unexpected method ${method}`); + }); + const lt = new LongTermMemory(t as never); + const entity = await lt.addEntity("Alice Smith", "person"); + expect(entity.name).toBe("Alice"); + expect(entity.confidence).toBeUndefined(); + expect(entity.description).toBeUndefined(); + expect(entity.updatedAt).toBeUndefined(); + }); + + it("falls back on 404 with a hosted type, local ISO timestamp, and separate merge confidence", async () => { + vi.useFakeTimers(); + vi.setSystemTime(new Date("2026-09-10T12:00:00Z")); + const t = mockTransport((method) => { + if (method === "add_entity") return MERGED_WIRE; + throw new TransportError("get_entity failed: entity not found", 404); + }); + const lt = new LongTermMemory(t as never); + const entity = await lt.addEntity("Alice Smith", "PERSON"); + expect(entity.id).toBe(ENTITY_ID); + expect(entity.name).toBe("Alice Smith"); + expect(entity.type).toBe("person"); + expect(entity.confidence).toBeUndefined(); + expect(entity.createdAt).toBe("2026-09-10T12:00:00.000Z"); + expect(entity.metadata?.nams_resolution).toEqual({ + resolution: "merged", + merged_into: ENTITY_ID, + merge_confidence: 0.93, + fallback: true, + }); + expect(t.request.mock.calls[0]?.[1]).toMatchObject({ type: "PERSON", entity_type: "PERSON" }); + }); + + it.each([ + new AuthenticationError("credentials expired", { requestId: "auth-request" }), + new ConnectionError("network unavailable"), + new TransportError("rate limited", 429), + new TransportError("server failure", 500), + new NotFoundError("not a REST 404"), + new NotSupportedError("unsupported transport"), + new ValidationError("invalid request"), + new DOMException("timed out", "TimeoutError"), + new Error("programming error"), + ])("preserves unrelated follow-up errors: %s", async (boom) => { + const t = mockTransport((method) => { + if (method === "add_entity") return MERGED_WIRE; + throw boom; + }); + const lt = new LongTermMemory(t as never); + await expect(lt.addEntity("Alice Smith", "person")).rejects.toBe(boom); + }); + + it.each([ + undefined, + null, + {}, + { id: ENTITY_ID }, + { id: ENTITY_ID, name: null, type: "person" }, + { id: ENTITY_ID, name: "", type: "person" }, + { id: ENTITY_ID, name: " ", type: "person" }, + ])("falls back for an empty or missing-name canonical response: %j", async (detail) => { + const t = mockTransport((method) => method === "add_entity" ? MERGED_WIRE : detail); + const entity = await new LongTermMemory(t as never).addEntity("Alice Smith", "person"); + expect(entity).toMatchObject({ id: ENTITY_ID, name: "Alice Smith", type: "person" }); + expect(entity.confidence).toBeUndefined(); + expect(Number.isFinite(Date.parse(entity.createdAt))).toBe(true); + expect(entity.metadata?.nams_resolution).toMatchObject({ fallback: true }); + }); + + it.each([ + [], + "entity", + { ...CANONICAL_WIRE, id: "wrong-id" }, + { ...CANONICAL_WIRE, id: 123 }, + { ...CANONICAL_WIRE, id: null }, + { ...CANONICAL_WIRE, id: "" }, + { name: "Alice", type: "person" }, + { id: ENTITY_ID, name: "Alice" }, + { ...CANONICAL_WIRE, name: 123 }, + { ...CANONICAL_WIRE, type: null }, + { ...CANONICAL_WIRE, type: "" }, + { ...CANONICAL_WIRE, created_at: null }, + { ...CANONICAL_WIRE, created_at: "" }, + { ...CANONICAL_WIRE, created_at: "yesterday" }, + { ...CANONICAL_WIRE, created_at: "2026-02-30T12:00:00Z" }, + { ...CANONICAL_WIRE, updated_at: "invalid" }, + { ...CANONICAL_WIRE, confidence: "0.9" }, + { ...CANONICAL_WIRE, confidence: 2 }, + { ...CANONICAL_WIRE, embedding: [null] }, + { ...CANONICAL_WIRE, metadata: [] }, + { ...CANONICAL_WIRE, relationships: [null] }, + { ...CANONICAL_WIRE, relationships: [{ id: "r1", type: "KNOWS" }] }, + // A missing name must not hide malformed fields elsewhere in the record. + { id: "wrong-id" }, + { id: ENTITY_ID, description: {} }, + { id: ENTITY_ID, created_at: "invalid" }, + ])("rejects malformed populated canonical fields: %j", async (detail) => { + const t = mockTransport((method) => method === "add_entity" ? MERGED_WIRE : detail); + await expect(new LongTermMemory(t as never).addEntity("Alice Smith", "person")) + .rejects.toBeInstanceOf(ValidationError); + }); + + it.each([ + { resolution: "merged" }, + { resolution: "merged", id: "" }, + { resolution: "merged", id: 123 }, + { resolution: "merged", id: null }, + { resolution: "merged", merged_into: " " }, + { ...MERGED_WIRE, merged_into: 123 }, + ])("rejects an invalid merge envelope before requesting the canonical entity: %j", async (wire) => { + const t = mockTransport(() => wire); + await expect(new LongTermMemory(t as never).addEntity("Alice Smith", "person")) + .rejects.toBeInstanceOf(ValidationError); + expect(t.request).toHaveBeenCalledTimes(1); + }); + + it("uses id when merged_into is absent", async () => { + const t = mockTransport((method) => method === "add_entity" + ? { resolution: "merged", id: ENTITY_ID } + : CANONICAL_WIRE); + const entity = await new LongTermMemory(t as never).addEntity("Alice Smith", "person"); + expect(t.request.mock.calls[1]?.[1]).toEqual({ entity_id: ENTITY_ID }); + expect(entity.metadata?.nams_resolution).toEqual({ + resolution: "merged", merged_into: ENTITY_ID, fallback: false, + }); + }); + + it.each([ + { ...MERGED_WIRE, merged_into: null }, + { ...MERGED_WIRE, merged_into: "" }, + { ...MERGED_WIRE, merged_into: " " }, + { ...MERGED_WIRE, id: null }, + ])("selects the first nonempty merge identifier: %j", async (wire) => { + const t = mockTransport((method) => method === "add_entity" ? wire : CANONICAL_WIRE); + const entity = await new LongTermMemory(t as never).addEntity("Alice Smith", "person"); + expect(entity.id).toBe(ENTITY_ID); + expect(t.request.mock.calls[1]?.[1]).toEqual({ entity_id: ENTITY_ID }); + }); + + it.each([false, true])("preserves user metadata and null values (fallback=%s)", async (fallback) => { + const metadata = { source: "import", nested: { retained: null } }; + const t = mockTransport((method) => { + if (method === "add_entity") return { ...MERGED_WIRE, metadata }; + return fallback + ? { id: ENTITY_ID, metadata: { source: "canonical", other: null } } + : { ...CANONICAL_WIRE, metadata: { source: "canonical", other: null } }; + }); + const entity = await new LongTermMemory(t as never).addEntity("Alice Smith", "person"); + expect(entity.metadata).toMatchObject({ source: "canonical", nested: { retained: null }, other: null }); + expect(entity.metadata?.nams_resolution).toMatchObject({ fallback }); + expect(metadata).toEqual({ source: "import", nested: { retained: null } }); + }); + + it.each([undefined, null, 0, -1, 2, "0.93"])("preserves the original merge score as metadata: %j", async (confidence) => { + for (const detail of [CANONICAL_WIRE, undefined]) { + const t = mockTransport((method) => method === "add_entity" + ? { ...MERGED_WIRE, confidence } + : detail); + const entity = await new LongTermMemory(t as never).addEntity("Alice Smith", "person"); + const resolution = entity.metadata?.nams_resolution; + if (confidence == null) expect(resolution).not.toHaveProperty("merge_confidence"); + else expect(resolution).toHaveProperty("merge_confidence", confidence); + expect(entity.confidence).toBe(detail ? 0.95 : undefined); + } + }); + + it.each([undefined, "created", "review_pending"])("parses non-merged responses directly: %s", async (resolution) => { + const t = mockTransport((method) => { + if (method === "add_entity") { + return { + ...CANONICAL_WIRE, + resolution, + duplicate_of: "00000000-0000-0000-0000-000000000002", + }; + } + throw new Error(`unexpected method ${method}`); + }); + const lt = new LongTermMemory(t as never); + const entity = await lt.addEntity("Alice", "person"); + // No follow-up GET — the response already carries the entity fields. + expect(t.request.mock.calls.map((c) => c[0])).toEqual(["add_entity"]); + expect(entity.name).toBe("Alice"); + expect(entity.type).toBe("person"); + }); +}); diff --git a/typescript/test/unit/long-term-null-fields.test.ts b/typescript/test/unit/long-term-null-fields.test.ts new file mode 100644 index 00000000..90b7750d --- /dev/null +++ b/typescript/test/unit/long-term-null-fields.test.ts @@ -0,0 +1,104 @@ +import { describe, expect, it, vi } from "vitest"; +import { LongTermMemory } from "../../src/long-term/index.js"; + +function memoryReturning(wire: unknown): LongTermMemory { + return new LongTermMemory({ request: vi.fn(async () => wire) } as never); +} + +describe("LongTermMemory nullable wire fields", () => { + it("normalizes entity and inline relationship optional fields", async () => { + const entity = await memoryReturning({ + id: "e1", + name: "Alice", + type: "person", + created_at: "2026-09-10T12:00:00Z", + subtype: null, + description: null, + embedding: null, + canonical_name: null, + updated_at: null, + confidence: null, + source_stage: null, + metadata: null, + relationships: [{ + id: "r1", type: "KNOWS", target_id: "e2", target_name: null, properties: null, + }], + }).getEntity("e1"); + for (const key of [ + "subtype", "description", "embedding", "canonicalName", "updatedAt", + "confidence", "sourceStage", "metadata", + ] as const) { + expect(entity[key]).toBeUndefined(); + } + expect(entity.relationships?.[0]).toEqual({ + id: "r1", type: "KNOWS", targetId: "e2", targetName: undefined, properties: undefined, + }); + }); + + it("preserves zero, empty strings and arrays, and nulls in user dictionaries", async () => { + const properties = { note: null, nested: { value: null } }; + const entity = await memoryReturning({ + id: "e1", name: "Alice", type: "person", created_at: "2026-09-10T12:00:00Z", + description: "", embedding: [], confidence: 0, metadata: properties, + relationships: [{ id: "r1", type: "KNOWS", target_id: "e2", target_name: "", properties }], + }).getEntity("e1"); + expect(entity.description).toBe(""); + expect(entity.embedding).toEqual([]); + expect(entity.confidence).toBe(0); + expect(entity.metadata).toEqual(properties); + expect(entity.relationships?.[0]?.targetName).toBe(""); + expect(entity.relationships?.[0]?.properties).toEqual(properties); + }); + + it("normalizes a null relationship list", async () => { + const entity = await memoryReturning({ + id: "e1", name: "Alice", type: "person", relationships: null, + }).getEntity("e1"); + expect(entity.relationships).toBeUndefined(); + }); + + it.each([null, undefined])("normalizes preference and fact optionals: %j", async (value) => { + const preference = await memoryReturning({ + id: "p1", category: "style", preference: "concise", context: value, embedding: value, + }).addPreference("style", "concise"); + expect(preference.context).toBeUndefined(); + expect(preference.embedding).toBeUndefined(); + const fact = await memoryReturning({ + id: "f1", subject: "Alice", predicate: "likes", object: "tea", embedding: value, + }).addFact("Alice", "likes", "tea"); + expect(fact.embedding).toBeUndefined(); + }); + + it("preserves empty preference context and embeddings", async () => { + const preference = await memoryReturning({ + id: "p1", category: "style", preference: "concise", context: "", embedding: [], + }).addPreference("style", "concise"); + expect(preference.context).toBe(""); + expect(preference.embedding).toEqual([]); + const fact = await memoryReturning({ + id: "f1", subject: "Alice", predicate: "likes", object: "tea", embedding: [], + }).addFact("Alice", "likes", "tea"); + expect(fact.embedding).toEqual([]); + }); + + it("normalizes mention message IDs without dropping an empty string", async () => { + const history = await memoryReturning({ + entity_id: "e1", + mentions: [null, undefined, ""].map((message_id) => ({ + conversation_id: "c1", message_id, content: "Alice", timestamp: "2026-09-10T12:00:00Z", + })), + }).getEntityHistory("e1"); + expect(history.mentions.map((mention) => mention.messageId)).toEqual([undefined, undefined, ""]); + }); + + it("defaults required relationship properties while preserving user null values", async () => { + const wire = { id: "r1", source_id: "e1", target_id: "e2", relationship_type: "KNOWS" }; + const relationship = await memoryReturning({ ...wire, properties: null }) + .addRelationship("e1", "e2", "KNOWS"); + expect(relationship.properties).toEqual({}); + const properties = { note: null, nested: { value: null } }; + const populated = await memoryReturning({ ...wire, properties }) + .addRelationship("e1", "e2", "KNOWS"); + expect(populated.properties).toEqual(properties); + }); +});