chore: address Sonar scan issues#91
Conversation
Signed-off-by: Will Killian <wkillian@nvidia.com>
github-actions
Bot
added
size:L
|
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 (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (16)go/nemo_flow/**/*.go📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
Files:
**/*.go📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{rs,go,js,ts,tsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{rs,py,go,js,ts,tsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{md,markdown,py,sh,bash,js,ts,java,cpp,go,rust}📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Files:
{crates/adaptive/**,python/nemo_flow/{adaptive,plugin}.py,go/nemo_flow/{adaptive,**}/*.go,**/*.{ts,js,wasm}}📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Files:
{crates/adaptive/**/*.rs,python/nemo_flow/adaptive.py,python/nemo_flow/plugin.py,go/nemo_flow/**/*.go,**/{helper,constructor,builder}.{ts,tsx,js}}📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Files:
{crates/adaptive/**/*.rs,python/nemo_flow/plugin.py,go/nemo_flow/**/*.go,**/{plugin,lifecycle}.{ts,tsx,js}}📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Files:
{crates/adaptive/**/*.rs,python/nemo_flow/plugin.py,go/nemo_flow/**/*.go,**/{context,plugin}.{ts,tsx,js,rs}}📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Files:
{docs/**,examples/**,crates/adaptive/**,python/nemo_flow/**,go/nemo_flow/**,**/{example,component}.{ts,tsx,js,rs,py,go}}📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Files:
**/*.{js,ts,tsx,jsx,py,rs,go,java,c,cpp,h,cc,cxx,cs,rb,php,swift,kt}📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Files:
go/**/*.go📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{py,js,ts,tsx,go,rs,md}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{rs,py,js,ts,tsx,go}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
go/nemo_flow/**/*⚙️ CodeRabbit configuration file
Files:
{crates/**/tests/**,python/tests/**,go/nemo_flow/**/*_test.go}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (1)
WalkthroughDecomposes the Rust CLI banner renderer into internal helpers and types, moves setup TOML exporters/agents assembly into builder helpers with a test for OpenInference omission, and consolidates Go test constants and helper assertions for ATOF/ATIF tests. ChangesRust CLI Refactoring
Go Test Infrastructure Consolidation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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
🤖 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/setup.rs`:
- Around line 142-144: The insertion of the OpenInference exporter is
unconditional when answers.openinference_endpoint is present, which can add the
exporter even if the chosen backend is not ObservabilityBackend::OpenInference;
change the logic around insert_openinference_exporter so it only runs when the
selected backend equals ObservabilityBackend::OpenInference (check the answers'
backend selection field used elsewhere), i.e., guard the call to
insert_openinference_exporter(&mut exporters, endpoint) with a match/if that
verifies the backend enum variant before inserting to avoid serializing
unintended exporters.
In `@go/nemo_flow/observability_plugin_test.go`:
- Around line 210-223: initializeAtifPlugin sets global plugin state but doesn't
guarantee cleanup on test exit; add a t.Cleanup registration at the start of
initializeAtifPlugin to call ClearPluginConfiguration() so plugin state is
always cleared even if the test fails. Specifically, inside initializeAtifPlugin
(near t.Helper()), call t.Cleanup(func(){ requireNoError(t,
ClearPluginConfiguration(), clearPluginConfigurationFailed) }) and remove or
keep the current ClearPluginConfiguration() call as appropriate so state is
reset when the test finishes; reference functions: initializeAtifPlugin and
ClearPluginConfiguration and the requireNoError helper.
🪄 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: 4c8c8b5a-d4bb-438d-bc6b-4ea44c4cf061
📒 Files selected for processing (4)
crates/cli/src/banner.rscrates/cli/src/setup.rsgo/nemo_flow/atof_test.gogo/nemo_flow/observability_plugin_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (17)
go/nemo_flow/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
Format changed Go packages with
cd go/nemo_flow && go fmt ./...Use
PascalCasenaming convention for Go API implementationsFor Go binding changes, run
go fmt ./...within the Go module to format all Go files
Files:
go/nemo_flow/atof_test.gogo/nemo_flow/observability_plugin_test.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.go: Usegofmtfor Go code formatting
Usego vet ./...for Go static analysis
Use PascalCase naming convention for Go code
Files:
go/nemo_flow/atof_test.gogo/nemo_flow/observability_plugin_test.go
**/*.{rs,go,js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license headers in all Rust, Go, JavaScript, and TypeScript source files using C-style comment syntax
Files:
go/nemo_flow/atof_test.gocrates/cli/src/setup.rscrates/cli/src/banner.rsgo/nemo_flow/observability_plugin_test.go
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use
SONAR_IGNORE_START/SONAR_IGNORE_ENDmarkers only for documented false positives that cannot be resolved in code; keep ignored blocks small, add explanatory comments, and require reviewer sign-off
Files:
go/nemo_flow/atof_test.gocrates/cli/src/setup.rscrates/cli/src/banner.rsgo/nemo_flow/observability_plugin_test.go
**/*.{md,markdown,py,sh,bash,js,ts,java,cpp,go,rust}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current in documentation
Files:
go/nemo_flow/atof_test.gogo/nemo_flow/observability_plugin_test.go
{crates/adaptive/**,python/nemo_flow/{adaptive,plugin}.py,go/nemo_flow/{adaptive,**}/*.go,**/*.{ts,js,wasm}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep adaptive config schema, plugin lifecycle, and bindings in sync across crates/adaptive, core, bindings, Python (python/nemo_flow/adaptive.py and python/nemo_flow/plugin.py), Go (go/nemo_flow/adaptive and go/nemo_flow), and Node/WebAssembly helpers
Files:
go/nemo_flow/atof_test.gogo/nemo_flow/observability_plugin_test.go
{crates/adaptive/**/*.rs,python/nemo_flow/adaptive.py,python/nemo_flow/plugin.py,go/nemo_flow/**/*.go,**/{helper,constructor,builder}.{ts,tsx,js}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Ensure typed helper constructors map cleanly to the same config document without divergence
Files:
go/nemo_flow/atof_test.gogo/nemo_flow/observability_plugin_test.go
{crates/adaptive/**/*.rs,python/nemo_flow/plugin.py,go/nemo_flow/**/*.go,**/{plugin,lifecycle}.{ts,tsx,js}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain consistent plugin lifecycle across all language bindings (Rust, Python, Go, Node/WebAssembly)
Files:
go/nemo_flow/atof_test.gogo/nemo_flow/observability_plugin_test.go
{crates/adaptive/**/*.rs,python/nemo_flow/plugin.py,go/nemo_flow/**/*.go,**/{context,plugin}.{ts,tsx,js,rs}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep plugin context surfaces aligned across all language implementations
Files:
go/nemo_flow/atof_test.gogo/nemo_flow/observability_plugin_test.go
{docs/**,examples/**,crates/adaptive/**,python/nemo_flow/**,go/nemo_flow/**,**/{example,component}.{ts,tsx,js,rs,py,go}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Any new adaptive component kind must have documentation, examples, and binding coverage across all supported languages
Files:
go/nemo_flow/atof_test.gogo/nemo_flow/observability_plugin_test.go
**/*.{js,ts,tsx,jsx,py,rs,go,java,c,cpp,h,cc,cxx,cs,rb,php,swift,kt}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changed files must be formatted with the language-native formatter
Files:
go/nemo_flow/atof_test.gocrates/cli/src/setup.rscrates/cli/src/banner.rsgo/nemo_flow/observability_plugin_test.go
go/**/*.go
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Go language surface changed, always run Go test target even when Rust core did not change
Files:
go/nemo_flow/atof_test.gogo/nemo_flow/observability_plugin_test.go
**/*.{py,js,ts,tsx,go,rs,md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Format changed files with the language-native formatter before the final lint/test pass
Files:
go/nemo_flow/atof_test.gocrates/cli/src/setup.rscrates/cli/src/banner.rsgo/nemo_flow/observability_plugin_test.go
**/*.{rs,py,js,ts,tsx,go}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
During iteration, prefer
uv run pre-commit run --files <changed files...>for targeted validation
Files:
go/nemo_flow/atof_test.gocrates/cli/src/setup.rscrates/cli/src/banner.rsgo/nemo_flow/observability_plugin_test.go
go/nemo_flow/**/*
⚙️ CodeRabbit configuration file
go/nemo_flow/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_flow/atof_test.gogo/nemo_flow/observability_plugin_test.go
{crates/**/tests/**,python/tests/**,go/nemo_flow/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_flow/**/*_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:
go/nemo_flow/atof_test.gogo/nemo_flow/observability_plugin_test.go
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
**/*.rs: Runcargo fmt --allfor FFI work as it is Rust work
Runjust test-rustfor FFI validation
Runcargo clippy --workspace --all-targets -- -D warningsto enforce warnings-as-errors lintingWhen Rust files changed as part of Python work, run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor Rust code formatting
Runcargo clippy --workspace --all-targets -- -D warningsto enforce Rust linting with no warnings
Runjust test-rustas the shared-runtime build/test wrapper for Rust changesUse Rust snake_case naming convention for Rust code
**/*.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 Rust code formatting when Node changes touch Rust files
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting when Rust files changed as part of Node work
**/*.rs: Always runjust test-rustwhen any Rust code changes
Always runcargo fmt --allwhen any Rust code changes
Always runcargo clippy --workspace --all-targets -- -D warningswhen any Rust code changes
Files:
crates/cli/src/setup.rscrates/cli/src/banner.rs
🔇 Additional comments (5)
crates/cli/src/banner.rs (3)
105-123: Clean orchestration inrender_frame_inner.The refactor into width/grid/overlay/row-render steps is clear and keeps the control flow easy to audit.
130-166: Width and dock-tag placement logic is well-factored.
frame_width+overlay_dock_tagform a coherent contract, and the span metadata keeps placement intent explicit.
168-224: Cell-style routing is a solid maintainability improvement.Centralizing border/cell/style handling removes duplicated branching and makes future styling changes safer.
crates/cli/src/setup.rs (1)
119-125: Good conditional top-level table emissionNice improvement: only writing
[exporters]/[agents]when present keeps generated TOML minimal and avoids empty sections.go/nemo_flow/atof_test.go (1)
15-54: Good refactor of repetitive test assertionsThe shared filename constant and
requireNoError/requireEqualhelpers reduce duplication without changing behavior in the lifecycle and mode tests.Also applies to: 136-149
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@go/nemo_flow/observability_plugin_test.go`:
- Around line 14-21: Constants and helper identifiers in this file use
lowerCamelCase but must be PascalCase per repo Go rules: rename
clearPluginConfigurationFailed → ClearPluginConfigurationFailed,
initializePluginsFailed → InitializePluginsFailed, trajectoryFilenamePrefix →
TrajectoryFilenamePrefix, firstAgentName → FirstAgentName, nestedAgentName →
NestedAgentName, secondAgentName → SecondAgentName, and likewise rename any
helper functions/variables in the 118-146, 134-146 and 210-250 ranges to
PascalCase; update every reference/usages in the package to the new names to
keep builds/tests passing.
🪄 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: 67588b3d-baf5-4248-b3e8-eede3b11d60e
📒 Files selected for processing (3)
crates/cli/src/setup.rscrates/cli/tests/coverage/setup_tests.rsgo/nemo_flow/observability_plugin_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (17)
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
**/*.rs: Runcargo fmt --allfor FFI work as it is Rust work
Runjust test-rustfor FFI validation
Runcargo clippy --workspace --all-targets -- -D warningsto enforce warnings-as-errors lintingWhen Rust files changed as part of Python work, run
cargo fmt --all,just test-rust, andcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Runcargo fmt --allfor Rust code formatting
Runcargo clippy --workspace --all-targets -- -D warningsto enforce Rust linting with no warnings
Runjust test-rustas the shared-runtime build/test wrapper for Rust changesUse Rust snake_case naming convention for Rust code
**/*.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 Rust code formatting when Node changes touch Rust files
Runcargo clippy --workspace --all-targets -- -D warningsto enforce strict linting when Rust files changed as part of Node work
**/*.rs: Always runjust test-rustwhen any Rust code changes
Always runcargo fmt --allwhen any Rust code changes
Always runcargo clippy --workspace --all-targets -- -D warningswhen any Rust code changes
Files:
crates/cli/tests/coverage/setup_tests.rscrates/cli/src/setup.rs
**/*.{rs,go,js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license headers in all Rust, Go, JavaScript, and TypeScript source files using C-style comment syntax
Files:
crates/cli/tests/coverage/setup_tests.rscrates/cli/src/setup.rsgo/nemo_flow/observability_plugin_test.go
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use
SONAR_IGNORE_START/SONAR_IGNORE_ENDmarkers only for documented false positives that cannot be resolved in code; keep ignored blocks small, add explanatory comments, and require reviewer sign-off
Files:
crates/cli/tests/coverage/setup_tests.rscrates/cli/src/setup.rsgo/nemo_flow/observability_plugin_test.go
**/*.{js,ts,tsx,jsx,py,rs,go,java,c,cpp,h,cc,cxx,cs,rb,php,swift,kt}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changed files must be formatted with the language-native formatter
Files:
crates/cli/tests/coverage/setup_tests.rscrates/cli/src/setup.rsgo/nemo_flow/observability_plugin_test.go
**/*.{py,js,ts,tsx,go,rs,md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Format changed files with the language-native formatter before the final lint/test pass
Files:
crates/cli/tests/coverage/setup_tests.rscrates/cli/src/setup.rsgo/nemo_flow/observability_plugin_test.go
**/*.{rs,py,js,ts,tsx,go}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
During iteration, prefer
uv run pre-commit run --files <changed files...>for targeted validation
Files:
crates/cli/tests/coverage/setup_tests.rscrates/cli/src/setup.rsgo/nemo_flow/observability_plugin_test.go
{crates/**/tests/**,python/tests/**,go/nemo_flow/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_flow/**/*_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/setup_tests.rsgo/nemo_flow/observability_plugin_test.go
go/nemo_flow/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
Format changed Go packages with
cd go/nemo_flow && go fmt ./...Use
PascalCasenaming convention for Go API implementationsFor Go binding changes, run
go fmt ./...within the Go module to format all Go files
Files:
go/nemo_flow/observability_plugin_test.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.go: Usegofmtfor Go code formatting
Usego vet ./...for Go static analysis
Use PascalCase naming convention for Go code
Files:
go/nemo_flow/observability_plugin_test.go
**/*.{md,markdown,py,sh,bash,js,ts,java,cpp,go,rust}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current in documentation
Files:
go/nemo_flow/observability_plugin_test.go
{crates/adaptive/**,python/nemo_flow/{adaptive,plugin}.py,go/nemo_flow/{adaptive,**}/*.go,**/*.{ts,js,wasm}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep adaptive config schema, plugin lifecycle, and bindings in sync across crates/adaptive, core, bindings, Python (python/nemo_flow/adaptive.py and python/nemo_flow/plugin.py), Go (go/nemo_flow/adaptive and go/nemo_flow), and Node/WebAssembly helpers
Files:
go/nemo_flow/observability_plugin_test.go
{crates/adaptive/**/*.rs,python/nemo_flow/adaptive.py,python/nemo_flow/plugin.py,go/nemo_flow/**/*.go,**/{helper,constructor,builder}.{ts,tsx,js}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Ensure typed helper constructors map cleanly to the same config document without divergence
Files:
go/nemo_flow/observability_plugin_test.go
{crates/adaptive/**/*.rs,python/nemo_flow/plugin.py,go/nemo_flow/**/*.go,**/{plugin,lifecycle}.{ts,tsx,js}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain consistent plugin lifecycle across all language bindings (Rust, Python, Go, Node/WebAssembly)
Files:
go/nemo_flow/observability_plugin_test.go
{crates/adaptive/**/*.rs,python/nemo_flow/plugin.py,go/nemo_flow/**/*.go,**/{context,plugin}.{ts,tsx,js,rs}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep plugin context surfaces aligned across all language implementations
Files:
go/nemo_flow/observability_plugin_test.go
{docs/**,examples/**,crates/adaptive/**,python/nemo_flow/**,go/nemo_flow/**,**/{example,component}.{ts,tsx,js,rs,py,go}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Any new adaptive component kind must have documentation, examples, and binding coverage across all supported languages
Files:
go/nemo_flow/observability_plugin_test.go
go/**/*.go
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Go language surface changed, always run Go test target even when Rust core did not change
Files:
go/nemo_flow/observability_plugin_test.go
go/nemo_flow/**/*
⚙️ CodeRabbit configuration file
go/nemo_flow/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_flow/observability_plugin_test.go
🔇 Additional comments (4)
crates/cli/src/setup.rs (2)
130-149: Good backend-to-exporter gating.
openinferenceis now emitted only whenObservabilityBackend::OpenInferenceis selected and an endpoint exists, which prevents stale endpoint-only serialization.
181-199: Helper split for agent table is clean and behavior-preserving.The
Nonefast-path for empty agents and the Hermes-onlyhooks_pathinsertion keep output minimal and deterministic.crates/cli/tests/coverage/setup_tests.rs (1)
117-132: Nice regression test for backend/endpoint decoupling.This test protects the exact contract: endpoint presence alone must not produce
[exporters.openinference].go/nemo_flow/observability_plugin_test.go (1)
210-226: Good hardening: helper now guarantees plugin-state cleanup.Adding
t.Cleanup(...)ininitializeAtifPluginprevents cross-test state leakage on early failures and closes the earlier teardown gap.
mnajafian-nv
left a comment
mnajafian-nv
left a comment
There was a problem hiding this comment.
LGTM. I checked the setup exporter gating, banner refactor, and Go test cleanup changes.
Signed-off-by: Will Killian <wkillian@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@go/nemo_flow/observability_plugin_test.go`:
- Around line 51-55: The initial ClearPluginConfiguration call in
TestObservabilityPluginAtofAndAtifFiles (and the other direct-initialization
test around the second block) only runs at test start and can leak global plugin
state on early failures; register an unconditional cleanup by calling t.Cleanup
with a function that calls ClearPluginConfiguration() so the global state is
cleared regardless of test outcome, keeping the existing initial error check
(t.Fatalf on initial ClearPluginConfiguration error) and adding the
t.Cleanup(...) registration immediately after the successful initialization in
both tests.
🪄 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: 412464d4-2806-41ca-9813-b0883cc64e79
📒 Files selected for processing (1)
go/nemo_flow/observability_plugin_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (16)
go/nemo_flow/**/*.go
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
Format changed Go packages with
cd go/nemo_flow && go fmt ./...Use
PascalCasenaming convention for Go API implementationsFor Go binding changes, run
go fmt ./...within the Go module to format all Go files
Files:
go/nemo_flow/observability_plugin_test.go
**/*.go
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.go: Usegofmtfor Go code formatting
Usego vet ./...for Go static analysis
Use PascalCase naming convention for Go code
Files:
go/nemo_flow/observability_plugin_test.go
**/*.{rs,go,js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license headers in all Rust, Go, JavaScript, and TypeScript source files using C-style comment syntax
Files:
go/nemo_flow/observability_plugin_test.go
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use
SONAR_IGNORE_START/SONAR_IGNORE_ENDmarkers only for documented false positives that cannot be resolved in code; keep ignored blocks small, add explanatory comments, and require reviewer sign-off
Files:
go/nemo_flow/observability_plugin_test.go
**/*.{md,markdown,py,sh,bash,js,ts,java,cpp,go,rust}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current in documentation
Files:
go/nemo_flow/observability_plugin_test.go
{crates/adaptive/**,python/nemo_flow/{adaptive,plugin}.py,go/nemo_flow/{adaptive,**}/*.go,**/*.{ts,js,wasm}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep adaptive config schema, plugin lifecycle, and bindings in sync across crates/adaptive, core, bindings, Python (python/nemo_flow/adaptive.py and python/nemo_flow/plugin.py), Go (go/nemo_flow/adaptive and go/nemo_flow), and Node/WebAssembly helpers
Files:
go/nemo_flow/observability_plugin_test.go
{crates/adaptive/**/*.rs,python/nemo_flow/adaptive.py,python/nemo_flow/plugin.py,go/nemo_flow/**/*.go,**/{helper,constructor,builder}.{ts,tsx,js}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Ensure typed helper constructors map cleanly to the same config document without divergence
Files:
go/nemo_flow/observability_plugin_test.go
{crates/adaptive/**/*.rs,python/nemo_flow/plugin.py,go/nemo_flow/**/*.go,**/{plugin,lifecycle}.{ts,tsx,js}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain consistent plugin lifecycle across all language bindings (Rust, Python, Go, Node/WebAssembly)
Files:
go/nemo_flow/observability_plugin_test.go
{crates/adaptive/**/*.rs,python/nemo_flow/plugin.py,go/nemo_flow/**/*.go,**/{context,plugin}.{ts,tsx,js,rs}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep plugin context surfaces aligned across all language implementations
Files:
go/nemo_flow/observability_plugin_test.go
{docs/**,examples/**,crates/adaptive/**,python/nemo_flow/**,go/nemo_flow/**,**/{example,component}.{ts,tsx,js,rs,py,go}}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Any new adaptive component kind must have documentation, examples, and binding coverage across all supported languages
Files:
go/nemo_flow/observability_plugin_test.go
**/*.{js,ts,tsx,jsx,py,rs,go,java,c,cpp,h,cc,cxx,cs,rb,php,swift,kt}
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changed files must be formatted with the language-native formatter
Files:
go/nemo_flow/observability_plugin_test.go
go/**/*.go
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If Go language surface changed, always run Go test target even when Rust core did not change
Files:
go/nemo_flow/observability_plugin_test.go
**/*.{py,js,ts,tsx,go,rs,md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Format changed files with the language-native formatter before the final lint/test pass
Files:
go/nemo_flow/observability_plugin_test.go
**/*.{rs,py,js,ts,tsx,go}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
During iteration, prefer
uv run pre-commit run --files <changed files...>for targeted validation
Files:
go/nemo_flow/observability_plugin_test.go
go/nemo_flow/**/*
⚙️ CodeRabbit configuration file
go/nemo_flow/**/*: Review Go binding changes for cgo memory ownership, race safety, callback cleanup, idiomatic exported APIs, and parity with Rust/FFI behavior.
Any API change should include focused Go tests and consider race-test behavior.
Files:
go/nemo_flow/observability_plugin_test.go
{crates/**/tests/**,python/tests/**,go/nemo_flow/**/*_test.go}
⚙️ CodeRabbit configuration file
{crates/**/tests/**,python/tests/**,go/nemo_flow/**/*_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:
go/nemo_flow/observability_plugin_test.go
🔇 Additional comments (1)
go/nemo_flow/observability_plugin_test.go (1)
14-21: LGTM!Also applies to: 60-71, 97-102, 117-147, 210-250
Signed-off-by: Will Killian <wkillian@nvidia.com>
mnajafian-nv
left a comment
mnajafian-nv
left a comment
There was a problem hiding this comment.
Re-reviewed latest head. CodeRabbit’s cleanup feedback is addressed now: both direct-initialization tests register t.Cleanup(...), and CI is green. LGTM.
2 participants
Overview
Addresses current Sonar scan maintainability findings by reducing flagged cognitive complexity and duplicate literals without changing public behavior.
Details
Validation:
go test -run 'Test(AtofExporter|ObservabilityPlugin|ObservabilityAtif)' ./...cargo test -p nemo-flow-clijust test-gojust test-rustgo fmt ./...cargo clippy --workspace --all-targets -- -D warningsgit diff --checkuv run pre-commit run --files crates/cli/src/banner.rs crates/cli/src/setup.rs go/nemo_flow/atof_test.go go/nemo_flow/observability_plugin_test.gowas run; all relevant Rust/Go hooks passed, with only the pre-existing generated-doc lychee failure remaining.Where should the reviewer start?
Start with
crates/cli/src/banner.rsfor the largest complexity refactor, then review the Go test helper extraction ingo/nemo_flow/observability_plugin_test.go.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
Refactor
Tests
Style