[SudeepGanguly-20]

Description

Adds a Thread Registry to the agent_memory module — a new abstraction for managing LangGraph agent thread lifecycle (creation, retrieval, listing, updating, deletion, TTL-based expiry).

Key changes:

• create_thread_registry() factory: auto-detects credentials and returns either a SAPAgentMemoryThreadRegistry (persistent, backed by Agent Memory Service) or an InMemoryThreadRegistry (in-process fallback). Supports a ttl_seconds parameter for thread inactivity expiry.
• ThreadRegistryEntry model and ThreadPage pagination type added to the public API.
• BaseThreadRegistry abstract base class defining the contract: create_or_register_thread, get_thread, list_threads, update_thread, delete_thread.
• threadDays retention field added to thread model, alongside the new ttl_seconds parameter on the registry.
• IAS ContextVar flow extended to create_thread_registry() — IAS tokens in the SDK context are forwarded automatically.
• create_checkpointer(ttl_seconds=...) deprecated: TTL is now owned by the thread registry. The parameter still works but emits a DeprecationWarning and a telemetry metric. A bare create_checkpointer() is unaffected.
• Telemetry: all thread registry operations and the deprecated checkpointer path are instrumented with Operation metrics.
• Unit and integration tests for all new functionality.
• User guide updated with thread registry usage, IAS flow, and deprecation guidance.

Type of Change

• New feature (non-breaking change that adds functionality)
• Bug fix (non-breaking change that fixes an issue)

How to Test

1. Install the package with extras: pip install sap-cloud-sdk[langgraph]
2. In-memory path (no credentials required):

   from sap_cloud_sdk.agent_memory.thread_registry import create_thread_registry
   
   registry = create_thread_registry(ttl_seconds=3600)
   registry.create_or_register_thread("agent-a", "user-1")
   thread = registry.get_thread(registry.create_or_register_thread("agent-a", "user-1").thread_id)

3. Service path (with Agent Memory Service credentials in environment):

   registry = create_thread_registry()  # auto-detects SAPAgentMemoryThreadRegistry

4. Deprecated ttl_seconds on create_checkpointer:

   import warnings
   with warnings.catch_warnings(record=True) as w:
       warnings.simplefilter("always")
       create_checkpointer(ttl_seconds=3600)
   assert issubclass(w[0].category, DeprecationWarning)

5. Run unit tests: uv run pytest tests/agent_memory/ -m "not integration" -q

Checklist

• My code follows the project's Code Guidelines
• I have added type hints for all public APIs
• I have added unit tests covering the new functionality
• I have added or updated the user guide (user-guide.md) in the module directory
• All existing tests pass locally
• My commits follow Conventional Commits format
• No confidential or SAP-internal information is included

Additional Notes

• create_checkpointer(ttl_seconds=...) remains functional (returns TimedInMemorySaver) to avoid breaking existing users, but emits a DeprecationWarning. It will be removed in a future major release.
• The IAS ContextVar token flow (IAS_TOKEN context key) is forwarded transparently through both the checkpointer and thread registry — callers do not need to pass tokens explicitly.

Disclaimer: No SAP-internal or customer-specific information is included in this PR. This is a public repository.

[cla-assistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[cassiofariasmachado]

The structured-code path checks error.code, errorCode, and code fields. This is correct. The fallback searches for "expired" in both the exception message and the response body. This is too broad. A gateway error such as "OAuth token has expired" in a 409 body will cause _is_expired_response to return True. The caller then gets AgentMemoryThreadExpiredError instead of AgentMemoryHttpError.

Fix: search only the response body in the fallback. Or document the exact string the service returns and match against that value only.

[cassiofariasmachado]

After the PATCH call succeeds, _get_thread runs a GET call to get the updated entry. Every call to update_thread costs two round trips. If the PATCH response returns the updated entity, the SDK can drop the GET call and reduce the cost to one round trip. Consider returning the updated entity in the PATCH response now or in a future service version.

[cassiofariasmachado]

The except AgentMemoryNotFoundError: pass block is correct. It follows the idempotency contract in _base.py:161. The problem is that there is no log entry, not even at DEBUG level. A double-delete leaves no trace in production logs.

Fix: add logger.debug("delete_thread: thread %r not found, treating as no-op", thread_id) in the except block.

[cassiofariasmachado]

All public methods use the parameter name tenant_id. The value is forwarded to HttpClient as tenant_subdomain. The expected value is a BTP subdomain string such as "acme-corp", not a UUID.

Option 1: rename the parameter to tenant_subdomain in the base class and both implementations.
Option 2: keep tenant_id and add a docstring entry: tenant_id (str): The BTP tenant subdomain string, for example "acme-corp".

The rename is cleaner. The docstring change has less impact.

[cassiofariasmachado]

The call is entry.is_expired(self._default_ttl). The name self._default_ttl suggests it is the authoritative TTL. It is not. is_expired uses it only when the entry has no thread_ttl value set (_in_memory.py:39-43). The logic is correct. The name makes it look wrong.

Fix: add a short inline comment at the call site to say that self._default_ttl is the registry-level fallback, not the per-thread TTL.

[cassiofariasmachado]

When IAS_TOKEN is absent from the context, get_session returns a session with no Authorization header. The caller in _service.py:207-208 calls _require_ias_token() first, so the unset-token path is never reached in practice. But someone reading IasAuthProvider in isolation sees a silent return that produces a valid-looking session object with no header set.

Fix: add logger.debug("IasAuthProvider: no IAS token in context, session has no Authorization header") at the silent-return branch. This does not change behavior.

[cassiofariasmachado]

All five public methods accept tenant_id but do not use it. All threads go into the same store. A developer who tests locally with InMemoryThreadRegistry and passes tenant_id will see no error but will get shared storage across all tenants.

Fix: add a note to the user guide under the Thread Registry section. State that InMemoryThreadRegistry accepts tenant_id but ignores it, and that tenant isolation is not available in this mode.

[cassiofariasmachado]

Please try to keep the paragraphs on the same line as shown below:

Suggested change

	The thread registry tracks conversation threads — their owner, idle TTL, and expiry. Unlike
	the LangGraph checkpointer, it is **framework-agnostic**: it works with LangGraph, Pydantic AI,
	CrewAI, or any custom agent.
	The thread registry tracks conversation threads — their owner, idle TTL, and expiry. Unlike the LangGraph checkpointer, it is **framework-agnostic**: it works with LangGraph, Pydantic AI, CrewAI, or any custom agent.

Please review the rest of the documentation (both user guides).

[cassiofariasmachado]

Please update the Multitenancy section with the new IAS mode option.

[cassiofariasmachado]

Consider creating methods to abstract these rules. This will make the code cleaner. Also, review the _service.py and consider the same strategy.

[cassiofariasmachado]

PR Review: #408 — feat(agent-memory): add thread registry and in-memory checkpoint support

Author: SudeepGanguly-20 Branch: contribs/agent-memory-checkpoint-inmemory → main
Verdict: ❌ Blocked
Summary: The type-check CI job fails due to invalid test assertions against auth_mode, and the CLA has not been signed.

---

A: Process & Compliance

#	Criterion	Status	Finding
A1	PR template complete	✅ Pass	All 9 core checklist items ticked. New module adds functionality to an existing module (not a brand-new module path), so the e2e evidence item is N/A. No security-sensitive auth changes that would require /security-review.
A2	Conventional Commits	✅ Pass	PR title feat(agent-memory): ... passes commitlint; Commit Validation CI job is green.
A3	Issue linked	⚠️ Warning	No Closes #N in PR body.
A4	AI-generated code disclosure	✅ Pass	No disclosure required; diff does not exhibit AI generation patterns.

B: Security & Sensitive Data

#	Criterion	Status	Finding
B1	No sensitive data in code	✅ Pass	No hardcoded credentials, internal URLs, or tenant IDs in the diff.
B2	No sensitive data in PR body	✅ Pass	PR body contains only public information; disclaimer present.

C: Code Quality

#	Criterion	Status	Finding
C1	CI checks passing	❌ Fail	Code Quality Checks (ty type check) is failing. Two diagnostics in tests/agent_memory/unit/test_config.py: (1) error[invalid-assignment] at line 107 — test assigns to the read-only auth_mode property (config.auth_mode = "xsuaa" with # type: ignore[misc] suppresses mypy but not ty); (2) error[unknown-argument] at line 109 — test instantiates AgentMemoryConfig(auth_mode="xsuaa"), which is not a valid constructor parameter. Additionally, the license/cla check is pending — the contributor has not signed the CLA.
C2	Version bump	➖ N/A	No manual version bump detected. Check Version Bump is green.
C3	Type hints	✅ Pass	All new public methods have full annotations. _create_timed_checkpointer is missing a return type annotation (-> TimedInMemorySaver) but it is a private helper.
C4	No hardcoded values	✅ Pass	_THREAD_COLLECTION = "/v1/agent/threads" is a module-level constant. No magic strings inline.
C5	Import organization	✅ Pass	All imports are top-level or inside TYPE_CHECKING blocks; no lazy imports introduced without justification.
C6	Naming conventions	✅ Pass	Enum values follow SCREAMING_SNAKE_CASE = "snake_case_value". New operations are in alphabetical order within their group. File names are snake_case.py.
C7	No unused code	✅ Pass	No unused imports or dead methods detected.
C8	No unjustified new dependencies	✅ Pass	No new runtime dependencies added. uv.lock bump is a transitive update only.
C9	Proto code freshness	➖ N/A	No .proto files changed.

D: API & Design

#	Criterion	Status	Finding
D1	API future-proofing	✅ Pass	create_thread_registry() factory present. BaseThreadRegistry ABC defines the contract cleanly. Configuration uses existing AgentMemoryConfig dataclass.
D2	Public API hygiene	✅ Pass	__init__.py and __all__ expose only public symbols. Internal helpers are prefixed with _ (_StoredEntry, _validate_ttl, _factory.py, etc.).
D3	Breaking changes marked	✅ Pass	create_checkpointer(ttl_seconds=...) is deprecated (not removed) with a proper DeprecationWarning and telemetry. No breaking changes.
D4	Pagination & tenant filtering	✅ Pass	list_threads follows the existing pagination pattern. tenant_id param present on all methods. OData $count=true / @odata.nextLink traversal matches existing modules.
D5	Telemetry instrumentation	✅ Pass	All five registry operations and the deprecated checkpointer path are decorated with @record_metrics. New operations added to core/telemetry/operation.py under the AGENT_MEMORY group.
D6	Multi-tenancy support	✅ Pass	tenant_id is threaded through every public method on both implementations, forwarded as tenant_subdomain to HttpClient. IAS ContextVar flow is wired. Documented in the user guide under ## Multitenancy.

E: Tests & Documentation

#	Criterion	Status	Finding
E1	Tests added/updated	✅ Pass	New test files for factory, in-memory, and service implementations. BDD feature file and scenario step runner added. All existing tests updated to reflect the new _build_agent_memory_http return signature (HttpClient, bool). Test names follow test_<functionality>_<condition>_<expected_result> convention.
E2	Documentation quality	✅ Pass	user-guide.md updated with a full Thread Registry section, TTL lifecycle explanation, resource management guidance, context-manager usage, and the deprecation notice for create_checkpointer(ttl_seconds=...).
E2a	Multi-tenancy documentation	✅ Pass	Existing ## Multitenancy section (line 179 of user-guide.md) covers XSUAA and IAS, documents access_strategy, and is updated with a note about IAS mode ignoring access_strategy/tenant. All four required fields are present.
E3	Module structure compliance	✅ Pass	thread_registry/ subpackage follows the standard structure: __init__.py, _base.py, _factory.py, _in_memory.py, _service.py. Tests mirror the layout under tests/agent_memory/.

---

❌ Blocking Issues

• [C1] Type check failure in test_config.py: ty reports two diagnostics that fail the Code Quality Checks CI job:

  • tests/agent_memory/unit/test_config.py:107 — config.auth_mode = "xsuaa" is an invalid assignment to a read-only @property. The # type: ignore[misc] suppresses mypy but ty still flags it. Fix: use monkeypatch.setattr(type(config), "auth_mode", property(lambda self: "xsuaa")) or restructure the test to avoid assigning to the property.
  • tests/agent_memory/unit/test_config.py:109 — AgentMemoryConfig(base_url="http://localhost:3000", auth_mode="xsuaa") passes a keyword argument that does not exist on the dataclass. Fix: remove auth_mode="xsuaa" from the constructor call; auth_mode is a derived property, not a field.

• [C1] CLA not signed: The license/cla check is pending. The contributor must sign the CLA before the PR can be merged.

⚠️ Non-Blocking Suggestions

• [C3] Missing return type on _create_timed_checkpointer: src/sap_cloud_sdk/agent_memory/factory/langgraph_checkpoint.py:34 — the private helper has no -> TimedInMemorySaver return annotation. Low impact since it is private, but annotating it would match the project's strict type-hint standard.

• [C1] Correctness concern in _get_live: src/sap_cloud_sdk/agent_memory/thread_registry/_in_memory.py:115 — _get_live calls entry.is_expired(self._default_ttl) but the per-thread TTL (entry.thread_ttl) is already factored into is_expired via its registry_ttl argument. The call is correct when the per-call ttl_seconds was folded into entry.thread_ttl at write time (which it is). However, create_or_register_thread stores ttl = self._effective_ttl(ttl_seconds) (which may be None if neither was set), and then _get_live evaluates expiry against self._default_ttl only — meaning a thread created with an explicit per-call ttl_seconds that differs from the registry default will have its expiry checked against the wrong TTL in _get_live. This is a latent bug worth investigating.

• [A3] No linked issue: Consider adding Closes #N or documenting that this feature does not have a tracked issue.

✅ Things Done Well

• The _is_expired_response helper in _service.py is a well-structured two-layer detection (structured JSON error codes + string-match fallback), which gracefully handles both current and future service error shapes.
• BaseThreadRegistry explicitly documents the async upgrade path (concrete non-abstract defaults via asyncio.to_thread), which prevents a future async addition from becoming a breaking change for subclass implementers.
• The deprecation of create_checkpointer(ttl_seconds=...) is handled correctly: DeprecationWarning at the call site, a logged warning, a dedicated _create_timed_checkpointer helper decorated with deprecated=True for telemetry, and clear migration guidance in both the code docstring and the user guide.
• Test coverage is thorough: 794-line service test file covers OData helpers, TTL edge cases, pagination via @odata.nextLink, IAS context forwarding, and all error branches.

[NicoleMGomes]

Suggested change

	Public API::
	Public API: