fix: route Relay headers through ChatNVIDIA transport#464
Conversation
Signed-off-by: Will Killian <wkillian@nvidia.com>
github-actions
Bot
added
size:M
|
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 (2)
📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used📓 Path-based instructions (15)**/*.{rs,py}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{rs,py,js,mjs,cjs,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
python/nemo_relay/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.py📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{rs,py,go,js,ts,c,h}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Files:
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Files:
{crates/**/src/**/*.rs,python/**/*.py}📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Files:
**/*.{py,go,js,ts}📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Files:
**/*📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{rs,py,go,js,ts}📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
Files:
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Files:
python/nemo_relay/**/*⚙️ CodeRabbit configuration file
Files:
python/tests/**/*.py📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
Files:
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (2)
WalkthroughChangesChatNVIDIA Relay Header Transport
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant payload_to_model_request
participant _chat_nvidia_with_relay_headers
participant ChatNVIDIA
payload_to_model_request->>_chat_nvidia_with_relay_headers: relay headers and model
_chat_nvidia_with_relay_headers->>ChatNVIDIA: create copied model with default_headers
ChatNVIDIA-->>payload_to_model_request: copied model or no match
payload_to_model_request->>payload_to_model_request: preserve headers in extra_headers for generic models
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 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 `@python/nemo_relay/integrations/langchain/_serialization.py`:
- Around line 248-252: The default header merge in the LangChain serialization
flow must be case-insensitive: update the header merge around model_copy to
remove existing model.default_headers entries whose lowercase names match
incoming Relay headers, then merge Relay headers so they take precedence. Add a
mixed-case regression test in
python/tests/integrations/langchain_tests/test_middleware.py covering provider
and Relay headers with different casing.
🪄 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: f23eebd8-5bef-43c7-bc12-eda262c12acf
📒 Files selected for processing (3)
docs/supported-integrations/langchain.mdxpython/nemo_relay/integrations/langchain/_serialization.pypython/tests/integrations/langchain_tests/test_middleware.py
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: Check / Run
- GitHub Check: Preview docs
🧰 Additional context used
📓 Path-based instructions (21)
**/*.mdx
📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md)
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)
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
Files:
docs/supported-integrations/langchain.mdx
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Update
README.md,fern/, package READMEs, and binding-support notes when public behavior, package names, examples, or supported bindings change.
**/*.{md,mdx}: Prefer the documented public API, not internal shortcuts
Keep package names, repo references, and build commands current
Keep release-process and release-notes guidance in repo-maintainer docs such asRELEASING.md, not as user-facing docs pages orCHANGELOG.md
Keep stable user-facing wrappers atscripts/root in docs and examples; only point at namespaced helper paths when documenting internal maintenance work
When detailed dynamic plugin guides exist, keep Rust native plugin examples, Python worker plugin examples, andgrpc-v1protocol details on separate pagesIf links in documentation change, run
just docs-linkcheck.
Files:
docs/supported-integrations/langchain.mdx
**/*.{md,markdown,mdx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Add the SPDX license header to all Markdown/MDX documentation files using the HTML comment block form.
Files:
docs/supported-integrations/langchain.mdx
{docs,examples}/**/*
📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)
Update docs and examples.
Files:
docs/supported-integrations/langchain.mdx
**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
**/*: Format changed files with the language-native formatter before the final lint/test pass.
If dynamic plugin behavior changed, usemaintain-dynamic-pluginsand include the native SDK, worker protocol, Python SDK, docs, packaging, and Codecov surfaces in the validation plan.
If code changes alter APIs, bindings, commands, paths, packaging behavior, observability/adaptive semantics, or documented best practices, update any dependent maintainer or consumer skills in the same branch.
During iteration, preferuv run pre-commit run --files <changed files...>.
Before review or handoff, runuv run pre-commit run --all-files.
Files:
docs/supported-integrations/langchain.mdxpython/nemo_relay/integrations/langchain/_serialization.pypython/tests/integrations/langchain_tests/test_middleware.py
docs/**/*
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If documentation examples or commands under
docs/change, run the targeted docs checks appropriate to the change.
Files:
docs/supported-integrations/langchain.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/supported-integrations/langchain.mdx
**/*.{rs,py}
📄 CodeRabbit inference engine (AGENTS.md)
Follow binding naming conventions in Rust and Python: use
snake_case.
Files:
python/nemo_relay/integrations/langchain/_serialization.pypython/tests/integrations/langchain_tests/test_middleware.py
**/*.{rs,py,js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{rs,py,js,mjs,cjs,ts,tsx}: UseJson = serde_json::Valuein Rust-facing runtime APIs where the existing code expects JSON payloads.
UseResult<T>withFlowErrorin core runtime paths, and keep errors explicit and binding-appropriate at the wrapper layer.
Keep async behavior on the existing tokio-based model; bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Files:
python/nemo_relay/integrations/langchain/_serialization.pypython/tests/integrations/langchain_tests/test_middleware.py
python/nemo_relay/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Python wrapper modules live under
python/nemo_relay/, and the native extension is built fromcrates/pythonwithmaturin.
Files:
python/nemo_relay/integrations/langchain/_serialization.py
**/*.py
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.py: When changing the Python wrapper package, tests, or docs tooling, lint with Ruff (E,F,W,I), format with Ruff formatter (120-character lines, double quotes), and passtytype checking.
Add the SPDX license header to all Python source files using the#comment form.
Files:
python/nemo_relay/integrations/langchain/_serialization.pypython/tests/integrations/langchain_tests/test_middleware.py
**/*.{rs,py,go,js,ts,c,h}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Use language-appropriate naming conventions: Rust
snake_case, C FFI exports prefixednemo_relay_, GoPascalCase, Node.jscamelCase, and Pythonsnake_case.
Files:
python/nemo_relay/integrations/langchain/_serialization.pypython/tests/integrations/langchain_tests/test_middleware.py
{crates/python/src/py_api/mod.rs,python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go,crates/node/src/api/**/*.rs}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update the language-native bindings for every exposed surface in Python, Go, and Node.js.
Files:
python/nemo_relay/integrations/langchain/_serialization.py
{python/nemo_relay/**/*.py,python/nemo_relay/**/*.pyi,go/nemo_relay/**/*.go}
📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)
Update language wrapper helpers such as Python wrapper modules, Python type stubs, and Go shorthand packages when the new behavior belongs in those helper layers.
Files:
python/nemo_relay/integrations/langchain/_serialization.py
{crates/**/src/**/*.rs,python/**/*.py}
📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md)
Do not add tests under
src; Rust tests belong in cratetests/trees, and Python SDK tests belong underpython/tests.
Files:
python/nemo_relay/integrations/langchain/_serialization.pypython/tests/integrations/langchain_tests/test_middleware.py
**/*.{py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md)
Keep Python, Go, and Node.js config objects and subscriber/exporter methods aligned so all bindings expose the same logical knobs and semantics.
Files:
python/nemo_relay/integrations/langchain/_serialization.pypython/tests/integrations/langchain_tests/test_middleware.py
**/*.{rs,py,go,js,ts}
📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)
If a language surface changed, always run that language's test target even when Rust core did not change.
Files:
python/nemo_relay/integrations/langchain/_serialization.pypython/tests/integrations/langchain_tests/test_middleware.py
**/*.{rs,py,js,ts,tsx,go,java,kt,swift}
📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)
Add tests covering registration and duplicate names, deregistration and missing names, priority ordering, callback failure policy, scope-local inheritance and cleanup, event payload semantics, immutable mark and scope fields, and parity across affected bindings.
Files:
python/nemo_relay/integrations/langchain/_serialization.pypython/tests/integrations/langchain_tests/test_middleware.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/integrations/langchain/_serialization.py
python/tests/**/*.py
📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md)
python/tests/**/*.py: Pytest is used to run tests.
Do not add@pytest.mark.asyncioto any test; async tests are automatically detected and run by the async runner.
Do not add a-> Nonereturn type annotation to test functions.
When mocking a class, do not define a new class; useunittest.mock.MagicMockorunittest.mock.AsyncMock, with thespecconstructor argument when necessary.
Name mocked classes with themockprefix, notfake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; if a fixture is needed in multiple test files, place it in aconftest.pyfile.
When creating a fixture, use@pytest.fixture(name="<fixture_name>"[, scope="<scope>"])and define the fixture function asdef <fixture_name>_fixture() -> <return_type>:; only specifyscopewhen it is notfunction.
Preferpytest.mark.parametrizeover creating individual tests for different input types.
Files:
python/tests/integrations/langchain_tests/test_middleware.py
{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:
python/tests/integrations/langchain_tests/test_middleware.py
🪛 ast-grep (0.44.1)
python/nemo_relay/integrations/langchain/_serialization.py
[info] 248-248: use jsonify instead of json.dumps for JSON output
Context: json.dumps(value, separators=(",", ":"))
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
🪛 Ruff (0.15.21)
python/nemo_relay/integrations/langchain/_serialization.py
[warning] 238-238: Dynamically typed expressions (typing.Any) are disallowed in model
(ANN401)
[warning] 238-238: Dynamically typed expressions (typing.Any) are disallowed in _chat_nvidia_with_relay_headers
(ANN401)
🔇 Additional comments (1)
docs/supported-integrations/langchain.mdx (1)
91-93: LGTM!
mnajafian-nv
left a comment
mnajafian-nv
left a comment
There was a problem hiding this comment.
Looks good overall. Conditional approval pending resolution of CodeRabbit’s case-insensitive header-merge comment.
Signed-off-by: Will Killian <wkillian@nvidia.com>
2 participants
Overview
Route Relay request headers for
ChatNVIDIAthrough its HTTP transport so they are not sent as unsupported NIM request-body fields.Details
ChatNVIDIAper request with Relay headers merged intodefault_headers, preserving provider headers while allowing Relay values to take precedence.extra_headerspath.Where should the reviewer start?
Start with
python/nemo_relay/integrations/langchain/_serialization.py, then review the focused cases inpython/tests/integrations/langchain_tests/test_middleware.py.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
Summary by CodeRabbit
ChatNVIDIAintegration now forwards NeMo Relay request headers via HTTP transport.ChatNVIDIAmodel when possible; otherwise it uses model settings.