Fix: Handle ToolNode in tools iteration for LangGraph compatibility#3946
Open
Utkarshkhandka wants to merge 3 commits intotraceloop:mainfrom
Open
Fix: Handle ToolNode in tools iteration for LangGraph compatibility#3946Utkarshkhandka wants to merge 3 commits intotraceloop:mainfrom
Utkarshkhandka wants to merge 3 commits intotraceloop:mainfrom
Conversation
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdjusted the agent-creation wrapper to robustly extract tool definitions from various Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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 the current code and only fix it if needed.
Inline comments:
In
`@packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/patch.py`:
- Around line 489-498: Fix the indentation and remove the duplicate function
call: ensure the branch that sets iterable_tools uses consistent indentation
(align the "iterable_tools =" lines under the initial if/elif/else block so
there are no stray or over-indented spaces) and eliminate the redundant call to
_extract_tool_definition by keeping a single assignment to tool_def inside the
for loop (for tool in iterable_tools: tool_def =
_extract_tool_definition(tool)). This touches the symbols tools, tools_by_name,
iterable_tools, _extract_tool_definition, tool_def, and the for tool in
iterable_tools loop.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 44ebe40d-aa4c-47d3-9157-3d155ca88773
📒 Files selected for processing (1)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/patch.py
...ges/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/patch.py
Show resolved
Hide resolved
Corrected comment punctuation and adjusted indentation for better readability. Streamlined tool extraction logic.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/patch.py (1)
487-504:⚠️ Potential issue | 🔴 CriticalCritical: Indentation errors will cause SyntaxError at import time.
The code has multiple indentation issues that break the module:
- Lines 489, 491, 493: The
if hasattr/elif/elseblock is at 12 spaces (same level asif tools:on line 487), making it execute regardless of whethertoolsis truthy.- Line 492: Has 17 spaces (one extra) instead of 20.
- Line 496: The
forloop is incorrectly nested inside theelse:block.- Lines 498, 500: Have 17 spaces (mismatched indentation) — Ruff confirms these as
invalid-syntax.This will raise
IndentationErrorwhen the module is imported, completely breaking the instrumentation.🐛 Proposed fix for indentation and structure
if tools: tool_definitions = [] - if hasattr(tools, "tools_by_name"): - iterable_tools = tools.tools_by_name.values() - elif isinstance(tools, (list, tuple, set)): - iterable_tools = tools - else: - iterable_tools = [tools] - - for tool in iterable_tools: + if hasattr(tools, "tools_by_name"): + iterable_tools = tools.tools_by_name.values() + elif isinstance(tools, (list, tuple, set)): + iterable_tools = tools + else: + iterable_tools = [tools] + + for tool in iterable_tools: tool_def = _extract_tool_definition(tool) - if tool_def: - tool_definitions.append(tool_def) - if tool_definitions: + if tool_def: + tool_definitions.append(tool_def) + if tool_definitions: span.set_attribute( GenAIAttributes.GEN_AI_TOOL_DEFINITIONS, json.dumps(tool_definitions) )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/patch.py` around lines 487 - 504, The block handling `tools` has broken indentation causing a SyntaxError; nest the `if hasattr(tools, "tools_by_name")` / `elif isinstance(tools, ...)` / `else` inside the `if tools:` branch so they only run when `tools` is truthy, ensure the `for tool in iterable_tools:` loop is at the same indentation level as those branches (not inside the `else`), and fix the misaligned lines that append `tool_def` (using `_extract_tool_definition(tool)`) and the final `span.set_attribute(GenAIAttributes.GEN_AI_TOOL_DEFINITIONS, json.dumps(tool_definitions))` so they execute after the loop when `tool_definitions` is non-empty; reference `tools`, `_extract_tool_definition`, `tool_definitions`, `span.set_attribute`, and `GenAIAttributes.GEN_AI_TOOL_DEFINITIONS` to locate and correct the indentation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/patch.py`:
- Around line 487-504: The block handling `tools` has broken indentation causing
a SyntaxError; nest the `if hasattr(tools, "tools_by_name")` / `elif
isinstance(tools, ...)` / `else` inside the `if tools:` branch so they only run
when `tools` is truthy, ensure the `for tool in iterable_tools:` loop is at the
same indentation level as those branches (not inside the `else`), and fix the
misaligned lines that append `tool_def` (using `_extract_tool_definition(tool)`)
and the final `span.set_attribute(GenAIAttributes.GEN_AI_TOOL_DEFINITIONS,
json.dumps(tool_definitions))` so they execute after the loop when
`tool_definitions` is non-empty; reference `tools`, `_extract_tool_definition`,
`tool_definitions`, `span.set_attribute`, and
`GenAIAttributes.GEN_AI_TOOL_DEFINITIONS` to locate and correct the indentation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 95c78da3-f0c1-4a4b-91ff-cc9c55d214ac
📒 Files selected for processing (1)
packages/opentelemetry-instrumentation-langchain/opentelemetry/instrumentation/langchain/patch.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When using
langgraph-supervisor,create_react_agentmay pass aToolNodeobject astools. SinceToolNodeis not directly iterable, the current loop inpatch.pyraises:Fixes #3921
Root Cause
ToolNodestores tools internally viatools_by_name(a dict), not as a direct iterable — sofor tool in toolsfails.Fix
Handles
ToolNode, standard iterables, and single tool objects — fully backward compatible.Testing
Impact
langgraph-supervisor+create_react_agentfeat(instrumentation): ...orfix(instrumentation): ....Summary by CodeRabbit