-
Notifications
You must be signed in to change notification settings - Fork 17
Fix: extract output with streaming #191
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
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.
Pull Request Overview
This PR extracts output with streaming support by refactoring the runtime architecture to use a more flexible graph resolution system. The key changes include separating graph resolution from runtime execution and improving streaming capabilities.
- Introduces a new
GraphResolver
class to handle graph configuration and loading - Replaces
LangGraphRuntime
withLangGraphScriptRuntime
that uses the resolver pattern - Improves streaming output extraction by handling both messages and updates in the same stream
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/uipath_langchain/_cli/cli_run.py |
Updates imports and runtime factory to use LangGraphScriptRuntime |
src/uipath_langchain/_cli/cli_eval.py |
Updates runtime usage and adds runtime generator function |
src/uipath_langchain/_cli/cli_dev.py |
Updates to use new runtime class with generator function |
src/uipath_langchain/_cli/_runtime/_runtime.py |
Major refactor: extracts graph resolution, improves streaming, creates base and script runtime classes |
src/uipath_langchain/_cli/_runtime/_graph_resolver.py |
New file implementing graph resolution logic extracted from runtime |
src/uipath_langchain/_cli/_runtime/_context.py |
Removes graph-related fields from runtime context |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
processed_input, | ||
graph_config, | ||
stream_mode="updates", | ||
stream_mode=["messages", "updates"], |
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.
[nitpick] The stream mode list contains both 'messages' and 'updates' but the code only handles these two specific modes. Consider using a constant or enum to make the supported stream modes more explicit and maintainable.
Copilot uses AI. Check for mistakes.
def _extract_graph_result( | ||
self, final_chunk, graph: CompiledStateGraph[Any, Any, Any] | ||
): | ||
def _extract_graph_result(self, final_chunk, output_channels: str | Sequence[str]): |
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.
The parameter output_channels
should have a type annotation that includes None
since the method checks for None
values implicitly through the isinstance checks, or add explicit None handling.
def _extract_graph_result(self, final_chunk, output_channels: str | Sequence[str]): | |
def _extract_graph_result(self, final_chunk, output_channels: Optional[str | Sequence[str]]): |
Copilot uses AI. Check for mistakes.
self.graph_config = None | ||
|
||
|
||
AsyncResolver = Callable[[], Awaitable[StateGraph[Any, Any, Any]]] |
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.
The AsyncResolver
type alias is duplicated in both _runtime.py
(line 26) and _graph_resolver.py
(line 105). Consider defining it once in a shared location to avoid duplication.
Copilot uses AI. Check for mistakes.
3e2a12e
to
c434555
Compare
Development Package