-
Notifications
You must be signed in to change notification settings - Fork 31
fix: thread RunnableConfig to analyze_files LLM call for proper span parenting #552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…parenting Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
src/uipath_langchain/agent/tools/internal_tools/analyze_files_tool.py
Outdated
Show resolved
Hide resolved
…hierarchy Thread RunnableConfig from LangGraph node through wrapper chain to tool.ainvoke() so inner LLM calls (e.g. Analyze_Files) produce correctly nested spans instead of orphaned siblings. - tool_node.py: accept config param, pass to tool.invoke() or set contextvar - job_attachment_wrapper.py: read config from contextvar, pass to tool.ainvoke() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| config = var_child_runnable_config.get(None) | ||
| tool_result = await tool.ainvoke(call, config=config) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we have this system in place? why even pass it through the structured tool? We could retrieve it like this right before the LLM invoke. Is there any reason why we need to pass it through structured tool? Does it affect the way the Tool spans integrate with the LLM span?
I have certain opinions on Langchain's system with RunnableConfig being passed everywhere (though it's nowhere near as an offender as ToolRuntime in that regard), so i'm reluctant to mix it in with our node.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call! Now we just read var_child_runnable_config.get(None) directly in analyze_files_tool.py right before llm.ainvoke(). No RunnableConfig in the node/wrapper layer at all.
…les_tool Instead of threading RunnableConfig through tool_node.py and wrappers, read it from the LangGraph-managed context variable where it's needed. This simplifies the code and avoids mixing config into the tool node layer. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts: # pyproject.toml # uv.lock
Summary
RunnableConfigfromBaseToolthroughtool_fntollm.ainvoke()in the Analyze Files toolparent_run_idwhentool_fndeclares aconfig: RunnableConfigparamTest plan
🤖 Generated with Claude Code