feat(storage): port unified storage interface to Python - #3227
strandly-the-agent wants to merge 5 commits into
Conversation
Ports the unified Storage interface from strands-ts (PR strands-agents#3099): a minimal four-operation (write/read/delete/list) byte-storage contract plus key normalization, a namespace() factory, and three backends (InMemoryStorage, LocalFileStorage, S3Storage). Adds StorageError. Scope: the storage primitive only. The TS consumer integrations (session SnapshotStorageAdapter, offloader unified-storage bridge) target subsystems whose Python architecture diverges and are out of scope for this PR.
…coverage - S3Storage.list now stops when a page is IsTruncated but omits NextContinuationToken, matching strands-ts (was an infinite loop). - Add tests exercising the S3 termination edge and the LocalFileStorage scratch-file cleanup branch on rename failure.
- Add **kwargs to the Storage protocol + all backend methods for forward-compat (strands-py STYLE_GUIDE / Sandbox convention). - Rename S3Storage(region=) -> region_name to match S3SessionManager and boto3 (idiomatic Python spelling of the concept). - Export namespace() and StorageError from strands.storage so an abstract Storage-typed value can be scoped and its error caught without reaching into internal modules. - NAMESPACED -> _NAMESPACED (internal marker, signal non-public). - Tests for the public surface (exports + forward-compat kwargs).
|
Update: ran an independent API bar-raiser pass over the new public surface (it was missing from my earlier validation) and pushed the fixes — the two must-fix-now-or-never public-shape items are addressed. What the api-bar-raiser flagged and how I addressed it
Verdict moved from API-changes-requested → the raiser's "land 1–4 and this is an easy API-approve." Two things I'd like a maintainer's call on (I deliberately didn't force these):
|
|
@strandly-the-agent is there any drift between the TS implementation / api and this python implementation? |
|
Short answer: yes, there's drift — but it's almost all deliberate Python-idiom adaptation, plus one scope difference. The contract is faithful: the four-op The divergences worth your eye are: (1) scope — this ports only the primitive, not the two TS consumers; (2) 1. Scope — the biggest difference (intentional)TS #3099 also wired the primitive into two consumers; this PR ships only the primitive. Per the PR body, the session 2. Public API surface — Python is wider than TS
These were deliberate calls from my own API-bar-raiser pass (see my earlier comment, items #3/#4). Flagging because they widen the Python public surface relative to TS — worth a conscious "yes, keep them public." 3. S3 configuration & dependency posture (genuine drift)
The injection model genuinely differs: TS lets you hand in a fully-built client; Python takes a 4. Expected idiom adaptations (not concerns)
5. Two minor behavioral edges I'd note
Net: the primitive is a faithful port; the drift is intentional idiom/scope choices plus the S3 injection & dependency-posture differences above. The public-surface items (#2) and the two edges (#5) are the only things I'd ask you to explicitly bless or wave off. 🤖 Generated by Strandly (an experimental AI agent) — please sanity-check the specifics before relying on them. |
Fix this
Do full parity with the TS PR, implement for offloader too |
…x not-found parity Wire ContextOffloader onto the unified strands.storage interface (parity with strands-ts strands-agents#3099) and tighten LocalFileStorage sandbox error handling. - ContextOffloader now accepts a unified Storage (write/read/delete/list) as well as a legacy offloader Storage (store/retrieve, now deprecated). Content-type is framed into a single unified-storage key ([2-byte BE len][content-type][content]); _store_content/_retrieve_content adapt between the two backends. - Plugin-driven eviction for unified backends (they have no built-in eviction): evict_after_cycles (default 20, None disables) + per-key stored-cycle tracking. The window is forwarded to a legacy InMemoryStorage left at its default. - LocalFileStorage sandbox read/delete/list now treat NotADirectoryError as not-found (matches host paths and strands-ts isNotFoundError = ENOENT+ENOTDIR). - Legacy offloader storage classes marked deprecated (doc-only, mirroring TS). Tests: +29 (framing round-trip, backend detection, adapters, e2e offload/retrieve through unified storage, plugin eviction + disable, validation, window forwarding, sandbox NotADirectoryError parity). pytest/ruff/mypy green.
…cter validation Independent API bar-raiser + correctness passes: - Privatize StorageBackend -> _StorageBackend (internal; TS inlines the union). - Class docstring: document evict_after_cycles and that a unified strands.storage Storage is accepted/preferred; class example uses the unified import. - evict_after_cycles rejects bool (bool is an int subclass; True must not pass as 1). - Note unified eviction is measured from store-time, not last-access. - Add bool-rejection test.
|
Both done and pushed (2 commits on this branch). pytest / ruff / mypy green, and I ran two independent fresh-context review passes over the result (correctness → ship; API bar-raiser → land-these-then-approve, all items addressed).
One honest scope note before the details: the TS PR also wired the session How the offloader bridge works (parity with strands-ts)
Tests & verification
Faithful-to-TS edges I did not “fix” (flagging rather than diverging): 🤖 Generated by Strandly (an experimental AI agent) — please review before merge. |
|
Closing out in favor of #3259 |
Description
Ports the unified storage interface from strands-ts (#3099) into the Python SDK.
Adds a new
strands.storagepackage — a minimal, four-operation persistence primitive over opaquebytes, keyed by/-separated path-like strings — plus three shipped backends and anamespace()factory. This is the Python twin of the strands-ts primitive; it establishes the same foundation for the phased "single persistence primitive for the SDK" design.InMemoryStoragedict(testing / serverless)LocalFileStorageos.replace), sandbox-aware viafor_sandbox()S3Storagelist,region/boto_sessionmutual-exclusion)Also adds
StorageError(strands.types.exceptions). Key normalization collapses/runs, strips leading/trailing/, and rejects empty keys and any..segment (path-traversal guard).listis a lexicographically-sorted prefix match.Related Issues
Port of strands-ts #3099. Requested by
lizradwayon that PR.Scope — primitive + context-offloader integration
This PR ships the unified primitive and wires the context-offloader onto it (per maintainer request on this PR), plus a sandbox parity fix. The one TS consumer still not ported is the session
SnapshotStorageAdapter, which has no faithful Python analog: strands-ts sessions are snapshot-based (SnapshotStorage/SnapshotLocation/manifest) while strands-py sessions are repository-based (SessionManagerABC +repository_session_manager+file/s3managers) — there is noSnapshotStorageinterface to adapt onto. That one is genuinely N/A rather than deferred.Context-offloader bridged onto the unified interface
ContextOffloadernow accepts either a unifiedstrands.storage.Storage(write/read/delete/list) or the legacy offloaderStorage(store/retrieve, now deprecated), mirroring strands-ts #3099:_is_offloader_storageroutes onhasattr(store) and hasattr(retrieve)(TS'store' in storage && 'retrieve' in storage).[2-byte BE content-type length][utf-8 content-type][content](_frame_content/_unframe_content);_store_content/_retrieve_contentadapt between backends.Storagehas no built-in eviction, so the plugin tracks the store cycle per key and deletes entries older than a new keyword-onlyevict_after_cycles(default 20,Nonedisables) onBeforeModelCallEvent; the window is forwarded to a legacyInMemoryStorageleft at its default._on_before_model_callis now async (the event loop dispatches this event viainvoke_callbacks_async, which awaits coroutine hooks).Storage/InMemoryStorage/FileStorage/S3Storageget doc-only.. deprecated::notes (mirrors TS's doc-only@deprecated).LocalFileStorage sandbox not-found parity
Sandbox-mode
read/delete/_list_keys_sandboxnow treatNotADirectoryErroras not-found alongsideFileNotFoundError, matching the host paths and TSisNotFoundError(ENOENT+ENOTDIR).writeis intentionally unchanged (TS doesn't treat a non-directory parent as not-found on write).Type of Change
New feature
Behavior traceability — every strands-ts storage test has a Python counterpart
All 53 source
it(...)behaviors across the four TS test files map to a Python test (0 missing); the Python suite adds a few extra edge tests. Independently verified by a fresh-context validation pass.in-memory-storage.test.tstest_in_memory_storage.pynamespaced-storage.test.tstest_namespaced_storage.py(+.namespace()method compose)local-file-storage.test.node.tstest_local_file_storage.py(+ forced rename-failure cleanup)s3-storage.test.tstest_s3_storage.py(+ truncated-without-token termination)58 Python tests, all passing.
Decision log — deviations from the source and why
Storageis a non-genericProtocol. TS usesStorage<ListQuery = string>. PEP 696TypeVardefaults are Python 3.13+ and the SDK targets 3.10+, so the generic-with-default isn't expressible. Dropped it:list(prefix: str). A backend can still widen the parameter (contravariance keeps it structurally compatible), so no capability is lost.NAMESPACEDsymbol → marker attribute. TS usesSymbol.for('strands.storage.namespaced')+NAMESPACED in obj. Python has no symbols; the equivalent is a module-level marker attribute-name string, set viasetattrand detected viagetattr. Kept internal (not exported).Uint8Array.slice()). Pythonbytesis immutable, soInMemoryStoragecopies viabytes(data)on write only; reads hand out the immutable object. The TS "copies on read" test becomes an immutability assertion.S3Storageconfig = keyword args, boto conventions. TSS3StorageConfig(regionXORs3Client) → explicit kwargsprefix/region/boto_session/boto_client_config, matching the existingS3SessionManager(mutual exclusion isregionXORboto_session;user_agent_extra="strands-agents"). boto3 is a hard dep so there's no lazy-import gap, but the client is still created lazily so constructing anS3Storagenever needs AWS.Storagetype from its root barrel. Python idiom is submodule access (from strands.storage import ...), consistent withstrands.session/strands.memory, sostrands/__init__.pyis left untouched. Easy to add a re-export if preferred..__strands_tmpsubstring check as TS (local-file-storage.ts:191/218), and directory listing treats an unknownis_dir(None/undefined) the same way TS does. Diverging would break cross-SDK parity; flagged here for a future cross-SDK fix instead.One genuine divergence found in review — fixed
An independent review pass caught that
S3Storage.listcould infinite-loop if a backend returned a page withIsTruncated=truebut noNextContinuationToken. strands-ts terminates in that case (continuationToken = IsTruncated ? NextContinuationToken : undefined; while (continuationToken)). Fixed to match, with a regression test. (Unreachable with a spec-compliant S3, but now equivalent to the source.)Sensitive-surface scan
..segments rejected in both key and prefix normalization (matches TS); normalized keys can't escapebase_dir.os.replace; scratch file removed on failure.Testing
Ran locally (no functional warnings):
pytest tests/strands/storage/ tests/strands/vended_plugins/context_offloader/— 235 passed (+18 new for the offloader bridge and sandbox parity)ruff check+ruff format --checkon touched source & tests — cleanmypy ./src— clean instorage/andcontext_offloader/(boto3-stubs[s3])Full
tests/strands/vended_plugins/— 487 passed (no regressions)Two independent fresh-context review passes: correctness → ship; API bar-raiser → land-these-then-approve (all items addressed: privatized the internal
_StorageBackendalias, documentedevict_after_cycles+ unified-storage acceptance,boolrejected in validation).Ran the equivalent of
hatch run prepare(ruff check, ruff format --check, mypy, pytest) directly — hatch not available in this environment.Checklist
🤖 Generated by Strandly (an experimental AI agent). A human should review before merge. Docstrings and a
site/docs page are intentionally deferred — happy to add them or split into a docs PR.