Skip to content

Conversation

@Pouyanpi
Copy link
Collaborator

@Pouyanpi Pouyanpi commented Oct 24, 2025

PR Description

Summary

Adds full LangChain 1.x support while maintaining backward compatibility with 0.x versions. This PR migrates all imports to use canonical langchain-core paths, removes deprecated features (Chain support, LLMParams, SummarizeDocument), and provides documentation updates.

BREAKING CHANGES:

  • Chain support removed from action dispatcher (use Runnable instead)
  • SummarizeDocument built-in action removed (implement custom version)
  • langchain-nvidia-ai-endpoints removed from optional dependencies

Key Changes

Dependency Updates

  • Extended langchain constraint from <0.4.0 to <2.0.0
  • Extended langchain-core constraint from <0.4.0 to <2.0.0
  • Extended langchain-community constraint from <0.4.0 to <2.0.0

Import Standardization (28 files)

Migrated all imports to canonical paths:

# before (deprecated)
from langchain.chat_models.base import BaseChatModel
from langchain_core.language_models.llms import BaseLLM

# after (correct)
from langchain_core.language_models import BaseChatModel, BaseLLM

Removed Deprecated Features

  • Chain support : Removed from action_dispatcher.py (use Runnable instead)
  • LLMParams : Removed context manager (use native LLM params)
  • SummarizeDocument : Removed built-in action (implement custom)
  • langchain-nvidia-ai-endpoints : Removed from optional deps (install manually untill it supports v1)

Documentation Updates

  • Completely rewrote custom LLM provider guide
  • Added BaseChatModel documentation (was missing!)
  • Added migration guide for Chain → Runnable
  • update CHANGELOG.md with breaking changes

Backward Compatibility

Examples include fallback pattern for LangChain 1.x users:

try:
    from langchain.chains import RetrievalQA
except ImportError:
    from langchain_classic.chains import RetrievalQA

@github-actions
Copy link
Contributor

Documentation preview

https://nvidia-nemo.github.io/Guardrails/review/pr-1472

@codecov-commenter
Copy link

codecov-commenter commented Oct 24, 2025

Codecov Report

❌ Patch coverage is 67.30769% with 17 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
nemoguardrails/evaluate/evaluate_factcheck.py 11.11% 8 Missing ⚠️
nemoguardrails/library/hallucination/actions.py 33.33% 4 Missing ⚠️
...moguardrails/llm/providers/huggingface/pipeline.py 0.00% 3 Missing ⚠️
nemoguardrails/llm/helpers.py 0.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@Pouyanpi
Copy link
Collaborator Author

@greptileai

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

This PR migrates the NeMo Guardrails codebase from LangChain v0.x to support LangChain v1.x, which introduced breaking changes through package reorganization (langchain_core, langchain_community) and API redesigns. The key changes include: (1) updating imports from deprecated langchain.* paths to langchain_core.* equivalents across 30+ files, (2) removing the custom LLMParams context manager in favor of LangChain v1's .bind() pattern for parameter passing, (3) eliminating special handling for the deprecated Chain class (now superseded by the Runnable interface), (4) migrating from LLMChain to direct LLM invocation patterns, (5) expanding dependency constraints to allow langchain versions <2.0.0, (6) adding backward-compatibility import fallbacks for langchain_classic in example configs. The migration maintains core guardrails functionality while aligning with LangChain's new compositional architecture.

Important Files Changed

Filename Score Overview
pyproject.toml 5/5 Expanded langchain dependency constraints from <0.4.0 to <2.0.0 to support v1.x releases
nemoguardrails/llm/params.py 4/5 Deleted the entire LLMParams context manager system replaced by langchain v1's .bind() method
nemoguardrails/actions/action_dispatcher.py 4/5 Removed deprecated Chain execution logic, now only supports Runnable interface
nemoguardrails/colang/v2_x/runtime/runtime.py 4/5 Removed special handling for Chain objects in action execution, treating all actions uniformly
nemoguardrails/colang/v1_0/runtime/runtime.py 4/5 Eliminated Chain-specific parameter extraction and action server routing logic
nemoguardrails/actions/summarize_document.py 4/5 Deleted entire file removing document summarization action that depended on deprecated APIs
nemoguardrails/llm/models/langchain_initializer.py 4/5 Commented out deprecation warning suppression and improved ImportError tracking
nemoguardrails/library/hallucination/actions.py 3/5 Migrated from LLMChain to .bind() and .agenerate() but may have parameter validation issues
nemoguardrails/evaluate/evaluate_factcheck.py 2/5 Replaced LLMChain with direct invocation but likely broken—calls .strip() on message object instead of .content
nemoguardrails/llm/providers/huggingface/pipeline.py 1/5 Updated imports but missing asyncio import causes runtime error on line 89
tests/test_streaming.py 2/5 Added streaming validation tests but fixtures use deprecated langchain v0.x imports that will fail
tests/test_streaming_output_rails.py 2/5 Refactored to enforce streaming errors but fixture has inconsistent config (streaming enabled, tests expect disabled)
tests/test_parallel_streaming_output_rails.py 3/5 Modified test to verify error raising when streaming config missing for output rails
tests/test_configs/with_custom_chat_model/custom_chat_model.py 2/5 Updated callback imports but BaseChatModel still uses deprecated langchain.chat_models.base path
tests/runnable_rails/test_metadata.py 3/5 Callback imports migrated but BaseChatModel import uses deprecated path instead of langchain_core
nemoguardrails/rails/llm/llmrails.py 5/5 Added validation to prevent stream_async() with output rails when streaming disabled
nemoguardrails/streaming.py 5/5 Migrated callback and message imports from langchain.schema to langchain_core
nemoguardrails/llm/helpers.py 5/5 Updated callback manager imports to langchain_core with no logic changes
nemoguardrails/llm/providers/providers.py 5/5 Changed BaseLLM import path from nested module to direct langchain_core export
nemoguardrails/llm/providers/trtllm/llm.py 5/5 Updated callback manager import to langchain_core
nemoguardrails/llm/filters.py 4/5 Removed 'summarize_document' from system actions filter list
nemoguardrails/logging/callbacks.py 4/5 Migrated imports and updated token usage field names to v1 standard (input_tokens/output_tokens)
nemoguardrails/actions/llm/utils.py 5/5 Migrated BaseLanguageModel and callback imports to langchain_core
tests/test_llm_params.py 5/5 Deleted entire test file for deprecated LLMParams context manager
tests/test_configs/with_custom_llm/custom_llm.py 4/5 Migrated to langchain_core and implemented previously stubbed methods
tests/rails/llm/test_config.py 5/5 Updated BaseLLM import path for langchain v1 compatibility
tests/test_callbacks.py 5/5 Migrated Generation and LLMResult imports from deprecated langchain.schema
tests/test_streaming_handler.py 5/5 Updated chunk and message imports to langchain_core structure
tests/utils.py 5/5 Migrated callback and output type imports to langchain_core
examples/configs/rag/custom_rag_output_rails/config.py 5/5 Updated PromptTemplate import to langchain_core
examples/configs/rag/multi_kb/tabular_llm.py 5/5 Migrated imports and changed base class from LLM to BaseLLM
examples/configs/rag/multi_kb/config.py 5/5 Added backward-compatible import fallback for langchain/langchain_classic
examples/configs/rag/pinecone/config.py 5/5 Added import fallback with helpful error message for langchain_classic
examples/scripts/langchain/experiments.py 5/5 Added nested import fallback for LLMMathChain with langchain_classic support
docs/user-guides/python-api.md 5/5 Added deprecation notice for Chain support directing users to Runnable

Confidence score: 2/5

  • This PR has multiple critical issues that will cause runtime errors and test failures immediately in certain scenarios
  • Score reduced due to: (1) missing asyncio import in huggingface pipeline.py causing NameError, (2) factcheck evaluation calling .strip() on message object instead of .content, (3) test fixtures using deprecated v0.x imports that will fail, (4) inconsistent test configurations expecting errors for "disabled streaming" but using configs with streaming: True, (5) incomplete migration where some test files still import BaseChatModel from deprecated paths
  • Pay close attention to nemoguardrails/llm/providers/huggingface/pipeline.py (missing asyncio), nemoguardrails/evaluate/evaluate_factcheck.py (broken response handling), tests/test_streaming.py (deprecated imports), tests/test_streaming_output_rails.py (inconsistent fixture), and all test files importing from langchain.chat_models.base

Sequence Diagram

sequenceDiagram
    participant User
    participant RunnableRails
    participant LLMRails
    participant ModelInitializer
    participant LangChainModel
    participant StreamingHandler
    
    User->>RunnableRails: invoke(prompt/messages)
    RunnableRails->>RunnableRails: _prepare_input()
    RunnableRails->>LLMRails: generate_async(messages)
    
    alt Model Not Initialized
        LLMRails->>ModelInitializer: init_llm_model(model_name, provider, mode)
        ModelInitializer->>ModelInitializer: try_initialization_method(initializers)
        
        alt Special Case Model
            ModelInitializer->>ModelInitializer: _handle_model_special_cases()
        else Chat Completion
            ModelInitializer->>ModelInitializer: _init_chat_completion_model()
            ModelInitializer->>LangChainModel: init_chat_model(model, provider)
        else Community Chat
            ModelInitializer->>ModelInitializer: _init_community_chat_models()
        else Text Completion
            ModelInitializer->>ModelInitializer: _init_text_completion_model()
        end
        
        ModelInitializer-->>LLMRails: initialized_model
        LLMRails->>LLMRails: _configure_main_llm_streaming(llm)
    end
    
    LLMRails->>LLMRails: _get_events_for_messages(messages)
    
    alt Streaming Enabled
        LLMRails->>StreamingHandler: create StreamingHandler
        LLMRails->>LangChainModel: ainvoke(messages, streaming=True)
        
        loop Token Streaming
            LangChainModel->>StreamingHandler: on_llm_new_token(token, chunk)
            StreamingHandler->>StreamingHandler: push_chunk(chunk, generation_info)
            StreamingHandler-->>User: yield chunk
        end
        
        LangChainModel->>StreamingHandler: on_llm_end(response)
        StreamingHandler->>StreamingHandler: extract metadata
        StreamingHandler->>StreamingHandler: push_chunk(END_OF_STREAM)
    else Non-Streaming
        LLMRails->>LangChainModel: ainvoke(messages)
        LangChainModel-->>LLMRails: response with metadata
    end
    
    LLMRails->>LLMRails: extract_tool_calls_from_events()
    LLMRails->>LLMRails: get_and_clear_response_metadata()
    LLMRails->>LLMRails: build GenerationResponse
    
    alt With Metadata Preservation
        LLMRails->>LLMRails: include response_metadata
        LLMRails->>LLMRails: include usage_metadata
        LLMRails->>LLMRails: include tool_calls
    end
    
    LLMRails-->>RunnableRails: GenerationResponse
    RunnableRails->>RunnableRails: _convert_to_ai_message(response)
    RunnableRails->>RunnableRails: restore_metadata_to_message()
    RunnableRails-->>User: AIMessage with metadata
Loading

Additional Comments (6)

  1. tests/runnable_rails/test_runnable_rails.py, line 664-709 (link)

    style: Test uses both unittest.mock.Mock (lines 685, 698) and FakeListLLM (line 670) but imports Mock inline at line 685. Consider importing Mock at the top with other imports (line 16-35) for consistency.

  2. nemoguardrails/llm/providers/huggingface/pipeline.py, line 89 (link)

    syntax: Missing import for asyncio module required by asyncio.get_running_loop() on this line

  3. tests/test_streaming.py, line 575-576 (link)

    syntax: Import uses deprecated langchain.chat_models.base which was removed in LangChain v1. Use langchain_core.language_models.chat_models instead:

  4. tests/test_streaming.py, line 586 (link)

    style: The streaming attribute should be removed or set via constructor. LangChain v1 BaseModels handle streaming via method returns and callbacks, not class attributes. Should these test classes follow the LangChain v1 streaming pattern (implementing _stream / _astream methods) or is this simplified version sufficient for testing?

  5. tests/test_streaming.py, line 614 (link)

    style: The streaming attribute should be removed. In LangChain v1, streaming is controlled by implementing _stream and _astream methods, not via a class attribute.

  6. nemoguardrails/library/hallucination/actions.py, line 74-79 (link)

    style: Checking llm.__fields__ assumes the LLM is a Pydantic model. LangChain v1 may use different internal structures. Consider using hasattr() or checking model configuration methods instead.

36 files reviewed, 18 comments

Edit Code Review Agent Settings | Greptile

@Pouyanpi Pouyanpi force-pushed the feat/support-langchain-v1 branch from cd21b11 to c92ebfb Compare October 27, 2025 15:29
@Pouyanpi Pouyanpi added this to the v0.19.0 milestone Oct 27, 2025
@Pouyanpi Pouyanpi self-assigned this Oct 27, 2025
@Pouyanpi Pouyanpi changed the base branch from develop to chore/remove-deprecated-llm-params October 27, 2025 15:42
@Pouyanpi Pouyanpi force-pushed the feat/support-langchain-v1 branch from 856828b to 8b264c4 Compare October 27, 2025 15:44
@Pouyanpi
Copy link
Collaborator Author

Summary of ALL LangChain Imports in Codebase:

  1. langchain_core imports (All CORRECT)
    from langchain_core.language_models import BaseLLM, BaseChatModel, BaseLanguageModel, LLM
    from langchain_core.callbacks.manager import ...
    from langchain_core.messages import ...
    from langchain_core.outputs import ...
    from langchain_core.prompts import ...
    from langchain_core.runnables import ...
    from langchain_core.tools import ...
    All are canonical paths from langchain-core

  2. langchain_community imports (All CORRECT)
    from langchain_community import llms
    from langchain_community.cache import SQLiteCache
    from langchain_community.chat_models import _module_lookup
    from langchain_community.llms import HuggingFacePipeline
    All are correct community package imports

  3. langchain (Main Package) - (✓ CORRECT)
    from langchain.chat_models import init_chat_model
    Confirmed: init_chat_model STAYS in langchain.chat_models in v1.0

  • It's part of the "streamlined core namespace"
  • Provides essential model initialization
  1. langchain_nvidia_ai_endpoints (✓ CORRECT)
    from langchain_nvidia_ai_endpoints import ChatNVIDIA
    Separate integration package, correct

all import paths are correct and V1.0 compatible!

No additional changes needed. The codebase is:

  • Using canonical langchain-core paths
  • Using correct langchain_community paths
  • Using correct init_chat_model from main package (stays in v1.0)
  • No deprecated proxy imports remaining
  • Fully compatible with langchain 0.2.x, 0.3.x, and 1.x

The only import from the main langchain package (init_chat_model) is intentionally kept there in v1.0 as part of the core functionality. Everything else correctly imports from langchain-core or
langchain-community.

…put rails streaming (#1470)

* fix(streaming): raise error when stream_async used with disabled output rails streaming

When output rails are configured but output.streaming.enabled is False (or not set),
calling stream_async() would result in undefined behavior or hangs due to the conflict
between streaming expectations and blocking output rail processing.

This change adds explicit validation in stream_async() to detect this misconfiguration
and raise a clear ValueError with actionable guidance:
- Set rails.output.streaming.enabled = True to use streaming with output rails
- Use generate_async() instead for non-streaming with output rails

Updated affected tests to expect and validate the new error behavior instead of
relying on the previous buggy behavior.
@Pouyanpi Pouyanpi force-pushed the feat/support-langchain-v1 branch from a6e15ff to 1370ec5 Compare October 27, 2025 16:45
@Pouyanpi Pouyanpi marked this pull request as ready for review October 27, 2025 16:46
@Pouyanpi Pouyanpi force-pushed the feat/support-langchain-v1 branch 2 times, most recently from 8e0f3b9 to 98f7722 Compare October 27, 2025 16:48
Migrate all imports from deprecated proxy paths to canonical
langchain-core paths
to ensure compatibility with LangChain 1.x. Changes include:
- Use `from langchain_core.language_models import BaseLLM,
BaseChatModel`
- Remove proxy imports from `langchain.chat_models.base`
- Standardize submodule imports to top-level
langchain_core.language_models

This ensures forward compatibility with LangChain 1.x where proxy
imports from the main langchain package will be removed.
…patcher

Remove support for registering LangChain Chain objects as actions in
favor of
the modern Runnable interface. Chain support is deprecated in LangChain
1.x
and users should migrate to using Runnable objects instead.

- Remove Chain handling logic from action_dispatcher.py
- Remove Chain-based tests from test_runnable_rails.py
- Add deprecation warning in python-api.md documentation
Remove the built-in SummarizeDocument action which relied on deprecated
LangChain Chain features. Users who need document summarization should
implement custom actions using LangChain Runnable chains.

- Delete nemoguardrails/actions/summarize_document.py
- Remove related import from llm/filters.py
Add try/except fallback patterns in examples to support both LangChain
0.x and 1.x. When using LangChain 1.x, legacy Chain features are
imported from langchain-classic package with helpful error messages.

This allows examples to work seamlessly across LangChain versions
without requiring code changes from users.

- Add fallback imports for RetrievalQA, embeddings, text splitters,
vectorstores
- Provide clear error messages directing users to install
langchain-classic
Update Colang runtime imports to use canonical langchain-core paths for
callbacks and runnables. Part of the broader migration to langchain-core
for LangChain 1.x compatibility.
… support

Complete rewrite of the custom LLM provider documentation with:
- Separate comprehensive guides for BaseLLM (text completion) and
BaseChatModel (chat)
- Correct method signatures (_call vs _generate)
- Proper async implementations
- Clear registration instructions (register_llm_provider vs
register_chat_provider)
- Working code examples with correct langchain-core imports
- Important notes on choosing the right base class

This addresses the gap where users were not properly guided on
implementing
custom chat models and were being directed to the wrong interface.
Extend LangChain dependency constraints to support both 0.x and 1.x versions:
- langchain: >=0.2.14,<0.4.0 → >=0.2.14,<2.0.0
- langchain-core: >=0.2.14,<0.4.0 → >=0.2.14,<2.0.0
- langchain-community: >=0.2.5,<0.4.0 → >=0.2.5,<2.0.0

Remove langchain-nvidia-ai-endpoints from optional dependencies as it
should be installed manually when needed.

fix
@Pouyanpi Pouyanpi force-pushed the feat/support-langchain-v1 branch from 98f7722 to 56cf606 Compare October 27, 2025 16:49
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Greptile Summary

This PR migrates NeMo Guardrails to support LangChain 1.x while maintaining 0.x backward compatibility. The changes standardize 28+ files to use canonical langchain_core import paths, remove deprecated Chain support from both Colang runtimes (v1_0 and v2_x), delete the LLMParams context manager and SummarizeDocument built-in action, and extend dependency constraints to <2.0.0. The migration follows a clear pattern: consolidate BaseChatModel and BaseLLM imports from langchain_core.language_models, update callback/output imports to langchain_core modules, and replace Chain-based patterns with direct LLM invocation using .bind(). Documentation was rewritten to distinguish text completion models from chat models and provide migration guidance. Example files add backward-compatible fallback imports to langchain_classic for Chain APIs.

Important Files Changed

Filename Score Overview
nemoguardrails/llm/providers/providers.py 2/5 Import migration incomplete - line 89 still imports from deprecated langchain.chat_models.base path instead of canonical langchain_core.chat_models
nemoguardrails/library/hallucination/actions.py 2/5 Migrated to direct LLM invocation with .bind(), but callbacks parameter type mismatch and Pydantic v1/v2 compatibility issue (model_fields vs __fields__)
tests/test_configs/with_custom_chat_model/custom_chat_model.py 1/5 Critical logic error - inherits from BaseChatModel but implements _call()/_acall() (BaseLLM methods) instead of _generate()/_stream()
pyproject.toml 3/5 Extended LangChain constraints to <2.0.0 (risky upper bound) and removed nvidia dependency group; protobuf pinned for security
nemoguardrails/evaluate/evaluate_factcheck.py 3/5 Migrated from LLMChain to direct invocation with .bind(), but max_tokens parameter and .content access may not work universally
nemoguardrails/llm/models/langchain_initializer.py 4/5 Import migration complete, but warning suppressions commented out (not removed) and ImportError in _init_nvidia_model not re-raised
examples/configs/rag/multi_kb/tabular_llm.py 4/5 Migrated imports but still uses deprecated langchain_core.language_models.llms.BaseLLM path instead of top-level import
tests/llm_providers/test_langchain_special_cases.py 4.5/5 Successful import migration with two minor docstring typos ('matche' on line 63, 'enure' on line 102)
tests/test_configs/with_custom_llm/custom_llm.py 4.5/5 Migrated to canonical imports and implemented required _generate()/_agenerate() methods for LangChain v1
examples/scripts/langchain/experiments.py 4.5/5 Added backward-compatible fallback imports for LLMMathChain with helpful error message
examples/configs/rag/custom_rag_output_rails/config.py 4/5 Migrated prompt imports but BaseLLM still uses deprecated sub-module path
docs/user-guides/configuration-guide/custom-initialization.md 4/5 Complete documentation rewrite for LangChain v1 with separate BaseLLM/BaseChatModel guidance
docs/user-guides/python-api.md 4/5 Added deprecation warning for Chain support, but relative path in link may be incorrect
examples/configs/rag/pinecone/config.py 4/5 Added fallback to langchain_classic, but line 39 uses deprecated import and continues using Chain APIs
tests/test_parallel_streaming_output_rails.py 3/5 Refactored test to validate error handling, but fixture may not reliably trigger expected error condition
nemoguardrails/colang/v1_0/runtime/runtime.py 4/5 Successfully removed Chain support from Colang v1.0 runtime and simplified action dispatcher logic
nemoguardrails/rails/llm/llmrails.py 5/5 Clean import migration and added streaming validation method for output rails
nemoguardrails/actions/llm/generation.py 5/5 Consolidated imports to canonical path in single line
nemoguardrails/llm/filters.py 5/5 Removed "summarize_document" from system_actions list to align with feature removal
nemoguardrails/llm/providers/huggingface/pipeline.py 5/5 All imports correctly updated to canonical langchain_core and langchain_community paths
All other files 5/5 Successful import path migration with no logic changes

Confidence score: 2/5

  • This PR contains multiple critical issues that will cause runtime failures: the with_custom_chat_model test fixture has a logic error that breaks the BaseChatModel contract, the providers registry has incomplete import migration, and several files have parameter compatibility issues with .bind() patterns
  • Score reduced due to: (1) critical logic error in custom_chat_model.py mixing BaseChatModel/BaseLLM interfaces, (2) incomplete import migration in providers.py, (3) Pydantic v1/v2 compatibility gap in hallucination/actions.py, (4) untested .bind() parameter support across multiple LLM providers, (5) risky <2.0.0 dependency upper bound without validation against LangChain 2.x breaking changes
  • Pay close attention to tests/test_configs/with_custom_chat_model/custom_chat_model.py, nemoguardrails/llm/providers/providers.py, nemoguardrails/library/hallucination/actions.py, and test the .bind() patterns in evaluate_factcheck.py with multiple LLM providers

Additional Comments (7)

  1. tests/runnable_rails/test_runnable_rails.py, line 664-710 (link)

    logic: Test mocks rails.generate() but invokes runnable_rails.invoke(), which calls rails.generate_async() under the hood. The sync mock will never be hit. Either mock generate_async or use asyncio.run(runnable_rails.ainvoke(...)) if a synchronous mock is required.

  2. nemoguardrails/llm/providers/providers.py, line 89 (link)

    syntax: importing from deprecated path langchain.chat_models.base. Use langchain_core.chat_models instead:

  3. examples/configs/rag/multi_kb/config.py, line 281-286 (link)

    logic: RetrievalQA.from_chain_type returns a Chain, which the PR description states is deprecated and removed. This code will break when users upgrade to LangChain 1.x without langchain-classic installed, contradicting the migration guide that says to use Runnable instead. Should this example demonstrate the Runnable migration pattern, or is it acceptable to require langchain-classic for this legacy RAG example?

  4. tests/llm_providers/test_langchain_special_cases.py, line 63 (link)

    syntax: Typo: 'matche' should be 'matches'

  5. tests/llm_providers/test_langchain_special_cases.py, line 102 (link)

    syntax: Typo: 'enure' should be 'ensure'

  6. nemoguardrails/llm/models/langchain_initializer.py, line 25-40 (link)

    style: Commented-out warning suppression imports and filterwarnings calls remain in the code. If these are no longer needed for LangChain v1, remove them entirely rather than leaving commented code.

  7. nemoguardrails/llm/models/langchain_initializer.py, line 351-355 (link)

    logic: Empty except block swallows the ImportError without re-raising it. The function should re-raise the custom ImportError message that was just created on line 352-354.

52 files reviewed, 19 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +26 to +40
from langchain.chains import RetrievalQA
from langchain.embeddings import HuggingFaceEmbeddings
from langchain.text_splitter import CharacterTextSplitter
from langchain.vectorstores import FAISS
except ImportError:
try:
from langchain_classic.chains import RetrievalQA
from langchain_classic.embeddings import HuggingFaceEmbeddings
from langchain_classic.text_splitter import CharacterTextSplitter
from langchain_classic.vectorstores import FAISS
except ImportError as e:
raise ImportError(
"Failed to import from langchain. If you're using LangChain >= 1.0.0, "
"please install langchain-classic: pip install langchain-classic"
) from e
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: The nested try-except blocks will shadow the original ImportError if langchain_classic also fails to import a different subset of classes. The error message on line 38-39 will incorrectly suggest installing langchain-classic even if RetrievalQA imports but HuggingFaceEmbeddings doesn't (for example). Use raise ... from None or re-raise the first ImportError.

AsyncCallbackManagerForLLMRun,
CallbackManagerForLLMRun,
)
from langchain_core.language_models import BaseLLM
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: Import BaseLLM from langchain_core.language_models (not langchain_core.language_models.llms) to match LangChain v1 canonical import paths used throughout the codebase

AsyncCallbackManagerForLLMRun,
CallbackManagerForLLMRun,
)
from langchain_core.language_models import BaseChatModel
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: Import BaseChatModel from langchain_core.language_models (not langchain_core.language_models.chat_models) to match LangChain v1 canonical import paths used throughout the codebase


### Chains as Actions

> **⚠️ DEPRECATED**: Chain support is deprecated and will be removed in a future release. Please use [Runnable](https://python.langchain.com/docs/expression_language/) instead. See the [Runnable as Action Guide](langchain/runnable-as-action/README.md) for examples.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Link to migration guide is incorrect - should be ../langchain/runnable-as-action/README.md (add ../ prefix) since this file is in user-guides/ and the target is in user-guides/langchain/runnable-as-action/

CallbackManagerForLLMRun,
)
from langchain.chat_models.base import BaseChatModel
from langchain_core.language_models import BaseChatModel
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: BaseChatModel requires implementing _generate() or _stream(), not _call(). The methods _call() (lines 26-32) and _acall() (lines 34-40) are BaseLLM methods. Either inherit from BaseLLM instead, or replace these with _generate() and _agenerate() for chat models. Is this test intended to verify a custom chat model or a custom LLM? The class name suggests chat model, but the implemented methods are for BaseLLM.

# limitations under the License.

from langchain.prompts import PromptTemplate
from langchain_core.language_models.llms import BaseLLM
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: import path for BaseLLM changed from langchain_core.language_models.llms to langchain_core.language_models in LangChain 1.x. Use from langchain_core.language_models import BaseLLM for consistency with other files in this PR

)

if "n" not in llm.__fields__:
if "n" not in llm.model_fields:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: model_fields is a Pydantic v2 attribute. LangChain v1 still supports Pydantic v1 models where the attribute is __fields__. This will break for Pydantic v1 users with AttributeError. Are all LangChain v1 LLMs guaranteed to use Pydantic v2, or should this use a fallback check like getattr(llm, 'model_fields', None) or getattr(llm, '__fields__', {})?

llm_with_config = llm.bind(temperature=1.0, n=num_responses)
extra_llm_response = await llm_with_config.agenerate(
[formatted_prompt],
callbacks=[logging_callback_manager_for_chain],
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: callbacks parameter expects a list of callback handlers, but logging_callback_manager_for_chain is a CallbackManager instance. Should pass [logging_callback_manager_for_chain] or use the manager's .handlers attribute.

Comment on lines +55 to +57
langchain = ">=0.2.14,<2.0.0"
langchain-core = ">=0.2.14,<2.0.0"
langchain-community = ">=0.2.5,<2.0.0"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Version constraint <2.0.0 is overly broad for a major migration. LangChain 2.x will introduce breaking changes that this codebase hasn't been tested against. Constrain to <1.0.0 or <1.x.0 to avoid unexpected runtime failures when LangChain 2.x is released.

"please install langchain-classic: pip install langchain-classic"
) from e

from langchain_core.language_models.llms import BaseLLM
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: Import path langchain_core.language_models.llms is deprecated. Use from langchain_core.language_models import BaseLLM instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants