Draft: enable missing_docs lint (phase 1) by rydrman · Pull Request #1327 · spkenv/spk

rydrman · 2026-02-24T00:43:32Z

Pull Request: docs: enable missing_docs lint (phase 1)

Branch: docs/missing-docs-phase-1

PR URL: https://github.com/spkenv/spk/pull/new/docs/missing-docs-phase-1

Summary

This is the first phase of incrementally enabling the deny(missing_docs) lint across the workspace. The goal is to improve API documentation while keeping merge requests accessible for review.

Phase 0 - Infrastructure (this PR)

Add workspace-level warn(missing_docs) lint to Cargo.toml
Add #![allow(missing_docs)] for generated code (FlatBuffers/protobuf) in:
- spfs-proto/src/lib.rs
- spfs/src/proto/mod.rs

Phase 1 - Document utility and CLI crates (this PR)

Added crate-level and module documentation to:

is_default_derive_macro - derive macro for IsDefault trait
parsedbuf - validated string wrapper macro
sentry-miette - sentry integration for miette errors
spk-schema/tera - template rendering with tera
spfs-cli/common - shared CLI argument structures
All spk-cli command crates (cmd-*, group*)

Upcoming Phases

This is the first of several PRs to fully enable the lint:

Phase	Target Crates	Est. Items
Phase 2	spk-solve sub-crates, spk-config, spk, spfs-cli/main	~160
Phase 3	spk-cli/common (deep), spk-solve, spk-build	~205
Phase 4	spk-solve/graph, spk-exec, spk-storage	~170
Phase 5	spk-schema, spk-schema/foundation, spfs	~1,330
Phase 6	Change workspace lint from `warn` to `deny`	-

Each subsequent phase will be a separate PR targeting ~100-150 documentation items to keep reviews manageable.

Test Plan

cargo check --workspace succeeds with warnings (not errors)
Generated code is properly excluded via #![allow(missing_docs)]
CI passes

Changes Summary

Files modified: 26
Lines added: ~130
Warnings reduced: from ~2,541 to ~2,031 (510 items documented)

Files Changed

Infrastructure

Cargo.toml - Added missing_docs = "warn" to workspace lints
crates/spfs-proto/src/lib.rs - Added crate docs and #![allow(missing_docs)]
crates/spfs/src/proto/mod.rs - Added #![allow(missing_docs)] for generated code

Utility Crates

crates/is_default_derive_macro/src/lib.rs
crates/parsedbuf/src/lib.rs
crates/sentry-miette/src/lib.rs
crates/spk-schema/crates/tera/src/error.rs

SPFS CLI

crates/spfs-cli/common/src/lib.rs
crates/spfs-cli/common/src/args.rs

SPK CLI Commands

crates/spk-cli/cmd-build/src/lib.rs
crates/spk-cli/cmd-convert/src/lib.rs
crates/spk-cli/cmd-debug/src/lib.rs
crates/spk-cli/cmd-du/src/lib.rs
crates/spk-cli/cmd-env/src/lib.rs
crates/spk-cli/cmd-explain/src/lib.rs
crates/spk-cli/cmd-install/src/lib.rs
crates/spk-cli/cmd-make-binary/src/lib.rs
crates/spk-cli/cmd-make-recipe/src/lib.rs
crates/spk-cli/cmd-make-source/src/lib.rs
crates/spk-cli/cmd-render/src/lib.rs
crates/spk-cli/cmd-repo/src/lib.rs
crates/spk-cli/cmd-test/src/lib.rs
crates/spk-cli/group1/src/lib.rs
crates/spk-cli/group2/src/lib.rs
crates/spk-cli/group3/src/lib.rs
crates/spk-cli/group4/src/lib.rs

The monolithic spfs::Error enum had 47 variants covering everything from runtime management to object storage to serialization. This makes it difficult for callers to reason about which errors can actually occur in a given context. This change extracts the 13 runtime-related variants into a new crate::runtime::error::Error type, following the existing pattern used by encoding::Error and graph::ObjectError. The new type is included in the root Error via a #[from] conversion, so existing code that propagates errors with `?` continues to work unchanged. Variants moved: - NothingToCommit, NoActiveRuntime, RuntimeNotInitialized - UnknownRuntime, RuntimeExists, RuntimeUpperDirAlreadyInUse - DoesNotSupportDurableRuntimePath, RuntimeAlreadyEditable - RuntimeReadError, RuntimeWriteError, RuntimeSetPermissionsError - CouldNotCreateSpfsRoot, RuntimeChangeToDurableError This is the first step toward more granular error types across the spfs API surface. Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Co-authored-by: Cursor <cursoragent@cursor.com>

Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>

This is the first phase of incrementally enabling the deny(missing_docs) lint across the workspace. This PR: Phase 0 - Infrastructure: - Add workspace-level warn(missing_docs) lint to Cargo.toml - Add #![allow(missing_docs)] for generated code in spfs-proto and spfs proto module Phase 1 - Document utility and CLI crates: - Add crate-level documentation to: - is_default_derive_macro - parsedbuf - sentry-miette - spk-schema/tera - spfs-cli/common - All spk-cli command crates (cmd-*, group*) This reduces the missing documentation warnings while keeping the lint as a warning to allow incremental progress. Upcoming phases will document the remaining crates: - Phase 2: spk-solve sub-crates, spk-config, spk, spfs-cli/main - Phase 3: spk-cli/common, spk-solve, spk-build - Phase 4: spk-solve/graph, spk-exec, spk-storage - Phase 5: spk-schema, spk-schema/foundation, spfs (large crates) - Phase 6: Change workspace lint from warn to deny Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Co-authored-by: Cursor <cursoragent@cursor.com>

codecov · 2026-02-24T02:03:30Z

Codecov Report

❌ Patch coverage is 11.76471% with 90 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
crates/spfs/src/env.rs	0.00%	40 Missing ⚠️
crates/spfs/src/runtime/storage.rs	24.13%	22 Missing ⚠️
crates/spfs/src/monitor.rs	0.00%	8 Missing ⚠️
crates/spfs/src/runtime/error.rs	0.00%	7 Missing ⚠️
crates/spfs-cli/cmd-render/src/cmd_render.rs	0.00%	3 Missing ⚠️
crates/spfs-cli/common/src/args.rs	0.00%	2 Missing ⚠️
crates/spfs-cli/main/src/cmd_write.rs	0.00%	2 Missing ⚠️
crates/spfs-cli/cmd-clean/src/cmd_clean.rs	0.00%	1 Missing ⚠️
crates/spfs-cli/main/src/cmd_commit.rs	0.00%	1 Missing ⚠️
crates/spfs-cli/main/src/cmd_edit.rs	0.00%	1 Missing ⚠️
... and 3 more

📢 Thoughts on this report? Let us know!

Copilot

Pull request overview

This PR begins rolling out workspace-wide missing_docs linting by adding top-level/module documentation across several utility/CLI crates, while also introducing a new spfs::runtime::Error type and updating call sites to use it (wrapping via spfs::Error::Runtime).

Changes:

Enable missing_docs = "warn" at the workspace level and allow missing_docs for generated protobuf/FlatBuffer code.
Add crate-level/module-level documentation across multiple spk-cli and utility crates.
Refactor SPFS runtime-related errors into spfs::runtime::Error and update matching/mapping throughout spfs, spfs-cli, and spk-cli.

Reviewed changes

Copilot reviewed 47 out of 47 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
Cargo.toml	Enable workspace `missing_docs` lint (warn).
crates/is_default_derive_macro/src/lib.rs	Add crate + macro docs for missing_docs rollout.
crates/parsedbuf/src/lib.rs	Add crate docs describing the `parsed!` macro.
crates/sentry-miette/src/lib.rs	Add crate docs for sentry/miette integration.
crates/spk-schema/crates/tera/src/error.rs	Add docs for template rendering error wrapper.
crates/spk-cli/cmd-build/src/lib.rs	Add crate/module docs for command crate.
crates/spk-cli/cmd-convert/src/lib.rs	Add crate/module docs for command crate.
crates/spk-cli/cmd-debug/src/lib.rs	Add crate/module docs for command crate.
crates/spk-cli/cmd-du/src/lib.rs	Add crate/module docs for command crate.
crates/spk-cli/cmd-env/src/lib.rs	Add crate/module docs for command crate.
crates/spk-cli/cmd-explain/src/lib.rs	Add crate/module docs for command crate.
crates/spk-cli/cmd-install/src/lib.rs	Add crate/module docs for command crate.
crates/spk-cli/cmd-make-binary/src/lib.rs	Add crate/module docs for command crate.
crates/spk-cli/cmd-make-recipe/src/lib.rs	Add crate/module docs for command crate.
crates/spk-cli/cmd-make-source/src/lib.rs	Add crate/module docs for command crate.
crates/spk-cli/cmd-render/src/lib.rs	Add crate/module docs for command crate.
crates/spk-cli/cmd-repo/src/lib.rs	Add crate/module docs for command crate.
crates/spk-cli/cmd-test/src/lib.rs	Add crate/module docs for command crate.
crates/spk-cli/group1/src/lib.rs	Add docs for group1 command modules.
crates/spk-cli/group2/src/lib.rs	Add docs for group2 command modules.
crates/spk-cli/group3/src/lib.rs	Add docs for group3 command modules.
crates/spk-cli/group4/src/lib.rs	Add docs for group4 command modules.
crates/spk-cli/group4/src/cmd_view.rs	Update error pattern match for new runtime error wrapping.
crates/spk-cli/common/src/env.rs	Update runtime-not-active handling for new runtime error wrapping.
crates/spfs-proto/src/lib.rs	Add crate docs + allow missing_docs for generated FlatBuffer code.
crates/spfs/src/proto/mod.rs	Allow missing_docs for generated tonic/proto module.
crates/spfs/src/runtime/mod.rs	Export new runtime error module/type.
crates/spfs/src/runtime/error.rs	Introduce new runtime-specific error enum.
crates/spfs/src/error.rs	Wrap runtime errors under `spfs::Error::Runtime`.
crates/spfs/src/status.rs	Update error docs/returns to runtime error wrapper.
crates/spfs/src/runtime/storage.rs	Switch runtime/storage error construction to `runtime::Error`.
crates/spfs/src/env.rs	Switch env/runtime operations to return `runtime::Error` (wrapped).
crates/spfs/src/env_win.rs	Switch Win env runtime init error to `runtime::Error` (wrapped).
crates/spfs/src/monitor.rs	Switch monitor runtime read/init errors to `runtime::Error` (wrapped).
crates/spfs/src/find_path.rs	Return `runtime::Error::NoActiveRuntime` (wrapped).
crates/spfs/src/commit.rs	Switch “nothing to commit” to runtime error (wrapped).
crates/spfs/src/commit_test.rs	Update tests to match new wrapped runtime error.
crates/spfs/src/storage/tar/repository.rs	Map durable-dir removal IO errors into runtime error (wrapped).
crates/spfs-vfs/src/winfsp/router.rs	Return runtime “already exists” error via runtime error (wrapped).
crates/spfs-cli/common/src/lib.rs	Improve crate docs; hide internal macro re-exports.
crates/spfs-cli/common/src/args.rs	Update error matching for ENOSPC + sentry capture exclusions.
crates/spfs-cli/main/src/cmd_info.rs	Update runtime-not-active matching for new wrapped runtime error.
crates/spfs-cli/main/src/cmd_edit.rs	Update “already editable” matching for new wrapped runtime error.
crates/spfs-cli/main/src/cmd_commit.rs	Switch “nothing to commit” to runtime error type.
crates/spfs-cli/main/src/cmd_write.rs	Switch file IO mapping to runtime write error type.
crates/spfs-cli/cmd-render/src/cmd_render.rs	Switch target dir IO mapping to runtime read/write error type.
crates/spfs-cli/cmd-clean/src/cmd_clean.rs	Switch durable cleanup IO mapping to runtime write error type.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-04T22:49:20Z

crates/spfs/src/runtime/mod.rs

@@ -4,6 +4,8 @@

 //! Handles the setup and initialization of runtime environments



pub mod error; is a newly public module but is missing a doc comment. With missing_docs enabled at the workspace level, this will warn; consider adding a short /// Runtime-specific error types. comment (or making it non-public if it’s not part of the public API).

Suggested change

/// Runtime-specific error types.

Copilot · 2026-03-04T22:49:20Z

crates/spfs/src/status.rs

 /// Unlock the current runtime file system so that it can be modified.
 ///
 /// Once modified, active changes can be committed
 ///
 /// Errors:
-/// - [`Error::NoActiveRuntime`]: if there is no active runtime
-/// - [`Error::RuntimeAlreadyEditable`]: if the active runtime is already editable
+/// - [`RuntimeError::NoActiveRuntime`]: if there is no active runtime
+/// - [`RuntimeError::RuntimeAlreadyEditable`]: if the active runtime is already editable
 /// - if there are issues remounting the filesystem
 pub async fn make_active_runtime_editable() -> Result<()> {
    let mut rt = active_runtime().await?;
    if rt.status.editable {
-        return Err(Error::RuntimeAlreadyEditable);
+        return Err(RuntimeError::RuntimeAlreadyEditable.into());


These docs now list RuntimeError::* variants, but the function returns crate::Result (i.e., spfs::Error), so callers will actually observe spfs::Error::Runtime(RuntimeError::...). Consider updating the docs to mention the wrapping (or link to Error::Runtime) to avoid misleading API documentation.

Copilot · 2026-03-04T22:49:20Z

crates/spfs/src/error.rs

+    #[error(transparent)]
+    #[diagnostic(forward(0))]
+    Runtime(#[from] runtime::Error),



This refactor removes several previously public spfs::Error variants (e.g. NoActiveRuntime, RuntimeAlreadyEditable, RuntimeWriteError, etc.) and replaces them with spfs::Error::Runtime(spfs::runtime::Error::...). That’s a breaking API change for downstream consumers that pattern-match on spfs::Error. If this is intended, it should be called out explicitly (and the crate versioning/release notes updated accordingly); otherwise consider keeping the old variants as deprecated compatibility shims.

Suggested change

/// Deprecated compatibility shim for the old `spfs::Error::NoActiveRuntime` variant.

///

/// This variant is no longer constructed by the library. Downstream code should

/// migrate to matching on `Error::Runtime(runtime::Error::NoActiveRuntime)` instead.

#[deprecated(

since = "0.0.0",

note = "use `spfs::Error::Runtime(spfs::runtime::Error::NoActiveRuntime)` instead"

)]

NoActiveRuntime,

/// Deprecated compatibility shim for the old `spfs::Error::RuntimeAlreadyEditable` variant.

///

/// This variant is no longer constructed by the library. Downstream code should

/// migrate to matching on `Error::Runtime(runtime::Error::RuntimeAlreadyEditable)` instead.

#[deprecated(

since = "0.0.0",

note = "use `spfs::Error::Runtime(spfs::runtime::Error::RuntimeAlreadyEditable)` instead"

)]

RuntimeAlreadyEditable,

/// Deprecated compatibility shim for the old `spfs::Error::RuntimeWriteError` variant.

///

/// This variant is no longer constructed by the library. Downstream code should

/// migrate to matching on `Error::Runtime(runtime::Error::RuntimeWriteError)` instead.

#[deprecated(

since = "0.0.0",

note = "use `spfs::Error::Runtime(spfs::runtime::Error::RuntimeWriteError)` instead"

)]

RuntimeWriteError,

Copilot · 2026-03-04T22:49:21Z

crates/spfs/src/error.rs

    #[error(transparent)]
    #[diagnostic(forward(0))]
    GraphObject(#[from] super::graph::error::ObjectError),
+    #[error(transparent)]
+    #[diagnostic(forward(0))]
+    Runtime(#[from] runtime::Error),



The PR description focuses on enabling missing_docs and adding documentation, but this diff also performs a substantial error-type/API refactor in spfs (moving runtime errors into spfs::runtime::Error and updating many call sites). Please update the PR description to mention this behavioral/API change, or split it into a separate PR to keep the docs-focused change easier to review.

Copilot · 2026-03-04T22:49:21Z

crates/spfs/src/runtime/error.rs

+pub type Result<T> = std::result::Result<T, Error>;
+
+#[derive(Diagnostic, Debug, Error)]
+#[diagnostic(
+    url(
+        "https://spkenv.dev/error_codes#{}",
+        self.code().unwrap_or_else(|| Box::new("spfs::runtime"))
+    )
+)]
+pub enum Error {
+    #[error("Nothing to commit, resulting filesystem would be empty")]
+    NothingToCommit,
+    #[error("No active runtime")]
+    NoActiveRuntime,
+    #[error("Runtime has not been initialized: {0}")]
+    RuntimeNotInitialized(String),
+    #[error("Runtime does not exist: {runtime}")]
+    UnknownRuntime {
+        runtime: String,
+        source: Box<dyn std::error::Error + Send + Sync>,
+    },
+    #[error("Runtime already exists: {0}")]
+    RuntimeExists(String),
+    #[error(


spfs::runtime::error introduces new public items (Result, Error and its variants) without Rustdoc comments, which will add missing_docs warnings right as the workspace lint is being enabled. Please either add /// docs for the public items/variants or add a scoped #![allow(missing_docs)] in this module until it’s documented.

rydrman and others added 3 commits February 18, 2026 17:45

rydrman self-assigned this Feb 24, 2026

rydrman added documentation Improvements or additions to documentation maintenance Cleanup, upgrades, etc labels Feb 24, 2026

jrray requested a review from Copilot March 4, 2026 22:44

Copilot started reviewing on behalf of jrray March 4, 2026 22:44 View session

rydrman changed the base branch from main to refactor/extract-runtime-error-type March 4, 2026 22:45

Copilot AI reviewed Mar 4, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Draft: enable missing_docs lint (phase 1)#1327

Draft: enable missing_docs lint (phase 1)#1327
rydrman wants to merge 3 commits intorefactor/extract-runtime-error-typefrom
docs/missing-docs-phase-1

rydrman commented Feb 24, 2026

Uh oh!

codecov bot commented Feb 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -4,6 +4,8 @@

		//! Handles the setup and initialization of runtime environments

+    /// Deprecated compatibility shim for the old `spfs::Error::NoActiveRuntime` variant.
+    ///
+    /// This variant is no longer constructed by the library. Downstream code should
+    /// migrate to matching on `Error::Runtime(runtime::Error::NoActiveRuntime)` instead.
+    #[deprecated(
+        since = "0.0.0",
+        note = "use `spfs::Error::Runtime(spfs::runtime::Error::NoActiveRuntime)` instead"
+    )]
+    NoActiveRuntime,
+    /// Deprecated compatibility shim for the old `spfs::Error::RuntimeAlreadyEditable` variant.
+    ///
+    /// This variant is no longer constructed by the library. Downstream code should
+    /// migrate to matching on `Error::Runtime(runtime::Error::RuntimeAlreadyEditable)` instead.
+    #[deprecated(
+        since = "0.0.0",
+        note = "use `spfs::Error::Runtime(spfs::runtime::Error::RuntimeAlreadyEditable)` instead"
+    )]
+    RuntimeAlreadyEditable,
+    /// Deprecated compatibility shim for the old `spfs::Error::RuntimeWriteError` variant.
+    ///
+    /// This variant is no longer constructed by the library. Downstream code should
+    /// migrate to matching on `Error::Runtime(runtime::Error::RuntimeWriteError)` instead.
+    #[deprecated(
+        since = "0.0.0",
+        note = "use `spfs::Error::Runtime(spfs::runtime::Error::RuntimeWriteError)` instead"
+    )]
+    RuntimeWriteError,

Conversation

rydrman commented Feb 24, 2026

Pull Request: docs: enable missing_docs lint (phase 1)

Summary

Phase 0 - Infrastructure (this PR)

Phase 1 - Document utility and CLI crates (this PR)

Upcoming Phases

Test Plan

Changes Summary

Files Changed

Infrastructure

Utility Crates

SPFS CLI

SPK CLI Commands

Uh oh!

codecov bot commented Feb 24, 2026

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants