feat(llm): add LangChain adapter and framework registry#1759
feat(llm): add LangChain adapter and framework registry#1759
Conversation
Greptile SummaryThis PR introduces a
|
| Filename | Overview |
|---|---|
| nemoguardrails/integrations/langchain/llm_adapter.py | New adapter wrapping LangChain BaseChatModel behind LLMModel protocol; streaming path omits additional_kwargs from provider_metadata, and _extract_reasoning's content_blocks branch is dead code with wrong keys. |
| nemoguardrails/integrations/langchain/message_utils.py | Message conversion utilities updated; chatmessage_to_langchain_message now raises ValueError for unsupported roles instead of silently falling back. |
| nemoguardrails/llm/frameworks.py | New framework registry with lazy LangChain registration; set_default_framework now validates against known/registered frameworks before accepting. |
| tests/test_langchain_llm_adapter.py | Comprehensive adapter tests covering generate, stream, reasoning, tool calls, usage, and message conversion; good parametric coverage of reasoning-model filtering. |
| tests/llm/test_frameworks.py | Framework registry tests cover registration, lazy loading, default framework, env-var override, and reset; all edge cases well covered. |
Sequence Diagram
sequenceDiagram
participant Caller
participant LangChainLLMAdapter
participant LangChainFramework
participant BaseChatModel
Caller->>LangChainFramework: create_model(model_name, provider_name, model_kwargs)
LangChainFramework->>BaseChatModel: init_langchain_model(...)
LangChainFramework-->>Caller: LangChainLLMAdapter(raw_llm)
Caller->>LangChainLLMAdapter: generate(prompt, **kwargs)
LangChainLLMAdapter->>LangChainLLMAdapter: _filter_reasoning_model_params(kwargs)
LangChainLLMAdapter->>BaseChatModel: llm.bind(**kwargs).ainvoke(messages, stop)
BaseChatModel-->>LangChainLLMAdapter: AIMessage
LangChainLLMAdapter->>LangChainLLMAdapter: _langchain_response_to_llm_response(response)
LangChainLLMAdapter-->>Caller: LLMResponse
Caller->>LangChainLLMAdapter: stream(prompt, **kwargs)
LangChainLLMAdapter->>BaseChatModel: llm.bind(**kwargs).astream(messages, stop)
loop each chunk
BaseChatModel-->>LangChainLLMAdapter: AIMessageChunk
LangChainLLMAdapter->>LangChainLLMAdapter: _langchain_chunk_to_llm_response_chunk(chunk)
LangChainLLMAdapter-->>Caller: LLMResponseChunk
end
Prompt To Fix All With AI
This is a comment left during a code review.
Path: nemoguardrails/integrations/langchain/llm_adapter.py
Line: 386-407
Comment:
**`additional_kwargs` silently dropped from streaming `provider_metadata`**
The non-streaming path explicitly passes `additional_kwargs` to `_build_provider_metadata`, but the streaming path does not extract or forward `additional_kwargs` at all. Any provider-specific metadata carried in `chunk.additional_kwargs` (other than `reasoning_content`, which is handled by `_extract_reasoning`) is silently discarded on every streamed response, while the same data would be surfaced in `provider_metadata` for a non-streaming call.
```python
# streaming fix
additional_kwargs = getattr(chunk, "additional_kwargs", None) or {}
return LLMResponseChunk(
...
provider_metadata=_build_provider_metadata(merged_metadata, additional_kwargs) or None,
)
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: nemoguardrails/integrations/langchain/llm_adapter.py
Line: 283-298
Comment:
**Dead code: `content_blocks` path uses wrong attribute and wrong keys**
`content_blocks` is not an attribute on any standard LangChain `AIMessage`/`AIMessageChunk`, so `getattr(response, "content_blocks", None)` always returns `None` and the entire first branch is never entered. Additionally, even if a custom class did expose `content_blocks`, the code checks `block.get("type") == "reasoning"` and reads `block.get("reasoning")` — but Anthropic's extended-thinking format uses `"thinking"` for both the type and content key. The working path is the `additional_kwargs["reasoning_content"]` fallback below.
Consider removing the dead branch, or fix the keys if future Anthropic thinking-block support is intended:
```python
for block in content_blocks:
if isinstance(block, dict) and block.get("type") == "thinking":
val = block.get("thinking")
if val:
return val
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (6): Last reviewed commit: "apply review suggestions" | Re-trigger Greptile
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
7cac41e to
3982772
Compare
525f8ee to
cb77d85
Compare
tgasser-nv
left a comment
There was a problem hiding this comment.
Looks good, please check the comments. Also can you review the code coverage and add more unit-tests to get the patch coverage up to 100%?
tgasser-nv
left a comment
There was a problem hiding this comment.
Added feedback to be addressed before merging
Add LangChainLLMAdapter wrapping BaseChatModel behind the LLMModel protocol, LangChainFramework implementing create_model(), and a pluggable framework registry. Pure additive, no existing files modified. Part of the LangChain decoupling epic feat(llm): implement provider_url on LangChainLLMAdapter Extract endpoint URL from the underlying LangChain LLM by checking common base URL attributes and the nested client object.
- Raise ValueError for unsupported ChatMessage roles - Return "community" for 2-segment langchain_community paths - Log debug when temperature is stripped for reasoning models - Validate set_default_framework against known frameworks - Fall back to input+output for missing total_tokens - Use builtin dict in isinstance check
Extract _extract_tool_calls, _extract_usage, _extract_model_info, _build_provider_metadata from the response/chunk converters. Remove dead else branch in tool call extraction (LangChain tool_calls is always List[dict]). Type the response parameters. s
cb77d85 to
a1814b3
Compare
📝 WalkthroughWalkthroughAdds LangChain integration to the system via a framework registry, LLM adapter with request/response conversion, message utilities for translating between nemoguardrails and LangChain message formats, and comprehensive test coverage. The implementation includes inference helpers, streaming support, and response normalization across multiple metadata fields. Changes
Sequence DiagramsequenceDiagram
participant Client
participant LangChainLLMAdapter
participant LangChain as LangChain LLM
participant Response as LangChain Response
Client->>LangChainLLMAdapter: generate(prompt, stop=...)
LangChainLLMAdapter->>LangChainLLMAdapter: _prepare_llm (filter params)
LangChainLLMAdapter->>LangChainLLMAdapter: chatmessages_to_langchain_messages
LangChainLLMAdapter->>LangChain: ainvoke(messages, stop=...)
LangChain->>Response: generate response
Response-->>LangChainLLMAdapter: AIMessage with content/tool_calls
LangChainLLMAdapter->>LangChainLLMAdapter: _langchain_response_to_llm_response
LangChainLLMAdapter->>LangChainLLMAdapter: _extract_tool_calls
LangChainLLMAdapter->>LangChainLLMAdapter: _extract_usage
LangChainLLMAdapter->>LangChainLLMAdapter: _build_provider_metadata
LangChainLLMAdapter-->>Client: LLMResponse
Client->>LangChainLLMAdapter: stream(prompt, stop=...)
LangChainLLMAdapter->>LangChain: astream(messages, stop=...)
loop for each chunk
LangChain-->>LangChainLLMAdapter: AIMessageChunk
LangChainLLMAdapter->>LangChainLLMAdapter: _langchain_chunk_to_llm_response_chunk
LangChainLLMAdapter->>LangChainLLMAdapter: _extract_usage
LangChainLLMAdapter-->>Client: LLMResponseChunk
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
tests/test_langchain_llm_adapter.py (1)
92-104:⚠️ Potential issue | 🟡 MinorAdd direct unit tests for the new
message_utilshelpers.This only proves that one converted message reached
ainvoke; it doesn't verify role mapping,ToolMessage.tool_call_id, or assistanttool_calls. Sincechatmessage_to_langchain_message()andchatmessages_to_langchain_messages()are standalone helpers now, they need direct coverage too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_langchain_llm_adapter.py` around lines 92 - 104, Add direct unit tests for the new message_utils helpers: write tests that call chatmessage_to_langchain_message() and chatmessages_to_langchain_messages() and assert correct role mapping (user/system/assistant), that ToolMessage instances preserve tool_call_id, and that assistant messages include tool_calls information where appropriate; update tests to construct ChatMessage/ToolMessage cases (including an assistant message with tool_calls and a ToolMessage with tool_call_id) and assert the resulting LangChain message objects contain the expected role, metadata.tool_call_id, and tool_calls fields rather than only verifying that adapter.generate() forwarded one message to ainvoke.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nemoguardrails/integrations/langchain/llm_adapter.py`:
- Around line 386-394: The returned LLMResponseChunk currently only sets
provider_metadata with merged_metadata, so normalized fields used by stream
consumers are missing; extract keys like "model", "finish_reason" or
"stop_reason" (map to LLMResponseChunk.finish_reason), and "request_id" from
merged_metadata (or from response_metadata/generation_info) and populate the
corresponding LLMResponseChunk attributes when constructing the chunk in
llm_adapter.py (the same block that calls _extract_usage and returns
LLMResponseChunk); ensure you preserve provider_metadata as merged_metadata or
None while also assigning the normalized fields.
In `@nemoguardrails/llm/frameworks.py`:
- Around line 22-23: The file currently assigns _default_framework directly from
os.environ which can bypass the validation performed by set_default_framework(),
leading to a value get_default_framework() returns that get_framework() cannot
load; change the initialization to validate the environment value using the same
validation path (call
set_default_framework(os.environ.get("NEMOGUARDRAILS_LLM_FRAMEWORK",
"langchain")) or run the same validation function) so a bad
NEMOGUARDRAILS_LLM_FRAMEWORK raises immediately; apply the same change to the
other direct assignments referenced (lines around the second occurrence) to
ensure all initializations use set_default_framework() or the shared validator
instead of direct assignment to _default_framework.
In `@tests/llm/test_frameworks.py`:
- Around line 30-33: The fixture clean_registry currently only resets the module
registry after each test; call _reset_frameworks() before yielding as well so
the registry is cleared on setup and teardown for every test—i.e., in the
clean_registry fixture invoke _reset_frameworks() before yield and keep the
existing _reset_frameworks() after yield to ensure tests don't inherit global
framework state; reference the clean_registry fixture and the
_reset_frameworks() helper when making this change.
---
Duplicate comments:
In `@tests/test_langchain_llm_adapter.py`:
- Around line 92-104: Add direct unit tests for the new message_utils helpers:
write tests that call chatmessage_to_langchain_message() and
chatmessages_to_langchain_messages() and assert correct role mapping
(user/system/assistant), that ToolMessage instances preserve tool_call_id, and
that assistant messages include tool_calls information where appropriate; update
tests to construct ChatMessage/ToolMessage cases (including an assistant message
with tool_calls and a ToolMessage with tool_call_id) and assert the resulting
LangChain message objects contain the expected role, metadata.tool_call_id, and
tool_calls fields rather than only verifying that adapter.generate() forwarded
one message to ainvoke.
🪄 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: CHILL
Plan: Pro
Run ID: 7883703e-90ee-4a46-9d04-6810bb322475
📒 Files selected for processing (5)
nemoguardrails/integrations/langchain/llm_adapter.pynemoguardrails/integrations/langchain/message_utils.pynemoguardrails/llm/frameworks.pytests/llm/test_frameworks.pytests/test_langchain_llm_adapter.py
|
Thanks @tgasser-nv . I addressed all the comments 👍🏻 |
Part of the LangChain decoupling stack:
Description
LangChainLLMAdapterwrappingBaseChatModelbehindLLMModelprotocolLangChainFrameworkimplementingcreate_model()register_framework()/get_framework()/set_default_framework()chatmessage_to_langchain_message()andchatmessages_to_langchain_messages()conversion utilsprovider_urlextracts endpoint from LangChain LLM attributesPure additive. No existing files modified except
message_utils.py.Summary by CodeRabbit
Release Notes
New Features
Tests