diff --git a/README.md b/README.md index 62faa82..9655be2 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ better-memory has two storage backends. Pick one: | **`sqlite`** (default) | Single-machine usage; full offline operation; no cloud cost. | None — works out of the box. | | **`agentcore`** | Multi-machine syncing; managed extraction by AWS; team-shared memory bucket. | Requires an AWS account with Bedrock AgentCore Memory available in your chosen region (`init --region` defaults to `eu-west-2`). See [AgentCore setup](website/agentcore-setup.md). | -Switching to agentcore is done by `better-memory agentcore init`, which provisions the AWS memories and activates the backend by writing `{"storage_backend": "agentcore"}` to `$BETTER_MEMORY_HOME/settings.json`. The MCP server, hooks, and CLI all resolve the backend the same way: `BETTER_MEMORY_STORAGE_BACKEND` env var if set (always wins), else `settings.json`, else `sqlite`. To revert, remove the `storage_backend` key from `settings.json` or set the env var to `sqlite`. Switching is one-way today — there is no bulk migration tool (deferred; clean start in agentcore mode is the supported path). +Switching to agentcore is done by `better-memory agentcore init`, which provisions the AWS memories and activates the backend by writing `{"storage_backend": "agentcore"}` to `$BETTER_MEMORY_HOME/settings.json`. The MCP server, hooks, and CLI all resolve the backend the same way: `BETTER_MEMORY_STORAGE_BACKEND` env var if set (always wins), else `settings.json`, else `sqlite`. To revert, remove the `storage_backend` key from `settings.json` or set the env var to `sqlite`. To carry existing sqlite memory across, `better-memory agentcore migrate` bulk-copies your distilled reflections and semantic memories into the AgentCore memories (idempotent, re-runnable; `--dry-run` previews the plan). Migration and activation are independent — `migrate` only writes records to AWS and never flips the backend, so use `init` (or set `storage_backend` yourself) to actually switch. See [AgentCore setup](website/agentcore-setup.md). `agentcore` mode needs the optional dependency group: `pip install 'better-memory[agentcore]'` (or `uv pip install '.[agentcore]'`). Sqlite-only installs skip boto3 entirely. diff --git a/better_memory/cli/_agentcore_strategies.py b/better_memory/cli/_agentcore_strategies.py index 6282e4a..0495b8c 100644 --- a/better_memory/cli/_agentcore_strategies.py +++ b/better_memory/cli/_agentcore_strategies.py @@ -39,6 +39,24 @@ {"key": "status", "type": "STRING"}, ] +# userPreference (semantic) memoryRecordSchema. Unlike the episodic schema +# (which governs only AWS-EXTRACTED records), this top-level schema governs +# CLIENT BASE writes: keys declared here are retained on create, survive +# updates, and are listable; UNDECLARED keys (e.g. source_row_id) are silently +# dropped by AWS (design §1b, proven live). The migration idempotency key +# source_row_id therefore MUST be declared here or semantic reconcile-by- +# source_row_id (design §3.2/§5.3) breaks — the migrator could not detect an +# already-migrated row and would duplicate it on re-run. +# +# OPERATIONAL NOTE for the migrate/--provision path (T5): this schema is baked +# into the userPreferenceMemoryStrategy block at CREATE_MEMORY time. A semantic +# memory PROVISIONED BEFORE source_row_id was added here does NOT have it +# declared, so client writes of source_row_id there are still dropped. Before +# writing, the migrate/--provision path must guarantee the target's live schema +# declares these keys — either widen it in place via update_memory_strategy +# (widening the metadataSchema; verify AWS permits post-hoc schema extension — +# unconfirmed as of §1b's open item) or re-provision a fresh semantic memory. +# Fresh `init` provisions with this (correct) schema and needs no migration. SEMANTIC_METADATA_SCHEMA: list[dict] = [ {"key": "useful_count", "type": "NUMBER"}, {"key": "missed_count", "type": "NUMBER"}, @@ -47,6 +65,10 @@ {"key": "overlooked_count", "type": "NUMBER"}, {"key": "last_credited_at", "type": "STRING"}, {"key": "status", "type": "STRING"}, + # Migration idempotency key (design §3.2). STRING: it holds the SQLite row + # id. Not server-indexed (indexed keys are frozen at provision, §5.3) — + # reconcile scans it client-side. + {"key": "source_row_id", "type": "STRING"}, ] INDEXED_KEYS: list[dict] = [ diff --git a/better_memory/cli/agentcore.py b/better_memory/cli/agentcore.py index cf60960..fc624e6 100644 --- a/better_memory/cli/agentcore.py +++ b/better_memory/cli/agentcore.py @@ -17,6 +17,7 @@ DEFAULT_SEMANTIC_NAME, DEFAULT_SEMANTIC_STRATEGY_NAME, INDEXED_KEYS, + SEMANTIC_METADATA_SCHEMA, episodic_strategy_block, semantic_strategy_block, ) @@ -72,9 +73,53 @@ def add_subparsers(parent: argparse.ArgumentParser) -> None: p_smoke.add_argument("--home", default=None) p_smoke.add_argument("--region", default=None) - subparsers.add_parser( - "migrate-from-sqlite", - help="(deferred) Bulk-migrate sqlite data to AgentCore", + p_migrate = subparsers.add_parser( + "migrate", + help="Migrate sqlite reflections + semantic memories into AgentCore", + ) + p_migrate.add_argument( + "--dry-run", + action="store_true", + dest="dry_run", + help="Compute the plan and print tallies without any AWS call", + ) + p_migrate.add_argument( + "--include", + default="reflections,semantic", + help=( + "Comma list of kinds to migrate: reflections, semantic, " + "observations (observations are lossy / non-retrievable)" + ), + ) + p_migrate.add_argument( + "--restart", + action="store_true", + help="Re-verify/upsert every eligible row, ignoring ledger 'migrated' state", + ) + p_migrate.add_argument( + "--provision", + action="store_true", + help="Create the target memories if missing/not ACTIVE (else hard error)", + ) + p_migrate.add_argument( + "--db", default=None, help="Source SQLite path (default /memory.db)" + ) + p_migrate.add_argument( + "--project", default=None, help="Limit migration to one project" + ) + p_migrate.add_argument("--region", default=None, help="AWS region override") + p_migrate.add_argument("--home", default=None, help="Override BETTER_MEMORY_HOME") + p_migrate.add_argument( + "--batch-size", + type=int, + default=25, + dest="batch_size", + help="Records per batch_create_memory_records call (default 25)", + ) + p_migrate.add_argument( + "--verify", + action="store_true", + help="Read a sample back via get_memory_record and diff against SQLite", ) @@ -85,7 +130,7 @@ def handle(args: argparse.Namespace) -> int: return _handle_status(args) if args.subcommand == "smoke": return _handle_smoke(args) - if args.subcommand == "migrate-from-sqlite": + if args.subcommand == "migrate": return _handle_migrate(args) print(f"unknown subcommand: {args.subcommand}", file=sys.stderr) return 2 @@ -611,11 +656,936 @@ def _handle_smoke(args: argparse.Namespace) -> int: return 1 +_VALID_INCLUDE_KINDS = frozenset({"reflections", "semantic", "observations"}) + + +def _memory_is_active(memory: dict) -> bool: + """Readiness rule (mirrors ``_poll_until_active``): memory ACTIVE AND + every strategy ACTIVE.""" + strategies = memory.get("strategies") or [] + return ( + memory.get("status") == "ACTIVE" + and bool(strategies) + and all(s.get("status") == "ACTIVE" for s in strategies) + ) + + +def _collect_declared_metadata_keys(node: Any) -> set[str] | None: + """Walk a GetMemory ``memory`` dict for any ``metadataSchema`` list and + collect the declared ``key`` names. + + Returns ``None`` if no ``metadataSchema`` is present anywhere (AWS did not + expose the strategy schema) so the caller can distinguish "schema lacks the + key" from "schema not introspectable". + """ + found: set[str] = set() + saw_schema = False + + def _walk(obj: Any) -> None: + nonlocal saw_schema + if isinstance(obj, dict): + for key, value in obj.items(): + if key == "metadataSchema" and isinstance(value, list): + saw_schema = True + for entry in value: + if isinstance(entry, dict) and "key" in entry: + found.add(str(entry["key"])) + else: + _walk(value) + elif isinstance(obj, list): + for item in obj: + _walk(item) + + _walk(node) + return found if saw_schema else None + + +_REPROVISION_GUIDANCE = ( + "Re-provision a fresh semantic memory with `better-memory agentcore init` " + "(or delete it in the AWS console and re-run migrate --provision) before " + "migrating semantic rows." +) + + +def _ensure_semantic_schema( + control: Any, cfg: AgentCoreConfig, semantic_memory: dict +) -> None: + """Guarantee the semantic strategy declares ``source_row_id`` before any + write (design §1b / T3). + + §1b requires the migrate/--provision path to *guarantee* the target schema + declares the needed keys before writing, because AWS silently drops + undeclared keys (notably ``source_row_id``) on client BASE writes — and a + dropped ``source_row_id`` breaks semantic reconcile-by-source_row_id, so + every re-run re-creates all semantic records (idempotency lost). + + Behaviour: + - If the live schema already declares ``source_row_id`` → return. + - Otherwise widen it in place via ``update_memory_strategy``, sending the + FULL :data:`SEMANTIC_METADATA_SCHEMA` (which declares ``source_row_id`` + plus the counters). The previous empty ``customExtractionConfiguration`` + payload could never add the key. + - After the update, RE-READ the memory and CONFIRM ``source_row_id`` is now + declared. AWS may accept a malformed / no-op update without raising; if + the key is still not declared (or the schema is not introspectable so we + cannot verify it), we raise rather than proceed — writing into an + unconfirmed-narrow schema would silently drop ``source_row_id``. + """ + declared = _collect_declared_metadata_keys(semantic_memory) + if declared is not None and "source_row_id" in declared: + return + + strategy_id = cfg.semantic.strategy_id + if declared is None: + print( + " semantic strategy schema is not introspectable; widening it in " + "place and re-verifying 'source_row_id' is declared..." + ) + else: + print( + " semantic strategy is missing the 'source_row_id' key; " + "widening its schema in place..." + ) + try: + control.update_memory_strategy( + memoryId=cfg.semantic.memory_id, + memoryStrategyId=strategy_id, + configuration={ + "userPreferenceOverride": { + "memoryRecordSchema": { + "metadataSchema": SEMANTIC_METADATA_SCHEMA + } + } + }, + ) + except Exception as exc: # noqa: BLE001 — re-raised as actionable guidance + raise RuntimeError( + "The target semantic memory was provisioned before " + "'source_row_id' was added to the userPreference schema, so " + "migrated records cannot be de-duplicated. Its schema could not " + f"be widened in place ({exc!r}). {_REPROVISION_GUIDANCE}" + ) from exc + + # Re-read and CONFIRM the key is now declared — the update above may have + # been accepted as a no-op without raising. + try: + refreshed = control.get_memory(memoryId=cfg.semantic.memory_id)["memory"] + except Exception as exc: # noqa: BLE001 — re-raised as actionable guidance + raise RuntimeError( + "Could not re-read the semantic memory to confirm its schema was " + f"widened to declare 'source_row_id' ({exc!r}). {_REPROVISION_GUIDANCE}" + ) from exc + redeclared = _collect_declared_metadata_keys(refreshed) + if redeclared is None or "source_row_id" not in redeclared: + raise RuntimeError( + "Widening the semantic strategy schema did not take effect: " + "'source_row_id' is still not declared after update_memory_strategy " + "(AWS accepted the call without adding the key, or the schema is not " + f"introspectable so retention cannot be guaranteed). " + f"{_REPROVISION_GUIDANCE}" + ) + + +def _resolve_target_memories( + control: Any, + cfg: AgentCoreConfig, + home: Path, + *, + provision: bool, + need_reflections: bool, + need_semantic: bool, +) -> tuple[AgentCoreConfig, dict, dict]: + """Verify (or, with ``--provision``, create) the episodic + semantic + memories. Returns ``(cfg, episodic_memory, semantic_memory)`` where ``cfg`` + is the (possibly updated) config. + + When ``--provision`` mints a REPLACEMENT for a missing / not-ACTIVE memory, + the new memory has a fresh memory_id + strategy_id. The returned + :class:`MemoryRecord` MUST replace the stale one in ``cfg`` and be persisted + to ``agentcore.json`` — otherwise the migration would push every batch to + the dead memory id and hit ``ResourceNotFoundException`` on every record. + The caller re-keys planned records to the (possibly new) strategy ids after + this returns. + + Raises ``RuntimeError`` when a needed memory is missing / not ACTIVE and + ``--provision`` was not passed. + """ + import dataclasses + + memories: dict[str, dict] = {} + targets = [] + if need_reflections: + targets.append(("episodic", cfg.episodic)) + if need_semantic: + targets.append(("semantic", cfg.semantic)) + + for label, record in targets: + memory: dict | None = None + try: + memory = control.get_memory(memoryId=record.memory_id)["memory"] + except Exception as exc: # noqa: BLE001 — not-found / access errors + if not provision: + raise RuntimeError( + f"{label} memory {record.memory_id!r} could not be read " + f"({exc!r}). Pass --provision to (re)create it, or run " + f"`better-memory agentcore init` first." + ) from exc + memory = None + + if memory is None or not _memory_is_active(memory): + if not provision: + raise RuntimeError( + f"{label} memory {record.memory_id!r} is not ACTIVE. " + f"Pass --provision to (re)create it, or wait for it to " + f"finish provisioning (`better-memory agentcore status`)." + ) + # Provisioning of a missing/broken memory is a heavy control-plane + # operation; only reached with --provision. Re-create via init's + # helper, capture the fresh MemoryRecord, persist it into cfg, and + # re-read. + created_ids: list[str] = [] + if label == "episodic": + new_record = _create_one_memory( + control, + name=DEFAULT_EPISODIC_NAME, + strategy_block=episodic_strategy_block(), + strategy_name=DEFAULT_EPISODIC_STRATEGY_NAME, + event_expiry_days=DEFAULT_EPISODIC_EVENT_EXPIRY_DAYS, + label="episodic", + created_ids=created_ids, + ) + cfg = dataclasses.replace(cfg, episodic=new_record) + else: + new_record = _create_one_memory( + control, + name=DEFAULT_SEMANTIC_NAME, + strategy_block=semantic_strategy_block(), + strategy_name=DEFAULT_SEMANTIC_STRATEGY_NAME, + event_expiry_days=DEFAULT_SEMANTIC_EVENT_EXPIRY_DAYS, + label="semantic", + created_ids=created_ids, + ) + cfg = dataclasses.replace(cfg, semantic=new_record) + # Persist the replacement id/strategy so a crash after this point + # (or a later run) targets the live memory, not the dead one. + save_agentcore_config(cfg, home) + memory = control.get_memory(memoryId=new_record.memory_id)["memory"] + + assert memory is not None # active, or reassigned via the provision branch + memories[label] = memory + + return cfg, memories.get("episodic", {}), memories.get("semantic", {}) + + +def _acquire_migration_lock(home: Path) -> int: + """Single-run advisory lock: a ``/agentcore_migration.lock`` file + created O_EXCL. Raises ``FileExistsError`` if a run is already in flight.""" + import os + + home.mkdir(parents=True, exist_ok=True) + lock_path = home / "agentcore_migration.lock" + fd = os.open(str(lock_path), os.O_CREAT | os.O_EXCL | os.O_WRONLY) + os.write(fd, str(os.getpid()).encode("ascii")) + return fd + + +def _release_migration_lock(home: Path, fd: int) -> None: + import os + + try: + os.close(fd) + except OSError: + pass + try: + (home / "agentcore_migration.lock").unlink() + except OSError: + pass + + +def _eligible_reflection_rows(conn: Any, project: str | None) -> list[Any]: + if project: + cur = conn.execute( + "SELECT * FROM reflections " + "WHERE status IN ('pending_review', 'confirmed') " + " AND (project = ? OR scope = 'general')", + (project,), + ) + else: + cur = conn.execute( + "SELECT * FROM reflections " + "WHERE status IN ('pending_review', 'confirmed')" + ) + return cur.fetchall() + + +def _retired_reflection_ids(conn: Any, project: str | None) -> list[str]: + if project: + cur = conn.execute( + "SELECT id FROM reflections " + "WHERE status IN ('retired', 'superseded') " + " AND (project = ? OR scope = 'general')", + (project,), + ) + else: + cur = conn.execute( + "SELECT id FROM reflections WHERE status IN ('retired', 'superseded')" + ) + return [r["id"] for r in cur.fetchall()] + + +def _eligible_semantic_rows(conn: Any, project: str | None) -> list[Any]: + if project: + cur = conn.execute( + "SELECT * FROM semantic_memories WHERE (project = ? OR scope = 'general')", + (project,), + ) + else: + cur = conn.execute("SELECT * FROM semantic_memories") + return cur.fetchall() + + +def _map_batch_response( + batch: list[dict[str, Any]], response: dict[str, Any] +) -> list[tuple[dict[str, Any], str | None, str | None]]: + """Zip a batch response back to its submitted records. + + Returns ``(record, minted_id, error)`` triples: exactly one of + ``minted_id`` / ``error`` is set. Maps by ``requestIdentifier`` when AWS + echoes it (the real behaviour), else falls back to submission order. + """ + successful = response.get("successfulRecords") or [] + failed = response.get("failedRecords") or [] + triples: list[tuple[dict[str, Any], str | None, str | None]] = [] + + have_req = bool(successful or failed) and all( + "requestIdentifier" in r for r in (*successful, *failed) + ) + if have_req: + by_req = {r["requestIdentifier"]: r for r in batch} + for s in successful: + triples.append( + (by_req[s["requestIdentifier"]], s.get("memoryRecordId"), None) + ) + for f in failed: + triples.append( + (by_req[f["requestIdentifier"]], None, + f.get("errorMessage", "unknown")) + ) + return triples + + # Positional fallback (submission order preserved): successes first. + for i, s in enumerate(successful): + triples.append((batch[i], s.get("memoryRecordId"), None)) + for j, f in enumerate(failed): + triples.append( + (batch[len(successful) + j], None, f.get("errorMessage", "unknown")) + ) + return triples + + def _handle_migrate(args: argparse.Namespace) -> int: - raise NotImplementedError( - "Bulk migration of sqlite data to AgentCore is deferred to a future " - "spec. See docs/superpowers/specs/2026-05-24-agentcore-storage-backend-" - "design.md § 'Open questions (deferred to implementation)' item 5. " - "Workaround for now: start fresh in agentcore mode; observations from " - "sqlite-mode sessions remain queryable in sqlite mode." + from better_memory.storage import agentcore_migrate as _mig + + home = _resolve_home(args.home) + + includes = {s.strip() for s in args.include.split(",") if s.strip()} + unknown = includes - _VALID_INCLUDE_KINDS + if unknown: + print( + f"unknown --include kind(s): {', '.join(sorted(unknown))}; " + f"valid: {', '.join(sorted(_VALID_INCLUDE_KINDS))}", + file=sys.stderr, + ) + return 1 + if "observations" in includes: + print( + "WARNING: --include=observations is lossy and non-retrievable — " + "migrated observation events are written under a synthetic session " + "id that cross-session retrieval cannot re-read, and " + "reinforcement_score has no event-plane home. Observation replay is " + "NOT performed; migrate treats observations as already distilled " + "into reflections.", + file=sys.stderr, + ) + need_reflections = "reflections" in includes + need_semantic = "semantic" in includes + if not need_reflections and not need_semantic: + print( + "nothing to migrate: --include names no supported kind " + "(reflections, semantic).", + file=sys.stderr, + ) + return 1 + + cfg = load_agentcore_config(home) + if cfg is None and not args.provision: + print( + f"No agentcore.json found at {home / 'agentcore.json'}. " + f"Run `better-memory agentcore init` first, or pass --provision.", + file=sys.stderr, + ) + return 1 + + db_path = Path(args.db).expanduser() if args.db else home / "memory.db" + if not db_path.exists(): + print(f"Source SQLite db not found at {db_path}.", file=sys.stderr) + return 1 + + strategy_reflections = cfg.episodic.strategy_id if cfg else "" + strategy_semantic = cfg.semantic.strategy_id if cfg else "" + + # ------------------------------------------------------------------ # + # Build the per-row plan from SQLite + the ledger. No AWS yet. + # ------------------------------------------------------------------ # + from better_memory.db.connection import connect as _db_connect + + conn = _db_connect(db_path) + try: + _mig.ensure_ledger(conn) + + # ---------------------------------------------------------------- # + # Phase A — build the per-row records (no ledger decision yet). The + # strategy id may still be a placeholder ('' when cfg is None and we + # will provision); it does not feed the content hash (§5.2) and is + # re-keyed to the real id before any write. + # ---------------------------------------------------------------- # + # planned_raw[kind] = list of (row, record, content_hash, namespace) + planned_raw: dict[str, list[tuple]] = {"reflection": [], "semantic": []} + + if need_reflections: + for row in _eligible_reflection_rows(conn, args.project): + rec = _mig.build_reflection_record( + row, strategy_id=strategy_reflections + ) + if rec is None: # defensive — query already filters status + continue + chash = _mig.canonical_content_hash(rec) + planned_raw["reflection"].append( + (row, rec, chash, rec["namespaces"][0]) + ) + + if need_semantic: + for row in _eligible_semantic_rows(conn, args.project): + rec = _mig.build_semantic_record( + row, strategy_id=strategy_semantic + ) + chash = _mig.canonical_content_hash(rec) + planned_raw["semantic"].append( + (row, rec, chash, rec["namespaces"][0]) + ) + + def _decide() -> tuple[dict, dict, dict]: + """Compute create/update/skip/retire decisions from the CURRENT + ledger state (re-runnable after reconcile mutates the ledger).""" + tallies: dict[tuple[str, str], dict[str, int]] = {} + + def _bump(kind: str, namespace: str, action: str) -> None: + slot = tallies.setdefault( + (kind, namespace), + {"create": 0, "update": 0, "skip": 0, "retire": 0}, + ) + slot[action] += 1 + + # planned[kind] = (row, record, content_hash, decision, namespace) + planned: dict[str, list[tuple]] = {"reflection": [], "semantic": []} + retires: dict[str, list[tuple[str, str]]] = {"reflection": []} + + for kind in ("reflection", "semantic"): + for row, rec, chash, ns in planned_raw[kind]: + decision = _mig.plan_row(conn, kind, row["id"], chash) + if args.restart and decision == "skip": + decision = "update" + planned[kind].append((row, rec, chash, decision, ns)) + _bump(kind, ns, decision) + + if need_reflections: + for retired_id in _retired_reflection_ids(conn, args.project): + decision = _mig.plan_row(conn, "reflection", retired_id, None) + if decision == "retire": + ledger = conn.execute( + "SELECT namespace, target_record_id FROM " + "agentcore_migration WHERE source_kind = 'reflection' " + "AND source_id = ?", + (str(retired_id),), + ).fetchone() + ns = ledger["namespace"] if ledger else "" + target = ledger["target_record_id"] if ledger else None + if target: + retires["reflection"].append((str(retired_id), target)) + _bump("reflection", ns, "retire") + + return planned, retires, tallies + + # ---------------------------------------------------------------- # + # Dry-run: decide from the ledger, print tallies, ZERO AWS calls. + # (No remote reconcile — dry-run may not touch AWS; its tallies are a + # best-effort estimate from local state.) + # ---------------------------------------------------------------- # + if args.dry_run: + _, _, tallies = _decide() + _print_migration_plan(tallies, dry_run=True) + return 0 + + # ---------------------------------------------------------------- # + # Real run. Build clients INSIDE the try so auth/region failures + # land on rc=1 (smoke pattern). + # ---------------------------------------------------------------- # + region = args.region or (cfg.region if cfg else "eu-west-2") + try: + control = _build_control_client(region) + data = _build_data_client(region) + + if cfg is None: + # --provision with no config: create both fresh, persist. + created_ids: list[str] = [] + episodic = _create_one_memory( + control, + name=DEFAULT_EPISODIC_NAME, + strategy_block=episodic_strategy_block(), + strategy_name=DEFAULT_EPISODIC_STRATEGY_NAME, + event_expiry_days=DEFAULT_EPISODIC_EVENT_EXPIRY_DAYS, + label="episodic", + created_ids=created_ids, + ) + semantic = _create_one_memory( + control, + name=DEFAULT_SEMANTIC_NAME, + strategy_block=semantic_strategy_block(), + strategy_name=DEFAULT_SEMANTIC_STRATEGY_NAME, + event_expiry_days=DEFAULT_SEMANTIC_EVENT_EXPIRY_DAYS, + label="semantic", + created_ids=created_ids, + ) + cfg = AgentCoreConfig( + schema_version=1, region=region, + semantic=semantic, episodic=episodic, + ) + save_agentcore_config(cfg, home) + + # Verify / provision targets. This may MINT REPLACEMENTS and return + # an updated cfg (new memory + strategy ids), so re-read the strategy + # ids from the returned cfg afterwards. + cfg, _epi_mem, _sem_mem = _resolve_target_memories( + control, cfg, home, + provision=args.provision, + need_reflections=need_reflections, + need_semantic=need_semantic, + ) + if need_semantic: + _ensure_semantic_schema(control, cfg, _sem_mem) + + # Re-key planned records to the FINAL strategy ids (covers both the + # no-config fresh-provision path and a --provision replacement). + # Hash is strategy-independent (§5.2), so this does not change any + # content_hash or ledger decision. + planned_raw = _rekey_strategy( + planned_raw, cfg.episodic.strategy_id, cfg.semantic.strategy_id + ) + + # §5.3 ledger-loss safety net: reconcile existing remote records by + # source_row_id BEFORE deciding, so a lost/partial ledger reattaches + # target ids (yielding 'update') instead of re-creating duplicates. + _reconcile_from_remote( + conn, data, cfg, planned_raw, + need_reflections=need_reflections, + need_semantic=need_semantic, + ) + except RuntimeError as exc: + print(f"migrate FAILED: {exc}", file=sys.stderr) + return 1 + except Exception as exc: # noqa: BLE001 — auth/region/client failures + print(f"migrate FAILED: {exc!r}", file=sys.stderr) + return 1 + + # Decide AFTER reconcile so reattached targets drive update-not-create. + planned, retires, tallies = _decide() + + try: + lock_fd = _acquire_migration_lock(home) + except FileExistsError: + print( + f"Another migrate run holds the lock " + f"({home / 'agentcore_migration.lock'}). Wait for it to " + f"finish or delete the stale lock file.", + file=sys.stderr, + ) + return 1 + + failures = 0 + try: + failures += _execute_kind( + conn, data, "reflection", + memory_id=cfg.episodic.memory_id, + planned=planned["reflection"], + retires=retires["reflection"], + batch_size=args.batch_size, + ) + failures += _execute_kind( + conn, data, "semantic", + memory_id=cfg.semantic.memory_id, + planned=planned["semantic"], + retires=[], + batch_size=args.batch_size, + ) + + if args.verify: + _verify_sample(conn, data, cfg) + finally: + _release_migration_lock(home, lock_fd) + + _print_migration_plan(tallies, dry_run=False) + if failures: + print( + f"\nmigrate completed with {failures} failed record(s); " + f"they are marked 'failed' in the ledger and a re-run will " + f"retry them.", + file=sys.stderr, + ) + return 2 + print("\nmigrate: all eligible rows converged.") + return 0 + finally: + conn.close() + + +def _rekey_strategy( + planned_raw: dict[str, list[tuple]], + strategy_reflections: str, + strategy_semantic: str, +) -> dict[str, list[tuple]]: + """Rewrite planned records' ``memoryStrategyId`` to the real strategy ids + after provisioning (fresh, or a --provision replacement). + + Operates on the phase-A ``(row, record, content_hash, namespace)`` tuples. + The content_hash is deliberately preserved: ``canonical_content_hash`` + excludes ``memoryStrategyId`` (§5.2), so re-keying must NOT change the hash + — otherwise the ledger's stored hash would never match a later run's hash, + forcing a spurious update every run.""" + out: dict[str, list[tuple]] = {"reflection": [], "semantic": []} + for row, rec, chash, ns in planned_raw["reflection"]: + rec = {**rec, "memoryStrategyId": strategy_reflections} + out["reflection"].append((row, rec, chash, ns)) + for row, rec, chash, ns in planned_raw["semantic"]: + rec = {**rec, "memoryStrategyId": strategy_semantic} + out["semantic"].append((row, rec, chash, ns)) + return out + + +def _list_namespace_records( + data: Any, memory_id: str, namespace: str +) -> list[dict[str, Any]]: + """Page through every record in one namespace via ``list_memory_records``. + + Returns the raw record summaries (each may carry ``memoryRecordId``, + ``content`` and ``metadata``). Used by the reconcile scan; a client-side + scan is required because ``source_row_id`` is not a server-indexed key + (§5.3).""" + records: list[dict[str, Any]] = [] + next_token: str | None = None + while True: + kwargs: dict[str, Any] = { + "memoryId": memory_id, + "namespace": namespace, + "maxResults": 100, + } + if next_token: + kwargs["nextToken"] = next_token + resp = data.list_memory_records(**kwargs) + records.extend(resp.get("memoryRecordSummaries") or []) + next_token = resp.get("nextToken") + if not next_token: + break + return records + + +def _reflection_source_row_id(record: dict[str, Any]) -> str | None: + """Extract ``source_row_id`` from a reflection record's JSON content body + (design §1b: all reflection state lives in the body). Only own migrated + records (``source_backend='sqlite'``) are eligible (§10 risk 6).""" + text = (record.get("content") or {}).get("text") + if not isinstance(text, str): + return None + try: + body = json.loads(text) + except (ValueError, TypeError): + return None + if not isinstance(body, dict): + return None + if body.get("source_backend") != "sqlite": + return None + srid = body.get("source_row_id") + return str(srid) if srid else None + + +def _semantic_source_row_id(record: dict[str, Any]) -> str | None: + """Extract ``source_row_id`` from a semantic record's declared metadata + (design §1b/§3.2: semantic idempotency key is declared metadata).""" + metadata = record.get("metadata") + if not isinstance(metadata, dict): + return None + entry = metadata.get("source_row_id") + if not isinstance(entry, dict): + return None + srid = entry.get("stringValue") + return str(srid) if srid else None + + +def _reconcile_from_remote( + conn: Any, + data: Any, + cfg: AgentCoreConfig, + planned_raw: dict[str, list[tuple]], + *, + need_reflections: bool, + need_semantic: bool, +) -> int: + """Client-side reconcile-by-source_row_id (design §5.3, one of the four + idempotency pillars in §1). + + Scans each target namespace this run would write to, indexes existing + records by ``source_row_id``, and reattaches the server-minted + ``target_record_id`` to any ledger row that lacks one. This makes + ``--restart`` — and any run after a lost/absent ledger — idempotent: + without it, ``plan_row`` returns 'create' for every eligible row and + re-creates duplicates (short-window ``requestIdentifier`` dedup, §5.1, is + no defense across a ledger loss). Returns the number of records reattached. + + Scanned namespaces are exactly those the planned records target, so an + empty plan performs no list calls. + """ + from better_memory.storage import agentcore_migrate as _mig + + reattached = 0 + + def _scan(kind: str, memory_id: str, extract) -> None: + nonlocal reattached + namespaces = {ns for (_row, _rec, _h, ns) in planned_raw[kind]} + for namespace in sorted(namespaces): + for record in _list_namespace_records(data, memory_id, namespace): + srid = extract(record) + target = record.get("memoryRecordId") + if not srid or not target: + continue + if _mig.reconcile_ledger( + conn, kind=kind, source_id=srid, + namespace=namespace, target_record_id=target, + ): + reattached += 1 + + if need_reflections: + _scan("reflection", cfg.episodic.memory_id, _reflection_source_row_id) + if need_semantic: + _scan("semantic", cfg.semantic.memory_id, _semantic_source_row_id) + + return reattached + + +def _execute_kind( + conn: Any, + data: Any, + kind: str, + *, + memory_id: str, + planned: list[tuple], + retires: list[tuple[str, str]], + batch_size: int, +) -> int: + """Execute the create/update/retire plan for one kind. Updates the ledger + per record; returns the number of failed records (for the exit code).""" + from better_memory.storage import agentcore_migrate as _mig + + creates = [p for p in planned if p[3] == "create"] + updates = [p for p in planned if p[3] == "update"] + failures = 0 + + # ---- CREATE (batched) ---- + create_records = [rec for (_row, rec, _h, _d, _ns) in creates] + meta_by_req = { + rec["requestIdentifier"]: (row, chash, ns) + for (row, rec, chash, _d, ns) in creates + } + for batch in _mig.chunk(create_records, batch_size): + try: + resp = _mig.push_batch(data, memory_id, batch) + except Exception as exc: # noqa: BLE001 — whole-batch failure + for rec in batch: + row, chash, ns = meta_by_req[rec["requestIdentifier"]] + _mig.record_failure( + conn, kind=kind, source_id=row["id"], + last_error=repr(exc), namespace=ns, content_hash=chash, + ) + failures += 1 + continue + for rec, minted_id, error in _map_batch_response(batch, resp): + row, chash, ns = meta_by_req[rec["requestIdentifier"]] + if error is not None: + _mig.record_failure( + conn, kind=kind, source_id=row["id"], + last_error=error, namespace=ns, content_hash=chash, + ) + failures += 1 + else: + _mig.record_success( + conn, kind=kind, source_id=row["id"], namespace=ns, + content_hash=chash, target_record_id=minted_id, + ) + + # ---- UPDATE (batched) ---- + update_records = [] + meta_by_record_id: dict[str, tuple] = {} + for (row, rec, chash, _d, ns) in updates: + ledger = conn.execute( + "SELECT target_record_id FROM agentcore_migration " + "WHERE source_kind = ? AND source_id = ?", + (kind, str(row["id"])), + ).fetchone() + target = ledger["target_record_id"] if ledger else None + if not target: + # No minted id to update against -> re-create instead. + _mig.record_failure( + conn, kind=kind, source_id=row["id"], + last_error="update requested but no target_record_id in ledger", + namespace=ns, content_hash=chash, + ) + failures += 1 + continue + upd = { + "memoryRecordId": target, + "timestamp": rec["timestamp"], + "content": rec["content"], + } + if "metadata" in rec: + upd["metadata"] = rec["metadata"] + update_records.append(upd) + meta_by_record_id[target] = (row, chash, ns) + + for batch in _mig.chunk(update_records, batch_size): + try: + resp = data.batch_update_memory_records( + memoryId=memory_id, records=batch + ) + except Exception as exc: # noqa: BLE001 + for upd in batch: + row, chash, ns = meta_by_record_id[upd["memoryRecordId"]] + _mig.record_failure( + conn, kind=kind, source_id=row["id"], + last_error=repr(exc), namespace=ns, content_hash=chash, + ) + failures += 1 + continue + successful = { + r.get("memoryRecordId") for r in (resp.get("successfulRecords") or []) + } + failed = { + r.get("memoryRecordId"): r.get("errorMessage", "unknown") + for r in (resp.get("failedRecords") or []) + } + for upd in batch: + rid = upd["memoryRecordId"] + row, chash, ns = meta_by_record_id[rid] + if rid in failed or (successful and rid not in successful): + _mig.record_failure( + conn, kind=kind, source_id=row["id"], + last_error=failed.get(rid, "update not confirmed"), + namespace=ns, content_hash=chash, + ) + failures += 1 + else: + _mig.record_success( + conn, kind=kind, source_id=row["id"], namespace=ns, + content_hash=chash, target_record_id=rid, + ) + + # ---- RETIRE (read-modify-write body status) ---- + from datetime import UTC, datetime + for source_id, target in retires: + try: + record = data.get_memory_record( + memoryId=memory_id, memoryRecordId=target + )["memoryRecord"] + text = record.get("content", {}).get("text", "") + body = json.loads(text) if isinstance(text, str) else {} + if not isinstance(body, dict): + body = {} + body["status"] = "retired" + data.batch_update_memory_records( + memoryId=memory_id, + records=[{ + "memoryRecordId": target, + "timestamp": datetime.now(UTC), + "content": {"text": json.dumps(body, sort_keys=True)}, + }], + ) + _mig.record_success( + conn, kind=kind, source_id=source_id, status="retired", + ) + except Exception as exc: # noqa: BLE001 + _mig.record_failure( + conn, kind=kind, source_id=source_id, last_error=repr(exc), + ) + failures += 1 + + return failures + + +def _verify_sample(conn: Any, data: Any, cfg: AgentCoreConfig) -> None: + """Read back one migrated record per kind (read-your-write, retry on + ResourceNotFoundException) and diff a key field against SQLite.""" + for kind, memory_id in ( + ("reflection", cfg.episodic.memory_id), + ("semantic", cfg.semantic.memory_id), + ): + ledger = conn.execute( + "SELECT source_id, target_record_id FROM agentcore_migration " + "WHERE source_kind = ? AND status = 'migrated' " + "AND target_record_id IS NOT NULL LIMIT 1", + (kind,), + ).fetchone() + if ledger is None: + continue + record = None + for attempt in range(1, 6): + try: + record = data.get_memory_record( + memoryId=memory_id, + memoryRecordId=ledger["target_record_id"], + )["memoryRecord"] + break + except Exception as exc: # noqa: BLE001 + code = getattr(exc, "response", {}).get( + "Error", {} + ).get("Code", "") + if code != "ResourceNotFoundException" or attempt == 5: + print( + f" verify: could not read back {kind} " + f"{ledger['target_record_id']}: {exc!r}", + file=sys.stderr, + ) + break + time.sleep(2) + if record is not None: + print( + f" verify: {kind} {ledger['target_record_id']} readback ok" + ) + + +def _print_migration_plan( + tallies: dict[tuple[str, str], dict[str, int]], *, dry_run: bool +) -> None: + header = "Migration plan (dry-run — no AWS calls)" if dry_run else \ + "Migration summary" + print(header) + totals = {"create": 0, "update": 0, "skip": 0, "retire": 0} + for (kind, namespace), counts in sorted(tallies.items()): + print( + f" {kind:<10} {namespace:<40} " + f"create={counts['create']} update={counts['update']} " + f"skip={counts['skip']} retire={counts['retire']}" + ) + for action in totals: + totals[action] += counts[action] + print( + f" {'TOTAL':<10} {'':<40} " + f"create={totals['create']} update={totals['update']} " + f"skip={totals['skip']} retire={totals['retire']}" ) diff --git a/better_memory/mcp/handlers/semantics.py b/better_memory/mcp/handlers/semantics.py index 4f79456..d7df6a0 100644 --- a/better_memory/mcp/handlers/semantics.py +++ b/better_memory/mcp/handlers/semantics.py @@ -77,16 +77,21 @@ async def semantic_retrieve(self, args: dict[str, Any]) -> list[TextContent]: for record in self._remote.semantic_list( project=project, scope_filter=scope_filter ): - record_id = record["id"] + # semantic_list now returns SemanticMemory objects (§6.3), + # matching the sqlite path below — attribute access, not + # dict subscripting. created_at/updated_at stay None: the + # stable UD-2 payload contract keeps agentcore semantic + # rows key-identical to sqlite with placeholder timestamps. + record_id = record.id if record_id in seen: continue seen.add(record_id) merged.append( { "id": record_id, - "content": record.get("content", ""), + "content": record.content, "project": None, - "scope": record.get("scope", "project"), + "scope": record.scope, "created_at": None, "updated_at": None, } diff --git a/better_memory/storage/agentcore.py b/better_memory/storage/agentcore.py index b1731ca..7c56822 100644 --- a/better_memory/storage/agentcore.py +++ b/better_memory/storage/agentcore.py @@ -55,6 +55,16 @@ class _ClientError(Exception): "overlooked": "overlooked_count", } +# For migrated (SQLite-origin) reflection records, rating state lives in the +# JSON content body under the SQLite column names (design §1b/§3.1). The +# AgentCore metadata counter key `overlooked_count` is stored in the body under +# the SQLite column name `times_overlooked`; every other counter key keeps the +# same name in the body. Used to translate a metadata counter key to its body +# field when read-modify-writing a migrated record's content. +_COUNTER_KEY_TO_BODY_FIELD: dict[str, str] = { + "overlooked_count": "times_overlooked", +} + class AgentCoreBackend: """boto3-backed StorageBackend implementation.""" @@ -374,10 +384,30 @@ def _parse_reflection_record( body = {} metadata_raw = rec.get("metadata", {}) + # Migrated (SQLite-origin) records carry all reflection state in the + # JSON content body; AWS-extracted records carry it in metadata (and + # never in the body). Every field below therefore resolves BODY-FIRST + # with a METADATA FALLBACK: a body without the key is exactly the + # AWS-extracted shape, so the fallback reproduces today's behavior + # byte-for-byte (no regression). See migration design §1b/§6. + body_dict: dict[str, Any] = body if isinstance(body, dict) else {} def _num(key: str) -> float: return float(metadata_raw.get(key, {}).get("numberValue", 0)) + def _count_body_first(body_key: str, meta_key: str) -> int: + """Integer counter, body value preferred over metadata numberValue. + + Body absent (or non-numeric) -> the existing metadata numberValue + path, preserving AWS-extracted-record behavior exactly.""" + body_val = body_dict.get(body_key) + if body_val is not None: + try: + return int(body_val) + except (TypeError, ValueError): + pass + return int(_num(meta_key)) + tech_value: str | None = body.get("tech") if isinstance(body, dict) else None if tech_filter is not None and tech_value != tech_filter: return None @@ -398,23 +428,64 @@ def _num(key: str) -> float: except (TypeError, ValueError): confidence = 0.0 - # Live dialect: NO updatedAt field exists on any record shape - # (createdAt only). Update recency lives in the system metadata key - # x-amz-agentcore-memory-updatedAt (dateTimeValue). + # updated_at, body-first (§6.2). Migrated records carry an ISO-8601 + # string in the body; AWS-extracted records have NO updatedAt on the + # record shape (createdAt only) — recency lives in the system metadata + # key x-amz-agentcore-memory-updatedAt (dateTimeValue), falling back to + # createdAt. Body absent -> identical to the pre-migration path. + body_updated = body_dict.get("updated_at") sys_updated = metadata_raw.get( f"{_AGENTCORE_SYSTEM_METADATA_PREFIX}updatedAt", {} ).get("dateTimeValue") - updated_at = sys_updated or rec.get("updatedAt") or rec.get("createdAt") - updated_at_ts = updated_at.timestamp() if isinstance(updated_at, datetime) else 0.0 + updated_at_raw = ( + body_updated + or sys_updated + or rec.get("updatedAt") + or rec.get("createdAt") + ) + if isinstance(updated_at_raw, datetime): + updated_at_value: str | None = updated_at_raw.isoformat() + updated_at_ts = updated_at_raw.timestamp() + elif isinstance(updated_at_raw, str): + updated_at_value = updated_at_raw + try: + updated_at_ts = datetime.fromisoformat(updated_at_raw).timestamp() + except ValueError: + updated_at_ts = 0.0 + else: + updated_at_value = None + updated_at_ts = 0.0 # Client-side bucketing inputs. Missing/unknown polarity buckets as # neutral and missing status parses as active — AgentCore's own # extraction strategy writes neither key, and those records must # still be retrievable (see _fetch_reflection_buckets). - polarity_value = metadata_raw.get("polarity", {}).get("stringValue") + # polarity is the bucket selector and is CRITICAL for migrated + # records — they carry it only in the body. Body-first, metadata + # fallback (AWS-extracted records carry neither -> neutral). + polarity_value = ( + body_dict.get("polarity") + or metadata_raw.get("polarity", {}).get("stringValue") + ) if polarity_value not in _POLARITIES: polarity_value = "neutral" - status_value = metadata_raw.get("status", {}).get("stringValue") or "active" + status_value = ( + body_dict.get("status") + or metadata_raw.get("status", {}).get("stringValue") + or "active" + ) + + # evidence_count body-first (§6.1). Prefer the stored body value + # (migrated, synthesis-recomputed source count); else the existing + # computed metadata useful+missed fallback (AWS-extracted records). + body_ec = body_dict.get("evidence_count") + if body_ec is not None: + try: + evidence_count = int(body_ec) + except (TypeError, ValueError): + evidence_count = int(_num("useful_count")) + int(_num("missed_count")) + else: + evidence_count = int(_num("useful_count")) + int(_num("missed_count")) return { # Public shape — must match ReflectionSynthesisService.retrieve_reflections @@ -427,15 +498,15 @@ def _num(key: str) -> float: "hints": hints_list, "confidence": confidence, "tech": tech_value, - "evidence_count": int(_num("useful_count")) + int(_num("missed_count")), - "useful_count": int(_num("useful_count")), - "times_misled": int(_num("times_misled")), - "updated_at": ( - updated_at.isoformat() if isinstance(updated_at, datetime) else None - ), + "evidence_count": evidence_count, + "useful_count": _count_body_first("useful_count", "useful_count"), + "times_misled": _count_body_first("times_misled", "times_misled"), + "updated_at": updated_at_value, # Internal ranking/bucketing helpers — stripped by # _fetch_reflection_buckets before the payload leaves the backend. - "_overlooked_count": int(_num("overlooked_count")), + "_overlooked_count": _count_body_first( + "times_overlooked", "overlooked_count" + ), "_updated_at_ts": updated_at_ts, "_polarity": polarity_value, "_status": status_value, @@ -594,6 +665,79 @@ def _full_metadata_snapshot( snapshot.update(updates) return snapshot + @staticmethod + def _record_body(record: dict[str, Any]) -> dict[str, Any]: + """Parse a memory record's JSON content body into a dict. + + Returns ``{}`` for non-JSON / non-object bodies. Migrated + (SQLite-origin) records carry ``source_backend='sqlite'`` here; the + rating paths use that marker to decide whether reflection state lives + in the content body (migrated) or in metadata (AWS-extracted).""" + text = record.get("content", {}).get("text", "") + try: + body = json.loads(text) if isinstance(text, str) else {} + except json.JSONDecodeError: + body = {} + return body if isinstance(body, dict) else {} + + def _credit_body_counter( + self, + *, + memory_id: str, + record_id: str, + body: dict[str, Any], + counter_key: str, + ) -> dict[str, Any]: + """Read-modify-write a migrated record's JSON content BODY: bump the + counter, refresh ``last_credited_at``, and persist via a CONTENT update. + + ``updated_at`` is deliberately NOT refreshed here. SQLite crediting + (``memory_rating.py``) bumps only the counter + ``last_*_at`` and never + ``reflections.updated_at``; because the reader resolves ``updated_at`` + body-first (design §6.2), bumping it here would drift a migrated + reflection's synthesis-time ``updated_at`` forward on every rating, + corrupting ``age_days`` (relevant.py) and the ``updated_at DESC`` + ranking tiebreak relative to the SQLite source — the exact parity §6.2 + exists to preserve. ``_mutate_namespace_and_status`` DOES bump + ``updated_at`` on purpose: sqlite promote/retire bump it too, so that + path is correct parity; only rating must leave it untouched. + + AWS silently drops the custom metadata map on client-authored BASE + records in the episodic reflections namespace (design §1b, proven + live), so a metadata write is a no-op there — rating state must live in + the content body, which DOES persist. The metadata counter key is + translated to its body field name (``overlooked_count`` → + ``times_overlooked``; all others unchanged). + + Concurrency: last-writer-wins. ``body`` was just read via + ``get_memory_record`` (read-your-write), and the data plane exposes no + conditional / optimistic-concurrency update, so a concurrent crediter's + increment can be lost. Acceptable for the low-contention rating path + (design §1b, documented).""" + body_field = _COUNTER_KEY_TO_BODY_FIELD.get(counter_key, counter_key) + try: + current = int(body.get(body_field, 0) or 0) + except (TypeError, ValueError): + current = 0 + now = datetime.now(UTC).isoformat() + new_body = dict(body) + new_body[body_field] = current + 1 + # last_credited_at lives in the body for migrated records (design §1b). + # updated_at is intentionally left unchanged (see docstring / §6.2). + new_body["last_credited_at"] = now + return self._retry_on_transient_404( + lambda: self._data.batch_update_memory_records( + memoryId=memory_id, + records=[ + { + "memoryRecordId": record_id, + "timestamp": datetime.now(UTC), + "content": {"text": json.dumps(new_body)}, + } + ], + ) + ) + def record_use( self, observation_id: str, *, outcome: UseOutcome | None = None ) -> None: @@ -603,9 +747,29 @@ def record_use( return record = self._get_record(observation_id) - metadata = record.get("metadata", {}) - counter_key = "useful_count" if outcome == "success" else "missed_count" + + # Migrated (SQLite-origin) reflections carry rating state in the content + # body; a metadata write is silently dropped by AWS (design §1b). + # Read-modify-write the body instead. AWS-extracted records (no marker) + # keep the metadata path below unchanged. + body = self._record_body(record) + if body.get("source_backend") == "sqlite": + response = self._credit_body_counter( + memory_id=self._cfg.episodic.memory_id, + record_id=observation_id, + body=body, + counter_key=counter_key, + ) + failed = response.get("failedRecords", []) + if failed: + raise RuntimeError( + f"AgentCore record_use failed for {observation_id}: " + f"{failed[0].get('errorMessage', 'unknown')}" + ) + return + + metadata = record.get("metadata", {}) current_count = float( metadata.get(counter_key, {}).get("numberValue", 0) ) @@ -740,24 +904,108 @@ def semantic_list( ) return [ - { - "id": rec["memoryRecordId"], - "content": rec.get("content", {}).get("text", ""), - "namespaces": rec.get("namespaces", []), - # Live dialect: stored namespaces gain a leading slash - # ("/general/semantic/") — normalize before classifying or - # every read-back record misreports as project scope. - "scope": ( - "general" - if (next(iter(rec.get("namespaces") or [""]), "")) - .lstrip("/") - .startswith("general/") - else "project" - ), - } + self._semantic_summary_to_model( + rec, project=project or self._project + ) for rec in response.get("memoryRecordSummaries", []) ] + def _semantic_summary_to_model( + self, rec: dict[str, Any], *, project: str + ) -> Any: + """Map a MemoryRecordSummary -> a SemanticMemory-shaped read model. + + §6.3: relevant.py (retrieve_relevant) reads semantic memories via + ATTRIBUTE access (getattr(s, 'content'), getattr(s, 'useful_count'), + ...). Returning plain dicts made every getattr silently resolve to its + default ('' / 0), so agentcore-mode semantic memories never cleared the + min-hits floor and semantic injection was dead. Returning the same + SemanticMemory dataclass the SQLite read model returns (services/ + semantic.py) fixes that: attribute access resolves to real content and + the declared metadata counters the migrator writes. + + Counters come from the declared numberValue metadata keys + (useful_count / times_misled / overlooked_count → times_overlooked); + the collapsed rating timestamp comes from last_credited_at stringValue. + Absent metadata (AWS-extracted or freshly-created records) → zeroed + counters, never None.""" + # Local import: the SemanticMemory read model lives in the services + # layer; importing at module scope would invert the storage→services + # layering. This is a lightweight frozen dataclass, imported lazily. + from better_memory.services.semantic import SemanticMemory + + metadata = rec.get("metadata", {}) or {} + + def _count(key: str) -> int: + raw = metadata.get(key, {}) + if not isinstance(raw, dict): + return 0 + try: + return int(raw.get("numberValue", 0) or 0) + except (TypeError, ValueError): + return 0 + + def _str_meta(key: str) -> str | None: + raw = metadata.get(key, {}) + if isinstance(raw, dict): + return raw.get("stringValue") + return None + + namespaces = rec.get("namespaces") or [] + # Live dialect: stored namespaces gain a leading slash + # ("/general/semantic/") — normalize before classifying or every + # read-back record misreports as project scope. + first_ns = next(iter(namespaces or [""]), "") + scope = ( + "general" + if first_ns.lstrip("/").startswith("general/") + else "project" + ) + + def _iso(value: Any) -> str | None: + if isinstance(value, datetime): + return value.isoformat() + if isinstance(value, str): + return value + return None + + created_at = _iso(rec.get("createdAt")) or "" + # updated_at drives relevant.py age_days. Prefer the system-managed + # updatedAt (dateTimeValue), then the record's updatedAt/createdAt. + sys_updated = metadata.get( + f"{_AGENTCORE_SYSTEM_METADATA_PREFIX}updatedAt", {} + ) + sys_updated_val = ( + sys_updated.get("dateTimeValue") + if isinstance(sys_updated, dict) + else None + ) + updated_at = ( + _iso(sys_updated_val) + or _iso(rec.get("updatedAt")) + or created_at + ) + + # The three per-class rating timestamps collapsed to one + # last_credited_at on write (design §3.2); surface it on the + # useful-at slot as best-effort recency signal. + last_credited_at = _str_meta("last_credited_at") + + return SemanticMemory( + id=rec["memoryRecordId"], + content=rec.get("content", {}).get("text", ""), + project=project, + scope=scope, + created_at=created_at, + updated_at=updated_at, + useful_count=_count("useful_count"), + last_useful_at=last_credited_at, + times_misled=_count("times_misled"), + last_misled_at=None, + times_overlooked=_count("overlooked_count"), + last_overlooked_at=None, + ) + def semantic_update_text(self, *, id: str, content: str) -> None: """Update the text of a semantic record. Metadata snapshot unchanged (but full — system keys stripped via _full_metadata_snapshot).""" @@ -890,8 +1138,42 @@ def _mutate_namespace_and_status( new_status: str, ) -> None: """Shared helper: read current metadata, mutate namespace + status, - write back with full metadata snapshot.""" + write back with full metadata snapshot. + + Migrated (SQLite-origin) reflections carry status in the content body, + not metadata (design §1b) — a metadata status write is silently dropped + by AWS. For those records the status change is a body read-modify-write + (CONTENT update); the namespace move is still applied via the + ``namespaces`` field, which is not part of the dropped metadata map. + AWS-extracted records keep the metadata path below unchanged.""" record = self._get_record(reflection_id) + + body = self._record_body(record) + if body.get("source_backend") == "sqlite": + new_body = dict(body) + new_body["status"] = new_status + new_body["updated_at"] = datetime.now(UTC).isoformat() + response = self._retry_on_transient_404( + lambda: self._data.batch_update_memory_records( + memoryId=self._cfg.episodic.memory_id, + records=[ + { + "memoryRecordId": reflection_id, + "timestamp": datetime.now(UTC), + "namespaces": new_namespaces, + "content": {"text": json.dumps(new_body)}, + } + ], + ) + ) + failed = response.get("failedRecords", []) + if failed: + raise RuntimeError( + f"AgentCore reflection mutation failed for {reflection_id}: " + f"{failed[0].get('errorMessage', 'unknown')}" + ) + return + metadata = record.get("metadata", {}) updates: dict[str, dict[str, Any]] = { @@ -1078,6 +1360,29 @@ def credit_one( record = self._get_record(id) memory_id = self._cfg.episodic.memory_id + # Migrated (SQLite-origin) EPISODIC reflections carry rating state in + # the content body; a metadata write is silently dropped by AWS in the + # episodic reflections namespace (design §1b). Read-modify-write the + # body instead. Semantic migrated records use DECLARED metadata (the + # userPreference schema governs client writes — design §1b), so they + # stay on the metadata path below; AWS-extracted reflections likewise. + if kind != "semantic": + body = self._record_body(record) + if body.get("source_backend") == "sqlite": + response = self._credit_body_counter( + memory_id=memory_id, + record_id=id, + body=body, + counter_key=counter_key, + ) + failed = response.get("failedRecords", []) + if failed: + return { + "applied": None, + "skipped": failed[0].get("errorMessage", "unknown"), + } + return {"applied": classification, "skipped": None} + metadata = record.get("metadata", {}) current = float(metadata.get(counter_key, {}).get("numberValue", 0)) diff --git a/better_memory/storage/agentcore_migrate.py b/better_memory/storage/agentcore_migrate.py new file mode 100644 index 0000000..4ee36d9 --- /dev/null +++ b/better_memory/storage/agentcore_migrate.py @@ -0,0 +1,472 @@ +"""Pure builders + local ledger for the SQLite -> AgentCore migration. + +This module is the *first-ever writer of reflection records* (design §1/§1b): +better-memory otherwise only mutates AWS-extracted reflection metadata. It +constructs ``batch_create_memory_records`` payloads directly from SQLite rows. + +Design principle (§1b, validated live against real AWS): the custom metadata +map is silently dropped on client-authored BASE records in the *episodic* +(reflections) namespace, so **all reflection state lives in the JSON content +body**. The *semantic* (userPreference) strategy DOES honour a declared +``memoryRecordSchema``, so semantic state lives in a declared metadata map with +the content staying the user's raw text. + +Everything here is a pure function (no boto3, no AWS) except ``push_batch``, +which is the thin batching wrapper, and the ledger helpers, which touch the +SOURCE SQLite db. Builders take the row + the target strategy id so they are +unit-testable without a live config. +""" + +from __future__ import annotations + +import hashlib +import json +import sqlite3 +from collections.abc import Iterator, Mapping, Sequence +from datetime import UTC, datetime +from typing import Any, Literal + +from better_memory.storage.session import resolve_actor_id, resolve_namespace + +# AWS caps requestIdentifier at 80 chars (mirrors agentcore.py content-hash +# dedup truncation). +_REQUEST_ID_MAX = 80 + +# System-managed metadata keys are stripped before any client write (mirrors +# agentcore.py::_full_metadata_snapshot); no migrated record emits them. +_AGENTCORE_SYSTEM_METADATA_PREFIX = "x-amz-agentcore-memory-" + +# SQLite reflection.status -> AgentCore status. ``superseded`` has no target +# (skipped on create). ``retired`` maps for the status-transition path but is +# also skipped on *create* (§2: retired/superseded excluded from retrieval). +_STATUS_REMAP: dict[str, str] = { + "pending_review": "active", + "confirmed": "promoted", + "retired": "retired", +} + +# Statuses that are never *created* as a fresh migrated record (design §2). +_SKIP_ON_CREATE: frozenset[str] = frozenset({"retired", "superseded"}) + +_LEDGER_DDL = """ +CREATE TABLE IF NOT EXISTS agentcore_migration ( + source_kind TEXT NOT NULL, + source_id TEXT NOT NULL, + namespace TEXT NOT NULL, + target_record_id TEXT, + content_hash TEXT NOT NULL, + status TEXT NOT NULL, + last_error TEXT, + migrated_at TEXT, + PRIMARY KEY (source_kind, source_id) +) +""" + + +# --------------------------------------------------------------------------- # +# Row access helpers (tolerate sqlite3.Row, dict, or any Mapping) +# --------------------------------------------------------------------------- # +def _get(row: Any, key: str, default: Any = None) -> Any: + """Column access that works for sqlite3.Row, dict, and Mapping.""" + try: + val = row[key] + except (KeyError, IndexError): + return default + return default if val is None else val + + +def _scope(row: Any) -> str: + scope = _get(row, "scope", "project") + return "general" if scope == "general" else "project" + + +def _namespace_for(row: Any, kind: Literal["reflections", "semantic"]) -> str: + """Resolve the target namespace for a reflection/semantic row. + + ``kind`` is the session-namespace kind: ``'reflections'`` or ``'semantic'``. + """ + if _scope(row) == "general": + return resolve_namespace("general", kind) + actor = resolve_actor_id(_get(row, "project")) + return resolve_namespace(actor, kind) + + +def _decode_hints(raw: Any) -> list[Any]: + """Return hints as a list (SQLite stores a JSON-encoded list, §retrieve).""" + if isinstance(raw, list): + return raw + if isinstance(raw, str): + if not raw.strip(): + return [] + try: + parsed = json.loads(raw) + except json.JSONDecodeError: + return [raw] + return parsed if isinstance(parsed, list) else [parsed] + return [] + + +def remap_status(sqlite_status: str) -> str | None: + """Map a SQLite reflection status to its AgentCore equivalent. + + ``superseded`` -> ``None`` (no target; skipped). See §3 status remap. + """ + return _STATUS_REMAP.get(sqlite_status) + + +def _max_last_credited_at(row: Any) -> str | None: + """Collapse the three per-class rating timestamps into one (§3, §3.2). + + Falls back to ``updated_at`` so the declared stringValue is always present. + """ + candidates = [ + _get(row, "last_useful_at"), + _get(row, "last_misled_at"), + _get(row, "last_overlooked_at"), + ] + stamps = [str(c) for c in candidates if c] + if stamps: + return max(stamps) + updated = _get(row, "updated_at") + return str(updated) if updated else None + + +# --------------------------------------------------------------------------- # +# Record builders (pure) +# --------------------------------------------------------------------------- # +def build_reflection_record( + row: Any, + *, + strategy_id: str, + timestamp: datetime | None = None, +) -> dict[str, Any] | None: + """Build the ``batch_create_memory_records`` payload for one reflection row. + + Returns ``None`` for ``retired`` / ``superseded`` rows (skipped on create, + §2). ALL reflection state lives in the JSON content body (design §1b); NO + custom metadata map is emitted (AWS would silently drop it on the episodic + namespace). + """ + status = _get(row, "status") + if status in _SKIP_ON_CREATE: + return None + + sqlite_id = _get(row, "id") + body: dict[str, Any] = { + "title": _get(row, "title", ""), + "use_cases": _get(row, "use_cases", ""), + "hints": _decode_hints(_get(row, "hints")), + "confidence": _get(row, "confidence"), + "tech": _get(row, "tech"), + "phase": _get(row, "phase"), + "evidence_count": _get(row, "evidence_count", 0), + "updated_at": _get(row, "updated_at"), + "polarity": _get(row, "polarity", "neutral"), + "status": remap_status(status), + "useful_count": _get(row, "useful_count", 0), + "times_misled": _get(row, "times_misled", 0), + "times_overlooked": _get(row, "times_overlooked", 0), + "source_row_id": str(sqlite_id), + "source_backend": "sqlite", + } + + return { + "requestIdentifier": f"bm-reflection-{sqlite_id}"[:_REQUEST_ID_MAX], + "namespaces": [_namespace_for(row, "reflections")], + # Canonical (sorted) body so the payload — and its content_hash — is + # deterministic across runs. + "content": {"text": json.dumps(body, sort_keys=True)}, + "timestamp": timestamp or datetime.now(UTC), + "memoryStrategyId": strategy_id, + # NO metadata map — everything is in the body (design §1b). + } + + +def build_semantic_record( + row: Any, + *, + strategy_id: str, + timestamp: datetime | None = None, +) -> dict[str, Any]: + """Build the ``batch_create_memory_records`` payload for one semantic row. + + ``content.text`` is the user's raw preference text (NOT JSON). Idempotency + + counters live in a DECLARED metadata map (userPreference strategy honours + its ``memoryRecordSchema``). ``source_row_id`` MUST be a declared key or AWS + drops it (design §1b probe 3). + """ + sqlite_id = _get(row, "id") + + metadata: dict[str, dict[str, Any]] = { + "useful_count": {"numberValue": int(_get(row, "useful_count", 0))}, + "times_misled": {"numberValue": int(_get(row, "times_misled", 0))}, + # SQLite ``times_overlooked`` -> declared key ``overlooked_count`` (§3.2). + "overlooked_count": {"numberValue": int(_get(row, "times_overlooked", 0))}, + "source_row_id": {"stringValue": str(sqlite_id)}, + "status": {"stringValue": "active"}, + } + # last_credited_at MUST be a stringValue, not dateTimeValue — it is a + # declared STRING indexed key; a dateTimeValue rejects the whole record + # (§3, agentcore.py:616-621 landmine). + credited = _max_last_credited_at(row) + if credited is not None: + metadata["last_credited_at"] = {"stringValue": credited} + + return { + "requestIdentifier": f"bm-semantic-{sqlite_id}"[:_REQUEST_ID_MAX], + "namespaces": [_namespace_for(row, "semantic")], + "content": {"text": _get(row, "content", "")}, + "timestamp": timestamp or datetime.now(UTC), + "memoryStrategyId": strategy_id, + "metadata": metadata, + } + + +# --------------------------------------------------------------------------- # +# Batching (§6.4 — the missing wrapper) +# --------------------------------------------------------------------------- # +def chunk( + records: Sequence[dict[str, Any]], batch_size: int +) -> Iterator[list[dict[str, Any]]]: + """Yield ``records`` in lists of at most ``batch_size`` (>=1).""" + if batch_size < 1: + raise ValueError(f"batch_size must be >= 1, got {batch_size}") + for start in range(0, len(records), batch_size): + yield list(records[start : start + batch_size]) + + +def push_batch( + data_client: Any, + memory_id: str, + records: Sequence[dict[str, Any]], +) -> dict[str, Any]: + """Write one batch via ``batch_create_memory_records``; return the response. + + Does not raise on ``failedRecords`` — partial-failure handling belongs to + the caller (design §8: no rollback, per-row ledger). + """ + return data_client.batch_create_memory_records( + memoryId=memory_id, + records=list(records), + ) + + +# --------------------------------------------------------------------------- # +# Deterministic content hash (§5.2 — sorted keys, timestamp excluded) +# --------------------------------------------------------------------------- # +def canonical_content_hash(record: Mapping[str, Any]) -> str: + """SHA-256 of the canonical (body+metadata) payload, sorted keys. + + The volatile ``timestamp`` is excluded so re-running the same source row + yields the *same* hash (the ledger's skip/update decision depends on this). + ``memoryStrategyId`` is ALSO excluded: it is a routing/target attribute, not + migrated content, and it is only known after provisioning. Excluding it keeps + the hash stable across the ``--provision`` path — records are planned with a + placeholder strategy id, then re-keyed to the real id before the write (see + ``_rekey_strategy``); if the strategy id fed the hash, the ledger would store + a hash-of-placeholder that never matches a later run's hash-of-real-id, + forcing a spurious ``update`` on EVERY subsequent run (§5.2 idempotency). + ``requestIdentifier``, ``content``, ``metadata`` and ``namespaces`` are all + deterministic, so the hash changes iff the migrated content changes. + """ + material = { + k: v + for k, v in record.items() + if k not in ("timestamp", "memoryStrategyId") + } + canonical = json.dumps( + material, sort_keys=True, separators=(",", ":"), default=str + ) + return hashlib.sha256(canonical.encode("utf-8")).hexdigest() + + +# --------------------------------------------------------------------------- # +# Migration ledger (in the SOURCE sqlite db) — §5.2 +# --------------------------------------------------------------------------- # +def ensure_ledger(conn: sqlite3.Connection) -> None: + """Lazily create the ``agentcore_migration`` ledger table (idempotent).""" + conn.execute(_LEDGER_DDL) + conn.commit() + + +def plan_row( + conn: sqlite3.Connection, + kind: str, + source_id: Any, + content_hash: str | None, +) -> str: + """Decide what to do with one source row this run. + + Pass ``content_hash=None`` to signal the source row is no longer eligible + (retired/superseded in SQLite) — that drives the retire transition. + + Returns one of ``'create' | 'update' | 'skip' | 'retire'``. + """ + existing = conn.execute( + "SELECT content_hash, status FROM agentcore_migration " + "WHERE source_kind = ? AND source_id = ?", + (kind, str(source_id)), + ).fetchone() + + if content_hash is None: + # Source row no longer eligible. + if existing is None: + return "skip" # never migrated -> nothing to converge + if existing[1] == "retired": + return "skip" # already converged + return "retire" + + if existing is None: + return "create" + existing_hash, status = existing[0], existing[1] + if status in ("pending", "failed"): + # Never successfully written; (re)create. + return "create" + if status == "migrated" and existing_hash == content_hash: + return "skip" + return "update" + + +def record_success( + conn: sqlite3.Connection, + *, + kind: str, + source_id: Any, + namespace: str | None = None, + content_hash: str | None = None, + target_record_id: str | None = None, + status: str = "migrated", +) -> None: + """Upsert a successful (or retired) ledger row. + + Missing ``namespace`` / ``content_hash`` / ``target_record_id`` on an + existing entry are preserved (used by the retire transition, which only + flips status). + """ + now = datetime.now(UTC).isoformat() + # Update-then-insert (not ON CONFLICT): SQLite validates the proposed + # INSERT row's NOT NULL constraints *before* routing to the conflict + # clause, so a retire transition (namespace/content_hash omitted) cannot + # ride the upsert. Explicit UPDATE lets absent fields COALESCE to the + # existing value; the INSERT branch runs only for genuinely new rows, + # where create/update callers always supply both required columns. + cur = conn.execute( + """ + UPDATE agentcore_migration SET + namespace = COALESCE(?, namespace), + target_record_id = COALESCE(?, target_record_id), + content_hash = COALESCE(?, content_hash), + status = ?, + last_error = NULL, + migrated_at = ? + WHERE source_kind = ? AND source_id = ? + """, + (namespace, target_record_id, content_hash, status, now, + kind, str(source_id)), + ) + if cur.rowcount == 0: + conn.execute( + """ + INSERT INTO agentcore_migration + (source_kind, source_id, namespace, target_record_id, + content_hash, status, last_error, migrated_at) + VALUES (?, ?, ?, ?, ?, ?, NULL, ?) + """, + (kind, str(source_id), namespace or "", target_record_id, + content_hash or "", status, now), + ) + conn.commit() + + +def reconcile_ledger( + conn: sqlite3.Connection, + *, + kind: str, + source_id: Any, + namespace: str, + target_record_id: str, +) -> bool: + """Reattach a remote record's server id to the ledger (§5.3 safety net). + + Client-side reconcile-by-``source_row_id``: when a run scans a target + namespace and finds a record whose ``source_row_id`` has no + ``target_record_id`` in the local ledger (lost or absent ledger), this + binds the found ``target_record_id`` so the subsequent plan yields + ``update`` instead of ``create`` — preventing the duplicate records that a + ledger-loss ``create`` would produce (design §4 ``--restart`` contract, + §5.3). Returns ``True`` if it reattached, ``False`` if the ledger already + had a target (nothing to do). + + ``content_hash`` is written empty so the next ``plan_row`` re-verifies the + reattached record via an idempotent ``update`` (the migrated payload's real + hash cannot match ``''``); we never reconstruct the remote hash. An existing + target is never overwritten. + """ + existing = conn.execute( + "SELECT target_record_id FROM agentcore_migration " + "WHERE source_kind = ? AND source_id = ?", + (kind, str(source_id)), + ).fetchone() + if existing is not None and existing[0]: + return False + + now = datetime.now(UTC).isoformat() + cur = conn.execute( + """ + UPDATE agentcore_migration SET + namespace = COALESCE(?, namespace), + target_record_id = ?, + status = 'migrated', + last_error = NULL, + migrated_at = ? + WHERE source_kind = ? AND source_id = ? + """, + (namespace, target_record_id, now, kind, str(source_id)), + ) + if cur.rowcount == 0: + conn.execute( + """ + INSERT INTO agentcore_migration + (source_kind, source_id, namespace, target_record_id, + content_hash, status, last_error, migrated_at) + VALUES (?, ?, ?, ?, '', 'migrated', NULL, ?) + """, + (kind, str(source_id), namespace or "", target_record_id, now), + ) + conn.commit() + return True + + +def record_failure( + conn: sqlite3.Connection, + *, + kind: str, + source_id: Any, + last_error: str, + namespace: str | None = None, + content_hash: str | None = None, +) -> None: + """Upsert a ``failed`` ledger row with ``last_error`` for a later resume.""" + cur = conn.execute( + """ + UPDATE agentcore_migration SET + namespace = COALESCE(?, namespace), + content_hash = COALESCE(?, content_hash), + status = 'failed', + last_error = ? + WHERE source_kind = ? AND source_id = ? + """, + (namespace, content_hash, last_error, kind, str(source_id)), + ) + if cur.rowcount == 0: + conn.execute( + """ + INSERT INTO agentcore_migration + (source_kind, source_id, namespace, target_record_id, + content_hash, status, last_error, migrated_at) + VALUES (?, ?, ?, NULL, ?, 'failed', ?, NULL) + """, + (kind, str(source_id), namespace or "", + content_hash or "", last_error), + ) + conn.commit() diff --git a/docs/superpowers/specs/2026-07-13-sqlite-agentcore-migration-design.md b/docs/superpowers/specs/2026-07-13-sqlite-agentcore-migration-design.md new file mode 100644 index 0000000..0f27430 --- /dev/null +++ b/docs/superpowers/specs/2026-07-13-sqlite-agentcore-migration-design.md @@ -0,0 +1,287 @@ +# Design: `better-memory agentcore migrate` — Idempotent SQLite → AgentCore Migration + +## 1. Overview + +`better-memory agentcore migrate` promotes an existing local SQLite knowledge base into an already-provisioned (or freshly provisioned) AWS Bedrock AgentCore memory, so that a user who has been running the default `sqlite` backend can switch `storage_backend` to `agentcore` without losing curated reflections and semantic memories. + +The command replaces the current stub. Today `migrate-from-sqlite` is registered in `add_subparsers` (`cli/agentcore.py:75-78`) but `_handle_migrate` (`cli/agentcore.py:614-621`) raises `NotImplementedError` and takes no arguments. + +The hard problem is **not** the write — it is guaranteeing that after the write, the *existing* AgentCore read path (`_fetch_reflection_buckets` → `_parse_reflection_record`, `agentcore.py:235-442`) surfaces each migrated reflection byte-for-byte the way `ReflectionSynthesisService.retrieve_reflections` (`reflection.py:1225-1258`) surfaces the SQLite original: same 11-key dict, same bucketing, same ranking, same counters. Two required fields (`evidence_count`, `updated_at`) do **not** round-trip through the current reader and force concrete backend edits (Section 7). + +A second hard constraint: **better-memory has no code path that creates a reflection record.** Reflections are produced only by AWS's `episodicMemoryStrategy` extraction (`supports_synthesis=False`, `agentcore.py:80-81`); better-memory only *mutates* their metadata after the fact (`credit_one`, `record_use`, `_mutate_namespace_and_status`). The migrator is therefore the first-ever writer of reflection records and must build them directly via `batch_create_memory_records` into the reflections namespace the reader scans. + +Design principle for idempotency: **stable source id + deterministic requestIdentifier + local ledger + client-side reconcile-by-source-row-id**, never rollback. This is the opposite of `init`'s failure model, which rolls back all `created_ids` via `delete_memory` on any exception (`agentcore.py:317-360`). + +--- + +## 1b. REVISION 2026-07-13 — validated architecture (SUPERSEDES the metadata plane in §3/§6) + +Three live probes against real AWS (acct 708306701628, eu-west-2, throwaway memories, torn down) invalidated the original assumption that client-authored records can use the AgentCore **metadata** plane. Corrected facts: + +1. **Client BASE writes are accepted and the JSON content body round-trips perfectly**, but on the **episodic/reflections** namespace the **entire custom metadata map is silently dropped** (only `x-amz-*` system keys survive). Metadata retention is **schema-gated by the strategy owning the namespace**: episodic declares its schema under `reflectionConfiguration.memoryRecordSchema`, which governs only AWS-**extracted** records — client BASE writes get nothing. `batch_update_memory_records` to add metadata is a silent no-op. +2. **Content-BODY updates on a client-authored BASE record DO persist durably** (confirmed by successive updates via read-your-write GET). +3. **userPreference (semantic) declares a top-level `memoryRecordSchema`** that DOES govern client BASE writes: keys **declared** in it are retained on create, survive updates, and are listable. **Undeclared** keys (e.g. `source_row_id`) are silently dropped. +4. A **non-strategy namespace** (no `memoryStrategyId`) retains full client metadata but records are **not listable** (`list_memory_records` → `[]`) → dead end for retrieval. `customMemoryStrategy` needs `memoryExecutionRoleArn` + SNS + S3 → too heavy. + +**Resulting architecture (hybrid):** + +- **Reflections → Option A: all state in the JSON content body.** The body carries `title, use_cases, hints, confidence, tech, phase, evidence_count, updated_at` **and** the fields §3 put in metadata: `polarity, status, useful_count, times_misled, times_overlooked, source_row_id, source_backend`. `_parse_reflection_record` reads **body-first with metadata fallback** — AWS-extracted records keep working via metadata; migrated records resolve from the body. No metadata map is written for migrated reflections. +- **Reflection rating on migrated records rewrites the BODY, not metadata.** `credit_one` / `record_use` / `_mutate_namespace_and_status` currently write metadata (a no-op on client records). For records carrying `source_backend='sqlite'` in the body, the mutate path must read-modify-write the content body instead. This changes existing agentcore rating behavior for migrated records only (extracted records unchanged) — a backend edit beyond §6. +- **Semantic → declared metadata.** Extend the userPreference `memoryRecordSchema` to **declare** every migration field (notably `source_row_id`; counters `useful_count/times_misled/overlooked_count` are already declared). Semantic content stays the user's text (not JSON), so the idempotency key MUST be declared metadata. Open item: whether the already-provisioned real semantic memory's strategy schema can be widened via `update_memory_strategy` or requires re-provision — the `migrate`/`--provision` path must guarantee the target schema declares the needed keys before writing. +- **Idempotency reconcile:** reflections by `body.source_row_id` (client-side scan of the listable reflections namespace); semantic by declared-metadata `source_row_id`. Both listable, so §5.3 reconcile holds with the key relocated. +- **Rejected:** non-strategy namespace (not listable); `customMemoryStrategy` (heavy infra, no gain). + +§3's mapping table and §6's reader edits below remain the reference for field names and read-path citations, but read "metadata" as "content body" for every reflection field marked metadata there. Evidence: `scratchpad/probe*_out.txt`; memory ids `c588ca1c…`, `9bd09ba6…`. + +--- + +## 2. Scope + +### In scope (default) + +- **Reflections** — the primary target. All rows in `reflections` with `status IN ('pending_review','confirmed')` and (`project = ` OR `scope = 'general'`), matching the retrieval admission rule (`reflection.py:1208-1209`). Full metadata: `confidence, evidence_count, useful_count, times_misled, times_overlooked, polarity, phase, use_cases, hints, updated_at, title, tech` (Section 4). +- **Semantic memories** — all rows in `semantic_memories` (`0008_semantic_memories.sql:10-18`), with rating counters `useful_count, times_misled, times_overlooked` preserved. `semantic_observe` already exists (`agentcore.py:683-695`) but its `_semantic_initial_metadata` seeds every counter to 0 (`agentcore.py:646-655`); the migrator writes the real values. + +### Out of scope (default; `--include=observations` opt-in, flagged lossy) + +- **Observations.** In AgentCore an observation is an **episodic event** (`observe()` → `create_event`, `agentcore.py:181-196`), not a record. Three facts make migrating them near-useless: + 1. `list_observations` only reads *current-session* events via `list_events` and **cross-session enumeration is unsupported** (`agentcore.py:444-527`) — migrated events, written under a synthetic migration session id, would never be re-read. + 2. `reinforcement_score` has **no event-plane home** and is returned as a constant `None` (`agentcore.py:506`); SQLite's `reinforcement_score REAL NOT NULL` and all `validated_*`/`used_count`/`retrieved_count` counters are unrepresentable. + 3. AWS's `episodicMemoryStrategy` **re-extracts** reflections from events internally, so replaying raw observations risks duplicate/uncontrolled reflection extraction that collides with our explicitly-migrated reflections. + We therefore treat observations as source material already distilled into reflections. `--include=observations` remains available as a best-effort episodic replay but prints a loud "non-retrievable, lossy" warning. + +### Explicitly not migrated (SQLite-only structural columns, no AgentCore home) + +Named for completeness (Section 4 mapping table lists each): `reflections.project` (encoded via namespace/actor), `superseded_by`, `created_at`, `status_changed_at`, `scope` (encoded via namespace); the per-class rating timestamps `last_useful_at/last_misled_at/last_overlooked_at` (collapsed — see below); observation-only columns `session_id, trigger_type, scope_path, validated_true/false, retrieved_count, used_count, last_retrieved/used/validated`. `retired` and `superseded` reflections are skipped on first migration (they are excluded from retrieval anyway, `reflection.py:1208`); status *transitions* to retired on later runs are handled by the ledger diff (Section 6). + +--- + +## 3. Data-model mapping — the metadata round-trip (the crux) + +AgentCore stores two planes per reflection **record**: the **content body** (`record.content.text`, a JSON string) and the **metadata map** (`{key: {numberValue|stringValue|dateTimeValue}}`). Which plane a field lands in is dictated entirely by where the *reader* looks — parity is defined by `_parse_reflection_record` (`agentcore.py:356-442`), not by preference. + +Key reader facts that pin the mapping: +- `confidence` is read from `json.loads(rec['content']['text'])['confidence']` — **content body only** (`agentcore.py:396-399`). There is no `confidence` key in `EPISODIC_METADATA_SCHEMA` (`_agentcore_strategies.py:11-40`). Putting confidence in metadata would be invisible. +- `title, use_cases, hints, phase, tech` are all parsed from the content body (`agentcore.py:381-397`). +- `polarity` is read from **metadata** `stringValue 'polarity'` into internal `_polarity`, used only as the bucket selector and stripped from the public dict (`agentcore.py:414, 332, 350-353`). `status` is **metadata** `stringValue 'status'` (`agentcore.py:417`). +- `useful_count`, `times_misled` are read from **metadata** `numberValue` (`agentcore.py:428-429`) — these two round-trip cleanly. +- `times_overlooked` has a **name mismatch**: SQLite column `times_overlooked` (`0010_overlooked_rating.sql:42`) ↔ AgentCore metadata key `overlooked_count` (`agentcore.py:438`, `_agentcore_strategies.py:37`). Read into `_overlooked_count`, ranking-only, stripped from the dict. +- `evidence_count` is **computed** `int(useful_count)+int(missed_count)` (`agentcore.py:430`), NOT stored — a parity gap (Section 7). +- `updated_at` is **derived** from system metadata `x-amz-agentcore-memory-updatedAt` (dateTimeValue), falling back to `createdAt` (`agentcore.py:401-408`) — better-memory cannot set the system key, a parity gap (Section 7). +- Per-class rating timestamps collapse to one metadata `stringValue 'last_credited_at'` (`agentcore.py:619, 1087`). **It MUST be `stringValue`, not `dateTimeValue`** — it is a declared STRING indexed key; a `dateTimeValue` fails the whole record update with "value type does not match declared indexed key type" (`agentcore.py:616-621`, the live root cause of the `applied:0` bug). + +### 3.1 Reflection mapping table + +| SQLite field (source) | AgentCore location | Wire type | Read-path surfacing (public dict key) | +|---|---|---|---| +| `title` | content body `.title` | JSON string | `title` (`agentcore.py:381-424`) | +| `use_cases` | content body `.use_cases` | JSON string | `use_cases` | +| `hints` (JSON-encoded list) | content body `.hints` | JSON list | `hints` (list, `agentcore.py:428`) | +| `confidence` | content body `.confidence` | JSON number | `confidence` (`agentcore.py:396-399`) | +| `tech` | content body `.tech` | JSON string | `tech` | +| `phase` | content body `.phase` | JSON string | `phase` | +| `evidence_count` | content body `.evidence_count` **(NEW field, requires reader edit)** | JSON number | `evidence_count` (after edit at `agentcore.py:430`) | +| `updated_at` | content body `.updated_at` **(NEW field, requires reader edit)** | JSON string | `updated_at` (after edit at `agentcore.py:401-408`) | +| `polarity` | metadata `polarity` | stringValue | bucket key `do`/`dont`/`neutral` (`agentcore.py:414, 332`) | +| `status` (remapped) | metadata `status` | stringValue (INDEXED) | filter, default active (`agentcore.py:417`) | +| `useful_count` | metadata `useful_count` | numberValue | `useful_count` (`agentcore.py:428`) | +| `times_misled` | metadata `times_misled` | numberValue | `times_misled` (`agentcore.py:429`) | +| `times_overlooked` | metadata `overlooked_count` **(renamed)** | numberValue | `_overlooked_count` → ranking only (`agentcore.py:438`) | +| — (derive `0`) | metadata `missed_count` | numberValue | feeds evidence_count fallback only (`agentcore.py:430`) | +| — (derive `0`) | metadata `ignored_count` | numberValue | stored, read nowhere | +| `max(last_useful_at,last_misled_at,last_overlooked_at)` | metadata `last_credited_at` | **stringValue** (INDEXED) | not surfaced; MUST be stringValue (`agentcore.py:616-621`) | +| `id` (sqlite PK) | metadata `source_row_id` **(NEW)** + `requestIdentifier` | stringValue | reconcile/idempotency (client-side scan) | +| — | metadata `source_backend='sqlite'` **(NEW)** | stringValue | reconcile filter | +| `project` | encoded in namespace/actor (`resolve_actor_id`) | — | not a field | +| `scope` | encoded in namespace choice | — | not a field | +| `superseded_by`, `created_at`, `status_changed_at` | — | — | not migrated (Section 2) | + +**Namespace + strategy:** project-scoped reflections → `projects/{actor}/reflections/` (`resolve_actor_id(project)`); `scope='general'` → `general/reflections/`. These are exactly the two namespaces `_fetch_reflection_buckets` lists (`agentcore.py:235-354`). Each record supplies `memoryStrategyId` = the episodic strategy id from `agentcore.json`. + +**Status remap** (SQLite `{pending_review,confirmed,retired,superseded}` → AgentCore `{active,promoted,retired}`, `agentcore.py:273-276,417`): `pending_review → active`, `confirmed → promoted`, `retired → retired`, `superseded → skip`. + +### 3.2 Semantic mapping table + +| SQLite field | AgentCore location | Read surfacing | +|---|---|---| +| `content` | content `.text` | `content` | +| `scope` | namespace (`projects/{actor}/...` vs `general/...`) | — | +| `useful_count` | metadata `useful_count` numberValue | (reader edit — Section 7.3) | +| `times_misled` | metadata `times_misled` numberValue | (reader edit) | +| `times_overlooked` | metadata `overlooked_count` numberValue (renamed) | ranking | +| `last_*_at` (3) | metadata `last_credited_at` stringValue | — | +| `created_at`/`updated_at` | content body `.created_at`/`.updated_at` | (reader edit) | +| `id` | metadata `source_row_id` + `requestIdentifier` | idempotency | + +--- + +## 4. CLI surface + +``` +better-memory agentcore migrate [flags] +``` + +| Flag | Default | Behavior | +|---|---|---| +| `--dry-run` | off | Read SQLite + ledger, compute the plan (create / update / skip / delete-transition counts per kind and namespace), print it, make **zero** AWS calls. | +| `--include=` | `reflections,semantic` | Comma list of `reflections`, `semantic`, `observations`. `observations` prints the lossy/non-retrievable warning. | +| `--restart` | off | Ignore the ledger's "migrated" state and re-verify/upsert every eligible row (still idempotent via `source_row_id` reconcile). Default (no flag) is resume-safe. | +| `--provision` | off | If target memories are missing/not ACTIVE, create them by reusing `init`'s `_create_one_memory` + `_poll_until_active` path; else abort with guidance. Without the flag, missing memories → hard error. | +| `--db ` | `/memory.db` | Source SQLite path. | +| `--project ` | all projects in db | Limit migration to one project. | +| `--region ` | `cfg.region` from `agentcore.json` else `--region` | Same resolution as `status`/`smoke` (`agentcore.py:442,485`); region lives in `agentcore.json`, not env (`factory.py:76-82`). | +| `--home ` | `--home` else `$BETTER_MEMORY_HOME` else `~/.better-memory` | Same `_resolve_home` as init (`agentcore.py:94-98`). | +| `--batch-size ` | `25` (conservative; API limit undocumented in-repo — Section 10) | Records per `batch_create_memory_records` call. | +| `--verify` | off | After writes, read a sample (or all) back via `get_memory_record` using smoke's read-your-write retry (`ResourceNotFoundException` ×5 / 2s, `agentcore.py` smoke path) and diff against the SQLite source. | + +Exit codes: `0` full convergence; `2` partial (some records failed, ledger records them for resume); `1` fatal (no config, not provisioned, auth failure). Auth failures surface as `1` by building the data client inside the try, mirroring smoke (`agentcore.py:473-611`). + +--- + +## 5. Idempotency & resumability + +### 5.1 Stable id → deterministic requestIdentifier + +Each record's `requestIdentifier = f"bm-{kind}-{sqlite_id}"` truncated to the 80-char AWS limit (mirrors the existing content-hash dedup at `agentcore.py:679-681`, but keyed on the **row id** so the same row always maps to the same identifier). `requestIdentifier` is correlation/dedup only and the **server mints its own `mem-` id** (`agentcore.py`, smoke; `memoryRecordId` is not a valid input key). AWS dedup on `requestIdentifier` is short-window only, so it is a *first line* of defense, not the source of truth. + +### 5.2 Local ledger (source of truth) + +A new SQLite table in the **source** db (created lazily by migrate; never touched by the normal backends): + +```sql +CREATE TABLE IF NOT EXISTS agentcore_migration ( + source_kind TEXT NOT NULL, -- 'reflection' | 'semantic' + source_id TEXT NOT NULL, -- sqlite row id (stable) + namespace TEXT NOT NULL, + target_record_id TEXT, -- server-minted mem-, NULL until first success + content_hash TEXT NOT NULL, -- sha256 of the canonical (body+metadata) payload + status TEXT NOT NULL, -- 'pending' | 'migrated' | 'failed' | 'retired' + last_error TEXT, + migrated_at TEXT, + PRIMARY KEY (source_kind, source_id) +); +``` + +Per-row decision on every run: +- **No ledger entry** → CREATE via `batch_create_memory_records`; on success record `target_record_id` + `content_hash` + `status='migrated'`. +- **Entry exists, `content_hash` unchanged, `status='migrated'`** → SKIP (converged). +- **Entry exists, `content_hash` changed** → UPDATE via `batch_update_memory_records` (`agentcore.py:625-636` shape: `{memoryRecordId, timestamp, metadata:full_snapshot, [content], [namespaces]}`, `_full_metadata_snapshot` strips `x-amz-*` keys, `agentcore.py:569-595`). Refresh hash. +- **SQLite status now `retired`/`superseded` but ledger `migrated`** → status-transition UPDATE to `status='retired'` metadata (converges retirements), mark ledger `retired`. +- **`status='failed'`** → retry (same as create/update). + +This makes re-runs idempotent and convergent regardless of AWS dedup behavior. + +### 5.3 Reconcile-by-metadata (ledger-loss safety net) + +Because `source_row_id` is written into metadata, a `--restart` (or a lost ledger) can rebuild state: list the two reflection namespaces client-side, index existing records by `metadata.source_row_id`, and reattach `target_record_id`. `source_row_id` is **not** a server-indexed key (indexed keys are fixed at provision time to `status, last_credited_at, overlooked_count`, `_agentcore_strategies.py:52-56` — cannot be extended post-hoc), so this reconcile is a client-side scan, acceptable for a maintenance command. + +### 5.4 Resume & dry-run + +- **Resume** is the default: the ledger already encodes progress; a crashed run leaves `migrated` rows skipped and `pending`/`failed` rows retried. No cursor needed beyond the ledger. +- **Dry-run** walks the same decision logic and prints the create/update/skip/retire tallies without any AWS call. +- A single-process **advisory lock** (a `agentcore_migration_lock` row / file in ``) prevents concurrent runs from double-creating within the AWS dedup blind spot. + +--- + +## 6. Backend changes required for read parity + +Migration is not just a writer; it must guarantee the reader surfaces every field. Concrete edits: + +### 6.1 `storage/agentcore.py :: _parse_reflection_record` — `evidence_count` (`agentcore.py:430`) + +Currently: `evidence_count = int(useful_count) + int(missed_count)`. +Change to prefer the stored value when present: + +```python +body_ec = body.get("evidence_count") +evidence_count = int(body_ec) if body_ec is not None else int(useful_count) + int(missed_count) +``` + +Locks parity with SQLite's stored, synthesis-recomputed `evidence_count` (`reflection.py:858,1254`), which otherwise diverges whenever `evidence_base != useful+missed` (scout gap). AWS-native extracted records (no `evidence_count` in body) keep the old computed fallback — no regression. + +### 6.2 `storage/agentcore.py :: _parse_reflection_record` — `updated_at` (`agentcore.py:401-408`) + +Prefer the migrated body value before the system-metadata fallback: + +```python +updated_at = body.get("updated_at") or _from_system_updated_at(rec) or rec.get("createdAt") +``` + +Without this, migrated reflections report **creation** time as update time, corrupting `age_days` in `relevant.py:103` / bootstrap and drifting the ranking tiebreak (`ORDER BY ... updated_at DESC`). Native records (no body `updated_at`) keep today's behavior. + +### 6.3 Semantic read parity — `semantic_list` (`agentcore.py:742-757`) and/or `relevant.py:112-125` + +`semantic_list` returns plain dicts `{id, content, namespaces, scope}` with **no counters**, but `relevant.py:112-125` uses attribute access (`getattr(s,'useful_count'...)`) that only works on SQLite's `SemanticMemory` dataclass — on dicts it silently returns defaults, so `content=''` and counters `0`, making semantic injection dead in agentcore mode (scout gap). Fix: have `semantic_list` return objects matching the `SemanticMemory` shape (`semantic.py:22-37`) populated from the metadata counters the migrator now writes, and surface `useful_count/times_misled/times_overlooked/updated_at`. (Required only if `semantic` is in `--include`; guard the change with a semantic round-trip test.) + +### 6.4 New writer module `better_memory/storage/agentcore_migrate.py` + +- `build_reflection_record(row) -> dict` — assembles content-body JSON (`title, use_cases, hints, confidence, tech, phase, evidence_count, updated_at`) + metadata map (polarity/status/counters/`last_credited_at` stringValue/`source_row_id`/`source_backend`) + `requestIdentifier`. Reuses the `_full_metadata_snapshot` coercion + `x-amz-*` stripping pattern (`agentcore.py:569-595`). +- `build_semantic_record(row)` — analogous, reusing `semantic_observe`'s per-record shape (`agentcore.py:683-695`) but with real counters instead of `_semantic_initial_metadata` zeros. +- `chunk(records, batch_size)` + `push_batch(data_client, records)` — the missing batching helper; existing call sites all pass `records=[]` (`agentcore.py:685,545`). + +### 6.5 CLI `_handle_migrate` (`cli/agentcore.py:614-621`) + argparse + +Replace `NotImplementedError`; add the Section-4 flags to the `add_subparsers` registration (`agentcore.py:75-78`). Reuse `load_agentcore_config`, `_build_data_client`/`_build_control_client` (`agentcore.py:154-170`), `_resolve_home`, and `_find_existing_memory` (`agentcore.py:201-214`). + +--- + +## 7. Auth & provisioning reuse + +**Auth is entirely ambient** — nothing to add. `_build_data_client`/`_build_control_client` (`agentcore.py:154-170`) and the runtime factory (`factory.py:79-84`) call `boto3.client(...)` with only `BotoConfig(region_name=..., retries={mode:standard, max_attempts:5})` and no credential args. Credentials come from boto3's default provider chain (env `AWS_ACCESS_KEY_ID/SECRET`, shared profile, IMDS/role). The IAM-user model the requirement assumes is exactly what already works; migrate reuses the same clients and the same retries config. + +**Provisioning:** migrate must verify or create the targets, not assume them. +1. Load `agentcore.json` via `load_agentcore_config` (schema-pinned to `schema_version==1`, `persistence.py:91-97`); if absent → abort unless `--provision`. +2. For each of episodic + semantic memory, call `_find_existing_memory` (`agentcore.py:201-214`, paginates `list_memories`, skips DELETING) and confirm ACTIVE via `get_memory` / the `_poll_until_active` readiness rule (`agentcore.py:173-198`: memory ACTIVE **and** every strategy ACTIVE). +3. If missing/not-ACTIVE and `--provision`: run init's `_create_one_memory` + `_poll_until_active` for the missing memory only, then persist via `save_agentcore_config` (atomic tmp+rename, `agentcore_persistence.py:59-69`). Do **not** touch `settings.json` `storage_backend` — flipping the backend is init/`--activate`'s job, not migrate's. +4. Region/home resolution identical to init (`agentcore.py:94-98`). + +--- + +## 8. Error handling & partial-failure + +- **No rollback.** Unlike init (delete all `created_ids` on any error, `agentcore.py:317-360`), migrate never deletes on failure — it records per-row `failed` in the ledger and continues, so re-run resumes. +- **Batch partial failure.** `batch_create_memory_records` returns both `successfulRecords` and `failedRecords`; update the ledger per successful record (capture `memoryRecordId`), leave failed rows `failed` with `last_error`, continue to the next batch. +- **The `last_credited_at` landmine.** Always write it as `stringValue`; a `dateTimeValue` rejects the entire record update with "value type does not match declared indexed key type" (`agentcore.py:616-621`). A unit test asserts the wire type. +- **Throttling / bulk backoff.** Rely on botocore `standard/5` retries; add jittered exponential backoff on `ThrottlingException` around each batch since bulk load is heavier than any existing call site. +- **Read-your-write lag.** `--verify` uses smoke's pattern: `get_memory_record` is read-your-write (~1s) with `ResourceNotFoundException` retried ×5/2s, while `list_memory_records` lags ~60s (`agentcore.py` smoke) — verification must not use list for immediate readback. +- **Auth/region misconfig** → data client built inside the try → clean `rc=1` (smoke pattern, `agentcore.py:473-611`). + +--- + +## 9. Test plan + +### T1 — unit, hermetic (no AWS) + +- **Reflection mapping invariants** (`build_reflection_record`): `confidence`/`evidence_count`/`updated_at` land in the content body; `polarity`/`status`/counters land in metadata; `times_overlooked`→`overlooked_count` rename; the three `last_*_at` collapse to a single `last_credited_at` emitted as **stringValue**; `requestIdentifier` deterministic from row id; status remap `pending_review→active`, `confirmed→promoted`; `retired`/`superseded` skipped on create. +- **Round-trip parity (locks Requirement 4):** feed a record built by `build_reflection_record` into the *edited* `_parse_reflection_record` and assert the resulting 11-key dict `{id,title,phase,use_cases,hints,confidence,tech,evidence_count,useful_count,times_misled,updated_at}` **equals** `retrieve_reflections`' dict for the same source row (`reflection.py:1246-1258`). Assert bucket = polarity, and that the ranking key `(useful_count + 3*overlooked_count, confidence, updated_at)` matches SQLite's `ORDER BY` (`reflection.py:1232`, weight 3, `memory_rating.py:71`). +- **Reader edits:** body `evidence_count` preferred over `useful+missed`; body `updated_at` preferred over system metadata; native records (no body keys) keep legacy fallbacks (no regression). +- **Ledger idempotency:** run 2 with unchanged `content_hash` ⇒ 0 create/update calls; changed hash ⇒ exactly 1 update; SQLite-retire-after-migrate ⇒ 1 status-transition update. +- **Semantic parity:** `semantic_list` returns counter-bearing objects; `relevant.py` sees non-zero `useful_count`. + +### T2 — fake-endpoint (stubbed bedrock-agentcore data plane) + +Reuse `tests/e2e/_agentcore_env.py` scaffolding with a stub/`moto`-style data client. +- Full migrate run: assert `batch_create_memory_records` called with the expected chunked payloads (namespaces, `requestIdentifier`, body JSON, metadata types). +- Idempotent re-run: ledger short-circuits, zero creates. +- `--dry-run`: zero AWS calls, correct printed tallies. +- Partial failure: injected `failedRecords` ⇒ ledger marks them `failed`, re-run retries only those. +- `--verify`: readback diff passes. + +### T3 — live, env-gated (real AWS, behind `BETTER_MEMORY_E2E`) + +- Provision (or reuse) memories, migrate a small fixture db, then call `backend.retrieve(project, tech)` and assert buckets/order/counters/`confidence`/`evidence_count`/`updated_at` match the SQLite `retrieve_reflections` output for the same fixture — the end-to-end parity invariant. +- Re-run and assert `list_memory_records` count is stable (no duplicates) — the end-to-end idempotency invariant. +- **Landmine validation:** confirm client-authored records are *accepted* into the `episodicMemoryStrategy` reflections namespace and returned by `_fetch_reflection_buckets` (Section 10 risk). + +--- + +## 10. Open questions / risks + +1. **Can we write records into the extraction strategy's namespace?** The reader lists `projects/{actor}/reflections/` and `general/reflections/` (`agentcore.py:235-354`), namespaces owned by AWS's `episodicMemoryStrategy`. If `batch_create_memory_records` rejects client-authored records there (or AWS re-extraction later mutates/evicts them), we need a dedicated manual/custom strategy namespace *and* a corresponding second list namespace added to `_fetch_reflection_buckets`. **Highest-risk unknown; T3 must validate before build sign-off.** +2. **`batch_create_memory_records` per-call record limit** is undocumented in-repo (scout gap). `--batch-size` defaults conservative (25); T3 probes the real ceiling. +3. **`requestIdentifier` dedup horizon.** If AWS enforces dedup only briefly, the ledger + single-run lock are the sole guard against concurrent double-writes. +4. **`evidence_count` semantic drift.** SQLite `evidence_count` is a synthesis-recomputed source count (`reflection.py:858`); we migrate the stored value into the body (fixed by 6.1). AWS-native reflections still compute it — mixed populations are expected and acceptable. +5. **Per-class recency loss.** `last_useful_at/last_misled_at/last_overlooked_at` collapse into one `last_credited_at` (`agentcore.py:619`) — one-way, irreversible; documented. +6. **Native vs migrated duplication.** If AWS extracts a reflection semantically identical to a migrated one, dedup fails (native records carry no `source_row_id`). Mitigation: reconcile only owns `source_backend='sqlite'` records; native duplicates are out of migrate's authority. +7. **Indexed keys are frozen at provision.** `source_row_id` cannot be server-indexed post-hoc, so reconcile is a client-side scan — fine for a maintenance command, not for hot-path reads. +8. **Retirement convergence.** A reflection retired in SQLite *after* migration converges only if the user re-runs migrate; there is no push. Acceptable for a manual command; note in help text. \ No newline at end of file diff --git a/tests/cli/test_agentcore_migrate.py b/tests/cli/test_agentcore_migrate.py index 3d8347f..ca0c31c 100644 --- a/tests/cli/test_agentcore_migrate.py +++ b/tests/cli/test_agentcore_migrate.py @@ -1,18 +1,622 @@ -"""Tests for `better-memory agentcore migrate-from-sqlite` (stubbed).""" +"""Tests for `better-memory agentcore migrate` (T5). + +Hermetic: the AWS control + data planes are mocked. Proves the dry-run makes +zero AWS calls, the config/provisioning gates return the right exit codes, a +full run emits the expected batched payloads and writes the ledger, re-runs are +idempotent, and partial failures are recorded for resume (rc=2). +""" from __future__ import annotations import argparse +import json +from pathlib import Path import pytest -from better_memory.cli.agentcore import _handle_migrate +from better_memory.cli import agentcore as cli +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from better_memory.storage.agentcore_persistence import ( + AgentCoreConfig, + MemoryRecord, + save_agentcore_config, +) + + +# --------------------------------------------------------------------------- # +# Fixtures +# --------------------------------------------------------------------------- # +def _args(home: Path, **over) -> argparse.Namespace: + d = dict( + subcommand="migrate", + home=str(home), + dry_run=False, + include="reflections,semantic", + restart=False, + provision=False, + db=None, + project=None, + region=None, + batch_size=25, + verify=False, + ) + d.update(over) + return argparse.Namespace(**d) + + +def _write_config(home: Path) -> None: + cfg = AgentCoreConfig( + schema_version=1, + region="eu-west-2", + episodic=MemoryRecord( + memory_id="epi-X", + memory_arn="arn:epi", + memory_name="better_memory_episodic", + strategy_id="epi-strat", + strategy_name="episodicReflections", + event_expiry_duration_days=90, + ), + semantic=MemoryRecord( + memory_id="sem-X", + memory_arn="arn:sem", + memory_name="better_memory_semantic", + strategy_id="sem-strat", + strategy_name="userPreference", + event_expiry_duration_days=365, + ), + ) + save_agentcore_config(cfg, home) + + +def _seed_db(home: Path, *, reflections=1, semantic=1) -> Path: + db_path = home / "memory.db" + conn = connect(db_path) + try: + apply_migrations(conn) + for i in range(reflections): + conn.execute( + """ + INSERT INTO reflections + (id, title, project, tech, phase, polarity, use_cases, + hints, confidence, status, superseded_by, evidence_count, + created_at, updated_at, scope, useful_count, last_useful_at, + times_misled, last_misled_at, times_overlooked, + last_overlooked_at) + VALUES (?, ?, ?, ?, 'implementation', 'do', 'when x', ?, 0.8, + 'pending_review', NULL, 3, + '2026-07-01T00:00:00+00:00', '2026-07-10T00:00:00+00:00', + 'project', 2, '2026-07-09T00:00:00+00:00', + 1, '2026-07-08T00:00:00+00:00', 0, NULL) + """, + (f"refl-{i}", f"Title {i}", "better-memory", "python", + json.dumps([f"hint {i}"])), + ) + for i in range(semantic): + conn.execute( + """ + INSERT INTO semantic_memories + (id, content, project, scope, created_at, updated_at, + useful_count, last_useful_at, times_misled, last_misled_at, + times_overlooked, last_overlooked_at) + VALUES (?, ?, 'better-memory', 'project', + '2026-07-01T00:00:00+00:00', '2026-07-10T00:00:00+00:00', + 3, '2026-07-05T00:00:00+00:00', 0, NULL, 1, + '2026-07-06T00:00:00+00:00') + """, + (f"sem-{i}", f"Preference {i}"), + ) + conn.commit() + finally: + conn.close() + return db_path + + +def _active_memory(memory_id: str, name: str, *, schema_keys=None) -> dict: + strat: dict = {"strategyId": "strat", "status": "ACTIVE", "name": name} + if schema_keys is not None: + strat["configuration"] = { + "userPreferenceOverride": { + "memoryRecordSchema": { + "metadataSchema": [ + {"key": k, "type": "STRING"} for k in schema_keys + ] + } + } + } + return { + "id": memory_id, + "name": name, + "status": "ACTIVE", + "strategies": [strat], + "eventExpiryDuration": 90, + "arn": f"arn:{memory_id}", + } + + +class _Control: + """Control-plane mock: both memories ACTIVE; semantic declares source_row_id.""" + + def __init__(self, *, episodic_status="ACTIVE", semantic_schema=True, + widen_effective=True): + self._widen_effective = widen_effective + sem_keys = ( + ["useful_count", "source_row_id", "status"] + if semantic_schema + else ["useful_count", "status"] + ) + self._mems = { + "epi-X": { + **_active_memory("epi-X", "better_memory_episodic"), + "status": episodic_status, + "strategies": [ + {"strategyId": "epi-strat", "status": episodic_status, + "name": "episodicReflections"} + ], + }, + "sem-X": _active_memory( + "sem-X", "better_memory_semantic", schema_keys=sem_keys + ), + } + self.update_strategy_calls: list = [] + + def get_memory(self, *, memoryId): + return {"memory": self._mems[memoryId]} + + def update_memory_strategy(self, *, memoryId, memoryStrategyId, configuration, **kw): + self.update_strategy_calls.append( + {"memoryId": memoryId, "memoryStrategyId": memoryStrategyId, + "configuration": configuration, **kw} + ) + # Model a successful in-place widen: adopt the metadataSchema keys the + # caller declared so the post-update re-read confirms source_row_id. + # widen_effective=False models AWS accepting the call as a no-op (the + # key is NOT actually added) — the confirm re-read must then fail. + override = configuration.get("userPreferenceOverride", {}) + schema = (override.get("memoryRecordSchema") or {}).get("metadataSchema") + if schema is not None and self._widen_effective: + self._mems[memoryId]["strategies"][0]["configuration"] = { + "userPreferenceOverride": { + "memoryRecordSchema": {"metadataSchema": schema} + } + } + return {} + + +class _FakeData: + def __init__(self, fail_reqs=None, remote_records=None): + self.create_calls: list = [] + self.update_calls: list = [] + self.list_calls: list = [] + self._fail = set(fail_reqs or []) + # remote_records: {namespace: [record_summary, ...]} for reconcile scans. + self._remote = remote_records or {} + self._n = 0 + + def list_memory_records(self, *, memoryId, namespace, maxResults=100, + nextToken=None): + self.list_calls.append((memoryId, namespace)) + return { + "memoryRecordSummaries": list(self._remote.get(namespace, [])), + "nextToken": None, + } + + def batch_create_memory_records(self, *, memoryId, records): + self.create_calls.append((memoryId, records)) + successful, failed = [], [] + for r in records: + req = r["requestIdentifier"] + if req in self._fail: + failed.append({"requestIdentifier": req, "errorMessage": "boom"}) + else: + self._n += 1 + successful.append( + {"requestIdentifier": req, "memoryRecordId": f"mem-{self._n}"} + ) + return {"successfulRecords": successful, "failedRecords": failed} + + def batch_update_memory_records(self, *, memoryId, records): + self.update_calls.append((memoryId, records)) + return { + "successfulRecords": [ + {"memoryRecordId": r["memoryRecordId"]} for r in records + ], + "failedRecords": [], + } + + def get_memory_record(self, *, memoryId, memoryRecordId): + return { + "memoryRecord": { + "memoryRecordId": memoryRecordId, + "content": {"text": "{}"}, + } + } + + +def _ledger_rows(db_path: Path): + conn = connect(db_path) + try: + return conn.execute( + "SELECT source_kind, source_id, status, target_record_id " + "FROM agentcore_migration ORDER BY source_kind, source_id" + ).fetchall() + finally: + conn.close() + + +# --------------------------------------------------------------------------- # +# Tests +# --------------------------------------------------------------------------- # +def test_dry_run_makes_zero_aws_calls_with_tallies(tmp_path, monkeypatch, capsys): + _write_config(tmp_path) + _seed_db(tmp_path, reflections=2, semantic=1) + + def _boom(region): + raise AssertionError("dry-run must not build an AWS client") + + monkeypatch.setattr(cli, "_build_control_client", _boom) + monkeypatch.setattr(cli, "_build_data_client", _boom) + + rc = cli._handle_migrate(_args(tmp_path, dry_run=True)) + assert rc == 0 + out = capsys.readouterr().out + assert "dry-run" in out.lower() + # 2 reflection creates + 1 semantic create, all fresh. + assert "create=2" in out # reflections namespace line + assert "create=1" in out # semantic namespace line + # No ledger writes for a dry-run either (planning is read-only). + assert _ledger_rows(tmp_path / "memory.db") == [] + + +def test_missing_config_without_provision_rc1(tmp_path, capsys): + # No agentcore.json and no --provision -> rc 1. + rc = cli._handle_migrate(_args(tmp_path)) + assert rc == 1 + assert "agentcore.json" in capsys.readouterr().err + + +def test_missing_memory_without_provision_rc1(tmp_path, monkeypatch, capsys): + _write_config(tmp_path) + _seed_db(tmp_path) + monkeypatch.setattr( + cli, "_build_control_client", + lambda region: _Control(episodic_status="CREATING"), + ) + monkeypatch.setattr(cli, "_build_data_client", lambda region: _FakeData()) + + rc = cli._handle_migrate(_args(tmp_path)) + assert rc == 1 + assert "not ACTIVE" in capsys.readouterr().err + + +def test_full_run_creates_batched_payloads_and_writes_ledger( + tmp_path, monkeypatch +): + _write_config(tmp_path) + db_path = _seed_db(tmp_path, reflections=2, semantic=1) + control = _Control() + data = _FakeData() + monkeypatch.setattr(cli, "_build_control_client", lambda region: control) + monkeypatch.setattr(cli, "_build_data_client", lambda region: data) + + rc = cli._handle_migrate(_args(tmp_path)) + assert rc == 0 + + # Two batch_create calls: one against the episodic memory (reflections), + # one against the semantic memory. + called_mems = {mem for (mem, _recs) in data.create_calls} + assert called_mems == {"epi-X", "sem-X"} + + # Reflection payload: body carries the migration marker + namespace. + epi_records = next(recs for (mem, recs) in data.create_calls if mem == "epi-X") + assert len(epi_records) == 2 + body = json.loads(epi_records[0]["content"]["text"]) + assert body["source_backend"] == "sqlite" + assert epi_records[0]["namespaces"] == ["projects/better-memory/reflections/"] + assert epi_records[0]["requestIdentifier"].startswith("bm-reflection-") + + # Semantic payload: declared metadata carries source_row_id. + sem_records = next(recs for (mem, recs) in data.create_calls if mem == "sem-X") + assert sem_records[0]["metadata"]["source_row_id"] == {"stringValue": "sem-0"} + + # No in-place schema widening was needed (schema already declares the key). + assert control.update_strategy_calls == [] + + # Ledger: every eligible row migrated with a minted target id. + rows = _ledger_rows(db_path) + assert len(rows) == 3 + assert all(r["status"] == "migrated" for r in rows) + assert all(r["target_record_id"] for r in rows) + + +def test_rerun_is_idempotent_zero_creates(tmp_path, monkeypatch): + _write_config(tmp_path) + _seed_db(tmp_path, reflections=2, semantic=1) + control = _Control() + monkeypatch.setattr(cli, "_build_control_client", lambda region: control) + + first = _FakeData() + monkeypatch.setattr(cli, "_build_data_client", lambda region: first) + assert cli._handle_migrate(_args(tmp_path)) == 0 + assert len(first.create_calls) == 2 # populated the ledger + + # Second run: ledger short-circuits every row -> zero creates. + second = _FakeData() + monkeypatch.setattr(cli, "_build_data_client", lambda region: second) + rc = cli._handle_migrate(_args(tmp_path)) + assert rc == 0 + assert second.create_calls == [] + assert second.update_calls == [] + + +def test_partial_failure_records_failed_and_rc2(tmp_path, monkeypatch, capsys): + _write_config(tmp_path) + db_path = _seed_db(tmp_path, reflections=2, semantic=0) + control = _Control() + monkeypatch.setattr(cli, "_build_control_client", lambda region: control) + + # Fail exactly one reflection create. + data = _FakeData(fail_reqs={"bm-reflection-refl-1"}) + monkeypatch.setattr(cli, "_build_data_client", lambda region: data) + + rc = cli._handle_migrate(_args(tmp_path, include="reflections")) + assert rc == 2 + err = capsys.readouterr().err + assert "failed record" in err + + rows = {r["source_id"]: r["status"] for r in _ledger_rows(db_path)} + assert rows["refl-0"] == "migrated" + assert rows["refl-1"] == "failed" + + # Re-run retries ONLY the failed row. + retry = _FakeData() + monkeypatch.setattr(cli, "_build_data_client", lambda region: retry) + rc = cli._handle_migrate(_args(tmp_path, include="reflections")) + assert rc == 0 + retried_reqs = [ + r["requestIdentifier"] + for (_mem, recs) in retry.create_calls + for r in recs + ] + assert retried_reqs == ["bm-reflection-refl-1"] + + +# --------------------------------------------------------------------------- # +# Issue 1 — _ensure_semantic_schema widens the schema for real and confirms it +# --------------------------------------------------------------------------- # +def test_narrow_semantic_schema_widened_in_place_then_migrates( + tmp_path, monkeypatch +): + _write_config(tmp_path) + db_path = _seed_db(tmp_path, reflections=0, semantic=1) + # Semantic strategy does NOT declare source_row_id -> must be widened. + control = _Control(semantic_schema=False) + data = _FakeData() + monkeypatch.setattr(cli, "_build_control_client", lambda region: control) + monkeypatch.setattr(cli, "_build_data_client", lambda region: data) + + rc = cli._handle_migrate(_args(tmp_path, include="semantic")) + assert rc == 0 + + # update_memory_strategy was called with the FULL schema declaring + # source_row_id (not an empty customExtractionConfiguration). + assert len(control.update_strategy_calls) == 1 + sent = control.update_strategy_calls[0]["configuration"] + declared = { + e["key"] + for e in sent["userPreferenceOverride"]["memoryRecordSchema"]["metadataSchema"] + } + assert "source_row_id" in declared + + # The semantic row migrated (schema now declares the key). + rows = _ledger_rows(db_path) + assert len(rows) == 1 + assert rows[0]["status"] == "migrated" + + +def test_narrow_semantic_schema_widen_noop_aborts_rc1(tmp_path, monkeypatch): + _write_config(tmp_path) + _seed_db(tmp_path, reflections=0, semantic=1) + # AWS accepts the widen call but does NOT actually add the key (no-op). + control = _Control(semantic_schema=False, widen_effective=False) + data = _FakeData() + monkeypatch.setattr(cli, "_build_control_client", lambda region: control) + monkeypatch.setattr(cli, "_build_data_client", lambda region: data) + + rc = cli._handle_migrate(_args(tmp_path, include="semantic")) + # Confirm re-read still lacks source_row_id -> hard fail, no writes. + assert rc == 1 + assert data.create_calls == [] + + +def test_non_introspectable_semantic_schema_aborts_rc1(tmp_path, monkeypatch): + _write_config(tmp_path) + _seed_db(tmp_path, reflections=0, semantic=1) + + # A control whose semantic memory exposes NO metadataSchema at all + # (introspection returns None) AND whose widen is a no-op -> cannot verify. + class _NoSchemaControl(_Control): + def __init__(self): + super().__init__(widen_effective=False) + # Strip the schema so _collect_declared_metadata_keys returns None. + self._mems["sem-X"]["strategies"][0].pop("configuration", None) + + control = _NoSchemaControl() + data = _FakeData() + monkeypatch.setattr(cli, "_build_control_client", lambda region: control) + monkeypatch.setattr(cli, "_build_data_client", lambda region: data) + + rc = cli._handle_migrate(_args(tmp_path, include="semantic")) + assert rc == 1 + assert data.create_calls == [] + + +# --------------------------------------------------------------------------- # +# Issue 2 — client-side reconcile-by-source_row_id (ledger-loss safety net) +# --------------------------------------------------------------------------- # +def test_lost_ledger_reconciles_and_updates_not_duplicates(tmp_path, monkeypatch): + _write_config(tmp_path) + db_path = _seed_db(tmp_path, reflections=1, semantic=0) + control = _Control() + + # The ledger is empty (lost) but the record ALREADY exists remotely with a + # matching source_row_id. Reconcile must reattach it so the run UPDATES the + # existing record rather than CREATING a duplicate. + remote_body = json.dumps( + {"source_backend": "sqlite", "source_row_id": "refl-0", + "title": "stale", "status": "active"} + ) + data = _FakeData(remote_records={ + "projects/better-memory/reflections/": [ + {"memoryRecordId": "mem-remote-1", "content": {"text": remote_body}} + ] + }) + monkeypatch.setattr(cli, "_build_control_client", lambda region: control) + monkeypatch.setattr(cli, "_build_data_client", lambda region: data) + + rc = cli._handle_migrate(_args(tmp_path, include="reflections")) + assert rc == 0 + + # No duplicate create; the reattached remote record was updated instead. + assert data.create_calls == [] + assert len(data.update_calls) == 1 + mem_id, recs = data.update_calls[0] + assert mem_id == "epi-X" + assert recs[0]["memoryRecordId"] == "mem-remote-1" + + # Ledger points at the reconciled remote id. + rows = _ledger_rows(db_path) + assert len(rows) == 1 + assert rows[0]["target_record_id"] == "mem-remote-1" + assert rows[0]["status"] == "migrated" + + +# --------------------------------------------------------------------------- # +# Issue 3 — --provision mints a replacement; cfg + writes target the NEW memory +# --------------------------------------------------------------------------- # +class _ReplaceControl: + """cfg's episodic memory is gone (get_memory raises); --provision recreates + it. Semantic is healthy and declares source_row_id.""" + + def __init__(self): + self._mems = { + "sem-X": _active_memory( + "sem-X", "better_memory_semantic", + schema_keys=["useful_count", "source_row_id", "status"], + ), + } + self.created: list = [] + + def get_memory(self, *, memoryId): + if memoryId not in self._mems: + raise RuntimeError(f"ResourceNotFoundException: {memoryId}") + return {"memory": self._mems[memoryId]} + + def create_memory(self, *, name, **kw): + new_id = "epi-NEW" + self.created.append(new_id) + mem = _active_memory(new_id, name) + mem["strategies"][0]["strategyId"] = "epi-new-strat" + self._mems[new_id] = mem + return { + "memory": { + "id": new_id, "arn": f"arn:{new_id}", "status": "CREATING", + "strategies": [ + {"strategyId": "epi-new-strat", "status": "CREATING", + "name": name} + ], + } + } + + +def test_provision_replacement_retargets_writes_and_persists_cfg( + tmp_path, monkeypatch +): + _write_config(tmp_path) # episodic memory_id = "epi-X" + db_path = _seed_db(tmp_path, reflections=2, semantic=0) + control = _ReplaceControl() + data = _FakeData() + monkeypatch.setattr(cli, "_build_control_client", lambda region: control) + monkeypatch.setattr(cli, "_build_data_client", lambda region: data) + monkeypatch.setattr(cli.time, "sleep", lambda _s: None) + + rc = cli._handle_migrate( + _args(tmp_path, include="reflections", provision=True) + ) + assert rc == 0 + assert control.created == ["epi-NEW"] + + # Every batch targeted the NEW memory, never the dead "epi-X". + called_mems = {mem for (mem, _recs) in data.create_calls} + assert called_mems == {"epi-NEW"} + + # cfg on disk was re-saved with the replacement memory + strategy id. + saved = json.loads((tmp_path / "agentcore.json").read_text()) + assert saved["episodic"]["memory_id"] == "epi-NEW" + assert saved["episodic"]["strategy_id"] == "epi-new-strat" + + rows = _ledger_rows(db_path) + assert len(rows) == 2 + assert all(r["status"] == "migrated" for r in rows) + + +# --------------------------------------------------------------------------- # +# Issue 4 — provision-from-no-config: strategy re-key must not force re-updates +# --------------------------------------------------------------------------- # +class _FreshProvisionControl: + """No prior config: --provision mints BOTH memories fresh.""" + + def __init__(self): + self._mems: dict = {} + self._n = 0 + + def get_memory(self, *, memoryId): + return {"memory": self._mems[memoryId]} + + def create_memory(self, *, name, **kw): + self._n += 1 + new_id = f"{name}-{self._n}" + schema_keys = ( + ["useful_count", "source_row_id", "status"] + if name == "better_memory_semantic" + else None + ) + self._mems[new_id] = _active_memory(new_id, name, schema_keys=schema_keys) + return { + "memory": { + "id": new_id, "arn": f"arn:{new_id}", "status": "CREATING", + "strategies": [ + {"strategyId": f"{new_id}-strat", "status": "CREATING", + "name": name} + ], + } + } + + +def test_provision_no_config_is_idempotent_second_run_no_updates( + tmp_path, monkeypatch +): + # No agentcore.json: first run provisions + migrates; second run (cfg now + # present with the REAL strategy ids) must re-key to a hash that still + # matches the ledger -> zero updates, zero creates. + _seed_db(tmp_path, reflections=2, semantic=1) + control = _FreshProvisionControl() + monkeypatch.setattr(cli, "_build_control_client", lambda region: control) + monkeypatch.setattr(cli.time, "sleep", lambda _s: None) + first = _FakeData() + monkeypatch.setattr(cli, "_build_data_client", lambda region: first) + rc = cli._handle_migrate(_args(tmp_path, provision=True)) + assert rc == 0 + assert len(first.create_calls) == 2 # reflections + semantic batches + assert (tmp_path / "agentcore.json").exists() -def test_migrate_raises_not_implemented_with_pointer() -> None: - args = argparse.Namespace(subcommand="migrate-from-sqlite") - with pytest.raises(NotImplementedError) as excinfo: - _handle_migrate(args) - msg = str(excinfo.value) - # Pointer text must mention the deferred spec / future work - assert "future" in msg.lower() or "deferred" in msg.lower() + # Second run: cfg loaded from disk with the real strategy ids. + second = _FakeData() + monkeypatch.setattr(cli, "_build_data_client", lambda region: second) + rc = cli._handle_migrate(_args(tmp_path, provision=True)) + assert rc == 0 + # The hash excludes memoryStrategyId, so the re-keyed records converge: + # no spurious update on the second run. + assert second.create_calls == [] + assert second.update_calls == [] diff --git a/tests/e2e/_fake_agentcore.py b/tests/e2e/_fake_agentcore.py index 5ade1c5..b07dc8d 100644 --- a/tests/e2e/_fake_agentcore.py +++ b/tests/e2e/_fake_agentcore.py @@ -46,6 +46,8 @@ import json import re import threading +import time +import uuid from dataclasses import dataclass, field from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from pathlib import Path @@ -237,6 +239,93 @@ def dialect_violation( return None +# --------------------------------------------------------------------------- # +# Live-verified metadata RETENTION dialect (design §1b, three real-AWS probes +# on 2026-07-12, acct 708306701628 eu-west-2, throwaway memories torn down; +# evidence scratchpad/probe*_out.txt, memory ids c588ca1c…, 9bd09ba6…). Whether +# a CLIENT-authored BASE record keeps its custom metadata is schema-gated by the +# STRATEGY that owns the namespace — the fake models this so the migrate T2 +# suite is meaningful (a fake that retained everything could not prove that +# reflection state MUST live in the body, nor that semantic MUST use declared +# metadata): +# +# * episodic reflections namespace (episodicMemoryStrategy): its schema under +# reflectionConfiguration.memoryRecordSchema governs only AWS-EXTRACTED +# records, so a client BASE write keeps NO custom metadata — the entire map +# is silently dropped (probe 1). Only the JSON content BODY round-trips, and +# a subsequent content-BODY update persists durably (probe 2, read-your-write +# GET). +# * userPreference semantic namespace: its TOP-LEVEL memoryRecordSchema DOES +# govern client BASE writes — keys DECLARED there are retained on create, +# survive updates, and are listable; UNDECLARED keys are silently dropped +# (probe 3). +# --------------------------------------------------------------------------- # + +#: Keys the userPreference strategy DECLARES for client BASE writes — mirrors +#: better_memory.cli._agentcore_strategies.SEMANTIC_METADATA_SCHEMA (the schema +#: the migrator's --provision path guarantees before writing). Anything outside +#: this set is dropped on the semantic namespace, exactly as real AWS does. +_SEMANTIC_DECLARED_METADATA_KEYS = frozenset( + { + "useful_count", + "missed_count", + "ignored_count", + "times_misled", + "overlooked_count", + "last_credited_at", + "status", + "source_row_id", + } +) + +#: Data-plane operations the opt-in record store serves statefully instead of +#: from a canned response (create persists, update mutates, list/get read back). +_RECORD_STORE_OPS = frozenset( + { + "BatchCreateMemoryRecords", + "BatchUpdateMemoryRecords", + "ListMemoryRecords", + "GetMemoryRecord", + } +) + +#: memoryId is the first path segment after /memories/; GetMemoryRecord carries +#: the record id after /memoryRecord/ (verified against the botocore requestUri +#: templates loaded in _load_routes). +_MEMORY_ID_RE = re.compile(r"/memories/([^/?]+)") +_RECORD_ID_RE = re.compile(r"/memoryRecord/([^/?]+)") + + +def _namespace_owning_strategy(namespaces: Any) -> str: + """Classify a record's namespace as ``'reflections'`` (episodic strategy) or + ``'semantic'`` (userPreference strategy) — the strategy whose schema gates + metadata retention. Live namespaces may gain a leading slash; normalize.""" + first = (list(namespaces) or [""])[0].lstrip("/") if namespaces else "" + if "reflections/" in first or first.endswith("reflections"): + return "reflections" + if "semantic/" in first or first.endswith("semantic"): + return "semantic" + return "other" + + +def gate_client_metadata(namespaces: Any, metadata: Any) -> dict[str, Any]: + """Apply the live-verified retention rule to a client BASE write's metadata + map: return only what the server would keep. Episodic reflections drop the + whole map; userPreference semantic keeps only DECLARED keys (design §1b).""" + if not isinstance(metadata, dict): + return {} + owner = _namespace_owning_strategy(namespaces) + if owner == "reflections": + return {} + if owner == "semantic": + return { + k: v + for k, v in metadata.items() + if k in _SEMANTIC_DECLARED_METADATA_KEYS + } + return dict(metadata) + + def _uri_to_regex(request_uri: str) -> re.Pattern[str]: """Compile a botocore ``requestUri`` (e.g. ``/memories/{memoryId}/events``) into an anchored path regex. ``{param}`` matches one path segment; @@ -338,15 +427,31 @@ def text(self) -> str: class FakeAgentCore: - """Recording fake for both bedrock-agentcore planes. Context manager.""" + """Recording fake for both bedrock-agentcore planes. Context manager. + + Pass ``record_store=True`` to enable the opt-in STATEFUL record store: the + four data-plane record operations (``BatchCreate`` / ``BatchUpdate`` / + ``ListMemoryRecords`` / ``GetMemoryRecord``) then persist and read back real + records, applying the live-verified metadata-retention dialect + (:func:`gate_client_metadata`) so a full migrate → ``backend.retrieve()`` + round-trip is exercised end-to-end. Default (``False``) preserves the pure + canned-response behavior the D1-D7 scenarios rely on.""" - def __init__(self) -> None: + def __init__(self, *, record_store: bool = False) -> None: self._routes = _load_routes() self._known_ops = {name for name, _, _ in self._routes} self._responses: dict[str, Any] = {} self._lock = threading.Lock() self.requests: list[RecordedRequest] = [] + # Opt-in stateful store: memoryId -> {memoryRecordId -> record dict}. + self._record_store_enabled = record_store + self._records: dict[str, dict[str, dict[str, Any]]] = {} + self._minted = 0 + #: requestIdentifiers a create should reject with a failedRecords entry + #: (the injected partial-failure lever for the T2 resume test). + self.fail_on_create: set[str] = set() + fake = self class _Handler(BaseHTTPRequestHandler): @@ -400,10 +505,17 @@ def requests_for(self, operation: str) -> list[RecordedRequest]: return [r for r in self.requests if r.operation == operation] def clear(self) -> None: - """Drop all recorded requests (canned responses are kept).""" + """Drop all recorded requests (canned responses + stored records kept).""" with self._lock: self.requests.clear() + def stored_records(self, memory_id: str) -> list[dict[str, Any]]: + """Snapshot of the records persisted under ``memory_id`` (record-store + mode only). Metadata reflects the retention gate already applied on + write, so tests can assert the drop/retain contract directly.""" + with self._lock: + return [dict(rec) for rec in self._records.get(memory_id, {}).values()] + def close(self) -> None: self._server.shutdown() self._server.server_close() @@ -450,19 +562,151 @@ def _serve(self, handler: BaseHTTPRequestHandler) -> None: override = dialect_violation(operation, body) if override is not None: status_code, payload = override + elif self._record_store_enabled and operation in _RECORD_STORE_OPS: + # Stateful record store: dialect already vetted the shape above, so + # anything reaching here is a request real AWS would accept. + status_code, payload = self._serve_record_store(operation, record) else: status_code = 200 payload = self._responses.get(operation or "", {}) if callable(payload): payload = payload(record) - data = json.dumps(payload).encode("utf-8") + data = json.dumps(payload, default=str).encode("utf-8") handler.send_response(status_code) - if status_code == 400: + if status_code >= 400 and isinstance(payload, dict) and "__type" in payload: # botocore's rest-json error parser reads this header (and the - # __type body member) to raise ClientError ValidationException. - handler.send_header("x-amzn-ErrorType", "ValidationException") + # __type body member) to raise the matching ClientError (400 + # ValidationException, 404 ResourceNotFoundException, ...). + handler.send_header("x-amzn-ErrorType", payload["__type"]) handler.send_header("Content-Type", "application/json") handler.send_header("Content-Length", str(len(data))) handler.end_headers() handler.wfile.write(data) + + # ----- opt-in stateful record store ----- + + def _mint_record_id(self) -> str: + """Mint a server-side id. Real AWS mints ``mem-``; the + MemoryRecordId shape constrains it to /mem-[A-Za-z0-9-_]*/ length 40-50, + so short ids would be rejected client-side on any later GET/update.""" + self._minted += 1 + return f"mem-fake-{uuid.uuid4().hex}"[:50] # 41 chars, in [40, 50] + + @staticmethod + def _public_record(rec: dict[str, Any]) -> dict[str, Any]: + """A record dict trimmed to the wire members (drop None, keep only what + the MemoryRecordSummary / MemoryRecord shapes surface).""" + out: dict[str, Any] = { + "memoryRecordId": rec["memoryRecordId"], + "content": rec["content"], + "namespaces": rec["namespaces"], + "metadata": rec["metadata"], + } + if rec.get("memoryStrategyId"): + out["memoryStrategyId"] = rec["memoryStrategyId"] + if rec.get("createdAt") is not None: + out["createdAt"] = rec["createdAt"] + return out + + def _serve_record_store( + self, operation: str, record: RecordedRequest + ) -> tuple[int, dict[str, Any]]: + memory_id_match = _MEMORY_ID_RE.search(record.path) + memory_id = memory_id_match.group(1) if memory_id_match else "" + body = record.body if isinstance(record.body, dict) else {} + + with self._lock: + bucket = self._records.setdefault(memory_id, {}) + + if operation == "BatchCreateMemoryRecords": + successful: list[dict[str, Any]] = [] + failed: list[dict[str, Any]] = [] + now = time.time() + for rec in body.get("records") or []: + req_id = rec.get("requestIdentifier", "") + if req_id in self.fail_on_create: + failed.append( + { + "requestIdentifier": req_id, + "status": "FAILED", + "errorCode": 500, + "errorMessage": "injected failedRecords (fake)", + } + ) + continue + rid = self._mint_record_id() + namespaces = list(rec.get("namespaces") or []) + bucket[rid] = { + "memoryRecordId": rid, + "content": rec.get("content") or {"text": ""}, + "namespaces": namespaces, + "memoryStrategyId": rec.get("memoryStrategyId"), + # Schema-gated retention — the crux (design §1b). + "metadata": gate_client_metadata( + namespaces, rec.get("metadata") + ), + "createdAt": now, + "updatedAt": now, + } + successful.append( + { + "requestIdentifier": req_id, + "memoryRecordId": rid, + "status": "SUCCEEDED", + } + ) + return 200, { + "successfulRecords": successful, + "failedRecords": failed, + } + + if operation == "BatchUpdateMemoryRecords": + ok: list[dict[str, Any]] = [] + bad: list[dict[str, Any]] = [] + now = time.time() + for rec in body.get("records") or []: + rid = rec.get("memoryRecordId") + existing = bucket.get(rid) + if existing is None: + bad.append( + { + "memoryRecordId": rid, + "status": "FAILED", + "errorCode": 404, + "errorMessage": "record not found", + } + ) + continue + # Content-BODY updates persist durably (design §1b probe 2). + if "content" in rec: + existing["content"] = rec["content"] + # Metadata updates re-apply the same retention gate. + if "metadata" in rec: + existing["metadata"] = gate_client_metadata( + existing["namespaces"], rec["metadata"] + ) + existing["updatedAt"] = now + ok.append({"memoryRecordId": rid, "status": "SUCCEEDED"}) + return 200, {"successfulRecords": ok, "failedRecords": bad} + + if operation == "ListMemoryRecords": + target = str(body.get("namespace") or "").lstrip("/") + summaries = [ + self._public_record(rec) + for rec in bucket.values() + if target + in {n.lstrip("/") for n in (rec.get("namespaces") or [])} + ] + return 200, {"memoryRecordSummaries": summaries, "nextToken": None} + + # GetMemoryRecord — record id is the last path segment. + record_id_match = _RECORD_ID_RE.search(record.path) + rid = record_id_match.group(1) if record_id_match else "" + existing = bucket.get(rid) + if existing is None: + return 404, { + "__type": "ResourceNotFoundException", + "message": f"memory record {rid!r} not found", + } + return 200, {"memoryRecord": self._public_record(existing)} diff --git a/tests/e2e/test_agentcore_migrate_t2.py b/tests/e2e/test_agentcore_migrate_t2.py new file mode 100644 index 0000000..2b6f1ef --- /dev/null +++ b/tests/e2e/test_agentcore_migrate_t2.py @@ -0,0 +1,686 @@ +"""T2 fake-endpoint integration tests for ``agentcore migrate`` (design §9 T2). + +Every test drives the REAL migrate CLI (`cli._handle_migrate`) with REAL boto3 +clients pointed at the local :class:`FakeAgentCore` — real botocore +serialization, real SigV4 signing (fake creds), real HTTP — and the fake runs +its opt-in STATEFUL record store, so writes actually persist and are read back +through the REAL reader (`AgentCoreBackend.retrieve` / `.semantic_list`). That +closes the end-to-end parity loop the pure-mock T1 CLI tests +(`tests/cli/test_agentcore_migrate.py`) cannot: a migrated row is only "correct" +if the shipped reader reconstructs it. + +The fake models the live-proven AgentCore metadata dialect (design §1b, three +real-AWS probes on 2026-07-12, acct 708306701628 eu-west-2; memory ids +c588ca1c…, 9bd09ba6…): a client BASE write on the **episodic reflections** +namespace has its entire custom metadata map silently dropped (only the JSON +content BODY round-trips, and content-BODY updates persist durably), while the +**userPreference semantic** namespace retains keys DECLARED in its +memoryRecordSchema (undeclared keys dropped). See +``tests/e2e/_fake_agentcore.py::gate_client_metadata`` for the enforcement and +the citation. These tests are meaningful precisely because of that gate: the +reflection reader is body-first (metadata would be invisible), and the semantic +idempotency key + counters ride declared metadata. +""" + +from __future__ import annotations + +import argparse +import json +from pathlib import Path +from typing import Any + +import pytest + +from better_memory.cli import agentcore as cli +from better_memory.db.connection import connect +from better_memory.db.schema import apply_migrations +from tests.e2e._agentcore_env import write_fake_agentcore_json +from tests.e2e._fake_agentcore import FakeAgentCore, RecordedRequest + +_REGION = "eu-west-2" +_EPI_ID = "EPI-FAKE-0001" +_SEM_ID = "SEM-FAKE-0001" +_PROJECT = "better-memory" + + +# --------------------------------------------------------------------------- # +# Process-env hygiene (in-process boto3 reads THIS process's config chains). +# --------------------------------------------------------------------------- # +@pytest.fixture +def scrubbed_aws_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + """Explicit endpoint/creds/region kwargs win for everything asserted here, + but scrub the hazardous vars so a dev shell with AWS_PROFILE/SSO material or + a corporate proxy can never influence (or swallow) the fake traffic.""" + for var in ( + "AWS_PROFILE", + "AWS_DEFAULT_REGION", + "AWS_REGION", + "AWS_SESSION_TOKEN", + "AWS_ENDPOINT_URL", + "AWS_IGNORE_CONFIGURED_ENDPOINT_URLS", + "HTTP_PROXY", + "HTTPS_PROXY", + "ALL_PROXY", + "http_proxy", + "https_proxy", + "all_proxy", + ): + monkeypatch.delenv(var, raising=False) + monkeypatch.setenv("NO_PROXY", "127.0.0.1,localhost") + monkeypatch.setenv( + "AWS_SHARED_CREDENTIALS_FILE", str(tmp_path / "no-such-credentials") + ) + monkeypatch.setenv("AWS_CONFIG_FILE", str(tmp_path / "no-such-config")) + + +# --------------------------------------------------------------------------- # +# Wiring helpers +# --------------------------------------------------------------------------- # +def _real_client(service: str, fake: FakeAgentCore, region: str) -> Any: + import boto3 + from botocore.config import Config as BotoConfig + + return boto3.client( + service, + endpoint_url=fake.endpoint_url, + region_name=region, + aws_access_key_id="bm-e2e-fake", + aws_secret_access_key="bm-e2e-fake-secret", + config=BotoConfig(retries={"mode": "standard", "max_attempts": 5}), + ) + + +def _point_clients_at_fake( + monkeypatch: pytest.MonkeyPatch, fake: FakeAgentCore +) -> None: + """Route the migrate CLI's data + control clients to the fake — the exact + seam AWS_ENDPOINT_URL provides for the real subprocess/hook paths.""" + monkeypatch.setattr( + cli, + "_build_data_client", + lambda region: _real_client("bedrock-agentcore", fake, region), + ) + monkeypatch.setattr( + cli, + "_build_control_client", + lambda region: _real_client("bedrock-agentcore-control", fake, region), + ) + + +def _install_active_get_memory(fake: FakeAgentCore) -> None: + """Can a GetMemory control response: both memories ACTIVE; the semantic + strategy DECLARES source_row_id so `_ensure_semantic_schema` is satisfied + without an in-place widen (the already-provisioned-correctly path).""" + + def _get_memory(record: RecordedRequest) -> dict[str, Any]: + is_semantic = "SEM" in record.path + strategy: dict[str, Any] = { + "strategyId": "STRAT-SEM-1" if is_semantic else "STRAT-EPI-1", + "status": "ACTIVE", + "name": "userPreference" if is_semantic else "episodicReflections", + } + if is_semantic: + strategy["memoryRecordSchema"] = { + "metadataSchema": [ + {"key": "useful_count", "type": "NUMBER"}, + {"key": "times_misled", "type": "NUMBER"}, + {"key": "overlooked_count", "type": "NUMBER"}, + {"key": "last_credited_at", "type": "STRING"}, + {"key": "status", "type": "STRING"}, + {"key": "source_row_id", "type": "STRING"}, + ] + } + memory_id = _SEM_ID if is_semantic else _EPI_ID + return { + "memory": { + "id": memory_id, + "name": memory_id, + "status": "ACTIVE", + "strategies": [strategy], + } + } + + fake.set_response("GetMemory", _get_memory) + + +def _args(home: Path, **over: Any) -> argparse.Namespace: + d: dict[str, Any] = dict( + subcommand="migrate", + home=str(home), + dry_run=False, + include="reflections,semantic", + restart=False, + provision=False, + db=None, + project=None, + region=None, + batch_size=25, + verify=False, + ) + d.update(over) + return argparse.Namespace(**d) + + +def _make_backend(fake: FakeAgentCore, cfg: Any) -> Any: + from better_memory.storage.agentcore import AgentCoreBackend + + return AgentCoreBackend( + config=cfg, + data_client=_real_client("bedrock-agentcore", fake, _REGION), + control_client=_real_client("bedrock-agentcore-control", fake, _REGION), + session_id="t2-migrate-readback", + project=_PROJECT, + ) + + +# --------------------------------------------------------------------------- # +# DB seeding +# --------------------------------------------------------------------------- # +_REFL_COLS = ( + "id, title, project, tech, phase, polarity, use_cases, hints, confidence, " + "status, superseded_by, evidence_count, created_at, updated_at, scope, " + "useful_count, last_useful_at, times_misled, last_misled_at, " + "times_overlooked, last_overlooked_at" +) + + +def _insert_reflection(conn: Any, **f: Any) -> None: + row = { + "id": f["id"], + "title": f.get("title", "T"), + "project": _PROJECT, + "tech": f.get("tech", "python"), + "phase": f.get("phase", "implementation"), + "polarity": f.get("polarity", "neutral"), + "use_cases": f.get("use_cases", "when x"), + "hints": json.dumps(f.get("hints", ["h1"])), + "confidence": f.get("confidence", 0.8), + "status": f.get("status", "pending_review"), + "superseded_by": None, + "evidence_count": f.get("evidence_count", 0), + "created_at": "2026-07-01T00:00:00+00:00", + "updated_at": f.get("updated_at", "2026-07-10T00:00:00+00:00"), + "scope": f.get("scope", "project"), + "useful_count": f.get("useful_count", 0), + "last_useful_at": f.get("last_useful_at"), + "times_misled": f.get("times_misled", 0), + "last_misled_at": f.get("last_misled_at"), + "times_overlooked": f.get("times_overlooked", 0), + "last_overlooked_at": f.get("last_overlooked_at"), + } + placeholders = ", ".join("?" for _ in _REFL_COLS.split(", ")) + conn.execute( + f"INSERT INTO reflections ({_REFL_COLS}) VALUES ({placeholders})", + tuple(row[c.strip()] for c in _REFL_COLS.split(",")), + ) + + +def _insert_semantic(conn: Any, **f: Any) -> None: + conn.execute( + """ + INSERT INTO semantic_memories + (id, content, project, scope, created_at, updated_at, useful_count, + last_useful_at, times_misled, last_misled_at, times_overlooked, + last_overlooked_at) + VALUES (?, ?, ?, ?, '2026-07-01T00:00:00+00:00', + '2026-07-10T00:00:00+00:00', ?, ?, ?, ?, ?, ?) + """, + ( + f["id"], + f["content"], + _PROJECT, + f.get("scope", "project"), + f.get("useful_count", 0), + f.get("last_useful_at"), + f.get("times_misled", 0), + f.get("last_misled_at"), + f.get("times_overlooked", 0), + f.get("last_overlooked_at"), + ), + ) + + +def _open_db(home: Path) -> Any: + conn = connect(home / "memory.db") + apply_migrations(conn) + return conn + + +def _ledger_rows(home: Path) -> list[Any]: + conn = connect(home / "memory.db") + try: + return conn.execute( + "SELECT source_kind, source_id, status, target_record_id " + "FROM agentcore_migration ORDER BY source_kind, source_id" + ).fetchall() + finally: + conn.close() + + +# --------------------------------------------------------------------------- # +# Tests +# --------------------------------------------------------------------------- # +@pytest.mark.usefixtures("scrubbed_aws_env") +class TestFullMigrateRoundTrip: + def test_reflections_and_semantic_read_back_through_the_real_reader( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A full migrate lands reflections with ALL state in the body and + semantic with DECLARED metadata; both read back through the shipped + `AgentCoreBackend.retrieve` / `.semantic_list` with correct buckets and + counters — end-to-end parity through the real reader, not a shape echo. + Also proves the metadata-drop: the stored reflection carries an EMPTY + metadata map (episodic drop) yet retrieve() reconstructs every field.""" + home = tmp_path / "home" + cfg = write_fake_agentcore_json(home) + conn = _open_db(home) + # 'do' project reflection (pending_review -> active, admitted in the + # project namespace) and a 'dont' general reflection (confirmed -> + # promoted, admitted only in general/reflections/). + _insert_reflection( + conn, id="refl-do", title="Do A", polarity="do", + status="pending_review", scope="project", useful_count=5, + times_misled=1, times_overlooked=0, evidence_count=4, + confidence=0.9, hints=["always pass --build"], + updated_at="2026-07-11T00:00:00+00:00", + ) + _insert_reflection( + conn, id="refl-dont", title="Dont B", polarity="dont", + status="confirmed", scope="general", useful_count=2, + times_misled=0, evidence_count=3, confidence=0.7, + updated_at="2026-07-09T00:00:00+00:00", + ) + _insert_semantic( + conn, id="sem-0", content="prefers uv over pip", useful_count=3, + times_misled=0, times_overlooked=1, + last_useful_at="2026-07-05T00:00:00+00:00", + ) + conn.commit() + conn.close() + + with FakeAgentCore(record_store=True) as fake: + _install_active_get_memory(fake) + _point_clients_at_fake(monkeypatch, fake) + + assert cli._handle_migrate(_args(home)) == 0 + + # -- ledger: every eligible row migrated with a minted target. ---- + rows = _ledger_rows(home) + assert {(r["source_kind"], r["source_id"]) for r in rows} == { + ("reflection", "refl-do"), + ("reflection", "refl-dont"), + ("semantic", "sem-0"), + } + assert all(r["status"] == "migrated" for r in rows) + assert all(r["target_record_id"] for r in rows) + + # -- episodic drop: reflection records carry an EMPTY metadata map; + # ALL state is in the JSON body (design §1b). -------------------- + epi_stored = fake.stored_records(_EPI_ID) + assert len(epi_stored) == 2 + for rec in epi_stored: + assert rec["metadata"] == {} + body = json.loads(rec["content"]["text"]) + assert body["source_backend"] == "sqlite" + + # -- reflection parity through the REAL reader. ------------------- + backend = _make_backend(fake, cfg) + buckets = backend.retrieve(project=_PROJECT) + assert set(buckets) == {"do", "dont", "neutral"} + assert buckets["neutral"] == [] + + assert len(buckets["do"]) == 1 + do = buckets["do"][0] + assert do["title"] == "Do A" + assert do["useful_count"] == 5 + assert do["times_misled"] == 1 + assert do["evidence_count"] == 4 + assert do["confidence"] == pytest.approx(0.9) + assert do["tech"] == "python" + assert do["phase"] == "implementation" + assert do["hints"] == ["always pass --build"] + assert do["updated_at"] == "2026-07-11T00:00:00+00:00" + # Public shape is key-identical to the sqlite reflection dict — no + # internal ranking/bucketing helpers leak. + assert not any(k.startswith("_") for k in do) + + assert len(buckets["dont"]) == 1 + assert buckets["dont"][0]["title"] == "Dont B" + assert buckets["dont"][0]["evidence_count"] == 3 + + # -- semantic parity: declared metadata retained + counters read. - + sem_stored = fake.stored_records(_SEM_ID) + assert len(sem_stored) == 1 + sem_meta = sem_stored[0]["metadata"] + assert sem_meta["source_row_id"] == {"stringValue": "sem-0"} + assert sem_meta["useful_count"] == {"numberValue": 3} + # userPreference content stays the user's raw text, not JSON. + assert sem_stored[0]["content"]["text"] == "prefers uv over pip" + + semantics = backend.semantic_list(project=_PROJECT) + assert len(semantics) == 1 + sm = semantics[0] + assert sm.content == "prefers uv over pip" + assert sm.useful_count == 3 + assert sm.times_overlooked == 1 + assert sm.scope == "project" + + +@pytest.mark.usefixtures("scrubbed_aws_env") +class TestIdempotentRerun: + def test_second_run_short_circuits_via_ledger_zero_creates( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Re-running an already-converged migrate against the SAME fake + + ledger issues ZERO BatchCreate/BatchUpdate calls (the ledger + short-circuit) and the remote record count is stable — no duplicates.""" + home = tmp_path / "home" + write_fake_agentcore_json(home) + conn = _open_db(home) + _insert_reflection(conn, id="refl-1", polarity="do") + _insert_reflection(conn, id="refl-2", polarity="dont", scope="general", + status="confirmed") + _insert_semantic(conn, id="sem-1", content="fact one") + conn.commit() + conn.close() + + with FakeAgentCore(record_store=True) as fake: + _install_active_get_memory(fake) + _point_clients_at_fake(monkeypatch, fake) + + assert cli._handle_migrate(_args(home)) == 0 + first_creates = len(fake.requests_for("BatchCreateMemoryRecords")) + assert first_creates == 2 # one reflection batch, one semantic batch + epi_count = len(fake.stored_records(_EPI_ID)) + sem_count = len(fake.stored_records(_SEM_ID)) + assert (epi_count, sem_count) == (2, 1) + + # -- second run: nothing to do. ---------------------------------- + fake.clear() + assert cli._handle_migrate(_args(home)) == 0 + assert fake.requests_for("BatchCreateMemoryRecords") == [] + assert fake.requests_for("BatchUpdateMemoryRecords") == [] + # Remote store unchanged — the idempotency invariant. + assert len(fake.stored_records(_EPI_ID)) == 2 + assert len(fake.stored_records(_SEM_ID)) == 1 + + # Ledger stable: still exactly the 3 migrated rows. + rows = _ledger_rows(home) + assert len(rows) == 3 + assert all(r["status"] == "migrated" for r in rows) + + +@pytest.mark.usefixtures("scrubbed_aws_env") +class TestDryRun: + def test_dry_run_makes_zero_wire_calls_and_prints_tallies( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + ) -> None: + """--dry-run computes the plan from SQLite + the ledger and makes ZERO + AWS calls (not even GetMemory / list); tallies are printed; the ledger + is untouched.""" + home = tmp_path / "home" + write_fake_agentcore_json(home) + conn = _open_db(home) + _insert_reflection(conn, id="refl-1", polarity="do") + _insert_reflection(conn, id="refl-2", polarity="do") + _insert_semantic(conn, id="sem-1", content="fact") + conn.commit() + conn.close() + + with FakeAgentCore(record_store=True) as fake: + _install_active_get_memory(fake) + _point_clients_at_fake(monkeypatch, fake) + + assert cli._handle_migrate(_args(home, dry_run=True)) == 0 + + # Not one byte on the wire (dry-run returns before client build). + assert fake.requests == [] + + out = capsys.readouterr().out.lower() + assert "dry-run" in out + assert "create=2" in out # reflections namespace line + assert "create=1" in out # semantic namespace line + # Planning is read-only — no ledger rows written. + assert _ledger_rows(home) == [] + + +@pytest.mark.usefixtures("scrubbed_aws_env") +class TestPartialFailureResume: + def test_injected_failed_record_is_marked_and_only_it_retries( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + ) -> None: + """An injected failedRecords entry -> ledger marks that row 'failed' + (rc 2); the converged row stays 'migrated'; a re-run retries ONLY the + failed row (exactly one create carrying its requestIdentifier).""" + home = tmp_path / "home" + write_fake_agentcore_json(home) + conn = _open_db(home) + _insert_reflection(conn, id="refl-ok", polarity="do") + _insert_reflection(conn, id="refl-bad", polarity="dont") + conn.commit() + conn.close() + + with FakeAgentCore(record_store=True) as fake: + _install_active_get_memory(fake) + _point_clients_at_fake(monkeypatch, fake) + + fake.fail_on_create = {"bm-reflection-refl-bad"} + rc = cli._handle_migrate(_args(home, include="reflections")) + assert rc == 2 + assert "failed record" in capsys.readouterr().err + + statuses = {r["source_id"]: r["status"] for r in _ledger_rows(home)} + assert statuses["refl-ok"] == "migrated" + assert statuses["refl-bad"] == "failed" + # Only the converged record actually persisted. + assert len(fake.stored_records(_EPI_ID)) == 1 + + # -- clear the injection and re-run: only refl-bad retries. ------- + fake.fail_on_create = set() + fake.clear() + rc = cli._handle_migrate(_args(home, include="reflections")) + assert rc == 0 + + retried_reqs = [ + rec["requestIdentifier"] + for req in fake.requests_for("BatchCreateMemoryRecords") + for rec in req.body["records"] + ] + assert retried_reqs == ["bm-reflection-refl-bad"] + assert {r["status"] for r in _ledger_rows(home)} == {"migrated"} + assert len(fake.stored_records(_EPI_ID)) == 2 + + +@pytest.mark.usefixtures("scrubbed_aws_env") +class TestVerifyReadback: + def test_verify_reads_back_one_record_per_kind_through_get_memory_record( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + ) -> None: + """`--verify` (design §9 T2, sixth bullet) exercises `_verify_sample`: + after the writes it reads ONE migrated record per kind back through the + real `get_memory_record` (the fake's read-your-write store, + `_fake_agentcore.py`), diffs against SQLite, and prints 'readback ok' + for each kind. Guards the shipped verify path (agentcore.py:1230/1530, + incl. its ResourceNotFoundException retry loop) against regressions in + the error-code check, the ledger query, or the get_memory_record + kwargs — every other T2/T1 case runs verify=False, so this is the sole + coverage.""" + home = tmp_path / "home" + write_fake_agentcore_json(home) + conn = _open_db(home) + _insert_reflection(conn, id="refl-v", polarity="do") + _insert_semantic(conn, id="sem-v", content="verify me") + conn.commit() + conn.close() + + with FakeAgentCore(record_store=True) as fake: + _install_active_get_memory(fake) + _point_clients_at_fake(monkeypatch, fake) + + assert cli._handle_migrate(_args(home, verify=True)) == 0 + + # The verify readback hit get_memory_record with the exact target + # id the ledger recorded for each kind — one GET per kind. + ledger = { + r["source_kind"]: r["target_record_id"] + for r in _ledger_rows(home) + } + get_paths = [ + req.path + for req in fake.requests_for("GetMemoryRecord") + ] + assert any(ledger["reflection"] in p for p in get_paths) + assert any(ledger["semantic"] in p for p in get_paths) + + out = capsys.readouterr().out + assert f"verify: reflection {ledger['reflection']} readback ok" in out + assert f"verify: semantic {ledger['semantic']} readback ok" in out + + +@pytest.mark.usefixtures("scrubbed_aws_env") +class TestInvariantsSurviveRoundTrip: + def test_last_credited_at_stringValue_and_status_remap_survive( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Two landmine invariants survive the wire round trip: + + * ``last_credited_at`` is emitted (and stored) as a **stringValue**, not + a dateTimeValue — the declared STRING indexed key whose dateTimeValue + form fails the whole record update on real AWS (agentcore.py:616-621). + * status is **remapped** on write (pending_review->active, + confirmed->promoted) and drives client-side retrieval admission: the + promoted general reflection is retrievable, the pending project one is + retrievable, and a promoted record in the PROJECT namespace would be + excluded (only active is admitted there).""" + home = tmp_path / "home" + cfg = write_fake_agentcore_json(home) + conn = _open_db(home) + # confirmed -> promoted, general scope: retrievable via general ns. + _insert_reflection( + conn, id="refl-promoted", title="Promoted", polarity="do", + status="confirmed", scope="general", useful_count=1, + ) + # pending_review -> active, project scope. + _insert_reflection( + conn, id="refl-active", title="Active", polarity="dont", + status="pending_review", scope="project", + ) + _insert_semantic( + conn, id="sem-cred", content="uses ruff", + last_useful_at="2026-07-05T00:00:00+00:00", + last_misled_at="2026-07-06T00:00:00+00:00", + ) + conn.commit() + conn.close() + + with FakeAgentCore(record_store=True) as fake: + _install_active_get_memory(fake) + _point_clients_at_fake(monkeypatch, fake) + assert cli._handle_migrate(_args(home)) == 0 + + # -- status remap persisted in the reflection bodies. ------------- + statuses = { + json.loads(r["content"]["text"])["source_row_id"]: + json.loads(r["content"]["text"])["status"] + for r in fake.stored_records(_EPI_ID) + } + assert statuses == {"refl-promoted": "promoted", "refl-active": "active"} + + # -- last_credited_at is a stringValue (max of the per-class stamps), + # NOT dateTimeValue — and survived the declared-metadata gate. -- + sem_meta = fake.stored_records(_SEM_ID)[0]["metadata"] + credited = sem_meta["last_credited_at"] + assert set(credited) == {"stringValue"} + assert credited["stringValue"] == "2026-07-06T00:00:00+00:00" + + # -- remap drives retrieval admission through the real reader. ----- + backend = _make_backend(fake, cfg) + buckets = backend.retrieve(project=_PROJECT) + titles = { + b["title"] for bucket in buckets.values() for b in bucket + } + assert titles == {"Promoted", "Active"} + assert buckets["do"][0]["title"] == "Promoted" # general/promoted + assert buckets["dont"][0]["title"] == "Active" # project/active + + +@pytest.mark.usefixtures("scrubbed_aws_env") +class TestFakeModelsProvenDialect: + def test_episodic_metadata_drop_and_body_persist_on_update( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Guards the fake extension itself against the live-proven facts + (design §1b probes 1 & 2), so the parity tests above are meaningful. + Raw boto3, no product code: a client BASE write to the episodic + reflections namespace has its custom metadata dropped while the content + BODY round-trips, and a subsequent content-BODY update persists durably + (read-your-write GET) — while a userPreference semantic write keeps its + DECLARED metadata and drops UNDECLARED keys (probe 3).""" + from datetime import UTC, datetime + + with FakeAgentCore(record_store=True) as fake: + _point_clients_at_fake(monkeypatch, fake) + client = _real_client("bedrock-agentcore", fake, _REGION) + + # -- episodic: custom metadata dropped, body round-trips. --------- + created = client.batch_create_memory_records( + memoryId=_EPI_ID, + records=[{ + "requestIdentifier": "probe-refl-1", + "namespaces": ["projects/p/reflections/"], + "content": {"text": json.dumps({"title": "orig", "v": 1})}, + "timestamp": datetime.now(UTC), + "metadata": { + "polarity": {"stringValue": "do"}, + "useful_count": {"numberValue": 9}, + }, + }], + ) + rid = created["successfulRecords"][0]["memoryRecordId"] + + fetched = client.get_memory_record( + memoryId=_EPI_ID, memoryRecordId=rid + )["memoryRecord"] + assert fetched.get("metadata", {}) == {} # dropped (probe 1) + assert json.loads(fetched["content"]["text"]) == {"title": "orig", "v": 1} + + # -- content-BODY update persists durably (probe 2). -------------- + client.batch_update_memory_records( + memoryId=_EPI_ID, + records=[{ + "memoryRecordId": rid, + "timestamp": datetime.now(UTC), + "content": {"text": json.dumps({"title": "updated", "v": 2})}, + }], + ) + refetched = client.get_memory_record( + memoryId=_EPI_ID, memoryRecordId=rid + )["memoryRecord"] + assert json.loads(refetched["content"]["text"]) == { + "title": "updated", "v": 2, + } + + # -- semantic: DECLARED kept, UNDECLARED dropped (probe 3). -------- + client.batch_create_memory_records( + memoryId=_SEM_ID, + records=[{ + "requestIdentifier": "probe-sem-1", + "namespaces": ["projects/p/semantic/"], + "content": {"text": "a preference"}, + "timestamp": datetime.now(UTC), + "metadata": { + "source_row_id": {"stringValue": "sem-9"}, + "useful_count": {"numberValue": 4}, + "not_declared": {"stringValue": "gone"}, + }, + }], + ) + sem = client.list_memory_records( + memoryId=_SEM_ID, namespace="projects/p/semantic/", + maxResults=100, + )["memoryRecordSummaries"][0] + assert set(sem["metadata"]) == {"source_row_id", "useful_count"} + assert "not_declared" not in sem["metadata"] diff --git a/tests/mcp/test_handlers_remote.py b/tests/mcp/test_handlers_remote.py index 5e6c1d5..788092a 100644 --- a/tests/mcp/test_handlers_remote.py +++ b/tests/mcp/test_handlers_remote.py @@ -28,6 +28,19 @@ SemanticToolHandlers, SessionToolHandlers, ) +from better_memory.services.semantic import SemanticMemory + + +def _sm(*, id: str, content: str, scope: str = "project") -> SemanticMemory: + """Build a SemanticMemory the agentcore semantic_list returns (§6.3).""" + return SemanticMemory( + id=id, + content=content, + project="projx", + scope=scope, + created_at="2026-06-01T00:00:00+00:00", + updated_at="2026-06-01T00:00:00+00:00", + ) #: >= 40 chars — botocore's client-side validation floor for memoryRecordId. _RECORD_ID_40 = "refl-unit-" + "0" * 30 @@ -278,10 +291,13 @@ async def test_semantic_retrieve_merges_project_and_general_with_stable_keys( """UD-2: two backend calls (project namespace + general namespace) merged, payload keeps the sqlite key set with None placeholders.""" - def _semantic_list(**kwargs: Any) -> list[dict[str, Any]]: + def _semantic_list(**kwargs: Any) -> list[SemanticMemory]: + # §6.3: agentcore semantic_list returns SemanticMemory objects, + # matching the sqlite backend — the handler reads them via + # attribute access. if kwargs.get("scope_filter") == "general": - return [{"id": "g1", "content": "general fact", "scope": "general"}] - return [{"id": "p1", "content": "project fact", "scope": "project"}] + return [_sm(id="g1", content="general fact", scope="general")] + return [_sm(id="p1", content="project fact", scope="project")] remote.semantic_list = MagicMock(side_effect=_semantic_list) svc = MagicMock(name="SemanticMemoryService") @@ -306,7 +322,7 @@ def _semantic_list(**kwargs: Any) -> list[dict[str, Any]]: async def test_semantic_retrieve_dedupes_duplicate_ids(self, remote) -> None: remote.semantic_list = MagicMock( - return_value=[{"id": "dup-1", "content": "same", "scope": "general"}] + return_value=[_sm(id="dup-1", content="same", scope="general")] ) handlers = SemanticToolHandlers(semantic=MagicMock(), remote=remote) rows = _payload(await handlers.semantic_retrieve({})) diff --git a/tests/storage/test_agentcore_migrate.py b/tests/storage/test_agentcore_migrate.py new file mode 100644 index 0000000..f49926d --- /dev/null +++ b/tests/storage/test_agentcore_migrate.py @@ -0,0 +1,437 @@ +"""Hermetic unit tests for storage/agentcore_migrate.py (T4). + +No AWS, no boto3. Exercises the pure record builders, the deterministic +content hash, the batching helper, and the SQLite migration ledger. +""" + +from __future__ import annotations + +import json +import sqlite3 +from datetime import UTC, datetime + +import pytest + +from better_memory.storage import agentcore_migrate as m + + +def _rr(row, **kw): + """build_reflection_record for an ACTIVE row, narrowed to non-None. + + Active rows always build a record; only retired/superseded rows return + None (covered separately). Keeps pyright's optional-subscript analysis + happy without changing any test's behavior.""" + rec = m.build_reflection_record(row, **kw) + assert rec is not None + return rec + + +# --------------------------------------------------------------------------- # +# Fixtures / row factories +# --------------------------------------------------------------------------- # +def _reflection_row(**over): + row = { + "id": "refl-123", + "title": "Prefer body-first parse", + "project": "better-memory", + "tech": "python", + "phase": "implementation", + "polarity": "do", + "use_cases": "when parsing records", + "hints": json.dumps(["hint one", "hint two"]), + "confidence": 0.8, + "status": "pending_review", + "superseded_by": None, + "evidence_count": 5, + "created_at": "2026-07-01T00:00:00+00:00", + "updated_at": "2026-07-10T00:00:00+00:00", + "scope": "project", + "useful_count": 4, + "last_useful_at": "2026-07-09T00:00:00+00:00", + "times_misled": 1, + "last_misled_at": "2026-07-08T00:00:00+00:00", + "times_overlooked": 2, + "last_overlooked_at": "2026-07-11T00:00:00+00:00", + } + row.update(over) + return row + + +def _semantic_row(**over): + row = { + "id": "sem-77", + "content": "The user prefers no emojis in output.", + "project": "better-memory", + "scope": "project", + "created_at": "2026-07-01T00:00:00+00:00", + "updated_at": "2026-07-10T00:00:00+00:00", + "useful_count": 3, + "last_useful_at": "2026-07-05T00:00:00+00:00", + "times_misled": 0, + "last_misled_at": None, + "times_overlooked": 1, + "last_overlooked_at": "2026-07-06T00:00:00+00:00", + } + row.update(over) + return row + + +@pytest.fixture() +def ledger_conn(): + conn = sqlite3.connect(":memory:") + m.ensure_ledger(conn) + yield conn + conn.close() + + +# --------------------------------------------------------------------------- # +# build_reflection_record — ALL state in the body, NO metadata +# --------------------------------------------------------------------------- # +def test_reflection_all_state_in_body_no_metadata(): + rec = _rr(_reflection_row(), strategy_id="strat-episodic") + + # No custom metadata map is emitted (AWS would silently drop it, §1b). + assert "metadata" not in rec + + body = json.loads(rec["content"]["text"]) + # Body carries the content fields AND the fields §3 put in metadata. + for key in ( + "title", "use_cases", "hints", "confidence", "tech", "phase", + "evidence_count", "updated_at", "polarity", "status", "useful_count", + "times_misled", "times_overlooked", "source_row_id", "source_backend", + ): + assert key in body, f"{key} missing from body" + + assert body["hints"] == ["hint one", "hint two"] # decoded to a list + assert body["confidence"] == 0.8 + assert body["evidence_count"] == 5 + assert body["source_row_id"] == "refl-123" + assert body["source_backend"] == "sqlite" + assert rec["memoryStrategyId"] == "strat-episodic" + assert "timestamp" in rec + + +def test_reflection_namespace_project_vs_general(): + proj = _rr(_reflection_row(), strategy_id="s") + assert proj["namespaces"] == ["projects/better-memory/reflections/"] + + gen = _rr( + _reflection_row(scope="general"), strategy_id="s" + ) + assert gen["namespaces"] == ["general/reflections/"] + + +def test_reflection_status_remap(): + a = _rr( + _reflection_row(status="pending_review"), strategy_id="s" + ) + assert json.loads(a["content"]["text"])["status"] == "active" + + b = _rr( + _reflection_row(status="confirmed"), strategy_id="s" + ) + assert json.loads(b["content"]["text"])["status"] == "promoted" + + +def test_reflection_retired_and_superseded_skipped_on_create(): + assert m.build_reflection_record( + _reflection_row(status="retired"), strategy_id="s" + ) is None + assert m.build_reflection_record( + _reflection_row(status="superseded"), strategy_id="s" + ) is None + + +def test_reflection_deterministic_request_identifier(): + r1 = _rr(_reflection_row(), strategy_id="s") + r2 = _rr( + _reflection_row(updated_at="2026-01-01T00:00:00+00:00"), strategy_id="s" + ) + assert r1["requestIdentifier"] == "bm-reflection-refl-123" + assert r1["requestIdentifier"] == r2["requestIdentifier"] + + +def test_reflection_request_identifier_truncated_to_80(): + long_id = "x" * 200 + rec = _rr( + _reflection_row(id=long_id), strategy_id="s" + ) + assert len(rec["requestIdentifier"]) == 80 + assert rec["requestIdentifier"].startswith("bm-reflection-x") + + +# --------------------------------------------------------------------------- # +# build_semantic_record — raw text content + DECLARED metadata +# --------------------------------------------------------------------------- # +def test_semantic_content_is_raw_text_not_json(): + rec = m.build_semantic_record(_semantic_row(), strategy_id="strat-pref") + assert rec["content"]["text"] == "The user prefers no emojis in output." + # Raw text, not JSON. + with pytest.raises(json.JSONDecodeError): + json.loads(rec["content"]["text"]) + assert rec["memoryStrategyId"] == "strat-pref" + + +def test_semantic_declares_source_row_id_and_counters_in_metadata(): + rec = m.build_semantic_record(_semantic_row(), strategy_id="s") + md = rec["metadata"] + + assert md["source_row_id"] == {"stringValue": "sem-77"} + assert md["useful_count"] == {"numberValue": 3} + assert md["times_misled"] == {"numberValue": 0} + # times_overlooked renamed to overlooked_count. + assert md["overlooked_count"] == {"numberValue": 1} + assert "times_overlooked" not in md + assert md["status"] == {"stringValue": "active"} + + +def test_semantic_last_credited_at_is_string_value(): + rec = m.build_semantic_record(_semantic_row(), strategy_id="s") + lc = rec["metadata"]["last_credited_at"] + # MUST be stringValue (declared STRING indexed key); dateTimeValue rejects + # the whole record (§3 landmine). + assert "stringValue" in lc + assert "dateTimeValue" not in lc + # Max of the three last_*_at timestamps. + assert lc["stringValue"] == "2026-07-06T00:00:00+00:00" + + +def test_semantic_request_identifier_deterministic(): + rec = m.build_semantic_record(_semantic_row(), strategy_id="s") + assert rec["requestIdentifier"] == "bm-semantic-sem-77" + + +def test_semantic_namespace_project_vs_general(): + proj = m.build_semantic_record(_semantic_row(), strategy_id="s") + assert proj["namespaces"] == ["projects/better-memory/semantic/"] + gen = m.build_semantic_record(_semantic_row(scope="general"), strategy_id="s") + assert gen["namespaces"] == ["general/semantic/"] + + +# --------------------------------------------------------------------------- # +# chunk() batching +# --------------------------------------------------------------------------- # +def test_chunk_batches_evenly_and_remainder(): + records = [{"i": i} for i in range(7)] + batches = list(m.chunk(records, 3)) + assert [len(b) for b in batches] == [3, 3, 1] + # Order + content preserved. + assert [r["i"] for b in batches for r in b] == list(range(7)) + + +def test_chunk_empty_and_single_batch(): + assert list(m.chunk([], 5)) == [] + assert list(m.chunk([{"a": 1}], 5)) == [[{"a": 1}]] + + +def test_chunk_rejects_zero_batch_size(): + with pytest.raises(ValueError): + list(m.chunk([{"a": 1}], 0)) + + +# --------------------------------------------------------------------------- # +# canonical_content_hash — deterministic, timestamp-independent +# --------------------------------------------------------------------------- # +def test_content_hash_ignores_timestamp(): + r1 = _rr( + _reflection_row(), strategy_id="s", + timestamp=datetime(2026, 1, 1, tzinfo=UTC), + ) + r2 = _rr( + _reflection_row(), strategy_id="s", + timestamp=datetime(2026, 12, 31, tzinfo=UTC), + ) + assert m.canonical_content_hash(r1) == m.canonical_content_hash(r2) + + +def test_content_hash_changes_with_body(): + base = _rr(_reflection_row(), strategy_id="s") + changed = _rr( + _reflection_row(confidence=0.1), strategy_id="s" + ) + assert m.canonical_content_hash(base) != m.canonical_content_hash(changed) + + +def test_content_hash_excludes_memory_strategy_id(): + # The strategy id is a routing attribute filled in only after provisioning + # (§5.2). Re-keying it MUST NOT change the hash, else the --provision path + # forces a spurious update on every later run. + a = _rr(_reflection_row(), strategy_id="") + b = _rr(_reflection_row(), strategy_id="real-strat") + assert a["memoryStrategyId"] != b["memoryStrategyId"] + assert m.canonical_content_hash(a) == m.canonical_content_hash(b) + + sa = m.build_semantic_record(_semantic_row(), strategy_id="") + sb = m.build_semantic_record(_semantic_row(), strategy_id="real-strat") + assert m.canonical_content_hash(sa) == m.canonical_content_hash(sb) + + +# --------------------------------------------------------------------------- # +# push_batch — thin wrapper +# --------------------------------------------------------------------------- # +def test_push_batch_calls_data_client(): + calls = {} + + class _Client: + def batch_create_memory_records(self, *, memoryId, records): + calls["memoryId"] = memoryId + calls["records"] = records + return {"successfulRecords": records, "failedRecords": []} + + resp = m.push_batch(_Client(), "mem-episodic", [{"a": 1}, {"a": 2}]) + assert calls["memoryId"] == "mem-episodic" + assert calls["records"] == [{"a": 1}, {"a": 2}] + assert resp["failedRecords"] == [] + + +# --------------------------------------------------------------------------- # +# Ledger decisions +# --------------------------------------------------------------------------- # +def test_ledger_no_entry_is_create(ledger_conn): + assert m.plan_row(ledger_conn, "reflection", "refl-1", "hashA") == "create" + + +def test_ledger_unchanged_hash_is_skip(ledger_conn): + m.record_success( + ledger_conn, kind="reflection", source_id="refl-1", + namespace="projects/p/reflections/", content_hash="hashA", + target_record_id="mem-xyz", + ) + assert m.plan_row(ledger_conn, "reflection", "refl-1", "hashA") == "skip" + + +def test_ledger_changed_hash_is_update(ledger_conn): + m.record_success( + ledger_conn, kind="reflection", source_id="refl-1", + namespace="projects/p/reflections/", content_hash="hashA", + target_record_id="mem-xyz", + ) + assert m.plan_row(ledger_conn, "reflection", "refl-1", "hashB") == "update" + + +def test_ledger_sqlite_retired_is_retire(ledger_conn): + m.record_success( + ledger_conn, kind="reflection", source_id="refl-1", + namespace="projects/p/reflections/", content_hash="hashA", + target_record_id="mem-xyz", + ) + # content_hash=None signals "source row no longer eligible" (retired). + assert m.plan_row(ledger_conn, "reflection", "refl-1", None) == "retire" + + +def test_ledger_retire_no_prior_entry_is_skip(ledger_conn): + assert m.plan_row(ledger_conn, "reflection", "never", None) == "skip" + + +def test_ledger_already_retired_is_skip(ledger_conn): + m.record_success( + ledger_conn, kind="reflection", source_id="refl-1", + namespace="ns", content_hash="hashA", target_record_id="mem-xyz", + status="retired", + ) + assert m.plan_row(ledger_conn, "reflection", "refl-1", None) == "skip" + + +def test_ledger_failed_status_retries_as_create(ledger_conn): + m.record_failure( + ledger_conn, kind="reflection", source_id="refl-1", + last_error="boom", content_hash="hashA", + ) + assert m.plan_row(ledger_conn, "reflection", "refl-1", "hashA") == "create" + + +def test_ledger_record_failure_then_success_clears_error(ledger_conn): + m.record_failure( + ledger_conn, kind="semantic", source_id="sem-1", last_error="boom", + ) + m.record_success( + ledger_conn, kind="semantic", source_id="sem-1", + namespace="ns", content_hash="hashZ", target_record_id="mem-1", + ) + r = ledger_conn.execute( + "SELECT status, last_error, target_record_id, content_hash " + "FROM agentcore_migration WHERE source_kind='semantic' AND source_id='sem-1'" + ).fetchone() + assert r == ("migrated", None, "mem-1", "hashZ") + + +def test_ledger_retire_preserves_target_and_hash(ledger_conn): + m.record_success( + ledger_conn, kind="reflection", source_id="refl-1", + namespace="ns", content_hash="hashA", target_record_id="mem-xyz", + ) + # Retire transition: only status flips; target + hash preserved. + m.record_success( + ledger_conn, kind="reflection", source_id="refl-1", status="retired", + ) + r = ledger_conn.execute( + "SELECT status, target_record_id, content_hash FROM agentcore_migration " + "WHERE source_kind='reflection' AND source_id='refl-1'" + ).fetchone() + assert r == ("retired", "mem-xyz", "hashA") + + +def test_reconcile_ledger_reattaches_when_no_entry(ledger_conn): + # Lost/absent ledger: reconcile binds a remote record's server id so the + # next plan_row yields 'update' (empty hash) rather than a duplicate create. + reattached = m.reconcile_ledger( + ledger_conn, kind="reflection", source_id="refl-1", + namespace="projects/p/reflections/", target_record_id="mem-remote", + ) + assert reattached is True + r = ledger_conn.execute( + "SELECT status, target_record_id, content_hash FROM agentcore_migration " + "WHERE source_kind='reflection' AND source_id='refl-1'" + ).fetchone() + assert r == ("migrated", "mem-remote", "") + # Empty stored hash != any real content hash -> plan_row returns 'update'. + assert m.plan_row(ledger_conn, "reflection", "refl-1", "realhash") == "update" + + +def test_reconcile_ledger_preserves_existing_target(ledger_conn): + m.record_success( + ledger_conn, kind="reflection", source_id="refl-1", + namespace="ns", content_hash="hashA", target_record_id="mem-original", + ) + reattached = m.reconcile_ledger( + ledger_conn, kind="reflection", source_id="refl-1", + namespace="ns", target_record_id="mem-DIFFERENT", + ) + assert reattached is False + r = ledger_conn.execute( + "SELECT target_record_id, content_hash FROM agentcore_migration " + "WHERE source_kind='reflection' AND source_id='refl-1'" + ).fetchone() + # Untouched: original target + hash preserved. + assert r == ("mem-original", "hashA") + + +def test_reconcile_ledger_reattaches_failed_row(ledger_conn): + # A create that failed locally but actually landed remotely: reconcile + # binds the found id so the retry updates instead of re-creating. + m.record_failure( + ledger_conn, kind="semantic", source_id="sem-1", last_error="boom", + ) + reattached = m.reconcile_ledger( + ledger_conn, kind="semantic", source_id="sem-1", + namespace="ns", target_record_id="mem-found", + ) + assert reattached is True + r = ledger_conn.execute( + "SELECT status, target_record_id, last_error FROM agentcore_migration " + "WHERE source_kind='semantic' AND source_id='sem-1'" + ).fetchone() + assert r == ("migrated", "mem-found", None) + + +def test_ensure_ledger_is_idempotent(ledger_conn): + # Second call must not raise. + m.ensure_ledger(ledger_conn) + cols = { + r[1] + for r in ledger_conn.execute("PRAGMA table_info(agentcore_migration)") + } + assert cols == { + "source_kind", "source_id", "namespace", "target_record_id", + "content_hash", "status", "last_error", "migrated_at", + } diff --git a/tests/storage/test_agentcore_semantic_parity.py b/tests/storage/test_agentcore_semantic_parity.py new file mode 100644 index 0000000..7fba97b --- /dev/null +++ b/tests/storage/test_agentcore_semantic_parity.py @@ -0,0 +1,251 @@ +"""T3 — agentcore semantic read-parity (design §1b/§3.2/§6.3). + +Two invariants proven hermetically (no AWS): + +1. ``AgentCoreBackend.semantic_list`` returns objects matching the + SqliteBackend ``SemanticMemory`` shape (``better_memory/services/semantic.py``), + populated from the declared metadata counters — NOT plain dicts. This is + what makes ``relevant.py`` (attribute access via ``getattr``) see real + content + non-zero counters instead of silently defaulting to ``''`` / ``0``. + +2. ``retrieve_relevant`` reads those objects and surfaces the counters. + +Plus a schema assertion (design §1b/§3.2): the userPreference strategy's +declared ``memoryRecordSchema`` DECLARES the migration idempotency key +``source_row_id`` — undeclared keys are silently dropped by AWS on client +BASE writes, which would break semantic idempotency reconcile. +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from unittest.mock import MagicMock + +import pytest + +from better_memory.cli._agentcore_strategies import ( + SEMANTIC_METADATA_SCHEMA, + semantic_strategy_block, +) +from better_memory.services.relevant import retrieve_relevant +from better_memory.services.semantic import SemanticMemory +from better_memory.storage.agentcore import AgentCoreBackend +from better_memory.storage.agentcore_persistence import ( + AgentCoreConfig, + MemoryRecord, +) + + +@pytest.fixture +def ac_config() -> AgentCoreConfig: + return AgentCoreConfig( + schema_version=1, + region="eu-west-2", + semantic=MemoryRecord( + memory_id="mem-sem-abc1234567", + memory_arn="arn:aws:bedrock-agentcore:eu-west-2:123:memory/mem-sem-abc1234567", + memory_name="better-memory-semantic", + strategy_id="userPreference-zXy1234567", + strategy_name="userPreference", + event_expiry_duration_days=365, + ), + episodic=MemoryRecord( + memory_id="mem-epi-def4567890", + memory_arn="arn:aws:bedrock-agentcore:eu-west-2:123:memory/mem-epi-def4567890", + memory_name="better-memory-episodic", + strategy_id="episodicReflections-qPr9876543", + strategy_name="episodicReflections", + event_expiry_duration_days=90, + ), + ) + + +@pytest.fixture +def mock_data_client() -> MagicMock: + return MagicMock(name="bedrock-agentcore-data") + + +@pytest.fixture +def backend(ac_config, mock_data_client) -> AgentCoreBackend: + return AgentCoreBackend( + config=ac_config, + data_client=mock_data_client, + control_client=MagicMock(name="control"), + session_id="test-session-xyz", + project="testproj", + ) + + +def _semantic_summary( + *, + record_id: str = "sm-1", + content: str = "prefer uv over pip for dependency management", + useful_count: int = 4, + times_misled: int = 1, + overlooked_count: int = 2, + namespaces: list[str] | None = None, +) -> dict: + """A list_memory_records summary carrying the declared counters the + migrator writes (design §3.2). Counters are numberValue; last_credited_at + is the collapsed stringValue timestamp.""" + return { + "memoryRecordId": record_id, + "content": {"text": content}, + "namespaces": namespaces or ["projects/testproj/semantic/"], + "createdAt": datetime(2026, 5, 25, tzinfo=UTC), + "updatedAt": datetime(2026, 6, 1, tzinfo=UTC), + "metadata": { + "useful_count": {"numberValue": useful_count}, + "times_misled": {"numberValue": times_misled}, + "overlooked_count": {"numberValue": overlooked_count}, + "last_credited_at": {"stringValue": "2026-06-01T00:00:00+00:00"}, + "status": {"stringValue": "active"}, + "source_row_id": {"stringValue": "row-123"}, + }, + } + + +# ----- Part 1a: semantic_list returns SemanticMemory-shaped objects ----- + + +def test_semantic_list_returns_semanticmemory_shaped_objects( + backend, mock_data_client +) -> None: + """§6.3: semantic_list must return objects with the SemanticMemory shape, + NOT plain dicts. Attribute access (relevant.py's getattr) must resolve to + the real content + counters, not silent defaults.""" + mock_data_client.list_memory_records.return_value = { + "memoryRecordSummaries": [_semantic_summary()] + } + result = backend.semantic_list() + assert len(result) == 1 + item = result[0] + + # Object shape, not a dict — dict subscripting would work on a dict but + # relevant.py uses getattr, which only resolves on the dataclass. + assert not isinstance(item, dict) + assert isinstance(item, SemanticMemory) + + # Counter-bearing, populated from the declared metadata numberValues. + assert item.id == "sm-1" + assert item.content == "prefer uv over pip for dependency management" + assert item.useful_count == 4 + assert item.times_misled == 1 + assert item.times_overlooked == 2 + assert item.scope == "project" + # updated_at must be a usable ISO timestamp for relevant.py age_days. + assert item.updated_at + assert "2026" in item.updated_at + + # Every SemanticMemory field is present (shape parity with the SQLite + # read model) — attribute access never AttributeErrors. + for field_name in ( + "id", "content", "project", "scope", "created_at", "updated_at", + "useful_count", "last_useful_at", "times_misled", "last_misled_at", + "times_overlooked", "last_overlooked_at", + ): + assert hasattr(item, field_name), field_name + + +def test_semantic_list_counters_default_to_zero_when_metadata_absent( + backend, mock_data_client +) -> None: + """AWS-extracted / freshly-created records with no counter metadata must + still yield a SemanticMemory with zeroed counters (no crash, no None).""" + mock_data_client.list_memory_records.return_value = { + "memoryRecordSummaries": [ + { + "memoryRecordId": "sm-bare", + "content": {"text": "bare fact"}, + "namespaces": ["projects/testproj/semantic/"], + } + ] + } + item = backend.semantic_list()[0] + assert isinstance(item, SemanticMemory) + assert item.content == "bare fact" + assert item.useful_count == 0 + assert item.times_misled == 0 + assert item.times_overlooked == 0 + + +def test_semantic_list_scope_classification_still_normalizes_leading_slash( + backend, mock_data_client +) -> None: + """Regression: the leading-slash namespace normalization survives the + dict→object change.""" + mock_data_client.list_memory_records.return_value = { + "memoryRecordSummaries": [ + _semantic_summary(record_id="g", namespaces=["/general/semantic/"]), + _semantic_summary( + record_id="p", namespaces=["/projects/testproj/semantic/"] + ), + ] + } + scopes = {m.id: m.scope for m in backend.semantic_list()} + assert scopes == {"g": "general", "p": "project"} + + +# ----- Part 1b: relevant.py reads the counter-bearing objects ----- + + +def test_relevant_reads_agentcore_semantic_counters_and_content( + backend, mock_data_client +) -> None: + """End-to-end §6.3 proof: retrieve_relevant over the agentcore backend + surfaces the real content + non-zero useful_count. Before the fix, + semantic_list returned dicts and getattr(s,'content') / getattr(s, + 'useful_count') defaulted to '' / 0, so this memory would never score + above the min-hits floor and semantic injection was dead.""" + # No reflections; semantic namespace has one strongly-matching memory. + mock_data_client.list_memory_records.return_value = { + "memoryRecordSummaries": [ + _semantic_summary( + content="always prefer uv over pip for python installs", + useful_count=7, + ) + ] + } + out = retrieve_relevant( + backend, + query="uv pip python", + project="testproj", + min_hits=2, + now=lambda: datetime(2026, 7, 13, tzinfo=UTC), + ) + assert len(out) == 1 + hit = out[0] + assert hit.kind == "semantic" + assert hit.text == "always prefer uv over pip for python installs" + # The non-zero useful_count made it through attribute access. + assert hit.useful_count == 7 + assert hit.age_days is not None # updated_at parsed → real age + + +# ----- Part 2: declared schema carries source_row_id ----- + + +def test_userpreference_schema_declares_source_row_id() -> None: + """§1b/§3.2: client BASE writes only retain keys DECLARED in the + userPreference memoryRecordSchema. The migration idempotency key + source_row_id MUST be declared or it is silently dropped.""" + keys = {entry["key"] for entry in SEMANTIC_METADATA_SCHEMA} + assert "source_row_id" in keys + src = next(e for e in SEMANTIC_METADATA_SCHEMA if e["key"] == "source_row_id") + assert src["type"] == "STRING" + + # Counters already relied upon by the read path stay declared. + assert {"useful_count", "times_misled", "overlooked_count"} <= keys + # last_credited_at / status used by the semantic write path stay declared. + assert {"last_credited_at", "status"} <= keys + + +def test_semantic_strategy_block_top_level_schema_declares_source_row_id() -> None: + """The declaration must live in the strategy block actually passed to + create_memory/provision — the top-level userPreference memoryRecordSchema.""" + block = semantic_strategy_block() + schema = block["userPreferenceMemoryStrategy"]["memoryRecordSchema"][ + "metadataSchema" + ] + keys = {entry["key"] for entry in schema} + assert "source_row_id" in keys diff --git a/tests/storage/test_agentcore_unit.py b/tests/storage/test_agentcore_unit.py index 09c3ba6..95b362b 100644 --- a/tests/storage/test_agentcore_unit.py +++ b/tests/storage/test_agentcore_unit.py @@ -5,6 +5,7 @@ from __future__ import annotations import hashlib +import json from datetime import UTC, datetime from unittest.mock import MagicMock @@ -1105,8 +1106,9 @@ def test_semantic_list_with_search_uses_retrieve_memory_records(backend, mock_da } result = backend.semantic_list(search="uv") assert len(result) == 1 - assert result[0]["id"] == "sm-1" - assert result[0]["content"] == "prefer uv" + # §6.3: semantic_list returns SemanticMemory objects, not dicts. + assert result[0].id == "sm-1" + assert result[0].content == "prefer uv" def test_semantic_list_without_search_uses_list_memory_records(backend, mock_data_client) -> None: @@ -1137,7 +1139,7 @@ def test_semantic_list_scope_classification_normalizes_leading_slash( ] } result = backend.semantic_list() - scopes = {r["id"]: r["scope"] for r in result} + scopes = {r.id: r.scope for r in result} assert scopes == { "sm-slash-general": "general", "sm-slash-project": "project", @@ -1456,3 +1458,398 @@ def test_session_bootstrap_returns_envelope_matching_sqlite_shape( assert result["episode_action"] == "opened" assert result["semantic_count"] == 0 assert result["reflections_counts"] == {"do": 0, "dont": 0, "neutral": 0} + + +# ===== T1: _parse_reflection_record body-first with metadata fallback ===== + + +def test_parse_reflection_extracted_record_unchanged(backend) -> None: + """INVARIANT (migration §1b): an AWS-extracted record — state in + metadata, NONE of the migration state keys in the body — parses exactly + as it did before the body-first change. Every field is asserted against + the pre-change computation (evidence_count = useful+missed; + polarity/status/counters from metadata; updated_at from createdAt).""" + import json + + created = datetime(2026, 5, 24, 12, 0, 0, tzinfo=UTC) + rec = { + "memoryRecordId": "rec-extracted", + "content": {"text": json.dumps({ + "title": "Extracted title", + "use_cases": "Applies when X", + "hints": "First hint. Second hint.", + "confidence": "0.8", + "tech": "python", + "phase": "build", + })}, + "namespaces": ["projects/testproj/reflections/"], + "createdAt": created, + "metadata": { + "polarity": {"stringValue": "dont"}, + "status": {"stringValue": "active"}, + "useful_count": {"numberValue": 4}, + "missed_count": {"numberValue": 2}, + "times_misled": {"numberValue": 1}, + "overlooked_count": {"numberValue": 3}, + }, + } + + parsed = backend._parse_reflection_record(rec) + + assert parsed == { + "id": "rec-extracted", + "title": "Extracted title", + "phase": "build", + "use_cases": "Applies when X", + "hints": ["First hint. Second hint."], + "confidence": 0.8, + "tech": "python", + # evidence_count computed from metadata useful(4)+missed(2) + "evidence_count": 6, + "useful_count": 4, + "times_misled": 1, + # updated_at derived from createdAt (no body updated_at, no sys key) + "updated_at": created.isoformat(), + "_overlooked_count": 3, + "_updated_at_ts": created.timestamp(), + "_polarity": "dont", + "_status": "active", + } + + +def test_parse_reflection_body_state_record_uses_body(backend) -> None: + """A migrated record carries ALL reflection state in the JSON body and + an EMPTY metadata map. It must parse from the body: polarity/status, + useful_count/times_misled/times_overlooked counters, evidence_count and + updated_at all resolve body-first (§6.1/§6.2).""" + import json + + rec = { + "memoryRecordId": "rec-migrated", + "content": {"text": json.dumps({ + "title": "Migrated title", + "use_cases": "Applies when Y", + "hints": ["Hint one.", "Hint two."], + "confidence": 0.9, + "tech": "rust", + "phase": "plan", + "evidence_count": 12, + "updated_at": "2026-06-01T09:30:00+00:00", + "polarity": "do", + "status": "promoted", + "useful_count": 7, + "times_misled": 2, + "times_overlooked": 5, + })}, + "namespaces": ["projects/testproj/reflections/"], + "createdAt": datetime(2026, 5, 24, tzinfo=UTC), + # No metadata map at all — migrated reflections carry none. + "metadata": {}, + } + + parsed = backend._parse_reflection_record(rec) + + # Body counters win over the (absent) metadata numberValues. + assert parsed["useful_count"] == 7 + assert parsed["times_misled"] == 2 + assert parsed["_overlooked_count"] == 5 + # evidence_count taken from the body (NOT recomputed as useful+missed=7). + assert parsed["evidence_count"] == 12 + # updated_at is the body ISO string, and it drives the ranking ts. + assert parsed["updated_at"] == "2026-06-01T09:30:00+00:00" + assert parsed["_updated_at_ts"] == datetime( + 2026, 6, 1, 9, 30, tzinfo=UTC + ).timestamp() + # Bucket selector comes from the body polarity — CRITICAL for migrated + # records, which carry polarity nowhere else. + assert parsed["_polarity"] == "do" + assert parsed["_status"] == "promoted" + + +def test_retrieve_buckets_migrated_record_by_body_polarity( + backend, mock_data_client +) -> None: + """End-to-end through retrieve(): a body-state record with no metadata + lands in the bucket named by its BODY polarity and surfaces its body + counters — proving _fetch_reflection_buckets honors the body-first parse.""" + import json + + def stub(**kwargs): + if kwargs["namespace"] == "projects/testproj/reflections/": + return {"memoryRecordSummaries": [{ + "memoryRecordId": "rec-migrated", + "content": {"text": json.dumps({ + "title": "Migrated", "use_cases": "u", + "hints": ["h"], "confidence": 0.9, + "polarity": "dont", "status": "active", + "useful_count": 7, "times_misled": 2, + "times_overlooked": 5, "evidence_count": 12, + "updated_at": "2026-06-01T09:30:00+00:00", + })}, + "namespaces": ["projects/testproj/reflections/"], + "createdAt": datetime(2026, 5, 24, tzinfo=UTC), + "metadata": {}, + }]} + return {"memoryRecordSummaries": []} + + mock_data_client.list_memory_records.side_effect = stub + result = backend.retrieve(project="testproj") + + assert [r["id"] for r in result["dont"]] == ["rec-migrated"] + assert result["do"] == [] + assert result["neutral"] == [] + refl = result["dont"][0] + assert refl["useful_count"] == 7 + assert refl["evidence_count"] == 12 + assert refl["updated_at"] == "2026-06-01T09:30:00+00:00" + # Internal helpers stripped from the public payload. + assert not any(k.startswith("_") for k in refl) + + +# ===== T2: reflection rating on migrated (body-state) records ===== +# +# AWS silently drops custom metadata on client-authored BASE records in the +# episodic reflections namespace (design §1b), so batch_update metadata writes +# are a no-op there. For migrated records (body carries source_backend=='sqlite') +# the rating paths must read-modify-write the JSON CONTENT BODY instead +# (content updates persist). AWS-extracted records (no marker) keep the +# metadata path unchanged. + + +def _make_migrated_reflection_record(rec_id: str, **body_overrides) -> dict: + """A migrated (SQLite-origin) reflection: all rating state in the JSON + content body, empty metadata (AWS drops it on client BASE records).""" + body = { + "title": "Migrated title", + "use_cases": "when X", + "hints": ["h1"], + "confidence": 0.8, + "tech": None, + "phase": "general", + "evidence_count": 3, + "updated_at": "2026-05-01T00:00:00+00:00", + "polarity": "do", + "status": "active", + "useful_count": 0, + "times_misled": 0, + "times_overlooked": 0, + "missed_count": 0, + "ignored_count": 0, + "last_credited_at": "2026-05-01T00:00:00+00:00", + "source_row_id": "42", + "source_backend": "sqlite", + } + body.update(body_overrides) + return { + "memoryRecord": { + "memoryRecordId": rec_id, + "content": {"text": json.dumps(body)}, + "memoryStrategyId": "episodicReflections-qPr9876543", + "namespaces": ["projects/testproj/reflections/"], + "createdAt": datetime(2026, 5, 1, tzinfo=UTC), + # Empty — AWS drops the custom metadata map on client records. + "metadata": {}, + } + } + + +class _FakeEpisodicStore: + """Minimal in-memory bedrock-agentcore data client that PERSISTS content / + namespace updates, so a subsequent get_memory_record reflects them + (proves the migrated read-modify-write round-trips).""" + + def __init__(self, record: dict) -> None: + self._record = record + self.update_calls: list[dict] = [] + + def get_memory_record(self, *, memoryId, memoryRecordId): # noqa: N803 + return self._record + + def batch_update_memory_records(self, *, memoryId, records): # noqa: N803 + self.update_calls.append({"memoryId": memoryId, "records": records}) + mr = self._record["memoryRecord"] + for r in records: + if "content" in r: + mr["content"] = r["content"] + if "namespaces" in r: + mr["namespaces"] = r["namespaces"] + if "metadata" in r: + mr["metadata"] = r["metadata"] + return { + "successfulRecords": [ + {"memoryRecordId": records[0]["memoryRecordId"], "status": "SUCCEEDED"} + ], + "failedRecords": [], + } + + +def _migrated_backend(ac_config, mock_control_client, record) -> AgentCoreBackend: + return AgentCoreBackend( + config=ac_config, + data_client=_FakeEpisodicStore(record), + control_client=mock_control_client, + session_id="test-session-xyz", + project="testproj", + ) + + +def test_t2_credit_one_migrated_rewrites_body_counter( + ac_config, mock_control_client +) -> None: + record = _make_migrated_reflection_record("mig-1", useful_count=2) + backend = _migrated_backend(ac_config, mock_control_client, record) + fake = backend._data + + result = backend.credit_one( + session_id="s", kind="reflection", id="mig-1", classification="cited" + ) + assert result == {"applied": "cited", "skipped": None} + + # It was a CONTENT update, NOT a metadata update. + sent = fake.update_calls[-1]["records"][0] + assert "content" in sent + assert "metadata" not in sent + assert fake.update_calls[-1]["memoryId"] == "mem-epi-def4567890" + + body = json.loads(sent["content"]["text"]) + assert body["useful_count"] == 3 + assert body["source_backend"] == "sqlite" # marker preserved + # last_credited_at refreshed in the BODY (not metadata). + assert isinstance(body["last_credited_at"], str) + assert body["last_credited_at"] != "2026-05-01T00:00:00+00:00" + # updated_at MUST NOT move on a rating: sqlite crediting never bumps + # reflections.updated_at, and the reader resolves it body-first (§6.2), so + # bumping here would corrupt age_days + the updated_at DESC tiebreak parity. + assert body["updated_at"] == "2026-05-01T00:00:00+00:00" + + # Read-back through the reader reflects the bumped counter. + parsed = backend._parse_reflection_record( + fake.get_memory_record(memoryId="x", memoryRecordId="mig-1")["memoryRecord"] + ) + assert parsed is not None + assert parsed["useful_count"] == 3 + + +def test_t2_credit_one_migrated_overlooked_bumps_times_overlooked( + ac_config, mock_control_client +) -> None: + """classification 'overlooked' → metadata key overlooked_count, but in the + body it lands under the SQLite column name times_overlooked.""" + record = _make_migrated_reflection_record("mig-o", times_overlooked=4) + backend = _migrated_backend(ac_config, mock_control_client, record) + fake = backend._data + + result = backend.credit_one( + session_id="s", kind="reflection", id="mig-o", classification="overlooked" + ) + assert result == {"applied": "overlooked", "skipped": None} + + body = json.loads(fake.update_calls[-1]["records"][0]["content"]["text"]) + assert body["times_overlooked"] == 5 + assert "overlooked_count" not in body + + parsed = backend._parse_reflection_record( + fake.get_memory_record(memoryId="x", memoryRecordId="mig-o")["memoryRecord"] + ) + assert parsed is not None + assert parsed["_overlooked_count"] == 5 + + +def test_t2_record_use_migrated_success_rewrites_body( + ac_config, mock_control_client +) -> None: + record = _make_migrated_reflection_record("mig-r", useful_count=1) + backend = _migrated_backend(ac_config, mock_control_client, record) + fake = backend._data + + backend.record_use("mig-r", outcome="success") + + sent = fake.update_calls[-1]["records"][0] + assert "content" in sent and "metadata" not in sent + body = json.loads(sent["content"]["text"]) + assert body["useful_count"] == 2 + + parsed = backend._parse_reflection_record( + fake.get_memory_record(memoryId="x", memoryRecordId="mig-r")["memoryRecord"] + ) + assert parsed is not None + assert parsed["useful_count"] == 2 + + +def test_t2_record_use_migrated_failure_bumps_missed_count_in_body( + ac_config, mock_control_client +) -> None: + record = _make_migrated_reflection_record("mig-f", missed_count=0) + backend = _migrated_backend(ac_config, mock_control_client, record) + fake = backend._data + + backend.record_use("mig-f", outcome="failure") + + body = json.loads(fake.update_calls[-1]["records"][0]["content"]["text"]) + assert body["missed_count"] == 1 + assert body["useful_count"] == 0 + + +def test_t2_retire_reflection_migrated_rewrites_body_status( + ac_config, mock_control_client +) -> None: + record = _make_migrated_reflection_record("mig-s") + backend = _migrated_backend(ac_config, mock_control_client, record) + fake = backend._data + + backend.retire_reflection(reflection_id="mig-s") + + sent = fake.update_calls[-1]["records"][0] + assert "content" in sent and "metadata" not in sent + # namespace move still applied via the namespaces field. + assert sent["namespaces"] == ["projects/testproj/retired/"] + body = json.loads(sent["content"]["text"]) + assert body["status"] == "retired" + + parsed = backend._parse_reflection_record( + fake.get_memory_record(memoryId="x", memoryRecordId="mig-s")["memoryRecord"] + ) + assert parsed is not None + assert parsed["_status"] == "retired" + + +def test_t2_credit_one_extracted_uses_metadata_path_unchanged( + backend, mock_data_client +) -> None: + """AWS-extracted reflection (no source_backend marker) → the existing + metadata write path, NOT a content update.""" + mock_data_client.get_memory_record.return_value = _make_record_response( + "ext-1", useful_count=2 + ) + mock_data_client.batch_update_memory_records.return_value = { + "successfulRecords": [{"memoryRecordId": "ext-1", "status": "SUCCEEDED"}], + "failedRecords": [], + } + result = backend.credit_one( + session_id="s", kind="reflection", id="ext-1", classification="cited" + ) + assert result == {"applied": "cited", "skipped": None} + + sent = mock_data_client.batch_update_memory_records.call_args.kwargs["records"][0] + assert "metadata" in sent + assert "content" not in sent + assert sent["metadata"]["useful_count"]["numberValue"] == 3 + assert set(sent["metadata"]["last_credited_at"]) == {"stringValue"} + + +def test_t2_record_use_extracted_uses_metadata_path_unchanged( + backend, mock_data_client +) -> None: + mock_data_client.get_memory_record.return_value = _make_record_response( + "ext-r", useful_count=1 + ) + mock_data_client.batch_update_memory_records.return_value = { + "successfulRecords": [{"memoryRecordId": "ext-r", "status": "SUCCEEDED"}], + "failedRecords": [], + } + backend.record_use("ext-r", outcome="success") + + sent = mock_data_client.batch_update_memory_records.call_args.kwargs["records"][0] + assert "metadata" in sent + assert "content" not in sent + assert sent["metadata"]["useful_count"]["numberValue"] == 2 diff --git a/website/agentcore-setup.md b/website/agentcore-setup.md index f93082d..779cc2f 100644 --- a/website/agentcore-setup.md +++ b/website/agentcore-setup.md @@ -24,6 +24,7 @@ Narrow policy — `bedrock-agentcore` (data plane) + `bedrock-agentcore-control` "bedrock-agentcore-control:GetMemory", "bedrock-agentcore-control:ListMemories", "bedrock-agentcore-control:DeleteMemory", + "bedrock-agentcore-control:UpdateMemoryStrategy", "bedrock-agentcore:CreateEvent", "bedrock-agentcore:ListEvents", "bedrock-agentcore:BatchCreateMemoryRecords", @@ -38,7 +39,7 @@ Narrow policy — `bedrock-agentcore` (data plane) + `bedrock-agentcore-control` } ``` -For tighter scoping, restrict `Resource` to the two memory ARNs after `init` writes them. +For tighter scoping, restrict `Resource` to the two memory ARNs after `init` writes them. `UpdateMemoryStrategy` is only exercised by `better-memory agentcore migrate` — it widens an older semantic memory's schema so it declares `source_row_id` — and is not needed by `init`, `status`, or `smoke`; drop it if you never migrate. ## Initialise @@ -75,6 +76,38 @@ better-memory agentcore smoke Drives a minimal observe → list_events → batch_create → list_records → batch_delete cycle. Exit 0 means the round-trip works end-to-end. Smoke validates AWS credentials and wire access, not MCP registration. This is the recommended ops check after any region or credential change. +## Migrate existing memory (optional) + +`init` gives you empty AgentCore memories. If you already have sqlite memory worth carrying across, `better-memory agentcore migrate` bulk-copies it into the two AgentCore memories: + +```bash +better-memory agentcore migrate --dry-run # preview the plan, zero AWS calls +better-memory agentcore migrate # execute +``` + +It migrates two kinds by default (`--include reflections,semantic`): + +- **Reflections** → the episodic memory. All of a migrated reflection's state — the rating counters, its `status`, and the `source_row_id` used for de-duplication — lives in the record's **JSON content body**, not its metadata. (Cloud-extracted records store that state in record *metadata*; migrated reflections can't, because the built-in episodic strategy owns the metadata schema.) +- **Semantic memories** → the semantic memory. Here `source_row_id` and the counters live in **declared metadata**, so migrate first ensures the semantic strategy schema declares `source_row_id` — widening it in place if needed — because AWS silently drops undeclared metadata keys on client writes. + +Migration is **idempotent**: a per-row ledger (the `agentcore_migration` table in `memory.db`) plus a reconcile-by-`source_row_id` scan means a re-run converges instead of duplicating. A crashed or partial run is safe to re-run; failed records are marked in the ledger and retried next time. Only one run at a time — a `/agentcore_migration.lock` file guards against concurrent runs. + +Flags (read from the `migrate` argparse): + +| Flag | Effect | +|---|---| +| `--dry-run` | Compute the plan and print create/update/skip/retire tallies without any AWS call. | +| `--include ` | Comma list of kinds to migrate (default `reflections,semantic`). `observations` is accepted but **lossy and non-retrievable** — observation replay is not performed; they are treated as already distilled into reflections. | +| `--restart` | Re-verify/upsert every eligible row, ignoring the ledger's `migrated` state. | +| `--provision` | Create the target memories (or replace a missing / not-ACTIVE one) before migrating, instead of hard-erroring; persists any new IDs to `agentcore.json`. | +| `--project ` | Limit migration to one project (plus `general`-scope rows). | +| `--db ` | Source SQLite path (default `/memory.db`). | +| `--batch-size ` | Records per `BatchCreateMemoryRecords` call (default 25). | +| `--verify` | Read one migrated record per kind back via `GetMemoryRecord` and diff it against SQLite. | +| `--region ` / `--home

` | Region / home overrides (default: region from `agentcore.json`, home from `$BETTER_MEMORY_HOME`). | + +**Migration does not activate the backend.** `migrate` only writes records into AWS; it never touches `settings.json`. Activate separately with `init` (or by setting `storage_backend` yourself) — see [Initialise](#initialise). Exit codes: `0` all rows converged, `2` completed with some failed records (re-run to retry), `1` a configuration or setup error. + ## What changes in agentcore mode - **Memory data lives in AWS.** Observations, reflections, semantic memories, and reinforcement all go to Bedrock AgentCore — `memory.observe`, `memory.retrieve`, `memory.retrieve_observations`, `memory.record_use`, the four `memory.semantic_*` tools, the rating tools (`memory.credit`, `memory.apply_session_ratings`, `memory.list_session_exposures`), and `memory.session_bootstrap` all dispatch to the AgentCore backend. A local `memory.db` is still created for hook-error logging and `knowledge.db` for knowledge tools; no memory content is stored in them. diff --git a/website/architecture.md b/website/architecture.md index b809740..d87e9c7 100644 --- a/website/architecture.md +++ b/website/architecture.md @@ -41,6 +41,9 @@ flowchart LR | Closure events | N/A | `CreateEvent(role=OTHER)` from Stop hook | | Episode tracking | Local `episodes` table | Internal to AgentCore (sessionId) | | Exposure log (`record_exposures`) | Writes `session_memory_exposure` rows (sources `bootstrap` / `retrieve` / `contextual`) | No-op — no exposure log, so the end-of-session rating sweep has nothing to trigger on; rating flows through `memory.credit` (and direct `memory.apply_session_ratings` calls) | +| Bulk import | N/A (clean start) | `better-memory agentcore migrate` copies existing sqlite reflections + semantic memories into AWS (idempotent, ledgered) | + +Migrating with `better-memory agentcore migrate` is distinct from activating: it only writes records to AWS and never flips `storage_backend`. A migrated reflection carries its rating counters, `status`, and `source_row_id` in the record's JSON **content body** — the built-in episodic strategy owns the metadata schema — whereas cloud-extracted records (and migrated semantic records) keep that state in record **metadata**. See [AgentCore setup > Migrate](agentcore-setup.md#migrate-existing-memory-optional). See [Configuration](configuration.md) for env vars and [AgentCore setup](agentcore-setup.md) for the agentcore path. diff --git a/website/configuration.md b/website/configuration.md index 8842b62..8a1c3c1 100644 --- a/website/configuration.md +++ b/website/configuration.md @@ -51,10 +51,11 @@ Under `BETTER_MEMORY_HOME`: ``` .better-memory/ -├── memory.db # observations, episodes, reflections, audit_log +├── memory.db # observations, episodes, reflections, audit_log (+ agentcore_migration ledger in agentcore mode) ├── knowledge.db # FTS5 index over knowledge-base/ ├── settings.json # optional; persists storage_backend selection (written by `agentcore init`) ├── agentcore.json # agentcore mode only: memory IDs + region (written by `agentcore init`) +├── agentcore_migration.lock # agentcore mode only: transient single-run lock held by `agentcore migrate` ├── spool/ # hook payloads awaiting drain │ └── .quarantine/ # malformed payloads (not deleted; kept for debug) ├── state/ # per-session context_seen_.json files (contextual_inject dedup)