fix: remove ConfigurableRuntimeFactory, read settings from schema.metadata#1061
fix: remove ConfigurableRuntimeFactory, read settings from schema.metadata#1061
Conversation
…adata Remove the incorrect implementation that wrapped the factory and created temporary files. Instead, read agent settings from schema.metadata which is populated by the low-code agents runtime. ### Changes - Remove ConfigurableRuntimeFactory class and tests - Update _get_agent_model() to read from schema.metadata["settings"]["model"] - Keep fallback to protocol for backwards compatibility - Update uipath-runtime dependency to >=0.3.4 (includes metadata field) - Remove temp file cleanup from __aexit__ ### Why this change? PR #1053 introduced a design that broke separation of concerns: - Wrapped the factory unnecessarily - Deserialized agent.json in the CLI layer - Created temporary modified JSON files - Required cleanup of temp files The correct design (per @cristipufu feedback): - Low-code repo populates schema.metadata with settings - CLI reads settings from schema.metadata - No temp files, no factory wrapper, no JSON deserialization in CLI 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32611a1341
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async def execute(self) -> UiPathRuntimeResult: | ||
| # Configure model settings override before creating runtime | ||
| await self._configure_model_settings_override() | ||
|
|
||
| runtime = await self.factory.new_runtime( | ||
| entrypoint=self.context.entrypoint or "", | ||
| runtime_id=self.execution_id, |
There was a problem hiding this comment.
Honor model settings override when creating runtime
The execute() path now constructs the runtime directly (factory.new_runtime at lines 286‑289) but no longer applies the requested model settings override. _get_model_settings_override() (lines 554‑601) is never invoked, so the --model-settings-id option wired through cli_eval is a no-op and evaluation runs always use the agent’s default settings. Users selecting a non‑default model/temperature in the evaluation set will silently execute against the wrong model.
Useful? React with 👍 / 👎.
|
handled by @mathurk's PR |
Summary
This PR fixes the design introduced in PR #1053 by removing
ConfigurableRuntimeFactoryand reading agent settings fromschema.metadatainstead.Problem with PR #1053
The previous implementation:
agent.jsonin the CLI layerPer feedback from @cristipufu:
The correct architecture
Changes
_configurable_factory.py- factory wrapper that created temp filestest_configurable_factory.py- tests for removed class_runtime.py:_get_agent_model()now reads fromschema.metadata["settings"]["model"]__init____aexit___configure_model_settings_override()to_get_model_settings_override()pyproject.toml-uipath-runtime>=0.3.4(includesmetadatafield)Related PR
Test plan
🤖 Generated with Claude Code