feat: layer code-driven plugin config#199
Conversation
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (35)
WalkthroughThis PR introduces plugin configuration layering—a cross-language feature enabling hierarchical composition of plugin configurations through code-driven overlays on file-backed defaults. The core layering logic merges JSON objects and components by ChangesPlugin Configuration Layering
🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
github-actions
Bot
added
size:L
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
github-actions
Bot
added
size:XL
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/cli/src/config.rs (1)
199-201:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve env-vs-flag provenance for plugin config layers.
ServerArgs.plugin_configis populated by clap from either--plugin-configorNEMO_RELAY_PLUGIN_CONFIG, butapply_cli_plugin_confighardcodes the provenance string as--plugin-config, so env-backed plugin config gets mis-tagged inplugin_config_source. That value is used bydoctor, launcher output, and plugin activation error messages—so provenance tracking is incorrect. Thread the actual value source through toapply_code_plugin_config_layer(e.g., label env asNEMO_RELAY_PLUGIN_CONFIG), rather than always using--plugin-config.Also applies to: 528-530, 806-811
🤖 Prompt for 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. In `@crates/cli/src/config.rs` around lines 199 - 201, ServerArgs.plugin_config is being labeled always as "--plugin-config" even when sourced from the environment; update apply_cli_plugin_config to determine the actual provenance (flag vs env) and pass that provenance string into apply_code_plugin_config_layer so plugin_config_source reflects "NEMO_RELAY_PLUGIN_CONFIG" for env-sourced values and "--plugin-config" for flag-sourced values; specifically, locate uses of ServerArgs.plugin_config in apply_cli_plugin_config and change the hardcoded provenance argument to compute and thread the real source through to apply_code_plugin_config_layer (repeat the same fix for the other spots noted around the apply_cli_plugin_config usages at the other ranges so doctor, launcher output, and plugin activation errors get the correct provenance).crates/cli/tests/coverage/session_tests.rs (1)
99-107: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winConsolidate duplicated
GatewayConfigliterals throughsession_test_config().These inline configs now need the same field churn as the shared helper at Lines 4085-4095. Switching the tests that only need the default test config to
session_test_config()would reduce future breakage whenGatewayConfigchanges again.Also applies to: 1235-1243, 1305-1313, 1384-1392, 1815-1823, 1891-1899, 1963-1971, 2019-2027, 2065-2073, 2122-2130, 2219-2227, 2334-2342, 2427-2435, 4100-4108
🤖 Prompt for 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. In `@crates/cli/tests/coverage/session_tests.rs` around lines 99 - 107, Replace the duplicated inline GatewayConfig constructions in session_tests.rs with the shared helper session_test_config() so tests that only require the default test setup use that central source of truth; find the inline GatewayConfig instances (constructed with fields like bind, openai_base_url, anthropic_base_url, metadata, plugin_config, plugin_config_source) and swap them to let config = session_test_config(), or if a test needs minor overrides, call session_test_config() and then modify only those fields; ensure session_test_config() is imported/visible in session_tests.rs and remove the duplicated literal to avoid future churn when GatewayConfig changes.
🤖 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/core/src/plugin.rs`:
- Around line 187-197: The layering loop in layer_plugin_config wrongly
collapses multiple overlay components with the same kind into the first base
match (see overlay_components, base_components, json_component_kind,
merge_json_value), breaking configs where validate_plugin_config and
allows_multiple_components permit duplicates; update layer_plugin_config to
either (A) detect duplicate kinds in the overlay for kinds that
allow_multiple_components() and reject early with a clear error, or (B)
implement stable per-instance matching instead of first-match merging — e.g.,
require or derive a per-instance identifier (or positional/occurrence index) and
use that to find the correct base entry before calling merge_json_value, falling
back to appending a new component when no matching instance-id is found. Ensure
behavior is consistent with validate_plugin_config and document which strategy
is used.
In `@crates/ffi/nemo_relay.h`:
- Around line 1116-1125: The header for nemo_relay_layer_plugin_config is
missing ownership semantics for out_json; update the function doc comment to
state that on success the function allocates a NUL-terminated JSON string and
stores it in *out_json and that the caller takes ownership and must free
*out_json using nemo_relay_string_free (or equivalent API) to avoid leaks;
reference nemo_relay_layer_plugin_config and nemo_relay_string_free in the
updated comment so C consumers know who is responsible for deallocation.
In `@crates/ffi/tests/unit/api/plugin_tests.rs`:
- Around line 8-23: Add two negative tests for nemo_relay_layer_plugin_config:
one that passes an invalid JSON C string (e.g., cstring("not-json")) as either
base or overlay and asserts the call returns the FFI error status for bad JSON
(compare to the appropriate NemoRelayStatus variant), and one that passes a null
mutable out_json pointer (use &mut ptr::null_mut() or equivalent) and asserts
the call returns the FFI error status for a null/invalid output pointer; place
these alongside test_ffi_layer_plugin_config_round_trips_merge and use the same
helpers (cstring, returned_json) but do not attempt to dereference or consume
out_json on error paths.
In `@docs/observability-plugin/atof.mdx`:
- Around line 44-45: The docs for ATOF's `output_directory` and `filename` are
inconsistent with runtime behavior: update the description for
`output_directory` to state that the runtime (and the `doctor` command) will
create the directory if missing and that `doctor` only emits a warning rather
than requiring pre-creation; keep `filename` notes about the exporter creating
the file but not parent directories but clarify that parent directories are
created by runtime when using `output_directory`. Edit the `output_directory`
and `filename` table entries in the ATOF docs to reflect this corrected contract
and mention `doctor` by name so readers know where the warning originates.
In `@go/nemo_relay/plugin_gap_test.go`:
- Around line 38-53: Add a failing-path test for LayerPluginConfig that ensures
marshaling errors are returned before crossing the cgo boundary: update
TestLayerPluginConfigRoundTripsMerge (or add a new test) to call
LayerPluginConfig with an unsupported value (e.g., a channel, function, or
cyclic structure) and assert that it returns a non-nil error and does not return
merged JSON; this will exercise the jsonCString/serialization failure path and
ensure the binding surfaces the Go marshaling failure properly.
In `@python/nemo_relay/plugin.py`:
- Around line 291-309: The current layer() call normalizes both base and overlay
via _normalize_object(), but _normalize() removes dict entries with value None
so an overlay intending to set a key to JSON null (e.g. {"config": {"timeout":
None}}) is lost; update the normalization logic so explicit None values from
overlays are preserved: either modify _normalize()/_normalize_object() to not
drop keys whose value is None (or add a flag like preserve_nulls and call
_normalize_object(overlay, preserve_nulls=True) from layer), then call
_layer_plugin_config with the normalized base and the overlay normalized in a
way that retains explicit None overrides (reference functions: layer,
_normalize_object, _normalize).
In `@python/tests/test_plugin_config.py`:
- Around line 7-10: Update the test_layer_plugin_config_round_trips_merge test
to also exercise the typed wrapper path: call plugin.layer with a PluginConfig
instance (e.g., PluginConfig({"a": 1}) and PluginConfig({"b": 2}) or one
PluginConfig and one dict) in addition to the raw dict case and assert the
returned merged JSON equals {"a": 1, "b": 2}; use the PluginConfig symbol from
the module so Python-side conversion is exercised and consider parametrizing the
test if you prefer both forms tested succinctly.
---
Outside diff comments:
In `@crates/cli/src/config.rs`:
- Around line 199-201: ServerArgs.plugin_config is being labeled always as
"--plugin-config" even when sourced from the environment; update
apply_cli_plugin_config to determine the actual provenance (flag vs env) and
pass that provenance string into apply_code_plugin_config_layer so
plugin_config_source reflects "NEMO_RELAY_PLUGIN_CONFIG" for env-sourced values
and "--plugin-config" for flag-sourced values; specifically, locate uses of
ServerArgs.plugin_config in apply_cli_plugin_config and change the hardcoded
provenance argument to compute and thread the real source through to
apply_code_plugin_config_layer (repeat the same fix for the other spots noted
around the apply_cli_plugin_config usages at the other ranges so doctor,
launcher output, and plugin activation errors get the correct provenance).
In `@crates/cli/tests/coverage/session_tests.rs`:
- Around line 99-107: Replace the duplicated inline GatewayConfig constructions
in session_tests.rs with the shared helper session_test_config() so tests that
only require the default test setup use that central source of truth; find the
inline GatewayConfig instances (constructed with fields like bind,
openai_base_url, anthropic_base_url, metadata, plugin_config,
plugin_config_source) and swap them to let config = session_test_config(), or if
a test needs minor overrides, call session_test_config() and then modify only
those fields; ensure session_test_config() is imported/visible in
session_tests.rs and remove the duplicated literal to avoid future churn when
GatewayConfig changes.
🪄 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: a835da00-09af-46cc-826f-6d45f8cb9660
📒 Files selected for processing (38)
crates/cli/src/config.rscrates/cli/src/doctor.rscrates/cli/src/launcher.rscrates/cli/src/server.rscrates/cli/tests/cli_tests.rscrates/cli/tests/coverage/config_tests.rscrates/cli/tests/coverage/doctor_tests.rscrates/cli/tests/coverage/gateway_tests.rscrates/cli/tests/coverage/server_tests.rscrates/cli/tests/coverage/session_tests.rscrates/core/src/plugin.rscrates/core/tests/unit/plugin_tests.rscrates/ffi/nemo_relay.hcrates/ffi/src/api/mod.rscrates/ffi/src/api/plugin.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/node/plugin.d.tscrates/node/plugin.jscrates/node/src/api/mod.rscrates/node/tests/plugin_tests.mjscrates/python/src/py_plugin.rscrates/wasm/src/api/mod.rscrates/wasm/tests-js/plugin_tests.mjscrates/wasm/wrappers/esm/index.jscrates/wasm/wrappers/esm/plugin.d.tscrates/wasm/wrappers/esm/plugin.jscrates/wasm/wrappers/nodejs/plugin.jsdocs/build-plugins/plugin-configuration-files.mdxdocs/build-plugins/register-behavior.mdxdocs/observability-plugin/atof.mdxgo/nemo_relay/plugin.gogo/nemo_relay/plugin_gap_test.gopython/nemo_relay/_native.pyipython/nemo_relay/plugin.pypython/nemo_relay/plugin.pyipython/tests/test_plugin_config.pyskills/nemo-relay-build-plugin/SKILL.mdskills/nemo-relay-tune-adaptive-config/SKILL.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (69)
**/*.{md,rst,html,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
**/*.{md,rst,html,txt}: Always spellNVIDIAin all caps. Do not useNvidia,nvidia,nVidia,nVIDIA, orNV.
Usean NVIDIAbefore a noun because the name starts with an 'en' sound.
Do not add a registered trademark symbol afterNVIDIAwhen referring to the company.
Use trademark symbols with product names only when the document type or legal guidance requires them.
Verify official capitalization, spacing, and hyphenation for product names.
Precede NVIDIA product names withNVIDIAon first mention when it is natural and accurate.
Do not rewrite product names for grammar or title-case rules.
Preserve third-party product names according to the owner's spelling.
Include the company name and full model qualifier on first use when it helps identify the model.
Preserve the official capitalization and punctuation of model names.
Use shorter family names only after the full name is established.
Spell out a term on first use and put the acronym in parentheses unless the acronym is widely understood by the intended audience.
Use the acronym on later mentions after it has been defined.
For long documents, reintroduce the full term if readers might lose context.
Form plurals of acronyms withs, not an apostrophe, such asGPUs.
In headings, common acronyms can remain abbreviated. Spell out the term in the first or second sentence of the body.
Common terms such asCPU,GPU,PC,API, andUIusually do not need to be spelled out for developer audiences.
Files:
skills/nemo-relay-build-plugin/SKILL.mdskills/nemo-relay-tune-adaptive-config/SKILL.md
**/*.{md,rst,html}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md)
Link the first mention of a product name when the destination helps the reader.
Files:
skills/nemo-relay-build-plugin/SKILL.mdskills/nemo-relay-tune-adaptive-config/SKILL.md
**/*.md
📄 CodeRabbit inference engine (.agents/skills/contribute-integration/SKILL.md)
Documentation must be updated if activation or usage changed
**/*.md: Use title case consistently in technical documentation headings
Avoid quotation marks, ampersands, and exclamation marks in headings
Keep product, event, research, and whitepaper names in their official title case
Use title case for table headers
Do not force social-media sentence case into technical docs
Format code elements, commands, parameters, package names, and expressions in monospace
Format directories, file names, and paths in monospace using backticks
Use angle brackets inside monospace for variables inside paths, such as/home/<username>/.login
Format error messages and strings in quotation marks, keeping literal code strings in code formatting when clearer
Format UI buttons, menus, fields, and labels in bold
Use angle brackets between UI labels for menu paths, such as File > Save As
Use italics for new terms on first use, sparingly and only when introducing the term
Use italics for publication titles
Format keyboard shortcuts in plain text, such as Press Ctrl+Alt+Delete
Use owner/repo link text for GitHub repositories, preferring[NVIDIA/NeMo](link)over prose references like 'the GitHub repo'
Introduce every code block with a complete sentence
Do not make a code block complete the grammar of the previous sentence
Do not continue a sentence after a code block
Use syntax highlighting when the format supports it for code blocks
Avoid the word 'snippet' unless the surrounding docs already use it as a term of art
Keep inline method, function, and class references consistent with nearby docs, omitting empty parentheses for prose readability when no call is shown
Use descriptive anchor text that matches the destination title when possible for links
Avoid raw URLs in running text
Avoid generic anchor text such as 'here,' 'this page,' and 'read more'
Include acronyms in link text when a linked term includes an acronym
Do not link long sentences or multiple sentences
Avoid links ...
Files:
skills/nemo-relay-build-plugin/SKILL.mdskills/nemo-relay-tune-adaptive-config/SKILL.md
**/{docs,examples,**/*.md,*.patch,*.diff,.github,*.sh,*.yaml,*.yml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update documentation, examples, CI configuration, and patch artifacts when performing rename operations
Files:
skills/nemo-relay-build-plugin/SKILL.mdskills/nemo-relay-tune-adaptive-config/SKILL.md
**/*.{md,rst,txt}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
Spell
NVIDIAin all caps. Do not useNvidia,nvidia, orNV.
Files:
skills/nemo-relay-build-plugin/SKILL.mdskills/nemo-relay-tune-adaptive-config/SKILL.md
**/*.{md,rst}
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md)
**/*.{md,rst}: Format commands, code elements, expressions, package names, file names, and paths as inline code.
Use descriptive link text. Avoid raw URLs and weak anchors such as "here" or "read more."
Use title case consistently for technical documentation headings.
Introduce code blocks, lists, tables, and images with complete sentences.
Write procedures as imperative steps. Keep steps parallel and split long procedures into smaller tasks.
Prefer active voice, present tense, short sentences, contractions, and plain English.
Usecanfor possibility and reservemayfor permission.
Useafterfor temporal relationships instead ofonce.
Preferrefer tooverseewhen the wording points readers to another resource.
Avoid culture-specific idioms, unnecessary Latinisms, jokes, and marketing exaggeration in technical docs.
Spell out months in body text, avoid ordinal dates, and use clear time zones.
Spell out whole numbers from zero through nine unless they are technical values, parameters, versions, or UI values.
Use numerals for 10 or greater and include commas in thousands.
Do not add trademark symbols to learning-oriented docs unless the source, platform, or legal guidance explicitly requires them.
Files:
skills/nemo-relay-build-plugin/SKILL.mdskills/nemo-relay-tune-adaptive-config/SKILL.md
{docs/**,README.md,CONTRIBUTING.md,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Run docs link validation with
just docs-linkcheckwhen links change
Files:
skills/nemo-relay-build-plugin/SKILL.mdskills/nemo-relay-tune-adaptive-config/SKILL.mddocs/observability-plugin/atof.mdxdocs/build-plugins/register-behavior.mdxdocs/build-plugins/plugin-configuration-files.mdx
{docs/**,README.md,**/Cargo.toml,**/package.json,**/*.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Ensure renamed public surfaces are reflected consistently in manifests and docs for large or public-facing changes
Files:
skills/nemo-relay-build-plugin/SKILL.mdskills/nemo-relay-tune-adaptive-config/SKILL.mddocs/observability-plugin/atof.mdxdocs/build-plugins/register-behavior.mdxdocs/build-plugins/plugin-configuration-files.mdx
**/*.{md,mdx,py,sh,yaml,yml,toml,json}
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
Keep package names, repo references, and build commands current
Files:
skills/nemo-relay-build-plugin/SKILL.mdskills/nemo-relay-tune-adaptive-config/SKILL.mdpython/nemo_relay/plugin.pydocs/observability-plugin/atof.mdxdocs/build-plugins/register-behavior.mdxpython/tests/test_plugin_config.pydocs/build-plugins/plugin-configuration-files.mdx
**/*.{html,md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Include SPDX license header in HTML and Markdown files using HTML comment syntax
Files:
skills/nemo-relay-build-plugin/SKILL.mdskills/nemo-relay-tune-adaptive-config/SKILL.mddocs/observability-plugin/atof.mdxdocs/build-plugins/register-behavior.mdxdocs/build-plugins/plugin-configuration-files.mdx
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}
📄 CodeRabbit inference engine (AGENTS.md)
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.
Files:
skills/nemo-relay-build-plugin/SKILL.mdcrates/wasm/wrappers/esm/index.jsskills/nemo-relay-tune-adaptive-config/SKILL.mdpython/nemo_relay/plugin.pycrates/node/plugin.jscrates/core/src/plugin.rscrates/wasm/wrappers/esm/plugin.d.tscrates/node/src/api/mod.rscrates/node/plugin.d.tscrates/ffi/src/api/mod.rscrates/wasm/wrappers/nodejs/plugin.jscrates/cli/tests/coverage/doctor_tests.rscrates/cli/src/launcher.rscrates/wasm/src/api/mod.rscrates/cli/tests/coverage/server_tests.rsgo/nemo_relay/plugin_gap_test.gocrates/cli/tests/cli_tests.rscrates/wasm/wrappers/esm/plugin.jsgo/nemo_relay/plugin.gocrates/core/tests/unit/plugin_tests.rscrates/cli/src/server.rscrates/cli/src/doctor.rscrates/ffi/src/api/plugin.rscrates/cli/tests/coverage/gateway_tests.rspython/tests/test_plugin_config.pycrates/ffi/tests/unit/api/plugin_tests.rscrates/python/src/py_plugin.rscrates/cli/tests/coverage/session_tests.rscrates/cli/src/config.rscrates/cli/tests/coverage/config_tests.rs
**/SKILL.md
📄 CodeRabbit inference engine (AGENTS.md)
SKILL.md files are skill entrypoints and do not need SPDX headers, but they must always start with YAML frontmatter containing at least
nameanddescription.
Files:
skills/nemo-relay-build-plugin/SKILL.mdskills/nemo-relay-tune-adaptive-config/SKILL.md
⚙️ CodeRabbit configuration file
**/SKILL.md: Do not flag SKILL.md files for missing SPDX headers. Skill entrypoints intentionally start with YAML frontmatter instead.
Verify that every SKILL.md keeps valid YAML frontmatter with at least name and description fields before the Markdown body.
Files:
skills/nemo-relay-build-plugin/SKILL.mdskills/nemo-relay-tune-adaptive-config/SKILL.md
{crates/adaptive/**,python/nemo_relay/adaptive.py,python/nemo_relay/plugin.py,go/nemo_relay/adaptive/**,go/nemo_relay/!(adaptive)/**,**/node/**,**/wasm/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Keep adaptive surface in sync across crates/adaptive, shared plugin behavior in core and bindings, Python adaptive/plugin wrappers in python/nemo_relay/adaptive.py and python/nemo_relay/plugin.py, Go adaptive helpers under go/nemo_relay/adaptive plus shared plugin helpers in go/nemo_relay, and Node/WebAssembly adaptive helpers and plugin wrappers
Files:
crates/wasm/wrappers/esm/index.jscrates/node/tests/plugin_tests.mjspython/nemo_relay/plugin.pycrates/node/plugin.jscrates/wasm/wrappers/esm/plugin.d.tscrates/node/src/api/mod.rscrates/node/plugin.d.tscrates/wasm/wrappers/nodejs/plugin.jscrates/wasm/src/api/mod.rscrates/wasm/tests-js/plugin_tests.mjscrates/wasm/wrappers/esm/plugin.js
{crates/adaptive/**,python/nemo_relay/plugin.py,go/nemo_relay/**,**/node/**,**/wasm/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
{crates/adaptive/**,python/nemo_relay/plugin.py,go/nemo_relay/**,**/node/**,**/wasm/**}: Maintain consistent plugin lifecycle across all language bindings (Python, Go, Node/WebAssembly, and Rust)
Keep plugin context surfaces aligned across all language implementations
Files:
crates/wasm/wrappers/esm/index.jscrates/node/tests/plugin_tests.mjspython/nemo_relay/plugin.pycrates/node/plugin.jscrates/wasm/wrappers/esm/plugin.d.tscrates/node/src/api/mod.rscrates/node/plugin.d.tscrates/wasm/wrappers/nodejs/plugin.jscrates/wasm/src/api/mod.rsgo/nemo_relay/plugin_gap_test.gocrates/wasm/tests-js/plugin_tests.mjscrates/wasm/wrappers/esm/plugin.jsgo/nemo_relay/plugin.go
**/*.{wasm,js,ts}{,x}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure WebAssembly package naming conventions are consistent with generated package expectations and downstream consumption
Files:
crates/wasm/wrappers/esm/index.jscrates/node/plugin.jscrates/wasm/wrappers/esm/plugin.d.tscrates/node/plugin.d.tscrates/wasm/wrappers/nodejs/plugin.jscrates/wasm/wrappers/esm/plugin.js
crates/wasm/{wrappers,tests-js,scripts}/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.agents/skills/test-wasm-binding/SKILL.md)
Format changed WebAssembly JS/TS wrapper files with
npm run precommit:format --workspace=nemo-relay-node -- crates/wasm/wrappers crates/wasm/tests-js crates/wasm/scripts
Files:
crates/wasm/wrappers/esm/index.jscrates/wasm/wrappers/esm/plugin.d.tscrates/wasm/wrappers/nodejs/plugin.jscrates/wasm/wrappers/esm/plugin.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Run Node.js formatting with
npm run format --workspace=nemo-relay-nodeInclude SPDX license header in all JavaScript and TypeScript source files using double-slash comment syntax
Files:
crates/wasm/wrappers/esm/index.jscrates/node/plugin.jscrates/wasm/wrappers/esm/plugin.d.tscrates/node/plugin.d.tscrates/wasm/wrappers/nodejs/plugin.jscrates/wasm/wrappers/esm/plugin.js
crates/wasm/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Run WebAssembly formatting with
npm run precommit:format --workspace=nemo-relay-node -- crates/wasm/wrappers crates/wasm/tests-js crates/wasm/scripts
Files:
crates/wasm/wrappers/esm/index.jscrates/wasm/wrappers/esm/plugin.d.tscrates/wasm/wrappers/nodejs/plugin.jscrates/wasm/wrappers/esm/plugin.js
**/*.{rs,py,go,js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions: Rust and Python use
snake_case, C FFI exports prefixednemo_relay_, Go usesPascalCasefor public APIs, Node.js usescamelCase.
Files:
crates/wasm/wrappers/esm/index.jspython/nemo_relay/plugin.pycrates/node/plugin.jscrates/core/src/plugin.rscrates/wasm/wrappers/esm/plugin.d.tscrates/node/src/api/mod.rscrates/node/plugin.d.tscrates/ffi/src/api/mod.rscrates/wasm/wrappers/nodejs/plugin.jscrates/cli/tests/coverage/doctor_tests.rscrates/cli/src/launcher.rscrates/wasm/src/api/mod.rscrates/cli/tests/coverage/server_tests.rsgo/nemo_relay/plugin_gap_test.gocrates/cli/tests/cli_tests.rscrates/wasm/wrappers/esm/plugin.jsgo/nemo_relay/plugin.gocrates/core/tests/unit/plugin_tests.rscrates/cli/src/server.rscrates/cli/src/doctor.rscrates/ffi/src/api/plugin.rscrates/cli/tests/coverage/gateway_tests.rspython/tests/test_plugin_config.pycrates/ffi/tests/unit/api/plugin_tests.rscrates/python/src/py_plugin.rscrates/cli/tests/coverage/session_tests.rscrates/cli/src/config.rscrates/cli/tests/coverage/config_tests.rs
crates/{python,ffi,node,wasm}/**/*
⚙️ CodeRabbit configuration file
crates/{python,ffi,node,wasm}/**/*: Treat binding changes as public API changes. Check for parity with the other language bindings, FFI ownership/lifetime safety,
callback error propagation, stable type conversion, and consistent async/stream semantics.
Flag changes that update one binding without corresponding tests or documentation for the same surface elsewhere.
Files:
crates/wasm/wrappers/esm/index.jscrates/node/tests/plugin_tests.mjscrates/node/plugin.jscrates/wasm/wrappers/esm/plugin.d.tscrates/ffi/nemo_relay.hcrates/node/src/api/mod.rscrates/node/plugin.d.tscrates/ffi/src/api/mod.rscrates/wasm/wrappers/nodejs/plugin.jscrates/wasm/src/api/mod.rscrates/wasm/tests-js/plugin_tests.mjscrates/wasm/wrappers/esm/plugin.jscrates/ffi/src/api/plugin.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/python/src/py_plugin.rs
{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/node/tests/plugin_tests.mjscrates/cli/tests/coverage/doctor_tests.rscrates/cli/tests/coverage/server_tests.rsgo/nemo_relay/plugin_gap_test.gocrates/cli/tests/cli_tests.rscrates/core/tests/unit/plugin_tests.rscrates/cli/tests/coverage/gateway_tests.rspython/tests/test_plugin_config.pycrates/ffi/tests/unit/api/plugin_tests.rscrates/cli/tests/coverage/session_tests.rscrates/cli/tests/coverage/config_tests.rs
{crates/python/src/py_api/**/*.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update Python native binding in
crates/python/src/py_api/mod.rswith Python wrapper docstring inpython/nemo_relay/<module>.pyand type stubs inpython/nemo_relay/*.pyimodules
Files:
python/nemo_relay/plugin.pypython/nemo_relay/plugin.pyipython/nemo_relay/_native.pyi
python/nemo_relay/**/*.py
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
snake_casenaming convention for Python identifiers (e.g.,nemo_relay.tools.call)Format changed Python wrapper and test files with
uv run ruff format pythonPython wrapper modules live under
python/nemo_relay/; the native extension is built fromcrates/pythonwithmaturin.
Files:
python/nemo_relay/plugin.py
{python/nemo_relay/adaptive.py,python/nemo_relay/plugin.py,go/nemo_relay/adaptive/**,**/node/adaptive/**,**/wasm/adaptive/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Ensure typed helper constructors still map cleanly to the same config document in adaptive bindings across Python, Go, Node, and WebAssembly
Files:
python/nemo_relay/plugin.py
{pyproject.toml,**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistency between Python package names in
pyproject.tomland import paths used throughout the codebase
Files:
python/nemo_relay/plugin.pypython/tests/test_plugin_config.py
**/*.{py,txt,toml,cfg,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Python package names and top-level module imports during coordinated rename operations
Files:
python/nemo_relay/plugin.pypython/tests/test_plugin_config.py
**/*.py
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*.py: Run Python formatting withuv run ruff format python
Run Python testing withuv run pytest -k "<pattern>"
**/*.py: Use Ruff with rule sets E, F, W, I for Python linting
Use Ruff formatter with line length 120 and double quotes for Python code formatting
Runtyfor Python type checking
Use Python snake_case naming convention for Python identifiers
Include SPDX license header in all Python source files using hash comment syntax
Validate Python code withuv run pre-commit run --all-filesto enforce Ruff linting and formatting, and ty type checking
Files:
python/nemo_relay/plugin.pypython/tests/test_plugin_config.py
python/nemo_relay/**/*
⚙️ CodeRabbit configuration file
python/nemo_relay/**/*: Review Python wrapper changes for typed API consistency, contextvars-based scope isolation, async behavior, and parity with the native extension.
Stubs and runtime implementations should stay aligned.
Files:
python/nemo_relay/plugin.pypython/nemo_relay/plugin.pyipython/nemo_relay/_native.pyi
crates/node/**/*.{js,ts,jsx,tsx,json}
📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md)
Format changed Node files with
npm run format --workspace=nemo-relay-node
Files:
crates/node/plugin.jscrates/node/plugin.d.ts
crates/node/**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Node.js public entry points include the main runtime package plus
nemo-relay-node/typed,nemo-relay-node/plugin, andnemo-relay-node/adaptive.
Files:
crates/node/plugin.jscrates/node/plugin.d.ts
**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
snake_casenaming convention for Rust identifiers (e.g.,nemo_relay_tool_call)
**/*.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 work
**/*.rs: Runcargo fmt --allto format all Rust code
Runcargo clippy --workspace --all-targets -- -D warningsto enforce all clippy lints as errors
**/*.rs: Runcargo fmt --allwhen Rust files changed as part of WebAssembly work
Runcargo clippy --workspace --all-targets -- -D warningswhen Rust files changed as part of WebAssembly work
**/*.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
Run Rust formatting withcargo fmt --all
Run Rust linting withcargo clippy --workspace --all-targets -- -D warnings
**/*.rs: Usecargo fmtfor Rust code formatting
Runcargo clippy -- -D warningsto lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code withuv run pre-commit run --all-filesto enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...
Files:
crates/core/src/plugin.rscrates/node/src/api/mod.rscrates/ffi/src/api/mod.rscrates/cli/tests/coverage/doctor_tests.rscrates/cli/src/launcher.rscrates/wasm/src/api/mod.rscrates/cli/tests/coverage/server_tests.rscrates/cli/tests/cli_tests.rscrates/core/tests/unit/plugin_tests.rscrates/cli/src/server.rscrates/cli/src/doctor.rscrates/ffi/src/api/plugin.rscrates/cli/tests/coverage/gateway_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/python/src/py_plugin.rscrates/cli/tests/coverage/session_tests.rscrates/cli/src/config.rscrates/cli/tests/coverage/config_tests.rs
**/{Cargo.toml,**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Maintain consistency between Rust package names in
Cargo.tomland their actual usage across the codebase
Files:
crates/core/src/plugin.rscrates/node/src/api/mod.rscrates/ffi/src/api/mod.rscrates/cli/tests/coverage/doctor_tests.rscrates/cli/src/launcher.rscrates/wasm/src/api/mod.rscrates/cli/tests/coverage/server_tests.rscrates/cli/tests/cli_tests.rscrates/core/tests/unit/plugin_tests.rscrates/cli/src/server.rscrates/cli/src/doctor.rscrates/ffi/src/api/plugin.rscrates/cli/tests/coverage/gateway_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/python/src/py_plugin.rscrates/cli/tests/coverage/session_tests.rscrates/cli/src/config.rscrates/cli/tests/coverage/config_tests.rs
**/*.{h,hpp,c,cpp,rs}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure FFI header and library naming follows consistent conventions across platform-specific builds
Files:
crates/core/src/plugin.rscrates/ffi/nemo_relay.hcrates/node/src/api/mod.rscrates/ffi/src/api/mod.rscrates/cli/tests/coverage/doctor_tests.rscrates/cli/src/launcher.rscrates/wasm/src/api/mod.rscrates/cli/tests/coverage/server_tests.rscrates/cli/tests/cli_tests.rscrates/core/tests/unit/plugin_tests.rscrates/cli/src/server.rscrates/cli/src/doctor.rscrates/ffi/src/api/plugin.rscrates/cli/tests/coverage/gateway_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/python/src/py_plugin.rscrates/cli/tests/coverage/session_tests.rscrates/cli/src/config.rscrates/cli/tests/coverage/config_tests.rs
{crates/core,crates/adaptive}/**/*
📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)
Changes to
crates/coreorcrates/adaptivemust run the full language matrix
Files:
crates/core/src/plugin.rscrates/core/tests/unit/plugin_tests.rs
**/*.{rs,toml}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Rust crate names and module prefixes during coordinated rename operations
Files:
crates/core/src/plugin.rscrates/node/src/api/mod.rscrates/ffi/src/api/mod.rscrates/cli/tests/coverage/doctor_tests.rscrates/cli/src/launcher.rscrates/wasm/src/api/mod.rscrates/cli/tests/coverage/server_tests.rscrates/cli/tests/cli_tests.rscrates/core/tests/unit/plugin_tests.rscrates/cli/src/server.rscrates/cli/src/doctor.rscrates/ffi/src/api/plugin.rscrates/cli/tests/coverage/gateway_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/python/src/py_plugin.rscrates/cli/tests/coverage/session_tests.rscrates/cli/src/config.rscrates/cli/tests/coverage/config_tests.rs
crates/core/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/coreor shared runtime semantics, also usevalidate-changefor broader validation
crates/core/**/*.rs: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.
Files:
crates/core/src/plugin.rscrates/core/tests/unit/plugin_tests.rs
crates/{core,adaptive}/**
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If
crates/coreorcrates/adaptivechanged, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly
Files:
crates/core/src/plugin.rscrates/core/tests/unit/plugin_tests.rs
crates/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
UseJson = serde_json::Valuein Rust-facing runtime APIs for JSON payload handling.
Files:
crates/core/src/plugin.rscrates/node/src/api/mod.rscrates/ffi/src/api/mod.rscrates/cli/tests/coverage/doctor_tests.rscrates/cli/src/launcher.rscrates/wasm/src/api/mod.rscrates/cli/tests/coverage/server_tests.rscrates/cli/tests/cli_tests.rscrates/core/tests/unit/plugin_tests.rscrates/cli/src/server.rscrates/cli/src/doctor.rscrates/ffi/src/api/plugin.rscrates/cli/tests/coverage/gateway_tests.rscrates/ffi/tests/unit/api/plugin_tests.rscrates/python/src/py_plugin.rscrates/cli/tests/coverage/session_tests.rscrates/cli/src/config.rscrates/cli/tests/coverage/config_tests.rs
crates/{core,adaptive}/**/*.rs
⚙️ CodeRabbit configuration file
crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.
Files:
crates/core/src/plugin.rscrates/core/tests/unit/plugin_tests.rs
{docs/**,README.md,CONTRIBUTING.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
{docs/**,README.md,CONTRIBUTING.md}: For docs-only changes, run targeted checks only if commands, package names, or examples changed. Usejust docsfor docs-site builds andjust docs-linkcheckwhen links changed
Run docs site build withjust docs
Files:
docs/observability-plugin/atof.mdxdocs/build-plugins/register-behavior.mdxdocs/build-plugins/plugin-configuration-files.mdx
{docs/**,README.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Verify README and docs entry points still match current package names and paths for large or public-facing changes
Files:
docs/observability-plugin/atof.mdxdocs/build-plugins/register-behavior.mdxdocs/build-plugins/plugin-configuration-files.mdx
{docs/**,examples/**,README.md}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Verify examples still run with documented commands for large or public-facing changes
Files:
docs/observability-plugin/atof.mdxdocs/build-plugins/register-behavior.mdxdocs/build-plugins/plugin-configuration-files.mdx
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md)
In MDX files, top-of-file comments must use JSX comment delimiters: {/* to open and */} to close. Do not use HTML comments for MDX SPDX headers.
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
Files:
docs/observability-plugin/atof.mdxdocs/build-plugins/register-behavior.mdxdocs/build-plugins/plugin-configuration-files.mdx
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Update embedded documentation snippets, patch docs, and binding-support notes if examples or supported bindings changed
Files:
docs/observability-plugin/atof.mdxdocs/build-plugins/register-behavior.mdxdocs/build-plugins/plugin-configuration-files.mdx
docs/**
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Run
just docsor./scripts/build-docs.sh htmlto regenerate ignored Fern API reference pages before validation for documentation site changes
Files:
docs/observability-plugin/atof.mdxdocs/build-plugins/register-behavior.mdxdocs/build-plugins/plugin-configuration-files.mdx
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}
⚙️ CodeRabbit configuration file
{docs/**,README.md,CONTRIBUTING.md,RELEASING.md,SECURITY.md}: Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.
Flag stale examples, missing SPDX headers where required, and instructions that no longer match CI or pre-commit behavior.
Files:
docs/observability-plugin/atof.mdxdocs/build-plugins/register-behavior.mdxdocs/build-plugins/plugin-configuration-files.mdx
**/*.{h,c}
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update C header names and symbol prefixes during coordinated rename operations
Files:
crates/ffi/nemo_relay.h
crates/ffi/**
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Rebuild the FFI crate in release mode so the shared library and header stay in sync when making changes to crates/ffi
Files:
crates/ffi/nemo_relay.hcrates/ffi/src/api/mod.rscrates/ffi/src/api/plugin.rscrates/ffi/tests/unit/api/plugin_tests.rs
crates/ffi/nemo_relay.h
📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md)
Check the generated header diff when any exported symbol or type changed in the FFI surface
Ensure FFI header sync for
crates/ffi/nemo_relay.hthrough Cargo/build.rs
Files:
crates/ffi/nemo_relay.h
crates/ffi/*.h
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Prefix C FFI exports with
nemo_relay_
Files:
crates/ffi/nemo_relay.h
crates/node/src/api/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update Node.js binding in
crates/node/src/api/mod.rsfor language-native bindings
Files:
crates/node/src/api/mod.rs
crates/node/src/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
camelCasenaming convention for Node.js identifiers (e.g.,toolCall)
Files:
crates/node/src/api/mod.rs
crates/node/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md)
Use
npm run check:docstrings --workspace=nemo-relay-nodeto validate public API docstring checks when surface docs changed
Files:
crates/node/plugin.d.ts
crates/ffi/src/api/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
crates/ffi/src/api/**/*.rs: Add or update FFI wrappers in relevantcrates/ffi/src/api/*.rsmodules, re-export throughcrates/ffi/src/api/mod.rs, and ensure generatedcrates/ffi/nemo_relay.hstays correct
Usenemo_relay_prefix for C FFI function names (e.g.,nemo_relay_tool_call)
Files:
crates/ffi/src/api/mod.rscrates/ffi/src/api/plugin.rs
crates/ffi/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)
If the change touched
crates/ffi, also usetest-ffi-surfacefor validation
Files:
crates/ffi/src/api/mod.rscrates/ffi/src/api/plugin.rscrates/ffi/tests/unit/api/plugin_tests.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Maintain documented and tested validation and report behavior for adaptive surfaces
Files:
crates/cli/tests/coverage/doctor_tests.rscrates/cli/tests/coverage/server_tests.rsgo/nemo_relay/plugin_gap_test.gocrates/cli/tests/cli_tests.rscrates/core/tests/unit/plugin_tests.rscrates/cli/tests/coverage/gateway_tests.rspython/tests/test_plugin_config.pycrates/ffi/tests/unit/api/plugin_tests.rscrates/cli/tests/coverage/session_tests.rscrates/cli/tests/coverage/config_tests.rs
crates/wasm/src/api/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update WebAssembly binding in
crates/wasm/src/api/mod.rsfor language-native bindings
Files:
crates/wasm/src/api/mod.rs
crates/wasm/src/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
camelCasenaming convention for WebAssembly identifiers (e.g.,toolCall)
Files:
crates/wasm/src/api/mod.rs
crates/wasm/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-wasm-binding/SKILL.md)
Run
cargo test -p nemo-relay-wasmwhen Rust-only WebAssembly helpers changed
Files:
crates/wasm/src/api/mod.rs
go/nemo_relay/**/*.go
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update Go wrapper in
go/nemo_relay/nemo_relay.gowith doc comment and shorthand package if the capability belongs there
go/nemo_relay/**/*.go: Format changed Go packages withcd go/nemo_relay && go fmt ./...
Run Go tests withjust test-goto build and test the NeMo Relay Go binding
Usejust build-gowhen you want an explicit build-only pass or need the artifact for other work
Usejust ci=true test-gowhen you need the CI-style coverage and JUnit path
On macOS, setDYLD_LIBRARY_PATHto the../../target/releasedirectory before running the rawgo testcommand directly
Files:
go/nemo_relay/plugin_gap_test.gogo/nemo_relay/plugin.go
go/**/*.go
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Use
PascalCasenaming convention for Go identifiers (e.g.,nemo_relay.ToolCall)Run Go formatting with
cd go/nemo_relay && go fmt ./...
Files:
go/nemo_relay/plugin_gap_test.gogo/nemo_relay/plugin.go
{go/nemo_relay/go.mod,go/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)
Ensure Go module path in
go/nemo_relay/go.modmatches import statements in Go source files
Files:
go/nemo_relay/plugin_gap_test.gogo/nemo_relay/plugin.go
**/*.go
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update Go module paths and package paths during coordinated rename operations
**/*.go: Usegofmtfor Go code formatting
Rungo vet ./...for Go static analysis
Use Go PascalCase naming convention for Go identifiers
Include SPDX license header in all Go source files using double-slash comment syntax
Validate Go code withuv run pre-commit run --all-filesto enforce gofmt formatting and go vet static analysis
Files:
go/nemo_relay/plugin_gap_test.gogo/nemo_relay/plugin.go
go/nemo_relay/**/*
⚙️ CodeRabbit configuration file
go/nemo_relay/**/*: 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_relay/plugin_gap_test.gogo/nemo_relay/plugin.go
**/test_*.{py,py}
📄 CodeRabbit inference engine (.agents/skills/add-integration/SKILL.md)
Relevant integration tests or smoke coverage must exist for the integration path
Files:
python/tests/test_plugin_config.py
**/*config*.{rs,ts,py,go,js,json,yaml,yml}
📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)
Ensure dynamic config shape still matches the documented canonical model
Files:
python/tests/test_plugin_config.pycrates/cli/src/config.rscrates/cli/tests/coverage/config_tests.rs
python/**/*test*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/**/*test*.py: Do not add@pytest.mark.asyncioto any test in Python test files
Do not add a-> Nonereturn type annotation to test functions
When mocking a class, useunittest.mock.MagicMockorunittest.mock.AsyncMockwith thespecconstructor argument when necessary, rather than defining a new class
Prefix mocked class names withmock, notfake
Prefer pytest fixtures over helper methods in Python tests
Preferpytest.mark.parametrizeover creating individual tests for different input types
Files:
python/tests/test_plugin_config.py
python/**/{conftest.py,*test*.py}
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
When creating a fixture follow the pattern:
@pytest.fixture(name="<fixture_name>"[, scope="<scope>"]) def <fixture_name>_fixture() -> <return_type>:and only specify the scope argument when the value is something other than "function"
Files:
python/tests/test_plugin_config.py
crates/python/**/*.rs
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
If the native Rust bridge changed, add the Rust crate tests for
nemo-relay-python
Files:
crates/python/src/py_plugin.rs
🧠 Learnings (1)
📚 Learning: 2026-05-07T18:04:44.387Z
Learnt from: mnajafian-nv
Repo: NVIDIA/NeMo-Flow PR: 67
File: integrations/openclaw/src/modules.ts:1-2
Timestamp: 2026-05-07T18:04:44.387Z
Learning: In NVIDIA/NeMo-Flow, TypeScript source files should use `//` line comments for SPDX headers (e.g., `// SPDX-FileCopyrightText: ...` and `// SPDX-License-Identifier: ...`) rather than C-style block comments (`/* ... */`). The repo’s copyright checker enforces this mapping, so `//` SPDX headers in `.ts` files should not be flagged as a style violation.
Applied to files:
crates/wasm/wrappers/esm/plugin.d.tscrates/node/plugin.d.ts
🔇 Additional comments (12)
crates/ffi/src/api/plugin.rs (1)
12-18: LGTM!Also applies to: 129-155
crates/node/tests/plugin_tests.mjs (1)
9-13: LGTM!crates/wasm/tests-js/plugin_tests.mjs (1)
18-22: LGTM!crates/wasm/wrappers/esm/index.js (1)
43-43: LGTM!go/nemo_relay/plugin.go (1)
28-28: LGTM!Also applies to: 94-112, 263-278, 588-598
crates/wasm/src/api/mod.rs (1)
2167-2178: LGTM!crates/node/plugin.d.ts (1)
164-176: LGTM!crates/node/plugin.js (1)
51-65: LGTM!Also applies to: 175-179
python/nemo_relay/plugin.pyi (1)
111-111: LGTM!crates/wasm/wrappers/esm/plugin.d.ts (1)
162-174: LGTM!crates/wasm/wrappers/esm/plugin.js (1)
4-13: LGTM!Also applies to: 54-68
crates/wasm/wrappers/nodejs/plugin.js (1)
6-15: LGTM!Also applies to: 56-70, 179-181
| for component in overlay_components { | ||
| let Some(kind) = json_component_kind(&component).map(str::to_owned) else { | ||
| base_components.push(component); | ||
| continue; | ||
| }; | ||
| if let Some(existing) = base_components | ||
| .iter_mut() | ||
| .find(|candidate| json_component_kind(candidate) == Some(kind.as_str())) | ||
| { | ||
| merge_json_value(existing, component); | ||
| } else { |
There was a problem hiding this comment.
Do not merge duplicate component kinds into the first match.
Line 192 makes layer_plugin_config lossy for valid configs that contain multiple components with the same kind. validate_plugin_config already allows duplicate kinds for plugins where allows_multiple_components() is true, but this loop always merges the overlay into the first matching base entry and leaves no way to target later instances. An overlay like [{kind:"x"}, {kind:"x"}] will also collapse onto one base component instead of preserving two distinct updates.
Please either reject duplicate kind values before layering or introduce a stable per-instance key for matching; otherwise this public API can silently rewrite the wrong component across every binding.
🤖 Prompt for 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.
In `@crates/core/src/plugin.rs` around lines 187 - 197, The layering loop in
layer_plugin_config wrongly collapses multiple overlay components with the same
kind into the first base match (see overlay_components, base_components,
json_component_kind, merge_json_value), breaking configs where
validate_plugin_config and allows_multiple_components permit duplicates; update
layer_plugin_config to either (A) detect duplicate kinds in the overlay for
kinds that allow_multiple_components() and reject early with a clear error, or
(B) implement stable per-instance matching instead of first-match merging —
e.g., require or derive a per-instance identifier (or positional/occurrence
index) and use that to find the correct base entry before calling
merge_json_value, falling back to appending a new component when no matching
instance-id is found. Ensure behavior is consistent with validate_plugin_config
and document which strategy is used.
| /** | ||
| * Layer one raw plugin config document over another and return the effective JSON document. | ||
| * | ||
| * # Safety | ||
| * `base_json` and `overlay_json` must be valid C strings and `out_json` must be a valid, | ||
| * non-null pointer. | ||
| */ | ||
| NemoRelayStatus nemo_relay_layer_plugin_config(const char *base_json, | ||
| const char *overlay_json, | ||
| char **out_json); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Document out_json ownership explicitly.
This API returns an allocated JSON string via out_json, but the header does not say the caller must free *out_json with nemo_relay_string_free. That omission is easy to turn into leaks for C consumers.
Proposed doc fix
/**
* Layer one raw plugin config document over another and return the effective JSON document.
+ *
+ * On success, writes an allocated JSON string to `out_json`. The caller must
+ * free `*out_json` with `nemo_relay_string_free`.
*
* # Safety
* `base_json` and `overlay_json` must be valid C strings and `out_json` must be a valid,
* non-null pointer.
*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Layer one raw plugin config document over another and return the effective JSON document. | |
| * | |
| * # Safety | |
| * `base_json` and `overlay_json` must be valid C strings and `out_json` must be a valid, | |
| * non-null pointer. | |
| */ | |
| NemoRelayStatus nemo_relay_layer_plugin_config(const char *base_json, | |
| const char *overlay_json, | |
| char **out_json); | |
| /** | |
| * Layer one raw plugin config document over another and return the effective JSON document. | |
| * | |
| * On success, writes an allocated JSON string to `out_json`. The caller must | |
| * free `*out_json` with `nemo_relay_string_free`. | |
| * | |
| * # Safety | |
| * `base_json` and `overlay_json` must be valid C strings and `out_json` must be a valid, | |
| * non-null pointer. | |
| */ | |
| NemoRelayStatus nemo_relay_layer_plugin_config(const char *base_json, | |
| const char *overlay_json, | |
| char **out_json); |
🤖 Prompt for 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.
In `@crates/ffi/nemo_relay.h` around lines 1116 - 1125, The header for
nemo_relay_layer_plugin_config is missing ownership semantics for out_json;
update the function doc comment to state that on success the function allocates
a NUL-terminated JSON string and stores it in *out_json and that the caller
takes ownership and must free *out_json using nemo_relay_string_free (or
equivalent API) to avoid leaks; reference nemo_relay_layer_plugin_config and
nemo_relay_string_free in the updated comment so C consumers know who is
responsible for deallocation.
| #[test] | ||
| fn test_ffi_layer_plugin_config_round_trips_merge() { | ||
| // Smoke test only: merge semantics are covered by the core crate. This | ||
| // verifies the FFI boundary forwards both documents and returns merged JSON. | ||
| let base = cstring(&json!({ "a": 1 }).to_string()); | ||
| let overlay = cstring(&json!({ "b": 2 }).to_string()); | ||
|
|
||
| unsafe { | ||
| let mut out_json = ptr::null_mut(); | ||
| assert_eq!( | ||
| nemo_relay_layer_plugin_config(base.as_ptr(), overlay.as_ptr(), &mut out_json), | ||
| NemoRelayStatus::Ok | ||
| ); | ||
| assert_eq!(returned_json(out_json), json!({ "a": 1, "b": 2 })); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add FFI error-path coverage for the new ABI.
This only pins the success case. Please add boundary tests for at least invalid JSON input and null out_json so the public FFI status mapping does not regress unnoticed.
As per coding guidelines, Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
🤖 Prompt for 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.
In `@crates/ffi/tests/unit/api/plugin_tests.rs` around lines 8 - 23, Add two
negative tests for nemo_relay_layer_plugin_config: one that passes an invalid
JSON C string (e.g., cstring("not-json")) as either base or overlay and asserts
the call returns the FFI error status for bad JSON (compare to the appropriate
NemoRelayStatus variant), and one that passes a null mutable out_json pointer
(use &mut ptr::null_mut() or equivalent) and asserts the call returns the FFI
error status for a null/invalid output pointer; place these alongside
test_ffi_layer_plugin_config_round_trips_merge and use the same helpers
(cstring, returned_json) but do not attempt to dereference or consume out_json
on error paths.
| | `output_directory` | Current working directory | Directory containing the JSONL file. The directory must exist before initialization. | | ||
| | `filename` | Timestamped `nemo-relay-events-*.jsonl` | Explicit output filename inside `output_directory`. The exporter creates the file but not parent directories. | |
There was a problem hiding this comment.
Align the ATOF directory docs with current runtime behavior.
These notes now contradict the current CLI/runtime contract: doctor treats a missing ATOF directory as a warning and explicitly says the runtime will create it. As written, this page tells users pre-creating the directory is required.
Suggested doc fix
-| `output_directory` | Current working directory | Directory containing the JSONL file. The directory must exist before initialization. |
-| `filename` | Timestamped `nemo-relay-events-*.jsonl` | Explicit output filename inside `output_directory`. The exporter creates the file but not parent directories. |
+| `output_directory` | Current working directory | Directory containing the JSONL file. If it does not exist, the runtime creates it. |
+| `filename` | Timestamped `nemo-relay-events-*.jsonl` | Explicit output filename inside `output_directory`. The exporter creates the file in that directory. |As per coding guidelines, "Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | `output_directory` | Current working directory | Directory containing the JSONL file. The directory must exist before initialization. | | |
| | `filename` | Timestamped `nemo-relay-events-*.jsonl` | Explicit output filename inside `output_directory`. The exporter creates the file but not parent directories. | | |
| | `output_directory` | Current working directory | Directory containing the JSONL file. If it does not exist, the runtime creates it. | | |
| | `filename` | Timestamped `nemo-relay-events-*.jsonl` | Explicit output filename inside `output_directory`. The exporter creates the file in that directory. | |
🤖 Prompt for 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.
In `@docs/observability-plugin/atof.mdx` around lines 44 - 45, The docs for ATOF's
`output_directory` and `filename` are inconsistent with runtime behavior: update
the description for `output_directory` to state that the runtime (and the
`doctor` command) will create the directory if missing and that `doctor` only
emits a warning rather than requiring pre-creation; keep `filename` notes about
the exporter creating the file but not parent directories but clarify that
parent directories are created by runtime when using `output_directory`. Edit
the `output_directory` and `filename` table entries in the ATOF docs to reflect
this corrected contract and mention `doctor` by name so readers know where the
warning originates.
| func TestLayerPluginConfigRoundTripsMerge(t *testing.T) { | ||
| // Smoke test only: merge semantics are covered by the core crate. This | ||
| // verifies the cgo boundary forwards both documents and returns merged JSON. | ||
| merged, err := LayerPluginConfig( | ||
| map[string]any{"a": float64(1)}, | ||
| map[string]any{"b": float64(2)}, | ||
| ) | ||
| if err != nil { | ||
| t.Fatalf("LayerPluginConfig failed: %v", err) | ||
| } | ||
|
|
||
| expected := map[string]any{"a": float64(1), "b": float64(2)} | ||
| if !reflect.DeepEqual(merged, expected) { | ||
| t.Fatalf("merged config mismatch:\n got: %#v\nwant: %#v", merged, expected) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Cover the new binding’s serialization failure path.
This only verifies the happy path. LayerPluginConfig now marshals arbitrary Go values before crossing cgo, so add a case with an unsupported value and assert it fails before FFI. That protects the jsonCString path this PR touched.
Suggested test addition
func TestLayerPluginConfigRoundTripsMerge(t *testing.T) {
// Smoke test only: merge semantics are covered by the core crate. This
// verifies the cgo boundary forwards both documents and returns merged JSON.
merged, err := LayerPluginConfig(
map[string]any{"a": float64(1)},
map[string]any{"b": float64(2)},
)
if err != nil {
t.Fatalf("LayerPluginConfig failed: %v", err)
}
expected := map[string]any{"a": float64(1), "b": float64(2)}
if !reflect.DeepEqual(merged, expected) {
t.Fatalf("merged config mismatch:\n got: %#v\nwant: %#v", merged, expected)
}
}
+
+func TestLayerPluginConfigSerializationErrorsSurfaceBeforeFFI(t *testing.T) {
+ _, err := LayerPluginConfig(
+ map[string]any{"a": float64(1)},
+ map[string]any{"bad": make(chan int)},
+ )
+ if err == nil {
+ t.Fatal("expected LayerPluginConfig serialization error")
+ }
+}As per coding guidelines, go/nemo_relay/**/*_test.go tests should cover the changed API surface, including error paths, and go/nemo_relay/**/* API changes should include focused Go tests.
🤖 Prompt for 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.
In `@go/nemo_relay/plugin_gap_test.go` around lines 38 - 53, Add a failing-path
test for LayerPluginConfig that ensures marshaling errors are returned before
crossing the cgo boundary: update TestLayerPluginConfigRoundTripsMerge (or add a
new test) to call LayerPluginConfig with an unsupported value (e.g., a channel,
function, or cyclic structure) and assert that it returns a non-nil error and
does not return merged JSON; this will exercise the jsonCString/serialization
failure path and ensure the binding surfaces the Go marshaling failure properly.
| def layer(base: PluginConfig | JsonObject, overlay: PluginConfig | JsonObject) -> JsonObject: | ||
| """Layer one plugin configuration over another. | ||
|
|
||
| Args: | ||
| base: Lower-precedence plugin config, usually loaded from files. | ||
| overlay: Higher-precedence plugin config, usually built in code. | ||
|
|
||
| Returns: | ||
| The effective raw JSON plugin config. | ||
|
|
||
| Behavior: | ||
| Objects merge recursively, arrays and scalar values are replaced by the | ||
| overlay, and top-level components merge by `kind`. Passing raw mappings | ||
| preserves omitted fields so they can inherit from the base config. | ||
| """ | ||
| return cast( | ||
| JsonObject, | ||
| _layer_plugin_config(_normalize_object(base), _normalize_object(overlay)), | ||
| ) |
There was a problem hiding this comment.
Preserve explicit None overrides in layer().
layer() normalizes both inputs through _normalize_object(), and _normalize() drops dict entries whose value is None. That means an overlay like {"config": {"timeout": None}} can never clear a base value to JSON null; the key disappears and the base value leaks through instead. This breaks the new layering contract for scalar replacement and raw-mapping overlays.
Proposed fix
-def _normalize(value: object) -> Json:
+def _normalize(value: object, *, drop_none: bool = True) -> Json:
if hasattr(value, "to_dict"):
- return cast(_SupportsToDict, value).to_dict()
+ return cast(_SupportsToDict, value).to_dict()
if is_dataclass(value) and not isinstance(value, type):
return {
- field_info.name: _normalize(field_value)
+ field_info.name: _normalize(field_value, drop_none=drop_none)
for field_info in fields(value)
- if (field_value := getattr(value, field_info.name)) is not None
+ if (field_value := getattr(value, field_info.name)) is not None or not drop_none
}
if isinstance(value, list):
- return [_normalize(item) for item in value]
+ return [_normalize(item, drop_none=drop_none) for item in value]
if isinstance(value, dict):
- return {cast(str, key): _normalize(val) for key, val in value.items() if val is not None}
+ return {
+ cast(str, key): _normalize(val, drop_none=drop_none)
+ for key, val in value.items()
+ if val is not None or not drop_none
+ }
return cast(Json, value)
-def _normalize_object(value: object) -> JsonObject:
- return cast(JsonObject, _normalize(value))
+def _normalize_object(value: object, *, drop_none: bool = True) -> JsonObject:
+ return cast(JsonObject, _normalize(value, drop_none=drop_none))
@@
def layer(base: PluginConfig | JsonObject, overlay: PluginConfig | JsonObject) -> JsonObject:
@@
return cast(
JsonObject,
- _layer_plugin_config(_normalize_object(base), _normalize_object(overlay)),
+ _layer_plugin_config(
+ _normalize_object(base, drop_none=False),
+ _normalize_object(overlay, drop_none=False),
+ ),
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def layer(base: PluginConfig | JsonObject, overlay: PluginConfig | JsonObject) -> JsonObject: | |
| """Layer one plugin configuration over another. | |
| Args: | |
| base: Lower-precedence plugin config, usually loaded from files. | |
| overlay: Higher-precedence plugin config, usually built in code. | |
| Returns: | |
| The effective raw JSON plugin config. | |
| Behavior: | |
| Objects merge recursively, arrays and scalar values are replaced by the | |
| overlay, and top-level components merge by `kind`. Passing raw mappings | |
| preserves omitted fields so they can inherit from the base config. | |
| """ | |
| return cast( | |
| JsonObject, | |
| _layer_plugin_config(_normalize_object(base), _normalize_object(overlay)), | |
| ) | |
| def layer(base: PluginConfig | JsonObject, overlay: PluginConfig | JsonObject) -> JsonObject: | |
| """Layer one plugin configuration over another. | |
| Args: | |
| base: Lower-precedence plugin config, usually loaded from files. | |
| overlay: Higher-precedence plugin config, usually built in code. | |
| Returns: | |
| The effective raw JSON plugin config. | |
| Behavior: | |
| Objects merge recursively, arrays and scalar values are replaced by the | |
| overlay, and top-level components merge by `kind`. Passing raw mappings | |
| preserves omitted fields so they can inherit from the base config. | |
| """ | |
| return cast( | |
| JsonObject, | |
| _layer_plugin_config( | |
| _normalize_object(base, drop_none=False), | |
| _normalize_object(overlay, drop_none=False), | |
| ), | |
| ) |
🤖 Prompt for 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.
In `@python/nemo_relay/plugin.py` around lines 291 - 309, The current layer() call
normalizes both base and overlay via _normalize_object(), but _normalize()
removes dict entries with value None so an overlay intending to set a key to
JSON null (e.g. {"config": {"timeout": None}}) is lost; update the normalization
logic so explicit None values from overlays are preserved: either modify
_normalize()/_normalize_object() to not drop keys whose value is None (or add a
flag like preserve_nulls and call _normalize_object(overlay,
preserve_nulls=True) from layer), then call _layer_plugin_config with the
normalized base and the overlay normalized in a way that retains explicit None
overrides (reference functions: layer, _normalize_object, _normalize).
| def test_layer_plugin_config_round_trips_merge(): | ||
| # Smoke test only: merge semantics are covered by the core crate. This | ||
| # verifies the binding forwards both documents and returns merged JSON. | ||
| assert plugin.layer({"a": 1}, {"b": 2}) == {"a": 1, "b": 2} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Cover the typed wrapper input path too.
This only exercises raw dict inputs. The public Python surface for layer also accepts PluginConfig, so add a second case for wrapper objects (or parametrize both forms) to catch Python-side conversion regressions that the core Rust tests will not see.
Suggested test shape
from nemo_relay import plugin
def test_layer_plugin_config_round_trips_merge():
# Smoke test only: merge semantics are covered by the core crate. This
# verifies the binding forwards both documents and returns merged JSON.
assert plugin.layer({"a": 1}, {"b": 2}) == {"a": 1, "b": 2}
+
+
+def test_layer_plugin_config_accepts_wrapper_types():
+ base = plugin.PluginConfig(
+ components=[plugin.ComponentSpec("example", config={"a": 1})],
+ )
+ overlay = plugin.PluginConfig(
+ components=[plugin.ComponentSpec("example", config={"b": 2})],
+ )
+
+ layered = plugin.layer(base, overlay)
+ assert layered["components"][0]["config"] == {"a": 1, "b": 2}As per coding guidelines "{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."
🤖 Prompt for 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.
In `@python/tests/test_plugin_config.py` around lines 7 - 10, Update the
test_layer_plugin_config_round_trips_merge test to also exercise the typed
wrapper path: call plugin.layer with a PluginConfig instance (e.g.,
PluginConfig({"a": 1}) and PluginConfig({"b": 2}) or one PluginConfig and one
dict) in addition to the raw dict case and assert the returned merged JSON
equals {"a": 1, "b": 2}; use the PluginConfig symbol from the module so
Python-side conversion is exercised and consider parametrizing the test if you
prefer both forms tested succinctly.
willkill07
left a comment
willkill07
left a comment
There was a problem hiding this comment.
code-driven plugin configuration is accomplished through each language binding's plugin initialize function.
- I do not see the "base" layer being inferred automatically from user/project/global config (which was the entire point of layering).
- Providing per-language binding layering is nice, but I believe we can drop
--plugin-configoutright. Folks should be using the configuration files rather than ad-hoc. - The added/improved error output is good!
Signed-off-by: Zhongxuan Wang <daniewang@nvidia.com>
2 participants
Overview
Layers code-driven plugin configuration over materialized file-backed plugin configuration so callers can combine discovered
plugins.toml, inline[plugins].config, CLI overrides, request headers, and binding-level config builders deterministically.Details
layer_plugin_configto the Rust core plugin API with recursive object merging, scalar/array replacement, and top-level component upsert bykind.--plugin-configand request header config overlay the effective base.Quick functional check:
Expected output:
{ "version": 1, "components": [ { "kind": "observability", "enabled": False, "config": { "atof": { "enabled": True, "filename": "base.jsonl", "mode": "overwrite", } }, } ], }Validation: full Rust, Python, Node.js, Go, WebAssembly, docs,
cargo fmt,cargo clippy --workspace --all-targets -- -D warnings, anduv run pre-commit run --all-filespassed. Focused reruns covered core layering, CLI layer precedence, the realnemo-relay run --dry-runbinary path, and the Python/Node.js/Go/FFI/WebAssembly wrapper paths.Where should the reviewer start?
Start with
crates/core/src/plugin.rsfor the shared layering semantics, thencrates/cli/src/config.rsfor source precedence and CLI/header composition.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
Release Notes
New Features
plugins.tomlfiles in standard locations.--dry-runoutput to display the active plugin config source.Breaking Changes
--plugin-configCLI flag andx-nemo-relay-plugin-configheader.Documentation