feat(lifecycle): add forget flag + TTL to v2 ingest API (#166)

[chiragkhatri19]

Summary

Closes part of #166 (memory lifecycle — PR #1 of a planned series).

This PR ships the forget + TTL feature on the v2 ingest path, as agreed with @ishaanxgupta before opening. Versioning and decay are the follow-up PRs.

What this adds

• forget: bool flag on POST /v2/memory/ingest — when true, the stored memory is tagged with forget=true and expires_at = now + TTL (default 30 days, configurable via MEMORY_FORGET_DEFAULT_TTL_DAYS).
• Retrieval-time filtering — expired/forgotten records are hidden from _search_summary and the profile catalog. Legacy records (no lifecycle keys) are never affected.
• No background sweeper required — enforcement is purely at read time. All backends already store metadata as an arbitrary JSON blob, so no schema migration is needed.

Architecture

v2 and v1 share the same IngestPipeline and Weaver; the v2 layer is only the route + Temporal workflow/activities. The feature is wired additively via extra_metadata=None all the way down — v1 callers and any ingest without forget=true are byte-identical to before.

Files changed

File	Change
src/api/schemas.py	Add forget: bool = False to IngestRequest
src/api/routes/v2/memory.py	Compute lifecycle_metadata when forget=true; include in idempotency
src/api/routes/v2/activities.py	Pass lifecycle_metadata into domain/pipeline activity state
src/pipelines/ingest.py	Pass extra_metadata to weaver.execute() in profile/image/summary nodes
src/pipelines/weaver.py	execute(extra_metadata=None) merged at both ADD sites; _now injectable
src/pipelines/retrieval.py	is_retrievable filter in _search_summary + _fetch_profile_catalog
src/pipelines/lifecycle.py	NEW — pure is_retrievable(meta, now) + build_lifecycle_metadata()
src/schemas/memory_lifecycle.py	NEW — MemoryLifecycle Pydantic model + RESERVED_LIFECYCLE_KEYS
src/config/settings.py	Add memory_forget_default_ttl_days: float = 30.0
tests/conftest.py	Fix InMemoryVectorStore.update() to merge metadata (not replace) — prod backends merge; fake was replacing, a false-green risk
tests/unit/test_memory_lifecycle.py	NEW — 13 unit tests
tests/integration/test_weaver_pipeline.py	2 new lifecycle round-trip integration tests
tests/integration/test_ingest_pipeline.py	Update RecordingWeaver.execute() signature
CHANGELOG.md	User-visible entry

Test plan

• ruff check — zero errors on all changed files
• 25 new/updated tests pass (13 unit + 10 integration + 2 e2e including existing)
• Coverage gate: 75.35% (gate: 70%)

uv run python -m pytest tests/unit/test_memory_lifecycle.py tests/integration/test_weaver_pipeline.py tests/integration/test_ingest_pipeline.py tests/e2e/test_memory_flow.py -v
# → 25 passed

On versioning (answering @ishaanxgupta's question)

Yes — the Weaver._vector_update seam is exactly right. That is the one place the pipeline currently overwrites and loses prior memory content. The plan for PR #2: before the overwrite, read the existing record, write a new version record with version = old_version + 1 and parent_memory_id, then flip the old record to is_current = False. Readers pick max(version) among is_current records. No cross-store transaction needed.

Out of scope (follow-up PRs)

• Versioning (Weaver._vector_update parent lineage)
• Forget for temporal/code/snippet domains
• Background TTL sweeper
• Decay (to be merged with the forget/TTL mechanism later, per maintainer)

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces a memory forget/TTL lifecycle feature for the v2 ingest API, allowing memories ingested with forget=true to be tagged with an expiration time and automatically filtered out at retrieval time. The feedback highlights several critical improvements: addressing a potential security vulnerability in weaver.py where system metadata fields could be overwritten by filtering extra_metadata with RESERVED_LIFECYCLE_KEYS; utilizing the new build_lifecycle_metadata helper in the API route to avoid duplication; making is_retrievable and metadata retrievals defensive against None values to prevent runtime crashes; and optimizing the datetime parsing logic in the lifecycle helper.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Merging extra_metadata directly into meta without sanitization or filtering can allow overwriting critical system metadata fields like user_id or domain. We should use the newly defined RESERVED_LIFECYCLE_KEYS constant to ensure only valid lifecycle fields are merged, preventing cross-user data contamination or privilege escalation.

Suggested change

	if extra_metadata:
	meta.update(extra_metadata)
	if extra_metadata:
	from src.schemas.memory_lifecycle import RESERVED_LIFECYCLE_KEYS
	meta.update({k: v for k, v in extra_metadata.items() if k in RESERVED_LIFECYCLE_KEYS})

[gemini-code-assist]

Merging extra_metadata directly into metadata in _vector_add without filtering can allow overwriting critical system metadata fields like user_id or domain. We should filter extra_metadata using RESERVED_LIFECYCLE_KEYS to ensure only valid lifecycle fields are merged.

Suggested change

	if extra_metadata:
	metadata.update(extra_metadata)
	if extra_metadata:
	from src.schemas.memory_lifecycle import RESERVED_LIFECYCLE_KEYS
	metadata.update({k: v for k, v in extra_metadata.items() if k in RESERVED_LIFECYCLE_KEYS})

[gemini-code-assist]

Instead of manually constructing the lifecycle_metadata dictionary, use the newly introduced build_lifecycle_metadata helper function from src.pipelines.lifecycle. This avoids code duplication and ensures that the created_at and updated_at fields are consistently populated on ingested records as designed.

    if req.forget:
        from src.pipelines.lifecycle import build_lifecycle_metadata
        now = datetime.now(timezone.utc)
        payload["lifecycle_metadata"] = build_lifecycle_metadata(
            now=now,
            ttl_days=settings.memory_forget_default_ttl_days,
        )

[gemini-code-assist]

The is_retrievable function should be defensive against metadata being None or not a mapping. If metadata is None, calling metadata.get will raise an AttributeError. Adding a guard clause to return True (treating it as a legacy-safe active record) prevents potential runtime crashes.

Suggested change

	def is_retrievable(metadata: Mapping[str, Any], now: datetime) -> bool:
	"""Return True when a record should appear in retrieval results.
	Rules (applied in order):
	1. ``lifecycle_state == "forgotten"`` → hidden (manual soft-forget).
	2. ``forget is True`` and ``expires_at`` is present and in the past → hidden (TTL expired).
	3. Everything else (including all legacy records with no lifecycle keys) → retrievable.
	Missing keys default to the legacy-safe value so records stored before
	lifecycle was introduced are never hidden.
	"""
	if metadata.get("lifecycle_state", "active") == "forgotten":
	return False
	def is_retrievable(metadata: Optional[Mapping[str, Any]], now: datetime) -> bool:
	"""Return True when a record should appear in retrieval results.
	Rules (applied in order):
	1. ``lifecycle_state == "forgotten"`` → hidden (manual soft-forget).
	2. ``forget is True`` and ``expires_at`` is present and in the past → hidden (TTL expired).
	3. Everything else (including all legacy records with no lifecycle keys) → retrievable.
	Missing keys default to the legacy-safe value so records stored before
	lifecycle was introduced are never hidden.
	"""
	if not metadata:
	return True
	if metadata.get("lifecycle_state", "active") == "forgotten":
	return False

[gemini-code-assist]

If expires_raw is already a datetime object (which can happen depending on the vector store backend or deserialization), calling str(expires_raw) and then datetime.fromisoformat is highly inefficient and redundant. We should check if it is already a datetime instance before parsing.

Suggested change

	expires_at = datetime.fromisoformat(str(expires_raw))
	# Make both sides timezone-aware or both naive for comparison
	if expires_at.tzinfo is None and now.tzinfo is not None:
	from datetime import timezone
	expires_at = expires_at.replace(tzinfo=timezone.utc)
	elif expires_at.tzinfo is not None and now.tzinfo is None:
	expires_at = expires_at.replace(tzinfo=None)
	if expires_at < now:
	return False
	except (ValueError, TypeError):
	pass
	if isinstance(expires_raw, datetime):
	expires_at = expires_raw
	else:
	expires_at = datetime.fromisoformat(str(expires_raw))
	# Make both sides timezone-aware or both naive for comparison
	if expires_at.tzinfo is None and now.tzinfo is not None:
	from datetime import timezone
	expires_at = expires_at.replace(tzinfo=timezone.utc)
	elif expires_at.tzinfo is not None and now.tzinfo is None:
	expires_at = expires_at.replace(tzinfo=None)
	if expires_at < now:
	return False
	except (ValueError, TypeError):
	pass

[gemini-code-assist]

If r.metadata is None (which is possible in some backends or mock tests, and is now handled safely by is_retrievable), calling r.metadata.get("main_content", "") on line 525 will raise an AttributeError. We should make this access defensive.

Suggested change

	for r in results:
	if not is_retrievable(r.metadata, now):
	continue
	live_results.append(r)
	main_content = r.metadata.get("main_content", "")
	if not main_content or main_content in seen:
	continue
	for r in results:
	if not is_retrievable(r.metadata, now):
	continue
	live_results.append(r)
	main_content = r.metadata.get("main_content", "") if r.metadata else ""
	if not main_content or main_content in seen:
	continue

[greptile-apps]

Greptile Summary

This PR adds a forget: bool flag and TTL-based lifecycle enforcement to the v2 ingest path, introducing is_retrievable() as a read-time filter for both _search_summary and the profile catalog. It ships with a new MemoryLifecycle schema, a pure lifecycle.py helper module, and fixes a pre-existing InMemoryVectorStore.update() test fake that was replacing metadata instead of merging it.

• New lifecycle module (lifecycle.py, memory_lifecycle.py) provides is_retrievable() and build_lifecycle_metadata() as side-effect-free helpers; the extra_metadata channel is wired additively through Weaver.execute() → _execute_batched_vector → _vector_add / _vector_update, so callers without forget=true are byte-identical to before.
• Retrieval filtering is applied at read time in _search_summary and _fetch_profile_catalog (and by extension _search_profile, which reuses the catalog's cached live results); no schema migration or background sweeper is required.
• The PR addresses the previously reported P1 regression where UPDATE ops in the batched vector path were silently dropping extra_metadata; regression tests are included for ADD, UPDATE, UPDATE→ADD-fallback, and mixed batches.

Confidence Score: 4/5

The forget/TTL feature is implemented correctly end-to-end for ADD and UPDATE ops; the previously reported batch-UPDATE regression is addressed with regression tests. Two open inline comments from prior reviews on memory.py (inline lifecycle dict diverging from build_lifecycle_metadata, and the TTL-absent idempotency hash) remain unaddressed and should be resolved before merging.

The core lifecycle logic, sanitization boundary, and retrieval filter are all correct and well-tested. The open prior-review comments on memory.py represent two concrete divergences between the production ingest path and the tested helper, one of which silently omits created_at/updated_at from stored records. Resolving those before merge would bring this to a clean pass.

src/api/routes/v2/memory.py — two prior-review comments (inline lifecycle dict and TTL in idempotency) are still open and unaddressed.

Important Files Changed

Filename	Overview
src/pipelines/lifecycle.py	New pure helper module — is_retrievable() handles naive/aware datetime normalization and legacy-safe defaults; build_lifecycle_metadata() builds the full TTL dict. Well-tested with 13 unit tests.
src/pipelines/weaver.py	extra_metadata is sanitized once at execute() and forwarded through every write path (batched ADD, UPDATE, UPDATE→ADD fallback); previously reported UPDATE regression is now fixed. Weaver._now is stored but never called — unused seam.
src/pipelines/retrieval.py	is_retrievable() filter correctly applied in _search_summary and _fetch_profile_catalog; _search_profile inherits filtering via cached live_results. Docstring for _fetch_profile_catalog still says 'raw_results — the full SearchResult list', which is now stale.
src/api/routes/v2/memory.py	build_lifecycle_metadata() called before idempotency check; batch-ingest guard added correctly. Inline lifecycle dict in the route still diverges from build_lifecycle_metadata() (flagged in prior review).
src/schemas/memory_lifecycle.py	New schema with RESERVED_LIFECYCLE_KEYS and PROTECTED_METADATA_KEYS constants; clean separation of lifecycle schema from denylist logic. RESERVED_LIFECYCLE_KEYS is exported but not imported by any changed file — currently unused.
tests/conftest.py	InMemoryVectorStore.update() now merges metadata (matching prod backends) instead of replacing — this was a false-green risk for lifecycle tests; a dedicated unit test locks the fix.
tests/integration/test_weaver_pipeline.py	10 new integration tests cover ADD/UPDATE/batch/fallback paths, protected-key denylist, unknown-key passthrough, and the full expired→hidden round-trip via RetrievalPipeline.
src/pipelines/ingest.py	lifecycle_metadata threaded into profile, summary, and image nodes via extra_metadata=state.get('lifecycle_metadata'); temporal/code/snippet nodes remain out of scope per PR description.
src/api/routes/v2/activities.py	lifecycle_metadata extracted once from payload and threaded into all four domain node dicts (profile, temporal, summary, image); temporal will silently ignore it until a follow-up PR, which is intentional.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Route as v2/memory.py
    participant Activity as activities.py
    participant Pipeline as ingest.py
    participant Weaver as weaver.py
    participant Store as VectorStore

    Client->>Route: "POST /v2/memory/ingest (forget=true)"
    Route->>Route: build_lifecycle_metadata(now, ttl_days)
    Route->>Activity: enqueue job with lifecycle_metadata
    Activity->>Pipeline: "_node_extract_*(state with lifecycle_metadata)"
    Pipeline->>Weaver: execute(judge_result, domain, user_id, extra_metadata)
    Weaver->>Weaver: sanitize PROTECTED_METADATA_KEYS
    Weaver->>Store: add/update with forget+expires_at merged
    Note over Client,Store: Retrieval time
    Client->>Pipeline: retrieve(query)
    Pipeline->>Store: search results
    Store-->>Pipeline: raw results
    Pipeline->>Pipeline: is_retrievable(metadata, now())
    Pipeline-->>Client: filtered results

Loading

_{Reviews (2): Last reviewed commit: "fix(lifecycle): patch UPDATE data-leak, ..." | Re-trigger Greptile}

[ishaanxgupta]

Hi @chiragkhatri19 have a look on the suggestions

[chiragkhatri19]

Review remediation — commit 8ae9176

Addressing all reviewer findings before re-review. Summary of changes:

Greptile P1 (UPDATE path drops lifecycle — data-leak)

_vector_update, _execute_vector, _execute_one, and the batched UPDATE/fallback branches in _execute_batched_vector now all accept and forward extra_metadata. The _vector_update → _vector_add fallback carries it too. Regression test test_weaver_update_preserves_extra_metadata_REGRESSION was failing on the previous commit and now passes, pinning the fix. Two more tests cover the fallback-to-add path and cross-flush-boundary propagation.

Gemini security-high ×2 (unsanitized extra_metadata merge)

execute() sanitizes extra_metadata once at the entry point using a denylist:

PROTECTED_METADATA_KEYS = frozenset({"user_id", "domain"})

All downstream merge sites use a single _merge_extra_metadata helper, so the sanitized value is what reaches storage. Why denylist, not the suggested allowlist: PR #2 (versioning) will thread version/parent_memory_id/is_current through this same channel — an allowlist would silently drop them, reproducing this bug class. A denylist of identity keys is forward-compatible. Tests test_weaver_extra_metadata_cannot_override_user_id + test_weaver_extra_metadata_passthrough_unknown_key pin both properties.

Gemini medium — use build_lifecycle_metadata helper

Route now calls build_lifecycle_metadata(now=..., ttl_days=...) instead of the inline dict, picking up created_at/updated_at that were previously missing.

Gemini medium — is_retrievable None guard

Signature is now Optional[Mapping[str, Any]]; returns True on None (legacy-safe).

Gemini medium — datetime parse short-circuit

isinstance(expires_raw, datetime) check avoids fromisoformat(str(datetime_obj)) round-trip.

Gemini medium — retrieval None guard

_fetch_profile_catalog uses (r.metadata or {}).get("main_content", "").

Not raised by bots — batch forget=true silent no-op → HTTP 400

batch_ingest_memory_v2 now rejects any request containing forget=true items with a 400 before any job is enqueued. Per-item batch forget is PR #2 scope.

Greptile P2 — TTL in idempotency hash (respectfully deferred)

content_hash intentionally does not include ttl_days. Idempotency = "same request → same job"; the request didn't change, the server config did. Folding server config into an idempotency key is wrong semantics. The correct fix is a client-supplied forget_ttl_days field (PR #2), at which point it does belong in the hash. Added a code comment at the hash site documenting this known limitation.

Tests

15 new tests added; full suite is 92/92 passing.