diff --git a/.github/workflows/ci-python.yml b/.github/workflows/ci-python.yml index 13af787b..25beb890 100644 --- a/.github/workflows/ci-python.yml +++ b/.github/workflows/ci-python.yml @@ -95,8 +95,13 @@ jobs: - name: Set up Python ${{ matrix.python-version }} run: uv python install ${{ matrix.python-version }} + # The `strands` extra is needed for the Strands integration's unit tests: + # tests/unit/integrations/test_strands_*.py all begin with + # `pytest.importorskip("strands")`, so without it they silently skip and + # this job reports green on code it never ran. Kept to that one extra -- + # the rest of the suite is designed to run on the dev group alone. - name: Install dependencies - run: uv sync --group dev + run: uv sync --group dev --extra strands - name: Run unit tests run: uv run pytest tests/unit -v --cov=neo4j_agent_memory --cov-report=xml --cov-report=term-missing diff --git a/CHANGELOG.md b/CHANGELOG.md index 37524762..922c0e57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Strands MemoryStore** (`Neo4jMemoryStore`) — cross-session recall for Strands + agents via `MemoryManager(stores=[...])`: long-term search, plus writes that feed + server-side extraction. Entities only on NAMS. Needs `strands-agents>=1.44.0`. + `get_tools()` adds `{name}_get_entity_graph` (multi-hop bolt, 1-hop NAMS) and, bolt-only with a configured user_id, the user-scoped `{name}_get_user_preferences`. + A store built from `settings=` owns its client and rebinds it when the event + loop changes (Strands' synchronous `Agent(...)` runs every call on a fresh + loop); a client passed as `client=` is never closed or reconnected, and a + loop change raises a named error instead of an opaque driver `RuntimeError`. + With a `user_id` set, preference recall is scoped to that user in both the + store and `Neo4jSessionManager` injection (`search_preferences` applies no + user filter, so an unscoped call could surface another tenant's + preferences). + Tool names are prefixed with the store's `name` so they coexist with + `context_graph_tools`' identically-named tools rather than silently + replacing them in the agent's registry, and `max_search_results` caps the + *total* rows per `search()` — shared across entities, preferences and facts + so no kind is crowded out. - **Strands SessionManager** (`Neo4jSessionManager`) — automatic conversation persistence/restore for AWS Strands agents via `Agent(session_manager=...)`, backed by any `MemoryClient` (bolt or NAMS). Includes opt-in long-term @@ -50,8 +67,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 they raised `AttributeError`). This lets `client.short_term` / `client.long_term` type-check without per-call-site `attr-defined` suppressions. +### Fixed + +- **`add_messages_batch` now accepts `user_identifier`**, enforcing `multi_tenant` + and linking the conversation to its `:User`; previously the bulk path silently + wrote unscoped, unlinked conversations — so a bulk write that used to succeed + now raises `ValueError` when `multi_tenant=True` and `user_identifier` is omitted. + ### Changed +- `strands` extra requires `strands-agents>=1.44.0` (was `>=0.1.0`). +- **`Neo4jSessionManager` now guards against a paired `Neo4jMemoryStore` duplicating its work**: raises if both would extract the same turns (always, on NAMS), warns once if both would inject context. +- `ShortTermProtocol.bulk_add_messages` takes explicit keyword-only params + (`generate_embeddings`, `extract_entities`, `extract_relations`, `user_identifier`) + instead of `**kwargs`. - **`MemoryClient` is now generic over its backend memory types** (`MemoryClient[ST, LT, RT]`, PEP 696 defaults). `client.short_term` / `.long_term` / `.reasoning` return the base `ShortTermProtocol` / diff --git a/docs/modules/ROOT/pages/how-to/integrations/aws-strands.adoc b/docs/modules/ROOT/pages/how-to/integrations/aws-strands.adoc index ecd75c85..9273bbb9 100644 --- a/docs/modules/ROOT/pages/how-to/integrations/aws-strands.adoc +++ b/docs/modules/ROOT/pages/how-to/integrations/aws-strands.adoc @@ -1,6 +1,6 @@ = AWS Strands Agents Integration :description: Integrate Neo4j Agent Memory with AWS Strands Agents SDK -:keywords: strands, aws, bedrock, neo4j, agent, tools, session manager +:keywords: strands, aws, bedrock, neo4j, agent, tools, session manager, memory store [NOTE] ==== @@ -12,25 +12,31 @@ wires up different Strands surfaces (`SnapshotStorage`, ==== This guide shows how to add Neo4j Context Graph memory to AWS Strands agents. -The integration offers two complementary modes, usable independently or +In Strands' own vocabulary: the session manager restores sessions, the +memory store feeds the agent loop. Three constructs, usable independently or together on the same agent: [cols="1,2,2"] |=== -| Mode | What it does | Reach for it when +| Construct | What it does | Reach for it when -| *Memory tools* (pull-based) -| `@tool` functions the agent calls on its own judgment: search memories, - explore the entity graph, store facts, read preferences. -| You want the agent in explicit control — deep queries, on-demand recall, - storing extra-conversational facts. +| *Memory store* (`MemoryStore`) +| `Neo4jMemoryStore` on `MemoryManager(stores=[...])`: long-term recall + (entities, preferences, facts) injected into the agent loop, plus writes + that feed server-side extraction. +| The preferred way to give an agent memory. Requires `strands-agents>=1.44.0`. | *Session manager* (push-based) | `Neo4jSessionManager` on `Agent(session_manager=...)`: every turn is - persisted automatically, history is restored on restart, and (opt-in) - relevant long-term memories are injected into each user message. -| You want zero-effort conversation capture and continuity — nothing depends + persisted automatically and history is restored on restart. +| You want zero-effort transcript capture and continuity — nothing depends on the model remembering to call a tool. + +| *Memory tools* (pull-based) +| `@tool` functions the agent calls on its own judgment: search memories, + explore the entity graph, store facts, read preferences. +| Deep graph queries the store doesn't cover, or `strands-agents<1.44` + (no `MemoryStore`). |=== [[_overview]] @@ -40,9 +46,9 @@ image::diagrams/multi-agent-architecture-aws-neo4j.png[Multi-agent architecture image::diagrams/shared-memory-data-flow-kyc-graph-credit.png[Shared memory data flow: KYC agent writes entities to Neo4j, Credit agent reads them — findings are immediately available across agents,width=540,align=left] -Both modes write into the same knowledge graph, which is what enables the -multi-agent "shared brain" shown above: what one agent learns, every other -agent can retrieve (see <<_the_shared_brain_pattern>>). +All three constructs write into the same knowledge graph, which is what +enables the multi-agent "shared brain" shown above: what one agent learns, +every other agent can retrieve (see <<_the_shared_brain_pattern>>). == Installation @@ -53,79 +59,312 @@ pip install neo4j-agent-memory[aws,strands] == Quick Start -This example combines both modes: the session manager captures and restores -the conversation transparently, while the tools give the agent explicit -memory operations. +`Neo4jMemoryStore` is the preferred way to give a Strands agent memory — +long-term recall injected straight into the agent loop: [source,python] ---- from strands import Agent +from strands.memory import ExtractionConfig, InvocationTrigger, MemoryManager + from neo4j_agent_memory import MemorySettings +from neo4j_agent_memory.integrations.strands import Neo4jMemoryStore, Neo4jMemoryStoreConfig + +settings = MemorySettings( + neo4j={"uri": "neo4j+s://xxx.databases.neo4j.io", "password": "your-password"}, +) +store = Neo4jMemoryStore( + Neo4jMemoryStoreConfig( + name="graph", + settings=settings, + user_id="user-123", + extraction=ExtractionConfig(trigger=InvocationTrigger()), + ) +) + +agent = Agent( + model="anthropic.claude-sonnet-4-20250514-v1:0", + memory_manager=MemoryManager(stores=[store]), +) + +agent("Remember that I prefer Python over JavaScript") +---- + +`extraction` is off by default and `MemoryManager` disables its `add_memory` +tool by default, so a store left at both defaults recalls but never writes. +`InvocationTrigger()` extracts on every turn; bare `extraction=True` means +every fifth. Writes go through `add_messages`, which the backend extracts +server-side — no extra model call. + +Add `Neo4jSessionManager` for transcript persistence/restore (see +<<_pairing_with_the_session_manager>>) and `context_graph_tools` for deep +graph queries the store doesn't cover — all three are independent and +combine on one agent: + +[source,python] +---- from neo4j_agent_memory.integrations.strands import ( - Neo4jRetrievalConfig, Neo4jSessionManager, context_graph_tools, ) -# Pull-based tools — the agent decides when to search or store. tools = context_graph_tools( neo4j_uri="neo4j+s://xxx.databases.neo4j.io", neo4j_password="your-password", embedding_provider="bedrock", ) -# Push-based session manager — every turn persisted, history restored, -# relevant memories injected into each user message (opt-in). -manager = Neo4jSessionManager( - "support-42", - settings=MemorySettings( - neo4j={ - "uri": "neo4j+s://xxx.databases.neo4j.io", - "password": "your-password", - }, - ), - retrieval_config=Neo4jRetrievalConfig(), +# The session manager owns extraction here, so this store is recall-only: +# a second config with extraction left at its default — see the pairing rule. +store = Neo4jMemoryStore( + Neo4jMemoryStoreConfig(name="graph", settings=settings, user_id="user-123") ) +manager = Neo4jSessionManager("support-42", settings=settings, extract_entities=True) agent = Agent( model="anthropic.claude-sonnet-4-20250514-v1:0", tools=tools, session_manager=manager, + memory_manager=MemoryManager(stores=[store]), ) - -agent("Remember that I prefer Python over JavaScript") ---- -Using the hosted NAMS service instead of self-hosted Neo4j? Swap in the -NAMS variants — both read `MEMORY_API_KEY` (and optionally -`MEMORY_ENDPOINT`) from the environment: +Using the hosted NAMS service instead of self-hosted Neo4j? `for_nams()` on +the store and the session manager, and `nams_context_graph_tools()`, all +read `MEMORY_API_KEY` (and optionally `MEMORY_ENDPOINT`) from the +environment: [source,python] ---- from neo4j_agent_memory.integrations.strands import ( - Neo4jRetrievalConfig, + Neo4jMemoryStore, + Neo4jMemoryStoreConfig, Neo4jSessionManager, nams_context_graph_tools, ) -manager = Neo4jSessionManager.for_nams( - "support-42", - retrieval_config=Neo4jRetrievalConfig(), -) +store = Neo4jMemoryStore.for_nams(Neo4jMemoryStoreConfig(name="graph")) +manager = Neo4jSessionManager.for_nams("support-42") tools = nams_context_graph_tools() ---- [IMPORTANT] ==== -Do *not* share a `MemoryClient` between the tools and the session manager. -The tools use their own cached clients with per-call open/close semantics; -the session manager holds a persistent client on its own event-loop thread. -Sharing one client would let a tool's teardown close the manager's transport -mid-session. Two transports per process is correct and cheap. +Do *not* share a `MemoryClient` between the tools factory, the session +manager, and the store. Each holds its own client with different lifecycle +semantics — sharing one risks a teardown closing another's transport +mid-session. Multiple transports per process is correct and cheap. ==== -Each mode also works on its own — the next two sections cover them -independently. +Each construct also works on its own — the following sections cover the +store, the tools, and the session manager independently. + +[[_memory_store]] +== Memory Store + +`Neo4jMemoryStore` implements Strands' `MemoryStore` protocol. Hand it to +`MemoryManager(stores=[...])` on `Agent(memory_manager=...)` and the manager +searches it for context and routes `add_memory`/programmatic `add()` calls +to it. + +=== Configuration + +`Neo4jMemoryStoreConfig` is a dataclass — every field is a checked +attribute, not a dict key. Pass exactly one of `client` (a pre-connected +`MemoryClient`, left open) or `settings` (the store builds and owns one). + +[cols="1,1,3"] +|=== +| Field | Default | Purpose + +| `name` +| _(required)_ +| Store name; also names the deterministic sink conversation. + +| `client` \| `settings` +| — +| Exactly one of a pre-connected `MemoryClient`, or `MemorySettings` to build one from. + +| `description` +| auto +| Shown to the model as the store's purpose. + +| `max_search_results` +| `None` +| Cap on the *total* rows one `search()` returns, shared across entities, + preferences and facts so no kind is crowded out. Falls back to the + manager's default (3) when unset. + +| `writable` +| `True` +| `False` disables `add()`/`add_messages()`. + +| `extraction` +| `False` +| Truthy enables store-side extraction. Leave `False` (recall-only) when paired with `Neo4jSessionManager` — see <<_pairing_with_the_session_manager>>. + +| `conversation_id` +| minted +| Explicit write-sink override; otherwise a deterministic name is minted in + `initialize()`. Do *not* point it at a chat conversation — see + <<_store_limitations>>. + +| `user_id` +| `None` +| Scopes *writes* to one tenant — on `bolt`, as a + `(:User)-[:HAS_CONVERSATION]->(:Conversation)` edge plus a denormalized + property; on NAMS, as a plain `userId` property on the conversation (no + `:User` node, no edge) that the conversation listing filters on. Also gates + the `get_user_preferences` tool, and scopes preference recall in `search()` + to that user — `search_preferences` applies no user filter, so a scoped + store lists the user's active preferences instead of searching all of them. + Entity and fact recall stay unscoped: no user-scoped primitive exists for + them. + +| `include_entities` / `include_preferences` / `include_facts` +| `True` +| Search fan-out. Preferences and facts are auto-gated off on NAMS (`NotSupportedError`). + +| `min_score` +| `0.2` +| Similarity floor. bolt only — NAMS ignores it. + +| `graph_tools` +| `True` +| Whether `get_tools()` returns anything. +|=== + +[source,python] +---- +from neo4j_agent_memory.integrations.strands import Neo4jMemoryStore, Neo4jMemoryStoreConfig + +store = Neo4jMemoryStore( + Neo4jMemoryStoreConfig(name="graph", client=client, user_id="user-123") +) +---- + +=== search(), add(), add_messages() + +`search(query)` fans out over entities, preferences, and facts and returns +`MemoryEntry` objects: `content` is a formatted line, `metadata` carries +`kind` (`entity`/`preference`/`fact`), `id`, `type`, and — bolt only — +`score`. **Entities only on NAMS**: `search_preferences`/`search_facts` +raise `NotSupportedError` there, so those two kinds are dropped from the +fan-out automatically. + +`add(content, metadata=None)` writes a message into the store's sink +conversation with extraction on by default — the one write path every +backend supports. Set `metadata["kind"]` to `"preference"`, `"fact"`, or +`"entity"` to route to a typed write (`add_preference`/`add_fact`/`add_entity`) +instead; a kind unsupported on the current backend (e.g. `"fact"` on NAMS) +falls back to the default sink and logs a warning once per store. + +`add_messages(messages, context)` bulk-ingests a batch of conversation turns +into the sink with extraction on — server-side on NAMS, inline on bolt — +so no extra model call happens. This is what a `MemoryManager` with +`extraction` truthy calls automatically. + +=== Graph tools + +`get_tools()` returns graph-native tools the manager itself can't provide, +bound to the store's own client. Names are prefixed with the store's `name` +so they coexist with the tools factory's (see <<_store_limitations>>) — +a store named `graph` yields `graph_get_entity_graph`: + +[cols="1,1,1,3"] +|=== +| Tool | bolt | NAMS | Notes + +| `{name}_get_entity_graph` +| yes +| yes +| Configurable `depth` on bolt (`get_related_entities`); NAMS traverses one hop only (`expand_graph`, keyed by node id — the entity name is resolved via search first). + +| `{name}_get_user_preferences` +| yes, if `user_id` set +| omitted +| Bolt-only, and only shipped when the store has a configured `user_id` (scopes the lookup to that tenant). NAMS has no preferences endpoint. +|=== + +[[_pairing_with_the_session_manager]] +=== Pairing with the session manager + +The session manager restores sessions; the memory store feeds the agent +loop — different jobs, and pairing both on one agent is supported and +recommended: + +[cols="1,3"] +|=== +| Shape | Behaviour + +| Store alone +| Store owns extraction (`extraction=True` or an explicit extractor) — the textbook Strands split. + +| Store + session manager (recommended) +| Store stays recall-only (`extraction=False`, the default); the session manager persists the transcript and extracts (bolt in place, NAMS server-side). + +| Store + session manager, with the store also extracting +| Raises at construction — both would write and extract the same turns twice. Exception: bolt with the session manager's own `extract_entities=False` does not raise, since then only one side extracts — the store. +|=== + +Recommended configuration: + +[source,python] +---- +Neo4jSessionManager(..., extract_entities=True) # transcript + extraction +Neo4jMemoryStore(Neo4jMemoryStoreConfig(name="graph")) # recall only (extraction=False default) +---- + +If both extract, `Neo4jSessionManager` raises at agent construction: + +---- +Neo4jSessionManager and Neo4jMemoryStore 'graph' would write and extract the +same turns twice. Set extraction=False on Neo4jMemoryStore (recall only, +recommended), or extract_entities=False on Neo4jSessionManager (let the +store own extraction). +---- + +Separately, `Neo4jSessionManager`'s own `Neo4jRetrievalConfig` and the +manager's default injection both fold context into the last user message — +pairing them injects memory twice. The session manager logs a warning +(once) naming the fix; see <<_retrieval_injection>>. + +[[_store_limitations]] +=== Limitations + +* **The sink conversation is separate from the readable chat history.** + Paired with `Neo4jSessionManager`, a turn the store ingests exists twice + in the graph: once in the restorable transcript, once in the store's + sink. The duplication is in `Message` nodes, not knowledge — entities + converge through resolution and dedupe. + +* **Retry dedupe is in-process only.** `AddMessagesContext.sequence_numbers` + reset every run, so the `(run_id, sequence_number)` set that skips + already-written turns cannot outlive the process. A restart in the middle + of a retried extraction batch can duplicate messages in the sink. + +* **Pointing `conversation_id` at the chat conversation is unsupported.** + It duplicates `Message` nodes *inside* the readable history, so restored + transcripts gain phantom turns. Not guarded — leave `conversation_id` + unset and let the store mint its own sink. + +* **Owned clients rebind across event loops; borrowed ones do not.** + Strands' synchronous `Agent(...)` runs each call on a fresh event loop, + and both Neo4j transports bind to the loop that opened them. A store built + from `settings=` owns its client and reconnects it when the loop changes + (one reconnect per invocation). A client passed as `client=` is never + closed or reconnected on your behalf: a loop change raises, naming both + remedies. Drive the agent from your own loop, or hand the store + `settings=`. + +* **Tool names are namespaced.** `get_tools()` prefixes each tool with the + store's `name` (`graph_get_entity_graph`), so the store's tools coexist + with `context_graph_tools`' identically-purposed `get_entity_graph` / + `get_user_preferences` — which take different arguments — instead of + silently replacing them in the agent's tool registry. + +See `examples/strands-memory-store/` for a runnable demo (`search()`, +`add()`, `get_tools()`) that requires no LLM or API key. == Memory Tools (Pull-Based) @@ -148,6 +387,13 @@ The integration provides four memory tools that agents can use: | Retrieve user preferences by category |=== +[NOTE] +==== +`search_context` is superseded by `Neo4jMemoryStore` (<<_memory_store>>) for +recall. The factory remains supported for deep graph work the store doesn't +cover. +==== + [TIP] ==== When a session manager is attached (see <<_session_manager_push_based>>), @@ -426,8 +672,19 @@ Alternatively, pass an already-constructed (but not yet connected) `MemoryClient` via `memory_client=`; see the loop-binding note under <<_limitations>>. +[[_retrieval_injection]] === Retrieval Injection +[NOTE] +==== +For new code, `MemoryManager` injection (via `Neo4jMemoryStore`, see +<<_memory_store>>) supersedes `Neo4jRetrievalConfig`. Both together inject +memory twice; the session manager logs a warning when it detects the +combination. `Neo4jRetrievalConfig` remains fully supported — it is the only +injection path on `strands-agents<1.44` and configures its sources +declaratively rather than through a store. +==== + When a `Neo4jRetrievalConfig` is supplied, each user message triggers concurrent long-term searches (entities, preferences, and optionally facts). Matching results are prepended inside a `` block: diff --git a/docs/superpowers/specs/2026-08-19-strands-memory-store-design.md b/docs/superpowers/specs/2026-08-19-strands-memory-store-design.md new file mode 100644 index 00000000..982664f2 --- /dev/null +++ b/docs/superpowers/specs/2026-08-19-strands-memory-store-design.md @@ -0,0 +1,386 @@ +# Strands MemoryStore for Neo4j Agent Memory — Design + +**Date**: 2026-08-19 · **Status**: approved, not yet implemented +**Verified against**: `strands-agents==1.52.0` (PyPI), `@strands-agents/sdk@1.13.0` (npm) + +## Overview + +`Neo4jMemoryStore` implements Strands' long-term-memory `MemoryStore` protocol in both +SDKs, so `MemoryManager(stores=[Neo4jMemoryStore(...)])` recalls from a Neo4j graph +across sessions. Requested by both Strands reviewers in +[harness-sdk#3871](https://github.com/strands-agents/harness-sdk/pull/3871). + +It is an adapter: every protocol member has a backing primitive in the library already. + +## Goals + +- `MemoryStore` conformance in Python and TypeScript, same shape in both. +- `search()` over long-term memory: entities, preferences, facts. +- Server-side extraction via `add_messages` — no extra model call. +- Graph-native tools the manager cannot provide (`get_entity_graph`). +- Defined behaviour when paired with `Neo4jSessionManager`. + +## Non-Goals (v1) + +- No `MemoryManager` subclass — it is framework-owned and concrete. +- No buffered writes (see Write durability). +- No new NAMS capabilities; the store lives within NAMS's entity-only surface. +- No `searchFacts` in the TypeScript client — no REST endpoint behind it. +- No change to `Neo4jSessionManager` persistence behaviour beyond two guards. +- No TCK work: it certifies client conformance by tier and does not cover integrations. +- `search()` does not use `get_context()`, which blends short-term and reasoning memory. + +## Verified contract + +`MemoryStore` is a `Protocol` (Python) / `interface` (TypeScript). Required: `name`, +`description`, `max_search_results`, `writable`, `extraction`, `search()`. Optional: +`add()`, `add_messages()`, `initialize()`, `get_tools()`. + +Manager behaviour that shapes this design: + +| Fact | Source | +|---|---| +| Extraction mode follows the sinks a store defines: `add` only → client-side `ModelExtractor`; `add_messages` present → server-side, no model call | `memory/extraction/resolve_extraction_config.py` | +| `_has_method` inspects `type(store)`, so an inherited Protocol stub counts as not-implemented | `memory/types.py` | +| A writable store needs ≥1 sink; an extractor requires `add`; extraction without an extractor requires `add_messages` | `MemoryManager.__init__` | +| `search()` fans out with `return_exceptions=True`; per-store failures are logged, not fatal | `MemoryManager.search` | +| `add()` aggregates per-store failures into `AggregateMemoryError` | `MemoryManager.add` | +| `init_agent` calls `store.initialize()`, wires extraction triggers, registers injection middleware | `MemoryManager.init_agent` | +| Defaults: 3 search results per store, 5 injected entries, `IntervalTrigger(turns=5)` | `memory_manager.py` | +| Manager ships its own per-model-call injection: default on, trigger `"userTurn"`, folded into the last user message | `memory/types.py`, `injection/` | +| `Agent(session_manager=..., memory_manager=...)` are two first-class parameters; `agent.memory_manager` is public | `agent/agent.py:203-204` | +| `AddMessagesContext` carries only `sequence_numbers`, which reset every run — no session identity reaches the store | `memory/types.py` | + +## Positioning mandate + +Binding on implementation and docs. + +| Job | Preferred construct | Supersedes | +|---|---|---| +| Recall into the agent loop | **`Neo4jMemoryStore` in a `MemoryManager`** | `search_context` from `context_graph_tools`; `Neo4jRetrievalConfig` injection | +| Transcript persistence + restore | `Neo4jSessionManager` | nothing; carries no memory framing | +| Deep graph queries the store lacks | `context_graph_tools` | nothing; factory retained | +| strands < 1.44 | `retrieval_config` + tools | documented as the pre-`MemoryStore` path | + +In Strands' vocabulary: the session manager restores sessions, the memory store feeds the +agent loop. + +## Architecture — Python + +New module `src/neo4j_agent_memory/integrations/strands/memory_store.py`, exported from +the package `__init__` through the existing try/except ImportError guard. `_retrieval.py` +gains a `MemoryEntry`-shaped sibling to `_retrieve_context`; `_messages.py` is reused +unchanged. + +Shape follows the vended stores (`strands.vended_memory_stores.bedrock_knowledge_base`, +`test_memory_store`) for subclassing the Protocol, but config is a plain +`@dataclass`, not a `TypedDict`/`Unpack`: `MemoryStoreConfig` is never received +by any Strands API (`MemoryManager.__init__` takes `stores: list[MemoryStore]`, +i.e. instances) — the real contract, per `strands.memory.types`, is "a store +exposes the `MemoryStoreConfig` fields as attributes." Inheriting the +`TypedDict` bought nothing at runtime or at any boundary, while forcing +unchecked `config.get(...)` reads internally (`total=False` rules out +`config["..."]`). A dataclass gives `mypy --strict`-checked attribute access +throughout, and `dataclasses.replace(config, name="team")` reuses one config +across several stores (personal / team / org). + +```python +@dataclass +class Neo4jMemoryStoreConfig: + name: str + client: MemoryClient | None = None + settings: MemorySettings | None = None + description: str | None = None + max_search_results: int | None = None + writable: bool = True + extraction: ExtractionConfig | bool = False + conversation_id: str | None = None + user_id: str | None = None + include_entities: bool = True + include_preferences: bool = True + include_facts: bool = True + min_score: float = 0.2 + graph_tools: bool = True + + def __post_init__(self) -> None: ... # non-empty name; not both client and settings + + +class Neo4jMemoryStore(MemoryStore): + def __init__(self, config: Neo4jMemoryStoreConfig) -> None: ... # assigns the five protocol fields onto self + + @classmethod + def for_nams(cls, config: Neo4jMemoryStoreConfig, *, endpoint: str | None = None, api_key: str | None = None, transport_mode: TransportMode = "auto") -> Neo4jMemoryStore: ... +``` + +`for_nams` mirrors `Neo4jSessionManager.for_nams`, reading `MEMORY_API_KEY` / +`MEMORY_ENDPOINT` from the environment; it does not mutate the caller's +`config` — `dataclasses.replace(config, settings=build_nams_settings(...))` +returns a copy with `settings` injected and constructs from that. + +### Config + +`Neo4jMemoryStoreConfig`'s fields, beyond `name`/`description`/ +`max_search_results`/`writable`/`extraction` (the ones `MemoryStore`'s +protocol requires as store attributes): + +| Field | Default | Purpose | +|---|---|---| +| `client` \| `settings` | — | pre-connected `MemoryClient`, or settings the store builds one from | +| `conversation_id` | minted in `initialize()` | write sink target | +| `user_id` | `None` | scopes *writes* to one tenant — on bolt a `(:User)-[:HAS_CONVERSATION]->(:Conversation)` edge plus a denormalized property (`memory/short_term.py::_link_user_to_conversation`), on NAMS a plain `userId` property on the conversation, no node and no edge (`nams/short_term.py`) — and gates the `get_user_preferences` tool. Reads are not narrowed — the long-term search APIs take no user filter | +| `include_entities` | `True` | search fan-out | +| `include_preferences` | `True` | auto-gated off on NAMS | +| `include_facts` | `True` | auto-gated off on NAMS | +| `min_score` | `0.2` | bolt only; NAMS ignores `threshold` | +| `graph_tools` | `True` | expose `get_tools()` | + +Inherited defaults: `writable=True`, `extraction=False`, `max_search_results=None` +(defers to the manager's 3), `description` naming the graph. + +### Protocol mapping + +| Member | Maps to | Notes | +|---|---|---| +| `search` | concurrent `search_entities` / `search_preferences` / `search_facts` | reshape of `_retrieval.py` `_retrieve_context`. Per-kind failures isolated and logged; whole-search failures propagate. Limit precedence: `options["max_search_results"]` → `self.max_search_results` → manager default. The limit caps the *total* rows, handed out round-robin across the enabled kinds: Strands treats it as a per-store cap (`memory_manager.py`) and injection slices the concatenation to the same number, so a per-kind limit would let saturated entity hits crowd preferences and facts out entirely | +| `add` | default: message into the sink with extraction. `metadata["kind"]` ∈ `{preference, fact, entity}` routes to `add_preference` / `add_fact` / `add_entity`, passing `user_identifier` where the primitive takes one (only `add_preference` does) | `NotSupportedError` falls back to the sink, logged once. **Not** `long_term.add()`, which makes the whole string an entity *name* of type `OBJECT` (`memory/long_term.py:389-398`) | +| `add_messages` | `bulk_add_messages(sink_id, msgs, extract_entities=True)` | protocol alias forwarding kwargs to `add_messages_batch` (`memory/short_term.py:560-572`) | +| `initialize` | connect an owned client; mint the sink when `conversation_id` was omitted | idempotent | +| `get_tools` | `{name}_get_entity_graph`, `{name}_get_user_preferences` | bound to the store's own client, not the tools factory's cached clients. Excludes search/add, which would collide with `context_graph_tools`' `add_memory` and the manager's own tool of that name. Names carry the store's `name` as a prefix because `ToolRegistry.register_tool` skips its duplicate-name check for `@tool` functions (`supports_hot_reload` is always true there) and silently overwrites — so unprefixed names would replace the factory's identically-named tools, which take different arguments | + +Tool availability is backend-gated; the differing depth is stated in the tool description +the model sees: + +| Tool | bolt | NAMS / TypeScript | +|---|---|---| +| `get_entity_graph` | `get_related_entities`, configurable depth | `expand_graph` / `expandGraph` — 1 hop, keyed by node id, so the name resolves through `search_entities` first (`get_entity_by_name` unsupported on NAMS) | +| `get_user_preferences` | `get_preferences_for` | omitted — no preferences endpoint (`nams/long_term.py:459`) | + +`MemoryEntry.content` is the formatted line (reusing `_format_entity` / +`_format_preference` / `_format_fact`); `metadata` carries: + +| Key | Value | +|---|---| +| `kind` | `entity` \| `preference` \| `fact` | +| `id` | node id | +| `type` | `full_type` for entities, `category` for preferences | +| `score` | similarity, bolt only — `search_entities`, `search_preferences` and `search_facts` each set `metadata["similarity"]` (`memory/long_term.py:1059`, `:1126`, `:2065`); NAMS sets none, and an absent score is omitted rather than zeroed | + +### Both sinks on one class + +The class defines `add` **and** `add_messages`, so: extraction defaults to server-side +with no model call; `add` stays available for the manager's `add_memory` tool and +programmatic `MemoryManager.add`; an explicit `extraction={"extractor": ModelExtractor()}` +still validates. One class per SDK, no per-transport split. + +### Scoping and the sink conversation + +`AddMessagesContext` carries no session identity, so the store owns its scope — matching +the Bedrock KB precedent ("for per-tenant isolation, construct one store per scope"). + +- Writes go to a dedicated sink conversation. Its name is deterministic — + `strands-memory-store/{user_id or "_"}/{name}` — so restarts reuse one sink instead of + accumulating orphans. Resolution splits by backend, as + `Neo4jSessionManager._aresolve_conversation` does: + - **bolt**: the deterministic name *is* the conversation key, and the first write + auto-creates the conversation (`add_message` and `add_messages_batch` both call + `_ensure_conversation`). So resolution makes no backend call. Nothing is tagged: bolt's + `CREATE_CONVERSATION` (`graph/queries.py`) has no metadata property, and + `create_conversation` drops a `metadata` kwarg (`memory/short_term.py:521-524`). + - **NAMS**: ids are server-minted and client session ids are dropped, so the sink is + found by matching `_STORE_KEY` metadata (accepted at creation, unsettable afterwards) + and the returned id is cached. +- Reads are conversation-independent; only `user_id` narrows them. +- Pointing the sink at the chat conversation duplicates `Message` nodes in the readable + history: documented as unsupported, not guarded. + +### Idempotency + +`CREATE_MESSAGE` uses `CREATE` with an internally generated `$id` — no caller-supplied +id, no `MERGE` (`graph/queries.py:68-86`) — so message writes are not idempotent, while +Strands' extraction writes are at-least-once. Mitigation: an in-process set of +`(run_id, sequence_number)`, `run_id` per store instance. Retries occur within one +process, which is what the set covers. NAMS offers nothing better. + +### Write durability + +Awaited writes. `write_mode="buffered"` applies only to explicit +`client.buffered.submit(cypher, params)` calls; the memory layers do not route through it +(`memory/buffered.py:99`, `__init__.py:877`), so using the buffer would mean hand-writing +Cypher and bypassing embeddings, extraction and message linking. It is also bolt-only. +Durability stays where Strands puts it: background extraction plus +`MemoryManager.flush()`, and `wait_for_writes` on the add tool. + +### Client ownership + +As the session manager: a store built from `settings` owns its client and closes it via +`aclose()` / async context manager; a store handed a live `MemoryClient` never closes it. + +Both transports bind to the event loop that opened them, and Strands' synchronous +entry points run every call on a fresh loop (`strands._async.run_async` is +`asyncio.run` in a worker thread), so `Agent.__init__` initializes the store on one +loop and each `Agent.__call__` drives it from another. `initialize()` therefore +records the running loop: on a change it closes and reconnects an **owned** client, +and raises a named error for a **borrowed** one — reconnecting someone else's client +is not the store's call, and the alternative is an opaque `RuntimeError` from inside +the driver. +The existing warning against sharing one client between the tools factory and the session +manager extends to the store. + +## Coexistence with Neo4jSessionManager + +The pairing is legitimate — two first-class `Agent` parameters, and transcript and +knowledge are different jobs. Two overlaps need handling. + +Both guards live in `session_manager.py`, not the store: `initialize()` receives no +agent, while the session manager gets one at `AgentInitializedEvent` and +`agent.memory_manager` is public. The store therefore stays free of Strands internals. +Enumerating the manager's stores reads `MemoryManager._stores`, a private — one read, in +one file, pinned by a test that fails loudly on a strands upgrade. + +### Guard 1 — double extraction (raises) + +The store can only extract by re-writing turns the session manager already persisted: +`extract_entities_from_session` (extract in place) is bolt-only +(`memory/short_term.py:1111`) and cannot be the portable path. Compounding it, NAMS +`add_message` accepts only `{content, role}` and silently drops extraction kwargs +(`nams/short_term.py:287-289`), so on NAMS `Neo4jSessionManager(extract_entities=...)` is +a no-op and a sink conversation is extracted a second time regardless. + +| Setup | Behaviour | +|---|---| +| Store alone | store owns extraction — the textbook Strands split | +| Store + session manager | store recall-only; session manager persists, bolt extracts in place, NAMS server-side | +| Store + session manager, `store.extraction` truthy | raises at construction, naming both one-line fixes | + +Recommended paired configuration, which the docs lead with: + +```python +Neo4jSessionManager(..., extract_entities=True) # transcript + extraction +Neo4jMemoryStore(name="graph") # recall only (extraction=False default) +``` + +### Guard 2 — double injection (warns) + +Manager injection is default-on and lands where `Neo4jRetrievalConfig` does — both fold +into the last user message on a user turn — so a current `retrieval_config` user adopting +the store gets two search fan-outs and two blocks. + +`Neo4jRetrievalConfig` is kept and fully supported: it is the only injection path below +strands 1.44, and its sources are configured declaratively rather than decided by a +store. When the session manager detects a `MemoryManager` injecting from our store it +logs a warning naming the duplication and both ways out, once per session-manager +instance. No deprecation, no runtime disabling. + +## Backend capability matrix + +Source: `nams/long_term.py:1-27` and its `NotSupportedError` methods. + +| Capability | bolt | NAMS | +|---|---|---| +| `search` entities | yes, with score | yes; no score, `threshold` ignored, singular `type` | +| `search` preferences / facts | yes | `NotSupportedError` → auto-gated off | +| `add` kind routing | entity / preference / fact | entity only; others fall back to the sink | +| `add_messages` | yes, `extract_entities=True` honoured | yes; extraction server-side and unconditional | +| `get_entity_graph` tool | yes, configurable depth | yes via `expand_graph`, 1 hop | +| `get_user_preferences` tool | yes | omitted | + +## TypeScript parity + +Same shape in TS idiom: one options object, `class Neo4jMemoryStore implements +MemoryStore`, type-only SDK imports so the published bundle keeps zero runtime +dependencies. `getTools()` reaches `tool()` through the existing lazy +`await import("@strands-agents/sdk")`. + +| Divergence | Handling | +|---|---| +| `searchFacts` absent; `search_preferences`, `get_related_entities`, `add_preference`, `add_fact`, `add_relationship`, `get_entity_by_name` are `"unsupported"` on REST (`transport/rest.ts:171-177`) | TS store is entities-only for `search()`. `includeFacts` / `includePreferences` are absent from the TS options type — a compile error, not a silently ignored flag. `get_entity_graph` is available via `expandGraph` (1 hop, `transport/rest.ts:284`); `get_user_preferences` is not | +| `bulkAddMessages` caps at 100 per call (`short-term/index.ts:283`) | store chunks internally | +| No extraction flags on TS writes | none needed — NAMS extracts server-side | +| No bolt transport | `minScore` accepted, meaningful on Python/bolt only | + +Those methods exist in the TS client because `BridgeTransport` is a generic +`POST {endpoint}/{snake_case_method}` forwarder used by the TCK against a Python +reference adapter: they work on the bridge, throw on REST. TS and Python agree on the +NAMS surface, so no live-API verification is needed. + +TS guards attach in `Neo4jConversationManager.initAgent`, the only place an agent +reference arrives. `Neo4jSessionStorage` used without the conversation manager therefore +gets no guard. + +## Error handling + +| Failure | Behaviour | +|---|---| +| One search kind fails | logged, skipped; other kinds still return | +| Whole `search` fails | propagates; the manager logs and continues with other stores | +| `add` / `add_messages` fails | propagates; the manager aggregates into `AggregateMemoryError` | +| `NotSupportedError` on a routed `add` | falls back to the sink, logged once per store | +| `initialize` fails | propagates, aborting agent construction as Strands intends | +| Paired with store extraction on | `ValueError` at construction | + +## Testing + +| Layer | Coverage | +|---|---| +| Python unit | `search` fan-out with per-kind isolation; limit precedence; `MemoryEntry` metadata; `add` kind routing incl. `NotSupportedError` fallback; `add_messages` dedupe across a retried batch; sink minting idempotence; `get_tools`; client ownership; both guards | +| Python integration | bolt via docker-compose. A key-gated NAMS suite is **not yet implemented** | +| TS unit | mirrored test names against the existing msw/bridge setup, in `test/unit/strands/memory-store.test.ts` | +| Guards | extend `tests/unit/integrations/strands_fakes.py` with a fake agent exposing `memory_manager`; assert the private-attribute read fails loudly if strands moves it | +| Examples | `tests/examples/test_no_phantom_methods.py` covers the new example automatically | + +Cross-language parity rests on mirrored test names and this spec's tables, not the TCK. + +## Documentation requirements + +1. Both guides lead their Quick Start with the store; `aws-strands.adoc` currently leads + with tools + session manager and gets restructured. +2. Memory-tools section: `search_context` is superseded by the store for recall; the + factory is retained for deep graph work. +3. `Neo4jRetrievalConfig` section: superseded for injection by `MemoryManager` injection, + consistent with Guard 2. +4. Both guides state the split in Strands' vocabulary and carry the three deployment + shapes plus the pairing rule. +5. "Memory" comes off the session-manager surface wherever it is not load-bearing. +6. TS guide states the store is entities-only, and why. + +The guides were cross-linked in PR #181; the `MemoryStore` sections land in that +structure. + +## Packaging + +- `pyproject.toml`: `strands` extra `strands-agents>=0.1.0` → `>=1.44.0` (`MemoryStore` + landed in 1.44.0; current release 1.52.0). Forces an upgrade on existing + session-manager users — accepted. +- `typescript/package.json`: devDep `@strands-agents/sdk` `^1.2.0` → `^1.13.0` (TS memory + landed in 1.6.0). +- CHANGELOG entries in both SDKs. +- Examples: `examples/strands-memory-store/` (Python, `llm=None` + local + sentence-transformers, no API keys, wired into `example-tests` CI) and a memory-store + variant under `typescript/examples/strands`. + +## Sequencing + +| PR | Contents | +|---|---| +| 1 | TS module split: `src/integrations/strands.ts` (846 lines) → `src/integrations/strands/` with `index.ts` re-exporting; no behaviour change. First, so the store lands on the final layout | +| 2 | Python store: implementation, tests, example, guide section, floor bump | +| 3 | TS store: implementation, tests, example, guide section, devDep bump | + +Then a second Strands catalog entry (`integrationType: memory-store`), bundled with the +pending `maintainedBy: partner` and session-manager description retune — one YAML PR +against `strands-agents/harness-sdk`, needing explicit approval. + +## Limitations (documented, accepted) + +- Retried extraction batches dedupe in-process only; a restart mid-retry can duplicate + messages in the sink conversation. +- Relevance scores are bolt-only. +- The TS store is entities-only, so `search()` is narrower than Python-on-bolt, and its + `get_entity_graph` traverses one hop rather than a configurable depth. +- A store paired with the session manager writes turns to a sink separate from the + readable history, so turn text exists twice in the graph. Entities converge via + resolution/dedupe; the duplication is in messages, not knowledge. +- The guards read two private strands attributes: `MemoryManager._stores` (guard 1) + and `MemoryManager._injection_config` (guard 2). Both reads are in one file and + pinned by a test that fails loudly on a strands rename. +- No TS guard when `Neo4jSessionStorage` is used without `Neo4jConversationManager`. diff --git a/examples/strands-memory-store/README.md b/examples/strands-memory-store/README.md new file mode 100644 index 00000000..543e5d9f --- /dev/null +++ b/examples/strands-memory-store/README.md @@ -0,0 +1,104 @@ +# Strands MemoryStore — long-term recall + +![Neo4j Labs](https://img.shields.io/badge/Neo4j-Labs-6366F1?logo=neo4j) +![Status: Beta](https://img.shields.io/badge/Status-Beta-6366F1) +![Community Supported](https://img.shields.io/badge/Support-Community-6B7280) + +> `Neo4jMemoryStore` implements Strands' `MemoryStore` protocol: pass it to +> `MemoryManager(stores=[...])` and the agent loop recalls entities, +> preferences, and facts from a Neo4j graph across sessions. + +> ⚠️ **Neo4j Labs Project** +> +> This example is part of [`neo4j-agent-memory`](https://github.com/neo4j-labs/agent-memory), a Neo4j Labs project. It is actively maintained but not officially supported. APIs may change. Community support is available via the [Neo4j Community Forum](https://community.neo4j.com). + +## What this demonstrates + +- **`search()`** — fans out over entities, preferences, and facts (entities + only on hosted NAMS), returning `MemoryEntry` objects with a formatted + `content` string and a `metadata["kind"]` tag. +- **`add()`** — default sink writes a message with extraction on; + `metadata["kind"]` (`"preference"` / `"fact"` / `"entity"`) routes to a + typed write instead. +- **`get_tools()`** — graph-native tools that a `MemoryManager` cannot provide + on its own: `{name}_get_entity_graph`, and — bolt-only, with a configured + `user_id` — `{name}_get_user_preferences`. The store's `name` prefixes them + so they coexist with `context_graph_tools`' identically-named tools. + +`Neo4jSessionManager` (`examples/strands-session-manager/`) remains for +transcript persistence — the session manager restores sessions, the memory +store feeds the agent loop. See the guide's "Pairing with the session +manager" section for combining both on one agent. + +## Prerequisites + +- Neo4j 5.x running at `bolt://localhost:7687` + (or set `NEO4J_URI`, `NEO4J_USERNAME`, `NEO4J_PASSWORD`). +- `neo4j-agent-memory[strands]` installed: + ```bash + uv pip install "neo4j-agent-memory[strands]" + # or, for the local sentence-transformers embedder: + uv pip install "neo4j-agent-memory[strands,sentence-transformers]" + ``` + +## Run + +```bash +make neo4j-start +NEO4J_PASSWORD=test-password uv run python examples/strands-memory-store/main.py +``` + +No LLM API key required — `llm=None` plus a local `sentence-transformers` +embedder. + +The container's data volume persists across `make neo4j-stop` / `neo4j-start` +(only `neo4j-clean` wipes it). If this container previously ran against a +different-dimension embedder (e.g. OpenAI's 1536-dim default), connecting +here fails with `EmbeddingDimensionMismatchError`, not a silent problem — +run `make neo4j-clean && make neo4j-start` to reset. + +Expected output: + +``` +search('what does the user prefer?'): + entity: [entity] Acme Corp (ORGANIZATION) + preference: [preference] ui: Prefers dark mode +add(...): {'kind': 'message', 'id': '...'} +tools: ['graph_get_entity_graph', 'graph_get_user_preferences'] +``` + +## With a real agent + +```python +from strands import Agent +from strands.memory import MemoryManager + +from neo4j_agent_memory.integrations.strands import ( + Neo4jMemoryStore, + Neo4jMemoryStoreConfig, +) + +store = Neo4jMemoryStore(Neo4jMemoryStoreConfig(name="graph", client=client)) + +agent = Agent( + model="anthropic.claude-sonnet-4-20250514-v1:0", + memory_manager=MemoryManager(stores=[store]), +) +``` + +## Files + +| File | Purpose | +|---|---| +| `main.py` | Seeds a preference and an entity, then exercises `search()`, `add()`, and `get_tools()`. | + +## Going further + +- **How-to guide:** `docs/modules/ROOT/pages/how-to/integrations/aws-strands.adoc` + — configuration, backend differences, and the session-manager pairing rule. + +## Support + +- 💬 [Neo4j Community Forum](https://community.neo4j.com) +- 🐛 [GitHub Issues](https://github.com/neo4j-labs/agent-memory/issues) +- 📖 [`neo4j-agent-memory` documentation](https://github.com/neo4j-labs/agent-memory#readme) diff --git a/examples/strands-memory-store/main.py b/examples/strands-memory-store/main.py new file mode 100644 index 00000000..e64e90d4 --- /dev/null +++ b/examples/strands-memory-store/main.py @@ -0,0 +1,75 @@ +"""Neo4jMemoryStore for Strands -- long-term recall, no API keys required. + +Neo4jMemoryStore is a Strands `MemoryStore`: hand it to +`MemoryManager(stores=[...])` for cross-session recall fed into the agent +loop. Neo4jSessionManager (examples/strands-session-manager/) remains for +transcript persistence -- the two are complementary, not alternatives; see +the "Pairing with the session manager" section of the guide. + + make neo4j-start + NEO4J_PASSWORD=test-password uv run python examples/strands-memory-store/main.py +""" + +from __future__ import annotations + +import asyncio +import os + +from pydantic import SecretStr + +from neo4j_agent_memory import MemoryClient, MemorySettings, Neo4jConfig +from neo4j_agent_memory.config.settings import ExtractionConfig, ExtractorType +from neo4j_agent_memory.integrations.strands import ( + Neo4jMemoryStore, + Neo4jMemoryStoreConfig, +) + + +def build_settings() -> MemorySettings: + """Local sentence-transformers embedder, no LLM -- runs with no API key. + + ``ExtractorType.NONE`` disables entity extraction so the demo works + whether or not spaCy / GLiNER extras are installed; the entity and + preference below are seeded directly instead. + """ + return MemorySettings( + neo4j=Neo4jConfig( + uri=os.getenv("NEO4J_URI", "bolt://localhost:7687"), + username=os.getenv("NEO4J_USERNAME", "neo4j"), + password=SecretStr(os.getenv("NEO4J_PASSWORD", "test-password")), + ), + llm=None, + embedding="sentence-transformers/all-MiniLM-L6-v2", + extraction=ExtractionConfig(extractor_type=ExtractorType.NONE), + ) + + +async def main() -> None: + async with MemoryClient(build_settings()) as client: + # user_identifier: a store with a user_id recalls only that user's + # preferences, so an unscoped one would not show up below. + await client.long_term.add_preference("ui", "Prefers dark mode", user_identifier="alice") + await client.long_term.add_entity("Acme Corp", "ORGANIZATION") + + store = Neo4jMemoryStore( + Neo4jMemoryStoreConfig(name="graph", client=client, user_id="alice") + ) + await store.initialize() + + print("search('what does the user prefer?'):") + for entry in await store.search("what does the user prefer?"): + kind = (entry.metadata or {}).get("kind", "?") + print(f" {kind:>10}: {entry.content}") + + # Default sink: written as a message with extraction on -- the one + # write path every backend supports. metadata["kind"] routes to a + # typed write (preference/fact/entity) instead; see the guide. + result = await store.add("The user's deployment target is us-east-1") + print(f"add(...): {result}") + + # Graph-native tools a MemoryManager can't provide on its own. + print("tools:", [tool.tool_name for tool in store.get_tools()]) + + +if __name__ == "__main__": + asyncio.run(main()) diff --git a/pyproject.toml b/pyproject.toml index b2a4a9a7..352acfa0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -66,7 +66,7 @@ google = ["google-cloud-aiplatform>=1.38.0"] aws = ["boto3>=1.26.0"] # AWS Strands Agents SDK -strands = ["strands-agents>=0.1.0", "boto3>=1.26.0"] +strands = ["strands-agents>=1.44.0", "boto3>=1.26.0"] # Entity extraction spacy = ["spacy>=3.7.0"] diff --git a/src/neo4j_agent_memory/core/protocols.py b/src/neo4j_agent_memory/core/protocols.py index 250b27bc..418783cd 100644 --- a/src/neo4j_agent_memory/core/protocols.py +++ b/src/neo4j_agent_memory/core/protocols.py @@ -187,8 +187,22 @@ async def bulk_add_messages( self, session_id: str, messages: list[dict[str, Any]], + *, + generate_embeddings: bool = True, + extract_entities: bool = False, + extract_relations: bool = True, + user_identifier: str | None = None, ) -> list[Message]: - """Bulk-insert messages for a session in one round-trip, preserving order.""" + """Bulk-insert messages for a session in one round-trip, preserving order. + + The portable subset: honoured on bolt (including tenant scoping via + ``user_identifier``, enforced when ``multi_tenant=True``); ignored on + NAMS (extraction is server-side there, and tenancy comes from the API + key/workspace, not a per-call argument). Bolt-only tuning + (``batch_size``) and the progress callbacks are intentionally off + this protocol — reach for ``BoltMemoryClient.add_messages_batch`` + when you need those. + """ ... async def get_observations( diff --git a/src/neo4j_agent_memory/integrations/strands/__init__.py b/src/neo4j_agent_memory/integrations/strands/__init__.py index eb25b6c9..c4ab9a6c 100644 --- a/src/neo4j_agent_memory/integrations/strands/__init__.py +++ b/src/neo4j_agent_memory/integrations/strands/__init__.py @@ -59,6 +59,10 @@ def llm_provider_from_strands(model: Any) -> LLMProvider: BEDROCK_LLM_MODELS, StrandsConfig, ) + from neo4j_agent_memory.integrations.strands.memory_store import ( + Neo4jMemoryStore, + Neo4jMemoryStoreConfig, + ) from neo4j_agent_memory.integrations.strands.session_manager import ( Neo4jRetrievalConfig, Neo4jSessionManager, @@ -79,6 +83,8 @@ def llm_provider_from_strands(model: Any) -> LLMProvider: "llm_provider_from_strands", "Neo4jSessionManager", "Neo4jRetrievalConfig", + "Neo4jMemoryStore", + "Neo4jMemoryStoreConfig", ] except ImportError: # strands-agents not installed diff --git a/src/neo4j_agent_memory/integrations/strands/_retrieval.py b/src/neo4j_agent_memory/integrations/strands/_retrieval.py index c9aba28e..3249a90e 100644 --- a/src/neo4j_agent_memory/integrations/strands/_retrieval.py +++ b/src/neo4j_agent_memory/integrations/strands/_retrieval.py @@ -57,8 +57,35 @@ def _format_fact(fact: Fact) -> str: return f"[fact] {fact.subject} {fact.predicate} {fact.object}" +def _preference_search( + long_term: LongTermProtocol, user_id: str | None +) -> Callable[..., Awaitable[list[Any]]]: + """The preference lookup that is safe for this store's tenancy. + + ``search_preferences`` takes no user identifier and applies no ``:User`` + filter, so on a user-scoped construct it can return another tenant's + preferences. ``get_preferences_for`` is the only user-scoped primitive on + ``LongTermProtocol``; it is a listing rather than a search, so a scoped + lookup trades query relevance for tenancy correctness and returns the + user's active preferences up to ``limit``. + """ + if user_id is None: + return long_term.search_preferences + + async def _scoped(query: str, *, limit: int, threshold: float) -> list[Any]: + preferences = await long_term.get_preferences_for(user_identifier=user_id, active_only=True) + return preferences[:limit] + + return _scoped + + async def _retrieve_context( - long_term: LongTermProtocol, query: str, cfg: Neo4jRetrievalConfig, *, nams: bool + long_term: LongTermProtocol, + query: str, + cfg: Neo4jRetrievalConfig, + *, + nams: bool, + user_id: str | None = None, ) -> str: """Run the configured long-term searches concurrently and format the block. @@ -73,7 +100,11 @@ async def _retrieve_context( # NAMS has no preference/fact search endpoints — skip rather than warn every turn. wanted: list[tuple[bool, Callable[..., Awaitable[list[Any]]], Callable[..., str]]] = [ (cfg.include_entities, long_term.search_entities, _format_entity), - (cfg.include_preferences and not nams, long_term.search_preferences, _format_preference), + ( + cfg.include_preferences and not nams, + _preference_search(long_term, user_id), + _format_preference, + ), (cfg.include_facts and not nams, long_term.search_facts, _format_fact), ] searches = [s(query, limit=cfg.top_k, threshold=cfg.min_score) for on, s, _ in wanted if on] @@ -89,3 +120,138 @@ async def _retrieve_context( return "" body = "\n".join(f"- {line}" for line in lines) return f"<{cfg.context_tag}>\nRelevant memory:\n{body}\n" + + +@dataclass +class _EntryRow: + """One long-term hit, formatted for a Strands ``MemoryEntry``.""" + + content: str + metadata: dict[str, Any] + + +def _row( + *, + kind: str, + entry_id: Any, + entry_type: str, + source_metadata: dict[str, Any] | None, + content: str, +) -> _EntryRow: + """Build one row. Keyword-only: three of five params are plain strings, + so a transposition would otherwise type-check silently.""" + metadata: dict[str, Any] = {"kind": kind, "id": str(entry_id), "type": entry_type} + score = (source_metadata or {}).get("similarity") + if score is not None: + # Bolt sets "similarity" on entities, preferences, and facts alike; NAMS + # sets it on none. Omit rather than default to 0, which would misrepresent + # an unscored hit as a bad match. + metadata["score"] = score + return _EntryRow(content=content, metadata=metadata) + + +def _entity_row(entity: Entity) -> _EntryRow: + return _row( + kind="entity", + entry_id=entity.id, + entry_type=entity.full_type or entity.type, + source_metadata=entity.metadata, + content=_format_entity(entity), + ) + + +def _preference_row(preference: Preference) -> _EntryRow: + return _row( + kind="preference", + entry_id=preference.id, + entry_type=preference.category, + source_metadata=preference.metadata, + content=_format_preference(preference), + ) + + +def _fact_row(fact: Fact) -> _EntryRow: + return _row( + kind="fact", + entry_id=fact.id, + entry_type=fact.predicate, + source_metadata=fact.metadata, + content=_format_fact(fact), + ) + + +async def _retrieve_entries( + long_term: LongTermProtocol, + query: str, + *, + limit: int, + min_score: float, + include_entities: bool, + include_preferences: bool, + include_facts: bool, + nams: bool, + user_id: str | None = None, +) -> list[_EntryRow]: + """Sibling of ``_retrieve_context``: same fan-out, rows instead of a string. + + ``limit`` caps the *total* rows returned, not each kind: Strands treats a + store's result count as a per-store cap (``MemoryManager.search``) and + injection then slices the concatenation to the same number, so returning + ``limit`` rows per kind would let a saturated entity search crowd + preferences and facts out of the model's context entirely. The budget is + handed out round-robin, so every enabled kind with a hit is represented + before any kind takes a second row, and unused capacity flows to whoever + has more hits. + + Per-kind failures are logged and skipped so one dead index doesn't lose + the others' hits. NAMS has no preference/fact search endpoints, so those + are skipped rather than raised on every call. + """ + wanted: list[ + tuple[str, bool, Callable[..., Awaitable[list[Any]]], Callable[..., _EntryRow]] + ] = [ + ("entity", include_entities, long_term.search_entities, _entity_row), + ( + "preference", + include_preferences and not nams, + _preference_search(long_term, user_id), + _preference_row, + ), + ("fact", include_facts and not nams, long_term.search_facts, _fact_row), + ] + active = [(kind, search, row) for kind, on, search, row in wanted if on] + results = await asyncio.gather( + *(search(query, limit=limit, threshold=min_score) for _, search, _ in active), + return_exceptions=True, + ) + per_kind: list[list[_EntryRow]] = [] + for (kind, _, to_row), result in zip(active, results): + if isinstance(result, BaseException): + logger.warning("Long-term %s search failed: %s", kind, result) + per_kind.append([]) + continue + per_kind.append([to_row(item) for item in result]) + + rows: list[_EntryRow] = [] + for share, hits in zip(_share_budget([len(hits) for hits in per_kind], limit), per_kind): + rows.extend(hits[:share]) + return rows + + +def _share_budget(counts: list[int], limit: int) -> list[int]: + """Split ``limit`` rows across kinds, round-robin, so none is starved. + + One row to each kind that still has hits, then a second to each, and so + on until the budget or the hits run out — which also means a kind with + more hits absorbs whatever the others leave unused. + """ + take = [0] * len(counts) + remaining = min(limit, sum(counts)) + while remaining > 0: + for index, available in enumerate(counts): + if remaining == 0: + break + if take[index] < available: + take[index] += 1 + remaining -= 1 + return take diff --git a/src/neo4j_agent_memory/integrations/strands/_store_tools.py b/src/neo4j_agent_memory/integrations/strands/_store_tools.py new file mode 100644 index 00000000..9392f2ba --- /dev/null +++ b/src/neo4j_agent_memory/integrations/strands/_store_tools.py @@ -0,0 +1,214 @@ +"""Graph-native @tool functions bound to one memory store's client. + +The tools a ``MemoryManager`` cannot provide: multi-hop traversal and +scoped preference lookup. Deliberately excludes search/add, which the +manager owns as ``search_memory`` / ``add_memory`` — and ``add_memory`` is +already the name ``context_graph_tools`` uses, so re-exposing it here would +collide. + +Unlike ``tools.py``, these bind to the store's own client instead of the +factory's per-call cached clients, so nothing can close a transport the store +is still using. +""" + +from __future__ import annotations + +import hashlib +from typing import TYPE_CHECKING, Any, cast + +if TYPE_CHECKING: + from strands.types.tools import AgentTool + + from neo4j_agent_memory import MemoryClient + from neo4j_agent_memory.integrations.strands.memory_store import Neo4jMemoryStore + from neo4j_agent_memory.memory.long_term import LongTermMemory + +_MAX_EDGES = 50 + + +async def _entity_graph( + client: MemoryClient, entity_name: str, *, depth: int, nams: bool +) -> dict[str, Any]: + """Return the neighbourhood of a named entity. + + bolt traverses ``get_related_entities`` to ``depth``; NAMS exposes only a + 1-hop ``expand_graph`` keyed by node id, so the name is resolved through + entity search first and the reported depth is 1. + """ + matches = await client.long_term.search_entities(entity_name, limit=1) + if not matches: + return {"error": f"entity not found: {entity_name}"} + centre = matches[0] + + if nams: + # expand_graph is NAMS-only, excluded from LongTermProtocol. Narrow with + # a real check rather than a cast: inside this branch httpx is installed + # by definition, so importing the NAMS module here is safe -- the same + # nested-import shape `for_nams` uses for `build_nams_settings`. + from neo4j_agent_memory.nams.long_term import NamsLongTermMemory + + long_term = client.long_term + if not isinstance(long_term, NamsLongTermMemory): + raise TypeError( + f"Neo4jMemoryStore: the client reports the NAMS backend, but its " + f"long_term layer is {type(long_term).__name__}, not " + f"NamsLongTermMemory -- expand_graph exists only on NAMS." + ) + expansion = await long_term.expand_graph(str(centre.id)) + return { + "center": centre.display_name, + "depth": 1, + "nodes": list(expansion.get("nodes") or [])[:_MAX_EDGES], + "edges": list(expansion.get("edges") or [])[:_MAX_EDGES], + } + + # get_related_entities' depth kwarg diverges from LongTermProtocol's + # portable, no-depth signature (core/protocols.py documents this as + # deliberate) -- cast to the concrete class so the call stays checked. + bolt_long_term = cast("LongTermMemory", client.long_term) + related = await bolt_long_term.get_related_entities(centre, depth=depth) + nodes = [{"name": centre.display_name, "type": centre.type, "is_center": True}] + edges: list[dict[str, str]] = [] + for other, relationship in related[:_MAX_EDGES]: + nodes.append({"name": other.display_name, "type": other.type, "is_center": False}) + # Report the orientation the library reports rather than inventing one: + # GET_ENTITY_RELATIONSHIPS matches undirected and get_related_entities + # sets source_id to the centre for every hit, so these ids are the only + # direction available. Likewise `type` is what the library resolved -- + # today always "RELATED_TO", because execute_read's result.data() + # flattens a relationship to (start, type, end) and drops its + # properties, so the property-level type never survives the round trip. + # A library-side fix would flow through here unchanged. + names = { + str(centre.id): centre.display_name, + str(other.id): other.display_name, + } + edges.append( + { + "from": names.get(str(relationship.source_id), centre.display_name), + "relationship": relationship.type, + "to": names.get(str(relationship.target_id), other.display_name), + } + ) + return {"center": centre.display_name, "depth": depth, "nodes": nodes, "edges": edges} + + +async def _user_preferences( + client: MemoryClient, user_id: str, category: str | None, *, limit: int +) -> list[dict[str, Any]]: + """Return the configured user's preferences, optionally narrowed to one category. + + ``get_preferences_for`` is user-scoped and needs no embedder, unlike + ``search_preferences`` (which returns ``[]`` with no embedder and no + ``:User`` filter at all -- both a silent-empty and a cross-tenant-leak + risk). It *is* on ``LongTermProtocol`` (``core/protocols.py``), keyword-only + ``user_identifier`` and all, so no cast is needed here. + """ + preferences = await client.long_term.get_preferences_for( + user_identifier=user_id, active_only=True + ) + if category: + preferences = [p for p in preferences if p.category.lower() == category.lower()] + return [ + {"category": p.category, "preference": p.preference, "context": p.context} + for p in preferences[:limit] + ] + + +def _tool_prefix(name: str) -> str: + """The store's name, reduced to something legal in a tool name, 1:1. + + Tool names are namespaced per store so they can coexist both with + ``context_graph_tools``' identically-named tools and with a second + store's (``dataclasses.replace(config, name="team")`` is the documented + way to run personal / team / org stores side by side). + + Sanitization alone is many-to-one — ``"team/graph"``, ``"team graph"`` and + ``"Team_Graph"`` all reduce to ``team_graph`` — and colliding prefixes + would silently overwrite each other in ``ToolRegistry``, the exact failure + namespacing exists to prevent. So a name that does not survive + sanitization unchanged carries a short digest of the original: names that + are already legal keep clean prefixes, and any two distinct names stay + distinct. + """ + slug = "".join(char if char.isalnum() else "_" for char in name.lower()).strip("_") + while "__" in slug: + slug = slug.replace("__", "_") + base = slug or "store" + if base == name: + return base + digest = hashlib.blake2s(name.encode(), digest_size=3).hexdigest() + return f"{base}_{digest}" + + +def build_store_tools(store: Neo4jMemoryStore) -> list[AgentTool]: + """Build the store's graph tools, gated by what the backend exposes. + + Names are prefixed with the store's own name. ``ToolRegistry`` silently + *overwrites* a duplicate name for ``@tool`` functions (its duplicate + check is skipped whenever ``supports_hot_reload`` is true, which it + always is for decorated functions), so unprefixed ``get_entity_graph`` + / ``get_user_preferences`` would replace the factory's tools of those + names -- which take different arguments -- with no warning. + """ + from strands import tool + + nams = store.is_nams + prefix = _tool_prefix(store.name) + + async def _get_entity_graph(entity_name: str, depth: int = 2) -> dict[str, Any]: + """Explore the graph neighbourhood of an entity. + + Use this to find how an entity connects to others — who works where, + what happened at which location. + + Args: + entity_name: The entity to start from. + depth: How many hops to traverse. Hosted backends traverse one hop + regardless. + """ + # Through the store, never straight to the captured client: with + # injection disabled a tool call can be the store's first operation on + # a new event loop, and only initialize() rebinds an owned client. + await store.initialize() + return await _entity_graph( + store._client, entity_name, depth=max(1, min(depth, 3)), nams=nams + ) + + # Annotated explicitly as AgentTool: @tool's overloads resolve this correctly + # under mypy --strict and ty, but some IDEs' inference falls back to the + # undecorated function's callable type instead of the decorator's declared + # return type. The explicit annotation on the assignment target sidesteps + # that inference gap without a cast/ignore/Any. + get_entity_graph: AgentTool = tool(name=f"{prefix}_get_entity_graph")(_get_entity_graph) + + tools: list[AgentTool] = [get_entity_graph] + + # get_preferences_for requires a user identifier and is bolt-only (NAMS + # has no preferences endpoint); an unscoped variant is exactly what + # risked leaking another tenant's preferences, so ship the tool only + # when both conditions hold. Unscoped recall still reaches the model + # through the manager's own search_memory, which includes preferences + # on bolt regardless. + if not nams and store.user_id: + user_id = store.user_id + + async def _get_user_preferences(category: str | None = None, limit: int = 20) -> Any: + """Retrieve the configured user's preferences, optionally filtered by category. + + Returns only preferences belonging to this store's configured + user -- not a global listing across all users. + + Args: + category: Optional category such as "food" or "ui". + limit: Maximum preferences to return. + """ + await store.initialize() + return await _user_preferences(store._client, user_id, category, limit=limit) + + get_user_preferences: AgentTool = tool(name=f"{prefix}_get_user_preferences")( + _get_user_preferences + ) + tools.append(get_user_preferences) + + return tools diff --git a/src/neo4j_agent_memory/integrations/strands/config.py b/src/neo4j_agent_memory/integrations/strands/config.py index 45f31dcd..b2f3fec5 100644 --- a/src/neo4j_agent_memory/integrations/strands/config.py +++ b/src/neo4j_agent_memory/integrations/strands/config.py @@ -184,7 +184,14 @@ def build_nams_settings( validate_on_connect: bool = False, ) -> MemorySettings: """Build NAMS-backed MemorySettings (validate_on_connect off by default — - Strands drives short synchronous bursts; skipping the probe saves a round-trip).""" + Strands drives short synchronous bursts; skipping the probe saves a round-trip). + + Args: + endpoint: NAMS base URL. + api_key: NAMS API key. + transport_mode: MCP / REST transport selection. + validate_on_connect: Whether to probe the service on connect. + """ from pydantic import SecretStr from neo4j_agent_memory import MemorySettings, NamsConfig diff --git a/src/neo4j_agent_memory/integrations/strands/memory_store.py b/src/neo4j_agent_memory/integrations/strands/memory_store.py new file mode 100644 index 00000000..61acd135 --- /dev/null +++ b/src/neo4j_agent_memory/integrations/strands/memory_store.py @@ -0,0 +1,523 @@ +"""Strands ``MemoryStore`` backed by neo4j-agent-memory (bolt or NAMS). + +Long-term recall for the agent loop. Distinct from +:class:`Neo4jSessionManager`, which persists and restores the transcript — +see the design spec's positioning section. +""" + +from __future__ import annotations + +import asyncio +import logging +import uuid +from dataclasses import dataclass, replace +from typing import TYPE_CHECKING, Any + +try: + from strands.memory import ( + AddMessagesContext, + MemoryEntry, + MemoryStore, + SearchOptions, + ) +except ImportError as import_error: # pragma: no cover - exercised via package __init__ + raise ImportError( + "strands-agents>=1.44.0 is required for the Strands memory store. " + "Install with: pip install 'neo4j-agent-memory[strands]'" + ) from import_error + +from neo4j_agent_memory.core.exceptions import NotSupportedError +from neo4j_agent_memory.integrations.strands._messages import _message_text +from neo4j_agent_memory.integrations.strands._retrieval import _retrieve_entries + +if TYPE_CHECKING: + from types import TracebackType + + from strands.memory import ExtractionConfig + from strands.types.content import Message as StrandsMessage + from strands.types.tools import AgentTool + + from neo4j_agent_memory import MemoryClient, MemorySettings + from neo4j_agent_memory.nams.endpoints import TransportMode + +logger = logging.getLogger(__name__) + +#: Conversation-metadata key marking a conversation as a memory-store sink. +_STORE_KEY = "strands_memory_store" + +#: Strands' own per-store default when neither caller nor store sets a limit +#: (mirrors ``strands.memory.memory_manager.DEFAULT_MAX_SEARCH_RESULTS``). +_DEFAULT_MAX_SEARCH_RESULTS = 3 + +#: NAMS caps bulk message writes; chunk to stay inside it on both backends. +_BULK_CHUNK = 100 + +__all__ = ["Neo4jMemoryStore", "Neo4jMemoryStoreConfig"] + + +def _running_loop() -> asyncio.AbstractEventLoop | None: + """The running loop, or ``None`` outside one.""" + try: + return asyncio.get_running_loop() + except RuntimeError: + return None + + +@dataclass +class Neo4jMemoryStoreConfig: + """Configuration for :class:`Neo4jMemoryStore`. + + ``name``, ``description``, ``max_search_results``, ``writable`` and + ``extraction`` are the fields ``MemoryStore``'s protocol requires the + store to expose as instance attributes; the rest are Neo4j connection, + scoping, and search knobs. + + Reuse one config across several stores — personal / team / org — with + ``dataclasses.replace(config, name="team")``. + """ + + name: str + client: MemoryClient | None = None + settings: MemorySettings | None = None + description: str | None = None + max_search_results: int | None = None + writable: bool = True + extraction: ExtractionConfig | bool = False + conversation_id: str | None = None + user_id: str | None = None + include_entities: bool = True + include_preferences: bool = True + include_facts: bool = True + min_score: float = 0.2 + graph_tools: bool = True + + def __post_init__(self) -> None: + if not self.name: + raise ValueError("Neo4jMemoryStore: 'name' is required and must be non-empty") + # Only the unambiguous mistake (both given) is checkable here. A + # config en route to `for_nams` legitimately has neither set yet — + # `for_nams` completes it with `settings` before construction — so + # "neither given" is checked in `Neo4jMemoryStore.__init__` instead, + # once we know no such completion step is coming. + if self.client is not None and self.settings is not None: + raise ValueError( + "Neo4jMemoryStore: pass exactly one of 'client' (borrowed, left open) " + "or 'settings' (a client is constructed and owned by the store)" + ) + + +class Neo4jMemoryStore(MemoryStore): + """Long-term memory recall and ingestion over a Neo4j context graph. + + Example: + store = Neo4jMemoryStore( + Neo4jMemoryStoreConfig( + name="graph", + client=client, # or settings=MemorySettings(...) + user_id="alice", + ) + ) + """ + + def __init__(self, config: Neo4jMemoryStoreConfig) -> None: + if config.client is None and config.settings is None: + raise ValueError( + "Neo4jMemoryStore: pass exactly one of 'client' (borrowed, left open) " + "or 'settings' (a client is constructed and owned by the store)" + ) + + # The five attributes MemoryStore's protocol requires the store to expose. + self.name = config.name + self.description = config.description or ( + f"Neo4j context graph '{config.name}': entities, preferences and facts." + ) + self.max_search_results = config.max_search_results + self.writable = config.writable + self.extraction = config.extraction + + self.user_id = config.user_id + self.graph_tools = config.graph_tools + self._include_entities = config.include_entities + self._include_preferences = config.include_preferences + self._include_facts = config.include_facts + self._min_score = config.min_score + + self._sink_key: str | None = config.conversation_id + self._owns_client = config.client is None + self._run_id = uuid.uuid4().hex + self._written: set[tuple[str, int]] = set() + self._initialized = False + # The loop we last connected on -- only ever compared against the + # current running loop, never awaited on (see `initialize()`). + self._connected_loop: asyncio.AbstractEventLoop | None = None + self._warned_unsupported_kinds: set[str] = set() + + if config.client is not None: + self._client: MemoryClient = config.client + else: + from neo4j_agent_memory import MemoryClient as _MemoryClient + + assert config.settings is not None + self._client = _MemoryClient(config.settings) + + @classmethod + def for_nams( + cls, + config: Neo4jMemoryStoreConfig, + *, + endpoint: str | None = None, + api_key: str | None = None, + transport_mode: TransportMode = "auto", + ) -> Neo4jMemoryStore: + """Construct a store against hosted NAMS. + + Reads ``MEMORY_API_KEY`` (and optionally ``MEMORY_ENDPOINT``) from the + environment when not passed explicitly. ``config`` is not mutated: + ``dataclasses.replace`` returns a copy with ``settings`` injected. + """ + from neo4j_agent_memory.integrations.strands.config import ( + build_nams_settings, + resolve_nams_connection, + ) + + endpoint, api_key = resolve_nams_connection(endpoint, api_key) + merged = replace(config, settings=build_nams_settings(endpoint, api_key, transport_mode)) + return cls(merged) + + @property + def is_nams(self) -> bool: + return self._client.is_nams + + @property + def _sink_name(self) -> str: + """Deterministic sink name, stable across processes and restarts.""" + return f"strands-memory-store/{self.user_id or '_'}/{self.name}" + + async def _resolve_sink(self) -> str: + """Return the conversation key writes go to, creating the sink if needed. + + An explicit ``conversation_id`` is used verbatim. Otherwise: bolt keys + conversations by ``session_id`` and ``add_message``/``add_messages_batch`` + both auto-create the sink (and tenant-link it via ``user_identifier``) + via ``_ensure_conversation`` on first write, so the deterministic sink + name *is* the whole contract — no backend call is made here, and none + is needed. NAMS mints its own conversation ids, so metadata is the + only portable handle there — list and match ``_STORE_KEY`` metadata, + else create. Same split as ``Neo4jSessionManager._aresolve_conversation``. + """ + if self._sink_key is not None: + return self._sink_key + + key = self._sink_name if not self.is_nams else await self._resolve_nams_sink() + self._sink_key = key + return key + + async def _resolve_nams_sink(self) -> str: + """List NAMS conversations, matching this store's ``_STORE_KEY`` metadata, else create one.""" + short_term = self._client.short_term + conversations = await short_term.list_conversations( + user_identifier=self.user_id, limit=1000 + ) + for conversation in conversations: + if (conversation.metadata or {}).get(_STORE_KEY) == self._sink_name: + return str(conversation.id) + + created = await short_term.create_conversation( + session_id=self._sink_name, + metadata={_STORE_KEY: self._sink_name, "session_type": "MEMORY_STORE"}, + user_identifier=self.user_id, + ) + return str(created.id) + + async def initialize(self) -> None: + """Connect the client, rebinding it if the event loop changed. + + Idempotent per loop. The neo4j async driver and the NAMS HTTP + transport both bind to the loop that opened them, while Strands' + synchronous entry points run every call on a fresh, throwaway loop + (``strands._async.run_async`` is ``asyncio.run`` in a worker + thread): ``Agent.__init__`` initializes the store on one loop and + each ``Agent.__call__`` drives it from another. A client bound to + the first loop then raises ``RuntimeError: ... attached to a + different loop`` from deep inside the driver. + + So: record the loop we connected on, and when it changes, + + * **owned client** (built from ``settings=``) — close and reconnect + on the current loop. One reconnect per synchronous ``Agent(...)`` + invocation is the price of that entry point. + * **borrowed client** (passed as ``client=``) — raise. Closing or + reconnecting someone else's client is not ours to do, and a clear + error beats the driver's opaque one. + """ + loop = asyncio.get_running_loop() + + # Keyed on the recorded loop, not on ``_initialized``: ``aclose()`` + # leaves a borrowed client connected on its original loop, so the guard + # has to survive an aclose/re-enter cycle. Were this ``if + # self._initialized``, that cycle would fall through to the + # already-connected branch, record the new loop and hand the caller the + # driver's opaque error instead of the named one below. + if self._connected_loop is not None and self._connected_loop is not loop: + if not self._owns_client: + raise RuntimeError( + f"Neo4jMemoryStore '{self.name}': the MemoryClient passed as " + "client= was connected on a different event loop and its " + "transport cannot be driven from this one. Strands' synchronous " + "Agent(...) entry point runs each call on a fresh loop, so a " + "borrowed client only works when your own loop drives the agent. " + "Either hand the store a MemoryClient connected on the loop that " + "drives the agent, or construct the store from settings= so it " + "owns its client and can rebind it." + ) + try: + await self._client.close() + except Exception as error: # noqa: BLE001 - the old loop is usually gone + logger.debug( + "Neo4jMemoryStore '%s': closing the client bound to the previous " + "event loop failed (expected once that loop is gone): %s", + self.name, + error, + ) + await self._client.connect() + elif self._initialized: + return + elif not self._client.is_connected: + await self._client.connect() + + self._connected_loop = loop + self._initialized = True + + async def search(self, query: str, options: SearchOptions | None = None) -> list[MemoryEntry]: + """Search long-term memory. No sink resolution: reads don't need one. + + Limit precedence: per-call option, then ``self.max_search_results``, + then Strands' own default. Per-kind failures are isolated in + ``_retrieve_entries``; a total failure here propagates so + ``MemoryManager.search`` can log a dead store rather than see an + empty, misleadingly-successful result. + + With a ``user_id`` configured, preference recall is user-scoped (see + ``_preference_search``) — ``search_preferences`` has no user filter, so + an unscoped call here would inject another tenant's preferences into + this user's turn. + """ + await self.initialize() + limit = (options or {}).get("max_search_results") + if limit is None: + limit = self.max_search_results + if limit is None: + limit = _DEFAULT_MAX_SEARCH_RESULTS + + rows = await _retrieve_entries( + self._client.long_term, + query, + limit=limit, + min_score=self._min_score, + include_entities=self._include_entities, + include_preferences=self._include_preferences, + include_facts=self._include_facts, + nams=self.is_nams, + user_id=self.user_id, + ) + return [MemoryEntry(content=row.content, metadata=row.metadata) for row in rows] + + async def add(self, content: str, metadata: dict[str, Any] | None = None) -> dict[str, Any]: + """Add one piece of content. + + Default sink: a message in the store's conversation, written with + extraction on — the one path available on every backend. ``metadata["kind"]`` + opts into a typed write (``preference`` / ``fact`` / ``entity``); on a + backend that does not expose it, the write falls back to the default sink + so the memory is never silently dropped. + + Extraction writes are at-least-once, so this tolerates duplicates. + """ + if not self.writable: + raise ValueError( + f"Neo4jMemoryStore '{self.name}': store is not writable. " + "Set writable=True to enable add()." + ) + if not content.strip(): + raise ValueError(f"Neo4jMemoryStore '{self.name}': content must not be empty") + + await self.initialize() + meta = metadata or {} + kind = meta.get("kind") + + if kind in ("preference", "fact", "entity"): + try: + return await self._add_typed(kind, content, meta) + except NotSupportedError as error: + if kind not in self._warned_unsupported_kinds: + self._warned_unsupported_kinds.add(kind) + logger.warning( + "Neo4jMemoryStore '%s': %s unsupported on this backend (%s); " + "falling back to the message sink. (This warning is logged " + "once per store; further %s writes fall back silently.)", + self.name, + kind, + error, + kind, + ) + + return await self._add_to_sink(content) + + async def _add_typed(self, kind: str, content: str, meta: dict[str, Any]) -> dict[str, Any]: + long_term = self._client.long_term + if kind == "preference": + # user_identifier as everywhere else in this class: without it + # multi_tenant=True raises ValueError (not NotSupportedError, so + # no sink fallback), and the preference gets no + # (:User)-[:HAS_PREFERENCE] edge -- which is exactly what the + # store's own get_user_preferences tool reads. + preference = await long_term.add_preference( + meta.get("category", "memory"), + content, + user_identifier=self.user_id, + ) + return {"kind": "preference", "id": str(preference.id)} + if kind == "fact": + subject = meta.get("subject") + predicate = meta.get("predicate") + obj = meta.get("object") + if not (subject and predicate and obj): + raise ValueError( + f"Neo4jMemoryStore '{self.name}': kind='fact' requires " + "subject, predicate and object in metadata" + ) + fact = await long_term.add_fact(subject, predicate, obj) + return {"kind": "fact", "id": str(fact.id)} + if kind == "entity": + # add_entity returns (Entity, DeduplicationResult) on bolt but a bare + # Entity on NAMS (no dedup pipeline there). + entity_result = await long_term.add_entity( + meta.get("name", content), meta.get("type", "OBJECT") + ) + entity = entity_result[0] if isinstance(entity_result, tuple) else entity_result + return {"kind": "entity", "id": str(entity.id)} + raise ValueError(f"Neo4jMemoryStore '{self.name}': unknown kind '{kind}'") + + async def _add_to_sink(self, content: str) -> dict[str, Any]: + sink = await self._resolve_sink() + message = await self._client.short_term.add_message( + sink, + "user", + content, + extract_entities=True, + user_identifier=self.user_id, + ) + return {"kind": "message", "id": str(message.id)} + + async def add_messages( + self, + messages: list[StrandsMessage], + context: AddMessagesContext | None = None, + ) -> dict[str, Any]: + """Ingest a batch of conversation turns into the sink conversation. + + The backend extracts them — server-side on NAMS, inline on bolt — so no + model call happens here. Extraction writes are at-least-once and + ``AddMessagesContext.sequence_numbers`` repeat on a retry, so a + ``(run_id, sequence_number)`` set skips turns already written by this + instance. The dedupe is in-process only: sequence numbers reset each run, + so there is nothing durable to key on. + """ + if not self.writable: + raise ValueError( + f"Neo4jMemoryStore '{self.name}': store is not writable. " + "Set writable=True to enable add_messages()." + ) + await self.initialize() + + sequence_numbers = (context.sequence_numbers if context else None) or [] + payload: list[dict[str, Any]] = [] + tokens: list[tuple[str, int] | None] = [] + skipped = 0 + + for index, message in enumerate(messages): + text = _message_text(message) + if not text.strip(): + skipped += 1 + continue + token: tuple[str, int] | None = None + if index < len(sequence_numbers): + token = (self._run_id, sequence_numbers[index]) + if token in self._written: + skipped += 1 + continue + payload.append({"role": message.get("role", "user"), "content": text}) + tokens.append(token) + + if not payload: + return {"written": 0, "skipped": skipped} + + sink = await self._resolve_sink() + for start in range(0, len(payload), _BULK_CHUNK): + await self._client.short_term.bulk_add_messages( + sink, + payload[start : start + _BULK_CHUNK], + extract_entities=True, + user_identifier=self.user_id, + ) + # Per chunk, not after the loop: Strands rolls its high-water mark + # back and retries the whole batch when this raises, so tokens + # banked only at the end would let an already-written chunk be + # written again by the retry. + self._written.update( + token for token in tokens[start : start + _BULK_CHUNK] if token is not None + ) + return {"written": len(payload), "skipped": skipped} + + def get_tools(self) -> list[AgentTool]: + """Graph-native tools registered alongside the manager's own tools. + + Empty when ``graph_tools=False``. Never includes ``search_memory`` or + ``add_memory`` — those belong to the ``MemoryManager``. + """ + if not self.graph_tools: + return [] + from neo4j_agent_memory.integrations.strands._store_tools import build_store_tools + + return build_store_tools(self) + + async def aclose(self) -> None: + """Close the client only when the store constructed it. + + Resets the initialization state either way, so re-entering the + async context manager reconnects instead of short-circuiting on a + stale ``_initialized``. + """ + if self._owns_client: + stale = self._connected_loop is not None and self._connected_loop is not _running_loop() + try: + await self._client.close() + except Exception: + # A client bound to a loop that has since closed cannot be + # shut down cleanly from here — its sockets went with the + # loop. Anywhere else, the caller deserves the error. + if not stale: + raise + logger.debug( + "Neo4jMemoryStore '%s': client bound to a closed event loop; " + "skipping its shutdown.", + self.name, + ) + self._initialized = False + if self._owns_client: + self._connected_loop = None + # Borrowed clients keep their recorded loop: this store did not close + # the client, so it is still bound to that loop and re-entering the + # store from another one must still raise the named error. + + async def __aenter__(self) -> Neo4jMemoryStore: + await self.initialize() + return self + + async def __aexit__( + self, + exc_type: type[BaseException] | None, + exc: BaseException | None, + tb: TracebackType | None, + ) -> None: + await self.aclose() diff --git a/src/neo4j_agent_memory/integrations/strands/session_manager.py b/src/neo4j_agent_memory/integrations/strands/session_manager.py index e6a0c9cb..98de386e 100644 --- a/src/neo4j_agent_memory/integrations/strands/session_manager.py +++ b/src/neo4j_agent_memory/integrations/strands/session_manager.py @@ -57,6 +57,12 @@ #: Conversation-metadata key linking a Conversation to a Strands session id. _SESSION_KEY = "strands_session_id" +#: Shared by the double-extraction ValueError and the double-injection warning. +_COEXISTENCE_HINT = ( + "Set extraction=False on Neo4jMemoryStore (recall only, recommended), or " + "extract_entities=False on Neo4jSessionManager (let the store own extraction)." +) + __all__ = ["Neo4jRetrievalConfig", "Neo4jSessionManager"] @@ -112,6 +118,7 @@ def __init__( self._last_persisted: StoredMessage | None = None # last stored (late redaction) self._trace_id: UUID | None = None # lazy reasoning trace (record_tool_calls) self._closed = False + self._warned_double_injection = False @property def _is_nams(self) -> bool: @@ -199,8 +206,59 @@ def _ensure_session(self) -> str: # ----------------------------------------------------- SessionManager API + def _our_stores(self, agent: Agent) -> list[Any]: + """Neo4jMemoryStore instances registered on the agent's MemoryManager. + + Reads ``MemoryManager._stores`` — a private, pinned by + ``test_strands_coexistence.py::TestPrivateAttributeCoupling``. + """ + manager = getattr(agent, "memory_manager", None) + if manager is None: + return [] + from neo4j_agent_memory.integrations.strands.memory_store import Neo4jMemoryStore + + stores = getattr(manager, "_stores", None) or [] + return [store for store in stores if isinstance(store, Neo4jMemoryStore)] + + def _check_memory_manager_coexistence(self, agent: Agent) -> None: + """Guard the two overlaps possible with a paired Neo4jMemoryStore. + + Pairing itself is supported: this manager persists/restores the + transcript, the store feeds the agent loop. Not supported: both + sides extracting the same turns, both sides injecting into the + same user message. + """ + stores = self._our_stores(agent) + if not stores: + return + + extracting = [store for store in stores if store.extraction] + if extracting: + # NAMS extracts every write server-side (add_message drops + # extraction kwargs there), so our own extract_entities flag + # cannot avert the duplication on that backend. + if self._extract_entities or self._is_nams: + raise ValueError( + f"Neo4jSessionManager and Neo4jMemoryStore " + f"'{extracting[0].name}' would write and extract the same " + f"turns twice. {_COEXISTENCE_HINT}" + ) + + if self._retrieval_config is not None and not self._warned_double_injection: + manager = getattr(agent, "memory_manager", None) + if getattr(manager, "_injection_config", None) is not False: + self._warned_double_injection = True + logger.warning( + "Memory is being injected twice: Neo4jRetrievalConfig on this " + "session manager and MemoryManager injection from " + "Neo4jMemoryStore '%s'. Drop retrieval_config, or pass " + "injection=False to MemoryManager.", + stores[0].name, + ) + def initialize(self, agent: Agent, **kwargs: Any) -> None: """Restore the agent's conversation history from the graph.""" + self._check_memory_manager_coexistence(agent) try: restored = self._bridge.run(self._ainitialize()) except Exception as e: @@ -257,7 +315,13 @@ def _inject_context(self, message: StrandsMessage) -> None: return # no text block, or already injected (event re-fired) try: block = self._bridge.run( - _retrieve_context(self._client.long_term, query, cfg, nams=self._is_nams) + _retrieve_context( + self._client.long_term, + query, + cfg, + nams=self._is_nams, + user_id=self._user_id, + ) ) except Exception as e: logger.warning( diff --git a/src/neo4j_agent_memory/memory/short_term.py b/src/neo4j_agent_memory/memory/short_term.py index cbeb3516..5f5c4a93 100644 --- a/src/neo4j_agent_memory/memory/short_term.py +++ b/src/neo4j_agent_memory/memory/short_term.py @@ -373,6 +373,7 @@ async def add_messages_batch( extract_relations: bool = True, on_progress: Callable[[int, int], None] | None = None, on_batch_complete: Callable[[int, list[Message]], None] | None = None, + user_identifier: str | None = None, ) -> list[Message]: """ Bulk load messages with transaction batching for better performance. @@ -398,6 +399,9 @@ async def add_messages_batch( (only applies when extract_entities=True) on_progress: Callback for progress updates (completed_count, total_count) on_batch_complete: Callback after each batch completes (batch_num, batch_messages) + user_identifier: When provided, scopes the conversation to a + :User node via ``(:User)-[:HAS_CONVERSATION]->(:Conversation)``. + Required when ``MemorySettings.memory.multi_tenant=True``. Returns: List of created Message objects @@ -405,8 +409,11 @@ async def add_messages_batch( if not messages: return [] - # Ensure conversation exists - conv_id = await self._ensure_conversation(session_id, None) + # Multi-tenant guardrail (same as add_message). + self._enforce_multi_tenant(user_identifier) + + # Ensure conversation exists, tenant-linked when user_identifier is given. + conv_id = await self._ensure_conversation(session_id, None, user_identifier=user_identifier) total = len(messages) all_created: list[Message] = [] @@ -567,7 +574,7 @@ async def bulk_add_messages( Thin ``ShortTermProtocol`` alias over :meth:`add_messages_batch`; extra keyword arguments (``batch_size``, ``generate_embeddings``, - ``extract_entities``, …) are forwarded. + ``extract_entities``, ``user_identifier``, …) are forwarded. """ return await self.add_messages_batch(session_id, messages, **kwargs) @@ -696,7 +703,7 @@ async def add_message( describing the entities to MERGE on and link. user_identifier: When provided, scopes the conversation to a :User node via ``(:User)-[:HAS_CONVERSATION]->(:Conversation)``. - Required when ``MemorySettings.multi_tenant=True``. + Required when ``MemorySettings.memory.multi_tenant=True``. Returns: The created message diff --git a/src/neo4j_agent_memory/nams/short_term.py b/src/neo4j_agent_memory/nams/short_term.py index 9e65c5cc..7b97205e 100644 --- a/src/neo4j_agent_memory/nams/short_term.py +++ b/src/neo4j_agent_memory/nams/short_term.py @@ -185,11 +185,12 @@ def _normalize_conversation( ) -> dict[str, Any]: """Map NAMS Conversation response → bolt Pydantic shape. - NAMS returns ``{id, userId, workspaceId, createdAt, updatedAt}`` from - GET, and only ``{id, userId, workspaceId}`` from create. The bolt - Pydantic ``Conversation`` model requires ``id``, ``session_id``, and - ``created_at``. We synthesize ``session_id`` from the caller-supplied - value (which is typically the NAMS conversation UUID). + NAMS returns ``{id, userId, workspaceId, metadata, createdAt, updatedAt}`` + from GET and ``{id, userId, workspaceId, metadata}`` from create. The bolt + ``Conversation`` model requires ``id``, ``session_id`` and ``created_at``, + so ``session_id`` is synthesized from the caller-supplied value (typically + the NAMS conversation UUID) and ``metadata`` defaults to ``{}`` when a + deployment omits it. """ data = snakeize_keys(payload) if isinstance(payload, dict) else {} if "session_id" not in data: diff --git a/tests/docs/test_code_snippets.py b/tests/docs/test_code_snippets.py index e3598d2c..5e0ab29c 100644 --- a/tests/docs/test_code_snippets.py +++ b/tests/docs/test_code_snippets.py @@ -241,6 +241,8 @@ def test_neo4j_agent_memory_imports_exist(self, python_snippets: list[CodeSnippe "nams_context_graph_tools", "Neo4jSessionManager", "Neo4jRetrievalConfig", + "Neo4jMemoryStore", + "Neo4jMemoryStoreConfig", "HybridMemoryProvider", "StrandsConfig", "MemoryType", @@ -357,6 +359,13 @@ def test_strands_session_manager_doc_imports_resolve(self) -> None: nams_context_graph_tools, ) + def test_strands_memory_store_doc_imports_resolve(self) -> None: + pytest.importorskip("strands", reason="strands-agents not installed") + from neo4j_agent_memory.integrations.strands import ( # noqa: F401 + Neo4jMemoryStore, + Neo4jMemoryStoreConfig, + ) + @pytest.mark.docs class TestSnippetConsistency: @@ -438,6 +447,30 @@ class TestSettingsFieldDrift: "EnrichmentConfig": "neo4j_agent_memory.config.settings:EnrichmentConfig", } + @staticmethod + def _foreign_names(tree: ast.AST) -> set[str]: + """Names a snippet imported from somewhere other than this library. + + Class names are not unique across the ecosystem: ``ExtractionConfig`` + is both ``neo4j_agent_memory.config.settings.ExtractionConfig`` and + ``strands.memory.ExtractionConfig``, and the two share no fields. A + name-only lookup reports the wrong model's fields for the wrong class, + so a snippet that explicitly imports the third-party one is skipped. + Bare, un-imported usages are still checked — fragment snippets rely on + that. + """ + names: set[str] = set() + for node in ast.walk(tree): + if isinstance(node, ast.ImportFrom): + if (node.module or "").startswith("neo4j_agent_memory"): + continue + names.update(alias.asname or alias.name for alias in node.names) + elif isinstance(node, ast.Import): + for alias in node.names: + if not alias.name.startswith("neo4j_agent_memory"): + names.add((alias.asname or alias.name).split(".")[0]) + return names + @staticmethod def _resolve_model_fields(class_name: str) -> set[str] | None: spec = TestSettingsFieldDrift._MODELS_TO_CHECK.get(class_name) @@ -466,6 +499,8 @@ def test_no_unknown_kwargs_in_doc_constructions(self, python_snippets: list[Code # Syntax test owns this — skip here so we only report drift. continue + foreign = self._foreign_names(tree) + for node in ast.walk(tree): if not isinstance(node, ast.Call): continue @@ -478,6 +513,9 @@ def test_no_unknown_kwargs_in_doc_constructions(self, python_snippets: list[Code else: continue + if class_name in foreign: + continue + fields = self._resolve_model_fields(class_name) if fields is None: continue diff --git a/tests/examples/test_strands_memory_store_example.py b/tests/examples/test_strands_memory_store_example.py new file mode 100644 index 00000000..39a4503f --- /dev/null +++ b/tests/examples/test_strands_memory_store_example.py @@ -0,0 +1,88 @@ +"""Smoke tests for the strands-memory-store example.""" + +from __future__ import annotations + +import ast +import importlib.util +import sys +from pathlib import Path + +import pytest + +pytest.importorskip("strands", reason="strands-agents not installed") + +EXAMPLES_DIR = Path(__file__).parent.parent.parent / "examples" +STRANDS_MS_DIR = EXAMPLES_DIR / "strands-memory-store" + + +@pytest.mark.syntax +class TestStrandsMemoryStoreStructure: + def test_required_files_exist(self): + for filename in ["README.md", "main.py"]: + assert (STRANDS_MS_DIR / filename).exists(), f"Missing: {filename}" + + def test_main_compiles(self): + ast.parse((STRANDS_MS_DIR / "main.py").read_text(encoding="utf-8")) + + +@pytest.mark.imports +class TestStrandsMemoryStoreImports: + def test_required_imports_resolve(self): + from neo4j_agent_memory import MemorySettings # noqa: F401 + from neo4j_agent_memory.integrations.strands import ( # noqa: F401 + Neo4jMemoryStore, + Neo4jMemoryStoreConfig, + ) + + def test_example_module_imports(self): + """The module must be importable and expose a callable main().""" + spec = importlib.util.spec_from_file_location( + "strands_ms_example", STRANDS_MS_DIR / "main.py" + ) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + try: + spec.loader.exec_module(module) # imports must succeed; main() not called + assert callable(module.main) + finally: + sys.modules.pop("strands_ms_example", None) + + def test_build_settings_structure(self, monkeypatch): + """build_settings() must produce a MemorySettings with no LLM.""" + pytest.importorskip("sentence_transformers") + + monkeypatch.setenv("NEO4J_URI", "bolt://localhost:7687") + monkeypatch.setenv("NEO4J_USERNAME", "neo4j") + monkeypatch.setenv("NEO4J_PASSWORD", "password") + + spec = importlib.util.spec_from_file_location( + "strands_ms_example_settings", STRANDS_MS_DIR / "main.py" + ) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + try: + spec.loader.exec_module(module) + settings = module.build_settings() + # No LLM — runs without API keys. + assert settings.llm is None + finally: + sys.modules.pop("strands_ms_example_settings", None) + + +@pytest.mark.syntax +class TestStrandsMemoryStoreContent: + def test_uses_the_public_store_api(self): + source = (STRANDS_MS_DIR / "main.py").read_text(encoding="utf-8") + assert ( + "from neo4j_agent_memory.integrations.strands import" in source + and "Neo4jMemoryStore" in source + ) + assert "Neo4jMemoryStoreConfig" in source, "construction is dataclass-config based" + assert "llm=None" in source, "the example must run without an API key" + + def test_calls_search_add_and_get_tools(self): + source = (STRANDS_MS_DIR / "main.py").read_text(encoding="utf-8") + assert "store.search(" in source + assert "store.add(" in source + assert "store.get_tools(" in source + assert "store.initialize(" in source diff --git a/tests/integration/nams/test_strands_memory_store.py b/tests/integration/nams/test_strands_memory_store.py new file mode 100644 index 00000000..9b0ea0c6 --- /dev/null +++ b/tests/integration/nams/test_strands_memory_store.py @@ -0,0 +1,84 @@ +"""Live-NAMS integration test — ``Neo4jMemoryStore`` sink resolution. + +``Neo4jMemoryStore._resolve_nams_sink`` finds its existing sink conversation by +a metadata tag, so it depends on NAMS returning conversation ``metadata`` from +both ``create_conversation`` and ``list_conversations``. This asserts that +round-trip against the live service. + +Creates conversations only — no messages, so NAMS extracts nothing and +``clear_session`` teardown is complete. +""" + +from __future__ import annotations + +from collections.abc import AsyncIterator + +import pytest +import pytest_asyncio + +pytest.importorskip("strands", reason="strands-agents not installed") + +from neo4j_agent_memory import MemoryClient +from neo4j_agent_memory.integrations.strands import Neo4jMemoryStore, Neo4jMemoryStoreConfig +from neo4j_agent_memory.integrations.strands.memory_store import _STORE_KEY + +pytestmark = pytest.mark.integration + + +@pytest_asyncio.fixture +async def _sink_cleanup(nams_client: MemoryClient) -> AsyncIterator[list[str]]: + """Conversation ids to delete afterwards, even if an assertion fails.""" + created: list[str] = [] + try: + yield created + finally: + for conversation_id in created: + try: + await nams_client.short_term.clear_session(conversation_id) + except Exception: # noqa: BLE001 - best-effort teardown + pass + + +@pytest.mark.asyncio +async def test_resolve_sink_reuses_metadata_tagged_conversation( + nams_client: MemoryClient, + test_run_id: str, + _sink_cleanup: list[str], +) -> None: + """A second store with the same ``name``/``user_id`` resolves the same sink. + + ``test_run_id`` is a per-invocation UUID prefix (see ``conftest.py``), so + this store name cannot collide with a concurrent or prior run's sink. + """ + store_name = f"{test_run_id}-store" + user_id = f"{test_run_id}-user" + + store_a = Neo4jMemoryStore( + Neo4jMemoryStoreConfig(name=store_name, client=nams_client, user_id=user_id) + ) + store_b = Neo4jMemoryStore( + Neo4jMemoryStoreConfig(name=store_name, client=nams_client, user_id=user_id) + ) + + # No sink exists yet: this creates one, tagged with metadata. + sink_a = await store_a._resolve_sink() + _sink_cleanup.append(sink_a) + + # An independent store instance with the same name/user_id must find that + # sink through the tag rather than create a second one. + sink_b = await store_b._resolve_sink() + + assert sink_a == sink_b + + conversations = await nams_client.short_term.list_conversations( + user_identifier=user_id, limit=1000 + ) + matching = [ + conversation + for conversation in conversations + if (conversation.metadata or {}).get(_STORE_KEY) == store_a._sink_name + ] + assert len(matching) == 1, ( + f"expected exactly one conversation tagged with this store's sink, found {len(matching)}" + ) + assert str(matching[0].id) == sink_a diff --git a/tests/integration/test_multi_tenant_scoping.py b/tests/integration/test_multi_tenant_scoping.py index b4f0fa63..c93da8b6 100644 --- a/tests/integration/test_multi_tenant_scoping.py +++ b/tests/integration/test_multi_tenant_scoping.py @@ -7,6 +7,7 @@ * ``add_message(user_identifier=...)`` writes ``(:User)-[:HAS_CONVERSATION]->(:Conversation)`` and denormalizes ``user_identifier`` onto the Conversation node. +* ``add_messages_batch(user_identifier=...)`` does the same on the bulk path. * ``start_trace(user_identifier=...)`` writes ``(:User)-[:HAS_TRACE]->(:ReasoningTrace)``. """ @@ -109,6 +110,54 @@ async def test_two_users_same_session_id_get_separate_conversations(self, clean_ assert sorted(rows[0]["users"]) == ["liam@omg.com", "sara@omg.com"] +@pytest.mark.integration +@pytest.mark.asyncio +class TestUserScopedBulkMessages: + """Bulk-path coverage for the ``add_messages_batch(user_identifier=...)`` fix. + + Previously ``add_messages_batch`` had no ``user_identifier`` parameter, so + a multi-tenant deployment writing through the bulk path got an unscoped, + unlinked conversation and no enforcement error. + """ + + async def test_add_messages_batch_writes_has_conversation_edge( + self, clean_memory_client, session_id + ): + client = clean_memory_client + await client.users.upsert_user(identifier="sara@omg.com") + + await client.short_term.add_messages_batch( + session_id, + [{"role": "user", "content": "Find healthcare team"}], + user_identifier="sara@omg.com", + ) + + rows = await client.graph.execute_read( + """ + MATCH (u:User {identifier: 'sara@omg.com'})-[:HAS_CONVERSATION]->(c:Conversation) + RETURN c.session_id AS session_id, c.user_identifier AS user_identifier + """, + ) + assert len(rows) == 1 + assert rows[0]["session_id"] == session_id + assert rows[0]["user_identifier"] == "sara@omg.com" + + async def test_conversation_discoverable_via_list_conversations( + self, clean_memory_client, session_id + ): + client = clean_memory_client + await client.users.upsert_user(identifier="sara@omg.com") + + await client.short_term.add_messages_batch( + session_id, + [{"role": "user", "content": "Find healthcare team"}], + user_identifier="sara@omg.com", + ) + + conversations = await client.short_term.list_conversations(user_identifier="sara@omg.com") + assert [c.session_id for c in conversations] == [session_id] + + @pytest.mark.integration @pytest.mark.asyncio class TestUserScopedReasoningTrace: @@ -148,6 +197,14 @@ async def test_start_trace_raises_without_user_identifier( with pytest.raises(ValueError, match="user_identifier"): await multi_tenant_client.reasoning.start_trace(session_id, "Some task") + async def test_add_messages_batch_raises_without_user_identifier( + self, multi_tenant_client, session_id + ): + with pytest.raises(ValueError, match="user_identifier"): + await multi_tenant_client.short_term.add_messages_batch( + session_id, [{"role": "user", "content": "Hello"}] + ) + async def test_add_preference_raises_without_user_identifier(self, multi_tenant_client): with pytest.raises(ValueError, match="user_identifier"): await multi_tenant_client.long_term.add_preference("food", "Italian") diff --git a/tests/integration/test_strands_memory_store_integration.py b/tests/integration/test_strands_memory_store_integration.py new file mode 100644 index 00000000..d846239a --- /dev/null +++ b/tests/integration/test_strands_memory_store_integration.py @@ -0,0 +1,218 @@ +"""Neo4jMemoryStore against a real Neo4j (bolt).""" + +from __future__ import annotations + +import pytest + +pytest.importorskip("strands", reason="strands-agents not installed") + +#: Dimensionality of ``tests/conftest.py``'s ``MockEmbedder``, which +#: ``clean_memory_client`` wires in for this module's tests. +_EXPECTED_DIMENSIONS = 1536 + +#: Same set as ``SchemaManager._MANAGED_VECTOR_INDEXES`` +#: (``src/neo4j_agent_memory/graph/schema.py``). +_MANAGED_VECTOR_INDEXES = ( + "entity_embedding_idx", + "fact_embedding_idx", + "message_embedding_idx", + "preference_embedding_idx", + "step_embedding_idx", + "task_embedding_idx", +) + + +@pytest.fixture(autouse=True) +def _skip_on_incompatible_vector_indexes(neo4j_connection_info) -> None: + """Skip with a clear reason if the container's schema won't fit this suite. + + The docker-compose Neo4j container (``docker-compose.test.yml``) uses a + *named* volume: data and vector indexes survive ``make neo4j-stop`` / + ``neo4j-start`` (only ``make neo4j-clean`` wipes it). If + ``examples/strands-memory-store/`` (a 384-dim sentence-transformers + embedder) was run against the same container, its vector indexes are + sized for 384 dimensions. ``clean_memory_client`` then connects with the + 1536-dim ``MockEmbedder`` and ``MemoryClient.connect()`` raises + ``EmbeddingDimensionMismatchError`` deep inside that fixture's generic + ``except Exception: pytest.skip(f"Neo4j not available: {e}")`` — a + reason that reads as "no Neo4j" when the real story is "wrong schema". + + This checks the raw index dimensions first (bypassing ``MemoryClient`` + and its embedder entirely) so the skip names the actual cause and the + fix. It only reads ``SHOW VECTOR INDEXES`` — it never drops or modifies + anything, since these indexes may serve other embedders the developer + is intentionally running against this container. + """ + from neo4j import GraphDatabase + + driver = GraphDatabase.driver( + neo4j_connection_info["uri"], + auth=(neo4j_connection_info["username"], neo4j_connection_info["password"]), + ) + try: + with driver.session() as session: + rows = session.run( + "SHOW VECTOR INDEXES YIELD name, options RETURN name AS name, options AS options" + ).data() + finally: + driver.close() + + mismatched = [] + for row in rows: + name = row.get("name") + if name not in _MANAGED_VECTOR_INDEXES: + continue + config = (row.get("options") or {}).get("indexConfig") or {} + dims = config.get("vector.dimensions") + if isinstance(dims, int) and dims != _EXPECTED_DIMENSIONS: + mismatched.append(f"{name} ({dims}d)") + + if mismatched: + pytest.skip( + f"Neo4j at {neo4j_connection_info['uri']!r} has vector indexes sized " + f"for a different embedder than this suite's {_EXPECTED_DIMENSIONS}-dim " + f"MockEmbedder: {', '.join(mismatched)}. Likely cause: a prior run of " + "examples/strands-memory-store/ (384-dim sentence-transformers) against " + "this same persistent container. Fix: `make neo4j-clean` (drops the " + "container's data volume), then `make neo4j-start`." + ) + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_round_trip_recall(clean_memory_client) -> None: + from neo4j_agent_memory.integrations.strands import ( + Neo4jMemoryStore, + Neo4jMemoryStoreConfig, + ) + + await clean_memory_client.long_term.add_preference("ui", "Prefers dark mode") + + store = Neo4jMemoryStore(Neo4jMemoryStoreConfig(name="graph", client=clean_memory_client)) + await store.initialize() + entries = await store.search("dark mode") + + assert any("dark mode" in entry.content for entry in entries) + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_add_messages_reuses_the_same_sink_across_instances(clean_memory_client) -> None: + """Two store instances with the same name/user_id resolve to one sink. + + On bolt the sink name is deterministic (``strands-memory-store/{user}/{name}``) + and *is* the conversation key — no backend lookup is needed to find it, unlike + NAMS where ids are server-minted and the sink is found by matching metadata. + This proves the bolt path: no duplicate sink conversation is created, and both + instances' writes land in the one conversation. + """ + from neo4j_agent_memory.integrations.strands import ( + Neo4jMemoryStore, + Neo4jMemoryStoreConfig, + ) + + batch = [{"role": "user", "content": [{"text": "I work at Acme Corp"}]}] + + first = Neo4jMemoryStore(Neo4jMemoryStoreConfig(name="graph", client=clean_memory_client)) + await first.initialize() + await first.add_messages(batch, None) + + second = Neo4jMemoryStore(Neo4jMemoryStoreConfig(name="graph", client=clean_memory_client)) + await second.initialize() + await second.add_messages(batch, None) + + sink_name = await first._resolve_sink() + assert await second._resolve_sink() == sink_name + + conversations = await clean_memory_client.short_term.list_conversations(limit=100) + matching = [c for c in conversations if c.session_id == sink_name] + assert len(matching) == 1, "exactly one sink conversation, not one per store instance" + + conversation = await clean_memory_client.short_term.get_conversation(sink_name) + assert len(conversation.messages) == 2, "both instances' batches landed in the one sink" + + +@pytest.mark.integration +def test_an_owned_client_survives_the_loop_change_strands_forces(neo4j_connection_info) -> None: + """The Quick Start's shape: ``settings=``, driven by synchronous ``Agent(...)``. + + ``Agent.__init__`` calls ``MemoryManager.init_agent`` -> ``store.initialize()`` + through ``strands._async.run_async`` (``asyncio.run`` in a throwaway thread), + and every ``Agent.__call__`` runs on a *different* loop. The neo4j async + driver stays bound to the loop that opened it, so before the rebind in + ``initialize()`` this raised ``RuntimeError: Task ... attached to a + different loop`` from deep inside the driver. + + Real work, not ``search()``: with no embedder ``search()`` short-circuits + to ``[]`` without touching the driver, so it would pass either way. + """ + from pydantic import SecretStr + from strands._async import run_async + + from neo4j_agent_memory import MemorySettings + from neo4j_agent_memory.config.settings import Neo4jConfig + from neo4j_agent_memory.integrations.strands import ( + Neo4jMemoryStore, + Neo4jMemoryStoreConfig, + ) + + settings = MemorySettings( + neo4j=Neo4jConfig( + uri=neo4j_connection_info["uri"], + username=neo4j_connection_info["username"], + password=SecretStr(neo4j_connection_info["password"]), + ) + ) + store = Neo4jMemoryStore(Neo4jMemoryStoreConfig(name="loop-rebind", settings=settings)) + + # Loop A — what Agent.__init__ does. + run_async(store.initialize) + connecting_loop = store._connected_loop + assert connecting_loop is not None + + # Loop B — what Agent.__call__ does. + async def real_work() -> list[dict[str, object]]: + await store.initialize() + return await store._client.query.cypher("RETURN 1 AS n") + + try: + assert run_async(real_work) == [{"n": 1}] + assert store._connected_loop is not connecting_loop, "the store rebound to the new loop" + finally: + run_async(store.aclose) + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_entity_graph_reports_what_the_bolt_stack_can_actually_report( + clean_memory_client, +) -> None: + """Pins the edge payload against a real Neo4j — and a library defect under it. + + ``Neo4jClient.execute_read`` returns ``result.data()``, which renders a + relationship as ``(start_props, type, end_props)`` and drops its + properties entirely. ``LongTermMemory.get_related_entities`` therefore + falls through to ``type="RELATED_TO"`` for every hit even though + ``CREATE_ENTITY_RELATIONSHIP`` stored ``r.type = "WORKS_AT"``, and it + hardcodes ``source_id`` to the centre because + ``GET_ENTITY_RELATIONSHIPS`` matches undirected. + + That defect is the library's, not the store's, and is deliberately left + for a separate change; this test pins today's behaviour so a fix shows + up here (and in ``strands_fakes.FakeLongTerm.get_related_entities``, + which mirrors it) rather than silently changing what the tool tells the + model. + """ + from neo4j_agent_memory.integrations.strands._store_tools import _entity_graph + + long_term = clean_memory_client.long_term + acme, _ = await long_term.add_entity("Acme Corp", "ORGANIZATION", deduplicate=False) + ada, _ = await long_term.add_entity("Ada Lovelace", "PERSON", deduplicate=False) + await long_term.add_relationship(ada, acme, "WORKS_AT") + + result = await _entity_graph(clean_memory_client, "Acme Corp", depth=2, nams=False) + + assert result["center"] == "Acme Corp" + assert result["edges"] == [ + {"from": "Acme Corp", "relationship": "RELATED_TO", "to": "Ada Lovelace"} + ] diff --git a/tests/unit/integrations/strands_fakes.py b/tests/unit/integrations/strands_fakes.py index e9515384..dbe9f50b 100644 --- a/tests/unit/integrations/strands_fakes.py +++ b/tests/unit/integrations/strands_fakes.py @@ -1,26 +1,216 @@ -"""Stateful fake MemoryClient for Strands session-manager unit tests. - -Implements only the methods Neo4jSessionManager touches, with NAMS-mode -semantics behind a flag (server-issued conversation UUIDs, kwargs -dropped on add_message). State-based tests beat call-sequence mocks for -round-trip behavior (append -> restore). +"""Test doubles for the Strands integration's unit tests. + +Two deliberately **asymmetric** backends behind one flag: + +* ``FakeMemoryClient(nams_mode=True)`` exposes the **real** + :class:`NamsShortTermMemory` / :class:`NamsLongTermMemory`, driven by + :class:`StubTransport` — a stub of the one narrow method the NAMS memory + classes use to reach the network + (``HttpTransport.request(spec, path_params=, json=, params=)``). Nothing + about the NAMS memory API is re-implemented here, so a test can no longer + assert against behaviour NAMS does not have. The observable surface is the + *requests the transport recorded*, which is what NAMS would really receive. + +* ``FakeMemoryClient()`` (bolt) keeps the hand-written duck-typed fakes below. + The real bolt classes drive a live Neo4j session through + ``Neo4jClient.execute_read/execute_write``, which a unit test cannot have; + bolt fidelity is pinned instead by + ``tests/integration/test_strands_memory_store_integration.py``. + +That asymmetry is intentional — please do not "finish the job" by making both +sides the same. Replacing the NAMS side with a duck-typed fake reintroduces +the class of defect this module exists to remove (a fake more permissive than +the backend it stands in for); replacing the bolt side with the real classes +requires a database. + +State-based tests beat call-sequence mocks for round-trip behavior +(append -> restore), which is why the bolt fakes are stateful. """ from __future__ import annotations import uuid +from dataclasses import dataclass, field from typing import Any -from neo4j_agent_memory.core.exceptions import MemoryError as NamMemoryError from neo4j_agent_memory.memory.short_term import Conversation, Message, MessageRole +from neo4j_agent_memory.nams.endpoints import EndpointSpec +from neo4j_agent_memory.nams.transport import HttpTransport + +# --------------------------------------------------------------------------- +# Stubbed NAMS wire +# --------------------------------------------------------------------------- + +#: Fixed timestamp for canned NAMS payloads (NAMS returns ISO 8601 strings). +_STUB_NOW = "2026-08-19T12:00:00+00:00" + +#: Sentinel: this NAMS operation has neither an override nor a canned default. +_MISSING = object() + + +@dataclass(frozen=True) +class StubCall: + """One request the NAMS memory classes made on the transport.""" + + method: str #: ``spec.bridge_method`` — the NAMS operation name. + spec: EndpointSpec + path_params: dict[str, object] = field(default_factory=dict) + json: Any = None + params: dict[str, Any] | None = None + + +def _canned_conversation(call: StubCall) -> dict[str, Any]: + """``POST /conversations`` — NAMS mints the id and echoes nothing else back. + + Deliberately does *not* echo ``metadata``: whether the create response + carries it is a server detail nothing in this package relies on (the + callers use ``created.id`` only). Assert on the recorded request body. + """ + return {"id": str(uuid.uuid4()), "createdAt": _STUB_NOW} + + +def _canned_message(call: StubCall) -> dict[str, Any]: + body = call.json if isinstance(call.json, dict) else {} + return { + "id": str(uuid.uuid4()), + "role": body.get("role", "user"), + "content": body.get("content", ""), + "createdAt": _STUB_NOW, + } + + +def _canned_bulk(call: StubCall) -> dict[str, Any]: + body = call.json if isinstance(call.json, dict) else {} + batch = body.get("messages") or [] + return { + "messages": [ + { + "id": str(uuid.uuid4()), + "role": m.get("role", "user"), + "content": m.get("content", ""), + "createdAt": _STUB_NOW, + } + for m in batch + ] + } + + +def _canned_entity(call: StubCall) -> dict[str, Any]: + body = call.json if isinstance(call.json, dict) else {} + return { + "id": str(uuid.uuid4()), + "name": body.get("name", ""), + # NAMS stores (and returns) its own lowercase type vocabulary; the + # memory class uppercases it again on the way back. + "type": body.get("type", "custom"), + "createdAt": _STUB_NOW, + } + + +#: Realistic empty/echo responses per NAMS operation, matching the response +#: envelopes the memory classes parse. A test overrides any of these via +#: ``transport.responses[method] = payload`` (or a ``StubCall -> payload`` +#: callable). An operation with neither an override nor a default raises — +#: an unexpected NAMS call must never quietly succeed. +_CANNED: dict[str, Any] = { + "create_conversation": _canned_conversation, + "list_conversations": {"conversations": []}, + "get_conversation": {"createdAt": _STUB_NOW}, + "list_messages": {"messages": []}, + "add_message": _canned_message, + "bulk_add_messages": _canned_bulk, + "search_messages": {"messages": [], "searchType": "vector"}, + "delete_conversation": None, + "add_entity": _canned_entity, + "search_entities": {"entities": [], "searchType": "vector"}, + "expand_graph": {"nodes": [], "edges": []}, + "get_extraction_status": {"messages": [], "summary": {}}, +} + + +class _NoAuth: + """``AuthProvider`` that adds no headers (nothing reaches the network).""" + + async def apply(self, headers: dict[str, str]) -> dict[str, str]: + return headers + + +class StubTransport(HttpTransport): + """Records NAMS requests and answers them from canned payloads. + + Subclasses the real :class:`HttpTransport` rather than duck-typing it, so + the stub is bound to the actual constructor and the actual ``request`` + signature: if ``request`` grows or renames a parameter, the NAMS memory + classes call this override with the new keyword and the tests fail with a + ``TypeError`` instead of silently drifting. No socket is ever opened — + ``request`` is overridden above the point where the httpx client is built. + """ + + def __init__(self) -> None: + # A ``/v1`` endpoint so ``detect_protocol`` selects the REST wire, the + # one the hosted service speaks. + super().__init__(endpoint="https://nams.invalid/v1", auth=_NoAuth()) + self.calls: list[StubCall] = [] + #: NAMS operation name -> payload, or a ``StubCall -> payload`` callable. + self.responses: dict[str, Any] = {} + + async def request( + self, + spec: EndpointSpec, + *, + path_params: dict[str, object] | None = None, + json: Any = None, + params: dict[str, Any] | None = None, + ) -> Any: + call = StubCall( + method=spec.bridge_method, + spec=spec, + path_params=dict(path_params or {}), + json=json, + params=params, + ) + self.calls.append(call) + canned = self.responses.get(spec.bridge_method, _CANNED.get(spec.bridge_method, _MISSING)) + if canned is _MISSING: + raise AssertionError( + f"StubTransport: unexpected NAMS call {spec.bridge_method!r} " + f"({spec.rest_method} {spec.rest_path}). Set " + f"transport.responses[{spec.bridge_method!r}] if the test intends it." + ) + return canned(call) if callable(canned) else canned + + # ------------------------------------------------------------ assertions + + @property + def methods(self) -> list[str]: + """NAMS operation names, in call order.""" + return [call.method for call in self.calls] + + def calls_for(self, method: str) -> list[StubCall]: + return [call for call in self.calls if call.method == method] + + def last(self, method: str) -> StubCall: + calls = self.calls_for(method) + assert calls, f"no {method!r} call was made (calls: {self.methods})" + return calls[-1] + + +# --------------------------------------------------------------------------- +# Bolt fakes (see the module docstring for why these are hand-written) +# --------------------------------------------------------------------------- class FakeShortTerm: - def __init__(self, nams_mode: bool) -> None: - self._nams_mode = nams_mode - # key -> Conversation. Bolt: key == session_id. NAMS: key == str(uuid). + """Duck-typed stand-in for the bolt ``ShortTermMemory``.""" + + def __init__(self) -> None: + #: session_id -> Conversation (bolt keys conversations by session_id). self.conversations: dict[str, Conversation] = {} self.add_message_calls: list[dict[str, Any]] = [] + self.bulk_calls: list[dict[str, Any]] = [] + #: Raise from the Nth ``bulk_add_messages`` call onwards (1-based), to + #: exercise a batch that fails partway through its chunks. + self.fail_bulk_from: int | None = None self.deleted_message_ids: list[str] = [] self.fail_next_add = False self.list_conversations_calls: list[dict[str, Any]] = [] @@ -30,14 +220,10 @@ def __init__(self, nams_mode: bool) -> None: async def create_conversation( self, session_id: str | None = None, **kwargs: Any ) -> Conversation: - conv_id = uuid.uuid4() - key = str(conv_id) if self._nams_mode else str(session_id) - conv = Conversation( - id=conv_id, - session_id=str(session_id), - metadata=kwargs.get("metadata") or {}, - ) - self.conversations[key] = conv + # Bolt's CREATE_CONVERSATION has no metadata property, so a bolt + # conversation never carries metadata no matter what the caller passes. + conv = Conversation(id=uuid.uuid4(), session_id=str(session_id), metadata={}) + self.conversations[str(session_id)] = conv return conv async def list_conversations(self, **kwargs: Any) -> list[Conversation]: @@ -49,8 +235,6 @@ async def get_conversation(self, session_id: str, **kwargs: Any) -> Conversation self.get_conversation_kwargs.append({"session_id": session_id, **kwargs}) conv = self.conversations.get(session_id) if conv is None: - if self._nams_mode: - raise NamMemoryError(f"NAMS: conversation {session_id} not found") # Bolt contract: empty conversation, no exception. return Conversation(session_id=session_id) return conv @@ -59,31 +243,49 @@ async def add_message(self, session_id: str, role: str, content: str, **kwargs: if self.fail_next_add: self.fail_next_add = False raise RuntimeError("backend down") - # Real NAMS accepts only {content, role} on add_message and silently - # drops everything else (metadata, user_identifier, bolt-only knobs). - # Mirror that here so NAMS-mode tests can't lean on dropped kwargs. - recorded = {"session_id": session_id, "role": role, "content": content} - if not self._nams_mode: - recorded.update(kwargs) - self.add_message_calls.append(recorded) + self.add_message_calls.append( + {"session_id": session_id, "role": role, "content": content, **kwargs} + ) if session_id not in self.conversations: - if self._nams_mode: - raise NamMemoryError(f"NAMS: unknown conversation {session_id}") await self.create_conversation(session_id=session_id) msg = Message(role=MessageRole(role), content=content) self.conversations[session_id].messages.append(msg) return msg + async def bulk_add_messages( + self, + session_id: str, + messages: list[dict[str, Any]], + *, + generate_embeddings: bool = True, + extract_entities: bool = False, + extract_relations: bool = True, + user_identifier: str | None = None, + ) -> list[Message]: + # Explicit parameters mirroring ShortTermProtocol.bulk_add_messages + # (no **kwargs catch-all) so this fake can't absorb a keyword the + # real bolt backend would reject. + self.bulk_calls.append( + { + "session_id": session_id, + "messages": messages, + "kwargs": { + "generate_embeddings": generate_embeddings, + "extract_entities": extract_entities, + "extract_relations": extract_relations, + "user_identifier": user_identifier, + }, + } + ) + if self.fail_bulk_from is not None and len(self.bulk_calls) >= self.fail_bulk_from: + raise RuntimeError("bulk write failed") + if session_id not in self.conversations: + await self.create_conversation(session_id=session_id, user_identifier=user_identifier) + stored = [Message(role=MessageRole(m["role"]), content=m["content"]) for m in messages] + self.conversations[session_id].messages.extend(stored) + return stored + async def delete_message(self, message_id: Any, **kwargs: Any) -> bool: - if self._nams_mode: - from neo4j_agent_memory.core.exceptions import NotSupportedError - - raise NotSupportedError( - backend="nams", - method="ShortTermMemory.delete_message", - message="NAMS does not expose a message-delete endpoint.", - workaround="Use clear_session(session_id) to clear an entire conversation.", - ) self.deleted_message_ids.append(str(message_id)) for conv in self.conversations.values(): conv.messages = [m for m in conv.messages if str(m.id) != str(message_id)] @@ -91,12 +293,37 @@ async def delete_message(self, message_id: Any, **kwargs: Any) -> bool: class FakeLongTerm: + """Duck-typed stand-in for the bolt ``LongTermMemory``. + + Bolt-only by construction: no ``expand_graph`` (that method exists on + NAMS alone), and every method behaves the way the bolt implementation + behaves — including ``add_entity`` returning the + ``(Entity, DeduplicationResult)`` tuple. + """ + def __init__(self) -> None: self.entities: list[Any] = [] self.preferences: list[Any] = [] self.facts: list[Any] = [] self.fail_searches = False + self.fail_preferences = False + self.fail_facts = False self.search_calls: int = 0 + self.search_kwargs: list[dict[str, Any]] = [] + self.added_preferences: list[tuple[str, str]] = [] + self.added_facts: list[tuple[str, str, str]] = [] + self.added_entities: list[tuple[str, str]] = [] + self.related: list[Any] = [] + self.related_kwargs: list[dict[str, Any]] = [] + self.preferences_for: list[Any] = [] + self.preferences_for_calls: list[dict[str, Any]] = [] + #: Preferences that got a ``(:User)-[:HAS_PREFERENCE]`` edge, keyed by + #: user -- the only ones ``get_preferences_for`` can see. + self.preferences_by_user: dict[str, list[Any]] = {} + #: Mirrors ``MemorySettings.memory.multi_tenant``: bolt's + #: ``_enforce_multi_tenant`` raises ``ValueError`` (not + #: ``NotSupportedError``) when it is on and no identifier is passed. + self.multi_tenant = False async def _maybe_fail(self) -> None: if self.fail_searches: @@ -104,21 +331,96 @@ async def _maybe_fail(self) -> None: async def search_entities(self, query: str, **kwargs: Any) -> list[Any]: self.search_calls += 1 + self.search_kwargs.append({"query": query, **kwargs}) await self._maybe_fail() return self.entities async def search_preferences(self, query: str, **kwargs: Any) -> list[Any]: self.search_calls += 1 + self.search_kwargs.append({"query": query, **kwargs}) + if self.fail_preferences: + raise RuntimeError("preference backend down") await self._maybe_fail() return self.preferences async def search_facts(self, query: str, **kwargs: Any) -> list[Any]: self.search_calls += 1 + self.search_kwargs.append({"query": query, **kwargs}) + if self.fail_facts: + raise RuntimeError("fact backend down") await self._maybe_fail() return self.facts + async def add_preference( + self, category: str, preference: str, *, user_identifier: str | None = None, **kwargs: Any + ) -> Any: + if self.multi_tenant and user_identifier is None: + raise ValueError( + "MemorySettings.memory.multi_tenant=True but no user_identifier was supplied." + ) + self.added_preferences.append((category, preference)) + from neo4j_agent_memory.memory.long_term import Preference + + stored = Preference(category=category, preference=preference) + # Bolt writes the (:User)-[:HAS_PREFERENCE] edge only when + # user_identifier is given, and get_preferences_for reads exactly + # that edge -- so an unscoped write is invisible to it. + if user_identifier is not None: + self.preferences_by_user.setdefault(user_identifier, []).append(stored) + return stored + + async def add_fact(self, subject: str, predicate: str, obj: str, **kwargs: Any) -> Any: + self.added_facts.append((subject, predicate, obj)) + from neo4j_agent_memory.memory.long_term import Fact + + return Fact(subject=subject, predicate=predicate, object=obj) + + async def add_entity(self, name: str, entity_type: str, **kwargs: Any) -> Any: + from neo4j_agent_memory.memory.long_term import Entity + + self.added_entities.append((name, entity_type)) + # Bolt returns (Entity, DeduplicationResult); NAMS returns a bare + # Entity -- and that path now runs the real NamsLongTermMemory. + return Entity(name=name, type=entity_type), None + + async def get_related_entities(self, entity: Any, **kwargs: Any) -> list[tuple[Any, Any]]: + """Real ``Relationship`` objects, shaped as the bolt path really shapes them. + + The bolt implementation cannot report a relationship's own type or + direction: ``Neo4jClient.execute_read`` returns ``result.data()``, + which renders a relationship as ``(start_props, type, end_props)`` and + drops its properties, so ``memory/long_term.py``'s parse falls through + to ``type="RELATED_TO"`` for every hit, with ``source_id`` hardcoded to + the centre. ``tests/integration/test_strands_memory_store_integration.py`` + asserts this against a live Neo4j; the fake reproduces it rather than + inventing a richer relationship the production stack never returns. + """ + self.related_kwargs.append(kwargs) + from neo4j_agent_memory.memory.long_term import Relationship + + centre_id = getattr(entity, "id", entity) + return [ + ( + other, + Relationship(source_id=centre_id, target_id=other.id, type="RELATED_TO"), + ) + for other in self.related + ] + + async def get_preferences_for(self, user_identifier: str, **kwargs: Any) -> list[Any]: + self.preferences_for_calls.append({"user_identifier": user_identifier, **kwargs}) + return [*self.preferences_for, *self.preferences_by_user.get(user_identifier, [])] + class FakeReasoning: + """Duck-typed reasoning layer. + + Bolt-shaped, and used in bolt mode only: the session manager's tool-call + mirroring is exercised there. If a NAMS-mode test ever needs reasoning, + wire ``NamsReasoningMemory`` to the ``StubTransport`` instead of extending + this class. + """ + def __init__(self) -> None: self.traces: list[dict[str, Any]] = [] self.steps: list[dict[str, Any]] = [] @@ -150,17 +452,38 @@ async def record_tool_call( class FakeMemoryClient: - """Duck-typed MemoryClient covering the session manager's surface.""" + """Duck-typed MemoryClient covering the Strands integration's surface. + + In NAMS mode ``short_term`` / ``long_term`` are the real NAMS classes over + :attr:`transport`; in bolt mode they are the fakes above. + """ def __init__(self, nams_mode: bool = False) -> None: self._nams_mode = nams_mode - self.short_term = FakeShortTerm(nams_mode) - self.long_term = FakeLongTerm() + self.transport: StubTransport | None = None + self.short_term: Any + self.long_term: Any + if nams_mode: + from neo4j_agent_memory.nams.long_term import NamsLongTermMemory + from neo4j_agent_memory.nams.short_term import NamsShortTermMemory + + self.transport = StubTransport() + self.short_term = NamsShortTermMemory(self.transport) + self.long_term = NamsLongTermMemory(self.transport) + else: + self.short_term = FakeShortTerm() + self.long_term = FakeLongTerm() self.reasoning = FakeReasoning() self.connect_calls = 0 self.close_calls = 0 self._connected = False + @property + def wire(self) -> StubTransport: + """The stubbed NAMS wire (NAMS mode only) — where the assertions live.""" + assert self.transport is not None, "wire is NAMS-mode only" + return self.transport + @property def is_nams(self) -> bool: return self._nams_mode @@ -176,3 +499,12 @@ async def connect(self) -> None: async def close(self) -> None: self.close_calls += 1 self._connected = False + + +class FakeAgent: + """Minimal Agent stand-in for the session manager's initialize(agent).""" + + def __init__(self, memory_manager: Any = None) -> None: + self.memory_manager = memory_manager + self.messages: list[Any] = [] + self.state: dict[str, Any] = {} diff --git a/tests/unit/integrations/test_strands_coexistence.py b/tests/unit/integrations/test_strands_coexistence.py new file mode 100644 index 00000000..3d4fa79c --- /dev/null +++ b/tests/unit/integrations/test_strands_coexistence.py @@ -0,0 +1,172 @@ +"""Coexistence of Neo4jSessionManager and Neo4jMemoryStore on one agent. + +Both constructs are first-class Agent parameters, so pairing them is normal. +Two overlaps are not: double extraction (raises) and double injection (warns). +""" + +from __future__ import annotations + +from typing import Any + +import pytest + +pytest.importorskip("strands", reason="strands-agents not installed") + +from tests.unit.integrations.strands_fakes import FakeAgent, FakeMemoryClient + + +def _manager_with_store(**store_kwargs: Any) -> Any: + from strands.memory import MemoryManager + + from neo4j_agent_memory.integrations.strands import Neo4jMemoryStore, Neo4jMemoryStoreConfig + + store = Neo4jMemoryStore( + Neo4jMemoryStoreConfig(name="graph", client=FakeMemoryClient(), **store_kwargs) + ) + return MemoryManager(stores=[store]) + + +class TestDoubleExtraction: + def test_raises_when_both_sides_extract(self) -> None: + from neo4j_agent_memory.integrations.strands import Neo4jSessionManager + + manager = Neo4jSessionManager("s1", memory_client=FakeMemoryClient(), extract_entities=True) + agent = FakeAgent(memory_manager=_manager_with_store(extraction=True)) + + with pytest.raises(ValueError) as excinfo: + manager.initialize(agent) + + message = str(excinfo.value) + assert "twice" in message + assert "extraction=False" in message + assert "extract_entities=False" in message + + def test_raises_on_nams_even_with_session_extraction_off(self) -> None: + """NAMS extracts server-side regardless of the session manager's flag.""" + from neo4j_agent_memory.integrations.strands import Neo4jSessionManager + + manager = Neo4jSessionManager( + "s1", memory_client=FakeMemoryClient(nams_mode=True), extract_entities=False + ) + agent = FakeAgent(memory_manager=_manager_with_store(extraction=True)) + + with pytest.raises(ValueError, match="twice"): + manager.initialize(agent) + + def test_allows_the_recommended_pairing(self) -> None: + from neo4j_agent_memory.integrations.strands import Neo4jSessionManager + + manager = Neo4jSessionManager("s1", memory_client=FakeMemoryClient(), extract_entities=True) + agent = FakeAgent(memory_manager=_manager_with_store()) # extraction off + + manager.initialize(agent) # must not raise + + def test_allows_store_owned_extraction_when_session_manager_does_not(self) -> None: + from neo4j_agent_memory.integrations.strands import Neo4jSessionManager + + manager = Neo4jSessionManager( + "s1", memory_client=FakeMemoryClient(), extract_entities=False + ) + agent = FakeAgent(memory_manager=_manager_with_store(extraction=True)) + + manager.initialize(agent) # must not raise + + def test_ignores_a_memory_manager_holding_only_foreign_stores(self) -> None: + """extraction=True on the foreign store, deliberately: the point is that + ``_our_stores`` filters by ``isinstance(store, Neo4jMemoryStore)``, not by + the extraction flag. A foreign store with extraction off would pass this + test even if the isinstance filter were replaced by duck-typing (e.g. + ``getattr(s, "extraction", False)``) — that regression must still fail + here, on a store that both extracts and isn't ours. + """ + from strands.memory import MemoryManager + from strands.vended_memory_stores.test_memory_store import TestMemoryStore + + from neo4j_agent_memory.integrations.strands import Neo4jSessionManager + + manager = Neo4jSessionManager("s1", memory_client=FakeMemoryClient(), extract_entities=True) + agent = FakeAgent( + memory_manager=MemoryManager(stores=[TestMemoryStore(name="t", extraction=True)]) + ) + + manager.initialize(agent) # not our store, not our problem + + def test_no_memory_manager_is_fine(self) -> None: + from neo4j_agent_memory.integrations.strands import Neo4jSessionManager + + manager = Neo4jSessionManager("s1", memory_client=FakeMemoryClient()) + + manager.initialize(FakeAgent(memory_manager=None)) # must not raise + + +class TestDoubleInjection: + def test_warns_once_when_both_inject(self, caplog: pytest.LogCaptureFixture) -> None: + from neo4j_agent_memory.integrations.strands import ( + Neo4jRetrievalConfig, + Neo4jSessionManager, + ) + + manager = Neo4jSessionManager( + "s1", + memory_client=FakeMemoryClient(), + extract_entities=False, + retrieval_config=Neo4jRetrievalConfig(), + ) + agent = FakeAgent(memory_manager=_manager_with_store()) + + manager.initialize(agent) + manager.initialize(agent) + + assert caplog.text.lower().count("injected twice") == 1 + + def test_silent_without_retrieval_config(self, caplog: pytest.LogCaptureFixture) -> None: + from neo4j_agent_memory.integrations.strands import Neo4jSessionManager + + manager = Neo4jSessionManager( + "s1", memory_client=FakeMemoryClient(), extract_entities=False + ) + agent = FakeAgent(memory_manager=_manager_with_store()) + + manager.initialize(agent) + + assert "injected twice" not in caplog.text.lower() + + def test_silent_when_manager_injection_is_disabled( + self, caplog: pytest.LogCaptureFixture + ) -> None: + from strands.memory import MemoryManager + + from neo4j_agent_memory.integrations.strands import ( + Neo4jMemoryStore, + Neo4jMemoryStoreConfig, + Neo4jRetrievalConfig, + Neo4jSessionManager, + ) + + store = Neo4jMemoryStore(Neo4jMemoryStoreConfig(name="graph", client=FakeMemoryClient())) + manager = Neo4jSessionManager( + "s1", + memory_client=FakeMemoryClient(), + extract_entities=False, + retrieval_config=Neo4jRetrievalConfig(), + ) + agent = FakeAgent(memory_manager=MemoryManager(stores=[store], injection=False)) + + manager.initialize(agent) + + assert "injected twice" not in caplog.text.lower() + + +class TestPrivateAttributeCoupling: + def test_memory_manager_still_exposes_stores(self) -> None: + """Fails loudly if a strands upgrade moves MemoryManager._stores.""" + from strands.memory import MemoryManager + from strands.vended_memory_stores.test_memory_store import TestMemoryStore + + manager = MemoryManager(stores=[TestMemoryStore(name="t")]) + + assert hasattr(manager, "_stores"), ( + "MemoryManager._stores is gone; the coexistence guards in " + "session_manager.py need a new way to enumerate stores." + ) + assert [s.name for s in manager._stores] == ["t"] diff --git a/tests/unit/integrations/test_strands_memory_protocol.py b/tests/unit/integrations/test_strands_memory_protocol.py new file mode 100644 index 00000000..a2be930b --- /dev/null +++ b/tests/unit/integrations/test_strands_memory_protocol.py @@ -0,0 +1,37 @@ +"""The store depends on strands' LTM module, added in strands-agents 1.44.0.""" + +from __future__ import annotations + +import pytest + +pytest.importorskip("strands", reason="strands-agents not installed") + + +class TestMemoryProtocolAvailable: + def test_memory_module_exports_the_protocol(self) -> None: + from strands.memory import ( + AddMessagesContext, + MemoryEntry, + MemoryStore, + MemoryStoreConfig, + SearchOptions, + ) + + assert MemoryStore is not None + assert MemoryStoreConfig is not None + assert SearchOptions is not None + assert AddMessagesContext is not None + assert MemoryEntry(content="x").content == "x" + + def test_optional_methods_are_detected_by_type_not_instance(self) -> None: + """_has_method inspects type(store); an inherited stub counts as absent.""" + from strands.memory.types import MemoryStore as Proto + from strands.memory.types import _has_method + + class Bare(Proto): + async def search(self, query: str, options: object = None) -> list[object]: + return [] + + assert _has_method(Bare(), "search") is True + assert _has_method(Bare(), "add") is False + assert _has_method(Bare(), "add_messages") is False diff --git a/tests/unit/integrations/test_strands_memory_store.py b/tests/unit/integrations/test_strands_memory_store.py new file mode 100644 index 00000000..8ffea0b5 --- /dev/null +++ b/tests/unit/integrations/test_strands_memory_store.py @@ -0,0 +1,1247 @@ +"""Neo4jMemoryStore — construction, attributes, lifecycle.""" + +from __future__ import annotations + +from typing import Any + +import pytest + +pytest.importorskip("strands", reason="strands-agents not installed") + +from tests.unit.integrations.strands_fakes import FakeMemoryClient + +#: Entity ids in canned NAMS payloads must parse as UUIDs (``Entity.id: UUID``). +_ENTITY_ID = "11111111-1111-1111-1111-111111111111" + + +def _store(**kw: Any) -> Any: + """Build a Neo4jMemoryStore from loose kwargs via its real config dataclass. + + Keeps the individual tests readable while still exercising the actual + ``Neo4jMemoryStoreConfig`` field names — ``Neo4jMemoryStoreConfig(**kw)`` + is the real dataclass constructor, so a typo in a field name here is a + ``TypeError``, not a silently-ignored key. + """ + from neo4j_agent_memory.integrations.strands import Neo4jMemoryStore, Neo4jMemoryStoreConfig + + return Neo4jMemoryStore(Neo4jMemoryStoreConfig(**kw)) + + +class TestConstruction: + def test_requires_a_name(self) -> None: + """``name`` must be non-empty; checked eagerly in + ``Neo4jMemoryStoreConfig.__post_init__``, before the store ever sees it.""" + + with pytest.raises(ValueError, match="name"): + _store(name="", client=FakeMemoryClient()) + + def test_requires_exactly_one_of_client_or_settings(self) -> None: + from pydantic import SecretStr + + from neo4j_agent_memory import MemorySettings + from neo4j_agent_memory.config.settings import Neo4jConfig + + with pytest.raises(ValueError, match="exactly one"): + _store(name="s") + with pytest.raises(ValueError, match="exactly one"): + _store( + name="s", + client=FakeMemoryClient(), + settings=MemorySettings(neo4j=Neo4jConfig(password=SecretStr("p"))), + ) + + def test_protocol_attribute_defaults(self) -> None: + store = _store(name="graph", client=FakeMemoryClient()) + + assert store.name == "graph" + assert store.writable is True + assert store.extraction is False + assert store.max_search_results is None + assert store.description is not None and "graph" in store.description.lower() + assert store.graph_tools is True + + def test_protocol_fields_are_all_assigned_onto_the_store(self) -> None: + """``MemoryStore``'s protocol requires name/description/max_search_results/ + writable/extraction as instance attributes. A future rename of one of + these on ``Neo4jMemoryStoreConfig`` that forgot the matching + ``self.x = config.x`` line in ``__init__`` would silently drop a + protocol attribute — this pins all five down against the config.""" + from neo4j_agent_memory.integrations.strands import Neo4jMemoryStore, Neo4jMemoryStoreConfig + + config = Neo4jMemoryStoreConfig( + name="graph", + client=FakeMemoryClient(), + description="a custom description", + max_search_results=9, + writable=False, + extraction=True, + ) + store = Neo4jMemoryStore(config) + + assert store.name == config.name + assert store.description == config.description + assert store.max_search_results == config.max_search_results + assert store.writable == config.writable + assert store.extraction == config.extraction + + def test_settings_construction_owns_the_client(self) -> None: + """Settings-constructed stores build and own a real MemoryClient. + + MemoryClient.__init__ is lazy (no connection, no embedder until + .connect()), so this is cheap and does not touch the network. + """ + from pydantic import SecretStr + + from neo4j_agent_memory import MemoryClient, MemorySettings + from neo4j_agent_memory.config.settings import Neo4jConfig + + store = _store( + name="graph", settings=MemorySettings(neo4j=Neo4jConfig(password=SecretStr("p"))) + ) + + assert store._owns_client is True + assert isinstance(store._client, MemoryClient) + + +class TestLifecycle: + @pytest.mark.asyncio + async def test_initialize_connects_an_owned_client_only(self) -> None: + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + + assert client.connect_calls == 1 + await store.aclose() + assert client.close_calls == 0 # borrowed client stays open + + @pytest.mark.asyncio + async def test_initialize_is_idempotent(self) -> None: + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + await store.initialize() + + assert client.connect_calls == 1 + + @pytest.mark.asyncio + async def test_context_manager_closes_an_owned_client(self) -> None: + """Ownership must come from the constructor (settings=), not a monkey-patched flag.""" + from unittest.mock import AsyncMock + + from pydantic import SecretStr + + from neo4j_agent_memory import MemorySettings + from neo4j_agent_memory.config.settings import Neo4jConfig + + store = _store( + name="graph", settings=MemorySettings(neo4j=Neo4jConfig(password=SecretStr("p"))) + ) + store._client.connect = AsyncMock() # type: ignore[method-assign] + store._client.close = AsyncMock() # type: ignore[method-assign] + + async with store: + pass + + store._client.close.assert_awaited_once() + + @pytest.mark.asyncio + async def test_aclose_resets_initialization_so_reentry_reconnects(self) -> None: + """Re-entering the context manager must reconnect, not skip on a stale flag. + + Ownership comes from the constructor (``settings=``) as it must; only + the transport underneath is swapped for a call-counting fake, so the + store still closes on exit and has to reconnect on re-entry. + """ + from pydantic import SecretStr + + from neo4j_agent_memory import MemorySettings + from neo4j_agent_memory.config.settings import Neo4jConfig + + store = _store( + name="graph", settings=MemorySettings(neo4j=Neo4jConfig(password=SecretStr("p"))) + ) + client = FakeMemoryClient() + store._client = client # type: ignore[assignment] + + async with store: + pass + async with store: + pass + + assert client.connect_calls == 2 + assert client.close_calls == 2 + + +class TestEventLoopRebinding: + """Strands' synchronous entry points drive each call on a fresh loop. + + ``Agent.__init__`` runs ``initialize()`` through + ``strands._async.run_async`` (``asyncio.run`` in a throwaway thread) and + every ``Agent.__call__`` uses a *different* loop. The neo4j driver and + the NAMS transport bind to the loop that opened them, so the store has + to notice the change. See ``tests/integration/`` for the same scenario + end-to-end against a real Neo4j. + """ + + def test_a_borrowed_client_raises_a_named_error_on_a_new_loop(self) -> None: + """A client we were handed is not ours to close or reconnect — so: raise. + + The error has to name the problem and both remedies; the alternative + is an opaque ``RuntimeError`` from inside the neo4j driver. + """ + from strands._async import run_async + + client = FakeMemoryClient() + store = _store(name="graph", client=client) + + run_async(store.initialize) + + with pytest.raises(RuntimeError, match="different event loop") as raised: + run_async(store.initialize) + + message = str(raised.value) + assert "settings=" in message and "client=" in message + # Untouched: neither closed nor reconnected behind the owner's back. + assert client.close_calls == 0 + assert client.connect_calls == 1 + + def test_a_borrowed_client_still_raises_after_aclose(self) -> None: + """``aclose()`` leaves a borrowed client connected, so the guard must hold. + + Clearing the recorded loop here would send the next ``initialize()`` + down the already-connected path, which records the new loop and lets + the driver raise its opaque error instead of this named one. + """ + from strands._async import run_async + + client = FakeMemoryClient() + store = _store(name="graph", client=client) + + run_async(store.initialize) + run_async(store.aclose) + + with pytest.raises(RuntimeError, match="different event loop"): + run_async(store.initialize) + + assert client.close_calls == 0 + + def test_a_tool_call_goes_through_the_store_lifecycle(self) -> None: + """Tools must not bypass ``initialize()``. + + With injection disabled a tool call can be the store's first + operation on a new loop; a tool holding the client directly would + drive a stale transport instead of taking the rebind path. + """ + from strands._async import run_async + + client = FakeMemoryClient() + store = _store(name="graph", client=client) + tool = next(t for t in store.get_tools() if t.tool_name.endswith("get_entity_graph")) + + run_async(store.initialize) + + use = {"toolUseId": "t1", "name": tool.tool_name, "input": {"entity_name": "Acme"}} + events: list[Any] = [] + + async def call() -> None: + async for event in tool.stream(use, {}): + events.append(event) + + run_async(call) + + # Strands turns a tool exception into an error result rather than + # propagating it, so assert on the result the agent would see. + result = events[-1]["tool_result"] + assert result["status"] == "error" + assert "different event loop" in result["content"][0]["text"] + + def test_the_same_loop_stays_idempotent(self) -> None: + """Only a *changed* loop triggers the rebind path.""" + from strands._async import run_async + + client = FakeMemoryClient() + store = _store(name="graph", client=client) + + async def twice() -> None: + await store.initialize() + await store.initialize() + + run_async(twice) + + assert client.connect_calls == 1 + + +class TestForNams: + def test_builds_nams_settings_from_env(self, monkeypatch: pytest.MonkeyPatch) -> None: + from neo4j_agent_memory.integrations.strands import ( + Neo4jMemoryStore, + Neo4jMemoryStoreConfig, + ) + + monkeypatch.setenv("MEMORY_API_KEY", "test-key") + + store = Neo4jMemoryStore.for_nams(Neo4jMemoryStoreConfig(name="graph")) + + settings = store._client._settings + assert settings.backend == "nams" + assert settings.nams.validate_on_connect is False + + def test_raises_without_api_key(self, monkeypatch: pytest.MonkeyPatch) -> None: + from neo4j_agent_memory.integrations.strands import ( + Neo4jMemoryStore, + Neo4jMemoryStoreConfig, + ) + + monkeypatch.delenv("MEMORY_API_KEY", raising=False) + + with pytest.raises(ValueError, match="api_key is required"): + Neo4jMemoryStore.for_nams(Neo4jMemoryStoreConfig(name="graph")) + + def test_does_not_mutate_the_callers_config(self, monkeypatch: pytest.MonkeyPatch) -> None: + """``for_nams`` uses ``dataclasses.replace`` to inject ``settings`` into + a copy, not the caller's own config instance.""" + from neo4j_agent_memory.integrations.strands import ( + Neo4jMemoryStore, + Neo4jMemoryStoreConfig, + ) + + monkeypatch.setenv("MEMORY_API_KEY", "test-key") + original_config = Neo4jMemoryStoreConfig(name="graph") + + Neo4jMemoryStore.for_nams(original_config) + + assert original_config.settings is None + + +class TestSinkResolution: + @pytest.mark.asyncio + async def test_creates_a_deterministically_named_sink(self) -> None: + """Bolt keys conversations by session_id; the deterministic name is the whole contract.""" + + client = FakeMemoryClient() + store = _store(name="graph", client=client, user_id="alice") + await store.initialize() + key = await store._resolve_sink() + + assert key == "strands-memory-store/alice/graph" + assert client.short_term.conversations == {} # nothing minted + assert client.short_term.list_conversations_calls == [] + + @pytest.mark.asyncio + async def test_reuses_an_existing_sink_across_instances(self) -> None: + """Bolt needs no round-trip for reuse: same name, same key, every time.""" + + client = FakeMemoryClient() + first = _store(name="graph", client=client) + await first.initialize() + key_one = await first._resolve_sink() + + second = _store(name="graph", client=client) + await second.initialize() + key_two = await second._resolve_sink() + + assert key_one == key_two + assert client.short_term.conversations == {} + assert client.short_term.list_conversations_calls == [] + + @pytest.mark.asyncio + async def test_reuses_the_nams_server_minted_id_by_metadata(self) -> None: + """NAMS mints conversation ids, so reuse matches on metadata, not id. + + The reuse contract spans two round-trips, so it is asserted across + them: the first store's ``POST /conversations`` body is fed back as + the second store's ``GET /conversations`` listing. That pins the + invariant that actually matters -- the metadata the store *writes* + is the metadata it later *matches on* -- rather than trusting a fake + server to echo it. + """ + + client = FakeMemoryClient(nams_mode=True) + first = _store(name="graph", client=client) + await first.initialize() + key_one = await first._resolve_sink() + + created = client.wire.last("create_conversation") + assert created.json["metadata"]["strands_memory_store"] == "strands-memory-store/_/graph" + client.wire.responses["list_conversations"] = { + "conversations": [{"id": key_one, "metadata": created.json["metadata"]}] + } + + second = _store(name="graph", client=client) + await second.initialize() + key_two = await second._resolve_sink() + + assert key_one == key_two + assert key_one != "strands-memory-store/_/graph" # the cached key is the minted uuid + # Exactly one conversation was ever created: the second store matched. + assert len(client.wire.calls_for("create_conversation")) == 1 + + @pytest.mark.asyncio + async def test_nams_scans_conversations_once(self) -> None: + """Symmetric to the bolt no-scan case: NAMS needs exactly one list round-trip.""" + + client = FakeMemoryClient(nams_mode=True) + store = _store(name="graph", client=client) + await store.initialize() + await store._resolve_sink() + + assert len(client.wire.calls_for("list_conversations")) == 1 + + @pytest.mark.asyncio + async def test_explicit_conversation_id_is_used_verbatim(self) -> None: + client = FakeMemoryClient() + store = _store(name="graph", client=client, conversation_id="chat-42") + await store.initialize() + + assert await store._resolve_sink() == "chat-42" + assert client.short_term.conversations == {} # nothing minted + + @pytest.mark.asyncio + async def test_two_stores_with_different_names_get_different_sinks(self) -> None: + client = FakeMemoryClient() + personal = _store(name="personal", client=client) + team = _store(name="team", client=client) + await personal.initialize() + await team.initialize() + + assert await personal._resolve_sink() != await team._resolve_sink() + + @pytest.mark.asyncio + async def test_bolt_does_not_scan_conversations(self) -> None: + """On bolt the deterministic name is the key; a list scan would be wasted work.""" + + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + await store._resolve_sink() + + assert client.short_term.list_conversations_calls == [] + + +class TestSearch: + @pytest.mark.asyncio + async def test_returns_memory_entries_with_metadata(self) -> None: + from neo4j_agent_memory.memory.long_term import Entity, Preference + + client = FakeMemoryClient() + entity = Entity(name="Acme Corp", type="ORGANIZATION") + entity.metadata["similarity"] = 0.9 + client.long_term.entities = [entity] + client.long_term.preferences = [Preference(category="ui", preference="dark mode")] + + store = _store(name="graph", client=client) + await store.initialize() + entries = await store.search("acme") + + assert [e.content for e in entries] == [ + "[entity] Acme Corp (ORGANIZATION)", + "[preference] ui: dark mode", + ] + assert entries[0].metadata is not None + assert entries[0].metadata["kind"] == "entity" + assert entries[0].metadata["score"] == 0.9 + + @pytest.mark.asyncio + async def test_limit_precedence_call_then_store_then_default(self) -> None: + client = FakeMemoryClient() + store = _store(name="graph", client=client, include_preferences=False, include_facts=False) + await store.initialize() + + await store.search("q") + assert client.long_term.search_kwargs[-1]["limit"] == 3 # protocol default + + store.max_search_results = 7 + await store.search("q") + assert client.long_term.search_kwargs[-1]["limit"] == 7 + + await store.search("q", {"max_search_results": 2}) + assert client.long_term.search_kwargs[-1]["limit"] == 2 + + # Explicit 0 must stay 0, not fall through to a truthiness check. + await store.search("q", {"max_search_results": 0}) + assert client.long_term.search_kwargs[-1]["limit"] == 0 + + @pytest.mark.asyncio + async def test_the_limit_caps_the_total_and_no_kind_is_starved(self) -> None: + """Entities alone could fill the limit; a preference must still get through. + + Strands treats a store's row count as the per-store cap and then + slices the concatenation to the same number, so returning ``limit`` + rows *per kind* meant five entity hits pushed every preference and + fact out of the injected block — starving exactly what a POLE graph + is for. + """ + from neo4j_agent_memory.memory.long_term import Entity, Fact, Preference + + client = FakeMemoryClient() + client.long_term.entities = [Entity(name=f"E{i}", type="OBJECT") for i in range(10)] + client.long_term.preferences = [Preference(category="ui", preference="dark mode")] + client.long_term.facts = [Fact(subject="Ada", predicate="uses", object="Python")] + + store = _store(name="graph", client=client, max_search_results=5) + await store.initialize() + entries = await store.search("q") + + kinds = [e.metadata["kind"] for e in entries if e.metadata] + assert len(entries) == 5, "the limit caps the total, not each kind" + assert kinds.count("preference") == 1 + assert kinds.count("fact") == 1 + assert kinds.count("entity") == 3, "unused capacity flows to the kind with more hits" + + @pytest.mark.asyncio + async def test_a_lone_kind_may_use_the_whole_budget(self) -> None: + """Round-robin must not reserve capacity for kinds that returned nothing.""" + from neo4j_agent_memory.memory.long_term import Entity + + client = FakeMemoryClient() + client.long_term.entities = [Entity(name=f"E{i}", type="OBJECT") for i in range(10)] + + store = _store(name="graph", client=client, max_search_results=4) + await store.initialize() + entries = await store.search("q") + + assert len(entries) == 4 + + @pytest.mark.asyncio + async def test_kind_flags_are_honoured(self) -> None: + """All three kinds have data; only the enabled one should ever be searched.""" + from neo4j_agent_memory.memory.long_term import Entity, Fact, Preference + + client = FakeMemoryClient() + client.long_term.entities = [Entity(name="Acme Corp", type="ORGANIZATION")] + client.long_term.preferences = [Preference(category="ui", preference="dark mode")] + client.long_term.facts = [Fact(subject="Acme", predicate="located_in", object="NYC")] + + store = _store( + name="graph", + client=client, + include_entities=False, + include_preferences=True, + include_facts=False, + ) + await store.initialize() + entries = await store.search("q") + + assert len(entries) == 1 + assert entries[0].metadata is not None + assert entries[0].metadata["kind"] == "preference" + # Proves entities/facts were never searched, not merely that they + # returned nothing (a dropped include_* kwarg would sail through + # on len(entries) == 1 alone). + assert client.long_term.search_calls == 1 + + @pytest.mark.asyncio + async def test_nams_returns_entities_only(self) -> None: + """NAMS has entity search only, so that is the only request made. + + The real ``NamsLongTermMemory.search_preferences`` / + ``search_facts`` raise ``NotSupportedError``, and ``_retrieve_entries`` + swallows per-kind failures -- so "only entities came back" alone would + pass even if they had been called. The recorded call list is what + proves they were skipped. + """ + + client = FakeMemoryClient(nams_mode=True) + client.wire.responses["search_entities"] = { + "entities": [{"id": _ENTITY_ID, "name": "Acme Corp", "type": "organization"}], + "searchType": "vector", + } + + store = _store(name="graph", client=client) + await store.initialize() + entries = await store.search("q") + + assert len(entries) == 1 + assert entries[0].metadata is not None + assert entries[0].metadata["kind"] == "entity" + assert client.wire.methods == ["search_entities"] + + @pytest.mark.asyncio + async def test_search_does_not_mint_a_sink(self) -> None: + """Reads are conversation-independent; only writes need the sink. + + Must use nams_mode=True: on bolt, _resolve_sink() makes no backend + call at all (it just sets self._sink_key locally), so an accidental + _resolve_sink() call from search() would be invisible there. NAMS + genuinely calls list_conversations (and possibly create_conversation), + so this is the only client mode that can catch that regression. + """ + + client = FakeMemoryClient(nams_mode=True) + store = _store(name="graph", client=client) + await store.initialize() + await store.search("q") + + assert client.wire.methods == ["search_entities"] # no list, no create + + @pytest.mark.asyncio + async def test_search_initializes_without_a_prior_initialize_call(self) -> None: + """Standalone use (no MemoryManager.init_agent) must still connect.""" + + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.search("q") + + assert client.connect_calls == 1 + + +class TestAdd: + @pytest.mark.asyncio + async def test_default_writes_a_message_into_the_sink_with_extraction(self) -> None: + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + result = await store.add("The user prefers dark mode") + + call = client.short_term.add_message_calls[-1] + assert call["content"] == "The user prefers dark mode" + assert call["role"] == "user" + assert call["extract_entities"] is True + assert result["kind"] == "message" + + @pytest.mark.asyncio + async def test_kind_preference_routes_to_add_preference(self) -> None: + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + result = await store.add("dark mode", {"kind": "preference", "category": "ui"}) + + assert client.long_term.added_preferences == [("ui", "dark mode")] + assert result["kind"] == "preference" + assert client.short_term.add_message_calls == [] + + @pytest.mark.asyncio + async def test_kind_preference_without_category_defaults_to_memory(self) -> None: + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + await store.add("dark mode", {"kind": "preference"}) + + assert client.long_term.added_preferences == [("memory", "dark mode")] + + @pytest.mark.asyncio + async def test_kind_fact_requires_a_triple(self) -> None: + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + + await store.add( + "Ada works at Acme", + {"kind": "fact", "subject": "Ada", "predicate": "works_at", "object": "Acme"}, + ) + assert client.long_term.added_facts == [("Ada", "works_at", "Acme")] + + with pytest.raises(ValueError, match="subject.*predicate.*object"): + await store.add("Ada works at Acme", {"kind": "fact"}) + + @pytest.mark.asyncio + async def test_kind_entity_routes_to_add_entity(self) -> None: + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + await store.add("Acme Corp", {"kind": "entity", "type": "ORGANIZATION"}) + + assert client.long_term.added_entities == [("Acme Corp", "ORGANIZATION")] + assert client.short_term.add_message_calls == [] + + @pytest.mark.asyncio + async def test_kind_entity_defaults_name_and_type(self) -> None: + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + await store.add("Acme Corp", {"kind": "entity"}) + + assert client.long_term.added_entities == [("Acme Corp", "OBJECT")] + + @pytest.mark.asyncio + async def test_kind_entity_on_nams_returns_bare_entity_id(self) -> None: + """NAMS add_entity returns a bare Entity, not a (Entity, Dedup) tuple. + + Without the isinstance narrowing in `_add_typed`, this raises trying + to subscript a bare Entity as a tuple. + + Note the POLE+O type is *not* what reaches the server: + ``NamsLongTermMemory`` maps it into NAMS' own lowercase vocabulary + (``ORGANIZATION`` -> ``organization``) and drops everything NAMS' + create body has no field for. + """ + + client = FakeMemoryClient(nams_mode=True) + store = _store(name="graph", client=client) + await store.initialize() + result = await store.add("Acme Corp", {"kind": "entity", "type": "ORGANIZATION"}) + + assert client.wire.last("add_entity").json == {"name": "Acme Corp", "type": "organization"} + assert result["kind"] == "entity" + assert result["id"] + + @pytest.mark.asyncio + async def test_unsupported_kind_on_nams_falls_back_to_the_sink(self, caplog) -> None: + client = FakeMemoryClient(nams_mode=True) + store = _store(name="graph", client=client) + await store.initialize() + result = await store.add("dark mode", {"kind": "preference", "category": "ui"}) + + assert result["kind"] == "message" + # NAMS accepts only {content, role} on a message: the sink write carries + # no category, no user identifier, no extraction flag. + assert client.wire.last("add_message").json == {"content": "dark mode", "role": "user"} + assert "falling back" in caplog.text.lower() + + @pytest.mark.asyncio + async def test_unsupported_kind_warning_is_logged_once_per_store(self, caplog) -> None: + client = FakeMemoryClient(nams_mode=True) + store = _store(name="graph", client=client) + await store.initialize() + + with caplog.at_level("WARNING"): + await store.add("dark mode", {"kind": "preference", "category": "ui"}) + await store.add("another one", {"kind": "preference", "category": "ui"}) + + warnings = [r for r in caplog.records if "unsupported on this backend" in r.message] + assert len(warnings) == 1 + assert len(client.wire.calls_for("add_message")) == 2 + + @pytest.mark.asyncio + async def test_rejects_writes_when_not_writable(self) -> None: + store = _store(name="graph", client=FakeMemoryClient(), writable=False) + await store.initialize() + + with pytest.raises(ValueError, match="not writable"): + await store.add("anything") + + @pytest.mark.asyncio + async def test_rejects_empty_content(self) -> None: + store = _store(name="graph", client=FakeMemoryClient()) + await store.initialize() + + with pytest.raises(ValueError, match="empty"): + await store.add(" ") + + +class TestAddMessages: + @pytest.mark.asyncio + async def test_writes_the_batch_to_the_sink_with_extraction(self) -> None: + from strands.memory import AddMessagesContext + + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + + result = await store.add_messages( + [ + {"role": "user", "content": [{"text": "I prefer dark mode"}]}, + {"role": "assistant", "content": [{"text": "Noted"}]}, + ], + AddMessagesContext(sequence_numbers=[0, 1]), + ) + + call = client.short_term.bulk_calls[-1] + assert call["kwargs"]["extract_entities"] is True + assert [m["content"] for m in call["messages"]] == ["I prefer dark mode", "Noted"] + assert [m["role"] for m in call["messages"]] == ["user", "assistant"] + assert result == {"written": 2, "skipped": 0} + + @pytest.mark.asyncio + async def test_a_retried_batch_is_not_written_twice(self) -> None: + """Extraction writes are at-least-once; the same sequence numbers repeat.""" + from strands.memory import AddMessagesContext + + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + batch = [{"role": "user", "content": [{"text": "ok"}]}] + + first = await store.add_messages(batch, AddMessagesContext(sequence_numbers=[0])) + second = await store.add_messages(batch, AddMessagesContext(sequence_numbers=[0])) + + assert first == {"written": 1, "skipped": 0} + assert second == {"written": 0, "skipped": 1} + assert len(client.short_term.bulk_calls) == 1 + + @pytest.mark.asyncio + async def test_identical_text_with_distinct_sequence_numbers_is_kept(self) -> None: + """Dedupe keys on sequence number, not content — two 'ok's are two messages.""" + from strands.memory import AddMessagesContext + + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + + result = await store.add_messages( + [ + {"role": "user", "content": [{"text": "ok"}]}, + {"role": "user", "content": [{"text": "ok"}]}, + ], + AddMessagesContext(sequence_numbers=[3, 4]), + ) + + assert result == {"written": 2, "skipped": 0} + + @pytest.mark.asyncio + async def test_without_sequence_numbers_everything_is_written(self) -> None: + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + batch = [{"role": "user", "content": [{"text": "ok"}]}] + + assert await store.add_messages(batch, None) == {"written": 1, "skipped": 0} + assert await store.add_messages(batch, None) == {"written": 1, "skipped": 0} + + @pytest.mark.asyncio + async def test_messages_with_no_text_blocks_are_dropped(self) -> None: + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + + result = await store.add_messages( + [{"role": "assistant", "content": [{"toolUse": {"name": "x", "input": {}}}]}], + None, + ) + + assert result == {"written": 0, "skipped": 1} + assert client.short_term.bulk_calls == [] + + @pytest.mark.asyncio + async def test_batches_larger_than_100_are_chunked(self) -> None: + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + + messages = [{"role": "user", "content": [{"text": f"m{i}"}]} for i in range(250)] + result = await store.add_messages(messages, None) + + assert result == {"written": 250, "skipped": 0} + assert [len(c["messages"]) for c in client.short_term.bulk_calls] == [100, 100, 50] + + @pytest.mark.asyncio + async def test_rejects_writes_when_not_writable(self) -> None: + store = _store(name="graph", client=FakeMemoryClient(), writable=False) + await store.initialize() + + with pytest.raises(ValueError, match="not writable"): + await store.add_messages([{"role": "user", "content": [{"text": "x"}]}], None) + + @pytest.mark.asyncio + async def test_bulk_kwargs_bind_against_the_real_bolt_signature(self) -> None: + """Regression guard for the ``user_identifier`` crash-on-bolt bug. + + The bug: the store passed ``user_identifier`` to ``bulk_add_messages`` + while the real ``ShortTermMemory.add_messages_batch`` had no such + parameter and no ``**kwargs`` catch-all, so bolt raised + ``TypeError: got an unexpected keyword argument 'user_identifier'``. + Fixed by adding the parameter to ``add_messages_batch`` itself (it now + enforces multi-tenancy and links the conversation, matching + ``add_message``), so the store keeps passing it. + + ``FakeShortTerm.bulk_add_messages`` now mirrors the real, explicit + parameter list (no ``**kwargs`` catch-all either), so it can no longer + swallow a keyword the real backend would reject — this suite would + have failed with the bug in place, and stays a live tripwire against + any future kwarg the store sends that the real signature doesn't + accept. Binding against the actual method's ``inspect.signature`` is + the belt-and-suspenders check on top of that fidelity fix. + """ + import inspect + + from neo4j_agent_memory.memory.short_term import ShortTermMemory + + client = FakeMemoryClient() + store = _store(name="graph", client=client, user_id="alice") + await store.initialize() + await store.add_messages([{"role": "user", "content": [{"text": "ok"}]}], None) + + call = client.short_term.bulk_calls[-1] + real_signature = inspect.signature(ShortTermMemory.add_messages_batch) + # `self` is positional-only for bind() purposes here; its value is + # never inspected, only its presence in the parameter list matters. + real_signature.bind(None, call["session_id"], call["messages"], **call["kwargs"]) + + +class TestPreferenceTenancy: + """A user-scoped store must not recall another tenant's preferences. + + ``search_preferences`` takes no user identifier and applies no ``:User`` + filter, so using it on a scoped store injects whatever the vector index + ranks highest — including preferences belonging to somebody else. + ``get_preferences_for`` is the only user-scoped primitive available. + """ + + @pytest.mark.asyncio + async def test_a_scoped_store_recalls_only_its_own_users_preferences(self) -> None: + client = FakeMemoryClient() + await client.long_term.add_preference("ui", "alice: dark mode", user_identifier="alice") + await client.long_term.add_preference("ui", "bob: light mode", user_identifier="bob") + + store = _store( + name="graph", + client=client, + user_id="alice", + include_entities=False, + include_facts=False, + ) + await store.initialize() + entries = await store.search("theme") + + assert [e.content for e in entries] == ["[preference] ui: alice: dark mode"] + assert client.long_term.preferences_for_calls == [ + {"user_identifier": "alice", "active_only": True} + ] + + @pytest.mark.asyncio + async def test_an_unscoped_preference_is_invisible_to_a_scoped_store(self) -> None: + """The leak, stated as a test: an unscoped write has no ``:User`` edge.""" + from neo4j_agent_memory.memory.long_term import Preference + + client = FakeMemoryClient() + # What search_preferences would have returned regardless of tenancy. + client.long_term.preferences = [Preference(category="ui", preference="somebody's theme")] + + store = _store( + name="graph", + client=client, + user_id="alice", + include_entities=False, + include_facts=False, + ) + await store.initialize() + + assert await store.search("theme") == [] + + @pytest.mark.asyncio + async def test_an_unscoped_store_still_searches(self) -> None: + """Without a ``user_id`` there is no tenant to scope to, so search stands.""" + from neo4j_agent_memory.memory.long_term import Preference + + client = FakeMemoryClient() + client.long_term.preferences = [Preference(category="ui", preference="dark mode")] + + store = _store(name="graph", client=client, include_entities=False, include_facts=False) + await store.initialize() + entries = await store.search("theme") + + assert [e.content for e in entries] == ["[preference] ui: dark mode"] + assert client.long_term.preferences_for_calls == [] + + +class TestPartialBatchRetry: + """A batch that fails partway must not re-write the chunks that landed. + + Strands rolls its high-water mark back and retries the whole batch when + ``add_messages`` raises, so the in-process ``_written`` set is what stops + an already-written chunk being written twice. + """ + + @pytest.mark.asyncio + async def test_a_chunk_that_landed_is_not_written_again_by_the_retry(self) -> None: + from strands.memory import AddMessagesContext + + client = FakeMemoryClient() + store = _store(name="graph", client=client) + await store.initialize() + + # 150 messages -> two chunks (_BULK_CHUNK == 100); the second fails. + messages = [{"role": "user", "content": [{"text": f"m{i}"}]} for i in range(150)] + context = AddMessagesContext(sequence_numbers=list(range(150))) + client.short_term.fail_bulk_from = 2 + + with pytest.raises(RuntimeError, match="bulk write failed"): + await store.add_messages(messages, context) + + assert len(client.short_term.bulk_calls) == 2 + first_chunk_texts = [m["content"] for m in client.short_term.bulk_calls[0]["messages"]] + assert first_chunk_texts == [f"m{i}" for i in range(100)] + + # The retry: same batch, same sequence numbers, now succeeding. + client.short_term.fail_bulk_from = None + result = await store.add_messages(messages, context) + + assert result == {"written": 50, "skipped": 100} + retried = [m["content"] for m in client.short_term.bulk_calls[2]["messages"]] + assert retried == [f"m{i}" for i in range(100, 150)] + + +class TestWriteSinks: + def test_declares_both_write_sinks(self) -> None: + """Both sinks on one class: server-side extraction, `add` still available.""" + from strands.memory.types import _has_method, _has_write_sink + + store = _store(name="graph", client=FakeMemoryClient()) + + assert _has_method(store, "add") is True + assert _has_method(store, "add_messages") is True + assert _has_write_sink(store) is True + + def test_server_side_extraction_is_the_resolved_default(self) -> None: + """`add_messages` present -> no ModelExtractor, so no extra model call. + + Non-vacuous only here: with `add` alone (task 7) this would resolve to a + ModelExtractor, and with neither sink it resolves to None trivially. + """ + from strands.memory.extraction.resolve_extraction_config import ( + _resolve_extraction_config, + ) + + store = _store(name="graph", client=FakeMemoryClient(), extraction=True) + resolved = _resolve_extraction_config(store.extraction, store) + + assert resolved is not None + assert resolved.extractor is None + + +class TestGetTools: + def test_bolt_exposes_both_graph_tools(self) -> None: + store = _store(name="graph", client=FakeMemoryClient(), user_id="alice") + names = {t.tool_name for t in store.get_tools()} + + assert names == {"graph_get_entity_graph", "graph_get_user_preferences"} + + def test_bolt_omits_preferences_tool_without_a_user_id(self) -> None: + """get_preferences_for requires a user identifier; with none, no tool.""" + store = _store(name="graph", client=FakeMemoryClient()) + names = {t.tool_name for t in store.get_tools()} + + assert names == {"graph_get_entity_graph"} + + def test_nams_omits_the_preferences_tool(self) -> None: + """NAMS exposes no preferences endpoint; expand_graph covers traversal.""" + store = _store(name="graph", client=FakeMemoryClient(nams_mode=True), user_id="alice") + names = {t.tool_name for t in store.get_tools()} + + assert names == {"graph_get_entity_graph"} + + def test_graph_tools_false_exposes_nothing(self) -> None: + store = _store(name="graph", client=FakeMemoryClient(), graph_tools=False) + + assert store.get_tools() == [] + + @pytest.mark.asyncio + async def test_entity_graph_traverses_with_depth_on_bolt(self) -> None: + from neo4j_agent_memory.integrations.strands._store_tools import _entity_graph + from neo4j_agent_memory.memory.long_term import Entity + + client = FakeMemoryClient() + centre = Entity(name="Acme Corp", type="ORGANIZATION") + client.long_term.entities = [centre] + client.long_term.related = [Entity(name="Ada", type="PERSON")] + + result = await _entity_graph(client, "Acme Corp", depth=2, nams=False) + + assert result["center"] == "Acme Corp" + assert {"name": "Ada", "type": "PERSON", "is_center": False} in result["nodes"] + # "RELATED_TO", centre-as-source: the only thing the bolt stack can + # report today. get_related_entities reads relationship properties out + # of execute_read's result.data(), which flattens a relationship to + # (start, type, end) and loses them, so the property-level type never + # survives and source_id is hardcoded to the centre. Pre-existing + # library defect, pinned here and in the integration suite so a fix + # shows up as a failing assertion rather than going unnoticed. + assert {"from": "Acme Corp", "relationship": "RELATED_TO", "to": "Ada"} in result["edges"] + assert client.long_term.related_kwargs[-1]["depth"] == 2 + + @pytest.mark.asyncio + async def test_entity_graph_depth_is_clamped_to_three_by_the_tool(self) -> None: + """The @tool wrapper clamps depth to [1, 3]; _entity_graph itself trusts its caller.""" + from neo4j_agent_memory.memory.long_term import Entity + + client = FakeMemoryClient() + centre = Entity(name="Acme Corp", type="ORGANIZATION") + client.long_term.entities = [centre] + + store = _store(name="graph", client=client, user_id="alice") + tools = {t.tool_name: t for t in store.get_tools()} + + await tools["graph_get_entity_graph"](entity_name="Acme Corp", depth=99) + + assert client.long_term.related_kwargs[-1]["depth"] == 3 + + @pytest.mark.asyncio + async def test_entity_graph_caps_edges_at_max_edges_on_bolt(self) -> None: + from neo4j_agent_memory.integrations.strands._store_tools import ( + _MAX_EDGES, + _entity_graph, + ) + from neo4j_agent_memory.memory.long_term import Entity + + client = FakeMemoryClient() + centre = Entity(name="Acme Corp", type="ORGANIZATION") + client.long_term.entities = [centre] + client.long_term.related = [ + Entity(name=f"Person {i}", type="PERSON") for i in range(_MAX_EDGES + 10) + ] + + result = await _entity_graph(client, "Acme Corp", depth=1, nams=False) + + assert len(result["edges"]) == _MAX_EDGES + + @pytest.mark.asyncio + async def test_entity_graph_uses_expand_graph_on_nams(self) -> None: + """NAMS: name resolved via search, then a 1-hop expansion by node id.""" + from neo4j_agent_memory.integrations.strands._store_tools import _entity_graph + + client = FakeMemoryClient(nams_mode=True) + expansion = { + "nodes": [{"id": "n2", "name": "Ada", "type": "PERSON"}], + "edges": [{"from": "n2", "to": _ENTITY_ID, "type": "WORKS_AT"}], + } + client.wire.responses["search_entities"] = { + "entities": [{"id": _ENTITY_ID, "name": "Acme Corp", "type": "organization"}] + } + client.wire.responses["expand_graph"] = expansion + + result = await _entity_graph(client, "Acme Corp", depth=3, nams=True) + + # The searched name goes out as the query; the *id* it resolved to is + # what the expansion is keyed by (NAMS has no expand-by-name). + assert client.wire.last("search_entities").json == {"query": "Acme Corp", "limit": 1} + assert client.wire.last("expand_graph").json == {"nodeId": _ENTITY_ID, "loadedIds": []} + assert result["depth"] == 1 # 1 hop is all NAMS offers + assert result["nodes"] == expansion["nodes"] + assert result["edges"] == expansion["edges"] + + @pytest.mark.asyncio + async def test_entity_graph_reports_an_unknown_entity(self) -> None: + from neo4j_agent_memory.integrations.strands._store_tools import _entity_graph + + client = FakeMemoryClient() + client.long_term.entities = [] + + result = await _entity_graph(client, "Nobody", depth=1, nams=False) + + assert result["error"] == "entity not found: Nobody" + + @pytest.mark.asyncio + async def test_get_user_preferences_forwards_the_stores_user_id(self) -> None: + """Regression guard: a hard-coded or dropped user id must fail this.""" + client = FakeMemoryClient() + store = _store(name="graph", client=client, user_id="alice") + tools = {t.tool_name: t for t in store.get_tools()} + + await tools["graph_get_user_preferences"]() + + assert client.long_term.preferences_for_calls[-1]["user_identifier"] == "alice" + + @pytest.mark.asyncio + async def test_get_user_preferences_category_filter_narrows_results(self) -> None: + from neo4j_agent_memory.memory.long_term import Preference + + client = FakeMemoryClient() + client.long_term.preferences_for = [ + Preference(category="food", preference="loves sushi"), + Preference(category="ui", preference="dark mode"), + ] + store = _store(name="graph", client=client, user_id="alice") + tools = {t.tool_name: t for t in store.get_tools()} + + result = await tools["graph_get_user_preferences"](category="food") + + assert result == [{"category": "food", "preference": "loves sushi", "context": None}] + + +class TestToolNamespacing: + """The store's tools must not silently replace the factory's. + + ``ToolRegistry.register_tool`` raises on a duplicate name only when the + tool does not support hot reload — and ``@tool``-decorated functions + always do, so both sides' tools take the same code path into + ``registry[name] = tool`` and the later registration wins, with no + warning. Both sides ship ``get_entity_graph`` and + ``get_user_preferences``, and they take different arguments. + """ + + def test_names_are_prefixed_with_the_store_name(self) -> None: + store = _store(name="graph", client=FakeMemoryClient(), user_id="alice") + + names = {t.tool_name for t in store.get_tools()} + + assert names == {"graph_get_entity_graph", "graph_get_user_preferences"} + + def test_no_name_collides_with_the_tools_factory(self) -> None: + from neo4j_agent_memory.integrations.strands import context_graph_tools + + store = _store(name="graph", client=FakeMemoryClient(), user_id="alice") + factory_names = { + t.tool_name + for t in context_graph_tools(neo4j_uri="bolt://localhost:7687", neo4j_password="p") + } + store_names = {t.tool_name for t in store.get_tools()} + + assert factory_names & store_names == set() + # And the names the collision was about are the factory's alone. + assert {"get_entity_graph", "get_user_preferences"} <= factory_names + + def test_two_stores_on_one_manager_get_distinct_names(self) -> None: + """`dataclasses.replace(config, name="team")` is the documented multi-store shape.""" + client = FakeMemoryClient() + personal = _store(name="personal", client=client, user_id="alice") + team = _store(name="team", client=client, user_id="alice") + + personal_names = {t.tool_name for t in personal.get_tools()} + team_names = {t.tool_name for t in team.get_tools()} + + assert personal_names & team_names == set() + + def test_a_name_needing_sanitising_still_yields_a_legal_prefix(self) -> None: + store = _store(name="Team / Graph!", client=FakeMemoryClient()) + + (name,) = {t.tool_name for t in store.get_tools()} + + assert name.startswith("team_graph_") + assert name.endswith("_get_entity_graph") + assert name.replace("_", "").isalnum() + + def test_names_that_sanitise_alike_stay_distinct(self) -> None: + """Sanitisation is many-to-one; the resulting prefixes must not be. + + ``ToolRegistry`` overwrites a duplicate ``@tool`` name silently, so + two stores whose names differ only in punctuation would clobber each + other's tools — the failure namespacing exists to prevent. + """ + client = FakeMemoryClient() + alike = ["team/graph", "team graph", "Team_Graph", "team_graph"] + + prefixes = [ + {t.tool_name for t in _store(name=name, client=client).get_tools()} for name in alike + ] + + flat = [name for names in prefixes for name in names] + assert len(flat) == len(set(flat)) + + +class TestPreferenceRoundTrip: + @pytest.mark.asyncio + async def test_a_written_preference_is_readable_by_the_stores_own_tool(self) -> None: + """``add(kind="preference")`` must be visible to ``get_user_preferences``. + + Both are user-scoped: the write needs ``user_identifier`` for the + ``(:User)-[:HAS_PREFERENCE]`` edge, and ``get_preferences_for`` reads + precisely that edge. Without it the store wrote something its own + tool could never return. + """ + client = FakeMemoryClient() + store = _store(name="graph", client=client, user_id="alice") + await store.initialize() + + await store.add("Prefers dark mode", {"kind": "preference", "category": "ui"}) + + tools = {t.tool_name: t for t in store.get_tools()} + result = await tools["graph_get_user_preferences"]() + + assert [p["preference"] for p in result] == ["Prefers dark mode"] + + @pytest.mark.asyncio + async def test_a_preference_write_is_tenant_scoped(self) -> None: + """Under multi_tenant=True an unscoped write raises ValueError, not NotSupportedError. + + ValueError is not caught by ``add``'s ``NotSupportedError`` fallback, + so it would have surfaced as a hard failure rather than a sink write. + """ + client = FakeMemoryClient() + client.long_term.multi_tenant = True + store = _store(name="graph", client=client, user_id="alice") + await store.initialize() + + await store.add("Prefers dark mode", {"kind": "preference"}) + + assert client.long_term.added_preferences == [("memory", "Prefers dark mode")] + assert client.short_term.add_message_calls == [], "no fallback to the sink" diff --git a/tests/unit/integrations/test_strands_retrieval_entries.py b/tests/unit/integrations/test_strands_retrieval_entries.py new file mode 100644 index 00000000..03b6b14d --- /dev/null +++ b/tests/unit/integrations/test_strands_retrieval_entries.py @@ -0,0 +1,137 @@ +"""_retrieve_entries: concurrent long-term fan-out returning entry rows.""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, cast + +import pytest + +pytest.importorskip("strands", reason="strands-agents not installed") + +from neo4j_agent_memory.memory.long_term import Entity, Fact, Preference + +if TYPE_CHECKING: + from neo4j_agent_memory.core.protocols import LongTermProtocol + + +def _entity() -> Entity: + e = Entity(name="Acme Corp", type="ORGANIZATION") + e.metadata["similarity"] = 0.83 + return e + + +def _preference() -> Preference: + p = Preference(category="ui", preference="dark mode") + p.metadata["similarity"] = 0.71 + return p + + +def _fact() -> Fact: + f = Fact(subject="Ada", predicate="works_at", object="Acme") + f.metadata["similarity"] = 0.55 + return f + + +class TestRetrieveEntries: + @pytest.mark.asyncio + async def test_maps_each_kind_to_a_row_with_metadata(self) -> None: + from neo4j_agent_memory.integrations.strands._retrieval import _retrieve_entries + from tests.unit.integrations.strands_fakes import FakeLongTerm + + long_term = FakeLongTerm() + long_term.entities = [_entity()] + long_term.preferences = [_preference()] + long_term.facts = [_fact()] + + rows = await _retrieve_entries( + cast("LongTermProtocol", long_term), # fake implements only the searched subset + "acme", + limit=10, + min_score=0.2, + include_entities=True, + include_preferences=True, + include_facts=True, + nams=False, + ) + + kinds = [r.metadata["kind"] for r in rows] + assert kinds == ["entity", "preference", "fact"] + assert rows[0].content == "[entity] Acme Corp (ORGANIZATION)" + assert rows[0].metadata["score"] == 0.83 + assert rows[0].metadata["type"] == "ORGANIZATION" + assert rows[1].metadata["type"] == "ui" + assert rows[1].metadata["score"] == 0.71 + assert rows[2].metadata["score"] == 0.55 + assert "id" in rows[0].metadata + + @pytest.mark.asyncio + async def test_nams_gates_preferences_and_facts_off(self) -> None: + from neo4j_agent_memory.integrations.strands._retrieval import _retrieve_entries + from tests.unit.integrations.strands_fakes import FakeLongTerm + + long_term = FakeLongTerm() + long_term.entities = [_entity()] + long_term.preferences = [Preference(category="ui", preference="dark mode")] + + rows = await _retrieve_entries( + cast("LongTermProtocol", long_term), # fake implements only the searched subset + "q", + limit=10, + min_score=0.2, + include_entities=True, + include_preferences=True, + include_facts=True, + nams=True, + ) + + assert [r.metadata["kind"] for r in rows] == ["entity"] + assert long_term.search_calls == 1 # preferences/facts never called + + @pytest.mark.asyncio + async def test_one_failing_kind_does_not_lose_the_others(self, caplog) -> None: + from neo4j_agent_memory.integrations.strands._retrieval import _retrieve_entries + from tests.unit.integrations.strands_fakes import FakeLongTerm + + long_term = FakeLongTerm() + long_term.entities = [_entity()] + long_term.fail_preferences = True + + rows = await _retrieve_entries( + cast("LongTermProtocol", long_term), # fake implements only the searched subset + "q", + limit=10, + min_score=0.2, + include_entities=True, + include_preferences=True, + include_facts=False, + nams=False, + ) + + assert [r.metadata["kind"] for r in rows] == ["entity"] + assert "preference search failed" in caplog.text.lower() + + @pytest.mark.asyncio + async def test_missing_score_is_omitted_not_zero(self) -> None: + from neo4j_agent_memory.integrations.strands._retrieval import _retrieve_entries + from tests.unit.integrations.strands_fakes import FakeLongTerm + + long_term = FakeLongTerm() + long_term.entities = [Entity(name="Acme Corp", type="ORGANIZATION")] # no similarity + long_term.preferences = [Preference(category="ui", preference="dark mode")] # no similarity + long_term.facts = [ + Fact(subject="Ada", predicate="works_at", object="Acme") + ] # no similarity + + rows = await _retrieve_entries( + cast("LongTermProtocol", long_term), # fake implements only the searched subset + "q", + limit=10, + min_score=0.2, + include_entities=True, + include_preferences=True, + include_facts=True, + nams=False, + ) + + assert [r.metadata["kind"] for r in rows] == ["entity", "preference", "fact"] + assert all("score" not in r.metadata for r in rows) diff --git a/tests/unit/integrations/test_strands_session_manager.py b/tests/unit/integrations/test_strands_session_manager.py index 11f8d7e5..6a1950cd 100644 --- a/tests/unit/integrations/test_strands_session_manager.py +++ b/tests/unit/integrations/test_strands_session_manager.py @@ -160,6 +160,9 @@ def test_formatters(self) -> None: from types import SimpleNamespace +#: Entity ids in canned NAMS payloads must parse as UUIDs (``Entity.id: UUID``). +_ENTITY_ID = "11111111-1111-1111-1111-111111111111" + def _make_manager(nams_mode: bool = False, **kwargs): """Build a manager wired to a FakeMemoryClient. Caller must close().""" @@ -213,21 +216,20 @@ def test_bolt_uses_session_id_directly_and_restores_history(self) -> None: manager.close() def test_nams_resolves_existing_conversation_by_metadata(self) -> None: - import asyncio - manager, client = _make_manager(nams_mode=True) + existing_id = "22222222-2222-2222-2222-222222222222" + client.wire.responses["list_conversations"] = { + "conversations": [ + {"id": existing_id, "metadata": {"strands_session_id": "sess-1"}}, + {"id": "33333333-3333-3333-3333-333333333333", "metadata": {}}, + ] + } try: - existing = asyncio.run( - client.short_term.create_conversation( - session_id="sess-1", - metadata={"strands_session_id": "sess-1"}, - ) - ) agent = _fake_agent() manager.initialize(agent) - assert manager._conversation_key == str(existing.id) + assert manager._conversation_key == existing_id # No second conversation was created. - assert len(client.short_term.conversations) == 1 + assert client.wire.calls_for("create_conversation") == [] finally: manager.close() @@ -236,10 +238,19 @@ def test_nams_creates_conversation_when_absent(self) -> None: try: agent = _fake_agent() manager.initialize(agent) - assert len(client.short_term.conversations) == 1 - conv = next(iter(client.short_term.conversations.values())) - assert conv.metadata["strands_session_id"] == "sess-1" - assert manager._conversation_key == str(conv.id) + created = client.wire.last("create_conversation") + # NAMS' create body is {userId?, metadata?} -- session_id and title + # are client-side concepts and never reach the server, so the + # Strands session id survives the round trip only inside metadata. + assert created.json == { + "metadata": {"strands_session_id": "sess-1", "session_type": "AGENT"} + } + assert manager._conversation_key + # The key is the server-minted id from the create response, and the + # subsequent history load is scoped to it. + assert client.wire.last("get_conversation").path_params == { + "conversation_id": manager._conversation_key + } finally: manager.close() @@ -499,6 +510,11 @@ def test_late_redaction_on_nams_warns_and_does_not_raise(self, caplog) -> None: {"role": "user", "content": [{"text": "[REDACTED]"}]}, agent ) assert any("redact" in r.message.lower() for r in caplog.records) + # Nothing was attempted on the wire: the original message stands and + # no second write followed it. (NAMS' real delete_message raises + # NotSupportedError, so a call would surface as an exception here.) + assert [c.method for c in client.wire.calls_for("add_message")] == ["add_message"] + assert client.wire.last("add_message").json["content"] == "secret" finally: manager.close() @@ -657,7 +673,16 @@ def test_injection_is_idempotent_for_same_message(self) -> None: finally: manager.close() - def test_nams_skips_unsupported_preference_and_fact_searches(self) -> None: + def test_nams_skips_unsupported_preference_and_fact_searches(self, caplog) -> None: + """The real NAMS methods raise ``NotSupportedError``; they must not be called. + + ``_retrieve_context`` gathers with ``return_exceptions=True`` and logs + per-kind failures, so a call would not fail the turn -- it would just + log. Hence both checks: the entity line is present, and neither a + preference/fact request nor a "search failed" warning appeared. + """ + import logging + from neo4j_agent_memory.integrations.strands.session_manager import ( Neo4jRetrievalConfig, ) @@ -665,27 +690,18 @@ def test_nams_skips_unsupported_preference_and_fact_searches(self) -> None: manager, client = _make_manager( nams_mode=True, retrieval_config=Neo4jRetrievalConfig(include_facts=True) ) - client.long_term.entities = [ - SimpleNamespace( - display_name="Acme", - type="ORGANIZATION", - full_type="ORGANIZATION", - description=None, - ) - ] - - # Make preference/fact searches behave like real NAMS: raise if called. - async def boom(query, **kwargs): - raise AssertionError("must not be called on NAMS") - - client.long_term.search_preferences = boom - client.long_term.search_facts = boom + client.wire.responses["search_entities"] = { + "entities": [{"id": _ENTITY_ID, "name": "Acme", "type": "organization"}] + } try: manager.initialize(_fake_agent()) message = {"role": "user", "content": [{"text": "Acme?"}]} - manager._inject_context(message) + with caplog.at_level(logging.WARNING): + manager._inject_context(message) text = message["content"][0]["text"] assert "[entity] Acme (ORGANIZATION)" in text # entity search still works + assert client.wire.methods.count("search_entities") == 1 + assert "search failed" not in caplog.text.lower() finally: manager.close() @@ -798,11 +814,50 @@ def test_nams_list_conversations_passes_user_identifier_and_limit(self) -> None: try: agent = _fake_agent() manager.initialize(agent) - # _aresolve_conversation calls list_conversations with scoping kwargs. - calls = client.short_term.list_conversations_calls + # _aresolve_conversation calls list_conversations with scoping kwargs, + # which NAMS receives as the camelCase query string ?userId=&limit=. + calls = client.wire.calls_for("list_conversations") assert len(calls) == 1 - assert calls[0].get("user_identifier") == "alice" - assert calls[0].get("limit") == 1000 + assert calls[0].params == {"userId": "alice", "limit": 1000} + finally: + manager.close() + + +class TestRetrievalInjectionTenancy: + """Injection must respect the manager's ``user_id``. + + ``search_preferences`` has no ``:User`` filter, so a user-scoped session + that searches would fold another tenant's preferences into this user's + turn. Same defect and same fix as ``Neo4jMemoryStore.search``. + """ + + def test_a_scoped_manager_injects_only_its_own_users_preferences(self) -> None: + from neo4j_agent_memory.integrations.strands.session_manager import ( + Neo4jRetrievalConfig, + ) + + manager, client = _make_manager( + user_id="alice", + retrieval_config=Neo4jRetrievalConfig(include_entities=False, include_facts=False), + ) + # What an unscoped search would have surfaced. + client.long_term.preferences = [ + SimpleNamespace(category="style", preference="somebody else's style") + ] + client.long_term.preferences_by_user["alice"] = [ + SimpleNamespace(category="style", preference="concise answers") + ] + try: + manager.initialize(_fake_agent()) + message = {"role": "user", "content": [{"text": "hi"}]} + manager._inject_context(message) + text = message["content"][0]["text"] + + assert "[preference] style: concise answers" in text + assert "somebody else's style" not in text + assert client.long_term.preferences_for_calls == [ + {"user_identifier": "alice", "active_only": True} + ] finally: manager.close() diff --git a/uv.lock b/uv.lock index fffe1a5e..5973e328 100644 --- a/uv.lock +++ b/uv.lock @@ -4780,7 +4780,7 @@ requires-dist = [ { name = "sentence-transformers", marker = "extra == 'sentence-transformers'", specifier = ">=2.2.0" }, { name = "spacy", marker = "extra == 'extraction'", specifier = ">=3.7.0" }, { name = "spacy", marker = "extra == 'spacy'", specifier = ">=3.7.0" }, - { name = "strands-agents", marker = "extra == 'strands'", specifier = ">=0.1.0" }, + { name = "strands-agents", marker = "extra == 'strands'", specifier = ">=1.44.0" }, { name = "typing-extensions", specifier = ">=4.4" }, ] provides-extras = ["openai", "anthropic", "sentence-transformers", "vertex-ai", "bedrock", "litellm", "instructor", "google", "aws", "strands", "spacy", "gliner", "extraction", "fuzzy", "cli", "opentelemetry", "opik", "observability", "langchain", "llamaindex", "pydantic-ai", "crewai", "openai-agents", "microsoft-agent", "mcp", "google-adk", "nams", "all", "full"] @@ -8214,24 +8214,26 @@ wheels = [ [[package]] name = "strands-agents" -version = "1.23.0" +version = "1.52.0" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "boto3" }, { name = "botocore" }, { name = "docstring-parser" }, + { name = "httpx" }, { name = "jsonschema" }, { name = "mcp" }, { name = "opentelemetry-api" }, { name = "opentelemetry-instrumentation-threading" }, { name = "opentelemetry-sdk" }, { name = "pydantic" }, + { name = "pyyaml" }, { name = "typing-extensions" }, { name = "watchdog" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/bb/e6/27e7dad580b0a91430e67c667c1e0abd9beb363e4f2830d361ff46a6af7e/strands_agents-1.23.0.tar.gz", hash = "sha256:85b528218b3bdf2629dd64926ec0a7dd1ce781e5466693310a9f55b67a41c3f9", size = 709108, upload-time = "2026-01-21T20:15:06.413Z" } +sdist = { url = "https://files.pythonhosted.org/packages/92/f5/7bcd5018b80dcc9eb70bb315ad42d0e8e1607473689a246bdbb763433f5a/strands_agents-1.52.0.tar.gz", hash = "sha256:e79f08527cf3b03ace3ead081f71ee43ee33de7285a4faa9ecafaef4618afeb0", size = 1333918, upload-time = "2026-08-12T18:52:03.743Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/8a/64/722a164f0bb4351da6918594496d831295136bba1be8a76caceb4fac4d6c/strands_agents-1.23.0-py3-none-any.whl", hash = "sha256:76cb65e7c667d06334940843a5c4fd56baab7b11f5d9997ab8c250dcc3c1b403", size = 335387, upload-time = "2026-01-21T20:15:04.443Z" }, + { url = "https://files.pythonhosted.org/packages/32/28/6b1733dfb50ccf2b250d892ff9fdb0c987d0486febaf1305c5b6a904154b/strands_agents-1.52.0-py3-none-any.whl", hash = "sha256:768ff0e865df9f18154a2111f71e0c7683b8505e897704b89981cf7f11667bee", size = 687868, upload-time = "2026-08-12T18:52:01.538Z" }, ] [[package]]