From 32611a134111bf96a0152361ce31ec796f4c5a77 Mon Sep 17 00:00:00 2001 From: Chibi Vikram Date: Fri, 2 Jan 2026 19:38:32 -0800 Subject: [PATCH] fix: remove ConfigurableRuntimeFactory, read settings from schema.metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- pyproject.toml | 2 +- .../_cli/_evals/_configurable_factory.py | 167 ---------------- src/uipath/_cli/_evals/_runtime.py | 52 +++-- tests/cli/eval/test_configurable_factory.py | 183 ------------------ uv.lock | 8 +- 5 files changed, 36 insertions(+), 376 deletions(-) delete mode 100644 src/uipath/_cli/_evals/_configurable_factory.py delete mode 100644 tests/cli/eval/test_configurable_factory.py diff --git a/pyproject.toml b/pyproject.toml index 1ec424b61..56b4ecb16 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,7 @@ readme = { file = "README.md", content-type = "text/markdown" } requires-python = ">=3.11" dependencies = [ "uipath-core>=0.1.4, <0.2.0", - "uipath-runtime>=0.3.1, <0.4.0", + "uipath-runtime>=0.3.4, <0.4.0", "click>=8.3.1", "httpx>=0.28.1", "pyjwt>=2.10.1", diff --git a/src/uipath/_cli/_evals/_configurable_factory.py b/src/uipath/_cli/_evals/_configurable_factory.py deleted file mode 100644 index 6ae473dc1..000000000 --- a/src/uipath/_cli/_evals/_configurable_factory.py +++ /dev/null @@ -1,167 +0,0 @@ -"""Configurable runtime factory that supports model settings overrides.""" - -import json -import logging -import os -import tempfile -from pathlib import Path - -from uipath.runtime import UiPathRuntimeFactoryProtocol, UiPathRuntimeProtocol - -from ._models._evaluation_set import EvaluationSetModelSettings - -logger = logging.getLogger(__name__) - - -class ConfigurableRuntimeFactory: - """Wrapper factory that supports model settings overrides for evaluation runs. - - This factory wraps an existing UiPathRuntimeFactoryProtocol implementation - and allows applying model settings overrides when creating runtimes. - """ - - def __init__(self, base_factory: UiPathRuntimeFactoryProtocol): - """Initialize with a base factory to wrap.""" - self.base_factory = base_factory - self.model_settings_override: EvaluationSetModelSettings | None = None - self._temp_files: list[str] = [] - - def set_model_settings_override( - self, settings: EvaluationSetModelSettings | None - ) -> None: - """Set model settings to override when creating runtimes. - - Args: - settings: The model settings to apply, or None to clear overrides - """ - self.model_settings_override = settings - - async def new_runtime( - self, entrypoint: str, runtime_id: str - ) -> UiPathRuntimeProtocol: - """Create a new runtime with optional model settings overrides. - - If model settings override is configured, creates a temporary modified - entrypoint file with the overridden settings. - - Args: - entrypoint: Path to the agent entrypoint file - runtime_id: Unique identifier for the runtime instance - - Returns: - A new runtime instance with overrides applied if configured - """ - # If no overrides, delegate directly to base factory - if not self.model_settings_override: - return await self.base_factory.new_runtime(entrypoint, runtime_id) - - # Apply overrides by creating modified entrypoint - modified_entrypoint = self._apply_overrides( - entrypoint, self.model_settings_override - ) - if modified_entrypoint: - # Track temp file for cleanup - self._temp_files.append(modified_entrypoint) - return await self.base_factory.new_runtime(modified_entrypoint, runtime_id) - - # If override failed, fall back to original - return await self.base_factory.new_runtime(entrypoint, runtime_id) - - def _apply_overrides( - self, entrypoint: str, settings: EvaluationSetModelSettings - ) -> str | None: - """Apply model settings overrides to an agent entrypoint. - - Creates a temporary modified version of the entrypoint file with - the specified model settings overrides applied. - - Args: - entrypoint: Path to the original entrypoint file - settings: Model settings to override - - Returns: - Path to temporary modified entrypoint, or None if override not needed/failed - """ - if ( - settings.model == "same-as-agent" - and settings.temperature == "same-as-agent" - ): - logger.debug( - "Both model and temperature are 'same-as-agent', no override needed" - ) - return None - - entrypoint_path = Path(entrypoint) - if not entrypoint_path.exists(): - logger.warning(f"Entrypoint file '{entrypoint_path}' not found") - return None - - try: - with open(entrypoint_path, "r") as f: - agent_data = json.load(f) - except (json.JSONDecodeError, IOError) as e: - logger.error(f"Failed to load entrypoint file: {e}") - return None - - original_settings = agent_data.get("settings", {}) - modified_settings = original_settings.copy() - - # Override model if not "same-as-agent" - if settings.model != "same-as-agent": - modified_settings["model"] = settings.model - logger.debug( - f"Overriding model: {original_settings.get('model')} -> {settings.model}" - ) - - # Override temperature if not "same-as-agent" - if settings.temperature not in ["same-as-agent", None]: - if isinstance(settings.temperature, (int, float)): - modified_settings["temperature"] = float(settings.temperature) - elif isinstance(settings.temperature, str): - try: - modified_settings["temperature"] = float(settings.temperature) - except ValueError: - logger.warning( - f"Invalid temperature value: '{settings.temperature}'" - ) - - if "temperature" in modified_settings: - logger.debug( - f"Overriding temperature: {original_settings.get('temperature')} -> " - f"{modified_settings['temperature']}" - ) - - if modified_settings == original_settings: - return None - - agent_data["settings"] = modified_settings - - # Create a temporary file with the modified agent definition - try: - temp_fd, temp_path = tempfile.mkstemp( - suffix=".json", prefix="agent_override_" - ) - with os.fdopen(temp_fd, "w") as temp_file: - json.dump(agent_data, temp_file, indent=2) - - logger.info(f"Created temporary entrypoint with overrides: {temp_path}") - return temp_path - except Exception as e: - logger.error(f"Failed to create temporary entrypoint file: {e}") - return None - - async def dispose(self) -> None: - """Dispose resources and clean up temporary files.""" - # Clean up any temporary files created - for temp_file in self._temp_files: - try: - os.unlink(temp_file) - logger.debug(f"Cleaned up temporary file: {temp_file}") - except Exception as e: - logger.warning(f"Failed to clean up temporary file {temp_file}: {e}") - - self._temp_files.clear() - - # Delegate disposal to base factory - if hasattr(self.base_factory, "dispose"): - await self.base_factory.dispose() diff --git a/src/uipath/_cli/_evals/_runtime.py b/src/uipath/_cli/_evals/_runtime.py index ef5f709cf..abdb086ca 100644 --- a/src/uipath/_cli/_evals/_runtime.py +++ b/src/uipath/_cli/_evals/_runtime.py @@ -56,7 +56,6 @@ from ...eval.models.models import AgentExecution, EvalItemResult from .._utils._eval_set import EvalHelpers from .._utils._parallelization import execute_parallel -from ._configurable_factory import ConfigurableRuntimeFactory from ._evaluator_factory import EvaluatorFactory from ._models._evaluation_set import ( EvaluationItem, @@ -199,8 +198,7 @@ def __init__( event_bus: EventBus, ): self.context: UiPathEvalContext = context - # Wrap the factory to support model settings overrides - self.factory = ConfigurableRuntimeFactory(factory) + self.factory: UiPathRuntimeFactoryProtocol = factory self.event_bus: EventBus = event_bus self.trace_manager: UiPathTraceManager = trace_manager self.span_exporter: ExecutionSpanExporter = ExecutionSpanExporter() @@ -225,10 +223,6 @@ async def __aexit__(self, *args: Any) -> None: self.coverage.stop() self.coverage.report(include=["./*"], show_missing=True) - # Clean up any temporary files created by the factory - if hasattr(self.factory, "dispose"): - await self.factory.dispose() - async def get_schema(self, runtime: UiPathRuntimeProtocol) -> UiPathRuntimeSchema: schema = await runtime.get_schema() if schema is None: @@ -290,9 +284,6 @@ async def initiate_evaluation( ) 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, @@ -560,14 +551,21 @@ def _get_and_clear_execution_data( return spans, logs - async def _configure_model_settings_override(self) -> None: - """Configure the factory with model settings override if specified.""" - # Skip if no model settings ID specified + def _get_model_settings_override( + self, + ) -> dict[str, Any] | None: + """Get model settings override from evaluation set if specified. + + Returns: + Model settings dict to use for override, or None if using defaults. + Settings are passed via schema.metadata to the runtime. + """ + # Skip if no model settings ID specified or using default if ( not self.context.model_settings_id or self.context.model_settings_id == "default" ): - return + return None # Load evaluation set to get model settings evaluation_set, _ = EvalHelpers.load_eval_set(self.context.eval_set or "") @@ -576,7 +574,7 @@ async def _configure_model_settings_override(self) -> None: or not evaluation_set.model_settings ): logger.warning("No model settings available in evaluation set") - return + return None # Find the specified model settings target_model_settings = next( @@ -592,15 +590,15 @@ async def _configure_model_settings_override(self) -> None: logger.warning( f"Model settings ID '{self.context.model_settings_id}' not found in evaluation set" ) - return + return None logger.info( - f"Configuring model settings override: id='{target_model_settings.id}', " + f"Using model settings override: id='{target_model_settings.id}', " f"model='{target_model_settings.model}', temperature='{target_model_settings.temperature}'" ) - # Configure the factory with the override settings - self.factory.set_model_settings_override(target_model_settings) + # Return settings as dict for schema.metadata override + return target_model_settings.model_dump(exclude_none=True) async def execute_runtime( self, @@ -687,15 +685,27 @@ async def run_evaluator( return result async def _get_agent_model(self, runtime: UiPathRuntimeProtocol) -> str | None: - """Get agent model from the runtime. + """Get agent model from the runtime schema metadata. + + The model is read from schema.metadata["settings"]["model"] which is + populated by the low-code agents runtime from agent.json. Returns: The model name from agent settings, or None if not found. """ try: + schema = await self.get_schema(runtime) + if schema.metadata and "settings" in schema.metadata: + settings = schema.metadata["settings"] + model = settings.get("model") + if model: + logger.debug(f"Got agent model from schema.metadata: {model}") + return model + + # Fallback to protocol-based approach for backwards compatibility model = self._find_agent_model_in_runtime(runtime) if model: - logger.debug(f"Got agent model from runtime: {model}") + logger.debug(f"Got agent model from runtime protocol: {model}") return model except Exception: return None diff --git a/tests/cli/eval/test_configurable_factory.py b/tests/cli/eval/test_configurable_factory.py deleted file mode 100644 index 690ef90aa..000000000 --- a/tests/cli/eval/test_configurable_factory.py +++ /dev/null @@ -1,183 +0,0 @@ -"""Tests for ConfigurableRuntimeFactory.""" - -import json -import tempfile -from pathlib import Path -from unittest.mock import AsyncMock, Mock - -import pytest - -from uipath._cli._evals._configurable_factory import ConfigurableRuntimeFactory -from uipath._cli._evals._models._evaluation_set import EvaluationSetModelSettings - - -@pytest.mark.asyncio -async def test_configurable_factory_no_override(): - """Test factory without any overrides.""" - mock_base_factory = AsyncMock() - mock_runtime = Mock() - mock_base_factory.new_runtime.return_value = mock_runtime - - factory = ConfigurableRuntimeFactory(mock_base_factory) - - result = await factory.new_runtime("test.json", "test-id") - - assert result == mock_runtime - mock_base_factory.new_runtime.assert_called_once_with("test.json", "test-id") - - -@pytest.mark.asyncio -async def test_configurable_factory_with_model_override(): - """Test factory with model override.""" - # Create a temporary agent.json file - test_agent = {"settings": {"model": "gpt-4", "temperature": 0.7}} - - with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: - json.dump(test_agent, f) - temp_path = f.name - - try: - mock_base_factory = AsyncMock() - mock_runtime = Mock() - mock_base_factory.new_runtime.return_value = mock_runtime - - factory = ConfigurableRuntimeFactory(mock_base_factory) - - # Set model override - settings = EvaluationSetModelSettings( - id="test-settings", model="gpt-3.5-turbo", temperature="same-as-agent" - ) - factory.set_model_settings_override(settings) - - result = await factory.new_runtime(temp_path, "test-id") - - assert result == mock_runtime - # Should have been called with a modified temp file - call_args = mock_base_factory.new_runtime.call_args - assert call_args[0][0] != temp_path # Different path (temp file) - assert call_args[0][1] == "test-id" - - # Verify the temp file has correct content - with open(call_args[0][0]) as f: - modified_data = json.load(f) - assert modified_data["settings"]["model"] == "gpt-3.5-turbo" - assert modified_data["settings"]["temperature"] == 0.7 # Unchanged - - finally: - Path(temp_path).unlink(missing_ok=True) - # Clean up temp files created by factory - await factory.dispose() - - -@pytest.mark.asyncio -async def test_configurable_factory_same_as_agent(): - """Test factory when both settings are 'same-as-agent'.""" - # Create a temporary agent.json file - test_agent = {"settings": {"model": "gpt-4", "temperature": 0.7}} - - with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: - json.dump(test_agent, f) - temp_path = f.name - - try: - mock_base_factory = AsyncMock() - mock_runtime = Mock() - mock_base_factory.new_runtime.return_value = mock_runtime - - factory = ConfigurableRuntimeFactory(mock_base_factory) - - # Set "same-as-agent" for both - settings = EvaluationSetModelSettings( - id="test-settings", model="same-as-agent", temperature="same-as-agent" - ) - factory.set_model_settings_override(settings) - - result = await factory.new_runtime(temp_path, "test-id") - - assert result == mock_runtime - # Should use original path (no override) - mock_base_factory.new_runtime.assert_called_once_with(temp_path, "test-id") - - finally: - Path(temp_path).unlink(missing_ok=True) - - -@pytest.mark.asyncio -async def test_configurable_factory_temperature_override(): - """Test factory with temperature override.""" - # Create a temporary agent.json file - test_agent = {"settings": {"model": "gpt-4", "temperature": 0.7}} - - with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: - json.dump(test_agent, f) - temp_path = f.name - - try: - mock_base_factory = AsyncMock() - mock_runtime = Mock() - mock_base_factory.new_runtime.return_value = mock_runtime - - factory = ConfigurableRuntimeFactory(mock_base_factory) - - # Set temperature override - settings = EvaluationSetModelSettings( - id="test-settings", model="same-as-agent", temperature=0.2 - ) - factory.set_model_settings_override(settings) - - result = await factory.new_runtime(temp_path, "test-id") - - assert result == mock_runtime - # Should have been called with a modified temp file - call_args = mock_base_factory.new_runtime.call_args - assert call_args[0][0] != temp_path # Different path (temp file) - - # Verify the temp file has correct content - with open(call_args[0][0]) as f: - modified_data = json.load(f) - assert modified_data["settings"]["model"] == "gpt-4" # Unchanged - assert modified_data["settings"]["temperature"] == 0.2 # Changed - - finally: - Path(temp_path).unlink(missing_ok=True) - await factory.dispose() - - -@pytest.mark.asyncio -async def test_configurable_factory_cleanup(): - """Test that temporary files are cleaned up.""" - test_agent = {"settings": {"model": "gpt-4", "temperature": 0.7}} - - with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: - json.dump(test_agent, f) - temp_path = f.name - - try: - mock_base_factory = AsyncMock() - mock_runtime = Mock() - mock_base_factory.new_runtime.return_value = mock_runtime - - factory = ConfigurableRuntimeFactory(mock_base_factory) - - settings = EvaluationSetModelSettings( - id="test-settings", model="gpt-3.5-turbo", temperature=0.5 - ) - factory.set_model_settings_override(settings) - - await factory.new_runtime(temp_path, "test-id") - - # Get the temp file created - call_args = mock_base_factory.new_runtime.call_args - temp_file_created = call_args[0][0] - - # Temp file should exist - assert Path(temp_file_created).exists() - - # Clean up - await factory.dispose() - - # Temp file should be deleted - assert not Path(temp_file_created).exists() - - finally: - Path(temp_path).unlink(missing_ok=True) diff --git a/uv.lock b/uv.lock index c5f9eadf5..0c8b87fc0 100644 --- a/uv.lock +++ b/uv.lock @@ -2542,7 +2542,7 @@ requires-dist = [ { name = "tenacity", specifier = ">=9.0.0" }, { name = "truststore", specifier = ">=0.10.1" }, { name = "uipath-core", specifier = ">=0.1.4,<0.2.0" }, - { name = "uipath-runtime", specifier = ">=0.3.1,<0.4.0" }, + { name = "uipath-runtime", specifier = ">=0.3.4,<0.4.0" }, ] [package.metadata.requires-dev] @@ -2588,14 +2588,14 @@ wheels = [ [[package]] name = "uipath-runtime" -version = "0.3.1" +version = "0.3.4" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "uipath-core" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/78/12/01a12e2503d8ae312d7617cbdb01f6d1a05f0c89fc11e19e617cee2c60ed/uipath_runtime-0.3.1.tar.gz", hash = "sha256:cb4c891b2b3fafb6b0c763a6ac8c611f690d63d451552a7fd07ed307b57023f4", size = 99320, upload-time = "2025-12-26T06:37:38.7Z" } +sdist = { url = "https://files.pythonhosted.org/packages/33/d5/2cd213db4950ae08473460eb803592af79409ecc7946e7ef0470689251a3/uipath_runtime-0.3.4.tar.gz", hash = "sha256:7516161631ea5c2cfe1925ca7711a400bccdc5e5ace5ebd7be918ba8a43f23f0", size = 99572, upload-time = "2025-12-30T17:24:16.851Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/b6/c1/763f014cd893d3d2040f9d5a1f5b6d3e8086b42861c7fc338922daa98cd8/uipath_runtime-0.3.1-py3-none-any.whl", hash = "sha256:4e8c7c8ae5a736eabf2c0283b69c477b0fa90581b56466f3edbb50593d8f8617", size = 38085, upload-time = "2025-12-26T06:37:37.226Z" }, + { url = "https://files.pythonhosted.org/packages/b1/58/536c0095528448f243156713d69e3013b23ba5b5b0a1b38cc8803dc2bcb3/uipath_runtime-0.3.4-py3-none-any.whl", hash = "sha256:074efb9274eca159989be01c4a00a6a5fbafbfad080856baacf64c1796a2931f", size = 38353, upload-time = "2025-12-30T17:24:15.617Z" }, ] [[package]]