-
Notifications
You must be signed in to change notification settings - Fork 21
fix: shared memory for evals #235
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 fixes shared memory handling for evaluations by centralizing the memory management and ensuring consistent state persistence across evaluation runs. The changes address issues with execution ID management and shared memory configuration.
Key changes:
- Introduces centralized connection string generation for SQLite state persistence
- Restructures evaluation middleware to properly initialize and share AsyncSqliteSaver memory across runtime contexts
- Removes hardcoded execution_id assignments in favor of default values
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/uipath_langchain/_cli/cli_run.py | Removes execution_id assignment and uses hardcoded "default" value for debug bridge |
| src/uipath_langchain/_cli/cli_eval.py | Adds shared memory initialization with AsyncSqliteSaver and restructures async flow |
| src/uipath_langchain/_cli/cli_debug.py | Removes execution_id assignment for consistency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| def get_connection_string(context: UiPathRuntimeContext) -> str: | ||
| if context.runtime_dir and context.state_file: | ||
| os.makedirs(context.runtime_dir, exist_ok=True) | ||
| return os.path.join(context.runtime_dir, context.state_file) |
Copilot
AI
Oct 23, 2025
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 fallback path '__uipath/state.db' does not ensure the directory exists before returning the path. This could cause a failure when AsyncSqliteSaver attempts to create the database file if the '__uipath' directory doesn't exist. Add os.makedirs('__uipath', exist_ok=True) before returning the fallback path.
| return os.path.join(context.runtime_dir, context.state_file) | |
| return os.path.join(context.runtime_dir, context.state_file) | |
| os.makedirs("__uipath", exist_ok=True) |
| _instrument_traceable_attributes() | ||
|
|
||
| event_bus = EventBus() | ||
|
|
Copilot
AI
Oct 23, 2025
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] Empty line 48 creates unnecessary whitespace before the function definition. Remove this blank line for cleaner code formatting.
Development Package