Conversation
Replace `Workspace`'s `Option<Storage>` with four non-optional trait objects — `PersistBackend`, `LoadBackend`, `LockBackend`, and `SessionBackend` — eliminating pervasive `Option` branching throughout the workspace layer. The root problem was that at least 15 methods on `Workspace` branched on `self.storage.as_ref()`, yielding three different failure modes for the same underlying condition: some silently no-oped, others returned errors, and others returned `None`. A `disable_persistence` boolean added a third dimension of optionality, and `ConversationMut` carried an `Option<writer>` that callers could not inspect. The new `jp_storage::backend` module introduces: - `FsStorageBackend` — wraps `Storage`, implements all four traits, and retains filesystem-specific path accessors (`storage_path`, `user_storage_path`) outside the trait surface. - `InMemoryStorageBackend` — fully in-memory implementation for tests and future non-filesystem environments. - `NullPersistBackend` — silently discards all writes; used for `--no-persist` and error-path persistence suppression. - `NoopLockGuard` — a no-op `ConversationLockGuard` for contexts where real locking is not needed. `Workspace` now holds `persist`, `loader`, `locker`, and `sessions` as always-present trait objects. `Workspace::new` initialises them with `NullPersistBackend` + `InMemoryStorageBackend`; callers opt into filesystem storage by calling `with_backend(fs)`. The old builder chain `Workspace::new(root).persisted_at(&storage)?.with_local_storage()?` is replaced by constructing `FsStorageBackend` first, then passing it in. `disable_persistence()` retains its signature but now swaps the `persist` field for `NullPersistBackend` instead of flipping a boolean. `ConversationLock` and `ConversationMut` lose their internal optionality: `writer` and `lock_guard` are always present. The `if let Some(writer)` branches in `flush` and `Drop` are gone — `NullPersistBackend` handles the no-write case through the trait. `jp_workspace::persist` is deleted; the traits are now owned by `jp_storage`. `Workspace::sanitize` delegates to `LoadBackend::sanitize`, which returns an empty report for in-memory backends. `remove_ephemeral_conversations` is reimplemented on `Workspace` using `LoadBackend::load_expired_conversation_ids` and `PersistBackend::remove`, preserving the fast-path metadata reader. In `jp_cli`, `Ctx` gains an `fs_backend: Option<Arc<FsStorageBackend>>` field. Path queries and config loading that previously went through `ctx.workspace.storage_path()` now go through `ctx.storage_path()` or `ctx.fs_backend`. `cleanup_stale_files` and `build_partial_from_cfg_args` receive the typed backend reference explicitly, keeping filesystem concerns out of the `Workspace` API. The RFD 073 design document is added alongside the implementation. Signed-off-by: Jean Mertz <git@jeanmertz.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace
Workspace'sOption<Storage>with four non-optional trait objects —PersistBackend,LoadBackend,LockBackend, andSessionBackend— eliminating pervasiveOptionbranching throughout the workspace layer.The root problem was that at least 15 methods on
Workspacebranched onself.storage.as_ref(), yielding three different failure modes for the same underlying condition: some silently no-oped, others returned errors, and others returnedNone. Adisable_persistenceboolean added a third dimension of optionality, andConversationMutcarried anOption<writer>that callers could not inspect.The new
jp_storage::backendmodule introduces:FsStorageBackend— wrapsStorage, implements all four traits, and retains filesystem-specific path accessors (storage_path,user_storage_path) outside the trait surface.InMemoryStorageBackend— fully in-memory implementation for tests and future non-filesystem environments.NullPersistBackend— silently discards all writes; used for--no-persistand error-path persistence suppression.NoopLockGuard— a no-opConversationLockGuardfor contexts where real locking is not needed.Workspacenow holdspersist,loader,locker, andsessionsas always-present trait objects.Workspace::newinitialises them withNullPersistBackend+InMemoryStorageBackend; callers opt into filesystem storage by callingwith_backend(fs). The old builder chainWorkspace::new(root).persisted_at(&storage)?.with_local_storage()?is replaced by constructingFsStorageBackendfirst, then passing it in.disable_persistence()retains its signature but now swaps thepersistfield forNullPersistBackendinstead of flipping a boolean.ConversationLockandConversationMutlose their internal optionality:writerandlock_guardare always present. Theif let Some(writer)branches influshandDropare gone —NullPersistBackendhandles the no-write case through the trait.jp_workspace::persistis deleted; the traits are now owned byjp_storage.Workspace::sanitizedelegates toLoadBackend::sanitize, which returns an empty report for in-memory backends.remove_ephemeral_conversationsis reimplemented onWorkspaceusingLoadBackend::load_expired_conversation_idsandPersistBackend::remove, preserving the fast-path metadata reader.In
jp_cli,Ctxgains anfs_backend: Option<Arc<FsStorageBackend>>field. Path queries and config loading that previously went throughctx.workspace.storage_path()now go throughctx.storage_path()orctx.fs_backend.cleanup_stale_filesandbuild_partial_from_cfg_argsreceive the typed backend reference explicitly, keeping filesystem concerns out of theWorkspaceAPI.The RFD 073 design document is added alongside the implementation.