Fixed: Document # Errors sections for 40 public API functions#107
Fixed: Document # Errors sections for 40 public API functions#107
Conversation
Document # Errors sections for 40 functions across 5 crates that were missing or had vague error documentation: - llm-coding-tools-core: BashRequest::parse, fs helpers, path resolvers - llm-coding-tools-agents: AgentLoader methods, Ruleset, runtime builders - llm-coding-tools-bubblewrap: wrap_command, build_command_wrap, probe - llm-coding-tools-serdesai: to_serdes_result, with_tool - llm-coding-tools-models-dev: load, load_at, sync functions
Sewer56
force-pushed
the
docs/error-documentation
branch
from
1268db8 to
e07185c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #107 +/- ##
=======================================
Coverage 81.04% 81.04%
=======================================
Files 110 110
Lines 4515 4517 +2
=======================================
+ Hits 3659 3661 +2
Misses 856 856
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
WalkthroughThis pull request adds comprehensive error documentation to public APIs across the llm-coding-tools codebase. Rustdoc comments and Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/llm-coding-tools-models-dev/src/catalog/mod.rs (1)
80-91: Reorder documentation sections to follow Rust conventions.The
# Errorssection appears after# Examples, which is non-standard for Rust documentation. Per Rust API guidelines, documentation sections should follow this order: description → Parameters → Returns → Errors → Panics → Safety → Examples.This issue also applies to the
load_at()function (lines 116-123).📚 Suggested reordering for `load()` function
Move the
# Errorssection (currently at lines 80-91) to appear before the# Examplessection (currently at lines 56-79). The# Errorssection should be placed immediately after the# Returnssection (after line 45).Similarly for
load_at(), move the# Errorssection (lines 116-123) to before the# Examplessection (lines 125-149), placing it after the# Returnssection (after line 114).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/llm-coding-tools-models-dev/src/catalog/mod.rs` around lines 80 - 91, Move the documentation `# Errors` sections for the Catalog functions so they follow Rust doc conventions: for `load()` and `load_at()` relocate each `# Errors` block so it appears immediately after the `# Returns` section and before the `# Examples` section; update the doc comment order around the `load()` and `load_at()` functions accordingly without changing any content of the error descriptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/llm-coding-tools-agents/src/loader.rs`:
- Around line 84-85: The docs claiming "Returns `AgentLoadError::Io` when the
directory cannot be accessed (permissions, etc.)" are too strong; update the doc
comments in src/llm-coding-tools-agents/src/loader.rs (the bullets around the
`AgentLoadError::Io` description at lines ~84 and ~106-108) to state that an
`Io` error is guaranteed only when the path exists but is not a directory, and
clarify that permission/read errors encountered during recursive walking may be
skipped or handled differently by the walker and therefore are not always
surfaced as `Err(AgentLoadError::Io)`; reference `AgentLoadError::Io` and the
directory-loading function (e.g., the loader function that performs the walk) so
readers know which behavior is guaranteed versus implementation-dependent.
In `@src/llm-coding-tools-agents/src/runtime/task.rs`:
- Around line 35-37: The doc comment for summarize_callable_targets (and the
similar comment at the later location) is too narrow—invalid patterns can come
from the entire caller.permission map, not just permission.task; update the
documentation to state that this function builds a ruleset from the full
caller.permission map and will return ExpandError if any permission entry
contains invalid patterns (reference summarize_callable_targets and the
callable_targets/ruleset-building behavior).
---
Nitpick comments:
In `@src/llm-coding-tools-models-dev/src/catalog/mod.rs`:
- Around line 80-91: Move the documentation `# Errors` sections for the Catalog
functions so they follow Rust doc conventions: for `load()` and `load_at()`
relocate each `# Errors` block so it appears immediately after the `# Returns`
section and before the `# Examples` section; update the doc comment order around
the `load()` and `load_at()` functions accordingly without changing any content
of the error descriptions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d67e1dd7-5242-43b7-9026-2817e1e1c122
📒 Files selected for processing (31)
src/llm-coding-tools-agents/src/extensions.rssrc/llm-coding-tools-agents/src/loader.rssrc/llm-coding-tools-agents/src/path/resolver.rssrc/llm-coding-tools-agents/src/runtime/builder.rssrc/llm-coding-tools-agents/src/runtime/task.rssrc/llm-coding-tools-bubblewrap/src/probe.rssrc/llm-coding-tools-bubblewrap/src/wrap/blocking.rssrc/llm-coding-tools-bubblewrap/src/wrap/command.rssrc/llm-coding-tools-bubblewrap/src/wrap/tokio.rssrc/llm-coding-tools-core/src/fs/blocking_impl.rssrc/llm-coding-tools-core/src/fs/tokio_impl.rssrc/llm-coding-tools-core/src/path/absolute.rssrc/llm-coding-tools-core/src/path/allowed.rssrc/llm-coding-tools-core/src/path/allowed_glob/normalize.rssrc/llm-coding-tools-core/src/tools/bash/mod.rssrc/llm-coding-tools-core/src/tools/edit.rssrc/llm-coding-tools-core/src/tools/glob.rssrc/llm-coding-tools-core/src/tools/grep.rssrc/llm-coding-tools-core/src/tools/read.rssrc/llm-coding-tools-core/src/tools/todo.rssrc/llm-coding-tools-core/src/tools/webfetch/mod.rssrc/llm-coding-tools-core/src/tools/write.rssrc/llm-coding-tools-models-dev/src/api/catalog_sources.rssrc/llm-coding-tools-models-dev/src/api/schema.rssrc/llm-coding-tools-models-dev/src/cache/path.rssrc/llm-coding-tools-models-dev/src/cache/payload.rssrc/llm-coding-tools-models-dev/src/catalog/load_cache.rssrc/llm-coding-tools-models-dev/src/catalog/mod.rssrc/llm-coding-tools-models-dev/src/catalog/sync.rssrc/llm-coding-tools-serdesai/src/agent_ext.rssrc/llm-coding-tools-serdesai/src/convert.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Semver Checks (Core Async)
- GitHub Check: Semver Checks (Serdesai Full)
- GitHub Check: Semver Checks (Bubblewrap Blocking)
- GitHub Check: Semver Checks (Serdesai Full+Linux)
- GitHub Check: Semver Checks (Core Blocking)
- GitHub Check: Blocking Linux
- GitHub Check: Async Linux
- GitHub Check: Blocking macOS
- GitHub Check: Async macOS
- GitHub Check: Async Windows
- GitHub Check: Blocking Windows
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2026-04-11T21:51:36.623Z
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 101
File: src/llm-coding-tools-agents/src/path/resolver.rs:149-151
Timestamp: 2026-04-11T21:51:36.623Z
Learning: In `src/llm-coding-tools-agents/src/path/resolver.rs`, the `/**` pattern optimization (which selects `AbsolutePathResolver` for unrestricted access) is intentionally Unix-only. On Windows, `/**` is not a valid absolute path and therefore the optimization simply does not trigger, falling through to `AllowedGlobResolver`. This cross-platform behavior is by design, allowing a single permission config to be shared across OSes without platform-specific guards.
Applied to files:
src/llm-coding-tools-core/src/path/absolute.rssrc/llm-coding-tools-agents/src/runtime/builder.rssrc/llm-coding-tools-core/src/path/allowed_glob/normalize.rssrc/llm-coding-tools-agents/src/path/resolver.rssrc/llm-coding-tools-core/src/path/allowed.rssrc/llm-coding-tools-bubblewrap/src/probe.rssrc/llm-coding-tools-agents/src/extensions.rs
📚 Learning: 2026-04-11T21:51:49.088Z
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 101
File: src/llm-coding-tools-agents/src/path/resolver.rs:149-151
Timestamp: 2026-04-11T21:51:49.088Z
Learning: In `src/llm-coding-tools-agents/src/path/resolver.rs` (Rust, llm-coding-tools-agents crate), the `/**` optimization in `try_globstar_optimisation` is intentionally Unix-only behavior. On Windows, `/**` is not a valid absolute path pattern, so the literal string match will never trigger there and the resolver safely falls through to `AllowedGlobResolver`. This cross-OS degradation is by design: Unix-authored configs using `/**` do not accidentally grant unrestricted access on Windows. Do not flag this as a missing `#[cfg(unix)]` guard.
Applied to files:
src/llm-coding-tools-core/src/path/absolute.rssrc/llm-coding-tools-agents/src/runtime/builder.rssrc/llm-coding-tools-core/src/path/allowed_glob/normalize.rssrc/llm-coding-tools-core/src/tools/glob.rssrc/llm-coding-tools-agents/src/path/resolver.rssrc/llm-coding-tools-core/src/path/allowed.rssrc/llm-coding-tools-core/src/fs/blocking_impl.rssrc/llm-coding-tools-bubblewrap/src/probe.rssrc/llm-coding-tools-agents/src/extensions.rssrc/llm-coding-tools-agents/src/loader.rs
📚 Learning: 2026-03-11T22:12:21.194Z
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 54
File: src/llm-coding-tools-models-dev/src/catalog/load_cache.rs:23-29
Timestamp: 2026-03-11T22:12:21.194Z
Learning: For src/llm-coding-tools-models-dev/src/catalog/load_cache.rs in this repository, assume the on-disk cache models.dev.catalog.v1.cache is written only by the local user/process. Do not add upper-bound validation on payload_len_decompressed before calling zstd::bulk::decompress, as external/malicious cache files are out of scope for this threat model.
Applied to files:
src/llm-coding-tools-models-dev/src/catalog/load_cache.rs
📚 Learning: 2026-03-28T02:14:04.465Z
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 69
File: src/llm-coding-tools-bubblewrap/src/profile/validation.rs:57-67
Timestamp: 2026-03-28T02:14:04.465Z
Learning: In `src/llm-coding-tools-bubblewrap/src/profile/` (Rust, llm-coding-tools-bubblewrap crate), the `Builder` API paths (workspace, synthetic_home, cache_root, mount lists, overlays, etc.) are always set by trusted application/operator code — the library consumer is the trusted party. Path normalization and `..`-component hardening in validators like `validate_absolute_path` is therefore NOT required to defend against traversal attacks. Untrusted input (LLM-generated shell commands) only enters through `wrap_command`/`execute_command_with_mode`, not through the `Builder`.
Applied to files:
src/llm-coding-tools-bubblewrap/src/wrap/blocking.rssrc/llm-coding-tools-core/src/path/allowed_glob/normalize.rssrc/llm-coding-tools-bubblewrap/src/wrap/command.rssrc/llm-coding-tools-agents/src/path/resolver.rssrc/llm-coding-tools-core/src/path/allowed.rssrc/llm-coding-tools-bubblewrap/src/wrap/tokio.rs
📚 Learning: 2026-03-11T22:12:27.804Z
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 54
File: src/llm-coding-tools-models-dev/src/catalog/load_cache.rs:23-29
Timestamp: 2026-03-11T22:12:27.804Z
Learning: In `src/llm-coding-tools-models-dev/src/cache/` (Rust, llm-coding-tools-models-dev crate), the on-disk cache (models.dev.catalog.v1.cache) is assumed to be written only by the local user/process. Malicious or externally-crafted cache files are explicitly out of scope for this threat model, so there is no need to add upper-bound validation on `payload_len_decompressed` before calling `zstd::bulk::decompress`.
Applied to files:
src/llm-coding-tools-models-dev/src/cache/payload.rssrc/llm-coding-tools-models-dev/src/cache/path.rssrc/llm-coding-tools-models-dev/src/api/catalog_sources.rssrc/llm-coding-tools-models-dev/src/catalog/sync.rssrc/llm-coding-tools-models-dev/src/catalog/mod.rs
📚 Learning: 2026-04-08T21:35:35.581Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-04-08T21:35:35.581Z
Learning: If relevant to your review task, read `.cargo/{general,performance,documentation}.md` for guidance
Applied to files:
src/llm-coding-tools-models-dev/src/cache/payload.rssrc/llm-coding-tools-models-dev/src/catalog/mod.rs
🔇 Additional comments (27)
src/llm-coding-tools-bubblewrap/src/probe.rs (1)
60-66: LGTM! Comprehensive error documentation.The error documentation accurately describes all three
LinuxBwrapError::Executionscenarios handled by the function implementation (lines 71-77, 81-85, 86-90). The rustdoc format is correct and the conditions are clearly stated.src/llm-coding-tools-bubblewrap/src/wrap/tokio.rs (1)
16-17: LGTM! Clear error documentation.The error documentation correctly describes the
LinuxBwrapError::InvalidPathconditions that propagate from the underlyingwrap_commandcall. The compact format listing all conditions in a single bullet is appropriate for this wrapper function.src/llm-coding-tools-bubblewrap/src/wrap/blocking.rs (1)
16-17: LGTM! Consistent error documentation.The error documentation matches the async variant in
tokio.rsand correctly describes theLinuxBwrapError::InvalidPathconditions. The consistency across blocking and async interfaces improves the API's usability.src/llm-coding-tools-bubblewrap/src/wrap/command.rs (1)
75-80: LGTM! Detailed and well-structured error documentation.The error documentation provides clear, separated conditions for each
LinuxBwrapError::InvalidPathscenario. The three-bullet format is appropriate for this lower-level validation function and provides more detail than the compact versions in the wrapper functions (tokio.rsandblocking.rs), improving overall API documentation consistency.src/llm-coding-tools-core/src/path/absolute.rs (1)
37-38: Error contract matches implementation.
resolvedoes returnToolError::InvalidPathfor non-absolute input, so this rustdoc addition is accurate.src/llm-coding-tools-core/src/path/allowed_glob/normalize.rs (1)
44-46:# Errorssection is accurate.This correctly describes the
ToolError::InvalidPathmapping when shell expansion fails.src/llm-coding-tools-core/src/path/allowed.rs (1)
72-74: Good error documentation for constructor validation.The listed
ToolError::InvalidPathcases align with the current checks and canonicalization path.src/llm-coding-tools-agents/src/path/resolver.rs (1)
90-92: Nice clarification of deny-path behavior.The added
ToolError::PermissionDeniedand the documented pattern/path error cases match the actual control flow.src/llm-coding-tools-agents/src/extensions.rs (1)
46-47:# Errorssection is aligned with implementation.
Rule::new(...)propagatesExpandError, so documenting invalid pattern failures here is correct.src/llm-coding-tools-agents/src/runtime/builder.rs (1)
74-75: Build error contract is well documented.The new
# Errorssection is clear and matches theResult<_, ExpandError>build path.src/llm-coding-tools-core/src/tools/read.rs (1)
27-31: Error contract docs match implementation.Nice addition — the
# Errorssection accurately documents theToolError::Jsonpath fromserde_json::from_value.src/llm-coding-tools-core/src/tools/webfetch/mod.rs (1)
24-27: Clear and accurate parse error documentation.The added
# Errorssection is precise and consistent with the parser behavior.src/llm-coding-tools-core/src/tools/grep.rs (1)
36-40: Good# Errorscoverage for request parsing.This matches the actual
ToolError::Jsonconversion path and improves API clarity.src/llm-coding-tools-core/src/tools/glob.rs (1)
24-28: Accurate deserialization error docs.The new section correctly communicates when
ToolError::Jsonis returned.src/llm-coding-tools-core/src/fs/tokio_impl.rs (1)
7-10: Async fs helpers now have solid error contracts.These
# Errorssections are consistent across helpers and correctly describeToolError::Iooutcomes.Also applies to: 16-19, 25-28, 34-37
src/llm-coding-tools-core/src/tools/bash/mod.rs (1)
81-84: Parse error docs are clear and correct.Good addition documenting
ToolError::Jsonfor malformed/invalid request payloads.src/llm-coding-tools-core/src/tools/edit.rs (2)
49-52:EditRequest::parseerror docs are accurate.The mapping to
ToolError::Jsonis documented correctly.
102-114:edit_filenow has strong, explicit error documentation.Great coverage of validation, path/I/O, and ambiguity/not-found cases with concrete variants.
src/llm-coding-tools-core/src/fs/blocking_impl.rs (1)
7-10: Blocking fs error docs are consistent and correct.The added sections clearly document
ToolError::Iobehavior and align with the implementation.Also applies to: 16-19, 25-28, 34-37
src/llm-coding-tools-core/src/tools/todo.rs (3)
78-85: LGTM! Clear and accurate error documentation.The error documentation correctly identifies that
ToolError::Jsonis returned on deserialization failures, and provides helpful context about the kinds of failures that can occur (missing fields, invalid structure).
94-100: LGTM! Consistent error documentation.The error documentation follows the same clear pattern as
TodoWriteRequest::parseand accurately describes the deserialization failure case.
114-137: LGTM! Comprehensive validation error documentation.The error documentation accurately describes both validation error cases and aligns perfectly with the implementation. The documentation is specific about when each
ToolError::Validationis returned (empty/whitespace-onlyidandcontent).src/llm-coding-tools-core/src/tools/write.rs (2)
18-25: LGTM! Clear and accurate error documentation.The error documentation correctly identifies
ToolError::Jsonfor deserialization failures and provides helpful examples of failure cases (missing required fields, invalid types). The documentation format is consistent with the project's rustdoc conventions.
45-78: LGTM! Comprehensive error documentation for file operations.The error documentation accurately maps to all three fallible operations in the implementation:
- Path resolution errors (
resolver.resolve())- Parent directory creation errors (
fs::create_dir_all())- File write errors (
fs::write())The documentation provides helpful context for when each error type occurs, making the API contract clear to users.
src/llm-coding-tools-serdesai/src/convert.rs (1)
50-53: LGTM! Clear and accurate error documentation.The
# Errorssection correctly documents thatto_serdes_resultreturnsSerdesErrorwhen the inputCoreResultcontains an error, and accurately references thecore_error_to_serdesconversion function. The documentation follows Rust conventions and aligns with the implementation on lines 59-61.src/llm-coding-tools-serdesai/src/agent_ext.rs (2)
107-110: LGTM! Accurate error documentation for trait method.The
# Errorssection correctly documents thatwith_toolreturnsAgentBuildError::ToolSettingsValidationwhen converting aToolError, and accurately describes that both the tool name and original error are preserved (as shown in the implementation on line 119).
115-117: LGTM! Consistent error documentation in implementation.The error documentation in the impl block correctly mirrors the trait documentation and accurately describes the behavior of the error conversion on line 119. Good practice to document both the trait and implementation.
1 participant
Description
Adds comprehensive error documentation (
# Errorssections) to public functions across 5 crates that were either missing documentation or had vague triggers.Changes
llm-coding-tools-core (12 functions)
BashRequest::parse- documentsToolError::Jsonfs::read_to_string,fs::write,fs::create_dir_all,fs::open_buffered(blocking & async)AbsolutePathResolver::resolve,AllowedPathResolver::newexpand_shell- documentsToolError::InvalidPathllm-coding-tools-agents (12 functions)
AgentLoader::add_directory,add_directory_with_errors,add_file,add_file_named,add_from_str,add_from_bytesRuleset::from_permission_config,AgentRuntimeBuilder::buildsummarize_callable_targets,callable_targetsbuild_resolver_for_tool- now documentsToolError::PermissionDeniedllm-coding-tools-bubblewrap (4 functions)
wrap_command- documents 3LinuxBwrapError::InvalidPathconditionsbuild_command_wrap(blocking & tokio) - replaces vague docs with specific variantresolve_backend_or_error_for- documentsLinuxBwrapError::Executionllm-coding-tools-serdesai (3 functions)
to_serdes_result- documentsSerdesErrorToolResultExt::with_tool- documentsAgentBuildError::ToolSettingsValidationllm-coding-tools-models-dev (9 functions)
ModelsDevCatalog::load,load_at- documents all 8CatalogErrorvariantsload_catalog_at_path,load_catalog_from_url- documents 8 variants eachshared_cache_path- added missingCatalogError::Configurationparse_api_json,cache_payload_from_api_json_bytes,catalog_from_cache_payload,load_catalog_from_cache_file_dataFormat
All error docs follow the required format: