test(cli): raise component coverage above 92 percent - #635
Conversation
Signed-off-by: Will Killian <wkillian@nvidia.com>
github-actions
Bot
added
size:XXL
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (8)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (10)**/*.rs📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Files:
**/*.{rs,py}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,js,mjs,cjs,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,go,js,ts,c,h}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{rs,go,js,ts}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{rs,py,go,js,ts}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Files:
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}⚙️ CodeRabbit configuration file
Files:
{crates/**/src/**/*.rs,python/**/*.py}📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Files:
🔇 Additional comments (1)
WalkthroughThe PR extracts CLI configuration and plugin prompts into dedicated modules, isolates runtime helpers, preserves unrelated configuration sections during writes, updates Codecov exclusions, and expands CLI coverage. ChangesCLI interaction and runtime refactor
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 14
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/cli/src/commands/configure/editor/prompt.rs`:
- Around line 213-219: Extract a shared require_nonempty validator returning
Result<(), &'static str> with the unified "value must not be empty" message,
then replace the inline empty-value validation closures at the three affected
prompt locations with .validate_with(require_nonempty).
In `@crates/cli/src/commands/configure/wizard/prompt.rs`:
- Around line 179-188: Unify the duplicated TTY validation through a shared
helper in the CLI TTY module. In
crates/cli/src/commands/configure/wizard/prompt.rs lines 179-188, replace
ensure_tty’s inline stdin-only check with the shared helper using the
interactive menu stream requirement and preserve its message; in
crates/cli/src/commands/configure/editor/prompt.rs lines 58-60, retain
ensure_tty_with for testability but delegate it to the same helper with its
required streams and message. Ensure the existing plugins prompt guard also uses
this shared definition.
- Around line 179-188: Consolidate the duplicated ensure_tty implementations by
introducing one shared helper that accepts the required stream-readiness checks
and error message. Update the configure editor prompt, plugin prompt, and wizard
prompt callers to use it, preserving each caller’s required stdin/stdout/stderr
checks and user-facing message while removing their local ensure_tty
definitions.
- Around line 87-94: Update the configuration save flow around build_config,
confirm_summary, and save_config so agent_hint being None preserves existing
non-agent sections instead of overwriting them. Merge the current file’s
sections such as upstream, gateway, and logging into the document before preview
and saving, or require an explicit destructive confirmation that accurately
displays their removal.
In `@crates/cli/src/plugins/dynamic_editor/prompt.rs`:
- Around line 352-401: No change is required for this diff; preserve the
existing prompt_raw_config sequence, including redaction before display, secret
restoration before validation, and committing only after successful validation.
Treat re-prompting on errors as a separate follow-up rather than modifying this
implementation.
- Around line 14-35: The condition in edit_dynamic_plugin redundantly checks
state.schema.is_none() because fields is empty in that case; simplify the menu
selection condition to fields.is_empty(), preserving the existing root-menu and
fields-menu behavior.
In `@crates/cli/src/plugins/prompt.rs`:
- Around line 529-531: Update the Reset handling in the MenuResponse shortcut
match to re-assert the expected container type after calling
collection_shortcut_value, ensuring malformed defaults cannot reach the next
iteration’s as_array().expect(...) or as_object().expect(...). Preserve the
existing Clear behavior and use the shortcut-specific collection type when
coercing the reset value.
In `@crates/cli/tests/coverage/commands/configure_editor_tests.rs`:
- Line 339: Update the tests around clear_key and clear_sink_key to assert the
resulting configuration state after each operation, not merely successful return
values. Verify clear_key("missing", "value") removes the intended key and
clear_sink_key(0, "level") removes the sink-level key, adding assertions
alongside the existing path checks while preserving unrelated state assertions.
- Around line 369-378: Update
project_path_defaults_to_start_when_no_ancestor_config_exists to constrain
project_config_path’s ancestor search to the temporary root, preventing
configuration files outside the test fixture from affecting the result while
preserving the expected nested default path.
In `@crates/cli/tests/coverage/shared/plugins_lifecycle_tests.rs`:
- Around line 4417-4517: Strengthen
lifecycle_commands_cover_json_and_human_output_paths by capturing each list,
add, validate, and inspect result instead of only calling unwrap. For JSON
responses, parse the output and assert schema_version, ok, command, and a
representative field under data; for human responses, assert meaningful
command-specific text. Follow assertion patterns used elsewhere in this test
file and include lifecycle-event or scope-stack behavior where available rather
than relying only on successful completion.
In `@crates/cli/tests/coverage/shared/plugins_schema_tests.rs`:
- Around line 280-286: Update native_config_field to replace the bare unwrap
with an expectation that includes the requested key, so lookup failures identify
which schema field is missing while preserving the existing return behavior.
In `@crates/cli/tests/coverage/shared/plugins_tests.rs`:
- Around line 2152-2176: Rename the second `path` binding in
`dynamic_editor_raw_menu_and_nested_value_paths_are_deterministic` to
`field_path`, and update the related `set_value_at_path`, `value_at_path`, and
`remove_value_at_path` calls to use it; leave the filesystem `path` binding
unchanged.
- Around line 599-634: Expand
menu_keys_cover_selection_shortcuts_and_cancellation to assert every key in the
multi-key mappings handled by menu_response_for_key: add Del as Clear, and CtrlC
plus Char('q') as Cancel. Keep the existing assertions and expected response
indices unchanged.
- Around line 746-758: Strengthen component_shortcut_fallbacks_are_safe_noops by
capturing components[0] before calling reset_component_menu_item and
clear_component_menu_item, then asserting it remains unchanged afterward. Keep
the existing successful-result checks and verify both None and Back fallback
actions preserve the component state.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: c1d1d959-14d8-49f0-832e-33c294d579c3
📒 Files selected for processing (20)
codecov.ymlcrates/cli/src/commands/configure/editor.rscrates/cli/src/commands/configure/editor/prompt.rscrates/cli/src/commands/configure/wizard.rscrates/cli/src/commands/configure/wizard/prompt.rscrates/cli/src/commands/mod.rscrates/cli/src/plugins/dynamic_editor.rscrates/cli/src/plugins/dynamic_editor/prompt.rscrates/cli/src/plugins/mod.rscrates/cli/src/plugins/prompt.rscrates/cli/src/process/launcher.rscrates/cli/src/server/mod.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/commands/configure_editor_tests.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/installer_tests.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/tests/coverage/shared/plugins_schema_tests.rscrates/cli/tests/coverage/shared/plugins_tests.rscrates/cli/tests/coverage/shared/server_tests.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Check / Run
🧰 Additional context used
📓 Path-based instructions (12)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
**/*.rs: Any Rust change must runjust test-rust
Any Rust change must runcargo fmt --all
Any Rust change must runcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor all FFI work since it is Rust work
Runjust test-rustto validate FFI changes
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting on FFI workWhen Rust files changed as part of Go work, also run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allwhen Rust files are changed as part of Node work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files are changed as part of Node work
Runjust test-rustwhen Rust files are changed as part of Node workWhen changing the core Rust runtime or Rust-facing API surface, format Rust code with
cargo fmt(rustfmt defaults), keepcargo clippy -- -D warningsclean, and satisfycargo deny checkperdeny.toml.
**/*.rs: If any Rust code changed, always runjust test-rust.
If any Rust code changed, also runcargo fmt --all.
If any Rust code changed, also runcargo clippy --workspace --all-targets -- -D warnings.
For Rust changes headed for review, runcargo fmt --allandcargo clippy --workspace --all-targets -- -D warningseven if relying on pre-commit.
Files:
crates/cli/src/commands/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/installer_tests.rscrates/cli/tests/coverage/commands/configure_editor_tests.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/plugins_schema_tests.rscrates/cli/src/commands/configure/editor.rscrates/cli/src/commands/configure/editor/prompt.rscrates/cli/tests/cli_tests.rscrates/cli/src/plugins/dynamic_editor/prompt.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/commands/configure/wizard.rscrates/cli/tests/coverage/shared/plugins_tests.rscrates/cli/src/commands/configure/wizard/prompt.rscrates/cli/src/plugins/mod.rscrates/cli/src/plugins/dynamic_editor.rscrates/cli/src/plugins/prompt.rs
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
crates/cli/src/commands/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/installer_tests.rscrates/cli/tests/coverage/commands/configure_editor_tests.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/plugins_schema_tests.rscrates/cli/src/commands/configure/editor.rscrates/cli/src/commands/configure/editor/prompt.rscrates/cli/tests/cli_tests.rscrates/cli/src/plugins/dynamic_editor/prompt.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/commands/configure/wizard.rscrates/cli/tests/coverage/shared/plugins_tests.rscrates/cli/src/commands/configure/wizard/prompt.rscrates/cli/src/plugins/mod.rscrates/cli/src/plugins/dynamic_editor.rscrates/cli/src/plugins/prompt.rs
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
crates/cli/src/commands/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/installer_tests.rscrates/cli/tests/coverage/commands/configure_editor_tests.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/plugins_schema_tests.rscrates/cli/src/commands/configure/editor.rscrates/cli/src/commands/configure/editor/prompt.rscrates/cli/tests/cli_tests.rscrates/cli/src/plugins/dynamic_editor/prompt.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/commands/configure/wizard.rscrates/cli/tests/coverage/shared/plugins_tests.rscrates/cli/src/commands/configure/wizard/prompt.rscrates/cli/src/plugins/mod.rscrates/cli/src/plugins/dynamic_editor.rscrates/cli/src/plugins/prompt.rs
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
crates/cli/src/commands/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/installer_tests.rscrates/cli/tests/coverage/commands/configure_editor_tests.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/plugins_schema_tests.rscrates/cli/src/commands/configure/editor.rscrates/cli/src/commands/configure/editor/prompt.rscrates/cli/tests/cli_tests.rscrates/cli/src/plugins/dynamic_editor/prompt.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/commands/configure/wizard.rscrates/cli/tests/coverage/shared/plugins_tests.rscrates/cli/src/commands/configure/wizard/prompt.rscrates/cli/src/plugins/mod.rscrates/cli/src/plugins/dynamic_editor.rscrates/cli/src/plugins/prompt.rs
**/*.{rs,go,js,ts}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Rust, Go, JavaScript, and TypeScript source files using the corresponding
//comment form.
Files:
crates/cli/src/commands/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/installer_tests.rscrates/cli/tests/coverage/commands/configure_editor_tests.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/plugins_schema_tests.rscrates/cli/src/commands/configure/editor.rscrates/cli/src/commands/configure/editor/prompt.rscrates/cli/tests/cli_tests.rscrates/cli/src/plugins/dynamic_editor/prompt.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/commands/configure/wizard.rscrates/cli/tests/coverage/shared/plugins_tests.rscrates/cli/src/commands/configure/wizard/prompt.rscrates/cli/src/plugins/mod.rscrates/cli/src/plugins/dynamic_editor.rscrates/cli/src/plugins/prompt.rs
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
crates/cli/src/commands/mod.rscrates/cli/src/server/mod.rscrates/cli/src/process/launcher.rscrates/cli/src/commands/configure/editor.rscrates/cli/src/commands/configure/editor/prompt.rscrates/cli/src/plugins/dynamic_editor/prompt.rscrates/cli/src/commands/configure/wizard.rscrates/cli/src/commands/configure/wizard/prompt.rscrates/cli/src/plugins/mod.rscrates/cli/src/plugins/dynamic_editor.rscrates/cli/src/plugins/prompt.rs
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
crates/cli/src/commands/mod.rscodecov.ymlcrates/cli/tests/coverage/shared/config_tests.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/installer_tests.rscrates/cli/tests/coverage/commands/configure_editor_tests.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/plugins_schema_tests.rscrates/cli/src/commands/configure/editor.rscrates/cli/src/commands/configure/editor/prompt.rscrates/cli/tests/cli_tests.rscrates/cli/src/plugins/dynamic_editor/prompt.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/commands/configure/wizard.rscrates/cli/tests/coverage/shared/plugins_tests.rscrates/cli/src/commands/configure/wizard/prompt.rscrates/cli/src/plugins/mod.rscrates/cli/src/plugins/dynamic_editor.rscrates/cli/src/plugins/prompt.rs
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
**/*.{rs,py,go,js,ts}: When observability configuration or lifecycle is exposed, keep FFI and Python, Go, and Node.js binding-native config objects and subscriber/exporter methods aligned in logical knobs and semantics.
Require every OpenTelemetry endpoint to have a type and nonblank destination; resolveheader_envvalues at activation and reject missing, blank, or duplicate headers.
Concatenate layered ATOF sink, ATIF storage, and OpenTelemetry endpoint lists with higher-precedence entries first.
Preserve correct handling of mark events, start/end events, orphan cases, and span or trajectory fields derived from intended event data.
Run affected Rust tests andjust test-rustwhen event fields change; runjust test-python,just test-go, andjust test-nodewhen binding-native configuration or lifecycle changes.
Files:
crates/cli/src/commands/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/installer_tests.rscrates/cli/tests/coverage/commands/configure_editor_tests.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/plugins_schema_tests.rscrates/cli/src/commands/configure/editor.rscrates/cli/src/commands/configure/editor/prompt.rscrates/cli/tests/cli_tests.rscrates/cli/src/plugins/dynamic_editor/prompt.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/commands/configure/wizard.rscrates/cli/tests/coverage/shared/plugins_tests.rscrates/cli/src/commands/configure/wizard/prompt.rscrates/cli/src/plugins/mod.rscrates/cli/src/plugins/dynamic_editor.rscrates/cli/src/plugins/prompt.rs
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
crates/cli/src/commands/mod.rscrates/cli/tests/coverage/shared/config_tests.rscrates/cli/src/server/mod.rscrates/cli/tests/coverage/shared/installer_tests.rscrates/cli/tests/coverage/commands/configure_editor_tests.rscrates/cli/src/process/launcher.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/plugins_schema_tests.rscrates/cli/src/commands/configure/editor.rscrates/cli/src/commands/configure/editor/prompt.rscrates/cli/tests/cli_tests.rscrates/cli/src/plugins/dynamic_editor/prompt.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/src/commands/configure/wizard.rscrates/cli/tests/coverage/shared/plugins_tests.rscrates/cli/src/commands/configure/wizard/prompt.rscrates/cli/src/plugins/mod.rscrates/cli/src/plugins/dynamic_editor.rscrates/cli/src/plugins/prompt.rs
codecov.yml
📄 CodeRabbit inference engine (.agents/skills/maintain-ci/SKILL.md)
Keep Codecov component paths aligned with new crates, packages, and generated outputs; dynamic plugin SDK/protocol paths belong in the plugin component.
Files:
codecov.yml
{justfile,codecov.yml,codecov.yaml,.github/workflows/**/*.yml,.github/workflows/**/*.yaml}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
justfile, Codecov, and CI package/test workflows must include new plugin crates and packages.
Files:
codecov.yml
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.
Files:
crates/cli/tests/coverage/shared/config_tests.rscrates/cli/tests/coverage/shared/installer_tests.rscrates/cli/tests/coverage/commands/configure_editor_tests.rscrates/cli/tests/coverage/shared/server_tests.rscrates/cli/tests/coverage/shared/plugins_schema_tests.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/shared/plugins_lifecycle_tests.rscrates/cli/tests/coverage/shared/plugins_tests.rs
🔇 Additional comments (43)
crates/cli/src/commands/mod.rs (2)
56-97: LGTM!Also applies to: 99-107
56-107: 📐 Maintainability & Code QualityRun the mandated Rust validation.
The supplied context does not confirm execution of the required Rust commands. Run
cargo fmt --all,cargo clippy --workspace --all-targets -- -D warnings, andjust test-rustbefore merge.
crates/cli/src/commands/mod.rs#L56-L107: validate the logging setup extraction.crates/cli/src/process/launcher.rs#L709-L779: validate the observability destination helper extraction.crates/cli/src/server/mod.rs#L415-L454: validate the server teardown logging helper extraction.As per coding guidelines, “Any Rust change must run
just test-rust” and must run the specified formatter and Clippy commands.Source: Coding guidelines
crates/cli/src/process/launcher.rs (1)
709-713: LGTM!Also applies to: 715-737, 739-759, 761-779
crates/cli/src/server/mod.rs (1)
415-418: LGTM!Also applies to: 431-454
crates/cli/tests/cli_tests.rs (2)
726-726: LGTM!Also applies to: 755-755, 775-813
1984-1984: LGTM!Also applies to: 2102-2102, 3528-3528, 3907-3907, 4003-4003
crates/cli/tests/coverage/shared/config_tests.rs (1)
4151-4188: LGTM!Also applies to: 4189-4238, 4240-4255
crates/cli/tests/coverage/shared/installer_tests.rs (1)
583-589: LGTM!Also applies to: 589-605, 607-640, 642-670
crates/cli/tests/coverage/shared/plugins_lifecycle_tests.rs (1)
1921-1921: LGTM!Also applies to: 1945-1946, 1989-2045, 4259-4346, 4348-4401, 4403-4415, 4519-4534
crates/cli/tests/coverage/shared/server_tests.rs (1)
676-705: LGTM!Also applies to: 2149-2190, 2192-2251, 2253-2268
crates/cli/src/commands/configure/editor.rs (1)
17-17: LGTM!Also applies to: 38-43
crates/cli/src/commands/configure/editor/prompt.rs (2)
19-56: LGTM!
280-309: LGTM!Also applies to: 311-342, 415-440
crates/cli/src/commands/configure/wizard.rs (2)
39-44: LGTM!
6-31: 🩺 Stability & AvailabilityNo issue found.
plugin_prompt_was_interruptedremains defined and resolves fromprompt.rs. The test module is#[cfg(test)], and the gated imports are used by the test module.> Likely an incorrect or invalid review comment.crates/cli/src/commands/configure/wizard/prompt.rs (4)
25-74: LGTM!
112-147: LGTM!Also applies to: 149-161
201-257: LGTM!Also applies to: 262-299
76-79: 🚀 Performance & ScalabilityKeep
runasync. Its caller awaits it, and the CLI uses a multi-threaded Tokio runtime. The synchronous wizard can block one worker thread during terminal input, but this matches the pre-refactor behavior and requires no change.crates/cli/tests/coverage/commands/configure_editor_tests.rs (2)
303-303: LGTM!Also applies to: 306-338
342-357: LGTM!crates/cli/src/plugins/dynamic_editor.rs (2)
25-25: LGTM!Also applies to: 153-156
363-363: 📐 Maintainability & Code QualityRetain
pub(super)for the tested helpers.
plugins_testsis an in-crate sibling ofdynamic_editor, so these calls require visibility throughcrate::plugins.pub(in crate::plugins::dynamic_editor)would exclude the test module. The widening is unnecessary forprompt.rsbut required by the tests.> Likely an incorrect or invalid review comment.crates/cli/src/plugins/dynamic_editor/prompt.rs (3)
37-105: LGTM!Also applies to: 107-162
164-277: LGTM!Also applies to: 317-350
289-289: 📐 Maintainability & Code QualityNo change is required. The workspace does not enable
clippy::uninlined_format_args, and this lint is not enabled by default.> Likely an incorrect or invalid review comment.crates/cli/src/plugins/mod.rs (3)
4-10: LGTM!Also applies to: 23-23, 106-108
235-246: LGTM!
32-35: 📐 Maintainability & Code QualityKeep these imports private; do not use
pub(crate) use. Descendant modules can use private parent imports.pub(crate) useis rejected because these functions arepub(super). Direct imports fromcrate::plugins::promptare optional.> Likely an incorrect or invalid review comment.crates/cli/src/plugins/prompt.rs (6)
14-55: LGTM!Also applies to: 57-122, 124-189
191-234: LGTM!Also applies to: 289-299
675-728: LGTM!Also applies to: 730-830
1254-1266: LGTM!
1194-1202: 🎯 Functional CorrectnessNo change needed:
parse_float_valueis still defined.crates/cli/src/plugins/mod.rs:742defines it, anduse super::*resolves the call.> Likely an incorrect or invalid review comment.
1220-1233: 🩺 Stability & AvailabilityDo not require an empty-enum guard for safety.
dialoguer::Select::interact()returns an error for an empty list, andeditor_errorconverts it toCliError::Config;values[idx]is not reached. An explicit field-specific error remains an optional clarity improvement.> Likely an incorrect or invalid review comment.crates/cli/tests/coverage/shared/plugins_schema_tests.rs (1)
275-278: LGTM!Also applies to: 319-334
crates/cli/tests/coverage/shared/plugins_tests.rs (6)
323-326: LGTM!Also applies to: 328-340, 342-408, 410-431
636-657: LGTM!Also applies to: 659-714, 716-744
790-807: LGTM!Also applies to: 1881-1907
2071-2103: LGTM!Also applies to: 2105-2150
2178-2193: LGTM!Also applies to: 2195-2261, 2324-2374, 2376-2399, 2401-2412
760-788: 📐 Maintainability & Code QualityNo change needed.
editable_componentsaddsSwitchyardunder the same feature gate as the enum variant, so the test covers every variant present in the active build. The shown CI job only publishesnemo-relay-switchyard; this does not make the test name inaccurate.> Likely an incorrect or invalid review comment.codecov.yml (1)
138-143: 🗄️ Data Integrity & IntegrationNo Codecov path update is required. The component paths are broad, all four prompt files exist, and no stale references remain.
> Likely an incorrect or invalid review comment.
Signed-off-by: Will Killian <wkillian@nvidia.com>
|
/merge |
1 similar comment
|
/merge |
3 participants
Overview
Raises the CLI component coverage target above 92 percent while excluding prompt-only logic that cannot be exercised reliably in automated coverage runs.
Details
Validation performed:
Where should the reviewer start?
Start with codecov.yml and crates/cli/src/commands/configure/editor.rs, then compare its prompt module and crates/cli/tests/coverage/commands/configure_editor_tests.rs.
Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
New Features
Bug Fixes