refactor(llm): remove LangChain imports from core modules#1770
refactor(llm): remove LangChain imports from core modules#1770
Conversation
Greptile SummaryThis PR removes LangChain imports from four core modules as part of the larger LangChain decoupling stack. Changes include replacing
|
| 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
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
b9bb707 to
41c9073
Compare
3807a9a to
77864fa
Compare
77864fa to
f2a54ce
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
41c9073 to
00cfc93
Compare
f2a54ce to
51565e8
Compare
00cfc93 to
0991b48
Compare
51565e8 to
514620d
Compare
tgasser-nv
left a comment
There was a problem hiding this comment.
Looks good, just a minor logging nit
0991b48 to
61abe37
Compare
514620d to
5ed106b
Compare
61abe37 to
189e3f0
Compare
- 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
5ed106b to
54d6f0b
Compare
📝 WalkthroughWalkthroughThese changes reduce coupling to LangChain by removing direct dependencies on Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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: 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
📒 Files selected for processing (4)
nemoguardrails/actions/action_dispatcher.pynemoguardrails/eval/cli.pynemoguardrails/evaluate/evaluate_factcheck.pynemoguardrails/library/hallucination/actions.py
Part of the LangChain decoupling stack:
Description
Summary by CodeRabbit
Bug Fixes
Refactor