Skip to content

refactor(llm): remove LangChain imports from core modules#1770

Merged
Pouyanpi merged 3 commits intodevelopfrom
feat/langchain-decouple/stack-5-remove-core-imports
Apr 16, 2026
Merged

refactor(llm): remove LangChain imports from core modules#1770
Pouyanpi merged 3 commits intodevelopfrom
feat/langchain-decouple/stack-5-remove-core-imports

Conversation

@Pouyanpi
Copy link
Copy Markdown
Collaborator

@Pouyanpi Pouyanpi commented Apr 7, 2026

Part of the LangChain decoupling stack:

  1. stack-1: canonical types (feat(types): add framework-agnostic LLM type system #1745)
  2. stack-2: adapter and framework registry (feat(llm): add LangChain adapter and framework registry #1759)
  3. stack-3: pipeline rewrite + caller migration (refactor(llm)!: atomic switch to LLMModel protocol #1760)
  4. stack-4: rename generate/stream to generate_async/stream_async (refactor(llm): rename generate/stream to generate_async/stream_async #1769)
  5. stack-5: remove LangChain imports from core modules (this PR)
  6. stack-6: move LangChain implementations into integrations/langchain/ (refactor(llm): move LangChain implementations into integrations/langchain/ #1772)
  7. stack-7: framework-owned provider registry (refactor(llm): framework-owned provider registry #1773)
  8. stack-8: framework-agnostic test infrastructure (TODO)

Description

  • Remove PromptTemplate from hallucination actions (was a no-op)
  • Replace Runnable isinstance check with duck-type ainvoke check in action_dispatcher
  • Remove PromptTemplate and llm.bind().invoke() from evaluate_factcheck, use LLMModel.generate_async via event loop
  • Make eval CLI LangChain cache a lazy optional import

Summary by CodeRabbit

  • Bug Fixes

    • LLM caching no longer causes startup failure if LangChain dependencies are unavailable; users receive a warning instead.
  • Refactor

    • Reduced direct dependency on specific LangChain APIs for improved flexibility and maintainability.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 7, 2026

Greptile Summary

This PR removes LangChain imports from four core modules as part of the larger LangChain decoupling stack. Changes include replacing isinstance(fn, Runnable) with a duck-type ainvoke check in action_dispatcher, removing the unused PromptTemplate from hallucination actions, migrating evaluate_factcheck to LLMModel.generate_async, and making the eval CLI's LangChain cache a lazy optional import.

Confidence Score: 5/5

Safe to merge; all remaining findings are P2 style suggestions that do not affect correctness.

The functional changes are correct: duck-type ainvoke check is a sound replacement for the Runnable isinstance guard, the PromptTemplate removal was a true no-op, and the evaluate_factcheck migration to generate_async matches the LLMModel protocol exactly. The only open finding is a narrow except clause in the eval CLI that leaves SQLiteCache init errors unhandled — a minor UX edge case that does not affect production guardrail logic.

nemoguardrails/eval/cli.py — the except ImportError scope around SQLiteCache initialization.

Important Files Changed

Filename Overview
nemoguardrails/actions/action_dispatcher.py Replaces isinstance(fn, Runnable) with a hasattr/callable duck-type check for ainvoke; correctly removes the LangChain import from the core dispatch path.
nemoguardrails/eval/cli.py Makes LangChain SQLiteCache a lazy optional import; except clause only covers ImportError, leaving SQLiteCache init errors unhandled.
nemoguardrails/evaluate/evaluate_factcheck.py Replaces langchain PromptTemplate + llm.bind().invoke() with direct LLMModel.generate_async; asyncio.run() is now used consistently throughout the module.
nemoguardrails/library/hallucination/actions.py Removes unused PromptTemplate import/usage (was a no-op); no functional change to the hallucination check logic.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[action_dispatcher.execute_action] --> B{fn is function/method?}
    B -- yes --> C[call fn directly / await if coroutine]
    B -- no --> D{hasattr fn.ainvoke AND callable?}
    D -- yes --> E[await fn.ainvoke input=params]
    D -- no --> F{has fn.run?}
    F -- yes --> G[call fn.run **params]
    F -- no --> H[raise Exception: no run method]

    subgraph evaluate_factcheck.py
        I[run] --> J{create_negatives?}
        J -- yes --> K[asyncio.run create_negative_samples\nuses LLMModel.generate_async directly]
        J -- no --> L[skip]
        K --> M[check_facts positive\nasyncio.run llm_call per sample]
        L --> M
        M --> N[check_facts negative\nasyncio.run llm_call per sample]
    end

    subgraph eval/cli.py check_compliance
        O[disable_llm_cache?] -- no --> P{import langchain?}
        P -- ImportError --> Q[warn: caching unavailable]
        P -- success --> R[set_llm_cache SQLiteCache\n⚠ non-ImportError not caught]
    end
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: nemoguardrails/eval/cli.py
Line: 173-180

Comment:
**Exception scope too narrow for SQLiteCache init**

Only `ImportError` is caught, but `SQLiteCache(database_path=".langchain.db")` can also raise `sqlite3.OperationalError` (no write permission to the cwd, disk full) or other OS errors. Those would propagate as an unhandled exception and crash the CLI instead of falling back gracefully. Consider broadening the catch or separating the import from the initialization.

```suggestion
    else:
        try:
            from langchain_community.cache import SQLiteCache
            from langchain_core.globals import set_llm_cache
        except ImportError:
            console.print("[yellow]langchain not installed, LLM caching unavailable.[/]")
        else:
            try:
                set_llm_cache(SQLiteCache(database_path=".langchain.db"))
                console.print("[green]Caching is enabled.[/]")
            except Exception as e:
                console.print(f"[yellow]LLM caching unavailable: {e}[/]")
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (8): Last reviewed commit: "apply review suggestion re event loop" | Re-trigger Greptile

Comment thread nemoguardrails/evaluate/evaluate_factcheck.py Outdated
Comment thread nemoguardrails/evaluate/evaluate_factcheck.py Outdated
@Pouyanpi Pouyanpi force-pushed the feat/langchain-decouple/stack-4-rename-async branch from b9bb707 to 41c9073 Compare April 7, 2026 16:24
@Pouyanpi Pouyanpi force-pushed the feat/langchain-decouple/stack-5-remove-core-imports branch from 3807a9a to 77864fa Compare April 7, 2026 16:24
@Pouyanpi Pouyanpi self-assigned this Apr 7, 2026
@Pouyanpi Pouyanpi force-pushed the feat/langchain-decouple/stack-5-remove-core-imports branch from 77864fa to f2a54ce Compare April 7, 2026 16:29
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 7, 2026

Codecov Report

❌ Patch coverage is 20.00000% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
nemoguardrails/eval/cli.py 0.00% 7 Missing ⚠️
nemoguardrails/evaluate/evaluate_factcheck.py 20.00% 4 Missing ⚠️
nemoguardrails/actions/action_dispatcher.py 50.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Collaborator

@tgasser-nv tgasser-nv left a comment

Choose a reason for hiding this comment

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

Looks good, just a minor logging nit

Comment thread nemoguardrails/eval/cli.py Outdated
@Pouyanpi Pouyanpi force-pushed the feat/langchain-decouple/stack-4-rename-async branch from 0991b48 to 61abe37 Compare April 14, 2026 10:13
@Pouyanpi Pouyanpi force-pushed the feat/langchain-decouple/stack-5-remove-core-imports branch from 514620d to 5ed106b Compare April 14, 2026 10:17
@Pouyanpi Pouyanpi force-pushed the feat/langchain-decouple/stack-4-rename-async branch from 61abe37 to 189e3f0 Compare April 16, 2026 07:26
Base automatically changed from feat/langchain-decouple/stack-4-rename-async to develop April 16, 2026 07:34
- Remove PromptTemplate from hallucination actions (was a no-op)
- Replace Runnable isinstance check with duck-type ainvoke check
  in action_dispatcher
- Remove PromptTemplate and llm.bind().invoke() from evaluate_factcheck,
  use LLMModel.generate_async via event loop
- Make eval CLI LangChain cache a lazy optional import
@Pouyanpi Pouyanpi force-pushed the feat/langchain-decouple/stack-5-remove-core-imports branch from 5ed106b to 54d6f0b Compare April 16, 2026 07:36
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 16, 2026

📝 Walkthrough

Walkthrough

These changes reduce coupling to LangChain by removing direct dependencies on PromptTemplate and Runnable, replacing them with duck-typing and direct string formatting. LangChain cache imports are made conditional with runtime error handling.

Changes

Cohort / File(s) Summary
Removed Runnable Type Dependency
nemoguardrails/actions/action_dispatcher.py
Replaced explicit isinstance(fn, Runnable) check with duck-typed detection of any callable exposing ainvoke method; calls await fn.ainvoke(input=params) with type-ignore annotations.
Removed PromptTemplate Usage
nemoguardrails/evaluate/evaluate_factcheck.py, nemoguardrails/library/hallucination/actions.py
Eliminated PromptTemplate dependency; replaced with direct str.format() for prompts. In evaluate_factcheck.py, LLM invocation now uses async event loop with generate_async() call instead of bound LLM wrapper.
Conditional LangChain Cache Imports
nemoguardrails/eval/cli.py
Moved SQLiteCache and set_llm_cache imports from module-level to runtime inside check_compliance, wrapped in try/except ImportError to gracefully handle missing LangChain dependencies with a warning message.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Test Results For Major Changes ⚠️ Warning PR contains major refactoring changes (removing LangChain imports, changing sync to async patterns) but the PR description lacks documented test results, test runs, or verification of correctness. Document test results in the PR description showing that the modified modules pass their test suites and verify no regressions were introduced by these architectural changes.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main objective of the PR—removing LangChain imports from core modules across four files.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/langchain-decouple/stack-5-remove-core-imports

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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/actions/action_dispatcher.py`:
- Around line 219-222: The duck-typed branch in action_dispatcher.py that
detects objects with an ainvoke attribute is calling await
fn.ainvoke(input=params), which breaks implementations whose ainvoke signature
expects positional args (e.g., ainvoke(self, messages,...)); change this to call
ainvoke with a positional parameter (e.g., await fn.ainvoke(params)) or try
positional first and fall back to the keyword form on TypeError so both callable
shapes are supported; update the elif branch that references fn and ainvoke and
ensure result is assigned from the successful call.

In `@nemoguardrails/evaluate/evaluate_factcheck.py`:
- Around line 92-103: The create_negative_samples function currently grabs an
event loop and calls loop.run_until_complete(self.llm.generate_async(...)),
which fails when an event loop is already running; change
create_negative_samples to be async (async def create_negative_samples(...)),
remove use of get_or_create_event_loop(), and replace
loop.run_until_complete(self.llm.generate_async(...)) with an await
self.llm.generate_async(formatted_prompt, temperature=0.8, max_tokens=300); then
update the CLI/entry point that calls create_negative_samples (similar to how
check_facts is invoked) to run it via asyncio.run(...) so the async function is
executed at the boundary without driving the loop inside the function. Ensure
references to create_negatives_template and self.llm.generate_async remain
unchanged except for using await.
🪄 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 Plus

Run ID: 46d33223-bb37-4924-a1c4-b07bf079aa90

📥 Commits

Reviewing files that changed from the base of the PR and between 9214d16 and 54d6f0b.

📒 Files selected for processing (4)
  • nemoguardrails/actions/action_dispatcher.py
  • nemoguardrails/eval/cli.py
  • nemoguardrails/evaluate/evaluate_factcheck.py
  • nemoguardrails/library/hallucination/actions.py

Comment thread nemoguardrails/actions/action_dispatcher.py
Comment thread nemoguardrails/evaluate/evaluate_factcheck.py Outdated
@Pouyanpi Pouyanpi merged commit 0a56d3e into develop Apr 16, 2026
7 checks passed
@Pouyanpi Pouyanpi deleted the feat/langchain-decouple/stack-5-remove-core-imports branch April 16, 2026 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants